Skip to content

Commit 2a4426c

Browse files
Anai-Guomudler
andauthored
fix(reasoning): don't persist request-scoped reasoning_effort as an operator disable (#10622) (#10623)
* fix(reasoning): don't persist request-scoped reasoning_effort into model config When a model sets `reasoning_effort: none` (or any default) in its YAML without an explicit `reasoning.disable`, ApplyReasoningEffort resolves that default at request time and sets ReasoningConfig.DisableReasoning on the request-scoped config copy. The post-load thinking/marker probe then wrote that request-scoped value back into the loader's persistent config via UpdateModelConfig, making it look as though the operator had explicitly set reasoning.disable=true. From then on, per-request `reasoning_effort` overrides were silently ignored (an explicit operator disable wins over a request asking to think). DetectThinkingSupportFromBackend only fills reasoning slots that are still nil, so a slot already set here came from ApplyReasoningEffort, not the probe. Snapshot which slots were nil before the probe and only persist those, so the probe's genuine backend detection is still saved while request-time reasoning effort never leaks into the persistent config. Fixes #10622 Signed-off-by: Tai An <antai12232931@outlook.com> * test(reasoning): cover persist-guard added in this PR, extract for testability ModelInference's post-probe persistence of ReasoningConfig.DisableReasoning / DisableReasoningTagPrefill had no test: the guard logic lived inline in a closure only reachable through a live gRPC backend. Extract it into persistProbedReasoning (pure refactor, no behavior change) so it can be exercised directly against a ModelConfigLoader, then add specs covering: - a probe-filled slot (nil beforehand) gets persisted - a slot that already carried a request-scoped value (e.g. from reasoning_effort: none) is left alone, i.e. the #10622 regression stays fixed - an operator's explicit persisted disable is preserved when the guard is false - the media marker still persists unconditionally Verified red/green: reverting persistProbedReasoning to the old unconditional copy fails exactly the two guard specs. Assisted-by: Claude:claude-sonnet-5 go vet Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * test(reasoning): ignore os.Remove error in temp file cleanup (errcheck) Signed-off-by: Tai An <antai12232931@outlook.com> * chore: empty commit to re-trigger flaky Agent Jobs CI test Signed-off-by: Tai An <antai12232931@outlook.com> --------- Signed-off-by: Tai An <antai12232931@outlook.com> Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@users.noreply.github.com>
1 parent 2348bdc commit 2a4426c

2 files changed

Lines changed: 122 additions & 7 deletions

File tree

core/backend/llm.go

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,28 @@ func needsThinkingProbe(c *config.ModelConfig) bool {
4747
c.ReasoningConfig.DisableReasoningTagPrefill == nil)
4848
}
4949

50+
// persistProbedReasoning writes the post-probe reasoning slots (and media
51+
// marker) from probed back into the loader's persisted config for modelName,
52+
// skipping any reasoning slot the probe was not actually allowed to fill.
53+
// persistDisableReasoning/persistDisableTagPrefill must be snapshotted from
54+
// probed's reasoning slots *before* the probe ran: a slot that already
55+
// carried a value at that point was populated by request-time
56+
// ApplyReasoningEffort, not by backend detection, and persisting it would
57+
// masquerade as an operator's explicit reasoning.disable (see #10622).
58+
func persistProbedReasoning(cl *config.ModelConfigLoader, modelName string, probed *config.ModelConfig, persistDisableReasoning, persistDisableTagPrefill bool) {
59+
cl.UpdateModelConfig(modelName, func(cfg *config.ModelConfig) {
60+
if persistDisableReasoning {
61+
cfg.ReasoningConfig.DisableReasoning = probed.ReasoningConfig.DisableReasoning
62+
}
63+
if persistDisableTagPrefill {
64+
cfg.ReasoningConfig.DisableReasoningTagPrefill = probed.ReasoningConfig.DisableReasoningTagPrefill
65+
}
66+
if probed.MediaMarker != "" {
67+
cfg.MediaMarker = probed.MediaMarker
68+
}
69+
})
70+
}
71+
5072
// HasChatDeltaContent returns true if any chat delta carries content or reasoning text.
5173
// Used to decide whether to prefer C++ autoparser deltas over Go-side tag extraction.
5274
func (t TokenUsage) HasChatDeltaContent() bool {
@@ -127,15 +149,19 @@ func ModelInference(ctx context.Context, s string, messages schema.Messages, ima
127149
needsMarkerProbe := c.MediaMarker == ""
128150
if shouldProbeThinking || needsMarkerProbe {
129151
modelOpts := grpcModelOpts(*c, o.SystemState.Model.ModelsPath)
152+
// DetectThinkingSupportFromBackend only fills reasoning slots that are
153+
// still nil, so a slot that already carries a value here was populated by
154+
// request-time ApplyReasoningEffort (e.g. a `reasoning_effort: none`
155+
// default), not by backend detection. Persisting such a request-scoped
156+
// value would masquerade as an operator's explicit reasoning.disable and
157+
// permanently defeat future per-request reasoning_effort overrides
158+
// (see #10622). Only persist the slots the probe is actually allowed to
159+
// fill.
160+
persistDisableReasoning := c.ReasoningConfig.DisableReasoning == nil
161+
persistDisableTagPrefill := c.ReasoningConfig.DisableReasoningTagPrefill == nil
130162
config.DetectThinkingSupportFromBackend(ctx, c, inferenceModel, modelOpts)
131163
// Update the config in the loader so it persists for future requests
132-
cl.UpdateModelConfig(c.Name, func(cfg *config.ModelConfig) {
133-
cfg.ReasoningConfig.DisableReasoning = c.ReasoningConfig.DisableReasoning
134-
cfg.ReasoningConfig.DisableReasoningTagPrefill = c.ReasoningConfig.DisableReasoningTagPrefill
135-
if c.MediaMarker != "" {
136-
cfg.MediaMarker = c.MediaMarker
137-
}
138-
})
164+
persistProbedReasoning(cl, c.Name, c, persistDisableReasoning, persistDisableTagPrefill)
139165
}
140166

141167
var protoMessages []*proto.Message

core/backend/llm_probe_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package backend
22

33
import (
4+
"os"
5+
46
"github.com/mudler/LocalAI/core/config"
57

68
"github.com/gpustack/gguf-parser-go/util/ptr"
@@ -27,3 +29,90 @@ var _ = Describe("thinking probe gating", func() {
2729
Expect(needsThinkingProbe(cfg)).To(BeFalse())
2830
})
2931
})
32+
33+
var _ = Describe("persistProbedReasoning", func() {
34+
const modelName = "probe-test"
35+
36+
// newLoaderWithConfig seeds a ModelConfigLoader with a single model config
37+
// parsed from yamlBody, mirroring how the loader is populated from disk.
38+
newLoaderWithConfig := func(yamlBody string) *config.ModelConfigLoader {
39+
tmp, err := os.CreateTemp("", "persist-probed-reasoning-*.yaml")
40+
Expect(err).ToNot(HaveOccurred())
41+
defer func() { _ = os.Remove(tmp.Name()) }()
42+
43+
_, err = tmp.WriteString(yamlBody)
44+
Expect(err).ToNot(HaveOccurred())
45+
Expect(tmp.Close()).To(Succeed())
46+
47+
cl := config.NewModelConfigLoader("")
48+
Expect(cl.ReadModelConfig(tmp.Name())).To(Succeed())
49+
return cl
50+
}
51+
52+
It("persists a reasoning slot the probe was allowed to fill (was nil beforehand)", func() {
53+
cl := newLoaderWithConfig("name: probe-test\nbackend: llama-cpp\n")
54+
55+
probed := &config.ModelConfig{}
56+
probed.Name = modelName
57+
probed.ReasoningConfig.DisableReasoning = ptr.To(false) // backend detected: supports thinking
58+
probed.ReasoningConfig.DisableReasoningTagPrefill = ptr.To(true)
59+
60+
persistProbedReasoning(cl, modelName, probed, true, true)
61+
62+
cfg, ok := cl.GetModelConfig(modelName)
63+
Expect(ok).To(BeTrue())
64+
Expect(cfg.ReasoningConfig.DisableReasoning).ToNot(BeNil())
65+
Expect(*cfg.ReasoningConfig.DisableReasoning).To(BeFalse())
66+
Expect(cfg.ReasoningConfig.DisableReasoningTagPrefill).ToNot(BeNil())
67+
Expect(*cfg.ReasoningConfig.DisableReasoningTagPrefill).To(BeTrue())
68+
})
69+
70+
It("does not persist a slot that already carried a request-scoped value before the probe ran", func() {
71+
cl := newLoaderWithConfig("name: probe-test\nbackend: llama-cpp\n")
72+
73+
probed := &config.ModelConfig{}
74+
probed.Name = modelName
75+
// Simulates ApplyReasoningEffort("none") having set this on the
76+
// request-scoped copy before the probe ran - not a genuine backend
77+
// detection, so it must never reach the persisted config (#10622).
78+
probed.ReasoningConfig.DisableReasoning = ptr.To(true)
79+
80+
persistProbedReasoning(cl, modelName, probed, false, false)
81+
82+
cfg, ok := cl.GetModelConfig(modelName)
83+
Expect(ok).To(BeTrue())
84+
Expect(cfg.ReasoningConfig.DisableReasoning).To(BeNil())
85+
Expect(cfg.ReasoningConfig.DisableReasoningTagPrefill).To(BeNil())
86+
})
87+
88+
It("preserves an operator's explicit persisted disable when the guard is false", func() {
89+
cl := newLoaderWithConfig("name: probe-test\nbackend: llama-cpp\nreasoning:\n disable: true\n")
90+
91+
probed := &config.ModelConfig{}
92+
probed.Name = modelName
93+
// Even if the request-scoped copy ends up holding a different value,
94+
// persistDisableReasoning=false must keep the operator's own setting.
95+
probed.ReasoningConfig.DisableReasoning = ptr.To(false)
96+
97+
persistProbedReasoning(cl, modelName, probed, false, false)
98+
99+
cfg, ok := cl.GetModelConfig(modelName)
100+
Expect(ok).To(BeTrue())
101+
Expect(cfg.ReasoningConfig.DisableReasoning).ToNot(BeNil())
102+
Expect(*cfg.ReasoningConfig.DisableReasoning).To(BeTrue())
103+
})
104+
105+
It("persists the media marker regardless of the reasoning guards", func() {
106+
cl := newLoaderWithConfig("name: probe-test\nbackend: llama-cpp\n")
107+
108+
probed := &config.ModelConfig{}
109+
probed.Name = modelName
110+
probed.MediaMarker = "<__media__>"
111+
112+
persistProbedReasoning(cl, modelName, probed, false, false)
113+
114+
cfg, ok := cl.GetModelConfig(modelName)
115+
Expect(ok).To(BeTrue())
116+
Expect(cfg.MediaMarker).To(Equal("<__media__>"))
117+
})
118+
})

0 commit comments

Comments
 (0)