Bounded reasoning dialect normalization (#135) #145
No reviewers
Labels
No labels
accessibility
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
P1
P2
P3
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Creatidy/scarcity-router!145
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "compat/reasoning-dialects"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #135 (child C of program #132). Stacked on PR #144 (child B): the flip ledger and error-table additions build on it. Retarget to
developas the stack merges.What
The fix for acceptance failure 2 (
unknown_parameteron the real client's reasoning payload). Three evidenced dialect forms are accepted through a bounded normalization layer and folded ONCE at parse time into the canonicalreasoning_effortintent — routing and adapters see exactly one canonical value; the dialect fields are never forwarded, stored or re-interpreted.reasoning: {"effort": "<effort>"}— closed object; effort-bearing; must agree withreasoning_effort(contradiction → typed 400conflicting_reasoning_parameters).thinking: {"type": "enabled"|"disabled"}andenable_thinking: true|false— consistency assertions, never effort selectors: disabled flag + explicit effort conflicts; contradictory flags conflict; enabled assertion with no effort anywhere → explicitreasoning_effort_required(no guessing); disabled-only = no reasoning requested; empty object asserts nothing (stream_options precedent).unknown_parameter(reasoning.<key>/thinking.<key>); malformed shapes (string instead of object, non-bool flag, bogus effort/type) are typed 400s.unsupported_reasoning_effort— the dialect layer changes parsing, never eligibility.minimal..high) reconciled with the shipped vocabulary (..xhigh|max|ultra, D-053); error table updated.Tests
New
tests/test_reasoning_dialects.py(17 tests): the exact four-form ZCode payload normalizes tomaxwith empty generation params; each dialect individually; agreeing duplicates; all three conflict classes; enabled-only explicit failure; disabled-only no-op; empty-object no-op; unknown nested keys; malformed shapes; unknown top-level keys; boundary tests (clean 400 at the HTTP surface; normalized-max executes against the exact max-only identity with selected==executed audit). The #135 pin in the representative ledger flips; #136/#137 pins untouched.Gates
uv sync --only-dev✓make check— 2100 tests OK ✓ (the load-flake #143 did not recur this run; it remains filed and unrelated)uv run basedpyright— 0 / 0 / 0 ✓git diff --check✓UX impact
Direct positive: the real client's payload is accepted with the semantics it asked for (
max), contradictory requests get a single actionable conflict code instead of a generic parse error, and nothing is silently dropped. No previously-valid request changes behavior.Review
Independent GLM-5.3-Flash exact-head review requested. Not security-critical (parsing only; no new trust boundary, no credential path; the strict-allowlist principle is preserved and extended, not weakened). PR left open for the human merge gate.
The evidenced client dialects — reasoning: {effort}, thinking: {type} and enable_thinking — are accepted alongside reasoning_effort and normalized ONCE at parse time into the single canonical reasoning intent. Strictness is preserved, not relaxed: - closed object shapes; unknown nested keys stay unknown_parameter - effort-bearing forms must agree; contradiction is the typed 400 conflicting_reasoning_parameters - enable/disable forms are consistency assertions, never effort selectors: disabled flag + explicit effort conflicts; contradictory flags conflict; enabled assertion with no effort anywhere fails explicitly (reasoning_effort_required) — no effort guessed - disabled-only means no reasoning requested; empty object asserts nothing - dialect fields are consumed at parse time: they never reach routing, adapters or generation params - D-054 untouched: a normalized effort still fails at the model boundary when the exact identity does not offer it Docs: execution-surface allowlist + new normalization section; stale effort-vocabulary line reconciled with the code vocabulary (xhigh, max, ultra, D-053); error table gains conflicting_reasoning_parameters. The #135 representative pin flips: the exact four-form payload now parses and streams like the canonical request.Independent review (GLM-5.3-Flash, fresh reviewer context) of reviewed_head
e29039f49ea6f2fd6068d571fed30b9d011cfc2a(diff8c3dd67..e29039f, on top of the approved #142/#144 stack) — VERDICT: APPROVE.Verified: normalization happens exactly once and the canonical effort is the only value stored; no leak path (ChatCompletionRequest carries no dialect fields, generation params unaffected, AdapterCall carries only
reasoning_effort); closed shapes with dottedunknown_parameterparams; all three conflict classes and the enabled-without-effort explicit failure match the frozen contract rule-for-rule; precedence deterministic; top-level strictness untouched; D-054 untouched (coordinator rejectionunsupported_reasoning_effortgenuinely exercised; normalized-max end-to-end asserts selected==executed max); previously-valid requests unaffected (normalization is a no-op for them); docs allowlist matchesCHAT_COMPLETION_ALLOWED_KEYSexactly, the new section matches the code, the effort-vocabulary line now matchesREASONING_EFFORTS, and the error table is updated; 17 discriminating hermetic tests; diff = exactly the 4 frozen files, one commit, no merge commits,git diff --checkclean.Findings (all DEFER, no blockers):
reasoning: {"effort": null}/thinking: {"type": null}are treated as absent whilereasoning_effort: nullandenable_thinking: nullare rejected; unspecified by the frozen contract, nothing guessed. Document a null convention or reject explicit nulls in a follow-up.code="invalid_request"in the OpenAI envelope, a field pre-existing parser errors leave null and which is absent from the docs error table; benign — align or document later.application.executeend-to-end test would close the parse→selection seam outright; current parse-level + execute-level tests compose to cover it.Reviewer gates:
uv run basedpyright0/0/0 at the reviewed head; 59 focused tests OK (dialect module 17/17 in 0.5 s); full suite deliberately not run (known #143 load-flake). UX impact: none-negative — the real client payload executes like the canonical control; all rejections are explicit typed 400s with preciseparamfields; no semantics silently dropped.Reviewer-model disclosure: per repository rule this review used a GLM-5.3-Flash-class reviewer (no plain GLM-5.3), and this PR is not classified security_critical so no Daybreak gate applies.