Skip to content

Commit 5d3c1ea

Browse files
committed
fix: restore thinking 'off' mode, fix file/skill prefix ordering, branch-aware turn_duration
- Gate non-adaptive thinking behind _thinkingMode so 'off' actually disables it - Prevent setEffortLevel() from unconditionally forcing _thinkingMode=true - Guard effort:set IPC in use-chat-messages.ts with isAdaptiveThinkingModel - Prevent SessionManager.setEffortLevel() from contaminating modePreferences - Append file @tokens instead of prepending so slash commands stay at position 0 - Add last_assistant_active tracking to skip dead-branch turn_duration events - Document that setEffortLevel/setThinkingMode are deferred to next session Fixes: verified-cycle-1-codex review findings #1, #2, #3, #4
1 parent c8da9f3 commit 5d3c1ea

5 files changed

Lines changed: 74 additions & 27 deletions

File tree

‎agent-bridge/src/agent/core/agent.ts‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -670,16 +670,20 @@ When browser is open, you also have access to Chrome DevTools Protocol tools via
670670
{ thinking: 'adaptive (CLI-managed)', effort: this._effortLevel ?? 'default' },
671671
'Adaptive thinking delegated to CLI for streaming compatibility'
672672
);
673-
} else {
674-
// All other models: default to extended thinking at ultra (32768 tokens).
675-
// If user explicitly configured a budget, use that; otherwise default to ultra.
673+
} else if (this._thinkingMode) {
674+
// Non-adaptive models with thinking enabled: use explicit extended thinking.
675+
// If user explicitly configured a budget, use that; otherwise default to ultra (32768).
676676
const budget = this._thinkingBudget > 0 ? this._thinkingBudget : 32768;
677677
options.thinking = { type: 'enabled', budgetTokens: budget };
678678
const modeName = budget <= 4096 ? 'think' : budget <= 10240 ? 'hard' : 'ultra';
679679
logger.info(
680680
{ thinkingMode: modeName, thinkingBudget: budget },
681681
'Extended thinking ENABLED (fixed budget)'
682682
);
683+
} else {
684+
// Non-adaptive models with thinking OFF: omit options.thinking entirely
685+
// so the SDK uses its default (no thinking).
686+
logger.info('Extended thinking DISABLED for non-adaptive model');
683687
}
684688

685689
// Permission handling based on session mode
@@ -1692,23 +1696,42 @@ When browser is open, you also have access to Chrome DevTools Protocol tools via
16921696
/**
16931697
* Set effort level for adaptive thinking (Opus 4.6).
16941698
* Maps effort levels to thinking token budgets via setMaxThinkingTokens.
1699+
*
1700+
* IMPORTANT: This does NOT mutate the active SDK query. The Claude Code SDK does not
1701+
* expose runtime setters for effort/thinking on an in-flight query. Changes are stored
1702+
* internally and take effect when `_createOptions()` is called for the next query.
1703+
* The frontend sends `effort:set` before each message (chat-actions.ts), so the
1704+
* timing works out — but mid-turn changes have no effect until the next user message.
16951705
*/
16961706
setEffortLevel(effort: 'low' | 'medium' | 'high' | 'max'): void {
16971707
this._effortLevel = effort;
16981708

1699-
// Also set budget as fallback for non-adaptive models
1709+
// Also set budget as fallback for non-adaptive models.
1710+
// Only update _thinkingMode/_thinkingBudget if thinking is not explicitly disabled.
1711+
// Without this guard, effort:set unconditionally forces _thinkingMode=true,
1712+
// which overrides the user's "thinking off" setting for non-adaptive models.
17001713
const budgetMap: Record<string, number> = {
17011714
low: 1024,
17021715
medium: 4096,
17031716
high: 10240,
17041717
max: 32768,
17051718
};
1706-
this._thinkingMode = true;
1707-
this._thinkingBudget = budgetMap[effort] ?? 4096;
1719+
if (this._thinkingMode) {
1720+
this._thinkingBudget = budgetMap[effort] ?? 4096;
1721+
}
17081722

17091723
logger.info({ effort }, 'Effort level set — applies on next query');
17101724
}
17111725

1726+
/**
1727+
* Enable or disable extended thinking for non-adaptive models.
1728+
*
1729+
* IMPORTANT: This does NOT mutate the active SDK query. The Claude Code SDK does not
1730+
* expose runtime setters for thinking on an in-flight query. Changes are stored
1731+
* internally and take effect when `_createOptions()` is called for the next query.
1732+
* The frontend sends `thinking:set` before each message (chat-actions.ts), so the
1733+
* timing works out — but mid-turn changes have no effect until the next user message.
1734+
*/
17121735
setThinkingMode(enabled: boolean, maxTokens?: number): void {
17131736
this._thinkingMode = enabled;
17141737
if (maxTokens !== undefined) {

‎agent-bridge/src/agent/session/session-manager.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1934,15 +1934,21 @@ export class SessionManager extends Disposable {
19341934
// Store as thinking preference so session creation picks it up.
19351935
// Without this, the first effort:set is lost (fires before session exists)
19361936
// and the session creates with default budget instead of the user's choice.
1937+
// NOTE: Only update thinkingEnabled/maxThinkingTokens if thinking is currently
1938+
// enabled (or was previously enabled in prefs). Without this guard, effort:set
1939+
// unconditionally forces thinkingEnabled=true, overriding the user's "thinking off"
1940+
// setting for non-adaptive models like Haiku.
19371941
const budgetMap: Record<string, number> = {
19381942
low: 1024,
19391943
medium: 4096,
19401944
high: 10240,
19411945
max: 32768,
19421946
};
19431947
const prefs = this.modePreferences.get(sessionId) ?? {};
1944-
prefs.thinkingEnabled = true;
1945-
prefs.maxThinkingTokens = budgetMap[effort] ?? 4096;
1948+
if (prefs.thinkingEnabled !== false) {
1949+
prefs.thinkingEnabled = true;
1950+
prefs.maxThinkingTokens = budgetMap[effort] ?? 4096;
1951+
}
19461952
this.modePreferences.set(sessionId, prefs);
19471953
}
19481954

‎apps/agent/src/components/chat/input/use-chat-input.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,22 +199,27 @@ export function useChatInput(options: UseChatInputOptions): UseChatInputReturn {
199199
let text = inputText.trim();
200200
if (!text) return;
201201

202-
// Prepend skill invocation if a skill chip is attached
203-
const skillItems = attachedContext.filter((item) => item.type === 'skill');
204-
if (skillItems.length > 0) {
205-
const skillPrefix = skillItems.map((s) => `/${s.name}`).join(' ');
206-
text = `${skillPrefix} ${text}`;
207-
}
208-
209-
// Prepend @filename tokens for attached files/folders so they persist in JSONL content.
202+
// Append @filename tokens for attached files/folders so they persist in JSONL content.
210203
// The file paths are still sent as attachments for the SDK, but the @tokens ensure
211204
// the user sees them after switching chats (JSONL content survives, attachedFiles doesn't).
205+
// NOTE: File tokens are appended (not prepended) so they don't interfere with
206+
// skill prefix `/command` at the start of the message — agent-bridge checks
207+
// message.startsWith('/') for slash expansion (agent.ts:1306).
212208
const fileItems = attachedContext.filter(
213209
(item) => item.type === 'file' || item.type === 'folder'
214210
);
215211
if (fileItems.length > 0) {
216-
const filePrefix = fileItems.map((item) => `@${item.name}`).join(' ');
217-
text = `${filePrefix} ${text}`;
212+
const fileSuffix = fileItems.map((item) => `@${item.name}`).join(' ');
213+
text = `${text} ${fileSuffix}`;
214+
}
215+
216+
// Prepend skill invocation if a skill chip is attached.
217+
// This must be the LAST prefix operation so `/skillName` stays at position 0,
218+
// ensuring agent-bridge's `message.startsWith('/')` check succeeds.
219+
const skillItems = attachedContext.filter((item) => item.type === 'skill');
220+
if (skillItems.length > 0) {
221+
const skillPrefix = skillItems.map((s) => `/${s.name}`).join(' ');
222+
text = `${skillPrefix} ${text}`;
218223
}
219224

220225
// Clear input immediately

‎apps/agent/src/hooks/chat/use-chat-messages.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -288,12 +288,14 @@ export function useChatMessages(): UseChatMessagesReturn {
288288
session_id: lastCreatedSessionId,
289289
model: toolState.model,
290290
});
291-
postMessage({
292-
type: 'effort:set',
293-
uuid: crypto.randomUUID(),
294-
session_id: lastCreatedSessionId,
295-
effort: toolState.effortLevel,
296-
});
291+
if (isAdaptiveThinkingModel(toolState.model)) {
292+
postMessage({
293+
type: 'effort:set',
294+
uuid: crypto.randomUUID(),
295+
session_id: lastCreatedSessionId,
296+
effort: toolState.effortLevel,
297+
});
298+
}
297299

298300
// Update title
299301
useUIStore.getState().updateConversationTitle(lastCreatedSessionId, text);

‎crates/common/conversations/src/lib.rs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,7 +1132,9 @@ fn parse_jsonl_lines(reader: BufReader<fs::File>, path: &Path) -> ParsedJsonl {
11321132
timestamp,
11331133
..
11341134
}) => {
1135-
if !is_active(&uuid, active_uuids.as_ref()) {
1135+
let active = is_active(&uuid, active_uuids.as_ref());
1136+
ctx.last_assistant_active = active;
1137+
if !active {
11361138
continue;
11371139
}
11381140
ctx.process_assistant_line(uuid, &value, timestamp.as_deref());
@@ -1145,8 +1147,13 @@ fn parse_jsonl_lines(reader: BufReader<fs::File>, path: &Path) -> ParsedJsonl {
11451147
..
11461148
}) => {
11471149
if subtype.as_deref() == Some("turn_duration") {
1148-
if let Some(ms) = duration_ms {
1149-
ctx.apply_turn_duration(ms);
1150+
// Only apply turn_duration if the most recent assistant was on
1151+
// the active branch. Dead-branch durations must not overwrite
1152+
// the active assistant's value.
1153+
if ctx.last_assistant_active {
1154+
if let Some(ms) = duration_ms {
1155+
ctx.apply_turn_duration(ms);
1156+
}
11501157
}
11511158
}
11521159
},
@@ -1175,6 +1182,9 @@ struct ParseContext {
11751182
last_timestamp: Option<u64>,
11761183
tool_results: HashMap<String, ToolResultData>,
11771184
last_msg_uuid: Option<String>,
1185+
/// Tracks whether the most recently encountered assistant line was on the active branch.
1186+
/// Used to skip `turn_duration` system lines that follow dead-branch assistants.
1187+
last_assistant_active: bool,
11781188
}
11791189

11801190
impl ParseContext {
@@ -1188,6 +1198,7 @@ impl ParseContext {
11881198
last_timestamp: None,
11891199
tool_results: HashMap::new(),
11901200
last_msg_uuid: None,
1201+
last_assistant_active: false,
11911202
}
11921203
}
11931204

0 commit comments

Comments
 (0)