feat: add default chat workspace#1799
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a built-in default chat workspace ( ChangesDefault Chat Workspace + First-Turn Readiness
Session Start Lag: AGENTS.md Cache
Sequence Diagram(s)sequenceDiagram
participant Renderer as ChatTabView (Renderer)
participant Route as startupGetBootstrapRoute
participant ProjectPresenter
participant ProjectStore as useProjectStore
participant WindowSideBar
Renderer->>Route: startup.getBootstrap
Route->>ProjectPresenter: ensureDefaultWorkspace()
ProjectPresenter-->>Route: defaultChatWorkspacePath
Route-->>Renderer: bootstrap { defaultProjectPath, defaultChatWorkspacePath }
Renderer->>ProjectStore: applyBootstrapDefaultProjectPath(defaultProjectPath, defaultChatWorkspacePath)
ProjectStore-->>WindowSideBar: defaultChatWorkspacePath ref updated
WindowSideBar->>WindowSideBar: isChatsGroup() detects built-in group
WindowSideBar->>WindowSideBar: getGroupLabel → "Chats", getGroupIcon → chat icon
sequenceDiagram
participant AgentSessionPresenter
participant AgentRuntimePresenter
participant processStream
participant waitForFirstTurnReady
AgentSessionPresenter->>AgentRuntimePresenter: processMessage(sessionId, msg)
AgentRuntimePresenter->>processStream: run with onFirstProviderRoundReady callback
processStream->>processStream: first non-empty blocks flushed
processStream->>AgentRuntimePresenter: onFirstProviderRoundReady()
AgentRuntimePresenter->>waitForFirstTurnReady: markFirstTurnReady(sessionId)
waitForFirstTurnReady-->>AgentSessionPresenter: resolves true
AgentSessionPresenter->>AgentSessionPresenter: waitForSessionTitleMessages → fetch messages → generateTitle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/presenter/agentRuntimePresenter/index.ts (1)
970-970: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMeasure the full pre-stream boundary.
pre-stream-totalstops beforerunStreamForMessage, so it misses the remaining setup in that method (SessionStart, context recovery, and rate-limit waiting) that still happens before the provider starts streaming. Move the timer intorunStreamForMessageor rename the metric so the warning reflects what it measures.🤖 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 `@src/main/presenter/agentRuntimePresenter/index.ts` at line 970, The logSlowPreStreamStep call with the 'pre-stream-total' metric is being logged before all pre-stream setup operations complete. Move the logSlowPreStreamStep call (currently logging 'pre-stream-total' with preStreamStartedAt) from its current location to the end of the runStreamForMessage method so that the timer captures the complete pre-stream boundary including SessionStart initialization, context recovery, and rate-limit waiting that occur within runStreamForMessage before the provider actually starts streaming.src/main/routes/index.ts (1)
2476-2491: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider parallelizing
ensureDefaultWorkspace()with agent loading.The
defaultChatWorkspacePathis fetched sequentially before thePromise.allthat loads agents and ACP status. Since these operations appear independent, running them in parallel could reduce bootstrap latency.⚡ Suggested optimization
if (!coordinator) { - const defaultChatWorkspacePath = await runtime.projectPresenter.ensureDefaultWorkspace() const activeSessionId = runtime.agentSessionPresenter.getActiveSessionId( context.webContentsId ) const activeSession = activeSessionId ? (( await runtime.agentSessionPresenter.getLightweightSessionsByIds([activeSessionId]) )[0] ?? null) : null - const [agents, acpEnabled] = await Promise.all([ + const [agents, acpEnabled, defaultChatWorkspacePath] = await Promise.all([ runtime.configPresenter.listAgents(), - runtime.configPresenter.getAcpEnabled() + runtime.configPresenter.getAcpEnabled(), + runtime.projectPresenter.ensureDefaultWorkspace() ])Apply the same pattern to the coordinator path at lines 2527-2541.
🤖 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 `@src/main/routes/index.ts` around lines 2476 - 2491, The ensureDefaultWorkspace() call is being executed sequentially before the Promise.all block that loads agents and ACP status, causing unnecessary sequential delays. Move the ensureDefaultWorkspace() call into the Promise.all alongside the runtime.configPresenter.listAgents() and runtime.configPresenter.getAcpEnabled() calls to parallelize these independent operations. Apply the same parallelization pattern to the coordinator path section mentioned in the review to maintain consistency across the codebase.
🤖 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 `@src/renderer/src/stores/ui/project.ts`:
- Around line 100-102: The code block checking if defaultChatWorkspacePath
differs from defaultProjectPath and clearing it introduces an unwanted coupling
between two independent concepts: the system-managed chat workspace path and the
user-configurable default project path. Since the chat workspace should remain
stable at its system location regardless of project path changes, you should
remove this conditional logic (the if block checking and resetting
defaultChatWorkspacePath). If there is a deliberate architectural reason for
this coupling, add a clear comment explaining the intended relationship before
the conditional instead of silently clearing the value.
In `@test/main/presenter/projectPresenter/projectPresenter.test.ts`:
- Around line 175-194: The warnSpy created on the console.warn object is only
restored at the end of the test after all assertions. If any assertion fails
before reaching the warnSpy.mockRestore() call, the spy remains active and
affects subsequent tests. Wrap the test logic in a try/finally block so that
warnSpy.mockRestore() is guaranteed to execute in the finally block, ensuring
cleanup happens regardless of whether assertions pass or fail.
In `@test/renderer/components/WindowSideBar.test.ts`:
- Around line 1310-1380: The test setup has a path mismatch between the
defaultChatWorkspacePath and the projectEnvironments list. The
defaultChatWorkspacePath is set to '/Users/test/Documents/DeepChat/' with a
trailing slash, while the corresponding path in projectEnvironments is
'/Users/test/Documents/DeepChat' without the trailing slash. Since the
normalizePath function only trims whitespace and doesn't remove trailing
slashes, these paths won't match when compared by the isChatsGroup filter,
causing the chat workspace to not be properly filtered out during reordering.
Fix this by removing the trailing slash from the defaultChatWorkspacePath value
in the setup call to make it consistent with the projectEnvironments entry.
---
Nitpick comments:
In `@src/main/presenter/agentRuntimePresenter/index.ts`:
- Line 970: The logSlowPreStreamStep call with the 'pre-stream-total' metric is
being logged before all pre-stream setup operations complete. Move the
logSlowPreStreamStep call (currently logging 'pre-stream-total' with
preStreamStartedAt) from its current location to the end of the
runStreamForMessage method so that the timer captures the complete pre-stream
boundary including SessionStart initialization, context recovery, and rate-limit
waiting that occur within runStreamForMessage before the provider actually
starts streaming.
In `@src/main/routes/index.ts`:
- Around line 2476-2491: The ensureDefaultWorkspace() call is being executed
sequentially before the Promise.all block that loads agents and ACP status,
causing unnecessary sequential delays. Move the ensureDefaultWorkspace() call
into the Promise.all alongside the runtime.configPresenter.listAgents() and
runtime.configPresenter.getAcpEnabled() calls to parallelize these independent
operations. Apply the same parallelization pattern to the coordinator path
section mentioned in the review to maintain consistency across the codebase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a62ee4b-70f3-4059-a19b-7c0edbe56a2e
📒 Files selected for processing (46)
docs/features/default-workspace/plan.mddocs/features/default-workspace/spec.mddocs/features/default-workspace/tasks.mddocs/issues/session-start-lag/plan.mddocs/issues/session-start-lag/spec.mddocs/issues/session-start-lag/tasks.mdsrc/main/lib/agentRuntime/systemEnvPromptBuilder.tssrc/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/agentSessionPresenter/index.tssrc/main/presenter/projectPresenter/index.tssrc/main/routes/index.tssrc/renderer/src/components/WindowSideBar.vuesrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/de-DE/chat.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/es-ES/chat.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/id-ID/chat.jsonsrc/renderer/src/i18n/it-IT/chat.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ms-MY/chat.jsonsrc/renderer/src/i18n/pl-PL/chat.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/tr-TR/chat.jsonsrc/renderer/src/i18n/vi-VN/chat.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/renderer/src/pages/NewThreadPage.vuesrc/renderer/src/stores/ui/project.tssrc/renderer/src/views/ChatTabView.vuesrc/shared/contracts/common.tssrc/shared/contracts/routes/sessions.routes.tssrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/project.presenter.d.tstest/main/lib/agentRuntime/systemEnvPromptBuilder.test.tstest/main/presenter/agentSessionPresenter/agentSessionPresenter.test.tstest/main/presenter/projectPresenter/projectPresenter.test.tstest/main/routes/dispatcher.test.tstest/renderer/components/NewThreadPage.test.tstest/renderer/components/WindowSideBar.test.tstest/renderer/stores/projectStore.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/main/presenter/agentSessionPresenter/index.ts`:
- Around line 2519-2545: The waitForSessionTitleMessages method unconditionally
polls for up to 30 seconds waiting for messages, which causes unnecessary
background work for sessions without an initial turn and can lead to duplicate
title-generation calls. Add a guard condition at the call-site where
waitForSessionTitleMessages is invoked to check if polling is actually needed
before calling the method, such as verifying the session has messages or some
initial state, so that empty sessions don't trigger expensive long-lived polling
loops.
In `@test/main/presenter/agentSessionPresenter/agentSessionPresenter.test.ts`:
- Around line 770-775: The test uses wall-clock dependent setTimeout waits (20ms
and 300ms) which can cause intermittent failures on slow CI systems. Replace
these timing waits with Vitest's fake timers by calling vi.useFakeTimers() at
the start of the test, then replace each await new Promise((r) => setTimeout(r,
delay)) call with vi.advanceTimersByTime(delay) to provide deterministic timing
control. Make sure to restore real timers at the end of the test using
vi.useRealTimers() to avoid affecting other tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58a10935-7d77-451a-8692-b05a5def7113
📒 Files selected for processing (6)
docs/features/default-workspace/spec.mddocs/issues/session-title-generation/spec.mdsrc/main/presenter/agentSessionPresenter/index.tssrc/renderer/src/components/WindowSideBar.vuetest/main/presenter/agentSessionPresenter/agentSessionPresenter.test.tstest/renderer/components/WindowSideBar.test.ts
✅ Files skipped from review due to trivial changes (2)
- docs/issues/session-title-generation/spec.md
- docs/features/default-workspace/spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
- test/renderer/components/WindowSideBar.test.ts
Contributor
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 `@src/main/presenter/agentRuntimePresenter/index.ts`:
- Line 529: Add guard conditions to the `onFirstProviderRoundReady` callback to
prevent stale stream callbacks from re-marking a session as ready after it has
been cleared or destroyed. Before marking the session as ready in this callback,
check that the session's active run ID matches the current run and that the
abort signal has not been triggered. This ensures that callbacks from streams
initiated before clearMessages or destroySession was called will be ignored and
cannot resurrect the readiness state of a cleared or destroyed session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bab2ef12-3b05-4c22-8598-523f9754950d
📒 Files selected for processing (8)
src/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/agentRuntimePresenter/process.tssrc/main/presenter/agentRuntimePresenter/types.tssrc/main/presenter/agentSessionPresenter/index.tssrc/shared/types/agent-interface.d.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/agentRuntimePresenter/process.test.tstest/main/presenter/agentSessionPresenter/agentSessionPresenter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/presenter/agentSessionPresenter/index.ts
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.
Summary
Closes #1795
Tests
Summary by CodeRabbit
AGENTS.mdreads with timeout budgeting.