Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

The eager-closure checker publishes per-chunk and headroom verdicts to $GITHUB_OUTPUT that the Bundle Analysis PR comment never reads #6230

Closed

Description

@yinlianghui-tw

Found while implementing #5924 on 48e53814e. Filed unassigned, observation class — not graded, not a bug in anything that ships.

What is there

scripts/check-eager-closure-budget.mjs writes five keys to $GITHUB_OUTPUT:

closure_status, closure_gzip_kb, closure_budget_kb, closure_chunks, closure_chunk_status

.github/workflows/performance-budget.yml passes only four of them into the comment step's env:

BUDGET_CLOSURE_STATUS:     ${{ steps.budget.outputs.closure_status }}
BUDGET_CLOSURE_GZIP_KB:    ${{ steps.budget.outputs.closure_gzip_kb }}
BUDGET_CLOSURE_BUDGET_KB:  ${{ steps.budget.outputs.closure_budget_kb }}
BUDGET_CLOSURE_CHUNKS:     ${{ steps.budget.outputs.closure_chunks }}

closure_chunk_status — added by PR #6210 with the per-chunk half — is written and never read. PR #6229 (this card's neighbour, for #5924) adds closure_headroom_status the same way, deliberately following the existing shape rather than diverging from it inside an unrelated card; so on main after both land there will be two published verdicts that no consumer reads.

Why it is worth a line rather than nothing

Nothing is broken and no verdict is lost: all three halves print to the step log, and the exit code still decides budget_status, which the comment does render. The cost is narrower — a reader of the PR comment sees "eager closure: pass, 3222.6 KB of 3266.6 KB" and cannot tell from the comment whether the per-chunk half or the sensitivity half was the one that objected, only that something did. That is the same class of thing #3152 was about: the comment inferring a verdict it was not handed.

Two dispositions, both cheap, and this is deliberately not choosing between them:

  1. Wire both keys into the comment env and render them, so the comment names which half objected.
  2. Decide the step log is the right home for the per-half detail and drop the two unread outputs, so the checker does not publish what nothing consumes.

⛔ What should not happen is a third round of this: a future half added to the checker, publishing a third unread key because the previous two set the precedent.

Refs: #5324 (the gauge) · #5490 / PR #6210 (closure_chunk_status) · #5924 / PR #6229 (closure_headroom_status) · #3152 (the comment renderer inferring a verdict).

Activity

  1. yinlianghui-tw commented on Aug 25, 2026

    @yinlianghui-tw
    ContributorAuthor

    Claiming this card.

    • session: session_019b5UBNMtTzKbVtZZGvFuxe
    • branch: claude/issue-6230-budget-comment-half-status (pushed, based on 0409b766d)

    Taking disposition 1 from the body: wire the unread verdicts into the comment env and render them, so the comment names which half objected. Reporting back on this card when the draft PR is open.

    Generated by Claude Code


    Generated by Claude Code

  2. yinlianghui-tw commented on Aug 25, 2026

    @yinlianghui-tw
    ContributorAuthor

    os-dev-report

    {
      "issue": 6230,
      "status": "done",
      "branch": "claude/issue-6230-budget-comment-half-status",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6242",
      "premise_still_valid": true,
      "summary": "Took disposition 1: wired closure_chunk_status and closure_headroom_status into the comment step's env in performance-budget.yml and rendered all three half verdicts in render-budget-comment.mjs. The breakdown appears ONLY when a half is not pass, so the healthy comment is byte-for-byte unchanged. Two things beyond the card's description: (a) closure_status was already passed into the renderer and never used - verdictBody read closure.gzipKb/budgetKb/chunks but never closure.status - so before this change NO half verdict was rendered, not even the wired one; (b) exit 2 maps to budget_status=error, which is not in MEASURED_STATUSES, so a drifted ceiling lands in notMeasuredBody, not the verdict body. Rendering the halves there exposed a sentence that is false on that path ('Nothing was measured') sitting directly above a table showing two ceilings that had just measured fine, so that branch now discriminates on the same emptiness the verdict branch keys on (the checker publishes closure_gzip_kb empty when it has no report, pinned by its own tests): measured plus error reads as 'gauge not trustworthy', genuinely-absent keeps the existing wording. The exit-code mapping is untouched and proven so mechanically. Six files' worth of gates green; empty-frontmatter changeset per the repo's CI-only precedent (objectui has no skip-changeset label, and I did not create one).",
      "tests": "All at final commit af60a25b8. FOUR LEGS, each predicted before running, rendered from the env each scenario produces against BOTH this renderer and the pre-change one read from the 0409b766d git blob (no working-tree mutation, no ablation needed): (1) all three pass - `cmp` clean, byte-for-byte identical to the old renderer, green comment unchanged; (2) aggregate fail - names 'Aggregate closure ceiling / over its ceiling', other two pass; (3) aggregate pass + chunk fail - names 'Per-chunk ceilings / over its ceiling' while the aggregate row reads pass. 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; (4) headroom error - kind 'not-measured', 'Ceiling sensitivity (headroom) / broken gauge', zero cross-mark glyphs, no 'Nothing was measured'. All four matched prediction. REVERSE-VERIFICATION of the new contract guard, its logic applied to the pre-change workflow blob: 'OLD workflow (0409b766d): RED - unread: closure_chunk_status, closure_headroom_status / NEW workflow (this branch): GREEN - every published verdict is wired'. Direction as predicted: reds on exactly the two keys the card reports. EXIT-CODE MAPPING pinned by diffing content-only greps of every budget_status= / CLOSURE_CODE / ENTRY_OVER / MAX_ENTRY_GZIP_KB= / exit-1 line against 0409b766d: 'IDENTICAL - every exit-code / budget_status line is unchanged' (only line numbers moved, by the 9 comment lines added above the checker invocation). SUITES - the four under scripts/__tests__ that read these two files, derived from disk by grepping for the two filenames: render-budget-comment, check-eager-closure-budget, ci-cd-pipeline-doc, turbo-build-inputs => 'Test Files 4 passed (4) / Tests 167 passed (167)', vitest EXIT=0 captured by redirect before any pipe. All 7 new tests confirmed COLLECTED BY NAME in a --reporter=verbose run rather than inferred from the total. GATES, each quoted from its own printed verdict line, never a bare exit status: pnpm type-check:scripts exit 0 with the script name echoed (guards against the objectui zero-match --filter trap - note this repo spells it type-check, hyphenated); pnpm lint:root run UNNARROWED over the whole root scope => 'x 28 problems (0 errors, 28 warnings)' exit 0, with zero occurrences of my files in the output and all 28 warnings pre-existing and elsewhere (so no narrowing needed to be declared); 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 ratchets unmoved; check-vi-mock-specifiers OK; check:esm-specifiers no un-ledgered extensionless relative specifier; changeset:check all three lines green. Workflow re-parsed with a real YAML parser and the render step's env block dumped: all six closure keys present. Heavy commands ran through scripts/pm/os-verify-lock.sh; longest hold 7s, no waits.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #6243: invoked-as.mjs's 'Nothing here enforces this yet' header section is stale in both halves - it states 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 file(s) still hand-type one'. The issue it names as tracking both halves (#6092) is closed. Aggravating: line 17 of the same header instructs readers to read that section 'before concluding otherwise', so a reader is actively told a gated rule is unenforced. Filed unassigned, finding label, no pm:queue."
      ]
    }

    Notes on the card's two premises, both of which held:

    • The issue body lists five $GITHUB_OUTPUT keys; there are now six on origin/main. That is not an error in the card — it was written before test(scripts): derive the eager-closure headroom check from the report, and re-baseline the aggregate ceiling #6229 landed, and the body predicts exactly this ("on main after both land there will be two published verdicts that no consumer reads"). Confirmed from writeGithubOutput at check-eager-closure-budget.mjs:888, not from a log.
    • The three verdict key names were correct as given. closure_headroom_status is pass | error only — evaluateHeadroomSensitivity never returns fail, deliberately (its header: a negative headroom is an over-budget bundle, which the other two halves own). The renderer handles all three statuses generically anyway, plus an unknown value, so a future fail there needs no renderer change.

    PR #6242 is draft; CI convergence is the PM's to read.

    Generated by Claude Code


    Generated by Claude Code

  3. claude commented on Aug 25, 2026

    @claude
    Contributor

    Triage state repair (concentrated round, mechanical): this card is claimed/in flight, so the stale finding label (which means awaiting first grading) is removed to keep the ungraded count honest. No other change; the claim and its labels are untouched.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

ci/cddomain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repopm:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions