Skip to content

feat(reports): report table cleanup — shared footnotes, deposit labels, claim metadata, total-only summary, usage area (#1923) - #1924

Merged
steilerDev merged 3 commits into
betafrom
feat/1923-report-table-cleanup
Aug 2, 2026
Merged

feat(reports): report table cleanup — shared footnotes, deposit labels, claim metadata, total-only summary, usage area (#1923)#1924
steilerDev merged 3 commits into
betafrom
feat/1923-report-table-cleanup

Conversation

@steilerDev

Copy link
Copy Markdown
Owner

Summary

  • AC1 — Shared split footnote: split-invoice rows now carry a single unnumbered marker; the footnote list has exactly one entry (no per-invoice numbering, no vendor/invoice-number prefix).
  • AC2 — Deposits labelled inline: constituted-deposit rows (allocation made up entirely by a deposit tagged to the reported source) show an inline "Deposit"/"Abschlagszahlung" Badge instead of a footnote marker; reduced-deposit rows keep a single shared, unnumbered footnote. The depositConstitutedFootnote key is removed (en+de).
  • AC3 — Claim reports omit metadata: the Source/Source Type/Reference/Generated block is entirely absent (not hidden) from both the HTML preview and the PDF export for claim-type reports; budget-overview/proof-of-funds are unaffected.
  • AC4 — Total-only summary: per-status subtotal rows are removed; only the Total row remains. The subtotal key is removed (en+de).
  • AC5 — Area in Usage column: SourceReportLinkedItem gains areaId/areaName, populated server-side via a LEFT JOIN areas on work_items.area_id/household_items.area_id (leaf area name only, no parent-path expansion). Rendered as a distinct, non-editable line below the Usage field (desktop table, mobile card, and PDF) — never baked into the editable usageText, so it survives AI-generated usage descriptions.
  • AC6 — No regressions: en/de i18n parity verified, mobile card layout updated to match desktop, existing unit/E2E coverage updated (not deleted).

Fixes #1923

Test plan

  • Unit tests pass (95%+ coverage — buildReportContent 100%, sourceReportService 100%/93% branch, ReportContentEditor 100%, overviewPdf 100%/98%)
  • Integration tests pass (sourceReportService.test.ts extended with 5 new AC5 area-join tests)
  • 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 frontend-developer (Haiku 4.5) noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) noreply@anthropic.com
Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) noreply@anthropic.com
Co-Authored-By: Claude translator (Sonnet 4.5) noreply@anthropic.com

steilerDev and others added 2 commits August 2, 2026 02:31
…a removal, total-only summary, area in usage column

Fixes #1923

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 frontend-developer (Haiku 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude translator (Sonnet 4.5) <noreply@anthropic.com>
…update realRender deposit assertions

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

[ux-designer]

Verdict: APPROVED

Reviewed against the visual spec posted on issue #1923 (HEAD 86e5c72). Implementation matches the spec point-for-point:

  • .depositBadge composes attachmentDeposit from Badge.module.css (no new variant, no drift risk) — exactly as specified.
  • .depositLabel (margin-left spacing-2) is applied only on the desktop badge; the mobile badge omits it and relies on .mobileCardAllocated's flex + gap: var(--spacing-2) + flex-wrap: wrap instead — correct, avoids double-spacing.
  • .usageAreaText (font-size-xs, text-muted, margin-top-1) renders as a separate <div> (desktop) / <span> (mobile) after the EditableField, never concatenated into usageText — confirmed by the new test asserting the input value stays 'Kitchen work' and doesn't include the area suffix.
  • Summary: .summaryTable tbody tr:last-child td { border-bottom: none; } added, and .summaryAmount gets font-size: var(--font-size-base) while .summaryLabel is untouched — matches the spec's "amount grows, label doesn't" guidance exactly.
  • Claim metadata block uses {!content.isClaim && (...)} with no placeholder/spacer — relies on the existing .container flex gap, per spec. Same pattern (guarded content.push) applied in overviewPdf.ts.
  • PDF: deposit suffix is a separate text run (color: '#6b7280', fontSize: 8), and the usage cell stack orders usage → area (style: 'small') → attachment note, both matching the spec's hierarchy and reusing the existing small style.

Token compliance: all new CSS values are var(--token-name) (no hardcoded colors/spacing in the CSS Module). The two #6b7280 references are PDF-only text-run colors, matching the pre-existing small style's baked hex (documented precedent — pdfmake exports are a fixed light-palette document, not theme-aware). Dark mode: .depositBadge and .usageAreaText route through --color-attachment-deposit-* and --color-text-muted, both of which flip correctly under [data-theme="dark"]. Accessibility: the deposit signal is a real text-labeled Badge (not color-only), consistent with the reused attachmentType.deposit chip used elsewhere in the reports feature.

No findings, blocking or otherwise.

@steilerDev

Copy link
Copy Markdown
Owner Author

[product-owner]

Product Owner review — PR #1924 vs. Story #1923 (27 AC)

Reviewed at 86e5c72f. Verified against gh pr diff, the files on disk, and the unit/E2E tests as evidence.

Verdict: COMMENT — 1 MUST FIX before merge

26 of 27 acceptance criteria are met, with genuinely good test evidence — the unit tests are AC-tagged, the E2E specs cover all three viewports, and the "no regression" counter-cases (non-claim metadata block, non-split reports, non-deposit rows, rows without an area) are all explicitly asserted rather than assumed. One non-functional display gap must be fixed before merge; it does not block the review loop.


MUST FIX — the inline Deposit label renders in the UI language, not the report language

AC2.1, and the spirit of AC1.4 ("appears identically in the HTML preview and the PDF export").

The two renderers get two different t:

  • ReportWizardPage.tsx L262 builds reportT = i18n.getFixedT(reportLanguage, 'budget') and passes it to the PDF path (L325). The PDF's deposit run (overviewPdf.ts, (${t('sourceReports.table.attachmentType.deposit')})) therefore renders in the report language. Correct.
  • ReportContentEditor receives the UI t (ReportWizardPage.tsx L905, from useTranslation('budget') at L61). Both Badge labels added in this PR (desktop cell and mobile card) call that t.

Net effect: with UI language de and report language en, the editable preview shows a row reading €400.00 + badge "Abschlagszahlung" under a report-language column header "Allocated Amount", while the exported PDF for the same row reads "(Deposit)". The preview stops being a faithful preview of the artifact.

This is the first piece of artifact content in the preview to be rendered with the UI t — everything else the editor prints as report content comes from content.labels / content.rows / content.footnotes, all built with reportT. Note this is not covered by the #1909 ruling that accepted mixed-language mobile cards: that ruling accepted UI-language edit affordances (reset buttons, edited-suffix, field labels) precisely because artifact content stays in the report language. The deposit label is artifact content, so it falls on the other side of that line.

Suggested resolution (product-level; exact shape is the architect's call): resolve the label once in buildReportContent.ts via reportT and expose it on ReportContentLabels (e.g. labels.deposit), then have both renderers consume content.labels.deposit. That also removes the only remaining t() call for artifact text from overviewPdf.ts. Please extend the existing realRender.test.ts both-locale assertion to cover the preview path too, so the divergence cannot silently return.


AC coverage

AC Verdict Evidence
1.1 shared on 3+ split rows Met buildReportContent.ts L185–187 emits bare '†'; unit buildReportContent.test.ts "AC1.2 … three included invoices are split"
1.2 exactly one footnote, no vendor prefix Met if (splitInvoiceIds.size > 0) pushes one { id: 'split', marker: '†', text: reportT('…splitFootnote') } — prefix template gone; unit AC1.2
1.3 no when nothing split Met Unit "AC1.3 … no † marker and no split footnote anywhere"
1.4 identical HTML/PDF Met for markers/footnotes (both read content.*); see MUST FIX for the deposit label
2.1 inline Deposit/Abschlagszahlung, no Met functionally (isDeposit row flag; Badge desktop + mobile; PDF run) — wording language is the MUST FIX. Unit ReportContentEditor.test.tsx ×2 + overviewPdf.test.ts "isDeposit inline label"; E2E Scenario 17 (desktop/tablet/mobile)
2.2 constituted footnote gone; key deleted both locales Met depositConstitutedFootnote absent from en/budget.json and de/budget.json (grep: zero hits); unit AC2.2; realRender.test.ts asserts neither "This is a deposit" nor "Dies ist eine Abschlagszahlung" survives anywhere
2.3 single shared + one footnote Met Unit AC2.3 ×2 (incl. multiple reduced invoices → still one entry); realRender.test.ts both locales
2.4 †‡ order, exactly two footnotes Met Unit AC2.4 (marker order and footnote array order [split, deposit-reduced])
2.5 *n skip footnotes unchanged Met skipFootnotesByInvoiceId block untouched; overviewPdf.test.ts L369–390 (€400.00*1†, €400.00*1*2)
3.1 claim preview omits metadata Met !content.isClaim && guard; unit AC3.1; E2E Scenario 16
3.2 claim PDF omits metadata Met if (!reportContent.isClaim) wraps the whole push; unit AC3.2
3.3 non-claim types unchanged Met Explicit counter-tests in both ReportContentEditor.test.tsx and overviewPdf.test.ts ("renders the sourceInfoStack when isClaim is false … unchanged")
3.4 claim title/cover letter/table/summary/footnotes still render Met E2E Scenario 16 asserts all of them
4.1 / 4.2 exactly one Total row, no subtotals Met Whole statusCounts/statusLabels loop deleted; unit AC4.1/4.2 (2+ statuses) and AC4.1 (single status); ReportContentEditor.test.tsx queryByText(/Subtotal/) → absent; E2E Scenario 19
4.3 total math unchanged Met includedTotal reduce untouched; unit AC4.3
4.4 subtotal key removed both locales Met grep: zero hits in en/de
5.1 areaId/areaName on the type, populated server-side Met shared/src/types/sourceReport.ts +2 fields; sourceReportService.ts LEFT JOIN areas awi/ahi + COALESCE; 5 new server tests incl. work-item and household-item paths
5.2 area shown in Usage, distinct + comma-joined, desktop/mobile/PDF Met getAreaText() uses uniqueInOrder(...).join(', '); unit dedupe test; ReportContentEditor.test.tsx desktop + mobile; overviewPdf.test.ts stack ordering; E2E Scenario 20
5.3 separate, non-editable, survives AI Met areaText is its own ReportContentRow field rendered outside EditableField; applyAiContent only ever writes usageText; E2E reportWizardAiGeneration.spec.ts Scenario 8 asserts the area line before and after "Generate with AI"
5.4 no area, no empty separator Met Three unit cases (empty budgetLines, areaName: null, description-only fallback) + renders no area element (desktop or mobile) when areaText is null + PDF "plain { text } cell (not a stack)"; E2E covers the no-area item
5.5 leaf area only Met Join is awi.id = wi.area_id — no recursion; server test "child area with a parent — no parent-path expansion"; client test asserts verbatim consumption
5.6 wiki Schema + API Contract Met, with a documented deviation wiki@f3101b5 updates API-Contract.md with linkedItem.areaId/areaName (incl. the leaf-only rule) and is pushed to origin/master. Schema.md was not touched — accepted: this PR adds no DB column, and work_items.area_id / household_items.area_id are already documented there (Schema.md L2673–2704). SourceReportLinkedItem is a response type, so API Contract is its correct home. The AC as written was over-specified on my side.
6.1 locale parity Met Two keys removed from both locales; no new keys added (the label reuses the existing attachmentType.deposit, whose de value Abschlagszahlung matches the AC)
6.2 all report types + 3 viewports incl. mobile cards Met Mobile card gets both the Badge and the area <span>; E2E Scenarios 17 and 20 assert desktop/tablet/mobile explicitly
6.3 existing coverage updated, not deleted Met buildReportContent.test.ts +244/−63, ReportContentEditor.test.tsx +104/−14, overviewPdf.test.ts +114/−7, realRender.test.ts rewritten in place (assertion strengthened, not weakened), E2E spec +483/−6, sourceReportService.test.ts +225/−1. Fixture-only touches to applyAiContent/applyOverrides/merge/coverLetterPdf/ReportInvoiceList tests. ReportWizardPage.test.tsx needed no change — checked, it holds no subtotal/footnote-format assertions.

Non-blocking notes (not MUST FIX)

  1. Hardcoded colour in the PDF. overviewPdf.ts uses color: '#6b7280' inline for the deposit run, while the neighbouring refund path uses the named REFUND_TEXT_COLOR from shared.js. Suggest a sibling constant for consistency — architect's call.
  2. Stale test fixtures. ReportContentEditor.test.tsx L516 and L870 still pass allocatedMarkers: '†1'. Harmless (the component renders the marker string opaquely), but it no longer matches any value buildReportContent can produce. Worth updating to '†' next time the file is touched.
  3. Scope: clean. No changes outside the five feedback items; the AI prompt inputs were correctly left alone per the story's "Out of scope" note.

Re-request my review once the deposit label resolves through the report-language t and I will approve.

@steilerDev

Copy link
Copy Markdown
Owner Author

[product-architect]

VERDICT: REQUEST CHANGES (posted as a comment — GitHub blocks a formal review on the same account that authored the PR)

Architecture review — PR #1924 (story #1923)

Verified against the wiki Architecture / API-Contract / Schema pages, the reportContent content-layout split (#1900, ADR-034), and the AC list on #1923.

What checks out

  • Areas join (AC5.1/5.4/5.5) — correct. invoice_budget_lines carries the XOR CHECK ((work_item_budget_id IS NOT NULL AND household_item_budget_id IS NULL) OR (…)) (migration 0028_add_quotation_status.sql L52-55), so wi and hi are mutually exclusive per row and COALESCE(awi.id, ahi.id) / COALESCE(awi.name, ahi.name) can never silently pick the "wrong" side. Both new LEFT JOIN areas are on areas.id (PK) → strictly 1:1, so no row multiplication and allocatedAmount / the Rail-A dedup (iblDetails.has(row.ibl_id)) are untouched. Leaf-only (no recursive parent walk) matches AC5.5. areas.name is NOT NULL, so the independent COALESCE on id and name cannot desynchronise.
  • Total math (AC4.3) — unchanged. includedTotal is still reduce(sum + inv.allocatedAmount) over included invoices; only the per-status subtotal loop was deleted. No allocation math moved.
  • Deposit classification (AC2) — semantics preserved exactly. The old guard if (!included || !invoice.isSplit) continue is faithfully re-expressed as isSplit && budgetLines.length > 0 / isSplit && deposits.length > 0, and taggedDeposit = deposits.some(d => d.budgetSourceId === report.source.id) is unchanged — so constituted → inline label + no marker, reduced → shared , matching AC2.1/2.3. Marker order († then ‡) and footnote order match AC2.4. Skip footnotes (*N) untouched per AC2.5.
  • i18n deletion completeness (AC2.2/4.4/6.1). depositConstitutedFootnote and table.subtotal removed from both en and de; no production references survive (only test/E2E comments documenting the removal). Locale parity holds.
  • API contract. SourceReportLinkedItem extension is documented on the wiki API-Contract page (f3101b5) with the leaf-area caveat spelled out. No Schema page change was needed — no DDL changed and work_items.area_id / household_items.area_id are already documented under migration 0028; AC5.6's Schema bullet is satisfied by existing content, not a miss.
  • Test coverage. Server tests cover both link types, the null-area case, the child-area leaf-only case, and the defensive linkedItem === null path. Client tests are AC-tagged across buildReportContent, both renderers, and realRender. CSS uses tokens throughout and composes the existing attachmentDeposit Badge variant rather than forking a style.

Blocking finding

[High] The inline deposit label is resolved with the UI-language t in the HTML preview but the report-language reportT in the PDF — the two renderers will print different languages.

  • client/src/components/reports/ReportContentEditor.tsx L204 (desktop) and L311 (mobile) call t('sourceReports.table.attachmentType.deposit'), where t is the wizard's useTranslation('budget') function (ReportWizardPage.tsx L61, passed at L905) — i.e. UI language.
  • client/src/lib/reportPdf/overviewPdf.ts resolves the same key through the t argument, which ReportWizardPage.tsx L320-325 supplies as reportT = i18n.getFixedT(reportLanguage, 'budget') — i.e. report language.

With UI = English and report language = German (a first-class scenario since #1903), the preview shows Deposit while the exported PDF shows (Abschlagszahlung) — the exact HTML/PDF divergence AC1.4 forbids for the other markers, on the one new string that isn't data-driven from the shared content model. No test catches it because every test passes a single t.

This also breaks the reportContent content/layout split: every report-language string is meant to be resolved once in buildReportContent via reportT and carried on ReportContent. The status Badge two lines above already does this correctly (label: row.statusText); the new deposit Badge is the only renderer-side t() call producing report content rather than editor chrome.

Fix (small):

  1. buildReportContent.ts — add to the labels object: deposit: reportT('sourceReports.table.attachmentType.deposit'),
  2. types.ts — add deposit: string; to ReportContentLabels.
  3. ReportContentEditor.tsx L204 + L311 and overviewPdf.ts — consume content.labels.deposit / reportContent.labels.deposit; overviewPdf then no longer needs t for this run.
  4. Add a regression test that builds content with reportT = getFixedT('de') and renders the editor with t = getFixedT('en'), asserting the badge reads Abschlagszahlung.

Non-blocking (address in refinement or ignore)

  • [Low] overviewPdf.ts hardcodes color: '#6b7280' and fontSize: 8 inline for the deposit run, while the file already imports named constants from ./shared.js (REFUND_TEXT_COLOR, TABLE_LAYOUT) and the doc definition defines a small style. Prefer a named constant (e.g. DEPOSIT_LABEL_COLOR) or style: 'small' for consistency.
  • [Low] On a row that is both isRefund and isDeposit, the run-level grey wins over the cell-level REFUND_TEXT_COLOR, so the deposit label renders grey inside an otherwise-red cell. Cosmetic only; flagging in case it's unintended.
  • [Info] overviewPdf.test.ts L569-570 still feeds subtotal-pending / subtotal-paid summary-row fixtures. Harmless (the PDF builder renders whatever summaryRows it receives), but the shape is no longer producible by buildReportContent — consider renaming the fixture keys so the test doesn't read as an AC4 regression.

Everything else is spec-compliant and consistent with the established architecture. Happy to re-review as soon as the deposit label is moved onto ReportContent.labels.

…ed content model

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

Copy link
Copy Markdown
Owner Author

[product-architect]

VERDICT: APPROVE — blocking finding resolved (posted as a comment; GitHub blocks a formal review on the same account that authored the PR).

Re-review of 86e5c72f..85f59f89

The High finding is fixed exactly as specified, and the fix is structural rather than local — it puts the string back under the content/layout contract instead of patching the call site.

Blocking finding — RESOLVED.

  • client/src/lib/reportContent/types.ts L56 adds deposit: string; // translated in report language to ReportContentLabels; client/src/lib/reportContent/buildReportContent.ts L293 resolves it once via reportT('sourceReports.table.attachmentType.deposit'), alongside the other 12 labels.
  • ReportContentEditor.tsx L204/L311 and overviewPdf.ts L168 both now read content.labels.deposit / reportContent.labels.deposit. Verified by grep: zero t('sourceReports.table.attachmentType.deposit') call sites remain anywhere under client/src (the three surviving attachmentType.deposit hits are documentCard.* in LinkedDocumentCard.tsx / LinkedDocumentsSection.tsx — a different namespace and genuine UI chrome, correctly left alone).
  • overviewPdf.ts still uses its t parameter for the skip footnotes at L234, so the signature stays meaningful and there is no dead argument.

Regression test — genuinely pins the failure mode. realRender.test.ts L895+ uses the production i18n singleton (not the isolated test instance), asserts i18n.language === 'en' before and after so getFixedT is proven not to mutate the ambient locale, builds content with getFixedT('de','budget') — the same construction as ReportWizardPage.tsx L262 — and asserts labels.deposit === 'Abschlagszahlung', plus an English contrast build proving the field tracks the passed reportT rather than a constant. It then renders through buildOverviewContent and asserts the allocated cell's second run is ' (Abschlagszahlung)', so the PDF path is pinned end-to-end. On the HTML side, ReportContentEditor.test.tsx swapping the assertion to the sentinel REPORT_DEPOSIT_LABEL from the LABELS fixture is a stronger assertion than before: with the identity-t mock, a regression back to t(key) would render the raw key and fail. Both renderers are now covered against re-drift.

Nit — RESOLVED. DEPOSIT_NOTE_TEXT_COLOR = '#6b7280' and DEPOSIT_NOTE_FONT_SIZE = 8 are now exported from client/src/lib/reportPdf/shared.ts next to REFUND_TEXT_COLOR, matching the file's established constant pattern; the test title was updated to describe intent rather than the literal values, so it no longer needs editing when the palette moves.

Remaining non-blocking items (unchanged from the first pass, both cosmetic — fine to leave or fold into refinement):

  • On a row that is both isRefund and isDeposit, the run-level gray still wins over the cell-level REFUND_TEXT_COLOR.
  • overviewPdf.test.ts L569-570 still uses subtotal-pending / subtotal-paid fixture keys for a shape buildReportContent can no longer emit.

Everything I verified in the first pass still holds: the COALESCE over the mutually-exclusive areas joins is sound under the invoice_budget_lines XOR CHECK with no row multiplication, allocatedAmount/total math is untouched, the constituted/reduced classification preserves the prior isSplit gating, both deleted i18n keys are gone from en and de with no dangling production references, and the wiki API-Contract documents the extended SourceReportLinkedItem. Approved — good to merge once the required gates are green (shard 5 red is pre-existing #1920).

@steilerDev

Copy link
Copy Markdown
Owner Author

[product-owner]

Product Owner review — PR #1924, round 2

Re-reviewed at 85f59f89 (git diff 86e5c72f..85f59f89).

Verdict: APPROVED

The round-1 MUST FIX is resolved, in the exact shape I asked for and then some.


MUST FIX verification — deposit label now resolves in the report language

Check Result
Label resolved once via reportT buildReportContent.ts L293: deposit: reportT('sourceReports.table.attachmentType.deposit') — built alongside every other report-language label
Exposed on the shared content model types.tsReportContentLabels.deposit: string; // translated in report language
Editor consumes it (desktop) ReportContentEditor.tsx L204 — label: content.labels.deposit
Editor consumes it (mobile card) ReportContentEditor.tsx L311 — label: content.labels.deposit
PDF consumes it overviewPdf.ts L168 — ` (${reportContent.labels.deposit})`
No renderer re-derives it from its own t Confirmed: overviewPdf.ts's only remaining t() call is the skip-footnote reason at L234 (correct — reportT is what's passed in from ReportWizardPage L325). The editor's remaining t() calls are all edit affordances (sourceReports.editable.*, and the field: interpolation for the reset aria-label) — the correct side of the #1909 artifact-content-vs-affordance line

The regression test is the right one. realRender.test.ts uses the real production i18n singleton (import('../../i18n/index.js')), not the file's isolated createInstance(), and asserts:

  • i18n.language === 'en' before and after — proving getFixedT never mutates the ambient/UI locale, so the "UI en / report de" split is genuinely exercised rather than simulated;
  • content.labels.deposit === 'Abschlagszahlung' — exact real string, not toContain;
  • the contrast case getFixedT('en')'Deposit', proving the field tracks whichever reportT was passed rather than being a fixed value;
  • the same German string pinned through the rendered PDF's inline deposit run (' (Abschlagszahlung)').

That last assertion is what makes this durable — it closes the loop end-to-end rather than only at the content-model boundary.

Bonus: the editor unit tests were strengthened as a side effect. They now assert the sentinel REPORT_DEPOSIT_LABEL sourced from the injected labels fixture rather than the raw key string sourceReports.table.attachmentType.deposit. The old form would have passed whether the label came from content.labels or from a key-echoing t mock; the new form can only pass if the value flows through the content model. Exactly the right correction.

Round-1 non-blocking note 1 also fixed: '#6b7280' / 8 are now DEPOSIT_NOTE_TEXT_COLOR / DEPOSIT_NOTE_FONT_SIZE in reportPdf/shared.ts, siblings of REFUND_TEXT_COLOR. Not required — appreciated.

Regression surface

ReportContentLabels gained a required field, so six fixture files were updated (ReportContentEditor.test.tsx, applyAiContent.test.ts, applyOverrides.test.ts, coverLetterPdf.test.ts, merge.test.ts, overviewPdf.test.ts). All additive deposit: entries — no assertion was weakened or removed anywhere in the delta. Quality Gates green on 85f59f89; PR MERGEABLE.

AC status: 27/27

AC1–AC6 all met. AC2.1 is now fully met including the report-language requirement. AC5.6 remains an accepted documented deviation (Schema.md untouched because no DB column was added; SourceReportLinkedItem is a response type documented in API-Contract.md at wiki@f3101b5, pushed).


Follow-up (out of scope for #1923 — do NOT expand this PR)

While verifying the t audit I found a pre-existing instance of the same defect class, unchanged by this PR and present on beta:

ReportContentEditor.tsx L82 renders the cover-letter date label via the UI t('sourceReports.coverLetter.dateLabel'), while the date value next to it (content.coverLetter.dateLine) is report-language. With UI de / report en the preview reads Datum: February 15, 2026. Artifact content, so it belongs in the report language — most likely as ReportContentCoverLetter.dateLabel, mirroring what this PR just did for deposit.

This predates #1923 and is not a regression, so it stays out of this PR per scope discipline. It doesn't fit the #1917 bucket either (that one is LLM-boundary scoped). Recommend a small standalone bug issue — say the word and I'll file it with acceptance criteria.

Remaining nit from round 1, still open and still not blocking: ReportContentEditor.test.tsx L516/L870 fixtures pass allocatedMarkers: '†1', a value buildReportContent can no longer produce. Harmless (the component renders the string opaquely) — worth cleaning next time the file is touched.

Approved for merge.

@steilerDev
steilerDev merged commit 0e4030d into beta Aug 2, 2026
60 of 66 checks passed
@steilerDev
steilerDev deleted the feat/1923-report-table-cleanup branch August 2, 2026 07:52
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

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

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