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 @@ -29,6 +29,7 @@
- [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.

- [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.

## Open follow-ups to flag to orchestrator
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
name: issue-1959-inline-meta-and-labels
description: PR #1959 reversed two earlier report-table designs (†/‡ shared footnotes from #1923, distinct area sub-line) into inline labels + one combined meta line; which E2E locators/scenarios had to be rewritten and how each new assertion was made non-vacuous.
metadata:
type: project
---

PR #1959 ("improve report PDF UX") deliberately **superseded** two designs earlier rounds had
asked for, in `ReportContentEditor.tsx` / `buildReportContent.ts`:

1. `†`/`‡` markers + the shared footnote list (Story #1923 AC1) → grey inline `<span
class*="inlineNote">` in the **Allocated Amount cell**: `(partial)` / `(less deposit)`
(de `(Teilbetrag)` / `(abzgl. Abschlag)`). `ReportContentRow.allocatedMarkers` → `isSplit` /
`isDepositReduced` booleans. `buildReportContent` now pushes **zero** footnotes, so
`.footnotes` has no producer at all — `footnotesBlock`/`footnoteItems` survive in the POM as
**negative-only** guards.
2. `.usageAreaText` sub-line + the separate editable `Attachments Note` column → ONE read-only
`.usageMetaText` line inside the Usage cell: `[areaText, attachmentsNote].join(' · ')`
(U+00B7 middle dot, spaces on both sides). The `attachmentsNote` `EditableField` is gone
entirely, so a content-table row / mobile card now has **exactly one textbox** (Usage) — the
crispest available guard against that column coming back.

**Why:** the user owns #1959 and asked for it in the promotion; source-of-truth hierarchy makes
the PR body the spec, so the tests were rewritten, not the code.

**How to apply:** POM renames are `usageAreaText`→`usageMetaText`,
`mobileUsageAreaText`→`mobileUsageMetaText`, plus new `inlineNote()`/`mobileInlineNote()`;
`attachmentsNoteField()` deleted. Rewritten scenarios: editableContent 2, 17, 18, 20 and
aiGeneration 8. Every "old design is gone" negative is paired with a positive so it cannot pass
against a mis-seeded page (e.g. Scenario 18 asserts `(partial)` present *and* `†`/`‡` absent
*and* the long-form footnote sentence absent from `main`; Scenario 20 asserts the attachments
note text IS rendered *and* the row has one textbox).

Facts worth reusing:

- `toHaveText`/`toContainText` normalize whitespace, so the desktop `inlineNote` span's leading
space (` (partial)`) is absorbed — `toHaveText('(partial)')` is correct.
- Attachment note text for a `claim` report with `attachmentType: 'invoice'` is
`1 attachment: Invoice`. The tier gate is
`server/src/services/shared/attachmentTierUtils.ts`: floors are quotation(1) for
budget-overview, deposit(2) for claim, invoice(3) for proof-of-funds; `null` counts as tier 3.
So a 'invoice'-tagged link shows up in claim reports — don't guess, that file is the only
definition.
- `document_links` is unique on `(entity_type, entity_id, paperless_document_id)`, so a
hardcoded `paperlessDocumentId` in a spec cannot collide across parallel workers/projects.
- `AppShell.tsx` renders a real `<main>` element, so `page.locator('main')` is a safe
page-scope text container.
- **Column-visibility checkboxes (`role="group"`, "Show/hide columns") shipped with #1959 with
NO E2E coverage** — deliberately not added on the critical path, because a new test case
reshuffles shard membership. Pick this up when the promotion isn't blocking.
- Sub-agents share the worktree here: a `prettier --check` on a `client/` file can transiently
fail because another agent is mid-write. Re-check before reporting it as broken.

See [[known-flakes-and-regressions]], [[story-1900-editable-report-preview]],
[[story-1879-report-wizard]].
2 changes: 2 additions & 0 deletions .claude/agent-memory/frontend-developer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ removing it will fail typecheck. Keep the export and note it as QA-deferred.
- `budget-invoices.md` — open for budget/invoice/subsidy pages: BudgetSubNav, budget line APIs, invoice CRUD patterns, pipeline dashboard cards
- `gantt-calendar.md` — open for Timeline/Gantt/Calendar/Milestone work: SVG architecture, drag hook (React 19 refs rule), arrow hover, lane allocation, constraints section, related E2E POM layout
- `api-routing.md` — open when calling APIs or navigating: UserResponse/PaginatedResponse shapes, auth routes, dependency direction-swap semantics, navigation origin state
- `i18n-invisible-chars.md` — open before adding an NBSP/invisible char to a locale file: literal-not-escape convention, why grep misses it, how to verify by render, and the identical-looking test failures it causes
- `qa-tripwire-tests.md` — open when a task says a fix will flip an `it.failing` guard: why the flip signal can be swallowed by a shared helper, and what to do instead
- `testing-mocks.md` — open when your change will break existing tests: ESM mock patterns, mock-block update checklist, Toast/context mocks, DefinePlugin globals in Jest
- `git-tooling.md` — open on webpack config changes or git/CI trouble: worktree remotes, object corruption/permission recovery, npm audit policy, Prettier/CI format gap
- `photo-metadata-sidepanel.md` — photo metadata side panel notes (open for photo UI work)
Expand Down
34 changes: 34 additions & 0 deletions .claude/agent-memory/frontend-developer/i18n-invisible-chars.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
name: i18n-invisible-chars
description: How to add and verify invisible characters (NBSP) in locale files — repo writes literal non-ASCII, and the resulting test/diff failures look like identical strings
metadata:
type: project
---

Multi-word inline labels rendered into the PDF wrap at their internal space. A single-word label
can't wrap; a two-word one breaks across lines and splits its own brackets (`(less` / `deposit)`).
The fix is U+00A0 (NBSP) instead of the space — same glyph advance, so no geometry constant moves,
and pdfmake's UAX-14 line breaker treats it as non-breaking.

**Why the details matter:** the change is invisible in every textual channel, so it produces
failures and diffs that look like no-ops and burn reviewer time.

**How to apply:**

- **Convention:** `client/src/i18n/**` uses **literal** non-ASCII characters (`…`, `—`, `·`, `↑`) —
there are zero `\uXXXX` escapes. Write a literal NBSP, not an escape. Insert it with a script
(`python3` + `u' '`), never by typing it, and verify with
`python3 -c "...print([hex(ord(c)) for c in value])"` — a hand-typed one silently lands as a plain
space.
- **grep will not find it:** `grep -P '\xc2\xa0'` returns nothing in a UTF-8 locale (PCRE reads the
escapes as characters, not bytes). Use `LC_ALL=C grep -n $'\xc2\xa0' <file>`, or `git diff | cat -A`
(NBSP shows as `M-BM-`).
- **`pdftotext` normalizes it back to a space**, so extraction can't confirm the character. Verify
the **break position** instead: render the real PDF, extract with `pdftotext -layout`, and check
the label lands on one line. Render a plain-space control alongside it — otherwise you can't tell
a fix from data that happened not to wrap.
- **Warn about the test fallout explicitly.** Any test asserting the label literally now fails with
`Expected " (less deposit)" / Received " (less deposit)"` — visually identical. Always report the
codepoints and the exact replacement, or someone will read it as a flake.

Related: [[qa-tripwire-tests]].
35 changes: 35 additions & 0 deletions .claude/agent-memory/frontend-developer/qa-tripwire-tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
name: qa-tripwire-tests
description: QA encodes known production defects as `it.failing` tripwires; a shared helper that hard-codes the broken model can swallow the flip signal, so verify the fix independently
metadata:
type: project
---

QA (`qa-integration-tester`) encodes a known-but-unfixed production defect as `it.failing` with a
passing control test alongside. The suite stays green while the defect is open; when production is
fixed the test starts passing and Jest errors with "Failing test passed even though it was supposed
to fail" — that error is the intended confirmation signal, and the fixing agent is expected to flip
`it.failing` → `it` (the one sanctioned developer edit to a test file).

**Why:** it keeps the defect encoded in executable code rather than only in a report, so a fix can't
land without someone noticing the guard.

**How to apply:** the flip signal is NOT reliable on its own. If the tripwire calls a shared render
helper that asserts the _broken_ model (e.g. `expect(body).toHaveLength(1 + usageChunkCount + 1)`,
which bakes in "meta never adds rows"), a correct fix makes the helper throw first — the tripwire
then still "passes" as a failing test and no signal appears. Seen on #1959 (`realRender.test.ts`
cell-scope block): the fix was verified, the tripwire never flipped.

So: (1) always verify the fix independently with your own scratch render/measurement harness before
trusting or distrusting the tripwire; (2) if the flip leaves the test failing inside a shared helper
rather than at its own assertions, revert to `it.failing`, leave QA's file byte-identical, and report
the exact helper line QA must update — do not "fix" the helper to make the flip work;
(3) expect sibling tests in the same block to fail too, for the same reason (they assert the broken
model at content-tree level, which is why they passed while the rendered PDF lost content).

**Outcome on #1959 (closing the loop):** reporting the exact blocking helper line was the right call
— QA re-derived the row count from the production packer itself, then converted the tripwire to a
normal guard. Reporting "cannot flip, here is the line" got it resolved in one round; editing the
helper myself would have put a developer edit in a QA-owned file for no gain.

Related: [[../MEMORY.md]] "Refinement Workflow — QA Test Coordination", [[i18n-invisible-chars]].
4 changes: 2 additions & 2 deletions .claude/agent-memory/product-architect/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

## Topic Files

- [Recurring patterns & traps](recurring-patterns.md) — polymorphic FK cleanup, XOR CHECK vs SET NULL, forked-function drift, test smells, cross-layer contract drift, ajv `anyOf`, N+1 sites, async writes surviving state resets, cross-reference rot in documented-bound comments (#1939), usePreferences per-instance store + serialized-write-queue review (#1955)
- [Recurring patterns & traps](recurring-patterns.md) — polymorphic FK cleanup, XOR CHECK vs SET NULL, forked-function drift, test smells, cross-layer contract drift, ajv `anyOf`, N+1 sites, async writes surviving state resets, cross-reference rot in documented-bound comments (#1939), usePreferences per-instance store + serialized-write-queue review (#1955), capability-retained-but-producer-removed (#1959), AC reversal by a polish issue (#1959)
- [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 + the reportContent content/layout split (#1900)
- [Client PDF pipeline](client-pdf-pipeline.md) — ADR-034 report PDF generation, reportContent content/layout split (#1900), `dontBreakRows` silent-drop rule + **owed ADR-034 corrections** (#1959)
- [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)
Expand Down
46 changes: 46 additions & 0 deletions .claude/agent-memory/product-architect/client-pdf-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,49 @@ baseline rather than blanking a field. Keep this shape — putting AI output in
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".

### PR #1959 review — the Usage-cell height bound, third scope revision

**The recurring defect** (#1929 r3 -> r4 -> #1959 r1) is always the same: `table.dontBreakRows: true` makes
pdfmake measure an over-tall row and then `PageElementWriter` **silently discards** the overflow. No throw,
no visible truncation. Symptom: page count **saturates** (~2) and can go **non-monotonic** (3 -> 2 as content
grows) while rendered line count rises linearly.

**The rule that fixes it durably: bound the height of what a cell RENDERS, never of a source field.**
Any per-field bound is coupled to a layout decision and detaches silently the moment the layout changes —
which is exactly how r4's per-field continuation rows became #1959's unbounded inline run.
`packUsageCellRows(segments, MAX_SAFE_USAGE_CHUNK_CHARS)` is the right shape because the bound now lives
where the cell's content **stream** is assembled, so a new segment is bounded automatically.

**Two detection recipes that actually caught it** (both threshold-free, both worth reusing for any generated
document): page count must be **monotonic** in content size, and **channel-independent** — the same text
costs the same paper whichever field carries it.

**The generic-assertion / hand-enumerated-input asymmetry** (the reason it recurred, now the top open item):
the per-row budget assertion counts characters generically over all runs, so a new channel *would* be
counted — but the test inputs are hand-listed (`usageText`/`areaText`/`attachmentsNote`), so a new
`ReportContentRow` string field defaults empty and every assertion passes **vacuously**. Fix is key-driven
saturation (`Object.entries(row)` -> saturate every string field), not another hand-written case.
Verified empirically during review: losslessness + per-row budget hold over 3k fuzzed inputs (0 failures);
a hypothetical **second** grey/meta segment trips the existing `splitUsageCell` "at most one grey run per row"
throw in ~49% of inputs, so *that* channel class is already guarded.

**Geometry constraint (blocks a feature):** `USAGE_WIDTH_7COL/_6COL` derive from `usableColumnWidth(n)`, and
`MAX_SAFE_USAGE_CHUNK_CHARS = 650` was **measured against the 7-column shape**. So making column visibility
affect the PDF widens the Usage column and **invalidates the measured ceiling** — "just plumb the toggles
through" is a re-measurement story, not a UI change. #1959's toggles are preview-only local `useState`.

**`packUsageCellRows` hangs on `maxChars <= 0`** (own `remaining <= 0 -> flush() no-op -> continue` loop).
New to the packer — `splitIntoPageSafeChunks` fails loudly instead (`RangeError`). Unreachable while the
budget is a constant; matters if it ever becomes computed.

### ADR-034 debt (owed, NOT yet written — carry this forward)

1. Add the `dontBreakRows` lesson + the "bound the rendered cell, not a field" rule + both detection recipes.
2. **Minimum-bar rule #1 is wrong**: `table._minWidth <= 515.28` fails on correct code (`_minWidth` is the
widest unbreakable *word*, not the laid-out width). Correct check: `max(horizontalRatio) <= 1`.
B2's narrative is fine; the generalized rule was mis-transcribed.
3. Module table drifted twice: add `pageGeometry.ts` (#1939) and `index.ts`; drop "PDF-local formatters" from
`shared.ts` (deleted in review round 2) and move "table layout constants" to `pageGeometry.ts`.
4. Override-key list (line 148): drop `attachmentsNote` — unreachable since #1959.
5. Record the fixed 6-or-7 column-count constraint above.
29 changes: 29 additions & 0 deletions .claude/agent-memory/product-architect/recurring-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,3 +389,32 @@ in flight. That is the whole proof. Two corollaries worth asserting in review:
Ordering claims: a client queue genuinely fixes server-side out-of-order application only if the next
write is not _dispatched_ until the previous response is back (i.e. `await` wraps the HTTP call, not just
the local state update). Check that, or the fix only narrows the window.

## Capability retained in code, disabled at the producer, documented by comment (PR #1959)

Three instances in one PR, all the same shape — **a type permits a state the producer can never emit, and the
constraint is enforced by a comment instead of by the type**:

- `ReportContent.footnotes` is now unconditionally `[]`, yet 4 consumers still branch on / copy it. The E2E
page object had to add a prose warning ("never assert a positive count on these").
- `applyOverrides` still honours `row.<id>.attachmentsNote` with **no producer left** (editor field removed;
overrides are ephemeral wizard state — no server persistence, nothing in migrations 0001-0044; and
`applyAiContent` only writes `usageText` + cover-letter fields).
- Column-visibility toggles labelled `Show/hide columns` that affect the preview but **not** the exported PDF.

Rule: **delete rather than comment.** A write path with no reader and no producer is how the #1929
round-3/round-4 confusion started — the code said one thing and the comment said another. When triaging a
"keep it as a capability?" question, check for a *producer* first: no producer => dead, remove it.

## Verify the AC record when a PR reverses a recently-shipped story (PR #1959)

#1959 reversed #1923's AC1.1/1.2/2.3/2.4 **one day** after #1923 shipped (2.13.0-beta.38, 2026-08-02), while
#1923 stayed CLOSED with those ACs marked delivered — so the record asserted two contradictory behaviours were
both correct on `beta`, and the pending promotion PR would have summarized reversed ACs as delivered.

Tell: the reversing issue had **zero comments**, no `**[product-owner]**` header, no numbered ACs (its body was
a PR-style summary), and no ux-designer visual spec — whereas the story it reversed had all four. **A
requirements reversal authored as a polish issue is the signature.** Cheap fix: PO supersession comment on the
old issue + PO ratification on the new one. Check this whenever a PR deletes user-visible report/document
content — and check whether the replacement text preserves *meaning* (`(abzgl. Abschlag)` lost the footnote's
"claimed separately", which is compliance-relevant in a bank-facing document).
Loading