feat(reports): reinstate partial and deposit-reduced legend sentences (#1965)

[steilerDev]

Summary

• Reinstate two explanatory legend sentences for partial and deposit-reduced amounts in buildReportContent.ts — the only missing piece was pushing entries to the already-declared footnotes[] array
• Split legend: "Amount shown reflects only the portion allocated to this source."
• Deposit-reduced legend: "This position reflects deposits claimed separately." (the separately is the load-bearing word for bank auditors)
• Both preview and PDF renderers already consumed content.footnotes — no other files needed changing

Fixes #1965

Test plan

• buildReportContent.test.ts: 64 tests pass including 6 new AC 1.1–1.5 scenarios
• realRender.test.ts: 70 tests pass; deposit-reduced legend sentence now asserted present
• Lint clean

🤖 Generated with Claude Code

[steilerDev]

[ux-designer]

Verdict: APPROVED

Scope reviewed: no CSS changes, no new components, no new i18n keys — the only visual change is that ReportContentEditor.tsx (unchanged) now has legend entries to render.

Token adherence: nothing to flag — zero CSS changes in this PR.

i18n copy accuracy: all four keys referenced in buildReportContent.ts are confirmed present in both locales at the correct path (budget:sourceReports.table.*):

Key	EN value	DE value
splitInlineLabel	partial	Teilbetrag
splitFootnote	Amount shown reflects only the portion allocated to this source.	Der angezeigte Betrag umfasst nur den dieser Quelle zugeordneten Anteil.
depositReducedInlineLabel	less deposit (U+00A0)	abzgl. Abschlag (U+00A0)
depositReducedFootnote	This position reflects deposits claimed separately.	Diese Position berücksichtigt separat eingereichte Abschlagszahlungen.

The non-breaking spaces in the inline-label values are correct and intentional (per existing parity-test notes for multi-word inline labels).

Test assertions: the realRender.test.ts depositFootnoteText expected values match the actual translated strings exactly in both locales. The legend-sentence split from goneFootnotes to a positive check is correct — goneFootnotes now only lists strings that must remain absent (constituted-deposit label), and the reinstated legend sentence is asserted present.

No design-system concerns.

[steilerDev]

[product-architect] PR review — #1979 (story #1965)

Verdict: CHANGES_REQUIRED (one high finding: the change makes a currently-green E2E scenario fail, and leaves the E2E page-object contract asserting the opposite of the new behaviour).

What I verified

• Producer placement is right. buildReportContent.ts:234-248 gates on splitInvoiceIds / depositReducedInvoiceIds, both of which are built in the includedInvoiceIds-filtered loop at L145-163 — so the legend is scoped to included rows (AC 1.1/1.2) and reuses the exact same derivation as the row-level isSplit/isDepositReduced booleans. No parallel classification logic was introduced; preview, PDF, and row labels cannot drift.
• Dedup by construction — set membership, not per-invoice push, so N split rows yield one entry (AC 1.5 in the model), split-before-depositReduced order is fixed by statement order (AC 1.3).
• Additive only. No change to the fix(reports): improve report PDF UX — paragraph breaks, inline meta, inline notes, column toggles #1959 inline labels, labels.splitNote/depositReducedNote, or attachmentType.deposit (AC 2.1). No †/‡ reintroduced (AC 2.2).
• AC 2.3 shared-token property genuinely holds: marker is sourceReports.table.{split,depositReduced}InlineLabel — the same strings the row cell brackets, including the U+00A0 in less deposit / abzgl. Abschlag, so the legend token and the row token are byte-identical. Good call; a paraphrase here would have silently broken the join.
• Non-editable, both surfaces. ReportContentEditor.tsx:456-466 renders a read-only <ul>; overviewPdf.ts:852-868 appends marker: text verbatim after the skip block, so *N skip numbering is untouched (AC 3.3) and the legend is not in the user-editable cover letter. One builder (buildOverviewContent) is the only PDF consumer, so there is no second path that would silently omit the legend.
• Both locale keys retained and resolve (en/budget.json:1262-1263, de/budget.json:1252/1263); the realRender assertion runs against real i18next bundles per locale.

Findings

HIGH 1 — E2E Scenario 18 now asserts the opposite of the shipped behaviour

e2e/tests/budget/reportWizardEditableContent.spec.ts:1722-1730 (Scenario 18, two split invoices on the reported source, each with budget lines):

await expect(wizard.footnotesBlock).toHaveCount(0);
await expect(wizard.footnoteItems).toHaveCount(0);
...
expect(pageText).not.toContain('Amount shown reflects only the portion allocated to this source.');

With this PR those rows put splitInvoiceIds.size > 0, so the preview renders the footnotes block containing exactly that sentence. All three assertions fail, on desktop + tablet + mobile. E2E Gates is main-only, so this merges green into beta and then blocks the next promotion — please do not leave it for /fix-e2e.

Fix it as this PR, and invert rather than delete: assert footnoteItems has count 1, that its text is the split sentence, and that the sentence occurs exactly once in main's text despite two split rows. That single edit also discharges AC 1.5's explicit "counting occurrences in the rendered preview DOM … not only by inspecting the array length" and gives AC 3.1 its preview-side leg.

Scenario 17 (:1628) is fine as-is — that fixture's source B has zero budget lines and its deposit is tagged, so neither set is populated. Worth adding a one-line comment saying why it still expects zero now that a producer exists, otherwise the next reader assumes it is stale too.

HIGH 2 — the page-object contract instructs future agents to do the wrong thing

e2e/pages/ReportWizardPage.ts:472-477: "As of Issue #1959 NOTHING populates content.footnotes any more … Kept purely as a NEGATIVE guard … Never assert a positive count on these." Same claim at :208 and :223-227, and in the spec header at reportWizardEditableContent.spec.ts:73-88.

These are now false, and unlike a stale comment they are a directive — the next agent touching this POM will follow them. Update all four sites to describe the reinstated legend (shared, unnumbered, one entry per flag, joined to rows by the inline-label token) and drop the "never assert a positive count" rule.

MEDIUM 3 — the new assertions are weaker than the ACs require

• realRender.test.ts:1015-1016: expect(footnotes[0].marker).not.toContain('†') is a negative that passes on an empty string. Assert the property that actually matters and that AC 2.3 names: expect(content.footnotes[0]!.marker).toBe(content.labels.splitNote) and …[1]!.marker).toBe(content.labels.depositReducedNote) — that pins the row↔legend shared token, and would catch a future paraphrase or a lost NBSP.
• AC 1.5's dedup evidence is array length only (buildReportContent.test.ts:510). Add an occurrence count in the rendered output: collectAllStrings(pdfContent).filter(s => s.includes(sentence)).length === 1 on a fixture with ≥2 split rows, plus the preview-DOM count from HIGH 1.
• AC 1.4's "neither sentence appears in the preview DOM or in the rendered PDF's extracted text" has no rendered-surface negative — only footnotes).toEqual([]). One collectAllStrings negative on a no-flag fixture closes it.
• AC 4.1 (measured real render with both sentences present) is not covered. Real risk is low — this is a free-flowing stack of style: 'small' text that pdfmake wraps, not a dontBreakRows table cell where overflow is silently dropped — so I am not blocking on it, but say so explicitly in the PR body rather than leaving the AC silently unmet.

MEDIUM 4 — wiki debt this PR reopens (ADR-034)

wiki/ADR-034-Client-Side-Report-PDF-Generation.md:12 describes the block as "footnotes for documents that could not be attached", and :119 states the generalizable rule "every footnote is referenced from the row that owns it". The reinstated legend is deliberately not row-referenced — it is a document-level legend joined by a shared literal token. Correct both lines (the footnote block now holds skip notes and a shared legend; the B4 rule applies to numbered markers only) and append a Deviation Log row. This also discharges the ADR-034 correction owed since #1959, so it belongs here rather than in yet another follow-up.

LOW 5 — stale in-code comments

• client/src/lib/reportPdf/overviewPdf.ts:830: "Add footnotes (skip block only; split/deposit annotations are now rendered inline)" — no longer true, the block below it appends the legend at L852.
• client/src/components/reports/ReportContentEditor.test.tsx:938-955: the fixture uses marker: '†'/'‡' and id deposit-reduced, none of which the producer can emit any more (ids are split/depositReduced, markers are the inline labels). It is synthetic so it still passes, but it models an impossible shape in the one test that pins the preview renderer — align it with the real producer output.
• client/src/lib/reportContent/types.ts:85: footnotes is the only field in that interface with no comment. Add one stating it is a report-level, non-editable legend (one entry per flag, unnumbered) — that is the constraint Report legend: reinstate the explanatory sentences for partial and deposit-reduced amounts #1965 spent a paragraph on and the next refactor should not have to rediscover it.

INFO — inherited proxy limitation

The legend inherits the isSplit && budgetLines.length > 0 / && deposits.length > 0 proxy: an invoice whose lines are all on source A with a deposit tagged to source B renders the split sentence where the deposit-reduced one is arguably more accurate. Not caused here, tracked as #1911 (splitKind); the sentence wording may need a third variant when that lands.

Requested changes: HIGH 1, HIGH 2, MEDIUM 3, MEDIUM 4. LOW 5 is welcome in the same push.

[steilerDev]

[product-owner]

Verdict: CHANGES_REQUIRED

Reviewed 610303ac against the 12 acceptance criteria on #1965. The production change is correct and minimal — the legend gating is exactly right, and I found no defect in the derivation. What blocks this is that the assertion method AC 1.5 explicitly rules out is the only one used, and the surface AC 1.4/3.1 name (the preview DOM) has no coverage at all. That is not bureaucratic: the missing assertion would have caught a live rendering defect that ships in this PR (finding 1).

What I verified as met

AC	Verdict	Evidence
1.1	met	buildReportContent.ts:235-241 — one entry, text: reportT('sourceReports.table.splitFootnote'), no vendor prefix, marker is the bare inline label (no numbering)
1.2	met	buildReportContent.ts:242-248, same shape
1.3	met	push order is split → depositReduced; asserted in buildReportContent.test.ts AC 1.3 case
2.1	met	diff touches no label key, no cell-render code, no CSS. labels.splitNote / depositReducedNote / attachmentType.deposit untouched
2.2	met	realRender.test.ts:1046-1047 asserts †/‡ absent across the whole collected tree, with a positive control ahead of it; new marker values are partial / less deposit
2.3	met	marker resolves to partial / less deposit, rendered as partial: <sentence>; the row cell renders (partial). Shared literal present in both, no glyph needed — exactly the join the AC illustrated
3.2	met	both keys retained in en/budget.json:1262-1263 and de/budget.json:1252,1263; no locale file touched; German wording unchanged (Abschlagszahlungen in full)
3.3	met	skip block still emitted first (overviewPdf.ts:834-850) with the legend appended after; overviewPdf.test.ts:1465-1520 unchanged and still passing

Also confirmed the legend stays non-editable and report-level, as the story required: ReportContentEditor.tsx:458-464 renders it as plain <li> text (not an EditableField), and both applyOverrides.ts:27 and applyAiContent.ts:26 copy footnotes through without exposing an override key. The separately sentence therefore cannot be deleted by a user — which was the whole point of keeping it out of the cover letter.

buildReportContent.test.ts + ReportContentEditor.test.tsx: 149/149 green locally. Static Analysis and Trailer Check green; unit shards and E2E still running at review time.

---

Blocking findings

1. HIGH — the preview renders partial:Amount shown… with no space after the marker; the PDF renders partial: Amount shown…

ReportContentEditor.tsx:461-462:

<span className={styles.footnoteMarker}>{note.marker}:</span>
{note.text}

The whitespace between </span> and {note.text} contains a newline, so JSX strips it entirely, and .footnoteMarker (ReportContentEditor.module.css:273-276) sets only font-weight and color — no trailing margin or ::after. The PDF side does have the space (overviewPdf.ts:857: `${footnote.marker}: ${footnote.text}`).

This code path was unreachable in production while footnotes was always empty; this PR is what makes it reachable, so it lands here. It matters more than a normal spacing nit because the preview's entire purpose is to show the recipient-facing document before export, and this is a document going to a bank. Fix is one line — an explicit separator in the JSX or a token-based margin-inline-end on .footnoteMarker; implementer's call, but tokens only.

2. HIGH — AC 1.4, AC 1.5 and AC 3.1 are asserted at the array level only, which AC 1.5 explicitly excludes

• AC 1.5 reads "asserted by counting occurrences in the rendered preview DOM and in the rendered PDF text, not only by inspecting the array length." The added test (buildReportContent.test.ts, AC 1.5 case) asserts toHaveLength(1) and nothing else.
• AC 1.4 requires that with neither flag set, "neither sentence appears in the preview DOM or in the rendered PDF's extracted text." The added test asserts content.footnotes is [] and stops there.
• AC 3.1 requires the legend to render identically on both surfaces. realRender.test.ts:1055-1057 asserts the deposit sentence is present in the rendered PDF tree — good, and the right pattern — but the split sentence is never asserted present in any rendered output, and the preview surface has no legend assertion with real content anywhere.

I am not treating this as a checklist item. #1965 exists because #1959 emptied footnotes[] and the only guard in the suite was expect(content.footnotes).toEqual([]) in this same file — an array-level assertion that was simply updated to match the removal. Re-landing the fix with array-level assertions for one sentence and rendered-output coverage for the other leaves the identical regression channel open on the split sentence and on the whole preview surface. Finding 1 is the proof: an assertion that read the rendered marker-plus-sentence string off the preview DOM would have failed on this very commit.

Needed (routes to qa-integration-tester):

• one real-render assertion per sentence, per locale, that the joined marker: text string appears in the rendered PDF tree — not just footnotes[n].id;
• the same two sentences asserted in the rendered preview DOM, built from buildReportContent output rather than a hand-written fixture;
• occurrence counts for the N>1 case (currently the fixture proves array length, not render count);
• the negative case asserted on both rendered surfaces, not on the array.

Non-blocking (fix before merge or file as follow-up)

3. MEDIUM — AC 4.1 has no measured layout assertion. No test renders a report with both flags present and measures the legend block against the printable area. Real overflow risk is low (two short, space-wrappable sentences; longest unbreakable token is Abschlagszahlungen. at ~19 chars), but the AC asked for measured, not reasoned, and the machinery is already in the file: PRINTABLE_WIDTH_PT = 515.28 (realRender.test.ts:657) and the laid-out-node y-position pattern at realRender.test.ts:2000-2030. I will accept a bounded version of this — both sentences present, every legend text node within the printable box, page count sane — rather than a full sweep.

4. LOW — ReportContentEditor.test.tsx:938-955 still feeds † / ‡ as footnote markers. Production markers are now partial / less deposit, so the preview footnote path is only ever exercised with fixture values that no longer occur. That fixture is precisely why finding 1 was invisible; updating it is most of the AC 3.1 preview work.

5. LOW / informational — buildReportContent.ts:236,244 calls reportT('sourceReports.table.splitInlineLabel') directly rather than reusing labels.splitNote. Same key and same reportT, so there is no divergence today, and the labels object is constructed later in the function (L293-295), so reuse would need a reorder. Recording it only because AC 2.3's shared-token guarantee now depends on two call sites agreeing.

6. LOW — the N>1 dedup case is covered for split only, not for depositReduced. Same code shape, so low risk; worth a second case while finding 2 is being addressed.

Scope

Clean. Additive, producer-only, four files, no drive-by changes, no scope creep. The agent-memory file is expected per CLAUDE.md. reportT (report language) is used rather than the UI t, consistent with the rest of the builder.

[steilerDev]

[product-architect] Re-review — #1979 (story #1965), HEAD d360e1d8

Verdict: CHANGES_REQUIRED — narrow and mechanical. Three of my four requested changes are fully discharged and the production code is unchanged and still correct. What remains is the other half of HIGH 2: two of the four enumerated docstring sites still carry the "nothing populates content.footnotes" directive, and Scenario 18's test title now states the opposite of what its body asserts. All three are comment/title edits in files this PR already touches.

Verified fixed

HIGH 1 — E2E Scenario 18 inverted. Discharged, and better than I asked for.
reportWizardEditableContent.spec.ts:1717-1735 now asserts footnotesBlock count 1, footnoteItems count 1, and pageText contains the split sentence. The footnoteItems).toHaveCount(1) on a two-split-row fixture is exactly AC 1.5's "count occurrences in the rendered preview DOM" leg, so it also discharges MEDIUM 3's dedup-in-rendered-output ask — better evidence than the collectAllStrings filter I proposed. I checked the fixture: seedSplitInvoice ×2, no deposits seeded, so depositReducedInvoiceIds stays empty and 1 (not 2) is the right expectation. I also confirmed the locator can't over-count — [class*="footnotes"] matches the single .footnotes wrapper (ReportContentEditor.tsx:457) and does not match .footnoteMarker. Scenario 17 also got the "why it still expects zero" comment I asked for (:1625-1631), and its footnotesBlock).toHaveCount(0) on a rendered surface closes AC 1.4's rendered-negative gap.

MEDIUM 3 — assertions strengthened correctly.
realRender.test.ts:1018-1023 replaces both not.toContain negatives with toBe(content.labels.splitNote) / toBe(content.labels.depositReducedNote). I traced the producer to confirm this is the assertion that matters and not a tautology: buildReportContent.ts:238/245 sets marker from sourceReports.table.{split,depositReduced}InlineLabel and :294-295 sets labels.{splitNote,depositReducedNote} from the same two keys — so the equality genuinely pins AC 2.3's row↔legend shared token, NBSP included, and a paraphrase of either side now fails. The added exact-text assertions against real per-locale i18next bundles (:1022-1023, both en and de sentence literals) plus the allStrings presence check for the split sentence (:1064-1066) give AC 3.1 its rendered leg in both locales.

MEDIUM 4 — ADR-034 corrected thoroughly. Wiki submodule bumped on-branch to f9cd21c8 (03ed804), so the ref rides this PR as required. The B4 rule no longer says "every footnote is referenced from the row that owns it" — it now distinguishes (a) numbered row-owned *N notes from (b) document-level legend entries keyed by a repeated inline label, and states explicitly that "one note per owning row" applies only to (a). Intro bullet 2 was corrected too (I only asked for :119), the new "legend is document-level and deduplicated" addendum records the cardinality bound, the label-not-identifier property, the link-by-repetition rule, and the "add a flag type in exactly one place" checklist, and there is a proper Deviation Log row. This also discharges the ADR-034 correction owed since #1959. Nothing further needed here.

Bonus, correctly caught: the JSX whitespace fix at ReportContentEditor.tsx:461 (</span> {note.text}). I missed this — once markers became words instead of †, the preview rendered partial:Amount shown… while the PDF path (overviewPdf.ts:857, ${marker}: ${text}) had the space, so this was a real preview/export parity break. Good find.

POM field declaration + spec header (2 of 4 HIGH 2 sites) are correctly rewritten and now describe the reinstated behaviour, including which scenarios expect zero vs one.

Remaining — HIGH 2 (residual)

My original finding named four sites. Two are fixed; the two with the strongest wording are not.

1. e2e/pages/ReportWizardPage.ts:206-209 (Story #1923 paragraph) — still ends "…and footnotesBlock/footnoteItems (declared above) consequently have no producer left." A producer exists again.

2. e2e/pages/ReportWizardPage.ts:223-227 (Issue #1959 paragraph) — still reads:

buildReportContent.ts no longer pushes any ReportContentFootnote at all … retained below purely as negative guards — they can never be populated by the current code path, and any test asserting a marker glyph or footnote <li> is asserting a superseded design.

This is the site that matters most. The spec header (:73-88) points readers here — "See ReportWizardPage.ts's class docstring for the full locator reference" — so this paragraph is the canonical contract for these two locators, and it instructs the next agent to treat a footnote-<li> assertion as superseded design. The only such assertion in the suite is the one this PR just added to carry AC 1.5/3.1.

3. reportWizardEditableContent.spec.ts:1644-1646 and the test title at :1649 — the block header still says "…and there is no footnote list at all", and the Playwright test name still ends "and no footnote list anywhere on the page" while the body asserts toHaveCount(1) twice. A test whose name states the inverse of its assertions is worse than a stale comment: it shows up that way in every CI report, and it is the first thing a future reader uses to decide the body is the mistake.

Why I'm not waving these through. Concrete failure path: agent reads the POM directive → opens Scenario 18 → sees a title claiming no footnote list over a body asserting one → concludes the body drifted → reverts to toHaveCount(0). That silently deletes this PR's only preview-side dedup evidence, and because E2E Gates is main-only it lands on beta unnoticed and surfaces as a promotion blocker. This is the same mechanism that produced #1965 in the first place — #1959 removed a producer and left behind capability comments asserting permanence; leaving two fresh copies of the inverse claim repeats it one issue later.

Fix: reword sites 1 and 2 to match the (already correct) field-declaration comment at :472-479, delete the "never assert a positive count" / "superseded design" directive, and retitle Scenario 18 to something like …with no legacy †/‡ marker and exactly one deduplicated legend entry.

Non-blocking follow-ups (welcome in the same push)

• LOW client/src/lib/reportPdf/overviewPdf.ts:830: "Add footnotes (skip block only; split/deposit annotations are now rendered inline)" — contradicted 22 lines below by the legend loop at :851-868.
• LOW client/src/lib/reportContent/types.ts:85: footnotes is still the only field in ReportContent with no comment. One line stating it is a report-level, non-editable, unnumbered legend with at most one entry per flag would keep the ADR-034 addendum discoverable from the type.
• LOW the whitespace parity fix at ReportContentEditor.tsx:461 has no regression guard — getByText('partial:') and the separate text regex both pass with or without the space. One assertion on the <li>'s normalized text (partial: Amount shown reflects only the portion allocated to this source.) pins it.
• INFO AC 4.1's measured real-layout check is still not covered; as I said originally I'm not blocking — this is a free-flowing stack of style: 'small' text that pdfmake wraps, not a dontBreakRows cell where overflow is silently dropped. Please state that explicitly in the PR body so the AC isn't silently unmet.
• INFO the legend inherits the isSplit && budgetLines.length > 0 proxy limitation tracked as Source report: add splitKind to distinguish deposit-driven splits from line splits #1911 (splitKind); when that lands the sentence wording likely needs a third variant.

Everything else — producer placement, dedup-by-construction, ordering, additive-only behaviour, *N numbering isolation, single PDF consumer, both locale keys resolving, trailers on 20dd6f4e (frontend-developer for the .tsx change, e2e-test-engineer for e2e/) — re-verified and clean. Push the three doc/title edits and I'll approve immediately.

[steilerDev]

[product-owner] — round 2

Verdict: APPROVED

Re-reviewed d360e1d8 (fix commits 4d936609 + 20dd6f4e) against the round-1 blocking findings and all 12 acceptance criteria on #1965. Both HIGH findings are fixed, and one of them is fixed better than I asked for. All 12 E2E shards and Quality Gates are green on the head commit; ReportContentEditor.test.tsx + buildReportContent.test.ts are 149/149 locally.

Two cheap MUST FIX items below — neither is an acceptance-criteria gap, and neither blocks the verdict.

---

Round-1 finding 1 (HIGH — preview rendered partial:Amount shown…): FIXED

ReportContentEditor.tsx:461:

<span className={styles.footnoteMarker}>{note.marker}:</span> {note.text}

The separator is now on the same line as the closing tag, so JSX preserves it as a text node (JSX only strips whitespace that spans a newline). The preview now renders partial: Amount shown…, matching overviewPdf.ts:857's `${footnote.marker}: ${footnote.text}`. Verified on disk, and the path is now exercised for real in a browser — see below.

Round-1 finding 2 (HIGH — array-level-only assertions on AC 1.4 / 1.5 / 3.1): FIXED

The core of this finding was that the exact regression channel which produced #1965 — an array-level assertion that gets updated when the producer stops producing — was left open on the split sentence and on the whole preview surface. It is now closed:

What I asked for	What landed
Both sentences asserted present in rendered output, per locale	realRender.test.ts:1061-1066 — deposit and split sentence asserted in the collected rendered PDF tree, en and de
No key-echo / no fixture-agreement escape	realRender.test.ts:1017-1023 — footnotes[0].text / [1].text compared to the literal English and German sentences, against real i18n bundles
Occurrence count for the N>1 dedup case, in the rendered preview DOM	reportWizardEditableContent.spec.ts:1730-1737 — Scenario 18's fixture has two split invoices and now asserts footnotesBlock count 1 and footnoteItems count 1, plus the sentence present in main's text
Negative case on a rendered surface	Scenario 17 (:1632) asserts footnotesBlock count 0 in a real DOM for the constituted-deposit row
Fixture markers stop being †/‡ (round-1 finding 4)	ReportContentEditor.test.tsx:938-961 — now partial / less deposit

Two things landed beyond the ask and are worth calling out:

• realRender.test.ts:1019-1021 replaced the weak expect(marker).not.toContain('†') with expect(footnotes[0].marker).toBe(content.labels.splitNote). That is now a direct assertion of AC 2.3's shared-token property — legend marker and row label proven byte-identical (including the deliberate U+00A0) rather than inferred from two call sites happening to use the same key. This closes round-1 finding 5 as a risk.
• ReportWizardPage.ts:469-479 no longer instructs future authors "Never assert a positive count on these." That page-object comment was the contract-level half of the problem — a locator documented as negative-only is a locator nobody will ever use to catch the legend disappearing again.

Credit where due: the rendered-preview occurrence count arrived as the fix for product-architect's HIGH-1 (Scenario 18 asserting the opposite of shipped behaviour). It happens to be precisely the AC 1.5 coverage I was asking for, and on a better surface than the jsdom fixture I proposed. Verified executed, not just written: E2E shard 2/16, test 161 — [desktop] reportWizardEditableContent.spec.ts:1649:3 — passed.

---

Acceptance criteria — final state

AC	Verdict	Notes
1.1	met	verified round 1
1.2	met	verified round 1
1.3	met	order pinned in buildReportContent.test.ts:485-487 and realRender.test.ts:1013-1016
1.4	met	array negative (:494, :524) + rendered preview DOM negative (Scenario 17). Rendered-PDF-text negative → #1980
1.5	met	rendered preview DOM count asserted with N=2 → 1. PDF-text count → #1980
2.1	met	verified round 1 — still additive, no label/cell/CSS change
2.2	met	realRender.test.ts glyph absence with positive control, retained
2.3	met	strengthened — marker/label identity now directly asserted
3.1	met	both sentences, both locales, real render; preview surface covered in a real browser (split) and in jsdom (both)
3.2	met	both keys retained, no locale file touched
3.3	met	skip block still first, *N numbering untouched (overviewPdf.ts:834-869)
4.1	not met	no measured layout assertion. Accepted as a tracked follow-up, not waived — see #1980 AC 1

On AC 4.1: I flagged this as non-blocking MEDIUM in round 1 and said I would accept a bounded version. It was not addressed, so rather than let "approved with follow-ups" absorb it, it is now criterion 1 of #1980 (tech-debt, Should Have, Backlog) together with the three residual assertion gaps and the missing deposit-reduced dedup case. The residual gaps are each covered transitively today — the array length is pinned and overviewPdf.ts's verbatim append is pinned by its own case, so a duplicate or a deletion cannot reach the PDF without failing something — which is why they are follow-ups rather than blockers.

---

MUST FIX before merge (not verdict-blocking, both one-liners)

M1 — the separator that finding 1 just fixed is unguarded. ReportContentEditor.test.tsx:954-961 asserts getByText('partial:') and getByText(/Amount shown…/) as two independent matches, and the E2E asserts the sentence against main.textContent() — where partial:Amount and partial: Amount are indistinguishable. Re-removing the space regresses silently, which is the same shape of gap as finding 1 itself. Assert the joined string on the <li>:

const items = screen.getAllByRole('listitem');
expect(items[0]).toHaveTextContent('partial: Amount shown reflects only the portion allocated to this source.');
expect(items[1]).toHaveTextContent('less deposit: This position reflects deposits claimed separately.');

toHaveTextContent collapses runs of whitespace but never inserts any, so a missing space fails the match. The collapse also converts the marker's U+00A0 to a plain space — type the expected string with a plain space, which incidentally makes the /^less\sdeposit:$/ workaround at :961 unnecessary. Routes to qa-integration-tester.

M2 — two stale statements in the E2E spec now assert the opposite of the shipped behaviour. The file docstring and the in-body comments were updated; these two were not:

• reportWizardEditableContent.spec.ts:1643-1645 (section banner): "Every split invoice carries its own inline (partial) label and there is no footnote list at all"
• :1649-1650 (the test() title): "…with no †/‡ marker and no footnote list anywhere on the page"

The body now asserts footnotesBlock count 1. The title is the worse of the two because it is what appears in CI output and the Playwright report — shard 2's log line above literally advertises "no footnote list anywhere on the page" for a test proving one exists. This is the same class of problem as the page-object comment the architect flagged, and leaving it invites a future author to "fix" the assertions back to 0 — re-breaking the exact thing #1965 exists to repair. Routes to e2e-test-engineer.

---

Scope

Clean across both rounds. Producer-only, additive, no drive-by production changes. The E2E inversion is required by the behaviour change and stays within it. Wiki submodule bump (ADR-034 legend correction) and agent-memory updates are expected per CLAUDE.md. #1965 is correctly In Progress on the board.

One process note: the PR body's Test plan still describes only the round-1 state (buildReportContent.test.ts / realRender.test.ts); it does not mention the JSX separator fix or the E2E inversion. Worth a refresh before merge so the squash body reflects what shipped, but purely cosmetic.

[steilerDev]

[product-architect] Final re-review — APPROVED

All three blocking sites from my previous review are correctly fixed in 6053ac4f. Verified against the diff and the file contents at HEAD:

1. e2e/pages/ReportWizardPage.ts:206-209 (Story #1923 paragraph) — fixed.
The absolute "footnotesBlock/footnoteItems consequently have no producer left" is now time-scoped: "had no producer at this point, but see Issue #1965 below — that changed." The †/‡-glyphs-are-gone claim is preserved (still true), and the forward pointer prevents a reader stopping at the #1923 paragraph and concluding the locators are dead.

2. ReportWizardPage.ts:223-234 (Issue #1959 paragraph) — fixed.
"can never be populated by the current code path" / "retained purely as negative guards" is replaced with "were retained as negative guards only — until Issue #1965 reinstated the legend", plus a new dedicated Issue #1965 paragraph that states the current contract precisely: populated when isSplit/isDepositReduced is true, block absent from the DOM entirely when neither flag appears, and one <li> per deduplicated flag type ("two split invoices produce exactly one <li> entry, not two"). The explicit note that toHaveCount(1) assertions "reflect the current design and are correct" closes the loop — a future author reading this cannot arrive at the superseded conclusion.

I cross-checked the DOM-absence claim against the producer: ReportContentEditor.tsx:456 gates on content.footnotes.length > 0 &&, so "absent from the DOM entirely" is accurate, not merely hidden.

3. e2e/tests/budget/reportWizardEditableContent.spec.ts:1643-1650 — fixed.
Both the section banner and the test title now say "produce exactly one deduplicated legend entry in the footnotes block (Issue #1965)". The stale "no footnote list anywhere on the page" / "superseded by Issue #1959" attributions are gone, and the title now matches what the body actually asserts (footnotesBlock → toHaveCount(1), footnoteItems → toHaveCount(1), plus the long-form sentence assertion). The surviving †/‡ negatives remain correct and are still paired with positives, so they cannot pass on an empty page.

M1 parity guard — verified adequate.
ReportContentEditor.test.tsx now locates the <li> via the already-proven partial: marker span (.closest('li')) and asserts toHaveTextContent('partial: Amount shown reflects only the portion allocated to this source.'). Because toHaveTextContent normalises whitespace on textContent before comparing, this fails on a missing or extra space between the marker <span> and the text node while staying NBSP-agnostic — which the two independent getByText assertions structurally could not detect. Guarding only the split <li> (not less deposit) is sufficient: both are emitted by the same JSX template at ReportContentEditor.tsx:461, so a spacing regression is a single-site defect.

Architecture/contract checks: no schema, API-contract, or shared-type surface touched; ADR-034's B4 rule and legend addendum plus the Deviation Log entry are on the wiki submodule ref carried by this branch; Trailer Check is green.

---

Non-blocking follow-up (medium — docstring structure, no functional impact):

The new Issue #1965 paragraph was inserted inside the #1959 section rather than after it, orphaning #1959's continuation. At ReportWizardPage.ts:

• L235-240 — "Instead, a split row appends a grey inline <span class*="inlineNote">…" is fix(reports): improve report PDF UX — paragraph breaks, inline meta, inline notes, column toggles #1959 content whose "Instead" refers back to L223's "markers … are GONE", but it now reads as a continuation of the Report legend: reinstate the explanatory sentences for partial and deposit-reduced amounts #1965 legend paragraph.
• L241-246 (row.areaText/attachmentsNote meta line) and L247-252 (column-visibility role="group") are also fix(reports): improve report PDF UX — paragraph breaks, inline meta, inline notes, column toggles #1959 bullets that now sit under the Report legend: reinstate the explanatory sentences for partial and deposit-reduced amounts #1965 heading.

The information is all correct and present — only the attribution is misleading. Fix by moving the Issue #1965: paragraph (L228-234) to after L252, so the #1959 section stays contiguous. Worth doing on the next touch of this file; not worth another CI cycle on its own.

Note: Static Analysis, Docker, and the six Test shards were still in_progress at review time — approval is conditional on Quality Gates going green, per the normal merge gate.

VERDICT: APPROVED

[github-actions]

🎉 This PR is included in version 2.13.1-beta.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 2.14.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀