Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .claude/checklists/implementation-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ This checklist is updated after each epic's lessons-learned sync (see `/epic-clo
- [ ] **Empty states**: Every list/table view must use the shared `EmptyState` component when data is empty. Never show a blank page or raw "No data" text.
- [ ] **Loading states**: Every async data fetch must show the shared `Skeleton` component during loading. Never show a blank container or raw "Loading..." text.
- [ ] **Fraction-to-percent display**: Decimal-fraction constants (e.g. `CONFIDENCE_MARGINS` where `0.2` means 20%) must be multiplied by 100 before percentage display. This is a recurring finding flagged independently by the architect and product owner (e.g. PR #401).
- [ ] **`Intl.NumberFormat` groups thousands; `toFixed()` never did**: When replacing a `toFixed()` call or manual string-building with a locale formatter, verify the old-vs-new output for values >= 1000, not just per-locale correctness. Grouping separators are an easy-to-miss regression in the *English* output too (PR #1845).

## Frontend — Forms & Validation

Expand All @@ -31,6 +32,7 @@ This checklist is updated after each epic's lessons-learned sync (see `/epic-clo
- [ ] **Semantic token usage**: Use tokens for their intended purpose. Hover backgrounds must use `var(--color-bg-hover)`, never `var(--color-border)` or other non-bg tokens as background values.
- [ ] **Dark mode**: All color properties must use CSS custom properties that switch in `[data-theme="dark"]`. Verify no hardcoded `#hex` or `rgb()` values.
- [ ] **No colors in inline style props**: Never put color values (`color-mix()`, `backgroundColor: 'var(--token)'`, etc.) in inline `style` props — inline styles bypass stylelint's token enforcement entirely (recurring: PR #792, PR #1681). Use a CSS-module class, or a `data-*` attribute with a CSS attribute selector for dynamic variants.
- [ ] **Never use `:global(.foo)` to style another module's class**: `:global(.foo)` matches only a literal, unhashed class string. A class applied via plain `className={otherStyles.foo}` resolves to a hashed name (`foo_a1b2c`) in real webpack builds, so the entire rule block silently never applies. **This is invisible in Jest** — `identity-obj-proxy` resolves classes to their literal key name, so the selector *does* match in tests and the suite stays green. The correct cross-module technique is `composes: foo from '../other/Other.module.css';` inside a locally-scoped class, then apply that local class (PR #1909). Suspect this whenever spec'd states (hover/focus/at-rest tint/indicator dots) go missing in the real app but pass in tests.

## Frontend — Shared Components

Expand All @@ -43,6 +45,8 @@ This checklist is updated after each epic's lessons-learned sync (see `/epic-clo
- [ ] **FormError usage**: Error display must use the shared `FormError` component.
- [ ] **No one-off components**: Every new UI component must be designed as a reusable shared component in `client/src/components/`.
- [ ] **Compose shared CSS classes**: CSS utility classes (buttons, modals, loading, empty states, sr-only) must use `composes:` from `client/src/styles/shared.module.css`. Never duplicate shared class definitions.
- [ ] **A new control in an editor must actually reach the export/generation pipeline**: Grep the control's state symbol *outside* its own component before assuming the value is consumed. PR #1959 shipped column-visibility checkboxes that changed only the on-screen preview and never reached the generated PDF; wiring them through was a separate story (#1973). For column show/hide specifically, `DataTable/DataTableColumnSettings.tsx` already exists — check it before building a bespoke checkbox row.
- [ ] **Preview components must match the exporter's constants, not just the design tokens**: When a component previews an exported artifact (PDF, print view), compare each style against the *exporter's* constants. A preview can be fully token-compliant and still not match what the export renders — PR #1959 shipped two grey annotations at two sizes, only one matching the PDF.

## Frontend — React Hooks

Expand All @@ -68,12 +72,16 @@ This checklist is updated after each epic's lessons-learned sync (see `/epic-clo
- [ ] **Parameterized queries**: All database queries must use parameterized values. Never interpolate user input into SQL strings.
- [ ] **Wiki documentation**: When adding or changing API endpoints, fields, or query parameters, update `wiki/API-Contract.md` and `wiki/Schema.md` accordingly. This is a recurring architect review finding.
- [ ] **Named error codes from acceptance criteria**: When an AC specifies a custom error code (e.g. `ACCOUNT_DEACTIVATED`, `MUTUALLY_EXCLUSIVE_BUDGET_LINK`), return that exact code via `AppError` with the code set explicitly. Do not use a `ValidationError` subclass that emits a generic `VALIDATION_ERROR` (recurring: PR #56, PR #414).
- [ ] **Reused error codes need feature-neutral copy**: When a second feature reuses an existing `ErrorCode`, re-read its message text. `LLM_NOT_CONFIGURED` read "Auto-itemization is not configured" while surfacing in the report wizard, and the `LLM_*` family all said "The extraction service ..." (PR #1916). Written for the first consumer, wrong for the second.
- [ ] **A wiki edit is not published until it is pushed**: `git -C wiki status --short` showing `M API-Contract.md` while `git -C wiki log -1` still equals `origin/master` means the page is written but unpublished — a "documented on the wiki" acceptance criterion is **not** satisfied. Check this on every story carrying a wiki documentation criterion.

## Backend — Data Handling

- [ ] **snake_case in DB, camelCase in TS**: Database columns use snake_case; TypeScript code uses camelCase. ORM mapping handles conversion.
- [ ] **Cascade deletes**: When deleting parent entities, ensure child records are cleaned up (via FK cascades or explicit deletion).
- [ ] **Transaction safety**: Multi-step mutations that must be atomic should use database transactions.
- [ ] **Money is stored in MAJOR units, not cents**: `real` columns and types like `SourceReportInvoice.allocatedAmount` hold `250` meaning EUR 250.00, and feed `Intl` currency formatting directly. The minor-units idiom `(amount / 100).toFixed(2)` understates every figure by 100x — it reached a bank-facing cover letter in PR #1916. For the same reason, `Math.round(x)` on major units rounds to whole currency units, not cents: the correct form is `Math.round(x * 100) / 100`. Treat a `// Round to nearest cent` comment sitting above a bare `Math.round(x)` as a defect.
- [ ] **Derived aggregates and their per-item components must agree**: When an exclusion rule adjusts an aggregate, verify the per-item values sent alongside it were adjusted by the same rule. PR #1916 subtracted excluded portions from `totalAmount` but emitted each invoice's raw `allocatedAmount`, so the parts visibly summed to more than the stated total.

## Shared — TypeScript Conventions

Expand All @@ -95,6 +103,16 @@ This checklist is updated after each epic's lessons-learned sync (see `/epic-clo
- [ ] **E2E text locators after label changes**: When a production PR renames a UI label, update all E2E test locators that match that text. Regex locators like `/hide linked/i` silently break when the label changes to "Hide already-linked documents" (no contiguous match). Prefer `data-testid` attributes for stability; when using text regex, keep the pattern broad enough to survive minor rewording (e.g. `/hide.*linked/i`).
- [ ] **E2E modal interception**: When a feature wraps an existing user interaction inside a new modal (e.g. file selection → "Add photo details" modal before upload), ALL existing E2E tests that exercise the downstream behavior (upload queue, photo card appearance) must be updated to dismiss the modal first. The dev-team-lead must flag this in `[MODE: review]` if affected E2E tests are not updated in the same PR.
- [ ] **E2E flake-avoidance patterns**: Timing-sensitive E2E work (canvas coordinates, `test.slow()` timeouts, post-reload locale waits, shard redistribution, stale cache-warmup CI) must follow the patterns in `.claude/agent-memory/e2e-test-engineer/flake-patterns.md`.
- [ ] **Read test titles *against* their assertions**: A test whose title states the contract but whose expectation encodes what the code currently does is a defect, not coverage. `reportContentGenerationService.test.ts` scenario 5 was titled "rounded to the nearest cent" and asserted `733` for a true `733.335` (PR #1916). When verifying a fix round, read the **deleted** test lines too — a relaxed assertion and a legitimate cleanup have the same diff shape.
- [ ] **Skipped is not passed**: When a CI gate aggregates job results, a `skipped` dependency must not be read as success unless the skip reason is "nothing to test". A failed upstream job also skips its dependents — this let `E2E Gates` report green with all 16 shards skipped on promotion PR #2041 (Issue #2043).
- [ ] **Confirm an E2E failure is a regression before attributing it to the PR**: Compare against the previous run on the same branch (`gh run list --branch <b>`, then `gh run view <id> --json jobs`) before treating a shard failure as caused by the change under review.

## LLM & Prompt Assembly

- [ ] **A constraint stated only in the prompt is not a guarantee**: For every behavioural rule the prompt asserts, identify what fails if the model ignores it. `prompts.test.ts`-style tests pin that the *instruction exists* — that is real coverage of the instruction and zero coverage of the outcome. #1932's plain-prose rule had no enforced counterpart and the render path is literal, so `**bold**` reached a bank-facing PDF (#1952).
- [ ] **Prompt content is usually untested**: Whenever money or other formatted numerics reach an LLM, assert the *rendered value* in the prompt, not merely that a label appears. Existing tests asserted `toContain('Invoice ID: inv-1')` and never a single amount (PR #1916).
- [ ] **Match the coerce-vs-reject policy the field already has**: If a validator truncates rather than throws for a field, a stricter failure mode for a *milder* violation is incoherent. One generation often yields several unrelated outputs — rejecting the whole response over a cosmetic defect in one field discards correct, expensive output and may fail identically on retry (#1952 ruled strip, not reject).
- [ ] **When the hardening is a text transform, false positives are the risk**: Domain punctuation collides with markup characters (`Pos. 3 - Dachstuhl`, `Rechnung #2024-117`, `Beträge < 500 EUR`, footnote `*`). A mangled reference number is worse than the markup, because nothing signals a character went missing. Write as many byte-identical-passthrough cases as stripping cases, plus "if the transform empties a non-empty value, keep the original".

## i18n — Translations

Expand Down