feat: overhaul AI devtools hook dashboard#632
Open
AlemTuzlak wants to merge 9 commits into
Open
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:
📝 WalkthroughWalkthroughChatClient now integrates a devtools bridge to track runs, emit snapshots, and observe stream chunks. The structured-output API refactors to use centralized request parsing and phase-count emission. A new /generation-hooks demo page showcases six AI generation hooks with fixture-based streaming. Documentation updates describe hook dashboard, tool fixtures, and event sources. ChangesChatClient Devtools Integration and Hook Dashboard Demo
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Changeset Version Preview8 package(s) bumped directly, 15 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 5685614
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
![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: 7
🧹 Nitpick comments (3)
packages/typescript/ai-event-client/src/envelope.ts (1)
72-102: 💤 Low valueVerify intentional use of module-level
runtimeIdat line 93.Line 82 uses
resolvedRuntimeId(which respectsinput.runtimeId), but line 93 uses the module-levelruntimeIddirectly. If this is intentional (to ensure local uniqueness via the module's own runtime ID + counter), consider adding a brief comment explaining the distinction. Otherwise, line 93 should useresolvedRuntimeIdfor consistency.🤖 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/typescript/ai-event-client/src/envelope.ts` around lines 72 - 102, The eventId construction in createAIDevtoolsEventEnvelope mixes resolvedRuntimeId (which respects input.runtimeId) and the module-level runtimeId directly; decide and make it consistent: either replace the literal runtimeId in the eventId parts with resolvedRuntimeId so the generated eventId uses the resolved value throughout, or, if the intent is to combine the resolved runtimeId with the module-level runtimeId for local uniqueness, add a short comment above the array explaining why runtimeId (module-level) is used intentionally; update the array used to build eventId accordingly (references: createAIDevtoolsEventEnvelope, resolvedRuntimeId, runtimeId, eventId).packages/typescript/ai-client/src/chat-client.ts (1)
508-516: 💤 Low valueRemove underscore prefix from used parameter.
The
_fixtureparameter naming convention suggests an unused variable, but it's immediately assigned tofixtureand used throughout the method. Consider renaming to justfixturedirectly in the parameter.Suggested fix
private async applyToolFixture( - _fixture: AIDevtoolsToolFixture, + fixture: AIDevtoolsToolFixture, ): Promise<void> { - const fixture = _fixture const messages = this.processor.getMessages() as Array<UIMessage>🤖 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/typescript/ai-client/src/chat-client.ts` around lines 508 - 516, In applyToolFixture, the parameter is named _fixture (suggesting unused) but immediately assigned to fixture and used; rename the parameter from _fixture to fixture and remove the redundant const fixture = _fixture assignment, then update usages in the function (e.g., threadId = fixture.threadId ?? this.threadId and the call to this.executeToolFixture(fixture, messages, threadId)) so the method uses the parameter directly.packages/typescript/ai-devtools/tests/preview-model.test.ts (1)
54-96: ⚡ Quick winConsider testing with actual DOM elements using happy-dom.
The test currently uses plain objects to simulate
element.dataset, which effectively tests the matching logic. However, ifhoverTargetMatchesElementis designed to work with real DOM elements in production, consider adding integration tests with actual elements using happy-dom to verify the complete flow.As per coding guidelines, Vitest unit tests should use happy-dom for DOM testing.
🧪 Example integration test with happy-dom
+import { beforeEach } from 'vitest' + +describe('preview model with DOM', () => { + beforeEach(() => { + document.body.innerHTML = '' + }) + + it('matches hover targets against real DOM elements', () => { + const target = createHoverTarget({ + messageIds: ['message-1'], + partIds: [toolCallPartId('call-1')], + origin: 'preview', + }) + + const element = document.createElement('div') + element.dataset.aiDevtoolsHoverMessageIds = 'message-1' + element.dataset.aiDevtoolsHoverPartIds = 'tool-call:call-1' + + expect(hoverTargetMatchesElement(target, element.dataset)).toBe(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/typescript/ai-devtools/tests/preview-model.test.ts` around lines 54 - 96, Test currently passes plain objects as element.dataset; replace/add an integration variant that uses a real DOM element created via happy-dom and exercise the same flows: create an element via the happy-dom document (e.g., document.createElement('div')), set dataset attributes or setAttribute for the same keys produced by getHoverDataAttributes, then assert hoverTargetMatchesElement(element, ...) and createHoverTargetFromDataAttributes(element.dataset) behave identically to the plain-object expectations; reference the existing helpers createHoverTarget, getHoverDataAttributes, hoverTargetMatchesElement, and createHoverTargetFromDataAttributes to locate where to plug the DOM-backed assertions and ensure the test file uses happy-dom as the Vitest DOM environment.
🤖 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 `@examples/ts-react-chat/src/routes/generations.structured-output.tsx`:
- Around line 223-233: In handleGenerate the guard that reports a missing
structured-output.complete is inverted; change the final condition from checking
chat.status !== 'ready' to chat.status === 'ready' so that when stream is true
and sawCompleteRef.current is false and chat.status === 'ready' you call
setError('Stream ended before structured-output.complete'); update the check
that references sawCompleteRef, chat.status and stream in the handleGenerate
function accordingly.
In `@packages/typescript/ai-devtools/src/components/hooks/HookDetails.tsx`:
- Around line 785-831: The part rows rendered in HookDetails (inside the For
over visiblePreviewPartsForMessage(message)) are missing
data-ai-devtools-hover-* attributes, so setHoverTargetFromEvent cannot resolve
part-level hover targets; update the outer div for each part (the element with
class styles().hookDetails.previewPart) to include data attributes such as
data-ai-devtools-hover-type="part",
data-ai-devtools-hover-message-id={message.id} (or message._id), and a unique
part identifier like data-ai-devtools-hover-part={part.key || part.label ||
index} so setHoverTargetFromEvent can detect and distinguish part-level targets
and enable part-level sync/scroll to preview.
In `@packages/typescript/ai-devtools/src/components/hooks/ToolFixtureForm.tsx`:
- Around line 275-283: In parseFieldValue, stop silently truncating integer
inputs: when field.type === 'integer' validate that the parsed Number(rawValue)
is an integer (use Number.isInteger) and throw a descriptive Error like "<name>
must be an integer" if it's not; otherwise return the integer value — replace
the current Math.trunc-based behavior in the parseFieldValue function.
In `@packages/typescript/ai-devtools/src/components/Shell.tsx`:
- Around line 89-99: onCleanup currently removes document event listeners and
emits the devtools:closed event but can leave document.body in drag mode if
handleMouseUp didn't run; update the onCleanup block to also reset global
drag-related styles by setting document.body.style.cursor = '' and
document.body.style.userSelect = '' (or revert to their prior values) so any
drag state set by handleMouseMove/handleMouseUp is cleared; locate the onCleanup
call in Shell.tsx and add these two style resets alongside the existing
document.removeEventListener and
aiEventClient.emit/createAIDevtoolsEventEnvelope logic.
In `@packages/typescript/ai-devtools/src/store/ai-context.tsx`:
- Around line 1341-1348: The normalization for message parts currently discards
the tool-result's non-text output by not including part.output in the returned
object; update the branch that handles part.type === 'tool-result' (the object
constructed with type: 'tool-result', toolCallId, content, state, error) to also
preserve output by adding output: part.output so downstream previews and store
state retain non-text tool results.
In `@packages/typescript/ai-preact/src/use-chat.ts`:
- Around line 160-170: The cleanup effect currently depends on options.live
causing client.dispose() (and client.stop()/client.unsubscribe()) to run when
live toggles; change the effect so it only runs on unmount or when the client
instance changes by removing options.live from the dependency array (i.e. depend
only on client) so that client.mountDevtools() is mounted once per client and
client.dispose() is invoked only on unmount/client replacement; leave the
dedicated subscribe/unsubscribe effect to handle options.live toggling.
In `@packages/typescript/ai-react/src/use-chat.ts`:
- Around line 157-168: The devtools mounting effect currently depends on
options.live and its cleanup always calls client.dispose(), causing the client
to be torn down whenever live toggles; change the effect to only depend on
client (remove options.live from the dependency array) and reference
optionsRef.current.live inside the cleanup to decide between
client.unsubscribe() and client.stop() so the separate live-toggle effect
continues to manage subscribe/unsubscribe; specifically update the useEffect
around client.mountDevtools to use optionsRef.current.live instead of
options.live and remove options.live from the dependency list so
client.dispose() is not invoked on live changes.
---
Nitpick comments:
In `@packages/typescript/ai-client/src/chat-client.ts`:
- Around line 508-516: In applyToolFixture, the parameter is named _fixture
(suggesting unused) but immediately assigned to fixture and used; rename the
parameter from _fixture to fixture and remove the redundant const fixture =
_fixture assignment, then update usages in the function (e.g., threadId =
fixture.threadId ?? this.threadId and the call to
this.executeToolFixture(fixture, messages, threadId)) so the method uses the
parameter directly.
In `@packages/typescript/ai-devtools/tests/preview-model.test.ts`:
- Around line 54-96: Test currently passes plain objects as element.dataset;
replace/add an integration variant that uses a real DOM element created via
happy-dom and exercise the same flows: create an element via the happy-dom
document (e.g., document.createElement('div')), set dataset attributes or
setAttribute for the same keys produced by getHoverDataAttributes, then assert
hoverTargetMatchesElement(element, ...) and
createHoverTargetFromDataAttributes(element.dataset) behave identically to the
plain-object expectations; reference the existing helpers createHoverTarget,
getHoverDataAttributes, hoverTargetMatchesElement, and
createHoverTargetFromDataAttributes to locate where to plug the DOM-backed
assertions and ensure the test file uses happy-dom as the Vitest DOM
environment.
In `@packages/typescript/ai-event-client/src/envelope.ts`:
- Around line 72-102: The eventId construction in createAIDevtoolsEventEnvelope
mixes resolvedRuntimeId (which respects input.runtimeId) and the module-level
runtimeId directly; decide and make it consistent: either replace the literal
runtimeId in the eventId parts with resolvedRuntimeId so the generated eventId
uses the resolved value throughout, or, if the intent is to combine the resolved
runtimeId with the module-level runtimeId for local uniqueness, add a short
comment above the array explaining why runtimeId (module-level) is used
intentionally; update the array used to build eventId accordingly (references:
createAIDevtoolsEventEnvelope, resolvedRuntimeId, runtimeId, eventId).
🪄 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: fd30edd5-da12-4872-a9c7-da8b3aa8d16e
📒 Files selected for processing (101)
.changeset/ai-devtools-hook-dashboard.mddocs/getting-started/devtools.mdexamples/ts-react-chat/src/components/Header.tsxexamples/ts-react-chat/src/routeTree.gen.tsexamples/ts-react-chat/src/routes/api.structured-output.tsexamples/ts-react-chat/src/routes/generation-hooks.tsxexamples/ts-react-chat/src/routes/generations.structured-output.tsxpackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai-client/src/devtools.tspackages/typescript/ai-client/src/events.tspackages/typescript/ai-client/src/generation-client.tspackages/typescript/ai-client/src/generation-types.tspackages/typescript/ai-client/src/index.tspackages/typescript/ai-client/src/types.tspackages/typescript/ai-client/src/video-generation-client.tspackages/typescript/ai-client/tests/devtools.test.tspackages/typescript/ai-client/tests/events.test.tspackages/typescript/ai-client/tests/generation-client.test.tspackages/typescript/ai-client/tests/generation-devtools.test.tspackages/typescript/ai-client/tests/video-generation-client.test.tspackages/typescript/ai-devtools/src/components/ConversationDetails.tsxpackages/typescript/ai-devtools/src/components/ConversationsList.tsxpackages/typescript/ai-devtools/src/components/Shell.tsxpackages/typescript/ai-devtools/src/components/conversation/ActivityEventsTab.tsxpackages/typescript/ai-devtools/src/components/conversation/ChunkBadges.tsxpackages/typescript/ai-devtools/src/components/conversation/ChunkItem.tsxpackages/typescript/ai-devtools/src/components/conversation/ChunksCollapsible.tsxpackages/typescript/ai-devtools/src/components/conversation/ChunksTab.tsxpackages/typescript/ai-devtools/src/components/conversation/ConversationHeader.tsxpackages/typescript/ai-devtools/src/components/conversation/ConversationTabs.tsxpackages/typescript/ai-devtools/src/components/conversation/IterationCard.tsxpackages/typescript/ai-devtools/src/components/conversation/IterationTimeline.tsxpackages/typescript/ai-devtools/src/components/conversation/MessageCard.tsxpackages/typescript/ai-devtools/src/components/conversation/MessageGroup.tsxpackages/typescript/ai-devtools/src/components/conversation/MessagesTab.tsxpackages/typescript/ai-devtools/src/components/conversation/SummariesTab.tsxpackages/typescript/ai-devtools/src/components/conversation/ToolCallDisplay.tsxpackages/typescript/ai-devtools/src/components/conversation/index.tspackages/typescript/ai-devtools/src/components/hooks/GenerationPanel.tsxpackages/typescript/ai-devtools/src/components/hooks/HookDashboard.tsxpackages/typescript/ai-devtools/src/components/hooks/HookDetails.tsxpackages/typescript/ai-devtools/src/components/hooks/ToolFixtureForm.tsxpackages/typescript/ai-devtools/src/components/hooks/hook-dashboard-model.tspackages/typescript/ai-devtools/src/components/hooks/index.tspackages/typescript/ai-devtools/src/components/hooks/preview-messages.tspackages/typescript/ai-devtools/src/components/hooks/preview-model.tspackages/typescript/ai-devtools/src/components/list/ConversationRow.tsxpackages/typescript/ai-devtools/src/components/list/index.tspackages/typescript/ai-devtools/src/components/utils/format.tspackages/typescript/ai-devtools/src/store/ai-context.tsxpackages/typescript/ai-devtools/src/store/hook-registry.tspackages/typescript/ai-devtools/src/store/message-event-utils.tspackages/typescript/ai-devtools/src/styles/use-styles.tspackages/typescript/ai-devtools/tests/hook-dashboard-model.test.tspackages/typescript/ai-devtools/tests/hook-registry.test.tspackages/typescript/ai-devtools/tests/message-event-utils.test.tspackages/typescript/ai-devtools/tests/preview-messages.test.tspackages/typescript/ai-devtools/tests/preview-model.test.tspackages/typescript/ai-event-client/src/devtools-middleware.tspackages/typescript/ai-event-client/src/envelope.tspackages/typescript/ai-event-client/src/index.tspackages/typescript/ai-event-client/tests/emit.test.tspackages/typescript/ai-event-client/tests/envelope.test.tspackages/typescript/ai-preact/src/types.tspackages/typescript/ai-preact/src/use-chat.tspackages/typescript/ai-react/src/use-chat.tspackages/typescript/ai-react/src/use-generate-audio.tspackages/typescript/ai-react/src/use-generate-image.tspackages/typescript/ai-react/src/use-generate-speech.tspackages/typescript/ai-react/src/use-generate-video.tspackages/typescript/ai-react/src/use-generation.tspackages/typescript/ai-react/src/use-summarize.tspackages/typescript/ai-react/src/use-transcription.tspackages/typescript/ai-solid/src/use-chat.tspackages/typescript/ai-solid/src/use-generate-audio.tspackages/typescript/ai-solid/src/use-generate-image.tspackages/typescript/ai-solid/src/use-generate-speech.tspackages/typescript/ai-solid/src/use-generate-video.tspackages/typescript/ai-solid/src/use-generation.tspackages/typescript/ai-solid/src/use-summarize.tspackages/typescript/ai-solid/src/use-transcription.tspackages/typescript/ai-svelte/src/create-chat.svelte.tspackages/typescript/ai-svelte/src/create-generate-audio.svelte.tspackages/typescript/ai-svelte/src/create-generate-image.svelte.tspackages/typescript/ai-svelte/src/create-generate-speech.svelte.tspackages/typescript/ai-svelte/src/create-generate-video.svelte.tspackages/typescript/ai-svelte/src/create-generation.svelte.tspackages/typescript/ai-svelte/src/create-summarize.svelte.tspackages/typescript/ai-svelte/src/create-transcription.svelte.tspackages/typescript/ai-svelte/src/types.tspackages/typescript/ai-vue/src/use-chat.tspackages/typescript/ai-vue/src/use-generate-audio.tspackages/typescript/ai-vue/src/use-generate-image.tspackages/typescript/ai-vue/src/use-generate-speech.tspackages/typescript/ai-vue/src/use-generate-video.tspackages/typescript/ai-vue/src/use-generation.tspackages/typescript/ai-vue/src/use-summarize.tspackages/typescript/ai-vue/src/use-transcription.tspackages/typescript/ai/src/activities/chat/index.tspackages/typescript/ai/src/activities/chat/middleware/types.tspackages/typescript/ai/src/activities/chat/stream/processor.ts
💤 Files with no reviewable changes (18)
- packages/typescript/ai-devtools/src/components/conversation/ToolCallDisplay.tsx
- packages/typescript/ai-devtools/src/components/conversation/ChunksTab.tsx
- packages/typescript/ai-devtools/src/components/list/ConversationRow.tsx
- packages/typescript/ai-devtools/src/components/ConversationsList.tsx
- packages/typescript/ai-devtools/src/components/conversation/SummariesTab.tsx
- packages/typescript/ai-devtools/src/components/conversation/ChunkBadges.tsx
- packages/typescript/ai-devtools/src/components/list/index.ts
- packages/typescript/ai-devtools/src/components/conversation/ConversationTabs.tsx
- packages/typescript/ai-devtools/src/components/conversation/ActivityEventsTab.tsx
- packages/typescript/ai-devtools/src/components/conversation/ConversationHeader.tsx
- packages/typescript/ai-devtools/src/components/conversation/ChunkItem.tsx
- packages/typescript/ai-devtools/src/components/ConversationDetails.tsx
- packages/typescript/ai-devtools/src/components/conversation/index.ts
- packages/typescript/ai-devtools/src/components/conversation/MessageCard.tsx
- packages/typescript/ai-devtools/src/components/utils/format.ts
- packages/typescript/ai-devtools/src/components/conversation/MessagesTab.tsx
- packages/typescript/ai-devtools/src/components/conversation/MessageGroup.tsx
- packages/typescript/ai-devtools/src/components/conversation/ChunksCollapsible.tsx
Comment on lines
+1341
to
+1348
| if (part.type === 'tool-result') { | ||
| return { | ||
| type: 'tool-result', | ||
| toolCallId: part.toolCallId, | ||
| content: part.content, | ||
| state: part.state, | ||
| error: part.error, | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve tool-result output when normalizing message parts.
Line 1341-1348 drops part.output, which loses non-text tool results in store state and downstream previews.
Suggested patch
if (part.type === 'tool-result') {
return {
type: 'tool-result',
toolCallId: part.toolCallId,
+ output: part.output,
content: part.content,
state: part.state,
error: part.error,
}
}📝 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
| if (part.type === 'tool-result') { | |
| return { | |
| type: 'tool-result', | |
| toolCallId: part.toolCallId, | |
| content: part.content, | |
| state: part.state, | |
| error: part.error, | |
| } | |
| if (part.type === 'tool-result') { | |
| return { | |
| type: 'tool-result', | |
| toolCallId: part.toolCallId, | |
| output: part.output, | |
| content: part.content, | |
| state: part.state, | |
| error: part.error, | |
| } |
🤖 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/typescript/ai-devtools/src/store/ai-context.tsx` around lines 1341 -
1348, The normalization for message parts currently discards the tool-result's
non-text output by not including part.output in the returned object; update the
branch that handles part.type === 'tool-result' (the object constructed with
type: 'tool-result', toolCallId, content, state, error) to also preserve output
by adding output: part.output so downstream previews and store state retain
non-text tool results.
- Extract bridge construction into buildDevtoolsBridgeOptions helpers on ChatClient, GenerationClient, VideoGenerationClient - Add ChatDevtoolsAwareEventEmitter that auto-attaches run context and emits snapshots so clients call this.events.X(...) like normal events - Ship NoOp bridges by default; tree-shake real bridges into the @tanstack/ai-client/devtools subpath entry - Update React, Vue, Solid, Svelte, Preact hooks to pass the real bridge factory - Expand E2E suite: registration, chat, structured output, tools, generation hooks, responsive layout - Add devtools-name-types tests per framework integration
…haul # Conflicts: # examples/ts-react-chat/src/routes/api.structured-output.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/typescript/ai-client/src/generation-client.ts (1)
247-261:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
chunkRunIdwhen starting the run forCUSTOMchunks
processStreamextractschunkRunId, but theCUSTOMbranch ignores it (ensureRunStarted(streamRunId ?? fallbackRunId)). If aCUSTOMchunk carries a serverrunIdbeforeRUN_STARTED, the devtools bridge will start the fallback run first and then create a second run whenRUN_STARTEDarrives (it can only rename before the firstrun:started). PreferchunkRunId ?? streamRunId ?? fallbackRunIdand keepstreamRunIdin sync.Suggested fix
case 'CUSTOM': { - this.devtoolsBridge.ensureRunStarted(streamRunId ?? fallbackRunId) + const activeRunId = chunkRunId ?? streamRunId ?? fallbackRunId + streamRunId = activeRunId + this.devtoolsBridge.ensureRunStarted(activeRunId) if (chunk.name === GENERATION_EVENTS.RESULT) { this.setResult(chunk.value as TResult) } else if (chunk.name === GENERATION_EVENTS.PROGRESS) {🤖 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/typescript/ai-client/src/generation-client.ts` around lines 247 - 261, The CUSTOM branch currently calls this.devtoolsBridge.ensureRunStarted(streamRunId ?? fallbackRunId) but ignores the extracted chunkRunId; change the logic to prefer chunkRunId by calling ensureRunStarted(chunkRunId ?? streamRunId ?? fallbackRunId) and, when chunkRunId is present, set streamRunId = chunkRunId so streamRunId stays in sync; update the CUSTOM case in processStream (the switch handling chunk.type, around the GENERATION_EVENTS.RESULT handling) to use those symbols (chunkRunId, streamRunId, fallbackRunId) and call ensureRunStarted with the combined fallback.
🧹 Nitpick comments (2)
packages/typescript/ai-devtools/src/store/hook-registry.ts (1)
640-695: 💤 Low valueConsider handling duplicate run IDs from multiple hooks.
syncRunsFromSnapshotunconditionally overwritesexisting.hookIdwhen updating an existing run record (line 680). If two hooks share the same run ID (unlikely but possible in edge cases), the last snapshot processed wins, potentially causing incorrect hook→run associations.This is a minor edge case given the current architecture, but worth noting.
🤖 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/typescript/ai-devtools/src/store/hook-registry.ts` around lines 640 - 695, syncRunsFromSnapshot currently unconditionally overwrites existing.hookId which can misassociate runs if the same run ID appears for multiple hooks; change the update logic in syncRunsFromSnapshot so it does not overwrite existing.hookId when an existing run already has a different hookId (e.g., only set existing.hookId = snapshot.hookId if !existing.hookId or existing.hookId === snapshot.hookId), and rely on attachRunToHook(state, snapshot.hookId, run.id) to record the hook→run relationship (or extend the run model to track multiple hookIds if you need multi-association); update any consumers expecting a single hookId accordingly.packages/typescript/ai-devtools/src/components/hooks/HookDetails.tsx (1)
1226-1254: 💤 Low valueAnimation cleanup on unmount is not handled.
The
scrollAnimationsWeakMap (line 51) storessetTimeouthandles, but there's no cleanup when the component unmounts. IfHookDetailsunmounts while an animation is in progress, the timeout callback will still fire and attempt to modifyelement.scrollTopon a potentially detached element. While this won't crash (it's a no-op on detached elements), it's a minor resource leak.Consider using
onCleanupfrom Solid to clear any active animation timeouts when the component unmounts.🤖 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/typescript/ai-devtools/src/components/hooks/HookDetails.tsx` around lines 1226 - 1254, animateScrollTop currently stores timeouts in the module-scoped scrollAnimations WeakMap but provides no way for HookDetails to cancel them on unmount; change animateScrollTop to return the active timeout id (or a cancel function) when starting the animation and/or convert scrollAnimations to an iterable Map so pending timeouts can be enumerated, then import onCleanup in the HookDetails component and register a cleanup that clears any outstanding timeouts (window.clearTimeout) for that component (use the returned ids or iterate the Map entries keyed by the element) to ensure timeouts are cleared when HookDetails unmounts.
🤖 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/typescript/ai-client/src/devtools-noop.ts`:
- Around line 60-107: NoOpChatDevtoolsBridge is missing the public methods
mountWithTools, notifyToolsChanged, and recordStreamId from the
ChatDevtoolsBridge contract, causing runtime crashes; update the class to add
matching method signatures (mountWithTools(...), notifyToolsChanged(...),
recordStreamId(streamId: string | null): void) with no-op implementations and
correct parameter types so the no-op bridge fully implements the
ChatDevtoolsBridge surface (match the signatures defined in ChatDevtoolsBridge
in devtools.ts).
In `@packages/typescript/ai-client/src/devtools.ts`:
- Around line 1348-1368: ensureRunStarted currently skips renaming when
activeRunStarted is true, causing a stale provisional run to remain; fix by
moving the rename logic before the early-return and always calling
this.renameRun(this.activeRunId, runId) when this.activeRunId exists and !==
runId (regardless of activeRunStarted). Keep the early return if
this.activeRunStarted && this.activeRunId === runId, then proceed to set
this.activeRunId = runId, this.activeRunStarted = true, upsertRun(...
'generating' ...), and emitRunLifecycle/emitState so the renamed entry retains
the started lifecycle; reference ensureRunStarted, activeRunId,
activeRunStarted, and renameRun.
In `@packages/typescript/ai-devtools/src/components/hooks/GenerationPanel.tsx`:
- Around line 108-115: The auto-scroll createEffect is setting scrollTop on the
non-scrollable runsContainer (the generationRuns div) so it never scrolls;
either change the ref to target the actual overflow element (the outer pane
container used to hold generation runs—e.g., introduce runsScrollContainer and
attach the ref to that DOM node used for scrolling) or make the generationRuns
element itself the scroll container by adding overflow:auto/overflow-y:auto and
a max-height in the styles (use-styles.ts) so
runsContainer.scrollTop/scrollHeight in createEffect (and the identical logic at
lines 129-134) operate on the real scrollable element. Ensure the referenced
symbol names (runsContainer/generationRuns or the new runsScrollContainer) and
createEffect are updated accordingly.
---
Outside diff comments:
In `@packages/typescript/ai-client/src/generation-client.ts`:
- Around line 247-261: The CUSTOM branch currently calls
this.devtoolsBridge.ensureRunStarted(streamRunId ?? fallbackRunId) but ignores
the extracted chunkRunId; change the logic to prefer chunkRunId by calling
ensureRunStarted(chunkRunId ?? streamRunId ?? fallbackRunId) and, when
chunkRunId is present, set streamRunId = chunkRunId so streamRunId stays in
sync; update the CUSTOM case in processStream (the switch handling chunk.type,
around the GENERATION_EVENTS.RESULT handling) to use those symbols (chunkRunId,
streamRunId, fallbackRunId) and call ensureRunStarted with the combined
fallback.
---
Nitpick comments:
In `@packages/typescript/ai-devtools/src/components/hooks/HookDetails.tsx`:
- Around line 1226-1254: animateScrollTop currently stores timeouts in the
module-scoped scrollAnimations WeakMap but provides no way for HookDetails to
cancel them on unmount; change animateScrollTop to return the active timeout id
(or a cancel function) when starting the animation and/or convert
scrollAnimations to an iterable Map so pending timeouts can be enumerated, then
import onCleanup in the HookDetails component and register a cleanup that clears
any outstanding timeouts (window.clearTimeout) for that component (use the
returned ids or iterate the Map entries keyed by the element) to ensure timeouts
are cleared when HookDetails unmounts.
In `@packages/typescript/ai-devtools/src/store/hook-registry.ts`:
- Around line 640-695: syncRunsFromSnapshot currently unconditionally overwrites
existing.hookId which can misassociate runs if the same run ID appears for
multiple hooks; change the update logic in syncRunsFromSnapshot so it does not
overwrite existing.hookId when an existing run already has a different hookId
(e.g., only set existing.hookId = snapshot.hookId if !existing.hookId or
existing.hookId === snapshot.hookId), and rely on attachRunToHook(state,
snapshot.hookId, run.id) to record the hook→run relationship (or extend the run
model to track multiple hookIds if you need multi-association); update any
consumers expecting a single hookId accordingly.
🪄 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: 0155858d-bdcf-4372-ada6-f475bf7cde13
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (90)
docs/getting-started/devtools.mdexamples/ts-react-chat/src/routes/generations.structured-output.tsxpackages/typescript/ai-client/package.jsonpackages/typescript/ai-client/src/chat-client.tspackages/typescript/ai-client/src/devtools-noop.tspackages/typescript/ai-client/src/devtools.tspackages/typescript/ai-client/src/generation-client.tspackages/typescript/ai-client/src/generation-types.tspackages/typescript/ai-client/src/index.tspackages/typescript/ai-client/src/types.tspackages/typescript/ai-client/src/video-generation-client.tspackages/typescript/ai-client/tests/devtools.test.tspackages/typescript/ai-client/tests/use-real-devtools-bridges.tspackages/typescript/ai-client/vite.config.tspackages/typescript/ai-devtools/src/components/Shell.tsxpackages/typescript/ai-devtools/src/components/hooks/FixtureNamePopover.tsxpackages/typescript/ai-devtools/src/components/hooks/GenerationPanel.tsxpackages/typescript/ai-devtools/src/components/hooks/HookDashboard.tsxpackages/typescript/ai-devtools/src/components/hooks/HookDetails.tsxpackages/typescript/ai-devtools/src/components/hooks/ToolFixtureForm.tsxpackages/typescript/ai-devtools/src/components/hooks/hook-dashboard-model.tspackages/typescript/ai-devtools/src/store/hook-registry.tspackages/typescript/ai-devtools/src/styles/use-styles.tspackages/typescript/ai-devtools/tests/hook-dashboard-model.test.tspackages/typescript/ai-devtools/tests/hook-registry.test.tspackages/typescript/ai-event-client/src/index.tspackages/typescript/ai-event-client/tests/emit.test.tspackages/typescript/ai-preact/src/types.tspackages/typescript/ai-preact/src/use-chat.tspackages/typescript/ai-preact/tests/devtools-name-types.test.tspackages/typescript/ai-react/src/types.tspackages/typescript/ai-react/src/use-chat.tspackages/typescript/ai-react/src/use-generate-audio.tspackages/typescript/ai-react/src/use-generate-image.tspackages/typescript/ai-react/src/use-generate-speech.tspackages/typescript/ai-react/src/use-generate-video.tspackages/typescript/ai-react/src/use-generation.tspackages/typescript/ai-react/src/use-summarize.tspackages/typescript/ai-react/src/use-transcription.tspackages/typescript/ai-react/tests/devtools-name-types.test.tspackages/typescript/ai-solid/src/types.tspackages/typescript/ai-solid/src/use-chat.tspackages/typescript/ai-solid/src/use-generate-audio.tspackages/typescript/ai-solid/src/use-generate-image.tspackages/typescript/ai-solid/src/use-generate-speech.tspackages/typescript/ai-solid/src/use-generate-video.tspackages/typescript/ai-solid/src/use-generation.tspackages/typescript/ai-solid/src/use-summarize.tspackages/typescript/ai-solid/src/use-transcription.tspackages/typescript/ai-solid/tests/devtools-name-types.test.tspackages/typescript/ai-svelte/src/create-chat.svelte.tspackages/typescript/ai-svelte/src/create-generate-audio.svelte.tspackages/typescript/ai-svelte/src/create-generate-image.svelte.tspackages/typescript/ai-svelte/src/create-generate-speech.svelte.tspackages/typescript/ai-svelte/src/create-generate-video.svelte.tspackages/typescript/ai-svelte/src/create-generation.svelte.tspackages/typescript/ai-svelte/src/create-summarize.svelte.tspackages/typescript/ai-svelte/src/create-transcription.svelte.tspackages/typescript/ai-svelte/src/types.tspackages/typescript/ai-svelte/tests/devtools-name-types.test.tspackages/typescript/ai-vue/src/types.tspackages/typescript/ai-vue/src/use-chat.tspackages/typescript/ai-vue/src/use-generate-audio.tspackages/typescript/ai-vue/src/use-generate-image.tspackages/typescript/ai-vue/src/use-generate-speech.tspackages/typescript/ai-vue/src/use-generate-video.tspackages/typescript/ai-vue/src/use-generation.tspackages/typescript/ai-vue/src/use-summarize.tspackages/typescript/ai-vue/src/use-transcription.tspackages/typescript/ai-vue/tests/devtools-name-types.test.tspackages/typescript/react-ai-devtools/src/AiDevtools.tsxtesting/e2e/fixtures/audio-gen/basic.jsontesting/e2e/fixtures/tts/basic.jsontesting/e2e/package.jsontesting/e2e/src/components/DevtoolsHarness.tsxtesting/e2e/src/lib/devtools-test.tstesting/e2e/src/routeTree.gen.tstesting/e2e/src/routes/api.summarize.tstesting/e2e/src/routes/devtools-chat.tsxtesting/e2e/src/routes/devtools-generation-hooks.tsxtesting/e2e/src/routes/devtools-route-a.tsxtesting/e2e/src/routes/devtools-route-b.tsxtesting/e2e/src/routes/devtools-structured.tsxtesting/e2e/src/routes/devtools-tools.tsxtesting/e2e/tests/devtools-chat-structured.spec.tstesting/e2e/tests/devtools-generation-hooks.spec.tstesting/e2e/tests/devtools-helpers.tstesting/e2e/tests/devtools-registration.spec.tstesting/e2e/tests/devtools-responsive.spec.tstesting/e2e/tests/devtools-tools.spec.ts
✅ Files skipped from review due to trivial changes (1)
- docs/getting-started/devtools.md
Comment on lines
+60
to
+107
| export class NoOpChatDevtoolsBridge { | ||
| readonly events: ChatClientEventEmitter | ||
|
|
||
| constructor(options: ChatDevtoolsBridgeOptions) { | ||
| this.events = new NoOpChatClientEventEmitter(options.clientId) | ||
| } | ||
|
|
||
| // base bridge surface | ||
| emitRegistered(): void {} | ||
| emitUpdated(): void {} | ||
| emitSnapshot(): void {} | ||
| emitToolsRegistered(): void {} | ||
| emitRunLifecycle( | ||
| _eventType: unknown, | ||
| _runId: string, | ||
| _status: unknown, | ||
| _options?: { error?: string }, | ||
| ): void {} | ||
| deactivate(): void {} | ||
| supersede(): void {} | ||
| dispose(): void {} | ||
|
|
||
| // chat-specific surface | ||
| setCurrentStreamId(_streamId: string | null): void {} | ||
| getCurrentStreamId(): string | null { | ||
| return null | ||
| } | ||
| getLastStreamId(): string | null { | ||
| return null | ||
| } | ||
| resolveStreamId(): string { | ||
| return '' | ||
| } | ||
| observeChunk(_chunk: StreamChunk): void {} | ||
| beginRun(_runId: string, _threadId: string): void {} | ||
| getCurrentRunEventContext(): ChatClientRunEventContext | undefined { | ||
| return undefined | ||
| } | ||
| getCurrentOrLastRunEventContext(): ChatClientRunEventContext | undefined { | ||
| return undefined | ||
| } | ||
| findToolCallContext(toolCallId: string): ChatClientEventContext { | ||
| return { toolCallId } | ||
| } | ||
| async applyFixture(_fixture: AIDevtoolsToolFixture): Promise<void> { | ||
| // intentionally empty | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
No-op chat bridge is missing public methods from the real bridge contract.
Because the factory casts through unknown, type-checking won’t catch drift. NoOpChatDevtoolsBridge currently omits mountWithTools, notifyToolsChanged, and recordStreamId, which are present on ChatDevtoolsBridge (packages/typescript/ai-client/src/devtools.ts). Any call on the default no-op path can crash at runtime.
Suggested fix
export class NoOpChatDevtoolsBridge {
readonly events: ChatClientEventEmitter
constructor(options: ChatDevtoolsBridgeOptions) {
this.events = new NoOpChatClientEventEmitter(options.clientId)
}
// base bridge surface
emitRegistered(): void {}
emitUpdated(): void {}
emitSnapshot(): void {}
emitToolsRegistered(): void {}
@@
// chat-specific surface
+ mountWithTools(_initialMessageCount: number): void {}
+ notifyToolsChanged(): void {}
setCurrentStreamId(_streamId: string | null): void {}
+ recordStreamId(_streamId: string): void {}
getCurrentStreamId(): string | null {
return null
}🤖 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/typescript/ai-client/src/devtools-noop.ts` around lines 60 - 107,
NoOpChatDevtoolsBridge is missing the public methods mountWithTools,
notifyToolsChanged, and recordStreamId from the ChatDevtoolsBridge contract,
causing runtime crashes; update the class to add matching method signatures
(mountWithTools(...), notifyToolsChanged(...), recordStreamId(streamId: string |
null): void) with no-op implementations and correct parameter types so the no-op
bridge fully implements the ChatDevtoolsBridge surface (match the signatures
defined in ChatDevtoolsBridge in devtools.ts).
Comment on lines
+1348
to
+1368
| ensureRunStarted(runId: string): void { | ||
| if (this.activeRunStarted && this.activeRunId === runId) return | ||
|
|
||
| if ( | ||
| !this.activeRunStarted && | ||
| this.activeRunId && | ||
| this.activeRunId !== runId | ||
| ) { | ||
| this.renameRun(this.activeRunId, runId) | ||
| } | ||
|
|
||
| this.activeRunId = runId | ||
| this.activeRunStarted = true | ||
| this.upsertRun(runId, { | ||
| status: 'generating', | ||
| isLoading: true, | ||
| clearError: true, | ||
| }) | ||
| this.emitRunLifecycle('run:started', runId, 'started') | ||
| this.emitState() | ||
| } |
Contributor
There was a problem hiding this comment.
Rename the provisional run even after early updates have started it.
beginRun() creates a client-side run id, and callers now invoke ensureRunStarted(...) from early CUSTOM/progress events before RUN_STARTED arrives. If the server later sends a different runId, this branch skips renameRun() because activeRunStarted is already true, so one stale run stays stuck in generating while later patches move to a second run. Rename whenever activeRunId !== runId before swapping the active id, then keep the started lifecycle on that renamed entry. A regression test where a CUSTOM chunk precedes RUN_STARTED would lock this down.
💡 Minimal fix
ensureRunStarted(runId: string): void {
- if (this.activeRunStarted && this.activeRunId === runId) return
-
- if (
- !this.activeRunStarted &&
- this.activeRunId &&
- this.activeRunId !== runId
- ) {
+ if (this.activeRunId && this.activeRunId !== runId) {
this.renameRun(this.activeRunId, runId)
}
+ if (this.activeRunStarted && this.activeRunId === runId) return
this.activeRunId = runId
this.activeRunStarted = 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/typescript/ai-client/src/devtools.ts` around lines 1348 - 1368,
ensureRunStarted currently skips renaming when activeRunStarted is true, causing
a stale provisional run to remain; fix by moving the rename logic before the
early-return and always calling this.renameRun(this.activeRunId, runId) when
this.activeRunId exists and !== runId (regardless of activeRunStarted). Keep the
early return if this.activeRunStarted && this.activeRunId === runId, then
proceed to set this.activeRunId = runId, this.activeRunStarted = true,
upsertRun(... 'generating' ...), and emitRunLifecycle/emitState so the renamed
entry retains the started lifecycle; reference ensureRunStarted, activeRunId,
activeRunStarted, and renameRun.
Comment on lines
+108
to
+115
| createEffect(() => { | ||
| generationRuns().length | ||
| queueMicrotask(() => { | ||
| if (runsContainer) { | ||
| runsContainer.scrollTop = runsContainer.scrollHeight | ||
| } | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
Auto-scroll is targeting a non-scrollable wrapper.
runsContainer is the generationRuns div, but that class is still just a growing flex column in packages/typescript/ai-devtools/src/styles/use-styles.ts. Once this section overflows, the scrollable element is the outer pane, so this microtask won't actually keep the latest run in view. Point the ref at the real overflow container, or make generationRuns the scroll container itself.
Also applies to: 129-134
🤖 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/typescript/ai-devtools/src/components/hooks/GenerationPanel.tsx`
around lines 108 - 115, The auto-scroll createEffect is setting scrollTop on the
non-scrollable runsContainer (the generationRuns div) so it never scrolls;
either change the ref to target the actual overflow element (the outer pane
container used to hold generation runs—e.g., introduce runsScrollContainer and
attach the ref to that DOM node used for scrolling) or make the generationRuns
element itself the scroll container by adding overflow:auto/overflow-y:auto and
a max-height in the styles (use-styles.ts) so
runsContainer.scrollTop/scrollHeight in createEffect (and the identical logic at
lines 129-134) operate on the real scrollable element. Ensure the referenced
symbol names (runsContainer/generationRuns or the new runsScrollContainer) and
createEffect are updated accordingly.
- Sort imports alphabetically across ai-client, ai-react, ai-solid, ai-vue, and ai-devtools-core - Drop leading underscore from NoOpGenerationDevtoolsBridge type parameter (used in constructor signature) - Disable no-restricted-syntax on intentional `as unknown as` casts in no-op bridge factories and generation result assignment, with reasons - Remove unnecessary type assertion in devtools.ts (auto-fixed)
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-client/src/video-generation-client.ts (1)
284-305:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
VIDEO_*CUSTOM event payloads before updating client/devtools state
parseSSEResponsedeserializes SSE chunks withJSON.parse(no runtime schema checks), andvideo-generation-client.tsthenas-castschunk.valueforvideo:job:created,video:status,generation:result, andgeneration:progressbefore callingsetJobId,setVideoStatus,setResult, andsetProgress. Add Zod schemas for these CUSTOM payloads andsafeParsethem (fail fast or ignore invalid events) to prevent malformed/version-skewed payloads from corrupting state.🤖 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/typescript/ai-client/src/video-generation-client.ts` around lines 284 - 305, The CUSTOM-case is trusting JSON-parsed chunk.value and casting it for VIDEO_JOB_CREATED, VIDEO_STATUS, RESULT, and PROGRESS which can corrupt client/devtools state on malformed payloads; add Zod schemas (e.g., VideoJobCreatedSchema, VideoStatusSchema, VideoResultSchema, VideoProgressSchema) and import zod, then replace the unchecked `as` casts inside the case 'CUSTOM' branch of video-generation-client.ts with safe parses (schema.safeParse(chunk.value)) and only call setJobId, setVideoStatus, setResult, setProgress, and callbacks when safeParse succeeds (otherwise ignore/log and fail fast if appropriate); reference the existing functions/methods setJobId, setVideoStatus, setResult, setProgress, the GENERATION_EVENTS names, and VideoStatusInfo/VideoGenerateResult types to guide schema shapes.packages/typescript/ai-client/src/generation-client.ts (1)
258-268:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate streamed
GENERATION_EVENTS.PROGRESSpayloads instead of type-casting
processStreamwriteschunk.valueto progress state/devtools with no runtime checks in bothpackages/typescript/ai-client/src/generation-client.tsandpackages/typescript/ai-client/src/video-generation-client.ts;parseSSEResponseonlyJSON.parses, so malformed progress payloads can slip through. Implement protocol-boundary validation (e.g., a Zod schema) and ensurezodis available at runtime (move fromdevDependenciestodependencies), or reuse existing runtime validators (e.g.,numberField/stringFieldpatterns fromdevtools.ts).🤖 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/typescript/ai-client/src/generation-client.ts` around lines 258 - 268, The GENERATION_EVENTS.PROGRESS branch in generation-client.ts currently type-casts chunk.value and calls setProgress without runtime validation; update processStream handling (the 'CUSTOM' case where chunk.name === GENERATION_EVENTS.PROGRESS) to validate chunk.value against a runtime schema before calling setProgress (e.g., require { progress: number, message?: string }) — either add a Zod schema and move zod from devDependencies to dependencies so it’s available at runtime, or reuse the existing runtime validators (numberField/stringField) from devtools.ts to validate progress and message; ensure parseSSEResponse consumers (including video-generation-client.ts) use the same validation and reject or log malformed payloads instead of blindly calling setProgress.
♻️ Duplicate comments (2)
packages/typescript/ai-client/src/devtools-noop.ts (1)
60-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the missing no-op chat bridge methods.
createNoOpChatDevtoolsBridge()still casts this class toChatDevtoolsBridge, butmountWithTools,notifyToolsChanged, andrecordStreamIdare not implemented here. Any consumer on the default no-op path that calls those methods will throw at runtime.Minimal patch
export class NoOpChatDevtoolsBridge { readonly events: ChatClientEventEmitter constructor(options: ChatDevtoolsBridgeOptions) { this.events = new NoOpChatClientEventEmitter(options.clientId) } // base bridge surface emitRegistered(): void {} emitUpdated(): void {} emitSnapshot(): void {} emitToolsRegistered(): void {} @@ // chat-specific surface + mountWithTools(_initialMessageCount: number): void {} + notifyToolsChanged(): void {} setCurrentStreamId(_streamId: string | null): void {} + recordStreamId(_streamId: string): void {} getCurrentStreamId(): string | null { return null }Also applies to: 164-171
🤖 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/typescript/ai-client/src/devtools-noop.ts` around lines 60 - 107, NoOpChatDevtoolsBridge is missing no-op implementations for mountWithTools, notifyToolsChanged, and recordStreamId even though createNoOpChatDevtoolsBridge casts it to ChatDevtoolsBridge; add empty/no-op method stubs named mountWithTools(...), notifyToolsChanged(...), and recordStreamId(streamId: string | null) (matching the ChatDevtoolsBridge signatures) to the NoOpChatDevtoolsBridge class so callers won't throw at runtime, keeping their bodies intentionally empty or returning appropriate no-op values.packages/typescript/ai-client/src/devtools.ts (1)
1347-1367:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRename provisional runs before the started-run early return.
ensureRunStarted()still only renames whileactiveRunStartedis false. If an early update starts the provisional run andRUN_STARTEDlater arrives with a differentrunId, one stale run stays ingeneratingand later updates attach to a second run.Minimal patch
ensureRunStarted(runId: string): void { - if (this.activeRunStarted && this.activeRunId === runId) return - - if ( - !this.activeRunStarted && - this.activeRunId && - this.activeRunId !== runId - ) { + if (this.activeRunId && this.activeRunId !== runId) { this.renameRun(this.activeRunId, runId) } + if (this.activeRunStarted && this.activeRunId === runId) return this.activeRunId = runId this.activeRunStarted = 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/typescript/ai-client/src/devtools.ts` around lines 1347 - 1367, The rename of provisional runs should happen before the early-return so stale runs don't remain; inside ensureRunStarted, detect when this.activeRunId exists and differs from the incoming runId and call renameRun(this.activeRunId, runId) before returning, rather than only when activeRunStarted is false. In short: in ensureRunStarted, move/adjust the block that calls renameRun to run whenever this.activeRunId && this.activeRunId !== runId (regardless of this.activeRunStarted) and then keep the existing early-return for the case this.activeRunStarted && this.activeRunId === runId; references: ensureRunStarted, renameRun, activeRunId, activeRunStarted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/typescript/ai-client/src/generation-client.ts`:
- Around line 258-268: The GENERATION_EVENTS.PROGRESS branch in
generation-client.ts currently type-casts chunk.value and calls setProgress
without runtime validation; update processStream handling (the 'CUSTOM' case
where chunk.name === GENERATION_EVENTS.PROGRESS) to validate chunk.value against
a runtime schema before calling setProgress (e.g., require { progress: number,
message?: string }) — either add a Zod schema and move zod from devDependencies
to dependencies so it’s available at runtime, or reuse the existing runtime
validators (numberField/stringField) from devtools.ts to validate progress and
message; ensure parseSSEResponse consumers (including
video-generation-client.ts) use the same validation and reject or log malformed
payloads instead of blindly calling setProgress.
In `@packages/typescript/ai-client/src/video-generation-client.ts`:
- Around line 284-305: The CUSTOM-case is trusting JSON-parsed chunk.value and
casting it for VIDEO_JOB_CREATED, VIDEO_STATUS, RESULT, and PROGRESS which can
corrupt client/devtools state on malformed payloads; add Zod schemas (e.g.,
VideoJobCreatedSchema, VideoStatusSchema, VideoResultSchema,
VideoProgressSchema) and import zod, then replace the unchecked `as` casts
inside the case 'CUSTOM' branch of video-generation-client.ts with safe parses
(schema.safeParse(chunk.value)) and only call setJobId, setVideoStatus,
setResult, setProgress, and callbacks when safeParse succeeds (otherwise
ignore/log and fail fast if appropriate); reference the existing
functions/methods setJobId, setVideoStatus, setResult, setProgress, the
GENERATION_EVENTS names, and VideoStatusInfo/VideoGenerateResult types to guide
schema shapes.
---
Duplicate comments:
In `@packages/typescript/ai-client/src/devtools-noop.ts`:
- Around line 60-107: NoOpChatDevtoolsBridge is missing no-op implementations
for mountWithTools, notifyToolsChanged, and recordStreamId even though
createNoOpChatDevtoolsBridge casts it to ChatDevtoolsBridge; add empty/no-op
method stubs named mountWithTools(...), notifyToolsChanged(...), and
recordStreamId(streamId: string | null) (matching the ChatDevtoolsBridge
signatures) to the NoOpChatDevtoolsBridge class so callers won't throw at
runtime, keeping their bodies intentionally empty or returning appropriate no-op
values.
In `@packages/typescript/ai-client/src/devtools.ts`:
- Around line 1347-1367: The rename of provisional runs should happen before the
early-return so stale runs don't remain; inside ensureRunStarted, detect when
this.activeRunId exists and differs from the incoming runId and call
renameRun(this.activeRunId, runId) before returning, rather than only when
activeRunStarted is false. In short: in ensureRunStarted, move/adjust the block
that calls renameRun to run whenever this.activeRunId && this.activeRunId !==
runId (regardless of this.activeRunStarted) and then keep the existing
early-return for the case this.activeRunStarted && this.activeRunId === runId;
references: ensureRunStarted, renameRun, activeRunId, activeRunStarted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbc38b0a-0e15-472c-855a-f316a27d1147
📒 Files selected for processing (8)
packages/typescript/ai-client/src/devtools-noop.tspackages/typescript/ai-client/src/devtools.tspackages/typescript/ai-client/src/generation-client.tspackages/typescript/ai-client/src/video-generation-client.tspackages/typescript/ai-devtools/src/components/hooks/HookDetails.tsxpackages/typescript/ai-react/src/types.tspackages/typescript/ai-solid/src/types.tspackages/typescript/ai-vue/src/types.ts
| }), | ||
| ) | ||
| } catch { | ||
| localStorage.removeItem(fixturesStorageKey) |
Contributor
There was a problem hiding this comment.
Critical: silently destroys user-saved fixtures on any parse error.
A transient JSON.parse failure, schema drift after a future migration, or even a quota error during removeItem will silently wipe the user's entire saved-fixture library — no log, no toast, no Sentry surface. They'll relaunch devtools and wonder where their fixtures went.
Per CLAUDE.md: "Don't create fallback code. It hides problems. Just display errors to the user."
Suggest at minimum logging the error, and reconsider whether removeItem is the right reaction (keeping the bad payload around for inspection is often safer than nuking it).
| threadId, | ||
| output: null, | ||
| errorText: | ||
| error instanceof Error ? error.message : 'Tool execution failed.', |
Contributor
There was a problem hiding this comment.
Critical: tool-fixture execution errors are flattened into a generic string with no logging.
This catch swallows everything thrown by executeFunc (and anything thrown by addToolResultForFixture itself — event emit, processor mutation, etc.). Non-Error throws are coerced to "Tool execution failed." with no stack, no console.error, no error event. The devtools UI shows a fixture marked output-error with nothing actionable.
At minimum: log the error before reporting it, and narrow the try to executeFunc(fixture.input) only so unrelated bugs in addToolResultForFixture don't masquerade as tool failures.
Per CLAUDE.md: "Don't create fallback code. It hides problems. Just display errors to the user."
| if (state.seenEventIds[key]) { | ||
| return false | ||
| } | ||
| state.seenEventIds[key] = true |
Contributor
There was a problem hiding this comment.
Critical: seenEventIds grows unboundedly for the life of a session.
Every devtools event inserts a key here at line 209, and the only place entries are ever removed is clearHookRegistry (line 377). In a long-running app — especially one with frequent streaming runs / tool chunks — this map will accumulate millions of keys, all of which are retained by the Solid store + immer produce copies on every state update. That's a real memory leak, not a theoretical one.
Options:
- LRU/ring buffer capped at e.g. 10k entries
- Evict keys associated with a hook when
hook:unregisteredfires - Evict keys older than some window when a new event is recorded
Whichever you pick, the dedupe contract should be made explicit ("events older than X minutes may dedupe-miss") so consumers don't depend on perfect global dedupe forever.
- log + preserve raw localStorage payload on fixture parse failure instead of silently calling removeItem - log + report full error name/message from tool-fixture execute, and narrow the try block to executeFunc only so unrelated bugs in addToolResultForFixture don't masquerade as tool failures - cap seenEventIds at 10k via FIFO ring buffer to prevent unbounded memory growth over the lifetime of long-running sessions; also reset the order array in clearHookRegistry - restore ai-code-mode/vite.config.ts to main (rename detection swapped it with ai-client's during the merge); ai-client now correctly declares devtools.ts as a separate entry
Contributor
Author
|
Synced with main (merge — directory-rename made a true rebase impractical) and pushed PR feedback fixes in b12231a:
|
Add a mandatory pre-PR quality gate to CLAUDE.md and a new AGENTS.md (cross-agent file) documenting that `pnpm test:pr` — the exact target set the CI `PR` workflow runs — must be run and pass locally before pushing. Falling back on CI as the first signal wastes review cycles when a quick local run would have caught the failure.
…types + comments)
Critical
- ToolFixtureForm: remove dead `'{}' ? : '{}'` ternary leftover from refactor
- ChatDevtoolsBridge.executeFixture: warn when execute=true but no client tool
is registered instead of silently substituting the saved output
- stringifyFixtureValue / parseFixtureResultContent: log + report the original
error instead of swallowing JSON.stringify / JSON.parse failures
- devtools-middleware: replace `chunk: any` with a mirrored discriminated
union (DevtoolsKnownChunk) + isKnownChunk type guard, drop the `as` cast
on chunk.error, fall back with the raw chunk payload instead of "Unknown
error" when RUN_ERROR.message is missing
- normalizeRunStatusFromSnapshot: return `null` for unknown statuses and let
the caller skip + warn instead of silently coercing them to `'updated'`
Important
- Remove unreachable `'unmounted'` HookLifecycle variant + dead filters
- Drop unused `runs` prop on GenerationPanel
- removeHookRecord: also clean up TimelineEvents attached to the hook
- IterationTimeline `totalUsage`: compare on `totalTokens` (not
prompt+completion) so providers that include reasoning tokens in
totalTokens don't get their max overwritten by a smaller later reading
- jsonSafeValue / stringifyToolArguments: log + return an explicit
placeholder instead of returning the original on failure
- No-op bridges: add compile-time parity checks (Exclude<keyof, keyof>)
so adding a public method to the real bridge fails the build until the
no-op is updated, instead of becoming a runtime TypeError
- devtools-middleware: wrap every emit with `safeEmit` so a subscriber
throw can't bubble into the host chat engine
- markEventSeen: warn when a dedupe key falls back entirely to literal
sentinels; tools:registered: warn when hookName is missing
- hook-registry tests: cover snapshot run backfill (success/error/idle/
unknown statuses) and lifecycle inference from snapshot state
Type design
- AIDevtoolsGenerationRunStatus union ('idle' | 'generating' | 'success'
| 'error' | 'cancelled') replacing the previous `status: string` and
threaded through GenerationDevtoolsCoreState / SnapshotBase / RunPatch
- Drop redundant `unknown | null` on input / videoStatus fields
- Remove `extends Record<string, unknown>` from public snapshot interfaces;
ClientDevtoolsBridge constraint relaxed to `extends object` and the
wire-envelope widening is pushed to emitSnapshot
- Drop pointless `as Record<string, unknown>` casts at JsonTree call sites
(JsonTree's TData accepts arbitrary value)
Comment hygiene
- Strip file-top docblocks and class-banner JSDoc rewriting the PR
description (devtools-noop, use-real-devtools-bridges, devtools-system-
prompt-mirror.test, several blocks in devtools.ts)
- Remove "for backward compatibility" / "exactly like it did before the
devtools work landed" PR-narrative phrases
- Drop ASCII section-banner separators in devtools.ts, use-styles.ts,
ai-context.tsx
- Compress multi-line JSDoc on short methods to one-liners or remove
where the name was self-describing
- Strip obvious line-narration comments in ai-context.tsx batching helpers
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
useChatTest Plan
pnpm formatpnpm test/generations/structured-outputregistersuseChatas a Structured hook in TanStack AI devtoolsSummary by CodeRabbit
New Features
Documentation