Files
Ryan Codrai 9f2c990be1 feat: convert index files between v5, v6 and v7 in any direction (#536)
* feat: convert index files between v5, v6 and v7 in any direction

The rest of the crate reads and writes v7 only, so bringing an older
file forward needed somewhere for the retired codecs to live. This is
that place, and it goes both ways: any of the three versions in, any of
the three out, for .tv and .tvim.

read() decodes into a version-neutral Image, write() re-encodes it,
convert_file() does both through a temp file and an atomic rename, and
version_of() reports a file's version without decoding it. An example
binary exposes the same from a shell.

Converting is a re-container, not a re-quantize: codes, scales,
calibration and ids cross untouched, which the tests assert directly
(the packed codes must come back byte-identical through all nine version
pairs). v7 output goes through the shipping writer, so it is
byte-identical to what this build would write.

v5 gains a writer it never had — it was read-only in every shipped build
— derived from the reader's layout. Verified the strongest way
available: files this converter writes are loaded by a genuine pre-#535
build's v5/v6 readers, and the search results match the v7 original
exactly, for both .tv and .tvim.

Refused rather than fudged: a lazy index cannot go to v5 or v6, neither
of which can express 'no dimension committed', and pre-v5 files remain
undecodable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: name precisely why v1-v4 cannot be converted

The old message lumped versions 1 through 4 together as 'predates the
v5 rotation change'. The record is more specific, and the distinction
matters to anyone holding such a file.

v1 (turbovec <= 0.4.3) was already refused by the build that introduced
v2 — it has never been decodable by a shipped reader.

v2-v4 are decodable in principle, but only under the pre-v5 rotation: a
QR of a seeded Gaussian, built through a BLAS this crate no longer
depends on, which differed by about one ulp across CPU architectures and
thread counts. That non-reproducibility is why v5 replaced it and why v4
carries a rotation fingerprint at all. Converting one forward is not a
re-container like v5<->v6<->v7: it would mean dequantizing,
inverse-rotating under a rotation this build cannot reliably reproduce,
re-rotating and re-quantizing — lossy, and not guaranteed to be the
rotation that wrote the file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: the lazy sentinel is not a v7 invention, and bound the row count

Two review findings on the converter, one of which was me being wrong.

I claimed v5 and v6 had no way to express "no dimension committed" and
refused to write one. They do: validate_header_fields on the previous
release accepts dim == 0 alongside n_vectors == 0, read_v5_header is
shared by both v5 and v6 readers, and that release's own test suite pins
the round trip. So lazily-saved files exist in the wild, and the
converter both refused to read them and refused to produce one — the
opposite of what a converter is for. It now carries the sentinel in
every direction, writing a zeroed codebook for v6 exactly as that
release did.

n_vectors came off the legacy header as an unvalidated u64 and fed every
size calculation: the blocked-length product, packed_row * n_vectors,
rd_f32s's n * 4, and a Vec::with_capacity. Large values wrap, so the
slice bounds checks pass on an empty payload and the allocation is still
reached. Bounded against the file first — a row costs at least five
bytes, so a file this small cannot describe that many rows whatever its
header says. dim is now capped at MAX_DIM on this path too.

Three tests: the sentinel converts in all nine directions and still
loads lazy through the real v7 loader; dim 0 claiming rows is refused by
every version; and a header claiming u64::MAX rows is refused rather
than multiplied out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: point the refusal at the converter, now that it exists

The base PR deliberately worded the docs and legacy_format_error for
what it shipped alone — no converter — and said the follow-up would
update them. This is that follow-up, so a v5 or v6 file is now told it
converts forward rather than that it has to be re-saved by an older
release. Versions 1 through 4 are unchanged: nothing can decode them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: validate a hand-built Image, and finish the sentinel reversal

Four review findings.

convert::write checked scales and ids but not the geometry. The v7 arm
inherits from_parts' validation; the two legacy arms had none, and Image
is public with public fields and no #[non_exhaustive], so a hand-built
one reaches them directly — a short code buffer would be written into a
file no reader can make sense of. Validate bit_width, dim and the packed
length once, before dispatch, for all three versions. The sentinel check
that was inside write_legacy moves there too, so it now covers v7 as
well. Tested per version: valid writes, a short code buffer, bit_width
7, dim 65, and an absurd dim.

Two documentation sites still described the refusal that the later
lazy-sentinel commit reversed within this same PR: Image::dim's doc
("only expressible in v6 and v7") and the CHANGELOG paragraph listing
the sentinel as something a downward conversion loses. Both now say what
the code does — all three versions express it, and the tests loop every
direction.

And the id_map doc comment moved back onto from_v7_load. Adding
from_index_and_ids above it re-orphaned the paragraph describing the
`path` parameter, which from_index_and_ids does not take. This is the
same slip that was fixed in #535; introducing the function here brought
it back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: validate the TQ+ pair, and cover calibration and ids together

The review found the one geometry field write() did not check, and the
mutation gate found three gaps. They turn out to be the same blind spot:
nothing exercised calibration and ids in the same file.

The legacy writers trust the TQ+ pair twice — n_calib comes from the
shift array's length while both arrays are emitted — so a pair of
unequal length writes a header saying "uncalibrated" followed by dim
stray floats. Those land where the id table starts and come back as ids,
with no error at any stage: the reader's `n_calib != dim` guard passes
vacuously at zero. v7 caught it through from_parts; v5 and v6 wrote it
happily. write() now requires the two arrays to match each other and to
be 0 or dim, for every version.

The missed mutants, each now killed by the test written for it, verified
by applying the mutation:

- `at += n_calib * 4` -> `-=` in read_legacy. The TQ+ trailer and the id
  table are adjacent, so an offset error in one reads the other — but
  the id-mapped matrix was uncalibrated and the calibration matrix had
  no ids, so neither could see it. A calibrated id-mapped index now
  converts in all nine directions, checking ids, both calibration
  arrays, and the codes.
- `bytes.len() < 5` -> `== 5` in detect. Under 5 bytes the slice of the
  first four would panic rather than error; the junk fixture was 8 bytes.
  Now every length from 0 to 4.
- `dim > MAX_DIM` -> `>=` in write. The largest legal index could not be
  written at all. Same boundary as the loader's, now pinned on both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 11:25:19 +01:00
..