feat: claude effort controls, ultrathink UI, and adapter robustness#1146
feat: claude effort controls, ultrathink UI, and adapter robustness#1146JustYannicc wants to merge 7 commits intopingdotgg:codething/648ca884-claudefrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
github-actions
bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Medium
In recoverSessionForThread, upsertSessionBinding is called at line 247 without passing persistedModelOptions and persistedProviderOptions, even though these values were extracted and used to start the session. After recovery, the runtime payload drops these options, so subsequent operations that read the persisted binding won't see the original model/provider configuration.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ProviderService.ts around line 247:
In `recoverSessionForThread`, `upsertSessionBinding` is called at line 247 without passing `persistedModelOptions` and `persistedProviderOptions`, even though these values were extracted and used to start the session. After recovery, the runtime payload drops these options, so subsequent operations that read the persisted binding won't see the original model/provider configuration.
Evidence trail:
- apps/server/src/provider/Layers/ProviderService.ts lines 229-230 (REVIEWED_COMMIT): `persistedModelOptions` and `persistedProviderOptions` are extracted from `input.binding.runtimePayload`
- apps/server/src/provider/Layers/ProviderService.ts lines 235-236 (REVIEWED_COMMIT): These values are passed to `adapter.startSession()`
- apps/server/src/provider/Layers/ProviderService.ts line 247 (REVIEWED_COMMIT): `upsertSessionBinding(resumed, input.binding.threadId)` - called WITHOUT `extra` parameter
- apps/server/src/provider/Layers/ProviderService.ts lines 162-174 (REVIEWED_COMMIT): `upsertSessionBinding` signature shows optional `extra` parameter with modelOptions/providerOptions
- apps/server/src/provider/Layers/ProviderService.ts lines 89-101 (REVIEWED_COMMIT): `toRuntimePayloadFromSession` only includes modelOptions/providerOptions if provided in `extra` (lines 98-99)
- apps/server/src/provider/Layers/ProviderService.ts lines 308-311 (REVIEWED_COMMIT): Correct usage in `startSession` passes the extra parameter
JustYannicc
force-pushed
the
codex/pr-179-no-teams
branch
from
abb5d9c to
2439fca
Compare
JustYannicc
changed the title
There was a problem hiding this comment.
🟠 High Layers/ClaudeCodeAdapter.ts:816
In completeTurn, context.inFlightTools.clear() is called at line 818 before iterating over context.inFlightTools.entries() at line 843. Since the map is cleared first, the loop never executes and in-flight tools are silently dropped without emitting item.completed events. The clear() should be moved after the loop or removed, since the loop already deletes each tool after processing.
const completeTurn = (
context: ClaudeSessionContext,
status: ProviderRuntimeTurnStatus,
errorMessage?: string,
result?: SDKResultMessage,
): Effect.Effect<void> =>
Effect.gen(function* () {
- // Clear any stale in-flight tools from interrupted content blocks
- context.inFlightTools.clear();
const turnState = context.turnState;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ClaudeCodeAdapter.ts around lines 816-826:
In `completeTurn`, `context.inFlightTools.clear()` is called at line 818 before iterating over `context.inFlightTools.entries()` at line 843. Since the map is cleared first, the loop never executes and in-flight tools are silently dropped without emitting `item.completed` events. The `clear()` should be moved after the loop or removed, since the loop already deletes each tool after processing.
Evidence trail:
apps/server/src/provider/Layers/ClaudeCodeAdapter.ts lines 810-860 at REVIEWED_COMMIT: Line 818 shows `context.inFlightTools.clear();` and line 843 shows `for (const [index, tool] of context.inFlightTools.entries())`. No code between these lines repopulates the map, confirming the loop operates on an already-cleared map.
|
had to do some sweeping changes to align with anthropic branding guidelines. can you resolve the conflicts? |
|
yes working on it |
- Auto-start synthetic turns for assistant messages arriving without an active turnState (fixes invisible background agent/subagent responses that arrive between user-initiated turns) - Auto-close stale synthetic turns in sendTurn to prevent session deadlock when a background response left an orphaned turn open - Clear inFlightTools between turns in completeTurn to prevent stale JSON fragments from corrupting the next turn's tool inputs - Fix missing Fiber import in ProviderHealth Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The clear() was placed before the loop that emits item.completed events for remaining in-flight tools, causing the loop to never execute. Move it after the loop so tools get properly finalized before cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Updates all remaining references from "claudeCode" to "claudeAgent" to match the Anthropic branding guidelines applied upstream. Includes provider kind literals, object property keys, test payloads, and component comparisons across server, web, and shared packages. Also fixes synthetic turn state to match upstream's refactored ClaudeTurnState interface (assistantTextBlocks map instead of the old assistantItemId/messageCompleted/emittedTextDelta fields). Also fixes inFlightTools.clear() placement: moved after the loop that emits item.completed events for remaining in-flight tools, so tools are properly finalized before cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JustYannicc
force-pushed
the
codex/pr-179-no-teams
branch
from
2439fca to
f882fc9
Compare
|
@juliusmarminge done. you should be able to merge it no problem now. |
| <svg {...props} preserveAspectRatio="xMidYMid" viewBox="0 0 256 257"> | ||
| <path | ||
| fill="#D97757" | ||
| fill="currentColor" |
| status.provider === "codex" | ||
| ? "Codex" | ||
| : status.provider === "claudeAgent" | ||
| ? "Claude Code" |
There was a problem hiding this comment.
not allowed to use the term 'Claude Code'
| MenuTrigger, | ||
| } from "../ui/menu"; | ||
|
|
||
| export const CodexTraitsPicker = memo(function CodexTraitsPicker(props: { |
There was a problem hiding this comment.
hmm not sure it's worth compacting these as one just yet... we don't really know what the future will look like so keeping them separate for now is the best. iirc i separated them when i had all codex/claude/cursor working because they had differernt enough logic to justify separate components instead of single component with switch statements internalyl
| const TaskProgressPayload = Schema.Struct({ | ||
| taskId: RuntimeTaskId, | ||
| description: TrimmedNonEmptyStringSchema, | ||
| summary: Schema.optional(TrimmedNonEmptyStringSchema), |
There was a problem hiding this comment.
what does this cover that "description" doesn't? is it something claude specific? not opposed to keeping mostly curious
2 participants
What Changed
Claude Effort Controls & Ultrathink
ClaudeCodeEfforttype with levels: low, medium, high, max, ultrathinkgetEffectiveClaudeCodeEffort()maps effort levels to Claude SDK valuesapplyClaudePromptEffortPrefix()prepends "Ultrathink:" prefix to promptsCodexTraitsPickerinto a provider-aware traits picker with Claude effort labelsAdapter Robustness (Background Agents & Subagents)
sendTurnto prevent session deadlockinFlightToolsmap incompleteTurnto prevent stale JSON fragments from corrupting subsequent turnsSubagent Progress Surfacing
task.progressevents surface as activities with summary andlastToolNamecollab_agent_tool_callclassification for Agent/Task tools with "Subagent task" labelsProvider Health
Why
Background agent results, subagent responses, and tool call details were silently dropped because the adapter's turn state machine only handled user-initiated turns. When a background agent finished work and the lead generated a response, that response was invisible in the UI.
The effort controls bring feature parity with the Codex reasoning effort UI, adapted for Claude's effort model including the ultrathink mode.
UI Changes
Checklist
Note
Add Claude effort controls, ultrathink UI, and adapter session robustness
ClaudeCodeEffortoptions (low/medium/high/max/ultrathink) to contracts, shared model utilities, and the Claude adapter; effort is forwarded through session start and persisted per thread so changes trigger session restarts.Ultrathink:\nwhen ultrathink effort is selected, both on the client inChatViewand server-side inClaudeAdapter.CodexTraitsPickerintoProviderTraitsPickerwith provider-aware effort labels and hides the Fast Mode toggle for non-Codex providers.ClaudeAdapter.sendTurnnow auto-completes a stale active turn instead of returning a validation error, and recovered sessions restore persistedmodelOptions.Macroscope summarized f882fc9.