diff options
| -rw-r--r-- | CHANGELOG.md | 14 | ||||
| -rw-r--r-- | llamachat/backend.py | 27 | ||||
| -rw-r--r-- | llamachat/config.py | 7 | ||||
| -rw-r--r-- | llamachat/providers.py | 4 | ||||
| -rwxr-xr-x | test_llamachat.py | 74 |
5 files changed, 118 insertions, 8 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index d3df977..6e9a831 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `model`. Token counts come from the stream's final usage chunk and power the cost readout; the model column records which model actually replied, so a switched conversation prices each turn correctly. +- Per-provider `thinking_budget` option, for endpoints (currently SiliconFlow) + that cap chain-of-thought tokens separately from the final answer. Unset + providers skip the key entirely so other endpoints do not receive an unknown + parameter. + +### Fixed + +- Model listing crashed when a provider's `/v1/models` endpoint returned a + bare JSON array instead of the standard `{"data": [...]}` envelope. The + parser now accepts both shapes. +- Cloud chat requests no longer rely on the provider's default `max_tokens`, + which could be too small for a reasoning model. Non-local providers now send + `max_tokens = 32768`, leaving room for both chain-of-thought reasoning and + the answer. Tool calling and reasoning output vary between providers. Web search on a cloud model may not work as reliably as it does with a local router. diff --git a/llamachat/backend.py b/llamachat/backend.py index ca76559..4f15936 100644 --- a/llamachat/backend.py +++ b/llamachat/backend.py @@ -170,10 +170,19 @@ def build_user_content(text: str, attachments: list[Attachment]): class Client: """Minimal OpenAI-compatible client for the local router.""" - def __init__(self, base_url: str, timeout: int = 300, api_key: str = ""): + def __init__( + self, + base_url: str, + timeout: int = 300, + api_key: str = "", + is_local: bool = False, + thinking_budget: int | None = None, + ): self.base_url = base_url.rstrip("/") self.timeout = timeout self.api_key = api_key + self.is_local = is_local + self.thinking_budget = thinking_budget def _headers(self) -> dict: """Bearer auth when the provider needs it, nothing when it does not.""" @@ -200,7 +209,10 @@ class Client: raise BackendError(f"Cannot reach router at {self.base_url}: {exc}") except ValueError as exc: raise BackendError(f"Router sent invalid JSON: {exc}") - return [m["id"] for m in payload.get("data", []) if "id" in m] + # Some OpenAI-compatible providers return a bare array instead of + # the standard {"data": [...]} envelope. Accept both. + data = payload if isinstance(payload, list) else payload.get("data", []) + return [m["id"] for m in data if "id" in m] def complete(self, model: str, messages: list[dict], max_tokens: int = 48) -> str: """One short non-streaming reply, for side errands like titling. @@ -359,6 +371,15 @@ class Client: # estimate, at no extra request. "stream_options": {"include_usage": True}, } + # Cloud providers apply their own max_tokens default when none is + # sent, and that default can be small enough that a reasoning model + # spends it all on reasoning_content and never produces a content + # reply. The local router has no such ceiling, so only set this for + # non-local providers. + if not self.is_local: + body["max_tokens"] = 32768 + if self.thinking_budget is not None: + body["thinking_budget"] = self.thinking_budget if tools: body["tools"] = tools try: @@ -413,6 +434,8 @@ class MultiClient: provider.base_url, timeout=self.timeout, api_key=self.resolver.resolve(provider), + is_local=provider.is_local, + thinking_budget=provider.thinking_budget, ) return self._clients[name] diff --git a/llamachat/config.py b/llamachat/config.py index 4add5ed..81b799b 100644 --- a/llamachat/config.py +++ b/llamachat/config.py @@ -247,6 +247,12 @@ def write_default(path: Path = CONFIG_PATH) -> Path: '# dialog saves goes to models.ini beside this file, so none of\n' '# these has to be set here.\n' '#\n' + '# thinking_budget sets a provider-specific cap on chain-of-thought\n' + '# tokens. It is currently sent only to SiliconFlow; other endpoints\n' + '# ignore it when unset, so leaving it out is safe. SiliconFlow\'s\n' + '# default is 4096; raise it for complex questions that need more\n' + '# room to reason.\n' + '#\n' '# [providers.together]\n' '# base_url = "https://api.together.xyz"\n' '# api_key = "pass:api/together"\n' @@ -254,6 +260,7 @@ def write_default(path: Path = CONFIG_PATH) -> Path: '# ctx_size = 32768\n' '# price_in = 0.60\n' '# price_out = 0.60\n' + '# thinking_budget = 8192\n' ) return path diff --git a/llamachat/providers.py b/llamachat/providers.py index ce2d01d..a8b2350 100644 --- a/llamachat/providers.py +++ b/llamachat/providers.py @@ -46,6 +46,9 @@ class Provider: vision: bool | None = None price_in: float | None = None price_out: float | None = None + # Provider-specific reasoning token budget. Currently SiliconFlow only; + # ignored when unset so other endpoints do not receive an unknown key. + thinking_budget: int | None = None @property def is_local(self) -> bool: @@ -185,6 +188,7 @@ def parse(values: dict, warnings: list[str] | None = None) -> dict[str, Provider vision=None if vision is None else bool(vision), price_in=_number(entry.get("price_in"), float), price_out=_number(entry.get("price_out"), float), + thinking_budget=_number(entry.get("thinking_budget"), int), ) return out diff --git a/test_llamachat.py b/test_llamachat.py index 98776b8..3f1f678 100755 --- a/test_llamachat.py +++ b/test_llamachat.py @@ -923,11 +923,12 @@ def test_provider_parsing(): "together": { "base_url": "https://api.example.org", "api_key": "env:TEST_KEY_NAME", - "filter": ["qwen", "deepseek"], - "ctx_size": 32768, - "price_in": 0.6, - "price_out": 0.9, - }, + "filter": ["qwen", "deepseek"], + "ctx_size": 32768, + "price_in": 0.6, + "price_out": 0.9, + "thinking_budget": 8192, + }, } } ) @@ -939,6 +940,7 @@ def test_provider_parsing(): assert parsed["together"].ctx_size == 32768 assert parsed["together"].price_in == 0.6 assert parsed["together"].price_out == 0.9 + assert parsed["together"].thinking_budget == 8192 # An old config: bare base_url, no providers table at all. legacy = providers.parse({"base_url": "http://localhost:8181"}) @@ -1395,12 +1397,14 @@ def test_config_providers(): "ctx_size = 32768\n" "price_in = 0.6\n" "price_out = 0.9\n" + "thinking_budget = 8192\n" ) cfg = config.load(path) assert set(cfg.providers) == {"local", "together"} assert cfg.providers["together"].api_key == "pass:api/together" assert cfg.providers["together"].filter == ["qwen"] assert cfg.providers["together"].price_out == 0.9 + assert cfg.providers["together"].thinking_budget == 8192 # A config with no base_url and no providers still loads, with the # built-in default synthesizing local. @@ -1863,6 +1867,13 @@ def test_client_auth_header(): ).models() assert sent["headers"]["Authorization"] == "Bearer sk-test-not-a-real-key" + # A provider that returns a bare array instead of {"data": [...]} + # must still parse: some OpenAI-compatible endpoints skip the envelope. + def _bare_list(url, timeout=None, headers=None): + return _HttpxResponse([{"id": "bare"}]) + with _patched(httpx, "get", _bare_list): + assert backend.Client("http://x.example.org").models() == ["bare"] + # The streaming path is the one that carries every real turn, and no # other test reaches it: the search tests all stub _stream_once out. class _Stream: @@ -1897,6 +1908,56 @@ def test_client_auth_header(): print("ok client authorization header") +def test_stream_body_for_cloud(): + """Cloud providers get max_tokens and thinking_budget; local does not.""" + captured = {} + + class _Stream: + status_code = 200 + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def iter_lines(self): + return iter( + ['data: {"choices":[{"delta":{"content":"hi"}}]}'] + ) + + def _recorder(method, url, json=None, timeout=None, headers=None): + captured["json"] = json + return _Stream() + + import httpx + with _patched(httpx, "stream", _recorder): + # Local client: no max_tokens, no thinking_budget. + local = backend.Client("http://localhost:8181", is_local=True) + list(local._stream_once("m", [], tools=None)) + assert "max_tokens" not in captured["json"] + assert "thinking_budget" not in captured["json"] + + # Cloud client with thinking_budget set. + cloud = backend.Client( + "http://api.example.org", + is_local=False, + thinking_budget=8192, + ) + list(cloud._stream_once("m", [], tools=None)) + assert captured["json"]["max_tokens"] == 32768 + assert captured["json"]["thinking_budget"] == 8192 + + # Cloud client without thinking_budget: max_tokens still sent. + cloud_no_budget = backend.Client( + "http://api.example.org", is_local=False + ) + list(cloud_no_budget._stream_once("m", [], tools=None)) + assert captured["json"]["max_tokens"] == 32768 + assert "thinking_budget" not in captured["json"] + print("ok stream body for cloud providers") + + def test_multi_client(): """Models fan out across providers; requests route by model id.""" from llamachat import providers @@ -1926,7 +1987,7 @@ def test_multi_client(): built = [] class _StubClient: - def __init__(self, base_url, timeout=300, api_key=""): + def __init__(self, base_url, timeout=300, api_key="", **kwargs): self.base_url = base_url self.api_key = api_key built.append(self) @@ -2820,6 +2881,7 @@ if __name__ == "__main__": test_metadata_and_cost() test_token_column_migration() test_client_auth_header() + test_stream_body_for_cloud() test_multi_client() test_model_dialog_values() test_cost_label_text() |