docs(roadmap): "our reader can open it" is not "the library can load it" - #87
Merged
Merged
Conversation
The support table reports `0 OUR READER cannot open`, and that zero is a METADATA fact. Every count in that block is scoped to the TOKENIZER, which lives in the KV header ahead of any tensor. A reader who takes the zero as "all thirty files are usable" is wrong about three of them, and nothing beside it said so. Measured 2026-09-09 by `tests/gguf_serving_census.rs`, which landed in #85 and is cited here rather than an open PR: SmolLM2-135M-Instruct-IQ3_XS unknown dtype for tensor 21 SmolLM2-135M-Instruct-IQ4_XS unknown dtype for tensor 23 SmolLM2-135M-Instruct-Q2_K unknown dtype for tensor 20 Their metadata reads normally -- which is exactly what #73-#76 fixed and why they left that group -- but `ParallelModelManager::load_gguf` cannot build a model from them. The tensor indices DIFFER and `Q2_K` is a K-quant rather than an IQ type, so these files MIX quantizations and one tensor in each uses a type candle and fuel both reject: a PER-TENSOR dtype limit, not a whole-file one.⚠️ AND I CORRECTED A WORD OF MY OWN FROM #84. That PR wrote that #73-#76 "recovered the three IQ-dtype SmolLM2 files". It recovered their METADATA. The sentence is now scoped, with a note saying why it was not: the word was written while the question in front of me was the tokenizer, and it reads, months later, as though those files became usable. A TERM THAT WAS UNAMBIGUOUS IN ITS OWN PARAGRAPH ACQUIRES A SECOND MEANING AS SOON AS A NEIGHBOURING CLAIM RANGES OVER SOMETHING ELSE. Nothing edited that sentence and nothing could have flagged it; what changed is that a second population now sits beside it.⚠️ AND A DOCS-ONLY CHANGE IS NOT AUTOMATICALLY OUTSIDE THE GATES HERE. `tests/roadmap_panic_claims.rs` READS ROADMAP.md and asserts no document claims a live panic macro at a path where none is live. Checked rather than assumed before deciding how much to run: the full ten-step chain is green and that test executed 6 tests against the edited file, non-vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JtTf3315ahKPPyBiYugnPh
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @ciresnave-bot, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 days and 14 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Reviewer's GuideUpdates ROADMAP.md to distinguish tokenizer metadata readability from full tensor/model loadability, documents three measured per-tensor dtype failures, and narrows historical “recovered” wording to metadata recovery. Flow diagram for GGUF metadata and tensor loadabilityflowchart LR
File[GGUF file] --> Header[Tokenizer metadata in KV header]
Header --> Reader[OUR READER can open]
File --> Tensors[Tensor directory]
Tensors --> DtypeCheck[Per-tensor dtype validation]
DtypeCheck -->|supported dtypes| Model[Library can load model]
DtypeCheck -->|unknown dtype| Reject[load_gguf rejects file]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Up to standards ✅🟢 Issues
|
ciresnave
pushed a commit
that referenced
this pull request
Sep 10, 2026
…ndex (#88) * fix(gguf): the number in that refusal is a DTYPE CODE, not a tensor index⚠️ RETRACTION. Two merged PRs and ROADMAP carry a claim of mine that is wrong. candle-core 0.10.2, src/quantized/mod.rs:295: _ => crate::bail!("unknown dtype for tensor {u}"), `{u}` IS THE DTYPE BEING LOOKED UP. Every English reading of "unknown dtype for tensor 21" says "tensor #21", and that is the reading I took. WHAT I PUBLISHED, in #85's body, #87's body and ROADMAP: "each failing with unknown dtype for tensor N at a DIFFERENT tensor index -- so it is a per-tensor dtype limit, not a whole-file format one", and "Q2_K is a K-quant rather than an IQ type, which means these files MIX quantizations, and one tensor in each uses a type candle and fuel both reject." MEASURED -- dtype histograms from our own parser, which reads codes without validating them: file dtype codes (code:count) unsupported candle names IQ3_XS 0:61 8:1 20:180 21:30 {20, 21} 21 IQ4_XS 0:61 8:1 20:180 23:30 {20, 23} 23 Q2_K 0:61 8:1 11:30 20:180 {20} 20 Q4_0 CONTROL 0:61 2:207 3:3 8:1 {} LOADS The reported number is a member of each file's unsupported set, three for three, and the control's set is empty and it loads. FOUR THINGS WRONG IN ONE SENTENCE: 1. "different tensor indices" -- they are dtype codes. 2. "one tensor in each" -- it is 180 to 210 of 272. Code 20 alone is 180 in all three. 3.⚠️ "these files MIX quantizations" -- TRUE AND VACUOUS, which is worse than false. The Q4_0 control mixes four dtypes and loads perfectly. MIXING IS UNIVERSAL IN GGUF AND IS NOT THE DISCRIMINATOR. A property every file in the corpus has was offered as the explanation for why three of them fail. A DISCRIMINATOR MUST BE ABSENT FROM THE CASES THAT SUCCEED. 4.⚠️ "Q2_K is a K-quant" READ A QUANTISATION OFF A FILENAME. That file contains NO Q2_K tensors -- code 10 is absent entirely. Found by the mlmf lane and confirmed in the histogram above. So the sentence had TWO broken inputs and I had only found one. WHAT DID NOT SAVE ME, because it is the useful part: I had a control, I stated my uncertainty about Q2_K, and I labelled the mixing claim as an inference. None of it helped. A CONTROL VALIDATES THE INSTRUMENT AND CANNOT TELL YOU THAT YOU MISREAD THE INSTRUMENT'S OUTPUT -- the error was in the INPUT to the inference, where no control was looking. FOUR CHANGES, and only the last prevents recurrence: 1. ROADMAP's table and the paragraph built on it. 2. ROADMAP's other instance of the string, at the "an error reading as a fact about the file" argument: READ, and NOT AFFECTED -- that argument does not depend on what the number denotes. Annotated rather than edited, noting the same string caught its own author a second way. Swept for the STRING, not the argument. 3. OUR WRAPPER MESSAGE, which repeated candle's wording verbatim and so propagated the trap downstream. It now carries the correction IMMEDIATELY after the quoted string, because a clarification at the end of a paragraph arrives after the reader has formed the wrong picture.⚠️ The upstream string is kept VERBATIM deliberately: it is what a reader pastes into a search box, and a rewritten copy is one we must keep in sync -- when it drifts, our "helpful" version becomes a second wrong message with nothing to catch it.⚠️ AND THE ACCEPTED SET IS NOT INLINED, on the mlmf lane's objection, which I accept: it is an unverifiable claim about a third-party library shipped inside a diagnostic, and it would fail in the FLATTERING direction -- if candle widens, our message calls a file unsupported when it is not, on a path that is already failing so nobody complains. The set lives in the test, where drift is caught. 4. tests/gguf_dtype_census.rs -- the histogram, with a denominator. THE TEST'S ASSERTIONS, each able to fail: - a file candle ACCEPTS must carry no unsupported dtype - a file candle REFUSES must carry at least one -⚠️ the number in the refusal must be a MEMBER of that file's unsupported set -- the claim this file exists for, and one the "tensor index" reading gives no reason to expect - BOTH ARMS non-empty, so it cannot pass having compared nothing - every file lands in exactly one bucket, so a future `continue` cannot silently shrink the population BORN-RED FOR THE DRIFT CLAIM, since keeping the accepted set in the test is only defensible if the test notices when it goes stale. Narrowing our constant to simulate candle widening: tinyllama-1.1b-chat-v1.0.Q4_0.gguf was ACCEPTED by candle while carrying dtypes it does not list: [2] CORROBORATION, AT TRUE STRENGTH RATHER THAN ITS MOST FLATTERING FORM. The mlmf lane hit this independently through a different reader within ten minutes, same files, same numbers. Our METHODS are independent; our ARTIFACT is not -- they confirmed the same corpus root and the same sha256. So: "the number is a type code" INDEPENDENT, and it rests on SOURCE: it would survive a fabricated corpus "these files contain these dtypes" ONE ARTIFACT READ TWICE Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JtTf3315ahKPPyBiYugnPh * refactor(gguf): split the dtype census so no function exceeds the LOC limit Codacy: "Method the_refused_number_is_a_dtype_code_and_not_a_tensor_index has 92 lines of code (limit is 50)", tests/gguf_dtype_census.rs#L117.⚠️ THE CHECK-RUN SUMMARY AND THE ANNOTATION DESCRIBE DIFFERENT FINDINGS. The summary said "Complexity: 1 medium. Metric Complexity: 15"; the ANNOTATION says 92 lines of code against a limit of 50. Those are different metrics, and the annotation is the one with a file, a line and a number in it. Read the annotation, never the summary line and never a remembered threshold. VERIFIED BEFORE FIXING, because a tool that was wrong once is not wrong always and a tool that is usually right is not right now: 117 physical lines, 6 blank, 19 comment-only, 92 CODE. Codacy is exactly right.⚠️ AND THE AGREEMENT IS NOT THE EVIDENCE. My count uses the same brace-matching family Codacy's does, so an exact match is the shared-method trap on its flattering side. WHAT SURVIVES A SHARED METHOD IS THE MARGIN: 92 exceeds 50 by FORTY-TWO LINES, and no plausible counting convention closes that gap. Split into `row_for` (what a file IS), `Census::of` (the tally), `report`, and the test body (the claims): the_refused_number_is_a_dtype_code_and_not_a_tensor_index 29 row_for 47 Census::of 13 Census::report 16⚠️ `row_for` has THREE LINES OF HEADROOM and its doc comment now says so, with the instruction that the next addition goes in a new function rather than on the end of this one. A margin that only exists in a gate is one the next author discovers by going red.⚠️ AND I CAUGHT A FALSE STATEMENT IN MY OWN DOC COMMENT WHILE WRITING IT. The first draft said "the per-file ASSERTIONS live in `Census::of`, deliberately" -- they live in `row_for`. Corrected to say where they are AND why: both facts are in hand there, and moving them would mean carrying candle's message through the `Row` type purely so the check could happen somewhere else. CONTROLS, both re-run after the refactor rather than assumed to survive it: - the census reproduces exactly: 7 accepted / 3 refused / 1 no-histogram - the drift guard still fires on its named arm -- tinyllama-1.1b-chat-v1.0.Q4_0.gguf was ACCEPTED by candle while carrying dtypes it does not list: [2] Eleven-step chain green, exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JtTf3315ahKPPyBiYugnPh --------- Co-authored-by: Lightbulb Agent <ciresnave+lightbulb@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The support table reports
0 OUR READER cannot open, and that zero is a metadata fact. Every count in that block is scoped to the tokenizer, which lives in the KV header ahead of any tensor. A reader who takes the zero as "all thirty files are usable" is wrong about three of them, and nothing beside it said so.Measured
By
tests/gguf_serving_census.rs, which landed in #85 — cited here as a test onmainrather than an open PR, which is why this was queued behind it:Their metadata reads normally — which is exactly what #73–#76 fixed, and why they left that group — but
ParallelModelManager::load_ggufcannot build a model from them. The tensor indices differ, andQ2_Kis a K-quant rather than an IQ type, so these files mix quantizations and one tensor in each uses a type candle and fuel both reject: a per-tensor dtype limit, not a whole-file format one.That PR wrote that #73–#76 "recovered the three IQ-dtype SmolLM2 files". It recovered their metadata. The sentence is now scoped, with a note saying why it wasn't: the word was written while the question in front of me was the tokenizer, and it reads, months later, as though those files became usable.
A term that was unambiguous in its own paragraph acquires a second meaning as soon as a neighbouring claim ranges over something else. Nothing edited that sentence and nothing could have flagged it — what changed is that a second population now sits beside it.
tests/roadmap_panic_claims.rsreadsROADMAP.mdand asserts that no document claims a live panic macro at a path where none is live. I checked that rather than assuming a Markdown edit was inert, and it decided how much to run.Full ten-step chain green at exit 0 — fmt, lockfile, doc, test, featgate, clippy, spm, sweep, e2e, census — and that test executed 6 tests against the edited file, non-vacuously.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JtTf3315ahKPPyBiYugnPh
Summary by Sourcery
Clarify GGUF support claims so metadata readability is not mistaken for complete library loadability.
Enhancements:
Documentation: