registry: refresh seven stale doc claims in the core engine (#565) - #578
Conversation
Doc/comment staleness the #532 audit flagged in #565, re-verified against current HEAD. Fixes: wrong file for extract_chunks_concurrently's caller and embed_stale/ refresh_passage_embeddings' location, a hardcoded export.rs line reference that moved, lock_unless_deleted's "every post-lookup lock" claim (false — a sibling helper and 15 direct-access sites exist), "up to eight unlinks" (now unbounded across fixed family files plus stale markers), SemaphorePermit's "held across exactly the provider call" (false — held from before the circuit-breaker check), CueCache's "~12 MB" (dimension-dependent, not a fixed number), and two undocumented dispatcher behaviors (uncaught panics, and an undifferentiated deadline/provider-error channel). Also fixes enforce_budget's doc, found stale during this same pass but not in the original 12: it describes eviction unconditionally reaching the budget, which a saturated single-context overage cannot do. Line-number references are dropped in favor of symbol names — the issue's own hardcoded references had already drifted by the time of this fix, which is the failure mode being corrected.
|
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:
📝 WalkthroughWalkthroughregistry コアエンジンのドキュメントコメントを、ロック、並行処理、予算処理、キャッシュ容量、ディスク処理の現在の実装に合わせて更新しました。実装コードと公開宣言は変更していません。 Changesregistry コアドキュメント更新
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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/engine.rs`:
- Around line 594-599: Update the documentation comment for enforce_budget to
remove the claim that except itself may fail to save; describe instead that
targeted dirty candidates can remain resident when their saves fail, while
preserving the existing saturation and later-call behavior.
In `@src/registry/lifecycle.rs`:
- Around line 36-41: Update the lifecycle documentation around
sweep_stale_stem_files to describe unlink counts only for paths that function
actually removes, excluding meta_path because save_files overwrites it. Keep the
references to stale rename/import marker cleanup and save_files fsyncs accurate.
🪄 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: 1b4237b2-4ee4-4c79-aa3a-7f65026595e5
📒 Files selected for processing (4)
src/registry.rssrc/registry/concurrency.rssrc/registry/engine.rssrc/registry/lifecycle.rs
- enforce_budget's doc claimed except's own save could fail, but the sweep skips except from eviction entirely (name == except continues past evict_entry) — the doc now attributes save failures to other dirty candidates the sweep actually targets. - create's doc phrase "an unlink per file in the stem's on-disk family" read as including meta_path, which sweep_stale_stem_files never touches (save_files overwrites it instead) — narrowed to the candidate paths the sweep actually removes.
Summary
doc/comment のみの修正。#532 の監査が起票した #565(registry コアエンジンのコメント/doc陳腐化 12件)を現行 HEAD で再検証し、現存する7件を修正した。既に #561〜#564 のフォローアップ(特に #562 の PR #572/#573)で解消済みだった5件は再検証結果を issue コメントに記録済み。加えて、再検証中に issue 本文に無い同種の陳腐化を1件発見し(
enforce_budgetの doc)、同じ性質の修正なので合わせて含めた。コード変更なし。挙動は一切変わらない。
修正した7件+1件
concurrency.rs:dispatch_chunks_concurrentlyの呼び出し元所在(src/extract.rs→Run::extract_chunks_concurrentlyinsrc/extract/run.rs; "below" →src/registry/embeddings.rs)registry.rs:PutSchemaError::ReservedAliasのEMPTY_SOURCE参照先(export.rs:315→export::renderの予約 source id 拒否)registry.rs:Entry::lock_unless_deletedの不変条件を「全post-lookupロックが通る」から、実際の4パターン(inline Slot match / tombstone を立てる当事者 / 呼び出し元が排他所有 / boot 時)へ書き直し。兄弟のread_unless_deletedの存在にも言及lifecycle.rs:AppState::createの "up to eight unlinks" を、固定本数+件数無制限のマーカー unlink という実態に合わせて数を持たない表現にconcurrency.rs:SemaphorePermit/Semaphoreの doc — permit はサーキットブレーカの拒否チェックより前に取得され、ブレーカ open の拒否も slot を短時間消費することを明記registry.rs:CueCache::CAPの "~12 MB" が3072次元前提であることを明記し、次元は provider レスポンス依存であることを追記concurrency.rs: ディスパッチャ doc に、f内 panic が捕捉されないこと(thread::scopeが join 後に unwind を再送)、およびErr(String)チャネルが deadline失効/slotタイムアウト/プロバイダエラーを区別できないことを追記engine.rs:enforce_budgetの doc に、except単独で予算超過し evict 不能な飽和状態では resident estimate が budget に収まらないことを追記(issue 本文には無いが、再検証中に発見した同種の陳腐化)再検証の結果、今回は対象外にした5件(#565 コメント参照)
51d5da6)で解消済みtruncate_walの既定値依存 — registry: キャッシュ予算/クォータ/ディスク会計のずれ 11 件 #562(c1ecb1c)で解消済み036eaf2)で解消済みapi/contexts.rsの mid-rename — 誤指摘(コメント自身が既に明示的に除外している)Test plan
cargo fmtcargo clippy --all-targets --all-features— 警告ゼロRUSTDOCFLAGS="-D rustdoc::broken_intra_doc_links" cargo doc --no-deps --document-private-items— 新規に追加した intra-doc リンク(Entry::read_unless_deleted,crate::export::render)は解決を確認。本 diff と無関係な既存の壊れたリンク(context/write.rs,deadline.rs等)が main にも存在することはgit stashで確認済みcargo test— フルスイート通過(500 passed、doc-only なので挙動不変の確認)cargo mutants --in-diff)— doc-only 差分のため対象ミュータント 0件("No mutants to filter")Closes #565
Summary by CodeRabbit