feat(message): extract core engine and migrate useMessage to adapters#325
Open
gene9831 wants to merge 16 commits intoopentiny:developfrom
Open
feat(message): extract core engine and migrate useMessage to adapters#325gene9831 wants to merge 16 commits intoopentiny:developfrom
gene9831 wants to merge 16 commits intoopentiny:developfrom
Conversation
WalkthroughAdds a shared message engine (createMessageEngine), Vue and native message adapters, core types and plugins (length, thinking, tool), test utilities and suites, documentation/type updates (including removal of fallbackRolePlugin), new exports including a ./core entry, and several utilities (mock providers, subscription controller, delta combiner). Changes
Sequence Diagram(s)sequenceDiagram
participant Client as UI / useMessage
participant Engine as createMessageEngine
participant Adapter as MessageStateAdapter
participant Provider as ResponseProvider
participant Plugins as Plugin Pipeline
Client->>Engine: sendMessage(content)
Engine->>Adapter: mutate(append user message)
Engine->>Plugins: onTurnStart
Engine->>Plugins: onBeforeRequest(requestBody)
Engine->>Provider: responseProvider(requestBody, signal)
loop stream chunks
Provider->>Engine: yield chunk
Engine->>Adapter: mutate(update assistant message)
Engine->>Plugins: onCompletionChunk(chunk, context)
end
Engine->>Plugins: onAfterRequest(context)
Plugins-->>Engine: may append messages or call requestNext
Engine->>Client: final state / publish updates
sequenceDiagram
participant Engine as createMessageEngine
participant Provider as ResponseProvider
participant ToolPlugin as toolPlugin
participant Adapter as MessageStateAdapter
Engine->>Provider: stream completion (may include tool_calls)
Provider->>Engine: yields chunk with tool_calls
Engine->>ToolPlugin: onAfterRequest(detect tool_calls)
ToolPlugin->>Adapter: append tool messages
ToolPlugin->>ToolPlugin: callTool (per tool_call) -> stream tool output
ToolPlugin->>Adapter: stream tool outputs into tool message (update metadata)
ToolPlugin->>Engine: notify completion / requestNext as needed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Preview build completed successfully!
|
Contributor
📦 Package Previewpnpm add https://pkg.pr.new/@opentiny/tiny-robot@fba5642 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-kit@fba5642 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-svgs@fba5642 commit: fba5642 |
… and new utility functions
…date message handling plugins
… and update imports
…response handling for message engine
…changes and remove fallbackRolePlugin references
…thod for runtime adaptability
…dating message exports
gene9831
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (6)
packages/kit/src/message/core/index.ts (1)
1-1: Consider also re-exporting core types from this barrel.Consumers of
./corebarrel (viasrc/core.ts) already get types through./message/types, but this sub-barrel only exports the factory. If you intendmessage/core/index.tsto be a standalone entrypoint, also re-exporting engine-related types (e.g.,MessageEngine,CreateMessageEngineOptions) from./enginehere would avoid forcing callers to reach into./enginedirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/message/core/index.ts` at line 1, The barrel currently only re-exports createMessageEngine; export the engine-related types as well so consumers can import them from this sub-barrel. Update message/core/index.ts to re-export MessageEngine and CreateMessageEngineOptions (and any other exported types from ./engine) alongside createMessageEngine so callers don't need to import directly from './engine'.packages/kit/package.json (1)
34-48: Minor: consider dropping legacy top-levelmain/module/typesor keeping them consistent withexports.With the
exportsmap now defined, modern tooling will prefer it overmain/module/types. The legacy fields at lines 34-36 still point atdist/index.*so they're consistent, but they're now largely redundant. Keeping them is fine for older bundlers; just something to track for a future cleanup. No change required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/package.json` around lines 34 - 48, The package.json currently defines legacy top-level fields "main", "module", and "types" that duplicate the newer "exports" map; either remove these legacy fields ("main", "module", "types") to avoid redundancy, or ensure they remain perfectly consistent with the "exports" entries (pointing to dist/index.js, dist/index.mjs, dist/index.d.ts respectively) so tooling that still reads them sees the same entrypoints as the "exports" map.packages/kit/src/message/adapters/index.ts (1)
1-2: Vue adapter leaks into native-only consumers via this barrel.Because
src/core.tsdoesexport * from './message/adapters', any consumer importing from@opentiny/tiny-robot-kit/core(even for native-only usage) transitively pulls in./vue, which imports fromvue. WithsideEffects: falseand tree-shaking this is usually fine, but it couples the core subpath to thevuepeer dependency. Consider splitting into./core/nativeand./core/vuesubpath exports so a pure-native consumer never needsvueinstalled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/message/adapters/index.ts` around lines 1 - 2, The barrel index currently re-exports both './native' and './vue' (export * from './native'; export * from './vue'), which causes Vue to be pulled into native-only consumers via core's export of the adapters; to fix, split the adapters barrel into distinct subpath exports (e.g., keep an adapters/native index that exports './native' and create adapters/vue that exports './vue'), update the top-level re-exports (e.g., the symbols exported from core.ts) to only re-export the native adapters (so core continues to export adapters/native but not adapters/vue), and leave a separate public subpath for the vue adapter so consumers that need Vue can import from the explicit vue subpath; ensure filenames/exports referencing 'native' and 'vue' match the new module names so tree-shaking and peer-dependency boundaries are preserved.packages/kit/src/vue/message/plugins/lengthPlugin.ts (1)
5-21: Consider centralizingVuePluginRuntimeand__corePluginFactorytyping.The
VuePluginRuntimetype and the__corePluginFactoryfield are duplicated verbatim acrossvue/message/plugins/{thinkingPlugin,lengthPlugin,toolPlugin}.ts, and the shape is also assumed by theuseMessageruntime on the consumer side. Hoisting this into a shared module (e.g.packages/kit/src/vue/message/plugins/runtime.tsor the Vue types file) would remove theas UseMessagePlugincasts needed to smuggle in the__corePluginFactoryproperty and keep the bridging contract single-sourced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/vue/message/plugins/lengthPlugin.ts` around lines 5 - 21, The VuePluginRuntime type and the __corePluginFactory field are duplicated; extract the VuePluginRuntime type (including the createCorePlugin signature) and a shared interface/type for plugins that expose __corePluginFactory into a single shared module, then update lengthPlugin (and the sibling files thinkingPlugin and toolPlugin) to import those shared types and use them directly so you can return the plugin object without using the as UseMessagePlugin cast; specifically update the lengthPlugin function that constructs lengthPlugin and its call to createCoreLengthPlugin to reference the shared runtime type and shared plugin interface so the bridging contract matches the useMessage runtime.packages/kit/src/vue/message/types.ts (1)
80-83: Align VueResponseProviderwith core type definition.Vue's
ResponseProvider(line 80) lacks the generic constraintT extends ChatCompletion | ChatCompletionChunkand defaults toChatCompletiononly. Core'sResponseProvider(packages/kit/src/message/types.ts:38) constrains the type parameter and defaults to the union. This divergence means streaming providers yieldingChatCompletionChunkwithout explicit generics will silently use the wrong type default, bypassing type safety intended for the primary streaming use case.Consider updating the Vue definition to match the core type signature, or re-exporting the core type for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/vue/message/types.ts` around lines 80 - 83, The Vue type ResponseProvider currently defaults T to ChatCompletion and lacks the generic constraint allowing ChatCompletionChunk; update ResponseProvider<T = ChatCompletion | ChatCompletionChunk> to constrain T extends ChatCompletion | ChatCompletionChunk (matching the core signature) or re-export the core ResponseProvider type to ensure consistency; change the declaration for ResponseProvider (which accepts MessageRequestBody and AbortSignal) to use the constrained generic so streaming providers that yield ChatCompletionChunk use the correct default and type checks.packages/kit/src/vue/message/mockResponseProvider.ts (1)
11-16: Keep mock completions checked against the SDK shape.
createMockCompletionis meant to produce aChatCompletion, but the unannotated return object plusfinish_reason?: string | nulllets invalid fixture shapes compile. Pin the helper to the OpenAI type so tests fail when the mock drifts from the real response contract.Type the mock fixture against ChatCompletion
-import type { ChatCompletionChunk } from 'openai/resources/index' +import type { ChatCompletion, ChatCompletionChunk } from 'openai/resources/index' @@ - finish_reason?: string | null + finish_reason?: ChatCompletion.Choice['finish_reason'] @@ -function createMockCompletion(step: MockResponseStep, index: number) { +function createMockCompletion(step: MockResponseStep, index: number): ChatCompletion {Also applies to: 46-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/vue/message/mockResponseProvider.ts` around lines 11 - 16, The mock fixtures are not strongly typed against the real OpenAI response shape; update MockResponseStep and the createMockCompletion helper to use the OpenAI ChatCompletion types so invalid shapes fail to compile. Specifically, change the finish_reason and other fields to match ChatCompletion (for example using ChatCompletion and ChatCompletion['choices'][number] types) and annotate createMockCompletion to return ChatCompletion; ensure onRequest and tool_calls still match their expected types while aligning the mock fields with ChatCompletion's exact property types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kit/src/core.ts`:
- Line 5: The export name "combileDeltaData" is misspelled; rename the function
symbol in the implementation to combineDeltaData (update its declaration in
message utils where combileDeltaData is declared) and update all places that
import or call it: change exports in core.ts to export { combineDeltaData,
normalizeToAsyncGenerator }, update imports in core/engine.ts and
plugins/toolPlugin.ts to import combineDeltaData instead of combileDeltaData,
and adjust any internal usages inside message/utils.ts (previous
combileDeltaData references) to the new name so all callers compile.
In `@packages/kit/src/message/adapters/native.ts`:
- Around line 19-30: The initialize function silently no-ops when called after
initial setup, which can hide binding/usage bugs; change initialize to throw an
error if initialized is already true (instead of returning) so repeated calls
immediately surface misuse — update the initialize implementation (check the
initialized flag and throw a descriptive Error mentioning initialize/initialized
and the adapter) and ensure behavior remains consistent with getState() and
mutate() which already throw when uninitialized.
- Around line 45-66: notifyListeners currently iterates the live Set listeners
which is not reentrancy-safe (a listener can subscribe/unsubscribe during
notification); fix by iterating a snapshot copy instead: capture snapshot =
getState() and make a stable array copy of listeners (e.g.,
Array.from(listeners) or [...listeners]) before the for loop, then iterate that
copy while preserving the existing kind-filtering logic (use entry.kinds,
entry.listener, MessageUpdateKinds and the existing kinds Set) so mutations to
the original listeners Set during notification won't affect this iteration.
In `@packages/kit/src/message/adapters/vue.ts`:
- Around line 84-120: The recipe can call skipNotify to defer reactive
notifications, but the current code returns early when notifySkipped is true
which prevents emitting a single atomic snapshot; change the flow so after
calling recipe(draft, skipNotify) you detect notifySkipped and instead of
returning immediately you clear notifySkipped and trigger one consolidated
notification (e.g., touch/assign the refs once) so subscribers to requestState
see a single atomic update for requestState+processingState; update the block
around recipe, skipNotify, notifySkipped and the later updateKinds handling
(symbols: recipe, skipNotify, notifySkipped, draft.requestState/requesting,
draft.processingState, messages and updateKinds) to perform one post-recipe
dispatch rather than letting Vue watchers run twice.
In `@packages/kit/src/message/core/engine.ts`:
- Around line 187-192: The call to responseProvider(requestBody, abortSignal)
and subsequent iteration over normalizeToAsyncGenerator(result) can hang if the
provider or generator ignores the abortSignal; wrap the provider invocation and
the async-iteration in abort-safe races (e.g., use a makeAbortable helper or
equivalent) so the provider call (responseProvider) is raced against abortSignal
and the async generator returned by normalizeToAsyncGenerator(result) is
consumed via an abortable wrapper (or by racing each iterator read against
abortSignal). Apply the same pattern for the other streaming block (the code
handling the second provider/stream around the other streaming loop) so both
provider invocation and each async iterator read are abort-safe.
In `@packages/kit/src/message/plugins/thinkingPlugin.ts`:
- Line 15: Replace the fallback using logical OR when computing
reasoning_content so empty strings are treated as valid; in thinkingPlugin.ts
update the assignment for the reasoning_content variable (currently using
c?.message?.reasoning_content || c?.delta?.reasoning_content) to use nullish
coalescing (??) instead, i.e. fall back to c?.delta?.reasoning_content only when
message.reasoning_content is null or undefined.
- Around line 45-64: The onTurnEnd guard is comparing the current thinking value
against itself so it often short-circuits and fails to reset thinking/open;
change the condition in onTurnEnd to detect whether we need to force-reset to
false by comparing the current state to the target state (thinking: false, open:
false) instead of Boolean(lastMessage.state.thinking). Specifically, inside
onTurnEnd, use stateShouldUpdate(lastMessage.state, { thinking: false, open:
false }) (or otherwise compute current !== target) before calling
mutate('messages') and then set lastMessage.state.thinking = false and
lastMessage.state.open = false; this ensures non-streamed or aborted turns get
their thinking/open flags cleared.
In `@packages/kit/src/message/plugins/toolPlugin.ts`:
- Around line 296-317: The current check uses toolMessage.content.length === 0
so when earlier object-chunks are merged (producing strings like "{}" or
"{\"partial\":...}") a failure will leave visible empty/partial JSON instead of
the failure text; fix by tracking whether any successful chunk was actually
produced (e.g., add a boolean flag producedChunk on the toolCall or toolMessage
and set it inside the chunk-merge code where mutate(...) updates toolMessage
content), then in the catch use that flag (or normalize content to treat "" and
"{}" as empty) before assigning toolCallFailedContent; keep toolCallEnd, mutate
and toolMessage assignments behavior otherwise.
- Around line 213-222: The optional chaining on getTools is dead because
getTools is required in the options type; call it directly instead of using
getTools?.() in the onBeforeRequest handler, i.e. replace getTools?.() with
getTools(), and keep the rest of the logic that assigns requestBody.tools and
returns restOptions.onBeforeRequest?.(context); alternatively, if the intent was
to allow missing getTools, make getTools optional in the options type (and where
it’s destructured) to match the runtime check.
- Around line 296-324: The code calls requestNext() unconditionally after
awaiting toolCallPromises, which can trigger a recursive executeRequest with an
already-aborted abortSignal; update the flow to guard the requestNext()
invocation by checking abortSignal.aborted (or a local cancelled flag set in the
catch branch) and only call requestNext() when not aborted, ensuring
toolCallEnd(...) stays as-is for cancellation/failed cases and avoiding phantom
follow-up requests.
In `@packages/kit/src/message/test/mockResponseProvider.ts`:
- Around line 12-44: In mockStreamOneAssistantReplyWithDelay update the
signature to mark delay optional (e.g., { abortSignal, delay?: number }) and
replace the plain setTimeout sleep with an abort-aware race: create a Promise
that sets a timeout and also registers an abortSignal 'abort' listener (with {
once: true }) that clears the timeout and rejects with an AbortError so the
generator will immediately throw if aborted during the delay; ensure you remove
the event listener/clear the timeout on resolve/reject to avoid leaks and keep
the existing top-of-loop abort check as well.
---
Nitpick comments:
In `@packages/kit/package.json`:
- Around line 34-48: The package.json currently defines legacy top-level fields
"main", "module", and "types" that duplicate the newer "exports" map; either
remove these legacy fields ("main", "module", "types") to avoid redundancy, or
ensure they remain perfectly consistent with the "exports" entries (pointing to
dist/index.js, dist/index.mjs, dist/index.d.ts respectively) so tooling that
still reads them sees the same entrypoints as the "exports" map.
In `@packages/kit/src/message/adapters/index.ts`:
- Around line 1-2: The barrel index currently re-exports both './native' and
'./vue' (export * from './native'; export * from './vue'), which causes Vue to
be pulled into native-only consumers via core's export of the adapters; to fix,
split the adapters barrel into distinct subpath exports (e.g., keep an
adapters/native index that exports './native' and create adapters/vue that
exports './vue'), update the top-level re-exports (e.g., the symbols exported
from core.ts) to only re-export the native adapters (so core continues to export
adapters/native but not adapters/vue), and leave a separate public subpath for
the vue adapter so consumers that need Vue can import from the explicit vue
subpath; ensure filenames/exports referencing 'native' and 'vue' match the new
module names so tree-shaking and peer-dependency boundaries are preserved.
In `@packages/kit/src/message/core/index.ts`:
- Line 1: The barrel currently only re-exports createMessageEngine; export the
engine-related types as well so consumers can import them from this sub-barrel.
Update message/core/index.ts to re-export MessageEngine and
CreateMessageEngineOptions (and any other exported types from ./engine)
alongside createMessageEngine so callers don't need to import directly from
'./engine'.
In `@packages/kit/src/vue/message/mockResponseProvider.ts`:
- Around line 11-16: The mock fixtures are not strongly typed against the real
OpenAI response shape; update MockResponseStep and the createMockCompletion
helper to use the OpenAI ChatCompletion types so invalid shapes fail to compile.
Specifically, change the finish_reason and other fields to match ChatCompletion
(for example using ChatCompletion and ChatCompletion['choices'][number] types)
and annotate createMockCompletion to return ChatCompletion; ensure onRequest and
tool_calls still match their expected types while aligning the mock fields with
ChatCompletion's exact property types.
In `@packages/kit/src/vue/message/plugins/lengthPlugin.ts`:
- Around line 5-21: The VuePluginRuntime type and the __corePluginFactory field
are duplicated; extract the VuePluginRuntime type (including the
createCorePlugin signature) and a shared interface/type for plugins that expose
__corePluginFactory into a single shared module, then update lengthPlugin (and
the sibling files thinkingPlugin and toolPlugin) to import those shared types
and use them directly so you can return the plugin object without using the as
UseMessagePlugin cast; specifically update the lengthPlugin function that
constructs lengthPlugin and its call to createCoreLengthPlugin to reference the
shared runtime type and shared plugin interface so the bridging contract matches
the useMessage runtime.
In `@packages/kit/src/vue/message/types.ts`:
- Around line 80-83: The Vue type ResponseProvider currently defaults T to
ChatCompletion and lacks the generic constraint allowing ChatCompletionChunk;
update ResponseProvider<T = ChatCompletion | ChatCompletionChunk> to constrain T
extends ChatCompletion | ChatCompletionChunk (matching the core signature) or
re-export the core ResponseProvider type to ensure consistency; change the
declaration for ResponseProvider (which accepts MessageRequestBody and
AbortSignal) to use the constrained generic so streaming providers that yield
ChatCompletionChunk use the correct default and type checks.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 773679df-9210-4568-b7b1-bb43943228c3
📒 Files selected for processing (29)
docs/src/migration/use-message-migration.mddocs/src/tools/message.mdpackages/kit/package.jsonpackages/kit/src/core.tspackages/kit/src/message/adapters/index.tspackages/kit/src/message/adapters/native.tspackages/kit/src/message/adapters/vue.tspackages/kit/src/message/core/engine.tspackages/kit/src/message/core/index.tspackages/kit/src/message/plugins/index.tspackages/kit/src/message/plugins/lengthPlugin.tspackages/kit/src/message/plugins/thinkingPlugin.tspackages/kit/src/message/plugins/toolPlugin.tspackages/kit/src/message/test/mockResponseProvider.tspackages/kit/src/message/test/native.test.tspackages/kit/src/message/test/vue.test.tspackages/kit/src/message/types.tspackages/kit/src/message/utils.tspackages/kit/src/types.tspackages/kit/src/vue/message/mockResponseProvider.tspackages/kit/src/vue/message/plugins/fallbackRolePlugin.tspackages/kit/src/vue/message/plugins/index.tspackages/kit/src/vue/message/plugins/lengthPlugin.tspackages/kit/src/vue/message/plugins/thinkingPlugin.tspackages/kit/src/vue/message/plugins/toolPlugin.tspackages/kit/src/vue/message/types.tspackages/kit/src/vue/message/useMessage.test.tspackages/kit/src/vue/message/useMessage.tspackages/kit/vitest.config.ts
💤 Files with no reviewable changes (2)
- packages/kit/src/vue/message/plugins/index.ts
- packages/kit/src/vue/message/plugins/fallbackRolePlugin.ts
…ssage adapter initialization error handling
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/kit/src/message/core/engine.ts (1)
187-192:⚠️ Potential issue | 🟠 MajorRace provider calls and stream reads against abort.
If
responseProvideror the async iterator ignoresabortSignal,abort()can wait forever forisProcessingto become false. The provider result and each iterator.next()need to be wrapped withmakeAbortable.Abort-safe stream consumption
- const result = responseProvider(requestBody, abortSignal) + const result = makeAbortable(Promise.resolve(responseProvider(requestBody, abortSignal)), abortSignal) const chunks = normalizeToAsyncGenerator(result) let lastChoice: ChatCompletionChoice | undefined = undefined - for await (const chunk of chunks) { + while (true) { + const { value: chunk, done } = await makeAbortable(chunks.next(), abortSignal) + if (done) { + break + } + setRequestState('processing', 'completing')Also applies to: 446-459
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/message/core/engine.ts` around lines 187 - 192, The call to responseProvider and the async-iterator consumption in the loop are not abort-safe: wrap the initial responseProvider(requestBody, abortSignal) invocation with makeAbortable so it rejects if abortSignal fires, and wrap each await on the iterator (each .next()/for-await step) with makeAbortable as well, passing the same abortSignal; update the usage around normalizeToAsyncGenerator(result) and the for-await (chunks) consumption to use the abortable-wrapped promises so abort() cannot hang waiting for isProcessing, and apply the same change to the other similar block that consumes the provider stream (the block around the later iterator usage).
🧹 Nitpick comments (1)
packages/kit/src/message/utils.ts (1)
133-133: Deprecation alias is optional but recommended for public APIs.The verification confirms no internal references to the old misspelled name exist in the codebase, so this rename poses no risk to internal consumers. However, if this is a public API consumed externally, adding a deprecated alias is still a best practice for smoother adoption across releases:
Optional compatibility alias
export const combineDeltaData = (target: Record<string, any>, source: Record<string, any>) => { for (const [sourceKey, sourceValue] of Object.entries(source)) { const targetValue = target[sourceKey] @@ return target } + +/** `@deprecated` Use `combineDeltaData` instead. */ +export const combileDeltaData = combineDeltaData🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kit/src/message/utils.ts` at line 133, Add a deprecated compatibility alias for the old misspelled export by exporting the original name (e.g., the old misspelled symbol) and pointing it to the new function combineDeltaData; add a JSDoc `@deprecated` tag on the alias that references combineDeltaData so external consumers see the deprecation notice while internal calls keep working. Ensure the alias is exported from the same module and simply re-exports or references combineDeltaData (no duplicate logic) so callers of the old name still work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kit/src/message/adapters/shared.ts`:
- Around line 20-64: The subscribe flow and notify loop must isolate listener
exceptions so a thrown listener doesn't stop later listeners or bubble into
mutate/engine flows and must ensure subscribe always returns an unsubscribe
handle even if the initial listener call throws; in
createStateSubscriptionController wrap the immediate listener invocation (the
listener(getState()) call in subscribe) in a try/catch that logs or swallows the
error so it cannot propagate, and similarly wrap each entry.listener(snapshot)
call inside notify in a try/catch so one listener's throw does not prevent
others from running; keep adding/removing entries using the existing listeners
Set and do not rethrow errors.
In `@packages/kit/src/message/plugins/toolPlugin.ts`:
- Around line 235-237: The early-return in toolPlugin.ts that checks if
lastChoice?.finish_reason !== 'tool_calls' || !currentMessage.tool_calls?.length
exits before invoking the wrapped restOptions.onAfterRequest, causing
consumer-provided onAfterRequest hooks to be dropped for normal completions;
update this branch (and the similar early-return around the other occurrence
noted) to call the original restOptions.onAfterRequest(with the same
args/context) before returning so the consumer hook still runs even when there
are no tool calls, and ensure you reference the same wrapped function used
elsewhere in toolPlugin (restOptions.onAfterRequest) so behavior remains
consistent.
- Around line 240-273: Wrap the pre-tool hook and each tool async-iterator read
with the existing makeAbortable so they respect the provided abortSignal: race
the beforeCallTools(...) call with makeAbortable using context.abortSignal; when
you call callTool(...) and get the normalized iterator
(normalizeToAsyncGenerator), don't await the whole generator directly—wrap each
awaited step (or the iterator.next() Promise) with makeAbortable using
context.abortSignal so the for-await loop can be aborted; keep using
toolCallStart, appendMessage and toolCallPromises but ensure both
beforeCallTools and per-chunk reads are made abortable via makeAbortable.
---
Duplicate comments:
In `@packages/kit/src/message/core/engine.ts`:
- Around line 187-192: The call to responseProvider and the async-iterator
consumption in the loop are not abort-safe: wrap the initial
responseProvider(requestBody, abortSignal) invocation with makeAbortable so it
rejects if abortSignal fires, and wrap each await on the iterator (each
.next()/for-await step) with makeAbortable as well, passing the same
abortSignal; update the usage around normalizeToAsyncGenerator(result) and the
for-await (chunks) consumption to use the abortable-wrapped promises so abort()
cannot hang waiting for isProcessing, and apply the same change to the other
similar block that consumes the provider stream (the block around the later
iterator usage).
---
Nitpick comments:
In `@packages/kit/src/message/utils.ts`:
- Line 133: Add a deprecated compatibility alias for the old misspelled export
by exporting the original name (e.g., the old misspelled symbol) and pointing it
to the new function combineDeltaData; add a JSDoc `@deprecated` tag on the alias
that references combineDeltaData so external consumers see the deprecation
notice while internal calls keep working. Ensure the alias is exported from the
same module and simply re-exports or references combineDeltaData (no duplicate
logic) so callers of the old name still work.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7d4c5186-cc21-4cd3-af8b-c6991d6a8733
📒 Files selected for processing (9)
packages/kit/src/core.tspackages/kit/src/message/adapters/native.tspackages/kit/src/message/adapters/shared.tspackages/kit/src/message/adapters/vue.tspackages/kit/src/message/core/engine.tspackages/kit/src/message/plugins/toolPlugin.tspackages/kit/src/message/test/native.test.tspackages/kit/src/message/test/vue.test.tspackages/kit/src/message/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/kit/src/core.ts
- packages/kit/src/message/adapters/native.ts
…ate and update state management feat(toolPlugin): improve tool handling by ensuring meaningful results and updating metadata fix(mockResponseProvider): make delay parameter optional in mock stream function
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kit/src/message/plugins/toolPlugin.ts`:
- Around line 283-297: The handler can drop earlier string chunks when a later
object chunk fails JSON.parse; update the logic in the tool chunk merge to
preserve prior string content: when chunk is an object but toolMessage.content
is currently a string (or content parse fails), treat the existing content as
text (e.g., set parsedContent to { text: existingString } or otherwise wrap it)
instead of defaulting to {}; then call combineDeltaData(parsedContent, chunk)
and stringify the result; add an explicit flag or use toolMessage.content type
checks to track whether the stream is in "string mode" vs "object mode" so
future chunks are merged consistently (refer to toolMessage, chunk,
combineDeltaData and the JSON.parse block).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 68fce8f1-3ee7-4616-8fa3-b56c3fa2b825
📒 Files selected for processing (3)
packages/kit/src/message/plugins/thinkingPlugin.tspackages/kit/src/message/plugins/toolPlugin.tspackages/kit/src/message/test/mockResponseProvider.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/kit/src/message/plugins/thinkingPlugin.ts
- packages/kit/src/message/test/mockResponseProvider.ts
…g errors from listeners
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.
背景
本次变更将
useMessage的核心能力从 Vue 实现中抽离到packages/kit/src/message/core，引入跨平台的 message engine + state adapter 模型，并完成 VueuseMessage到 core 的迁移。目标是把消息处理流程、插件生命周期和状态管理能力收敛到一套 core 实现中，避免 Vue 与非 Vue 场景分别维护两套逻辑。
架构图
主要变更
message/core/engine.ts，统一承载消息发送、流式合并、状态流转、插件生命周期、递归 request 等核心流程MessageStateAdapter抽象，用于解耦不同运行时的状态实现native/vue两个 adaptermutate + subscriberef/reactive/computedthinkingPluginlengthPlugintoolPluginuseMessage改为基于createMessageEngine + createVueMessageAdapter实现fallbackRolePlugin相关逻辑与文档残留设计说明
createMessage进入响应式系统，保证对message.content、message.state等字段的原地修改能够被正确追踪createMessage构造，避免 raw object 和响应式对象混用变更结果
useMessage迁移到 core 后，仍然保留原有响应式体验测试
已补充并通过以下测试：
src/message/test/native.test.tssrc/message/test/vue.test.tssrc/vue/message/useMessage.test.tsCore 使用示例
说明
这次 PR 主要完成了 message core 抽取与 Vue
useMessage迁移。后续如果继续推进，剩余工作会主要集中在：Summary by CodeRabbit
New Features
Documentation
Tests