Visualization crashes on switch to diagram view

[williamthorsen]

Toggling a working factory view can trigger at least two different bugs.

1. Displays diagram in the visualization container; error in console

Run codeassembly/87:

• diagram displays
• error is seen in console:

run-to-flow.ts:260 Uncaught TypeError: Cannot read properties of undefined (reading 'orchestrated-reviewer')
    at buildReviewerNodes (run-to-flow.ts:260:48)
    at createFlowConfig (run-to-flow.ts:666:25)
    at FlowDiagram.tsx:61:32
    at FlowDiagramInner (FlowDiagram.tsx:61:18)

2. Displays nothing in the visualization container; error in console

Example: codeassembly/CODY-113

run-to-flow.ts:260 Uncaught TypeError: Cannot read properties of undefined (reading 'orchestrated-reviewer')
    at buildReviewerNodes (run-to-flow.ts:260:48)
    at createFlowConfig (run-to-flow.ts:666:25)
    at FlowDiagram.tsx:61:32
    at FlowDiagramInner (FlowDiagram.tsx:61:18)

Root cause

extractReviewerNames() in run-to-flow.ts correctly handles three data shapes for reviewer names (flat reviewers record, iterations[].perReviewer, and reviewerDetails). However, four downstream access sites assume parallelReview.reviewers is always a defined object. For V2 run data, the Zod .partial().loose() schema doesn't enforce the reviewers field, so it can be undefined at runtime despite the TypeScript type declaring it as required.

Affected access sites (all in run-to-flow.ts):

• Line 260 in buildReviewerNodes — primary crash location
• Line 286 in resolveReviewerStatus
• Line 524 in buildReviewerEdges
• Line 572 in buildReviewerEdges (re-review section)

Proposed solution

Add optional chaining (?.) at all 4 access sites. All downstream uses of the resulting reviewerInfo already handle undefined via optional chaining or explicit checks.

Acceptance criteria

• No crash when switching to diagram view for runs with V2 data where parallelReview.reviewers is absent
• Reviewer nodes render gracefully (with idle status) when reviewer info is unavailable
• Regression tests added: exercises both iterations[].perReviewer and reviewerDetails paths with reviewers undefined

[williamthorsen]

Implementation plan

Changes

1. Add optional chaining at 4 access sites

File: packages/factory/src/client/components/FlowDiagram/mappers/run-to-flow.ts

Line	Current	Fix
260	phases.parallelReview.reviewers[name]	phases.parallelReview.reviewers?.[name]
286	parallelReview.reviewers[reviewerName]	parallelReview.reviewers?.[reviewerName]
524	phases.parallelReview.reviewers[name]	phases.parallelReview.reviewers?.[name]
572	phases.parallelReview.reviewers[name]	phases.parallelReview.reviewers?.[name]

All downstream uses of the resulting reviewerInfo already handle undefined via optional chaining or explicit checks:

• Lines 273-276: reviewerInfo?.criticality, reviewerInfo?.reReviewCriticality
• Line 287: if (reviewerInfo === undefined) return 'idle'
• Lines 525, 559-560, 573, 605: all use optional chaining

2. Add regression tests

File: packages/factory/src/client/components/FlowDiagram/mappers/__tests__/run-to-flow.test.ts

Add a describe('V2 data with undefined reviewers') block after the existing 'reviewer extraction from alternate data shapes' block (after line 730):

• Test A: reviewers is undefined with names from iterations[].perReviewer — uses as unknown as ParallelReviewPhase cast to simulate V2 runtime data; verifies no crash, 2 reviewer nodes with status: 'idle', and 2 dispatch + 2 return edges
• Test B: reviewers is undefined with names from reviewerDetails — same pattern; verifies no crash, 2 reviewer nodes, and edges

The as unknown as ParallelReviewPhase cast is intentional: it simulates the runtime mismatch where TypeScript's type is a lie due to loose Zod parsing.

Verification

# Run the specific test file
pnpm vitest run packages/factory/src/client/components/FlowDiagram/mappers/__tests__/run-to-flow.test.ts

# Run full factory tests
pnpm vitest run --project factory

# Typecheck
pnpm run typecheck

[williamthorsen]

Closed by #138