feat(a2ui): load server catalog from cdn#2772
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConsolidates per-component catalog artifacts into a generated full catalog; adds BASIC_CATALOG_ID and runtime loader with CDN fetch + local fallback; updates extractor CLI/API, tests, prompt API (async), agent/service, routes, CLI wiring, docs, and a consolidated server catalog JSON. ChangesDynamic A2UI Catalog Loading Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
.github/a2ui-catalog.instructions.md (1)
2-2: ⚡ Quick winExpand
applyToto cover the server catalog files.This new rule is meant to guide changes in
packages/genui/server/agent/**as well, but the current glob only matchespackages/genui/a2ui*/**, so the instruction will not be applied where the runtime loader and fallback snapshot actually live. As per coding guidelines, ".github/*.instructions.md: Update or create.github/*.instructions.mdfiles with natural language instructions, using applyTo frontmatter with glob syntax to specify applicable files or directories."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/a2ui-catalog.instructions.md at line 2, The applyTo glob in .github/a2ui-catalog.instructions.md currently only targets packages/genui/a2ui*/**, so update the frontmatter applyTo entry to also include the server catalog paths (e.g., add packages/genui/server/agent/** or packages/genui/server/**) so the instructions apply to the runtime loader and fallback snapshot files; ensure the final applyTo value is a comma- or newline-separated list of globs covering both packages/genui/a2ui*/** and the server catalog directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/genui/a2ui-catalog-extractor/src/index.ts`:
- Around line 332-339: The full catalog write is currently passing
CatalogFunction[] (which contains internal filePath) into
createA2UICatalog/writeA2UICatalog, leaking local build paths; before calling
createA2UICatalog (and likewise when producing the --typedoc-json output) map
each CatalogFunction to a plain FunctionDefinition by copying all public fields
and omitting filePath (e.g., transform functions => functions.map(f => ({ /* all
fields except filePath */ }))), then pass that normalized array into
createA2UICatalog and the typedoc-json builder so the top-level catalog and
typedoc export match the per-function files and do not include filePath.
In `@packages/genui/a2ui-catalog-extractor/test/extractor.test.ts`:
- Around line 214-229: The test file is using `@rstest/core` but the repo requires
Vitest; replace any imports from '`@rstest/core`' with Vitest equivalents and
ensure the file uses Vitest's globals (import { test, expect, beforeEach,
afterEach, vi } from 'vitest' as needed). Update usages of rstest-specific
decorators or helpers (e.g., rstest, before, fixture utilities) to Vitest
patterns and adapters, keeping the existing helper functions runCli,
createTempDir, catalogFixtureDir, fixtureDir, and readFullCatalogJson unchanged;
run the test file with the Vitest runner to confirm it resolves to the same
assertions. Ensure no `@rstest/core` remains in the file or imports.
In `@packages/genui/a2ui/package.json`:
- Line 144: The build script "build:catalog" is embedding a non-immutable
catalogId (the bare unpkg URL) which can change across releases; change the
script that generates the catalogId so it includes the package version (or
another immutable identifier) rather than pointing to the latest tag — update
the "build:catalog" npm script in package.json (the command string for
build:catalog) to construct an immutable URL that includes the package version
(for example by injecting the package.json version or using an env var) before
passing it to the generator so the generated catalog.json contains a stable
catalog-id.
In `@packages/genui/server/agent/a2ui-catalog-id.ts`:
- Around line 5-6: The exported constant BASIC_CATALOG_ID currently points at an
unversioned moving URL; change it to a release-pinned catalog URL or make it
build-time injectable so the server always fetches a catalog that matches its
packaged code. Update the export BASIC_CATALOG_ID in a2ui-catalog-id.ts to
either hardcode the exact release path (e.g. include the published package
version in the unpkg path) or read a build-time injected variable (e.g.
process.env.PUBLISHED_VERSION or a generated constant) that your build/publish
step sets; ensure any cache keys or logs that use BASIC_CATALOG_ID will now
reflect the stable, versioned URL.
In `@packages/genui/server/agent/a2ui-catalog.ts`:
- Around line 257-261: The manifest is currently trusted without validating
manifest.functions, so extend the guard to ensure manifest.functions is an array
of well-formed function-spec objects before passing it to
createA2UICatalogFromExtractedManifest; update the validation used by
isExtractedCatalogManifest (or add a small helper like
isExtractedCatalogManifestWithFunctions) to check that manifest.functions is
either absent or an array where each item has a string name, a string
returnType, and a parameters array whose items have string name and type (or the
minimal shape renderCatalogReference expects), and throw an error if validation
fails; apply the same stricter check at both call sites that currently call
createA2UICatalogFromExtractedManifest so malformed CDN payloads are rejected
early.
- Around line 243-247: The current loadBasicCatalog implementation only dedupes
concurrent fetches because pendingBasicCatalog is cleared in finally, which
causes every subsequent call to refetch; change it to maintain a persistent
cached result (e.g., cachedBasicCatalog) that is returned immediately when
present, set cachedBasicCatalog to the resolved A2UICatalog when
fetchBasicCatalog() succeeds, and only clear pendingBasicCatalog after the fetch
completes (or clear cached on explicit refresh/error as appropriate). Update
loadBasicCatalog, pendingBasicCatalog handling, and the fetchBasicCatalog
success/error flow so that pendingBasicCatalog is used for in-flight dedupe
while cachedBasicCatalog holds the last-good result (also apply the same change
to the similar block around lines 250-252).
In `@packages/genui/server/agent/a2ui-prompt.ts`:
- Around line 180-189: In buildA2UISystemPrompt, guard against opts being
null/undefined before destructuring: check if opts is falsy and throw the same
Error used for missing catalog (or route into that error path) so callers
passing undefined get the guided migration error; then safely destructure const
{ catalog } = opts and retain the existing catalog-null check and thrown
message; update BuildSystemPromptOptions usage accordingly.
In `@packages/genui/server/app/a2ui/action/stream/route.ts`:
- Around line 200-201: The catalog resolution using loadBasicCatalog() should be
wrapped in a try/catch before constructing the SSE/ReadableStream so failures
produce a controlled CORS-safe response; modify the code around the const
catalog = opts.catalog ?? await loadBasicCatalog() and optsWithCatalog = {
...opts, catalog } to try to resolve opts.catalog or await loadBasicCatalog()
inside a try block and on catch return a jsonWithCors(...) error response (or
alternatively move the await loadBasicCatalog() into the stream's error path),
ensuring downstream logging/repair paths are invoked and clients receive the
expected error contract.
In `@packages/genui/server/app/a2ui/stream/route.ts`:
- Around line 143-144: The code calls await loadBasicCatalog() outside the
route's try/catch, so failures produce an unhandled 500; wrap the catalog load
in its own try/catch inside the route handler where opts.catalog is resolved
(before building optsWithCatalog) and on error return the route's CORS-safe
error using jsonWithCors(...) with an appropriate message/status. Specifically,
when computing const catalog = opts.catalog ?? await loadBasicCatalog(), instead
first check opts.catalog and if missing call loadBasicCatalog() inside a
try/catch, assign to catalog on success, and on failure return jsonWithCors(...)
so the handler keeps its normal error response behavior; then construct
optsWithCatalog = { ...opts, catalog } and continue.
---
Nitpick comments:
In @.github/a2ui-catalog.instructions.md:
- Line 2: The applyTo glob in .github/a2ui-catalog.instructions.md currently
only targets packages/genui/a2ui*/**, so update the frontmatter applyTo entry to
also include the server catalog paths (e.g., add packages/genui/server/agent/**
or packages/genui/server/**) so the instructions apply to the runtime loader and
fallback snapshot files; ensure the final applyTo value is a comma- or
newline-separated list of globs covering both packages/genui/a2ui*/** and the
server catalog directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7cf9e3ce-568b-4551-8154-5a1da5b0b3b7
📒 Files selected for processing (44)
.github/a2ui-catalog.instructions.mdpackages/genui/a2ui-catalog-extractor/README.mdpackages/genui/a2ui-catalog-extractor/etc/genui-a2ui-catalog-extractor.api.mdpackages/genui/a2ui-catalog-extractor/readme.zh_cn.mdpackages/genui/a2ui-catalog-extractor/src/cli.tspackages/genui/a2ui-catalog-extractor/src/index.tspackages/genui/a2ui-catalog-extractor/test/extractor.test.tspackages/genui/a2ui-prompt/README.mdpackages/genui/a2ui-prompt/etc/genui-a2ui-prompt.api.mdpackages/genui/a2ui-prompt/tsconfig.build.jsonpackages/genui/a2ui-prompt/tsconfig.jsonpackages/genui/a2ui-prompt/turbo.jsonpackages/genui/a2ui/package.jsonpackages/genui/cli/bin/cli.jspackages/genui/etc/genui.api.mdpackages/genui/index.tspackages/genui/server/agent/a2ui-agent.tspackages/genui/server/agent/a2ui-catalog-id.tspackages/genui/server/agent/a2ui-catalog.tspackages/genui/server/agent/a2ui-examples.tspackages/genui/server/agent/a2ui-prompt.tspackages/genui/server/agent/catalog/Button/catalog.jsonpackages/genui/server/agent/catalog/Card/catalog.jsonpackages/genui/server/agent/catalog/CheckBox/catalog.jsonpackages/genui/server/agent/catalog/ChoicePicker/catalog.jsonpackages/genui/server/agent/catalog/Column/catalog.jsonpackages/genui/server/agent/catalog/DateTimeInput/catalog.jsonpackages/genui/server/agent/catalog/Divider/catalog.jsonpackages/genui/server/agent/catalog/Icon/catalog.jsonpackages/genui/server/agent/catalog/Image/catalog.jsonpackages/genui/server/agent/catalog/LineChart/catalog.jsonpackages/genui/server/agent/catalog/List/catalog.jsonpackages/genui/server/agent/catalog/Loading/catalog.jsonpackages/genui/server/agent/catalog/Modal/catalog.jsonpackages/genui/server/agent/catalog/RadioGroup/catalog.jsonpackages/genui/server/agent/catalog/Row/catalog.jsonpackages/genui/server/agent/catalog/Slider/catalog.jsonpackages/genui/server/agent/catalog/Tabs/catalog.jsonpackages/genui/server/agent/catalog/Text/catalog.jsonpackages/genui/server/agent/catalog/TextField/catalog.jsonpackages/genui/server/agent/catalog/catalog.jsonpackages/genui/server/app/a2ui/action/stream/route.tspackages/genui/server/app/a2ui/stream/route.tspackages/genui/server/service/a2ui-agent.ts
💤 Files with no reviewable changes (19)
- packages/genui/server/agent/catalog/ChoicePicker/catalog.json
- packages/genui/server/agent/catalog/Loading/catalog.json
- packages/genui/server/agent/catalog/CheckBox/catalog.json
- packages/genui/server/agent/catalog/Slider/catalog.json
- packages/genui/server/agent/catalog/Card/catalog.json
- packages/genui/server/agent/catalog/LineChart/catalog.json
- packages/genui/server/agent/catalog/Button/catalog.json
- packages/genui/server/agent/catalog/DateTimeInput/catalog.json
- packages/genui/server/agent/catalog/Image/catalog.json
- packages/genui/server/agent/catalog/Tabs/catalog.json
- packages/genui/server/agent/catalog/RadioGroup/catalog.json
- packages/genui/server/agent/catalog/List/catalog.json
- packages/genui/server/agent/catalog/Icon/catalog.json
- packages/genui/server/agent/catalog/Text/catalog.json
- packages/genui/server/agent/catalog/Divider/catalog.json
- packages/genui/server/agent/catalog/Modal/catalog.json
- packages/genui/server/agent/catalog/TextField/catalog.json
- packages/genui/server/agent/catalog/Row/catalog.json
- packages/genui/server/agent/catalog/Column/catalog.json
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/genui/a2ui-catalog-extractor/test/extractor-functions.test.ts (1)
86-95: ⚡ Quick winAssert
catalogIdandfilePathstructurally here.This new coverage still misses the new
catalogIdcontract, andJSON.stringify(fullCatalog)).not.toContain('filePath')is brittle because any string value containing that substring will fail the test for the wrong reason. SincewriteCatalogArtifactsnow serializescatalogIdintocatalog.jsonand the CLI threads--catalog-idthrough that path, assertfullCatalog.catalogIddirectly and check each function object lacks afilePathkey instead of scanning the whole JSON string.Suggested test shape
const fullCatalog = readFullCatalogJson(outDir); + expect(fullCatalog['catalogId']).toBe('catalog.json'); const fullCatalogFunctions = fullCatalog['functions']; expect(Array.isArray(fullCatalogFunctions)).toBe(true); const fullCatalogFunctionList = fullCatalogFunctions as unknown[]; expect([...fullCatalogFunctionList].sort(compareByName)).toEqual([ readExpectedFunctionJson('formatString')['formatString'], readExpectedFunctionJson('required')['required'], ]); - expect(JSON.stringify(fullCatalog)).not.toContain('filePath'); + expect( + fullCatalogFunctionList.every( + value => + typeof value === 'object' && + value !== null && + !('filePath' in value), + ), + ).toBe(true);As per coding guidelines: "Keep README examples tied to tests. If a documented component contract, generated schema, CLI flow, or API example changes, update or add a test fixture or test case in this package so the documented behavior is checked."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/genui/a2ui-catalog-extractor/test/extractor-functions.test.ts` around lines 86 - 95, Assert the new catalogId field and check for filePath keys structurally: in the test using readFullCatalogJson and fullCatalog (and existing symbols fullCatalogFunctions, fullCatalogFunctionList, compareByName, readExpectedFunctionJson), add an assertion that fullCatalog.catalogId equals the expected catalog id value (the value the test fixture/CLI should provide), and replace the brittle expect(JSON.stringify(fullCatalog)).not.toContain('filePath') with a structural check that iterates fullCatalogFunctions and asserts each function object does not have a filePath property (e.g., for each func: expect(Object.prototype.hasOwnProperty.call(func, 'filePath')).toBe(false)); keep the existing function comparison by name intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/genui/a2ui-catalog-extractor/test/extractor-functions.test.ts`:
- Around line 86-95: Assert the new catalogId field and check for filePath keys
structurally: in the test using readFullCatalogJson and fullCatalog (and
existing symbols fullCatalogFunctions, fullCatalogFunctionList, compareByName,
readExpectedFunctionJson), add an assertion that fullCatalog.catalogId equals
the expected catalog id value (the value the test fixture/CLI should provide),
and replace the brittle
expect(JSON.stringify(fullCatalog)).not.toContain('filePath') with a structural
check that iterates fullCatalogFunctions and asserts each function object does
not have a filePath property (e.g., for each func:
expect(Object.prototype.hasOwnProperty.call(func, 'filePath')).toBe(false));
keep the existing function comparison by name intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd8e3770-6d7d-47da-8100-8e5dcc533a89
📒 Files selected for processing (8)
.github/a2ui-catalog.instructions.mdpackages/genui/a2ui-catalog-extractor/src/index.tspackages/genui/a2ui-catalog-extractor/test/extractor-functions.test.tspackages/genui/a2ui-catalog-extractor/test/extractor.test.tspackages/genui/server/agent/a2ui-catalog.tspackages/genui/server/agent/a2ui-prompt.tspackages/genui/server/app/a2ui/action/stream/route.tspackages/genui/server/app/a2ui/stream/route.ts
✅ Files skipped from review due to trivial changes (1)
- .github/a2ui-catalog.instructions.md
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/genui/server/app/a2ui/stream/route.ts
- packages/genui/a2ui-catalog-extractor/test/extractor.test.ts
- packages/genui/server/app/a2ui/action/stream/route.ts
- packages/genui/server/agent/a2ui-prompt.ts
- packages/genui/server/agent/a2ui-catalog.ts
- packages/genui/a2ui-catalog-extractor/src/index.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/genui/server/agent/a2ui-prompt.ts (1)
180-223:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPublic sync prompt construction is now a breaking API change.
This change makes the exported sync builder require
opts.catalog, while the zero-config path moved tobuildA2UISystemPromptAsync().packages/genui/etc/genui.api.mdalso reflects the removed eager prompt export, so existing consumers that calledbuildA2UISystemPrompt()or importedA2UI_SYSTEM_PROMPTwill break on upgrade. Please either preserve a compatibility path/deprecation shim or ship this behind an explicit breaking-change release and migration note.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/genui/server/agent/a2ui-prompt.ts` around lines 180 - 223, Restore backward compatibility by providing the old zero-config sync path: add an exported A2UI_SYSTEM_PROMPT constant (populated by starting loadBasicCatalog() at module init and storing the rendered prompt when the promise resolves) and update buildA2UISystemPrompt to, when called with no opts or no opts.catalog, return that A2UI_SYSTEM_PROMPT while emitting a deprecation warning; keep buildA2UISystemPromptAsync as the recommended async path. Reference buildA2UISystemPrompt, buildA2UISystemPromptAsync, A2UI_SYSTEM_PROMPT, loadBasicCatalog, and renderCatalogReference/renderCatalogExamples when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/genui/server/agent/a2ui-prompt.ts`:
- Around line 180-223: Restore backward compatibility by providing the old
zero-config sync path: add an exported A2UI_SYSTEM_PROMPT constant (populated by
starting loadBasicCatalog() at module init and storing the rendered prompt when
the promise resolves) and update buildA2UISystemPrompt to, when called with no
opts or no opts.catalog, return that A2UI_SYSTEM_PROMPT while emitting a
deprecation warning; keep buildA2UISystemPromptAsync as the recommended async
path. Reference buildA2UISystemPrompt, buildA2UISystemPromptAsync,
A2UI_SYSTEM_PROMPT, loadBasicCatalog, and
renderCatalogReference/renderCatalogExamples when making the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c1586dff-289a-4ed5-a276-c9fa9639e651
📒 Files selected for processing (3)
packages/genui/a2ui-prompt/etc/genui-a2ui-prompt.api.mdpackages/genui/etc/genui.api.mdpackages/genui/server/agent/a2ui-prompt.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/genui/a2ui-prompt/etc/genui-a2ui-prompt.api.md
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Checklist