From d2ac391979658db875937e142cdd0f3bc1c8dae2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 10:25:53 +0000 Subject: [PATCH] fix(console): return packages/core to the framework chunk by grouping rule, and pin chunk membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `data-adapter` declares `includeDependenciesRecursively: false`, so the group holds exactly what its regex names. The rule that re-attributed the subgraph is rolldown's `includeDependenciesRecursively` (default `true`): a group captures its matched modules AND everything they import, and the same option's priority doc states those are then removed from lower-priority groups whose regex does match them. `packages/data-objectstack` imports `@object-ui/core` and this group outranks `framework` (84 over 80), so the recursive half handed `framework`'s declared members to a group with no ceiling and no baseline. The transfer tracks TREE-SHAKING, which is why it moved with no edit here: while `@object-ui/core` was imported by name only, the slice reachable through `data-objectstack` was a handful of modules; objectui#9185 added an `import('@object-ui/core')` of the BARREL, every export became live, and the same walk reached all 92. Measured, console build either side of the one-line repair: data-adapter 78,110 -> 18,537 gzipped, holding only packages/data-objectstack framework 45,278 -> 104,636 gzipped, holding core|react|types and nothing else eager closure -461 bytes across the whole bundle The bytes were always downloaded — `data-adapter` is in the eager closure — so this is a re-attribution, not a payload change. Second half: `scripts/check-eager-closure-budget.mjs` gains a MEMBERSHIP half. `apps/console/vite.config.ts` emits `dist/chunk-membership.json` (which chunk each workspace package's modules landed in, counted over every emitted chunk, lazy ones included) and `evaluatePerChunkMembership` asserts each budgeted group's declared packages landed wholly in their declared chunk. EXACT, not a ratchet: one stray module is a finding, by name. Every pass-by-measuring-nothing route is an error rather than a pass — absent artifact, unknown version, empty attribution, a declared package absent from the bundle, a declaration naming a chunk no ceiling governs. No ceiling, baseline or exemption constant moves. Refs: objectui#9345 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013VGeMu3p6qEFWR6K6GGLaW --- .github/workflows/performance-budget.yml | 4 +- apps/console/vite.config.ts | 209 ++++++++++- .../check-eager-closure-budget.test.ts | 343 +++++++++++++++++- .../__tests__/render-budget-comment.test.ts | 1 + scripts/check-eager-closure-budget.mjs | 288 ++++++++++++++- scripts/render-budget-comment.mjs | 11 +- 6 files changed, 828 insertions(+), 28 deletions(-) diff --git a/.github/workflows/performance-budget.yml b/.github/workflows/performance-budget.yml index 9474c67ade..6dffefe8e6 100644 --- a/.github/workflows/performance-budget.yml +++ b/.github/workflows/performance-budget.yml @@ -418,7 +418,8 @@ jobs: echo "" echo "📦 Eager closure (what a page load actually pays for):" # Writes closure_status / closure_gzip_kb / closure_budget_kb / - # closure_chunks / closure_chunk_status / closure_headroom_status to + # closure_chunks / closure_chunk_status / closure_membership_status / + # closure_headroom_status to # $GITHUB_OUTPUT itself. EVERY key it publishes is passed into the # comment step below and rendered there — a verdict published to # $GITHUB_OUTPUT that no consumer reads is a verdict the PR comment @@ -570,6 +571,7 @@ jobs: # so without these the comment can say a budget objected but not which # half did — the reader has to open the job log to find out. BUDGET_CLOSURE_CHUNK_STATUS: ${{ steps.budget.outputs.closure_chunk_status }} + BUDGET_CLOSURE_MEMBERSHIP_STATUS: ${{ steps.budget.outputs.closure_membership_status }} BUDGET_CLOSURE_HEADROOM_STATUS: ${{ steps.budget.outputs.closure_headroom_status }} # The freshness half (objectui#6245). Empty on a run it does not apply # to; the renderer filters an empty half out rather than rendering it. diff --git a/apps/console/vite.config.ts b/apps/console/vite.config.ts index e2fa9b84c0..a3fd869965 100644 --- a/apps/console/vite.config.ts +++ b/apps/console/vite.config.ts @@ -379,6 +379,128 @@ function emitEagerClosureReport(reportFileName = 'eager-closure.json'): Plugin { }; } +/** + * Emits `dist/chunk-membership.json` — which CHUNK each workspace package's + * modules were written to, counted over EVERY chunk in the bundle. + * + * ## Why this exists (objectui#9345) + * + * `advancedChunks.groups` declares membership by regex, and a group can end up + * holding modules no regex of its own matches: rolldown's + * `includeDependenciesRecursively` lets a higher-priority group take a + * lower-priority group's declared members along an import edge. When that + * happened to `packages/core` the per-chunk ceilings above it went GREEN — the + * bytes had moved to a chunk with no ceiling, while the browser went on + * downloading every one of them. A budget that weighs named chunks is bypassed + * by moving bytes between chunks, and nothing in this repository could see it. + * + * So the verdict this feeds (`evaluatePerChunkMembership` in + * `scripts/check-eager-closure-budget.mjs`) asks a question no byte count can: + * did each budgeted group's declared packages actually land where the config + * says? ⛔ It is an EXACT claim, not a ratchet — a single stray module is a + * finding, named. + * + * ## Why a separate file rather than a field of `eager-closure.json` + * + * That report's `reportVersion` is a contract with a SECOND reader — + * `scripts/check-eager-locale-catalogues.mjs` pins the version it accepts, and + * its own tests pin that a later version is REFUSED. Growing a new required + * field there means bumping that version and revising a deliberate refusal in + * a gate this card does not touch. A separate artifact with its own version + * costs one file and bends no existing contract. + * + * ## What it counts, said in words + * + * The population is every module in every emitted chunk whose id contains a + * `packages//` segment — workspace source, not `node_modules`, and not + * limited to the eager closure: a budgeted package's module hiding in a LAZY + * chunk is exactly as much a membership breach as one hiding in an eager + * neighbour, and the eager-closure walk cannot see it. + * + * ⚠️ This plugin only MEASURES — same split as `emitEagerClosureReport` above, + * and for the same reason: a membership verdict that failed `vite build` would + * fail every preview deploy too, which is how a gate gets switched off. + */ +function emitChunkMembershipReport(reportFileName = 'chunk-membership.json'): Plugin { + // The module ids rolldown records are realpaths, so a workspace module reads + // as `/packages//src/...` no matter which symlink resolved it — + // the same assumption the `advancedChunks` group tests above are written on. + const WORKSPACE_MODULE = /[\\/]packages[\\/]([^\\/]+)[\\/]/; + + return { + name: 'emit-chunk-membership-report', + writeBundle(options, bundle) { + const outDir = options.dir ?? path.resolve(import.meta.dirname, 'dist'); + + /** package name -> chunk name -> how many of its modules landed there. */ + const packages: Record> = {}; + let totalChunkCount = 0; + let unnamedChunks = 0; + + for (const output of Object.values(bundle)) { + if (output.type !== 'chunk') continue; + totalChunkCount += 1; + const chunkName = output.name; + if (typeof chunkName !== 'string' || chunkName === '') { + unnamedChunks += 1; + continue; + } + for (const id of Object.keys(output.modules)) { + const match = WORKSPACE_MODULE.exec(id); + if (!match) continue; + const pkg = match[1]; + (packages[pkg] ??= {})[chunkName] = (packages[pkg][chunkName] ?? 0) + 1; + } + } + + // Counter-probe 1 — an unnamed chunk is a HOLE in this report, not a + // cosmetic gap: a budgeted package's module sitting in one would be + // counted nowhere, and "nowhere" reads to the checker exactly like "not + // in a chunk it should not be in". Refused rather than published, in the + // same direction as every probe in `emitEagerClosureReport` above. + if (unnamedChunks > 0) { + this.error( + `[emit-chunk-membership-report] ${unnamedChunks} emitted chunk(s) carry no \`name\`, so ` + + `any workspace module inside them would be attributed to NOTHING. A membership check ` + + `reads an absent attribution as "no stray module", which is the silent direction: it ` + + `would pass by measuring less, not by finding less. Fix the chunk naming before ` + + `publishing this report.`, + ); + } + + // Counter-probe 2 — the other direction, and the one that matters most + // for a check whose green state is "nothing was found somewhere it should + // not be". A report naming no workspace package at all makes every + // membership claim vacuously true. + const packageCount = Object.keys(packages).length; + if (packageCount === 0) { + this.error( + `[emit-chunk-membership-report] not one emitted module id matched ` + + `\`${WORKSPACE_MODULE}\`, so this report attributes NOTHING and every membership ` + + `assertion built on it would agree with everything. Either the bundle contains no ` + + `workspace source — which the console cannot be built without — or module ids have ` + + `stopped being realpaths under \`packages/\` and this matcher needs rewriting.`, + ); + } + + const report = { + // Independent of `eager-closure.json`'s version on purpose; see the + // docblock above. Bump when the shape below changes, so a stale report + // is REFUSED rather than read for fields it does not carry. + membershipReportVersion: 1, + totalChunkCount, + packages, + }; + + fs.writeFileSync(path.join(outDir, reportFileName), `${JSON.stringify(report, null, 2)}\n`); + this.info( + `chunk membership: ${packageCount} workspace packages attributed across the bundle ` + + `→ ${reportFileName}`, + ); + }, + }; +} + /** * Dev-only Vite plugin: serves runtime branding assets at /runtime/assets/*. * @@ -654,6 +776,13 @@ export default defineConfig({ // 0.67% of it (objectui#5324). Measurement only: the verdict is the // workflow's, so a size regression never blocks a preview deploy. emitEagerClosureReport(), + // Writes `dist/chunk-membership.json` — which chunk each workspace + // package's modules were written to. The per-chunk ceilings weigh BYTES, + // and bytes can be moved off a budgeted line without shrinking by one + // (objectui#9345); this is the artifact that lets the same gate assert + // WHERE a budgeted group's declared packages landed. Measurement only, + // same as above. + emitChunkMembershipReport(), // Rolldown's `INEFFECTIVE_DYNAMIC_IMPORT` warnings, pinned to a ledger // instead of scrolling past 43 at a time (objectui#5325). The pinned ones // are replaced by one summary line; an UNPINNED one keeps rolldown's own @@ -844,13 +973,18 @@ export default defineConfig({ // chunks that now name two files where they named one. Nothing here // may be read as headroom that was earned. // - // ⚠️ Disclosed rather than smoothed over: `data-adapter` now also - // holds 5 modules (8.5 KB raw) from `core`/`types` that are reached - // ONLY through `data-objectstack`. That is the same shared-module - // pull-in this comment is about, one tier down and three orders of - // magnitude smaller. It is rolldown's behaviour, not a choice - // available here: `framework` cannot be lifted above these two - // without re-absorbing the catalogue, which is the whole defect. + // ⚠️ This paragraph used to disclose that `data-adapter` ALSO held a + // handful of `core`/`types` modules reached only through + // `data-objectstack` — "rolldown's behaviour, not a choice available + // here". ⛔ The second half of that was wrong, and objectui#9345 is + // what it cost: the same pull-in grew from a handful to the whole of + // `packages/core` on a tree-shaking change made in another package, + // and the budgeted line stopped being able to see its own members. + // There WAS a choice available — `includeDependenciesRecursively`, + // read out in full at the `data-adapter` group below, which now + // declares it `false`. The escape the paragraph correctly refused + // (lifting `framework` above these two) is still refused, for the + // reason it gave. // // ## Why there are ELEVEN i18n groups and not one (objectui#7479) // @@ -888,7 +1022,66 @@ export default defineConfig({ { name: 'i18n-locale-ru', test: /[\\/]packages[\\/]i18n[\\/]src[\\/]locales[\\/]ru\.ts$/, priority: 84 }, { name: 'i18n-locale-ar', test: /[\\/]packages[\\/]i18n[\\/]src[\\/]locales[\\/]ar\.ts$/, priority: 84 }, { name: 'i18n-runtime', test: /[\\/]packages[\\/]i18n[\\/]/, priority: 83 }, - { name: 'data-adapter', test: /[\\/]packages[\\/]data-objectstack[\\/]/, priority: 84 }, + // + // ## `includeDependenciesRecursively: false` — the rule that decides + // ## membership here, named (objectui#9345) + // + // ⭐ This flag is rolldown's `CodeSplittingGroup.includeDependenciesRecursively` + // and its DEFAULT IS `true`. With it on, a group captures the modules + // its `test` matches AND, transitively, everything those modules + // import — and the priority doc for the same option states the other + // half: "when converting the group to a chunk, modules of that group + // will be removed from other groups". So a group can take modules + // that its own regex does not match, out of a lower-priority group + // whose regex does. + // + // That is what happened here. `packages/data-objectstack` imports + // `@object-ui/core`, this group outranks `framework` (84 over 80), and + // so the recursive half of this rule handed `framework`'s declared + // members to a group whose regex never mentioned them. The size of + // the transfer tracks TREE-SHAKING, which is why it moved without any + // edit here: while `@object-ui/core` was only ever imported by name, + // the retained slice reachable through `data-objectstack` was 5 + // modules (the disclosure below, written when it was 5). objectui#9185 + // added an `import('@object-ui/core')` of the BARREL in + // `packages/app-shell`, every export of core became live, and the same + // recursive walk then reached all 92 of them. Measured on the console + // build of `ff1d5ea8d1`: `packages/core` contributed 92 modules to + // `data-adapter` and 0 to `framework`, and `packages/types` split 3/19 + // across the two. + // + // ⛔ The repair is NOT a priority change. Lifting `framework` above 84 + // would re-run objectui#7399 exactly: `@object-ui/react` depends on + // both `@object-ui/i18n` and `@object-ui/data-objectstack`, so a + // recursive `framework` sitting above them would swallow the locale + // catalogues and this group in one move — the defect the two-tier + // layout above exists to prevent. Turning the recursive half OFF for + // this group leaves every priority untouched and makes this group's + // membership exactly what its `test` declares. + // + // Measured across the repair, same tree, console build either side: + // `data-adapter` 78,110 -> 18,537 gzipped and holds only + // `packages/data-objectstack`; `framework` 45,278 -> 104,636 and holds + // core|react|types and nothing else; the whole eager closure moves by + // -461 bytes. ⚠️ Those bytes were ALWAYS downloaded — `data-adapter` + // is in the eager closure — so this is a re-attribution and ⛔ must not + // be read as a payload change in either direction. + // + // ⚠️ `framework` is over its ceiling at that reading, by an amount + // `scripts/check-eager-closure-budget.mjs` prints on every run. That + // is this repair making a pre-existing overage VISIBLE, not causing + // it; ⛔ no constant was moved to absorb it (objectui#9345 rules that + // none may be). + // + // ⚠️ Rolldown documents a cost for turning this off: recursive capture + // "reduces the chance of generating circular chunks", and the same + // paragraph recommends `preserveEntrySignatures: false` and + // `strictExecutionOrder: true` alongside disabling it. Neither is set + // here, and neither was needed: the build emits the same chunk + // population as before, with the ONE group narrowed. A future group + // taking this flag should re-check that rather than inherit the + // reading. + { name: 'data-adapter', test: /[\\/]packages[\\/]data-objectstack[\\/]/, priority: 84, includeDependenciesRecursively: false }, { name: 'framework', test: /[\\/]packages[\\/](core|react|types)[\\/]/, priority: 80 }, { name: 'ui-components', test: /[\\/]packages[\\/](components|fields)[\\/]/, priority: 80 }, { name: 'ui-layout', test: /[\\/]packages[\\/]layout[\\/]/, priority: 80 }, diff --git a/scripts/__tests__/check-eager-closure-budget.test.ts b/scripts/__tests__/check-eager-closure-budget.test.ts index e0647ba170..116a1f310f 100644 --- a/scripts/__tests__/check-eager-closure-budget.test.ts +++ b/scripts/__tests__/check-eager-closure-budget.test.ts @@ -21,6 +21,7 @@ import { MAX_EAGER_CLOSURE_GZIP_BYTES, PER_CHUNK_BASELINE, PER_CHUNK_GZIP_CEILINGS, + PER_CHUNK_MEMBERSHIP, REGRESSION_THIS_GATE_MUST_CATCH_BYTES, SUPPORTED_REPORT_VERSION, VERDICT_CEILING_CONSTANTS, @@ -28,6 +29,7 @@ import { evaluateClosureBudget, evaluateHeadroomSensitivity, evaluatePerChunkBudgets, + evaluatePerChunkMembership, extractCeilingDeclarations, RECOGNISED_HALF_STATUSES, foldHalfStatuses, @@ -435,7 +437,20 @@ describe('per-chunk ceilings', () => { */ describe('chunk attribution (objectui#7399)', () => { /** A group as `advancedChunks.groups` declares it. */ - type Group = { name: string; priority: number; test: RegExp | null }; + type Group = { + name: string; + priority: number; + test: RegExp | null; + /** + * Whatever the group declares AFTER `priority`, verbatim — `''` when it + * declares nothing. objectui#9345 put an option there + * (`includeDependenciesRecursively`), and the parse that could not see one + * did not degrade gracefully: it stopped matching the group ENTIRELY, so + * every case below quietly lost a subject. Keeping the tail is what lets a + * pin be written about an option instead of only about a regex. + */ + options: string; + }; /** * Parse the groups out of the console's vite config. @@ -448,13 +463,14 @@ describe('chunk attribution (objectui#7399)', () => { function parseGroups(): Group[] { const source = fs.readFileSync(viteConfigPath, 'utf8'); const entry = - /\{\s*name:\s*'([^']+)',\s*test:\s*(\/(?:[^/\\\n]|\\.|\[[^\]\n]*\])+\/[a-z]*|[A-Za-z_$][\w$]*)\s*,\s*priority:\s*(\d+)\s*\}/g; - return [...source.matchAll(entry)].map(([, name, test, priority]) => { + /\{\s*name:\s*'([^']+)',\s*test:\s*(\/(?:[^/\\\n]|\\.|\[[^\]\n]*\])+\/[a-z]*|[A-Za-z_$][\w$]*)\s*,\s*priority:\s*(\d+)\s*((?:,\s*[A-Za-z_$][\w$]*:\s*[^,{}]+)*)\s*,?\s*\}/g; + return [...source.matchAll(entry)].map(([, name, test, priority, options]) => { const literal = /^\/(.*)\/([a-z]*)$/s.exec(test); return { name, priority: Number(priority), test: literal ? new RegExp(literal[1], literal[2]) : null, + options: (options ?? '').trim(), }; }); } @@ -537,6 +553,35 @@ describe('chunk attribution (objectui#7399)', () => { expect(claiming[0].priority).toBeGreaterThan(framework!.priority); }); + /** + * objectui#9345 — the half the priority cases above cannot see. + * + * Every case in this block asks which group's `test` CLAIMS a module id. + * That question was answered correctly the whole time `packages/core` was + * being written into `data-adapter`: rolldown's + * `includeDependenciesRecursively` (default `true`) also gives a group the + * modules its captured modules IMPORT, and the priority doc for the same + * option says those are then removed from the lower-priority groups whose + * regex does match them. `data-adapter` outranks `framework` and + * `packages/data-objectstack` imports `@object-ui/core`, so all 92 modules + * of `packages/core` went to a chunk with no ceiling — while a static read + * of the group table, and every case above, stayed green. + * + * ⇒ the repair is this flag, and this is the pin that stops it being + * dropped in a reformat. The bundle-level half — the modules actually + * landed where the config says — is `evaluatePerChunkMembership`, which + * needs a build; this one reds in a unit run. + */ + it('narrows `data-adapter` to its own regex, so it cannot absorb `framework`s members', () => { + const dataAdapter = groups.find((g) => g.name === 'data-adapter'); + expect(dataAdapter).toBeDefined(); + expect(dataAdapter!.options).toContain('includeDependenciesRecursively: false'); + // The control: the parse can see an options tail at all, and does not + // report one where none is written. A tail-blind parse would satisfy the + // line above by reading `''` from every group. + expect(groups.find((g) => g.name === 'framework')!.options).toBe(''); + }); + it('leaves no second claimant at the winner`s priority', () => { for (const id of [LOCALE_MODULE, RESIDENT_LOCALE_MODULE, DATA_MODULE]) { const claiming = claimants(id); @@ -1056,6 +1101,192 @@ describe('ceiling sensitivity, judged live (objectui#5924)', () => { }); }); +/** + * A membership artifact shaped exactly like `emitChunkMembershipReport`'s + * output, with every declared package landing wholly in its declared chunk. + * + * Built FROM {@link PER_CHUNK_MEMBERSHIP} rather than written out, so a package + * added to the declaration cannot be left silently unrepresented here — which + * would make the pass case pass for a package nobody checked. + */ +function passingMembership(overrides: Record = {}) { + const packages: Record> = {}; + for (const [chunk, pkgs] of Object.entries(PER_CHUNK_MEMBERSHIP)) { + for (const pkg of pkgs) packages[pkg] = { [chunk]: 12 }; + } + // A package nothing budgets, present in every real build, so the evaluator is + // never handed a map containing only its own subjects. + packages['app-shell'] = { index: 40, 'some-lazy-view': 3 }; + return { membershipReportVersion: 1, totalChunkCount: 2_000, packages, ...overrides }; +} + +/** + * Chunk membership — the half that asks WHERE, not HOW BIG (objectui#9345). + * + * ⚠️ Read the error cases as the substance of this block, not as its edges. + * This half's green state is an ABSENCE — "no declared package was found in a + * chunk it is not declared for" — and that sentence is equally true of an + * artifact that attributed nothing, a package that vanished from the bundle, + * and a declaration pointed at a chunk no ceiling governs. Each of those is + * pinned below as an ERROR, because each of them would otherwise be a pass + * bought by measuring less. + */ +describe('chunk membership (objectui#9345)', () => { + it('passes when every declared package landed wholly in its declared chunk', () => { + const result = evaluatePerChunkMembership({ membership: passingMembership() }); + expect(result.status).toBe('pass'); + // The population, named in the verdict: a green line that does not say what + // it weighed is indistinguishable from a green line that weighed nothing. + for (const pkgs of Object.values(PER_CHUNK_MEMBERSHIP)) { + for (const pkg of pkgs) expect(result.message).toContain(`\`packages/${pkg}\``); + } + }); + + it('FAILS, naming the package and the chunk that took it, on one stray module', () => { + // The incident, reduced to its smallest form: `packages/core` split between + // `framework` and a group whose regex never mentioned it. + const membership = passingMembership(); + (membership.packages as Record>).core = { + framework: 11, + 'data-adapter': 1, + }; + const result = evaluatePerChunkMembership({ membership }); + expect(result.status).toBe('fail'); + expect(result.message).toContain('`packages/core`'); + expect(result.message).toContain('`data-adapter`'); + expect(result.message).toContain('`framework`'); + // ⛔ The remedy this verdict may never suggest. + expect(result.message).toContain('Do NOT move a ceiling'); + }); + + it('FAILS when the whole package moved, not only when it split', () => { + const membership = passingMembership(); + (membership.packages as Record>).core = { 'data-adapter': 92 }; + const result = evaluatePerChunkMembership({ membership }); + expect(result.status).toBe('fail'); + expect(result.message).toContain('0 of its 92 modules landed in `framework`'); + }); + + it('is EXACT, not a ratchet — a majority in the right chunk is still a fail', () => { + // The shape a headroom-bearing pin would wave through, and the one the + // ruling on objectui#9345 forbids: 99 of 100 modules in place. + const membership = passingMembership(); + (membership.packages as Record>).core = { + framework: 99, + 'plugin-grid': 1, + }; + expect(evaluatePerChunkMembership({ membership }).status).toBe('fail'); + }); + + describe('refuses a verdict rather than passing by measuring nothing', () => { + it('errors when the artifact is absent', () => { + const result = evaluatePerChunkMembership({ membership: null }); + expect(result.status).toBe('error'); + expect(result.message).toContain('PREREQUISITE NOT MET'); + }); + + it('errors on a version it does not understand', () => { + const result = evaluatePerChunkMembership({ + membership: passingMembership({ membershipReportVersion: 99 }), + }); + expect(result.status).toBe('error'); + expect(result.message).toContain('membershipReportVersion'); + }); + + it('errors when the artifact attributes no package at all', () => { + const result = evaluatePerChunkMembership({ + membership: passingMembership({ packages: {} }), + }); + expect(result.status).toBe('error'); + expect(result.message).toContain('vacuously true'); + }); + + it('errors when the bundle it describes has no chunk in it', () => { + const result = evaluatePerChunkMembership({ + membership: passingMembership({ totalChunkCount: 0 }), + }); + expect(result.status).toBe('error'); + expect(result.message).toContain('totalChunkCount'); + }); + + it('errors when a declared package contributed no module anywhere', () => { + // ⭐ The case that separates this half from a vacuous one. A package + // absent from the bundle cannot be in a chunk it should not be in, so the + // stray scan agrees with everything about it. + const membership = passingMembership(); + delete (membership.packages as Record).core; + const result = evaluatePerChunkMembership({ membership }); + expect(result.status).toBe('error'); + expect(result.message).toContain('contributed no module'); + expect(result.message).toContain('`packages/core`'); + }); + + it('errors when a declared package is present but attributed to nothing', () => { + const membership = passingMembership(); + (membership.packages as Record>).core = {}; + expect(evaluatePerChunkMembership({ membership }).status).toBe('error'); + }); + + it('errors when the declaration names a chunk no ceiling governs', () => { + const result = evaluatePerChunkMembership({ + membership: passingMembership(), + declaration: { 'data-adapter': ['data-objectstack'] }, + }); + expect(result.status).toBe('error'); + expect(result.message).toContain('PER_CHUNK_GZIP_CEILINGS'); + }); + }); + + describe('the declaration itself', () => { + it('names only chunks that carry a per-chunk ceiling', () => { + for (const chunk of Object.keys(PER_CHUNK_MEMBERSHIP)) { + expect(PER_CHUNK_GZIP_CEILINGS).toHaveProperty(chunk); + } + // Non-vacuity: the live table is not empty, and an invented key is still + // not a budgeted chunk. + expect(Object.keys(PER_CHUNK_MEMBERSHIP).length).toBeGreaterThan(0); + expect(PER_CHUNK_GZIP_CEILINGS).not.toHaveProperty('a-chunk-nothing-budgets'); + }); + + /** + * ⭐ The cross-check that keeps this declaration from becoming a second + * opinion about the console config. Each package name below must be matched + * by the `test` of the group it is declared under — the same regex rolldown + * itself matches — so a group whose regex is narrowed without updating this + * table reds here rather than going quietly out of date. + */ + it('declares only packages the group`s own regex claims', () => { + const source = fs.readFileSync(viteConfigPath, 'utf8'); + for (const [chunk, pkgs] of Object.entries(PER_CHUNK_MEMBERSHIP)) { + // ⚠️ Anchored on `priority:` deliberately. Without a terminator the + // alternation inside the test literal stops at the first `/` of a + // `[\\/]` class and hands back a truncated, INVALID regex — a parse + // that throws rather than one that lies, but a parse that reads + // nothing all the same. + const declaration = new RegExp( + String.raw`\{\s*name:\s*'${chunk}',\s*test:\s*(/(?:[^/\\\n]|\\.|\[[^\]\n]*\])+/[a-z]*)\s*,\s*priority:`, + ).exec(source); + // Fails closed: a group this parse cannot find is an error, not a pass. + expect(declaration, `no regex-tested group named \`${chunk}\` in the console config`) + .not.toBeNull(); + const literal = /^\/(.*)\/([a-z]*)$/s.exec(declaration![1])!; + const test = new RegExp(literal[1], literal[2]); + for (const pkg of pkgs) { + expect( + test.test(path.join(repoRoot, `packages/${pkg}/src/index.ts`)), + `\`${chunk}\` is declared to hold packages/${pkg}, but its own test does not match it`, + ).toBe(true); + } + // The must-miss control, so a regex that matched everything could not + // satisfy the loop above. + expect(test.test(path.join(repoRoot, 'packages/not-a-real-package/src/index.ts'))).toBe( + false, + ); + } + }); + }); +}); + describe('renderTopChunks', () => { it('names the biggest eager chunks so a failure has suspects', () => { const lines = renderTopChunks(report(), 2).split('\n'); @@ -1088,11 +1319,25 @@ describe('main', () => { * what makes these cases exercise the non-pull_request path deterministically * instead of by luck. Pass it through `env` to opt a case in. */ - function run(reportBody: unknown, env: Record = {}) { + function run( + reportBody: unknown, + env: Record = {}, + membershipBody: unknown = passingMembership(), + ) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'closure-budget-')); const reportPath = path.join(dir, 'eager-closure.json'); const outputPath = path.join(dir, 'github-output'); if (reportBody !== undefined) fs.writeFileSync(reportPath, JSON.stringify(reportBody)); + // Written into the SAME directory on purpose — that is the production + // relationship between the two artifacts, and `main` derives one path from + // the other. `undefined` opts a case out, which is the absent-artifact + // case rather than a shortcut. + if (membershipBody !== undefined) { + fs.writeFileSync( + path.join(dir, 'chunk-membership.json'), + JSON.stringify(membershipBody), + ); + } try { const code = main(['--report', reportPath], { GITHUB_OUTPUT: outputPath, ...env }); const outputs = Object.fromEntries( @@ -1478,6 +1723,89 @@ describe('main', () => { * `BUDGET_CLOSURE_BUDGET_KB: 3990.2` — 4,086,000 bytes — with conclusion * `success`. `theRealIncident` below replays exactly that pair of numbers. */ +/** + * The membership half, folded — objectui#9345. + * + * Local to this block rather than merged into `describe('main')` above for the + * reason that block's own freshness sibling gives: these cases need the second + * artifact under their control, and a shared helper that always wrote a healthy + * one could not express the absent case at all. + */ +describe('main folds chunk membership into the exit code (objectui#9345)', () => { + /** + * Local runner, like the freshness block's: `describe('main')`'s helper is + * scoped to that block, and these cases need the SECOND artifact under their + * own control — including the case where it is absent, which a helper that + * always wrote a healthy one could not express. + */ + function runPair(reportBody: unknown, membershipBody: unknown) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'closure-membership-')); + const reportPath = path.join(dir, 'eager-closure.json'); + const outputPath = path.join(dir, 'github-output'); + fs.writeFileSync(reportPath, JSON.stringify(reportBody)); + if (membershipBody !== undefined) { + fs.writeFileSync(path.join(dir, 'chunk-membership.json'), JSON.stringify(membershipBody)); + } + try { + const code = main(['--report', reportPath], { GITHUB_OUTPUT: outputPath }); + const outputs = Object.fromEntries( + fs + .readFileSync(outputPath, 'utf8') + .split('\n') + .filter(Boolean) + .map((line) => { + const at = line.indexOf('='); + return [line.slice(0, at), line.slice(at + 1)] as [string, string]; + }), + ); + return { code, outputs }; + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + } + + /** The report every case here starts from: nothing is over any line. */ + function healthyBudget() { + return report({ + eagerGzipBytes: BASELINE.gzipBytes, + files: [ + { fileName: 'assets/index-A.js', name: 'index', bytes: 90_000, gzipBytes: BASELINE.gzipBytes - PER_CHUNK_BASELINE['vendor-objectstack'] - PER_CHUNK_BASELINE.framework - PER_CHUNK_BASELINE['ui-components'] - PER_CHUNK_BASELINE['i18n-locale-en'] }, + { fileName: 'assets/vendor-objectstack-B.js', name: 'vendor-objectstack', bytes: 5_000_000, gzipBytes: PER_CHUNK_BASELINE['vendor-objectstack'] }, + { fileName: 'assets/framework-C.js', name: 'framework', bytes: 300_000, gzipBytes: PER_CHUNK_BASELINE.framework }, + { fileName: 'assets/ui-components-D.js', name: 'ui-components', bytes: 900_000, gzipBytes: PER_CHUNK_BASELINE['ui-components'] }, + { fileName: 'assets/i18n-locale-en-E.js', name: 'i18n-locale-en', bytes: 120_000, gzipBytes: PER_CHUNK_BASELINE['i18n-locale-en'] }, + ], + eagerChunkCount: 5, + }); + } + + it('exits 0 and publishes `pass` when every declared package is in place', () => { + const { code, outputs } = runPair(healthyBudget(), passingMembership()); + expect(outputs.closure_membership_status).toBe('pass'); + expect(code).toBe(0); + }); + + it('exits 1 — a size verdict`s code — when a budgeted package landed elsewhere', () => { + const membership = passingMembership(); + (membership.packages as Record>).core = { + framework: 60, + 'data-adapter': 32, + }; + const { code, outputs } = runPair(healthyBudget(), membership); + expect(outputs.closure_membership_status).toBe('fail'); + // ⭐ 1, not 2. A package in the wrong chunk is a real verdict about the + // bundle, in the same class as a chunk over its ceiling — not a gauge that + // produced nothing. + expect(code).toBe(1); + }); + + it('exits 2 when the artifact is absent — an unbuilt tree is not a pass', () => { + const { code, outputs } = runPair(healthyBudget(), undefined); + expect(outputs.closure_membership_status).toBe('error'); + expect(code).toBe(2); + }); +}); + describe('ceiling freshness (objectui#6245)', () => { const checkerSource = fs.readFileSync(checkerPath, 'utf8'); @@ -1721,6 +2049,13 @@ describe('ceiling freshness (objectui#6245)', () => { const reportPath = path.join(dir, 'eager-closure.json'); const outputPath = path.join(dir, 'github-output'); fs.writeFileSync(reportPath, JSON.stringify(healthyReport())); + // The membership half resolves its artifact beside the report. These + // cases are about FRESHNESS, so it is written healthy here — an absent + // one would exit 2 for a reason none of them is asking about. + fs.writeFileSync( + path.join(dir, 'chunk-membership.json'), + JSON.stringify(passingMembership()), + ); const write = (name: string, body: string) => { const at = path.join(dir, name); fs.writeFileSync(at, body); diff --git a/scripts/__tests__/render-budget-comment.test.ts b/scripts/__tests__/render-budget-comment.test.ts index c2290139e0..652f027ef8 100644 --- a/scripts/__tests__/render-budget-comment.test.ts +++ b/scripts/__tests__/render-budget-comment.test.ts @@ -501,6 +501,7 @@ describe('performance-budget.yml contract', () => { expect(published).toContain('closure_chunk_status'); expect(published).toContain('closure_headroom_status'); expect(published).toContain('closure_freshness_status'); + expect(published).toContain('closure_membership_status'); for (const key of published) { expect(workflow, `workflow must pass steps.budget.outputs.${key} to the comment step`) diff --git a/scripts/check-eager-closure-budget.mjs b/scripts/check-eager-closure-budget.mjs index 6c52f489eb..9af5b152d3 100644 --- a/scripts/check-eager-closure-budget.mjs +++ b/scripts/check-eager-closure-budget.mjs @@ -1258,6 +1258,251 @@ export const PER_CHUNK_BASELINE = Object.freeze({ 'ui-components': 265_937, }); +/** + * The membership artifact's file name, and its default path. + * + * ⚠️ The NAME is the load-bearing half. `main` resolves this artifact next to + * whatever `--report` names, because the two files are written by the same + * build into the same `dist` — deriving one from the other is what keeps a run + * pointed at some other build's report from silently weighing THIS tree's + * membership, which is a mismatch no assertion below could detect. + */ +const MEMBERSHIP_REPORT_FILE_NAME = 'chunk-membership.json'; +const MEMBERSHIP_REPORT_PATH = `apps/console/dist/${MEMBERSHIP_REPORT_FILE_NAME}`; + +/** + * The membership artifact's own version, independent of + * {@link SUPPORTED_REPORT_VERSION}. + * + * ⛔ Not a duplicate of that constant and ⛔ not to be merged into it: + * `eager-closure.json`'s version is a contract with a SECOND reader, + * `scripts/check-eager-locale-catalogues.mjs`, whose own tests pin that a later + * version is REFUSED. Two artifacts, two versions, each bumped by the reader + * that has to understand it. + */ +const SUPPORTED_MEMBERSHIP_REPORT_VERSION = 1; + +/** + * WHERE each budgeted group's declared packages must land — an EXACT claim, + * ⛔ not a ratchet with headroom (objectui#9345). + * + * ## The hole this closes + * + * Every other verdict in this file weighs BYTES against a line drawn per chunk + * NAME. That arrangement has an exit nobody was watching: move the bytes to a + * chunk with no line, and every line goes green while the browser downloads + * exactly what it downloaded before. It was not hypothetical. All 92 modules of + * `packages/core` left the budgeted `framework` chunk for `data-adapter` — + * which has no ceiling and no baseline and is in the eager closure — on one + * commit that edited no chunking config at all (objectui#9185, measured on + * objectui#9205). `framework` fell far enough below its own baseline to raise a + * DIFFERENT question, and the answer to that question was three cards away. + * + * ⇒ this half asks the question a byte count structurally cannot: did the + * budgeted groups' declared packages land where the config says they land? + * + * ## Why the keys are exactly the budgeted groups + * + * The subject of the claim is a BUDGET that can be bypassed, so its population + * is the chunks that carry a budget — the keys of + * {@link PER_CHUNK_GZIP_CEILINGS}, cross-checked below rather than trusted. + * Two of those four keys name no workspace package and are absent here for + * reasons, not by oversight: + * + * - `vendor-objectstack` holds `node_modules` only, so no `packages/` + * claim can be made about it. + * - `i18n-locale-en` is ONE FILE inside `packages/i18n`, whose other modules + * belong to `i18n-runtime` by design — package granularity cannot express + * that, and `scripts/check-eager-locale-catalogues.mjs` already pins the + * catalogues' membership by chunk name. + * + * ## Why the values are package names and not regexes + * + * A regex here would be a SECOND opinion about the group tests in + * `apps/console/vite.config.ts` — one that goes on reading plausibly while it + * matches something else. `scripts/__tests__/check-eager-closure-budget.test.ts` + * instead requires each name below to be matched by that group's own declared + * test, so the two cannot drift apart without a red test. + */ +export const PER_CHUNK_MEMBERSHIP = Object.freeze({ + framework: Object.freeze(['core', 'react', 'types']), + 'ui-components': Object.freeze(['components', 'fields']), +}); + +/** + * Read the membership artifact, or `null` when it is not there / not JSON. + * + * @param {string} [reportPath] + * @returns {unknown} + */ +export function readMembershipReport(reportPath = MEMBERSHIP_REPORT_PATH) { + try { + return JSON.parse(fs.readFileSync(path.resolve(reportPath), 'utf8')); + } catch { + return null; + } +} + +/** + * The membership verdict. + * + * ⚠️ Read the ERROR branches before the fail branch. Every one of them exists + * because this half's green state is "no declared package was found anywhere it + * should not be" — a sentence that is also true of a report that attributed + * nothing, of a package that vanished from the bundle, and of a declaration + * that names a chunk no ceiling governs. A check whose pass condition is an + * ABSENCE has to prove it looked. + * + * @param {object} input + * @param {unknown} input.membership parsed `chunk-membership.json`, or null + * @param {Record} [input.declaration] + * @param {Record} [input.budgetedChunks] + * @param {string} [input.reportPath] + * @returns {{ status: 'pass' | 'fail' | 'error', message: string }} + */ +export function evaluatePerChunkMembership({ + membership, + declaration = PER_CHUNK_MEMBERSHIP, + budgetedChunks = PER_CHUNK_GZIP_CEILINGS, + reportPath = MEMBERSHIP_REPORT_PATH, +} = {}) { + if (membership === null || membership === undefined || typeof membership !== 'object') { + return { + status: 'error', + message: + `PREREQUISITE NOT MET: \`${reportPath}\` is missing or is not JSON, so no chunk ` + + `membership was weighed. This half reads a BUILT bundle — run ` + + `\`pnpm --filter @object-ui/console build\` first. ⛔ This is NOT a pass: a gate that ` + + `could not run is not a gate that ran clean.`, + }; + } + + const r = /** @type {Record} */ (membership); + if (r.membershipReportVersion !== SUPPORTED_MEMBERSHIP_REPORT_VERSION) { + return { + status: 'error', + message: + `\`${reportPath}\` declares membershipReportVersion ` + + `${JSON.stringify(r.membershipReportVersion)}, expected ` + + `${SUPPORTED_MEMBERSHIP_REPORT_VERSION} — the emitter in ` + + `\`apps/console/vite.config.ts\` and this half have drifted apart, and a shape this ` + + `half does not understand is refused rather than read for fields it may not carry.`, + }; + } + + if (typeof r.totalChunkCount !== 'number' || !Number.isFinite(r.totalChunkCount) || r.totalChunkCount < 1) { + return { + status: 'error', + message: + `\`${reportPath}\` reports totalChunkCount ${JSON.stringify(r.totalChunkCount)} — a ` + + `bundle with no chunk in it attributed nothing, and "nothing was attributed" reads to ` + + `the check below exactly like "no module is out of place".`, + }; + } + + const packages = r.packages; + if (packages === null || typeof packages !== 'object' || Object.keys(packages).length === 0) { + return { + status: 'error', + message: + `\`${reportPath}\` attributes no workspace package at all, so every membership claim ` + + `below would be vacuously true. The emitter refuses to publish such a report; one ` + + `reaching this half means it was edited, truncated or hand-written.`, + }; + } + const attribution = /** @type {Record>} */ (packages); + + // The declaration is about BUDGETED chunks. A key here that carries no + // ceiling would be pinning membership for a line nothing weighs — harmless + // to assert and misleading to read, since this half's whole argument is that + // it guards the byte budget's flank. + const unbudgeted = Object.keys(declaration).filter((chunk) => !(chunk in budgetedChunks)); + if (unbudgeted.length > 0) { + return { + status: 'error', + message: + `PER_CHUNK_MEMBERSHIP declares ${unbudgeted.map((c) => `\`${c}\``).join(', ')}, which ` + + `${unbudgeted.length === 1 ? 'is' : 'are'} not among the budgeted chunks in ` + + `PER_CHUNK_GZIP_CEILINGS. This half exists to guard a byte budget's flank; a membership ` + + `pin on a chunk with no budget guards nothing and reads as though it did.`, + }; + } + + /** @type {string[]} */ + const missing = []; + /** @type {string[]} */ + const strays = []; + /** @type {string[]} */ + const held = []; + + for (const [chunk, pkgs] of Object.entries(declaration)) { + for (const pkg of pkgs) { + const landed = attribution[pkg]; + if (landed === undefined || typeof landed !== 'object' || Object.keys(landed).length === 0) { + missing.push(pkg); + continue; + } + const total = Object.values(landed).reduce((n, count) => n + count, 0); + const elsewhere = Object.entries(landed).filter(([name]) => name !== chunk); + if (elsewhere.length > 0) { + const where = elsewhere + .sort((a, b) => b[1] - a[1]) + .map(([name, count]) => `${count} in \`${name}\``) + .join(', '); + strays.push( + `\`packages/${pkg}\` is declared in \`${chunk}\` but ${where} ` + + `(${landed[chunk] ?? 0} of its ${total} modules landed in \`${chunk}\`)`, + ); + } else { + held.push(`\`packages/${pkg}\` ${total} modules in \`${chunk}\``); + } + } + } + + // Ahead of the stray verdict on purpose. A declared package that contributed + // NO module to the bundle cannot be out of place, so the stray scan would + // pass on it — by measuring nothing, which is the one direction every probe + // in this file refuses. + if (missing.length > 0) { + return { + status: 'error', + message: + `${missing.length} declared package(s) contributed no module to any chunk: ` + + `${missing.map((p) => `\`packages/${p}\``).join(', ')}. ⛔ Not a pass: a package that ` + + `is not in the bundle is not "in its declared chunk", and the membership scan would ` + + `agree with everything about it. Either the package was renamed or removed — update ` + + `PER_CHUNK_MEMBERSHIP deliberately — or the emitter has stopped recognising its module ` + + `ids, in which case this gate is matching nothing.`, + }; + } + + if (strays.length > 0) { + return { + status: 'fail', + message: + `${strays.length} budgeted package(s) did not land in the chunk the console config ` + + `declares for them:\n` + + strays.map((line) => ` ❌ ${line}`).join('\n') + + `\nChunk membership is decided by the grouping rules in ` + + `\`apps/console/vite.config.ts\`, ⛔ not by the side effect of an import edge: ` + + `rolldown's \`includeDependenciesRecursively\` lets a higher-priority group take a ` + + `lower-priority group's declared members along an import, and the group that receives ` + + `them may carry no ceiling at all — in which case the bytes go on being downloaded ` + + `while every per-chunk line above turns green (objectui#9345). ⛔ Do NOT move a ceiling ` + + `or a baseline to absorb this. Repair the grouping rule, or change this declaration ` + + `deliberately and say in the PR which chunk now owns the package and why.`, + }; + } + + return { + status: 'pass', + message: + `Chunk membership: ${held.length} budgeted package(s) each landed wholly in their ` + + `declared chunk — ${held.join('; ')}. (Counted over every emitted chunk, lazy ones ` + + `included, from \`${reportPath}\`.)`, + }; +} + /** * The LOWER bound on a ceiling's headroom, as a fraction of * {@link REGRESSION_THIS_GATE_MUST_CATCH_BYTES} (objectui#8554). @@ -2493,14 +2738,14 @@ export function readReport(reportPath) { } /** - * Every status the four halves are declared to produce, and the only ones + * Every status the halves are declared to produce, and the only ones * {@link foldHalfStatuses} knows how to weigh. * - * DERIVED, not invented: it is the union of the four `@returns` unions above — - * {@link evaluateClosureBudget} and {@link evaluatePerChunkBudgets} - * (`pass | fail | error`), {@link evaluateHeadroomSensitivity} - * (`pass | error`), and {@link evaluateCeilingFreshness} - * (`pass | error | not-applicable`). `scripts/__tests__/` re-derives that union + * DERIVED, not invented: it is the union of the `@returns` unions above — + * {@link evaluateClosureBudget}, {@link evaluatePerChunkBudgets} and + * {@link evaluatePerChunkMembership} (`pass | fail | error`), + * {@link evaluateHeadroomSensitivity} (`pass | error`), and + * {@link evaluateCeilingFreshness} (`pass | error | not-applicable`). `scripts/__tests__/` re-derives that union * from this file's own text and reds when the two disagree, so a half that * gains a FIFTH status cannot gain it without also being given a code here. * That test is the reason this list may be written down at all (AGENTS.md #9): @@ -2578,8 +2823,10 @@ function writeGithubOutput(entries, outputPath = process.env.GITHUB_OUTPUT) { } /** - * Exit codes: `0` within budget, `1` over budget — the aggregate ceiling or any - * per-chunk ceiling — and `2` no trustworthy verdict (report missing, + * Exit codes: `0` within budget and in place, `1` over budget — the aggregate + * ceiling or any per-chunk ceiling — or a budgeted package that landed outside + * the chunk the console config declares for it (objectui#9345), and `2` no + * trustworthy verdict (report missing, * stale-shaped, internally inconsistent, missing a budgeted chunk, governed by * a ceiling that has drifted out of range of the regression it must catch, * governed by a ceiling with no headroom left to measure with — objectui#8554, @@ -2594,10 +2841,11 @@ function writeGithubOutput(entries, outputPath = process.env.GITHUB_OUTPUT) { * the workflow fails the step. It never prints a verdict about a bundle nobody * weighed, and it never exits 0 having measured nothing. * - * All FOUR halves are evaluated and printed before any of them decides the - * code: a run that reports the total and hides which chunk moved (or hides - * whether either line still means anything, or whether the line it used is the - * line in force) teaches readers to ignore the half they cannot see. + * EVERY half is evaluated and printed before any of them decides the code: a + * run that reports the total and hides which chunk moved (or hides whether + * either line still means anything, or whether the line it used is the line in + * force, or whether the budgeted chunks still hold what they are named for) + * teaches readers to ignore the half they cannot see. */ export function main(argv = process.argv.slice(2), env = process.env) { const flagIndex = argv.indexOf('--report'); @@ -2606,6 +2854,15 @@ export function main(argv = process.argv.slice(2), env = process.env) { const report = readReport(resolved); const result = evaluateClosureBudget({ report, reportPath }); const perChunk = evaluatePerChunkBudgets({ report, reportPath }); + // The fifth half reads a DIFFERENT artifact — `chunk-membership.json`, from + // the same build — because it asks a question `eager-closure.json` carries no + // field for: WHERE a budgeted group's declared packages landed, counted over + // every emitted chunk rather than the eager closure alone (objectui#9345). + const membershipPath = path.join(path.dirname(resolved), MEMBERSHIP_REPORT_FILE_NAME); + const membership = evaluatePerChunkMembership({ + membership: readMembershipReport(membershipPath), + reportPath: membershipPath, + }); const sensitivity = evaluateHeadroomSensitivity({ report, reportPath }); // The fourth half asks about the CEILING rather than the payload, so its // inputs are source texts and not the report: this file as checked out, @@ -2631,6 +2888,11 @@ export function main(argv = process.argv.slice(2), env = process.env) { } else { console.error(`❌ ${perChunk.message}`); } + if (membership.status === 'pass') { + console.log(`✅ ${membership.message}`); + } else { + console.error(`❌ ${membership.message}`); + } if (sensitivity.status === 'pass') { console.log(`✅ ${sensitivity.message}`); } else { @@ -2658,6 +2920,7 @@ export function main(argv = process.argv.slice(2), env = process.env) { closure_budget_kb: kb(result.budgetBytes), closure_chunks: result.chunkCount === null ? '' : String(result.chunkCount), closure_chunk_status: perChunk.status, + closure_membership_status: membership.status, closure_headroom_status: sensitivity.status, // Empty on a run this half does not apply to, so the PR comment's half // table filters it out instead of rendering a blank verdict as a row. @@ -2689,6 +2952,7 @@ export function main(argv = process.argv.slice(2), env = process.env) { const { code, unrecognised } = foldHalfStatuses({ closure: result.status, 'per-chunk': perChunk.status, + membership: membership.status, sensitivity: sensitivity.status, freshness: freshness.status, }); diff --git a/scripts/render-budget-comment.mjs b/scripts/render-budget-comment.mjs index 10a7717867..cfd47810f4 100644 --- a/scripts/render-budget-comment.mjs +++ b/scripts/render-budget-comment.mjs @@ -51,6 +51,7 @@ const text = (value) => (typeof value === 'string' ? value.trim() : ''); * @param {string} [input.closureBudgetKb] the closure ceiling it was compared against * @param {string} [input.closureChunks] how many chunks the eager closure spans * @param {string} [input.closureChunkStatus] `closure_chunk_status` — the per-chunk half + * @param {string} [input.closureMembershipStatus] `closure_membership_status` — the membership half * @param {string} [input.closureHeadroomStatus] `closure_headroom_status` — the sensitivity half * @param {string} [input.closureFreshnessStatus] `closure_freshness_status` — the freshness half * @param {string} [input.message] human-readable reason when `status` is `error` @@ -72,6 +73,7 @@ export function renderBudgetComment(input = {}) { budgetKb: text(input.closureBudgetKb), chunks: text(input.closureChunks), chunkStatus: text(input.closureChunkStatus), + membershipStatus: text(input.closureMembershipStatus), headroomStatus: text(input.closureHeadroomStatus), freshnessStatus: text(input.closureFreshnessStatus), }; @@ -97,10 +99,11 @@ export function renderBudgetComment(input = {}) { } /** - * The eager-closure checker evaluates four halves and publishes a verdict for - * each. The step's exit code folds all four into ONE `budget_status`, so a + * The eager-closure checker evaluates several halves and publishes a verdict + * for each. The step's exit code folds them all into ONE `budget_status`, so a * comment that renders only that says "something objected" and sends the reader - * to the job log to learn which — the aggregate total, one chunk, a ceiling that + * to the job log to learn which — the aggregate total, one chunk, a budgeted + * package that landed outside its declared chunk (objectui#9345), a ceiling that * has stopped measuring anything (objectui#6230), or a ceiling the base branch * replaced after this checkout (objectui#6245). These labels name the halves the * way the step log names them. @@ -115,6 +118,7 @@ const FRESHNESS_HALF = 'freshnessStatus'; const CLOSURE_HALVES = [ ['status', 'Aggregate closure ceiling'], ['chunkStatus', 'Per-chunk ceilings'], + ['membershipStatus', 'Per-chunk membership (declared packages)'], ['headroomStatus', 'Ceiling sensitivity (headroom)'], [ FRESHNESS_HALF, @@ -318,6 +322,7 @@ export function renderFromEnv(env = process.env, sizeReportPath = 'size-report.m closureBudgetKb: env.BUDGET_CLOSURE_BUDGET_KB, closureChunks: env.BUDGET_CLOSURE_CHUNKS, closureChunkStatus: env.BUDGET_CLOSURE_CHUNK_STATUS, + closureMembershipStatus: env.BUDGET_CLOSURE_MEMBERSHIP_STATUS, closureHeadroomStatus: env.BUDGET_CLOSURE_HEADROOM_STATUS, closureFreshnessStatus: env.BUDGET_CLOSURE_FRESHNESS_STATUS, budgetOutcome: env.BUDGET_STEP_OUTCOME,