feat: validate the distance type in the model header (closes #23) - #24
Open
Dani-giron wants to merge 1 commit into
Open
feat: validate the distance type in the model header (closes #23)#24Dani-giron wants to merge 1 commit into
Dani-giron wants to merge 1 commit into
Conversation
3 tasks
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
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
dump()/load()validated M, M0, EF and HEURISTIC but not the distance type. A model built with one distance loaded silently into a type using another: bincode does not depend onD(distance structs are zero-sized, no trace in the serialized body), so the graph deserialized fine and every later search computed distances with the wrong function, with no error. Verified empirically: with the check disabled, loading aNormalDistancemodel into anInvertedDistancetype returnsOkwith the full graph recovered.Changes
src/datalayer/algorithms.rs:DistanceAlgorithmgains a stableconst TYPE_NAME: &'static str, declared by both implementations (NormalDistance,TlshDistance).src/controllers/apotheosis.rs:dump()writes an FNV-1a hash ofD::TYPE_NAMEas 4 extra header bytes (17 -> 21).load()recomputes it for the loading type and rejects a mismatch with a descriptive error. FNV-1a instead oftype_name()/std hashers because it is fully specified and stable across Rust versions, which a persistence format needs.tests/header_distance.rs(new): mismatch is rejected, matching distance loads fine.Record type
Ris not validated here, intentionally out of scope (see issue #23):Rtypes usually have non-zero-sized fields, so a mismatch there is more likely to surface through bincode on its own.Test plan
cargo test --test header_distance: 2/2 pass.load()returnedOkwith the graph fully populated under a mismatched distance, confirming the silent-acceptance behavior before writing the fix.cargo clippy/cargo fmton the full crate could not be verified locally (unrelated toolchain issue on this machine, disk near full); the two changed files were checked individually withrustfmt --checkand pass. Will be validated by CI on this PR.Closes #23