fix(data-objectstack): every view write path invalidates the override map (#4363) - #4374
Merged
Merged
Conversation
… map (#4363) `ObjectStackAdapter` caches two view-shaped reads — `getView` under `view:{object}:{viewId}` and `listViewOverrides` under `view-overrides:{object}`. Of the four write paths that touch view rows, only `updateViewConfig` invalidated the second key, so `createView` / `updateView` / `deleteView` left the batch override map stale for `MetadataCache`'s default 5-minute TTL. The gap does not self-heal: `loadViewOverrides` (app-shell `ObjectView`) treats a resolved map as authoritative and deliberately does not re-probe per view (#3774), so the per-view `getView` fallback that would mask a stale map is by design unreachable, and `listViews` — uncached — answers fresh beside it. All four paths now emit the same ordered pair, per method rather than per branch. `updateView`'s draft half joins its published half (deliberate over-invalidation: both readers enumerate published rows, and a missed invalidation costs the whole TTL while a spare one costs a refetch), and `createView` names the per-view key because `saveItem` is an upsert. Extends the pin suite from #4328 / PR #4366 to assert the full key set for all five call sites; its two sweep pins stay as untouched controls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4363
Built on post-#4366 main: branched from
origin/mainat275d7df13, which is downstream of #4366's squashd9d346307(dead-surface batch 3). This PR extends that sweep'sviewCacheInvalidation.pin.test.tsrather than re-creating it, and lands in the slot #4366 deliberately left open — itscreateViewcase asserted "noviews:key" rather than "invalidates nothing", precisely so this card could answer the override-map question without fighting a frozen pin.The defect
ObjectStackAdaptercaches two view-shaped reads. Four write paths touch view rows. Before this PR only one of them invalidated the batch map:getView)listViewOverrides)updateViewConfigcreateViewupdateView(draft half)updateView(published half)deleteViewMetadataCache's default TTL is 5 minutes (cache/MetadataCache.ts:90) and the adapter is long-lived, so the stale window is minutes of real use, not one navigation.It does not self-heal.
loadViewOverrides(app-shell/src/views/ObjectView.tsx:283) treats a RESOLVED map as authoritative and deliberately does not re-probe per view — that is #3774's fix and it is correct, since re-probing reinstates the 404 flurry the batch read exists to remove. So the per-viewgetViewfallback that would have masked a stale map is by design unreachable, and the stale map is served in full. Beside it,listViewsis uncached and answers fresh: the switcher can list a view whose override body came from a map written minutes earlier, and the fresher of the two reads is not the one supplying the override.The fix
All four paths now emit the same ordered pair — the per-view key, then the object's override map. The rule is uniform per method, not per branch.
objectNameis spelled exactly asupdateViewConfigalready spells it: the method's own first parameter, interpolated directly. No new derivation was introduced —viewItemObjectName()(#3774's converged reader) is for narrowing items read back, and every one of these write paths already receives the object name as an argument, so there was nothing to parse.Two choices in the diff that are decisions rather than mechanics, both recorded in the code and pinned:
updateView's draft half invalidates both keys, like its published half. This is deliberate over-invalidation: both readers enumerate PUBLISHED rows, so a draft write stales neither — exactly as was already true of the per-view line it now pairs with. The costs are not symmetric (a spare invalidation costs one refetch; a missed one costs the whole TTL), and "which half am I in?" is not a question a future edit to this method should have to re-answer.createViewnames the per-view key too, not just the map.saveItemis an upsert, so an explicitspec.namethat already exists overwrites a published row a priorgetViewmay hold cached. On the ordinary create-a-new-row path the extra call is aMap.deleteon an absent key — measured, not assumed:MetadataCache.getstores only on fulfilment (const data = await fetcher(); this.set(key, data)), so there is no negative caching for a generated name to collide with.Tests
Extends #4366's pin suite from 6 cases to 8, keeping its two sweep pins as untouched controls.
Consumer sweep direction stated explicitly: prefix filter
...@object-ui/data-objectstack= the 34 downstream consumers; the two that call these paths are@object-ui/app-shell(ObjectViewdrivesupdateViewfor rename/pin,deleteView,updateViewConfig, and readslistViewOverrides) and@object-ui/console. Both are in the runs above. Repo-root vitest with path filters per AGENTS.md §测试纪律 — neverpnpm --filter test, which would silently run someone else's package.Reverse verification
Fix committed first, then removed with
git checkoutand restored from the commit — nevergit stash. Both directions predicted before running.A — remove the fix, keep the new pin. Predicted: the four key-set cases red on a missing
view-overrides:account; all four controls green. Observed exactly that (4 failed | 4 passed):createView'sexpected []is the issue's table confirmed at the byte level: that path invalidated nothing whatsoever.B — keep the fix, restore #4366's ORIGINAL pin file. The mirror check, and the one that proves the sweep's pins are undisturbed. Predicted: the three "…invalidates the getView key only" cases red on an extra key, while
listViews,updateViewConfigand theno views: keycontrol stay green. Observed exactly that (3 failed | 3 passed):The
no views: keypin staying green in direction B is the load-bearing control:createViewnow emits two keys where it previously emitted none, and neither is aviews:key, so #3778's deletion is still pinned dead by an assertion this PR did not touch. Only that case's comment was updated, since "so it now invalidates nothing" had become false.Pin suite shape
New case
listViewOverrides reads back under the key the write paths invalidateasserts the reader/invalidator pairing rather than a bare string — theviews:mistake was a key with no reader, and one rename away it could recur. The remaining new cases assert full ordered key sets per path.Out of scope, filed as its own card
#4373 — the console's real create-view flow never calls
ObjectStackAdapter.createViewat all:handleViewCreatewrites through the ADR-0034 seam (createRuntimeMetadata→metadataClient.save), and Publish (RuntimeDraftBar→publishRuntimeMetadata) invalidates nothing, so publishing a draft view leaves the same map stale. This PR makes the adapter correct at its own door; that is a second door into the same rows, inpackages/app-shell, outside this card's scope. The dormant sibling (MetadataService.saveMetadataItemwould invalidateview:{name}, a key with no reader, if anything ever passed'view'— nothing does) is recorded there rather than filed separately.Related
views:{objectName}缓存键被 4 处 invalidate,却从没有任何读路径写入过 —— listViews 实为无缓存,那 4 行是惰性代码 #3778 removed the fiveviews:{object}no-ops, and the finding was filed as data-objectstack:view-overrides:{object}is only invalidated by updateViewConfig — createView / updateView / deleteView leave the batch map stale for the 5-minute TTL #4363 from that implementationmeta/{对象名}(把对象名当 metadata type),写却落在type='view'—— 两边键空间不相交,已保存的视图个性化永远读不回来 #3774 — madelistViewOverridesauthoritative, which is what removes the accidental self-healingupdateView's draft/published addressing, whose two halves this keeps symmetricGenerated by Claude Code