test(pdf): #204 extract PdfAuditor page-grid geometry to pure method + 9 tests - #562
Merged
Merged
Conversation
…+ 9 tests Extract the inline A4-based page-grid arithmetic from GetExpectedImageOrder (nbColumns/nbRows/nbCardsPerPage) into the pure, determinatic ComputeAuditPageGeometry so the audit's layout contract is unit-testable without a PDF render — output-neutral (call site preserves the exact computation). 9 contract tests pin the contract: configured-columns honored, zero/ negative config fallback to floor(A4/card), floor truncation not rounding, rows = floor(A4h/cardH), cards-per-page = rows×cols, hardcoded A4 (observable), and a DIVERGENCE pin documenting the latent correctness bug vs the renderer. Latent bug documented, NOT fixed (No-Pendulum — a behavior change belongs in its own PR): the audit hardcodes PageSizes.A4 and ignores the page margin + header band that the renderer (ComputePageGeometry) subtracts, so its NbCardsPerPage is systematically larger than the renderer's whenever the document is not plain A4-without-header — silently producing false audit mismatches or false passes. The divergence test pins the renderer value (84 cards, headered) vs the audit value (96) so the gap is observable and fixable in isolation. Suite 438/0/5 (baseline 429 + 9). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
clusterManager-Myia
left a comment
Collaborator
There was a problem hiding this comment.
[NanoClaw] Refactor-correctness pass — extraction is behavior-preserving, method is pure, tests are genuine.
- Output-neutral confirmed. The moved arithmetic is byte-identical: same
PageSizes.A4constant, same guards (configuredNbColumns > 0), same(int)truncation casts, samenbRows * nbColumns. Call site passes the exact values (cardWidthPoints, cardHeightPoints, docConfig.NbColumns) that were inlined. The only structural change isnbRowsnow surfaces on the struct (previously used only as a local to derivenbCardsPerPage) — no runtime difference. - Pure method.
ComputeAuditPageGeometry(float, float, int)→AuditPageGeometrystruct: no I/O, no PDF render, noDateTime.Now/random, deterministic. Genuinely unit-testable. - Tests are real assertions, not tautologies. Cross-checked
Columns_FloorTruncation: A4 595.2756/100 = 5.95 →(int)truncates to 5 (not 6 with rounding) — a genuine floor-vs-round edge case.CardsPerPage_IsRowsTimesColumns: 11×8 = 88, verified manually. The divergence test correctly pins the latent A4-vs-renderer bug as observable (audit 96 > renderer 84 with header) without fixing it — appropriate scope for an output-neutral PR. - Minor: the
using staticworkaround for the namespace/class name collision (Argumentum.AssetConverter.PdfAuditor.PdfAuditor) is a legitimate C# quirk, well-commented in the test file.
No concerns blocking merge. Good extraction — fragile inline geometry now pinned by 9 contract tests.
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
Extract the inline A4-based page-grid arithmetic from
PdfAuditor.GetExpectedImageOrderinto a pure, deterministic methodComputeAuditPageGeometry, so the audit's layout contract is unit-testable without a PDF render. Output-neutral — the call site preserves the exact computation, no behavior change.Part of the #204 output-neutral extraction lane (cont. po-2024): the matrice ClassMap is closed (#561), SVG disambiguation is pinned (#560). A targeted audit (Explore + code read) found one real gap left: the audit computed its page-grid geometry inline, with only a code comment guarding the assumption.
Why
GetExpectedImageOrderchunked the deck into page-sized groups using geometry derived from:PageSizes.A4(comment: "Assuming A4 for calculation, needs to be dynamic if possible")…all of which diverge from the renderer (
PrintAndPlayDocument.ComputePageGeometry, which resolves the actualdocConfig.PageSizeand subtracts margin + header). The audit'sNbCardsPerPageis therefore systematically larger than the renderer's whenever the document is not plain A4-without-header — chunking the deck into pages of the wrong capacity and silently producing false audit mismatches or false passes, with no signal beyond the PDF render. This arithmetic had zero unit coverage.Changes
PdfAuditor.cs— extraction (output-neutral):ComputeAuditPageGeometry(cardWidthPoints, cardHeightPoints, configuredNbColumns)pure static method +AuditPageGeometryreadonly struct (NbColumns,NbRows,NbCardsPerPage).GetExpectedImageOrdercalls it (same arithmetic, same result).PdfAuditorPageGeometryContractTests.cs— 9 contract tests pinning the real contract:> 0> 0guard)Math.Roundwould regress)No-Pendulum
The latent bug is documented and pinned, NOT fixed. A behavior change (route the audit through
ComputePageGeometry, or accept margin/header inputs) belongs in its own PR — this one is strictly output-neutral extraction + contract pinning, matching the lane discipline (#553/#556/#560).Verification
dotnet test --filter PdfAuditorPageGeometryContractTests→ 9/9 pass