fix(thinking): update ValidateConfig to include fromSuffix parameter and adjust budget validation logic

This commit is contained in:
hkfires
2026-01-18 16:37:14 +08:00
parent 99c7abbbf1
commit cb6caf3f87
4 changed files with 79 additions and 64 deletions

View File

@@ -30,7 +30,7 @@ var (
type LogFormatter struct{} type LogFormatter struct{}
// logFieldOrder defines the display order for common log fields. // logFieldOrder defines the display order for common log fields.
var logFieldOrder = []string{"provider", "model", "mode", "budget", "level", "original_value", "original_level", "min", "max", "clamped_to", "error"} var logFieldOrder = []string{"provider", "model", "mode", "budget", "level", "original_mode", "original_value", "min", "max", "clamped_to", "error"}
// Format renders a single log entry with custom formatting. // Format renders a single log entry with custom formatting.
func (m *LogFormatter) Format(entry *log.Entry) ([]byte, error) { func (m *LogFormatter) Format(entry *log.Entry) ([]byte, error) {

View File

@@ -159,7 +159,7 @@ func ApplyThinking(body []byte, model string, fromFormat string, toFormat string
} }
// 5. Validate and normalize configuration // 5. Validate and normalize configuration
validated, err := ValidateConfig(config, modelInfo, fromFormat, providerFormat) validated, err := ValidateConfig(config, modelInfo, fromFormat, providerFormat, suffixResult.HasSuffix)
if err != nil { if err != nil {
log.WithFields(log.Fields{ log.WithFields(log.Fields{
"provider": providerFormat, "provider": providerFormat,

View File

@@ -18,12 +18,14 @@ import (
// - Clamps budget values to model's allowed range // - Clamps budget values to model's allowed range
// - When converting Budget -> Level for level-only models, clamps the derived standard level to the nearest supported level // - When converting Budget -> Level for level-only models, clamps the derived standard level to the nearest supported level
// (special values none/auto are preserved) // (special values none/auto are preserved)
// - When config comes from a model suffix, strict budget validation is disabled (we clamp instead of error)
// //
// Parameters: // Parameters:
// - config: The thinking configuration to validate // - config: The thinking configuration to validate
// - support: Model's ThinkingSupport properties (nil means no thinking support) // - support: Model's ThinkingSupport properties (nil means no thinking support)
// - fromFormat: Source provider format (used to determine strict validation rules) // - fromFormat: Source provider format (used to determine strict validation rules)
// - toFormat: Target provider format // - toFormat: Target provider format
// - fromSuffix: Whether config was sourced from model suffix
// //
// Returns: // Returns:
// - Normalized ThinkingConfig with clamped values // - Normalized ThinkingConfig with clamped values
@@ -33,7 +35,7 @@ import (
// - Budget-only model + Level config → Level converted to Budget // - Budget-only model + Level config → Level converted to Budget
// - Level-only model + Budget config → Budget converted to Level // - Level-only model + Budget config → Budget converted to Level
// - Hybrid model → preserve original format // - Hybrid model → preserve original format
func ValidateConfig(config ThinkingConfig, modelInfo *registry.ModelInfo, fromFormat, toFormat string) (*ThinkingConfig, error) { func ValidateConfig(config ThinkingConfig, modelInfo *registry.ModelInfo, fromFormat, toFormat string, fromSuffix bool) (*ThinkingConfig, error) {
fromFormat, toFormat = strings.ToLower(strings.TrimSpace(fromFormat)), strings.ToLower(strings.TrimSpace(toFormat)) fromFormat, toFormat = strings.ToLower(strings.TrimSpace(fromFormat)), strings.ToLower(strings.TrimSpace(toFormat))
model := "unknown" model := "unknown"
support := (*registry.ThinkingSupport)(nil) support := (*registry.ThinkingSupport)(nil)
@@ -52,7 +54,7 @@ func ValidateConfig(config ThinkingConfig, modelInfo *registry.ModelInfo, fromFo
} }
allowClampUnsupported := isBudgetBasedProvider(fromFormat) && isLevelBasedProvider(toFormat) allowClampUnsupported := isBudgetBasedProvider(fromFormat) && isLevelBasedProvider(toFormat)
strictBudget := fromFormat != "" && isSameProviderFamily(fromFormat, toFormat) strictBudget := !fromSuffix && fromFormat != "" && isSameProviderFamily(fromFormat, toFormat)
budgetDerivedFromLevel := false budgetDerivedFromLevel := false
capability := detectModelCapability(modelInfo) capability := detectModelCapability(modelInfo)
@@ -238,7 +240,7 @@ func clampLevel(level ThinkingLevel, modelInfo *registry.ModelInfo, provider str
log.WithFields(log.Fields{ log.WithFields(log.Fields{
"provider": provider, "provider": provider,
"model": model, "model": model,
"original_level": string(level), "original_value": string(level),
"clamped_to": string(clamped), "clamped_to": string(clamped),
}).Debug("thinking: level clamped |") }).Debug("thinking: level clamped |")
return clamped return clamped

View File

@@ -1001,15 +1001,17 @@ func TestThinkingE2EMatrix_Suffix(t *testing.T) {
includeThoughts: "true", includeThoughts: "true",
expectErr: false, expectErr: false,
}, },
// Case 85: Gemini to Gemini, budget 64000 → exceeds Max error // Case 85: Gemini to Gemini, budget 64000 → clamped to Max
{ {
name: "85", name: "85",
from: "gemini", from: "gemini",
to: "gemini", to: "gemini",
model: "gemini-budget-model(64000)", model: "gemini-budget-model(64000)",
inputJSON: `{"model":"gemini-budget-model(64000)","contents":[{"role":"user","parts":[{"text":"hi"}]}]}`, inputJSON: `{"model":"gemini-budget-model(64000)","contents":[{"role":"user","parts":[{"text":"hi"}]}]}`,
expectField: "", expectField: "generationConfig.thinkingConfig.thinkingBudget",
expectErr: true, expectValue: "20000",
includeThoughts: "true",
expectErr: false,
}, },
// Case 86: Claude to Claude, budget 8192 → passthrough thinking.budget_tokens // Case 86: Claude to Claude, budget 8192 → passthrough thinking.budget_tokens
{ {
@@ -1022,20 +1024,21 @@ func TestThinkingE2EMatrix_Suffix(t *testing.T) {
expectValue: "8192", expectValue: "8192",
expectErr: false, expectErr: false,
}, },
// Case 87: Claude to Claude, budget 200000 → exceeds Max error // Case 87: Claude to Claude, budget 200000 → clamped to Max
{ {
name: "87", name: "87",
from: "claude", from: "claude",
to: "claude", to: "claude",
model: "claude-budget-model(200000)", model: "claude-budget-model(200000)",
inputJSON: `{"model":"claude-budget-model(200000)","messages":[{"role":"user","content":"hi"}]}`, inputJSON: `{"model":"claude-budget-model(200000)","messages":[{"role":"user","content":"hi"}]}`,
expectField: "", expectField: "thinking.budget_tokens",
expectErr: true, expectValue: "128000",
expectErr: false,
}, },
// Case 88: Antigravity to Antigravity, budget 8192 → passthrough thinkingBudget // Case 88: Gemini-CLI to Antigravity, budget 8192 → passthrough thinkingBudget
{ {
name: "88", name: "88",
from: "antigravity", from: "gemini-cli",
to: "antigravity", to: "antigravity",
model: "antigravity-budget-model(8192)", model: "antigravity-budget-model(8192)",
inputJSON: `{"model":"antigravity-budget-model(8192)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`, inputJSON: `{"model":"antigravity-budget-model(8192)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`,
@@ -1044,15 +1047,17 @@ func TestThinkingE2EMatrix_Suffix(t *testing.T) {
includeThoughts: "true", includeThoughts: "true",
expectErr: false, expectErr: false,
}, },
// Case 89: Antigravity to Antigravity, budget 64000 → exceeds Max error // Case 89: Gemini-CLI to Antigravity, budget 64000 → clamped to Max
{ {
name: "89", name: "89",
from: "antigravity", from: "gemini-cli",
to: "antigravity", to: "antigravity",
model: "antigravity-budget-model(64000)", model: "antigravity-budget-model(64000)",
inputJSON: `{"model":"antigravity-budget-model(64000)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`, inputJSON: `{"model":"antigravity-budget-model(64000)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`,
expectField: "", expectField: "request.generationConfig.thinkingConfig.thinkingBudget",
expectErr: true, expectValue: "20000",
includeThoughts: "true",
expectErr: false,
}, },
// iflow tests: glm-test and minimax-test (Cases 90-105) // iflow tests: glm-test and minimax-test (Cases 90-105)
@@ -1236,45 +1241,53 @@ func TestThinkingE2EMatrix_Suffix(t *testing.T) {
// Gemini Family Cross-Channel Consistency (Cases 106-114) // Gemini Family Cross-Channel Consistency (Cases 106-114)
// Tests that gemini/gemini-cli/antigravity as same API family should have consistent validation behavior // Tests that gemini/gemini-cli/antigravity as same API family should have consistent validation behavior
// Case 106: Gemini to Antigravity, budget 64000 → exceeds Max error (same family strict validation) // Case 106: Gemini to Antigravity, budget 64000 (suffix) → clamped to Max
{ {
name: "106", name: "106",
from: "gemini", from: "gemini",
to: "antigravity", to: "antigravity",
model: "gemini-budget-model(64000)", model: "gemini-budget-model(64000)",
inputJSON: `{"model":"gemini-budget-model(64000)","contents":[{"role":"user","parts":[{"text":"hi"}]}]}`, inputJSON: `{"model":"gemini-budget-model(64000)","contents":[{"role":"user","parts":[{"text":"hi"}]}]}`,
expectField: "", expectField: "request.generationConfig.thinkingConfig.thinkingBudget",
expectErr: true, expectValue: "20000",
includeThoughts: "true",
expectErr: false,
}, },
// Case 107: Gemini to Gemini-CLI, budget 64000 → exceeds Max error (same family strict validation) // Case 107: Gemini to Gemini-CLI, budget 64000 (suffix) → clamped to Max
{ {
name: "107", name: "107",
from: "gemini", from: "gemini",
to: "gemini-cli", to: "gemini-cli",
model: "gemini-budget-model(64000)", model: "gemini-budget-model(64000)",
inputJSON: `{"model":"gemini-budget-model(64000)","contents":[{"role":"user","parts":[{"text":"hi"}]}]}`, inputJSON: `{"model":"gemini-budget-model(64000)","contents":[{"role":"user","parts":[{"text":"hi"}]}]}`,
expectField: "", expectField: "request.generationConfig.thinkingConfig.thinkingBudget",
expectErr: true, expectValue: "20000",
includeThoughts: "true",
expectErr: false,
}, },
// Case 108: Gemini-CLI to Antigravity, budget 64000 → exceeds Max error (same family strict validation) // Case 108: Gemini-CLI to Antigravity, budget 64000 (suffix) → clamped to Max
{ {
name: "108", name: "108",
from: "gemini-cli", from: "gemini-cli",
to: "antigravity", to: "antigravity",
model: "gemini-budget-model(64000)", model: "gemini-budget-model(64000)",
inputJSON: `{"model":"gemini-budget-model(64000)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`, inputJSON: `{"model":"gemini-budget-model(64000)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`,
expectField: "", expectField: "request.generationConfig.thinkingConfig.thinkingBudget",
expectErr: true, expectValue: "20000",
includeThoughts: "true",
expectErr: false,
}, },
// Case 109: Gemini-CLI to Gemini, budget 64000 → exceeds Max error (same family strict validation) // Case 109: Gemini-CLI to Gemini, budget 64000 (suffix) → clamped to Max
{ {
name: "109", name: "109",
from: "gemini-cli", from: "gemini-cli",
to: "gemini", to: "gemini",
model: "gemini-budget-model(64000)", model: "gemini-budget-model(64000)",
inputJSON: `{"model":"gemini-budget-model(64000)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`, inputJSON: `{"model":"gemini-budget-model(64000)","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}}`,
expectField: "", expectField: "generationConfig.thinkingConfig.thinkingBudget",
expectErr: true, expectValue: "20000",
includeThoughts: "true",
expectErr: false,
}, },
// Case 110: Gemini to Antigravity, budget 8192 → passthrough (normal value) // Case 110: Gemini to Antigravity, budget 8192 → passthrough (normal value)
{ {
@@ -2301,10 +2314,10 @@ func TestThinkingE2EMatrix_Body(t *testing.T) {
expectField: "", expectField: "",
expectErr: true, expectErr: true,
}, },
// Case 88: Antigravity to Antigravity, thinkingBudget=8192 → passthrough // Case 88: Gemini-CLI to Antigravity, thinkingBudget=8192 → passthrough
{ {
name: "88", name: "88",
from: "antigravity", from: "gemini-cli",
to: "antigravity", to: "antigravity",
model: "antigravity-budget-model", model: "antigravity-budget-model",
inputJSON: `{"model":"antigravity-budget-model","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}],"generationConfig":{"thinkingConfig":{"thinkingBudget":8192}}}}`, inputJSON: `{"model":"antigravity-budget-model","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}],"generationConfig":{"thinkingConfig":{"thinkingBudget":8192}}}}`,
@@ -2313,10 +2326,10 @@ func TestThinkingE2EMatrix_Body(t *testing.T) {
includeThoughts: "true", includeThoughts: "true",
expectErr: false, expectErr: false,
}, },
// Case 89: Antigravity to Antigravity, thinkingBudget=64000 → exceeds Max error // Case 89: Gemini-CLI to Antigravity, thinkingBudget=64000 → exceeds Max error
{ {
name: "89", name: "89",
from: "antigravity", from: "gemini-cli",
to: "antigravity", to: "antigravity",
model: "antigravity-budget-model", model: "antigravity-budget-model",
inputJSON: `{"model":"antigravity-budget-model","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}],"generationConfig":{"thinkingConfig":{"thinkingBudget":64000}}}}`, inputJSON: `{"model":"antigravity-budget-model","request":{"contents":[{"role":"user","parts":[{"text":"hi"}]}],"generationConfig":{"thinkingConfig":{"thinkingBudget":64000}}}}`,
@@ -2744,9 +2757,9 @@ func runThinkingTests(t *testing.T, cases []thinkingTestCase) {
t.Fatalf("field %s: expected %q, got %q, body=%s", tc.expectField, tc.expectValue, actualValue, string(body)) t.Fatalf("field %s: expected %q, got %q, body=%s", tc.expectField, tc.expectValue, actualValue, string(body))
} }
if tc.includeThoughts != "" && (tc.to == "gemini" || tc.to == "antigravity") { if tc.includeThoughts != "" && (tc.to == "gemini" || tc.to == "gemini-cli" || tc.to == "antigravity") {
path := "generationConfig.thinkingConfig.includeThoughts" path := "generationConfig.thinkingConfig.includeThoughts"
if tc.to == "antigravity" { if tc.to == "gemini-cli" || tc.to == "antigravity" {
path = "request.generationConfig.thinkingConfig.includeThoughts" path = "request.generationConfig.thinkingConfig.includeThoughts"
} }
itVal := gjson.GetBytes(body, path) itVal := gjson.GetBytes(body, path)