Skip to content

fix: strip Claude title reasoning effort#31

Open
thomasmaerz wants to merge 1 commit into
Alph4d0g:mainfrom
thomasmaerz:fix/claude-title-reasoning-effort
Open

fix: strip Claude title reasoning effort#31
thomasmaerz wants to merge 1 commit into
Alph4d0g:mainfrom
thomasmaerz:fix/claude-title-reasoning-effort

Conversation

@thomasmaerz

Copy link
Copy Markdown

Summary

  • Route all outgoing JSON body rewrites through a shared request payload sanitizer.
  • Strip reasoning_effort/reasoningEffort only for Claude requests that match OpenCode's hidden title-generation prompt, avoiding Anthropic OAuth's temperature/thinking validation error.
  • Preserve reasoning options for normal Claude chat requests and keep existing Gemini schema sanitization behavior.

Fixes #30.

Testing

  • npm test
  • npm run check:exports

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the request payload sanitization logic to support stripping reasoning_effort and reasoningEffort from Claude model title generation requests, while keeping the existing Gemini tool schema sanitization. It introduces several helper functions to identify Claude models and detect title prompts, along with corresponding unit tests. Feedback on the changes highlights two issues: first, the use of logical OR assignment (||) for tracking changes is fragile and prone to short-circuiting bugs if refactored; second, in isOpenCodeTitlePrompt, if payload.messages is present but does not contain the title prompt, the function immediately returns false, skipping the evaluation of payload.input.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/plugin.ts
Comment on lines +952 to +957
let changed = false;

changed = stripClaudeTitleReasoningEffort(clonedPayload) || changed;
changed = sanitizeGeminiToolSchemas(clonedPayload) || changed;

return changed ? JSON.stringify(clonedPayload) : undefined;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Updating the changed flag using logical OR (||) with function calls on the left-hand side is correct but highly fragile and non-idiomatic. If a future refactoring reorders the operands (e.g., changed = changed || ...), the function call will be short-circuited and skipped when changed is already true. It is much safer and more readable to evaluate the functions independently first.

Suggested change
let changed = false;
changed = stripClaudeTitleReasoningEffort(clonedPayload) || changed;
changed = sanitizeGeminiToolSchemas(clonedPayload) || changed;
return changed ? JSON.stringify(clonedPayload) : undefined;
const changedClaude = stripClaudeTitleReasoningEffort(clonedPayload);
const changedGemini = sanitizeGeminiToolSchemas(clonedPayload);
const changed = changedClaude || changedGemini;
return changed ? JSON.stringify(clonedPayload) : undefined;

Comment thread src/plugin.ts
Comment on lines +995 to +1015
function isOpenCodeTitlePrompt(payload: Record<string, unknown>): boolean {
if (contentContainsTitlePrompt(payload.instructions)) return true;

const messages = payload.messages;
if (Array.isArray(messages)) {
return messages.some((message) => {
if (!isRecord(message) || message.role !== 'system') return false;
return contentContainsTitlePrompt(message.content);
});
}

const input = payload.input;
if (Array.isArray(input)) {
return input.some((item) => {
if (!isRecord(item) || item.role !== 'system') return false;
return contentContainsTitlePrompt(item.content);
});
}

return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If payload.messages is an array, the function immediately returns the result of messages.some(...). If it does not match, the function returns false and completely skips checking payload.input (even if payload.input is present and contains the title prompt). It is more robust to only return true if a match is found, and otherwise continue checking the remaining fields.

function isOpenCodeTitlePrompt(payload: Record<string, unknown>): boolean {
  if (contentContainsTitlePrompt(payload.instructions)) return true;

  const messages = payload.messages;
  if (Array.isArray(messages)) {
    const hasPrompt = messages.some((message) => {
      if (!isRecord(message) || message.role !== 'system') return false;
      return contentContainsTitlePrompt(message.content);
    });
    if (hasPrompt) return true;
  }

  const input = payload.input;
  if (Array.isArray(input)) {
    const hasPrompt = input.some((item) => {
      if (!isRecord(item) || item.role !== 'system') return false;
      return contentContainsTitlePrompt(item.content);
    });
    if (hasPrompt) return true;
  }

  return false;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude title generation fails when reasoning_effort is forwarded

1 participant