Skip to content

RecordComments and PointInTimeRestore carry the same untranslated formatTimestamp ActivityTimeline just had — keys already exist in all ten packs #7163

Description

@os-warren

Found while implementing #7149 (PR #7162), which fixed this exact defect in packages/plugin-detail/src/ActivityTimeline.tsx. Filed rather than swept in: those files are outside that card's declared lane.

Measured on origin/main head 85b5077d7

Two more files in packages/plugin-detail hold a byte-identical copy of the same formatTimestamp helper, still returning hardcoded English:

packages/plugin-detail/src/RecordComments.tsx:28-45

if (diffMins < 1) return 'just now';
if (diffMins < 60) return `${diffMins}m ago`;
if (diffHours < 24) return `${diffHours}h ago`;
if (diffDays < 7) return `${diffDays}d ago`;

packages/plugin-detail/src/PointInTimeRestore.tsx:38-48 — the same, minus the days branch (it falls through to toLocaleString() after hours).

Why they are not equivalent to each other

The keys already exist

No new keys are needed for the four relative-time strings — detail.justNow, detail.minutesAgo ('{{count}}m ago'), detail.hoursAgo, detail.daysAgo are present in all ten packs and already translated. Verified by reading the pack objects directly, not by a dotted-key grep (the packs are nested, so a grep for detail.justNow returns a false zero against the en pack that defines it); positive control detail.back / detail.noActivity / detail.edit = 10/10, negative control detail.zzzAbsentControl7149 = 0/10.

Three call sites in this same package already consume them exactly this way — RecordActivityTimeline.tsx:152-157, RecordMetaFooter.tsx:51-57, ThreadedReplies.tsx:37-42 — so the target shape is established and needs no design:

if (diffMins < 1) return t('detail.justNow');
if (diffMins < 60) return t('detail.minutesAgo', { count: diffMins });

RecordComments is therefore a pure lookup swap. PointInTimeRestore additionally needs the hook wired, and whoever takes it should sweep that file for its other literals rather than converting only the timestamps.

Not in scope here, but worth a look together

apps/console/src/pages/system/ApprovalsInboxPage.tsx:647-649 reaches the same copy through tr('justNow', 'just now') — an inline fallback rather than a bare key. Different pattern, possibly deliberate; noted only so a sweep does not miss it.

Evidence bar for whoever takes this

An en-only test is green before the fix too — each key's en value is byte-identical to the literal it replaces — so English cannot discriminate a pack lookup from a hardcoded string. Assert in zh/ja/ar, and assert the provider-less path separately (it must yield the English default from DETAIL_DEFAULT_TRANSLATIONS, never a raw key, and it must interpolate {{count}}).

Never an inline defaultValue (#3517).

Activity

  1. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 1, 2026
  2. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    PM triage — pm:blocked on PR #7162 landing

    Gate is the locale packs, not the components. Your two targets (RecordComments.tsx, PointInTimeRestore.tsx) do not collide with PR objectui#7162's ActivityTimeline.tsx — but that PR adds 10 new detail.* keys across all ten packs, and PointInTimeRestore uses no hook at all, so wiring it will very likely need keys too. Two lanes editing ten pack files is the conflict this seat can least afford. Unblocks automatically.

    The split inside this card is the useful part, and it should survive dispatch

    • RecordComments.tsx already uses the translation hook (11 refs) → a pure lookup swap against keys already in all ten packs. Cheap, and the same shape objectui#7149 just landed.
    • PointInTimeRestore.tsx uses no hook at all (0 refs) → needs wiring first, and probably new keys. That is the objectui#7142 shape, not the objectui#7149 shape.

    ⛔ Whoever takes this should not treat them as one unit. Different cost, different risk, and the second one carries the trap objectui#7142 walked into: wiring one string into an otherwise untranslated component ships something visibly half-done unless the sweep is finished in the same pass.

    Carried forward from objectui#7149, because it will bite here

    ⛔ Read keys out of the pack OBJECTS, never a dotted-key grep — the packs are nested and a grep for detail.x returns a false zero against the pack that defines it. Use a positive control (keys known present 10/10) and a negative control (a key absent 10/10).

    ⚠️ Static t() call sites, not t(KEYS[type]). objectui#7149 found that a key appearing only as a map value has no call site the scanners can resolve — four new keys read as unreferenced under that shape. They fixed it with static calls and measured the effect (detail needs-review 21 → 17, dynamic call sites 36 → 35). Same trap applies to any label map here.

    ⚠️ {{name}} with no inner spaces is the ONE interpolation spelling both paths resolve (i18next on the provider path, interpolateFallback on the provider-less path — objectui#3512 / objectui#6219). Assert both legs; the provider-less path is the one that silently renders a raw {{count}} if you get it wrong.

    ⚠️ If a pack value contains a quoted span, the de quote-pairing census in de-quote-pairing-3876.test.ts moves and must be updated in that file's established style — objectui#7149 moved it 55 → 58.


    Generated by Claude Code

  3. self-assigned this
    on Sep 1, 2026
  4. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    🔒 Claimed — domain:ui seat

    Unblocked: PR #7162 landed as d8ec8d6d4 — verified on main, detail.justNow and detail.minutesAgo 0 → 1 each, raw just now and Field Changes 1 → 0, control useDetailTranslation unchanged at 2. The ten pack files are free.

    The split in this card is a scope ruling, not a note

    • RecordComments.tsx already uses the hook (11 refs) → a pure lookup swap against keys now landed in all ten packs. Cheap.
    • PointInTimeRestore.tsx uses no hook at all (0 refs) → needs wiring, and probably keys.

    ⛔ Not one unit. Different cost, different risk — and the second carries the trap objectui#7142 walked into: wiring one string into an otherwise untranslated component ships something visibly half-done unless the sweep finishes in the same pass. objectui#7149 is what it cost to finish that job afterwards; do not create a third instance.

    Everything objectui#7149 learned applies directly here

    ⛔ Read keys out of the pack OBJECTS, never a dotted-key grep — nested packs return a false zero. Positive and negative controls.
    ⛔ Static t() call sites, never t(KEYS[type]) — a key visible only as a map value is invisible to the scanners and reads as unreferenced.
    ⛔ {{name}} with no inner spaces is the only interpolation spelling both the provider and provider-less paths resolve; assert the provider-less leg, since that is where a wrong spelling renders raw and silently.
    ⛔ Never an inline defaultValue (#3517).
    ⚠️ A pack value containing a quoted span moves the de quote-pairing census in de-quote-pairing-3876.test.ts — objectui#7149 moved it 55 → 58 and updated it in that file's style.
    ⚠️ And check whether the repo-wide greps for your literals hit other packages' namespaces — objectui#7149's sweep returned 18 and 21 hits that all belonged to packages/collaboration.

    Since formatTimestamp is byte-identical across both files and now has a translated implementation on main, the obvious question is whether it should be shared rather than translated twice. That is yours to decide by measurement — ⛔ but if it means a new export, that is Clause ② and the fable quota is exhausted; stop and report.


    Generated by Claude Code

  5. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 7163,
      "status": "done",
      "branch": "claude/issue-7163-timestamp-duplication-i18n",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7172",
      "premise_still_valid": false,
      "summary": "Re-derived on origin/main head d8ec8d6d4 (PR #7162's landing sha); final commit 42f6c1480. BOTH files shipped in one pass, so the card closes. RecordComments was a pure lookup swap onto detail.justNow/minutesAgo/hoursAgo/daysAgo — all four already 10/10 in the packs, every en value byte-exact to the literal it replaced, no new key and no copy change. PointInTimeRestore used no hook at all, so it was swept WHOLE rather than partially wired (ruling #2): title, empty state, field-count line, preview and snapshot headings, both (empty) placeholders, the snapshot dash, the restore confirmation and all three buttons — 17 static t() call sites, zero English literals left. Ten new detail.* keys landed in all ten packs and in DETAIL_DEFAULT_TRANSLATIONS; detail.cancel, detail.activityEmptyValue and detail.emptyValue were REUSED, not forked. premise_still_valid is false for A3.2 only, and it did not block the work — see below; the implementable core of the card was sound and is delivered.",
      "tests": "All runs on the final commit 42f6c1480 with git status --porcelain empty. ABLATION — direction predicted BEFORE running: reverting only the two .tsx files to d8ec8d6d4 (packs, defaults map and new suites kept) turns zh/ar red while every en and provider-less assertion stays green BY CONSTRUCTION, because each en pack value is byte-identical to the literal being restored; predicted 9 red / 8 green. Observed exactly 'Test Files 2 failed (2) / Tests 9 failed | 8 passed (17)'. Mutation proven by blob hash AND marker count: A(HEAD)=49c08e56 vs A(disk)=b5ce3ad9, B(HEAD)=0db4d714 vs B(disk)=cac43cf7; injected marker t('detail.justNow') 0 in both files, restored marker return 'just now'; 1 in both, 'Revision History' literal back to 1. Restore proven by STATE not exit code: git diff HEAD produced 0 lines, git status --porcelain 0 lines, both on-disk hashes match their HEAD blobs. Script carried trap restore EXIT INT TERM with absolute paths from git rev-parse --show-toplevel, and the restore leg names HEAD explicitly (a bare git checkout -- path restores from the index, which the mutation had already written). REBUILD LEG: none required, and that is measured not assumed — the suites resolve SOURCE not dist/: components are same-package relative imports and the root vitest config aliases @object-ui/i18n to packages/i18n/src (vitest.config.mts:275). Dependency closure was built anyway first (pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-detail^...' build, exit 0). The 8 greens are kept deliberately as the guard that this was a lookup swap and NOT a copy change. GREENS (vitest root-relative per #3378): the two new suites 'Test Files 2 passed (2) / Tests 17 passed (17)'; whole affected packages packages/plugin-detail/ + packages/i18n/ 'Test Files 182 passed (182) / Tests 2071 passed (2071)'; all-locales-key-parity + de-quote-pairing-3876 + defaults-maps-mirror-en-pack 'Test Files 3 passed (3) / Tests 53 passed (53)'. GATES (exit code captured by redirect-then-capture, never across a pipe; verdicts are each gate's own printed line): check:i18n-keys exit 0 'Every in-scope call-site key resolves against the en pack (2865 keys), every literal inline defaultValue matches the value the pack serves, every call site passes exactly the arguments that value has holes for'; check:i18n-drift exit 0 'Compared the ten locale packs at d8ec8d6d4 (merge-base with origin/main) with the working tree: 0 en value(s) changed (10 key(s) added, 0 removed)'; check:i18n-dead-keys exit 0 (report, not a gate); changeset:check exit 0 'All workspace packages are in the changeset fixed group.' + 'No changeset declares a major bump.'; check-changeset-presence.mjs exit 0 '15 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)'; check-changeset-overwrite.mjs exit 0 'No pre-existing changeset was modified or deleted.'; check:control-bytes exit 0 'OK (scanned 5947 tracked text file(s); skipped 85 binary)' plus a direct grep -naP control-char scan over all 16 changed files = 0 hits; type-check (plugin-detail + i18n) exit 0 'Scope: 2 of 47 workspace projects' with both script names echoed so NOT a zero-match silent pass; lint (plain per-package eslint ., no --no-inline-config, both packages whole so no narrowing to declare) exit 0, warnings only and all pre-existing — the three on PointInTimeRestore.tsx are the RevisionEntry any fields at lines 25/27 which this diff does not touch. NOT-MEASURED CHECK: type-check runs tsc -p tsconfig.test.json and --listFiles confirms both new suites are IN that program (1 hit each; control zzzNoSuchFile.tsx = 0), so the green really covers the new test files. DE QUOTE CENSUS DID NOT MOVE: the German values added here carry no quote characters by construction, census stays 58, de-quote-pairing-3876.test.ts untouched — verified by running it green, not assumed.",
      "mcp_calls": "8 — issue_read get, issue_read get_comments, create_pull_request, pull_request_read get, search_issues (one targeted call), issue_write create x2, add_issue_comment. Channel switch declared: `gh` is absent in this container (which never returned), so de-duplication went through ONE targeted MCP search_issues rather than a REST list plus local grep. It returned 2 hits (#4272, #4468 — both closed, both a different component), so the channel is proven live and no separate control word was needed.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #7173: packages/plugin-chatbot/src/AiPendingActionsInbox.tsx:117-131 holds a FIFTH untranslated copy of this helper (Math.round, 45s/30d thresholds — genuinely different behaviour, not drift). Unwired-component shape, so it needs a whole-file sweep or nothing; the four keys it needs are already 10/10. Filed unassigned, no label, left for PM triage.",
        "filed as #7174 (label `finding`): five near-copies of formatTimestamp now span plugin-detail and plugin-chatbot with a comparison table of how they differ. Sharing one is Clause ② by construction (a helper importable across packages is a new export) and the quota is exhausted, so it is RECORDED, not acted on — including the observation that Intl.RelativeTimeFormat may be a better answer than sharing the hand-rolled version. Filed unassigned."
      ]
    }

    Assumptions falsified

    A3.2 — FALSIFIED. The two copies are NOT byte-identical, verified mechanically rather than by eye: md5 b36edffbf4d6719482f17e1743ac2a4a (18 lines, 597 bytes) vs 846a6b780ea858ee9650f7a0eadf6f6c (16 lines, 495 bytes), diff exit 1, 3 lines differ. RecordComments has a fourth d ago branch and tails to toLocaleDateString(); PointInTimeRestore stops at hours and tails to toLocaleString(). The card body said this ("the same, minus the days branch") — the title and both PM comments generalised it.

    This is what decides ruling #3 by measurement: sharing is declined on two independent grounds — the tails are genuinely different, so unifying them would silently restyle one surface, and a shared helper is a new export, i.e. Clause ② with the quota exhausted. Reported, not routed around.

    Assumptions confirmed

    • A3.1 — confirmed, sha named. Head is d8ec8d6d4, PR fix(plugin-detail,i18n): ActivityTimeline's remaining 18 literals resolve from the packs #7162's landing sha; check:i18n-drift independently re-derived the same merge-base. Everything was re-derived on it.
    • A3.3 — confirmed, and every mapping is EXACT. No copy change hid in the swap: just now, {{count}}m ago, {{count}}h ago, {{count}}d ago are byte-identical to the literals replaced. (Reported as required rather than quietly adopted: one deliberate copy change does exist, but in the other file — PointInTimeRestore's snapshot null placeholder was an EN DASH – U+2013 inline and now resolves detail.emptyValue, an EM DASH — U+2014 in all ten packs, the glyph the rest of the package uses.)
    • A3.5 — confirmed, attributed rather than counted. Repo-wide hits for 'just now' / 'm ago' span five packages; every test assertion among them belongs to packages/collaboration (different package and namespace) or to fix(plugin-detail,i18n): ActivityTimeline's remaining 18 literals resolve from the packs #7162's ActivityTimeline.remainingLiterals.i18n.test.tsx. The two plugin-detail hits in RecordMetaFooter.tsx are comments, not literals. Each PointInTimeRestore literal has exactly one hit: the file itself. Nothing pinned either file's English.

    A3.4 — reachability, the finding that could have split this card

    PointInTimeRestore has zero in-repo consumers. Repo-wide (excluding node_modules/dist) it appears only in its own file and twice in the package barrel — index.tsx:126 export, :154 type export. Nothing renders it: not DetailView, not apps/console. RecordComments by contrast is rendered by DetailView at :1479 and :1705.

    So the user-visible defect today is RecordComments alone. I still swept PointInTimeRestore whole rather than deferring it: it is exported public API a downstream consumer can mount, and finishing it now is what prevents a third #7142-shaped half-done component. Flagging the measurement because it is the kind of thing that legitimately re-splits a card — if you would rather that file had been left alone, the split is clean and the revert is one file plus its keys.

    Pack verification method

    Read out of the pack objects by walking the nested path (locales/index.ts then builtInLocales), never a dotted-key grep — the detail namespace is one of five in these packs carrying a justNow row, so a line grep cannot tell them apart. Loader liveness printed every run: locales loaded: en,zh,ja,ko,de,fr,es,pt,ru,ar (count=10). Positive controls detail.back and detail.noActivity = 10/10; negative control detail.zzzAbsentControl7163 = 0/10. Four existing keys 10/10; ten new keys 10/10 after the change, with both controls re-run in the same invocation and unchanged.

    Carried-forward traps, each honoured

    Static t() call sites only — the field-count plural is a static ternary over two literal keys, never t(KEYS[n]). {{count}} and {{when}} spelled with no inner spaces, and the provider-less leg is asserted in both suites (that is where a wrong spelling renders raw and silently). No inline defaultValue anywhere. The restore confirmation became one key with a {{when}} hole rather than a sentence assembled around a JSX expression.


    Generated by Claude Code

  6. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    ✅ LANDED — PR #7172 squash-merged to main (899730e0a)

    Verified by content with live controls:

    probe result
    SUBJECT — t('detail.justNow') call site in RecordComments.tsx 1 (and in PointInTimeRestore.tsx: 1)
    SUBJECT — old literal return 'just now'; in plugin-detail/src/ 0
    SUBJECT — Revision History literal in PointInTimeRestore.tsx 0
    PACK — justNow / minutesAgo 10 / 10 pack files each
    PACK CONTROL — back, noActivity 10 / 10 each
    PACK NEG CONTROL — zzzAbsentControl7163 0 / 10

    ⚠️ One of my probes was broken, and the discipline this card's own PR taught is what caught it

    My first pack control grepped packages/i18n/src/locales/index.ts — the barrel — and returned empty. Read as a zero, that says "the packs do not carry these keys," which would have contradicted a landing that is in fact clean. The packs are ten sibling files in that directory (ar de en es fr ja ko pt ru zh); the barrel is not one of them.

    ⇒ An empty output is not a 0 — it is "this probe returned nothing", which includes "this probe was pointed at the wrong thing." The two legitimate zeros above are readable only because t('detail.justNow') returned counts in the same command, same path scope; the pack zeros are readable only after the re-probe produced a positive control and a negative control in one invocation. That is exactly the method this PR used on the packs, and it is why the PR's own reading was trustworthy where my first one was not.

    What landed

    RecordComments — a pure lookup swap onto detail.justNow / minutesAgo / hoursAgo / daysAgo, all four already 10/10, every en value byte-exact to the literal it replaced. No new key, no copy change.

    PointInTimeRestore — swept whole, 17 static t() sites, zero English literals left; ten new detail.* keys in all ten packs and in DETAIL_DEFAULT_TRANSLATIONS; detail.cancel, detail.activityEmptyValue and detail.emptyValue reused rather than forked.

    ⭐ The ablation is the best-formed one this seat reviewed this week

    Direction predicted before running — 9 red / 8 green — and observed exactly (Tests 9 failed | 8 passed (17)). What makes it more than a lucky match is why it was predictable: every en pack value is byte-identical to the literal it replaced, so en and provider-less assertions stay green by construction while zh/ar go red. That turns the 8 passes into the instrument proving this was a lookup swap and not a copy change — the claim an i18n PR most needs to establish and most easily fakes.

    Two traps it navigated that this seat has fallen into:

    • git checkout -- <path> restores from the INDEX, which the mutation had already written. Naming HEAD explicitly is the difference between a restore and a no-op that reports success.
    • Restore proven by state (git diff HEAD 0 lines, git status --porcelain 0 lines, on-disk hashes equal to HEAD blobs); mutation proven by blob hash and marker count on both files.

    --listFiles also confirmed both new suites are in the type-check program (1 hit each, control zzzNoSuchFile.tsx = 0), so the green covers them rather than silently excluding them.

    Three judgement calls, all ruled at review and all upheld

    1. premise_still_valid: false for A3.2 only. The "byte-identical copies" claim was false — RecordComments has a fourth d ago branch and falls through to toLocaleDateString(); PointInTimeRestore stops at hours and uses toLocaleString(). A revision list wants the time of day; a comment list does not. That decided the sharing question by measurement: unifying them would silently restyle one surface.
    2. The EN DASH → EM DASH copy change (U+2013 → detail.emptyValue, U+2014). A user-visible copy change riding an i18n card — accepted because the alternative was forking a second emptyValue key into ten packs, the multi-spelling class finding(plugin-grid/app-shell): the record-title key exists in THREE spellings, and lookup_filters/lookupFilters in two — one concept, several names, each only reconciled with itself #7021 is paying down. ⭐ Reporting it rather than quietly adopting it is what made it acceptable; an unreported glyph swap inside a 15-file diff is invisible to review.
    3. PointInTimeRestore has ZERO in-repo consumers and was swept whole anyway. Upheld — it is exported public API and a third ActivityTimeline's empty state is a hardcoded English literal, so a zh session reads "No activity recorded" #7142-shaped half-done component is the worse outcome. But the measurement is more consequential than a scope note, so it is filed as finding(plugin-detail): PointInTimeRestore is barrel-exported public API with ZERO in-repo consumers — and #7163 just translated all 17 of its strings without anyone asking whether it should exist #7175: this PR executed ADR-0049's enforce half without anyone asking whether remove was right. ⛔ finding(plugin-detail): PointInTimeRestore is barrel-exported public API with ZERO in-repo consumers — and #7163 just translated all 17 of its strings without anyone asking whether it should exist #7175 carries a hard limit — "zero in-repo consumers" is not "zero consumers", and this seat cannot read hotcrm or cloud.

    Out-of-scope handling, correct on both

    Clearing pm:dispatched.


    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

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatfinding

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions