fix(plugin-audit): resolve reference titles in activityMilestones summary tokens (#7290) - #7333
Conversation
…mary tokens (#7290) `matchMilestone` (ADR-0052 §5b.2) interpolated `{token}` from the after-row with no title map, so a `lookup` / `master_detail` / `user` token fell through to the raw id: `Deal won by oBK25…`. The milestone branch takes precedence over the tracked-change branch, so #7230's fix never reached milestone-declaring objects. `matchMilestone` is split into detection (`matchMilestone`, still sync), planning (`planMilestoneTokenReads`) and rendering (`renderMilestoneSummary`), so the read plan is built from the tokens of the template that actually fired. `resolveTrackedLookupTitles` becomes `resolveLookupTitles(api, plan)` — one read/mask/title implementation shared by both summary branches. Measured with a copy-returning counting driver: 0 added reads on every create, every delete, every non-firing update of a milestone-declaring object and every fired milestone naming no reference token; 1 read per distinct target object on the transition itself. The rejected before-match placement measured 3 reads on a write that fires nothing. 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:
|
…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
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 31366863728 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
|
Merge-queue ejection triaged — this is a deliberate single re-queue, not a blind one. Walking the Step 1 — is the failing test in a package this PR changed? No. This PR changes three files, all Step 2 — has the same test appeared in another PR's queue-failure comment? Not in the 60 most recently updated PRs — this is the first recorded sighting, so I cannot claim "flaky 实锤" on a repeat. Filed #7369 anyway, because step 2 asks for the issue regardless and the analysis holds without a second sighting: the case races a chokidar delivery against two fixed wall-clock budgets (a Step 3 — semantic conflict with the batch? Nothing in the diff can interact with a filesystem watcher. Action: re-queued once. If it ejects again on the same test, that is the second sighting, and I will stop re-queueing and escalate on #7369 rather than burn further full-queue rounds — the checklist is explicit that repeated re-queues rebuild everything behind me. Nothing about this PR changed: still the two reviewed commits, still 23 success + 2 skipped-by-design on head Generated by Claude Code |
…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 #7290
matchMilestone(ADR-0052 §5b.2) interpolated a milestone summary's{token}s from the after-row by callingdisplayFieldValuewith no title map, so alookup/master_detail/usertoken fell through toString(value)— the raw 32-char id. A milestone declared as{ field: 'stage', value: 'closed_won', summary: 'Deal won by {owner}' }shippedDeal won by oBK25…to the record timeline.The milestone branch takes precedence over the tracked-change branch, so on every object that declares
activityMilestonesthis was the string users actually saw, and #7230 / PR #7291's fix never reached them. Closing that is what this card is.Anchors re-verified at the branch point (
7fa2aae45)The card recorded its anchors at
a768f81a1(pre-#7230) and warned the file had moved three times in a day. It had:a768f81a1)7fa2aae45displayFieldValue:308:344(gained #7230'stitlesForparam)matchMilestone:355:472{token}interpolation:368-375:485-490matchMilestonecall site:872:1071The premise itself was re-verified rather than taken on trust: at the branch point the interpolation still read
return field ? displayFieldValue(field, v) : String(v)— two arguments, no title map — andmatchMilestonehad exactly one caller, already inside anasyncfunction. Premise valid, unchanged by #7291.Placement — re-derived, and measured both ways
The card recommended resolving after a milestone fires, keyed on the matched template's tokens, at the cost of making
matchMilestoneasync. That direction is right and is what shipped. The async part turned out to be avoidable, so it was avoided:matchMilestoneis split rather than made async — detection (matchMilestone, still sync, no longer takesfields), planning (planMilestoneTokenReads), rendering (renderMilestoneSummary). The call site awaits one read between them. This gives the recommended read profile with zero async blast radius, and it mirrors theplanTrackedLookupReads+ resolver shape #7291 established one commit earlier in the same file rather than inventing a second one beside it.resolveTrackedLookupTitlesbecameresolveLookupTitles(api, plan)— it takes the plan instead of building it. Both branches now end in one read/mask/title implementation, which is what keeps #6977's masking contract from being re-typed for the milestone path.Read counts, measured with the copy-returning counting driver
The rejected placement, measured rather than argued. Resolving before knowing whether a milestone fired was implemented as mutation M3 and run: on a non-firing update of a row holding references it measured 3 reads (
sys_user1,crm_account1,crm_region1;crm_vault0, its title being masked) where the shipped shape pays 0. That is the shape #6656 / PR #6977's Option A+ ruling was obtained to remove from this write path, so it was not adopted.Why the milestone plan does not filter on
trackHistory. #7230's gate is "a field that is BOTH tracked AND a reference actually moved". A milestone token is none of those: it is the author's own token, read from the whole after-row. The reported declaration itself ('Deal won by {owner}') names a field that need not be tracked at all, so reusing that gate would leave the exact reported defect shipping — mutation M4 below measures it.#7291 and #6977 are re-asserted, not assumed
audit-lookup-summary.test.tsandaudit-bound-previous.test.tsare untouched and green, and #6977's single-id count is re-asserted inside the new file on the firing path (still exactly onefindOneon the audited object).ledgerView, the secret mask and the virtual-field drop are unchanged; the milestone read plan is built from the same masked after-view the interpolation renders from.crm_vaultin the new fixture declaresnameField: 'api_key'on a realsecretfield (with a crypto provider wired, so the column is genuinely encrypted rather than conveniently empty). The read is skipped, not masked afterwards.What did not change
The author's template wording is theirs: only what a token resolves to changes. Punctuation, emoji, and the empty-token rule are byte-identical — an empty value still renders as the empty string, not
displayFieldValue's∅, because a milestone sentence is prose and not a diff row. Milestone templates are still not translated. Select option-label rendering is untouched — localizing those is #7289, deliberately held behind this card in the same file and left alone here.Tests
New file
packages/plugins/plugin-audit/src/audit-milestone-summary.test.ts— 28 cases on a realObjectQLengine with the copy-returning counting driver (a driver handing back live store references lets the read path rewrite the store under a measurement).The label/locale fixture is a real
createMemoryI18nbundle, and that earned its keep immediately: the first draft wrote{object}in the bundle and measured已更新{object}"{label}"— the shipped bundles interpolate{{param}}, which is deliberately not the milestone template's single-brace{token}. A stub translator would have hidden that.Family gates, all green locally:
check:nul-bytes,check:durability-log-level,check:engine-double-contract,check:error-code-casing,check:route-envelope,check:wildcard-fallthrough,check:empty-changeset,check:adr-0087-registration,check:changeset-gate-self-tests.Reverse verification — direction predicted in writing before the first mutation
Predictions were written to a file and the tree committed before any mutation ran.
renderMilestoneSummary(the pre-#7290 interpolation, plan and read kept)id: { $in: [...] }matchMilestonetrackHistorygate on the milestone planM2 is the one I got wrong, and the reason is worth recording. I predicted the mutation would only coarsen batching. It did not: the plan map's key is the object name
resolveLookupTitlesreads from and the keyrenderMilestoneSummarylooks titles up by. Keying it by field therefore made the resolver query objects namedowner/account_id, the best-effortcatchswallowed the failures, and no title resolved at all — 14 red, mostly rendering. The mutation was wrong, not the code. M2' was written to isolate the property M2 was supposed to test, and it does. My sub-prediction inside M2 — that the repeated-token case pins token dedup rather than object grouping, so it should stay green — held under M2'.The trap that PR #7291 documented is live here and was designed around, not discovered. Every zero-read guard runs against
deal_2, a row that already holds all six reference fields; one case (a reference token whose value is EMPTY) necessarily runs against the reference-freedeal_1, and it is the one case that stayed green under M3. That is green-for-the-wrong-reason, predicted as such in writing beforehand and flagged in the file rather than counted as coverage. Likewise the raw-id fallback case was written from the start as a mixed assertion (Won by usr_1 for APAC— unresolvable side stays raw, resolvable side must still become a title), because #7291 measured that a single-token fallback assertion survives deletion of the render limb; under M1 it went red, as intended.Deliberately not done
sys_activity.summaryare never localized — the same defect class as #7230, one line over indisplayFieldValue#7289's select option-label localization — same file, same function's neighbourhood, held atpm:queuebehind this card. Not touched; a case pins the current rendering so the seam is visible when that card lands.multiple: truelookups stored as an unparsed JSON string still fall through to the raw value, exactly as on plugin-audit: renderTrackedChangeSummary never receives the translator, and displayFieldValue prints raw lookup ids — sys_activity.summary shipsRating Owner: ∅ → oBK25…onto every feed surface (re-routed from objectui#4055) #7230's branch — in-memory arrays are unpacked and resolved from the same batch.Generated by Claude Code