feat(reports): usage column, attachment notes, deposit footnotes & column cleanup

[steilerDev]

Summary

• Add a Usage column to report line items, deriving usage from linked work items/household items
• Add an attachment count/type note per line item
• Replace the generic split footnote with a dedicated deposit footnote family (‡) for deposit-driven amounts
• Drop the status column on claim/proof-of-funds report sections and remove the appendix column

Fixes #1898

Test plan

• Unit tests pass (95%+ coverage)
• Integration tests pass
• CI Quality Gates pass (typecheck, tests, build, audit)

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com
Co-Authored-By: Claude frontend-developer (Haiku 4.5) noreply@anthropic.com
Co-Authored-By: Claude translator (Sonnet 4.5) noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) noreply@anthropic.com

[steilerDev]

[ux-designer]

Reviewed client/src/lib/reportPdf/overviewPdf.ts against my visual spec on #1898 (issuecomment-5140357473). This is a pdfmake content-tree story — no CSS/tokens/dark-mode surface applies (confirmed N/A per §5 of my own spec).

Spec compliance — all confirmed in the diff and cross-checked against the added tests:

• Usage column appended last, order vendor, invoiceNumber, date, [status], invoiceAmount, allocatedAmount, usage — correct in both columns.push(...) and the row-building code.
• Attachment note rendered as a stack second line, style: 'small' (9pt #6b7280), margin: [0, 2, 0, 0] — matches §2 verbatim, no invented style, no italics.
• Em-dash placeholder (—) renders at tableCell style (peer weight), not the muted small style — correct per §1.
• Comma-separated inline lists via uniqueInOrder(...).join(', ') for both the usage list and the attachment-type list — matches.
• ‡ (deposit) marker family has its own independent counter (depositFootnoteNum), separate from */† — matches §4.
• In-cell marker order *N → †N → ‡N — confirmed by test [Scenario 19] asserting '€400.00*1†1‡1'.
• Footnote block order skip → split → deposit, with margin: [0, 4, 0, 0] on the first entry of the split and deposit blocks only (not within-family, not on skip) — confirmed by test [Scenario 20], which explicitly asserts margin is set only on splitEntries[0] / depositEntries[0] and undefined elsewhere.
• Subtotal/Total row label placement: buildSummaryRow's leadingCount = isOverview ? 4 : 3 computes the label position as "columns preceding Invoice Amount," which lands the label immediately left of Invoice Amount in both the 7-col (status slot) and 6-col (date slot) layouts, and the trailing Usage cell is always empty — satisfies §1/§3's index-agnostic requirement. (Technically a one-line ternary rather than a fully computed index, but it's derived from real column structure, not a magic offset tied to appendix presence — I consider this compliant with the intent.)

Deliberate deviation — Usage column width, 2* → *:

My spec called for 2* (Usage gets 2x Vendor's share of the flexible space). QA's finding that pdfmake 0.3.11 doesn't support weighted-star syntax is correct and well-documented in the PR (realRender.test.ts cites @types/pdfmake's Size type — number | 'auto' | '*' | percentage-string — and notes the real renderer crashed with unsupported number: NaN before the fix). This isn't a shortcut; it's a genuine library constraint discovered via the real (unmocked) pdfMake.createPdf().getBlob() path, which the earlier mocked tests couldn't have caught.

Given that constraint, I accept the equal 50/50 split ('*', 'auto', 'auto', 'auto', 'auto', 'auto', '*') as the right fallback, not a percentage-based alternative:

• A '30%'/'45%'-style split requires reserving budget against the five/four auto (content-fit) columns whose rendered width isn't knowable at spec time — exactly the page-size-assumption risk flagged in the task brief. That risk is real: German status/date labels are wider than English ones (this file has prior width regressions from that exact cause — see the "German overview table column widths" test block), so a fixed percentage could overflow in one locale and look sparse in the other.
• Equal-star split has no such risk: it's still proportional to whatever space is actually left after the auto columns resolve, in both locales, and it strictly improves on the pre-story state (Vendor no longer monopolizes 100% of the flex space now that Usage exists).
• Both widths are verified against the real renderer for both column-count variants (realRender.test.ts, "German overview table column widths" describe block) — not just asserted, but proven to not crash.

This is a reasonable, well-substantiated engineering trade-off. I'm not requesting a percentage-based fix. If a future pdfmake upgrade adds proper weighted-star support, revisiting the 1:2 ratio would be a nice-to-have, not a requirement — worth a one-line note in the PR or a follow-up issue so it isn't forgotten, but not blocking here.

Verdict: APPROVED. No token/dark-mode/accessibility surface to check (correctly out of scope per my own spec), and every in-scope layout/typography/footnote convention from the spec is implemented and test-verified. The width deviation is justified, documented, and safer than the original spec's literal instruction given the newly discovered library constraint.

[steilerDev]

[product-architect]

Verdict: APPROVE — medium findings noted for refinement, no blockers in the diff

Reviewed against issue #1898 ACs, the ux-designer visual spec (issue comment), wiki/API-Contract.md §Source Reports (L3607–3630), and server/src/services/sourceReportService.ts (the actual dual-rail derivation behind isSplit / budgetLines[] / deposits[]).

What I verified

• Signature stability — buildOverviewContent's 8-arg signature is unchanged; merge.ts:117-120 still passes appendixByInvoiceId at position 3 and still consumes it at merge.ts:188/205 for PDF page-embed ordering. AC 3 ("appendix append order unchanged") holds.
• Column construction — 7 cols for budget-overview, 6 for claim/proof-of-funds; appendix column removed unconditionally (header, cell, width slot, and the appendix i18n key in both locales). Matches the ux-designer count correction.
• Summary rows — buildSummaryRow places the bold label immediately left of Invoice Amount in both layouts (index 3 / index 2) and emits an empty trailing Usage cell. This retires the hardcoded per-index cell pushes the ux-designer flagged as a structural risk.
• Width deviation from the visual spec is justified. The spec called for weighted '2*' on Usage; the implementation ships plain '*'. @types/pdfmake@0.3.x's Size type widens to | string, so '2*' typechecks but crashes pdfkit's Renderer.renderVector with unsupported number: NaN at getBlob(). Caught by realRender.test.ts, not by the content-tree unit tests — exactly the class of bug that suite exists for. Deviation is documented in the QA memory file and the realRender.test.ts header. Consequence: Vendor and Usage now share leftover width 1:1 instead of 1:2 — cosmetic, ux-designer's call whether to pursue a percentage-string alternative.
• Test quality — strong. realRender.test.ts exercises real i18next bundles + real formatters, asserts literal German strings ('2 Anhänge: Angebot, Rechnung', 'Dies ist eine Abschlagszahlung.'), runs the raw-key-leak walk over the whole content tree via collectAllStrings, and drives a real generateReportPdf() → getBlob() on a fixture that carries ‡ markers in both locales. No assertions were weakened; the makeMixedReport fixture was correctly repaired (budgetLines added to splitNoDoc/splitWithDoc) rather than the rule bent to fit stale fixtures.
• Coverage exception accepted — 100% stmts/funcs/lines, 94.82% branch. The three uncovered branches are unreachable by construction (the skipFootnotesByInvoiceId.has() guard over a Map's unique keys; the two .find(...) ?? '—' fallbacks in the split/deposit blocks, whose maps are keyed from report.invoices itself). Reasoning is sound — don't chase these with contrived fixtures. See L4 for a way to make one of them structurally go away.
• No wiki update required by this PR — client-only, no endpoint/schema/response-shape change. See M2 for a pre-existing wiki inaccuracy this PR surfaced.

---

Medium findings (address in refinement — not blocking merge)

M1 — The † vs ‡ inference is unsound for one reachable case, and the contract can't currently fix it.

The classification is † iff isSplit && budgetLines.length > 0, ‡ iff isSplit && deposits.length > 0. Both budgetLines[] and deposits[] are this-source-scoped server-side:

• budgetLines[] is built from railALineContributions, i.e. Rail A rows, whose WHERE clause is wib2.budget_source_id = ${sourceId} / hib2.budget_source_id = ${sourceId} (sourceReportService.ts step a → step i). It contains only this source's lines.
• deposits[] is filtered budgetSourceId === null || === sourceId (step j).

So consider: an invoice whose budget lines all sit on source A (the reported source), plus a deposit tagged to source B, and no untagged deposits. isSplit is true (lines→A UNION deposit→B = 2 sources). From A's report the client sees budgetLines.length > 0, deposits.length === 0 → renders † (generic split), no ‡.

AC 4 bullet 1 names exactly this case — "an invoice whose only cross-source funding is a tagged deposit (its budget lines all belong to the reported source...)" → should get the deposit footnote and not the generic split footnote. And it is precisely the depositReducedFootnote wording: per the invoice-level residual rule, a B-tagged deposit does reduce A's Rail A share, so "This position reflects deposits claimed separately" is the accurate sentence. Note the mirror view is handled correctly — from B's report the same invoice yields budgetLines: [] + the tagged deposit → ‡ constituted, no †.

This is not fixable client-side: the discriminating deposit is filtered out server-side, so scenario "lines on A + deposit tagged to B" and scenario "lines genuinely split across A and B" are byte-identical in the response. overviewPdf.test.ts [Scenario 15] pins the ambiguous shape as "pure line-split", which is the reasonable default but encodes the ambiguity as settled.

Impact is bounded: the emitted footnote is less specific, not false, and it is the same text shipped today — no regression, just an incompletely-delivered improvement in one sub-case. Hence medium, not blocking.

Ask: file a follow-up for a server-side discriminator so the client stops inferring. Cheapest shape that closes it without leaking other sources' data:

/** How this invoice's cross-source split arises, from the server's own UNION. Only meaningful when isSplit. */
splitKind: 'lines' | 'deposits' | 'both';

derived directly in the existing isSplit UNION query (the two arms already distinguish line-sourced from deposit-sourced source_ids), then the client becomes † iff splitKind !== 'deposits', ‡ iff splitKind !== 'lines' with no .length proxies at all. I'll own the contract/ADR side once the issue exists. Until then, please add a short comment above the classification block in overviewPdf.ts recording that budgetLines.length > 0 is a proxy for line-split and naming the case it cannot see — right now a future reader will reasonably assume it's exact.

M2 — wiki/API-Contract.md misdescribes budgetLines[] (pre-existing, my fix).

L3610 reads "all ibl lines per invoice (even portion 0)", and shared/src/types/sourceReport.ts repeats it. As traced above, the server returns only lines belonging to the reported source. The distinction is load-bearing for exactly this story's logic, so it must not stay wrong. Not introduced by this PR — I'll correct the page and SourceReportInvoice's doc comment plus a Deviation Log entry in a separate wiki PR. Flagging here for traceability.

M3 — getAttachmentNote widens attachmentType to string, re-opening the raw-key-on-a-bank-PDF failure mode.

overviewPdf.ts:49 types the param as Array<{ attachmentType: string | null }> and interpolates it into a template-literal key: t(`sourceReports.table.attachmentType.${type}`). If a fourth AttachmentType member is ever added server-side, this compiles, passes unit tests, and prints sourceReports.table.attachmentType.<new> onto a bank-facing PDF. The story's own Notes call this out as the failure mode from earlier rounds.

Suggested fix — make it a compile error instead:

import type { AttachmentType } from '@cornerstone/shared';

const ATTACHMENT_TYPE_KEYS: Record<AttachmentType, string> = {
  quotation: 'sourceReports.table.attachmentType.quotation',
  deposit: 'sourceReports.table.attachmentType.deposit',
  invoice: 'sourceReports.table.attachmentType.invoice',
};

Adding an enum member then breaks the build at the map. The as string[] cast on line 60 goes away too if the filter becomes a type predicate.

---

Low / informational

• L1 — appendixByInvoiceId is now a genuinely dead parameter. "Signature stability" isn't a real constraint here: one internal call site (merge.ts:117), no external consumers. Prefer deleting the param and updating the call site over leaving a dead arg that the next editor has to re-derive is unused. If it's kept deliberately, the comment at overviewPdf.ts:315-316 should live on the parameter, not at the old cell-push location.
• L2 — buildSummaryRow's const leadingCount = isOverview ? 4 : 3 re-derives the layout from isOverview rather than from the column set it must align to. columns.length - 3 (usage + the two amount columns) is genuinely index-agnostic and won't drift if another conditional column lands in Bank report wizard: Settings step with report language selection #1899/Bank report wizard: editable HTML preview before export #1900.
• L3 — the raw-key-leak walk in realRender.test.ts [Scenario 23] anchors on /^sourceReports\.[a-zA-Z.]+$/ and only runs the claim layout. The status cell uses the sources.lines.invoiceStatus.* namespace and only renders under budget-overview — a leak there is invisible to this guard. Broaden the pattern to a generic dotted-key shape and add a budget-overview pass; it's the one column the walk currently can't see.
• L4 — the report.invoices.find(...) lookups inside the split and deposit footnote loops are O(n²) and produce the two dead ?? '—' branches noted in the coverage exception. Storing { num, wording, vendorName, invoiceNumber } in the footnote maps at classification time removes the scan and the unreachable branch — the coverage ceiling then resolves itself rather than needing an exception.
• L5 — getUsageText filters line.description !== null, so an empty-string description survives and contributes an empty segment to the comma-join ("Foo, , Bar"). Falsy-check instead of null-check.
• L6 — typo deducedTypes → dedupedTypes (overviewPdf.ts:71); [columns as Content[]] (L202) is a redundant cast now that columns is declared Content[].
• L7 — for UAT: eyeball the ‡ (U+2021) glyph in a generated PDF. pdfmake won't throw on a missing glyph, it renders a box, so no test can catch it. Risk is low († already renders from the same Unicode block) but it's a one-look check on a bank-facing document.

---

Merge gate

Quality Gates is currently red, solely on server/src/routes/timeline.test.ts → "returns projectedDate equal to the max endDate of linked work items" (Expected: "2026-07-30", Received: "2026-07-31"). Unrelated to this diff; the announced follow-up commit is not on the branch as of a65ff3a8. Two notes for whoever lands it:

• The test already freezes Date to 2026-01-15 via jest.useFakeTimers, yet the handler still resolved today's real date — so the clock source the CPM "today floor" actually reads is not the faked one. Fix the clock source; don't just bump the literal, or it re-expires tomorrow.
• CI must be green before merge regardless — this approval is conditional on that, not a waiver.

[steilerDev]

[product-owner]

Verdict: APPROVE — all AC groups of #1898 met

Reviewed gh pr diff 1902 against every acceptance criterion in #1898, plus the ux-designer visual spec comment on that issue. Verified against the contract in shared/src/types/sourceReport.ts and the real-render evidence in client/src/lib/reportPdf/realRender.test.ts.

---

1. Usage column — PASS

AC	Evidence
New Usage column exists	Appended as trailing column, both layouts (Scenarios 1, 2)
Distinct linked-item names	getUsageText → uniqueInOrder(...).join(', '); Scenario 4 + real-render cross-check asserts 'Roof Replacement' once from two lines sharing the item
Fallback to distinct descriptions	Scenario 5
— when neither	Scenario 7 (budgetLines: [])
Wraps / no overflow / no collapse	Real pdfmake render produces a non-empty blob for the multi-item fixture in both locales (Scenario 23)
Remaining columns still fit	Widths asserted ['*','auto','auto','auto','auto','auto','*'] (7-col) and ['*','auto','auto','auto','auto','*'] (6-col)

The mixed case (some lines linked, some description-only) shows linked names only — that matches the AC as written ("one or more linkedItem entries → shows the distinct linked names"), and Scenario 6 pins it deliberately.

Deviation from the ux-designer spec, accepted: the spec called for '2*' on the Usage column. That is not a valid pdfmake 0.3.11 width unit and crashed the real renderer with unsupported number: NaN — documented in the realRender.test.ts header and fixed to a plain '*'. Correct call: an infeasible spec detail, caught by real rendering rather than shipped. Vendor and Usage now split remaining space evenly, which still satisfies the AC. Worth folding back into the Style Guide so 2* isn't re-specified for a future PDF table.

2. Attachments note — PASS

Count + deduped translated types (Scenarios 9–11); null-type documents counted but contributing no label, degrading to a plain count (Scenarios 12, 13); singular/plural in both locales asserted against real i18next, not a key-echo mock — 1 attachment: Invoice / 2 attachments: Quotation, Invoice and 1 Anhang: Rechnung / 2 Anhänge: Angebot, Rechnung (Scenario 25). No documents → plain { text } cell, no stack, no empty line (Scenario 8).

The note is derived purely from invoice.documents and never consults appendixByInvoiceId or skippedDocuments, so it correctly describes the invoice's attachments regardless of the "attach documents" option — Scenario 23 exercises exactly that path with attachDocuments: false.

I specifically checked the attachmentsNote_one + { count } combination, since passing count to an already-suffixed key is a plausible way to end up with a raw key on a bank-facing PDF. The real-render assertions prove it resolves in both locales, and the leaked-key sweep (/^sourceReports\.[a-zA-Z.]+$/ over the whole content tree) is a good structural guard. This is the right response to the earlier rounds of this mini-epic shipping raw keys under a green suite.

3. Column cleanup — PASS

Appendix column removed unconditionally — header, cell, subtotal/total placeholder, and width slot all gone; Scenario 3 pins that a non-empty appendixByInvoiceId produces no column in either use case, and the sourceReports.table.appendix key is deleted from both locales. Append order is unchanged: merge.ts is untouched by this PR (verified against the changed-file list), so appendix numbering and PDF page-embed ordering are byte-for-byte as before. The skipped-document footnote family is untouched and still attributes vendor + invoice number.

Status column present for budget-overview, absent for claim/proof-of-funds. Subtotals remain status-driven and the summary rows are now index-agnostic via buildSummaryRow — label lands at index 3 of 7 (overview) and index 2 of 6 (claim), i.e. always immediately left of Invoice Amount, with the bold amount in the Allocated Amount slot and an empty trailing Usage cell. Scenario 22 pins both shapes. This is the fix the ux-designer flagged as the main non-visual risk, and it was done properly rather than with a report-type branch on cell position.

4. Deposit footnote — PASS on all four pinned cases

The implemented rule († iff isSplit && budgetLines.length > 0; ‡ iff isSplit && deposits.length > 0, wording constituted when a deposit is tagged to this source, reduced otherwise) resolves the four cases correctly:

1. Deposit-only cross-source invoice (budgetLines: [], tagged deposit) → ‡1 "This is a deposit.", no † (Scenario 14). This was the case I most wanted to see and it is exactly right.
2. Pure line-split → †1 with unchanged wording, no ‡ (Scenario 15).
3. Both facts → †1 and ‡1, neither suppressing the other (Scenario 16).
4. No cross-source funding → neither marker (Scenario 18).

isSplit is consumed directly from the server payload; allocatedAmount < invoiceAmount appears nowhere in the classification. Markers use three independent counters in the *N → †N → ‡N order (Scenario 19), and both footnote maps drive the markers and the footnote entries from the same source, so a marker without an entry or an entry without a marker is structurally impossible. Footnote block ordering and the [0,4,0,0] first-entry margins match the spec (Scenario 20). Both ‡ wordings render for real in en and de (real-render test).

One residual gap, not a blocker — this is a limitation in my own AC, not in the implementation. AC case 1 also covers the variant "its budget lines all belong to the reported source" alongside "or it has none". SourceReportBudgetLine carries no source identifier, and deposits[] is server-filtered to untagged-or-this-source, so the client has no signal that would let it distinguish "budget lines span sources" from "budget lines all on this source, split caused by a deposit". Suppressing † in that variant is not expressible without a server change, which #1898 explicitly ruled out. The chosen rule is the maximum fidelity available against the current contract, the generic split sentence remains factually true in that variant, and the deposit fact is still conveyed alongside it. I'll track the precise variant as a follow-up rather than hold this PR.

Related refinement for that follow-up: in the co-occurrence case the ‡ wording is constituted ("This is a deposit.") even though the position is only partly deposit-funded. AC case 3's actual requirement — both facts conveyed, neither suppressed — is met, so this is a wording precision item, not a gap.

5. Translations — PASS

All new keys present in both client/src/i18n/en/budget.json and client/src/i18n/de/budget.json with matching key sets: usage, attachmentType.{quotation,deposit,invoice}, attachmentsNote_one/_other, attachmentsNoteNoType_one/_other, depositReducedFootnote, depositConstitutedFootnote; appendix removed from both. attachmentType keys match the AttachmentType union exactly ('quotation' | 'deposit' | 'invoice'), so no t() path can miss. German glossary terms are correct and asserted against real bundles: Angebot, Abschlagszahlung, Rechnung. No hardcoded user-facing text.

Scope

Clean. Changes are confined to client/src/lib/reportPdf/ and the two budget.json files. Wizard step 3 untouched, no report-language work (#1899), no editable preview (#1900), no AI usage text (#1901), no stage-matched attachment indicator (#1888).

---

Non-blocking nits (no re-review needed)

1. appendixByInvoiceId is now dead inside buildOverviewContent — retained only in the signature. The inline comment explains why, but the param is genuinely unused by this function now; worth a look from product-architect on whether it should be dropped from the signature.
2. deducedTypes in getAttachmentNote reads like a typo for dedupedTypes.
3. ‡ glyph subsetting: the real render succeeds and produces a non-empty blob for a fixture containing deposit footnotes, so a hard failure is ruled out, but a visual check that it isn't a tofu box is still worth doing once during UAT. Low risk — † already ships from the same Unicode block.

Merge precondition (outside my AC scope, per the task framing)

Quality Gates is currently red on Test (Shard 3/6) from server/src/routes/timeline.test.ts › returns projectedDate equal to the max endDate of linked work items — a date-drift failure unrelated to this story; the fix commit is not yet on the branch. E2E Gates is red on the known pre-existing shard-5 flake (invoices.spec.ts › "Effective Amount" column (Issue #1876)), which this PR does not touch. Neither affects my verdict, but Quality Gates must be green before merging to beta.

[steilerDev]

[product-owner] Note on mechanics: gh pr review --approve is rejected by GitHub ("Can not approve your own pull request") because this PR is authored by the same account. The review above is therefore posted as a comment — treat it as the product-owner approval for the PR review gate.

[github-actions]

🎉 This PR is included in version 2.13.0-beta.31 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀