Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR implements Active Input Routing: Enter sends when idle, Enter steers active generations, and Tab queues drafts. It adds a steer IPC route, presenter/runtime steer handling to interrupt and restart streams, renderer event/toolbar updates, pending-input UI changes, i18n additions, and tests for routing and interruption. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant ChatService
participant SessionPresenter
participant RuntimePresenter
participant Stream
Note over Renderer,RuntimePresenter: Idle — Enter (send)
Renderer->>ChatService: sendMessage(sessionId, content)
ChatService->>SessionPresenter: sendMessage(sessionId, content)
SessionPresenter->>RuntimePresenter: processMessage(sessionId, content)
RuntimePresenter->>Stream: start generation
Note over Renderer,RuntimePresenter: Generating — Enter (steer)
Renderer->>ChatService: steerActiveTurn(sessionId, content)
ChatService->>SessionPresenter: steerActiveTurn(sessionId, content)
SessionPresenter->>RuntimePresenter: steerActiveTurn(sessionId, content)
RuntimePresenter->>Stream: abort (reason="steer")
Stream-->>RuntimePresenter: aborted
RuntimePresenter->>RuntimePresenter: settle partial assistant message
RuntimePresenter->>RuntimePresenter: consume steer buffer
RuntimePresenter->>Stream: restart generation with steer content
Note over Renderer: Generating — Tab (queue)
Renderer->>Renderer: emit queue-submit
Renderer->>Renderer: pendingInputStore.queueInput(content)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
🚥 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: 5
🧹 Nitpick comments (3)
test/main/routes/dispatcher.test.ts (1)
690-706: Assert the steer route response payload too.Line [690] currently validates only forwarding. Capturing and asserting the route result (
{ accepted: true }) would better protect the route contract.Proposed test tightening
- await dispatchDeepchatRoute( + const steerResult = await dispatchDeepchatRoute( runtime, 'chat.steerActiveTurn', { sessionId: 'session-1', content: 'refine the active answer' @@ expect(agentSessionPresenter.steerActiveTurn).toHaveBeenCalledWith( 'session-1', 'refine the active answer' ) + expect(steerResult).toEqual({ accepted: true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/routes/dispatcher.test.ts` around lines 690 - 706, The test currently only asserts that agentSessionPresenter.steerActiveTurn was called; capture the result returned by dispatchDeepchatRoute for the 'chat.steerActiveTurn' call and add an assertion that the response payload equals { accepted: true } to validate the route contract. Locate the invocation of dispatchDeepchatRoute in the test and store its return value, then add an expect(result).toEqual({ accepted: true }) (or equivalent) in addition to the existing expect for agentSessionPresenter.steerActiveTurn.test/renderer/components/ChatInputBox.test.ts (1)
282-313: Add thequeueSubmitEnabled: falsebranch to match the test intent.Current assertions cover Shift+Tab and disabled=true, but not the explicit disabled-by-feature-flag path.
♻️ Suggested test expansion
it('emits queue-submit on Tab only when queue submit is available', async () => { const wrapper = await mountComponent() await wrapper.setProps({ queueSubmitEnabled: true, queueSubmitDisabled: false }) await wrapper.get('[data-testid="chat-input-editor"]').trigger('keydown', { key: 'Tab' }) expect(wrapper.emitted('queue-submit')).toEqual([[]]) + await wrapper.setProps({ + queueSubmitEnabled: false, + queueSubmitDisabled: false + }) + await wrapper.get('[data-testid="chat-input-editor"]').trigger('keydown', { + key: 'Tab' + }) + + expect(wrapper.emitted('queue-submit')).toEqual([[]]) + await wrapper.setProps({ - queueSubmitDisabled: false + queueSubmitEnabled: true, + queueSubmitDisabled: false }) await wrapper.get('[data-testid="chat-input-editor"]').trigger('keydown', { key: 'Tab', shiftKey: true }) expect(wrapper.emitted('queue-submit')).toEqual([[]]) await wrapper.setProps({ + queueSubmitEnabled: true, queueSubmitDisabled: true }) await wrapper.get('[data-testid="chat-input-editor"]').trigger('keydown', { key: 'Tab' }) expect(wrapper.emitted('queue-submit')).toEqual([[]]) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/components/ChatInputBox.test.ts` around lines 282 - 313, The test 'emits queue-submit on Tab only when queue submit is available' needs an extra branch that sets queueSubmitEnabled: false and verifies no 'queue-submit' emission; update the test to setProps({ queueSubmitEnabled: false, queueSubmitDisabled: false }) (or just queueSubmitEnabled: false) then trigger the Tab key on the '[data-testid="chat-input-editor"]' and assert wrapper.emitted('queue-submit') remains unchanged, referencing the existing test function name and the wrapper.get('[data-testid="chat-input-editor"]') trigger to locate where to add this case.src/main/presenter/agentSessionPresenter/index.ts (1)
744-794: Extract the shared send/steer preflight.
steerActiveTurnnow duplicates most ofsendMessage's draft promotion, ACP provider resolution, workdir sync, and title-generation setup. Pulling that into one helper would make the new routing behavior much easier to keep aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/agentSessionPresenter/index.ts` around lines 744 - 794, steerActiveTurn duplicates sendMessage preflight (draft promotion, normalize input, resolve agent/state/provider, acp workdir checks/sync, and title-generation flag); extract a shared helper (e.g., prepareSendOrSteer) that takes sessionId and content and returns { session, normalizedInput, agent, state, providerId, wasDraft, hadMessages } after doing session lookup, normalizeSendMessageInput, sessionManager.update promotion, resolveAgentImplementation, agent.getSessionState/getMessages, providerId fallback for 'acp', assertAcpSessionHasWorkdir and syncAcpSessionWorkdir; call that helper from both steerActiveTurn and sendMessage and keep the same generateSessionTitle invocation logic (use generateSessionTitle(sessionId, session.title, providerId, state?.modelId ?? '') where appropriate).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/agentRuntimePresenter/index.ts`:
- Around line 424-463: The steer flow currently only triggers the steer-abort
handling in processMessage(), so when a streaming run started by
resumeAssistantMessage() is steered it is treated as a user_stop and never goes
through the consumeAbortSteerInput()/restart path, leaving
activeGenerationAbortReasons and steerInterruptInputs uncleared and losing the
steer payload; modify resumeAssistantMessage() (and the other similar resume
paths around the referenced ranges) to detect steer interrupts the same way
steerActiveTurn() does by calling enqueueSteerInterruptInput(sessionId,
normalizedInput) and routing the abort through consumeAbortSteerInput()/the
restart logic (instead of directly marking user_stop/cancel), ensure you set
activeGenerationAbortReasons.set(sessionId, 'steer') and abort the same
abortController used by activeGenerations.get(sessionId) so the existing
drain/cleanup of steerInterruptInputs and activeGenerationAbortReasons runs
consistently.
In `@src/main/presenter/agentSessionPresenter/index.ts`:
- Around line 728-731: The current interrupt path performs a heavy
getMessages(sessionId) before calling agent.steerActiveTurn, adding latency and
risk of rejecting interrupts on transient read failures; change the flow in the
block checking state?.status === 'generating' so it calls await
agent.steerActiveTurn(sessionId, normalizedInput) immediately (using the
existing sessionId/normalizedInput) and defer any full getMessages(sessionId) or
title-generation logic until after dispatch; alternatively, if a lightweight
check is needed before steering, call hasSessionMessages() or getMessageIds()
instead of getMessages() to avoid the full-history read; update the same pattern
in the adjacent region covering lines ~744-777 as well.
In `@src/renderer/src/components/chat/ChatInputBox.vue`:
- Around line 312-320: The handler currently treats plain Tab (isPlainTab) as
the "queue-submit" shortcut which blocks normal focus traversal; change the
shortcut detection so plain Tab is no longer intercepted and instead use a
non-Tab combo (for example Ctrl+Enter or Ctrl+Q). Update the condition that
checks isPlainTab to a new predicate (e.g., isQueueShortcut) that requires a
modifier (e.g., e.ctrlKey) and still respects
props.queueSubmitEnabled/props.queueSubmitDisabled and the mentions guards
(mentions.isSuggestionMenuOpen.value / mentions.shouldSuppressSubmit()), then
call emit('queue-submit') only when that new non-Tab shortcut is detected.
Ensure you remove e.preventDefault() for plain Tab so browser focus traversal
remains intact.
In `@src/renderer/src/pages/ChatPage.vue`:
- Around line 918-927: The queue path in onQueueSubmit currently calls
pendingInputStore.queueInput(props.sessionId, { text, files }) without source
metadata; update onQueueSubmit to pass an explicit source (e.g., include source:
'queue' on the input object) and then update pendingInputStore.queueInput and
the renderer→client forwarding to accept and forward that source through to
queuePendingInput(sessionId, input, options) (or the existing signature that
carries options.source) so the main-side handler can see options.source ===
'queue' instead of defaulting to 'send'. Ensure the symbol names to change are
onQueueSubmit, pendingInputStore.queueInput, and the code path that invokes
queuePendingInput/options.source so the source is preserved end-to-end.
In `@test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts`:
- Around line 582-595: The test is selecting the first user insert (which can be
the original prompt) instead of the steered user insert; update the call that
finds the insert in sqlitePresenter.deepchatMessagesTable.insert.mock.calls to
filter for the user row whose parsed content.text equals the steered text (e.g.,
'Refine before stream') so steeredUserInsert is selected reliably, and keep the
rest of the wait/assert logic (referencing steeredUserInsert and processStream)
unchanged.
---
Nitpick comments:
In `@src/main/presenter/agentSessionPresenter/index.ts`:
- Around line 744-794: steerActiveTurn duplicates sendMessage preflight (draft
promotion, normalize input, resolve agent/state/provider, acp workdir
checks/sync, and title-generation flag); extract a shared helper (e.g.,
prepareSendOrSteer) that takes sessionId and content and returns { session,
normalizedInput, agent, state, providerId, wasDraft, hadMessages } after doing
session lookup, normalizeSendMessageInput, sessionManager.update promotion,
resolveAgentImplementation, agent.getSessionState/getMessages, providerId
fallback for 'acp', assertAcpSessionHasWorkdir and syncAcpSessionWorkdir; call
that helper from both steerActiveTurn and sendMessage and keep the same
generateSessionTitle invocation logic (use generateSessionTitle(sessionId,
session.title, providerId, state?.modelId ?? '') where appropriate).
In `@test/main/routes/dispatcher.test.ts`:
- Around line 690-706: The test currently only asserts that
agentSessionPresenter.steerActiveTurn was called; capture the result returned by
dispatchDeepchatRoute for the 'chat.steerActiveTurn' call and add an assertion
that the response payload equals { accepted: true } to validate the route
contract. Locate the invocation of dispatchDeepchatRoute in the test and store
its return value, then add an expect(result).toEqual({ accepted: true }) (or
equivalent) in addition to the existing expect for
agentSessionPresenter.steerActiveTurn.
In `@test/renderer/components/ChatInputBox.test.ts`:
- Around line 282-313: The test 'emits queue-submit on Tab only when queue
submit is available' needs an extra branch that sets queueSubmitEnabled: false
and verifies no 'queue-submit' emission; update the test to setProps({
queueSubmitEnabled: false, queueSubmitDisabled: false }) (or just
queueSubmitEnabled: false) then trigger the Tab key on the
'[data-testid="chat-input-editor"]' and assert wrapper.emitted('queue-submit')
remains unchanged, referencing the existing test function name and the
wrapper.get('[data-testid="chat-input-editor"]') trigger to locate where to add
this case.
🪄 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: 90faec92-c455-4f41-a3fc-ee8eec35038e
📒 Files selected for processing (38)
docs/specs/active-input-routing/plan.mddocs/specs/active-input-routing/spec.mddocs/specs/active-input-routing/tasks.mdsrc/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/agentSessionPresenter/index.tssrc/main/routes/chat/chatService.tssrc/main/routes/hotPathPorts.tssrc/main/routes/index.tssrc/renderer/api/ChatClient.tssrc/renderer/src/components/chat/ChatInputBox.vuesrc/renderer/src/components/chat/ChatInputToolbar.vuesrc/renderer/src/components/chat/PendingInputLane.vuesrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/en-US/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/ja-JP/chat.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/ru-RU/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/ChatPage.vuesrc/shared/contracts/routes.tssrc/shared/contracts/routes/chat.routes.tssrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/agent-session.presenter.d.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/agentSessionPresenter/agentSessionPresenter.test.tstest/main/routes/chatService.test.tstest/main/routes/contracts.test.tstest/main/routes/dispatcher.test.tstest/renderer/api/clients.test.tstest/renderer/components/ChatInputBox.test.tstest/renderer/components/ChatInputToolbar.test.tstest/renderer/components/ChatPage.test.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/components/chat/PendingInputLane.vue
Comment on lines
+424
to
+463
| async steerActiveTurn(sessionId: string, content: string | SendMessageInput): Promise<void> { | ||
| const state = await this.getSessionState(sessionId) | ||
| if (!state) { | ||
| throw new Error(`Session ${sessionId} not found`) | ||
| } | ||
| if (this.isAwaitingToolQuestionFollowUp(sessionId) || this.hasPendingInteractions(sessionId)) { | ||
| throw new Error('Please resolve pending tool interactions before steering.') | ||
| } | ||
|
|
||
| const normalizedInput = this.normalizeUserMessageInput(content) | ||
| if (!normalizedInput.text.trim() && (normalizedInput.files?.length ?? 0) === 0) { | ||
| return | ||
| } | ||
|
|
||
| const activeGeneration = this.activeGenerations.get(sessionId) | ||
| if (!activeGeneration) { | ||
| const preStreamController = this.abortControllers.get(sessionId) | ||
| if (state.status === 'generating' && preStreamController) { | ||
| this.enqueueSteerInterruptInput(sessionId, normalizedInput) | ||
| this.activeGenerationAbortReasons.set(sessionId, 'steer') | ||
| preStreamController.abort() | ||
| this.abortDeferredToolAbortControllers(sessionId) | ||
| this.clearActiveProviderPermissionsForSession(sessionId) | ||
| return | ||
| } | ||
|
|
||
| void this.processMessage(sessionId, normalizedInput, { | ||
| projectDir: this.resolveProjectDir(sessionId) | ||
| }).catch((error) => { | ||
| console.error('[AgentRuntime] Failed to process steer input:', error) | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| this.enqueueSteerInterruptInput(sessionId, normalizedInput) | ||
| this.activeGenerationAbortReasons.set(sessionId, 'steer') | ||
| activeGeneration.abortController.abort() | ||
| this.abortDeferredToolAbortControllers(sessionId) | ||
| this.clearActiveProviderPermissionsForSession(sessionId) | ||
| } |
Contributor
There was a problem hiding this comment.
Handle steer-triggered aborts from resumed generations too.
This new steer flow is only completed in processMessage(). If the user steers while a run started by resumeAssistantMessage() is streaming, that path still treats the abort as user_stop, marks the assistant message canceled, and never drains activeGenerationAbortReasons / steerInterruptInputs. The steer payload can then be lost or leak into a later unrelated abort.
Please route resumeAssistantMessage() through the same consumeAbortSteerInput() / restart path.
Also applies to: 684-693, 721-751
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/agentRuntimePresenter/index.ts` around lines 424 - 463,
The steer flow currently only triggers the steer-abort handling in
processMessage(), so when a streaming run started by resumeAssistantMessage() is
steered it is treated as a user_stop and never goes through the
consumeAbortSteerInput()/restart path, leaving activeGenerationAbortReasons and
steerInterruptInputs uncleared and losing the steer payload; modify
resumeAssistantMessage() (and the other similar resume paths around the
referenced ranges) to detect steer interrupts the same way steerActiveTurn()
does by calling enqueueSteerInterruptInput(sessionId, normalizedInput) and
routing the abort through consumeAbortSteerInput()/the restart logic (instead of
directly marking user_stop/cancel), ensure you set
activeGenerationAbortReasons.set(sessionId, 'steer') and abort the same
abortController used by activeGenerations.get(sessionId) so the existing
drain/cleanup of steerInterruptInputs and activeGenerationAbortReasons runs
consistently.
Comment on lines
+728
to
+731
| if (state?.status === 'generating' && agent.steerActiveTurn) { | ||
| await agent.steerActiveTurn(sessionId, normalizedInput) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Keep steering off the full-history path.
These entry points now do a full getMessages(sessionId) read before they can call agent.steerActiveTurn, even though that lookup is only used later for title generation. That adds avoidable latency to the new interrupt path, and a transient history-read failure will now reject the send/steer request before it ever reaches the runtime. Please defer the first-message check until after dispatch, or reuse the lighter hasSessionMessages() / getMessageIds() path here.
Also applies to: 744-777
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/agentSessionPresenter/index.ts` around lines 728 - 731,
The current interrupt path performs a heavy getMessages(sessionId) before
calling agent.steerActiveTurn, adding latency and risk of rejecting interrupts
on transient read failures; change the flow in the block checking state?.status
=== 'generating' so it calls await agent.steerActiveTurn(sessionId,
normalizedInput) immediately (using the existing sessionId/normalizedInput) and
defer any full getMessages(sessionId) or title-generation logic until after
dispatch; alternatively, if a lightweight check is needed before steering, call
hasSessionMessages() or getMessageIds() instead of getMessages() to avoid the
full-history read; update the same pattern in the adjacent region covering lines
~744-777 as well.
Comment on lines
+312
to
+320
| const isPlainTab = e.key === 'Tab' && !e.shiftKey && !e.altKey && !e.ctrlKey && !e.metaKey | ||
| if (isPlainTab && props.queueSubmitEnabled && !props.queueSubmitDisabled) { | ||
| if (mentions.isSuggestionMenuOpen.value || mentions.shouldSuppressSubmit()) { | ||
| return | ||
| } | ||
| e.preventDefault() | ||
| emit('queue-submit') | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Keep plain Tab for focus traversal.
Intercepting plain Tab here blocks normal keyboard navigation whenever streaming + draft input are active, so users can no longer tab out of the editor to the stop/queue controls or the rest of the page. Please move queueing to a non-Tab shortcut and leave plain Tab to the browser.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/components/chat/ChatInputBox.vue` around lines 312 - 320,
The handler currently treats plain Tab (isPlainTab) as the "queue-submit"
shortcut which blocks normal focus traversal; change the shortcut detection so
plain Tab is no longer intercepted and instead use a non-Tab combo (for example
Ctrl+Enter or Ctrl+Q). Update the condition that checks isPlainTab to a new
predicate (e.g., isQueueShortcut) that requires a modifier (e.g., e.ctrlKey) and
still respects props.queueSubmitEnabled/props.queueSubmitDisabled and the
mentions guards (mentions.isSuggestionMenuOpen.value /
mentions.shouldSuppressSubmit()), then call emit('queue-submit') only when that
new non-Tab shortcut is detected. Ensure you remove e.preventDefault() for plain
Tab so browser focus traversal remains intact.
Comment on lines
+918
to
927
| async function onQueueSubmit() { | ||
| if (isReadOnlySession.value) return | ||
| if (isAcpWorkdirMissing.value) return | ||
| if (activePendingInteraction.value || isHandlingInteraction.value) return | ||
| const text = message.value.trim() | ||
| const files = [...attachedFiles.value].map((f) => toRaw(f)) | ||
| if (!text && files.length === 0) return | ||
| await pendingInputStore.queueInput(props.sessionId, { text, files }) | ||
| message.value = '' | ||
| attachedFiles.value = [] |
Contributor
There was a problem hiding this comment.
Pass an explicit queue source here.
onQueueSubmit() currently goes through pendingInputStore.queueInput(...), and that store forwards to queuePendingInput(sessionId, input) without source metadata. On the main side, missing options.source is treated as 'send', so this “queue” action can be claimed immediately during tool follow-up instead of remaining an editable future turn.
Please plumb a dedicated queue source (for example, 'queue') through the renderer store/client for this path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/pages/ChatPage.vue` around lines 918 - 927, The queue path
in onQueueSubmit currently calls pendingInputStore.queueInput(props.sessionId, {
text, files }) without source metadata; update onQueueSubmit to pass an explicit
source (e.g., include source: 'queue' on the input object) and then update
pendingInputStore.queueInput and the renderer→client forwarding to accept and
forward that source through to queuePendingInput(sessionId, input, options) (or
the existing signature that carries options.source) so the main-side handler can
see options.source === 'queue' instead of defaulting to 'send'. Ensure the
symbol names to change are onQueueSubmit, pendingInputStore.queueInput, and the
code path that invokes queuePendingInput/options.source so the source is
preserved end-to-end.
Comment on lines
+582
to
+595
| let steeredUserInsert: any = null | ||
| for (let attempt = 0; attempt < 20; attempt += 1) { | ||
| steeredUserInsert = sqlitePresenter.deepchatMessagesTable.insert.mock.calls.find( | ||
| ([row]) => row.role === 'user' | ||
| )?.[0] | ||
| if (steeredUserInsert) { | ||
| break | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, 0)) | ||
| } | ||
|
|
||
| expect(steeredUserInsert).toBeTruthy() | ||
| expect(JSON.parse(steeredUserInsert.content).text).toBe('Refine before stream') | ||
| expect(processStream).toHaveBeenCalledTimes(1) |
Contributor
There was a problem hiding this comment.
Select the steered user insert, not the first user insert.
processMessage('First prompt') already inserts a user row, so .find(([row]) => row.role === 'user') will usually grab the original prompt instead of the steered one. That makes this assertion validate the wrong record and can fail even when steering works.
Suggested fix
let steeredUserInsert: any = null
for (let attempt = 0; attempt < 20; attempt += 1) {
- steeredUserInsert = sqlitePresenter.deepchatMessagesTable.insert.mock.calls.find(
- ([row]) => row.role === 'user'
- )?.[0]
- if (steeredUserInsert) {
+ const userInserts = sqlitePresenter.deepchatMessagesTable.insert.mock.calls
+ .map(([row]) => row)
+ .filter((row) => row.role === 'user')
+ steeredUserInsert = userInserts[userInserts.length - 1] ?? null
+ if (
+ steeredUserInsert &&
+ JSON.parse(steeredUserInsert.content).text === 'Refine before stream'
+ ) {
break
}
await new Promise((resolve) => setTimeout(resolve, 0))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let steeredUserInsert: any = null | |
| for (let attempt = 0; attempt < 20; attempt += 1) { | |
| steeredUserInsert = sqlitePresenter.deepchatMessagesTable.insert.mock.calls.find( | |
| ([row]) => row.role === 'user' | |
| )?.[0] | |
| if (steeredUserInsert) { | |
| break | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, 0)) | |
| } | |
| expect(steeredUserInsert).toBeTruthy() | |
| expect(JSON.parse(steeredUserInsert.content).text).toBe('Refine before stream') | |
| expect(processStream).toHaveBeenCalledTimes(1) | |
| let steeredUserInsert: any = null | |
| for (let attempt = 0; attempt < 20; attempt += 1) { | |
| const userInserts = sqlitePresenter.deepchatMessagesTable.insert.mock.calls | |
| .map(([row]) => row) | |
| .filter((row) => row.role === 'user') | |
| steeredUserInsert = userInserts[userInserts.length - 1] ?? null | |
| if ( | |
| steeredUserInsert && | |
| JSON.parse(steeredUserInsert.content).text === 'Refine before stream' | |
| ) { | |
| break | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, 0)) | |
| } | |
| expect(steeredUserInsert).toBeTruthy() | |
| expect(JSON.parse(steeredUserInsert.content).text).toBe('Refine before stream') | |
| expect(processStream).toHaveBeenCalledTimes(1) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts`
around lines 582 - 595, The test is selecting the first user insert (which can
be the original prompt) instead of the steered user insert; update the call that
finds the insert in sqlitePresenter.deepchatMessagesTable.insert.mock.calls to
filter for the user row whose parsed content.text equals the steered text (e.g.,
'Refine before stream') so steeredUserInsert is selected reliably, and keep the
rest of the wait/assert logic (referencing steeredUserInsert and processStream)
unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/renderer/components/ChatPage.test.ts (1)
287-311:⚠️ Potential issue | 🟡 MinorChatInputToolbar mock doesn't render the steer button needed by the capacity test.
The mock template is a simple
<div class="chat-input-toolbar-stub" />, but the test at line 718-719 expects to find[data-testid="chat-steer-button"]. This assertion will always fail because the mock doesn't render any internal elements.Either update the mock to conditionally render a steer button element when
isGenerating && hasInput, or change the test to verify steer availability through a different mechanism (e.g., checking thatqueueDisableddoesn't imply steer is disabled).🔧 Suggested mock update
vi.doMock('@/components/chat/ChatInputToolbar.vue', () => ({ default: defineComponent({ name: 'ChatInputToolbar', props: { isGenerating: { type: Boolean, default: false }, hasInput: { type: Boolean, default: false }, sendDisabled: { type: Boolean, default: false }, queueDisabled: { type: Boolean, default: false } }, - emits: ['attach', 'queue', 'send', 'stop'], - template: '<div class="chat-input-toolbar-stub" />' + emits: ['attach', 'queue', 'steer', 'send', 'stop'], + template: `<div class="chat-input-toolbar-stub"> + <button v-if="isGenerating && hasInput" data-testid="chat-steer-button" `@click`="$emit('steer')" /> + </div>` }) }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/components/ChatPage.test.ts` around lines 287 - 311, The ChatInputToolbar mock currently renders only a stub div so tests that query [data-testid="chat-steer-button"] fail; update the mock (the defineComponent used in vi.doMock for ChatInputToolbar) so its template conditionally includes a button element with data-testid="chat-steer-button" when the props isGenerating and hasInput are true (keep the existing props and emits), or alternatively adjust the test to assert steer availability by inspecting the queueDisabled/sendDisabled props rather than querying the DOM; implementing the conditional button in the ChatInputToolbar mock is the recommended fix.
♻️ Duplicate comments (2)
src/main/presenter/agentRuntimePresenter/index.ts (1)
688-696:⚠️ Potential issue | 🔴 CriticalHandle steer aborts in
resumeAssistantMessage()too.The new steer restart path is only wired into
processMessage(). A steer issued while a stream resumed byresumeAssistantMessage()is running still falls through the olduser_stoppath, which cancels the assistant message and leaves the buffered steer input behind for a later abort. Please route resumed runs through the sameconsumeAbortSteerInput()/ restart flow.Also applies to: 725-755
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/agentRuntimePresenter/index.ts` around lines 688 - 696, resumeAssistantMessage() currently leaves steer-abort inputs unhandled and falls back to the old user_stop path; update resumeAssistantMessage() to mirror the steer-abort restart flow used in processMessage(): call consumeAbortSteerInput(sessionId) at the start of resumed runs, and if it returns a steerInput, invoke settleSteerInterruptedAssistant(sessionId, assistantMessageId), setSessionStatus(sessionId, 'idle'), ensure clearActiveGeneration(sessionId, runId) is called in a finally block, and then call continueWithSteerInput(sessionId, steerInput, projectDir) so resumed streams are restarted the same way as in processMessage().test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts (1)
592-600:⚠️ Potential issue | 🟠 MajorSelect the steered user insert, not the first user insert.
At Line 594,
.find(([row]) => row.role === 'user')can grab the original'First prompt'row, so Line 604 may assert against the wrong record.Proposed fix
let steeredUserInsert: any = null for (let attempt = 0; attempt < 20; attempt += 1) { - steeredUserInsert = sqlitePresenter.deepchatMessagesTable.insert.mock.calls.find( - ([row]) => row.role === 'user' - )?.[0] + const userInserts = sqlitePresenter.deepchatMessagesTable.insert.mock.calls + .map(([row]) => row) + .filter((row) => row.role === 'user') + steeredUserInsert = + userInserts.find((row) => JSON.parse(row.content).text === 'Refine before stream') ?? null if (steeredUserInsert) { break } await new Promise((resolve) => setTimeout(resolve, 0)) }Also applies to: 603-604
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts` around lines 592 - 600, The test currently picks the first user insert via sqlitePresenter.deepchatMessagesTable.insert.mock.calls.find(([row]) => row.role === 'user'), which may return the original "First prompt" row; update the predicate used in that find to specifically identify the steered user insert (for example check row.role === 'user' && (row.metadata?.steered === true || row.content !== 'First prompt' or another unique marker produced by the steered flow)) so steeredUserInsert references the actual steered record; adjust the similar selection later in the test the same way.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/agentRuntimePresenter/index.ts`:
- Around line 688-696: The abort path currently calls
rollbackClaimedQueueInputTurn(...) before handling steer restarts, causing
queued inputs to be requeued for steer-triggered aborts; change the control flow
in the abort handling around consumeAbortSteerInput(sessionId) so that if
consumeAbortSteerInput returns a steerInput you do NOT call
rollbackClaimedQueueInputTurn or requeue the pendingQueueItem (treat it as
consumed), but still call settleSteerInterruptedAssistant(sessionId,
assistantMessageId), setSessionStatus(sessionId, 'idle'),
clearActiveGeneration(sessionId, runId), and then
continueWithSteerInput(sessionId, steerInput, projectDir); ensure the same
adjustment is applied to the other abort block(s) referenced (the 709-755
region) so steer-triggered aborts skip rollbackClaimedQueueInputTurn.
In `@src/main/presenter/agentSessionPresenter/index.ts`:
- Around line 792-794: The presenter currently silently returns when
agent.steerActiveTurn is missing; change it to fail fast by checking for the
capability and throwing the same unsupported-operation error used by other
capability-gated entry points instead of returning. In the method containing the
agent.steerActiveTurn call, replace the no-op path with an explicit throw (e.g.,
throw new Error or the project’s UnsupportedOperationError) with a clear message
like "Agent does not support steering", and only call await
agent.steerActiveTurn(sessionId, normalizedInput) when the property exists.
In `@src/renderer/src/pages/ChatPage.vue`:
- Around line 894-898: The queued-input code path is missing source metadata
when calling pendingInputStore.queueInput from onSubmit and onCommandSubmit
while isGenerating is true, causing queued items to be claimed immediately;
update both onSubmit and onCommandSubmit to pass the same source options you use
for chatClient.sendMessage (or create a small helper e.g.,
queueOrSend(sessionId, payload, source)) so that
pendingInputStore.queueInput(sessionId, { text, files, source:
<appropriateSource> }) is always called, or alternatively extend
pendingInputStore.queueInput to accept and forward a source option; reference
onSubmit, onCommandSubmit, pendingInputStore.queueInput, isGenerating and
chatClient.sendMessage when making the change.
---
Outside diff comments:
In `@test/renderer/components/ChatPage.test.ts`:
- Around line 287-311: The ChatInputToolbar mock currently renders only a stub
div so tests that query [data-testid="chat-steer-button"] fail; update the mock
(the defineComponent used in vi.doMock for ChatInputToolbar) so its template
conditionally includes a button element with data-testid="chat-steer-button"
when the props isGenerating and hasInput are true (keep the existing props and
emits), or alternatively adjust the test to assert steer availability by
inspecting the queueDisabled/sendDisabled props rather than querying the DOM;
implementing the conditional button in the ChatInputToolbar mock is the
recommended fix.
---
Duplicate comments:
In `@src/main/presenter/agentRuntimePresenter/index.ts`:
- Around line 688-696: resumeAssistantMessage() currently leaves steer-abort
inputs unhandled and falls back to the old user_stop path; update
resumeAssistantMessage() to mirror the steer-abort restart flow used in
processMessage(): call consumeAbortSteerInput(sessionId) at the start of resumed
runs, and if it returns a steerInput, invoke
settleSteerInterruptedAssistant(sessionId, assistantMessageId),
setSessionStatus(sessionId, 'idle'), ensure clearActiveGeneration(sessionId,
runId) is called in a finally block, and then call
continueWithSteerInput(sessionId, steerInput, projectDir) so resumed streams are
restarted the same way as in processMessage().
In `@test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts`:
- Around line 592-600: The test currently picks the first user insert via
sqlitePresenter.deepchatMessagesTable.insert.mock.calls.find(([row]) => row.role
=== 'user'), which may return the original "First prompt" row; update the
predicate used in that find to specifically identify the steered user insert
(for example check row.role === 'user' && (row.metadata?.steered === true ||
row.content !== 'First prompt' or another unique marker produced by the steered
flow)) so steeredUserInsert references the actual steered record; adjust the
similar selection later in the test the same way.
🪄 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: b92f4634-e2fc-44b5-a2ff-0a3b14c0f3f8
📒 Files selected for processing (9)
src/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/agentSessionPresenter/index.tssrc/renderer/src/components/chat/ChatInputToolbar.vuesrc/renderer/src/pages/ChatPage.vuesrc/shared/types/agent-interface.d.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/agentSessionPresenter/agentSessionPresenter.test.tstest/renderer/components/ChatInputToolbar.test.tstest/renderer/components/ChatPage.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/shared/types/agent-interface.d.ts
- test/main/presenter/agentSessionPresenter/agentSessionPresenter.test.ts
Comment on lines
+688
to
+696
| const steerInput = result.status === 'aborted' ? this.consumeAbortSteerInput(sessionId) : null | ||
| if (steerInput) { | ||
| try { | ||
| this.settleSteerInterruptedAssistant(sessionId, assistantMessageId) | ||
| this.setSessionStatus(sessionId, 'idle') | ||
| } finally { | ||
| this.clearActiveGeneration(sessionId, runId) | ||
| } | ||
| this.continueWithSteerInput(sessionId, steerInput, projectDir) |
Contributor
There was a problem hiding this comment.
Do not rollback claimed queue items when the abort reason is steer.
For immediately claimed queued inputs (pendingQueueItemSource === 'queue'), the aborted path rolls the item back before the steer restart is checked. That deletes the active user turn and re-queues the old draft, so after the steer restart the original queued input can resurface and run later out of order. Treat steer-triggered aborts as a consumed turn instead of calling rollbackClaimedQueueInputTurn(...).
Also applies to: 709-755
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/agentRuntimePresenter/index.ts` around lines 688 - 696,
The abort path currently calls rollbackClaimedQueueInputTurn(...) before
handling steer restarts, causing queued inputs to be requeued for
steer-triggered aborts; change the control flow in the abort handling around
consumeAbortSteerInput(sessionId) so that if consumeAbortSteerInput returns a
steerInput you do NOT call rollbackClaimedQueueInputTurn or requeue the
pendingQueueItem (treat it as consumed), but still call
settleSteerInterruptedAssistant(sessionId, assistantMessageId),
setSessionStatus(sessionId, 'idle'), clearActiveGeneration(sessionId, runId),
and then continueWithSteerInput(sessionId, steerInput, projectDir); ensure the
same adjustment is applied to the other abort block(s) referenced (the 709-755
region) so steer-triggered aborts skip rollbackClaimedQueueInputTurn.
Comment on lines
+792
to
+794
| if (agent.steerActiveTurn) { | ||
| await agent.steerActiveTurn(sessionId, normalizedInput) | ||
| } |
Contributor
There was a problem hiding this comment.
Fail fast when an agent does not support steering.
This method currently returns successfully when agent.steerActiveTurn is absent, so the caller gets no signal that the steer was dropped. The other capability-gated entry points in this presenter throw explicit unsupported-operation errors; this should do the same.
Suggested fix
- if (agent.steerActiveTurn) {
- await agent.steerActiveTurn(sessionId, normalizedInput)
- }
+ if (!agent.steerActiveTurn) {
+ throw new Error(`Agent ${session.agentId} does not support active-turn steering.`)
+ }
+
+ await agent.steerActiveTurn(sessionId, normalizedInput)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/presenter/agentSessionPresenter/index.ts` around lines 792 - 794,
The presenter currently silently returns when agent.steerActiveTurn is missing;
change it to fail fast by checking for the capability and throwing the same
unsupported-operation error used by other capability-gated entry points instead
of returning. In the method containing the agent.steerActiveTurn call, replace
the no-op path with an explicit throw (e.g., throw new Error or the project’s
UnsupportedOperationError) with a clear message like "Agent does not support
steering", and only call await agent.steerActiveTurn(sessionId, normalizedInput)
when the property exists.
Comment on lines
+894
to
+898
| if (isGenerating.value) { | ||
| await pendingInputStore.queueInput(props.sessionId, { text, files }) | ||
| } else { | ||
| await chatClient.sendMessage(props.sessionId, { text, files }) | ||
| } |
Contributor
There was a problem hiding this comment.
Same missing source metadata issue in onSubmit and onCommandSubmit queue paths.
Both onSubmit (line 895) and onCommandSubmit (line 912) now route to pendingInputStore.queueInput(...) when isGenerating is true, but neither passes source metadata. This has the same implication as onQueueSubmit — the queued input may be claimed immediately rather than remaining editable.
Consider consolidating these queue calls through a helper that always passes the appropriate source, or update pendingInputStore.queueInput to accept and forward source options.
Also applies to: 911-916
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/pages/ChatPage.vue` around lines 894 - 898, The queued-input
code path is missing source metadata when calling pendingInputStore.queueInput
from onSubmit and onCommandSubmit while isGenerating is true, causing queued
items to be claimed immediately; update both onSubmit and onCommandSubmit to
pass the same source options you use for chatClient.sendMessage (or create a
small helper e.g., queueOrSend(sessionId, payload, source)) so that
pendingInputStore.queueInput(sessionId, { text, files, source:
<appropriateSource> }) is always called, or alternatively extend
pendingInputStore.queueInput to accept and forward a source option; reference
onSubmit, onCommandSubmit, pendingInputStore.queueInput, isGenerating and
chatClient.sendMessage when making the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI Updates
Documentation
Tests