emrg: the llm.jsonl rotation drops the generations numbered past a gap

emrg/session.py

@@ -913,6 +913,31 @@ def append_llm(self, record: dict) -> None:
         with open(self._llm_path, "a", encoding="utf-8") as f:
             f.write(line)
 
+    def _stray_generations(self) -> list[Path]:
+        """Backup generations numbered at or above ``_LLM_LOG_BACKUP_COUNT``.
+
+        The directory is listed and each name read, rather than the numbering being
+        counted up to the first absent one: a gap must not hide the generations past
+        it. Only a name that is exactly ``<live>.<digits>`` is a generation — the
+        suffix has to be ASCII digits, so no other file and no spelled-out numeral is
+        taken for one.
+        """
+        prefix = self._llm_path.name + "."
+        try:
+            entries = list(self._llm_path.parent.iterdir())
+        except OSError:
+            return []
+        strays = []
+        for entry in entries:
+            if not entry.name.startswith(prefix):
+                continue
+            suffix = entry.name[len(prefix):]
+            if not (suffix.isascii() and suffix.isdigit()):
+                continue
+            if int(suffix) >= _LLM_LOG_BACKUP_COUNT:
+                strays.append(entry)
+        return strays
+
     def _rotate_llm_log(self) -> None:
         """Rotate llm.jsonl if it exceeds _LLM_LOG_MAX_BYTES."""
         if not self._llm_path.exists():
@@ -925,25 +950,19 @@ def _rotate_llm_log(self) -> None:
             return
 
         # Shift the backups down, dropping the oldest: .{n-1} → .n, …, .1 → .2,
-        # main → .1. The oldest generation is removed outright, so the file set
-        # is the live log plus _LLM_LOG_BACKUP_COUNT backups and no more.
+        # main → .1. Every generation numbered at or above the count goes, wherever
+        # it sits in the numbering, so the set that remains is the live log plus
+        # _LLM_LOG_BACKUP_COUNT backups and no more.
         #
-        # Every generation numbered at or above the count goes, not only the one
-        # the count names. A session written by the loop this replaces already
-        # carries a third generation, and leaving it would keep three backups
-        # for the rest of that session's life — while the shift below would then
-        # delete the second-newest and preserve the oldest, which is the wrong
-        # two to keep.
-        generation = _LLM_LOG_BACKUP_COUNT + 1
-        while True:
-            stray = Path(str(self._llm_path) + f".{generation}")
-            if not stray.exists():
-                break
-            stray.unlink()
-            generation += 1
-        oldest = Path(str(self._llm_path) + f".{_LLM_LOG_BACKUP_COUNT}")
-        if oldest.exists():
-            oldest.unlink()
+        # The strays are enumerated, not counted up to. A session can hold .4 with
+        # no .3 — a numbering left by a directory that was restored, copied or
+        # edited by hand — and a sweep that stopped at the first absent name would
+        # leave that generation behind for the rest of the session's life, while
+        # the shift below deleted the second-newest and preserved the oldest: the
+        # wrong two to keep. A session written by the loop this replaces carries a
+        # third generation too, which the same list removes.
+        for stray in self._stray_generations():
+            stray.unlink(missing_ok=True)
         for i in range(_LLM_LOG_BACKUP_COUNT - 1, 0, -1):
             old = Path(str(self._llm_path) + f".{i}")
             if old.exists():

tests/test_session.py

@@ -1509,3 +1509,58 @@ def test_a_fifo_input_history_does_not_wedge_the_write(self, tmp_path):
 
     def _session_dir(self, tmp_path, name="s1"):
         return tmp_path / ".emrg" / "sessions" / name
+
+
+# Which generations a session already holds, and which two must remain afterwards.
+# The numbering is the one the code writes — `llm.jsonl.1` is the newest backup —
+# and the last two rows are the states a count-up sweep read wrongly.
+ROTATION_STARTING_STATES = [
+    ((), {1: "live"}),
+    ((1,), {1: "live", 2: "old-1"}),
+    ((1, 2), {1: "live", 2: "old-1"}),
+    ((1, 2, 3), {1: "live", 2: "old-1"}),
+    ((1, 2, 3, 4, 5), {1: "live", 2: "old-1"}),
+    ((1, 2, 4), {1: "live", 2: "old-1"}),
+    ((1, 3), {1: "live", 2: "old-1"}),
+]
+
+
+@pytest.mark.parametrize("backups, expected", ROTATION_STARTING_STATES)
+def test_the_rotation_keeps_the_newest_backups_whatever_the_starting_state(
+    tmp_path, monkeypatch, backups, expected
+):
+    """The survivors are named, because a count cannot tell "wrong two" from "one too many".
+
+    Each backup's first line names the generation that wrote it, so this asserts which
+    generations remain rather than how many files there are — the two readings that come
+    apart when a shift keeps the oldest instead of the newest.
+
+    The row carrying a gap past the count (`live .1 .2 .4`, with no `.3`) is the one a
+    sweep that stopped at the first absent name read wrongly: `.4` survived and the set
+    never came back to the two generations the count declares, so that row fails against
+    such a sweep while the other six pass. A gap *below* the count is not the same case —
+    `.2` is overwritten by the shift whatever was there — and `live .1 .3` is carried to
+    say so, since it passes either way.
+    """
+    monkeypatch.setattr("emrg.session._LLM_LOG_MAX_BYTES", 64)
+
+    session = Session("s1", tmp_path)
+    live = session._llm_path
+    live.write_text("live\n" + "x" * 80, encoding="utf-8")
+    for generation in backups:
+        Path(str(live) + f".{generation}").write_text(
+            f"old-{generation}\n" + "x" * 80, encoding="utf-8"
+        )
+
+    session._rotate_llm_log()
+
+    survivors = {}
+    for entry in sorted(live.parent.iterdir()):
+        if not entry.name.startswith(live.name + "."):
+            continue
+        survivors[entry.name] = entry.read_text(encoding="utf-8").splitlines()[0]
+
+    assert survivors == {
+        f"{live.name}.{number}": generation
+        for number, generation in expected.items()
+    }