Repository navigation
linearize: the negative control had only one judge in it - #44
Merged
Merged
Conversation
#40 asks for this before a linearizing writer exists, and the suite had done half of it. TestAnOrdinaryFileIsRefusedByBothChecks feeds this package's own writer output to Linearized and CheckLinearized and asserts both refuse it — but "both" there means the two checks in this file, which were written together, by me, from the same reading of Annex F. Two judges that have only ever agreed prove nothing about either. ⛔ qpdf WAS THE ONE THAT HAD NOT BEEN ASKED. It validates the hint tables, which this package explicitly does not, so it is the judge that can disagree. It now sees the same ordinary file and says `File is not linearized`. ⛔⛔ AND ITS EXIT STATUS IS 0 FOR A FILE IT HAS JUST CALLED NOT LINEARIZED. Measured on qpdf 12.4.2: $ qpdf --check plain.pdf ; echo $? checking plain.pdf PDF Version: 1.7 File is not encrypted File is not linearized No syntax or stream encoding errors found; ... 0 0 for "not linearized", 3 for a file it merely warns about, 2 only when it cannot read one at all. So the status cannot carry this verdict in either direction and the error is discarded on purpose, with the reason written where somebody will edit it. ⛔ THE PAIR IS WHAT MAKES IT A TEST. On its own, "File is not linearized" could be a line qpdf prints about every file, and the assertion would hold against a judge that judges nothing. So qpdf's OWN linearized output is checked for the absence of that phrase in the same test. ⛔ AND THE CONTROL HAS A CONTROL. If qpdf failed to open the file, it would print no verdict about linearization, and "the phrase is absent" would read exactly like "the file is linearized". Measured on a 16-byte non-PDF: qpdf prints four warnings, never `PDF Version:`, and exits 2. So both halves first require `PDF Version:` in the output — proof it read the file — and t.Fatal otherwise. This is a control, not a gate on new behaviour: nothing in linearized.go changes, and the row it serves stays MISSING until something writes one. gofmt clean, go vet clean, 0 uncovered blocks of 1806. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tannevaled
added a commit
that referenced
this pull request
Nov 14, 2026
⛔⛔ SECOND OCCURRENCE IN TWO DAYS, AND I PREDICTED THIS ONE IN WRITING. The
first push of the qpdf lane failed on
go: …/cmd/aptneed@latest: fleettools@v0.112.0 requires go >= 1.27.4
(running go 1.27.3; GOTOOLCHAIN=local)
go-pdfkit/ops hit the identical line on 2026-11-13, and after fixing it I
wrote on that pull request: "Worth knowing for other repositories in the fleet
that pin a toolchain and install fleettools commands in CI: the same pin
blocks them the moment they try." Then I walked into it myself, one repository
later, the next day. A note is applied when somebody remembers to apply it,
and at the moment of adding an install step one is thinking about the install.
So it is recorded as a recurrence rather than a third note. The gate that
catches it is a check over `.github/workflows` for a `<module>/cmd/<x>@latest`
install beside a `go-version` pin below that module's `go` directive —
already filed as pending on go-fleettools/fleettools#103, where the `@latest`
half of the same problem lives. `tagscan` cannot see either: it reads
repositories, not workflows.
All three jobs move to 1.27.4, not only the one that installs aptneed. The
other two do not need it, but a matrix measuring on two different toolchains
is a difference nobody chose, and this fleet has already been bitten by
`stable` quietly selecting a minor.
⛔ GOTOOLCHAIN IS STILL LEFT ALONE. That is what made this a clean failure
naming both versions instead of a runner silently fetching 1.27.4 and
measuring a chain nobody picked.
go.mod stays at `go 1.27.1`: that is the language version this module needs,
not the toolchain a runner must have, and nothing here requires it to move.
actionlint reports only the pre-existing SC2086 discussed in the previous
commit.
Refs #40, #44.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tannevaled
added a commit
that referenced
this pull request
Nov 14, 2026
…d so (#45) * ci: the foreign judge had never run in CI, and nothing could have said so ⛔⛔ EVERY TEST THAT AGREES WITH qpdf HAS BEEN SKIPPING ON EVERY LANE. Measured on run 43595391186: the string "qpdf" appears ZERO times in the whole log. ⛔ AND THE TIME IS THE WITNESS, which is the only thing that could have been noticed: this job reports ok github.com/go-pdfkit/reader 0.197s (CI) ok github.com/go-pdfkit/reader 0.758s (a workstation with qpdf) The sweep generates 42 files by invoking qpdf; it cannot do that in 0.197s. So the agreement across 1, 2, 3, 5, 25 and 100 pages in five shapes, AND the negative control that an ordinary file is REFUSED by the foreign judge — the one #44 added yesterday — have never been checked by anything. ⛔ `go test` WITHOUT -v PRINTS NOTHING ABOUT A SKIP, so the log was not hiding this badly, it could not report it at all. And coverage stayed at 100.0% throughout: the same statements are reached by the tests that need no qpdf, so the coverage gate cannot see this and never could. Two gates were green about a third thing neither measures. ⛔ THIS REPOSITORY ALREADY HOLDS THE OPPOSITE STANDARD, one job down, about a missing qemu emulator: "A missing emulator FAILS this lane. A lane that reports success for an architecture it never ran is the one outcome this matrix exists to rule out." A missing foreign judge is that outcome for a different reason. go-pdfkit/ops already does it right for pdftoppm. ⛔ REQUIRE_QPDF, NOT CI, because not every lane CAN run it and a rule that demands the impossible gets deleted. Under qemu-user the test binary is emulated and an execve of the host's x86-64 qpdf is not something the emulator can carry out; the Windows lane has no apt. So ONE lane installs qpdf and sets REQUIRE_QPDF, and there a missing judge FAILS. The others skip, which is now a stated design instead of an accident nobody could see. Measured, both directions, rather than predicted: REQUIRE_QPDF=1, qpdf off PATH -> FAIL, naming the reason unset, qpdf off PATH -> skip, exit 0 ⛔ aptneed FOR THE INSTALL, because a mirror outage must not arrive as a missing judge: go-pdfkit/ops#90 was four red runs in half an hour on a third party's DNS. `--need qpdf` asks for the BINARY, since apt saying yes is not qpdf being on PATH. And `qpdf --version` is printed, so which judge agreed is answerable afterwards rather than assumed. ⚠ NOT FIXED, and deliberately: actionlint reports SC2086 on `go test $ex ./...` in the qemu job. It is pre-existing — main carries it at line 141 — and shellcheck is WRONG here. `ex` is empty on the 386 lane, which runs natively on purpose, so quoting it would hand `go test` an empty package pattern and break the one lane that must not be emulated. Reader's CI never ran actionlint, which is why it went unnoticed; silencing it properly is its own change. gofmt clean, go vet clean, 0 uncovered blocks of 1806. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: Go 1.27.4, because installing a fleet tool requires it ⛔⛔ SECOND OCCURRENCE IN TWO DAYS, AND I PREDICTED THIS ONE IN WRITING. The first push of the qpdf lane failed on go: …/cmd/aptneed@latest: fleettools@v0.112.0 requires go >= 1.27.4 (running go 1.27.3; GOTOOLCHAIN=local) go-pdfkit/ops hit the identical line on 2026-11-13, and after fixing it I wrote on that pull request: "Worth knowing for other repositories in the fleet that pin a toolchain and install fleettools commands in CI: the same pin blocks them the moment they try." Then I walked into it myself, one repository later, the next day. A note is applied when somebody remembers to apply it, and at the moment of adding an install step one is thinking about the install. So it is recorded as a recurrence rather than a third note. The gate that catches it is a check over `.github/workflows` for a `<module>/cmd/<x>@latest` install beside a `go-version` pin below that module's `go` directive — already filed as pending on go-fleettools/fleettools#103, where the `@latest` half of the same problem lives. `tagscan` cannot see either: it reads repositories, not workflows. All three jobs move to 1.27.4, not only the one that installs aptneed. The other two do not need it, but a matrix measuring on two different toolchains is a difference nobody chose, and this fleet has already been bitten by `stable` quietly selecting a minor. ⛔ GOTOOLCHAIN IS STILL LEFT ALONE. That is what made this a clean failure naming both versions instead of a runner silently fetching 1.27.4 and measuring a chain nobody picked. go.mod stays at `go 1.27.1`: that is the language version this module needs, not the toolchain a runner must have, and nothing here requires it to move. actionlint reports only the pre-existing SC2086 discussed in the previous commit. Refs #40, #44. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
tannevaled
added a commit
that referenced
this pull request
Nov 14, 2026
…ed inside /H (#46) * ci: the foreign judge had never run in CI, and nothing could have said so ⛔⛔ EVERY TEST THAT AGREES WITH qpdf HAS BEEN SKIPPING ON EVERY LANE. Measured on run 43595391186: the string "qpdf" appears ZERO times in the whole log. ⛔ AND THE TIME IS THE WITNESS, which is the only thing that could have been noticed: this job reports ok github.com/go-pdfkit/reader 0.197s (CI) ok github.com/go-pdfkit/reader 0.758s (a workstation with qpdf) The sweep generates 42 files by invoking qpdf; it cannot do that in 0.197s. So the agreement across 1, 2, 3, 5, 25 and 100 pages in five shapes, AND the negative control that an ordinary file is REFUSED by the foreign judge — the one #44 added yesterday — have never been checked by anything. ⛔ `go test` WITHOUT -v PRINTS NOTHING ABOUT A SKIP, so the log was not hiding this badly, it could not report it at all. And coverage stayed at 100.0% throughout: the same statements are reached by the tests that need no qpdf, so the coverage gate cannot see this and never could. Two gates were green about a third thing neither measures. ⛔ THIS REPOSITORY ALREADY HOLDS THE OPPOSITE STANDARD, one job down, about a missing qemu emulator: "A missing emulator FAILS this lane. A lane that reports success for an architecture it never ran is the one outcome this matrix exists to rule out." A missing foreign judge is that outcome for a different reason. go-pdfkit/ops already does it right for pdftoppm. ⛔ REQUIRE_QPDF, NOT CI, because not every lane CAN run it and a rule that demands the impossible gets deleted. Under qemu-user the test binary is emulated and an execve of the host's x86-64 qpdf is not something the emulator can carry out; the Windows lane has no apt. So ONE lane installs qpdf and sets REQUIRE_QPDF, and there a missing judge FAILS. The others skip, which is now a stated design instead of an accident nobody could see. Measured, both directions, rather than predicted: REQUIRE_QPDF=1, qpdf off PATH -> FAIL, naming the reason unset, qpdf off PATH -> skip, exit 0 ⛔ aptneed FOR THE INSTALL, because a mirror outage must not arrive as a missing judge: go-pdfkit/ops#90 was four red runs in half an hour on a third party's DNS. `--need qpdf` asks for the BINARY, since apt saying yes is not qpdf being on PATH. And `qpdf --version` is printed, so which judge agreed is answerable afterwards rather than assumed. ⚠ NOT FIXED, and deliberately: actionlint reports SC2086 on `go test $ex ./...` in the qemu job. It is pre-existing — main carries it at line 141 — and shellcheck is WRONG here. `ex` is empty on the 386 lane, which runs natively on purpose, so quoting it would hand `go test` an empty package pattern and break the one lane that must not be emulated. Reader's CI never ran actionlint, which is why it went unnoticed; silencing it properly is its own change. gofmt clean, go vet clean, 0 uncovered blocks of 1806. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: Go 1.27.4, because installing a fleet tool requires it ⛔⛔ SECOND OCCURRENCE IN TWO DAYS, AND I PREDICTED THIS ONE IN WRITING. The first push of the qpdf lane failed on go: …/cmd/aptneed@latest: fleettools@v0.112.0 requires go >= 1.27.4 (running go 1.27.3; GOTOOLCHAIN=local) go-pdfkit/ops hit the identical line on 2026-11-13, and after fixing it I wrote on that pull request: "Worth knowing for other repositories in the fleet that pin a toolchain and install fleettools commands in CI: the same pin blocks them the moment they try." Then I walked into it myself, one repository later, the next day. A note is applied when somebody remembers to apply it, and at the moment of adding an install step one is thinking about the install. So it is recorded as a recurrence rather than a third note. The gate that catches it is a check over `.github/workflows` for a `<module>/cmd/<x>@latest` install beside a `go-version` pin below that module's `go` directive — already filed as pending on go-fleettools/fleettools#103, where the `@latest` half of the same problem lives. `tagscan` cannot see either: it reads repositories, not workflows. All three jobs move to 1.27.4, not only the one that installs aptneed. The other two do not need it, but a matrix measuring on two different toolchains is a difference nobody chose, and this fleet has already been bitten by `stable` quietly selecting a minor. ⛔ GOTOOLCHAIN IS STILL LEFT ALONE. That is what made this a clean failure naming both versions instead of a runner silently fetching 1.27.4 and measuring a chain nobody picked. go.mod stays at `go 1.27.1`: that is the language version this module needs, not the toolchain a runner must have, and nothing here requires it to move. actionlint reports only the pre-existing SC2086 discussed in the previous commit. Refs #40, #44. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * linearize: the shared object hint table is required, and nothing looked inside /H First result of reading Annex F itself, which #40 listed under "what has NOT been read yet". ⛔⛔ /S IS REQUIRED AND A WRITER THAT FORGOT IT PASSED EVERYTHING. Table F.2 lists the shared object hint table as "(Required)", and F.3.6 says the primary hint stream's dictionary "shall contain entries giving the position of the beginning of each hint table in the stream". Every check this file had about /H was about the stream's EXTENT — it starts at a real indirect object, that object is a stream, its length matches what the parse measured. Its dictionary was never read. So a primary hint stream naming no hint table at all was sound by this checker, and a reader that fetched the front of the file would have nothing to look up. This is the first thing checked INSIDE a hint stream rather than about it. Everything else is an offset or a length; /S is content. ⛔ ONLY THE PRIMARY, AND THE SPEC IS EXPLICIT ABOUT WHY. F.3.6: "the dictionary of the overflow hint stream shall not contain these entries". The same requirement in checkOverflowHint would refuse exactly the conforming file. The two halves of /H are not symmetric and this is where they differ — which is also why no test asserts the absence of the check there: with the fixture's overflow pair pointing at the primary stream, adding it would break nothing, so it would be a claim no mutation could kill. ⛔ PRESENCE, NOT POSITION. The page offset table "shall be the first table in the stream and shall start at offset 0" and has no key at all, so the positions inside the stream are F.4's question and this does not open it. Measured on both judges: - all three committed fixtures carry /S — the two qpdf shapes AND the Ghostscript one, so the positive control spans two producers; - a same-width edit of `/S 43` to `/X 43` is refused here by name; - `qpdf --check` refuses that same file too, with "ERROR: Integer operation attempted on invalid object" — it reads the missing /S as an integer. So this is NOT stricter than the reference, and the message here is the more useful of the two. ⛔ AND ONE PROPERTY OF THE FOREIGN JUDGE WORTH RECORDING: qpdf --check prints "File is linearized" from the parameter dictionary BEFORE validating the hint tables, and then errors. That line is not its approval. Anything reading qpdf as a verdict rather than a report has to read past it. ⛔ A COMMENT OF MINE WAS WRONG AND THE SPEC CORRECTED IT. checkFirstPageEnd said "Annex F puts the first-page section at part 3 and the main one at part 10". The main cross-reference table is part 11; part 10 is the OVERFLOW HINT STREAM. The code was right — it compares against /T — and the sentence explaining it named the wrong part. Mutation, judged by cmd/failset on the failing NAMES: dropping the /S check is red on TestAPrimaryHintStreamWithNoSharedObjectTableIsRefused and no other test. gofmt clean, go vet clean, 0 uncovered blocks of 1808. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * linearize: /S has to be a direct integer, not merely present ⛔ PRESENCE IS WEAKER THAN WHAT THE SPEC SAYS, and I shipped the weaker thing in the previous commit. F.3.6: "the values of all entries in the hint streams' dictionaries shall be direct objects and may contain no indirect object references", the stream Length excepted. /S is a position counted in bytes from the start of the stream data. A name, or an indirect reference, is unusable by precisely the reader this format exists for — one that has fetched the front of the file and can resolve nothing. `/S /x` passed the presence check and names no position at all. Same-width edit again, `/S 43` -> `/S /x`, so only the value changes. gofmt clean, go vet clean, 0 uncovered blocks of 1810. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * linearize: the /S test was weaker than the /S check, and -1 was a position Review found the gap by mutating what I had not mutated, which is exactly why it was asked. ⛔⛔ THE TEST COULD ONLY EXPRESS ONE WRONG KIND. Narrowing the check from "refuse anything that is not an Integer" to "refuse only a Name" left the suite GREEN: `/S /x` is the single case a same-width edit of `/S 43` can write, and it is a Name, so the narrowed check still refused it. The code was right and the evidence for it covered one seventh of what it claims. ⛔ SAME-WIDTH CANNOT REACH THE REST, and that is why they were missing rather than forgotten. `/S 4.3` is six characters against five; an indirect reference or an array is longer still. And the hint stream sits AFTER the fixture's padding run, so growing it there moves the object /H points at and the file is refused for its hint length instead — the wrong reason, which reads as a pass for this test. ⭐ SO THE FUNCTION IS ASKED DIRECTLY. checkHintStream takes bytes and parameters; these cases need nothing else. /H's extent is computed FROM THE PARSE so that nothing earlier can be what refuses the file, and each case differs from the accepted one in exactly one thing: the kind of /S. Seven kinds — name, indirect reference, array, real, string, boolean, null — and a CONTROL asserting an integer /S is accepted, without which the seven would hold against a version that refused this hand-built stream outright. Re-measured: narrowing to Name is now red on six of the seven and the parent, and no other test. The one it does not redden is the Name case, which that mutation still refuses — as it should be. ⛔ AND -1 WAS A POSITION. Review measured `/S -1` accepted, qpdf answering "overflow reading bit stream" — a symptom of seeking outside the data, not a statement about /S. F.3.6: the positions "shall be counted in bytes relative to the beginning of the stream data", so negative names nothing. Same-width edit, so only the value changes. ⚠ THE UPPER BOUND STAYS OPEN, DELIBERATELY. /S is an offset into the DECODED stream data, so bounding it means decoding the hint stream — the first step of reading the tables themselves (F.4), which belongs with that work and not smuggled in here. qpdf reports "/S (shared object) offset is out of bounds" for a /S past the data and this does not. Stated in the README and recorded on #40 rather than left for somebody to discover. ⭐ And the review settled the question this check stands or falls on. /S is required UNCONDITIONALLY, from F.4.2: "All objects associated with the first page (part 6) shall have entries in the shared object hint table, regardless of whether they are actually shared." The first page always has at least its page object, so the table is never empty and no conforming writer may omit it. Requiring a Real is also wrong per F.3.6's "counted in bytes". gofmt clean, go vet clean, 0 uncovered blocks of 1812. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
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.
#40asks for this before a linearizing writer exists, and the suite had done half of it.TestAnOrdinaryFileIsRefusedByBothChecksfeeds this package's own writer output toLinearizedandCheckLinearizedand asserts both refuse it. But "both" there means the two checks in this one file — written together, from the same reading of Annex F, by the same hand. Two judges that have only ever agreed prove nothing about either.qpdf was the one that had not been asked
It validates the hint tables, which this package explicitly does not, so it is the judge that can disagree. It now sees the same ordinary file and says
File is not linearized.⛔ Its exit status is 0 for a file it has just called not linearized
Measured on qpdf 12.4.2:
0for "not linearized",3for a file it merely warns about,2only when it cannot read one at all. The status cannot carry this verdict in either direction, so the error is discarded deliberately, with the reason written where somebody will later be tempted to "fix" it.The pair is what makes it a test
On its own,
File is not linearizedcould be a line qpdf prints about every file, and the assertion would hold against a judge that judges nothing. So qpdf's own linearized output is checked for the absence of that phrase, in the same test.And the control has a control
If qpdf failed to open the file it would print no verdict about linearization at all — and "the phrase is absent" would then read exactly like "the file is linearized". Measured on a 16-byte non-PDF:
No
PDF Version:line, exit 2. So both halves first requirePDF Version:in the output — proof qpdf actually read the file — andt.Fatalotherwise.Scope
This is a control, not a gate on new behaviour. Nothing in
linearized.gochanges, andLinearize PDFstaysMISSINGin the gap table until something writes one. What it buys is that when the writer does arrive, the suite it is developed against has already been shown to refuse a file that is not linearized — by a judge that was not written to let it through.gofmt clean,
go vetclean, 0 uncovered blocks of 1806.Refs #40.
🤖 Generated with Claude Code