Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 171 additions & 0 deletions apps/desktop/src/main/services/chat/agentChatService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3617,6 +3617,69 @@ describe("createAgentChatService", () => {
]);
});

it("keeps Codex reasoning deltas tied to the active turn and thinking activity", async () => {
const events: AgentChatEventEnvelope[] = [];
const { service } = createService({
onEvent: (event: AgentChatEventEnvelope) => {
events.push(event);
},
});

const session = await service.createSession({
laneId: "lane-1",
provider: "codex",
model: "gpt-5.4",
});

await service.sendMessage({
sessionId: session.id,
text: "Think through the options.",
}, { awaitDispatch: true });

await waitForEvent(
events,
(event): event is AgentChatEventEnvelope =>
event.event.type === "status"
&& event.event.turnStatus === "started"
&& event.event.turnId === "turn-1",
);

mockState.emitCodexPayload({
jsonrpc: "2.0",
method: "item/reasoning/summaryTextDelta",
params: {
itemId: "reasoning-1",
delta: "Checking the relevant paths.",
},
});

await waitForEvent(
events,
(event): event is AgentChatEventEnvelope =>
event.event.type === "reasoning"
&& event.event.turnId === "turn-1"
&& event.event.itemId === "reasoning-1",
);

expect(events).toEqual(expect.arrayContaining([
expect.objectContaining({
event: expect.objectContaining({
type: "activity",
activity: "thinking",
turnId: "turn-1",
}),
}),
expect.objectContaining({
event: expect.objectContaining({
type: "reasoning",
text: "Checking the relevant paths.",
itemId: "reasoning-1",
turnId: "turn-1",
}),
}),
]));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

it("ignores unsolicited Codex turn notifications when no turn is active", async () => {
const events: Array<{ type: string; turnId?: string; text?: string }> = [];
const { service } = createService({
Expand Down Expand Up @@ -6581,6 +6644,109 @@ describe("createAgentChatService", () => {
await sendPromise;
});

it("does not duplicate Claude thinking when the final assistant message repeats streamed content", async () => {
const events: AgentChatEventEnvelope[] = [];
const setPermissionMode = vi.fn().mockResolvedValue(undefined);
const send = vi.fn().mockResolvedValue(undefined);
let streamCall = 0;

const stream = vi.fn(() => (async function* () {
streamCall += 1;
if (streamCall === 1) {
yield {
type: "system",
subtype: "init",
session_id: "sdk-session-thinking",
slash_commands: [],
};
return;
}

yield {
type: "stream_event",
event: {
type: "content_block_start",
index: 0,
content_block: { type: "thinking", thinking: "" },
},
};
yield {
type: "stream_event",
event: {
type: "content_block_delta",
index: 0,
delta: {
type: "thinking_delta",
thinking: "Checking both imports before editing.",
},
},
};
yield {
type: "assistant",
message: {
content: [{ type: "thinking", thinking: "Checking both imports before editing." }],
usage: { input_tokens: 1, output_tokens: 1 },
},
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
yield {
type: "result",
usage: { input_tokens: 1, output_tokens: 1 },
};
})());

vi.mocked(unstable_v2_createSession).mockReturnValue({
send,
stream,
close: vi.fn(),
sessionId: "sdk-session-thinking",
setPermissionMode,
} as any);

const { service } = createService({
onEvent: (event: AgentChatEventEnvelope) => events.push(event),
});

const session = await service.createSession({
laneId: "lane-1",
provider: "claude",
model: "claude-sonnet-4-6",
modelId: "anthropic/claude-sonnet-4-6",
});

await service.runSessionTurn({
sessionId: session.id,
text: "Resolve the PR comments.",
});

const reasoningEvents = events
.map((event) => event.event)
.filter((event): event is Extract<AgentChatEventEnvelope["event"], { type: "reasoning" }> => event.type === "reasoning");
expect(reasoningEvents.map((event) => event.text)).toEqual(["Checking both imports before editing."]);
expect(events.some((event) => event.event.type === "activity" && event.event.activity === "thinking")).toBe(true);
const sessionOpts = vi.mocked(unstable_v2_createSession).mock.calls[0]?.[0] as {
executableArgs?: string[];
settings?: Record<string, unknown>;
} | undefined;
expect(sessionOpts?.settings).toEqual(expect.objectContaining({
showThinkingSummaries: true,
alwaysThinkingEnabled: true,
}));
expect(sessionOpts?.executableArgs).toEqual(expect.arrayContaining([
"--include-partial-messages",
"--thinking",
"adaptive",
"--thinking-display",
"summarized",
]));
const settingsArgIndex = sessionOpts?.executableArgs?.indexOf("--settings") ?? -1;
expect(settingsArgIndex).toBeGreaterThanOrEqual(0);
const settingsJson = sessionOpts?.executableArgs?.[settingsArgIndex + 1];
expect(JSON.parse(String(settingsJson))).toEqual(expect.objectContaining({
showThinkingSummaries: true,
alwaysThinkingEnabled: true,
}));
});

it("emits completed Claude tool_result rows when tool_use_summary arrives", async () => {
const events: AgentChatEventEnvelope[] = [];
const setPermissionMode = vi.fn().mockResolvedValue(undefined);
Expand Down Expand Up @@ -7297,6 +7463,11 @@ describe("createAgentChatService", () => {
(event) => event.event.type === "status" && event.event.turnStatus === "started",
),
).toBe(true);
expect(
events.some(
(event) => event.event.type === "activity" && event.event.activity === "thinking",
),
).toBe(true);

for (let i = 0; i < 20 && !resolveNewSession; i += 1) {
await new Promise<void>((resolve) => setTimeout(resolve, 0));
Expand Down
102 changes: 81 additions & 21 deletions apps/desktop/src/main/services/chat/agentChatService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,11 @@ const CLAUDE_EFFORT_TO_TOKENS: Record<string, number> = {
high: 16384,
};

const CLAUDE_THINKING_SETTINGS = {
showThinkingSummaries: true,
alwaysThinkingEnabled: true,
};

const KNOWN_CLAUDE_EFFORTS = new Set(CLAUDE_REASONING_EFFORTS.map((e) => e.effort));

const CODEX_FALLBACK_MODELS: AgentChatModelInfo[] = listModelDescriptorsForProvider("codex").map((descriptor) => ({
Expand Down Expand Up @@ -1072,6 +1077,27 @@ function validateReasoningEffort(provider: "codex" | "claude", effort: string |
return known.has(aliased) ? aliased : fallback;
}

function buildClaudeV2ExecutableArgs(args: {
supportsReasoning: boolean;
effort?: string | null;
}): string[] {
const executableArgs = [
"--include-partial-messages",
"--settings",
JSON.stringify(CLAUDE_THINKING_SETTINGS),
];

if (args.supportsReasoning) {
executableArgs.push("--thinking", "adaptive", "--thinking-display", "summarized");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

buildClaudeV2ExecutableArgs() now always sends adaptive reasoning to the Claude binary, even when the same session is configured a few lines later with an explicit token budget via opts.thinking = { type: "enabled", budgetTokens: ... } for low/medium/high effort. That gives one SDK session two conflicting reasoning modes, so explicit-effort sessions can still behave like adaptive thinking and skip the visible reasoning stream on simpler prompts.

// apps/desktop/src/main/services/chat/agentChatService.ts
if (args.supportsReasoning) {
  executableArgs.push("--thinking", "adaptive", "--thinking-display", "summarized");
  const effort = args.effort;
  if (effort === "low" || effort === "medium" || effort === "high" || effort === "max") {
    executableArgs.push("--effort", effort);
  }
}

Align executableArgs with the same branch used for opts.thinking (enabled + --budget-tokens for budgeted tiers, adaptive only when no budget applies), or drop the conflicting CLI flag entirely so both layers honor the same reasoning contract.

const effort = args.effort;
if (effort === "low" || effort === "medium" || effort === "high" || effort === "max") {
executableArgs.push("--effort", effort);
}
}

return executableArgs;
}

function describeClaudeModel(value: string): string | null {
const lower = value.trim().toLowerCase();
if (lower.includes("opus")) return "Highest capability for complex strategy and review.";
Expand Down Expand Up @@ -6298,6 +6324,8 @@ export function createAgentChatService(args: {
let firstStreamEventLogged = false;
const emittedClaudeToolIds = new Set<string>();
const emittedSyntheticItemIds = new Set<string>();
const streamedClaudeTextContentIndexes = new Set<number>();
const streamedClaudeThinkingContentIndexes = new Set<number>();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
const openClaudeToolUses = new Map<string, { toolName: string }>();
const toolInputJsonByContentIndex = new Map<number, string>();
const toolUseMetaByContentIndex = new Map<number, { toolName: string; itemId: string }>();
Expand Down Expand Up @@ -6719,27 +6747,31 @@ export function createAgentChatService(args: {
if (betaMessage?.content && Array.isArray(betaMessage.content)) {
for (const [blockIndex, block] of betaMessage.content.entries()) {
if (block.type === "text") {
assistantText += block.text ?? "";
emitChatEvent(managed, {
type: "text",
text: block.text ?? "",
turnId,
});
if (!streamedClaudeTextContentIndexes.has(blockIndex)) {
assistantText += block.text ?? "";
emitChatEvent(managed, {
type: "text",
text: block.text ?? "",
turnId,
});
}
} else if (block.type === "thinking") {
const thinkingText = block.thinking ?? block.text ?? "";
const reasoningItemId = buildClaudeContentItemId("thinking", blockIndex);
emitChatEvent(managed, {
type: "activity",
activity: "thinking",
detail: REASONING_ACTIVITY_DETAIL,
turnId,
});
emitChatEvent(managed, {
type: "reasoning",
text: thinkingText,
...(reasoningItemId ? { itemId: reasoningItemId } : {}),
turnId,
});
if (thinkingText.trim().length > 0 && !streamedClaudeThinkingContentIndexes.has(blockIndex)) {
emitChatEvent(managed, {
type: "activity",
activity: "thinking",
detail: REASONING_ACTIVITY_DETAIL,
turnId,
});
emitChatEvent(managed, {
type: "reasoning",
text: thinkingText,
...(reasoningItemId ? { itemId: reasoningItemId } : {}),
turnId,
});
}
} else if (block.type === "tool_use") {
const toolName = String(block.name ?? "tool");
const itemId = buildClaudeContentItemId(
Expand Down Expand Up @@ -6798,12 +6830,18 @@ export function createAgentChatService(args: {
if (delta?.type === "text_delta") {
const text = delta.text ?? "";
if (text.length) {
if (typeof contentIndex === "number") {
streamedClaudeTextContentIndexes.add(contentIndex);
}
assistantText += text;
emitChatEvent(managed, { type: "text", text, turnId });
}
} else if (delta?.type === "thinking_delta") {
const text = delta.thinking ?? delta.text ?? "";
if (text.length) {
if (typeof contentIndex === "number") {
streamedClaudeThinkingContentIndexes.add(contentIndex);
}
const reasoningItemId = buildClaudeContentItemId("thinking", contentIndex);
emitChatEvent(managed, {
type: "activity",
Expand Down Expand Up @@ -6850,6 +6888,9 @@ export function createAgentChatService(args: {
// Some SDK versions include initial thinking text on block start
const startText = block.thinking ?? block.text ?? "";
if (startText.length) {
if (typeof contentIndex === "number") {
streamedClaudeThinkingContentIndexes.add(contentIndex);
}
emitChatEvent(managed, {
type: "reasoning",
text: startText,
Expand Down Expand Up @@ -8740,10 +8781,19 @@ export function createAgentChatService(args: {
if (method === "item/reasoning/summaryTextDelta" || method === "item/reasoning/textDelta") {
const delta = String((params.delta as string | undefined) ?? "");
if (!delta.length) return;
const turnId = typeof params.turnId === "string"
? params.turnId
: turnIdFromParams ?? runtime.activeTurnId ?? undefined;
emitChatEvent(managed, {
type: "activity",
activity: "thinking",
detail: REASONING_ACTIVITY_DETAIL,
turnId,
});
emitChatEvent(managed, {
type: "reasoning",
text: delta,
turnId: typeof params.turnId === "string" ? params.turnId : undefined,
turnId,
itemId: typeof params.itemId === "string" ? params.itemId : undefined,
summaryIndex: typeof params.summaryIndex === "number" ? params.summaryIndex : undefined
});
Expand Down Expand Up @@ -9368,6 +9418,7 @@ export function createAgentChatService(args: {
includePartialMessages: true,
agentProgressSummaries: true,
promptSuggestions: true,
settings: CLAUDE_THINKING_SETTINGS,
maxBudgetUsd: chatConfig.sessionBudgetUsd ?? undefined,
model: resolveClaudeCliModel(managed.session.model),
pathToClaudeCodeExecutable: claudeExecutable.path,
Expand Down Expand Up @@ -9433,20 +9484,24 @@ export function createAgentChatService(args: {
}
const claudeDescriptor = resolveSessionModelDescriptor(managed.session);
const claudeSupportsReasoning = claudeDescriptor?.capabilities.reasoning ?? true;
opts.executableArgs = buildClaudeV2ExecutableArgs({
supportsReasoning: claudeSupportsReasoning,
effort: managed.session.reasoningEffort,
});
if (claudeSupportsReasoning) {
const effort = managed.session.reasoningEffort;
if (effort === "low" || effort === "medium" || effort === "high" || effort === "max") {
opts.effort = effort as any;
}
const tokens = effort ? CLAUDE_EFFORT_TO_TOKENS[effort] : undefined;
if (tokens) {
opts.thinking = { type: "enabled", budgetTokens: tokens };
opts.thinking = { type: "enabled", budgetTokens: tokens, display: "summarized" } as any;
} else {
// Use adaptive thinking when no specific budget applies (e.g. "max",
// "xhigh", or no effort set). The SDK defaults to adaptive for models
// that support it, but being explicit ensures thinking is always active
// for reasoning-capable models.
opts.thinking = { type: "adaptive" };
opts.thinking = { type: "adaptive", display: "summarized" } as any;
}
}
const model = opts.model ?? resolveClaudeCliModel(managed.session.model) ?? DEFAULT_CLAUDE_MODEL;
Expand Down Expand Up @@ -11712,6 +11767,11 @@ export function createAgentChatService(args: {
});
emitChatEvent(prepared.managed, { type: "status", turnStatus: "started", turnId });
captureTurnBeforeSha(prepared.managed);
emitChatEvent(prepared.managed, {
type: "activity",
...initialTurnActivity(prepared.managed.session),
turnId,
});
setSessionActive(prepared.managed);
persistChatState(prepared.managed);
// NOTE: onDispatched is NOT called here. It will be called inside
Expand Down
Loading
Loading