bind:group requires a writable $state variable; $derived is read-only
in Svelte 5, so every click was silently reset to unchecked, making
the group picker non-functional.
Also wraps checkboxes in <fieldset>/<legend> for WCAG 1.3.1 compliance.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- load() fetches /api/groups in parallel with /api/invites; returns
sorted groups array and groupsLoadError for partial failures
- create action forwards groupIds[] to POST /api/invites so invited
users are placed in the selected groups on registration
- +page.svelte: group checkboxes via UserGroupsSection inside the form;
amber warning banner when groups could not be loaded
- page.svelte.test.ts: groups checkboxes + warning banner tests
- page.server.test.ts: parallel fetch, sorting, error fallback,
groupIds in POST body
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds the error code to the ErrorCode union and getErrorMessage() switch.
Adds admin_new_invite_groups, admin_invite_groups_load_error, and
error_group_has_active_invites to all three locale files (de/en/es).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract ImportStatus type to types.ts — removes duplication across
+page.svelte, ImportStatusCard.svelte, and test file (Felix blocker)
- Fix H2 to match CLAUDE.md card pattern: text-xs uppercase tracking-widest
text-ink-3 mb-5 (Leonie blocker 1)
- Add font-sans to RUNNING and DONE status labels (Leonie blocker 2)
- Add data-testid="processed-count" to count elements in both states
- Replace document.querySelector with locator API in spinner tests
- Tighten getByText('7') to getByTestId('processed-count') (Felix/Sara)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
toBeAttached() is not in the vitest-browser matcher set; toBeVisible() was
previously ruled out because the spinner is 0x0 px. Mirror the querySelector
pattern already used for the negative case in the same file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CI Chromium runs with German locale so hardcoded English strings like
'No spreadsheet file found.' never matched. Use m.admin_system_import_*()
to assert whatever locale the browser resolves to.
Spinner test used toBeVisible() on an empty <span> whose dimensions come
entirely from Tailwind CSS. Without layout CSS the span is 0×0 and fails
the visibility check; toBeAttached() asserts DOM presence, which is the
right semantic here.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three test files were written against the old API shape (raw `message` field) before
the statusCode i18n field was introduced, or used the wrong `expect` import path:
- ImportStatusCard.svelte.test.ts: `@vitest/browser/context` does not export `expect`
in this project's Vitest setup — use `vitest` like every other test file.
- page.svelte.spec.ts: FAILED mock lacked `statusCode`; assertion matched old German
raw message instead of the i18n string for IMPORT_FAILED_NO_SPREADSHEET.
- page.svelte.test.ts: same pattern — mock lacked `statusCode`; assertion checked for
raw backend string "database error" instead of the rendered i18n text.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove dead `message` field from both frontend ImportStatus types
(field is now @JsonIgnore'd on the backend)
- Extract failure message ternary into `$derived` — business logic off
the template (Felix)
- Add motion-reduce:animate-none to spinner — WCAG 2.1 SC 2.3.3 (Leonie)
- Replace text-green-600 with text-green-800 — WCAG AA contrast 6.1:1
on bg-green-50 (Leonie)
- Add min-h-[44px] to all three buttons — WCAG 2.2 44px touch target (Leonie)
- Add 6 missing tests: IMPORT_FAILED_INTERNAL path, IDLE state text,
null importStatus, ontrigger called on DONE/FAILED/IDLE buttons (Sara)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extracts the mass-import block from +page.svelte into ImportStatusCard.svelte.
Changes per the three UX fixes from issue #533:
- RUNNING: animated spinner (animate-spin) + processed count at text-base;
auto-poll at 2 s was already in place
- DONE: processed count at text-base, label at text-xs uppercase tracking-widest
- FAILED: maps statusCode (IMPORT_FAILED_NO_SPREADSHEET / IMPORT_FAILED_INTERNAL)
to Paraglide messages — no raw German backend string rendered
Adds vitest-browser tests covering spinner visibility, count display,
and per-statusCode FAILED message selection.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use [key: string]: unknown index signature so TS does not reject the
extra fields (location, status) passed to the redirect/failure result
in the spec helpers.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mirror the groups/new fix: replace inline beforeNavigate/isDirty with
createUnsavedWarning() + UnsavedWarningBanner and add an enhance callback
that calls clearOnSuccess() before update() on redirect results.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use createUnsavedWarning() + UnsavedWarningBanner to replace the inline
beforeNavigate/isDirty pattern, and add an enhance callback that calls
clearOnSuccess() before update() so the guard is disarmed before
SvelteKit's internal goto() fires on a redirect result.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CDP-based Playwright clicks (locator.click()) do not reliably trigger
Svelte 5 onclick handlers — documented in commit 0c765d81 which fixed
13 other specs. The layout dropdown tests were missed in that pass.
Applies the same pattern: ((await locator.element()) as HTMLElement).click()
for button interactions, and native KeyboardEvent dispatch for the Escape
test (dispatched on the button so it bubbles to the parent div's onkeydown).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Scans every src/**/*.svelte.{spec,test}.ts file for vi.mock first-arg
strings, canonicalises each by stripping a trailing .js/.ts after
.svelte, groups by canonical id, and fails if any canonical id is
referenced under two or more distinct raw spellings.
Mirrors the shape of src/__meta__/no-async-mock-factories.test.ts:
source-text regex scan (no AST parser dependency), red/green self-test
fixtures inline, then one corpus assertion that the whole suite is
clean.
This is the in-suite defence-in-depth layer for the duplicate-id birpc
race named in ADR-012 / #553 and fixed upstream by vitest PR #10267.
Harder to disable than ESLint (cross-file invariant ESLint cannot
express anyway) and harder to scope around than a CI grep.
Refs: #553
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five test files mocked $lib/shared/services/confirm.svelte under BOTH
spellings (.svelte and .svelte.js) within the same file; two more mocked
only the .svelte.js form. Both resolve to the same module URL but register
two distinct Playwright route handlers in @vitest/browser-playwright. The
cleanup logic only removes one, leaving an orphan that fires when the next
session loads the module — crashing the run with
"[birpc] rpc is closed, cannot call resolveManualMock".
This is the exact trigger fixed upstream by vitest PR #10267 (issue #9957).
Normalise every confirm.svelte mock to the no-extension form, matching
production imports and the source file basename (confirm.svelte.ts).
After this commit: 8 confirm.svelte mocks across 8 spec files, all under
one canonical ID. A meta-test (next commit) prevents the duplicate-id
pattern from reappearing.
Refs: #553 · vitest-dev/vitest#9957 · vitest-dev/vitest#10267
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Production code referenced $lib/shared/services/confirm.svelte under two
spellings — 4 files with the .js extension and one without. Standardise on
the no-extension form to match Svelte 5 rune-module convention and the
source file basename (confirm.svelte.ts).
Why this matters: vitest browser mode's @vitest/browser-playwright resolves
both spellings to the same module URL but registers a separate Playwright
route per spelling. The route-cleanup logic only unregisters the latest,
leaving an orphan that crashes the next session with
"[birpc] rpc is closed, cannot call resolveManualMock". Fixed upstream in
vitest PR #10267 (merged, not yet released). Normalising the spelling
removes the trigger from our side.
Refs: #553. Companion test-file changes follow in the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hover-prefetch has two surfaces in SvelteKit:
- data-sveltekit-preload-data (route loader data)
- data-sveltekit-preload-code (route JS chunks)
The original fix turned off only the loader-data side. Route-code chunks
prefetched on hover can also include manually-mocked module URLs; an
in-flight code prefetch landing after iframe teardown hits the same
Playwright route handler that resolves manual mocks, raising the
unhandled rejection. Disable both surfaces.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In-suite belt-and-braces detector for the birpc teardown race named in
ADR-012 / #553. Catches `vi.mock(<arg>, async ... { ... await import(...)
... })` in any browser spec on every vitest invocation — the layer hardest
to disable or scope around (ESLint can be silenced; CI grep runs only in
CI; this test runs whenever the suite runs).
Demonstrated red→green by planting a synthetic offender locally and
watching the live-scan assertion fail; removing the offender returned it
to green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hover-prefetch fires real fetch requests for route loader chunks; those
requests go through the same Playwright route handler that serves mocked
modules. An in-flight prefetch landing after iframe teardown can hit the
handler with a closed birpc channel, raising an unhandled rejection that
exits the run with code 1 even when every individual test was green.
Add `src/test-setup.ts` that sets `document.body.dataset.sveltekitPreloadData
= 'off'` and wire it via `setupFiles` in both `vite.config.ts` (client
project) and `vitest.client-coverage.config.ts` (Istanbul coverage config).
Add `src/__meta__/browser-preload-disabled.svelte.test.ts` asserting the
setup ran. Zero production impact.
Issue #553 secondary trigger.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The async vi.mock factory in EnrichmentBlock.svelte.spec.ts performed an
`await import(...)` in its body — the same mechanism #535/#546 fixed for
pdfjs-dist. Issue #553: when Chromium's playwright route handler fetches
the mocked module after the worker's birpc channel has closed, the
factory's RPC roundtrip raises `[birpc] rpc is closed, cannot call
"resolveManualMock"` and the run exits 1.
Migrate EnrichmentBlock from the deprecated `$app/stores.navigating`
(store) to the modern `$app/state.navigating` (reactive proxy). The
spec uses vi.hoisted + a sync vi.mock factory with a getter that defers
the read — no dynamic import in the factory body. Delete the now-unused
__mocks__/navigatingStore.ts.
Fix path applied: $app/state migration (Markus's recommendation /
Felix's Path 2). See ADR-012.
Refs #553
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The role=link override on a <button> creates a WCAG 4.1.2 keyboard-contract
mismatch: ARIA role=link tells AT users "press Enter to activate (Space does
nothing)", but the native <button> responds to both Enter and Space. Removes
the override so the element is announced as "button" (accurate).
Test selectors updated from getByRole('link') to getByRole('button')
accordingly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SvelteKit's capture-phase link interceptor fires before the component's
onclick handler, so e.preventDefault() was structurally too late to stop
iframe navigation in vitest-browser. Replacing the <a href> with a
<button type="button"> removes the href entirely — the interceptor never
fires — and the existing goto() mock in tests is sufficient.
Also splits the single view-all test into two focused it() blocks and
clears mocks in afterEach to prevent cross-test mock leakage.
Fixes#551
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract makeFakePdfjsLib / makeFakeLibLoader to testHelpers.ts — single
source of truth used by both PdfViewer.svelte.test.ts and
usePdfRenderer.svelte.test.ts; removes the diverging-fidelity DRY violation
flagged by @felixbrandt and @saraholt in the PR review
- Add 'loadDocument sets error and loading=false when getDocument().promise
rejects' test to usePdfRenderer.svelte.test.ts — closes the error-path gap
flagged by @felixbrandt and @saraholt
- Replace toBeInTheDocument() with toBeVisible() in the three absorbed
spec-file tests — uniform assertion style across the loaded-state describe
block, as flagged by @felixbrandt
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Five tests in usePdfRenderer.svelte.test.ts called createPdfRenderer() without
a libLoader, causing init() to dynamically import pdfjs-dist in the browser.
Every dynamic import goes through Playwright's route handler, which calls
resolveManualMock via birpc to check for mocks. If the RPC closes during
teardown while one of these imports is in flight, the birpc race fires —
even though pdfjs-dist was never explicitly vi.mock()-ed.
Replace all bare createPdfRenderer() calls that invoke init() with
createPdfRenderer(makeFakeLibLoader()), identical to the pattern already
used in PdfViewer.svelte.test.ts. No real module loads, no route-handler
calls, no birpc exposure.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Absorbs the three tests from PdfViewer.svelte.spec.ts (nav buttons, zoom
controls, page counter) into the loaded-state describe in test.ts, then
deletes the now-empty spec file. One spec file per component.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes both vi.mock('pdfjs-dist', …) calls that caused the birpc teardown
race (ADR 012). Replaces with static import + makeFakeLibLoader() helper
injected via the libLoader prop on every render() call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two root causes:
1. In-flight test: resolveFetch() was the last line, leaving the async
finally-block writing `training = false` after cleanup destroyed the
component. Awaiting the button becoming re-enabled ensures the finally
block settles before cleanup runs.
2. Success-dismiss test: startTraining() schedules setTimeout(5000) which
fired after cleanup destroyed the component. vi.useFakeTimers() +
vi.runAllTimers() scoped to the describe block drains the timer while
the component is still alive.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Svelte defers DOM updates to microtasks; .query() is a synchronous
snapshot that can fire before the element disappears — making the
absence assertions in AnnotationShape and AnnotationLayer non-deterministic.
Sweeps all 4 instances across both spec files (Sara's ≤5 threshold).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prototype-style assignment was a vi.mock hoisting artifact from the old
version of the file. Rest of the codebase uses class syntax — aligning.
Addresses Felix Brandt round-4 suggestion on PR #536.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds idempotency test: calling init() twice must invoke libLoader only once.
Adds `if (pdfjsReady) return;` guard to satisfy the contract.
Addresses Felix Brandt round-4 suggestion on PR #536.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
.catch(()=>{}) swallowed the rejection, so the test passed vacuously even
if a future refactor silently caught the error. rejects.toThrow() proves
the propagation contract holds before asserting pdfjsReady stays false.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Regression-protection test: init() propagates the loader rejection
before pdfjsReady is set, so the renderer stays in a safe unready state.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without untrack, a reactive libLoader prop reference change would
reinitialise the whole renderer and lose all loaded state.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Exporting LibLoader gives the type a stable, named identity.
PdfViewer.svelte and PdfViewer.svelte.spec.ts now import it directly
instead of using Parameters<typeof createPdfRenderer>[0].
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- import PdfViewer left mid-file from vi.mock hoisting — no longer needed (Sara/Felix)
- adds one-line comment explaining as unknown as cast is an intentional partial fake (Felix)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes both vi.mock('pdfjs-dist', factory) and
vi.mock('pdfjs-dist/build/pdf.worker.min.mjs?url', factory) from
PdfViewer.svelte.spec.ts — the ManualMockedModule registrations that were
racing with vitest-browser-playwright's birpc teardown channel.
PdfViewer.svelte now accepts an optional libLoader prop (typed as
Parameters<typeof createPdfRenderer>[0]) that is passed untracked to
createPdfRenderer(). Tests supply a vi.fn() fake loader directly as a prop;
production code uses the default loader that imports the real pdfjs-dist.
The birpc route handler for pdfjs-dist is never registered, so no teardown
race is possible. Fixes#535.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an optional LibLoader parameter (defaults to the real pdfjs-dist dynamic
imports) and a failing test that verified the loader is called during init().
This is the first step toward removing ManualMockedModule registrations that
race with vitest-browser-playwright's birpc teardown (#535).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
