Skip to content

fix: If top_p is not provided, ignore it#3141

Merged
Calcium-Ion merged 3 commits into
QuantumNous:mainfrom
seefs001:fix/claude-thinking-top_p
Mar 6, 2026
Merged

fix: If top_p is not provided, ignore it#3141
Calcium-Ion merged 3 commits into
QuantumNous:mainfrom
seefs001:fix/claude-thinking-top_p

Conversation

@seefs001
Copy link
Copy Markdown
Collaborator

@seefs001 seefs001 commented Mar 6, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Corrected initialization of a sampling parameter for Claude Opus models in adaptive and extended thinking modes so defaults are applied consistently.

@seefs001 seefs001 linked an issue Mar 6, 2026 that may be closed by this pull request
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 6, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3244cdc4-cec0-42c0-8154-aab8c19e606b

📥 Commits

Reviewing files that changed from the base of the PR and between 267c99b and be8a623.

📒 Files selected for processing (1)
  • relay/claude_handler.go

Walkthrough

Removed explicit assignments that set TopP = 0 in two Claude thinking-related code paths; all other parameter initializations and control flow remain unchanged.

Changes

Cohort / File(s) Summary
Claude handler
relay/claude_handler.go
Removed explicit TopP = 0 assignments in two thinking-related branches (adaptive thinking and ThinkingAdapterEnabled with -thinking suffix). Other initializations (e.g., OutputConfig, Temperature, Thinking, model adjustments) unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

Possibly related PRs

Poem

🐰 I hop through code at break of dawn,
A quiet tweak — a zero's gone.
Defaults now hum where numbers slept,
Claude thinks on, no zero kept. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: conditionally setting TopP to 1 when not provided for Claude models, which matches the code modification in claude_handler.go.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Calcium-Ion Calcium-Ion merged commit e768ae4 into QuantumNous:main Mar 6, 2026
@seefs001 seefs001 changed the title fix: If top_p is not provided, Claude's logic will set to 1 fix: If top_p is not provided, ignore it Mar 6, 2026
ennnnny pushed a commit to ennnnny/new-api that referenced this pull request Mar 17, 2026
…-top_p

fix: If top_p is not provided, Claude's logic will set to 1
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-thinking 模型报错:v0.11.2

2 participants