test: add checkpoint save/restore round-trip tests

[SHA888]

What

Adds 12 tests for CheckpointManager save/restore round-trip, covering the gap identified in TODO.md: "Add test for checkpoint save/restore".

Tests

1. Basic round-trip — create + restore reproduces all files
2. Metadata preservation — config, metrics, result survive the cycle
3. Integrity hash verification — passes immediately after creation
4. Tamper detection — hash fails when a file is modified post-checkpoint
5. Nonexistent restore — raises CheckpointError
6. Multi-checkpoint isolation — each checkpoint restores its own data
7. List/delete operations — registry stays consistent
8. Target directory clearing — protected dirs (.git/) preserved
9. Size reporting — positive byte count
10. System state capture — restored when capture_system_state=True
11. Mid-evolution interruption recovery — checkpoint, corrupt workspace, restore → original state

Verification

• ruff format --check . ✅
• ruff check evoseal/ tests/ ✅
• pytest tests/unit/core/test_checkpoint_save_restore.py -q → 12 passed ✅

TODO.md

Checks off: "Add test for checkpoint save/restore"
Tracking table updated: P2 20→21, Total 60→61

Summary by CodeRabbit

• Tests
  • Strengthened checkpoint integrity validation by adding a dedicated tamper-restore scenario to confirm tampered checkpoints are rejected during restore.
  • Improved negative-case assertions by relaxing brittle error-message matching while still verifying integrity-related failures.
  • Enhanced checkpoint size reporting checks using deterministic byte expectations and tighter bounds.
• Documentation
  • Updated checkpoint save/restore testing coverage to reflect a larger, more comprehensive suite (13 unit tests) and refreshed progress counts (P2 done: 21; total done: 61).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 74ab61be-d6f0-4929-bfb1-ed68646158d3

📥 Commits

Reviewing files that changed from the base of the PR and between a552ef2 and 3bd47ba.

📒 Files selected for processing (1)

• tests/unit/core/test_checkpoint_save_restore.py

---

📝 Walkthrough

Walkthrough

Expands checkpoint save/restore coverage to 13 tests, including tamper detection during restore, filesystem deletion checks, size bounds, protected-directory restoration, system-state assertions, and interruption recovery. Updates TODO tracking counts for the completed test.

Changes

Checkpoint coverage

Layer / File(s)	Summary
Checkpoint test foundation tests/unit/core/test_checkpoint_save_restore.py	Defines isolated directory fixtures, CheckpointManager setup, and the sample checkpoint payload used by the test suite.
Round-trip and integrity validation tests/unit/core/test_checkpoint_save_restore.py	Tests restoration, metadata preservation, integrity verification, tamper rejection, missing checkpoints, and isolation between multiple checkpoints.
Lifecycle and recovery validation tests/unit/core/test_checkpoint_save_restore.py, TODO.md	Tests listing, deletion from disk, size bounds, protected .git handling, system-state capture, and interruption recovery; updates coverage and completion counts.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding checkpoint save/restore round-trip tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/checkpoint-save-restore

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/core/test_checkpoint_save_restore.py`:
- Around line 146-150: Update test_delete_checkpoint to capture the path
returned by create_checkpoint, then assert that path no longer exists after
delete_checkpoint("del") succeeds, while retaining the existing registry
assertion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cefa8155-72b9-4468-b7b9-1285146049a9

📥 Commits

Reviewing files that changed from the base of the PR and between 85e92c2 and fc015fd.

📒 Files selected for processing (2)

• TODO.md
• tests/unit/core/test_checkpoint_save_restore.py

[SHA888]

Addressed review feedback — here's what changed and what I verified against the implementation:

1. Hardcoded checkpoint layout in test_integrity_hash_fails_when_file_tampered ✅ Fixed

• Replaced checkpoint_dir / "checkpoint_v4" / "src" / "main.py" with Path(manager.get_checkpoint_path("v4")) / "src" / "main.py".
• The test no longer depends on the internal checkpoint_{version_id} naming convention.

2. Brittle match="not found" in test_restore_nonexistent_raises ✅ Fixed

• Removed the substring match — now just asserts pytest.raises(CheckpointError).
• The exception type is the contract; the message text is an implementation detail.

3. Missing return-value check in test_restore_clears_target_except_protected ✅ Fixed

• Now captures the result dict and asserts result["success"] is True before checking directory state.

4. CodeRabbit inline comment: assert disk deletion in test_delete_checkpoint ✅ Applied

• Captures the path from create_checkpoint() and asserts it no longer exists after delete_checkpoint().

5. Caveat about CheckpointManager implementation — Not actionable

• I verified the actual implementation at evoseal/core/checkpoint_manager.py: the config key is checkpoint_directory, restore_checkpoint raises CheckpointError with "not found" for missing versions, and .git/ is in the protected_dirs set. All assumptions in the tests match the real code.

[SHA888]

Addressed review feedback:

Point 1 — "Tests assume behaviors not visible in the diff": Verified against the actual CheckpointManager implementation (evoseal/core/checkpoint_manager.py). All three behaviors are correct:

• get_checkpoint_path returns None after delete_checkpoint — confirmed: delete_checkpoint does del self.checkpoints[version_id] + shutil.rmtree, and get_checkpoint_path checks both the dict and disk.
• CheckpointError("Integrity verification failed ...") is raised in restore_checkpoint when verify_checkpoint_integrity returns False.
• system_state["system_info"] and ["capture_timestamp"] are both set in _capture_system_state.

No change needed — the reviewer couldn't confirm from the diff alone, but the implementation matches.

Point 2 — hardcoded size < 10_000 upper bound: Valid. Replaced the arbitrary constant with a content-relative bound (expected_content_bytes * 100) so the test adapts if metadata format grows rather than flaking on a magic number. Committed as f4132a9.

[SHA888]

Addressed review feedback (commit 3bd47ba):

1. test_checkpoint_size_reported — size upper bound too loose ✅ Fixed

• Reduced multiplier from 100x → 30x (measured actual overhead: ~24x for 43 B of content → 1050 B total).
• Reviewer suggested 10-20x, but that is too tight for the real implementation (20x = 860 < 1050 actual). 30x (= 1290 B ceiling) catches meaningful bloat without flaking.
• Added the observed ratio to the comment for future reference.

2. test_restore_rejects_tampered_checkpoint — hardcoded error message match ✅ Fixed

• Shortened match="Integrity verification failed" → match="Integrity". The test still validates the correct failure mode but survives harmless wording changes (e.g. "Integrity check failed").

3. Caveat about CheckpointManager API assumptions — Not actionable

• Already verified against the actual implementation in a prior round. All method signatures and result shapes match.