diff --git a/.claude/agent-memory/e2e-test-engineer/MEMORY.md b/.claude/agent-memory/e2e-test-engineer/MEMORY.md index c2037cf37..ddaab131a 100644 --- a/.claude/agent-memory/e2e-test-engineer/MEMORY.md +++ b/.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 diff --git a/.claude/agent-memory/e2e-test-engineer/issue-1973-column-visibility-pdf.md b/.claude/agent-memory/e2e-test-engineer/issue-1973-column-visibility-pdf.md new file mode 100644 index 000000000..0235d92f3 --- /dev/null +++ b/.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 `` 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, `` (E2E 25/26/27 target it). Round-2 H2 mirror: stale _comment_ over fresh `expect` = flag, not block. #1888 re-verified by diffing its file against the accepted commit (empty). Coordinator AC numbering wrong a **5th** time. Prior rounds: #1910 REJECTED **three times**: round 1 on AC3 (`lang` blanket-tagged on `.container`, only `

`s counter-tagged → `EditableField` chrome announced in report language, correct before the PR); round 2 on the "Option A surgical tagging" fix (`64c07b8a`) — **H2 AC5 red** (E2E Scenario 25 still asserts `container` lang, only the _comment_ was updated; Shard 2/16 test #168 confirms), **H3 AC1 regression** (`coverLetter.dateLine`/`closing` report-language spans lost coverage when the blanket tag narrowed), **H4 AC3 residue** (`.tableWrapper`/`.mobileCardList` still enclose the reset button + sr-only edited hint). Patterns: **AC enumerating element classes → tick each one off**; **override-by-inheritance fix → "what else inherits from the node you tagged?"**; **behaviour-inverting fix → grep for the OLD assertion, not the old comment**; **blanket→surgical refactor → audit what the blanket was silently covering**; **price intrinsic tensions differently from oversights** (offered a documented deviation for the one-element-one-lang `aria-label` conflict). Round 3 (`04e4ae0c`): H2/H3/H4/M1/L1 all verified fixed (**shard-diff across the PR's own commits** is the cheap proof — Shard 2/16 red→green), rejected again on **H5**: emptying `.tableWrapper`/`.mobileCardList` and reconciling only `

` leaves the desktop `` and the **entire** `.mobileCardList` untagged, and CSS hides `.table` at ≤767px → **zero `lang` on the mobile viewport**, AC1 unmet for a whole viewport + net regression vs the prior commit. Lessons: **when demanding a tag be removed, name the replacement coverage in the same breath**; **retract your own misread takeaway** (round 2's `uiLang` deletion was not a ruling against targeted counter-tagging); `npm run lint` has **no Prettier** and CI has no `format:check`, so formatting drift merges silently; `gh pr review` can't request changes on a human-authored PR → verdict goes in a comment. Detail in [bank-report-wizard.md](bank-report-wizard.md) §§"PR #2004 review" + "round 2" + "round 3". - **Bank Report Wizard mini-epic** (no parent epic) — all rulings, contract facts, per-PR review outcomes and filed follow-ups in [bank-report-wizard.md](bank-report-wizard.md). Shipped: #1876→#1877→#1878→#1879, Round 2 #1898–#1901, Round 3 #1929–#1933 (all merged; #1929 took 4 rounds, #1925 closed as duplicate). **Open**: #1888 indicator, #1891 (2 wiki MUST FIX), #1895→#1896/#1897 claim close-out, #1910 `lang` attr, #1917 consolidated follow-ups (incl. `KI` glossary entry + `computeIncludedTotal` extraction), #1937/#1938 PDF header bugs, #1939 geometry hygiene, #1940/#1941/#1950, #1946 in-flight AI generation (Must Have), #1947 `useReducer`, #1952/#1953, #1965–#1972 (PR #1959 sweep), **#1973** column visibility (Should Have, Todo, blocked-by #1965). #1931 merged but **not Done** — ACs 3.2/3.3 need live-LLM UAT. - **Reusable rulings from this cluster** (detail in [bank-report-wizard.md](bank-report-wizard.md), patterns in [pr-review-patterns.md](pr-review-patterns.md)): **merge is a code gate, Done is an acceptance gate** (unverifiable AC _with_ a substitute assertion = documented deviation; _without_ one → UAT, reopen on failure); **a finding that defeats the PR's own AC belongs in that PR, not a follow-up**; **closed/released ACs get a dated supersession comment, never a rewrite**; **ACs that misdescribe reality fail correct implementations at UAT** (seen 3×: #1943 AC4, #1933 AC2.1/2.7, my own #1925/#1932 transcription); **comment keeps the rationale, issue owns the guard**; bounded-and-quantified earns a tracked owner, unbounded-and-estimated gets documentation only. -- **#1973 column visibility wired through to the PDF** (user-story, Should Have, Todo, 2026-08-03, **blocked-by #1965**) — user reversed #1959's preview-only hint. **My proposed "at least one of Vendor/Invoice #" floor was rejected as an invented compliance rule**; only Allocated Amount is mandatory (survived because its justification is _structural_ — summary amounts + #1959 inline labels live in that cell — not purposive). 96 legal subsets (overview 2^6=64, claim 2^5=32), floor 1 column. Rulings: legend stays **unconditional** (AC 6.1 forbids `if invoiceAmount hidden`) because `(less deposit)` was insufficient regardless of adjacency; base set **IS** the ceiling for a **data** reason (`status: isOverview ? status : null`) — corrected the coordinator's "arbitrary means no ceiling" reading; per-session state, not `useColumnPreferences`; narrower-than-page table when neither Usage nor Vendor visible. **#1966 CLOSED as superseded** (board Wont-Do) — its AC1 would pass while the PDF still contained every column. Recommended **after** the #1958 promotion. **Rev 3 (spec reconciliation)**: adopted the dev-team-lead's **three-tier summary-label fallback** over my "same cell" ruling (92 subsets last-leading-column → Invoice Amount → separate block beneath table for 4; tier 3 _increases_ preview parity, `ReportContentEditor.tsx:442-445`); added **AC 3.7 one-sided chunk-budget clamp** (650 scales _down_ never _up_ — the hazard is a future _added_ column narrowing Usage, not this change); 72 subsets = `printableWidth()`, 24 narrower (84.00–315.00pt). **Process failure: I rewrote the body but reported only the rulings, so two agents spec'd from a stale rev 1 and re-filed an already-fixed contradiction — always say "body rewritten, numbering reassigned".** Detail in [bank-report-wizard.md](bank-report-wizard.md) §"#1973 column visibility" + §"rev 3". +- **#1973 column visibility wired through to the PDF** (user-story, Should Have, Todo, 2026-08-03, **blocked-by #1965**) — user reversed #1959's preview-only hint. **My proposed "at least one of Vendor/Invoice #" floor was rejected as an invented compliance rule**; only Allocated Amount is mandatory (survived because its justification is _structural_ — summary amounts + #1959 inline labels live in that cell — not purposive). 96 legal subsets (overview 2^6=64, claim 2^5=32), floor 1 column. Rulings: legend stays **unconditional** (AC 6.1 forbids `if invoiceAmount hidden`) because `(less deposit)` was insufficient regardless of adjacency; base set **IS** the ceiling for a **data** reason (`status: isOverview ? status : null`) — corrected the coordinator's "arbitrary means no ceiling" reading; per-session state, not `useColumnPreferences`; narrower-than-page table when neither Usage nor Vendor visible. **#1966 CLOSED as superseded** (board Wont-Do) — its AC1 would pass while the PDF still contained every column. Recommended **after** the #1958 promotion. **Rev 3 (spec reconciliation)**: adopted the dev-team-lead's **three-tier summary-label fallback** over my "same cell" ruling (92 subsets last-leading-column → Invoice Amount → separate block beneath table for 4; tier 3 _increases_ preview parity, `ReportContentEditor.tsx:442-445`); added **AC 3.7 one-sided chunk-budget clamp** (650 scales _down_ never _up_ — the hazard is a future _added_ column narrowing Usage, not this change); 72 subsets = `printableWidth()`, 24 narrower (84.00–315.00pt). **Process failure: I rewrote the body but reported only the rulings, so two agents spec'd from a stale rev 1 and re-filed an already-fixed contradiction — always say "body rewritten, numbering reassigned".** **PR #2010 APPROVED round 1** (`b5b03bec`, 2026-08-05, verdict in a comment — self-authored PR): 28/33 ACs met, 5 Medium partials (all the same shape — an AC's *second clause* dropped while the first was met: 4.1 header text, 4.2 continuation rows, 4.5 all-96 label, 3.3 vendor-absorber, 7.4 PR-body reporting), 0 functional gaps, all 16 E2E shards green. **Two AC errors were mine, corrected on the issue: AC 4.6's "92" → Tier 1 is 88 (Tier 2 = 4, folded in by 96−4); AC 5.3's "leaving and re-entering the step" over-reached R5 and would have mandated a third #1943/#1946 silent-state-loss.** **#1973 → UAT, stays In Progress** (R7 narrow table + R2 tier-3 block are visual judgments). **Follow-ups filed 2026-08-05, all Backlog + blocked-by #1973: #2011** (bug, tier-3 block not width-constrained — R7's "stretched numeric table" failure relocated below the table; **live UAT-rejection candidate, so width-rejection→#2011 vs design-rejection→reopen #1973 is stated up front**), **#2012** (tech-debt, the 3 "correct by construction, therefore untested" ACs + stale `92` in a test title), **#2013** (bug, German mixed quotes, both instances), **#2014** (tech-debt, `ContentTier` DISCARD_EDITS rule in the reducer). Standing ruling: **a green PR is not reopened to absorb non-blocking findings — file, don't expand.** Detail in [bank-report-wizard.md](bank-report-wizard.md) §"#1973 column visibility" + §"rev 3" + §"PR #2010" + §"#2010 follow-ups". ## Requirements Coverage diff --git a/.claude/agent-memory/product-owner/bank-report-wizard.md b/.claude/agent-memory/product-owner/bank-report-wizard.md index 301976b93..57fa090f6 100644 --- a/.claude/agent-memory/product-owner/bank-report-wizard.md +++ b/.claude/agent-memory/product-owner/bank-report-wizard.md @@ -762,3 +762,100 @@ record a deviation on AC1 — the one criterion whose stated measurement exists Architect's Lows still open: `collectAllStrings` forked 3× (`:884`/`:1151`/`:3207`); helper comment header still says *"content overflowed the page horizontally"* (the framing the ADR corrects) and cites `src/DocumentContext.js:528` vs the ADR's `DocumentContext.js:490`. + +--- + +## PR #2010 — #1973 column visibility wired through to the PDF — APPROVED round 1 (2026-08-05, `b5b03bec`) + +Verdict in a **comment** (PR authored by `steilerDev` = the token identity, so `gh pr review --approve` +is rejected as self-approval — same constraint as #2004/#2008). 33 criteria across 8 groups walked +individually: **28 satisfied, 5 Medium partials, 0 functional gaps.** All 16 E2E shards + `E2E Gates` +green on the head SHA. + +**What landed well, and is worth copying into future AC design:** + +- **A sanity test on the enumerator itself.** `allLegalHiddenSets()` bitmasks over the free-column list; + a separate test asserts it yields 64/32 and never puts `allocatedAmount` in a hidden set. Without that, + a silently-shrunk enumerator takes every downstream 96-subset assertion with it and nothing goes red. + Every subset loop also asserts `checked === 96/72/24` rather than trusting the iteration. +- **A positive control on a negative assertion.** E2E AC 5.2 fires a real PATCH via `page.evaluate()`, + asserts the interceptor caught it, resets the counter, *then* asserts the toggles produce zero. This is + the general fix for "assertions that pass on nothing" — demand it whenever an AC is "X never happens". +- **A forced click as behavioural proof of `disabled`.** `uncheck({ force: true }).catch(() => {})` then + re-assert `toBeChecked()` — genuinely different from restating `toBeDisabled()`. +- **A size-diff against the test's own baseline**, not a bare `> 1000 bytes`, as the E2E proxy for + "the toggle reached PDF generation". The bare-size shape is what let #1966 pass while preview-only. + +**Five Medium partials — all the same shape: an AC's second clause dropped while the first was met.** +M1 AC 4.1 (cell *count* asserted, header *text in order* not). M2 AC 4.2 (continuation rows explicitly +excluded from the 96-loop by fixture choice — the test comment says so). M3 AC 4.5 (96-loop checks +`amountText` only, never the label — the exact half I'd flagged as most likely to be quietly unmet). +M4 AC 3.3 (`expect(absorber).not.toBeNull()` would pass for *any* absorber; AC 3.5's loop `continue`s past +the absorber, so a wrong one is doubly unchecked). M5 AC 7.4 (PR body still says results "will be appended"). +**Lesson: when an AC is a compound sentence, tick each clause off separately — the first clause getting a +test is what makes the second one invisible.** + +**Verified by hand what the missing assertion would have covered** rather than just reporting the hole: +worst-case vendor-as-absorber is the 6-column overview subset, `515.28 − 51.5 − 272 = 191.78pt`, never +below the 45pt pin — so M4 is a coverage hole, not a defect, and gets capped at Medium on that basis. +`tableOffsetsTotal(n) = n*8.5 + 0.5`; `printableWidth() = 515.28`. + +**Two AC-text errors were mine** (posted as a dated correction comment on #1973, body left intact): +1. **AC 4.6's "92 subsets" is wrong — Tier 1 is 88.** 92 = 96−4, which folded Tier 2 into Tier 1. Tier 2 is + `{alloc, invoiceAmount}` and `{alloc, invoiceAmount, usage}` × 2 use cases = 4. 88+4+4 = 96. The tests + asserted the partition *behaviourally* (exact row arrays per tier) and never the count, which is why a + correct implementation didn't fail — **assert partitions behaviourally, not by cardinality.** +2. **AC 5.3's "leaving and re-entering the step" over-reached R5.** R5 says the selection dies with the + *run*; `overrides` survive in-run step navigation, so `hiddenColumns` should too. The AC as written would + have mandated a *third* instance of the #1943/#1946 silent-state-loss class. Implementation resets on + reload + use-case change, preserves across step nav and `DISCARD_EDITS`, documents why. Corrected reading + published; AC satisfied under it. + +**R6 non-smuggling confirmed three ways** (worth reusing as a checklist for "did the fix quietly grant the +thing I declined?"): the enabling file (`buildReportContent.ts`) is absent from the diff entirely; the data +is structurally absent (`CLAIM_COLUMNS` has no `status`); and a test passes `hiddenColumns` containing +`'status'` to a claim editor and still finds no checkbox. + +**Merge gate vs Done gate applied:** every AC is machine-checked (incl. the degenerate 1-column case through +*real unmocked pdfmake* in `realRender.test.ts`), so nothing blocks merge. But R7's narrower-than-page table +(84.00pt single column on a 515.28pt page) and R2's Tier-3 summary block are *visual* judgments no assertion +can make — **#1973 → UAT, stays In Progress, not Done-on-merge. If UAT rejects either, reopen #1973; the +ruling would be what was wrong, not the implementation.** + +**L2 for the user:** `de` warning renders `„Verwendung"` — German opening low-9 quote closed by ASCII `"`. +Translator documented it as matching `selectForMergeAriaLabel`, honestly — but `de/budget.json` has exactly +**2** `„` and **0** `“`, so the "convention" is one prior instance. Flagged, not blocked; worth a follow-up +fixing both. **When an agent cites a codebase convention, count the instances.** + +### #2010 follow-ups filed (2026-08-05) — #2011 / #2012 / #2013 / #2014 + +Coordinator ruling: **a green PR is not reopened to absorb non-blocking findings.** All four filed as +issues, Backlog, blocked-by #1973. Reusable: this is the standing disposition for architect/PO Mediums +raised on an already-green PR — file, don't expand. + +- **#2011** (`bug`, Should Have) — architect M3: tier-3 summary block laid out against `printableWidth()` + instead of the table's own width. `{allocatedAmount}` alone → total ~431pt from an 84pt table. **R7's + own "a stretched numeric table looks broken" failure mode, relocated below the table.** + **Why it slipped, and the rule that generalises: R7 and R2 were written separately, in different + revisions, against different problems — neither anticipated that the same degenerate subsets fire both. + 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.** +- **#2012** (`tech-debt`, Should Have) — M1+M2+M4 bundled, plus the stale `92 subsets` surviving in the + test title at `overviewPdf.test.ts:2026` (folded in, not filed separately — one line, same file). + Framed around the shared shape: **"correct by construction, therefore untested" is a claim about + today's code shape, and its whole value is that it stops holding silently.** +- **#2013** (`bug`, Should Have) — German mixed quotes, **both** instances (user ruled: fix, but not on + that PR). AC 2 asks for a repo-wide `„`-count == `“`-count check so the mixed form can't re-accumulate. +- **#2014** (`tech-debt`, Could Have) — state the `ContentTier` `DISCARD_EDITS` preserve-vs-discard rule + in `wizardReducer.ts`. Architect put the architecture in the wiki ("Multi-step wizard state: tier + factories") but was authorised only one production-comment edit. **Inverse of "comment keeps the + rationale, issue owns the guard": here the guard already exists and what's missing is the rule telling + the next person the guard was a decision, not an accident.** + +**#1973 disposition recorded on the issue:** stays **In Progress pending UAT** on merge, not Done — +R7's narrow table and R2's tier-3 block are visual judgments. **Separated #2011 from a #1973 reopen +explicitly, because #2011 is a live candidate for exactly that UAT rejection:** a **width** rejection is +#2011; a **design** rejection ("the total should not have left the table at all, at any width") reopens +#1973, because then the *ruling* was wrong. Worth reusing whenever a follow-up issue overlaps the same +surface a parent issue is going to UAT on — say which rejection routes where, before UAT runs. diff --git a/.claude/agent-memory/translator/MEMORY.md b/.claude/agent-memory/translator/MEMORY.md index 87be77e38..7532a8b93 100644 --- a/.claude/agent-memory/translator/MEMORY.md +++ b/.claude/agent-memory/translator/MEMORY.md @@ -102,6 +102,10 @@ New `sourceReports.expand.*` (chevron-expand sub-tables for budget lines + depos - [NBSP for inline labels](nbsp-inline-labels.md) — multi-word inline PDF labels need U+00A0, not a regular space, or pdfmake wraps mid-bracket; includes the ad-hoc real-render/pdftoppm verification recipe - [Abschlag glossary short-form](abschlag-glossary-shortform.md) — PO-approved `Abschlag` short form recorded IN glossary.json (75pt column has no room for any qualifier); `split`'s 3 German forms deliberately un-pinned +## Inline Label Quoting (Issue #1973, 2026-08-05) + +- [Inline label quoting](inline-label-quoting-1973.md) — „...\" (curly open, straight close) is the established codebase convention for naming a UI label inline in a sentence; cross-check the referenced label's own translation, don't re-translate the noun independently + ## Audit Protocol History - [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 diff --git a/.claude/agent-memory/translator/inline-label-quoting-1973.md b/.claude/agent-memory/translator/inline-label-quoting-1973.md new file mode 100644 index 000000000..27a1a489b --- /dev/null +++ b/.claude/agent-memory/translator/inline-label-quoting-1973.md @@ -0,0 +1,14 @@ +--- +name: inline-label-quoting-1973 +description: Convention for quoting a UI column/label name inline inside a German sentence (warning/hint text) +metadata: + type: project +--- + +When a hint/warning sentence needs to name a specific column header or UI label inline (e.g. "hiding Usage removes..."), the established `de/budget.json` precedent (`selectForMergeAriaLabel`: `"„{{description}}\" zum Zusammenführen auswählen"`) uses an opening German curly quote `„` paired with a **straight** closing quote `"` — not the typographically-correct closing `“`. This is inconsistent typography but is the codebase's existing convention; match it rather than "fixing" it to `„...“`, to avoid a gratuitous style drift in an otherwise-unrelated change. + +Applied in Issue #1973 (`sourceReports.editable.usageHiddenAttachmentsWarning`): named the "Usage" column inline as `„Verwendung"`, matching the exact German column-header translation at `sourceReports.table.usage` (confirmed via the EN/DE table headers, not guessed) — the warning must name a column the user can actually find, so always cross-check the referenced column's own header translation before writing the sentence, don't independently translate the noun. + +Also confirmed during this pass: `sourceReports.editable.columnVisibilityHint` deletion (superseded by these two new keys, AC 1.3) was already clean in `de/budget.json` before I started — zero dangling references anywhere in `client/src` — and a full flattened-key parity diff across the entire `budget` namespace (not just `sourceReports.editable`) came back 0/0 in both directions, so no pre-existing drift to report for this namespace at this time. + +See [[history-2026-h1]] for the general dash-substitution rule (spaced en dash `–` for English's em dash `—`) applied again here in `allocatedAmountRequiredHint`. diff --git a/.claude/agent-memory/ux-designer/feature-spec-history.md b/.claude/agent-memory/ux-designer/feature-spec-history.md index 651e7c792..7b55acb1f 100644 --- a/.claude/agent-memory/ux-designer/feature-spec-history.md +++ b/.claude/agent-memory/ux-designer/feature-spec-history.md @@ -19,10 +19,10 @@ Replaces Step 5's live PDF iframe with editable HTML (cover letter fields + repo Four-defect bug issue on `ReportInvoiceList.tsx`/`.module.css` (Bank Report Wizard Step 3), same cluster as #1900/#1901. - **New shared component spec'd**: `IconLinkButton` (`client/src/components/IconLinkButton/`) — icon-only link, `to`/`ariaLabel`/`icon`/`tooltip`/`newTab` props, renders react-router `Link` (supports `target="_blank"` natively — Link skips SPA interception whenever `target !== '_self'`), bakes `rel="noopener noreferrer"` in automatically when `newTab`. Justified by three pre-existing one-off external-link icon buttons (`DocumentCard.tsx` `.openInPaperlessButton`, `LinkedDocumentCard.tsx` `.openLink`, `SourceBudgetLinePanel.tsx` inline) that were never consolidated — check this component before hand-rolling a fourth. -- **Nested-label click-forwarding footgun, confirmed by reading `TriStateCheckbox.tsx`**: it renders its own internal ``; several list rows across the codebase then wrap it *again* in an outer `
`+`` 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 --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). diff --git a/.claude/agent-memory/e2e-test-engineer/known-flakes-and-regressions.md b/.claude/agent-memory/e2e-test-engineer/known-flakes-and-regressions.md index 902bb9655..13412ac99 100644 --- a/.claude/agent-memory/e2e-test-engineer/known-flakes-and-regressions.md +++ b/.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). diff --git a/.claude/agent-memory/product-architect/MEMORY.md b/.claude/agent-memory/product-architect/MEMORY.md index 1cbc88a80..f74da89ba 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; 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) diff --git a/.claude/agent-memory/product-architect/client-pdf-pipeline.md b/.claude/agent-memory/product-architect/client-pdf-pipeline.md index c04c0e669..6b2fb74a6 100644 --- a/.claude/agent-memory/product-architect/client-pdf-pipeline.md +++ b/.claude/agent-memory/product-architect/client-pdf-pipeline.md @@ -523,3 +523,64 @@ figure that still appears at ADR-034 lines 105, 120, 129, 138 and in the Deviati Narrowest text column is **Vendor at 45pt** (`VENDOR_WIDTH`, `overviewPdf.ts:26`, `VENDOR_SAFE_TOKEN_CHARS` = 5) — it is the binding constraint for the `wordBreak` rule, not Usage, and as of PR #2008 it has no `_minWidth` coverage. + +## #1973 / PR #2010 — column-visibility geometry engine (reviewed 2026-08-05, CHANGES REQUESTED) + +The two hardcoded table shapes are gone. `overviewPdf.ts` now renders any of **96 legal column +subsets** (64 budget-overview + 32 claim; `allocatedAmount` locked, R1), driven by +`client/src/lib/reportContent/columns.ts` — the AC 2.1 single derivation consumed by *both* +`overviewPdf.ts` and `ReportContentEditor.tsx`. + +**Width mechanism (endorsed): single absorber.** `computeColumnWidths(visible)` picks +`usage` → else `vendor` → else `null`; the absorber takes `usableColumnWidth(n) − fixedSum`, every +other column keeps its pinned constant. This discharges R7/AC 3.1–3.5 *algebraically* +(`total = printableWidth()` exactly with an absorber, strictly less without) instead of by 96 +assertions. `'*'` was correctly rejected (#1929 `columnCalculator.js` case-1); proportional +slack-sharing would violate R7's "don't stretch a numeric table across 515pt". + +**Key derived fact: `USAGE_WIDTH_7COL` (138.28pt) is the NARROWEST Usage width across all 96 +subsets.** Removing any column both shrinks `fixedSum` (≥ 40pt) and grows `usableColumnWidth` +(+8.5pt), so Usage is strictly monotone-decreasing in column count. That is why +`MAX_SAFE_USAGE_CHUNK_CHARS` (650, measured at 138.28pt) needed **no re-measurement** — hiding +columns only makes it more conservative. `usageChunkCharsForWidth` is a deliberate one-sided clamp +(`min(650, floor(650 · w/138.28))`): scales down for a future *added* column, never up. + +**Standing review lesson — a wiki page can state a PROHIBITION that a later PR deletes.** +ADR-034's "Geometry constraint that blocks a feature" (line 153) said the PDF column count is +fixed at 6 or 7 and that wiring the toggles through "is a re-measurement story, not a UI change." +PR #2010 falsified every clause and touched no wiki file. **When reviewing a PR that removes a +constraint, grep the wiki for the constraint's own statement, not just for the API/schema surface +the PR touches** — a stale prohibition actively steers the next agent away from work that already +shipped. Also stale on that page and folded into the same ask: line 144 quotes +`packUsageCellRows(segments, MAX_SAFE_USAGE_CHUNK_CHARS)` (now per-subset `usageChunkChars`), and +line 155's "Related sharp edge" says `packUsageCellRows` **hangs** on `maxChars <= 0` — it throws +(`overviewPdf.ts:338`), and its own prediction ("it matters the moment it becomes computed") went +live with this PR. + +**Summary-label three-tier fallback (R2)** — tier 1 last visible leading column, tier 2 +`invoiceAmount`, tier 3 a stack block below the table. Real subset counts are **88 / 4 / 4**; the +issue body and the test title both say "92" for tier 1 (they merged tiers 1+2). Tier 3's block is +**not width-constrained** — two implicit `'*'` columns across 515.28pt while the +`{allocatedAmount}`-alone table is only 84pt, so the total floats 431pt from its column (2 of 96 +subsets; flagged M3). + +**ADR-034 debt for #1973 is PAID (wiki `eb24774`, 2026-08-05, by me during PR #2010 review.)** The old +"Geometry constraint that blocks a feature" section is replaced by **"Column geometry is a computed +engine, not two pinned shapes"**, which now carries the narrowest-Usage-width proof, the absorber +rationale (and why `'*'` / proportional slack-sharing were rejected), the one-sided clamp, the +residual "adding a column IS still a re-measurement story" constraint, and the forcing-function +audit. `reportContent/columns.ts` is in the module structure. Same commit added Architecture.md's +**"Multi-step wizard state: tier factories"** subsection — the `freshXTier()`-spread convention had +never been documented anywhere despite being the fix for #1943/#1946/#1947. + +**Still open on ADR-034 after this pass** (all pre-existing, none introduced by #1973): every quoted +`69.28`/`33.54`/`266.16pt` figure is stale (lines ~105/120/129/138), and rule #1 says "every cell" +while `realRender.test.ts` covers only Usage — Vendor at 45pt is the binding column. + +**Exhaustiveness audit pattern for a keyed geometry engine.** Compile-error-enforced here: +`PINNED_WIDTHS: Record`, `HEADER_LABEL: Record`, +`buildBodyCell`'s `default`-less switch. **Not** enforced: `OVERVIEW_COLUMNS`/`CLAIM_COLUMNS` in +`columns.ts`, `LEADING_COLUMNS`/`RIGHT_ALIGNED_COLUMNS`, and the absorber ternary — all hand-typed +lists over the same union, and the tests that "count" them pin the literals 7/6, so a new key is +silently absent everywhere. Recommended fix shape: derive the canonical order from an exhaustive +`Record` and define the base sets as filters over it. diff --git a/.claude/agent-memory/product-architect/recurring-patterns.md b/.claude/agent-memory/product-architect/recurring-patterns.md index abe21f03f..b4594e259 100644 --- a/.claude/agent-memory/product-architect/recurring-patterns.md +++ b/.claude/agent-memory/product-architect/recurring-patterns.md @@ -1296,3 +1296,33 @@ where DE is the documented binding locale (#1937). Usage at 138-187pt is the _wi **How to apply:** when a test lands against a universally-quantified documented rule, check the quantifier. Picking the column the issue happened to mention is not the same as picking the binding one — and when the render already happened, iterating all cells is nearly free. + +## A wiki page can state a PROHIBITION that the PR under review deletes + +**Why:** #1973/PR #2010 generalised `overviewPdf.ts` to 96 column subsets. ADR-034's "Geometry +constraint that blocks a feature" said the PDF column count is fixed at 6 or 7 and that wiring the +wizard's toggles through "is a re-measurement story, not a UI change." The PR touched no wiki file, +so the merged state would have documented the shipped feature as impossible — actively steering the +next agent away from it. Same page, same round: a quoted constant reference (`MAX_SAFE_USAGE_CHUNK_CHARS` +→ per-subset `usageChunkChars`) and a "this function **hangs** on `maxChars <= 0`" claim that a prior +fix had already turned into a throw. + +**How to apply:** on any PR that *removes* a limitation, grep the wiki for the limitation's own +statement — not just for the API/schema surface the diff touches. Constraint prose lives in ADR +Consequences and "sharp edge" sections that no schema/contract diff would ever point you at. Bonus +tell: if the issue body cites a wiki constraint as its motivation, that exact paragraph is the one +the PR must rewrite. + +## Exhaustiveness audit for a keyed engine: list which links are compile-enforced and which are not + +**Why:** #1973's geometry engine keys everything off a `ReportColumnKey` union. Three links force a +new key at compile time (`Record`, `Record`, a +`default`-less switch with a declared return type). Four do not — the base-set arrays in `columns.ts`, +`LEADING_COLUMNS`, `RIGHT_ALIGNED_COLUMNS`, and the absorber-priority ternary — and the tests that +look like they'd catch it pin literal counts (`toHaveLength(7)`) derived from the test's own hand-typed +array, so they can't. + +**How to apply:** when reviewing a union-keyed engine, enumerate every site that consumes the union and +classify each as forcing or non-forcing; report the non-forcing ones even when they are correct today. +The cheap fix is almost always to derive the hand-typed list from an exhaustive `Record` and +filter, which converts a silent omission into a compile error. diff --git a/.claude/agent-memory/product-owner/MEMORY.md b/.claude/agent-memory/product-owner/MEMORY.md index 4cf80f37b..c90014d75 100644 --- a/.claude/agent-memory/product-owner/MEMORY.md +++ b/.claude/agent-memory/product-owner/MEMORY.md @@ -46,7 +46,7 @@ Full detail in [standalone-bugs-and-stories.md](standalone-bugs-and-stories.md) - **PR #2004 (#1888 + #1910)** — **APPROVED, M1 closed round 5** (`6a3eb7ec`, 2026-08-05): `lang={lang}` on the `
` breaks baseline alignment with sibling cells, independent of font-size** — a column-direction flex container's baseline is *synthesized* per spec rather than derived from real text-baseline metrics, so it doesn't line up with sibling ``s using default `vertical-align: baseline`. Fix: `vertical-align: top` on the shared `.table td` rule (kills reliance on baseline entirely) + drop `display:flex` from the desktop cell variant, replacing `gap` with `> div + div { margin-top: ... }` for plain block-stacked spacing. Where the *same class* is reused as a mobile `
` (not a `
`), keep the flex+gap version but move it under the mobile media query — safe specifically when the two usages are already mutually exclusive via existing `display:none` breakpoints (verify that before assuming it's safe to fork the class by viewport). +- **"Mobile card" ≠ assume it exists — verify.** The issue's ACs referenced "the mobile card" for invoice rows; `ReportInvoiceList`'s top-level `.invoiceRow`/`.listHeader` turned out to be a single CSS Grid at all viewports (no separate mobile-card JSX branch — `mobileCardList`/`mobileCard` classes in this file only exist for the _nested_ budget-lines/deposits sub-tables). Corrected this in the spec rather than designing a card layout that would have been an out-of-scope restructuring; touch-target sizing was applied unconditionally (not gated behind a mobile media query) since there's no separate mobile layout to gate it in. +- **`display: flex` on a `` breaks baseline alignment with sibling cells, independent of font-size** — a column-direction flex container's baseline is _synthesized_ per spec rather than derived from real text-baseline metrics, so it doesn't line up with sibling ``s using default `vertical-align: baseline`. Fix: `vertical-align: top` on the shared `.table td` rule (kills reliance on baseline entirely) + drop `display:flex` from the desktop cell variant, replacing `gap` with `> div + div { margin-top: ... }` for plain block-stacked spacing. Where the _same class_ is reused as a mobile `
` (not a `
`), keep the flex+gap version but move it under the mobile media query — safe specifically when the two usages are already mutually exclusive via existing `display:none` breakpoints (verify that before assuming it's safe to fork the class by viewport). - **Shared `grid-template-columns` string between a list header and its rows (`.listHeader`/`.invoiceRow` both defining the same 6-then-7-track template) is a drift hazard** — when one fix (adding a trailing column) and another fix (header-checkbox alignment) land in the same PR, flag explicitly that both edits target the same literal string in two places so they don't get applied against stale copies of each other. - **`Tooltip`'s `aria-describedby` goes on a `display:contents` wrapper span, not the focusable child** — doesn't reliably get announced on focus. Not a blocker when the focusable element already carries its own `aria-label` (name comes from there, independent of Tooltip's describedby wiring) — noted as a known pre-existing gap, not something to fix per-usage. @@ -45,16 +45,16 @@ Adds an opt-in "Enable AI assistance" toggle (Step 4) + "Generate with AI" batch Client-only content-model cleanup on `ReportContentEditor.tsx`/`overviewPdf.ts` (mostly de-numbering footnotes + moving 2 facts from footnote-only to inline). Spec posted covering all 5 ACs. - **"Deposit" inline label reuses `Badge`'s existing `.attachmentDeposit` variant** (`--color-attachment-deposit-bg`/`-text`, teal) rather than inventing a new badge color — the exact same wording (`sourceReports.table.attachmentType.deposit`) already exists as a Document-Type badge elsewhere in the same reports feature (`ReportInvoiceList.tsx`). When a new inline "this row is an X" label is needed, check `attachmentType`-style badges first — this app already has a full palette of document/entry-type pills before reaching for a new variant. -- **PDF "no Badge" fallback pattern**: pdfmake has no pill primitive, so a Badge-equivalent becomes a bracketed plain-text suffix `(Deposit)` in the *same* translation key, rendered as a separate lower-weight text run (`{ text: '...', color: '#6b7280', fontSize: 8 }`) via pdfmake's array-of-runs `text` field — not a new stacked line, not a filled/colored cell (per-run `fillColor` doesn't compose reliably with a whole-cell `text` array in pdfmake tables). `#6b7280` is the PDF-baked literal equivalent of `--color-text-muted`/`--color-gray-500` (confirmed via `merge.ts`'s hardcoded `styles.small.color`); PDF exports have no dark mode by design (fixed light-background documents), so hardcoded hex is expected/correct there, unlike in `client/src/**/*.css` where it would be a stylelint violation. +- **PDF "no Badge" fallback pattern**: pdfmake has no pill primitive, so a Badge-equivalent becomes a bracketed plain-text suffix `(Deposit)` in the _same_ translation key, rendered as a separate lower-weight text run (`{ text: '...', color: '#6b7280', fontSize: 8 }`) via pdfmake's array-of-runs `text` field — not a new stacked line, not a filled/colored cell (per-run `fillColor` doesn't compose reliably with a whole-cell `text` array in pdfmake tables). `#6b7280` is the PDF-baked literal equivalent of `--color-text-muted`/`--color-gray-500` (confirmed via `merge.ts`'s hardcoded `styles.small.color`); PDF exports have no dark mode by design (fixed light-background documents), so hardcoded hex is expected/correct there, unlike in `client/src/**/*.css` where it would be a stylelint violation. - **Conditional block removal — no placeholder, just omit from the render tree**: for "this metadata block doesn't apply to this report type" (claim reports skip `sourceInfoBlock`), the correct spec is `{condition &&
...}` (full omission), relying on the parent's existing `display:flex; gap: var(--spacing-N)` to naturally close the space — never a `display:none`-but-present placeholder or a manually tightened margin override. Same principle applies to the pdfmake side: skip the whole `content.push(...)` call rather than pushing an empty/near-empty stack. -- **Secondary/muted metadata line under an editable field**: reused the `.dateLineLabel`/`.footnotes` muted-xs-text convention (`--font-size-xs` + `--color-text-muted`) for the new "area name" sub-line under Usage — this is the established in-file precedent for "annotation, not content" text, not a new pattern. Placed *below* the `EditableField`, never inline/parenthetical beside it, specifically because AC required it be visually distinguishable as non-editable — inline-beside-an-input reads as part of the same string. +- **Secondary/muted metadata line under an editable field**: reused the `.dateLineLabel`/`.footnotes` muted-xs-text convention (`--font-size-xs` + `--color-text-muted`) for the new "area name" sub-line under Usage — this is the established in-file precedent for "annotation, not content" text, not a new pattern. Placed _below_ the `EditableField`, never inline/parenthetical beside it, specifically because AC required it be visually distinguishable as non-editable — inline-beside-an-input reads as part of the same string. - **PDF stack-building refactor flagged, not just a style note**: extending `overviewPdf.ts`'s Usage cell from a 2-way ternary (`attachmentsNote ? stack : text`) to a 3-optional-line array build (usage + area + attachmentsNote) is a real code-shape change for `frontend-developer`, called out explicitly in the spec as an implementation note so it isn't missed as "just add one more line." ## Issue #1931 — Single "Enhance with AI" action (removes #1901's double opt-in) Deletes Step 4's "Enable AI assistance" checkbox entirely; Step 5's action gates on `llmEnabled` (from `GET /api/config`) instead of the now-removed `aiEnabled`, and relabels "Generate with AI" → "Enhance with AI". Corrects course on the opt-in-toggle pattern this same memory file recorded for #1901 — that pattern is now retired for this feature. -- **Removing a `.settingsDivider` section from the middle of a flex-column `gap`-based card is a clean no-artifact deletion** — confirmed by checking that each section's separator is a *top* border owned by that section itself (not a trailing divider owned by the section above), and spacing comes from `gap` not margin-bottom. Deleting the last child leaves nothing vestigial. This is the general check to run before flagging "orphaned divider" concerns on any `settingsCard`/`settingsDivider`-shaped removal — don't assume a spacing bug exists without tracing which element owns which border. +- **Removing a `.settingsDivider` section from the middle of a flex-column `gap`-based card is a clean no-artifact deletion** — confirmed by checking that each section's separator is a _top_ border owned by that section itself (not a trailing divider owned by the section above), and spacing comes from `gap` not margin-bottom. Deleting the last child leaves nothing vestigial. This is the general check to run before flagging "orphaned divider" concerns on any `settingsCard`/`settingsDivider`-shaped removal — don't assume a spacing bug exists without tracing which element owns which border. - **"Revealed by opt-in" → "always present" gating change does NOT by itself require new visual weight.** The existing `.aiGenerateRow` bordered container + `btnSecondary` (not `btnPrimary`) already read as "optional, user-initiated, not a required step" — that visual signal comes from button hierarchy/container styling, not from conditional-rendering-as-a-proxy-for-optionality. When a future story removes a gating toggle, check the button's own style tier before assuming the removal demands a prominence change. - **A11y gap from removing an opt-in's helper text**: the deleted checkbox's helper text was the only place (for AT and sighted users alike) that pre-explained the action's overwrite behavior before first encounter. Recommended fix pattern: a static (not conditionally-swapped) visually-hidden `sharedStyles.srOnly` span + `aria-describedby` on the button, describing the destructive-of-edits behavior unconditionally — cheaper and more reliable than trying to mirror the sighted user's click-triggered confirm-modal experience for screen-reader users in advance. Reusable pattern for any "action always visible, consequence disclosed only after click via modal" case. - **German label for "Enhance" (vs. existing "generieren"/"aktivieren" copy)**: recommended **"Mit KI verbessern"** over "Mit KI überarbeiten" — "verbessern" (improve) is the tighter semantic match for "enhance existing content" than "überarbeiten" (rework/substantially rewrite), and preserves the established verb-first "Mit KI ___" sentence shape. Anchors the translator's discretion call; formal `KI` glossary entry still pending in #1917. @@ -170,6 +170,17 @@ Part of the Bank Report Wizard Refinement Round 2 mini-epic (follows #1898). Spe - **Found a real responsive risk while auditing `WizardStepper` for a 5th step**: the desktop `