refactor(database): resolve_writable_collection delegates, and its comments match reality - #2385
cyberlife-coder wants to merge 1 commit into
Conversation
|
All contributors have signed the CLA. Thank you! |
3bad750 to
f262b60
Compare
|
Everything else on the current head ( Generated by Claude Code |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…mments match reality resolve_writable_collection and resolve_collection had byte-identical bodies (vector, then graph, then metadata registries), but three call-site comments described a behavioral difference that never existed: ddl_executor's TRUNCATE fallback claimed the writable resolver skips metadata collections and chained an .or_else that could never actually run, and training.rs attributed the "no vectors on a metadata collection" guard to the wrong function (it actually lives in extract_training_vectors's is_empty filter). Make resolve_writable_collection delegate to resolve_collection, drop the now-provably-dead .or_else fallback, and correct the three comments to say what the code does today.
f262b60 to
781d8a0
Compare
|
Still waiting on your CLA-sign comment; everything else should re-check green against Generated by Claude Code |
This branch has not been deployed
1 participant
refactor/*→ targetsdevelop. ✅Description
resolve_writable_collectionandresolve_collection(both incrates/velesdb-core/src/database/query_engine.rs) had byte-identicalbodies — check the vector registry, then graph, then metadata — but
three call sites carried comments claiming a behavioral difference
that never existed:
ddl_executor.rs'sexecute_truncatechained.resolve_writable_collection(...).or_else(|_| self.resolve_collection(...))with a comment saying the writable resolver "skips" metadata
collections. Since the two resolvers are identical, that fallback
branch is provably dead code — it can never fire.
training.rs'sresolve_train_collectionattributed the "novectors on a metadata-only collection" guard to
resolve_writable_collectionitself. That guard actually lives inextract_training_vectors, which filters out points with an emptyvector before training — not in collection resolution.
This is the AGENTS.md-flagged pattern of documentation describing
enforcement the code doesn't actually perform (docs must describe
what the code enforces, not what it aspires to).
Type of Change
What changed
resolve_writable_collectionnow delegates toresolve_collectioninstead of repeating its body, with an accurate doc comment
explaining the two names exist for call-site intent (write path vs.
read path), not a difference in what gets resolved.
.or_elsefallback inexecute_truncateand itsstale comment.
resolve_train_collection's doc comment to point at theactual location of the "no vectors" guard.
No behavior change: same resolution order, same error on an unknown
collection, same TRUNCATE and TRAIN semantics.
How Has This Been Tested?
cargo check -p velesdb-core --features persistencecargo clippy -p velesdb-core --all-targets --features persistence,gpu,update-check,openapi,test-fault-injection -- -D warnings -D clippy::pedanticcargo clippy -p velesdb-core --lib --bins --features persistence,gpu,update-check -- -A warnings -D clippy::undocumented_unsafe_blockscargo test -p velesdb-core --features persistence --lib database:: -- --test-threads=1(207 passed)cargo test -p velesdb-core --features persistence --lib database::ddl_executor_tests:: -- --test-threads=1(39 passed, covers TRUNCATE)cargo test -p velesdb-core --features persistence --lib velesql::train_tests:: -- --test-threads=1(10 passed)cargo fmt --all -- --check(clean)Test Configuration:
rust-toolchain.toml(MSRV 1.90)Checklist
Additional Notes
Not a high-risk change: doesn't touch
hnsw,storage,Drop,unsafe, or SIMD dispatch paths. Scoped tocrates/velesdb-core/src/database/only.