diff --git a/eval/harbor/RUN_LUNA_TB21.md b/eval/harbor/RUN_LUNA_TB21.md new file mode 100644 index 00000000..dcb721c1 --- /dev/null +++ b/eval/harbor/RUN_LUNA_TB21.md @@ -0,0 +1,120 @@ +# Running `openai/gpt-5.6-luna` on terminal-bench 2.1 + +OpenRouter-proxied GPT-5.6 Luna, at reasoning effort `max`, through the +clawcodex Harbor adapter. + +## Model facts (OpenRouter `/models`, fetched 2026-07-31) + +| | | +|---|---| +| id | `openai/gpt-5.6-luna` (a `-pro` variant exists with identical specs) | +| context | 1,050,000 tokens (registered as 1,048,576 = 2^20, matching the sibling gpt-5.6 rows; under-reading is the safe direction) | +| max output | 128,000 tokens | +| price | $0.10/M in, $0.60/M out — doubling to $0.20 / $0.90 above 272K prompt tokens | +| reasoning | `reasoning_effort` supported; verified honored, not just accepted — reasoning tokens rise low 148 → medium 154 → high 266 → max 516 on a fixed prompt | +| tools | `tools` + `tool_choice` supported; returns `finish_reason=tool_calls` | +| vision | text + image + file → text | + +## Prerequisite: a build that actually sends the effort + +Until 2026-07-31, clawcodex emitted reasoning effort **only** on the +Anthropic wire. On the headless `--print` path the adapter drives, +`--effort` was a silent no-op for every OpenAI-compatible provider — +OpenRouter, OpenAI, DeepSeek, Z.AI. The job config would record +`effort=max` and the request body would carry no effort field at all. + +Two consequences: + +1. Containers must install a clawcodex build dated 2026-07-31 or later, so + pin `--ak source=git+…@` at a commit that includes the + fix. The PyPI package (1.2.1) does not. +2. Earlier non-Anthropic runs labelled `effort=max` — including the + `tb21-deepseek-max` job in `jobs/` — actually ran at the provider's + default effort. Do not compare a new Luna run against those as if the + effort setting matched. + +## Commands + +Run from the repo root. `PYTHONPATH` must point at `eval/harbor` so Harbor +can import the adapter in the host process. + +```bash +export OPENROUTER_API_KEY=$(python3 -c \ + "import json,os;print(json.load(open(os.path.expanduser('~/.clawcodex/config.json')))['providers']['openrouter']['api_key'])") + +export CX_SOURCE=git+https://github.com/agentforce314/clawcodex@main # must contain the effort fix +``` + +### Smoke first — two tasks, ~5 minutes + +Never start an 89-task run without this; a broken install or a missing key +fails identically on every task and costs an hour to find out. + +```bash +PYTHONPATH=$PWD/eval/harbor harbor run \ + --dataset terminal-bench/terminal-bench-2-1 \ + --agent clawcodex_agent:Clawcodex \ + --model openrouter/openai/gpt-5.6-luna \ + --ak effort=max \ + --ak source=$CX_SOURCE \ + -i 'terminal-bench/fix-git' \ + -i 'terminal-bench/openssl-selfsigned-cert' \ + --job-name smoke-tb21-luna-max \ + --jobs-dir eval/harbor/jobs \ + --n-concurrent 2 +``` + +Expect reward 1.0 on both and zero infra errors. Then confirm the effort +really shipped — the whole point of the pin: + +```bash +grep -ro '"reasoning_effort": *"[a-z]*"' eval/harbor/jobs/smoke-tb21-luna-max | head +``` + +### Full run — all 89 tasks + +```bash +PYTHONPATH=$PWD/eval/harbor harbor run \ + --dataset terminal-bench/terminal-bench-2-1 \ + --agent clawcodex_agent:Clawcodex \ + --model openrouter/openai/gpt-5.6-luna \ + --ak effort=max \ + --ak source=$CX_SOURCE \ + --job-name tb21-luna-max \ + --jobs-dir eval/harbor/jobs \ + --n-concurrent 4 +``` + +`-k 5` matches the official leaderboard's pass@5 methodology (the published +Claude Code 0.79 is k=5); the in-repo 3-way comparison numbers — clawcodex +0.58, openclaude 0.551, claude-code 0.719 — are all k=1, so keep k=1 to +compare against those. + +### Results + +```bash +harbor view eval/harbor/jobs # browse trajectories +python3 eval/harbor/compare_trajectories.py … # NOT ad-hoc counting: + # a denominator mistake + # once inverted 4 metrics +``` + +## Gotchas + +- **Model string is doubly-qualified.** Harbor splits `--model` on the FIRST + slash only, so `openrouter/openai/gpt-5.6-luna` → `--provider openrouter + --model openai/gpt-5.6-luna`. That is intended; do not "fix" it to a + single slash. +- **Hub datasets namespace task names.** Include filters need the prefix: + `-i 'terminal-bench/fix-git'`. A bare `-i fix-git` matches nothing. +- **The key must be in the host environment.** The adapter forwards + `OPENROUTER_API_KEY` from the host env into the container; it does not + read `~/.clawcodex/config.json` for the provider key. Hence the `export` + above. Alternatively pass `--ae OPENROUTER_API_KEY="$OPENROUTER_API_KEY"`. +- **Docker credential helper.** If image pulls hang forever, check + `~/.docker/config.json` for `credsStore: "desktop"` — a Docker Desktop + update can rewrite it, and it wedges even anonymous pulls of public + images. It should be `osxkeychain`. +- **Cost.** Luna is cheap (~$0.10/M in). A full 89-task run at k=1 lands in + the low single-digit dollars, versus roughly two orders of magnitude more + for an Opus run. diff --git a/eval/harbor/clawcodex_agent.py b/eval/harbor/clawcodex_agent.py index 34ee6fbc..4e6b412b 100644 --- a/eval/harbor/clawcodex_agent.py +++ b/eval/harbor/clawcodex_agent.py @@ -26,6 +26,20 @@ ``--provider deepseek --model deepseek-v4-flash``). A bare name is passed through as ``--model`` alone, falling back to clawcodex's own routing. +The split is on the FIRST slash only, so an OpenRouter id — itself +``vendor/model`` — round-trips correctly by stacking the two conventions: +``openrouter/openai/gpt-5.6-luna`` → ``--provider openrouter --model +openai/gpt-5.6-luna``. For example, terminal-bench 2.1 at maximum +reasoning effort:: + + export OPENROUTER_API_KEY=sk-or-v1-... + PYTHONPATH=eval/harbor harbor run \ + --dataset terminal-bench/terminal-bench-2-1 \ + --agent clawcodex_agent:Clawcodex \ + --model openrouter/openai/gpt-5.6-luna \ + --ak effort=max \ + --jobs-dir eval/harbor/jobs --n-concurrent 4 + API keys reach the container two ways, either is sufficient: * exported in the host environment (the running provider's key vars are @@ -38,13 +52,35 @@ * ``max_turns`` — clawcodex ``--max-turns`` (default 300 here; the CLI's own default of 50 is too low for terminal-bench tasks). The ``CLAWCODEX_MAX_TURNS`` host env var works as a fallback. -* ``effort`` — clawcodex ``--effort`` (low|medium|high|xhigh|max) for - models that support ``output_config.effort`` (Opus 5, Opus 4.6/4.8, - Sonnet 4.6, Fable 5). ``xhigh`` is model-dependent (opus-5/opus-4-8 yes; - sonnet-4-6/opus-4-6 no) — clawcodex degrades it to ``high`` where - rejected. A model missing from clawcodex's effort allowlist drops the - flag SILENTLY, so a new model needs a clawcodex build that registers it - (see ``source`` below) before an effort number means anything. +* ``effort`` — clawcodex ``--effort`` (low|medium|high|xhigh|max). How it + reaches the wire depends on the provider family, and the two behave + differently: + + - **Anthropic wire** — sent as ``output_config.effort``, and gated on + clawcodex's effort allowlist (Opus 5, Opus 4.6/4.8, Sonnet 4.6, + Fable 5). A model missing from that allowlist drops the flag SILENTLY, + so a new Anthropic model needs a clawcodex build that registers it + (see ``source`` below) before an effort number means anything. + ``xhigh`` is model-dependent (opus-5/opus-4-8 yes; sonnet-4-6/opus-4-6 + no) and degrades to ``high`` where rejected. + - **OpenAI-compatible wire** (openrouter, openai, deepseek, zai, …) — + sent as the top-level ``reasoning_effort`` body field, with no model + allowlist and no ``xhigh`` clamp; the level passes through verbatim. + ONE exception: the ChatGPT-subscription path (``subscription=true`` + with ``--model openai/…``) clamps ``xhigh`` and ``max`` down to + ``high`` before sending, because that backend advertises only + low/medium/high and rejects higher tiers. So ``--ak effort=max`` plus + subscription auth really runs at ``high`` — an API-key or OpenRouter + run of the same model does not. + + REQUIRES a clawcodex build from 2026-07-31 or later. Before that, effort + was emitted ONLY on the Anthropic branch, so ``--effort`` was a silent + no-op for every OpenAI-compatible provider — an eval run against, say, + ``openrouter/openai/gpt-5.6-luna`` would report ``effort=max`` in its + config and send nothing. Pin ``source`` accordingly when benchmarking a + non-Anthropic model at a specific effort, and confirm the level is really + on the wire rather than trusting the job config. + ``CLAWCODEX_EFFORT`` host env var works as a fallback. * ``version`` — pin a ``clawcodex-cli`` PyPI version (default: latest). * ``source`` — full pip-installable spec overriding the PyPI package, e.g. diff --git a/src/models/configs.py b/src/models/configs.py index 6b3450fa..291d2419 100644 --- a/src/models/configs.py +++ b/src/models/configs.py @@ -416,6 +416,31 @@ class ModelConfig: context_window=1_048_576, max_output_tokens=128_000, ), + # The same model as the row above, under its OpenRouter id. This table is + # keyed by BARE name and ``get_model_config`` deliberately does not strip + # a ``/`` prefix (see its docstring), so without this row the id + # that actually reaches the provider on the OpenRouter path matches + # nothing and silently falls back to DEFAULT_CONTEXT_WINDOW (200K) — a 5x + # under-read that makes auto-compact fire at a fifth of the real window. + # Added for the terminal-bench harness, which drives this model as + # ``--model openrouter/openai/gpt-5.6-luna``; Harbor splits on the FIRST + # slash, so clawcodex receives ``--model openai/gpt-5.6-luna``. + # + # Only Luna is duplicated, not the whole Sol/Terra family: a qualified row + # is the narrow, per-model answer to a general gap, and adding rows + # nobody routes yet is speculative duplication. The general fix (teaching + # the resolver the vendor prefix, as ``get_pricing`` already does) is + # "decision #1" and stays out of scope — see the docstring. + # + # Base for the prefix fallback is "openai/gpt-5.6", which nothing else + # claims, so ``openai/gpt-5.6-luna-pro`` resolves here too — matching how + # the bare rows above let ``gpt-5.6-sol-pro`` through. + "openai/gpt-5.6-luna": ModelConfig( + model_id="openai/gpt-5.6-luna", + display_name="GPT-5.6 Luna", + context_window=1_048_576, + max_output_tokens=128_000, + ), "gpt-5.5": ModelConfig( model_id="gpt-5.5", display_name="GPT-5.5", @@ -486,7 +511,25 @@ class ModelConfig: def get_model_config(model_id: str) -> ModelConfig | None: - """Get config for a model, or None if unknown.""" + """Get config for a model, or None if unknown. + + Exact match, then a prefix fallback for date-variant ids (a row's claimed + prefix is its key minus the last ``-``-segment). + + NOT attempted: stripping a leading ``/`` segment so OpenRouter ids + resolve to their bare row. ``get_pricing`` (services/pricing.py) does + exactly that, and mirroring it here is tempting — but it is deliberately + out of scope, the same call ``tests/test_deepseek_prefix_cache.py`` pins + as "decision #1" (``deepseek/deepseek-v4-pro`` keeps the 200K default). + Two reasons it is not a free win: it would silently outrank a user's own + ``modelLimits`` override, which ``get_context_window_for_model`` consults + only when this returns ``None``; and it would resolve ids whose bare name + prefix-matches an unrelated row, in the window-WIDENING direction, which + overflows the request instead of merely compacting early. Reversing that + decision is its own change with its own test sweep. A vendor-qualified + model that needs a real window gets an explicit row instead — see + ``openai/gpt-5.6-luna``. + """ if model_id in MODEL_CONFIGS: return MODEL_CONFIGS[model_id] # Try prefix match (for date-variant models) diff --git a/src/providers/openai_provider.py b/src/providers/openai_provider.py index 0bdf5881..2845039e 100644 --- a/src/providers/openai_provider.py +++ b/src/providers/openai_provider.py @@ -55,12 +55,23 @@ def _subscription_reasoning_effort(requested: str | None = None) -> str: """Reasoning effort for subscription requests. Precedence: the session's ``/effort`` setting (arrives as - ``extra_body.reasoning_effort`` via the agent-server's - ``_EffortProvider`` wrapper) → ``CLAWCODEX_OPENAI_REASONING_EFFORT`` - → ``medium`` (OpenCode's default, transform.ts:1176, and the - backend's own default_reasoning_level). ``xhigh``/``max`` clamp to - ``high`` — the general gpt-5.x models advertise low/medium/high and - reject higher tiers. + ``extra_body.reasoning_effort``, injected at the wire boundary by + ``query.py::_call_model_sync`` for every OpenAI-compatible provider) + → ``CLAWCODEX_OPENAI_REASONING_EFFORT`` → ``medium`` (OpenCode's + default, transform.ts:1176, and the backend's own + default_reasoning_level). + + ``xhigh``/``max`` clamp to ``high`` HERE, and only here: this is the + ChatGPT-subscription backend (chatgpt.com/backend-api/codex), whose + general gpt-5.x models advertise low/medium/high and reject higher + tiers (probed 2026-07-25). That is narrower than the public API — + developers.openai.com/api/docs/guides/reasoning lists none | minimal | + low | medium | high | xhigh | max and notes support varies by model — + and narrower than what a gateway may accept (``openai/gpt-5.6-luna`` + via OpenRouter takes both ``xhigh`` and ``max``, probed 2026-07-31, + with reasoning-token counts rising monotonically across the ladder). + So the clamp is a property of THIS backend, not of the level names; + the generic OpenAI-compatible path deliberately does not clamp. """ for candidate in (requested, os.environ.get("CLAWCODEX_OPENAI_REASONING_EFFORT")): effort = (candidate or "").strip().lower() @@ -275,8 +286,8 @@ def _subscription_request_body( "stream": True, "include": list(INCLUDE_ENCRYPTED_REASONING), "reasoning": { - # /effort arrives as extra_body.reasoning_effort via the - # agent-server's _EffortProvider wrapper (agent_server.py). + # /effort arrives as extra_body.reasoning_effort, injected + # at the wire boundary by query.py::_call_model_sync. "effort": _subscription_reasoning_effort( (kwargs.get("extra_body") or {}).get("reasoning_effort") ), diff --git a/src/query/agent_loop_compat.py b/src/query/agent_loop_compat.py index 53933fc5..1dc2cacb 100644 --- a/src/query/agent_loop_compat.py +++ b/src/query/agent_loop_compat.py @@ -403,13 +403,17 @@ async def _maybe_recall_memories( query_text = _last_user_text(messages) if not query_text.strip(): return None - # R5 (ch11 N1) — unwrap /effort's _EffortProvider so the recall SELECTOR - # runs on the raw provider: (a) _resolve_recall_model's - # isinstance(AnthropicProvider) check sees through the wrapper → the - # small_fast_model cost pin applies in effort mode too (it was bypassed — - # a bare wrapper class isn't an AnthropicProvider); (b) the wrapper's - # reasoning_effort injection doesn't leak into the cheap selector call. - # Safe: _inner is _EffortProvider-exclusive, so this is a no-op otherwise. + # R5 (ch11 N1) — unwrap any provider decorator so the recall SELECTOR runs + # on the raw provider: _resolve_recall_model's isinstance(AnthropicProvider) + # check has to see through it, or the small_fast_model cost pin is bypassed + # (a bare wrapper class isn't an AnthropicProvider). + # + # This was introduced for /effort's ``_EffortProvider``, which has since + # been deleted — reasoning effort is now applied at the wire boundary in + # query.py for both provider families rather than by wrapping. Kept as a + # cheap general guard: ``_inner`` is not an attribute any real provider + # defines, so this is a no-op unless some future decorator reintroduces + # the same shape. provider = getattr(provider, "_inner", provider) try: from src.memdir import get_auto_mem_path diff --git a/src/query/query.py b/src/query/query.py index 30971a9c..0394211c 100644 --- a/src/query/query.py +++ b/src/query/query.py @@ -554,8 +554,10 @@ def _model_supports_xhigh_effort(model: str | None) -> bool: VALID_THINKING_EFFORT_LEVELS = ("low", "medium", "high", "xhigh", "max") -def resolve_thinking_effort(explicit: str | None, model: str | None) -> str | None: - """Effective ``output_config.effort`` value for one request, or ``None`` +def resolve_thinking_effort( + explicit: str | None, model: str | None, *, clamp_xhigh: bool = True +) -> str | None: + """Effective reasoning-effort value for one request, or ``None`` to omit the parameter entirely. Precedence mirrors TS main.tsx:2631 ``parseEffortValue(options.effort) @@ -571,6 +573,19 @@ def resolve_thinking_effort(explicit: str | None, model: str | None) -> str | No :func:`_model_supports_xhigh_effort`'s allowlist rather than 400ing the request; ``"max"`` passes through everywhere effort-capable (see the probe notes on the allowlist helper). + + ``clamp_xhigh=False`` disables that degradation, for callers on the + OpenAI-compatible wire. The allowlist is a list of ANTHROPIC model names + (``opus-5``, ``opus-4-8``, …) checked by substring, so it matches nothing + on that wire and would downgrade every ``xhigh`` to ``high`` — silently + ignoring what the user asked for. ``xhigh`` is a first-class OpenAI level + (developers.openai.com/api/docs/guides/reasoning lists none | minimal | + low | medium | high | xhigh | max, and recommends xhigh precisely for + "agentic tasks that require long runs"); verified accepted by + ``openai/gpt-5.6-luna`` on 2026-07-31. Providers that don't know the + field ignore it, so passing it through is the safe direction there — + whereas on the Anthropic wire an unsupported ``xhigh`` is a hard 400, + which is why the clamp stays on by default. """ value = (explicit or "").strip().lower() if value not in VALID_THINKING_EFFORT_LEVELS: @@ -584,7 +599,7 @@ def resolve_thinking_effort(explicit: str | None, model: str | None) -> str | No value = "" if value not in VALID_THINKING_EFFORT_LEVELS: return None - if value == "xhigh" and not _model_supports_xhigh_effort(model): + if value == "xhigh" and clamp_xhigh and not _model_supports_xhigh_effort(model): logger.debug( "effort xhigh not supported on %s; sending high instead", model ) @@ -1156,6 +1171,56 @@ async def _call_model_sync( # resolve_thinking_effort). if resolved_effort is not None: call_kwargs["output_config"] = {"effort": resolved_effort} + elif not is_anthropic: + # NON-Anthropic wire: reasoning effort is a top-level + # ``reasoning_effort`` body field, NOT ``output_config``. This is the + # ONLY site that applies effort for this family — the interactive path + # hands the level down as ``thinking_effort`` and does not wrap the + # provider (see ``AgentSession._turn_effort_routing``, which used to + # wrap it in an ``_EffortProvider`` and collided with this branch). + # One level, one injection site: two of them silently inverted the + # documented precedence, with ``settings.effort`` beating an explicit + # session ``/effort``. + # + # Before this branch existed, effort was emitted only on the Anthropic + # side, so ``--effort`` on the headless ``-p`` path (the one the + # terminal-bench harness drives) was a SILENT no-op for every + # OpenAI-compatible provider. Verified 2026-07-31 against a capture + # server: ``--effort max --provider openrouter`` produced a body of + # {messages, model, stream, stream_options, tools}, no effort field. + # + # Not gated on ``_model_supports_effort``: that allowlist is a list of + # Anthropic model names for the ``output_config`` parameter and matches + # nothing here. Gating on it would reintroduce the silent drop. + # + # SCOPE — this is ``not is_anthropic``, which is broader than + # "OpenAI-compatible": Gemini lands here too, and its provider picks + # named kwargs out of ``**kwargs`` rather than forwarding + # ``extra_body``, so effort is still dropped there. Harmless (no + # 400), but it means Gemini keeps the silent-no-op bug this branch + # exists to kill; fixing it needs Gemini's own generation-config + # shape, not this field. + # + # Every real OpenAI-compatible provider does forward it: the base + # ``chat``/``chat_stream``/``_stream_attempt`` splat leftover kwargs + # into ``client.chat.completions.create``, which handles ``extra_body`` + # natively, and openrouter/zai/deepseek add no overrides. The + # ChatGPT-subscription path reads it back out of ``extra_body`` + # instead (openai_provider ``_subscription_reasoning_effort``) rather + # than forwarding it into a Responses body that would reject it. + # Providers that simply don't know the field ignore it (probed + # 2026-07-31: deepseek-v4-pro and glm-5.2 both accept it without error). + # + # ``setdefault`` so an explicit caller-supplied extra_body wins. + resolved_effort = resolve_thinking_effort( + thinking_effort, + getattr(provider, "model", None) or call_kwargs.get("model"), + clamp_xhigh=False, + ) + if resolved_effort is not None: + extra_body = dict(call_kwargs.get("extra_body") or {}) + extra_body.setdefault("reasoning_effort", resolved_effort) + call_kwargs["extra_body"] = extra_body # TS callModel() uses SSE streaming for faster first-byte latency and # progressive text display. Use chat_stream_response() which streams diff --git a/src/server/agent_server.py b/src/server/agent_server.py index b7b7565c..5e7314fe 100644 --- a/src/server/agent_server.py +++ b/src/server/agent_server.py @@ -197,10 +197,11 @@ class _AgentSession: init_error: str | None = None _session_name: str | None = None # user-set label (/rename) shown in /resume _mcp_runtime: Any = None # McpRuntime (connected MCP servers) when configured - # /effort reasoning level. Routed TWO different ways by provider family - # (see _turn_effort_routing): Anthropic takes ``output_config.effort`` - # via the query loop's ``thinking_effort``; OpenAI-compatible providers - # take ``reasoning_effort`` in the request body via _EffortProvider. + # /effort reasoning level. Carried to the query loop as + # ``thinking_effort`` (see _turn_effort_routing) and turned into the + # parameter each provider family accepts at the wire boundary: + # ``output_config.effort`` on the Anthropic wire, a top-level + # ``reasoning_effort`` body field on the OpenAI-compatible one. _effort: str | None = None _knowledge: Any = None # KnowledgeGraph (lazy-loaded), populated at each turn end _knowledge_enabled: bool = True # the original's knowledgeGraphEnabled (default on) @@ -2299,40 +2300,32 @@ def _turn_effort_routing(self) -> tuple[Any, str | None]: """Return ``(provider_for_this_turn, thinking_effort)`` for ``/effort``. The two provider families take reasoning effort as DIFFERENT wire - parameters, and sending one family's shape to the other is a hard - 400 — so the level has to be routed, not injected uniformly: - - * **Anthropic** (incl. Minimax, which speaks the Anthropic shape): - ``output_config={"effort": …}``. Returned as ``thinking_effort`` - so ``resolve_thinking_effort`` applies it at the wire boundary - with the per-model gating (unsupported ``xhigh`` clamps to high). - * **OpenAI-compatible**: ``reasoning_effort`` as a top-level body - field, which is what :class:`_EffortProvider` injects. - - Before this split, every provider got the ``extra_body`` injection. - On the Anthropic wire that is rejected — probed 2026-07-25 against - claude-opus-5: ``400 invalid_request_error — reasoning_effort: - Extra inputs are not permitted`` — so a ``/effort`` in an - interactive Anthropic session broke every following request in that - session, while the headless ``--effort`` path (which always went - through ``thinking_effort``) worked. - - ``is_anthropic_wire`` is the shared predicate (``src/providers``), - the same one ``query.py`` uses to decide whether ``output_config`` - is emitted at all — they have to agree or this bug comes back. - Deliberately NOT wrapped in a try/except: the only failure mode - would be an unimportable provider module, in which case the - provider could not be an instance of it anyway, and falling back to - the ``_EffortProvider`` branch on an Anthropic session would pick - the guaranteed-400 path over a merely-omitted effort. + parameters — ``output_config.effort`` on the Anthropic wire (incl. + Minimax, which speaks the Anthropic shape), a top-level + ``reasoning_effort`` body field on the OpenAI-compatible one — and + sending one family's shape to the other is a hard 400. Probed + 2026-07-25 against claude-opus-5: ``400 invalid_request_error — + reasoning_effort: Extra inputs are not permitted``. + + That routing now lives ENTIRELY at the wire boundary in + ``query.py::_call_model_sync``, which branches on the same + ``is_anthropic_wire`` predicate. So this method just hands the level + over as ``thinking_effort`` and does not wrap the provider. + + It used to wrap OpenAI-compatible providers in an ``_EffortProvider`` + that injected ``extra_body.reasoning_effort`` itself, because + ``query.py`` emitted effort only on its Anthropic branch. When + query.py learned the OpenAI-compatible half, the two injection sites + collided: routing returned ``thinking_effort=None`` for this family, + so ``resolve_thinking_effort`` fell through to ``settings.effort`` + and filled ``extra_body`` first, and the wrapper's ``setdefault`` + then found the key taken. The session's ``/effort`` was silently + discarded in favour of the persisted setting — an inversion of the + documented precedence (explicit beats persisted), reproducible as + ``/effort max`` + ``settings.effort medium`` putting ``medium`` on + the wire. One level, one injection site, no drift. """ - if not self._effort: - return self.provider, None - from src.providers import is_anthropic_wire - - if is_anthropic_wire(self.provider): - return self.provider, self._effort - return _EffortProvider(self.provider, self._effort), None + return self.provider, (self._effort or None) def _do_set_effort(self, request_id: object, effort: object) -> None: """``/effort`` backend: reasoning levels plus the ``ultracode`` @@ -4600,10 +4593,11 @@ def on_message(message: Any) -> None: on_message=on_message, abort_controller=abort, extended_thinking=self._thinking, # None = model default; True/False = ThinkingToggle - # /effort on the Anthropic path: resolved at the wire - # boundary into ``output_config.effort`` with the per-model - # gating (xhigh clamps to high where unsupported). None for - # OpenAI-compat providers, which got _EffortProvider instead. + # /effort for BOTH provider families: resolved at the wire + # boundary into ``output_config.effort`` (Anthropic, with the + # per-model gating — xhigh clamps to high where unsupported) + # or a top-level ``reasoning_effort`` body field + # (OpenAI-compatible, no clamp). thinking_effort=turn_thinking_effort, fallback_model=self.config.fallback_model, pipeline_config=pipeline_config, @@ -4991,7 +4985,7 @@ def _build_runtime(sess: _AgentSession, perm_mode: str | None) -> None: # treats it as "nothing requested" and silently substitutes # settings.effort, while the init frame's badge displays the value # the user asked for. Unnormalized case has the same shape on the - # OpenAI-compat side, where _EffortProvider injects it verbatim. + # OpenAI-compat side, which sends the level verbatim. # ``isinstance`` rather than ``or ""``: a non-str effort from a # programmatic caller would raise inside this try block, and # _build_runtime converts any raise into init_error — killing the @@ -5698,55 +5692,6 @@ def _fmt_rule(rule: Any) -> str: return f"{tool}({content})" if content else tool -class _EffortProvider: - """Wraps a provider to inject ``reasoning_effort`` via ``extra_body`` on chat - calls (the original's /effort). Used only when /effort is set; delegates - everything else to the inner provider, so the default path is untouched. - - OpenAI-compatible providers ONLY — ``reasoning_effort`` is their body - field. The Anthropic wire rejects it (``400 … Extra inputs are not - permitted``, probed 2026-07-25) and takes ``output_config.effort`` - instead, so :meth:`AgentSession._turn_effort_routing` sends Anthropic - sessions down the ``thinking_effort`` path and never wraps them here. - """ - - def __init__(self, inner: Any, effort: str) -> None: - self._inner = inner - self._effort = effort - - def __getattr__(self, name: str) -> Any: # model, get_available_models, … - # Guard the delegate itself: without this, an instance created - # WITHOUT __init__ (copy.copy / copy.deepcopy build one that way, - # then probe for __setstate__/__deepcopy__) recurses forever — - # __getattr__ looks up self._inner, which is missing, which calls - # __getattr__ … until RecursionError. Two live sites copy the - # session provider (src/agent/run_agent.py's per-subagent model - # override and src/permissions/yolo_classifier.py), and both - # swallow Exception — which RecursionError is — so the failure was - # silent. - if name == "_inner": - raise AttributeError(name) - return getattr(self._inner, name) - - def _inject(self, kwargs: dict) -> dict: - eb = dict(kwargs.get("extra_body") or {}) - eb.setdefault("reasoning_effort", self._effort) - kwargs["extra_body"] = eb - return kwargs - - def chat_stream_response(self, *args: Any, **kwargs: Any) -> Any: - return self._inner.chat_stream_response(*args, **self._inject(kwargs)) - - def chat(self, *args: Any, **kwargs: Any) -> Any: - return self._inner.chat(*args, **self._inject(kwargs)) - - def chat_stream(self, *args: Any, **kwargs: Any) -> Any: - return self._inner.chat_stream(*args, **self._inject(kwargs)) - - async def chat_async(self, *args: Any, **kwargs: Any) -> Any: - return await self._inner.chat_async(*args, **self._inject(kwargs)) - - def _sessions_dir() -> Path: # Honors $CLAWCODEX_CONFIG_DIR (default ~/.clawcodex/sessions). from src.utils.clawcodex_dirs import get_sessions_dir diff --git a/src/services/pricing.py b/src/services/pricing.py index 7cb9ab0b..6a96ae7e 100644 --- a/src/services/pricing.py +++ b/src/services/pricing.py @@ -135,6 +135,35 @@ "cache_creation": 1.25 / 1_000_000, "cache_read": 0.15 / 1_000_000, } +# OpenAI GPT-5.6 Luna / Luna Pro, as proxied by OpenRouter (both variants +# publish identical rates). Read off OpenRouter's /models pricing record +# 2026-07-31: prompt $0.10/M, completion $0.60/M, input_cache_write +# $0.125/M, input_cache_read $0.01/M. +# +# That record also carries a long-context override: above 272,000 prompt +# tokens every rate roughly doubles. Implemented (not merely documented) +# because this is a 1.05M-window model whose reason for being registered is a +# benchmark that will cross 272K on long tasks, and whose cost gets compared +# against other agents' — under-reporting the expensive half of a run by ~2x +# would corrupt exactly the number the eval exists to produce. +# +# Cache rates remain inert in practice: the generic OpenAI-compat usage +# builder does not map ``prompt_tokens_details.cached_tokens`` onto +# ``cache_read_input_tokens`` (only the hand-written DeepSeek provider does), +# so cached input bills at the full input rate. Recorded for when that lands. +_GPT_56_LUNA_INPUT_TIER_LIMIT = 272_000 +_TIER_GPT_56_LUNA = { + "input": 0.10 / 1_000_000, + "output": 0.60 / 1_000_000, + "cache_creation": 0.125 / 1_000_000, + "cache_read": 0.01 / 1_000_000, +} +_TIER_GPT_56_LUNA_LONG = { + "input": 0.20 / 1_000_000, + "output": 0.90 / 1_000_000, + "cache_creation": 0.25 / 1_000_000, + "cache_read": 0.02 / 1_000_000, +} # Exact-match table — keyed by canonical model name. Order DOESN'T matter @@ -171,6 +200,15 @@ "MiniMax-M2.7": _TIER_MINIMAX_M27, # Meta Muse Spark (api.meta.ai) "muse-spark-1.1": _TIER_MUSE_SPARK, + # OpenAI GPT-5.6 Luna. Reached from OpenRouter's ``openai/gpt-5.6-luna`` + # via get_pricing's vendor-prefix strip, same as the DeepSeek rows. + # VALUES UNUSED — these two rows act only as membership gates for + # ``get_pricing``'s ``model in PRICING`` checks; the live rates are + # picked by prompt size in ``_get_exact_pricing``, which returns before + # reaching ``PRICING.get(model)``. Same shape as the MiniMax-M3 row. + # Editing the dicts below changes nothing; edit the tiers instead. + "gpt-5.6-luna": _TIER_GPT_56_LUNA, + "gpt-5.6-luna-pro": _TIER_GPT_56_LUNA, } @@ -211,6 +249,15 @@ def _get_exact_pricing( input_tokens: int, service_tier: str, ) -> dict[str, float] | None: + # Context-tiered models: the published rate depends on how big THIS + # request's prompt is. ``model`` is already the canonical bare key here + # (get_pricing strips any ``/`` prefix before calling). + if model in ("gpt-5.6-luna", "gpt-5.6-luna-pro"): + return ( + _TIER_GPT_56_LUNA_LONG + if input_tokens > _GPT_56_LUNA_INPUT_TIER_LIMIT + else _TIER_GPT_56_LUNA + ) if model != "MiniMax-M3": return PRICING.get(model) diff --git a/tests/server/test_agent_server_workflows.py b/tests/server/test_agent_server_workflows.py index d831f453..166001e8 100644 --- a/tests/server/test_agent_server_workflows.py +++ b/tests/server/test_agent_server_workflows.py @@ -196,12 +196,26 @@ async def test_effort_routing_matches_the_provider_wire_shape(tmp_path): Sending the OpenAI shape to Anthropic is a hard 400 (probed 2026-07-25: ``reasoning_effort: Extra inputs are not permitted``), which used to break every request after a ``/effort`` in an interactive Anthropic - session. Pin both directions of the split. + session. + + That split now lives ENTIRELY at the wire boundary in + ``query.py::_call_model_sync`` (covered by + tests/test_query_openai_compat_effort.py, which asserts the actual kwargs + each family receives). Routing's own job shrank to "hand the level over, + unwrapped, for both families" — so that is what this pins. + + It used to wrap OpenAI-compat providers in an ``_EffortProvider`` and + return ``thinking_effort=None``. Once query.py learned to emit + ``reasoning_effort`` itself, the two injection sites collided: query.py + filled ``extra_body`` from ``settings.effort`` first and the wrapper's + ``setdefault`` no-op'd, so an explicit ``/effort`` was silently discarded + in favour of the persisted setting. The wrapper was deleted; asserting the + provider comes back UNWRAPPED is what keeps a second injection site from + reappearing. """ from unittest.mock import MagicMock from src.providers.anthropic_provider import AnthropicProvider - from src.server.agent_server import _EffortProvider async with _spawned(tmp_path, _TextProvider) as (handle, gen): sess = _session_of(handle) @@ -210,24 +224,20 @@ async def test_effort_routing_matches_the_provider_wire_shape(tmp_path): sess._effort = None assert sess._turn_effort_routing() == (sess.provider, None) - # Anthropic → the real provider plus output_config.effort. The - # provider must NOT be wrapped: wrapping is what injected the - # rejected body field. + # Anthropic → the real provider plus the level; query.py turns it + # into output_config.effort. sess.provider = AnthropicProvider(api_key="sk-test", model="claude-opus-5") sess._effort = "xhigh" provider, thinking_effort = sess._turn_effort_routing() assert provider is sess.provider - assert not isinstance(provider, _EffortProvider) assert thinking_effort == "xhigh" - # OpenAI-compatible → wrapped, and effort stays out of the query - # loop's Anthropic-only parameter. + # OpenAI-compatible → same shape. The provider must NOT be wrapped: + # query.py is the single injection site for reasoning_effort. sess.provider = MagicMock(name="openai-compat") provider, thinking_effort = sess._turn_effort_routing() - assert isinstance(provider, _EffortProvider) - assert thinking_effort is None - injected = provider._inject({}) - assert injected["extra_body"]["reasoning_effort"] == "xhigh" + assert provider is sess.provider + assert thinking_effort == "xhigh" async def test_effort_reaches_the_query_loop_kwarg(tmp_path, monkeypatch): @@ -294,13 +304,12 @@ async def test_launch_effort_flag_seeds_the_session(tmp_path): async with _spawned(tmp_path, _TextProvider, config) as (handle, gen): sess = _session_of(handle) assert sess._effort == "xhigh" - # _TextProvider is not Anthropic-shaped, so the level routes down - # the OpenAI-compat branch — the point here is only that the launch - # flag SEEDED a level at all. Per-family routing is pinned by + # The point here is only that the launch flag SEEDED a level at all; + # routing hands it over unwrapped for either family, and the + # per-family wire shape is pinned by # test_effort_routing_matches_the_provider_wire_shape. provider, thinking_effort = sess._turn_effort_routing() - assert provider is not sess.provider and thinking_effort is None - assert provider._inject({})["extra_body"]["reasoning_effort"] == "xhigh" + assert provider is sess.provider and thinking_effort == "xhigh" # A later /effort still wins over the launch flag, and auto clears. r = await _control(handle, gen, "e1", {"subtype": "set_effort", "effort": "low"}) @@ -333,8 +342,9 @@ async def test_launch_effort_flag_ignores_off_ladder_values(tmp_path, seed): async def test_launch_effort_flag_is_normalized(tmp_path): """Case is normalized at the seed, matching /effort's ``.lower()``. - ``_EffortProvider`` injects the level verbatim into the request body, so - an unnormalized "MAX" would go out on the OpenAI-compat wire as-is. + The OpenAI-compat wire sends the level verbatim as ``reasoning_effort`` + (query.py injects it at the wire boundary), so an unnormalized "MAX" + would go out as-is. """ async with _spawned(tmp_path, _TextProvider, AgentServerConfig(effort=" MAX ")) as ( handle, diff --git a/tests/test_query_openai_compat_effort.py b/tests/test_query_openai_compat_effort.py new file mode 100644 index 00000000..d327f800 --- /dev/null +++ b/tests/test_query_openai_compat_effort.py @@ -0,0 +1,404 @@ +"""Reasoning-effort wiring for OpenAI-compatible providers, plus the +OpenRouter-qualified model-config lookup that GPT-5.6 Luna needs. + +Background — the bugs these lock down. + +1. The two provider families take reasoning effort as different wire + parameters (``output_config.effort`` on the Anthropic wire, a top-level + ``reasoning_effort`` body field on the OpenAI-compatible one). + ``_call_model_sync`` emitted it only inside its ``is_anthropic`` branch, + so on the headless ``-p`` path — the one the terminal-bench harness + drives — ``--effort`` was a SILENT no-op for every OpenAI-compatible + provider. Captured against a local capture server on 2026-07-31, + ``--effort max --provider openrouter`` produced a request body of + ``{messages, model, stream, stream_options, tools}``: no effort field of + any kind, no error, no log line. + +2. Fixing (1) then collided with ``_AgentSession._turn_effort_routing``, + which wrapped OpenAI-compatible providers in an ``_EffortProvider`` that + injected the same field with ``setdefault``. Routing passed + ``thinking_effort=None`` for that family, so query.py filled the key from + ``settings.effort`` first and the wrapper's ``setdefault`` no-op'd: an + explicit session ``/effort max`` went out as ``medium``, inverting the + documented precedence. The wrapper is now deleted — one level, one + injection site. + +3. ``MODEL_CONFIGS`` is keyed by BARE model name, so ``openai/gpt-5.6-luna`` + — the id that actually reaches the provider on the OpenRouter path — + matched nothing and fell back to ``DEFAULT_CONTEXT_WINDOW`` (200K) + against a real 1M-class window, i.e. auto-compact at a fifth of capacity. + Fixed with one explicit vendor-qualified row, NOT by teaching the resolver + to strip ``/`` (that is "decision #1" and stays out of scope). +""" + +from __future__ import annotations + +import asyncio +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest import mock +from unittest.mock import MagicMock + +from src.models.configs import MODEL_CONFIGS, get_model_config +from src.models.context import ( + DEFAULT_CONTEXT_WINDOW, + get_context_window_for_model, + get_model_max_output_tokens, +) +from src.providers.anthropic_provider import AnthropicProvider +from src.providers.base import ChatResponse +from src.providers.openrouter_provider import OpenRouterProvider +from src.query.query import QueryParams, query +from src.services.pricing import compute_cost, get_pricing +from src.tool_system.context import ToolContext +from src.tool_system.defaults import build_default_registry +from src.types.messages import UserMessage +from src.utils.abort_controller import AbortController + +LUNA = "openai/gpt-5.6-luna" + + +def _run(coro): + return asyncio.run(coro) + + +def _no_settings_effort(): + """Pin ``settings.effort`` empty so a developer's own configured effort + can't mask an assertion that the parameter is omitted.""" + return mock.patch( + "src.settings.settings.get_settings", + return_value=SimpleNamespace(effort=""), + ) + + +def _make_openrouter_mock(model: str = LUNA) -> MagicMock: + """A mock that fails ``is_anthropic_wire`` — i.e. takes the + OpenAI-compatible branch. Streaming is forced into the ``chat()`` + fallback so assertions can read kwargs off ``chat.call_args``.""" + provider = MagicMock(spec=OpenRouterProvider) + provider.model = model + provider.base_url = "https://openrouter.ai/api/v1" + provider.chat_stream_response.side_effect = NotImplementedError() + provider.chat.return_value = ChatResponse( + content="ok", + model=model, + usage={"input_tokens": 1, "output_tokens": 1}, + finish_reason="stop", + tool_uses=None, + ) + return provider + + +def _make_anthropic_mock(model: str) -> MagicMock: + provider = MagicMock(spec=AnthropicProvider) + provider.model = model + provider.chat_stream_response.side_effect = NotImplementedError() + provider.chat.return_value = ChatResponse( + content="ok", + model=model, + usage={"input_tokens": 1, "output_tokens": 1}, + finish_reason="end_turn", + tool_uses=None, + ) + return provider + + +class TestOpenAICompatEffortOnTheWire(unittest.TestCase): + """Drive one real turn and inspect what the provider actually received.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.registry = build_default_registry() + self.context = ToolContext(workspace_root=Path(self.tmp.name)) + self.abort = AbortController() + + def tearDown(self): + self.tmp.cleanup() + + def _drive_one_turn(self, provider: MagicMock, **extra) -> dict: + params = QueryParams( + messages=[UserMessage(content="hi")], + system_prompt="hello", + tools=self.registry.list_tools(), + tool_registry=self.registry, + tool_use_context=self.context, + provider=provider, + abort_controller=self.abort, + max_turns=1, + **extra, + ) + + async def run(): + async for _ in query(params): + pass + + _run(run()) + self.assertTrue(provider.chat.called, "provider.chat() should have run") + return provider.chat.call_args.kwargs + + def test_effort_max_reaches_the_wire(self): + """The reported bug: --effort max must actually ship.""" + with _no_settings_effort(): + kw = self._drive_one_turn( + _make_openrouter_mock(), thinking_effort="max" + ) + self.assertEqual( + (kw.get("extra_body") or {}).get("reasoning_effort"), + "max", + "reasoning_effort must be on the OpenAI-compatible body", + ) + + def test_each_level_ships_verbatim(self): + for level in ("low", "medium", "high", "max"): + with self.subTest(level=level), _no_settings_effort(): + kw = self._drive_one_turn( + _make_openrouter_mock(), thinking_effort=level + ) + self.assertEqual( + (kw.get("extra_body") or {}).get("reasoning_effort"), level + ) + + def test_xhigh_is_not_clamped_on_this_wire(self): + """``_model_supports_xhigh_effort`` is an allowlist of ANTHROPIC model + names matched by substring, so it matches nothing here and would + downgrade every xhigh to high. xhigh is a first-class OpenAI level + (and the one their guide recommends for long agentic runs).""" + with _no_settings_effort(): + kw = self._drive_one_turn( + _make_openrouter_mock(), thinking_effort="xhigh" + ) + self.assertEqual( + (kw.get("extra_body") or {}).get("reasoning_effort"), "xhigh" + ) + + def test_xhigh_still_clamped_on_the_anthropic_wire(self): + """The clamp must stay on where an unsupported xhigh is a hard 400.""" + with _no_settings_effort(): + kw = self._drive_one_turn( + _make_anthropic_mock("claude-sonnet-4-6"), thinking_effort="xhigh" + ) + self.assertEqual(kw.get("output_config"), {"effort": "high"}) + + def test_output_config_never_sent_on_this_wire(self): + """``output_config`` is the Anthropic shape; sending it here is the + mirror-image of the 400 that motivated the interactive split.""" + with _no_settings_effort(): + kw = self._drive_one_turn( + _make_openrouter_mock(), thinking_effort="max" + ) + self.assertNotIn("output_config", kw) + self.assertNotIn("thinking", kw) + + def test_absent_when_no_effort_requested(self): + """The default path must be byte-identical to before the fix.""" + with _no_settings_effort(): + kw = self._drive_one_turn(_make_openrouter_mock()) + self.assertNotIn("reasoning_effort", kw.get("extra_body") or {}) + + def test_settings_effort_is_honored(self): + """No flag, but a persisted ``settings.effort`` — the same source the + harness seeds into a container so subagents inherit the level.""" + with mock.patch( + "src.settings.settings.get_settings", + return_value=SimpleNamespace(effort="max"), + ): + kw = self._drive_one_turn(_make_openrouter_mock()) + self.assertEqual( + (kw.get("extra_body") or {}).get("reasoning_effort"), "max" + ) + + def test_anthropic_still_uses_output_config(self): + """The other half of the routing split must not regress: Anthropic + keeps ``output_config`` and must never grow ``reasoning_effort``, + which that wire rejects with a hard 400.""" + with _no_settings_effort(): + kw = self._drive_one_turn( + _make_anthropic_mock("claude-opus-4-8"), thinking_effort="max" + ) + self.assertEqual(kw.get("output_config"), {"effort": "max"}) + self.assertNotIn("reasoning_effort", kw.get("extra_body") or {}) + + +class TestLunaOpenRouterIdRegistration(unittest.TestCase): + """The bare ``gpt-5.6-*`` family is registered elsewhere (#773). What is + pinned here is only the OpenRouter-qualified id, which is what actually + reaches the provider when the terminal-bench harness runs this model.""" + + def test_openrouter_id_resolves_to_the_real_window(self): + self.assertEqual(get_context_window_for_model(LUNA), 1_048_576) + self.assertEqual(get_model_max_output_tokens(LUNA), 128_000) + + def test_not_the_200k_default(self): + """The specific failure mode: silently sized at 200K, auto-compacting + at a fifth of the model's real capacity.""" + self.assertNotEqual( + get_context_window_for_model(LUNA), DEFAULT_CONTEXT_WINDOW + ) + + def test_qualified_and_bare_agree(self): + """A model must not have two different windows depending on which + gateway routed it.""" + self.assertEqual( + get_context_window_for_model(LUNA), + get_context_window_for_model("gpt-5.6-luna"), + ) + + def test_pro_variant_resolves_through_the_prefix_fallback(self): + for model_id in ("openai/gpt-5.6-luna-pro", "gpt-5.6-luna-pro"): + with self.subTest(model=model_id): + self.assertEqual( + get_context_window_for_model(model_id), 1_048_576 + ) + + def test_decision_1_upheld_no_vendor_prefix_stripping(self): + """``get_model_config`` must NOT strip a ``/`` prefix. Pinned + by tests/test_deepseek_prefix_cache.py as "decision #1"; duplicated + here because adding a vendor-qualified row is exactly the change that + tempts someone to generalize it into a resolver tier.""" + self.assertEqual( + get_context_window_for_model("deepseek/deepseek-v4-pro"), 200_000 + ) + self.assertIsNone(get_model_config("anthropic/claude-sonnet-4.5")) + self.assertIsNone(get_model_config("openai/gpt-4o")) + + def test_the_new_row_does_not_perturb_other_ids(self): + """The added key claims prefix "openai/gpt-5.6". Nothing else may + start resolving through it. Hardcoded expectations rather than a + reimplementation of the resolver, so this cannot degenerate into a + tautology.""" + expected = { + # Inside the claimed prefix — these DO now resolve, to the same + # window their bare equivalents get from #773's rows. Pinned so + # the size of the claim is a fact rather than a comment. + "openai/gpt-5.6-mini": 1_048_576, + "openai/gpt-5.6-sol": 1_048_576, + # Outside it — must stay unresolved. + "openai/gpt-4o": None, + "openai/gpt-5.5": None, + "openai/gpt-5.4-mini": None, + "openai/o1": None, + "anthropic/claude-opus-4-8": None, + "deepseek/deepseek-v4-flash": None, + # Bare ids — untouched by the qualified row. + "gpt-5.5": 272_000, + "gpt-4o": 128_000, + "claude-opus-4-8": 1_000_000, + "some-unknown-model": None, + } + for model_id, window in expected.items(): + with self.subTest(model=model_id): + config = get_model_config(model_id) + if window is None: + self.assertIsNone(config, f"{model_id} should not resolve") + else: + self.assertEqual(config.context_window, window) + + def test_user_model_limits_override_still_reachable(self): + """``_settings_limit`` is consulted only when get_model_config returns + None, so every row added here shadows a user's explicit ``modelLimits``. + + The shadowed surface is the row's PREFIX, not just its key: the added + row's derived base is ``openai/gpt-5.6``, so a ``modelLimits`` entry + for any ``openai/gpt-5.6*`` id now loses to the table (they all get + 1,048,576, matching what their bare equivalents already get — so the + qualified namespace mirrors the bare one rather than diverging). + What must NOT happen is that claim spreading further, which is what + this pins: an override outside the prefix still wins.""" + limits = { + "openai/gpt-oss-120b": SimpleNamespace( + context_window=131_072, max_output_tokens=None + ) + } + with mock.patch( + "src.settings.settings.get_settings", + return_value=SimpleNamespace(model_limits=limits, effort=""), + ): + self.assertEqual( + get_context_window_for_model("openai/gpt-oss-120b"), 131_072 + ) + + def test_pricing_resolves_through_the_vendor_prefix(self): + """``get_pricing`` DOES strip the vendor prefix (its own documented + tier 2), so one bare pricing key covers the OpenRouter id.""" + pricing = get_pricing(LUNA) + self.assertIsNotNone(pricing, "luna must not be priced as unknown") + self.assertAlmostEqual(pricing["input"] * 1_000_000, 0.10, places=6) + self.assertAlmostEqual(pricing["output"] * 1_000_000, 0.60, places=6) + + def test_long_context_pricing_tier(self): + """Above 272K prompt tokens OpenRouter roughly doubles every rate. A + 1M-window model on a benchmark will cross that, and cost is a number + the eval reports.""" + short = get_pricing(LUNA, input_tokens=100_000) + long = get_pricing(LUNA, input_tokens=300_000) + self.assertAlmostEqual(short["input"] * 1_000_000, 0.10, places=6) + self.assertAlmostEqual(long["input"] * 1_000_000, 0.20, places=6) + self.assertAlmostEqual(long["output"] * 1_000_000, 0.90, places=6) + + def test_cost_uses_the_long_tier_for_a_big_prompt(self): + """End-to-end through compute_cost, which is what the eval reports.""" + cheap = compute_cost(LUNA, {"input_tokens": 100_000, "output_tokens": 1_000}) + pricey = compute_cost(LUNA, {"input_tokens": 300_000, "output_tokens": 1_000}) + self.assertAlmostEqual(cheap, 100_000 * 1e-7 + 1_000 * 6e-7, places=9) + self.assertAlmostEqual(pricey, 300_000 * 2e-7 + 1_000 * 9e-7, places=9) + + +class TestEffortRoutingHasOneInjectionSite(unittest.TestCase): + """``_EffortProvider`` used to inject ``reasoning_effort`` itself. With + query.py doing it too, the wrapper's ``setdefault`` found the key already + filled from ``settings.effort`` and the session's ``/effort`` was silently + dropped — inverting the documented precedence. The wrapper is gone; this + pins that it stays gone and that routing just forwards the level.""" + + def test_effort_provider_class_is_removed(self): + import src.server.agent_server as agent_server + + self.assertFalse( + hasattr(agent_server, "_EffortProvider"), + "a second injection site reintroduces the precedence inversion", + ) + + def test_routing_forwards_the_level_without_wrapping(self): + from src.server.agent_server import _AgentSession + + session = object.__new__(_AgentSession) + provider = _make_openrouter_mock() + session.provider = provider + session._effort = "max" + turn_provider, thinking_effort = _AgentSession._turn_effort_routing(session) + self.assertIs(turn_provider, provider, "provider must not be wrapped") + self.assertEqual(thinking_effort, "max") + + def test_routing_passes_none_when_effort_unset(self): + from src.server.agent_server import _AgentSession + + session = object.__new__(_AgentSession) + session.provider = _make_openrouter_mock() + session._effort = None + self.assertIsNone(_AgentSession._turn_effort_routing(session)[1]) + + def test_explicit_effort_beats_persisted_setting(self): + """The precedence the double injection inverted, end to end.""" + with mock.patch( + "src.settings.settings.get_settings", + return_value=SimpleNamespace(effort="medium"), + ): + driver = TestOpenAICompatEffortOnTheWire("run") + driver.setUp() + try: + kw = driver._drive_one_turn( + _make_openrouter_mock(), thinking_effort="max" + ) + finally: + driver.tearDown() + self.assertEqual( + (kw.get("extra_body") or {}).get("reasoning_effort"), + "max", + "session /effort must win over settings.effort", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_r5_final_verdict_polish.py b/tests/test_r5_final_verdict_polish.py index 6da16ae2..bb30e969 100644 --- a/tests/test_r5_final_verdict_polish.py +++ b/tests/test_r5_final_verdict_polish.py @@ -17,16 +17,38 @@ from unittest.mock import MagicMock, patch +class _DecoratedProvider: + """Stand-in for a provider decorator — anything exposing ``_inner``. + + This used to be the real ``_EffortProvider`` from agent_server, which + wrapped OpenAI-compatible providers to inject ``reasoning_effort``. That + class was deleted once reasoning effort moved to the wire boundary in + query.py (two injection sites silently inverted /effort vs settings.effort + precedence). The UNWRAP it motivated is still live in + ``agent_loop_compat._maybe_recall_memories`` as a general guard, so the + behaviour is still worth pinning — a future decorator with the same shape + must not hide an AnthropicProvider from the recall cost-pin. Hence a local + minimal wrapper rather than deleting these tests with the class. + """ + + def __init__(self, inner): + self._inner = inner + + def __getattr__(self, name): + if name == "_inner": + raise AttributeError(name) + return getattr(self._inner, name) + + class TestEffortProviderUnwrapForRecall(unittest.TestCase): - """N1 — the /effort wrapper no longer hides the AnthropicProvider from the - recall cost-pin.""" + """N1 — a provider decorator no longer hides the AnthropicProvider from + the recall cost-pin.""" def test_wrapped_bypasses_pin_unwrapped_restores_it(self): from src.memdir.find_relevant_memories import _resolve_recall_model from src.providers.anthropic_provider import AnthropicProvider - from src.server.agent_server import _EffortProvider - wrapped = _EffortProvider(AnthropicProvider(api_key="k"), "high") + wrapped = _DecoratedProvider(AnthropicProvider(api_key="k")) settings = MagicMock() settings.small_fast_model = "claude-3-5-haiku-20241022" @@ -43,10 +65,9 @@ def test_maybe_recall_unwraps_effort_provider(self): # provider that reaches the recall — it must be the UNWRAPPED inner. from src.providers.anthropic_provider import AnthropicProvider from src.query import agent_loop_compat as alc - from src.server.agent_server import _EffortProvider inner = AnthropicProvider(api_key="k") - wrapped = _EffortProvider(inner, "high") + wrapped = _DecoratedProvider(inner) captured = {} async def _fake_reminder(query, memdir, *, provider, already_surfaced,