Repository navigation
fix(forgetting): a restored episode keeps the time it happened - #191
Merged
Merged
Conversation
`_restore_episode` calls `EpisodicMemory.save(...)` without `created_at`, so a restored episode was stamped with the moment of its return. That is not only a display problem: `created_at` is what decay, the purge window and the L4 promotion order (`consolidation.py`, oldest first) all read, so a restored old episode never aged, never fell under a purge cutoff and queued as if it were new. The time was already lost before the restore: `archived_memories` had no `created_at` column, only `archived_at`. Measured on the live bases, that is 3 159 archived episodes in Hermes, 2 in Mimocode, 0 in CowAgent — all of them irrecoverable, because nothing recorded when they happened. `archive()` now takes `original_created_at` and every path that archives an episode passes the episode's own value (7 paths: `core/episodic.archive_old`, `features/compression`, both episode branches of the `forget` primitive, `dedup_episode_twins`, and the episode branches of `purge_foreign_rows` and `purge_speaker_layer_leaks`). Core-memory archive paths are untouched — that is a different table and a different finding. The column is added by `ensure()` with an idempotent `ALTER TABLE`, because `ensure()` is called from constructors that never run migrations: success is decided by re-reading the schema, never by the text of the error, so a real failure still raises and a lost race is treated as success. A matching alembic revision continues the chain from `20261004_2200_g27`; `ALTER TABLE ADD COLUMN` is schema-only, so the 3 159 existing rows are not rewritten and read as NULL. A NULL means "this row predates the column": the restore falls back to `now`, exactly as before. Substituting `archived_at` was considered and rejected — that is when the row was removed, not when it happened, and inventing a time is worse than admitting there is none. mypy drops 63 -> 42 with no new errors: passing `created_at` through `dedup_episode_twins` surfaced a pre-existing shadowing bug there (`d` used for both the classify() result and the loop variable), which is renamed here. The differential run on identical scratch bases produces byte-identical dry-run output and the same applied row sets.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Cipher208
added a commit
that referenced
this pull request
Nov 4, 2026
…nto (#192) `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. Co-authored-by: Cipher208 <269750686+Cipher208@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Что
_restore_episodeвызываетEpisodicMemory.save(...)безcreated_at, поэтому восстановленный эпизод получал время возврата.Это не косметика:
created_atчитают затухание, окно purge и порядок промоушена в L4 (consolidation.pyсортирует oldest-first). Восстановленный старый эпизод не старел, не попадал под cutoff и вставал в очередь как новый.Время терялось раньше
В
archived_memoriesне было колонкиcreated_at— толькоarchived_at.Значение было в руках при архивации (
archive_oldделаетSELECT *), просто не сохранялось. Эти 3159 строк невосстановимы: выдавать за нихarchived_atбыло бы выдумкой — это время удаления, а не события.Фикс
archive()принимаетoriginal_created_at, и 7 путей архивации эпизодов передают собственное время:core/episodic.archive_old,features/compression, обе эпизодные ветки примитиваforget,dedup_episode_twins, эпизодные веткиpurge_foreign_rowsиpurge_speaker_layer_leaks.core_memoryне тронуты — другая таблица, другая находка.ensure()идемпотентнымALTER TABLE:ensure()зовут конструкторы, которые миграций не запускают. Успех решается перечитыванием схемы, а не текстом ошибки, поэтому настоящий сбой по-прежнему пробрасывается, а проигранная гонка считается успехом.20261004_2200_g27.NULLозначает «строка старше колонки» → restore падает на прежнее поведение (now), без исключения.Проверка
7b88d04). Падение — из-за переименования вdedup_episode_twins, гдеdиспользовалась и для результатаclassify(), и как переменная цикла; переименование не меняет поведения (дифференциальный прогон на идентичных базах: dry-run байт-в-байт, apply — те же id и строки);ensure()добавляет колонку, строки целы; 4 процесса × 3 раунда гонки ALTER — ни одного исключения;alembic upgrade headна базе с 5 строками — строки не изменились.Найдено попутно, не тронуто
scripts/purge_speaker_layer_leaks.py:226пишетmemory_type="episodes"(множественное число), аroute_ofраспознаёт только"episode"— такие строки при restore ушли бы вcore_memoryвместоepisodes. Баг существовал до этого PR; чинится одной строкой, но это отдельная находка.