Answer a card write from the written card alone

[habdelra]

A card+json POST or PATCH read its result back out of the index with
loadLinks: true, skipQueryBackedExpansion: false — assembling the transitive
closure of the written card's links and running a query for each of its
query-backed fields. Both readbacks now ask for the card and nothing around it.

Nothing consumes what that assembled

The host takes a write response for the id the realm assigned and for realm
metadata. persistAndUpdate deletes data.attributes and data.relationships
before merging, and _updateFromSerialized reaches included[] only by walking
resource.relationships — so with those gone, nothing references the
side-loaded resources at all.

The case that looks like it needs them is a write that side-loads a card by
lid: the realm names it, and the caller has to learn which id it got. That
mapping does not travel on the response. tryFindingCardItem in the host's card
store correlates the last path segment of a remote URL with a local id, and
names the two places identity is reconciled — handleInvalidations and
api.setId / updateFromSerialized. handleInvalidations assigns
instance.id under that same last-segment check, which works because the realm
names a created file <CardDirectory>/<lid>.json. The realm invalidation event
carries it; the write response never did.

Corroborating: writes that answer from the serialized echo — prerender and
skip-index-wait callers — have always returned no included at all, lid
creates included, and nothing has noticed.

Outside the host, the only reader of the member anywhere is the CLI's search
command, which reads it off a search entry document.

The validator, and one behaviour this removes

A body carrying less of the card's link graph is a different representation of
the same card at the same indexed_at, so it takes its own ETag variant —
buildCardJsonEtag already did exactly this for the links-only read shape. The
boolean becomes a CardJsonShape of full / links-only / write-echo;
links-only renders byte-identical to what it rendered before, so no existing
validator rotates.

This is not housekeeping. card-endpoints-test.ts contracted that a PATCH's
ETag short-circuits a later GET of the same URL — "the contract that lets the
caller cache the post-patch body without an extra round-trip GET". Once the
write body stops carrying included[], honouring that would 304 a caller
into holding the narrower body as the card's read representation. So that
shortcut is gone: a write's validator no longer satisfies a GET, and the
tests now assert the opposite and say why. A caller that wants the read shape
issues a normal conditional GET and revalidates against the ETag that GET
returned.

Where a write's time goes

A slow write could only be compared against the duration of the indexing job it
waited on, and when those two disagreed there was nothing to attribute the
difference to. realm:write-timing now emits one line per card write:

PATCH <url> corr=<id> handler=<ms>ms lock=<ms> drain=<ms> stage=<ms> write=<ms> readback=<ms>

lock, drain, stage and write are stamped inside commitBatch, which is
where they happen and where they are not observable from outside; readback is
stamped by the handler. Unlike search timing this is not gated on the caller
sending a correlation id — writes are a small fraction of a realm's requests,
and the slow ones worth explaining are the ones nobody instrumented in advance.
A correlation id is stamped when the caller does send one, so the line joins to
that request's realm:requests entry.

Emitted from a finally, so a write that ends in an error is attributed too.

Tests

a write side-loads none of the written card's links and a write does not resolve the written card's query-backed fields each read the same card back in
the same test, so what the write omits is asserted against what a read of that
card actually carries rather than against nothing.

Verified

card-endpoints-test.ts: 102 pass / 0 fail locally.

The lid tests are unchanged. They never asserted a write response's
included[] — each of those five deepEquals is over a GET of a created
child, which is also where the promotion is observable — so they pass as they
stand, and they are what shows a create that side-loads new cards still
round-trips the ids it was given.

The host's realm-test.gts write-response expectations drop their included
arrays. Those run in CI rather than here.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-16T22:48:51.416000Z	4776ab7	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4776ab7042

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Update card-copy tests for the narrowed POST echo

When either card-copy flow creates a card with an existing linked pet, its onSave callback receives this POST response, but the unchanged assertions in packages/host/tests/integration/components/card-copy-test.gts at lines 1039–1045 and 1173–1179 still require one included resource and dereference it. Calling cardDocument without loadLinks makes json.included undefined, so both host tests fail; update those expectations as part of changing the write-response contract.

AGENTS.md reference: AGENTS.md:L172-L175

Useful? React with 👍 / 👎.

[habdelra]

[Claude Code 🤖] Confirmed, and there was a third site: interact-submode-creation-and-permissions-test.gts asserts doc.included!.length on the consumer's first save in the cross-realm link test. All three read the response through the save subscriber, which receives the body whole — the delete data.attributes / delete data.relationships happen on a clone used only for the merge.

Fixed in 68026d5. The two card-copy sites now assert the write side-loads nothing, with the relationship carrying the target's identity. The cross-realm one asserts what its own comment describes — that friends.1 names nothing while the new card is unsaved — which pairs with the existing save-2 assertion that it becomes populated.

The readback comment was also wrong as written and has been restated to cover both surfaces.

[habdelra]

[Claude Code 🤖] Reviewed against the stacked base, not main. The lens: who actually reads a write response's included[] and relationships (host store, card-operations executors, boxel-cli, the VS Code extension, ai-bot, and the host/realm-server suites), what else drops out of a cardDocument() call with no options, whether the links-only validator rendering really is unchanged, whether the new stage decomposition is sequential and complete, and whether the new tests fail when the readback is put back. Percy, performance, and anything the base PR changed were out of scope.

One blocking finding: the "nothing consumes a write response's included[]" sweep stopped at _updateFromSerialized and missed the save subscriber, which three host tests use — they will fail. Everything else is a comment or an instrumentation gap. The wire change itself holds up: outside the host nothing reads included off a write (every reader — boxel-cli/src/commands/search.ts, vscode-boxel-tools/src/skills.ts, card-operations/executors.ts — is on a search document or a request body), lid-assigned identity really does arrive via the invalidation event rather than the response, and 'links-only' renders the same bytes it rendered before, so no existing validator rotates.

Recommendations, in order:

1. Update the three host write-response expectations and widen the claim they falsify — see the comment on the create-path readback in realm.ts.
2. Drop the stale "only a render's own read narrows" sentence in #patchCardInstance.
3. Decide what CardJsonShape says about the prerender body, which shares the full validator — see the comment on the buildCardJsonEtag header.
4. Stamp stage from a finally in commitBatch so an errored write's line isn't missing the stage that failed.
5. Gate write-timing emission on a write having happened, and decide whether a stage covers the response serialization — see the comment on #emitWriteTiming.
6. Make the realm:write-timing header and the new test's comment timeless.
7. Add a create-side shape test; both new ones are PATCH.

On removing the PATCH→GET 304 shortcut: nothing in the tree depends on it. The only If-None-Match a host client sends for an instance is the composite entry validator in fetchCardEntry (card+html), browsers don't cache PATCH responses, and the assertion that contracted it was the sole consumer. The change is safe; the tests now pinning the opposite are the right replacement.

CI was still queued when this was written — the host shards are where recommendation 1 shows up, so that's the run to watch rather than a rerun candidate.

Adjacent, not asked of this change: on the no-op PATCH path that falls through to commit(true), readEntry() runs twice and RequestTimings.add sums both legs, so readback= on such a line is two index reads reported as one. Worth a sentence in the channel's own docs whenever someone next touches it.

[habdelra]

[Claude Code 🤖] _updateFromSerialized is not the only host reader of a write response. StoreService.persistAndUpdate hands onSaveSubscriber the whole saveCardDocument result — the delete data.attributes / delete data.relationships happen on a cloneDeep used only for the merge, so the subscriber sees the POST/PATCH body intact, included[] and all. Three tests read it there and are unchanged on this branch, so they fail:

• packages/host/tests/integration/components/card-copy-test.gts — can copy a card that has a relative link to card in source realm and can copy a card that has a link to card in destination realm each assert json.included?.length === 1 and then dereference included[0] inside this.onSave. (Same pair the automated reviewer flagged — one ask, not two.)
• packages/host/tests/acceptance/interact-submode-creation-and-permissions-test.gts — new linked card is created in a different realm than its consuming reference asserts doc.included!.length === 1 and doc.included![0].id on the consumer's first save.

Both of those go through the index-readback branch today (they see included at all, which the echo branch never produces), so the assertions are unconditionally reachable and unconditionally broken by this change.

Ask: update the three expectations, and restate this paragraph so it covers the save-subscriber surface rather than only the deserializer — a reader who trusts it as written will conclude the response body is unobservable to anything but updateFromSerialized.

Regression, blocking.

[habdelra]

[Claude Code 🤖] Right on both counts, and the comment was the more serious half — it would have sent the next reader to updateFromSerialized and told them the body was unobservable.

Fixed in 5e68d69: the paragraph now names both surfaces, says the subscriber gets the body whole and exists for tests, and adds the reason the response could not be the reconciliation path — the index answers in ids, never in the lid a caller would match on.

All three test sites updated. I found the interact-submode one by brace-matching every this.onSave(...) body rather than grepping near a request call, which is what made me miss them the first time.

[habdelra]

[Claude Code 🤖] "Only a render's own read narrows what it resolves" is about an argument readEntry no longer takes. Every no-op patch narrows now, prerender or not — the sentence points a reader at the opposite conclusion, that a plain no-op PATCH still answers with the resolved link graph.

Ask: drop the last sentence, or fold it into the readback comment above so both halves say the same thing.

Regression (prose), non-blocking but belongs in this change.

[habdelra]

[Claude Code 🤖] Agreed — that sentence described the argument readEntry no longer takes.

Fixed in 5e68d69. It now says the no-op answer takes the same narrow shape every other write does, which is the thing a reader needs to know at that branch.

[habdelra]

[Claude Code 🤖] "Each takes its own validator" is not true of the enumeration it heads, and naming the type CardJsonShape makes the omission read as exhaustive.

#cardJsonLinkShape returns { skipQueryBackedExpansion: true, resolveLinksOnly: false } for a prerender GET, so a prerender body — relationships present, query-backed fields unexpanded — is a fourth shape at one indexed_at, and it reaches buildCardJsonEtag with 'full', i.e. the bare CARD_JSON_ETAG_VARIANT a live full read gets. What actually keeps those two apart is CardDocumentCache.getOrPopulate, which folds skipQueryBackedExpansion into its md5 key; the validator does not, and the card+json response declares only Vary: Accept.

Ask: either say here that the prerender shape is deliberately validator-equal to full and separated at the cache key instead, or give it a fourth member and pass it from #cardJsonLinkShape.

Pre-existing, but this comment newly asserts the rule the next reader will build a short-circuit on. Non-blocking.

[habdelra]

[Claude Code 🤖] Correct — the prerender GET is a fourth shape at one indexed_at and it reaches buildCardJsonEtag as full, so "each takes its own validator" claimed more than the enumeration delivers.

Took the first option in 5e68d69 rather than adding a member: the comment now says these three take different validators because a client holding one must not be 304'd to another, then states that the prerender body shares full's validator deliberately — it is never served to a client that caches, and the response cache separates it by folding skipQueryBackedExpansion into its own key rather than into the ETag.

Say so if you would rather have the fourth member; a validator that describes every shape is the more conservative position and I would not argue against it.

[habdelra]

[Claude Code 🤖] stage is stamped only on the success path. A batch that throws out of stageEntry or any of the five assertions above emits a line carrying lock and drain and nothing at all for the work that failed — which is exactly the write the handler's finally exists to attribute.

Ask: stamp it from a finally around the staging block, so an errored line reads like a successful one:

let stageStart = Date.now();
try {
  // …indexLids through assertDestinationsFree…
} finally {
  timings?.add('stage', Date.now() - stageStart);
}

Or, if partial attribution on the error path is intended, say so where the emission comment promises that an errored write is attributed too.

Regression in the new instrumentation, non-blocking.

[habdelra]

[Claude Code 🤖] Agreed — that contradicted the finally the handler emits from.

Fixed in 5e68d69, with staged and baseHashes hoisted above the try since the commit below the block reads them. An errored batch now reports stage alongside lock and drain.

[habdelra]

[Claude Code 🤖] Two asks about what this line covers.

It fires for requests that never reached a write. The try wraps the whole handler, so a PATCH to a path holding non-JSON bytes (415), to a _-prefixed path (405), or with a body that isn't a card resource (400) all emit — with handler= and an empty stage fragment, since toLogFragment() returns '' (which also leaves the line with a trailing space). Emission is ungated by design, so these are lines an operator filters past on the way to a slow write. Ask: skip when no stage was stamped, or start the timed region below the validation.

No stage covers the response leg. getRealmInfo() and the JSON.stringify of the assembled doc in #patchedCardResponse sit between readback and the end of handler, so they land in an unattributed residual — while the search channel this mirrors does stamp stringify. Given that the rest of this change is about how big that body is, it's the stage most worth having. Ask: add it, or state in the channel comment that the stages deliberately don't sum to handler.

Follow-up on both, non-blocking.

[habdelra]

[Claude Code 🤖] Both taken in 5e68d69.

Emission is now skipped when no stage was stamped, so a 415/405/400 that never reached the commit emits nothing — every line on the channel describes a request that tried to write. That removes the trailing space too, since it only appeared when the fragment was empty.

Added a stringify stage in both response builders rather than documenting the gap. It meant threading the collector into #patchedCardResponse, which is a touch on the stacked base's shared builder — but you are right that it is the stage most worth having here, since the body size is what this change decides. getRealmInfo() still sits outside it; the residual is now the cache-priming read alone.

[habdelra]

[Claude Code 🤖] This paragraph states the before-state rather than the contract: "has had a stage breakdown for a while", "a write has had none", "could only ever be compared", "were not separately observable". A reader a year out has no anchor for when that was, and the channel's own definition is what they came for. Same shape in card-endpoints-test.ts above a write reports where its time went ("A slow write could previously only be compared against…").

Ask: restate both timelessly — what the channel emits, which stages, and why each one can be the whole of a slow write — and let the commit message carry the rest.

Non-blocking.

[habdelra]

[Claude Code 🤖] Fair, and it is a rule I hold myself to elsewhere — the channel comment is the one place a reader lands with no other context.

Rewritten in 5e68d69 as a stage table: what each stage covers, and the property that makes the set worth having (any one of them can be the whole of a slow write, and none is distinguishable from outside). The test comment above a write reports where its time went is restated the same way.

[habdelra]

[Claude Code 🤖] The control here is real — hassan↔jade is a cycle, so the read genuinely side-loads something the write would have assembled, and reverting the readback turns assert.notOk(write.body.included) red.

What's missing is the create side: #createCard has its own cardDocument call and its own justification comment, and both new shape tests exercise only PATCH. The write whose included[] the host suite was reading is a POST — a copy into another realm, which links to a card that already exists.

Ask: add a create-side case in the POST module — POST a card carrying a linksTo at an existing card, assert the 201 body has no included[], and use a GET of the assigned URL as the control the same way this test does.

Non-blocking.

[habdelra]

[Claude Code 🤖] Added in 5e68d69 — a create side-loads none of the new card's links, in the POST module, POSTing a card that links at the fixture's existing hassan and using a GET of the assigned URL as the control the same way the patch test does.

Writing it turned up a difference between the two paths worth recording. On a create the echo carries the stored links.self but no resolved data member: relationships.{field}.data for a static link is in the pristine row only when the indexer wrote it there, and a card the request just named has not been through that. The patch case looks different only because its link was already stored resolved. The assertion is on the stored link for that reason (68026d5), and the closure assertion — no included[] on the write, side-loaded on the read — is the part that carries the test.

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

  1 files  ±    0    1 suites  ±0   10m 43s ⏱️ - 2h 23m 13s
277 tests  - 4 579  277 ✅  - 4 560  0 💤  - 14  0 ❌ ±0 
277 runs   - 4 594  277 ✅  - 4 570  0 💤  - 14  0 ❌  - 5 

Results for commit 5750a75. ± Comparison against earlier commit 759a57b.

Realm Server Test Results

    1 files  ±0    235 suites  ±0   1h 27m 0s ⏱️ -28s
3 399 tests ±0  3 399 ✅ ±0  0 💤 ±0  0 ❌ ±0 
3 448 runs  ±0  3 448 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 5750a75. ± Comparison against earlier commit 759a57b.

[jurgenwerk]

[Claude Code 🤖] Second pass, taken after the earlier round's fixes: whether each of those seven landed as its reply describes, what else in the tree reads a card+json write response (the host store and its save subscriber, the deserializer, boxel-cli, the VS Code extension, the card-operations executors, ai-bot, worker), whether the shape tests fail with the readback put back, and whether the timing stages cover what the channel comment promises. Percy and the stacked base's own changes were not looked at.

No blocking issues. The seven earlier threads each resolve as their replies say; what remains is two comments that still describe the code this change replaced.

On the open question in the CardJsonShape thread, whether the prerender body needs a fourth member: the comment-only position holds. A validator only has to separate bodies that one client can hold for one URL, and no client that caches a live card+json read ever receives a prerender body, so the two never meet in a cache that compares them.

Recommendations:

1. The header comment on #patchedCardResponse in packages/runtime-common/realm.ts says the 200 is "validated by the same ETag a GET of it would carry". The builder now passes 'write-echo', and the new tests assert the two validators differ. Restate it as: validated by the write-echo variant of the ETag a GET would carry, so the echo cannot satisfy a GET's conditional. Regression in prose, non-blocking.
2. The class comment on RequestTimings in packages/runtime-common/request-timings.ts says one instance is created "per instrumented _search request in handle-search" and is only ever threaded for instrumented searches. The card create and patch handlers now each construct one on every write, ungated. Widen that first paragraph to name the write path, or the next reader will read an ungated instantiation as a mistake. Non-blocking; it belongs here because this change adds the sites.

CI: Percy is red with 55 missing snapshots. Ignore it for this PR: the change touches no UI, and all 16 host shards ran to green on the head commit. The 277-test figure on the results comment is not a partial run; every shard job completed at full length.

Adjacent, out of scope: the diagnostic skills that name realm:search-timing as the server-side stage line (indexing-diagnostics Mode G, client-perf-diagnosis, search-shape-diagnosis) do not mention that a write now has a sibling channel. Whoever next touches those skills can add realm:write-timing beside it.