diff --git a/lifecycle/forgetting.py b/lifecycle/forgetting.py index dd06ba4..907a53b 100644 --- a/lifecycle/forgetting.py +++ b/lifecycle/forgetting.py @@ -111,6 +111,30 @@ def _carried_into_episodes(row: Any) -> frozenset[str]: return _CARRIED_INTO_EPISODES | {"created_at"} +def _carried_into_core(row: Any) -> frozenset[str]: + """Return the core columns this archived row can fill — one answer per row. + + `memory_kind` is carried only when the bin recorded a real kind. `memory_type` + is also the route label, and older versions of the purge scripts recorded the + *source table* (`core_memory`) there for rows they selected from `core_memory`; + writing that label back would be a kind no registry entry matches, so + `_restore_core` writes `NULL` instead and this answer must say the kind was not + carried. `fact` is not + substituted: the archive never recorded one, and inventing it is the guess + `route_of` exists to avoid. + + The column is read, not guarded. `restore_entries` dereferences + `row["memory_type"]` in `refusal_reason` before this helper is reached, so a + bin without that column fails earlier with an `IndexError` and `reason="error"` + instead of arriving here. The column belongs to the bin's original schema, so + its absence means a broken base, not an older one. + """ + recorded = row["memory_type"] + if isinstance(recorded, str) and validate_kind(recorded): + return _CARRIED_INTO_CORE + return _CARRIED_INTO_CORE - {"memory_kind"} + + 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**. @@ -432,7 +456,7 @@ async def restore_entries(self, ids: list[int], user_id: str, memory_type: str | if route_of(row["memory_type"]) == ROUTE_EPISODES: destination, carried = "episodes", _carried_into_episodes(row) else: - destination, carried = "core_memory", _CARRIED_INTO_CORE + destination, carried = "core_memory", _carried_into_core(row) if destination == "core_memory": # Asked *before* the write, and in `dry_run` as well as in the pass, # because otherwise the two modes answer differently about the same @@ -581,11 +605,16 @@ async def _restore_core(self, conn: Any, row: Any, original_id: int, now: float) `partition` splits on the first `=` only, so a value that contains one survives whole: `k=v=w` returns key `k`, value `v=w`. - The kind is written back **exactly as recorded**. An earlier version - substituted `"fact"` when the column was empty — a guess about a row this - method never saw written, and the same guess `fetchone()` used to make about - which row it had found. A row with no recorded kind cannot reach here: - `refusal_reason` refuses it first. + The kind is written back only when it is a **real kind**. `memory_type` is + also the route label, and older versions of the purge scripts recorded the + source table (`core_memory`) there for rows they selected from `core_memory`; + that label routes correctly but is not a kind. Writing it into `memory_kind` + made the + row invisible to kind-filtered reads and exempt from `decay_importance`, + which skips an unvalidated kind — so the row became immortal. An invalid or + absent label is therefore written as `NULL`, "kind unknown", not `fact`: the + archive never recorded a kind, and substituting one is the guess `route_of` + exists to avoid. `_carried_into_core` reports that loss per row. **The layer is named, and that is a repair.** This `INSERT` used to leave the column out, so it took the column default `'user'` whatever layer the caller @@ -598,6 +627,9 @@ async def _restore_core(self, conn: Any, row: Any, original_id: int, now: float) another layer on a collision course with the rows already there. """ key, value = _split_core_content(row, original_id) + recorded_kind = row["memory_type"] + # Only a validated kind is written; a source-table label is a route, not a kind. + kind = recorded_kind if isinstance(recorded_kind, str) and validate_kind(recorded_kind) else None await conn.execute( "INSERT INTO core_memory (layer, user_id, key, value, importance, created_at, updated_at, memory_kind) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", ( @@ -608,7 +640,7 @@ async def _restore_core(self, conn: Any, row: Any, original_id: int, now: float) row["importance"], now, now, - row["memory_type"], + kind, ), ) diff --git a/scripts/purge_foreign_rows.py b/scripts/purge_foreign_rows.py index 1d80a67..fd00489 100755 --- a/scripts/purge_foreign_rows.py +++ b/scripts/purge_foreign_rows.py @@ -45,12 +45,14 @@ flag that does not exist and never did. Putting a run back is an out-of-band operator job, and it would have to bypass the refusal deliberately rather than by calling it. -`memory_type` is written as the **source**, not the kind: `"episode"` for a row from -`episodes` and `"core_memory"` for one from `core_memory`, which is how `route_of` -reads it. The content is stored as `key = value` with spaces, whereas -`_restore_core` splits on the first `=` without trimming — so a restore of one of -these rows would keep the padding in the key. Unreachable today because the reason -refuses first; noted here so that unwinding the refusal is not done blind. +`memory_type` records where a row came from, in the form `route_of` reads: an +episode is written as `"episode"` (the singular label for the `episodes` table), +and a fact is written as its own `memory_kind`, falling back to `"fact"` when that +column is empty — never the source table name. The content is stored as +`key = value` with spaces; `_split_core_content` splits on the first `=` and strips +both sides, so a restore recovers the original key and value without the padding. +Unreachable today because the reason refuses first; noted here so that unwinding +the refusal is not done blind. The L0 hash chain is not touched at all: `l0_journal` rows are left in place with `status='foreign_agent'`, so `verify_chain` stays clean and the chain remains a @@ -136,7 +138,7 @@ def plan(db: Path) -> dict[str, Any]: # --- facts: via metadata.source_raw_id --- foreign_facts: list[sqlite3.Row] = [] - for row in conn.execute("SELECT entry_id, key, value, layer, user_id, importance, metadata FROM core_memory"): + for row in conn.execute("SELECT entry_id, key, value, memory_kind, layer, user_id, importance, metadata FROM core_memory"): meta = row["metadata"] if not meta: continue @@ -227,7 +229,9 @@ async def apply(p: dict[str, Any], db: Path) -> None: await am.archive( user_id=r["user_id"] or "default", content=f"{r['key']} = {r['value']}", - memory_type="core_memory", + # The row's own kind, not the source table: a restore writes this value + # into `memory_kind`, where a table name is not a kind. + memory_type=r["memory_kind"] or "fact", importance=float(r["importance"] or 0.0), original_id=int(r["entry_id"]), reason=ARCHIVE_REASON, diff --git a/scripts/purge_speaker_layer_leaks.py b/scripts/purge_speaker_layer_leaks.py index 1611b82..1cb26ef 100755 --- a/scripts/purge_speaker_layer_leaks.py +++ b/scripts/purge_speaker_layer_leaks.py @@ -198,7 +198,9 @@ async def _audit(target_id: str, details: dict[str, Any]) -> None: await am.archive( user_id=r["user_id"] or "default", content=f"{r['key']} = {r['value']}", - memory_type="core_memory", + # The row's own kind, not the source table: a restore writes this + # value into `memory_kind`, where a table name is not a kind. + memory_type=r["memory_kind"] or "fact", importance=float(r["importance"] or 0.0), original_id=int(r["entry_id"]), reason=ARCHIVE_REASON, diff --git a/shared/archived_memories.py b/shared/archived_memories.py index 01e5156..7d58a78 100644 --- a/shared/archived_memories.py +++ b/shared/archived_memories.py @@ -52,9 +52,11 @@ #: #: `core_memory` is deliberately **not** in this set. It was, on a story that turned #: out to be invented — that the value was a table name which had leaked into a kind -#: field. It has not: `scripts/purge_foreign_rows.py:225` writes it for rows it +#: field. It had not: older versions of the purge scripts wrote it for rows they #: selected **from** `core_memory`, so it names the source, exactly as `episode` -#: names `episodes`. See `route_of`. +#: names `episodes`. Those writers now record the row's real kind, but their earlier +#: bins still hold the label (65 rows on the Hermes base, measured 2026-11-03), so +#: the route must keep accepting it. See `route_of`. _NO_ROUTE_KINDS = frozenset({"project_archive"}) _NO_ROUTE_PREFIXES = ("graph:",) @@ -93,11 +95,12 @@ def route_of(memory_type: str | None) -> str | None: The convention is that the value names the table the row came from: `episode` for `episodes` (the name is singular, the table is not), `core_memory` for - `core_memory` — which is what `scripts/purge_foreign_rows.py:225` writes — and - an L4 kind such as `fact` or `rule` for a row from `core_memory`, where the kind - is the more useful thing to know. So `episodes` is named explicitly and - `core_memory` is the default, with a small closed set of labels that name no - table this can write to (`_NO_ROUTE_KINDS`, `_NO_ROUTE_PREFIXES`). + `core_memory` — which older versions of the purge scripts wrote, and whose bins + still hold it — and an L4 kind such as `fact` or `rule` for a row from + `core_memory`, where the kind is the more useful thing to know. So `episodes` is + named explicitly and `core_memory` is the default, with a small closed set of + labels that name no table this can write to (`_NO_ROUTE_KINDS`, + `_NO_ROUTE_PREFIXES`). `None` is answered in two different situations on purpose — a kind that was never recorded, and a label with no restore path — because both mean the same thing to @@ -243,7 +246,7 @@ async def archive( guessing a destination. So the two ends are not the same expression, even though they look alike. - Two of this method's callers pass `memory_kind or "fact"`, and on the + Several of this method's callers pass `memory_kind or "fact"`, and on the **writing** side that is legitimate: the row was just read out of `core_memory`, so naming it a core kind records where it came from. The same `or "fact"` on the **reading** side is the defect `route_of` exists to diff --git a/tests/test_lifecycle/test_forget_undo.py b/tests/test_lifecycle/test_forget_undo.py index 296859b..4e28128 100644 --- a/tests/test_lifecycle/test_forget_undo.py +++ b/tests/test_lifecycle/test_forget_undo.py @@ -162,9 +162,14 @@ async def test_a_restored_core_row_goes_back_to_core_memory(app: Any, ctx: Any, async def test_a_row_labelled_with_its_source_table_goes_back_to_core_memory(app: Any, ctx: Any, cm: Any) -> None: """`memory_type` names a source when it is a table name, and the route follows it. - `core_memory` is written by `scripts/purge_foreign_rows.py` for rows it selected from + Older versions of the purge scripts wrote `core_memory` for rows they selected from `core_memory`, so it means "this came from `core_memory`" — the same convention as the - singular `episode` naming the table `episodes`. + singular `episode` naming the table `episodes`. Their bins still hold the label. + + The route is the only thing the label can supply. It is not a kind, so the restored + row's `memory_kind` is `NULL` — "unknown", not the table name. Writing the label as a + kind would exempt the row from decay (`validate_kind` rejects it) and hide it from + kind-filtered reads; `fact` would invent a provenance the archive never recorded. """ await _archive(cm, original_id=43, content="a_key=a_value", memory_type="core_memory", reason="inactive_90d") @@ -173,7 +178,8 @@ async def test_a_row_labelled_with_its_source_table_goes_back_to_core_memory(app assert result["restored"] == 1 rows = await _rows(cm, "SELECT key, value, memory_kind FROM core_memory WHERE user_id=?", (U1,)) assert rows[0][0] == "a_key" and rows[0][1] == "a_value" - assert rows[0][2] == "core_memory", "the recorded label is written back exactly, not replaced by a guess" + assert rows[0][2] is None, "the source label is a route, not a kind" + assert "memory_kind" in result["restored_items"][0]["not_recorded"] async def test_undo_reports_what_came_back_and_where_it_went(app: Any, ctx: Any, cm: Any) -> None: diff --git a/tests/test_lifecycle/test_restore_entries.py b/tests/test_lifecycle/test_restore_entries.py index 03d2a15..f545b01 100644 --- a/tests/test_lifecycle/test_restore_entries.py +++ b/tests/test_lifecycle/test_restore_entries.py @@ -362,8 +362,9 @@ async def test_an_unroutable_kind_is_refused_instead_of_written(cm: Any) -> None This test used to use `core_memory` as its example, on the reasoning that the value was a table name leaked into a kind field. That reasoning was invented and - is now known to be false — `purge_foreign_rows.py` writes it deliberately for rows - it selected from `core_memory`, so it routes there (see the test above). The + is now known to be false — older versions of `purge_foreign_rows.py` wrote it + deliberately for rows they selected from `core_memory`, so it routes there (see + the test above). The example is `project_archive` instead, which really has nowhere to go: `project.py` archives a **wiki page** body, and this repository has no restore for one. """ @@ -636,31 +637,61 @@ async def test_a_restored_core_row_without_a_separator_keeps_its_text(cm: Any) - async def test_a_row_labelled_with_its_source_table_routes_there(cm: Any) -> None: """`episode` names the table `episodes` and routes there. `core_memory` names - the table `core_memory`, and the value is written by `scripts/purge_foreign_rows.py` - for rows it selected **from** `core_memory` — so the same convention applies. + the table `core_memory`, and older versions of the purge scripts wrote it for + rows they selected **from** `core_memory` — so the same convention applies. The first version of `route_of` refused it, on a story that turned out to be invented: that the value was a table name which had leaked into a kind field. It had not. The label is deliberate and it names the source; the reason these rows are not recoverable in practice is `archive_reason`, not the route. - What the label does **not** carry is the row's original kind, so the restored - row keeps the label as its kind — a kind the registry does not list, which is - visible rather than disguised as `fact`. + What the label does **not** carry is the row's original kind. An earlier version + kept the label as `memory_kind`, on the argument that a visible junk kind is + better than an invented `fact`. That argument was wrong about the cost: the + label is not a kind the registry knows, and `decay_importance` skips a row whose + kind does not validate — so the row becomes immortal, and a kind-filtered lookup + can never find it under any real kind. A junk value is not a visible marker; it + is an exemption from the rules. So an unvalidated label is written as `NULL`, + which means "kind unknown": it decays as `fact` like any row written without a + kind, and `not_recorded` tells the caller the kind did not come back. The cost is + real and bounded — the restored row cannot be selected by its original kind and + is treated as `fact` by decay — but it is declared, not disguised. """ await _archive(cm, original_id=901, content="purged_key=purged_value", memory_type="core_memory", reason="inactive_90d") outcome = await _fs(cm).restore_entries([901], user_id=U1) assert outcome.restored == 1, "a row whose label names its source table was called unroutable" + assert outcome.restored_items[0]["destination"] == "core_memory", "the route still follows the source label" row = await _rows(cm, "SELECT key, value, memory_kind FROM core_memory WHERE user_id=?", (U1,)) assert row[0][0] == "purged_key" and row[0][1] == "purged_value" - # The consequence, asserted rather than left to be discovered: the label names the - # source, not the kind, so the restored row carries a kind the registry does not - # list. `validate_kind` rejects it and `decay_importance` skips such a row, so it - # will not decay. That is a real difference from a `fact` — and a visible one, - # which is the point of writing what was recorded instead of substituting a default. - assert row[0][2] == "core_memory" + # The label names the source, not the kind, so there is no kind to write back. + # `NULL` is the only honest value: `fact` would invent a provenance the archive + # never recorded, and the label itself would exempt the row from decay. + assert row[0][2] is None, "the source-table label was written into memory_kind" + assert "memory_kind" in outcome.restored_items[0]["not_recorded"], "the lost kind was not reported" + + +async def test_the_recorded_kind_decides_whether_memory_kind_is_carried(cm: Any) -> None: + """`not_recorded` is per row, not per destination: one call restores both kinds. + + A row archived with a real kind keeps it, and the answer does not claim the + kind was lost. A row archived with its source table has no kind to keep, so it + comes back as `NULL` and the answer says so. Both are asserted in one call + because a destination-wide answer — "core rows lose their kind" or "core rows + keep their kind" — would be wrong for one of these two rows. + """ + await _archive(cm, original_id=911, content="kept=fact_value", memory_type="fact") + await _archive(cm, original_id=912, content="lost=label_value", memory_type="core_memory") + + outcome = await _fs(cm).restore_entries([911, 912], user_id=U1) + + assert outcome.restored == 2 + rows = {r["key"]: r["memory_kind"] for r in await _rows(cm, "SELECT key, memory_kind FROM core_memory WHERE user_id=?", (U1,))} + assert rows == {"kept": "fact", "lost": None}, "a real kind was dropped or a source label was kept as a kind" + by_id = {item["original_id"]: item for item in outcome.restored_items} + assert "memory_kind" not in by_id[911]["not_recorded"], "a kind that came back was reported as lost" + assert "memory_kind" in by_id[912]["not_recorded"], "a kind the archive never kept was reported as returned" async def test_the_listing_never_offers_what_the_restore_refuses(cm: Any) -> None: diff --git a/tests/test_scripts/test_purge_foreign_rows.py b/tests/test_scripts/test_purge_foreign_rows.py new file mode 100644 index 0000000..b2397ee --- /dev/null +++ b/tests/test_scripts/test_purge_foreign_rows.py @@ -0,0 +1,75 @@ +"""purge_foreign_rows: the bin records the row's kind, not the source table. + +The defect this guards is the one round 1 repaired: the writer archived every +foreign fact under `"core_memory"` — the table it came from. `route_of` accepts +that label, but it is not a kind: a restore writes it into `memory_kind`, where +`validate_kind` rejects it and `decay_importance` skips the row forever. The plan +already selects `memory_kind`; the writer must pass it through. +""" + +from __future__ import annotations + +import json + +import pytest + +from shared.connection import connection_manager +from shared.constants import DB_NAME +from shared.migrations import MigrationManager + + +@pytest.fixture +async def db(tmp_path): + original = connection_manager.base_dir + connection_manager.base_dir = tmp_path # НЕ подменять объект! + connection_manager._conns.clear() + await MigrationManager(cm=connection_manager).migrate() + yield connection_manager + connection_manager._conns.clear() + connection_manager.base_dir = original # восстановить: stale tmp-dir травит no-db тесты + + +async def _foreign_l0() -> int: + conn = await connection_manager.get(DB_NAME) + cur = await conn.execute( + "INSERT INTO l0_journal (ts, event, layer, user_id, text, raw_type, status, decisions)" + " VALUES (1.0, 'new_message', 'user', 'default', 'чужая реплика', 'user-message'," + " 'foreign_agent', '[]')", + ) + await conn.commit() + return int(cur.lastrowid or 0) + + +async def _fact(key: str, value: str, raw_id: int, kind: str | None) -> int: + conn = await connection_manager.get(DB_NAME) + cur = await conn.execute( + "INSERT INTO core_memory (user_id, key, value, importance, memory_kind, layer, source, metadata," + " created_at, updated_at)" + " VALUES ('default', ?, ?, 0.5, ?, 'user', 'new_message', ?, 1.0, 1.0)", + (key, value, kind, json.dumps({"source_raw_id": raw_id})), + ) + await conn.commit() + return int(cur.lastrowid or 0) + + +async def test_a_fact_is_archived_under_its_kind_not_the_source_table(db: object) -> None: + """The writer must not regress to `memory_type="core_memory"`. + + Reverting only `scripts/purge_foreign_rows.py` leaves every round-1 test green: + they all feed the bin a label directly. This drives the real writer — plan, + then apply — and reads the row it produced. + """ + from scripts.purge_foreign_rows import apply, plan + + raw = await _foreign_l0() + eid = await _fact("rule:чужая", "чужая реплика", raw, "rule") + + db_path = connection_manager.base_dir / DB_NAME + p = plan(db_path) + assert [int(r["entry_id"]) for r in p["facts"]] == [eid], "the fact was not selected" + + await apply(p, db_path) + + conn = await connection_manager.get(DB_NAME) + archived = await (await conn.execute("SELECT memory_type FROM archived_memories WHERE original_id=?", (eid,))).fetchall() + assert [r[0] for r in archived] == ["rule"], "the bin recorded the source table instead of the row's kind" diff --git a/tests/test_scripts/test_purge_speaker_layer_leaks.py b/tests/test_scripts/test_purge_speaker_layer_leaks.py index fd3337f..784ba2d 100644 --- a/tests/test_scripts/test_purge_speaker_layer_leaks.py +++ b/tests/test_scripts/test_purge_speaker_layer_leaks.py @@ -206,6 +206,35 @@ async def test_an_episode_archived_by_the_script_restores_into_episodes(db, tmp_ assert int((facts or (0,))[0]) == 0, "the episode was filed as an L4 fact instead of an episode" +async def test_a_fact_archived_by_the_script_keeps_its_kind(db, tmp_path) -> None: + """The script knows the row's kind from the plan, so the bin must record it. + + It used to archive every fact under `"core_memory"` — the source table. `route_of` + accepts that label, but it is not a kind: a restore would write it into + `memory_kind`, where `validate_kind` rejects it and `decay_importance` skips the + row forever. The plan already selected `memory_kind`; passing it through is the + whole repair. + """ + from lifecycle.forgetting import ForgettingSystem + from scripts.purge_speaker_layer_leaks import apply, plan + + agent_l0 = await _l0("agent", "реплика агента") + eid = await _fact("user", "rule:утекло", "моя реплика", agent_l0) + + db_path = connection_manager.base_dir / DB_NAME + await apply(plan(db_path), db_path) + + conn = await connection_manager.get(DB_NAME) + archived = await (await conn.execute("SELECT memory_type FROM archived_memories WHERE original_id=?", (eid,))).fetchall() + assert [str(r[0]) for r in archived] == ["rule"], "the bin recorded the source table instead of the row's kind" + + outcome = await ForgettingSystem(cm=connection_manager, layer="user").restore_entries([eid], user_id="default") + + assert outcome.restored == 1, outcome + restored = await (await conn.execute("SELECT memory_kind FROM core_memory WHERE user_id='default'")).fetchall() + assert [r[0] for r in restored] == ["rule"], "the recorded kind did not survive the restore" + + async def test_apply_removes_the_episodes_the_plan_reports(db, tmp_path) -> None: """Регресс: план и отчёт считали эпизоды, а apply их не удалял — молча.