From 82c2e7aff4223b1d1223928dfdc2e7c023556538 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 11:41:44 +0200 Subject: [PATCH 01/16] feat: criteria grounding in approved requirements and first criterion round-trip through observer persistence Amp-Thread-ID: https://ampcode.com/threads/T-019d762a-383e-77bb-a53e-d9c8768dceaf Co-authored-by: Amp --- src/server/app.test.ts | 151 +++++++++++++++++++++++++++++++++++++ src/server/context.test.ts | 34 +++++++++ src/server/context.ts | 21 ++++++ src/server/interview.ts | 14 +++- 4 files changed, 218 insertions(+), 2 deletions(-) diff --git a/src/server/app.test.ts b/src/server/app.test.ts index 67611e46..4ea15ad1 100644 --- a/src/server/app.test.ts +++ b/src/server/app.test.ts @@ -223,6 +223,85 @@ async function seedActiveDesign(projectId: number) { return { ...seededScope, designTurn }; } +async function seedCriteriaReady(projectId: number) { + const { + advanceHead, + confirmPhaseOutcome, + createPhaseOutcome, + createTurn, + createKnowledgeItem, + linkKnowledgeItemToTurn, + } = await import('./db.js'); + const seededRequirements = await seedRequirementsReady(projectId); + + const approvedRequirement = createKnowledgeItem( + db, + projectId, + 'requirement', + 'Resume the interview from SQLite after restart', + ); + const rejectedRequirement = createKnowledgeItem( + db, + projectId, + 'requirement', + 'Support exporting the spec as a PDF', + ); + + const reviewTurn = createTurn(db, projectId, { + phase: 'requirements', + parent_turn_id: seededRequirements.designConfirmationTurn.id, + question: 'Are these requirements all reviewed now?', + answer: 'Yes — approve resume and reject PDF export', + }); + linkKnowledgeItemToTurn(db, approvedRequirement.id, reviewTurn.id, 'reviewed'); + linkKnowledgeItemToTurn(db, rejectedRequirement.id, reviewTurn.id, 'rejected'); + advanceHead(db, projectId, reviewTurn.id); + + const requirementsProposalTurn = createTurn(db, projectId, { + phase: 'requirements', + parent_turn_id: reviewTurn.id, + question: '', + answer: 'The requirement set has explicit review coverage and is ready to move into criteria.', + }); + advanceHead(db, projectId, requirementsProposalTurn.id); + + const requirementsOutcome = createPhaseOutcome(db, { + projectId, + phase: 'requirements', + proposal_turn_id: requirementsProposalTurn.id, + summary: 'The requirement set has explicit review coverage and is ready to move into criteria.', + }); + + const requirementsConfirmationTurn = createTurn(db, projectId, { + phase: 'requirements', + parent_turn_id: requirementsProposalTurn.id, + question: '', + answer: 'Confirm requirements closure', + user_parts: JSON.stringify([ + { type: 'text', text: 'Confirm requirements closure' }, + { + type: 'data-confirmation', + data: { + kind: 'confirm-proposed-phase-closure', + proposalTurnId: requirementsProposalTurn.id, + phase: 'requirements', + }, + }, + ]), + }); + confirmPhaseOutcome(db, requirementsOutcome.id, requirementsConfirmationTurn.id); + advanceHead(db, projectId, requirementsConfirmationTurn.id); + + return { + ...seededRequirements, + approvedRequirement, + rejectedRequirement, + reviewTurn, + requirementsProposalTurn, + requirementsConfirmationTurn, + }; +} + async function seedRequirementsReady(projectId: number) { const { advanceHead, confirmPhaseOutcome, createPhaseOutcome, createTurn } = await import('./db.js'); const seededDesign = await seedActiveDesign(projectId); @@ -1532,6 +1611,78 @@ describe('phase outcomes + scope closure', () => { expect(refreshedProjectRes.body.turns.at(-1).phase).toBe('criteria'); }); + it('grounds the first criteria turn in approved requirements and round-trips a criterion through observer persistence', async () => { + const projectId = await createTestProject(); + await seedCriteriaReady(projectId); + + mockStreamInterviewer.mockImplementation(async () => + makeTextInterviewer('What would prove the resume flow is complete?'), + ); + mockRunObserver.mockImplementation(async (dbArg, turnArg, observedProjectId) => { + const { createKnowledgeItem } = await import('./db.js'); + const criterion = createKnowledgeItem( + dbArg as DB, + observedProjectId as number, + 'criterion', + 'Closing and reopening the browser restores the active path', + ); + return { + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [criterion.id], + decisions: [], + assumptions: [], + }; + }); + + await request(app) + .post(`/api/projects/${projectId}/chat`) + .send({ + messages: [ + { + id: 'u-criteria-grounding', + role: 'user', + parts: [{ type: 'text', text: 'Let us define the first acceptance criterion' }], + }, + ], + }) + .expect(200); + + expect(mockStreamInterviewer).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ phase: 'criteria' }), + expect.any(Array), + expect.any(String), + 'criteria', + ); + + expect(mockRunObserver).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ phase: 'criteria' }), + projectId, + ); + + const projectRes = await request(app).get(`/api/projects/${projectId}`).expect(200); + expect(projectRes.body.workflow.phases.criteria).toEqual( + expect.objectContaining({ + status: 'in_progress', + closeability: false, + }), + ); + + const entitiesRes = await request(app).get(`/api/projects/${projectId}/entities`).expect(200); + expect(entitiesRes.body.criteria).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + content: 'Closing and reopening the browser restores the active path', + }), + ]), + ); + }); + it('force-closes design through the shared confirmation seam and enters requirements mode on the next turn', async () => { const projectId = await createTestProject(); mockStreamInterviewer.mockImplementation(async (dbArg, turn) => diff --git a/src/server/context.test.ts b/src/server/context.test.ts index 90af62d1..89a1b259 100644 --- a/src/server/context.test.ts +++ b/src/server/context.test.ts @@ -213,6 +213,40 @@ describe('buildInterviewerContext', () => { expect(result).toContain('User: Q3?'); }); + it('includes the approved requirement inventory when criteria review is active', () => { + const turns: TurnWithOptions[] = [ + { + id: 10, + project_id: 1, + parent_turn_id: 9, + phase: 'criteria', + question: 'What would prove the resume flow is complete?', + answer: 'It should restore the active path after restart.', + why: null, + impact: null, + is_resolution: false, + user_parts: null, + assistant_parts: null, + created_at: '2026-01-04', + }, + ]; + + const result = buildInterviewerContext(turns, 'Propose a first criterion', { + phase: 'criteria', + entities: { + approvedRequirements: [ + { id: 5, content: 'Resume the interview from SQLite after restart' }, + { id: 7, content: 'Export the reviewed spec as markdown' }, + ], + }, + }); + + expect(result).toContain('Approved requirements for criteria review:'); + expect(result).toContain('- [5] Resume the interview from SQLite after restart'); + expect(result).toContain('- [7] Export the reviewed spec as markdown'); + expect(result).toContain('User: Propose a first criterion'); + }); + it('includes the current requirement inventory when requirements review is active', () => { const turns: TurnWithOptions[] = [ { diff --git a/src/server/context.ts b/src/server/context.ts index d9a0d92b..1c58272b 100644 --- a/src/server/context.ts +++ b/src/server/context.ts @@ -8,9 +8,22 @@ interface InterviewerContextOptions { phase?: TurnWithOptions['phase']; entities?: { requirements?: Array<{ id: number; content: string }>; + approvedRequirements?: Array<{ id: number; content: string }>; }; } +function formatApprovedRequirementInventory( + approvedRequirements: NonNullable['approvedRequirements'], +): string | null { + if (!approvedRequirements || approvedRequirements.length === 0) { + return null; + } + + return `Approved requirements for criteria review:\n${approvedRequirements + .map((requirement) => `- [${requirement.id}] ${requirement.content}`) + .join('\n')}`; +} + function formatRequirementReviewInventory( requirements: NonNullable['requirements'], ): string | null { @@ -72,6 +85,14 @@ export function buildInterviewerContext( sections.push(requirementInventory); } + const approvedRequirementInventory = + options.phase === 'criteria' + ? formatApprovedRequirementInventory(options.entities?.approvedRequirements) + : null; + if (approvedRequirementInventory) { + sections.push(approvedRequirementInventory); + } + if (sections.length === 0) { return currentPrompt; } diff --git a/src/server/interview.ts b/src/server/interview.ts index f81380ca..93f3fceb 100644 --- a/src/server/interview.ts +++ b/src/server/interview.ts @@ -169,17 +169,27 @@ export async function streamInterviewer( phase: Phase, ) { const agent = createInterviewerAgent(db, turn.id, phase, turn.project_id); + const entities = getEntitiesForProject(db, turn.project_id); const fullPrompt = buildInterviewerContext(activePath, userMessage, { phase, entities: phase === 'requirements' ? { - requirements: getEntitiesForProject(db, turn.project_id).requirements.map((requirement) => ({ + requirements: entities.requirements.map((requirement) => ({ id: requirement.id, content: requirement.content, })), } - : undefined, + : phase === 'criteria' + ? { + approvedRequirements: entities.requirements + .filter((requirement) => requirement.reviewStatus === 'approved') + .map((requirement) => ({ + id: requirement.id, + content: requirement.content, + })), + } + : undefined, }); return agent.stream({ prompt: fullPrompt, From 85aa38164b3986e23681ee0dfdee26a575a78c07 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 11:42:18 +0200 Subject: [PATCH 02/16] docs: mark slice 10.1 done, add I97 criteria-review grounding invariant Amp-Thread-ID: https://ampcode.com/threads/T-019d762a-383e-77bb-a53e-d9c8768dceaf Co-authored-by: Amp --- memory/PLAN.md | 2 +- memory/SPEC.md | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/memory/PLAN.md b/memory/PLAN.md index afcc37e2..f297fb8a 100644 --- a/memory/PLAN.md +++ b/memory/PLAN.md @@ -144,7 +144,7 @@ - Result: interviewer grounded in requirement inventory; targeted approve/reject via review metadata + `turn_knowledge_item` links; closeability from full review coverage; shared phase-close seam reused for requirements → criteria handoff - Tracer bullets: 9.1 inventory grounding `done`, 9.2 targeted approval `done`, 9.3 targeted rejection `done`, 9.4 closeability + proposal `done`, 9.5 closure + criteria handoff `done` -10.1 **Criteria grounding + first synthesis/review loop** — Prove the first post-requirements criteria turn is grounded in the approved requirement set and can round-trip one first criterion through the existing seams without widening into the full criteria lifecycle. `not-started` +10.1 **Criteria grounding + first synthesis/review loop** `done` - Requirements: → SPEC.md §Requirements #6, #8, #12 - Assumptions: → SPEC.md §Assumptions A28, A40 - Decisions: → SPEC.md §Decisions D25, D55, D56, D71 diff --git a/memory/SPEC.md b/memory/SPEC.md index 3afaedb7..f65c6de4 100644 --- a/memory/SPEC.md +++ b/memory/SPEC.md @@ -314,6 +314,12 @@ Detailed schema and mode-model rationale: `docs/design/INTERVIEW_MODE_MODEL.md`. | --- | ---------------------------------------------------------- | ------------------------- | ------------------------------------ | ----------- | | I87 | Requirements-review mode grounds the interviewer in the current requirement inventory, targeted approve/reject actions persist durable active-path review links with latest-action-wins projection, closeability derives from full review coverage, and the shared phase-close proposal/confirmation seam reuses for requirements → criteria handoff with correct mode advancement | Slices 9.1–9.5 | context.test.ts, interview.test.ts, db.test.ts, app.test.ts, core.test.ts, EntitySidebar.test.tsx | D24, D25, D51, D61, D65, D66, D70, D71, D77, D78, D79, A28, A44, A45, A46 | +### Criteria-review seam + +| # | Invariant | Established by | Protected by | Proves | +| --- | ---------------------------------------------------------- | ------------------------- | ------------------------------------ | ----------- | +| I97 | Criteria-mode interviewer context is grounded in the approved requirement inventory (not the full or pending set), and one initial criterion can round-trip through criteria-mode observer persistence → entities projection while criteria remains `in_progress` | Slice 10.1 | context.test.ts, app.test.ts | D25, D71, A28, A40 | + ## Lexicon - -# Cards - -## Scope Card — `10.1` Criteria grounding + first synthesis/review loop - -### Target Behavior -After requirements closes, the first prepared criteria turn is grounded in the approved requirement set and can round-trip one initial criterion through the existing seams without dropping out of `criteria` mode. - -### Boundary Crossings -```text -→ requirements phase is confirmed closed from slice 9.5, with criteria now the active phase -→ next prepared turn / /chat invocation selects `phase: 'criteria'` -→ interviewer context injects the approved requirement inventory (and any existing criterion inventory) -→ criteria-mode interviewer emits a criteria-shaped question rather than a generic follow-up -→ user replies through the existing turn-response seam -→ observer persists one criterion through the generic knowledge seam -→ workflow remains `criteria` / `in_progress` and entities projection exposes the new criterion -``` - -### Risks and Assumptions -- **RISK:** Criteria mode may technically activate but still receive weak or stale grounding, producing a generic follow-up instead of a verification-oriented question. - - **MITIGATION:** Lock both artifacts independently: approved-requirement inventory reaching the interviewer seam and the emitted turn remaining criteria-shaped. - -- **RISK:** The first criterion round-trip could accidentally widen into explicit criterion review-state or closeability work. - - **MITIGATION:** Keep this slice synthesis-thin: prove one first criterion can emerge and persist; defer approve/reject/closeability to `10.2`. - -- **RISK:** Criteria grounding may include rejected or otherwise out-of-scope requirements rather than the reviewed active set. - - **MITIGATION:** Assert approved-requirement grounding specifically, not just generic requirement inventory presence. - -- **ASSUMPTION:** The criteria interviewer can be usefully grounded by the approved requirement inventory plus any earlier criterion-like signals without first introducing a dedicated criteria workspace. - - **VALIDATE:** criteria context/prompt tests and one criterion persistence round-trip stay coherent. - - → existing `SPEC.md` assumptions/decisions already cover this (`A28`, `A40`, `D25`, `D55`, `D56`, `D71`) - -### Acceptance Criteria -```text -✓ src/server/context.test.ts or src/server/interview.test.ts — criteria-mode interviewer context includes the approved requirement inventory (not just prior chat history) -✓ src/server/app.test.ts — the first post-9.5 /chat turn runs the interviewer and observer with `phase: 'criteria'` and emits a criteria-shaped question rather than a generic follow-up -✓ src/server/app.test.ts or src/server/db.test.ts — one initial criterion can round-trip through criteria-mode reply → observer persistence → entities projection while criteria remains `in_progress` -``` - -### Verification Approach -- **Inner:** fast unit tests — proves criteria context/prompt grounding and criteria-mode phase selection. -- **Middle:** round-trip oracle (criteria synthesis) — approved requirements → criteria turn → observer-created criterion → entities projection with no drift. -- **Outer:** manual walkthrough — finish requirements, enter criteria, verify the first criteria question feels tied to the reviewed requirement set. - -### Traceability -- **PLAN:** Slice `10.1` -- **SPEC requirements:** `#6`, `#8`, `#12` -- **SPEC assumptions:** `A28`, `A40` -- **SPEC decisions:** `D25`, `D55`, `D56`, `D71` -- **Invariants to respect:** `I18`, `I19`, `I21`, `I24`, `I95`, `I96` -- **Deferred to later cards:** explicit per-criterion review state, criteria closeability, final criteria closure - ---- - -## Scope Card — `10.2` Explicit criterion review state + minimal closeability - -### Target Behavior -Criteria review can persist explicit per-criterion positive/non-positive review state, project `approved` / `rejected` / `pending` status from the active path, and mark criteria closeable only when no current criterion remains pending. - -### Boundary Crossings -```text -→ criteria mode has a current criterion set on the active path -→ interviewer emits a targeted criteria-review turn carrying explicit criterion review metadata -→ user responds through the existing turn-response seam -→ response handling records durable active-path criterion review linkage -→ entities / sidebar read model projects `approved` / `rejected` / `pending` criterion state from the latest explicit review action -→ workflow projection computes criteria `closeability: true` only when every current criterion has non-pending review state -``` - -### Risks and Assumptions -- **RISK:** Repeating the slice-9 pattern too literally could split criterion approval, rejection, and closeability into separate tracer bullets that do not unblock the app. - - **MITIGATION:** Bundle the minimum useful criterion lifecycle into one slice: explicit positive action, explicit non-positive action, latest-action projection, and deterministic closeability. - -- **RISK:** Criterion review semantics may diverge from requirement review enough that reusing the same metadata/linkage seam becomes awkward. - - **MITIGATION:** Keep the first seam narrow and structural; defer edit/merge/split/stale semantics to `13a`. - -- **RISK:** Criteria closeability could accidentally widen into richer coverage logic such as "every approved requirement must have N criteria" before the thin end-to-end path is working. - - **MITIGATION:** Start with the same deterministic minimum-bar approach as earlier phases: current criterion set has no pending explicit review state. - -- **ASSUMPTION:** The first explicit criterion review seam can reuse the same targeted-review + active-path link pattern already proven for requirements. - - **VALIDATE:** targeted criterion review round-trip and latest-action projection pass without introducing a separate mutation path. - - → likely SPEC follow-on from existing seams (`A15`, `A28`, `D24`, `D61`, `D65`, `D66`, `D70`) - -### Acceptance Criteria -```text -✓ src/shared/chat.ts or src/server/interview.test.ts — criteria review metadata validates targeted criterion review actions through the existing structured question seam -✓ src/server/app.test.ts — one explicit positive criterion review action and one explicit non-positive criterion review action persist through the response seam and project correctly in entities/sidebar state -✓ src/server/db.test.ts — criterion review projection resolves `approved` / `rejected` / `pending` from the latest explicit active-path action -✓ src/server/db.test.ts — criteria becomes closeable only when every current criterion has explicit non-pending review state -``` - -### Verification Approach -- **Inner:** fast unit tests — proves criterion review metadata, read-model projection, and closeability rule. -- **Middle:** round-trip oracle (criteria review) — targeted review metadata → response submit → durable review linkage → projected criterion state with no drift. -- **Middle:** model-based lifecycle oracle (criteria review) — criteria remains `in_progress` until all current criteria are explicitly reviewed. -- **Outer:** manual walkthrough — confirm the thin approve/reject semantics are legible enough to keep moving toward completed workflow. - -### Traceability -- **PLAN:** Slice `10.2` -- **SPEC requirements:** `#7`, `#8`, `#12`, `#13` -- **SPEC assumptions:** `A15`, `A28` -- **SPEC decisions:** `D24`, `D61`, `D65`, `D66`, `D70` -- **Invariants to respect:** `I18`, `I21`, `I24`, `I62`, `I63`, `I96` -- **Deferred to later cards:** criterion edit/add/merge/split flows, richer requirement↔criterion coverage logic, stale review invalidation - ---- - -## Scope Card — `10.3` Criteria closure + completed workflow state - -### Target Behavior -Once criteria is closeable, the shared phase-close seam can propose and confirm final criteria closure, leaving workflow with all phases closed and no stale active interviewer phase before export. - -### Boundary Crossings -```text -→ criteria phase is active and closeable after 10.2 -→ interviewer emits `propose_phase_closure` for `criteria` -→ `/chat` streams `data-phase-summary` and persists a proposed criteria `phase_outcome` -→ user confirms the proposed criteria closure through `data-confirmation` -→ `phase_outcome` persistence confirms the final criteria outcome with durable closure basis -→ workflow projection marks criteria `closed` and projects no remaining active phase -→ next app/project state reads as interview-complete rather than reopening stale criteria mode -``` - -### Risks and Assumptions -- **RISK:** The current server-side "first unclosed phase or `criteria` fallback" behavior may fabricate an active criteria phase even after final closure. - - **MITIGATION:** Lock completed-workflow projection explicitly and treat "all phases closed" as its own tested structural state. - -- **RISK:** Final-phase closure could widen into export gating, dashboard polish, or force-close variants. - - **MITIGATION:** Keep this slice about closure mechanics only: proposed final close, confirmation, and completed workflow projection. Leave export/dashboard concerns to slices `11a` and `13`. - -- **RISK:** Criteria closure may be implemented correctly in persistence but fail to suppress stale interviewer activity on the next app interaction. - - **MITIGATION:** Add an app/core assertion that post-confirmation state has no remaining active interview phase before export. - -- **ASSUMPTION:** The shared phase-close transport used for scope, design, and requirements can close the terminal `criteria` phase without a criteria-specific mutation path. - - **VALIDATE:** proposal/confirmation round-trip persists and projects a completed workflow state through the same seam. - - → existing `SPEC.md` assumptions/decisions already cover this (`A15`, `A28`, `D65`, `D66`, `D71`) - -### Acceptance Criteria -```text -✓ src/server/app.test.ts — criteria can emit a shared `data-phase-summary` proposal and persist a proposed final `phase_outcome` -✓ src/server/db.test.ts or src/server/app.test.ts — confirming the proposed criteria outcome closes criteria with durable closure provenance and leaves all workflow phases closed -✓ src/server/core.test.ts or src/server/app.test.ts — post-confirmation app state projects no stale active interviewer phase before export rather than reopening `criteria` -``` - -### Verification Approach -- **Inner:** fast unit/integration tests — proves final-phase proposal/confirmation persistence and completed-workflow projection. -- **Middle:** round-trip oracle (phase outcome) — criteria proposal → confirmation → confirmed final `phase_outcome` → completed workflow state with no drift. -- **Outer:** manual walkthrough — complete the interview, confirm final criteria closure, and verify the app feels done before export-specific polish lands. - -### Traceability -- **PLAN:** Slice `10.3` -- **SPEC requirements:** `#7`, `#8`, `#13` -- **SPEC assumptions:** `A15`, `A28` -- **SPEC decisions:** `D65`, `D66`, `D71` -- **Invariants to respect:** `I18`, `I24`, `I96` -- **Deferred to later cards:** export predicate details, dashboard summarization, low-readiness/forced-close variants if they prove necessary later From 76608a59a687227af8605c5940b62ed685dcbf86 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 12:06:55 +0200 Subject: [PATCH 09/16] =?UTF-8?q?docs:=20refactor=20plan=20=E2=80=94=20uni?= =?UTF-8?q?fy=20review=20seam=20duplication?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amp-Thread-ID: https://ampcode.com/threads/T-019d76c6-3181-71a8-9d32-1e546f5a0f17 Co-authored-by: Amp --- memory/REFACTOR.md | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 memory/REFACTOR.md diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md new file mode 100644 index 00000000..53951aaa --- /dev/null +++ b/memory/REFACTOR.md @@ -0,0 +1,43 @@ + + +# Refactor: Unify review seam duplication + +## Problem Statement + +The requirement-review and criterion-review implementations in `db.ts` and `chat.ts` are near-identical copies that differ only by knowledge kind (`requirement` vs `criterion`) and schema field name (`review` vs `criterionReview`). This duplication makes the review seam shallow — every future review-kind addition (13a) would require copying ~120 lines. The `review` field name on `structuredQuestionSchema` is also unqualified, creating an asymmetry with `criterionReview`. + +## Solution + +1. Unify the duplicated review-status projection, review-extraction, and review-recording functions in `db.ts` into single parameterized versions. +2. Rename the `review` field on `structuredQuestionSchema` to `requirementReview` for naming symmetry. +3. Unify the superRefine validation for both review fields into a single helper. +4. Collapse `RequirementReviewStatus` / `CriterionReviewStatus` into a single `ReviewStatus` type. + +## Commits + +1. **Rename `review` → `requirementReview` on `structuredQuestionSchema`** — rename the field, update the superRefine, update the type export, update all consumers in `db.ts` and test fixtures. This is a rename-only commit — no logic changes. + +2. **Extract shared `getReviewStatusesOnActivePath(db, projectId, kind)` from the two projection functions** — parameterize by `kind`, delete both originals, update callers. Collapse `RequirementReviewStatus` / `CriterionReviewStatus` into `ReviewStatus`. + +3. **Extract shared `getReviewFromTurn(turn, field)` from the two extraction functions** — parameterize by field name, delete both originals, update callers. + +4. **Extract shared `recordReviewFromTurnResponse(db, turn, positions, field, kind)` from the two recording functions** — parameterize by extraction function and expected kind, delete both originals, update `app.ts` to call the unified version. Collapse the two `app.ts` import/call sites into one. + +5. **Extract shared `validateReviewOptionPosition` helper in `structuredQuestionSchema` superRefine** — deduplicate the two parallel validation blocks. + +## Decisions + +- The `review` → `requirementReview` rename is a breaking change for any `assistant_parts` JSON persisted before this commit. Acceptable because: (a) the app is pre-distribution, (b) review linkage is already durably persisted in `turn_knowledge_item` so projections survive, and (c) only `recordRequirementReviewFromTurnResponse` replay on old turns would silently no-op, which has no user-visible effect. +- The unified `ReviewStatus` type replaces both `RequirementReviewStatus` and `CriterionReviewStatus`. The semantic space is identical. +- The unified `recordReviewFromTurnResponse` remains two call sites in `app.ts` (one per kind) rather than a single call with a loop, to keep the call site explicit about which kinds are handled. + +## Testing Decisions + +- No new tests needed. Existing tests for both requirement and criterion review seams (db.test.ts: 6 tests, app.test.ts: 4 tests) serve as characterization tests. All must pass after each commit. +- Test fixtures in `app.test.ts` must be updated in commit 1 to use `requirementReview` field name. + +## Out of Scope + +- Unifying the `requirementReview` / `criterionReview` fields into a single discriminated `review` union on `structuredQuestionSchema` — this is a deeper schema design question better deferred to 13a when review lifecycle refinement may change the shape anyway. +- Making `reviewStatus` non-optional on entity types (finding 9 from the review) — trivial but separate from this duplication refactor. +- Any 13a review lifecycle changes (edit/merge/split/stale semantics). From 1f78574029e790654d04e773ee645a00e6bb2aab Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 12:58:44 +0200 Subject: [PATCH 10/16] =?UTF-8?q?refactor:=20rename=20review=20=E2=86=92?= =?UTF-8?q?=20requirementReview=20on=20structuredQuestionSchema?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amp-Thread-ID: https://ampcode.com/threads/T-019d7708-5385-70ab-bb6e-9d9d10a0632f Co-authored-by: Amp --- src/server/app.test.ts | 4 ++-- src/server/db.ts | 4 ++-- src/shared/chat.ts | 22 ++++++++++++---------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/server/app.test.ts b/src/server/app.test.ts index 900dda1f..a62502f4 100644 --- a/src/server/app.test.ts +++ b/src/server/app.test.ts @@ -2302,7 +2302,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { { content: 'Approve this requirement', is_recommended: true }, { content: 'This requirement needs correction', is_recommended: false }, ], - review: { + requirementReview: { kind: 'requirement-approval' as const, requirementId: approvedRequirement.id, approveOptionPosition: 0, @@ -2402,7 +2402,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { { content: 'Reject this requirement', is_recommended: true }, { content: 'Keep this requirement for now', is_recommended: false }, ], - review: { + requirementReview: { kind: 'requirement-rejection' as const, requirementId: rejectedRequirement.id, rejectOptionPosition: 0, diff --git a/src/server/db.ts b/src/server/db.ts index 7a9490d9..fd965cdf 100644 --- a/src/server/db.ts +++ b/src/server/db.ts @@ -794,11 +794,11 @@ function getRequirementReview(turn: Pick): RequirementR } const parsedInput = structuredQuestionSchema.safeParse(part.input); - if (!parsedInput.success || !parsedInput.data.review) { + if (!parsedInput.success || !parsedInput.data.requirementReview) { continue; } - return parsedInput.data.review; + return parsedInput.data.requirementReview; } return null; diff --git a/src/shared/chat.ts b/src/shared/chat.ts index ebea8cd5..ea178e40 100644 --- a/src/shared/chat.ts +++ b/src/shared/chat.ts @@ -48,26 +48,28 @@ export const structuredQuestionSchema = z }), ) .min(2), - review: requirementReviewSchema.optional(), + requirementReview: requirementReviewSchema.optional(), criterionReview: criterionReviewSchema.optional(), }) .superRefine((value, ctx) => { - if (value.review) { + if (value.requirementReview) { const reviewOptionPosition = - value.review.kind === 'requirement-approval' - ? value.review.approveOptionPosition - : value.review.rejectOptionPosition; + value.requirementReview.kind === 'requirement-approval' + ? value.requirementReview.approveOptionPosition + : value.requirementReview.rejectOptionPosition; if (!value.options[reviewOptionPosition]) { ctx.addIssue({ code: 'custom', message: - value.review.kind === 'requirement-approval' - ? 'review.approveOptionPosition must reference an existing option' - : 'review.rejectOptionPosition must reference an existing option', + value.requirementReview.kind === 'requirement-approval' + ? 'requirementReview.approveOptionPosition must reference an existing option' + : 'requirementReview.rejectOptionPosition must reference an existing option', path: [ - 'review', - value.review.kind === 'requirement-approval' ? 'approveOptionPosition' : 'rejectOptionPosition', + 'requirementReview', + value.requirementReview.kind === 'requirement-approval' + ? 'approveOptionPosition' + : 'rejectOptionPosition', ], }); } From 9fa77a61f2152e76d24e5b9f8cc1af4f5cf12987 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 13:09:09 +0200 Subject: [PATCH 11/16] refactor: extract shared getReviewStatusesOnActivePath, collapse ReviewStatus type Amp-Thread-ID: https://ampcode.com/threads/T-019d7708-5385-70ab-bb6e-9d9d10a0632f Co-authored-by: Amp --- src/server/db.ts | 63 +++++++++--------------------------------------- 1 file changed, 12 insertions(+), 51 deletions(-) diff --git a/src/server/db.ts b/src/server/db.ts index fd965cdf..f13166c9 100644 --- a/src/server/db.ts +++ b/src/server/db.ts @@ -512,15 +512,14 @@ export interface EntityRelationship { target: EntityReference; } -export type RequirementReviewStatus = 'approved' | 'rejected' | 'pending'; -export type CriterionReviewStatus = 'approved' | 'rejected' | 'pending'; +export type ReviewStatus = 'approved' | 'rejected' | 'pending'; export type RequirementEntity = KnowledgeItem & { - reviewStatus?: RequirementReviewStatus; + reviewStatus?: ReviewStatus; }; export type CriterionEntity = KnowledgeItem & { - reviewStatus?: CriterionReviewStatus; + reviewStatus?: ReviewStatus; }; export interface EntitiesForProject { @@ -676,10 +675,11 @@ function getEntityCollectionForKind(kind: KnowledgeKind): EntityCollection { return 'knowledge_item'; } -function getRequirementReviewStatusesOnActivePath( +function getReviewStatusesOnActivePath( db: DB, projectId: number, -): Map { + kind: 'requirement' | 'criterion', +): Map { const activePath = getActivePath(db, projectId); if (activePath.length === 0) { return new Map(); @@ -698,7 +698,7 @@ function getRequirementReviewStatusesOnActivePath( .where( and( eq(schema.knowledgeItem.project_id, projectId), - eq(schema.knowledgeItem.kind, 'requirement'), + eq(schema.knowledgeItem.kind, kind), inArray(schema.turnKnowledgeItem.relation, ['reviewed', 'rejected']), inArray(schema.turnKnowledgeItem.turn_id, activeTurnIds), ), @@ -707,7 +707,7 @@ function getRequirementReviewStatusesOnActivePath( reviewRows.sort((left, right) => (turnOrder.get(left.turnId) ?? 0) - (turnOrder.get(right.turnId) ?? 0)); - const statuses = new Map(); + const statuses = new Map(); for (const row of reviewRows) { statuses.set(row.itemId, row.relation === 'reviewed' ? 'approved' : 'rejected'); } @@ -716,57 +716,18 @@ function getRequirementReviewStatusesOnActivePath( } function getRequirementEntitiesForProject(db: DB, projectId: number): RequirementEntity[] { - const requirementReviewStatuses = getRequirementReviewStatusesOnActivePath(db, projectId); + const reviewStatuses = getReviewStatusesOnActivePath(db, projectId, 'requirement'); return getKnowledgeItemsForProjectByKind(db, projectId, 'requirement').map((item) => ({ ...item, - reviewStatus: requirementReviewStatuses.get(item.id) ?? 'pending', + reviewStatus: reviewStatuses.get(item.id) ?? 'pending', })); } -function getCriterionReviewStatusesOnActivePath( - db: DB, - projectId: number, -): Map { - const activePath = getActivePath(db, projectId); - if (activePath.length === 0) { - return new Map(); - } - - const activeTurnIds = activePath.map((turn) => turn.id); - const turnOrder = new Map(activePath.map((turn, index) => [turn.id, index])); - const reviewRows = db - .select({ - itemId: schema.turnKnowledgeItem.item_id, - turnId: schema.turnKnowledgeItem.turn_id, - relation: schema.turnKnowledgeItem.relation, - }) - .from(schema.turnKnowledgeItem) - .innerJoin(schema.knowledgeItem, eq(schema.knowledgeItem.id, schema.turnKnowledgeItem.item_id)) - .where( - and( - eq(schema.knowledgeItem.project_id, projectId), - eq(schema.knowledgeItem.kind, 'criterion'), - inArray(schema.turnKnowledgeItem.relation, ['reviewed', 'rejected']), - inArray(schema.turnKnowledgeItem.turn_id, activeTurnIds), - ), - ) - .all() as Array<{ itemId: number; turnId: number; relation: 'reviewed' | 'rejected' }>; - - reviewRows.sort((left, right) => (turnOrder.get(left.turnId) ?? 0) - (turnOrder.get(right.turnId) ?? 0)); - - const statuses = new Map(); - for (const row of reviewRows) { - statuses.set(row.itemId, row.relation === 'reviewed' ? 'approved' : 'rejected'); - } - - return statuses; -} - function getCriterionEntitiesForProject(db: DB, projectId: number): CriterionEntity[] { - const criterionReviewStatuses = getCriterionReviewStatusesOnActivePath(db, projectId); + const reviewStatuses = getReviewStatusesOnActivePath(db, projectId, 'criterion'); return getKnowledgeItemsForProjectByKind(db, projectId, 'criterion').map((item) => ({ ...item, - reviewStatus: criterionReviewStatuses.get(item.id) ?? 'pending', + reviewStatus: reviewStatuses.get(item.id) ?? 'pending', })); } From 3d5e7b98cca308bfbf3e2ef33b829f1e3c5bf960 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 13:30:14 +0200 Subject: [PATCH 12/16] refactor: extract shared getReviewFromTurn, remove per-kind extraction functions Amp-Thread-ID: https://ampcode.com/threads/T-019d7708-5385-70ab-bb6e-9d9d10a0632f Co-authored-by: Amp --- src/server/db.ts | 37 +++++++++---------------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/src/server/db.ts b/src/server/db.ts index f13166c9..86b914a5 100644 --- a/src/server/db.ts +++ b/src/server/db.ts @@ -3,12 +3,7 @@ import { and, desc, eq, inArray, sql, type InferSelectModel } from 'drizzle-orm' import { drizzle } from 'drizzle-orm/better-sqlite3'; import { migrate } from 'drizzle-orm/better-sqlite3/migrator'; -import { - isAskQuestionUIPart, - structuredQuestionSchema, - type CriterionReview, - type RequirementReview, -} from '../shared/chat.js'; +import { isAskQuestionUIPart, structuredQuestionSchema, type StructuredQuestion } from '../shared/chat.js'; import { genericKnowledgeKindRegistry, type GenericKnowledgeCollectionKey, @@ -731,35 +726,21 @@ function getCriterionEntitiesForProject(db: DB, projectId: number): CriterionEnt })); } -function getCriterionReview(turn: Pick): CriterionReview | null { - for (const part of safeDeserializeAssistantParts(turn.assistant_parts)) { - if (!isAskQuestionUIPart(part) || !('input' in part)) { - continue; - } - - const parsedInput = structuredQuestionSchema.safeParse(part.input); - if (!parsedInput.success || !parsedInput.data.criterionReview) { - continue; - } - - return parsedInput.data.criterionReview; - } - - return null; -} - -function getRequirementReview(turn: Pick): RequirementReview | null { +function getReviewFromTurn( + turn: Pick, + field: F, +): NonNullable | null { for (const part of safeDeserializeAssistantParts(turn.assistant_parts)) { if (!isAskQuestionUIPart(part) || !('input' in part)) { continue; } const parsedInput = structuredQuestionSchema.safeParse(part.input); - if (!parsedInput.success || !parsedInput.data.requirementReview) { + if (!parsedInput.success || !parsedInput.data[field]) { continue; } - return parsedInput.data.requirementReview; + return parsedInput.data[field]; } return null; @@ -770,7 +751,7 @@ export function recordRequirementReviewFromTurnResponse( turn: Turn, selectedPositions: number[], ): void { - const review = getRequirementReview(turn); + const review = getReviewFromTurn(turn, 'requirementReview'); if (!review) { return; } @@ -799,7 +780,7 @@ export function recordRequirementReviewFromTurnResponse( } export function recordCriterionReviewFromTurnResponse(db: DB, turn: Turn, selectedPositions: number[]): void { - const review = getCriterionReview(turn); + const review = getReviewFromTurn(turn, 'criterionReview'); if (!review) { return; } From 1a34b2fd2617f0723d28d42dd9560af35e46993d Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 13:31:37 +0200 Subject: [PATCH 13/16] refactor: extract shared recordReviewFromTurnResponse, remove per-kind recording functions Amp-Thread-ID: https://ampcode.com/threads/T-019d7708-5385-70ab-bb6e-9d9d10a0632f Co-authored-by: Amp --- src/server/app.ts | 7 +++--- src/server/db.ts | 55 +++++++++++++---------------------------------- 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/src/server/app.ts b/src/server/app.ts index 74628663..cdd5a18e 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -39,8 +39,7 @@ import { getOptionsForTurn, updateTurn, getEntitiesForProject, - recordRequirementReviewFromTurnResponse, - recordCriterionReviewFromTurnResponse, + recordReviewFromTurnResponse, } from './db.js'; import { persistFallbackQuestionText, streamInterviewer } from './interview.js'; import { runObserver } from './observer.js'; @@ -116,8 +115,8 @@ export function createApp(dbPath?: string) { return; } applyTurnResponseSelections(db, turnId, uniquePositions); - recordRequirementReviewFromTurnResponse(db, turn, uniquePositions); - recordCriterionReviewFromTurnResponse(db, turn, uniquePositions); + recordReviewFromTurnResponse(db, turn, uniquePositions, 'requirementReview', 'requirement'); + recordReviewFromTurnResponse(db, turn, uniquePositions, 'criterionReview', 'criterion'); const selectedOptionIds = selectedOptions.map((option) => option.id); const selectedOptionContents = selectedOptions.map((option) => option.content); diff --git a/src/server/db.ts b/src/server/db.ts index 86b914a5..2a136110 100644 --- a/src/server/db.ts +++ b/src/server/db.ts @@ -746,65 +746,40 @@ function getReviewFromTurn( return null; } -export function recordRequirementReviewFromTurnResponse( +export function recordReviewFromTurnResponse( db: DB, turn: Turn, selectedPositions: number[], + field: 'requirementReview' | 'criterionReview', + kind: 'requirement' | 'criterion', ): void { - const review = getReviewFromTurn(turn, 'requirementReview'); + const review = getReviewFromTurn(turn, field); if (!review) { return; } const selectedReviewOptionPosition = - review.kind === 'requirement-approval' ? review.approveOptionPosition : review.rejectOptionPosition; + 'approveOptionPosition' in review ? review.approveOptionPosition : review.rejectOptionPosition; if (!selectedPositions.includes(selectedReviewOptionPosition)) { return; } - const requirement = db - .select() - .from(schema.knowledgeItem) - .where(eq(schema.knowledgeItem.id, review.requirementId)) - .get() as KnowledgeItem | undefined; - if (!requirement || requirement.project_id !== turn.project_id || requirement.kind !== 'requirement') { - return; - } - - linkKnowledgeItemToTurn( - db, - requirement.id, - turn.id, - review.kind === 'requirement-approval' ? 'reviewed' : 'rejected', - ); -} - -export function recordCriterionReviewFromTurnResponse(db: DB, turn: Turn, selectedPositions: number[]): void { - const review = getReviewFromTurn(turn, 'criterionReview'); - if (!review) { - return; - } - - const selectedReviewOptionPosition = - review.kind === 'criterion-approval' ? review.approveOptionPosition : review.rejectOptionPosition; - if (!selectedPositions.includes(selectedReviewOptionPosition)) { - return; - } - - const criterion = db - .select() - .from(schema.knowledgeItem) - .where(eq(schema.knowledgeItem.id, review.criterionId)) - .get() as KnowledgeItem | undefined; - if (!criterion || criterion.project_id !== turn.project_id || criterion.kind !== 'criterion') { + const entityId = + kind === 'requirement' + ? (review as { requirementId: number }).requirementId + : (review as { criterionId: number }).criterionId; + const entity = db.select().from(schema.knowledgeItem).where(eq(schema.knowledgeItem.id, entityId)).get() as + | KnowledgeItem + | undefined; + if (!entity || entity.project_id !== turn.project_id || entity.kind !== kind) { return; } linkKnowledgeItemToTurn( db, - criterion.id, + entity.id, turn.id, - review.kind === 'criterion-approval' ? 'reviewed' : 'rejected', + 'approveOptionPosition' in review ? 'reviewed' : 'rejected', ); } From c814b029821c5645f4263a7ee45aabeb0a857359 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 13:32:49 +0200 Subject: [PATCH 14/16] refactor: extract shared validateReviewOptionPosition in structuredQuestionSchema superRefine Amp-Thread-ID: https://ampcode.com/threads/T-019d7708-5385-70ab-bb6e-9d9d10a0632f Co-authored-by: Amp --- src/shared/chat.ts | 62 ++++++++++++++++------------------------------ 1 file changed, 21 insertions(+), 41 deletions(-) diff --git a/src/shared/chat.ts b/src/shared/chat.ts index ea178e40..8c3c9d88 100644 --- a/src/shared/chat.ts +++ b/src/shared/chat.ts @@ -35,6 +35,25 @@ export const criterionRejectionReviewSchema = z.object({ export const criterionReviewSchema = z.union([criterionApprovalReviewSchema, criterionRejectionReviewSchema]); +function validateReviewOptionPosition( + review: { approveOptionPosition?: number; rejectOptionPosition?: number }, + field: string, + optionCount: number, + ctx: z.RefinementCtx, +): void { + const isApproval = 'approveOptionPosition' in review; + const position = isApproval ? review.approveOptionPosition! : review.rejectOptionPosition!; + const positionField = isApproval ? 'approveOptionPosition' : 'rejectOptionPosition'; + + if (position >= optionCount) { + ctx.addIssue({ + code: 'custom', + message: `${field}.${positionField} must reference an existing option`, + path: [field, positionField], + }); + } +} + export const structuredQuestionSchema = z .object({ question: z.string().min(1), @@ -53,49 +72,10 @@ export const structuredQuestionSchema = z }) .superRefine((value, ctx) => { if (value.requirementReview) { - const reviewOptionPosition = - value.requirementReview.kind === 'requirement-approval' - ? value.requirementReview.approveOptionPosition - : value.requirementReview.rejectOptionPosition; - - if (!value.options[reviewOptionPosition]) { - ctx.addIssue({ - code: 'custom', - message: - value.requirementReview.kind === 'requirement-approval' - ? 'requirementReview.approveOptionPosition must reference an existing option' - : 'requirementReview.rejectOptionPosition must reference an existing option', - path: [ - 'requirementReview', - value.requirementReview.kind === 'requirement-approval' - ? 'approveOptionPosition' - : 'rejectOptionPosition', - ], - }); - } + validateReviewOptionPosition(value.requirementReview, 'requirementReview', value.options.length, ctx); } - if (value.criterionReview) { - const reviewOptionPosition = - value.criterionReview.kind === 'criterion-approval' - ? value.criterionReview.approveOptionPosition - : value.criterionReview.rejectOptionPosition; - - if (!value.options[reviewOptionPosition]) { - ctx.addIssue({ - code: 'custom', - message: - value.criterionReview.kind === 'criterion-approval' - ? 'criterionReview.approveOptionPosition must reference an existing option' - : 'criterionReview.rejectOptionPosition must reference an existing option', - path: [ - 'criterionReview', - value.criterionReview.kind === 'criterion-approval' - ? 'approveOptionPosition' - : 'rejectOptionPosition', - ], - }); - } + validateReviewOptionPosition(value.criterionReview, 'criterionReview', value.options.length, ctx); } }); From c13a90016165129c1f549863262dc692316bcf20 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 13:33:07 +0200 Subject: [PATCH 15/16] chore: retire completed review-seam refactor plan Amp-Thread-ID: https://ampcode.com/threads/T-019d7708-5385-70ab-bb6e-9d9d10a0632f Co-authored-by: Amp --- memory/REFACTOR.md | 43 ------------------------------------------- 1 file changed, 43 deletions(-) delete mode 100644 memory/REFACTOR.md diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md deleted file mode 100644 index 53951aaa..00000000 --- a/memory/REFACTOR.md +++ /dev/null @@ -1,43 +0,0 @@ - - -# Refactor: Unify review seam duplication - -## Problem Statement - -The requirement-review and criterion-review implementations in `db.ts` and `chat.ts` are near-identical copies that differ only by knowledge kind (`requirement` vs `criterion`) and schema field name (`review` vs `criterionReview`). This duplication makes the review seam shallow — every future review-kind addition (13a) would require copying ~120 lines. The `review` field name on `structuredQuestionSchema` is also unqualified, creating an asymmetry with `criterionReview`. - -## Solution - -1. Unify the duplicated review-status projection, review-extraction, and review-recording functions in `db.ts` into single parameterized versions. -2. Rename the `review` field on `structuredQuestionSchema` to `requirementReview` for naming symmetry. -3. Unify the superRefine validation for both review fields into a single helper. -4. Collapse `RequirementReviewStatus` / `CriterionReviewStatus` into a single `ReviewStatus` type. - -## Commits - -1. **Rename `review` → `requirementReview` on `structuredQuestionSchema`** — rename the field, update the superRefine, update the type export, update all consumers in `db.ts` and test fixtures. This is a rename-only commit — no logic changes. - -2. **Extract shared `getReviewStatusesOnActivePath(db, projectId, kind)` from the two projection functions** — parameterize by `kind`, delete both originals, update callers. Collapse `RequirementReviewStatus` / `CriterionReviewStatus` into `ReviewStatus`. - -3. **Extract shared `getReviewFromTurn(turn, field)` from the two extraction functions** — parameterize by field name, delete both originals, update callers. - -4. **Extract shared `recordReviewFromTurnResponse(db, turn, positions, field, kind)` from the two recording functions** — parameterize by extraction function and expected kind, delete both originals, update `app.ts` to call the unified version. Collapse the two `app.ts` import/call sites into one. - -5. **Extract shared `validateReviewOptionPosition` helper in `structuredQuestionSchema` superRefine** — deduplicate the two parallel validation blocks. - -## Decisions - -- The `review` → `requirementReview` rename is a breaking change for any `assistant_parts` JSON persisted before this commit. Acceptable because: (a) the app is pre-distribution, (b) review linkage is already durably persisted in `turn_knowledge_item` so projections survive, and (c) only `recordRequirementReviewFromTurnResponse` replay on old turns would silently no-op, which has no user-visible effect. -- The unified `ReviewStatus` type replaces both `RequirementReviewStatus` and `CriterionReviewStatus`. The semantic space is identical. -- The unified `recordReviewFromTurnResponse` remains two call sites in `app.ts` (one per kind) rather than a single call with a loop, to keep the call site explicit about which kinds are handled. - -## Testing Decisions - -- No new tests needed. Existing tests for both requirement and criterion review seams (db.test.ts: 6 tests, app.test.ts: 4 tests) serve as characterization tests. All must pass after each commit. -- Test fixtures in `app.test.ts` must be updated in commit 1 to use `requirementReview` field name. - -## Out of Scope - -- Unifying the `requirementReview` / `criterionReview` fields into a single discriminated `review` union on `structuredQuestionSchema` — this is a deeper schema design question better deferred to 13a when review lifecycle refinement may change the shape anyway. -- Making `reviewStatus` non-optional on entity types (finding 9 from the review) — trivial but separate from this duplication refactor. -- Any 13a review lifecycle changes (edit/merge/split/stale semantics). From 3d8eb1cb848e0638d627289fb13c663fc4c7e7cb Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Fri, 10 Apr 2026 13:41:41 +0200 Subject: [PATCH 16/16] =?UTF-8?q?chore:=20ln-sync=20=E2=80=94=20update=20c?= =?UTF-8?q?overage=20counts,=20refresh=20parallelism=20notes=20after=20rev?= =?UTF-8?q?iew-seam=20refactor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amp-Thread-ID: https://ampcode.com/threads/T-019d7708-5385-70ab-bb6e-9d9d10a0632f Co-authored-by: Amp --- memory/PLAN.md | 6 ++---- memory/SPEC.md | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/memory/PLAN.md b/memory/PLAN.md index 8c7260a4..817653b2 100644 --- a/memory/PLAN.md +++ b/memory/PLAN.md @@ -293,10 +293,8 @@ Phase 8: 14 ──→ 15 (drizzle-kit audit remediation) ### Parallelism opportunities -- With 7, 7a, 7b, 8, and 9 all done, the next primary slice is 10.1 (criteria grounding + first synthesis/review loop). -- 10.2 and 10.3 should follow linearly; they are intentionally the minimum slices needed to unblock completed interview flow rather than separate variants of the same review seam. -- 11a (project dashboard workflow state) can begin once 10.3 lands; it does not need the broader knowledge workspace. -- 12 (knowledge workspace) and 13 (export) can proceed in parallel once 10.3 stabilizes the criteria artifacts and completed-workflow state. +- 10.1–10.3 are done; the review seam has been unified (refactor landed between 10.3 and 11a). +- 11a (project dashboard workflow state), 12 (knowledge workspace), and 13 (export) are all unblocked and can proceed in parallel. - 13a (review lifecycle refinement) is explicitly deferred; it should collect rarer review variants after 12 and 13 stabilize rather than fragmenting slices 9 and 10. - 14 (npx) can start early with a basic launcher, completing after slice 13 when the export predicate stabilizes. - 15 (drizzle-kit audit remediation) should wait until 14 lands. diff --git a/memory/SPEC.md b/memory/SPEC.md index d4dd1f4e..cc9bbcf3 100644 --- a/memory/SPEC.md +++ b/memory/SPEC.md @@ -542,11 +542,11 @@ This projection difference is a deliberate design choice, not an implementation | ----------------------------- | ----- | ----------------------------------------------------- | | db.test.ts | 43 | I5, I6, I9, I10, I11, I20, I48, I54, I72, I87, I98 | | knowledge.test.ts | 1 | I48 | -| app.test.ts | 31 | I1, I2, I3, I7, I14, I21, I23, I44, I48, I54, I72, I87, I98, I99 | +| app.test.ts | 36 | I1, I2, I3, I7, I14, I21, I23, I44, I48, I54, I72, I87, I98, I99 | | core.test.ts | 10 | I12, I13, I18, I72, I87 | -| interview.test.ts | 9 | I16, I72, I87 | +| interview.test.ts | 10 | I16, I72, I87 | | parts.test.ts | 15 | I17, I18, I44, I54, I72 | -| context.test.ts | 14 | I19, I44, I48, I54, I87 | +| context.test.ts | 15 | I19, I44, I48, I54, I87 | | observer.test.ts | 9 | I20, I21, I44, I48, I54 | | phase-close.test.ts | 13 | I72 | | turn-response.test.ts | 4 | I44 |