model: enforce shard size at open (round-robin layout) - #55
Merged
Conversation
A short or long shard previously loaded silently: bank_open checks nothing about size, and the round-robin reader would serve a neighbor record's bytes as the expert asked for. Bound each shard against the file: shard s must hold floor(n/k) records plus one for the first n%k shards, exactly what split_banks.py writes. Found by negative control while re-verifying parity on 0.7.0: long/short shards both previously returned rc=0 with correct- looking output on untouched experts.
marcobambini
added a commit
that referenced
this pull request
Aug 28, 2026
#55's check is right and I should have written it. waste_file_size has been in platform.h with a POSIX and a Windows implementation the whole time; I looked for a portable size helper before f8833c8, grepped for fstat/st_size, missed the name, and built a probe read to get part of what a direct call gives. The size check is cheaper and strictly stronger for the claim it makes, and the two are kept because neither subsumes the other: size is the whole-file claim and needs no read, the probe is the readability claim and a file can be exactly the right length and still not be readable the way the engine reads it. The justification is corrected. The branch said a short shard "would serve a neighbor record's bytes as the expert asked for". It cannot: every record carries the expert it belongs to and record_check reads it on the way past. Two shards padded to the sizes N=2 expects but carrying a 3-way split's records refuse on the first read -- "expert 7 of layer 1: record header is not what the bank index describes" -- and produce nothing. A misplaced record is a refusal, never a substitution. What the check actually buys is *when*, and that is worth having on its own. A long shard loaded happily and generated correct output before it, since the extra records are simply never read; a short one waited for the router to reach the missing expert, which on K3 can be thousands of tokens in. Both are a refusal at load now. The negative controls, on this branch: a correct 2-way split rc=0 a shard one record too long rc=1 (was 0) a shard one record too short rc=1 a 3-way split mounted as 2 rc=1 right sizes, another split's records rc=1 (record_check) Synthetic 66 passed / 0 failed / 10 skipped, K3 70 / 0 / 6. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
marcobambini
added a commit
that referenced
this pull request
Aug 28, 2026
Enforce shard size at open, companion to #53. Rebased onto the O_DIRECT probe and complementary to it: size is the whole-file claim and needs no read, so it also covers containers whose record size is not a whole number of blocks, where the probe skips; the probe is the readability claim. Landed with the justification corrected to what a negative control shows. A misplaced record cannot be served as another expert's -- record_check reads the expert id out of every record header and refuses. What the size check buys is when: a long shard loaded and generated correct output before it, and a short one waited for the router to reach the missing expert.
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
2 participants
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.
Companion to #53. A shard's size was never checked at open:
bank_openvalidatesthe record geometry, but nothing bound the file against the layout
split_banks.pywrites. Under round-robin placement (
bank_fetch: shard = e % n_sh) a short shardlets the reader serve a neighbor record's bytes as the expert asked for, and a long
one hides the same confusion one record over. Both cases previously returned rc=0
with correct-looking output on untouched experts.
Found by negative control while re-verifying parity on 0.7.0, not by a failing test.
This rebases onto
f8833c8, which added the O_DIRECT probe read at the same site.The two checks are complementary and both are kept:
read, so it also covers containers whose record size is not a whole number of
blocks (where the probe deliberately skips).
still not be readable the way the engine will read it.
Neither subsumes the other, so the size check runs first and the probe is unchanged
in behavior.
recsis now computed once and shared: theceil((n_experts - s) / n_sh)form from the probe is equivalent to thefloor(n/k) + (s < n%k)form the original commit used, so this is asimplification, not a semantic change. The size check is gated on
n_sh > 1because a single unsharded bank is bounded by the manifest already.
Gate:
makeclean (no new warnings) andtests/run.sh= 55 passed, 0 failed,13 skipped on macOS arm64. The skips are all missing-asset (K3 container,
tokenizer, vision source), unchanged from main.