Story

As a maintainer who will be asked whether the qa-lead persona is any good,
I want its held-out eval set to actually be scorable and scored,
so that the evals.required_before: stable gate in its manifest is a real gate rather than a declaration, and there is a baseline to compare against before its exposure is broadened.

The problem, reproduced on origin/main (85658ac)

personas/qa-lead/persona.yml declares an eval gate. It cannot run. Three independent breakages:

1. The cases do not conform to the case schema.

$ python3 evals/validate-cases.py evals/qa-lead/holdout/cases.jsonl
::error::eval cases invalid: line 1: schema violation at expected:
         Additional properties are not allowed ('recommend', 'risk_tier' were unexpected)

evals/case.schema.json requires expected to be exactly {escalate, risk} with additionalProperties: false. Every qa-lead case uses {risk_tier, escalate, recommend}.

2. CI is green over an unscorable set. The tree-mode validator that CI runs only checks split structure and id discipline:

$ python3 evals/validate-cases.py
OK: 12 skill(s), 96 case(s) across dev+holdout, no cross-split id overlap.

Per-case schema conformance is only checked in file mode, which nothing invokes. That is why this has been invisible since #1174.

3. The scorer cannot find the prompt.

$ bash scripts/evals/run-eval.sh qa-lead
::error::eval scorer: no skill prompt for 'qa-lead' (expected .../prompts/qa-lead.md)

run-eval.sh resolves prompts/<skill>.md. The persona runtime's convention is prompts/<id>/advisory.md (pr_advisory_prompt_path in scripts/lib/persona-runner.sh). The two conventions have never been reconciled.

And even with 1 and 3 fixed, the default scorer is the wrong one. score_deterministic compares .expected.escalate and .expected.risk by equality. risk is absent from all six qa-lead cases, and a test-risk advisory is prose — a paragraph of "what I'd shore up" cannot be scored by string equality against a reference. run-eval.sh already supports the right mode:

scorer.json:  { "mode": "llm-judge", "judge_prompt": "judge.md", "pass_threshold": 0.7 }

evals/judge.md exists and deep-review already uses this mode.

Consequence: skill-eval-report.yml has run 82 times (78 scheduled, 4 dispatched) and every single run scored triage — the workflow's default (SKILL: ${{ inputs.skill || 'triage' }}). qa-lead has never been scored, not once.

Acceptance Criteria

1. python3 evals/validate-cases.py evals/qa-lead/dev/cases.jsonl and .../holdout/cases.jsonl both pass in file mode (per-case schema conformance), not just tree mode.
2. The expected payload is reconciled with evals/case.schema.json. Either the cases adopt {escalate, risk}, or the schema is extended to describe the advisory shape — but if the schema is extended, the extension must be deliberate and documented, not a widening of additionalProperties. State which path was taken and why in the PR.
3. No case's meaning changes. The six held-out cases encode real judgments (money-path idempotency, coverage gaming, and the negative control that must not manufacture risk on a well-covered refactor). Renaming risk_tier→risk is mechanical; silently dropping the recommend guidance or flipping an escalate value is not. Preserve every judgment; if recommend has no home in the schema, preserve it somewhere the judge can read.
4. evals/qa-lead/scorer.json selects mode: llm-judge against evals/judge.md with an explicit pass_threshold. Deterministic equality scoring is not acceptable for a prose advisory — justify the threshold chosen.
5. bash scripts/evals/run-eval.sh qa-lead resolves prompts/qa-lead/advisory.md and runs to completion. Reconcile the two prompt-path conventions rather than special-casing qa-lead — scripts/lib/persona-runner.sh already owns pr_advisory_prompt_path; prefer reusing it over duplicating the convention.
6. CI catches this class of breakage from now on. Per-case schema validation runs over the whole tree in CI, so a non-conforming case set fails a PR instead of sitting green for six weeks. Expect this to fail loudly on the other 11 skills — that is [qa-lead S8] Fleet-wide: 11 of 12 eval sets fail case.schema.json and CI never notices #1651's job, so scope the CI gate so it can be introduced without blocking every unrelated PR (e.g. gate qa-lead now with a documented allowlist for the rest, removed by [qa-lead S8] Fleet-wide: 11 of 12 eval sets fail case.schema.json and CI never notices #1651).
7. A first baseline is captured: run-eval.sh qa-lead is executed against prompts/qa-lead/advisory.md and the per-case results plus the aggregate score are recorded in the PR. This is the "before" number; it is the whole point of the story.
8. The offline bats coverage (tests/test_skill_evals.bats) exercises the llm-judge path for qa-lead with a stubbed engine, so scoring is testable with no network.

Tasks / Subtasks

• Decide and document the schema reconciliation (AC Go-live improvements for PR review agent #2), then rewrite the 10 case payloads preserving every judgment. (AC: test issue from agent #1, Go-live improvements for PR review agent #2, Add @claude delegation, auto-merge, and rebase handling #3)
• Add evals/qa-lead/scorer.json with mode: llm-judge, judge_prompt, pass_threshold. (AC: Optimize review: small-PR and incremental fast paths #4)
• Reconcile the prompt-path convention between run-eval.sh and scripts/lib/persona-runner.sh. (AC: feat: add Copilot engine support via REVIEW_ENGINE toggle #5)
• Add tree-wide per-case schema validation to CI with a scoped allowlist. (AC: feat: feature ideation framework — adversarial proposer/challenger/synthesizer #6)
• Run the scorer, capture the baseline, paste per-case results + aggregate into the PR. (AC: Workflow should skip no-op PRs in MAX_PRS count #7)
• Extend tests/test_skill_evals.bats for the qa-lead llm-judge path with a stub engine. (AC: Add Claude Code workflow #8)

Dev Notes

• The held-out set is CODEOWNER-gated and holdout-guard.yml covers it. Editing evals/qa-lead/holdout/ will require CODEOWNER review — that gate is intentional and must not be circumvented. Expect the PR to need it.
• Do not tune the cases to make the scorer pass. The dev/holdout split exists so a proposer cannot teach to the test (evals/README.md, "The no-overlap rule"). This story fixes encoding and plumbing. If the baseline score is bad, that is a finding to report — not a reason to edit the held-out cases.
• validate-personas.py enforces min_cases only once status has reached evals.required_before (stable). At draft it is unenforced, which is a second reason nothing has caught this. Fixing that is out of scope here but worth noting in the PR.
• run-eval.sh distinguishes infra failure (engine exited non-zero — throttled) from quality miss (engine answered, answer wrong), exiting 2 vs 1. Do not collapse that distinction; Skill eval: API throttling masquerades as a held-out regression (infra failure scored as regression, not error) #920 exists because a transient throttle once opened a false regression tracker.
• Scoring runs through the engine abstraction (scripts/engine.sh), never a re-implemented model call, and both the skill and judge commands are injectable (EVAL_ENGINE_CMD / EVAL_JUDGE_CMD) so offline tests drive stubs.
• This story gates [qa-lead S3] Serve the pull_request surface so qa-lead runs without being summoned #1646 (serve the pull_request surface) and [qa-lead S8] Fleet-wide: 11 of 12 eval sets fail case.schema.json and CI never notices #1651 (the fleet-wide fix). Sequencing follows [Phase 4] Capture the pre-rollout baseline for the three success metrics, including a new no-action-comment noise metric #1411's precedent: capture the baseline before the change that alters what is being measured, or the "before" is contaminated.

Project Structure Notes

evals/qa-lead/{dev,holdout}/cases.jsonl (CODEOWNER-gated), new evals/qa-lead/scorer.json, scripts/evals/run-eval.sh, possibly evals/case.schema.json, tests/test_skill_evals.bats, and the CI wiring in .github/workflows/lint.yml.

References

• evals/case.schema.json, evals/validate-cases.py, evals/README.md
• scripts/evals/run-eval.sh (scorer modes, scorer.json contract, infra-vs-quality exit codes)
• evals/judge.md, evals/deep-review/ (the existing llm-judge precedent)
• scripts/lib/persona-runner.sh — pr_advisory_prompt_path
• .github/workflows/skill-eval-report.yml — defaults to triage
• .github/workflows/holdout-guard.yml
• Add Murat persona instance — worked example for the persona standard #1174 (added the seed cases), Skill eval: API throttling masquerades as a held-out regression (infra failure scored as regression, not error) #920 (infra-vs-quality distinction)

⚑ AC amendment — 2026-09-07 (maintainer). Supersedes where it conflicts.

PR #1656 has burned all 3 automated review cycles and is at needs-human-review with the
same findings carried across four passes. The blockers are not code quality — CI is
fully green and ACs #1, #2, #3, #5, #6, #8 are independently verified MET at head. The
amendment below exists because two of the original ACs asked for work the agent will not
do, and one review finding turned out to be correct after the automated reviewer dismissed
it. Read this section as authoritative over the original ACs where they differ.

A. AC #7 (baseline) is now MAINTAINER-DELIVERED — do not attempt it

The original AC #7 asked the implementer to run the scorer and record the baseline. That
was a mis-scoped ask: capturing it requires a tokened engine run, and dev-lead has
correctly answered no-changes to it four times rather than fabricate a number.

The maintainer has run it. The result is posted as a comment on PR #1656 and is
reproduced there in full. AC #7 is CLOSED. Do not run the scorer, do not edit the
held-out cases, and do not treat a missing baseline as outstanding work.

B. AC #4 — record the threshold rationale (small, in the PR body)

Add one or two sentences to the PR description justifying pass_threshold: 0.7,
covering: it matches the deep-review precedent for the same shared judge, and it encodes
"must get the escalate/risk decision right and surface most of the expected guidance,"
so a prose advisory that flips the decision cannot clear it. No code change.

C. NEW AC #9 — the --schema-tree gate must enforce the STRUCTURAL invariants too

Two codeant-ai threads on evals/validate-cases.py were dismissed by the automated
reviewer on the grounds that "split structure, duplicate ids, and cross-split overlap
remain enforced by the directory-mode validator CI already runs." That is false and the
threads are correct. Verified 2026-09-07:

• --schema-tree is the only validate-cases.py invocation in any workflow
  (.github/workflows/lint.yml:390). Directory/tree mode is invoked by nothing in CI.
• On main there is no validate-cases.py CI invocation at all — so --schema-tree
  is net-new and this PR is not a regression. But it is the only gate that will exist,
  and it currently checks schema conformance only.
• validate_schema_tree() does if not cases_path.is_file(): continue — a missing
  dev/ or holdout/ split is silently skipped, so a skill can report schema-valid
  while violating the split contract (thread 1, exactly as written).
• It performs no id-uniqueness check within a split and no cross-split overlap
  check (thread 2, exactly as written).

The cross-split overlap rule is not cosmetic: evals/README.md calls it the whole point of
the split — "the proposer could see — and overfit to — a case the gate later scores
against, which defeats the entire held-out guarantee ('teaching to the test')." Today
nothing in CI enforces it.

Required:

1. --schema-tree fails when a gated skill is missing either split's cases.jsonl
   (replace the silent continue). An allowlisted skill stays skipped as today.
2. It enforces id uniqueness within each split, and disjointness of ids across
   dev/holdout, for every gated skill — reusing the existing directory-mode logic
   rather than reimplementing it.
3. Failures name the skill, split, and offending id, matching the existing diagnostic style.
4. Bats coverage for each new failure mode: missing split, duplicate id within a split,
   id present in both splits — plus a passing control asserting qa-lead still validates.
5. The allowlist semantics are unchanged: exempted skills are skipped wholesale, so this
   cannot fail unrelated PRs before [qa-lead S8] Fleet-wide: 11 of 12 eval sets fail case.schema.json and CI never notices #1651 lands.

Then reply to both codeant-ai threads stating what changed, and resolve them. Do not
resolve them as "won't fix" — they were right.

D. What "done" means now

Scope discipline: change evals/validate-cases.py, its bats suite, and the PR description.
Do not touch scripts/evals/run-eval.sh, evals/qa-lead/**, evals/case.schema.json, or
.github/workflows/lint.yml — all four are already correct at head, and evals/qa-lead/**
is CODEOWNER-gated held-out data that must not be edited to fit a score.

E. NEW AC #10 — qa-lead needs its OWN judge (added 2026-09-07, after the baseline run)

The AC #7 baseline is captured: 0.167 aggregate, 1/6 passed
(run 34133681006, full
per-case table in the PR comment). engine_rc=0 throughout, so nothing was throttled.

The dominant, in-scope cause is a judge-rubric mismatch, and it is a defect in AC #4's delivery:
evals/qa-lead/scorer.json points at the shared evals/judge.md, which begins

"The deep-review skill emits a JSON verdict with at least decision (approve | escalate),
risk (LOW | MEDIUM | HIGH), and a findings array."

qa-lead emits prose, so the judge marks a correct advisory down for structure it was never
supposed to produce. Its own words on the 0.00 case: "missing decision field (approve/escalate),
risk classification, and findings array entirely."

The correct pattern already exists in this repo. #1642 gave the sibling persona its own judge —
evals/solution-architect/scorer.json → "judge_prompt": "solution-architect/judge.md". qa-lead
must do the same.

Required:

1. Add evals/qa-lead/judge.md — a judge that grades a prose test-risk advisory against the
   case's expected reference: does it reach the right risk tier, the right escalate call, and
   surface the substance of recommend? Model it on evals/solution-architect/judge.md for shape;
   do not copy deep-review's JSON-verdict rubric.
2. Point evals/qa-lead/scorer.json at it ("judge_prompt": "qa-lead/judge.md"). Match
   solution-architect's scorer shape, which also carries gate_threshold alongside pass_threshold.
3. Keep pass_threshold: 0.7 — the rationale is recorded in the PR description. Do not lower the
   threshold to raise the score. That is precisely the reward-hacking the held-out split exists to
   prevent.
4. Do not edit any file under evals/qa-lead/dev/ or evals/qa-lead/holdout/. The cases are
   correct and CODEOWNER-gated; the judge is what is wrong.
5. Judge-path coverage in tests/test_skill_evals.bats with a stubbed judge (offline), asserting
   qa-lead resolves qa-lead/judge.md rather than the shared judge.md.

Out of scope here — do not attempt: the other two confounds behind the 0.167 are shared-harness
problems (the harness scores the Haiku tier while production runs Opus; a live-GitHub prompt is fed a
static case string). They are filed as #1686 and affect all nine personas. Do not change
scripts/evals/run-eval.sh, scripts/engine.sh, or prompts/qa-lead/advisory.md in this PR.

Re-baselining is not required of you. After AC #10 lands, the maintainer re-runs the scorer and
records the delta.

F. Revised definition of done

Remaining implementer work is exactly #9 and #10. Touch only:
evals/validate-cases.py, evals/qa-lead/judge.md (new), evals/qa-lead/scorer.json,
tests/test_validate_cases.bats, tests/test_skill_evals.bats.