Skip to content

fix(reports): running header timestamp and German word-break (#1937, #1938) - #1982

Merged
steilerDev merged 3 commits into
betafrom
fix/1937-1938-pdf-header-wordbreak
Aug 4, 2026
Merged

fix(reports): running header timestamp and German word-break (#1937, #1938)#1982
steilerDev merged 3 commits into
betafrom
fix/1937-1938-pdf-header-wordbreak

Conversation

@steilerDev

Copy link
Copy Markdown
Owner

Summary

Fixes #1938
Fixes #1937

Test plan

  • merge.test.ts: updated assertion pins full "Generated At: 01/15/2026" string (label + separator + value), not bare i18n key
  • realRender.test.ts: 3 new AC7 tests pin DE label length bounds (≤ 8 chars for vendor, ≤ 9 for invoiceAmount) plus exact value pins; stale 'Auftragnehmer'/'Rechnungsbetrag' assertions updated

🤖 Generated with Claude Code

steilerDev and others added 2 commits August 4, 2026 09:02
#1938)

- merge.test.ts: update the header-callback assertion to expect the
  interpolated "Generated At: 01/15/2026" string (labels.generatedAt +
  separator + generatedAtText), not the bare i18n key — pins AC8 so
  the value cannot silently disappear again (#1938)

- realRender.test.ts:
  - HIGH1 budget-overview: 'Auftragnehmer'→'Firma', 'Rechnungsbetrag'→
    'Betrag'; assert single-line render (positions.length === 1) since
    both short labels fit their 45pt/48pt columns without wrapping
  - HIGH1 claim (6-col): same label updates
  - Production singleton describe: 'Auftragnehmer'→'Firma' to match the
    updated de/budget.json translation
  - New '#1937 AC7' describe: length-bound assertions (≤ 8 / ≤ 9 chars,
    derived from 5.19pt/char measured Roboto average advance) plus exact
    value pins for DE ('Firma', 'Betrag') and EN stability checks
    ('Vendor', 'Invoice Amount')

Fixes #1937
Fixes #1938

Co-Authored-By: Claude qa-integration-tester <noreply@anthropic.com>
…#1937, #1938)

- merge.ts: pass `labels.generatedAt + ': ' + generatedAtText` to
  buildPageHeader so pages 2+ show the label and value (AC1-5, #1938).
  Previously only the bare label was passed, leaving the timestamp blank
  on every page after the first.
- de/budget.json: shorten `sourceReports.table.vendor` from
  "Auftragnehmer" (67.5pt > 45pt column) to "Firma" (5 chars, ~26pt)
  and `sourceReports.table.invoiceAmount` from "Rechnungsbetrag"
  (78.7pt > 48pt column) to "Betrag" (6 chars, ~27pt), eliminating
  mid-word breaks on German reports (#1937).

Fixes #1938
Fixes #1937

Co-Authored-By: Claude frontend-developer <noreply@anthropic.com>
Co-Authored-By: Claude translator <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

[ux-designer]

Design review for PR #1982 — two PDF pipeline bug fixes (#1937, #1938).


Scope

Both fixes live entirely in client/src/lib/reportPdf/. There are no CSS Module, design token, React component, dark-mode, or web-UI accessibility changes in this PR. The UX designer review is correspondingly scoped to PDF visual output consistency.


#1938 — Running Header Text Content

Finding: Correct. The merge.ts change (line 131) now passes:

`${reportContent.labels.generatedAt}: ${reportContent.sourceInfo.generatedAtText}`

I cross-checked this against overviewPdf.ts line 531, which renders the same field on page 1 as:

text: `${reportContent.labels.generatedAt}: ${reportContent.sourceInfo.generatedAtText}`

The format strings are byte-for-byte identical. Pages 2+ now match page 1's output — the bug was the running header previously discarding the value and showing only the bare translated label string.


#1937 — German PDF Column Header Labels

Finding: Correct. The two DE translation changes in client/src/i18n/de/budget.json:

Key Before After Column width Fits?
sourceReports.table.vendor Auftragnehmer (13 chars, ~67.5pt) Firma (5 chars, ~26pt) 45pt Yes, with margin
sourceReports.table.invoiceAmount Rechnungsbetrag (15 chars, ~78.7pt) Betrag (6 chars, ~31pt) 48pt Yes, with margin

Verified against the AC7 column-capacity math (5.19pt/char average at 10pt Roboto Bold): "Firma" is well within the 8-char floor and "Betrag" is well within the 9-char floor for their respective columns. The realRender.test.ts HIGH1 tests assert positions.length === 1 for both header cells at a full pdfmake-render level, which is the strongest possible evidence of actual fit.

The adjacency of "Betrag" (invoiceAmount) alongside the unchanged "Zugeordneter Betrag" (allocatedAmount, 75pt column) is noted: the translator's rationale is that column context (adjacent to "Rechnungsnr." and actual numeric amounts) makes "Betrag" unambiguous. No UX objection.

VENDOR_HEADER_WORST_CASE_LINES in overviewPdf.ts retains the conservative 'Auftragnehmer'.length-based upper bound. This is intentionally not updated — it is a space-reservation constant, not a label assertion — and is correct as-is.


No Findings

  • No CSS custom properties, design tokens, or hardcoded values introduced (pre-existing #991b1b/#6b7280/#d1d5db constants in the PDF pipeline are unchanged and out of scope for this PR)
  • No web UI component changes, dark-mode changes, or accessibility regressions
  • No component reuse violations (pdfmake pipeline, not a web component)

Verdict: APPROVED. Both fixes are minimal, well-targeted, and correctly verified by the existing real-render test suite.

@steilerDev

Copy link
Copy Markdown
Owner Author

[product-architect] Architecture review — APPROVED (no critical/high findings)

Both fixes are minimal, contract-consistent, and pinned by tests I ran locally against this branch:
npx jest realRender -t '#1937' → 5 passed (including the two real-render positions.length === 1
assertions), npx jest reportPdf/merge.test -t 'pdfmake header callback' → 1 passed. Static Analysis,
Docker, Trailer Check, and E2E Smoke are green; unit shards were still running at review time.

Verified

#1938 (merge.ts:131) — correct and complete against all 8 ACs:

  • AC2/AC3 hold by construction: the new template literal is character-identical to the page-1
    source-info block (overviewPdf.ts:531), so the value and the label: value separator/ordering
    cannot diverge.
  • AC4/AC5 hold: both halves now come from the report content model
    (labels.generatedAtreportT, sourceInfo.generatedAtTextreportFormatters), so the
    interface t is no longer in the path — the feat(reports): editable HTML report preview with on-demand PDF export #1909 artifact-content rule is satisfied. The
    merge.test.ts assertion genuinely discriminates this: the mocked interface t returns the bare
    key, so a regression to t() fails the test rather than silently passing.
  • AC6 (fits PAGE_TOP_MARGIN) — analysed, no risk: headerFootprint() models only the left stack
    (title line + two subheader lines = 57.2pt) plus the 20pt block margin. The generated-at line is
    the second child of buildPageHeader's two-column node at implicit '*' width (~257pt on A4
    with 40pt side margins) in small style, so Erstellt am: 15. Januar 2026 sits on one line — and
    even a two-line wrap (~18pt) stays far below the left stack. PAGE_TOP_MARGIN = 93 is unaffected;
    no re-derivation needed.
  • AC7: page 1 still returns null; the page-1 block is untouched.

#1937 (de/budget.json) — correct against all 7 ACs:

  • AC1/AC2 are proven at the renderer level, not by arithmetic: the updated HIGH1 tests render the
    real pipeline in de and assert each header cell resolves to exactly one line. Good choice of
    assertion — it is the property the AC actually states.
  • AC6 checked: the only consumers of these two keys are overviewPdf.ts (the PDF) and
    ReportContentEditor.tsx (the wizard's <th> preview, mobile card captions, and column-toggle
    labels). ReportContentLabels is derived from reportT and is not user-editable, and the editor
    is deliberately a mirror of the PDF — so the shortened form is not only acceptable there, keeping
    it identical is required. No separate PDF-only key needed.
  • The wordBreak: 'break-all' mechanism stays covered: overviewPdf.test.ts:833-861 builds its
    labels as hardcoded fixtures rather than reading the de bundle, so shortening the live
    translations did not silently drop coverage of buildHeaderCell's per-token path. That path must
    stay — vendor data still depends on it (Report PDF: German table header labels break mid-word (Auftragnehmer, Rechnungsbetrag) #1937's own out-of-scope note).
  • No E2E breakage: the Auftragnehmer assertions in e2e/tests/i18n/i18n.spec.ts:266-275 target
    the Vendors page heading and sub-nav (different keys), which are unchanged.

No schema, API-contract, or architecture-page change is implied by this PR.

Findings (all non-blocking)

M1 — realRender.test.ts:146-153 is a forked copy of the production header callback and was not
updated.
The helper still passes t('sourceReports.table.generatedAt') — the exact expression this
PR removed from production. Its own doc comment claims parity ("imported from
pageGeometry.ts/merge.ts, never hand-copied — #1929 AC11 requires the render to match production's
actual page setup"), and that claim is now false for the header's content. Consequence: every
multi-page real-render test — including the 3-page long-sourceName test at ~L2650 that explicitly
reasons about running-header clipping — measures a shorter header string than production emits.
Not blocking, because the AC6 geometry analysis above shows the margin is dominated by the left
stack, but this is precisely the forked-callback drift that hides header bugs. Suggested fix: build
the same ${content.labels.generatedAt}: ${content.sourceInfo.generatedAtText} string in the helper
(ideally by passing content instead of { tableTitle, sourceName }).

M2 — the new AC7 length-bound tests introduce a second, weaker width model than production's.
Production's header threshold is safeTokenChars(VENDOR_WIDTH, HEADER_WORST_CASE_CHAR_WIDTH_PT) =
floor(45 / 10.4) = 4 chars — i.e. buildHeaderCell still classifies "Firma" (5 chars) as an
oversized token and applies wordBreak: 'break-all' to it. The test derives 8/9 from a
5.19pt/char average advance. Two consequences worth correcting:

  • The bound is not an upper bound in the worst-case sense. An 8-character label made of wide glyphs
    passes length <= 8 and still breaks in the PDF, so the test can green-light the very regression
    it exists to catch.
  • The comment "will fit without wrapping even at the AVERAGE glyph width" overstates what an average
    buys — an average advance says nothing about a specific string.

The load-bearing guard is the positions.length === 1 real-render assertion, which is correct and
sufficient. Suggest either dropping the arithmetic (keeping the exact-value pins, which are the part
that actually catches a translation change) or re-deriving the bound from the production constants
and labelling it explicitly as an average-case tripwire rather than a proof.

M3 — stale cross-references, now that the two DE strings no longer exist. No behavioural impact,
but this is the failure mode that got #1965 filed (a comment asserting a removal was permanent
outliving the removal). Four sites:

  1. overviewPdf.ts:397-398buildHeaderCell's docstring still presents "Auftragnehmer" /
    "Rechnungsbetrag" as the live DE labels motivating the mechanism. This one matters most: a
    future reader who greps the DE bundle, finds neither string, and concludes the mechanism is dead
    will remove protection that vendor data and other locales still need. Needs an explicit
    "(Report PDF: German table header labels break mid-word (Auftragnehmer, Rechnungsbetrag) #1937: the DE labels were since shortened to Firma/Betrag; the mechanism is retained for
    free-form vendor data and future locales)".
  2. overviewPdf.ts:429-446VENDOR_HEADER_WORST_CASE_LINES still derives from
    'Auftragnehmer'.length. Keep the value (conservative, only ever subtracted from a height
    budget, so over-reserving cannot invert a safety check — and Report PDF: German table header labels break mid-word (Auftragnehmer, Rechnungsbetrag) #1937 forbids touching these
    constants), but the docstring's basis, "the narrowest fixed column holding its longest bare
    header word", is now a synthetic worst case rather than an observed one. Annotate.
  3. overviewPdf.test.ts:843,854 — the [HIGH1] test titles describe those strings as the German
    header labels. Retitle as synthetic over-wide tokens; the fixtures themselves are fine.
  4. realRender.test.ts:2040-2053 — the "AUTHORITATIVE measurement … 45.8125pt" comment cites the
    "Auftragnehmer" wrap as its basis. The number itself is probably still right (the header row's
    height is set by the tallest cell, and "Zugeordneter Betrag" still wraps to two lines per the
    test right above), but the stated basis is gone. Re-confirm or annotate.

M4 — glossary divergence is undocumented where a future audit will look (product-owner's AC3
call).
client/src/i18n/glossary.json:10 maps VendorAuftragnehmer, and AC3 requires
consistency with the glossary. "Firma" is a different noun rather than an abbreviation of the
glossary term, and the rationale currently lives only in translator agent memory. A later
glossary-compliance audit sees an unannotated divergence and may "correct" it back, silently
reintroducing #1937. Recommend a note/exception entry in glossary.json recording that
sourceReports.table.vendor is a width-constrained PDF column-header short form while
Auftragnehmer remains canonical everywhere else. Term acceptability itself is product-owner /
translator territory — flagging, not adjudicating.

L5 — expect(...).toEqual(1) on a number (realRender.test.ts, two places): prefer toBe.

L6 — follow-up worth filing: merge.ts:134 has the same defect #1938 just fixed, one line below.
buildPageFooter(t('sourceReports.table.pageLabel')) resolves the page label through the interface
t, so with interface DE / report EN the footer reads Seite 2 / 5 under an English report — a
mixed-language line on every page of every report, exactly the class of bug #1938 AC4/AC5 exists to
close. ReportContentLabels has no pageLabel field today, so fixing it needs a new label sourced
from reportT; genuinely out of scope for #1938, but it should be an issue rather than a loose end.

Mine to do (no action for this PR): ADR-034's generalizable-rules list should gain a B-rule for
the constraint this PR establishes — a fixed-width PDF column imposes a hard per-locale character
budget on its header key, the break-all fallback is a last resort rather than the fix, and the
guard is a real-render single-line assertion — plus the companion rule that running headers and
footers source every string from the report content model, never the interface t. I own
ADR-034 and will land it separately rather than making a two-string bug fix carry a wiki
submodule bump.

…and harness drift

- client-pdf-pipeline.md: fixed-width columns impose a per-locale character
  budget on sourceReports.table.* header keys; break-all is the fallback, a
  shorter DE label is the fix; running header/footer must resolve through
  reportT/reportFormatters (merge.ts:134 footer still violates this)
- recurring-patterns.md: realRender.test.ts forks merge.ts's docDefinition
  callbacks; a test-derived width bound looser than the production threshold
  greenlights the regression it guards
- story-reviews.md: PR #1982 verdict, findings, and the owed ADR-034 B-rule

Co-Authored-By: Claude product-architect <noreply@anthropic.com>
@steilerDev
steilerDev merged commit ed2b7b3 into beta Aug 4, 2026
31 checks passed
@steilerDev
steilerDev deleted the fix/1937-1938-pdf-header-wordbreak branch August 4, 2026 07:24
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.13.1-beta.5 🎉

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