test(cli): normalize F32 digits before the cross-version export comparison - #664
Merged
azimafroozeh merged 2 commits intoSep 6, 2026
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
What & why
Follow-up to #627, which turned the post-merge
V5 ↔ V6 Format Fencejob red onmain.assert_export_fidelitycompared the predecessor binary's export of theml-introembedding raw against the current binary's re-export: the v5 binary widens everyF32cell to 64-bit digits (0.10000000149011612), the writer from RFC 0051 prints the cell at its own width (0.1).canonical_export_rowsalready ran both sides throughnormalize_f32_and_nulls; the fidelity check now does the same before comparing, so it tests vector values, not digit spelling.Backing issue / RFC
docs/rfcs/0051-json-output-via-arrow.md) invalidated.Checklist
crossversion_upgrade.rsnormalized through the existing helper)Local verification
cargo test --locked -p omnigraph-cli --test crossversion_upgrade current_v6_refuses_and_rebuilds_genuine_v5_and_v5_refuses_v6 -- --exact --nocapturewithOMNIGRAPH_V5_BINpointing at anomnigraphbuilt from the pinned final-v5 commit46b6d908(the job's own invocation) — without this change it fails with the job's exact left/right (0.1vs0.10000000149011612); with it,1 passedcargo clippy --locked -p omnigraph-cli --all-targets -- -D warnings -W clippy::dbg_macro— cleancargo fmt --all --check— cleanOMNIGRAPH_OLD_BIN,OMNIGRAPH_PREVIOUS_BIN) — not run: no predecessor binaries built; same helper, same wideningNotes for reviewers
v5_v6_format_fenceruns only whengithub.event_name != 'pull_request', so this PR's CI will not exercise the fence either; the local run above is the evidence, and the first CI run is the push tomain.