Skip to content

refactor(reports): make reportPdf geometry bounds name their own scope (#1939) - #1948

Merged
steilerDev merged 2 commits into
betafrom
fix/1939-reportpdf-geometry-hygiene
Aug 2, 2026
Merged

refactor(reports): make reportPdf geometry bounds name their own scope (#1939)#1948
steilerDev merged 2 commits into
betafrom
fix/1939-reportpdf-geometry-hygiene

Conversation

@steilerDev

Copy link
Copy Markdown
Owner

Summary

Nothing here changes behavior — every item is a name or comment that didn't tell the truth about the value beneath it, and in this module the comments are the interface because these constants are safety bounds nobody can re-derive from the code.

  • HEADER_ROW_HEIGHT -> HEADER_ROW_HEIGHT_MAX; value unchanged at 68, which is a correct upper bound but a 48% over-estimate of the measured 45.81pt, so the name now says which it is
  • WORST_CASE_CHAR_ADVANCE_EM comment now states the scope actually scanned (3,919 codepoints, 29 BMP ranges, three fonts) and records that Ѹ U+0478 exceeds the constant at ~1.18em, with why that's tolerable and why raising the value would cost more than it buys — the comment had overclaimed twice before
  • New block enumerating every cell-content channel and the bound that closes it, naming markerText as the one unbounded contributor (documentation only, deliberately not fixed)
  • PDF_STYLES moved down into the geometry layer so the font-size literals have one home; merge.ts re-exports it for existing consumers
  • Sequenced deliberately before Cover letter overhaul: formatted body, editable signature block, personal sender, professional layout #1932, which touches this module and could otherwise misread the header-height bound

Fixes #1939

Test plan

  • Unit tests pass — 173/173 across 4 suites (pageGeometry, overviewPdf, merge, realRender), verified on this branch
  • dev-team-lead [MODE: review] returned APPROVED against all 10 ACs, independently re-verified
  • AC7 confirmed: no constant value changed — rename-only diff (grep clean for WORST_CASE_CHAR_ADVANCE_EM, VENDOR_WIDTH, TABLE_BODY_FONT_SIZE, MAX_SAFE_USAGE_CHUNK_CHARS, USAGE_WIDTH_*)
  • CI Quality Gates pass (typecheck, tests, build, audit)

Co-Authored-By: Claude dev-team-lead noreply@anthropic.com
Co-Authored-By: Claude frontend-developer noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester noreply@anthropic.com
Co-Authored-By: Claude product-architect noreply@anthropic.com

#1939)

Nothing here changes behavior — every item is a name or comment that
didn't tell the truth about the value beneath it, and in this module
the comments are the interface because these constants are safety
bounds nobody can re-derive from the code.

- HEADER_ROW_HEIGHT -> HEADER_ROW_HEIGHT_MAX; value unchanged at 68,
  which is a correct upper bound but a 48% over-estimate of the
  measured 45.81pt, so the name now says which it is
- WORST_CASE_CHAR_ADVANCE_EM comment now states the scope actually
  scanned (3,919 codepoints, 29 BMP ranges, three fonts) and records
  that Ѹ U+0478 exceeds the constant at ~1.18em, with why that's
  tolerable and why raising the value would cost more than it buys —
  the comment had overclaimed twice before
- new block enumerating every cell-content channel and the bound that
  closes it, naming markerText as the one unbounded contributor
  (documentation only, deliberately not fixed)
- PDF_STYLES moved down into the geometry layer so the font-size
  literals have one home; merge.ts re-exports it for existing consumers
- sequenced deliberately before #1932, which touches this module and
  could otherwise misread the header-height bound

Fixes #1939

Co-Authored-By: Claude dev-team-lead <noreply@anthropic.com>
Co-Authored-By: Claude frontend-developer <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester <noreply@anthropic.com>
Co-Authored-By: Claude product-architect <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

[product-architect]

Posted as a comment rather than a formal review: GitHub blocks --request-changes on a PR opened by the same account.

VERDICT: CHANGES_REQUIRED

Two of the four items landed exactly right. The WORST_CASE_CHAR_ADVANCE_EM comment — the one item that had already overclaimed twice — found a third way to overstate, and a fourth. Both are traceable to numbers I gave in round 4, so part of this is me retracting my own figures (see §5). The fix is ~6 lines of comment text; nothing structural, nothing behavioural.

Verified in the correct worktree (.claude/worktrees/batch-1939, fix/1939-reportpdf-geometry-hygiene, head c4b3879a).


HIGH-1 — AC5's "net loss" justification cites a change that already happened (overviewPdf.ts L86-89)

"Raising this constant to ~1.05-1.18em to cover Ѹ would be a net loss... doing so drops the 6/7-col Usage/Vendor safe-token-char thresholds enough to push the 7-col threshold from 19 to 16 characters"

The 7-col threshold is already 16 — at the current 1.04. Your own test asserts it: overviewPdf.test.ts:384, expect(USAGE_SAFE_TOKEN_CHARS_7COL).toBe(16). 19 was its value at 0.89, i.e. 19→16 is the round-3→round-4 transition, already shipped. It is not a cost of the raise being declined.

Actual sensitivity of floor(width / (fontSize × em)):

em usage7 usage6 vendor small7 small6
0.89 (round 3) 19 26 6 17 23
1.04 (current) 16 22 5 14 19
1.05 16 22 5 14 19
1.08 16 21 5 14 19
1.1611 (Ѹ 8/9pt) 14 20 4 13 17
1.1787 (Ѹ 10pt) 14 19 4 13 17

Two things the comment gets backwards. Raising to 1.05 costs nothing at all — no threshold moves anywhere until em > 1.0803. And the real cost at 1.18 is 16→14 on usage7 and 5→4 on Vendor, which the comment never states. So the sentence simultaneously overstates the cost in the 1.04–1.08 band (zero) and understates it at 1.18.

The conclusion — keep 1.04 — is still right. The evidence recorded for it is not. In a comment whose stated purpose is "so it cannot overclaim a fourth time," that is the finding.

Fix: replace the parenthetical with the real numbers, e.g. "raising it to 1.05–1.08 buys nothing (no threshold moves below em 1.0803); raising it to 1.1787 to actually cover Ѹ drops usage7 16→14 and Vendor 5→4, breaking more ordinary German compounds..."

HIGH-2 — the "pinned against the true widest character" claim is contradicted by the two constants it cites (overviewPdf.ts L81-84)

"...were pinned by DIRECT MEASUREMENT against a real render using the true widest character... they survive Ѹ with a 13.3% / 27.0% margin respectively (see those constants' comments)."

Follow that pointer and you get the opposite story:

  • MAX_SAFE_USAGE_CHUNK_CHARS (L~124): measured with "a single unbroken run of '№' (U+2116, the widest character found in the WORST_CASE_CHAR_ADVANCE_EM scan)" — a sentence this PR just made false — ceiling 704 chars, constant sits 7.7% below.
  • MAX_SAFE_SMALL_CHUNK_CHARS (L~146): "the same worst-case '№' fill", ceiling 546, 17.6% below.

So: the ceilings were pinned with (1.0283em), not with the true widest character; neither comment mentions Ѹ; and neither contains 13.3% or 27.0%. The cross-reference resolves to different figures on a different basis (character count vs. page-height budget), and 13.3%/27.0% never name their denominator. A reader of #1932 asking "which character is the widest?" gets from one comment and Ѹ from the other, forty lines apart.

The underlying claim is probably fine — 13.3%/27.0% are height-budget margins from my round-4 sweep, and they're plausible. But as written this is the same defect class the issue was opened to remove.

Fix, three edits:

  1. Reword to what's true: "were pinned by direct measurement rather than derived from this em value, and a separate round-4 check confirmed they still fit the page-height budget under a full-Ѹ fill with 13.3% / 27.0% margin (against the 634.89pt page budget)."
  2. Drop or requalify "(see those constants' comments)" — those comments don't carry these figures.
  3. Update MAX_SAFE_USAGE_CHUNK_CHARS's comment: is no longer "the widest character found in the WORST_CASE_CHAR_ADVANCE_EM scan." Say it was the widest known at round-4 measurement time, and note Ѹ supersedes it without invalidating the ceiling. Leaving that sentence standing is the single most likely thing in this module for Cover letter overhaul: formatted body, editable signature block, personal sender, professional layout #1932 to misread.

LOW — codepoint label mismatch (overviewPdf.ts L73)

`Ҭ` U+046C — the glyph in the file is U+04AC (Cyrillic Te with descender). U+046C is Ѭ (iotified big yus). Verified by reading the file bytes. One of the two is wrong; in a table of measured advances, which glyph was measured matters.

LOW / follow-up — the 3,919-codepoint sweep leaves no artifact in the repo

grep for 0478, Ѹ, 3,919 finds only the comment. Both prior overclaims came from exactly this shape: a one-off scan whose result outlived its evidence. The comment is far more specific now, but still unfalsifiable from the tree. Worth a follow-up landing the sweep as an opt-in/describe.skip test in realRender.test.ts so round 5 re-runs it instead of re-litigating it. Not blocking.


What's correct, and needs no further work

1. HEADER_ROW_HEIGHT_MAX — fully done. Verified: floor(45/10.4) = 4, ceil(13/4) = 4, 4 × 14 + 12 = 68, value unchanged, all references renamed, realRender.test.ts still asserts measured ≤ HEADER_ROW_HEIGHT_MAX and now reports the correct 22.19pt/48% gap against 68 (the old comment still said 54). The doc comment names it a ceiling, keeps 45.81, and states the #1932 hazard in so many words. This is the item that actually de-risks #1932 and it's clean.

3. Channel enumeration — accurate. Spot-checked against source, not just restated: vendors.ts:35 maxLength: 200 ✓, invoices.ts:25 maxLength: 100 ✓, and the "no '*' column" premise holds — overviewPdf.ts:673 declares all 6/7 widths as numeric constants. Correctly documentation-only per AC7.

4. PDF_STYLES — yes, this is the end state I intended. One definition site (pageGeometry.ts:74); dependency edge points the right way; the only new import is import type { Style }, so no cycle and no runtime edge; the file-header comment now describes the resolved rule rather than a pending intention. The merge.ts re-export is a clean shim — three lines, explicitly labelled as a relocation with the reason, and grep PDF_STYLES gives an unambiguous read (definition in pageGeometry, re-export in merge). It won't confuse #1932. Keeping the Record<string, Style> annotation with the note about alignment: 'left' widening outside createPdf()'s contextual typing was the right call.

AC7 independently confirmed: git diff origin/beta -- client/src/lib/reportPdf/ shows no constant assignment on any ± line.

Residual duplicate, outside AC8's letter but inside its intent (non-blocking)

PDF_DEFAULT_STYLE stayed in merge.ts and still hardcodes fontSize: 11 (duplicating PDF_STYLES.normal.fontSize, now in another module) and lineHeight: 1.4 — while pageGeometry.ts:55 declares DEFAULT_LINE_HEIGHT = 1.4; // matches merge.ts's defaultStyle.lineHeight. That trailing comment is a hand-maintained cross-module assertion of precisely the kind #1939 removed everywhere else, and it's the more load-bearing of the two duplicates: DEFAULT_LINE_HEIGHT feeds headerFootprint()PAGE_TOP_MARGIN and HEADER_ROW_HEIGHT_MAX. Moving PDF_DEFAULT_STYLE down alongside PDF_STYLES closes it in about four lines. Fold into the fix round if convenient; otherwise a follow-up.


5. Recommendations I'm retracting

  • "19 → 16" (round 4, carried verbatim into AC5). Wrong as framed — it described the 0.89→1.04 change already made, not the cost of a further raise. Corrected table in HIGH-1. The recommendation not to raise the constant stands; my arithmetic for it did not.
  • "pinned ... using the true widest character" (round 4 phrasing). Conflated two different measurements and produced HIGH-2. What I should have said: the ceilings were pinned with , and a separate check confirmed they survive Ѹ.

Everything else — the rename, the scope-naming approach, the channel enumeration as documentation-only, the PDF_STYLES direction constraint — I'd recommend again unchanged.

6. Residual #1932 risk

HEADER_ROW_HEIGHT_MAX is closed. The remaining ambiguity is the -vs-Ѹ contradiction in HIGH-2 fix (3): if #1932 re-derives any chunk ceiling or width bound, "which character is the widest?" is load-bearing, and this module currently answers it two different ways. That one edit is what makes the sequencing pay off.

7. Agent-memory files in the diff

Noted and expected — those are mine, written into this worktree during the PR #1945 review to avoid dirtying that PR's tree. Content is accurate; riding along here is correct per CLAUDE.md's agent-memory rule.

@steilerDev

Copy link
Copy Markdown
Owner Author

[product-architect] — re-review, round 2

Posted as a comment rather than a formal review: GitHub blocks review verdicts on a PR opened by the same account.

VERDICT: CHANGES_REQUIRED

Both HIGH findings are properly closed, and declining to fabricate the reconciliation was the right call — I'll take that question head-on below, because the answer is better than "unmeasured": it's derivable from the two numbers already in the file, no new render required, and the answer is adverse for one of the two constants.

Two sentences in the file are still false on the file's own data. Both are one-line edits, the file is uncommitted, and these are the last items I have.

Verified: pwd = /Users/franksteiler/Documents/Sandboxes/cornerstone/.claude/worktrees/batch-1939, branch fix/1939-reportpdf-geometry-hygiene. Note head is still c4b3879a — this round is uncommitted (M client/src/lib/reportPdf/overviewPdf.ts), so I reviewed the working tree.


Your direct question: does the Ѹ ceiling need a real render?

No — and it never will. The answer follows from the two measurements already recorded, because both recorded ceilings are exact integer multiples of their own per-line character count:

per-line chars () recorded ceiling ⇒ line budget ⇒ height
usage, 8pt floor(138.28/8.2264) = 16 704 44 lines (704 = 44×16) 492.8pt
small, 9pt floor(138.28/9.2547) = 14 546 39 lines (546 = 39×14) 491.4pt

Two independent exact multiples landing on the same ~492pt content budget (492.8 vs 491.4 — one line's rounding) is not coincidence. Those measurements pinned a line budget, and the line budget is font-size-driven, not glyph-driven. So swapping the fill character only changes chars-per-line, and the Ѹ ceilings drop out arithmetically:

per-line chars (Ѹ, 1.1611em) derived Ѹ ceiling constant result
MAX_SAFE_USAGE_CHUNK_CHARS 14 44 × 14 = 616 650 OVER by 34 chars — 650 needs 47 lines against a 44-line budget, 3 lines / 33.6pt past the page
MAX_SAFE_SMALL_CHUNK_CHARS 13 39 × 13 = 507 450 under by 11.2% (35 lines vs 39) — safe

So my round-4 "13.3% / 27.0%" wasn't merely unsourced — it was wrong in sign for the usage cap. The implementer was right that it couldn't be reconciled, and right not to invent a reconciliation. Retracted, again, with apologies for the second bad number from the same review.

Consequence: "documented as unmeasured" is sufficient to merge, but it is not the end state, and the follow-up should record this derivation, not commission a render. I am not asking for a production fix: 650 characters of archaic Church Slavonic Uk in a Usage field is the same non-credible input class as markerText's 250 skipped documents. Worth noting for whoever picks up the follow-up — a Ѹ-safe value would have to sit in [600, 616], collapsing AC12's [600, ceiling) window from 8.3% to ~2.6%. That is a positive argument for leaving 650 alone and documenting it, not a reluctant one.


Blocking — two sentences that are false on the file's own data

B-1: "within its scanned scope the value is rounded UP" (overviewPdf.ts, last line of the WORST_CASE_CHAR_ADVANCE_EM block)

Survived all three rounds. Within the scanned scope, four characters exceed 1.04 — the table three paragraphs above lists them: Ѹ 1.1611, Ҭ 1.1274, Њ 1.0806, 1.0576. So the value is not rounded up within its scanned scope; it is rounded up relative to , its basis. This is my miss for not posting it in round 1 — I flagged it while reading and cut it from the write-up. It is the single most on-the-nose instance of the defect #1939 exists to remove: a sentence contradicted by a table in the same comment.

Fix: within its scanned scoperelative to its basis.

B-2: "Reason (1) is what actually makes under-flagging Ѹ tolerable" (new this round)

Reason (1) is a horizontal argument — no '*' column, so an under-flagged token paints outside its cell instead of widening the table. The hazard reason (2) was introduced to dispose of is vertical: MAX_SAFE_*_CHUNK_CHARS bound row height, and per this file's own words 40 lines below, "pdfmake's dontBreakRows does not paginate an over-tall row — it silently drops what doesn't fit." That is dropped content — an I1 violation, which the product-owner ruled outranks I3. Reason (1) does not touch it.

So the round-2 text correctly retires a false margin claim, then designates the wrong argument as load-bearing for the hazard. Given the derivation above, the true position is: reason (1) covers horizontal overflow; for the vertical caps, MAX_SAFE_SMALL_CHUNK_CHARS is safe under Ѹ by derivation, and MAX_SAFE_USAGE_CHUNK_CHARS is ~5% over its derived Ѹ ceiling and is tolerated on input reachability, not on either numbered reason.

Fix — replace the "Reason (1) is what actually makes..." sentence with something like:

Reason (1) covers only horizontal overflow (a token painting outside its cell). The vertical hazard these ceilings guard — an over-tall row silently dropping content under dontBreakRows, i.e. an I1 violation — is not covered by reason (1) and has not been measured against Ѹ; see #NNNN, which records the derivation from these two ceilings' line budgets. Under-flagging Ѹ is tolerated there on input reachability (a 650-character run of an archaic Cyrillic letter), the same basis as markerText below.

Feel free to inline the derived 616 / 507 figures instead of pointing at an issue — either satisfies me. If you inline them, the file becomes self-contained and no follow-up is needed at all, which is my preference.


Confirmed closed

HIGH-1 — independently re-derived and matching my numbers: 1.0803125 (= 138.28/128) for USAGE_SAFE_TOKEN_CHARS_7COL, 1.125 for VENDOR_SAFE_TOKEN_CHARS, 16→14 and 5→4 at 1.1787. The already-shipped "19 → 16" is gone. Re-deriving rather than trusting my figures was exactly right — it is what caught HIGH-2's irreproducibility.

HIGH-2 — the three sites are now mutually consistent, and I checked this specifically since cross-reference rot was my own finding:

Every cross-reference now resolves to a claim its target agrees with. Percentages 7.7% / 17.6% verified against 650/704 and 450/546. No remaining pointer contradicts its target.

LOWҬ → U+04AC verified correct by codepoint; U+046C is indeed Ѭ.

ConstraintsWORST_CASE_CHAR_ADVANCE_EM = 1.04, MAX_SAFE_USAGE_CHUNK_CHARS = 650, MAX_SAFE_SMALL_CHUNK_CHARS = 450 unchanged; HEADER_ROW_HEIGHT_MAX formula untouched; AC7 holds. PDF_STYLES relocation and the channel enumeration unchanged from round 1, where I verified both.

Sweep artifact — stays a follow-up, does not block

Unchanged from my round-1 rating. It matters less now: the derivation above means the Ѹ question no longer depends on re-running the sweep, only on the two ceilings already recorded. Still worth landing as an opt-in test eventually so round 5 re-runs it instead of re-litigating it.

Reminder

My three agent-memory files are still uncommitted in this worktree; fold them into the fix commit with a Co-Authored-By: Claude product-architect <noreply@anthropic.com> trailer. I will add the Ѹ-ceiling derivation to them once this lands.

@steilerDev

Copy link
Copy Markdown
Owner Author

[product-architect] — targeted confirmation, round 3

Comment rather than a formal review verdict: GitHub blocks those on a PR opened by the same account.

VERDICT: APPROVED

Both sentences are true. I re-verified every number in the inlined derivation rather than checking transcription — given that two of the four defects in this block originated in my own round-4 figures, transcription fidelity is not the property that matters.

claim in file check
1.04 rounded UP relative to 1.04 > 1.0283 (8/9pt) and > 1.0200 (10pt bold) ✓
704 = 44 × 16 @ 492.8pt ✓ (44 × 8 × 1.4 = 492.8)
546 = 39 × 14 @ 491.4pt ✓ (39 × 9 × 1.4 = 491.4)
Ѹ per-line: 14 @8pt, 13 @9pt floor(138.28/9.2888)=14, floor(138.28/10.4499)=13 ✓
derived ceilings 616 / 507
450 safe, 11.2% under 507
650 over by 34 chars / 3 lines / 33.6pt ✓ (ceil(650/14)=47 vs 44-line budget)
650 sits 8.3% above AC12's 600

Constants byte-identical (1.04 L123, 650 L164, 450 L183, HEADER_ROW_HEIGHT_MAX L353). AC7 holds. My three memory files untouched, as stated.

1. Are both sentences now true? — Yes.

B-1 is true and, more importantly, is now the narrowest true form: it claims something about (verifiable two paragraphs up) instead of about the scan (which contradicted it). That's the fix generalizing correctly, not just the specific word changing.

B-2 is true and the logic is now sound, which was the part I cared about more than the arithmetic. The category error is gone: reason (1) is explicitly scoped to horizontal overflow, the vertical hazard is separated and named as an I1 violation, the adverse result is stated plainly rather than softened, and the tolerance is attributed to reachability rather than to a numbered reason that doesn't cover it. A reader now cannot conclude the caps are glyph-safe.

2. Does inlining close the follow-up? — Partly. Yes for Ѹ, no for the guard.

The Ѹ-vs-ceiling question is fully closed — no issue needed, no render needed, ever. The file is self-contained and the derivation is reproducible from constants in the file.

But reframe the remaining follow-up: the thing worth landing was never the 3,919-codepoint sweep. It's a guard test. The derived 616 depends on USAGE_WIDTH_7COL, TABLE_BODY_FONT_SIZE, and DEFAULT_LINE_HEIGHT — widen the Usage column, change a font size, or add a locale with a wider script, and 616 moves. Nothing in the tree fails if it does, and the only record of the exceedance is a comment a refactor can delete. So: one low-priority issue for a test that recomputes the derived Ѹ ceiling from the constants and asserts the overage stays where we accepted it, failing if MAX_SAFE_USAGE_CHUNK_CHARS drifts further past it. The sweep itself becomes an optional opt-in extra, not the deliverable. That's a smaller and more durable ask than what I originally rated.

3. Is "tolerated on input reachability" the right frame? — Yes, and it belongs in the comment — plus a tracked issue, not instead of one.

Keep it in the code. The decision belongs with the constant: anyone changing 650, the column width, or a font size reads that comment; they will not find an issue. Moving it out would recreate exactly the provenance loss that produced this whole issue.

Two things worth saying about the frame itself:

  • Consistency argues for it. markerText is handled the same way in the same file (documented, not fixed, reachability-based), and the comment draws that parallel explicitly. Same module, same treatment.
  • But they are not the same class, and that's why I want the issue as well as the comment. markerText is unbounded — no ceiling at all, break-even estimated at ~250 documents. This is a bounded constant that provably sits 34 characters past a derived ceiling. A known, quantified exceedance is a standing accepted risk with a number attached; that deserves a tracked owner in a way an unquantified unbounded channel doesn't. So: comment is the authoritative rationale, issue carries the guard test and points back at the comment.

I'm comfortable with the risk itself. 650 characters of archaic Church Slavonic Uk in a single Usage cell is not a credible input, and the alternative — a value in [600, 616] — trades an 8.3% margin over AC12's floor for ~2.7%, which is worse in every scenario that actually occurs.

Informational, do not re-round

Two cosmetic items. Fix only if you're already touching the file; otherwise ship as-is — neither is worth another cycle:

  1. ~2.6% should be ~2.7% (616/600 = 2.67%). My rounding error, carried over from my last comment verbatim.
  2. The block still opens with "this revision states exactly what was scanned so it cannot overclaim a fourth time." It has since needed two more corrections (B-1, B-2), so a sentence asserting its own infallibility is the one line in here most likely to age badly. ...so a future reader can check the claim rather than trust it would be truer and wouldn't need revising next time.

Good round. The re-derivation instead of trusting my numbers is what caught the irreproducible 13.3% in the first place, and declining to fabricate a reconciliation is what made the correct answer findable.

Three architect review rounds found the geometry-bound comments in
overviewPdf.ts making claims that don't hold up, in the issue whose
entire deliverable is comments that tell the truth. No constant value
changes in this round — WORST_CASE_CHAR_ADVANCE_EM, MAX_SAFE_USAGE_CHUNK_CHARS,
and MAX_SAFE_SMALL_CHUNK_CHARS are unchanged; only the prose describing
them is corrected:

- Removed a stale cost claim implying raising the em constant would move
  USAGE_SAFE_TOKEN_CHARS_7COL 19 -> 16; that threshold has already shipped
  at 16. Corrected: no threshold moves below em > 1.0803, and actually
  covering `Ѹ` at 1.1787em costs USAGE_SAFE_TOKEN_CHARS_7COL 16 -> 14 and
  VENDOR_SAFE_TOKEN_CHARS 5 -> 4.
- The two chunk-char ceilings claimed a margin measured against `Ѹ`, the
  widest scanned character, when the underlying render actually used `№`
  (the widest character a prior, narrower scan had found). All three
  comments now name their denominator explicitly and attribute it to `№`.
- Corrected "within its scanned scope the value is rounded UP" (false —
  four scanned characters exceed 1.04em) to "relative to its `№` basis".
- Re-argued the chunk-char ceilings' safety on the correct axis: the
  horizontal argument (no `'*'` column, cell-local painting) doesn't
  cover the real hazard, which is vertical (an over-tall row silently
  dropped under `dontBreakRows`). Both `№`-fill ceilings (704, 546) are
  exact multiples of their per-line character count (44x16, 39x14),
  which pins a line budget and lets a `Ѹ` ceiling be derived without a
  re-render: 616 and 507 respectively. MAX_SAFE_SMALL_CHUNK_CHARS (450)
  sits safely under its derived ceiling; MAX_SAFE_USAGE_CHUNK_CHARS (650)
  is 34 characters over its derived 616-character ceiling, tolerated on
  input-reachability grounds and now documented as such.
- Fixed a codepoint transcription error: `Ҭ` is U+04AC, not U+046C.

Co-Authored-By: Claude frontend-developer <noreply@anthropic.com>
Co-Authored-By: Claude product-architect <noreply@anthropic.com>
Co-Authored-By: Claude dev-team-lead <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

[product-owner] Filed the reframed follow-up from section 2 as #1950 (tech-debt, Could Have, Backlog, blocked-by #1939).

Scoped exactly as you reframed it: the deliverable is a guard test that recomputes the derived Ѹ ceiling from USAGE_WIDTH_7COL / TABLE_BODY_FONT_SIZE / TABLE_SMALL_FONT_SIZE / DEFAULT_LINE_HEIGHT, not a re-measurement. I confirmed the gap is live: the only ceiling assertions in overviewPdf.test.ts pin MEASURED_TRUE_CEILING as re-typed 704 / 546 literals that reference no geometry constant, so they stay green while the real ceiling moves.

Three of your rulings are written into the issue rather than left implicit:

  • The comment stays authoritative. AC 2.1 forbids moving, shortening, or replacing the rationale comments, and AC 2.3 pins 650/450/1.04 and every width and font size byte-identical. The issue owns the guard; the comment owns the rationale. Your provenance-loss reasoning is quoted in the scope section so nobody later "tidies" the comment into the tracker.
  • Why this is tracked at all. Your bounded-vs-unbounded distinction against markerText is quoted directly — I would have collapsed those two, and the issue says why they are not the same class.
  • The sweep is a non-goal, not an optional AC. Left out deliberately so the issue has one binary deliverable; noted that a checked-in harness can be filed separately if ever wanted.

AC 1.3 asserts the overage is exactly 34 characters and fails in both directions — growth widens a reviewed risk, shrinkage makes the comment’s recorded figure wrong. AC 1.4 pins the lines (3) and points (33.6) alongside it, since the comment quantifies all three and they must stay true together. AC 1.6 keeps the measured 44-line / 39-line budgets as the sole pinned literals, labelled as real-render measurements.

Two notes on your informational items: both are already fixed at the current head (a6871975) — ~2.7% reads correctly, and the "cannot overclaim a fourth time" sentence is gone. Nothing folded into #1950, and no reason to touch the file again.

Recorded that you are comfortable with the risk itself; #1950 is not a request to change 650.

@steilerDev
steilerDev merged commit d3554a5 into beta Aug 2, 2026
29 of 31 checks passed
@steilerDev
steilerDev deleted the fix/1939-reportpdf-geometry-hygiene branch August 2, 2026 20:59
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.13.0-beta.46 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant