[M3gA-Mind]

Summary

Test only. cleanup erases a legacy tree wholesale only when its export lists nothing as incomplete. Otherwise it forgets the moved items one by one. No test pinned that guard.

The new test uses a legacy engine whose export reports one partly stored item, and whose erase panics. Cleanup must finish (Phase::Cleaned) through per-id forgets, without ever calling erase.

Related issue

None. Found in an edge-case audit (a coverage gap; the behaviour is correct).

API or behavior changes

None.

Validation

Commands actually run, with their outcome:

• N/A: cargo fmt --all -- --check. rustfmt --check (edition 2024) passed on the touched file. CI runs the full check.
• N/A: cargo clippy --all-targets --all-features -- -D warnings. Left to CI.
• N/A: cargo build --all-targets --all-features. Left to CI.
• cargo test -p tinymemory-tools --lib a_partly_stored_legacy_item_blocks_the_scope_wide_erase:
  • break-check, with the guard reduced to let mut can_erase = !shared; (the !found.partial term dropped): FAILED, panicking at the erase call. The file was restored by copy.
  • unchanged code: 1 passed.

Tests

New a_partly_stored_legacy_item_blocks_the_scope_wide_erase, plus the PartlyStored wrapper engine it needs.

Documentation

Not needed (test only).

Checklist

• The change is focused on one logical change
• No new #[allow(...)], #[ignore], or relaxed lints
• No secrets, tokens, or .env contents in the diff or the description

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

Or wait 57 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 735df4e0-f04a-4b10-8778-8e38f70a0f66

📥 Commits

Reviewing files that changed from the base of the PR and between 4f5145c and aecb071.

📒 Files selected for processing: 1

• crates/tinymemory-tools/src/layout_migration/cleanup_tests.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[tinysweeper]

Tiny Sweeper review

Reviewed head: aecb07104d9fd8255e5c3e846c2a07655007858d

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 1 of 1 files.
Pull request purpose (title): test(layout_migratio…

Diagnostics and supporting details

Full summary

Pull request purpose (title): test(layout_migration): pin that a partly stored legacy item blocks the scope-wide erase. Test-only change that pins the cleanup guard deciding between a wholesale legacy erase and per-id forgets. Per the PR description, `cleanup` erases a legacy tree wholesale only when its export lists nothing as incomplete; otherwise it forgets moved items one by one. The PR adds a wrapper engine whose export reports one partly stored item and whose `erase` panics, plus a test asserting cleanup finishes with `Phase::Cleaned` without ever calling `erase`. No production code changed per the description and diffstat (1 file, 78 additions).

Review snapshot

Change surface	Files	Review signal	Count
Production	0	Active findings	0
Tests	1	Noted findings	0
Documentation	0	Resolved findings	0
Configuration	0	Pending checks/questions	0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

Adds a test-only `PartlyStored` wrapper around `ReferenceEngine` in `cleanup_tests.rs` that delegates the engine methods but overrides `export` to push a synthetic `incomplete` id and overrides `erase` to panic. Adds test `a_partly_stored_legacy_item_blocks_the_scope_wide_erase`, which stores five facts into the wrapped legacy engine, runs `cleanup`, and asserts `state.phase == Phase::Cleaned` and that the legacy tree's texts are empty, demonstrating moved items were forgotten by id rather than erased scope-wide. No production behavior or API changes.

Features

• Added — PartlyStored wrapper engine reporting an incomplete export item with a panicking erase: Provides test scaffolding that forces cleanup down the per-id forget path and makes any scope-wide erase of a partly stored tree fail loudly instead of silently succeeding. (crates/tinymemory-tools/src/layout_migration/cleanup_tests.rs)
• Added — a_partly_stored_legacy_item_blocks_the_scope_wide_erase: Pins the guard in `cleanup` that suppresses scope-wide erase when the export lists an incomplete item; per the description, the test fails if the `!found.partial` term is dropped from `can_erase`. (crates/tinymemory-tools/src/layout_migration/cleanup_tests.rs async fn after_an_unconfirmed_import_moved_items_are_forgotten_by_id_never_erase, crates/tinymemory-tools/src/layout_migration/cleanup_tests.rs)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The critique lane answered clear, finding no defect in the changed lines and describing the test as asserting forget-by-id and `Phase::Cleaned` without calling `erase`.
• Lane summary: The new test wraps the reference engine so every export reports a partly stored item, then asserts `cleanup` forgets by id and reaches `Phase::Cleaned` without calling `erase`. I found no defect in the changed lines; the only thing I could not verify is the full `MemoryEngine` trait surface, so I could not confirm every required method is forwarded by the wrapper. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

security

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The security lane answered clear, describing the wrapper as self-contained test scaffolding with no untrusted input, no new I/O, and no rule violation.
• Lane summary: The new test-only `PartlyStored` wrapper in `cleanup_tests.rs` injects a synthetic `incomplete` id so the survey takes the forget-by-id path and its `erase` panics if that contract breaks; it is self-contained test scaffolding with no untrusted input, no new I/O, and no rule violation. Nothing reportable. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

tests

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The tests lane answered clear, noting the panic on `erase` fires if the `!found.partial` guard regresses and that the change follows the `<module>_tests.rs` convention and descriptive-naming rule.
• Lane summary: This adds a test-only change: a `PartlyStored` engine wrapper whose `export` reports an incomplete chunk and whose `erase` panics, plus `a_partly_stored_legacy_item_blocks_the_scope_wide_erase`, which asserts that cleanup reaches `Phase::Cleaned` and empties the legacy tree without calling `erase`. It is a genuine behavioural assertion (the panic on `erase` fires if the `!found.partial` guard at cleanup.rs:165 regresses), it follows the repository's `<module>_tests.rs` convention and descriptive-naming rule, and no production code changed. Nothing to fix. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

commits

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The commits lane answered clear, reporting no sensitive content in committed files.
• Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

description

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The description lane answered clear, finding the change matches the title and body exactly and is fully accounted for.
• Lane summary: The change adds a cleanup test plus a `PartlyStored` engine wrapper whose `export` reports an incomplete item and whose `erase` panics; it matches the title and body exactly, follows the repository's test-file and test-naming rules, and its assertions hold against the `can_erase = !found.partial && !shared` guard in `remove_moved`. Nothing in the diff is incorrect or unmentioned. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

Evidence and run details

• Models: ladder/vectors-oai3, deepseek-v4.1-flash
• Spend: $0.000582
• Tokens: 228428 input · 4527 output · 62464 cached · 184 embedding
• Updated: 2026-10-22T23:43:16Z

Head	State	Pass summary
aecb07104d9f	ready for maintainer review	0 active finding(s), 0 resolved finding(s) (at 2026-10-22T23:43:16Z)

[tinysweeper]

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0006