test: frontend view and component coverage gaps (#716)

[Chris0Jeky]

Summary

• Adds 107 new test cases across 8 new test files for previously untested or under-tested frontend views and components
• ArchiveView (11 tests): item restore flow (success, cancel, error, partial failure), empty/loading states, refresh action, entity type badges
• MetricsView (16 tests): retry button, CSV export (enabled/disabled/error), forecast section (loading, error, data, confidence band, caveats, null date), ARIA accessibility attributes
• BoardView (12 tests): loading/error states, column creation flow (submit, empty name, cancel), column rendering order, fetchBoard and realtime lifecycle, unmount cleanup
• ReviewView (10 tests): approve action, apply-to-board with confirmation, reject with reason, error toasts, summary card counts, two-step flow indicator, risk level badges
• AutomationChatView (16 tests): message sending (success, empty, error), message display and role labels, session creation/list/switching, loading state, LLM health states
• CardItem (21 tests): context move menu (show/hide/select/escape), blocked badge, label rendering with colors, selection/aria state, click/keyboard events, description display
• BoardCanvas (12 tests): empty state, column rendering with card counts, drag state CSS classes, drag event emissions, ARIA attributes
• BoardActionRail (9 tests): all button emissions, review-first trust hint, data attribute, primary button distinction

Closes #716

Test plan

• All 107 new test cases pass
• Full existing frontend test suite still passes (2349 total tests, 198 files)
• TypeScript typecheck passes

[Chris0Jeky]

Adversarial Self-Review

Findings

1. Good: Meaningful interaction testing

• Tests consistently use trigger('click'), check emitted() events, and verify toHaveBeenCalledWith arguments rather than just checking static text rendering
• The CardItem move menu tests verify the full open/select/close lifecycle, not just initial state
• ReviewView tests cover the confirm() → API call → toast chain

2. Good: Error path coverage

• ArchiveView: restore failure shows error toast AND keeps item in list
• MetricsView: CSV export error, forecast error with retry button
• ReviewView: approve/apply failure toasts
• AutomationChatView: session creation failure, message send failure

3. Good: Accessibility assertions

• BoardCanvas: role="group" and aria-label on column wrappers
• CardItem: tabindex, role="option", aria-selected, aria-haspopup/aria-expanded on move menu
• MetricsView: role="status" with aria-live on loading, role="alert" on error

4. Acceptable: Some tests check text content

• Tests like "shows empty state" and "renders all action buttons" use toContain() for text. This is appropriate for view-level tests where the contract IS the user-visible text.

5. Minor gap: No negative flow on BoardCanvas drag

• BoardCanvas tests check that drag events are emitted but don't test that columnDragStart is NOT emitted from the wrapper div itself (column DnD is handle-initiated in the actual ColumnLane). This is acceptable because ColumnLane tests already cover handle-initiated drag guardrails.

6. No remaining issues requiring fixes.
All tests pass, typecheck clean, patterns match existing test conventions.

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive suite of unit and integration tests for various frontend components and views, including BoardActionRail, BoardCanvas, CardItem, ArchiveView, AutomationChatView, BoardView, MetricsView, and ReviewView. The feedback focuses on improving test reliability and robustness. Specifically, it recommends consistently unmounting wrappers when using attachTo: document.body to prevent DOM pollution, replacing brittle Promise.resolve() chains with flushPromises() for handling asynchronous UI updates, and adding explicit assertions before using non-null operators to ensure clearer failure messages.

[gemini-code-assist]

The mountView helper uses attachTo: document.body, but the tests do not consistently unmount the wrapper. This leads to DOM pollution where components from previous tests remain in the document, which can cause side effects (like duplicate IDs or roles) or slow down the test suite. It is recommended to store the wrapper and unmount it in an afterEach block, or avoid attachTo if not strictly necessary for these tests.

[gemini-code-assist]

Using multiple await Promise.resolve() calls to wait for asynchronous UI updates is brittle and relies on internal implementation details of the microtask queue. It is better to use flushPromises() from @vue/test-utils (after importing it), which waits for all pending promises to resolve, ensuring the component has finished its async lifecycle before assertions run.

async function waitForAsyncUi() {
  await flushPromises()
}

[gemini-code-assist]

Using the non-null assertion operator (!) on the result of find() is an anti-pattern in tests. If the button is missing due to a regression, the test will fail with a generic TypeError instead of a clear assertion failure. It is better to explicitly expect the element to be defined before interacting with it.

Suggested change

	const btn = wrapper.findAll('button').find((b) => b.text().trim() === 'Capture here')
	await btn!.trigger('click')
	const btn = wrapper.findAll('button').find((b) => b.text().trim() === 'Capture here')
	expect(btn).toBeDefined()
	await btn!.trigger('click')

[Copilot]

Pull request overview

Adds targeted frontend unit tests to close coverage gaps for several high-impact views and board-related components, improving confidence in common user flows and UI states.

Changes:

• Added new coverage-focused specs for Review, Metrics, Board, Archive, and Automation Chat views.
• Added new coverage-focused specs for key board UI components (CardItem, BoardCanvas, BoardActionRail).
• Expanded assertions around loading/error/empty states and some accessibility attributes (ARIA roles/labels).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
frontend/taskdeck-web/src/tests/views/ReviewView.coverage.spec.ts	Adds tests for proposal review flows (approve/apply/reject), summary cards, risk badges, and error handling.
frontend/taskdeck-web/src/tests/views/MetricsView.coverage.spec.ts	Adds tests for retry/export behaviors, forecast rendering states, and ARIA attributes.
frontend/taskdeck-web/src/tests/views/BoardView.coverage.spec.ts	Adds tests for board loading/error, column creation, column ordering, realtime lifecycle, and help callout.
frontend/taskdeck-web/src/tests/views/AutomationChatView.coverage.spec.ts	Adds tests for chat send flow, session creation/list/switching, loading state, and LLM health banners.
frontend/taskdeck-web/src/tests/views/ArchiveView.coverage.spec.ts	Adds tests for archive restore flows, loading/empty/error states, refresh behavior, and entity type badges.
frontend/taskdeck-web/src/tests/components/CardItem.coverage.spec.ts	Adds tests for card move menu behavior, blocked/labels rendering, selection/accessibility, and interactions.
frontend/taskdeck-web/src/tests/components/BoardCanvas.coverage.spec.ts	Adds tests for empty state, column lane rendering, DnD CSS classes and emitted events, and ARIA labeling.
frontend/taskdeck-web/src/tests/components/BoardActionRail.coverage.spec.ts	Adds tests for action button rendering/emissions and a few UI hints/attributes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

createDeferred<typeof buildSession[]>() is typed as an array of the buildSession function type, not an array of session objects. This will fail TypeScript typechecking (and makes the deferred’s resolve signature wrong). Use createDeferred<ReturnType<typeof buildSession>[]>() (or a dedicated session type) instead.

Suggested change

	const deferred = createDeferred<typeof buildSession[]>()
	const deferred = createDeferred<ReturnType<typeof buildSession>[]>()

[Copilot]

Test name is misleading: it asserts that a board restore is not performed when the confirm dialog is cancelled (i.e., updateBoard is not called). Consider renaming to something like “does not restore board when confirm is cancelled” so the intent matches the assertions.

Suggested change

	it('does not cancel board restore when confirm is cancelled', async () => {
	it('does not restore board when confirm is cancelled', async () => {

[Copilot]

mountView() uses attachTo: document.body, but the mounted wrappers in these tests aren’t unmounted afterward. That leaves DOM nodes/component instances around across tests and can cause flaky behavior (event listeners, timers, Teleports). Consider either removing attachTo (if not needed) or adding an afterEach that unmounts the wrapper(s).

[Chris0Jeky]

Round 2 Adversarial Review

Critical (CI-blocking)

1. Lint failure: unused vi import in CardItem.coverage.spec.ts breaks CI
CardItem.coverage.spec.ts line 1 imports { describe, it, expect, vi } but vi is never used in the file. ESLint's @typescript-eslint/no-unused-vars fires as an error, which fails the Frontend Unit CI check on both Ubuntu and Windows. This is currently blocking the PR from merging.

Fix: Remove vi from the import.

---

Important

2. BoardView tests: DOM pollution from attachTo: document.body without cleanup
mountView() in BoardView.coverage.spec.ts mounts the component with attachTo: document.body but has no afterEach hook that unmounts the wrapper. Only one test (clears presence and stops realtime on unmount) explicitly calls wrapper.unmount(). The other 11 tests leak mounted components into the DOM across test runs, which can cause:

• Accumulated DOM nodes affecting query selectors in later tests
• Event listener leaks (SignalR stubs, watchers)
• Flaky failures when tests interact with stale DOM

Compare with ReviewView.coverage.spec.ts which correctly stores the wrapper in mountedWrapper and unmounts in afterEach.

Fix: Add let mountedWrapper + afterEach(() => { mountedWrapper?.unmount(); mountedWrapper = null }) and assign in each mountView() call, following the ReviewView pattern.

3. AutomationChatView: createDeferred<typeof buildSession[]>() has incorrect generic type
On the line:

const deferred = createDeferred<typeof buildSession[]>()

typeof buildSession resolves to the function type (overrides?: Record<string, unknown>) => {...}, not the return type. typeof buildSession[] is parsed as (typeof buildSession)[] -- an array of function types. The intended type is ReturnType<typeof buildSession>[].

This doesn't blow up at runtime because deferred.resolve() is never called in the test, but it's a type-safety hole and would fail if someone later tried to resolve the deferred with actual session objects.

Fix: createDeferred<ReturnType<typeof buildSession>[]>()

4. waitForAsyncUi() uses chained Promise.resolve() -- brittle async flushing
Multiple test files (ArchiveView, AutomationChatView, BoardView) use:

async function waitForAsyncUi() {
  await Promise.resolve()
  await Promise.resolve()
}

This is a fragile microtask tick hack. If the component adds another async step (e.g., a .then() chain), these tests will break silently. MetricsView.coverage.spec.ts already uses the correct approach: flushPromises() from @vue/test-utils.

Not blocking, but inconsistency within a single PR is a maintenance risk.

---

Minor

5. Non-null assertions (!) on find() results hide root-cause failures
Throughout all test files, button lookups use wrapper.findAll('button').find(...) followed by btn!.trigger('click'). If the button text changes due to a product update, the test fails with a cryptic TypeError: Cannot read properties of undefined instead of a clear "button not found" assertion. At minimum, adding expect(btn).toBeDefined() before the ! call (as gemini already suggested) would improve debuggability.

6. ArchiveView test name: "does not cancel board restore when confirm is cancelled"
This test name is confusing -- it reads as "does NOT cancel" which suggests the restore proceeds, but the test actually asserts that updateBoard is NOT called (i.e., the restore IS cancelled). A clearer name: "does not restore board when confirm is cancelled".

7. ReviewView: originalPrompt saved but window.prompt only overridden in one test
The beforeEach saves originalPrompt = window.prompt and afterEach would ideally restore it, but the test that uses window.prompt = vi.fn(...) sets it inline. The beforeEach/afterEach save/restore is good practice, but adding window.prompt = originalPrompt in the afterEach would be more defensive.

---

What Looks Good

• Interaction depth: Tests cover full user flows (open menu -> select -> verify emission -> verify menu closes), not just render checks.
• Error paths: Every view has explicit error-state tests (API rejection -> toast -> item stays in list).
• Accessibility coverage: ARIA roles, aria-selected, aria-expanded, aria-haspopup, role="status" with aria-live, keyboard navigation (Enter, Space, Escape) -- this is above-average for a test coverage PR.
• ReviewView cleanup: Proper afterEach unmounting, mountedWrapper pattern, real Vue Router with createMemoryHistory.
• ArchiveView restore flow: Tests cover success, cancel, API error, and partial failure (success=false) -- all four branches of the restore logic.