fix(review): distinguish carried findings in severity gate (#34)

* fix(review): distinguish carried findings in severity gate

* fix(review): guard provenance counters

Author: 777genius

__tests__/unit/core/orchestrator.health.test.ts

@@ -293,6 +293,10 @@ describe('ReviewOrchestrator health check guard rails', () => {
     expect(incrementalReviewer.saveReview).not.toHaveBeenCalled();
     expect(review.summary).toBe('Previous completed review');
     expect(review.metrics.cached).toBe(true);
+    expect(review.findingProvenance).toEqual({
+      fromCurrentReview: { critical: 0, major: 0, minor: 0 },
+      carriedForward: { critical: 0, major: 1, minor: 0 },
+    });
     expect(commentPoster.postSummary).toHaveBeenCalledWith(
       1,
       'Cached review',
@@ -301,6 +305,155 @@ describe('ReviewOrchestrator health check guard rails', () => {
     );
   });
 
+  it('marks unchanged-file findings as carried forward in a delta review', async () => {
+    const carriedFinding: Finding = {
+      file: 'src/unchanged.ts',
+      line: 7,
+      severity: 'major',
+      title: 'Existing finding',
+      message: 'Still applies to an unchanged file.',
+    };
+    const incrementalReviewer = {
+      planReview: jest.fn().mockResolvedValue({
+        mode: IncrementalReviewPlanMode.Delta,
+        files: [],
+        invalidatedPaths: [],
+        lastReview: {
+          prNumber: 1,
+          lastReviewedCommit: 'previous-head',
+          baseSha: 'b',
+          timestamp: Date.now(),
+          findings: [carriedFinding],
+          reviewSummary: 'Previous review',
+        },
+      }),
+      mergeFindings: jest.fn().mockReturnValue([carriedFinding]),
+      generateIncrementalSummary: jest.fn().mockReturnValue('Delta review'),
+      saveReview: jest.fn(),
+    } as any;
+    const orchestrator = makeOrchestrator({
+      config: {
+        ...DEFAULT_CONFIG,
+        dryRun: true,
+        enableCaching: false,
+        analyticsEnabled: false,
+        graphEnabled: false,
+        skipTrivialChanges: false,
+        incrementalEnabled: true,
+        providers: [],
+        fallbackProviders: [],
+      },
+      incrementalReviewer,
+    });
+
+    const review = await orchestrator.executeReview(
+      makePR([
+        {
+          filename: 'docs/update.md',
+          status: 'modified',
+          additions: 1,
+          deletions: 0,
+          changes: 1,
+        },
+      ])
+    );
+
+    expect(review.findingProvenance).toEqual({
+      fromCurrentReview: { critical: 0, major: 0, minor: 0 },
+      carriedForward: { critical: 0, major: 1, minor: 0 },
+    });
+    expect(incrementalReviewer.mergeFindings).toHaveBeenCalledWith(
+      [carriedFinding],
+      [],
+      [],
+      []
+    );
+  });
+
+  it('preserves finding provenance when an incremental adapter clones findings', async () => {
+    const changedFile: FileChange = {
+      filename: 'src/changed.ts',
+      status: 'modified',
+      additions: 1,
+      deletions: 0,
+      changes: 1,
+    };
+    const carriedFinding: Finding = {
+      file: 'src/unchanged.ts',
+      line: 7,
+      severity: 'critical',
+      title: 'Existing finding',
+      message: 'Still applies to an unchanged file.',
+    };
+    const currentFinding: Finding = {
+      file: changedFile.filename,
+      line: 1,
+      severity: 'major',
+      title: 'Current finding',
+      message: 'Produced while reviewing the changed file.',
+    };
+    const incrementalReviewer = {
+      planReview: jest.fn().mockResolvedValue({
+        mode: IncrementalReviewPlanMode.Delta,
+        files: [changedFile],
+        invalidatedPaths: [],
+        lastReview: {
+          prNumber: 1,
+          lastReviewedCommit: 'previous-head',
+          baseSha: 'b',
+          timestamp: Date.now(),
+          findings: [carriedFinding],
+          reviewSummary: 'Previous review',
+        },
+      }),
+      mergeFindings: jest
+        .fn()
+        .mockImplementation((previous: Finding[], current: Finding[]) =>
+          JSON.parse(JSON.stringify([...previous, ...current]))
+        ),
+      generateIncrementalSummary: jest.fn().mockReturnValue('Delta review'),
+      saveReview: jest.fn(),
+    } as any;
+    const orchestrator = makeOrchestrator({
+      config: {
+        ...DEFAULT_CONFIG,
+        dryRun: true,
+        enableCaching: false,
+        analyticsEnabled: false,
+        graphEnabled: false,
+        skipTrivialChanges: false,
+        incrementalEnabled: true,
+        providers: [],
+        fallbackProviders: [],
+      },
+      incrementalReviewer,
+      providerRegistry: {
+        createProviders: jest.fn().mockResolvedValue([]),
+        discoverAdditionalFreeProviders: jest.fn().mockResolvedValue([]),
+      } as any,
+      llmExecutor: {
+        filterHealthyProviders: jest.fn().mockResolvedValue({
+          healthy: [],
+          healthCheckResults: [],
+        }),
+        execute: jest.fn(),
+      } as any,
+      synthesis: {
+        synthesize: jest.fn().mockReturnValue({
+          ...emptyReview,
+          findings: [currentFinding],
+        }),
+      } as any,
+    });
+
+    const review = await orchestrator.executeReview(makePR([changedFile]));
+
+    expect(review.findingProvenance).toEqual({
+      fromCurrentReview: { critical: 0, major: 1, minor: 0 },
+      carriedForward: { critical: 1, major: 0, minor: 0 },
+    });
+  });
+
   it('detects a trivial PR before building its code graph', async () => {
     const graphBuilder = {
       buildGraph: jest.fn(),

__tests__/unit/output/severity-gate.test.ts

@@ -6,6 +6,7 @@ import {
   Finding,
   LifecycleThreadRecord,
   Review,
+  ReviewFindingProvenance,
   Severity,
 } from '../../../src/types';
 
@@ -30,10 +31,12 @@ const previousThread = (
 
 const review = (
   findings: Finding[],
-  previousStillValid: LifecycleThreadRecord[] = []
+  previousStillValid: LifecycleThreadRecord[] = [],
+  findingProvenance?: ReviewFindingProvenance
 ): Review =>
   ({
     findings,
+    findingProvenance,
     threadLifecycle: {
       previousStillValid,
     },
@@ -47,7 +50,7 @@ describe('severity gate formatting', () => {
     );
 
     expect(result).toBe(
-      'ReviewRouter found 1 blocking major+ finding: 1 new current major+ finding. Review comments were posted before failing this check.'
+      'ReviewRouter found 1 blocking major+ finding: 1 major+ finding produced by this review. Review comments were posted before failing this check.'
     );
   });
 
@@ -56,11 +59,37 @@ describe('severity gate formatting', () => {
 
     expect(getBlockingFindingBreakdown(input, 'major')).toEqual({
       current: 0,
+      fromCurrentReview: 0,
+      carriedForward: 0,
+      unclassifiedCurrent: 0,
       previousStillValid: 1,
       total: 1,
     });
     expect(formatBlockingFindingFailure(input, 'major')).toBe(
-      'ReviewRouter found 1 blocking major+ finding: 1 previous unresolved major+ finding still valid. No new current major+ findings were kept after filtering. Review comments were posted before failing this check.'
+      'ReviewRouter found 1 blocking major+ finding: 1 previous unresolved major+ finding still valid. No major+ findings were produced by this review. Review comments were posted before failing this check.'
+    );
+  });
+
+  it('reports carried-forward findings without calling them new', () => {
+    const input = review(
+      [finding('major'), finding('major'), finding('major')],
+      [],
+      {
+        fromCurrentReview: { critical: 0, major: 0, minor: 0 },
+        carriedForward: { critical: 0, major: 3, minor: 0 },
+      }
+    );
+
+    expect(getBlockingFindingBreakdown(input, 'major')).toEqual({
+      current: 3,
+      fromCurrentReview: 0,
+      carriedForward: 3,
+      unclassifiedCurrent: 0,
+      previousStillValid: 0,
+      total: 3,
+    });
+    expect(formatBlockingFindingFailure(input, 'major')).toBe(
+      'ReviewRouter found 3 blocking major+ findings: 3 carried-forward major+ findings from unchanged files. No major+ findings were produced by this review. Review comments were posted before failing this check.'
     );
   });
 
@@ -71,7 +100,7 @@ describe('severity gate formatting', () => {
     );
 
     expect(result).toBe(
-      'ReviewRouter found 2 blocking major+ findings: 1 new current major+ finding and 1 previous unresolved major+ finding still valid. Review comments were posted before failing this check.'
+      'ReviewRouter found 2 blocking major+ findings: 1 major+ finding produced by this review and 1 previous unresolved major+ finding still valid. Review comments were posted before failing this check.'
     );
   });
 
@@ -83,6 +112,9 @@ describe('severity gate formatting', () => {
 
     expect(getBlockingFindingBreakdown(input, 'major')).toEqual({
       current: 1,
+      fromCurrentReview: 1,
+      carriedForward: 0,
+      unclassifiedCurrent: 0,
       previousStillValid: 0,
       total: 1,
     });