feat(a2ui): load server catalog from cdn#2778
Open
Sherry-hue wants to merge 1 commit into
Open
Conversation
|
Contributor
📝 WalkthroughWalkthroughThe PR migrates GenUI's basic catalog from static/synchronous imports to dynamic asynchronous fetching with local fallback. It consolidates per-component catalog JSON definitions into a single remotely-hosted ChangesAsync Catalog Loading and Consolidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/genui/server/agent/a2ui-prompt.ts (1)
175-196:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the sync prompt builder require
catalogin its type.
buildA2UISystemPromptnow throws wheneveropts.catalogis absent, butBuildSystemPromptOptionsstill markscatalogas optional. That meansbuildA2UISystemPrompt({})still passes type-checking and only fails at runtime for external consumers. Tighten the sync API to a catalog-required options type and keep the async helper as the no-catalog entrypoint.Based on learnings: when editing exported functions or constants reachable through the a2ui-prompt package, use explicit types for exported APIs because `isolatedDeclarations` requires them.Proposed fix
export interface BuildSystemPromptOptions { catalog?: A2UICatalog; appendix?: string; } -export function buildA2UISystemPrompt(opts: BuildSystemPromptOptions): string; +export interface BuildSystemPromptWithCatalogOptions + extends BuildSystemPromptOptions { + catalog: A2UICatalog; +} + +export function buildA2UISystemPrompt( + opts: BuildSystemPromptWithCatalogOptions, +): string; export function buildA2UISystemPrompt( - opts?: BuildSystemPromptOptions, + opts?: BuildSystemPromptWithCatalogOptions, ): string { if (!opts) { throw new Error( '[a2ui-prompt] buildA2UISystemPrompt requires a catalog. ' + 'Use buildA2UISystemPromptAsync() to load the basic catalog.', @@ export async function buildA2UISystemPromptAsync( opts: BuildSystemPromptOptions = {}, ): Promise<string> { const catalog = opts.catalog ?? await loadBasicCatalog(); return buildA2UISystemPrompt({ ...opts, catalog }); }🤖 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 175 - 196, The sync API's options type must require a catalog: update BuildSystemPromptOptions to make the catalog property non-optional (remove the ?), and adjust any exported overload/signature of buildA2UISystemPrompt to accept that required type; keep the existing buildA2UISystemPromptAsync as the no-catalog entrypoint for callers who don't have a catalog, and ensure exported function signatures remain explicit to satisfy isolatedDeclarations.packages/genui/server/service/a2ui-agent.ts (1)
108-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid caching caller-supplied catalogs by
idalone.The stream/action routes pass request-scoped
catalogobjects into this service, but this key only distinguishes them bycatalog.id. Two different catalogs with the same id will reuse the same cached agent prompt, whilegenerateValidated()still validates against the current request's catalog. That can produce stale instructions and false validation failures on a valid request path. Either hash the full catalog for custom inputs or bypass caching whenopts.catalogis provided.Suggested fix
private async getAgent(opts: ChatOptions): Promise<A2UIAgent> { const startedAt = performance.now(); const catalog = opts.catalog ?? await loadBasicCatalog(); - const cacheKey = `${opts.baseURL ?? 'default'}:${opts.model ?? 'default'}:${ - hashApiKey(opts.apiKey) - }:${catalog.id}`; - let cached = this.agentCache.get(cacheKey); + const cacheKey = opts.catalog + ? undefined + : `${opts.baseURL ?? 'default'}:${opts.model ?? 'default'}:${ + hashApiKey(opts.apiKey) + }:${catalog.id}`; + let cached = cacheKey ? this.agentCache.get(cacheKey) : undefined; if (cached) { opts.onPerformanceEvent?.('agent.cache.hit', { durationMs: performance.now() - startedAt, cacheSize: this.agentCache.size, }); @@ cached = createA2UIAgent(pickDefined({ apiKey: opts.apiKey, baseURL: opts.baseURL, model: opts.model, catalog, })).then(({ agent }) => agent); - this.agentCache.set(cacheKey, cached); + if (cacheKey) { + this.agentCache.set(cacheKey, cached); + }🤖 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/service/a2ui-agent.ts` around lines 108 - 127, The current cacheKey for agentCache only includes catalog.id which lets caller-supplied catalogs with identical ids reuse cached agents; update the logic in the function that builds cacheKey (the block that references catalog, hashApiKey, loadBasicCatalog, and createA2UIAgent) to either (a) bypass caching when opts.catalog is explicitly provided by the caller, or (b) include a deterministic hash of the full catalog object (not just catalog.id) in cacheKey so different catalog contents produce different keys; ensure the rest of the flow (creating cached via createA2UIAgent and storing in agentCache) remains consistent with the chosen approach and that generateValidated continues to validate against the request's catalog.
🤖 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/server/agent/a2ui-catalog-id.ts`:
- Around line 5-6: BASIC_CATALOG_ID currently points to an unversioned unpkg URL
that will move as new `@lynx-js/genui` releases are published; change
BASIC_CATALOG_ID to reference a versioned immutable asset (for example the same
unpkg URL but including the package version like
'https://unpkg.com/@lynx-js/genui@<version>/a2ui/dist/catalog/catalog.json' or a
build-time substituted artifact URL from your CI/artifacts store) so the shipped
server always loads the catalog that matches the shipped code; update the
constant BASIC_CATALOG_ID accordingly and, if applicable, wire it to read the
package version (package.json) or a build environment variable used during
release to ensure it is pinned per build.
In `@packages/genui/server/agent/a2ui-catalog.ts`:
- Around line 256-274: Add a deadline to the CDN fetch in fetchBasicCatalog by
using an AbortController: create an AbortController, pass its signal to
fetch(BASIC_CATALOG_ID, { signal, cache: 'no-store' }), set a short timeout
(e.g., 2–5s) to call controller.abort(), and clear the timer on success; catch
the abort/FetchError the same as other errors so the function falls back to
returning the local catalog (keep existing isExtractedCatalogManifest and
createA2UICatalogFromExtractedManifest logic). Ensure the timer is cleared on
both success and error to avoid leaks and that aborted fetches are handled in
the existing catch block.
- Around line 301-312: isFunctionSpec currently accepts any string for
spec.returnType; change it to reject unknown returnType values by validating
against the allowed union members instead of just typeof string. Update the
returnType check in isFunctionSpec (and/or add a small helper like
isA2UIReturnType) to test membership against the canonical set of allowed return
types (e.g., the union or enum used for A2UIFunctionSpec.returnType) and only
return true if returnType is one of those values; keep the existing checks for
name, parameters, and description unchanged.
---
Outside diff comments:
In `@packages/genui/server/agent/a2ui-prompt.ts`:
- Around line 175-196: The sync API's options type must require a catalog:
update BuildSystemPromptOptions to make the catalog property non-optional
(remove the ?), and adjust any exported overload/signature of
buildA2UISystemPrompt to accept that required type; keep the existing
buildA2UISystemPromptAsync as the no-catalog entrypoint for callers who don't
have a catalog, and ensure exported function signatures remain explicit to
satisfy isolatedDeclarations.
In `@packages/genui/server/service/a2ui-agent.ts`:
- Around line 108-127: The current cacheKey for agentCache only includes
catalog.id which lets caller-supplied catalogs with identical ids reuse cached
agents; update the logic in the function that builds cacheKey (the block that
references catalog, hashApiKey, loadBasicCatalog, and createA2UIAgent) to either
(a) bypass caching when opts.catalog is explicitly provided by the caller, or
(b) include a deterministic hash of the full catalog object (not just
catalog.id) in cacheKey so different catalog contents produce different keys;
ensure the rest of the flow (creating cached via createA2UIAgent and storing in
agentCache) remains consistent with the chosen approach and that
generateValidated continues to validate against the request's catalog.
🪄 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: 3ccecb60-1b43-40b9-ac5c-ebf58dbcff95
📒 Files selected for processing (36)
packages/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/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/Icon/catalog.json
- packages/genui/server/agent/catalog/Divider/catalog.json
- packages/genui/server/agent/catalog/ChoicePicker/catalog.json
- packages/genui/server/agent/catalog/CheckBox/catalog.json
- packages/genui/server/agent/catalog/Text/catalog.json
- packages/genui/server/agent/catalog/TextField/catalog.json
- packages/genui/server/agent/catalog/Tabs/catalog.json
- packages/genui/server/agent/catalog/DateTimeInput/catalog.json
- packages/genui/server/agent/catalog/Image/catalog.json
- packages/genui/server/agent/catalog/List/catalog.json
- packages/genui/server/agent/catalog/Slider/catalog.json
- packages/genui/server/agent/catalog/Row/catalog.json
- packages/genui/server/agent/catalog/Loading/catalog.json
- packages/genui/server/agent/catalog/Column/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/RadioGroup/catalog.json
- packages/genui/server/agent/catalog/Modal/catalog.json
Comment on lines
+5
to
+6
| export const BASIC_CATALOG_ID = | ||
| 'https://unpkg.com/@lynx-js/genui/a2ui/dist/catalog/catalog.json'; |
Contributor
There was a problem hiding this comment.
Pin BASIC_CATALOG_ID to an immutable artifact.
This URL tracks the latest published @lynx-js/genui package on unpkg, so a future release can change the basic catalog without any server deploy. That breaks the contract between the shipped prompt/examples code and the remotely loaded schema. Please point this at a versioned asset (or another immutable same-build artifact) instead.
🤖 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-catalog-id.ts` around lines 5 - 6,
BASIC_CATALOG_ID currently points to an unversioned unpkg URL that will move as
new `@lynx-js/genui` releases are published; change BASIC_CATALOG_ID to reference
a versioned immutable asset (for example the same unpkg URL but including the
package version like
'https://unpkg.com/@lynx-js/genui@<version>/a2ui/dist/catalog/catalog.json' or a
build-time substituted artifact URL from your CI/artifacts store) so the shipped
server always loads the catalog that matches the shipped code; update the
constant BASIC_CATALOG_ID accordingly and, if applicable, wire it to read the
package version (package.json) or a build environment variable used during
release to ensure it is pinned per build.
Comment on lines
+256
to
+274
| async function fetchBasicCatalog(): Promise<A2UICatalog> { | ||
| try { | ||
| const response = await fetch(BASIC_CATALOG_ID, { cache: 'no-store' }); | ||
| if (!response.ok) { | ||
| throw new Error(`${response.status} ${response.statusText}`); | ||
| } | ||
|
|
||
| const manifest = await response.json() as unknown; | ||
| if (!isExtractedCatalogManifest(manifest)) { | ||
| throw new Error('invalid catalog payload'); | ||
| } | ||
| return createA2UICatalogFromExtractedManifest(manifest); | ||
| } catch (error) { | ||
| console.warn( | ||
| `[a2ui-catalog] Failed to fetch catalog ${BASIC_CATALOG_ID}; ` | ||
| + 'using local fallback catalog.', | ||
| error, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Add a timeout around the CDN fetch.
loadBasicCatalog() is awaited from request-time paths, so this unbounded fetch() can hold the whole request open until the platform times out if the CDN connection hangs. Please abort after a short deadline so the code falls back to catalog.json promptly instead of turning a transient CDN issue into a request outage.
🤖 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-catalog.ts` around lines 256 - 274, Add a
deadline to the CDN fetch in fetchBasicCatalog by using an AbortController:
create an AbortController, pass its signal to fetch(BASIC_CATALOG_ID, { signal,
cache: 'no-store' }), set a short timeout (e.g., 2–5s) to call
controller.abort(), and clear the timer on success; catch the abort/FetchError
the same as other errors so the function falls back to returning the local
catalog (keep existing isExtractedCatalogManifest and
createA2UICatalogFromExtractedManifest logic). Ensure the timer is cleared on
both success and error to avoid leaks and that aborted fetches are handled in
the existing catch block.
Comment on lines
+301
to
+312
| function isFunctionSpec(value: unknown): value is A2UIFunctionSpec { | ||
| if (!isRecord(value)) return false; | ||
| const spec = value as Partial<A2UIFunctionSpec>; | ||
| const name = spec.name; | ||
| const parameters = spec.parameters; | ||
| const returnType = spec.returnType; | ||
| const description = spec.description; | ||
| return typeof name === 'string' | ||
| && isRecord(parameters) | ||
| && typeof returnType === 'string' | ||
| && (description === undefined || typeof description === 'string'); | ||
| } |
Contributor
There was a problem hiding this comment.
Reject unknown returnType values during manifest validation.
isFunctionSpec() accepts any string here, so a payload like { returnType: "date" } still passes isExtractedCatalogManifest() and bypasses the local fallback. That weakens the new “validated fetch” path and can leak unsupported function signatures into prompt generation. Constrain this check to the allowed union members before accepting the manifest.
🤖 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-catalog.ts` around lines 301 - 312,
isFunctionSpec currently accepts any string for spec.returnType; change it to
reject unknown returnType values by validating against the allowed union members
instead of just typeof string. Update the returnType check in isFunctionSpec
(and/or add a small helper like isA2UIReturnType) to test membership against the
canonical set of allowed return types (e.g., the union or enum used for
A2UIFunctionSpec.returnType) and only return true if returnType is one of those
values; keep the existing checks for name, parameters, and description
unchanged.
Merging this PR will not alter performance
Comparing 
Footnotes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
buildA2UISystemPromptAsync()for asynchronous system prompt generation.loadBasicCatalog()for dynamic catalog loading.Breaking Changes
buildA2UISystemPrompt()now requires options parameter (no longer optional).A2UI_SYSTEM_PROMPTandBASIC_CATALOGconstants.Updates
Checklist