From 1ed3a239b6877cd605697022abc458f0bbcaa9af Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Tue, 4 Aug 2026 09:02:26 +0200 Subject: [PATCH 1/3] test(reports): pin running-header value and DE column-fit labels (#1937/#1938) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- client/src/lib/reportPdf/merge.test.ts | 6 +- client/src/lib/reportPdf/realRender.test.ts | 79 +++++++++++++++++---- 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/client/src/lib/reportPdf/merge.test.ts b/client/src/lib/reportPdf/merge.test.ts index cca87e684..8a1edb978 100644 --- a/client/src/lib/reportPdf/merge.test.ts +++ b/client/src/lib/reportPdf/merge.test.ts @@ -680,7 +680,7 @@ describe('generateReportPdf', () => { expect(result.blob).toBeInstanceOf(Blob); }); - it('pdfmake header callback omits the header on page 1, renders it on subsequent pages, and reads title/sourceName from reportContent', async () => { + it('pdfmake header callback omits the header on page 1, renders it on subsequent pages, and reads title/sourceName from reportContent including the generatedAt value', async () => { const invoice = makeInvoice(); const report = makeReport([invoice]); const content = makeContent({ @@ -705,10 +705,12 @@ describe('generateReportPdf', () => { const sharedModule = (await import('./shared.js')) as unknown as { buildPageHeader: jest.Mock; }; + // #1938: the third arg is "label: value", not the bare i18n key — the label cannot silently + // lose its value again. labels.generatedAt='Generated At', generatedAtText='01/15/2026'. expect(sharedModule.buildPageHeader).toHaveBeenCalledWith( 'My Title', 'My Source', - 'sourceReports.table.generatedAt', + 'Generated At: 01/15/2026', ); // [regression #1929] On current beta this is the hardcoded [40, 40, 40, 60] — the top margin diff --git a/client/src/lib/reportPdf/realRender.test.ts b/client/src/lib/reportPdf/realRender.test.ts index 613ec0409..e203bf75e 100644 --- a/client/src/lib/reportPdf/realRender.test.ts +++ b/client/src/lib/reportPdf/realRender.test.ts @@ -1958,7 +1958,7 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { return { headerRow: tableItem.table.body[0]! }; } - it('[HIGH1] "Auftragnehmer" (vendor header, real German) and "Rechnungsbetrag" (invoiceAmount header) render without throwing, full text recoverable, and both genuinely wrap to multiple lines (their real measured widths — 67.50pt/78.66pt — exceed their 45pt/48pt columns even at real, not just worst-case, glyph metrics)', async () => { + it('[HIGH1] "Firma" (vendor header, real German #1937 fix) and "Betrag" (invoiceAmount header) render without throwing, full text recoverable, and both fit their columns in a single rendered line', async () => { const { headerRow } = await renderGermanHeaderRow('budget-overview'); const vendorHeader = headerRow[0] as { text: unknown; positions?: { pageNumber: number }[] }; const invoiceAmountHeader = headerRow[4] as { @@ -1966,16 +1966,18 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { positions?: { pageNumber: number }[]; }; - expect(usageCellText(vendorHeader.text)).toBe('Auftragnehmer'); - expect(usageCellText(invoiceAmountHeader.text)).toBe('Rechnungsbetrag'); + // #1937: DE labels changed from "Auftragnehmer"/"Rechnungsbetrag" (too wide) to + // "Firma"/"Betrag" (fit their 45pt/48pt columns at real Roboto glyph metrics). + expect(usageCellText(vendorHeader.text)).toBe('Firma'); + expect(usageCellText(invoiceAmountHeader.text)).toBe('Betrag'); - // Both are single unbroken words wider than their column even at REAL (not worst-case) - // metrics, per the architect's own measurement — so both must genuinely wrap across - // multiple rendered lines, not merely carry the flag without needing it. + // Both new labels are short enough to fit their columns without wrapping — each renders + // as exactly 1 line. This is the regression guard: a future DE translation that reintroduces + // a wide single-token word would push positions.length above 1. expect(vendorHeader.positions).toBeDefined(); - expect(vendorHeader.positions!.length).toBeGreaterThan(1); + expect(vendorHeader.positions!.length).toEqual(1); expect(invoiceAmountHeader.positions).toBeDefined(); - expect(invoiceAmountHeader.positions!.length).toBeGreaterThan(1); + expect(invoiceAmountHeader.positions!.length).toEqual(1); }); it('[HIGH1] "Zugeordneter Betrag" (allocatedAmount header, 75pt column) is NOT force-broken mid-character — it renders as exactly 2 lines (one word per line, wrapped at the natural space), proving the conservative per-token flag on "Zugeordneter" never actually needed to invoke a mid-character split', async () => { @@ -1995,12 +1997,12 @@ describe('report PDF pipeline — real, unmocked end-to-end render', () => { expect(allocatedHeader.positions!.length).toBe(2); }); - it('[HIGH1] the claim (6-column) shape header row also renders "Auftragnehmer"/"Rechnungsbetrag" without throwing and with full text recoverable — the same protection applies regardless of table shape', async () => { + it('[HIGH1] the claim (6-column) shape header row also renders "Firma"/"Betrag" without throwing and with full text recoverable — the same protection applies regardless of table shape (#1937)', async () => { const { headerRow } = await renderGermanHeaderRow('claim'); const vendorHeader = headerRow[0] as { text: unknown }; const invoiceAmountHeader = headerRow[3] as { text: unknown }; // no status column in claim shape - expect(usageCellText(vendorHeader.text)).toBe('Auftragnehmer'); - expect(usageCellText(invoiceAmountHeader.text)).toBe('Rechnungsbetrag'); + expect(usageCellText(vendorHeader.text)).toBe('Firma'); + expect(usageCellText(invoiceAmountHeader.text)).toBe('Betrag'); }); }); @@ -2815,7 +2817,8 @@ describe('production i18n singleton — getFixedT resolves a language independen expect(i18n.language).toBe('en'); const fixedDe = i18n.getFixedT('de', 'budget'); - expect(fixedDe('sourceReports.table.vendor')).toBe('Auftragnehmer'); + // #1937: DE vendor label changed from "Auftragnehmer" to "Firma" (fits the 45pt column). + expect(fixedDe('sourceReports.table.vendor')).toBe('Firma'); expect(fixedDe('sourceReports.download')).toBe('PDF herunterladen'); // Calling getFixedT for a different locale must not mutate the singleton's own active @@ -2926,3 +2929,55 @@ describe('production i18n singleton — getFixedT resolves a language independen expect(allocatedCell.text[1]!.text).toBe(' (Abschlagszahlung)'); }); }); + +// ─── #1937: DE header-label column-fit pin (AC7) ───────────────────────────────────────────────── +// +// Pins the DE label lengths for the two narrow fixed-width columns whose German translations +// previously overflowed: Vendor (45pt column) and Invoice Amount (48pt column). +// +// Bound derivation: the Roboto font's measured average character advance at 10pt bold +// (the table header font) is 5.19pt/char — derived from "Auftragnehmer" (13 chars) measuring +// 67.50pt in a real render, giving 67.50 / 13 = 5.19pt/char. That yields practical column +// capacities of floor(45 / 5.19) = 8 chars for Vendor and floor(48 / 5.19) = 9 chars for +// Invoice Amount. A single-token DE label shorter than these bounds will fit without wrapping +// even at the AVERAGE glyph width, not just the conservatively wide worst-case metric. +// +// The HIGH1 tests above exercise the same labels via a full real pdfmake render and assert +// that each header cell resolves to exactly 1 rendered line — a stronger, renderer-level proof +// of the same property. The length assertions here are a cheap structural guard: if a future +// translation lands a wider single-token word, the length check fires immediately without +// needing the full pdfmake render cycle. +describe('#1937 AC7: DE header labels fit their narrow fixed-width columns (column-fit pin)', () => { + // Measured average glyph advance at 10pt bold Roboto (derived from "Auftragnehmer" real render: + // 67.50pt / 13 chars = 5.19pt/char). Used to compute realistic per-column character capacities. + const AVG_CHAR_WIDTH_PT = 5.19; + const VENDOR_WIDTH_PT = 45; + const INVOICE_AMOUNT_WIDTH_PT = 48; + // A single-token label this length or shorter fits without pdfmake needing to word-wrap it. + const VENDOR_COLUMN_CHAR_CAPACITY = Math.floor(VENDOR_WIDTH_PT / AVG_CHAR_WIDTH_PT); // 8 + const INVOICE_AMOUNT_COLUMN_CHAR_CAPACITY = Math.floor( + INVOICE_AMOUNT_WIDTH_PT / AVG_CHAR_WIDTH_PT, + ); // 9 + + it('DE vendor label ("Firma") is at most VENDOR_COLUMN_CHAR_CAPACITY chars — fits the 45pt column without wrapping', () => { + // Uses the isolated i18next instance loaded with the real de/budget.json bundle (see beforeAll + // at the top of this file). The value must match the production JSON exactly. + const deVendorLabel = tDe('sourceReports.table.vendor'); + expect(deVendorLabel).toBe('Firma'); // exact current value pin + expect(deVendorLabel.length).toBeLessThanOrEqual(VENDOR_COLUMN_CHAR_CAPACITY); + }); + + it('DE invoiceAmount label ("Betrag") is at most INVOICE_AMOUNT_COLUMN_CHAR_CAPACITY chars — fits the 48pt column without wrapping', () => { + const deInvoiceAmountLabel = tDe('sourceReports.table.invoiceAmount'); + expect(deInvoiceAmountLabel).toBe('Betrag'); // exact current value pin + expect(deInvoiceAmountLabel.length).toBeLessThanOrEqual(INVOICE_AMOUNT_COLUMN_CHAR_CAPACITY); + }); + + it('EN vendor and invoiceAmount labels are unchanged from their baseline values', () => { + // EN labels are stable reference points: "Vendor" (6 chars, fits the 45pt column); + // "Invoice Amount" (14 chars) has an internal space so pdfmake wraps at the word boundary — + // it never needs break-all and is not subject to the single-token width constraint. + expect(tEn('sourceReports.table.vendor')).toBe('Vendor'); + expect(tEn('sourceReports.table.invoiceAmount')).toBe('Invoice Amount'); + }); +}); From 7377b4f2dd7331c3353270ca90847589028b213f Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Tue, 4 Aug 2026 09:04:34 +0200 Subject: [PATCH 2/3] fix(reports): complete running-header timestamp and German word-break (#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 Co-Authored-By: Claude translator --- .../pr-1937-1938-pdf-header-labels.md | 57 +++++++++++++++++++ .claude/agent-memory/translator/MEMORY.md | 11 ++++ client/src/i18n/de/budget.json | 4 +- client/src/lib/reportPdf/merge.ts | 2 +- 4 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 .claude/agent-memory/qa-integration-tester/pr-1937-1938-pdf-header-labels.md diff --git a/.claude/agent-memory/qa-integration-tester/pr-1937-1938-pdf-header-labels.md b/.claude/agent-memory/qa-integration-tester/pr-1937-1938-pdf-header-labels.md new file mode 100644 index 000000000..11f42e3ed --- /dev/null +++ b/.claude/agent-memory/qa-integration-tester/pr-1937-1938-pdf-header-labels.md @@ -0,0 +1,57 @@ +--- +name: pr-1937-1938-pdf-header-labels +description: #1937/#1938 PDF running-header value and DE column-fit label test updates (2026-08-04) +metadata: + type: project +--- + +## Fix 1 — #1938: Running header now shows label + value + +**Production change**: `merge.ts` line 131 changed from +`t('sourceReports.table.generatedAt')` (i18n key only) to +`` `${reportContent.labels.generatedAt}: ${reportContent.sourceInfo.generatedAtText}` `` + +**Test updated**: `merge.test.ts` — the header-callback assertion changed from +`'sourceReports.table.generatedAt'` to `'Generated At: 01/15/2026'`. +`makeContent()` has `labels.generatedAt: 'Generated At'` and `generatedAtText: '01/15/2026'`. +**Why:** The bare i18n key assertion let the value silently disappear again. + +## Fix 2 — #1937: DE header labels fit their columns + +**Production change**: `de/budget.json` `sourceReports.table.vendor`: +`"Auftragnehmer"` → `"Firma"` (5 chars, fits 45pt); +`sourceReports.table.invoiceAmount`: `"Rechnungsbetrag"` → `"Betrag"` (6 chars, fits 48pt) + +**Pre-existing tests that were BROKEN by the translation change and needed updating:** + +1. `realRender.test.ts` HIGH1 budget-overview test (was asserting 'Auftragnehmer'/'Rechnungsbetrag') + - The old test also asserted `positions.length > 1` (multi-line wrap). The new short words + render in 1 line, so assertions changed to `toEqual(1)`. +2. `realRender.test.ts` HIGH1 claim (6-col) test — same label updates. +3. `realRender.test.ts` production singleton describe (line ~2818) — 'Auftragnehmer' → 'Firma'. + +**New tests added**: AC7 describe block at the end of `realRender.test.ts`: +- Length bounds: `content.labels.vendor.length <= 8`, `content.labels.invoiceAmount.length <= 9` + (derived from 5.19pt/char measured Roboto average advance at 10pt bold) +- Exact value pins: `tDe('...vendor') === 'Firma'`, `tDe('...invoiceAmount') === 'Betrag'` +- EN stability: `tEn('...vendor') === 'Vendor'`, `tEn('...invoiceAmount') === 'Invoice Amount'` + +## `VENDOR_HEADER_WORST_CASE_LINES` — leave as-is + +`overviewPdf.ts` still uses `'Auftragnehmer'.length` (13 chars) to compute `VENDOR_HEADER_WORST_CASE_LINES`. +This is the **designed worst-case upper bound** for space reservation — intentionally conservative, +independent of the current DE translation. Do not change it. + +## Pattern: update ALL stale translation-value assertions when DE label changes + +When a DE translation key changes, grep realRender.test.ts for the OLD string value — there are +typically 3+ places (HIGH1 tests + production singleton describe). All must be updated together +or tests fail at a confusing set of locations. + +## Column-fit math reference + +- Roboto 10pt bold average advance: 5.19pt/char (measured: "Auftragnehmer" 67.50pt / 13 chars) +- VENDOR_WIDTH (45pt) / 5.19 = 8.67 → floor = 8 chars +- INVOICE_AMOUNT_WIDTH (48pt) / 5.19 = 9.25 → floor = 9 chars +- Labels with a space (e.g. "Invoice Amount") are NOT subject to single-token width constraint — + pdfmake wraps at word boundaries, no break-all needed. diff --git a/.claude/agent-memory/translator/MEMORY.md b/.claude/agent-memory/translator/MEMORY.md index 667997067..87be77e38 100644 --- a/.claude/agent-memory/translator/MEMORY.md +++ b/.claude/agent-memory/translator/MEMORY.md @@ -106,6 +106,17 @@ New `sourceReports.expand.*` (chevron-expand sub-tables for budget lines + depos - [Audit pitfalls](audit-pitfalls.md) — incident history behind the mandatory 4-step full-coverage audit protocol: a parity-only audit missed 13 code-referenced keys (Area UI raw-key bug); loose substring greps flagged 52 false positives +## PDF Column Header Short Forms Under Width Constraint (Issue #1937, 2026-08-04) + +`sourceReports.table.vendor` ("Auftragnehmer", 13 chars, 67.5pt) overflows its 45pt column. `sourceReports.table.invoiceAmount` ("Rechnungsbetrag", 15 chars, 78.66pt) overflows its 48pt column. Font: Roboto Bold 10pt, avg ~5.19pt/char from "Auftragnehmer" measurement. + +Fixes applied (following the Abschlag measured-space-constraint precedent): + +- `vendor`: "Auftragnehmer" → **"Firma"** (5 chars, ~26pt). Rationale: no standard German abbreviation of "Auftragnehmer" fits within 8 chars without ambiguity ("Auftr." could be Auftraggeber). "Firma" (company/firm) is universally clear to any German bank employee; column content (actual company names) makes context self-evident. Glossary note: this is a PDF column-header short form under a measured constraint — "Auftragnehmer" remains the canonical term everywhere else. +- `invoiceAmount`: "Rechnungsbetrag" → **"Betrag"** (6 chars, ~27pt). Rationale: no abbreviation of "Rechnungsbetrag" fits in 9 chars in a `Rechnungsnr.`-style form. "Betrag" (amount) is universally clear; it is unambiguous adjacent to "Zugeordneter Betrag" (allocated amount column), which remains unchanged per AC5. + +General rule: when a glossary term overshoots a measured PDF column, prefer the shortest universally-understood German synonym or generic noun over a coined abbreviation that lacks standard status. + ## Cover Letter Signature Block Keys (Issue #1932, 2026-08-02) `sourceReports.editable.signatureLabel` → "Unterschrift"; `sourceReports.coverLetter.closing` → "Mit freundlichen Grüßen,"; `sourceReports.editable.closingLabel` → "Grußformel". Confirmed: neither "signature" nor "closing salutation" belongs in the glossary (grep across `glossary.json` for signature/closing/Gruß terms found nothing, and these are generic letter-writing vocabulary, not Cornerstone domain terms) — did not add. diff --git a/client/src/i18n/de/budget.json b/client/src/i18n/de/budget.json index 559ef39b5..834e9fc5d 100644 --- a/client/src/i18n/de/budget.json +++ b/client/src/i18n/de/budget.json @@ -1239,11 +1239,11 @@ "reference": "Referenz", "generatedAt": "Erstellt am", "pageLabel": "Seite", - "vendor": "Auftragnehmer", + "vendor": "Firma", "invoiceNumber": "Rechnungsnr.", "date": "Datum", "status": "Status", - "invoiceAmount": "Rechnungsbetrag", + "invoiceAmount": "Betrag", "allocatedAmount": "Zugeordneter Betrag", "total": "Gesamt", "refundNote": "(Rückerstattung)", diff --git a/client/src/lib/reportPdf/merge.ts b/client/src/lib/reportPdf/merge.ts index a57a2fc99..6d42ec1d0 100644 --- a/client/src/lib/reportPdf/merge.ts +++ b/client/src/lib/reportPdf/merge.ts @@ -128,7 +128,7 @@ export async function generateReportPdf( return buildPageHeader( reportContent.tableTitle, reportContent.sourceInfo.sourceName, - t('sourceReports.table.generatedAt'), + `${reportContent.labels.generatedAt}: ${reportContent.sourceInfo.generatedAtText}`, ); }, footer: buildPageFooter(t('sourceReports.table.pageLabel')), From 550808df1e76bae241fdd0805268ec3d1b23dec1 Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Tue, 4 Aug 2026 09:13:27 +0200 Subject: [PATCH 3/3] =?UTF-8?q?docs(memory):=20record=20PR=20#1982=20revie?= =?UTF-8?q?w=20=E2=80=94=20PDF=20header=20label=20width=20budget=20and=20h?= =?UTF-8?q?arness=20drift?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../agent-memory/product-architect/MEMORY.md | 2 +- .../product-architect/client-pdf-pipeline.md | 52 ++++++++++++++++--- .../product-architect/recurring-patterns.md | 25 +++++++-- .../product-architect/story-reviews.md | 23 ++++++++ 4 files changed, 92 insertions(+), 10 deletions(-) diff --git a/.claude/agent-memory/product-architect/MEMORY.md b/.claude/agent-memory/product-architect/MEMORY.md index 96cac9dfa..a4d656cae 100644 --- a/.claude/agent-memory/product-architect/MEMORY.md +++ b/.claude/agent-memory/product-architect/MEMORY.md @@ -6,7 +6,7 @@ - [Dual-rail aggregation](dual-rail-aggregation.md) — Rail A/B tagged-deposit invariants (#1891/PR #1894), residual-denominator rule, isSplit UNION - [Source-report split inference](source-report-split-inference.md) — budgetLines[]/deposits[] are this-source-scoped, so †/‡ classification is a proxy; proposed `splitKind`; pdfmake `'2*'` width trap - [Story reviews](story-reviews.md) — per-story and per-PR review log -- [Client PDF pipeline](client-pdf-pipeline.md) — ADR-034 report PDF generation, reportContent content/layout split (#1900), `dontBreakRows` silent-drop rule, document-level deduplicated legend (#1965) — ADR-034 B4 rule + legend addendum + Deviation Log landed in PR #1979, discharging the #1959 debt +- [Client PDF pipeline](client-pdf-pipeline.md) — ADR-034 report PDF generation, reportContent content/layout split (#1900), `dontBreakRows` silent-drop rule, document-level deduplicated legend (#1965) — ADR-034 B4 rule + legend addendum landed in PR #1979; per-locale header character budget + "no interface `t` in header/footer" (#1937/#1938, PR #1982) — **ADR-034 B-rule addendum still owed** - [Diary drafts pattern](diary-drafts-pattern.md) — ADR-022 draft lifecycle via status column on parent table - [EPIC-03 refinement](epic03-refinement.md) — 40 consolidated refinement items - [EPIC-04 household items](epic04-household-items.md) · [EPIC-05 budget](epic05-budget.md) · [EPIC-17 i18n](epic17-i18n.md) · [EPIC-18 areas & trades](epic18-areas-trades.md) diff --git a/.claude/agent-memory/product-architect/client-pdf-pipeline.md b/.claude/agent-memory/product-architect/client-pdf-pipeline.md index c44827a07..1268cbbcc 100644 --- a/.claude/agent-memory/product-architect/client-pdf-pipeline.md +++ b/.claude/agent-memory/product-architect/client-pdf-pipeline.md @@ -290,15 +290,15 @@ Regression to guard when adding a flag type: emitting one entry per flagged row. The ADR-034 debt owed since #1959 is now paid. Two structurally different note kinds share the block below the overview table and must never share a numbering scheme: -| Kind | Marker | Cardinality | Built by | -| --- | --- | --- | --- | -| Skipped-document note | `*N`, numbered, referenced by the owning row | one per skipped document | `overviewPdf.ts` at generation time (not in `ReportContent`) | -| Legend entry (`content.footnotes[]`) | repeated inline word label — `partial`, `less deposit` | **at most one per flag type per document** | `buildReportContent.ts` | +| Kind | Marker | Cardinality | Built by | +| ------------------------------------ | ------------------------------------------------------ | ------------------------------------------ | ------------------------------------------------------------ | +| Skipped-document note | `*N`, numbered, referenced by the owning row | one per skipped document | `overviewPdf.ts` at generation time (not in `ReportContent`) | +| Legend entry (`content.footnotes[]`) | repeated inline word label — `partial`, `less deposit` | **at most one per flag type per document** | `buildReportContent.ts` | B4's old generalized rule ("every footnote is referenced from the row that owns it") applied only to the numbered kind and was reworded. Invariants now recorded in the ADR's legend addendum: -- `footnotes[].marker` is `sourceReports.table.{split,depositReduced}InlineLabel` — the *same* keys as +- `footnotes[].marker` is `sourceReports.table.{split,depositReduced}InlineLabel` — the _same_ keys as `labels.{splitNote,depositReducedNote}` and as the inline label the row cell prints. Row↔legend joins by **repetition of that literal**, not by id/index/number. NBSP in `less deposit` / `abzgl. Abschlag` is load-bearing; `expect(footnotes[0].marker).toBe(content.labels.splitNote)` is the assertion that pins it. @@ -307,6 +307,46 @@ numbered kind and was reworded. Invariants now recorded in the ADR's legend adde - Adding a flag type = new `Set` + `size > 0` push in `buildReportContent.ts`, new boolean on `ReportContentRow`, new inline label in `overviewPdf.ts`. Assert exact `footnotes.length` (not `>= 1`) on a fixture where several rows share a flag. -- Preview/export parity trap: once markers became *words*, `ReportContentEditor`'s +- Preview/export parity trap: once markers became _words_, `ReportContentEditor`'s `{marker}:{text}` ran them together while the PDF used `${marker}: ${text}`. Fixed in #1979 — any change to either surface must keep the separator identical. + +## Fixed-width column headers impose a per-locale character budget (#1937/#1938, PR #1982) + +The overview table's columns are fixed-width (`VENDOR_WIDTH = 45`, `INVOICE_AMOUNT_WIDTH = 48`, …) and +pdfmake's `elasticWidth` never grows a fixed column to fit its own header. So **every DE translation of a +`sourceReports.table.*` header key is width-constrained**, and DE is always the binding locale. + +- `buildHeaderCell` applies `buildUsageTextRuns` (per-token `wordBreak: 'break-all'`) to every header cell. + That is a *last-resort* fallback (pdfmake 0.3.x has no hyphenation), not the fix: a mid-word break with + no hyphen on a bank-facing document is a defect in its own right. The fix is a shorter localized label. +- #1937 shortened `vendor` `Auftragnehmer` → `Firma` and `invoiceAmount` `Rechnungsbetrag` → `Betrag`. + The break-all mechanism **must stay** — vendor *data* (server cap 200 chars, German compounds) still + needs it, and #1937 explicitly accepted broken vendor names as unfixable without a layout change. +- Correct guard: a real-render assertion that the header cell resolves to `positions.length === 1` in the + `de` locale. Character-count arithmetic is a weaker proxy (see recurring-patterns.md). +- `overviewPdf.test.ts:833-861` and `VENDOR_HEADER_WORST_CASE_LINES` use hardcoded `'Auftragnehmer'` + fixtures/literals, *not* the live bundle — so they survive translation changes, but their comments and + test titles rot into claiming to describe the live DE labels. +- Consumers of `labels.*`: `overviewPdf.ts` (PDF) and `ReportContentEditor.tsx` (`` preview, mobile + card captions, column-toggle text). `ReportContentLabels` is `reportT`-derived and **not user-editable**, + so a shortened label is safe — and must be identical in both surfaces by design. +- Glossary tension: `glossary.json` maps `Vendor` → `Auftragnehmer`. PDF column-header short forms diverge + from glossary terms under a measured constraint; that exception needs recording *in glossary.json*, not + just in translator memory, or an audit reverts it. + +### Running header/footer must source strings from the report content model + +`merge.ts`'s `header:` callback took the interface `t` for the generated-at label and never passed the +value (#1938) — a bare label on pages 2+ of every multi-page report. Fixed in PR #1982 to +`` `${reportContent.labels.generatedAt}: ${reportContent.sourceInfo.generatedAtText}` ``, byte-identical to +the page-1 block in `overviewPdf.ts:531`. Rule (from #1909): **artifact content resolves through +`reportT`/`reportFormatters`; only edit affordances use the interface `t`.** + +- **Still violating it: `merge.ts:134`** — `buildPageFooter(t('sourceReports.table.pageLabel'))`. With + interface DE / report EN the footer reads `Seite 2 / 5` under an English report. Needs a new + `pageLabel` on `ReportContentLabels`; flagged as a follow-up in the PR #1982 review. +- Header height budget: `headerFootprint()` (`pageGeometry.ts`) models only the LEFT stack (title + + two-line subheader = 57.2pt) + 20pt block margin → `PAGE_TOP_MARGIN = 93`. The generated-at line is the + right child of a two-column node at implicit `'*'` (~257pt on A4) in `small` style, so appending the + value cannot threaten the margin — even a two-line wrap (~18pt) stays far under the left stack. diff --git a/.claude/agent-memory/product-architect/recurring-patterns.md b/.claude/agent-memory/product-architect/recurring-patterns.md index b6b21c190..6384e2f90 100644 --- a/.claude/agent-memory/product-architect/recurring-patterns.md +++ b/.claude/agent-memory/product-architect/recurring-patterns.md @@ -32,6 +32,25 @@ core formula against the original line by line — that divergence is where the `splitByDepositsExcludingTagged` (PR #1894), where the residual expression was the sole difference and the sole defect. Prefer an options flag over a fork; when a fork ships anyway, file the collapse follow-up. +### Forked *test harness* — `realRender.test.ts` re-implements merge.ts's docDefinition + +`renderOverviewPdfContent` (`client/src/lib/reportPdf/realRender.test.ts` ~L136-159) hand-copies +production's pdfmake `header:`/`footer:` callbacks while its own docstring claims parity with merge.ts +("never hand-copied — #1929 AC11"). It imports `pageMargins`/`styles` but forks the callbacks. PR #1982 +changed `merge.ts`'s header string and left the harness on the old expression, so every multi-page +real-render test (incl. the 3-page long-`sourceName` clipping test) measures a string production no +longer emits. **Whenever `merge.ts`'s docDefinition changes, grep this helper.** Fix direction: pass +`content` and build the same string, rather than re-deriving it. + +### Proxy bound looser than the production threshold it guards + +PR #1982's AC7 tests bound DE header labels at `floor(width / 5.19pt)` (an *average* glyph advance) — +8/9 chars — while production's own break trigger is `safeTokenChars(width, HEADER_WORST_CASE_CHAR_WIDTH_PT += 10.4pt)` = 4 chars. An 8-char wide-glyph label passes the test and still breaks in the PDF. When a test +re-derives a width/size bound instead of importing the production constant, check which direction the +error runs: a bound *looser* than production's greenlights the regression it exists to catch. The real +guard there is the renderer-level `positions.length === 1` assertion. + ## Test smells worth escalating in review - A combined-path test that places the two interacting entities on **different** parents proves nothing @@ -491,7 +510,7 @@ content — and check whether the replacement text preserves _meaning_ (`(abzgl. ## Enumerated multi-site doc fixes come back half-done (PR #1979 r2) -When a review finding names N sites for the same stale claim, expect the fix commit to update the *nearest* +When a review finding names N sites for the same stale claim, expect the fix commit to update the _nearest_ ones and miss the rest. #1979's HIGH 2 named four sites for "nothing populates `content.footnotes`"; the fix updated the field-declaration comment and the spec header (both adjacent to the changed assertions) and left the two class-docstring paragraphs — which contained the strongest form ("they can never be populated by the @@ -501,10 +520,10 @@ Two habits that follow: - **Re-grep the literal on re-review**, never trust the fix commit's diff to cover the enumeration. One `grep -n -i footnote e2e/pages/ReportWizardPage.ts` found both misses instantly. -- **Check the test *name*, not just the body.** #1979 inverted Scenario 18's assertions to `toHaveCount(1)` +- **Check the test _name_, not just the body.** #1979 inverted Scenario 18's assertions to `toHaveCount(1)` but left the Playwright title reading "and no footnote list anywhere on the page". A title that states the inverse of its body is worse than a stale comment: it renders that way in every CI report and is the first - artifact a future reader uses to conclude the *body* drifted. Same for the `// Scenario NN:` block header. + artifact a future reader uses to conclude the _body_ drifted. Same for the `// Scenario NN:` block header. Why this is worth blocking on (I did, r2): the POM class docstring is the contract the spec header points at ("See `ReportWizardPage.ts`'s class docstring for the full locator reference"), so a directive there plus a diff --git a/.claude/agent-memory/product-architect/story-reviews.md b/.claude/agent-memory/product-architect/story-reviews.md index a9c5d70dd..3ec67f91c 100644 --- a/.claude/agent-memory/product-architect/story-reviews.md +++ b/.claude/agent-memory/product-architect/story-reviews.md @@ -518,3 +518,26 @@ Open follow-ups I own or should file: - Pre-hydration toggle window (F4): editing before the mount fetch resolves discards stored prefs for the session. Practically unreachable; `usePreferences.isLoading` is available if it ever matters. - `isLoaded` is dead API surface — returned by the hook, not destructured by `DataTable.tsx:171-172`. + +## PR #1982 — #1937 (DE header word-break) + #1938 (running-header timestamp) — APPROVED + +Two-line production diff (`merge.ts` header string, two DE strings) plus test updates. Verified locally: +`npx jest realRender -t '#1937'` (5 passed, incl. the two `positions.length === 1` real-render assertions) +and `npx jest reportPdf/merge.test -t 'pdfmake header callback'`. Note the jest invocation trap here: +`--modulePathIgnorePatterns='/.claude/worktrees/'` matches the worktree's own rootDir and silently yields +"0 files checked across 3 projects" — drop it when running inside a worktree. + +AC6 of #1938 (header still fits `PAGE_TOP_MARGIN`) discharged by analysis, not a new test — see +client-pdf-pipeline.md for the footprint reasoning. AC4/AC5 are pinned discriminatingly because the mocked +interface `t` returns the bare key, so a regression to `t()` fails rather than passing. + +Findings, all non-blocking: M1 forked harness header callback; M2 average-vs-worst-case bound in the new +AC7 tests; M3 four stale `Auftragnehmer`/`Rechnungsbetrag` cross-references (the `buildHeaderCell` +docstring one matters — it could lead someone to delete break-all protection vendor *data* still needs); +M4 undocumented glossary divergence (`Vendor` → `Auftragnehmer` vs `Firma`); L6 follow-up: `merge.ts:134` +footer page label still uses the interface `t`. + +**Mine to do:** ADR-034 B-rule addendum — fixed-width columns impose a per-locale header character budget +(break-all is the fallback, a shorter label is the fix, real-render single-line assertion is the guard), +plus the companion rule that running headers/footers never use the interface `t`. Deliberately not made a +condition of this PR to avoid a wiki submodule bump on a two-string fix.