fix: restore model activity reasoning stream
This commit is contained in:
User
@@ -0,0 +1,307 @@
|
||||
# Workflow UI Regressions Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Restore Phase 1 model activity, make join review read-only, improve CTE presentation, and prevent malformed phase summaries from crashing the UI.
|
||||
|
||||
**Architecture:** Preserve the existing Pi→backend→SSE→React pipeline while introducing one explicit `activity_delta` event and one explicit `join-review` widget contract. Enforce model-authored artifact shapes at the gate and retain defensive rendering for legacy payloads.
|
||||
|
||||
**Tech Stack:** TypeScript, Fastify, React 18, Zustand, Vitest/Testing Library, Node test runner, Pi gate extension, Docker Compose.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- UI strings remain English; persisted workspace content remains in the workspace language.
|
||||
- The harness remains the owner of workflow decisions and persistence.
|
||||
- Joins are informational and are persisted as a complete set only after `Continue`.
|
||||
- `Other — specify` is the only join-editing path and must not persist the rejected proposal.
|
||||
- F3 rewrite approval remains automatic with no reviewer widget.
|
||||
- No verbatim reasoning is persisted to session artifacts.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Restore the Model Activity stream
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/test/routes-sessions.test.ts`
|
||||
- Modify: `backend/test/session-bridge.test.ts`
|
||||
- Modify: `backend/src/routes/sessions.ts`
|
||||
- Modify: `backend/src/bridge/session-bridge.ts`
|
||||
- Modify: `frontend/src/api/types.ts`
|
||||
- Modify: `frontend/src/store/sessionStore.ts`
|
||||
- Modify: `frontend/src/store/sessionStore.test.ts`
|
||||
- Modify: `frontend/src/shell/ModelActivityPanel.tsx`
|
||||
- Modify: `frontend/src/shell/ModelActivityPanel.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `ClientEvent`/`StreamEvent` variant `{ type: "activity_delta"; text: string }`.
|
||||
- Produces: Zustand `activity: Entry[]`, consumed by `ModelActivityPanel`.
|
||||
|
||||
- [x] **Step 1: Write backend failing tests**
|
||||
|
||||
Add route assertions showing create passes the configured `thinking` level and resume passes the
|
||||
manifest level. Add a bridge test that emits:
|
||||
|
||||
```ts
|
||||
rpc.emit("event", {
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "thinking_delta", delta: "reasoning" },
|
||||
});
|
||||
expect(events).toContainEqual({ type: "activity_delta", text: "reasoning" });
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run backend tests and verify RED**
|
||||
|
||||
Run: `cd backend && npx vitest run test/routes-sessions.test.ts test/session-bridge.test.ts`
|
||||
|
||||
Expected: failures show `thinking` is still `off` and no `activity_delta` is emitted.
|
||||
|
||||
- [x] **Step 3: Implement backend event/config changes**
|
||||
|
||||
Use `thinking: s.thinking` for new sessions and
|
||||
`thinking: saved?.thinking ?? settings.thinking` on resume. Extend the bridge event union and map
|
||||
nested `thinking_delta` to `activity_delta` without changing final `text_delta` behavior.
|
||||
|
||||
- [x] **Step 4: Write frontend failing tests**
|
||||
|
||||
Assert `applyEvent({ type: "activity_delta", text: "reasoning" })` appends to `activity` and not
|
||||
`transcript`; render the activity panel after applying only `activity_delta` and expect the text.
|
||||
|
||||
- [x] **Step 5: Run frontend tests and verify RED**
|
||||
|
||||
Run: `cd frontend && npx vitest run src/store/sessionStore.test.ts src/shell/ModelActivityPanel.test.tsx`
|
||||
|
||||
Expected: TypeScript/test failures because the event variant and activity state do not exist.
|
||||
|
||||
- [x] **Step 6: Implement frontend activity state**
|
||||
|
||||
Add the event type, `activity` state, append logic mirroring streamed entry accumulation, reset it
|
||||
with the session, and have the panel select `state.activity`.
|
||||
|
||||
- [x] **Step 7: Run targeted tests and typechecks**
|
||||
|
||||
Run: `cd backend && npx vitest run test/routes-sessions.test.ts test/session-bridge.test.ts && npx tsc --noEmit -p .`
|
||||
|
||||
Run: `cd frontend && npx vitest run src/store/sessionStore.test.ts src/shell/ModelActivityPanel.test.tsx && npx tsc -b`
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
### Task 2: Replace editable join selection with read-only review
|
||||
|
||||
**Files:**
|
||||
- Modify: `harness/.pi/extensions/gate/builders.js`
|
||||
- Modify: `harness/.pi/extensions/gate/__tests__/builders.test.js`
|
||||
- Modify: `harness/.pi/extensions/tht-gate.js`
|
||||
- Create: `harness/.pi/extensions/gate/__tests__/gate_join_review.test.js`
|
||||
- Modify: `harness/.pi/skills/tht-sessione/SKILL.md`
|
||||
- Create: `frontend/src/widgets/JoinReviewWidget.tsx`
|
||||
- Create: `frontend/src/widgets/JoinReviewWidget.test.tsx`
|
||||
- Modify: `frontend/src/widgets/index.ts`
|
||||
- Modify: `frontend/src/api/types.ts`
|
||||
- Modify: `frontend/src/widgets/registry.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `buildJoinReviewRequest({ id, phase, title, options })` with widget `join-review`.
|
||||
- Consumes: options `{ id, label, detail?, rationale? }`.
|
||||
- Produces: response `{ id, kind: "join-review", choices: allOptionIds }` on `Continue`.
|
||||
|
||||
- [ ] **Step 1: Write failing builder and gate tests**
|
||||
|
||||
Assert the builder emits read-only option details, `confirm_label: "Continue"`, and reserved controls.
|
||||
Exercise `reviewer_decide` with only `join_modified` decisions; queue a Continue response containing
|
||||
all ids and assert every `tht decision add` call occurs. Queue `control:"freetext"` and assert no
|
||||
decision is written.
|
||||
|
||||
- [ ] **Step 2: Run harness tests and verify RED**
|
||||
|
||||
Run: `cd harness && node --test .pi/extensions/gate/__tests__/builders.test.js .pi/extensions/gate/__tests__/gate_join_review.test.js`
|
||||
|
||||
Expected: builder/export/widget contract is missing and join calls still emit `multiselect`.
|
||||
|
||||
- [ ] **Step 3: Implement builder and gate routing**
|
||||
|
||||
Add `buildJoinReviewRequest`. In `reviewer_decide`, detect a non-empty, join-only merit list:
|
||||
|
||||
```js
|
||||
const joinOnly = opts.length > 0 && opts.every((o) => o.decision.type === "join_modified");
|
||||
```
|
||||
|
||||
Emit `join-review` with `detail` and `rationale`; after Continue persist all original decisions.
|
||||
On free text, return feedback without persistence. Keep all other decisions on `multiselect`.
|
||||
|
||||
- [ ] **Step 4: Write failing frontend widget tests**
|
||||
|
||||
Render two join cards and assert there are no checkboxes. Click `Continue` and expect all ids in the
|
||||
response. Open `Other — specify`, submit correction text, and expect a freetext control response.
|
||||
|
||||
- [ ] **Step 5: Run frontend widget tests and verify RED**
|
||||
|
||||
Run: `cd frontend && npx vitest run src/widgets/JoinReviewWidget.test.tsx src/widgets/registry.test.tsx`
|
||||
|
||||
Expected: widget and registry entry are missing.
|
||||
|
||||
- [ ] **Step 6: Implement and register JoinReviewWidget**
|
||||
|
||||
Render semantic cards with label, detail, and rationale, one `Continue` primary button, and
|
||||
`ReservedControls`. Register `join-review` and extend `WidgetOption` with optional `detail` and
|
||||
`rationale` strings.
|
||||
|
||||
- [ ] **Step 7: Update model instructions and verify targeted tests**
|
||||
|
||||
Document that joins must be a separate join-only `reviewer_decide` call; the reviewer cannot remove
|
||||
individual joins and textual corrections require a complete revised proposal.
|
||||
|
||||
Run: `cd harness && npm test`
|
||||
|
||||
Run: `cd frontend && npx vitest run src/widgets/JoinReviewWidget.test.tsx src/widgets/registry.test.tsx && npx tsc -b`
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
### Task 3: Improve CTE plan layout and hide inert SQL controls
|
||||
|
||||
**Files:**
|
||||
- Modify: `frontend/src/viewers/CtePlanViewer.tsx`
|
||||
- Modify: `frontend/src/viewers/CtePlanViewer.test.tsx`
|
||||
- Modify: `frontend/src/viewers/SqlViewer.tsx`
|
||||
- Modify: `frontend/src/viewers/SqlViewer.test.tsx`
|
||||
- Modify: `frontend/src/viewers/CteResultViewer.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- `SqlViewer({ blocks })` shows the layout toggle only when `blocks.length > 1`.
|
||||
- CTE plan data shape remains unchanged.
|
||||
|
||||
- [ ] **Step 1: Write failing semantic/layout tests**
|
||||
|
||||
Assert long filter data is rendered in distinct elements labeled Column, Operator, and Value;
|
||||
assert card sections expose stable headings. Assert a one-block SQL viewer has no Horizontal or
|
||||
Vertical controls while a two-block viewer retains both.
|
||||
|
||||
- [ ] **Step 2: Run tests and verify RED**
|
||||
|
||||
Run: `cd frontend && npx vitest run src/viewers/CtePlanViewer.test.tsx src/viewers/SqlViewer.test.tsx src/viewers/CteResultViewer.test.tsx`
|
||||
|
||||
Expected: structured filter labels are absent and the one-block toggle is present.
|
||||
|
||||
- [ ] **Step 3: Implement responsive CTE cards**
|
||||
|
||||
Use a bordered header grid, prose blocks with `leading-relaxed`, metadata rows with fixed labels,
|
||||
`break-words`/`font-mono` for physical identifiers, and a responsive filter grid such as
|
||||
`grid-cols-1 sm:grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)]`. Keep output columns wrapping.
|
||||
|
||||
- [ ] **Step 4: Hide the single-block toggle**
|
||||
|
||||
Wrap the layout control in `{blocks.length > 1 && (...)}` without changing multi-block state or
|
||||
rendering.
|
||||
|
||||
- [ ] **Step 5: Run targeted tests and typecheck**
|
||||
|
||||
Run: `cd frontend && npx vitest run src/viewers/CtePlanViewer.test.tsx src/viewers/SqlViewer.test.tsx src/viewers/CteResultViewer.test.tsx && npx tsc -b`
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
### Task 4: Harden phase summaries against malformed open questions
|
||||
|
||||
**Files:**
|
||||
- Modify: `harness/.pi/extensions/gate/artifact-contracts.js`
|
||||
- Modify: `harness/.pi/extensions/gate/__tests__/artifact-contracts.test.js`
|
||||
- Modify: `harness/.pi/skills/tht-sessione/SKILL.md`
|
||||
- Modify: `frontend/src/viewers/artifactV2.ts`
|
||||
- Modify: `frontend/src/viewers/PhaseSummaryViewer.tsx`
|
||||
- Modify: `frontend/src/viewers/PhaseSummaryViewer.test.tsx`
|
||||
- Modify: `frontend/src/viewers/ArtifactView.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Gate contract: `open_questions?: string[]`.
|
||||
- Frontend legacy normalization: string entries pass through; objects prefer `question`, then
|
||||
`label`; all other values become safe text or are omitted.
|
||||
|
||||
- [ ] **Step 1: Write failing gate validation test**
|
||||
|
||||
Pass the exact observed payload shape:
|
||||
|
||||
```js
|
||||
open_questions: [{ label: "pazienti_finale restituisce 0 righe", question: "Verificare i filtri" }]
|
||||
```
|
||||
|
||||
Expect `ok:false` and an error naming `open_questions[0]`.
|
||||
|
||||
- [ ] **Step 2: Run gate test and verify RED**
|
||||
|
||||
Run: `cd harness && node --test .pi/extensions/gate/__tests__/artifact-contracts.test.js`
|
||||
|
||||
Expected: payload is currently accepted.
|
||||
|
||||
- [ ] **Step 3: Implement strict gate validation**
|
||||
|
||||
Reject a non-array `open_questions` value and every non-string entry with an indexed error.
|
||||
Document the exact array-of-strings shape in the session skill.
|
||||
|
||||
- [ ] **Step 4: Write failing frontend resilience test**
|
||||
|
||||
Render a phase-summary artifact containing the observed object and assert the screen displays
|
||||
`Verificare i filtri` and does not show the ErrorBoundary fallback.
|
||||
|
||||
- [ ] **Step 5: Run frontend test and verify RED**
|
||||
|
||||
Run: `cd frontend && npx vitest run src/viewers/PhaseSummaryViewer.test.tsx src/viewers/ArtifactView.test.tsx`
|
||||
|
||||
Expected: React reports an object child/rendering failure.
|
||||
|
||||
- [ ] **Step 6: Implement safe legacy normalization**
|
||||
|
||||
Add a small `phaseOpenQuestionText(value: unknown): string | null` helper and map/filter entries
|
||||
before rendering. Preserve the strict public TypeScript contract for new v2 payloads.
|
||||
|
||||
- [ ] **Step 7: Run targeted tests and typechecks**
|
||||
|
||||
Run: `cd harness && npm test`
|
||||
|
||||
Run: `cd frontend && npx vitest run src/viewers/PhaseSummaryViewer.test.tsx src/viewers/ArtifactView.test.tsx && npx tsc -b`
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
### Task 5: Full verification, durable notes, and Docker deployment
|
||||
|
||||
**Files:**
|
||||
- Modify: `PROJECT_STATE.md`
|
||||
- Modify or create under `brain/codebase/` and update `brain/index.md` if the vault is present.
|
||||
|
||||
**Interfaces:**
|
||||
- Running Compose services must use the newly built `thothii-core:local` and
|
||||
`thothii-frontend:local` image ids.
|
||||
|
||||
- [ ] **Step 1: Run all regression suites**
|
||||
|
||||
Run: `cd backend && npx vitest run && npx tsc --noEmit -p . && npm run build`
|
||||
|
||||
Run: `cd frontend && npx vitest run && npx tsc -b && npm run build`
|
||||
|
||||
Run: `cd harness && npm test`
|
||||
|
||||
Expected: all pass.
|
||||
|
||||
- [ ] **Step 2: Update project state and durable architectural note**
|
||||
|
||||
Record the implemented contracts, verification counts, deployment state, and the distinction
|
||||
between building an image and recreating a service.
|
||||
|
||||
- [ ] **Step 3: Build and recreate Compose services**
|
||||
|
||||
Run: `docker compose build`
|
||||
|
||||
Run: `docker compose up -d --force-recreate`
|
||||
|
||||
Expected: both services are recreated from the new images.
|
||||
|
||||
- [ ] **Step 4: Verify deployed containers**
|
||||
|
||||
Run: `docker compose ps`
|
||||
|
||||
Run: `docker inspect thothii-core thothii-frontend --format '{{.Name}} {{.Image}} {{.State.Health.Status}}'`
|
||||
|
||||
Expected: both report the new image ids and `healthy`.
|
||||
|
||||
- [ ] **Step 5: Review diff and commit intentionally**
|
||||
|
||||
Run: `git diff --check && git status --short && git diff --stat`
|
||||
|
||||
Commit only the files in this plan with a scoped message after review.
|
||||
@@ -0,0 +1,68 @@
|
||||
# Workflow UI Regressions Design
|
||||
|
||||
## Goal
|
||||
|
||||
Restore visible model reasoning and make the F4/F6 review experience deterministic,
|
||||
readable, and resilient to malformed model-authored artifacts.
|
||||
|
||||
## Diagnosed causes
|
||||
|
||||
- Session creation and resume force Pi `thinking` to `off`, regardless of the saved setting.
|
||||
- The backend bridge forwards assistant `text_delta` events but drops `thinking_delta` events.
|
||||
- The Model Activity panel reads final assistant text rather than a dedicated activity stream.
|
||||
- F4 joins use the editable `multiselect` contract; `recommended` is not translated into
|
||||
`selected` outside F2, so every join initially appears unchecked.
|
||||
- The CTE result viewer passes one SQL block to a viewer that always displays its
|
||||
multi-block Horizontal/Vertical control.
|
||||
- CTE plan cards flatten long filters, table names, keys, and rationale into loosely spaced text.
|
||||
- The F6 phase-summary payload accepted an object in `open_questions`; React then attempted to
|
||||
render that object directly and the widget error boundary displayed the generic failure message.
|
||||
- The F3 auto-approval fix was present in the image tag but not in the running containers because
|
||||
they were built without being recreated.
|
||||
|
||||
## Design
|
||||
|
||||
### Model activity
|
||||
|
||||
Session creation uses the global `thinking` preference and resume uses the persisted manifest
|
||||
preference, falling back to the current global setting. `SessionBridge` maps Pi's nested
|
||||
`thinking_delta` into a distinct SSE `activity_delta`. The frontend stores that stream separately
|
||||
from final assistant text, and Model Activity renders only activity deltas. This prevents internal
|
||||
reasoning from leaking into transcript-oriented state while making the panel accurately reflect
|
||||
the configured model activity.
|
||||
|
||||
### Join review
|
||||
|
||||
F4 calls to `reviewer_decide` whose merit decisions are all `join_modified` become a read-only
|
||||
`join-review` widget. Each proposed join is rendered as an informational card with its name,
|
||||
join expression, and rationale. `Continue` returns every join id; the gate persists all decisions.
|
||||
`Other — specify` returns textual feedback without persisting the current proposal, so the model
|
||||
must revise and present the complete join set again. Mixed join/non-join calls remain regular
|
||||
multiselects, and the skill instructs the model to keep joins in a separate call.
|
||||
|
||||
### CTE presentation
|
||||
|
||||
The plan viewer uses a responsive card layout: a compact numbered header, purpose and rationale
|
||||
as readable prose, aligned metadata rows for dependencies and keys, wrapped table rows, and a
|
||||
structured filter grid with separate column/operator/value fields. Output columns remain compact
|
||||
wrapping chips. The SQL layout switch is hidden whenever the viewer receives fewer than two blocks.
|
||||
|
||||
### Phase-summary resilience
|
||||
|
||||
The gate validates that `open_questions` is an array of strings and reports a corrective error to
|
||||
the model before emitting a widget. The frontend also normalizes legacy malformed entries to a
|
||||
safe string (preferring `question`, then `label`) so old or externally produced payloads cannot
|
||||
crash React. The session skill documents the exact `string[]` contract.
|
||||
|
||||
### Deployment
|
||||
|
||||
After targeted and full regression tests, build both Docker images and recreate the Compose
|
||||
services. Verify that the running containers use the newly built image ids and that both health
|
||||
checks pass.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Persisting verbatim model reasoning in session artifacts.
|
||||
- Allowing individual joins to be removed from the join review.
|
||||
- Redesigning multi-block SQL comparison behavior.
|
||||
- Changing the eight-phase workflow or F3 auto-approval semantics.
|
||||
Reference in New Issue
Block a user