Conversation
A `query` operation is declared next to a card type's other operations and carried out by the search engine rather than by the operations endpoint: it answers the live entries resource a search runs as, and `.query()` answers the wire query behind it for a card that renders the rows itself. The declaration is translated to an entry-wire query template by the same function the realm uses when it captures the type's definition, so a saved search means one thing wherever it is read, and the markers it leaves standing are filled from the invocation — the caller's identity for `actor()`, the payload for `params()`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…reads A batch is anchored on a card and a query runs against a type, so nothing a batch registers can name a saved search; the extra rule that said the same thing could not change an answer. The card-facing module now says what a declared query is instead: index-fresh, invoked on the class, and answered with a resource to keep rather than a result to await. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f39b81266
ℹ️ 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".
habdelra
left a comment
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖]
Reviewed the delta against the branch this is stacked on, with the weight on three questions: whether the translation this consolidates can still mean two things on the two sides, what the returned resource's life actually is, and whether the new tests discriminate. I ran the client-core suite standalone (82 passing) and drove the member directly through a stub session to exercise the refusals, the scope rules and the thunk. I did not run the host integration suite, and I did not review the stacked base.
No blocking issues. Two of the eight asks change behavior rather than prose — the ownerless search's lifetime and the default realm scope for a nested code ref — and I would settle those before merge.
Recommendations, detail in the linked threads:
- Bound an ownerless search's life, or say the bound is the tab — thread on
searchinpackages/host/app/services/operations.ts. The card path is the ownerless one, and it currently parents to the one object nothing destroys. - Resolve the default scope through
moduleFromrather than a top-level.modulecast — the bot's thread onqueryScope. I confirmed it: anancestorOfref (whatidentifyCardhands back for a card class reached as a superclass rather than as its own export) yieldsundefinedand the call is refused as unscoped, whilemoduleFromresolves the same ref to its module. Replied there with the repro. - Decide what the thunk is for — thread on
queryMember. The eager resolve front-loads the first refusal rather than keeping one out of a render, and "varies with what the payload holds" needs a reactive payload to be true. - Make the thunk test discriminate — thread on
card-operations-client-test.ts. The assertion compares the thunk against itself. - Settle what a declared
realms: []means — thread onresolveQuery. Two comments in this change give opposite answers and the refusal message describes the declaration wrongly. refusingSinkrenders the path twice, one segment short — thread on it, with a suggestion.- Share the three predicates the consolidation left in two homes — thread on
query.ts. - Carry the host's two load-bearing sentences onto the card-facing row type — thread on
SearchEntry.
One decision no thread covers: a saved search invoked inside the prerender app. routes/render.ts marks that app for its whole life, and StoreService#resolvesQueryFieldsEagerly declines to resolve a card's query fields there on the stated ground that those stores "must render as a pure function of the document they were handed". The write half of the operations transport has its own gate for that app; a saved search has none. Invoked from a card's template during an index render it issues a live federated search, and actor() resolves to whatever that app authenticates as rather than to a viewer — so an actor-scoped saved search bakes one identity's rows into HTML every viewer is then served. @context.searchResultsComponent already searches during a prerender, so the search itself may be the settled answer; the actor is the new part. Worth deciding explicitly rather than by omission.
Adjacent, not asked of this change: RealmService#realmOf answers with the key a realm was registered under, which may be a scoped identifier rather than a URL, so queryScope's two branches can put different spellings of "a realm" into the same realms array. normalizeRealms handles both today (it builds RealmPaths from the string rather than parsing a URL), so nothing is broken — it is just a spelling difference that only holds because every downstream reader happens to avoid new URL.
| // A search with no owner is tied to this service, which lives as long as the | ||
| // session: right for a search whose results the session keeps, and why a | ||
| // caller with a shorter life — a component, a controller — names itself as | ||
| // the owner and has its search torn down with it. | ||
| search: OperationsSearch = { | ||
| actor: () => this.matrixService.userId ?? undefined, | ||
| realmFor: (identifier: string) => this.realm.realmOf(rri(identifier)), | ||
| entries: ( | ||
| getQuery: () => SearchEntryWireQuery, | ||
| opts?: { owner?: object }, | ||
| ): SearchEntries => getSearchEntriesResource(opts?.owner ?? this, getQuery), |
There was a problem hiding this comment.
[Claude Code 🤖]
An ownerless search never ends, and "lives as long as the session" is the part that has to hold. getSearchEntriesResource states its own bound — "per-render calls pile up live instances on the parent until the parent is destroyed" — and parenting to a service removes it. MatrixService#logout clears auth, calls session.notifySessionEnded(), resets matrix and realm state and transitions the router; nothing in that path destroys a service. So every search any card ever started is still parented here after a logout and a re-login, still subscribed to its realms, and still re-running whenever an index event touches a type its query is anchored on. The real bound is the tab.
That matters more here than it would at a host-owned call site, because the ownerless path is the card path: card-api never calls setOwner, so a CardDef instance is not an Ember-owned destroyable and cannot be passed as owner — a card author only has one to name from inside a component class. myReports = operations(Report).myReports() as a field on the card gets the unbounded search by construction, which is exactly the shape the member's own docs recommend.
Two ways out: hold what this service parents and drop it when the session resets (RealmService already registers with session for this, and this service already registers a destructor for the global bridge); or keep the default and say plainly, in both this comment and SearchInvokeOptions#owner, that an ownerless search lives for the tab — so an author can price a per-card call.
Regression, non-blocking, but I'd settle it here: the cost is per call and permanent.
There was a problem hiding this comment.
[Claude Code 🤖]
Taken as written, and the wording is what changed rather than the parenting.
An ownerless search is now described for what it is: parented to a service, which is destroyed with the application instance and not by a sign-out, so its life is the tab. The comment says that, says which shape of call takes it (a card holding the resource in a field, since a card instance is not application-owned and cannot be named as an owner), and says what to reach for instead when the search should end with a view — hand the query to the search component rather than holding the resource.
The other half of the concern is smaller after the thunk change in the same commit: a signed-out session withdraws the actor and empties the realm map, and the thunk now answers no query at all in that case, so a tab-lived search parks rather than going on re-running against a session that ended. What it keeps is its realm subscriptions.
Bounding the parenting itself — dropping what this service holds when the session resets — is a design step I'd rather take deliberately than fold in here, since it decides what a live resource sees when its realms go away.
Fixed in 9a224be749 and fd79561993.
| // The resource reads its query back through a thunk, so the search this call | ||
| // stands for varies with what the payload holds rather than with how many | ||
| // times the call is made. That makes the call itself the thing to make once — | ||
| // a field, a one-time assignment, never inside a getter or during render — | ||
| // exactly as the underlying resource documents: every call builds an | ||
| // independent search with its own realm subscriptions. | ||
| // | ||
| // What it answers with is index freshness. A search reads the index, which | ||
| // lags a write until that write is indexed, so a card just written is read | ||
| // directly (`read`, or the store) rather than queried for; the resource | ||
| // refreshes itself as index events arrive. | ||
| function queryMember( | ||
| subject: OperationsSubject, | ||
| env: OperationsEnvironment, | ||
| name: string, | ||
| info: CarriedOperationInfo, | ||
| ): QueryMember { | ||
| let wireQuery = ( | ||
| payload?: Record<string, unknown>, | ||
| opts?: SearchInvokeOptions, | ||
| ) => resolveQuery(subject, env, name, info, payload, opts); | ||
| let member = (( | ||
| payload?: Record<string, unknown>, | ||
| opts?: SearchInvokeOptions, | ||
| ) => { | ||
| let search = searchBridge(env, name); | ||
| // Resolved once here, where the caller is: a payload the declaration | ||
| // cannot resolve is a mistake at the call, and left to the thunk it would | ||
| // first be raised inside a render instead. | ||
| wireQuery(payload, opts); | ||
| return search.entries( | ||
| () => wireQuery(payload, opts), | ||
| opts?.owner === undefined ? undefined : { owner: opts.owner }, | ||
| ); |
There was a problem hiding this comment.
[Claude Code 🤖]
Two asks about the thunk.
The eager call front-loads the first refusal; it does not keep one out of a render. The thunk re-runs the whole of resolveQuery on every recompute of the resource's args, and modify runs "inside a tracked computation (a property access on the resource proxy, typically mid-render)" — the resource's own words. Two of the refusals are about the session rather than the payload, and I drove both through a stub session: with the actor withdrawn the thunk throws actor-required, and with realmFor answering undefined it throws names no realm to search. A recompute is reachable because search.actor() reads MatrixService#userId → this.client → the @tracked _client, which resetState() reassigns — and that same reset has already emptied RealmService's realm map. So: is the eager call meant to be the only guard? getSearchEntriesResource takes SearchEntryWireQuery | undefined and goes idle on undefined, so the thunk could answer undefined for the two session conditions and keep the throw at the call, which is what this comment says it wants.
"varies with what the payload holds" holds only for a reactive payload. The thunk closes over the payload object and a re-read does follow a mutation of it — I checked. But a plain Record<string, unknown> entangles nothing, and neither do the other two reads: RealmService keeps _realms as a deliberately untracked Map, and the actor is the only tracked read in the whole thunk. So for the payload shape both of this member's examples show — byStatus({ status: 'closed' }) — the args never invalidate, the thunk runs once, and the standing search is a fixed search. Worth saying what a payload has to be for the affordance to exist, since an author reading this will otherwise hold one search and change the payload.
Regression, non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖]
Both asks answered, and the first one changed behavior.
The thunk no longer throws for the two session conditions: it answers no query, which parks the resource. A signed-out session raises both of them at once, and a search that threw from inside a render would take the render with it rather than showing nothing — which is what a session that can no longer answer a query should show. A payload the declaration cannot resolve is still the caller's mistake, so that one is still raised at the call; the eager resolve is what does it, and the comment no longer claims more for it than that.
On the second: the doc now says what the affordance needs. One call stands for a search that moves only if the payload's own values are tracked; a payload of literals is a fixed search, which is the common case and the reason the call is the thing to make once.
The suite asks for both halves now — a re-read follows a mutation of the payload object and follows the session's actor, and a third case drives the parking through a sign-out and back.
Fixed in 9a224be749 and 00645d9ac3.
| if (!query.realms?.length) { | ||
| // A search with no realms fans out across every realm the session can | ||
| // read, which is never what a saved search meant to say: a declaration | ||
| // that named none left its scope to the caller, and a caller that named | ||
| // none left it to the type's own realm. Refused here rather than sent, so | ||
| // a scope nobody chose is not answered with results from everywhere. | ||
| throw new Error( | ||
| `operation "${name}" on ${subject.displayName} names no realm to search: the declaration names none, the call named none, and the realm holding the type could not be resolved — omitting them would search every realm this session can read`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖]
A declaration that writes realms: [] is refused here by a message saying it named none, and lowerQueryOperation takes the opposite position one file away: "An authored realms is a deliberate scope and stands, empty included — realms: [] says 'these and no others', which is not the same as saying nothing." It duly leaves it alone, and this then rejects it as unscoped — confirmed against a declaration carrying realms: [], which throws with "the declaration names none".
Pick one. If the refusal is right, lowerQueryTemplate is where to say that a declared empty scope is not a scope, and the message here should name the case it actually hit rather than describe the declaration wrongly.
Regression, non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖]
Settled toward the refusal, which is the behavior that was already there; the two comments now say the same thing.
What reads the query cannot tell "these and no others" from "any of them" — no realms on the wire means every realm to the search engine — so an empty list is not a scope a caller can honor, and widening it silently is the one outcome a saved search never meant. The resolution still leaves a declared list alone, and its comment now says why that is not the same as honoring an empty one. The refusal names the case it actually hit: a declaration that wrote the empty list is told so, rather than being described as having named none.
Covered by a second case in the scope test.
Fixed in 9a224be749.
| function refusingSink(subject: OperationsSubject, name: string) { | ||
| return { | ||
| add(_code: string, path: string, message: string): never { | ||
| throw new Error( | ||
| `operation "${name}" on ${subject.displayName} declares a query that cannot be run: ${path} ${message}`, | ||
| ); | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖]
The path lands twice, and one segment short. Every sink.add call already opens its message with the path, so an unresolvable class reads:
... declares a query that cannot be run: query.on `query.on` names a class that no module exports, so there is no code ref to store for it
And query.on is not where it is: lowerQueryTemplate calls resolveTypes on declaration.filter with the default path = 'query', so a filter's type reports query.on and a sort entry's reports query[0].on rather than query.filter.on / query.sort[0].on. The labels predate this change, but on the realm they only ever land on a stored definition's issue list; here they are what a card author reads at the call, so it is worth passing 'query.filter' and 'query.sort' as the starting paths while this is in hand.
| function refusingSink(subject: OperationsSubject, name: string) { | |
| return { | |
| add(_code: string, path: string, message: string): never { | |
| throw new Error( | |
| `operation "${name}" on ${subject.displayName} declares a query that cannot be run: ${path} ${message}`, | |
| ); | |
| }, | |
| }; | |
| } | |
| function refusingSink(subject: OperationsSubject, name: string) { | |
| return { | |
| add(_code: string, _path: string, message: string): never { | |
| throw new Error( | |
| `operation "${name}" on ${subject.displayName} declares a query that cannot be run: ${message}`, | |
| ); | |
| }, | |
| }; | |
| } |
Doubling is this change; the labels are pre-existing and now user-facing. Non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖]
Both taken, suggestion included.
The sink drops the path — every message already opens with it — and lowerQueryTemplate now hands resolveTypes the starting paths query.filter and query.sort, so a filter's type reports query.filter.on and a sort entry query.sort[0].on. The labels are what a caller reads at the call now, which is what makes the segment worth paying for.
Fixed in 9a224be749.
| export interface SearchEntry { | ||
| id: string; | ||
| realmUrl: string; | ||
| html: SearchEntryRendering[]; | ||
| item?: CardResource<Saved> | FileMetaResource; | ||
| iconHtml?: string; | ||
| displayName?: string; | ||
| codeRef?: ResolvedCodeRef; | ||
| indexGeneration?: number; | ||
| htmlGeneration?: number; | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖]
This is the row type a card author reads, and it drops the two things the host's copy says are needed to read one safely: an empty html means the row matched but no rendering satisfies the query's htmlQuery yet (the re-run fills it in), and item is the raw wire serialization — sparse when it carries meta.sparseFields — never a store instance. Without those, an author branches on html.length as "this row has no rendering" and treats entry.item.attributes as a whole card.
The host's entries implementation annotates its return as SearchEntries, so a member the resource renames or drops does redden — the shapes cannot silently diverge in that direction. What that check cannot see is the host gaining a member this copy never learns, or this prose going stale against it. Carrying those two sentences here is the cheap half.
Non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖]
Carried over — both sentences are on the members they are about: an empty html means no rendering satisfies the query's html terms yet and the re-run fills it in, and item is the wire serialization, sparse when it says so, never a card the store is holding.
Fixed in 9a224be749.
| function isMarker(value: unknown): value is Record<string, unknown> { | ||
| return isPlainObject(value) && typeof value.$ref === 'string'; | ||
| } | ||
|
|
||
| function isBxl(value: unknown): value is { $bxl: string } { | ||
| return isPlainObject(value) && typeof value.$bxl === 'string'; | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖]
The translation has one home now; three predicates around it still have two. isMarker and isBxl here are byte-identical to the pair still in lowering.ts, and the class→code-ref step each side feeds into lowerQueryTemplate is also a hand copy: identify in lowering.ts and identifyDef in packages/base/operations.ts differ only in whether identifyCard arrives on a context or is imported. A marker spelling that lands in one copy and not the other, or a thunk-unwrapping rule that does, puts the two sides back to meaning different things — which is the failure this move exists to close.
lowering.ts can import the two predicates from here; the dependency already runs that way. The identify pair needs a home both sides can reach — client.ts is what the barrel re-exports and what base/operations.ts imports, so exporting the helper here and re-exporting it there would give it one.
isMarker/isBxl duplication arrives with this change; identify/identifyDef predates it and this is what makes it load-bearing. Non-blocking follow-up.
There was a problem hiding this comment.
[Claude Code 🤖]
Both halves done, including the one you called a follow-up.
isMarker and isBxl are exported from query.ts and lowering.ts imports them, so the marker spellings have one definition. The identify pair is now one function too: codeRefForDef in code-ref.ts, which is where identifying a class already lives and which both sides can reach — lowering.ts passes the identifier its context carries, and base/operations.ts passes the imported one. That was the copy that mattered, since a thunk-unwrapping rule landing on one side only would put the two readings of a written class back to differing.
Fixed in 9a224be749.
| assert.deepEqual( | ||
| searched[0].getQuery(), | ||
| searched[0].query, | ||
| 'the query is read back through the thunk, which is how the search follows what the invocation resolves to', | ||
| ); |
There was a problem hiding this comment.
[Claude Code 🤖]
This assertion cannot fail for the reason it states. The harness builds Searched with query: getQuery(), so this compares the thunk against itself — it holds for any deterministic thunk, including () => aFrozenConstant. Nothing anywhere in the suite pins that re-reading it follows anything, which leaves the design's central affordance unguarded.
The stub already owns both inputs, so the discriminator is cheap: read getQuery(), flip the harness's actor (or mutate the payload object the call was given), read it again, and assert the second differs. I ran both changes against the member directly and the re-read does follow them, so the test would pass — it just is not asked.
Test gap, non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖]
You're right that it compared the thunk against itself, and the replacement asks for the affordance rather than restating the stub.
The harness's session is now a record the test can write to. The case mutates the payload object the call was handed and asserts the re-read follows it, then moves the session's actor and asserts the re-read follows that. A separate case drives the parking: actor withdrawn, realms withdrawn, then both restored, with the same search resuming.
Shown to discriminate — pinning the thunk to resolve once turns three of those assertions red.
Fixed in 9a224be749 and 00645d9ac3.
…ding each The default scope reads the type's module through `moduleFrom`, so a class named by a nested ref — what identifying a class reached as a superclass answers with — covers its own realm instead of being refused as unscoped. A standing search parks when the session stops being able to resolve its query: signing out withdraws the actor and the realms, and a thunk that threw there would take a render with it. A payload the declaration cannot resolve is still raised at the call. A declared empty realm list is refused for what it is rather than described as naming none, the class-to-code-ref reading and the two marker predicates each have one home, and the refusal a caller reads no longer says the path twice. The suite asks the thunk to follow both things it resolves against, covers the nested ref and the parked search, and each new case was shown to fail without the change it names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o viewer A render authenticates as itself so it can render any card, and what it produces is served to everyone — so a search that compares against the caller resolves nothing there, rather than putting one identity's rows into shared HTML. The live render that follows fills them in. The same answer covers the other two ways a session cannot say who the caller is: nobody signed in, and a client that is not up yet. A saved search that does not read the caller is unaffected in all three, and what a declaration or a payload gets wrong is still raised at the call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
[Claude Code 🤖] Answering the decision ask in the review body — the one no thread covered — with code rather than a note, because it is a hazard this change introduces rather than one it inherits. A render has no viewer, so a saved search that compares against one now answers none. Before this change a card had no supported way to get the viewer's identity into a search; So the host's search bridge reports no actor inside the prerender app, and "the session cannot say who the caller is" is now one rule with one answer: no query, which the resource reads as an idle search and the search component renders as no rows. The live render that follows resolves it against the viewer and fills the rows in. The same rule covers the two ordinary cases — nobody signed in, and a matrix client that is not up yet, which throws rather than answering. A saved search that does not read the caller is unaffected in all three. One consequence worth seeing in the diff: Not taken here: bounding what the operations service parents. An ownerless search is now described as living for the tab, and the parking above means a signed-out one stops resolving rather than going on re-running, but dropping the searches a service holds when a session resets decides what a live resource sees when its realms disappear, and that is a design step I would rather take deliberately than fold into this. On the adjacent note about realm spellings: agreed it is not broken and not this change's to fix. |
…rites Both sides added to the same three places. The operations service now holds the matrix service a saved search reads its caller from and the card service a write reports authorship through; the client core carries the query member beside the batch's local ids; and the client suite's harness describes both a session that runs searches and one that hands back resources. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t resolver Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Background
A card type can declare operations — named actions like
addCommentorescalatethat the realm carries out against a card without loading any cardJavaScript. An author writes them as statics next to the card's fields, and a
caller invokes them through
operations(card).addComment({ body }), whichbuilds a request, sends it to the realm's operations endpoint, and answers with
what the realm wrote.
One of the base behaviors an operation can build on is
query, and it isthe odd one out: a
querydeclaration is not an action against one card, it isa saved search over a collection —
Searching is something this platform already does, through a single engine: a
query goes to the realm server's
/_federated-searchendpoint, comes back as astream of entries (each row is either prerendered HTML or a live
serialization, and a consumer renders it without caring which), and on the host
it is held by
getSearchEntriesResource— a reactive resource that subscribesto every realm it covers and re-runs itself when those realms report new index
data. Operations do not re-own any of that. So a declared query is declared
like every other operation and carried out nowhere near them.
Until now the operations bucket simply left queries out:
operations(Report)carried the creates and nothing else, and a declared query had no member at all.
This PR gives it one.
What a caller gets
operations(Report).openReportsis now a member with two ways in.Called, it answers the live entries resource — the same
{ entries, isLoading, meta, errors }every other search on the host answerswith:
.query()answers the wire query that same invocation resolves to — or noquery at all when the session cannot say who the caller is, which the search
component reads as an idle search (see When there is no viewer) — for a card
that would rather render the rows itself through the search component the host
provides on
@context:A payload fills the markers the declaration names —
params('status')takes itsvalue from
operations(Report).byStatus({ status: 'closed' })— andactor()resolves to the signed-in user, so a saved search can be written "mine" and mean
it.
How the declaration becomes a search
Two translations happen, and both already existed; this PR makes the first one
shared rather than re-implemented.
A declaration writes its query against JavaScript:
on: () => Reportnames aclass,
actor()andparams('x')are markers standing where a value is onlyknown at invocation. Lowering turns that into an entry-wire query template —
classes become code refs, the card-rooted filter becomes the entry-addressed one
(
status→item.status) — with the markers left standing. The realm alreadydid this when it captured the type's definition for its cache. Resolving
then fills the markers in from the invocation and yields a concrete wire query.
The caller has the class in hand, so it lowers the declaration itself rather
than fetching the type's definition over the network — which is what lets
.query()be an ordinary synchronous call. The important part is that it runsthe same function the realm runs: the translation moved out of
lowering.ts(which reaches the BXL program canonicalizer, and so can never be imported by a
card module) into
card-operations/query.ts(which reaches nothing but thequery grammar), and
lowering.tsnow calls it. A saved search therefore meansone thing wherever it is read, instead of two translations that have to be kept
in step by hand.
Which realms a search covers
A search that names no realms fans out across every realm the session can read,
which is never what a saved search meant to say. So the scope is resolved in
order, and a query that reaches the end with nothing is refused rather than sent:
realms— a deliberate scope, which a calldoes not widen.
operations(Report).openReports(undefined, { realms: [...] }).service. This is the case a card is in when it invokes a saved search on a
type it imports: the type and the cards live in the same realm, so "the
type's realm" is the card's realm.
A query is scoped to a type, not to one card, so it is invoked on the class —
operations(Report), neveroperations(oneReport)— and for the same reason noentry of a batch can name one: a batch is anchored on a card and commits writes
under one realm's lock.
When there is no viewer
A saved search can compare against the caller —
eq: { postedBy: actor() }isthe point of a "mine" search — and that raises a question about where the search
runs. The prerender app authenticates as itself so it can render any card, and
the HTML it produces is served to everyone; a search that resolved the actor
there would put one identity's rows into a rendering every viewer reads. The
store already declines to resolve a card's query fields in that app for the
neighbouring reason: a render must be a pure function of the document it was
handed.
So the rule is one sentence: when the session cannot say who the caller is, a
search that compares against them answers no query at all. The resource reads
that as an idle search and the search component renders no rows; the live render
that follows resolves against the viewer and fills them in. Three situations
land on it — nobody signed in, a sign-out that withdrew an actor the search had,
and a render. A saved search that does not read the caller is unaffected in all
three, and anything a declaration or a payload gets wrong is still raised at the
call rather than silently idling.
This is the one place the delivered shape departs from what was specified:
.query()answersSearchEntryWireQuery | undefinedrather than always aquery. The search component already accepts an absent query, so a card hands the
result over unchanged — and the alternative was for a card's own getter to throw
during a prerender.
The session a query runs in
The host service that carries operations to a realm gains a second, optional
half of that bridge, for the things a saved search needs and a batch does not:
who the caller is (what
actor()resolves to), which realm holds a module, andthe live resource itself. It is optional because it is the half an environment
can genuinely lack — in Node there is nothing to build a reactive resource with,
so a query there says so rather than half-working. Nothing about how a batch
reaches a realm changed.
Two refusals are worth calling out, both raised at the call rather than inside a
render: a payload that leaves a declared marker unfilled, and a query that
compares against the caller when nobody is signed in. The second is not a bad
payload — the search would run as written, compare stored values against nobody,
match nothing, and read as a saved search that genuinely found none.
Freshness, and calling it once
A query reads the search index, which lags a write until that write has been
indexed. A card you just wrote is read directly (
read, or the store); a queryis for collections, and its resource refreshes itself as the realms it covers
index. Both the card-facing module and the client core say so where an author
will read them.
The other thing they say: make the call once. Every call builds an independent
search with its own realm subscriptions, exactly as the underlying resource
documents, so the returned resource belongs in a field or a one-time assignment
rather than in a getter. The query itself is read back through a thunk, so a
standing search follows what its payload resolves to rather than needing to be
rebuilt.
Where the changes live
packages/runtime-common/card-operations/query.ts— now holds both halves: thedeclaration → template translation (moved here from
lowering.ts) and thetemplate → wire query resolution.
packages/runtime-common/card-operations/lowering.ts— calls the sharedtranslation instead of carrying its own copy.
packages/runtime-common/card-operations/client.ts— the query member, itsscope resolution, and the search half of the transport interface.
packages/host/app/services/operations.ts— the host's implementation of thathalf: the session's user, the realm service, and
getSearchEntriesResource.packages/base/operations.ts— the card-facing typing: a declared query iscallable with its declared payload and answers the resource, and
.query()answers the wire query.
getSearchEntriesResource, the store's search, and the federated searchendpoint are untouched — this only produces wire queries and hands them over.
Testing
packages/realm-server/tests/card-operations-client-test.tsdrives the clientcore with a recording session: what
.query()resolves to (markers filled, thefilter entry-addressed), that a call answers with what the session built and
reads the query back through the thunk, the three-step realm scope, and each
refusal. Every new assertion was run against a deliberately broken
implementation first, so each one is known to go red for its own reason.
packages/host/tests/integration/operations-query-test.gtsruns the real thingagainst an in-browser realm: a declared query answers the cards its filter
matches,
actor()resolves to the signed-in user and excludes another user'sreport, a payload selects a different set, the resolved wire query renders rows
through
@context.searchResultsComponent, a search covers the type's realm untilthe call names others, and a report written after the search started joins it
once the realm indexes it.
The existing operations invocation suite and the operation lowering unit suite
were re-run unchanged.
What the review changed, and what has been run since. Eight findings came
back (threads below); the two behaviour-level ones — the nested code ref and the
thunk — plus the no-viewer rule above are in the last three commits. The
client-core suite has been run against every one of them, each new case shown to
fail without the change it names, and lint and type-checks are clean in all four
packages. The two host integration files have not been re-run since those
commits: the machine's single test stack has been held by other work, and rather
than sit on the change I am letting CI be the first run of them. If CI disagrees
with anything here, that is the reason.
Stacked on #6198 (the
operations()entry point), which it branches from; retarget tomainbefore merging.🤖 Generated with Claude Code