diff --git a/backend/internal/config/capability_test.go b/backend/internal/config/capability_test.go index 4be30571..4e68713c 100644 --- a/backend/internal/config/capability_test.go +++ b/backend/internal/config/capability_test.go @@ -130,3 +130,40 @@ models: }) } } + +// TestThinksOnWireMirrorsTheWireForEachControl pins the predicate the load-time echo-exposure warning is +// built on. It exists because the extra_body_disable arm used to read `reasoning != "off"`, which answered +// "it thinks" for an UNSET effort — while Capability.applyToBody merges the thinking-disable for `""` and +// `"off"` alike. The predicate therefore contradicted the wire in exactly the case that matters most: a +// GLM-shaped model configured with no `reasoning:` key at all, i.e. the DEFAULT, shipping dense CJK into +// the echo zone with nothing said out loud. +func TestThinksOnWireMirrorsTheWireForEachControl(t *testing.T) { + m := &Models{ + Providers: map[string]Provider{"p": {Kind: "openai"}}, + Models: map[string]Model{ + "disable": {Provider: "p", Capabilities: &CapabilitiesConfig{Reasoning: &ReasoningCapCfg{ + Control: "extra_body_disable", OffExtraBody: map[string]any{"thinking": map[string]any{"type": "disabled"}}}}}, + "effort": {Provider: "p", Capabilities: &CapabilitiesConfig{Reasoning: &ReasoningCapCfg{ + Control: "effort", OffEffort: "none"}}}, + "none": {Provider: "p"}, // ReasoningNone — off-by-omission, nothing reaches the wire + }, + } + for _, tc := range []struct { + model, effort string + thinks bool + why string + }{ + {"disable", "", false, "an unset effort MERGES the disable — this is the case the old predicate got wrong"}, + {"disable", "off", false, "an explicit off merges the disable"}, + {"disable", "low", true, "an explicit level opts into thinking (OnExtraBody)"}, + {"effort", "", true, "an unset effort emits nothing, so the provider's own default stands"}, + {"effort", "off", false, "off sends the explicit OffEffort switch"}, + {"effort", "high", true, "a level is sent as-is"}, + {"none", "", true, "nothing reasoning-related ever reaches the wire"}, + {"none", "off", true, "off is a deliberate NO-OP here — the DeepSeek echo-mine guarantee"}, + } { + if got := m.ThinksOnWire(tc.model, tc.effort); got != tc.thinks { + t.Errorf("ThinksOnWire(%q, %q) = %v, want %v — %s", tc.model, tc.effort, got, tc.thinks, tc.why) + } + } +} diff --git a/backend/internal/config/internal_call.go b/backend/internal/config/internal_call.go new file mode 100644 index 00000000..34f1daf8 --- /dev/null +++ b/backend/internal/config/internal_call.go @@ -0,0 +1,91 @@ +package config + +// internal_call.go: the ONE derivation of a config.Stage for a call the ENGINE makes on its own behalf — +// a bank role (terminologist / type-classifier), a repair span, tomorrow's annotator or Ф2 judge. +// +// Why it exists (D39.87 §2): those calls used to be hand-written `config.Stage{Name: …, Role: …, Model: …}` +// literals — three of them in shipping code plus one in the live rig — and every literal silently dropped +// every OTHER field the owner had configured. That is how the reasoning knob went missing from the bank +// roles: nobody "forgot a key", the shape of the code loses fields by construction. Adding the key to three +// literals would reproduce the defect at the fourth site, so the literals are gone instead: this is the only +// place in the tree that builds a Stage outside the loader, and TestSyntheticStageSeamIsSingle keeps it that +// way. + +// ValidReasoningEffort reports whether v is the engine's NEUTRAL effort vocabulary. ONE definition, shared +// by the stage key and by every gate that configures a call class, so the two can never drift into accepting +// different words for the same knob. +// +// ⚠ It is deliberately provider-BLIND, and that is a known limit rather than an oversight: the capability +// layer maps these words per provider, and a per-model enum does not exist in models.yaml. So `medium` on a +// DeepSeek-shaped model is undefined vendor behaviour, and the vendor's own `xhigh`/`max` are unreachable +// from our config. Validating against the RESOLVED model's capability would need a new per-model table — a +// new mechanism, not a use of an existing one — so it stays a named finding, not a silent gap. +func ValidReasoningEffort(v string) bool { + switch v { + case "", "off", "low", "medium", "high": + return true + } + return false +} + +// InternalCall is everything an engine-internal call says about itself. Every OTHER Stage field is decided +// ONCE in Stage() below, with the reason written down — so a new call site inherits the decisions instead of +// re-taking them by accident, and a new Stage FIELD is a compile-visible decision rather than a silent zero +// (TestInternalCallDecidesEveryStageField). +type InternalCall struct { + // Name is the stage name the call is accounted under. It is a request-hash axis and the log axis, so it + // must be the name the operator sees in `tmctl report` for this call class. + Name string + // Role is the engine role — the COST axis (RoleSpentUSD, the per-role sub-budgets) and the log axis. + Role string + // Model is the RESOLVED model this call goes to (gates carry their own `model:`, already validated). + Model string + // Reasoning is the effort the owner configured for this call CLASS, in the engine's neutral vocabulary + // ("" | off | low | medium | high). "" means "leave the provider's own default", which is not the same + // as off — on a capability whose control is extra_body_disable it is the provider's default of THINKING + // OFF (llm.Capability.applyToBody), which is why the value has to travel rather than be assumed. + Reasoning string +} + +// Stage renders the internal call as the config.Stage the runner's money path takes. It is deliberately +// TOTAL over Stage's fields: what is set is listed here, and what stays zero stays zero for a written reason. +func (c InternalCall) Stage() Stage { + return Stage{ + Name: c.Name, + Role: c.Role, + Model: c.Model, + Reasoning: c.Reasoning, + // ResolvedModel mirrors Model: an internal call is NOT label-routed (label routing resolves the + // book's `stages:` at load), so the configured model IS the resolved one. It is set rather than left + // zero because the load-time exposure walk reads ResolvedModel to name a stage×model pair. + ResolvedModel: c.Model, + + // --- deliberately zero --------------------------------------------------------------------- + // Temperature: 0, and NOT inherited from a parent prose stage. These calls return STRUCTURE that + // the engine re-parses — a term table, a type verdict, a span replacement — where sampling variance + // is pure risk; the owner's stage temperature is a STYLE choice about prose and means nothing here. + // (It is also a request-hash field, so this is the value every existing bank checkpoint was bought + // at; changing it would re-buy them, which is a reason to be sure, not the reason to choose.) + // + // ReasoningMaxTokens: 0 — it reserves the ADDITIVE reasoning buffer (D6.2/D13.6), and the gates that + // make internal calls carry no such key. That is not an oversight left open here: the loader REFUSES + // an additive-billing provider for gates.terminology.model / gates.repair.model precisely because + // the block cannot reserve the buffer, so a call reaching this seam is never on an additive provider. + // A future gate that wants an additive model must add the key AND lift that refusal together. + // + // PromptOverride / PromptPath / PromptVersion: internal calls arrive with their messages ALREADY + // rendered (each gate resolves and versions its own prompt), so the stage template machinery is not + // consulted for them. + // + // EscalateTo / ResolvedHop: no escalation hop. maybeEscalate is reached from runStage, not from the + // internal-call paths, and these calls degrade by leaving their work unchanged rather than by paying + // a second model. + // + // LabelModels: see ResolvedModel — no label routing. A model that may not receive the book's content + // labels is refused loudly by clientFor before any money moves, so this is fail-closed, not silent. + // + // FewShot: nil — few-shot is a prose-translation device; a term table has its own prompt. + // + // LegacyPrompt / LegacyPrompts / LegacyChannel: retired keys, declared only to be REJECTED at load. + } +} diff --git a/backend/internal/config/internal_call_test.go b/backend/internal/config/internal_call_test.go new file mode 100644 index 00000000..5aff5a29 --- /dev/null +++ b/backend/internal/config/internal_call_test.go @@ -0,0 +1,96 @@ +package config + +import ( + "reflect" + "testing" +) + +// TestInternalCallDecidesEveryStageField is the second half of the "a fourth synthetic stage cannot be +// forgotten" guarantee. The seam test in package pipeline forces every CALL SITE through InternalCall; this +// one forces every stage FIELD through a decision. +// +// The defect it prevents is the one that produced this pack: a field exists on Stage, an engine-internal +// call needs it, and nobody notices it is missing because a zero value is indistinguishable from a choice. +// Adding a field to Stage now fails HERE, at the one place that has to say what internal calls do with it — +// carry it, or deliberately leave it zero with the reason written next to it in internal_call.go. +func TestInternalCallDecidesEveryStageField(t *testing.T) { + carried := map[string]bool{ + "Name": true, + "Role": true, + "Model": true, + "Reasoning": true, + "ResolvedModel": true, + } + // Every entry here is argued in InternalCall.Stage()'s "deliberately zero" block; the value is the + // one-line reason, kept next to the name so this list cannot silently become a dumping ground. + deliberatelyZero := map[string]string{ + "Temperature": "structured replies the engine re-parses; sampling variance is pure risk", + "ReasoningMaxTokens": "gates carry no such key, and the loader refuses additive-billing models for them", + "PromptOverride": "internal calls arrive with messages already rendered", + "PromptPath": "same", + "PromptVersion": "same — each gate versions its own prompt", + "EscalateTo": "no hop; these calls degrade by leaving their work unchanged", + "ResolvedHop": "same", + "LabelModels": "no label routing; clientFor refuses an ineligible model loudly", + "FewShot": "few-shot is a prose-translation device", + "LegacyPrompt": "retired key, declared only to be rejected at load", + "LegacyPrompts": "same", + "LegacyChannel": "same", + } + + st := reflect.TypeOf(Stage{}) + for i := 0; i < st.NumField(); i++ { + name := st.Field(i).Name + if carried[name] == (deliberatelyZero[name] != "") { + t.Errorf("Stage field %q is not decided for engine-internal calls: name it in InternalCall.Stage() "+ + "(carrying it) or in deliberatelyZero here (leaving it, with the reason) — exactly one of the two", name) + } + } + // And the reverse direction: a field REMOVED from Stage must not leave a stale decision behind. + for _, m := range []map[string]bool{carried, boolKeys(deliberatelyZero)} { + for name := range m { + if _, ok := st.FieldByName(name); !ok { + t.Errorf("%q is decided here but no longer exists on Stage — drop the stale entry", name) + } + } + } +} + +func boolKeys(m map[string]string) map[string]bool { + out := make(map[string]bool, len(m)) + for k := range m { + out[k] = true + } + return out +} + +// TestInternalCallCarriesTheKnobAndPinsTheRest is the value-level companion: the fields the seam DOES set +// arrive intact, and the ones it pins stay pinned. Cheap, but it is what makes the completeness test above +// mean something — a seam could satisfy "every field is listed" while carrying the wrong value. +func TestInternalCallCarriesTheKnobAndPinsTheRest(t *testing.T) { + got := InternalCall{Name: "terminology", Role: "terminologist", Model: "m1", Reasoning: "low"}.Stage() + if got.Name != "terminology" || got.Role != "terminologist" || got.Model != "m1" || got.Reasoning != "low" { + t.Fatalf("the call's own four facts must arrive intact: %+v", got) + } + if got.ResolvedModel != "m1" { + t.Fatalf("an internal call is not label-routed, so the resolved model IS the model: %q", got.ResolvedModel) + } + if got.Temperature != 0 || got.ReasoningMaxTokens != 0 || got.FewShot != nil || got.EscalateTo != "" { + t.Fatalf("the pinned fields must stay zero: %+v", got) + } +} + +func TestValidReasoningEffortIsOneVocabulary(t *testing.T) { + for _, ok := range []string{"", "off", "low", "medium", "high"} { + if !ValidReasoningEffort(ok) { + t.Errorf("%q must be accepted — it is the engine's neutral effort vocabulary", ok) + } + } + // The vendor's own words are NOT ours: they are mapped by the capability layer, and accepting them here + // would let a config name a level the wire has no meaning for. + for _, bad := range []string{"none", "xhigh", "max", "minimal", "LOW"} { + if ValidReasoningEffort(bad) { + t.Errorf("%q must be refused — the engine's vocabulary is provider-neutral", bad) + } + } +} diff --git a/backend/internal/config/models.go b/backend/internal/config/models.go index b85ff8f5..93f0a416 100644 --- a/backend/internal/config/models.go +++ b/backend/internal/config/models.go @@ -374,8 +374,19 @@ func (m *Models) providerReasoning(modelName string) string { // AdditiveReasoningTokens. func (m *Models) ThinksOnWire(modelName, reasoning string) bool { switch m.ResolveCapability(modelName).Reasoning.Control { - case llm.ReasoningEffortField, llm.ReasoningExtraBodyDisable: - return reasoning != "off" // an explicit off-switch exists and is sent at "off" + case llm.ReasoningExtraBodyDisable: + // ⚠ EMPTY is not "the provider default" for this control — it is DISABLED. Capability.applyToBody + // merges OffExtraBody for `effort == "" || effort == "off"` alike, because a disable-capability's + // whole purpose is to keep thinking off unless someone opts in. This arm used to read + // `reasoning != "off"` and so answered "it thinks" for the unset case, disagreeing with the wire it + // exists to describe — which silently exempted the shape it matters most for: a GLM-shaped model + // configured with no `reasoning:` key at all (the DEFAULT), shipping dense CJK into the echo zone + // with no warning. Must mirror applyToBody exactly. + return reasoning != "" && reasoning != "off" + case llm.ReasoningEffortField: + // Here empty DOES mean the provider default: applyToBody emits nothing, and the off-switch is sent + // only at an explicit "off" (grok then thinks at its own default `low`). + return reasoning != "off" default: // none (off-by-omission) | mandatory — no switch reaches the wire return true } @@ -441,8 +452,17 @@ func (m *Models) ResolveCapability(modelName string) llm.Capability { // on the wire — top-level (GLM/DeepSeek {"thinking":…}, Qwen {"enable_thinking":…}, a // raw {"reasoning_effort":…}) OR nested (DeepSeek-V3.1+ disables via // chat_template_kwargs.thinking:false). On an echo-prone provider their mere PRESENCE, -// at any depth, is the mine, whatever the value: thinking must stay at the provider -// default, and these keys exist only to move it off that default. +// at any depth, is the mine, whatever the value. +// +// ⚠ What is banned is the RAW WIRE channel, not the subject: since D39.87 a stage or a +// gate MAY set the effort level (`reasoning: "low"`) on such a provider, and on a +// ReasoningNone capability that emits the very same reasoning_effort key legally. The +// difference is not the byte on the wire, it is who decided it: the config path goes +// through Capability.applyToBody, which knows what "off" means for this control and +// keeps a disable from ever being emitted here, while extra_body is merged verbatim and +// can therefore SUPPRESS thinking — which is the mine. So: the effort level is the +// owner's to choose; the on/off switch is not, and this map is what keeps the second +// one out of the first one's clothing. var thinkingControlExtraKeys = map[string]bool{ "thinking": true, "enable_thinking": true, "reasoning_effort": true, "reasoning": true, } diff --git a/backend/internal/config/pipeline.go b/backend/internal/config/pipeline.go index 4ff42331..3b3e320a 100644 --- a/backend/internal/config/pipeline.go +++ b/backend/internal/config/pipeline.go @@ -317,6 +317,19 @@ type TerminologyGate struct { // LangpackVersion and re-buy both waves of every book of the pair. The banknote fold reads the same // declaration, so a book running that channel without the terminologist may set it with the gate off. TargetScript string `yaml:"target_script"` + // Reasoning is how much the bank roles may THINK, in the engine's neutral vocabulary + // ("" | off | low | medium | high) — the same key a stage carries, because it is the same knob: the + // gate is simply the place a BOOK-LEVEL call class is configured, having no stage of its own. ONE knob + // for both roles: they share the batching, the money path and the checkpoint axis, and the difference + // between a long render and a short classification is a max_tokens difference, not an effort one — a + // second key would be split without a measurement asking for it. + // + // "" is NOT "no thinking": it means "leave the provider's default", which on a DeepSeek-shaped + // capability is thinking ON at the vendor's own effort (that default moved to `high` on 2026-07-31 and + // walled the role — D39.86), and on a GLM-shaped one (control extra_body_disable) is thinking OFF. That + // asymmetry is why the value has to be configurable here at all instead of assumed; Runner + // .sourceEchoExposure names the second case out loud at load. + Reasoning string `yaml:"reasoning"` // PromptPath is the RESOLVED role prompt (`//terminologist.md`), filled by // LoadPipeline by the ordinary role convention. Not a config key. PromptPath string `yaml:"-"` @@ -360,6 +373,16 @@ func (g TerminologyGate) ClassifierModel() string { type RepairGate struct { Enabled bool `yaml:"enabled"` Model string `yaml:"model"` + // Reasoning is how much a repair call may THINK, in the engine's neutral vocabulary + // ("" | off | low | medium | high) — its OWN key, deliberately NOT inherited from the stage whose text + // is being repaired. Inheriting was the first design and it was wrong twice over: the effort's MEANING + // is per-CONTROL (on an extra_body_disable capability "off" is a live thinking disable, on a + // ReasoningNone one it is a documented no-op), and `gates.repair.model` is independent of the stage's + // model — so a final stage carrying `reasoning: "off"` as a no-op for DeepSeek would have armed a real + // disable on a GLM repair model. It also moves the request hash, so inheritance would have re-bought + // every repair call already paid for by every book whose final stage sets the key (which is all of + // them). Unset ⇒ "" ⇒ byte-identical to the pre-key behaviour. + Reasoning string `yaml:"reasoning"` // MaxCallsPerUnit bounds the calls one output unit may spend (the blast-radius cap); the loop itself is // a SINGLE round — a repaired text is never re-repaired. MaxCallsPerUnit int `yaml:"max_calls_per_unit"` @@ -951,9 +974,7 @@ func LoadPipeline(path string, models *Models, pair string, labels []string) (*P bad("stage %q: prompt_override template %s is not readable: %v", st.Name, p.Stages[i].PromptPath, err) } } - switch st.Reasoning { - case "", "off", "low", "medium", "high": - default: + if !ValidReasoningEffort(st.Reasoning) { bad("stage %q: reasoning must be off|low|medium|high, got %q", st.Name, st.Reasoning) } // D13.6: on an ADDITIVE-billing provider (xAI — reasoning bills ON TOP of completion) a @@ -1035,6 +1056,9 @@ func LoadPipeline(path string, models *Models, pair string, labels []string) (*P if rep.MaxCallsPerUnit <= 0 { bad("gates.repair.max_calls_per_unit must be > 0 when the gate is enabled") } + if !ValidReasoningEffort(rep.Reasoning) { + bad("gates.repair.reasoning must be off|low|medium|high, got %q", rep.Reasoning) + } if pair == "" { bad("gates.repair is enabled but the book declares no language pair — the repair prompts are resolved as //repair/.md") } else { @@ -1054,6 +1078,11 @@ func LoadPipeline(path string, models *Models, pair string, labels []string) (*P if tg.BudgetUSD <= 0 { bad("gates.terminology.budget_usd must be > 0 when the gate is enabled (a gate that can never spend is a silent no-op)") } + // The bank roles' effort knob answers to the same vocabulary as a stage's, by the same validator — + // the gate is where a book-level call class is configured, not a second dialect of the same key. + if !ValidReasoningEffort(tg.Reasoning) { + bad("gates.terminology.reasoning must be off|low|medium|high, got %q", tg.Reasoning) + } // Validated against the same standard-library table the runner resolves with // (terminology.ScriptByName), so an accepted name can never fail to resolve later. if tg.TargetScript == "" { diff --git a/backend/internal/llm/capability.go b/backend/internal/llm/capability.go index 57989ada..9649f985 100644 --- a/backend/internal/llm/capability.go +++ b/backend/internal/llm/capability.go @@ -51,11 +51,21 @@ type ReasoningControl string const ( // ReasoningNone is the OpenAI-compat baseline: low|medium|high go out as // reasoning_effort; off and "" emit nothing, leaving the provider's OWN default. - // Use it for a model whose thinking must NOT be pushed off that default from our - // side: DeepSeek-flash on the draft path defaults to thinking ON, and "off" here is - // a deliberate NO-OP (emitting nothing keeps it ON) — DISABLING DeepSeek thinking - // arms the echo mine (it returns the untranslated CJK source at HTTP 200; - // config.echoMineViolation fail-fasts on it). This is the WRONG control for a model + // Use it for a model that must never be TURNED OFF from our side: DeepSeek-flash + // defaults to thinking ON, and "off" here is a deliberate NO-OP (emitting nothing + // keeps it ON) — DISABLING DeepSeek thinking arms the echo mine (it returns the + // untranslated CJK source at HTTP 200; config.echoMineViolation fail-fasts on it). + // + // ⚠ "Never turned off" is NOT "never configured" (D39.87): low|medium|high are a + // ratified way to size the thinking BUDGET on such a model, and they go out here as + // reasoning_effort with thinking still on — proven on the wire, 22 bodies carrying + // reasoning_effort:"low", zero `thinking` keys, every reply with non-empty + // reasoning_content. That distinction became load-bearing when DeepSeek moved its OWN + // default effort to `high` on 2026-07-31 and dense-Han calls began hitting max_tokens + // with an empty body: riding the provider default is a choice whose owner is the + // vendor, not an absence of one. + // + // This is the WRONG control for a model // whose thinking is ON BY OMISSION: xAI-Grok defaults reasoning_effort to "low" (it // thinks), so a role that must NOT think (the editor) uses ReasoningEffortField with // OffEffort "none" to send an EXPLICIT reasoning_effort:"none" instead. diff --git a/backend/internal/pipeline/escalation.go b/backend/internal/pipeline/escalation.go index e6e68c04..26cf5eaa 100644 --- a/backend/internal/pipeline/escalation.go +++ b/backend/internal/pipeline/escalation.go @@ -103,13 +103,10 @@ func (r *Runner) maybeEscalate(ctx context.Context, st config.Stage, snapID stri // checkpoint exists and was already paid — REPLAY it for free regardless of the // budget, so a crash AFTER the hop settled but BEFORE chunk_status was written // re-serves it on resume rather than discarding a paid, successful translation - // and flipping the verdict OK→flagged. Only a FRESH hop is budget-gated. The - // hash mirrors runAttempt's for (model=ResolvedHop, attempt=0, maxTokens=hopMaxTokens). - fbHash := RequestHash(Request{ - BookID: r.Book.BookID, Chapter: ch.Chapter, ChunkIdx: ch.ChunkIdx, Attempt: 0, - Stage: st.Name, Role: st.Role, Model: st.ResolvedHop, Temperature: st.Temperature, Reasoning: st.Reasoning, - JSONOnly: false, MaxTokens: hopMaxTokens, SnapshotID: snapID, Messages: msgs, - }) + // and flipping the verdict OK→flagged. Only a FRESH hop is budget-gated. The identity is BUILT by the + // same helper the hop's own runAttempt uses (attemptRequest), so "mirrors runAttempt" is structural + // rather than a promise two field lists keep by vigilance. + fbHash := RequestHash(r.attemptRequest(st, st.ResolvedHop, snapID, ch, 0, hopMaxTokens, msgs)) fbExists, err := r.Store.GetCheckpoint(fbHash) if err != nil { return out, err diff --git a/backend/internal/pipeline/live_reprobe_test.go b/backend/internal/pipeline/live_reprobe_test.go index 0b641fb0..2ceba8be 100644 --- a/backend/internal/pipeline/live_reprobe_test.go +++ b/backend/internal/pipeline/live_reprobe_test.go @@ -23,13 +23,15 @@ package pipeline import ( "context" "encoding/json" + "fmt" "os" "path/filepath" + "strconv" "testing" "time" "textmachine/backend/internal/chunk" - "textmachine/backend/internal/config" + "textmachine/backend/internal/llm" "textmachine/backend/internal/obs" "textmachine/backend/internal/terminology" "textmachine/backend/internal/text" @@ -84,58 +86,107 @@ func TestLiveClassifierHarmSet(t *testing.T) { if err != nil { t.Fatalf("render production classifier messages: %v", err) } - model := r.Pipeline.Gates.Terminology.ClassifierModel() - _, maxTokens := r.bankCallBudget(model, msgs) + // The stage comes from the PRODUCTION derivation, not a literal: a probe that hand-built its stage would + // measure a shape the engine never sends — and once the effort knob landed, its green 6/6 would say + // nothing about the level the bank roles actually ride (D39.87 §0.2, the fourth synthetic-stage site). + st := r.bankStage(roleClassifier) + _, maxTokens := r.bankCallBudget(st.Model, msgs) - // The probe's own snapshot axis: a literal, so re-running it replays the paid checkpoint for $0 - // instead of re-buying the answer — the same property the production bank roles have. - const snapID = "reprobe-classifier-6of6" - // jobs.snapshot_id is a FOREIGN KEY into snapshots — the production path upserts the wave snapshot - // before opening jobs, so the probe registers its own axis the same way instead of borrowing one. - if err := r.Store.UpsertSnapshot(snapID, "", `{"probe":"classifier-harm-set-6of6"}`); err != nil { - t.Fatalf("register the probe snapshot axis: %v", err) + // N SAMPLES, not one. The verdict this probe produces is a judgement call by a stochastic model, and a + // single call cannot tell a level difference from ordinary variance — reading n=1 as a measurement is + // exactly the manufactured-convergence failure this repo has paid for before. Each run gets its OWN + // snapshot axis so it is a FRESH call; a repeat of the same index still replays that run for $0. + // Default 5, not 1. The comment above condemns n=1 and the first version of this rig then DEFAULTED to + // it — so the next person to run it would have got exactly the reading it warns against. A run costs + // ~$0.0002-0.0009, so five samples are free at the scale of any probe that would bother running this. + runs := 5 + if v := os.Getenv("TM_CLASSIFY6_N"); v != "" { + if n, cerr := strconv.Atoi(v); cerr == nil && n > 0 { + runs = n + } } - st := config.Stage{Name: terminologyStageName, Role: roleClassifier, Model: model} - job, err := r.Store.EnsureJob(r.Book.BookID, 0, terminologyStageName, snapID) - if err != nil { - t.Fatalf("ensure job: %v", err) + effort := st.Reasoning + if effort == "" { + effort = "unset" } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) defer cancel() - start := time.Now() - att, err := r.runBankAttempt(ctx, roleClassifier, model, st, snapID, chunk.Chunk{Chapter: 0, ChunkIdx: 0}, job, msgs) - if err != nil { - t.Fatalf("live classifier call failed: %v", err) + type sample struct { + Run int `json:"run"` + Finish string `json:"finish"` + Verdict string `json:"verdict"` + Usage llm.Usage `json:"usage"` + CostUSD float64 `json:"cost_usd"` + LatencyMS int64 `json:"latency_ms"` + Reply string `json:"reply"` + Parsed map[string]string `json:"parsed"` + BadLines int `json:"bad_lines"` + Hits int `json:"hits_term"` + DiscriminatingHits int `json:"discriminating_hits"` } - - got, stats := terminology.ParseTypes(att.text, candKeys(harmSet), text.NormalizeSourceKey) - hits, decided := 0, 0 - for _, c := range harmSet { - if got[c.Key] != "term" { - continue + samples := make([]sample, 0, runs) + full, totalCost := 0, 0.0 + for i := 0; i < runs; i++ { + snapID := fmt.Sprintf("reprobe-classifier-6of6-%s-%d", effort, i) + // jobs.snapshot_id is a FOREIGN KEY into snapshots — the production path upserts the wave snapshot + // before opening jobs, so the probe registers its own axis the same way instead of borrowing one. + if err := r.Store.UpsertSnapshot(snapID, "", `{"probe":"classifier-harm-set-6of6","effort":"`+effort+`"}`); err != nil { + t.Fatalf("register the probe snapshot axis: %v", err) } - hits++ - if c.Key != "元石" && c.Key != "灵泉" { // the two the prompt names verbatim - decided++ + job, jerr := r.Store.EnsureJob(r.Book.BookID, 0, terminologyStageName, snapID) + if jerr != nil { + t.Fatalf("ensure job: %v", jerr) } + start := time.Now() + att, aerr := r.runBankAttempt(ctx, st, snapID, chunk.Chunk{Chapter: 0, ChunkIdx: 0}, job, msgs) + if aerr != nil { + t.Fatalf("live classifier call %d failed: %v", i, aerr) + } + got, stats := terminology.ParseTypes(att.text, candKeys(harmSet), text.NormalizeSourceKey) + hits, decided := 0, 0 + for _, c := range harmSet { + if got[c.Key] != "term" { + continue + } + hits++ + if c.Key != "元石" && c.Key != "灵泉" { // the two the prompt names verbatim + decided++ + } + } + if hits == len(harmSet) { + full++ + } + totalCost += att.runCost + samples = append(samples, sample{i, att.finish, string(att.cls.Reason), att.usage, att.runCost, + time.Since(start).Milliseconds(), att.text, got, stats.Bad, hits, decided}) + t.Logf("CLASSIFIER 6/6 [effort=%s run=%d/%d]: model=%s max_tokens=%d finish=%q verdict=%q latency=%dms usage=%+v cost_usd=%.6f bad_lines=%d hits=%d/6 (discriminating %d/4) got=%v", + effort, i+1, runs, att.modelActual, maxTokens, att.finish, att.cls.Reason, + samples[i].LatencyMS, att.usage, att.runCost, stats.Bad, hits, decided, got) } - t.Logf("CLASSIFIER 6/6 PROBE: model=%s max_tokens=%d finish=%q verdict=%q latency=%dms usage=%+v cost_usd=%.6f bad_lines=%d hits=%d/6 (discriminating %d/4) got=%v", - att.modelActual, maxTokens, att.finish, att.cls.Reason, time.Since(start).Milliseconds(), att.usage, att.runCost, stats.Bad, hits, decided, got) + t.Logf("CLASSIFIER 6/6 SUMMARY [effort=%s]: %d/%d runs reached the 6/6 threshold · total_cost_usd=%.6f", effort, full, runs, totalCost) // Persist the raw wire evidence next to the probe project — "persist, not scratch": every number in - // the report must be re-readable from the artifact that produced it. + // the report must be re-readable from the artifact that produced it. The file is per-EFFORT, so a + // comparison between levels does not overwrite its own other half. blob, _ := json.MarshalIndent(map[string]any{ - "model": att.modelActual, "max_tokens": maxTokens, "finish": att.finish, - "verdict": string(att.cls.Reason), "usage": att.usage, "cost_usd": att.runCost, - "reply": att.text, "parsed": got, "bad_lines": stats.Bad, - "hits_term": hits, "discriminating_hits": decided, + "model": st.Model, "effort": effort, "max_tokens": maxTokens, + "runs": runs, "runs_at_threshold": full, "total_cost_usd": totalCost, "samples": samples, }, "", " ") - if werr := os.WriteFile(filepath.Join(filepath.Dir(cfg), "classifier-6of6.json"), blob, 0o644); werr != nil { + if werr := os.WriteFile(filepath.Join(filepath.Dir(cfg), "classifier-6of6-"+effort+".json"), blob, 0o644); werr != nil { t.Errorf("persist probe evidence: %v", werr) } - if hits != len(harmSet) { - t.Fatalf("acceptance threshold is 6/6 term (D39.69 §2); got %d/6: %v — reply:\n%s", hits, got, att.text) + // ⚠ THE THRESHOLD AND THE SAMPLE DISAGREE, and that is an OWNER decision, not something to soften here. + // D39.69 §2 ratified "6/6" against a SINGLE call; nobody defined it over a sample. Measured 02.08 on + // live 0731 weights: effort `low` reaches 6/6 on 4 of 5 runs, `high` on 5 of 5 — a difference n=5 cannot + // separate — while `high` costs 2.30x per call. So the level this engine now recommends for the bank + // roles makes this gate RED. Reporting that honestly is the point; relaxing the gate to fit the + // recommendation would be fitting the acceptance criterion to the result. + if full != runs { + t.Fatalf("the ratified acceptance threshold is 6/6 term (D39.69 §2) and it was reached on %d of %d runs "+ + "at effort %q — see classifier-6of6-%s.json. ⚠ The threshold was defined for ONE call, not for a "+ + "sample: decide with the owner whether it means \"every run\" or \"the median run\" before treating "+ + "this as a regression.", full, runs, effort, effort) } } diff --git a/backend/internal/pipeline/repair.go b/backend/internal/pipeline/repair.go index 6914aee8..2c2682ca 100644 --- a/backend/internal/pipeline/repair.go +++ b/backend/internal/pipeline/repair.go @@ -344,18 +344,29 @@ func (r *Runner) maybeRepair(ctx context.Context, st config.Stage, snapID string } // repairCheckpointExists reports whether THIS repair request was already paid for in an earlier run. It -// mirrors runRepairAttempt's request identity exactly (same synthetic stage, same ordinal, same budget -// derivation), because the two must address the same checkpoint — the escalation hop keeps the same -// discipline for the same reason (a crash after settle but before chunk_status must not lose paid work). +// mirrors runRepairAttempt's request identity BY CONSTRUCTION — same derived stage, same helper — because +// the two must address the same checkpoint; the doccomment used to promise that mirroring while the two +// field lists were maintained apart, which held only while the dropped fields were zero. func (r *Runner) repairCheckpointExists(st config.Stage, snapID string, ch chunk.Chunk, ordinal int, msgs []llm.Message) (bool, error) { model, maxTokens := r.repairCallBudget(msgs) - cp, err := r.Store.GetCheckpoint(RequestHash(Request{ - BookID: r.Book.BookID, Chapter: ch.Chapter, ChunkIdx: ch.ChunkIdx, Attempt: ordinal, - Stage: st.Name, Role: roleRepair, Model: model, MaxTokens: maxTokens, SnapshotID: snapID, Messages: msgs, - })) + rst := r.repairStage(st, model) + cp, err := r.Store.GetCheckpoint(RequestHash(r.attemptRequest(rst, model, snapID, ch, ordinal, maxTokens, msgs))) return cp != nil, err } +// repairStage derives the stage of a repair call from the stage whose output is being repaired. It takes +// the NAME from that stage (the accounting axis) and everything else from the repair gate. +// +// The effort is the GATE's, NOT inherited from the parent stage — see RepairGate.Reasoning for why: the +// value's meaning is per-control, the model is a different one, and inheriting would have moved the +// request hash of every already-paid repair call. +func (r *Runner) repairStage(st config.Stage, model string) config.Stage { + return config.InternalCall{ + Name: st.Name, Role: roleRepair, Model: model, + Reasoning: r.Pipeline.Gates.Repair.Reasoning, + }.Stage() +} + // repairCallBudget resolves the model and the max_tokens of a repair call — ONE definition, so the // checkpoint probe and the call itself can never address different request hashes. func (r *Runner) repairCallBudget(msgs []llm.Message) (string, int) { @@ -374,14 +385,19 @@ func (r *Runner) repairCallBudget(msgs []llm.Message) (string, int) { // runRepairAttempt performs ONE repair call on the shared money path: reserve → call → settle+checkpoint, // with a checkpoint hit replayed for free. It reuses runAttempt rather than re-implementing the money // sequence — a second copy of that sequence is exactly the drift the snapshot fold helper was extracted to -// prevent. The synthetic stage carries the repair ROLE and MODEL (both request-hash fields, so a repair call -// can never collide with the stage's own attempts) and no reasoning setting, so the provider default holds — -// on DeepSeek that means thinking stays ON and the echo mine is not armed. +// prevent. The derived stage carries the repair ROLE and MODEL (both request-hash fields, so a repair call +// can never collide with the stage's own attempts) and INHERITS the repaired stage's effort. +// +// It used to carry no reasoning setting at all, on a doccomment that read "the provider default holds — on +// DeepSeek that means thinking stays ON and the echo mine is not armed". The premise was falsified on the +// wire (D39.86): riding the vendor default is not a safe resting place, because the vendor MOVED it — to +// effort `high`, which walls a dense-Han call at max_tokens with an empty body. Thinking staying on was +// never the whole question; how much of the budget it eats is the other half. func (r *Runner) runRepairAttempt(ctx context.Context, st config.Stage, snapID string, ch chunk.Chunk, job *store.Job, ordinal int, msgs []llm.Message) (stageAttempt, error) { model, maxTokens := r.repairCallBudget(msgs) - rst := config.Stage{Name: st.Name, Role: roleRepair, Model: model} + rst := r.repairStage(st, model) // isFinal=false: the reply is a SPAN, not a shipping text, so the output sanitizer (which reasons about // whole chunks — preambles, trailing note blocks) must not judge it. The intrinsic classifier still runs // inside runAttempt and is a real guard: a model answering «Не могу помочь…» would otherwise pass every diff --git a/backend/internal/pipeline/runner.go b/backend/internal/pipeline/runner.go index 006065fc..c25e404c 100644 --- a/backend/internal/pipeline/runner.go +++ b/backend/internal/pipeline/runner.go @@ -6,6 +6,7 @@ import ( "log/slog" "os" "path/filepath" + "sort" "strings" "sync" "unicode" @@ -400,20 +401,48 @@ const sourcePlaceholder = "{{text}}" // It fires on no shipping path today (deepseek resolves to ReasoningNone, so "off" is a no-op and // thinking stays ON) and does fire for a reasoning-off editor arm on a CJK source, which is exactly the // exposure the stale "the editor only ever sees a Russian draft" comments used to deny. +// It covers the ENGINE'S OWN calls too, not just the book's stages. Those calls carry a knob only since +// D39.87, and the knob has an asymmetric empty value: on a ReasoningNone capability "" means the provider's +// default (thinking ON), while on an extra_body_disable one (GLM) it means thinking DISABLED — the echo zone +// — and gates.terminology.model is not restricted to a family. An unset knob pointed at such a model is +// therefore a silent thinking-off with no config line describing it, which is precisely what this walk +// exists to refuse to leave silent. func (r *Runner) sourceEchoExposure() []string { if !lang.IsCJKScriptLang(r.Book.SourceLang) { return nil } var out []string - for _, st := range r.Pipeline.Stages { - t := r.templates[st.Name] - if t == nil || !strings.Contains(t.System+t.FewShot+t.User, sourcePlaceholder) { - continue - } - if !r.Models.ThinksOnWire(st.ResolvedModel, st.Reasoning) { - out = append(out, st.Name+"→"+st.ResolvedModel) + shipsSource := func(t *PromptTemplate) bool { + return t != nil && strings.Contains(t.System+t.FewShot+t.User, sourcePlaceholder) + } + add := func(name, model, reasoning string) { + if !r.Models.ThinksOnWire(model, reasoning) { + out = append(out, name+"→"+model) } } + for _, st := range r.Pipeline.Stages { + if shipsSource(r.templates[st.Name]) { + add(st.Name, st.ResolvedModel, st.Reasoning) + } + } + // The bank roles ship source WITHOUT the placeholder: their candidate blocks carry KWIC contexts cut + // from the book, which is the whole reason the roles can render a term at all. So membership is by + // construction (the template is loaded ⇔ the role runs), not by scanning for a marker they do not use. + bank := r.Pipeline.Gates.Terminology + if r.terminologyTemplate != nil { + add(roleTerminologist, bank.Model, bank.Reasoning) + } + if r.classifierTemplate != nil { + add(roleClassifier, bank.ClassifierModel(), bank.Reasoning) + } + // Repair sends the source SPAN through the ordinary placeholder, per class — so it is scanned like a + // stage, and a class whose prompt is monolingual is correctly not listed. + for class, t := range r.repairTemplates { + if shipsSource(t) { + add(roleRepair+"/"+string(class), r.Pipeline.Gates.Repair.Model, r.Pipeline.Gates.Repair.Reasoning) + } + } + sort.Strings(out) // map iteration above — the warning must not reorder between runs return out } diff --git a/backend/internal/pipeline/stagerun.go b/backend/internal/pipeline/stagerun.go index 8fb300cb..c0b08fa6 100644 --- a/backend/internal/pipeline/stagerun.go +++ b/backend/internal/pipeline/stagerun.go @@ -393,17 +393,31 @@ type stageAttempt struct { freshCall bool // a provider call was made this run } +// attemptRequest is the ONE definition of a call's REQUEST IDENTITY — the tuple RequestHash addresses a +// checkpoint by. Every site that needs that identity goes through it: the attempt itself, the escalation +// hop's idempotency probe, and the two "was this already paid for?" probes of the bank and repair paths. +// +// It exists because those probes used to assemble the tuple by hand, each omitting Temperature and +// Reasoning, and agreed with runAttempt only while both values happened to be zero. Giving the internal +// calls a reasoning knob makes them non-zero, and a divergence there is not a stale read — it is money, in +// BOTH directions: a false "unpaid" makes a resumed pass re-run the budget pre-check and can cut a pass off +// before its first call, while a false "paid" skips the ROLE sub-budget gate (which only runs on an unpaid +// batch) and lets the attempt buy a fresh call under nothing but the book ceiling. One tuple, one place. +func (r *Runner) attemptRequest(st config.Stage, model, snapID string, ch chunk.Chunk, attempt, maxTokens int, msgs []llm.Message) Request { + return Request{ + BookID: r.Book.BookID, Chapter: ch.Chapter, ChunkIdx: ch.ChunkIdx, Attempt: attempt, + Stage: st.Name, Role: st.Role, Model: model, Temperature: st.Temperature, Reasoning: st.Reasoning, + JSONOnly: false, MaxTokens: maxTokens, SnapshotID: snapID, Messages: msgs, + } +} + // runAttempt executes exactly one attempt on the request-hash axis: a checkpoint // hit is classified for free (self-heal, incl. legacy Phase-0 empty/decode // checkpoints — no re-billing), otherwise a fresh reserve → call → settle+ // checkpoint. It returns an error only on an infra failure; a bad completion // comes back as a classification on the attempt. func (r *Runner) runAttempt(ctx context.Context, st config.Stage, model, snapID string, ch chunk.Chunk, job *store.Job, attempt, maxTokens int, msgs []llm.Message, escalation, isFinal bool) (stageAttempt, error) { - reqHash := RequestHash(Request{ - BookID: r.Book.BookID, Chapter: ch.Chapter, ChunkIdx: ch.ChunkIdx, Attempt: attempt, - Stage: st.Name, Role: st.Role, Model: model, Temperature: st.Temperature, Reasoning: st.Reasoning, - JSONOnly: false, MaxTokens: maxTokens, SnapshotID: snapID, Messages: msgs, - }) + reqHash := RequestHash(r.attemptRequest(st, model, snapID, ch, attempt, maxTokens, msgs)) att := stageAttempt{reqHash: reqHash, attempt: attempt, modelActual: model} // Resume on the attempt axis: a checkpoint means THIS attempt already happened diff --git a/backend/internal/pipeline/synthetic_stage_seam_test.go b/backend/internal/pipeline/synthetic_stage_seam_test.go new file mode 100644 index 00000000..c011963d --- /dev/null +++ b/backend/internal/pipeline/synthetic_stage_seam_test.go @@ -0,0 +1,395 @@ +package pipeline + +// synthetic_stage_seam_test.go: the mechanical guards behind the effort knob (D39.87). +// +// The defect the knob fixed was never "somebody forgot a key" — it was the SHAPE of the code: engine-internal +// calls each hand-built their own config.Stage, and a hand-built literal drops every field it does not name. +// Adding the key to the three shipping literals would have left the fourth site (and the fifth, tomorrow) +// exactly as broken, so the literals are gone. These tests are what keeps them gone. +// +// They walk the AST, not the bytes. A byte scan for `config.Stage{` was the first version and it was +// defeated five ways — `var s config.Stage` plus field assignment, `new(config.Stage)`, a type alias, a +// newline between `RequestHash(` and `Request{`, and the most natural rewrite of all, `req := Request{…}` +// followed by `RequestHash(req)`. It also failed the build on a doccomment that merely NAMED the type. A +// guard that a careless edit slips past is worse than none, because it reads as protection. + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" + "testing" + + "textmachine/backend/internal/chunk" + "textmachine/backend/internal/config" + "textmachine/backend/internal/llm" + "textmachine/backend/internal/store" +) + +// configPkgPath is the import path the guards resolve against. Matching the PATH, not the local package +// NAME, is load-bearing: `import cfg "…/internal/config"` followed by `cfg.Stage{…}` is a perfectly ordinary +// thing for an engineer to write, and a name-based predicate waves it through. That evasion was found by +// planting a realistic fifth call site — an annotator role — in shipping code, and the name-based version of +// this guard reported `ok`. +const configPkgPath = "textmachine/backend/internal/config" + +// walkedFileFloor is the smallest number of files a healthy walk parses. Without it, a walk that parses +// NOTHING — a bad root, a rename, a build-tag change — is indistinguishable from a clean tree, and the guard +// silently becomes decoration. That is the exact "reads as protection" failure this file's header warns +// about, so it is asserted rather than assumed. The tree holds ~200 .go files; the floor is deliberately far +// below that, because it exists to catch ZERO, not to track the file count. +const walkedFileFloor = 50 + +// fileImports maps a file's LOCAL package identifiers to the import paths they stand for. A dot-import is +// recorded under "." so a bare `Stage{…}` can be resolved too. +func fileImports(f *ast.File) map[string]string { + out := map[string]string{} + for _, im := range f.Imports { + path := strings.Trim(im.Path.Value, `"`) + name := path[strings.LastIndex(path, "/")+1:] // the default local name + if im.Name != nil { + name = im.Name.Name + } + out[name] = path + } + return out +} + +// walkGoFiles parses every .go file under the REPO root — not just internal/ — and hands each file's AST to +// visit. The reach past the Go module is deliberate and load-bearing: eval/ has no go.mod of its own yet +// imports this module's config package, and the ORIGINAL defect's fourth call site was exactly such a rig. +// Any go/types-based tool would be blind to it. +func walkGoFiles(t *testing.T, exempt func(slashPath string) bool, visit func(path string, fset *token.FileSet, f *ast.File, imports map[string]string)) { + t.Helper() + root := filepath.Join("..", "..", "..") // internal/pipeline -> backend -> repo root + parsed := 0 + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + // Trees that hold no call sites of ours and cost seconds to walk. + switch info.Name() { + case ".git", "node_modules", "vendor", "dist", "backups": + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") { + return nil + } + slash := filepath.ToSlash(path) + if exempt(slash) { + return nil + } + fset := token.NewFileSet() + f, perr := parser.ParseFile(fset, path, nil, 0) // no comments: a doccomment naming the type is not a call site + if perr != nil { + return nil // not part of this module's build (a testdata fixture, a scratch file) + } + parsed++ + visit(path, fset, f, fileImports(f)) + return nil + }) + if err != nil { + t.Fatal(err) + } + if parsed < walkedFileFloor { + t.Fatalf("the guard parsed only %d files (floor %d) — it is walking the wrong root or nothing at all, "+ + "which makes it decoration rather than a guard", parsed, walkedFileFloor) + } +} + +// namedType reports whether expr denotes pkgPath.name in a file with these imports, unwrapping the +// composite-literal wrappers `[]T`, `map[K]T` and `*T`. The unwrapping matters: `[]config.Stage{{…}}` ELIDES +// the element type on each element, so a literal-only check sees `&ast.CompositeLit{Type:nil}` and misses it +// — verified by planting exactly that and watching the guard report `ok`. +func namedType(expr ast.Expr, imports map[string]string, pkgPath, name string) bool { + switch t := expr.(type) { + case *ast.ArrayType: + return namedType(t.Elt, imports, pkgPath, name) + case *ast.MapType: + return namedType(t.Value, imports, pkgPath, name) + case *ast.StarExpr: + return namedType(t.X, imports, pkgPath, name) + case *ast.Ident: // a dot-imported type, or a package-local one + return t.Name == name && imports["."] == pkgPath + case *ast.SelectorExpr: + if t.Sel == nil || t.Sel.Name != name { + return false + } + id, ok := t.X.(*ast.Ident) + return ok && imports[id.Name] == pkgPath // by PATH, so an alias cannot slip through + } + return false +} + +// TestSyntheticStageSeamIsSingle: outside the config package there is exactly ONE way to obtain a Stage for +// an engine-internal call — config.InternalCall.Stage() — so a fifth call site inherits every decision the +// seam took instead of re-taking them by omission. This is the mechanical answer to the acceptance question +// "a FOURTH synthetic stage appears tomorrow: does it get the knob, or is it forgotten again?". +func TestSyntheticStageSeamIsSingle(t *testing.T) { + // Exemptions are FULL PATHS, never basenames: a basename exemption spreads to every file in the repo that + // happens to share the name, which is how `internal/anything/fewshot_test.go` would have inherited a pass. + exemptPaths := []string{ + "/backend/internal/pipeline/fewshot_test.go", // pure predicate test: no wire, no ledger, no checkpoint + "/backend/internal/pipeline/synthetic_stage_seam_test.go", // this guard builds a PARENT stage to test inheritance + } + var offenders []string + walkGoFiles(t, + func(slashPath string) bool { + for _, p := range exemptPaths { + if strings.HasSuffix(slashPath, p) { + return true + } + } + // The type's OWN package — the directory exactly, not a substring: `strings.Contains` would also + // exempt every SUBpackage (internal/config/annotator/) and any other tree ending in that path. + return strings.HasSuffix(filepath.ToSlash(filepath.Dir(slashPath)), "/backend/internal/config") + }, + func(path string, fset *token.FileSet, f *ast.File, imports map[string]string) { + isStage := func(e ast.Expr) bool { return namedType(e, imports, configPkgPath, "Stage") } + ast.Inspect(f, func(n ast.Node) bool { + var bad ast.Node + switch v := n.(type) { + case *ast.CompositeLit: // config.Stage{...}, []config.Stage{{...}}, map[string]config.Stage{...} + if v.Type != nil && isStage(v.Type) { + bad = v + } + case *ast.ValueSpec: // var s config.Stage (NOT unwrapped: `var ss []config.Stage` is a legitimate slice) + if v.Type != nil && namedTypeExact(v.Type, imports, configPkgPath, "Stage") { + bad = v + } + case *ast.CallExpr: // new(config.Stage) + if id, ok := v.Fun.(*ast.Ident); ok && id.Name == "new" && len(v.Args) == 1 && namedTypeExact(v.Args[0], imports, configPkgPath, "Stage") { + bad = v + } + case *ast.TypeSpec: // type myStage = config.Stage / type myStage config.Stage + if namedTypeExact(v.Type, imports, configPkgPath, "Stage") { + bad = v + } + } + if bad != nil { + offenders = append(offenders, fset.Position(bad.Pos()).String()) + } + return true + }) + }) + if len(offenders) > 0 { + t.Fatalf("an engine-internal call must derive its stage through config.InternalCall.Stage(), not by "+ + "building one (a hand-built stage silently drops reasoning/temperature — D39.87 §2); found: %v", offenders) + } +} + +// TestRequestIdentitySeamIsSingle: the tuple RequestHash addresses a checkpoint by is assembled in exactly +// ONE place (Runner.attemptRequest). Two assemblies of it agreeing is not a property anyone can maintain by +// eye — it held only while the omitted fields happened to be zero, and the effort knob makes them non-zero. +// +// The guard is "no Request composite literal outside attemptRequest", not "no RequestHash(Request{…})": +// the second form is one refactor away from invisible (hoist the literal into a variable), and it is the +// literal that is the hazard. +func TestRequestIdentitySeamIsSingle(t *testing.T) { + const seamFile = "stagerun.go" // the file that owns attemptRequest + // The tests OF the hashing contract legitimately build Requests — that is their subject. Named + // individually, so the exemption cannot spread to a file that merely happens to end in _test.go. + exemptPaths := []string{ + "/backend/internal/pipeline/" + seamFile, + "/backend/internal/pipeline/render.go", // where Request is DEFINED and hashed + // The tests OF the hashing contract legitimately build Requests — that is their subject. + "/backend/internal/pipeline/render_test.go", + "/backend/internal/pipeline/render_memory_test.go", + "/backend/internal/pipeline/synthetic_stage_seam_test.go", + } + var offenders []string + walkGoFiles(t, + func(slashPath string) bool { + for _, p := range exemptPaths { + if strings.HasSuffix(slashPath, p) { + return true + } + } + return false + }, + func(path string, fset *token.FileSet, f *ast.File, imports map[string]string) { + inPipeline := strings.HasSuffix(filepath.ToSlash(filepath.Dir(path)), "/backend/internal/pipeline") + ast.Inspect(f, func(n ast.Node) bool { + lit, ok := n.(*ast.CompositeLit) + if !ok || lit.Type == nil { + return true + } + hit := namedType(lit.Type, imports, pipelinePkgPath, "Request") + if !hit && inPipeline { // package-local `Request{…}` carries no qualifier to resolve + hit = namedTypeLocal(lit.Type, "Request") + } + if hit { + offenders = append(offenders, fset.Position(lit.Pos()).String()) + } + return true + }) + }) + if len(offenders) > 0 { + t.Fatalf("a call's request identity must come from Runner.attemptRequest, not a re-assembled Request "+ + "literal (a probe that drifts from the attempt turns the role sub-budget off — see attemptRequest); found: %v", offenders) + } +} + +// TestBankProbeAndAttemptAddressOneCheckpoint is the BEHAVIOURAL half, and it runs the REAL functions: +// bankCheckpointExists is asked about a checkpoint written under the hash the attempt path derives. If the +// two ever address different tuples the probe answers "unpaid" for paid work, or — the expensive direction +// — "paid" for work that was never bought, which skips the role sub-budget gate (it only runs on an unpaid +// batch) and lets the attempt buy a fresh call under nothing but the book ceiling. +// +// An earlier version of this test called attemptRequest twice with identical arguments and compared the +// results. That proves the function is deterministic and nothing else; it would have stayed green through +// exactly the drift it was named for. +func TestBankProbeAndAttemptAddressOneCheckpoint(t *testing.T) { + for _, effort := range []string{"", "low", "high"} { + t.Run("effort="+effort, func(t *testing.T) { + st, err := store.Open(filepath.Join(t.TempDir(), "probe.db")) + if err != nil { + t.Fatal(err) + } + defer st.Close() + r := &Runner{Book: &config.Book{BookID: "b1"}, Pipeline: &config.Pipeline{}, Models: &config.Models{}, Store: st} + r.Pipeline.Gates.Terminology.Model = "m1" + r.Pipeline.Gates.Terminology.Reasoning = effort + + msgs := []llm.Message{{Role: "user", Content: "candidate block"}} + ch := chunk.Chunk{Chapter: 0, ChunkIdx: 3} + const snapID = "snap-1" + stage := r.bankStage(roleTerminologist) + if stage.Reasoning != effort { + t.Fatalf("the gate's knob must reach the bank stage: want %q, got %q", effort, stage.Reasoning) + } + + paid, err := r.bankCheckpointExists(stage, snapID, ch, msgs) + if err != nil { + t.Fatal(err) + } + if paid { + t.Fatal("a fresh store must report the batch as unpaid") + } + + // Write a checkpoint under the hash the ATTEMPT path derives, through the PRODUCTION money + // sequence — then the probe must find it. Nothing here re-states the field list, so the test + // cannot pass by agreeing with a copy of the bug. + _, maxTokens := r.bankCallBudget(stage.Model, msgs) + hash := RequestHash(r.attemptRequest(stage, stage.Model, snapID, ch, 0, maxTokens, msgs)) + if err := st.UpsertSnapshot(snapID, "", "{}"); err != nil { + t.Fatal(err) + } + job, err := st.EnsureJob("b1", 0, terminologyStageName, snapID) + if err != nil { + t.Fatal(err) + } + resv, verdict, err := st.Reserve("b1", 0.001, store.Ceilings{BookUSD: 100, DayUSD: 100}) + if err != nil || verdict != store.ReserveOK { + t.Fatalf("reserve: %v verdict=%v", err, verdict) + } + if err := st.SettleWithCheckpoint(resv, 0.001, store.Checkpoint{ + RequestHash: hash, JobID: job.ID, ChunkIdx: ch.ChunkIdx, Stage: terminologyStageName, + Role: roleTerminologist, ModelRequested: stage.Model, ModelActual: stage.Model, + ResponseText: "reply", UsageJSON: "{}", CostUSD: 0.001, FinishReason: "stop", + }); err != nil { + t.Fatal(err) + } + paid, err = r.bankCheckpointExists(stage, snapID, ch, msgs) + if err != nil { + t.Fatal(err) + } + if !paid { + t.Fatalf("the probe missed the checkpoint the attempt path addressed (hash %s) — the two have "+ + "drifted apart, which turns the role sub-budget gate off", hash[:12]) + } + }) + } +} + +// TestReasoningIsPartOfTheRequestIdentity pins the axis itself, changing exactly ONE field. The previous +// version varied the effort AND the model at once, so it would have passed with Reasoning removed from +// RequestHash entirely — a green test over the very defect it named. +func TestReasoningIsPartOfTheRequestIdentity(t *testing.T) { + r := &Runner{Book: &config.Book{BookID: "b1"}, Pipeline: &config.Pipeline{}, Models: &config.Models{}} + r.Pipeline.Gates.Terminology.Model = "m1" + msgs := []llm.Message{{Role: "user", Content: "candidates"}} + ch := chunk.Chunk{Chapter: 0, ChunkIdx: 3} + + hashAt := func(effort string) string { + r.Pipeline.Gates.Terminology.Reasoning = effort + st := r.bankStage(roleTerminologist) + if st.Model != "m1" { + t.Fatalf("model must be held constant across the comparison, got %q", st.Model) + } + return RequestHash(r.attemptRequest(st, st.Model, "snap", ch, 0, 4096, msgs)) + } + unset, low, high := hashAt(""), hashAt("low"), hashAt("high") + if unset == low || low == high || unset == high { + t.Fatalf("reasoning must be part of the request identity; got unset=%s low=%s high=%s", + unset[:12], low[:12], high[:12]) + } +} + +// TestRepairStageTakesItsEffortFromItsOwnGate pins the decision the adversarial review forced: repair does +// NOT inherit the repaired stage's effort. Inheriting moved the request hash of every already-paid repair +// call (every shipping pipeline sets `reasoning: "off"` on its final stage), and it re-pointed a value +// whose MEANING is per-control at a DIFFERENT model — a documented DeepSeek no-op would have armed a live +// thinking-disable on a GLM repair model. +func TestRepairStageTakesItsEffortFromItsOwnGate(t *testing.T) { + r := &Runner{Book: &config.Book{BookID: "b1"}, Pipeline: &config.Pipeline{}} + parent := config.Stage{Name: "edit", Role: roleEditor, Model: "m-edit", Reasoning: "off", Temperature: 0.4} + + rst := r.repairStage(parent, "m-repair") + if rst.Reasoning != "" { + t.Fatalf("with gates.repair.reasoning unset the repair call must carry NO effort (byte-identical to "+ + "the pre-key behaviour, so paid checkpoints survive), got %q", rst.Reasoning) + } + r.Pipeline.Gates.Repair.Reasoning = "low" + if got := r.repairStage(parent, "m-repair").Reasoning; got != "low" { + t.Fatalf("the repair gate's own key must reach the call, got %q", got) + } + if rst.Role != roleRepair || rst.Model != "m-repair" { + t.Fatalf("repair keeps its OWN role and model (the hash axis that separates it from the stage): %+v", rst) + } + if rst.Temperature != 0 { + t.Fatalf("a repair span is re-parsed structure, not prose — temperature stays 0, got %v", rst.Temperature) + } +} + +// pipelinePkgPath is this package's own import path, for resolving `pipeline.Request{…}` written from +// outside it (eval/ harnesses do exactly this). +const pipelinePkgPath = "textmachine/backend/internal/pipeline" + +// namedTypeExact is namedType WITHOUT the []/map/* unwrapping. `var ss []config.Stage` is a legitimate +// slice variable, not a hand-built stage, so unwrapping there would false-flag ordinary code — it did, on +// waveStages' parameter list. +func namedTypeExact(expr ast.Expr, imports map[string]string, pkgPath, name string) bool { + switch t := expr.(type) { + case *ast.Ident: + return t.Name == name && imports["."] == pkgPath + case *ast.SelectorExpr: + if t.Sel == nil || t.Sel.Name != name { + return false + } + id, ok := t.X.(*ast.Ident) + return ok && imports[id.Name] == pkgPath + } + return false +} + +// namedTypeLocal matches an UNQUALIFIED type name, unwrapping composite-literal wrappers. Only meaningful +// for a file inside the package that declares the type. +func namedTypeLocal(expr ast.Expr, name string) bool { + switch t := expr.(type) { + case *ast.ArrayType: + return namedTypeLocal(t.Elt, name) + case *ast.MapType: + return namedTypeLocal(t.Value, name) + case *ast.StarExpr: + return namedTypeLocal(t.X, name) + case *ast.Ident: + return t.Name == name + } + return false +} diff --git a/backend/internal/pipeline/terminologist.go b/backend/internal/pipeline/terminologist.go index 4758ef27..7fc35b8e 100644 --- a/backend/internal/pipeline/terminologist.go +++ b/backend/internal/pipeline/terminologist.go @@ -283,7 +283,7 @@ func (r *Runner) runTerminologist(ctx context.Context, snapID string, cands []te // The signed bank is read ONCE for the whole role, not per batch: it is the same law for every batch. canon := r.approvedNeighbours() plan := bankRolePlan{ - role: roleTerminologist, model: r.Pipeline.Gates.Terminology.Model, budgetUSD: r.Pipeline.Gates.Terminology.BudgetUSD, + role: roleTerminologist, budgetUSD: r.Pipeline.Gates.Terminology.BudgetUSD, messages: func(b []terminology.Candidate) ([]llm.Message, error) { return r.terminologyMessages(b, canon) }, } run, err := r.runBankRoleBatches(ctx, snapID, plan, batches, "render") @@ -392,7 +392,7 @@ func (r *Runner) runClassifier(ctx context.Context, snapID string, cands []termi } batchRunes, _, _ := r.terminologyOpts() batches := terminology.Batch(cands, batchRunes, nil) - plan := bankRolePlan{role: roleClassifier, model: g.ClassifierModel(), budgetUSD: g.ClassifyBudgetUSD, messages: r.classifierMessages} + plan := bankRolePlan{role: roleClassifier, budgetUSD: g.ClassifyBudgetUSD, messages: r.classifierMessages} run, err := r.runBankRoleBatches(ctx, snapID, plan, batches, "classify") if err != nil { return nil, run, err @@ -481,24 +481,28 @@ const terminologyReplyFloor = 256 // bankCallEstimateUSD projects ONE call's cost with the same price/estimate arithmetic the reservation uses, // so the pre-call number and the reserved number are the same number. -func (r *Runner) bankCallEstimateUSD(model string, msgs []llm.Message) float64 { - _, maxTokens := r.bankCallBudget(model, msgs) +func (r *Runner) bankCallEstimateUSD(st config.Stage, msgs []llm.Message) float64 { + _, maxTokens := r.bankCallBudget(st.Model, msgs) promptEst := 0 for _, m := range msgs { promptEst += EstimateTokens(m.Content) } - return ledger.EstimateUSD(r.Pricer.PriceFor(model), promptEst, maxTokens, - r.Models.AdditiveReasoningTokens(model, "", 0)) + // The buffer is read off the SAME stage the call will use rather than passed as a literal, so this line + // cannot drift from the attempt's. ⚠ It is structurally 0 on every path today and that is NOT because of + // the effort: AdditiveReasoningTokens ignores its effort argument entirely (D39.26 добор B) and returns 0 + // whenever the declared buffer is 0, which InternalCall pins. The gate's additive-provider refusal + // (config.LoadPipeline) is what makes that safe rather than blind. + return ledger.EstimateUSD(r.Pricer.PriceFor(st.Model), promptEst, maxTokens, + r.Models.AdditiveReasoningTokens(st.Model, st.Reasoning, st.ReasoningMaxTokens)) } // bankCheckpointExists reports whether THIS batch was already paid for in an earlier run, on the role's own -// request-hash axis. -func (r *Runner) bankCheckpointExists(role, model string, st config.Stage, snapID string, ch chunk.Chunk, msgs []llm.Message) (bool, error) { - _, maxTokens := r.bankCallBudget(model, msgs) - cp, err := r.Store.GetCheckpoint(RequestHash(Request{ - BookID: r.Book.BookID, Chapter: ch.Chapter, ChunkIdx: ch.ChunkIdx, Attempt: 0, - Stage: st.Name, Role: role, Model: model, MaxTokens: maxTokens, SnapshotID: snapID, Messages: msgs, - })) +// request-hash axis — the SAME identity runBankAttempt will address (attemptRequest), never a hand-rebuilt +// copy of it: this probe gates the role sub-budget, so an identity that drifts from the attempt's turns the +// gate off (see attemptRequest). +func (r *Runner) bankCheckpointExists(st config.Stage, snapID string, ch chunk.Chunk, msgs []llm.Message) (bool, error) { + _, maxTokens := r.bankCallBudget(st.Model, msgs) + cp, err := r.Store.GetCheckpoint(RequestHash(r.attemptRequest(st, st.Model, snapID, ch, 0, maxTokens, msgs))) return cp != nil, err } @@ -508,27 +512,58 @@ func (r *Runner) bankCheckpointExists(role, model string, st config.Stage, snapI // isFinal=false: the reply is a term table, not shipping prose, so the output sanitizer must not judge it; // the intrinsic classifier still runs and is a real guard (a refusal reply would otherwise be parsed as // terminology). -func (r *Runner) runBankAttempt(ctx context.Context, role, model string, st config.Stage, snapID string, ch chunk.Chunk, +func (r *Runner) runBankAttempt(ctx context.Context, st config.Stage, snapID string, ch chunk.Chunk, job *store.Job, msgs []llm.Message) (stageAttempt, error) { - _, maxTokens := r.bankCallBudget(model, msgs) - tst := config.Stage{Name: st.Name, Role: role, Model: model} + _, maxTokens := r.bankCallBudget(st.Model, msgs) // The log axis, exactly as runStage sets it for a chunk stage: without it every bank-role line in a paid // run reads "calling model deepseek-v4-flash" with no book, no role and no batch — unattributable in a // multi-book log and unmatchable to the batch that produced a bad reply. Chunk carries the batch ordinal. ri, _ := obs.ReqInfoFromContext(ctx) - ri.Book, ri.Chapter, ri.Chunk, ri.Stage, ri.Role = r.Book.BookID, ch.Chapter, ch.ChunkIdx, tst.Name, role + ri.Book, ri.Chapter, ri.Chunk, ri.Stage, ri.Role = r.Book.BookID, ch.Chapter, ch.ChunkIdx, st.Name, st.Role ctx = obs.WithReqInfo(ctx, ri) - return r.runAttempt(ctx, tst, model, snapID, ch, job, 0, maxTokens, msgs, false, false) + return r.runAttempt(ctx, st, st.Model, snapID, ch, job, 0, maxTokens, msgs, false, false) +} + +// bankRoleSettings resolves what a bank ROLE is called with. The classifier phase may run its OWN model +// (classification is cheaper than a render), but BOTH phases share ONE effort knob — deliberately, and on +// evidence: paired samples of the ratified 6/6 acceptance set put the classifier at 4/5 on `low` against +// 5/5 at `high`, which n=5 cannot separate, while `high` costs 2.3× per call. The render phase, by +// contrast, is FORCED to a low effort (at the vendor default it burns the whole budget thinking and +// returns an empty body). One level satisfies both, so a second key would be a split nothing measured. +func (r *Runner) bankRoleSettings(role string) (model, reasoning string) { + g := r.Pipeline.Gates.Terminology + switch role { + case roleClassifier: + return g.ClassifierModel(), g.Reasoning + case roleTerminologist: + return g.Model, g.Reasoning + } + // A future bank role (the annotator, the Ф2 judge this seam anticipates) must not silently inherit the + // terminologist's model and effort: that is the same "a default is indistinguishable from a decision" + // shape this pack exists to remove. Panic is right here — the role set is a compile-time constant of the + // engine, so reaching this is a programming error, not a config one, and it can only happen before any + // money moves. + panic("pipeline: bankRoleSettings has no settings for bank role " + role + " — add its resolution to the gate") +} + +// bankStage is the ONE place a bank-role call's stage is derived — for BOTH roles and for the live rig, so +// "the classifier probe measured the production shape" is structural rather than two literals kept in step +// by hand. The settings come from the gate that owns the call class (config.InternalCall). +func (r *Runner) bankStage(role string) config.Stage { + model, reasoning := r.bankRoleSettings(role) + return config.InternalCall{Name: terminologyStageName, Role: role, Model: model, Reasoning: reasoning}.Stage() } // bankRolePlan is one bank-level role's plan over a batch list: its cost axis (role/model), its own book-wide // ceiling, and how it builds one batch's wire messages. The money sequence — estimate, budget-gate, // checkpoint-or-call — is identical for the classifier and the terminologist; only the messages and the reply // PARSING differ, and parsing is the caller's job on the returned texts. +// ⚠ NO `model` field: the model is derived from the role by bankRoleSettings, inside bankStage. It used to +// be carried here too and read only by a log line, which made the log a SECOND source of truth about what +// was billed — the exact shape this pack removed everywhere else. type bankRolePlan struct { role string - model string budgetUSD float64 messages func(batch []terminology.Candidate) ([]llm.Message, error) } @@ -548,6 +583,9 @@ type bankRoleRun struct { // aborts on this optional step. logKind names the phase (render|classify) in the logs. func (r *Runner) runBankRoleBatches(ctx context.Context, snapID string, plan bankRolePlan, batches [][]terminology.Candidate, logKind string) (bankRoleRun, error) { run := bankRoleRun{texts: make([]string, len(batches))} + // ONE stage for the whole pass — the estimate, the checkpoint probe and the attempt all read it, so the + // three can never be sized against different knobs (the estimate is the reservation's own upper bound). + st := r.bankStage(plan.role) msgsPer := make([][]llm.Message, len(batches)) for i, b := range batches { m, err := plan.messages(b) @@ -555,17 +593,16 @@ func (r *Runner) runBankRoleBatches(ctx context.Context, snapID string, plan ban return run, err } msgsPer[i] = m - run.estimateUSD += r.bankCallEstimateUSD(plan.model, m) + run.estimateUSD += r.bankCallEstimateUSD(st, m) } r.Log.InfoContext(ctx, "terminology "+logKind+": estimate before any call", - "book", r.Book.BookID, "role", plan.role, "batches", len(batches), "model", plan.model, + "book", r.Book.BookID, "role", plan.role, "batches", len(batches), "model", st.Model, "reasoning", st.Reasoning, "estimate_usd", fmt.Sprintf("%.6f", run.estimateUSD), "budget_usd", plan.budgetUSD, "version", terminologyVersion) spent, err := r.Store.RoleSpentUSD(r.Book.BookID, plan.role) if err != nil { return run, fmt.Errorf("pipeline: %s budget read: %w", plan.role, err) } - st := config.Stage{Name: terminologyStageName, Role: plan.role, Model: plan.model} job, err := r.Store.EnsureJob(r.Book.BookID, 0, terminologyStageName, snapID) if err != nil { return run, fmt.Errorf("pipeline: %s job: %w", plan.role, err) @@ -574,19 +611,19 @@ func (r *Runner) runBankRoleBatches(ctx context.Context, snapID string, plan ban // The synthetic chunk addresses the batch: chapter 0 is the BOOK level (no real chapter is 0), and the // batch ordinal is the chunk index, so two batches can never collide on one checkpoint. ch := chunk.Chunk{Chapter: 0, ChunkIdx: i} - paid, perr := r.bankCheckpointExists(plan.role, plan.model, st, snapID, ch, msgsPer[i]) + paid, perr := r.bankCheckpointExists(st, snapID, ch, msgsPer[i]) if perr != nil { return run, perr } // The budget is checked BEFORE the call and against what the call would COST (the reservation's own // upper bound), so it refuses on the conservative side and the config number is the bound it looks like. - if want := r.bankCallEstimateUSD(plan.model, msgsPer[i]); !paid && spent+want > plan.budgetUSD { + if want := r.bankCallEstimateUSD(st, msgsPer[i]); !paid && spent+want > plan.budgetUSD { r.Log.WarnContext(ctx, "terminology "+logKind+": budget would be exceeded by the next batch; the remaining terms are left unchanged", "book", r.Book.BookID, "role", plan.role, "spent_usd", fmt.Sprintf("%.6f", spent), "next_batch_usd", fmt.Sprintf("%.6f", want), "budget_usd", plan.budgetUSD, "batches_left", len(batches)-i) break } - att, aerr := r.runBankAttempt(ctx, plan.role, plan.model, st, snapID, ch, job, msgsPer[i]) + att, aerr := r.runBankAttempt(ctx, st, snapID, ch, job, msgsPer[i]) if aerr != nil { // A ceiling denial must not abort the book: this step is optional and the draft wave is already // paid for. Degrade to "no change" exactly as the repair sub-step degrades.