feat(reports): AI-generated usage descriptions and cover letter for bank report wizard

[steilerDev]

Summary

• Server-side AI content generation (d8b47625): new POST /api/source-reports/generate-content endpoint issues one batched LLM call producing per-invoice usage descriptions and a cover letter subject/body in the requested report language. The server re-assembles all prompt context from the database itself (never trusts client-supplied invoice text), enforces a strict response schema with a validator (length caps, all-requested-ids-present), and persists nothing. Reuses the existing auto-itemize LLM client/config path end-to-end. Also fixes a pre-existing buildRequestBody bug where the anthropic provider profile hardcoded the extraction response schema for every call type.
• Wizard UI (59dda421): Step 4 gains an "Enable AI assistance" toggle, shown only when GET /api/config's new llmEnabled flag is true (absent entirely otherwise — never presented as available when it can't work). Step 5 gains an explicit "Generate with AI" button (no auto-generation) with spinner + elapsed-seconds progress, mirroring the existing auto-itemize UX. Generated content becomes a new editable baseline (never a silent override) — manual edits on top of it behave exactly like edits on the derived baseline, survive to every export path, and regenerating over unsaved edits warns before overwriting.
• Bug fix (Fixes reportContentGenerationService.ts imports non-existent schema exports work_items/household_items — crashes buildApp() entirely #1915, found by QA while adding tests): reportContentGenerationService.ts initially imported non-existent schema exports (work_items/household_items instead of workItems/householdItems), which broke buildApp() compilation everywhere. While fixing it, linked-item descriptions fed to the LLM prompt were also corrected to read from the work/household item's own description column (previously mistakenly re-reading the budget-record description already surfaced elsewhere in the report) — reportContentGenerationService.test.ts scenario 6c is the regression guard proving the two sources are independent.
• Full English + German translations for all new UI copy; unit/integration coverage (95%+) across the new service, route, and client helpers; E2E coverage for toggle visibility, batched generation, overwrite-confirm, and error paths.

Fixes #1901
Fixes #1915

Test plan

• Unit tests pass (95%+ coverage on new/modified files)
• Integration tests pass (server route + service, incl. auth/validation/error-taxonomy matrix)
• CI Quality Gates pass (typecheck, tests, build, audit)
• E2E coverage for toggle visibility, batched generation, overwrite-confirm modal, and 502/error paths (page.route-mocked, no new containers needed)

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 e2e-test-engineer (Sonnet 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 translator (Sonnet 4.5) noreply@anthropic.com

🤖 Generated with Claude Code

[steilerDev]

[ux-designer]

Design review of the client/src changes against the visual spec posted on #1901 (comment history also captured in .claude/agent-memory/ux-designer/feature-spec-history.md). Scope: ReportWizardPage.tsx, ReportWizardPage.module.css, Step4Settings.tsx (the only visually-relevant files in this diff — applyAiContent.ts/sourceReportsApi.ts are non-visual logic).

Spec placement decisions — all verified correct:

• "Generate with AI" is a standalone .aiGenerateRow above ReportContentEditor inside the Step 5 body, not folded into Step5Actions — correctly applies the mutates-vs-consumes-content litmus test from the spec.
• llmEnabled gating is absent-entirely ({llmEnabled && (...)} in Step4Settings.tsx), never disabled-with-tooltip.
• AI-as-baseline confirmed: runAiGeneration calls setOverrides({}) alongside setAiContent(result), and ReportContentEditor's isFieldEdited = (key) => key in overrides — so every AI-filled field reads isEdited: false immediately post-generation. No edited-dots appear until a real manual edit occurs. applyAiContent.ts is a pure, non-mutating overlay onto the derived baseline, matching the data-model note from the spec.
• Elapsed-seconds spinner pattern for an inline action: Spinner size="sm" color="muted" wrapped in <span aria-hidden="true">, exactly mirroring Step5Actions's existing per-button spinner treatment (not the full-page size="lg" pattern). Separate <p aria-live="polite"> caption for "Generating… ({{seconds}}s)" — correctly not combined with role="status" (would double-announce).
• Error surfacing: aiError renders via inline FormError, not a toast — correct per spec (retryable in-place action, not a terminal notice).
• Provenance note: single .aiGeneratedNote line after a successful run, not per-field AI badges — matches the spec's rejection of per-field badges clashing with the status Badge column.
• AI overwrite confirmation reuses the shared Modal component with the standard .modalFooter two-button layout — no new dialog chrome.

Token adherence: All three new CSS rules (.aiGenerateRow, .aiGeneratingCaption, .aiGeneratedNote) use only var(--...) tokens (--spacing-3/4, --color-bg-secondary, --color-border, --radius-lg, --font-size-xs, --color-text-muted) — no hardcoded hex/px values. Verified each token exists in tokens.css and is a semantic (Layer 2) alias that flips under [data-theme="dark"]. No new Layer-1 palette token references.

Dark mode: No issues found — everything routes through semantic tokens already covered by the dark theme block.

Accessibility: Checkbox/label association in Step4Settings.tsx (id="enableAiAssistance" / htmlFor) follows the same pattern as the pre-existing attachDocuments/includeCoverLetter checkboxes. Disabled state on the Generate button (disabled={isGeneratingAi}) prevents concurrent runs. Focus rings are inherited from the shared btnSecondary/btnPrimary classes (box-shadow: var(--shadow-focus)), not touched by this PR.

Responsive: .aiGenerateRow uses flex-wrap: wrap, consistent with the existing .buttonRow pattern elsewhere on this page — no dedicated breakpoint needed for this content length.

Component reuse: No new shared components introduced; correctly reuses Modal, FormError, Spinner, EditableField, Badge. Appropriate — this feature only needed page-local layout classes.

One informational, non-blocking nit: .aiGeneratingCaption and .aiGeneratedNote are byte-identical CSS rules (font-size: var(--font-size-xs); color: var(--color-text-muted); margin: 0;). Consider consolidating into one class (or reusing .optionHelper, which has the same declarations) — pure DRY cleanup, not a functional or visual issue.

Verdict: APPROVE

No token, dark-mode, accessibility, or responsive issues found. Implementation is a faithful, precise match to every placement/gating/baseline decision from the #1901 spec.

[steilerDev]

[security-engineer] Security review of PR #1916 (issue #1901: AI report content generation).

Verdict: APPROVE

No critical or high findings. The implementation follows the established LLM-integration security pattern from budgetExtraction (auto-itemize) closely, and the acceptance-criteria-driven tests explicitly assert the security-relevant guarantees.

Reviewed

• server/src/routes/sourceReports.ts (new POST /generate-content route)
• server/src/services/reportContentGenerationService.ts (new)
• server/src/services/budgetExtraction/openAICompatibleProvider.ts (generateReportContent, validateGenerateReportContentResult, callChatCompletion)
• server/src/services/budgetExtraction/prompts.ts (REPORT_CONTENT_SYSTEM_PROMPT, buildReportContentUserPrompt)
• server/src/errors/AppError.ts / server/src/plugins/errorHandler.ts (LLM error taxonomy, suppressDetails)
• server/src/plugins/config.ts, server/src/routes/config.ts (llmEnabled alias)
• client/src/lib/sourceReportsApi.ts, client/src/lib/reportContent/applyAiContent.ts, client/src/components/reports/ReportContentEditor.tsx, client/src/lib/reportPdf/coverLetterPdf.ts
• server/src/routes/sourceReports.generateContent.test.ts (612 lines, full matrix)

1. AuthN/AuthZ — OK

if (!request.user) throw new UnauthorizedError() matches the identical pattern used by the sibling GET / and POST /mark-claimed handlers in the same file — no admin/member split (consistent with the rest of this route group, both roles are permitted). Confirmed by test allows a member user to call the endpoint.

2. Input validation — OK

Fastify JSON-schema is tight: type/language are closed enums, sourceId and array items have minLength/maxLength, includedInvoiceIds requires minItems: 1, maxItems: 200, excludedLineIds capped at maxItems: 500, and additionalProperties: false (verified stripped via global removeAdditional, not silently accepted — test strips unknown/additional properties). All DB reads in reportContentGenerationService.ts go through Drizzle's query builder (inArray(invoices.id, invoiceIds) etc.) — fully parameterized, no raw SQL string interpolation.

IDOR-relevant defense-in-depth (good catch by the implementer): the client-supplied includedInvoiceIds/excludedLineIds are never trusted directly — the server re-fetches the report via getSourceReport() and filters includedInvoiceIds down to reportInvoiceIds (the report's actual invoice set) before doing anything else, and empty-after-filter triggers EmptySelectionError. This prevents a client from asking the LLM to describe invoices/budget lines outside the requested sourceId's report.

3. Prompt injection — adequate, residual risk accepted (informational)

REPORT_CONTENT_SYSTEM_PROMPT rule 7 explicitly labels all invoice-derived text (vendor names, notes, line/linked-item descriptions) as untrusted data and instructs the model to never follow embedded instructions. This is standard-practice mitigation, not a guarantee — prompt injection can't be fully eliminated by instruction alone. Two factors bound the blast radius even if injection succeeds:

• Output-side allowlisting: validateGenerateReportContentResult() rejects anything that isn't the exact {letterSubject, letterBody, descriptions[]} shape, enforces length caps (200/3000 chars server + 150/2000/200 prompt-requested), and requires every requested invoice ID to be present — an attacker can at best distort the wording of a description/letter, not exfiltrate data or change response structure.
• Rendering is plain-text everywhere: confirmed no dangerouslySetInnerHTML/innerHTML anywhere in the report content path; ReportContentEditor.tsx binds AI output via controlled value= props on <textarea>/<input>-style fields (React-escaped), and the PDF path (coverLetterPdf.ts) passes the text through pdfmake's text: node (drawn as literal text, not parsed as markup). No injected content can become executable HTML/JS or alter PDF structure.

No finding raised here — this is the correct architecture for the residual risk (untrusted-LLM-output-as-plain-text), not a gap.

4. Information disclosure — OK, verified by dedicated test

LlmUnreachableError/LlmInvalidResponseError/LlmUpstreamError all set suppressDetails: true (AppError.ts:353-372), and errorHandler.ts:46 only includes details in the response when !error.suppressDetails. The message field for each is a static string (e.g. `LLM upstream returned ${response.status}` — status code only, never body/prompt content). Test does not leak raw provider error details in the HTTP response (suppressDetails) asserts the response body does not contain the mocked upstream payload and error.details is undefined. The LLM API key is never placed into any details object at all (only Authorization header, never logged/returned). Confirmed no LLM credentials or prompt content reach the client on any code path.

5. SSRF / outbound — OK

getProvider() builds baseUrl exclusively from config.llmBaseUrl (server env var LLM_BASE_URL), which is set once at process start — no per-request or client-supplied URL ever reaches callChatCompletion()'s fetch(). Timeout is enforced via AbortController + setTimeout(..., config.llmRequestTimeoutMs), cleared on both success and failure paths. Consistent with the existing budgetExtraction provider (previously audited, unchanged).

6. No-persistence claim — verified

generateReportContent() performs only db.select()...where(inArray(...)).all() reads (invoices, work items, household items) — no insert/update/delete anywhere in the service. Confirmed further by the dedicated test does not write any DB rows (nothing persisted server-side), which snapshots invoices/invoiceBudgetLines row counts before/after the call.

7. Rate limiting / concurrency (non-blocking, informational)

The rate-limit plugin is global: false and no per-route limiter is attached to /generate-content (consistent with the sibling invoiceAutoItemize LLM route, which also has none — not a regression introduced by this PR). Client disables the "Generate" button while isGeneratingAi is true, but nothing stops a script from hammering the endpoint server-side, each call triggering a real outbound LLM request (cost amplification / self-inflicted DoS against the configured LLM provider). Given the stated threat model (1-5 trusted homeowners, self-hosted), this is low risk and does not block. Recommend tracking alongside the existing open recommendation to add rate limiting to sensitive/expensive routes (see Security Audit wiki, open recommendations list) rather than fixing ad hoc in this PR.

Informational: stale test-file comment

server/src/routes/sourceReports.generateContent.test.ts lines 8-15 carry a header comment claiming reportContentGenerationService.ts imports non-existent work_items/household_items schema exports and that buildApp() fails for every server test (referencing GitHub issue #1915). I diffed the two commits in this PR: commit d8b47625 did have that bug, but commit 59dda421 fixed the import to the correct workItems/householdItems names — confirmed against server/src/db/schema.ts's actual exports. The comment is now stale/inaccurate as of the current HEAD and should be removed or updated (and issue #1915 closed if it was tracking exactly this). Not a security vulnerability, but worth a quick follow-up so future readers don't mistrust the whole test file's premise.

---

Summary: No Critical/High findings. Auth parity confirmed, input validation tight, IDOR-safe (server re-derives invoice set from DB), prompt-injection mitigations reasonable given output is always rendered as plain text/PDF-text (never HTML), no secret/prompt/provider-payload leakage (test-verified), SSRF not applicable (env-only base URL), and the "no persistence" claim is test-verified. One informational note on rate limiting (matches existing sibling-endpoint posture, not a regression) and one informational note on a stale test comment.

[steilerDev]

[product-owner] Requirements review of PR #1916 against story #1901 (Bank report wizard — AI-generated usage descriptions and cover letter).

I verified all six AC sections against the implementation and tests on feat/1901-ai-report-content (d8b47625 + 59dda421). The shape of this story is right: one explicit button, one batched call, generated text as a new baseline under the #1900 override model, nothing persisted. Coverage of the behavioural criteria is genuinely thorough — the client test file alone walks 25 scenarios that map almost one-to-one onto the AC list.

However, I am returning this with CHANGES_REQUIRED. There is a functional defect in the numbers fed to the model that would put materially wrong currency figures into a document a homeowner sends to their bank.

---

Per-section coverage

1. Availability and opt-in — MET

Criterion	Status	Evidence
Toggle shown on Step 4 when LLM configured	MET	Step4Settings.tsx renders the enableAiAssistance block behind llmEnabled, sourced from the new GET /api/config flag
Hidden or disabled-with-explanation when not configured	MET	Hidden entirely — the section is not rendered. Covered by client test "hides the toggle entirely when llmEnabled is false" and E2E Scenario 1 against the real (LLM-less) container config
Toggle off → no Generate action, wizard behaves as #1900	MET	Step 5 AI row is gated on aiEnabled
Toggle on → Generate button offered	MET	E2E Scenario 2
No automatic generation	MET	Generation is only reachable from handleGenerateWithAiClick; asserted by "does NOT call generateReportContent automatically just from reaching step 5"

2. Batched generation — NOT MET (see Blocking 1 and 3)

Criterion	Status	Evidence
One batched call for the whole report	MET	Single POST /api/source-reports/generate-content → single provider.generateReportContent
Returns a description per included invoice + letter subject and body	MET	Validator rejects a response missing any requested invoice ID (LLM_INVALID_RESPONSE)
Per-invoice prompt draws on the invoice, its budget lines, and linked item names + descriptions	PARTIAL	Structure is right (see the adjudicated deviation below), but the invoice amount in the prompt is wrong — Blocking 1 and 3
Cover-letter prompt draws on source, report type, totals, invoice summary	NOT MET	All four inputs are present, but totalAmount is misscaled and rounded — Blocking 1 and 2
All output in the Step 4 report language	MET	reportLanguage (not UI locale) is sent; system-prompt rule 1 plus the explicit Language: line; route test asserts German forwarding
Step 3 exclusions not described and not in the prompt summary	MET	Client sends only non-excluded IDs; server re-filters against the report's own invoice set; service scenario 2

3. Generated text becomes an editable baseline — MET

Criterion	Status	Evidence
Results populate Step 5 fields as a new baseline	MET	applyAiContent is applied inside baselineContent, below the override layer — not as overrides
Edits behave like #1900 manual edits and survive every export path	MET	effectiveContent = applyOverrides(baselineContent, overrides) feeds generatePdfFromContent, which is the single source for preview, download, Paperless upload, and mark-claimed. Test: "a further manual edit on the AI-populated field shows a reset button, and resetting reverts to the AI text"
Regenerating over manual edits warns and can be cancelled	MET	Overwrite modal; "Keep Editing" and Escape both cancel with no call (client + E2E Scenario 4)
No warning when there are no manual edits	MET	Including the regenerate-after-a-prior-AI-run case (E2E Scenario 5)
Partial results leave ungenerated fields at their derived value	MET	applyAiContent skips undefined and ''; test "fields without generated content retain their derived baseline value"
A confirmed Step 1–4 change clears AI text along with manual edits	MET	guardedUpdate now treats aiContent !== null as dirty and clears it; covers report-language and cover-letter changes too. E2E Scenario 7

4. Progress and error handling — MET, with one copy defect (see Must Fix 4)

Criterion	Status	Evidence
Spinner + elapsed-seconds counter	MET	Spinner + generating key with {{seconds}}; fake-timer test asserts the increment
Button disabled during generation, wizard not silently frozen	MET	disabled={isGeneratingAi}; aria-live="polite" caption; rest of the wizard stays interactive
Unreachable/timeout → clear translated error, content preserved, retry possible	PARTIAL	Behaviour is correct and tested (content untouched, retry succeeds) — but the message reads "The extraction service is unavailable", naming a different feature. Must Fix 4
Not-configured → established error code + translated message	PARTIAL	LLM_NOT_CONFIGURED (503) is correctly raised and surfaced, but its copy says "Auto-itemization is not configured on this server." That does not tell me AI report drafting is unavailable. Must Fix 4
Invalid/unparseable response → clear error, no malformed or partial content written	MET	validateGenerateReportContentResult throws before anything reaches state; malformed-JSON and missing-invoice route tests
No raw provider payloads, prompts, or API keys shown	MET	Client reads only err.error.code; server suppresses details on LLM errors, with a dedicated leak test

5. Server endpoint — MET, except the wiki (see Must Fix 5)

Criterion	Status	Evidence
Rejected without a valid session	MET	401 test
Nothing persisted	MET	Service performs reads only; explicit "does not write any DB rows" test
Follows API conventions and is documented on the API Contract wiki	NOT MET (docs)	Route and envelope are correct; the wiki page is written but uncommitted — Must Fix 5
Server instructs the model in the supplied report language	MET	System-prompt rule 1 + Language: English/German
Reuses the established LLM config path	MET	getProvider(config) and the shared callChatCompletion; no parallel client
finishReason: "length" treated as a failure	MET	Handled in the shared callChatCompletion path, so it applies here by construction

6. Translations — MET

Every new string resolves in both locales: settingsStep.enableAiAssistance(.Helper), editable.generateWithAi, .generating, .aiGeneratedNote, .aiGenerationFailed, .aiOverwriteConfirmTitle, .aiOverwriteConfirmBody, .aiOverwriteAndGenerate, plus the new EMPTY_SELECTION error key. No hardcoded copy in the new JSX. German is internally consistent on "KI".

---

Blocking — must be fixed before merge

1. Every currency amount sent to the model is 1/100th of its real value.

buildReportContentUserPrompt (server/src/services/budgetExtraction/prompts.ts) treats the amounts as minor units:

const amountFormatted = (input.totalAmount / 100).toFixed(2);
...
const invAmount = (inv.amount / 100).toFixed(2);

But SourceReportInvoice.allocatedAmount is a major-unit real ("rounded to 2dp" per shared/src/types/sourceReport.ts), fed straight to Intl currency formatting on the client — buildReportContent.test.ts asserts allocatedAmount: 250 renders as €250.00. So a €12,500.00 invoice is presented to the model as "125.00 EUR", and a €733.34 report total as "7.33 EUR".

The system prompt then instructs the model "Do NOT invent or alter amounts or dates" — so it will faithfully copy the wrong figure into the cover letter body. The PDF table renders the correct number alongside it, so the exported bank document contradicts itself. This fails AC 2.4 (the cover letter must draw on the totals) and weakens AC 2.3.

Fix: drop both / 100 conversions. No test currently asserts the rendered amount string, which is why this got through — please add a regression guard asserting the formatted total and a per-invoice amount appear in the user prompt at full value.

2. includedTotal is rounded to whole currency units, not to cents.

reportContentGenerationService.ts line 83:

// Round to nearest cent
includedTotal = Math.round(includedTotal);

On major units this rounds to whole euros — the comment and the code disagree. reportContentGenerationService.test.ts scenario 5 is titled "rounded to the nearest cent" yet asserts 733 for a true total of 733.335. The test encodes the defect rather than the contract, so it must be corrected together with the code (Math.round(x * 100) / 100).

3. Per-invoice amount ignores excludedLineIds while totalAmount honours them.

The includedTotal loop subtracts each excluded line's allocatedPortion, but the value pushed into llmInvoices is the raw inv.allocatedAmount. With line-level exclusions in play, the model receives invoices that sum to more than the stated total, and each per-invoice figure is higher than what the report table shows for that same row (the client's applyLineExclusions adjusts allocatedAmount). Scenario 5's own fixture demonstrates it: invoice A is sent as 1000 while contributing 400 to the total.

Fix: send the exclusion-adjusted contribution — the per-invoice value the includedTotal loop already computes.

---

Must fix before merge (non-blocking, no re-review needed)

4. Shared LLM error copy names the wrong feature. All four codes reachable from this flow are worded for auto-itemization: LLM_UNREACHABLE / LLM_INVALID_RESPONSE / LLM_UPSTREAM_ERROR say "The extraction service …", and LLM_NOT_CONFIGURED says "Auto-itemization is not configured on this server." A user who just pressed "Generate with AI" in the report wizard has no idea what the extraction service is. Please reword all four in en and de to feature-neutral wording (for example "the AI service"), which serves the auto-itemize call sites equally well. AC 4.4 in particular asks for a message "telling me AI is unavailable".

5. The API Contract wiki edit is uncommitted. The POST /api/source-reports/generate-content section is written and accurate — request/response tables, behaviour notes, and a complete error-code matrix, including the "generated content is NOT persisted" statement. But the wiki submodule is at 616bc90 with API-Contract.md modified in the working tree only, so nothing is published yet and AC 5.3 is not satisfied as things stand. Please commit and push the wiki submodule (the ADR-034 edits sitting alongside it are the outstanding #1891 follow-up and can ride along).

---

Non-blocking observations

• buildReportContentUserPrompt opens with for a ${input.language === 'en' ? 'German construction project' : 'Konstruktionsprojekt'} financial report. When the report language is German this drops a single German noun into an otherwise English prompt, and "Konstruktionsprojekt" is not idiomatic (a native writer would say "Bauprojekt"). The language directive is explicit elsewhere so nothing is broken, but the ternary reads like leftover scaffolding and hardcodes an assumption that every project is German. Worth simplifying.
• llmEnabled is a straight alias of autoItemizeEnabled, so AI report drafting cannot be enabled independently of auto-itemization. That matches AC 1.1 as written ("an LLM is configured via the existing LLM_* environment configuration"), so I am accepting it — flagging only in case operators later ask for per-feature gating.
• The aiEnabled toggle is per-session and resets on reload. Consistent with "no persistence of generated text"; no action.

Adjudicated deviation — confirmed satisfied

AC 2.3 asks the prompt to draw on "the names and descriptions of the linked work items and household items". The implementation reads workItems.description / householdItems.description from the entity tables as a distinct signal, alongside the budget-record description already carried on the line. That is the literal reading of the criterion — the work item's own description — and it strictly increases the signal rather than substituting one source for another. Scenario 6c is the right regression guard, proving the two sources are independent. Accepted, no change needed.

Glossary

Agreed on the translator's flag: "AI" → "KI" deserves a glossary entry, and I am tracking it for the next glossary refinement pass rather than adding it mid-story. The German copy here already uses "KI" consistently ("KI-Unterstützung aktivieren", "Mit KI generieren", "KI-Generierung fehlgeschlagen"), which is the correct convention and is exactly what the entry should codify.

---

Verdict

CHANGES_REQUIRED — Blocking items 1, 2, and 3. All three sit in the same two files (prompts.ts and reportContentGenerationService.ts) and are numeric-accuracy defects in the figures handed to the model; for a bank-facing document they cannot ship. Items 4 and 5 are MUST FIX before merge but do not require another round with me.

Everything else across the six AC sections is met, and the baseline/override integration with #1900 is exactly right.

[steilerDev]

[product-architect]

Architecture review — PR #1916 (story #1901)

Verdict: CHANGES_REQUIRED — 1 blocking finding (2 linked defects at the same seam + the test gap that let them through), 9 non-blocking findings.

Reviewed against the Wiki Architecture, API Contract and Schema pages, ADR-034, and the forward guidance I recorded when reviewing #1900.

---

What is right (and worth keeping)

• The content-layering model is exactly correct. applyAiContent folds AI text into the baseline (buildReportContent → applyAiContent → applyOverrides) rather than into the overrides map. This was the specific trap I flagged coming out of Bank report wizard: editable HTML preview before export #1900: AI text dumped into overrides would have marked every generated field as user-edited, offered a reset-to-non-AI-text, and been silently wiped by guardedUpdate. Layer 2 is the right answer, and extending guardedUpdate's dirty check to aiContent !== null keeps the discard-guard honest. Documented in ADR-034 (see wiki note at the bottom).
• No second LLM path. generateReportContent is a third method on the existing BudgetExtractionProvider, going through the same callChatCompletion / buildRequestBody / provider-profile machinery. One gateway, one timeout policy, one error taxonomy.
• The buildRequestBody responseSchema fix is a genuine latent bug fix, not incidental refactoring: the anthropic branch previously hardcoded EXTRACTED_LINES_SCHEMA for every call, so summarizeMerge on an Anthropic-profile deployment was requesting structured output against the wrong schema. Making responseSchema a required field of RequestBodyInput means a future call site cannot forget it, and the named regression guard ('anthropic profile sends the REPORT_CONTENT_SCHEMA (not EXTRACTED_LINES_SCHEMA — bug-fix regression guard)') pins it. Good.
• Server-side context re-assembly. The client sends only selection (type, sourceId, language, includedInvoiceIds, excludedLineIds); the server re-fetches the report and intersects the ID list. Client-supplied vendor names or amounts can never reach the prompt. paperlessEnabled: false correctly avoids a pointless external round trip.
• Stateless by construction — the service performs no writes, and the endpoint is a pure read + LLM call. Now stated as a contract guarantee on the API Contract page.
• Prompt-injection hardening (system prompt rule 7) plus the defence-in-depth re-filter of descriptions to the accepted invoice IDs. LlmInvalidResponseError details are suppressed, so prompt content cannot leak back through the error envelope.
• Error taxonomy reuses existing codes — EMPTY_SELECTION, LLM_*, NOT_FOUND, VALIDATION_ERROR — no new codes invented, and all the documented HTTP statuses check out against AppError.ts (502 for the three LLM failure modes, 503 for LLM_NOT_CONFIGURED).

---

BLOCKING — B1: every monetary figure sent to the LLM is 100× too small

The unit convention breaks across the service → prompt seam.

allocatedAmount / allocatedPortion / totalAmount are major currency units rounded to 2 dp throughout the report pipeline — shared/src/types/sourceReport.ts documents allocatedAmount as "Net contribution to this source … rounded to 2dp", sourceReportService.ts:494 produces toCents(...)/100, and formatCurrency consumes major units.

reportContentGenerationService.ts passes those major-unit values straight through:

// reportContentGenerationService.ts:183
amount: inv.allocatedAmount,        // e.g. 1234.56 (euros)
// …and totalAmount: includedTotal  // e.g. 12345.67 (euros)

but prompts.ts divides both by 100, treating them as cents:

// prompts.ts:151
const amountFormatted = (input.totalAmount / 100).toFixed(2);   // 12345.67 -> "123.46"
// prompts.ts:165
const invAmount = (inv.amount / 100).toFixed(2);                // 1234.56  -> "12.35"

The system prompt explicitly instructs the model to "Reference the source name, report type …, total amount and currency", so this lands in the headline figure of a bank-facing cover letter: a €12,345.67 claim is described to the bank as 123.46 EUR. This is the same failure class as ADR-034 finding B3, which is why the ADR now carries an explicit monetary-unit invariant.

Linked defect B1b — Math.round drops the cents. reportContentGenerationService.ts:83:

includedTotal = Math.round(includedTotal);   // comment says "Round to nearest cent"

Math.round on a major-unit value rounds to the nearest whole euro, not the nearest cent. Even with the /100 removed, the letter's total would disagree with the overview table's total by up to €0.50 — and the letter and the table are in the same PDF. reportContentGenerationService.test.ts:479 (expect(input.totalAmount).toBe(733) for 733.335) currently encodes this behaviour, and its own title ("rounded to the nearest cent") contradicts the assertion. Per the Test Failure Debugging Protocol the contract wins, so the test changes with the code.

Required fix (keeps the codebase's major-unit convention — do not convert to cents):

1. prompts.ts:151 → input.totalAmount.toFixed(2); prompts.ts:165 → inv.amount.toFixed(2).
2. reportContentGenerationService.ts:73-83 → mirror applyLineExclusions + buildReportContent exactly so the letter total and the table total are the same number: round per invoice, then sum, with no final rounding —
   invContribution = Math.round((inv.allocatedAmount - excludedPortion) * 100) / 100, then includedTotal += invContribution. Drop line 83.
3. budgetExtraction/types.ts:30,40 → document the unit in JSDoc on GenerateReportContentLlmInvoice.amount and GenerateReportContentLlmInput.totalAmount ("major currency units, 2 dp — not cents"). The missing unit annotation is the root cause.

Linked test gap. buildReportContentUserPrompt has no test in prompts.test.ts — the only coverage is in openAICompatibleProvider.test.ts:1656, which asserts 'Language: German', the source name and 'Invoice ID: inv-1', i.e. everything except the numbers. The reported 95.94 % statement coverage on the new service is real but line coverage cannot catch a unit error; only an assertion on the rendered string can. Please add a prompts.test.ts case asserting the exact substrings Total Amount: 12345.67 EUR and Amount: 1234.56 EUR, and update reportContentGenerationService.test.ts scenario 5 to the cents-preserving expectation.

---

Non-blocking (medium — please fold into refinement rather than this PR unless trivial)

• M1 — llmEnabled is a permanent duplicate of autoItemizeEnabled with no deprecation path. Two always-equal required booleans on AppConfigResponse forced edits to eight-plus object literals in this PR alone, and getProvider() still gates the new feature on config.autoItemizeEnabled — so the report feature is switched by a flag named after auto-itemize. The alias is the right direction; the missing half is retiring the old name. Add @deprecated JSDoc to autoItemizeEnabled in shared/src/types/config.ts and file removal as a follow-up. I have recorded the "no third alias; divergence needs an ADR" rule on the API Contract page.
• M2 — duplicated included-total derivation. The same arithmetic now exists in client/src/lib/reportExclusions.ts + buildReportContent.ts and in reportContentGenerationService.ts. B1b is the first drift; there will be others. Extract the rule into @cornerstone/shared (computeIncludedTotal(report, includedIds, excludedLineIds)) so both sides import one implementation.
• M3 — the route returns a server-internal type. The handler returns GenerateReportContentLlmResult (from budgetExtraction/types.ts), not the shared GenerateReportContentResponse. They are structurally identical today, so drift is silent. The sibling handlers in the same file bind const response: MarkClaimedResponse = …; do the same here so a contract change becomes a compile error.
• M4 — BudgetExtractionProvider is now a general LLM gateway under a budget-extraction name. Three unrelated capabilities on one interface, and every test double across the server (backupService, draftCleanupService, three invoiceAutoItemize* suites) now has to stub generateReportContent. Consolidating on one gateway was the right call; the name and the module path should follow. Worth a rename to llmGateway/ (or splitting the interface per capability) as a standalone refactor — not in this PR.

Non-blocking (low)

• L1 — sourceId! non-null assertion in runAiGeneration (ReportWizardPage.tsx). Narrow it with the existing early return (if (!report || !useCase || !sourceId) return;).
• L2 — includedInvoiceIds.includes(...) is called inside four separate loops in reportContentGenerationService.ts; the Set is already built two lines above. Cheap at maxItems: 200, but use the Set.
• L3 — prompts.ts:154: ${input.language === 'en' ? 'German construction project' : 'Konstruktionsprojekt'}. The output language is being used to select a noun that describes the project, which is not what the ternary reads as; and Konstruktionsprojekt is not idiomatic German for a building project (Bauprojekt). Suggest a single language-independent phrase.
• L4 — errors.json: LLM_UNREACHABLE / LLM_UPSTREAM_ERROR / LLM_INVALID_RESPONSE still read "The extraction service …". They now surface in report generation too; reword generically ("The AI service …").

E2E

Agreed, unrelated. Shard 5's only non-recovering failure is tests/invoices/invoices.spec.ts:841 ("Effective Amount" column, Issue #1876) with tests/navigation/dashboard.spec.ts:566 passing on retry — both are the known pre-existing flakes already on record as promotion blockers for E2E Gates, and neither touches the report wizard. Quality Gates is green, which is the required gate for a beta PR. The new e2e/tests/budget/reportWizardAiGeneration.spec.ts passed in its shard.

Wiki (pushed)

Wiki master 254db1d, submodule ref bumped on this branch in cddfd07b:

• API Contract — the pending POST /api/source-reports/generate-content section was accurate against the implementation (validation bounds, the four behavioural guarantees, and all eight error rows verified against AppError.ts and sourceReportService.ts). Amended with the two-tier length-cap explanation (prompt asks 150/2000/200, validator truncates at 200/3000/300), the descriptions-keying caveat, the llmEnabled gating guidance, and the llmEnabled vs autoItemizeEnabled rule from M1.
• ADR-034 — the pending edits were a Prettier pass only, with no content for Bank report wizard: editable HTML preview before export #1900/Bank report wizard: AI-generated usage descriptions and cover letter #1901. Added two substantive addenda: report-language decoupling from the UI locale (Bank report wizard: Settings step with report language selection #1899, an outstanding debt), and the baseline → AI → user-overrides content layering (Bank report wizard: editable HTML preview before export #1900/Bank report wizard: AI-generated usage descriptions and cover letter #1901) including the rationale for keeping AI out of the overrides map and the monetary-unit invariant from B1.

The remaining ADR-034 corrections tracked under #1914 (width-assertion rule, stale module table) are deliberately not in this push.

[steilerDev]

[product-owner] Re-review of PR #1916 (story #1901) — round 2, against b70d821b.

All five findings from my round-1 review are resolved. I verified each one against the code on disk rather than the commit message, and ran the new prompt tests locally.

Blocking items — all fixed

1. Currency scaling — FIXED. Both / 100 conversions are gone from buildReportContentUserPrompt; input.totalAmount.toFixed(2) and inv.amount.toFixed(2) now render major units verbatim. The regression guard is real and well-targeted: prompts.test.ts gained a dedicated describe('amount formatting (major units — regression guard for the ×100 division bug)') block asserting 12345.67 → "12345.67", a per-invoice 999.99 → "999.99", two-decimal formatting for whole numbers, and the currency code rendering adjacent to both the total and the per-invoice amounts. That last one matters — it pins the unit and its label together. I ran the file locally: 98/98 pass.

2. Cents rounding — FIXED. Math.round(x * 100) / 100 at both levels. Scenario 5 now asserts 733.34 and its title matches its assertion.

3. Per-invoice exclusion adjustment — FIXED. invoiceAmountsAdjusted is populated in the same loop that builds includedTotal, so the per-invoice figure and the total can no longer drift from one another by construction. Scenario 5 asserts invoice A at 400 (not its raw 1000) and invoice B at 333.34, with an inline comment recording why.

Worth calling out as a bonus: rounding each invoice to cents before summing makes the server's math identical to the client's applyLineExclusions (Math.round((allocatedAmount - excludedPortion) * 100) / 100 per invoice, then summed). The figures the model sees and the figures the PDF table renders are now derived by the same rule, which is a stronger guarantee than the AC strictly required.

Must-fix items — both done

4. Error copy — FIXED, and better than I asked for. All four codes are feature-neutral in both locales:

• LLM_NOT_CONFIGURED: "AI assistance is not configured on this server." / "KI-Unterstützung ist auf diesem Server nicht konfiguriert."
• LLM_UNREACHABLE / LLM_INVALID_RESPONSE / LLM_UPSTREAM_ERROR: "The AI service …" / "Der KI-Dienst …"

The three transient ones also gained an explicit "Please try again." / "Bitte versuchen Sie es erneut.", which directly strengthens AC 4.3's "and I can retry". The German uses "KI-Unterstützung" and "KI-Dienst" — exactly the convention the translator flagged, so the pending glossary entry now has a second call site backing it.

5. Wiki — FIXED. wiki submodule is clean, HEAD 254db1d equals origin/master (so it is pushed, not just committed), and the parent repo's pointer was bumped in cddfd07b. AC 5.3 is satisfied on the published page.

Test-integrity check

I checked the 9 lines removed from sourceReports.generateContent.test.ts — they are the stale header note describing the (now-fixed) #1915 import bug, not a weakened assertion. No test was relaxed to accommodate the fixes; scenario 5 was tightened, not loosened.

CI

Quality Gates passes on b70d821b. E2E Tests (Shard 5/16) fails, but it failed identically on 59dda421 before any of these fixes landed — it is the pre-existing shard-5 flake already tracked as a beta → main promotion blocker, not a regression from this PR, and E2E Gates is not a required check for beta. It still needs triage before promotion.

Remaining non-blocking notes (carry-forward, no action required here)

• The prompt still opens with for a ${language === 'en' ? 'German construction project' : 'Konstruktionsprojekt'} financial report. Unchanged from round 1, where I flagged it as non-blocking — but note it is now pinned by a test ("renders 'Language: German' and the German project phrase"), so whoever cleans up the non-idiomatic "Konstruktionsprojekt" (a native writer would say "Bauprojekt") will need to update that assertion too.
• Shard-5 E2E flake, as above.

Verdict

APPROVED. All six AC sections of #1901 are met. The three numeric-accuracy defects are fixed at the root with permanent regression coverage, the error copy now speaks to the feature the user is actually in, and the API contract is published. Good, disciplined turnaround — particularly on the prompt test coverage, which went from zero direct tests to a purpose-built guard for the exact class of bug that slipped through.

[steilerDev]

[product-architect]

Re-review round 2 — PR #1916 (story #1901)

Verdict: APPROVE. The blocking finding is fully resolved. Nothing remains blocking.

B1 — verified fixed (b70d821b)

Item	Status
prompts.ts:151 (input.totalAmount / 100).toFixed(2) → input.totalAmount.toFixed(2)	Fixed
prompts.ts:165 (inv.amount / 100).toFixed(2) → inv.amount.toFixed(2)	Fixed
B1b whole-euro rounding → per-invoice Math.round(x * 100) / 100, summed, then rounded once more	Fixed
Rendered-string regression tests	Added
types.ts unit JSDoc	Not done — see L5 below

I re-read the diff rather than taking the summary on trust, and spot-ran both suites locally: prompts.test.ts 98/98 and reportContentGenerationService.test.ts 15/15 green.

The regression guard is the right shape — it asserts the rendered substring (Total Amount: 12345.67 EUR, Amount: 999.99 EUR) plus negative assertions against the divided-by-100 forms, and carries a comment explaining the class of bug. That is exactly the test that would have caught this, and it is now permanent. The multi-currency case (250.00 CHF) is a good addition I did not ask for.

Rounding parity re-derived and confirmed. Server: round per invoice → sum → round the sum. Client: applyLineExclusions rounds per invoice (only when an exclusion applies) → buildReportContent sums with no final round → formatCurrency renders at 2 dp. The server's extra unconditional per-invoice round is stricter than the client's conditional one, and the two can only produce different displayed totals for an allocatedAmount carrying more than 2 dp — which sourceReportService.ts:442 already excludes upstream (allocatedAmount: roundedAmount, and the shared type documents it as "rounded to 2dp"). So the extra rounding is defensive-only and parity holds in production; scenario 5's 333.335 fixture reaches it by direct DB insert, bypassing that invariant. No issue — worth recording so nobody "simplifies" the rounding away later.

PO's linked finding folded in correctly. Per-invoice amount is now the exclusion-adjusted figure from invoiceAmountsAdjusted, not raw allocatedAmount. This was the right call to fix together: sending raw per-invoice amounts alongside an exclusion-adjusted total gave the model a set of parts that did not sum to the stated whole, which is a plausible source of hallucinated reconciliation text in a bank-facing letter. Now recorded as a rule in ADR-034.

L4 addressed too (not required, but welcome): the four shared LLM_* strings are now feature-neutral in both locales. One follow-up for the PO rather than this PR — the German uses KI-Unterstützung / KI-Dienst, and "KI" still has no glossary.json entry. That was already an open question from the #1901 translator pass; now that the term is user-visible in four error strings it should be settled.

New low finding

• L5 — the root-cause annotation is still missing. Item 3 of the B1 fix list was not applied: GenerateReportContentLlmInvoice.amount (budgetExtraction/types.ts:30) and GenerateReportContentLlmInput.totalAmount (:40) still carry no unit in their JSDoc. The behaviour is correct and pinned by tests, so this is not blocking — but the absent unit annotation is precisely what allowed the seam to break, and the next field added to these interfaces inherits the same silence. Two comment lines (/** Major currency units, 2 dp — not cents. */).
• Informational — invoiceAmountsAdjusted.get(inv.invoiceId) ?? inv.allocatedAmount: the fallback is unreachable, since the map is populated under exactly the same filter that guards the read. Harmless; noting it only so it is not later mistaken for a meaningful default.

Findings carried forward (still non-blocking, unchanged)

To answer the coordinator's question — yes, please track these. I'd suggest one follow-up refinement issue rather than five, since M1–M4 are all "the LLM integration outgrew its original naming/boundaries":

• M1 — llmEnabled / autoItemizeEnabled duplication with no deprecation path; getProvider() still gates the report feature on the auto-itemize flag. (@deprecated JSDoc + removal plan. The "no third alias; divergence needs an ADR" rule is already recorded on the API Contract page.)
• M2 — included-total derivation duplicated client/server. B1b was the first drift; extracting computeIncludedTotal into @cornerstone/shared is the structural fix. Highest value of the four.
• M3 — route returns the server-internal GenerateReportContentLlmResult instead of the shared GenerateReportContentResponse; bind the annotation so drift becomes a compile error.
• M4 — BudgetExtractionProvider is now a general LLM gateway under a budget-extraction name, with three unrelated capabilities and five unrelated test doubles stubbing all of them. Rename to llmGateway/ (or split per capability) as a standalone refactor.
• L1 sourceId! assertion · L2 .includes() inside four loops where the Set already exists · L3 the 'German construction project' / 'Konstruktionsprojekt' ternary in the prompt (also not idiomatic — Bauprojekt) · L5 above.

CI

Quality Gates green on b70d821b — the required gate for a beta PR. E2E Gates red on shard 5 only, and it is the same pre-existing flake as the previous run: tests/invoices/invoices.spec.ts:841 ("Effective Amount" column, Issue #1876), unrelated to this PR and already on record as a beta → main promotion blocker to be cleared separately.

Wiki

master 905182d, submodule ref bumped in 4a610f31. Amended the ADR-034 monetary-unit invariant to match what actually shipped: the final round-the-sum step, and the "parts must add up to the stated whole" rule for per-invoice amounts handed to the generator. The API Contract page needed no change — this fix did not alter the request/response shape.

Nice work on the turnaround, and thank you for adding the negative assertions rather than just the positive ones.

[github-actions]

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

The release is available on GitHub release

Your semantic-release bot 📦🚀