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
1 change: 1 addition & 0 deletions .claude/agent-memory/e2e-test-engineer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
- [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.
- [story-1901-ai-report-generation.md](story-1901-ai-report-generation.md) — AI-generated usage descriptions/cover letter: new `reportWizardAiGeneration.spec.ts` + POM AI locators; llmEnabled-mock pattern; overwrite-modal-guard-checks-overrides-not-aiContent gotcha; cover-letter-fields-need-contactAddress/reference seed trap; reused auto-itemize LLM error strings.
- [story-1900-editable-report-preview.md](story-1900-editable-report-preview.md) — step 5 reworked from always-present auto-regenerating iframe → live editable surface + on-demand PDF Modal; POM rename `waitForPreviewReady/Regenerated` → `openPdfPreviewModal`/`closePdfPreviewModal` (must close before another modal-opening action); 4 filed bugs (#1904-#1907, all now FIXED+CLOSED, see file's re-verification section); deliberate deviation editing a file explicitly marked "do not touch" because leaving it broke `tsc` workspace-wide — see file for the general lesson; `footnoteFetchFailed` skip note naturally reachable with no Paperless container, no mock needed; re-verification added `mobileCard()`/`mobileUsageField()` POM locators + Scenario 15 (#1907 regression guard) + found/filed NEW bug #1908 (mobile-card fallback visible on desktop, no default `display:none`) with its own expected-to-fail Scenario 1b.

## Open follow-ups to flag to orchestrator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ metadata:

## Currently open / unresolved

(none — see Resolved section for the #1829 shard-3 fix)
- **Shard 5/16 recurring on every beta PR since ~2026-07-30 (report-wizard mini-epic PRs #1894/#1902/#1903/#1909, and the earlier full-matrix run 28934922175 doesn't show it — first clearly reproduced from PR #1894 onward)**: `invoices.spec.ts:841` "Effective Amount"/"Remaining Amount" column toggle AND `dashboard.spec.ts:608` (line shifted from 566 — "Card re-enable (Scenario 7): Customize dropdown lists dismissed card and clicking re-enables it") both fail, same shard, every run, both attempt+retry for the invoices one. **Correction to the "Resolved" entry below**: PR #1883's column-preference-singleton fix (`getColumnCellText` + defensive preference-DELETE) did NOT durably fix `invoices.spec.ts:841` — it recurred identically on 4 subsequent PRs. Not related to report-wizard work (zero diff in `invoices.spec.ts`/`dashboard.spec.ts` on those branches) — pre-existing, gates `E2E Gates` (main-only) but not beta merges. See orchestrator project memory `bank-report-wizard-followups.md` for the promotion-blocker tracking; needs a real fix pass (likely via `/fix-e2e`) before the next beta→main promotion, not before any individual story PR.

- `i18n/i18n.spec.ts` "German text does not overflow navigation sidebar on desktop" — pre-existing locale-init race, needs separate investigation.
- `i18n.spec.ts` "Key page headings render in German" — intermittent ~10-20%: concurrent worker `afterEach(resetToEnglish)` races with another test's `setLanguage('de')`. Pre-existing.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
---
name: story-1901-ai-report-generation
description: Bank Report Wizard AI-generated usage descriptions/cover letter (Story #1901) — new spec file, POM additions, mocking pattern for GenerateReportContent.
metadata:
type: project
---

Story #1901 adds an opt-in "Enable AI assistance" toggle (Step 4 Settings, `#enableAiAssistance`,
only rendered in the DOM at all when `GET /api/config`'s `llmEnabled` is true) and a "Generate
with AI" button (Step 5) to the Bank Report Wizard. One batched `POST
/api/source-reports/generate-content` call returns `{ letterSubject, letterBody, descriptions:
Record<invoiceId, string> }` and is applied as a NEW BASELINE (`applyAiContent`, layered before
manual `overrides`, not as an override itself) — so freshly-generated text shows NO edited-dot
indicator anywhere until a human subsequently edits it.

New spec file: `e2e/tests/budget/reportWizardAiGeneration.spec.ts` (8 scenarios: 1 real/unmocked +
7 with `**/api/config` + `**/api/source-reports/generate-content` route mocks). New POM additions
in `e2e/pages/ReportWizardPage.ts`: `aiToggle`, `aiGenerateRow`, `generateWithAiButton`,
`aiGeneratingCaption`, `aiErrorBanner`, `aiGeneratedNote`, `aiOverwriteConfirmModal` +
`aiOverwriteAndGenerateButton`/`aiOverwriteKeepEditingButton`, plus methods
`toggleAiEnabled`/`clickGenerateWithAi`/`confirmAiOverwrite`/`cancelAiOverwrite`. Added
`API.sourceReportsGenerateContent` to `e2e/fixtures/testData.ts`.

**Key gotchas found while writing this**:
- The E2E container config (`e2e/containers/cornerstoneContainer.ts`'s `environment` object) sets
no `LLM_*` env vars at all — `llmEnabled` is deterministically `false` against the real,
unmocked backend. This is what makes Scenario 1 (toggle absent) a TRUE e2e test with zero
mocking, and it's also why every other scenario MUST mock `GET /api/config` (`mockLlmEnabled` —
fetch-real-then-override-one-field, same pattern as `auto-itemize.spec.ts`'s
`mockConfigEnabled`) before it can reach the AI UI at all.
- `handleGenerateWithAiClick` in `ReportWizardPage.tsx` gates the overwrite-confirm modal on
`Object.keys(overrides).length > 0` ONLY — NOT on whether `aiContent` already exists. So
regenerating a second time right after a first AI generation (no manual edits since) runs
directly with no modal (Scenario 5). Don't assume "AI content already present" alone triggers
the guard for regeneration — it doesn't (only for the SEPARATE discard-confirm guard on step
1-4 mutations, where `isDirty = overrides.length > 0 || aiContent !== null` DOES include
`aiContent`).
- **Trap I hit and fixed**: `wizard.letterField('subject')`/`letterField('body')` only render at
all when `report.source.contactAddress` or `.reference` is set (drives `includeCoverLetter`'s
default). Any AI-generation scenario that touches the cover letter fields MUST seed the budget
source with `contactAddress`/`reference` — I initially forgot this on 3 of 7 mocked scenarios
(Scenarios 5, 6, 7) and had to backfill it. If a future edit adds a scenario using
`letterField(...)`, check the source seed includes both fields first.
- LLM error translations are SHARED, feature-neutral keys in `errors.json`
(`LLM_NOT_CONFIGURED`/`LLM_UNREACHABLE`/`LLM_INVALID_RESPONSE`/`LLM_UPSTREAM_ERROR`) used by
BOTH auto-itemize and report-content generation. **Reworded PR #1916 (2026-08-01, PO review
feedback)**: the original wording said "extraction service"/"Auto-itemization is not
configured", which was auto-itemize-specific and misleading when the same code renders for
report generation. Current (feature-neutral) English strings: `LLM_NOT_CONFIGURED` = "AI
assistance is not configured on this server.", `LLM_UNREACHABLE` = "The AI service could not be
reached. Please try again.", `LLM_INVALID_RESPONSE` = "The AI service returned an unusable
response. Please try again.", `LLM_UPSTREAM_ERROR` = "The AI service reported an error. Please
try again." (German: "Der KI-Dienst …" / "KI-Unterstützung ist auf diesem Server nicht
konfiguriert."). Updated the 3 e2e occurrences of the old wording (2 in
`reportWizardAiGeneration.spec.ts` — one in the `LLM_UNREACHABLE` mock body, one in the actual
`toContainText` assertion — and 1 in `invoice-auto-itemize-page.spec.ts`'s mock body, which
doesn't assert message text so was updated for fixture realism only, not test correctness). Use
the CURRENT `errors.json` strings above when asserting error
text, not a report-specific wording.
- `aiErrorBanner` is scoped to `aiGenerateRow` (`this.aiGenerateRow.locator('[role="alert"]')`) —
needed because the claim-flow's own error banner (`claimErrorBanner`) is a SEPARATE
`[role="alert"]` elsewhere on the same step-5 page; an unscoped `page.getByRole('alert')` would
strict-mode-collide once both could theoretically be present.
- Delayed-response gated-mock pattern (register route, await an externally-resolved `Promise<void>`
gate before `route.fulfill`) is the SAME technique already established in
`invoice-auto-itemize-page.spec.ts`'s `LLM_UNREACHABLE` scenario — reused verbatim, not
reinvented, to assert the pending spinner/caption state deterministically before releasing the
mock response.

**Not executed locally**: no live browser run in this sandbox session (see
`sandbox-live-verification.md` — Playwright's browser-binary download is still network-policy
blocked here). Verified via `npx tsc --noEmit -p e2e` (zero errors in the new/modified files; the
many pre-existing errors elsewhere in `e2e/` are unrelated repo-wide noise) and
`npx eslint`/`npx prettier --write` on just the touched files (clean).
19 changes: 13 additions & 6 deletions .claude/agent-memory/product-architect/client-pdf-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ took `t: TFunction` + `formatters?: Formatters` params and contain **no** ambien
`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.
DONE 2026-08-01: ADR-034 now records this contract (wiki master `254db1d`, "Addendum: report language is
decoupled from the UI locale").

## Content/layout split: `client/src/lib/reportContent/` (Story #1900, PR #1909)

Expand Down Expand Up @@ -151,8 +152,14 @@ One-line statement of the rule: **visible captions of exported data follow the r
screen-reader affordance sentences (`ariaLabel`, `resetAriaLabel`) are wholly UI language** — never splice a
report-language noun into a UI-language sentence. This pre-decides the same question for #1901.

**Forward note for #1901 (AI generation):** `ReportContentOverrides` has no provenance concept, and
`buildReportContent` is a closed pure derivation with no injection seam. AI text dumped into `overrides`
would show the "edited" dot on every field, offer a reset-to-non-AI-text, and be silently wiped by
`guardedUpdate` on any step 1-4 change. Design a third layer (`baseline -> generated -> user`) or a
`generatedText` parameter — do not let it default into the overrides map.
### AI layer (#1901, PR #1916) — the forward note was followed

`applyAiContent(content, aiContent | null)` sits **between** `buildReportContent` and `applyOverrides`, so
the layer order is `baseline -> AI -> user overrides`. AI text is therefore not "edited", gets no reset
affordance, and survives until `guardedUpdate` clears both. `guardedUpdate`'s dirty check was widened to
`Object.keys(overrides).length > 0 || aiContent !== null`. Empty strings from the generator fall back to
baseline rather than blanking a field. Keep this shape — putting AI output in `overrides` is the trap.

Server side: `POST /api/source-reports/generate-content` re-fetches the report (client sends selection only),
persists nothing, and reuses the single `budgetExtraction/` LLM gateway as a third provider method. Both
documented in ADR-034 "Addendum: content layers".
33 changes: 33 additions & 0 deletions .claude/agent-memory/product-architect/recurring-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,36 @@ step — a stylelint error is a hard beta-PR blocker. But `stylelint` only globs
`var(--color-refund-text)`, a token defined nowhere; the declaration is invalid-at-computed-value and the
styling silently does nothing. When reviewing client code, grep every `var(--…)` appearing inside a `.tsx`
against `client/src/styles/tokens.css`, and push the value into a CSS Module class instead.

## Monetary units are major currency units (2 dp) everywhere in this repo — never cents

`allocatedAmount`, `allocatedPortion`, `totalAmount`, `invoiceAmount`, and everything `formatCurrency`
consumes are **euros, rounded to 2 dp**. There is a `toCents()` helper in `sourceReportService.ts` but it is
used only *inside* a `toCents(x)/100` round-trip — it never escapes into a field.

PR #1916 (#1901) broke this across a new module seam: the service passed `inv.allocatedAmount` (euros) into
`GenerateReportContentLlmInvoice.amount`, and `prompts.ts` rendered `(inv.amount / 100).toFixed(2)` — every
figure in an AI-written bank cover letter came out **100× too small**. Root cause: the interface field had no
unit in its JSDoc. Coverage was 95.94% and green, because line coverage cannot catch a unit error — the only
test that catches it asserts the **rendered string** (`Total Amount: 12345.67 EUR`), and no test called
`buildReportContentUserPrompt` at all.

Same PR, second defect at the same spot: `Math.round(includedTotal)` (commented "round to nearest cent")
rounds to the nearest whole euro. Cent-rounding is `Math.round(x * 100) / 100`.

**Review rules that follow:**
- Any monetary value crossing a module boundary must carry its unit in the type's JSDoc.
- When a server path re-derives a total the client already derives, demand it mirror the client formula
*shape*, not just its intent — `applyLineExclusions` rounds **per invoice** then `buildReportContent` sums
the already-rounded values with no final round. A single trailing round is a different number.
- Grep new prompt builders for `/ 100`, `* 100`, and `toFixed(` — that is where unit assumptions hide.
- Better still: push shared derivations into `@cornerstone/shared` so there is one implementation
(recommended as M2 on #1916; not yet done).
- When a total is exclusion-adjusted, the **per-item** figures handed to the same consumer must be adjusted
too. #1916 shipped an adjusted total alongside raw per-invoice amounts — an LLM handed parts that do not
sum to the stated whole. Check both halves whenever you see an exclusion filter.

Fixed in `b70d821b` (round 2 of the #1916 review); the permanent guard is the
`amount formatting (major units — regression guard for the ×100 division bug)` describe block in
`server/src/services/budgetExtraction/prompts.test.ts`, which asserts rendered substrings **and** negative
assertions against the divided form. Copy that shape for any new prompt builder.
Loading
Loading