Skip to content

feat(ratio-setting): add compatibility for gemma-4 model suffix handling - #4063

Open
sxjeru wants to merge 3 commits into
QuantumNous:mainfrom
sxjeru:gemma
Open

feat(ratio-setting): add compatibility for gemma-4 model suffix handling#4063
sxjeru wants to merge 3 commits into
QuantumNous:mainfrom
sxjeru:gemma

Conversation

@sxjeru

@sxjeru sxjeru commented Apr 3, 2026

Copy link
Copy Markdown

根据 AIS 和相关文档,Gemma 4 已支持通过 thinkingLevel 控制思考开关。

即使 newapi 支持 gemma-4-26b-a4b-it-minimal

image

Summary by CodeRabbit

  • Bug Fixes
    • Improved normalization of gemma-4 model names to ensure consistent formatting across different model variants and configurations.

Copilot AI review requested due to automatic review settings April 3, 2026 04:25
@coderabbitai

coderabbitai Bot commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

A new NormalizeGemma4EffortModelName function is added to normalize gemma-4* model names by trimming effort suffixes when the Gemini thinking adapter setting is enabled. The existing FormatMatchingModelName function is extended to invoke this normalization as an additional processing step.

Changes

Cohort / File(s) Summary
Model Ratio Normalization
setting/ratio_setting/model_ratio.go
Added NormalizeGemma4EffortModelName exported function to trim effort suffixes from gemma-4* models using reasoning.TrimEffortSuffix. Extended FormatMatchingModelName to invoke this normalization after gemini-2.5-* thinking-budget processing and before GPT/gizmo prefix mappings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • seefs001

Poem

🐰 With whiskers twitching, effort suffixes fall,
Gemma-4 models trimmed down, normalized for all,
No -minimal, no -high to clutter the way,
Just clean model names to brighten the day! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: adding normalization support for gemma-4 model names with effort suffixes (-minimal/-high) when the thinking adapter setting is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the model-name normalization logic used by ratio/price lookups to handle Gemma 4 “reasoning effort” suffixes (e.g., -minimal, -high) by trimming them to the base model name, improving compatibility with upstream model naming variants.

Changes:

  • Add setting/reasoning dependency to reuse the shared effort-suffix parsing logic.
  • Update FormatMatchingModelName to trim Gemma 4 effort suffixes (via reasoning.TrimEffortSuffix) so ratio matching uses the base model name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Calcium-Ion

Copy link
Copy Markdown
Member

这个修改的意义是?

@sxjeru

sxjeru commented Apr 6, 2026

Copy link
Copy Markdown
Author

贵项目当前已支持 gemini-3-flash-preview-minimal 通过添加后缀控制 思考等级 参数。

本 pr 应当能让 gemma 4 也支持控制思考参数。

@Calcium-Ion

Calcium-Ion commented Apr 6, 2026

Copy link
Copy Markdown
Member

你这个pr的修改并不能支持后缀控制。

@sxjeru

sxjeru commented Apr 8, 2026

Copy link
Copy Markdown
Author

应该可以用了。

image

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
setting/ratio_setting/model_ratio.go (1)

726-739: ⚡ Quick win

Add table-driven tests for adapter on/off and suffix trimming cases.

This path is config-gated and easy to regress; a small test matrix would lock behavior down.

✅ Proposed test diff
+// setting/ratio_setting/model_ratio_test.go
+package ratio_setting
+
+import (
+	"testing"
+
+	"github.com/QuantumNous/new-api/setting/model_setting"
+)
+
+func TestNormalizeGemma4EffortModelName(t *testing.T) {
+	gs := model_setting.GetGeminiSettings()
+	orig := gs.ThinkingAdapterEnabled
+	defer func() { gs.ThinkingAdapterEnabled = orig }()
+
+	tests := []struct {
+		name      string
+		enabled   bool
+		input     string
+		expected  string
+	}{
+		{"adapter off keeps suffix", false, "gemma-4-26b-it-minimal", "gemma-4-26b-it-minimal"},
+		{"adapter on trims suffix", true, "gemma-4-26b-it-minimal", "gemma-4-26b-it"},
+		{"adapter on non-effort unchanged", true, "gemma-4-26b-it", "gemma-4-26b-it"},
+		{"non-gemma unchanged", true, "gemini-2.5-flash", "gemini-2.5-flash"},
+	}
+
+	for _, tt := range tests {
+		gs.ThinkingAdapterEnabled = tt.enabled
+		if got := NormalizeGemma4EffortModelName(tt.input); got != tt.expected {
+			t.Fatalf("%s: got %q, want %q", tt.name, got, tt.expected)
+		}
+	}
+}

Also applies to: 752-752

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@setting/ratio_setting/model_ratio.go` around lines 726 - 739, Add
table-driven unit tests for NormalizeGemma4EffortModelName covering combinations
of ThinkingAdapterEnabled true/false and model name variants (no gemma prefix,
gemma-4 with no suffix, gemma-4 with effort suffixes like -minimal/-high) to
ensure suffix trimming only occurs when adapter is enabled; use subtests or a
loop with cases that assert expected output from NormalizeGemma4EffortModelName
given each input and toggle
model_setting.GetGeminiSettings().ThinkingAdapterEnabled accordingly. Also add
the same matrix tests for the related function referenced near line 752 (the
other Normalize* function that uses reasoning.TrimEffortSuffix) to prevent
regressions. Ensure tests restore any global settings after each case to avoid
cross-test pollution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@setting/ratio_setting/model_ratio.go`:
- Around line 726-739: Add table-driven unit tests for
NormalizeGemma4EffortModelName covering combinations of ThinkingAdapterEnabled
true/false and model name variants (no gemma prefix, gemma-4 with no suffix,
gemma-4 with effort suffixes like -minimal/-high) to ensure suffix trimming only
occurs when adapter is enabled; use subtests or a loop with cases that assert
expected output from NormalizeGemma4EffortModelName given each input and toggle
model_setting.GetGeminiSettings().ThinkingAdapterEnabled accordingly. Also add
the same matrix tests for the related function referenced near line 752 (the
other Normalize* function that uses reasoning.TrimEffortSuffix) to prevent
regressions. Ensure tests restore any global settings after each case to avoid
cross-test pollution.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 13eb6fae-a17b-4fd6-b6df-a3f1a381f3fa

📥 Commits

Reviewing files that changed from the base of the PR and between f56397b and 168eaf5.

📒 Files selected for processing (1)
  • setting/ratio_setting/model_ratio.go

@Calcium-Ion
Calcium-Ion force-pushed the main branch 2 times, most recently from 51fdfc5 to 2b6f1df Compare August 30, 2026 15:03
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.

3 participants