diff --git a/docs/USAGE.md b/docs/USAGE.md index 0282ba8..9415f8c 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -735,6 +735,40 @@ fallback that has not been needed is not what you configured. default) so it can be read instead of paid for again. Pass `--artifacts ''` to keep nothing. +### What each role is shown + +Worth knowing, because it is what the models are actually judged on, and +because each of these was once absent and cost real attempts to discover. + +| Role | Sees | +|---|---| +| planner | the brief, and the repository's file listing | +| implementer | the brief, its own plan, a bounded slice of the repository **target file first**, the check commands that will run on its diff, and why the previous attempt was refused if there was one | +| reviewer | the brief, the diff, **the files that diff touched as they now stand**, and whether the checks passed | + +Three of those are recent and are worth stating plainly: + +- **The implementer is told the checks.** It is graded by them; keeping them + secret from it costs an attempt and two model calls to discover a formatter. + It does not weaken the gate — the command still runs and still refuses. +- **The reviewer is given the touched files.** Asked whether a change is wired + in where it should be, and holding only the change, it must answer "the diff + does not show" — and "the task cannot be judged from what you were given" is + grounds to reject. A file too large for the budget is **named as absent**, + because a reviewer that thinks a partial view is complete is worse than one + that knows it is partial. +- **A retry is told why the last attempt was refused.** It is *not* a + resumption: the item is re-planned against the current brief exactly as + before, no prior diff is fed back, and nothing is treated as progress. It + simply does not repeat the last mistake blind. + +**A brief that does not bound its own scope will be rejected.** The reviewer is +told to assume the work is wrong, and a sufficiently sceptical model can always +name one more path it has not been shown. An item that says what "done" is — +finitely, and including what is *not* in scope — is judged; one that does not +collects rejections that are each individually reasonable. That cost lands as +retries, and it is the plan's to fix, not the reviewer's. + --- ## 4. Resume after anything @@ -1163,7 +1197,42 @@ curl -sH "Authorization: Bearer $TOKEN" 'localhost:8099/api/work/T4' \ can branch on it: `checks_failed`, `check_escalated`, `check_transient`, `review_rejected`, `patch_rejected`, `no_target`, `worker_error`, `provider_exhausted`, `budget_exhausted`, `dependency_invalidated`, -`agent_timeout`, `claim_lost`. +`agent_timeout`, `claim_lost`, `item_wall_clock`, `item_spend`, +`hold_expired`, `context_unavailable`. + +### 6a.1 When the target does not fit in the prompt + +The implementer is shown a bounded slice of the repository — 60,000 characters +by default, the file the planner named first and a relevance-ordered fallback +after it. A repository whose relevant file is *larger than the whole budget* +therefore has a target that cannot be supplied at all. + +That used to proceed anyway: the target was dropped, the fallback filled the +space with whatever else was nearby, and the implementer was asked to change a +file it had never seen. It answers — models do not refuse for want of evidence +— and the diff then fails to apply, which reads in the log as a bad model and +is not one. + +Now the item stops **before** the implementer is called: + +```json +{"state": "blocked", "disposition": "escalated", + "reason_kind": "context_unavailable", "attempts": 0, + "last_error": "the planner's target(s) crates/gateway/src/main.rs (612334 bytes) + do not fit the context budget of 60000 characters, …"} +``` + +It costs no attempt, because no attempt could have succeeded and retrying will +not make the file smaller. Two things fix it, and both are yours to choose: + +```bash +agent-harness run --context-budget 400000 … # or $HARNESS_CONTEXT_BUDGET +``` + +or split the file. The ceiling that actually matters is the model's context +window, which the harness does not know and will not guess — a budget large +enough to overflow it turns a working item into a provider error, so raise it +deliberately rather than to the maximum. ### 6b. A check has five answers, not two @@ -1350,6 +1419,7 @@ durable stage. | `HARNESS_ENDPOINT` | `run`, `serve` | Model API base URL. | | `HARNESS_ROUTE_PRESET` | `run`, `serve` | Default route preset (`--preset`) for roles that name none: the wire protocol, the authentication header, the response reader and a failure classifier, as one name. Default `chat-completions`. | | `HARNESS_ROUTE_PRESETS` | all | Extra presets to make resolvable, as `name=module:attribute` pairs. For a preset that lives in your own code rather than in an installed distribution's entry points. | +| `HARNESS_CONTEXT_BUDGET` | `run` | How many characters of repository the implementer is shown (`--context-budget`, default 60000). A file bigger than this cannot be supplied at all — see [§6a.1](#6a1-when-the-target-does-not-fit-in-the-prompt). | | `HARNESS_ROOT_PATH` | `serve` | Prefix when behind a proxy, e.g. `/api/harness`. | | `AIDEVENV_URL` | `run`, `serve` | Session host, enabling attachable agents. In `serve` it is what makes the deployment supervised rather than monitoring-only. | | `AIDEVENV_TOKEN` | `run`, `serve` | Session host token. | diff --git a/src/agent_harness/__main__.py b/src/agent_harness/__main__.py index 50ea34b..7da7b4d 100644 --- a/src/agent_harness/__main__.py +++ b/src/agent_harness/__main__.py @@ -394,7 +394,7 @@ def _run(args: argparse.Namespace) -> int: code, so it says exactly what it will do before doing any of it.""" import json as _json - from .executor import Checks, Executor + from .executor import Checks, ContextPolicy, Executor from .github import GitHub from .model_client import Chain, ModelClient, chains_from_map from .work import RUNNING, WorkQueue, WorkRecord @@ -686,6 +686,7 @@ def live_routes() -> dict[str, Chain]: client, args.work, checks=checks, + context_policy=ContextPolicy(budget=args.context_budget), durability=durability, github=GitHub(args.repo) if args.repo else None, base_branch=args.base, @@ -830,6 +831,11 @@ def _adopt(args: argparse.Namespace) -> int: def main(argv: list[str] | None = None) -> int: + # Imported here, as every other executor name in this module is: the CLI + # starts for `--help` on a machine with no queue and no credentials, and + # only the default value is needed to print it. + from .executor import DEFAULT_CONTEXT_BUDGET + parser = argparse.ArgumentParser(prog="agent-harness", description=__doc__) parser.add_argument( "--db", @@ -1129,6 +1135,17 @@ def main(argv: list[str] | None = None) -> int: "then the default. The pre-review git checkpoint is unaffected by all " "three (or $HARNESS_DURABILITY).", ) + p_run.add_argument( + "--context-budget", + type=int, + default=int(os.environ.get("HARNESS_CONTEXT_BUDGET", "") or DEFAULT_CONTEXT_BUDGET), + help="how many characters of repository the implementer is shown " + f"(default {DEFAULT_CONTEXT_BUDGET}, or $HARNESS_CONTEXT_BUDGET). A file " + "larger than this cannot be supplied at all, and an item whose target " + "does not fit is stopped before the implementer is paid rather than " + "asked to change a file it cannot see. Raise it for a repository with " + "large files; the ceiling that matters is the model's context window.", + ) p_run.add_argument( "--demo", action="store_true", diff --git a/src/agent_harness/adapters/otlp.py b/src/agent_harness/adapters/otlp.py index bfae41c..b30e63a 100644 --- a/src/agent_harness/adapters/otlp.py +++ b/src/agent_harness/adapters/otlp.py @@ -99,6 +99,7 @@ "no_changes": "error", "agent_timeout": "error", "agent_failed": "error", + "context_unavailable": "error", } diff --git a/src/agent_harness/executor.py b/src/agent_harness/executor.py index 0163e6b..9055143 100644 --- a/src/agent_harness/executor.py +++ b/src/agent_harness/executor.py @@ -52,6 +52,7 @@ BUDGET_EXHAUSTED, CLAIM_LOST, COMPLETED, + CONTEXT_UNAVAILABLE, CRASHED, DECIDED, DEPENDENCY_INVALIDATED, @@ -198,8 +199,20 @@ def run_git(repo: Path, *args: str, check: bool = True) -> str: #: How much repository the implementer is shown. Big enough that a small #: project arrives whole, small enough not to dominate the prompt. +#: +#: A **default**, not a constant: a repository whose files are larger than this +#: cannot be worked on at all until it is raised, and no number chosen here is +#: right for every project. `run --context-budget` and `HARNESS_CONTEXT_BUDGET` +#: set it. DEFAULT_CONTEXT_BUDGET = 60_000 +#: Why a file was left out, as tokens rather than prose. The difference is +#: load-bearing: running out of room for context nobody asked for is normal, +#: and being unable to supply a file the planner *named* means the implementer +#: is about to be asked to edit something it cannot see. +TARGET_OVER_BUDGET = "named target exceeds remaining content budget" +BUDGET_SPENT = "content budget exhausted" + #: Files that are never worth spending the budget on, whatever their size. _UNINTERESTING = (".png", ".jpg", ".jpeg", ".gif", ".ico", ".pdf", ".zip", ".gz", ".woff", ".woff2") @@ -447,14 +460,7 @@ def relevance(path: str) -> tuple[int, int, str]: block = f"--- {path} ---\n{body}\n" if spent + len(block) > policy.budget: truncated = True - omitted.append( - ( - path, - "named target exceeds remaining content budget" - if explicit - else "content budget exhausted", - ) - ) + omitted.append((path, TARGET_OVER_BUDGET if explicit else BUDGET_SPENT)) continue parts.append(block) supplied.append(path) @@ -1063,7 +1069,7 @@ def is_disk_exhaustion(detail: str) -> bool: You are planning one unit of work. Do not write code yet. {brief} - +{listing} Reply with one JSON object and no commentary: {{ @@ -1077,6 +1083,14 @@ def is_disk_exhaustion(detail: str) -> bool: the reason in `cannot_identify_target` instead of inventing a path. """ +#: The repository's tracked paths, for the role whose entire job is naming +#: some of them. Paths only, never content: the planner has to know what +#: exists, and the implementer is the one that needs to read it. +PLAN_LISTING_PROMPT = """ +Files in this repository: +{listing} +""" + IMPLEMENT_PROMPT = """\ Implement this change and reply with a unified diff and nothing else. @@ -1087,11 +1101,43 @@ def is_disk_exhaustion(detail: str) -> bool: Repository context: {context} - +{unavailable}{checks}{prior} Reply with a single unified diff (`diff --git` / `---` / `+++` / `@@`) that applies cleanly at the repository root. No commentary outside the diff. """ +#: Supporting targets the budget could not carry. Named rather than silently +#: dropped, and paired with an instruction not to edit them — a model that +#: knows a file exists but has not read it will otherwise write a plausible +#: hunk against it, which is the failure this whole path exists to prevent. +STARVED_PROMPT = """ +The planner also named these files, and they were too large to include. You +have NOT seen them, so do not change them — if the task cannot be done without +changing one, say so instead of guessing at its contents: +{paths} +""" + +#: The gates, told to the writer as well as the marker. Empty when a project +#: configured none, so a run without checks reads exactly as it did before. +CHECKS_PROMPT = """ +These commands run on your diff before any reviewer sees it, and a non-zero +exit refuses the change: +{commands} +""" + +#: Why the previous attempt was refused. **This is a new attempt informed by +#: the last refusal, not a resumption of it** — the item is re-planned against +#: the current brief exactly as before (D11), and nothing here is treated as +#: progress. Without it a retry repeats the same mistake blind: measured on +#: rdpapp, three of four attempts at one item were refused by the same +#: formatter, each one unaware the last had been. +PRIOR_FAILURE_PROMPT = """ +A previous attempt at this task was refused. You are starting again from the +current brief, not continuing that attempt — but do not reproduce the fault: + +{error} +""" + REVIEW_PROMPT = """\ Review this change. You did not write it, and your job is not to be agreeable. @@ -1102,35 +1148,86 @@ def is_disk_exhaustion(detail: str) -> bool: The task: {brief} -The diff: +The diff, which is the change **in full** — it was produced from the +repository, so any file not appearing in it is unchanged: ```diff {diff} ``` - -Checks: {checks} +{context} +{checks} ## Answer First line exactly APPROVED or REJECTED. Then, in order: -1. **What I verified** — the specific things in the diff you actually checked - against the task. If you cannot name any, that is a REJECTED. -2. **What I could not verify** — anything the diff claims that the diff alone - does not show. Say it, do not assume it. +1. **What I verified** — the specific things you actually checked against the + task. If you cannot name any, that is a REJECTED. +2. **What I could not verify** — anything the change claims that *what you were + given* does not show. Say it, do not assume it. + + **Read the files above before writing this section.** They are the touched + files in full, at their post-change state, so questions like "is this the + only caller", "is there another path that bypasses it" and "does this + signature fit its callers" are answerable — answer them. Only a file listed + as not included is genuinely unavailable to you, and "the diff does not + show it" is not a reason when the file does. 3. **Why** — one paragraph. ## Reject if - It does not do what the task asked, or does more than the task asked. -- It claims an effect the diff does not demonstrate. +- It claims an effect the change does not demonstrate. - It changes something unrelated, however small. -- The task cannot be judged from what you were given. + +## Do not reject if + +- **The task's scope is narrower than the problem.** Scope is the task's to + set, not yours. Where the task says what is *out* of scope, judge the change + against what it asked for; that the wider problem remains afterwards is not a + fault in this work. Note it under "what I could not verify" so a person sees + it — a note is the right size for that, and a rejection is not. +- The task cannot be judged from what you were given — but only after you have + looked at what you were given. Wanting evidence that was in front of you is + not grounds to reject, and neither is wanting evidence the task did not ask + for. Approving work that does not do what was asked is the expensive failure here: it reaches a pull request, a human reads it as reviewed, and the cost lands much later. An unnecessary rejection costs one retry. """ +#: What the checks were and who ran them. The reviewer used to be handed the +#: bare word `passed`, which reads as the author's claim about their own work +#: — and a reviewer told to assume the work is wrong will discount it, as one +#: did: "I cannot verify that the file still type-checks, because no build +#: output was provided beyond the claim 'Checks: passed'." The harness ran +#: them, on this exact tree, before the reviewer was called. Saying so is a +#: fact, not a reassurance. +REVIEW_CHECKS_PROMPT = """ +These commands were run by the harness on this exact tree, after the change +was applied and before you were called. All of them exited zero: +{commands} +""" + +#: When a project has configured none. Said plainly, because "no checks" is +#: information a reviewer should weigh, and silence would let it assume there +#: were some. +REVIEW_NO_CHECKS_PROMPT = """ +This project has no checks configured, so nothing has been run against this +change. You are the only gate it has. +""" + +#: The files the diff touched, as they now stand. Without this the reviewer is +#: asked whether a change is wired in correctly while holding only the change, +#: and "the task cannot be judged from what you were given" — which the prompt +#: lists as grounds to reject — becomes true by construction rather than by +#: fault. What is missing is named, because a reviewer that does not know its +#: view is partial will treat it as complete. +REVIEW_CONTEXT_PROMPT = """ +The files it touched, as they now stand: +{files}{omitted} +""" + class Executor: """Drives work items through the model roles.""" @@ -1587,7 +1684,11 @@ def _execute(self, record: WorkRecord) -> Outcome: if planner is not None: self._emit(record, "resumed", detail=f"{A.PLANNED} (mode={mode})") else: - planner_reply = self._call(record, PLANNER, PLAN_PROMPT.format(brief=record.brief)) + planner_reply = self._call( + record, + PLANNER, + PLAN_PROMPT.format(brief=record.brief, listing=self._repo_listing()), + ) planner = parse_planner_result(planner_reply) log.record( self.project_id, @@ -1652,6 +1753,37 @@ def _execute(self, record: WorkRecord) -> Outcome: sort_keys=True, ), ) + starved = [path for path, reason in context.omitted if reason == TARGET_OVER_BUDGET] + # The planner is asked to order its targets by importance, so the first + # usable one is the file the work is *in*; the rest are supporting. + # Losing the first is fatal — the implementer would be asked to change + # a file it has not been shown, and it will answer rather than refuse, + # because models do not decline for want of evidence. Losing a + # supporting file is not fatal, and blocking the item over it would + # make a large repository unworkable for the sake of a file nobody was + # going to edit. + primary = next((target.path for target in context.targets if target.usable), None) + if starved and (primary is None or primary in starved): + outcome.reason = ( + "the planner's target(s) " + + ", ".join(f"{path} ({self._size_of(path)})" for path in starved) + + f" do not fit the context budget of {context.budget} characters, " + "so the implementer would be asked to change a file it cannot see. " + "Raise --context-budget (or HARNESS_CONTEXT_BUDGET), or split the file." + ) + self._emit(record, "context_unavailable", detail=outcome.reason) + outcome.stop = Stop( + ESCALATED, + CONTEXT_UNAVAILABLE, + detail=outcome.reason, + state=BLOCKED, + # A ceiling this deployment set stopped it; the item did not + # fail. Spending an attempt on the same file every claim would + # exhaust it for a condition no attempt can change. + consumes_attempt=False, + ) + outcome.state = BLOCKED + return outcome reply = self._call( record, IMPLEMENTER, @@ -1659,6 +1791,15 @@ def _execute(self, record: WorkRecord) -> Outcome: brief=record.brief, plan=planner.plan, context=context.text, + # The reviewer is told what the checks said; the implementer + # was never told what they are. So a diff is refused by a + # formatter the model was not shown, which costs an attempt and + # a model call to discover something the harness knew before it + # asked. Naming the commands is not weakening the gate: the + # gate still runs, and still refuses. + checks=self._checks_prompt(), + prior=self._prior_failure_prompt(record), + unavailable=self._starved_prompt(starved), ), ) outcome.stages.append("implement") @@ -1963,8 +2104,12 @@ def _review_stage( REVIEW_PROMPT.format( brief=record.brief, diff=applied_diff[:20000], - # Always "passed" by here: a non-passing check returned above. - checks="passed", + context=self._review_context(applied_diff), + # Always passing by here: a non-passing check returned + # above. What the reviewer needs is *which* commands + # passed, and that the harness rather than the author ran + # them. + checks=self._review_checks_prompt(), ), ) outcome.stages.append("review") @@ -2049,6 +2194,128 @@ def _base_for(self, record: WorkRecord) -> tuple[str, str | None]: note = f"{candidates[0]}; NOT stacked on {', '.join(candidates[1:])}" return first.branch, note + def _review_context(self, diff: str) -> str: + """The touched files as they now stand, for the reviewer. + + The reviewer is asked whether a change is wired in where it should be + and whether anything unrelated moved, and was given only the change. + Measured on rdpapp: two of the three reasons in a rejection were "the + diff does not show whether …", which no diff ever can. That is a gate + rejecting for the shape of its own prompt. + + The same budget bounds it, and a file too large to include is **named + as absent** rather than quietly left out — a reviewer that believes a + partial view is complete is worse than one that knows it is partial. + """ + paths: list[str] = [] + for line in diff.splitlines(): + if line.startswith("+++ ") and not line.startswith("+++ /dev/null"): + path = line[4:].strip() + path = path[2:] if path.startswith("b/") else path + if path and path not in paths: + paths.append(path) + if not paths: + return "" + + blocks: list[str] = [] + missing: list[str] = [] + spent = 0 + for path in paths: + try: + body = (self.repo / path).read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + missing.append(f" {path} — could not be read") + continue + block = f"--- {path} ---\n{body}\n" + if spent + len(block) > self.context_policy.budget: + missing.append(f" {path} — {len(body)} characters, too large to include") + continue + blocks.append(block) + spent += len(block) + omitted = ( + "\nNot included, so you have not seen them:\n" + "\n".join(missing) if missing else "" + ) + return REVIEW_CONTEXT_PROMPT.format(files="\n".join(blocks), omitted=omitted) + + def _review_checks_prompt(self) -> str: + """Which commands passed, and that the harness ran them.""" + commands = [" ".join(command) for command in self.checks.commands if command] + if not commands: + return REVIEW_NO_CHECKS_PROMPT + return REVIEW_CHECKS_PROMPT.format( + commands="\n".join(f" {command}" for command in commands) + ) + + def _starved_prompt(self, starved: Sequence[str]) -> str: + """Supporting targets that did not fit, named so they are not guessed at.""" + if not starved: + return "" + return STARVED_PROMPT.format( + paths="\n".join(f" {path} ({self._size_of(path)})" for path in starved) + ) + + def _repo_listing(self) -> str: + """The tracked paths, for the planner. + + The planner's whole job is to name files, and it was given only the + brief — so it could only name a path the brief had already quoted, and + anything else was either a guess or an honest `cannot_identify_target`. + Measured on rdpapp: an item that described a surface rather than a file + got "I do not have the repository tree or actual file paths", and the + implementer then received relevance-guessed files instead of the one + the work was in. + + Paths only. The planner needs to know what exists; reading it is the + implementer's job and it has its own budget for that. + """ + try: + tracked = [path for path in run_git(self.repo, "ls-files").splitlines() if path] + except GitError: + return "" + if not tracked: + return "" + lines: list[str] = [] + spent = 0 + for path in tracked: + line = f" {path}\n" + if spent + len(line) > self.context_policy.budget: + lines.append(f" … and {len(tracked) - len(lines)} more not shown\n") + break + lines.append(line) + spent += len(line) + return PLAN_LISTING_PROMPT.format(listing="".join(lines).rstrip("\n")) + + def _prior_failure_prompt(self, record: WorkRecord) -> str: + """Why the last attempt was refused, for the attempt replacing it. + + Bounded, because a check's output can be a whole build log and the + useful part is at the top, where the tool says what it objected to. + """ + error = (record.last_error or "").strip() + if not error: + return "" + return PRIOR_FAILURE_PROMPT.format(error=error[:4000]) + + def _checks_prompt(self) -> str: + """The project's checks, as the implementer's prompt renders them.""" + commands = [" ".join(command) for command in self.checks.commands if command] + if not commands: + return "" + return CHECKS_PROMPT.format(commands="\n".join(f" {command}" for command in commands)) + + def _size_of(self, path: str) -> str: + """How big a file the budget could not fit, for the message that says so. + + A number nobody can read is not evidence, and "does not fit" without + one leaves the reader guessing whether the budget is off by a little or + by two orders of magnitude. Unreadable is reported as unknown rather + than as zero: a size of 0 would be a measurement, and this is not one. + """ + try: + return f"{(self.repo / path).stat().st_size} bytes" + except OSError: + return "size unknown" + def _prepare_branch(self, branch: str, base: str | None = None) -> None: """A clean tree at `base`, on a branch of this item's own. diff --git a/src/agent_harness/outcomes.py b/src/agent_harness/outcomes.py index 0b62add..98e2a8f 100644 --- a/src/agent_harness/outcomes.py +++ b/src/agent_harness/outcomes.py @@ -199,6 +199,13 @@ def as_dict(self) -> dict[str, Any]: #: A question went unanswered for longer than the hold allowed. The item is #: `blocked`, never `ready`: a hold that times out has not been approved. HOLD_EXPIRED = "hold_expired" +#: The harness could not show the implementer a file the planner named, because +#: that file alone is larger than the whole context budget. Kept apart from +#: `NO_TARGET`, which is the planner failing to find one: here the target is +#: known and correct, and the *harness* cannot supply it. Retrying changes +#: nothing — the file is the size it is — so it needs a person to raise the +#: budget or split the file, which is why it escalates. +CONTEXT_UNAVAILABLE = "context_unavailable" REASON_KINDS = ( CHECKS_FAILED, @@ -216,6 +223,7 @@ def as_dict(self) -> dict[str, Any]: ITEM_WALL_CLOCK, ITEM_SPEND, HOLD_EXPIRED, + CONTEXT_UNAVAILABLE, ) diff --git a/src/agent_harness/schemas.py b/src/agent_harness/schemas.py index f425373..a06feae 100644 --- a/src/agent_harness/schemas.py +++ b/src/agent_harness/schemas.py @@ -116,7 +116,8 @@ class WorkItem(BaseModel): "can branch on it: `checks_failed`, `check_escalated`, `check_transient`, " "`review_rejected`, `patch_rejected`, `no_target`, `worker_error`, " "`provider_exhausted`, `budget_exhausted`, `dependency_invalidated`, " - "`agent_timeout`, `claim_lost`.", + "`agent_timeout`, `claim_lost`, `item_wall_clock`, `item_spend`, " + "`hold_expired`, `context_unavailable`.", ) branch: str | None = None pr_url: str | None = None diff --git a/tests/test_executor.py b/tests/test_executor.py index 3e438b4..257b88e 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -1272,6 +1272,366 @@ def test_context_selection_and_planner_targets_are_observable_events( assert context["fallback_relevance"] is False +def test_a_target_that_cannot_be_shown_stops_the_item_before_the_implementer( + repo: Path, tmp_path: Path +) -> None: + """A file larger than the whole budget is a stop, not a smaller prompt. + + Found importing a second repository whose one relevant source file is + 600 KB. The planner named it, the budget could not hold it, and the + implementer was called anyway — with unrelated files in the space the + target should have had. It answered, because a model asked to patch a file + it has not seen writes a plausible diff rather than refusing. + """ + events: list[dict[str, Any]] = [] + executor, queue, transport = build( + repo, + tmp_path, + { + "planner": json.dumps( + { + "plan": "Edit the large file.", + "targets": [{"path": "large.txt", "reason": "is the thing to change"}], + "cannot_identify_target": None, + } + ), + "implementer": DIFF, + "reviewer": "APPROVED\nfine", + }, + events=events, + ) + executor.context_policy = ContextPolicy(budget=100) + (repo / "large.txt").write_text("target line\n" * 200) + git(repo, "add", "-A") + git(repo, "commit", "-q", "-m", "a file larger than the budget") + add_item(queue) + + executor.run_once() + + assert "implementer" not in transport.roles, "the implementer was paid for an impossible task" + stopped = next(event for event in events if event["outcome"] == "context_unavailable") + assert "large.txt" in stopped["detail"] + assert "2400 bytes" in stopped["detail"], "the size that did not fit is part of the answer" + assert "--context-budget" in stopped["detail"], "and so is what to do about it" + + item = queue.get("T1") + assert item is not None + assert item.state == "blocked", "retrying cannot make the file smaller" + assert item.disposition == "escalated" + assert item.reason_kind == "context_unavailable" + assert item.attempts == 0, "a ceiling this deployment set is not the item failing" + + +def test_the_reviewer_sees_the_files_the_diff_touched(repo: Path, tmp_path: Path) -> None: + """Measured on rdpapp: two of three rejection reasons were "the diff does + not show whether …", which no diff ever can. The prompt lists "the task + cannot be judged from what you were given" as grounds to reject, so a + diff-only reviewer rejects for the shape of its own prompt.""" + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + shown = capturing.prompts["reviewer"] + assert "--- hello.txt ---" in shown + assert "hello harness" in shown, "the file as it now stands, not as it was" + + +def test_a_touched_file_too_large_to_show_the_reviewer_is_named(repo: Path, tmp_path: Path) -> None: + """A reviewer that does not know its view is partial treats it as whole.""" + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + executor.context_policy = ContextPolicy(budget=10) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + shown = capturing.prompts["reviewer"] + assert "Not included, so you have not seen them:" in shown + assert "hello.txt — 14 characters, too large to include" in shown + assert "--- hello.txt ---" not in shown + + +def test_the_reviewer_is_told_the_diff_is_complete_and_which_checks_ran( + repo: Path, tmp_path: Path +) -> None: + """Measured on rdpapp. The reviewer verified a change was correct and in + scope, then rejected it: it could not confirm "no other file has changed", + and treated `Checks: passed` as the author's claim — "no build output was + provided beyond the claim". The harness knows both for certain.""" + checks = Checks(commands=[["true"]]) + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + checks=checks, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + shown = capturing.prompts["reviewer"] + assert "any file not appearing in it is unchanged" in shown + assert "run by the harness on this exact tree" in shown + assert " true" in shown, "and which commands they were" + + +def test_the_reviewer_is_told_scope_belongs_to_the_task(repo: Path, tmp_path: Path) -> None: + """Measured on rdpapp. An item declared two constants and said in as many + words that adopting them at the call sites was out of scope. The reviewer + verified every requirement, quoted the exclusion back, and rejected anyway + because "the repository still has the inconsistency the task described" — + overruling the plan author on scope, which is not its call.""" + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + shown = capturing.prompts["reviewer"] + assert "The task's scope is narrower than the problem." in shown + assert "a rejection is not" in shown + + +def test_a_reviewer_with_no_checks_behind_it_is_told_so(repo: Path, tmp_path: Path) -> None: + """Silence would let it assume there were some.""" + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + assert "You are the only gate it has." in capturing.prompts["reviewer"] + + +def test_a_supporting_target_that_does_not_fit_is_named_not_fatal( + repo: Path, tmp_path: Path +) -> None: + """Losing the file the work is in is fatal; losing a supporting one is not. + + Measured on rdpapp: the planner named the 634 KB file the change belonged + in — which fit — plus a 144 KB file and a document, which did not. Blocking + the item lost work over files nobody was going to edit. + """ + (repo / "big.txt").write_text("x" * 400) + git(repo, "add", "-A") + git(repo, "commit", "-q", "-m", "a supporting file that will not fit") + executor, queue, _ = build( + repo, + tmp_path, + { + "planner": json.dumps( + { + "plan": "Edit the greeting.", + "targets": [ + {"path": "hello.txt", "reason": "the work is here"}, + {"path": "big.txt", "reason": "supporting"}, + ], + "cannot_identify_target": None, + } + ), + "implementer": DIFF, + "reviewer": "APPROVED\nfine", + }, + ) + executor.context_policy = ContextPolicy(budget=300) + capturing = PromptCapturingModel( + { + "planner": json.dumps( + { + "plan": "Edit the greeting.", + "targets": [ + {"path": "hello.txt", "reason": "the work is here"}, + {"path": "big.txt", "reason": "supporting"}, + ], + "cannot_identify_target": None, + } + ), + "implementer": DIFF, + "reviewer": "APPROVED\nfine", + } + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + shown = capturing.prompts["implementer"] + assert "hello world" in shown, "the file the work is in was still supplied" + assert "do not change them" in shown + assert "big.txt" in shown + record = queue.get("T1") + assert record is not None and record.state != "blocked" + + +def test_the_planner_is_shown_what_files_exist(repo: Path, tmp_path: Path) -> None: + """Its entire job is naming paths, and it was given only the brief. + + Measured on rdpapp: an item whose brief described a surface rather than + quoting a path got back "I do not have the repository tree or actual file + paths", and the implementer was then handed relevance-guessed files + instead of the one the work was in. + """ + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + shown = capturing.prompts["planner"] + assert "Files in this repository:" in shown + assert "hello.txt" in shown + assert "hello world" not in shown, "paths only — reading them is the implementer's job" + + +def test_a_listing_too_long_for_the_budget_says_how_much_is_missing( + repo: Path, tmp_path: Path +) -> None: + for index in range(200): + (repo / f"file-{index:03}.txt").write_text("x") + git(repo, "add", "-A") + git(repo, "commit", "-q", "-m", "many files") + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + executor.context_policy = ContextPolicy(budget=200) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + assert "more not shown" in capturing.prompts["planner"] + + +def test_a_retry_is_told_why_the_last_attempt_was_refused(repo: Path, tmp_path: Path) -> None: + """Measured on rdpapp: three of four attempts at one item were refused by + the same formatter, each unaware the last had been.""" + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + queue.release("T1", FAILED, error="`cargo fmt --all -- --check` failed: line 12 too long") + queue.requeue("T1") + + executor.run_once() + + shown = capturing.prompts["implementer"] + assert "line 12 too long" in shown + assert "not continuing that attempt" in shown, "a new attempt, not a resumption" + + +def test_a_first_attempt_is_told_about_no_prior_failure(repo: Path, tmp_path: Path) -> None: + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + assert "previous attempt" not in capturing.prompts["implementer"] + + +def test_the_implementer_is_told_which_checks_will_judge_it(repo: Path, tmp_path: Path) -> None: + """The reviewer was told what the checks said; the writer was told nothing. + + Measured on rdpapp: a correct change — the right function, in the right + place, with both tests — was refused by `cargo fmt --all -- --check`, a + command the model was never shown. That costs an attempt and two model + calls to discover something the harness knew before it asked. + """ + checks = Checks(commands=[["cargo", "fmt", "--all", "--", "--check"]]) + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + checks=checks, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + assert "cargo fmt --all -- --check" in capturing.prompts["implementer"] + + +def test_a_project_with_no_checks_says_nothing_about_them(repo: Path, tmp_path: Path) -> None: + """No checks configured must read exactly as it did before.""" + executor, queue, _ = build( + repo, + tmp_path, + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"}, + ) + capturing = PromptCapturingModel( + {"planner": "plan", "implementer": DIFF, "reviewer": "APPROVED\nfine"} + ) + executor.client.transport = capturing + add_item(queue) + + executor.run_once() + + assert "run on your diff" not in capturing.prompts["implementer"] + + def test_the_implementer_is_shown_the_repository(repo: Path, tmp_path: Path) -> None: """The regression for #135.