diff --git a/docs/files.md b/docs/files.md index 8f85aa4..930791e 100644 --- a/docs/files.md +++ b/docs/files.md @@ -15,7 +15,7 @@ files.write("notes/plan.md", "...") ## read -`read(path, start_line=1, end_line=None, limit=16000)` returns numbered lines. The numbers are the ones `edit(line_hint=)` accepts. A window that reaches the limit ends with the line to resume from, so a large file is read in explicit steps rather than truncated silently. +`read(path, start_line=1, end_line=None, *, limit=None, max_chars=16000)` returns numbered lines. `limit` is a number of lines; `max_chars` is a separate character budget that keeps a huge window out of the context. A window stopped by either ends with which one stopped it and the line to resume from, so a large file is read in explicit steps rather than truncated silently. Lines are never shortened, and the numbers are the ones `edit(line_hint=)` accepts. ## edit @@ -29,4 +29,8 @@ files.write("notes/plan.md", "...") ## search -`await find(pattern, path=".", glob=..., max_results=50, literal=False, case_sensitive=None, hidden=False)` searches contents and returns rows that print as `path:line: text`. `await paths(pattern=None, path=".", glob=...)` searches file names. Both bound their results and say when the list was cut. +`await find(pattern, path=".", glob=..., context=0, max_results=50, literal=False, case_sensitive=None, hidden=False)` searches contents and returns rows that print as `path:line: text`. `path` may be a list of paths. `context=N` adds up to N lines around each match, printed grep-style as `path-line- text` and marked `context=True`; `max_results` counts matches, not context lines. Slicing or indexing before the await applies to the rows, so `await find(...)[:10]` reads as intended. `await paths(pattern=None, path=".", glob=...)` searches file names. Both bound their results and say when the list was cut. + +## await + +`read`, `ls`, `edit`, and `write` return their result immediately, and that result may also be awaited: `files.read(path)` and `await files.read(path)` are the same call. `find` and `paths` run a background search, so they must be awaited; using one without `await` prints, or raises on iteration, a message saying so rather than a coroutine. diff --git a/priv/python/albedo_plugins/files.py b/priv/python/albedo_plugins/files.py index 84af6e8..6e04d11 100644 --- a/priv/python/albedo_plugins/files.py +++ b/priv/python/albedo_plugins/files.py @@ -15,6 +15,7 @@ import stat import tempfile from dataclasses import dataclass from pathlib import Path +from collections.abc import Awaitable, Callable, Generator from typing import Iterable, Sequence from albedo_api import PythonApi @@ -31,24 +32,49 @@ PREVIEW_LINES = 2 @dataclass(frozen=True) class Match: + """One matching line, or with `context=` a surrounding line that prints + grep-style as `path-line- text`.""" path: str line: int text: str + context: bool = False def __str__(self) -> str: - return f"{self.path}:{self.line}: {self.text}" + mark = "-" if self.context else ":" + return f"{self.path}{mark}{self.line}{mark} {self.text}" - def to_dict(self) -> dict[str, str | int]: - return {"path": self.path, "line": self.line, "text": self.text} + def to_dict(self) -> dict[str, str | int | bool]: + fields: dict[str, str | int | bool] = {"path": self.path, "line": self.line, "text": self.text} + if self.context: + fields["context"] = True + return fields + + +def _ready(value): + """`await` on a value that is already here: returns it without suspending.""" + yield from () + return value + + +class Text(str): + """A result that is ready now. Awaiting it is harmless, so `files.read(...)` + and `await files.read(...)` both work.""" + + def __await__(self) -> Generator[object, None, "Text"]: + return _ready(self) class Rows(list): - """A list that prints one row per line, so a REPL result reads like output.""" + """A list that prints one row per line, so a REPL result reads like output. + Like Text, it may be awaited or used directly.""" def __init__(self, items: Iterable[object] = (), truncated: bool = False) -> None: super().__init__(items) self.truncated = truncated + def __await__(self) -> Generator[object, None, "Rows"]: + return _ready(self) + def __repr__(self) -> str: if not self: return "[]" @@ -58,6 +84,42 @@ class Rows(list): __str__ = __repr__ +class Search: + """A search that runs a supervised job, so its rows exist only once awaited. + Using it without `await` explains that instead of printing a coroutine.""" + + def __init__(self, name: str, run: Callable[[], Awaitable[Rows]]) -> None: + self._name = name + self._run = run + + def __await__(self) -> Generator[object, None, Rows]: + return self._run().__await__() + + def __getitem__(self, index: object) -> "Search": + """`await files.find(...)[:10]` slices before it awaits; apply the + slice to the rows instead of failing on precedence.""" + async def run() -> object: + rows = await self._run() + picked = rows[index] # type: ignore[index] + return Rows(picked, truncated=rows.truncated) if isinstance(index, slice) else picked + return Search(self._name, run) # type: ignore[arg-type] + + def _unawaited(self) -> TypeError: + return TypeError(f"files.{self._name}(...) searches in the background; " + f"use `await files.{self._name}(...)` to get its rows") + + def __repr__(self) -> str: + return f"" + + __str__ = __repr__ + + def __iter__(self): + raise self._unawaited() + + def __len__(self) -> int: + raise self._unawaited() + + async def _run(command: str) -> tuple[int | None, str]: """One supervised shell job, awaited to completion, with its bounded output.""" job = jobs.bash(command, timeout=SEARCH_TIMEOUT) @@ -73,40 +135,48 @@ def _numbered(number: int, lines: Sequence[str]) -> str: class Files: """Workspace file access with the diagnostics an exact edit needs.""" - def read(self, path: str, *, start_line: int = 1, end_line: int | None = None, - limit: int = READ_LIMIT) -> str: - """Complete numbered lines within `limit`; retry an oversized line with a larger limit. + def read(self, path: str, start_line: int = 1, end_line: int | None = None, *, + limit: int | None = None, max_chars: int = READ_LIMIT) -> Text: + """Numbered lines from start_line through end_line, at most `limit` lines. - The numbers are what `edit(line_hint=)` takes. Edit diagnostics still use - short previews, but a read never silently shortens source lines. + `limit` counts lines. `max_chars` is a separate character budget (at + most 200000) that keeps a huge window from flooding the context; when it + stops a read early, the result says so and names the line to resume + from. Lines are never shortened. The numbers are what `edit(line_hint=)` + takes. """ - if start_line < 1 or (end_line is not None and end_line < start_line) or not 0 < limit <= 200_000: - raise ValueError("start_line >= 1, end_line >= start_line, 0 < limit <= 200000") + if start_line < 1 or (end_line is not None and end_line < start_line) \ + or (limit is not None and limit < 1) or not 0 < max_chars <= 200_000: + raise ValueError("start_line >= 1, end_line >= start_line, limit >= 1 line, " + "0 < max_chars <= 200000") target = Path(path).expanduser() - data = target.read_bytes() - lines = data.decode("utf-8", errors="replace").splitlines() - last = len(lines) if end_line is None else min(end_line, len(lines)) + lines = target.read_bytes().decode("utf-8", errors="replace").splitlines() + if start_line > len(lines): + return Text(f"[{path} has {len(lines)} lines; nothing at line {start_line}]") + requested = len(lines) if end_line is None else min(end_line, len(lines)) + last = requested if limit is None else min(requested, start_line + limit - 1) body, used = [], 0 for number in range(start_line, last + 1): row = f"{number:>6} | {lines[number - 1]}" - if used + len(row) + 1 > limit: + if used + len(row) + 1 > max_chars: if not body: retry = ( - f"read again with start_line={number}, end_line={number}, limit={len(row) + 1}" - if len(row) + 1 <= 200_000 else "exceeds the 200000-character read limit" - ) - body.append( - f"[line {number} has {len(row)} characters including its line number; " - f"exceeds limit={limit}; {retry}]" + f"read it alone with start_line={number}, end_line={number}, max_chars={len(row) + 1}" + if len(row) + 1 <= 200_000 else "it exceeds the 200000-character maximum" ) + body.append(f"[line {number} is {len(row)} characters with its number, over " + f"max_chars={max_chars}; {retry}]") else: - body.append(f"[{last - number + 1} more lines; read again with start_line={number}]") + body.append(f"[stopped at max_chars={max_chars} characters; lines {number}-{last} " + f"not shown; read again with start_line={number}, or raise max_chars]") break used += len(row) + 1 body.append(row) - if start_line > len(lines): - return f"[{path} has {len(lines)} lines; nothing at line {start_line}]" - return "\n".join(body) + else: + if last < requested: + body.append(f"[limit={limit} lines reached; {requested - last} more through line " + f"{requested}; read again with start_line={last + 1}]") + return Text("\n".join(body)) def ls(self, path: str = ".", pattern: str | None = None, *, hidden: bool = False) -> Rows: """One directory, directories suffixed with `/`.""" @@ -120,14 +190,28 @@ class Files: entries.append(entry.name + ("/" if entry.is_dir() else "")) return Rows(entries) - async def find(self, pattern: str, path: str = ".", *, glob: str | Sequence[str] | None = None, - max_results: int = 50, literal: bool = False, - case_sensitive: bool | None = None, hidden: bool = False) -> Rows: - """Content search through ripgrep when it is installed, else pure Python.""" - target = Path(path).expanduser() + def find(self, pattern: str, path: str | Sequence[str] = ".", *, + glob: str | Sequence[str] | None = None, context: int = 0, + max_results: int = 50, literal: bool = False, + case_sensitive: bool | None = None, hidden: bool = False) -> Search: + """Content search through ripgrep when it is installed, else pure Python. + `path` may be one path or a list; `context=N` adds N lines around each + match. Await it: `await files.find(pattern)`.""" + if not 0 <= context <= 50: + raise ValueError("0 <= context <= 50") + return Search("find", lambda: self._find(pattern, path, glob, context, max_results, + literal, case_sensitive, hidden)) + + async def _find(self, pattern: str, path: str | Sequence[str], glob: str | Sequence[str] | None, + context: int, max_results: int, literal: bool, case_sensitive: bool | None, + hidden: bool) -> Rows: + targets = [Path(item).expanduser() for item in ([path] if isinstance(path, str) else path)] if not _which("rg"): - return _fallback_find(pattern, target, glob, max_results, literal, case_sensitive, hidden) + return _fallback_find(pattern, targets, glob, context, max_results, + literal, case_sensitive, hidden) flags = ["--json"] + if context: + flags += ["-C", str(context)] if literal: flags.append("-F") if case_sensitive is True: @@ -139,28 +223,37 @@ class Files: for value in ([glob] if isinstance(glob, str) else list(glob or [])): flags += ["-g", value] command = " ".join(shlex.quote(part) for part in - ["rg", *flags, "-e", pattern, str(target)]) - status, output = await _run(f"{command} | head -n {max(1, max_results) * 4}") + ["rg", *flags, "-e", pattern, *map(str, targets)]) + per_match = 4 + 2 * context + status, output = await _run(f"{command} | head -n {max(1, max_results) * per_match}") if status not in (0, 1, None) and not output.strip(): raise RuntimeError(f"search failed: {output.strip()[:400]}") - results = [] + results, matched = [], 0 for line in output.splitlines(): try: event = json.loads(line) except ValueError: continue - if event.get("type") != "match": + kind = event.get("type") + if kind not in ("match", "context"): continue + if kind == "match" and matched >= max_results: + return Rows(results, truncated=True) data = event["data"] results.append(Match(data["path"]["text"], data["line_number"], - data["lines"]["text"].rstrip("\r\n")[:LINE_WIDTH * 4])) - if len(results) >= max_results: - return Rows(results, truncated=True) + data["lines"]["text"].rstrip("\r\n")[:LINE_WIDTH * 4], + context=kind == "context")) + matched += kind == "match" return Rows(results) - async def paths(self, pattern: str | None = None, path: str = ".", *, - glob: str | None = None, max_results: int = 100, hidden: bool = False) -> Rows: - """File names, not contents; the same ripgrep-or-Python split.""" + def paths(self, pattern: str | None = None, path: str = ".", *, + glob: str | None = None, max_results: int = 100, hidden: bool = False) -> Search: + """File names, not contents; the same ripgrep-or-Python split. + Await it: `await files.paths(pattern)`.""" + return Search("paths", lambda: self._paths(pattern, path, glob, max_results, hidden)) + + async def _paths(self, pattern: str | None, path: str, glob: str | None, + max_results: int, hidden: bool) -> Rows: target = Path(path).expanduser() if not _which("rg"): return _fallback_paths(pattern, target, glob, max_results, hidden) @@ -176,7 +269,7 @@ class Files: if line and (matcher is None or matcher.search(line))] return Rows(found[:max_results], truncated=len(found) > max_results) - def edit(self, path: str, old_str: str, new_str: str, line_hint: int | None = None) -> str: + def edit(self, path: str, old_str: str, new_str: str, line_hint: int | None = None) -> Text: """Replace one exact, unique string. A miss reports what is actually there.""" if not old_str: raise ValueError("old_str must be non-empty") @@ -195,14 +288,14 @@ class Files: chosen = _choose(content, old_str, found, line_hint, path) replacement = (content[:chosen.index] + new_str + content[chosen.index + len(old_str):]).encode("utf-8") _replace(target, snapshot, replacement) - return f"Edited {target}" + return Text(f"Edited {target}") - def write(self, path: str, content: str) -> str: + def write(self, path: str, content: str) -> Text: """Create or replace a whole file. Use `edit` to change part of one.""" target = Path(path).expanduser() target.parent.mkdir(parents=True, exist_ok=True) target.write_text(content, encoding="utf-8") - return f"Wrote {target.resolve()} ({len(content.encode())} bytes)" + return Text(f"Wrote {target.resolve()} ({len(content.encode())} bytes)") def _which(name: str) -> str | None: @@ -213,23 +306,32 @@ def _which(name: str) -> str | None: return None -def _fallback_find(pattern, path: Path, glob, max_results, literal, case_sensitive, hidden) -> Rows: +def _fallback_find(pattern, targets: list[Path], glob, context, max_results, + literal, case_sensitive, hidden) -> Rows: flags = 0 if case_sensitive else re.IGNORECASE matcher = re.compile(re.escape(pattern) if literal else pattern, flags) globs = [glob] if isinstance(glob, str) else list(glob or []) - results = [] - for file in _walk(path, hidden): - if globs and not any(fnmatch.fnmatch(str(file), value) or fnmatch.fnmatch(file.name, value) for value in globs): - continue - try: - text = file.read_text(encoding="utf-8", errors="replace") - except OSError: - continue - for number, line in enumerate(text.splitlines(), 1): - if matcher.search(line): - results.append(Match(str(file), number, line[:LINE_WIDTH * 4])) - if len(results) >= max_results: + results, matched = [], 0 + for target in targets: + for file in _walk(target, hidden): + if globs and not any(fnmatch.fnmatch(str(file), value) or fnmatch.fnmatch(file.name, value) + for value in globs): + continue + try: + lines = file.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError: + continue + hits = [number for number, line in enumerate(lines, 1) if matcher.search(line)] + hit = set(hits) + shown = 0 + for number in hits: + if matched >= max_results: return Rows(results, truncated=True) + for around in range(max(number - context, shown + 1), min(number + context, len(lines)) + 1): + results.append(Match(str(file), around, lines[around - 1][:LINE_WIDTH * 4], + context=around not in hit)) + shown = around + matched += 1 return Rows(results) diff --git a/src/albedo/harness/extensions/files/extension.gleam b/src/albedo/harness/extensions/files/extension.gleam index b57a6a9..cde84b5 100644 --- a/src/albedo/harness/extensions/files/extension.gleam +++ b/src/albedo/harness/extensions/files/extension.gleam @@ -5,7 +5,7 @@ pub fn extension() -> extension.Extension { "files", "Read, search, and exactly edit workspace files from Python.", "files", - "files.read(path, start_line=, end_line=, limit=) returns complete numbered lines within a bounded result (increase limit for a long line), files.ls(path) lists one directory, and files.edit(path, old, new) replaces one exact unique string, reporting every occurrence with line ranges when it is not unique; retry it with line_hint=. await files.find(pattern, path, glob=) searches contents and await files.paths(pattern) searches names. Prefer these over shell equivalents, and start any other command with bash(command): never use subprocess, os.system, or os.popen, which spawn processes this session cannot supervise.", + "files.read(path, start_line=, end_line=, limit=) returns complete numbered lines; limit counts lines, and a separate max_chars= budget (default 16000 characters) stops very large windows and says where to resume, files.ls(path) lists one directory, and files.edit(path, old, new) replaces one exact unique string, reporting every occurrence with line ranges when it is not unique; retry it with line_hint=. These return immediately, and awaiting them also works. await files.find(pattern, path_or_paths, glob=, context=N) searches contents with N surrounding lines and await files.paths(pattern) searches names; both run a background search and must be awaited. Prefer these over shell equivalents, and start any other command with bash(command): never use subprocess, os.system, or os.popen, which spawn processes this session cannot supervise.", ["python", "bash"], ) } diff --git a/test/harness/files_plugin_test.py b/test/harness/files_plugin_test.py index b3ac32a..adfe940 100644 --- a/test/harness/files_plugin_test.py +++ b/test/harness/files_plugin_test.py @@ -51,8 +51,9 @@ class FilesPluginTest(unittest.TestCase): files = plugin.Files() self.assertEqual(files.read(str(path), start_line=2, end_line=3), " 2 | line 2\n 3 | line 3") - bounded = files.read(str(path), limit=40) - self.assertIn("more lines; read again with start_line=", bounded) + bounded = files.read(str(path), max_chars=40) + self.assertIn("stopped at max_chars=40 characters", bounded) + self.assertIn("read again with start_line=", bounded) self.assertLess(len(bounded.splitlines()), 20) self.assertIn("nothing at line 99", files.read(str(path), start_line=99)) with self.assertRaises(ValueError): @@ -64,11 +65,27 @@ class FilesPluginTest(unittest.TestCase): files = plugin.Files() self.assertEqual(files.read(str(path), start_line=2, end_line=2), f" 2 | {long_line}") - bounded = files.read(str(path), start_line=2, end_line=2, limit=100) - self.assertIn("line 2 has", bounded) - self.assertIn("start_line=2, end_line=2, limit=", bounded) + bounded = files.read(str(path), start_line=2, end_line=2, max_chars=100) + self.assertIn("line 2 is", bounded) + self.assertIn("start_line=2, end_line=2, max_chars=", bounded) self.assertNotIn(" 2 | x", bounded) - self.assertIn("read again with start_line=2", files.read(str(path), limit=20)) + self.assertIn("read again with start_line=2", files.read(str(path), max_chars=20)) + + # Every model read `limit` as a line count; one asked for lines 144-400 + # with limit=400, got six lines of a character budget, and fell back to sed. + def test_limit_counts_lines(self): + path = self.write("many.txt", "".join(f"line {n}\n" for n in range(1, 501))) + files = plugin.Files() + window = files.read(str(path), start_line=144, end_line=400, limit=400) + self.assertEqual(len(window.splitlines()), 257) + self.assertTrue(window.endswith(" 400 | line 400")) + capped = files.read(str(path), start_line=10, limit=3) + self.assertEqual(capped.splitlines()[:3], + [" 10 | line 10", " 11 | line 11", " 12 | line 12"]) + self.assertIn("limit=3 lines reached", capped) + self.assertIn("start_line=13", capped) + with self.assertRaises(ValueError): + files.read(str(path), limit=0) def test_edit_replaces_one_exact_occurrence_and_keeps_the_mode(self): path = self.write("edit.py", "alpha\nbeta\ngamma\n") @@ -137,6 +154,58 @@ class FilesPluginTest(unittest.TestCase): self.assertEqual([Path(name).name for name in names], ["one.py"]) self.assertEqual(list(files.ls(str(self.root))), ["nested/"]) + # Agents kept awaiting the synchronous calls and forgetting to await the + # searches; both spellings of the synchronous calls must work, and a + # forgotten await must say what to do instead of printing a coroutine. + def test_ready_results_may_be_awaited_or_used_directly(self): + path = self.write("sample.py", "alpha\nbeta\n") + files = plugin.Files() + + async def awaited(): + return (await files.read(str(path)), await files.ls(str(self.root)), + await files.edit(str(path), "beta", "gamma"), + await files.write(str(self.root/"new.txt"), "x")) + + read, listing, edited, wrote = self.loop.run_until_complete(awaited()) + self.assertEqual(read, " 1 | alpha\n 2 | beta") + self.assertEqual(list(listing), ["sample.py"]) + self.assertIn("Edited", edited) + self.assertIn("Wrote", wrote) + self.assertIsInstance(files.read(str(path)), str) + + def test_an_unawaited_search_explains_itself(self): + files = plugin.Files() + pending = files.find("needle", str(self.root)) + self.assertIn("await files.find(", repr(pending)) + with self.assertRaisesRegex(TypeError, r"use `await files\.find"): + list(pending) + with self.assertRaisesRegex(TypeError, r"use `await files\.paths"): + len(files.paths("x", str(self.root))) + + # Agents shelled out to `grep -B2 -A8` and multi-path greps because find + # had no context and one path; they also wrote `await files.find(...)[:35]`, + # which slices before it awaits, and `files.read(path, 600, 720)`. + def test_find_context_paths_slicing_and_positional_reads(self): + self.write("a/one.py", "zero\nalpha\nneedle\nbeta\nneedle\ngamma\n") + self.write("b/two.py", "needle\n") + files = plugin.Files() + with patch.object(plugin, "_which", lambda name: None): + found = self.loop.run_until_complete(files.find( + "needle", [str(self.root/"a"), str(self.root/"b")], context=1)) + first = self.loop.run_until_complete(files.find("needle", str(self.root/"a"))[:1]) + one = self.loop.run_until_complete(files.find("needle", str(self.root/"a"))[0]) + rows = [(Path(item.path).name, item.line, item.context) for item in found] + self.assertEqual(rows, [("one.py", 2, True), ("one.py", 3, False), ("one.py", 4, True), + ("one.py", 5, False), ("one.py", 6, True), ("two.py", 1, False)]) + self.assertTrue(str(found[0]).endswith("one.py-2- alpha")) + self.assertTrue(str(found[1]).endswith("one.py:3: needle")) + self.assertEqual(len(first), 1) + self.assertEqual(one.line, 3) + path = self.root/"a"/"one.py" + self.assertEqual(files.read(str(path), 2, 3), " 2 | alpha\n 3 | needle") + with self.assertRaises(ValueError): + files.find("x", context=51) + if __name__ == "__main__": unittest.main()