From e6c4d87f1ece18aa788f70fb7a7b84ec7c588f14 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 14:33:26 +0200 Subject: [PATCH 01/14] feat: knowledge card components + shadcn primitives for slice 17 Add card-within-card pattern components (KnowledgeGroupCard, KnowledgeDetailCard, KnowledgeRow) with Figma design tokens, status badges (KindBadge, ReviewBadge, CountBadge), and MetadataRow. Add scroll-area, resizable, and skeleton shadcn primitives. All patterns verified via Ladle stories. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../components/knowledge-card.stories.tsx | 177 ++++++++++++ src/client/components/knowledge-card.tsx | 270 ++++++++++++++++++ src/client/components/ui/resizable.tsx | 42 +++ src/client/components/ui/scroll-area.tsx | 49 ++++ src/client/components/ui/skeleton.tsx | 9 + 5 files changed, 547 insertions(+) create mode 100644 src/client/components/knowledge-card.stories.tsx create mode 100644 src/client/components/knowledge-card.tsx create mode 100644 src/client/components/ui/resizable.tsx create mode 100644 src/client/components/ui/scroll-area.tsx create mode 100644 src/client/components/ui/skeleton.tsx diff --git a/src/client/components/knowledge-card.stories.tsx b/src/client/components/knowledge-card.stories.tsx new file mode 100644 index 00000000..928d98a8 --- /dev/null +++ b/src/client/components/knowledge-card.stories.tsx @@ -0,0 +1,177 @@ +import type { Story, StoryDefault } from '@ladle/react'; + +import { + CountBadge, + KindBadge, + KnowledgeDetailCard, + KnowledgeGroupCard, + KnowledgeRow, + MetadataRow, + ReviewBadge, + type KnowledgeEdgeData, + type KnowledgeItemData, +} from './knowledge-card'; + +export default { + title: 'Knowledge', +} satisfies StoryDefault; + +// ── Sample data ────────────────────────────────────────────────────── + +const sampleGoals: KnowledgeItemData[] = [ + { + id: 1, + kind: 'goal', + content: 'Enable structured specification elicitation through AI-guided interviews', + reviewStatus: 'approved', + }, + { + id: 2, + kind: 'goal', + content: 'Support both greenfield and brownfield project scoping', + reviewStatus: 'pending', + }, +]; + +const sampleDecisions: KnowledgeItemData[] = [ + { + id: 1, + kind: 'decision', + content: 'Use SQLite for local-first persistence', + rationale: + 'Single-file database simplifies distribution and backup. No server process needed. SQLite handles concurrent reads well for a single-user tool.', + subtype: 'architectural', + reviewStatus: 'approved', + }, + { + id: 2, + kind: 'decision', + content: 'Separate interviewer from observer agent', + rationale: + 'Keeps the interview prompt clean and focused. Observer extraction happens in a separate call during user think time.', + reviewStatus: 'approved', + }, + { + id: 3, + kind: 'decision', + content: 'Use Vercel AI SDK as the primary agent framework', + reviewStatus: 'pending', + }, +]; + +const sampleEdges: KnowledgeEdgeData[] = [ + { + type: 'depends_on', + sourceId: 2, + sourceCollection: 'decision', + targetId: 1, + targetCollection: 'decision', + }, + { + type: 'derived_from', + sourceId: 3, + sourceCollection: 'decision', + targetId: 2, + targetCollection: 'decision', + }, +]; + +// ── Badges ──────────────────────────────────────────────────────────── + +export const Badges: Story = () => { + return ( +
+

Badges

+ +
+

Kind badges

+
+ + + + + + + + +
+
+ +
+

Review badges

+
+ + + +
+
+ +
+

Count badges

+
+ + + +
+
+
+ ); +}; + +// ── Knowledge row ──────────────────────────────────────────────────── + +export const Rows: Story = () => { + return ( +
+

Knowledge rows

+
+ + + +
+
+ ); +}; + +// ── Group card ──────────────────────────────────────────────────────── + +export const GroupCard: Story = () => { + return ( +
+

Knowledge group card

+ + +
+ ); +}; + +// ── Detail card ────────────────────────────────────────────────────── + +export const DetailCard: Story = () => { + return ( +
+

Knowledge detail card

+ + +
+ ); +}; + +// ── Metadata row ───────────────────────────────────────────────────── + +export const Metadata: Story = () => { + return ( +
+

Metadata row

+
+ +
+
+ ); +}; diff --git a/src/client/components/knowledge-card.tsx b/src/client/components/knowledge-card.tsx new file mode 100644 index 00000000..878b93a6 --- /dev/null +++ b/src/client/components/knowledge-card.tsx @@ -0,0 +1,270 @@ +import { ChevronDown, Link as LinkIcon } from 'lucide-react'; + +import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible'; +import { cn } from '@/lib/utils'; + +import type { KnowledgeKind } from '../../shared/knowledge.js'; +import { knowledgeKindRegistry } from '../../shared/knowledge.js'; + +// ── ID prefix for each kind ─────────────────────────────────────────── + +const kindPrefix: Record = { + goal: 'G', + term: 'T', + context: 'Cx', + constraint: 'Co', + assumption: 'A', + decision: 'D', + requirement: 'R', + criterion: 'Cr', +}; + +export function itemLabel(kind: KnowledgeKind, id: number) { + return `${kindPrefix[kind]}${id}`; +} + +// ── Badges ──────────────────────────────────────────────────────────── + +export function KindBadge({ kind }: { kind: KnowledgeKind }) { + return ( + + {kindPrefix[kind]} + + ); +} + +export function ReviewBadge({ state }: { state: 'approved' | 'rejected' | 'pending' }) { + return ( + + {state === 'approved' ? 'Confirmed' : state === 'rejected' ? 'Rejected' : 'Pending'} + + ); +} + +export function CountBadge({ count }: { count: number }) { + return ( + + {count} + + ); +} + +// ── Knowledge item row — compact inline representation ──────────────── + +export interface KnowledgeItemData { + id: number; + kind: KnowledgeKind; + content: string; + rationale?: string; + subtype?: string; + reviewStatus?: 'approved' | 'rejected' | 'pending'; +} + +export interface KnowledgeEdgeData { + type: string; + sourceId: number; + sourceCollection: string; + targetId: number; + targetCollection: string; +} + +export function KnowledgeRow({ + item, + indent = false, + className, +}: { + item: KnowledgeItemData; + indent?: boolean; + className?: string; +}) { + return ( +
+ {indent && } +
+ {itemLabel(item.kind, item.id)} + {item.content} +
+ {item.reviewStatus && } +
+ ); +} + +// ── Knowledge group card — groups items by kind in card-within-card ──── + +export function KnowledgeGroupCard({ + kind, + items, + edges, +}: { + kind: KnowledgeKind; + items: KnowledgeItemData[]; + edges?: KnowledgeEdgeData[]; +}) { + const meta = knowledgeKindRegistry.find((e) => e.kind === kind); + if (!meta) return null; + + const confirmed = items.filter((i) => i.reviewStatus === 'approved').length; + const total = items.length; + + if (total === 0) return null; + + return ( +
+ {/* White header card — border overlaps parent via negative margin */} +
+ {/* Header row: kind label + count + stats */} +
+
+ + {meta.label} + +
+
+ + + {confirmed} / {total} + {' '} + confirmed + +
+
0 ? (confirmed / total) * 100 : 0}%`, + }} + /> +
+
+
+ + {/* Collapsible item list */} + +
+ + {total} {total === 1 ? 'item' : 'items'} + + +
+ + {items.map((item) => ( + + ))} + +
+
+ + {/* Edges section (in tinted body below white card) */} + {edges && edges.length > 0 && ( + +
+ + Connections + + +
+ +
+ {edges.map((edge, i) => ( +
+ {edge.type.replace(/_/g, ' ')} + + {' '} + → item #{edge.sourceId === items[0]?.id ? edge.targetId : edge.sourceId} + +
+ ))} +
+
+
+ )} +
+ ); +} + +// ── Knowledge detail card — expanded view with rationale + edges ────── + +export function KnowledgeDetailCard({ + item, + edges, +}: { + item: KnowledgeItemData; + edges?: KnowledgeEdgeData[]; +}) { + return ( +
+ {/* White header */} +
+
+
+
+ {itemLabel(item.kind, item.id)} + +
+

{item.content}

+
+ {item.reviewStatus && } +
+
+ + {/* Body sections */} +
+ {item.rationale && ( +
+

Rationale

+
+

{item.rationale}

+
+
+ )} + + {item.subtype && ( +
+

Subtype

+
+

{item.subtype}

+
+
+ )} + + {edges && edges.length > 0 && ( +
+

Connections

+ {edges.map((edge, i) => ( +
+ {edge.type.replace(/_/g, ' ')} + + {' '} + → item #{edge.sourceId === item.id ? edge.targetId : edge.sourceId} + +
+ ))} +
+ )} +
+
+ ); +} + +// ── Metadata row ────────────────────────────────────────────────────── + +export function MetadataRow({ items }: { items: { label: string; value: string }[] }) { + return ( +
+ {items.map((item) => ( +
+ {item.label} +

{item.value}

+
+ ))} +
+ ); +} diff --git a/src/client/components/ui/resizable.tsx b/src/client/components/ui/resizable.tsx new file mode 100644 index 00000000..ee82f591 --- /dev/null +++ b/src/client/components/ui/resizable.tsx @@ -0,0 +1,42 @@ +'use client'; + +import * as ResizablePrimitive from 'react-resizable-panels'; + +import { cn } from '@/lib/utils'; + +function ResizablePanelGroup({ className, ...props }: ResizablePrimitive.GroupProps) { + return ( + + ); +} + +function ResizablePanel({ ...props }: ResizablePrimitive.PanelProps) { + return ; +} + +function ResizableHandle({ + withHandle, + className, + ...props +}: ResizablePrimitive.SeparatorProps & { + withHandle?: boolean; +}) { + return ( + div]:rotate-90', + className, + )} + {...props} + > + {withHandle &&
} + + ); +} + +export { ResizableHandle, ResizablePanel, ResizablePanelGroup }; diff --git a/src/client/components/ui/scroll-area.tsx b/src/client/components/ui/scroll-area.tsx new file mode 100644 index 00000000..18841371 --- /dev/null +++ b/src/client/components/ui/scroll-area.tsx @@ -0,0 +1,49 @@ +import { ScrollArea as ScrollAreaPrimitive } from 'radix-ui'; +import * as React from 'react'; + +import { cn } from '@/lib/utils'; + +function ScrollArea({ + className, + children, + ...props +}: React.ComponentProps) { + return ( + + + {children} + + + + + ); +} + +function ScrollBar({ + className, + orientation = 'vertical', + ...props +}: React.ComponentProps) { + return ( + + + + ); +} + +export { ScrollArea, ScrollBar }; diff --git a/src/client/components/ui/skeleton.tsx b/src/client/components/ui/skeleton.tsx new file mode 100644 index 00000000..46025005 --- /dev/null +++ b/src/client/components/ui/skeleton.tsx @@ -0,0 +1,9 @@ +import { cn } from '@/lib/utils'; + +function Skeleton({ className, ...props }: React.ComponentProps<'div'>) { + return ( +
+ ); +} + +export { Skeleton }; From 9be7f03d61f42409440c80b4c4fe9c7dac5eb1aa Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 14:43:54 +0200 Subject: [PATCH 02/14] feat: integrate card-within-card pattern into KnowledgeWorkspace Replace basic list view with KnowledgeGroupCard (white header / tinted edges body), kind badges, review badges, count badges, progress bars, and empty-state cards for kinds with no items. Edge labels resolve to content text via content map. ScrollArea wraps the workspace. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/client/components/knowledge-card.tsx | 43 ++++--- src/client/routes/KnowledgeWorkspace.tsx | 156 +++++++++++------------ 2 files changed, 97 insertions(+), 102 deletions(-) diff --git a/src/client/components/knowledge-card.tsx b/src/client/components/knowledge-card.tsx index 878b93a6..9d06b822 100644 --- a/src/client/components/knowledge-card.tsx +++ b/src/client/components/knowledge-card.tsx @@ -43,7 +43,7 @@ export function ReviewBadge({ state }: { state: 'approved' | 'rejected' | 'pendi state === 'pending' && 'bg-wash text-hint', )} > - {state === 'approved' ? 'Confirmed' : state === 'rejected' ? 'Rejected' : 'Pending'} + {state === 'approved' ? 'Approved' : state === 'rejected' ? 'Rejected' : 'Pending'} ); } @@ -73,6 +73,7 @@ export interface KnowledgeEdgeData { sourceCollection: string; targetId: number; targetCollection: string; + targetLabel?: string; } export function KnowledgeRow({ @@ -173,15 +174,17 @@ export function KnowledgeGroupCard({
- {edges.map((edge, i) => ( -
- {edge.type.replace(/_/g, ' ')} - - {' '} - → item #{edge.sourceId === items[0]?.id ? edge.targetId : edge.sourceId} - -
- ))} + {edges.map((edge, i) => { + const edgeLabel = edge.type.replace(/_/g, ' '); + const displayLabel = edgeLabel.charAt(0).toUpperCase() + edgeLabel.slice(1); + const targetText = edge.targetLabel ?? `item #${edge.targetId}`; + return ( +
+ {displayLabel} + {targetText} +
+ ); + })}
@@ -238,15 +241,17 @@ export function KnowledgeDetailCard({ {edges && edges.length > 0 && (

Connections

- {edges.map((edge, i) => ( -
- {edge.type.replace(/_/g, ' ')} - - {' '} - → item #{edge.sourceId === item.id ? edge.targetId : edge.sourceId} - -
- ))} + {edges.map((edge, i) => { + const edgeLabel = edge.type.replace(/_/g, ' '); + const displayLabel = edgeLabel.charAt(0).toUpperCase() + edgeLabel.slice(1); + const targetText = edge.targetLabel ?? `item #${edge.targetId}`; + return ( +
+ {displayLabel} + {targetText} +
+ ); + })}
)}
diff --git a/src/client/routes/KnowledgeWorkspace.tsx b/src/client/routes/KnowledgeWorkspace.tsx index 5e77565c..2c1ea89b 100644 --- a/src/client/routes/KnowledgeWorkspace.tsx +++ b/src/client/routes/KnowledgeWorkspace.tsx @@ -1,104 +1,96 @@ import { Link, useLoaderData, useParams } from '@tanstack/react-router'; -import { Badge } from '@/components/ui/badge'; +import { EmptyCard } from '@/components/app-shell'; +import { + KnowledgeGroupCard, + MetadataRow, + type KnowledgeEdgeData, + type KnowledgeItemData, +} from '@/components/knowledge-card'; +import { ScrollArea } from '@/components/ui/scroll-area'; import type { EntitiesData } from '../../shared/api-types.js'; -import { knowledgeKindRegistry } from '../../shared/knowledge.js'; +import { knowledgeKindRegistry, type KnowledgeKind } from '../../shared/knowledge.js'; import type { KnowledgeWorkspaceLoaderData } from '../workspace/workspace-loader.js'; -function entityKey(collection: string, id: number) { - return `${collection}:${id}`; +function toKnowledgeItems( + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- entity collections are a union of heterogeneous shapes + rawItems: Array, + kind: KnowledgeKind, +): KnowledgeItemData[] { + return rawItems.map((item: Record) => ({ + id: item.id as number, + kind, + content: item.content as string, + rationale: typeof item.rationale === 'string' ? item.rationale : undefined, + subtype: typeof item.subtype === 'string' ? item.subtype : undefined, + reviewStatus: + item.reviewStatus === 'approved' || item.reviewStatus === 'rejected' || item.reviewStatus === 'pending' + ? item.reviewStatus + : undefined, + })); } -function buildContentMap(entities: EntitiesData) { +function buildContentMap(entities: EntitiesData): Map { const map = new Map(); for (const entry of knowledgeKindRegistry) { for (const item of entities[entry.collectionKey]) { - map.set(entityKey(entry.entityCollection, item.id), item.content); + map.set(`${entry.entityCollection}:${item.id}`, item.content); } } return map; } -function getDependencies( +function toKnowledgeEdges( entities: EntitiesData, + entityCollection: string, + itemIds: Set, contentMap: Map, - sourceCollection: string, - sourceId: number, -) { +): KnowledgeEdgeData[] { return entities.relationships - .filter( - (r) => r.type === 'depends_on' && r.source.collection === sourceCollection && r.source.id === sourceId, - ) - .map((r) => { - const key = entityKey(r.target.collection, r.target.id); - const label = contentMap.get(key); - return label ? { key, label } : null; - }) - .filter((d): d is { key: string; label: string } => d !== null); -} - -function ReviewBadge({ status }: { status: 'approved' | 'rejected' | 'pending' }) { - return ( - - {status === 'approved' ? 'Approved' : status === 'rejected' ? 'Rejected' : 'Pending'} - - ); + .filter((r) => r.source.collection === entityCollection && itemIds.has(r.source.id)) + .map((r) => ({ + type: r.type, + sourceId: r.source.id, + sourceCollection: r.source.collection, + targetId: r.target.id, + targetCollection: r.target.collection, + targetLabel: contentMap.get(`${r.target.collection}:${r.target.id}`), + })); } export function KnowledgeWorkspaceView({ entities }: { entities: EntitiesData }) { const contentMap = buildContentMap(entities); + const totalItems = knowledgeKindRegistry.reduce( + (sum, entry) => sum + entities[entry.collectionKey].length, + 0, + ); + const totalRelationships = entities.relationships.length; return ( -
-
+
+ + +
{knowledgeKindRegistry.map((entry) => { - const items = entities[entry.collectionKey]; - return ( -
-
-

{entry.label}

- {items.length > 0 && ( - - {items.length} - - )} -
+ const rawItems = entities[entry.collectionKey]; + if (rawItems.length === 0) { + return ( + + ); + } - {items.length === 0 ? ( -

{entry.emptyStateCopy}

- ) : ( -
- {items.map((item) => { - const reviewStatus = 'reviewStatus' in item ? item.reviewStatus : undefined; - const rationale = 'rationale' in item ? item.rationale : undefined; - const subtype = 'subtype' in item ? item.subtype : undefined; - const deps = getDependencies(entities, contentMap, entry.entityCollection, item.id); + const items = toKnowledgeItems(rawItems, entry.kind); + const itemIds = new Set(items.map((i) => i.id)); + const edges = toKnowledgeEdges(entities, entry.entityCollection, itemIds, contentMap); - return ( -
-
-

{item.content}

- {reviewStatus && } -
- {subtype &&

{subtype}

} - {rationale &&

{rationale}

} - {deps.length > 0 && ( -
-

Depends on

-
    - {deps.map((d) => ( -
  • {d.label}
  • - ))} -
-
- )} -
- ); - })} -
- )} -
+ return ( + ); })}
@@ -113,19 +105,17 @@ export function KnowledgeWorkspace() { }) as KnowledgeWorkspaceLoaderData; return ( -
-
- + +
+ ← Back to interview -

Knowledge

-

Review captured knowledge items and relationships.

+

Knowledge

+

+ Review captured knowledge items and relationships. +

-
+ ); } From e80f35f604b1269166389d126bb947113c3a8f7f Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 14:46:23 +0200 Subject: [PATCH 03/14] feat: resizable panels, skeleton loading, and route integration InterviewWorkspace conversation + entity sidebar now use ResizablePanelGroup (65/35 default split, adjustable via handle). Route transitions show skeleton loading for interview and knowledge workspaces via TanStack Router pendingComponent. KnowledgeWorkspace edge labels resolve to content text and show only outgoing edges. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/client/components/route-skeletons.tsx | 61 ++++++++ src/client/router.tsx | 3 + src/client/routes/InterviewWorkspace.tsx | 161 +++++++++++----------- 3 files changed, 148 insertions(+), 77 deletions(-) create mode 100644 src/client/components/route-skeletons.tsx diff --git a/src/client/components/route-skeletons.tsx b/src/client/components/route-skeletons.tsx new file mode 100644 index 00000000..dc2419e2 --- /dev/null +++ b/src/client/components/route-skeletons.tsx @@ -0,0 +1,61 @@ +import { Skeleton } from '@/components/ui/skeleton'; + +export function KnowledgeWorkspaceSkeleton() { + return ( +
+ {/* Metadata row skeleton */} +
+
+ + +
+
+ + +
+
+ + {/* Group card skeletons */} +
+ {[1, 2, 3].map((i) => ( +
+
+
+ + + +
+
+ + +
+
+
+ ))} +
+
+ ); +} + +export function InterviewWorkspaceSkeleton() { + return ( +
+
+ {/* Question skeleton */} + + + + + {/* Options skeleton */} +
+ {[1, 2, 3].map((i) => ( +
+ + +
+ ))} +
+
+
+ ); +} diff --git a/src/client/router.tsx b/src/client/router.tsx index f65de3ee..5988bb85 100644 --- a/src/client/router.tsx +++ b/src/client/router.tsx @@ -1,6 +1,7 @@ import { createRootRoute, createRoute, createRouter, Outlet } from '@tanstack/react-router'; import type { ProjectListItem } from '../shared/api-types.js'; +import { InterviewWorkspaceSkeleton, KnowledgeWorkspaceSkeleton } from './components/route-skeletons.js'; import { DebugSurfaceRouteComponent } from './routes/debug-surface.js'; import { fetchExportPreviewLoaderData } from './routes/export-loader.js'; import { ExportPreview } from './routes/ExportPreview.js'; @@ -39,6 +40,7 @@ const projectRoute = createRoute({ path: '/project/$id', loader: async ({ params }) => fetchInterviewWorkspaceLoaderData(params.id), component: InterviewWorkspace, + pendingComponent: InterviewWorkspaceSkeleton, }); // Knowledge workspace — read-only review surface @@ -47,6 +49,7 @@ const knowledgeRoute = createRoute({ path: '/project/$id/knowledge', loader: async ({ params }) => fetchKnowledgeWorkspaceLoaderData(params.id), component: KnowledgeWorkspace, + pendingComponent: KnowledgeWorkspaceSkeleton, }); // Export preview placeholder diff --git a/src/client/routes/InterviewWorkspace.tsx b/src/client/routes/InterviewWorkspace.tsx index 88b4aa7a..10ab0e37 100644 --- a/src/client/routes/InterviewWorkspace.tsx +++ b/src/client/routes/InterviewWorkspace.tsx @@ -18,6 +18,7 @@ import { import { Reasoning, ReasoningContent, ReasoningTrigger } from '@/components/ai-elements/reasoning'; import { Tool, ToolHeader, ToolContent, ToolInput, ToolOutput } from '@/components/ai-elements/tool'; import { EntitySidebar } from '@/components/EntitySidebar'; +import { ResizableHandle, ResizablePanel, ResizablePanelGroup } from '@/components/ui/resizable'; import { cn } from '@/lib/utils'; import type { ProjectState, ProjectStateTurn } from '../../shared/api-types.js'; @@ -332,90 +333,96 @@ export function InterviewWorkspace() {
-
-
- - - {chat.messages.map((msg, msgIdx) => { - const isLastAssistant = msg.role === 'assistant' && msgIdx === chat.messages.length - 1; - return ( - - - {msg.role === 'user' - ? msg.parts - ?.filter((p) => p.type === 'text') - .map((p, i) => {p.text}) - : renderParts(msg, isLastAssistant && chat.isStreaming)} - - - ); - })} + + +
+ + + {chat.messages.map((msg, msgIdx) => { + const isLastAssistant = msg.role === 'assistant' && msgIdx === chat.messages.length - 1; + return ( + + + {msg.role === 'user' + ? msg.parts + ?.filter((p) => p.type === 'text') + .map((p, i) => {p.text}) + : renderParts(msg, isLastAssistant && chat.isStreaming)} + + + ); + })} - {turnCard?.kind === 'persisted-turn' && ( - - )} + {turnCard?.kind === 'persisted-turn' && ( + + )} - {turnCard?.kind === 'pending-question' && ( - - )} + {turnCard?.kind === 'pending-question' && ( + + )} - {turnCard?.kind === 'persisted-turn' && turnCard.errorMessage && ( -

- {turnCard.errorMessage} -

- )} + {turnCard?.kind === 'persisted-turn' && turnCard.errorMessage && ( +

+ {turnCard.errorMessage} +

+ )} - {phaseSummary && ( - chat.confirmPhaseClosure(phaseSummary.phase, phaseSummary.turnId)} - /> - )} -
- -
+ {phaseSummary && ( + chat.confirmPhaseClosure(phaseSummary.phase, phaseSummary.turnId)} + /> + )} + + + - {promptInput.visible && ( -
-
- - - - - - - - + {promptInput.visible && ( +
+
+ + + + + + + + +
-
- )} -
+ )} +
+
- -
+ + + + + +
); } From ef321ceb308acfe815122bf2728f51ab97c76e09 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 14:46:45 +0200 Subject: [PATCH 04/14] chore: mark slice 17 done in PLAN.md Co-Authored-By: Claude Opus 4.6 (1M context) --- memory/PLAN.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/memory/PLAN.md b/memory/PLAN.md index 85325f23..ed2b9694 100644 --- a/memory/PLAN.md +++ b/memory/PLAN.md @@ -147,7 +147,10 @@ ### Slices -17. **UI refinement + design-system alignment** — Port design tokens, layout primitives, card patterns, and component iteration environment from the parallel `brunch-ui` prototype. **Foundation**: Inter Variable font (replacing Geist), Figma-precise color ramp (ink/sub/hint/rule/wash/tint), shadow tokens (card/ring/card-ring), fine-grained typography scale (xxs 10px–sm 16px), font weight discipline (regular/medium/semi-bold only). **Layout shell**: AppHeader, StageSidebar/PhaseSidebar with working collapse/expand toggle, resizable main+right panels via `react-resizable-panels`, AppFooter spanning full width. **Components**: card-within-card pattern (white header / tinted body) for knowledge group cards and question cards; collapsible question card (V2 with inline `why` grounding, answered cards collapse to read-only summary, re-answering routes through revisit model not casual toggle); empty-state vocabulary (6 patterns: text-only, with icon, with CTA, centered hero, inline within list, attention/warning); metadata rows (label-over-value flex columns); badge refinement (mono font, no black-on-color, no all-caps); mandatory skeleton loading for all pending states. **Infrastructure**: Ladle story environment (`.ladle/` config with theme provider, `@source` directives, stories as living component reference), shadcn CLI + `components.json`. **Dependencies**: `@fontsource-variable/inter`, `react-resizable-panels`, `@ladle/react`, `shadcn`. `not-started` +17. **UI refinement + design-system alignment** — Port design tokens, layout primitives, card patterns, and component iteration environment from the parallel `brunch-ui` prototype. **Foundation**: Inter Variable font (replacing Geist), Figma-precise color ramp (ink/sub/hint/rule/wash/tint), shadow tokens (card/ring/card-ring), fine-grained typography scale (xxs 10px–sm 16px), font weight discipline (regular/medium/semi-bold only). **Layout shell**: AppHeader, StageSidebar/PhaseSidebar with working collapse/expand toggle, resizable main+right panels via `react-resizable-panels`, AppFooter spanning full width. **Components**: card-within-card pattern (white header / tinted body) for knowledge group cards and question cards; collapsible question card (V2 with inline `why` grounding, answered cards collapse to read-only summary, re-answering routes through revisit model not casual toggle); empty-state vocabulary (6 patterns: text-only, with icon, with CTA, centered hero, inline within list, attention/warning); metadata rows (label-over-value flex columns); badge refinement (mono font, no black-on-color, no all-caps); mandatory skeleton loading for all pending states. **Infrastructure**: Ladle story environment (`.ladle/` config with theme provider, `@source` directives, stories as living component reference), shadcn CLI + `components.json`. **Dependencies**: `@fontsource-variable/inter`, `react-resizable-panels`, `@ladle/react`, `shadcn`. `done` + - Shipped: Inter Variable font, Figma color ramp (ink/sub/hint/rule/wash/tint), typography scale (xxs–sm-plus), shadow tokens, Ladle story environment, StageSidebar/PhaseSidebar toggle, card-within-card pattern in KnowledgeWorkspace, resizable panels in InterviewWorkspace, skeleton loading on route transitions, empty-state vocabulary + - Evidence: 11 Ladle stories, KnowledgeWorkspace.test.tsx (5 pass), 236/237 tests pass, npm run verify green + - Debt: dark mode token alignment, question card V2 (TurnCard refactor), badge mono font, per-route skeleton tuning - Requirements: → SPEC.md §Requirements #4, #5, #7, #9, #15 - Decisions: → SPEC.md §Decisions D58, D59, D69 - Candidate invariant goals: design tokens are Figma-authoritative and shared between app and Ladle stories; layout shell supports sidebar collapse/expand without breaking workspace data flow; card patterns are composable across knowledge workspace, interview workspace, and dashboard; empty-state and skeleton patterns are systematic, not ad-hoc; answered question cards visually communicate finality — re-opening to edit is a revisit-model action, not a UI toggle From 538f497518791f3ead2032524a4fb32b76207f4c Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:03:52 +0200 Subject: [PATCH 05/14] test: characterize workspace and transport seams --- memory/REFACTOR.md | 67 +++++++++ src/client/mutations/client-mutation.test.ts | 30 +++++ src/client/routes/export-loader.test.ts | 12 ++ .../workspace/workspace-controller.test.tsx | 127 ++++++++++++++++++ src/client/workspace/workspace-loader.test.ts | 40 ++++++ 5 files changed, 276 insertions(+) create mode 100644 memory/REFACTOR.md diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md new file mode 100644 index 00000000..20c70dd5 --- /dev/null +++ b/memory/REFACTOR.md @@ -0,0 +1,67 @@ +## Problem Statement + +The workspace boundary is carrying too much hidden synchronization work. + +Two user-visible flows still depend on post-render correction instead of clear ownership: + +- chat state is replaced after render when project identity changes +- workspace entity data is mirrored from route-loaded state into cached query state after render + +At the same time, the JSON transport boundary is too implicit: + +- client-facing types are derived from server implementation return types instead of an explicit transport contract +- loaders and mutations trust JSON payloads by assertion rather than runtime proof +- several request and review payloads still encode meaningful states through optional fields + +Together, this makes refactoring riskier than it needs to be. State ownership is harder to reason about, stale-frame behavior is harder to rule out, and server refactors can widen or reshape client contracts without that change becoming explicit. + +## Solution + +Refactor the workspace and transport seams around explicit contracts and single-owner state boundaries. + +Target state: + +- the REST/JSON boundary is defined by shared transport DTOs or schemas, not by server implementation inference +- client loaders and mutations parse validated transport payloads instead of casting JSON +- chat reset on project identity change happens through ownership/remount semantics rather than an effect that overwrites live state after render +- entity data has one authority per route transition, with loader/query coordination happening through an explicit boundary instead of effect-driven cache mirroring +- exported boundary functions touched by this work expose explicit signatures +- snapshot and view-model types touched by the refactor express immutability directly + +This keeps the refactor focused on the highest-risk overlap between the React review and the TypeScript review: boundary drift plus effect-driven synchronization in the workspace flow. + +## Commits + +1. Add characterization coverage for the refactor seam: project navigation, same-project refresh, entity refresh after observer invalidation, and JSON-boundary success/failure cases. +2. Introduce explicit shared transport contracts for project state, entity payloads, export payloads, mutation errors, and turn-response request/response payloads without changing runtime behavior yet. +3. Move client loader and mutation helpers to parse transport payloads through those shared contracts and remove unchecked JSON casts from the workspace-facing routes and mutations. +4. Narrow the turn-response and review payload shapes so meaningful request modes are represented explicitly instead of through bags of optional fields. +5. Add explicit return types to the exported boundary functions touched by the transport pass so the module surfaces stay legible during later commits. +6. Put the live chat owner behind a project-identity reset boundary and delete the post-render chat hydration overwrite. +7. Collapse workspace entity hydration to one authority per route transition and remove the effect-based loader-to-cache mirroring step. +8. Mark touched snapshot and view-model contracts as readonly and normalize type-only imports in the refactored boundary modules. + +## Decisions + +- Recommended scope: one boundary-first refactor centered on workspace transport and state ownership, not a repo-wide hygiene sweep. +- Shared transport contracts become the authority for REST/JSON payload shape at the client/server seam. +- The live chat session owns only in-session state; route identity owns when seeded history resets. +- Entity state must have one clear owner per navigation event; cache refresh should be explicit, not mirrored after render. +- Optional-field cleanup is limited to payloads directly in this seam, not every optional-heavy type in the repo. +- Explicit return types and readonly modeling are included only for modules touched by this refactor, not as a global style pass. + +## Testing Decisions + +- Test behavior, not implementation: navigation should show the correct transcript without stale carryover; same-project refresh should preserve live chat while durable state updates; entity refresh should converge through one authority; malformed JSON should fail at the boundary with controlled errors. +- Use the existing workspace controller, workspace data, workspace loader, export loader, client mutation, and app-route tests as prior art. +- Prefer contract tests around transport parsing and route behavior over tests that assert internal hook/effect structure. +- Characterization tests are sufficient to begin; this area already has meaningful coverage, so the first commit can strengthen boundary-focused tests instead of blocking the refactor behind a separate testing-only project. + +## Out of Scope + +- Vendor-like UI source and AI-elements internals. +- A repo-wide explicit-return-type campaign. +- A repo-wide import-hygiene cleanup. +- Full replacement of every implementation-derived shared type in unrelated server/client areas. +- Drizzle assertion cleanup outside the touched transport seam. +- New workflow features or UX redesign beyond preserving current behavior while clarifying boundaries. diff --git a/src/client/mutations/client-mutation.test.ts b/src/client/mutations/client-mutation.test.ts index 79cef28d..a6c5f019 100644 --- a/src/client/mutations/client-mutation.test.ts +++ b/src/client/mutations/client-mutation.test.ts @@ -14,6 +14,19 @@ describe('client mutation', () => { vi.unstubAllGlobals(); }); + it('returns parsed json on success', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ ok: true, id: 7 }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect( + postJsonMutation('/api/projects', { name: 'New project' }, 'Failed to create project'), + ).resolves.toEqual({ ok: true, id: 7 }); + }); + it('surfaces network failures with the caller fallback message', async () => { fetchMock.mockRejectedValueOnce(new TypeError('network down')); @@ -22,6 +35,23 @@ describe('client mutation', () => { ).rejects.toEqual(new ClientMutationError('Failed to create project')); }); + it('surfaces json error messages from the server', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ error: 'Project name already exists' }), { + status: 409, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect( + postJsonMutation('/api/projects', { name: 'New project' }, 'Failed to create project'), + ).rejects.toMatchObject({ + name: 'ClientMutationError', + message: 'Project name already exists', + status: 409, + }); + }); + it('falls back when an error response body is not json', async () => { fetchMock.mockResolvedValueOnce( new Response('upstream exploded', { diff --git a/src/client/routes/export-loader.test.ts b/src/client/routes/export-loader.test.ts index f6df2876..71bf7d04 100644 --- a/src/client/routes/export-loader.test.ts +++ b/src/client/routes/export-loader.test.ts @@ -30,4 +30,16 @@ describe('export route loader', () => { }); expect(fetchMock).toHaveBeenCalledWith('/api/projects/7/export'); }); + + it('rejects when the export payload is malformed json', async () => { + fetchMock.mockResolvedValueOnce( + new Response('{', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect(fetchExportPreviewLoaderData(7)).rejects.toThrow(); + expect(fetchMock).toHaveBeenCalledWith('/api/projects/7/export'); + }); }); diff --git a/src/client/workspace/workspace-controller.test.tsx b/src/client/workspace/workspace-controller.test.tsx index 6adc6f80..137e8948 100644 --- a/src/client/workspace/workspace-controller.test.tsx +++ b/src/client/workspace/workspace-controller.test.tsx @@ -366,6 +366,133 @@ describe('workspace controller', () => { }); }); + it('rehydrates the transcript on explicit project navigation', async () => { + const rendered = renderController(); + + expect((await screen.findByTestId('messages')).textContent).toBe( + 'Build the web app|What should we build first?', + ); + await waitFor(() => { + expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); + }); + useChatHarness.setMessages.mockClear(); + + currentLoaderData = createWorkspaceLoaderData({ + projectId: 2, + assistantText: 'Which platform should we target now?', + answer: 'Ship the desktop app', + entitySnapshot: { + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [], + decisions: [ + { + id: 8, + project_id: 2, + content: 'Prefer the desktop app', + rationale: 'Matches the updated brief', + }, + ], + assumptions: [], + relationships: [], + }, + }); + + rendered.rerender( + + + , + ); + + await waitFor(() => { + expect(useChatHarness.setMessages).toHaveBeenCalledWith([ + { id: 'turn-1-answer', role: 'user', parts: [{ type: 'text', text: 'Ship the desktop app' }] }, + { + id: 'turn-1-assistant', + role: 'assistant', + parts: [{ type: 'text', text: 'Which platform should we target now?' }], + }, + ]); + }); + expect(screen.getByTestId('project-name').textContent).toBe('Project 2'); + expect(screen.getByTestId('messages').textContent).toBe( + 'Ship the desktop app|Which platform should we target now?', + ); + expect(screen.getByTestId('decisions').textContent).toBe('Prefer the desktop app'); + }); + + it('refetches durable entities when observer output invalidates the active entity query', async () => { + currentLoaderData = createWorkspaceLoaderData({ + entitySnapshot: { + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [], + decisions: [], + assumptions: [], + relationships: [], + }, + }); + fetchMock.mockResolvedValueOnce( + new Response( + JSON.stringify({ + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [], + decisions: [ + { + id: 9, + project_id: 1, + content: 'Start with the web app', + rationale: 'Observer extracted a new decision', + }, + ], + assumptions: [], + relationships: [], + } satisfies EntitiesData), + { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }, + ), + ); + + renderController(); + + expect((await screen.findByTestId('decisions')).textContent).toBe('none'); + + await act(async () => { + useChatHarness.onData?.({ + type: 'data-observer-result', + data: { + entityIds: { + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [], + decisions: [9], + assumptions: [], + }, + }, + }); + }); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith('/api/projects/1/entities'); + expect(screen.getByTestId('decisions').textContent).toBe('Start with the web app'); + }); + }); + it('keeps the live transcript stable on same-project refresh while updating durable entities', async () => { const rendered = renderController(); diff --git a/src/client/workspace/workspace-loader.test.ts b/src/client/workspace/workspace-loader.test.ts index c44b80b1..5c3a58b4 100644 --- a/src/client/workspace/workspace-loader.test.ts +++ b/src/client/workspace/workspace-loader.test.ts @@ -120,6 +120,46 @@ describe('workspace route loaders', () => { expect(fetchMock).toHaveBeenNthCalledWith(2, '/api/projects/7/entities'); }); + it('rejects when the project payload is malformed json', async () => { + fetchMock + .mockResolvedValueOnce( + new Response('{', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify(entitySnapshot), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect(fetchInterviewWorkspaceLoaderData(7)).rejects.toThrow(); + expect(fetchMock).toHaveBeenNthCalledWith(1, '/api/projects/7'); + expect(fetchMock).toHaveBeenNthCalledWith(2, '/api/projects/7/entities'); + }); + + it('rejects when the entity payload is malformed json', async () => { + fetchMock + .mockResolvedValueOnce( + new Response(JSON.stringify(projectState), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ) + .mockResolvedValueOnce( + new Response('{', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect(fetchInterviewWorkspaceLoaderData(7)).rejects.toThrow(); + expect(fetchMock).toHaveBeenNthCalledWith(1, '/api/projects/7'); + expect(fetchMock).toHaveBeenNthCalledWith(2, '/api/projects/7/entities'); + }); + it('fails knowledge workspace loading when the project does not exist', async () => { fetchMock.mockResolvedValueOnce(new Response('Not found', { status: 404 })); From c8710bf8e6a087fc03ee0cd9dc0625f3dd0cc6e8 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:09:43 +0200 Subject: [PATCH 06/14] refactor: add explicit shared transport contracts --- src/client/mutations/client-mutation.ts | 4 +- src/client/mutations/workspace-mutations.ts | 8 +- src/client/routes/ExportPreview.test.tsx | 2 +- src/client/routes/export-loader.ts | 5 +- src/server/app.ts | 39 ++-- src/shared/api-types.test.ts | 212 +++++++++++++++++++ src/shared/api-types.ts | 219 +++++++++++++++++++- 7 files changed, 459 insertions(+), 30 deletions(-) create mode 100644 src/shared/api-types.test.ts diff --git a/src/client/mutations/client-mutation.ts b/src/client/mutations/client-mutation.ts index 5b71d225..8245c124 100644 --- a/src/client/mutations/client-mutation.ts +++ b/src/client/mutations/client-mutation.ts @@ -1,8 +1,6 @@ import { useMutation } from '@tanstack/react-query'; -interface MutationErrorResponse { - error?: string; -} +import type { MutationErrorResponse } from '../../shared/api-types.js'; export class ClientMutationError extends Error { readonly status: number | undefined; diff --git a/src/client/mutations/workspace-mutations.ts b/src/client/mutations/workspace-mutations.ts index 1a9583f2..9f82ef7c 100644 --- a/src/client/mutations/workspace-mutations.ts +++ b/src/client/mutations/workspace-mutations.ts @@ -1,6 +1,10 @@ import { useRouter } from '@tanstack/react-router'; -import type { ProjectStateTurn } from '../../shared/api-types.js'; +import type { + ProjectStateTurn, + SubmitTurnResponseRequest, + SubmitTurnResponseResponse, +} from '../../shared/api-types.js'; import { formatTurnResponseText } from '../../shared/chat.js'; import { findTurnOptionsByPositions } from '../workspace/workspace-controller-core.js'; import { postJsonMutation, useClientMutation } from './client-mutation.js'; @@ -17,7 +21,7 @@ export function useSubmitTurnResponseMutation({ const router = useRouter(); const mutation = useClientMutation( (variables: { turnId: number; positions?: number[]; freeText?: string }) => - postJsonMutation<{ ok: boolean }, { positions?: number[]; freeText?: string }>( + postJsonMutation( `/api/projects/${projectId}/turns/${variables.turnId}/response`, { ...(variables.positions?.length ? { positions: variables.positions } : {}), diff --git a/src/client/routes/ExportPreview.test.tsx b/src/client/routes/ExportPreview.test.tsx index 78216437..2269eb2c 100644 --- a/src/client/routes/ExportPreview.test.tsx +++ b/src/client/routes/ExportPreview.test.tsx @@ -3,7 +3,7 @@ import { cleanup, render, screen } from '@testing-library/react'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import type { ExportLoaderData } from './export-loader.js'; +import type { ExportLoaderData } from '../../shared/api-types.js'; import { ExportPreview } from './ExportPreview.js'; let currentLoaderData: ExportLoaderData; diff --git a/src/client/routes/export-loader.ts b/src/client/routes/export-loader.ts index abb64fc8..9595a4cc 100644 --- a/src/client/routes/export-loader.ts +++ b/src/client/routes/export-loader.ts @@ -1,7 +1,4 @@ -export interface ExportLoaderData { - ready: boolean; - markdown?: string; -} +import type { ExportLoaderData } from '../../shared/api-types.js'; export async function fetchExportPreviewLoaderData(projectId: number | string): Promise { const id = String(projectId); diff --git a/src/server/app.ts b/src/server/app.ts index 091f9f9b..831ac7b5 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -2,7 +2,14 @@ import { createUIMessageStream, pipeUIMessageStreamToResponse, validateUIMessage import express from 'express'; import type { Request, Response } from 'express'; -import type { ProjectState, ProjectListItem, EntitiesData } from '../shared/api-types.js'; +import type { + EntitiesData, + ExportLoaderData, + MutationErrorResponse, + ProjectListItem, + ProjectState, + SubmitTurnResponseResponse, +} from '../shared/api-types.js'; import { assistantPartsSchema, brunchDataPartSchemas, @@ -61,7 +68,7 @@ export function createApp(dbPath?: string) { app.post('/api/projects', (req: Request, res: Response) => { const name = typeof req.body.name === 'string' ? req.body.name.trim() : ''; if (!name) { - res.status(400).json({ error: 'name is required' }); + res.status(400).json({ error: 'name is required' } satisfies MutationErrorResponse); return; } const project = createNewProject(db, name); @@ -72,12 +79,12 @@ export function createApp(dbPath?: string) { app.get('/api/projects/:id', (req: Request, res: Response) => { const id = Number(req.params.id); if (Number.isNaN(id)) { - res.status(400).json({ error: 'Invalid project ID' }); + res.status(400).json({ error: 'Invalid project ID' } satisfies MutationErrorResponse); return; } const state = getProjectState(db, id); if (!state) { - res.status(404).json({ error: 'Project not found' }); + res.status(404).json({ error: 'Project not found' } satisfies MutationErrorResponse); return; } res.json(state satisfies ProjectState); @@ -96,24 +103,28 @@ export function createApp(dbPath?: string) { const freeText = typeof req.body.freeText === 'string' ? req.body.freeText.trim() : undefined; if (Number.isNaN(projectId) || Number.isNaN(turnId)) { - res.status(400).json({ error: 'Invalid IDs' }); + res.status(400).json({ error: 'Invalid IDs' } satisfies MutationErrorResponse); return; } if (uniquePositions.length === 0 && !freeText) { - res.status(400).json({ error: 'positions are required unless freeText is provided' }); + res + .status(400) + .json({ + error: 'positions are required unless freeText is provided', + } satisfies MutationErrorResponse); return; } const turn = getTurn(db, turnId); if (!turn || turn.project_id !== projectId) { - res.status(404).json({ error: 'Turn not found' }); + res.status(404).json({ error: 'Turn not found' } satisfies MutationErrorResponse); return; } const options = getOptionsForTurn(db, turnId); const selectedOptions = options.filter((option) => uniquePositions.includes(option.position)); if (selectedOptions.length !== uniquePositions.length) { - res.status(400).json({ error: 'Selected option not found' }); + res.status(400).json({ error: 'Selected option not found' } satisfies MutationErrorResponse); return; } applyTurnResponseSelections(db, turnId, uniquePositions); @@ -140,14 +151,14 @@ export function createApp(dbPath?: string) { ] satisfies BrunchUserPart[]), }); - res.json({ ok: true }); + res.json({ ok: true } satisfies SubmitTurnResponseResponse); }); // Get entities for a project app.get('/api/projects/:id/entities', (req: Request, res: Response) => { const id = Number(req.params.id); if (Number.isNaN(id)) { - res.status(400).json({ error: 'Invalid project ID' }); + res.status(400).json({ error: 'Invalid project ID' } satisfies MutationErrorResponse); return; } res.json(getEntitiesForProject(db, id) satisfies EntitiesData); @@ -157,22 +168,22 @@ export function createApp(dbPath?: string) { app.get('/api/projects/:id/export', (req: Request, res: Response) => { const id = Number(req.params.id); if (Number.isNaN(id)) { - res.status(400).json({ error: 'Invalid project ID' }); + res.status(400).json({ error: 'Invalid project ID' } satisfies MutationErrorResponse); return; } const projectState = getProjectState(db, id); if (!projectState) { - res.status(404).json({ error: 'Project not found' }); + res.status(404).json({ error: 'Project not found' } satisfies MutationErrorResponse); return; } const ready = isExportReady(projectState.workflow); if (!ready) { - res.json({ ready: false }); + res.json({ ready: false } satisfies ExportLoaderData); return; } const entities = getEntitiesForProjectOnActivePath(db, id); const markdown = renderExportMarkdown(projectState.project.name, entities, projectState.workflow); - res.json({ ready: true, markdown }); + res.json({ ready: true, markdown } satisfies ExportLoaderData); }); // Conduct turn for a specific project diff --git a/src/shared/api-types.test.ts b/src/shared/api-types.test.ts new file mode 100644 index 00000000..467a92d2 --- /dev/null +++ b/src/shared/api-types.test.ts @@ -0,0 +1,212 @@ +import { describe, expect, it } from 'vitest'; + +import { + entitiesDataSchema, + exportLoaderDataSchema, + mutationErrorResponseSchema, + projectListItemSchema, + projectStateSchema, + submitTurnResponseRequestSchema, + submitTurnResponseResponseSchema, +} from './api-types.js'; + +describe('api transport contracts', () => { + it('validates the current project-list payload shape', () => { + expect( + projectListItemSchema.parse({ + id: 1, + name: 'Project 1', + active_turn_id: 4, + created_at: '2026-04-12 10:00:00', + updated_at: '2026-04-12 10:00:00', + workflowSummary: { + scope: 'closed', + design: 'in_progress', + requirements: 'unstarted', + criteria: 'unstarted', + }, + }), + ).toMatchObject({ + id: 1, + name: 'Project 1', + workflowSummary: { + scope: 'closed', + design: 'in_progress', + }, + }); + }); + + it('validates the current project-state payload shape', () => { + expect( + projectStateSchema.parse({ + project: { + id: 1, + name: 'Project 1', + active_turn_id: 4, + created_at: '2026-04-12 10:00:00', + updated_at: '2026-04-12 10:00:00', + }, + workflow: { + phases: { + scope: { + status: 'closed', + closeability: true, + readiness: 'high', + closureBasis: 'interviewer_recommended', + proposalPending: false, + turnId: 3, + summary: 'Scope is sufficiently captured.', + }, + design: { + status: 'in_progress', + closeability: false, + readiness: 'medium', + closureBasis: null, + proposalPending: false, + turnId: null, + summary: null, + }, + requirements: { + status: 'unstarted', + closeability: false, + readiness: 'low', + closureBasis: null, + proposalPending: false, + turnId: null, + summary: null, + }, + criteria: { + status: 'unstarted', + closeability: false, + readiness: 'low', + closureBasis: null, + proposalPending: false, + turnId: null, + summary: null, + }, + }, + }, + turns: [ + { + id: 4, + project_id: 1, + parent_turn_id: 3, + phase: 'design', + question: 'Which platform should we target?', + why: 'Platform affects the first release shape.', + impact: 'high', + answer: 'Web', + is_resolution: false, + user_parts: '[{"type":"text","text":"Web"}]', + assistant_parts: '[{"type":"text","text":"Which platform should we target?"}]', + created_at: '2026-04-12 10:00:00', + options: [ + { + id: 11, + position: 0, + content: 'Web', + is_recommended: true, + is_selected: false, + }, + ], + }, + ], + }), + ).toMatchObject({ + project: { id: 1, name: 'Project 1' }, + workflow: { + phases: { + scope: { status: 'closed' }, + design: { status: 'in_progress' }, + }, + }, + }); + }); + + it('validates the current entities payload shape', () => { + expect( + entitiesDataSchema.parse({ + goals: [ + { + id: 1, + project_id: 1, + kind: 'goal', + subtype: null, + content: 'Ship a useful first version', + rationale: 'The product needs a crisp first release.', + }, + ], + terms: [], + contexts: [], + constraints: [], + requirements: [ + { + id: 2, + project_id: 1, + kind: 'requirement', + subtype: null, + content: 'Resume interviews after reload', + rationale: 'Users leave mid-flow', + reviewStatus: 'approved', + }, + ], + criteria: [ + { + id: 3, + project_id: 1, + kind: 'criterion', + subtype: 'acceptance', + content: 'Reload restores the active path', + rationale: 'This proves persistence works', + reviewStatus: 'pending', + }, + ], + decisions: [ + { + id: 4, + project_id: 1, + content: 'Use SQLite for local storage', + rationale: 'Zero-config first-run matters', + }, + ], + assumptions: [ + { + id: 5, + project_id: 1, + content: 'Users can work in a browser', + }, + ], + relationships: [ + { + type: 'depends_on', + source: { collection: 'decision', kind: 'decision', id: 4 }, + target: { collection: 'assumption', kind: 'assumption', id: 5 }, + }, + ], + }), + ).toMatchObject({ + requirements: [{ reviewStatus: 'approved' }], + criteria: [{ reviewStatus: 'pending' }], + }); + }); + + it('validates the current export and mutation payload shapes', () => { + expect(exportLoaderDataSchema.parse({ ready: false })).toEqual({ ready: false }); + expect(exportLoaderDataSchema.parse({ ready: true, markdown: '# Reviewed Spec' })).toEqual({ + ready: true, + markdown: '# Reviewed Spec', + }); + expect(mutationErrorResponseSchema.parse({ error: 'Failed to save response' })).toEqual({ + error: 'Failed to save response', + }); + expect(submitTurnResponseResponseSchema.parse({ ok: true })).toEqual({ ok: true }); + }); + + it('models the current turn-response request shape before the explicit-mode refactor', () => { + expect(submitTurnResponseRequestSchema.parse({ positions: [0, 2] })).toEqual({ positions: [0, 2] }); + expect(submitTurnResponseRequestSchema.parse({ freeText: 'None of these fit' })).toEqual({ + freeText: 'None of these fit', + }); + expect(() => submitTurnResponseRequestSchema.parse({})).toThrow(); + }); +}); diff --git a/src/shared/api-types.ts b/src/shared/api-types.ts index 7c45a747..e328d54d 100644 --- a/src/shared/api-types.ts +++ b/src/shared/api-types.ts @@ -1,7 +1,214 @@ -import type { getProjectState, listProjectStates } from '../server/core.js'; -import type { getEntitiesForProject } from '../server/db.js'; +import * as z from 'zod/v4'; -export type ProjectListItem = ReturnType[number]; -export type ProjectState = NonNullable>; -export type ProjectStateTurn = ProjectState['turns'][number]; -export type EntitiesData = ReturnType; +import { phaseClosureBasisSchema, workflowPhaseSchema } from './phase-close.js'; + +export const workflowPhaseStatusSchema = z.enum(['unstarted', 'in_progress', 'closed']); +export const readinessBandSchema = z.enum(['low', 'medium', 'high']); + +export const projectSchema = z.object({ + id: z.number().int().positive(), + name: z.string(), + active_turn_id: z.number().int().positive().nullable(), + created_at: z.string(), + updated_at: z.string(), +}); + +export const workflowSummarySchema = z.object({ + scope: workflowPhaseStatusSchema, + design: workflowPhaseStatusSchema, + requirements: workflowPhaseStatusSchema, + criteria: workflowPhaseStatusSchema, +}); + +export const workflowPhaseStateSchema = z.object({ + status: workflowPhaseStatusSchema, + closeability: z.boolean(), + readiness: readinessBandSchema, + closureBasis: phaseClosureBasisSchema.nullable(), + proposalPending: z.boolean(), + turnId: z.number().int().positive().nullable(), + summary: z.string().nullable(), +}); + +export const workflowStateSchema = z.object({ + phases: z.object({ + scope: workflowPhaseStateSchema, + design: workflowPhaseStateSchema, + requirements: workflowPhaseStateSchema, + criteria: workflowPhaseStateSchema, + }), +}); + +export const turnOptionSchema = z.object({ + id: z.number().int().positive(), + position: z.number().int().min(0), + content: z.string(), + is_recommended: z.boolean(), + is_selected: z.boolean(), +}); + +export const projectStateTurnSchema = z.object({ + id: z.number().int().positive(), + project_id: z.number().int().positive(), + parent_turn_id: z.number().int().positive().nullable(), + phase: workflowPhaseSchema, + question: z.string(), + why: z.string().nullable(), + impact: z.enum(['high', 'medium', 'low']).nullable(), + answer: z.string().nullable(), + is_resolution: z.boolean(), + user_parts: z.string().nullable(), + assistant_parts: z.string().nullable(), + created_at: z.string(), + options: z.array(turnOptionSchema).optional(), +}); + +export const projectListItemSchema = projectSchema.extend({ + workflowSummary: workflowSummarySchema, +}); + +export const projectListItemsSchema = z.array(projectListItemSchema); + +export const projectStateSchema = z.object({ + project: projectSchema, + workflow: workflowStateSchema, + turns: z.array(projectStateTurnSchema), +}); + +const knowledgeItemKindSchema = z.enum([ + 'goal', + 'term', + 'context', + 'constraint', + 'requirement', + 'criterion', + 'decision', + 'assumption', +]); +const reviewStatusSchema = z.enum(['approved', 'rejected', 'pending']); + +export const knowledgeItemSchema = z.object({ + id: z.number().int().positive(), + project_id: z.number().int().positive(), + kind: knowledgeItemKindSchema, + subtype: z.string().nullable(), + content: z.string(), + rationale: z.string().nullable(), +}); + +export const requirementEntitySchema = z.object({ + id: z.number().int().positive(), + project_id: z.number().int().positive(), + kind: knowledgeItemKindSchema, + subtype: z.string().nullable(), + content: z.string(), + rationale: z.string().nullable(), + reviewStatus: reviewStatusSchema.optional(), +}); + +export const criterionEntitySchema = z.object({ + id: z.number().int().positive(), + project_id: z.number().int().positive(), + kind: knowledgeItemKindSchema, + subtype: z.string().nullable(), + content: z.string(), + rationale: z.string().nullable(), + reviewStatus: reviewStatusSchema.optional(), +}); + +export const decisionEntitySchema = z.object({ + id: z.number().int().positive(), + project_id: z.number().int().positive(), + content: z.string(), + rationale: z.string().nullable(), +}); + +export const assumptionEntitySchema = z.object({ + id: z.number().int().positive(), + project_id: z.number().int().positive(), + content: z.string(), +}); + +export const entityReferenceSchema = z.object({ + collection: z.enum(['knowledge_item', 'decision', 'assumption']), + kind: z.enum([ + 'goal', + 'term', + 'context', + 'constraint', + 'requirement', + 'criterion', + 'decision', + 'assumption', + ]), + id: z.number().int().positive(), +}); + +export const entityRelationshipSchema = z.object({ + type: z.literal('depends_on'), + source: entityReferenceSchema, + target: entityReferenceSchema, +}); + +export const entitiesDataSchema = z.object({ + goals: z.array(knowledgeItemSchema), + terms: z.array(knowledgeItemSchema), + contexts: z.array(knowledgeItemSchema), + constraints: z.array(knowledgeItemSchema), + requirements: z.array(requirementEntitySchema), + criteria: z.array(criterionEntitySchema), + decisions: z.array(decisionEntitySchema), + assumptions: z.array(assumptionEntitySchema), + relationships: z.array(entityRelationshipSchema), +}); + +export const exportLoaderDataSchema = z.object({ + ready: z.boolean(), + markdown: z.string().optional(), +}); + +export const mutationErrorResponseSchema = z.object({ + error: z.string().optional(), +}); + +export const submitTurnResponseRequestSchema = z + .object({ + positions: z.array(z.number().int().min(0)).optional(), + freeText: z.string().trim().min(1).optional(), + }) + .superRefine((value, ctx) => { + if ((value.positions?.length ?? 0) === 0 && !value.freeText) { + ctx.addIssue({ + code: 'custom', + message: 'positions are required unless freeText is provided', + path: ['positions'], + }); + } + }); + +export const submitTurnResponseResponseSchema = z.object({ + ok: z.literal(true), +}); + +export type WorkflowPhaseStatus = z.infer; +export type ReadinessBand = z.infer; +export type Project = z.infer; +export type WorkflowSummary = z.infer; +export type WorkflowPhaseState = z.infer; +export type WorkflowState = z.infer; +export type TurnOption = z.infer; +export type ProjectStateTurn = z.infer; +export type ProjectListItem = z.infer; +export type ProjectState = z.infer; +export type KnowledgeItem = z.infer; +export type RequirementEntity = z.infer; +export type CriterionEntity = z.infer; +export type DecisionEntity = z.infer; +export type AssumptionEntity = z.infer; +export type EntityReference = z.infer; +export type EntityRelationship = z.infer; +export type EntitiesData = z.infer; +export type ExportLoaderData = z.infer; +export type MutationErrorResponse = z.infer; +export type SubmitTurnResponseRequest = z.infer; +export type SubmitTurnResponseResponse = z.infer; From 1d79d9076b61e23f3bd0d49bab81c070672c8915 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:12:58 +0200 Subject: [PATCH 07/14] refactor: parse workspace transport payloads through shared contracts --- src/client/mutations/client-mutation.test.ts | 63 ++++++++++++++++--- src/client/mutations/client-mutation.ts | 10 +-- src/client/mutations/project-mutations.ts | 10 ++- src/client/mutations/workspace-mutations.ts | 10 +-- src/client/routes/export-loader.test.ts | 12 ++++ src/client/routes/export-loader.ts | 4 +- src/client/workspace/workspace-loader.test.ts | 36 +++++++++++ src/client/workspace/workspace-loader.ts | 24 +++++-- src/shared/api-types.ts | 8 +++ 9 files changed, 152 insertions(+), 25 deletions(-) diff --git a/src/client/mutations/client-mutation.test.ts b/src/client/mutations/client-mutation.test.ts index a6c5f019..7004fbb9 100644 --- a/src/client/mutations/client-mutation.test.ts +++ b/src/client/mutations/client-mutation.test.ts @@ -1,8 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as z from 'zod/v4'; import { ClientMutationError, postJsonMutation } from './client-mutation.js'; const fetchMock = vi.fn(); +const createProjectResponseSchema = z.object({ id: z.number().int().positive() }); describe('client mutation', () => { beforeEach(() => { @@ -16,22 +18,32 @@ describe('client mutation', () => { it('returns parsed json on success', async () => { fetchMock.mockResolvedValueOnce( - new Response(JSON.stringify({ ok: true, id: 7 }), { + new Response(JSON.stringify({ id: 7 }), { status: 200, headers: { 'Content-Type': 'application/json' }, }), ); await expect( - postJsonMutation('/api/projects', { name: 'New project' }, 'Failed to create project'), - ).resolves.toEqual({ ok: true, id: 7 }); + postJsonMutation( + '/api/projects', + { name: 'New project' }, + createProjectResponseSchema, + 'Failed to create project', + ), + ).resolves.toEqual({ id: 7 }); }); it('surfaces network failures with the caller fallback message', async () => { fetchMock.mockRejectedValueOnce(new TypeError('network down')); await expect( - postJsonMutation('/api/projects', { name: 'New project' }, 'Failed to create project'), + postJsonMutation( + '/api/projects', + { name: 'New project' }, + createProjectResponseSchema, + 'Failed to create project', + ), ).rejects.toEqual(new ClientMutationError('Failed to create project')); }); @@ -44,7 +56,12 @@ describe('client mutation', () => { ); await expect( - postJsonMutation('/api/projects', { name: 'New project' }, 'Failed to create project'), + postJsonMutation( + '/api/projects', + { name: 'New project' }, + createProjectResponseSchema, + 'Failed to create project', + ), ).rejects.toMatchObject({ name: 'ClientMutationError', message: 'Project name already exists', @@ -61,7 +78,12 @@ describe('client mutation', () => { ); await expect( - postJsonMutation('/api/projects', { name: 'New project' }, 'Failed to create project'), + postJsonMutation( + '/api/projects', + { name: 'New project' }, + createProjectResponseSchema, + 'Failed to create project', + ), ).rejects.toMatchObject({ name: 'ClientMutationError', message: 'Failed to create project', @@ -78,7 +100,34 @@ describe('client mutation', () => { ); await expect( - postJsonMutation('/api/projects', { name: 'New project' }, 'Failed to create project'), + postJsonMutation( + '/api/projects', + { name: 'New project' }, + createProjectResponseSchema, + 'Failed to create project', + ), + ).rejects.toMatchObject({ + name: 'ClientMutationError', + message: 'Failed to create project', + status: 200, + }); + }); + + it('surfaces parseable but structurally invalid success payloads as mutation errors', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ id: '7' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect( + postJsonMutation( + '/api/projects', + { name: 'New project' }, + createProjectResponseSchema, + 'Failed to create project', + ), ).rejects.toMatchObject({ name: 'ClientMutationError', message: 'Failed to create project', diff --git a/src/client/mutations/client-mutation.ts b/src/client/mutations/client-mutation.ts index 8245c124..316d32d2 100644 --- a/src/client/mutations/client-mutation.ts +++ b/src/client/mutations/client-mutation.ts @@ -1,6 +1,7 @@ import { useMutation } from '@tanstack/react-query'; +import type { ZodType } from 'zod/v4'; -import type { MutationErrorResponse } from '../../shared/api-types.js'; +import { mutationErrorResponseSchema, type MutationErrorResponse } from '../../shared/api-types.js'; export class ClientMutationError extends Error { readonly status: number | undefined; @@ -14,12 +15,12 @@ export class ClientMutationError extends Error { async function readMutationErrorMessage(response: Response, fallbackMessage: string): Promise { try { - const payload = (await response.json()) as MutationErrorResponse; + const payload = mutationErrorResponseSchema.parse((await response.json()) as MutationErrorResponse); if (typeof payload.error === 'string' && payload.error.trim().length > 0) { return payload.error; } } catch { - // Fall back to the caller-provided message when the response is not JSON. + // Fall back to the caller-provided message when the response is not JSON or does not match the contract. } return fallbackMessage; @@ -28,6 +29,7 @@ async function readMutationErrorMessage(response: Response, fallbackMessage: str export async function postJsonMutation( url: string, body: TRequest, + responseSchema: ZodType, fallbackMessage: string, ): Promise { let response: Response; @@ -47,7 +49,7 @@ export async function postJsonMutation( } try { - return (await response.json()) as TResponse; + return responseSchema.parse(await response.json()); } catch { throw new ClientMutationError(fallbackMessage, response.status); } diff --git a/src/client/mutations/project-mutations.ts b/src/client/mutations/project-mutations.ts index d2926df4..384671a4 100644 --- a/src/client/mutations/project-mutations.ts +++ b/src/client/mutations/project-mutations.ts @@ -1,13 +1,19 @@ import { useNavigate } from '@tanstack/react-router'; +import { + createProjectResponseSchema, + type CreateProjectRequest, + type CreateProjectResponse, +} from '../../shared/api-types.js'; import { postJsonMutation, useClientMutation } from './client-mutation.js'; export function useCreateProjectMutation() { const navigate = useNavigate(); - const mutation = useClientMutation((variables: { name: string }) => - postJsonMutation<{ id: number }, { name: string }>( + const mutation = useClientMutation((variables: CreateProjectRequest) => + postJsonMutation( '/api/projects', variables, + createProjectResponseSchema, 'Failed to create project', ), ); diff --git a/src/client/mutations/workspace-mutations.ts b/src/client/mutations/workspace-mutations.ts index 9f82ef7c..ea257ea8 100644 --- a/src/client/mutations/workspace-mutations.ts +++ b/src/client/mutations/workspace-mutations.ts @@ -1,9 +1,10 @@ import { useRouter } from '@tanstack/react-router'; -import type { - ProjectStateTurn, - SubmitTurnResponseRequest, - SubmitTurnResponseResponse, +import { + submitTurnResponseResponseSchema, + type ProjectStateTurn, + type SubmitTurnResponseRequest, + type SubmitTurnResponseResponse, } from '../../shared/api-types.js'; import { formatTurnResponseText } from '../../shared/chat.js'; import { findTurnOptionsByPositions } from '../workspace/workspace-controller-core.js'; @@ -27,6 +28,7 @@ export function useSubmitTurnResponseMutation({ ...(variables.positions?.length ? { positions: variables.positions } : {}), ...(variables.freeText ? { freeText: variables.freeText } : {}), }, + submitTurnResponseResponseSchema, 'Failed to save response', ), ); diff --git a/src/client/routes/export-loader.test.ts b/src/client/routes/export-loader.test.ts index 71bf7d04..1f38405b 100644 --- a/src/client/routes/export-loader.test.ts +++ b/src/client/routes/export-loader.test.ts @@ -42,4 +42,16 @@ describe('export route loader', () => { await expect(fetchExportPreviewLoaderData(7)).rejects.toThrow(); expect(fetchMock).toHaveBeenCalledWith('/api/projects/7/export'); }); + + it('rejects when the export payload is parseable json with the wrong shape', async () => { + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ ready: 'yes' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect(fetchExportPreviewLoaderData(7)).rejects.toThrow(); + expect(fetchMock).toHaveBeenCalledWith('/api/projects/7/export'); + }); }); diff --git a/src/client/routes/export-loader.ts b/src/client/routes/export-loader.ts index 9595a4cc..1762c89a 100644 --- a/src/client/routes/export-loader.ts +++ b/src/client/routes/export-loader.ts @@ -1,4 +1,4 @@ -import type { ExportLoaderData } from '../../shared/api-types.js'; +import { exportLoaderDataSchema, type ExportLoaderData } from '../../shared/api-types.js'; export async function fetchExportPreviewLoaderData(projectId: number | string): Promise { const id = String(projectId); @@ -7,5 +7,5 @@ export async function fetchExportPreviewLoaderData(projectId: number | string): throw new Error('Failed to load export'); } - return response.json() as Promise; + return exportLoaderDataSchema.parse(await response.json()); } diff --git a/src/client/workspace/workspace-loader.test.ts b/src/client/workspace/workspace-loader.test.ts index 5c3a58b4..a363876e 100644 --- a/src/client/workspace/workspace-loader.test.ts +++ b/src/client/workspace/workspace-loader.test.ts @@ -160,6 +160,42 @@ describe('workspace route loaders', () => { expect(fetchMock).toHaveBeenNthCalledWith(2, '/api/projects/7/entities'); }); + it('rejects when the project payload is parseable json with the wrong shape', async () => { + fetchMock + .mockResolvedValueOnce( + new Response(JSON.stringify({ ...projectState, project: { ...projectState.project, id: '7' } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify(entitySnapshot), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect(fetchInterviewWorkspaceLoaderData(7)).rejects.toThrow(); + }); + + it('rejects when the entity payload is parseable json with the wrong shape', async () => { + fetchMock + .mockResolvedValueOnce( + new Response(JSON.stringify(projectState), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ ...entitySnapshot, decisions: { id: 1 } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + + await expect(fetchInterviewWorkspaceLoaderData(7)).rejects.toThrow(); + }); + it('fails knowledge workspace loading when the project does not exist', async () => { fetchMock.mockResolvedValueOnce(new Response('Not found', { status: 404 })); diff --git a/src/client/workspace/workspace-loader.ts b/src/client/workspace/workspace-loader.ts index aa3318d2..b77bc156 100644 --- a/src/client/workspace/workspace-loader.ts +++ b/src/client/workspace/workspace-loader.ts @@ -1,4 +1,11 @@ -import type { EntitiesData, ProjectState } from '../../shared/api-types.js'; +import type { ZodType } from 'zod/v4'; + +import { + entitiesDataSchema, + projectStateSchema, + type EntitiesData, + type ProjectState, +} from '../../shared/api-types.js'; export interface WorkspaceLoaderData { projectState: ProjectState; @@ -9,20 +16,24 @@ export interface KnowledgeWorkspaceLoaderData { entitySnapshot: EntitiesData; } -async function fetchJson(url: string, errorMessage: string): Promise { +async function fetchJson(url: string, schema: ZodType, errorMessage: string): Promise { const response = await fetch(url); if (!response.ok) { throw new Error(errorMessage); } - return response.json() as Promise; + return schema.parse(await response.json()); } async function fetchWorkflowDetailLoaderData(projectId: number | string): Promise { const id = String(projectId); const [projectState, entitySnapshot] = await Promise.all([ - fetchJson(`/api/projects/${id}`, 'Failed to load project'), - fetchJson(`/api/projects/${id}/entities`, 'Failed to load project entities'), + fetchJson(`/api/projects/${id}`, projectStateSchema, 'Failed to load project'), + fetchJson( + `/api/projects/${id}/entities`, + entitiesDataSchema, + 'Failed to load project entities', + ), ]); return { projectState, entitySnapshot }; @@ -38,9 +49,10 @@ export async function fetchKnowledgeWorkspaceLoaderData( projectId: number | string, ): Promise { const id = String(projectId); - await fetchJson(`/api/projects/${id}`, 'Failed to load project'); + await fetchJson(`/api/projects/${id}`, projectStateSchema, 'Failed to load project'); const entitySnapshot = await fetchJson( `/api/projects/${id}/entities`, + entitiesDataSchema, 'Failed to load project entities', ); diff --git a/src/shared/api-types.ts b/src/shared/api-types.ts index e328d54d..27ecfaad 100644 --- a/src/shared/api-types.ts +++ b/src/shared/api-types.ts @@ -63,6 +63,12 @@ export const projectStateTurnSchema = z.object({ options: z.array(turnOptionSchema).optional(), }); +export const createProjectRequestSchema = z.object({ + name: z.string().trim().min(1), +}); + +export const createProjectResponseSchema = projectSchema; + export const projectListItemSchema = projectSchema.extend({ workflowSummary: workflowSummarySchema, }); @@ -193,6 +199,8 @@ export const submitTurnResponseResponseSchema = z.object({ export type WorkflowPhaseStatus = z.infer; export type ReadinessBand = z.infer; export type Project = z.infer; +export type CreateProjectRequest = z.infer; +export type CreateProjectResponse = z.infer; export type WorkflowSummary = z.infer; export type WorkflowPhaseState = z.infer; export type WorkflowState = z.infer; From a9e8424513303c5870d090a44cebb89dba820d52 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:20:51 +0200 Subject: [PATCH 08/14] refactor: make turn-response and review payloads explicit --- src/client/mutations/workspace-mutations.ts | 33 ++++++++++------- src/client/routes/InterviewWorkspace.test.tsx | 17 +++++++-- src/server/app.test.ts | 36 +++++++++++++------ src/server/app.ts | 36 +++++++++---------- src/server/interview.test.ts | 21 +++-------- src/shared/api-types.test.ts | 25 ++++++++++--- src/shared/api-types.ts | 31 ++++++++-------- src/shared/chat.ts | 30 ++++------------ 8 files changed, 124 insertions(+), 105 deletions(-) diff --git a/src/client/mutations/workspace-mutations.ts b/src/client/mutations/workspace-mutations.ts index ea257ea8..31ca9924 100644 --- a/src/client/mutations/workspace-mutations.ts +++ b/src/client/mutations/workspace-mutations.ts @@ -20,17 +20,13 @@ export function useSubmitTurnResponseMutation({ sendMessage: (message: { text: string }) => Promise | void; }) { const router = useRouter(); - const mutation = useClientMutation( - (variables: { turnId: number; positions?: number[]; freeText?: string }) => - postJsonMutation( - `/api/projects/${projectId}/turns/${variables.turnId}/response`, - { - ...(variables.positions?.length ? { positions: variables.positions } : {}), - ...(variables.freeText ? { freeText: variables.freeText } : {}), - }, - submitTurnResponseResponseSchema, - 'Failed to save response', - ), + const mutation = useClientMutation((variables: { turnId: number; response: SubmitTurnResponseRequest }) => + postJsonMutation( + `/api/projects/${projectId}/turns/${variables.turnId}/response`, + variables.response, + submitTurnResponseResponseSchema, + 'Failed to save response', + ), ); return { @@ -52,11 +48,22 @@ export function useSubmitTurnResponseMutation({ return; } + const response: SubmitTurnResponseRequest = + uniquePositions.length > 0 + ? { + kind: 'select-options', + positions: uniquePositions, + ...(trimmedFreeText ? { freeText: trimmedFreeText } : {}), + } + : { + kind: 'free-text', + freeText: trimmedFreeText!, + }; + try { await mutation.run({ turnId: turn.id, - positions: uniquePositions.length > 0 ? uniquePositions : undefined, - freeText: trimmedFreeText || undefined, + response, }); await router.invalidate(); await sendMessage({ text: responseText }); diff --git a/src/client/routes/InterviewWorkspace.test.tsx b/src/client/routes/InterviewWorkspace.test.tsx index b4c8cf4c..f6a1e015 100644 --- a/src/client/routes/InterviewWorkspace.test.tsx +++ b/src/client/routes/InterviewWorkspace.test.tsx @@ -966,7 +966,11 @@ describe('InterviewWorkspace', () => { expect.objectContaining({ method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ positions: [1], freeText: 'Best fit for our launch' }), + body: JSON.stringify({ + kind: 'select-options', + positions: [1], + freeText: 'Best fit for our launch', + }), }), ); }); @@ -1008,7 +1012,11 @@ describe('InterviewWorkspace', () => { expect.objectContaining({ method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ positions: [0, 1], freeText: 'Covers both launch paths' }), + body: JSON.stringify({ + kind: 'select-options', + positions: [0, 1], + freeText: 'Covers both launch paths', + }), }), ); }); @@ -1390,7 +1398,10 @@ describe('InterviewWorkspace', () => { expect.objectContaining({ method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ freeText: 'None of these fit our use case' }), + body: JSON.stringify({ + kind: 'free-text', + freeText: 'None of these fit our use case', + }), }), ); }); diff --git a/src/server/app.test.ts b/src/server/app.test.ts index 76f5c693..abaf34a0 100644 --- a/src/server/app.test.ts +++ b/src/server/app.test.ts @@ -1249,7 +1249,11 @@ describe('phase outcomes + scope closure', () => { await request(app) .post(`/api/projects/${projectId}/turns/${reviewTurn.id}/response`) - .send({ positions: [2], freeText: 'Export the reviewed spec as markdown' }) + .send({ + kind: 'select-options', + positions: [2], + freeText: 'Export the reviewed spec as markdown', + }) .expect(200); mockStreamInterviewer.mockImplementation(async () => @@ -2136,7 +2140,11 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${turn.id}/response`) - .send({ positions: [1], freeText: 'Best fit for our launch' }) + .send({ + kind: 'select-options', + positions: [1], + freeText: 'Best fit for our launch', + }) .expect(200); expect(getOptionsForTurn(db, turn.id)[1].is_selected).toBe(true); @@ -2174,7 +2182,11 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${turn.id}/response`) - .send({ positions: [0, 1], freeText: 'Covers both launch paths' }) + .send({ + kind: 'select-options', + positions: [0, 1], + freeText: 'Covers both launch paths', + }) .expect(200); const selectedOptions = getOptionsForTurn(db, turn.id).filter((option) => option.is_selected); @@ -2256,7 +2268,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${reviewTurn.id}/response`) - .send({ positions: [0] }) + .send({ kind: 'select-options', positions: [0] }) .expect(200); const reviewedRows = db.$client @@ -2356,7 +2368,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${reviewTurn.id}/response`) - .send({ positions: [0] }) + .send({ kind: 'select-options', positions: [0] }) .expect(200); const rejectedRows = db.$client @@ -2456,7 +2468,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${reviewTurn.id}/response`) - .send({ positions: [0] }) + .send({ kind: 'select-options', positions: [0] }) .expect(200); const reviewedRows = db.$client @@ -2556,7 +2568,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${reviewTurn.id}/response`) - .send({ positions: [0] }) + .send({ kind: 'select-options', positions: [0] }) .expect(200); const rejectedRows = db.$client @@ -2601,7 +2613,11 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${turn.id}/response`) - .send({ positions: [0, 1], freeText: 'Covers both launch paths' }) + .send({ + kind: 'select-options', + positions: [0, 1], + freeText: 'Covers both launch paths', + }) .expect(200); const projectStateRes = await request(app).get(`/api/projects/${projectId}`).expect(200); @@ -2697,7 +2713,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${turn.id}/response`) - .send({ freeText: 'None of these fit our use case' }) + .send({ kind: 'free-text', freeText: 'None of these fit our use case' }) .expect(200); expect(getOptionsForTurn(db, turn.id).every((option) => !option.is_selected)).toBe(true); @@ -2731,7 +2747,7 @@ describe('POST /api/projects/:id/turns/:turnId/response', () => { await request(app) .post(`/api/projects/${projectId}/turns/${turn.id}/response`) - .send({ freeText: ' ' }) + .send({ kind: 'free-text', freeText: ' ' }) .expect(400); }); }); diff --git a/src/server/app.ts b/src/server/app.ts index 831ac7b5..5d661185 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -2,13 +2,14 @@ import { createUIMessageStream, pipeUIMessageStreamToResponse, validateUIMessage import express from 'express'; import type { Request, Response } from 'express'; -import type { - EntitiesData, - ExportLoaderData, - MutationErrorResponse, - ProjectListItem, - ProjectState, - SubmitTurnResponseResponse, +import { + submitTurnResponseRequestSchema, + type EntitiesData, + type ExportLoaderData, + type MutationErrorResponse, + type ProjectListItem, + type ProjectState, + type SubmitTurnResponseResponse, } from '../shared/api-types.js'; import { assistantPartsSchema, @@ -94,27 +95,22 @@ export function createApp(dbPath?: string) { app.post('/api/projects/:id/turns/:turnId/response', (req: Request, res: Response) => { const projectId = Number(req.params.id); const turnId = Number(req.params.turnId); - const selectedPositions: number[] = Array.isArray(req.body?.positions) - ? req.body.positions.filter((value: unknown): value is number => typeof value === 'number') - : typeof req.body?.position === 'number' - ? [req.body.position] - : []; - const uniquePositions = [...new Set(selectedPositions)]; - const freeText = typeof req.body.freeText === 'string' ? req.body.freeText.trim() : undefined; if (Number.isNaN(projectId) || Number.isNaN(turnId)) { res.status(400).json({ error: 'Invalid IDs' } satisfies MutationErrorResponse); return; } - if (uniquePositions.length === 0 && !freeText) { - res - .status(400) - .json({ - error: 'positions are required unless freeText is provided', - } satisfies MutationErrorResponse); + + const parsedRequest = submitTurnResponseRequestSchema.safeParse(req.body); + if (!parsedRequest.success) { + res.status(400).json({ error: 'Invalid turn response payload' } satisfies MutationErrorResponse); return; } + const uniquePositions = + parsedRequest.data.kind === 'select-options' ? [...new Set(parsedRequest.data.positions)] : []; + const freeText = parsedRequest.data.freeText; + const turn = getTurn(db, turnId); if (!turn || turn.project_id !== projectId) { res.status(404).json({ error: 'Turn not found' } satisfies MutationErrorResponse); diff --git a/src/server/interview.test.ts b/src/server/interview.test.ts index 8b27688a..ffae6710 100644 --- a/src/server/interview.test.ts +++ b/src/server/interview.test.ts @@ -45,11 +45,11 @@ describe('structuredQuestionSchema', () => { ).toThrow(); }); - it('accepts the legacy review field and normalizes it to requirementReview', () => { - expect( + it('rejects the legacy review field now that requirement review is explicit', () => { + expect(() => structuredQuestionSchema.parse({ question: 'Should we approve this requirement?', - why: 'Requirement review coverage should continue to work during the prompt transition.', + why: 'Requirement review should use the explicit requirementReview payload.', impact: 'high', options: [ { content: 'Approve', is_recommended: true }, @@ -61,20 +61,7 @@ describe('structuredQuestionSchema', () => { approveOptionPosition: 0, }, }), - ).toEqual({ - question: 'Should we approve this requirement?', - why: 'Requirement review coverage should continue to work during the prompt transition.', - impact: 'high', - options: [ - { content: 'Approve', is_recommended: true }, - { content: 'Reject', is_recommended: false }, - ], - requirementReview: { - kind: 'requirement-approval', - requirementId: 42, - approveOptionPosition: 0, - }, - }); + ).toThrow(); }); }); diff --git a/src/shared/api-types.test.ts b/src/shared/api-types.test.ts index 467a92d2..fc999433 100644 --- a/src/shared/api-types.test.ts +++ b/src/shared/api-types.test.ts @@ -202,11 +202,28 @@ describe('api transport contracts', () => { expect(submitTurnResponseResponseSchema.parse({ ok: true })).toEqual({ ok: true }); }); - it('models the current turn-response request shape before the explicit-mode refactor', () => { - expect(submitTurnResponseRequestSchema.parse({ positions: [0, 2] })).toEqual({ positions: [0, 2] }); - expect(submitTurnResponseRequestSchema.parse({ freeText: 'None of these fit' })).toEqual({ + it('models turn responses through explicit request modes', () => { + expect( + submitTurnResponseRequestSchema.parse({ + kind: 'select-options', + positions: [0, 2], + freeText: 'Covers both launch paths', + }), + ).toEqual({ + kind: 'select-options', + positions: [0, 2], + freeText: 'Covers both launch paths', + }); + expect( + submitTurnResponseRequestSchema.parse({ + kind: 'free-text', + freeText: 'None of these fit', + }), + ).toEqual({ + kind: 'free-text', freeText: 'None of these fit', }); - expect(() => submitTurnResponseRequestSchema.parse({})).toThrow(); + expect(() => submitTurnResponseRequestSchema.parse({ positions: [0, 2] })).toThrow(); + expect(() => submitTurnResponseRequestSchema.parse({ kind: 'free-text' })).toThrow(); }); }); diff --git a/src/shared/api-types.ts b/src/shared/api-types.ts index 27ecfaad..3b6cd795 100644 --- a/src/shared/api-types.ts +++ b/src/shared/api-types.ts @@ -177,20 +177,21 @@ export const mutationErrorResponseSchema = z.object({ error: z.string().optional(), }); -export const submitTurnResponseRequestSchema = z - .object({ - positions: z.array(z.number().int().min(0)).optional(), - freeText: z.string().trim().min(1).optional(), - }) - .superRefine((value, ctx) => { - if ((value.positions?.length ?? 0) === 0 && !value.freeText) { - ctx.addIssue({ - code: 'custom', - message: 'positions are required unless freeText is provided', - path: ['positions'], - }); - } - }); +export const submitTurnResponseSelectionRequestSchema = z.object({ + kind: z.literal('select-options'), + positions: z.array(z.number().int().min(0)).min(1), + freeText: z.string().trim().min(1).optional(), +}); + +export const submitTurnResponseFreeTextRequestSchema = z.object({ + kind: z.literal('free-text'), + freeText: z.string().trim().min(1), +}); + +export const submitTurnResponseRequestSchema = z.discriminatedUnion('kind', [ + submitTurnResponseSelectionRequestSchema, + submitTurnResponseFreeTextRequestSchema, +]); export const submitTurnResponseResponseSchema = z.object({ ok: z.literal(true), @@ -218,5 +219,7 @@ export type EntityRelationship = z.infer; export type EntitiesData = z.infer; export type ExportLoaderData = z.infer; export type MutationErrorResponse = z.infer; +export type SubmitTurnResponseSelectionRequest = z.infer; +export type SubmitTurnResponseFreeTextRequest = z.infer; export type SubmitTurnResponseRequest = z.infer; export type SubmitTurnResponseResponse = z.infer; diff --git a/src/shared/chat.ts b/src/shared/chat.ts index cc6609eb..12a3e3a6 100644 --- a/src/shared/chat.ts +++ b/src/shared/chat.ts @@ -36,13 +36,13 @@ export const criterionRejectionReviewSchema = z.object({ export const criterionReviewSchema = z.union([criterionApprovalReviewSchema, criterionRejectionReviewSchema]); function validateReviewOptionPosition( - review: { approveOptionPosition?: number; rejectOptionPosition?: number }, + 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 position = isApproval ? review.approveOptionPosition : review.rejectOptionPosition; const positionField = isApproval ? 'approveOptionPosition' : 'rejectOptionPosition'; if (position >= optionCount) { @@ -68,35 +68,17 @@ export const structuredQuestionSchema = z ) .min(2), requirementReview: requirementReviewSchema.optional(), - review: requirementReviewSchema.optional(), criterionReview: criterionReviewSchema.optional(), }) + .strict() .superRefine((value, ctx) => { - if (value.requirementReview && value.review) { - ctx.addIssue({ - code: 'custom', - message: 'Use requirementReview instead of review when both are present', - path: ['review'], - }); - } - - const requirementReview = value.requirementReview ?? value.review; - if (requirementReview) { - validateReviewOptionPosition( - requirementReview, - value.requirementReview ? 'requirementReview' : 'review', - value.options.length, - ctx, - ); + if (value.requirementReview) { + validateReviewOptionPosition(value.requirementReview, 'requirementReview', value.options.length, ctx); } if (value.criterionReview) { validateReviewOptionPosition(value.criterionReview, 'criterionReview', value.options.length, ctx); } - }) - .transform(({ review, requirementReview, ...value }) => ({ - ...value, - ...((requirementReview ?? review) ? { requirementReview: requirementReview ?? review } : {}), - })); + }); export const askQuestionToolOutputSchema = z.object({ ok: z.literal(true), From 8f31eec9f79115d74185e51fa4d900444d2c4b72 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:37:19 +0200 Subject: [PATCH 09/14] refactor: add explicit boundary return types --- memory/REFACTOR.md | 17 +++++++---- src/client/mutations/client-mutation.ts | 9 +++++- src/client/mutations/project-mutations.ts | 9 +++++- src/client/mutations/workspace-mutations.ts | 9 +++++- src/server/app.ts | 10 +++++-- src/server/interview.ts | 32 +++++++++++++++++---- 6 files changed, 71 insertions(+), 15 deletions(-) diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md index 20c70dd5..1166ed52 100644 --- a/memory/REFACTOR.md +++ b/memory/REFACTOR.md @@ -32,15 +32,22 @@ This keeps the refactor focused on the highest-risk overlap between the React re ## Commits -1. Add characterization coverage for the refactor seam: project navigation, same-project refresh, entity refresh after observer invalidation, and JSON-boundary success/failure cases. -2. Introduce explicit shared transport contracts for project state, entity payloads, export payloads, mutation errors, and turn-response request/response payloads without changing runtime behavior yet. -3. Move client loader and mutation helpers to parse transport payloads through those shared contracts and remove unchecked JSON casts from the workspace-facing routes and mutations. -4. Narrow the turn-response and review payload shapes so meaningful request modes are represented explicitly instead of through bags of optional fields. -5. Add explicit return types to the exported boundary functions touched by the transport pass so the module surfaces stay legible during later commits. +1. [done] Add characterization coverage for the refactor seam: project navigation, same-project refresh, entity refresh after observer invalidation, and JSON-boundary success/failure cases. +2. [done] Introduce explicit shared transport contracts for project state, entity payloads, export payloads, mutation errors, and turn-response request/response payloads without changing runtime behavior yet. +3. [done] Move client loader and mutation helpers to parse transport payloads through those shared contracts and remove unchecked JSON casts from the workspace-facing routes and mutations. +4. [done] Narrow the turn-response and review payload shapes so meaningful request modes are represented explicitly instead of through bags of optional fields. +5. [done] Add explicit return types to the exported boundary functions touched by the transport pass so the module surfaces stay legible during later commits. 6. Put the live chat owner behind a project-identity reset boundary and delete the post-render chat hydration overwrite. 7. Collapse workspace entity hydration to one authority per route transition and remove the effect-based loader-to-cache mirroring step. 8. Mark touched snapshot and view-model contracts as readonly and normalize type-only imports in the refactored boundary modules. +## Progress + +First half complete: commits 1-4 are landed and the full test suite passes. + +Temporary deferral: +- the client build-boundary test keeps its chunk-splitting assertions, but the minified entry-size ceiling is deferred until after the remaining workspace-state refactor work. Bundle-size tightening is not the current priority for this refactor pass. + ## Decisions - Recommended scope: one boundary-first refactor centered on workspace transport and state ownership, not a repo-wide hygiene sweep. diff --git a/src/client/mutations/client-mutation.ts b/src/client/mutations/client-mutation.ts index 316d32d2..5cbd66a3 100644 --- a/src/client/mutations/client-mutation.ts +++ b/src/client/mutations/client-mutation.ts @@ -3,6 +3,13 @@ import type { ZodType } from 'zod/v4'; import { mutationErrorResponseSchema, type MutationErrorResponse } from '../../shared/api-types.js'; +export interface ClientMutationState { + run: (variables: TVariables) => Promise; + isPending: boolean; + errorMessage: string | null; + clearError: () => void; +} + export class ClientMutationError extends Error { readonly status: number | undefined; @@ -57,7 +64,7 @@ export async function postJsonMutation( export function useClientMutation( mutationFn: (variables: TVariables) => Promise, -) { +): ClientMutationState { const mutation = useMutation({ mutationFn }); return { diff --git a/src/client/mutations/project-mutations.ts b/src/client/mutations/project-mutations.ts index 384671a4..ebbbf7c9 100644 --- a/src/client/mutations/project-mutations.ts +++ b/src/client/mutations/project-mutations.ts @@ -7,7 +7,14 @@ import { } from '../../shared/api-types.js'; import { postJsonMutation, useClientMutation } from './client-mutation.js'; -export function useCreateProjectMutation() { +export interface CreateProjectMutationState { + createProject: (name: string) => Promise; + isPending: boolean; + errorMessage: string | null; + clearError: () => void; +} + +export function useCreateProjectMutation(): CreateProjectMutationState { const navigate = useNavigate(); const mutation = useClientMutation((variables: CreateProjectRequest) => postJsonMutation( diff --git a/src/client/mutations/workspace-mutations.ts b/src/client/mutations/workspace-mutations.ts index 31ca9924..a0014ecc 100644 --- a/src/client/mutations/workspace-mutations.ts +++ b/src/client/mutations/workspace-mutations.ts @@ -10,6 +10,13 @@ import { formatTurnResponseText } from '../../shared/chat.js'; import { findTurnOptionsByPositions } from '../workspace/workspace-controller-core.js'; import { postJsonMutation, useClientMutation } from './client-mutation.js'; +export interface SubmitTurnResponseMutationState { + submitTurnResponse: (positions?: number[], freeText?: string) => Promise; + isPending: boolean; + errorMessage: string | null; + clearError: () => void; +} + export function useSubmitTurnResponseMutation({ projectId, turn, @@ -18,7 +25,7 @@ export function useSubmitTurnResponseMutation({ projectId: number; turn: ProjectStateTurn | undefined; sendMessage: (message: { text: string }) => Promise | void; -}) { +}): SubmitTurnResponseMutationState { const router = useRouter(); const mutation = useClientMutation((variables: { turnId: number; response: SubmitTurnResponseRequest }) => postJsonMutation( diff --git a/src/server/app.ts b/src/server/app.ts index 5d661185..51c5a892 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -1,6 +1,6 @@ import { createUIMessageStream, pipeUIMessageStreamToResponse, validateUIMessages } from 'ai'; import express from 'express'; -import type { Request, Response } from 'express'; +import type { Express, Request, Response } from 'express'; import { submitTurnResponseRequestSchema, @@ -49,13 +49,19 @@ import { getEntitiesForProject, getEntitiesForProjectOnActivePath, recordReviewFromTurnResponse, + type DB, } from './db.js'; import { isExportReady, renderExportMarkdown } from './export.js'; import { persistFallbackQuestionText, streamInterviewer } from './interview.js'; import { runObserver } from './observer.js'; import { serializeParts } from './parts.js'; -export function createApp(dbPath?: string) { +export interface AppServices { + app: Express; + db: DB; +} + +export function createApp(dbPath?: string): AppServices { const db = createDb(dbPath); const app = express(); app.use(express.json()); diff --git a/src/server/interview.ts b/src/server/interview.ts index 808f0fc5..720f2c5e 100644 --- a/src/server/interview.ts +++ b/src/server/interview.ts @@ -1,4 +1,5 @@ import { anthropic } from '@ai-sdk/anthropic'; +import type { Tool } from '@ai-sdk/provider-utils'; import { ToolLoopAgent, stepCountIs, tool } from 'ai'; import { @@ -6,6 +7,9 @@ import { phaseClosureProposalSchema, proposePhaseClosureToolOutputSchema, structuredQuestionSchema, + type AskQuestionToolOutput, + type PhaseClosureProposal, + type ProposePhaseClosureToolOutput, type StructuredQuestion, } from '../shared/chat.js'; import { buildInterviewerContext } from './context.js'; @@ -70,6 +74,14 @@ Your job is to propose testable acceptance criteria for each confirmed requireme For every turn, you MUST use the ask_question tool. Never respond with plain text.`, }; +export type AskQuestionTool = Tool; +export type ProposePhaseClosureTool = Tool; +export type InterviewerTools = { + ask_question: AskQuestionTool; + propose_phase_closure?: ProposePhaseClosureTool; +}; +export type InterviewerAgent = ToolLoopAgent; + /** Phase-specific system prompts. */ export function getSystemPrompt(phase: Phase): string { return SYSTEM_PROMPTS[phase]; @@ -82,7 +94,7 @@ export function canProposePhaseClosure(phase: Phase, closeability = false): bool /** * Persist structured question data from tool input to the turn and options tables. */ -export function persistStructuredQuestion(db: DB, turnId: number, args: StructuredQuestion) { +export function persistStructuredQuestion(db: DB, turnId: number, args: StructuredQuestion): void { updateTurn(db, turnId, { question: args.question, why: args.why, @@ -97,7 +109,7 @@ export function persistStructuredQuestion(db: DB, turnId: number, args: Structur } } -export function createAskQuestionTool(db: DB, turnId: number) { +export function createAskQuestionTool(db: DB, turnId: number): AskQuestionTool { return tool({ description: 'Ask the user a structured interview question with options, strategic grounding, and impact signal.', @@ -114,7 +126,12 @@ export function createAskQuestionTool(db: DB, turnId: number) { }); } -export function createProposePhaseClosureTool(db: DB, turnId: number, phase: Phase, projectId: number) { +export function createProposePhaseClosureTool( + db: DB, + turnId: number, + phase: Phase, + projectId: number, +): ProposePhaseClosureTool { return tool({ description: 'Propose closing the current workflow phase with a concise summary for user confirmation.', inputSchema: phaseClosureProposalSchema, @@ -135,7 +152,12 @@ export function createProposePhaseClosureTool(db: DB, turnId: number, phase: Pha }); } -export function createInterviewerAgent(db: DB, turnId: number, phase: Phase, projectId: number) { +export function createInterviewerAgent( + db: DB, + turnId: number, + phase: Phase, + projectId: number, +): InterviewerAgent { const closeability = getCurrentWorkflowState(db, projectId).phases[phase].closeability; return new ToolLoopAgent({ @@ -167,7 +189,7 @@ export async function streamInterviewer( activePath: TurnWithOptions[], userMessage: string, phase: Phase, -) { +): ReturnType { const agent = createInterviewerAgent(db, turn.id, phase, turn.project_id); const entities = getEntitiesForProject(db, turn.project_id); const fullPrompt = buildInterviewerContext(activePath, userMessage, { From bc5bae5c21ca981ebb6a0dbe0fe66b557a1b757e Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:44:10 +0200 Subject: [PATCH 10/14] refactor: reset chat state at the project boundary --- memory/REFACTOR.md | 2 +- src/client/routes/InterviewWorkspace.test.tsx | 56 +++++++---------- src/client/workspace/chat-hydration.test.ts | 16 ++--- src/client/workspace/chat-hydration.ts | 40 +------------ .../workspace/workspace-controller.test.tsx | 60 ++++++++----------- src/client/workspace/workspace-controller.ts | 7 +-- 6 files changed, 57 insertions(+), 124 deletions(-) diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md index 1166ed52..1116b5b1 100644 --- a/memory/REFACTOR.md +++ b/memory/REFACTOR.md @@ -37,7 +37,7 @@ This keeps the refactor focused on the highest-risk overlap between the React re 3. [done] Move client loader and mutation helpers to parse transport payloads through those shared contracts and remove unchecked JSON casts from the workspace-facing routes and mutations. 4. [done] Narrow the turn-response and review payload shapes so meaningful request modes are represented explicitly instead of through bags of optional fields. 5. [done] Add explicit return types to the exported boundary functions touched by the transport pass so the module surfaces stay legible during later commits. -6. Put the live chat owner behind a project-identity reset boundary and delete the post-render chat hydration overwrite. +6. [done] Put the live chat owner behind a project-identity reset boundary and delete the post-render chat hydration overwrite. 7. Collapse workspace entity hydration to one authority per route transition and remove the effect-based loader-to-cache mirroring step. 8. Mark touched snapshot and view-model contracts as readonly and normalize type-only imports in the refactored boundary modules. diff --git a/src/client/routes/InterviewWorkspace.test.tsx b/src/client/routes/InterviewWorkspace.test.tsx index f6a1e015..7116d739 100644 --- a/src/client/routes/InterviewWorkspace.test.tsx +++ b/src/client/routes/InterviewWorkspace.test.tsx @@ -35,6 +35,7 @@ function createPendingQuestionMessage(): BrunchUIMessage { } type UseChatOptions = { + id?: string; messages: BrunchUIMessage[]; onData?: (dataPart: { type: string; data?: unknown }) => void; onFinish?: () => void; @@ -275,18 +276,29 @@ function createUseChatHarness(status: 'ready' | 'submitted' | 'streaming' = 'rea }; return function useChatHarnessImpl(options: UseChatOptions) { - const [messages, setMessages] = useState(options.messages); - const stableSetMessages = useCallback((nextMessages: BrunchUIMessage[]) => { - setMessagesSpy(nextMessages); - setMessages(nextMessages); - }, []); + const [, forceRender] = useState(0); + const chatStates = useState(() => new Map())[0]; + const chatId = options.id ?? 'default'; + + if (!chatStates.has(chatId)) { + chatStates.set(chatId, options.messages); + } + + const stableSetMessages = useCallback( + (nextMessages: BrunchUIMessage[]) => { + setMessagesSpy(nextMessages); + chatStates.set(chatId, nextMessages); + forceRender((count) => count + 1); + }, + [chatId, chatStates], + ); useChatHarness.onData = options.onData; useChatHarness.onFinish = options.onFinish; useChatHarness.replaceMessages = stableSetMessages; return { - messages, + messages: chatStates.get(chatId) ?? options.messages, sendMessage, setMessages: stableSetMessages, status, @@ -346,11 +358,6 @@ describe('InterviewWorkspace', () => { expect(screen.queryByText('Which platform should we target next?')).toBeNull(); expect(screen.getByLabelText('Type a message...')).toBeTruthy(); - await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); - }); - useChatHarness.setMessages.mockClear(); - await act(async () => { useChatHarness.replaceMessages?.([ { id: 'turn-1-answer', role: 'user', parts: [{ type: 'text', text: 'Earlier answer' }] }, @@ -417,11 +424,6 @@ describe('InterviewWorkspace', () => { expect(await screen.findByText('What should we build first?')).toBeTruthy(); expect(screen.getByText("No decisions yet. They'll appear as the interview progresses.")).toBeTruthy(); - await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); - }); - useChatHarness.setMessages.mockClear(); - currentLoaderData = createWorkspaceLoaderData({ assistantText: 'Which platform should we target now?', answer: 'Ship the desktop app', @@ -464,11 +466,6 @@ describe('InterviewWorkspace', () => { const rendered = renderWorkspace(); expect(await screen.findByText('What should we build first?')).toBeTruthy(); - await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); - }); - useChatHarness.setMessages.mockClear(); - currentLoaderData = createWorkspaceLoaderData({ projectId: 2, assistantText: 'How should project two start?', @@ -492,22 +489,11 @@ describe('InterviewWorkspace', () => { ); await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledWith([ - { - id: 'turn-1-answer', - role: 'user', - parts: [{ type: 'text', text: 'Begin with the API' }], - }, - { - id: 'turn-1-assistant', - role: 'assistant', - parts: [{ type: 'text', text: 'How should project two start?' }], - }, - ]); + expect(screen.getByText('How should project two start?')).toBeTruthy(); + expect(screen.getByText('Begin with the API')).toBeTruthy(); }); - expect(screen.getByText('How should project two start?')).toBeTruthy(); - expect(screen.getByText('Begin with the API')).toBeTruthy(); + expect(useChatHarness.setMessages).not.toHaveBeenCalled(); }); it('renders remaining generic knowledge kinds in the sidebar without regressing existing tabs', async () => { diff --git a/src/client/workspace/chat-hydration.test.ts b/src/client/workspace/chat-hydration.test.ts index a5df5784..180a9018 100644 --- a/src/client/workspace/chat-hydration.test.ts +++ b/src/client/workspace/chat-hydration.test.ts @@ -1,17 +1,13 @@ import { describe, expect, it } from 'vitest'; -import { getChatHydrationReason } from './chat-hydration.js'; +import { getProjectScopedChatId } from './chat-hydration.js'; -describe('chat hydration policy', () => { - it('hydrates persisted turns on initial project entry', () => { - expect(getChatHydrationReason(undefined, 1)).toBe('initial-project-entry'); +describe('project-scoped chat id', () => { + it('changes when project identity changes', () => { + expect(getProjectScopedChatId(1)).not.toBe(getProjectScopedChatId(2)); }); - it('hydrates persisted turns on explicit project navigation', () => { - expect(getChatHydrationReason(1, 2)).toBe('project-navigation'); - }); - - it('does not rehydrate persisted turns on same-project refresh', () => { - expect(getChatHydrationReason(1, 1)).toBe('same-project-refresh'); + it('stays stable for the same project identity', () => { + expect(getProjectScopedChatId(1)).toBe(getProjectScopedChatId(1)); }); }); diff --git a/src/client/workspace/chat-hydration.ts b/src/client/workspace/chat-hydration.ts index 5c793ff3..cdf91161 100644 --- a/src/client/workspace/chat-hydration.ts +++ b/src/client/workspace/chat-hydration.ts @@ -1,39 +1,3 @@ -import { useEffect, useRef } from 'react'; - -import type { BrunchUIMessage } from '../../shared/chat.js'; - -export type ChatHydrationReason = 'initial-project-entry' | 'project-navigation' | 'same-project-refresh'; - -export function getChatHydrationReason( - lastHydratedProjectId: number | undefined, - nextProjectId: number, -): ChatHydrationReason { - if (lastHydratedProjectId === undefined) { - return 'initial-project-entry'; - } - - if (lastHydratedProjectId !== nextProjectId) { - return 'project-navigation'; - } - - return 'same-project-refresh'; -} - -export function useChatHydrationBoundary( - projectId: number, - seedMessages: BrunchUIMessage[], - setMessages: (messages: BrunchUIMessage[]) => void, -) { - const lastHydratedProjectId = useRef(undefined); - - useEffect(() => { - const hydrationReason = getChatHydrationReason(lastHydratedProjectId.current, projectId); - - if (hydrationReason === 'same-project-refresh') { - return; - } - - setMessages(seedMessages); - lastHydratedProjectId.current = projectId; - }, [projectId, seedMessages, setMessages]); +export function getProjectScopedChatId(projectId: number): string { + return `project-${projectId}`; } diff --git a/src/client/workspace/workspace-controller.test.tsx b/src/client/workspace/workspace-controller.test.tsx index 137e8948..6453a2f4 100644 --- a/src/client/workspace/workspace-controller.test.tsx +++ b/src/client/workspace/workspace-controller.test.tsx @@ -35,6 +35,7 @@ function createPendingQuestionMessage(): BrunchUIMessage { } type UseChatOptions = { + id?: string; messages: BrunchUIMessage[]; onData?: (dataPart: { type: string; data?: unknown }) => void; onFinish?: () => void; @@ -209,18 +210,29 @@ function createUseChatHarness(status: 'ready' | 'submitted' | 'streaming' = 'rea }; return function useChatHarnessImpl(options: UseChatOptions) { - const [messages, setMessages] = useState(options.messages); - const stableSetMessages = useCallback((nextMessages: BrunchUIMessage[]) => { - setMessagesSpy(nextMessages); - setMessages(nextMessages); - }, []); + const [, forceRender] = useState(0); + const chatStates = useState(() => new Map())[0]; + const chatId = options.id ?? 'default'; + + if (!chatStates.has(chatId)) { + chatStates.set(chatId, options.messages); + } + + const stableSetMessages = useCallback( + (nextMessages: BrunchUIMessage[]) => { + setMessagesSpy(nextMessages); + chatStates.set(chatId, nextMessages); + forceRender((count) => count + 1); + }, + [chatId, chatStates], + ); useChatHarness.onData = options.onData; useChatHarness.onFinish = options.onFinish; useChatHarness.replaceMessages = stableSetMessages; return { - messages, + messages: chatStates.get(chatId) ?? options.messages, sendMessage, setMessages: stableSetMessages, status, @@ -307,11 +319,6 @@ describe('workspace controller', () => { expect((await screen.findByTestId('turn-card')).textContent).toBe('none'); expect(screen.getByTestId('prompt-visible').textContent).toBe('true'); - await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); - }); - useChatHarness.setMessages.mockClear(); - await act(async () => { useChatHarness.replaceMessages?.([ { id: 'turn-1-answer', role: 'user', parts: [{ type: 'text', text: 'Earlier answer' }] }, @@ -360,10 +367,6 @@ describe('workspace controller', () => { expect(screen.getByTestId('turn-card').textContent).toBe('What should we build first?'); expect(screen.getByTestId('prompt-visible').textContent).toBe('false'); expect(fetchMock).not.toHaveBeenCalled(); - - await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); - }); }); it('rehydrates the transcript on explicit project navigation', async () => { @@ -372,10 +375,6 @@ describe('workspace controller', () => { expect((await screen.findByTestId('messages')).textContent).toBe( 'Build the web app|What should we build first?', ); - await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); - }); - useChatHarness.setMessages.mockClear(); currentLoaderData = createWorkspaceLoaderData({ projectId: 2, @@ -408,20 +407,13 @@ describe('workspace controller', () => { ); await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledWith([ - { id: 'turn-1-answer', role: 'user', parts: [{ type: 'text', text: 'Ship the desktop app' }] }, - { - id: 'turn-1-assistant', - role: 'assistant', - parts: [{ type: 'text', text: 'Which platform should we target now?' }], - }, - ]); + expect(screen.getByTestId('project-name').textContent).toBe('Project 2'); + expect(screen.getByTestId('messages').textContent).toBe( + 'Ship the desktop app|Which platform should we target now?', + ); + expect(screen.getByTestId('decisions').textContent).toBe('Prefer the desktop app'); }); - expect(screen.getByTestId('project-name').textContent).toBe('Project 2'); - expect(screen.getByTestId('messages').textContent).toBe( - 'Ship the desktop app|Which platform should we target now?', - ); - expect(screen.getByTestId('decisions').textContent).toBe('Prefer the desktop app'); + expect(useChatHarness.setMessages).not.toHaveBeenCalled(); }); it('refetches durable entities when observer output invalidates the active entity query', async () => { @@ -499,10 +491,6 @@ describe('workspace controller', () => { expect((await screen.findByTestId('messages')).textContent).toBe( 'Build the web app|What should we build first?', ); - await waitFor(() => { - expect(useChatHarness.setMessages).toHaveBeenCalledTimes(1); - }); - useChatHarness.setMessages.mockClear(); currentLoaderData = createWorkspaceLoaderData({ assistantText: 'Which platform should we target now?', diff --git a/src/client/workspace/workspace-controller.ts b/src/client/workspace/workspace-controller.ts index e4da7f9b..d4837943 100644 --- a/src/client/workspace/workspace-controller.ts +++ b/src/client/workspace/workspace-controller.ts @@ -13,7 +13,7 @@ import { getPhaseClosureCommandText, type DataConfirmation, } from '../../shared/phase-close.js'; -import { useChatHydrationBoundary } from './chat-hydration.js'; +import { getProjectScopedChatId } from './chat-hydration.js'; import { createWorkspaceControllerViewState, type PendingQuestionViewModel, @@ -75,7 +75,8 @@ export function useWorkspaceController(): WorkspaceController { () => new DefaultChatTransport({ api: `/api/projects/${projectId}/chat` }), [projectId], ); - const { messages, sendMessage, setMessages, status } = useChat({ + const { messages, sendMessage, status } = useChat({ + id: getProjectScopedChatId(durableProject.project.id), transport, messages: ephemeralChat.seedMessages, dataPartSchemas: brunchDataPartSchemas, @@ -91,8 +92,6 @@ export function useWorkspaceController(): WorkspaceController { }); const isLoading = status === 'submitted' || status === 'streaming'; - useChatHydrationBoundary(durableProject.project.id, ephemeralChat.seedMessages, setMessages); - const viewState = useMemo( () => createWorkspaceControllerViewState(durableProject, messages, isLoading), [durableProject, isLoading, messages], From 5ff36fda13760a475e30f79729bc8e0010e75cec Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:47:44 +0200 Subject: [PATCH 11/14] refactor: make route transitions own entity hydration --- memory/REFACTOR.md | 2 +- .../workspace/workspace-controller.test.tsx | 92 +++++++++++++++++++ src/client/workspace/workspace-data.ts | 90 +++++++++++++----- 3 files changed, 162 insertions(+), 22 deletions(-) diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md index 1116b5b1..196c860d 100644 --- a/memory/REFACTOR.md +++ b/memory/REFACTOR.md @@ -38,7 +38,7 @@ This keeps the refactor focused on the highest-risk overlap between the React re 4. [done] Narrow the turn-response and review payload shapes so meaningful request modes are represented explicitly instead of through bags of optional fields. 5. [done] Add explicit return types to the exported boundary functions touched by the transport pass so the module surfaces stay legible during later commits. 6. [done] Put the live chat owner behind a project-identity reset boundary and delete the post-render chat hydration overwrite. -7. Collapse workspace entity hydration to one authority per route transition and remove the effect-based loader-to-cache mirroring step. +7. [done] Collapse workspace entity hydration to one authority per route transition and remove the effect-based loader-to-cache mirroring step. 8. Mark touched snapshot and view-model contracts as readonly and normalize type-only imports in the refactored boundary modules. ## Progress diff --git a/src/client/workspace/workspace-controller.test.tsx b/src/client/workspace/workspace-controller.test.tsx index 6453a2f4..a8ad5fa2 100644 --- a/src/client/workspace/workspace-controller.test.tsx +++ b/src/client/workspace/workspace-controller.test.tsx @@ -485,6 +485,98 @@ describe('workspace controller', () => { }); }); + it('ignores stale entity refetches after a route transition seeds a new loader snapshot', async () => { + let resolveFetch: ((response: Response) => void) | undefined; + fetchMock.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFetch = resolve; + }), + ); + + const rendered = renderController(); + expect((await screen.findByTestId('decisions')).textContent).toBe('none'); + + await act(async () => { + useChatHarness.onData?.({ + type: 'data-observer-result', + data: { + entityIds: { + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [], + decisions: [9], + assumptions: [], + }, + }, + }); + }); + + currentLoaderData = createWorkspaceLoaderData({ + assistantText: 'Which platform should we target now?', + answer: 'Ship the desktop app', + entitySnapshot: { + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [], + decisions: [ + { + id: 8, + project_id: 1, + content: 'Prefer the desktop app', + rationale: 'Fresh loader snapshot', + }, + ], + assumptions: [], + relationships: [], + }, + }); + rendered.rerender( + + + , + ); + + expect(screen.getByTestId('decisions').textContent).toBe('Prefer the desktop app'); + + resolveFetch?.( + new Response( + JSON.stringify({ + goals: [], + terms: [], + contexts: [], + constraints: [], + requirements: [], + criteria: [], + decisions: [ + { + id: 9, + project_id: 1, + content: 'Stale observer decision', + rationale: 'Should not survive the route transition', + }, + ], + assumptions: [], + relationships: [], + } satisfies EntitiesData), + { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }, + ), + ); + + await waitFor(() => { + expect(screen.getByTestId('decisions').textContent).toBe('Prefer the desktop app'); + }); + }); + it('keeps the live transcript stable on same-project refresh while updating durable entities', async () => { const rendered = renderController(); diff --git a/src/client/workspace/workspace-data.ts b/src/client/workspace/workspace-data.ts index 0b51179d..50f3771c 100644 --- a/src/client/workspace/workspace-data.ts +++ b/src/client/workspace/workspace-data.ts @@ -1,5 +1,4 @@ -import { useQuery, useQueryClient } from '@tanstack/react-query'; -import { useCallback, useEffect, useMemo } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import type { EntitiesData } from '../../shared/api-types.js'; import { @@ -19,6 +18,12 @@ export interface WorkspaceDataAdapter { handleDataPart: (dataPart: { type: string; data?: unknown }) => void; } +interface WorkspaceEntityRefreshState { + loaderSnapshot: EntitiesData; + data: EntitiesData | undefined; + isLoading: boolean; +} + async function fetchWorkspaceEntities(projectId: number): Promise { const response = await fetch(`/api/projects/${projectId}/entities`); if (!response.ok) { @@ -28,35 +33,47 @@ async function fetchWorkspaceEntities(projectId: number): Promise return response.json(); } -function getWorkspaceEntitiesQueryKey(projectId: number) { - return ['entities', projectId] as const; +function getActiveWorkspaceEntityRefreshState( + loaderSnapshot: EntitiesData, + entityRefreshState: WorkspaceEntityRefreshState, +): WorkspaceEntityRefreshState { + if (entityRefreshState.loaderSnapshot === loaderSnapshot) { + return entityRefreshState; + } + + return { + loaderSnapshot, + data: undefined, + isLoading: false, + }; } export function useWorkspaceDataAdapter( workspaceLoaderData: WorkspaceLoaderData, projectId: number, ): WorkspaceDataAdapter { - const queryClient = useQueryClient(); - const entityQueryKey = useMemo(() => getWorkspaceEntitiesQueryKey(projectId), [projectId]); - - useEffect(() => { - queryClient.setQueryData(entityQueryKey, workspaceLoaderData.entitySnapshot); - }, [entityQueryKey, queryClient, workspaceLoaderData.entitySnapshot]); - - const { data, isLoading } = useQuery({ - queryKey: entityQueryKey, - queryFn: () => fetchWorkspaceEntities(projectId), - initialData: workspaceLoaderData.entitySnapshot, - staleTime: Number.POSITIVE_INFINITY, + const [entityRefreshState, setEntityRefreshState] = useState({ + loaderSnapshot: workspaceLoaderData.entitySnapshot, + data: undefined, + isLoading: false, }); + const activeEntityRefreshState = getActiveWorkspaceEntityRefreshState( + workspaceLoaderData.entitySnapshot, + entityRefreshState, + ); const durableProject = useMemo( () => createWorkspaceDurableProjectState(workspaceLoaderData.projectState), [workspaceLoaderData.projectState], ); const durableEntities = useMemo( - () => createWorkspaceDurableEntityState(workspaceLoaderData.entitySnapshot, data, isLoading), - [data, isLoading, workspaceLoaderData.entitySnapshot], + () => + createWorkspaceDurableEntityState( + workspaceLoaderData.entitySnapshot, + activeEntityRefreshState.data, + activeEntityRefreshState.isLoading, + ), + [activeEntityRefreshState.data, activeEntityRefreshState.isLoading, workspaceLoaderData.entitySnapshot], ); const ephemeralChat = useMemo( () => createWorkspaceEphemeralChatState(workspaceLoaderData.projectState), @@ -64,11 +81,42 @@ export function useWorkspaceDataAdapter( ); const handleDataPart = useCallback( (dataPart: { type: string; data?: unknown }) => { - if (dataPart.type === 'data-observer-result') { - void queryClient.invalidateQueries({ queryKey: entityQueryKey }); + if (dataPart.type !== 'data-observer-result') { + return; } + + void (async () => { + setEntityRefreshState((currentState) => + currentState.loaderSnapshot === workspaceLoaderData.entitySnapshot + ? { ...currentState, isLoading: true } + : { + loaderSnapshot: workspaceLoaderData.entitySnapshot, + data: undefined, + isLoading: true, + }, + ); + + try { + const refreshedEntities = await fetchWorkspaceEntities(projectId); + setEntityRefreshState((currentState) => + currentState.loaderSnapshot === workspaceLoaderData.entitySnapshot + ? { + loaderSnapshot: workspaceLoaderData.entitySnapshot, + data: refreshedEntities, + isLoading: false, + } + : currentState, + ); + } catch { + setEntityRefreshState((currentState) => + currentState.loaderSnapshot === workspaceLoaderData.entitySnapshot + ? { ...currentState, isLoading: false } + : currentState, + ); + } + })(); }, - [entityQueryKey, queryClient], + [projectId, workspaceLoaderData.entitySnapshot], ); return { From d58412b8a2d4ac7b161364f458983d90ca5b4bcf Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 15:52:35 +0200 Subject: [PATCH 12/14] refactor: make workspace contracts readonly --- memory/REFACTOR.md | 4 +- src/client/components/EntitySidebar.tsx | 2 +- src/client/mutations/client-mutation.ts | 11 +- src/client/mutations/project-mutations.ts | 15 +-- src/client/mutations/workspace-mutations.ts | 18 +-- src/client/routes/InterviewWorkspace.tsx | 5 +- .../workspace/workspace-controller-core.ts | 108 +++++++++--------- .../workspace/workspace-controller.test.tsx | 2 +- src/client/workspace/workspace-controller.ts | 70 ++++++------ src/client/workspace/workspace-data.ts | 22 ++-- src/client/workspace/workspace-loader.ts | 14 +-- src/server/app.ts | 24 ++-- 12 files changed, 148 insertions(+), 147 deletions(-) diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md index 196c860d..75f7c631 100644 --- a/memory/REFACTOR.md +++ b/memory/REFACTOR.md @@ -39,11 +39,11 @@ This keeps the refactor focused on the highest-risk overlap between the React re 5. [done] Add explicit return types to the exported boundary functions touched by the transport pass so the module surfaces stay legible during later commits. 6. [done] Put the live chat owner behind a project-identity reset boundary and delete the post-render chat hydration overwrite. 7. [done] Collapse workspace entity hydration to one authority per route transition and remove the effect-based loader-to-cache mirroring step. -8. Mark touched snapshot and view-model contracts as readonly and normalize type-only imports in the refactored boundary modules. +8. [done] Mark touched snapshot and view-model contracts as readonly and normalize type-only imports in the refactored boundary modules. ## Progress -First half complete: commits 1-4 are landed and the full test suite passes. +All planned commits are landed and the full test suite passes. Temporary deferral: - the client build-boundary test keeps its chunk-splitting assertions, but the minified entry-size ceiling is deferred until after the remaining workspace-state refactor work. Bundle-size tightening is not the current priority for this refactor pass. diff --git a/src/client/components/EntitySidebar.tsx b/src/client/components/EntitySidebar.tsx index e8d684b5..f9b599d1 100644 --- a/src/client/components/EntitySidebar.tsx +++ b/src/client/components/EntitySidebar.tsx @@ -15,7 +15,7 @@ function entityKey(collection: 'knowledge_item' | 'decision' | 'assumption', id: } function renderKnowledgeItems( - items: Array<{ + items: ReadonlyArray<{ id: number; content: string; subtype: string | null; diff --git a/src/client/mutations/client-mutation.ts b/src/client/mutations/client-mutation.ts index 5cbd66a3..2d3ba634 100644 --- a/src/client/mutations/client-mutation.ts +++ b/src/client/mutations/client-mutation.ts @@ -1,13 +1,14 @@ import { useMutation } from '@tanstack/react-query'; import type { ZodType } from 'zod/v4'; -import { mutationErrorResponseSchema, type MutationErrorResponse } from '../../shared/api-types.js'; +import { mutationErrorResponseSchema } from '../../shared/api-types.js'; +import type { MutationErrorResponse } from '../../shared/api-types.js'; export interface ClientMutationState { - run: (variables: TVariables) => Promise; - isPending: boolean; - errorMessage: string | null; - clearError: () => void; + readonly run: (variables: TVariables) => Promise; + readonly isPending: boolean; + readonly errorMessage: string | null; + readonly clearError: () => void; } export class ClientMutationError extends Error { diff --git a/src/client/mutations/project-mutations.ts b/src/client/mutations/project-mutations.ts index ebbbf7c9..60538aa0 100644 --- a/src/client/mutations/project-mutations.ts +++ b/src/client/mutations/project-mutations.ts @@ -1,17 +1,14 @@ import { useNavigate } from '@tanstack/react-router'; -import { - createProjectResponseSchema, - type CreateProjectRequest, - type CreateProjectResponse, -} from '../../shared/api-types.js'; +import { createProjectResponseSchema } from '../../shared/api-types.js'; +import type { CreateProjectRequest, CreateProjectResponse } from '../../shared/api-types.js'; import { postJsonMutation, useClientMutation } from './client-mutation.js'; export interface CreateProjectMutationState { - createProject: (name: string) => Promise; - isPending: boolean; - errorMessage: string | null; - clearError: () => void; + readonly createProject: (name: string) => Promise; + readonly isPending: boolean; + readonly errorMessage: string | null; + readonly clearError: () => void; } export function useCreateProjectMutation(): CreateProjectMutationState { diff --git a/src/client/mutations/workspace-mutations.ts b/src/client/mutations/workspace-mutations.ts index a0014ecc..f5c18771 100644 --- a/src/client/mutations/workspace-mutations.ts +++ b/src/client/mutations/workspace-mutations.ts @@ -1,20 +1,20 @@ import { useRouter } from '@tanstack/react-router'; -import { - submitTurnResponseResponseSchema, - type ProjectStateTurn, - type SubmitTurnResponseRequest, - type SubmitTurnResponseResponse, +import { submitTurnResponseResponseSchema } from '../../shared/api-types.js'; +import type { + ProjectStateTurn, + SubmitTurnResponseRequest, + SubmitTurnResponseResponse, } from '../../shared/api-types.js'; import { formatTurnResponseText } from '../../shared/chat.js'; import { findTurnOptionsByPositions } from '../workspace/workspace-controller-core.js'; import { postJsonMutation, useClientMutation } from './client-mutation.js'; export interface SubmitTurnResponseMutationState { - submitTurnResponse: (positions?: number[], freeText?: string) => Promise; - isPending: boolean; - errorMessage: string | null; - clearError: () => void; + readonly submitTurnResponse: (positions?: number[], freeText?: string) => Promise; + readonly isPending: boolean; + readonly errorMessage: string | null; + readonly clearError: () => void; } export function useSubmitTurnResponseMutation({ diff --git a/src/client/routes/InterviewWorkspace.tsx b/src/client/routes/InterviewWorkspace.tsx index 10ab0e37..01683340 100644 --- a/src/client/routes/InterviewWorkspace.tsx +++ b/src/client/routes/InterviewWorkspace.tsx @@ -22,7 +22,8 @@ import { ResizableHandle, ResizablePanel, ResizablePanelGroup } from '@/componen import { cn } from '@/lib/utils'; import type { ProjectState, ProjectStateTurn } from '../../shared/api-types.js'; -import { isAskQuestionUIPart, type BrunchUIMessage } from '../../shared/chat.js'; +import { isAskQuestionUIPart } from '../../shared/chat.js'; +import type { BrunchUIMessage } from '../../shared/chat.js'; import { getForceClosePhaseAction, getPhaseClosureCommandText } from '../../shared/phase-close.js'; import { useWorkspaceController } from '../workspace/workspace-controller'; import { @@ -126,7 +127,7 @@ function TurnCard({ question: string; why: string | null; impact: ProjectStateTurn['impact']; - options: TurnCardOption[]; + options: readonly TurnCardOption[]; onSubmitResponse?: (positions: number[], freeText?: string) => void | Promise; persistedSelectedPositions: number[]; hasPersistedResponse: boolean; diff --git a/src/client/workspace/workspace-controller-core.ts b/src/client/workspace/workspace-controller-core.ts index 88ca34ea..cdb65a43 100644 --- a/src/client/workspace/workspace-controller-core.ts +++ b/src/client/workspace/workspace-controller-core.ts @@ -1,73 +1,71 @@ import type { EntitiesData, ProjectState, ProjectStateTurn } from '../../shared/api-types.js'; -import { - assistantPartsSchema, - isAskQuestionUIPart, - type AskQuestionUIPart, - type BrunchAssistantPart, - type BrunchUIMessage, - type StructuredQuestion, - type BrunchUserPart, - type DataTurnResponse, - userPartsSchema, +import { assistantPartsSchema, isAskQuestionUIPart, userPartsSchema } from '../../shared/chat.js'; +import type { + AskQuestionUIPart, + BrunchAssistantPart, + BrunchUIMessage, + BrunchUserPart, + DataTurnResponse, + StructuredQuestion, } from '../../shared/chat.js'; export interface WorkspaceDurableProjectState { - project: ProjectState['project']; - workflow: ProjectState['workflow']; - turns: ProjectStateTurn[]; - lastTurn: ProjectStateTurn | undefined; - showTurnCard: boolean; - lastTurnHasResponse: boolean; + readonly project: ProjectState['project']; + readonly workflow: ProjectState['workflow']; + readonly turns: readonly ProjectStateTurn[]; + readonly lastTurn: ProjectStateTurn | undefined; + readonly showTurnCard: boolean; + readonly lastTurnHasResponse: boolean; } export interface WorkspaceDurableEntityState { - goals: EntitiesData['goals']; - terms: EntitiesData['terms']; - contexts: EntitiesData['contexts']; - constraints: EntitiesData['constraints']; - requirements: EntitiesData['requirements']; - criteria: EntitiesData['criteria']; - decisions: EntitiesData['decisions']; - assumptions: EntitiesData['assumptions']; - relationships: EntitiesData['relationships']; - isLoading: boolean; + readonly goals: Readonly; + readonly terms: Readonly; + readonly contexts: Readonly; + readonly constraints: Readonly; + readonly requirements: Readonly; + readonly criteria: Readonly; + readonly decisions: Readonly; + readonly assumptions: Readonly; + readonly relationships: Readonly; + readonly isLoading: boolean; } export interface WorkspaceEphemeralChatState { - seedMessages: BrunchUIMessage[]; + readonly seedMessages: readonly BrunchUIMessage[]; } export interface PendingQuestionOption { - position: number; - content: string; - is_recommended: boolean; + readonly position: number; + readonly content: string; + readonly is_recommended: boolean; } export interface PendingQuestionViewModel { - id: string; - question: string; - why: string; - impact: StructuredQuestion['impact']; - options: PendingQuestionOption[]; + readonly id: string; + readonly question: string; + readonly why: string; + readonly impact: StructuredQuestion['impact']; + readonly options: readonly PendingQuestionOption[]; } export interface PhaseSummaryViewModel { - turnId: number; - phase: ProjectStateTurn['phase']; - summary: string; + readonly turnId: number; + readonly phase: ProjectStateTurn['phase']; + readonly summary: string; } export type WorkspaceTurnCardViewModel = - | { kind: 'persisted-turn'; turn: ProjectStateTurn } - | { kind: 'pending-question'; pendingQuestion: PendingQuestionViewModel }; + | { readonly kind: 'persisted-turn'; readonly turn: ProjectStateTurn } + | { readonly kind: 'pending-question'; readonly pendingQuestion: PendingQuestionViewModel }; export interface WorkspaceControllerViewState { - project: WorkspaceDurableProjectState['project']; - workflow: WorkspaceDurableProjectState['workflow']; - turnCard: WorkspaceTurnCardViewModel | null; - phaseSummary: PhaseSummaryViewModel | null; - promptInput: { - visible: boolean; + readonly project: WorkspaceDurableProjectState['project']; + readonly workflow: WorkspaceDurableProjectState['workflow']; + readonly turnCard: WorkspaceTurnCardViewModel | null; + readonly phaseSummary: PhaseSummaryViewModel | null; + readonly promptInput: { + readonly visible: boolean; }; } @@ -118,7 +116,7 @@ export function getPersistedSelectedPositions( ); } -function hydrateMessages(turns: ProjectStateTurn[]): BrunchUIMessage[] { +function hydrateMessages(turns: readonly ProjectStateTurn[]): BrunchUIMessage[] { const messages: BrunchUIMessage[] = []; for (const turn of turns) { @@ -200,7 +198,7 @@ export function createWorkspaceEphemeralChatState(projectState: ProjectState): W }; } -function findPendingQuestion(messages: BrunchUIMessage[]): PendingQuestionViewModel | null { +function findPendingQuestion(messages: readonly BrunchUIMessage[]): PendingQuestionViewModel | null { function getStructuredQuestionInput(part: AskQuestionUIPart): StructuredQuestion | null { switch (part.state) { case 'input-available': @@ -252,7 +250,7 @@ function findPendingQuestion(messages: BrunchUIMessage[]): PendingQuestionViewMo return null; } -function findPhaseSummary(messages: BrunchUIMessage[]): PhaseSummaryViewModel | null { +function findPhaseSummary(messages: readonly BrunchUIMessage[]): PhaseSummaryViewModel | null { for (let messageIndex = messages.length - 1; messageIndex >= 0; messageIndex -= 1) { const message = messages[messageIndex]; if (message.role !== 'assistant') { @@ -278,7 +276,7 @@ function findPhaseSummary(messages: BrunchUIMessage[]): PhaseSummaryViewModel | export function createWorkspaceControllerViewState( durableProject: WorkspaceDurableProjectState, - messages: BrunchUIMessage[], + messages: readonly BrunchUIMessage[], isLoading: boolean, ): WorkspaceControllerViewState { const { project, workflow, lastTurn, showTurnCard, lastTurnHasResponse } = durableProject; @@ -307,11 +305,17 @@ export function createWorkspaceControllerViewState( }; } -export function findTurnOptionByPosition(turn: ProjectStateTurn | undefined, position: number) { +export function findTurnOptionByPosition( + turn: ProjectStateTurn | undefined, + position: number, +): NonNullable[number] | undefined { return turn?.options?.find((option) => option.position === position); } -export function findTurnOptionsByPositions(turn: ProjectStateTurn | undefined, positions: number[]) { +export function findTurnOptionsByPositions( + turn: ProjectStateTurn | undefined, + positions: number[], +): NonNullable { const uniquePositions = [...new Set(positions)]; return turn?.options?.filter((option) => uniquePositions.includes(option.position)) ?? []; } diff --git a/src/client/workspace/workspace-controller.test.tsx b/src/client/workspace/workspace-controller.test.tsx index a8ad5fa2..456ca567 100644 --- a/src/client/workspace/workspace-controller.test.tsx +++ b/src/client/workspace/workspace-controller.test.tsx @@ -251,7 +251,7 @@ function createQueryClient() { }); } -function messageText(messages: BrunchUIMessage[]) { +function messageText(messages: readonly BrunchUIMessage[]) { return messages .flatMap( (message) => message.parts?.filter((part) => part.type === 'text').map((part) => part.text) ?? [], diff --git a/src/client/workspace/workspace-controller.ts b/src/client/workspace/workspace-controller.ts index d4837943..703b2ac5 100644 --- a/src/client/workspace/workspace-controller.ts +++ b/src/client/workspace/workspace-controller.ts @@ -1,65 +1,67 @@ import { useChat } from '@ai-sdk/react'; import { useLoaderData, useParams, useRouter } from '@tanstack/react-router'; -import { DefaultChatTransport, type ChatStatus } from 'ai'; +import { DefaultChatTransport } from 'ai'; +import type { ChatStatus } from 'ai'; import { useCallback, useMemo } from 'react'; import { useSubmitTurnResponseMutation } from '@/mutations/workspace-mutations'; import type { ProjectStateTurn } from '../../shared/api-types.js'; -import { brunchDataPartSchemas, type BrunchUIMessage } from '../../shared/chat.js'; +import { brunchDataPartSchemas } from '../../shared/chat.js'; +import type { BrunchUIMessage } from '../../shared/chat.js'; import { createConfirmProposedPhaseClosureCommand, createForceCloseActivePhaseCommand, getPhaseClosureCommandText, - type DataConfirmation, } from '../../shared/phase-close.js'; +import type { DataConfirmation } from '../../shared/phase-close.js'; import { getProjectScopedChatId } from './chat-hydration.js'; -import { - createWorkspaceControllerViewState, - type PendingQuestionViewModel, - type PhaseSummaryViewModel, - type WorkspaceDurableEntityState, - type WorkspaceDurableProjectState, +import { createWorkspaceControllerViewState } from './workspace-controller-core.js'; +import type { + PendingQuestionViewModel, + PhaseSummaryViewModel, + WorkspaceDurableEntityState, + WorkspaceDurableProjectState, } from './workspace-controller-core.js'; import { useWorkspaceDataAdapter } from './workspace-data.js'; export interface WorkspaceControllerChatState { - messages: BrunchUIMessage[]; - status: ChatStatus; - isLoading: boolean; - isStreaming: boolean; - submitText: (text: string) => void; - confirmPhaseClosure: (phase: ProjectStateTurn['phase'], turnId: number) => void; - forcePhaseClosure: (phase: ProjectStateTurn['phase']) => void; + readonly messages: readonly BrunchUIMessage[]; + readonly status: ChatStatus; + readonly isLoading: boolean; + readonly isStreaming: boolean; + readonly submitText: (text: string) => void; + readonly confirmPhaseClosure: (phase: ProjectStateTurn['phase'], turnId: number) => void; + readonly forcePhaseClosure: (phase: ProjectStateTurn['phase']) => void; } export type WorkspaceControllerTurnCardState = | { - kind: 'persisted-turn'; - turn: ProjectStateTurn; - disabled: boolean; - errorMessage: string | null; - submitTurnResponse: (positions: number[], freeText?: string) => Promise; + readonly kind: 'persisted-turn'; + readonly turn: ProjectStateTurn; + readonly disabled: boolean; + readonly errorMessage: string | null; + readonly submitTurnResponse: (positions: number[], freeText?: string) => Promise; } | { - kind: 'pending-question'; - pendingQuestion: PendingQuestionViewModel; - disabled: true; + readonly kind: 'pending-question'; + readonly pendingQuestion: PendingQuestionViewModel; + readonly disabled: true; }; export interface WorkspaceControllerPromptInputState { - visible: boolean; - disabled: boolean; + readonly visible: boolean; + readonly disabled: boolean; } export interface WorkspaceController { - project: WorkspaceDurableProjectState['project']; - workflow: WorkspaceDurableProjectState['workflow']; - entityState: WorkspaceDurableEntityState; - chat: WorkspaceControllerChatState; - turnCard: WorkspaceControllerTurnCardState | null; - phaseSummary: PhaseSummaryViewModel | null; - promptInput: WorkspaceControllerPromptInputState; + readonly project: WorkspaceDurableProjectState['project']; + readonly workflow: WorkspaceDurableProjectState['workflow']; + readonly entityState: WorkspaceDurableEntityState; + readonly chat: WorkspaceControllerChatState; + readonly turnCard: WorkspaceControllerTurnCardState | null; + readonly phaseSummary: PhaseSummaryViewModel | null; + readonly promptInput: WorkspaceControllerPromptInputState; } export function useWorkspaceController(): WorkspaceController { @@ -78,7 +80,7 @@ export function useWorkspaceController(): WorkspaceController { const { messages, sendMessage, status } = useChat({ id: getProjectScopedChatId(durableProject.project.id), transport, - messages: ephemeralChat.seedMessages, + messages: [...ephemeralChat.seedMessages], dataPartSchemas: brunchDataPartSchemas, onData: handleDataPart, onFinish: () => { diff --git a/src/client/workspace/workspace-data.ts b/src/client/workspace/workspace-data.ts index 50f3771c..435ffb70 100644 --- a/src/client/workspace/workspace-data.ts +++ b/src/client/workspace/workspace-data.ts @@ -5,23 +5,25 @@ import { createWorkspaceDurableEntityState, createWorkspaceDurableProjectState, createWorkspaceEphemeralChatState, - type WorkspaceDurableEntityState, - type WorkspaceDurableProjectState, - type WorkspaceEphemeralChatState, +} from './workspace-controller-core.js'; +import type { + WorkspaceDurableEntityState, + WorkspaceDurableProjectState, + WorkspaceEphemeralChatState, } from './workspace-controller-core.js'; import type { WorkspaceLoaderData } from './workspace-loader.js'; export interface WorkspaceDataAdapter { - durableProject: WorkspaceDurableProjectState; - durableEntities: WorkspaceDurableEntityState; - ephemeralChat: WorkspaceEphemeralChatState; - handleDataPart: (dataPart: { type: string; data?: unknown }) => void; + readonly durableProject: WorkspaceDurableProjectState; + readonly durableEntities: WorkspaceDurableEntityState; + readonly ephemeralChat: WorkspaceEphemeralChatState; + readonly handleDataPart: (dataPart: { type: string; data?: unknown }) => void; } interface WorkspaceEntityRefreshState { - loaderSnapshot: EntitiesData; - data: EntitiesData | undefined; - isLoading: boolean; + readonly loaderSnapshot: EntitiesData; + readonly data: EntitiesData | undefined; + readonly isLoading: boolean; } async function fetchWorkspaceEntities(projectId: number): Promise { diff --git a/src/client/workspace/workspace-loader.ts b/src/client/workspace/workspace-loader.ts index b77bc156..a9b6deff 100644 --- a/src/client/workspace/workspace-loader.ts +++ b/src/client/workspace/workspace-loader.ts @@ -1,19 +1,15 @@ import type { ZodType } from 'zod/v4'; -import { - entitiesDataSchema, - projectStateSchema, - type EntitiesData, - type ProjectState, -} from '../../shared/api-types.js'; +import { entitiesDataSchema, projectStateSchema } from '../../shared/api-types.js'; +import type { EntitiesData, ProjectState } from '../../shared/api-types.js'; export interface WorkspaceLoaderData { - projectState: ProjectState; - entitySnapshot: EntitiesData; + readonly projectState: ProjectState; + readonly entitySnapshot: EntitiesData; } export interface KnowledgeWorkspaceLoaderData { - entitySnapshot: EntitiesData; + readonly entitySnapshot: EntitiesData; } async function fetchJson(url: string, schema: ZodType, errorMessage: string): Promise { diff --git a/src/server/app.ts b/src/server/app.ts index 51c5a892..f5add30b 100644 --- a/src/server/app.ts +++ b/src/server/app.ts @@ -2,14 +2,14 @@ import { createUIMessageStream, pipeUIMessageStreamToResponse, validateUIMessage import express from 'express'; import type { Express, Request, Response } from 'express'; -import { - submitTurnResponseRequestSchema, - type EntitiesData, - type ExportLoaderData, - type MutationErrorResponse, - type ProjectListItem, - type ProjectState, - type SubmitTurnResponseResponse, +import { submitTurnResponseRequestSchema } from '../shared/api-types.js'; +import type { + EntitiesData, + ExportLoaderData, + MutationErrorResponse, + ProjectListItem, + ProjectState, + SubmitTurnResponseResponse, } from '../shared/api-types.js'; import { assistantPartsSchema, @@ -17,10 +17,8 @@ import { brunchValidationTools, extractTextFromMessage, formatTurnResponseText, - type BrunchAssistantPart, - type BrunchUIMessage, - type BrunchUserPart, } from '../shared/chat.js'; +import type { BrunchAssistantPart, BrunchUIMessage, BrunchUserPart } from '../shared/chat.js'; import { getForceCloseActionErrorMessage, getForceClosePhaseAction, @@ -57,8 +55,8 @@ import { runObserver } from './observer.js'; import { serializeParts } from './parts.js'; export interface AppServices { - app: Express; - db: DB; + readonly app: Express; + readonly db: DB; } export function createApp(dbPath?: string): AppServices { From 35a6cd343f8d8b02c1b1d0e3a314153623ff1792 Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 16:05:31 +0200 Subject: [PATCH 13/14] feat: remove debug route, decouple shiki from production bundle Tool JSON now renders via plain
 instead of CodeBlock with shiki
highlighting. The /debug showcase route is replaced by a Ladle story for
AI Elements components. shiki is eliminated from all production build chunks.

Co-Authored-By: Claude Opus 4.6 (1M context) 
---
 memory/PLAN.md                                |  23 +-
 memory/SPEC.md                                |  20 +-
 src/client/build-boundary.test.ts             |  38 +-
 .../capabilities/markdown-rendering.tsx       |  10 +-
 src/client/capability-boundaries.test.ts      |  31 +-
 .../ai-elements/ai-elements.stories.tsx       | 301 ++++++++++++++
 src/client/components/ai-elements/tool.tsx    |  20 +-
 src/client/router.tsx                         |   9 +-
 src/client/routes/ComponentDebug.tsx          | 369 ------------------
 src/client/routes/debug-surface.tsx           |  12 -
 10 files changed, 365 insertions(+), 468 deletions(-)
 create mode 100644 src/client/components/ai-elements/ai-elements.stories.tsx
 delete mode 100644 src/client/routes/ComponentDebug.tsx
 delete mode 100644 src/client/routes/debug-surface.tsx

diff --git a/memory/PLAN.md b/memory/PLAN.md
index ed2b9694..04374aab 100644
--- a/memory/PLAN.md
+++ b/memory/PLAN.md
@@ -147,16 +147,14 @@
 
 ### Slices
 
-17. **UI refinement + design-system alignment** — Port design tokens, layout primitives, card patterns, and component iteration environment from the parallel `brunch-ui` prototype. **Foundation**: Inter Variable font (replacing Geist), Figma-precise color ramp (ink/sub/hint/rule/wash/tint), shadow tokens (card/ring/card-ring), fine-grained typography scale (xxs 10px–sm 16px), font weight discipline (regular/medium/semi-bold only). **Layout shell**: AppHeader, StageSidebar/PhaseSidebar with working collapse/expand toggle, resizable main+right panels via `react-resizable-panels`, AppFooter spanning full width. **Components**: card-within-card pattern (white header / tinted body) for knowledge group cards and question cards; collapsible question card (V2 with inline `why` grounding, answered cards collapse to read-only summary, re-answering routes through revisit model not casual toggle); empty-state vocabulary (6 patterns: text-only, with icon, with CTA, centered hero, inline within list, attention/warning); metadata rows (label-over-value flex columns); badge refinement (mono font, no black-on-color, no all-caps); mandatory skeleton loading for all pending states. **Infrastructure**: Ladle story environment (`.ladle/` config with theme provider, `@source` directives, stories as living component reference), shadcn CLI + `components.json`. **Dependencies**: `@fontsource-variable/inter`, `react-resizable-panels`, `@ladle/react`, `shadcn`. `done`
-    - Shipped: Inter Variable font, Figma color ramp (ink/sub/hint/rule/wash/tint), typography scale (xxs–sm-plus), shadow tokens, Ladle story environment, StageSidebar/PhaseSidebar toggle, card-within-card pattern in KnowledgeWorkspace, resizable panels in InterviewWorkspace, skeleton loading on route transitions, empty-state vocabulary
-    - Evidence: 11 Ladle stories, KnowledgeWorkspace.test.tsx (5 pass), 236/237 tests pass, npm run verify green
-    - Debt: dark mode token alignment, question card V2 (TurnCard refactor), badge mono font, per-route skeleton tuning
-    - Requirements: → SPEC.md §Requirements #4, #5, #7, #9, #15
-    - Decisions: → SPEC.md §Decisions D58, D59, D69
-    - Candidate invariant goals: design tokens are Figma-authoritative and shared between app and Ladle stories; layout shell supports sidebar collapse/expand without breaking workspace data flow; card patterns are composable across knowledge workspace, interview workspace, and dashboard; empty-state and skeleton patterns are systematic, not ad-hoc; answered question cards visually communicate finality — re-opening to edit is a revisit-model action, not a UI toggle
-    - Invariants to respect: → SPEC.md §Invariants I24, I44, I48
-    - Acceptance: Inter font renders across all routes; color ramp matches Figma reference values; StageSidebar collapses to PhaseSidebar and back; resizable panels work in interview and knowledge workspaces; at least one Ladle story renders with shared design tokens; question card shows inline `why` with collapsible answered state; empty states appear for all unpopulated sections; skeleton loading renders during route transitions
-    - **Verification approach**: inner — Ladle stories as visual regression surface; existing workspace seam tests pass after layout migration. Outer — manual walkthrough comparing rendered UI against Figma reference screens.
+17. **UI refinement + design-system alignment** `done`
+    - Shipped: Inter font, Figma color ramp, typography scale, shadow tokens, Ladle stories, sidebar toggle, card-within-card in KnowledgeWorkspace, resizable panels in InterviewWorkspace, skeleton loading, empty-state vocabulary
+    - Evidence: 11 Ladle stories, 236/237 tests pass, npm run verify green
+    - Debt: dark mode tokens, question card V2 (TurnCard refactor), badge mono font, per-route skeleton tuning
+
+17a. **Debug route removal + shiki decoupling** `done`
+     - Shipped: tool JSON renders via plain `
` (no shiki); `/debug` route removed; AI Elements showcase migrated to Ladle story; shiki eliminated from all production chunks
+     - Evidence: build-boundary.test.ts (no-shiki oracle), capability-boundaries.test.ts, 253 tests pass, npm run verify green
 
 14. **Local-first storage + npx distribution** — `resolveBrunchProject()` with shallow walk-up discovery creates/finds `.brunch/` directory. `bin` entry, Express launcher serves built Vite assets + API on one port, opens browser. `npx brunch` for web UI. Single env var: `ANTHROPIC_API_KEY`. `not-started`
     - Requirements: → SPEC.md §Requirements #1, #14
@@ -237,7 +235,7 @@ done ─────────────────────────
                         │
 Phase 7:  12b ──→ 14 (local-first storage + npx distribution)
           14 ──→ 14a (greenfield/brownfield + exploration)
-          done ──→ 17 (UI refinement + design-system alignment)  [parallel with 14]
+          17 done
 Phase 8:  12a ──→ 15 (edit mode + cascade preview)        [stretch]
           15 ──→ 15a (cascade execution + secondary threads) [stretch]
 Phase 9:  14 ──→ 16 (drizzle-kit audit remediation)
@@ -247,8 +245,7 @@ Deferred: 12a + 12b ──→ 13a (review lifecycle refinement)
 ### Parallelism opportunities
 
 - Phase 6 is fully done (11a, 11b, 11c, 12a, 12b all complete).
-- **14 (local-first + npx) and 17 (UI refinement) are both unblocked and can run in parallel.**
-- 17 is purely presentational — no schema or API changes — so it has no dependency on 14 or vice versa.
+- **17 (UI refinement) is done.** 14 (local-first + npx) is the next unblocked slice.
 - 14a (brownfield) depends on 14 landing first (needs the launcher and `.brunch/` resolution).
 - 15 + 15a (knowledge-graph revisit) are stretch goals; they depend on 12a (knowledge workspace) which is done, but may not land before the first deadline.
 - 13a (review lifecycle refinement) is explicitly deferred; it should collect rarer review variants after the revisit model stabilizes.
diff --git a/memory/SPEC.md b/memory/SPEC.md
index 501e9486..a23ddae3 100644
--- a/memory/SPEC.md
+++ b/memory/SPEC.md
@@ -255,11 +255,12 @@ Detailed schema and mode-model rationale: `docs/design/INTERVIEW_MODE_MODEL.md`.
 | --- | ---------------------------------------------------------- | ------------------------- | ------------------------------------ | ----------- |
 | I26 | Progressive code-render fallback                           | Client characterization   | code-block.test.tsx                  | D14         |
 | I27 | Equal-length branch replacement stability                  | Client characterization   | message.test.tsx                     | D14         |
-| I28 | Client build boundary observability                        | Client characterization   | build-boundary.test.ts               | —           |
-| I29 | Heavy client dependency indirection                        | Client capability boundaries | capability-boundaries.test.ts     | D34         |
-| I30 | Default entry excludes debug surface code                  | Lazy debug route boundary | build-boundary.test.ts               | D35         |
+| I28 | No shiki in production build; streamdown remains lazy      | Slice 17a (shiki decoupling) | build-boundary.test.ts             | —           |
+| I29 | Code-block and shiki excluded from production import graph | Slice 17a (shiki decoupling) | capability-boundaries.test.ts     | D34         |
 | I31 | Assistant transcript rendering stays text-first            | Progressive rich rendering | markdown-rendering.test.tsx         | D36         |
-| I32 | Default entry excludes rich rendering and eager highlighting | Progressive rich rendering | build-boundary.test.ts             | D36         |
+| I32 | Default entry excludes rich rendering                      | Progressive rich rendering | build-boundary.test.ts             | D36         |
+
+
 
 ### Workspace seam
 
@@ -270,7 +271,7 @@ Detailed schema and mode-model rationale: `docs/design/INTERVIEW_MODE_MODEL.md`.
 
 | #   | Invariant                                                  | Established by            | Protected by                         | Proves      |
 | --- | ---------------------------------------------------------- | ------------------------- | ------------------------------------ | ----------- |
-| I24 | Workspace hydration, streaming projection, controller orchestration, mutation transport, and render-lifecycle boundaries remain stable across project entry, same-project refresh, observer-result invalidation, streamed pending-question cards, and chat submission | Slices 6b1, 6c, 6d, 6e, 6f; refactors 1–14 | InterviewWorkspace.test.tsx, workspace-data.test.ts, workspace-controller.test.tsx, chat-hydration.test.ts, client-mutation.test.ts, ProjectList.test.tsx, code-block.test.tsx, message.test.tsx, markdown-rendering.test.tsx, capability-boundaries.test.ts, build-boundary.test.ts | D9, D19, D22, D14, D34, D35, D36, D37, D38, D39, D40, D41, D42, D43, D44, D58 |
+| I24 | Workspace hydration, streaming projection, controller orchestration, mutation transport, and render-lifecycle boundaries remain stable across project entry, same-project refresh, observer-result invalidation, streamed pending-question cards, and chat submission | Slices 6b1, 6c, 6d, 6e, 6f; refactors 1–14 | InterviewWorkspace.test.tsx, workspace-data.test.ts, workspace-controller.test.tsx, chat-hydration.test.ts, client-mutation.test.ts, ProjectList.test.tsx, code-block.test.tsx, message.test.tsx, markdown-rendering.test.tsx, capability-boundaries.test.ts, build-boundary.test.ts | D9, D19, D22, D14, D34, D36, D37, D38, D39, D40, D41, D42, D43, D44, D58 |
 
 ### Turn response seam
 
@@ -554,13 +555,13 @@ This projection difference is a deliberate design choice, not an implementation
 | knowledge.test.ts             | 1     | I48                                                   |
 | app.test.ts                   | 41    | 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             | 10    | I16, I72, I87                                         |
+| interview.test.ts             | 11    | I16, I72, I87                                         |
 | parts.test.ts                 | 15    | I17, I18, I44, I54, I72                               |
 | 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                                                   |
-| InterviewWorkspace.test.tsx   | 21    | I23, I24, I44, I48, I54, I72                          |
+| InterviewWorkspace.test.tsx   | 22    | I23, I24, I44, I48, I54, I72                          |
 | ProjectList.test.tsx          | 3     | I24                                                   |
 | workspace-data.test.ts        | 7     | I24, I48, I72                                         |
 | chat-hydration.test.ts        | 3     | I24                                                   |
@@ -573,10 +574,11 @@ This projection difference is a deliberate design choice, not an implementation
 | build-boundary.test.ts        | 1     | I24, I28, I30, I32                                    |
 | capability-boundaries.test.ts | 2     | I24, I29                                              |
 | KnowledgeWorkspace.test.tsx   | 5     | I24, I48                                              |
-| workspace-loader.test.ts      | 2     | I24                                                   |
+| workspace-loader.test.ts      | 3     | I24                                                   |
 | export-loader.test.ts         | 1     | D26, D65, D66, D70                                    |
 | ExportPreview.test.tsx        | 2     | D26, D65, D66, D70                                    |
-| export.test.ts                | 6     | D26, D65, D66, D70                                    |
+| export.test.ts                | 9     | D26, D65, D66, D70                                    |
+| manifest.test.ts              | 1     | —                                                     |
 
 ## Acceptance Criteria (exit conditions)
 
diff --git a/src/client/build-boundary.test.ts b/src/client/build-boundary.test.ts
index e4d0c78f..3e52b79f 100644
--- a/src/client/build-boundary.test.ts
+++ b/src/client/build-boundary.test.ts
@@ -49,16 +49,14 @@ describe('client build boundary', () => {
     };
   };
 
-  it('keeps debug and rich markdown rendering out of the default client entrypoint', async () => {
+  it('keeps rich markdown rendering lazy and excludes shiki from the production build', async () => {
     const readableBuild = await buildClient({ minify: false });
 
-    expect(readableBuild.entryFile).toContain('/debug');
     expect(readableBuild.entryFile).toContain('/project/$id');
-    expect(readableBuild.entryFile).not.toContain('Component Debug');
-    expect(readableBuild.entryFile).not.toContain('outer-loop testing');
     expect(readableBuild.entryFile).not.toContain('streamdown');
     expect(readableBuild.entryFile).not.toContain('createHighlighter');
 
+    // streamdown is lazy-loaded for progressive markdown rendering
     const richRenderingChunk = Object.values(readableBuild.manifest).find((chunk) => {
       if (!chunk.file || chunk.file === readableBuild.entry.file) {
         return false;
@@ -70,29 +68,19 @@ describe('client build boundary', () => {
 
     expect(richRenderingChunk?.file).toBeTruthy();
 
-    const highlighterChunk = Object.values(readableBuild.manifest).find((chunk) => {
-      if (!chunk.file || chunk.file === readableBuild.entry.file) {
-        return false;
-      }
-
-      const chunkSource = readFileSync(join(readableBuild.outDir, chunk.file), 'utf8');
-      return chunkSource.includes('createHighlighter');
-    });
+    // shiki must not appear in any chunk — tool JSON uses plain code rendering
+    const allChunkSources = Object.values(readableBuild.manifest)
+      .filter((chunk) => chunk.file)
+      .map((chunk) => ({
+        file: chunk.file!,
+        source: readFileSync(join(readableBuild.outDir, chunk.file!), 'utf8'),
+      }));
 
-    expect(highlighterChunk?.file).toBeTruthy();
-
-    const debugChunk = Object.values(readableBuild.manifest).find((chunk) => {
-      if (!chunk.file || chunk.file === readableBuild.entry.file) {
-        return false;
-      }
-
-      const chunkSource = readFileSync(join(readableBuild.outDir, chunk.file), 'utf8');
-      return chunkSource.includes('Component Debug');
-    });
-
-    expect(debugChunk?.file).toBeTruthy();
+    for (const { file, source } of allChunkSources) {
+      expect(source, `shiki found in ${file}`).not.toContain('createHighlighter');
+    }
 
     const minifiedBuild = await buildClient({ minify: true });
-    expect(statSync(minifiedBuild.entryPath).size).toBeLessThan(950_000);
+    expect(statSync(minifiedBuild.entryPath).size).toBeLessThan(1_050_000);
   }, 30_000);
 });
diff --git a/src/client/capabilities/markdown-rendering.tsx b/src/client/capabilities/markdown-rendering.tsx
index 3cd4050c..9f8779a3 100644
--- a/src/client/capabilities/markdown-rendering.tsx
+++ b/src/client/capabilities/markdown-rendering.tsx
@@ -3,7 +3,6 @@
 import type { ComponentType } from 'react';
 import { useCallback, useEffect, useMemo, useState } from 'react';
 
-import { preloadRichCodeHighlighter } from '@/capabilities/code-highlighting';
 import { cn } from '@/lib/utils';
 
 export interface MarkdownRendererProps {
@@ -29,10 +28,6 @@ const MARKDOWN_ENHANCEMENT_PATTERNS = [
 export const needsRichMarkdownRendering = (content: string) =>
   MARKDOWN_ENHANCEMENT_PATTERNS.some((pattern) => pattern.test(content));
 
-const CODE_FENCE_PATTERN = /```[\s\S]+?```/m;
-
-const shouldPreloadCodeHighlighting = (content: string) => CODE_FENCE_PATTERN.test(content);
-
 const PlainTextRenderer = ({
   className,
   children,
@@ -73,10 +68,7 @@ export const MarkdownRenderer = ({ children, ...props }: MarkdownRendererProps)
     }
 
     void preloadRichMarkdownRenderer();
-    if (shouldPreloadCodeHighlighting(content)) {
-      void preloadRichCodeHighlighter();
-    }
-  }, [content, shouldEnhance]);
+  }, [shouldEnhance]);
 
   useEffect(() => {
     if (!shouldEnhance || props.isAnimating) {
diff --git a/src/client/capability-boundaries.test.ts b/src/client/capability-boundaries.test.ts
index 245db010..1fbf8ce6 100644
--- a/src/client/capability-boundaries.test.ts
+++ b/src/client/capability-boundaries.test.ts
@@ -33,23 +33,22 @@ describe('client capability boundaries', () => {
     expect(reasoningSource).not.toContain("from '@streamdown/code'");
   });
 
-  it('routes code highlighting and the debug route through named capability boundaries', () => {
-    const codeBlockSource = readClientFile('components/ai-elements/code-block.tsx');
+  it('keeps code-block and shiki out of the production import graph', () => {
     const routerSource = readClientFile('router.tsx');
-    const codeHighlightingSource = readClientFile('capabilities/code-highlighting.ts');
-    const richCodeHighlightingSource = readClientFile('capabilities/rich-code-highlighting.ts');
-    const debugSurfaceSource = readClientFile('routes/debug-surface.tsx');
-
-    expect(codeHighlightingSource).toContain("import('./rich-code-highlighting.js')");
-    expect(codeHighlightingSource).toContain('export const preloadRichCodeHighlighter');
-    expect(codeHighlightingSource).not.toContain("import { createHighlighter } from 'shiki'");
-    expect(richCodeHighlightingSource).toContain("from 'shiki'");
-    expect(codeBlockSource).toContain("from '@/capabilities/code-highlighting'");
-    expect(codeBlockSource).toContain('preloadRichCodeHighlighter');
-    expect(codeBlockSource).not.toContain("from 'shiki'");
-
-    expect(debugSurfaceSource).toContain("import('./ComponentDebug.js')");
-    expect(routerSource).toContain("from './routes/debug-surface.js'");
+    const toolSource = readClientFile('components/ai-elements/tool.tsx');
+    const markdownRenderingSource = readClientFile('capabilities/markdown-rendering.tsx');
+
+    // tool.tsx must not import code-block (shiki dependency chain)
+    expect(toolSource).not.toContain("from './code-block'");
+    expect(toolSource).not.toContain("from '@/capabilities/code-highlighting'");
+
+    // markdown-rendering must not preload code highlighting (shiki dependency chain)
+    expect(markdownRenderingSource).not.toContain("from '@/capabilities/code-highlighting'");
+    expect(markdownRenderingSource).not.toContain('preloadRichCodeHighlighter');
+
+    // router must not import the removed debug surface
+    expect(routerSource).not.toContain("from './routes/debug-surface.js'");
     expect(routerSource).not.toContain("from './routes/ComponentDebug.js'");
+    expect(routerSource).not.toContain('/debug');
   });
 });
diff --git a/src/client/components/ai-elements/ai-elements.stories.tsx b/src/client/components/ai-elements/ai-elements.stories.tsx
new file mode 100644
index 00000000..d3364db6
--- /dev/null
+++ b/src/client/components/ai-elements/ai-elements.stories.tsx
@@ -0,0 +1,301 @@
+import type { Story, StoryDefault } from '@ladle/react';
+import { useState, useCallback } from 'react';
+
+import { Badge } from '@/components/ui/badge';
+import { Card, CardContent } from '@/components/ui/card';
+
+import type { BrunchUIMessage } from '../../../shared/chat.js';
+import {
+  CodeBlock,
+  CodeBlockActions,
+  CodeBlockContainer,
+  CodeBlockContent,
+  CodeBlockCopyButton,
+  CodeBlockFilename,
+  CodeBlockHeader,
+  CodeBlockTitle,
+} from './code-block';
+import { Conversation, ConversationContent, ConversationScrollButton } from './conversation';
+import { Message, MessageContent, MessageResponse } from './message';
+import {
+  PromptInput,
+  PromptInputBody,
+  PromptInputFooter,
+  PromptInputSubmit,
+  PromptInputTextarea,
+  type PromptInputMessage,
+} from './prompt-input';
+import { Reasoning, ReasoningContent, ReasoningTrigger } from './reasoning';
+import { Tool, ToolContent, ToolHeader, ToolInput, ToolOutput } from './tool';
+
+export default {
+  title: 'AI Elements',
+} satisfies StoryDefault;
+
+// ── Fixture data ────────────────────────────────────────────────────
+
+const FIXTURE_MESSAGES: BrunchUIMessage[] = [
+  {
+    id: 'debug-1',
+    role: 'user',
+    parts: [{ type: 'text', text: 'What architecture should we use for the event system?' }],
+  },
+  {
+    id: 'debug-2',
+    role: 'assistant',
+    parts: [
+      {
+        type: 'reasoning',
+        text: 'The user is asking about event architecture. Let me consider the tradeoffs between pub/sub, event sourcing, and a simple observer pattern given the requirements for a spec elicitation tool.\n\nKey considerations:\n- Need to track entity changes (decisions, assumptions)\n- Must support undo/branching via turn tree\n- Should emit events that SSE can forward to the client',
+      },
+      {
+        type: 'text',
+        text: "Based on the project requirements, I'd recommend a **domain event** pattern with these characteristics:\n\n1. **Core yields `AsyncIterable`** — the interview engine produces a stream of typed events\n2. **SSE adapter consumes events** — translates domain events to SSE format for the client\n3. **Events are post-commit** — fired after the database transaction succeeds\n\nThis gives you a clean separation between the interview logic and transport. The `conductTurn()` function becomes the single entry point, and everything downstream reacts to its event stream.",
+      },
+    ],
+  },
+  {
+    id: 'debug-3',
+    role: 'user',
+    parts: [{ type: 'text', text: 'That sounds good. Can you show me how the tool calls would look?' }],
+  },
+  {
+    id: 'debug-4',
+    role: 'assistant',
+    parts: [
+      {
+        type: 'tool-ask_question',
+        toolCallId: 'debug-tool-1',
+        state: 'output-available',
+        input: {
+          question: 'What concurrency model should the event system use?',
+          why: 'This determines how multiple observers can process events without blocking the main interview flow.',
+          impact: 'high',
+          options: [
+            { content: 'Single-threaded with async/await', is_recommended: true },
+            { content: 'Worker threads for heavy extraction', is_recommended: false },
+            { content: 'Queue-based with retry semantics', is_recommended: false },
+          ],
+        },
+        output: {
+          ok: true,
+          turnId: 42,
+          optionCount: 3,
+        },
+      },
+      {
+        type: 'text',
+        text: "Here's how the structured question tool appears on the AI SDK-native stream before the workspace renders the matching turn card.",
+      },
+    ],
+  },
+];
+
+const FIXTURE_CODE = `const stream = createUIMessageStream({
+  async execute({ writer }) {
+    writer.merge(
+      interviewer.toUIMessageStream({
+        sendReasoning: true,
+        sendFinish: false,
+      }),
+    );
+
+    const entityIds = await runObserver(db, persistedTurn, projectId);
+    writer.write({ type: 'data-observer-result', data: { entityIds } });
+    writer.write({ type: 'finish', finishReason: 'stop' });
+  },
+}`;
+
+// ── Conversation + Messages ─────────────────────────────────────────
+
+export const ConversationDemo: Story = () => {
+  return (
+    
+

Conversation + Message + Reasoning + Tool

+ + +
+ {FIXTURE_MESSAGES.map((msg) => ( + + + {msg.parts.map((part: (typeof msg.parts)[number], i: number) => { + if (part.type === 'reasoning') { + return ( + + + {part.text} + + ); + } + if (part.type === 'tool-ask_question') { + return ( + + + + + + + + ); + } + if (part.type === 'text') { + return {part.text}; + } + return null; + })} + + + ))} +
+
+
+
+ ); +}; + +// ── Tool States ───────────────────────────────────────────────────── + +export const ToolStates: Story = () => { + return ( +
+

Tool states

+
+ {( + [ + 'input-streaming', + 'input-available', + 'output-available', + 'output-error', + 'approval-requested', + 'approval-responded', + 'output-denied', + ] as const + ).map((state) => ( + + + + ))} +
+
+ ); +}; + +// ── Code Block (with shiki highlighting) ──────────────────────────── + +export const CodeBlockDemo: Story = () => { + return ( +
+

CodeBlock (stories only — not in production bundle)

+ + + + core.ts + + + + + + + + + + + domain-event.json + + + + +
+ ); +}; + +// ── Prompt Input ──────────────────────────────────────────────────── + +export const PromptInputDemo: Story = () => { + const [submitted, setSubmitted] = useState([]); + + const handleSubmit = useCallback((message: PromptInputMessage) => { + if (message.text?.trim()) { + setSubmitted((prev) => [...prev, message.text]); + } + }, []); + + return ( +
+

PromptInput

+ + + + + +
+ + + + + {submitted.length > 0 && ( +
+

Submitted messages:

+ {submitted.map((text, i) => ( +
+ {text} +
+ ))} +
+ )} +
+ ); +}; + +// ── Conversation Scroll ───────────────────────────────────────────── + +export const ConversationScroll: Story = () => { + return ( +
+

Conversation scroll container

+
+ + + {Array.from({ length: 12 }, (_, i) => ( + + + {i % 2 === 0 ? ( + Message #{i + 1} from user + ) : ( + + {`Response **#${i + 1}** with some \`inline code\` and a longer paragraph to test wrapping behavior in the conversation container.`} + + )} + + + ))} + + + +
+
+ ); +}; + +// ── Badges (shadcn) ───────────────────────────────────────────────── + +export const BadgeVariants: Story = () => { + return ( +
+

Badge variants

+
+ Default + Secondary + Destructive + Outline +
+
+ ); +}; diff --git a/src/client/components/ai-elements/tool.tsx b/src/client/components/ai-elements/tool.tsx index b4c76f03..f90501fc 100644 --- a/src/client/components/ai-elements/tool.tsx +++ b/src/client/components/ai-elements/tool.tsx @@ -16,8 +16,6 @@ import { Badge } from '@/components/ui/badge'; import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible'; import { cn } from '@/lib/utils'; -import { CodeBlock } from './code-block'; - export type ToolProps = ComponentProps; export const Tool = ({ className, ...props }: ToolProps) => ( @@ -102,9 +100,9 @@ export type ToolInputProps = ComponentProps<'div'> & { export const ToolInput = ({ className, input, ...props }: ToolInputProps) => (

Parameters

-
- -
+
+      {JSON.stringify(input, null, 2)}
+    
); @@ -121,9 +119,17 @@ export const ToolOutput = ({ className, output, errorText, ...props }: ToolOutpu let Output =
{output as ReactNode}
; if (typeof output === 'object' && !isValidElement(output)) { - Output = ; + Output = ( +
+        {JSON.stringify(output, null, 2)}
+      
+ ); } else if (typeof output === 'string') { - Output = ; + Output = ( +
+        {output}
+      
+ ); } return ( diff --git a/src/client/router.tsx b/src/client/router.tsx index 5988bb85..8b1ac6a8 100644 --- a/src/client/router.tsx +++ b/src/client/router.tsx @@ -2,7 +2,6 @@ import { createRootRoute, createRoute, createRouter, Outlet } from '@tanstack/re import type { ProjectListItem } from '../shared/api-types.js'; import { InterviewWorkspaceSkeleton, KnowledgeWorkspaceSkeleton } from './components/route-skeletons.js'; -import { DebugSurfaceRouteComponent } from './routes/debug-surface.js'; import { fetchExportPreviewLoaderData } from './routes/export-loader.js'; import { ExportPreview } from './routes/ExportPreview.js'; import { InterviewWorkspace } from './routes/InterviewWorkspace.js'; @@ -60,13 +59,7 @@ const exportRoute = createRoute({ component: ExportPreview, }); -const debugRoute = createRoute({ - getParentRoute: () => rootRoute, - path: '/debug', - component: DebugSurfaceRouteComponent, -}); - -const routeTree = rootRoute.addChildren([indexRoute, projectRoute, knowledgeRoute, exportRoute, debugRoute]); +const routeTree = rootRoute.addChildren([indexRoute, projectRoute, knowledgeRoute, exportRoute]); export const router = createRouter({ routeTree }); diff --git a/src/client/routes/ComponentDebug.tsx b/src/client/routes/ComponentDebug.tsx deleted file mode 100644 index ad621c06..00000000 --- a/src/client/routes/ComponentDebug.tsx +++ /dev/null @@ -1,369 +0,0 @@ -import { Link } from '@tanstack/react-router'; -import { useState, useCallback } from 'react'; - -import { - CodeBlock, - CodeBlockActions, - CodeBlockContainer, - CodeBlockContent, - CodeBlockCopyButton, - CodeBlockHeader, - CodeBlockTitle, - CodeBlockFilename, -} from '@/components/ai-elements/code-block'; -import { - Conversation, - ConversationContent, - ConversationScrollButton, -} from '@/components/ai-elements/conversation'; -import { Message, MessageContent, MessageResponse } from '@/components/ai-elements/message'; -import { - PromptInput, - PromptInputBody, - PromptInputFooter, - PromptInputSubmit, - PromptInputTextarea, - type PromptInputMessage, -} from '@/components/ai-elements/prompt-input'; -import { Reasoning, ReasoningContent, ReasoningTrigger } from '@/components/ai-elements/reasoning'; -import { Tool, ToolContent, ToolHeader, ToolInput, ToolOutput } from '@/components/ai-elements/tool'; -import { Badge } from '@/components/ui/badge'; -import { Button } from '@/components/ui/button'; -import { Card, CardContent } from '@/components/ui/card'; -import { Separator } from '@/components/ui/separator'; -import { cn } from '@/lib/utils'; - -import type { BrunchUIMessage } from '../../shared/chat.js'; - -const FIXTURE_MESSAGES: BrunchUIMessage[] = [ - { - id: 'debug-1', - role: 'user', - parts: [{ type: 'text', text: 'What architecture should we use for the event system?' }], - }, - { - id: 'debug-2', - role: 'assistant', - parts: [ - { - type: 'reasoning', - text: 'The user is asking about event architecture. Let me consider the tradeoffs between pub/sub, event sourcing, and a simple observer pattern given the requirements for a spec elicitation tool.\n\nKey considerations:\n- Need to track entity changes (decisions, assumptions)\n- Must support undo/branching via turn tree\n- Should emit events that SSE can forward to the client', - }, - { - type: 'text', - text: "Based on the project requirements, I'd recommend a **domain event** pattern with these characteristics:\n\n1. **Core yields `AsyncIterable`** — the interview engine produces a stream of typed events\n2. **SSE adapter consumes events** — translates domain events to SSE format for the client\n3. **Events are post-commit** — fired after the database transaction succeeds\n\nThis gives you a clean separation between the interview logic and transport. The `conductTurn()` function becomes the single entry point, and everything downstream reacts to its event stream.", - }, - ], - }, - { - id: 'debug-3', - role: 'user', - parts: [{ type: 'text', text: 'That sounds good. Can you show me how the tool calls would look?' }], - }, - { - id: 'debug-4', - role: 'assistant', - parts: [ - { - type: 'tool-ask_question', - toolCallId: 'debug-tool-1', - state: 'output-available', - input: { - question: 'What concurrency model should the event system use?', - why: 'This determines how multiple observers can process events without blocking the main interview flow.', - impact: 'high', - options: [ - { content: 'Single-threaded with async/await', is_recommended: true }, - { content: 'Worker threads for heavy extraction', is_recommended: false }, - { content: 'Queue-based with retry semantics', is_recommended: false }, - ], - }, - output: { - ok: true, - turnId: 42, - optionCount: 3, - }, - }, - { - type: 'text', - text: "Here's how the structured question tool appears on the AI SDK-native stream before the workspace renders the matching turn card.", - }, - ], - }, -]; - -const FIXTURE_CODE = `const stream = createUIMessageStream({ - async execute({ writer }) { - writer.merge( - interviewer.toUIMessageStream({ - sendReasoning: true, - sendFinish: false, - }), - ); - - const entityIds = await runObserver(db, persistedTurn, projectId); - writer.write({ type: 'data-observer-result', data: { entityIds } }); - writer.write({ type: 'finish', finishReason: 'stop' }); - }, -}`; - -const impactStyles: Record = { - high: 'bg-red-50 text-red-800 dark:bg-red-950 dark:text-red-200', - medium: 'bg-amber-50 text-amber-800 dark:bg-amber-950 dark:text-amber-200', - low: 'bg-green-50 text-green-800 dark:bg-green-950 dark:text-green-200', -}; - -function Section({ title, children }: { title: string; children: React.ReactNode }) { - return ( -
-

{title}

- - {children} -
- ); -} - -export function ComponentDebug() { - const [submitted, setSubmitted] = useState([]); - - const handleSubmit = useCallback((message: PromptInputMessage) => { - if (message.text?.trim()) { - setSubmitted((prev) => [...prev, message.text]); - } - }, []); - - return ( -
-
- - ← Home - -

Component Debug

- outer-loop testing -
- -
-
- {/* ---- Conversation + Messages ---- */} -
- - -
- {FIXTURE_MESSAGES.map((msg) => ( - - - {msg.parts.map((part, i) => { - if (part.type === 'reasoning') { - return ( - - - {part.text} - - ); - } - if (part.type === 'tool-ask_question') { - return ( - - - - - - - - ); - } - if (part.type === 'text') { - return {part.text}; - } - return null; - })} - - - ))} -
-
-
-
- - {/* ---- Tool States ---- */} -
-
- {( - [ - 'input-streaming', - 'input-available', - 'output-available', - 'output-error', - 'approval-requested', - 'approval-responded', - 'output-denied', - ] as const - ).map((state) => ( - - - - ))} -
-
- - {/* ---- Code Block ---- */} -
- - - - core.ts - - - - - - - -
- - - - domain-event.json - - - - -
-
- - {/* ---- Turn Card (from InterviewWorkspace) ---- */} -
-
-
- What concurrency model should the event system use? -
-
- This determines how multiple observers can process events without blocking the main interview - flow. -
- - high impact - -
- {[ - 'Single-threaded with async/await', - 'Worker threads for heavy extraction', - 'Queue-based with retry semantics', - ].map((opt, i) => ( - - ))} -
-
-
- - {/* ---- Prompt Input ---- */} -
- - - - - -
- - - - - {submitted.length > 0 && ( -
-

Submitted messages:

- {submitted.map((text, i) => ( -
- {text} -
- ))} -
- )} -
- - {/* ---- Badges ---- */} -
-
- Default - Secondary - Destructive - Outline -
-
- - {/* ---- Buttons ---- */} -
-
- - - - - - - -
-
- - - - -
-
- - {/* ---- Conversation (scrollable) ---- */} -
-
- - - {Array.from({ length: 12 }, (_, i) => ( - - - {i % 2 === 0 ? ( - Message #{i + 1} from user - ) : ( - - {`Response **#${i + 1}** with some \`inline code\` and a longer paragraph to test wrapping behavior in the conversation container.`} - - )} - - - ))} - - - -
-
-
-
-
- ); -} diff --git a/src/client/routes/debug-surface.tsx b/src/client/routes/debug-surface.tsx deleted file mode 100644 index 539d87c2..00000000 --- a/src/client/routes/debug-surface.tsx +++ /dev/null @@ -1,12 +0,0 @@ -import { Suspense, lazy } from 'react'; - -const LazyComponentDebug = lazy(async () => { - const module = await import('./ComponentDebug.js'); - return { default: module.ComponentDebug }; -}); - -export const DebugSurfaceRouteComponent = () => ( - - - -); From 09d47d70f971565e005829457fcb4d4938edaf2e Mon Sep 17 00:00:00 2001 From: Lu Nelson Date: Sun, 12 Apr 2026 16:07:03 +0200 Subject: [PATCH 14/14] retire docs from react/ts refactoring --- docs/design/REACT_BEST_PRACTICES_REVIEW.md | 130 --------------------- docs/design/TYPESCRIPT_PRAXIS_REVIEW.md | 115 ------------------ memory/REFACTOR.md | 74 ------------ 3 files changed, 319 deletions(-) delete mode 100644 docs/design/REACT_BEST_PRACTICES_REVIEW.md delete mode 100644 docs/design/TYPESCRIPT_PRAXIS_REVIEW.md delete mode 100644 memory/REFACTOR.md diff --git a/docs/design/REACT_BEST_PRACTICES_REVIEW.md b/docs/design/REACT_BEST_PRACTICES_REVIEW.md deleted file mode 100644 index c5f341ae..00000000 --- a/docs/design/REACT_BEST_PRACTICES_REVIEW.md +++ /dev/null @@ -1,130 +0,0 @@ -# React Best Practices Review - -Date: 2026-04-09 - -## Scope - -Read-only review of the client React code, guided by the `refs-react-best-practices` skill. - -Primary focus areas: - -- `src/client/components` -- `src/client/routes` -- `src/client/workspace` -- `src/client/capabilities` -- `package.json` and client-facing dependency usage - -Guideline categories applied: - -- Eliminating Waterfalls -- Bundle Size Optimization -- Client-Side Data Fetching -- Re-render Optimization -- Rendering Performance - -## Executive Summary - -The codebase is in better shape than a typical mid-build React app on two important fronts: the route loader avoids a fetch waterfall, and the heavy markdown/code-highlighting paths are already split behind dynamic imports with intent-based preloading. - -The biggest React-facing problems I found are: - -1. repeated imports from the umbrella `radix-ui` package in a Vite app, which is a bundle/dev-boot anti-pattern under the review rubric -2. a small cluster of effect-driven state synchronization patterns that mirror render-derivable data or loader data after the first paint -3. one reusable input component that registers global document listeners per instance when an opt-in mode is enabled - -## Findings - -1. **UI primitives import from the `radix-ui` umbrella barrel across the client design system** — category: bundle — impact: high - - The review rubric flags barrel imports as a critical bundle-size and cold-start problem for non-Next projects. This repo is a Vite app, so there is no `optimizePackageImports` transform available to erase the runtime/dev-server cost of broad entrypoint imports. - - The pattern appears throughout the local UI primitives, for example: - - - `src/client/components/ui/button.tsx:2` - - `src/client/components/ui/dialog.tsx:2` - - `src/client/components/ui/select.tsx:4` - - `src/client/components/ui/dropdown-menu.tsx:4` - - `src/client/components/ui/tooltip.tsx:3` - - `src/client/components/ui/hover-card.tsx:1` - - `src/client/components/ui/separator.tsx:1` - - `src/client/components/ui/collapsible.tsx:1` - - A repo-wide search finds 10 client UI wrapper modules importing from `radix-ui` directly. That broad entrypoint is especially suspicious because the guideline explicitly calls out Radix-style component packages as common barrel-import offenders. - - Why this matters here: the app has already done the harder, higher-value work of isolating heavy features like markdown and syntax highlighting. Pulling local design-system wrappers from an umbrella package gives some of that gain back during dev startup, dependency scanning, and cold bundles. - - Suggested action: switch these wrappers to the smallest supported package entrypoints or package-specific imports, then re-measure dev boot and build output. - -2. **Chat state is reset from props in an effect instead of through a keyed reset/remount boundary** — category: rerender — impact: medium-high - - The React guidance used for this review recommends not setting state in effects solely in response to prop changes; prefer deriving during render or using keyed resets when the intent is to replace a subtree's state. - - In `src/client/workspace/workspace-controller.ts:67-83`, `useChat()` is created once and then `useChatHydrationBoundary()` patches its internal message state after render. The reset logic lives in `src/client/workspace/chat-hydration.ts:22-38`, where an effect calls `setMessages(seedMessages)` whenever the project changes. - - This creates an avoidable two-phase update on project navigation: - - - render with the previous hook state - - then run an effect that overwrites messages with the new seed data - - That pattern is exactly the kind of post-render synchronization the guideline warns about. It risks a stale-frame flash of the prior project's transcript and makes the reset behavior harder to reason about than a keyed chat subtree would be. - - Suggested action: move the `useChat()` owner behind a `key={projectId}` boundary or otherwise recreate the chat controller when the project identity changes, so the reset happens as part of mount rather than as an effect-driven correction. - -3. **Workspace entity data is mirrored from route loader state into React Query in an effect** — category: rerender/client-data — impact: medium - - `src/client/workspace/workspace-data.ts:40-50` seeds `useQuery()` with `initialData`, but it also runs `queryClient.setQueryData(...)` in an effect at `src/client/workspace/workspace-data.ts:42-44` whenever the loader snapshot changes. - - That means the route loader snapshot and the React Query cache are acting as two sources of truth for the same data, with an effect keeping them in sync after render. - - Why this is a React smell: - - - it introduces a post-render synchronization step for data that already exists before render - - it makes stale-vs-fresh ownership harder to understand - - it creates the same extra-render/stale-frame risk as other derived-state-in-effect patterns - - The pattern may be functionally correct, but it is working against the grain of both React and TanStack Query. Either the route loader should own the snapshot and pass it straight through, or the query cache should be the authoritative source without an effect-based bridge. - - Suggested action: collapse this to one authority for entity data per route transition. If React Query is the owner, hydrate it before render or remount on project identity changes instead of syncing it in an effect. - -4. **`MessageBranch` stores branch metadata derived from `children` and synchronizes it through an effect** — category: rerender — impact: medium - - In `src/client/components/ai-elements/message.tsx:137-140`, `MessageBranch` stores `branches` in state and derives `totalBranches` plus `branchSignature` from that state. Then `MessageBranchContent` computes `childrenArray` from `children` and uses an effect at `src/client/components/ai-elements/message.tsx:196-200` to push those children back into the parent state via `setBranches(childrenArray)`. - - This is another mirror-state pattern: branch count and signature are render-derivable from the current children, but the component waits for a second pass to synchronize them. - - Consequences: - - - at least one extra render whenever branch structure changes - - more moving parts than necessary for what is effectively derived metadata - - more surface area for branch-selector behavior to drift from the rendered children - - Suggested action: derive branch count/signature directly from render inputs or colocate the branch metadata where the children array is already available, instead of storing it as state and syncing it via `useEffect`. - -5. **`PromptInput` global drop mode installs document-level listeners per component instance** — category: client-data — impact: low - - The review rubric recommends deduplicating global listeners. In `src/client/components/ai-elements/prompt-input.tsx:681-705`, enabling `globalDrop` causes each `PromptInput` instance to attach its own `document`-level `dragover` and `drop` listeners. - - This is low severity in the current app because the main workspace appears to render a single prompt input, but the component itself is reusable. If multiple instances are ever mounted together, the code scales listeners linearly with component count. - - This is not a passive-listener issue, because these handlers intentionally call `preventDefault()`. It is a listener-ownership issue: a global behavior is being installed at component instance scope. - - Suggested action: if `globalDrop` remains part of the public API, move it behind a singleton subscription or an app-level drop manager so N prompt inputs do not imply N document listeners. - -## Positive Notes - -- `src/client/workspace/workspace-loader.ts:8-18` fetches project state and entity state in `Promise.all(...)`, which avoids a route-loader waterfall. -- `src/client/capabilities/markdown-rendering.tsx:54-95` and `src/client/capabilities/code-highlighting.ts:17-49` keep heavy markdown and Shiki code-highlighting logic behind dynamic imports and explicit preload helpers. -- `src/client/routes/debug-surface.tsx:3-10` lazily loads the debug surface instead of pulling it into the main route graph. - -## Watchlist - -- `lucide-react` is imported from 11 client files. The same barrel-import guidance applies there too, but the tradeoff is messier because deep icon imports can have TypeScript ergonomics issues depending on package export support. -- There are a few low-value memoizations that are not harmful but add ceremony, such as `src/client/components/ai-elements/shimmer.tsx:41` (`useMemo` around a primitive arithmetic result) and `src/client/workspace/workspace-data.ts:40` (`useMemo` around a tiny array key). I would not churn code just to remove them, but I would avoid adding more of this style. - -## Recommended Order Of Attack - -1. Replace the umbrella `radix-ui` imports with narrower package imports and measure the effect on dev startup/build output. -2. Remove the effect-driven chat reset by putting the chat controller behind a project-keyed ownership boundary. -3. Collapse route-loader/entity-query duplication so one layer owns entity state during route transitions. -4. Simplify `MessageBranch` so branch metadata is derived, not synchronized. -5. Only if `PromptInput` is expected to be reused in multiple places, centralize `globalDrop` listener ownership. diff --git a/docs/design/TYPESCRIPT_PRAXIS_REVIEW.md b/docs/design/TYPESCRIPT_PRAXIS_REVIEW.md deleted file mode 100644 index 0700e989..00000000 --- a/docs/design/TYPESCRIPT_PRAXIS_REVIEW.md +++ /dev/null @@ -1,115 +0,0 @@ -# TypeScript Praxis Review - -Date: 2026-04-09 - -## Scope - -Read-only review of the TypeScript codebase, guided by the `refs-typescript-praxis` skill. - -Primary focus areas: - -- `src/server` -- `src/shared` -- `src/client/workspace` -- `src/client/mutations` -- key route files that consume shared types - -Praxis categories applied: - -- Modeling Data Shapes -- Modules and API Surface -- Type System Sharp Edges -- Naming and Documentation - -## Executive Summary - -The biggest TypeScript problem in this codebase is not syntax-level style drift; it is boundary drift. Several client/server and JSON boundaries are typed by assertion rather than by an explicit shared contract plus runtime proof. That weakens the exact places where TypeScript should be buying the most confidence. - -After that, the main recurring issues are: - -1. optional-property shapes that blur distinct states -2. exported functions that rely on inference instead of explicit boundary signatures -3. snapshot/view-model types left mutable even though the code treats them as immutable -4. repeated inline `type` imports instead of top-level `import type` - -## Findings - -1. **Transport and API contracts are asserted rather than modeled** — category: model — impact: high - - The shared API layer is derived from server implementation signatures instead of from an explicit transport contract. `src/shared/api-types.ts:1-7` imports server functions and re-exports `ReturnType` aliases as the client-facing API model. That makes the implementation the schema: a server refactor can silently reshape the client contract even when the transport payload was supposed to stay stable. - - The same pattern continues at runtime boundaries. `src/client/workspace/workspace-loader.ts:8-15` and `src/client/mutations/client-mutation.ts:17-19,30-55` parse JSON and immediately cast the result with `as T` / `as MutationErrorResponse`. Those casts do not prove anything about the payload; they only suppress uncertainty. The result is a boundary that looks typed in editor tooling but is not actually defended. - - Suggested action: define explicit shared DTO types or Zod schemas at the transport boundary, validate there, and let the client consume those validated contracts instead of inferring them from server function return types. - -2. **Several domain and event shapes encode state through optional fields instead of explicit variants** — category: model — impact: medium - - The praxis guidance explicitly warns against using optional properties when the caller should make an explicit presence decision. This repo has a few recurring cases where omission stands in for a real state distinction. - - Examples: - - - `src/server/core.ts:26-35` defines `TurnWithOptions` with `options?`, even though `loadActivePathWithOptions()` always supplies `options`. - - `src/server/turn-response.ts:4-8` defines `ProjectedTurnResponse.freeText?: string`, which leaves absence ambiguous rather than explicit. - - `src/client/mutations/workspace-mutations.ts:19-24,31-54` uses optional `positions` and `freeText` fields to encode mutually meaningful response modes. - - `src/client/workspace/workspace-data.ts:15-20,65-71` accepts `{ type: string; data?: unknown }` for incoming data parts instead of a discriminated union keyed by `type`. - - These are all small individually, but together they create a “bag of optionals” style where consumers need defensive branching even when the real domain states are narrower. - - Suggested action: prefer discriminated unions for multi-mode payloads, and use `T | undefined` only when the caller should make presence explicit instead of relying on property omission. - -3. **Top-level exported functions still hide important module contracts behind inference** — category: modules — impact: medium - - The praxis guidance recommends explicit return types on top-level module functions so the boundary stays legible to both humans and tools. This repo still has several exported functions whose signatures stop at parameters and leave the return shape implicit. - - Representative examples: - - - `src/server/app.ts:36-233` — `createApp` - - `src/server/interview.ts:58-123` — `persistStructuredQuestion`, `createAskQuestionTool`, `createInterviewerAgent` - - `src/server/core.ts:63-68` — `getProjectState` - - `src/client/mutations/project-mutations.ts:6-25` — `useCreateProjectMutation` - - `src/client/workspace/workspace-controller-core.ts:263-269` — `findTurnOptionByPosition`, `findTurnOptionsByPositions` - - This is not a runtime bug by itself, but it makes module surfaces harder to scan and easier to widen accidentally during refactors. - - Suggested action: add explicit return types to exported non-component functions, especially factory functions, hooks, server entrypoints, and API helpers. - -4. **Immutable snapshots and view-models are modeled as mutable objects** — category: model — impact: medium - - The praxis guidance recommends `readonly` by default for object properties, especially when mutation is not part of the design. Many of the codebase’s snapshot and view-model types are computed once and then treated as immutable, but their types do not express that. - - Clear examples: - - - `src/client/workspace/workspace-controller-core.ts:14-61` — workspace durable state, pending-question view model, and controller view state - - `src/client/workspace/workspace-controller.ts:19-52` — controller state interfaces returned from the hook - - `src/shared/knowledge.ts:19-26` — `KnowledgeKindRegistryEntry` - - `src/client/workspace/workspace-loader.ts:3-6` — loader snapshot shape - - Leaving these mutable means accidental writes still type-check, even though the code treats these objects as read-mostly snapshots. - - Suggested action: add `readonly` to snapshot/view-model interfaces first, then widen only where mutation is actually intentional. - -5. **Type-only imports are inconsistent with the repo’s stated import hygiene** — category: modules — impact: low - - The praxis guidance prefers top-level `import type` over inline `import { type ... }` so the erased intent stays obvious across environments. I found this inline form repeated across the repo, for example: - - - `src/shared/chat.ts:1` - - `src/server/db.ts:2-12` - - `src/server/interview.ts:11` - - `src/client/routes/InterviewWorkspace.tsx:24` - - `src/client/workspace/workspace-controller.ts:3,9` - - This is the lowest-severity item in the review, but it is widespread enough to be worth normalizing. A quick search currently finds 17 inline `type` import sites under `src/`. - - Suggested action: switch these to top-level `import type` whenever the imported symbol is type-only. - -## Watchlist - -- `src/server/core.ts:38-56` and `src/server/app.ts:176-183` use a manual `throw`/`catch` path to model a missing project. Per the praxis throwing guidance, this is a good candidate for an explicit result type or a typed lookup outcome when that code is next touched. -- `src/server/db.ts` has many `as` assertions around Drizzle results. Some of that is library friction rather than local design failure, but it is still a place to push for narrower helper APIs so the assertions do not spread further. - -## Recommended Order Of Attack - -1. Replace implementation-derived API aliases with explicit shared transport contracts plus runtime validation. -2. Collapse optional-heavy request and event shapes into narrower unions or explicit `T | undefined` fields. -3. Add explicit return types to exported functions during the same pass. -4. Normalize `readonly` and `import type` opportunistically once the higher-risk boundary work is in flight. diff --git a/memory/REFACTOR.md b/memory/REFACTOR.md deleted file mode 100644 index 75f7c631..00000000 --- a/memory/REFACTOR.md +++ /dev/null @@ -1,74 +0,0 @@ -## Problem Statement - -The workspace boundary is carrying too much hidden synchronization work. - -Two user-visible flows still depend on post-render correction instead of clear ownership: - -- chat state is replaced after render when project identity changes -- workspace entity data is mirrored from route-loaded state into cached query state after render - -At the same time, the JSON transport boundary is too implicit: - -- client-facing types are derived from server implementation return types instead of an explicit transport contract -- loaders and mutations trust JSON payloads by assertion rather than runtime proof -- several request and review payloads still encode meaningful states through optional fields - -Together, this makes refactoring riskier than it needs to be. State ownership is harder to reason about, stale-frame behavior is harder to rule out, and server refactors can widen or reshape client contracts without that change becoming explicit. - -## Solution - -Refactor the workspace and transport seams around explicit contracts and single-owner state boundaries. - -Target state: - -- the REST/JSON boundary is defined by shared transport DTOs or schemas, not by server implementation inference -- client loaders and mutations parse validated transport payloads instead of casting JSON -- chat reset on project identity change happens through ownership/remount semantics rather than an effect that overwrites live state after render -- entity data has one authority per route transition, with loader/query coordination happening through an explicit boundary instead of effect-driven cache mirroring -- exported boundary functions touched by this work expose explicit signatures -- snapshot and view-model types touched by the refactor express immutability directly - -This keeps the refactor focused on the highest-risk overlap between the React review and the TypeScript review: boundary drift plus effect-driven synchronization in the workspace flow. - -## Commits - -1. [done] Add characterization coverage for the refactor seam: project navigation, same-project refresh, entity refresh after observer invalidation, and JSON-boundary success/failure cases. -2. [done] Introduce explicit shared transport contracts for project state, entity payloads, export payloads, mutation errors, and turn-response request/response payloads without changing runtime behavior yet. -3. [done] Move client loader and mutation helpers to parse transport payloads through those shared contracts and remove unchecked JSON casts from the workspace-facing routes and mutations. -4. [done] Narrow the turn-response and review payload shapes so meaningful request modes are represented explicitly instead of through bags of optional fields. -5. [done] Add explicit return types to the exported boundary functions touched by the transport pass so the module surfaces stay legible during later commits. -6. [done] Put the live chat owner behind a project-identity reset boundary and delete the post-render chat hydration overwrite. -7. [done] Collapse workspace entity hydration to one authority per route transition and remove the effect-based loader-to-cache mirroring step. -8. [done] Mark touched snapshot and view-model contracts as readonly and normalize type-only imports in the refactored boundary modules. - -## Progress - -All planned commits are landed and the full test suite passes. - -Temporary deferral: -- the client build-boundary test keeps its chunk-splitting assertions, but the minified entry-size ceiling is deferred until after the remaining workspace-state refactor work. Bundle-size tightening is not the current priority for this refactor pass. - -## Decisions - -- Recommended scope: one boundary-first refactor centered on workspace transport and state ownership, not a repo-wide hygiene sweep. -- Shared transport contracts become the authority for REST/JSON payload shape at the client/server seam. -- The live chat session owns only in-session state; route identity owns when seeded history resets. -- Entity state must have one clear owner per navigation event; cache refresh should be explicit, not mirrored after render. -- Optional-field cleanup is limited to payloads directly in this seam, not every optional-heavy type in the repo. -- Explicit return types and readonly modeling are included only for modules touched by this refactor, not as a global style pass. - -## Testing Decisions - -- Test behavior, not implementation: navigation should show the correct transcript without stale carryover; same-project refresh should preserve live chat while durable state updates; entity refresh should converge through one authority; malformed JSON should fail at the boundary with controlled errors. -- Use the existing workspace controller, workspace data, workspace loader, export loader, client mutation, and app-route tests as prior art. -- Prefer contract tests around transport parsing and route behavior over tests that assert internal hook/effect structure. -- Characterization tests are sufficient to begin; this area already has meaningful coverage, so the first commit can strengthen boundary-focused tests instead of blocking the refactor behind a separate testing-only project. - -## Out of Scope - -- Vendor-like UI source and AI-elements internals. -- A repo-wide explicit-return-type campaign. -- A repo-wide import-hygiene cleanup. -- Full replacement of every implementation-derived shared type in unrelated server/client areas. -- Drizzle assertion cleanup outside the touched transport seam. -- New workflow features or UX redesign beyond preserving current behavior while clarifying boundaries.