ADR 034 Client Side Report PDF Generation - steilerDev/cornerstone GitHub Wiki
Accepted
The Bank Report Wizard (mini-epic "Bank Report Wizard", stories #1878/#1879) produces the single most externally-visible artifact Cornerstone generates: a PDF claim package handed to a bank or subsidy authority. It is composed of three parts:
- A cover letter β addressee, claim period, total amount, signature block.
- An overview table β one row per invoice/budget line (vendor, invoice number, date, invoice amount, allocated amount, budget source, appendix marker), followed by a legend block: numbered notes for documents that could not be attached, plus document-level legend entries explaining the inline row annotations (see the legend addendum below).
- An appendix β the original invoice PDFs, fetched from Paperless-ngx and appended page-for-page behind the generated pages.
Part 3 is the constraint that shapes everything else: the deliverable is not a rendered document, it is a generated document merged with foreign PDFs that already exist in Paperless. Any solution must both author pages and splice in pages from arbitrary third-party PDFs.
The wizard is also interactive. Step 4 offers options (attach documents on/off, include zero-allocation lines, grouping) and shows a live preview in an iframe. The product requirement is "whatever is previewed is what ships" β the bytes in the preview must be byte-identical to the bytes that are downloaded or uploaded back to Paperless. This rules out any design where preview and export are separate render passes.
Deployment context matters as much as the feature: Cornerstone is a single Docker container serving fewer than 5 users, with SQLite on a volume and no worker process, no job queue, and no headless-browser runtime (ADR-003, ADR-007). Paperless-ngx is already the system of record for documents (ADR-015), so the app is not itself a document archive β a generated report either goes straight to the user's disk or is uploaded back into Paperless.
| Option | Where it runs | Can merge foreign PDFs | Container cost | Native binaries | Preview fidelity |
|---|---|---|---|---|---|
| pdfmake + pdf-lib in the browser (chosen) | Client | Yes (pdf-lib copyPages) |
None | No (pure JS) | Exact β preview is the artifact |
| Puppeteer / headless Chromium HTMLβPDF | Server | No (needs a second tool) | Very high (~300MB+ Chromium, sandbox/--no-sandbox) |
Yes (Chromium binary) | Approximate (re-render per request) |
| PDFKit (or pdfmake) on the server | Server | No (needs pdf-lib anyway) | Moderate (CPU/RSS spikes on the only process) | No | Approximate (needs a preview endpoint + cache) |
| Server-side pdfmake + pdf-lib | Server | Yes | Moderate; adds a stateful preview/temp-file lifecycle | No | Only via a cached-artifact endpoint |
| jsPDF in the browser | Client | Partial/awkward | None | No | Exact |
Client-side generation (chosen).
-
It is what makes "preview equals download" true by construction. The wizard already holds the full report payload (
GET /api/source-reports) and can fetch each linked document through the existing proxy (GET /api/paperless/documents/:id/preview). Generating in the browser produces oneBlobthat is simultaneously the iframesrc(viaURL.createObjectURL) and the bytes handed to the download anchor and to the Paperless upload. There is no second render pass to diverge, and no server-side artifact cache, temp directory, or TTL to reason about. - Zero added server surface. No new server dependency, no new runtime in the image, no PDF code path to keep patched, and no CPU/RSS spike on the single process that also serves the API and the SPA. A 40-invoice report merging 40 multi-megabyte PDFs is a meaningful memory transient; on the client it is the user's own tab, and it cannot degrade the API for anyone else.
- The client is already where the work is. Every option toggle changes the document; server-side generation would mean a network round trip per toggle plus an artifact lifecycle, for a document only ever consumed by the person who requested it.
- Fits the scale. Fewer than 5 users, one container. Server-side PDF rendering is machinery for multi-tenant/scheduled reporting; neither exists here.
- Paperless is the archive. The report does not need to be persisted server-side to be durable β the upload sink puts it back into Paperless, which is already the document system of record (ADR-015). This removes the strongest argument for server-side generation (server-owned, retrievable artifacts).
Puppeteer / headless Chromium (rejected). Adds a platform-specific Chromium binary β a direct violation of the dependency policy's no-native-binaries rule for build/render tooling (CLAUDE.md, ADR-004), and a known hazard in the ARM64-emulated dev environment. It would multiply the image size several-fold, add a sandbox/--no-sandbox security decision to the container, and become a recurring CVE-patching surface. It still cannot append foreign PDF pages, so pdf-lib (or similar) would be needed anyway. Rejected outright.
PDFKit / pdfmake on the server (rejected). Pure JS and dependency-policy-clean, but it buys nothing the client does not already provide while costing: (a) a new preview endpoint with an artifact cache or temp-file lifecycle, (b) a per-toggle round trip that ships every generated PDF over the wire twice (preview, then export), (c) render CPU and peak RSS on the single process, and (d) the loss of the byte-identity guarantee unless the preview artifact is itself retained and re-served. Since a server-side design would still need pdf-lib for the appendix, the only difference from the chosen design is where the bytes are built β and the server is the worse place at this scale.
jsPDF (rejected). Viable for simple output, but its declarative table/layout story is weaker than pdfmake's document-definition model (which the cover letter and 7-column overview lean on heavily), and it does not solve foreign-page merging either.
This is the non-obvious part of the decision. pdfmake generates; pdf-lib merges. pdfmake takes a declarative document definition (styles, tables, column widths, headers/footers) and renders it to a fresh PDF, but it has no ability to embed pages from an existing PDF. pdf-lib can load arbitrary PDFs and copyPages them into a target document, but authoring a styled multi-page table with it means hand-positioning text. The split is therefore along a real capability seam, not redundancy:
pdfmake: report data + i18n + formatters -> cover letter + overview pages (Blob)
pdf-lib: that Blob + N fetched Paperless PDFs -> final merged Blob
Generate report PDFs entirely in the browser, using [email protected] for document authoring and [email protected] for merging fetched Paperless invoice PDFs into the appendix. Both are exact-pinned (no caret) per the dependency policy, together with @types/[email protected]. The combined subtree (27 transitive packages: fontkit, pdfkit, restructure, pako, brotli, linebreak, @noble/*, unicode-*) is pure JavaScript with no install scripts and no platform-specific binaries, and hoists to the root node_modules/ with zero workspace nesting.
No server-side PDF dependency is added, and no new API endpoint is introduced β the pipeline consumes only pre-existing endpoints (GET /api/source-reports, POST /api/source-reports/mark-claimed, POST /api/paperless/documents, GET /api/paperless/documents/:id/preview).
| Module | Role |
|---|---|
loader.ts |
Lazy import() of both packages, promise-cached; owns vfs/font wiring |
pageGeometry.ts |
Sole owner of the pt coordinate system: PAGE_WIDTH/HEIGHT, PAGE_MARGIN_*, CELL_PADDING_X, V_LINE_WIDTH, font sizes, PDF_STYLES, printableWidth/Height(), tableOffsetsTotal(cols), usableColumnWidth(cols), headerFootprint(), derived PAGE_TOP_MARGIN, and (since #2161) PDF_DEFAULT_STYLE
|
shared.ts |
Page header/footer builders, TABLE_LAYOUT callbacks (which read their numbers from pageGeometry.ts), PDF-local colours/sizes for cell annotations |
coverLetterPdf.ts |
Pure function: ReportContent (+ optional { partsNotice }, #2161) -> cover-letter Content[]
|
overviewPdf.ts |
Pure function: ReportContent (+ hiddenColumns) -> overview-table Content[]; owns the per-subset column geometry (computeColumnWidths, the pinned width constants, the Usage word-break/chunk budgets), cell packing, and the *N skip notes |
merge.ts |
Single-PDF orchestration: fetch documents, build, pdfmake render via buildReportDocDefinition, pdf-lib merge. Re-exports PDF_DEFAULT_STYLE / PDF_STYLES from pageGeometry.ts for existing test consumers |
docDefinition.ts |
(#2161) Sole owner of the pdfmake document definition: buildReportDocDefinition(content, reportContent) returns page size, margins, header (none on page 1), footer, defaultStyle and styles. Used by both merge.ts and parts.ts
|
sinks.ts |
Output: browser download / preview blob URL / upload to Paperless |
types.ts |
ReportPdfOptions, GeneratedReport, SkippedDocument; since #2161 also the multi-PDF types (ReportAttachment, AttachmentCache, AcquireResult, FailedFetch, GeneratedReportPart(s), ReportPartWarning) |
index.ts |
Barrel: the public surface of the single-PDF pipeline (also re-exports ReportContent from ../reportContent/). Deliberately does not re-export anything from the multi-PDF modules below (#2161) |
partPlan.ts |
Multi-PDF (#2161). Pure, zero imports: BYTES_PER_MB, parseMaxFileSizeInput, the ordered greedy planner planReportParts, and shiftLastItem (the verification pass's re-plan input). Safe to import statically |
partNaming.ts |
Multi-PDF (#2161). Pure, zero imports: reportBaseName, padPartNumber, partFileName. Safe to import statically |
attachments.ts |
Multi-PDF (#2161). Fetches each linked invoice PDF once through the existing preview proxy, validates it with pdf-lib, and records it in the caller-owned AttachmentCache; countUncachedDocuments
|
continuationLetterPdf.ts |
Multi-PDF (#2161). Pure function: ReportContent + pre-resolved ContinuationLetterText -> the one-page attachment letter that opens every part after the first |
parts.ts |
Multi-PDF (#2161). Orchestration: overhead estimation, planning, per-part render + merge, the bounded verification pass, warnings. Loaded only via dynamic import() (see the multi-PDF addendum) |
One module outside this directory is load-bearing for the table and belongs in the same picture:
Module (outside reportPdf/) |
Role |
|---|---|
../reportContent/columns.ts (#1973) |
Sole derivation of which columns exist: ReportColumnKey, the base set per use case (reportColumnsForUseCase), the locked column (REQUIRED_REPORT_COLUMN / isColumnLocked), and visibleReportColumns(isOverview, hidden). Consumed by both overviewPdf.ts and components/reports/ReportContentEditor.tsx
|
It lives in reportContent/, not reportPdf/, deliberately: the rule it encodes is a property of the content model, not of the layout. A claim report has no status value at all (buildReportContent.ts, status: isOverview ? status : null), so status is physically absent from CLAIM_COLUMNS and no code path can add it β a structural guarantee rather than a runtime filter. The dependency direction is the pre-existing one (reportPdf/ already consumes reportContent/); it adds no new edge and no cycle. This is the same "one definition, two consumers" invariant stated for ReportContentLabels below, and it exists for the same reason: columns.ts decides which columns the toggle UI offers and which columns the PDF renders, so the preview cannot drift from the document.
shared.ts deliberately owns no formatters. It briefly did; they were deleted in review because a PDF-local formatter is a second implementation of client/src/lib/formatters.ts bound to a different locale β exactly the B3 defect below. All formatting reaches the builders through the injected reportFormatters.
pageGeometry.ts was extracted in #1939 because the page geometry had been duplicated across four sites (PAGE_TOP_MARGIN in shared.ts, L/R/B margins inline in merge.ts, a printable-width prose comment in overviewPdf.ts, paddings in TABLE_LAYOUT) and drifted. Note that tokens.css is explicitly not the home for these values: the pdfmake layer is its own pt coordinate system, outside the design system (Style Guide, ux-designer's domain).
The content builders are pure functions. They therefore cannot call React hooks, which has a direct consequence: anything locale- or configuration-dependent must be threaded in as a parameter, not read from a module default. Reaching for the bare formatCurrency/formatDate module exports inside a builder silently falls back to en-US + EUR, ignoring both the user's locale and the CURRENCY env config β see finding B3 in the Lessons section.
Since #1900 the builders take (reportContent: ReportContent, β¦) and no formatters at all: every formatted string is pre-derived into ReportContent by buildReportContent, which is the single place reportFormatters is applied (see the content-layers addendum). Since #2001 they take no TFunction either: the last three generation-time strings (overviewPdf's *N skip-note reasons, coverLetterPdf's Reference: / Subject: prefixes) moved onto ReportContentLabels. buildReportContent is now the only module in the whole pipeline that accepts a TFunction, which turns invariant 1 below from a caller convention into a compile-time property of reportPdf/* β there is no longer a channel through which the wrong t could be passed. The fix for a new generation-time string is a new ReportContentLabels field, never a new parameter. A string whose interpolation values are only known after ReportContent is built (a part number, a part count) is the one exception, and it is still not a parameter: it is a closure on ReportContent.partTexts, built by buildReportContent over reportT (#2161, see the multi-PDF addendum).
The ~2MB cost (dominated by the base64 Roboto virtual file system) stays out of the initial bundle only because every static reference to either package is import type, which TypeScript erases. The sole value-level references in the entire client graph are the import() calls inside loader.ts. Webpack's splitChunks: { chunks: 'all' } (ADR-004) then keeps them in async chunks, and ReportWizardPage is additionally lazy()-loaded in App.tsx.
One plain import { X } from 'pdfmake/...' anywhere in the client graph silently pulls all ~2MB into the initial bundle. The guard is:
grep -rn "from 'pdfmake\|from 'pdf-lib" client/src --include=*.ts --include=*.tsx | grep -v "import type" | grep -v '\.test\.'This must return nothing. (The .test. exclusion is required: realRender.test.ts and parts.realRender.test.ts import PDFDocument from pdf-lib directly to inspect rendered output, and test files never enter the client bundle.) Any future consumer of these libraries must go through loader.ts.
These are library-version-specific facts that cost review rounds to discover. They are recorded here because they are institutional knowledge, not test trivia:
-
pdfmake/build/pdfmakeis a UMD/CJS bundle. Under NodeNext ESM the real exports live on the dynamic-import namespace's.default, not on the namespace. The namespace object is non-extensible, so assigning.vfsonto it throwsTypeError: Cannot add property vfs. -
pdfmake/build/vfs_fontsdefault-exports the font map directly. The.pdfMake.vfswrapper shape is pre-0.3.x and does not apply. -
pdfMake.fontsdefaults to{ Roboto: β¦ }only. Any other font family named indefaultStylethrowsFont 'X' in style 'normal' is not defined. By contrast, an unknown style name is silently ignored (probed β no throw), so straystyle:references are dead code rather than crashes. -
widths: 'auto'applies no total-width constraint β an over-wide table simply runs off the page rather than wrapping or erroring. A4 portrait with 40pt margins gives a printable width of exactly 515.28pt. -
Declared widths are content widths, not occupied widths. pdfmake subtracts
_offsets.totalfrom the available width before distributing the declared widths:offsetsTotal = cols * (paddingLeft + paddingRight + vLineWidth) + vLineWidth. This is why every width budget must be computed againstprintableWidth() - tableOffsetsTotal(cols), never against 515.28 directly β henceusableColumnWidth(cols)inpageGeometry.ts. Budgeting against the raw page width was how a review comment came to claim the Usage column got 185.28pt when it actually got 69.28pt. -
A
'*'column never shrinks below its longest unbreakable word.columnCalculator.js:66-75: whenminW >= availableWidththe star is set tostarMaxMinand the table overflows. No static assertion on thewidthsarray can therefore prove "no horizontal overflow" β a German compound (WΓ€rmedΓ€mmverbundsystem, ~128pt at 10pt Roboto) pushes a 69.28pt star to 128pt. The overview table's columns are consequently all fixed widths, which has its own cost:elasticWidthnever grows a fixed column to fit its own header, so every DE translation of asourceReports.table.*header key is width-constrained and DE is always the binding locale (#1937). -
dontBreakRowslives ontable, notlayout.TableProcessor.js:123readstableNode.table.dontBreakRows. Thelayoutobject is packed intonode._layout(DocMeasure.js:469) and only feeds the border/padding/fill callbacks β nothing reads_layout.dontBreakRows. Putting it inlayoutproduces a byte-identical PDF (modulo/CreationDateand/ID) while a unit test assertingTABLE_LAYOUT.dontBreakRows === truepasses and proves nothing. The@types/pdfmakedeclarations place it onCustomTableLayout, so the wrong placement also type-checks. -
_calcWidthis the verification lever.createPdf(def)mutates each entry oftable.widthsin place into an object; afterawait getBlob()each carries_calcWidth, the resolved pt width. It is a private field, so pin the version β but the claim that pdfmake's public Node API cannot expose computed widths is wrong. For text measurement,pdfkitplus the Roboto TTF out ofvfs_fontsgivesdoc.widthOfString(s)(average lowercase prose character β 4.68pt at 10pt Roboto). -
getBlob()is promise-based per@types/[email protected]and works under jsdom.getBuffer()with a callback did not fire in plain Node CJS β probe inside Jest/jsdom, not a bare Node script. - Rendering mutates the
contentobjects in place (pdfmake annotates_minWidth,_maxWidth,positions,__height). This is the mechanism by which real layout geometry can be asserted in tests.
Any change to the report pipeline must be covered by at least one test that renders the real content builders through the real pdfmake, for every supported locale. loader.test.ts deliberately does not mock either package and is the template to copy.
This is not a stylistic preference β it is the direct lesson of the PR #1887 review (see below). That PR shipped through three review rounds with 355+ green client tests while carrying five code defects (B1-B5, four of them empirically confirmable by probing the real libraries), all invisible to the suite because the pipeline was mocked at every layer (loader, shared, coverLetterPdf, overviewPdf, paperlessApi in merge.test.ts; generateReportPdf wholesale in ReportWizardPage.test.tsx). Nothing ever laid out the real document tree with the real library.
Minimum bar for a generated-document pipeline:
-
Real-layout width bound, per locale. Render, then assert on geometry pdfmake resolved β for both
enandde:-
No token overflows its cell (the content-extent check): for every cell of the overview table, that cell's
_minWidth <= widths[i]._calcWidth. Both fields are written during render._calcWidthis the resolved column width;_minWidthis measured afterTextBreakerhas applied the cell'swordBreak, so it is the width of the widest run pdfmake cannot break β which is exactly the quantity that overflows a fixed-width column. Measured against the 69.28pt Usage column: a long compound token withwordBreak: 'break-all'yields_minWidth33.54pt (widest single character) β passes; the same token withoutbreak-allyields 266.16pt β fails; plain prose yields 33.54pt β passes. This is the falsifiable assertion for rule #1, and the one that catches a missingwordBreakor an unbreakable token wider than its column. -
The table box fits:
tableOffsetsTotal(cols) + sum(widths[i]._calcWidth) <= printableWidth()(equality, for the current all-fixed-width shape)._calcWidthis written onto each declared width object during render. -
No element is positioned past the right margin:
max(node.positions[].horizontalRatio) <= 1.horizontalRatiois(x - pageMargins.left) / innerWidth(DocumentContext.js:490), recorded at the left edge of each text line and before the line is placed (ElementWriter.js:32). It is therefore a cell-origin bound, not a content-extent check. Scope it accordingly:- It does fire when an element's origin lands off-page β e.g.
widths: [600, 50], where the second column starts beyond the printable right edge. That is its legitimate use: table-box positioning. - It cannot fire on token overflow inside fixed-width columns whose widths sum to
printableWidth(), because every line still starts inside the margins no matter how long the token is. AssertingmaxHorizontalRatio(pdfContent) <= 1on the production report content is therefore vacuous β empirically confirmed by mutation testing in PR #2008: removingwordBreak: 'break-all'leaves the assertion green. Do not present it as the overflow check.
- It does fire when an element's origin lands off-page β e.g.
-
Width-sharing
columnsgroups (not table cells) need a sum check, not the per-cell check. For a pdfmakecolumnsnode with'auto'/'*'members, the per-cell_minWidth <= _calcWidthcomparison is vacuous: when the members cannot fit,columnCalculator.js's case 1 sets every'auto'member's_calcWidthto exactly its_minWidthand every'*'member's tostarMaxMin, so the inequality holds while the group runs past its container. The discriminating assertion is that the laid-out group spans its container:amountNode.positions[0].left + amountNode._calcWidth - labelNode.positions[0].leftequals the container width (it exceeds it only in case 1). Implemented for the tier-3 summary block inrealRender.test.tsdescribe'#2011: tier-3 summary block real render'; mutation-verified in the PR #2133 review (two'*'members instead of'*'+'auto'β group width 89.87pten/ 93.84ptdeagainst the 84pt container, 4 red). -
Implementing test:
client/src/lib/reportPdf/realRender.test.tsβ describe'ADR-034 rule #1: horizontal-overflow via _minWidth <= _calcWidth (issue #2003)'. It asserts per-cell_minWidth <= _calcWidthon the real rendered overview content for both locales (the content-extent check), and separately keeps a revert-test that renders awidths: [600, 50]table to confirmmaxHorizontalRatio()returns> 1for genuine table-box overflow (the positioning check). Both must be present: the_minWidthassertions are the ones a missingwordBreakcan break.
Do not assert the table-level
table._minWidth <= printableWidth(). The table-level_minWidthis the sum of the columns' minimum widths, and as a whole-table bound it both false-negatives and false-positives: a table whose columns wrap fine still overflows if a'*'column'sminWexceeds the available width (columnCalculator.js:66-75sets the star tostarMaxMinand lets the table run off the page). An earlier revision of this ADR stated that form as rule #1; it was a mis-transcription of finding B2's narrative (where the table-level_minWidthwas the diagnostic that located the bug) into a correctness check, which it is not.This ban does not extend to the per-cell form. A cell's own
_minWidthcompared against that cell's own_calcWidthis a different measurement and is the correct check (see the first bullet above). A previous revision banned_minWidthoutright on the grounds thatwordBreak: 'break-all'makes it false-positive; that rationale is empirically false for the per-cell form, because_minWidthis computed afterTextBreakerapplieswordBreakand so already reflects the break (33.54pt withbreak-all, not the 266.16pt natural token width). The two forms were conflated; keep them distinct. -
No token overflows its cell (the content-extent check): for every cell of the overview table, that cell's
-
Height bound on what a cell renders, per locale. See "Unbreakable rows are silently dropped" below. Page count must be monotonic in content size and channel-independent.
-
Unique-value mocks for anything whose return feeds
setState. A mock returning a constant can terminate a loop that never terminates in production. -
Idle-stability assertion. Settle, then assert the generation call count has not grown.
-
Export sinks disabled on preview error. For any preview+export pair, assert the export path is unavailable when the preview is in an error state.
table.dontBreakRows: true makes pdfmake measure a row as one unit. If that unit is taller than the printable height (pageHeight - topMargin - bottomMargin = 706.89pt at A4 with the current margins), pdfmake does not paginate it β PageElementWriter discards the overflow. No throw, no visible truncation, no warning. Measured on a 7-column table with a 69.28pt Usage column: 450 characters render, 500 characters render the header row only, and the text-show-op count stays flat no matter how much longer the text gets.
This defect recurred across three review rounds, each time with a fix that passed its own unit tests. The rule that finally held:
Bound the height of what a cell renders, never the length of a source field.
A per-field cap is coupled to a layout decision and detaches silently the moment the layout changes β which is exactly how one round's per-field continuation rows became the next round's unbounded inline run. The bound belongs where the cell's content stream is assembled (packUsageCellRows(segments, usageChunkChars) in overviewPdf.ts), so a newly added segment is bounded automatically rather than requiring the cap to be extended. The budget argument was the bare constant MAX_SAFE_USAGE_CHUNK_CHARS until #1973 made it per-subset β see "Column geometry is a computed engine, not two pinned shapes" below.
Two threshold-free detection recipes β both reusable for any generated document:
- Page count must be monotonic in content size. The symptom of a dropped row is saturation (page count pins at ~2) and sometimes non-monotonicity (3 β 2 pages as content grows) while the rendered line count rises linearly.
- Page count must be channel-independent. The same text costs the same paper whichever field carries it. A discrepancy means one field's content is escaping the bound.
The assertion asymmetry that let it recur, and the fix. The per-row character budget is asserted generically (it counts characters across all runs, so a new channel would be counted), but the test inputs are hand-enumerated (usageText, areaText, attachmentsNote). A newly added string field on ReportContentRow therefore defaults to empty and every assertion passes vacuously. The durable fix is key-driven saturation β iterate Object.entries(row) and saturate every string field β not another hand-written case.
Until #1973 this section carried the opposite claim β that the PDF's column count was fixed at 6 or 7, and that letting the wizard's column-visibility toggles reach the PDF "would widen the Usage column and invalidate the measured ceiling β that is a re-measurement story, not a UI change." That is no longer true, and the reason it stopped being true is the useful part.
What the table renders now. Any of the 96 legal column subsets: allocatedAmount is the only mandatory column, every other column is freely hideable in any combination, and the report type's base set is the ceiling (64 subsets for budget-overview, 32 for claim/proof-of-funds, which has no status value in the content model at all β buildReportContent.ts, status: isOverview ? status : null). Visible column counts range 1β7.
The width mechanism: a single absorber. computeColumnWidths(visible) in overviewPdf.ts picks one absorber column β usage if visible, else vendor, else none β which takes usableColumnWidth(n) - fixedSum; every other visible column keeps its pinned constant. This is chosen over the two alternatives for reasons that should survive a future edit:
-
'*'is excluded by the constraint two sections up (a'*'column never shrinks below its longest unbreakable word, so itsstarMaxMinbranch pushes the whole table pastprintableWidth()). Every column stays an explicit number. - Distributing the slack proportionally across all visible columns would stretch bounded numeric columns; a two-column numeric table spread across 515.28pt reads as broken in a bank document.
The single-absorber form also makes the width invariants algebraic rather than enumerated: with an absorber, total = tableOffsetsTotal(n) + fixedSum + (usableColumnWidth(n) - fixedSum) = printableWidth() exactly; with no absorber, no term consumes the slack, so the table renders strictly narrower than the page and left-aligned. Non-absorber columns keep their pinned widths by construction. The 96-subset test suite confirms the construction; it is not what establishes it. Anything laid out beneath the table follows the table, not the page (#2011): the tier-3 summary block (visible β {allocatedAmount, usage}, rendered as a stack below the table) takes a right margin of printableWidth() - (tableOffsetsTotal(n) + Ξ£ widths), so it is exactly the rendered table's width β 84pt for {allocatedAmount} alone, a zero margin whenever an absorber exists. Its rows are label '*' + amount 'auto', never two '*': two stars split the width evenly and a six-digit de total (123.456,78 β¬, 46.92pt bold) forces case 1. The preview's .summaryTable stays width: 100%, which is the same rule on that surface, because the preview's .table is also always 100% wide.
Why MAX_SAFE_USAGE_CHUNK_CHARS (650) needed no re-measurement β the load-bearing fact. It was measured against a real render at the 7-column shape's Usage width, USAGE_WIDTH_7COL = 138.28pt. That width is provably the narrowest Usage can ever be across all 96 subsets: hiding a column both shrinks fixedSum (by at least the 40pt STATUS_WIDTH) and grows usableColumnWidth(n) (by tableOffsetsTotal's 8.5pt per-column increment), so Usage is strictly monotone-decreasing in visible column count and is at its minimum at the full 7-column set. Hiding columns therefore only ever makes the measured ceiling more conservative β characters-per-line rises (16 β up to 50), row heights fall, and the 650 budget cannot be breached by anything this feature can do. The reference measurement is a floor, not a midpoint, which is why generalising the geometry was safe without re-rendering.
The constraint that remains real, and the clamp that guards it. The hazard is the opposite direction and arrives later: a future added column narrows Usage below 138.28pt and drops the true ceiling below 650, silently reinstating #1929's content-loss defect. usageChunkCharsForWidth(w) is therefore a deliberate one-sided clamp β min(MAX_SAFE_USAGE_CHUNK_CHARS, floor(MAX_SAFE_USAGE_CHUNK_CHARS * w / USAGE_WIDTH_7COL)) β that may scale the budget down for a narrower Usage column and must never scale it up for a wider one. The linear model is correct because the budget is lines x chars-per-line at a page-height-fixed line count, so halving the width halves the character budget. No subset among today's 96 reaches the downward branch; it exists for the future case, and is tested by calling the function directly with synthetic widths rather than by enumerating today's subsets. Do not "simplify" this to always return the constant β the asymmetry is the entire point. The USAGE_SAFE_TOKEN_CHARS_*COL word-break thresholds are likewise recomputed per subset via usageSafeTokenCharsForWidth(w).
Adding a column is still a re-measurement story. What #1973 removed is the prohibition on hiding columns, not the coupling between Usage width and the measured ceiling. A new column shrinks Usage below the reference width, and the clamp will quietly tighten the budget rather than fail β which is the safe behaviour, but it also means the new true ceiling is unverified until someone re-renders. Re-measure before relying on the clamped value.
Which links force a new column and which do not. Adding a key to ReportColumnKey is a compile error in PINNED_WIDTHS (Record<FixedColumnKey, number>), HEADER_LABEL (Record<ReportColumnKey, string>), and buildBodyCell's default-less switch with a declared return type. It is not a compile error in columns.ts's OVERVIEW_COLUMNS/CLAIM_COLUMNS, nor in overviewPdf.ts's LEADING_COLUMNS, RIGHT_ALIGNED_COLUMNS, or the absorber-priority ternary β those are hand-maintained lists over the same union, and the tests that appear to count them pin literal 7/6 against the test's own hand-typed array. A new column omitted from them fails soft (absent everywhere, or a summary label landing one column early), not loudly.
Related sharp edge, now historical: this section previously recorded that packUsageCellRows hangs on maxChars <= 0 while splitIntoPageSafeChunks fails loudly, and closed with "unreachable while the budget is a constant; it matters the moment it becomes computed." #1973 is that moment β and the prediction held, in the sense that the guard was needed. It was also already fixed by then: both functions now throw a plain Error on a non-positive budget (overviewPdf.ts, packUsageCellRows and splitIntoPageSafeChunks), so a computed budget fails fast instead of spinning. overviewPdf.ts passes 0 when Usage is hidden and relies on a caller-side if (!usageVisible) guard to never reach either function; that is correct but is a guard rather than a structural impossibility.
Each of these was found by probing the real libraries, and each maps to one of the rules above:
-
B1 β unbounded regeneration loop.
regeneratePdf'suseCallbackdeps includedpreviewUrl, and the callback itself setpreviewUrlfromURL.createObjectURLβ which returns a unique string per call. The debounce effect depended on the callback, closing a self-retriggering cycle: 6 full report generations in 2.5s idle, growing without bound, each one re-fetching every linked Paperless document over HTTP. The suite was green solely because the test mockedcreatePreviewUrlwith a constant return: React bails out of re-render onObject.is-equal state, so the cycle terminated at 2 calls in the test and never in the browser. Generalizable rule: when a mocked function's return value flows intosetStateand that state sits in auseCallbackdep array that an effect depends on, the mock must return unique values and the test must assert idle stability. -
B2 β German overview table overflowed A4 and clipped its last column.
widths: ['auto', β¦]imposes no total constraint. Measured on the real layout:en_minWidth504.4pt (2% headroom against 515.28pt),de553.7pt β overflows by 38pt, leaving ~4pt for the final column. The driver was header labels (Zugeordneter BetragvsAllocated Amount), not data. Generalizable rule: layout bugs in generated documents are locale-dependent and German is consistently the worst case; assert real-layout widths for every locale. Note that long multi-word values do not widen anautocolumn (pdfmake's_minWidthis the widest single word, so it wraps) β headers are the risk. The_minWidthvalues quoted here are table-level sums, which are a diagnostic rather than a correctness check β the correctness check is the per-cell_minWidth <= _calcWidthcomparison in minimum-bar rule #1. Note also that the table has since moved to all-fixed widths, which changes the failure mode from "column overflows" to "header cannot fit its own label" (#1937). -
B3 β amounts and dates ignored locale and
CURRENCY. Two causes: (a)t(bodyKey, { total: report.totalAmount })β the i18n config sets onlyescapeValue: falsewith noformatfunction, so i18next interpolated a bare1234.56as the headline figure of a bank-facing letter; (b) the pure builders called the module-defaultformatCurrency/formatDateexports, so adeinstance withCURRENCY=CHFrenderedβ¬1,234.56/Jan 10, 2026in the PDF while the screen beside it showedCHF 1.234,56/10.01.2026. Generalizable rule: pure export/report builders cannot useuseFormatters(), so resolved formatters must be threaded in as parameters, and interpolated money/date placeholders must be pre-formatted before reachingt(). -
B4 β marker namespace collision.
*Nwas used for two independent counters in one document (appendix numbers and skipped-document footnotes), and the footnotes were orphaned β no row referenced them and the owning invoice id was discarded. Generalizable rule: a document has one marker/footnote numbering source, and no note may be orphaned β every note must be resolvable from what the reader actually sees in the table. There are exactly two legitimate note shapes and they must not share a numbering scheme: (a) a numbered, row-owned note (*N), which the owning row must reference by that number β this is what B4 broke; and (b) a document-level legend entry keyed by a repeated inline label that any number of rows may carry, where the reader resolves the label rather than a number. Shape (b) is what the split / deposit-reduced annotations became after #1965 β see the legend addendum below. Note that "one entry per owning row" is a property of shape (a) only; do not generalize it to the legend. -
B5 β stale export after failed regeneration. The catch path nulled
previewUrlbut keptpreviewBlob, and the export buttons were gated only onisRegenerating. So "uncheck Attach documents β generation fails β download" yielded a PDF that still had the documents attached β breaking the exact invariant the feature exists to guarantee. Generalizable rule: the preview-equals-export invariant must hold on the error path too. -
M4 β the root cause. No test rendered the real builders through the real library;
loader.test.tsrendered only{ text: 'hello' }. That single gap is why B2 and B3 survived three review rounds. One integration test pipingbuildCoverLetterContent+buildOverviewContentinto a realpdfMake.createPdf(...).getBlob()for both locales closes the whole class.
The report's language is chosen per-run in the wizard's Settings step and is independent of the app's ambient locale. Exactly two locale-bound artifacts are built in ReportWizardPage.tsx and threaded into the pipeline; nothing inside reportPdf/ is locale-aware on its own:
-
reportT = i18n.getFixedT(reportLanguage, 'budget')β synchronous becauseclient/src/i18n/index.tsstatically imports both theenanddebundles into oneresourcesobject. Never calli18n.changeLanguage()to switch report language β that mutates the app UI. -
reportFormatters = createFormatters(localeTag, currency)(client/src/lib/formatters.ts).useFormatters()is a thin wrapper over the same factory, so screen and report formatting cannot diverge in implementation, only in bound locale.
Report-language rendering flows exclusively through the two injected artifacts above. Stated as invariants that must be preserved on every change:
-
reportPdf/*andreportContent/*never reach the i18n singleton or the ambient locale. No module under either directory may import thei18ninstance, calli18n.t/useTranslation(), or use rawIntl.*/toLocaleString/toLocaleDateString. Every locale-dependent value arrives as a parameter:reportTfor text,reportFormattersfor numbers, money, and dates. -
Every key passed to
reportTis bare (sourceReports.*, resolved against thebudgetnamespace fixed atgetFixedTtime), never namespace-prefixed. A singlet('common:β¦')resolves against the interface bundle and leaks the UI locale into the PDF. -
i18n.changeLanguage()is never called to switch report language. It mutates the app UI.getFixedTis the whole mechanism. -
The bound locale is chosen once, at the page boundary.
ReportWizardPage.tsxis the only place that mapsreportLanguagetoreportT/reportFormatters. There is no second entry point, and no module downstream re-derives the locale. -
No module under
reportPdf/accepts, imports, or references aTFunctionat all (#2001).buildReportContentis the single point at which aTFunctionenters the pipeline; every module downstream of it receives already-resolved strings onReportContent. This is the strongest of the five invariants and the only one a compiler can enforce: with noTFunctionin scope, there is no wrongtavailable to pass.
Why this is a contract and not a style preference: the preview and the export are the same bytes by construction (see the Decision above), so a locale leak does not produce a wrong preview and a right export β it produces a document whose language is internally inconsistent, and which is inconsistent with the language the user selected. The failure is silent: a leaked string is still a valid string, just in the wrong language, and it renders without error. Nothing in the type system distinguishes reportT from the interface t β both are TFunction β so wherever a TFunction is in scope at all, the only defences are this contract, review, and per-locale real-render tests. Invariant 5 exists to shrink that surface to a single module: for reportPdf/* the contract is now compiler-enforced, and only buildReportContent still depends on the caller having picked reportT.
Grep guard for invariant 1:
grep -rnE "useTranslation|/i18n/|Intl\.|toLocale" \
client/src/lib/reportPdf client/src/lib/reportContent \
--include='*.ts' | grep -v '\.test\.' | grep -v "i18n/unionKeys.js"The unionKeys.js exclusion is exact, not a loophole: I18N_UNION_KEYS (#2029) is a pure key-set module whose only import is @cornerstone/shared; buildReportContent uses it to build keys that it then passes to reportT, so no i18n instance or ambient locale is reached. Any other /i18n/ hit is a violation.
Grep guard for invariant 5:
grep -rn "TFunction" client/src/lib/reportPdf --include='*.ts' | grep -v '\.test\.'Both must return nothing. Note that the invariant-1 guard cannot see an invariant-5 violation: a t: TFunction parameter matches none of useTranslation / /i18n/ / Intl. / toLocale, which is why a leak of this shape survived three separate fix rounds (#1938, #1993, #2001). Run both. (The .test. exclusion is deliberate and narrow: realRender.test.ts legitimately imports both the en and de budget.json bundles directly and builds a reportT per locale, because asserting per-locale layout requires it.)
Locale-decoupling invariant β the class is closed (#1938, #1993, #2001). Three rounds moved the last generation-time strings onto ReportContentLabels, each populated via reportT in buildReportContent alongside the other entries: the page header (#1938), the page footer (#1993), and finally the cover-letter Reference: / Subject: prefixes plus the *N skip-note reasons (#2001). With #2001 the TFunction parameter itself is gone from reportPdf/*, so the contract is no longer merely unviolated β it is unreachable from any call site. The general rule all three instantiate: artifact content resolves through reportT/reportFormatters; only edit affordances use the interface t. The process lesson is worth as much as the fix: each of the three rounds left the next instance to be found by review, because the call sites were being fixed one at a time while the channel that made them possible stayed open. When a hazard is enforced only by convention, remove the channel, not the individual call.
Does this exact string appear in the exported PDF? If yes it follows the report language wherever it captions that data (including responsive/mobile variants of the same caption β otherwise the language flips at a breakpoint). Screen-reader affordance sentences (ariaLabel, resetAriaLabel) are wholly UI language. Never splice a report-language noun into a UI-language sentence.
Worked cases, so the boundary is not re-litigated:
-
labels.usageis a PDFtableHeaderβ report language everywhere it captions that data, including the responsive mobile card. Sourcing the mobile caption from the interfacetmade the language flip at a breakpoint. -
labels.attachmentsNoteis not exported (there is no attachments column in the PDF) β but its desktop header already reads fromcontent.labels, so the mobile variant must match or desktop and mobile split. - Cover-letter
senderLabel/recipientLabel/bodyLabelstay UI language becausecoverLetterPdfrenders those blocks with no label prefix at all β they are genuinely unexported. (Reference:andSubject:do print, so since #2001 they live onReportContentLabelsascoverLetterReferenceLabel/coverLetterSubjectLabel, resolved fromsourceReports.coverLetter.*viareportT. Note these coexist with the UI-languagesourceReports.editable.referenceLabel/subjectLabelthat caption the same two fields inReportContentEditor.tsxβ two labels for one field is correct here, and is the discriminator working as intended: the editor's is an edit affordance, the PDF's is artifact content.) The cover-letter card is therefore not a precedent for putting editable-field labels on the interfacet.
An editable content model sits between the API response and the pdfmake builders. The layer order is load-bearing:
SourceReportResponse
--buildReportContent(report, includedIds, useCase, reportT, reportFormatters, opts)--> ReportContent (layer 1: baseline)
--applyAiContent(content, aiContent | null)--> ReportContent (layer 2: AI, optional)
--applyOverrides(content, overrides)--> ReportContent (layer 3: user edits)
--generateReportPdf(...)--> Blob
-
buildReportContentowns all text derivation, includingincludedTotaland per-status subtotals.merge.ts/overviewPdf.ts/coverLetterPdf.tsare pure layout with zero derivation. -
applyAiContentfolds AI output into the baseline, not into the overrides map. This is deliberate: overrides carry "the user edited this" semantics (the edited-dot indicator, per-field reset, and the discard-confirm guard). AI text placed inoverrideswould mark every generated field as user-edited, offer a reset that reverts to non-AI text, and be silently wiped by the step 1β4 change guard. Empty strings from the generator fall back to the baseline value rather than blanking a field. -
applyOverridesis pure and ignores unknown keys. Override keys are enumerated inreportContent/overrideKeys.ts:coverLetter.{sender,recipient,reference,subject,body,signature}androw.<invoiceId>.usageText.coverLetter.signatureis the one derived field, with a three-way precedence that must be preserved: an explicitsignatureoverride always wins; otherwise, if and only ifsenderwas overridden, the signature is re-derived from the first line of the new sender; otherwise the baseline signature stands. (The re-derivation is gated on the sender actually having changed β an unconditional recompute would overwrite a baseline signature thatbuildReportContentderived differently.) -
Translated labels live in the model β a consumer must never re-derive them.
ReportContentLabelsis built once withreportTinbuildReportContentand consumed by bothoverviewPdf.ts(the PDF) andReportContentEditor.tsx(the on-screen<th>s, mobile card captions, and column-toggle text). One translated string set per report is what makes a second consumer unable to drift. Any directt()call in a consumer ofReportContentsilently reintroduces the preview/export locale mismatch β it resolves against the interface bundle while everything around it resolves againstreportT. When adding a consumer, the first question is whichtit receives; when adding a label, it goes onReportContentLabels, not into the consumer.- Corollary: because
ReportContentLabelsisreportT-derived and not user-editable, changing a label's translation (e.g. shortening a DE column header to fit a fixed width, #1937) is safe and lands in both surfaces at once β but it must be identical in both by design, so neither surface may hold its own copy. -
Nothing downstream is re-translated independently any more (#2001). Every string the PDF prints comes from
ReportContentLabels: page header (#1938), page footer (#1993), and finally the cover-letterReference:/Subject:prefixes and the*Nskip-note reasons (#2001). Because theTFunctionparameter is gone from all threereportPdf/*builders (invariant 5), this bullet can no longer regress by a caller passing the wrongtβ only by someone re-introducing a parameter. -
Value-keyed label sets must be keyed by the union, not indexed with a widened
string. The*Nskip-note reasons are the one label chosen by a runtime value rather than fixed at the call site:labels.skipReasonLabelsis aRecord<ReportSkipReason, string>, andReportSkipReasonis declared once inreportContent/types.tsand reused by every declaration that carries a skip reason (SkippedDocument.reason, theMap<string, ReportSkipReason[]>threaded frommerge.tsintobuildOverviewContent). That is what makes adding a skip reason a compile error at the single population site inbuildReportContent.ts. The shape to avoid β caught in the PR #2007 review β is a widenedMap<string, string[]>re-narrowed at the consumer withlabels.skipReasonLabels[reason as 'a' | 'b'] ?? reason: it type-checks, adds no compile error when the union grows, and the dead-looking??fallback then prints the raw identifier into a bank-facing PDF. Apply the same rule to any future value-keyed label set.
- Corollary: because
-
ReportContentRow.statusis the raw key (feeding theBadgevariant map);statusTextis the report-language label. Both fields are needed β a consumer that renders the badge's label from its ownt()is the same UI-locale leak in miniature. -
Two independent references by design:
coverLetter.reference(editable) vssourceInfo.referenceText(the overview block's read-only Reference line). Both baseline offreport.source.reference; editing the letter does not change the table.
AI generation is server-side and stateless (POST /api/source-reports/generate-content, story #1901). The server re-fetches the report from the database rather than trusting client-supplied invoice data; the client sends only selection (includedInvoiceIds, excludedLineIds), report type, sourceId, and language. Nothing is persisted β the response exists only to seed layer 2. It reuses the single LLM gateway (server/src/services/llmGateway/, renamed from budgetExtraction/ in #1917 once it stopped being budget-specific) rather than introducing a second provider path; the system prompt treats all invoice-derived text as untrusted data and forbids acting on instructions embedded in it.
Monetary-unit invariant across the AI seam. Report amounts (allocatedAmount, totalAmount, allocatedPortion) are major currency units rounded to 2 dp, never minor units/cents. Any value handed to the prompt builder must already be in that convention and must be rendered without a further scale factor. The included total shown in the letter and the total shown in the overview table are the same number, so the server's total must mirror applyLineExclusions + buildReportContent: round per invoice (Math.round((allocatedAmount β excludedPortion) * 100) / 100), then sum, then round the sum once more to shed float-accumulation dust. The per-invoice amounts handed to the generator must be the same exclusion-adjusted figures that the total is built from β sending raw allocatedAmount alongside an exclusion-adjusted total gives the model a set of parts that does not add up to the stated whole. Generalizable rule: when a monetary value crosses a module boundary, its unit belongs in the type's JSDoc, and a test must assert the rendered string, not just the numeric field.
The overview table carries two structurally different kinds of note, and conflating them is the mistake B4 above records. They are now separated:
| Kind | Marker in the cell | Cardinality | Built by | Rendered by |
|---|---|---|---|---|
| Skipped-document note |
*N, numbered, referenced by the owner row |
one per skipped document (row-owned) |
overviewPdf.ts at generation time |
overviewPdf.ts (generation-time data, not in ReportContent) |
Legend entry (content.footnotes[]) |
a repeated inline word label in the Allocated Amount cell β partial, less deposit β not a symbol or number |
at most one per flag type for the whole document | buildReportContent.ts |
overviewPdf.ts legend block; ReportContentEditor on screen |
content.footnotes[] entries are document-level legend entries, not per-row footnotes. Each entry is one deduplicated sentence per flag type across all rows in the report. buildReportContent.ts accumulates splitInvoiceIds / depositReducedInvoiceIds as Set<string> while building rows, then pushes at most one entry per flag β gated on set.size > 0, never per row:
if (splitInvoiceIds.size > 0) footnotes.push({ id: 'split', marker: splitInlineLabel, text: splitFootnote })
if (depositReducedInvoiceIds.size > 0) footnotes.push({ id: 'depositReduced', marker: depositReducedInlineLabel, text: depositReducedFootnote })
So content.footnotes.length is bounded by the number of flag types (currently 2: split, depositReduced), regardless of whether one row or forty carry the flag. Consequences that follow from this and must be preserved:
-
markeris a label, not an identifier. It is the same human-readable word (partial/less deposit, report-language) that the row cell prints inline, and it is what the reader matches on.id('split'/'depositReduced') is the stable machine key; nothing keys off the marker string. -
The rowβlegend link is by repetition, not by reference. Rows carry the boolean flags
isSplit/isDepositReduced;overviewPdf.tsappends the corresponding inline label to the Allocated Amount cell. No row stores a footnote id, index, or number, and none should β that would reintroduce a second numbering namespace alongside*N(B4). -
The two note kinds share the legend block but not a numbering scheme.
overviewPdf.tsemits the*Nskip notes first, then thereportContent.footnotes[]entries. The*Ncounter must never be extended to cover legend entries. -
A flag type is added in exactly one place. New annotation β a new
Set+ a newsize > 0push inbuildReportContent.ts, a new boolean onReportContentRow, and a new inline label inoverviewPdf.ts. Emitting one legend entry per flagged row instead is the regression to guard against; assertfootnotes.length(not>= 1) on a fixture where several rows share a flag. -
Legend text is report-language and comes from
reportTinbuildReportContent, like every other string inReportContentβ it prints in the exported PDF, so the export test in the report-language addendum applies.
Banks and subsidy portals cap upload size, so the wizard's Settings step takes an optional maximum file size. When attachments are on and the limit parses as valid (partsMode = attachDocuments && limitParse.status === 'valid' in ReportWizardPage.tsx), the report is generated as one or more parts. Otherwise the single-PDF path (generateReportPdf in merge.ts) runs exactly as before; it is not modified by this story.
- Part 1 = cover letter (with a multi-part notice when there is more than one part) + overview table + as many attachments as fit.
- Parts 2..N = a one-page continuation letter + attachments only. The report pages are never repeated.
- Attachments keep report order (invoice order, then document order within an invoice) across all parts.
The six new modules are listed in the module table above. Their dependency shape is deliberate:
| Module | Imports | Why it is shaped this way |
|---|---|---|
partPlan.ts |
nothing | Input parsing and planning are pure arithmetic over { key, size }[], unit-testable without a render, and safe for the page to import statically (parseMaxFileSizeInput). |
partNaming.ts |
nothing | File names are needed by the page before and after generation; pure and statically importable. |
attachments.ts |
loader.ts, paperlessApi.ts, types |
The only module that fetches. Fills a caller-owned cache, so the cache's lifetime is the page's decision, not the library's. |
continuationLetterPdf.ts |
pdfmake types, ReportContent type |
Pure builder like coverLetterPdf.ts; receives every string pre-resolved (ContinuationLetterText). The invoice list is always a bulleted list (ul), whatever its length, so the letter's layout does not change shape at a line-count threshold. |
docDefinition.ts |
shared.ts (header/footer builders), pageGeometry.ts, types |
The one document definition both orchestrators render with. A separate module because pageGeometry.ts cannot host it: shared.ts imports pageGeometry.ts, so pageGeometry.ts importing shared.ts would be a cycle. |
merge.ts |
loader, coverLetterPdf, overviewPdf, docDefinition, paperlessApi.ts, types |
Single-PDF orchestration, unchanged in behaviour by #2161. Takes its document definition from docDefinition.ts and no longer defines PDF_DEFAULT_STYLE (it re-exports it from pageGeometry.ts). |
parts.ts |
loader, docDefinition, the three builders, partPlan, types |
Orchestration only. Imports no value from merge.ts: the multi-PDF path is a second orchestrator beside the single-PDF one, not a wrapper around it. Re-exports attachments.ts. |
coverLetterPdf.ts gained one optional parameter, { partsNotice?: string }, printed as the last body paragraph. It is pre-resolved text, so it is not a locale channel.
Rule: one document definition, two orchestrators. merge.ts and parts.ts both render text pages with pdfMake.createPdf(buildReportDocDefinition(content, reportContent)), and neither builds a document definition of its own. docDefinition.ts is the only place that sets page size, margins, header (none on page 1), footer, defaultStyle and styles. The values it assembles stay where they already lived: geometry, PDF_STYLES and PDF_DEFAULT_STYLE in pageGeometry.ts (PDF_DEFAULT_STYLE moved there from merge.ts in #2161; merge.ts re-exports it), and the header/footer builders in shared.ts. A new render setting goes into buildReportDocDefinition, never into one orchestrator's createPdf call. A copy drifts silently: a part-1 PDF that differs typographically from the single-PDF report is a valid PDF and fails no assertion.
-
reportPdf/index.tsis unchanged. It re-exports nothing from the multi-PDF modules. -
parts.jsis reached only through dynamicimport('../../lib/reportPdf/parts.js'), at exactly two sites inpages/ReportWizardPage/useReportParts.ts(startSizingandensureParts).useReportParts.tshas no other static imports beyond React and types. -
partPlan.tsandpartNaming.tsare the only multi-PDF modules the page imports statically. Both have zero imports, so this is safe.
This contract does not protect the ~2MB pdfmake/pdf-lib cost; loader.ts already does that (see the lazy-loading contract above), and parts.ts, like merge.ts, reaches both libraries only through loadPdfLibs(). Its two reasons are narrower:
-
Test isolation. The existing wizard page tests mock
reportPdf/index.jsandpaperlessApi.jswith only some of their exports. Reachingparts.jsonly throughimport()keeps it, and its module graph, out of those tests' module graph, so the partial mocks never have to grow the exportsparts.jsandattachments.jsneed. Each multi-PDF test mocksparts.jsexplicitly, withjest.unstable_mockModule('β¦/reportPdf/parts.js', β¦). - Load on demand (a side effect, not the driver). The multi-PDF pipeline code is fetched only when a user actually sets a size limit.
It is not about keeping paperlessApi off the page, despite what earlier code comments claimed: ReportWizardPage.tsx already imports paperlessApi statically (through reportPdf/index.ts β merge.ts / sinks.ts), and attachments.ts imports it as well.
Guard (must return nothing; type-only imports and tests are allowed):
grep -rnE "from '[^']*/parts\.js'" client/src --include=*.ts --include=*.tsx \
| grep -v "import type" | grep -v '\.test\.'Continuation-letter text, the multi-part notice and Paperless part titles are report-language artifact content (the export test applies). Their interpolation values (part number, part count) are only known after planning, and planning happens long after buildReportContent has run. A plain string field cannot hold them. A TFunction parameter on reportPdf/* is the channel invariant 5 closed.
The resolution is closures. ReportContentPartTexts (reportContent/types.ts) holds pre-resolved strings plus (part, total) => string-shaped functions. buildReportContent builds them over reportT when called with includePartTexts: true, and attaches them as the optional ReportContent.partTexts. Downstream code sees (n: number) => string, never a TFunction.
-
Invariant 5 still holds. No module under
reportPdf/accepts, imports or references aTFunction. The invariant-5 grep guard returns nothing on #2161. -
Invariants 1β4 hold for the same reason the labels do. The closures are built in exactly one place (
buildReportContent), over the onereportTthe page boundary chose. -
The type does not distinguish a closure over
reportTfrom one over the interfacet. As withReportContentLabels, the protection is thatbuildReportContentis the only producer. - Never build a
partTextsclosure anywhere else. -
Opt-in and checked. The wizard always passes
includePartTexts: true.generateReportPartsthrows ifpartTextsis absent instead of falling back to a hardcoded string. The single-PDF path ignores the field. -
Consumers:
parts.ts(continuation letter, multi-part notice in part 1 or as a standalone line when there is no cover letter) andReportWizardPage.tsx(paperlessTitlefor each uploaded part). The Paperless title is artifact metadata, so it follows the report language, not the UI. -
Closures survive the content layers by reference.
applyAiContent/applyOverridesshallow-spreadReportContent, sopartTextspasses through untouched. Consequently aReportContentbuilt withincludePartTextsis not JSON-serialisable orstructuredClone-able. Never round-trip it through either; a clone silently drops the functions, and the result is a thrown error at generation time. -
identifieris captured at build time fromtableTitle,sourceInfo.sourceNameandgeneratedAtText. That is sound only because none of the three is an override key or an AI-content field. If one ever becomes editable,identifiermust be derived from the post-override content instead, or the continuation letters will name the report differently from part 1.
General rule (extends the "new ReportContentLabels field, never a new parameter" rule in the Decision section): a generation-time string with fixed text is a ReportContentLabels field. A generation-time string whose interpolation values are only known after content is built is a closure on a reportT-built field of ReportContent. Neither is ever a parameter.
-
What is measured. Each part's size is computed from the actual bytes that will be merged.
attachments.tsfetches each linked document through the pre-existingGET /api/paperless/documents/:id/previewproxy, the same URLmerge.tsuses, and takesArrayBuffer.byteLength. - Why not a size endpoint or Paperless metadata. Paperless's size metadata describes the original/archive files, which need not be the bytes the preview proxy serves. The bytes are needed anyway to merge. So a size endpoint would add API surface to produce a number that is less accurate than one the client gets for free. No endpoint is added, and the ADR's "no new API endpoint" statement stands.
-
Validation at sizing time. Every fetched document is parsed once with
PDFDocument.load. Invalid documents becomefootnoteInvalidPdfskip notes before planning, so the planner never budgets for a document that will not be merged. -
The cache.
AttachmentCacheis aMap<documentId, { status: 'ok', bytes } | { status: 'invalid' }>. It is owned byuseReportPartsin a ref for the lifetime of the mounted wizard. It is scoped bycacheScope = ${useCase}:${sourceId}: a scope change discards it wholesale. It is keyed bydocumentId, not by(invoiceId, documentId), so a document linked to two invoices is fetched once. -
What the cache does not hold. Fetch failures are not cached; a new sizing round retries them. The user can "continue without" failed documents (
acceptedFailures, passed asskipFetch), which turns them intofootnoteFetchFailedskip notes for the current scope. - When sizing runs. Sizing runs on the Settings β Actions transition, with cancellable progress throttled to 500 ms. Generation then reuses the cached bytes, so each document crosses the wire once per wizard session regardless of how many times the user regenerates.
Estimate. Overheads are measured by real renders, not computed:
-
Single-part attempt first. The part-1 text pages are rendered without a multi-part notice. If those pages plus all attachments fit within the limit, the report is one part, and it is exactly the single-part document. The worst-case estimate below is never applied to a report that fits in one PDF.
-
Fallback, only if one part does not fit:
- Part 1: the part-1 text pages rendered with the multi-part notice at a worst-case part count of 99.
- Continuation: a continuation letter rendered as part 99 of 99, listing every attachment's invoice.
Both are conservative upper bounds.
Plan (planReportParts). An ordered greedy pass over the attachments in report order:
- An item joins the open part if
estimate + size <= limit; otherwise the part closes. - An item that does not fit even into an empty continuation part becomes an oversized singleton: its own part, with
oversizedItemKeyset. - If part 1's overhead alone exceeds the limit, part 1 is closed with no attachments and
reportExceedsLimitis set.
Order is never traded for packing density. A reader follows the overview table into the attachments, and reordering to save a part would break that correspondence.
Verify. A merged part's size is not the sum of its inputs (pdf-lib re-serialises, xref and object overhead vary). So every planned part is rendered and measured, and then:
- Find the first part that is over the limit and has a movable item (part 1 with at least one attachment, or a continuation part with at least two).
- Freeze every part before it.
- Remove that part's last item (
shiftLastItemβfixedPartCounts), then greedily re-plan the remainder.
The loop is bounded at the initial part count + 1 passes. Renders are memoised by (index, total, keys) within one generation. Whatever is still over the limit after the bound is reported, never looped on and never an error.
Warnings. ReportPartWarning has three mutually exclusive kinds per part:
-
oversizedAttachment: one document is larger than the limit on its own. -
reportExceedsLimit: the report pages alone exceed it. -
partOverLimit: the verification bound was hit.
Generation always yields at least one part.
-
Input. The limit is entered in MB, accepts
.or,as the decimal separator, allows at most one decimal place, and must be at least 1. Empty means "no split".parseMaxFileSizeInputconverts withBYTES_PER_MB = 1_000_000(partPlan.ts). The user's reference point is a portal or mail limit, and those are stated in decimal megabytes. -
Display. The limit and every size shown next to it (part sizes, oversized-attachment sizes, warnings) render through the new
formatFileSizeDecimal(client/src/lib/formatters.ts): SI units (B,kB,MB), one decimal place, with the unit chosen after rounding so 999,999 B reads "1.0 MB", not "1,000.0 kB". -
formatFileSizeis unchanged. It stays binary (1024,KB) for every other file-size surface in the app. Never mix the two on one surface: a 10 MB limit displayed beside a binary-formatted 9.8 MB (decimal) part would read as "9.3 MB" and look wrong. -
Locale. Sizes are UI text, not artifact content (none prints in the PDF). The wizard therefore calls
formatFileSizeDecimalwith the UI locale (toBcp47Locale(resolvedLocale)), not the report locale.
-
File names are language-neutral and come from
partNaming.ts. A single part is${useCase}-${slug}-${isoDate}.pdf, unchanged from before. Multiple parts areβ¦-part-${k}-of-${N}.pdf, withkzero-padded to the width ofNonceN >= 10. -
Paperless upload is one
POST /api/paperless/documentsper part, sequential, with per-part status, and a retry that re-sends only the failed parts. Each part is titled withpartTexts.paperlessTitle. -
Preview-equals-export holds per part. One
GeneratedReportPartsresult feeds the per-part preview, every download and every upload.useReportPartskeys it by aGenerationKeyholding the identity of every input.ensurePartsreturns the stored result when the key matches, joins an in-flight generation for the same key, and publishes state only for the still-current key (epoch guard). Changing any input marks the resultstalerather than exporting it, and a failed regeneration reportserrorover a stale result. This is the B5 rule applied to parts.
Easier:
- The preview-equals-download invariant is structural, not enforced by convention: one
Blobfeeds the iframe, the download anchor, and the Paperless upload. - No server-side PDF surface at all β no Chromium, no render library, no artifact cache, no temp-file lifecycle, no TTL/cleanup job, and no PDF CVE-patching burden on the container image.
- No server CPU or memory cost. A large merge is a transient in the user's own tab and cannot degrade the API for the other users of the single-process container.
- Option toggles regenerate locally β no round trip per toggle, and no report bytes crossing the wire twice.
- Dependency policy stays clean: pure JS throughout, exact-pinned, no install scripts, no ARM64-emulation hazard.
- A reusable client-side document pipeline now exists (
reportPdf/) with clear seams, available to future export features.
More difficult:
-
~2MB of lazily-loaded JavaScript, dominated by the base64 Roboto vfs. It is kept out of the initial bundle by a fragile, easily-broken invariant (
import typeonly outsideloader.ts) that must be re-verified with the grep guard on every change. A single careless static import regresses initial load for every page in the app. -
Merge memory is client-side and unbounded by design. Every appendix document is fetched fully into memory and parsed by pdf-lib. The practical ceiling is the browser tab, with the server's 50MB multipart cap (
server/src/app.ts) bounding only the upload of the finished report β a merged report larger than that cannot be pushed back into Paperless even though it generated fine. Large-claim reports (dozens of multi-megabyte invoices) are the risk case; document fetching is currently serial, which is slow but keeps peak memory lower than a parallel fetch would. Since #2161 a user-set maximum file size splits the report into parts that are uploaded one by one, so a limit at or below the server cap makes every part uploadable except anoversizedAttachmentorreportExceedsLimitpart. The flip side is that in parts mode the attachment cache keeps every fetched document's bytes for the whole wizard session, not just for one generation: peak memory is still bounded by the tab, but it is held longer. - Typography is Roboto-only. Any other family requires shipping and registering additional font files, adding further weight to the async chunk. Font choice is therefore effectively frozen for generated documents, independent of the app's on-screen type (Style Guide, ux-designer's domain).
- Generation quality depends on the client. Old or memory-constrained browsers, and mobile tabs in particular, may struggle with a large merge. There is no server-side fallback path.
- Two libraries to keep current rather than one, and their combined 27-package subtree must be re-verified as pure-JS on every upgrade (Dependabot handles the bumps).
- Testing costs more. Mocked unit tests are actively misleading for this pipeline; real-render tests for both locales are mandatory, and they are slower than the mocked equivalents. This is a deliberate trade: the four defects in PR #1887 that a real render would have caught were each cheaper to prevent than to find.
-
Locale/config plumbing is manual. Because the builders are pure, every locale- or config-dependent value must be explicitly threaded in; forgetting to do so fails silently with
en-US+EURdefaults rather than raising.
-
ADR-004 β Webpack
splitChunksis what makes the async chunk possible; the no-native-binaries dependency policy originates here. - ADR-015 β the document proxy the appendix fetches through, and the archive the finished report is uploaded back into.
-
ADR-021 β the i18n configuration whose lack of a
formatfunction caused finding B3. - ADR-013, ADR-017, ADR-033 β precedent for documenting client rendering/library choices.
| Date | Deviation | Resolution |
|---|---|---|
| 2026-08-03 | Finding B4's generalized rule read "every footnote is referenced from the row that owns it". That was true when the split / deposit-reduced markers were inline symbols (β , β‘) printed in the cell and paired with a note per owning row. Since issue #1965 the markers are repeated inline word labels (partial, less deposit) and content.footnotes[] holds document-level, deduplicated legend entries β at most one sentence per flag type for the whole report, gated on Set.size > 0 in buildReportContent.ts. The wiki still described per-row footnotes. |
Wiki was outdated; code is correct. B4's rule reworded to distinguish numbered row-owned notes (*N) from document-level legend entries, and to state explicitly that "one note per owning row" applies only to the former. Added the "legend is document-level and deduplicated" addendum recording the cardinality bound, the label-not-identifier property, and the rowβlegend-by-repetition link. Intro bullet 2 updated to name the legend block. |
| 2026-08-04 |
Minimum-bar rule #1 was wrong. It read "Assert table._minWidth <= 515.28 for both en and de". _minWidth is the width of the widest unbreakable token, not the laid-out width, so the assertion fails on correct code that relies on wordBreak: 'break-all' and passes on incorrect code whose '*' column overflows via starMaxMin. It was a mis-transcription of finding B2's diagnostic (where _minWidth legitimately located the bug) into a correctness check. Owed since the #1876β#1879 round and tracked as issue #1914. |
Wiki was wrong; code is correct. Rule #1 replaced with two render-derived assertions: max(node.positions[].horizontalRatio) <= 1 (verified against DocumentContext.js:490) for overflow, and tableOffsetsTotal(cols) + sum(widths[i]._calcWidth) <= printableWidth() for the table box. Added an explicit "do not assert _minWidth" paragraph with the reason, and a pointer from B2's narrative clarifying diagnostic-vs-check. |
| 2026-08-04 |
Module table drifted twice. It listed shared.ts as owning "PDF-local formatters" (deleted in the #1879 review round 2, precisely because a PDF-local formatter is a second formatter implementation bound to a different locale β finding B3) and "table layout constants" (moved to pageGeometry.ts in #1939), and omitted pageGeometry.ts and index.ts entirely. Tracked as issue #1914. |
Wiki was outdated; code is correct. Rewrote the module table: added pageGeometry.ts as sole owner of the pt coordinate system and index.ts as the barrel, restated shared.ts's role without formatters or geometry constants, and added notes recording why the formatters were removed and why tokens.css is not the home for pt geometry. |
| 2026-08-04 |
The override-key list was stale. It read row.<invoiceId>.{usageText,attachmentsNote} β attachmentsNote stopped being an override key in #1959 β and omitted coverLetter.signature, added later with a three-way precedence rule (explicit override > re-derive when sender changed > baseline) that the flat sentence "recomputes coverLetter.signature from sender" misdescribed as unconditional. |
Wiki was outdated; code is correct. Corrected the key list against reportContent/overrideKeys.ts and wrote out the signature precedence explicitly, including why the re-derivation is gated on senderChanged. Source of truth: client/src/lib/reportContent/applyOverrides.ts. |
| 2026-08-04 |
Three empirically-established constraint sets were never recorded, so each was rediscovered at review cost: the dontBreakRows-on-table-not-layout placement trap (a wrong placement type-checks, produces a byte-identical PDF, and passes a config-shape unit test); the silent-drop-of-over-tall-unbreakable-rows defect with its "bound the rendered cell, not a source field" rule and two threshold-free detection recipes; and the fixed 6-or-7 column-count constraint that makes PDF column-visibility toggles a re-measurement story. Owed since #1929/#1959. |
Added "Unbreakable rows are silently dropped" as a section, added minimum-bar rule #2 (height bound) pointing at it, and added four bullets to the pdfmake 0.3.x constraint list (content-vs-occupied widths and usableColumnWidth, '*' never shrinking below its longest token plus the resulting all-fixed-width consequence, dontBreakRows placement, and _calcWidth as the verification lever). |
| 2026-08-04 |
The locale-decoupling contract was described but not stated as enforceable invariants (architect finding L5, PR #1903 review; tracked as issue #1914). The addendum recorded the two injected artifacts and the two properties to preserve, but not the injection-only contract as numbered invariants, no grep guard, and no record of the outstanding merge.ts footer violation (buildPageFooter(t('sourceReports.table.pageLabel')) β interface t, so an English report gets a Seite 2 / 5 footer under a German UI). |
Added an "injection is the only channel" subsection with four numbered invariants, the reason the failure mode is silent (nothing in the type system distinguishes reportT from the interface t), a verified grep guard, and the known open violation with its fix shape. Promoted the export-test discriminator to its own subsection with the three worked cases. The merge.ts footer violation is a code defect and is NOT fixed by this pass β flagged for a follow-up issue. |
| 2026-08-04 |
merge.ts page-footer had a fragile caller contract. generateReportPdf accepts a TFunction t parameter, and the footer called buildPageFooter(t('sourceReports.table.pageLabel')). The only production caller (ReportWizardPage.tsx) always passed reportT, so no mislocalised footer was ever observable; but t and reportT live one character apart in the same component, so the contract required every future caller to pass the right one β with no type-system enforcement and no grep guard catching a t()-on-injected-param call. The 2026-08-04 locale-decoupling row above documented this as a known open violation; it was re-characterised as fragility rather than a live defect in the PR #2000 review. |
Hardened in PR #2000 (issue #1993): added pageLabel to ReportContentLabels, populated it via reportT in buildReportContent.ts, and replaced the parameter-dependent t() call with reportContent.labels.pageLabel in merge.ts. The footer now follows the same injection-only path as every other ReportContentLabels field, making the locale contract impossible to violate from the call site. Added regression assertion in merge.test.ts and updated all seven test files that construct ReportContentLabels literals. |
| 2026-08-04 |
LLM gateway path references went stale when #1917 renamed server/src/services/budgetExtraction/ to server/src/services/llmGateway/ (the directory stopped being budget-specific once report-content generation started using it). Live references existed on this page (1), API-Contract.md (2), and Security-Audit.md (2). |
Updated the references on this page, API-Contract.md, and Security-Audit.md in the same PR as the rename, noting the old name once for searchability. Historical Deviation Log rows keep the then-current path deliberately. (Security-Audit.md is security-engineer-owned; the path-only update was approved by that agent as part of this PR.) |
| 2026-08-04 |
The content-model addendum understated the labels rule (post-#1909). It said labels live in the model and "when adding a consumer, check which t it receives", but did not state that a direct t() call in a ReportContent consumer *silently reintroduces the preview/export locale mismatch, nor name the two consumers, nor record that ReportContentRow.status/statusText both exist for the same reason. |
Expanded the labels bullet to name both consumers (overviewPdf.ts, ReportContentEditor.tsx), state the direct-t() failure explicitly, record the not-user-editable corollary that makes translation changes safe (#1937), and list the sites still translating independently β including the merge.ts footer exception. Added the status/statusText bullet. |
| 2026-08-05 |
The locale-decoupling contract was documented at its pre-#2001 strength in five places, all falsified by PR #2007 (issue #2001) removing the TFunction parameter from reportPdf/*. Line 82 quoted the builder signature as (reportContent, β¦, t: TFunction) and described a "residual t" that "is always reportT" β instructing a future contributor to add one back. The "why this is a contract" paragraph ended "nothing in the type system distinguishes reportT from the interface t β¦ so the only defences are this contract, review, and per-locale real-render tests", which had become false for reportPdf/*. The invariant list stopped at 4 and its single grep guard (useTranslation|/i18n/|Intl.|toLocale) structurally cannot match a t: TFunction parameter β which is why this leak shape survived three fix rounds. The content-model addendum still listed overviewPdf.ts's *N skip reasons and coverLetterPdf.ts lines 46/54 as "still re-translated independently", a now-empty list. The export-test worked case said Reference:/Subject: print "from separate sourceReports.coverLetter.* keys" without noting they had moved onto ReportContentLabels. |
Wiki was outdated; code is correct. Rewrote line 82 to state that buildReportContent is the pipeline's only TFunction entry point and that a new generation-time string means a new ReportContentLabels field, never a new parameter. Added invariant 5 (no module under reportPdf/ accepts, imports, or references a TFunction) with its own grep guard, plus an explicit note that the invariant-1 guard cannot see an invariant-5 violation and both must be run. Re-scoped the "nothing in the type system distinguishes" sentence to "wherever a TFunction is in scope at all", naming buildReportContent as that single remaining surface. Generalised the #1993 footer paragraph into "the class is closed (#1938, #1993, #2001)" with the process lesson: when a hazard is convention-enforced, remove the channel, not the individual call. Replaced the "still re-translated independently" bullet with the closed-class statement, and added a value-keyed label set rule recording the Record<ReportSkipReason, string> design and the widen-then-as-narrow anti-pattern caught in the PR #2007 review. Corrected the export-test worked case to name coverLetterReferenceLabel / coverLetterSubjectLabel and to record that their coexistence with the UI-language sourceReports.editable.* labels is the discriminator working as intended, not a duplicate. |
| 2026-08-05 |
Minimum-bar rule #1 was wrong a second time β its replacement test is vacuous (3rd correction, PR #2008). The 2026-08-04 row above swapped the banned _minWidth form for max(node.positions[].horizontalRatio) <= 1 and called it "the only assertion that actually proves nothing ran off the page". Issue #2003 implemented that guidance, and mutation testing in the PR #2008 review proved the production-content assertion cannot fail: horizontalRatio is recorded at the left edge of each text line and before the line is placed (ElementWriter.js:32), so it is a cell-origin bound, not a content-extent check. In the production overview table β all-fixed widths summing exactly to printableWidth() β every line starts inside the margins regardless of token length, so removing wordBreak: 'break-all' from a cell with a wide token left the assertion green. The same row also banned _minWidth outright citing wordBreak: 'break-all' false-positives; that rationale conflated the table-level sum with the per-cell measurement and is empirically false for the latter. |
Wiki was wrong; code is correct. Rule #1's content-extent check is now per-cell _minWidth <= widths[i]._calcWidth after a real render. _minWidth is computed after TextBreaker applies the cell's wordBreak, so it is falsifiable and discriminating: on the 69.28pt Usage column a wide compound token yields 33.54pt with break-all (passes) vs 266.16pt without it (fails), and plain prose yields 33.54pt (passes). maxHorizontalRatio was not deleted β it was re-scoped and relabelled as the narrower table-box positioning check (does any element's origin land past the right margin), which it does perform correctly; the existing widths: [600, 50] revert-test validating it is legitimate for that purpose and stays. Added an explicit vacuity warning against asserting it on production content. Split the _minWidth ban into table-level (still banned, with the starMaxMin reason) vs per-cell (the correct check), corrected finding B2's cross-reference to say its quoted _minWidth figures are table-level sums, and updated the implementing-test pointer to describe both assertions. 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.
|
| 2026-08-05 |
A documented constraint became a false prohibition (#1973, PR #2010). The "Geometry constraint that blocks a feature" section stated that the PDF's column count is fixed at 6 or 7 and that making the wizard's column-visibility toggles affect the PDF "would widen the Usage column and invalidate the measured ceiling β that is a re-measurement story, not a UI change", closing with "the toggles introduced in #1959 are deliberately preview-only local state". PR #2010 falsified every clause: overviewPdf.ts now renders any of 96 legal column subsets. Two adjacent statements were stale in the same section: the bound's call site was quoted as packUsageCellRows(segments, MAX_SAFE_USAGE_CHUNK_CHARS) (now the per-subset usageChunkChars), and the "Related sharp edge" paragraph claimed packUsageCellRows hangs on maxChars <= 0 when a prior fix had already made it throw. Found in the product-architect review of PR #2010, which touched no wiki file. |
Wiki was wrong; code is correct. Replaced the section with "Column geometry is a computed engine, not two pinned shapes", recording (a) the single-absorber algorithm (usage -> else vendor -> else none) and why '*' and proportional slack-sharing were both rejected, (b) the load-bearing fact that had existed only in a function doc comment β USAGE_WIDTH_7COL (138.28pt) is provably the narrowest Usage width across all 96 subsets, because hiding a column both shrinks fixedSum and grows usableColumnWidth(n), so the measured 650-character ceiling is a floor and hiding columns only ever makes it more conservative β which is why no re-measurement was needed, (c) the one-sided usageChunkCharsForWidth clamp and the future-added-column hazard it guards, (d) the residual constraint that adding a column is still a re-measurement story, and (e) which links force a new ReportColumnKey at compile time and which fail soft. Corrected the call-site quote and rewrote the sharp-edge paragraph to record that its own prediction ("it matters the moment it becomes computed") went live and that both functions now throw. Added reportContent/columns.ts to the module structure as the sole derivation of which columns exist, with the reason it lives outside reportPdf/. Process lesson: when a PR removes a limitation, grep the wiki for the limitation's own statement β constraint prose lives in ADR Consequences and "sharp edge" sections that no schema or API-contract diff would ever point you at.
|
| 2026-10-01 |
Module table and builder-signature rule pre-dated the multi-PDF split (#2161). types.ts was listed as holding only ReportPdfOptions / GeneratedReport / SkippedDocument; index.ts was described as "the public surface of the pipeline" although the multi-PDF modules are deliberately kept out of it; merge.ts was described as "the" orchestrator; coverLetterPdf.ts did not mention its new partsNotice option; the five new modules were missing. The rule "a new generation-time string is a new ReportContentLabels field" had no answer for strings whose interpolation values (part number/count) only exist after planning. |
Wiki was outdated; code is correct. Added the five modules to the module table, re-scoped merge.ts / index.ts to the single-PDF path, extended types.ts and coverLetterPdf.ts, and extended the builder-signature rule with the ReportContent.partTexts closure exception. Added the multi-PDF addendum. |
| 2026-10-01 |
Two grep guards documented as "must return nothing" did not. The lazy-loading guard matched realRender.test.ts (and, since #2161, parts.realRender.test.ts), which import PDFDocument from pdf-lib by value to inspect rendered output. The invariant-1 guard has matched buildReportContent.ts's import { I18N_UNION_KEYS } from '../../i18n/unionKeys.js' since #2029, because /i18n/ is a path substring. Neither hit is a violation, but a guard that is always non-empty trains readers to ignore its output, which is how a real hit gets missed. Found while verifying #2161. |
Wiki was outdated; code is correct. Added | grep -v '\.test\.' to the lazy-loading guard and | grep -v "i18n/unionKeys.js" to the invariant-1 guard, each with the reason the exclusion is exact. Both guards (and the invariant-5 guard) re-run against the #2161 branch and return nothing. |
| 2026-10-01 | The merge-memory consequence did not account for parts mode (#2161). It said a merged report larger than the server's 50MB multipart cap cannot be uploaded, which is still true of the single-PDF path, but parts mode now uploads per part and holds every fetched document for the whole wizard session. | Extended the bullet: parts mode makes every part uploadable when the limit is at or below the cap (except an oversizedAttachment / reportExceedsLimit part), at the cost of session-long retention of attachment bytes. |
| 2026-10-01 |
Correction to the #2161 addendum (PR #2167 review), before merge. The first version said the parts.js dynamic import mainly gave the page a type-only dependency and a test seam. It also described the overhead estimate as always the N=99 worst case, and implied PDF_DEFAULT_STYLE would merely be required to be shared. |
Rewrote the lazy-load rationale: (1) it keeps parts.js and its module graph out of the existing page tests' partial mocks of reportPdf/index.js; (2) the pipeline code loads only when a size limit is set. Recorded the PR #2167 fixes: PDF_DEFAULT_STYLE moved to pageGeometry.ts and is shared by merge.ts and parts.ts; planning first tries a single part using the part-1 size without the notice and falls back to the N=99 estimate only if one part does not fit; the continuation letter's invoice list is always bulleted. |
| 2026-10-01 |
The #2161 addendum did not cover docDefinition.ts (PR #2167 round 2, finding 2). The fix commit added client/src/lib/reportPdf/docDefinition.ts (buildReportDocDefinition) as the single owner of the pdfmake document definition used by both merge.ts and parts.ts. The addendum still described the rule as "both import the values from pageGeometry.ts / shared.ts", listed shared and pageGeometry as parts.ts dependencies, and had no merge.ts or docDefinition.ts row. The lazy-load test-isolation reason named only the reportPdf/index.js mock. |
Wiki was outdated; code is correct. Added docDefinition.ts to the module table and to the addendum's dependency table, with the reason it is not part of pageGeometry.ts (shared.ts β pageGeometry.ts would become a cycle). Added a merge.ts dependency row, corrected the parts.ts row, and restated the rule around buildReportDocDefinition. Aligned the lazy-load wording with the code comments: the page tests partially mock both reportPdf/index.js and paperlessApi.js, and loading on demand is a side effect. |