Skip to content

feat(reports): add source report data, mark-claimed, and Paperless upload endpoints - #1885

Merged
steilerDev merged 4 commits into
betafrom
feat/1878-source-report-backend
Jul 29, 2026
Merged

feat(reports): add source report data, mark-claimed, and Paperless upload endpoints#1885
steilerDev merged 4 commits into
betafrom
feat/1878-source-report-backend

Conversation

@steilerDev

Copy link
Copy Markdown
Owner

Summary

  • Adds GET /api/source-reports — generates budget-overview/claim/proof-of-funds reports for a budget source, enriched with Paperless-ngx document metadata (degrades gracefully to null fields if Paperless is unreachable)
  • Adds POST /api/source-reports/mark-claimed — atomically flips a batch of invoices and their eligible deposits to claimed
  • Adds POST /api/paperless/documents — multipart PDF upload proxy into Paperless-ngx's consumption pipeline, returning the async task UUID
  • Allows the pending -> claimed deposit status transition (previously only reachable via paid)

Fixes #1878
Fixes #1884

Test plan

  • Unit tests pass (95%+ coverage) — sourceReportService, sourceReports route, paperlessService.uploadDocument, paperless.ts upload route, invoiceDepositService transition, depositAggregateUtils.computeStatusContributionByInvoice
  • Integration tests pass (Fastify app.inject())
  • 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 backend-developer (Haiku 4.5) noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) noreply@anthropic.com

steilerDev and others added 2 commits July 29, 2026 20:29
…load endpoints

Adds GET /api/source-reports (budget-overview/claim/proof-of-funds report
generation with Paperless-ngx document enrichment), POST
/api/source-reports/mark-claimed (batch invoice + deposit claiming), and
POST /api/paperless/documents (PDF upload proxy to Paperless-ngx's
consumption pipeline). Also allows the pending -> claimed deposit status
transition needed for direct claiming.

Fixes a Map-iteration bug in getSourceReport where document enrichment
iterated a Map's [key, value] tuples instead of .values(), silently
leaving ASN/title fields unpopulated.

Fixes #1878
Fixes #1884

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
… transition

Scenario 47 asserted the pre-#1878 contract (pending -> claimed disallowed);
repurposed to exercise claimed -> pending, which remains disallowed. Added
scenario 47b asserting pending -> claimed now succeeds with both paidDate
and claimedDate auto-set to today.

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

[security-engineer] Security review of PR #1885 (Story #1878, feat/1878-source-report-backend → beta).

Scope reviewed: POST /api/paperless/documents (multipart upload proxy), GET /api/source-reports, POST /api/source-reports/mark-claimed, sourceReportService.ts, paperlessService.uploadDocument, invoiceDepositService.ts (new pending → claimed transition), and the new shared types/error code.

No critical or high findings.

Verified clean

  • SQL injection: sourceReportService.ts uses Drizzle's sql\...`tagged templates with${sourceId}interpolation (parameterized, not string-concatenated) andsql.join(reportInvoiceIds.map(id => sql`${id}`), sql`, `)for theIN (...)clause — also correctly parameterized per-element, not a raw string join. No injection vector in either the main report query or theisSplit` / unallocated-invoices queries.
  • Upload MIME gate ordering: paperless.ts:326 checks file.mimetype !== 'application/pdf' before file.toBuffer() — an oversized/malicious non-PDF part is rejected before its body is buffered into memory, avoiding an easy memory-exhaustion vector via repeated large non-PDF uploads.
  • Upload size cap: global @fastify/multipart fileSize: 50 * 1024 * 1024 cap (app.ts:97) applies to this route (no per-route override), consistent with the rest of the app.
  • SSRF: the upload proxy only ever forwards to fastify.config.paperlessUrl (server-side config, not client input) via requirePaperless() — no client-controlled URL reaches fetch(). Same pattern as the existing GET proxy endpoints.
  • Credential handling: PAPERLESS_API_TOKEN is read from server config only, sent as Authorization: Token <token> to the upstream, never echoed back to the client or logged.
  • Path traversal: file.filename is forwarded only as the multipart filename metadata in the outbound FormData to Paperless (paperlessService.ts:592) — never used to construct a filesystem path on Cornerstone's own server, so no traversal risk on our side.
  • Error responses: upstream failures surface as PAPERLESS_ERROR / PAPERLESS_UNREACHABLE 502s built from response.status + response.statusText only (paperlessService.ts:610-616) — the upstream response body is never included, so no internal Paperless error detail leaks to the client. getStatus() additionally runs error messages through sanitizeErrorMessage() to strip IPs/hostnames.
  • mark-claimed transaction integrity: markInvoicesClaimed validates claimability for all requested invoices before writing any of them (fail-closed — InvoicesNotClaimableError thrown before the write loop), and the read-check-write happens inside a single db.transaction(), which is synchronous/atomic under better-sqlite3 (no TOCTOU window). The 409 details.invoiceIds only echoes back IDs the caller itself submitted — no cross-user data exposure.
  • Authorization model: both new endpoints require only session auth (no requireRole('admin')), consistent with this app's established household-trust model (per prior audits — RBAC is reserved for users.ts/backups.ts; all other household-scoped resources, including invoice/deposit status changes, are member-writable). Not a new gap.
  • Input schemas: GET /api/source-reports and POST /api/source-reports/mark-claimed both use additionalProperties: false JSON schemas with enum/type constraints, consistent with the rest of the codebase.

Informational (non-blocking)

  1. POST /api/paperless/documents trusts the client-declared MIME type, not file content. file.mimetype is the multipart part's Content-Type header, which the client fully controls — a caller can label arbitrary bytes as application/pdf and the check at paperless.ts:326 will pass. This is the same pattern already used for image uploads in photos.ts:450 (file.mimetype !== 'image/webp'), so it's a pre-existing app-wide convention, not a new regression. Risk is limited here because Cornerstone never parses/renders the buffer itself — it's forwarded as an opaque blob to Paperless-ngx's own consumption pipeline, which performs its own file-type detection. Worth a defense-in-depth follow-up (e.g. magic-byte sniffing via a file-type-style check) across all upload endpoints, not specific to this PR.
  2. No maxLength on the title form field. The multipart route has no JSON schema (multipart bodies aren't schema-validated), so title is only checked for presence, not length — bounded only by @fastify/multipart's default per-field size limit (1MB) and the container's request-size ceiling. Low impact (forwarded as a document title string to Paperless), but consider adding an explicit length guard for consistency with other free-text fields in the app (which use AJV maxLength).
  3. No maxItems on mark-claimed's invoiceIds array. A very large array could hit SQLite's default bound-parameter limit inside the inArray() queries and surface as an unhandled 500 rather than a clean validation error. Low severity (household-trust model, no exploit path — just a robustness/DoS-shaped edge case), but a maxItems cap (e.g. 500) would give a cleaner 400 instead.

None of the above block merge. Verdict: APPROVED.

…ns spec

Story #1877 added the Household tab to ManagePage, making 6 tabs total.
orientations.spec.ts's secondary tab-count assertion (line ~389) wasn't
updated at the time and was failing full-matrix shards 6/11/15.

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

[product-owner] Product Owner review of PR #1885 against story #1878.

I verified every acceptance criterion against the implementation and the accompanying test suites (sourceReportService.test.ts, sourceReports.test.ts, depositAggregateUtils.test.ts, paperlessService.test.ts, paperless.test.ts, invoiceDepositService.test.ts, invoiceDeposits.test.ts).

AC coverage

Report data — GET /api/source-reports

AC Verdict Evidence
budget-overview returns all four statuses incl. quotation Met getSourceReport status switch adds all 4; scenario 8
claim = pending+paid only; proof-of-funds = claimed only Met scenarios 9, 10
Multi-source invoice → partial allocatedAmount + isSplit: true Met single COUNT(DISTINCT COALESCE(wib…, hib…)) split query; scenarios 11, 12a, 12b (12b covers the COUNT DISTINCT NULL exclusion)
Matching-status invoice with zero budget lines → unallocatedInvoices[], not main list Met dedicated NOT EXISTS query; scenarios 15, 15b
Exactly-zero net contribution dropped; negative kept as lineKind: 'refund-adjustment' with negative amount Met drop evaluated post-rounding (correct, matches the aggregation helper's contract); scenarios 13, 14
Stage-based document rule + untagged docs always included, in every report type Met quotation-status → quotation; residual (non-quotation, in-slice) → invoice; in-slice deposit/refund fractions → deposit; attachmentType === null always kept. Scenarios 16a–16e
Batched id__in ASN/title fetch in try/catch; degrades to doc-ID-only, never hard-fails Met getDocuments uses id__in; try/catch returns null ASN/title. Scenarios 17 (reachable → populated), 18 (throws → degrades), 19 (unconfigured → no fetch at all)
Missing/invalid type/sourceId → 400; unknown source → 404 Met AJV querystring schema with enum + required; NotFoundError. Route tests cover missing type, missing sourceId, invalid enum value, unknown sourceId
2 dp rounding server-side, total summed from rounded values Met per-line toCents(...)/100, then total summed from the rounded lines; scenario 21 asserts exact addition on decimal-noisy input

Mark claimed — POST /api/source-reports/mark-claimed

AC Verdict Evidence
Single transaction; invoices flip to claimed incl. pending → claimed, with onInvoiceStatusChanged; eligible deposits and refunds flip with claimed_date = today, paid_date preserved, via onDepositStatusChanged Met validate-then-write inside one db.transaction; deposit eligibility driven off the shared ALLOWED_TRANSITIONS table rather than a duplicated status list. Scenarios 23, 24 (paidDate stays null), 25 (paidDate preserved), 26 (already-claimed invoice + pending refund → invoice untouched, refund flips), 30 (diaryAutoEvents=false), 31, 32
Stale wizard → 409 listing offending IDs, no partial changes Met InvoicesNotClaimableError (409, INVOICES_NOT_CLAIMABLE, details.invoiceIds) thrown before any write. Scenarios 27 and 28 verify rollback by re-querying rather than trusting the throw — good rigour; 29/29b cover the "claimed invoice with nothing sweepable" case

Paperless upload proxy — POST /api/paperless/documents

AC Verdict Evidence
Multipart PDF + title → forwarded to post_document, filter tag applied when configured, { taskId } returned Met scenarios 31, 35
Unconfigured → 503 via requirePaperless; non-PDF rejected Met gating runs before the multipart parse; MIME check before toBuffer(). Scenarios 36, 37 (upload never attempted), 38, 39, 40
Filter-tag resolution failure → uploads untagged with a warning, does not fail Met scenarios 32 and the route-level "unresolvable filter tag → 201 untagged" test

Scope and process

  • No scope creep. The two additions beyond the endpoint trio — widening deposit ALLOWED_TRANSITIONS.pending to include claimed, and fixing getSourceReport: Paperless ASN/title enrichment never populates (Map iteration bug) #1884 — were both explicitly anticipated in the story's implementation notes / arose from it.
  • The two refinements are accepted as documented deviations: unallocatedInvoices[] uses a lighter dedicated type instead of a dummy-filled SourceReportInvoice (cleaner — the informational rows genuinely have no allocation, no lineKind, no documents), and the PATCH pending → claimed path auto-setting both dates ("fully settled" single-deposit semantics) versus the batch sweep preserving paidDate including null ("submitted for reimbursement") is a deliberate, wiki-documented split. Both semantics are now written down in API-Contract.md, which is what makes the divergence safe.
  • Wiki accuracy: API-Contract.md was updated in the same change and matches the implementation I read (degradation behaviour, 409 shape with "NO rows modified", paidDate preserved/never cleared, pending → claimed auto-set rules, 503 gating). No deviation found.
  • Browser E2E deferred to Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 is accepted — these endpoints have no UI surface until the wizard consumes them.
  • CI: Quality Gates (the required check for beta) passes. E2E shards 5/6/11/15 fail on invoices.spec.ts:841 ("Effective Amount" column, from Deposit refunds with negative claim adjustments #1876) and dashboard.spec.ts:566 — both pre-existing and unrelated to this backend-only diff (no client/ or e2e/ files changed).

Follow-ups (non-blocking — do NOT hold the merge)

  1. N+1 Paperless round-trips. getSourceReport issues the documentLinks SELECT and the paperlessService.getDocuments() call inside the per-invoice loop. The AC's "batched id__in" is satisfied per invoice, and the try/catch degrades correctly at every iteration, so this is functionally correct. But a claim report over 80 invoices makes 80 sequential HTTP round-trips to Paperless, and the wizard in Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 sits directly on this latency. Recommend hoisting to one document-links query plus one getDocuments() over the union of IDs before the loop. Say the word and I will file this as a backlog item against the Bank Report Wizard mini-epic so Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 does not inherit a slow report call.
  2. Duplicated filter predicate. The link.attachmentType === null || stages.has(...) filter runs twice (once to build documentIds, once to build documents). Harmless today, but two copies of a filtering rule is exactly where a future stage-rule change goes half-applied.
  3. Stale test comment. The comment block above scenario 17 in sourceReportService.test.ts still narrates that the test "is skipped (not deleted, not inverted) until getSourceReport: Paperless ASN/title enrichment never populates (Map iteration bug) #1884 is fixed". getSourceReport: Paperless ASN/title enrichment never populates (Map iteration bug) #1884 is fixed in this very PR and the test is active. Worth trimming to a one-line historical note so a future reader does not go hunting for a skip that is not there.

Process note for the orchestrator

Per CLAUDE.md, the architecture, security, and QA reviews are required on every story PR. At the time of this review no other agent reviews or comments are posted on #1885. Those must land before merge — my acceptance covers requirements and AC coverage only, not the security or architectural verdict.

Verdict: APPROVED

@steilerDev

Copy link
Copy Markdown
Owner Author

[product-architect] Architecture review of PR #1885 (story #1878).

Reviewed against wiki/API-Contract.md, the deposit-aggregation contract in depositAggregateUtils.ts, and the story ACs. Scope: shared types, sourceReportService, sourceReports route, Paperless upload proxy, ALLOWED_TRANSITIONS widening, wiki.

What I verified as correct

  • Aggregation design. computeStatusContributionByInvoice is a clean per-invoice generalisation of computeStatusContribution: it reuses splitByDeposits (so refund sign handling, deposit dedup, and the max(0, …) residual clamp stay in exactly one place) and deliberately does not drop zero/negative, deferring that to the caller so "zero" is evaluated post-rounding. That layering is right — the doc comment saying so is appreciated.
  • Status slices per report type match the ACs (budget-overview = all 4 incl. quotation; claim = pending+paid; proof-of-funds = claimed).
  • Refund-adjustment lines: lineKind derived from the sign of the rounded net, negatives kept, exact zeros dropped. Correct, and scenario 13/14 pin both.
  • Rounding: per-line toCents(...)/100 first, totalAmount summed from the already-rounded values — PDF columns will add up. Scenario 21 pins the decimal-noise case.
  • Stage-based document selection is coherent: quotation-status → quotation; residual portion under a non-quotation in-slice status → invoice; any in-slice deposit/refund → deposit; untagged always included. Mapping refund entries onto the deposit stage is the right call given AttachmentType = 'quotation' | 'deposit' | 'invoice'.
  • markInvoicesClaimed: single db.transaction, claimability computed for all IDs before any write, InvoicesNotClaimableError (409, details.invoiceIds) thrown inside the transaction so nothing commits. Scenarios 27/28 verify the rollback by re-query rather than by trusting the throw — that is the right way to test it.
  • The pending → claimed widening and its two different date semantics are correct and, importantly, documented. ALLOWED_TRANSITIONS.pending now includes claimed; the PATCH path sets both paidDate and claimedDate to today (the deposit really was paid and claimed in one action), while the mark-claimed sweep sets only claimedDate and preserves paidDate (a pending deposit swept during a batch claim genuinely has no known paid date, so fabricating one would be wrong). API-Contract.md documents both — the transition table under PATCH /api/invoice-deposits/:id and the "Behavior" bullets under mark-claimed. Good; this is exactly the kind of intentional asymmetry that silently rots otherwise.
  • ALLOWED_TRANSITIONS as the single claimability oracle in the sweep (claimed: ['paid'] ⇒ already-claimed deposits are naturally skipped) rather than a second hardcoded status list. Scenarios 29b/32 cover it.
  • Upload proxy: MIME check before toBuffer(), tag resolution failure degrades to an untagged upload, 502 split between PAPERLESS_UNREACHABLE and PAPERLESS_ERROR, { taskId } returned with no document_link row (correct — post_document yields a task UUID, not a document id).
  • Wiki accuracy: I checked the API-Contract additions field-by-field against the shared types and the route/service. Query params, response shapes, status codes (400/401/404, 400/401/409, 400/401/413/502/503), error codes, and the claimability rules all match the implementation. INVOICES_NOT_CLAIMABLE added to the ErrorCode union. Response envelope ({ report }) matches the budgetOverview/budgetSources convention.
  • Test shape: 36 named scenarios against a real SQLite DB plus route-level app.inject() suites. Deferring browser E2E to Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 is the right sequencing given there is no UI yet.

Blocking

H1 — Paperless metadata resolution runs inside the per-invoice loop (network N+1)

server/src/services/sourceReportService.ts:199-242 — for every report invoice the loop does its own documentLinks select and its own await paperlessService.getDocuments(...).

getDocuments is not a cheap call: it issues /api/tags/?page_size=1000 (uncached — only the filter tag id is memoised, not the tag map) in addition to /api/documents/?id__in=…, then resolves correspondents and document types, and none of that is deduplicated across invoices. So an N-invoice report costs roughly 2–4N sequential HTTP round trips to Paperless. On a claim report with 40 invoices that is well over a hundred serialized requests at ~100ms each.

Two things make this worth fixing before the wizard frontend (#1879) builds on it:

  1. It inverts the AC. AC EPIC-07: Reporting & Document Export #7 asks for a batched id__in fetch; the batching currently only holds within a single invoice, which is the case where it matters least.
  2. The report pays for enrichment it throws away. Only archiveSerialNumber and title are consumed — the correspondent/document-type round trips are pure waste, multiplied by N.

It also contradicts the codebase's own stated convention: paperlessService.ts:408 carries the comment "Collect unique correspondent and document type IDs to avoid N+1 calls".

Requested change — hoist both fetches out of the loop:

  • One documentLinks query for all report invoice ids (inArray(documentLinks.entityId, reportInvoiceIds) with entityType = 'invoice'), grouped into a Map<invoiceId, links[]>.
  • Compute the per-invoice stages set first, take the union of surviving paperlessDocumentIds across all invoices, and make a single getDocuments call for that union, wrapped in the existing try/catch. On failure the whole map stays empty and every line degrades to null ASN/title — identical observable behaviour to today (scenarios 18/19 should still pass unchanged), just once instead of N times.
  • Then build documents[] per invoice from the shared map.

While you are in there: the same .filter(...) predicate is currently evaluated twice (once to build documentIds, once to build documents) — after the hoist, filter once and reuse.

Non-blocking (fine for refinement / #1879)

  • M1 — the stage rule is not in the contract. API-Contract.md only says documents are "filtered by document stage". The actual rule (quotation-status invoice → quotation; in-slice deposit/refund portions → deposit; residual portion under an in-slice non-quotation status → invoice; untagged documents always included in every report type) is load-bearing for how the wizard renders and labels attachments. Please spell it out under GET /api/source-reports — it is behaviour the frontend has to reason about, not an implementation detail.
  • L1 — unused shared type. PaperlessUploadResponse is defined and exported from shared/, but paperlessService.uploadDocument and the route both use an inline Promise<{ taskId: string }>. Use the shared type as the return annotation so the contract type is actually load-bearing.
  • L2 — taskId shape is unchecked. (await response.json()) as string trusts Paperless to return a bare JSON string. A narrow typeof !== 'string' guard mapping to PAPERLESS_ERROR would fail loudly rather than propagate a non-string taskId into the archive record.
  • L3 — upload size limit. The PDF upload relies solely on the global 50MB multipart cap in app.ts, whereas routes/photos.ts enforces a per-config limit on top. Worth a follow-up decision on whether report PDFs deserve their own bound.
  • Informational — the per-invoice documentLinks select is an in-process SQLite N+1, which is within our accepted trade-offs at this scale; I am asking for it to be hoisted only because it falls out of the H1 fix for free. console.warn('[sourceReport] …') matches the existing service logging convention (diaryAutoEventService, paperlessService) — no change wanted.

Nothing here touches the schema, so no Schema-page or migration follow-up is owed. No ADR is required: the report is a read-model over existing entities and the transition widening is documented in the contract.

Happy to re-review promptly once H1 is addressed — it is a contained change and the existing degradation tests should cover it unmodified.

VERDICT: CHANGES_REQUIRED

…oad response handling

Applies product-architect review findings from #1878:
- H1: batch document-link lookup and Paperless getDocuments resolution
  across all report invoices into one query + one call, instead of
  per-invoice N+1 fetches
- M1: document the concrete Paperless document-stage filtering rule in
  the API contract
- L1: use the shared PaperlessUploadResponse type instead of an inline
  { taskId: string } shape
- L2: guard the Paperless upload response, throwing PAPERLESS_ERROR if
  taskId is not a string instead of trusting an unchecked cast

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com>
@steilerDev

Copy link
Copy Markdown
Owner Author

[product-architect] Round 2 review of PR #1885 — re-reviewed commit ce8972df and wiki 589167d.

H1 (blocking) — verified fixed on the merits

The restructuring in sourceReportService.ts is exactly the shape I asked for, and I checked it does what the commit message claims rather than taking it on trust:

  • One documentLinks query for the whole report: inArray(documentLinks.entityId, reportInvoiceIds) conjoined with entityType = 'invoice', replacing the per-invoice eq() select.
  • Stage filtering still per-invoice — the stages set is computed inside the new step-7 loop from that invoice's own split, so the quotation / residual / deposit rules are unchanged. I re-read them against the round-1 version line by line; the predicate is identical, only its position moved.
  • One getDocuments call over Array.from(allPaperlessDocIds) — the union across all invoices — inside the same try/catch. The paperlessEnabled && url && token && size > 0 guard is preserved, so the unconfigured path still makes zero HTTP calls.
  • Duplicate .filter() eliminated; linksByInvoiceId is built once and mapped in step 9.

Net: an N-invoice report went from ~2–4N sequential Paperless round trips to 2–4 total, regardless of N.

Behavioural side-effect worth recording, and it is an improvement: degradation is now uniform across the report. Previously a transient Paperless failure mid-loop could produce a report where invoice 1 carried real ASN/title and invoice 2 carried nulls — an internally inconsistent document to hand to a bank. Now the whole report degrades together. Scenarios 18/19 passing unchanged is the right evidence that the observable contract is intact.

Empty-report regression checked explicitly, since the fix introduced a new inArray on a list that can legitimately be empty (a budget source with no allocated invoices). Two paths verified rather than assumed:

  • inArray(col, []) in drizzle-orm 0.45.2 returns sql\false`(confirmed in the installedsql/expressions/conditions.js:73-81`) — no throw, no malformed SQL.
  • The pre-existing isSplit query builds a raw IN (${sql.join(...)}), which renders IN () when the id list is empty; SQLite accepts an empty IN list as a documented extension (confirmed against better-sqlite3).

So a zero-invoice source returns an empty report rather than a 500. Good.

M1 / L1 / L2 — verified fixed

  • M1: the concrete stage rule is now in API-Contract.md, including the load-bearing "untagged documents are always included regardless of report type" clause. That is the part the Bank report wizard frontend: /budget/reports, PDF pipeline & claim flow #1879 wizard needs.
  • L1: uploadDocument now returns Promise<PaperlessUploadResponse> — the shared type is load-bearing instead of decorative.
  • L2: taskId is guarded with typeof !== 'string'PAPERLESS_ERROR 502, consistent with the other upstream-failure mapping in the same function.

Wiki formatting — fixed by me

The M1 edit had inserted the "Document Filtering Rule" prose inside the response-field table, orphaning the totalAmount / unallocatedInvoices / generatedAt rows as stray pipe-text below it. I moved those three rows back into the table and placed the prose after it.

Pushed to the wiki repo directly: 1723f7d (docs(api): repair split response-field table in source report section). Cosmetic only — the submodule pointer on this branch still references 589167d and can be bumped in any later commit; nothing blocks the merge.

Non-blocking findings (for refinement / #1879, not this PR)

  • M2 — no regression guard for H1. git diff 79a8b80e..ce8972df touches no test file. The behaviour is well covered, but nothing pins the call count, which is the property that was actually wrong. A future refactor could quietly re-loop the fetch and every test would stay green. The mock is already in place in scenarios 17/18 — a two-invoice fixture plus expect(getDocumentsSpy).toHaveBeenCalledTimes(1) is ~3 lines and locks in the fix.
  • M3 — the new L2 guard branch is untested. uploadDocument's typeof taskId !== 'string' → 502 path has no covering case in paperlessService.test.ts. Small, but it is a new uncovered branch against the 95% target.
  • InformationalallPaperlessDocIds is accumulated over all reportInvoiceIds, including invoices later dropped in step 9 for an exactly-zero net contribution, so the batch can request a few document ids that never appear in the response. Harmless (one batched call either way) and not worth restructuring for; noting it so nobody later reads it as a bug.
  • InformationalallDocumentLinks.filter(...) inside the per-invoice loop is O(N×M) in memory, and allPaperlessDocIds.add() is a side effect inside a .map(). Both are fine at our scale; a Map<invoiceId, links[]> grouping pass would read cleaner if this area is touched again.
  • Informational — the stage-rule bullets in the contract read as alternatives, but the implementation is additive: one invoice can qualify for several stages at once (e.g. a paid invoice with in-slice deposits collects both invoice and deposit). Worth a clarifying half-sentence next time the page is edited; I left the wording alone to keep this fix purely structural.
  • Scope note — the branch also carries 382908e9 (fix(e2e): update ManagePage tab count for Household tab), an unrelated 4-line E2E fix not present on beta. Correctly trailered to e2e-test-engineer and it unblocks the shards, so no objection to it riding along — flagging it only so it is not lost in the squash summary.

Everything I raised in round 1 is addressed, the fix is correct on inspection rather than merely plausible, and the contract now matches the implementation. No schema changes, so no Schema-page or migration follow-up is owed; no ADR required.

VERDICT: APPROVED

@steilerDev
steilerDev merged commit fc7f6ad into beta Jul 29, 2026
31 of 33 checks passed
@steilerDev
steilerDev deleted the feat/1878-source-report-backend branch July 29, 2026 19:26
@github-actions

Copy link
Copy Markdown
Contributor

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant