feat(fp-judge): domain contracts + rubric + FP-control + overlay consumer - #31
Conversation
…umer Phase-0/1 domain side of the FP-judge. 007's private `o7 judge` triages own-check's residual findings (real leak / false positive / uncertain); this repo owns the rubric, the finding identity, and consuming the overlay. Contract across the seam = the overlay file + verdict-contract.md, nothing else. - docs/fp-judge/verdict-contract.md — finding_id = sha1(path\x1f rule\x1f message)[:16] (line-independent; identical patterns share an id), fp-verdicts.json fields, and generated_from = sha256 of the raw findings.json bytes (staleness key). - docs/fp-judge/rubric.md — subscription / idisposable / region-escape criteria for real|false_positive|uncertain; the rebind-setter case is pinned to `uncertain`. - docs/fp-judge/STATUS.md — resume note: Phase-1 gate PASSED both directions; next = the real STS run, then merge + dashboard. - oracle/fixtures/findings-fp-control.json — adversarial FP control: own-check-shaped findings aimed at the Fixed*ViewModel counterparts (which detach/dispose), so the judge must return false_positive. Closes the false_positive direction of the gate (leaks-only oracle proved only the `real` direction). - viz/apply_verdicts.py — overlay consumer: verify generated_from vs the current findings.json (refuse a stale overlay), join verdicts by finding_id, split into real / uncertain / unjudged / judged-FP (confident FP retired, counted not hidden). --selftest covers the staleness guard + finding_id + classify + merge. - viz/build_dashboard.py — read any audit-output folder via a positional DATA_DIR arg (default: artifacts); no hardcode on sts_audit/artifacts. Verified: apply_verdicts --selftest OK; live guard correctly rejects the oracle overlay against the STS findings.json (digest mismatch); dashboard builds from artifacts/ (210). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds FP-judge domain documentation (rubric, verdict-contract, status notes), a new ChangesFP-judge overlay contract and verdict application
Dashboard artifacts source rewiring
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20ee3b6e4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ap.add_argument("data_dir", nargs="?", default="artifacts", | ||
| help="folder with findings.json + health-report.md (default: artifacts). " | ||
| "A relative path resolves from the repo root.") |
There was a problem hiding this comment.
Use the triaged findings for dashboard builds
When following the new FP workflow (apply_verdicts --out artifacts/findings-triaged.json then build_dashboard.py), this argument only accepts an audit directory and collect() still opens <DATA_DIR>/findings.json, so the dashboard never reads the merged finding_id/triage_class output. In that context, confident judged_fp rows remain in every KPI/chart/table as raw findings, defeating the overlay’s de-noising; prefer/accept the triaged file or make apply_verdicts update the file this path reads before rendering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accurate observation, but intentionally out of scope for this PR. build_dashboard.py's change here is only the positional DATA_DIR (read any audit-output folder); wiring the triaged overlay into the dashboard is explicitly deferred — see the PR description's "Next (not in this PR)": apply_verdicts (guard now passes) + dashboard.
Worth noting the fix is also bigger than swapping the input filename: collect() doesn't read triage_class at all, so pointing it at findings-triaged.json alone wouldn't de-noise anything — the dashboard needs genuine triage-aware filtering (drop/flag judged_fp), which is the feature that follow-up will add. Leaving this thread open as a tracked reminder for that PR.
Generated by Claude Code
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@viz/apply_verdicts.py`:
- Around line 126-129: The merged summary in apply_verdicts.run() is dropping
run_id from the original document, so preserve it when building merged alongside
the existing verdict_summary and findings. Update the merged dictionary
construction to carry fdoc’s run_id through unchanged (along with the other
summary fields) so downstream consumers can use it in the report footer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 206c559b-e676-46cb-80e1-6cc5b3fe943a
📒 Files selected for processing (8)
.gitignoredocs/fp-judge/STATUS.mddocs/fp-judge/rubric.mddocs/fp-judge/verdict-contract.mdoracle/fixtures/findings-fp-control.jsonviz/apply_verdicts.pyviz/build_dashboard.pyviz/history.jsonl
The verdict-contract (§2) specifies run_id as an fp-verdicts.json envelope field that the domain carries through into the report footer. run() forwarded model and generated_from but dropped run_id, losing judge-run provenance for downstream consumers. Pull it from the overlay alongside the other provenance fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFSpD9yXgXePrFFX62XZ53
What & why
Domain (OwnAudit) side of the FP-judge — the layer that triages own-check's residual
findings into real leak / false positive / uncertain. The judging harness itself is
007's private
o7 judge(claude -p, read-only, whole-file); this PR is everything thedomain owns: the rubric, the finding identity, the contract, and the
consumer that merges the verdict overlay back into the audit — with a staleness guard
so an overlay judged against an old audit can never render as current truth.
Follows the delegate-normalization fix (Own.NET#163) that already cut subscription
findings 323→156; this makes the remaining 156 triageable.
Contents
docs/fp-judge/verdict-contract.mdfinding_id(line-independent sha1 of path+rule+message),fp-verdicts.jsonfields,generated_from= sha256(findings bytes)docs/fp-judge/rubric.mduncertaindocs/fp-judge/STATUS.mdoracle/fixtures/findings-fp-control.jsonFixed*ViewModelcounterparts → must judgefalse_positiveviz/apply_verdicts.pyfinding_id+ triage split;--selftestviz/build_dashboard.pyDATA_DIR(defaultartifacts)Phase-1 gate — PASSED both directions
real, grounded reasoning.findings-fp-control.json(fixed counterparts) → bothfalse_positive.A judge that only ever says
realwould have failed this.How verified
python viz/apply_verdicts.py --selftest— guard (digest match→merge, mismatch→reject) + finding_id + classify + merge.findings.json(generated_from ≠ STS-210 digest) — the staleness guard working.python viz/build_dashboard.py [artifacts|<dir>]builds; bad dir errors cleanly.Next (not in this PR)
The real STS run (
o7 judge --repo <SectorTS root> --findings artifacts/findings.json …),then
apply_verdicts(guard now passes) + dashboard. Seedocs/fp-judge/STATUS.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores