Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/agent-memory/e2e-test-engineer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
- [bug-1829-shard3-flakes.md](bug-1829-shard3-flakes.md) — root cause + fix for the shard-3 diary flakes blocking main promotions; also proves `maxFailures` already tolerates one retry-passing test (Playwright source citation).
- [story-1876-deposit-refunds.md](story-1876-deposit-refunds.md) — deposit `entryType` refunds; DataTable hidden-column read pattern (`getColumnCellText`), table+card dual-DOM visible-filter pitfall.
- [story-1877-contact-fields-attachment-typing.md](story-1877-contact-fields-attachment-typing.md) — budget-source contact fields, household settings singleton (needs `mode:'serial'`, not just try/finally), document-link attachment typing; route-mock last-registered-runs-first ordering hazard; sandbox lint/tsc caveats.
- [story-1879-report-wizard.md](story-1879-report-wizard.md) — Bank Report Wizard POM/spec; Blocker bug #1886 (budgetSources envelope crash blocks all progress past step 1) + compile errors/missing i18n keys found via `tsc`; source-report E2E seeding pattern (WI budget → invoice-budget-line link).
- [story-1879-report-wizard.md](story-1879-report-wizard.md) — Bank Report Wizard POM/spec; Blocker bug #1886 (budgetSources envelope crash blocks all progress past step 1) + compile errors/missing i18n keys found via `tsc`; source-report E2E seeding pattern (WI budget → invoice-budget-line link); Story #1899 added a 5th step ("Settings" — report language + moved toggles), preview iframe only exists on step 5 now.
- [story-1891-wizard-followup.md](story-1891-wizard-followup.md) — expandable invoice rows, CSP `blob:` frame-src hardened preview check (SUPERSEDED TWICE — `page.frames()` proof, then in-page blob fetch → now header+console-only, see general-e2e-patterns.md), deposit budget-source tagging; 2 filed bugs (#1892 full-exclusion display, #1893 missing deposit-default heuristic); sandbox CAN now build+boot the full container stack (see `sandbox-live-verification.md`) but browser binary download is still network-policy-blocked.
- [sandbox-live-verification.md](sandbox-live-verification.md) — **dhi.io build access is sandbox-dependent, re-check each session**: this session successfully built `cornerstone:e2e` and booted the full container stack (app+OIDC+proxy all healthy), a first — but Playwright's own browser binary download (`playwright.download.prss.microsoft.com`/`cdn.playwright.dev`) is blocked by network policy, and Ubuntu's `chromium-browser` apt package is a non-functional snap stub (no snapd) — no way found yet to get an actual live browser run in this sandbox class.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: story-1879-report-wizard
description: Story #1879 Bank Report Wizard (/budget/reports) — POM/spec authored against a frontend implementation with a Blocker runtime crash (filed #1886); orientations.spec.ts and invoices.spec.ts:395 known-flake fixes confirmed self-healed on beta before this story.
description: Story #1879 Bank Report Wizard (/budget/reports) — POM/spec authored against a frontend implementation with a Blocker runtime crash (filed #1886); orientations.spec.ts and invoices.spec.ts:395 known-flake fixes confirmed self-healed on beta before this story. Story #1899 added a 5th step (Settings — report language + moved toggles).
metadata:
type: project
---
Expand Down Expand Up @@ -168,3 +168,48 @@ with "received 6" on PR #1883 (desktop/tablet/mobile) — PR #1883 added a 6th "
(Household, Areas, ...)") landed in the very next commit (`fc7f6ad2`, PR #1885) and is present at
my branch's HEAD. No new regressions from this triage — see `known-flakes-and-regressions.md` for
the canonical entries; this file only records the fresh confirmation.

## Story #1899 — 5th step inserted ("Settings"), old step 4 renumbered to step 5

`ReportWizardPage.tsx` gained a NEW step 4 ("Settings" — `Step4Settings.tsx`): a report-language
radio group (`input[name="reportLanguage"][value="en"|"de"]`, plain/visible, literal
"English"/"Deutsch" labels NOT wrapped in `t()`) PLUS the `#attachDocuments`/`#includeCoverLetter`
toggles relocated verbatim from the old step 4 (same DOM ids). The old step 4 (preview iframe +
`Step4Options.tsx`) was renamed `Step5Actions.tsx` and pushed to step 5, content byte-identical
minus the toggle props/JSX. **Key consequence for every E2E scenario that used to do
`goNextFromStep3(); waitForPreviewReady();`**: the preview `<iframe>`/`ReportPdfPreview` component
is now ONLY mounted at step 5 — there is no way to observe `previewLoadingOverlay`/
`previewIframe`/`getPreviewSrc()` while on the new step 4. Every such call site became
`goNextFromStep3(); step4NextButton.click(); waitForPreviewReady();` (12 call sites across
`reportWizard.spec.ts` + `reportWizardExpansion.spec.ts`). One consequence: Scenario 1's old
"toggle attach/cover-letter, verify regeneration via `waitForPreviewRegenerated` after EACH
individual toggle" pattern no longer works (nothing to observe on step 4) — simplified to
"toggle all options on step 4 (asserting only checkbox check-state, not regeneration), advance
once, single `waitForPreviewReady()` on step 5". The still-valid multi-step regeneration proof
(`getPreviewSrc()` before, `waitForPreviewRegenerated(previousSrc)` after) now requires reaching
step 5 fully (via step 4) each time, and going back now needs TWO `goBack()` calls (step 5 → step
4 → step 3) instead of one — `goBack()` itself needed NO change since it's a generic
`[class*="buttonRow"] [class*="btnSecondary"]` re-query (works for whichever step is currently
mounted; only one `buttonRow` div is ever in the DOM at a time across all 5 steps, confirmed via
grep — this is also why `step2BackButton`/`step3BackButton`/`step4BackButton` are literally the
same object reference, and `.first()`/`.last()` are never load-bearing disambiguators, just
defensive tie-breakers).

Added `reportLanguageRadio(lang)`/`selectReportLanguage(lang)` (POM) and a new Scenario 12
(`reportWizard.spec.ts`) proving a `reportLanguage` change on step 4 regenerates the preview
(blob src change, via `waitForPreviewRegenerated`) while the wizard's OWN chrome (heading,
stepper labels, Back/Next button text) stays English — `reportLanguage` only feeds
`generateReportPdf`'s fixed-locale `i18n.getFixedT(reportLanguage, 'budget')`/`createFormatters()`
pair, completely independent of the app's `resolvedLocale`/normal `t`. Deliberately did NOT
assert on PDF byte content (that's the Jest `realRender.test.ts` unit test's job per the QA
spec) — only the blob-src-changed proof + suggestedFilename pattern (still untranslated:
`${useCase}-${slug}-${date}.pdf`, confirmed unaffected).

Verified via `npx eslint`, `npx prettier --check`, `npx tsc --noEmit -p e2e/tsconfig.json`
(zero errors referencing ReportWizard files; the ~123 pre-existing errors in ~22 unrelated files,
mostly `capture-docs-screenshots.spec.ts`'s `TestDetails` type mismatch, are the same
sandbox/stale-@playwright/test-version noise as prior stories — confirm this is still true each
session rather than assuming). No live Playwright run — browser binary download is blocked by
network policy in this sandbox (`cdn.playwright.dev`/`playwright.download.prss.microsoft.com`
both 403 "blocked by network policy: domain ... — no matching allow rule"), consistent with
`sandbox-live-verification.md`'s finding. CI's full E2E matrix is the real verification gate.
21 changes: 21 additions & 0 deletions .claude/agent-memory/product-architect/client-pdf-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,24 @@ and pdf-lib merges/appends existing Paperless PDFs -- pdfmake cannot embed forei

`GET /api/source-reports`, `POST /api/source-reports/mark-claimed`,
`POST /api/paperless/documents`, `GET /api/paperless/documents/:id/preview`.

## Report language is decoupled from the UI locale (Story #1899, PR #1903)

The report's language is chosen per-run in the wizard's Settings step and is **independent of the
app's ambient locale**. Two locale-bound artifacts are built in `ReportWizardPage.tsx` and passed
into `generateReportPdf` -- nothing else in the pipeline is locale-aware:

- `reportT = i18n.getFixedT(reportLanguage, 'budget')` -- works synchronously because
`client/src/i18n/index.ts` statically imports *both* `en` and `de` bundles into one `resources`
object. **Never call `i18n.changeLanguage()` to switch report language** -- that mutates the app UI.
- `reportFormatters = createFormatters(localeTag, currency)` in `client/src/lib/formatters.ts`.
`useFormatters()` is now a thin wrapper over the same factory, so UI and report formatting can
never diverge in implementation (only in bound locale).

Why `reportPdf/*` needed zero changes: `merge.ts` / `overviewPdf.ts` / `coverLetterPdf.ts` already
took `t: TFunction` + `formatters?: Formatters` params and contain **no** ambient `i18n`, `Intl.`, or
`toLocale*` usage, and **no namespace-prefixed keys** (all keys are bare `sourceReports.*` in the
`budget` ns). Preserve both properties -- a single `t('common:…')` call or a raw `Intl` use inside
`reportPdf/` would silently leak the UI locale into the exported PDF.

TODO (mine): ADR-034 does not yet record this contract. Add it.
19 changes: 19 additions & 0 deletions .claude/agent-memory/product-architect/story-reviews.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,22 @@ Request changes (2 critical, 3 high, 2 medium):

Round 1 CHANGES_REQUIRED (1 critical: Rail A/B double-count), round 2 APPROVED after commit 86a9770a.
Full invariants and review heuristics in `dual-rail-aggregation.md`.

## PR #1903 (2026-07-31) -- Story #1899 report-language Settings step (5-step wizard)

APPROVED (2 medium, 3 low). Architecture verified sound; see `client-pdf-pipeline.md` for the
locale-decoupling contract this established.

- **M1**: `reportLanguage` seeded once via `useState(resolvedLocale)`. `LocaleContext.syncWithServer`
sets `resolvedLocale` *asynchronously* after auth AND deletes the `locale` localStorage key when
the server has a preference -- so every load starts at `detectBrowserLocale()` and flips later.
Any component seeding state from `resolvedLocale` at mount has this bug. **Reusable review check:
never `useState(resolvedLocale)` / `useState(currency)` / `useState(vatRate)` without a
seed-until-touched effect.** jsdom tests can't catch it (LocaleProvider's async paths never resolve).
- **M2**: `resolvedLocale === 'de' ? 'de-DE' : 'en-US'` duplicated in `formatters.ts` and the page.
Needs one exported `resolvedLocaleToIntlTag()` -- silent divergence risk on a 3rd locale.
- L: `.step4Body/.step4Layout/.step4Column` class names + `stepper.options` key left off-by-one
after the renumber; `createFormatters` return type written inline instead of a named interface;
filename AC vacuously satisfied (ISO date + raw enum, nothing locale-dependent -- flag to PO).
- CI: Quality Gates green; E2E shard 5/16 red (known recurring flake in this wizard area -- must be
triaged before beta->main, not before the beta merge).
2 changes: 1 addition & 1 deletion .claude/agent-memory/product-owner/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Full detail in [standalone-bugs-and-stories.md](standalone-bugs-and-stories.md)
- Auto-itemize: #1545/#1546/#1547 mini-epic (2026-05-21), #1600 (2026-05-26), **#1833 duplicate budget lines on commit retry (2026-07-07)**
- Diary: #1426 critical photo data loss (2026-05-15)
- Photo: #1723 lightbox picker UX (2026-06-16)
- **Bank Report Wizard mini-epic** (no parent epic): #1876 refunds (PR #1880) → #1877 contact/household/attachment typing (PR #1883) → #1878 report backend → #1879 wizard+PDF (PR #1887, CHANGES_REQUIRED then **APPROVED** round 2, 2026-07-30). Domain decisions, contract facts (refund sign, `isSplit`, `totalAmount` scope) and deferrals in [bank-report-wizard.md](bank-report-wizard.md). **Refinement Round 2** (2026-07-31, no parent epic, all Todo): #1898 report table refinements → #1899 settings step + report language → #1900 editable HTML preview → #1901 AI usage/cover-letter generation. Open: **#1891** user-verification follow-up (Todo, PR #1894 **APPROVED** 32/32 round 2, 2026-07-30 — 2 wiki MUST FIX outstanding); **#1888** stage-matched attachment indicator (Backlog, blocked-by #1879); **#1895** HIGH claim close-out cross-source sweep, **#1896** quotation-deposit 409 (blocked-by #1895), **#1897** deposit-blind drill-down — all Backlog, from the #1891 architect audit; E2E shard 5 pre-existing flake must be triaged before promoting to `main`.
- **Bank Report Wizard mini-epic** (no parent epic): #1876 refunds (PR #1880) → #1877 contact/household/attachment typing (PR #1883) → #1878 report backend → #1879 wizard+PDF (PR #1887, CHANGES_REQUIRED then **APPROVED** round 2, 2026-07-30). Domain decisions, contract facts (refund sign, `isSplit`, `totalAmount` scope) and deferrals in [bank-report-wizard.md](bank-report-wizard.md). **Refinement Round 2** (2026-07-31, no parent epic, all Todo): #1898 report table refinements (PR #1902, merged) → #1899 settings step + report language (PR #1903, PO review 2026-07-31: **APPROVED w/ 1 MUST FIX** — 5-step wizard, `getFixedT`/`createFormatters` threading and en/de report output all verified; AC 2.2 default-locale seeding is stale on hard load, see [pr-review-patterns.md](pr-review-patterns.md) `useState(contextValue)` entry) → #1900 editable HTML preview → #1901 AI usage/cover-letter generation. Open: **#1891** user-verification follow-up (Todo, PR #1894 **APPROVED** 32/32 round 2, 2026-07-30 — 2 wiki MUST FIX outstanding); **#1888** stage-matched attachment indicator (Backlog, blocked-by #1879); **#1895** HIGH claim close-out cross-source sweep, **#1896** quotation-deposit 409 (blocked-by #1895), **#1897** deposit-blind drill-down — all Backlog, from the #1891 architect audit; E2E shard 5 pre-existing flake must be triaged before promoting to `main`.

## Requirements Coverage

Expand Down
1 change: 1 addition & 0 deletions .claude/agent-memory/product-owner/pr-review-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ When a PR adds a mobile card list beside a desktop table, re-check rather than a
- **Duplicate fetch logic** — `useEffect` fetch body duplicated for re-fetch after delete; extract shared fetch.
- **Dead placeholder pages after route refactor** — PR #150 left old BudgetPage after swapping to BudgetCategoriesPage. Delete orphans.
- **COOKIE_NAME duplication** — triplicated across `plugins/auth.ts`, `routes/auth.ts`, `routes/oidc.ts`. Extract to shared constant.
- **`useState(contextValue)` seeding from LocaleContext/ThemeContext is stale-prone** — `useState(resolvedLocale)` only runs its initializer on first render, but `LocaleContext.syncWithServer` (fired from `App.tsx`'s `LocaleServerSync` after auth) applies the server-stored locale *asynchronously* AND clears `localStorage`, so every later hard load starts at the browser-detected locale before flipping. Any page seeding state from `resolvedLocale`/`currency`/`vatRate` at mount can capture the pre-sync value and never correct. Fix pattern: `const [override, setOverride] = useState<T | null>(null); const value = override ?? contextValue;` — default tracks context, explicit choice sticks. Found in PR #1903 (AC "defaults to my current UI locale"). Same class of bug applies to `ThemeContext.syncWithServer`.

## Chore/Maintenance PR patterns

Expand Down
2 changes: 1 addition & 1 deletion .claude/agent-memory/ux-designer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
- `role="status"` already implies `aria-live="polite"` — never add both attributes.
- `BadgeVariantMap` entries need both `label` and `className` — a missing `className` makes the variant's style dead on arrival.
- New component reuse audit: check `Badge`, `SearchPicker`, `Modal`, `Skeleton`, `EmptyState`, `FormError` before proposing anything new; see [component-patterns.md](component-patterns.md) for established section-card/badge/picker conventions.
- Cannot `gh pr review --request-changes` **or** `--approve` on your own PRs (GraphQL rejects both, not just request-changes) — post a `gh pr comment` instead, with an explicit "Verdict: APPROVED/CHANGES_REQUIRED" line in the body. If a specific invocation is blocked by the sandbox's permission system, treat that as a real denial: report it to the orchestrator and let the human-configured policy decide — do not engineer alternative invocations to route around a block.
- Cannot `gh pr review --request-changes` **or** `--approve` on your own PRs (GraphQL rejects both, not just request-changes) — post a `gh pr comment` instead, with an explicit "Verdict: APPROVED/CHANGES_REQUIRED" line in the body. This block is keyed to the underlying `gh` auth identity, not the PR's displayed `author.login` — a PR showing a human GitHub username (e.g. the repo owner) as author still hit "Can not approve your own pull request" (PR #1903), so don't skip the self-review check just because `gh pr view --json author` shows a person's name. If a specific invocation is blocked by the sandbox's permission system, treat that as a real denial: report it to the orchestrator and let the human-configured policy decide — do not engineer alternative invocations to route around a block.
- No segmented-control component exists in the app — for a "choose one of N" form field that isn't a `<select>`, reuse the plain `<input type="radio">` + `role="group"` pattern from `AutoItemizePage.module.css` `.modeSelector` (the only radio group in the codebase) rather than inventing a segmented/button-group look. See Issue #1876 in [feature-spec-history.md](feature-spec-history.md).
- For a "this is a negative/refund/adjustment row" signal, don't rely on red text color alone — pair it with a `Badge` carrying a real text label (reuse `--color-status-blocked-bg`/`-text`, the existing red badge pair — there's no separate `--color-danger-badge-*` family) plus `formatCurrency(-amount)`'s literal minus sign. Don't recolor a _computed remainder_ (e.g. "remaining amount") red just because refunds reduce it — that falsely implies an error state; only the actual refund row gets the negative treatment.
- `Intl.NumberFormat` enables thousands-grouping by default; `toFixed()`/manual string-building never did. When a PR swaps a `toFixed()` call for a locale formatter, script-verify the _old vs new_ output isn't just per-locale-correct but also identical for large values (≥1000) — grouping separators are an easy-to-miss 3rd English-output regression beyond whatever the PR explicitly discloses. See PR #1845 in [pr-review-findings.md](pr-review-findings.md).
Expand Down
Loading
Loading