refactor(reports): split LETTER_SUBJECT_FONT_SIZE from SUBHEADER_FONT_SIZE (#1953)

[steilerDev]

Summary

The cover letter's subject line reused SUBHEADER_FONT_SIZE, but that constant is load-bearing arithmetic for the running header: headerFootprint() consumes it, and the result drives PAGE_TOP_MARGIN — the constant #1929 spent four review rounds getting right, because getting it wrong clips the header off the top of every page.

So two unrelated 12pt values shared one name, in both directions:

• Shrink SUBHEADER_FONT_SIZE to reclaim header vertical space — a plausible future fix for Report PDF: German table header labels break mid-word (Auftragnehmer, Rechnungsbetrag) #1937 or Bug: report PDF running header shows a 'generated at' label with no timestamp on pages 2+ #1938, both open against that same header — and the cover letter's subject line silently shrinks with it, on a document already reviewed and signed off.
• Bump the subject line for legibility and you quietly change PAGE_TOP_MARGIN on every page of every report.

Why "coincidental" and not "shared"

The ux-designer spec did direct the reuse, so this was worth settling rather than leaving to the next author's taste. But its rationale — "don't hand-write fontSize: 12 as a second copy of that constant" — is an argument against a magic literal, not a claim that the two sizes are the same thing. Its design reasoning for the subject line is standalone ("bold + bumped size makes it read as a subject, not a sixth identical paragraph") and never references the running header.

Hence the new constant carries its own literal 12. const LETTER_SUBJECT_FONT_SIZE = SUBHEADER_FONT_SIZE; would have satisfied the naming complaint while preserving the exact coupling that is the problem.

This is #1939's drift class inverted: that issue removed two drifting copies of one value; this removes one shared name for two values that merely happen to be equal. Both produce the same symptom — an edit with a consequence somewhere the author never looked.

Also recorded

The now-inverted comment directing the reuse is removed — it stated the opposite of the rule now in force, which is worse than no comment.

The module header gains the PDF_STYLES split trigger: letterSubject is the first style entry with no geometry consumer at all, a split is not warranted yet, the trigger is the second such entry, and the target shape is pageGeometry <- pdfStyles <- merge, preserving the edge direction #1939 established. That belongs where the next author will read it, not only in a closed issue.

Test evidence

The two font sizes are now pinned independently — each against its own literal, with a comment forbidding a future reader from "deduplicating" two assertions that happen to assert the same number. Re-coupling them in the tests would have undone in the suite exactly what production just decoupled.

Verified by mutation in both directions:

• LETTER_SUBJECT_FONT_SIZE 12 → 13: exactly 1 test fails (the subject assertion); the subheader and all PAGE_TOP_MARGIN / headerFootprint() assertions stay green.
• SUBHEADER_FONT_SIZE 12 → 11: 4 tests fail — the subheader assertion, the new PAGE_TOP_MARGIN assertion, and two pre-existing scenario assertions, correctly, since that constant genuinely is load-bearing. The subject assertion is not among them.

Production file restored byte-identical after each mutation (md5-verified). A third test pins that PAGE_TOP_MARGIN derives from headerFootprint() alone, addressing the issue's "change the subject size and no page reflows" guarantee, which the two fontSize checks do not assert on their own.

No behaviour change. headerFootprint() and PAGE_TOP_MARGIN evaluate byte-identically; the subject line still renders 12pt, bold, #111827; no existing geometry assertion needed modification.

Fixes #1953

🤖 Generated with Claude Code

[steilerDev]

[product-architect]

Approving. This is my own LOW from the #1951 review, and it landed the way I meant it. All six verification points confirmed against the worktree:

1. The constant is genuinely independent. pageGeometry.ts:86 — const LETTER_SUBJECT_FONT_SIZE = 12;. Its own literal, not = SUBHEADER_FONT_SIZE, not aliased, not derived. This was the one thing that could have satisfied the naming complaint while preserving the coupling, and it didn't happen.

2. SUBHEADER_FONT_SIZE retains exactly the intended consumer chain. Grepped the whole tree: two production references remain — PDF_STYLES.subheader (line 108) and headerFootprint() (line 168). Nothing cover-letter-related touches it. The other letterSubject hits across the repo are the unrelated LLM/content field name.

3. headerFootprint() and PAGE_TOP_MARGIN are byte-identical. The diff touches exactly two hunks in pageGeometry.ts — the module header comment and the new constant + the letterSubject style line. Lines 156-177 are untouched. This was the only thing here that could have gone wrong in a way that mattered, and it didn't.

4. The comment carries the reason. Lines 77-85 name the actual asymmetry rather than just prohibiting the edit: SUBHEADER_FONT_SIZE is footprint arithmetic feeding headerFootprint() and through it PAGE_TOP_MARGIN; LETTER_SUBJECT_FONT_SIZE is letter typography with no geometry consumer. It also states the equality is coincidence, not design. That is the version that survives the next confident reader.

5. The inverted comment is gone. // 12pt — reuse the existing constant, don't add a new literal removed. Correct — it stated the opposite of the rule now in force.

6. Split trigger note landed where it will be read. Module header, lines 16-22, adjacent to the existing #1939 directional rule. Records all four elements: first geometry-unconsumed entry, not warranted yet, trigger is the second such entry, target pageGeometry <- pdfStyles <- merge. Cites #1953.

---

On the test shape — a comment is the right tool here

You asked for a read rather than a checkbox, so: keep the comment, don't add machinery. Two reasons, and I want to be specific because "no better tool exists" would be a weaker answer than the one that's actually true.

First, what's being guarded is a test's own discrimination, not a production invariant. If a future reader collapses the two assertions into toBe(PDF_STYLES.subheader.fontSize), production stays decoupled — the split, the two identifiers, and the comment at pageGeometry.ts:77 are all still there. The worst case is a silent coverage loss, not a regression. Building enforcement machinery to defend a test against a future editor is machinery about machinery, and it would be the most elaborate guard in this module for the least consequential failure mode in it.

Second — and this is the part that settles it — any structural guard would have to encode the very coupling you removed. To make re-coupling fail to compile, you'd need a statement of the form "these two numbers must be permitted to differ." TypeScript has no such statement, and the closest approximation is exactly what already exists: two separate identifiers with two separate literals. The production split is the structural guard. The test comment is documentation of why the test looks redundant, which is a genuinely different job.

I did look for a free structural strengthening — breaking the shape symmetry, since the invitation to collapse comes from two adjacent assertions being identical in shape. The two candidates both cost more than they're worth: asserting subheader.fontSize through its role in headerFootprint() means restating production's own formula in the test, which is precisely the anti-pattern this module's header comment exists to warn against (#1929's "a hand-derived bound that restates its own derivation cannot catch an error in that derivation"); and mirroring the TABLE_SMALL_FONT_SIZE test's constant-to-style tie would require exporting a module-private constant purely so a test can read it, widening the module surface to prove something the comment already says. Neither is an improvement.

The third test is a restatement, not a guard (medium, non-blocking)

pageGeometry.test.ts:98-101 is assertion-for-assertion identical to the pre-existing test at pageGeometry.test.ts:160-167 — same expect(PAGE_TOP_MARGIN).toBe(93), same expect(PAGE_TOP_MARGIN).toBe(Math.ceil(headerFootprint() + 15)), only the order is swapped. It catches exactly the mutation set the older test already catches, so it adds zero discrimination. The QA mutation evidence in the PR body corroborates this rather than contradicting it: SUBHEADER_FONT_SIZE 12 → 11 failing 4 tests includes both halves of this duplicated pair, which is why the number is 4 and not 3.

More importantly, its title asserts a proposition its body does not test. The name claims "PAGE_TOP_MARGIN does not depend on letterSubject.fontSize" — but neither assertion references letterSubject at any point. What the body actually proves is "PAGE_TOP_MARGIN equals its documented formula," which is what the line-160 test is for. A test whose name licenses a stronger claim than its assertions establish is worse than no test, because it is the thing a future reader consults when deciding whether the independence guarantee is covered.

The #1953 guarantee — "change the subject size and nothing reflows" — genuinely is not unit-assertable; it's established by the production split plus the fact that headerFootprint()'s inputs are enumerable and letterSubject is not among them, which is what the comment at pageGeometry.ts:77 records. I'd delete lines 98-101 and let the two font-size pins plus the existing line-160 test stand. If it's kept for narrative continuity, retitle it to claim only what it asserts and cross-reference the older test as the primary. Not blocking either way — a duplicated assertion is harmless in itself; the misleading title is the part worth fixing.

One low finding

LETTER_SUBJECT_FONT_SIZE (line 86) sits inside the // --- Header footprint (AC6/AC7/AC13) --- section that opens at line 69, as its last member — i.e. the constant whose entire purpose is being not part of the header footprint is filed under the header-footprint banner. The doc comment above it mitigates this completely for anyone who reads it, but the section marker is what a skimmer reads. A one-line // --- Cover letter typography --- above it fixes the grouping.

No wiki or ADR change needed — and no fifth ADR-034 item

Confirmed: ADR-034 line 67 already records pageGeometry.ts as sole owner of the pt coordinate system including PDF_STYLES and the font sizes, which is still true after this change. The split trigger is a note about a possible future module split, not a decision taken — recording it in the module header is the right home, and it correctly says a split is not warranted yet. Promoting it to the ADR would document a decision nobody has made. The four open ADR-034 items from this batch (stale pt figures, the "every cell" vs Usage-only scope on width rule #1, the Formatters/AppFormatters naming line, and the input-cap counterpart to line 148) stand at four.

[github-actions]

🎉 This PR is included in version 2.14.0-beta.22 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 2.14.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀