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/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())); + }); +}); 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