feature: surface aged, suppressed and override counts in the run summary - #12
Open
addyCooks wants to merge 1 commit into
Open
feature: surface aged, suppressed and override counts in the run summary#12addyCooks wants to merge 1 commit into
addyCooks wants to merge 1 commit into
Conversation
…n summary ReconcileResult already carried incremented, suppressed and suppressedByOverride, but the CLI and the persisted run record dropped them, so pack authors calibrating suppressions could not tell whether their markers were silencing the right findings. The per-repo live-run line now reads: myorg/myrepo: filed 3, touched 5, aged 2, suppressed 1, suppressed-by-override 0, resolved 0 FilingSummary carries the same three numbers, so the run record committed to the config repo shows the full picture in the dashboard. Formatting moves into renderFilingLine so the line is covered by tests rather than living untested in cli.ts.
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.
Closes #11
Description
ReconcileResultalready carriedincremented,suppressed, andsuppressedByOverride, but the CLI and the persisted run record dropped themso a pack author calibrating suppressions could not tell whether their
sentinel:false-positivemarkers were silencing the right findings, or whethersuppression was over-firing.
The per-repo live-run line. Before:
After:
Default-on and always printed, including zeros per the issue, a zero is the
signal that a layer is not firing, so hiding it hides the calibration answer.
incrementedsurfaces asaged, which reads better besideresolved.The run record.
FilingSummarycarries the same three numbers, summedacross repos in the loop
buildRunRecordalready runs.The dashboard. The issue asks for the record to show the full picture in
the static dashboard, and
dashboard.tsrendered onlyfiledso thecounters were persisted but invisible to the org admin triaging a scheduled run.
Added six columns (
Filed · Touched · Aged · Suppressed · By override · Resolved), driven off oneFILING_COLUMNStable so the header and the rowcannot drift apart. Non-live runs dash out every filing column, as
filedalready did.
Formatting moved out of
cli.tsintorenderFilingLineinsource/run/report.ts.cli.tshas no spec and is excluded from coverage, sothe line was otherwise untestable; this puts the format itself under regression
cover.
No new dependencies, no new files, no hosted surface.
Type of change
Testing
Verified end-to-end by driving one real
reconcileFindingscall into allthree surfaces, so the CLI line, the record, and the dashboard cannot silently
disagree:
Each counter is reached via a distinct real path in that fixture: 3 new
findings, 1 hash match, 1 aged
misses 0→1, 1sentinel:false-positiveclose,1
vendor/**per-repo override, and 1 resolved atmisses 2→3againstresolveAfterMisses: 3.That fixture also surfaced an edge case worth guarding: a below-threshold
finding must not inflate
suppressed. It is filtered before planning so itcounts in neither suppression bucket, but nothing asserted it and a regression
there would directly mislead the pack author this feature exists to serve. Added
to the existing threshold test.
11 tests added (253 → 264), 2 rewritten. Edge cases covered:
the exact format string from the issue
all-zero rendering, proving default-on rather than omit-when-zero
cross-repo summation with asymmetric values, so a mis-wired field cannot pass
a live run that reconciled nothing
filedcountingcreated.lengthrather than a scalarthe audited repo vs
targetRepounderaggregate_to_config_repotolerated errors staying off the line
dashboard dash-out on a dry-run, and
0rendering as0(notor·)a row-cells-equals-column-headings invariant that catches a broken table
pnpm test:allpasses locally.Added or updated tests for the change.
Manual testing notes: run on a clean LF checkout 273 tests pass, coverage
96.23% lines (
dashboard.ts100%,record.ts100%,report.ts99.05%).Format, types, lint, knip and audit all pass. Semgrep was not installed
locally, so
test.shskipped that step; it will run in CI.Checklist
sentinel.yamlschema, rule-pack manifest, findingsmodel) if changed, the docs and the relevant spec are updated in this PR.
Notes for review
FilingSummarygains three required fields. It is exported fromsource/index.ts, so anyone constructing one by hand against the library APImust add them. Records already written stay readable the dashboard renders
a missing counter the same way it renders a dry-run. Flagging it as the one
contract-ish surface this touches; happy to make the fields optional if you
would rather it be additive-only.
docs/findings/index.md,since the three layers documented there map exactly onto
touched/suppressed/suppressed-by-override, which is the calibration emphasisthe issue points at.
I read the proposed solution as asking for them ("shows the full picture in
the static dashboard"). Happy to split them into a follow-up if you would
rather keep this PR to the CLI and the record.