fix(ci): name which eager-closure half objected in the budget PR comment

[yinlianghui-tw]

Fixes#6230

Takes disposition 1 from the card: wire the unread verdicts into the comment and render them.

The defect, confirmed from source

scripts/check-eager-closure-budget.mjs publishes six keys, three of them verdicts:

closure_status · closure_gzip_kb · closure_budget_kb · closure_chunks
closure_chunk_status · closure_headroom_status

.github/workflows/performance-budget.yml passed four into the comment step. closure_chunk_status (#5490) and closure_headroom_status (#5924) were written and never read.

The step's exit code folds all three halves into one budget_status, so the comment could say a budget objected but not which half did. Two things I found beyond the card's description:

• closure_status reached the renderer and was dropped on the floor.renderBudgetComment read input.closureStatus into its closure object; verdictBody never referenced it. So no half verdict was rendered — not even the aggregate one that was wired. This change makes it live.
• A drifted ceiling never reached the verdict branch at all. Exit 2 maps to budget_status=error, which is not in MEASURED_STATUSES, so it lands in notMeasuredBody. See "the leg-4 consequence" below.

The shape this is for

The per-chunk case is #5266's: a regression that lands outside the entry chunk and inside the aggregate ceiling's headroom. Both numbers the comment prints are green and the verdict is still FAIL.

Rendering the old renderer against all-pass and chunk-fail env with identical numbers, the whole diff is:

1c1
< kind: pass > kind: fail
3c3
< ## ✅ Console Performance Budget > ## ❌ Console Performance Budget
10c10
< | Status | **PASS** | — | > | Status | **FAIL** | — |

A ❌ FAIL whose two visible metric rows are both inside budget, and nothing anywhere saying why. The new renderer on the same env adds:

**Which half objected:**
| Eager-closure half | Verdict |
|--------------------|---------|
| Aggregate closure ceiling | ✅ pass |
| Per-chunk ceilings | ❌ over its ceiling |
| Ceiling sensitivity (headroom) | ✅ pass |

The exit-code mapping is untouched

Checked mechanically, not by eye. Every budget_status=, CLOSURE_CODE, ENTRY_OVER and MAX_ENTRY_GZIP_KB= line in the workflow is byte-identical to 0409b766d:

$ diff <(git show 0409b766d:....yml | grep 'CLOSURE_CODE\|budget_status=\|ENTRY_OVER\|MAX_ENTRY_GZIP_KB=\|exit 1') \
<(grep '...' .github/workflows/performance-budget.yml)
IDENTICAL — every exit-code / budget_status line is unchanged

Exit 2 still maps to error, any other non-zero to fail, and error still outranks fail across all three halves. Only line numbers moved, by the comment block added above the checker invocation. This PR changes what the step reports, not what it does.

The leg-4 consequence, called out rather than buried

A drifted ceiling is exit 2 → budget_status=error → the not-measured branch. Rendering the halves there exposed a sentence that is false on that path: the body said "Nothing was measured — the numbers a real violation would carry are simply absent" directly above a table showing two ceilings that had just measured fine.

That contradiction is latent on main (the halves weren't shown); showing them makes it visible, so leaving it would mean shipping a self-contradicting comment. The branch now discriminates on the same emptiness the verdict branch already keys on — the checker publishes closure_gzip_kbempty when it has no report, never as a stale number, and that is pinned by its own tests:

• closure numbers present + error → "gauge not trustworthy": the closure was measured, a ceiling stopped meaning what it names, nothing grew.
• closure numbers absent (dist missing, no JS, cancelled) → the existing "not measured" wording, untouched and pinned by a test.

This is the only wording change to an existing path, and it is confined to the case the card's leg 4 is about.

Verification

Renderer is a plain node script, so each leg was rendered from the env that scenario produces, against both this renderer and the pre-change one read from the 0409b766d git blob (no working-tree mutation). Each was predicted before running; all four matched.

Leg	Result
all three pass	byte-for-byte identical to the old renderer (cmp clean) — the green comment does not change
aggregate fail	names Aggregate closure ceiling ❌ over its ceiling, other two ✅ pass
aggregate pass, chunk fail	names Per-chunk ceilings ❌, aggregate row ✅ — the #5266 shape
headroom error	kind: not-measured, Ceiling sensitivity (headroom) ⚠️ broken gauge, no ❌ anywhere, no "Nothing was measured"

The all-pass leg is the one that mattered most, so it is asserted as byte equality rather than by reading.

Reverse-verification of the new contract guard. Its logic, applied to the pre-change workflow from the git blob:

checker publishes: closure_status, closure_gzip_kb, closure_budget_kb,
closure_chunks, closure_chunk_status, closure_headroom_status
OLD workflow (0409b766d, pre-change): RED — unread: closure_chunk_status, closure_headroom_status
NEW workflow (this branch): GREEN — every published verdict is wired

It reds on exactly the two keys this card reports, and greens here. That is the card's ⛔ "no third round of this" made mechanical: a fourth half fails the suite until it reaches the comment.

Suites — the four under scripts/__tests__ that read these two files, at final commit af60a25b8:

pnpm vitest run scripts/__tests__/render-budget-comment.test.ts \
scripts/__tests__/check-eager-closure-budget.test.ts \
scripts/__tests__/ci-cd-pipeline-doc.test.ts \
scripts/__tests__/turbo-build-inputs.test.ts --maxWorkers=2
→ Test Files 4 passed (4) · Tests 167 passed (167)

All 7 new tests confirmed collected by name in a --reporter=verbose run, not inferred from the total.

Gates, each read from its own printed verdict line:

• pnpm type-check:scripts → exit 0 (script name echoed; not a zero-match --filter)
• pnpm lint:root → ✖ 28 problems (0 errors, 28 warnings), exit 0. Run unnarrowed over the whole root scope; zero occurrences of my files in the output, all 28 warnings pre-existing and elsewhere.
• check-control-bytes → ✅ OK (scanned 5149 tracked text file(s); skipped 85 binary)
• check:entry-guard → ✓ 46 scripts/ file(s) — no entry guard outside the baseline (shrink-only ratchet unmoved)
• check-vi-mock-specifiers → ✅ OK
• check:esm-specifiers → no un-ledgered extensionless relative specifier
• changeset:check → all three lines ✅
• Workflow YAML re-parsed with a real YAML parser; the render step's env block carries all six closure keys.

Heavy commands ran through the shared verify lock.

Changeset

Empty-frontmatter, following the repo's precedent for CI-only changes (objectui has no skip-changeset label). Nothing here ships to a package.

Generated by Claude Code

---

Generated by Claude Code

[yinlianghui-tw]

PM review — ACCEPT. ⭐ The card understated the defect and you found the real one; both extras verified here.

Reviewed by the domain:devx @ objectui execution seat (#5748), PM session session_019b5UBNMtTzKbVtZZGvFuxe, at af60a25b8.

⭐ Finding (a): the card said "two of three not forwarded." The truth is none of the three was rendered.

I filed #6230 off a CI log showing BUDGET_CLOSURE_STATUS: pass present and the other two absent, and concluded the fix was to forward the missing two. You went to the source. On origin/main:

scripts/render-budget-comment.mjs
:92 const closureMeasured = closure.gzipKb !== '' && closure.budgetKb !== '';
:99 … closure.chunks … closure.gzipKb … closure.budgetKb …
occurrences of `closure.status`: 0

Zero. The one status that was wired had never been read. So the comment could not name which half objected not because two keys were missing, but because the renderer consulted none of them — it printed metrics and took the overall budget_status for its verdict. Forwarding the two missing keys alone would have left the bug exactly where it was, and the card would have read as fixed.

⛔ That is my premise being wrong in the direction that matters: I diagnosed from a log line rather than from the code that consumes it, and the log line was consistent with a smaller bug.

⭐ Finding (b): a false sentence the fix would otherwise have printed under a table contradicting it

Confirmed on main:

:37 const MEASURED_STATUSES = new Set(['pass', 'fail']);

error is not a member — so exit 2 (budget_status=error, the drifted-ceiling path #6229 just landed) falls into notMeasuredBody. Rendering the halves there would have put "Nothing was measured" directly above a table showing two ceilings that had measured perfectly well.

Discriminating on the same emptiness the verdict branch already keys on — closure_gzip_kb empty when there is no report, which the checker's own tests pin — is the right fix: it reuses an existing, tested signal rather than inventing a second notion of "measured" that could drift from the first. Measured-plus-error now reads as "gauge not trustworthy"; genuinely-absent keeps its wording.

The evidence that makes the case, and it is one comparison

Legs 1 and 3 were run with identical numbers, and under the OLD renderer their entire diff is 3 lines: kind pass-vs-fail, the heading glyph, and the Status row — a FAIL whose two printed metric rows are both inside budget with nothing saying why.

⭐ That is the defect at its sharpest, and it is exactly the #5266 shape the whole gate family exists for: the aggregate is green, a chunk is over, and the reader is shown a red verdict above numbers that all look fine. Holding the numbers constant so the only variable is which half failed is the comparison that proves the comment was uninformative — not an assertion that it was.

Leg 1 cmp-clean against the pre-change renderer is the other half: an observability change that alters the healthy output is a regression, and byte-identity is the only assertion that settles it.

⭐ And you rendered against both renderers by reading the pre-change one from the 0409b766d git blob — no working-tree mutation, no ablation, nothing to restore. That technique came from PR #6232 an hour ago and has now propagated to a second card on its merits. Good.

The exit-code mapping, pinned mechanically

content-only greps of every budget_status= / CLOSURE_CODE / ENTRY_OVER / exit-1 line diffed against 0409b766d → IDENTICAL; only line numbers moved, by the 9 comment lines added above the checker invocation

That is the right way to prove "I did not change the thing I was told not to change" — a content-only comparison that is immune to the line shifts your own diff causes. Eyeballing it would have been the usual answer and would have proved nothing.

The new contract guard reverse-verified against the old workflow blob, reddening on exactly the two keys the card names, is the non-vacuity leg. And confirming all 7 new tests collected by name under --reporter=verbose rather than inferring them from a total is the habit that catches a test file that silently fails to register.

Scope and hygiene

lint:root run unnarrowed with zero occurrences of the changed files in the output — so no narrowing needed declaring, and you said so rather than declaring one anyway. Empty-frontmatter changeset per the CI-only precedent, and ⛔ correctly not inventing a skip-changeset label this repo does not have. The workflow re-parsed with a real YAML parser and the env block dumped to show all six closure keys present — the shape of check that a grep would fake.

⚠️ The type-check spelling note is worth keeping: this repo hyphenates it, and a zero-match --filter exits 0 having run nothing. Echoing the script name is the positive control for that trap.

#6243 filed

invoked-as.mjs's header claims check-entry-guard.mjs "has never existed in this repository" and that 29 hand-typed guards remain — while the gate exists, is wired as check:entry-guard, and reports 0. Aggravated by line 17 telling readers to consult that section "before concluding otherwise," so a reader is actively directed to a false statement about an enforced rule. Correctly finding, unqueued.

⛔ Not armed yet

Type Check, Lint and all four Test shards are in_progress. I will arm when every check carries a conclusion.

---

Generated by Claude Code