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 @@ -21,9 +21,21 @@ class*="inlineNote">` in the **Allocated Amount cell**: `(partial)` / `(less dep
`depositReducedInvoiceIds.size > 0` → one `'depositReduced'` footnote. `footnotesBlock` /
`footnoteItems` are NO LONGER negative-only guards. Scenarios with split or deposit-reduced rows
must assert a **positive** count; constituted-deposit-only rows (Scenario 17) still assert
`toHaveCount(0)` because neither set is non-empty for them. Scenario 18 (two split invoices)
asserts `footnotesBlock` count=1, `footnoteItems` count=1, and the legend sentence IS present in
`main`'s text content.
`toHaveCount(0)` because neither set is non-empty for them.

**AC5 Issue #1980 update (fix/2003-1980-realrender-overflow-legend-assertions, commit 4ffc1425):**
Scenario 18 extended with a third invoice (`${testPrefix}-SPLITDR-003`) that is BOTH split AND
deposit-reduced (split via `seedSplitInvoice` across the same two sources + untagged deposit via
`createDepositViaApi` with `budgetSourceId: null`). Assertions updated:
- `footnoteItems` count: **2** (was 1)
- `footnoteItems.nth(0)` contains the split sentence
- `footnoteItems.nth(1)` contains "This position reflects deposits claimed separately."
- `invoice3`'s row asserts `inlineNote` count=2 (both `(partial)` and `(less deposit)`)
- `toContainText('(less deposit)')` on the page-wide text confirms both labels appear

**Deposit-reduced trigger**: `budgetSourceId: null` on a deposit = untagged = never matches
any source ID → `isDepositReduced: true` from `buildReportContent.ts` when the deposit is on an
invoice that also has budget-line allocation to the reported source (`isSplit: true` too).

2. `.usageAreaText` sub-line + the separate editable `Attachments Note` column → ONE read-only
`.usageMetaText` line inside the Usage cell: `[areaText, attachmentsNote].join(' · ')`
Expand Down
9 changes: 7 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, the revert test for fixes that only relax an invariant — re-run it yourself on round 2 (#1968/PR #2002), cross-reference rot in documented-bound comments (#1939), usePreferences per-instance store + serialized-write-queue review (#1955), capability-retained-but-producer-removed (#1959), reinstated-producer-vs-negative-guards (#1965), AC reversal by a polish issue (#1959), amount-threshold booleans narrowing status-existence booleans (#1897), prettier is not CI-gated, single-occurrence delimiter guard tests + German ordinals vs list markers + pre-validating regex fix specs (#1952), `Pick<>` is not a forcing function + caller-supplied monotonic seq reintroduces the ref + cascade tables smuggle behaviour changes + neutralised-trigger-left-in-code (#1947), tier factory only forces the cases that spread it (#1988), regex mirroring a third-party grammar + `parseInt` trailing garbage + env vars documented in four places (#1970, PR #1989), guard-deleted-because-it-looked-like-the-bug + rate-limit identity-check gate + `request.ip` nullability types-lie + CVE test needs a negative control (#1995, PR #1998), prettier config resolution is path-based so /tmp baseline checks lie + wiki is not prettier-ignored + document the invariant not the absence-of-code (#1998 wiki pass), comment-refreshed-but-assertion-left-behind + contract inversion makes pre-existing negatives unconditional + surgical tagging misses read-only value nodes (#1910, PR #2004 r2), the-prop-landed-is-not-the-prop-is-wired + redundant-tag-a-test-asserts + `aria-label` cannot be language-tagged (#1910, PR #2004 r3), `count >= 1` + all-match is a per-instance assertion masquerading as coverage — revert each call site individually, use `toBe(N)` (#1910, PR #2004 r4), untyped E2E route fixtures drift from shared contracts + consumer early-return masks an incomplete fixture + duplicate Playwright route globs are an ordering dependency (#2005, PR #2006), widen-then-`as`-narrow defeats union exhaustiveness + a hardening PR falsifies its own ADR in four predictable places + key-echo fixtures are non-discriminating (#2001, PR #2007)
- [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, the revert test for fixes that only relax an invariant — re-run it yourself on round 2 (#1968/PR #2002), cross-reference rot in documented-bound comments (#1939), usePreferences per-instance store + serialized-write-queue review (#1955), capability-retained-but-producer-removed (#1959), reinstated-producer-vs-negative-guards (#1965), AC reversal by a polish issue (#1959), amount-threshold booleans narrowing status-existence booleans (#1897), prettier is not CI-gated, single-occurrence delimiter guard tests + German ordinals vs list markers + pre-validating regex fix specs (#1952), `Pick<>` is not a forcing function + caller-supplied monotonic seq reintroduces the ref + cascade tables smuggle behaviour changes + neutralised-trigger-left-in-code (#1947), tier factory only forces the cases that spread it (#1988), regex mirroring a third-party grammar + `parseInt` trailing garbage + env vars documented in four places (#1970, PR #1989), guard-deleted-because-it-looked-like-the-bug + rate-limit identity-check gate + `request.ip` nullability types-lie + CVE test needs a negative control (#1995, PR #1998), prettier config resolution is path-based so /tmp baseline checks lie + wiki is not prettier-ignored + document the invariant not the absence-of-code (#1998 wiki pass), comment-refreshed-but-assertion-left-behind + contract inversion makes pre-existing negatives unconditional + surgical tagging misses read-only value nodes (#1910, PR #2004 r2), the-prop-landed-is-not-the-prop-is-wired + redundant-tag-a-test-asserts + `aria-label` cannot be language-tagged (#1910, PR #2004 r3), `count >= 1` + all-match is a per-instance assertion masquerading as coverage — revert each call site individually, use `toBe(N)` (#1910, PR #2004 r4), untyped E2E route fixtures drift from shared contracts + consumer early-return masks an incomplete fixture + duplicate Playwright route globs are an ordering dependency (#2005, PR #2006), widen-then-`as`-narrow defeats union exhaustiveness + a hardening PR falsifies its own ADR in four predictable places + key-echo fixtures are non-discriminating (#2001, PR #2007), a revert test can prove a _different_ proposition than the one it licenses + implementing a documented rule for the first time is when you learn the rule is wrong + three forked `collectAllStrings` copies (#2003, PR #2008)
- [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 documented but unenforced (`horizontalRatio`: 0 hits in `client/`) → issue #2003, mine**
- [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.**
- [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 Expand Up @@ -39,6 +39,11 @@
`git -C wiki log` (shows unpushed commits as HEAD) or a refspec-less `fetch` (leaves origin/master stale)
- Pages: Architecture, Schema, API-Contract, Home, ADR-Index, ADR-NNN-*, Style-Guide (ux-designer), Security-Audit (security-engineer)
- **Always push wiki before creating the PR** — the submodule ref must be committed on the feature branch. If you push wiki content outside the branch, flag that the PR's ref needs bumping.
- **No workflow in `.github/workflows/` checks out submodules** — an unpushed wiki commit recorded as the
parent's submodule ref keeps CI fully green and merges silently, then breaks `git submodule update` on
`beta`. Recurred on PR #2008 (recorded `da1324b`, remote at `b12ebb1`; I pushed it during re-review).
**Make the ls-remote-vs-ls-tree check a standing step of every PR review that touches `wiki`**, not just
of your own wiki pushes — a later commit in the same branch can advance local HEAD past what was pushed.

### Wiki Update Discipline (CRITICAL)

Expand Down
93 changes: 93 additions & 0 deletions .claude/agent-memory/product-architect/client-pdf-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,96 @@ The grey `areaText`/`attachmentsNote` suffix inherits `tableCell`'s `TABLE_BODY_
deposit/split labels in the AllocatedAmount cell do (`overviewPdf.ts:720-740`). That is why
`usageSafeTokenChars` (from `BODY_WORST_CASE_CHAR_WIDTH_PT`) is the correct break threshold for it.
Adding a `fontSize` to those runs later would decouple threshold from rendered size.

## ADR-034 rule #1 is WRONG A SECOND TIME: `horizontalRatio` is a cell-origin bound, not an overflow check

Measured against `pdfmake@0.3.11` during the PR #2008 review (#2003). `ElementWriter.addLine`
(`src/ElementWriter.js:32`) captures `position = this.getCurrentPositionOnPage()` **before** placing the
line, so `positions[].horizontalRatio` records the **left origin of each text line**, never its right
extent. Consequences:

| fixture (A4, 40pt margins, five 80pt fixed cols) | `max(horizontalRatio)` |
| ------------------------------------------------ | ---------------------- |
| 400-char unbreakable `W` token in an 80pt column | 0.7006 |
| **4000**-char unbreakable `W` token, same column | 0.7006 (identical) |
| 4000-char token in one 500pt column | 0.0097 |
| `widths: [600, 50]` | 1.1916 |
| widths summing 900pt | 1.2091 |

The overview table is all-fixed-width summing to exactly `printableWidth()` (asserted at
`realRender.test.ts:1751`/`:1890`), so **no cell origin is ever past the right margin** ⇒
`max(horizontalRatio) <= 1` is _unconditionally true_ on every production fixture. It cannot detect token
overflow within a column — the failure mode `safeTokenChars`/`wordBreak: 'break-all'` exist to prevent.
It only catches table-box overflow, which the `tableOffsetsTotal + sum(_calcWidth) === printableWidth()`
assertion already forbids more strictly. Note even B2 as literally described (last column starts at
~511pt of 515.28) would have scored < 1.

**The assertion that DOES work: per-cell `cell._minWidth <= table.widths[c]._calcWidth`.**
`_minWidth` is computed _after_ `TextBreaker` applies `wordBreak`, so `break-all` collapses it to one
glyph. Measured on the real inline-run cell shape, Usage column `_calcWidth` = 69.28pt:

| cell | `_minWidth` |
| ------------------------------------------------- | ----------------- |
| prose + grey 30-`W` areaName **with** `break-all` | 33.54 → PASS |
| same **without** `break-all` (the regression) | 266.16 → **FAIL** |
| all plain prose | 33.54 → PASS |

Same cell with `'W'.repeat(400)`: 3548.83 without `break-all`, **8.87** with. So ADR-034's
"Do not assert `_minWidth`" paragraph is right about the **table-level** form but its `wordBreak`
false-positive rationale is **empirically false**, and it wrongly generalises to the **cell-level**
form, which is the discriminating revert-provable check. Needs no new renders — existing renders already
mutate both fields.

**PAID 2026-08-05** (wiki `b12ebb1`, parent ref `2a1862c7`, on the PR #2008 branch). Rule #1 now leads with
the per-cell `_minWidth <= widths[i]._calcWidth` content-extent check; `maxHorizontalRatio` was re-scoped
(not deleted) to "no element is positioned past the right margin" with an explicit vacuity warning against
asserting it on production content; the `_minWidth` ban was split into table-level (still banned, the
`starMaxMin` reason) vs per-cell (the correct check); finding B2's cross-reference now says its quoted
`_minWidth` figures are table-level sums. Third Deviation Log row added, carrying the process lesson:
**a revert test proves the helper can fire on _some_ input, not that it can fire on the input the rule is
about — the negative control must mutate the production code path the rule governs.** Note the ADR row I
wrote on 2026-08-04 is the one that introduced the wrong guidance, and #2003 implemented it faithfully:
when I replace a rule, I owe the replacement its own falsifiability check, not just a source-line citation.

### Probing pdfmake from bare Node (no jsdom)

Memory previously said `getBuffer()` callbacks don't fire in bare Node CJS. Workaround that does work
for layout-geometry probes (`createPdf().getBlob()` not needed):

```js
const PdfPrinter = require('<repo>/node_modules/pdfmake/js/printer.js').default;
const printer = new PdfPrinter(
fonts,
{ existsSync: () => false },
{ resolve() {}, resolved: async () => {} },
() => true,
);
const doc = await printer.createPdfKitDocument(def); // async in 0.3.x; mutates def.content in place
```

Fonts: real TTF paths under `node_modules/pdfmake/fonts/Roboto/`. The `virtualfs`/`urlResolver`/
`localAccessPolicy` ctor args are required (positional) or `resolveUrls` throws on `undefined.resolve`.
Line numbers differ between `src/` (528) and the `js/` build (490) — cite the file, not just the number.

## Measured overview-table width/`_minWidth` figures (PR #2008 re-review, 2026-08-05)

Measured directly by instrumenting `realRender.test.ts`'s `#2003` block. **Use these, not the numbers
in ADR-034's rule #1 prose — those are stale (see M1 in the PR #2008 round-2 review).**

| Quantity | Value | Source |
| ------------------------------------------------ | -------- | --------------------------------------- |
| Usage `_calcWidth`, 6-col (`claim`) | 186.78pt | `USAGE_WIDTH_6COL`, `overviewPdf.ts:58` |
| Usage `_calcWidth`, 7-col (`budget-overview`) | 138.28pt | `USAGE_WIDTH_7COL`, `overviewPdf.ts:57` |
| Cell `_minWidth`, 30x'W' **with** `break-all` | 7.098pt | single 'W' glyph @ 8pt `tableCell` |
| Cell `_minWidth`, 30x'W' **without** `break-all` | 212.93pt | 30 x 7.098pt |

Which use case maps to which shape: `claim` → 6 columns, `budget-overview` → 7 columns
(`reportContent.isOverview` selects the width, `overviewPdf.ts:536`).

**69.28pt is not the Usage column width and has not been for some time** — it is a pre-rebalance
figure that still appears at ADR-034 lines 105, 120, 129, 138 and in the Deviation Log. Treat any
`69.28` in this pipeline's docs as suspect.

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