UX Review — @leonievoss
Overall: well-scoped and independent of the other notification issues. Two things to nail before implementation.
🔴 High — Print stylesheet must be specified,…
Architect review (@mkeller): ✅ Do this today. One file, zero risk, no discussion needed. A test suite with a test that checks 1 + 2 === 3 is harder to trust than one without it.
Architect review (@mkeller): Descoping for now.
24 committed baseline screenshots (4 pages × 3 breakpoints × 2 color schemes) create significant friction during active UI development. Every…
Architect review (@mkeller): ✅ Well-reasoned and correctly scoped. Do this alongside #119.
One clarification: the title calls these "integration tests" but the implementation mocks fetch…
Architect review (@mkeller): Closing as duplicate of #118.
Both tickets cover the same goal — automated accessibility checks in the Playwright E2E suite. The difference is the package: -…
Architect review (@mkeller): ⚠️ Right direction, two concerns before implementing.
Scope: src/lib/** as the include covers Svelte components. Branch coverage on compiled Svelte…
Architect review (@mkeller): ⚠️ The JaCoCo config is correct, but the sequencing matters.
Do #119 (Testcontainers + real PostgreSQL) before adding this gate. If you add an 80% branch…
Architect review (@mkeller): ✅ Most important ticket in this batch — do this first.
H2 in test scope is a genuine liability. Flyway migrations never running in CI will eventually cause a…
Architect review (@mkeller): ✅ Approach is correct — @axe-core/playwright is the official Deque integration, the right package choice.
One implementation prerequisite: the spec uses…
Architecture review — @mkeller
Two concerns, one hard blocker.
🔴 Blocker: {@html renderBody()} is an XSS injection point
The plan says:
Use
{@html renderBody(comment)}in…
Architecture review — @mkeller
Overall the backend design is clean. Schema is minimal, email reuse is pragmatic, preference storage as two boolean columns on AppUser is the right call (no…