feat(reports): wire report column visibility toggles through to the generated PDF (#1973)

[steilerDev]

Summary

• Generalizes overviewPdf.ts from 2 hardcoded table shapes to the full 96-legal-subset column-geometry engine, so the PDF renders whatever combination of columns the preview allows.
• Introduces a single derivation of visible/legal columns, client/src/lib/reportContent/columns.ts (AC 2.1), consumed by both the ReportContentEditor preview toggles and the overviewPdf builder — eliminating drift between what the user sees and what gets generated.
• Adds per-run hiddenColumns state to the report wizard reducer, reset whenever the use case changes (AC 5.1); never persisted server-side (AC 5.2) — every wizard run starts from the full column set.
• Locks the Allocated Amount checkbox (always visible, cannot be hidden) and adds the new shared .bannerWarning class in shared.module.css for the wizard's warning banner.
• Deletes the superseded columnVisibilityHint i18n string, replaced by the locked-checkbox affordance.

Fixes #1973

Test plan

• Unit tests pass (95%+ coverage) — columns.ts/columns.test.ts, overviewPdf.ts/overviewPdf.test.ts, wizardReducer.ts/wizardReducer.test.ts, ReportContentEditor.tsx/.test.tsx, merge.ts/merge.test.ts, realRender.test.ts
• Integration tests pass
• CI Quality Gates pass (typecheck, tests, build, audit)
• E2E Gates — all 16 shards green, see AC 7.4 shard-hygiene section below

AC 7.4 — E2E shard hygiene

This story adds 19 new E2E tests (e2e/tests/budget/reportWizardEditableContent.spec.ts) across three viewports. New tests rehash Playwright's worker-hash assignment and can redistribute the whole suite across shards, so an unrelated shard going red is a live possibility on this PR — it must be triaged, not re-run past.

Prior beta state (baseline, not this PR): PR #2007 (merged 2026-08-05) had shard 10/16 fail on invoices/invoice-vendor-change.spec.ts:129 [tablet], on both the initial attempt and the retry, with TimeoutError: locator.waitFor: Timeout 10000ms exceeded. Triaged as not caused by #2007 — that PR's diff is scoped entirely to reportContent/reportPdf/ReportWizardPage.tsx, nothing under invoices/ or vendors/. Unrelated to #1973 as well (different domain, different files). Logged in .claude/agent-memory/e2e-test-engineer/known-flakes-and-regressions.md.

This PR's own shard results (final, head b5b03bec): the full matrix completed green — 33/33 check-runs successful. All 16 E2E shards passed, plus E2E Gates, E2E Smoke Tests, E2E Cache Warmup, Merge E2E Reports, all 6 Jest shards, Static Analysis, Docker, Docker PR Release, Coverage Report and Trailer Check.

Determination: zero red shards. AC 7.4's triage obligation therefore resolves to "no shard to triage" — no re-runs were performed, and no shard was retried to reach this state. The redistribution hazard flagged above did not materialise despite the 19 added tests. AC 7.5 does not fire, since no latent cross-file hazard was surfaced.

The prior-beta finding recorded above (PR #2007, shard 10/16, invoices/invoice-vendor-change.spec.ts) did not recur on this run — shard 10/16 passed here. It remains logged as an open single-occurrence flake for whoever next touches that spec; it is not evidence about this PR either way.

Co-Authored-By: Claude dev-team-lead noreply@anthropic.com
Co-Authored-By: Claude frontend-developer noreply@anthropic.com
Co-Authored-By: Claude translator noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester noreply@anthropic.com
Co-Authored-By: Claude e2e-test-engineer noreply@anthropic.com
Co-Authored-By: Claude ux-designer noreply@anthropic.com

[steilerDev]

[ux-designer]

Verdict: APPROVED

Design-system review of PR #2010 against the visual spec I posted on issue #1973 (comment 5196838267).

(Note: posted as a regular comment, not a formal PR review — gh pr review --approve was rejected with "Can not approve your own pull request" under this session's gh identity.)

1. Locked Allocated Amount checkbox (AC 2.2) — matches spec

• ReportContentEditor.tsx:256-270: renders checked disabled unconditionally via isColumnLocked(col) (client/src/lib/reportContent/columns.ts), with the aria-hidden * marker and the shared legend (.columnToggleRequiredHint) wired via aria-describedby={requiredHintId} (useId()-generated, stable).
• The legend <p id={requiredHintId}> is rendered unconditionally, once, below .columnToggles — not gated per-checkbox — so aria-describedby always resolves to non-empty text. ReportContentEditor.test.tsx:1221-1232 (scenario 25) verifies this directly: resolves the id, asserts the element exists and textContent.trim().length > 0. That's a real assertion of the resolved accessible description, not just attribute presence.
• Disabled-state CSS (ReportContentEditor.module.css:121-124) is .columnToggle:has(input:disabled) { opacity: 0.6; cursor: not-allowed; } — byte-identical to the ReportWizardPage.module.css:218-232 .optionCheckbox:disabled precedent I cited.
• .requiredMarker and .columnToggleRequiredHint both route through --color-text-muted, which already has correct light/dark values in tokens.css — no local dark-mode override needed, none added.
• The dead .columnToggleHint rule and the columnVisibilityHint i18n key are both fully removed (confirmed via repo-wide grep — zero remaining references in source). Scenario 31 in the test file also asserts the string never renders.

2. .bannerWarning (AC 6.2) — matches spec

• shared.module.css:452-459 is a byte-for-byte match of the CSS block in my spec: --color-warning-bg / --color-warning (border) / --color-warning-text-on-light, --radius-md, --spacing-3, --font-size-sm. Placed correctly in the existing STATUS BANNERS section, directly after .bannerError, completing the .bannerSuccess/.bannerError/.bannerWarning family rather than forking a new pattern.
• Token values verified against tokens.css: light --color-warning-text-on-light = --color-orange-700 (#c2410c), dark = --color-orange-300 (#fdba74) — matches the light/dark pair I specified.
• Uses the plain-text tier (no icon/heading), consistent with InvoiceDepositsSection.warningBanner's weight, not MassMoveModal's heavier tier.
• role="status" (not role="alert") on the single conditional <div> (ReportContentEditor.tsx:279-283), no aria-live duplicated alongside it, no aria-describedby/disabled wired to any input from it — matches the "advisory, not blocking" framing in R8.
• ReportContentEditor.test.tsx:1378-1402 (scenario 30) verifies the banner appears in exactly one of the four (usageHidden × attachDocuments) combinations and asserts role="status" — correctly modeling the AND condition, not just "usage hidden."

3. 44px touch-target fix — matches spec

ReportContentEditor.module.css:126-130: @media (max-width: 1024px) { .columnToggle { min-height: 44px; } }, scoped to tablet/mobile only, desktop density unchanged. The 1024px/44px literals are consistent with the rest of the codebase — tokens.css has no --breakpoint-* custom properties (checked directly; CSS custom properties can't be used in @media feature values without a preprocessor anyway), and every other component in the tree (Sidebar, AppShell, ThemeToggle, BudgetLineForm, etc.) hardcodes @media (max-width: 1024px) the same way. Not a token-adherence gap.

4. Token adherence — clean

Ran npx stylelint directly against both changed CSS files (ReportContentEditor.module.css, shared.module.css) — zero warnings. Manually diffed every added line: no hardcoded hex colors, only the two expected literal px values covered in point 3 (breakpoint + touch-target, both precedented). Everything else is var(--token-name).

5. Dark mode — clean

Every new/changed rule (.requiredMarker, .columnToggleRequiredHint, .columnToggle:has(input:disabled), .bannerWarning) routes exclusively through Layer 2 semantic tokens that already have [data-theme="dark"] overrides in tokens.css (--color-text-muted, --color-warning-bg, --color-warning, --color-warning-text-on-light). No component-local dark-mode block needed or added.

6. Layout at the extremes — clean

• .table still has no table-layout: fixed — confirmed unchanged. Down to the single-column (allocatedAmount-only) case, the remaining <th>/<td> sizes naturally; :last-child border rules degrade correctly at any subset size. Test scenario 26 explicitly renders this degenerate case (table.querySelectorAll('thead th') → length 1) and it passes.
• .mobileCard/.mobileCardRow conditional rendering inside the gap-based flex column produces no leftover artifacts when columns are unticked — unchanged from the prior verified pattern.
• Removing columnVisibilityHint left no layout artifact: the vacated .columnToggleGroup slot is filled by the new .columnToggleRequiredHint legend, which reuses the identical CSS shape (margin: 0; font-size: var(--font-size-xs); color: var(--color-text-muted); font-style: italic;) the deleted hint had. No orphaned spacing.

7. data-column-key test hook — inert, as expected

Grepped the full diff: data-column-key={col} appears exactly once, on the <input> (ReportContentEditor.tsx:262), and is referenced only from the test file. No CSS selector (attribute selector or otherwise) targets it. Purely a test hook, zero styling surface.

8. Mobile column-toggle finding — confirmed still true, E2E scope stands

Re-verified against the implementation: show(col) (derived from visibleReportColumns) still gates both the desktop <table> (lines 288-298) and the mobile .mobileCard tree (lines 393-465) identically, and .tableHeadingRow/.columnToggleGroup/.columnToggles still have no responsive display: none anywhere in the stylesheet — only .table/.mobileCardList switch at the 767px breakpoint. The premise my spec's E2E-scope recommendation (all three viewports) rested on is unchanged by this PR; no invalidation to flag.

Non-blocking observation (informational, not a design-system issue)

The German usageHiddenAttachmentsWarning string in client/src/i18n/de/budget.json:1227 mixes a straight " with the opening German „ quote mark („Verwendung" instead of „Verwendung"). This is a translator/glossary concern, not a token/CSS/a11y issue, so it doesn't block this review — flagging for the translator's awareness.

Verified

• Read ReportContentEditor.tsx, ReportContentEditor.module.css, shared.module.css, ReportWizardPage.tsx (wiring), columns.ts, both budget.json locale files, and ReportContentEditor.test.tsx's relevant scenarios (25, 26, 30, 31) directly in the worktree.
• Ran npx stylelint against both changed CSS files — clean.
• Cross-checked every cited token against client/src/styles/tokens.css light/dark values directly (not from memory).

[steilerDev]

[product-owner]

Verdict: APPROVE (requirements coverage / acceptance criteria)

Reviewed against #1973 rev 3 (re-read in full, including my own amendment log — the R- and AC-numbering was reassigned in the rev 1 → rev 2 rewrite, so this walk is against the current body, not memory). 8 groups, 33 criteria walked individually at b5b03bec.

28 satisfied, 5 partially satisfied (all Medium, non-blocking), 0 functional gaps. Two of the "partial" items are errors in my own AC text, not in the implementation — corrections are on me and are listed at the bottom.

---

The four criteria I flagged up front as most likely to be quietly unmet

AC 1.3 — grep verified myself. columnVisibilityHint is gone from both client/src/i18n/en/budget.json and client/src/i18n/de/budget.json (confirmed by parsing both files, not by reading the diff). A repo-wide grep excluding node_modules/.git returns exactly two hits: a test asserting its absence, and an agent-memory note. No production or locale hit. Satisfied. AC 1.4 likewise — PREVIEW-ONLY returns zero hits repo-wide, and no surviving comment claims the PDF always contains every column.

AC 2.1 — the round-1 fix is real and complete. client/src/lib/reportContent/columns.ts is the sole derivation. ReportContentEditor.tsx:250 now maps reportColumnsForUseCase(content.isOverview) directly; the re-encoded literal array (with its inline ...(content.isOverview ? [['status', …]] : []) branch) is deleted. The remaining COLUMN_LABEL record is a label lookup keyed by ColumnKey, not a second enumeration — reordering or corrupting it cannot change which columns render, because the iteration order comes from the shared array. overviewPdf.ts:635 consumes visibleReportColumns against the same module. Two independent copies of the rule genuinely do not exist. Satisfied, and columns.test.ts asserts the exact arrays (toEqual, not .length) plus that overview-minus-status equals claim with status at index 3 — a reorder fails.

AC 4.5 / 4.6 — tiers asserted per tier; 4.5's all-96 half is short. AC 4.6 is fully met and is the best-tested part of the PR: Tier 1 asserts the exact summary row array; Tier 2 asserts the label lands in the invoiceAmount cell and checks bold/alignment to prove it was not folded into the right-aligned amount cell (this is the specific failure my original ruling would have produced, so I'm glad it's pinned); Tier 3 runs it.each over all four subsets and asserts both that the table body is just the header (no unlabelled bare-number row leaked in) and that the stack block follows the table with the right label and amount. The tier-3 block I adopted over my own proposal landed exactly as ruled, including the preview-parity rationale in the code comment.

AC 4.5 is the short one: the 96-subset loop in the AC4.4 describe block asserts only amountText (toContain('€100.00')), never the label. AC 4.5 is explicit that the label is the thing to assert. Behaviour is correct by construction — buildSummaryRow is only reached when hasLeadingVisible || invoiceAmountVisible, and each of those branches emits a label cell; the complement is exactly usesSeparateSummaryBlock — so I've capped this at Medium. One expect(allStrings).toContain(<label>) inside the existing loop closes it.

AC 6.1 — verified the test's form, not just its existence. The legend is unconditional in the production path: overviewPdf.ts's footnote block is built from reportContent.footnotes and skippedDocuments with no reference to visible, hiddenColumns, or invoiceAmountVisible anywhere in it. The test is written in the shape R3 requires — it asserts the legend sentinel with Invoice Amount hidden and again with it visible, explicitly commenting that R3 rejects a conditional in both directions. No if (invoiceAmount hidden) then legend exists. Satisfied.

AC 6.2 — the 4-combination matrix is asserted in both the unit test and E2E (all three viewports). The banner is a sibling of the toggle group with role="status"; it disables no checkbox and gates no generation path. R8 honoured.

AC 8.1 — both new keys (allocatedAmountRequiredHint, usageHiddenAttachmentsWarning) exist in en and de. Tier 3 correctly needed no new key (it reuses row.label / row.amountText). No missing-key fallbacks.

AC 2.4 / 3.x / 4.x — real enumeration, not sampling

allLegalHiddenSets() bitmasks over the free-column list per use case, and there is a sanity test on the enumerator itself (64 / 32, and allocatedAmount never in any hidden set) so the loop cannot silently shrink and take every downstream assertion with it. The 96-subset loops assert checked === 96 / === 72 / === 24 rather than trusting the iteration. AC 3.2 and 3.4's figures are computed from tableOffsetsTotal + a REFERENCE_WIDTHS object derived from a real computeColumnWidths call — not the bare literals #1950 objected to. The 84.00pt / 315.00pt endpoints land as specified.

AC 2.7 gets the strongest available treatment: beyond the doc-definition check, realRender.test.ts renders the degenerate {allocatedAmount}-alone document through real, unmocked pdfmake for both use cases and asserts a loadable PDF with ≥1 page. That is the only assertion in the suite that can actually catch a pdfmake layout failure rather than a malformed Content[] tree.

R6 — Status was not smuggled in

Confirmed clean, three ways: buildReportContent.ts is not in the diff at all; CLAIM_COLUMNS physically omits status (with the status: isOverview ? status : null data reason recorded in the comment, so the constraint is documented as structural rather than policy); and the AC 2.6 test renders a claim editor with hiddenColumns explicitly containing 'status' and still finds no Status checkbox. No empty column reached a claim report. If the user does want Status on claim reports, that remains the separate buildReportContent change R6 describes, to be requested explicitly.

AC 7.4 — full E2E result (this is now knowable)

The PR body sets the triage framework up correctly — it states the hazard, names the prior-beta baseline (#2007's shard 10/16 invoice-vendor-change.spec.ts failure) with a file-scope argument for why it is unrelated, and commits to appending this PR's own result rather than re-running past it. That is the right shape.

The run has since completed. On b5b03bec: all 16 E2E shards green, E2E Gates green, Merge E2E Reports green, Quality Gates green, all 6 Jest shards green, Trailer Check green. Zero red shards, so there is nothing to triage and AC 7.5 does not fire. What remains is purely the reporting obligation — see M5.

---

Non-blocking findings (Medium — fix before merge or in refinement)

M1 — AC 4.1's second half is unasserted. The 96-subset loop checks the header row's cell count but never "each header cell's text equals the expected label for that column in order." A HEADER_LABEL mis-mapping would keep every count correct and pass. The AC 4.3 baseline comparison does not cover it — it maps only table.body.slice(1, 3), the data rows. Implementation reads correct (visible.map(col => buildHeaderCell(HEADER_LABEL[col], …))).

M2 — AC 4.2 excludes continuation rows by construction. The all-96 loop's own comment says "no continuation rows possible with this short fixture text," and the existing continuation-row tests (scenarios 9–11) run with no hiddenColumns. So "content rows, Usage continuation rows, and summary rows alike" is met for two of the three. Correct by construction ([...nonUsageVisible.map(buildEmptyBodyCell), buildUsageCell(...)]), but one long-usage fixture run through a couple of subsets would close it.

M3 — AC 4.5's all-96 label assertion. As above.

M4 — AC 3.3's "otherwise Vendor" half is unasserted. The 72-subset test only asserts expect(absorber).not.toBeNull() — it would pass if the absorber were invoiceNumber, date, or any other column, since the total would still equal printableWidth(). And AC 3.5's loop continues past the absorber, so a wrongly-chosen absorber's pinned width also goes unchecked. AC 3.3 spells out the intended assertion ("checking which entry of the widths array differs from its pinned constant"). I verified by hand that the behaviour is right and that AC 3.5 genuinely holds for the vendor-absorber case: the worst case is the 6-column overview subset {vendor, invoiceNumber, date, status, invoiceAmount, allocatedAmount}, giving vendor 515.28 − 51.5 − 272 = 191.78pt — never below its 45pt pinned width. So this is a coverage hole, not a defect. Asserting absorber === 'vendor' for the usage-hidden/vendor-visible group closes it.

M5 — AC 7.4's reporting half is still open on the PR body. The body says results "will be appended here once the full E2E run completes." They have. Append the green-across-all-16 result (and note that the #2007 shard-10 baseline did not recur) so the criterion is closed on the artifact the AC names, rather than only in this review comment.

Low / informational

L1 — AC 6.3 asserts a proxy. The test compares skip-footnote output with and without Usage hidden, not the set of embedded documents. Combined with merge.ts passing hiddenColumns only into buildOverviewContent (asserted by the 3-arg plumbing tests), I'm satisfied tier selection is structurally unaffected — but the assertion is one step removed from what the AC names.

L2 — German quotation marks in usageHiddenAttachmentsWarning. The string renders „Verwendung" — a German opening low-9 quote closed by an ASCII straight quote. The translator documented this as deliberately matching selectForMergeAriaLabel, and that is honest, but I checked: across the whole of de/budget.json there are exactly 2 occurrences of „ and 0 of “ — the "convention" is one prior instance. Not a merge concern and I'd rather not churn it inside this PR, but flagging it for the user to rule on, since a mismatched quote pair is visible to a German reader. Worth a small follow-up fixing both occurrences together if the user agrees.

---

Corrections I owe on #1973 (my errors, not this PR's)

AC 4.6's "92 subsets" figure is wrong. The correct partition is Tier 1 = 88, Tier 2 = 4, Tier 3 = 4. The 92 came from 96 − 4, which silently folded Tier 2 into Tier 1 — but Tier 2 is exactly {allocatedAmount, invoiceAmount} and {allocatedAmount, invoiceAmount, usage} for each use case, i.e. 4 subsets, and those are Tier 2 by my own R2. The tests were right not to assert 92 anywhere; they assert the partition behaviourally instead. I'll post a dated correction on the issue so a later reader doesn't fail a correct implementation against a wrong count.

AC 5.3's "leaving and re-entering the step" clause over-reaches R5. The implementation resets hiddenColumns on page reload (E2E Scenario 33, asserted) and on use-case change (AC 5.1, asserted), but deliberately preserves it across in-run step navigation — including through DISCARD_EDITS, with the reasoning recorded in wizardReducer.ts and pinned by both a reducer test and E2E Scenario 31's step 5 → 4 → 5 round trip. That is the correct reading: R5 says the selection "dies with the wizard run," and resetting a user's column choices every time they step back to check a setting is precisely the silent-state-loss class that #1943 and #1946 already cost us. I'll amend the AC to state the chosen reading rather than leave a clause the implementation is right to contradict.

---

Merge gate vs Done gate

Every acceptance criterion here is machine-checked, including the degenerate single-column case through real pdfmake — so nothing blocks the merge on a human check.

But two outcomes are stated by my rulings and pinned numerically by tests without anyone having confirmed they look right in a real bank document:

1. R7's narrower-than-page table — the 24 no-absorber subsets, down to an 84.00pt single-column table sitting left-aligned on a 515.28pt page.
2. R2's Tier-3 summary block — totals rendered as a two-column stack beneath the table rather than inside it.

The geometry is provably within bounds and the placement is provably present, which is what the ACs asked for. Whether either reads as a valid funding document to a recipient is a visual judgment no assertion can make. So: merge on the code gate; #1973 goes to UAT for a visual check of those two states before it moves to Done. If UAT rejects either, reopen #1973 rather than filing a follow-up — the ruling would be the thing that was wrong, not the implementation.

Board: #1973 stays In Progress through UAT, not Done-on-merge.

[steilerDev]

[product-architect] Architecture review — PR #2010 (#1973)

Read: issue #1973 rev 3 (R1–R8, AC 1.1–8.1), the full diff (27 files), wiki/ADR-034-Client-Side-Report-PDF-Generation.md, wiki/Architecture.md, wiki/Schema.md, wiki/API-Contract.md. Ran columns.test.ts + overviewPdf.test.ts + wizardReducer.test.ts locally: 198 passed. Prettier clean on all touched source files. Wiki submodule ref is da1324b and matches origin/master — no unpublished-ref hazard on this branch.

Verdict: CHANGES REQUESTED — one HIGH finding, and it is documentation, not code

The engineering here is genuinely good, and I have no correctness objection to the geometry engine, the clamp, the module boundary, or the state placement. The blocker is that this PR deletes an architectural constraint that ADR-034 currently states as a prohibition, and touches no wiki file.

---

HIGH — ADR-034 still says this feature is impossible, and this PR does not update it

wiki/ADR-034-Client-Side-Report-PDF-Generation.md:153, section "Geometry constraint that blocks a feature":

USAGE_WIDTH_7COL / USAGE_WIDTH_6COL derive from usableColumnWidth(n), and MAX_SAFE_USAGE_CHUNK_CHARS was measured against the 7-column shape. The PDF's column count is therefore fixed at 6 or 7 (7 for budget-overview, 6 otherwise). Making the wizard's column-visibility toggles affect the PDF would widen the Usage column and invalidate the measured ceiling — that is a re-measurement story, not a UI change. The toggles introduced in #1959 are deliberately preview-only local state.

Every clause of that paragraph is now false: the column count is 1–7, the toggles are not preview-only, and — the interesting part — no re-measurement was needed, because hiding a column only ever widens Usage and usageChunkCharsForWidth's one-sided clamp makes the 650 budget strictly more conservative in that direction. That reasoning is the actual architectural finding of this story and it currently exists only in a function doc comment. Left as-is, the ADR tells the next agent in reportPdf/ not to attempt what already shipped — on a page that has already cost three correction rounds (see its own Deviation Log, 2026-08-04 ×3 and 2026-08-05).

This is squarely CLAUDE.md's Wiki Update Discipline ("decision → ADR-NNN + ADR-Index, in the same PR"). Please add to this branch:

1. Replace the "Geometry constraint that blocks a feature" section with the post-Report column visibility: wire the preview toggles through to the generated PDF #1973 statement: the visible column set is any of the 96 legal subsets (Allocated Amount locked, R1); widths come from computeColumnWidths(visible), a single-absorber algorithm (usage → else vendor → else none) where every non-absorber column keeps its pinned constant; and MAX_SAFE_USAGE_CHUNK_CHARS survived without re-measurement because USAGE_WIDTH_7COL (138.28pt) is provably the narrowest Usage width across all 96 subsets, so the reference measurement is the floor and the clamp is one-sided by design. State the residual constraint that remains real: adding a new column narrows Usage below the reference and is a re-measurement story — that is the case the clamp exists to fail safe on.
2. Update the packUsageCellRows(segments, MAX_SAFE_USAGE_CHUNK_CHARS) reference at line 144 — the budget is now usageChunkChars, per-subset.
3. Fix the stale "Related sharp edge" paragraph (line 155): it says packUsageCellRows hangs on maxChars <= 0. It does not — overviewPdf.ts:338 throws. Its own closing sentence ("Unreachable while the budget is a constant; it matters the moment it becomes computed") is now live, and the answer is "both callees throw loudly." Worth recording that the prediction came true and how it was handled.
4. Module table (line ~70): add reportContent/columns.ts as the AC 2.1 single derivation of the base column set per use case and the locked column, consumed by both overviewPdf.ts and ReportContentEditor.tsx — this is the same "one definition, two consumers" invariant the page already states for ReportContentLabels at line 230, and it belongs beside it.
5. Deviation Log row at the bottom of the page.

No Schema.md / API-Contract.md change is needed — nothing crosses the wire, and AC 5.2 is precisely that it must not. No new ADR is warranted: this is an amendment to ADR-034's scope, not a new decision.

---

Answers to the five questions you asked

1. Is the absorber the right mechanism, and is it robust to a future column?

Yes, and it is the only one of the three plausible mechanisms that satisfies R7 as written. '*' is out on #1929's grounds (columnCalculator.js case-1 pushes the whole table past printableWidth()). Proportional distribution of slack across all visible columns would satisfy AC 3.1/3.5 but violates R7's "stretching a two-column numeric table across 515pt makes a bank document look broken." A single absorber gives you the algebraic proof for free: total = tableOffsetsTotal(n) + fixedSum + (usableColumnWidth(n) − fixedSum) = printableWidth() exactly when an absorber exists, < printableWidth() when it doesn't, and every other column is untouched. That is AC 3.1/3.2/3.4/3.5 discharged by construction rather than by 96 assertions, and the tests then confirm the construction. Endorsed.

On robustness, the forcing functions are asymmetric and worth knowing:

• Compile-error-enforced (a new ReportColumnKey cannot be forgotten): PINNED_WIDTHS: Record<FixedColumnKey, number>, HEADER_LABEL: Record<ReportColumnKey, string>, and buildBodyCell's default-less switch over FixedColumnKey. Three good ones.
• Not enforced: OVERVIEW_COLUMNS / CLAIM_COLUMNS in columns.ts, LEADING_COLUMNS and RIGHT_ALIGNED_COLUMNS in overviewPdf.ts, and the absorber's usage → vendor → null ternary. All hand-maintained lists over the same union. See findings M3/M4 below — none is a defect today, and the failure mode of each is soft (a new column silently absent, or a summary label landing one column earlier), not a malformed table.

2. The one-sided clamp. Correct as implemented and correctly asymmetric. Math.floor(650 × w/138.28) then min(650, ·) is the right shape because the underlying budget genuinely is linear in width — 650 ≈ 44 lines × ~14.8 chars/line at 138.28pt, and the line count is fixed by page height, so halving the width halves the character budget at constant height. The direction is also the safe one under float error: if the ratio ever came back 0.9999…, floor yields 649 and you chunk one character earlier. Documentation is adequate and explicitly names the "do not simplify this away" hazard; the two synthetic-width tests exercise both branches plus the inclusive boundary, which is the only way to test it since no live subset reaches the downward branch. One small addition requested in L2 below.

3. Module home. Right home, right direction. reportPdf/ already depends on reportContent/ (ReportContent, ReportSkipReason), and pageGeometry.ts's header comment pins that edge as one-way; columns.ts adds no new edge and no cycle. More importantly it is co-located with the fact it encodes: R6's claim-has-no-status rule is a property of buildReportContent.ts:203 (status: isOverview ? status : null), which lives two files away. Encoding it as a physically absent key in CLAIM_COLUMNS rather than a runtime check is the right call — AC 2.6 becomes structural. Endorsed.

4. State placement. Endorsed, and the reasoning in R5 holds up under review: no network dependency in a step that currently has none, no inheritance of #1955's race or #1972's silent-save failure, and SELECT_USE_CASE and SELECT_SOURCE both spread freshContentTier() so the reset is free rather than bolted on. One architectural note rather than an objection: DISCARD_EDITS's hiddenColumns: state.hiddenColumns is the first deliberate per-field opt-out from the freshContentTier()-spread convention that #1947 introduced specifically so a new ContentTier field could not be forgotten. It is documented in the reducer and pinned by its own test, so this is not drift. But it is a precedent: if a second field ever needs the same treatment, split the tier (a PresentationTier reset by SELECT_USE_CASE/SELECT_SOURCE but not by DISCARD_EDITS) rather than accumulating opt-outs.

5. ADR / wiki. ADR-034 amendment required (the HIGH above). No new ADR, no Schema/API-Contract change.

---

MEDIUM — non-blocking, but worth fixing in this round

M1 — AC 4.2's continuation rows are named by the AC and never exercised under a hidden-column subset. Both enumeration fixtures use short Usage text; the tests say so ("no continuation rows possible with this short fixture text"). Continuation rows are a different code path from content rows (nonUsageVisible.map(buildEmptyBodyCell) vs nonUsageVisible.map(col => buildBodyCell(...))), so structural inspection is doing the work the AC asked a test to do. One case suffices: usage visible, two or three columns hidden, usageText longer than usageChunkCharsForWidth(widths.usage) — assert every row length === visible.length. Reachable, because the budget stays clamped at 650 no matter how wide Usage gets.

M2 — AC 4.5 asks for the label at all 96 subsets; only 6 are covered. The all-96 loop (#1973 AC4.4) asserts amountText presence via JSON.stringify(result).toContain(...), but not the label — and AC 4.5 is explicit that "asserted for the label specifically, not just the amount" is the point. Placement is covered at 1 tier-1 subset, 1 tier-2 subset, and the 4 tier-3 subsets. One line in the existing loop closes it: expect(allStrings).toContain(summaryRow.label).

M3 — the tier-3 summary block is not width-constrained, so on {allocatedAmount} alone the total floats 431pt away from the column it totals. overviewPdf.ts pushes { stack: [{ columns: [label, amount] }] } with no width on either entry, so pdfmake gives them two '*' columns across the full 515.28pt printable width. For {allocatedAmount, usage} that is fine (usage absorbs, the table is already 515.28pt). For {allocatedAmount} alone the table is 84pt, left-aligned and the amount right-aligns at 515.28pt. That is the same "looks broken in a bank document" outcome R7 exists to prevent, relocated from the table to the block beneath it. 2 of 96 subsets, no content lost, so not blocking — but the fix is small: constrain the block to the table's own total (tableOffsetsTotal(n) + Σ widths), e.g. label width: total − ALLOCATED_AMOUNT_WIDTH, amount width: ALLOCATED_AMOUNT_WIDTH, so the total's right edge lines up with the column it belongs to. realRender.test.ts's AC 2.7 case already renders exactly this subset and could assert it.

M4 — columns.ts's base-set arrays are the one non-forcing link in an otherwise exhaustive chain. Adding a key to ReportColumnKey produces compile errors in PINNED_WIDTHS, HEADER_LABEL and buildBodyCell — but not in OVERVIEW_COLUMNS/CLAIM_COLUMNS, and neither columns.test.ts (its ALL_COLUMNS is also hand-typed, and the toHaveLength(7) line is a sanity check on the test's own literal) nor the editor's AC 2.5 test (pins 7/6) would fail. So a new column would silently appear nowhere. Suggested shape, which makes it a compile error like everything else:

const COLUMN_ORDER: Record<ReportColumnKey, number> = { vendor: 0, invoiceNumber: 1, date: 2, status: 3, invoiceAmount: 4, allocatedAmount: 5, usage: 6 };
const ALL_COLUMNS = (Object.keys(COLUMN_ORDER) as ReportColumnKey[]).sort((a, b) => COLUMN_ORDER[a] - COLUMN_ORDER[b]);
const CLAIM_COLUMNS = ALL_COLUMNS.filter((c) => c !== 'status'); // R6

Same class, softer, in overviewPdf.ts: LEADING_COLUMNS and the absorber ternary. Please at least lift the absorber rule into a named constant — const ABSORBER_PRIORITY: readonly ReportColumnKey[] = ['usage', 'vendor']; with visible.find(c => ABSORBER_PRIORITY.includes(c)) — so R7's "free-form text columns absorb, bounded ones don't" rule is stated once and greppable instead of living in a nested ternary.

---

LOW / informational

L1 — AC 4.6's "92 subsets" for tier 1 is arithmetically wrong, and the test title copies it. Tier 3 is 4 subsets; tier 2 ({invoiceAmount, allocatedAmount} and {invoiceAmount, allocatedAmount, usage}, × 2 use cases) is also 4. Tier 1 is therefore 88, not 92 — the issue body appears to have merged tiers 1 and 2. The implementation is right; only the number is wrong. Please correct the test title (overviewPdf.test.ts, "Tier 1 (92 subsets)") so the wrong figure doesn't propagate, and ideally amend the issue body.

L2 — record why the clamp returns exactly 650 at the reference width. usageChunkCharsForWidth(USAGE_WIDTH_7COL) === 650 depends on exact float equality between the incoming widths.usage and USAGE_WIDTH_7COL. It holds only because every pinned width is an integer and both sums use the same integer addends — introduce one non-integer pinned width and Math.floor can silently return 649. The direction is safe and the boundary test pins it; one sentence in the doc comment saves the next reader the derivation.

L3 — usageChunkChars = 0 when Usage is hidden relies on a caller-side guard. packUsageCellRows and splitIntoPageSafeChunks both throw on maxChars <= 0 (loudly, by design), and the if (!usageVisible) { rows.push(nonUsageCells); continue; } guard means it is unreachable. Fine as-is; making it structurally impossible (compute the budget inside the usageVisible branch, or type it number | null) would remove the reliance entirely.

L4 — AC 5.3's "leaving and re-entering the step" is implemented as per-run persistence, and I think that is correct. GO_TO_STEP deliberately does not reset hiddenColumns, so stepping 5 → 4 → 5 preserves the selection — same lifecycle as overrides. A literal reading of AC 5.3 would call that a failure, but the literal reading contradicts R5 ("dies with the wizard run") and would make the control unusable (Back discards your column choice). Reload and use-case change do reset, and both are covered. Flagging so UAT judges the behaviour rather than the sentence — amend the AC text, don't change the code.

L5 — the AC 1.3 test name overstates its guard. expect(screen.queryByText(/columnVisibilityHint/)).not.toBeInTheDocument() can only fail if a missing-key i18n fallback renders the raw key; it cannot fail from the key still being present in budget.json. The real proof is the repo-wide grep, which I re-ran independently: zero hits outside agent-memory and this test. Not a defect — the assertion is just narrower than its title.

L6 — type ColumnKey = ReportColumnKey; in ReportContentEditor.tsx is a vestigial alias. Use ReportColumnKey directly.

---

What I verified as correct and am not asking you to change

• computeColumnWidths' derivation re-checked independently: usableColumnWidth(n) = 515.28 − (8.5n + 0.5); removing any column both shrinks fixedSum by ≥ 40 and grows usableColumnWidth by 8.5, so Usage is strictly monotone-decreasing in column count and 138.28pt at the full 7-column set is the true floor across all 96 subsets. The clamp's premise holds.
• Absorber-is-vendor case: worst case ({vendor, invoiceNumber, date, status, invoiceAmount, allocatedAmount}) gives vendor 191.78pt, never below its 45pt pinned floor — AC 3.5 holds for the absorber too, not just the pinned columns.
• AC 3.2/3.4 subset counts independently derived: 64 − 2⁴ = 48 and 32 − 2³ = 24 → 72 with an absorber, 24 without. Matches.
• rowTexts() reconstructs run-array cells, so AC 4.3's per-column comparison is a real assertion and not the undefined === undefined shape — I checked specifically because vendor, allocatedAmount and usage are all run arrays.
• AC 2.6 is structural (no status key in CLAIM_COLUMNS) rather than a runtime filter — the stronger form.
• visibleReportColumns' unconditional re-inclusion of the locked column is real defense-in-depth beneath the disabled checkbox, and it is what makes AC 4.7 hold by construction.
• realRender.test.ts covers the degenerate single-column case through unmocked pdfmake layout resolution, which is the only place a real table-layout failure could surface.
• E2E Scenario 29's size-diff is a genuinely falsifiable PDF-level proxy given no text-extraction library, and the comment correctly forbids weakening it back to Scenario 8's non-trivial-size shape.
• --color-warning-bg / --color-warning / --color-warning-text-on-light all exist in both light and dark blocks of tokens.css; the new .bannerWarning uses tokens only.
• Wiki submodule ref published and identical to origin/master — no unpushed-ref trap.

Re-request me once ADR-034 is updated on this branch (with the submodule pushed and the parent ref bumped). The M/L items are yours to take or defer; none of them blocks.

[steilerDev]

[product-owner]

Follow-ups filed — none expands this PR

Per the coordinator's ruling that a green PR is not to be reopened to absorb non-blocking findings, the review findings are now issues. All four are on the board in Backlog, blocked by #1973.

Issue	Finding	Label / Priority
#2011	M3 — tier-3 summary block is laid out against printableWidth() rather than the width of the table above it. On {allocatedAmount} alone that puts the total ~431pt from an 84pt table: R7's "a stretched numeric table looks broken" outcome relocated below the table. 2 of 96 subsets, visual only, no content loss and no width-invariant violation.	bug / Should Have
#2012	M1 + M2 + M4 — three criteria that hold by construction but are unasserted: AC 4.2's Usage continuation rows never exercised under a hidden-column subset; AC 4.5's per-subset label assertion covered at 6 of 96 (the all-96 loop checks amountText only); AC 3.3's "otherwise Vendor" absorber never asserted (not.toBeNull() passes for any absorber, and AC 3.5's loop skips the absorber). Also carries the stale 92 subsets in the test title at overviewPdf.test.ts:2026 — folded in here rather than filed separately, since it is one line in the same file.	tech-debt / Should Have
#2013	L2 — German mixed quotation marks, both instances. The user ruled it worth fixing but not on this PR.	bug / Should Have
#2014	The architect's suggestion — state the ContentTier DISCARD_EDITS preserve-vs-discard rule in wizardReducer.ts, next to where the next person adding a tier field will look. The wiki keeps the architecture; the reducer gets the reminder.	tech-debt / Could Have

M5 is closed — the PR body now carries the E2E determination.

On why M3 slipped, since it is worth not repeating

R7 (table geometry when no free-form column is visible) and R2 (summary-label placement when no leading cell exists) were written separately, in different revisions of #1973, against different problems. Neither anticipated that the same degenerate subsets fire both: R7 makes the table narrow, R2 moves the total out of it, and the interaction — a narrow table under a full-width block — was stated by neither. The generalisable rule, recorded on #2011: when a ruling introduces a new element outside an existing element's geometry, state its width relationship to that element explicitly. Two rulings that each constrain a different element can produce an unstated third outcome wherever their triggers coincide.

#1973's disposition

Recorded on the issue (#1973): it stays In Progress pending UAT on merge, not Done, for R7's narrow table and R2's tier-3 block. #2011 is a live candidate for that UAT rejection, so the two are separated there — a width rejection is #2011, a design rejection ("the total should not have left the table at all") reopens #1973, because then the ruling was what was wrong.

[github-actions]

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

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