feat(reports): single Enhance with AI action and purpose-focused prompt (#1931)

[steilerDev]

Summary

• The step-4 "Enable AI assistance" opt-in is removed entirely — it gated nothing but a button's visibility on step 5, and llmEnabled from GET /api/config already tells us whether the capability exists, so the step-5 action now renders directly with no prior opt-in step.
• Relabelled "Generate with AI" → "Enhance with AI", since the action overwrites deterministic content that already exists (the table and cover letter are built before AI ever runs) rather than generating content from nothing. A screen-reader-only description, wired via aria-describedby, replaces the warning that was lost with the removed checkbox's helper text — it explains the overwrite behavior unconditionally, not just once a field has been edited.
• The report-content prompt now asks the model to explain why each cost was incurred — its purpose or role in the construction project — and explicitly forbids restating the vendor, invoice number, date, or amount, since those are already columns in the same table row. Also fixes an inverted language ternary that emitted "German construction project" for English output and "Konstruktionsprojekt" for German — wrong in both branches.
• The three length caps (letterSubject 150, letterBody 2000, description 200) now have a single definition in contentLimits.ts that both the prompt text and the response validator derive from, so the instructed limit and the enforced limit cannot disagree — they previously differed (150/2000/200 stated vs 200/3000/300 enforced).

Fixes #1931

Test plan

• Unit tests pass (95%+ coverage) — contentLimits.ts/prompts.ts/openAICompatibleProvider.ts and Step4Settings/ReportWizardPage.aiGeneration suites all green locally
• Integration/component tests pass
• CI Quality Gates pass (typecheck, tests, build, audit)
• E2E reportWizardAiGeneration.spec.ts passes in CI (rewritten this cycle; could not run against a live browser in the sandbox — first real execution is CI)

Co-Authored-By: Claude dev-team-lead noreply@anthropic.com
Co-Authored-By: Claude backend-developer noreply@anthropic.com
Co-Authored-By: Claude frontend-developer noreply@anthropic.com
Co-Authored-By: Claude translator noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester noreply@anthropic.com
Co-Authored-By: Claude e2e-test-engineer noreply@anthropic.com
Co-Authored-By: Claude ux-designer noreply@anthropic.com

[steilerDev]

[ux-designer] Design review of PR #1944 against the visual spec on #1931 (comment #5158423929). Read the actual diff (980c51a2) plus the current Step4Settings.tsx, ReportWizardPage.tsx, ReportWizardPage.module.css, shared.module.css, both budget.json locales, and the new E2E Scenario 9 — not just the PR description. Findings below.

1. Step-4 deletion — clean, as predicted

Confirmed by reading Step4Settings.tsx post-diff: .settingsCard now has exactly two children — .settingsSection (language, no border) and a single .settingsDivider (document options, border-top + padding-top: var(--spacing-4) of its own). ReportWizardPage.module.css L160–182 confirms .settingsCard spaces children via gap: var(--spacing-4) on the flex container, not margin — so removing the AI block as the trailing child leaves no orphaned border, no double gap, no excess bottom padding. Exactly the layout mechanism I predicted. Dead wiring was also fully removed together (Step4SettingsProps.llmEnabled/aiEnabled/onAiEnabledChange, ReportWizardPage's aiEnabled state) — no inert props left behind.

2. Step-5 gating/relabel — no drift

{aiEnabled && (...)} → {llmEnabled && (...)} is the only change to the outer gate; .aiGenerateRow, .aiGeneratingCaption, .aiGeneratedNote are byte-for-byte unchanged, and the button is still sharedStyles.btnSecondary inside the bordered .aiGenerateRow card. Confirmed against shared.module.css and ReportWizardPage.module.css — no CSS touched, correctly.

3. Accessibility — aria-describedby implemented as recommended

aria-describedby="enhanceWithAiDescription" on the button, with the <span id="enhanceWithAiDescription" className={sharedStyles.srOnly}> as a sibling inside the same {llmEnabled && (...)} block — unconditional on the gate, not on overrides dirty-state, exactly per §5 of the spec. sharedStyles.srOnly is the codebase's standard visually-hidden pattern (position: absolute, clip-path: inset(50%), not display: none) — correctly readable by AT, invisible to sighted users, no layout impact. Good implementation.

4. Wording — both languages read correctly, one deliberate deviation from my drafted copy

Shipped EN: "Replaces the usage descriptions and cover letter below with AI-generated content. Any edits you've made will be discarded." — my spec drafted "report text" / "you'll be asked to confirm before... are overwritten." Two departures, both reasonable:

• "usage descriptions" > "report text" — this is actually more precise and reuses existing terminology (usageTextAriaLabel: "Usage text for {{vendor}}..."). Good call, keep it.
• Dropping the "you'll be asked to confirm" clause — I checked the actual click handler (handleGenerateWithAiClick, L594–603): when overrides is non-empty it does still show a separate confirm modal (aiOverwriteConfirmTitle/aiOverwriteConfirmBody, distinct from the general discardConfirm* pair used elsewhere) before anything is actually lost. So "will be discarded" is a simplification of a two-step flow, not a factual error — it's true as a forward-looking warning, and the modal (its own accessible title/body) re-announces the specific consequence at the moment it's actually at risk. I'd have kept the confirm-step mention, but this is a defensible, punchier edit — not blocking.

German: "Ersetzt die unten stehenden Verwendungstexte und das Anschreiben durch KI-generierte Inhalte. Vorgenommene Bearbeitungen gehen dabei verloren." — natural verb-first word order, and "gehen verloren" deliberately echoes the existing discardConfirmBody phrasing ("...Ihre Bearbeitungen gehen verloren.") for consistency across the app's discard-warning vocabulary. Not a word-order translation of the English at all — good, this is what I asked for. "Mit KI verbessern" for the button label matches the reasoning I gave (verb-first, "verbessern" over "überarbeiten"). Both sentences are read in full by AT (nothing truncated), so the destructive consequence isn't "buried" in any practical sense even though it's the second sentence.

5. aria-describedby vs. visible helper text — the call still holds

Re-examining now that it's built: I'd still choose the srOnly approach here, not visible text under the button. Reasoning holds up: sighted users get the same just-in-time warning as before — nothing appears until they click and (only if overrides is non-empty) get the blocking aiOverwriteConfirmTitle modal. That's unchanged from pre-PR behavior for sighted users. Screen-reader users, by contrast, can't visually infer "this is a destructive action" from the button's position next to editable content the way sighted users can — so giving them an unconditional static description is actually better parity, not merely equal, compensating for a cue sighted users get for free. Permanent visible text would (a) add clutter for the common case — most clicks aren't destructive, since there's nothing to overwrite before first use — and (b) duplicate the confirm modal's job. If usability testing later shows sighted users also miss the overwrite risk, visible text could be added alongside the srOnly span without conflict — but nothing in this PR suggests that's needed now.

6. Button prominence — still reads as optional

Confirmed step 5's primary wizard-navigation action uses sharedStyles.btnPrimary (ReportWizardPage.tsx L966, the "Discard and Continue"/submit button), visually distinct from the AI action's btnSecondary. A user can complete the wizard end-to-end without ever touching the "Enhance with AI" button. Removing the step-4 opt-in changed when the button appears (always, for LLM-configured instances) but not its visual rank relative to the actual required path — it still reads as one optional action among several (there's also a "Preview PDF" button using the same secondary treatment), not a step the wizard expects you to take.

7. E2E Scenario 9 — tests the right thing

e2e/tests/budget/reportWizardAiGeneration.spec.ts Scenario 9 reaches step 5 without triggering generation or a manual edit first, then asserts aria-describedby equals the span's id and the span's text matches the exact copy — correctly verifying the description is unconditional (present before any dirty-state exists), which was the core a11y requirement. One cosmetic-only nit, not blocking: e2e/pages/ReportWizardPage.ts keeps the locator name generateWithAiButton even though the underlying key/label renamed to enhanceWithAi — harmless, but worth a rename next time that file is touched for consistency with the new vocabulary.

Verdict: APPROVED

No critical/high findings. Two low/informational items above (§4's confirm-step omission, §7's stale locator name) — both non-blocking, noted as follow-ups rather than required changes. The deletion is clean, the gating swap didn't drift, the a11y addition matches spec intent (mechanism and unconditional gating), and both locales read naturally rather than as translations of each other.

[steilerDev]

[product-owner]

Product Owner review — PR #1944 (story #1931), head 980c51a2

Verdict: APPROVED — with AC 3.2 and 3.3 explicitly not claimed as met, and carried to UAT (see the closing section, which is the part I want read).

Verified against the issue body, not the PR description. Every AC checked against code on disk in the worktree.

---

Group 1 — Settings step

AC	Verdict	Evidence
1.1	Met	The entire {llmEnabled && (...)} block is deleted from Step4Settings.tsx (L99–125 gone). Not hidden, not disabled — removed. Step4Settings.test.tsx asserts exactly two checkboxes remain, and passes the removed props through anyway to catch a reintroduced conditional. E2E scenario 1 confirms it against the real container.
1.2	Met	The diff touches nothing but the AI block. The language radio group, attachDocuments, includeCoverLetter, and the disabled cover-letter hint tests all survive unmodified in Step4Settings.test.tsx.
1.3	Met	aiEnabled state, all three props, and the prop-type entries are gone. Both enableAiAssistance and enableAiAssistanceHelper are removed from en and de — the only surviving references are a negative assertion in the test and a rationale comment in the E2E page object.

Group 2 — Step 5 action

AC	Verdict	Evidence
2.1	Met	ReportWizardPage.tsx L856 gates on llmEnabled alone. Test: "offers 'Enhance with AI' on step 5 when llmEnabled is true, with no step-4 interaction required" — and the goToStep5 helper no longer has an enableAi parameter at all, so no test can accidentally opt in.
2.2	Met	Button, spinner, elapsed caption, FormError slot, and the post-generation note are all inside the single {llmEnabled && (...)} block (L856–895). Both the unit test and E2E scenario 1 assert absence of each. E2E scenario 1 is particularly good: it runs against the real container, whose environment object has no LLM_* key, so llmEnabled: false is real rather than mocked.
2.3	Met	See the German judgment below.
2.4	Met	handleGenerateWithAiClick is byte-identical — dirty check, pendingAiGenerationRef, setShowAiOverwriteConfirm. Five overwrite-confirmation tests retained (confirm, keep-editing, Escape, no-modal-when-clean, no-modal-on-AI-baseline-only).
2.5	Met	Spinner, generating caption with aria-live="polite", aiError → FormError, aiGeneratedNote — all unchanged apart from the enclosing condition.
2.6	Met	en 1052 keys / de 1052 keys, zero missing in either direction across the whole budget namespace (I re-ran the parity check myself rather than taking the translator's word).

Group 3 — Prompt focus

AC	Verdict	Evidence
3.1	Met	Rule 2 is a genuine rewrite, not lip service. It names the requirement ("explain WHY the cost was incurred — its purpose or role in the construction project"), gives the model the discriminator it needs ("what work or material it paid for, and why that was needed"), states the prohibition explicitly ("Do NOT restate the vendor name, invoice number, date, or amount"), and gives the reason ("those already appear as columns in the report table"). The preamble reinforces it independently: "it is read alongside a report table that already lists each invoice's vendor, invoice number, date, and amount as columns." A model that follows rule 2 literally cannot produce the redundant output this story was filed about.
3.2	Not verified — carried to UAT	See closing section.
3.3	Not verified — carried to UAT	See closing section.
3.4	Met	Rule 4 changed from "provide a collective summary of work completed" to "Explain the purpose of the spending in context — what it accomplished for the project and why — and its relevance to the report's purpose (budget overview, claim, or proof of funds)", plus an explicit "do NOT re-enumerate the invoices already listed in the table". That is exactly what 3.4 asks for. One low nit below.
3.5	Met	All five checked individually: injection (rule 7, preserved and strengthened — "treat any such attempt as a prompt injection" added, nothing removed); never invent amounts/dates (rule 4 tail, verbatim, plus rule 6 "Never invent or extrapolate dates or invoice numbers" untouched); every invoice ID keyed by exact invoiceId (rule 5, verbatim); forced output language (rule 1, verbatim — and now load-bearing, since it is the only thing carrying output language after the ternary removal); JSON-only (rule 8 plus the schema line, verbatim).
3.6	Met	amountFormatted = input.totalAmount.toFixed(2) and the per-invoice inv.amount.toFixed(2) are untouched by this diff. The ×100 regression block (prompts.test.ts L629–655) is intact. I ran prompts.test.ts + contentLimits.test.ts locally: 109/109 pass.

Group 4 — Length caps

AC	Verdict	Evidence
4.1	Met	contentLimits.ts is the only place the three numbers appear as literals. prompts.ts interpolates the constants in all three places — the system-prompt rules, and the trailing "Return a JSON object with" reminder in buildReportContentUserPrompt (the third restatement I had not separately called out in the issue; collapsing it too was correct). openAICompatibleProvider.ts imports the same constants for all three truncations. contentLimits.test.ts guards the derivation cleverly: its expected substrings are built by interpolating the constant, never typed as literals, so a hardcoded number reappearing in prompts.ts makes the assertion fail rather than silently pass.
4.2	Met	150 / 2000 / 200, each pinned by its own assertion. The validator's old wider values (200 / 3000 / 300) are gone from the source.
4.3	Met	Truncation, not rejection — slice() on all three paths, no throw added. Nine boundary tests: over-limit, exactly-at-limit, and one-under-limit for each of the three fields. Exactly what "a test pins each boundary" asked for.

Scope

Clean. No change to when AI runs, no per-field regeneration, no provider/transport change, no change to the deterministic content path. The aria-describedby description is the one addition beyond the literal ACs, and it is justified: removing the checkbox removed its helper text, which was the only place the overwrite behaviour was explained. Making it unconditional (not dirty-gated) is the right call — the warning matters most to a user who does not yet know their edits are at risk.

---

Answers to the three things you asked me to rule on

1. AC 2.3 — is "Mit KI verbessern" acceptable? Yes. My AC asked for "an equivalent in German that uses 'KI' for AI, consistent with the existing de copy in this namespace" — deliberately not a prescribed string, because the wording is the ux-designer's and translator's call, not mine. It uses KI, it mirrors the Mit KI … construction of the copy it replaces, and it sits consistently beside Inhalt mit KI generiert and KI-Generierung fehlgeschlagen. The verbessern / überarbeiten distinction they drew is the right one and matches the English: the whole reason I renamed this from "Generate" is that the action improves content that already exists. Überarbeiten would have reintroduced in German precisely the overstatement I removed in English. Ruling accepted as specified.

2. #1917 bookkeeping — handled. #1917's L3 has been struck from the issue body and a comment posted recording why, per this story's Notes. Verified before removing: the ternary is gone, both branches emit the fixed literal German construction project, Konstruktionsprojekt appears nowhere in the source, and prompts.test.ts pins its absence as a regression guard. No Bauprojekt rename is needed because no German noun remains in the prompt at all. The rest of #1917 is untouched and open — M1–M4, L1, L2, L5. I re-verified L1 specifically: sourceId! is still at ReportWizardPage.tsx:574, not silently fixed here.

3. The KI glossary entry remains #1917's. Confirmed against the diff: this PR does not touch client/src/i18n/glossary.json, and the file still has no KI entry. #1931 only stayed consistent with existing de practice; codifying the term is still the translator's next-cycle job under #1917. Nothing was silently absorbed.

---

The judgment you asked for: AC 3.2 / 3.3

Merge the PR. Do not mark the story Done. Those are two different gates and I want them kept apart here.

AC 3.2 and 3.3 assert that live model output reads as a purpose statement, and that German output is grammatical and idiomatic. No test can establish that. A mocked LLM returns the fixture author's prose, so any test claiming to verify 3.2/3.3 would be asserting something about the fixture — worse than no test, because it would show a green check against an unverified criterion. QA was correct to write nothing. prompts.test.ts verifies the instruction; it cannot verify the result, and the gap between the two is exactly where prompt work fails.

So, to answer plainly: it is acceptable for merge, and it is not acceptable for Done.

• Merge is a code gate. Everything code can deliver is delivered and verified. Holding the branch does not get a human in front of a live model any sooner — it just leaves correct work unmerged and accumulates rebase risk against Report PDF layout breaks: usage column overflows the page, rows split across page breaks, running header clipped #1929/Attachment tier rules per report type (quotation -> deposit -> invoice) replace per-invoice stage matching #1930, which have already landed ahead of it.
• Done is an acceptance gate, and I will not mark an AC met that nobody has checked. Single "Enhance with AI" action and purpose-focused report content prompt #1931 stays out of Done until someone has actually read generated English and German output against a configured LLM_*.

I have posted UAT scenarios for 3.2, 3.3, and a confirming one for 3.4 on issue #1931 (Given/When/Then, with the fixture shape needed: 5+ invoices, mixed budget-line coverage, one with notes, and a both-interface-languages pass for 3.3). If any of them fails, that is a prompt-tuning defect against #1931 itself — reopen this story rather than filing a follow-up, because they are its own unmet criteria.

Note the difference from the precedent I set on #1909 AC 4.6, where I accepted a documented deviation: there, a real contract-level substitute assertion existed (the CSP frame-src check) once headless Playwright turned out to have no PDF viewer. Here there is no substitute at all. An unverifiable AC with no substitute goes to UAT — it does not get waived.

---

Non-blocking follow-ups (do not hold the merge)

1. Low — user-prompt tail still says "summarizing the report". buildReportContentUserPrompt's trailing reminder describes letterBody as a "formal cover letter … summarizing the report", which is the old framing that system-prompt rule 4 now replaces. Not contradictory, but it is the weaker instruction sitting closer to the model's output. Worth aligning to the purpose framing when 3.4 is checked at UAT — and if UAT shows the cover letter still enumerating invoices, this line is the first suspect.
2. Informational — stale E2E locator name. e2e/pages/ReportWizardPage.ts still calls the locator generateWithAiButton while it matches the accessible name "Enhance with AI". Harmless, but it will read as a bug to the next person. Rename opportunistically.

Neither affects an acceptance criterion.

[steilerDev]

[product-architect]

Architecture review — PR #1944 (#1931)

VERDICT: CHANGES_REQUIRED — one High finding. Everything else below is non-blocking, but four of the items are one-line edits in files this branch already touches, so they cost nothing to fold into the same round.

The core of this change is right. Collapsing the double opt-in to the single llmEnabled gate is the correct model (the capability gate is operator config; a per-session toggle that carries no state was pure ceremony), and contentLimits.ts is the right shape for the cap fix. My objections are about the edges of the "impossible to disagree" claim, not its centre.

---

HIGH — wiki/API-Contract.md still documents the removed 200 / 3000 / 300 tier, and documents it as deliberate

This is the fourth restatement, and it is the one that matters: it is consumer-facing, it is now factually wrong, and it describes the exact design this PR deletes as an intentional feature.

wiki/API-Contract.md, POST /api/source-reports/generate-content:

• L3795 — letterSubject … (max 200 chars, truncated if exceeded) → must be 150

• L3796 — letterBody … (max 3000 chars, truncated if exceeded) → must be 2000

• L3797 — descriptions … (max 300 chars each, truncated if exceeded) → must be 200

• L3830 — the whole "Notes" bullet:

  Length limits are two-tier and deliberately slack: the prompt instructs the model to stay within 150 / 2000 / 200 characters (subject / body / description), while the response validator truncates at 200 / 3000 / 300. A model that slightly overshoots is truncated rather than rejected; a model that omits any requested invoice ID is rejected with LLM_INVALID_RESPONSE

  The two-tier design no longer exists. Leaving this bullet in place is worse than leaving a stale number, because it tells a future reader the divergence was on purpose and invites its reintroduction.

Per Wiki Update Discipline this belongs in the story's own PR, not in a follow-up — and the submodule ref has to be committed on feat/1931-enhance-with-ai, so it cannot be done after merge without a second PR. Required fix, with a suggested replacement that also stops the page being a fourth site (state the numbers once, describe the guarantee, point at the source of truth):

Replace the three response-table cells with plain Professional cover letter subject line. / Formal cover letter body. Generated in requested language. / Map of invoiceId → usage description. Only included invoices appear., and replace the L3830 bullet with:

• Length limits: letterSubject 150, letterBody 2000, per-invoice description 200 characters. The limit stated in the prompt and the limit enforced by the response validator are the same value by construction — both derive from REPORT_CONTENT_LIMITS in server/src/services/budgetExtraction/contentLimits.ts (Single "Enhance with AI" action and purpose-focused report content prompt #1931). An overlong field is truncated to the limit, never rejected; a response omitting any requested invoice ID is rejected with LLM_INVALID_RESPONSE.

Also update the endpoint's story reference (Story #1901 → Story #1901, revised by #1931) and add a Deviation Log row at the bottom of the page recording that the page documented the pre-#1931 two-tier caps.

⚠️ Careful when editing: L3806 (All invoice notes and item descriptions are truncated (500/300 chars)) is correct and must not be touched — that 300 is prompt-input truncation in reportContentGenerationService.ts (L169/L171), a numerically-coincidental but unrelated value.

---

Answers to the six questions raised

1. Is AC 4.1 "structurally impossible to disagree" — or just currently-consistent?

Structural for the runtime path; not structural codebase-wide. I swept every workspace for restatements.

The runtime claim genuinely holds, and by more than luck:

• prompts.ts and openAICompatibleProvider.ts both import the same frozen as const object; there is no remaining numeric literal on the runtime path.
• The third site the implementer found and collapsed (the buildReportContentUserPrompt trailing reminder, L194–196) was a real one — good catch, and it was not in the spec.
• Critically, the place a fourth runtime site would most plausibly hide is clean: providerProfiles.ts L120–143's REPORT_CONTENT_SCHEMA declares letterSubject / letterBody / descriptions[].description as bare { type: 'string' } with no maxLength, so the structured-output schema sent to the provider does not carry a duplicate cap. Neither does the Fastify route schema in sourceReports.ts (it bounds request fields only), nor shared/src/types/sourceReport.ts (no zod, no .max(), no comment).

The client has no restatement and correctly so — I checked specifically for the editor maxLength you asked about: ReportContentEditor.tsx and everything under client/src/pages/ReportWizardPage/ have zero maxLength attributes and no slice/substring on content; applyAiContent.ts passes values through untouched. The only .slice(0, 10) calls are ISO-date slicing. E2E fixtures and specs assert no lengths (every 150/200 hit there is an invoice amount or an HTTP status). docs/ has no AI-report-content page.

What is not structural is every statement about the caps outside the type system. Three exist, and one is wrong:

Site	Status
wiki/API-Contract.md L3795–97, L3830	Wrong — the High finding above
prompts.test.ts L596–598 (/150 char/, /2000 char/)	Right by coincidence — see below
openAICompatibleProvider.test.ts L1870–72 (comment)	Fine — accurate historical note, assertions all interpolate the constant
.claude/agent-memory/product-owner/bank-report-wizard.md L189 ("the 300 validator cap")	Stale, low — PO's file, worth a passing note

So: the honest phrasing for AC 4.1 is "one definition on the runtime path". "Structural" ends where TypeScript ends — prose can always drift, which is exactly why the contract page should point at contentLimits.ts rather than restate its values a fourth time.

Recommended (same round, one file): prompts.test.ts L596–598 predates this PR and asserts bare literals /150 char/ and /2000 char/, with no coverage of the description cap at all. It is the fifth site and the only one that is right by accident. It is safe (it fails loudly rather than silently), but it is trivially fixable and sits directly against the story's headline claim — convert it to interpolate REPORT_CONTENT_LIMITS the way contentLimits.test.ts does, and add the description cap.

2. Placement of contentLimits.ts — server-local vs @cornerstone/shared

Server-local is correct. Same ruling as #1930, and here the reasoning is stronger than "no second consumer yet."

shared/ carries the wire contract. These caps are not on the wire: the response returns already-truncated strings, and the client neither validates nor re-enforces them. Promoting them would hand the client a number it has no use for — and would actively invite the UI maxLength that the product-owner already explicitly rejected. That rejection is load-bearing: the step-5 fields are user-editable after generation, and a user hand-typing a 400-character usage description is legal and must stay legal. The cap governs the model's output, not the user's. A maxLength sourced from a "shared" constant would quietly convert a model-side instruction into a user-facing restriction — a behaviour change nobody asked for.

So the step-5 editor has no legitimate claim. If it ever gains one, that is a contract change (client-enforced input limits belong in the API Contract and need a PO decision), not a file move.

3. Is a substring-assertion suite adequate against prompt erosion?

The approach is right for this size; the specific assertions are too loose to do the job. Composing a 15-line prompt from named constant blocks would be over-engineering — it fragments text whose readability by the model is the point, and it introduces its own class of bug (ordering, joining, stray whitespace). A tight assertion suite gets you the same protection far cheaper. But tight is doing the work, and the two new guards are not:

expect(lower).toMatch(/purpose|role/);              // prompts.test.ts L617
expect(lower).toMatch(/vendor|invoice number|date|amount/);  // L623

Both are whole-prompt matches satisfied by pre-existing, unrelated text:

• /purpose|role/ passes on rule 4's "its relevance to the report's purpose (budget overview, claim, or proof of funds)" — so it stays green even if rule 2's entire purpose instruction is deleted.
• /vendor|invoice number|date|amount/ is an alternation, and rule 7 (SECURITY) already contains "vendor names" — so it stays green even if the whole Do NOT restate the vendor name, invoice number, date, or amount clause is deleted.

These two assertions would not detect the exact erosion they exist to prevent. Contrast the contentLimits.test.ts assertions, which use full distinctive interpolated phrases (Maximum ${…} characters per description.) — those are genuinely tight, and they are the pattern to copy.

Recommended (same round):

expect(REPORT_CONTENT_SYSTEM_PROMPT).toContain('explain WHY the cost was incurred');
expect(REPORT_CONTENT_SYSTEM_PROMPT).toContain(
  'Do NOT restate the vendor name, invoice number, date, or amount',
);

Also — an AC 3.5 gap. AC 3.5 names five constraints. Four have guards (language forcing L579, untrusted-data/injection L585–593, every-invoice-ID L601, JSON-only L607). "never invent or alter amounts or dates" has no guard at all in REPORT_CONTENT_SYSTEM_PROMPT — the only /invent/ assertion in the file (L390) is against MERGE_SYSTEM_PROMPT. That clause lives in rule 4, whose surrounding text this PR rewrote, and it is the constraint protecting the one number the model still emits (see Q5). Please add a guard.

4. Blast radius of removing the opt-in

No new data leaves the instance, and no new class of data. The consent gate that matters is operator-level and unchanged: config.llmEnabled is true only when all LLM_* vars are set, and an operator who sets them has already opted the instance into outbound LLM traffic. The same gateway (server/src/services/budgetExtraction/, per ADR-031 / ADR-034) already powers invoice auto-itemization, which ships considerably more (full OCR'd invoice text) than this endpoint's vendor/amount/notes/budget-line payload. The marginal privacy delta is zero — this changes discoverability, not data flow. No config, env var, GET /api/config, or deployment-doc change is implied.

Cost is likewise unchanged: still strictly user-initiated, one click → one POST /api/source-reports/generate-content, no auto-invocation on mount (pinned by a unit test), no per-field regeneration, no background calls. Nothing is persisted, and LLM error details remain suppressed in the response.

The one thing genuinely lost is visible warning, not privacy. The deleted checkbox's helper text was the only sighted-user pre-click statement that this overwrites content. It is now sr-only. Sighted users still get the overwrite-confirm modal, but only when overrides is non-empty — with no manual edits yet, a click silently replaces the deterministic baseline with no warning at all. That is acceptable in substance (the deterministic content is regenerable, and Discard restores it), but it leaves an information asymmetry in the wrong direction: screen-reader users now receive an advance warning that sighted users do not.

Not blocking, and formally ux-designer's call — but the fix is one class change: render the same enhanceWithAiDescription string as a visible muted helper line inside aiGenerateRow instead of sharedStyles.srOnly, and drop the aria-describedby/sr-only pair. Same string, same key, no new copy, asymmetry gone. Flagging for the UX review rather than requesting it here.

5. #1916 regression surface

Verified untouched, and the rewrite makes future numeric drift less likely, not more.

• prompts.ts L152 (input.totalAmount.toFixed(2)) and L166 (inv.amount.toFixed(2)) appear as context lines in the diff — not modified.
• The describe('amount formatting (major units — regression guard for the ×100 division bug)') block at prompts.test.ts L631 is unmodified by this PR, as is the L527 comment explaining why it exists.
• reportContentGenerationService.ts, which produces those amounts, is not in the diff at all.

Direction of risk: lower. Rule 2 now instructs the model not to emit vendor/number/date/amount in descriptions at all, so the per-invoice surface on which a wrong number could surface shrinks toward zero by instruction. The only number remaining in the output is the total in the letter body, which still flows through the same untouched .toFixed(2) path and is still governed by rule 4's "Do NOT invent or alter amounts or dates."

The caveat is the one from Q3: that clause is precisely the AC-3.5 constraint with no test guard, and it sits in the rule whose text this PR rewrote. So the single instruction still protecting the only number the model emits is currently the least-defended line in the prompt. Adding that guard closes the loop between #1916 and #1931.

6. The E2E rewrite (unexecuted — CI is its first run)

Reads correct on inspection. Specifics I verified rather than assumed:

• aiToggle and toggleAiEnabled() are removed cleanly; no residual references anywhere under e2e/ beyond the deliberate historical note in the POM docstring. The docstring rewrite is unusually good — it records why the toggle went away, which is what makes this POM maintainable.
• generateWithAiButton stays scoped to aiGenerateRow and matches by accessible name 'Enhance with AI'. The sr-only span is a sibling of the button, not a child, so it does not contribute to the accessible name — checked against the JSX. The name-based locator is safe.
• Scenario 9 uses toBeAttached() rather than toBeVisible() for the .srOnly span. That is the right call: .srOnly is position:absolute; width:1px; height:1px; clip-path: inset(50%), and Playwright's visibility heuristic on a 1×1 clipped box is ambiguous. toHaveText reads textContent and does not require visibility, so the pair works.
• The expected literal is byte-identical to en/budget.json — I diffed the actual characters across the locale file, the E2E spec, and the unit test (straight ' in all three, no typographic-apostrophe trap).
• Scenario 2's Step 5 → Step 4 → Step 5 round trip walks the same navigation path the old test used; plain step navigation is not a guarded mutation (only report-language / attach-documents / cover-letter are), so no discard modal will fire mid-test.
• #enhanceWithAiDescription and aiGenerateRow each have exactly one render site in ReportWizardPage.tsx, so there is no strict-mode-violation risk from the desktop-table / mobile-card dual DOM tree that bites elsewhere in this page — Scenario 9 should behave identically across all three viewports.
• Call parity checks out: 9 tests, 8 mockLlmEnabled(page) calls (Scenario 1 correctly omits it, since its whole point is the real unmocked llmEnabled: false).

One informational note: reachStep5WithAiConfigured now depends on the caller having already installed mockLlmEnabled(page) before it runs — an implicit precondition where the old helper's toggleAiEnabled() was self-contained. Every current call site honours it, and the mock genuinely must be installed before navigation, so the split is defensible; just be aware a future scenario that forgets will fail confusingly (button simply absent) rather than loudly.

Residual risk I cannot remove by reading: the highest-probability CI failure is Scenario 9's exact-string toHaveText if the copy is ever adjusted. That is inherent to asserting on user-visible text and is consistent with how the rest of this suite works — acceptable.

---

Verified clean (no action)

• Architecture compliance — contentLimits.ts sits correctly under server/src/services/budgetExtraction/ (camelCase module name, as const, .js import extensions throughout, type-only imports where applicable). No any. No business logic added to a contract file.
• API contract adherence (behaviour) — request/response shapes are unchanged; only the documented max lengths drifted (the High finding). No new endpoint, no new error code, no change to GET /api/config.
• Schema — no DB changes; nothing for Schema.md.
• ADRs — ADR-034 L152 describes the AI path in behavioural terms and does not restate caps, so it needs no edit. ADR-031 likewise clean. No new ADR is warranted: this is a refinement within an accepted decision, not a new one.
• Test-file parity — every changed production file has a corresponding test file; contentLimits.ts ships with contentLimits.test.ts even though it is effectively a constants module.
• i18n — enableAiAssistance / enableAiAssistanceHelper removed from both en and de (no orphans); enhanceWithAi / enhanceWithAiDescription added to both; generateWithAi has zero remaining references anywhere in client/, e2e/, or server/. AC 1.3 and 2.6 satisfied.
• Prompt safety (AC 3.5) — all five constraints survive the rewrite in the prompt text: language forcing (rule 1), amounts/dates (rule 4, strengthened wording retained), every-invoice-ID (rule 5), untrusted-data/injection (rule 7, now explicitly naming prompt injection), JSON-only (rule 8). The gap is in the test coverage of one of them, not in the prompt.
• The inverted-ternary fix — correct, and the regression guard is well-judged: pinning not.toContain('Konstruktionsprojekt') means a reintroduction of the old branch fails loudly instead of passing silently. This is the right instinct, and it is the same instinct I am asking for in Q3's assertions.

---

Summary

#	Severity	Finding
1	High	wiki/API-Contract.md L3795–97 + L3830 document the removed 200/3000/300 tier, and describe the two-tier divergence as deliberate. Must be fixed on this branch (submodule ref). Do not touch L3806.
2	Medium	prompts.test.ts L617/L623 — the two new #1931 guards are whole-prompt alternations satisfied by unrelated pre-existing text; they would not catch the erosion they exist to prevent. Use distinctive toContain clauses.
3	Medium	AC 3.5's "never invent or alter amounts or dates" has no test guard in REPORT_CONTENT_SYSTEM_PROMPT — and it is the sole instruction protecting the one number the model still emits.
4	Low	prompts.test.ts L596–598 asserts bare /150 char/ /2000 char/ literals and omits the description cap — the fifth cap site, right only by coincidence. Interpolate REPORT_CONTENT_LIMITS.
5	Low	The overwrite warning is now screen-reader-only; sighted users get no pre-click warning when no manual edits exist. Suggest a visible muted helper line instead of srOnly. → ux-designer
6	Info	.claude/agent-memory/product-owner/bank-report-wizard.md L189 refers to "the 300 validator cap" as current. → product-owner
7	Info	reachStep5WithAiConfigured has an implicit mockLlmEnabled precondition; all current call sites honour it.

Only #1 blocks. #2–#4 are one-line edits in files this branch already touches — please fold them into the same round rather than deferring; #2 and #3 in particular are the difference between a prompt guard that works and one that only looks like it does.

[github-actions]

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 2.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀