feat(doris-repo-review): flag regressions per finding and floor them at Major

[morningman]

Why

The 2026-09-13 local review of apache/doris#67900 (head 5181cba) found that the refactor had dropped the per-attempt reset of the Flight result-location flag, wrote it up correctly (F-02), and rated it Minor because the trigger was narrow ("Flight + replan + a failing retry") and the fix was one line. The verdict stayed APPROVE, a PASS receipt was posted with that regression as its first note, and the fix threshold downstream ("only Blocker/Major") dropped it. The next reviewer sent the same issue back on head d60a614 as a must-fix.

The finding was not missed — its severity was, and the pipeline had no rule that made the mis-classification impossible. This PR adds that rule and enforces it mechanically.

What changes

• Mandatory regression flag per finding. Every finding now carries - **Regression**: yes | no (ZH - **回归**：是 | 否) right after its severity: does HEAD behave differently from the base in a way the PR body does not declare as intended? verify-review-docs.py rejects a finding without the line, rejects EN/ZH documents whose flags disagree, and reports regressions in its JSON result.
• Severity floor. A regression in a correctness / concurrency / lifecycle / compatibility / config / data category is at least Major; the verifier rejects a regression rated Minor or Nit. For a PR presented as behaviour-preserving ([refactor], "Behavior changed: No", …) every undeclared differing cell of the D1 differential table is such a regression. Severity is the consequence when it triggers, never its probability or the size of the fix (references/doc-templates.md, new "Regression flag and severity floors" section).
• Downgrades need a rationale and a challenge. Rating a candidate below what a subagent proposed, or dismissing a correctness / lifecycle candidate, now requires a written Severity rationale and one severity-challenge subagent (references/prompts.md section E) before it is final (SKILL.md step 6). The ledger skeletons written by prepare-review-context.sh carry the new fields.
• Poster gate. post-pass-comment.sh additionally refuses to post when the verified result carries any regression finding, so a PASS receipt can never sit on top of a known behaviour change even if the two scripts ever disagree. The doris-repo-review/v1 comment schema is unchanged.
• Tests. verify/repo-review covers the new rules: 9 new contract tests (regression rated Minor/Nit rejected, count reported, parenthetical notes accepted, missing line rejected, EN/ZH disagreement rejected, unparseable value rejected, poster refusals). Full suite: 47 PASS.

./verify/repo-review/run.sh

Also validated against real review documents (apache/doris#67916): docs with Regression: no verify as before; flipping one Minor finding to Regression: yes is rejected with finding F-01 is marked as a regression but rated Minor; a regression against the base is at least Major.

Compatibility

Documents produced by the previous template fail verification until the Regression line is added to each finding — that is intended: the line is the judgment this change forces the reviewer to make explicitly.

🤖 Generated with Claude Code

[shuke987]

@morningman Could you confirm whether the following two cases need to be addressed in this PR? This is based on the current commit, f127757.

1. Should a small regression affecting only logs or wording also have to be rated Major?

   The documented rules allow regressions in observability, wording, and tests to be rated according to their impact. However, the verifier does not distinguish categories: every finding marked as a regression must be at least Major.

   For example, a feature PR accidentally omits some diagnostic information from a debug log, affecting only troubleshooting convenience. A reviewer follows the documentation and records “observability / Regression: yes / Minor,” but the verifier rejects the report.

   Is the intended rule “all regressions must be at least Major,” or “only regressions in the specified categories must be at least Major”? Should the documentation and code be aligned? The former could be addressed by removing the documented category exceptions; the latter would require category-aware validation. Posting a PASS receipt can still have its own separate gate.

2. When a PR branch is behind the target branch, could a fix made on the target branch be mistaken for a regression introduced by the PR?

   The new regression assessment compares HEAD with BASE_SHA. Here, BASE_SHA is the latest target-branch commit, whereas the PR diff starts at their common ancestor, MERGE_BASE.

   For example, after the PR branch was created, the target branch received a fix that resets some state. The PR itself only changes a log message. Its HEAD does not yet contain that target-branch fix, so comparing it directly with the latest target branch looks like a “missing reset” and could lead to a Major regression finding under the new rule. But the PR did not remove the reset, and a normal merge would preserve the fix.

   Should regression attribution use MERGE_BASE, or include an additional check to exclude differences caused solely by the PR branch being behind? Compatibility with the latest target branch could be assessed separately.

All 47 existing local tests passed. I also checked the category-validation and diverged-branch scenarios above. I would like to confirm the intended behavior before deciding whether changes are needed.

[morningman]

@shuke987 Thanks — both are real gaps, and both are addressed in 49ae610.

1. Category-aware floor (the documented rule wins; the verifier now enforces it).

The intended rule is the documented one: only regressions in specified categories are floored at Major. The verifier now knows the categories instead of flooring every Regression: yes:

• Every finding carries a mandatory Category / 类别 line from a closed vocabulary (verify-review-docs.py L44-L60, doc-templates.md "Category"). The class names the consequence; the domain (concurrency, lifecycle, …) goes in a parenthesis after it, e.g. functional-loss (lifecycle). An unknown value is rejected rather than defaulted, so a typo cannot slip past the floor; EN and ZH must agree.
• Floored (regression ⇒ at least Major): functional-bug, functional-loss, data-error, resource-leak, performance.
• Judged normally: observability, test-coverage, wording, maintainability — but a Regression: yes rated below Major must carry a Severity rationale paragraph saying why the behaviour change is acceptable; the verifier checks that it exists (L264-L275). Your example (debug log lost some detail → observability / Regression: yes / Minor) now verifies with such a paragraph.
• The PASS gate is separate, as you suggested: the poster refuses a floored regression (only possible if the two scripts ever disagree) and otherwise still posts, but refuses when a non-floored regression would go undisclosed — the receipt must name each one in the notes (post-pass-comment.sh L104-L142). The doris-repo-review/v1 comment schema is unchanged; the verifier JSON gains floored_regressions.

2. Regression attribution uses MERGE_BASE.

You are right that BASE_SHA is the target-branch tip. Regression evidence is now git show $MERGE_BASE:<path> everywhere — SKILL.md premise checks and step 6, the subagent preamble / D1 / severity-challenge prompts ({MERGE_BASE} is a new placeholder), the ledger skeletons, and the template (doc-templates.md "Regression", SKILL.md step 6). BASE_SHA keeps its two jobs: the diff range and the commit the receipt binds to. prepare-review-context.sh records TARGET_AHEAD (target-branch commits since the merge base) in meta.env and warns when it is non-zero (L157-L160); the case is also listed under common traps (SKILL.md). Compatibility with a target branch that has moved on is called out as its own concern, never as Regression: yes.

For what it is worth, the real review of apache/doris#67916 already had this shape: its header shows BASE_SHA ≠ MERGE_BASE, with 5 target-branch commits in between.

Tests. verify/repo-review/run.sh: 63 PASS (was 47) — each floored category rejected at Minor, each non-floored category accepted at Minor with a rationale and rejected without one, the parenthetical domain note, spelling variants, missing / unknown / disagreeing categories, and the poster's disclosure gate. Also replayed against the real #67916 documents: they verify after relabelling with the closed vocabulary; flipping a test-coverage finding to Regression: yes is rejected without a rationale and accepted with one; flipping a functional-bug finding to Regression: yes at Minor is rejected.

Compatibility note: documents from the previous template now also need the Category line, same as the Regression line this PR already required.