Michael Chihlas
196c003876
Design doc for the FlowPilot migration's remaining open items: - NoTemplateDialog narrow-lane bug (resolved by moving the dialog to the chat region alongside ProposalBanner — three cards fit naturally at that width; grid-cols fix no longer needed) - Tabbed Script Builder inside the chat (new [Chat] [Script Builder ●] tab strip; AI chat default with 'Write it myself' Monaco escape hatch) Plus a Phase 8 cleanup: - EscalateInterceptDialog fourth 'I applied some of it — partial' choice All six architecture decisions settled via brainstorming before writing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
297 lines
16 KiB
Markdown
297 lines
16 KiB
Markdown
# FlowPilot Phase 9 — Tabbed Script Builder + NoTemplateDialog relocation
|
||
|
||
**Date:** 2026-04-23
|
||
**Branch target:** `feat/flowpilot-migration` (continuation of Phases 0–8)
|
||
**Depends on:** Phase 8 (ProposalBanner in chat region)
|
||
|
||
---
|
||
|
||
## Goal
|
||
|
||
Close the two remaining open items from the FlowPilot migration handoff:
|
||
|
||
1. **NoTemplateDialog narrow-lane bug** — today the dialog renders in the task lane (~340px) and its `grid-cols-1 sm:grid-cols-3` layout crushes the three option cards. When the AI proposes a fix with no drafted script, all three cards render disabled, producing a dead end.
|
||
2. **Tabbed Script Builder inside the chat** — give the engineer a way to draft the missing script without leaving the session (either by chatting with the AI or hand-writing in a code editor), then feed the draft back into the existing fix lifecycle.
|
||
|
||
Plus one Phase 8 cleanup item flagged during code review:
|
||
|
||
3. **`EscalateInterceptDialog` missing the Partial choice** — if a fix is in `applied_partial` when the engineer escalates, the intercept dialog's current three choices (worked / didn't work / never applied) don't match. Add a fourth choice for partial.
|
||
|
||
This phase depends on Phase 8's `ProposalBanner` already being in the chat region — it reuses the same "chat-region-owns-Apply-flow" philosophy.
|
||
|
||
---
|
||
|
||
## Architectural decisions (settled during brainstorming)
|
||
|
||
| # | Decision | Rationale |
|
||
|---|---|---|
|
||
| 1 | When a fix has no `ai_drafted_script`, the banner's Apply button routes **directly** to the Script Builder tab (bypassing `NoTemplateDialog` entirely). | Banner is the single entry point for Apply. `NoTemplateDialog` stays narrowly scoped to evaluating a draft that actually exists. |
|
||
| 2 | Inside the Script Builder tab, the default experience is **`ScriptBuilderChat` (AI-driven)**. A "✎ Write it myself" button in the tab's header toolbar swaps into a Monaco editor. | AI is the common path. Manual editing is an escape hatch for engineers who already know what they want. |
|
||
| 3 | The manual editor uses **Monaco**, reusing the pattern from `frontend/src/components/tree-editor/code-mode/CodeModeEditor.tsx`. | Monaco is already a dependency (`@monaco-editor/react` + `monaco-editor`). No bundle cost, proven pattern. |
|
||
| 4 | The Script Builder tab is **always present while the fix is non-terminal** (no close affordance). An **indicator dot** on the tab signals in-progress draft state. | Matches Phase 8's `display: none` philosophy — engineers move freely between chat and draft without tracking a separate open/close state. |
|
||
| 5 | `NoTemplateDialog` (draft-exists case) moves from `TaskLane.bottomSlot` to the **chat region** (sibling of `ProposalBanner`, slides up above composer). | Script evaluation is an action surface, not a context surface — belongs with the other action surfaces. Chat region is wide enough for the three cards to actually fit side-by-side. |
|
||
| 6 | `EscalateInterceptDialog` gains a **fourth "Partial" choice** that writes `applied_partial` with a notes prompt. | Closes the gap flagged in Phase 8 final review. Minimal incremental cost since the dialog is already getting touched. |
|
||
|
||
---
|
||
|
||
## Architecture
|
||
|
||
### 1. Chat region gets a tab strip
|
||
|
||
A two-tab strip at the top of the chat region:
|
||
|
||
```
|
||
┌──────────────────────────────────────┐
|
||
│ [Chat] [Script Builder ●] │
|
||
├──────────────────────────────────────┤
|
||
│ │
|
||
│ (content per active tab, │
|
||
│ via display:none toggling) │
|
||
│ │
|
||
└──────────────────────────────────────┘
|
||
```
|
||
|
||
- **When the strip renders:** only when an `activeFix` exists AND the fix is non-terminal AND (`fix.ai_drafted_script` is null AND `fix.script_template_id` is null) — i.e., the fix genuinely needs a script drafted. Otherwise the chat region shows without tabs.
|
||
- **Tab switching uses `display: none`**, not unmount. Chat scroll position, draft message, and Script Builder state all persist across switches.
|
||
- **Indicator dot** on the Script Builder tab fires when there's in-progress draft state: at least one AI message sent in the `ScriptBuilderChat`, or non-empty Monaco buffer. Clears when the draft is submitted.
|
||
- **Session switch** clears tab state via the existing `resetSessionDerivedState` helper.
|
||
|
||
### 2. Script Builder tab content
|
||
|
||
Default mode is `ScriptBuilderChat` embedded inside the tab. A header toolbar above the chat hosts the mode toggle:
|
||
|
||
```
|
||
┌──────────────────────────────────────┐
|
||
│ Script Builder · Outlook fix │
|
||
│ [✎ Write myself]│
|
||
├──────────────────────────────────────┤
|
||
│ (mode-specific content) │
|
||
│ │
|
||
└──────────────────────────────────────┘
|
||
```
|
||
|
||
- Clicking **✎ Write myself** flips `scriptBuilderMode` to `'editor'` — Monaco renders in place of `ScriptBuilderChat`, pre-loaded with a scaffold (fix description as a language-appropriate comment header + an empty body).
|
||
- A reciprocal **✨ Back to AI** button in editor mode returns to the chat.
|
||
- Switching modes **does not discard** work. The Monaco buffer and the `ScriptBuilderChat` session both persist across toggles. This matters when an engineer drafts with AI, switches to editor to tweak a line, then considers going back.
|
||
- Both modes share a single terminal action: **Submit → `PATCH /ai-sessions/{sid}/suggested-fixes/{fid}/script`**. On success the fix now has `ai_drafted_script`; the tab strip disappears (since the fix no longer needs a script) and the banner's Apply button now routes to `NoTemplateDialog` in the chat region.
|
||
|
||
### 3. NoTemplateDialog relocation to chat region
|
||
|
||
- Removed from `TaskLane.bottomSlot`. Renders in the chat region, slide-up-above-composer (same mechanical placement as `ProposalBanner`).
|
||
- The three-card layout (`grid-cols-3` at the chat region's natural width) actually fits — no `grid-cols-1` regression needed.
|
||
- Opens when the engineer clicks Apply on the banner AND `fix.ai_drafted_script` is non-empty.
|
||
- Existing `handleScriptDecision` logic unchanged; only the render location moves.
|
||
|
||
### 4. Banner Apply routing (updated)
|
||
|
||
Three mutually-exclusive outcomes based on the fix's shape:
|
||
|
||
```
|
||
handleApplyFix():
|
||
if fix.script_template_id:
|
||
open TemplateMatchPanel (unchanged — still renders in task lane for now)
|
||
elif fix.ai_drafted_script:
|
||
open NoTemplateDialog in chat region (new location, Chat tab)
|
||
else:
|
||
open Script Builder tab in chat region (new tab)
|
||
```
|
||
|
||
The NoTemplateDialog-in-chat-region path lives on the **Chat tab** (slides up above composer; the tab strip only renders for the no-draft case, so when NoTemplateDialog shows, the tab strip is not on screen). The Script Builder tab path is the opposite — tab strip renders, engineer is on the Script Builder tab.
|
||
|
||
`TemplateMatchPanel` stays in the task lane for this phase — it's a different surface with different interactions and it's not broken. Moving it is possible future work.
|
||
|
||
### 5. EscalateInterceptDialog partial choice
|
||
|
||
Adds a fourth button to the existing popover:
|
||
|
||
| Existing choices | New choice |
|
||
|---|---|
|
||
| The fix didn't work | (existing) |
|
||
| It worked — escalating for another reason | (existing) |
|
||
| Never actually applied it | (existing) |
|
||
| **I applied some of it — partial** | **NEW** |
|
||
|
||
- When clicked: prompts for partial notes (same pattern as the banner's Partial path — `window.prompt` for now, matching Phase 8's interim), then calls `patchOutcome('applied_partial', notes)`.
|
||
- `handleInterceptChoice` gains an `applied_partial` branch. The `InterceptChoice` type already includes `'applied_partial'` via `FixOutcome | 'never_applied'`, so no type changes needed.
|
||
- When a fix enters the dialog already in `applied_partial` state, the fourth button is hidden (can't transition partial → partial with different semantics). The "didn't work" button remains available to progress to `applied_failed`.
|
||
|
||
---
|
||
|
||
## Data model
|
||
|
||
### New migration
|
||
|
||
```sql
|
||
ALTER TABLE script_builder_sessions
|
||
ADD COLUMN origin VARCHAR(20) NOT NULL DEFAULT 'standalone',
|
||
ADD COLUMN parent_pilot_session_id UUID NULL REFERENCES ai_sessions(id) ON DELETE SET NULL;
|
||
|
||
ALTER TABLE script_builder_sessions
|
||
ADD CONSTRAINT ck_script_builder_sessions_origin
|
||
CHECK (origin IN ('standalone', 'pilot_inline'));
|
||
```
|
||
|
||
Both columns nullable-defaulted so existing rows stay valid.
|
||
|
||
- `origin = 'standalone'` → existing `/script-builder` page usage.
|
||
- `origin = 'pilot_inline'` → new Script Builder tab. `parent_pilot_session_id` is set to the pilot session's `ai_sessions.id`.
|
||
|
||
`origin` enables future split-quota billing (decided to count as one billable session for now, but tagged for analytics). `parent_pilot_session_id` enables cross-reference when investigating pilot sessions.
|
||
|
||
### New backend endpoint
|
||
|
||
```
|
||
PATCH /api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/script
|
||
```
|
||
|
||
Request:
|
||
```json
|
||
{
|
||
"ai_drafted_script": "string (required, 1..50_000 chars)",
|
||
"ai_drafted_parameters": { /* optional JSONB */ }
|
||
}
|
||
```
|
||
|
||
Behavior:
|
||
- Auth: `require_engineer_or_admin` + `_load_session_or_404`.
|
||
- 404 if fix not found on that session.
|
||
- 409 if fix is in a terminal status (`applied_success`, `applied_failed`, `dismissed`) — a drafted script can't be attached after the fix is done.
|
||
- Sets `fix.ai_drafted_script` + `fix.ai_drafted_parameters`.
|
||
- **Bumps `ai_sessions.state_version`** — the fix just transitioned from "needs drafting" to "has draft", which affects Resolve/Escalate preview regeneration.
|
||
- Returns `SessionSuggestedFixResponse`.
|
||
|
||
### Lift of `ScriptBuilderChat`
|
||
|
||
Add a new prop:
|
||
```ts
|
||
mode: 'standalone' | 'ephemeral'
|
||
```
|
||
|
||
- `'standalone'`: current behavior — on submit, creates a `script_templates` row.
|
||
- `'ephemeral'`: on submit, invokes a caller-provided callback `onScriptDrafted(body, parameters)` instead of creating a template row. The caller (AssistantChatPage) uses this callback to fire the new PATCH /script endpoint.
|
||
|
||
Implementation: look for the submission path in `ScriptBuilderChat`, split the `script_templates` INSERT from the script-assembly logic, gate the INSERT on `mode === 'standalone'`.
|
||
|
||
---
|
||
|
||
## State
|
||
|
||
### Frontend state (AssistantChatPage)
|
||
|
||
New local state:
|
||
- `chatTab: 'chat' | 'script_builder'` — which tab is visible. Defaults to `'chat'`.
|
||
- `scriptBuilderMode: 'ai' | 'editor'` — which sub-view inside the Script Builder tab. Defaults to `'ai'`.
|
||
- `scriptBuilderHasProgress: boolean` — drives the indicator dot.
|
||
|
||
Reset in `resetSessionDerivedState`: all three back to defaults.
|
||
|
||
Banner's Apply handler (`handleApplyFix`) updated:
|
||
- If no script + no template → set `chatTab = 'script_builder'` (and show tab strip).
|
||
- If drafted script → open NoTemplateDialog in the chat region (new state or existing `scriptPanelOpen` reused).
|
||
- If template → existing TemplateMatchPanel flow (unchanged).
|
||
|
||
### Tab strip visibility
|
||
|
||
The tab strip is derived, not state:
|
||
```ts
|
||
const showTabStrip =
|
||
activeFix != null &&
|
||
activeFix.status !== 'dismissed' &&
|
||
activeFix.status !== 'applied_success' &&
|
||
activeFix.status !== 'applied_failed' &&
|
||
!activeFix.script_template_id &&
|
||
!activeFix.ai_drafted_script
|
||
```
|
||
|
||
When the strip hides (e.g., after script is drafted), `chatTab` resets to `'chat'` to avoid stuck state.
|
||
|
||
### Tab switching guard
|
||
|
||
The existing `currentChatRef` pattern (Async-select-load-apply guard) applies: when the engineer switches chats, any in-flight tab-derived state is discarded.
|
||
|
||
---
|
||
|
||
## Out of scope
|
||
|
||
- **NoTemplateDialog grid fix.** Moved to the chat region (wide enough), so the `grid-cols-1 sm:grid-cols-3` layout now works as intended. No grid edit required.
|
||
- **`window.prompt` replacement** for partial-notes / failure-reason capture. Still the Phase 8 interim pattern; replacement is deferred to a later design debt pass.
|
||
- **TemplateMatchPanel relocation** to the chat region. Different surface, different interactions, not broken today. Possible future work.
|
||
- **Dedicated "clear AI outcome proposal" button in the UI.** Already covered by Phase 8 Issue #3 fix (DELETE endpoint + clear-on-outcome-write).
|
||
- **Task lane bottom-slot audit.** With NoTemplateDialog removed from the slot, it may be empty on most sessions. Keep the slot API stable; any cleanup is out of scope.
|
||
|
||
---
|
||
|
||
## Tests
|
||
|
||
### Backend
|
||
|
||
- **Migration:** forward + downgrade reversibility.
|
||
- **PATCH /script endpoint** (new test file `test_fix_script_endpoint.py`):
|
||
- happy path — 200, `ai_drafted_script` set, `state_version` bumped
|
||
- 404 on wrong session
|
||
- 409 on terminal status
|
||
- 400 on empty body
|
||
- **ScriptBuilderChat ephemeral mode** — existing test suite extended; submitting does NOT create a `script_templates` row when `mode='ephemeral'`.
|
||
|
||
### Frontend
|
||
|
||
Manual verification (no component test harness in this codebase per CLAUDE.md):
|
||
- No-draft fix → Apply click opens Script Builder tab.
|
||
- AI path: chat with AI, submit, tab disappears, NoTemplateDialog becomes eligible.
|
||
- Manual path: ✎ Write myself → Monaco loads with scaffold → edit → submit → tab disappears.
|
||
- Drafted fix → Apply click opens NoTemplateDialog in chat region (three cards side-by-side).
|
||
- Tab indicator dot appears on first AI message / non-empty Monaco buffer; clears on submit.
|
||
- Session switch with open Script Builder tab → tab/mode state resets.
|
||
- EscalateInterceptDialog partial choice → applied_partial written with notes.
|
||
|
||
### Build discipline
|
||
|
||
- `tsc -b` clean
|
||
- `npm run build` clean
|
||
- `docker exec resolutionflow_backend pytest` — all pre-existing suites still pass, no regression from the new endpoint.
|
||
|
||
---
|
||
|
||
## Files to touch (rough inventory)
|
||
|
||
**Backend — new:**
|
||
- `backend/alembic/versions/<hash>_script_builder_origin.py`
|
||
- `backend/tests/test_fix_script_endpoint.py`
|
||
|
||
**Backend — modified:**
|
||
- `backend/app/models/script_builder_session.py` — add columns
|
||
- `backend/app/schemas/session_suggested_fix.py` — add `SessionSuggestedFixScriptRequest`
|
||
- `backend/app/api/endpoints/session_suggested_fixes.py` — add PATCH /script endpoint
|
||
- `backend/app/services/script_builder_service.py` — stamp `origin='pilot_inline'` + `parent_pilot_session_id` when a session is created from the Script Builder tab; gate the `script_templates` INSERT on `origin='standalone'`
|
||
- `backend/app/models/session_suggested_fix.py` — unchanged (schema already has `ai_drafted_script`)
|
||
|
||
**Frontend — new:**
|
||
- `frontend/src/components/pilot/ChatTabStrip.tsx` — renders the `[Chat] [Script Builder ●]` strip
|
||
- `frontend/src/components/pilot/ScriptBuilderTab.tsx` — houses the AI/editor toggle + Monaco wrapper
|
||
- `frontend/src/components/pilot/NoTemplateDialogInline.tsx` (or reuse existing `NoTemplateDialog` with a new wrapper for chat-region styling)
|
||
|
||
**Frontend — modified:**
|
||
- `frontend/src/api/sessionSuggestedFixes.ts` — add `patchScript(sessionId, fixId, body, parameters)` method
|
||
- `frontend/src/components/script-builder/ScriptBuilderChat.tsx` — add `mode` prop
|
||
- `frontend/src/pages/AssistantChatPage.tsx` — wire tab strip, tab content, banner Apply routing, NoTemplateDialog chat-region render
|
||
- `frontend/src/components/pilot/EscalateInterceptDialog.tsx` — add fourth choice
|
||
- `frontend/src/components/pilot/TaskLane.tsx` — remove `bottomSlot` usage of NoTemplateDialog (leave prop API stable)
|
||
|
||
**Frontend — deleted:**
|
||
- None (existing components get refactored, not deleted).
|
||
|
||
---
|
||
|
||
## Rollout
|
||
|
||
- Single branch, merged as part of the in-flight `feat/flowpilot-migration` PR (same as Phase 8).
|
||
- No feature flag — the new surface is strictly additive to the banner's Apply flow; old behavior for drafted-script fixes is preserved (just renders in a different location).
|
||
|
||
---
|
||
|
||
## Open deferrals (acknowledged, not in this phase)
|
||
|
||
- `window.prompt` → inline input migration for partial notes / failure reasons.
|
||
- Anti-parrot compliance of any new AI system prompt used by `ScriptBuilderChat` for ephemeral mode — verify it's the same safe prompt the standalone mode already uses (no new content to guard).
|
||
- Telemetry events for tab opens / AI→editor toggles / script submissions from tab — add in the Phase 9 implementation plan if we want them.
|