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..475c55a6 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 { +function lastBatchHasNonNullApply(meta: ReadonlyArray<{ patchId: string; applied: unknown }>): boolean { 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 { - 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,115 +137,126 @@ 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 ( -
-
- -
- {state.canUndo ? ( + + › + + {countLabel} + +
+ {state.canUndo ? ( + + ) : null} - ) : null} - +
+ {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..cc1e60b9 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'; @@ -27,37 +30,40 @@ export function makeEditApplier(specificationId: number): ApplyPatchFn {}, 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, { + const undoResponse = await editKnowledgeItemRequest(specificationId, patch.anchor.itemId, { content: previousContent, rationale: previousRationale, }); await invalidateEntityQueriesAfterEdit(specificationId); + if (undoResponse.impact === 'hard') { + await invalidateOpenReconciliationNeeds(specificationId); + } }, applied: { impact: response.impact, previousContent, previousRationale }, }; diff --git a/src/client/routes/specification/$id/-specification-data.ts b/src/client/routes/specification/$id/-specification-data.ts index 0b50b3f4..aaa9cbad 100644 --- a/src/client/routes/specification/$id/-specification-data.ts +++ b/src/client/routes/specification/$id/-specification-data.ts @@ -1,9 +1,10 @@ -import { useQueryClient, useSuspenseQuery } from '@tanstack/react-query'; +import { useQuery, useQueryClient, useSuspenseQuery } from '@tanstack/react-query'; import { useParams } from '@tanstack/react-router'; import { useCallback } from 'react'; import { queryClient } from '@/client/query-client.js'; import type { EntitiesData } from '@/shared/api-types.js'; +import type { ReconciliationNeedRecord } from '@/shared/reconciliation-need.js'; import type { SpecificationState } from '@/shared/specification.js'; export const specificationQueryKeys = { @@ -11,6 +12,8 @@ export const specificationQueryKeys = { entities: (specificationId: string) => ['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,35 @@ 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: [], + initialDataUpdatedAt: 0, + }); + 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; +}