diff --git a/README.md b/README.md index 0f736ca4..7201022c 100644 --- a/README.md +++ b/README.md @@ -934,6 +934,7 @@ agentic: workspace_root: "data/agentic/workspaces" max_write_budget_bytes: 100000 max_handoff_chars: 200000 # outbound-prompt cap for cloud egress + planner_max_tokens: 2048 # real-repo completion cap; keep it within Ollama num_ctx allow_cloud_providers: false # gate 3 of the cloud chain providers: grok: { enabled: false, model: "grok-4.5" } diff --git a/agentic/cli.py b/agentic/cli.py index 77ea7b84..ebf141c5 100644 --- a/agentic/cli.py +++ b/agentic/cli.py @@ -616,6 +616,7 @@ def cmd_real_repo_run_plan(args: argparse.Namespace) -> int: client, instruction=args.instruction, context=context_text or "", + max_tokens=cfg.deepagent_github.planner_max_tokens, config_path=args.config, cfg=app_cfg, ) @@ -850,6 +851,7 @@ def cmd_real_repo_run(args: argparse.Namespace) -> int: branch_name=args.branch, commit_message=args.commit_message, max_iterations=args.max_iterations, + max_tokens=cfg.deepagent_github.planner_max_tokens, reason=args.reason, confirm=args.confirm, context=context_text, diff --git a/agentic/config.py b/agentic/config.py index 9133c832..cdbf14dc 100644 --- a/agentic/config.py +++ b/agentic/config.py @@ -89,6 +89,9 @@ # up to 8k chars of GitHub context + up to 4k chars of prior-iteration # feedback) is comparably large. DEFAULT_PLANNER_TIMEOUT_SEC = 600 +# Completion budget for each local/cloud proposer call. Keep the existing +# implicit client default so this new operator control is rollout-neutral. +DEFAULT_PLANNER_MAX_TOKENS = 2048 _VALID_MODES = ("read", "write") # Post-Ollama migration: "lmstudio" is retired as a provider id. Use "ollama" @@ -218,6 +221,7 @@ class DeepAgentGitHubConfig: max_write_budget_bytes: int = DEFAULT_MAX_WRITE_BUDGET_BYTES max_handoff_chars: int = DEFAULT_MAX_HANDOFF_CHARS planner_timeout_sec: int = DEFAULT_PLANNER_TIMEOUT_SEC + planner_max_tokens: int = DEFAULT_PLANNER_MAX_TOKENS # The escape hatch for agentic.harness_optimizer.governance.inspect_code_shape. # Defaults ON (fail safe). It exists because that scanner is a HEURISTIC that # hard-blocks an iteration, and a heuristic gate with no recourse is a broken @@ -319,6 +323,13 @@ def __post_init__(self) -> None: "agentic.deepagent_github.planner_timeout_sec must be a positive integer", details={"received": self.planner_timeout_sec}, ) + if not isinstance(self.planner_max_tokens, int) or isinstance( + self.planner_max_tokens, bool + ) or self.planner_max_tokens <= 0: + raise AgenticConfigError( + "agentic.deepagent_github.planner_max_tokens must be a positive integer", + details={"received": self.planner_max_tokens}, + ) def _coerce_providers(self) -> None: # Nested blocks arrive as plain dicts from yaml. Unlike the top-level diff --git a/agentic/real_repo_loop.py b/agentic/real_repo_loop.py index 2b230068..cb96dbad 100644 --- a/agentic/real_repo_loop.py +++ b/agentic/real_repo_loop.py @@ -131,10 +131,9 @@ class ProposerClient(Protocol): without lying about what type it claims to be, and so mypy can check the substitution rather than only trusting a duck-typed hope. - Keyword-only, matching every real call site in this loop -- ``max_tokens``/ - ``temperature`` are declared with defaults because this loop never passes - them explicitly, so a conforming implementation must supply its own - sensible values, not rely on the loop to. + Keyword-only, matching every real call site in this loop. ``max_tokens``/ + ``temperature`` retain conservative defaults for direct callers; the CLI + passes its validated completion budget explicitly. ``temperature`` is ``float | None`` rather than ``float`` so an implementation can drop the parameter entirely: Anthropic rejects a @@ -624,6 +623,7 @@ def generate_plan( *, instruction: str, context: str = "", + max_tokens: int = 2048, config_path: str = "config.yaml", cfg: dict | None = None, ) -> str: @@ -649,6 +649,8 @@ def generate_plan( """ if not isinstance(instruction, str) or not instruction.strip(): raise AgenticError("plan instruction must be a non-empty string") + if not isinstance(max_tokens, int) or isinstance(max_tokens, bool) or max_tokens <= 0: + raise AgenticError("max_tokens must be a positive integer", details={"received": max_tokens}) quoted_context = f"{_UNTRUSTED_OPEN}\n{_defuse_fence(context)}\n{_UNTRUSTED_CLOSE}" if context else "" user_prompt = "\n\n".join( @@ -662,6 +664,7 @@ def generate_plan( response = client.invoke( system_prompt=PLAN_SYSTEM_PROMPT, user_prompt=user_prompt, + max_tokens=max_tokens, config_path=config_path, cfg=cfg, ) @@ -689,6 +692,7 @@ def run_real_repo_loop( max_iterations: int, reason: str, confirm: bool, + max_tokens: int = 2048, context: str | None = None, read_paths: Sequence[str] = (), protected_write_paths: Sequence[str] = (), @@ -775,6 +779,8 @@ def run_real_repo_loop( raise AgenticError("loop instruction must be a non-empty string") if not isinstance(max_iterations, int) or isinstance(max_iterations, bool) or max_iterations <= 0: raise AgenticError("max_iterations must be a positive integer", details={"received": max_iterations}) + if not isinstance(max_tokens, int) or isinstance(max_tokens, bool) or max_tokens <= 0: + raise AgenticError("max_tokens must be a positive integer", details={"received": max_tokens}) if not checks: raise AgenticError("checks must not be empty -- an empty check list vacuously accepts every candidate") @@ -815,6 +821,7 @@ def run_real_repo_loop( response = client.invoke( system_prompt=PLANNER_SYSTEM_PROMPT, user_prompt=user_prompt, + max_tokens=max_tokens, config_path=config_path, cfg=cfg, ) diff --git a/config.yaml b/config.yaml index 0325725b..c9f4f810 100644 --- a/config.yaml +++ b/config.yaml @@ -517,6 +517,9 @@ agentic: # Was hardcoded at 30s with no override before this key existed -- a hard, # unrecoverable, first-iteration failure for any real local-model completion. planner_timeout_sec: 600 + # Reserved completion tokens for each real-repo plan/coding call. Keep + # prompt + this budget below Ollama num_ctx; raise only after measuring. + planner_max_tokens: 2048 allow_deepagents_dependency: false # extras must be installed explicitly # Governs the RETIRED DeepAgents virtual tool surface only (permissions.py, # tools.py) -- NOT the real-repo pipeline, whose write_file/add/commit/push diff --git a/tests/test_agentic_config.py b/tests/test_agentic_config.py index b253eb01..e6c07a0a 100644 --- a/tests/test_agentic_config.py +++ b/tests/test_agentic_config.py @@ -190,6 +190,26 @@ def test_deepagent_config_accepts_a_custom_planner_timeout_sec(tmp_path: Path) - assert cfg.deepagent_github.planner_timeout_sec == 900 +def test_deepagent_config_defaults_planner_max_tokens(tmp_path: Path) -> None: + from agentic.config import DEFAULT_PLANNER_MAX_TOKENS + + cfg = load_agentic_config(_write_config(tmp_path, _base_block())) + assert cfg.deepagent_github.planner_max_tokens == DEFAULT_PLANNER_MAX_TOKENS + + +@pytest.mark.parametrize("bad", [0, -1, "2048", 1.5, True]) +def test_deepagent_config_rejects_invalid_planner_max_tokens(tmp_path: Path, bad) -> None: + block = _base_block(deepagent_github={"planner_max_tokens": bad}) + with pytest.raises(AgenticConfigError): + load_agentic_config(_write_config(tmp_path, block)) + + +def test_deepagent_config_accepts_a_custom_planner_max_tokens(tmp_path: Path) -> None: + block = _base_block(deepagent_github={"planner_max_tokens": 3072}) + cfg = load_agentic_config(_write_config(tmp_path, block)) + assert cfg.deepagent_github.planner_max_tokens == 3072 + + def test_deepagent_config_rejects_shell_metachar_model(tmp_path: Path) -> None: block = _base_block(deepagent_github={"model": "good;bad"}) with pytest.raises(AgenticConfigError): diff --git a/tests/test_agentic_plan_handoff.py b/tests/test_agentic_plan_handoff.py index 49393d4a..a830d54d 100644 --- a/tests/test_agentic_plan_handoff.py +++ b/tests/test_agentic_plan_handoff.py @@ -45,12 +45,14 @@ def __init__(self, content: str = _PLAN_TEXT) -> None: self.content = content self.system_prompts: list[str] = [] self.user_prompts: list[str] = [] + self.max_tokens: list[int] = [] self.closed = False def invoke(self, *, system_prompt, user_prompt, max_tokens=2048, temperature=0.0, config_path="config.yaml", cfg=None): self.system_prompts.append(system_prompt) self.user_prompts.append(user_prompt) + self.max_tokens.append(max_tokens) return LocalProposerResponse(content=self.content, model="stub") def close(self) -> None: @@ -65,6 +67,17 @@ def test_generate_plan_returns_the_models_text(audit_cfg) -> None: assert generate_plan(client, instruction="do a thing", cfg=audit_cfg) == _PLAN_TEXT +def test_generate_plan_forwards_the_completion_budget(audit_cfg) -> None: + client = _StubClient() + generate_plan(client, instruction="do a thing", max_tokens=3072, cfg=audit_cfg) + assert client.max_tokens == [3072] + + +def test_generate_plan_rejects_an_invalid_completion_budget(audit_cfg) -> None: + with pytest.raises(AgenticError, match="max_tokens"): + generate_plan(_StubClient(), instruction="do a thing", max_tokens=0, cfg=audit_cfg) + + def test_generate_plan_uses_the_plan_prompt_not_the_coder_prompt(audit_cfg) -> None: """A planner told to emit '=== FILE ===' blocks would route around the human review entirely -- it would be writing code, not proposing an approach.""" @@ -227,6 +240,25 @@ def test_plan_command_prints_a_plan_and_creates_no_run(cfg_path, tmp_path, monke assert not runs_dir.exists() or not list(runs_dir.glob("*.json")) +def test_plan_command_threads_the_configured_completion_budget(cfg_path, monkeypatch, capsys) -> None: + captured: list[int] = [] + + def fake_invoke(self, **kwargs): + captured.append(kwargs["max_tokens"]) + return LocalProposerResponse(content=_PLAN_TEXT, model="local-test-model") + + monkeypatch.setattr(LocalProposerClient, "invoke", fake_invoke) + src = yaml.safe_load(Path(cfg_path).read_text(encoding="utf-8")) + src["agentic"]["deepagent_github"]["planner_max_tokens"] = 3072 + Path(cfg_path).write_text(yaml.safe_dump(src), encoding="utf-8") + from utils.logger import reset_config_cache + + reset_config_cache() + assert main(["--config", cfg_path, "real-repo-run-plan", "--repo", "--instruction", "do a thing"]) == EXIT_OK + capsys.readouterr() + assert captured == [3072] + + def test_plan_command_writes_to_out_file(cfg_path, tmp_path, monkeypatch) -> None: monkeypatch.setattr( LocalProposerClient, "invoke", diff --git a/tests/test_agentic_real_repo_run_cli.py b/tests/test_agentic_real_repo_run_cli.py index d1f8ee9d..e57934fc 100644 --- a/tests/test_agentic_real_repo_run_cli.py +++ b/tests/test_agentic_real_repo_run_cli.py @@ -180,32 +180,32 @@ def test_run_accepts_and_persists_a_pending_decision(cfg_path, checks_file, monk assert Path(record["dest"]).is_dir() -def test_run_threads_planner_timeout_sec_into_the_local_client(cfg_path, checks_file, monkeypatch, capsys): +def test_run_threads_planner_limits_into_the_local_client(cfg_path, checks_file, monkeypatch, capsys): """Regression for the confirmed 2026-08-02 finding: LocalProposerClient's own 30.0s constructor default was never overridden by any caller, so a real local-model completion over 30 wall-clock seconds killed the whole run on - iteration 1 with no retry. cli.py must now pass - agentic.deepagent_github.planner_timeout_sec through to the client's - underlying httpx.Client, not rely on the class's own hardcoded default. + iteration 1 with no retry. The CLI must pass the validated timeout to the + client and the completion budget to its invoke call. """ - captured_timeout = [] + captured_limits = [] def fake_invoke(self, *, system_prompt, user_prompt, max_tokens=2048, temperature=0.0, config_path="config.yaml", cfg=None): - captured_timeout.append(self._client.timeout.read) + captured_limits.append((self._client.timeout.read, max_tokens)) return LocalProposerResponse(content=_RIGHT_BLOCK, model=self.model) monkeypatch.setattr(LocalProposerClient, "invoke", fake_invoke) src = yaml.safe_load(Path(cfg_path).read_text(encoding="utf-8")) src["agentic"]["deepagent_github"]["planner_timeout_sec"] = 723 + src["agentic"]["deepagent_github"]["planner_max_tokens"] = 3072 Path(cfg_path).write_text(yaml.safe_dump(src), encoding="utf-8") from utils.logger import reset_config_cache reset_config_cache() assert _run_start(cfg_path, checks_file) == EXIT_OK - assert captured_timeout == [723.0] + assert captured_limits == [(723.0, 3072)] def test_run_exhausts_and_discards_the_clone(cfg_path, checks_file, monkeypatch, capsys):