From b934c46714c1169f1c1d67ca2095293bc3ea846e Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Thu, 24 Sep 2026 22:54:49 +0000 Subject: [PATCH] Enforce the tool-call contract, asking servers only where it works The executor ended a milestone on any reply without a tool call, and nothing held a model to the offered tools. Measured on the 4070: tool_choice "required" is enforced by vLLM (local-fast) and Anthropic, but qwen3-14b (local-coder) under it cannot stop and writes until the output ceiling, a known model bug. So enforcement is asked for per model, and the loop holds the contract itself everywhere. - The catalog reads supports_tool_choice from LiteLLM's model_info; BudgetedClient sends tool_choice only to models declared true, as "required" on the normalized route and {"type": "any"} on the Anthropic passthrough, and only when tools are offered - The executor gains an explicit finish tool, the only way to end a milestone. Finishing with nothing changed is refused once; a prose reply gets one nudge, then escalates; a call to a tool not offered counts toward failure_limit, like a failed command; calls after finish are recorded as not run - Clients record the model that actually answered. ModelFacts.answered recognises a proxy fallback, which once credited Claude's answers to a local model that could not call tools at all - The context probe reports a fallback instead of trusting it; rerun, both local models now refuse over-long prompts, as they should - The capability matrix checks every answer came from the model asked, that models call only offered tools, and that a declared supports_tool_choice is true - Spec A15 and the tuning guide say how enforcement is requested, declared and backed up Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/Architecture-Build-Spec.md | 8 +- docs/Tuning-Guide.md | 61 +++++++++-- nisse-dev/src/nisse_dev/agent.py | 3 + nisse-dev/src/nisse_dev/cancel.py | 10 +- nisse-dev/src/nisse_dev/catalog.py | 16 +++ nisse-dev/src/nisse_dev/context.py | 20 +++- nisse-dev/src/nisse_dev/executor.py | 117 ++++++++++++++------ nisse-dev/src/nisse_dev/llm.py | 15 +++ nisse-dev/src/nisse_dev/probe.py | 46 ++++++-- nisse-dev/tests/test_agent_build.py | 2 + nisse-dev/tests/test_context.py | 52 ++++++++- nisse-dev/tests/test_executor.py | 119 ++++++++++++++++----- nisse-dev/tests/test_llm.py | 71 ++++++++++++ nisse-dev/tests/test_probe.py | 38 ++++++- scripts/probe_context.py | 5 +- tests/integration/test_model_capability.py | 71 ++++++++++++ 16 files changed, 568 insertions(+), 86 deletions(-) diff --git a/docs/Architecture-Build-Spec.md b/docs/Architecture-Build-Spec.md index d7ce157..b795a53 100644 --- a/docs/Architecture-Build-Spec.md +++ b/docs/Architecture-Build-Spec.md @@ -668,7 +668,13 @@ subsystem. ### A15. Local Model Accommodations -- **Constrained decoding for tool-call JSON.** +- **Constrained decoding for tool-call JSON.** Requested with `tool_choice` + (`required`), and only of models whose LiteLLM entry declares + `supports_tool_choice`: enforcement is the model server's, and asking one that + can't honour it can be worse than not asking. Where it isn't available, the + loop enforces the contract itself: an explicit `finish` tool ends the work, a + prose reply is a violation, and a call to a tool not offered counts as a + failure (A12). - **Reduced tool schema for the executor role:** fewer tools, flatter arguments, no deeply nested optionals. Maintain **two tool manifests keyed on role**, rendered at request time like everything else. diff --git a/docs/Tuning-Guide.md b/docs/Tuning-Guide.md index ab2ce87..af4c631 100644 --- a/docs/Tuning-Guide.md +++ b/docs/Tuning-Guide.md @@ -282,6 +282,48 @@ log. - **Deciding:** if milestones stop with "cannot fit", look first at `tools.max_output_chars` and the model's window, not at these. +### Enforced tool calls — `supports_tool_choice`, declared in LiteLLM + +The agent's loops depend on every turn being a well-formed call to a real +tool. A model only *writes text*: a "tool call" is text in a format the model +was trained to produce, which the server recognises and turns into a +structured call. By default (`tool_choice: auto`) the server just parses +whatever comes out, so prose, broken JSON and calls to invented tools are all +possible. + +With `tool_choice: required`, a server that supports it **constrains +decoding**: at every step it rules out any token that couldn't lead to a +valid call to one of the offered tools, with arguments matching its schema. +The model still chooses the tool and the arguments; it can't produce anything +else. That makes it a property of the **model server** (and sometimes of the +model), not something a prompt can buy. + +So the agent asks for it only where it is known to work: models whose LiteLLM +entry declares `supports_tool_choice: true`. Measured on the 4070: + +| Alias | Server | `required` on a chat prompt | Declare | +|---|---|---|---| +| `local-fast` (Qwen3-8B-AWQ) | vLLM 0.29 | calls `finish` every time | `true` | +| `local-coder` (Qwen3-14B) | llama.cpp | can't stop; writes until the output ceiling (a known Qwen3-14B bug) | `false` | +| `claude-haiku-4-5` | Anthropic | calls `finish` every time | `true` (LiteLLM already says so) | + +Asking a model that can't honour it can be worse than not asking: the +Qwen3-14B turn above costs a full output ceiling of generation, where a plain +prose reply would have cost a few hundred tokens. + +Where it isn't enforced, the executor holds the contract itself: + +- **`finish` is the only way to end a milestone.** It is refused once if + nothing has been changed yet. +- **A prose reply is a violation.** The model is told once; a second escalates. +- **A call to a tool that wasn't offered counts as a failure** toward + `failure_limit`, like a failed command, so an inventing model escalates in a + couple of turns instead of using up `max_turns`. + +The capability matrix checks the declaration: a model declared `true` that +answers a required call in prose fails +`test_supports_tool_choice_is_declared_truthfully`. + ### Declaring a local model's limits LiteLLM knows commercial models' limits. For a local model, declare them in @@ -292,8 +334,10 @@ model_list: - model_name: local-fast litellm_params: {model: openai/qwen3-8b-awq, api_base: http://inference-host:8080/v1} model_info: - max_input_tokens: 32768 # the server's per-request context - max_output_tokens: 4096 # the most one response may use + max_input_tokens: 32768 # the server's per-request context + max_output_tokens: 4096 # the most one response may use + supports_function_calling: true + supports_tool_choice: true # only once measured; see above ``` `max_input_tokens` is the *whole* window, prompt plus response. The agent @@ -332,7 +376,7 @@ Upstream choices change what's right downstream, so tune in this order and change one thing at a time. 1. **Model and quantization.** Start with whether a candidate holds the - tool-call contract at all: + tool-call contract at all, and whether its server enforces `tool_choice`: uv run pytest -m integration tests/integration/test_model_capability.py \ --capability-models=alias-a,alias-b @@ -342,6 +386,14 @@ change one thing at a time. each one actually points at in LiteLLM (`/model/info`, the `litellm_params.model` field). Two aliases believed to be the same model once weren't, and that cost a lot of debugging. + + **Make sure LiteLLM has no fallback for a model you are measuring.** A + fallback answers a failed request with another model, under that model's + name, in a response that otherwise looks normal. One made a local model + that couldn't call tools at all look like it "held the tool contract 20 of + 20": every answer was Claude's. The capability matrix and the probe now + check which model answered and fail on a mismatch, but the fallback also + quietly turns local work into frontier spending in production. 2. **Context length and KV-cache precision** on the server. Take the largest context that fits beside the weights with 1–2 GB of headroom. Try an 8-bit KV cache before giving up context. @@ -369,8 +421,5 @@ section for each: - **Compaction.** A model-written summary replacing old turns, instead of dropping them. The log already has an event for it; it needs a summarizing model and a policy for what is safe to summarize. -- **Enforced tool calls.** An explicit `finish` tool, so the server can be told - a tool call is required and hold the model to the contract, where today it - can fall back to prose. - **A benchmark script** that runs the capability matrix and representative milestones across candidate models and prints a comparison table. diff --git a/nisse-dev/src/nisse_dev/agent.py b/nisse-dev/src/nisse_dev/agent.py index 87cf354..4f1a04b 100644 --- a/nisse-dev/src/nisse_dev/agent.py +++ b/nisse-dev/src/nisse_dev/agent.py @@ -174,6 +174,9 @@ def build_agent( max_output_tokens=config.max_output_tokens, keep_recent_turns=tuning.context.keep_recent_turns, safety_margin=tuning.context.safety_margin, + enforces_tool_choice=lambda m: ( + catalog.facts(m).supports_tool_choice is True + ), ) normalized = budgeted( diff --git a/nisse-dev/src/nisse_dev/cancel.py b/nisse-dev/src/nisse_dev/cancel.py index a66c818..4531378 100644 --- a/nisse-dev/src/nisse_dev/cancel.py +++ b/nisse-dev/src/nisse_dev/cancel.py @@ -29,10 +29,12 @@ class Cancel: return self._event.is_set() -def skip_calls(store: StateStore, calls: list[ToolCall]) -> None: - """Record the calls a stop left unrun. Every tool call needs a result for - the transcript to render again, and the result says what happened.""" +def skip_calls( + store: StateStore, calls: list[ToolCall], reason: str = STOPPED_RESULT +) -> None: + """Record calls left unrun. Every tool call needs a result for the + transcript to render again, and the result says what happened.""" for call in calls: store.append_event( - ToolResult(call_id=call.call_id, result=STOPPED_RESULT, success=False) + ToolResult(call_id=call.call_id, result=reason, success=False) ) diff --git a/nisse-dev/src/nisse_dev/catalog.py b/nisse-dev/src/nisse_dev/catalog.py index 0bab212..8bb97d1 100644 --- a/nisse-dev/src/nisse_dev/catalog.py +++ b/nisse-dev/src/nisse_dev/catalog.py @@ -54,11 +54,26 @@ class ModelFacts: max_input_tokens: int | None = None # The most one response may use (model_info.max_output_tokens). max_output_tokens: int | None = None + # Whether its server enforces tool_choice (model_info.supports_tool_choice). + # Only a declared True counts: a model that says nothing is not asked. + supports_tool_choice: bool | None = None @property def native_anthropic(self) -> bool: return self.provider == ANTHROPIC_PROVIDER + def answered(self, reported: str | None) -> bool: + """Whether a response reporting this model name came from this model. + + A proxy fallback answers under another model's name. LiteLLM reports + the alias; a provider may report its own name for the backing model, + sometimes with a date suffix ("claude-haiku-4-5-20251001"). + """ + if not reported: + return True # nothing reported, nothing contradicted + own_name = self.backing.partition("/")[2] or self.backing + return reported in (self.alias, self.backing) or reported.startswith(own_name) + class ModelCatalog: """What one LiteLLM key can see, as of the moment it was asked.""" @@ -87,6 +102,7 @@ class ModelCatalog: supports_tools=info.get("supports_function_calling"), max_input_tokens=_positive_int(info.get("max_input_tokens")), max_output_tokens=_positive_int(info.get("max_output_tokens")), + supports_tool_choice=info.get("supports_tool_choice"), ) return cls(facts) diff --git a/nisse-dev/src/nisse_dev/context.py b/nisse-dev/src/nisse_dev/context.py index 1151dd5..d5c162b 100644 --- a/nisse-dev/src/nisse_dev/context.py +++ b/nisse-dev/src/nisse_dev/context.py @@ -215,9 +215,15 @@ class BudgetedClient: max_output_tokens: int = DEFAULT_MAX_OUTPUT_TOKENS, keep_recent_turns: int = 6, safety_margin: float = 0.1, + enforces_tool_choice: Callable[[str], bool] = lambda model: False, ) -> None: self._inner = inner self._limits_for = limits_for + # Whether a model's server holds it to the offered tools when asked + # (LiteLLM's supports_tool_choice). Asking one that doesn't can be + # worse than not asking: qwen3-14b can't stop, and rambles until the + # output ceiling. + self._enforces_tool_choice = enforces_tool_choice self._max_output_tokens = max_output_tokens self._keep_recent_turns = keep_recent_turns self._safety_margin = safety_margin @@ -229,6 +235,10 @@ class BudgetedClient: def last_usage(self) -> dict[str, int]: return getattr(self._inner, "last_usage", {}) or {} + @property + def last_model(self) -> str | None: + return getattr(self._inner, "last_model", None) + def budget_for(self, model: str) -> Budget: window, model_max_output = self._limits_for(model) ceiling = self._max_output_tokens @@ -252,7 +262,15 @@ class BudgetedClient: factor = self._factors.get(model, 1.0) fitted = fit(events, system, tools, budget, factor) produced = self._inner.complete( - fitted, tools, model, system=system, max_tokens=budget.max_output + fitted, + tools, + model, + system=system, + max_tokens=budget.max_output, + # Every caller that offers tools needs one called: the executor + # finishes with finish, the planner with submit_plan or + # ask_human, a delegation with hand_back. + require_tool_call=bool(tools) and self._enforces_tool_choice(model), ) reported = int(self.last_usage.get("input", 0)) if reported > 0: diff --git a/nisse-dev/src/nisse_dev/executor.py b/nisse-dev/src/nisse_dev/executor.py index 6ec80fa..1eeb75e 100644 --- a/nisse-dev/src/nisse_dev/executor.py +++ b/nisse-dev/src/nisse_dev/executor.py @@ -27,20 +27,42 @@ EXECUTOR_SYSTEM = """You are nisse-dev's executor, doing mechanical work. Follow the approved plan below exactly. It was written to be executed verbatim; do not redesign it, expand its scope, or skip steps. -Work by calling tools. Verify your work by running the project's tests. -When every step of the plan is done and the tests pass, reply with a short -plain-text summary and no tool call — that ends the milestone.""" +Every reply must be a tool call. Verify your work by running the project's +tests. When every step of the plan is done and the tests pass, call finish +with a short summary — that, and nothing else, ends the milestone.""" DEFAULT_MAX_TURNS = 24 DEFAULT_FAILURE_LIMIT = 2 -# A turn with no tool call means "the plan is done". Believing that before -# any real work has happened would record an empty milestone as a success, so -# the claim is challenged once first. -_NUDGE = ( - "\n\nYou have not made any change yet, so the plan cannot be complete. " - "Carry out the next step of the plan with a tool call, or run the tests " - "to show the work is already done." +FINISH_TOOL: dict[str, Any] = { + "type": "function", + "function": { + "name": "finish", + "description": ( + "End the milestone: every step of the plan is done and the tests pass." + ), + "parameters": { + "type": "object", + "properties": { + "summary": {"type": "string", "description": "What was done."} + }, + "required": ["summary"], + }, + }, +} + +# Finishing before any real work has happened would record an empty +# milestone as a success, so the claim is refused once first. +_NOT_YET = ( + "You have not made any change yet, so the plan cannot be complete. Carry " + "out the next step of the plan, or run the tests to show the work is " + "already done, before calling finish." +) +# A reply in prose breaks the tool contract. A server that enforces +# tool_choice never allows one; on one that doesn't, the model is told once. +_PROSE_NUDGE = ( + "\n\nYour last reply was prose. Every reply must be a tool call: carry " + "out the next step, or call finish if the plan is complete." ) @@ -128,8 +150,8 @@ class LLMExecutor: # A12: ask_expert exists in addition to the observable escalation # triggers, never as a replacement for them. if self._expert is None: - return EXECUTOR_TOOLS - return [*EXECUTOR_TOOLS, ASK_EXPERT_TOOL] + return [*EXECUTOR_TOOLS, FINISH_TOOL] + return [*EXECUTOR_TOOLS, ASK_EXPERT_TOOL, FINISH_TOOL] def _consult(self, arguments: dict[str, Any]) -> ToolOutcome: assert self._expert is not None @@ -173,9 +195,12 @@ class LLMExecutor: if plan is not None: # In the system prompt, so the context budget never drops it. system += f"\n\nThe approved plan:\n{plan}" + tools = self._tools() + offered = {tool["function"]["name"] for tool in tools} consecutive_failures = 0 did_work = False - challenged = False + finish_refused = False + prose_nudged = False for turn in range(self._max_turns): # A4: `stop` takes effect at the next tool boundary. The start of @@ -185,7 +210,7 @@ class LLMExecutor: try: produced = self._client.complete( self._store.read_events(after=after), - tools=self._tools(), + tools=tools, model=model, system=system, ) @@ -199,19 +224,12 @@ class LLMExecutor: calls.append(event) if not calls: - if did_work: - return ExecutorOutcome( - "completed", "executor reported the plan done" - ) - if not challenged: - # Claiming completion without having touched anything is - # not evidence of completion; ask once before believing it. - challenged = True - system += _NUDGE + if not prose_nudged: + prose_nudged = True + system += _PROSE_NUDGE continue return ExecutorOutcome( - "escalate", - "executor claimed the milestone was done without making any change", + "escalate", "executor kept replying in prose instead of tools" ) self._report(turn, calls) @@ -219,11 +237,47 @@ class LLMExecutor: if self._cancel.requested: skip_calls(self._store, calls[index:]) return ExecutorOutcome("stopped", f"during turn {turn + 1}") - if call.tool in MUTATING_TOOLS: - did_work = True - if call.tool == "ask_expert": + if call.tool == "finish": + if not did_work and not finish_refused: + finish_refused = True + outcome = ToolOutcome(_NOT_YET, success=False) + elif not did_work: + return ExecutorOutcome( + "escalate", + "executor claimed the milestone was done without " + "making any change", + ) + else: + self._store.append_event( + ToolResult( + call_id=call.call_id, + result="milestone finished", + success=True, + ) + ) + skip_calls( + self._store, + calls[index + 1 :], + "not run: finish had already ended the milestone", + ) + summary = str(call.arguments.get("summary", "")).strip() + return ExecutorOutcome( + "completed", summary or "executor reported the plan done" + ) + elif call.tool not in offered: + # A model inventing tools is not doing the work, and with + # no server enforcement nothing else would stop it, so it + # counts toward escalation like a failed command (A12). + outcome = ToolOutcome( + f"no such tool: {call.tool}; the tools are " + + ", ".join(sorted(offered)), + success=False, + ) + elif call.tool == "ask_expert": outcome = self._consult(call.arguments) else: + if call.tool in MUTATING_TOOLS: + did_work = True outcome = self._workspace.dispatch(call.tool, call.arguments) self._store.append_event( ToolResult( @@ -232,9 +286,10 @@ class LLMExecutor: success=outcome.success, ) ) - # Only commands count toward escalation: a failed read is the - # model looking around, a failed command is work not working. - if call.tool == "run_command": + # Commands and invented tools count toward escalation: a + # failed read is the model looking around, a failed command + # is work not working. + if call.tool == "run_command" or call.tool not in offered: consecutive_failures = ( 0 if outcome.success else consecutive_failures + 1 ) diff --git a/nisse-dev/src/nisse_dev/llm.py b/nisse-dev/src/nisse_dev/llm.py index 8cec4bf..10c8877 100644 --- a/nisse-dev/src/nisse_dev/llm.py +++ b/nisse-dev/src/nisse_dev/llm.py @@ -142,6 +142,7 @@ class LiteLLMClient: # a side channel rather than a return value, so the LLMClient # protocol stays the same for every implementation. self.last_usage: dict[str, int] = {} + self.last_model: str | None = None def complete( self, @@ -151,8 +152,12 @@ class LiteLLMClient: system: str | None = None, *, max_tokens: int | None = None, + require_tool_call: bool = False, ) -> list[CanonicalEvent]: body = self._renderer.render(events, tools, model, system=system) + if require_tool_call and tools: + # Only for models whose server enforces it; see BudgetedClient. + body["tool_choice"] = "required" # A ceiling on this route too: without one a runaway turn is bounded # only by the provider's default, and the same truncation check has # nothing to check against. A caller that knows the model's limits @@ -182,6 +187,9 @@ class LiteLLMClient: "llm.max_output_tokens, or the model's max_output_tokens in LiteLLM" ) produced_by = data.get("model") or requested_model + # What actually answered, which is not always what was asked for: a + # proxy fallback answers under another model's name. + self.last_model = produced_by usage = data.get("usage") or {} self.last_usage = { "input": int(usage.get("prompt_tokens") or 0), @@ -254,6 +262,7 @@ class AnthropicPassthroughClient: self._max_tokens = max_tokens or DEFAULT_MAX_OUTPUT_TOKENS self._sleep = sleep self.last_usage: dict[str, int] = {} + self.last_model: str | None = None def complete( self, @@ -263,6 +272,7 @@ class AnthropicPassthroughClient: system: str | None = None, *, max_tokens: int | None = None, + require_tool_call: bool = False, ) -> list[CanonicalEvent]: body = self._renderer.render( events, @@ -271,6 +281,8 @@ class AnthropicPassthroughClient: system=system, max_tokens=max_tokens or self._max_tokens, ) + if require_tool_call and tools: + body["tool_choice"] = {"type": "any"} # Anthropic's name for it response = _post_with_rate_limit_retry( self._http, "/v1/messages", body, self._sleep ) @@ -293,6 +305,9 @@ class AnthropicPassthroughClient: if not isinstance(content, list): raise LLMError(f"malformed passthrough response: {str(data)[:300]}") produced_by = data.get("model") or requested_model + # What actually answered, which is not always what was asked for: a + # proxy fallback answers under another model's name. + self.last_model = produced_by usage = data.get("usage") or {} self.last_usage = { "input": int(usage.get("input_tokens") or 0), diff --git a/nisse-dev/src/nisse_dev/probe.py b/nisse-dev/src/nisse_dev/probe.py index 54ef411..1fda6e9 100644 --- a/nisse-dev/src/nisse_dev/probe.py +++ b/nisse-dev/src/nisse_dev/probe.py @@ -11,6 +11,10 @@ with it: - **It took the whole prompt.** The server allows more than is declared, so the declaration is conservative. +Every answer is checked for which model gave it. A proxy that falls back to +another model on an error returns a normal-looking answer under that model's +name, and once made this probe report a refusing server as roomy. + Run it with ``scripts/probe_context.py``. It spends one small and one large request on the model, so it refuses windows above a ceiling unless told otherwise: commercial models' limits are already known to LiteLLM. @@ -20,6 +24,8 @@ from dataclasses import dataclass import httpx +from nisse_dev.catalog import ModelFacts + # Of a probe's size, how much must come back for it to count as uncut. A # chat template adds a little, and a tokenizer counts a little differently. _UNCUT = 0.9 @@ -86,7 +92,14 @@ def _prompt(words: int) -> str: return " ".join(f"item{i}" for i in range(words)) + "\n\nReply with just: ok" -def _send(http: httpx.Client, model: str, words: int) -> tuple[int | None, str | None]: +class Fallback(Exception): + def __init__(self, answered_by: str) -> None: + self.answered_by = answered_by + + +def _send( + http: httpx.Client, model: str, words: int, backing: str +) -> tuple[int | None, str | None]: response = http.post( "/chat/completions", json={ @@ -97,18 +110,33 @@ def _send(http: httpx.Client, model: str, words: int) -> tuple[int | None, str | ) if response.status_code != 200: return None, f"{response.status_code}: {response.text}" - usage = response.json().get("usage") or {} + data = response.json() + answered = str(data.get("model") or "") + facts = ModelFacts(alias=model, backing=backing, provider="", supports_tools=None) + if not facts.answered(answered): + raise Fallback(answered) + usage = data.get("usage") or {} return int(usage.get("prompt_tokens") or 0), None -def probe(http: httpx.Client, model: str, declared: int) -> ProbeResult: +def probe( + http: httpx.Client, model: str, declared: int, backing: str = "" +) -> ProbeResult: """Measure how many tokens a word of the probe costs on this model, then send a prompt half again as long as the declared window.""" calibration_words = 200 - tokens, error = _send(http, model, calibration_words) - if error is not None or not tokens: - return ProbeResult("failed", f"a short calibration request failed: {error}") - per_word = tokens / calibration_words - sent = int(declared * 1.5) - reported, error = _send(http, model, int(sent / per_word)) + try: + tokens, error = _send(http, model, calibration_words, backing) + if error is not None or not tokens: + return ProbeResult("failed", f"a short calibration request failed: {error}") + per_word = tokens / calibration_words + sent = int(declared * 1.5) + reported, error = _send(http, model, int(sent / per_word), backing) + except Fallback as fallback: + return ProbeResult( + "fallback", + f"{fallback.answered_by} answered instead of {model}: the proxy fell " + "back to another model, so nothing here describes this one. Remove " + "the fallback, or probe the server directly.", + ) return interpret(declared, sent, reported, error) diff --git a/nisse-dev/tests/test_agent_build.py b/nisse-dev/tests/test_agent_build.py index 4ba7548..3e25d59 100644 --- a/nisse-dev/tests/test_agent_build.py +++ b/nisse-dev/tests/test_agent_build.py @@ -344,4 +344,6 @@ def test_tuning_reaches_every_component( assert executor._client._inner._max_tokens == 8192 assert delegation._client._inner._max_tokens == 8192 assert executor._client._max_output_tokens == 8192 + # tool_choice is asked of exactly the models LiteLLM declares enforce it. + assert executor._client._enforces_tool_choice("claude-opus-5") is False agent.stop() diff --git a/nisse-dev/tests/test_context.py b/nisse-dev/tests/test_context.py index 6bda0d0..5da70a1 100644 --- a/nisse-dev/tests/test_context.py +++ b/nisse-dev/tests/test_context.py @@ -150,8 +150,8 @@ class RecordingClient: self.calls: list[dict[str, Any]] = [] self.last_usage = {"input": reported_input, "output": 5} - def complete(self, events, tools, model, system=None, *, max_tokens=None): - self.calls.append({"events": events, "max_tokens": max_tokens}) + def complete(self, events, tools, model, system=None, *, max_tokens=None, **kwargs): + self.calls.append({"events": events, "max_tokens": max_tokens, **kwargs}) return [AssistantText(text="ok", model=model)] @@ -292,3 +292,51 @@ def test_a_request_that_cannot_fit_stops_the_milestone(tmp_path: Path) -> None: assert outcome.status == "escalate" assert "fit" in outcome.note assert inner.calls == [] + + +# --- tool_choice is only asked of servers that enforce it ------------------- + +FINISH = [{"type": "function", "function": {"name": "finish", "parameters": {}}}] + + +def test_required_is_asked_only_of_models_declared_to_enforce_it() -> None: + # Measured: qwen3-14b under "required" cannot stop and rambles to the + # output ceiling, so a model that is not declared is never asked. + inner = RecordingClient() + client = BudgetedClient( + inner, lambda m: (32768, 8192), enforces_tool_choice=lambda m: m == "local-fast" + ) + client.complete(transcript(1, 10), FINISH, "local-fast") + client.complete(transcript(1, 10), FINISH, "local-coder") + client.complete(transcript(1, 10), None, "local-fast") + assert [c["require_tool_call"] for c in inner.calls] == [True, False, False] + + +def test_the_catalog_reads_supports_tool_choice() -> None: + catalog = ModelCatalog.from_payload( + { + "data": [ + {"model_name": "a", "model_info": {"supports_tool_choice": True}}, + {"model_name": "b", "model_info": {"supports_tool_choice": False}}, + {"model_name": "c", "model_info": {}}, + ] + } + ) + assert [catalog.facts(m).supports_tool_choice for m in "abc"] == [True, False, None] + + +def test_a_fallback_answer_is_recognised() -> None: + facts = ModelCatalog.from_payload( + { + "data": [ + { + "model_name": "claude-haiku-4-5", + "litellm_params": {"model": "anthropic/claude-haiku-4-5"}, + } + ] + } + ).facts("claude-haiku-4-5") + assert facts.answered("claude-haiku-4-5") # LiteLLM's alias + assert facts.answered("claude-haiku-4-5-20251001") # the provider's own name + assert facts.answered(None) # nothing reported + assert not facts.answered("claude-sonnet-5") # somebody else answered diff --git a/nisse-dev/tests/test_executor.py b/nisse-dev/tests/test_executor.py index a19ef20..846721a 100644 --- a/nisse-dev/tests/test_executor.py +++ b/nisse-dev/tests/test_executor.py @@ -4,7 +4,13 @@ from typing import Any import pytest -from nisse_dev.canonical import AssistantText, CanonicalEvent, StoredEvent, ToolCall +from nisse_dev.canonical import ( + AssistantText, + CanonicalEvent, + StoredEvent, + ToolCall, + ToolResult, +) from nisse_dev.executor import LLMExecutor from nisse_dev.llm import LLMError, RoleMap from nisse_dev.oplog import OpLog @@ -33,7 +39,7 @@ class ScriptedClient: ) -> list[CanonicalEvent]: self.calls.append({"tools": tools, "model": model, "system": system}) if not self._turns: - return [AssistantText(text="done", model=model)] + return [finish(call_id=f"fin-{len(self.calls)}")] return self._turns.pop(0) @@ -46,6 +52,10 @@ def call(tool: str, call_id: str = "c1", **arguments: Any) -> ToolCall: return ToolCall(call_id=call_id, tool=tool, arguments=arguments) +def finish(summary: str = "done", call_id: str = "fin") -> ToolCall: + return ToolCall(call_id=call_id, tool="finish", arguments={"summary": summary}) + + @pytest.fixture def store(tmp_path: Path) -> StateStore: return StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())) @@ -66,15 +76,16 @@ def test_tool_calls_are_executed_and_results_logged( client = ScriptedClient( [ [call("write_file", path="app.py", content="print(1)\n")], - [AssistantText(text="wrote the file", model="llama3.2")], + [finish("wrote the file")], ] ) outcome = build(client, store, ws).run(milestone=1) assert outcome.status == "completed" + assert outcome.note == "wrote the file" assert (ws.root / "app.py").read_text() == "print(1)\n" kinds = [type(e.event).__name__ for e in store.read_events()] - assert kinds == ["ToolCall", "ToolResult", "AssistantText"] + assert kinds == ["ToolCall", "ToolResult", "ToolCall", "ToolResult"] def test_uses_executor_role_and_reduced_manifest( @@ -84,50 +95,99 @@ def test_uses_executor_role_and_reduced_manifest( build(client, store, ws).run(milestone=2) assert client.calls[0]["model"] == "llama3.2" # never the planner names = [t["function"]["name"] for t in client.calls[0]["tools"]] - assert names == ["read_file", "write_file", "list_dir", "run_command"] + assert names == ["read_file", "write_file", "list_dir", "run_command", "finish"] assert "milestone 2" in client.calls[0]["system"] -def test_no_tool_call_after_work_ends_the_milestone( - store: StateStore, ws: Workspace -) -> None: +def test_prose_no_longer_ends_the_milestone(store: StateStore, ws: Workspace) -> None: + # Only finish ends a milestone. Prose breaks the tool contract: the model + # is told once, and here it gets back to work. client = ScriptedClient( [ [call("write_file", path="a.py", content="x")], [AssistantText(text="all steps done", model="m")], + [finish()], ] ) assert build(client, store, ws).run(milestone=1).status == "completed" + assert "Every reply must be a tool call" in client.calls[2]["system"] -def test_completion_claimed_without_any_work_is_challenged( +def test_persisting_in_prose_escalates(store: StateStore, ws: Workspace) -> None: + prose = [AssistantText(text="let me just chat", model="m")] + client = ScriptedClient([prose, prose]) + outcome = build(client, store, ws).run(milestone=1) + assert outcome.status == "escalate" + assert "prose" in outcome.note + + +def test_finishing_without_any_work_is_refused_once( store: StateStore, ws: Workspace ) -> None: - # A model that answers with prose on turn one has done nothing; believing - # it would record an empty milestone as a success. + # A model that finishes on turn one has done nothing; believing it would + # record an empty milestone as a success. client = ScriptedClient( [ - [AssistantText(text="looks done to me", model="m")], + [finish("looks done to me", call_id="f1")], [call("write_file", path="a.py", content="x")], - [AssistantText(text="now it is done", model="m")], + [finish("now it is done", call_id="f2")], ] ) outcome = build(client, store, ws).run(milestone=1) assert outcome.status == "completed" - assert (ws.root / "a.py").exists() # the nudge produced actual work - assert "not made any change yet" in client.calls[1]["system"] + assert (ws.root / "a.py").exists() # the refusal produced actual work + [refusal] = [ + e.event + for e in store.read_events() + if isinstance(e.event, ToolResult) and e.event.call_id == "f1" + ] + assert not refusal.success + assert "not made any change yet" in refusal.result -def test_persisting_in_an_empty_claim_escalates( +def test_persisting_in_an_empty_finish_escalates( store: StateStore, ws: Workspace ) -> None: - prose = [AssistantText(text="nothing to do", model="m")] - client = ScriptedClient([prose, prose]) + client = ScriptedClient([[finish(call_id="f1")], [finish(call_id="f2")]]) outcome = build(client, store, ws).run(milestone=1) assert outcome.status == "escalate" assert "without making any change" in outcome.note +def test_calls_after_finish_are_not_run(store: StateStore, ws: Workspace) -> None: + client = ScriptedClient( + [ + [call("write_file", path="a.py", content="x")], + [finish(), call("write_file", call_id="late", path="b.py", content="y")], + ] + ) + assert build(client, store, ws).run(milestone=1).status == "completed" + assert not (ws.root / "b.py").exists() + [late] = [ + e.event + for e in store.read_events() + if isinstance(e.event, ToolResult) and e.event.call_id == "late" + ] + assert "finish had already ended the milestone" in late.result + + +def test_an_invented_tool_counts_toward_escalation( + store: StateStore, ws: Workspace +) -> None: + # Measured: a model on a server that doesn't enforce tool_choice will + # call tools that were never offered ("chat", "respond"). Without this it + # would burn every turn up to the cap. + client = ScriptedClient( + [[call("chat", call_id="x1")], [call("respond_user", call_id="x2")]] + ) + outcome = build(client, store, ws, failure_limit=2).run(milestone=1) + assert outcome.status == "escalate" + assert len(client.calls) == 2 + results = [e.event for e in store.read_events() if isinstance(e.event, ToolResult)] + assert "no such tool: chat" in results[0].result + assert "finish" in results[0].result # it is told what does exist + + def test_two_consecutive_command_failures_escalate( store: StateStore, ws: Workspace ) -> None: @@ -150,7 +210,7 @@ def test_a_success_resets_the_failure_count(store: StateStore, ws: Workspace) -> [call("run_command", command="exit 1")], [call("run_command", command="true")], [call("run_command", command="exit 1")], - [AssistantText(text="done", model="m")], + [finish()], ] ) assert build(client, store, ws, failure_limit=2).run(milestone=1).status == ( @@ -168,7 +228,7 @@ def test_failed_reads_do_not_count_toward_escalation( [call("read_file", path="missing-a.txt")], [call("read_file", path="missing-b.txt")], [call("write_file", path="a.py", content="x")], - [AssistantText(text="done", model="m")], + [finish()], ] ) assert build(client, store, ws, failure_limit=2).run(milestone=1).status == ( @@ -182,8 +242,8 @@ def test_reading_alone_is_not_doing_the_work(store: StateStore, ws: Workspace) - client = ScriptedClient( [ [call("list_dir", path=".")], - [AssistantText(text="looks done to me", model="m")], - [AssistantText(text="really, done", model="m")], + [finish("looks done to me", call_id="f1")], + [finish("really, done", call_id="f2")], ] ) outcome = build(client, store, ws).run(milestone=1) @@ -214,7 +274,7 @@ def test_multiple_calls_in_one_turn_all_run(store: StateStore, ws: Workspace) -> call("write_file", call_id="a", path="one.txt", content="1"), call("write_file", call_id="b", path="two.txt", content="2"), ], - [AssistantText(text="done", model="m")], + [finish()], ] ) build(client, store, ws).run(milestone=1) @@ -222,7 +282,7 @@ def test_multiple_calls_in_one_turn_all_run(store: StateStore, ws: Workspace) -> results = [ e.event for e in store.read_events() if type(e.event).__name__ == "ToolResult" ] - assert {r.call_id for r in results} == {"a", "b"} + assert {r.call_id for r in results} == {"a", "b", "fin"} class StubDesk: @@ -255,7 +315,8 @@ def test_ask_expert_is_offered_only_when_a_desk_exists( client2 = ScriptedClient([]) build(client2, store, ws, expert=StubDesk()).run(milestone=1) names = [t["function"]["name"] for t in client2.calls[0]["tools"]] - assert names[-1] == "ask_expert" + assert "ask_expert" in names + assert names[-1] == "finish" # A12: in addition to the observable triggers, not instead of them. assert "run_command" in names @@ -268,7 +329,7 @@ def test_ask_expert_answer_comes_back_as_a_tool_result( [ [call("ask_expert", question="sqlite or postgres?")], [call("write_file", path="a.py", content="x")], - [AssistantText(text="done", model="m")], + [finish()], ] ) build(client, store, ws, expert=desk).run(milestone=1) @@ -286,8 +347,8 @@ def test_consulting_is_not_doing_the_work(store: StateStore, ws: Workspace) -> N client = ScriptedClient( [ [call("ask_expert", question="what should I do?")], - [AssistantText(text="I think it is done", model="m")], - [AssistantText(text="really done", model="m")], + [finish("I think it is done", call_id="f1")], + [finish("really done", call_id="f2")], ] ) outcome = build(client, store, ws, expert=StubDesk()).run(milestone=1) @@ -304,7 +365,7 @@ def test_exhausted_budget_is_reported_to_the_model_not_raised( [ [call("ask_expert", question="again?")], [call("write_file", path="a.py", content="x")], - [AssistantText(text="done", model="m")], + [finish()], ] ) outcome = build(client, store, ws, expert=desk).run(milestone=1) diff --git a/nisse-dev/tests/test_llm.py b/nisse-dev/tests/test_llm.py index 5b1238e..fd930ff 100644 --- a/nisse-dev/tests/test_llm.py +++ b/nisse-dev/tests/test_llm.py @@ -1,5 +1,6 @@ import json from pathlib import Path +from typing import Any import httpx import pytest @@ -341,3 +342,73 @@ def test_budget_refusal_is_a_kind_of_upstream_trouble() -> None: # So one catch covers the family, but it can still be told apart where # the difference matters — it will not clear within the hour. assert issubclass(BudgetExhausted, UpstreamUnavailable) + + +# --- tool_choice (A15: constrained decoding) -------------------------------- + +TOOLS = [ + { + "type": "function", + "function": { + "name": "finish", + "description": "done", + "parameters": {"type": "object", "properties": {}}, + }, + } +] + + +def capture_bodies() -> tuple[list[dict], Any]: + bodies: list[dict] = [] + + def handler(request: httpx.Request) -> httpx.Response: + bodies.append(json.loads(request.content)) + return completion({"role": "assistant", "content": "ok"}) + + return bodies, handler + + +def test_required_is_sent_only_when_asked_and_tools_are_offered() -> None: + bodies, handler = capture_bodies() + client = make_client(handler) + client.complete(EVENTS, tools=TOOLS, model="m", require_tool_call=True) + client.complete(EVENTS, tools=TOOLS, model="m") + client.complete(EVENTS, tools=None, model="m", require_tool_call=True) + assert bodies[0]["tool_choice"] == "required" + assert "tool_choice" not in bodies[1] # the server's default, auto + assert "tool_choice" not in bodies[2] # nothing to choose from + + +def test_the_passthrough_asks_in_anthropics_terms() -> None: + bodies: list[dict] = [] + + def handler(request: httpx.Request) -> httpx.Response: + bodies.append(json.loads(request.content)) + return httpx.Response( + 200, + json={ + "model": "claude-haiku-4-5", + "stop_reason": "tool_use", + "content": [ + {"type": "tool_use", "id": "t", "name": "finish", "input": {}} + ], + }, + ) + + client = AnthropicPassthroughClient( + "https://litellm.test", "tok", transport=httpx.MockTransport(handler) + ) + client.complete( + EVENTS, tools=TOOLS, model="claude-haiku-4-5", require_tool_call=True + ) + assert bodies[0]["tool_choice"] == {"type": "any"} + + +def test_the_answering_model_is_recorded() -> None: + # A proxy fallback answers under another model's name; this is how a + # caller can tell. + client = make_client( + lambda request: completion({"role": "assistant", "content": "hi"}) + ) + client.complete(EVENTS, tools=None, model="local-fast") + assert client.last_model == "gpt-test-1" diff --git a/nisse-dev/tests/test_probe.py b/nisse-dev/tests/test_probe.py index ca1be0d..3543f78 100644 --- a/nisse-dev/tests/test_probe.py +++ b/nisse-dev/tests/test_probe.py @@ -42,7 +42,9 @@ def test_probe_calibrates_then_sends_half_again_the_window() -> None: words = len(json.loads(request.content)["messages"][0]["content"].split()) sizes.append(words * 2) reported = min(words * 2, 4096) - return httpx.Response(200, json={"usage": {"prompt_tokens": reported}}) + return httpx.Response( + 200, json={"model": "llama3.2", "usage": {"prompt_tokens": reported}} + ) with httpx.Client( base_url="http://litellm", transport=httpx.MockTransport(handler) @@ -51,3 +53,37 @@ def test_probe_calibrates_then_sends_half_again_the_window() -> None: assert result.verdict == "matches" assert sizes[1] >= 4096 * 1.4 # the probe really exceeded the window + + +def test_a_fallback_answer_is_never_credited_to_the_model() -> None: + """Measured: LiteLLM answered an oversized prompt to local-coder with + claude-sonnet-5, and the probe called the refusing server roomy.""" + + def handler(request: httpx.Request) -> httpx.Response: + words = len(json.loads(request.content)["messages"][0]["content"].split()) + model = "local-coder" if words < 1000 else "claude-sonnet-5" + return httpx.Response( + 200, json={"model": model, "usage": {"prompt_tokens": words * 2}} + ) + + with httpx.Client( + base_url="http://litellm", transport=httpx.MockTransport(handler) + ) as http: + result = probe(http, "local-coder", declared=32768, backing="openai/qwen3-14b") + + assert result.verdict == "fallback" + assert "claude-sonnet-5 answered instead of local-coder" in result.message + + +def test_a_provider_reporting_its_own_model_name_is_not_a_fallback() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, json={"model": "qwen3-14b", "usage": {"prompt_tokens": 400}} + ) + + with httpx.Client( + base_url="http://litellm", transport=httpx.MockTransport(handler) + ) as http: + result = probe(http, "local-coder", declared=32768, backing="openai/qwen3-14b") + + assert result.verdict != "fallback" diff --git a/scripts/probe_context.py b/scripts/probe_context.py index f23704b..0364a44 100644 --- a/scripts/probe_context.py +++ b/scripts/probe_context.py @@ -45,7 +45,8 @@ def main() -> int: return 2 catalog = ModelCatalog.discover(config.llm_base_url, config.llm_token) - declared = catalog.facts(args.alias).max_input_tokens + facts = catalog.facts(args.alias) + declared = facts.max_input_tokens if declared is None: print( f"LiteLLM declares no max_input_tokens for {args.alias}; set one first", @@ -65,7 +66,7 @@ def main() -> int: headers={"Authorization": f"Bearer {config.llm_token}"}, timeout=600, ) as http: - result = probe(http, args.alias, declared) + result = probe(http, args.alias, declared, facts.backing) print(f"{args.alias}: {result.message}") return 0 if result.verdict in ("matches", "refuses", "declared_conservative") else 1 diff --git a/tests/integration/test_model_capability.py b/tests/integration/test_model_capability.py index 8205cf5..6411177 100644 --- a/tests/integration/test_model_capability.py +++ b/tests/integration/test_model_capability.py @@ -30,6 +30,7 @@ from nisse_dev.canonical import ( ) from nisse_dev.catalog import ModelCatalog, select_client from nisse_dev.config import AgentConfig +from nisse_dev.executor import FINISH_TOOL from nisse_dev.expert import parse_answer from nisse_dev.llm import AnthropicPassthroughClient, LiteLLMClient, LLMError from nisse_dev.planner import PLANNER_TOOLS @@ -73,6 +74,16 @@ def catalog(live_config: AgentConfig) -> ModelCatalog: return ModelCatalog.discover(live_config.llm_base_url, live_config.llm_token) +def assert_answered(client: object, model: str, catalog: ModelCatalog) -> None: + """A proxy fallback answers under another model's name. Crediting its + answer to this model is how a local model once 'held the tool contract + 20/20' while never answering at all.""" + reported = getattr(client, "last_model", None) + assert catalog.facts(model).answered(reported), ( + f"{reported} answered instead of {model}: a fallback, not a measurement" + ) + + def user(text: str) -> list[StoredEvent]: return [StoredEvent(id=1, ts="t", event=UserMessage(text=text, source="probe"))] @@ -83,6 +94,7 @@ def test_model_answers_at_all(model: str, catalog: ModelCatalog, routes) -> None produced = client.complete( user("Reply with exactly one word: pong"), None, model, system="Be terse." ) + assert_answered(client, model, catalog) texts = [e.text for e in produced if isinstance(e, AssistantText)] assert texts, f"{model} produced no text: {produced!r}" @@ -103,6 +115,7 @@ def test_model_holds_the_planner_contract( "ask_human. Plain prose is not accepted." ), ) + assert_answered(client, model, catalog) calls = [e for e in produced if isinstance(e, ToolCall)] assert calls, f"{model} answered in prose instead of calling a tool" assert calls[0].tool in {"submit_plan", "ask_human"} @@ -140,6 +153,7 @@ def test_model_round_trips_a_tool_result( ), ) ) + assert_answered(client, model, catalog) follow_up = client.complete(events, EXECUTOR_TOOLS, model, system="Use a tool.") assert follow_up, f"{model} produced nothing after its tool result" @@ -161,6 +175,7 @@ def test_model_holds_the_prompt_level_json_contract( '"caveats": "..."}' ), ) + assert_answered(client, model, catalog) text = "\n".join(e.text for e in produced if isinstance(e, AssistantText)) assert parse_answer(text) is not None, ( f"{model} did not produce parseable JSON: {text[:200]!r}" @@ -185,3 +200,59 @@ def test_route_selection_matches_the_model( chosen.complete(user("say pong"), None, model, system="Be terse.") except LLMError as exc: pytest.fail(f"{model} failed on its selected route: {exc}") + assert_answered(chosen, model, catalog) + + +FINISH_TOOLS = [*EXECUTOR_TOOLS, FINISH_TOOL] +OFFERED = {tool["function"]["name"] for tool in FINISH_TOOLS} + + +def test_model_calls_only_the_tools_it_was_offered( + model: str, catalog: ModelCatalog, routes +) -> None: + """Measured on a server that did not enforce tool_choice: a model asked + to chat called "chat", "say_hello" and "respond", none of them offered. + The executor counts those as failures; this measures how often.""" + normalized, passthrough = routes + client = select_client(model, normalized, passthrough, catalog) + produced = client.complete( + user("Hi! How are you today? Just chat with me for a moment."), + FINISH_TOOLS, + model, + system="You are an agent. Every reply must be a tool call.", + ) + assert_answered(client, model, catalog) + called = {e.tool for e in produced if isinstance(e, ToolCall)} + assert called <= OFFERED, ( + f"{model} called tools it was never offered: {called - OFFERED}" + ) + + +def test_supports_tool_choice_is_declared_truthfully( + model: str, catalog: ModelCatalog, routes +) -> None: + """The agent asks for tool_choice only of models LiteLLM declares + supports_tool_choice for, so the declaration must be true. Asked of a + model that can't honour it, "required" can be worse than not asking: + qwen3-14b can't stop, and writes until the output ceiling.""" + facts = catalog.facts(model) + if facts.supports_tool_choice is not True: + pytest.skip( + f"{model} is not declared to support tool_choice, so is never asked" + ) + normalized, passthrough = routes + client = select_client(model, normalized, passthrough, catalog) + produced = client.complete( + user("Hi! How are you today? Just chat with me for a moment."), + FINISH_TOOLS, + model, + system="You are a helpful assistant.", + require_tool_call=True, + ) + assert_answered(client, model, catalog) + calls = [e for e in produced if isinstance(e, ToolCall)] + assert calls, ( + f"{model} is declared supports_tool_choice but answered a required call " + "in prose; set it to false in LiteLLM" + ) + assert {c.tool for c in calls} <= OFFERED