review: a medium importance tier, reviewer-proposed, validator-adjudicated, code-vetoed - #382
Conversation
🦋 Changeset detectedLatest commit: cdd1ac0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // nitpick ban wins over the tier — a medium nitpick is a labeling | ||
| // contradiction, and the ban is the stricter rule — and medium | ||
| // spends the budget like any other non-blocking claim). | ||
| if (blockingMedium && claim.importance !== "medium") { |
There was a problem hiding this comment.
note (non-blocking): The blocking-medium gate runs ahead of the documentation-label exemption. The exemption below it exists because the documentation autofix reads posted threads, and the documentation reviewer is advisory-only by charter so it never gets the importance field, which makes that exemption unreachable under this dial. Pre-existing under blocking-only, which suppresses the same claims one branch earlier, but this change amplifies it by documenting blocking-medium as the recommended dial and blocking-only as the rollback, so more consumers land on a setting where the documentation autofix receives no threads.
review details
found by correctness-reviewer| // roster uses the tier at all is unobservable from posted output alone | ||
| // (under-use looks exactly like the pre-tier surface), so every plan | ||
| // records its post-veto medium count, zero included. | ||
| notes.push( |
There was a problem hiding this comment.
question (non-blocking): The tier's only calibration instrument doesn't reach the repo's cross-run aggregation surface. The post-veto medium count lands in submission-plan.json's notes, and counters-report.ts — this repo's existing cross-run report — never reads notes (grep for it there returns only an unrelated comment), so "a week of zeros means prompt iteration" requires opening each run's artifact by hand. Would the counters report be the natural home for a signal whose whole job is to be noticed across runs?
review details
found by first-principles
58e4bd9 to
d225ae3
Compare
…rvives dedup, the veto widens to removal-adjacent lines
|
Pushed 188e970 addressing the review:
On the doc-autofix note (blocking-medium collapses documentation suggestions since the doc reviewer never carries importance): true, pre-existing under blocking-only, and #383's body-sourced work list is the fix that makes those reachable again; that's also where the counters-report question about the medium count's cross-run home belongs, logged for the tuning pass rather than bolted on here. |
…rvives dedup, the veto widens to removal-adjacent lines
188e970 to
0b19322
Compare
…rvives dedup, the veto widens to removal-adjacent lines
0b19322 to
ab49fa4
Compare
…rvives dedup, the veto widens to removal-adjacent lines
ab49fa4 to
895bbff
Compare
…rvives dedup, the veto widens to removal-adjacent lines
895bbff to
5c85031
Compare
There was a problem hiding this comment.
Approved — no blocking issues found.
9 of 11 prior review threads resolved; 2 still unaddressed as of 5c85031:
2 non-blocking threads still open
- question (non-blocking)
workflows/review/lib/submission.ts:425: The tier's only calibration instrument doesn't reach the repo's cross-run aggregation surface. The post-veto medium coun... - note (non-blocking)
workflows/review/lib/submission.ts:743: The blocking-medium gate runs ahead of the documentation-label exemption. The exemption below it exists because the docu...
Non-blocking observations (8)
workflows/review/lib/submission.ts:390suggestion (non-blocking): The blocking-medium full-depth escape has no test, unlike its blocking-only sibling. (test-adequacy)workflows/review/lib/dispatch-contracts.ts:764suggestion (non-blocking): The shipped veto accepts removal-adjacent anchors, but every reviewer-facing contract says "added line" only. (completeness)workflows/review/lib/submission.ts:397suggestion (non-blocking, documentation): The veto comment says "added line" but the veto also keeps removal-adjacent anchors. (documentation)workflows/review/lib/submission.ts:403note (non-blocking): The veto's scoped-diff branch misses flip-gated depth, the other reduced depth blocking-medium covers. (correctness-reviewer)workflows/review/lib/dedup.ts:247note (non-blocking): The medium tie-break outranks confidence, but the merge keeps the elected copy's lower confidence. (correctness-reviewer).changeset/pra7-severity-tier.md:5question (non-blocking): The changeset (and the consumer-facing README paragraph it mirrors) describes the medium veto as stripping the tier from "any claim not anchored on an added diff line", butapplyMediumVetoin workflows/review/lib/dispatch-contracts.ts also keeps the tier on removal-adjacent anchors (file.removedAdjacent.includes(claim.line), dispatch-contracts.ts line 764, and the JSDoc at lines 736-740 states the wider rule explicitly). (skill-auditor (out-of-lane))workflows/review/lib/submission.ts:743thought (non-blocking): The motivating collapse comes from this repo's ownblocking-onlyline, and the fix is gated on a signal the PR calls uncalibrated. (first-principles)workflows/review/lib/dedup.ts:247question (non-blocking): The new medium tiebreak insurvivorFirstruns ahead of the confidence tiebreak, so cross-source dedup now elects a different survivor among same-severity copies — which changes the posted comment's prose and its attribution footer — while the changeset states the full-depth output-shape effect is "none by itself (medium only reorders which non-blocking claims take the existing budget's slots)". (skill-auditor (out-of-lane))
review details
review-v1.19.0 | schema 2 | depth scoped | re-review scoped blocking-only | enable holistic,completeness,test-adequacy,first-principles,conventions,documentation
Review live A/BBaseline: Ruler: matcher deterministic+arbiter; corpus 0441b7c6a581 (10 cases).
Adversarial hard gate: PASSED on the candidate arm. Single-run-stable rows: recall, verdict agreement, regressions, adversarial gate. Judge quality and noise are not: they jitter run-to-run at this corpus size, and a regressed reviewer can score HIGHER on judge quality (fewer, surer comments each read better). Recall against the labeled specs is the load-bearing metric. Measured noise floor (identical arms, run 29069228968, 2026-07-10, 6 arm-samples, full corpus x3, pre-arbiter; budget skips left the samples on unequal case sets, so these v1 bands also carry case-mix variance): must-catch recall 54%-86% (sd 10%), verdict agreement 75%-100% (sd 9%), noise (unmatched posted) 50%-60% (sd 3%), judge mean quality 82%-86% (sd 2%). A single-run delta whose arms both sit inside a band is indistinguishable from run-to-run wobble; use |
…-proposed, validator-adjudicated, code-vetoed
…rvives dedup, the veto widens to removal-adjacent lines
5c85031 to
cdd1ac0
Compare
2 participants
Stacked on #380. The other half of the PRA-7 fix: the tier that decides which non-blocking findings deserve the inline slots, so a verified "your new guard never fires" stops collapsing behind a bare count on an approving re-review (this repo's #367, #371, #366 on 08-24; the retroactive corpus read is records/pra7-collapsed-tail-2026-08-24.md in the plans repo).
The design splits the tier call three ways, so no single party owns it:
"importance": "medium"field; the structured lens schema gains amediumseverity. The bar, verbatim in every contract: a verified defect or gap in code this PR adds, one a reasonable author would fix before merge. Anything but the literalmediumreads as minor, fail-quiet; a drifted value never voids a finding. first-principles, conventions, and documentation don't get the field (advisory by charter; documentation is budget-exempt anyway).corrected.importanceworks both directions on confirmed claims (strip an over-marked medium, grant an under-marked one);plausibleand the author-dispute cap strip it mechanically, since unconfirmed fails the tier's "verified" test. This is deliberately not a new judge pass: the validator already does the verification work a severity call needs, so the tier costs a few hundred prompt tokens, not a roster seat.applyMediumVetostrips the tier (never the claim) from anything not anchored on an added line of the staged diff, applied at the posting surface after validation so code has the last word. On the 08-24 corpus it barely binds (86 of 91 collapsed entries already anchored to added lines), which is the point: a cheap floor, not a daily filter.Surface mechanics: medium outranks minor for #380's non-blocking budget, and a new ROUTING modifier
re-review <mode> blocking-mediumlets medium findings post inline on reduced-depth re-reviews; strictblocking-onlystays as the per-consumer rollback dial if the tier inflates somewhere. No new labels, which is what keeps the verdict, recap parser, dedup guards, and flip gate untouched by construction (medium renders with the same non-blocking labels advisory does, and the schema enum extension needs no version bump since it invalidates nothing serialized).The known unknown is day-one calibration: nothing measures the roster's severity judgment yet, and the one data point (the dead-guard finding labeled a mere note) says it under-grades. Inflation is bounded by machinery (validator strip, veto, the 3-slot budget), but under-use is silent, so every submission plan now notes its post-veto medium count, zero included; a week of zeros means prompt iteration, and the 367/371/366 findings are the known-answer cases for whether the contract wording lands.
KORE-2464