Skip to content

[finding] The same dead i18n fallback spelled t(key) || 'English' — 24 sites, all on keys en defines, and #3810's new rule cannot see any of them #4117

Description

@yinlianghui

Measured while implementing #3810 (PR aligning 43 inline defaultValue strings and making the call-site gate enforce byte-equality). Not fixed there — the delegated ruling on #3810 took option B as written, i.e. the defaultValue: spelling only. This is the other spelling of the identical mechanism, filed so the measurement does not stay buried in a closed card's comments.

The measurement (first-hand, current origin/main @ 877385a76)

AST sweep of every t(...)/tt(...) call used as the LEFT operand of || or ??, with a literal key:

sites: 24 | keys en defines: 24 | distinct keys: 18
{
  "packages/app-shell/src/console/marketplace/MarketplacePackagePage.tsx": 16,
  "packages/app-shell/src/views/ObjectView.tsx": 5,
  "packages/app-shell/src/layout/ContextSelectors.tsx": 2,
  "packages/app-shell/src/views/ActionResultDialog.tsx": 1
}

24 of 24 are on keys en already defines, so t() returns a non-empty string, so the right-hand side never evaluates. Structurally the same dead code #3810 is about, one operator away.

(The earlier comments on #3810 counted 21 for two files; ContextSelectors.tsx and ActionResultDialog.tsx add the three that sweep did not cover.)

Why the new gate does not catch it

The rule that landed for #3810 reads the defaultValue property of a t() call's options argument. These sites carry no options argument at all — the fallback is a sibling expression. scripts/check-i18n-call-site-keys.mjs already holds the AST node, so the criterion could be written as the SEMANTIC one ("a fallback for a key that exists must repeat its value") rather than the syntactic one, with node.parent deciding whether the call is a binary operand.

What it costs, and one edge that is worse than #3810's

Most of the 24 are merely redundant — the dead string equals the pack value. Five say something different, and three of those are the "misleads the reader" shape in full:

site en renders dead right-hand side
MarketplacePackagePage.tsx:675 marketplace.detail.moreOptions More install options 'More options'
MarketplacePackagePage.tsx:429 marketplace.detail.purgeSuccess Removed {{count}} sample record(s). `Removed ${removed} sample record(s).`
MarketplacePackagePage.tsx:453 marketplace.detail.reseedLocalSuccess Re-seeded sample data: {{inserted}} inserted, {{updated}} updated. `Sample data re-seeded (inserted=${inserted}, updated=${updated}).`
ObjectView.tsx:939 console.objectView.deleteViewConfirm Are you sure you want to delete the view "{{name}}"? This cannot be undone. template literal inlining viewLabel
ContextSelectors.tsx:411 actionDialog.selectPlaceholder Select {{label}} `Select ${label}…`

The edge defaultValue does not have: || cannot rescue anything, in either direction. With a provider, the pack wins (dead). With no provider and no initialised i18next instance, react-i18next's not-ready t returns the key itself — truthy — so the right side is still skipped and the user reads a raw console.objectView.delete. So on a provider-less host this spelling degrades strictly worse than the defaultValue spelling, which at least renders English there. Same family as #3865 (createSafeTranslation's provider-less path ignoring the inline default and rendering the raw key); this is the || half of it.

Suggested direction

Widen #3810's rule to the fallback SEMANTICS rather than adding a second rule: when t(literalKey) is the left operand of ||/?? and the key exists in en, require the right operand — where it is a static string — to equal the en value, and treat a template-literal right-hand side whose pack value uses i18next interpolation as a finding in its own right (those five rows). Non-literal right sides (ContextSelectors.tsx:409, whose fallback is a variable) stay counted, not judged, exactly as #3810 treats computed defaults.

Not urgent: nothing a console user hits today, since every one of these keys is in the pack.

Related: #3810 (the defaultValue half, gate + 43 alignments), #3865 (provider-less path renders raw keys), #3546 (slice one deleted the five || sites whose keys were MISSING; these 24 were invisible to it precisely because their keys exist).


Generated by Claude Code

Activity

  1. self-assigned this
    on Aug 10, 2026
  2. yinlianghui commented on Aug 10, 2026

    @yinlianghui
    CollaboratorAuthor

    Claim: PM loop round 6 (develop-only window — ⛔ PRs stay draft/unmerged until the release window reopens)
    Session: session_017Qqyix2QcnpUC9XeYVDzx3 · Branch: claude/issue-4117-dead-or-fallbacks · Worktree: objectui-issue-4117
    File surface: scripts/check-i18n-call-site-keys.mjs (+ self-test) and the 24 call sites in the four named app-shell files. Serial constraints: stacks on claude/issue-3845-i18n-param-hole-parity (PR #4136, accepted but frozen unmerged) — same script; verify at start and stack.
    Container & model: S, mode:subagent, model: opus

    Delegated ruling (maintainer's 2026-08-10 delegation; veto window open): sharper than the card's sketch — a static-string (or template-literal) sibling fallback on a key en defines is dead on EVERY path (provider → pack wins; provider-less → not-ready t returns the raw key, which is truthy, so the RHS never evaluates there either — the card's own measurement), so the rule is existence-is-red demanding deletion, not RHS-equality: one blessed fallback spelling (defaultValue, gated by #3810's rule) instead of two competing ones. Stock: delete all 24 dead right-hand operands. At the five divergent rows (pack value carries {{holes}}): verify PER ROW what the call actually passes and what renders today — if a call does not pass the params its pack value's holes need, deleting the || must come WITH wiring the params (that half is then guarded by #3845's interpolation-parity class; a raw {{count}} reaching a user is exactly what that rule exists to stop). Non-literal right sides: counted, not judged, per the family's abstention discipline.


    Generated by Claude Code

  3. os-zhuang commented on Aug 10, 2026

    @os-zhuang
    Contributor

    Findings triage: stale finding label removed (label changed with this comment; pm:queue / pm:dispatched / the claim untouched).

    This card was claimed and dispatched with a delegated ruling at 12:16Z while still carrying finding — a held-observation label on an in-flight card is the half-state the label discipline forbids, and grading is moot once a maintainer-delegated ruling exists. Recording the removal here so the state change has its paired comment; no position is taken on the work itself, which proceeds under the existing claim.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  4. yinlianghui commented on Aug 10, 2026

    @yinlianghui
    CollaboratorAuthor

    ACCEPT (objectui seat PM, session session_017Qqyix2QcnpUC9XeYVDzx3, review of record) — PR #4147, and the record should show the dev's measurement OVERRODE this seat's ruling where they conflicted, correctly.

    The delegated ruling said all 24 sites die; the measurement says 22 — the two ContextSelectors sites are t?.(key) ?? '…' on an OPTIONAL prop, so when no translator is supplied the call is undefined and the fallback genuinely renders (their own test mounts the hook bare). The card had listed :411 among the five rows to fix; deleting it would have shipped undefined to a UI. Both now abstain as a named class (optionalCallFallbacks), test-pinned. Second correction: none of the five divergent rows was a live rendering bug — every hole-carrying value already receives its params, proven mechanically by #4136's parity class reporting nothing. And the meta-find: the gate's own file pre-filter matched no spelling in ContextSelectors.tsx, so that file had silently escaped ALL FIVE classes — widened, two call sites join the census, zero new findings (the blind spot was real but empty). The empty-frontmatter changeset is argued per-path (provider: pack wins; provider-less: truthy raw key; createSafeTranslation: defaults[key] || key) — byte-identical rendering everywhere, and the presence gate blessed it.

    Reverse verification predicted 10-red exactly, and the restored-stock probe named its site to the column. One blessed fallback spelling remains repo-wide; the || spelling is now structurally extinct and gated. Flipping ready + auto-merge.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions