From b8db564805d5124b8ad7fe3619a9b7b73da664ee Mon Sep 17 00:00:00 2001 From: Rok Benko Date: Mon, 14 Sep 2026 15:58:32 +0200 Subject: [PATCH 1/3] feat(providers): a body field the server wants and quackd never sends Every request an OpenAI-compatible provider sends is built from a fixed set of keys, so a field quackd has no name for cannot be sent at all. Qwen3 on vLLM needs one: thinking is on unless the request body says otherwise, and the switch is a chat template argument rather than a sampling parameter, so it cannot be set at serve time on a server somebody else runs. One step of find-and-kick spent 150 seconds and 1717 output tokens deliberating (#12). `extra_body` is the SDK's own door for this. It merges the object into the top level of the request, so this is a passthrough rather than a new code path, and it reaches every provider that speaks OpenAI's API: nine of the eleven cloud vendors and all five local presets. Anthropic and Gemini have other SDKs and other knobs, and ignore it as they already ignore --base-url. Sent on both APIs. `step` moves a run from Chat Completions to Responses when the API asks it to, and a knob that quietly stops working halfway through a run is worse than one the server ignores. Six keys are refused rather than passed: model, messages, input, instructions, tools, stream. `model` would put a model on the wire that run_start does not name, and `stream` changes the shape of the reply without telling the SDK, which then fails on the content type long after the robot has connected. The other four are the conversation, and `instructions` is the one that is easy to miss: on Chat Completions the system prompt is the first of `messages`, but on Responses it is a field of its own, so leaving it out would have let a passthrough quietly replace the whole contract on one API and not the other. Everything else goes through, tool_choice and reasoning_effort included, because overriding those is the point. run_start records what was sent. A run whose model was told not to think reads nothing like one that was, and the transcript is the only place a reader can tell which of the two they are holding. Refs #12 Co-Authored-By: Claude Opus 5 (1M context) --- quackd/agent/loop.py | 4 ++ quackd/agent/providers/factory.py | 41 ++++++++++- quackd/agent/providers/local.py | 2 + quackd/agent/providers/openai.py | 63 +++++++++++++++++ tests/conftest.py | 10 +++ tests/test_catalogue.py | 1 + tests/test_local_provider.py | 23 +++++- tests/test_loop.py | 25 +++++++ tests/test_providers.py | 113 +++++++++++++++++++++++++++++- 9 files changed, 276 insertions(+), 6 deletions(-) diff --git a/quackd/agent/loop.py b/quackd/agent/loop.py index 5eb0495..9f183fe 100644 --- a/quackd/agent/loop.py +++ b/quackd/agent/loop.py @@ -462,6 +462,10 @@ async def run(self) -> RunResult: duck_path=self.duck.path, provider=cfg.provider.name, model=cfg.provider.model, + # Fields a passthrough added to every request (#12). A run whose model was told not + # to think reads very differently from one that was, and the transcript is the only + # place a reader can tell which they are holding. + extra_body=getattr(cfg.provider, "extra_body", None), transport=backend_name(cfg.transport), adapter=adapter_name(cfg.transport), robot=manifest.model_dump(mode="json") if manifest is not None else None, diff --git a/quackd/agent/providers/factory.py b/quackd/agent/providers/factory.py index 0e87af7..f23f330 100644 --- a/quackd/agent/providers/factory.py +++ b/quackd/agent/providers/factory.py @@ -13,6 +13,7 @@ import importlib import os +from typing import Any from quackd.agent.providers.base import LLMProvider, ProviderError from quackd.agent.providers.catalogue import CATALOGUE as CATALOGUE @@ -136,6 +137,17 @@ def resolve_model(provider: str, model: str | None, *, source: str = "--model") raise ProviderError(_unknown_model(provider, model, source)) +def _extra_body(text: str | None) -> dict[str, Any] | None: + """`--extra-body` as the dict a provider takes, parsed here so a bad value names the flag + and stops before a key is read or a packet is sent. None hands the provider nothing, and it + reads `QUACKD_EXTRA_BODY` itself: that is how the flag outranks the variable.""" + if text is None: + return None + from quackd.agent.providers.openai import parse_extra_body + + return parse_extra_body(text, source="--extra-body") + + def make_provider( name: str, *, @@ -145,8 +157,14 @@ def make_provider( base_url: str | None = None, api_key: str | None = None, vision: bool | None = None, + extra_body: str | None = None, ) -> LLMProvider: name = name.lower() + # Before the branches, so a typo is refused the same way whichever provider was named, + # `fake` included. Anthropic and Gemini ignore the value as they ignore `--base-url`, + # but ignoring a field is not the same as swallowing a mistake, and `--provider fake` + # is then the cheapest way to find out whether a shell mangled the quoting. + body = _extra_body(extra_body) if name == "fake": from quackd.agent.providers.fake import FakeProvider @@ -163,7 +181,13 @@ def make_provider( if name == "openai": from quackd.agent.providers.openai import OpenAIProvider - return OpenAIProvider(model=model, api_key=api_key, base_url=base_url, vision=vision) + return OpenAIProvider( + model=model, + api_key=api_key, + base_url=base_url, + vision=vision, + extra_body=body, + ) if name == "gemini": from quackd.agent.providers.gemini import GeminiProvider @@ -172,11 +196,22 @@ def make_provider( module = importlib.import_module(f"quackd.agent.providers.{name}") vendor = getattr(module, OPENAI_COMPATIBLE[name]) provider: LLMProvider = vendor( - model=model, api_key=api_key, base_url=base_url, vision=vision + model=model, + api_key=api_key, + base_url=base_url, + vision=vision, + extra_body=body, ) return provider if name in LOCAL_NAMES: from quackd.agent.providers.local import LocalProvider - return LocalProvider(model, preset=name, base_url=base_url, api_key=api_key, vision=vision) + return LocalProvider( + model, + preset=name, + base_url=base_url, + api_key=api_key, + vision=vision, + extra_body=body, + ) raise ProviderError(f"unknown provider {name!r}; choose one of {', '.join(PROVIDER_NAMES)}") diff --git a/quackd/agent/providers/local.py b/quackd/agent/providers/local.py index 42a473f..8e8e0a5 100644 --- a/quackd/agent/providers/local.py +++ b/quackd/agent/providers/local.py @@ -122,6 +122,7 @@ def __init__( api_key: str | None = None, tool_choice: str | None = None, vision: bool | None = None, + extra_body: dict[str, Any] | None = None, ) -> None: if preset not in PRESETS: raise ProviderError(f"unknown local preset {preset!r}; one of {', '.join(LOCAL_NAMES)}") @@ -148,6 +149,7 @@ def __init__( base_url=url, tool_choice=choice, vision=vision, + extra_body=extra_body, ) self.text_fallbacks = 0 diff --git a/quackd/agent/providers/openai.py b/quackd/agent/providers/openai.py index 5958e50..6a12c46 100644 --- a/quackd/agent/providers/openai.py +++ b/quackd/agent/providers/openai.py @@ -289,6 +289,53 @@ def parse_response(response: Any) -> ProviderTurn: ) +REFUSED_EXTRA_BODY_KEYS = frozenset( + {"model", "messages", "input", "instructions", "tools", "stream"} +) +"""Keys quackd owns, and will not let a passthrough replace. `model` would walk past the +catalogue and put a model on the wire that `run_start` does not name. `messages`, `input`, +`instructions` and `tools` are the conversation itself, and `instructions` is the one that is +easy to miss: on Chat Completions the system prompt is the first of `messages`, but on +Responses it is a field of its own, so leaving it out would have let a passthrough quietly +replace the whole contract on one API and not the other. `stream` changes the shape of the +reply without telling the SDK, which then fails on the content type long after the robot has +connected. Everything else goes through, `tool_choice` and `reasoning_effort` included: +overriding those is the point.""" + +_EXTRA_BODY_EXAMPLE = '{"chat_template_kwargs": {"enable_thinking": false}}' + + +def parse_extra_body(text: str | None, *, source: str) -> dict[str, Any] | None: + """`--extra-body` or `QUACKD_EXTRA_BODY` as the object the SDK merges into the request. + + One JSON object and nothing else: a list or a bare string has no top level to merge into. + The error names the flag or the variable and echoes what arrived, because python-dotenv + truncates an unquoted value at a ` #` and drops a double-quoted one entirely, and both + look fine in the file. Empty is unset, so a `.env` line can be blanked rather than deleted, + and `--extra-body '{}'` silences one for a single run. + """ + if text is None or not text.strip(): + return None + try: + body = json.loads(text) + except json.JSONDecodeError as e: + raise ProviderError( + f"{source}: not valid JSON ({e.msg} at column {e.colno}) in {text.strip()[:60]!r}: " + f"one object, e.g. {_EXTRA_BODY_EXAMPLE}" + ) from e + if not isinstance(body, dict): + kind = "null" if body is None else type(body).__name__ + raise ProviderError( + f"{source}: a JSON object was expected, not {kind}: e.g. {_EXTRA_BODY_EXAMPLE}" + ) + for key in sorted(REFUSED_EXTRA_BODY_KEYS & body.keys()): + raise ProviderError( + f"{source}: {key!r} is quackd's to send and cannot be replaced here. " + f"Anything the server wants beside it can: e.g. {_EXTRA_BODY_EXAMPLE}" + ) + return body + + class OpenAIProvider: """OpenAI's own API. Subclasses (Grok, the local servers) only change the class knobs.""" @@ -318,6 +365,7 @@ def __init__( vision: bool | None = None, reasoning_effort: str | None = None, api: str | None = None, + extra_body: dict[str, Any] | None = None, ) -> None: # No model means the catalogue's default. The empty string is what the local presets # pass up, and it means the opposite: ask the server (`LocalProvider.ensure_model`). @@ -331,6 +379,17 @@ def __init__( self.reasoning_effort = reasoning_effort or _os.environ.get( "QUACKD_OPENAI_REASONING_EFFORT" ) + #: Fields the server wants and quackd never sends, merged into the top level of every + #: request body by the SDK's own `extra_body`, on either API. `{"chat_template_kwargs": + #: {"enable_thinking": false}}` is how Qwen3's thinking is turned off on vLLM (#12). + #: A dict from the caller wins over the environment, which is how `--extra-body` beats + #: `QUACKD_EXTRA_BODY`: the factory parses the flag and hands it down. An empty dict is + #: not None, so it skips the environment and sends nothing. + self.extra_body = ( + extra_body + if extra_body is not None + else parse_extra_body(_os.environ.get("QUACKD_EXTRA_BODY"), source="QUACKD_EXTRA_BODY") + ) #: "chat" or "responses". A model the catalogue marks `responses` starts there, which #: saves the failed call `step` would otherwise pay to learn it, and is the only way in #: for a model that is Responses only: its refusal is worded differently and @@ -383,6 +442,8 @@ def _params( params["parallel_tool_calls"] = False if self.reasoning_effort: params["reasoning_effort"] = self.reasoning_effort + if self.extra_body: + params["extra_body"] = self.extra_body return params def _params_responses( @@ -400,6 +461,8 @@ def _params_responses( params["parallel_tool_calls"] = False if self.reasoning_effort: params["reasoning"] = {"effort": self.reasoning_effort} + if self.extra_body: + params["extra_body"] = self.extra_body return params async def _call(self, system: str, history: list[Exchange], tools: list[dict[str, Any]]): diff --git a/tests/conftest.py b/tests/conftest.py index e4c7cb1..18863f7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -95,6 +95,16 @@ def _no_model_override(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("QUACKD_MODEL", "") +@pytest.fixture(autouse=True) +def _no_extra_body(monkeypatch: pytest.MonkeyPatch) -> None: + """`QUACKD_EXTRA_BODY` adds fields to every request an OpenAI-compatible provider sends, so + one line in a developer's `.env` would reach every test in this suite that reads a request + body, and the tests asserting a field is *absent* would fail on their machine and nowhere + else. Empty reads as unset, and unlike `delenv` it survives `load_dotenv`. The tests that + exercise the variable set it themselves.""" + monkeypatch.setenv("QUACKD_EXTRA_BODY", "") + + @pytest.fixture def registry() -> VerbRegistry: return default_registry() diff --git a/tests/test_catalogue.py b/tests/test_catalogue.py index 399e3ea..7601080 100644 --- a/tests/test_catalogue.py +++ b/tests/test_catalogue.py @@ -256,6 +256,7 @@ def _record(self: Any, model: str | None = None, **kwargs: Any) -> None: "base_url": "http://gpu:8000/v1", "api_key": None, "vision": True, + "extra_body": None, } diff --git a/tests/test_local_provider.py b/tests/test_local_provider.py index 0b6f796..701893c 100644 --- a/tests/test_local_provider.py +++ b/tests/test_local_provider.py @@ -107,6 +107,17 @@ async def test_tool_choice_none_omits_the_field(monkeypatch: pytest.MonkeyPatch) assert "tool_choice" not in client.kwargs +async def test_local_forwards_extra_body() -> None: + """The presets are where #12 came from: a vLLM server wants a field in the body and + `LocalProvider` has to carry it up to the base class, which is what sends it.""" + body = {"chat_template_kwargs": {"enable_thinking": False}} + client = FakeClient(reply(text="{}")) + await LocalProvider("m", preset="vllm", client=client, extra_body=body).step( + "S", history(), TOOLS + ) + assert client.kwargs["extra_body"] == body + + async def test_cloud_openai_keeps_strict_params() -> None: client = FakeClient(reply(tool_calls=[NS(id="c1", function=NS(name="kick", arguments="{}"))])) await OpenAIProvider("gpt-5", client=client).step("S", history(), TOOLS) @@ -227,20 +238,28 @@ def test_prompt_hint_only_for_local() -> None: def test_factory_builds_local_presets(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("quackd.agent.providers.local.LocalProvider.__init__", _record_init) - p = make_provider("vllm", model="Qwen/Qwen3-8B", base_url="http://gpu:8000/v1", vision=True) + p = make_provider( + "vllm", + model="Qwen/Qwen3-8B", + base_url="http://gpu:8000/v1", + vision=True, + extra_body='{"chat_template_kwargs": {"enable_thinking": false}}', + ) assert p.recorded == { # type: ignore[attr-defined] "model": "Qwen/Qwen3-8B", "preset": "vllm", "base_url": "http://gpu:8000/v1", "api_key": None, "vision": True, + # the flag arrives as text and the factory hands the provider the parsed object + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, } def _record_init(self: Any, model: Any = None, **kw: Any) -> None: self.recorded = { "model": model, - **{k: kw.get(k) for k in ("preset", "base_url", "api_key", "vision")}, + **{k: kw.get(k) for k in ("preset", "base_url", "api_key", "vision", "extra_body")}, } self.name = kw.get("preset", "local") self.model = model or "" diff --git a/tests/test_loop.py b/tests/test_loop.py index 53d981d..32cc7be 100644 --- a/tests/test_loop.py +++ b/tests/test_loop.py @@ -21,6 +21,31 @@ GOLDEN_HELLO = ["assess_task", "quack", "walk", "quack", "declare_success"] +async def test_run_start_records_the_extra_body(hello_duck: DuckFile, tmp_path: Path) -> None: + """A run whose model was told not to think reads nothing like one that was, and the + transcript is the only place a reader can tell which of the two they are holding. The + emit is a `getattr`, which would record None for ever if the attribute were renamed.""" + body = {"chat_template_kwargs": {"enable_thinking": False}} + provider = FakeProvider.for_duck(hello_duck.name) + provider.extra_body = body # type: ignore[attr-defined] + result = await run_duck( + RunConfig(duck=hello_duck, provider=provider, transport=MockTransport(), runs_dir=tmp_path) + ) + start = Transcript.read(result.run_dir / "transcript.jsonl")[0] + assert start["kind"] == "run_start" and start["extra_body"] == body + + # a provider with no such attribute records None rather than raising + plain = await run_duck( + RunConfig( + duck=hello_duck, + provider=FakeProvider.for_duck(hello_duck.name), + transport=MockTransport(), + runs_dir=tmp_path / "plain", + ) + ) + assert Transcript.read(plain.run_dir / "transcript.jsonl")[0]["extra_body"] is None + + async def test_hello_world_golden(hello_duck: DuckFile, tmp_path: Path) -> None: transport = MockTransport() result = await run_duck( diff --git a/tests/test_providers.py b/tests/test_providers.py index 6b74118..cff5aa6 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -4,6 +4,7 @@ import base64 import json +import re from collections.abc import Callable from types import SimpleNamespace as NS from typing import Any @@ -22,7 +23,7 @@ render_contents, ) from quackd.agent.providers.grok import GrokProvider -from quackd.agent.providers.openai import OpenAIProvider +from quackd.agent.providers.openai import OpenAIProvider, parse_extra_body from quackd.agent.providers.openai import render_messages as o_messages from quackd.verbs.registry import default_registry @@ -396,6 +397,116 @@ async def test_openai_reasoning_effort_can_be_set_by_hand() -> None: assert client.kwargs["reasoning_effort"] == "low" +# ── extra_body: a field the server wants and quackd never sends (#12) ──────────────────── + +BODY = {"chat_template_kwargs": {"enable_thinking": False}} +BODY_JSON = '{"chat_template_kwargs": {"enable_thinking": false}}' + + +async def test_extra_body_reaches_the_chat_body() -> None: + """Nothing is added unless it was asked for, and what was asked for arrives as the SDK's + own `extra_body`, which merges it into the top level of the request.""" + client = FakeOpenAI(openai_response("stop", "{}")) + await OpenAIProvider(model="gpt-5", client=client).step("SYS", history(), TOOLS) + assert "extra_body" not in client.kwargs + + await OpenAIProvider(model="gpt-5", client=client, extra_body=BODY).step( + "SYS", history(), TOOLS + ) + assert client.kwargs["extra_body"] == BODY + + +async def test_extra_body_survives_the_switch_to_responses(_no_effort_env: None) -> None: + """`step` moves a run from Chat Completions to Responses when the API asks for it, and the + passthrough has to still be there afterwards. A knob that quietly stops working halfway + through a run is worse than one the server ignores, which is why it is sent on both.""" + client = RefusesToolsOnChat(responses_result("walk", '{"vx": 0.2}')) + p = OpenAIProvider(model=UNHINTED, client=client, extra_body=BODY) + turn = await p.step("SYS", history(), TOOLS) + assert p.api == "responses" and turn.tool_calls[0].name == "walk" + assert client.chat_calls[0]["extra_body"] == BODY + assert client.responses_calls[0]["extra_body"] == BODY + + +async def test_extra_body_from_the_environment(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("QUACKD_EXTRA_BODY", BODY_JSON) + client = FakeOpenAI(openai_response("stop", "{}")) + await OpenAIProvider(model="gpt-5", client=client).step("SYS", history(), TOOLS) + assert client.kwargs["extra_body"] == BODY + + # blanked rather than deleted, which is what a commented-out `.env` line leaves behind + monkeypatch.setenv("QUACKD_EXTRA_BODY", " ") + client = FakeOpenAI(openai_response("stop", "{}")) + await OpenAIProvider(model="gpt-5", client=client).step("SYS", history(), TOOLS) + assert "extra_body" not in client.kwargs + + +async def test_the_flag_outranks_the_environment(monkeypatch: pytest.MonkeyPatch) -> None: + """The flag reaches a provider as the dict the factory parsed, so a dict beats the + variable. An empty one is not None, so it skips the variable and sends nothing: that is + how a `.env` line is silenced for a single run.""" + monkeypatch.setenv("QUACKD_EXTRA_BODY", '{"from": "the environment"}') + client = FakeOpenAI(openai_response("stop", "{}")) + await OpenAIProvider(model="gpt-5", client=client, extra_body=BODY).step( + "SYS", history(), TOOLS + ) + assert client.kwargs["extra_body"] == BODY + + client = FakeOpenAI(openai_response("stop", "{}")) + await OpenAIProvider(model="gpt-5", client=client, extra_body={}).step("SYS", history(), TOOLS) + assert "extra_body" not in client.kwargs + + +@pytest.mark.parametrize("bad", ["{not json", "[1, 2]", '"text"', "42", "null"]) +def test_extra_body_must_be_a_json_object(bad: str, monkeypatch: pytest.MonkeyPatch) -> None: + """Both doors refuse it, and each says which one it was: a flag somebody has just typed + and a line in a `.env` they have forgotten want different answers.""" + monkeypatch.setenv("QUACKD_EXTRA_BODY", bad) + with pytest.raises(ProviderError, match="QUACKD_EXTRA_BODY"): + OpenAIProvider(model="gpt-5", client=FakeOpenAI(None)) + + monkeypatch.setenv("QUACKD_EXTRA_BODY", "") + with pytest.raises(ProviderError, match="--extra-body"): + # the parse is what fails, before the SDK this machine does not have is imported + make_provider("vllm", model="m", base_url="http://gpu:8000/v1", extra_body=bad) + + +@pytest.mark.parametrize("provider", ["openai", "grok", "vllm"]) +def test_extra_body_reaches_every_provider_that_speaks_openais_api( + provider: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """The scope of this knob is the whole OpenAI-compatible family, and every other test of + it happens to use a local preset. Without this one the cloud branches of the factory could + stop forwarding it and nothing would go red.""" + seen: dict[str, Any] = {} + + def record(self: Any, *a: Any, **kw: Any) -> None: + seen.update(kw) + self.name, self.model, self.supports_vision = provider, "m", False + + monkeypatch.setattr(OpenAIProvider, "__init__", record) + make_provider(provider, model=None, base_url="http://host:8000/v1", extra_body=BODY_JSON) + assert seen["extra_body"] == BODY, f"{provider} was not handed the parsed object" + + +@pytest.mark.parametrize("key", ["model", "messages", "input", "instructions", "tools", "stream"]) +def test_extra_body_refuses_the_keys_quackd_owns(key: str) -> None: + """`model` would put a model on the wire that `run_start` does not name, and `stream` + changes the shape of the reply without telling the SDK, which then fails on the content + type long after the robot has connected.""" + with pytest.raises(ProviderError, match=re.escape(repr(key))): + make_provider( + "vllm", model="m", base_url="http://gpu:8000/v1", extra_body=json.dumps({key: "x"}) + ) + + +def test_extra_body_lets_you_override_what_quackd_sends() -> None: + """The refusal list is short on purpose. Replacing `tool_choice` is the escape hatch the + passthrough exists to be, and the SDK merges last, so it wins.""" + body = parse_extra_body('{"tool_choice": "auto"}', source="--extra-body") + assert body == {"tool_choice": "auto"} + + async def test_openai_bad_json_arguments_do_not_crash() -> None: p = OpenAIProvider(client=FakeOpenAI(openai_response("walk", "{not json"))) turn = await p.step("S", history()[:1], TOOLS) From 758e85f2d7c831007823c2e2d60095be1af3772a Mon Sep 17 00:00:00 2001 From: Rok Benko Date: Mon, 14 Sep 2026 15:58:44 +0200 Subject: [PATCH 2/3] feat(cli): --extra-body on run and record, and the flag beats the variable The variable alone would have been the cheaper change, and for most of the knobs in this file it is the whole story. This one earns a flag: it is per run rather than per machine, the value differs between two servers a person points at in the same afternoon, and the reporter in #12 is on Windows, where a JSON string in a `.env` file is easier to get right than one on a command line. Both are read, and the flag wins. The factory parses it and hands the provider the object, so a provider that was given one never looks at the environment. An empty object is not nothing, so `--extra-body '{}'` silences a `.env` line for a single run. Threaded through every path that builds a provider, which is more than the one that looks obvious: `run`, `record`, and both kinds of flock. A pilot flock builds one whole pilot per body and a coordinator flock builds one referee for all of them, and they are two different call sites, so the test walks all four routes rather than the one that would have looked like enough. A bad value stops the command before anything connects, and before the branch that would have ignored it. Parsing in one place rather than in the three branches that consume it means a typo is refused the same way whichever provider was named, which also makes `--provider fake` the cheapest way to find out whether a shell mangled the quoting. Each door names itself: a flag somebody has just typed and a line in a `.env` they have forgotten want different answers. Refs #12 Co-Authored-By: Claude Opus 5 (1M context) --- quackd/cli.py | 21 ++++++++++++++ tests/test_cli.py | 70 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/quackd/cli.py b/quackd/cli.py index 7c45709..2ab864e 100644 --- a/quackd/cli.py +++ b/quackd/cli.py @@ -624,6 +624,7 @@ def _run_impl( base_url: str | None = None, api_key: str | None = None, vision: bool | None = None, + extra_body: str | None = None, flock: str | None = None, *, robot: str | None = None, @@ -769,6 +770,7 @@ def _run_impl( base_url=base_url, api_key=api_key, vision=vision, + extra_body=extra_body, max_steps=max_steps, fov_deg=fov_deg, memory=memory, @@ -815,6 +817,7 @@ def _run_impl( base_url=base_url, api_key=api_key, vision=vision, + extra_body=extra_body, n_override=flock_n, max_steps=max_steps, trace=trace, @@ -831,6 +834,7 @@ def _run_impl( base_url=base_url, api_key=api_key, vision=vision, + extra_body=extra_body, ) duck_transport = make_adapter( spec, @@ -1032,6 +1036,7 @@ def _run_pilots_impl( base_url: str | None, api_key: str | None, vision: bool | None, + extra_body: str | None, max_steps: int | None, fov_deg: float | None, memory: bool, @@ -1063,6 +1068,7 @@ def _run_pilots_impl( base_url=base_url, api_key=api_key, vision=vision, + extra_body=extra_body, ) for name, entry in roster.items() } @@ -1231,6 +1237,7 @@ def _run_flock_impl( base_url: str | None, api_key: str | None, vision: bool | None, + extra_body: str | None, n_override: int | None, max_steps: int | None, trace: bool | None = None, @@ -1284,6 +1291,7 @@ def _run_flock_impl( base_url=base_url, api_key=api_key, vision=vision, + extra_body=extra_body, ) except (ProviderError, ImportError) as e: _fail(str(e)) @@ -1512,6 +1520,15 @@ def _complete_model(ctx: typer.Context, incomplete: str) -> list[tuple[str, str] help="API key override (local servers do not need one).", rich_help_panel="Model", ) +_EXTRA_BODY = typer.Option( + None, + "--extra-body", + help="A JSON object merged into every request body on the OpenAI-compatible providers, for " + "a field the server wants and quackd never sends. Qwen3 on vLLM stops thinking with " + '\'{"chat_template_kwargs": {"enable_thinking": false}}\'. QUACKD_EXTRA_BODY does the ' + "same when the flag is absent, and spares you the shell quoting.", + rich_help_panel="Model", +) _VISION = typer.Option( None, "--vision/--no-vision", @@ -1663,6 +1680,7 @@ def run( base_url: str | None = _BASEURL, api_key: str | None = _APIKEY, vision: bool | None = _VISION, + extra_body: str | None = _EXTRA_BODY, flock: str | None = _FLOCK, memory: bool = _MEMORY, memory_dir: str | None = _MEMORY_DIR, @@ -1692,6 +1710,7 @@ def run( base_url=base_url, api_key=api_key, vision=vision, + extra_body=extra_body, flock=flock, robot=robot, robots=robots, @@ -1717,6 +1736,7 @@ def record( base_url: str | None = _BASEURL, api_key: str | None = _APIKEY, vision: bool | None = _VISION, + extra_body: str | None = _EXTRA_BODY, flock: str | None = typer.Option( None, "--flock", @@ -1755,6 +1775,7 @@ def record( base_url=base_url, api_key=api_key, vision=vision, + extra_body=extra_body, flock=flock, robot="microduck:sim2d", trace=trace, diff --git a/tests/test_cli.py b/tests/test_cli.py index 6b3028c..a26cdf5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -778,8 +778,76 @@ def _help(argv: list[str]) -> str: return " ".join(re.sub(r"\[[0-9;]*m", "", out).split()) +@pytest.mark.parametrize( + "argv", + [ + ["run", "hello-world", "--robot", "microduck:mock", "--no-gif"], + ["record", "hello-world", "--gif-size", "64"], + ["run", "flock-hello", "--no-gif"], + ["run", "flock-kick", "--no-gif"], + ], + ids=["run", "record", "pilot-flock", "coordinator-flock"], +) +def test_extra_body_reaches_every_provider_a_run_builds( + argv: list[str], tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A flock builds one provider per member, so a flag that only reached the single-robot + path would be a silent no-op on the very runs that cost the most. The two kinds of flock + are two call sites: `flock-hello` is `method: pilots` and goes through `_run_pilots_impl`, + `flock-kick` is `method: auction` and goes through `_run_flock_impl`.""" + body = '{"chat_template_kwargs": {"enable_thinking": false}}' + seen: list[object] = [] + real = __import__("quackd.agent.providers.factory", fromlist=["make_provider"]).make_provider + + def recorder(name: str, **kw: object) -> object: + seen.append(kw.get("extra_body")) + return real("fake", duck_name=kw.get("duck_name")) # type: ignore[arg-type] + + monkeypatch.setattr("quackd.agent.providers.factory.make_provider", recorder) + result = runner.invoke( + app, [*argv, "--provider", "fake", "--runs-dir", str(tmp_path / "r"), "--extra-body", body] + ) + assert result.exit_code == 0, result.output + assert seen, "no provider was built" + assert all(s == body for s in seen), "the factory is handed the text as it was typed" + # a pilot flock is one whole pilot per body; a coordinator flock is one referee for all + # of them, so only the first should build more than one provider + if "flock-hello" in argv: + assert len(seen) > 1, "a pilot flock is one provider per member" + + +def test_a_bad_extra_body_stops_before_anything_connects( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The parse is in the factory, and the factory runs before the adapter, so the run + directory is never made. Each door names itself: a flag somebody has just typed and a + line in a `.env` they have forgotten want different answers.""" + runs = tmp_path / "r" + common = [ + "run", + "hello-world", + "--provider", + "vllm", + "--robot", + "microduck:mock", + "--no-gif", + "--runs-dir", + str(runs), + "--memory-dir", + str(tmp_path / "m"), + ] + result = runner.invoke(app, [*common, "--extra-body", "[1]"]) + assert result.exit_code == 1 + assert "--extra-body" in result.output and "Traceback" not in result.output + assert not runs.exists() or not list(runs.iterdir()) + + monkeypatch.setenv("QUACKD_EXTRA_BODY", "[1]") + result = runner.invoke(app, common) + assert result.exit_code == 1 and "QUACKD_EXTRA_BODY" in result.output + + def test_the_help_groups_the_flags_and_keeps_the_brackets_of_an_extra() -> None: - """Twenty seven flags in one flat list is a list nobody reads. And `rich_markup_mode` + """Twenty eight flags in one flat list is a list nobody reads. And `rich_markup_mode` reads `quackd[lan]` as markup, which printed an install that does not exist.""" out = _help(["run", "--help"]) assert "quackd[live]" in out, "an extra a reader is meant to type must survive" From e71451ea79752b06f7b8d6f44aced888f78668e4 Mon Sep 17 00:00:00 2001 From: Rok Benko Date: Mon, 14 Sep 2026 16:01:15 +0200 Subject: [PATCH 3/3] docs: where --extra-body is configured, and the Qwen3 line that asked for it A knob nobody can find is a knob nobody has, and this one is harder to find by reading the source than most: the field it carries belongs to the server, so the name somebody would search for is never in this repository at all. The Knobs table gains a row, and the sentence about what is never sent to local servers now says what can be. The vLLM section gets the case itself, with both ways to turn Qwen3's thinking off rather than one. The issue said this could not be fixed on the server side; vLLM's own --default-chat-template-kwargs does exactly that, once, at serve time. It is the better answer for a server you run yourself, and no answer at all for one you share, which is what --extra-body is for. Saying only the second would have been a smaller truth than the reader needs. Two traps are written down because both were found by running them rather than by reading. No single spelling of a JSON string survives bash, PowerShell 5.1 and cmd.exe, so the docs show the shells and then show the .env line that sidesteps all three. And in that file the quoting is not cosmetic: double quotes around JSON make python-dotenv drop the line and set nothing, so a run reads as though the line had never been written. The browser demo has no such door, and web/README.md keeps the canonical list of what it does not have, so it is recorded there rather than left to be discovered. docs/architecture.md's run_start row names the new field, because that table is what a reader checks a transcript against. The guard is the house shape: one test naming every file that has to mention it. Closes #12 Co-Authored-By: Claude Opus 5 (1M context) --- .env.example | 5 +++++ CHANGELOG.md | 23 ++++++++++++++++++++++ README.md | 2 +- docs/architecture.md | 2 +- docs/faq.md | 6 ++++-- docs/local-llms.md | 45 ++++++++++++++++++++++++++++++++++++++++++-- tests/test_docs.py | 26 +++++++++++++++++++++++++ web/README.md | 4 ++++ 8 files changed, 107 insertions(+), 6 deletions(-) diff --git a/.env.example b/.env.example index 5e0baa1..d4397d7 100644 --- a/.env.example +++ b/.env.example @@ -33,6 +33,11 @@ META_API_KEY= # LOCAL_API_KEY=not-needed # QUACKD_TOOL_CHOICE=auto # auto (default) | required | none (omit the field) # QUACKD_VISION=0 # 1 to send camera frames to a local vision model +# A JSON object merged into every request body, for a field the server wants and quackd +# never sends: this one turns Qwen3's thinking off on vLLM. Every provider that speaks +# OpenAI's API reads it, and --extra-body beats it. Single quotes or none, never double +# quotes, which python-dotenv drops without setting anything. +# QUACKD_EXTRA_BODY='{"chat_template_kwargs": {"enable_thinking": false}}' # Optional: override the model for the chosen provider. A cloud provider takes an id from # quackd's catalogue and refuses anything else before it calls out, so `quackd list-models` diff --git a/CHANGELOG.md b/CHANGELOG.md index db53d05..f3471d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **A body field the server wants and quackd never sends: `--extra-body` and + `QUACKD_EXTRA_BODY`.** One JSON object, merged into the top of every request body on every + provider that speaks OpenAI's API, which is nine of the eleven cloud vendors and all five + local presets, and sent on Chat Completions and Responses both, so it keeps working when a + run moves from one to the other mid-flight. The case that asked for it: Qwen3 on vLLM thinks + before it answers unless the request body says `{"chat_template_kwargs": {"enable_thinking": + false}}`, and that switch is a chat template argument rather than a sampling parameter, so on + a server somebody else runs there was nowhere to say it. One reported step spent 150 s and + 1717 output tokens deliberating before a decision that was correct anyway. The flag beats the + variable, an empty object sends nothing, and a value that is not one JSON object is refused + before a robot is connected, naming the flag or the variable it came from. Six keys are + refused because they are quackd's to send — `model`, `messages`, `input`, `instructions`, + `tools` and `stream`, the fourth being the system prompt on Responses the way the second is + on Chat Completions — and everything else replaces what quackd would have sent, `tool_choice` + included, because overriding it is the point. `run_start` records the object, since a run + whose model was told not to think reads nothing like one that was. Anthropic and Gemini + ignore it, as they already ignore `--base-url`. It is in `.env.example` with the others, + where single quotes matter: double ones make python-dotenv drop the line without a word. If + you run the server yourself, vLLM's own `--default-chat-template-kwargs` does the same thing + once at serve time, and the docs now name both. Thanks to + [@Vallhalen](https://github.com/Vallhalen) (#12), who measured it and proposed the + passthrough ([docs/local-llms.md](docs/local-llms.md#knobs)). + - **A bring-up checklist and a lookout task for the LeRobot arm, which were the last two missing.** Every other experimental backend had both; the arm had neither, and this file has said so since 0.7. [docs/lerobot-hardware-checklist.md](docs/lerobot-hardware-checklist.md) diff --git a/README.md b/README.md index 124e50c..b9ce88f 100644 --- a/README.md +++ b/README.md @@ -583,7 +583,7 @@ browser test. | Model | `--model` or `QUACKD_MODEL`, an id from the catalogue. An id a cloud vendor does not list is refused before any call, and the refusal prints the ids that vendor does take. `quackd list-models` prints them all. The defaults are `claude-opus-5`, `gpt-5.6-sol`, `gemini-3.8-flash`, `grok-4.6`, `mistral-medium-3-5`, `deepseek-flash`, `command-a-plus-05-2026`, `qwen3.8-max`, `kimi-k3`, `glm-5.3` and `muse-spark-1.3` | | Claude reasoning effort | `QUACKD_EFFORT` (`low` to `max`, default `medium`). `QUACKD_ANTHROPIC_FALLBACKS=0` disables server side refusal fallbacks. `QUACKD_THINKING_DISPLAY=omitted` stops Claude returning a summary of its reasoning, and `QUACKD_GEMINI_THOUGHTS=0` does the same for Gemini | | OpenAI API and effort | `QUACKD_OPENAI_API=responses` opens on the Responses API instead of Chat Completions, and `QUACKD_OPENAI_REASONING_EFFORT` sets the effort on either. Neither is usually needed: quackd already knows which models want Responses, and moves a run there by itself when one says so ([FAQ](docs/faq.md)) | -| Local models | `--provider ollama`, `vllm`, `llamacpp`, `lmstudio` or `local --base-url http://host:port/v1`. No key. `--model` takes any id the server serves, and without it quackd uses the first model the server lists. The catalogue is for cloud vendors only, so nothing here is refused for being unlisted. `--vision` sends frames. `QUACKD_TOOL_CHOICE=auto`, `required` or `none` for picky servers. See [docs/local-llms.md](docs/local-llms.md) | +| Local models | `--provider ollama`, `vllm`, `llamacpp`, `lmstudio` or `local --base-url http://host:port/v1`. No key. `--model` takes any id the server serves, and without it quackd uses the first model the server lists. The catalogue is for cloud vendors only, so nothing here is refused for being unlisted. `--vision` sends frames. `QUACKD_TOOL_CHOICE=auto`, `required` or `none` for picky servers. `--extra-body` or `QUACKD_EXTRA_BODY` merges a JSON object into every request body, which is how Qwen3 is told not to think on vLLM, and it works on every vendor that speaks OpenAI's API. See [docs/local-llms.md](docs/local-llms.md) | | Robot | `--robot :` or a name from `quackd robot add`, or a `robots:` line in the `.duck`, the flag wins. Default `microduck:sim2d`. `quackd list-adapters` lists the seven that ship, `quackd list-verbs --robot X` what each can do | | Physics simulator | `--robot microduck:mujoco`, with `quackd[mujoco]`. The model and the policies are fetched once into `~/.quackd/cache`, where `QUACKD_CACHE_DIR` moves them and `QUACKD_MICRODUCK_ASSETS` points at your own `microduck_rl` checkout instead. `QUACKD_MUJOCO_BODY=puppet` runs the kinematic stand-in, which downloads nothing and is the body the tests build. `--live` opens MuJoCo's own viewer | | Determinism | `--seed N` makes a simulator run repeatable | diff --git a/docs/architecture.md b/docs/architecture.md index f5faaee..03f5b61 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -132,7 +132,7 @@ One JSON object per line: `{"t": seconds, "kind": ..., ...}`. | Kind | What it records | |---|---| -| `run_start` | contract, system prompt, tool names, robot manifest, how long connecting took | +| `run_start` | contract, system prompt, tool names, robot manifest, any `extra_body` sent with every request, how long connecting took | | `observation` | what the model was shown this turn, and how long gathering it took | | `llm_request` | how many messages went out, how many still carry an image, whether this is the re-prompt | | `llm` | text, `thinking`, tool_calls, usage (this turn and the run's total), stop_reason, latency, or `error` when the call failed | diff --git a/docs/faq.md b/docs/faq.md index da2b040..f04a817 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -120,7 +120,8 @@ is more than one to choose from: Qwen goes to DashScope's *international* endpoi (`dashscope-intl.aliyuncs.com`), so a key issued on Alibaba's China console will not authenticate, and Cohere goes to its OpenAI compatibility path rather than its native one. `--base-url` moves any vendor that speaks OpenAI's API, which is every one of them except -Anthropic and Gemini, where the flag is accepted and ignored. +Anthropic and Gemini, where the flag is accepted and ignored. `--extra-body` adds a field +that vendor wants and quackd never sends, on the same terms. **Why did my OpenAI run move to a different API mid-flight?** Because some OpenAI models refuse function tools on `/v1/chat/completions` at every reasoning effort and name `/v1/responses` in @@ -137,7 +138,8 @@ OpenAI's Chat Completions API, so `--provider ollama`, `vllm`, `llamacpp`, `lmst `local --base-url http://host:port/v1` works with no API key. Tool calling must be enabled on the server (`llama-server --jinja`, `vllm serve --enable-auto-tool-choice --tool-call-parser …`), vision is off unless you pass `--vision`, and a small model that -writes its tool call as plain JSON is still understood. Details: [local-llms.md](local-llms.md). +writes its tool call as plain JSON is still understood. A field the server wants in the +body goes in with `--extra-body`, which is how Qwen3 is told not to think on vLLM. Details: [local-llms.md](local-llms.md). ## Seeing what happened diff --git a/docs/local-llms.md b/docs/local-llms.md index 1d86a26..cc9a98c 100644 --- a/docs/local-llms.md +++ b/docs/local-llms.md @@ -58,6 +58,33 @@ quackd run find-and-kick --provider vllm --model Qwen/Qwen3-8B The `--tool-call-parser` value depends on the model family (`hermes` for Qwen and Hermes models, `llama3_json` for Llama 3.x, `mistral` for Mistral). vLLM's docs list the pairs. +Qwen3 thinks before it answers unless the request says otherwise, and the switch is a chat +template argument rather than a sampling parameter. One reported step of `find-and-kick` spent +150 s and 1717 output tokens on the reasoning before deciding (#12). There are two places to +turn it off. On a server you run yourself, do it once at serve time: + +```bash +vllm serve Qwen/Qwen3-8B --enable-auto-tool-choice --tool-call-parser hermes \n --reasoning-parser qwen3 --default-chat-template-kwargs '{"enable_thinking": false}' +``` + +On a server somebody else runs, or when you want it per run, send it with the request: + +```bash +quackd run find-and-kick --provider vllm --model Qwen/Qwen3-8B \n --extra-body '{"chat_template_kwargs": {"enable_thinking": false}}' +``` + +That flag is a JSON string, and no single spelling of one survives every shell: the line above +is for bash, PowerShell 5.1 wants `'{\"chat_template_kwargs\": {\"enable_thinking\": false}}'`, +and `cmd.exe` wants the whole thing in double quotes with the inner ones escaped. The way round +all of it is a line in `.env`, which every shell leaves alone: + +``` +QUACKD_EXTRA_BODY='{"chat_template_kwargs": {"enable_thinking": false}}' +``` + +Single quotes there, or none. Double quotes around JSON make python-dotenv drop the variable +without setting it, and the run then thinks out loud as though you had never written the line. + **LM Studio** Developer tab → Start Server (default port 1234), load a model that supports tools, then @@ -93,8 +120,22 @@ servers reject image parts. The text observation already carries what the camera | `--api-key` / `LOCAL_API_KEY` | any string | `not-needed` (servers ignore it) | | `QUACKD_TOOL_CHOICE` | `auto`, `required`, `none` | `auto` (`none` omits the field for servers that reject it) | | `--vision` / `QUACKD_VISION` | on, off | off | - -`parallel_tool_calls` is never sent to local servers, because some reject unknown fields. +| `--extra-body` / `QUACKD_EXTRA_BODY` | one JSON object, merged into the top of every request body | nothing extra is sent | + +`parallel_tool_calls` is never sent to local servers, because some reject unknown fields, and +nothing else is added unless `--extra-body` asks for it. + +`--extra-body` works on every provider that speaks OpenAI's API, which is nine of the eleven +cloud vendors and all five local presets, and on Chat Completions and Responses alike, so it +keeps working when a run moves from one to the other. The flag beats the variable, and an empty +object sends nothing, which is how a `.env` line is silenced for a single run. Six keys are +refused because they are quackd's to send: `model`, `messages`, `input`, `instructions`, +`tools` and `stream`. The odd one there is `instructions`, which is the system prompt on the +Responses API the way `messages` carries it on Chat Completions. Everything else replaces what +quackd would have sent, `tool_choice` included, because overriding it is the point. That cuts +both ways: `n` or `response_format` will reach the server too, and what the model answers with +afterwards is yours to live with. In a flock the object goes to every member that speaks +OpenAI's API, and there is no per robot value in the registry. Add physics by asking for both extras and naming the backend: diff --git a/tests/test_docs.py b/tests/test_docs.py index 2be758b..ec6c3c7 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -254,6 +254,32 @@ def test_the_docs_name_every_gate_the_code_can_fire() -> None: assert not missing, f"architecture.md and mcp.md name no gate called: {missing}" +def test_extra_body_is_documented_where_it_is_configured() -> None: + """A knob nobody can find is a knob nobody has. This one is worse than most to discover by + reading the source, because the field it carries belongs to the server rather than to + quackd, so the name to search for is never in this repository at all.""" + for path, needles in ( + ("README.md", ("--extra-body", "QUACKD_EXTRA_BODY")), + ( + "docs/local-llms.md", + ( + "--extra-body", + "QUACKD_EXTRA_BODY", + "chat_template_kwargs", + # the serve-time way round, so the docs do not imply the client is the only one + "--default-chat-template-kwargs", + ), + ), + ("docs/faq.md", ("--extra-body",)), + (".env.example", ("QUACKD_EXTRA_BODY", "chat_template_kwargs")), + # the page has no such door, and its own list of differences is where that is recorded + ("web/README.md", ("extra_body",)), + ): + text = (REPO / path).read_text(encoding="utf-8") + for needle in needles: + assert needle in text, f"{path} does not mention {needle!r}" + + def test_the_trace_is_documented_where_it_is_configured() -> None: for path, needles in ( ( diff --git a/web/README.md b/web/README.md index 5ece50d..858c9c1 100644 --- a/web/README.md +++ b/web/README.md @@ -313,6 +313,10 @@ Deliberately, and none of it is a bug. This list is the canonical one: `README.m - **A seed means the same distributions, not the same layout.** The arena here is laid out by a xorshift and in Python by numpy's PCG64. The spawn ranges and the rejection rules match; the stream does not, so seed 3 is a different arena in each. +- **No `extra_body`.** The CLI can merge a JSON object into every request body, with + `--extra-body` or `QUACKD_EXTRA_BODY`, for a field a server wants and quackd never sends. + This page has no such door, so a model that has to be told something in the body, Qwen3 and + its thinking being the one that came up, cannot be told it here. - **There is no scripted pilot.** Python's `--provider fake` walks the whole task with no model. Here the pre-filled goal still needs a key, or a local server, before anything happens.