feat(scanner): add new/resolved finding deltas to scan:completed#2367
Open
dividedmind wants to merge 1 commit into
Open
feat(scanner): add new/resolved finding deltas to scan:completed#2367dividedmind wants to merge 1 commit into
dividedmind wants to merge 1 commit into
Conversation
Diff each rescanned AppMap's findings against its previous appmap-findings.json report (by stable hash_v2) to find findings newly introduced or resolved since the last scan. Aggregate over the batch window and add numNewFindings / numResolvedFindings metrics, plus newFindingsByRule / newFindingsByImpactDomain breakdowns for new findings. Reuses the per-AppMap report files as prior state -- no new storage and no in-memory tracking, so it survives restarts. Deleting an AppMap is deliberately not treated as resolution. Assisted-by: Claude:claude-opus-4-8
Contributor
There was a problem hiding this comment.
Pull request overview
Adds “new vs resolved findings” delta metrics to the scan:completed telemetry event by diffing each rescanned AppMap’s current findings against the findings persisted in its prior appmap-findings.json report (keyed by hash_v2), then aggregating over the batch window.
Changes:
- Extend
scan:completedtelemetry payload withnumNewFindings/numResolvedFindingsand breakdowns for new findings by rule and impact domain. - Introduce
diffFindings(prior, current)to compute new/resolved findings by stablehash_v2. - Update watch-scan flow to read prior report findings, compute per-scan diffs, and include them in aggregated telemetry; add/adjust tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/scanner/test/report/scanResults.spec.ts | Updates expectations for new delta telemetry fields on scan:completed. |
| packages/scanner/test/report/findingsDiff.spec.ts | Adds unit tests for diffFindings behavior (new/resolved, dedupe, no prior). |
| packages/scanner/test/cli/scan/watchScanTelemetry.spec.ts | Updates watch-scan telemetry aggregation test to include diff-driven delta metrics. |
| packages/scanner/src/report/scanResults.ts | Extends ScanTelemetry, adds new event properties/metrics, exports countFindings. |
| packages/scanner/src/report/findingsDiff.ts | Adds diffFindings implementation keyed by hash_v2. |
| packages/scanner/src/cli/scan/watchScanTelemetry.ts | Aggregates per-scan diffs into delta telemetry counts/breakdowns. |
| packages/scanner/src/cli/scan/watchScan.ts | Reads prior report findings, diffs against current scan findings, emits diff with scan event. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+11
| * Compare a prior set of findings to the current set, identifying findings by their | ||
| * stable `hash_v2`. Both sides are deduped by hash so repeated findings don't skew the | ||
| * result. With no prior findings, everything is new. |
Comment on lines
+219
to
+220
| const priorFindings = reportStats ? await readPriorFindings(reportFile) : []; | ||
| const diff = diffFindings(priorFindings, rawScanResults.findings); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Diff each rescanned AppMap's findings against its previous appmap-findings.json report (by stable hash_v2) to find findings newly introduced or resolved since the last scan. Aggregate over the batch window and add numNewFindings / numResolvedFindings metrics, plus newFindingsByRule / newFindingsByImpactDomain breakdowns for new findings.
Reuses the per-AppMap report files as prior state -- no new storage and no in-memory tracking, so it survives restarts. Deleting an AppMap is deliberately not treated as resolution.