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
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ an `<iframe>` whose target content type the headless Chromium shell can't native
is the known case; likely also true for other browser-plugin-dependent content).**

**Replacement pattern — verify the CSP contract server-side, not the browser's enforcement of it:**

1. **Direct CSP header assertion** (the deterministic core): `page.request.get(route)` (shares
the page's cookie jar/session automatically) then read
`response.headers()['content-security-policy']`, regex out the specific directive
Expand All @@ -121,7 +122,7 @@ is the known case; likely also true for other browser-plugin-dependent content).
asked to render the content. This is the primary, CI-reliable regression guard now.
2. Keep a zero-CSP-console-violation assertion as defense in depth — real (non-headless-shell)
browsers still log synchronously on a block, so this remains a meaningful second signal even
though it can't be the *only* signal in CI.
though it can't be the _only_ signal in CI.
3. Do NOT try to fetch the iframe's `blob:` src from within the page as a third signal — see the
"blob: fetch is connect-src-governed, not frame-src" entry immediately below for why that's a
dead end, not just an unnecessary extra.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Even with the container stack fully working, `npx playwright test` still fails a
This is a `sbx policy allow network <domain>` decision for the user/host, not something to
work around unilaterally.
2. **Ubuntu's `chromium-browser` apt package is a non-functional snap stub.** `apt-get install
chromium-browser` succeeds and installs `/usr/bin/chromium-browser`, but running it (even
chromium-browser` succeeds and installs `/usr/bin/chromium-browser`, but running it (even
pointed to via a local `playwright.config.ts` override's `use.launchOptions.executablePath`)
fails immediately:
```
Expand All @@ -66,8 +66,8 @@ Even with the container stack fully working, `npx playwright test` still fails a
Ubuntu release. No other route to a real browser binary was found in this session
(no cached `~/.cache/ms-playwright` binaries, no `google-chrome`/`firefox` either).

**Net effect**: a fully live Playwright *browser* run is still not achievable in this sandbox
class, even though the *container/app* side now works. If a future story's AC needs an actual
**Net effect**: a fully live Playwright _browser_ run is still not achievable in this sandbox
class, even though the _container/app_ side now works. If a future story's AC needs an actual
red/green browser proof, either (a) ask the user to `sbx policy allow network
playwright.download.prss.microsoft.com,cdn.playwright.dev` for that session, or (b) fall back
to the established pattern: build+boot the containers to prove the app-level behavior (e.g. the
Expand Down Expand Up @@ -112,7 +112,7 @@ module resolution works — a file outside the workspace fails with `Cannot find
- `rm` any scratch `playwright.*.config.ts` file.
- `rm -rf e2e/e2e/ e2e/playwright-output/ e2e/playwright-report/ e2e/test-results/` — a
misconfigured `--config` path or an interrupted run can leave a nested `e2e/e2e/test-results/
.state/containers.json` artifact (from `containers/setup.ts`'s state file) that `git status`
.state/containers.json` artifact (from `containers/setup.ts`'s state file) that `git status`
won't flag as tracked but that still clutters the worktree.
- `docker images` / `docker rmi cornerstone:e2e` if you built a throwaway/deliberately-broken
image for a red-test proof — a stale local image with the wrong config baked in will silently
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ failures (shard 2/16 desktop scenarios 1/4/9/11, shard 13/16 mobile scenario 10)
skipped entirely when `invoice.documents` is empty), so the whole regen can complete fast
enough that the overlay's "visible" window is never reliably observed by Playwright's
polling. The suite's own established convention elsewhere
(`invoice-auto-itemize-page.spec.ts:1589`) only ever asserts the overlay's terminal *hidden*
(`invoice-auto-itemize-page.spec.ts:1589`) only ever asserts the overlay's terminal _hidden_
state, never its transient appearance — same lesson applies here. Fixed by adding
`ReportWizardPage.getPreviewSrc()` / `waitForPreviewRegenerated(previousSrc)` to the POM,
which prove a regeneration happened via the iframe's `blob:` src actually changing (every
`URL.createObjectURL()` call yields a unique URL) instead of racing the spinner.
- **Scenario 4 (empty state) — TEST_BUG, fixed.** `ReportInvoiceList.tsx`'s `<EmptyState>` only
renders when BOTH this source's allocated invoices AND the *household-wide* unallocated list
renders when BOTH this source's allocated invoices AND the _household-wide_ unallocated list
are empty — `sourceReportService.ts`'s `unallocRows` query has **no** `budget_source_id`
filter at all (confirmed by reading the SQL), so it's global across every vendor/spec file.
Under full 8-worker parallel CI this is essentially always non-zero, making the EmptyState
Expand Down
1 change: 1 addition & 0 deletions .claude/agent-memory/product-architect/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- [Recurring patterns & traps](recurring-patterns.md) — polymorphic FK cleanup, XOR CHECK vs SET NULL, forked-function drift, test smells, accepted N+1 sites
- [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
- [Story reviews](story-reviews.md) — per-story and per-PR review log
- [Client PDF pipeline](client-pdf-pipeline.md) — ADR-034 report PDF generation
- [Diary drafts pattern](diary-drafts-pattern.md) — ADR-022 draft lifecycle via status column on parent table
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
name: source-report-split-inference
description: Why the report PDF's †/‡ footnote classification is a proxy, not exact — budgetLines[] and deposits[] are both this-source-scoped, so one cross-source case is undetectable client-side (found reviewing PR #1902 / story #1898)
metadata:
type: project
---

# Source-report split inference is a proxy, not a fact

`client/src/lib/reportPdf/overviewPdf.ts` classifies footnotes as
`† iff isSplit && budgetLines.length > 0`, `‡ iff isSplit && deposits.length > 0`
(wording `constituted` iff a deposit carries `budgetSourceId === report.source.id`, else `reduced`).

**Both arrays are this-source-scoped server-side** — this is the load-bearing fact:

- `budgetLines[]` comes from Rail A rows, whose SQL `WHERE` is `wib2/hib2.budget_source_id = ${sourceId}`
(`sourceReportService.ts` step a → step i). **Only this source's lines**, never other sources'.
- `deposits[]` is filtered `budgetSourceId === null || === sourceId` (step j).

## The blind spot

Invoice with all lines on source A + a deposit tagged to source B, no untagged deposits:
`isSplit` true, `budgetLines.length > 0`, `deposits.length === 0` → renders `†` (generic split).
AC #1898-4 says it should render `‡ reduced` — and that wording is *correct*, because the
invoice-level residual rule means a B-tagged deposit does reduce A's Rail A share
(see [[dual-rail-aggregation]]).

**It is indistinguishable from a genuine A/B line split in the response.** Not fixable client-side.
`overviewPdf.test.ts` [Scenario 15] pins the ambiguous shape as "pure line-split".

The mirror view is correct: from B's report the same invoice has `budgetLines: []` +
the tagged deposit → `‡ constituted`, no `†`.

## Proposed fix (not yet filed as of 2026-07-31)

Add to `SourceReportInvoice`, derived in the existing `isSplit` UNION query whose two arms already
separate line-sourced from deposit-sourced `source_id`s — no extra query, no other-source data leak:

```ts
/** How the cross-source split arises. Only meaningful when isSplit. */
splitKind: 'lines' | 'deposits' | 'both';
```

Client becomes `† iff splitKind !== 'deposits'`, `‡ iff splitKind !== 'lines'` — no `.length` proxies.

## Wiki deviation to fix (open)

`wiki/API-Contract.md` L~3610 and `shared/src/types/sourceReport.ts` both describe `budgetLines[]`
as *"all ibl lines per invoice (even portion 0)"*. Wrong — it is all of **this source's** ibl lines.
Pre-existing since #1878/#1891. Needs the correction + a Deviation Log row on API-Contract.md.

## pdfmake width gotcha (confirmed by QA on #1898)

`@types/pdfmake@0.3.x`'s `Size` type is `number | 'auto' | '*' | string` — the trailing `| string`
lets `'2*'` typecheck, but pdfmake 0.3.11 has **no weighted-star feature**; it crashes pdfkit's
`Renderer.renderVector` with `unsupported number: NaN` at `getBlob()`. Only valid widths: number,
percentage string, `'auto'`, plain `'*'`. Content-tree unit tests cannot catch this — only
`realRender.test.ts`'s real `getBlob()` can. The ux-designer spec for #1898 recommended `'2*'`;
implementation correctly deviated to `'*'`.
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). 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 → #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`.

## Requirements Coverage

Expand Down
13 changes: 13 additions & 0 deletions .claude/agent-memory/product-owner/bank-report-wizard.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ metadata:
- **#1888 — stage-matched attachment indicator** (Backlog, deferred from #1879).
- **#1895 / #1896 / #1897 — claim close-out defects** from the product-architect audit of #1891's dual-rail deposit aggregation (PR #1894). All Backlog, created 2026-07-30. See §"Claim close-out defects" below.

## Refinement Round 2 (stories #1898–#1901, created 2026-07-31)

Source: user verification of the merged wizard; decomposed and approved via `/mini-epic` planning. No parent epic issue — standalone stories chained by blocked-by. All four board **Todo**. Chain: #1898 → #1899 → #1900 → #1901 (#1900 blocked-by both 1898+1899; #1901 blocked-by 1899+1900).

- **#1898 — Report table refinements.** Usage column (distinct `budgetLines[].linkedItem` names → fallback line `description` → `—`), attachments note from `documents[].attachmentType` (count + distinct types; untyped counted plainly), appendix-number column **deleted** (numbers had no in-document reference; append order unchanged), status column **dropped for `claim` + `proof-of-funds`, kept for `budget-overview`**, and a **deposit-specific footnote** distinct from the generic `†` split footnote.
- **#1899 — Settings step + report language.** Wizard goes to 5 steps: Report Type → Budget Source → Select Invoices → **Settings** → Preview & Export. New step 4 holds the en/de report-language picker (defaults to UI locale) plus the two relocated toggles. Report output (PDF text, dates, currency, cover letter, filename) fully renders in the selected language regardless of UI locale; wizard chrome stays in the UI locale.
- **#1900 — Editable HTML preview.** Step 5 becomes an HTML edit surface with **on-demand PDF** (no live PDF pane). Full cover letter — sender, recipient, reference, subject, body — all editable. Wording editable, **amounts/totals read-only** (derived). Edits are overrides on a generated baseline; a steps-1–4 change regenerates the baseline and clears edits behind a confirmation.
- **#1901 — AI generation** (Should Have). Explicit "Generate with AI" button gated by an "Enable AI assistance" toggle in step 4, only when `LLM_*` is configured. **One batched** server call → per-invoice usage descriptions + cover letter subject/body, in the selected report language. Fills the editable fields as a new baseline; regenerating warns before overwriting. Auto-itemize is the reference for plumbing + spinner/elapsed-seconds UX. Auth-required endpoint, **no persistence**.

**User decisions baked in — do not re-litigate:** status-column policy per report type; "letterhead" = the full cover letter with every block editable; AI is button-triggered, batched, and language-scoped; step 5 is HTML-edit + on-demand PDF with derived amounts locked.

**Deposit footnote rule (the subtle one):** an invoice whose shown amount is reduced or constituted by deposits must NOT get the generic "Amount shown reflects only the portion allocated to this source." An invoice whose only cross-source funding is a tagged/claimed deposit → deposit footnote only. A budget-line split across sources → keeps `†`. Both → both facts conveyed. Marker glyph/numbering is the dev-team-lead's call, but must not collide with the `†N` (split) / `*N` (skipped doc) namespaces.

## Claim close-out defects found auditing #1891 (2026-07-30)

The architect audit of the dual-rail aggregation confirmed **budget totals conserve correctly** — every defect found is in the **claim close-out** path, not the money math. Keep that distinction when triaging: conservation and close-out are separate correctness domains here, and a green conservation check says nothing about close-out.
Expand Down
Loading
Loading