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

.claude/agent-memory/e2e-test-engineer/MEMORY.md

@@ -31,6 +31,7 @@
 
 - [issue-1959-inline-meta-and-labels.md](issue-1959-inline-meta-and-labels.md) — #1959 reversed #1923's †/‡ footnotes → inline `(partial)`/`(less deposit)` and merged area+attachments into one `.usageMetaText` line; POM renames, rewritten scenarios 2/17/18/20 + AI 8, attachment-tier facts, untested column toggles.
 - [claim-deposit-scope-1922.md](claim-deposit-scope-1922.md) — PR #1922 invoice/deposit claim-scope split: `handleMarkClaimed`'s two-array submit, server-truth success-banner counts, the three "deposit surfaces the invoice" shapes, `claimNothingClaimable` guard.
+- [issue-1973-column-visibility-pdf.md](issue-1973-column-visibility-pdf.md) — column-visibility toggles wired to the PDF (supersedes #1966): reducer reset facts (use-case change resets `hiddenColumns`, `SET_ATTACH_DOCUMENTS` doesn't), Scenarios 28-33, size-diff PDF-consequence proof pattern, `goBack()` viewport-independent multi-step walk, new `columnToggleGroup`/`usageHiddenAttachmentsWarning` POM locators.
 
 ## Open follow-ups to flag to orchestrator
 

.claude/agent-memory/e2e-test-engineer/issue-1973-column-visibility-pdf.md

@@ -0,0 +1,121 @@
+---
+name: issue-1973-column-visibility-pdf
+description: Issue #1973 E2E coverage — column-visibility toggles wired through to the generated PDF, superseding #1966's DOM-only coverage. New POM locators, 6 rewritten/added scenarios, viewport-scope reasoning.
+metadata:
+  type: project
+---
+
+## What changed in production (frontend-developer, this issue)
+
+`hiddenColumns` moved from `ReportContentEditor` local `useState` into `ReportWizardPage`'s
+wizard-reducer `ContentTier` (`wizardReducer.ts`), threaded through to
+`generatePdfFromContent`/`overviewPdf.ts`. Single source of truth for the base column set per use
+case and the locked column lives in `client/src/lib/reportContent/columns.ts`
+(`isColumnLocked`/`visibleReportColumns`/`REQUIRED_REPORT_COLUMN = 'allocatedAmount'`), consumed
+by both the editor UI and the PDF geometry engine (AC 2.1).
+
+Key reducer facts (`client/src/pages/ReportWizardPage/wizardReducer.ts`):
+- `SELECT_USE_CASE` spreads `freshContentTier()` → `hiddenColumns` resets to `new Set()` on every
+  use-case change (AC 5.1). This is the SAME mechanism that already clears `overrides`/`aiContent`.
+- `DISCARD_EDITS` explicitly PRESERVES `hiddenColumns` (`hiddenColumns: state.hiddenColumns`
+  overridden back in after the `freshContentTier()` spread) — column visibility is a presentation
+  choice, not a "content edit" that the discard-confirm modal guards.
+- `hiddenColumns` is never persisted (no preference endpoint involved at all) — AC 5.2/5.3 fall
+  out for free: a full page reload wipes the in-memory reducer state entirely, same as every other
+  wizard-run-scoped field.
+- `SET_ATTACH_DOCUMENTS` only touches `SettingsTier` — going back to Settings and toggling
+  attachDocuments does NOT reset `hiddenColumns`, so a single seeded fixture can walk through all
+  4 combinations of the AC 6.2 warning-banner matrix without re-navigating from scratch.
+
+Both the desktop `<table>` and the mobile `.mobileCardList` gate on the exact same `show(col)`
+derivation in `ReportContentEditor.tsx` — confirmed by grep, no `@media` rule anywhere touches
+`.columnToggles`/`.columnToggleGroup`. The ux-designer's finding that the toggle group itself has
+**no responsive hiding** is correct and verified independently here.
+
+## E2E work done
+
+Rewrote the pre-existing "Scenario 24, #1966" describe block in
+`e2e/tests/budget/reportWizardEditableContent.spec.ts` (renamed **Scenario 28** — the file already
+had an unrelated, unrenumbered "Scenario 24" collision for the signature-reset test at the OLD
+line ~2236; both used the literal string "Scenario 24" and I did not touch the signature one) and
+added 5 new scenarios (29-33), picking fresh numbers past the file's existing max (27, the
+lang-attribute scenarios) rather than reusing/renumbering anything else in the file.
+
+- **Scenario 28** (desktop only, by documented exclusion): DOM-level baseline carried forward
+  verbatim per the spec's explicit instruction — checkbox presence/count, `<th>`+`<td>` removal
+  via `getByRole('columnheader'/'cell')` (requires real table semantics, absent from the mobile
+  card list), no-PATCH assertion (AC 7.1, AC 5.2).
+- **Scenario 29** (desktop only — download mechanism isn't viewport-dependent): THE scenario that
+  closes the #1966 gap (AC 1.2/7.2). Size-diff, not byte-parsing: seed Usage with ~40 sentences of
+  real text, download baseline (all columns), hide Usage, download again, assert
+  `hiddenSize < baselineSize`. Deliberately NOT a bare `>1000 bytes` check (that's Scenario 8's
+  weaker shape, which would pass identically whether or not the toggle reached generation) — a
+  code comment at the assertion says so explicitly per the spec's instruction, to survive a future
+  "simplify this" pass.
+- **Scenario 30** (`@responsive`, all 3 viewports): AC 2.2 locked checkbox — `toBeDisabled()`,
+  non-empty resolvable `aria-describedby` target, and `uncheck({force: true})` (bypasses
+  Playwright's actionability check, which would otherwise refuse to interact with a disabled
+  element outright) still leaves it checked afterward — a genuine behavioral proof, not a
+  restatement of `toBeDisabled()`.
+- **Scenario 31** (`@responsive`, all 3 viewports): AC 6.2 warning banner, all 4 combinations of
+  (Usage hidden/visible) × (attachDocuments on/off) in ONE test/ONE fixture, using the
+  `SET_ATTACH_DOCUMENTS`-doesn't-reset-`hiddenColumns` fact above to avoid re-seeding.
+- **Scenario 32** (`@responsive`, all 3 viewports): AC 5.1 use-case reset — hide a column on
+  `claim` (6 checkboxes), walk back to step 1 via 4× `goBack()` (viewport-independent — see
+  below), switch to `budget-overview`, walk forward, assert 7 checkboxes ALL checked (not just
+  "no longer hidden" — proves the new use case's own base-set SIZE, not just a stale 6).
+- **Scenario 33** (`@responsive`, all 3 viewports): AC 5.3 reload reset — reach step 5 via the
+  `?sourceId=` deep-link pattern (`wizard.goto(sourceId)`, mirroring `reportWizard.spec.ts`
+  Scenario 7), hide a column, `page.reload()` (URL still carries the query param), re-walk the
+  deep-link flow, assert the full 6-checkbox base set restored.
+
+## Viewport-scope decision (AC 7.3)
+
+Scenario 28 stays desktop-only WITH a documented reason (ARIA table-role dependency) — this is the
+one exclusion AC 7.3 explicitly allows ("a code comment states which are excluded and why").
+Scenarios 30-33 (checkbox-state assertions that don't depend on table semantics at all — the
+toggle group renders identically at every viewport) run at all 3 configured viewports via
+`{ tag: '@responsive' }`, with NO per-viewport branching needed in the assertion bodies (verified
+via `--list`: 53 tests total in the file across [desktop]/[tablet]/[mobile], vs. 34 before this
+issue). Scenario 29 (PDF download) stays desktop-only — the download mechanism itself isn't
+viewport-dependent, only the DOM-interaction scenarios are; this matches the E2E spec's explicit
+carve-out.
+
+## POM additions (`e2e/pages/ReportWizardPage.ts`)
+
+- `columnToggleGroup` = `page.getByRole('group', { name: 'Show/hide columns' })` — was previously
+  inlined at every call site in the old #1966 test; factored out since it's now reused across 6
+  scenarios.
+- `usageHiddenAttachmentsWarning` = `page.locator('[class*="bannerWarning"]')` — scoped by
+  CSS-module class (verified via grep: `bannerWarning` is used by exactly ONE component in the
+  whole client tree, `ReportContentEditor.tsx`), not by text, so it's stable against copy edits
+  and unambiguous against the page's other `role="status"` regions (`Toast`, several
+  `srOnly`/loading indicators — confirmed via grep there are ~15 other `role="status"` elements
+  across the app, several of which could plausibly be present on this same page).
+
+## Reusable navigation fact confirmed this session
+
+`goBack()` (`page.locator('[class*="buttonRow"] [class*="btnSecondary"]').first()`, re-queried
+lazily at each `.click()`) is safe to call repeatedly to walk backward through MULTIPLE steps
+(verified: only one `buttonRow` is ever mounted at a time across all 5 wizard steps) — 4 calls
+walk step 5 → step 1. This is viewport-independent, unlike `goToStep()` which clicks the
+desktop-only stepper widget (CSS-hidden below 768px) and would fail at the `mobile` project.
+Prefer `goBack()` over `goToStep()` for any `@responsive`-tagged test that needs to navigate
+backward.
+
+## Prior-CI triage performed for this session (see also `known-flakes-and-regressions.md`)
+
+Checked recent beta-merged PRs' full E2E results (`gh pr view <n> --json statusCheckRollup`,
+since `gh run list --branch beta --workflow "Quality Gates"` only surfaces promotion-PR runs, not
+individual story/bugfix PRs — those run as `pull_request` checks on the PR's own head branch, not
+a push to `beta`). Found ONE red shard in the last ~15 merges: PR #2007 ("refactor(reports):
+remove TFunction from reportPdf/*", merged 2026-08-05), shard 10/16, both attempt+retry failed on
+`invoices/invoice-vendor-change.spec.ts:129` [tablet] "Changing the vendor and saving updates the
+detail page and vendor list" with `TimeoutError: locator.waitFor: Timeout 10000ms exceeded`.
+Confirmed via `gh pr view 2007 --json files` that PR #2007's diff touches ONLY
+`reportContent`/`reportPdf`/`ReportWizardPage.tsx` files — nothing under `invoices/` or
+`vendors/` — so this is unrelated to that PR's own change and NOT caused by #1973's work either
+(different domain entirely). Not yet triaged to root cause (single occurrence so far, not
+established as a recurring flake) — flagging here for whoever next touches
+`invoice-vendor-change.spec.ts` or investigates a shard-10 tablet failure. Did not attempt a fix
+(out of scope for #1973, and a single occurrence isn't enough evidence to diagnose confidently).

.claude/agent-memory/e2e-test-engineer/known-flakes-and-regressions.md

@@ -7,6 +7,7 @@ metadata:
 
 ## Currently open / unresolved
 
+- **PR #2007 shard 10/16 (2026-08-05, merged to beta despite this)**: `invoices/invoice-vendor-change.spec.ts:129` [tablet] "Changing the vendor and saving updates the detail page and vendor list" — `TimeoutError: locator.waitFor: Timeout 10000ms exceeded` on both attempt and retry. Confirmed via `gh pr view 2007 --json files` that #2007's entire diff is `reportContent`/`reportPdf`/`ReportWizardPage.tsx` — nothing under `invoices/`/`vendors/` — so this is unrelated to that PR and not a regression it introduced. Single occurrence so far (not yet confirmed as a recurring flake vs. one-off CI resource contention) — no root-cause investigation done yet. Flag for whoever next touches this spec file or hits a shard-10/tablet failure.
 - `i18n-categories.spec.ts` "German locale: Manage trades tab shows 'Sanitär'..." — intermittent, locale doesn't initialize before English page renders. Pre-existing (seen before PR #1186 too).
 - **Latent cross-file hazard (unfixed, needs owner):** `e2e/tests/navigation/dashboard.spec.ts`'s file-level `beforeEach` PATCHes `locale='en'` on the **shared admin** user for all ~30 of its tests (added defensively so an i18n test's leftover `de` wouldn't break German card headings). `i18n.spec.ts`'s file-scoped serial mode does NOT protect against this — it only serializes within its own file. They happen to land in different shards today (verified: shard 4 contains `i18n*.spec.ts` but not `dashboard.spec.ts`), so it is currently latent. If E2E shard redistribution ever co-locates them, every i18n German assertion becomes racy again and the real fix is dedicated users in `i18n.spec.ts` (`i18n-categories.spec.ts` pattern).
 - `budget-overview-print.spec.ts` "Dark mode: print resets CSS variables" — HARD FAIL, production bug #1451 (`:global(@media print)` dropped by bundler).

.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; wiki + shared type JSDoc both fixed (API-Contract #1914, sourceReport.ts #1917/PR #1994)
 - [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 landed in PR #1979; per-locale header character budget + "no interface `t` in header/footer" (#1937/#1938, PR #1982); pdfmake `Content` is unspreadable (TS2698) but `Object.assign` needs no cast, and per-item `wordBreak`/newline-only-run facts (#1968, PR #2002). **ADR-034 debt fully PAID 2026-08-04 (#1914)**: width rule #1 (`max(horizontalRatio) <= 1`, not `_minWidth`), module table, override keys, dontBreakRows/height-bound section, injection-only locale contract. `merge.ts` footer/header interface-`t` defect **CLOSED** (footer PR #2000/#1993, header #1938); `TFunction` removed from all of `reportPdf/*` in #2001/PR #2007, so the locale contract is now compiler-enforced there and `buildReportContent.ts` is the single legitimate `TFunction` entry point — ADR-034 lines 82/178/186-188/200/219 all state the old weaker contract and needed a wiki pass (flagged in the PR #2007 review). **ADR-034 rule #1 is WRONG A SECOND TIME (proven in the PR #2008 review, 2026-08-05): `horizontalRatio` is a cell-origin bound, not a content-extent check — it is unconditionally `<= 1` on the all-fixed-width production table and cannot detect token overflow. The check that works is per-cell `_minWidth <= _calcWidth` (the ADR's `wordBreak` false-positive rationale for banning `_minWidth` is empirically false). **Third correction PAID 2026-08-05** (wiki `da1324b`): rule #1 leads with per-cell `_minWidth`, `maxHorizontalRatio` re-scoped to table-box positioning with a vacuity warning, `_minWidth` ban split table-level vs per-cell, 3rd Deviation Log row added. Semantics verified by mutation test in the PR #2008 round-2 review; **two follow-ups still open on the page — every quoted figure (69.28/33.54/266.16pt) is stale, and the rule says "every cell" while the test covers only Usage.**
+- [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); pdfmake `Content` is unspreadable (TS2698) but `Object.assign` needs no cast, and per-item `wordBreak`/newline-only-run facts (#1968, PR #2002). **ADR-034 debt fully PAID 2026-08-04 (#1914)**: width rule #1 (`max(horizontalRatio) <= 1`, not `_minWidth`), module table, override keys, dontBreakRows/height-bound section, injection-only locale contract. `merge.ts` footer/header interface-`t` defect **CLOSED** (footer PR #2000/#1993, header #1938); `TFunction` removed from all of `reportPdf/*` in #2001/PR #2007, so the locale contract is now compiler-enforced there and `buildReportContent.ts` is the single legitimate `TFunction` entry point — ADR-034 lines 82/178/186-188/200/219 all state the old weaker contract and needed a wiki pass (flagged in the PR #2007 review). **ADR-034 rule #1 is WRONG A SECOND TIME (proven in the PR #2008 review, 2026-08-05): `horizontalRatio` is a cell-origin bound, not a content-extent check — it is unconditionally `<= 1` on the all-fixed-width production table and cannot detect token overflow. The check that works is per-cell `_minWidth <= _calcWidth` (the ADR's `wordBreak` false-positive rationale for banning `_minWidth` is empirically false). **Third correction PAID 2026-08-05** (wiki `da1324b`): rule #1 leads with per-cell `_minWidth`, `maxHorizontalRatio` re-scoped to table-box positioning with a vacuity warning, `_minWidth` ban split table-level vs per-cell, 3rd Deviation Log row added. Semantics verified by mutation test in the PR #2008 round-2 review; **two follow-ups still open on the page — every quoted figure (69.28/33.54/266.16pt) is stale, and the rule says "every cell" while the test covers only Usage.** **#1973/PR #2010 (2026-08-05, CHANGES REQUESTED): the 2-hardcoded-shapes era is over — 96 legal column subsets via a single-absorber `computeColumnWidths`, `reportContent/columns.ts` as the AC 2.1 single derivation; `USAGE_WIDTH_7COL` 138.28pt is the NARROWEST Usage width so the 650 chunk budget needed no re-measurement (one-sided clamp). ADR-034's "Geometry constraint that blocks a feature" (line 153) was a false prohibition, plus line 144's constant reference and line 155's "hangs" claim (it throws). **All three PAID by me 2026-08-05, wiki `eb24774`**: section replaced with "Column geometry is a computed engine, not two pinned shapes", `columns.ts` added to the module structure, Deviation Log row. Same commit documented the wizard **tier-factory convention** in Architecture.md (never documented anywhere before, born #1947 after #1943/#1946) incl. the "second `DISCARD_EDITS` opt-out must trigger a tier split" limit.**
 - [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)