fix(chat): restore chat naming pipeline with UI fallbacks#2072
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds workspace linkage after session creation, captures user-submitted messages to drive title generation, and propagates auto-generated titles to both tabs and panes via new store APIs and UI wiring; runtime and service code updated with title-extraction and tests added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatInterface
participant ChatPane
participant SessionSelector
participant TabsStore
participant TRPC
participant Runtime
User->>ChatInterface: submit message
ChatInterface->>ChatInterface: handleSend / auto-launch
ChatInterface->>ChatPane: onUserMessageSubmitted(message)
ChatPane->>ChatPane: compute fallback title
ChatPane->>TabsStore: setPaneAutoTitle(paneId, title)
ChatPane->>TabsStore: setTabAutoTitle(tabId, title)
ChatPane->>SessionSelector: provide fallbackTitle for session creation
ChatInterface->>TRPC: send message to session
TRPC->>Runtime: deliver message to runtime
Runtime->>Runtime: extractTextContent / generateAndSetTitle(options)
Runtime->>TRPC: request updateTitle (persist generated title)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
6 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/hooks/useChatMastraPaneController/useChatMastraPaneController.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/hooks/useChatMastraPaneController/useChatMastraPaneController.ts:83">
P3: Remove or gate the new console.debug logs; these run in frequently executed hooks and will spam production output.
(Based on your team's feedback about keeping logs lean.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx:90">
P2: Remove this debug logging effect from the session selector; it introduces noisy production logging and unnecessary per-update work in a hot UI path.
(Based on your team's feedback about keeping debug logging lean in frequently executed paths.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/stores/tabs/store.ts">
<violation number="1" location="apps/desktop/src/renderer/stores/tabs/store.ts:343">
P2: Remove the new debug log from `setTabAutoTitle`; this hot-path store action should not emit verbose console output with user-derived titles.
(Based on your team's feedback about removing excessive logs and dead comments proactively.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/stores/tabs/store.ts:1004">
P2: Remove the new debug log from `setPaneAutoTitle`; this introduces noisy console output in a frequently-used store path.
(Based on your team's feedback about removing excessive logs and dead comments proactively.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx:197">
P3: Remove or gate these debug logs; they are in frequently executed sync effects and log chat/session metadata that should not be emitted routinely.
(Based on your team's feedback about keeping debug logging lean in frequently executed paths.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/api/src/app/api/chat/[sessionId]/route.ts">
<violation number="1" location="apps/api/src/app/api/chat/[sessionId]/route.ts:70">
P3: Avoid unconditional `console.debug` on this hot success path; gate it to non-production (or remove it) to reduce log noise and metadata exposure.
(Based on your team's feedback about keeping debug logging lean in production paths.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/hooks/useChatMastraPaneController/useChatMastraPaneController.ts (1)
231-237: Minor: Memoize the fallback empty array to avoid unnecessary recomputation.When
allSessionsDataisundefined, the?? []creates a new array reference on each render, causing thesessionsuseMemo to recompute even when the data hasn't changed.♻️ Suggested fix
+const EMPTY_SESSIONS: typeof allSessionsData = []; + export function useChatMastraPaneController({Then at line 231:
- const allSessions = allSessionsData ?? []; + const allSessions = allSessionsData ?? EMPTY_SESSIONS;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/hooks/useChatMastraPaneController/useChatMastraPaneController.ts` around lines 231 - 237, The fallback empty array created by `allSessionsData ?? []` produces a new reference each render causing `sessions` (the `useMemo` block) to recompute; fix by memoizing the empty array and use that memoized value for `allSessions` (e.g., create a stable `EMPTY_SESSIONS` via `useMemo(() => [], [])` or `useRef([])` and then set `const allSessions = allSessionsData ?? EMPTY_SESSIONS`) so that `sessions` only recomputes when `allSessionsData` or `workspaceId` truly change; update references to `allSessions`, `allSessionsData`, and `sessions` accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx`:
- Around line 196-209: The debug snapshot in GroupStrip.tsx (inside the
useEffect block that currently calls console.debug with activeWorkspaceId,
shouldSyncChatTitles, targetSessionIds, chatSessions info) must be removed from
production flow; either delete the console.debug call(s) or wrap them behind a
development-only guard (e.g., check NODE_ENV or an isDev flag) so sensitive
session IDs/workspace IDs/titles are never logged in production. Update both the
useEffect at the top that logs the chat-title snapshot and the subsequent
similar console.debug block further down to use the same removal/guard approach.
- Around line 227-240: The current logic uses session.title || "New Chat" which
overwrites better client-derived fallback titles; change it to only apply a
title when session.title is defined/non-empty: use session.title (or a guarded
check like if (session.title && session.title.trim().length)) before calling
setTabAutoTitle(tabId, title) and setPaneAutoTitle(paneId, title), leaving
existing tab/pane titles untouched when session.title is missing; update the
block around setTabAutoTitle and setPaneAutoTitle in GroupStrip.tsx to perform
this guard using the session.title symbol and the target.tabIds/target.paneIds
iteration.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 90-103: Remove the render-time debug logging inside the useEffect
in SessionSelector: delete or disable the console.debug call that logs session
metadata (the block referencing isOpen, currentSessionId, sessions.length,
visibleCount and sessionsSample) to avoid leaking private info; if you need
runtime diagnostics, replace it with a guarded logger behind a development-only
flag or environment check (or log only non-identifying counters like
sessionCount) and ensure references to useEffect, console.debug,
currentSessionId, sessions, and visibleCount are updated accordingly.
In `@apps/desktop/src/renderer/stores/tabs/store.ts`:
- Around line 1000-1003: The setPaneAutoTitle updater currently overwrites
pane.name and can clobber user-renamed titles; add a pane-level userTitle
property (analogous to tab userTitle) on the pane objects and modify
setPaneAutoTitle (and the related auto-title logic around the same block at
lines ~1009-1015) to no-op when pane.userTitle is set; when applying an auto
title, only update pane.name if pane.userTitle is falsy, and ensure any manual
rename handler writes to pane.userTitle (and keeps pane.name in sync if you want
display fallback).
- Around line 343-347: The console.debug in setTabAutoTitle is logging
user-provided titles (the from/to properties) and should be removed or guarded;
update the code in the setTabAutoTitle handler (the console.debug call) to
either run only in dev (check an isDev/ENV flag) or log only non-sensitive
metadata (e.g., tabId and status) without including the actual title strings,
ensuring similar changes for the other occurrences around the 1004-1008 region.
In `@packages/chat-mastra/src/server/trpc/utils/runtime/runtime.ts`:
- Around line 234-239: The current type guard in extractTextContent only checks
c.type === "text" but TextContentPart has text: string (required), so parts like
{type: "text"} slip through and yield undefined; update the filter predicate in
extractTextContent to verify both c.type === "text" and that c.text is a string
(e.g. typeof c.text === "string" or c.text != null) so the predicate truly
narrows to TextContentPart before mapping to c.text, ensuring only valid text
strings are joined.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/hooks/useChatMastraPaneController/useChatMastraPaneController.ts`:
- Around line 231-237: The fallback empty array created by `allSessionsData ??
[]` produces a new reference each render causing `sessions` (the `useMemo`
block) to recompute; fix by memoizing the empty array and use that memoized
value for `allSessions` (e.g., create a stable `EMPTY_SESSIONS` via `useMemo(()
=> [], [])` or `useRef([])` and then set `const allSessions = allSessionsData ??
EMPTY_SESSIONS`) so that `sessions` only recomputes when `allSessionsData` or
`workspaceId` truly change; update references to `allSessions`,
`allSessionsData`, and `sessions` accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46bc6aea-359d-4946-846e-b865bb4966dd
📒 Files selected for processing (13)
apps/api/src/app/api/chat/[sessionId]/route.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/hooks/useChatMastraPaneController/useChatMastraPaneController.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tspackages/chat-mastra/src/server/trpc/service.tspackages/chat-mastra/src/server/trpc/utils/runtime/index.tspackages/chat-mastra/src/server/trpc/utils/runtime/runtime.test.tspackages/chat-mastra/src/server/trpc/utils/runtime/runtime.ts
# Conflicts: # apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
343-347:⚠️ Potential issue | 🟠 MajorDrop raw titles from these debug logs.
These setters still emit chat-derived
from/tostrings, so every auto-title update leaks user content into renderer logs.Also applies to: 1006-1010
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/tabs/store.ts` around lines 343 - 347, The debug call in setTabAutoTitle (and the other setter that logs chat-derived titles) leaks raw user content via the `from`/`to` fields; remove raw titles from the log and only emit non-sensitive metadata (e.g., tabId and a redaction or hash/length indicator). Update the console.debug in setTabAutoTitle and any similar debug statements so they no longer include `tab.name` or `title` directly—replace them with a redacted placeholder or a deterministic hash/length indicator and a short context string to preserve usefulness without exposing user text.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx (1)
16-16: Prefer therenderer/...alias here.This deep relative import is brittle compared with the alias-based imports already used elsewhere in desktop code.
♻️ Proposed cleanup
-import { getRelativeTime } from "../../../../../../../WorkspacesListView/utils"; +import { getRelativeTime } from "renderer/screens/main/components/WorkspaceView/WorkspacesListView/utils";As per coding guidelines,
apps/desktop/**/*.{ts,tsx}: Use alias as defined intsconfig.jsonwhen possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` at line 16, Replace the brittle deep relative import in SessionSelector.tsx with the project alias import: update the line that currently imports getRelativeTime from "../../../../../../../WorkspacesListView/utils" to use the renderer alias (e.g., "renderer/..." path) so it matches other desktop code and tsconfig.json aliases; locate the import in SessionSelector.tsx and change only the module path to the corresponding renderer/... alias for the module that exports getRelativeTime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`:
- Around line 508-510: The handler handleSend currently references an undefined
variable text when calling onUserMessageSubmitted; replace both uses of text
with the correctly defined variable content so onUserMessageSubmitted?.(content)
is called; locate handleSend in ChatMastraInterface and update the two
occurrences to use content to fix the TypeScript error.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraPane.tsx`:
- Around line 96-126: The locally generated fallback title from
applySubmittedMessageFallbackTitle (it calls setPaneAutoTitle and
setTabAutoTitle for paneId/tabId) is being overwritten by the session sync in
GroupStrip where session.title || "New Chat" is applied; change GroupStrip (the
session-to-pane/tab sync logic) to skip applying "New Chat" when the persisted
session.title is empty (i.e., only apply session.title if it is non-empty) or
detect and preserve existing auto titles on the target pane/tab (check pane.name
and tab.userTitle / auto title state before writing), so empty persisted titles
do not clobber the locally computed fallback.
---
Duplicate comments:
In `@apps/desktop/src/renderer/stores/tabs/store.ts`:
- Around line 343-347: The debug call in setTabAutoTitle (and the other setter
that logs chat-derived titles) leaks raw user content via the `from`/`to`
fields; remove raw titles from the log and only emit non-sensitive metadata
(e.g., tabId and a redaction or hash/length indicator). Update the console.debug
in setTabAutoTitle and any similar debug statements so they no longer include
`tab.name` or `title` directly—replace them with a redacted placeholder or a
deterministic hash/length indicator and a short context string to preserve
usefulness without exposing user text.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Line 16: Replace the brittle deep relative import in SessionSelector.tsx with
the project alias import: update the line that currently imports getRelativeTime
from "../../../../../../../WorkspacesListView/utils" to use the renderer alias
(e.g., "renderer/..." path) so it matches other desktop code and tsconfig.json
aliases; locate the import in SessionSelector.tsx and change only the module
path to the corresponding renderer/... alias for the module that exports
getRelativeTime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 447fe4cf-d0c8-4696-970e-b02c248f25cf
📒 Files selected for processing (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/stores/tabs/types.ts
…itenite/familiar-frigate # Conflicts: # apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx (1)
116-121: Avoid using"New Chat"as a control-flow sentinel.This couples the fallback logic to display copy. If the label changes or gets localized, the selector will start treating placeholder text as a real title. Prefer having the caller pass
undefined/nullwhen no real fallback exists, or thread an explicit boolean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` around lines 116 - 121, The code uses the literal "New Chat" as a sentinel in resolvedFallbackTitle/currentTitle which breaks if the display label changes or is localized; update the logic in SessionSelector so fallbackTitle is treated as absent only when null/undefined (not when equal to "New Chat") and compute resolvedFallbackTitle from fallbackTitle != null && fallbackTitle !== "" (or add an explicit boolean like hasFallbackTitle) and then set currentTitle = current?.title || resolvedFallbackTitle || (isSessionInitializing ? "Creating Chat" : "New Chat"); also update callers to pass null/undefined (or set hasFallbackTitle=false) when there is no real fallback instead of passing the display string so the selector no longer relies on the UI text as control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 116-121: The code uses the literal "New Chat" as a sentinel in
resolvedFallbackTitle/currentTitle which breaks if the display label changes or
is localized; update the logic in SessionSelector so fallbackTitle is treated as
absent only when null/undefined (not when equal to "New Chat") and compute
resolvedFallbackTitle from fallbackTitle != null && fallbackTitle !== "" (or add
an explicit boolean like hasFallbackTitle) and then set currentTitle =
current?.title || resolvedFallbackTitle || (isSessionInitializing ? "Creating
Chat" : "New Chat"); also update callers to pass null/undefined (or set
hasFallbackTitle=false) when there is no real fallback instead of passing the
display string so the selector no longer relies on the UI text as control flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3d30134-f917-4474-b105-6d04dece5b19
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx
- apps/desktop/src/renderer/stores/tabs/types.ts
Summary
Validation
Follow-up
Summary by cubic
Restores Mastra chat title sync to tabs and panes with instant client-side fallbacks from the user’s last message. Titles now generate at send time with dedupe, and sessions scope and backfill correctly.
Bug Fixes
New Features
Written for commit 196dd11. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests