docs(decisions): ADR 0243 — one review eval stage keyed by a surface sub-discriminator (#4976)

[usirin]

When the three review gates merge into one review skill, the eval corpus has to say what a review row is. Today a code-review label and a doc-review label carry different fields, and the grader picks its rubric off the stage name — so one bare review key would let both label shapes sit under it and quietly send two rubrics to one grader.

This ADR records the founder ruling on #4976: keep one review stage key, and give every review entry a required surface field (code | doc | skill) that decides both the legal label shape and which grader runs. gradeEntry dispatches on the (stage, surface) pair, never on the stage alone, and a PR reviewed on two surfaces records one row per surface for the same input instead of one averaged row.

Scope is deliberately narrow: this is eval-corpus data shape only. surface is not a GitHub label, not a triage input, and not the runtime rubric-router — runtime routing stays derived from the diff.

What's in the diff

One added file, nothing else:

• .decisions/0243-review-eval-stage-surface-discriminator.md (+190)

Acceptance criteria

Checked one by one against #4976 before arming the closing keyword:

• ADR naming the label shape, the discriminator, and the dispatch key — §1 gives the per-surface label table and states that a mismatched label stays unrepresentable; §2 fixes gradeEntry on the (stage, surface) pair.
• What a merged pass-rate means, and whether surfaces may be aggregated — §4: the report's cell key carries surface, and rows from different surfaces are never aggregated into one undifferentiated review number.
• Cites its source — the Context section reads the two label shapes out of packages/fabrika-cli/src/eval/corpus.ts, the grader switch out of packages/fabrika-cli/src/eval/oracle.ts, and the namespace class out of packages/fabrika-cli/src/wire/verdict-marker.ts.
• Each rejected alternative named with its reason — §6 covers the union label, the superset struct, and three separate stage keys, and states what new information would be needed to reopen them.

All four are delivered by this diff, so the seam is a full close.

Fixes #4976

Deviations

• Authoring provenance (outside the seven classes, disclosed anyway) — Said: the write-code
  contract assumes the lane that opens a PR is the lane that authored the diff. Did: the ADR was
  written and pushed by a separate agent on umut/adr-0243-review-surface-discriminator; this lane
  only verified the branch and opened the PR, and edited nothing. Why: the authoring seam and the
  ship seam were split across two agents; re-authoring here would have meant editing another lane's
  committed record. Disposition: for the reviewer to judge — the reviewer is grading a diff this
  lane did not write, so no self-review firewall concern applies either way.
• Scope narrowing — Said: Decide what a merged review eval stage's label, grader and pass-rate are #4976 asks for the decision only. Did: exactly that; the schema,
  the grader and the report key are untouched. Why: the issue fences implementation out of scope.
  Disposition: no action needed — the implementing children are Land the merged review eval stage — schema entry, grader dispatch, manifest key #4979 (schema + gradeEntry) and
  Carry the review surface onto the scorecard so a merged pass-rate never averages two grading regimes #4980 (report key), both already filed.

[github-actions]

No preview deploy

• No preview deploy for this PR — its diff touches no deploy-relevant path, so no preview stack was minted and e2e is not applicable. _(583e0ec)

• web — Stage pr-5030 torn down.

[usirin]

review-doc: advisory — PASS (all four acceptance criteria met; §CP blocking-set, so no bindable first-line marker per ADR 0111/0151)

Reviewed-head: @ 583e0ec

Routing

• class-probe classify --namespaces on the real changed-file set → 1 file → has-docs → required namespace set = review-doc (one namespace; not UI-affecting, no code root, no skill path).
• cp-classify classify → content-undetermined [content-source-present] — no path matched, but 1 .decisions/** file can be §CP by content. Not a non-§CP answer.
• guard-content-probe classify on the ADR body at head (ADR 0164) → guard-touching (§CP) [guard-vocabulary-match].

Settled §CP ⇒ this verdict takes the advisory form: the head is bound in Reviewed-head: above, not in a first-line @ <sha>. A failing criterion would still have been posted as a bindable FAIL (ADR 0226).

Diff scope

One file, added, +190/-0: .decisions/0243-review-eval-stage-surface-discriminator.md. Confirmed first-hand against the PR files API — nothing else in the diff. The local body is 190 lines, matching the addition count.

Fidelity to the governing ruling (#4976, usirin, 2026-08-09T03:40:53Z)

I read the full #4976 thread before judging, not just the ADR. The ADR transcribes the ruling without widening or narrowing it.

Ruling clause	ADR
"One review stage key"	Decision: "The eval corpus keeps one review stage key"
"a surface discriminator (code | doc | skill) that selects both the label shape and the grader"	§1 (label shape, per-surface table) + §2 (grader)
"gradeEntry dispatches per discriminated (stage, surface) pair, never per stage alone"	§2: "gradeEntry dispatches per discriminated (stage, surface) pair, never per stage alone"; "Dispatching on stage alone is banned"
"A mixed-surface PR is represented natively: one recorded label per surface for the same inputRef, each graded by its own grader"	§3: "one recorded label per surface for the same inputRef — two rows, each graded by its own grader"; no composite label, no primary-surface tiebreak
"this is eval-corpus data shape only — no GitHub label, no triage involvement, and runtime routing stays derived-from-the-diff"	§5, reproduced verbatim as the record's own fence, and restated as a Banned bullet: "Treating surface as a GitHub label, a triage input, or the runtime rubric-router"

Scope fence holds. I checked specifically for reach beyond eval-corpus data shape. There is none: surface is scoped to the corpus schema, gradeEntry, and the report bucket key. §5 additionally fences out packages/fabrika-cli/src/wire/ and the fate of already-committed v1-keyed rows. The report-key clause (§4) is not a widening — #4976's own AC 2 demands the pass-rate meaning, and the reporting child (#4980) is named rather than pre-empted.

No supersession asserted, no accepted ADR's decision text touched. The diff is a single added file, so by construction it edits no existing ADR. ADR 0144 is not referenced anywhere in the body (grep: zero hits for 0144, supersede, superseded). The "where this sits" paragraph explicitly holds ADRs 0112 and 0146 untouched — "both rulings stand untouched… No status line moves" — which is the correct posture rather than an asserted supersession.

Fixes #4976 seam — per-AC check

I checked each of #4976's four acceptance criteria myself rather than taking the PR body's assertion.

1. ADR naming the label shape, the discriminator, and the dispatch key — met. §1 gives the per-surface label table and states the mismatch stays unrepresentable, with surface required and defaultless (a missing surface is a decode failure, not a fallback-graded row). §2 fixes the dispatch key at (stage, surface).
2. What a merged pass-rate means, and whether surfaces may be aggregated — met. §4, in one actionable sentence: "a pass-rate is only a measurement over one grading regime, so the report's cell key carries surface and rows from different review surfaces are never aggregated into a single undifferentiated review number." Both halves answered.
3. Cites its source — met, and each citation verified against the source at this head, not accepted on assertion:
   • packages/fabrika-cli/src/eval/corpus.ts — ReviewCodeEntry is stage: Schema.Literal("review-code") with label.acFindings; ReviewDocEntry is stage: Schema.Literal("review-doc") with label.findings. The "a label whose shape doesn't match its stage is unrepresentable" sentence is in the module docblock. CorpusEntry is a Schema.Union of five stage-pinned members, and CorpusManifest.stages re-groups per stage key. All as described.
   • packages/fabrika-cli/src/eval/oracle.ts — gradeEntry is switch (entry.stage) with case "review-code" → gradeReviewCode and case "review-doc" → gradeReviewDoc. As described.
   • packages/fabrika-cli/src/wire/verdict-marker.ts — const NAMESPACE = /^review(-[a-z0-9]+)*$/. The regex is quoted exactly right.
   • The ADR-0242 §3 cross-reference also checks out: §3 is "review — from a family to one skill", and its table row absorbs review-code/review-doc/review-skill as three rubric files.
4. Each rejected alternative named with its reason — met. §6 covers the union label, the superset struct (with the "sniffing a shape" reason), and three separate live stage keys, and closes with what new information reopening would need. Decide what a merged review eval stage's label, grader and pass-rate are #4976 itself named three live options, two of which are rejections; the ADR's third rejection is additive, not a gap.

The Fixes seam is not an over-claim. All four ACs are delivered by this one file, so the auto-close is correct.

#4977 cross-reference — pointer, not duplicate

The Context paragraph names the sibling ruling ("recorded provenance wins": already-committed v1-keyed rows keep their original stage keys and are never re-keyed), links #4977, states how the two compose ("that ruling governs rows already on disk; this one governs the live schema"), and stops. It does not re-derive #4977's grounds — the "don't fake the baseline" reasoning from the ruling comment is absent by design. §5 and the Consequences bullet likewise point rather than restate. This is the right shape: #4977's own ADR is still owed, and nothing here will drift out of sync with it when that record lands.

ADR mechanics

• Number 0243 does not collide. Not present on main (.decisions contents at main run …0236, 0238, 0239, 0241, 0242). Not claimed by any of the 10 other open PRs — I checked each open PR's file list; the only in-flight ADR files are 0237 (PR docs(decisions): 0237 — agent brevity binds messages, not board artifacts (#4694) #4703) and 0235 (PR docs(decisions): ADR 0235 — a fence carries zero expansions; everything dynamic lives inside the script #4614). The claimed-but-unopened 0240 is untouched here.
• Frontmatter well-formed: id: 0243, title, status: accepted, date, tags — same shape as the neighbouring 0242. id matches the filename 0243-review-eval-stage-surface-discriminator.md.
• pipeline-cli decisions-index validate over the .decisions tree at this head: ADR files valid (no duplicate or mismatched id).
• Links are repo-relative and resolve; the head's check docs have no dead internal links job is green.

Head CI (read live from REST check-runs at this head)

All completed checks are success or skipped; zero failures, zero cancelled. ci-required is success. Relevant to this diff: validate ADR files — no duplicate/mismatched ADR number success, check docs have no dead internal links success, scan changed files for leaks success. One job, produce run-evidence bundle, was still in_progress at read time — reported as such, not counted green and not counted red.

Non-blocking nits (no repair round required)

1. §1's skill row leaves its label shape unnamed — "the skill rubric's own label shape". Honest, since no ReviewSkillEntry exists in corpus.ts today and there is nothing to cite. Decide what a merged review eval stage's label, grader and pass-rate are #4976 fences schema-writing out of scope, so this is correctly deferred to Land the merged review eval stage — schema entry, grader dispatch, manifest key #4979 — but Land the merged review eval stage — schema entry, grader dispatch, manifest key #4979 inherits an undetermined shape for one of the three surfaces and will have to settle it.
2. A literal-reading gap worth naming. The ruling says "every corpus entry/label carries a surface discriminator"; the ADR scopes surface to review entries only ("required on every review corpus entry"). I read the ADR's narrowing as the correct reading — a triage or ship-it row has no review surface, and the whole ruling is about the review stage — but flagging it so it can be corrected if the broader literal reading was meant.
3. Minor self-description mismatch. The Context paragraph says of Decide live stage key vs recorded provenance — the v1 baseline rows and the capture join key #4977 "They are referenced, not restated" immediately after restating that ruling's conclusion in a sentence. The substance is a pointer (no grounds re-derived), so it composes correctly; only the sentence describing itself is slightly off.

Verdict

PASS (advisory). The record is faithful to the #4976 ruling clause by clause, holds the eval-corpus-data-shape fence without reaching into GitHub labels, triage or runtime routing, cites three source facts that each verify against the source at this head, asserts no supersession and edits no accepted ADR, and delivers all four acceptance criteria — so Fixes #4976 is earned. §CP: merge needs the control-plane approval, not this comment.