From 1e9d3e89d8964b78c5fe93865f71d25ce2af9e57 Mon Sep 17 00:00:00 2001 From: dtzp555 Date: Mon, 27 Jul 2026 12:02:21 +1000 Subject: [PATCH] =?UTF-8?q?fix(models):=20rewrite=20the=20maxTokens=20rati?= =?UTF-8?q?onale=20=E2=80=94=20my=20justification=20was=20factually=20wron?= =?UTF-8?q?g?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The numbers were right, the code was right, and the reason I gave for both was wrong. Verified every part of the review's finding first-hand before rewriting. WHAT I CLAIMED: OpenClaw caps outbound requests via `maxTokens = Math.min(baseMaxTokens + thinkingBudget, modelMaxTokens)`, so a 16384 declaration at `high` reasoning left ~1024 tokens for the visible answer, and raising it would produce longer answers and higher quota burn. WHY THAT CANNOT HAPPEN FOR OCP — three independent breaks, each confirmed here: scripts/sync-openclaw.mjs:75 api: "openai-completions" The clamp I cited lives behind `case "anthropic-messages"`; the openai-completions transport has no thinkingBudget concept at all, so that code never runs for a claude-local provider. buildCliArgs pushes: --input-format, --disallowedTools, --allowedTools, --dangerously-skip-permissions, --mcp-config No output-token flag of any kind reaches the CLI. OCP cannot cap output even if it wanted to. grep max_completion_tokens server.mjs lib/ -> 0 occurrences That is the field OpenClaw would send to a local openai-completions endpoint, and this repo never reads it. So "expect longer answers and higher per-request quota burn" was a promise that would not have materialised. That is worse than a wrong number: a release note that tells operators to expect a behavioural change which cannot occur. The change still stands, on the honest reason: models.json is the SPOT (ADR 0003) and its values should be true. maxTokens is ADVERTISED metadata consumed only by clients that choose to honour it. CHANGELOG and the test comment now say exactly that, and say plainly that OCP behavior is unchanged. Test replaced, not just re-worded. The old one asserted `maxTokens > 16384`, which review showed lets every entry sit at 16385 and still call itself "registry-aligned" — the actual claim had no coverage. Now pinned per model against a recorded table of CLI 2.1.220 id-anchored values, with an explicit failure for any model missing a row. Mutation-proven on both holes: all entries at 16385 -> CAUGHT (the old test passed this) sonnet-4-6 wrongly at 64000 -> CAUGHT Also fixed ocp-connect's unknown-id fallback (8192 -> 32000). Review flagged it as contradicting the invariant: 32000 is the LOWEST max_output_tokens.default in the registry, so it is the safe floor and no longer sits below every family in its own table. Unreachable today, wrong to leave inconsistent. Verified: bash -n on ocp-connect and py_compile on its embedded python block. Tests: 458 passed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8 --- CHANGELOG.md | 2 +- ocp-connect | 6 +++++- test-features.mjs | 34 ++++++++++++++++++++-------------- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b7d019..44eaa6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changed -- **`maxTokens` now matches the CLI registry instead of a uniform 16384 (#195).** Every Opus entry and `claude-sonnet-5` go to **64000**, `claude-sonnet-4-6` and `claude-haiku-4-5` to **32000** — the `max_output_tokens.default` each model actually declares in the compiled CLI 2.1.220 registry. OCP never enforced this value; it is propagated to OpenClaw, where it **caps the outbound request**: `maxTokens = Math.min(baseMaxTokens + thinkingBudget, modelMaxTokens)`. OpenClaw's reasoning budgets are medium 8192 / high 16384, and when `maxTokens <= thinkingBudget` it clamps thinking to `maxTokens - 1024` — so a model declared at 16384 running at **high** reasoning spent ~15360 on thinking and left **~1024 tokens for the actual answer**. `ocp-connect`'s independent family table moves to the family floor (opus 64000, sonnet 32000, haiku 32000) since prefixes cannot distinguish versions. Expect longer answers and correspondingly higher per-request quota burn on long generations. +- **`maxTokens` now matches the CLI registry instead of a uniform 16384 (#195).** Every Opus entry and `claude-sonnet-5` go to **64000**, `claude-sonnet-4-6` and `claude-haiku-4-5` to **32000** — the `max_output_tokens.default` each model declares in the compiled CLI 2.1.220 registry. This corrects **advertised metadata only**: `models.json` is the SPOT (ADR 0003) and every value in it should be the truth about the model. **It changes nothing about how OCP behaves.** OCP never enforces `maxTokens` — `buildCliArgs` passes no output-token flag to the CLI at all — and OpenClaw addresses a local OCP over `openai-completions`, whose request field (`max_completion_tokens`) appears nowhere in this repo. The value is consumed only by clients that choose to honour it, via `setup.mjs` / `scripts/sync-openclaw.mjs` / `ocp-connect`. **Expect no change in answer length or quota burn.** `ocp-connect`'s independent family table moves to the family floor (opus 64000, sonnet 32000, haiku 32000) since prefixes cannot distinguish versions. ## v3.25.0 — 2026-07-27 diff --git a/ocp-connect b/ocp-connect index 2495491..15cc7b3 100755 --- a/ocp-connect +++ b/ocp-connect @@ -147,7 +147,11 @@ def get_model_meta(mid): for prefix, meta in sorted(model_meta.items(), key=lambda x: -len(x[0])): if mid.startswith(prefix): return meta - return {"name": mid + " (OCP)", "reasoning": False, "maxTokens": 8192} + # Unknown id = most likely a model newer than this script. 32000 is the LOWEST + # max_output_tokens.default in the CLI 2.1.220 registry (haiku/sonnet-4-6), so it is the + # safe floor: never over-advertises a real model, and no longer contradicts the rest of + # this table by sitting below every family in it. + return {"name": mid + " (OCP)", "reasoning": False, "maxTokens": 32000} for mid in model_ids: meta = get_model_meta(mid) diff --git a/test-features.mjs b/test-features.mjs index a932eef..6eb9b69 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -4299,21 +4299,27 @@ test("models.json: every aliases value resolves to a real models[].id (referenti } }); -// maxTokens must leave room for a visible answer once a client's thinking budget is taken out -// (#195). OCP never enforces maxTokens itself — it is propagated to OpenClaw (setup.mjs, -// scripts/sync-openclaw.mjs) where it CAPS the outbound request: -// maxTokens = Math.min(baseMaxTokens + thinkingBudget, modelMaxTokens) -// OpenClaw's reasoning budgets are medium 8192 / high 16384, and when maxTokens <= thinkingBudget -// it clamps thinking to maxTokens - 1024. So a model declared at 16384 running at `high` spent -// ~15360 on thinking and left ~1024 for the actual answer — which is what this repo shipped until -// v3.25.0. Asserting the PRINCIPLE rather than pinning per-model numbers: a value test would need -// editing every time a model is added, and would not say why. -const _spotHighThinkingBudget = 16384; // OpenClaw's `high` reasoning budget -test("models.json: every maxTokens exceeds the high thinking budget, leaving room for output (#195)", () => { +// maxTokens is ADVERTISED metadata, not an OCP-enforced limit (#195). OCP never reads it — +// buildCliArgs passes no output-token flag to the CLI — and OpenClaw reaches a local OCP over +// `openai-completions`, whose request field (max_completion_tokens) appears nowhere in this repo. +// It is consumed only by clients that choose to honour it, via setup.mjs / sync-openclaw.mjs / +// ocp-connect. So the invariant worth testing is simply that models.json tells the truth: each +// value must equal the model's max_output_tokens.default in the CLI registry. +// +// Pinned per model deliberately. A threshold assertion would let every entry sit at some arbitrary +// value above the bar and still call itself "registry-aligned" — which is the actual claim. Adding +// a model means adding a row here, and that is the point: the row is where you record what the +// registry said when you checked. +const _spotRegistryMaxTokens = { // CLI 2.1.220, id-anchored max_output_tokens.default + "claude-opus-5": 64000, "claude-opus-4-8": 64000, "claude-opus-4-7": 64000, "claude-opus-4-6": 64000, + "claude-sonnet-5": 64000, "claude-sonnet-4-6": 32000, "claude-haiku-4-5-20251001": 32000, +}; +test("models.json: every maxTokens equals the CLI registry's max_output_tokens.default (#195)", () => { for (const m of _spotModels.models) { - assert.ok(m.maxTokens > _spotHighThinkingBudget, - `${m.id} declares maxTokens=${m.maxTokens} <= ${_spotHighThinkingBudget}: at OpenClaw's 'high' reasoning ` + - `level the thinking budget would consume all but ~1024 tokens of the response`); + const want = _spotRegistryMaxTokens[m.id]; + assert.ok(want !== undefined, + `${m.id} has no recorded registry value — extract it id-anchored from the CLI binary and add a row`); + assert.equal(m.maxTokens, want, `${m.id}: models.json says ${m.maxTokens}, CLI registry says ${want}`); } });