Bounded reasoning dialect normalization (#135) #145

Merged
adrian.tkacz merged 2 commits from compat/reasoning-dialects into develop 2026-09-27 15:47:30 +00:00
Member

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 develop as the stack merges.

What

The fix for acceptance failure 2 (unknown_parameter on 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 canonical reasoning_effort intent — 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 with reasoning_effort (contradiction → typed 400 conflicting_reasoning_parameters).
  • thinking: {"type": "enabled"|"disabled"} and enable_thinking: true|false — consistency assertions, never effort selectors: disabled flag + explicit effort conflicts; contradictory flags conflict; enabled assertion with no effort anywhere → explicit reasoning_effort_required (no guessing); disabled-only = no reasoning requested; empty object asserts nothing (stream_options precedent).
  • Closed shapes throughout: unknown nested keys stay unknown_parameter (reasoning.<key> / thinking.<key>); malformed shapes (string instead of object, non-bool flag, bogus effort/type) are typed 400s.
  • D-054 intact: normalization is model-independent; a normalized effort the exact identity does not offer still fails at the model boundary with unsupported_reasoning_effort — the dialect layer changes parsing, never eligibility.
  • Docs: execution-surface allowlist + new "Reasoning-dialect normalization" section; the stale effort-vocabulary line (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 to max with 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.

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 `develop` as the stack merges. ## What The fix for acceptance failure 2 (`unknown_parameter` on 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 canonical `reasoning_effort` intent — 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 with `reasoning_effort` (contradiction → typed 400 `conflicting_reasoning_parameters`). - `thinking: {"type": "enabled"|"disabled"}` and `enable_thinking: true|false` — consistency assertions, never effort selectors: disabled flag + explicit effort conflicts; contradictory flags conflict; enabled assertion with no effort anywhere → explicit `reasoning_effort_required` (no guessing); disabled-only = no reasoning requested; empty object asserts nothing (stream_options precedent). - Closed shapes throughout: unknown nested keys stay `unknown_parameter` (`reasoning.<key>` / `thinking.<key>`); malformed shapes (string instead of object, non-bool flag, bogus effort/type) are typed 400s. - **D-054 intact:** normalization is model-independent; a normalized effort the exact identity does not offer still fails at the model boundary with `unsupported_reasoning_effort` — the dialect layer changes parsing, never eligibility. - Docs: execution-surface allowlist + new "Reasoning-dialect normalization" section; the stale effort-vocabulary line (`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 to `max` with 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.
Author
Member

Independent review (GLM-5.3-Flash, fresh reviewer context) of reviewed_head e29039f49ea6f2fd6068d571fed30b9d011cfc2a (diff 8c3dd67..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 dotted unknown_parameter params; 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 rejection unsupported_reasoning_effort genuinely exercised; normalized-max end-to-end asserts selected==executed max); previously-valid requests unaffected (normalization is a no-op for them); docs allowlist matches CHAT_COMPLETION_ALLOWED_KEYS exactly, the new section matches the code, the effort-vocabulary line now matches REASONING_EFFORTS, and the error table is updated; 17 discriminating hermetic tests; diff = exactly the 4 frozen files, one commit, no merge commits, git diff --check clean.

Findings (all DEFER, no blockers):

  1. DEFER — null-value asymmetry: reasoning: {"effort": null} / thinking: {"type": null} are treated as absent while reasoning_effort: null and enable_thinking: null are rejected; unspecified by the frozen contract, nothing guessed. Document a null convention or reject explicit nulls in a follow-up.
  2. DEFER — dialect shape errors carry 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.
  3. DEFER — test-strengthening nit: one dialect-only (no canonical field) application.execute end-to-end test would close the parse→selection seam outright; current parse-level + execute-level tests compose to cover it.

Reviewer gates: uv run basedpyright 0/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 precise param fields; 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.

Independent review (GLM-5.3-Flash, fresh reviewer context) of reviewed_head `e29039f49ea6f2fd6068d571fed30b9d011cfc2a` (diff `8c3dd67..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 dotted `unknown_parameter` params; 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 rejection `unsupported_reasoning_effort` genuinely exercised; normalized-max end-to-end asserts selected==executed max); previously-valid requests unaffected (normalization is a no-op for them); docs allowlist matches `CHAT_COMPLETION_ALLOWED_KEYS` exactly, the new section matches the code, the effort-vocabulary line now matches `REASONING_EFFORTS`, and the error table is updated; 17 discriminating hermetic tests; diff = exactly the 4 frozen files, one commit, no merge commits, `git diff --check` clean. Findings (all DEFER, no blockers): 1. DEFER — null-value asymmetry: `reasoning: {"effort": null}` / `thinking: {"type": null}` are treated as absent while `reasoning_effort: null` and `enable_thinking: null` are rejected; unspecified by the frozen contract, nothing guessed. Document a null convention or reject explicit nulls in a follow-up. 2. DEFER — dialect shape errors carry `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. 3. DEFER — test-strengthening nit: one dialect-only (no canonical field) `application.execute` end-to-end test would close the parse→selection seam outright; current parse-level + execute-level tests compose to cover it. Reviewer gates: `uv run basedpyright` 0/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 precise `param` fields; 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.
adrian.tkacz changed target branch from compat/logical-model-resolution to develop 2026-09-27 15:46:49 +00:00
Merge branch 'develop' into compat/reasoning-dialects
All checks were successful
ci / check (pull_request) Successful in 7m41s
e07b257bb6
adrian.tkacz deleted branch compat/reasoning-dialects 2026-09-27 15:47:30 +00:00
Sign in to join this conversation.
No description provided.