Skip to content

Record review finding counts in Run metrics - #1144

Merged
JacobStephens2 merged 2 commits into
issue-981from
issue-1035
Oct 23, 2026
Merged

JacobStephens2 merged 2 commits into
issue-981from
issue-1035

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

Review summaries preserve finding counts by Standards and Spec in each Session stage's Run metrics.

review → summary.json → Session sidecar → Command record → show --json

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

  • Whole binary against fake Harnesses: Run completion and show --json expose review counts, with absent or invalid summaries missing rather than zero.
  • Existing prompt-generation check: the Session prompt and embedded Factory skill remain reflected in generated pages.

Evidence

  • Before: cargo test --test run_metrics a_fake_review_keeps_its_finding_counts_in_the_runs_metrics -- --exact failed: Null instead 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.
  • Spec review regression: ran the reviewer's exact command, 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 head 7e8807d, following docs/testing.md; stopped the duplicate local Rust run after CI passed. CI's cargo test step 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() into Option<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; existing run_metrics (5 passed) and review_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

@JacobStephens2
JacobStephens2 merged commit 371e40a into issue-981 Oct 23, 2026
18 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-1035 branch October 23, 2026 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant