feat(guardian): structured findings + benchmark harness + baseline#153
Conversation
…tation plan Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… extract_json Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…llback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add optional base_ref to ContextCollector for exact-SHA diff ranges needed by benchmark replays; create scripts/guardian_bench.py to replay past PRs against curated ground truth and append scored JSONL entries. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
39 GT findings + 7 ambiguous entries across PRs 122/140/141/142/143/144, line coords verified at each PR's review head (parent of the first review-fix commit — NOT the final pull head, which already contains the fixes; spec §3.1 erratum). PR 143 uses its second-round review head: four of six defects were introduced by feature commits between review waves. CURATION.md documents per-PR provenance and the curation policy. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…imum Caught by the first live bench run: google-genai's Schema type forbids exclusiveMinimum (produced by gt=0), failing every gemini call before the request was sent. ge=1 emits minimum, which gemini accepts, and is semantically identical for ints. Regression test pins the JSON schema. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CI builds graph.db with `cgis ingest ./src`, so node ids are relative to src/ (cgis.query.engine), but the collector looked up repo-relative FQNs (src.cgis.query.engine) — every lookup missed and guardian has silently run with zero graph context. Found by the bench replay's "No graph context found" warning. New source_root param (default "src") must match the ingest root. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tring Final-review findings: source_root="./src" (the literal form CI uses for ingest) silently failed to strip, reproducing the zero-graph-context bug for that input form; and a test docstring still referenced the removed gt=0 constraint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
13 gemini-2.5-flash runs (every PR covered; 5 runs short of 3x due to free-tier 20 req/day cap) and 2 mistral-medium runs (PR 122 only; plan quota exhausted). Error lines kept in the log per spec §7. Top-up runs to follow once quotas reset. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces structured findings JSON as the core contract for the Guardian reviewer, alongside a benchmark harness for replaying and scoring past PRs against hand-curated ground truth. It defines Pydantic models for findings, implements native JSON generation for Gemini and Mistral providers, adds retry/fallback parsing logic, and provides a markdown renderer for PR comments. The review feedback correctly identifies an issue in the benchmark precision calculation, where including ambiguous hits in the denominator unfairly penalizes the model, and suggests a standard precision formula of TP / (TP + FP) to resolve this.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Review polish for PR #153: - precision = TP/(TP+FP) per gemini review — ambiguous hits are neither correct nor noise (spec §3.1) and must not depress precision; spec §3.3 updated, results.jsonl precision fields recomputed from raw counts - guardian_bench.py: blocking ingest subprocess and JSONL appends moved behind asyncio.to_thread (Sonar S7487/S7493); noqa suppression removed - test_guardian_bench.py: float comparisons via pytest.approx (Sonar); new test pins that ambiguous hits leave precision at 1.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/gemini review |
|
/guardian review |
There was a problem hiding this comment.
Code Review
This pull request implements a structured findings JSON contract for the Guardian AI code reviewer, introducing Pydantic models, markdown rendering, and a benchmark harness to replay and score past PRs. The reviewer provided valuable feedback to make the JSON extraction utility more robust against varying LLM markdown fence formats and suggested sorting the rendered findings by severity to improve readability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
LGTM — no defects found in this diff. Reviewed the entire Pull Request. Checked the implementation of the structured findings JSON contract, the Pydantic models (Finding, ReviewResult), the fallback parsing logic with validation-error retries, and the benchmark harness. Checked the python_extractor module prefix stripping logic, the matching algorithms, and the unit tests. The implementation is highly robust, fully type-safe under strict mypy standards, handles edge cases cleanly, and conforms perfectly to the specified engineering standards and project ontology.
|
Gemini round-2 review: extract_json left trailing backticks when the closing fence shared a line with the JSON (valid JSON never ends with backticks, so removesuffix is safe); render_report now orders findings critical -> major -> minor as the report format promises. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a structured findings JSON contract for the Guardian AI code reviewer, replacing the previous markdown-based output. It introduces Pydantic models (Finding and ReviewResult) for structured data, a markdown renderer, and a testable runner module. Additionally, it establishes a benchmark harness (scripts/guardian_bench.py and src/cgis/guardian/bench.py) to replay and score reviews against curated ground-truth YAML files for past PRs. The context collector is updated to support base reference overrides and source root stripping, and the Python extractor is adjusted to handle dot-slash prefixes. Comprehensive unit tests are added for all new components. As there are no review comments provided, I have no feedback to provide on the review itself.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Top up the 5 runs missing due to free-tier quota: 142x2, 143x1, 144x2. Full baseline: recall=0.22, precision=0.61, noise=0.94/PR, 0 parse failures in 18 runs. Mistral runs still pending its quota. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mistral 429 code 1300 turned out to be a per-minute token cap, not an exhausted plan quota: prompts <=13k tokens pass with ~60s spacing, while PRs 140/143/144 (36-62k token prompts) exceed the per-minute budget in a single request and can never run on the free tier. Error lines for those attempts kept per spec §7. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|



Implements spec §§2–3 of the guardian sprint (docs/specs/2026-06-10-guardian-sprint-design.md): replace free-text reviews with a structured-findings contract, add a deterministic benchmark harness, and record the pre-improvement baseline.
What's here
Structured findings (§2)
guardian/findings.py— frozenFinding/ReviewResultmodels;extract_jsonfence-stripper that survives trailing prosegenerate_structuredon both providers: gemini nativeresponse_schema, mistraljson_objectmoderun_reviewreturnsReviewResultwith one schema-error retry, then raw-text fallback withparse_failed=Trueguardian/render.py— markdown renderer (sorted by severity, double-backtick evidence spans)scripts/guardian_review.pyslimmed to a thin shell overguardian/runner.pyBenchmark harness (§3)
guardian/bench.py— ground-truth models + pure greedy matcher + scorer (recall / precision / noise)scripts/guardian_bench.py— replays guardian on past PRs in detached worktrees, appends JSONLbenchmarks/guardian/CURATION.mdSpec §3.1 erratum: replay
head= review head (parent of the first review-fix commit), not the final pull head — the final head contains the fixes, so every fixed finding would be unfindable. #143 uses its second-round review head (two-wave review).Bugs found by running the bench (fixed here)
Finding.lineusedgt=0→ google-genaiSchemarejectsexclusiveMinimum; every gemini call failed before sending. Nowge=1+ schema regression test.cgis ingest ./srcstores FQNs relative tosrc/, but the collector looked up repo-relative FQNs — every lookup missed. Newsource_rootparam (defaultsrc) +./-prefix normalization.Baseline (complete — 6 PRs × 3 runs, gemini)
* PR 141 has zero GT findings (noise probe) — recall is vacuous there.
gemini mean-over-PRs: recall 0.22, noise 0.94/PR, precision 0.61. Zero parse failures in 18 gemini + 9 mistral runs. Mistral free tier caps tokens per minute: PRs 140/143/144 (36–62k token prompts) exceed the budget in a single request and cannot run on it — mistral rows cover the three feasible PRs only.
The headline number is the point: recall 0.22 is the baseline the rest of the sprint (§§4–6: context upgrades, multi-pass skeptic, inline comments) must beat. Recall=0 on the three largest-diff PRs (36–62k prompt tokens) — exactly the context-starvation §4 targets.
🤖 Generated with Claude Code