Skip to content

test: cover compact_passages_if_worthwhile's four untested branches - #594

Merged
t0k0sh1 merged 5 commits into
mainfrom
587-compact-passages-untested
Aug 12, 2026
Merged

test: cover compact_passages_if_worthwhile's four untested branches#594
t0k0sh1 merged 5 commits into
mainfrom
587-compact-passages-untested

Conversation

@t0k0sh1

@t0k0sh1 t0k0sh1 commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • compact_passages_if_worthwhile (src/registry/context_io.rs, the sync-only compaction helper taguru-code calls, issue taguru-code: passage WALがコンパクションされずディスクを食い続ける #452) had zero tests anywhere in the repository despite four early-return branches: replica, unknown context, deleted context, and zero passage-log watermark.
  • Adds direct coverage for all four early returns plus the one path where compaction actually runs (a log past the floor folds into the snapshot and truncates).
  • The mutation gate found two missed mutants on the watermark guard (store.watermark() > 0): the whole guard forced to true, and > replaced with >=. Both are excluded in .cargo/mutants.toml as equivalent — watermark(): u64 can never be negative, so >= 0 is a tautology identical to true, and PassageStore::load never produces watermark() == 0 together with nonzero log/snapshot bytes, so compact_if_log_outgrew_snapshot's own condition is unconditionally false there regardless of this guard. No test can observe a difference because there is none.
  • CodeRabbit review: two findings addressed. The deleted-context test called state.delete(), which removes the entry entirely and only exercises lookup()'s None branch, not the intended read_unless_deleted()Slot::Deleted branch — fixed by flipping the slot directly on a still-registered entry (matching the existing run_maintenance_compaction_skips_a_deleted_entry_without_panicking pattern). The compaction test only checked that the WAL emptied, which an implementation that truncates the log before the snapshot lands would also pass while losing the write — fixed by restarting AppState from disk and dereferencing the passage back through lookup_passages.

This closes out issue #587 (untested-branches audit from #533) together with #595 and #596. A CI mutants-sweep dispatched across all 5 audited files (context_io.rs, wal.rs, storage.rs, wal_replay.rs, meta_io.rs, 227 mutants total) confirms wal_replay.rs (covered by #586's earlier work) and meta_io.rs (covered by #585) already have zero missed mutants — no further PR needed for those.

Closes #587.

Test plan

  • cargo fmt
  • cargo clippy --bin taguru --tests (clean)
  • cargo test (full suite green)
  • Pre-PR mutation gate (cargo mutants --in-diff): 0 mutants (test-only diff)
  • cargo mutants --file src/registry/context_io.rs, run in isolation: 53 tested, 45 caught, 8 unviable, 0 missed
  • Cross-checked against a CI mutants-sweep dispatched on main across all 5 audited files — findings match exactly, no gaps

https://claude.ai/code/session_013NPb5cDU8ojZAKGJaq8rYy

Issue #587 (#533 persistence audit): the sync-only compaction helper
had zero tests in the repository despite four early-return branches
(replica, unknown context, deleted context, zero watermark) plus the
one path where it actually runs. Add direct coverage for each.

Refs #587
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f2d06553-0ef6-469e-b295-a735764d1828

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

compact_passages_if_worthwhile のテストを追加した。圧縮しない条件と、ログが指定フロアを超えた場合に圧縮して WAL を空にする条件を検証する。

Changes

コンテキスト圧縮テスト

Layer / File(s) Summary
圧縮条件と非圧縮条件の検証
src/registry/context_io.rs
テスト用の plain を追加した。レプリカ、未知、削除済み、passage 履歴なしのコンテキストでは、圧縮せずにストアとログを変更しないことを検証する。ログが指定フロアを超えた場合は、圧縮して WAL を空にすることを検証する。

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

Possibly related issues

  • t0k0sh1/taguru issue 587 — compact_passages_if_worthwhile の同じ分岐とウォーターマーク条件のテストを扱う。

Possibly related PRs

  • t0k0sh1/taguru#446 — コンテキスト圧縮を実行する条件と実行しない条件のテストを追加している。
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルはcompact_passages_if_worthwhileの未テスト分岐をテストでカバーする主な変更を明確に示しています。
✨ 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 587-compact-passages-untested

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@src/registry/context_io.rs`:
- Around line 904-910: Update
compact_passages_if_worthwhile_declines_a_deleted_context so the test retains
the registry entry and sets its slot to Slot::Deleted, following the existing
setup pattern around the referenced Slot::Deleted usage, rather than calling
state.delete(). Keep the assertion focused on exercising the
read_unless_deleted() deleted-entry branch.
- Around line 957-965: 拡張対象は compact_passages_if_worthwhile のテストで、WAL
が空になった確認だけでなく再起動後の復元も検証してください。state を drop して永続化状態から再作成し、lookup_passages で a.md
の passage 内容が期待値どおり復元されることをアサートしてください。
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 128f2529-7725-4245-adef-6bc10d2b8af1

📥 Commits

Reviewing files that changed from the base of the PR and between 4ba0960 and e39ed1e.

📒 Files selected for processing (1)
  • src/registry/context_io.rs

Comment thread src/registry/context_io.rs
Comment thread src/registry/context_io.rs
The pre-PR mutation gate found one missed mutant: the watermark guard
(store.watermark() > 0) replaced with true. PassageStore::load never
produces watermark() == 0 together with nonzero log_bytes or
snapshot_bytes — a fresh or healed-clean store always has all three
at zero, and compact_if_log_outgrew_snapshot's own condition
(log_bytes > floor_bytes.max(ratio * snapshot_bytes)) is then false
for every floor_bytes regardless of this guard. No test can kill it
because there is no reachable state where it would matter.

Refs #587
… guard

The CI sweep (dispatched on main before this branch's tests existed)
found a second mutant on compact_passages_if_worthwhile's watermark
guard that the earlier local --file run had already excluded past:
`>` replaced with `>=` rather than the whole guard replaced with
`true`. Same equivalence reasoning applies — watermark(): u64 can
never be negative, so `watermark() >= 0` is a tautology, identical to
`true` for every possible value.

Refs #587
- compact_passages_if_worthwhile_declines_a_deleted_context called
  state.delete(), which removes the entry from the registry entirely —
  the test exercised lookup()'s None branch, not the intended
  read_unless_deleted() -> Slot::Deleted branch. Flip the slot
  directly on a still-registered entry instead, matching the pattern
  run_maintenance_compaction_skips_a_deleted_entry_without_panicking
  already uses in this file.
- compact_passages_if_worthwhile_compacts_past_the_floor only checked
  that the WAL file emptied, which an implementation that truncates
  the log before the snapshot lands would also satisfy while losing
  the write. Restart AppState from disk and dereference the passage
  back through lookup_passages to prove it actually survived.

Refs #587
@t0k0sh1
t0k0sh1 merged commit 1d7b0c8 into main Aug 12, 2026
15 checks passed
@t0k0sh1
t0k0sh1 deleted the 587-compact-passages-untested branch August 12, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

永続化/WAL 層の未検証分岐

1 participant