Closed
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR renames the CustomEvent payload property from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 6s | View ↗ |
nx run-many --targets=build --exclude=examples/** |
❌ Failed | 3s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-02-20 23:49:25 UTC
Co-Authored-By: Warp <agent@warp.dev>
jherr
force-pushed
the
ag-ui-customevents-v2
branch
from
0e52ef8 to
8c2d0a4
Compare
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/typescript/ai-client/tests/test-utils.ts (1)
189-199:⚠️ Potential issue | 🔴 CriticalUpdate processor.ts to use
chunk.valueinstead ofchunk.datafor CUSTOM events.The refactoring is incomplete. While test-utils.ts correctly renamed the field to
value,packages/typescript/ai/src/activities/chat/stream/processor.tslines 866–867 still accesschunk.datawhen handling CUSTOM events:this.events.onCustomEvent?.(chunk.name, chunk.data, { toolCallId: (chunk.data as any)?.toolCallId, })This must be updated to use
chunk.valueto match the new event structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai-client/tests/test-utils.ts` around lines 189 - 199, The CUSTOM event handling in processor.ts still reads chunk.data but the event shape was renamed to chunk.value; update the handler in the relevant method (where events.onCustomEvent is invoked) to pass chunk.value instead of chunk.data and extract the toolCallId from (chunk.value as any)?.toolCallId when calling this.events.onCustomEvent?.(chunk.name, ...). Ensure all occurrences in the processor's CUSTOM branch that reference chunk.data are changed to chunk.value so onCustomEvent receives the new event payload.
🧹 Nitpick comments (1)
packages/typescript/smoke-tests/adapters/src/harness.ts (1)
292-308: Consider renaming the local variabledatafor consistency with the rename.After extracting
chunk.value, assigning it to a variable nameddatare-introduces the old name immediately, which is likely to confuse readers during maintenance.♻️ Proposed rename
- if (chunk.name === 'approval-requested' && chunk.value) { - const data = chunk.value as { + if (chunk.name === 'approval-requested' && chunk.value) { + const payload = chunk.value as { toolCallId: string toolName: string input: any approval: any } const approval: ApprovalCapture = { - toolCallId: data.toolCallId, - toolName: data.toolName, - input: data.input, - approval: data.approval, + toolCallId: payload.toolCallId, + toolName: payload.toolName, + input: payload.input, + approval: payload.approval, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/smoke-tests/adapters/src/harness.ts` around lines 292 - 308, Local variable "data" reintroduces the old name after chunk.value was extracted; rename it to something consistent like "chunkValue" (or "valueObj") in the approval-requested branch where you build the ApprovalCapture so readers use the same conceptual name as chunkData/value. Update the block that checks if (chunk.name === 'approval-requested' && chunk.value) to declare const chunkValue = chunk.value as { toolCallId: string; toolName: string; input: any; approval: any } and then construct the ApprovalCapture using chunkValue.toolCallId, chunkValue.toolName, chunkValue.input, chunkValue.approval before pushing to approvalRequests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/perky-sails-brush.md:
- Around line 2-5: The changeset wrongly marks a breaking rename
(CustomEvent.data → CustomEvent.value) as a patch; update the .changeset so that
'@tanstack/ai' and '@tanstack/ai-client' are bumped to at least "minor" (or
"major" if you follow strict SemVer) while keeping '@tanstack/tests-adapters' as
"patch", and while you’re here consider adding a compatibility note or a
temporary shim (e.g., map event.data to event.value) in the code paths that emit
CustomEvent so consumers reading event.data won’t silently break; reference the
symbols CustomEvent.data, CustomEvent.value and the package names '@tanstack/ai'
and '@tanstack/ai-client' when making these edits.
---
Outside diff comments:
In `@packages/typescript/ai-client/tests/test-utils.ts`:
- Around line 189-199: The CUSTOM event handling in processor.ts still reads
chunk.data but the event shape was renamed to chunk.value; update the handler in
the relevant method (where events.onCustomEvent is invoked) to pass chunk.value
instead of chunk.data and extract the toolCallId from (chunk.value as
any)?.toolCallId when calling this.events.onCustomEvent?.(chunk.name, ...).
Ensure all occurrences in the processor's CUSTOM branch that reference
chunk.data are changed to chunk.value so onCustomEvent receives the new event
payload.
---
Nitpick comments:
In `@packages/typescript/smoke-tests/adapters/src/harness.ts`:
- Around line 292-308: Local variable "data" reintroduces the old name after
chunk.value was extracted; rename it to something consistent like "chunkValue"
(or "valueObj") in the approval-requested branch where you build the
ApprovalCapture so readers use the same conceptual name as chunkData/value.
Update the block that checks if (chunk.name === 'approval-requested' &&
chunk.value) to declare const chunkValue = chunk.value as { toolCallId: string;
toolName: string; input: any; approval: any } and then construct the
ApprovalCapture using chunkValue.toolCallId, chunkValue.toolName,
chunkValue.input, chunkValue.approval before pushing to approvalRequests.
Comment on lines
+2
to
+5
| '@tanstack/tests-adapters': patch | ||
| '@tanstack/ai-client': patch | ||
| '@tanstack/ai': patch | ||
| --- |
Contributor
There was a problem hiding this comment.
Breaking API change incorrectly versioned as patch.
Renaming CustomEvent.data → CustomEvent.value silently breaks any consumer that reads event.data (they get undefined with no error). This is a breaking change and must be versioned accordingly:
@tanstack/aiand@tanstack/ai-client: should beminorat minimum (ormajorif following strict SemVer). Apatchbump gives consumers no semver signal that their code may break.@tanstack/tests-adapters:patchis acceptable since it is a test/dev-only package.
🔧 Suggested fix
---
-'@tanstack/tests-adapters': patch
-'@tanstack/ai-client': patch
-'@tanstack/ai': patch
+'@tanstack/tests-adapters': patch
+'@tanstack/ai-client': minor
+'@tanstack/ai': minor
---📝 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
| '@tanstack/tests-adapters': patch | |
| '@tanstack/ai-client': patch | |
| '@tanstack/ai': patch | |
| --- | |
| '@tanstack/tests-adapters': patch | |
| '@tanstack/ai-client': minor | |
| '@tanstack/ai': minor | |
| --- |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/perky-sails-brush.md around lines 2 - 5, The changeset wrongly
marks a breaking rename (CustomEvent.data → CustomEvent.value) as a patch;
update the .changeset so that '@tanstack/ai' and '@tanstack/ai-client' are
bumped to at least "minor" (or "major" if you follow strict SemVer) while
keeping '@tanstack/tests-adapters' as "patch", and while you’re here consider
adding a compatibility note or a temporary shim (e.g., map event.data to
event.value) in the code paths that emit CustomEvent so consumers reading
event.data won’t silently break; reference the symbols CustomEvent.data,
CustomEvent.value and the package names '@tanstack/ai' and '@tanstack/ai-client'
when making these edits.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/typescript/ai/src/activities/chat/index.ts (1)
1095-1106:⚠️ Potential issue | 🔴 Critical
createCustomEventChunkstill emitsdatainstead ofvalue— TypeScript error and runtime bug.
createCustomEventChunkwas not updated as part of this rename. It returns{ ..., data }(shorthand fordata: data), butCustomEventonly declaresvalue?: unknown— there is nodataproperty. This causes:
- TypeScript compile error: object literal excess-property check will reject
dataon aCustomEventreturn type.- Runtime bug: tools that call
context.emitCustomEvent(...)will produce events withdataset andvalueundefined, silently breaking every consumer readingchunk.value.🐛 Proposed fix
private createCustomEventChunk( eventName: string, data: Record<string, any>, ): CustomEvent { return { type: 'CUSTOM', timestamp: Date.now(), model: this.params.model, name: eventName, - data, + value: data, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai/src/activities/chat/index.ts` around lines 1095 - 1106, The createCustomEventChunk function is returning an object with a data property which mismatches the CustomEvent type (which expects value) and causes TS and runtime errors; update createCustomEventChunk to return the event with value: data (i.e., replace the shorthand data property with value) so the returned object matches CustomEvent, and run a quick search for uses of createCustomEventChunk / context.emitCustomEvent to ensure callers expect chunk.value (adjust if any caller relied on chunk.data).packages/typescript/ai-client/tests/test-utils.ts (1)
149-162:⚠️ Potential issue | 🟠 MajorUpdate
createCustomEventChunksto usevalueinstead ofdata.The PR renames the CUSTOM event payload property from
datatovalue, but this helper was missed. TheStreamChunkCUSTOM type now definesvalue?: unknown, andcreateToolCallChunks(line 224) already uses the new property.createCustomEventChunksstill usesdatain both the parameter type (line 149) and the emitted chunk (line 160), causing a type mismatch and breaking any tests that use this helper.Proposed fix
export function createCustomEventChunks( - events: Array<{ name: string; data?: unknown }>, + events: Array<{ name: string; value?: unknown }>, model: string = 'test', ): Array<StreamChunk> { const chunks: Array<StreamChunk> = [] for (const event of events) { chunks.push({ type: 'CUSTOM', model, timestamp: Date.now(), name: event.name, - data: event.data, + value: event.value, }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai-client/tests/test-utils.ts` around lines 149 - 162, The helper createCustomEventChunks is still using the old `data` property in its parameter type and when emitting StreamChunk objects, causing a type mismatch with the updated StreamChunk CUSTOM shape that uses `value?: unknown`; update the events parameter type to use `value?: unknown` and change the emitted chunk property from `data: event.data` to `value: event.value` (consistent with how createToolCallChunks uses `value`) so the returned Array<StreamChunk> matches the new CUSTOM type.
🧹 Nitpick comments (1)
packages/typescript/ai/src/types.ts (1)
351-373:emitCustomEventparameter and JSDoc still referencedata.The callback parameter is named
dataand its JSDoc says@param data - Event payload data. After this rename the argument is stored asvaluein the emittedCustomEvent. The mismatch is harmless (it's a local param name), but it can mislead tool authors who inspect the signature and expect the emitted event to carry adatafield.♻️ Suggested rename for clarity
- emitCustomEvent: (eventName: string, data: Record<string, any>) => void + emitCustomEvent: (eventName: string, value: Record<string, any>) => voidAnd in the JSDoc:
- * `@param` data - Event payload data + * `@param` value - Custom event value🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai/src/types.ts` around lines 351 - 373, The emitCustomEvent signature and JSDoc are inconsistent with the actual emitted payload field; update the ToolExecutionContext.emitCustomEvent parameter name from "data" to "value" and change the JSDoc `@param` description accordingly so it documents the event payload as "value" (matching the emitted CustomEvent payload field), e.g., update the function declaration in ToolExecutionContext and the corresponding JSDoc param line to use "value" and describe it as the event payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/typescript/ai-client/tests/test-utils.ts`:
- Around line 149-162: The helper createCustomEventChunks is still using the old
`data` property in its parameter type and when emitting StreamChunk objects,
causing a type mismatch with the updated StreamChunk CUSTOM shape that uses
`value?: unknown`; update the events parameter type to use `value?: unknown` and
change the emitted chunk property from `data: event.data` to `value:
event.value` (consistent with how createToolCallChunks uses `value`) so the
returned Array<StreamChunk> matches the new CUSTOM type.
In `@packages/typescript/ai/src/activities/chat/index.ts`:
- Around line 1095-1106: The createCustomEventChunk function is returning an
object with a data property which mismatches the CustomEvent type (which expects
value) and causes TS and runtime errors; update createCustomEventChunk to return
the event with value: data (i.e., replace the shorthand data property with
value) so the returned object matches CustomEvent, and run a quick search for
uses of createCustomEventChunk / context.emitCustomEvent to ensure callers
expect chunk.value (adjust if any caller relied on chunk.data).
---
Duplicate comments:
In @.changeset/perky-sails-brush.md:
- Around line 2-5: The changeset incorrectly marks a breaking rename
(CustomEvent.data → .value) as 'patch' — update the entries for '@tanstack/ai'
and '@tanstack/ai-client' in the changeset so they are bumped to 'minor' (or
'major' if your release policy requires strict SemVer) instead of 'patch'; edit
the change description line for those two packages in the .changeset file to
reflect the higher bump and keep the note about the breaking change so release
tooling and consumers are properly signaled.
---
Nitpick comments:
In `@packages/typescript/ai/src/types.ts`:
- Around line 351-373: The emitCustomEvent signature and JSDoc are inconsistent
with the actual emitted payload field; update the
ToolExecutionContext.emitCustomEvent parameter name from "data" to "value" and
change the JSDoc `@param` description accordingly so it documents the event
payload as "value" (matching the emitted CustomEvent payload field), e.g.,
update the function declaration in ToolExecutionContext and the corresponding
JSDoc param line to use "value" and describe it as the event payload.
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.
🎯 Changes
Moving to the AG-UI standard for custom events.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Refactor
datatovalueacross TypeScript and Python APIs — update code that reads event payloads.Documentation
valueinstead ofdata.Tests
valuein emitted events.