refactor(reports): remove TFunction from reportPdf/* (ADR-034 enforcement)

[steilerDev]

Summary

• Removed TFunction parameter from buildCoverLetterContent, buildOverviewContent, and generateReportPdf — these three functions in reportPdf/* now read all locale-resolved strings exclusively from ReportContentLabels
• Added coverLetterReferenceLabel, coverLetterSubjectLabel, and skipReasonLabels fields to ReportContentLabels; populated in buildReportContent.ts via reportT at the report-content layer where the locale is already resolved
• AC7 sentinel tests in coverLetterPdf.test.ts, overviewPdf.test.ts, and buildReportContent.test.ts discriminate: they fail if any string is sourced from TFunction instead of labels.*

Fixes #2001

Test plan

• Unit tests pass (95%+ coverage on all modified files)
• grep -rn "TFunction" client/src/lib/reportPdf/ --include="*.ts" | grep -v test returns no output
• Quality Gates green

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

🤖 Generated with Claude Code

[steilerDev]

[ux-designer]

Design Review — PR #2007 (refactor: remove TFunction from reportPdf/*)

Verdict: APPROVED — no design system concerns

Scope confirmed

This PR is a pure backend/library refactor with no visual surface area changes:

• ReportWizardPage.tsx — one call site simplified: reportT removed from generateReportPdf args and from the useCallback dep array. No JSX, no styling, no token usage changed.
• client/src/lib/reportPdf/*.ts — TFunction param removed from buildCoverLetterContent, buildOverviewContent, generateReportPdf. Labels now sourced from ReportContentLabels fields pre-populated by buildReportContent.ts via the same reportT() calls.
• client/src/lib/reportContent/types.ts and buildReportContent.ts — 3 new string fields added to ReportContentLabels (coverLetterReferenceLabel, coverLetterSubjectLabel, skipReasonLabels). Populated from the identical i18n keys that were previously called inline.

Design system checklist

Check	Result
Token adherence (hardcoded values)	No CSS changes of any kind
Dark mode correctness	Not applicable
Responsive behavior	Not applicable
Accessibility (ARIA, keyboard, contrast)	Not applicable
Shared component reuse	Not applicable — no component changes
Animation/transition	Not applicable
CSS Module conventions	Not applicable

Visual delta

Zero. The PDF output reads the same translated strings it always did — the refactor only moves where they are resolved (from deep inside reportPdf/ to upstream in reportContent/). No label text, spacing, layout, or visual property is changed.

[steilerDev]

[product-architect] Architecture review of PR #2007 (issue #2001) — CHANGES REQUESTED

The refactor itself is the right shape and the primary deliverable lands cleanly. Two blocking findings, both narrow.

Verified

• AC5 (the real deliverable) holds. No TFunction in any production file under client/src/lib/reportPdf/ on the head commit. Remaining hits are test-only, and realRender.test.ts's t: TFunction helper params are legitimate (they feed buildReportContent, which is supposed to take reportT).
• ADR-034 invariant-1 grep guard passes. Ran the wiki's own guard (useTranslation|/i18n/|Intl\.|toLocale) over reportPdf/ + reportContent/ production files: zero hits.
• AC1/AC3/AC4 correct. coverLetterReferenceLabel / coverLetterSubjectLabel / skipReasonLabels are populated once in buildReportContent.ts:300-305 via reportT, bound to the same budget namespace, with bare keys (invariant 2 respected). Signatures shed the param at coverLetterPdf.ts:8, overviewPdf.ts:491, merge.ts:35; ReportWizardPage.tsx:313 and its useCallback dep array updated in step, and reportT remains in use at lines 276/287 so nothing is left dangling.
• AC6 holds. All four keys pre-exist in both client/src/i18n/en/budget.json and de/budget.json — no new keys, no parity risk.
• AC7 sentinel blocks are structurally sound and do discriminate. coverLetterPdf.test.ts:306 and overviewPdf.test.ts:1637 assert unique sentinel values (LABEL-REF-SENTINEL, FETCH-SENTINEL) that the key-echo mock t cannot produce, so a revert to t('sourceReports.…') fails the assertion on value, independently of also failing to compile.
  • Non-blocking note for the record: every other skip-footnote assertion in overviewPdf.test.ts (lines 1140, 1149, 1479, 1499, 1515, 1535) expects the literal i18n key string, because makeLabels() uses the key-echo convention. Those are indistinguishable from a mock-t result and therefore contribute zero discrimination for this contract. The AC7 block is the only load-bearing coverage — it must not later be "harmonised" into the key-echo style.
• AC8 sweep is complete. Checked every file constructing a ReportContentLabels literal; all seven carry the three new fields. applyOverrides.ts / applyAiContent.ts never touch labels, so no production construction site was missed. Static Analysis (typecheck) and all gates are green.
• A previously-tracked defect is closed here. merge.ts's header: and footer: now both read from reportContent.labels; combined with AC5 the "interface t reachable from merge.ts" hazard is gone at the type level.

---

H1 (blocking) — the type assertion at overviewPdf.ts:835-838 reintroduces the exact class of hazard this story closes

const reasonLabel =
  reportContent.labels.skipReasonLabels[
    reason as 'footnoteFetchFailed' | 'footnoteInvalidPdf'
  ] ?? reason;

SkippedDocument.reason (reportPdf/types.ts:17) is already the exact union. It is widened to string only by an implementation detail — merge.ts:101's new Map<string, string[]>() — and then re-narrowed by assertion at the consumer. Consequences:

1. The union loses exhaustiveness. Adding a third reason to SkippedDocument.reason produces no compile error: not at the as, not at the hand-written two-field literal in buildReportContent.ts:302-305, not at the ReportContentLabels declaration. The ?? reason fallback then silently prints the raw identifier (footnoteWhateverNext) into a bank-facing PDF — which is precisely the "no key echoed into the PDF" outcome AC2 exists to prevent. The hazard has been moved from t() to a label map, not removed.
2. ?? reason is dead code today (the indexed access is typed string, non-optional), so it reads as a guard while guarding nothing — and it is exactly what would mask the regression above rather than surface it.
3. This is a compiler-enforcement PR. A widening-then-as-narrowing round trip is convention re-entering through the back door.

Fix (mechanical, ~6 lines of production change):

• client/src/lib/reportContent/types.ts: export type ReportSkipReason = 'footnoteFetchFailed' | 'footnoteInvalidPdf'; and change the field to skipReasonLabels: Record<ReportSkipReason, string>;
• client/src/lib/reportContent/index.ts: add ReportSkipReason to the export type { … } list.
• client/src/lib/reportPdf/types.ts:17: reason: ReportSkipReason; importing from '../reportContent/index.js'. Keep the dependency in this direction — reportPdf already consumes reportContent; do not have reportContent import from reportPdf.
• client/src/lib/reportPdf/merge.ts:101: new Map<string, ReportSkipReason[]>().
• client/src/lib/reportPdf/overviewPdf.ts:493: skippedDocuments: Map<string, ReportSkipReason[]>, and line 835 becomes const reasonLabel = reportContent.labels.skipReasonLabels[reason]; — no assertion, no ??.
• Test call sites: the eight new Map<string, string[]>([...]) literals in overviewPdf.test.ts / realRender.test.ts:1719 and the two as Map<string, string[]> casts in merge.test.ts:316,330. I checked all of them — none passes an out-of-union reason, so this is a pure type-annotation change with no assertion churn. The header comment at overviewPdf.test.ts:10 documents the old signature and needs the same edit.

After this, Record<ReportSkipReason, string> makes adding a reason a compile error at the population site in buildReportContent.ts, which is the enforcement AC5 was reaching for.

H2 (blocking) — ADR-034 now misdescribes the code this PR changed

wiki/ADR-034-Client-Side-Report-PDF-Generation.md is the source of truth for the invariant being enforced, and the PR leaves four passages falsified. Per CLAUDE.md's Wiki Update Discipline ("update the wiki as part of story implementation, never as a review catch") this belongs on this branch, and the ADR-034 debt closed on 2026-08-04 under #1914 would otherwise reopen immediately:

• Line 82 — "Since Bank report wizard: editable HTML preview before export #1900 the builders take (reportContent: ReportContent, …, t: TFunction) … The residual t covers … overviewPdf's *N skip-note reasons and coverLetterPdf's Reference: / Subject: prefixes — and it is always reportT, never the interface t." There is no residual t. This paragraph now instructs a future contributor to add one back.
• Line 178 — "Nothing in the type system distinguishes reportT from the interface t — both are TFunction — so the only defences are this contract, review, and per-locale real-render tests." False for reportPdf/* as of this PR: the defence is now structural (the modules have no channel to any TFunction). The sentence should be re-scoped to buildReportContent's boundary, which is where the distinction genuinely still rests on naming.
• Lines 186-188 (grep guard) — the invariant-1 guard cannot catch a TFunction parameter. Promote AC5's grep to a numbered invariant with its own guard: grep -rn "TFunction" client/src/lib/reportPdf/ --include=*.ts | grep -v test must return nothing, with a note that reportContent/buildReportContent.ts is the single legitimate TFunction entry point.
• Line 219 — "Still re-translated independently … overviewPdf.ts's *N skip-note reasons and coverLetterPdf.ts's cover-letter field derivations (lines 46 and 54)" — that list is now empty; every printed string reaches the PDF through ReportContentLabels.
• Line 200 (minor, same pass) — the parenthetical "(Reference: and Subject: do print, but from separate sourceReports.coverLetter.* keys)" should say they print from labels.coverLetterReferenceLabel / labels.coverLetterSubjectLabel, which is what makes the UI-language sourceReports.editable.* labels in ReportContentEditor.tsx:123,140 a deliberate split rather than a duplicate.

Plus a Deviation Log row at the bottom of the page recording that the locale contract moved from convention-enforced to compiler-enforced for reportPdf/*.

I own this page — route the wiki delta back to product-architect rather than to a dev agent (ADR prose is not implementer work). Push the submodule and commit the bumped wiki ref on this branch before re-requesting review.

---

Informational (no action on this PR)

• buildReportContent.ts:82 — the module-private getAttachmentNote(invoice, t: TFunction) helper. Single caller, passed reportT (line 196), inside the module that owns reportT, so it is correct; renaming the param reportT would remove the last one-character confusion inside the boundary.
• Same helper resolves a dynamic key from server data: t(\sourceReports.table.attachmentType.${type}`). An attachmentTypevalue with no translation echoes a raw key into the PDF — the same key-echo class as the skip reasons, one layer up, and not addressed by this story. Worth its own issue ifattachmentType` is not a closed set at the API boundary.
• merge.ts pushes to skippedDocuments (line ~179, appendix load failures) after buildOverviewContent has already consumed skippedByInvoice, so an attachment that fails during merge gets no *N footnote. Pre-existing, out of scope here.

Re-request review once H1 and H2 land; the rest of the PR is good to go.

[steilerDev]

[product-owner]

Requirements Review — PR #2007 (#2001: remove TFunction from reportPdf/*)

Verdict: APPROVED. All 9 acceptance criteria met. One Medium, non-blocking finding (below) — the story's stated goal ("compiler-enforced, not convention-enforced") is met for the TFunction class but one new unchecked escape hatch was introduced in overviewPdf.ts; fix in-PR or as a follow-up, your call.

AC verification

AC	Result	Evidence
1. Three label fields added, populated via reportT in the budget namespace	PASS	types.ts:67-72 adds coverLetterReferenceLabel, coverLetterSubjectLabel, skipReasonLabels{footnoteFetchFailed,footnoteInvalidPdf}; buildReportContent.ts:300-305 populates all four via reportT, which is getFixedT(reportLanguage, 'budget') (ReportWizardPage.tsx:254) — namespace matches
2. Dynamic skip-footnote key resolved where reportT is available; every producible reason resolves in en + de	PASS	SkippedDocument['reason'] (reportPdf/types.ts:17) is a closed union of exactly footnoteFetchFailed / footnoteInvalidPdf; both producers (merge.ts:62,82,91,182) emit only those two; both are keys of skipReasonLabels; both resolve in en (Document could not be retrieved / Document is not a valid PDF) and de (Dokument konnte nicht abgerufen werden / Dokument ist keine gültige PDF-Datei). No key echo possible today
3. buildCoverLetterContent / buildOverviewContent drop the TFunction param	PASS	coverLetterPdf.ts:8, overviewPdf.ts:491-493
4. generateReportPdf drops the TFunction param; ReportWizardPage updated	PASS	merge.ts:31-38; call site ReportWizardPage.tsx:313 plus reportT removed from the useCallback dep array
5. grep TFunction client/src/lib/reportPdf/ --include=*.ts | grep -v test → no matches	PASS	Enumerated all 9 production .ts files under reportPdf/ at df7d2c7: zero matches. Went one step further — none of the 9 references i18n, useTranslation, toLocale*, or Intl. either, so the module boundary is now genuinely locale-free, not just TFunction-free
6. Every key exists in en/budget.json and de/budget.json	PASS	All four keys pre-existed in both locales; no i18n files touched by this PR (correct — nothing new to translate)
7. Regression test fails if a string is resolved through TFunction, verified to discriminate	PASS — verified empirically, see below
8. npm run typecheck passes (sweep all ReportContentLabels construction sites)	PASS	Static Analysis green. The #2000 trap the AC warned about did fire and was caught: commit 3b26a82c fixed the four missed construction sites (realRender.test.ts, ReportContentEditor.test.tsx, applyOverrides.test.ts, applyAiContent.test.ts)
9. Quality Gates green	PASS	Green, plus Static Analysis, all 6 Jest shards, and all 16 E2E shards green (mergeStateStatus: CLEAN)

AC7 discrimination — verified by mutation, not by reading

I did not take "the tests pass" as evidence. I checked out df7d2c7 in a throwaway worktree, mutated each production line, and confirmed the sentinels go red — then reverted:

1. coverLetterPdf.ts:45,53 — replaced ${reportContent.labels.coverLetterReferenceLabel} / ...SubjectLabel with literals → both AC7 sentinels fail, plus 2 pre-existing prefix tests.
2. overviewPdf.ts:840 — replaced ${reasonLabel} with ${reason} (i.e. label lookup bypassed) → both skipReasonLabels sentinels fail, plus 3 pre-existing footnote tests.
3. buildReportContent.ts:300,303 — swapped in the wrong i18n key (sourceReports.table.reference for the reference label, footnoteInvalidPdf for footnoteFetchFailed) → the 2 corresponding #2001 label tests fail while the other 66 stay green, proving the AC1 tests pin the specific key and not merely "some string".

9 tests red under mutation, 116 green after revert. The sentinel fixture values (LABEL-REF-SENTINEL, FETCH-SENTINEL, …) are deliberately distinct from what an identity-t would echo, so none of these can pass vacuously.

Behavioural regression check (not an AC, but the one way this refactor could have broken something)

Moving these three strings from generate time (reportT passed into generateReportPdf) to build time (reportContent.labels) would produce stale labels if the report language could change without reportContent being rebuilt. It can't: baselineContent's useMemo lists reportT in its dependency array (ReportWizardPage.tsx:282-292) and reportT is memoized on reportLanguage, so switching the report language rebuilds every label. applyAiContent and applyOverrides both shallow-spread ...content, so labels survives the override pipeline untouched. No regression.

Finding — M1 (Medium, non-blocking): the new lookup re-opens the same hazard class in a new form

overviewPdf.ts:835-838:

const reasonLabel =
  reportContent.labels.skipReasonLabels[
    reason as 'footnoteFetchFailed' | 'footnoteInvalidPdf'
  ] ?? reason;

The as assertion plus the ?? reason fallback is exactly the "enforced by the convention that everyone remembers" shape the story was written to eliminate — just moved from TFunction to the label lookup. If a third value is ever added to SkippedDocument['reason'] (reportPdf/types.ts:17), the compiler stays silent, skipReasonLabels silently lacks the field, and the raw identifier (footnoteWhateverNew) gets printed into a document handed to a bank. That is the same "found by the next reviewer" failure mode as #1938 and #1993, and AC2's "no key echoed into the PDF" would then be false.

I'm capping this at Medium and not blocking, because AC2 as written is about the values the code can produce today, and it holds — the union is closed and both members are mapped. But the story's purpose is to close the class, so it is worth closing properly.

Concrete suggestion (roughly two lines, both inside code this PR already touches): type the parameter as the union instead of string, and drop both the cast and the fallback —

• overviewPdf.ts:493: skippedDocuments: Map<string, SkippedDocument['reason'][]>
• merge.ts:101: new Map<string, SkippedDocument['reason'][]>() (skip.reason already has that type, so nothing else changes)

Adding a reason then fails the build in types.ts/buildReportContent.ts until a label is added — which is what AC5 asks for in spirit. If you'd rather not widen the diff, say so and I'll file it as a follow-up issue instead.

Informational

• I1 — Scope is tight and appropriate: 3 production files, 1 page, 7 test files, 2 agent-memory files (rides along per CLAUDE.md). No i18n changes, no behavioural changes. Two incidental Prettier reformats got swept in (ReportContentEditor.test.tsx:1611, and an italics normalisation in qa-integration-tester/MEMORY.md) — harmless, and since CI has no format:check, formatting-on-touch is the right habit.
• I2 — No UAT scenario for this story, per its own Notes: there is no user-visible defect to observe, and AC5 is statically verifiable. Every AC here is machine-checkable, so refactor(reports): remove TFunction from reportPdf/* so ADR-034 locale decoupling is compiler-enforced #2001 goes straight to Done on merge — it does not need to sit in UAT.
• I3 — All 16 E2E shards are green on this head commit, including shard 8/16. E2E shard 8/16 red across four beta PRs: dashboard "New Invoice" shortcut opens no modal (Scenario 13, #1735) #2005 (the dashboard "New Invoice" failure that was blocking the next beta → main promotion) is now closed, and this run confirms the fix has landed on the branch's base. The promotion blocker is clear from this PR's perspective.

[steilerDev]

[product-architect] Re-review of PR #2007 (round 2) — APPROVED

Both blocking findings verified resolved against the pushed head 7a40fa3, not on report.

H1 — closed, and closed properly

ReportSkipReason is now a single authoritative declaration with no duplicate and no assertion anywhere in the chain:

Site	State
reportContent/types.ts:50	export type ReportSkipReason = 'footnoteFetchFailed' | 'footnoteInvalidPdf' — sole declaration
reportContent/index.ts:13	re-exported from the barrel
reportContent/types.ts:71	skipReasonLabels: Record<ReportSkipReason, string>
reportPdf/types.ts:18	reason: ReportSkipReason — the inline duplicate is gone
reportPdf/merge.ts:101	new Map<string, ReportSkipReason[]>() — no widening to string
reportPdf/overviewPdf.ts:493	parameter typed Map<string, ReportSkipReason[]>
reportPdf/overviewPdf.ts:835	const reasonLabel = reportContent.labels.skipReasonLabels[reason]; — no as, no ?? reason

The producer→transport→label-table chain now shares one type, so adding a skip reason is a compile error at the single population site in buildReportContent.ts rather than a raw identifier printed into a bank-facing PDF. That the widened-Map variant is gone matters as much as the cast: keeping Map<string, string[]> would have forced the assertion straight back.

Static Analysis passing is itself the proof that no call site smuggles an out-of-union reason — that would now be a type error, not a silent fallback. Ten test annotations updated across overviewPdf.test.ts and realRender.test.ts; none needed an escape hatch, which is what I expected from checking them in round 1.

H2 — closed

ADR-034 updated in wiki commit cfbb0bd (published; ls-remote origin master confirms), and git ls-tree <head> wiki on this branch resolves to that exact commit, so the PR carries the ref rather than depending on a later bump. The page now states the contract at its post-#2001 strength: invariant 5 (no TFunction under reportPdf/) with its own grep guard, the note that the invariant-1 guard structurally cannot match a t: TFunction parameter, the re-scoped "nothing in the type system distinguishes reportT" sentence, the closed-class statement replacing the #1993 footer paragraph, the value-keyed-label-set rule, and the corrected Reference:/Subject: worked case. Deviation Log row added for 2026-08-05.

Re-verified on the pushed head

• Both ADR-034 grep guards return nothing. Invariant 5 (TFunction in reportPdf/ production files) and invariant 1 (useTranslation|/i18n/|Intl\.|toLocale across reportPdf/ + reportContent/). AC5 holds.
• AC7 sentinel blocks intact and unweakened — all four unique sentinels (LABEL-REF-SENTINEL, LABEL-SUBJ-SENTINEL, FETCH-SENTINEL, INVALID-SENTINEL) still assert exact equality. The H1 refactor did not dilute them into the surrounding key-echo style.
• The unrelated prettier drift did not ride along. shared/src/types/{diary,dependency,document}.ts, client/src/lib/budgetSourceColors.ts, DashboardPage.tsx, ManagePage.tsx show an empty diff against beta — the collapsed union-type reformatting stayed out of the commit. Thanks for catching that; it would have made this refactor's diff unreviewable.
• CI: Quality Gates, Static Analysis, Coverage Report, Trailer Check, Docker all green. mergeStateStatus: UNSTABLE is only shards 12 and 13 of the full E2E matrix still running — not a required gate for a beta-targeted PR.

Non-blocking follow-ups (do not hold the merge)

1. overviewPdf.test.ts:10 — the file-header comment still documents the old signature: buildOverviewContent(reportContent: ReportContent, skippedDocuments: Map<string, string[]>): Content[]. Same cross-reference rot class as the stale ADR lines this PR just fixed, one layer down. Worth a one-line touch-up whenever this file is next opened.
2. merge.test.ts:316,330 — mock.calls[0]![1] as Map<string, string[]>. These are read-side casts on a captured mock argument, so they widen rather than narrow and cannot lie in the dangerous direction; but typing them Map<string, ReportSkipReason[]> would make the test fail if merge.ts ever passed the wrong element type, which is the property the rest of this PR just established.
3. buildReportContent.ts:82 — getAttachmentNote(invoice, t: TFunction) resolves a dynamic key from server data (sourceReports.table.attachmentType.${type}). An attachmentType value with no translation echoes a raw identifier into the PDF — the same key-echo class this story closed, one layer up and outside its scope. Worth its own issue if attachmentType is not a closed set at the API boundary; the fix shape is now well-established by ReportSkipReason.
4. Pre-existing, unrelated: merge.ts pushes appendix-load failures into skippedDocuments after buildOverviewContent has already consumed the map, so those skips get no *N footnote.

Good refactor — it closes a hazard class that had been fixed one call site at a time across three separate rounds (#1938, #1993, #2001), and it closes it by removing the channel rather than the symptom. Approved.

[github-actions]

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

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 📦🚀