From 1c09c0968e547ac073f27000deaa68d33c06b005 Mon Sep 17 00:00:00 2001 From: Cipher208 <269750686+Cipher208@users.noreply.github.com> Date: Wed, 4 Nov 2026 08:51:59 +0100 Subject: [PATCH] fix(forgetting): the bin's label decides the table a restore writes into MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `purge_speaker_layer_leaks` archived an episode with `memory_type="episodes"`. `route_of` names the TABLE the row came from and expects the singular `episode`; anything it does not recognise falls through to the `core_memory` default. A restore would therefore have written an episode's summary into `core_memory`. Measured on the live bases, no row carries the wrong label, so the defect was latent — but any replay that reintroduces the leak class would have produced them. Fixed end-to-end: the episode now lands back in `episodes`. While proving that, a second and opposite error turned up. Since #191 the restore carries `created_at` when the bin recorded it, but `_CARRIED_INTO_EPISODES` is a static set, so `not_recorded` still listed `created_at` as lost for a row whose time WAS restored. That report is exactly what a caller reads to learn the limits of undo, so it was understating what came back. `_carried_into_episodes(row)` now answers per row: `created_at` joins the carried set only when `original_created_at` is present and not NULL. The check goes through `row.keys()` rather than `"col" in row` — on a `sqlite3.Row` the latter tests VALUES and silently answers False — and a bin older than the column behaves like NULL, which is what it means. Audited every `memory_type=` written in the repository: no other label routes to the wrong table. `graph:*` and `project_archive` are refused by design, the `memory_kind` values and `core_memory` all route to `core_memory`, and a missing label is refused rather than guessed. Not fixed here, reported instead: `memory_type="core_memory"` routes to the right table but `_restore_core` writes that label into `memory_kind`, which is not a valid kind — such rows are skipped by type-aware decay and invisible to kind-filtered reads. That is a separate finding with its own decision. --- lifecycle/forgetting.py | 21 ++++++++++++++- scripts/purge_speaker_layer_leaks.py | 2 +- tests/test_lifecycle/test_restore_entries.py | 27 +++++++++++++++++++ .../test_purge_speaker_layer_leaks.py | 27 +++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/lifecycle/forgetting.py b/lifecycle/forgetting.py index 55042652..dd06ba47 100644 --- a/lifecycle/forgetting.py +++ b/lifecycle/forgetting.py @@ -92,6 +92,25 @@ class RestoreOutcome: _CARRIED_INTO_CORE = frozenset({"user_id", "key", "value", "importance", "memory_kind"}) +def _carried_into_episodes(row: Any) -> frozenset[str]: + """Return the episode columns this archived row can fill — one answer per row. + + `created_at` is carried only when the bin recorded the moment the episode + happened. Since #191 `_restore_episode` writes that time back, so reporting it + as lost was a limit stated in the caller's favour: `not_recorded` exists to tell + a caller what undo cannot bring back. + + The column may be absent on a bin older than it, and `in row` on `sqlite3.Row` + tests *values*, not column names, so the check goes through `keys()`. A missing + column means the same as `NULL` here — no recorded time. + """ + if "original_created_at" not in row.keys(): # noqa: SIM118 + return _CARRIED_INTO_EPISODES + if row["original_created_at"] is None: + return _CARRIED_INTO_EPISODES + return _CARRIED_INTO_EPISODES | {"created_at"} + + async def _not_carried(conn: Any, table: str, carried: frozenset[str]) -> set[str]: """Return the columns of `table` an archive row cannot fill — **read from the schema**. @@ -411,7 +430,7 @@ async def restore_entries(self, ids: list[int], user_id: str, memory_type: str | ) continue if route_of(row["memory_type"]) == ROUTE_EPISODES: - destination, carried = "episodes", _CARRIED_INTO_EPISODES + destination, carried = "episodes", _carried_into_episodes(row) else: destination, carried = "core_memory", _CARRIED_INTO_CORE if destination == "core_memory": diff --git a/scripts/purge_speaker_layer_leaks.py b/scripts/purge_speaker_layer_leaks.py index 4416aa02..1611b824 100755 --- a/scripts/purge_speaker_layer_leaks.py +++ b/scripts/purge_speaker_layer_leaks.py @@ -223,7 +223,7 @@ async def _audit(target_id: str, details: dict[str, Any]) -> None: await am.archive( user_id=r["user_id"] or "default", content=str(r["summary"]), - memory_type="episodes", + memory_type="episode", importance=float(r["emotional_weight"] or 0.0), original_id=int(r["episode_id"]), reason=ARCHIVE_REASON, diff --git a/tests/test_lifecycle/test_restore_entries.py b/tests/test_lifecycle/test_restore_entries.py index cc0f5138..03d2a154 100644 --- a/tests/test_lifecycle/test_restore_entries.py +++ b/tests/test_lifecycle/test_restore_entries.py @@ -78,6 +78,7 @@ async def _archive( reason: str = "inactive_90d", user_id: str = U1, importance: float = 0.5, + original_created_at: float | None = None, ) -> int: store = _am(cm) return await store.archive( @@ -87,6 +88,7 @@ async def _archive( importance=importance, original_id=original_id, reason=reason, + original_created_at=original_created_at, ) @@ -185,6 +187,31 @@ async def test_a_restored_episode_keeps_the_moment_it_happened(cm: Any) -> None: assert row["created_at"] == pytest.approx(t0), "the restored episode was stamped with the moment of its return" +async def test_not_recorded_names_created_at_only_when_the_time_was_not_kept(cm: Any) -> None: + """`not_recorded` is what a caller reads to know the limits of undo. + + Since #191 `_restore_episode` gives the time back when the bin holds + `original_created_at`. The report kept saying `created_at` was lost for that + row anyway, which is the limit lying in the caller's favour — the opposite of + its job. Both sides are asserted so the fix cannot collapse into "never lost". + """ + await _archive( + cm, + original_id=851, + content="a moment the bin kept", + memory_type="episode", + original_created_at=time.time() - 86400, + ) + await _archive(cm, original_id=852, content="a moment the bin never kept", memory_type="episode") + + kept = await _fs(cm).restore_entries([851], user_id=U1) + lost = await _fs(cm).restore_entries([852], user_id=U1) + + assert kept.restored == 1 and lost.restored == 1 + assert "created_at" not in kept.restored_items[0]["not_recorded"], "a time that came back was reported as lost" + assert "created_at" in lost.restored_items[0]["not_recorded"], "a time the archive never kept was reported as returned" + + async def test_an_archived_episode_with_no_recorded_time_comes_back_now(cm: Any) -> None: """The 3159 rows already in the bin have no time to give back — measured on the Hermes base 2026-11-03 — and `archived_at` is when the row was removed, diff --git a/tests/test_scripts/test_purge_speaker_layer_leaks.py b/tests/test_scripts/test_purge_speaker_layer_leaks.py index f797b514..fd3337f2 100644 --- a/tests/test_scripts/test_purge_speaker_layer_leaks.py +++ b/tests/test_scripts/test_purge_speaker_layer_leaks.py @@ -179,6 +179,33 @@ async def test_apply_writes_one_audit_row_per_removed_record(db, tmp_path) -> No assert all("speaker_layer_leak" in str(r[3]) for r in rows), "причина названа в details" +async def test_an_episode_archived_by_the_script_restores_into_episodes(db, tmp_path) -> None: + """The label the script writes must be the one `route_of` understands. + + The script used to archive an episode under `"episodes"` — the table name in the + plural — while `route_of` names a table by the singular label `"episode"`. + Anything unrecognised falls through to `core_memory`, so a replay that produced + this class of leak would have restored the summary as an L4 key/value row. + """ + from lifecycle.forgetting import ForgettingSystem + from scripts.purge_speaker_layer_leaks import apply, plan + + agent_l0 = await _l0("agent", "реплика агента") + ep_id = await _episode("user", "утекший эпизод", agent_l0) + + db_path = connection_manager.base_dir / DB_NAME + await apply(plan(db_path), db_path) + + outcome = await ForgettingSystem(cm=connection_manager, layer="user").restore_entries([ep_id], user_id="default") + + assert outcome.restored == 1, outcome + conn = await connection_manager.get(DB_NAME) + episodes = await (await conn.execute("SELECT summary FROM episodes")).fetchall() + assert [str(r[0]) for r in episodes] == ["утекший эпизод"], "the episode did not return to episodes" + facts = await (await conn.execute("SELECT COUNT(*) FROM core_memory")).fetchone() + assert int((facts or (0,))[0]) == 0, "the episode was filed as an L4 fact instead of an episode" + + async def test_apply_removes_the_episodes_the_plan_reports(db, tmp_path) -> None: """Регресс: план и отчёт считали эпизоды, а apply их не удалял — молча.