Skip to content

Fix catalog/base/skills/openrouter search returning zero results#5388

Draft
richardhjtan wants to merge 2 commits into
mainfrom
fix/catalog-search-item-id-mismatch
Draft

Fix catalog/base/skills/openrouter search returning zero results#5388
richardhjtan wants to merge 2 commits into
mainfrom
fix/catalog-search-item-id-mismatch

Conversation

@richardhjtan

Copy link
Copy Markdown
Contributor

Summary

  • A search-entry response's item resource inherited its id straight from the stored pristine_doc, which realms with a registered alias prefix (catalog, base, skills, openrouter) store in unresolved alias form (e.g. @cardstack/catalog/...) for storage portability across environments.
  • The item's own search-entry relationship always points at the row's resolved absolute URL instead, so the two ids never matched. A consumer doing a strict JSON:API (type, id) lookup on included (e.g. the host's search hydration) silently found nothing, so any search requesting fields[search-entry]=item against these realms returned zero hydrated results despite the backend having correct, fully-indexed data.
  • Confirmed this is not just a local edge case — spot-checked staging's boxel_index and found the same id mismatch present across a majority of instance rows in the catalog/base/skills/openrouter realms there.
  • Fix: set the item's id/links.self to the same resolved URL its relationship already uses, instead of trusting whatever form happened to be in storage.

Test plan

  • Added a regression test that stores a card under a simulated alias-form pristine_doc.id and asserts the served item's id still matches its relationship
  • tsc --noEmit clean on the changed file
  • Full qunit run of search-entries-engine-test.ts (blocked locally by sandbox disk space; needs a clean re-run)

🤖 Generated with Claude Code

The `item` resource in a search-entry response inherited its id
directly from the stored pristine_doc, which realms with a registered
alias prefix (catalog, base, skills, openrouter) store in unresolved
alias form (e.g. @cardstack/catalog/...) for storage portability. The
item's search-entry relationship, however, always points at the row's
resolved absolute URL. The mismatched ids meant a strict JSON:API
(type, id) lookup on the consuming side never found the included item,
so searches against these realms silently returned zero results.

Set the item's id/links.self to the same resolved URL the relationship
already uses.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab5c18f742

ℹ️ 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".

Comment on lines +420 to +421
id: cardUrl as RealmResourceIdentifier,
links: { self: cardUrl },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize loadLinks root identities consistently

When this path is used through Realm.searchEntries, loadLinks is enabled by default (packages/runtime-common/realm.ts:5393-5395). For registered-prefix realms, this new resolved id means the roots are placed in loadLinks's visited/omit sets as resolved URLs, while fetched linked resources and relationshipIdStr are still compared in unresolved alias form (visited.has(linkResource.id), omitSet.has(entry.relationshipIdStr)). If a full item links to itself or to another card that is already a search result, loadLinks no longer recognizes that root and can add the same logical card again under the alias id and rewrite the relationship to that alias, leaving duplicate/mismatched identities in the search-entry document. Normalize both the root ids and the loadLinks comparison keys to the same form when overriding the item id here.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Good catch, confirmed and fixed in 95f05d3. The item's id override left loadLinks's visited/omitSet comparing resolved-URL roots against alias-form ids elsewhere in the same pass. Fixed by indexing both spellings of each root via VirtualNetwork.equivalentURLForms so either form matches.

@richardhjtan richardhjtan marked this pull request as draft July 2, 2026 14:14
@richardhjtan richardhjtan requested a review from Copilot July 2, 2026 14:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a JSON:API identity mismatch in search-entry results for realms that store pristine_doc.id in registered-alias form (e.g. @cardstack/catalog/...). The projection engine now ensures the included item resource uses the same resolved URL (cardUrl) as the search-entry id and the item relationship, allowing consumers that hydrate via strict (type, id) matching on included to find the item.

Changes:

  • Set included item.id and item.links.self to the resolved cardUrl instead of the stored pristine_doc.id.
  • Add a regression test that simulates an alias-form stored pristine_doc.id and asserts the included item is still keyed by the resolved id.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/runtime-common/realm-index-query-engine.ts Forces included item resources to use cardUrl for id/links.self so JSON:API relationship hydration matches.
packages/realm-server/tests/search-entries-engine-test.ts Adds coverage for the alias-stored-pristine_doc.id case to prevent regressions in search-entry item hydration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Overriding the search-entry item's id to the resolved cardUrl (previous
commit) left loadLinks comparing that against alias-form ids elsewhere
in the same pass: a fetched linked resource's own id, and
relationshipIdStr, both stay in registered-alias form for storage
portability. A root that links to itself, or to another card already
present as a search result, was no longer recognized as visited/omitted
under its alias spelling, risking a duplicate included entry under a
mismatched id.

Index both the resolved and alias spellings of each omitted/visited
root via VirtualNetwork.equivalentURLForms so either form matches.
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files      1 suites   2h 16m 22s ⏱️
3 372 tests 3 357 ✅ 15 💤 0 ❌
3 391 runs  3 376 ✅ 15 💤 0 ❌

Results for commit 95f05d3.

Realm Server Test Results

    1 files      1 suites   9m 6s ⏱️
1 675 tests 1 675 ✅ 0 💤 0 ❌
1 754 runs  1 754 ✅ 0 💤 0 ❌

Results for commit 95f05d3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants