From e004b8ecc962711e87339dc74506ef0ebec45b56 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Fri, 8 May 2026 17:25:48 +0200 Subject: [PATCH 1/7] FE-674: Surface open reconciliation_need rows; drop deferred banner (V3.0 card 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The V3.0 hard-impact apply contract no longer carries a `deferred: true` shape or any "Hard impact — coming in V3" banner copy. Open reconciliation_need rows opened by card 1 now render in a "Pending review" section inside the patch list overlay, satisfying the second clause of invariant I112 ("never returns deferred: true") and the read side of the queue. Server: - New GET /api/specifications/:id/reconciliation-needs (handler in src/server/reconciliation-needs-route.ts) returns { openNeeds: [...] } for the spec; resolved rows excluded; cross-spec leakage prevented at the application layer; 404 / 400 on missing or non-numeric spec id. Shared: - New src/shared/reconciliation-need.ts — ReconciliationNeedRecord type so the client doesn't import server types. Client: - specificationQueryKeys gains `reconciliationNeeds(specId)`. - New useSpecificationOpenReconciliationNeeds hook (non-suspending; initial data []) and invalidateOpenReconciliationNeeds() helper. - edit-applier: hard-impact apply returns { impact: 'hard', previousContent, previousRationale, openedNeedIds } with no `deferred: true` field. Undo for hard apply remains a no-op (card 3 ships real resolve semantics). Hard apply now also invalidates the reconciliation-needs query so the Pending review section refreshes. - patch-list-overlay: deferred-banner code path removed. New "Pending review" region renders open needs with a kind chip (supersedes / confirm) and source/target item references. Region only shows when openNeedsCount > 0. Saved-toast continues to fire after any apply (including hard). Tests: - 6 new cases in reconciliation-needs-route.test.ts cover empty list, scoped list, resolved-row exclusion, cross-spec isolation, 404, 400. - patch-list-overlay tests replace deferred-banner cases with Pending-review cases (rendering, empty state, co-existence with staged-changes, no V3 banner copy, saved-toast after hard apply). - side-chat-host tests stub the open-needs hook so PatchListOverlay can render without router/query scaffolding. - -specification-data test asserts the new query-key list shape. - edit-applier tests assert the new hard-impact applied shape and no-op undo. Verified: npm run verify (1053 tests, 0 lint warnings). SIDE_CHAT.md §5.1 soft-tier wording clarified to note no durable needs are opened for the soft path (only hard fires Path 1). --- docs/design/SIDE_CHAT.md | 2 +- .../__tests__/patch-list-overlay.test.tsx | 192 ++++++-------- .../__tests__/side-chat-host.test.tsx | 14 + src/client/components/patch-list-overlay.tsx | 243 ++++++++---------- src/client/lib/edit-applier.test.ts | 19 +- src/client/lib/edit-applier.ts | 46 ++-- .../specification/$id/-specification-data.ts | 36 ++- .../__tests__/-specification-data.test.tsx | 7 +- src/server/app.ts | 7 + src/server/reconciliation-needs-route.test.ts | 121 +++++++++ src/server/reconciliation-needs-route.ts | 33 +++ src/shared/reconciliation-need.ts | 20 ++ 12 files changed, 468 insertions(+), 272 deletions(-) create mode 100644 src/server/reconciliation-needs-route.test.ts create mode 100644 src/server/reconciliation-needs-route.ts create mode 100644 src/shared/reconciliation-need.ts diff --git a/docs/design/SIDE_CHAT.md b/docs/design/SIDE_CHAT.md index 9a154667..0fdb2cc5 100644 --- a/docs/design/SIDE_CHAT.md +++ b/docs/design/SIDE_CHAT.md @@ -209,7 +209,7 @@ When a patch with kind `edit` is applied, the system routes by **two questions i | Tier | Trigger | Path | |---|---|---| | **None** | `affectedCount === 0` (item is a graph leaf with no downstream edges) | Apply directly. Single-item content update; brief inline confirmation card in the panel: "Updated `[X]`." | -| **Soft** | `1 ≤ affectedCount ≤ 2` AND no anchor or affected item is in an active review set *(active = generated and not yet accepted)* | Apply with **soft recomputing**. Patch lands directly; brief inline confirmation lists the affected items: "Updated `[X]`; recomputed `[Y]`, `[Z]`." No cascade preview. | +| **Soft** | `1 ≤ affectedCount ≤ 2` AND no anchor or affected item is in an active review set *(active = generated and not yet accepted)* | Apply directly with affected-item context. Patch lands directly; brief inline confirmation lists the affected items: "Updated `[X]`; `[Y]`, `[Z]` may need a refresh." No cascade preview or durable `reconciliation_need` rows. | | **Hard** | High downstream count, OR any anchor or affected item is in an active review set | **Cascade preview** backed by `reconciliation_need` rows → batch-resolution mode in the side-chat panel (§5.3). The archived REVISIT_MODULE walk is superseded. | ### 5.2 Confidence model — V1 diff --git a/src/client/components/__tests__/patch-list-overlay.test.tsx b/src/client/components/__tests__/patch-list-overlay.test.tsx index 058f4800..0c871e3d 100644 --- a/src/client/components/__tests__/patch-list-overlay.test.tsx +++ b/src/client/components/__tests__/patch-list-overlay.test.tsx @@ -3,6 +3,8 @@ import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ReconciliationNeedRecord } from '@/shared/reconciliation-need.js'; + import { PatchListProvider, usePatchList, @@ -12,8 +14,45 @@ import { import { PatchListOverlayBridgeProvider } from '../patch-list-overlay-bridge.js'; import { PatchListOverlay } from '../patch-list-overlay.js'; +// Inject a controllable stub for the open-needs hook so the overlay can be +// tested without TanStack Router / QueryClientProvider scaffolding. Default +// returns []; individual tests override via setMockOpenNeeds. +let mockOpenNeeds: ReconciliationNeedRecord[] = []; +function setMockOpenNeeds(needs: ReconciliationNeedRecord[]): void { + mockOpenNeeds = needs; +} + +vi.mock('@/client/routes/specification/$id/-specification-data.js', () => ({ + useSpecificationOpenReconciliationNeeds: () => mockOpenNeeds, + // Stub the rest so accidental imports don't blow up. + specificationQueryKeys: { + bundle: (id: string) => ['specification', id, 'bundle'] as const, + entities: (id: string) => ['specification', id, 'entities'] as const, + entitiesProjectWide: (id: string) => ['specification', id, 'entities', 'project-wide'] as const, + reconciliationNeeds: (id: string) => ['specification', id, 'reconciliation-needs'] as const, + }, + invalidateOpenReconciliationNeeds: vi.fn(), +})); + +function makeNeed(overrides: Partial = {}): ReconciliationNeedRecord { + return { + id: overrides.id ?? 1, + specification_id: overrides.specification_id ?? 1, + source_item_id: overrides.source_item_id ?? 10, + target_item_id: overrides.target_item_id ?? 20, + kind: overrides.kind ?? 'needs_confirmation', + status: overrides.status ?? 'open', + reason: overrides.reason ?? null, + caused_by_turn_id: overrides.caused_by_turn_id ?? null, + caused_by_patch_id: overrides.caused_by_patch_id ?? null, + created_at: overrides.created_at ?? '2026-05-08T00:00:00Z', + resolved_at: overrides.resolved_at ?? null, + }; +} + afterEach(() => { cleanup(); + setMockOpenNeeds([]); }); beforeEach(() => { @@ -192,43 +231,41 @@ describe('PatchListOverlay', () => { expect(editApplier).toHaveBeenCalledTimes(1); }); - it('shows the deferred banner with the message after a hard-impact deferred apply', async () => { - const editApplier = vi.fn(() => - Promise.resolve({ - undo: () => Promise.resolve(), - applied: { - deferred: true, - impact: 'hard', - message: 'Hard impact — coming in V3 cascade preview', - }, - }), - ); - const appliers = makeAppliers({ edit: editApplier as unknown as PatchAppliers['edit'] }); + it('renders the Pending review section listing open reconciliation needs (V3.0 card 2)', () => { + setMockOpenNeeds([ + makeNeed({ id: 1, source_item_id: 10, target_item_id: 20, kind: 'needs_confirmation' }), + makeNeed({ id: 2, source_item_id: 10, target_item_id: 21, kind: 'supersedes' }), + ]); + const appliers = makeAppliers(); render( - , ); - fireEvent.click(screen.getByText('stage-edit')); - await act(async () => { - fireEvent.click(screen.getByRole('button', { name: /apply/i })); - }); - const banner = await screen.findByRole('status', { name: /hard impact deferred to v3/i }); - expect(banner.textContent).toContain('Hard impact — coming in V3 cascade preview'); - expect(screen.getByRole('button', { name: /dismiss/i })).toBeTruthy(); - // Saved-toast should NOT show in the overlay for a deferred-only batch - expect(screen.queryByRole('status', { name: /change saved/i })).toBeNull(); + const section = screen.getByRole('region', { name: /pending review/i }); + expect(section.getAttribute('data-open-needs-count')).toBe('2'); + expect(section.textContent).toContain('2 pending reviews'); + // Each need rendered with its kind chip and source→target reference + expect(section.querySelector('[data-need-id="1"]')?.textContent).toContain('source #10'); + expect(section.querySelector('[data-need-id="1"]')?.textContent).toContain('target #20'); + expect(section.querySelector('[data-need-id="1"][data-need-kind="needs_confirmation"]')).toBeTruthy(); + expect(section.querySelector('[data-need-id="2"][data-need-kind="supersedes"]')).toBeTruthy(); }); - it('clicking Dismiss hides the deferred banner', async () => { - const editApplier = vi.fn(() => - Promise.resolve({ - undo: () => Promise.resolve(), - applied: { deferred: true, impact: 'hard', message: 'Hard impact — coming in V3 cascade preview' }, - }), + it('hides the Pending review section when there are zero open needs', () => { + setMockOpenNeeds([]); + const appliers = makeAppliers(); + render( + + + , ); - const appliers = makeAppliers({ edit: editApplier as unknown as PatchAppliers['edit'] }); + expect(screen.queryByRole('region', { name: /pending review/i })).toBeNull(); + }); + + it('renders both staged-changes and Pending review when both exist', () => { + setMockOpenNeeds([makeNeed({ id: 7 })]); + const appliers = makeAppliers(); render( @@ -236,19 +273,20 @@ describe('PatchListOverlay', () => { , ); fireEvent.click(screen.getByText('stage-edit')); - await act(async () => { - fireEvent.click(screen.getByRole('button', { name: /apply/i })); - }); - await screen.findByRole('status', { name: /hard impact deferred to v3/i }); - fireEvent.click(screen.getByRole('button', { name: /dismiss/i })); - expect(screen.queryByRole('status', { name: /hard impact deferred to v3/i })).toBeNull(); + expect(screen.getByRole('region', { name: /staged changes/i })).toBeTruthy(); + expect(screen.getByRole('region', { name: /pending review/i })).toBeTruthy(); }); - it('hides the deferred banner when the applied batch is undone before the timeout', async () => { + it('does not surface any "Hard impact — coming in V3" banner copy', async () => { const editApplier = vi.fn(() => Promise.resolve({ undo: () => Promise.resolve(), - applied: { deferred: true, impact: 'hard', message: 'Hard impact — coming in V3 cascade preview' }, + applied: { + impact: 'hard', + previousContent: 'old', + previousRationale: null, + openedNeedIds: [101], + }, }), ); const appliers = makeAppliers({ edit: editApplier as unknown as PatchAppliers['edit'] }); @@ -256,19 +294,14 @@ describe('PatchListOverlay', () => { - , ); fireEvent.click(screen.getByText('stage-edit')); await act(async () => { fireEvent.click(screen.getByRole('button', { name: /apply/i })); }); - await screen.findByRole('status', { name: /hard impact deferred to v3/i }); - - await act(async () => { - fireEvent.click(screen.getByRole('button', { name: /undo-outside-overlay/i })); - }); - + expect(screen.queryByText(/coming in V3/i)).toBeNull(); + expect(screen.queryByText(/cascade pending review/i)).toBeNull(); expect(screen.queryByRole('status', { name: /hard impact deferred to v3/i })).toBeNull(); }); @@ -293,37 +326,23 @@ describe('PatchListOverlay', () => { expect(screen.getByRole('status', { name: /change saved/i })).toBeTruthy(); }); - it('auto-hides the deferred banner after the timeout even when staging activity churns mid-window', async () => { + it('shows the saved-toast after a hard-impact apply (V3.0 card 2 — no deferred banner blocking)', async () => { const editApplier = vi.fn(() => Promise.resolve({ undo: () => Promise.resolve(), - applied: { deferred: true, impact: 'hard', message: 'Hard impact — coming in V3 cascade preview' }, + applied: { + impact: 'hard', + previousContent: 'old', + previousRationale: null, + openedNeedIds: [101], + }, }), ); const appliers = makeAppliers({ edit: editApplier as unknown as PatchAppliers['edit'] }); - - function DiscardAllStaged() { - const patchList = usePatchList(); - const state = usePatchListState(); - return ( - - ); - } - render( - , ); @@ -331,50 +350,7 @@ describe('PatchListOverlay', () => { await act(async () => { fireEvent.click(screen.getByRole('button', { name: /apply/i })); }); - await screen.findByRole('status', { name: /hard impact deferred to v3/i }); - fireEvent.click(screen.getByText('stage-edit')); - - await act(async () => { - await vi.advanceTimersByTimeAsync(5_000); - }); - - fireEvent.click(screen.getByText('discard-all')); - - expect(screen.queryByRole('status', { name: /hard impact deferred to v3/i })).toBeNull(); - }); - - it('replaces a deferred banner with the saved-toast after a later non-deferred apply', async () => { - const editApplier = vi - .fn() - .mockResolvedValueOnce({ - undo: () => Promise.resolve(), - applied: { deferred: true, impact: 'hard', message: 'Hard impact — coming in V3 cascade preview' }, - }) - .mockResolvedValueOnce({ - undo: () => Promise.resolve(), - applied: { impact: 'soft', previousContent: 'old' }, - }); - const appliers = makeAppliers({ edit: editApplier as unknown as PatchAppliers['edit'] }); - render( - - - - , - ); - - fireEvent.click(screen.getByText('stage-edit')); - await act(async () => { - fireEvent.click(screen.getByRole('button', { name: /apply/i })); - }); - await screen.findByRole('status', { name: /hard impact deferred to v3/i }); - - fireEvent.click(screen.getByText('stage-edit')); - await act(async () => { - fireEvent.click(screen.getByRole('button', { name: /apply/i })); - }); - - expect(screen.queryByRole('status', { name: /hard impact deferred to v3/i })).toBeNull(); expect(screen.getByRole('status', { name: /change saved/i })).toBeTruthy(); }); }); diff --git a/src/client/components/__tests__/side-chat-host.test.tsx b/src/client/components/__tests__/side-chat-host.test.tsx index 6dda163a..c1692de0 100644 --- a/src/client/components/__tests__/side-chat-host.test.tsx +++ b/src/client/components/__tests__/side-chat-host.test.tsx @@ -23,6 +23,20 @@ vi.mock('@/client/lib/side-chat-stream.js', () => ({ streamSideChatResponse: vi.fn(() => Promise.resolve()), })); +// V3.0 card 2: PatchListOverlay reads open reconciliation needs via this hook, +// which depends on TanStack Router context. Stub it here so the side-chat host +// tests can render the overlay without a full router setup. +vi.mock('@/client/routes/specification/$id/-specification-data.js', () => ({ + useSpecificationOpenReconciliationNeeds: () => [], + specificationQueryKeys: { + bundle: (id: string) => ['specification', id, 'bundle'] as const, + entities: (id: string) => ['specification', id, 'entities'] as const, + entitiesProjectWide: (id: string) => ['specification', id, 'entities', 'project-wide'] as const, + reconciliationNeeds: (id: string) => ['specification', id, 'reconciliation-needs'] as const, + }, + invalidateOpenReconciliationNeeds: vi.fn(), +})); + vi.mock('@/client/lib/annotation-api.js', async (importOriginal) => { const actual = await importOriginal(); return { diff --git a/src/client/components/patch-list-overlay.tsx b/src/client/components/patch-list-overlay.tsx index 1e18aade..fb217c60 100644 --- a/src/client/components/patch-list-overlay.tsx +++ b/src/client/components/patch-list-overlay.tsx @@ -1,20 +1,18 @@ // PatchListOverlay — the canonical persistent patch-list surface (SIDE_CHAT.md §4). // -// Renders a sticky bar below the global app top-bar whenever there are staged -// patches or a transient post-apply message to surface. Lives outside the -// side-chat popover so it stays visible regardless of whether the panel is open -// — this is the surface V4's architect loop will eventually deposit into too. +// Renders sticky bars below the global app top-bar: +// • Staged-changes bar when there are staged patches. +// • Pending-review section listing open reconciliation_need rows (V3.0 card 2, +// SIDE_CHAT.md §5.3 — driven by useSpecificationOpenReconciliationNeeds). +// • Saved-toast for soft / none / hard impact applies (transient post-apply). // -// Two transient-message cases share this overlay: -// • saved-toast for soft / none impact applies (mirrors the popover's -// existing "Change saved" toast but works even with the panel closed) -// • deferred-banner for V2 hard-impact applies (per SIDE_CHAT.md §6.3: -// "Hard impact — coming in V3 cascade preview"). The deferred-applied -// marker carried in lastBatchAppliedMeta drives this; the popover's -// saved-toast is suppressed for deferred-only batches via prop. +// Lives outside the side-chat popover so it stays visible regardless of whether +// the panel is open — V4's architect loop will deposit into the same surface. import { useEffect, useRef, useState } from 'react'; +import { useSpecificationOpenReconciliationNeeds } from '@/client/routes/specification/$id/-specification-data.js'; + import { ContentDiff } from './content-diff.js'; import { ImpactChip } from './impact-chip.js'; import { kindAccentHex } from './knowledge-card'; @@ -25,38 +23,9 @@ import { usePatchListUndoOverride } from './patch-list-undo-context.js'; const MESSAGE_DURATION_MS = 5000; -interface DeferredBanner { - message: string; -} - -function readDeferredFromAppliedMeta(applied: unknown): DeferredBanner | null { - if (!applied || typeof applied !== 'object') { - return null; - } - const record = applied as { deferred?: unknown; message?: unknown }; - if (record.deferred !== true) { - return null; - } - const message = - typeof record.message === 'string' ? record.message : 'Hard impact — coming in V3 cascade preview'; - return { message }; -} - -function lastBatchHasDeferred( - meta: ReadonlyArray<{ patchId: string; applied: unknown }>, -): DeferredBanner | null { - for (const entry of meta) { - const banner = readDeferredFromAppliedMeta(entry.applied); - if (banner) { - return banner; - } - } - return null; -} - -function lastBatchHasNonDeferredApply(meta: ReadonlyArray<{ patchId: string; applied: unknown }>): boolean { +function lastBatchHasNonNullApply(meta: ReadonlyArray<{ patchId: string; applied: unknown }>): boolean { for (const entry of meta) { - if (readDeferredFromAppliedMeta(entry.applied) === null && entry.applied) { + if (entry.applied) { return true; } } @@ -106,10 +75,11 @@ export function PatchListOverlay(): React.ReactElement | null { const lastBatchAppliedMeta = useLastBatchAppliedMeta(); const undoOverride = usePatchListUndoOverride(); const overlayBridge = usePatchListOverlayBridge(); + const openNeeds = useSpecificationOpenReconciliationNeeds(); const stagedCount = state.staged.length; + const openNeedsCount = openNeeds.length; - const [deferredBanner, setDeferredBanner] = useState(null); const [savedToastVisible, setSavedToastVisible] = useState(false); const [expanded, setExpanded] = useState(false); const lastSeenBatchIdRef = useRef(null); @@ -126,25 +96,16 @@ export function PatchListOverlay(): React.ReactElement | null { // // Deps are intentionally narrow: a wider dep array re-runs cleanup on // unrelated churn (e.g. stagedCount change), cancelling the auto-hide - // timer and leaving the banner stuck on screen. + // timer and leaving the toast stuck on screen. useEffect(() => { if (state.lastBatchId === null || state.lastBatchId === lastSeenBatchIdRef.current) { return; } lastSeenBatchIdRef.current = state.lastBatchId; - const banner = lastBatchHasDeferred(lastBatchAppliedMeta); - const hasNonDeferred = lastBatchHasNonDeferredApply(lastBatchAppliedMeta); + const hasApply = lastBatchHasNonNullApply(lastBatchAppliedMeta); - if (banner) { - setDeferredBanner(banner); - setSavedToastVisible(false); - const handle = window.setTimeout(() => setDeferredBanner(null), MESSAGE_DURATION_MS); - return () => window.clearTimeout(handle); - } - - setDeferredBanner(null); - if (hasNonDeferred && !state.isApplying && stagedCount === 0 && state.canUndo) { + if (hasApply && !state.isApplying && stagedCount === 0 && state.canUndo) { setSavedToastVisible(true); const handle = window.setTimeout(() => setSavedToastVisible(false), MESSAGE_DURATION_MS); return () => window.clearTimeout(handle); @@ -152,10 +113,9 @@ export function PatchListOverlay(): React.ReactElement | null { // eslint-disable-next-line react-hooks/exhaustive-deps }, [state.lastBatchId]); - // Hide transient post-apply messages when canUndo flips back to false (the user undid). + // Hide transient post-apply toast when canUndo flips back to false (the user undid). useEffect(() => { if (!state.canUndo) { - setDeferredBanner(null); setSavedToastVisible(false); } }, [state.canUndo]); @@ -177,36 +137,38 @@ export function PatchListOverlay(): React.ReactElement | null { void patchList.apply(); } - // Nothing to surface: no staged patches, no transient message. - if (stagedCount === 0 && !deferredBanner && !savedToastVisible) { + // Nothing to surface: no staged patches, no open needs, no transient toast. + if (stagedCount === 0 && openNeedsCount === 0 && !savedToastVisible) { return null; } - if (stagedCount > 0) { - const countLabel = `${stagedCount} pending change${stagedCount === 1 ? '' : 's'}`; - return ( -
-
- + + › + + {countLabel} +
{state.canUndo ? (
+
+ {expanded ? ( +
    + {state.staged.map((patch) => ( + + ))} +
+ ) : null}
- {expanded ? ( -
    - {state.staged.map((patch) => ( - + ) : null} + {openNeedsCount > 0 ? ( +
    + + {openNeedsCount} pending review{openNeedsCount === 1 ? '' : 's'} + +
      + {openNeeds.map((need) => ( +
    • + + {need.kind === 'supersedes' ? 'supersedes' : 'confirm'} + + + source #{need.source_item_id} → target #{need.target_item_id} + +
      • ))}
    - ) : null} -
    - ); - } - - if (deferredBanner) { - return ( -
    - {deferredBanner.message} -
    + ) : null} + {savedToastVisible ? ( +
    - Dismiss - -
    - ); - } - - if (savedToastVisible) { - return ( -
    - Change saved - {state.canUndo ? ( - - ) : null} -
    - ); - } - - return null; + Change saved + {state.canUndo ? ( + + ) : null} + + ) : null} + + ); } diff --git a/src/client/lib/edit-applier.test.ts b/src/client/lib/edit-applier.test.ts index 3107816b..4e88b6be 100644 --- a/src/client/lib/edit-applier.test.ts +++ b/src/client/lib/edit-applier.test.ts @@ -108,11 +108,12 @@ describe('makeEditApplier', () => { ); }); - it('returns a deferred-applied marker on hard-impact response so the patch leaves staged cleanly', async () => { + it('returns applied state with openedNeedIds on hard-impact response (V3.0 card 2)', async () => { const fetchMock = vi.mocked(globalThis.fetch); - // V3.0: hard-impact apply now mutates source content and opens reconciliation needs, - // but card 1 keeps the deferred banner active by detecting impact === 'hard' on the - // client. Card 2 will replace the banner with the Pending review surface. + // V3.0 card 2: hard-impact apply has no `deferred: true` shape and no banner + // message. The patch transitions out of staged like any soft/none apply, and + // open reconciliation_need rows render in the patch-list-overlay's Pending + // review section (driven by a separate query). fetchMock.mockResolvedValueOnce( jsonResponse({ impact: 'hard', @@ -127,12 +128,14 @@ describe('makeEditApplier', () => { const applier = makeEditApplier(SPEC_ID); const result = await applier(makeEditPatch()); expect(result.applied).toEqual({ - deferred: true, impact: 'hard', - message: 'Hard impact — cascade pending review', + previousContent: 'Old content', + previousRationale: 'Old rationale', + openedNeedIds: [101, 102], }); - // Undo is a no-op for V3.0 deferred-banner behavior; card 2 will introduce real undo - // semantics (resolve / re-open needs) once the patch list overlay surfaces them. + // Undo for hard-impact apply is a no-op in card 2 (source mutation + // persists; user resolves via the Pending review section). Card 3 wires + // real undo semantics once the resolve endpoint exists. await expect(result.undo()).resolves.toBeUndefined(); }); diff --git a/src/client/lib/edit-applier.ts b/src/client/lib/edit-applier.ts index 28eec05f..7786a617 100644 --- a/src/client/lib/edit-applier.ts +++ b/src/client/lib/edit-applier.ts @@ -21,36 +21,45 @@ async function invalidateEntityQueriesAfterEdit(specificationId: number): Promis ]); } +// V3.0 card 2: refresh the Pending review section after a hard-impact apply +// opens new reconciliation_need rows. Card 3 will also call this from +// resolution actions. +async function invalidateOpenReconciliationNeedsAfterApply(specificationId: number): Promise { + await queryClient.invalidateQueries({ + queryKey: specificationQueryKeys.reconciliationNeeds(String(specificationId)), + }); +} + export function makeEditApplier(specificationId: number): ApplyPatchFn { return async (patch) => { const response = await editKnowledgeItemRequest(specificationId, patch.anchor.itemId, { content: patch.newContent, rationale: patch.newRationale, }); + if (response.previousContent === undefined) { + throw new Error('Edit applier: server reported updated but did not return previousContent'); + } + const previousContent = response.previousContent; + const previousRationale = response.previousRationale; + await invalidateEntityQueriesAfterEdit(specificationId); if (response.impact === 'hard') { - // V3.0 card 1: hard-impact apply now mutates the source and opens - // reconciliation_need rows on the server (D139, I112). The patch list - // overlay's Pending review surface lands in card 2; until then the - // deferred banner stays as the user-visible signal so the patch leaves - // staged cleanly. Undo is a no-op for the deferred-banner phase — card 2 - // wires real resolution actions through the queue. Entity queries are - // invalidated because content did mutate. - await invalidateEntityQueriesAfterEdit(specificationId); + // V3.0 card 2 (D139, I112): hard-impact apply mutates source and opens + // one reconciliation_need per typed dependency edge. The Pending review + // section in patch-list-overlay surfaces the queue. Undo for hard + // applies is a no-op for the source mutation in card 2 — card 3 wires + // real resolution semantics (resolve openedNeedIds + restore content). + // Until then the user resolves through the Pending review surface. + await invalidateOpenReconciliationNeedsAfterApply(specificationId); return { undo: async () => {}, applied: { - deferred: true, - impact: response.impact, - message: 'Hard impact — cascade pending review', + impact: 'hard', + previousContent, + previousRationale, + openedNeedIds: response.openedNeedIds, }, }; } - if (response.previousContent === undefined) { - throw new Error('Edit applier: server reported updated but did not return previousContent'); - } - const previousContent = response.previousContent; - const previousRationale = response.previousRationale; - await invalidateEntityQueriesAfterEdit(specificationId); return { undo: async () => { await editKnowledgeItemRequest(specificationId, patch.anchor.itemId, { @@ -58,6 +67,9 @@ export function makeEditApplier(specificationId: number): ApplyPatchFn ['specification', specificationId, 'entities'] as const, entitiesProjectWide: (specificationId: string) => ['specification', specificationId, 'entities', 'project-wide'] as const, + reconciliationNeeds: (specificationId: string) => + ['specification', specificationId, 'reconciliation-needs'] as const, }; const inflightSpecificationStateRequests = new Map>(); @@ -125,3 +128,34 @@ export function useSpecificationEntitiesProjectWide(): EntitiesData { queryFn: () => fetchSpecificationEntitiesProjectWide(specificationId), }).data; } + +async function fetchOpenReconciliationNeeds(specificationId: string): Promise { + const response = await fetch(`/api/specifications/${specificationId}/reconciliation-needs`); + if (!response.ok) { + throw new Error('Failed to load reconciliation needs'); + } + const body = (await response.json()) as { openNeeds: ReconciliationNeedRecord[] }; + return body.openNeeds; +} + +/** + * Open reconciliation_need rows for the current specification (V3.0 card 2). + * Returns [] until the producer (card 1) opens any. Non-suspending — the + * patch-list overlay renders without blocking on this fetch. + */ +export function useSpecificationOpenReconciliationNeeds(): ReconciliationNeedRecord[] { + const specificationId = useSpecificationId(); + + const { data } = useQuery({ + queryKey: specificationQueryKeys.reconciliationNeeds(specificationId), + queryFn: () => fetchOpenReconciliationNeeds(specificationId), + initialData: [], + }); + return data; +} + +export async function invalidateOpenReconciliationNeeds(specificationId: number): Promise { + await queryClient.invalidateQueries({ + queryKey: specificationQueryKeys.reconciliationNeeds(String(specificationId)), + }); +} diff --git a/src/client/routes/specification/$id/__tests__/-specification-data.test.tsx b/src/client/routes/specification/$id/__tests__/-specification-data.test.tsx index d587f804..1f84e668 100644 --- a/src/client/routes/specification/$id/__tests__/-specification-data.test.tsx +++ b/src/client/routes/specification/$id/__tests__/-specification-data.test.tsx @@ -130,7 +130,12 @@ describe('specification data ownership', () => { const firstPrime = await primeSpecificationBundle('42'); const secondPrime = await primeSpecificationBundle('42'); - expect(Object.keys(specificationQueryKeys)).toEqual(['bundle', 'entities', 'entitiesProjectWide']); + expect(Object.keys(specificationQueryKeys)).toEqual([ + 'bundle', + 'entities', + 'entitiesProjectWide', + 'reconciliationNeeds', + ]); expect(firstPrime).toEqual(minimalSpecificationState); expect(secondPrime).toEqual(minimalSpecificationState); expect(queryClient.getQueryData(specificationQueryKeys.bundle('42'))).toEqual(minimalSpecificationState); diff --git a/src/server/app.ts b/src/server/app.ts index d8c9e3eb..92a0b226 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -57,6 +57,7 @@ import { persistFallbackQuestionText, streamInterviewer } from './interview.js'; import { runObserver } from './observer.js'; import { safeDeserializeAssistantParts, serializeParts } from './parts.js'; import { submitPhaseIntentWithRuntimeCompatibility } from './phase-intent-runtime.js'; +import { handleListOpenReconciliationNeeds } from './reconciliation-needs-route.js'; import { handleSideChatRequest } from './side-chat-route.js'; import { createCoreTools } from './tools/index.js'; import { materializeTurnArtifacts } from './turn-artifacts.js'; @@ -241,6 +242,7 @@ export function createApp(dbPathOrOptions?: string | AppOptions): AppServices { '/api/specifications/:id/knowledge-edges/validate', ] as const; const specificationKnowledgeEdgesPaths = ['/api/specifications/:id/knowledge-edges'] as const; + const specificationReconciliationNeedsPaths = ['/api/specifications/:id/reconciliation-needs'] as const; const registerGet = (paths: readonly string[], handler: RequestHandler) => { for (const path of paths) { @@ -638,5 +640,10 @@ export function createApp(dbPathOrOptions?: string | AppOptions): AppServices { handleDeleteKnowledgeEdge(db, req, res); }); + // V3.0 card 2: list open reconciliation_need rows for the Pending review surface + registerGet(specificationReconciliationNeedsPaths, (req: Request, res: Response) => { + handleListOpenReconciliationNeeds(db, req, res); + }); + return { app, db }; } diff --git a/src/server/reconciliation-needs-route.test.ts b/src/server/reconciliation-needs-route.test.ts new file mode 100644 index 00000000..e49018a4 --- /dev/null +++ b/src/server/reconciliation-needs-route.test.ts @@ -0,0 +1,121 @@ +import request from 'supertest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { createApp } from './app.js'; +import { + addKnowledgeRelationship, + createKnowledgeItem, + openReconciliationNeed, + resolveReconciliationNeed, +} from './db.js'; + +let app: ReturnType['app']; +let db: ReturnType['db']; + +async function createSpec(name = 'Reconciliation needs spec'): Promise { + const res = await request(app).post('/api/specifications').send({ name }).expect(201); + return res.body.id; +} + +beforeEach(() => { + const created = createApp(); + app = created.app; + db = created.db; +}); + +afterEach(() => { + db.$client.close(); +}); + +describe('GET /api/specifications/:id/reconciliation-needs', () => { + it('returns an empty list when no needs exist for the specification', async () => { + const specId = await createSpec(); + + const res = await request(app).get(`/api/specifications/${specId}/reconciliation-needs`).expect(200); + + expect(res.body).toEqual({ openNeeds: [] }); + }); + + it('returns open reconciliation_need rows scoped to the specification', async () => { + const specId = await createSpec(); + const goal = createKnowledgeItem(db, specId, 'goal', 'Central goal'); + const r1 = createKnowledgeItem(db, specId, 'requirement', 'R1'); + const r2 = createKnowledgeItem(db, specId, 'requirement', 'R2'); + addKnowledgeRelationship(db, r1.id, goal.id, 'depends_on'); + addKnowledgeRelationship(db, r2.id, goal.id, 'derived_from'); + openReconciliationNeed(db, { + specificationId: specId, + sourceItemId: goal.id, + targetItemId: r1.id, + kind: 'needs_confirmation', + }); + openReconciliationNeed(db, { + specificationId: specId, + sourceItemId: goal.id, + targetItemId: r2.id, + kind: 'supersedes', + }); + + const res = await request(app).get(`/api/specifications/${specId}/reconciliation-needs`).expect(200); + + expect(res.body.openNeeds).toHaveLength(2); + const byTarget = new Map( + ( + res.body.openNeeds as Array<{ + id: number; + source_item_id: number; + target_item_id: number; + kind: string; + }> + ).map((n) => [n.target_item_id, { kind: n.kind, source_item_id: n.source_item_id }]), + ); + expect(byTarget.get(r1.id)?.kind).toBe('needs_confirmation'); + expect(byTarget.get(r2.id)?.kind).toBe('supersedes'); + for (const need of res.body.openNeeds) { + expect(need.source_item_id).toBe(goal.id); + } + }); + + it('excludes resolved reconciliation_need rows', async () => { + const specId = await createSpec(); + const a = createKnowledgeItem(db, specId, 'goal', 'A'); + const b = createKnowledgeItem(db, specId, 'requirement', 'B'); + const need = openReconciliationNeed(db, { + specificationId: specId, + sourceItemId: a.id, + targetItemId: b.id, + kind: 'needs_confirmation', + }); + + resolveReconciliationNeed(db, need.id); + + const res = await request(app).get(`/api/specifications/${specId}/reconciliation-needs`).expect(200); + + expect(res.body.openNeeds).toEqual([]); + }); + + it('does not leak needs from other specifications', async () => { + const ownerSpecId = await createSpec('Owner spec'); + const otherSpecId = await createSpec('Other spec'); + const a = createKnowledgeItem(db, ownerSpecId, 'goal', 'A'); + const b = createKnowledgeItem(db, ownerSpecId, 'requirement', 'B'); + openReconciliationNeed(db, { + specificationId: ownerSpecId, + sourceItemId: a.id, + targetItemId: b.id, + kind: 'needs_confirmation', + }); + + const res = await request(app).get(`/api/specifications/${otherSpecId}/reconciliation-needs`).expect(200); + + expect(res.body.openNeeds).toEqual([]); + }); + + it('returns 404 when the specification does not exist', async () => { + await request(app).get('/api/specifications/99999/reconciliation-needs').expect(404); + }); + + it('returns 400 on a non-numeric specification id', async () => { + await request(app).get('/api/specifications/abc/reconciliation-needs').expect(400); + }); +}); diff --git a/src/server/reconciliation-needs-route.ts b/src/server/reconciliation-needs-route.ts new file mode 100644 index 00000000..2bbaba8e --- /dev/null +++ b/src/server/reconciliation-needs-route.ts @@ -0,0 +1,33 @@ +// V3.0 card 2: read side of the reconciliation_need queue. +// +// The producer half (open needs on hard-impact apply) shipped in card 1; this +// handler exposes those rows so the patch-list overlay can render the +// "Pending review" section. Resolution actions and the resolve endpoint +// arrive in card 3. + +import type { Request, Response } from 'express'; + +import type { MutationErrorResponse } from '@/shared/api-types.js'; + +import { getSpecification, listOpenReconciliationNeeds, type DB, type ReconciliationNeed } from './db.js'; + +export interface ListOpenReconciliationNeedsResponse { + openNeeds: ReconciliationNeed[]; +} + +export function handleListOpenReconciliationNeeds(db: DB, req: Request, res: Response): void { + const specificationId = Number(req.params.id); + if (Number.isNaN(specificationId)) { + res.status(400).json({ error: 'Invalid specification ID' } satisfies MutationErrorResponse); + return; + } + + const specification = getSpecification(db, specificationId); + if (!specification) { + res.status(404).json({ error: 'Specification not found' } satisfies MutationErrorResponse); + return; + } + + const openNeeds = listOpenReconciliationNeeds(db, specificationId); + res.json({ openNeeds } satisfies ListOpenReconciliationNeedsResponse); +} diff --git a/src/shared/reconciliation-need.ts b/src/shared/reconciliation-need.ts new file mode 100644 index 00000000..13c11ddb --- /dev/null +++ b/src/shared/reconciliation-need.ts @@ -0,0 +1,20 @@ +// Shared shape for reconciliation_need rows surfaced to the client. +// Mirrors the durable schema with explicit field types so the client +// doesn't have to import server types. + +export type ReconciliationNeedKind = 'supersedes' | 'needs_confirmation'; +export type ReconciliationNeedStatus = 'open' | 'resolved'; + +export interface ReconciliationNeedRecord { + id: number; + specification_id: number; + source_item_id: number; + target_item_id: number; + kind: ReconciliationNeedKind; + status: ReconciliationNeedStatus; + reason: string | null; + caused_by_turn_id: number | null; + caused_by_patch_id: number | null; + created_at: string; + resolved_at: string | null; +} From 86432d22f32e894c21ab9e221426271d04d4c441 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Mon, 11 May 2026 11:11:54 +0200 Subject: [PATCH 2/7] FE-674: Dedupe invalidateOpenReconciliationNeeds in edit-applier Drop the private invalidateOpenReconciliationNeedsAfterApply copy and import the body-identical invalidateOpenReconciliationNeeds already exported from -specification-data.ts. specificationQueryKeys was already imported from that module, so this is a one-line import addition with no new dependency surface. --- src/client/lib/edit-applier.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/client/lib/edit-applier.ts b/src/client/lib/edit-applier.ts index 7786a617..c7b70b07 100644 --- a/src/client/lib/edit-applier.ts +++ b/src/client/lib/edit-applier.ts @@ -5,7 +5,10 @@ import type { EditPatch, } from '@/client/components/patch-list-host.js'; import { queryClient } from '@/client/query-client.js'; -import { specificationQueryKeys } from '@/client/routes/specification/$id/-specification-data.js'; +import { + invalidateOpenReconciliationNeeds, + specificationQueryKeys, +} from '@/client/routes/specification/$id/-specification-data.js'; import { createEdgeRequest, deleteEdgeRequest, editKnowledgeItemRequest } from './edit-api.js'; @@ -21,15 +24,6 @@ async function invalidateEntityQueriesAfterEdit(specificationId: number): Promis ]); } -// V3.0 card 2: refresh the Pending review section after a hard-impact apply -// opens new reconciliation_need rows. Card 3 will also call this from -// resolution actions. -async function invalidateOpenReconciliationNeedsAfterApply(specificationId: number): Promise { - await queryClient.invalidateQueries({ - queryKey: specificationQueryKeys.reconciliationNeeds(String(specificationId)), - }); -} - export function makeEditApplier(specificationId: number): ApplyPatchFn { return async (patch) => { const response = await editKnowledgeItemRequest(specificationId, patch.anchor.itemId, { @@ -49,7 +43,7 @@ export function makeEditApplier(specificationId: number): ApplyPatchFn {}, applied: { @@ -68,7 +62,7 @@ export function makeEditApplier(specificationId: number): ApplyPatchFn Date: Mon, 11 May 2026 11:13:41 +0200 Subject: [PATCH 3/7] FE-674: Stack overlay sticky sections in one column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staged-changes, Pending review, and saved-toast sections all used sticky top-0 z-30 individually, which made them fight for the same sticky offset when more than one was visible at a time (a real co- occurrence: an open need plus the post-apply saved toast). Result: bars visibly overlapped or covered each other while scrolling. Wrap the three sections in a single sticky flex column so they share one sticky context and stack vertically as one block — children no longer need sticky/z-index themselves. --- src/client/components/patch-list-overlay.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/client/components/patch-list-overlay.tsx b/src/client/components/patch-list-overlay.tsx index fb217c60..3524d444 100644 --- a/src/client/components/patch-list-overlay.tsx +++ b/src/client/components/patch-list-overlay.tsx @@ -144,15 +144,19 @@ export function PatchListOverlay(): React.ReactElement | null { const countLabel = `${stagedCount} pending change${stagedCount === 1 ? '' : 's'}`; + // Wrap the three sections in a single sticky flex column so they stack + // vertically as one block — putting `sticky top-0` on each child made them + // fight for the same offset and overlap when more than one was visible at + // the same time (e.g. an open need plus the post-apply saved toast). return ( - <> +
    {stagedCount > 0 ? (
    ); } From ca312414d37eda8702cc8468d09f4dc01e419d50 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Mon, 11 May 2026 11:41:07 +0200 Subject: [PATCH 4/7] FE-674: Force immediate fetch of open reconciliation needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit initialData: [] inherits the global staleTime: 30_000, which made React Query treat the empty seed as fresh and skip the first fetch for 30s — existing open needs were invisible in the Pending review section until the stale window expired. initialDataUpdatedAt: 0 marks the seed as stale so a background fetch runs immediately on mount. --- src/client/routes/specification/$id/-specification-data.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/routes/specification/$id/-specification-data.ts b/src/client/routes/specification/$id/-specification-data.ts index a1842a7a..aaa9cbad 100644 --- a/src/client/routes/specification/$id/-specification-data.ts +++ b/src/client/routes/specification/$id/-specification-data.ts @@ -150,6 +150,7 @@ export function useSpecificationOpenReconciliationNeeds(): ReconciliationNeedRec queryKey: specificationQueryKeys.reconciliationNeeds(specificationId), queryFn: () => fetchOpenReconciliationNeeds(specificationId), initialData: [], + initialDataUpdatedAt: 0, }); return data; } From 7bade112859170621f73504ac4fd0b9f711ea7a3 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Mon, 11 May 2026 11:44:59 +0200 Subject: [PATCH 5/7] FE-674: Strip over-commenting from sticky overlay fix --- src/client/components/patch-list-overlay.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/client/components/patch-list-overlay.tsx b/src/client/components/patch-list-overlay.tsx index 3524d444..a84a2280 100644 --- a/src/client/components/patch-list-overlay.tsx +++ b/src/client/components/patch-list-overlay.tsx @@ -144,10 +144,6 @@ export function PatchListOverlay(): React.ReactElement | null { const countLabel = `${stagedCount} pending change${stagedCount === 1 ? '' : 's'}`; - // Wrap the three sections in a single sticky flex column so they stack - // vertically as one block — putting `sticky top-0` on each child made them - // fight for the same offset and overlap when more than one was visible at - // the same time (e.g. an open need plus the post-apply saved toast). return (
    {stagedCount > 0 ? ( From fc594043759c40089ec97161785de9345b9c63c7 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Mon, 11 May 2026 12:06:14 +0200 Subject: [PATCH 6/7] FE-674: Restore undoResponse binding for hard-reclassification check --- src/client/lib/edit-applier.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/lib/edit-applier.ts b/src/client/lib/edit-applier.ts index c7b70b07..cc1e60b9 100644 --- a/src/client/lib/edit-applier.ts +++ b/src/client/lib/edit-applier.ts @@ -56,7 +56,7 @@ export function makeEditApplier(specificationId: number): ApplyPatchFn { - await editKnowledgeItemRequest(specificationId, patch.anchor.itemId, { + const undoResponse = await editKnowledgeItemRequest(specificationId, patch.anchor.itemId, { content: previousContent, rationale: previousRationale, }); From 994c3f3685b78e2b11898272a82969ab751d0c63 Mon Sep 17 00:00:00 2001 From: Kostandin Angjellari Date: Mon, 11 May 2026 15:42:16 +0200 Subject: [PATCH 7/7] FE-674: Fix text-text-sub typo in pending review list The class `text-text-sub` doesn't match any defined token; the theme exposes `--color-sub` (utility `text-sub`). The misnamed class was silently dropped, so the pending-review `
      ` inherited `text-ink` instead of the intended subdued gray. Co-authored-by: Cursor --- src/client/components/patch-list-overlay.tsx | 42 ++++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/client/components/patch-list-overlay.tsx b/src/client/components/patch-list-overlay.tsx index a84a2280..475c55a6 100644 --- a/src/client/components/patch-list-overlay.tsx +++ b/src/client/components/patch-list-overlay.tsx @@ -169,30 +169,30 @@ export function PatchListOverlay(): React.ReactElement | null { {countLabel} -
      - {state.canUndo ? ( +
      + {state.canUndo ? ( + + ) : null} - ) : null} - -
      +
    {expanded ? (
      {openNeedsCount} pending review{openNeedsCount === 1 ? '' : 's'} -
        +
          {openNeeds.map((need) => (