nisse-review: read long files in parts; a retry starts afresh

From its failed review of #156: read_file stopped at 20,000 characters
with no way on, so the reviewer hunted each file's end a definition at a
time and ran out of turns; its retry carried every read before it, and a
model call timed out. Both drafts were lost.

- read_file takes a start line and says where the next part starts.
- Each run's view starts at its brief (Transcript.since); the log keeps
  the rest.
- A run out of turns with points drafted posts them as a partial review.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

nisse-review/src/nisse_review/reader.py

@@ -27,9 +27,11 @@ MAX_ISSUES = 20
 REF = "head (the PR's code) or base (the branch it merges into)."
 READ_FILE = tool(
     "read_file",
-    "Read a file in the repository. Its lines are numbered, for comment.",
+    "Read a file in the repository. Its lines are numbered, for comment. A "
+    "long file comes in parts: the end of each says where the next starts.",
     path="The file's path from the repository root.",
     ref=REF,
+    start="Optional: the line to start from (1, the default, is the top).",
 )
 LIST_FILES = tool(
     "list_files",
@@ -131,11 +133,27 @@ class Repository:
         lines = self.lines(path, ref)
         if lines is None:
             return ToolOutcome(f"{path}: no such file at {ref}", False)
-        text = "\n".join(f"{n:>5}  {line}" for n, line in enumerate(lines, 1))
-        if len(text) > MAX_FILE_CHARS:
-            more = len(text) - MAX_FILE_CHARS
-            text = text[:MAX_FILE_CHARS] + f"\n[{more} more characters not shown]"
-        return ToolOutcome(f"[{self.repo}@{ref}: {path}]\n{text}", True)
+        raw = str(arguments.get("start") or "1").strip()
+        if not raw.isdigit() or not 1 <= int(raw) <= max(len(lines), 1):
+            return ToolOutcome(f"start is a line number from 1 to {len(lines)}", False)
+        # A part of at most MAX_FILE_CHARS, in whole lines; the rest is a
+        # read away, never out of reach (#156: a long file's end was).
+        first = int(raw)
+        shown: list[str] = []
+        size = 0
+        for n in range(first, len(lines) + 1):
+            row = f"{n:>5}  {lines[n - 1]}"
+            if shown and size + len(row) + 1 > MAX_FILE_CHARS:
+                break
+            shown.append(row[:MAX_FILE_CHARS])
+            size += len(row) + 1
+        last = first + len(shown) - 1
+        where = f"lines {first}-{last} of {len(lines)}"
+        more = (
+            f"\n[{where}: read on with start={last + 1}]" if last < len(lines) else ""
+        )
+        text = "\n".join(shown)
+        return ToolOutcome(f"[{self.repo}@{ref}: {path}, {where}]\n{text}{more}", True)
 
     def _list(self, arguments: Mapping[str, Any]) -> ToolOutcome:
         folder = str(arguments.get("path", "")).strip().strip("/")

nisse-review/src/nisse_review/review.py

@@ -118,6 +118,10 @@ class Reviewer:
         """The review run: it ends waiting on post_review, or not."""
         transcript = self._store.transcript(item_id, "driver")
         start = transcript.read()[-1].id if transcript.read() else 0
+        # Each run starts afresh from its brief: a retry doesn't carry the
+        # failed run's reads (#156: they filled the context, and a call timed
+        # out). The log keeps them.
+        transcript = transcript.since(start)
         self._brief(transcript, repo.repo, pr)
         project = _project(repo.repo)
         index = self._knowledge.index(item_id, project)

nisse-review/src/nisse_review/scheduler.py

@@ -38,6 +38,12 @@ from nisse_review.state import (
     pr_of,
 )
 
+PARTIAL = (
+    "A partial review: I ran out of turns before finishing, so these are the "
+    "{n} point(s) I had drafted, without an overall view. A new push gets a "
+    "full look."
+)
+
 _WIP = re.compile(r"^\s*(\[wip\]|wip\b)", re.IGNORECASE)
 
 
@@ -238,10 +244,18 @@ class Scheduler:
                 outcome, draft = self._reviewer.run(item.id, pr, repo)
             finally:
                 repo.close()
-            if outcome.kind != "wait":
+            if outcome.kind == "capped" and draft.comments:
+                # Out of turns with points drafted: they're worth more posted
+                # than lost (#156), as a review that says it's partial.
+                self._oplog.emit(
+                    "review.partial", cause=outcome.note, work_item=item.id
+                )
+                summary = PARTIAL.format(n=len(draft.comments))
+            elif outcome.kind != "wait":
                 self._failed(item, known, pr, outcome.note or outcome.kind)
                 return
-            summary = str((outcome.arguments or {}).get("summary", ""))
+            else:
+                summary = str((outcome.arguments or {}).get("summary", ""))
             posted = self._reviewer.post(item.id, known.repo, pr, summary, draft)
         except GiteaError as exc:
             self._failed(item, known, None, f"Gitea: {exc}")

nisse-review/tests/test_review.py

@@ -553,3 +553,67 @@ def test_the_config_names_every_problem(tmp_path: Path) -> None:
         ReviewConfig.from_tree({**tree, "matrix": {"x": 1}}, {"LLM": "k"})
     assert "matrix.x: unknown setting" in str(raised.value)
     assert "references ${GITEA}" in str(raised.value)
+
+
+def test_a_long_file_is_read_in_parts_to_its_end(tmp_path: Path) -> None:
+    # #156: read_file stopped at 20,000 characters with no way on, so the
+    # reviewer hunted the rest a line at a time and ran out of turns.
+    from nisse_review.reader import MAX_FILE_CHARS, Repository
+
+    body = "".join(f"x{n} = {'y' * 60}\n" for n in range(1, 1001))
+    gitea = FakeGitea(files={"long.py": body})
+    repo = Repository(gitea, REPO, {"head": "abc"}, tmp_path)  # type: ignore[arg-type]
+    first = repo.dispatch("read_file", {"path": "long.py"}).result
+    assert len(first) < MAX_FILE_CHARS + 200
+    assert "lines 1-" in first and "of 1000" in first
+    seen, start = [], 1
+    while True:
+        part = repo.dispatch("read_file", {"path": "long.py", "start": start})
+        assert part.success
+        seen += [
+            int(r.split()[0])
+            for r in part.result.splitlines()[1:]
+            if r[:5].strip().isdigit()
+        ]
+        if "read on with start=" not in part.result:
+            break
+        start = int(part.result.rsplit("start=", 1)[1].rstrip("]"))
+    assert seen == list(range(1, 1001))  # every line, once
+    bad = repo.dispatch("read_file", {"path": "long.py", "start": "2000"})
+    assert not bad.success and "1 to 1000" in bad.result
+
+
+def test_a_retry_starts_afresh_from_its_brief(tmp_path: Path) -> None:
+    # #156: the retry carried the failed run's reads, and a call timed out.
+    prose = [AssistantText(text="Reading on...", model="m")]
+    model = Scripted(prose, prose, *REVIEW)  # prose twice: the first run fails
+    w = world(tmp_path, model, attempts=2)
+    w["scheduler"].round()
+    assert pr_of(item(w)).attempts == 1
+    w["scheduler"].round()
+    retry = model.seen[2]  # the retry's first call
+    briefs = [e for e in retry if "Review o/demo#7" in str(e.event)]
+    assert len(briefs) == 1 and retry[0] is briefs[0]
+    assert not any("Reading on" in str(e.event) for e in retry)
+    assert item(w).position == REVIEWED
+
+
+def test_out_of_turns_its_drafted_points_are_posted_as_partial(
+    tmp_path: Path,
+) -> None:
+    model = Scripted(*REVIEW[:3])  # drafts two comments, never posts
+    w = world(tmp_path, model)
+    w["scheduler"]._reviewer._max_turns = 3
+    w["scheduler"].round()
+    [posted] = w["gitea"].posted
+    assert len(posted["comments"]) == 2 and "A partial review" in posted["body"]
+    assert item(w).position == REVIEWED
+
+
+def test_out_of_turns_with_nothing_drafted_is_a_failed_run(tmp_path: Path) -> None:
+    model = Scripted(REVIEW[0], REVIEW[0], REVIEW[0])
+    w = world(tmp_path, model, attempts=2)
+    w["scheduler"]._reviewer._max_turns = 3
+    w["scheduler"].round()
+    assert w["gitea"].posted == []
+    assert (item(w).position, pr_of(item(w)).attempts) == (READY, 1)