From ec4d31becc6625b4d56fad399cd213502025df2d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 20:35:57 +0000 Subject: [PATCH] fix(cli): keep the Runtime: summary row at zero plugins, and stop counting an unrendered metric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `printMetadataStats` rendered `Runtime:` outside the `sections` loop, wrapped in `if (stats.plugins > 0 || stats.devPlugins > 0)`, so a stack with no plugins printed no `Runtime:` row at all — the same "reads as never asked, not as zero" drop #10504 and #10952 removed from the four sections. Folded into `sections` so it inherits the loop's no-silent-drop guarantee and the required `zeroFallback` typing, with its shipped comma-joined dim fragment style carried over verbatim through a new optional per-section `render` hook. `MetadataStats.translations` was collected on every run by `collectMetadataStats` and read by no render path at any value. Removed implementation-first; a new pin holds the general property (every collected metric reaches the output) rather than the absence of one field name. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- ...data-stats-runtime-row-and-translations.md | 36 ++++++ packages/cli/src/utils/format.ts | 85 ++++++++++++-- .../print-metadata-stats-zero-row.test.ts | 106 +++++++++++++++++- 3 files changed, 213 insertions(+), 14 deletions(-) create mode 100644 .changeset/cli-metadata-stats-runtime-row-and-translations.md diff --git a/.changeset/cli-metadata-stats-runtime-row-and-translations.md b/.changeset/cli-metadata-stats-runtime-row-and-translations.md new file mode 100644 index 0000000000..0e56111803 --- /dev/null +++ b/.changeset/cli-metadata-stats-runtime-row-and-translations.md @@ -0,0 +1,36 @@ +--- +"@objectstack/cli": patch +--- + +**Fix:** the `Runtime:` row of the `os validate` / `os info` / `os compile` summary is no longer dropped on a stack with no plugins, and `MetadataStats` no longer counts a metric nothing prints (#11172). + +Two separate holes in one function, `printMetadataStats` in `packages/cli/src/utils/format.ts`. Both were measured against the real CLI (`bin/run-dev.js validate`, `NO_COLOR=1`) on a stack declaring nothing. + +**1. `Runtime:` vanished at zero.** The row was rendered *outside* the `sections` loop, as a standalone `if (stats.plugins > 0 || stats.devPlugins > 0)` after the loop closed, so the whole summary was: + +``` + Data: 0 Objects + UI: 0 Apps + Logic: 0 Flows + Security: 0 Positions 0 Permissions +``` + +with no `Runtime:` line at all. That is the same "reads as never asked, not as zero" shape #10504 and #10952 removed from the four sections — a stack that declares no plugins is indistinguishable from a summary that simply does not report on the runtime. It now prints: + +``` + Data: 0 Objects + UI: 0 Apps + Logic: 0 Flows + Security: 0 Positions 0 Permissions + Runtime: 0 plugins +``` + +`Runtime:` was **folded into the `sections` array** rather than fixed where it stood. Being outside the loop was not incidental to the defect: it is why #10952's `zeroFallback` mechanism was structurally unable to reach this row, and a zero case hand-rolled beside the loop would have been a second, un-enforced copy of the same invariant — while `zeroFallback` is a *required* field on the array's element type precisely so the next row cannot be added without naming what it prints at zero. The per-item `> 0` filter this row already applied is the same filter the loop applies, so the only thing that had to be carried across was its fragment style, and it is carried exactly: the shipped non-zero rendering stays `Runtime: 2 plugins, 1 devPlugins` — comma-joined, fully dim, lowercase item names — rather than being restyled into the sections' ` ` two-space shape. The ruling was about the row's presence at zero, not its typography. + +`plugins` is the row's zero signal: `devPlugins` is a dev-only overlay on it, so `Runtime: 0 devPlugins` would have reported the narrower fact and stayed silent about the broader one. A row with one non-zero peer still reports only that peer (`Runtime: 4 devPlugins`), exactly as `Security:` behaves. + +**2. `translations` was counted on every run and read by nothing.** `MetadataStats` declared `translations: number` and `collectMetadataStats` populated it with `count(config.translations)`, but no render path ever read it — a stack with 40 translation bundles reported them nowhere in the summary, at *every* value rather than only at zero. The field is removed implementation-first (zero readers); giving it a rendered home, in `UI:` or a new `i18n:` row, was considered and explicitly not taken. + +The invariant that replaces it is enforced from both ends: TypeScript already requires `collectMetadataStats` to populate every field `MetadataStats` declares, and a new pin requires every field it collects to reach the rendered output. Declared ⇒ collected ⇒ rendered — a metric counted on every `os validate` and shown nowhere cannot satisfy the chain, whatever it is called, so the pin fails for the next unread metric as well as for this one. + +**One externally visible consequence beyond the summary text.** All three commands spread the whole `stats` struct into their `--json` payload, so `os validate --json`, `os info --json` and `os compile --json` no longer carry a `stats.translations` key. That field was undocumented (the CLI docs describe `--json` for these commands but declare no payload shape for `stats`), carried no schema, and has no reader anywhere in the repo — a repo-wide search for `stats.translations` returns zero consumers. The other 18 keys are unchanged. diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 32ab392f66..c9589cd40a 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -330,6 +330,24 @@ export function formatZodErrors(error: ZodError) { // ─── Metadata Statistics ──────────────────────────────────────────── +/** + * Every field here is rendered by {@link printMetadataStats}. + * + * #11172 — `translations: number` used to sit in this interface, collected by + * {@link collectMetadataStats} (`count(config.translations)`) on every + * `os validate` / `os info` / `os compile` run and then read by nothing: the + * printer had no `translations` fragment at any value, so a stack with 40 + * translation bundles reported them nowhere. The maintainer ruled + * implementation-first (2026-08-23) — delete the unread field rather than + * invent a row for it; an `i18n:` summary row was explicitly NOT approved. + * + * The invariant that replaces it: this struct carries no metric the summary + * does not print. It is enforced from both ends — TypeScript requires + * `collectMetadataStats` to populate every field declared here, and the + * `[#11172]` pin in `print-metadata-stats-zero-row.test.ts` requires every + * collected field to reach the rendered output. A metric that is declared but + * never rendered cannot satisfy both. + */ export interface MetadataStats { objects: number; objectExtensions: number; @@ -347,7 +365,6 @@ export interface MetadataStats { positions: number; permissions: number; datasources: number; - translations: number; plugins: number; devPlugins: number; } @@ -386,7 +403,6 @@ export function collectMetadataStats(config: any): MetadataStats { positions: count(config.positions), permissions: count(config.permissions), datasources: count(config.datasources), - translations: count(config.translations), plugins: count(config.plugins), devPlugins: count(config.devPlugins), }; @@ -842,6 +858,25 @@ export function printMetadataStats(stats: MetadataStats) { * items; the rationale sits at its entry below. */ zeroFallback: [string, ...string[]]; + /** + * How this section's surviving items become the printed line. + * + * Omitted by every section that renders the shipped #10504 shape — + * ` ` with the count in white and the item name dim, joined + * by two spaces (`Data: 1 Objects 2 Fields`). `Runtime:` is the one row + * that has never rendered that way and still does not: it prints + * `2 plugins, 1 devPlugins`, comma-joined and fully dim, with lowercase + * item names. That difference is pre-existing shipped output and #11172 + * deliberately did NOT change it — the ruling was about the row's + * PRESENCE at zero, not its typography, and rewriting a user-visible row's + * look while fixing its zero state would be an unruled widening. + * + * This hook is what let `Runtime:` join the array (see its entry) instead + * of getting a second, parallel no-silent-drop mechanism bolted on beside + * the loop. One row's formatting is data on the row; the "never dropped" + * guarantee stays single-sourced in the loop below. + */ + render?: (shown: Array<[string, number]>) => string; }> = [ { label: 'Data', @@ -897,13 +932,46 @@ export function printMetadataStats(stats: MetadataStats) { // so it is the shipped shape rather than a second formatting concept. zeroFallback: ['Positions', 'Permissions'], }, + { + // #11172 — `Runtime:` used to be rendered OUTSIDE this loop, as a + // standalone `if (stats.plugins > 0 || stats.devPlugins > 0)` after the + // loop closed, so a stack with no plugins and no devPlugins printed no + // `Runtime:` line at all. Same "reads as never asked, not as zero" defect + // #10504 and #10952 removed from the sections, and measured the same way + // (`bin/run-dev.js validate`, `NO_COLOR=1`, a stack declaring nothing). + // The maintainer ruled it in (2026-08-23): `Runtime:` renders + // unconditionally, joining the no-silent-drop invariant. + // + // Folded into the array rather than fixed in place. Being outside the + // loop was not incidental to the defect — it is why #10952's mechanism + // could not reach this row, and a hand-rolled zero case beside the loop + // would have been a SECOND copy of the invariant, un-enforced by the + // `zeroFallback` typing that stops the next row from being added without + // one. The per-item `> 0` filter this row already applied is the same + // filter the loop applies, so the only thing that had to be carried over + // was its fragment style — see `render` on the type above. + label: 'Runtime', + items: [ + ['plugins', stats.plugins], + ['devPlugins', stats.devPlugins], + ], + // `plugins` is this row's signal: `devPlugins` is a dev-only overlay on + // it, so `Runtime: 0 devPlugins` would report the narrower fact and stay + // silent about the broader one. + zeroFallback: ['plugins'], + render: (shown) => chalk.dim(shown.map(([k, v]) => `${v} ${k}`).join(', ')), + }, ]; + /** The shipped #10504 section shape — see `render` on the type above. */ + const countFragments = (shown: Array<[string, number]>) => + shown.map(([k, v]) => `${chalk.white(v)} ${chalk.dim(k)}`).join(' '); + for (const section of sections) { let shown = section.items.filter(([, v]) => v > 0); if (shown.length === 0) { - // Never drop the row (#10504, #10952) — the row is what says "this - // project has none of this"; its absence says nothing at all. + // Never drop the row (#10504, #10952, #11172) — the row is what says + // "this project has none of this"; its absence says nothing at all. shown = section.zeroFallback .map((key) => section.items.find(([itemKey]) => itemKey === key)) .filter((item): item is [string, number] => item !== undefined); @@ -913,14 +981,7 @@ export function printMetadataStats(stats: MetadataStats) { if (shown.length === 0) continue; } - const line = shown.map(([k, v]) => `${chalk.white(v)} ${chalk.dim(k)}`).join(' '); + const line = (section.render ?? countFragments)(shown); console.log(` ${chalk.bold(section.label + ':')} ${line}`); } - - if (stats.plugins > 0 || stats.devPlugins > 0) { - const parts: string[] = []; - if (stats.plugins > 0) parts.push(`${stats.plugins} plugins`); - if (stats.devPlugins > 0) parts.push(`${stats.devPlugins} devPlugins`); - console.log(` ${chalk.bold('Runtime:')} ${chalk.dim(parts.join(', '))}`); - } } diff --git a/packages/cli/test/print-metadata-stats-zero-row.test.ts b/packages/cli/test/print-metadata-stats-zero-row.test.ts index 0b05e47e30..880f2be35f 100644 --- a/packages/cli/test/print-metadata-stats-zero-row.test.ts +++ b/packages/cli/test/print-metadata-stats-zero-row.test.ts @@ -25,7 +25,7 @@ */ import { describe, expect, it } from 'vitest'; -import { printMetadataStats, type MetadataStats } from '../src/utils/format.js'; +import { collectMetadataStats, printMetadataStats, type MetadataStats } from '../src/utils/format.js'; /** Drop SGR sequences so an assertion reads the words, not chalk's opinion. */ const stripAnsi = (s: string) => s.replace(/\u001B\[[0-9;]*m/g, ''); @@ -63,7 +63,6 @@ const ZERO_APPS_STATS: MetadataStats = { positions: 0, permissions: 0, datasources: 0, - translations: 0, plugins: 0, devPlugins: 0, }; @@ -178,3 +177,106 @@ describe('[#10952] printMetadataStats prints every section\'s zero state — no expect(out).not.toContain('0 Positions'); }); }); + +/** + * #11172 — the two rows of `printMetadataStats` that #10952 did not reach. + * + * Both measured at that card's head against the real CLI (`bin/run-dev.js + * validate`, `NO_COLOR=1`) on a stack declaring nothing. The whole summary was + * + * Data: 0 Objects + * UI: 0 Apps + * Logic: 0 Flows + * Security: 0 Positions 0 Permissions + * + * with no `Runtime:` row at all, and no report of `stats.translations` at any + * value — the count was collected on every run and read by nothing. + * + * The maintainer ruled both halves on 2026-08-23 (issue comment 5386673724), + * direction 1: `Runtime:` renders unconditionally, and `translations` is + * REMOVED from `MetadataStats` implementation-first. Giving `translations` a + * rendered home — in `UI:` or a new `i18n:` row — was explicitly not approved, + * so no pin here may assert one. + */ +describe('[#11172] printMetadataStats: the Runtime: row survives zero, and no metric is counted unread', () => { + it('Runtime: prints "Runtime: 0 plugins" on a stack with no plugins and no devPlugins', () => { + const out = render(ALL_ZERO_STATS); + // The regression: before the fix `Runtime:` was rendered OUTSIDE the + // sections loop, wrapped in `if (stats.plugins > 0 || stats.devPlugins > 0)`, + // so this output contained no `Runtime:` line whatsoever. + expect(out).toContain('Runtime: 0 plugins'); + }); + + it('the zero row is the only thing that changed: the shipped non-zero rendering is byte-identical', () => { + // `2 plugins, 1 devPlugins` — comma-joined, lowercase item names, no + // two-space section join. Folding this row into the `sections` array is + // what let it inherit the no-silent-drop guarantee, and this pin is what + // stops that fold from quietly restyling it into `2 plugins 1 devPlugins`. + const out = render({ ...ALL_ZERO_STATS, plugins: 2, devPlugins: 1 }); + expect(out).toContain('Runtime: 2 plugins, 1 devPlugins'); + expect(out).not.toContain('Runtime: 0 plugins'); + }); + + it('a non-zero peer still suppresses the zero fallback, one item at a time', () => { + expect(render({ ...ALL_ZERO_STATS, plugins: 3 })).toContain('Runtime: 3 plugins'); + // devPlugins alone: the row reports the peer that exists and does NOT pad + // itself with the `0 plugins` fallback, exactly as `Security:` behaves. + const devOnly = render({ ...ALL_ZERO_STATS, devPlugins: 4 }); + expect(devOnly).toContain('Runtime: 4 devPlugins'); + expect(devOnly).not.toContain('0 plugins'); + }); + + it('Runtime: stays the last row — the fold must not reorder the summary', () => { + const rows = render(ALL_ZERO_STATS).split('\n').filter((l) => l.includes(':')); + expect(rows.map((l) => l.trim().split(':')[0])).toEqual(['Data', 'UI', 'Logic', 'Security', 'Runtime']); + }); + + /** + * The `translations` half, pinned as the general property rather than as the + * absence of one field name. + * + * `collectMetadataStats` returns a `MetadataStats`, so TypeScript already + * forces every field DECLARED on that interface to be collected. This pin + * closes the other end — every field COLLECTED must reach the printed + * output. Declared ⇒ collected ⇒ rendered: a metric counted on every + * `os validate` and shown nowhere cannot satisfy the chain, whatever it is + * called, so this fails for the next unread metric as well as for the one + * the card measured. + * + * Every count is given a distinct non-zero value because a `0` is legitimately + * filtered out of its section's fragments — zero-valued items are covered by + * the zeroFallback pins above, and mixing the two would make this one unable + * to distinguish "filtered at zero" from "has no renderer at all". + */ + it('every metric collectMetadataStats counts is rendered somewhere in the summary', () => { + const keys = Object.keys(collectMetadataStats({})); + // Anti-vacuity floor: an empty (or accidentally shrunken) key list would + // satisfy the loop below perfectly while asserting nothing. 18 is the count + // after #11172 retired `translations`; retiring another metric under + // enforce-or-remove means lowering this deliberately, which is the point. + expect(keys.length).toBeGreaterThanOrEqual(18); + + // Distinct 3-digit counts, so no metric's value can be satisfied by another + // metric's rendered number. + const stats = Object.fromEntries(keys.map((k, i) => [k, 101 + i])) as unknown as MetadataStats; + const out = render(stats); + + for (const [key, value] of Object.entries(stats)) { + expect(out, `${key} is counted by collectMetadataStats but never rendered by printMetadataStats`) + .toMatch(new RegExp(`\\b${value}\\b`)); + } + }); + + it('translations specifically: the field the ruling deleted is neither collected nor rendered', () => { + // Named explicitly because the general pin above cannot see a field that is + // re-declared and re-collected under a rendered alias, and because the + // ruling was about THIS field. A config that declares 40 bundles must not + // put a `40` back into the summary through a `translations` count. + const collected = collectMetadataStats({ + translations: Array.from({ length: 40 }, (_, i) => ({ [`l${i}`]: {} })), + }); + expect(Object.keys(collected)).not.toContain('translations'); + expect(render(collected as MetadataStats)).not.toMatch(/\b40\b/); + expect(render(collected as MetadataStats)).not.toMatch(/i18n|[Tt]ranslation/); + }); +});