diff --git a/docs/FlowAssist_Migration/phase-9-script-builder-tab.md b/docs/FlowAssist_Migration/phase-9-script-builder-tab.md new file mode 100644 index 00000000..0dcfaa4f --- /dev/null +++ b/docs/FlowAssist_Migration/phase-9-script-builder-tab.md @@ -0,0 +1,296 @@ +# 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/_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.