feat: add mine_epoch tracking and revision snapshots - #607
Conversation
Adds per-mine-run epoch counter and revision history snapshots, building on top of MemPalace#521's delete-before-insert fix. - _load_epoch/_save_epoch helpers for {palace}/epoch.json - mine_epoch metadata tag on every drawer - _snapshot_revisions writes old chunks to {palace}/revisions.jsonl before upstream's purge runs, preserving previous versions - status command shows current epoch No changes to existing delete-before-insert logic.
11 pytest tests covering epoch tracking (4), stale eviction behavior (3), revision snapshots (3), and backward compatibility (1). All pass in ~24s. No network, no API keys, follows existing pytest conventions.
web3guru888
left a comment
There was a problem hiding this comment.
This is a well-scoped feature pair — epoch tracking and revision snapshots compose cleanly and the implementation is non-intrusive.
What works well
- Epoch counter is monotonic and persisted atomically to
epoch.json— simple, no external deps, survives crashes between runs. _snapshot_revisions()is wrapped intry/exceptso a write failure never breaks mining. Right call.- The
mine_epoch: int = 0default onprocess_file()andadd_drawer()means callers that don't passpalace_pathget graceful degradation (epoch 0 in metadata, no snapshot attempt). The backward-compat test validates this explicitly. - Test coverage is solid: epoch lifecycle, eviction behaviour, snapshot content integrity, accumulation across runs. The
UNIQUE_MARKER_STRING_XYZZY_12345round-trip test is particularly good — it proves the content is right, not just that a file was created.
One real concern — revisions.jsonl grows unboundedly
Every re-mine of a changing file appends records. A project that re-mines a large codebase every day will accumulate tens of thousands of lines quickly. There's no rotation, truncation, or max-size guard. A 50K-drawer palace that re-mines 10% of its files daily would write ~5K revision records per run — ~1.5 million records after a year, uncompressed.
Worth at least a MAX_REVISIONS constant (default ~50,000 lines) with a tail-truncation on open, or a note in the docstring that the file is intentionally unbounded and callers should manage it themselves. The --purge command from #562 is a natural place to reset revisions too.
Minor: epoch.json is inside the palace directory
This is fine for the common case, but palace directories may be version-controlled or synced. epoch.json and revisions.jsonl should probably be documented as ephemeral/local files that shouldn't be committed. A note in the PR description or a .gitignore suggestion for {palace}/*.jsonl and {palace}/epoch.json would help.
Interaction with #562's cmd_purge
The purge-and-rebuild command in #562 deletes and recreates the palace directory. After a purge, _load_epoch() returns 0 and the counter restarts from 1, which means epoch numbers become non-monotonic across purge boundaries. If continuity matters (audit trail), cmd_purge would need to preserve epoch.json (or at least record the last epoch before purge in the new one). Minor edge case, but worth a comment in cmd_purge.
Verdict: LGTM with the revisions.jsonl growth concern as the main thing to address before merge.
Addresses review feedback: - mine_epoch is now int(time.time()) with +1 bump on same-second collision - Survives cmd_purge rebuilds automatically (Unix time is monotonic) - Adds REVISION_RETENTION_SECONDS (default 90 days, env-var configurable) - Time filter + MAX_REVISIONS hard cap applied in one rewrite pass
…al, retention 15 tests total (up from 12). New tests: - test_epoch_is_unix_time_and_monotonic - test_same_second_mine_bumps_epoch_by_one - test_epoch_survives_cmd_purge_equivalent - test_revisions_time_retention_drops_ancient_records Updated existing tests to drop hardcoded epoch=1/2/3 assertions since epochs are now Unix timestamps.
Each line in revisions.jsonl is one chunk snapshot, not a whole-file revision. A file with N chunks that gets re-mined produces N lines. Updated docstring to spell this out, and added MEMPALACE_MAX_REVISIONS env var for consistency with MEMPALACE_REVISION_RETENTION_DAYS. No behavior change. All 15 tests still pass.
|
Thanks for this contribution, and apologies for the slow turnaround.
If you'd rather not pick it back up, no problem at all — just say so and I'll close it out, and thanks either way for taking the time to send it. |
I'll work up to 3.8.0 and roll back in 😊 I'm finding it a lovely useful thing to have. Thank you 😊 |
What does this PR do?
Adds two features that build on top of #521's delete-before-insert fix, giving every mine run a version identity and preserving a queryable history of previous file versions.
1. Mine epoch tracking
Each
mine()run increments a monotonic counter stored in{palace}/epoch.json. Every chunk filed during that run gets tagged with itsmine_epochin metadata. This gives every drawer a "generation number" that enables:current_epoch - chunk.mine_epoch)The
statuscommand now displays the current epoch.2. Revision snapshots
When a file is re-mined, #521's delete-before-insert purges all existing chunks for that file before the fresh ones are written. This avoids hnswlib segfaults but also destroys the previous version of the file. This PR adds a snapshot step that runs just before the delete, capturing the chunks to
{palace}/revisions.jsonlso the previous version can still be retrieved.Each revision record captures:
superseded_at/superseded_by_epoch— when and by which runsource_file/chunk_index— what it wascontent— exact chunk textoriginal_epoch/original_filed_at— provenanceWhy this is valuable
The two features compose naturally:
How to test
11 tests, ~24s, no API keys, no network:
Design notes
json+ existing ChromaDBprocess_file()accepts new args with defaults, existing drawers withoutmine_epochmetadata default to 0Checklist
python -m pytest tests/test_mine_epoch_tracking.py -v)ruff check .) — not run locally, please verify