fix(plugin-audit): localize the tracked-change activity label and render lookup titles instead of raw ids (#7230) - #7291
Conversation
…itles (#7230) `sys_activity.summary`'s tracked-change branch (ADR-0052 §5b) shipped strings like `Rating Owner: ∅ → oBK25…` onto every feed surface at once. Two causes, both fixed here: - `renderTrackedChangeSummary` was the one summary branch never handed the locale-bound `translate` its three siblings resolve through (ADR-0053 / #3039). The field label now resolves through it on the bundles' own key shape, falling back to the authored label and then the machine key. - `displayFieldValue` resolved select option labels only, so a lookup / master_detail / user value printed its raw id. It now renders the referenced record's title via ADR-0079's `resolveDisplayField`, falling back to the raw id when unresolvable. The resolution is batched: 0 added reads unless a tracked reference field actually moved, then exactly one `id: { $in: [...] }` per distinct target object — so #6656 / PR #6977's retirement of the redundant pre-image read from this write path is not handed back. Read counts are pinned with the same counting driver that measured that retirement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
|
PM review — PASS. Marked ready and enqueued. Identity-lane PM seat (#6022), session CI on head The question this dispatch was built around, answered with numbersThe dispatch warned that #6656 / PR #6977 (ruling A+) had retired the redundant pre-image read from this exact file the day before — 2→1 per single-id write, 3→0 per predicate write — and that a naive per-row lookup resolution would hand those reads straight back. The answer:
And the limit is stated rather than buried: Mutation D is the best thing in this reportPredicted RED (resolve from the written row instead of from the diff — the #6977 regression shape). First run came back 160/160 GREEN. Rather than banking the green, the author diagnosed why: the hot-path cases ran against a deal holding no references, where a naive resolve-from-the-written-row implementation also reads nothing. The guard was green for the wrong reason and would not have caught the regression it exists to catch. Fixture hardened with a second deal carrying That is the "passes for the empty reason" failure mode this lane has been chasing all session (#6964's vacuous D7 denials, #7151's decorative-pin refusal), caught by an author in their own test, against their own prediction. It is also why the read-count claim above can be trusted: the guard behind it demonstrably bites. Two prediction misses reported rather than smoothed: under mutation B the raw-id fallback case went red because it had been rewritten into a mixed assertion ( Other judgement calls this seat agrees with
Follow-ups filed, unassigned, for triage#7289 — select option labels in the same function are still unlocalized ( Generated by Claude Code |
…ne non-biting zero-read case (#7290) The `seed()` doc block claimed "Every zero-read case below therefore runs against `deal_2`". That is false for `a reference token whose value is EMPTY pays ZERO reads`, which must run against the reference-free `deal_1` because an empty reference is what it measures. On a row with nothing to read every candidate placement pays zero, so that case cannot constrain read placement — measured, it was the one case that stayed green under the mutation moving the read before `matchMilestone`. This is the doc block that teaches the next reader about the trap PR #7291 measured, so as written it taught something untrue about its own file and a reader would have counted a non-biting case as a guard. The exception is now stated in `seed()` and flagged at the call site itself. Comments only — no assertion, fixture, source or changeset change; counts are unchanged and were not re-derived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
…e activity summary (#7289) `displayFieldValue` rendered a select/picklist value by returning the matching option's authored `label` from `engine.getSchema(name)` — locale-independent metadata — while the shipped bundles carry those labels under `objects.<object>.fields.<field>.options.<value>`. After #7230 localized the field label, a zh-CN workspace read `阶段: Proposal → Closed Won`. The tracked-change branch now resolves the option label through the same locale-bound translator its field label already uses, authored label as fallback. The fired-milestone branch is deliberately NOT localized and the opt-out is by construction: it passes no option resolver, so its author-written sentence and the author's own option label stay in one language. Zero added reads on every write shape — a bundle lookup, not I/O — so #6656 / PR #6977's retirement, preserved by #7291 and #7333, still stands, and `displayFieldValue` stays synchronous. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
Fixes #7230
sys_activity.summaryis composed at write time and shipped verbatim to every consumer at once — the record discussion feed, console home activity, the header inbox, the Setupsys_activitylist, and mobile/REST/SDUI. Its tracked-change branch (ADR-0052 §5b, the"[label]: [old] → [new]"template) was producingRating Owner: ∅ → oBK25…at the bottom of an otherwise fully-localized zh-CN page. One string, two independent causes — both fixed in one file, per the card's recommendation A.Anchors re-verified at the branch point (
a768f81a1)All three still hold exactly where the card recorded them, so nothing had to be re-scoped:
a768f81a1displayFieldValue:308:308renderTrackedChangeSummary:330:330translate:882:882translate:848/:852/:856:847(translate) /:848(displayLabelFor) /:853/:857objectui was not touched (verified pass-through per the card), and no file under
content/docs/releases/was touched.Label half — the branch that was never handed the translator
renderTrackedChangeSummarynow takes the object name and the same locale-boundtranslateits three siblings (messages.activityCreated/messages.activityDeleted/messages.activityUpdated, plusdisplayLabelForfor the object label) already resolve through, and reads the field label on the bundles' own key shape:That is the shape
zh-CN.objects.generated.tsand every other shipped bundle actually carries, so the fix is against real data rather than against a convention. A miss still returnsundefined, so both existing fallbacks answer exactly as before — ADR-0053 / #3039 write-time localization, applied to the one branch that missed it.Value half — a reference renders its record's title
displayFieldValuegains a reference branch forlookup/master_detail/user.useris included because the spec defines it as "a lookup specialized to thesys_usersystem object — stored IDENTICALLY (FK string column)", and it is the exact field class the symptom was reported on.treeis excluded: it carries noreference, so there is no target object to read.The title field comes from ADR-0079's
resolveDisplayField(nameField→ the deprecateddisplayNameFieldalias → deterministic derivation), imported rather than re-derived. A local "try name, then title, then subject" heuristic here would be a second de-facto contract that disagrees with the record picker, the search companion and the approval inbox the day an author setsnameField— the same argument the file's existingSECRET_MASK/collectMaskedReadFieldsimport makes for itself.The change is restore-invariant: an id with no resolved title — target removed out of band, unregistered object, failing read, or a title field that resolves to
id— renders exactly as it did before. It can replace an id with a title, never a title with an id. The∅ →notation is unchanged, andmatchMilestone's{token}interpolation passes no resolver, so its behaviour is byte-identical.Read cost — measured, because #6977 landed in this file one day ago
#6656 / PR #6977 retired
captureBefore's redundant pre-image read from this write path under maintainer ruling Option A+ (2 → 1 reads per single-id write, 3 → 0 per predicate write). A naive per-row lookup resolution would have handed that straight back. Measured with the same copy-returning counting driver, and pinned as cases rather than asserted in prose:id: { $in: [...] }, not two point reads#6977's own counts are re-asserted alongside: the single-id update still pays exactly onefindOneon the audited object, andaudit-bound-previous.test.tsis untouched and green.Three properties do the work: the read plan is built from the diff, not from the written row; it is keyed by target object, not by field or by value; and only
['id', titleField]is selected, so resolving a title cannot drag a wide row through the write path.What batching cannot remove, stated honestly:
writeAuditis dispatched per row, so a predicate update over N rows that moves a tracked reference on each pays N reads (one per row per distinct target). That is inherent to per-row summary composition, not to this batching — and unlike the pre-image read #6977 removed, it is gated on "this object declares a tracked reference field and it moved", so the default write pays nothing.#6977's masking is not regressed
resolveDisplayFieldhonours an explicitnameFieldpointer even when it points at a title-ineligible type, so an object could in principle designate a credential field as its title. Rather than trust downstream masking, the read is skipped when the resolved title field is incollectMaskedReadFields(def)— the same contract predicateledgerViewmasks with, so "no credential value reaches a user-facing activity summary" holds on this path by the same definition, not a second one.ledgerView, the secret mask and the virtual-field drop are otherwise untouched; the summary still renders from the masked views, and the read plan is built from those same masked views.Tests
New file
packages/plugins/plugin-audit/src/audit-lookup-summary.test.ts— 17 cases on a realObjectQLengine with the copy-returning counting driver lifted fromaudit-bound-previous.test.ts(its copy-returning rule included deliberately: a driver handing out live store references lets the read path rewrite the store under a measurement).The two halves are pinned separately, because a test asserting only the finished string could be satisfied by fixing either one. The label half is pinned against a real locale —
createMemoryI18nloaded with realobjects.[object].fields.[field].labeldata — not a stub translator that would pass whatever key shape the code used.One case earns a note: the raw-id fallback could not be written the obvious way. The engine's
assertReferencesResolverefuses to write a dangling reference (ValidationError: Account: no crm_account record has id "acc_missing"), so the realistic unresolvable case is a reference that was valid when written and whose target has since gone away — reproduced by removing the row out of band.Family gates run locally, all green:
check:nul-bytes,check:durability-log-level(this file is in its vocabulary),check:engine-double-contract,check:error-code-casing,check:route-envelope,check:wildcard-fallthrough,check:empty-changeset,check:adr-0087-registration.check:i18n/check:i18n-coveragereport PREREQUISITE NOT MET locally (they run the built CLI); this diff adds no declared label and edits no translation bundle, so neither can drift from it — CI runs them regardless.Reverse verification — direction predicted in writing before running
Predictions were written to a file before the first mutation ran. Four mutations, each reverting one property of the change.
translate(...)limb from the labelexpected 'Owner: ∅ → 张伟' to be '负责人: ∅ → 张伟'displayFieldValueexpected 2 to be 1Mutation B, the two things I got wrong. The raw-id fallback case went RED, not green: after the engine refused a dangling reference the case was rewritten to a mixed assertion (
Account: acc_2 → Acme Corp), where the unresolvable side stays raw and the resolvable side must still become a title — so it pins both directions and cannot survive the mutation. And three read-count cases went red too. The underlying claim still held exactly as stated — no count assertion failed, every one of those failures was the trailinglastSummaryassertion — so read counts really are blind to a missing render branch; my prediction was wrong at the case level only because those cases carry a second, rendering assertion.Mutation D is the one that changed the change. It came back 160/160 green where I predicted red. Diagnosed: the hot-path cases ran against a deal holding no references, where a naive resolve-from-the-written-row implementation also reads nothing — there is nothing on the row to read. The guard was green for the wrong reason and would not have caught the exact regression it exists to catch. The fixture was hardened (a second deal seeded with
account_idandowneralready set), and mutation D re-run against it:Both properties now go red on the regression, and the fixture comment records why the seeded references are load-bearing rather than incidental.
Deliberately not done
sys_activity.summaryis. Historical rows keep their write-time composition; only new writes improve.displayFieldValuereadsfield.options[].labelfrom the schema, which is authored-language, while the bundles carryobjects.[object].fields.[field].options.[value]. That is the same defect class as the label half in a neighbouring line, but it is a third change the card does not scope — filed separately as plugin-audit: select option labels insys_activity.summaryare never localized — the same defect class as #7230, one line over indisplayFieldValue#7289.{token}interpolation still prints raw lookup ids (matchMilestone). Resolving it would require a read before knowing whether a milestone fires, i.e. on writes that produce no milestone at all — the opposite of this change's hot-path shape. Filed as plugin-audit:activityMilestonessummary templates still interpolate raw lookup ids —{owner_id}rendersoBK25…after #7230 #7290.multiple: truelookups stored as an unparsed JSON string fall through to the raw value. In-memory arrays are unpacked and resolved from the same batch; parsing a JSON column here would be this file inventing a storage contract it does not own.Generated by Claude Code