diff --git a/nisse-dev/src/nisse_dev/agent.py b/nisse-dev/src/nisse_dev/agent.py index 4ffa084..78522cf 100644 --- a/nisse-dev/src/nisse_dev/agent.py +++ b/nisse-dev/src/nisse_dev/agent.py @@ -19,7 +19,11 @@ from nisse_dev.executor import LLMExecutor from nisse_dev.expert import ExpertDesk from nisse_dev.git import GitRepo, authenticated_url from nisse_dev.gitea import Comment, GiteaClient -from nisse_dev.llm import AnthropicPassthroughClient, LiteLLMClient +from nisse_dev.llm import ( + AnthropicPassthroughClient, + LiteLLMClient, + select_client, +) from nisse_dev.matrix import MatrixTransport from nisse_dev.oplog import OpLog from nisse_dev.planner import LLMPlanningAgent @@ -98,19 +102,22 @@ def build_agent( workspace = Workspace(config.workspace) normalized = LiteLLMClient(config.llm_base_url, config.llm_token) - # Delegations take the passthrough route; fidelity is cheap elsewhere (A5). + # Delegations prefer the vendor-native route; fidelity is cheap + # elsewhere (A5). Which route a given model gets is decided by + # select_client, not by where the caller happens to sit. passthrough = AnthropicPassthroughClient(config.llm_base_url, config.llm_token) transport = MatrixTransport(config.matrix) announce = notify or transport.send_message expert = ExpertDesk(normalized, store, config.roles) + delegate_model = config.roles.resolve("delegate") delegation = Delegation( - passthrough, + select_client(delegate_model, normalized, passthrough), store, workspace, git_repo, - model=config.roles.resolve("delegate"), + model=delegate_model, ) executor = LLMExecutor( normalized, diff --git a/nisse-dev/src/nisse_dev/delegation.py b/nisse-dev/src/nisse_dev/delegation.py index aba2bbe..c79212c 100644 --- a/nisse-dev/src/nisse_dev/delegation.py +++ b/nisse-dev/src/nisse_dev/delegation.py @@ -36,7 +36,7 @@ from nisse_dev.canonical import ( ToolResult, ) from nisse_dev.git import GitError, GitRepo -from nisse_dev.llm import LLMClient, LLMError +from nisse_dev.llm import LLMClient, LLMError, UpstreamUnavailable from nisse_dev.reconcile import Reconciler from nisse_dev.state import Intent, StateStore from nisse_dev.tools import EXECUTOR_TOOLS, MUTATING_TOOLS, Workspace @@ -129,7 +129,7 @@ class Handback: @dataclass class DelegationResult: - status: Literal["handback", "capped", "failed"] + status: Literal["handback", "capped", "failed", "interrupted"] note: str handback: Handback | None = None branch: str | None = None @@ -137,6 +137,17 @@ class DelegationResult: tokens: int = 0 duration_s: float = 0.0 + @property + def interrupted(self) -> bool: + """Stopped by something external rather than by the delegate. + + The work on the branch may be perfectly good, so it is preserved and + the intent left pending: the next boot treats it exactly like a crash + mid-delegation (A3) and resumes if the cut was clean. Discarding here + would throw away work because the budget ran out. + """ + return self.status == "interrupted" + @property def usable(self) -> bool: """Only a successful handback may be built upon. A cap breach or a @@ -212,6 +223,11 @@ class Delegation: exit_status=result.status, ) ) + if result.interrupted: + # Leave the branch and the intent alone: this delegation is not + # finished, it was cut off, and A3's resume rule is exactly the + # machinery for picking it back up. + return result if not result.usable: # Abort discards the branch: the caller must never resume on top # of a partially-edited tree (A13). @@ -294,6 +310,7 @@ class Delegation: ) touched: set[str] = set() did_work = False + spent = 0 for turn in range(self._caps.turns): elapsed = self._now() - started_at @@ -304,6 +321,7 @@ class Delegation: f"after {turn} turns", branch=branch, artifacts=sorted(touched), + tokens=spent, ) try: produced = self._client.complete( @@ -312,9 +330,35 @@ class Delegation: model=self._model, system=system, ) + except UpstreamUnavailable as exc: + return DelegationResult( + "interrupted", + f"stopped by something upstream, not by the work: {exc}", + branch=branch, + artifacts=sorted(touched), + tokens=spent, + ) except LLMError as exc: return DelegationResult( - "failed", f"delegate call failed: {exc}", branch=branch + "failed", + f"delegate call failed: {exc}", + branch=branch, + tokens=spent, + ) + # Metered from what the provider reported, not estimated. The cap + # is a scope limit (A13), distinct from the money cap A5 keeps on + # the LiteLLM key — but an unbounded delegation can still exhaust + # a daily budget and take external models away until it resets. + usage = getattr(self._client, "last_usage", {}) or {} + spent += int(usage.get("input", 0)) + int(usage.get("output", 0)) + if spent >= self._caps.tokens: + return DelegationResult( + "capped", + f"token cap of {self._caps.tokens} reached ({spent} spent) " + f"after {turn + 1} turns", + branch=branch, + artifacts=sorted(touched), + tokens=spent, ) calls: list[ToolCall] = [] @@ -336,6 +380,7 @@ class Delegation: "delegate produced neither a tool call nor text", branch=branch, artifacts=sorted(touched), + tokens=spent, ) for call in calls: @@ -385,6 +430,7 @@ class Delegation: handback=handback if handback.succeeded else None, branch=branch, artifacts=sorted(touched), + tokens=spent, ) outcome = self._workspace.dispatch(call.tool, call.arguments) @@ -411,6 +457,7 @@ class Delegation: f"turn cap of {self._caps.turns} reached without a handback", branch=branch, artifacts=sorted(touched), + tokens=spent, ) def _commit_turn(self, turn: int) -> None: diff --git a/nisse-dev/src/nisse_dev/llm.py b/nisse-dev/src/nisse_dev/llm.py index 55f6ac1..cfaf0eb 100644 --- a/nisse-dev/src/nisse_dev/llm.py +++ b/nisse-dev/src/nisse_dev/llm.py @@ -41,6 +41,60 @@ class LLMError(RuntimeError): RATE_LIMIT_STATUS = 429 RATE_LIMIT_RETRY_S = 20.0 +# Generous by default. Output is billed by what is produced, not by the +# ceiling, and a ceiling too low does not fail loudly — it truncates a tool +# call mid-argument and hands back something that parses. Measured: Opus +# writing an architecture plan routinely spends 3,000-4,000 output tokens and +# was observed at 4,064 against a 4,096 limit. +DEFAULT_MAX_OUTPUT_TOKENS = 16384 + + +def max_output_tokens(env: Mapping[str, str] = os.environ) -> int: + raw = env.get("NISSE_MAX_OUTPUT_TOKENS") + return int(raw) if raw else DEFAULT_MAX_OUTPUT_TOKENS + + +class UpstreamUnavailable(LLMError): + """The proxy or provider could not be reached, or refused for reasons + unrelated to what was asked: a network blip, a timeout, a 5xx, a rate + limit that did not clear, an exhausted budget. + + Worth distinguishing because the *work* is not at fault. A delegation + stopped by one of these has a branch that may be perfectly good, so it is + preserved and resumed rather than discarded — unlike a delegate that made + a mess, which is cleaned up. + """ + + +class BudgetExhausted(UpstreamUnavailable): + """The proxy refused because a spend limit is gone. + + Distinct from the rest of its family because it will not clear on its + own within the hour: an exhausted daily budget takes external models away + until it resets, so retrying wastes time and the operator needs telling. + A5 keeps the budget itself at the proxy; recognising its refusal is ours. + """ + + +def _looks_like_budget_refusal(text: str) -> bool: + # Pattern-matched rather than contracted: LiteLLM's wording is not a + # documented interface, so this errs toward missing one rather than + # misreporting an unrelated failure as a budget problem. + lowered = text.lower() + return "budget" in lowered and ( + "exceed" in lowered or "exhaust" in lowered or "limit" in lowered + ) + + +class TruncatedResponse(LLMError): + """The model was cut off mid-answer. + + Raised rather than returned, because the damage is silent: a truncated + tool call arrives with required arguments simply absent, and a truncated + write_file would write half a file and report success. A cut-off turn is + a failed turn, and every caller already retries or gates on LLMError. + """ + class LLMClient(Protocol): def complete( @@ -92,6 +146,7 @@ class LiteLLMClient: timeout_s: float = 120.0, transport: httpx.BaseTransport | None = None, sleep: Any = time.sleep, + max_tokens: int | None = None, ) -> None: self._http = httpx.Client( base_url=base_url.rstrip("/"), @@ -101,6 +156,11 @@ class LiteLLMClient: ) self._renderer = OpenAIChatRenderer() self._sleep = sleep + self._max_tokens = max_tokens or max_output_tokens() + # Set after each call so a caller can meter itself (A13 caps); + # a side channel rather than a return value, so the LLMClient + # protocol stays the same for every implementation. + self.last_usage: dict[str, int] = {} @classmethod def from_env(cls, env: Mapping[str, str] = os.environ) -> "LiteLLMClient": @@ -118,10 +178,15 @@ class LiteLLMClient: system: str | None = None, ) -> list[CanonicalEvent]: body = self._renderer.render(events, tools, model, system=system) + # 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. + body["max_tokens"] = self._max_tokens response = _post_with_rate_limit_retry( self._http, "/chat/completions", body, self._sleep ) if response.status_code != 200: + _raise_for_upstream(response) raise LLMError( f"LLM request returned {response.status_code}: {response.text[:500]}" ) @@ -131,10 +196,21 @@ class LiteLLMClient: self, data: dict[str, Any], requested_model: str ) -> list[CanonicalEvent]: try: - message = data["choices"][0]["message"] + choice = data["choices"][0] + message = choice["message"] except (KeyError, IndexError) as exc: raise LLMError(f"malformed completion response: {exc}") from exc + if choice.get("finish_reason") == "length": + raise TruncatedResponse( + f"{requested_model} hit its output limit mid-answer; " + "raise NISSE_MAX_OUTPUT_TOKENS" + ) produced_by = data.get("model") or requested_model + usage = data.get("usage") or {} + self.last_usage = { + "input": int(usage.get("prompt_tokens") or 0), + "output": int(usage.get("completion_tokens") or 0), + } out: list[CanonicalEvent] = [] if message.get("content"): out.append(AssistantText(text=message["content"], model=produced_by)) @@ -184,7 +260,7 @@ class AnthropicPassthroughClient: token: str, *, timeout_s: float = 300.0, - max_tokens: int = 4096, + max_tokens: int | None = None, transport: httpx.BaseTransport | None = None, sleep: Any = time.sleep, ) -> None: @@ -199,8 +275,9 @@ class AnthropicPassthroughClient: transport=transport, ) self._renderer = AnthropicMessagesRenderer() - self._max_tokens = max_tokens + self._max_tokens = max_tokens or max_output_tokens() self._sleep = sleep + self.last_usage: dict[str, int] = {} @classmethod def from_env( @@ -226,6 +303,7 @@ class AnthropicPassthroughClient: self._http, "/v1/messages", body, self._sleep ) if response.status_code != 200: + _raise_for_upstream(response) raise LLMError( f"passthrough returned {response.status_code}: {response.text[:500]}" ) @@ -234,10 +312,20 @@ class AnthropicPassthroughClient: def _parse( self, data: dict[str, Any], requested_model: str ) -> list[CanonicalEvent]: + if data.get("stop_reason") == "max_tokens": + raise TruncatedResponse( + f"{requested_model} hit the output limit of {self._max_tokens} " + f"tokens mid-answer; raise NISSE_MAX_OUTPUT_TOKENS" + ) content = data.get("content") if not isinstance(content, list): raise LLMError(f"malformed passthrough response: {str(data)[:300]}") produced_by = data.get("model") or requested_model + usage = data.get("usage") or {} + self.last_usage = { + "input": int(usage.get("input_tokens") or 0), + "output": int(usage.get("output_tokens") or 0), + } out: list[CanonicalEvent] = [] for block in content: kind = block.get("type") @@ -271,8 +359,52 @@ def _post_with_rate_limit_retry( try: response = http.post(path, json=body) except httpx.HTTPError as exc: - raise LLMError(f"LLM request failed: {exc}") from exc + # Could not reach the proxy at all: nothing about the request was + # wrong, so callers holding work in progress should keep it. + raise UpstreamUnavailable(f"LLM request failed: {exc}") from exc if response.status_code != RATE_LIMIT_STATUS or attempt == 2: return response sleep(RATE_LIMIT_RETRY_S) raise AssertionError("unreachable") + + +# Model families we have a native renderer adapter for. A13: delegations take +# the vendor's passthrough route because a normalising middlebox is lossy +# exactly where a long-running delegation lives. +PASSTHROUGH_PREFIXES = ("claude-", "anthropic/") + + +def has_native_adapter(model: str) -> bool: + return model.startswith(PASSTHROUGH_PREFIXES) + + +def select_client( + model: str, normalized: LLMClient, passthrough: LLMClient +) -> LLMClient: + """Choose the route from the model, not from where the caller sits. + + A13 spells out the rule: use the vendor-native adapter where one exists, + and otherwise *provisionally* accept LiteLLM's normalization for that + vendor — validating it against the A16 eval set before trusting it with + delegations. Hard-wiring the passthrough would mean pointing a role at a + non-Anthropic model silently sent it Anthropic-shaped request bodies, + which is the opposite of the vendor fluidity A5 requires. + """ + return passthrough if has_native_adapter(model) else normalized + + +def _raise_for_upstream(response: httpx.Response) -> None: + """Classify a non-200 as upstream trouble where it plainly is. + + A 4xx that is not a rate limit means the request itself was wrong, and + preserving work on the strength of one would hide a real bug — so the + benefit of the doubt stops at the server's own door. + """ + if _looks_like_budget_refusal(response.text): + raise BudgetExhausted( + f"the proxy refused on budget grounds: {response.text[:300]}" + ) + if response.status_code >= 500 or response.status_code == RATE_LIMIT_STATUS: + raise UpstreamUnavailable( + f"upstream returned {response.status_code}: {response.text[:300]}" + ) diff --git a/nisse-dev/src/nisse_dev/planner.py b/nisse-dev/src/nisse_dev/planner.py index be599b0..f0b4e86 100644 --- a/nisse-dev/src/nisse_dev/planner.py +++ b/nisse-dev/src/nisse_dev/planner.py @@ -13,7 +13,7 @@ from typing import Any from nisse_dev.canonical import AssistantText, ToolCall, ToolResult from nisse_dev.control import AskHuman, PlanningError, SubmitPlan -from nisse_dev.llm import LLMClient, LLMError, RoleMap +from nisse_dev.llm import BudgetExhausted, LLMClient, LLMError, RoleMap from nisse_dev.state import StateStore # Plans may be executed by a materially weaker model, which cannot @@ -125,6 +125,11 @@ class LLMPlanningAgent: output is observable, never the model's own claim).""" try: return self._turn(model, system), None + except BudgetExhausted: + # Retrying cannot help: the budget will not reappear within a + # turn. Surface it now so the operator is told rather than the + # loop spending a second call to fail identically. + raise except LLMError as exc: return None, str(exc) diff --git a/nisse-dev/tests/test_delegation.py b/nisse-dev/tests/test_delegation.py index 53dcdc8..8a7fa55 100644 --- a/nisse-dev/tests/test_delegation.py +++ b/nisse-dev/tests/test_delegation.py @@ -21,7 +21,7 @@ from nisse_dev.delegation import ( delegation_reconciler, ) from nisse_dev.git import GitRepo -from nisse_dev.llm import LLMError +from nisse_dev.llm import BudgetExhausted, LLMError, UpstreamUnavailable from nisse_dev.oplog import OpLog from nisse_dev.state import StateStore from nisse_dev.tools import Workspace @@ -507,3 +507,143 @@ def test_recover_leaves_an_unclean_cut_for_the_boot_gate( assert build(ScriptedClient([]), store, ws, repo).recover(lambda _m: None) is None # Still pending, so reconcile_on_boot gates rather than blind-resuming. assert [i.kind for i in store.pending_intents()] == [DELEGATION_INTENT] + + +# --- the token cap, which A13 calls hard and was decoration ------------- + + +class MeteredClient(ScriptedClient): + """A client that reports usage, as the real ones do.""" + + def __init__(self, turns: list, per_call: int = 1000) -> None: + super().__init__(turns) + self._per_call = per_call + self.last_usage: dict[str, int] = {} + + def complete(self, *args: Any, **kwargs: Any): + produced = super().complete(*args, **kwargs) + self.last_usage = {"input": self._per_call, "output": self._per_call} + return produced + + +def test_token_cap_stops_a_delegation( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + # An unbounded delegation can exhaust a daily budget and take external + # models away until it resets, so the cap has to be real. + client = MeteredClient( + [[call("list_dir", path=".")] for _ in range(20)], per_call=600 + ) + result = build(client, store, ws, repo, caps=Caps(turns=20, tokens=2000)).run( + brief="b", branch="d" + ) + + assert result.status == "capped" + assert "token cap of 2000" in result.note + # 600 in + 600 out per call: stops on the second turn, not the twentieth. + assert len(client.calls) == 2 + + +def test_tokens_spent_are_reported_not_left_at_zero( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + client = MeteredClient( + [[call("write_file", path="a.py", content="x")], [hand_back()]], per_call=500 + ) + result = build(client, store, ws, repo).run(brief="b", branch="d") + + assert result.usable + assert result.tokens == 2000 # two calls, 1000 each + + completed = next( + e.event for e in store.read_events() if isinstance(e.event, DelegationCompleted) + ) + # A16 evaluates cost against outcome, which needs a real number. + assert completed.tokens == 2000 + + +def test_a_client_without_usage_still_works( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + # Scripted and third-party clients need not report usage; the cap simply + # does not bind rather than crashing the delegation. + client = ScriptedClient( + [[call("write_file", path="a.py", content="x")], [hand_back()]] + ) + assert build(client, store, ws, repo).run(brief="b", branch="d").usable + + +def test_budget_exhaustion_preserves_the_work_for_later( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + """An outage is not a bad delegate. + + The branch may hold perfectly good work, so it is kept and the intent + left pending — the next boot treats it exactly like a crash mid-delegation + and resumes if the cut was clean. Discarding would throw away work + because the budget ran out. + """ + + class BrokeClient(ScriptedClient): + def complete(self, *args: Any, **kwargs: Any): + if self.calls: + raise BudgetExhausted("daily budget exhausted") + return super().complete(*args, **kwargs) + + client = BrokeClient([[call("write_file", path="progress.py", content="real")]]) + result = build(client, store, ws, repo).run(brief="b", branch="d") + + assert result.status == "interrupted" + assert result.interrupted and not result.usable + # The work survives, on its branch. + assert repo.current_branch() == "d" + assert (repo.root / "progress.py").exists() + # And the intent stays pending, which is what A3's resume rule keys on. + assert [i.kind for i in store.pending_intents()] == [DELEGATION_INTENT] + + +def test_an_ordinary_failure_still_discards( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + # Only external interruptions are preserved; a delegate that made a mess + # is still cleaned up (A13). + client = ScriptedClient([[hand_back(succeeded=False, changed="cannot")]]) + result = build(client, store, ws, repo).run(brief="b", branch="d") + + assert result.status == "failed" + assert not result.interrupted + assert repo.current_branch() == "main" + assert store.pending_intents() == [] + + +def test_a_network_outage_also_preserves_the_work( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + # Same reasoning as an exhausted budget: the branch may be fine, and the + # outage says nothing about the quality of the work on it. + class FlakyClient(ScriptedClient): + def complete(self, *args: Any, **kwargs: Any): + if self.calls: + raise UpstreamUnavailable("LLM request failed: connection refused") + return super().complete(*args, **kwargs) + + client = FlakyClient([[call("write_file", path="progress.py", content="real")]]) + result = build(client, store, ws, repo).run(brief="b", branch="d") + + assert result.status == "interrupted" + assert (repo.root / "progress.py").exists() + assert [i.kind for i in store.pending_intents()] == [DELEGATION_INTENT] + + +def test_a_bad_request_is_not_given_the_benefit_of_the_doubt( + store: StateStore, ws: Workspace, repo: GitRepo +) -> None: + # A 4xx that is not a rate limit means we sent something wrong; treating + # that as an outage would preserve branches and hide a real bug. + class BadRequestClient(ScriptedClient): + def complete(self, *args: Any, **kwargs: Any): + raise LLMError("LLM request returned 400: malformed tool schema") + + result = build(BadRequestClient([]), store, ws, repo).run(brief="b", branch="d") + assert result.status == "failed" + assert not result.interrupted diff --git a/nisse-dev/tests/test_llm.py b/nisse-dev/tests/test_llm.py index 25706f9..9f6b7fa 100644 --- a/nisse-dev/tests/test_llm.py +++ b/nisse-dev/tests/test_llm.py @@ -11,7 +11,20 @@ from nisse_dev.canonical import ( UserMessage, ) from nisse_dev.envfile import load_env_file -from nisse_dev.llm import RATE_LIMIT_RETRY_S, LiteLLMClient, LLMError, RoleMap +from nisse_dev.llm import ( + DEFAULT_MAX_OUTPUT_TOKENS, + RATE_LIMIT_RETRY_S, + AnthropicPassthroughClient, + BudgetExhausted, + LiteLLMClient, + LLMError, + RoleMap, + TruncatedResponse, + UpstreamUnavailable, + has_native_adapter, + max_output_tokens, + select_client, +) EVENTS = [StoredEvent(id=1, ts="t", event=UserMessage(text="hi", source="m:1"))] @@ -203,3 +216,167 @@ def test_load_env_file_missing_is_noop(tmp_path: Path) -> None: env: dict[str, str] = {} load_env_file(tmp_path / "nope.env", env) assert env == {} + + +# --- route selection (A5 vendor fluidity) -------------------------------- + + +def test_anthropic_models_take_the_native_route() -> None: + normalized, passthrough = object(), object() + for model in ("claude-haiku-4-5", "claude-opus-5", "anthropic/claude-sonnet-5"): + assert select_client(model, normalized, passthrough) is passthrough + + +def test_other_vendors_take_the_normalized_route() -> None: + # A13: provisionally accept LiteLLM's normalization for a vendor whose + # renderer adapter does not exist yet, rather than sending it request + # bodies shaped for someone else. + normalized, passthrough = object(), object() + for model in ("gpt-6-astra", "local-coder", "local-fast", "llama3.2"): + assert select_client(model, normalized, passthrough) is normalized + + +def test_native_adapter_detection_is_explicit() -> None: + assert has_native_adapter("claude-opus-5") + assert not has_native_adapter("gpt-6-astra") + # A model merely mentioning the vendor is not the vendor. + assert not has_native_adapter("my-claude-clone") + + +# --- truncation (silent until measured against Opus) --------------------- + + +def test_openai_route_raises_on_truncation() -> None: + # finish_reason "length" means the answer was cut off; parsing it as + # complete yields a tool call with required arguments simply absent. + truncated = httpx.Response( + 200, + json={ + "model": "gpt-test-1", + "choices": [ + { + "finish_reason": "length", + "message": { + "content": None, + "tool_calls": [ + { + "id": "c", + "type": "function", + "function": { + "name": "submit_plan", + "arguments": '{"type": "arch"}', + }, + } + ], + }, + } + ], + }, + ) + client = make_client(lambda request: truncated) + with pytest.raises(TruncatedResponse, match="output limit"): + client.complete(EVENTS, tools=None, model="gpt-6-astra") + + +def test_anthropic_route_raises_on_truncation() -> None: + # Observed for real: Opus spent 4,064 output tokens against a 4,096 + # ceiling, and a longer plan came back as submit_plan with no content. + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "model": "claude-opus-5", + "stop_reason": "max_tokens", + "content": [ + { + "type": "tool_use", + "id": "t", + "name": "submit_plan", + "input": {"type": "arch"}, + } + ], + }, + ) + + client = AnthropicPassthroughClient( + "https://litellm.test", + "tok", + transport=httpx.MockTransport(handler), + sleep=lambda _s: None, + ) + with pytest.raises(TruncatedResponse, match="output limit"): + client.complete(EVENTS, tools=None, model="claude-opus-5") + + +def test_a_complete_answer_is_not_mistaken_for_truncation() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "model": "claude-opus-5", + "stop_reason": "tool_use", + "content": [ + { + "type": "tool_use", + "id": "t", + "name": "submit_plan", + "input": {"type": "arch", "content": "a real plan"}, + } + ], + }, + ) + + client = AnthropicPassthroughClient( + "https://litellm.test", "tok", transport=httpx.MockTransport(handler) + ) + [call] = client.complete(EVENTS, tools=None, model="claude-opus-5") + assert call.arguments["content"] == "a real plan" + + +def test_output_ceiling_is_configurable_and_generous_by_default() -> None: + # Output is billed by what is produced, so a low ceiling buys nothing and + # costs silent truncation. + assert DEFAULT_MAX_OUTPUT_TOKENS >= 8192 + assert max_output_tokens({}) == DEFAULT_MAX_OUTPUT_TOKENS + assert max_output_tokens({"NISSE_MAX_OUTPUT_TOKENS": "2048"}) == 2048 + + +def test_truncation_is_an_llm_error_so_callers_already_handle_it() -> None: + # The planner retries, the executor and delegation gate. None of them + # need to learn a new exception type. + assert issubclass(TruncatedResponse, LLMError) + + +def test_a_5xx_is_upstream_trouble() -> None: + client = make_client(lambda request: httpx.Response(503, text="bad gateway")) + with pytest.raises(UpstreamUnavailable, match="503"): + client.complete(EVENTS, tools=None, model="local-fast") + + +def test_a_connection_failure_is_upstream_trouble() -> None: + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("refused") + + with pytest.raises(UpstreamUnavailable, match="request failed"): + make_client(handler).complete(EVENTS, tools=None, model="local-fast") + + +def test_a_persistent_rate_limit_is_upstream_trouble() -> None: + # It cleared for neither attempt, so the caller should keep its work + # rather than discard it. + client = make_client(lambda request: httpx.Response(429, text="slow down")) + with pytest.raises(UpstreamUnavailable): + client.complete(EVENTS, tools=None, model="local-fast") + + +def test_a_bad_request_is_not_upstream_trouble() -> None: + client = make_client(lambda request: httpx.Response(400, text="bad schema")) + with pytest.raises(LLMError) as excinfo: + client.complete(EVENTS, tools=None, model="local-fast") + assert not isinstance(excinfo.value, UpstreamUnavailable) + + +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) diff --git a/nisse-dev/tests/test_planner.py b/nisse-dev/tests/test_planner.py index 9dcce14..0334242 100644 --- a/nisse-dev/tests/test_planner.py +++ b/nisse-dev/tests/test_planner.py @@ -20,7 +20,7 @@ from nisse_dev.control import ( PlanningError, SubmitPlan, ) -from nisse_dev.llm import RoleMap +from nisse_dev.llm import BudgetExhausted, RoleMap from nisse_dev.oplog import OpLog from nisse_dev.planner import PLANNER_TOOLS, LLMPlanningAgent from nisse_dev.state import StateStore @@ -279,3 +279,21 @@ def test_rejection_feedback_reaches_the_planner_via_the_log( isinstance(e, UserMessage) and "too vague" in e.text for e in replan_context ) assert "v2" in transport.outbound[-1].text + + +def test_budget_exhaustion_is_not_retried(store: StateStore) -> None: + """A budget will not reappear within a turn, so a retry buys nothing and + delays telling the operator.""" + + class BrokeClient: + def __init__(self) -> None: + self.calls = 0 + + def complete(self, *args: Any, **kwargs: Any): + self.calls += 1 + raise BudgetExhausted("daily budget exhausted") + + client = BrokeClient() + with pytest.raises(BudgetExhausted): + LLMPlanningAgent(client, store, ROLES).plan("arch", "demo") + assert client.calls == 1 # not two diff --git a/tests/integration/test_model_capability.py b/tests/integration/test_model_capability.py new file mode 100644 index 0000000..710b8eb --- /dev/null +++ b/tests/integration/test_model_capability.py @@ -0,0 +1,162 @@ +"""Capability matrix across models (spec Part H, A15). + +Part H says the thresholds that depend on local capability — A12's failure +count, A14's consult budget, A15's manifest size — must be tuned "at +build-order step 5 on the real hardware". This is the measurement that makes +that possible, rather than reasoning about model strength from reputation. + +What it measures is the thing that actually breaks: whether a model can hold +the tool-call contract our loops depend on. llama3.2 could not — it returned +`{` as an entire plan — and no amount of prompt work fixed it. + +Opt in with NISSE_CAPABILITY_MODELS, comma separated. Skipped otherwise, +because it spends real tokens across several models. +""" + +import os +from pathlib import Path + +import pytest + +from nisse_dev.canonical import ( + AssistantText, + StoredEvent, + ToolCall, + ToolResult, + UserMessage, +) +from nisse_dev.envfile import load_env_file +from nisse_dev.expert import parse_answer +from nisse_dev.llm import ( + AnthropicPassthroughClient, + LiteLLMClient, + LLMError, + select_client, +) +from nisse_dev.planner import PLANNER_TOOLS +from nisse_dev.tools import EXECUTOR_TOOLS + +load_env_file(Path(__file__).resolve().parents[2] / ".env") + +SENTINEL = "NISSE_CAPABILITY_MODELS" +MODELS = [m.strip() for m in os.environ.get(SENTINEL, "").split(",") if m.strip()] + +pytestmark = [ + pytest.mark.integration, + pytest.mark.skipif( + not MODELS, reason=f"set {SENTINEL} to a comma-separated model list" + ), +] + + +def clients() -> tuple[LiteLLMClient, AnthropicPassthroughClient]: + return LiteLLMClient.from_env(), AnthropicPassthroughClient.from_env() + + +def user(text: str) -> list[StoredEvent]: + return [StoredEvent(id=1, ts="t", event=UserMessage(text=text, source="probe"))] + + +@pytest.mark.parametrize("model", MODELS) +def test_model_answers_at_all(model: str) -> None: + normalized, passthrough = clients() + client = select_client(model, normalized, passthrough) + produced = client.complete( + user("Reply with exactly one word: pong"), None, model, system="Be terse." + ) + texts = [e.text for e in produced if isinstance(e, AssistantText)] + assert texts, f"{model} produced no text: {produced!r}" + + +@pytest.mark.parametrize("model", MODELS) +def test_model_holds_the_planner_contract(model: str) -> None: + """A4's gating is structural at the tool layer: a model that answers in + prose cannot be gated, because there is no plan to approve.""" + normalized, passthrough = clients() + client = select_client(model, normalized, passthrough) + produced = client.complete( + user("Draft an architecture plan for a command-line slug generator."), + PLANNER_TOOLS, + model, + system=( + "You must answer by calling exactly one tool: submit_plan or " + "ask_human. Plain prose is not accepted." + ), + ) + 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"} + if calls[0].tool == "submit_plan": + content = calls[0].arguments.get("content", "") + # llama3.2 emitted "{" here — structurally a tool call, substantively + # nothing. Length is a crude proxy, but it separates the two. + assert len(str(content)) > 80, ( + f"{model} submitted a degenerate plan: {content!r}" + ) + + +@pytest.mark.parametrize("model", MODELS) +def test_model_round_trips_a_tool_result(model: str) -> None: + """The executor loop feeds results back; a model that cannot consume its + own tool result cannot run a milestone.""" + normalized, passthrough = clients() + client = select_client(model, normalized, passthrough) + events = user("List the files in the current directory using your tools.") + produced = client.complete(events, EXECUTOR_TOOLS, model, system="Use a tool.") + calls = [e for e in produced if isinstance(e, ToolCall)] + assert calls, f"{model} made no tool call" + + next_id = 2 + for event in produced: + events.append(StoredEvent(id=next_id, ts="t", event=event)) + next_id += 1 + events.append( + StoredEvent( + id=next_id, + ts="t", + event=ToolResult( + call_id=calls[0].call_id, result="app.py\ntests/", success=True + ), + ) + ) + follow_up = client.complete(events, EXECUTOR_TOOLS, model, system="Use a tool.") + assert follow_up, f"{model} produced nothing after its tool result" + + +@pytest.mark.parametrize("model", MODELS) +def test_model_holds_the_prompt_level_json_contract(model: str) -> None: + """A14 contracts structured output at the prompt level because vendors + differ on JSON enforcement. This measures whether that holds per model.""" + normalized, passthrough = clients() + client = select_client(model, normalized, passthrough) + produced = client.complete( + user("Is SQLite sufficient for a single-process agent's task state?"), + None, + model, + system=( + "Reply with a single JSON object and nothing else: " + '{"answer": "...", "confidence": "high"|"medium"|"low", ' + '"caveats": "..."}' + ), + ) + 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}" + ) + + +@pytest.mark.parametrize("model", MODELS) +def test_route_selection_matches_the_model(model: str) -> None: + """A non-Anthropic model must not be sent Anthropic-shaped bodies. This + was hardcoded until gpt-6-astra made it observable.""" + normalized, passthrough = clients() + chosen = select_client(model, normalized, passthrough) + if model.startswith(("claude-", "anthropic/")): + assert chosen is passthrough + else: + assert chosen is normalized + # And the chosen route must actually work for this model. + try: + chosen.complete(user("say pong"), None, model, system="Be terse.") + except LLMError as exc: + pytest.fail(f"{model} failed on its selected route: {exc}")