🧠 narrow thinking policy for codex and pi - #163
Conversation
The thinkingPolicy eitherOf group had one fullSupport branch covering codex-app-server and pi with all seven canonical values. Neither backend accepts all seven: the Codex app-server answers HTTP 400 for off and minimal across the GPT-5.6 family, and Pi has no minimal level. - Split fullSupport into codexSupport (low through max) and piSupport (off plus low through max). Start and resume stay identical for both. - Regenerate thinking_policy.gen.go from the schema. - Flip the two conformance rows that asserted the old policy, and add rows for each new exclusion boundary on start and resume. - Keep minimal in the canonical tuple. No backend accepts it now, but it is the vocabulary hosts derive their enums from, and a backend can gain it without a vocabulary change. A test pins the current state. - Update the per-backend tables in docs/thinking-level.md and docs/backends.md, and record why each exclusion exists. The TypeScript evaluator in packages/core derives the policy from the same schema and needed no change. Verified with go build ./..., go vet ./..., go test ./..., go test -race -count=1 on internal/thinkingpolicy, internal/runtime, and internal/runtime/pi, and bun test packages/core/src/thinking-policy.test.ts (42 pass, 0 fail). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closing unmerged. The premise is wrong. This PR encodes The existing design already draws this line. Adding a The Separate follow-up, not addressed here: avenor passes the canonical value through verbatim, and no codex effort level is spelled |
![umpire-bot[bot]](https://avatars.githubusercontent.com/in/3739611?s=60&v=4){kind=small}
| Satisfied: func() bool { v := f.Thinking; return v != nil && *v != "" }(), | ||
| Fair: (f.Thinking == nil || ((c.Backend == "codex-app-server" || c.Backend == "pi") && (f.Thinking != nil && *f.Thinking == "off" || f.Thinking != nil && *f.Thinking == "minimal" || f.Thinking != nil && *f.Thinking == "low" || f.Thinking != nil && *f.Thinking == "medium" || f.Thinking != nil && *f.Thinking == "high" || f.Thinking != nil && *f.Thinking == "xhigh" || f.Thinking != nil && *f.Thinking == "max")) || ((c.Backend == "claude" || c.Backend == "claude-channel") && c.Resume == false && (f.Thinking != nil && *f.Thinking == "low" || f.Thinking != nil && *f.Thinking == "medium" || f.Thinking != nil && *f.Thinking == "high" || f.Thinking != nil && *f.Thinking == "xhigh" || f.Thinking != nil && *f.Thinking == "max"))), | ||
| Fair: (f.Thinking == nil || (c.Backend == "codex-app-server" && (f.Thinking != nil && *f.Thinking == "low" || f.Thinking != nil && *f.Thinking == "medium" || f.Thinking != nil && *f.Thinking == "high" || f.Thinking != nil && *f.Thinking == "xhigh" || f.Thinking != nil && *f.Thinking == "max")) || (c.Backend == "pi" && (f.Thinking != nil && *f.Thinking == "off" || f.Thinking != nil && *f.Thinking == "low" || f.Thinking != nil && *f.Thinking == "medium" || f.Thinking != nil && *f.Thinking == "high" || f.Thinking != nil && *f.Thinking == "xhigh" || f.Thinking != nil && *f.Thinking == "max")) || ((c.Backend == "claude" || c.Backend == "claude-channel") && c.Resume == false && (f.Thinking != nil && *f.Thinking == "low" || f.Thinking != nil && *f.Thinking == "medium" || f.Thinking != nil && *f.Thinking == "high" || f.Thinking != nil && *f.Thinking == "xhigh" || f.Thinking != nil && *f.Thinking == "max"))), | ||
| Reason: func() *string { |
There was a problem hiding this comment.
The Reason (and Reasons) fields in ThinkingPolicyAvailability.Thinking are computed in the generated Check function (thinking_policy.gen.go:85-122) but are never asserted on by any test. TestBackendPolicyGenerated only checks Fair and Valid (backend_policy_test.go:58-65). Since these reason strings encode the per-backend block explanations (e.g. "pi supports off and low through max"), a change to them would go uncaught. Add assertions on avail.Thinking.Reason (and Reasons) for at least one representative case so the generated reason text is pinned.
| // Pi: off plus low..max, start and resume. Pi has no minimal level. | ||
| {"pi", str("off"), false, true, true}, | ||
| {"pi", str("xhigh"), true, true, true}, | ||
| {"pi", str("minimal"), false, false, true}, |
There was a problem hiding this comment.
Minor duplication: the two adjacent table entries at backend_policy_test.go:31-32 ({"pi", str("minimal"), false, false, true} and {"pi", str("minimal"), true, false, true}) differ only in the resume flag with identical expected values. The identical expectations do document that resume does not change the outcome for pi+minimal, which is legitimate, but the near-duplicate rows add a little noise. Consider a brief comment noting the resume invariance or collapsing into a single row with a resume-agnostic note.
1 participant
What
The
thinkingPolicyeitherOf group had onefullSupportbranch. It coveredcodex-app-serverandpitogether with all seven canonical values. Neither backend accepts all seven, so avenor forwarded values certain to fail after the session already existed.codex-app-serverlow,medium,high,xhigh,maxpioff,low,medium,high,xhigh,maxclaude/claude-channellow–max(unchanged)fullSupportbecomescodexSupportandpiSupport. The generated branch constantFullSupportsplits intoCodexSupportandPiSupport. One test referenced it.Why each exclusion
The Codex app-server has no
offorminimaleffort level. It answers HTTP 400 for both across the GPT-5.6 family, verified againstgpt-5.6-luna,-terra,-sol, and-mini.lowthroughmaxare accepted on start and on resume.Pi has no
minimallevel.offis a real Pi level. It reaches Pi verbatim through--thinkingon a fresh client andset_thinking_levelon a reused one.Both providers pass the canonical string through as the wire value (
internal/runtime/codexappserver/provider.go:89,internal/runtime/pi/provider.go:371). The schema is therefore the only place that can reject these before a provider starts.minimalis now canonical with no backendNo backend accepts
minimalafter this change. It stays in the canonical tuple, which is the shared vocabulary hosts derive their enums from. A backend can gain the level without a vocabulary change. Removing it would touchTHINKING_LEVELS,CanonicalValues(), the MCP enum, and every fixture row.TestMinimalIsCanonicalButUnsupportedpins the current state. A backend that starts acceptingminimalfails that test, which points at the policy docs.TypeScript needed no change
packages/core/src/thinking-policy.tsparses the same schema'seitherOfbranches at module load and evaluates them directly. Its 42 conformance tests pass on the new branch shape with no source edit. Go derivessupportedBackendsfrom the schema the same way. That is the propertyinternal/thinkingpolicy/doc.goclaims, exercised by a real policy change.Verification
go build ./...,go vet ./..., andgo test ./...pass.go test -race -count=1passes oninternal/thinkingpolicy,internal/runtime, andinternal/runtime/pi.bun test packages/core/src/thinking-policy.test.tsreports 42 pass, 0 fail.Four hand-written expectation sites tracked the old policy and now assert the new one:
internal/thinkingpolicy/backend_policy_test.goFullSupport→CodexSupport/PiSupport; per-backend accept and reject rowsinternal/thinkingpolicy/thinking_policy_test.goEvaluatetable driven from per-backend supported sets;StartValuesfor codex and piinternal/runtime/thinking_test.goUnsupportedValuerather than start-onlyinternal/runtime/pi/provider_test.gominimaldropped from the fresh-start flag subtestsA full
bun testrun has 20 other failures. All are missingnode_modules(@umpire/core,zod,@earendil-works/pi-tui), unrelated to this change.thinking_policy.gen.gowas regenerated with thego:generatedirective indoc.go(umpire-go-gen@v0.1.1) and is committed. Nothing regenerates it at build or test time.Provenance of the pi row
The codex and claude rows are measured. The pi row —
offsupported, nominimallevel — comes from the repository owner rather than from a run against Pi 0.83.0. Confirm it againstpi --helpbefore release.Adjacent, not addressed here
No CI job runs
go generateand diffs the result. A schema edit that changes only branch logic, with no fixture change, would compile and pass. The conformance tests compare fixtures against the compiledCheck, so they catch fixture-versus-generated drift but not schema-versus-generated drift.TestSchemaCanonicalLockstepcovers only the canonical tuple and the generated struct shapes.readSchemaRulesreads../../schemas/thinking_policy.umpire.jsonrelative to the working directory. Outside the package's own tests it therefore falls back to the hardcoded backend list invalidate.go. That list is still correct, and it only selects the wording of a rejection becauseFaircomes from compiled code. It does narrow the schema-only-change promise indoc.go.Roster support for
thinkingis separate. There is no roster entry spec inschemas/.spawn_selection.umpire.jsongoverns how a spawn selects an entry, not what an entry contains. A roster entry validates at load time on a fresh start, sothinkingpolicy.Evaluate(entry.Backend, entry.Thinking, false)answers it without a new schema.🤖 Generated with Claude Code