Repository navigation
Record review finding counts in Run metrics - #1144
Merged
Merged
Conversation
This was referenced Oct 23, 2026
Merged
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.
Summary
Review summaries preserve finding counts by Standards and Spec in each Session stage's Run metrics.
Counts include findings later fixed or declined. Missing or invalid summaries remain
null. Delivery stages referencing the same Session do not duplicate its counts; a Resume updates its originating review's summary once. Updates the Factory skill, shared Session prompt, generated prompts and site documentation.Closes #1035
Test seams
show --jsonexpose review counts, with absent or invalid summaries missing rather than zero.Evidence
cargo test --test run_metrics a_fake_review_keeps_its_finding_counts_in_the_runs_metrics -- --exactfailed:Nullinstead of{"standards":2,"spec":0}.After: the same test passes. The metrics integration file also covers missing/malformed/negative summaries, retention after deleting sidecars, and counts recorded once per Session.
cargo check,cargo test --test run_metrics, the fresh-prompt integration test,cargo test prompts_page, and formatting checks pass.cargo test --test review_summary_resume_repro -- --exact resuming_background_work_does_not_count_the_original_review_twice --nocapture. It failed with two copies before the fix and passes after it. Kept the test and extended it to cover a Resume updating its review counts.scripts/affected-tests issue-981: all 44 local site tests passed. Reused the successful full Rust CI run on exact head7e8807d, followingdocs/testing.md; stopped the duplicate local Rust run after CI passed. CI'scargo teststep took 11m 56s, including test compilation and excluding queue time. The same CI run passed formatting, clippy, Python tooling, site and Console browser checks.Unaddressed findings
Standards
None.
Spec
None. The Resume duplication finding was reproduced, fixed and retained as a regression test.
Security
None. Guidance-mode review of the issue-1035 diff against merge base 0767f7c found no introduced vulnerability: reviewer-written summary.json is parsed fallibly (
serde_json::from_slice(&bytes).ok()intoOption<u64>counts, malformed input stays missing rather than zero), file names and sidecar paths are literals in the same log directory (no traversal), counts feed only metrics display with no authority or sink, and Resume writes target the originating review's own sidecar. A throwaway probe feeding u64-max, wrong-type, HTML-bearing, and 5 MB garbage summaries through the real Run path completed cleanly with invalid input recorded as missing; existingrun_metrics(5 passed) andreview_summary_resume_repro(1 passed) suites pass.Security review outcome
No unaddressed introduced findings.
Review coverage
Independent Standards and Spec reviews used
issue-981(380e63a) as the fixed point, including prerequisite #1033. Standards: 0 findings. Spec: 1 finding, reproduced and fixed. Both axes inspected all 28 paths in that review diff, including generated output. Unread changed files: none. Generated-output exclusions: none.Merge Danger
Door: two-way
Blast Radius: metrics
A release carrying the Factory skill change needs an Experiment once #977 exists.
Built with codex · gpt-6.1-sol · xhigh