From 3f55b9ea985e115642679bc64f5175b29ca4a5e6 Mon Sep 17 00:00:00 2001 From: Jorge Garcia Date: Tue, 29 Sep 2026 18:33:46 -0700 Subject: [PATCH 1/5] fix(sourcehunt): make file read pagination truthful --- REREADS.md | 212 +++++++++++++++++++++++ clearwing/agent/tools/hunt/deep_agent.py | 15 +- clearwing/sourcehunt/hunter.py | 71 ++++++-- tests/test_deep_agent_loop.py | 74 ++++++++ tests/test_deep_agent_tools.py | 4 +- tests/test_sourcehunt_hunter.py | 1 + 6 files changed, 355 insertions(+), 22 deletions(-) create mode 100644 REREADS.md diff --git a/REREADS.md b/REREADS.md new file mode 100644 index 00000000..bc0fbcfd --- /dev/null +++ b/REREADS.md @@ -0,0 +1,212 @@ +# Sourcehunt reread investigation + +> This document describes the behavior observed in the September 28 runs. +> The read-path changes below were made afterward; the trace evidence and +> historical code analysis remain as recorded. + +## Read-path fix status + +The deep `read_file` default is now 100 lines. The hunter sends complete source +lines up to a 12,000-character target, reports the file's total lines and an +explicit next line, and sets EOF only when the delivered content reaches the +file end. Tool-start events use the same requested range as the handler. +The handler's existing 100,000-character raw output cap still applies to +exceptionally long source lines. + +The run-wide coverage ledger, policy for deliberate rereads, and resolved-lead +memory are separate follow-up areas. The September 28 traces predate the +read-path changes, so they do not measure their effect. + +## TL;DR + +The strongest reread causes are: + +- `read_file` accepts a default 2,000-line window, but the hunter sends only 3,000 characters of that result to the model. Large requests therefore become implicit pagination work. +- The response says `EOF: True` when the **requested** range reaches the file end, even when the model received only a clipped prefix. That makes the range metadata contradictory. +- The durable read ledger used by the stall guard is internal. The model does not receive `deep_read_ranges`, and the model-facing `visible_read_ranges` ledger is cleared after context compaction. +- Overlapping reads are not blocked. The duplicate-call guard catches repeated identical calls, while different overlapping ranges remain valid tool calls. +- Pagination and recovery reads consume ordinary model steps, tokens, cost, and time. The runtime does not distinguish necessary recovery from redundant rereading. +- The tool schema advertises a 2,000-line default while the hunt guidance recommends 40–120-line windows. The schema makes broad first reads the natural behavior. +- Long hunts lose detailed reasoning during context summarization. There is no durable model-visible ledger for inspected ranges or resolved leads, so the model can reopen hypotheses and reread code it previously resolved. + +The September 28 traces support two distinct behaviors. Many apparent overlaps were legitimate pagination after the 3,000-character clip. The OpenSSL trace also shows genuine non-convergent rereading: after dismissing its main lead, it made seven consecutive reads that returned no new lines. The jq trace ended with a model-service request failure; gitoxide spent most of its late runtime waiting for compilation; html-sanitizer spent its late runtime trying to obtain PHP. + +## Evidence from the September 28 traces + +Counts below use the line ranges actually shown to the model in trajectory `tool_result` summaries, not the larger ranges requested by the model. + +| Hunt | `read_file` calls | Calls with repeated shown lines | Fully redundant calls | Repeated shown lines | +|---|---:|---:|---:|---:| +| OpenSSL (`sh-6a1007b0`) | 46 | 11 | 10 | 465 | +| jq (`sh-20215022`) | 50 | 4 | 2 | 167 | +| gitoxide (`sh-1f3e2641`) | 27 | 3 | 1 | 8 | +| html-sanitizer (`sh-f5011a27`) | 17 | 3 | 0 | 8 | + +OpenSSL is the clearest stall example. Steps 53–59 returned no new lines after the model dismissed its OCB tag hypothesis at step 52. The model then stopped with `stop_reason=stalled` at step 60. Its reasoning repeatedly said to re-read or reconsider already examined nonce, stream, table, and tag logic. + +The other traces show why requested-range overlap is not enough to diagnose the problem. For example, a request for lines 1–2000 can expose only lines 1–98 to the model. The next request beginning at line 98 overlaps the requested interval but is necessary to retrieve content that was never visible. + +## 1. Response clipping turns one read into multiple model steps + +The deep-agent `read_file` handler executes `awk` over the requested range: + +- `clearwing/agent/tools/hunt/deep_agent.py:300-338` + +The handler's local output cap is 100,000 characters (`deep_agent.py:41`, `deep_agent.py:109-114`). The sourcehunt loop then applies a stricter cap before constructing the model-facing result: + +- `clearwing/sourcehunt/hunter.py:2687-2724` +- `hunter.py:2698` calls `_clip_text(content, 3000)`. + +The model therefore may request 2,000 lines but receive only the first few thousand characters. The `Returned` range is calculated from the clipped text (`hunter.py:2699-2704`), so `Returned: 1-98` accurately describes what the model saw in that response. + +This creates extra model turns for large files. Those turns are ordinary investigation steps and consume context, tokens, cost, and wall-clock time even when they only retrieve the rest of a previously requested window. + +## 2. `EOF` describes the request, not the delivered response + +The response metadata computes EOF from the requested range: + +- `clearwing/sourcehunt/hunter.py:2707` + +```python +eof = total_lines is not None and requested[1] >= total_lines +``` + +It does not use the last line actually delivered. This permits a contradictory response such as: + +```text +Requested: 1-2000 +Returned: 1-98 +EOF: True +Truncated: True +``` + +The model can reasonably interpret `EOF: True` as saying that there is no more source to retrieve, while `Truncated: True` says that the response was clipped. This ambiguity encourages repeated probing and makes the model's next offset choice less reliable. + +## 3. The initial default is much larger than the model-visible window + +The tool schema exposes these defaults: + +- `clearwing/agent/tools/hunt/deep_agent.py:49-67` +- `deep_agent.py:511-520` + +```python +offset = 0 +limit = 2000 +``` + +When the model calls `read_file` with only a path, it is not reporting that it knows the file has 2,000 lines. It is accepting the tool's default maximum window. It generally does not know the total line count unless it first runs a command such as `wc -l`. + +The older hunt rules recommend tight 40–120-line windows (`clearwing/sourcehunt/hunter.py:400-412`), but the deep-agent schema advertises 2,000 lines. This contract mismatch makes broad first reads likely and guarantees more clipping on long files. + +## 4. The model never receives the durable `deep_read_ranges` ledger + +The stall loop keeps a durable per-run map called `deep_read_ranges`: + +- initialization: `clearwing/sourcehunt/hunter.py:1636-1637` +- update: `hunter.py:2183-2197` + +The map is used internally to decide whether a returned range contains any lines not previously returned. It is not serialized into a prompt, situation report, or tool schema. The model cannot directly see a statement such as: + +```text +You already inspected src/foo.c lines 200-300. +``` + +The model may remember earlier tool output while it remains in context, but it has no authoritative run-wide coverage map to consult. The tool also does not reject a new overlapping range. + +## 5. Context compaction resets the model-facing coverage map + +The hunter maintains a second map, `visible_read_ranges`, for calculating the `Overlap` and `New lines` fields shown to the model: + +- initialization: `clearwing/sourcehunt/hunter.py:1606-1613` +- use when formatting a read: `hunter.py:2183-2189` +- update: `hunter.py:2195-2197` + +When the conversation is summarized, the code clears this map: + +- `clearwing/sourcehunt/hunter.py:1829-1833` + +```python +messages = await self.summarizer.summarize(messages, self.llm) +visible_read_ranges.clear() +``` + +The summarizer activates when estimated message size exceeds 80% of 150,000 tokens, approximately 120,000 estimated tokens: + +- `clearwing/data/memory/summarizer.py:23-40` + +The durable `deep_read_ranges` map is not cleared, so the progress guard can still know that a range was seen. The model-facing overlap metadata, however, starts a new epoch. A reread after compaction can therefore be reported as having no overlap even though the same lines were shown earlier in the run. + +The summarizer preserves some old tool messages, but it does not provide a durable, explicit source-coverage ledger or resolved-lead ledger in the model prompt. That leaves the model dependent on imperfect memory and attention over the remaining conversation. + +## 6. The duplicate-call guard does not block overlapping ranges + +The runtime counts repeated tool calls by a key based on tool name and serialized arguments: + +- `clearwing/sourcehunt/hunter.py:2020-2070` + +For `read_file`, arguments are deliberately kept literal because offsets and limits are legitimate pagination parameters. As a result, calls such as these are different calls: + +```text +read_file(... offset=213, limit=50) # lines 214-263 +read_file(... offset=213, limit=30) # lines 214-243 +``` + +The exact-call skip mechanism does not identify the second call as a subset of the first. The range-aware progress check notices that it adds no new lines, but only after the model has spent the step. + +## 7. Necessary pagination and redundant rereads share the same budget + +The stall guard treats a read as progress only when its returned range contains previously unseen lines: + +- `clearwing/sourcehunt/hunter.py:1621-1628` +- `hunter.py:2190-2194` +- `_uncovered_read_ranges`: `hunter.py:2655-2675` + +If a clipped response is followed by a continuation containing new lines, the continuation resets the no-progress counter. If a response contains only old lines, it does not. In both cases, however, the model step and token/cost accounting already apply. + +There is no separate budget for source pagination, no refund for a required continuation, and no distinction between: + +- recovering content omitted by the response cap; +- a small boundary overlap; +- deliberate revalidation; and +- a completely redundant reread. + +This makes a large file or a long context more likely to hit time, token, or step limits before the investigation reaches synthesis. + +## 8. Resolved leads are not durable enough to prevent hypothesis churn + +The OpenSSL trace provides direct evidence of a behavioral loop independent of pagination. The model: + +1. formed a plausible OCB tag hypothesis; +2. investigated and dismissed it at step 52; +3. immediately reread already covered OCB ranges at steps 53–59; +4. continued reconsidering related nonce, stream, and table logic; +5. stopped as `stalled` without a finding. + +The runtime tracks potential objects and findings, but it does not inject a concise, durable record saying that a lead was resolved and which evidence resolved it. Context summarization can further weaken the model's memory of the reasoning. The same pattern appeared in jq around speculative string-allocation overflow checks, although jq ultimately failed on a model-service request rather than the stall guard. + +## 9. Range bookkeeping has inconsistent defaults + +The tool schema and handler use a 2,000-line default (`deep_agent.py:56-58`, `deep_agent.py:300-303`), while `_run_tool` uses a 500-line fallback when emitting tool-start events: + +- `clearwing/sourcehunt/hunter.py:2503-2508` + +This does not change the handler's actual read result when the model omits `limit`, but it can make event logs and diagnostics report a different requested end line from the tool that actually ran. That inconsistency complicates reread analysis and can hide how large the model's implicit request was. + +## 10. Observability hides the reasoning needed to separate the causes + +The original Phoenix report script stores model step text and tool calls but does not include `reasoning_content` or hunter span error messages. That made the first pass look like many broad overlapping reads and hid: + +- that most reads were clipped pagination; +- that OpenSSL explicitly re-read after dismissing a lead; and +- that jq's hunter ended with a LiteLLM request error while the root run was marked completed. + +The original trajectories contain the model-visible `Returned`, `Overlap`, and `New lines` headers and are required for accurate reread counts. Any future report should preserve those fields, the hunter `statusMessage`, and model reasoning or a structured explanation of why a read was repeated. + +## References + +- Hunter implementation: `clearwing/sourcehunt/hunter.py` +- Deep-agent tools: `clearwing/agent/tools/hunt/deep_agent.py` +- Context summarization: `clearwing/data/memory/summarizer.py` +- September 28 normalized report: `/tmp/sourcehunts_sep28_2026.json` +- September 28 reasoning export: `/tmp/sourcehunts_sep28_reasoning.json` +- Original trajectory records: Phoenix run artifacts under `cvehunt-runs/20260928T182839/artifacts/.../results/sourcehunt/` diff --git a/clearwing/agent/tools/hunt/deep_agent.py b/clearwing/agent/tools/hunt/deep_agent.py index e13ead38..e896bc7c 100644 --- a/clearwing/agent/tools/hunt/deep_agent.py +++ b/clearwing/agent/tools/hunt/deep_agent.py @@ -39,6 +39,7 @@ logger = logging.getLogger(__name__) _OUTPUT_CAP = 100_000 # 100 KB cap on stdout/stderr per execute call +READ_FILE_DEFAULT_LINES = 100 class ExecuteInput(ToolInputModel): @@ -54,8 +55,8 @@ class ReadFileInput(ToolInputModel): default=0, description="Line offset (0-based, default 0). Or use start_line (1-based)." ) limit: int = Field( - default=2000, - description="Max lines to return (default 2000). Or use end_line with start_line.", + default=READ_FILE_DEFAULT_LINES, + description="Max lines to return (default 100). Or use end_line with start_line.", ) start_line: int | None = Field( default=None, @@ -300,7 +301,7 @@ def execute(command: str, timeout: int = 300, **_: object) -> dict: def read_file( path: str, offset: int = 0, - limit: int = 2000, + limit: int = READ_FILE_DEFAULT_LINES, start_line: int | None = None, end_line: int | None = None, **_: object, @@ -309,11 +310,14 @@ def read_file( return "error: no sandbox available" # Accept both idioms. Model naturally reaches for start_line/end_line # (the prompt used to advertise them); swallowing them via **_ made the - # tool silently return lines 1-2000 and looked like a tool bug. + # tool silently return the default first window and looked like a tool bug. if start_line is not None: + start_line = max(1, start_line) offset = start_line - 1 if end_line is not None: limit = max(1, end_line - start_line + 1) + offset = max(0, offset) + limit = max(1, limit) start = offset + 1 end = offset + limit # Previously this was `sed ... | cat -n`, which numbers output @@ -514,7 +518,8 @@ def read_function(name: str, **_: object) -> dict: "Read lines from a relevant grep_source or find_source hit. " "Repository-relative paths resolve inside /workspace. " "Parameters: path (required), offset (line offset, default 0), " - "limit (max lines, default 2000). No other parameters exist." + "limit (max lines, default 100), or start_line and end_line. " + "The response reports returned lines and the next line to read." ), schema=ReadFileInput.model_json_schema(), handler=read_file, diff --git a/clearwing/sourcehunt/hunter.py b/clearwing/sourcehunt/hunter.py index 60df7cd4..f4afdf29 100644 --- a/clearwing/sourcehunt/hunter.py +++ b/clearwing/sourcehunt/hunter.py @@ -24,6 +24,7 @@ build_hunter_tools, build_propagation_auditor_tools, ) +from clearwing.agent.tools.hunt.deep_agent import READ_FILE_DEFAULT_LINES from clearwing.core.events import EventBus, EventType from clearwing.data.memory import ContextSummarizer from clearwing.llm import ( @@ -1000,7 +1001,7 @@ def _build_propagation_prompt(file_target: FileTarget) -> str: - execute(command): Build, debug, and test with gcc, gdb, strace, valgrind, make, and similar commands. Shell grep/find/rg searches are blocked. - grep_source(pattern, path, file_glob): Search repository source content with bounded results. - find_source(query, path, kind): Find repository files and directories by name. -- read_file(path): Read a file from the container. +- read_file(path, offset, limit): Read a source window from the container. - write_file(path, contents): Write a file in the container. - record_trace_step(file, line, function, code_snippet, note): Record one step in the vulnerability dataflow trace as you read code. Build the trace incrementally from attacker entry to sink. - record_finding(...): Submit a vulnerability finding with severity, CWE, evidence level, and description. @@ -1017,6 +1018,8 @@ def _build_propagation_prompt(file_target: FileTarget) -> str: DEEP_SEARCH_GUIDANCE = """ For source navigation, use grep_source for content and find_source for filenames or directories. Then read_file on a relevant hit before drawing a conclusion. +read_file reports the lines actually returned, total lines, and Next line. +When more source is needed, continue at Next line; EOF refers to returned content. Both searches are limited to the repository. A no_matches result means the query found nothing in that scope; a truncated result needs a narrower scope. If successive queries return the same locations, change approach or finish. @@ -2075,9 +2078,9 @@ async def arun(self) -> HunterRunResult: rel = str(fpath).removeprefix("/workspace/").removeprefix("/") files_visited.add(rel) if tool_call.fn_name == "read_file": - offset = int(tool_arguments.get("offset", 0)) - limit = int(tool_arguments.get("limit", 2000)) - lines_read.setdefault(rel, []).append((offset + 1, offset + limit)) + lines_read.setdefault(rel, []).append( + _requested_read_range(tool_arguments) + ) elif tool_call.fn_name == "flag_potential": flags_raised += 1 @@ -2501,10 +2504,13 @@ async def _run_tool( } sandbox_id = self.ctx.sandbox.short_id if self.ctx.sandbox else None if tool_call.fn_name in ("read_source_file", "read_file"): - offset = arguments.get("offset", 0) - limit = arguments.get("limit", 500) - start = arguments.get("start_line", offset + 1) - end = arguments.get("end_line", offset + limit) + if tool_call.fn_name == "read_file": + start, end = _requested_read_range(arguments) + else: + offset = arguments.get("offset", 0) + limit = arguments.get("limit", 500) + start = arguments.get("start_line", offset + 1) + end = arguments.get("end_line", offset + limit) EventBus().emit( EventType.TOOL_START, { @@ -2621,11 +2627,13 @@ def _requested_read_range(arguments: dict[str, Any]) -> tuple[int, int]: start = max(1, int(start_line)) end_line = arguments.get("end_line") end = ( - int(end_line) if end_line is not None else start + int(arguments.get("limit", 2000)) - 1 + int(end_line) + if end_line is not None + else start + int(arguments.get("limit", READ_FILE_DEFAULT_LINES)) - 1 ) return start, max(start, end) offset = max(0, int(arguments.get("offset", 0))) - limit = max(1, int(arguments.get("limit", 2000))) + limit = max(1, int(arguments.get("limit", READ_FILE_DEFAULT_LINES))) return offset + 1, offset + limit @@ -2676,6 +2684,24 @@ def _uncovered_read_ranges( _READ_FILE_METADATA = re.compile(r"\n?\[CLEARWING_READ_METADATA total_lines=(\d+)\]\s*$") +_READ_FILE_OUTPUT_TRUNCATION = re.compile(r"\n\n\[file truncated at \d+ characters\]\s*$") +# Bound model context while preserving a complete last line and a usable cursor. +_READ_FILE_VISIBLE_CHAR_LIMIT = 12_000 + + +def _clip_read_file_lines(content: str, limit: int) -> tuple[str, bool]: + """Keep complete source lines so the continuation cursor stays accurate.""" + if len(content) <= limit: + return content, False + chunks = content.splitlines(keepends=True) + kept: list[str] = [] + size = 0 + for chunk in chunks: + if kept and size + len(chunk) > limit: + break + kept.append(chunk) + size += len(chunk) + return "".join(kept), len(kept) < len(chunks) def _format_line_ranges(ranges: list[tuple[int, int]]) -> str: @@ -2691,11 +2717,17 @@ def _read_file_tool_response( ) -> tuple[str, tuple[int, int] | None]: """Render truthful read metadata from source lines actually sent to the model.""" + if raw_output.startswith(("error reading ", "error: no sandbox available")): + return raw_output, None requested = _requested_read_range(arguments) metadata_match = _READ_FILE_METADATA.search(raw_output) total_lines = int(metadata_match.group(1)) if metadata_match else None content = _READ_FILE_METADATA.sub("", raw_output) - rendered_content = _clip_text(content, 3000) + raw_truncated = bool(_READ_FILE_OUTPUT_TRUNCATION.search(content)) + content = _READ_FILE_OUTPUT_TRUNCATION.sub("", content) + rendered_content, display_truncated = _clip_read_file_lines( + content, _READ_FILE_VISIBLE_CHAR_LIMIT + ) numbered = [ int(match.group(1)) for line in rendered_content.splitlines() @@ -2704,18 +2736,27 @@ def _read_file_tool_response( returned = (numbered[0], numbered[-1]) if numbered else None coverage = _range_coverage_fraction(returned, visible_ranges) if returned is not None else 0.0 uncovered = _uncovered_read_ranges(returned, visible_ranges) if returned is not None else [] - eof = total_lines is not None and requested[1] >= total_lines - truncated = bool(re.search(r"\[(?:file )?truncated at \d+ characters\]", content)) or len( - content - ) > len(rendered_content) + truncated = raw_truncated or display_truncated + eof = total_lines is not None and ( + (returned is not None and returned[1] >= total_lines and not truncated) + or (returned is None and requested[0] > total_lines) + ) + if eof: + next_line = None + elif returned is not None: + next_line = returned[1] + 1 + else: + next_line = requested[0] header = "\n".join( [ "[READ_FILE]", f"Requested: {requested[0]}-{requested[1]}", f"Returned: {_format_line_ranges([returned] if returned else [])}", + f"Total lines: {total_lines if total_lines is not None else 'Unknown'}", f"EOF: {eof}", f"Truncated: {truncated}", + f"Next line: {next_line if next_line is not None else 'None'}", f"Overlap: {coverage:.0%}", f"New lines: {_format_line_ranges(uncovered)}", "[/READ_FILE]", diff --git a/tests/test_deep_agent_loop.py b/tests/test_deep_agent_loop.py index fc7284b9..78456160 100644 --- a/tests/test_deep_agent_loop.py +++ b/tests/test_deep_agent_loop.py @@ -85,6 +85,80 @@ def test_read_metadata_does_not_treat_source_word_as_truncation(): assert "Truncated: False" in summary +def test_read_metadata_uses_delivered_lines_for_eof_and_continuation(): + source = "".join(f"{line:6d}\t{'x' * 220}\n" for line in range(1, 101)) + summary, returned = _read_file_tool_response( + {"path": "parser.py"}, + f"{source}[CLEARWING_READ_METADATA total_lines=100]", + [], + ) + + assert returned is not None + assert returned[0] == 1 + assert returned[1] < 100 + assert "Requested: 1-100" in summary + assert "Total lines: 100" in summary + assert "EOF: False" in summary + assert "Truncated: True" in summary + assert f"Next line: {returned[1] + 1}" in summary + assert f"{returned[1] + 1:6d}\t" not in summary + assert f"{returned[1]:6d}\t{'x' * 220}" in summary + + +def test_read_metadata_reports_eof_only_after_last_line_is_delivered(): + summary, returned = _read_file_tool_response( + {"path": "parser.py", "offset": 98, "limit": 100}, + " 99\tlast but one\n 100\tlast\n[CLEARWING_READ_METADATA total_lines=100]", + [], + ) + + assert returned == (99, 100) + assert "EOF: True" in summary + assert "Next line: None" in summary + + +def test_read_metadata_reports_eof_for_request_past_end(): + summary, returned = _read_file_tool_response( + {"path": "parser.py", "offset": 100}, + "\n[CLEARWING_READ_METADATA total_lines=100]", + [], + ) + + assert returned is None + assert "EOF: True" in summary + assert "Next line: None" in summary + + +def test_read_metadata_does_not_claim_eof_after_raw_output_cap(): + summary, returned = _read_file_tool_response( + {"path": "parser.py"}, + " 1\tfirst\n 2\tsecond\n\n[file truncated at 100000 characters]" + "\n[CLEARWING_READ_METADATA total_lines=100]", + [], + ) + + assert returned == (1, 2) + assert "EOF: False" in summary + assert "Truncated: True" in summary + assert "Next line: 3" in summary + assert "[file truncated" not in summary + + +def test_read_metadata_preserves_an_oversized_source_line(): + long_line = "x" * 12_100 + summary, returned = _read_file_tool_response( + {"path": "parser.py"}, + f" 1\t{long_line}\n 2\ttail\n[CLEARWING_READ_METADATA total_lines=2]", + [], + ) + + assert returned == (1, 1) + assert f" 1\t{long_line}" in summary + assert " 2\ttail" not in summary + assert "EOF: False" in summary + assert "Next line: 2" in summary + + def test_structured_search_summary_preserves_status_and_next_action(): result = { "status": "truncated", diff --git a/tests/test_deep_agent_tools.py b/tests/test_deep_agent_tools.py index 0cda7f0c..01571bde 100644 --- a/tests/test_deep_agent_tools.py +++ b/tests/test_deep_agent_tools.py @@ -246,9 +246,9 @@ def test_read_file_with_defaults(tools, mock_sandbox): cmd = mock_sandbox.exec.call_args[0][0] assert "awk" in cmd assert "/workspace/foo.c" in cmd - # Default offset=0, limit=2000 → start=1, end=2000 + # Default offset=0, limit=100 → start=1, end=100 assert "s=1" in cmd - assert "e=2000" in cmd + assert "e=100" in cmd assert "line1" in result diff --git a/tests/test_sourcehunt_hunter.py b/tests/test_sourcehunt_hunter.py index 2138f35f..0a9a0184 100644 --- a/tests/test_sourcehunt_hunter.py +++ b/tests/test_sourcehunt_hunter.py @@ -51,6 +51,7 @@ def test_read_range_coverage_distinguishes_refresh_from_new_code() -> None: requested = _requested_read_range({"start_line": 150, "end_line": 249}) + assert _requested_read_range({}) == (1, 100) assert _range_coverage_fraction(requested, [(1, 500)]) == 1.0 assert _range_coverage_fraction(requested, [(1, 199)]) == 0.5 assert _range_coverage_fraction(requested, []) == 0.0 From 4a1e5abf588db5b0ac5b56d4996317c03a452a99 Mon Sep 17 00:00:00 2001 From: Jorge Garcia Date: Tue, 29 Sep 2026 18:40:38 -0700 Subject: [PATCH 2/5] chore: keep reread analysis local --- REREADS.md | 212 ----------------------------------------------------- 1 file changed, 212 deletions(-) delete mode 100644 REREADS.md diff --git a/REREADS.md b/REREADS.md deleted file mode 100644 index bc0fbcfd..00000000 --- a/REREADS.md +++ /dev/null @@ -1,212 +0,0 @@ -# Sourcehunt reread investigation - -> This document describes the behavior observed in the September 28 runs. -> The read-path changes below were made afterward; the trace evidence and -> historical code analysis remain as recorded. - -## Read-path fix status - -The deep `read_file` default is now 100 lines. The hunter sends complete source -lines up to a 12,000-character target, reports the file's total lines and an -explicit next line, and sets EOF only when the delivered content reaches the -file end. Tool-start events use the same requested range as the handler. -The handler's existing 100,000-character raw output cap still applies to -exceptionally long source lines. - -The run-wide coverage ledger, policy for deliberate rereads, and resolved-lead -memory are separate follow-up areas. The September 28 traces predate the -read-path changes, so they do not measure their effect. - -## TL;DR - -The strongest reread causes are: - -- `read_file` accepts a default 2,000-line window, but the hunter sends only 3,000 characters of that result to the model. Large requests therefore become implicit pagination work. -- The response says `EOF: True` when the **requested** range reaches the file end, even when the model received only a clipped prefix. That makes the range metadata contradictory. -- The durable read ledger used by the stall guard is internal. The model does not receive `deep_read_ranges`, and the model-facing `visible_read_ranges` ledger is cleared after context compaction. -- Overlapping reads are not blocked. The duplicate-call guard catches repeated identical calls, while different overlapping ranges remain valid tool calls. -- Pagination and recovery reads consume ordinary model steps, tokens, cost, and time. The runtime does not distinguish necessary recovery from redundant rereading. -- The tool schema advertises a 2,000-line default while the hunt guidance recommends 40–120-line windows. The schema makes broad first reads the natural behavior. -- Long hunts lose detailed reasoning during context summarization. There is no durable model-visible ledger for inspected ranges or resolved leads, so the model can reopen hypotheses and reread code it previously resolved. - -The September 28 traces support two distinct behaviors. Many apparent overlaps were legitimate pagination after the 3,000-character clip. The OpenSSL trace also shows genuine non-convergent rereading: after dismissing its main lead, it made seven consecutive reads that returned no new lines. The jq trace ended with a model-service request failure; gitoxide spent most of its late runtime waiting for compilation; html-sanitizer spent its late runtime trying to obtain PHP. - -## Evidence from the September 28 traces - -Counts below use the line ranges actually shown to the model in trajectory `tool_result` summaries, not the larger ranges requested by the model. - -| Hunt | `read_file` calls | Calls with repeated shown lines | Fully redundant calls | Repeated shown lines | -|---|---:|---:|---:|---:| -| OpenSSL (`sh-6a1007b0`) | 46 | 11 | 10 | 465 | -| jq (`sh-20215022`) | 50 | 4 | 2 | 167 | -| gitoxide (`sh-1f3e2641`) | 27 | 3 | 1 | 8 | -| html-sanitizer (`sh-f5011a27`) | 17 | 3 | 0 | 8 | - -OpenSSL is the clearest stall example. Steps 53–59 returned no new lines after the model dismissed its OCB tag hypothesis at step 52. The model then stopped with `stop_reason=stalled` at step 60. Its reasoning repeatedly said to re-read or reconsider already examined nonce, stream, table, and tag logic. - -The other traces show why requested-range overlap is not enough to diagnose the problem. For example, a request for lines 1–2000 can expose only lines 1–98 to the model. The next request beginning at line 98 overlaps the requested interval but is necessary to retrieve content that was never visible. - -## 1. Response clipping turns one read into multiple model steps - -The deep-agent `read_file` handler executes `awk` over the requested range: - -- `clearwing/agent/tools/hunt/deep_agent.py:300-338` - -The handler's local output cap is 100,000 characters (`deep_agent.py:41`, `deep_agent.py:109-114`). The sourcehunt loop then applies a stricter cap before constructing the model-facing result: - -- `clearwing/sourcehunt/hunter.py:2687-2724` -- `hunter.py:2698` calls `_clip_text(content, 3000)`. - -The model therefore may request 2,000 lines but receive only the first few thousand characters. The `Returned` range is calculated from the clipped text (`hunter.py:2699-2704`), so `Returned: 1-98` accurately describes what the model saw in that response. - -This creates extra model turns for large files. Those turns are ordinary investigation steps and consume context, tokens, cost, and wall-clock time even when they only retrieve the rest of a previously requested window. - -## 2. `EOF` describes the request, not the delivered response - -The response metadata computes EOF from the requested range: - -- `clearwing/sourcehunt/hunter.py:2707` - -```python -eof = total_lines is not None and requested[1] >= total_lines -``` - -It does not use the last line actually delivered. This permits a contradictory response such as: - -```text -Requested: 1-2000 -Returned: 1-98 -EOF: True -Truncated: True -``` - -The model can reasonably interpret `EOF: True` as saying that there is no more source to retrieve, while `Truncated: True` says that the response was clipped. This ambiguity encourages repeated probing and makes the model's next offset choice less reliable. - -## 3. The initial default is much larger than the model-visible window - -The tool schema exposes these defaults: - -- `clearwing/agent/tools/hunt/deep_agent.py:49-67` -- `deep_agent.py:511-520` - -```python -offset = 0 -limit = 2000 -``` - -When the model calls `read_file` with only a path, it is not reporting that it knows the file has 2,000 lines. It is accepting the tool's default maximum window. It generally does not know the total line count unless it first runs a command such as `wc -l`. - -The older hunt rules recommend tight 40–120-line windows (`clearwing/sourcehunt/hunter.py:400-412`), but the deep-agent schema advertises 2,000 lines. This contract mismatch makes broad first reads likely and guarantees more clipping on long files. - -## 4. The model never receives the durable `deep_read_ranges` ledger - -The stall loop keeps a durable per-run map called `deep_read_ranges`: - -- initialization: `clearwing/sourcehunt/hunter.py:1636-1637` -- update: `hunter.py:2183-2197` - -The map is used internally to decide whether a returned range contains any lines not previously returned. It is not serialized into a prompt, situation report, or tool schema. The model cannot directly see a statement such as: - -```text -You already inspected src/foo.c lines 200-300. -``` - -The model may remember earlier tool output while it remains in context, but it has no authoritative run-wide coverage map to consult. The tool also does not reject a new overlapping range. - -## 5. Context compaction resets the model-facing coverage map - -The hunter maintains a second map, `visible_read_ranges`, for calculating the `Overlap` and `New lines` fields shown to the model: - -- initialization: `clearwing/sourcehunt/hunter.py:1606-1613` -- use when formatting a read: `hunter.py:2183-2189` -- update: `hunter.py:2195-2197` - -When the conversation is summarized, the code clears this map: - -- `clearwing/sourcehunt/hunter.py:1829-1833` - -```python -messages = await self.summarizer.summarize(messages, self.llm) -visible_read_ranges.clear() -``` - -The summarizer activates when estimated message size exceeds 80% of 150,000 tokens, approximately 120,000 estimated tokens: - -- `clearwing/data/memory/summarizer.py:23-40` - -The durable `deep_read_ranges` map is not cleared, so the progress guard can still know that a range was seen. The model-facing overlap metadata, however, starts a new epoch. A reread after compaction can therefore be reported as having no overlap even though the same lines were shown earlier in the run. - -The summarizer preserves some old tool messages, but it does not provide a durable, explicit source-coverage ledger or resolved-lead ledger in the model prompt. That leaves the model dependent on imperfect memory and attention over the remaining conversation. - -## 6. The duplicate-call guard does not block overlapping ranges - -The runtime counts repeated tool calls by a key based on tool name and serialized arguments: - -- `clearwing/sourcehunt/hunter.py:2020-2070` - -For `read_file`, arguments are deliberately kept literal because offsets and limits are legitimate pagination parameters. As a result, calls such as these are different calls: - -```text -read_file(... offset=213, limit=50) # lines 214-263 -read_file(... offset=213, limit=30) # lines 214-243 -``` - -The exact-call skip mechanism does not identify the second call as a subset of the first. The range-aware progress check notices that it adds no new lines, but only after the model has spent the step. - -## 7. Necessary pagination and redundant rereads share the same budget - -The stall guard treats a read as progress only when its returned range contains previously unseen lines: - -- `clearwing/sourcehunt/hunter.py:1621-1628` -- `hunter.py:2190-2194` -- `_uncovered_read_ranges`: `hunter.py:2655-2675` - -If a clipped response is followed by a continuation containing new lines, the continuation resets the no-progress counter. If a response contains only old lines, it does not. In both cases, however, the model step and token/cost accounting already apply. - -There is no separate budget for source pagination, no refund for a required continuation, and no distinction between: - -- recovering content omitted by the response cap; -- a small boundary overlap; -- deliberate revalidation; and -- a completely redundant reread. - -This makes a large file or a long context more likely to hit time, token, or step limits before the investigation reaches synthesis. - -## 8. Resolved leads are not durable enough to prevent hypothesis churn - -The OpenSSL trace provides direct evidence of a behavioral loop independent of pagination. The model: - -1. formed a plausible OCB tag hypothesis; -2. investigated and dismissed it at step 52; -3. immediately reread already covered OCB ranges at steps 53–59; -4. continued reconsidering related nonce, stream, and table logic; -5. stopped as `stalled` without a finding. - -The runtime tracks potential objects and findings, but it does not inject a concise, durable record saying that a lead was resolved and which evidence resolved it. Context summarization can further weaken the model's memory of the reasoning. The same pattern appeared in jq around speculative string-allocation overflow checks, although jq ultimately failed on a model-service request rather than the stall guard. - -## 9. Range bookkeeping has inconsistent defaults - -The tool schema and handler use a 2,000-line default (`deep_agent.py:56-58`, `deep_agent.py:300-303`), while `_run_tool` uses a 500-line fallback when emitting tool-start events: - -- `clearwing/sourcehunt/hunter.py:2503-2508` - -This does not change the handler's actual read result when the model omits `limit`, but it can make event logs and diagnostics report a different requested end line from the tool that actually ran. That inconsistency complicates reread analysis and can hide how large the model's implicit request was. - -## 10. Observability hides the reasoning needed to separate the causes - -The original Phoenix report script stores model step text and tool calls but does not include `reasoning_content` or hunter span error messages. That made the first pass look like many broad overlapping reads and hid: - -- that most reads were clipped pagination; -- that OpenSSL explicitly re-read after dismissing a lead; and -- that jq's hunter ended with a LiteLLM request error while the root run was marked completed. - -The original trajectories contain the model-visible `Returned`, `Overlap`, and `New lines` headers and are required for accurate reread counts. Any future report should preserve those fields, the hunter `statusMessage`, and model reasoning or a structured explanation of why a read was repeated. - -## References - -- Hunter implementation: `clearwing/sourcehunt/hunter.py` -- Deep-agent tools: `clearwing/agent/tools/hunt/deep_agent.py` -- Context summarization: `clearwing/data/memory/summarizer.py` -- September 28 normalized report: `/tmp/sourcehunts_sep28_2026.json` -- September 28 reasoning export: `/tmp/sourcehunts_sep28_reasoning.json` -- Original trajectory records: Phoenix run artifacts under `cvehunt-runs/20260928T182839/artifacts/.../results/sourcehunt/` From 849fd879fdd532b59611d2ec20f2a4f2d59024df Mon Sep 17 00:00:00 2001 From: Jorge Garcia Date: Wed, 30 Sep 2026 16:22:15 -0700 Subject: [PATCH 3/5] Track source coverage and retry redundant reads --- clearwing/agent/tools/hunt/deep_agent.py | 5 + clearwing/sourcehunt/hunter.py | 264 ++++++++++++++++------- tests/test_deep_agent_loop.py | 178 ++++++++++++++- 3 files changed, 372 insertions(+), 75 deletions(-) diff --git a/clearwing/agent/tools/hunt/deep_agent.py b/clearwing/agent/tools/hunt/deep_agent.py index e896bc7c..9551ae43 100644 --- a/clearwing/agent/tools/hunt/deep_agent.py +++ b/clearwing/agent/tools/hunt/deep_agent.py @@ -65,6 +65,10 @@ class ReadFileInput(ToolInputModel): end_line: int | None = Field( default=None, description="Alias — 1-based inclusive end line. Requires start_line." ) + refresh: bool = Field( + default=False, + description="Set true to deliberately reread lines already shown during this hunt.", + ) class WriteFileInput(ToolInputModel): @@ -519,6 +523,7 @@ def read_function(name: str, **_: object) -> dict: "Repository-relative paths resolve inside /workspace. " "Parameters: path (required), offset (line offset, default 0), " "limit (max lines, default 100), or start_line and end_line. " + "Set refresh=true to deliberately revisit covered lines. " "The response reports returned lines and the next line to read." ), schema=ReadFileInput.model_json_schema(), diff --git a/clearwing/sourcehunt/hunter.py b/clearwing/sourcehunt/hunter.py index f4afdf29..8ce9dfc8 100644 --- a/clearwing/sourcehunt/hunter.py +++ b/clearwing/sourcehunt/hunter.py @@ -12,6 +12,7 @@ import json import logging import os +import posixpath import re import time from dataclasses import dataclass, field @@ -1583,6 +1584,88 @@ def _should_stop(self, step: int, cost_usd: float, steps_since_progress: int = 0 return "stalled" return None + async def _request_with_coverage_retry( + self, + messages: list[ChatMessage], + active_tools: list[NativeToolSpec], + read_coverage: _ReadCoverageLedger, + warned_read_paths: set[str], + trajectory: HunterTrajectoryLogger, + step: int, + ) -> tuple[Any, int, int, float]: + """Retry one unprompted covered read before committing its tool calls.""" + + provider_name = getattr(self.llm, "provider_name", None) + total_input = 0 + total_output = 0 + total_cost = 0.0 + coverage_retry_used = False + while True: + response = await self.llm.achat( + messages=list(messages), + system=self.prompt, + tools=active_tools, + cache_prefix=True, + prompt_cache_key=(f"{self.ctx.session_id or ''}:{self.ctx.work_item_id or ''}"), + ) + input_tokens = response.usage.prompt_tokens or 0 + output_tokens = response.usage.completion_tokens or 0 + details = getattr(response.usage, "prompt_tokens_details", None) + cached_tokens = (getattr(details, "cached_tokens", None) or 0) if details else 0 + call_cost = _estimate_cost_usd( + input_tokens, output_tokens, self.llm.model_name, cached_tokens + ) + total_input += input_tokens + total_output += output_tokens + total_cost += call_cost + if input_tokens or output_tokens: + CostTracker().record_llm_call( + input_tokens, + output_tokens, + self.llm.model_name, + cached_tokens=cached_tokens, + provider=provider_name, + ) + + warning: tuple[str, str] | None = None + if self.agent_mode == "deep" and not coverage_retry_used: + for proposed_call in response.tool_calls: + if proposed_call.fn_name != "read_file": + continue + arguments = proposed_call.fn_arguments + if not isinstance(arguments, dict) or arguments.get("refresh") is True: + continue + path = str(arguments.get("path") or "") + key = _canonical_read_path(path) + if key in warned_read_paths: + continue + note = read_coverage.redundant_read_note(path, _requested_read_range(arguments)) + if note: + warning = (key, note) + break + if warning is None: + return response, total_input, total_output, total_cost + + # Nothing from the rejected assistant response has entered messages, + # so the retry leaves no unmatched tool call or result behind. + key, note = warning + warned_read_paths.add(key) + coverage_retry_used = True + trajectory.log( + "coverage_retry", + { + "step": step, + "path": key, + "content": note, + "rejected_tool_calls": [ + _serialize_tool_call(call) for call in response.tool_calls + ], + "reasoning_content": response.reasoning_content, + "usage": {"input_tokens": input_tokens, "output_tokens": output_tokens}, + }, + ) + messages.append(ChatMessage("user", note)) + @tracer.agent(name="sourcehunt.hunter") async def arun(self) -> HunterRunResult: user_msg = ( @@ -1606,14 +1689,11 @@ async def arun(self) -> HunterRunResult: last_reasoning_content = "" last_output_tokens = 0 files_visited: set[str] = set() - # {rel_path: set of (start_line, end_line) tuples from read_file calls} - lines_read: dict[str, list[tuple[int, int]]] = {} - # Ranges still present in the active conversation epoch. Unlike - # lines_read, this is cleared after compaction so a focused reread can - # legitimately boost code back into recent context. - visible_read_ranges: dict[str, list[tuple[int, int]]] = { - path: list(ranges) for path, ranges in self.ctx.read_ranges.items() - } + read_coverage = _ReadCoverageLedger() + for path, ranges in self.ctx.read_ranges.items(): + for returned_range in ranges: + read_coverage.add(path, returned_range) + warned_read_paths: set[str] = set() flags_raised: int = 0 empty_response_nudges: int = 0 potential_reminder_active = False @@ -1622,9 +1702,8 @@ async def arun(self) -> HunterRunResult: synthesis_injected = False step = 0 # Progress guard: a step counts as progress when it adds a finding, a - # potential, reads a previously-unread file, or returns a previously- - # unread range from a deep read_file call. A successful execute or - # write_file does not count — otherwise a stream of varied failing + # potential, or returns previously-undelivered source lines. A successful + # execute or write_file does not count — otherwise a stream of varied failing # commands would silently reset the stall counter. # When nothing advances for max_steps_without_progress steps the hunter # stops (see _should_stop). Repeated structured searches count toward @@ -1633,10 +1712,8 @@ async def arun(self) -> HunterRunResult: len(self.ctx.findings), tuple(sorted(str(p.get("id")) for p in self.ctx.potentials)), len(self.ctx.files_read), - len(self.ctx.deep_files_read), 0, ) - deep_read_ranges: dict[str, list[tuple[int, int]]] = {} deep_read_range_progress = 0 seen_search_locations: set[tuple[str, str, int]] = set() search_progress_epoch = 0 @@ -1650,7 +1727,6 @@ async def arun(self) -> HunterRunResult: len(self.ctx.findings), tuple(sorted(str(p.get("id")) for p in self.ctx.potentials)), len(self.ctx.files_read), - len(self.ctx.deep_files_read), deep_read_range_progress, ) if progress_sig != last_progress_sig: @@ -1832,10 +1908,12 @@ async def arun(self) -> HunterRunResult: if self.summarizer and self.summarizer.should_summarize(messages): pre = len(messages) messages = await self.summarizer.summarize(messages, self.llm) - visible_read_ranges.clear() + # The targeted warning may have been summarized away. Give + # the model one fresh reminder before penalizing a later + # covered read whose warning is no longer in context. + warned_read_paths.clear() logger.info("Hunter context summarized: %d → %d messages", pre, len(messages)) - provider_name = getattr(self.llm, "provider_name", None) active_tools = [] if final_synthesis_turn else self.tools if final_decision_turn and active_tools: active_tools = [ @@ -1852,30 +1930,22 @@ async def arun(self) -> HunterRunResult: active_tools = [ tool for tool in active_tools if tool.name not in inactive_potential_tools ] - response = await self.llm.achat( - messages=messages, - system=self.prompt, - tools=active_tools, - # Prompt caching: mark the growing prefix cacheable so each - # turn re-reads system + tools + prior history from cache - # instead of paying full input price to re-send it. This is - # a transport/billing hint only — the model still receives - # byte-identical input, so findings are unchanged. Inert on - # providers without caching. The key is stable per hunt so - # OpenAI-style routing keeps hitting the same prefix cache. - cache_prefix=True, - prompt_cache_key=(f"{self.ctx.session_id or ''}:{self.ctx.work_item_id or ''}"), - ) - input_tokens = response.usage.prompt_tokens or 0 - output_tokens = response.usage.completion_tokens or 0 - details = getattr(response.usage, "prompt_tokens_details", None) - cached_tokens = (getattr(details, "cached_tokens", None) or 0) if details else 0 - call_cost = _estimate_cost_usd( - input_tokens, - output_tokens, - self.llm.model_name, - cached_tokens, + ( + response, + request_input, + request_output, + request_cost, + ) = await self._request_with_coverage_retry( + messages, + active_tools, + read_coverage, + warned_read_paths, + trajectory, + step, ) + total_input_tokens += request_input + total_output_tokens += request_output + total_cost_usd += request_cost # Preserve the provider's reasoning_content alongside the # visible text. `response.first_text` only returns the # first Text part — reasoning/thinking blocks are separate @@ -1903,23 +1973,6 @@ async def arun(self) -> HunterRunResult: "model": response.provider_model_name, }, ) - total_input_tokens += response.usage.prompt_tokens or 0 - total_output_tokens += response.usage.completion_tokens or 0 - # Older genai-pyo3 responses and lightweight test doubles may not - # expose prompt_tokens_details at all. Treat that the same as a - # response where nothing was cache-served. - total_cost_usd += call_cost - # Keep process-wide cost/UI metrics separate from the OTel span, - # which is emitted directly around the model request above. - if input_tokens or output_tokens: - CostTracker().record_llm_call( - input_tokens, - output_tokens, - self.llm.model_name, - cached_tokens=cached_tokens, - provider=provider_name, - ) - last_assistant_text = response.first_text or "" last_reasoning_content = response.reasoning_content or "" last_output_tokens = response.usage.completion_tokens or 0 @@ -2077,10 +2130,6 @@ async def arun(self) -> HunterRunResult: if fpath: rel = str(fpath).removeprefix("/workspace/").removeprefix("/") files_visited.add(rel) - if tool_call.fn_name == "read_file": - lines_read.setdefault(rel, []).append( - _requested_read_range(tool_arguments) - ) elif tool_call.fn_name == "flag_potential": flags_raised += 1 @@ -2185,19 +2234,16 @@ async def arun(self) -> HunterRunResult: seen_search_locations.update(locations) if tool_call.fn_name == "read_file" and isinstance(tool_output, str): reread_path = str(tool_arguments.get("path") or "") + read_coverage.note_file_length(reread_path, tool_output) tool_summary, returned_range = _read_file_tool_response( tool_arguments, tool_output, - visible_read_ranges.get(reread_path, []), + read_coverage.ranges_for(reread_path), ) - if returned_range is not None: - ranges = deep_read_ranges.setdefault(reread_path, []) - if _uncovered_read_ranges(returned_range, ranges): - deep_read_range_progress += 1 - ranges.append(returned_range) - visible_read_ranges.setdefault(reread_path, []).append( - returned_range - ) + if returned_range is not None and read_coverage.add( + reread_path, returned_range + ): + deep_read_range_progress += 1 else: tool_summary = _tool_output_text( tool_call.fn_name, @@ -2637,6 +2683,77 @@ def _requested_read_range(arguments: dict[str, Any]) -> tuple[int, int]: return offset + 1, offset + limit +def _canonical_read_path(path: str) -> str: + """Use one coverage key for repository-relative and /workspace paths.""" + + normalized = posixpath.normpath(path) + if normalized == "/workspace": + return "." + if normalized.startswith("/workspace/"): + return normalized.removeprefix("/workspace/") + return normalized + + +@dataclass +class _ReadCoverageLedger: + """Source lines delivered to this hunter's model during the whole hunt.""" + + ranges: dict[str, list[tuple[int, int]]] = field(default_factory=dict) + file_lengths: dict[str, int] = field(default_factory=dict) + + def ranges_for(self, path: str) -> list[tuple[int, int]]: + return self.ranges.get(_canonical_read_path(path), []) + + def add(self, path: str, returned: tuple[int, int]) -> bool: + """Record a delivered range and return whether it contains new lines.""" + + start, end = returned + if start < 1 or end < start: + return False + key = _canonical_read_path(path) + covered = self.ranges.get(key, []) + has_new_lines = bool(_uncovered_read_ranges(returned, covered)) + merged: list[tuple[int, int]] = [] + for left, right in sorted([*covered, returned]): + if merged and left <= merged[-1][1] + 1: + merged[-1] = (merged[-1][0], max(merged[-1][1], right)) + else: + merged.append((left, right)) + self.ranges[key] = merged + return has_new_lines + + def note_file_length(self, path: str, raw_output: str) -> None: + match = _READ_FILE_METADATA.search(raw_output) + if match: + self.file_lengths[_canonical_read_path(path)] = int(match.group(1)) + + def redundant_read_note(self, path: str, requested: tuple[int, int]) -> str | None: + """Explain a fully covered request without scanning unrelated files.""" + + key = _canonical_read_path(path) + covered = self.ranges.get(key, []) + if not covered: + return None + start, end = requested + end = min(end, self.file_lengths.get(key, end)) + if start > end or _uncovered_read_ranges((start, end), covered): + return None + relevant = [ + (max(start, left), min(end, right)) + for left, right in covered + if left <= end and right >= start + ] + return ( + "[SOURCE COVERAGE — THIS HUNT]\n" + f"Your read_file request for {key} lines {start}-{end} is already covered " + "by source delivered earlier in this hunt, possibly before context compaction.\n" + f"Previously delivered here: {_format_line_ranges(relevant)}.\n" + "That tool call was not executed. Choose an unread range, finish, or " + "call read_file with refresh=true if you need these lines back in recent context. " + "A refresh adds no new source coverage." + ) + + def _range_coverage_fraction( requested: tuple[int, int], covered_ranges: list[tuple[int, int]] ) -> float: @@ -2663,7 +2780,7 @@ def _range_coverage_fraction( def _uncovered_read_ranges( requested: tuple[int, int], covered_ranges: list[tuple[int, int]] ) -> list[tuple[int, int]]: - """Return requested line intervals not already represented in context.""" + """Return line intervals not previously delivered during this hunt.""" start, end = requested intersections = sorted( (max(start, covered_start), min(end, covered_end)) @@ -2713,7 +2830,7 @@ def _format_line_ranges(ranges: list[tuple[int, int]]) -> str: def _read_file_tool_response( arguments: dict[str, Any], raw_output: str, - visible_ranges: list[tuple[int, int]], + covered_ranges: list[tuple[int, int]], ) -> tuple[str, tuple[int, int] | None]: """Render truthful read metadata from source lines actually sent to the model.""" @@ -2734,8 +2851,8 @@ def _read_file_tool_response( if (match := re.match(r"\s*(\d+)\t", line)) ] returned = (numbered[0], numbered[-1]) if numbered else None - coverage = _range_coverage_fraction(returned, visible_ranges) if returned is not None else 0.0 - uncovered = _uncovered_read_ranges(returned, visible_ranges) if returned is not None else [] + coverage = _range_coverage_fraction(returned, covered_ranges) if returned is not None else 0.0 + uncovered = _uncovered_read_ranges(returned, covered_ranges) if returned is not None else [] truncated = raw_truncated or display_truncated eof = total_lines is not None and ( (returned is not None and returned[1] >= total_lines and not truncated) @@ -2757,6 +2874,7 @@ def _read_file_tool_response( f"EOF: {eof}", f"Truncated: {truncated}", f"Next line: {next_line if next_line is not None else 'None'}", + "Coverage scope: this hunt", f"Overlap: {coverage:.0%}", f"New lines: {_format_line_ranges(uncovered)}", "[/READ_FILE]", diff --git a/tests/test_deep_agent_loop.py b/tests/test_deep_agent_loop.py index 78456160..ac6ac69e 100644 --- a/tests/test_deep_agent_loop.py +++ b/tests/test_deep_agent_loop.py @@ -14,8 +14,14 @@ from clearwing.agent.tools.hunt.deep_agent import build_deep_agent_tools from clearwing.agent.tools.hunt.potentials import build_potential_tools from clearwing.agent.tools.hunt.sandbox import HunterContext +from clearwing.data.memory import ContextSummarizer from clearwing.llm.native import NativeToolSpec -from clearwing.sourcehunt.hunter import NativeHunter, _read_file_tool_response, _tool_output_text +from clearwing.sourcehunt.hunter import ( + NativeHunter, + _read_file_tool_response, + _ReadCoverageLedger, + _tool_output_text, +) @dataclass @@ -159,6 +165,23 @@ def test_read_metadata_preserves_an_oversized_source_line(): assert "Next line: 2" in summary +def test_read_coverage_merges_path_aliases_and_warns_only_for_covered_range(): + ledger = _ReadCoverageLedger() + assert ledger.add("src/./parser.c", (1, 20)) + assert ledger.add("/workspace/src/parser.c", (11, 30)) + assert not ledger.add("src/parser.c", (12, 18)) + assert ledger.ranges_for("/workspace/src/parser.c") == [(1, 30)] + note = ledger.redundant_read_note("/workspace/src/parser.c", (12, 18)) + assert note is not None and "src/parser.c lines 12-18" in note + assert ledger.redundant_read_note("src/parser.c", (25, 35)) is None + short_file = _ReadCoverageLedger() + short_file.add("src/short.c", (1, 2)) + short_file.note_file_length( + "src/short.c", " 1\ta\n 2\tb\n[CLEARWING_READ_METADATA total_lines=2]" + ) + assert short_file.redundant_read_note("src/short.c", (1, 100)) is not None + + def test_structured_search_summary_preserves_status_and_next_action(): result = { "status": "truncated", @@ -681,7 +704,7 @@ def read_handler(offset=0, limit=2000, **kwargs): tool_calls_list=[ _make_tool_call( "read_file", - {"path": "views.py", "offset": 10, "limit": 10}, + {"path": "views.py", "offset": 10, "limit": 10, "refresh": True}, ) ] ), @@ -947,6 +970,157 @@ async def test_deep_read_repeated_range_does_not_reset_stall(): assert ctx.deep_files_read == {"/workspace/only_file.c"} +@pytest.mark.asyncio +async def test_deep_read_coverage_survives_compaction_and_retries_before_tool_call(): + sandbox = _stub_sandbox_for_deep_read() + hunter, llm, _ = _build_deep_hunter(sandbox) + hunter.max_steps_without_progress = 2 + summarizer = ContextSummarizer() + summarizer.should_summarize = MagicMock(side_effect=[False, True, False, False]) + hunter.summarizer = summarizer + llm.aask_text.return_value = FakeResponse(text="Earlier source was inspected.") + llm.achat.side_effect = [ + FakeResponse( + tool_calls_list=[_make_tool_call("read_file", {"path": "src/foo.c", "limit": 2})] + ), + FakeResponse( + tool_calls_list=[ + _make_tool_call("read_file", {"path": "/workspace/src/foo.c", "limit": 2}) + ] + ), + FakeResponse( + tool_calls_list=[ + _make_tool_call("read_file", {"path": "src/./foo.c", "limit": 2, "refresh": True}) + ] + ), + FakeResponse( + tool_calls_list=[_make_tool_call("read_file", {"path": "src/foo.c", "limit": 2})] + ), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + logger = MagicMock() + mock_traj.for_hunter.return_value = logger + result = await hunter.arun() + + assert result.stop_reason == "stalled" + assert result.tokens_used == 4 * FakeUsage().total_tokens + assert llm.aask_text.call_count == 1 + second_messages = llm.achat.call_args_list[1].kwargs["messages"] + assert not any(message.content.startswith("[SOURCE COVERAGE") for message in second_messages) + retry_messages = llm.achat.call_args_list[2].kwargs["messages"] + assert "src/foo.c lines 1-2" in retry_messages[-1].content + assert "possibly before context compaction" in retry_messages[-1].content + assert "refresh=true" in retry_messages[-1].content + assert not any( + message.role == "assistant" + and message.tool_calls + and any( + call.fn_arguments.get("path") == "/workspace/src/foo.c" for call in message.tool_calls + ) + for message in retry_messages + ) + second_read = [ + call.args[1]["tool_summary"] + for call in logger.log.call_args_list + if call.args and call.args[0] == "tool_result" + ][1] + assert "Coverage scope: this hunt" in second_read + assert "Overlap: 100%" in second_read + assert "New lines: None" in second_read + retries = [ + call for call in logger.log.call_args_list if call.args and call.args[0] == "coverage_retry" + ] + assert len(retries) == 1 + assert len([call for call in logger.log.call_args_list if call.args[0] == "tool_result"]) == 3 + assert sandbox.exec.call_count == 3 + + +@pytest.mark.asyncio +async def test_deep_hunter_seeds_coverage_from_initial_source_window(): + hunter, llm, ctx = _build_deep_hunter(_stub_sandbox_for_deep_read()) + ctx.read_ranges["src/target.c"] = [(20, 40)] + llm.achat.side_effect = [ + FakeResponse( + tool_calls_list=[ + _make_tool_call("read_file", {"path": "src/target.c", "offset": 19, "limit": 21}) + ] + ), + FakeResponse(text="done"), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + mock_traj.for_hunter.return_value = MagicMock() + result = await hunter.arun() + + assert result.stop_reason == "completed" + first_messages = llm.achat.call_args_list[0].kwargs["messages"] + assert not any(message.content.startswith("[SOURCE COVERAGE") for message in first_messages) + retry_messages = llm.achat.call_args_list[1].kwargs["messages"] + assert "src/target.c lines 20-40" in retry_messages[-1].content + + +@pytest.mark.asyncio +async def test_coverage_retry_drops_entire_mixed_tool_batch(): + sandbox = _stub_sandbox_for_deep_read() + hunter, llm, _ = _build_deep_hunter(sandbox) + llm.achat.side_effect = [ + FakeResponse( + tool_calls_list=[_make_tool_call("read_file", {"path": "src/foo.c", "limit": 2})] + ), + FakeResponse( + tool_calls_list=[ + _make_tool_call("read_file", {"path": "src/foo.c", "limit": 2}), + _make_tool_call("execute", {"command": "touch /workspace/should-not-run"}), + ] + ), + FakeResponse(text="done"), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + logger = MagicMock() + mock_traj.for_hunter.return_value = logger + result = await hunter.arun() + + assert result.stop_reason == "completed" + assert sandbox.exec.call_count == 1 + retry_messages = llm.achat.call_args_list[2].kwargs["messages"] + assert retry_messages[-1].content.startswith("[SOURCE COVERAGE") + assert all("should-not-run" not in str(message.to_dict()) for message in retry_messages) + assert ( + len([call for call in logger.log.call_args_list if call.args[0] == "coverage_retry"]) == 1 + ) + + +@pytest.mark.asyncio +async def test_compaction_rearms_coverage_warning_for_a_file(): + hunter, llm, _ = _build_deep_hunter(_stub_sandbox_for_deep_read()) + hunter.max_steps_without_progress = 5 + summarizer = ContextSummarizer() + summarizer.should_summarize = MagicMock(side_effect=[False, False, True]) + hunter.summarizer = summarizer + llm.aask_text.return_value = FakeResponse(text="Earlier source was inspected.") + + def read(): + return FakeResponse( + tool_calls_list=[_make_tool_call("read_file", {"path": "src/foo.c", "limit": 2})] + ) + + llm.achat.side_effect = [read(), read(), read(), read(), FakeResponse(text="done")] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + logger = MagicMock() + mock_traj.for_hunter.return_value = logger + result = await hunter.arun() + + assert result.stop_reason == "completed" + assert llm.aask_text.call_count == 1 + retries = [call for call in logger.log.call_args_list if call.args[0] == "coverage_retry"] + assert len(retries) == 2 + after_compaction = llm.achat.call_args_list[4].kwargs["messages"] + assert after_compaction[-1].content.startswith("[SOURCE COVERAGE") + + @pytest.mark.asyncio async def test_deep_execute_failing_does_not_reset_stall(): # A stream of varied FAILING execute commands (distinct arguments, so not From 3174772c7ad736850d31caca4876d48c51151683 Mon Sep 17 00:00:00 2001 From: Jorge Garcia Date: Wed, 30 Sep 2026 20:50:36 -0700 Subject: [PATCH 4/5] fix(sourcehunt): track function reads as source coverage --- clearwing/agent/tools/hunt/deep_agent.py | 38 ++++-- clearwing/sourcehunt/hunter.py | 123 +++++++++++++---- tests/test_deep_agent_loop.py | 161 +++++++++++++++++++++++ 3 files changed, 280 insertions(+), 42 deletions(-) diff --git a/clearwing/agent/tools/hunt/deep_agent.py b/clearwing/agent/tools/hunt/deep_agent.py index 9551ae43..cf96f833 100644 --- a/clearwing/agent/tools/hunt/deep_agent.py +++ b/clearwing/agent/tools/hunt/deep_agent.py @@ -99,6 +99,22 @@ class ListFunctionsInput(ToolInputModel): class ReadFunctionInput(ToolInputModel): name: str = Field(description="Exact function name to read (e.g. 'foo_bar_baz').") + refresh: bool = Field( + default=False, + description="Set true to deliberately reread a function already shown during this hunt.", + ) + + +def function_locations(callgraph: object, name: str) -> list[tuple[str, int, int]]: + """Return distinct exact-name definitions in the callgraph.""" + return list( + dict.fromkeys( + (path, info.start_line, info.end_line) + for path, infos in callgraph.function_info.items() + for info in infos + if info.name == name + ) + ) class FindSourceInput(ToolInputModel): @@ -417,26 +433,21 @@ def read_function(name: str, **_: object) -> dict: cg = ctx.callgraph if cg is None: return {"error": "callgraph not available"} - hits = [(f, fi) for f, infos in cg.function_info.items() for fi in infos if fi.name == name] - if not hits: + locations = function_locations(cg, name) + if not locations: all_names = {fi.name for infos in cg.function_info.values() for fi in infos} near = difflib.get_close_matches(name, all_names, n=5, cutoff=0.6) return {"error": f"no function named '{name}'", "did_you_mean": near} - # De-dup identical (file, start, end) — callgraph sometimes double-lists. - uniq = list({(f, fi.start_line, fi.end_line): (f, fi) for f, fi in hits}.values()) - if len(uniq) > 1: + if len(locations) > 1: return { "error": "ambiguous name; multiple definitions", "candidates": [ - {"file": f, "start_line": fi.start_line, "end_line": fi.end_line} - for f, fi in uniq + {"file": f, "start_line": start, "end_line": end} for f, start, end in locations ], } - f, fi = uniq[0] - body = read_file( - f"/workspace/{f}", offset=fi.start_line - 1, limit=fi.end_line - fi.start_line + 1 - ) - return {"file": f, "start_line": fi.start_line, "end_line": fi.end_line, "body": body} + f, start, end = locations[0] + body = read_file(f"/workspace/{f}", offset=start - 1, limit=end - start + 1) + return {"file": f, "start_line": start, "end_line": end, "body": body} reporting_tools = build_reporting_tools(ctx) @@ -475,7 +486,8 @@ def read_function(name: str, **_: object) -> dict: description=( "Read a function body by exact name. Returns {file, start_line, " "end_line, body}. On miss: did_you_mean suggestions. On ambiguity: " - "candidate list." + "candidate list. Uses the same source coverage and 12,000-character " + "visible limit as read_file; use refresh=true for a deliberate reread." ), schema=ReadFunctionInput.model_json_schema(), handler=read_function, diff --git a/clearwing/sourcehunt/hunter.py b/clearwing/sourcehunt/hunter.py index 8ce9dfc8..b5aa71bf 100644 --- a/clearwing/sourcehunt/hunter.py +++ b/clearwing/sourcehunt/hunter.py @@ -25,7 +25,7 @@ build_hunter_tools, build_propagation_auditor_tools, ) -from clearwing.agent.tools.hunt.deep_agent import READ_FILE_DEFAULT_LINES +from clearwing.agent.tools.hunt.deep_agent import READ_FILE_DEFAULT_LINES, function_locations from clearwing.core.events import EventBus, EventType from clearwing.data.memory import ContextSummarizer from clearwing.llm import ( @@ -1630,16 +1630,31 @@ async def _request_with_coverage_retry( warning: tuple[str, str] | None = None if self.agent_mode == "deep" and not coverage_retry_used: for proposed_call in response.tool_calls: - if proposed_call.fn_name != "read_file": + if proposed_call.fn_name not in {"read_file", "read_function"}: continue arguments = proposed_call.fn_arguments if not isinstance(arguments, dict) or arguments.get("refresh") is True: continue - path = str(arguments.get("path") or "") + if proposed_call.fn_name == "read_function": + callgraph = self.ctx.callgraph + locations = ( + function_locations(callgraph, str(arguments.get("name") or "")) + if callgraph is not None + else [] + ) + if len(locations) != 1: + continue + path, start, end = locations[0] + requested = (start, end) + else: + path = str(arguments.get("path") or "") + requested = _requested_read_range(arguments) key = _canonical_read_path(path) if key in warned_read_paths: continue - note = read_coverage.redundant_read_note(path, _requested_read_range(arguments)) + note = read_coverage.redundant_read_note( + path, requested, tool_name=proposed_call.fn_name + ) if note: warning = (key, note) break @@ -2117,7 +2132,7 @@ async def arun(self) -> HunterRunResult: # two tools' arguments literal — only tools without an # inherent legitimate-pagination shape benefit from # normalizing away incrementing digits. - if tool_call.fn_name in ("read_file", "read_source_file"): + if tool_call.fn_name in ("read_file", "read_function", "read_source_file"): key = (tool_call.fn_name, tool_call.fn_arguments_json[:300]) else: normalized_args = re.sub(r"\d+", "#", tool_call.fn_arguments_json) @@ -2232,13 +2247,32 @@ async def arun(self) -> HunterRunResult: if locations - seen_search_locations: search_progress_epoch += 1 seen_search_locations.update(locations) - if tool_call.fn_name == "read_file" and isinstance(tool_output, str): - reread_path = str(tool_arguments.get("path") or "") - read_coverage.note_file_length(reread_path, tool_output) + read_arguments = tool_arguments + raw_read_output = tool_output if isinstance(tool_output, str) else None + function_name = None + if ( + tool_call.fn_name == "read_function" + and isinstance(tool_output, dict) + and isinstance(tool_output.get("body"), str) + ): + function_name = str(tool_arguments.get("name") or "") + read_arguments = { + "path": tool_output["file"], + "start_line": tool_output["start_line"], + "end_line": tool_output["end_line"], + } + raw_read_output = tool_output["body"] + if ( + tool_call.fn_name in {"read_file", "read_function"} + and raw_read_output is not None + ): + reread_path = str(read_arguments.get("path") or "") + read_coverage.note_file_length(reread_path, raw_read_output) tool_summary, returned_range = _read_file_tool_response( - tool_arguments, - tool_output, + read_arguments, + raw_read_output, read_coverage.ranges_for(reread_path), + function_name=function_name, ) if returned_range is not None and read_coverage.add( reread_path, returned_range @@ -2727,7 +2761,9 @@ def note_file_length(self, path: str, raw_output: str) -> None: if match: self.file_lengths[_canonical_read_path(path)] = int(match.group(1)) - def redundant_read_note(self, path: str, requested: tuple[int, int]) -> str | None: + def redundant_read_note( + self, path: str, requested: tuple[int, int], *, tool_name: str = "read_file" + ) -> str | None: """Explain a fully covered request without scanning unrelated files.""" key = _canonical_read_path(path) @@ -2745,11 +2781,11 @@ def redundant_read_note(self, path: str, requested: tuple[int, int]) -> str | No ] return ( "[SOURCE COVERAGE — THIS HUNT]\n" - f"Your read_file request for {key} lines {start}-{end} is already covered " + f"Your {tool_name} request for {key} lines {start}-{end} is already covered " "by source delivered earlier in this hunt, possibly before context compaction.\n" f"Previously delivered here: {_format_line_ranges(relevant)}.\n" "That tool call was not executed. Choose an unread range, finish, or " - "call read_file with refresh=true if you need these lines back in recent context. " + f"call {tool_name} with refresh=true if you need these lines back in recent context. " "A refresh adds no new source coverage." ) @@ -2831,6 +2867,8 @@ def _read_file_tool_response( arguments: dict[str, Any], raw_output: str, covered_ranges: list[tuple[int, int]], + *, + function_name: str | None = None, ) -> tuple[str, tuple[int, int] | None]: """Render truthful read metadata from source lines actually sent to the model.""" @@ -2858,28 +2896,55 @@ def _read_file_tool_response( (returned is not None and returned[1] >= total_lines and not truncated) or (returned is None and requested[0] > total_lines) ) - if eof: + function_complete = ( + function_name is not None + and returned is not None + and returned[1] >= requested[1] + and not truncated + ) + if eof or function_complete: next_line = None elif returned is not None: next_line = returned[1] + 1 else: next_line = requested[0] - header = "\n".join( - [ - "[READ_FILE]", - f"Requested: {requested[0]}-{requested[1]}", - f"Returned: {_format_line_ranges([returned] if returned else [])}", - f"Total lines: {total_lines if total_lines is not None else 'Unknown'}", - f"EOF: {eof}", - f"Truncated: {truncated}", - f"Next line: {next_line if next_line is not None else 'None'}", - "Coverage scope: this hunt", - f"Overlap: {coverage:.0%}", - f"New lines: {_format_line_ranges(uncovered)}", - "[/READ_FILE]", - ] - ) + header_lines = [ + "[READ_FILE]", + *( + [ + f"Function: {function_name}", + f"File: {arguments['path']}", + f"Function end: {requested[1]}", + f"Function complete: {function_complete}", + ] + if function_name is not None + else [] + ), + f"Requested: {requested[0]}-{requested[1]}", + f"Returned: {_format_line_ranges([returned] if returned else [])}", + f"Total lines: {total_lines if total_lines is not None else 'Unknown'}", + f"EOF: {eof}", + f"Truncated: {truncated}", + f"Next line: {next_line if next_line is not None else 'None'}", + *( + [ + f"Continue with read_file(path={arguments['path']!r}, " + f"start_line={next_line}, end_line={requested[1]})." + ] + if function_name is not None + and not function_complete + and not eof + and next_line is not None + and next_line <= requested[1] + else [] + ), + "Coverage scope: this hunt", + f"Overlap: {coverage:.0%}", + f"New lines: {_format_line_ranges(uncovered)}", + "[/READ_FILE]", + ] + header = "\n".join(header_lines) return f"{header}\n{rendered_content}", returned diff --git a/tests/test_deep_agent_loop.py b/tests/test_deep_agent_loop.py index ac6ac69e..9beca0f1 100644 --- a/tests/test_deep_agent_loop.py +++ b/tests/test_deep_agent_loop.py @@ -16,6 +16,7 @@ from clearwing.agent.tools.hunt.sandbox import HunterContext from clearwing.data.memory import ContextSummarizer from clearwing.llm.native import NativeToolSpec +from clearwing.sourcehunt.callgraph import CallGraph, FunctionInfo from clearwing.sourcehunt.hunter import ( NativeHunter, _read_file_tool_response, @@ -165,6 +166,39 @@ def test_read_metadata_preserves_an_oversized_source_line(): assert "Next line: 2" in summary +def test_function_read_uses_file_limit_and_function_boundary(): + source = "".join(f"{line:6d}\t{'x' * 220}\n" for line in range(10, 110)) + arguments = {"path": "src/parser.c", "start_line": 10, "end_line": 109} + summary, returned = _read_file_tool_response( + arguments, + f"{source}[CLEARWING_READ_METADATA total_lines=200]", + [], + function_name="parse_input", + ) + + assert returned is not None and 10 <= returned[1] < 109 + assert "Function: parse_input" in summary + assert "File: src/parser.c" in summary + assert "Function complete: False" in summary + assert "Truncated: True" in summary + assert f"Next line: {returned[1] + 1}" in summary + assert ( + f"Continue with read_file(path='src/parser.c', start_line={returned[1] + 1}, end_line=109)." + ) in summary + assert len(summary.split("[/READ_FILE]\n", 1)[1]) <= 12_000 + + final, final_range = _read_file_tool_response( + arguments, + " 109\tlast line\n[CLEARWING_READ_METADATA total_lines=200]", + [(10, 108)], + function_name="parse_input", + ) + assert final_range == (109, 109) + assert "Function complete: True" in final + assert "EOF: False" in final + assert "Next line: None" in final + + def test_read_coverage_merges_path_aliases_and_warns_only_for_covered_range(): ledger = _ReadCoverageLedger() assert ledger.add("src/./parser.c", (1, 20)) @@ -889,6 +923,133 @@ def _build_deep_hunter(sandbox, max_steps=20): return hunter, llm, ctx +def _add_function(ctx, name, start, end, path="src/foo.c"): + if ctx.callgraph is None: + ctx.callgraph = CallGraph() + ctx.callgraph.function_info[path].append(FunctionInfo(name, start, end)) + + +@pytest.mark.asyncio +async def test_function_reads_share_file_coverage_and_reset_stall(): + sandbox = _stub_sandbox_for_deep_read_ranges() + hunter, llm, ctx = _build_deep_hunter(sandbox) + _add_function(ctx, "first", 1, 2) + _add_function(ctx, "second", 3, 4) + _add_function(ctx, "third", 5, 6) + hunter.tools = build_deep_agent_tools(ctx) + hunter.max_steps_without_progress = 2 + llm.achat.side_effect = [ + FakeResponse(tool_calls_list=[_make_tool_call("read_function", {"name": "first"})]), + FakeResponse(tool_calls_list=[_make_tool_call("read_function", {"name": "second"})]), + FakeResponse(tool_calls_list=[_make_tool_call("read_function", {"name": "third"})]), + FakeResponse(text="done"), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + logger = MagicMock() + mock_traj.for_hunter.return_value = logger + result = await hunter.arun() + + assert result.stop_reason == "completed" + assert sandbox.exec.call_count == 3 + summaries = [ + call.args[1]["tool_summary"] + for call in logger.log.call_args_list + if call.args[0] == "tool_result" + ] + assert all("New lines:" in summary for summary in summaries) + assert "New lines: 5-6" in summaries[2] + + +@pytest.mark.asyncio +async def test_long_function_is_delivered_with_file_read_limit(): + sandbox = _stub_sandbox_for_deep_read() + sandbox.exec.return_value.stdout = "".join( + f"{line:6d}\t{'x' * 220}\n" for line in range(1, 101) + ) + sandbox.exec.return_value.stderr = "__CLEARWING_TOTAL_LINES__=200\n" + hunter, llm, ctx = _build_deep_hunter(sandbox) + _add_function(ctx, "long_function", 1, 100) + hunter.tools = build_deep_agent_tools(ctx) + llm.achat.side_effect = [ + FakeResponse(tool_calls_list=[_make_tool_call("read_function", {"name": "long_function"})]), + FakeResponse(text="done"), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + logger = MagicMock() + mock_traj.for_hunter.return_value = logger + result = await hunter.arun() + + assert result.stop_reason == "completed" + summary = next( + call.args[1]["tool_summary"] + for call in logger.log.call_args_list + if call.args[0] == "tool_result" + ) + assert "Function: long_function" in summary + assert "Truncated: True" in summary + assert len(summary.split("[/READ_FILE]\n", 1)[1]) <= 12_000 + assert "Continue with read_file" in summary + assert "New lines: 1-52" in summary + + +@pytest.mark.asyncio +async def test_covered_function_retries_and_refresh_shares_file_ledger(): + sandbox = _stub_sandbox_for_deep_read() + hunter, llm, ctx = _build_deep_hunter(sandbox) + _add_function(ctx, "foo", 1, 2) + hunter.tools = build_deep_agent_tools(ctx) + llm.achat.side_effect = [ + FakeResponse( + tool_calls_list=[ + _make_tool_call("read_file", {"path": "/workspace/src/./foo.c", "limit": 2}) + ] + ), + FakeResponse(tool_calls_list=[_make_tool_call("read_function", {"name": "foo"})]), + FakeResponse( + tool_calls_list=[_make_tool_call("read_function", {"name": "foo", "refresh": True})] + ), + FakeResponse(text="done"), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + logger = MagicMock() + mock_traj.for_hunter.return_value = logger + result = await hunter.arun() + + assert result.stop_reason == "completed" + assert sandbox.exec.call_count == 2 + assert "src/foo.c lines 1-2" in llm.achat.call_args_list[2].kwargs["messages"][-1].content + summaries = [ + call.args[1]["tool_summary"] + for call in logger.log.call_args_list + if call.args[0] == "tool_result" + ] + assert "New lines: None" in summaries[1] + + +@pytest.mark.asyncio +async def test_missing_and_ambiguous_functions_do_not_add_coverage(): + sandbox = _stub_sandbox_for_deep_read() + hunter, llm, ctx = _build_deep_hunter(sandbox) + _add_function(ctx, "duplicate", 1, 2) + _add_function(ctx, "duplicate", 3, 4, path="src/other.c") + hunter.tools = build_deep_agent_tools(ctx) + hunter.max_steps_without_progress = 2 + llm.achat.side_effect = [ + FakeResponse(tool_calls_list=[_make_tool_call("read_function", {"name": "missing"})]), + FakeResponse(tool_calls_list=[_make_tool_call("read_function", {"name": "duplicate"})]), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + mock_traj.for_hunter.return_value = MagicMock() + result = await hunter.arun() + + assert result.stop_reason == "stalled" + sandbox.exec.assert_not_called() + + @pytest.mark.asyncio async def test_deep_read_file_counts_as_progress(): # Deep-mode read_file doesn't populate ctx.files_read (that set is fed by From 03a85f74a16c30d92afc1427daed447d76001b8e Mon Sep 17 00:00:00 2001 From: Jorge Garcia Date: Wed, 30 Sep 2026 22:08:58 -0700 Subject: [PATCH 5/5] fix(sourcehunt): enforce covered read guard on every request --- clearwing/sourcehunt/hunter.py | 95 +++++++++++++++++++--------------- tests/test_deep_agent_loop.py | 95 ++++++++++++++++++++++++++++------ 2 files changed, 133 insertions(+), 57 deletions(-) diff --git a/clearwing/sourcehunt/hunter.py b/clearwing/sourcehunt/hunter.py index b5aa71bf..960adb66 100644 --- a/clearwing/sourcehunt/hunter.py +++ b/clearwing/sourcehunt/hunter.py @@ -1589,7 +1589,6 @@ async def _request_with_coverage_retry( messages: list[ChatMessage], active_tools: list[NativeToolSpec], read_coverage: _ReadCoverageLedger, - warned_read_paths: set[str], trajectory: HunterTrajectoryLogger, step: int, ) -> tuple[Any, int, int, float]: @@ -1630,33 +1629,10 @@ async def _request_with_coverage_retry( warning: tuple[str, str] | None = None if self.agent_mode == "deep" and not coverage_retry_used: for proposed_call in response.tool_calls: - if proposed_call.fn_name not in {"read_file", "read_function"}: - continue - arguments = proposed_call.fn_arguments - if not isinstance(arguments, dict) or arguments.get("refresh") is True: - continue - if proposed_call.fn_name == "read_function": - callgraph = self.ctx.callgraph - locations = ( - function_locations(callgraph, str(arguments.get("name") or "")) - if callgraph is not None - else [] - ) - if len(locations) != 1: - continue - path, start, end = locations[0] - requested = (start, end) - else: - path = str(arguments.get("path") or "") - requested = _requested_read_range(arguments) - key = _canonical_read_path(path) - if key in warned_read_paths: - continue - note = read_coverage.redundant_read_note( - path, requested, tool_name=proposed_call.fn_name + warning = _covered_read_warning( + proposed_call, self.ctx, read_coverage ) - if note: - warning = (key, note) + if warning is not None: break if warning is None: return response, total_input, total_output, total_cost @@ -1664,7 +1640,6 @@ async def _request_with_coverage_retry( # Nothing from the rejected assistant response has entered messages, # so the retry leaves no unmatched tool call or result behind. key, note = warning - warned_read_paths.add(key) coverage_retry_used = True trajectory.log( "coverage_retry", @@ -1708,7 +1683,6 @@ async def arun(self) -> HunterRunResult: for path, ranges in self.ctx.read_ranges.items(): for returned_range in ranges: read_coverage.add(path, returned_range) - warned_read_paths: set[str] = set() flags_raised: int = 0 empty_response_nudges: int = 0 potential_reminder_active = False @@ -1923,10 +1897,6 @@ async def arun(self) -> HunterRunResult: if self.summarizer and self.summarizer.should_summarize(messages): pre = len(messages) messages = await self.summarizer.summarize(messages, self.llm) - # The targeted warning may have been summarized away. Give - # the model one fresh reminder before penalizing a later - # covered read whose warning is no longer in context. - warned_read_paths.clear() logger.info("Hunter context summarized: %d → %d messages", pre, len(messages)) active_tools = [] if final_synthesis_turn else self.tools @@ -1954,7 +1924,6 @@ async def arun(self) -> HunterRunResult: messages, active_tools, read_coverage, - warned_read_paths, trajectory, step, ) @@ -2150,6 +2119,11 @@ async def arun(self) -> HunterRunResult: repeated_tool_calls[key] = repeated_tool_calls.get(key, 0) + 1 skipped = repeated_tool_calls[key] > 3 + covered_read_warning = ( + _covered_read_warning(tool_call, self.ctx, read_coverage) + if self.agent_mode == "deep" + else None + ) if final_decision_blocked: tool_output = { @@ -2192,6 +2166,19 @@ async def arun(self) -> HunterRunResult: "dynamic_verification_blocked": True, }, ) + elif covered_read_warning is not None: + tool_output = {"error": covered_read_warning[1]} + tool_summary = covered_read_warning[1] + trajectory.log( + "tool_result", + { + "step": step, + "tool_call": _serialize_tool_call(tool_call), + "tool_output": tool_output, + "tool_summary": tool_summary, + "coverage_blocked": True, + }, + ) elif skipped: total_repeated_skips += 1 prev_step_had_skip = tool_call.fn_name not in {"grep_source", "find_source"} @@ -2774,22 +2761,48 @@ def redundant_read_note( end = min(end, self.file_lengths.get(key, end)) if start > end or _uncovered_read_ranges((start, end), covered): return None - relevant = [ - (max(start, left), min(end, right)) - for left, right in covered - if left <= end and right >= start - ] + covered_lines = "\n".join(f"- lines {left}-{right}" for left, right in covered) + total_lines = self.file_lengths.get(key) + total_label = total_lines if total_lines is not None else "an unknown number of" return ( "[SOURCE COVERAGE — THIS HUNT]\n" f"Your {tool_name} request for {key} lines {start}-{end} is already covered " "by source delivered earlier in this hunt, possibly before context compaction.\n" - f"Previously delivered here: {_format_line_ranges(relevant)}.\n" - "That tool call was not executed. Choose an unread range, finish, or " + f"{key} is covered in this hunt:\n{covered_lines}\n" + f"out of {total_label} lines.\n" + "That tool call was not executed. Choose unread source, finish, or " f"call {tool_name} with refresh=true if you need these lines back in recent context. " "A refresh adds no new source coverage." ) +def _covered_read_warning( + tool_call: ToolCall, ctx: HunterContext, coverage: _ReadCoverageLedger +) -> tuple[str, str] | None: + """Return the coverage warning for an unprompted fully covered read.""" + + if tool_call.fn_name not in {"read_file", "read_function"}: + return None + arguments = tool_call.fn_arguments + if not isinstance(arguments, dict) or arguments.get("refresh") is True: + return None + if tool_call.fn_name == "read_function": + locations = ( + function_locations(ctx.callgraph, str(arguments.get("name") or "")) + if ctx.callgraph is not None + else [] + ) + if len(locations) != 1: + return None + path, start, end = locations[0] + requested = (start, end) + else: + path = str(arguments.get("path") or "") + requested = _requested_read_range(arguments) + note = coverage.redundant_read_note(path, requested, tool_name=tool_call.fn_name) + return (_canonical_read_path(path), note) if note else None + + def _range_coverage_fraction( requested: tuple[int, int], covered_ranges: list[tuple[int, int]] ) -> float: diff --git a/tests/test_deep_agent_loop.py b/tests/test_deep_agent_loop.py index 9beca0f1..17dd44e9 100644 --- a/tests/test_deep_agent_loop.py +++ b/tests/test_deep_agent_loop.py @@ -216,6 +216,22 @@ def test_read_coverage_merges_path_aliases_and_warns_only_for_covered_range(): assert short_file.redundant_read_note("src/short.c", (1, 100)) is not None +def test_coverage_warning_lists_all_merged_ranges_and_file_length(): + ledger = _ReadCoverageLedger() + ledger.add("src/jv.c", (1, 20)) + ledger.add("src/jv.c", (30, 50)) + ledger.note_file_length("src/jv.c", "[CLEARWING_READ_METADATA total_lines=50]") + + fragmented = ledger.redundant_read_note("src/jv.c", (5, 10)) + assert fragmented is not None + assert "src/jv.c is covered in this hunt:\n- lines 1-20\n- lines 30-50\nout of 50 lines." in fragmented + + ledger.add("src/jv.c", (21, 29)) + complete = ledger.redundant_read_note("src/jv.c", (5, 10)) + assert complete is not None + assert "src/jv.c is covered in this hunt:\n- lines 1-50\nout of 50 lines." in complete + + def test_structured_search_summary_preserves_status_and_next_action(): result = { "status": "truncated", @@ -1157,6 +1173,7 @@ async def test_deep_read_coverage_survives_compaction_and_retries_before_tool_ca FakeResponse( tool_calls_list=[_make_tool_call("read_file", {"path": "src/foo.c", "limit": 2})] ), + FakeResponse(text="done"), ] with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: @@ -1164,8 +1181,8 @@ async def test_deep_read_coverage_survives_compaction_and_retries_before_tool_ca mock_traj.for_hunter.return_value = logger result = await hunter.arun() - assert result.stop_reason == "stalled" - assert result.tokens_used == 4 * FakeUsage().total_tokens + assert result.stop_reason == "completed" + assert result.tokens_used == 5 * FakeUsage().total_tokens assert llm.aask_text.call_count == 1 second_messages = llm.achat.call_args_list[1].kwargs["messages"] assert not any(message.content.startswith("[SOURCE COVERAGE") for message in second_messages) @@ -1192,9 +1209,9 @@ async def test_deep_read_coverage_survives_compaction_and_retries_before_tool_ca retries = [ call for call in logger.log.call_args_list if call.args and call.args[0] == "coverage_retry" ] - assert len(retries) == 1 - assert len([call for call in logger.log.call_args_list if call.args[0] == "tool_result"]) == 3 - assert sandbox.exec.call_count == 3 + assert len(retries) == 2 + assert len([call for call in logger.log.call_args_list if call.args[0] == "tool_result"]) == 2 + assert sandbox.exec.call_count == 2 @pytest.mark.asyncio @@ -1254,20 +1271,58 @@ async def test_coverage_retry_drops_entire_mixed_tool_batch(): @pytest.mark.asyncio -async def test_compaction_rearms_coverage_warning_for_a_file(): +async def test_coverage_retry_blocks_a_different_covered_range_in_the_same_file(): + sandbox = _stub_sandbox_for_deep_read() + hunter, llm, ctx = _build_deep_hunter(sandbox) + ctx.read_ranges["src/foo.c"] = [(1, 2), (4, 5)] + llm.achat.side_effect = [ + FakeResponse( + tool_calls_list=[ + _make_tool_call("read_file", {"path": "src/foo.c", "start_line": 1, "end_line": 2}) + ] + ), + FakeResponse( + tool_calls_list=[ + _make_tool_call("read_file", {"path": "src/foo.c", "start_line": 4, "end_line": 5}) + ] + ), + FakeResponse(text="done"), + ] + + with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: + logger = MagicMock() + mock_traj.for_hunter.return_value = logger + result = await hunter.arun() + + assert result.stop_reason == "completed" + assert sandbox.exec.call_count == 0 + retries = [call for call in logger.log.call_args_list if call.args[0] == "coverage_retry"] + assert len(retries) == 1 + blocked = [ + call for call in logger.log.call_args_list + if call.args[0] == "tool_result" and call.args[1].get("coverage_blocked") + ] + assert len(blocked) == 1 + assert "src/foo.c lines 4-5" in blocked[0].args[1]["tool_summary"] + + +@pytest.mark.asyncio +async def test_covered_reads_are_blocked_after_retry_and_rechecked_next_turn(): hunter, llm, _ = _build_deep_hunter(_stub_sandbox_for_deep_read()) hunter.max_steps_without_progress = 5 - summarizer = ContextSummarizer() - summarizer.should_summarize = MagicMock(side_effect=[False, False, True]) - hunter.summarizer = summarizer - llm.aask_text.return_value = FakeResponse(text="Earlier source was inspected.") - def read(): + def read(refresh=False): return FakeResponse( - tool_calls_list=[_make_tool_call("read_file", {"path": "src/foo.c", "limit": 2})] + tool_calls_list=[ + _make_tool_call( + "read_file", {"path": "src/foo.c", "limit": 2, "refresh": refresh} + ) + ] ) - llm.achat.side_effect = [read(), read(), read(), read(), FakeResponse(text="done")] + llm.achat.side_effect = [ + read(), read(), read(), read(), read(refresh=True), FakeResponse(text="done") + ] with patch("clearwing.sourcehunt.hunter.HunterTrajectoryLogger") as mock_traj: logger = MagicMock() @@ -1275,11 +1330,19 @@ def read(): result = await hunter.arun() assert result.stop_reason == "completed" - assert llm.aask_text.call_count == 1 + assert llm.aask_text.call_count == 0 retries = [call for call in logger.log.call_args_list if call.args[0] == "coverage_retry"] assert len(retries) == 2 - after_compaction = llm.achat.call_args_list[4].kwargs["messages"] - assert after_compaction[-1].content.startswith("[SOURCE COVERAGE") + blocked = [ + call for call in logger.log.call_args_list + if call.args[0] == "tool_result" and call.args[1].get("coverage_blocked") + ] + assert len(blocked) == 1 + assert "refresh=true" in blocked[0].args[1]["tool_summary"] + assert llm.achat.call_args_list[4].kwargs["messages"][-1].content.startswith( + "[SOURCE COVERAGE" + ) + assert hunter.ctx.sandbox.exec.call_count == 2 @pytest.mark.asyncio