[PR #162] [MERGED] feat(pr-risk): complement grade + reducibility verdict in the concentration sentence (BE-7378) #171

New issue

Closed

opened 2026-10-26 22:37:00 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:37:00 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/162
Author: @mattmillerai
Created: 8/13/2026
Status: ✅ Merged
Merged: 8/13/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-7378-complement-grade

📝 Commits (3)

25891f9 feat(pr-risk): complement grade + reducibility verdict in the concentration sentence (BE-7378)
6acbca4 fix(pr-risk): gate the reducibility clause on the printed share, label it from the record (BE-7378)
81bd375 test(pr-risk): make the not-path-decided fixture gradable (BE-7378)

📊 Changes

3 files changed (+134 additions, -9 deletions)

View changed files

📝 scripts/pr-risk/README.md (+18 -1)
📝 scripts/pr-risk/publish-risk-surfaces.sh (+52 -8)
📝 scripts/pr-risk/tests/test_publish_risk_surfaces.sh (+64 -0)

📄 Description

ELI-5

The risk grader already tells you "94% of this diff is R0/R1/R2; the 6% that puts the path floor at R3 is 1 file, 40 lines." That is half an answer. It does not say what the other 94% would grade as if you peeled the risky file out — and that is the whole decision: split into two PRs, or don't bother. The same sentence is true whether the remainder rubber-stamps at R1 or is still a normal R2 review.

Now it says: "…is 1 file(s), 40 lines; peeled into their own PR, the remaining 1 file(s) would path-floor at R0 (final grade still depends on provenance and checks at PR time)."

What changed

publish-risk-surfaces.sh — the render program computes the complement floor: the worst per-file path floor over the files strictly below the overall floor (exactly the set the sentence already counts as the below-floor share). It is appended to the concentration sentence when the path axis is what decided the tier and that remainder is non-empty.
The clause is a floor with its assumptions named, never a promised grade — the remainder's own provenance (a human-authored split floors at R1) and its future check rollup are unknowable at render time, so the parenthetical says so.
The per-file floors come from grade-pr-risk.sh and are re-derived here by the same worst-of rule the floor itself uses. This script computes no risk of its own and does not become a second grading model.
$drivers moved above the sentence (it now gates both conditional clauses, from opposite sides), and $driver's path branch now reads the shared $path_decided predicate instead of re-spelling it — so the <details> cannot offer a split under a headline that credits another axis.
README publish-surfaces section documents the clause and its silences.

Where it stays silent

Irreducible — every changed line already at the floor. The existing "All N changed lines sit at R3" wording is the whole answer; there is nothing to peel.
Path axis did not decide — provenance or reversibility supplied the headline, so peeling the top path files leaves the tier where it is. The existing axis-attribution tail already covers that case.
A tie — another axis proposes the same tier as the floor. Rank equality alone would read this as "path decided"; it is not, because the remainder is still held at that tier and a promised reduction would be the one wrong answer. This is the case the guard exists for.
Ungraded / unknown records never reach the sentence.

Verification

All six hermetic pr-risk suites green, 386 assertions, 0 failed (test_publish_risk_surfaces.sh 76 — was 66, +10 new). shellcheck -x clean on both changed shell files.

Ten new hermetic assertions cover complement→R0, →R1 (worst-of over a mixed remainder, proving it is not "biggest file" or "last file"), →R2 (the ticket's worked example: the split that is not worth much, reported honestly), irreducible, non-path-decided, the tie, and the ungraded record.

Byte-identity was measured, not asserted from my own premise. I ran the origin/main render program and this one side by side over twelve synthetic records: the whole surfaces object (check_title, check_summary, comment_body, concentration, tier, graded) is byte-identical on all nine suppression records, and on the three path-decided ones exactly one line differs — the concentration sentence. That is what rules out a regression from the $driver refactor, which no unit test of mine could establish on its own.

Bounds kept

Markdown escaping is unchanged and untouched by this clause: everything it interpolates is an integer ($below | length, and a rank from tier_rank, which maps any unrecognised tier to 3 rather than passing text through), so no new PR-controlled string reaches the body. The added text is a fixed ~140 characters regardless of file count, so the 65536-char comment bound, the row-dropping and the unconditional truncation backstop are all unaffected. RENDER_ONLY=1 still writes nothing. No change to grading semantics, the label sync, or the record schema.

Judgment calls

The above-the-fold headline ($conc_short) is deliberately unchanged. It stays "6% of 640 changed lines carry it (1 file(s))." The file header defends a one-line visible budget on a surface that already sits under CodeRabbit and an 8-cell review panel, and the ticket's acceptance criteria name the concentration sentence. Promoting the complement above the fold is a real product question — worth doing if the v1 /derisk rung wants it, not worth spending the line here unasked.
A tie suppresses the clause. The ticket says "when the path axis decided" without settling the tie case; I read a tie as not-decided-by-path, matching the predicate $conc_short already gates on. Suppressing is the conservative direction — it withholds an offer rather than promising a reduction that peeling would not deliver.
Renames verified, no change needed (spec item 4): grade-pr-risk.sh matches each per-file floor against destination and origin path, so a rename out of a guarded class keeps that class's tier on its row and lands at-or-above the floor — it never appears in the reducible remainder. No regression, and no new feature was added for it.
Negative-claim falsification: the trigger does not fire. The diff adds no "not supported"/"unavailable"/"STOP" string, no throw or deny dead-end, and flips no test to assert a dead-end. The user-facing outcome is purely additive advisory text; the hasnt assertions scope new text rather than denying an existing capability, and nothing that worked before stops working.

All five acceptance criteria are met; none unmet.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/162 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/13/2026 **Status:** ✅ Merged **Merged:** 8/13/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-7378-complement-grade` --- ### 📝 Commits (3) - [`25891f9`](https://github.com/Comfy-Org/github-workflows/commit/25891f9f2d116acf468f69bc74b1f25a64f276ae) feat(pr-risk): complement grade + reducibility verdict in the concentration sentence (BE-7378) - [`6acbca4`](https://github.com/Comfy-Org/github-workflows/commit/6acbca4e3d0981980d0e0e3a1b327f6b0bccbe92) fix(pr-risk): gate the reducibility clause on the printed share, label it from the record (BE-7378) - [`81bd375`](https://github.com/Comfy-Org/github-workflows/commit/81bd375cc3f2a2aca24139b9cdde2119ac856034) test(pr-risk): make the not-path-decided fixture gradable (BE-7378) ### 📊 Changes **3 files changed** (+134 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `scripts/pr-risk/README.md` (+18 -1) 📝 `scripts/pr-risk/publish-risk-surfaces.sh` (+52 -8) 📝 `scripts/pr-risk/tests/test_publish_risk_surfaces.sh` (+64 -0) </details> ### 📄 Description ## ELI-5 The risk grader already tells you "94% of this diff is R0/R1/R2; the 6% that puts the path floor at R3 is 1 file, 40 lines." That is half an answer. It does not say what the other 94% would grade as if you peeled the risky file out — and that is the whole decision: split into two PRs, or don't bother. The same sentence is true whether the remainder rubber-stamps at R1 or is still a normal R2 review. Now it says: "…is 1 file(s), 40 lines; peeled into their own PR, the remaining 1 file(s) would path-floor at **R0** (final grade still depends on provenance and checks at PR time)." ## What changed - `publish-risk-surfaces.sh` — the render program computes the **complement floor**: the worst per-file path floor over the files strictly below the overall floor (exactly the set the sentence already counts as the below-floor share). It is appended to the concentration sentence when the path axis is what decided the tier and that remainder is non-empty. - The clause is a **floor with its assumptions named, never a promised grade** — the remainder's own provenance (a human-authored split floors at R1) and its future check rollup are unknowable at render time, so the parenthetical says so. - The per-file floors come from `grade-pr-risk.sh` and are re-derived here by the same worst-of rule the floor itself uses. This script computes no risk of its own and does not become a second grading model. - `$drivers` moved above the sentence (it now gates both conditional clauses, from opposite sides), and `$driver`'s path branch now reads the shared `$path_decided` predicate instead of re-spelling it — so the `<details>` cannot offer a split under a headline that credits another axis. - README publish-surfaces section documents the clause and its silences. ## Where it stays silent - **Irreducible** — every changed line already at the floor. The existing "All N changed lines sit at R3" wording is the whole answer; there is nothing to peel. - **Path axis did not decide** — provenance or reversibility supplied the headline, so peeling the top path files leaves the tier where it is. The existing axis-attribution tail already covers that case. - **A tie** — another axis proposes the *same* tier as the floor. Rank equality alone would read this as "path decided"; it is not, because the remainder is still held at that tier and a promised reduction would be the one wrong answer. This is the case the guard exists for. - Ungraded / `unknown` records never reach the sentence. ## Verification All six hermetic pr-risk suites green, 386 assertions, 0 failed (`test_publish_risk_surfaces.sh` 76 — was 66, +10 new). `shellcheck -x` clean on both changed shell files. Ten new hermetic assertions cover complement→R0, →R1 (worst-of over a mixed remainder, proving it is not "biggest file" or "last file"), →R2 (the ticket's worked example: the split that is *not* worth much, reported honestly), irreducible, non-path-decided, the tie, and the ungraded record. **Byte-identity was measured, not asserted from my own premise.** I ran the `origin/main` render program and this one side by side over twelve synthetic records: the whole surfaces object (`check_title`, `check_summary`, `comment_body`, `concentration`, `tier`, `graded`) is byte-identical on all nine suppression records, and on the three path-decided ones exactly one line differs — the concentration sentence. That is what rules out a regression from the `$driver` refactor, which no unit test of mine could establish on its own. ## Bounds kept Markdown escaping is unchanged and untouched by this clause: everything it interpolates is an integer (`$below | length`, and a rank from `tier_rank`, which maps any unrecognised tier to 3 rather than passing text through), so no new PR-controlled string reaches the body. The added text is a fixed ~140 characters regardless of file count, so the 65536-char comment bound, the row-dropping and the unconditional truncation backstop are all unaffected. `RENDER_ONLY=1` still writes nothing. No change to grading semantics, the label sync, or the record schema. ## Judgment calls - **The above-the-fold headline (`$conc_short`) is deliberately unchanged.** It stays "6% of 640 changed lines carry it (1 file(s))." The file header defends a one-line visible budget on a surface that already sits under CodeRabbit and an 8-cell review panel, and the ticket's acceptance criteria name the concentration *sentence*. Promoting the complement above the fold is a real product question — worth doing if the v1 `/derisk` rung wants it, not worth spending the line here unasked. - **A tie suppresses the clause.** The ticket says "when the path axis decided" without settling the tie case; I read a tie as not-decided-by-path, matching the predicate `$conc_short` already gates on. Suppressing is the conservative direction — it withholds an offer rather than promising a reduction that peeling would not deliver. - **Renames verified, no change needed** (spec item 4): `grade-pr-risk.sh` matches each per-file floor against destination *and* origin path, so a rename out of a guarded class keeps that class's tier on its row and lands at-or-above the floor — it never appears in the reducible remainder. No regression, and no new feature was added for it. - **Negative-claim falsification: the trigger does not fire.** The diff adds no "not supported"/"unavailable"/"STOP" string, no throw or deny dead-end, and flips no test to assert a dead-end. The user-facing outcome is purely additive advisory text; the `hasnt` assertions scope *new* text rather than denying an existing capability, and nothing that worked before stops working. All five acceptance criteria are met; none unmet. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:37:00 +00:00