refactor(a2ui): headless renderer, MessageStore, pluggable catalog#2536
Closed
PupilTong wants to merge 2 commits into
Closed
refactor(a2ui): headless renderer, MessageStore, pluggable catalog#2536PupilTong wants to merge 2 commits into
PupilTong wants to merge 2 commits into
Conversation
|
Contributor
|
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:
📝 WalkthroughWalkthroughRemoves a chunk-scoped ChangesWeb Core Fetch Enablement
A2UI Catalog & Store Refactor
🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will not alter performance
Comparing 
Footnotes
|
React External#923 Bundle Size — 680.82KiB (0%).f32fcbc(current) vs 5f3b6eb main#913(baseline) Bundle metrics
|
| Current #923 |
Baseline #913 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch PupilTong:claude/quirky-merkle-9... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9381 Bundle Size — 900.02KiB (~-0.01%).f32fcbc(current) vs 5f3b6eb main#9371(baseline) Bundle metrics
|
| Current #9381 |
Baseline #9371 |
|
|---|---|---|
| Initial JS |
44.46KiB |
44.46KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
 Cache Invalidation |
1.53% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
230(+0.88%) |
228 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.28%(-0.04%) |
27.29% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 
1 improvement
| Current #9381 |
Baseline #9371 |
|
|---|---|---|
| JS |
495.88KiB (~-0.01%) |
495.9KiB |
| Other |
401.92KiB |
401.92KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch PupilTong:claude/quirky-merkle-9... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#940 Bundle Size — 196.68KiB (0%).f32fcbc(current) vs 5f3b6eb main#930(baseline) Bundle metrics
|
| Current #940 |
Baseline #930 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
174 |
174 |
| Duplicate Modules |
66 |
66 |
| Duplicate Code |
44.05% |
44.05% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #940 |
Baseline #930 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
85.45KiB |
85.45KiB |
Bundle analysis report Branch PupilTong:claude/quirky-merkle-9... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7808 Bundle Size — 225.52KiB (0%).f32fcbc(current) vs 5f3b6eb main#7798(baseline) Bundle metrics
|
| Current #7808 |
Baseline #7798 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
180 |
180 |
| Duplicate Modules |
69 |
69 |
| Duplicate Code |
44.54% |
44.54% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7808 |
Baseline #7798 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
79.77KiB |
79.77KiB |
Bundle analysis report Branch PupilTong:claude/quirky-merkle-9... Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
from
ff50c0c to
7e61ec6
Compare
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
2 times, most recently
from
a4bded1 to
d4cc23e
Compare
5 tasks
PupilTong
added a commit
to PupilTong/lynx-stack
that referenced
this pull request
May 6, 2026
Move the catalog `<Name>/index.tsx` files (and `defineCatalog.ts`) to import from `store/types` and `react/A2UIRenderer` so they sit on the post-refactor module layout. Add three thin compatibility re-exports so the new paths resolve against the still-present `core/` implementation: - `src/store/types.ts` → re-exports `core/types.js` - `src/react/A2UIRenderer.tsx` → re-exports `A2UIRender as A2UIRenderer` and `NodeRenderer` from `core/A2UIRender.jsx` - `src/react/useDataBinding.ts` → re-exports `core/useDataBinding.js` The full headless-renderer refactor (lynx-family#2536) replaces these shims with real implementations; this PR only relocates the catalog import surface so that follow-up rebases shrink to a no-op delete of the shims. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PupilTong
added a commit
that referenced
this pull request
May 6, 2026
## Summary
- Adds a runtime `defineCatalog` API that lets renderer consumers
compose per-instance catalogs without depending on the global
`componentRegistry` side-effect path. Bundlers tree-shake unused
built-ins because the import surface is per-component.
- Adds `mergeCatalogs`, `serializeCatalog`, and `resolveCatalog` so app
shells can layer brand/page-level catalogs and emit the agent-handshake
JSON (with optional per-component schemas) directly.
- Harden the extractor CLI's bin-entry detection so
`bin/a2ui-catalog-extractor.{m,c,}js` is recognized alongside direct ESM
execution.
This PR is **additive** — `export * from './all.js'` is preserved in
`catalog/index.ts`, so existing consumers of the global registry are
unaffected.
Companion to the broader headless-renderer refactor in
[#2536](#2536); this slice
can land independently because nothing in `core/` or `chat/` depends on
it.
## Test plan
- [ ] `pnpm -F @lynx-js/a2ui-reactlynx build` — extractor still emits
`dist/catalog/<Name>/catalog.json` for all 10 built-ins.
- [ ] `pnpm -F @lynx-js/a2ui-catalog-extractor test` — 6/6 extractor
tests pass.
- [ ] `pnpm -F @lynx-js/a2ui-reactlynx exec tsc --noEmit` — clean
(pre-existing `lynx-ui-input` error in `chat/Conversation.tsx` is
unrelated).
- [ ] Spot-check `defineCatalog([Text])` produces `[{ name: 'Text',
component: Text }]`.
- [ ] Spot-check `defineCatalog([[Text, textManifest]])` reads the name
from the manifest key (survives minification).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a catalog system for composing, merging, resolving, and
serializing UI component collections.
* **Documentation**
* Added a comprehensive catalog README with usage, schema guidance,
recipes, and public API notes.
* **Refactor**
* Stabilized React renderer and data-binding import paths via
compatibility re-exports.
* **Chores**
* Updated internal component imports to the new renderer/store paths;
component APIs and behavior unchanged.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
5 times, most recently
from
cc021bc to
021772c
Compare
PupilTong
requested review from
HuJean,
Sherry-hue,
fzx2666-fz and
gaoachao
as code owners
![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: 10
🧹 Nitpick comments (9)
packages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.ts (1)
92-92: 💤 Low valueRemove commented-out dead code instead of leaving it in source.
Leaving
// 'fetch'and// tt.fetch,as inline comments achieves the same runtime result as deleting them, but accumulates dead code in production source. The changeset and git history already document why the parameter was removed.🧹 Proposed cleanup
- // 'fetch', 'XMLHttpRequest',- // tt.fetch, tt.XMLHttpRequest,Also applies to: 137-137
🤖 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/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.ts` at line 92, Remove the leftover commented-out dead code entries for the removed runtime fetch parameter (specifically the inline comments "'fetch'" and "tt.fetch," present in createChunkLoading.ts)—delete those commented lines so the source contains no commented placeholders; update any adjacent comment formatting if needed to keep the surrounding code clean and run a quick build/test to confirm nothing else references the removed entries (look for these comments near the createChunkLoading-related functions).packages/web-platform/web-core/ts/types/BTSChunk.ts (1)
35-35: 💤 Low valueSame dead-code concern applies here — delete rather than comment out.
// fetch: unknown,inside theBTSChunkEntryfunction type is inert dead code. Delete it to keep the type definition clean; the removal rationale lives in the changeset and git history.🧹 Proposed cleanup
prompt: unknown, - // fetch: unknown, XMLHttpRequest: unknown,🤖 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/web-platform/web-core/ts/types/BTSChunk.ts` at line 35, Remove the inert commented-out dead code "// fetch: unknown," from the BTSChunkEntry type definition in BTSChunk.ts; open the BTSChunkEntry function/type declaration, delete that commented line so the type remains clean and rely on git history/changeset for rationale.packages/genui/a2ui/tsconfig.build.json (1)
2-8: ⚡ Quick winPrevent test-global type leakage into build config.
This file extends
./tsconfig.json, which includes"types": ["@rstest/core/globals"]. The build config does not override this, so test globals are inherited. While no test-global usage currently exists insrc, explicitly setting"types": []prevents accidental imports and clarifies intent for production builds.Suggested patch
{ "extends": "./tsconfig.json", "compilerOptions": { "noEmit": false, "rootDir": "./src", + "types": [] }, "include": ["src"], "exclude": ["test"], }🤖 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/tsconfig.build.json` around lines 2 - 8, The build tsconfig inherits test globals from ./tsconfig.json; to prevent test-global type leakage, add an explicit "types": [] entry under "compilerOptions" in tsconfig.build.json to override the inherited "types" (ensure you modify the existing "compilerOptions" block where "noEmit" and "rootDir" are set), so production builds do not pick up `@rstest/core/globals` or any other test-only types.packages/genui/a2ui/src/store/MessageProcessor.ts (2)
441-446: ⚡ Quick win
messageIdfallback is inconsistent withbeginRendering.
beginRenderingfalls back tosurface:${surfaceId}when the message lacks amessageId(lines 325-326), butdeleteSurfacehere passes(message as { messageId?: string }).messageIdstraight through. Downstream subscribers that key resources bymessageIdmay receiveundefinedfor delete events while getting a synthetic id for begin events from the same surface.🛠 Apply the same fallback
this.emitUpdate({ type: 'deleteSurface', surfaceId, targetId: surface?.rootComponentId ?? surfaceId, - messageId: (message as { messageId?: string }).messageId, + messageId: (message as { messageId?: string }).messageId + ?? `surface:${surfaceId}`, });🤖 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/src/store/MessageProcessor.ts` around lines 441 - 446, The deleteSurface update emits a messageId that can be undefined, causing inconsistency with beginRendering which uses a synthetic id; update the emitUpdate call in MessageProcessor (the deleteSurface branch) to use the same fallback as beginRendering by replacing the raw (message as { messageId?: string }).messageId with message.messageId ?? `surface:${surfaceId}` so downstream subscribers see the same key; keep other fields (surfaceId and targetId/surface?.rootComponentId) unchanged.
40-42: 💤 Low valueConsider isolating listener errors during broadcast.
If any
updateListenerthrows synchronously, the loop aborts and remaining listeners never see the update. With multi-subscriber semantics now intentional (and StrictMode double-mounting amplifying the chance of stray listener errors), wrapping each invocation intry/catchmakes the channel resilient to faulty subscribers.🤖 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/src/store/MessageProcessor.ts` around lines 40 - 42, The emitUpdate method currently calls each listener directly so a thrown error from one listener aborts the loop; modify emitUpdate (the method using this.updateListeners) to wrap each callback invocation in a try/catch so a failing listener does not prevent subsequent listeners from receiving the update, and in the catch log the error (e.g., console.error or existing logger) with context including the listener identity and the data being emitted before continuing to the next listener.packages/genui/a2ui/src/store/Resource.ts (1)
20-23: 💤 Low value
onUpdatedoc claim "Behaves identically" doesn't match the implementation.The wrapped listener early-returns when
value === undefined, soonUpdatewill not fire forfail()(and won't fire forcomplete(undefined)either), unlikesubscribe. Either correct the JSDoc or align the wrapper so the deprecated path truly mirrorssubscribe.Also applies to: 89-97
🤖 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/src/store/Resource.ts` around lines 20 - 23, The JSDoc for onUpdate is incorrect because its wrapper currently early-returns when value === undefined, so onUpdate callbacks are skipped for fail() and complete(undefined) and thus do not mirror subscribe; update the wrapper in Resource.onUpdate (and the duplicate wrapper around lines 89-97) to stop ignoring undefined—either remove the early-return or explicitly forward undefined to the callback so the deprecated onUpdate behaves identically to subscribe for fail() and complete(undefined), and then adjust the JSDoc if you prefer to keep behavior different.packages/genui/a2ui/test/processor.test.ts (1)
125-146: ⚡ Quick win
dispatchresult isvoid-ed but the test relies on synchronous listener invocation
void proc.dispatch({ x: 1 })(lines 138, 143) fires the dispatch without awaiting, then immediately assertscalls1.length/calls2.length. This is only deterministic ifdispatchcallsonEventlisteners synchronously. If the implementation ever becomes async (deferred resolution, microtask scheduling, etc.), the assertions would pass vacuously because the counts would still be 0 at assertion time.Consider awaiting the result to make the intent explicit and guard against future async changes:
✏️ Suggested fix
- test('onEvent multi-listener with disposer', () => { + test('onEvent multi-listener with disposer', async () => { // ... - void proc.dispatch({ x: 1 }); + await proc.dispatch({ x: 1 }); expect(calls1.length).toBe(1); expect(calls2.length).toBe(1); d1(); - void proc.dispatch({ x: 2 }); + await proc.dispatch({ x: 2 }); expect(calls1.length).toBe(1); expect(calls2.length).toBe(2); });🤖 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/test/processor.test.ts` around lines 125 - 146, The test relies on synchronous delivery from MessageProcessor.dispatch but calls it with "void proc.dispatch(...)" which hides its asynchronous nature; update the test to await dispatch so the assertions run after listeners have been invoked: make the test function async and replace the two "void proc.dispatch({ x: ... })" calls with "await proc.dispatch({ x: ... })" (keeping the disposer d1() call as-is) so it deterministically waits for MessageProcessor.dispatch to complete and for onEvent handlers to run.packages/genui/a2ui/src/store/MessageStore.ts (1)
54-56: ⚡ Quick win
notify()iterates aSetthat can be mutated by subscriber callbacksIf a subscriber's callback invokes
dispose()on another subscription (or even its own), thefor...ofloop oversubscriberscan miss entries: removing a not-yet-visited element skips it. Snapshotting the set before iteration prevents this.🛡️ Defensive fix
- const notify = () => { - for (const cb of subscribers) cb(); - }; + const notify = () => { + for (const cb of [...subscribers]) cb(); + };🤖 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/src/store/MessageStore.ts` around lines 54 - 56, The notify function currently iterates the subscribers Set directly which can be mutated by callbacks and cause entries to be skipped; fix notify() (the function closing over the subscribers Set in MessageStore) by snapshotting the collection before iteration (e.g., convert subscribers to an array like Array.from(subscribers) or [...subscribers]) and iterate that snapshot so removals/additions during callbacks don’t affect which callbacks are invoked.packages/genui/a2ui/src/react/A2UI.tsx (1)
99-99: 💤 Low valueReturn type uses a redundant inline import when
ReactNodeis already imported
ReactNodeis already brought in viaimport type { ReactNode } from '@lynx-js/react'on line 14; the function signature'simport('@lynx-js/react').ReactNodeis redundant and inconsistent.✏️ Suggested fix
-function A2UIImpl(props: A2UIProps): import('@lynx-js/react').ReactNode { +function A2UIImpl(props: A2UIProps): ReactNode {🤖 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/src/react/A2UI.tsx` at line 99, The function A2UIImpl currently declares its return type as import('@lynx-js/react').ReactNode which is redundant and inconsistent with the top-level import; change the signature of A2UIImpl to use the already imported ReactNode type (i.e., return type ReactNode) so it matches the existing import type { ReactNode } from '@lynx-js/react' and removes the inline import; update the A2UIImpl declaration accordingly to reference ReactNode directly.
🤖 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-playground/examples/io-mock/mockAgent.ts`:
- Around line 52-60: The inter-message delay in streamInto still awaits a plain
setTimeout so abort.signal.abort() doesn't cancel an in-flight wait; change the
delay logic in streamInto to use an abort-aware sleep: when delayMs > 0 create a
Promise that sets a timeout and also subscribes to abort.signal (via
addEventListener or signal.onabort) to clear the timeout and resolve immediately
(or reject if you prefer) when aborted, and ensure you remove the listener after
either path; keep the existing abort.signal.aborted checks before pushing
messages (store.push) and after the awaited sleep so start()/onAction() can
settle immediately when stop() flips the signal.
In `@packages/genui/a2ui-playground/examples/io-sse/sseAgent.ts`:
- Around line 129-204: The send() promise can hang because some exit paths only
close the EventSource; ensure send() always settles and cleans up by calling
resolve() or reject() on every path, closing eventSource, removing the abort
listener, and stopping further queue processing. Update the onAbort handler (and
the 'error', parse-failure catches in 'delta'/'complete', and the 'complete'
path) to set isCompleted = true, clear or drain the queue and prevent flush from
continuing (e.g., toggle a flag like isProcessing or isCancelled), call
eventSource.close(), remove abort.signal listener (onAbort), and then call
reject() or resolve() as appropriate so send() never hangs; ensure ingestPayload
errors trigger the same cleanup sequence. Reference functions/variables: send(),
stop(), onAbort, eventSource, abort.signal, ingestPayload, flush, queue,
isCompleted, hasReceivedDelta.
In `@packages/genui/a2ui/src/catalog/README.md`:
- Around line 3-7: Update the README note about "no all-in-one catalog" to
explicitly state the exception: mention the `lynxBuiltinCatalog` default
introduced at the PR-level and specify exactly which entrypoint/export exposes
it (name the module that exports `lynxBuiltinCatalog`), and clarify that the
package itself still avoids a top-level aggregate to preserve tree-shaking while
offering `lynxBuiltinCatalog` as an opt-in convenience from that specific
entrypoint; update the README text and the PR-level API description to reference
the same export name and import path so docs and API are consistent.
In `@packages/genui/a2ui/src/react/A2UI.tsx`:
- Around line 119-229: The session stored in sessionRef (sessionRef.current /
session.processor / session.processedCount) is never reset when the messageStore
prop instance changes, causing the old MessageProcessor graph to be reused for a
new message stream; to fix, detect when the messageStore object identity changes
(e.g., via a useEffect that depends on the messageStore reference or messages
subscribe function) and recreate the session by assigning sessionRef.current = {
processor: new MessageProcessor(), resources: new Map(), activeMessageId: null,
processedCount: 0 } so a fresh MessageProcessor and empty bookkeeping are used
for the new store; ensure to perform this reset before processing new messages
so session.processor and session.processedCount match the new messageStore.
- Around line 148-191: The onUpdate handler currently calls
proc.getOrCreateSurface(surfaceId) unconditionally which creates an empty
Surface for deleteSurface events and prevents completing the real resource;
modify the handler so getOrCreateSurface(surfaceId) is only called inside
branches that need a live surface (e.g., beginRendering and surfaceUpdate) and
for the deleteSurface branch avoid creating a new Surface — instead read
targetId from the incoming data and complete the resource via
session.resources.get(targetId) (or check session.resources) so the existing
resource is completed; refer to proc.onUpdate, getOrCreateSurface, the
deleteSurface branch and session.resources to locate the code to change.
In `@packages/genui/a2ui/src/react/useDataBinding.ts`:
- Around line 41-55: In useDataBinding, stop stringifying primitive static
values: instead of setting initialValue = String(dynamicValue) when typeof
dynamicValue === 'number' || 'boolean', assign the raw primitive (initialValue =
dynamicValue) and ensure initialValue's type supports string | number | boolean
| undefined; similarly update the other occurrence where numbers/booleans are
being stringified (the second branch that handles dynamicValue primitives). Keep
the existing object/path handling for dynamic bindings (path variable)
unchanged; adjust types/signatures for useDataBinding and any related
return/consumer code so they accept and propagate string|number|boolean without
coercion to string.
In `@packages/genui/a2ui/src/store/payloadNormalizer.ts`:
- Around line 102-147: The handler function handle in normalizePayloadToMessages
currently returns for non-string primitives (e.g., 0, false) and thus drops
them; update handle to treat primitive non-string values (number, boolean,
bigint, symbol) as fallback text by converting them to strings and calling
add(createFallbackMessagesFromPlainText(String(value))); keep existing early
returns for null/undefined and strings, and also apply the same stringification
logic for v['data'] when kind==='data' if v['data'] is a primitive so it doesn't
get ignored.
In `@packages/genui/a2ui/src/store/Resource.ts`:
- Around line 29-34: getSnapshot currently returns only the raw value so React's
Object.is check can miss transitions like fail() or complete(sameRef); fix by
caching and returning a single immutable snapshot object that contains { value,
status, error } and replace every state transition (e.g., fail(), complete(),
any status-updating methods in Resource) to create a new snapshot object and
notify subscribers; change getSnapshot to return that snapshot reference and
update A2UIRenderer.tsx to read value/status/error from resource.getSnapshot()
(destructure the snapshot) rather than accessing resource.status/resource.error
directly so concurrent rendering sees every transition.
- Around line 75-88: The complete method can overwrite a terminal error state;
update complete (in Resource.ts) to mirror fail's guard by returning early
unless status === 'pending' so you never transition from 'error' to 'success'
(i.e., only set value, status='success', call resolve(res) and notify() when
pending). Reference the existing symbols complete, fail, status, error, value,
resolve, reject, and notify and ensure you do not clear or reuse error when
skipping the transition.
In `@packages/genui/a2ui/src/store/types.ts`:
- Around line 77-82: Update GenericComponentProps.sendAction to reflect the real
action contract: replace the loose (Record<string, unknown>) => void signature
with the async action type used by useAction(), i.e. sendAction?: (action:
v0_9.Action) => Promise<void>; and import or reference the v0_9.Action type
where GenericComponentProps is declared so callers and implementers (and catalog
components) get proper type-checking and the async boundary is preserved.
---
Nitpick comments:
In `@packages/genui/a2ui/src/react/A2UI.tsx`:
- Line 99: The function A2UIImpl currently declares its return type as
import('@lynx-js/react').ReactNode which is redundant and inconsistent with the
top-level import; change the signature of A2UIImpl to use the already imported
ReactNode type (i.e., return type ReactNode) so it matches the existing import
type { ReactNode } from '@lynx-js/react' and removes the inline import; update
the A2UIImpl declaration accordingly to reference ReactNode directly.
In `@packages/genui/a2ui/src/store/MessageProcessor.ts`:
- Around line 441-446: The deleteSurface update emits a messageId that can be
undefined, causing inconsistency with beginRendering which uses a synthetic id;
update the emitUpdate call in MessageProcessor (the deleteSurface branch) to use
the same fallback as beginRendering by replacing the raw (message as {
messageId?: string }).messageId with message.messageId ?? `surface:${surfaceId}`
so downstream subscribers see the same key; keep other fields (surfaceId and
targetId/surface?.rootComponentId) unchanged.
- Around line 40-42: The emitUpdate method currently calls each listener
directly so a thrown error from one listener aborts the loop; modify emitUpdate
(the method using this.updateListeners) to wrap each callback invocation in a
try/catch so a failing listener does not prevent subsequent listeners from
receiving the update, and in the catch log the error (e.g., console.error or
existing logger) with context including the listener identity and the data being
emitted before continuing to the next listener.
In `@packages/genui/a2ui/src/store/MessageStore.ts`:
- Around line 54-56: The notify function currently iterates the subscribers Set
directly which can be mutated by callbacks and cause entries to be skipped; fix
notify() (the function closing over the subscribers Set in MessageStore) by
snapshotting the collection before iteration (e.g., convert subscribers to an
array like Array.from(subscribers) or [...subscribers]) and iterate that
snapshot so removals/additions during callbacks don’t affect which callbacks are
invoked.
In `@packages/genui/a2ui/src/store/Resource.ts`:
- Around line 20-23: The JSDoc for onUpdate is incorrect because its wrapper
currently early-returns when value === undefined, so onUpdate callbacks are
skipped for fail() and complete(undefined) and thus do not mirror subscribe;
update the wrapper in Resource.onUpdate (and the duplicate wrapper around lines
89-97) to stop ignoring undefined—either remove the early-return or explicitly
forward undefined to the callback so the deprecated onUpdate behaves identically
to subscribe for fail() and complete(undefined), and then adjust the JSDoc if
you prefer to keep behavior different.
In `@packages/genui/a2ui/test/processor.test.ts`:
- Around line 125-146: The test relies on synchronous delivery from
MessageProcessor.dispatch but calls it with "void proc.dispatch(...)" which
hides its asynchronous nature; update the test to await dispatch so the
assertions run after listeners have been invoked: make the test function async
and replace the two "void proc.dispatch({ x: ... })" calls with "await
proc.dispatch({ x: ... })" (keeping the disposer d1() call as-is) so it
deterministically waits for MessageProcessor.dispatch to complete and for
onEvent handlers to run.
In `@packages/genui/a2ui/tsconfig.build.json`:
- Around line 2-8: The build tsconfig inherits test globals from
./tsconfig.json; to prevent test-global type leakage, add an explicit "types":
[] entry under "compilerOptions" in tsconfig.build.json to override the
inherited "types" (ensure you modify the existing "compilerOptions" block where
"noEmit" and "rootDir" are set), so production builds do not pick up
`@rstest/core/globals` or any other test-only types.
In
`@packages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.ts`:
- Line 92: Remove the leftover commented-out dead code entries for the removed
runtime fetch parameter (specifically the inline comments "'fetch'" and
"tt.fetch," present in createChunkLoading.ts)—delete those commented lines so
the source contains no commented placeholders; update any adjacent comment
formatting if needed to keep the surrounding code clean and run a quick
build/test to confirm nothing else references the removed entries (look for
these comments near the createChunkLoading-related functions).
In `@packages/web-platform/web-core/ts/types/BTSChunk.ts`:
- Line 35: Remove the inert commented-out dead code "// fetch: unknown," from
the BTSChunkEntry type definition in BTSChunk.ts; open the BTSChunkEntry
function/type declaration, delete that commented line so the type remains clean
and rely on git history/changeset for rationale.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f6418c87-630a-445b-a1d7-98c8b934764d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (63)
.changeset/quirky-merkle-web-core-fetch.mdpackages/genui/a2ui-playground/examples/README.mdpackages/genui/a2ui-playground/examples/io-mock/mockAgent.tspackages/genui/a2ui-playground/examples/io-sse/sseAgent.tspackages/genui/a2ui-playground/lynx-src/App.tsxpackages/genui/a2ui-playground/lynx-src/tsconfig.jsonpackages/genui/a2ui-playground/lynx.config.tspackages/genui/a2ui-playground/package.jsonpackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui/package.jsonpackages/genui/a2ui/rstest.config.tspackages/genui/a2ui/src/catalog/Button.tspackages/genui/a2ui/src/catalog/Card.tspackages/genui/a2ui/src/catalog/CheckBox.tspackages/genui/a2ui/src/catalog/Column.tspackages/genui/a2ui/src/catalog/Divider.tspackages/genui/a2ui/src/catalog/Image.tspackages/genui/a2ui/src/catalog/List.tspackages/genui/a2ui/src/catalog/README.mdpackages/genui/a2ui/src/catalog/RadioGroup.tspackages/genui/a2ui/src/catalog/Row.tspackages/genui/a2ui/src/catalog/Text.tspackages/genui/a2ui/src/catalog/all.tspackages/genui/a2ui/src/catalog/index.tspackages/genui/a2ui/src/chat/Conversation.tsxpackages/genui/a2ui/src/chat/index.tspackages/genui/a2ui/src/chat/useLynxClient.tspackages/genui/a2ui/src/core/A2UIRender.tsxpackages/genui/a2ui/src/core/BaseClient.tspackages/genui/a2ui/src/core/ComponentRegistry.tspackages/genui/a2ui/src/core/index.tspackages/genui/a2ui/src/core/types.tspackages/genui/a2ui/src/core/useAction.tspackages/genui/a2ui/src/core/useDataBinding.tspackages/genui/a2ui/src/index.tspackages/genui/a2ui/src/react/A2UI.tsxpackages/genui/a2ui/src/react/A2UIProvider.tsxpackages/genui/a2ui/src/react/A2UIRenderer.tsxpackages/genui/a2ui/src/react/index.tspackages/genui/a2ui/src/react/useA2UIContext.tspackages/genui/a2ui/src/react/useAction.tspackages/genui/a2ui/src/react/useCatalog.tspackages/genui/a2ui/src/react/useDataBinding.tspackages/genui/a2ui/src/store/MessageProcessor.tspackages/genui/a2ui/src/store/MessageStore.tspackages/genui/a2ui/src/store/Resource.tspackages/genui/a2ui/src/store/SignalStore.tspackages/genui/a2ui/src/store/index.tspackages/genui/a2ui/src/store/payloadNormalizer.tspackages/genui/a2ui/src/store/types.tspackages/genui/a2ui/src/utils/ComponentRegistry.tspackages/genui/a2ui/src/utils/createResource.tspackages/genui/a2ui/src/utils/index.tspackages/genui/a2ui/test/catalog.test.tspackages/genui/a2ui/test/createResource.test.tspackages/genui/a2ui/test/messageStore.test.tspackages/genui/a2ui/test/payloadNormalizer.test.tspackages/genui/a2ui/test/processor.test.tspackages/genui/a2ui/tsconfig.build.jsonpackages/genui/a2ui/tsconfig.jsonpackages/genui/tsconfig.jsonpackages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.tspackages/web-platform/web-core/ts/types/BTSChunk.ts
💤 Files with no reviewable changes (24)
- packages/genui/a2ui/src/utils/ComponentRegistry.ts
- packages/genui/a2ui/src/utils/createResource.ts
- packages/genui/a2ui/src/core/BaseClient.ts
- packages/genui/a2ui/src/catalog/Image.ts
- packages/genui/a2ui/src/chat/useLynxClient.ts
- packages/genui/a2ui/src/catalog/Row.ts
- packages/genui/a2ui/src/catalog/List.ts
- packages/genui/a2ui/src/utils/index.ts
- packages/genui/a2ui/src/core/types.ts
- packages/genui/a2ui/src/catalog/Divider.ts
- packages/genui/a2ui/src/catalog/Text.ts
- packages/genui/a2ui/src/chat/Conversation.tsx
- packages/genui/a2ui/src/catalog/Column.ts
- packages/genui/a2ui/src/chat/index.ts
- packages/genui/a2ui/src/core/A2UIRender.tsx
- packages/genui/a2ui/src/core/ComponentRegistry.ts
- packages/genui/a2ui/src/core/index.ts
- packages/genui/a2ui/src/catalog/RadioGroup.ts
- packages/genui/a2ui/src/core/useAction.ts
- packages/genui/a2ui/src/catalog/Card.ts
- packages/genui/a2ui/src/catalog/Button.ts
- packages/genui/a2ui/src/catalog/all.ts
- packages/genui/a2ui/src/core/useDataBinding.ts
- packages/genui/a2ui/src/catalog/CheckBox.ts
Comment on lines
+3
to
+7
| The package intentionally **does not** ship an "all-in-one" catalog | ||
| constant. A top-level array referencing every built-in defeats | ||
| tree-shaking — every consumer of such an aggregate would bundle every | ||
| component, even the nine you don't use. Composition is per-component, and | ||
| the cost is visible at the import site. |
Contributor
There was a problem hiding this comment.
Clarify all-in-one catalog statement vs exported defaults.
This section says the package intentionally does not ship an all-in-one catalog constant, but the PR-level API description introduces a lynxBuiltinCatalog default. Please clarify scope (e.g., which entrypoint exports it) so docs and API don’t conflict.
🤖 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/src/catalog/README.md` around lines 3 - 7, Update the
README note about "no all-in-one catalog" to explicitly state the exception:
mention the `lynxBuiltinCatalog` default introduced at the PR-level and specify
exactly which entrypoint/export exposes it (name the module that exports
`lynxBuiltinCatalog`), and clarify that the package itself still avoids a
top-level aggregate to preserve tree-shaking while offering `lynxBuiltinCatalog`
as an opt-in convenience from that specific entrypoint; update the README text
and the PR-level API description to reference the same export name and import
path so docs and API are consistent.
Collaborator
Author
There was a problem hiding this comment.
Stale — this PR doesn't introduce lynxBuiltinCatalog or any all-in-one aggregate. Consumers compose via defineCatalog([Text, Button, ...]) per-instance. No fix needed.
Contributor
There was a problem hiding this comment.
@PupilTong, thanks for the correction — my comment was mistaken. I incorrectly flagged this based on a mismatch between the PR summary description and the actual code, which does not introduce lynxBuiltinCatalog or any all-in-one aggregate. The README accurately reflects the design. Apologies for the noise.
✏️ Learnings added
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 2536
File: packages/genui/a2ui/src/catalog/README.md:3-7
Timestamp: 2026-05-06T11:05:22.575Z
Learning: In the `lynx-js/a2ui-reactlynx` (packages/genui/a2ui) package, there is no `lynxBuiltinCatalog` all-in-one aggregate export. Consumers compose catalogs per-instance using `defineCatalog([Text, Button, ...])`. The package intentionally avoids a top-level aggregate to preserve tree-shaking.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+77
to
+82
| export interface GenericComponentProps { | ||
| id?: string; | ||
| surface: Surface; | ||
| setValue?: (key: string, value: unknown) => void; | ||
| sendAction?: (action: Record<string, unknown>) => void; | ||
| dataContextPath?: string; |
Contributor
There was a problem hiding this comment.
Tighten sendAction to the actual action contract.
GenericComponentProps.sendAction no longer matches the hook-backed implementation: useAction() sends a v0_9.Action and returns a promise. Keeping this as (Record<string, unknown>) => void weakens type safety for catalog components and hides the async boundary.
Suggested fix
export interface GenericComponentProps {
id?: string;
surface: Surface;
setValue?: (key: string, value: unknown) => void;
- sendAction?: (action: Record<string, unknown>) => void;
+ sendAction?: (action: v0_9.Action) => Promise<unknown>;
dataContextPath?: string;
[key: string]: unknown;
}🤖 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/src/store/types.ts` around lines 77 - 82, Update
GenericComponentProps.sendAction to reflect the real action contract: replace
the loose (Record<string, unknown>) => void signature with the async action type
used by useAction(), i.e. sendAction?: (action: v0_9.Action) => Promise<void>;
and import or reference the v0_9.Action type where GenericComponentProps is
declared so callers and implementers (and catalog components) get proper
type-checking and the async boundary is preserved.
Collaborator
Author
There was a problem hiding this comment.
Intentional — catalog components (the Text, Button, etc. built-ins and any user-supplied custom components) shouldn't have to import @a2ui/web_core/v0_9 directly. The renderer wraps the typed dispatch in useAction() internally and the catalog component receives a loose dispatcher. Tightening here would couple every catalog component to the protocol version.
Contributor
There was a problem hiding this comment.
@PupilTong, that makes sense — keeping GenericComponentProps.sendAction as a loose (action: Record<string, unknown>) => void maintains a clean abstraction boundary between catalog components and the protocol layer. Since useAction() already handles the typed dispatch internally, catalog authors don't need to take a dependency on @a2ui/web_core/v0_9, which would otherwise create an undesirable coupling to a specific protocol version. I'll withdraw this suggestion.
✏️ Learnings added
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 2536
File: packages/genui/a2ui/src/store/types.ts:77-82
Timestamp: 2026-05-06T11:05:14.676Z
Learning: In `packages/genui/a2ui/src/store/types.ts`, `GenericComponentProps.sendAction` is intentionally typed as `(action: Record<string, unknown>) => void` (loose/untyped). Catalog components (Text, Button, etc. and user-supplied custom components) must not import `a2ui/web_core/v0_9` directly; the renderer wraps the typed dispatch in `useAction()` internally and passes a loose dispatcher to catalog components to avoid coupling them to the protocol version. Do not flag this as a type-safety issue.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Huxpro
Repo: lynx-family/lynx-stack PR: 2325
File: packages/rspeedy/create-rspeedy/template-react-ts/src/useFlappy.ts:43-45
Timestamp: 2026-03-10T08:25:14.775Z
Learning: In reviews for the lynx-family/lynx-stack repo, accept both 'background only' and 'background-only' spellings for the SWC directive DCE plugin (swc_plugin_directive_dce/lib.rs matches both). Do not flag 'background only' as incorrect; apply this understanding to all TypeScript/TSX files where the directive spelling may appear.
e9cff8e to
d106bfb
Compare
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
from
d106bfb to
cd8338d
Compare
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
from
cd8338d to
0c2a9ed
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (13)
packages/genui/a2ui-playground/examples/io-mock/mockAgent.ts (1)
49-50: 💤 Low value
stop()is permanently sticky —start()cannot be resumed afterward.The
AbortControlleris created once at module level andstartedis never reset, so afterstop():
start()returns the already-resolved (or aborted-mid-stream)startedpromise without restreaming.onAction()quietly pushes nothing sincestreamIntoearly-exits onabort.signal.aborted.If that's intentional (single-shot driver), the docs on
stop()("Stop streaming and discard any pending messages") should make this terminal contract explicit. Otherwise, recreateabortand clearstartedinsidestop().Also applies to: 64-79
🤖 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-playground/examples/io-mock/mockAgent.ts` around lines 49 - 50, The module-level AbortController (abort) and the started promise are left sticky so after stop() is called start() and onAction()/streamInto cannot resume; update stop() to recreate the AbortController and clear/reset started so the driver can be restarted: inside stop() set abort = new AbortController() (or otherwise reinitialize the abort signal) and set started = null (or an unresolved promise) so subsequent calls to start() will create a fresh stream; ensure references to abort, started, start(), stop(), onAction(), and streamInto are updated accordingly to use the new controller and cleared promise.packages/genui/a2ui-playground/lynx-src/App.tsx (3)
263-281: 💤 Low valueOrder of operations: streaming starts before the cancel check.
agent.start()is invoked at line 271 before theif (cancelled)guard at line 273, so messages may be pushed intonext(and thedelayMstimer started) for a store that will never be installed wheneffectiveDatachanges mid-load. Move the start after the cancellation/install:♻️ Suggested reorder
- const next = createMessageStore(); - const agent = createMockAgent(next, { - initialMessages, - actionMocks, - delayMs: 800, - }); - - // Begin streaming the demo's initial messages into the buffer. - void agent.start(); - - if (cancelled) { - agent.stop(); - return; - } - agentRef.current?.stop(); - storeRef.current = next; - agentRef.current = agent; - setStore(next); + if (cancelled) return; + + const next = createMessageStore(); + const agent = createMockAgent(next, { + initialMessages, + actionMocks, + delayMs: 800, + }); + + agentRef.current?.stop(); + storeRef.current = next; + agentRef.current = agent; + setStore(next); + + // Begin streaming after the agent is owned by the ref so the + // useEffect cleanup can stop it. + void agent.start();🤖 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-playground/lynx-src/App.tsx` around lines 263 - 281, The code currently calls agent.start() before checking cancelled and before installing the new store, which can push messages/timers into next for a store that may never be used; change the order so you create the agent (createMockAgent/next) but do not call agent.start() until after the cancellation guard and after installing the store and refs (call agentRef.current?.stop(), assign storeRef.current = next, agentRef.current = agent, setStore(next)), and only then call agent.start(); ensure that if cancelled is true you call agent.stop() and return without starting it.
255-261: 💤 Low value
actionMocksonly handles array-form payloads — the function-form contract is silently lost.
MockAgentOptions.actionMocksacceptsreadonly ServerToClientMessage[] | ((ctx) => readonly ServerToClientMessage[]), but herevaluefrom the loaded JSON is always passed throughnormalizePayloadToMessages(...). If a user supplies a typedActionMocksobject literal containing functions (e.g., from in-appeffectiveData.actionMocks), each function gets replaced with[]. The current data-loading paths return JSON-only, so this is latent rather than active — worth narrowing the type or detecting and forwarding callable entries.🤖 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-playground/lynx-src/App.tsx` around lines 255 - 261, The loader is converting every rawActionMocks entry through normalizePayloadToMessages, which destroys the function-form contract for MockAgentOptions.actionMocks; update the assignment logic in the block that builds actionMocks so that for each [name, value] from rawActionMocks you detect if value is a callable (function) and if so assign it directly to actionMocks[name], otherwise call normalizePayloadToMessages(value) and assign the resulting ServerToClientMessage[]; reference the actionMocks object, rawActionMocks input, normalizePayloadToMessages helper and the MockAgentOptions.actionMocks contract (and ensure any code that consumes actionMocks still types it to accept readonly ServerToClientMessage[] | ((ctx)=>readonly ServerToClientMessage[])).
148-167: 💤 Low valueLocal
normalizePayloadToMessagesshadows the same-named export from@lynx-js/a2ui-reactlynx.The package now exports a
normalizePayloadToMessages(perpackages/genui/a2ui/src/store/index.ts) with different semantics — it returnsServerToClientMessage[]and falls back to text/Card surfaces, whereas this local helper returns a rawResponseMessagesarray. The naming collision is confusing for readers tracing through the playground; rename the local helper (e.g.extractRawMessages) or import and adopt the canonical one.🤖 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-playground/lynx-src/App.tsx` around lines 148 - 167, The local helper normalizePayloadToMessages conflicts with the exported normalizePayloadToMessages from `@lynx-js/a2ui-reactlynx` and should be disambiguated: either rename the local function (e.g., to extractRawMessages) and update all local calls to that new name, or remove the local implementation and import the canonical normalizePayloadToMessages (which returns ServerToClientMessage[] and handles text/Card surfaces) from the package and adapt call sites to its return type instead of ResponseMessages; ensure references to the local symbol (normalizePayloadToMessages) are updated accordingly and types are adjusted to use ResponseMessages vs ServerToClientMessage where needed.packages/genui/a2ui/src/react/useDataBinding.ts (2)
129-137: 💤 Low valueDead branch in
resolveProperties.The
string | number | booleanarm and the trailingelseboth assignresult[key] = prop, so thetypeofcheck has no effect. Either drop the typeof guard or actually diverge in behavior (e.g., reject objects withoutpath/componentId).♻️ Simplification
- } else if ( - typeof prop === 'string' - || typeof prop === 'number' - || typeof prop === 'boolean' - ) { - result[key] = prop; - } else { - result[key] = prop; - } + } else { + result[key] = prop; + }🤖 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/src/react/useDataBinding.ts` around lines 129 - 137, In resolveProperties the typeof string/number/boolean branch is redundant because both branches set result[key] = prop; replace this with a clear divergence: if prop is a primitive (typeof prop === 'string' || 'number' || 'boolean') set result[key] = prop, else if prop is an object with required fields (e.g., has prop.path or prop.componentId) transform/extract the intended value before assigning to result[key], and otherwise throw or reject invalid objects; update references in resolveProperties (variables result, key, prop) accordingly so objects without path/componentId are handled explicitly instead of falling through to the same assignment.
172-181: 💤 Low value
subscribeperforms a discardedresolvePropertiescall per change.The intent appears to be re-registering signal reads inside the
effectso the listener notifies on any binding change, but the resolved object is thrown away — every notification recomputes the whole map twice (once here, once incomputeSnapshot). For props maps with many bindings this is a noticeable hot path.A lighter pattern is to track the subscribed signals once and call
cb()from eacheffect, or to memoize the resolved object inside the effect and feed it to the snapshot via a ref.🤖 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/src/react/useDataBinding.ts` around lines 172 - 181, The subscribe callback currently calls resolveProperties inside the effect and discards its result, causing duplicate work when computeSnapshot later recomputes; change subscribe to capture/memoize the resolved props inside the effect (or record the set of signals read) and store that result into a ref/shared place used by computeSnapshot, then have the effect only call cb() on changes instead of recomputing the whole map twice. Concretely, update the subscribe implementation (function subscribe) to call resolveProperties once per effect and assign the resolved object to a ref (or track the signal set) tied to properties/surface/dataContextPath, and ensure computeSnapshot reads from that ref rather than re-resolving, while still using effect to register reads and invoke cb().packages/genui/a2ui/src/store/MessageProcessor.ts (3)
321-331: 💤 Low value
messageIdfallbacksurface:${surfaceId}is reused across re-creations of the same surface.If a surface is deleted and later recreated with the same
surfaceId, the synthesizedmessageIdcollides with the prior one. Downstream maps keyed bymessageId(e.g.,session.resourcesinA2UI.tsx) may pick up the staleResourceand never see a freshbeginRendering. Consider mixing in a monotonic counter orrandomId()to keep the fallback unique per surface lifecycle.🤖 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/src/store/MessageProcessor.ts` around lines 321 - 331, The synthesized fallback messageId `surface:${surfaceId}` can collide across surface re-creations; update the fallback logic in MessageProcessor where `messageId` is derived (the block that calls `this.emitUpdate({ type: 'beginRendering', surfaceId, messageId })`) so it mixes in a per-surface monotonic counter or a random suffix (e.g., `surface:${surfaceId}:${counter}` or `surface:${surfaceId}:${randomId()}`) instead of the plain `surface:${surfaceId}`; implement a small Map to track and increment a counter for each surfaceId (or call the existing randomId() helper) to ensure each synthesized messageId is unique across surface lifecycles so downstream maps keyed by messageId (like session.resources) will treat re-created surfaces as new.
44-52: 💤 Low value
dispatchonly honors the first responder when multiple event listeners exist.A single
resolveis shared acrossfor (const cb of this.eventListeners) cb({ message, resolve }). Whichever listener callsresolvefirst wins; later callers' results are silently dropped. If multi-subscriber semantics are intentional foronEvent(the docstring suggests they are now), the contract that only one listener can answer should be made explicit, or the API should expose per-listener responses (e.g.,Promise.race/aggregation). Worth a clarifying comment at minimum.🤖 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/src/store/MessageProcessor.ts` around lines 44 - 52, The current dispatch implementation shares a single resolve across all subscribers so the first listener to call resolve wins and other listeners' results are dropped; update dispatch (and document behavior) so each subscriber gets its own response promise: for each callback in eventListeners create a per-listener promise or wrapper and call cb({ message, resolve: perListenerResolve }) then return an aggregated result (e.g., Promise.all to collect all responses, or Promise.race if you want the first responder) instead of a single shared resolve; refer to the dispatch method and the eventListeners set (and the onEvent contract) when making the change and add a short comment clarifying whether multi-subscriber aggregation or single-responder semantics are intended.
25-42: 💤 Low valueListener-set mutation during broadcast.
Both
emitUpdateand the existingdispatchiterateupdateListeners/eventListenersdirectly. If a callback synchronously calls the disposer it received fromonUpdate/onEvent, you mutate theSetmid-iteration — JS makes that defined-but-subtle (the visited entry is already invoked; later additions in the same tick may or may not run). Snapshotting the listeners before iterating gives deterministic behavior and is cheap given listener counts here.♻️ Suggested change
- private emitUpdate(data: Record<string, unknown>): void { - for (const cb of this.updateListeners) cb(data); - } + private emitUpdate(data: Record<string, unknown>): void { + for (const cb of [...this.updateListeners]) cb(data); + }🤖 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/src/store/MessageProcessor.ts` around lines 25 - 42, emitUpdate (and the dispatch loop over eventListeners) currently iterates the Set directly which can be mutated by callbacks that call the disposer returned by onUpdate/onEvent; snapshot the listeners before iterating to avoid mid-iteration mutation: in emitUpdate take const listeners = Array.from(this.updateListeners) and iterate that array, and do the same in dispatch for this.eventListeners (and any similar emitter methods), leaving onUpdate/onEvent and their disposer logic unchanged.packages/genui/a2ui/src/store/index.ts (1)
8-9: 💤 Low valueTwo
Resourcesymbols re-exported from one barrel —RawResourcevsResource.
Resource.jsships aResource(renamed toRawResource) whiletypes.jsexposes a differentResource(the user-facing facade type). Consumers importing from@lynx-js/a2ui-reactlynxwill see both names, and the relationship is non-obvious from the export list alone. A short JSDoc above each export, or a single naming convention (e.g.,RawResource↔Resourcedocumented inline), would help downstream readers tell them apart.Also applies to: 17-27
🤖 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/src/store/index.ts` around lines 8 - 9, The barrel currently re-exports two different Resource symbols which is confusing; update the export lines for createResource, RawResource (the internal Resource from './Resource.js') and ResourceStatus to include short JSDoc comments that explicitly state that RawResource is the low-level/internal shape and that the user-facing Resource type is exported from the separate types module (the other Resource), or alternatively unify naming by exporting the internal type as RawResource and documenting that the public Resource facade comes from './types.js'; include the symbols createResource, RawResource, ResourceStatus and Resource in the JSDoc so readers can immediately see the distinction.packages/genui/a2ui/src/store/payloadNormalizer.ts (1)
158-199: 💤 Low valueMutating caller-supplied messages via
msg.messageId ??=— and a heavy type-assertion churn.Two related observations:
- Line 193 mutates the caller-supplied
ServerToClientMessageobjects in place. This is fine for fresh transport buffers, butMessageStore.pushproduces frozen arrays whose element references are shared across snapshots — once tagged, replays/rehydrations carry the original processing'smessageId. Worth a doc note on the contract or producing tagged copies.- The block at lines 180-188 repeats the same
(msg as { updateComponents?: { components?: unknown[] } })cast four times for one boolean check; a single narrowed local variable would make it readable.♻️ Suggested simplification for the readability portion
- if ( - ((msg as { updateComponents?: { components?: unknown[] } }) - .updateComponents - && Array.isArray( - (msg as { updateComponents?: { components?: unknown[] } }) - .updateComponents?.components, - ) - && (((msg as { updateComponents?: { components: unknown[] } }) - .updateComponents?.components ?? []).length > 0)) - ) { - hasComponentUpdate = true; - } + const components = (msg as { updateComponents?: { components?: unknown[] } }) + .updateComponents?.components; + if (Array.isArray(components) && components.length > 0) { + hasComponentUpdate = true; + }🤖 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/src/store/payloadNormalizer.ts` around lines 158 - 199, prepareMessagesForProcessing currently mutates caller-supplied ServerToClientMessage objects by using the in-place nullish-assignment msg.messageId ??= messageId and also repeats verbose type assertions when checking updateComponents; instead, avoid mutating inputs by creating shallow copies before tagging (e.g., clone msg into a local variable and set messageId on the copy) so frozen messages from MessageStore.push are not modified, and simplify the updateComponents check by extracting a single narrowed local like const uc = (msg as { updateComponents?: { components?: unknown[] } }).updateComponents and then use uc && Array.isArray(uc.components) && (uc.components.length > 0) to set hasComponentUpdate; update the function prepareMessagesForProcessing to operate on the copies and return those copies in messages.packages/genui/a2ui/src/store/MessageStore.ts (1)
68-75: 💤 Low valueOptional: subscriber-set mutation during
notify().
notify()iteratessubscribersdirectly. If a callback callssubscribe()/the returned unsubscribe during the iteration (not unusual under React 18useSyncExternalStorein concurrent mode), you'll be mutating aSetmid-iteration. JS makes this defined-but-subtle (added entries may or may not be visited; deleted entries already visited still fire). Snapshotting before iterating avoids the surprise and keeps semantics deterministic.♻️ Suggested change
const notify = () => { - for (const cb of subscribers) cb(); + for (const cb of [...subscribers]) cb(); };🤖 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/src/store/MessageStore.ts` around lines 68 - 75, The notify() call currently iterates the subscribers Set directly which can be mutated if a subscriber calls subscribe()/the returned unsubscribe during iteration; update notify() to iterate over a shallow snapshot of the Set (e.g., Array.from(subscribers) or [...subscribers]) so additions/removals during notify are deterministic and won't affect the current iteration; locate the notify function and change its iteration to use the snapshot approach, and ensure subscribe/unsubscribe semantics (the function that returns the unsubscribe) remain unchanged.packages/genui/a2ui/src/react/A2UIProvider.tsx (1)
34-37: 💤 Low valueRedundant inline import type — use the already-imported
ReactNode.Line 36 uses
import('@lynx-js/react').ReactNodewhenReactNodeis already imported as a type alias on line 5.♻️ Proposed fix
export function A2UIProvider( props: ProviderProps, -): import('@lynx-js/react').ReactNode { +): ReactNode {🤖 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/src/react/A2UIProvider.tsx` around lines 34 - 37, The function signature for A2UIProvider currently uses an inline import type import('@lynx-js/react').ReactNode; replace that with the already-imported ReactNode type (used alongside ProviderProps) so the return type reads simply ReactNode; update the A2UIProvider function declaration to use ReactNode instead of the inline import to remove redundancy and rely on the existing import.
🤖 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-playground/package.json`:
- Line 9: The dev script ("dev" in package.json) no longer ensures the Lynx
bundle exists which lets rsbuild.config.ts still advertise /__rspeedy_url ->
main.lynx.js while lynx.config.ts is responsible for writing that file; update
the dev startup to either run the Lynx build step before rsbuild dev
(reintroduce a predev or prestart that runs the existing "build:lynx" script) or
change rsbuild.config.ts to stop advertising main.lynx.js when the bundle is
missing; locate the "dev" script in package.json and the "build:lynx" task, and
modify package.json to run build:lynx as a pre-step or adjust rsbuild.config.ts
to check for the existence of main.lynx.js before exposing /__rspeedy_url.
In `@packages/genui/a2ui-playground/rsbuild.config.ts`:
- Around line 108-123: Before reading payloadStore in the GET /__a2ui/...
handler, invoke gcPayloads() so expired entries are removed first; if gcPayloads
is async await it, then proceed to use payloadStore.get(id) and sendJson as
before (refer to gcPayloads, payloadStore.get, sendJson, and the GET branch
checking req.method and url.startsWith('/__a2ui/')).
In `@packages/genui/a2ui/src/catalog/index.ts`:
- Around line 20-32: The barrel currently re-exports many components but omits
the helper defineCatalogEntry; add a re-export for defineCatalogEntry from the
same module that exports
defineCatalog/mergeCatalogs/resolveCatalog/serializeCatalog (i.e., add an export
line exporting the symbol defineCatalogEntry from the module that provides the
other catalog utilities) so consumers importing from this catalog index can
access defineCatalogEntry via the canonical entrypoint.
In `@packages/genui/a2ui/src/catalog/README.md`:
- Around line 15-35: The README quick-start still uses createMessageStore() and
onAction; update the example to demonstrate the new createMessageStore({ send })
API and show wiring actions through the store instead of onAction. Replace the
createMessageStore() call with createMessageStore({ send: async (action) => { /*
forward to agent and push response via store.push(...) */ } }), remove or no-op
the A2UI onAction prop, and update the comment around streaming to mention using
store.push for incoming protocol messages and store.send (or the provided send)
for outgoing user actions so readers see the new send boundary and action flow.
In `@packages/genui/a2ui/src/react/A2UIRenderer.tsx`:
- Around line 48-60: The spread {...resolvedProps} is placed after internal
plumbing props in buildNodeRecursive so server-provided keys can silently
overwrite plumbing (e.g., surface, setValue, sendAction, id, dataContextPath);
move the spread before the explicit props in the JSX returned by Component so
resolvedProps is applied first and the renderer's explicit props (id, surface,
setValue, sendAction, dataContextPath) remain authoritative, ensuring sendAction
and other internal handlers passed to Component cannot be overwritten by
resolvedProps.
In `@packages/genui/a2ui/src/react/useAction.ts`:
- Around line 67-69: The Array branch in useAction.ts currently returns elements
with an identity map (value.map((v) => v)) so nested DataBinding or FunctionCall
items are not resolved; replace that identity mapping with a recursive call to
resolveDynamicValue for each element (i.e., value.map((v) =>
resolveDynamicValue(v, /* same context/params used elsewhere */))) so array
elements are handled the same way as object properties and event context; ensure
you pass the same context/args to resolveDynamicValue as used around line 93 and
line 136 so FunctionCall/DataBinding inside arrays get resolved.
In `@packages/genui/a2ui/src/store/payloadNormalizer.ts`:
- Around line 93-100: The add function currently uses a truthy guard that drops
valid falsy primitives; update add so it only treats null/undefined as no-ops
(e.g., change the guard from a truthy check to a nullish check like value ==
null), keep the Array.isArray recursion and the messages.push(value as
ServerToClientMessage) behavior, and ensure this aligns with any changes in
handle() that stringify primitives so legitimate values 0, false, and '' are
preserved.
---
Nitpick comments:
In `@packages/genui/a2ui-playground/examples/io-mock/mockAgent.ts`:
- Around line 49-50: The module-level AbortController (abort) and the started
promise are left sticky so after stop() is called start() and
onAction()/streamInto cannot resume; update stop() to recreate the
AbortController and clear/reset started so the driver can be restarted: inside
stop() set abort = new AbortController() (or otherwise reinitialize the abort
signal) and set started = null (or an unresolved promise) so subsequent calls to
start() will create a fresh stream; ensure references to abort, started,
start(), stop(), onAction(), and streamInto are updated accordingly to use the
new controller and cleared promise.
In `@packages/genui/a2ui-playground/lynx-src/App.tsx`:
- Around line 263-281: The code currently calls agent.start() before checking
cancelled and before installing the new store, which can push messages/timers
into next for a store that may never be used; change the order so you create the
agent (createMockAgent/next) but do not call agent.start() until after the
cancellation guard and after installing the store and refs (call
agentRef.current?.stop(), assign storeRef.current = next, agentRef.current =
agent, setStore(next)), and only then call agent.start(); ensure that if
cancelled is true you call agent.stop() and return without starting it.
- Around line 255-261: The loader is converting every rawActionMocks entry
through normalizePayloadToMessages, which destroys the function-form contract
for MockAgentOptions.actionMocks; update the assignment logic in the block that
builds actionMocks so that for each [name, value] from rawActionMocks you detect
if value is a callable (function) and if so assign it directly to
actionMocks[name], otherwise call normalizePayloadToMessages(value) and assign
the resulting ServerToClientMessage[]; reference the actionMocks object,
rawActionMocks input, normalizePayloadToMessages helper and the
MockAgentOptions.actionMocks contract (and ensure any code that consumes
actionMocks still types it to accept readonly ServerToClientMessage[] |
((ctx)=>readonly ServerToClientMessage[])).
- Around line 148-167: The local helper normalizePayloadToMessages conflicts
with the exported normalizePayloadToMessages from `@lynx-js/a2ui-reactlynx` and
should be disambiguated: either rename the local function (e.g., to
extractRawMessages) and update all local calls to that new name, or remove the
local implementation and import the canonical normalizePayloadToMessages (which
returns ServerToClientMessage[] and handles text/Card surfaces) from the package
and adapt call sites to its return type instead of ResponseMessages; ensure
references to the local symbol (normalizePayloadToMessages) are updated
accordingly and types are adjusted to use ResponseMessages vs
ServerToClientMessage where needed.
In `@packages/genui/a2ui/src/react/A2UIProvider.tsx`:
- Around line 34-37: The function signature for A2UIProvider currently uses an
inline import type import('@lynx-js/react').ReactNode; replace that with the
already-imported ReactNode type (used alongside ProviderProps) so the return
type reads simply ReactNode; update the A2UIProvider function declaration to use
ReactNode instead of the inline import to remove redundancy and rely on the
existing import.
In `@packages/genui/a2ui/src/react/useDataBinding.ts`:
- Around line 129-137: In resolveProperties the typeof string/number/boolean
branch is redundant because both branches set result[key] = prop; replace this
with a clear divergence: if prop is a primitive (typeof prop === 'string' ||
'number' || 'boolean') set result[key] = prop, else if prop is an object with
required fields (e.g., has prop.path or prop.componentId) transform/extract the
intended value before assigning to result[key], and otherwise throw or reject
invalid objects; update references in resolveProperties (variables result, key,
prop) accordingly so objects without path/componentId are handled explicitly
instead of falling through to the same assignment.
- Around line 172-181: The subscribe callback currently calls resolveProperties
inside the effect and discards its result, causing duplicate work when
computeSnapshot later recomputes; change subscribe to capture/memoize the
resolved props inside the effect (or record the set of signals read) and store
that result into a ref/shared place used by computeSnapshot, then have the
effect only call cb() on changes instead of recomputing the whole map twice.
Concretely, update the subscribe implementation (function subscribe) to call
resolveProperties once per effect and assign the resolved object to a ref (or
track the signal set) tied to properties/surface/dataContextPath, and ensure
computeSnapshot reads from that ref rather than re-resolving, while still using
effect to register reads and invoke cb().
In `@packages/genui/a2ui/src/store/index.ts`:
- Around line 8-9: The barrel currently re-exports two different Resource
symbols which is confusing; update the export lines for createResource,
RawResource (the internal Resource from './Resource.js') and ResourceStatus to
include short JSDoc comments that explicitly state that RawResource is the
low-level/internal shape and that the user-facing Resource type is exported from
the separate types module (the other Resource), or alternatively unify naming by
exporting the internal type as RawResource and documenting that the public
Resource facade comes from './types.js'; include the symbols createResource,
RawResource, ResourceStatus and Resource in the JSDoc so readers can immediately
see the distinction.
In `@packages/genui/a2ui/src/store/MessageProcessor.ts`:
- Around line 321-331: The synthesized fallback messageId `surface:${surfaceId}`
can collide across surface re-creations; update the fallback logic in
MessageProcessor where `messageId` is derived (the block that calls
`this.emitUpdate({ type: 'beginRendering', surfaceId, messageId })`) so it mixes
in a per-surface monotonic counter or a random suffix (e.g.,
`surface:${surfaceId}:${counter}` or `surface:${surfaceId}:${randomId()}`)
instead of the plain `surface:${surfaceId}`; implement a small Map to track and
increment a counter for each surfaceId (or call the existing randomId() helper)
to ensure each synthesized messageId is unique across surface lifecycles so
downstream maps keyed by messageId (like session.resources) will treat
re-created surfaces as new.
- Around line 44-52: The current dispatch implementation shares a single resolve
across all subscribers so the first listener to call resolve wins and other
listeners' results are dropped; update dispatch (and document behavior) so each
subscriber gets its own response promise: for each callback in eventListeners
create a per-listener promise or wrapper and call cb({ message, resolve:
perListenerResolve }) then return an aggregated result (e.g., Promise.all to
collect all responses, or Promise.race if you want the first responder) instead
of a single shared resolve; refer to the dispatch method and the eventListeners
set (and the onEvent contract) when making the change and add a short comment
clarifying whether multi-subscriber aggregation or single-responder semantics
are intended.
- Around line 25-42: emitUpdate (and the dispatch loop over eventListeners)
currently iterates the Set directly which can be mutated by callbacks that call
the disposer returned by onUpdate/onEvent; snapshot the listeners before
iterating to avoid mid-iteration mutation: in emitUpdate take const listeners =
Array.from(this.updateListeners) and iterate that array, and do the same in
dispatch for this.eventListeners (and any similar emitter methods), leaving
onUpdate/onEvent and their disposer logic unchanged.
In `@packages/genui/a2ui/src/store/MessageStore.ts`:
- Around line 68-75: The notify() call currently iterates the subscribers Set
directly which can be mutated if a subscriber calls subscribe()/the returned
unsubscribe during iteration; update notify() to iterate over a shallow snapshot
of the Set (e.g., Array.from(subscribers) or [...subscribers]) so
additions/removals during notify are deterministic and won't affect the current
iteration; locate the notify function and change its iteration to use the
snapshot approach, and ensure subscribe/unsubscribe semantics (the function that
returns the unsubscribe) remain unchanged.
In `@packages/genui/a2ui/src/store/payloadNormalizer.ts`:
- Around line 158-199: prepareMessagesForProcessing currently mutates
caller-supplied ServerToClientMessage objects by using the in-place
nullish-assignment msg.messageId ??= messageId and also repeats verbose type
assertions when checking updateComponents; instead, avoid mutating inputs by
creating shallow copies before tagging (e.g., clone msg into a local variable
and set messageId on the copy) so frozen messages from MessageStore.push are not
modified, and simplify the updateComponents check by extracting a single
narrowed local like const uc = (msg as { updateComponents?: { components?:
unknown[] } }).updateComponents and then use uc && Array.isArray(uc.components)
&& (uc.components.length > 0) to set hasComponentUpdate; update the function
prepareMessagesForProcessing to operate on the copies and return those copies in
messages.
🪄 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: 4eb6f8d7-2eb8-4828-b461-a5dfe90bf110
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (65)
.changeset/quirky-merkle-web-core-fetch.mdpackages/genui/a2ui-playground/examples/README.mdpackages/genui/a2ui-playground/examples/io-mock/mockAgent.tspackages/genui/a2ui-playground/examples/io-sse/sseAgent.tspackages/genui/a2ui-playground/lynx-src/App.tsxpackages/genui/a2ui-playground/lynx-src/tsconfig.jsonpackages/genui/a2ui-playground/lynx.config.tspackages/genui/a2ui-playground/package.jsonpackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui-playground/turbo.jsonpackages/genui/a2ui/package.jsonpackages/genui/a2ui/rstest.config.tspackages/genui/a2ui/src/catalog/Button.tspackages/genui/a2ui/src/catalog/Card.tspackages/genui/a2ui/src/catalog/CheckBox.tspackages/genui/a2ui/src/catalog/Column.tspackages/genui/a2ui/src/catalog/Divider.tspackages/genui/a2ui/src/catalog/Image.tspackages/genui/a2ui/src/catalog/List.tspackages/genui/a2ui/src/catalog/README.mdpackages/genui/a2ui/src/catalog/RadioGroup.tspackages/genui/a2ui/src/catalog/Row.tspackages/genui/a2ui/src/catalog/Text.tspackages/genui/a2ui/src/catalog/all.tspackages/genui/a2ui/src/catalog/index.tspackages/genui/a2ui/src/chat/Conversation.tsxpackages/genui/a2ui/src/chat/index.tspackages/genui/a2ui/src/chat/useLynxClient.tspackages/genui/a2ui/src/core/A2UIRender.tsxpackages/genui/a2ui/src/core/BaseClient.tspackages/genui/a2ui/src/core/ComponentRegistry.tspackages/genui/a2ui/src/core/index.tspackages/genui/a2ui/src/core/types.tspackages/genui/a2ui/src/core/useAction.tspackages/genui/a2ui/src/core/useDataBinding.tspackages/genui/a2ui/src/index.tspackages/genui/a2ui/src/react/A2UI.tsxpackages/genui/a2ui/src/react/A2UIProvider.tsxpackages/genui/a2ui/src/react/A2UIRenderer.tsxpackages/genui/a2ui/src/react/index.tspackages/genui/a2ui/src/react/useA2UIContext.tspackages/genui/a2ui/src/react/useAction.tspackages/genui/a2ui/src/react/useCatalog.tspackages/genui/a2ui/src/react/useDataBinding.tspackages/genui/a2ui/src/store/MessageProcessor.tspackages/genui/a2ui/src/store/MessageStore.tspackages/genui/a2ui/src/store/Resource.tspackages/genui/a2ui/src/store/SignalStore.tspackages/genui/a2ui/src/store/index.tspackages/genui/a2ui/src/store/payloadNormalizer.tspackages/genui/a2ui/src/store/types.tspackages/genui/a2ui/src/utils/ComponentRegistry.tspackages/genui/a2ui/src/utils/createResource.tspackages/genui/a2ui/src/utils/index.tspackages/genui/a2ui/test/catalog.test.tspackages/genui/a2ui/test/createResource.test.tspackages/genui/a2ui/test/messageStore.test.tspackages/genui/a2ui/test/payloadNormalizer.test.tspackages/genui/a2ui/test/processor.test.tspackages/genui/a2ui/tsconfig.build.jsonpackages/genui/a2ui/tsconfig.jsonpackages/genui/tsconfig.jsonpackages/rspeedy/core/turbo.jsonpackages/web-platform/web-core/ts/client/background/background-apis/createChunkLoading.tspackages/web-platform/web-core/ts/types/BTSChunk.ts
💤 Files with no reviewable changes (24)
- packages/genui/a2ui/src/catalog/Card.ts
- packages/genui/a2ui/src/chat/Conversation.tsx
- packages/genui/a2ui/src/core/types.ts
- packages/genui/a2ui/src/core/BaseClient.ts
- packages/genui/a2ui/src/chat/index.ts
- packages/genui/a2ui/src/core/useAction.ts
- packages/genui/a2ui/src/core/ComponentRegistry.ts
- packages/genui/a2ui/src/catalog/RadioGroup.ts
- packages/genui/a2ui/src/utils/createResource.ts
- packages/genui/a2ui/src/catalog/List.ts
- packages/genui/a2ui/src/utils/index.ts
- packages/genui/a2ui/src/catalog/Divider.ts
- packages/genui/a2ui/src/core/A2UIRender.tsx
- packages/genui/a2ui/src/catalog/Text.ts
- packages/genui/a2ui/src/utils/ComponentRegistry.ts
- packages/genui/a2ui/src/catalog/Image.ts
- packages/genui/a2ui/src/chat/useLynxClient.ts
- packages/genui/a2ui/src/catalog/Button.ts
- packages/genui/a2ui/src/catalog/Row.ts
- packages/genui/a2ui/src/catalog/all.ts
- packages/genui/a2ui/src/catalog/CheckBox.ts
- packages/genui/a2ui/src/core/useDataBinding.ts
- packages/genui/a2ui/src/catalog/Column.ts
- packages/genui/a2ui/src/core/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/genui/a2ui/src/react/index.ts
- packages/genui/a2ui/package.json
Comment on lines
+20
to
+32
| // Per-component re-exports so consumers can pick exactly what they need. | ||
| // Each is an independently tree-shakeable ESM re-export — pulling `Text` | ||
| // does not drag `Button` into the bundle. | ||
| export { Button } from './Button/index.jsx'; | ||
| export { Card } from './Card/index.jsx'; | ||
| export { CheckBox } from './CheckBox/index.jsx'; | ||
| export { Column } from './Column/index.jsx'; | ||
| export { Divider } from './Divider/index.jsx'; | ||
| export { Image } from './Image/index.jsx'; | ||
| export { List } from './List/index.jsx'; | ||
| export { RadioGroup } from './RadioGroup/index.jsx'; | ||
| export { Row } from './Row/index.jsx'; | ||
| export { Text } from './Text/index.jsx'; |
Contributor
There was a problem hiding this comment.
Re-export defineCatalogEntry from this barrel as well.
The PR-level API exposes defineCatalogEntry, but this catalog barrel still only forwards defineCatalog/mergeCatalogs/resolveCatalog/serializeCatalog plus the built-ins. Consumers using the canonical catalog entrypoint will miss that helper.
Suggested fix
export {
defineCatalog,
+ defineCatalogEntry,
mergeCatalogs,
resolveCatalog,
serializeCatalog,
} from './defineCatalog.js';🤖 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/src/catalog/index.ts` around lines 20 - 32, The barrel
currently re-exports many components but omits the helper defineCatalogEntry;
add a re-export for defineCatalogEntry from the same module that exports
defineCatalog/mergeCatalogs/resolveCatalog/serializeCatalog (i.e., add an export
line exporting the symbol defineCatalogEntry from the module that provides the
other catalog utilities) so consumers importing from this catalog index can
access defineCatalogEntry via the canonical entrypoint.
Collaborator
Author
There was a problem hiding this comment.
defineCatalogEntry doesn't exist in this PR. The public API is defineCatalog / mergeCatalogs / serializeCatalog / resolveCatalog plus the built-ins, all of which are already re-exported here. No fix needed.
Contributor
There was a problem hiding this comment.
@PupilTong, you're right — my apologies for the false positive. defineCatalogEntry does not exist in this PR, and the barrel already exports the full public API (defineCatalog, mergeCatalogs, resolveCatalog, serializeCatalog, and all the built-in components). No change needed here.
✏️ Learnings added
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 2536
File: packages/genui/a2ui/src/catalog/index.ts:20-32
Timestamp: 2026-05-06T11:05:38.628Z
Learning: In `lynx-family/lynx-stack`, the `lynx-js/a2ui` catalog public API (as of PR `#2536`) consists of `defineCatalog`, `mergeCatalogs`, `resolveCatalog`, `serializeCatalog`, and the built-in component re-exports (Button, Card, CheckBox, Column, Divider, Image, List, RadioGroup, Row, Text). There is no `defineCatalogEntry` helper in this package — do not flag its absence as an issue.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Huxpro
Repo: lynx-family/lynx-stack PR: 2325
File: packages/rspeedy/create-rspeedy/template-react-ts/src/useFlappy.ts:43-45
Timestamp: 2026-03-10T08:25:14.775Z
Learning: In reviews for the lynx-family/lynx-stack repo, accept both 'background only' and 'background-only' spellings for the SWC directive DCE plugin (swc_plugin_directive_dce/lib.rs matches both). Do not flag 'background only' as incorrect; apply this understanding to all TypeScript/TSX files where the directive spelling may appear.
Comment on lines
+15
to
+35
| import { | ||
| A2UI, | ||
| Text, | ||
| Button, | ||
| createMessageStore, | ||
| } from '@lynx-js/a2ui-reactlynx'; | ||
|
|
||
| const store = createMessageStore(); | ||
|
|
||
| // Push raw protocol messages from your IO module (fetch, SSE, ...). | ||
| // async function streamFromAgent(input) { | ||
| // for await (const msg of myAgent.stream(input)) store.push(msg); | ||
| // } | ||
|
|
||
| const catalog = defineCatalog([Text, Button]); | ||
| <A2UI | ||
| messageStore={store} | ||
| catalogs={[Text, Button]} | ||
| onAction={(action) => { | ||
| /* forward to your agent and push response messages back */ | ||
| }} | ||
| />; |
Contributor
There was a problem hiding this comment.
The quick-start still shows the pre-MessageStore calling pattern.
This PR introduces createMessageStore({ send }), but the example still uses createMessageStore() with no send implementation and pushes action handling onto an onAction prop. As written, readers won't see the new send boundary or how user actions flow back through the store.
Suggested fix
-const store = createMessageStore();
+const store = createMessageStore({
+ send: async (event) => {
+ /* forward to your agent and push response messages back */
+ },
+});
-<A2UI
- messageStore={store}
- catalogs={[Text, Button]}
- onAction={(action) => {
- /* forward to your agent and push response messages back */
- }}
-/>;
+<A2UI
+ messageStore={store}
+ catalogs={[Text, Button]}
+/>;🤖 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/src/catalog/README.md` around lines 15 - 35, The README
quick-start still uses createMessageStore() and onAction; update the example to
demonstrate the new createMessageStore({ send }) API and show wiring actions
through the store instead of onAction. Replace the createMessageStore() call
with createMessageStore({ send: async (action) => { /* forward to agent and push
response via store.push(...) */ } }), remove or no-op the A2UI onAction prop,
and update the comment around streaming to mention using store.push for incoming
protocol messages and store.send (or the provided send) for outgoing user
actions so readers see the new send boundary and action flow.
Collaborator
Author
There was a problem hiding this comment.
createMessageStore is a no-arg dumb buffer in this PR — there's no send option. The README example matches the actual API: developers push raw protocol messages into the store from their IO module, and user actions flow back through the onAction prop. No fix needed.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual MessageStore implementation to verify the API
rg -n "createMessageStore|MessageStoreOptions|send" --type ts packages/genui/a2ui/src/store/MessageStore.ts
Length of output: 121
🏁 Script executed:
#!/bin/bash
cat packages/genui/a2ui/src/store/MessageStore.ts
Length of output: 2828
@PupilTong, you're absolutely right — I apologise for the noise. Looking at the implementation, MessageStoreOptions only has initialMessages; there is no send callback. The store is a pure append-only buffer (subscribe / getSnapshot / push / clear) and the README example accurately reflects that: raw protocol messages are pushed into the store from the IO module, and user actions flow out through the onAction prop on <A2UI>. No changes needed.
✏️ Learnings added
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 2536
File: packages/genui/a2ui/src/catalog/README.md:15-35
Timestamp: 2026-05-06T11:05:57.304Z
Learning: In `packages/genui/a2ui/src/store/MessageStore.ts`, `createMessageStore` is a no-arg (or `{ initialMessages? }` only) dumb append-only buffer. It has no `send` option. The `MessageStore` interface exposes only `subscribe`, `getSnapshot`, `push`, and `clear`. User actions flow back through the `onAction` prop on `<A2UI>`, not through the store. The README at `packages/genui/a2ui/src/catalog/README.md` correctly documents this pattern.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
from
0c2a9ed to
5932ae7
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/genui/a2ui/src/react/A2UI.tsx (1)
154-161:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't materialize a new surface in the
deleteSurfacepath.Line 161 calls
getOrCreateSurface(surfaceId)before the branch check. If the processor has already removed that surface, thedeleteSurfacebranch runs against a fresh emptySurface, so the root resource never gets completed and the old tree can linger.Please verify whether the delete notification is emitted after the surface is removed from
MessageProcessor:#!/bin/bash set -euo pipefail file="$(fd '^MessageProcessor\.ts$' packages/genui/a2ui/src/store | head -n1)" echo "Inspecting: $file" rg -n -C4 "deleteSurface|surfaces\\.delete|targetId|emitUpdate|onUpdate" "$file"If that ordering is confirmed, this branch should stop calling
getOrCreateSurface()up front and instead complete the resource from the pre-delete surface reference (or haveMessageProcessorinclude that reference in the event payload).Also applies to: 185-194
🤖 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/src/react/A2UI.tsx` around lines 154 - 161, The offUpdate handler currently calls proc.getOrCreateSurface(surfaceId) before checking for a deleteSurface update, which can materialize a fresh Surface when the processor already removed it; modify the offUpdate callback so it does not call getOrCreateSurface() up-front—inspect the incoming data.type (and targetId/messageId) first, and if type signals deletion use the pre-delete surface reference provided by MessageProcessor (or request that MessageProcessor include the Surface reference in the event payload) to complete/cleanup the root resource; update the logic in the offUpdate handler (the proc.onUpdate callback) and similarly adjust the other occurrences mentioned (around the block covering lines ~185-194) so deleteSurface paths never create a new Surface via getOrCreateSurface().
🧹 Nitpick comments (5)
packages/genui/a2ui-playground/rsbuild.config.ts (1)
66-69: ⚡ Quick winUse exact route matching instead of broad
startsWithchecks.
Line 68andLine 78currently match any prefixed path (for example/__a2ui_payload_extra). Tightening these checks avoids accidental endpoint interception and future route conflicts.Suggested fix
+function isA2uiPayloadPath(url: string): boolean { + return /^\/__a2ui_payload(?:\?|$)/.test(url); +} + if ( req.method === 'OPTIONS' - && (url.startsWith('/__a2ui_payload') || url.startsWith('/__a2ui/')) + && (isA2uiPayloadPath(url) || url.startsWith('/__a2ui/')) ) { @@ - if (req.method === 'POST' && url.startsWith('/__a2ui_payload')) { + if (req.method === 'POST' && isA2uiPayloadPath(url)) {Also applies to: 78-79
🤖 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-playground/rsbuild.config.ts` around lines 66 - 69, Replace the loose startsWith checks with exact route comparisons so only the intended endpoints are matched: in the conditional that checks req.method === 'OPTIONS' and uses url.startsWith('/__a2ui_payload') || url.startsWith('/__a2ui/'), change those to strict comparisons (e.g. url === '/__a2ui_payload' and url === '/__a2ui/' or url === '/__a2ui' depending on the canonical route) and apply the same change to the second occurrence later in the file; update the checks around the req.method/url logic in rsbuild.config.ts to use these exact string matches instead of startsWith.packages/genui/a2ui-playground/examples/io-mock/mockAgent.ts (1)
86-91: ⚡ Quick winConcurrent
onActioncalls can interleave messages in the store.Two overlapping
onActioninvocations both enterstreamIntoindependently; there is no queue or mutex, so their messages can arrive in an arbitrary interleaved order. For a demo this is probably fine, but if any test asserts deterministic message ordering across sequential actions it will be flaky.A serial queue (e.g. chain each call onto a shared
Promise) removes the ambiguity:💡 Proposed approach
- let started: Promise<void> | null = null; + let started: Promise<void> | null = null; + let tail: Promise<void> = Promise.resolve(); return { ... async onAction(action) { const mock = actionMocks[action.name]; if (!mock) return; const stream = typeof mock === 'function' ? mock(action) : mock; - await streamInto(stream); + tail = tail.then(() => streamInto(stream)); + await tail; }, };🤖 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-playground/examples/io-mock/mockAgent.ts` around lines 86 - 91, The onAction handler can run concurrently and cause interleaved messages because multiple calls call streamInto directly; fix by serializing calls with a shared promise queue: add a module-level pending promise (e.g., pendingActionPromise = Promise.resolve()) and in onAction chain the work onto it (pendingActionPromise = pendingActionPromise.then(() => { const stream = typeof mock === 'function' ? mock(action) : mock; return streamInto(stream); }).catch(() => {})), returning the chained promise so each invocation waits for prior ones to finish; update references to onAction and streamInto accordingly and ensure errors are swallowed or logged so the chain continues.packages/genui/a2ui/src/store/Resource.ts (1)
70-75: ⚡ Quick winNotify is not resilient to listener exceptions or re-entrant subscribe/unsubscribe.
for (const fn of listeners) fn()has two practical hazards:
- If any single subscriber throws (e.g. a React internal/store consumer in development), the loop aborts, so remaining subscribers — including the
useSyncExternalStorere-render scheduler — never see this transition. TheResource's state has already been mutated, so observers go permanently out of sync until the nextcomplete/fail.- Iterating the live
Setwhile a listener disposes/adds another listener (re-entrantsubscribe) is technically defined in JS, but it makes ordering subtle and means a listener that subscribes during notify gets its first callback for the current transition.Snapshot the listeners and isolate failures so one bad subscriber can't desync the rest:
♻️ Proposed fix
const notify = () => { snapshot = { status, value, error }; - for (const fn of listeners) fn(); + // Snapshot the listener list so subscribe/unsubscribe inside a callback + // doesn't affect this dispatch, and isolate failures so one throwing + // subscriber can't desync the rest. + for (const fn of [...listeners]) { + try { + fn(); + } catch (err) { + // eslint-disable-next-line no-console + console.error('[a2ui] Resource listener threw:', err); + } + } };🤖 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/src/store/Resource.ts` around lines 70 - 75, The notify function must avoid iterating the live Set and let one subscriber's exception abort the rest; change notify (the function referencing listeners, snapshot, status, value, error) to first update snapshot as now, then create a stable copy of listeners (e.g. Array.from(listeners)) and iterate that copy, wrapping each fn() call in a try/catch so a thrown error from one subscriber doesn't stop others (this also prevents re-entrant subscribe/unsubscribe from affecting the current delivery and ensures useSyncExternalStore's scheduler always runs).packages/genui/a2ui/src/react/useAction.ts (1)
128-172: 💤 Low valueHoist
makeResolversout of the dispatch path.
makeResolvers(processor)is invoked on everysendActioncall, rebuilding three closures per dispatch even though it depends only onprocessor. Wrapping it inuseMemo(or moving the call to module scope and threadingprocessorper call) avoids the per-invocation allocations and matches the memoization story called out in the PR summary foruseAction/ActionProps.♻️ Proposed refactor
-import { useCallback } from '@lynx-js/react'; +import { useCallback, useMemo } from '@lynx-js/react'; @@ const { id, surfaceId, dataContext } = props; const { processor } = useA2UIContext(); + const { resolveDynamicValue, resolveFunctionCall } = useMemo( + () => makeResolvers(processor), + [processor], + ); const sendAction = useCallback( (action: v0_9.Action) => { let name = 'unknownAction'; let context: Record<string, unknown> = {}; - const { resolveDynamicValue, resolveFunctionCall } = makeResolvers( - processor, - ); @@ - [id, surfaceId, dataContext, processor], + [id, surfaceId, dataContext, processor, resolveDynamicValue, resolveFunctionCall], );🤖 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/src/react/useAction.ts` around lines 128 - 172, The sendAction callback in useAction repeatedly calls makeResolvers(processor) on every dispatch, reallocating closures (resolveDynamicValue, resolveFunctionCall) unnecessarily; hoist makeResolvers out of the dispatch path by creating the resolvers once with useMemo (e.g., const { resolveDynamicValue, resolveFunctionCall } = useMemo(() => makeResolvers(processor), [processor])) at the top of useAction (or alternatively move makeResolvers to module scope and call it once per processor change), then update sendAction to use the memoized resolveDynamicValue and resolveFunctionCall references; ensure the useCallback deps still include processor (or the memoized resolver refs) so closures remain stable.packages/genui/a2ui/src/react/useDataBinding.ts (1)
135-143: ⚡ Quick winRedundant branches in
resolveProperties— both arms do the same assignment.The
else if (primitive)branch and the trailingelseboth executeresult[key] = prop;. The primitive type check is dead code and adds noise. Collapse to a single fallthrough.♻️ Proposed simplification
- } else if ( - typeof prop === 'string' - || typeof prop === 'number' - || typeof prop === 'boolean' - ) { - result[key] = prop; - } else { - result[key] = prop; - } + } else { + result[key] = prop; + }🤖 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/src/react/useDataBinding.ts` around lines 135 - 143, In resolveProperties collapse the redundant branches that both assign result[key] = prop by removing the primitive type check (the else if that tests typeof prop === 'string'|'number'|'boolean') and leaving a single fallback assignment; update the function so only one code path assigns result[key] = prop (referencing resolveProperties, result, prop, and key to locate the change).
🤖 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-playground/rsbuild.config.ts`:
- Around line 33-46: The readJsonBody function currently accumulates request
data without limits; add a MAX_BODY_BYTES constant (e.g., a few MB) and check
the accumulated size on each 'data' event inside readJsonBody: if chunks total
length exceeds MAX_BODY_BYTES, reject the promise with a clear error (or emit an
HTTP 413-type error), remove listeners and call req.destroy() to terminate the
connection and prevent further memory growth; keep existing JSON parsing
behavior on 'end' and preserve the error handling path in the same function.
In `@packages/genui/a2ui/src/react/A2UI.tsx`:
- Around line 172-184: When handling the 'surfaceUpdate' branch in A2UI.tsx,
schedule a re-render when the update fills the root resource so A2UIRenderer
(which returns null until surface.resources.get(surface.rootComponentId) exists)
gets a chance to run; detect when update.id === surface.rootComponentId after
calling r?.complete(...) and invoke the surface's render-scheduling API (e.g.,
surface.requestRender() / surface.scheduleUpdate() / surface.invalidate() —
whichever method your surface exposes) or otherwise call the same render trigger
used by beginRendering so the newly-arrived root resource causes a fresh render.
---
Duplicate comments:
In `@packages/genui/a2ui/src/react/A2UI.tsx`:
- Around line 154-161: The offUpdate handler currently calls
proc.getOrCreateSurface(surfaceId) before checking for a deleteSurface update,
which can materialize a fresh Surface when the processor already removed it;
modify the offUpdate callback so it does not call getOrCreateSurface()
up-front—inspect the incoming data.type (and targetId/messageId) first, and if
type signals deletion use the pre-delete surface reference provided by
MessageProcessor (or request that MessageProcessor include the Surface reference
in the event payload) to complete/cleanup the root resource; update the logic in
the offUpdate handler (the proc.onUpdate callback) and similarly adjust the
other occurrences mentioned (around the block covering lines ~185-194) so
deleteSurface paths never create a new Surface via getOrCreateSurface().
---
Nitpick comments:
In `@packages/genui/a2ui-playground/examples/io-mock/mockAgent.ts`:
- Around line 86-91: The onAction handler can run concurrently and cause
interleaved messages because multiple calls call streamInto directly; fix by
serializing calls with a shared promise queue: add a module-level pending
promise (e.g., pendingActionPromise = Promise.resolve()) and in onAction chain
the work onto it (pendingActionPromise = pendingActionPromise.then(() => { const
stream = typeof mock === 'function' ? mock(action) : mock; return
streamInto(stream); }).catch(() => {})), returning the chained promise so each
invocation waits for prior ones to finish; update references to onAction and
streamInto accordingly and ensure errors are swallowed or logged so the chain
continues.
In `@packages/genui/a2ui-playground/rsbuild.config.ts`:
- Around line 66-69: Replace the loose startsWith checks with exact route
comparisons so only the intended endpoints are matched: in the conditional that
checks req.method === 'OPTIONS' and uses url.startsWith('/__a2ui_payload') ||
url.startsWith('/__a2ui/'), change those to strict comparisons (e.g. url ===
'/__a2ui_payload' and url === '/__a2ui/' or url === '/__a2ui' depending on the
canonical route) and apply the same change to the second occurrence later in the
file; update the checks around the req.method/url logic in rsbuild.config.ts to
use these exact string matches instead of startsWith.
In `@packages/genui/a2ui/src/react/useAction.ts`:
- Around line 128-172: The sendAction callback in useAction repeatedly calls
makeResolvers(processor) on every dispatch, reallocating closures
(resolveDynamicValue, resolveFunctionCall) unnecessarily; hoist makeResolvers
out of the dispatch path by creating the resolvers once with useMemo (e.g.,
const { resolveDynamicValue, resolveFunctionCall } = useMemo(() =>
makeResolvers(processor), [processor])) at the top of useAction (or
alternatively move makeResolvers to module scope and call it once per processor
change), then update sendAction to use the memoized resolveDynamicValue and
resolveFunctionCall references; ensure the useCallback deps still include
processor (or the memoized resolver refs) so closures remain stable.
In `@packages/genui/a2ui/src/react/useDataBinding.ts`:
- Around line 135-143: In resolveProperties collapse the redundant branches that
both assign result[key] = prop by removing the primitive type check (the else if
that tests typeof prop === 'string'|'number'|'boolean') and leaving a single
fallback assignment; update the function so only one code path assigns
result[key] = prop (referencing resolveProperties, result, prop, and key to
locate the change).
In `@packages/genui/a2ui/src/store/Resource.ts`:
- Around line 70-75: The notify function must avoid iterating the live Set and
let one subscriber's exception abort the rest; change notify (the function
referencing listeners, snapshot, status, value, error) to first update snapshot
as now, then create a stable copy of listeners (e.g. Array.from(listeners)) and
iterate that copy, wrapping each fn() call in a try/catch so a thrown error from
one subscriber doesn't stop others (this also prevents re-entrant
subscribe/unsubscribe from affecting the current delivery and ensures
useSyncExternalStore's scheduler always runs).
🪄 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: 22d64d99-8ca8-472f-8332-bfa2b8d08565
📒 Files selected for processing (13)
.changeset/quirky-merkle-web-core-fetch.mdpackages/genui/a2ui-playground/examples/io-mock/mockAgent.tspackages/genui/a2ui-playground/examples/io-sse/sseAgent.tspackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui-playground/turbo.jsonpackages/genui/a2ui/src/react/A2UI.tsxpackages/genui/a2ui/src/react/A2UIRenderer.tsxpackages/genui/a2ui/src/react/useAction.tspackages/genui/a2ui/src/react/useDataBinding.tspackages/genui/a2ui/src/store/Resource.tspackages/genui/a2ui/src/store/payloadNormalizer.tspackages/genui/a2ui/test/createResource.test.tspackages/rspeedy/core/turbo.json
🚧 Files skipped from review as they are similar to previous changes (4)
- .changeset/quirky-merkle-web-core-fetch.md
- packages/genui/a2ui/test/createResource.test.ts
- packages/genui/a2ui-playground/examples/io-sse/sseAgent.ts
- packages/genui/a2ui/src/store/payloadNormalizer.ts
Comment on lines
+33
to
+46
| function readJsonBody(req: IncomingMessage): Promise<unknown> { | ||
| return new Promise((resolve, reject) => { | ||
| const chunks: Buffer[] = []; | ||
| req.on('data', (c: Buffer) => chunks.push(c)); | ||
| req.on('end', () => { | ||
| try { | ||
| const raw = Buffer.concat(chunks).toString('utf8'); | ||
| resolve(raw ? JSON.parse(raw) : {}); | ||
| } catch (e) { | ||
| reject(e instanceof Error ? e : new Error(String(e))); | ||
| } | ||
| }); | ||
| req.on('error', reject); | ||
| }); |
Contributor
There was a problem hiding this comment.
Add a request-body size cap in readJsonBody to prevent memory exhaustion.
Line 35 accumulates the entire payload in memory with no limit. Since this middleware is exposed on 0.0.0.0, a large POST can exhaust memory and destabilize the dev server.
Suggested fix
function readJsonBody(req: IncomingMessage): Promise<unknown> {
return new Promise((resolve, reject) => {
+ const MAX_BODY_BYTES = 1 * 1024 * 1024; // 1MB
const chunks: Buffer[] = [];
+ let total = 0;
req.on('data', (c: Buffer) => chunks.push(c));
+ req.on('data', (c: Buffer) => {
+ total += c.length;
+ if (total > MAX_BODY_BYTES) {
+ reject(new Error('payload too large'));
+ req.destroy();
+ } else {
+ chunks.push(c);
+ }
+ });
req.on('end', () => {
try {
const raw = Buffer.concat(chunks).toString('utf8');
resolve(raw ? JSON.parse(raw) : {});
} catch (e) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function readJsonBody(req: IncomingMessage): Promise<unknown> { | |
| return new Promise((resolve, reject) => { | |
| const chunks: Buffer[] = []; | |
| req.on('data', (c: Buffer) => chunks.push(c)); | |
| req.on('end', () => { | |
| try { | |
| const raw = Buffer.concat(chunks).toString('utf8'); | |
| resolve(raw ? JSON.parse(raw) : {}); | |
| } catch (e) { | |
| reject(e instanceof Error ? e : new Error(String(e))); | |
| } | |
| }); | |
| req.on('error', reject); | |
| }); | |
| function readJsonBody(req: IncomingMessage): Promise<unknown> { | |
| return new Promise((resolve, reject) => { | |
| const MAX_BODY_BYTES = 1 * 1024 * 1024; // 1MB | |
| const chunks: Buffer[] = []; | |
| let total = 0; | |
| req.on('data', (c: Buffer) => { | |
| total += c.length; | |
| if (total > MAX_BODY_BYTES) { | |
| reject(new Error('payload too large')); | |
| req.destroy(); | |
| } else { | |
| chunks.push(c); | |
| } | |
| }); | |
| req.on('end', () => { | |
| try { | |
| const raw = Buffer.concat(chunks).toString('utf8'); | |
| resolve(raw ? JSON.parse(raw) : {}); | |
| } catch (e) { | |
| reject(e instanceof Error ? e : new Error(String(e))); | |
| } | |
| }); | |
| req.on('error', reject); | |
| }); | |
| } |
🤖 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-playground/rsbuild.config.ts` around lines 33 - 46, The
readJsonBody function currently accumulates request data without limits; add a
MAX_BODY_BYTES constant (e.g., a few MB) and check the accumulated size on each
'data' event inside readJsonBody: if chunks total length exceeds MAX_BODY_BYTES,
reject the promise with a clear error (or emit an HTTP 413-type error), remove
listeners and call req.destroy() to terminate the connection and prevent further
memory growth; keep existing JSON parsing behavior on 'end' and preserve the
error handling path in the same function.
Comment on lines
+172
to
+184
| } else if (type === 'surfaceUpdate') { | ||
| const updates = | ||
| (data as { updates?: ReadonlyArray<{ id?: string }> }).updates ?? []; | ||
| for (const update of updates) { | ||
| if (!update.id) continue; | ||
| const r = surface.resources.get(update.id); | ||
| r?.complete({ | ||
| type: 'surfaceUpdate', | ||
| surfaceId, | ||
| surface, | ||
| component: update as ResourceInfo['component'] & object, | ||
| } as ResourceInfo); | ||
| } |
Contributor
There was a problem hiding this comment.
Trigger a rerender when the root resource arrives via surfaceUpdate.
A2UIRenderer returns null until surface.resources.get(surface.rootComponentId) exists. When beginRendering and the first root update land in different store snapshots, this branch fills the root resource after commit but never schedules another render, so the surface can stay blank until some unrelated update happens.
💡 Minimal fix
} else if (type === 'surfaceUpdate') {
const updates =
(data as { updates?: ReadonlyArray<{ id?: string }> }).updates ?? [];
+ let shouldForceRender = false;
for (const update of updates) {
if (!update.id) continue;
const r = surface.resources.get(update.id);
r?.complete({
type: 'surfaceUpdate',
surfaceId,
surface,
component: update as ResourceInfo['component'] & object,
} as ResourceInfo);
+ if (update.id === surface.rootComponentId) {
+ shouldForceRender = true;
+ }
}
+ if (shouldForceRender) forceUpdate();
}🤖 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/src/react/A2UI.tsx` around lines 172 - 184, When handling
the 'surfaceUpdate' branch in A2UI.tsx, schedule a re-render when the update
fills the root resource so A2UIRenderer (which returns null until
surface.resources.get(surface.rootComponentId) exists) gets a chance to run;
detect when update.id === surface.rootComponentId after calling r?.complete(...)
and invoke the surface's render-scheduling API (e.g., surface.requestRender() /
surface.scheduleUpdate() / surface.invalidate() — whichever method your surface
exposes) or otherwise call the same render trigger used by beginRendering so the
newly-arrived root resource causes a fresh render.
5932ae7 to
f32fcbc
Compare
React Example with Element Template#74 Bundle Size — 198.12KiB (0%).f32fcbc(current) vs 5f3b6eb main#64(baseline) Bundle metrics
|
| Current #74 |
Baseline #64 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
78 |
78 |
| Duplicate Modules |
23 |
23 |
| Duplicate Code |
40.58% |
40.58% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #74 |
Baseline #64 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
52.36KiB |
52.36KiB |
Bundle analysis report Branch PupilTong:claude/quirky-merkle-9... Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
2 times, most recently
from
28e5f6c to
a877ae4
Compare
Foundational data layer for the headless A2UI renderer. Pure data logic, no React surface yet — consumers can compose their own rendering shell on top. - `MessageStore`: append-only buffer of raw protocol messages with a `useSyncExternalStore`-friendly snapshot/subscribe API. - `MessageProcessor`: turns the v0.9 stream into surface state (component graph, data-model signals, per-component resources) and emits typed update events for downstream consumers. - `Resource`: pending/success/error state machine whose snapshot reference changes on every transition, so React doesn't bail out on `pending → error`. - `SignalStore`: thin `@preact/signals` wrapper used as the per-surface data-model store. - `payloadNormalizer`: turns arbitrary developer payloads (string, number, structured) into a flat `ServerToClientMessage[]`. Full unit-test coverage (34 tests) for the buffer, processor, resource, and payload normalizer. Adds a per-package `rstest.config.ts` plus split `tsconfig.json` (lint + tests) / `tsconfig.build.json` (emit) so tests run via `pnpm -F @lynx-js/a2ui-reactlynx test` without polluting the emit rootDir. This PR is additive — the existing `core/` and `chat/` paths (BaseClient, A2UIRender, etc.) keep working untouched. The React renderer that consumes this store ships in a follow-up.
Build the public React surface on top of the message buffer + surface
processor introduced in the previous PR. The renderer is headless:
it ships no styles or chrome, and consumers wrap surfaces themselves.
- `<A2UI>`: all-in-one component that owns a `MessageProcessor` per
mount, subscribes to the developer's `MessageStore`, processes new
tail messages each render, and renders the most recent surface.
- `<A2UIRenderer>` / `NodeRenderer`: lower-level building blocks for
consumers that want manual control over surface lifecycle.
- `<A2UIProvider>`: internal context carrying the active processor
+ catalog map.
- `useAction` / `useDataBinding` / `useCatalog`: hooks the catalog
components reach for; resolve actions, data bindings, and catalog
lookups against the current provider.
Catalog migration:
- Built-in components (`Text`, `Button`, `Card`, `Column`, `List`,
`Row`, `CheckBox`, `RadioGroup`, `Image`, `Divider`) move from
`core/A2UIRender` + global `componentRegistry` to the new
`react/A2UIRenderer`. Side-effect re-exports (`<Name>.ts`,
`all.ts`) and the global registry are dropped — every consumer
composes via `defineCatalog([...])`.
- New public surface in `src/index.ts`; `package.json` exports
trimmed accordingly (`./core`, `./chat`, `./catalog/all` removed;
`./catalog/<Name>/catalog.json` subpaths kept for the manifest
imports).
Cleanup:
- Delete `src/core/` (BaseClient, A2UIRender, ComponentRegistry,
processor, types, useAction, useDataBinding) — replaced by the
new layered design.
- Delete `src/chat/` (Conversation, useLynxClient) — now lives as
an example pattern in the playground README, not as a package
export.
- Delete `src/utils/` (ComponentRegistry, SignalStore,
createResource) — replaced by the typed `src/store/` equivalents.
Playground:
- `lynx-src/App.tsx` switches from the old `BaseClient` +
`A2UIRender` flow to `<A2UI messageStore={...} catalogs={...}>`
with the per-component manifest tuple form.
- `examples/io-mock/` and `examples/io-sse/` show how to push raw
protocol messages into a `MessageStore` from a developer-owned IO
module (mock + SSE transports).
- `lynx-src/tsconfig.json` enables `resolveJsonModule` so the
catalog manifest imports resolve.
Stacks on top of `feat(a2ui): add headless message buffer and surface
processor`. Once that lands and main rebases here, the diff is just
the React layer + catalog migration + playground.
PupilTong
force-pushed
the
claude/quirky-merkle-96ccad
branch
from
a877ae4 to
6d0a83d
Compare
Collaborator
Author
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.
Restructure @lynx-js/a2ui-reactlynx around three layers — store / catalog / react — plus a single all-in-one component. Hide protocol session, resource bookkeeping, and IO plumbing behind a MessageStore that exposes a useSyncExternalStore contract; developers implement IO inside the store's send callback (HTTP, SSE, WebSocket, MCP, mock — all out of scope for the package).
Public API
React + StrictMode hardening
Removed / reorganized
Examples (a2ui-playground/examples/)
Extractor
Tests
Verification
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Checklist