feat(chat): extract reusable chat package with configurable runtime and page primitives#338
feat(chat): extract reusable chat package with configurable runtime and page primitives#338SonyLeo wants to merge 3 commits into
Conversation
WalkthroughThis PR adds a new package ChangesChat package: API, runtime, components, styles, build & CI
Sequence DiagramsequenceDiagram
participant User
participant TrChat as TrChat Component
participant Runtime as ChatRuntime
participant ChatKit as useChatKit
participant Provider as ResponseProvider
participant LLM as LLM API
User->>TrChat: Provide TrChatConfig / interact (send message, actions)
TrChat->>Runtime: createRuntimeFromConfig / resolve runtime
Runtime->>ChatKit: initialize engine and conversation state
User->>TrChat: Send message
TrChat->>Runtime: beforeSend (hook) -> prepare content/attachments
Runtime->>ChatKit: sendMessage(content, attachments)
ChatKit->>Provider: build request body and POST (stream)
Provider->>LLM: streaming request
LLM-->>Provider: SSE chunks (deltas, usage)
Provider-->>ChatKit: parsed ChatCompletion chunks
ChatKit->>ChatKit: apply onChunk/onFinish, append messages
ChatKit-->>TrChat: reactive messages update
TrChat->>User: UI updates (bubbles, feedback)
User->>TrChat: Retry / Regenerate / Edit
TrChat->>Runtime: invoke retry/regenerate/edit flows
Runtime->>ChatKit: perform retry/regenerate/edit resend
ChatKit->>Provider: new request created
Provider->>LLM: stream new response
LLM-->>Provider: new completion
Provider-->>ChatKit: chunks processed
ChatKit-->>TrChat: messages updated
TrChat->>User: updated response displayed
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
SonyLeo
changed the title
|
✅ Preview build completed successfully!
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟡 Minor comments (20)
package.json-17-17 (1)
17-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDefault build path likely misses chat package output
On Line 17,
build:packagescorrectly includes@opentiny/tiny-robot-chat, but the rootbuildscript still targetsbuild:components. If contributors runpnpm build, chat won’t be built by default, which can drift from CI/release expectations.Suggested adjustment
- "build": "pnpm build:components", + "build": "pnpm build:packages",🤖 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 `@package.json` at line 17, The root npm script "build" currently runs "build:components" only, so running `pnpm build` will not execute the "build:packages" step (which includes `@opentiny/tiny-robot-chat`); update the "build" script to invoke "build:packages" (or chain it: run "build:components" && "build:packages") so that the "build" script triggers the same package-level build as the "build:packages" script and ensures the chat package is built by default; modify the "build" entry in package.json to reference "build:packages" (or include both) and keep "build:packages" unchanged.packages/chat/src/components/history/ChatHistoryNewSession.vue-23-26 (1)
23-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
type="button"on the<button>element — risk of accidental form submission.Without an explicit
type, buttons inside a<form>default totype="submit". If this component is ever rendered inside a form (directly or via a parent component), clicking "new session" will submit the form instead of callinghandleCreateNewSession.🛡️ Proposed fix
- <button class="new-session-btn" `@click`="handleCreateNewSession"> + <button type="button" class="new-session-btn" `@click`="handleCreateNewSession">🤖 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/chat/src/components/history/ChatHistoryNewSession.vue` around lines 23 - 26, The <button> element in ChatHistoryNewSession.vue (class "new-session-btn") lacks an explicit type which can cause accidental form submission; update the button element that invokes handleCreateNewSession (and renders <IconNewSession /> and {{ chatMessages.history.newSession }}) to include type="button" so it only triggers the click handler and not form submit.packages/chat/src/components/renderers/ToolCallRenderer.vue-25-30 (1)
25-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
JSON.stringify(undefined)returnsundefined, not a string — guardtoolCallWithResult.value.If
toolCallWithResult.valueisundefined(e.g., before the tool call result arrives),JSON.stringify(undefined, null, 2)returnsundefined, which silently setsnode.codeandnode.rawtoundefined. Downstream,MarkdownCodeBlockNodewould receive a non-stringcode, which may crash or render incorrectly.🛡️ Proposed fix
watchEffect(() => { - const code = JSON.stringify(toolCallWithResult.value, null, 2) + const code = JSON.stringify(toolCallWithResult.value ?? null, null, 2) node.code = code node.raw = code })🤖 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/chat/src/components/renderers/ToolCallRenderer.vue` around lines 25 - 30, The watchEffect currently assigns JSON.stringify(toolCallWithResult.value, null, 2) directly to node.code and node.raw which yields undefined when toolCallWithResult.value is undefined; update the watchEffect to guard toolCallWithResult.value (from the reactive ref) and only call JSON.stringify when it's defined, otherwise set node.code and node.raw to a safe string (e.g., empty string or "{}") so MarkdownCodeBlockNode always receives a string; locate the watchEffect block and the references to toolCallWithResult.value, node.code and node.raw to implement this guard.packages/chat/src/styles/model-selector.css-132-136 (1)
132-136:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
cursor: not-allowedis dead code whenpointer-events: noneis also set.
pointer-events: nonecauses the browser to ignore the element'scursordeclaration — the cursor of the element underneath is shown instead, notnot-allowed. Users lose the visual signal that the option is disabled.Either drop
pointer-events: noneand guard clicks in the component logic, or removecursor: not-allowed(accepting that no disabled cursor will appear).♻️ Option A — keep pointer-events: none, drop cursor (simpler)
.tr-model-selector__option.is-disabled { opacity: 0.4; - cursor: not-allowed; pointer-events: none; }♻️ Option B — keep cursor feedback, handle clicks in JS
.tr-model-selector__option.is-disabled { opacity: 0.4; cursor: not-allowed; - pointer-events: none; }🤖 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/chat/src/styles/model-selector.css` around lines 132 - 136, The CSS sets pointer-events: none on .tr-model-selector__option.is-disabled which prevents the not-allowed cursor from showing; remove pointer-events: none and keep cursor: not-allowed so users see the disabled cursor, and then guard clicks in the component logic (e.g., in the option click handler such as onOptionClick / handleOptionSelect / selectModel) to ignore or return early when the option has the disabled state (and ensure aria-disabled or equivalent prop is set); alternatively, if you prefer the simpler approach, drop cursor: not-allowed and keep pointer-events: none — but be explicit in the component code about which approach you chose.packages/chat/src/components/workspace/ChatWorkspaceRightEmpty.vue-40-40 (1)
40-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
color-mix()has limited browser support — verify minimum target browsers.
color-mix(in srgb, ...)is used in the background value at line 40. This is also used inChatHistoryNewSession.vue(line 62), and is not supported in browsers older than Chrome 111 / Firefox 113 / Safari 16.2. If the package targets environments with older browsers, this will silently fall back to the initial/inherited value, leaving the icon background invisible. Add a fallback CSS value as recommended in MDN documentation.🤖 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/chat/src/components/workspace/ChatWorkspaceRightEmpty.vue` at line 40, The CSS uses color-mix(in srgb, ...) in the background of ChatWorkspaceRightEmpty.vue (and similarly in ChatHistoryNewSession.vue) which lacks support in older browsers; add a fallback background value before the color-mix declaration (e.g., a solid rgba() or CSS variable default) so browsers that don't support color-mix will render a visible background, then keep the existing color-mix(...) line as the progressive-enhancement value; update both components (ChatWorkspaceRightEmpty and ChatHistoryNewSession) to include the fallback.packages/chat/src/components/history/ChatHistorySearch.vue-13-18 (1)
13-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSearch input has no accessible label
The
<input>relies solely onplaceholderfor labeling, which is insufficient for screen readers (placeholders disappear on input and have poor contrast by default). Add anaria-labelor a visually-hidden<label>.♻️ Proposed fix
<input v-model="historyState.searchQuery.value" type="text" + :aria-label="chatMessages.history.searchPlaceholder" :placeholder="chatMessages.history.searchPlaceholder" class="search-input" />🤖 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/chat/src/components/history/ChatHistorySearch.vue` around lines 13 - 18, The search <input> in ChatHistorySearch.vue currently uses only a placeholder (bound to chatMessages.history.searchPlaceholder) which is not accessible; update the <input> with an accessible label by adding an aria-label (e.g. aria-label bound to chatMessages.history.searchPlaceholder) or add a visually-hidden <label> element tied to the input (using for/id) and keep v-model="historyState.searchQuery.value" and class="search-input" unchanged so screen readers receive a persistent label.packages/chat/src/components/history/ChatHistorySearch.vue-27-37 (1)
27-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
box-sizing: border-boxmay cause the search input to overflow its container
width: 100%+padding: 10px 14pxwithoutbox-sizing: border-boxmeans the computed width exceeds the container width, causing horizontal overflow.♻️ Proposed fix
.search-input { width: 100%; + box-sizing: border-box; padding: 10px 14px;🤖 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/chat/src/components/history/ChatHistorySearch.vue` around lines 27 - 37, The .search-input rule currently uses width: 100% with padding, which can make the element overflow its container; update the .search-input CSS to include box-sizing: border-box so padding is included in the element's width calculation (modify the .search-input selector in ChatHistorySearch.vue to add box-sizing: border-box).packages/chat/src/styles/drawer.css-4-4 (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
--chat-drawer-z-indexhas no fallback —calc()will produce an invalid value if undefined
calc(var(--chat-drawer-z-index) - 1)evaluates tocalc(NaN - 1)when--chat-drawer-z-indexis not defined, silently zeroing out the overlay z-index. The main drawer's z-index on Line 24 has the same issue.♻️ Proposed fix
- z-index: calc(var(--chat-drawer-z-index) - 1); + z-index: calc(var(--chat-drawer-z-index, 300) - 1);- z-index: var(--chat-drawer-z-index); + z-index: var(--chat-drawer-z-index, 300);🤖 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/chat/src/styles/drawer.css` at line 4, The z-index calc uses var(--chat-drawer-z-index) without a fallback which yields an invalid calc result when the custom property is undefined; update the two occurrences that use calc(var(--chat-drawer-z-index) - 1) (the overlay z-index and the main drawer z-index) to provide a sensible fallback inside var(), e.g. var(--chat-drawer-z-index, <fallback>), so the calc never evaluates to NaN and the z-index stays valid.packages/chat/src/components/feedback/ChatUsagePanel.vue-34-38 (1)
34-38:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPanel briefly flashes at
top: 0 / left: 0beforeupdatePositionresolves
visible.value = truecauses the panel to render at{ top: '0px', left: '0px' }for one frame beforeupdatePositioncomputes the correct coordinates. Consider initializing the panel asvisibility: hiddenuntil positioned, or usingv-showwith an additionalpositionedguard.♻️ Proposed fix
const panelStyle = ref({ top: '0px', left: '0px' }) +const positioned = ref(false) async function show() { visible.value = true + positioned.value = false await nextTick() await updatePosition() + positioned.value = true }Then add
:style="[panelStyle, positioned ? {} : { visibility: 'hidden' }]"to the panel element.🤖 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/chat/src/components/feedback/ChatUsagePanel.vue` around lines 34 - 38, The panel flashes because visible.value = true renders it at top/left before updatePosition runs; add a positioned reactive flag (e.g., positioned.value = false), set positioned false before setting visible true in the show() function, call await nextTick() and await updatePosition(), then set positioned true after positioning completes; update the panel element to apply :style="[panelStyle, positioned ? {} : { visibility: 'hidden' }]" (or use v-show with the positioned guard) so the DOM is rendered but stays hidden until updatePosition has set correct coordinates; reference show(), visible, positioned, updatePosition and panelStyle when making the changes.packages/chat/src/components/ChatWelcome.vue-29-35 (1)
29-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
emit('prompt-click', ...)can emitundefinedwhen bothitem.descriptionanditem.labelare absent.If a prompt item has neither
descriptionnorlabel, the event payload isundefined. Depending on how the parent handlesprompt-click, this may silently no-op or cause unexpected behavior. Consider adding a guard or falling back to an empty string:🛡️ Proposed fix
- `@item-click`="(_ev, item) => emit('prompt-click', item.description ?? item.label)" + `@item-click`="(_ev, item) => emit('prompt-click', item.description ?? item.label ?? '')"🤖 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/chat/src/components/ChatWelcome.vue` around lines 29 - 35, The click handler passed to TrPrompts can emit undefined when both item.description and item.label are missing; update the inline handler used in the TrPrompts component (the `@item-click` listener) to guard against undefined and emit a safe fallback (e.g., empty string or a sentinel) instead of possibly emitting undefined—ensure the check uses the same identifiers (item.description, item.label) and calls emit('prompt-click', fallback) so parent listeners always receive a defined payload.packages/chat/src/components/renderers/ErrorRenderer.vue-79-89 (1)
79-89:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrong CSS token namespace —
--rc-color-danger-lightwon't adapt in dark mode.The error box background (line 83) uses
--rc-color-danger-lightfrom an external token namespace that has no dark-mode override in the chat token system. The correct chat-scoped token is--chat-danger-soft-bg, which is already defined intokens.cssand covered by dark-mode overrides.🎨 Proposed fix
.error-renderer { font-size: 12px; padding: 0.5rem; margin: 0.25rem 0; - background-color: var(--rc-color-danger-light); + background-color: var(--chat-danger-soft-bg); border-radius: 0.5rem;🤖 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/chat/src/components/renderers/ErrorRenderer.vue` around lines 79 - 89, The .error-renderer CSS uses the external token --rc-color-danger-light which lacks dark-mode overrides; replace that token with the chat-scoped token --chat-danger-soft-bg in the background-color declaration inside the .error-renderer rule so the error box picks up the defined dark-mode variants (update the background-color: var(...) in the .error-renderer selector to use --chat-danger-soft-bg instead of --rc-color-danger-light).packages/chat/src/shared/attachments.ts-33-35 (1)
33-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
getExtensionbreaks on URL strings with query parameters
name.split('.').pop()on a URL like"photo.png?v=123"returns"png?v=123", not"png", sodetectFileTypereturns'other'for such inputs. WhileisImageAttachmentpassesattachment.name(usually a clean filename), any caller providing a URL string todetectFileTypewill silently get the wrong type.🐛 Proposed fix
function getExtension(name: string): string { - return (name.split('.').pop() ?? '').toLowerCase() + // Strip query string / hash before parsing extension + const clean = name.split('?')[0].split('#')[0] + return (clean.split('.').pop() ?? '').toLowerCase() }🤖 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/chat/src/shared/attachments.ts` around lines 33 - 35, getExtension currently extracts the extension with name.split('.').pop() which fails on URLs with query strings (e.g. "photo.png?v=123"); update getExtension to strip any query string and fragment (split on '?' and '#' and take the first part) before extracting the final dot segment, then return the lowercased extension; this will ensure detectFileType (and helpers like isImageAttachment which expect reliable extensions when given URLs instead of plain filenames) correctly identifies types for URL inputs.packages/chat/README.md-165-165 (1)
165-165:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced code block
The directory tree code block is missing a language specifier, which triggers a
markdownlintMD040 warning.📝 Proposed fix
-``` +```text src/ entry/ ...🤖 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/chat/README.md` at line 165, The fenced code block containing the directory tree in README.md lacks a language tag and triggers markdownlint MD040; update the opening triple-backtick for that directory-tree block to include a language specifier (e.g., "text") so the block becomes ```text, ensuring the directory listing is correctly recognized as a code block by linters and renderers.packages/chat/src/components/history/ChatHistory.vue-25-29 (1)
25-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrawer overlay is not keyboard-dismissable
The overlay
<div>only handles mouse clicks; keyboard users have no way to close the drawer via the overlay. WCAG 2.1 SC 2.1.1 requires all functionality be reachable via keyboard.♿ Proposed fix — add keyboard and role attributes
<div class="tr-chat-drawer-overlay" :class="{ 'is-open': chatUi.history.visible.value }" + role="button" + tabindex="0" + :aria-label="chatMessages.history?.closeLabel" `@click`="chatUi.history.close()" + `@keydown.enter.prevent`="chatUi.history.close()" + `@keydown.space.prevent`="chatUi.history.close()" + `@keydown.escape`="chatUi.history.close()" />🤖 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/chat/src/components/history/ChatHistory.vue` around lines 25 - 29, The overlay div with class "tr-chat-drawer-overlay" only handles mouse clicks and is not keyboard-dismissable; make it accessible by adding appropriate interactive semantics and keyboard handling: give the element a role="button" and tabindex="0", add a keydown handler that calls chatUi.history.close() when Enter or Space is pressed, and ensure the existing `@click` handler remains; update any event handler names (e.g., use onOverlayKeydown or similar) and keep chatUi.history.close referenced so the drawer can be dismissed by keyboard and screen readers.packages/chat/src/runtime/engine/useChatRequest.ts-12-15 (1)
12-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
extractStatusCodecaptures408but the classification logic never handles it as'timeout'.The regex on line 13 includes
408, so a message like"408 Request Timeout"will setstatusCode = 408. However, none of the subsequentif-branches treats408explicitly: it is not401/403, not429, and not>= 500, so it falls through to thelowerCasedMessage.includes('timeout')text check. If the message happens not to contain the word "timeout", it will be misclassified as'network','provider', or'unknown'.🐛 Proposed fix
if (statusCode === 429 || lowerCasedMessage.includes('rate limit') || code === 'rate_limit_exceeded') { ... } + if (statusCode === 408) { + return { + type: 'timeout', + message, + retryable: retryable ?? true, + httpStatus, + statusCode, + code, + providerId, + originalError: error, + } + } + if (statusCode && statusCode >= 500) {🤖 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/chat/src/runtime/engine/useChatRequest.ts` around lines 12 - 15, extractStatusCode currently captures 408 but the downstream classification never treats 408 as a timeout; update the classification logic (the block that reads statusCode and determines errorType in useChatRequest.ts) to explicitly map statusCode === 408 to the 'timeout' category (same branch that covers text-based timeout detection), so that any statusCode 408 is classified as 'timeout' regardless of message content; ensure you reference extractStatusCode's output (statusCode variable) and add the 408 check alongside existing checks for 401/403, 429, and >=500.packages/chat/src/components/history/ChatHistoryPanel.vue-122-122 (1)
122-122:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
border-radius: 80%on a non-square element produces an unexpected shape.CSS
border-radiuspercentage values reference the element's own width/height independently, so80%on a non-square.selected-countbadge (fixedheight: 20px, variablemin-width: 18px) will not produce a circle or pill. Use50%for a circle (when width == height) or999px/9999pxfor a pill-shaped badge.🎨 Proposed fix
-.selected-count { +.selected-count { min-width: 18px; height: 20px; ... - border-radius: 80%; + border-radius: 999px;🤖 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/chat/src/components/history/ChatHistoryPanel.vue` at line 122, The .selected-count badge uses border-radius: 80% which distorts non-square elements; change the CSS for .selected-count to use either border-radius: 50% when you expect a perfect circle (only when width == height) or a large fixed value like border-radius: 9999px to ensure a pill-shaped badge for variable widths; update the .selected-count rule (and any related height/min-width declarations) to use one of these values so the badge renders as an intended circle or pill.packages/chat/src/components/mcp/ChatMcpPanel.vue-125-127 (1)
125-127:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a semantic button for the add-form close control.
Line 127 binds close behavior to
IconClosedirectly, which is not keyboard-accessible by default. Wrap it in a<button type="button">(with label) so keyboard and assistive tech users can reliably close the panel.🤖 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/chat/src/components/mcp/ChatMcpPanel.vue` around lines 125 - 127, The close control currently binds closeAddForm directly to the IconClose component which is not keyboard-accessible; wrap IconClose in a semantic <button type="button"> (move the `@click`="closeAddForm" to that button), give the button an accessible label (aria-label="Close" or visible/visually-hidden text from chatMessages.mcp.installPlugin if appropriate), keep existing classes for styling (e.g., apply mcp-add-form-shell__close to the button), and remove any direct click binding from IconClose so keyboard and assistive-tech users can activate closeAddForm.packages/chat/src/components/model-selector/ModelSelector.vue-61-64 (1)
61-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisabled model clicks currently close the dropdown.
selectModelrejects disabled options, but Line 63 still closes the menu. This creates a confusing UX (disabled option behaves like a close action).Suggested fix
function handleSelectModel(model: ModelOption) { + if (model.disabled) return selectModel(model) isOpen.value = false }Also applies to: 127-135
🤖 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/chat/src/components/model-selector/ModelSelector.vue` around lines 61 - 64, handleSelectModel currently closes the dropdown unconditionally even when selectModel rejects disabled options; change it so the menu only closes when selection succeeds—either by checking model.disabled before calling selectModel or by awaiting selectModel and only setting isOpen.value = false on success (catch and do not close on error). Apply the same change to the other selection handler that mirrors this behavior so disabled clicks don't close the menu.packages/chat/src/components/model-selector/ModelSelector.vue-91-97 (1)
91-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit button types to avoid accidental form submission.
On Line 91 and Line 127,
<button>defaults totype="submit"inside forms.Suggested fix
<button ref="referenceEl" + type="button" class="tr-model-selector__trigger" `@click`="toggleDropdown" @@ <button + type="button" class="tr-model-selector__option"Also applies to: 127-135
🤖 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/chat/src/components/model-selector/ModelSelector.vue` around lines 91 - 97, The buttons in ModelSelector.vue are missing explicit types and can default to type="submit" inside forms; update the trigger button (ref="referenceEl", class="tr-model-selector__trigger", `@click`="toggleDropdown", :aria-label="chatMessages.modelSelector.triggerLabel") and the other button(s) in the dropdown (the button in the 127-135 block) to include type="button" so they don't submit enclosing forms accidentally; locate the elements by their class/ref and add the type attribute to each button element.packages/chat/src/runtime/engine/chatMessageState.ts-22-26 (1)
22-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
ensureChatMessageStatewill throw if called withnull/undefined.All internal setters already guard with
if (!message) return, butensureChatMessageStateis exported and performs an unchecked property write. An external caller passingnullwill get aTypeError: Cannot set properties of null.🛡️ Proposed fix
export function ensureChatMessageState(message: unknown): ChatMessageRuntimeState { + if (!message) throw new TypeError('[ensureChatMessageState] message must be a non-null object') const stateCarrier = message as MessageStateCarrier stateCarrier.state ??= {} return stateCarrier.state as ChatMessageRuntimeState }🤖 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/chat/src/runtime/engine/chatMessageState.ts` around lines 22 - 26, ensureChatMessageState currently writes to message.state without guarding against null/undefined; change it so if message is nullish (message == null) it returns a fresh empty ChatMessageRuntimeState (do not attempt to mutate the null), otherwise cast to MessageStateCarrier and perform stateCarrier.state ??= {} and return stateCarrier.state as ChatMessageRuntimeState; update references to ensureChatMessageState, MessageStateCarrier and stateCarrier.state accordingly.
🧹 Nitpick comments (19)
packages/chat/src/components/renderers/MarkStreamRenderer.vue (1)
7-7: ⚡ Quick winMove
enableMermaid()to app bootstrap instead of per-instance setup.Line 7 executes
enableMermaid()for every component mount. According to markstream-vue guidance, this is a global configuration toggle meant to run once at app initialization, not per component. Repeated calls add unnecessary overhead and duplicate global registration work. Move this to your app's bootstrap/setup phase (e.g., inmain.tsor a global setup function), not in individual component<script setup>blocks.🤖 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/chat/src/components/renderers/MarkStreamRenderer.vue` at line 7, The call to enableMermaid() in MarkStreamRenderer.vue is being run per-component; remove that call from the component and instead invoke enableMermaid() once during app bootstrap (for example in main.ts or your global setup function) so the markstream-vue global configuration is registered only once; update MarkStreamRenderer.vue to delete the enableMermaid() invocation and add a single enableMermaid() call in your application entrypoint (e.g., main.ts) before mounting the app.packages/chat/src/components/feedback/actions/copyAction.ts (1)
24-30: 💤 Low valueConsider adding error handling for copy operations.
The async copy operations could fail (e.g., clipboard access denied, network issues). Consider wrapping in try-catch or ensuring the caller handles errors appropriately.
💡 Optional: Add error handling
onClick: async () => { + try { if (runtime && primaryMessageId.value) { await runtime.message.copy(primaryMessageId.value) return } copy(content.value) + } catch (error) { + console.error('Failed to copy:', error) + // Optionally notify user + } },🤖 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/chat/src/components/feedback/actions/copyAction.ts` around lines 24 - 30, The onClick handler in copyAction.ts currently calls runtime.message.copy(primaryMessageId.value) or copy(content.value) without error handling; wrap the async copy logic in a try-catch inside the onClick async function (referencing onClick, runtime, primaryMessageId.value, content.value and the copy call) and handle failures by logging the error and/or surfacing a user-friendly notification so clipboard/permission/network errors are caught rather than thrown to the caller.packages/chat/src/styles/model-selector.css (1)
59-67: 💤 Low value
z-index: 50on.tr-model-selector__dropdownhas no effect — element is not positioned.
z-indexonly applies to positioned elements (positionother thanstatic). Since.tr-model-selector__dropdownhas nopositionproperty, this declaration is dead. The stacking is already handled byz-index: 9999on the positioned.tr-model-selector__dropdown-wrapperancestor.♻️ Proposed cleanup
.tr-model-selector__dropdown { min-width: 200px; overflow: hidden; - z-index: 50; border: 1px solid var(--model-selector-border-color); border-radius: 12px; background-color: var(--model-selector-bg); box-shadow: var(--model-selector-shadow-dropdown); }🤖 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/chat/src/styles/model-selector.css` around lines 59 - 67, The z-index: 50 on .tr-model-selector__dropdown is ineffective because z-index only applies to positioned elements and stacking is already handled by the positioned ancestor .tr-model-selector__dropdown-wrapper (which uses z-index: 9999); remove the z-index: 50 declaration from the .tr-model-selector__dropdown rule to clean up the stylesheet (or alternatively, if you intended to control stacking on the dropdown itself, make .tr-model-selector__dropdown positioned by adding position: relative and adjust z-index accordingly).packages/chat/src/components/renderers/ToolCallRenderer.vue (1)
36-44: 💤 Low value
textAndIconMapis reconstructed on every computed call — extract the constant icon mapping.The
Mapobject and all four entries are allocated on everytextAndIconrecomputation. Only the text strings change (based onchatMessages). Consider splitting the constant icon part from the reactive text lookup.♻️ Proposed refactor
+const statusIconMap: Record<string, Component> = { + running: IconLoading, + success: IconPlugin, + failed: IconError, + cancelled: IconCancelled, +} const textAndIcon = computed(() => { - const textAndIconMap = new Map<string, { text: string; icon: Component }>([ - ['running', { text: chatMessages.value.toolCall.running, icon: IconLoading }], - ['success', { text: chatMessages.value.toolCall.success, icon: IconPlugin }], - ['failed', { text: chatMessages.value.toolCall.failed, icon: IconError }], - ['cancelled', { text: chatMessages.value.toolCall.cancelled, icon: IconCancelled }], - ]) - - return textAndIconMap.get(state.value?.status || '') || { text: '', icon: IconPlugin } + const status = state.value?.status || '' + const textMap: Record<string, string> = { + running: chatMessages.value.toolCall.running, + success: chatMessages.value.toolCall.success, + failed: chatMessages.value.toolCall.failed, + cancelled: chatMessages.value.toolCall.cancelled, + } + return { + text: textMap[status] ?? '', + icon: statusIconMap[status] ?? IconPlugin, + } })🤖 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/chat/src/components/renderers/ToolCallRenderer.vue` around lines 36 - 44, The computed recreates textAndIconMap on every run; extract a constant status->icon mapping (e.g., ICON_BY_STATUS) outside the computed that maps 'running'|'success'|'failed'|'cancelled' to IconLoading, IconPlugin, IconError, IconCancelled, then update the computed (where textAndIconMap and the return live) to only read the current status from state.value?.status and compose the returned object using chatMessages.value.toolCall.[status] for text and ICON_BY_STATUS[status] (with a default icon) for icon; keep the computed responsibility limited to reactive text lookup while icons remain a static constant.packages/chat/package.json (1)
28-28: 💤 Low value
devscript runs fullvue-tsctype-check before entering watch mode — slow DX
"vue-tsc && vite build --watch"blocks watch startup on a complete type-check pass. Typically thedevscript is just"vite build --watch", with type checking done via a separatetype-checkscript or IDE.♻️ Proposed fix
-"dev": "vue-tsc && vite build --watch", +"dev": "vite build --watch",🤖 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/chat/package.json` at line 28, The dev script currently runs a full vue-tsc type-check before starting watch; update the "dev" script (scripts.dev) to only run the watcher (remove the leading vue-tsc) so watch/startup is not blocked, and add a separate "type-check" script (scripts["type-check"]) that runs vue-tsc (use --noEmit for pure checking) so type checking can run independently or in CI/IDE.packages/chat/vite.config.ts (1)
21-48: ⚡ Quick winNo source maps configured for the library build
minify: truewithoutsourcemap: truemakes it very difficult for consumers to debug issues that originate in this library. Library builds typically ship source maps.♻️ Proposed fix
build: { lib: { ... }, cssMinify: true, minify: true, + sourcemap: 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/chat/vite.config.ts` around lines 21 - 48, The build config currently minifies the library but doesn't emit source maps; update the Vite build configuration (the build object and its nested lib/rollupOptions/output) to enable source maps by setting build.sourcemap = true and ensure rollupOptions.output.sourcemap = true so the library and its rollup output emit .map files for consumers to debug minified code.packages/chat/src/components/feedback/ChatUsagePanel.vue (1)
44-47: ⚡ Quick win
useDateFormatinside a plain function is idiomatically suboptimal — use a plain date formatter insteadWhile
useDateFormatdoes work when called insideformatTime(), it creates ephemeralComputedRefobjects on every invocation. For a pure date formatting task, a simple date formatter is more efficient and idiomatic:♻️ Suggested refactor
-function formatTime(ts?: number) { - if (!ts) return '-' - return useDateFormat(ts * 1000, 'YYYY-MM-DD HH:mm:ss').value -} +function formatTime(ts?: number) { + if (!ts) return '-' + const d = new Date(ts * 1000) + const pad = (n: number) => String(n).padStart(2, '0') + return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}` +}🤖 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/chat/src/components/feedback/ChatUsagePanel.vue` around lines 44 - 47, The formatTime function currently calls useDateFormat on each invocation which creates ephemeral ComputedRefs; replace that with a plain date formatter: create a reusable formatter (e.g. Intl.DateTimeFormat or a simple formatting helper) defined outside formatTime and have formatTime convert the seconds ts to milliseconds and return the formatted string or '-' when ts is falsy; update the function name formatTime to use that external formatter and remove any dependency on useDateFormat.packages/chat/src/components/history/ChatHistoryList.vue (1)
27-28: 💤 Low valueConsider passing
filteredHistoryDatadirectly instead of syncing via a separate ref.
syncRef(filteredHistoryData, historyData, { direction: 'ltr' })copies a computed ref into a plain ref. UnlessTrHistorymutates its:dataprop internally (e.g., for optimistic reordering), usingfilteredHistoryDatadirectly as:dataeliminates the extra ref and sync watcher.♻️ Proposed simplification
-const historyData = ref<HistoryItem[]>([]) -syncRef(filteredHistoryData, historyData, { direction: 'ltr' })- :data="historyData" + :data="filteredHistoryData"🤖 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/chat/src/components/history/ChatHistoryList.vue` around lines 27 - 28, The code creates an extra plain ref historyData and syncs it from filteredHistoryData using syncRef; instead pass the computed ref filteredHistoryData directly to the TrHistory :data prop instead of maintaining historyData and calling syncRef(filteredHistoryData, historyData, { direction: 'ltr' }); update the template/prop usage to use filteredHistoryData and remove the historyData ref and the syncRef call unless TrHistory intentionally mutates its :data for optimistic updates.packages/chat/src/shared/utils/iconMap.ts (1)
33-48: 💤 Low valueType signature doesn't match the defensive null guard — update one or the other.
getProviderIcondeclaresmodel: ModelOption | string(non-nullable) but line 34 guards against falsy values, implying callers may passnullorundefined. If that's realistic (e.g., no model is selected), broaden the signature to avoid the type/runtime contract mismatch:🔧 Proposed fix
-export function getProviderIcon(model: ModelOption | string): Component | null { +export function getProviderIcon(model: ModelOption | string | null | undefined): Component | 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/chat/src/shared/utils/iconMap.ts` around lines 33 - 48, The function getProviderIcon has a defensive null check but its signature (model: ModelOption | string) is non-nullable; update the signature to accept nullable inputs (model: ModelOption | string | null | undefined) so callers can pass no selection, and keep the existing guard/logic that returns null for falsy model; ensure references to model.icon and model.providerId still compile and that PROVIDER_ICON_MAP lookup (providerId.toLowerCase() and model.toLowerCase()) remain guarded by the same null/typeof checks.packages/chat/src/shared/utils/typeGuards.ts (1)
23-35: 💤 Low value
conditionalPropis a strict subset ofextractProp
conditionalProp(obj, key)is equivalent toextractProp(obj, key, undefined)— both signatures, return types, and implementations are identical save for the omitteddefaultValueparameter. This is redundant.♻️ Proposed refactor — remove the duplicate
-/** - * 类型安全的条件属性提取 - * ... - */ -// eslint-disable-next-line `@typescript-eslint/no-explicit-any` -export function conditionalProp<T extends Record<string, any>, K extends keyof T>(obj: T, key: K): T[K] | undefined { - return key in obj ? (obj[key] as T[K]) : undefined -}Call sites can use
extractProp(obj, key)directly (nodefaultValueargument), which already returnsT[K] | undefined.🤖 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/chat/src/shared/utils/typeGuards.ts` around lines 23 - 35, The file defines conditionalProp which is functionally identical to extractProp; remove the duplicate by deleting conditionalProp and replacing all its call sites to use extractProp(obj, key) (which already returns T[K] | undefined); ensure exports are updated (remove conditionalProp from exports) and update any imports/usages across the codebase to import extractProp instead, or alternately implement conditionalProp as a one-line wrapper that delegates to extractProp(obj, key) if you need to keep the API for compatibility.packages/chat/src/runtime/engine/useChatRequest.ts (2)
182-185: 💤 Low valueWrapping
shallowRefincomputedjust to expose a readonly ref is unnecessary.
lastErroris already aShallowRef<ChatErrorInfo | null>. Wrapping it incomputed(() => lastError.value)adds an extra reactive layer with no benefit;shallowReadonly(lastError)or simply typing it asReadonly<ShallowRef<...>>would be cleaner and avoid an extra dependency-tracking node.🤖 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/chat/src/runtime/engine/useChatRequest.ts` around lines 182 - 185, The returned lastError is wrapped in computed unnecessarily; instead expose the existing ShallowRef without an extra reactive layer by returning shallowReadonly(lastError) or casting it as Readonly<ShallowRef<ChatErrorInfo | null>>; locate the return object in useChatRequest (the lastError property) and replace computed(() => lastError.value) with shallowReadonly(lastError) or an appropriate readonly type cast to eliminate the extra computed wrapper.
97-112: ⚡ Quick win
includes('fetch')is too broad and will false-positive on unrelated messages.
lowerCasedMessage.includes('fetch')matches any message containing the substring "fetch", including API names, library names, or phrases like "could not fetch config". The more specific'failed to fetch'already covers the primary browser fetch-abort pattern. Consider removing the bare'fetch'term or tightening it to'fetch error'/'fetcherror'.🔧 Proposed fix
if ( lowerCasedMessage.includes('failed to fetch') || - lowerCasedMessage.includes('network') || - lowerCasedMessage.includes('fetch') + lowerCasedMessage.includes('network error') || + lowerCasedMessage.includes('networkerror') ) {🤖 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/chat/src/runtime/engine/useChatRequest.ts` around lines 97 - 112, The check using lowerCasedMessage.includes('fetch') in useChatRequest.ts is too broad and causes false positives; update the conditional that builds the network error response (the block returning type: 'network' using lowerCasedMessage, message, retryable, httpStatus, statusCode, code, providerId, originalError) to drop the bare 'fetch' check and instead use a more specific match such as lowerCasedMessage.includes('failed to fetch') || lowerCasedMessage.includes('fetch error') (or a normalized 'fetcherror') so only genuine fetch-related errors trigger the network branch.packages/chat/src/components/feedback/actions/refreshAction.ts (1)
29-30: ⚡ Quick win
whenpredicate testsprimaryMessageId.valuebutonClickdispatches oncontext.messageId— verify they are always equivalent.
whenreturnstruefor all assistant messages wheneverruntimeis set andprimaryMessageId.valueis truthy (a single global ref). If a user somehow triggers the action on a different message, the onClick guardruntime && context.messageIdwill still pass, but the message-level view state checked will be the clicked message's, not the primary one. Confirm this is the intended UX; if the refresh action should only be visible on the primary/focused message, thewhenpredicate should also verifycontext.messageId === primaryMessageId.value.🤖 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/chat/src/components/feedback/actions/refreshAction.ts` around lines 29 - 30, The visibility predicate `when: () => Boolean((runtime && primaryMessageId.value) || (fallbackRuntime && !isStreaming.value && lastUserContent.value))` can show the action for any clicked message while `onClick` uses `context.messageId`; make them consistent by either: (A) updating the `when` predicate to accept the same `context` and require `context.messageId === primaryMessageId.value` when `runtime` is set (so the action is only visible on the primary/focused message), or (B) change the `onClick` guard to use `primaryMessageId.value` instead of `context.messageId` if the intent is to always act on the primary message; adjust the check in the `when` predicate or the `onClick` guard accordingly (referencing `when`, `primaryMessageId.value`, `context.messageId`, `onClick` and its `runtime && context.messageId` guard).packages/chat/src/components/renderers/ToolCallsRenderer.vue (1)
14-20: ⚡ Quick win
tool.idused as:keywithout a fallback — an absent or duplicate ID will break list reconciliation.OpenAI-compatible tool call IDs should be unique strings, but a malformed or provider-specific response could have an empty/absent
id. Iftool.idisundefined, Vue will warn about duplicate empty keys and may misreconcile the list.🛡️ Suggested defensive key
- :key="tool.id" + :key="tool.id ?? index"🤖 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/chat/src/components/renderers/ToolCallsRenderer.vue` around lines 14 - 20, The list uses tool.id as the Vue key which can be undefined/duplicate; update the v-for key generation for the ToolCall renderer (the ToolCall component loop over props.message.tool_calls) to use a defensive fallback when tool.id is falsy—e.g., construct a stable key combining tool.id when present with the loop index or tool.name, or otherwise use a deterministic index-based fallback—so change the :key="tool.id" usage to a composed key that guarantees uniqueness/stability.packages/chat/src/entry/index.ts (1)
5-5: 💤 Low value
RootBootstrapProviderbreaks theTr*naming convention used by all other exported components.All other public component exports (
TrChat,TrChatRoot,TrChatPage,TrChatProvider) use theTrprefix. ExportingRootBootstrapProviderwithout this prefix is inconsistent and may confuse consumers as to whether this is a first-class public API or an internal implementation detail.Consider either renaming it
TrChatRootBootstrapProvideror, if it is truly internal, not exporting it from the public entry at all.🤖 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/chat/src/entry/index.ts` at line 5, The exported symbol RootBootstrapProvider violates the Tr* public component naming convention; either rename the export to TrChatRootBootstrapProvider (update the default export in './RootBootstrapProvider.vue' and this re-export) to make it a clearly public API, or remove this export from the package entry to keep it internal; update any imports/usages to the new name if renaming, or move internal consumers to import directly from the file if you choose to stop exporting it publicly.packages/chat/src/shared/messages/index.ts (1)
71-71: ⚡ Quick win
triggerActiveTitlecontains{count}placeholder with no interpolation helper.
'已激活 {count} 个插件'embeds a{count}token, but nothing in the messages module provides a way to resolve it. Consumers are expected to perform manualstring.replace('{count}', n)at the call site, which is easy to forget and produces a broken UI if missed. Consider exporting a small formatting helper alongside the message key, or documenting the expected usage clearly.🤖 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/chat/src/shared/messages/index.ts` at line 71, The message key triggerActiveTitle currently contains an interpolated token '{count}' but no helper to resolve it; update the messages module by replacing the static string triggerActiveTitle with either (a) a small exported function triggerActiveTitle(count: number) that returns `已激活 ${count} 个插件`, or (b) export an accompanying helper formatTriggerActiveTitle(count) and keep the key, then update any imports to use the function/helper so callers don't have to manually string.replace; ensure the symbol name triggerActiveTitle (or formatTriggerActiveTitle) is exported from index.ts for consumers.packages/chat/src/entry/createRootBootstrapState.ts (1)
285-294: 💤 Low valueRedundant
computedwrapper around a constantfallbackChatKit.
fallbackChatKitis created once and never reassigned, so wrapping it incomputed(() => fallbackChatKit)adds a reactive dependency layer with no upside. Returning the value directly (or typing the field asUseChatKitReturn) is simpler.♻️ Proposed refactor
- return { - chatKit: computed<UseChatKitReturn>(() => fallbackChatKit), + return { + chatKit: fallbackChatKit, messages: computed<ChatMessagesOverrides | undefined>(() => uiRef.value?.labels),🤖 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/chat/src/entry/createRootBootstrapState.ts` around lines 285 - 294, The chatKit field is unnecessarily wrapped in computed(() => fallbackChatKit) even though fallbackChatKit is created once via createFallbackChatKit and never changes; replace the computed wrapper by returning fallbackChatKit directly and ensure the chatKit property is typed as UseChatKitReturn (or the existing UseChatKitReturn generic) so callers keep the expected type; locate the createFallbackChatKit call and the chatKit: computed<UseChatKitReturn>(...) entry and remove the computed wrapper, returning the constant instead.packages/chat/src/components/model-selector/useKeyboardNavigation.ts (1)
50-50: 🏗️ Heavy liftAll key listeners are global (
window) — multiple concurrent instances will conflict.
onKeyStrokelistens onkeydownevents onwindowby default. If two instances ofuseKeyboardNavigationare active at the same time (e.g., a model dropdown open while another overlay is mounted), every handler fires on each keystroke regardless of which is "active". Pass a scopedtargetelement toonKeyStroke(the dropdown container ref) or gate activation with a shared "focus lock" signal.Also applies to: 90-119
🤖 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/chat/src/components/model-selector/useKeyboardNavigation.ts` at line 50, The current useKeyboardNavigation hook uses useMagicKeys() and onKeyStroke which install global window keydown listeners (ArrowUp/ArrowDown) causing multiple instances to conflict; update useKeyboardNavigation to accept or create a container ref (dropdown container ref) and pass that as the scoped target to onKeyStroke (or gate handlers with a focus-lock boolean) so key handlers only run when that specific dropdown/container is active; change usages in the hook where ArrowUp/ArrowDown handlers are registered (the onKeyStroke calls around lines 50 and 90-119) to use the provided ref.current (or the focus-lock signal) as the target instead of the global listener.packages/chat/src/runtime/engine/useChatKit.ts (1)
229-258: 🏗️ Heavy liftMutating inner properties of a
shallowRefinsidewatchEffectmay not behave as expected.
optimisticTurnis ashallowRef. Lines 240-249 mutateoptimisticTurn.value.userMessageandoptimisticTurn.value.assistantMessagedirectly.shallowRefonly tracks the reference, so these property writes don't trigger reactivity or re-schedule thewatchEffect. The effect only re-runs when something else in its dependency set changes (e.g.,conversation.activeConversation.value?.engine.messages.value). If message list updates are batched or don't fire between the two assignments, the assistant-message branch may be evaluated in the same run as the user-message branch whileuserMessageis stillnullin the reactive graph.Consider replacing
shallowRefwithrefforoptimisticTurn, or restructuring the reconciliation to be triggered by an explicit signal rather than relying on side-effecting watchEffect reads.🤖 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/chat/src/runtime/engine/useChatKit.ts` around lines 229 - 258, The watchEffect mutates properties on optimisticTurn (a shallowRef), so those inner writes won't trigger reactivity; change optimisticTurn to a deep ref or make updates by replacing the whole value so Vue sees the change. Concretely: replace the shallowRef declaration of optimisticTurn with ref(...) (or use reactive(...)), or when setting userMessage/assistantMessage assign optimisticTurn.value = { ...optimisticTurn.value, userMessage: ..., assistantMessage: ... } so the ref reference changes; keep existing cleanup helpers (clearOptimisticTurn, clearPendingEditRollback) and calls to setChatMessageOptimistic and setChatMessageTurnId unchanged but ensure any code that expects optimisticTurn to be a shallowRef is updated to the new ref/reactive shape.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/chat/README.md`:
- Line 166: The fenced code block in the README.md that shows the directory
structure (the block starting with "src/" and the entry/ lines) is missing a
language specifier; update that fenced code block to include a language tag
(e.g., add "text" after the opening triple backticks) so Markdown renderers
treat it consistently (locate the block containing "src/" and "entry/" and
change ``` to ```text).
🪄 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: db22dae1-23ca-4202-b9e6-1a66dc4d3c2b
📒 Files selected for processing (7)
packages/chat/README.mdpackages/chat/src/components/feedback/useChatFeedback.tspackages/chat/src/components/history/ChatHistoryList.vuepackages/chat/src/components/model-selector/useKeyboardNavigation.tspackages/chat/src/runtime/config/createRuntimeFromConfig.tspackages/chat/src/types/config.tspackages/chat/src/types/runtime.ts
✅ Files skipped from review due to trivial changes (1)
- packages/chat/src/components/model-selector/useKeyboardNavigation.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/chat/src/components/history/ChatHistoryList.vue
- packages/chat/src/types/runtime.ts
- packages/chat/src/components/feedback/useChatFeedback.ts
- packages/chat/src/types/config.ts
- packages/chat/src/runtime/config/createRuntimeFromConfig.ts
1 participant
背景
本次提交将聊天能力从现有实现中抽离为可复用模块，基于
@opentiny/tiny-robot和@opentiny/tiny-robot-kit提供完整聊天页能力，同时支持从黑盒接入到白盒定制的多层使用方式。
文档在线预览👉
主要改动
packages/chat独立包，补齐构建产物、类型产物和样式导出TrChat：基于配置的开箱即用入口TrChat.Root + TrChat.Page：保留官方页面编排的白盒入口TrChat.Root + primitives：细粒度组件拼装createRuntimeFromConfig，支持从配置创建 chat runtimepackages/chat纳入构建、CI 产物和发布流程验证
pnpm -F @opentiny/tiny-robot-chat type-checkpnpm -F @opentiny/tiny-robot-chat build备注
Summary by CodeRabbit