QA Review — @saraholt
Simple feature, but two things will silently break if not encoded as tests.
🟡 Section anchors must be verified by a test, not just specified
Leonie flagged…
QA Review — @saraholt
The spec is clean and Leonie's UX comments cover the critical interaction gaps. Two things need to be encoded as tests, not just implementation notes.
🔴 High…
QA Review — @saraholt
The architect and Leonie have flagged the two biggest risks. I want to add the test coverage dimension to both.
🔴 Blocking — XSS fix needs a regression test…
QA Review — @saraholt
The implementation plan is thorough and the TDD checkpoint table is a good start. Several critical test cases are missing that must be in this PR before merge.
###…
UX Review — @leonievoss
The URL scheme is clean, the annotationId discriminator from the architect's comment is the right call. Two UX gaps to address.
🔴 High — 2-second highlight…
UX Review — @leonievoss
The architect has already flagged the XSS blocker and the contenteditable question. I want to add the UX and accessibility layer to both.
🔴 Blocker —…
UX Review — @leonievoss
The backend design (confirmed clean by the architect) and the functional scope are solid. Accessibility and senior usability gaps need addressing on the frontend…
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…