audit: close the second-pass findings — replica refresh debt, migration float equality, and five sibling asymmetries - #523
Merged
Merged
Conversation
float equality, and five sibling asymmetries Findings from the focused second-pass review (issue #522), each with a test that kills its mutants: - replica: the tailer now carries a pending_refresh debt across polls — a stem whose hydration a per-request loader completed after the tailer's own failed attempt never turns stale again, yet its in-memory meta (pinned, description, revision bookkeeping) was never re-read; the remembered debt pays out on the next poll instead of freezing the meta until an unrelated manifest change. - replication: replica_refresh re-stats both WAL gauges — on a replica the bytes arrive as tailed file copies, never through the writer's live accounting, so a cold unpinned context understated taguru_wal_bytes indefinitely. - context/image: the pre-v5 migration's sourceless-call detection compares the two summation orders under a first-order rounding bound (extracted as summation_gap_is_real, boundary-pinned) instead of exact f64 equality — regrouping noise no longer credits a phantom attribution that retract_source can never remove. - context/consolidation: contradiction-group ObjectRow.sources drops zero-sum records, the same posture sign_conflicts already takes — a source that cancelled its own assertion no longer reads as attesting evidence to the judge. - api/consolidation: `checks` passes the same overlong() ceiling every sibling list input enforces (dedup folds only consecutive repeats, so an alternating list dodged both selector guards at any length the body cap admits). - extract: --source-id trims its emptiness check like --tag's. - mcp/schema: search_passages/explain_search declare `minimum: 0` on since/until like every sibling tool. Closes #522 Claude-Session: https://claude.ai/code/session_011NozdDS9JqgCpi9Z3wo4Pd
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough統合処理、旧形式属性移行、入力検証、WAL統計、レプリカ更新の7件の監査指摘を修正しました。各変更に回帰テストを追加しました。 Changes監査指摘の修正
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/context/image.rs`:
- Around line 757-759: Update summation_gap_is_real and its migration callers to
conservatively account for cancellation among records from the same source,
rather than relying only on final LegacyAttributionRecord.weight-derived
magnitude. Avoid classifying an unexplainable gap as a sourceless call when the
legacy image lacks source-level addition history, and add a regression test
covering A:+1e16, B:+1.0, A:-1e16 followed by retracting A and B without leaving
a phantom count.
In `@src/registry/replication.rs`:
- Around line 114-119: Update the WAL size assignments for inner.wal_bytes and
inner.passages_wal_bytes to distinguish metadata outcomes: set the gauge to zero
only for NotFound, use the file length on success, and preserve the existing
gauge value for all other I/O or permission errors. Avoid unwrap_or(0) for both
wal_path and passages_wal_path.
In `@src/replica.rs`:
- Around line 319-329: Move the `report.stale` insertion into
`self.pending_refresh` in `retarget()` so it occurs before the fallible
`self.hydrator.hydrate_shared().await?` call, preserving the refresh debt when
hydration fails. Keep the existing worklist construction based on
`pending_refresh`, and add a regression test covering the `hydrate_shared()`
failure path and confirming `replica_refresh` is eventually performed.
🪄 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: e9c98faa-5f95-4b41-a512-b27a9c156dcc
📒 Files selected for processing (9)
src/api/consolidation.rssrc/context/consolidation.rssrc/context/image.rssrc/extract/args.rssrc/extract/tests.rssrc/mcp/schema.rssrc/registry/replication.rssrc/replica.rstests/http_api/consolidation.rs
zero WAL gauges only on NotFound CodeRabbit on #523: - replica: report.stale joins pending_refresh BEFORE hydrate_shared can fail the poll — a debt recorded only after the shared pass was lost with the error, reopening the same reader-hydrates-first freeze one step earlier. Regression test tears the published meta.json object so the shared pass itself aborts the poll. - replication: the WAL gauge re-stat zeroes only on NotFound (no WAL shipped yet); any other stat failure keeps the last-known value instead of walking a live gauge to nothing on a transient error. Claude-Session: https://claude.ai/code/session_011NozdDS9JqgCpi9Z3wo4Pd
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
2周目の重点監査(#522)で見つかった7件の修正。各修正にミュータントを殺すテスト付き。
pending_refresh(refresh債務)をポーリング横断で持たせた。tailerのhydration失敗後にクライアント読みが先にstemを完了させると、retargetは二度とstaleと報告せずreplica_refreshが永続スキップされ、メタデータ(pinned/description/revision)だけ凍結していた — 債務は次のポーリングで必ず支払われるreplica_refreshが両WALゲージを再stat。replicaではバイトがtailerのファイルコピーで届くため、cold・unpinnedなコンテキストのtaguru_wal_bytesが無期限に過小報告だったsummation_gap_is_realとして切り出し、境界値を厳密にピン)に変更 — 加算順序の再結合ノイズがretract_sourceで消せないphantom attributionを生まなくなるObjectRow.sourcesがzero-sumレコードを除外(sign_conflictsと同じ姿勢) — 自己相殺したソースがLLM判定の証拠として並ばないchecksに兄弟リスト入力と同じoverlong()上限(dedupは連続重複しか畳まないため交互リストが素通りだった)--source-idの空チェックを--tag同様trim付きにsearch_passages/explain_searchのsince/untilに兄弟ツール同様minimum: 01周目(#520)からの継続で、tombstoneフェンス契約・ship世代フェンシング・hydrator状態機械・route mapホットリロード・breakerは網羅追跡の結果、指摘ゼロでした。
Closes #522
Test plan
cargo fmt/cargo clippy --all-targets(警告ゼロ)cargo testフルスイート 3219 件パス(失敗 0)— replica競合はフォールト注入テスト(torn-segment方式)で再現・検証--iterate再検証で 35/35 caught、missed 0https://claude.ai/code/session_011NozdDS9JqgCpi9Z3wo4Pd
Summary by CodeRabbit
バグ修正
入力検証
--source-idを無効として扱うよう変更しました。