registry: close 9 boot.rs/lifecycle.rs test gaps, fix a masked mutation (#564) - #576
Conversation
…on (#564) boot.rs: interleaving a context rename with its containing group's own rename in one crash-resume boot (neither marker kind's existing tests plant both at once); reconcile_groups's write_group failure arm (warn-only, memory correct but the file stays stale); preload_pinned's multi-worker path (every existing pinned test uses a single context, so workers==1 always) alongside its per-worker Err warn arm. hydrate.rs: the boot-time hydrator manifest registration loop's name_from_stem None arm, for a manifest stem that fails to decode. lifecycle.rs: a pinned rename's re-preload failure (warn-only, stays cold rather than failing the rename); the boot straggler contract (landed && !complete) with a group actually needing its membership rewritten, isolating it from the one existing incidental case (no group present); update_meta's dice_floor/semantic_floor clamps (every call site already passes in-range values, so the clamp itself was never exercised, and the PATCH handler has no other guard); update_meta's pinned ensure_hot failure rollback; hidden_label's Err arm. Also: cargo mutants flagged one real gap in sweep_stale_stem_files (lifecycle.rs) — a destination-targeting marker whose removal fails for a real reason (not NotFound) was silently swallowed rather than propagated, with no test distinguishing the two. Added a directly-targeted test using the persistence fault injector. Separately, boot.rs's passage-vector ANN heads-up log condition had no observable behavior for mutants to catch (log-only); extracted it into its own #[mutants::skip]'d helper rather than skip all of boot_with. Verified: cargo mutants --profile=mutants --file <module> --jobs 4 on both files, 0 missed after the sweep-fix and the extraction. Refs #564
📝 WalkthroughWalkthroughHydrator登録、ブート時のリネーム復旧とプリロード、ライフサイクルのロールバック、スキーマ解決に関する回帰テストを追加しました。ANN休止判定を専用ヘルパーへ抽出しました。 Changesレジストリ復旧とライフサイクル
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/registry/lifecycle.rs (1)
1655-1662: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
resident_estimateのロールバックも検証してください。このテストは
pinnedとloadedを確認しますが、説明にある budget のresident_estimateを確認しません。rollback_meta後のrecount_entryが欠けても、このアサーションは成功します。失敗前後の resident estimate を比較してください。🤖 Prompt for 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. In `@src/registry/lifecycle.rs` around lines 1655 - 1662, Extend the failed forced-preload test around rollback_meta and recount_entry to capture the entry’s resident_estimate before the operation and assert afterward that it is unchanged. Keep the existing pinned and loaded assertions, ensuring the test detects missing resident-estimate rollback.
🤖 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.
Nitpick comments:
In `@src/registry/lifecycle.rs`:
- Around line 1655-1662: Extend the failed forced-preload test around
rollback_meta and recount_entry to capture the entry’s resident_estimate before
the operation and assert afterward that it is unchanged. Keep the existing
pinned and loaded assertions, ensuring the test detects missing
resident-estimate rollback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5670a358-eebf-476f-9695-3dc22e091e21
📒 Files selected for processing (3)
src/hydrate.rssrc/registry/boot.rssrc/registry/lifecycle.rs
Summary
PR-B of 3 for #564. Covers
src/registry/boot.rs,src/registry/lifecycle.rs(+ one fixture-driven test insrc/hydrate.rs). PR-A (concurrency.rs + gauges.rs, #575) is the first; PR-C (engine.rs + registry.rs) will close #564.boot.rs
reconcile_groups'swrite_groupfailure arm (warn-only: memory is reconciled, the on-disk file stays stale until the next successful write). The two existing boot-time persistence faults consume their single-shot injector earlier in the same boot, never reaching this call.preload_pinned's multi-worker path + its per-workerErrwarn arm — every existing pinned test in the suite boots with exactly one pinned context, soworkers == 1always.hydrate.rs
name_from_stemNonearm, for a manifest stem that fails to percent-decode.lifecycle.rs
landed && !complete) with a group that actually needs its membership rewritten — the one existing incidental case has no group present, so the rewrite loop runs vacuously.update_meta'sdice_floor/semantic_floorclamps — every call site in the suite already passes an in-range value, so the clamp itself was never exercised; it's also the only guard left on the PATCH path (the create handler clamps up front, PATCH does not).update_meta's pinnedensure_hot-failure rollback (meta.pinnedmust roll back tofalse, not strand a pinned-but-unloadable context).hidden_label'sErrarm — never called from any test in the suite.Also fixed (found by the mutation sweep, in scope of #564's mandate to verify with
cargo mutants)sweep_stale_stem_files's destination-targeting-marker removal loop silently swallowed a real (non-NotFound) removal failure — mutants flagged!=→==as undetected. Added a test that isolates the exact op via the persistence fault injector.#[mutants::skip]'d helper (matching the existingremove_persisted_file_quietlyprecedent) rather than skip mutation coverage on all ofboot_with.Verified against #532's original claims; #564's own comment lists which of that audit's items are already closed by earlier PRs (#568/#569/#572/#573/#574) and out of scope here.
Test plan
cargo fmtcargo clippy --all-targets— warnings ゼロcargo test— full suite passescargo mutants --profile=mutants --file src/registry/boot.rs --jobs 4— 0 missed (after extracting the log-only condition)cargo mutants --profile=mutants --file src/registry/lifecycle.rs --jobs 4— 0 missed (after thesweep_stale_stem_filesfix)cargo mutants --in-diff) — 1 in-diff mutant, unviable (doesn't build), effectively 0 missedRefs #564
Summary by CodeRabbit
バグ修正
テスト