P10: Layer B finding-quality eval + all cycle-3 hardening (clean replay of #11)

[OrenAshkenazy]

Summary

Clean replacement for closed PR #11. The original P8 PR cycled through 3 reviews on its own eval-suite code, hit the cap, and stopped with 4 actionable defensive-hardening findings unaddressed. This PR ships the exact same eval suite with all 15 of those findings already applied — and adds the 4 final cycle-3 hardening fixes the cap blocked.

Net: zero unaddressed Gemini findings on day-one merge.

What ships

evals/ — finding-quality eval

LLM-as-judge over Gemini's findings. Cross-vendor (OpenAI judges Gemini) to reduce self-eval bias. Default model gpt-4o-mini (~$0.02/run), overridable via OPENAI_JUDGE_MODEL.

• evals/judge.py — JudgeClient wraps OpenAI chat completions. All API calls wrapped in try/except → JudgeError so transient API/auth/network failures get a clean stderr message instead of a raw SDK traceback. resp.choices non-empty check. Non-dict JSON payloads rejected. reason field str()-coerced before .strip().
• evals/run_eval.py — CLI runner. --samples validated >= 1 up-front (was: 0 or negative would IndexError in majority_judge_label). main() catches JudgeError, exits with code 2 + clean stderr message. UTF-8 explicit on every read_text / write_text. --report path parent dir created if missing. --fixture typo prints available stems instead of a raw traceback.
• evals/test_eval.py — 32 hermetic pytest cases. Mocked judge factories (fake_constant, fake_sequence). Covers: JudgeClient parsing/error paths, majority voting, agreement metrics, confusion matrix, summary rendering, end-to-end against real fixtures with calibrated/uncalibrated fakes, --samples validation, JudgeError handling in main().
• evals/fixtures/ — 4 hand-labeled fixtures (PRs P3: --min-severity flag, Variations table, fast-track README #6/P4: --sticky-receipt for live PR-comment visibility #7/P5: auto version bump + tag + GitHub Release on PR merge #8/P6: use RELEASE_TOKEN PAT in release workflow (unblocks v0.1.1) #9) with 11 ground-truth findings (10 useful + 1 false-positive — PR P5: auto version bump + tag + GitHub Release on PR merge #8's CHANGELOG misread).
• evals/README.md — cross-vendor rationale, fixture format, calibration workflow, cost analysis (~$0.02 per run), miscalibration sanity checks.

.github/workflows/eval-weekly.yml — weekly CI

• Sundays 00:00 UTC + workflow_dispatch.
• 3 samples per finding for variance measurement.
• Posts summary to a rolling per-quarter GitHub Issue.
• Fails the run if judge↔human agreement drops below 80% (calibration gate).
• Catches drift in Gemini's output format + judge model regressions over time.
• Needs OPENAI_API_KEY repo secret + optional OPENAI_JUDGE_MODEL var.

Why a clean replay instead of merging #11

PR #11 was a meta-PR: an eval suite reviewed by the bot it evaluates. Defensive Python in API-calling code attracts maximum Gemini scrutiny. Each round of hardening yields fresh suggestions for more hardening. After 3 cycles the loop is supposed to stop and hand to a human — and it did. Rather than merge with 4 known follow-ups, this PR rolls them in cleanly.

The 4 cycle-3 findings now applied:

1. judge.py:135 — Wrap OpenAI API call in try/except so transient failures raise JudgeError (uniform error surface for the runner).
2. run_eval.py:27 — Import JudgeError for catching in main().
3. run_eval.py:286 — Validate --samples >= 1. 0/negative previously crashed majority_judge_label with IndexError.
4. run_eval.py:314 — Catch JudgeError in main(), exit code 2 with stderr message.

Test plan

• pytest evals/ tests/ → 88/88 pass (was 56 plugin + 32 eval; was 56+28 in P8: Layer B finding-quality eval (LLM-as-judge, cross-vendor) #11)
• ruff check plugins/ tests/ evals/ → clean
• python3 -m evals.run_eval --judge fake --samples 0 → exit 1, friendly message
• python3 -m evals.run_eval --judge fake --fixture pr-8 → exit 0, sensible metrics
• After merge: set OPENAI_API_KEY repo secret, manually trigger Weekly finding-quality eval once to confirm end-to-end against the real OpenAI API + tracking-issue creation. Subsequent runs fire automatically every Sunday.

Release

Default patch bump → v0.1.4 on merge.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a Layer B finding-quality evaluation framework under the evals/ directory. It features an LLM-as-judge system (using OpenAI's gpt-4o-mini by default) to rate Gemini Code Assist findings and compare them against hand-labeled human ground-truth fixtures. The PR includes the judge client, a CLI runner, a comprehensive pytest suite, and several initial fixtures. The code reviewer provided valuable feedback to improve the robustness and formatting of the generated reports, including handling backticks in markdown excerpts, formatting list outputs cleanly, and preserving UTF-8 characters in JSON dumps.

[OrenAshkenazy]

@gemini-code-assist please review the latest changes.

[gemini-code-assist]

Code Review

This pull request introduces a Layer B finding-quality evaluation suite under evals/ that uses an LLM-as-judge (OpenAI gpt-4o-mini by default) to rate Gemini Code Assist findings against human ground-truth labels. It includes several PR fixtures, a CLI runner, a weekly CI workflow, and unit tests. The reviewer feedback highlights three valuable improvement opportunities: adding a defensive check in majority_judge_label to handle empty judge_labels gracefully, specifying a timeout for the OpenAI API call to prevent the runner from hanging, and catching JudgeError during individual finding evaluations to skip problematic findings instead of aborting the entire run.

[OrenAshkenazy]

@gemini-code-assist please review the latest changes.

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive LLM-as-judge evaluation framework under evals/ to rate Gemini Code Assist findings and compare them against human-labeled ground-truth fixtures. The review feedback correctly identifies two potential runtime crashes in evals/run_eval.py: a KeyError in confusion_matrix when a judge label falls outside of VALID_LABELS (such as "unknown"), and an IndexError in render_summary when attempting to access judge_reasons[0] on an empty list.

[OrenAshkenazy]

@gemini-code-assist please review the latest changes.

[gemini-code-assist]

Code Review

This pull request introduces a Layer B finding-quality evaluation framework under evals/ that uses an LLM-as-judge (OpenAI gpt-4o-mini) to rate Gemini Code Assist findings and compare them against human-labeled ground-truth fixtures. It includes the judge client, a CLI runner, unit tests, and initial fixtures. The review feedback focuses on defensive hardening of the runner, suggesting safer JSON parsing and file I/O error handling, explicit type validation of nested configuration keys, and explicit None checks instead of falsy checks when coercing external payload values.

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.

[gemini-code-assist]

When reading and parsing JSON files, it is important to catch both OSError and ValueError (which covers json.JSONDecodeError and UnicodeDecodeError) to safely handle file system issues and decoding/parsing errors. Additionally, specifying errors='replace' when reading text files prevents crashes on invalid UTF-8 bytes.

Suggested change

	def load_fixture_pair(pr_id: str) -> tuple[dict, dict]:
	"""Load (findings, labels) for a fixture, keyed on filename stem like 'pr-6'."""
	findings_path = FIXTURES_DIR / f"{pr_id}.json"
	labels_path = FIXTURES_DIR / f"{pr_id}.label.json"
	if not findings_path.exists():
	raise FileNotFoundError(f"Missing findings fixture: {findings_path}")
	if not labels_path.exists():
	raise FileNotFoundError(
	f"Missing human-label sidecar: {labels_path}. "
	"Every fixture needs a `.label.json` for the eval to compute agreement."
	)
	# Explicit utf-8 encoding so platforms with non-UTF8 locale defaults (some
	# Windows + older CI runners) don't decode-error on Unicode in finding bodies.
	return (
	json.loads(findings_path.read_text(encoding="utf-8")),
	json.loads(labels_path.read_text(encoding="utf-8")),
	)
	def load_fixture_pair(pr_id: str) -> tuple[dict, dict]:
	"""Load (findings, labels) for a fixture, keyed on filename stem like 'pr-6'."""
	findings_path = FIXTURES_DIR / f"{pr_id}.json"
	labels_path = FIXTURES_DIR / f"{pr_id}.label.json"
	if not findings_path.exists():
	raise FileNotFoundError(f"Missing findings fixture: {findings_path}")
	if not labels_path.exists():
	raise FileNotFoundError(
	f"Missing human-label sidecar: {labels_path}. "
	"Every fixture needs a `.label.json` for the eval to compute agreement."
	)
	# Explicit utf-8 encoding so platforms with non-UTF8 locale defaults (some
	# Windows + older CI runners) don't decode-error on Unicode in finding bodies.
	try:
	return (
	json.loads(findings_path.read_text(encoding="utf-8", errors="replace")),
	json.loads(labels_path.read_text(encoding="utf-8", errors="replace")),
	)
	except (OSError, ValueError) as exc:
	raise JudgeError(f"Failed to read or parse fixture files for {pr_id}: {exc}") from exc

References

1. When parsing JSON files, catch ValueError (which covers json.JSONDecodeError) and OSError to safely handle both decoding/parsing errors and file system issues.
2. When reading text files that may contain invalid UTF-8 bytes, use errors='replace' and catch both OSError and ValueError (which covers UnicodeDecodeError) to prevent crashes.

[gemini-code-assist]

When parsing configuration files (such as JSON), explicitly validate the types of nested keys before performing membership checks or type-specific operations to prevent TypeError crashes. Additionally, when validating that a dictionary field is a non-empty list, use isinstance(val, list) and val to prevent type errors from non-list truthy values while safely excluding empty lists.

    findings_doc, labels_doc = load_fixture_pair(stem)
    
    pr = findings_doc.get("pr")
    if not isinstance(pr, int):
        raise JudgeError(f"Fixture {stem} is missing a valid integer 'pr' field.")
        
    human_labels = labels_doc.get("human_labels")
    if not (isinstance(human_labels, list) and human_labels):
        raise JudgeError(f"Fixture sidecar {stem}.label.json is missing a non-empty 'human_labels' list.")
        
    findings = findings_doc.get("findings")
    if not (isinstance(findings, list) and findings):
        raise JudgeError(f"Fixture {stem}.json is missing a non-empty 'findings' list.")

    label_by_id = {}
    for lbl in human_labels:
        if isinstance(lbl, dict) and "comment_id" in lbl:
            label_by_id[lbl["comment_id"]] = lbl

    out: list[EvaluatedFinding] = []
    for finding in findings:
        if not isinstance(finding, dict):
            continue
        cid = finding.get("comment_id")
        if not cid:
            continue
        human = label_by_id.get(cid)
        if human is None:
            print(
                f"warning: PR #{pr} finding {cid} has no human label; skipping.",
                file=sys.stderr,
            )
            continue

References

1. When parsing configuration files (such as JSON), explicitly validate the types of nested keys before performing membership checks or type-specific operations to prevent TypeError crashes.
2. When validating that a dictionary field is a non-empty list, use isinstance(val, list) and val to prevent type errors from non-list truthy values (like True or non-empty strings) while safely excluding empty lists.

[gemini-code-assist]

When defaulting or coercing values from external payloads (such as JSON findings), check for None explicitly rather than using falsy checks (e.g., val or "") to preserve valid falsy values like 0, False, or empty strings, while still allowing defensive type coercion.

        severity = finding.get("severity")
        path = finding.get("path")
        body_val = finding.get("body")
        label_val = human.get("label")
        out.append(
            EvaluatedFinding(
                pr=pr,
                comment_id=cid,
                severity=str(severity) if severity is not None else None,
                path=str(path) if path is not None else None,
                line=finding.get("line"),
                human_label=str(label_val) if label_val is not None else "unknown",
                judge_labels=[r.label for r in judge_results],
                judge_confidences=[r.confidence for r in judge_results],
                judge_reasons=[r.reason for r in judge_results],
                # Strip backticks: the excerpt is rendered inside backticks in
                # render_summary's disagreement block, and an unescaped backtick
                # would prematurely close the markdown code span.
                body_excerpt=str(body_val)[:120].replace("\n", " ").replace("`", "'") if body_val is not None else "",
            )
        )

References

1. When defaulting or coercing values from external payloads (such as LLM responses), check for None explicitly rather than using falsy checks (e.g., val or "") to preserve valid falsy values like 0, False, or empty strings, while still allowing defensive type coercion.

[gemini-code-assist]

When wrapping best-effort operations (such as disk I/O) in a try-except block, ensure the exception handler returns early (e.g., returning a success or default status) instead of falling through if subsequent code references variables that would remain unbound, thereby preventing a NameError.

    if args.report:
        report = {
            "metrics": m,
            "rows": [dataclasses.asdict(r) for r in rows],
            "summary_md": summary,
        }
        report_path = pathlib.Path(args.report)
        try:
            # Create parent dirs so users can pass nested paths like /tmp/eval/report.json.
            report_path.parent.mkdir(parents=True, exist_ok=True)
            # ensure_ascii=False lets non-ASCII characters in findings/reasons
            # write through as utf-8 instead of \uXXXX escapes. Cheaper to read.
            report_path.write_text(json.dumps(report, indent=2, ensure_ascii=False), encoding="utf-8")
            print(f"\nReport written to {args.report}", file=sys.stderr)
        except OSError as exc:
            print(f"error: failed to write report to {args.report}: {exc}", file=sys.stderr)
            return 1

References

1. When wrapping best-effort operations (such as disk I/O) in a try-except block, ensure the exception handler returns early (e.g., returning a success or default status) instead of falling through if subsequent code references variables that would remain unbound, thereby preventing a NameError.