Skip to content

Unified search: shared render-type resolution rule#5152

Merged
habdelra merged 4 commits into
mainfrom
cs-11430-unified-search-render-type-resolution-rule-default-filteron
Jun 8, 2026
Merged

Unified search: shared render-type resolution rule#5152
habdelra merged 4 commits into
mainfrom
cs-11430-unified-search-render-type-resolution-rule-default-filteron

Conversation

@habdelra

@habdelra habdelra commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Phase 0 of Unify prerendered-card-search and card-search — the single, pure render-type resolution rule (Decision #1) that the server and host both call so they pick the same type. Additive; no behavior change on existing paths.

Stacked on #5150 (CS-11429) — it uses that PR's SearchRenderType. Review the top commit here; the base will be retargeted to main once #5150 merges.

The rule — resolveRenderType({ renderType, filterOn, types }) (runtime-common search-utils)

  • an explicit renderType CodeRef → use it;
  • the "native" escape valve → the result's own most-derived type (types[0]);
  • omitted → the query's filter.on (the common ancestor searched on); when the query has no filter.on, fall back to the most-derived type.

It returns the CodeRef used both to select the HTML column and echo meta.renderType (server, CS-11433) and to drive the live CardRenderer @codeRef (host, CS-11434). The default is filter.on — an intended change from today's silent types[0] that only takes effect on the unified path; the legacy compat shim (CS-11433) preserves today's types[0]-unless-renderType behavior until callers migrate. The rule governs only the prefer-HTML path; dataOnly results are the actual types, so the request carries no render spec to resolve.

Tests

Shared contract tests run via the realm-server runner (5 cases): explicit CodeRef wins; "native"types[0]; omitted → filter.on; the missing-filter.on fallback; and that a dataOnly request carries no render (so the rule is inapplicable).

🤖 Generated with Claude Code

A single pure render-type resolver (Decision #1) shared by the server (HTML
column selection + meta.renderType echo) and the host (live CardRenderer
@CodeRef) so both pick the same type:

- an explicit renderType CodeRef wins;
- the "native" escape valve resolves to the result's own most-derived type
  (types[0]);
- omitted resolves to the query's filter.on (the common ancestor searched on),
  falling back to the most-derived type when the query has no filter.on.

Governs only the prefer-HTML path; dataOnly results are the actual types
(nothing renders), so the request carries no render spec to resolve.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@habdelra

habdelra commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

If you are only testing on the realm server, then don't put teh test in common. that pattern is only used for tests that need to execute in both host and realm -server

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

This PR introduces a shared, pure “render-type resolution” rule in @cardstack/runtime-common so both the realm-server and host can resolve renderType consistently for unified search (prefer-HTML) results, and adds shared contract tests to lock the behavior down.

Changes:

  • Added resolveRenderType({ renderType, filterOn, types }) to packages/runtime-common/search-utils.ts.
  • Added runtime-common shared contract tests covering the resolution matrix and the dataOnly “no render spec” invariant.
  • Wired the new shared tests into the realm-server test runner.

Reviewed changes

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

File Description
packages/runtime-common/tests/render-type-resolution-test.ts Adds shared contract tests for resolveRenderType and verifies dataOnly requests omit render.
packages/runtime-common/search-utils.ts Adds the shared resolveRenderType rule used to pick a rendering CodeRef consistently across server/host.
packages/realm-server/tests/render-type-resolution-test.ts Executes the shared contract tests via runSharedTest in realm-server.
packages/realm-server/tests/index.ts Registers the new realm-server test module so it runs in CI.

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

habdelra and others added 2 commits June 8, 2026 15:42
…edicates' into cs-11430-unified-search-render-type-resolution-rule-default-filteron
resolveRenderType is pure runtime-common logic tested only on the realm server,
so the tests live directly in packages/realm-server/tests as a self-contained
qunit module importing from @cardstack/runtime-common, rather than via the
runtime-common/tests + consumer SharedTests indirection (which is reserved for
suites that run in both host and realm-server).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@habdelra

habdelra commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] Done — both suites moved out of runtime-common/tests into self-contained realm-server qunit modules (importing from @cardstack/runtime-common), shared files + consumers removed: render-type resolution here (2f27551) and the unified-search contracts on #5150 (b5f1def).

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files  ±0      1 suites  ±0   1h 51m 23s ⏱️ -53s
2 947 tests ±0  2 932 ✅ ±0  15 💤 ±0  0 ❌ ±0 
2 966 runs  ±0  2 951 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit 1528dda. ± Comparison against earlier commit 2f27551.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   10m 28s ⏱️ -16s
1 566 tests ±0  1 566 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 657 runs  ±0  1 657 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 1528dda. ± Comparison against earlier commit 2f27551.

Comment thread packages/runtime-common/search-utils.ts Outdated
};
}

// The single render-type resolution rule (Decision #1), shared by the server

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.

Is this “Decision #1” reference something we want in the comments forever?

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 🤖] Agreed — no, it should not live in the comment forever. Removed in 1528dda; the rule itself stays documented, just without the decision-number reference (comments describe the enduring contract, not the rollout). Swept the same kind of reference (a CS-11433 mention) out of the stacked #5157 too.

Code comments describe the enduring contract, not the rollout/decision history.
@habdelra
habdelra changed the base branch from cs-11429-unified-search-resource-types-relationships-predicates to main June 8, 2026 21:29
@habdelra
habdelra merged commit d7b79b5 into main Jun 8, 2026
65 of 66 checks passed
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.

3 participants