#136: Fix diagram view crash on undefined reviewers

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

@@ -729,6 +729,80 @@ describe('createFlowConfig', () => {
     });
   });
 
+  describe('V2 data with undefined reviewers', () => {
+    it('does not crash when reviewers is undefined and names come from iterations[].perReviewer', () => {
+      const iteration = Object.assign(
+        { reviewers: ['orchestrated-reviewer', 'aspect-test-reviewer'] },
+        {
+          perReviewer: {
+            'orchestrated-reviewer': { status: 'failed', criticality: 'medium' },
+            'aspect-test-reviewer': { status: 'completed', criticality: 'medium' },
+          },
+        },
+      );
+      const parallelReview: ParallelReviewPhase = {
+        aggregatedCriticality: 'low',
+        reviewRoundsUsed: 1,
+        coderFixCycleRan: false,
+        selectiveReReview: undefined,
+        iterations: [iteration],
+      };
+
+      const status = createMockRunStatus({
+        status: 'completed',
+        completedAt: '2026-01-01T01:00:00Z',
+        phases: { ...emptyPhases(), parallelReview },
+      });
+
+      const { nodes, edges } = createFlowConfig(status);
+      const reviewerNodes = nodes.filter((n) => n.id.startsWith('reviewer-'));
+
+      expect(reviewerNodes.length).toBe(2);
+      expect(reviewerNodes[0]?.data.status).toBe('idle');
+      expect(reviewerNodes[1]?.data.status).toBe('idle');
+
+      // Dispatch + return edges for each reviewer
+      const dispatchEdges = edges.filter((e) => e.id.startsWith('dispatch-reviewer-'));
+      const returnEdges = edges.filter((e) => e.id.startsWith('return-reviewer-'));
+      expect(dispatchEdges.length).toBe(2);
+      expect(returnEdges.length).toBe(2);
+    });
+
+    it('does not crash when reviewers is undefined and names come from reviewerDetails', () => {
+      const base: ParallelReviewPhase = {
+        aggregatedCriticality: 'low',
+        reviewRoundsUsed: 1,
+        coderFixCycleRan: false,
+        selectiveReReview: undefined,
+      };
+      const parallelReview = Object.assign(base, {
+        reviewerDetails: {
+          'code-reviewer': { status: 'completed', criticality: 'none' },
+          'test-reviewer': { status: 'completed', criticality: 'low' },
+        },
+      });
+
+      const status = createMockRunStatus({
+        status: 'completed',
+        completedAt: '2026-01-01T01:00:00Z',
+        phases: { ...emptyPhases(), parallelReview },
+      });
+
+      const { nodes, edges } = createFlowConfig(status);
+      const reviewerNodes = nodes.filter((n) => n.id.startsWith('reviewer-'));
+
+      expect(reviewerNodes.length).toBe(2);
+      expect(reviewerNodes[0]?.data.status).toBe('idle');
+      expect(reviewerNodes[1]?.data.status).toBe('idle');
+
+      // Dispatch + return edges for each reviewer
+      const dispatchEdges = edges.filter((e) => e.id.startsWith('dispatch-reviewer-'));
+      const returnEdges = edges.filter((e) => e.id.startsWith('return-reviewer-'));
+      expect(dispatchEdges.length).toBe(2);
+      expect(returnEdges.length).toBe(2);
+    });
+  });
+
   describe('reviewer dimming', () => {
     it('dims reviewers not in selectiveReReview.reviewersDispatched when ran is true', () => {
       const status = createMockRunStatus({

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

@@ -257,7 +257,7 @@ function buildReviewerNodes(phases: Phases): Node<FlowNodeData>[] {
   const reReviewSet = isReReviewActive ? new Set(selectiveReReview.reviewersDispatched) : undefined;
 
   for (const [i, name] of reviewerNames.entries()) {
-    const reviewerInfo = phases.parallelReview.reviewers[name];
+    const reviewerInfo = phases.parallelReview.reviewers?.[name];
     const isDimmed = reReviewSet !== undefined && !reReviewSet.has(name);
     nodes.push({
       id: `reviewer-${name}`,
@@ -283,7 +283,7 @@ function buildReviewerNodes(phases: Phases): Node<FlowNodeData>[] {
 }
 
 function resolveReviewerStatus(parallelReview: ParallelReviewPhase, reviewerName: string): FlowNodeData['status'] {
-  const reviewerInfo = parallelReview.reviewers[reviewerName];
+  const reviewerInfo = parallelReview.reviewers?.[reviewerName];
   if (reviewerInfo === undefined) return 'idle';
   if (reviewerInfo.status === 'completed') return 'completed';
   if (reviewerInfo.status === 'failed') return 'failed';
@@ -521,7 +521,7 @@ function buildReviewerEdges(phases: Phases): Array<Edge<DispatchEdgeData> | Edge
   const offsetMap = new Map<string, number>();
 
   for (const name of reviewerNames) {
-    const reviewerInfo = phases.parallelReview.reviewers[name];
+    const reviewerInfo = phases.parallelReview.reviewers?.[name];
     const isCompleted = reviewerInfo?.status === 'completed';
     const edgeStatus: DispatchEdgeData['status'] = isCompleted ? 'completed' : 'pending';
 
@@ -569,7 +569,7 @@ function buildReviewerEdges(phases: Phases): Array<Edge<DispatchEdgeData> | Edge
   const selectiveReReview = phases.parallelReview.selectiveReReview;
   if (selectiveReReview !== undefined && selectiveReReview.ran) {
     for (const name of selectiveReReview.reviewersDispatched) {
-      const reviewerInfo = phases.parallelReview.reviewers[name];
+      const reviewerInfo = phases.parallelReview.reviewers?.[name];
       const reReviewCompleted = reviewerInfo?.reReviewCriticality !== undefined;
       const reReviewStatus: DispatchEdgeData['status'] = reReviewCompleted ? 'completed' : 'pending';
 

packages/factory/src/client/game/state/agent-state-resolver.ts

@@ -30,7 +30,9 @@ const PHASE_STATUS_ACCESSORS: Record<PhaseName, PhaseStatusAccessor> = {
       if (phases.parallelReview.status !== undefined) {
         return phases.parallelReview.status;
       }
-      const hasRunningReviewer = Object.values(phases.parallelReview.reviewers).some((r) => r.status === undefined);
+      const hasRunningReviewer = Object.values(phases.parallelReview.reviewers ?? {}).some(
+        (r) => r.status === undefined,
+      );
       return hasRunningReviewer ? 'in_progress' : 'completed';
     }
     return phases.review?.status;

packages/factory/src/shared/__tests__/event-folder.test.ts

@@ -106,12 +106,12 @@ describe('foldEvents', () => {
 
     const result = foldEvents(header, events);
 
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']).toMatchObject({
       ran: true,
       status: undefined,
       criticality: undefined,
     });
-    expect(result.phases.parallelReview?.reviewers['test-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['test-reviewer']).toMatchObject({
       ran: true,
     });
   });
@@ -132,7 +132,7 @@ describe('foldEvents', () => {
 
     const result = foldEvents(header, events);
 
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']).toMatchObject({
       status: 'completed',
       criticality: 'low',
     });
@@ -185,8 +185,8 @@ describe('foldEvents', () => {
 
     const result = foldEvents(header, events);
 
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']?.reReviewCriticality).toBe('none');
-    expect(result.phases.parallelReview?.reviewers['test-reviewer']?.reReviewCriticality).toBe('low');
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']?.reReviewCriticality).toBe('none');
+    expect(result.phases.parallelReview?.reviewers?.['test-reviewer']?.reReviewCriticality).toBe('low');
   });
 
   it('appends artifacts on artifact_written', () => {

packages/run-core/src/__tests__/event-folder.test.ts

@@ -106,12 +106,12 @@ describe('foldEvents', () => {
 
     const result = foldEvents(header, events);
 
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']).toMatchObject({
       ran: true,
       status: undefined,
       criticality: undefined,
     });
-    expect(result.phases.parallelReview?.reviewers['test-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['test-reviewer']).toMatchObject({
       ran: true,
     });
   });
@@ -132,7 +132,7 @@ describe('foldEvents', () => {
 
     const result = foldEvents(header, events);
 
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']).toMatchObject({
       status: 'completed',
       criticality: 'low',
     });
@@ -185,8 +185,8 @@ describe('foldEvents', () => {
 
     const result = foldEvents(header, events);
 
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']?.reReviewCriticality).toBe('none');
-    expect(result.phases.parallelReview?.reviewers['test-reviewer']?.reReviewCriticality).toBe('low');
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']?.reReviewCriticality).toBe('none');
+    expect(result.phases.parallelReview?.reviewers?.['test-reviewer']?.reReviewCriticality).toBe('low');
   });
 
   it('appends artifacts on artifact_written', () => {
@@ -644,9 +644,9 @@ describe('foldEvents', () => {
     const result = foldEvents(header, events);
 
     // Known reviewer is updated
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']?.reReviewCriticality).toBe('low');
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']?.reReviewCriticality).toBe('low');
     // Unknown reviewer is not added to the reviewers map
-    expect(result.phases.parallelReview?.reviewers['unknown-reviewer']).toBeUndefined();
+    expect(result.phases.parallelReview?.reviewers?.['unknown-reviewer']).toBeUndefined();
   });
 
   // -- Full event sequence integration test (F1) ----------------------------------------
@@ -834,13 +834,13 @@ describe('foldEvents', () => {
     expect(result.phases.parallelReview?.aggregatedCriticality).toBe('none');
     expect(result.phases.parallelReview?.reviewRoundsUsed).toBe(2);
     expect(result.phases.parallelReview?.coderFixCycleRan).toBe(true);
-    expect(result.phases.parallelReview?.reviewers['code-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['code-reviewer']).toMatchObject({
       ran: true,
       status: 'completed',
       criticality: 'low',
       reReviewCriticality: 'none',
     });
-    expect(result.phases.parallelReview?.reviewers['test-reviewer']).toMatchObject({
+    expect(result.phases.parallelReview?.reviewers?.['test-reviewer']).toMatchObject({
       ran: true,
       status: 'completed',
       criticality: 'medium',

packages/run-core/src/event-folder.ts

@@ -167,6 +167,7 @@ function applyReviewerDispatched(
   review: ParallelReviewPhase,
   event: Extract<ReviewEvent, { event: 'reviewer_dispatched' }>,
 ): void {
+  review.reviewers ??= {};
   review.reviewers[event.reviewer] = {
     ran: true,
     status: undefined,
@@ -186,7 +187,7 @@ function applyReviewerCompleted(
   review: ParallelReviewPhase,
   event: Extract<ReviewEvent, { event: 'reviewer_completed' }>,
 ): void {
-  const entry = review.reviewers[event.reviewer];
+  const entry = review.reviewers?.[event.reviewer];
   if (entry) {
     entry.status = event.status;
     entry.criticality = event.criticality;
@@ -216,7 +217,7 @@ function applyReReviewCompleted(
   event: Extract<ReviewEvent, { event: 're_review_completed' }>,
 ): void {
   for (const [reviewer, crit] of Object.entries(event.criticalities)) {
-    const entry = review.reviewers[reviewer];
+    const entry = review.reviewers?.[reviewer];
     if (entry) {
       entry.reReviewCriticality = crit;
     }

packages/run-core/src/types/canonical.ts

@@ -93,7 +93,7 @@ export interface ReviewIteration {
 export interface ParallelReviewPhase {
   aggregatedCriticality: Criticality | undefined;
   reviewRoundsUsed: number;
-  reviewers: Record<string, ReviewerInfo>;
+  reviewers?: Record<string, ReviewerInfo>;
   coderFixCycleRan: boolean;
   selectiveReReview: SelectiveReReview | undefined;
   status?: PhaseStatus;