From 028f271f93dd9493f962e6a0edc8cb32bc780938 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 17:52:42 +0000 Subject: [PATCH 1/2] fix(cli): os i18n check counts the coverage an app actually owns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectExpectedEntries` walks the Studio metadata-form registries unconditionally, so every stack's expected set carries ~773 `metadataForms.*` keys that `@objectstack/platform-objects` translates and the runtime serves. `os lint` hides them and says so; `os i18n extract` has `--no-metadata-forms`; `os i18n check` — the one command that publishes a PERCENTAGE — carried them in its denominator, so an application with its own surface fully translated read 38.9% and `--strict` / `--threshold` could not gate it. Ownership is now OBSERVED rather than assumed: the baseline counts when the stack itself ships those translations, and does not when it does not. That keeps the package which owns the family gated with no flag, which an unconditional exclusion would not. `--include-platform` (`os lint`'s own spelling) and `--no-include-platform` force either way. `os lint` is unchanged: the shared seam still defaults to counting the baseline, because lint folds it away one seam later and counts what it folded for its own hint line. Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 Co-authored-by: Claude --- packages/cli/src/commands/i18n/check.ts | 78 ++++++ packages/cli/src/utils/i18n-coverage.ts | 128 ++++++++- .../i18n-check-platform-bucket.e2e.test.ts | 175 +++++++++++++ .../cli/test/i18n-platform-bucket.test.ts | 245 ++++++++++++++++++ .../cli/test/i18n-walk-output-parity.test.ts | 220 ++++++++++++++++ 5 files changed, 845 insertions(+), 1 deletion(-) create mode 100644 packages/cli/test/i18n-check-platform-bucket.e2e.test.ts create mode 100644 packages/cli/test/i18n-platform-bucket.test.ts create mode 100644 packages/cli/test/i18n-walk-output-parity.test.ts diff --git a/packages/cli/src/commands/i18n/check.ts b/packages/cli/src/commands/i18n/check.ts index 4ebbcdf62a..bbdc5b8ea2 100644 --- a/packages/cli/src/commands/i18n/check.ts +++ b/packages/cli/src/commands/i18n/check.ts @@ -1,5 +1,44 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +/** + * `os i18n check` — the coverage gate. + * + * ## The platform `metadataForms.*` baseline, across all three commands + * + * `collectExpectedEntries` walks the Studio metadata-form registries + * unconditionally, identically for every config — ~773 keys that + * `@objectstack/platform-objects` translates and the runtime serves. Three + * commands see that family and each has to say something about it. They used + * to say three different things, and this one said nothing at all, which is + * how `--strict` / `--threshold` — the two flags whose entire purpose is CI + * gating — became unusable for an application package: every app read ~39% + * with its own surface fully translated, and the only way to "fix" the number + * was to ship a copy of the platform's bundle that would override it and go + * stale at the next upgrade. + * + * command what the baseline does to it default opt-in / out + * ------------------ ------------------------------- ------------- --------------------------- + * os lint adds findings to the report hidden --include-platform + * os i18n extract adds a companion FILE / JSON emitted --no-metadata-forms + * member (`metadataFormsCounts` + * reports its size either way) + * os i18n check moves the coverage DENOMINATOR auto: counted --include-platform / + * only when --no-include-platform + * this stack + * ships their + * translations + * + * ⚠️ The three differ because the OUTPUTS differ, and reading the table as + * three dialects of one setting is the mistake it exists to prevent: `lint` + * reports findings and can fold at the report seam; `extract` writes files and + * chooses a file set; only `check` publishes a **percentage**, so for it the + * question is which keys are in the denominator. That is also why this command + * is the one that can answer it without a flag — ownership of the baseline is + * observable from the config's own bundles ({@link stackAuthorsMetadataForms}), + * so an app gets its own number and `platform-objects`, which ships those + * translations, keeps being gated on them. + */ + import { Args, Command, Flags } from '@oclif/core'; import chalk from 'chalk'; import { normalizeStackInput } from '@objectstack/spec'; @@ -52,6 +91,7 @@ export default class I18nCheck extends Command { '$ os i18n check ./objectstack.config.ts', '$ os i18n check --locales=en,zh-CN,ja-JP', '$ os i18n check --strict --threshold=95', + '$ os i18n check --include-platform', '$ os i18n check --json', ]; @@ -78,6 +118,25 @@ export default class I18nCheck extends Command { 'show-keys': Flags.boolean({ description: 'List every missing key (otherwise the first 20 per locale are shown)', }), + // The same flag NAME and the same default as `os lint`, deliberately: this + // command was the odd one out of three, and a third vocabulary for one + // decision is what made an author go read the source to find out whether + // the platform bucket counts. `os i18n extract` spells its half + // `--no-metadata-forms`, which selects an emitted FILE SET rather than a + // gated population — see the table in the module note at the top of this + // file. + // + // `allowNo` gives the third state a percentage gate needs. Absent, the + // decision is `auto` — observed from the config, so neither an app nor the + // platform package has to discover a flag to get the right number. + // `--include-platform` forces the baseline in; `--no-include-platform` + // forces it out, for a package that ships a partial baseline and does not + // intend to own the rest of it. + 'include-platform': Flags.boolean({ + allowNo: true, + description: + 'Count platform built-in metadata forms toward coverage (default: only when this stack ships their translations — the platform packages own them otherwise)', + }), }; async run(): Promise { @@ -98,6 +157,14 @@ export default class I18nCheck extends Command { defaultLocale: flags['default-locale'], locales: flags.locales ? flags.locales.split(',').map((s) => s.trim()).filter(Boolean) : undefined, strict: flags.strict, + // Unset ⇒ `auto`. ⛔ Not `?? false`: an absent boolean and an explicit + // `--no-include-platform` are different requests here, and collapsing + // them would delete the observed-ownership default that makes this + // command usable without a flag on both sides. + platformMetadataForms: + flags['include-platform'] === undefined + ? 'auto' + : flags['include-platform'] ? 'include' : 'exclude', }); const thresholdViolations = flags.threshold !== undefined @@ -126,6 +193,17 @@ export default class I18nCheck extends Command { chalk.dim(` (${stat.translated}/${stat.expected}, missing ${stat.missing})`), ); } + // Printed under the table, where the denominator it explains is: every + // number above was computed without these keys. Same sentence shape as + // `os lint`'s, and rendered from the same two fields `--json` carries in + // `platformMetadataForms`, so the two faces cannot disagree. + if (report.platformMetadataForms.excludedKeys > 0) { + console.log( + chalk.dim( + ` platform built-ins: ${report.platformMetadataForms.excludedKeys} key(s) not counted — rerun with --include-platform to gate them here`, + ), + ); + } console.log(''); // ── Per-locale missing keys ── diff --git a/packages/cli/src/utils/i18n-coverage.ts b/packages/cli/src/utils/i18n-coverage.ts index b3741cac62..c25440746e 100644 --- a/packages/cli/src/utils/i18n-coverage.ts +++ b/packages/cli/src/utils/i18n-coverage.ts @@ -99,8 +99,47 @@ export interface CoverageReport { errors: number; warnings: number; }; + /** + * What this run did with the registry-driven `metadataForms.*` baseline, and + * how many keys that decision moved. + * + * Reported rather than left implicit because the decision moves the + * **denominator**: `stats[].expected` and `coveragePercent` mean different + * things under the two modes, and a consumer reading a percentage out of + * `os i18n check --json` has no other way to tell which one it is holding. + * The console hint line is rendered from these same two numbers, so the two + * faces of the command cannot disagree about it. + * + * `excludedKeys` is `0` under `'included'` — never absent, so a machine + * consumer keying off presence never has to distinguish "counted them" from + * "this version does not tell me". + */ + platformMetadataForms: { + mode: PlatformMetadataFormsMode; + /** Authored platform keys dropped from the expected set (0 when included). */ + excludedKeys: number; + }; } +/** The disposition a report actually reached — never `'auto'`, which is a request. */ +export type PlatformMetadataFormsMode = 'included' | 'excluded'; + +/** + * What a caller asks for; {@link resolvePlatformMetadataForms} turns it into a + * {@link PlatformMetadataFormsMode}. + * + * - `'include'` — count the baseline. The **default**, because `os lint` is + * the other caller and it folds the baseline away at the REPORT seam + * instead, off `CoverageIssue['source']`, so it needs the issues to exist + * in order to count them for its `--include-platform` hint line. ⛔ Flipping + * this default would zero that hint silently; `i18n-platform-bucket.test.ts` + * pins the coupling. + * - `'exclude'` — drop it. + * - `'auto'` — drop it unless this stack authors it (see + * {@link stackAuthorsMetadataForms}). + */ +export type PlatformMetadataFormsOption = 'include' | 'exclude' | 'auto'; + export interface CoverageOptions { /** * The locale that *must* be translated. Missing keys here surface as @@ -119,6 +158,12 @@ export interface CoverageOptions { * errors. Useful for CI gates that demand full translation parity. */ strict?: boolean; + /** + * How to treat the registry-driven `metadataForms.*` baseline. Defaults to + * `'include'` — see {@link PlatformMetadataFormsOption} for why that, and not + * `'auto'`, is the default at THIS seam. + */ + platformMetadataForms?: PlatformMetadataFormsOption; } // ─── Bundle helpers ──────────────────────────────────────────────────── @@ -177,6 +222,74 @@ function flattenBundles(bundles: TranslationBundle[]): { merged: TranslationBund return { merged, locales: Array.from(localesSet).sort() }; } +// ─── Who owns the platform baseline ──────────────────────────────────── + +/** + * Does this stack author the registry-driven `metadataForms.*` baseline + * itself? + * + * ## Why the question is asked of the CONFIG and not of a flag + * + * The `metadataForms.*` family is not walked out of the stack under + * examination at all: {@link collectExpectedEntries} builds it from + * `METADATA_FORM_REGISTRY` + `DEFAULT_METADATA_TYPE_REGISTRY`, identically for + * every config, empty ones included — ~773 Studio-form keys. For an + * application that is somebody else's surface: `@objectstack/platform-objects` + * ships those translations and the runtime serves them, so an app-shipped copy + * would *override* the platform's and go stale at the next upgrade. Counting + * them against an app's coverage percentage therefore reports a debt the app + * must not pay, which is what made `--strict` / `--threshold` unusable for an + * app package — the two flags whose entire purpose is CI gating. + * + * An unconditional exclusion is the wrong repair and is deliberately not what + * this is. It would turn the app side green by deleting the gate on the side + * that *does* own those strings: `platform-objects`' own extract config carries + * `metadataForms` in every locale bundle it declares, and its coverage number + * is a real number about real work. So ownership is **observed**, from the one + * place it is already written down — the bundles the stack itself attaches. + * Ship the baseline and you are asked to complete it; ship none of it and it + * is not yours. + * + * A non-empty **string leaf** is the test, not the mere presence of the group: + * an empty `metadataForms: {}`, or a scaffold of empty strings, is what `os + * i18n extract --fill=empty` leaves behind before anyone translates anything, + * and reading that as a claim of ownership would hand an app the 773-key debt + * on the strength of a placeholder. That is the same rule {@link lookupKey} + * applies on every other bundle read: an empty translation is not a + * translation. + */ +export function stackAuthorsMetadataForms(config: any): boolean { + const bundles: unknown[] = Array.isArray(config?.translations) ? config.translations : []; + const hasText = (node: unknown): boolean => { + if (typeof node === 'string') return node.length > 0; + if (!node || typeof node !== 'object' || Array.isArray(node)) return false; + return Object.values(node as Record).some(hasText); + }; + for (const bundle of bundles) { + if (!bundle || typeof bundle !== 'object') continue; + for (const data of Object.values(bundle as Record)) { + if (!data || typeof data !== 'object') continue; + if (hasText((data as Record).metadataForms)) return true; + } + } + return false; +} + +/** Turn a caller's request into the disposition a report will record. */ +function resolvePlatformMetadataForms( + option: PlatformMetadataFormsOption | undefined, + config: any, +): PlatformMetadataFormsMode { + switch (option ?? 'include') { + case 'exclude': + return 'excluded'; + case 'auto': + return stackAuthorsMetadataForms(config) ? 'included' : 'excluded'; + default: + return 'included'; + } +} + // ─── Expected key extraction ─────────────────────────────────────────── interface ExpectedKey { @@ -501,12 +614,24 @@ export function computeI18nCoverage(config: any, opts: CoverageOptions = {}): Co // for two opposite facts. const authoredInBundle = (path: string[]): boolean => Object.values(merged).some((data) => lookupKey(data, path) !== undefined); - const expected = collectExpectedKeys(config).filter( + const authored = collectExpectedKeys(config).filter( (key) => key.inline !== undefined || inlineLocaleAny(key.inlineLocales) !== undefined || authoredInBundle(key.path), ); + + // The platform baseline is dropped from the POPULATION, not from the issue + // list, because this report's headline number is a percentage: an app that + // has translated every string it owns reads 38.9% while 773 of its 1265 + // "expected" keys belong to `@objectstack/platform-objects`. `os lint` folds + // the same family away one seam later (`foldCoverageIssues`, keyed on + // `CoverageIssue['source']`) and can afford to, because it reports findings + // and never a denominator. + const platformMode = resolvePlatformMetadataForms(opts.platformMetadataForms, config); + const expected = + platformMode === 'included' ? authored : authored.filter((key) => key.source !== 'metadataForm'); + const excludedPlatformKeys = authored.length - expected.length; const issues: CoverageIssue[] = []; const stats: CoverageStats[] = []; @@ -570,5 +695,6 @@ export function computeI18nCoverage(config: any, opts: CoverageOptions = {}): Co errors, warnings, }, + platformMetadataForms: { mode: platformMode, excludedKeys: excludedPlatformKeys }, }; } diff --git a/packages/cli/test/i18n-check-platform-bucket.e2e.test.ts b/packages/cli/test/i18n-check-platform-bucket.e2e.test.ts new file mode 100644 index 0000000000..a3b85faf7e --- /dev/null +++ b/packages/cli/test/i18n-check-platform-bucket.e2e.test.ts @@ -0,0 +1,175 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os i18n check --strict --threshold` can gate an application package — driven + * through the published command, not through the functions behind it. + * + * `i18n-platform-bucket.test.ts` pins the decision at the seam that makes it. + * This file exists because the card is about a COMMAND: the two flags whose + * entire purpose is CI gating exited 1 on an app whose own surface was fully + * translated, and nothing short of running the command proves that they no + * longer do. It also settles the one thing a unit test structurally cannot — + * that `--include-platform` and `--no-include-platform` PARSE, and that the + * absent flag is a third state rather than a `false`. + * + * Both fixtures translate their own surface completely. The variable is + * ownership of the `metadataForms.*` baseline and nothing else: + * + * app ships no `metadataForms` bundle → not its debt → 100% + * platform ships one (as platform-objects does) → its own work → gated + * + * ## Fixture placement + * + * The stack configs go under this package's git-ignored `tmp/`, for the reason + * `i18n-extract-key-count.e2e` records: `bundle-require` writes its bundled + * module next to the config, so Node resolves the bare `@objectstack/spec` + * specifier from THAT directory, and only under `packages/cli/tmp/` does that + * lookup reach this package's real `node_modules`. `afterAll` removes only this + * suite's own `mkdtemp` directory — several suites share that root and run + * concurrently. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +const CLI_PACKAGE_ROOT = resolve(HERE, '..'); + +/** An app that translates everything it owns and ships no platform bundle. */ +const APP_CONFIG = [ + "import { defineStack } from '@objectstack/spec';", + '', + 'export default defineStack({', + " i18n: { defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'] },", + ' objects: [', + " { name: 'inquiry', label: 'Inquiry', fields: { name: { type: 'text', label: 'Name' } } },", + ' ],', + ' translations: [', + " { 'zh-CN': { objects: { inquiry: { label: '咨询', fields: { name: { label: '姓名' } } } } } },", + ' ],', + '});', + '', +].join('\n'); + +/** + * The same app, plus a `metadataForms` bundle — the shape + * `packages/platform-objects/scripts/i18n-extract.config.ts` has. Deliberately + * a partial baseline: shipping the family is the claim of ownership, finishing + * it is the work the gate then asks for. + */ +const PLATFORM_CONFIG = [ + "import { defineStack } from '@objectstack/spec';", + '', + 'export default defineStack({', + " i18n: { defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'] },", + ' objects: [', + " { name: 'sys_user', label: 'User', fields: { name: { type: 'text', label: 'Name' } } },", + ' ],', + ' translations: [', + " { en: { metadataForms: { object: { label: 'Object' } } } },", + " { 'zh-CN': {", + " objects: { sys_user: { label: '用户', fields: { name: { label: '姓名' } } } },", + " metadataForms: { object: { label: '对象' } },", + ' } },', + ' ],', + '});', + '', +].join('\n'); + +let fixtureRoot: string; +let APP: string; +let PLATFORM: string; + +beforeAll(() => { + const sharedRoot = join(CLI_PACKAGE_ROOT, 'tmp'); + mkdirSync(sharedRoot, { recursive: true }); + fixtureRoot = mkdtempSync(join(sharedRoot, 'os-i18n-16681-')); + APP = join(fixtureRoot, 'app.config.ts'); + PLATFORM = join(fixtureRoot, 'platform.config.ts'); + writeFileSync(APP, APP_CONFIG, 'utf8'); + writeFileSync(PLATFORM, PLATFORM_CONFIG, 'utf8'); +}); + +afterAll(() => { + rmSync(fixtureRoot, { recursive: true, force: true }); +}); + +interface Run { + stdout: string; + status: number | null; +} + +function runCheck(args: readonly string[]): Run { + const child = spawnSync(TSX, [CLI, 'i18n', 'check', ...args], { + cwd: CLI_PACKAGE_ROOT, + encoding: 'utf8', + env: childEnv(), + timeout: 180_000, + }); + return { stdout: `${child.stdout ?? ''}${child.stderr ?? ''}`, status: child.status }; +} + +/** The one JSON document `--json` is contracted to print. */ +function json(run: Run): any { + const start = run.stdout.indexOf('{'); + if (start === -1) throw new Error(`no JSON in:\n${run.stdout}`); + return JSON.parse(run.stdout.slice(start)); +} + +const zhCN = (report: any) => report.stats.find((s: any) => s.locale === 'zh-CN'); + +describe('os i18n check — an app package can gate on its own coverage (#16681)', () => { + it('reports 100% and exits 0 under --strict --threshold=100', () => { + const run = runCheck([APP, '--json', '--strict', '--threshold=100']); + const report = json(run); + expect(report.platformMetadataForms.mode).toBe('excluded'); + expect(report.platformMetadataForms.excludedKeys).toBeGreaterThan(0); + expect(zhCN(report).coveragePercent).toBe(100); + expect(report.thresholdViolations).toEqual([]); + expect(run.status).toBe(0); + }, 120_000); + + it('is the SAME invocation that failed before — the control is --include-platform', () => { + // The firing control for the case above: identical argv plus the opt-in, + // on the identical fixture. This is what the command did unconditionally, + // and it must still be reachable — an app that wants to audit the baseline + // asks for it, and gets exactly the old numbers back. + const run = runCheck([APP, '--json', '--strict', '--threshold=100', '--include-platform']); + const report = json(run); + expect(report.platformMetadataForms).toEqual({ mode: 'included', excludedKeys: 0 }); + expect(zhCN(report).coveragePercent).toBeLessThan(100); + expect(report.thresholdViolations.length).toBeGreaterThan(0); + expect(run.status).toBe(1); + }, 120_000); + + it('prints the hidden-bucket hint on the console face', () => { + const run = runCheck([APP]); + expect(run.stdout).toContain('platform built-ins:'); + expect(run.stdout).toContain('--include-platform'); + }, 120_000); + + it('⛔ still gates the package that SHIPS the baseline, with no flag at all', () => { + // Triage's negative control, end to end. `--threshold=100` on a stack that + // owns the baseline and has translated one key of it must FAIL. + const run = runCheck([PLATFORM, '--json', '--strict', '--threshold=100']); + const report = json(run); + expect(report.platformMetadataForms).toEqual({ mode: 'included', excludedKeys: 0 }); + expect(report.issues.some((i: any) => i.source === 'metadataForm')).toBe(true); + expect(run.status).toBe(1); + }, 120_000); + + it('accepts --no-include-platform, so the third state really is three states', () => { + // Proves the absent flag is not a parsed `false`: the same fixture answers + // `included` with the flag absent and `excluded` with it negated. + const negated = json(runCheck([PLATFORM, '--json', '--no-include-platform'])); + expect(negated.platformMetadataForms.mode).toBe('excluded'); + expect(negated.platformMetadataForms.excludedKeys).toBeGreaterThan(0); + expect(negated.issues.some((i: any) => i.source === 'metadataForm')).toBe(false); + }, 120_000); +}); diff --git a/packages/cli/test/i18n-platform-bucket.test.ts b/packages/cli/test/i18n-platform-bucket.test.ts new file mode 100644 index 0000000000..fc0d9ad35f --- /dev/null +++ b/packages/cli/test/i18n-platform-bucket.test.ts @@ -0,0 +1,245 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os i18n check` counts the coverage of the strings the stack under + * examination actually OWNS. + * + * ## What was wrong + * + * `collectExpectedEntries` walks the Studio metadata-form registries + * unconditionally — identically for every config, an empty one included — so + * every stack's expected set carries ~773 `metadataForms.*` keys that + * `@objectstack/platform-objects` translates and the runtime serves. `os lint` + * has always known those are not the author's: it hides them and says so + * ("platform built-ins: 773 i18n issue(s) hidden — rerun with + * --include-platform"). `os i18n extract` has always known: `--no-metadata-forms`. + * `os i18n check` did not, and it is the one command that publishes a + * PERCENTAGE, so the baseline sat in its denominator: + * + * Coverage by locale + * en ████████████████████████ 100.0% (1265/1265, missing 0) + * zh-CN █████████░░░░░░░░░░░░░░░ 38.9% (492/1265, missing 773) + * + * — an application with every key it owns translated, reading 38.9%. That made + * `--strict` and `--threshold`, whose entire purpose is CI gating, unusable for + * an app package. The only way to move the number was to ship a copy of the + * platform's bundle, which would override the platform's own and go stale at + * the next upgrade: the workaround is worse than the defect. + * + * ## Why ownership is OBSERVED and not simply excluded + * + * An unconditional exclusion turns the app side green by deleting the gate on + * the side that does own those strings. `platform-objects`' extract config + * carries `metadataForms` in every locale bundle it declares, and its coverage + * number is a real number about real work — so the negative control below is + * as load-bearing as the main case: THE PLATFORM PACKAGE MUST STILL BE GATED, + * and without passing a flag, because "the third command should not require an + * author to discover a flag" is the whole point of the repair. + * + * ## No absolute counts + * + * The baseline's size moves whenever a metadata type or a form field is added. + * A pin on 773 would red on unrelated work and, worse, would go green again if + * the family were dropped to zero — the regression it exists to catch. So every + * assertion here is a RELATION between two runs of the same fixture, and every + * "none of these" has a run of the same fixture that produces them. + */ + +import { describe, it, expect } from 'vitest'; +import { Parser } from '@oclif/core'; +import { + computeI18nCoverage, + stackAuthorsMetadataForms, + type CoverageReport, +} from '../src/utils/i18n-coverage.js'; +import { foldCoverageIssues } from '../src/commands/lint.js'; +import I18nCheck from '../src/commands/i18n/check.js'; + +const LOCALE = 'zh-CN'; + +/** An ordinary application: its own object, no platform bundle of any kind. */ +function appStack(): any { + return { + i18n: { defaultLocale: 'en', supportedLocales: ['en', LOCALE] }, + objects: [ + { name: 'inquiry', label: 'Inquiry', fields: { name: { label: 'Name' } } }, + ], + }; +} + +/** + * The same application with its own surface fully translated — the reporter's + * situation exactly, and the one where the old denominator was visible as a + * percentage rather than as a key list. + */ +function fullyTranslatedAppStack(): any { + return { + ...appStack(), + translations: [ + { [LOCALE]: { objects: { inquiry: { label: '咨询', fields: { name: { label: '姓名' } } } } } }, + ], + }; +} + +/** + * A stack shaped like `packages/platform-objects/scripts/i18n-extract.config.ts`: + * it SHIPS the metadata-form baseline, so the baseline is its own work. + * Deliberately partial — ownership is a claim about who translates the family, + * not a claim to have finished. + */ +function platformStack(): any { + return { + i18n: { defaultLocale: 'en', supportedLocales: ['en', LOCALE] }, + objects: [{ name: 'sys_user', label: 'User', fields: { name: { label: 'Name' } } }], + translations: [ + { en: { metadataForms: { object: { label: 'Object' } } } }, + { [LOCALE]: { metadataForms: { object: { label: '对象' } } } }, + ], + }; +} + +const platformIssues = (r: CoverageReport) => r.issues.filter((i) => i.source === 'metadataForm'); + +describe('stackAuthorsMetadataForms — ownership is read off the bundles', () => { + it('is false for an application that ships none', () => { + expect(stackAuthorsMetadataForms(appStack())).toBe(false); + expect(stackAuthorsMetadataForms(fullyTranslatedAppStack())).toBe(false); + expect(stackAuthorsMetadataForms({})).toBe(false); + }); + + it('is true for a stack that ships the baseline', () => { + expect(stackAuthorsMetadataForms(platformStack())).toBe(true); + }); + + it('reads an empty scaffold as NOT authored', () => { + // `os i18n extract --fill=empty` leaves this behind before anyone + // translates anything. Reading it as a claim of ownership would hand an app + // the whole baseline on the strength of a placeholder. + const scaffolded = { + translations: [{ [LOCALE]: { metadataForms: { object: { label: '', fields: {} } } } }], + }; + expect(stackAuthorsMetadataForms(scaffolded)).toBe(false); + // Firing control: the same shape with one real string is authored. + const translated = { + translations: [{ [LOCALE]: { metadataForms: { object: { label: '对象', fields: {} } } } }], + }; + expect(stackAuthorsMetadataForms(translated)).toBe(true); + }); +}); + +describe('os i18n check — the platform baseline is out of an app’s denominator', () => { + it('drops it by default, and the app’s own surface is what remains', () => { + const config = appStack(); + const auto = computeI18nCoverage(config, { platformMetadataForms: 'auto' }); + const included = computeI18nCoverage(config, { platformMetadataForms: 'include' }); + + expect(auto.platformMetadataForms.mode).toBe('excluded'); + expect(platformIssues(auto)).toHaveLength(0); + // The firing control for that zero: the SAME fixture produces them when + // the baseline is counted, so the empty list above is a decision and not + // an inert assertion over a family this fixture never reaches. + expect(platformIssues(included).length).toBeGreaterThan(0); + + expect(auto.totals.expectedKeys).toBeLessThan(included.totals.expectedKeys); + expect(auto.platformMetadataForms.excludedKeys).toBe( + included.totals.expectedKeys - auto.totals.expectedKeys, + ); + expect(included.platformMetadataForms).toEqual({ mode: 'included', excludedKeys: 0 }); + }); + + it('lets an app with its own surface translated reach 100%', () => { + // The reporter's measurement, as a property: 38.9% became 100% without a + // single new translation, because the 773 keys were never the app's. + const config = fullyTranslatedAppStack(); + const auto = computeI18nCoverage(config, { platformMetadataForms: 'auto' }); + const included = computeI18nCoverage(config, { platformMetadataForms: 'include' }); + + const pct = (r: CoverageReport) => r.stats.find((s) => s.locale === LOCALE)?.coveragePercent; + expect(pct(auto)).toBe(100); + expect(pct(included)).toBeLessThan(100); + expect(auto.totals.errors).toBe(0); + }); + + it('⛔ still gates the package that SHIPS the baseline — no flag needed', () => { + // Triage's negative control. An unconditional exclusion would make the app + // side green by deleting this gate, so this case is what forbids that + // implementation. + const config = platformStack(); + const auto = computeI18nCoverage(config, { platformMetadataForms: 'auto' }); + + expect(auto.platformMetadataForms).toEqual({ mode: 'included', excludedKeys: 0 }); + expect(platformIssues(auto).length).toBeGreaterThan(0); + expect(auto.totals.expectedKeys).toBe( + computeI18nCoverage(config, { platformMetadataForms: 'include' }).totals.expectedKeys, + ); + }); + + it('honours both explicit requests, against the fixture’s own default', () => { + // `--include-platform` on an app that would otherwise be excluded … + const app = computeI18nCoverage(appStack(), { platformMetadataForms: 'include' }); + expect(app.platformMetadataForms.mode).toBe('included'); + expect(platformIssues(app).length).toBeGreaterThan(0); + + // … and `--no-include-platform` on a stack that would otherwise be included. + const platform = computeI18nCoverage(platformStack(), { platformMetadataForms: 'exclude' }); + expect(platform.platformMetadataForms.mode).toBe('excluded'); + expect(platform.platformMetadataForms.excludedKeys).toBeGreaterThan(0); + expect(platformIssues(platform)).toHaveLength(0); + }); +}); + +describe('the shared seam keeps os lint whole', () => { + it('counts the baseline when no caller asks otherwise', () => { + // ⛔ The default at `computeI18nCoverage` stays `include`, and this is why: + // `os lint` folds the family away one seam later, off + // `CoverageIssue['source']`, and COUNTS what it folded for its own hint + // line. Flipping the default here would zero that hint silently — the + // issues would never be produced to be counted. + const report = computeI18nCoverage(appStack()); + expect(report.platformMetadataForms).toEqual({ mode: 'included', excludedKeys: 0 }); + + const { folded, hiddenPlatform } = foldCoverageIssues(report.issues, false); + expect(hiddenPlatform).toBeGreaterThan(0); + expect(folded.some((i) => i.rule === 'i18n/missing-metadataForm')).toBe(false); + // Firing control: the same issues, asked for, do reach the report. + expect( + foldCoverageIssues(report.issues, true).folded.some((i) => i.rule === 'i18n/missing-metadataForm'), + ).toBe(true); + }); +}); + +describe('the flag is the one os lint already publishes', () => { + it('is spelled --include-platform and carries its negation', () => { + // The card ranked "the same platform-bucket default `lint` has" ahead of + // "at minimum the `--no-metadata-forms` switch". Sharing `lint`'s spelling + // is the readable half of taking its default: an author who has met one of + // these commands has met the other. + const flag: any = (I18nCheck.flags as any)['include-platform']; + expect(flag).toBeDefined(); + expect(flag.type).toBe('boolean'); + expect(flag.allowNo).toBe(true); + // Absent ⇒ `auto`. A `default` here would erase the third state and with it + // the observed-ownership behaviour every case above depends on. + expect(flag.default).toBeUndefined(); + }); + + it('parses as THREE states, through oclif’s own parser', () => { + // Structural assertions above describe the declaration; this one describes + // the behaviour an operator gets, and it is the half that decides whether + // `auto` exists at all. Run against `I18nCheck.flags` itself — the object + // the command hands oclif — so a later `default: false` reddens here rather + // than silently collapsing the absent case onto `--no-include-platform`. + // The published command is driven end to end in the sibling e2e file; this + // is the part that need not spawn a process to be true. + return Promise.all( + ([ + [[], undefined], + [['--include-platform'], true], + [['--no-include-platform'], false], + ] as Array<[string[], boolean | undefined]>).map(async ([argv, expected]) => { + const parsed = await Parser.parse(argv, { flags: I18nCheck.flags as any, strict: true }); + expect((parsed.flags as any)['include-platform']).toBe(expected); + }), + ); + }); +}); diff --git a/packages/cli/test/i18n-walk-output-parity.test.ts b/packages/cli/test/i18n-walk-output-parity.test.ts new file mode 100644 index 0000000000..0b7391c9d5 --- /dev/null +++ b/packages/cli/test/i18n-walk-output-parity.test.ts @@ -0,0 +1,220 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The two consumers of one walk are asked for the SAME KEYS — equality, not + * "no duplicates". + * + * `collectExpectedEntries`' own docblock states the invariant this file + * measures: + * + * > This is the single place the gate lives, so `os lint`'s coverage report and + * > `os i18n extract`'s skeleton can never disagree about which keys an author + * > is being asked for. + * + * They disagreed, and the shape of the disagreement is why it survived so long: + * the same array reaches two consumers, and only ONE of them de-duplicates. + * `os i18n extract` materialises a NESTED TREE, where writing one path twice + * collapses onto one leaf; the coverage report counts `expected.length` on the + * FLAT array. A key emitted twice was therefore invisible in the skeleton and + * load-bearing in the percentage — measured on a real application at 482 keys + * from the extractor against 492 from `check`, the surplus being exactly ten + * `objects.*._actions.*` keys. + * + * ## Why equality and not "the walk has no repeats" + * + * "No duplicate paths" is the property `i18n-duplicate-demand.test.ts` pins at + * the seam it is caused at, and it is necessary. It is not sufficient, because + * it is a statement about ONE output. What the docblock promises is a relation + * BETWEEN the two — and a relation between two outputs is the only assertion + * that fails when a future change makes one of them drop a key the other still + * demands. A repair that deleted one of the two action walks would satisfy "no + * duplicates" perfectly while dropping a whole class of declaration from + * translation, so the two negative controls below are load-bearing: an app that + * declares its actions ON THE OBJECT and an app that declares them TOP-LEVEL, + * BOUND TO AN OBJECT must EACH come out complete. + * + * ## How each side is read + * + * Both are read from a PUBLISHED face, never from the walker directly — the + * point is that two consumers agree, and reading the shared upstream would + * assert only that it equals itself. + * + * extract the leaves of the skeleton `extractTranslations` builds, narrowed + * to the stack's own surface with the extractor's OWN + * `stackAuthoredSubtree` — the same function `os i18n extract` uses + * to decide what goes in the stack module, not a second definition + * of "the stack's own keys". + * check the keys `computeI18nCoverage` reports missing for a locale the + * fixture declares and ships no bundle for. With nothing translated + * there, that finding set IS the expected set, key for key — the + * same list `os i18n check --json` publishes — and reading it this + * way keeps the assertion on the command's real output face rather + * than on a count that happens to match. + * + * The fixtures author every prop they declare, deliberately. An UNAUTHORED + * optional prop is legitimately absent from both sides (nothing to scaffold, + * nothing to translate), and an unauthored DERIVED prop is legitimately present + * in one and not the other — the extractor seeds a fallback so the skeleton + * stays usable, while coverage does not demand a translation of a string nobody + * wrote. Neither asymmetry is this invariant, so the fixtures stay clear of + * both rather than encoding an exception here. + */ + +import { describe, it, expect } from 'vitest'; +import { + collectExpectedEntries, + extractTranslations, + stackAuthoredSubtree, +} from '../src/utils/i18n-extract.js'; +import { computeI18nCoverage } from '../src/utils/i18n-coverage.js'; + +const LOCALE = 'zh-CN'; + +/** Every leaf path in a translation tree, dot-joined, sorted. */ +function leafPaths(node: unknown, prefix: string[] = []): string[] { + if (node === null || typeof node !== 'object') return [prefix.join('.')]; + return Object.entries(node as Record).flatMap(([key, value]) => + leafPaths(value, [...prefix, key]), + ); +} + +/** What `os i18n extract` would scaffold for this stack, its own surface only. */ +function extractKeys(config: any): string[] { + const { bundles } = extractTranslations(config, { locales: ['en'] }); + return leafPaths(stackAuthoredSubtree(bundles.en)).sort(); +} + +/** + * What `os i18n check` asks the author for — read off the published finding + * list for a declared locale with no bundle, where every expected key is + * missing by construction. + */ +function checkKeys(config: any): string[] { + const report = computeI18nCoverage(config, { platformMetadataForms: 'exclude' }); + return report.issues.filter((i) => i.locale === LOCALE).map((i) => i.key).sort(); +} + +/** + * The action every fixture declares. One literal object; each fixture decides + * WHERE it hangs, which is the whole variable under test. + */ +function inquiryAction() { + return { + name: 'ats_convert_inquiry', + label: 'Convert Inquiry', + objectName: 'inquiry', + description: 'Turn this inquiry into a candidate.', + confirmText: 'Convert this inquiry?', + successMessage: 'Inquiry converted.', + params: [{ name: 'owner', label: 'New owner' }], + }; +} + +/** The surrounding app, identical in every fixture so only the action moves. */ +function baseStack() { + return { + i18n: { defaultLocale: 'en', supportedLocales: ['en', LOCALE] }, + objects: [ + { + name: 'inquiry', + label: 'Inquiry', + pluralLabel: 'Inquiries', + description: 'An inbound application.', + fields: { + name: { label: 'Name', help: 'Full legal name.', placeholder: 'Ada Lovelace' }, + }, + } as any, + ], + apps: [{ name: 'ats', label: 'Hiring', description: 'Applicant tracking.' }], + }; +} + +/** Shape ①: the action is declared ON the object (`obj.actions`). */ +function actionOnObject(): any { + const stack = baseStack(); + stack.objects[0].actions = [inquiryAction()]; + return stack; +} + +/** Shape ②: the action is declared TOP-LEVEL, bound to the object. */ +function actionTopLevel(): any { + return { ...baseStack(), actions: [inquiryAction()] }; +} + +/** + * Shape ③: what the normalizer really hands the walk — ONE action object + * carried by BOTH lists, by reference. This is the config that produced the + * measured 492-against-482, and the reference sharing is the point: a copy + * would not reproduce it faithfully. + */ +function actionOnBothCarriers(): any { + const stack: any = baseStack(); + const shared = inquiryAction(); + stack.objects[0].actions = [shared]; + stack.actions = [shared]; + return stack; +} + +const ACTION_KEYS = [ + 'objects.inquiry._actions.ats_convert_inquiry.label', + 'objects.inquiry._actions.ats_convert_inquiry.description', + 'objects.inquiry._actions.ats_convert_inquiry.confirmText', + 'objects.inquiry._actions.ats_convert_inquiry.successMessage', + 'objects.inquiry._actions.ats_convert_inquiry.params.owner.label', +]; + +describe('os i18n extract and os i18n check ask for the same keys', () => { + const shapes: Array<[string, () => any]> = [ + ['actions declared on the object', actionOnObject], + ['actions declared top-level, bound to an object', actionTopLevel], + ['actions on both carriers, one reference (the normalizer output)', actionOnBothCarriers], + ]; + + for (const [name, build] of shapes) { + describe(name, () => { + it('publishes the same key SET from both faces', () => { + const config = build(); + expect(checkKeys(config)).toEqual(extractKeys(config)); + }); + + it('publishes the same key COUNT from both faces', () => { + // The number the card is about — 482 from the extractor against 492 + // from `check`. Implied by the set equality above and asserted anyway, + // because the count is the thing an operator reads and the thing the + // percentage divides by. + const config = build(); + expect(checkKeys(config).length).toBe(extractKeys(config).length); + }); + + it('asks for the complete action set — the walk is not repaired by deletion', () => { + // The firing control for both assertions above: equality over an empty + // intersection would be trivially true, so this pins that the shared + // population actually CONTAINS the family under test. A fix that + // deleted either action walk turns the two assertions above green and + // reddens this one. + const config = build(); + const keys = extractKeys(config); + expect(keys).toEqual(expect.arrayContaining(ACTION_KEYS)); + expect(checkKeys(config)).toEqual(expect.arrayContaining(ACTION_KEYS)); + }); + + it('emits each action key exactly once in the walk itself', () => { + // The upstream seam, so a regression is attributable: the collapse + // belongs to the walker (`dedupeByPath`), not to either consumer. + const paths = collectExpectedEntries(build()).map((e) => e.path.join('.')); + for (const key of ACTION_KEYS) { + expect(paths.filter((p) => p === key)).toHaveLength(1); + } + }); + }); + } + + it('reaches the same key set however the action was declared', () => { + // The two negative controls, stated as one fact: WHERE an action is + // declared changes nothing about which keys its translator is asked for. + expect(extractKeys(actionTopLevel())).toEqual(extractKeys(actionOnObject())); + expect(extractKeys(actionOnBothCarriers())).toEqual(extractKeys(actionOnObject())); + expect(checkKeys(actionTopLevel())).toEqual(checkKeys(actionOnObject())); + expect(checkKeys(actionOnBothCarriers())).toEqual(checkKeys(actionOnObject())); + }); +}); From 03ca4f4eba6c0400edaecc47fb12d31cd70eb4fc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 18:00:05 +0000 Subject: [PATCH 2/2] docs(i18n): state who owns the metadata-form baseline where the gate is taught MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `content/docs/ui/translations.mdx` recommends `os i18n check --strict --threshold` as the CI gate; that advice is now reachable for an app package, and the page has to say what is and is not in the number it produces — including why shipping your own `metadataForms` bundle to move it is the wrong repair. The published i18n skill listed metadata forms among the surfaces `check` always reports on. That sentence is false as of this change, so it is corrected in place rather than left to read as a scope statement. `content/docs/protocol/kernel/i18n-standard.mdx` names `metadataForms` only in its ORPHAN-key paragraph (the reverse direction, which `os validate` / `os lint` / `os compile` answer) — untouched by this change and left alone. Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 Co-authored-by: Claude --- ...8n-check-platform-bucket-and-app-gating.md | 63 +++++++++++++++++++ content/docs/ui/translations.mdx | 17 +++++ skills/objectstack-i18n/SKILL.md | 7 ++- 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 .changeset/i18n-check-platform-bucket-and-app-gating.md diff --git a/.changeset/i18n-check-platform-bucket-and-app-gating.md b/.changeset/i18n-check-platform-bucket-and-app-gating.md new file mode 100644 index 0000000000..bd1f096b9c --- /dev/null +++ b/.changeset/i18n-check-platform-bucket-and-app-gating.md @@ -0,0 +1,63 @@ +--- +"@objectstack/cli": minor +--- + +fix(cli): `os i18n check` counts the coverage an app actually owns, so `--strict` / `--threshold` can gate an app package (#16681) + +## What was wrong + +`collectExpectedEntries` walks the Studio metadata-form registries +unconditionally — identically for every config, an empty one included — so +every stack's expected set carries ~773 `metadataForms.*` keys that +`@objectstack/platform-objects` translates and the runtime already serves. + +Two of the three commands that see that family already knew it is not the +author's. `os lint` hides it and says so ("platform built-ins: 773 i18n +issue(s) hidden — rerun with `--include-platform`"); `os i18n extract` has +`--no-metadata-forms`. `os i18n check` is the one command that publishes a +**percentage**, and it carried the baseline in its denominator: + +``` +Coverage by locale + en ████████████████████████ 100.0% (1265/1265, missing 0) + zh-CN █████████░░░░░░░░░░░░░░░ 38.9% (492/1265, missing 773) +``` + +That is an application with every key it owns translated. `--strict` and +`--threshold` — the two flags whose entire purpose is CI gating — therefore +could not gate an app package at all, and the only way to move the number was +to ship a copy of the platform's bundle, which would *override* the platform's +own and go stale at the next upgrade. The workaround was worse than the defect. + +## What it does now + +**Ownership is observed, not assumed.** The baseline counts toward coverage +when the stack under examination ships those translations itself, and does not +when it does not — read from the config's own `translations` bundles, requiring +a non-empty string leaf so an `--fill=empty` scaffold is not mistaken for a +claim of ownership. An app gets a number about its own surface with no flag; +`platform-objects`, which does ship the family, stays gated on it with no flag +either. An unconditional exclusion would have turned the app side green by +deleting the platform's own gate, and is what the negative-control tests forbid. + +**The flag is `os lint`'s, spelling and all.** `--include-platform` forces the +baseline in; `--no-include-platform` forces it out, for a package that ships a +partial baseline and does not intend to own the rest. Absent, the decision is +the observed one — three states, not two. + +**Both output faces carry the decision.** `--json` gains +`platformMetadataForms: { mode, excludedKeys }`, and the console prints +`platform built-ins: N key(s) not counted — rerun with --include-platform to +gate them here` under the coverage table, rendered from those same two numbers. + +`os lint` is unchanged. The shared `computeI18nCoverage` seam still counts the +baseline by default, because lint folds it away one seam later and counts what +it folded for its own hint line. + +## Compatibility + +Additive on the command surface; an invocation that was refused is now +accepted, and no flag is removed or renamed. The behaviour that changes is the +**default coverage number for a stack that ships no `metadataForms` bundle** — +it stops reporting a debt that stack must not pay. A run that wants the old +numbers back asks for them with `--include-platform`, on the same argv. diff --git a/content/docs/ui/translations.mdx b/content/docs/ui/translations.mdx index bdc44f6324..4e16119f3b 100644 --- a/content/docs/ui/translations.mdx +++ b/content/docs/ui/translations.mdx @@ -244,6 +244,23 @@ A missing string in the **default** locale is an error; missing strings in other locales are warnings until you set `--strict` / `--threshold`. The Todo example ships a completeness test alongside its bundles — worth copying. +### What counts as *your* coverage + +The Studio's own metadata forms (`metadataForms.*` — several hundred keys +across every metadata type) are translated by `@objectstack/platform-objects` +and served from there, so they are **not** in your coverage number: an app that +translated everything it declares reads 100%, not 39%. ⛔ Do not "fix" a low +number by shipping your own `metadataForms` bundle — yours would override the +platform's and go stale at the next upgrade. + +The rule is **ownership**, read from your own bundles rather than assumed: ship +translations for that family and you are asked to complete them, which is how +the platform packages stay gated on the strings they do own. Pass +`--include-platform` to audit the baseline anyway (`os lint`'s flag, same +meaning), or `--no-include-platform` to keep it out even though you ship part +of it. The command prints how many keys it left out, and `--json` carries the +same two numbers as `platformMetadataForms`. + ### Which locales get checked Your project decides, and the tooling never assumes. `os lint`, `os i18n check` diff --git a/skills/objectstack-i18n/SKILL.md b/skills/objectstack-i18n/SKILL.md index 9340036289..fb23505a3a 100644 --- a/skills/objectstack-i18n/SKILL.md +++ b/skills/objectstack-i18n/SKILL.md @@ -277,9 +277,10 @@ os i18n check --strict --threshold=95 # CI gate: locale parity + minimum covera It compares registered bundles against source metadata and reports missing keys per locale for every surface the extractor walks — objects and their sub-keys, -global actions, apps, dashboards, pages, flow screens, metadata forms. Gaps in -the default locale are errors, `--strict` promotes the rest, `--show-keys` lists -them all; `os lint --i18n-strict` folds the same gate into lint. +global actions, apps, dashboards, pages, flow screens. The platform's metadata +forms count only for a stack that ships their translations (`--include-platform` +audits them anyway). Gaps in the default locale are errors, `--strict` promotes +the rest, `--show-keys` lists them all; `os lint --i18n-strict` folds it in. ### `os i18n extract --check` — freshness, not coverage