Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/e2e/fixtures/mock-api.ts`:
- Around line 57-60: The catch-all Playwright route handler (page.route)
currently fulfills requests with an incompatible body ({ data: [], total: 0 });
update the route.fulfill payload to match ApiResponse/ PaginatedResponse
envelopes by returning at minimum { data: [], error: null, error_detail: null,
success: true } for non-paginated endpoints and include a pagination object {
total: 0, offset: 0, limit: 10 } for paginated endpoints so tests receive the
expected fields; adjust the single catch-all in mock-api.ts (the page.route
callback that calls route.fulfill) to send the proper envelope based on the
request path or always return the full paginated envelope if that covers test
expectations.

In `@web/e2e/visual/dashboard.spec.ts`:
- Around line 9-15: The test's page.addInitScript sets auth_token and auth_user
but omits auth_token_expires_at which the auth store (init logic reading
auth_token and auth_token_expires_at) requires — add an auth_token_expires_at
key with a future timestamp so the token isn't cleared; update the addInitScript
block in dashboard.spec.ts (the page.addInitScript callback) to set
localStorage.setItem('auth_token_expires_at', new Date(Date.now() +
60*60*1000).toISOString()) and remove or ignore setting auth_user since the auth
store doesn't read it.

In `@web/package.json`:
- Line 14: The "analyze" npm script in package.json sets VITE_ANALYZE using
Unix-style assignment which breaks on Windows; add cross-env to devDependencies
and update the "analyze" script to use cross-env (referencing the "analyze"
script name in package.json) so the environment variable is set cross-platform,
then run npm install --save-dev cross-env to ensure it works on all platforms.

In `@web/playwright.config.ts`:
- Around line 49-53: Playwright's webServer is binding to port 4173 implicitly
(webServer.port) which can lead to cryptic failures if Vite's default or that
port changes; to fix, explicitly declare the preview port and strictPort in your
Vite config by adding a preview block (e.g., preview.port = 4173 and
preview.strictPort = true) in the project’s defineConfig export (vite.config.ts)
so the server fails fast when the port is unavailable and matches Playwright's
webServer.port.

In `@web/src/__tests__/hooks/useAutoScroll.test.ts`:
- Around line 23-30: The test currently asserts that scrollTop equals
scrollHeight but omits verifying that calling scrollToBottom() re-enables
auto-scroll; update the test for useAutoScroll to simulate a user scroll away
(e.g., create container with non-zero scrollTop and dispatch a 'scroll' event),
assert result.current.isAutoScrolling is false, then call
result.current.scrollToBottom() and assert result.current.isAutoScrolling is
true; reference the hook/useAutoScroll, its scrollToBottom method and
isAutoScrolling property (and helper makeContainer) when adding the assertion
and the pre-scroll simulation.

In `@web/src/__tests__/hooks/useBreakpoint.test.ts`:
- Around line 105-111: The test name is misleading: it either should be renamed
to reflect that it asserts the case where all media queries match (e.g.,
"returns desktop when all breakpoints match") when calling mockMatchMedia(true,
true, true) and rendering useBreakpoint via renderHook, or it should be changed
to exercise the SSR fallback by stubbing global window to undefined (use
vi.stubGlobal('window', undefined)) before rendering useBreakpoint and
remove/adjust mockMatchMedia; update the test title and assertions accordingly
so the name matches the actual behavior of useBreakpoint.

In `@web/src/__tests__/hooks/useCountAnimation.test.ts`:
- Around line 89-92: The test name is misleading: in useCountAnimation.test the
spec labeled "handles string values by parsing and formatting" actually passes a
number (42) and only asserts the return type; update the test to make behavior
and name consistent — either rename the it() description to something like
"returns a number when initialized with numeric value" (referencing the test
using useCountAnimation and result.current) or change the input to a string
(e.g., useCountAnimation('42')) and add assertions that the hook
parses/formatting occurs (assert parsed numeric value and any formatted output
the hook provides); modify the test body accordingly around renderHook(() =>
useCountAnimation(...)) and expect(...) to reflect the chosen fix.
- Around line 15-17: The cancelAnimationFrame mock currently calls
clearTimeout(id) but the requestAnimationFrame mock returns a different rafId,
so clearTimeout doesn't cancel the scheduled callback; update the mocks so the
id returned by window.requestAnimationFrame is the actual timeout handle used by
setTimeout (or keep a map of rafId->timeoutHandle) and make
window.cancelAnimationFrame clear the corresponding timeout; adjust the vi.spyOn
mocks for requestAnimationFrame and cancelAnimationFrame to return and clear the
same handle (or use an internal map) so cancelling actually prevents the
callback from running.

In `@web/src/__tests__/integration/ws-dashboard.test.tsx`:
- Around line 43-55: The test currently only checks activities.length; update it
to assert the actual activity content produced by
useAnalyticsStore.getState().updateFromWsEvent(event): retrieve activities from
useAnalyticsStore.getState().activities and find the activity corresponding to
task-99, then assert its fields (e.g., event_type === 'task.created', channel
=== 'tasks', payload.task_id === 'task-99', payload.title === 'New integration
task') and that the timestamp is present/parsable; use equality or partial
matching on those properties instead of only checking length.
- Around line 77-79: Replace the hardcoded "50" assertion with the shared
constant to avoid a magic number: import and use MAX_ACTIVITIES (the cap defined
in analytics.ts) in the test assertion that reads
useAnalyticsStore.getState().activities and assert activities.length <=
MAX_ACTIVITIES; if MAX_ACTIVITIES is not exported, either export it from
analytics.ts or add a short explanatory comment next to the expect line
referencing the analytics.ts cap to make the coupling explicit.

In `@web/src/__tests__/integration/ws-tasks.test.tsx`:
- Around line 58-86: The test uses a module-scoped Set named pendingTransitions
(defined in tasks.ts) which can leak state between tests if a failing test never
calls rollback(); ensure tests always clean up by either adding an afterEach
that clears pendingTransitions via a new helper (expose
clearPendingTransitions() on the useTasksStore or export a test-only
clearPendingTransitions function) or by calling that exposed clear method in the
test teardown; update tests (ws-tasks.test.tsx) to call the new
clearPendingTransitions() in an afterEach to guarantee isolation and reference
the existing optimisticTransition/rollback() flow
(useTasksStore.getState().optimisticTransition and rollback()) when locating
where to clear.

In `@web/src/components/layout/Sidebar.tsx`:
- Around line 146-156: Wrap calls to openCommandPalette so the tablet drawer's
FocusScope is unmounted first: when invoking openCommandPalette from
SidebarFooter (and the other call sites around the same component), first close
the drawer/overlay (via the existing toggleCollapse or the drawer-close helper
used by the Sidebar) and wait for the drawer to fully close/unmount (or
synchronously ensure the FocusScope is torn down) before calling
openCommandPalette; update the prop passed to SidebarFooter (openCommandPalette)
to be this wrapper function so the drawer trap cannot remain active when the
command palette opens.
- Around line 300-339: Replace the inline colored dot and text in Sidebar with
the shared StatusBadge component and add a polite live region for announcements:
determine status from wsConnected and wsReconnectExhausted, render <StatusBadge
...> (instead of the size-2 span) with the same title/aria-label logic, render
the textual label only when not collapsed, and add a visually-hidden element
(e.g., sr-only span) with aria-live="polite" (or role="status") that outputs the
same status string so changes are announced to screen readers; remove the inline
dot markup and ensure StatusBadge and the live-region use the
wsConnected/wsReconnectExhausted state values.
- Around line 119-159: The overlay/backdrop/focus logic in Sidebar is duplicated
instead of reusing the shared Drawer UI; replace the manual backdrop, aside and
FocusScope block with the shared Drawer component (imported from
web/src/components/ui/Drawer) and move the existing children into Drawer: pass
onOverlayClose as the Drawer close handler, set Drawer open state to the same
condition that renders this overlay, and render <SidebarNav collapsed={false} />
and <SidebarFooter ... /> as Drawer content so focus trapping and backdrop
behavior come from Drawer rather than the custom aside/FocusScope; ensure you
keep the close button handler (onOverlayClose), aria-labels, and props passed to
SidebarFooter (toggleCollapse, openCommandPalette, shortcutKey, wsConnected,
wsReconnectExhausted, user, logout).

In `@web/src/components/layout/StatusBar.tsx`:
- Around line 146-159: The WebSocket status span lacks an aria-live attribute;
update the span inside the StatusItem that renders the
wsConnected/wsReconnectExhausted text to include aria-live="polite" so screen
readers announce status changes (locate the StatusItem containing Dot and the
span that uses wsConnected and wsReconnectExhausted and add the attribute to
that span).

In `@web/src/components/ui/agent-card.tsx`:
- Around line 13-15: AgentCardProps is currently declared but not exported;
export the interface so consumers can import the prop types — update the
AgentCardProps declaration to "export interface AgentCardProps" and ensure any
files using these props (stories, tests, consumers) import { AgentCardProps }
from the component module; keep the AgentCard component and its flashStyle prop
unchanged, only change the interface declaration visibility.

In `@web/src/components/ui/empty-state.tsx`:
- Line 37: The EmptyState component currently always sets role="status" causing
unwanted screen-reader announcements; add an optional boolean prop announce
(default false) to the EmptyState props and change the JSX to only include
role="status" (and related aria-live attributes if present) when announce is
true, update the component's TypeScript prop type/interface (e.g.,
EmptyStateProps) and any default prop assignment, and audit usages to opt-in
only where live-region announcements are desired.

In `@web/src/components/ui/live-region.tsx`:
- Around line 3-10: The LiveRegionProps interface is currently unexported;
export it so external consumers (stories, tests, wrappers) can import the prop
contract directly. Update the declaration of LiveRegionProps to be exported
(export interface LiveRegionProps { ... }) and ensure any local references
(e.g., component props typing for the LiveRegion component) continue to use the
exported symbol.

In `@web/src/components/ui/sparkline.tsx`:
- Around line 11-12: The SparklineProps interface is currently defined but not
exported; update the declaration of SparklineProps to be exported (export
interface SparklineProps { ... }) so downstream code, stories and docs can
import and reuse the type; ensure any existing references to SparklineProps in
the same file (e.g., the Sparkline component signature) continue to compile
after making the interface exported.
- Around line 64-67: The Sparkline component currently falls back to aria-hidden
when ariaLabel is omitted, hiding meaningful charts from assistive tech; change
the prop typing to a discriminated union (e.g., make props either { decorative:
true; ariaLabel?: never } or { decorative?: false; ariaLabel: string }) in the
SparklineProps/interface so callers must provide ariaLabel unless explicitly
decorative, and update the JSX that spreads ARIA attributes (the existing
conditional using ariaLabel) to use the new decorative flag (render role="img"
and aria-label when ariaLabel is present, otherwise render aria-hidden when
decorative is true) so omissions are caught at compile time (refer to the
Sparkline component and SparklineProps type).

In `@web/src/components/ui/status-badge.tsx`:
- Around line 40-58: The animated branch in StatusBadge (the motion.span when
animated is true) omits the base color class from DOT_COLOR_CLASSES[color], so
initial paint can be transparent; update the className on the motion.span inside
the StatusBadge component to include DOT_COLOR_CLASSES[color] (alongside
'size-1.5 shrink-0 rounded-full' and pulse && 'animate-pulse') and keep
spreading motionProps as before so the animated background overlays the same
base color.

In `@web/src/components/ui/task-status-indicator.tsx`:
- Line 32: TaskStatusIndicator currently always sets role="status" (and should
also set aria-live="polite") which causes excessive screen-reader announcements;
add an optional announce boolean prop (default false) to the TaskStatusIndicator
props and only include role="status" and aria-live="polite" on the rendered
element when announce is true. Update the component signature/type to include
announce?: boolean = false, use conditional attributes (e.g., announce && {
role: "status", "aria-live": "polite" }) or conditional JSX props so existing
callers are unaffected, and ensure any places that rely on announcements pass
announce={true} where appropriate.

In `@web/src/hooks/useAutoScroll.ts`:
- Around line 26-43: The effect currently captures containerRef.current once and
may leave a listener on a stale element if the ref changes; update the useEffect
to read and bind to the current element each run and ensure cleanup removes the
listener from that same element by: inside the effect assign const current =
containerRef?.current; if (!current) return; add the listener to current via
current.addEventListener('scroll', onScroll, { passive: true }) and in the
cleanup call current.removeEventListener('scroll', onScroll); also include the
element identity in the dependency list (e.g. containerRef.current or a derived
stable refVal) along with thresholdPx so the effect re-runs when the container
element changes; keep onScroll, isAutoScrollingRef and setIsAutoScrolling logic
unchanged.

In `@web/src/hooks/useCountAnimation.ts`:
- Around line 22-25: When starting a new RAF tween in useCountAnimation, don't
use previousValueRef.current as the tween "from" value because that can differ
from what's actually rendered; instead compute the start from the current
displayed value (parse Number(display) or use display state) and cancel any
pending animationFrameRef before restarting. Update previousValueRef at the end
of the animation (or when target settles) rather than as the immediate source of
the next tween so the next tween begins from the onscreen number; ensure the
same change is applied to the code paths around the other tween starts
referenced (the blocks around the current target-change handling and the main
RAF loop).
- Line 21: Clamp and validate the public durationMs input in useCountAnimation:
ensure durationMs is coerced to a finite positive number (e.g., check
Number.isFinite and > 0) and fall back to DEFAULT_DURATION_MS if invalid, and
similarly guard any progress computations so the denominator is never zero/NaN;
update the declaration of durationMs and the progress/display calculations
(where progress is computed in the RAF loop) to use the validated/clamped value
so the RAF loop can terminate and display cannot overshoot or become NaN.

In `@web/src/hooks/useRovingTabIndex.ts`:
- Around line 59-90: The arrow-key handler in useRovingTabIndex derives
nextIndex from the stored focusedIndex which can be stale; update the handler to
compute the origin index from the actual DOM focus (e.g., const origin =
items.findIndex(el => el === document.activeElement) and fall back to
focusedIndex if origin === -1) and use that origin when calculating nextIndex,
then continue to clamp/loop and call setFocusedIndex and
items[nextIndex]?.focus(); reference the handler, focusedIndex, items,
setFocusedIndex and nextIndex when making the change.

In `@web/src/pages/dashboard/ActivityFeed.tsx`:
- Around line 22-28: The auto-scroll check uses visible.length which is capped
by MAX_VISIBLE so it never increases after the cap; change the effect to compare
the total activities count (e.g., activities.length or totalActivitiesLength)
against prevCountRef.current instead of visible.length, update
prevCountRef.current to that total count, and keep isAutoScrolling and
scrollToBottom in the deps so scrollToBottom is called when new activities
arrive even when visible is capped; reference the variables visible,
prevCountRef, isAutoScrolling, scrollToBottom, and MAX_VISIBLE when making the
change.

In `@web/src/pages/TaskDetailPage.tsx`:
- Around line 35-42: TaskDetailPage currently calls useWebSocket(...) and
ignores its return value, so setup failures are swallowed; change the call to
capture the returned state (e.g., const { connected, setupError } =
useWebSocket({ bindings: wsBindings })) and pass connected where needed, then
render the same warning banner pattern used on other pages when setupError is
non-null (show a dismissed/inline warning near the top of the TaskDetailPage
component) so users see WebSocket setup errors; locate the useMemo that defines
wsBindings and the useWebSocket call in TaskDetailPage and update them
accordingly, and use the same banner/component style as
OrgEditPage/TaskBoardPage for consistency.

In `@web/src/pages/tasks/TaskCreateDialog.tsx`:
- Around line 271-272: INPUT_CLASSES and TEXTAREA_CLASSES duplicate the same
focus-ring utility string; extract that shared string into a single constant
(e.g., FOCUS_RING_CLASSES or FOCUS_RING_TOKEN) and compose INPUT_CLASSES and
TEXTAREA_CLASSES by concatenating the original base class strings with that
shared token so both use the same focus-ring source and avoid duplication/drift.

In `@web/vite.config.ts`:
- Around line 45-51: The Vite config currently never uses VITE_ANALYZE so the
rollup visualizer never runs; update web/vite.config.ts to conditionally import
and add the visualizer plugin when process.env.VITE_ANALYZE (or
import.meta.env.VITE_ANALYZE) is truthy: dynamically import
'rollup-plugin-visualizer' (e.g., const { visualizer } = await
import('rollup-plugin-visualizer')) and push visualizer({ filename:
'stats.html', open: true }) into the plugins array in the exported config
(alongside existing plugins and build.rollupOptions.manualChunks) only when the
env flag is set. Ensure the import is dynamic to avoid requiring the dependency
in non-analyze runs and use the same env variable name used by the analyze
script.

---

Outside diff comments:
In `@web/src/pages/budget/CostBreakdownChart.tsx`:
- Around line 71-99: The custom radiogroup (using DIMENSION_OPTIONS, dimension,
onDimensionChange and deptDisabled) lacks proper roving-tab/arrow-key handling
and should be replaced with the shared SegmentedControl from
web/src/components/ui; switch the <div>+<button> map to a SegmentedControl, pass
the current value and onChange (use onDimensionChange), map DIMENSION_OPTIONS to
its items/labels, mark the 'department' option disabled when deptDisabled, and
preserve visual classes like FOCUS_RING and cn for styling and accessible
aria-label. Ensure the SegmentedControl receives the same aria-label ("Breakdown
dimension") and that disabled state and active styling mirror the previous
behavior.

In `@web/src/pages/budget/PeriodSelector.tsx`:
- Around line 16-38: Replace the custom radiogroup buttons with the shared
SegmentedControl component to get correct keyboard behaviour: import
SegmentedControl from web/src/components/ui/, pass the current value and
onChange, map PERIODS to SegmentedControl.Item (or the component's item API)
using period.value as key and period.label as content, and move any styling
(e.g. FOCUS_RING, cn-based classes) into the SegmentedControl/className props;
remove the manual role="radiogroup"/role="radio" and per-button tabindex logic
since SegmentedControl provides proper ArrowLeft/ArrowRight navigation and
roving tabindex out of the box.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/lighthouserc.js`:
- Around line 14-18: The current lighthouserc.js assertions under the assert
object use 'warn' severity for 'categories:performance',
'categories:accessibility', and 'categories:best-practices', which does not fail
the run when scores drop below 0.9; change the severity from 'warn' to 'error'
for those assertion entries so the 0.9 minScore is enforced as a failing
condition (update the assertions for 'categories:performance',
'categories:accessibility', and 'categories:best-practices' accordingly) and
re-run the Lighthouse check to verify the enforcement.
- Line 8: The configured Lighthouse test URL currently points at '/login' which
won't exercise the dashboard lazy bundles or WebSocket-driven views; update the
url array in lighthouserc.js (the url property) to include a route that actually
loads the new dashboard code (for example the dashboard entry route used by the
app such as '/dashboard' or the main app shell) so the Lighthouse run will load
CodeMirror, WebSocket layout, and task view bundles introduced in this PR.

In `@web/src/__tests__/hooks/useBreakpoint.test.ts`:
- Around line 105-109: Remove the duplicate test case named "returns desktop
when all breakpoints match" in useBreakpoint.test.ts: locate the redundant
it(...) block that calls mockMatchMedia(true, true, true) and renderHook(() =>
useBreakpoint()) and delete that entire test (the original test at lines ~58-66
should remain), ensuring no other tests or helper calls (mockMatchMedia,
renderHook, useBreakpoint) are altered.

In `@web/src/__tests__/hooks/useCountAnimation.test.ts`:
- Around line 73-81: The mid-animation assertion is too permissive because it
allows result.current === 100 which won't catch instant-jump regressions; update
the assertions in useCountAnimation.test.ts (the block that advances timers by
100ms and checks result.current) to assert the value is strictly between the
start and target (e.g., use toBeGreaterThan(0) and toBeLessThan(100) or
equivalent) so the test fails if the animation jumps immediately to the target.

In `@web/src/components/ui/command-palette.tsx`:
- Line 155: The component currently hardcodes responsive width with
"max-[1023px]:max-w-[calc(100vw-2rem)]"; update the wrapper in CommandPalette
(the element using that class string) to remove the calc and instead use design
token-based spacing and responsive width (e.g., use max-w-full or the tokenized
max-width plus the layout padding token such as the app's horizontal padding
token) so the component relies on layout padding tokens rather than hardcoded
calc values; ensure you replace the "max-[1023px]:max-w-[calc(100vw-2rem)]"
fragment with the appropriate token-based classes from the design system and
adjust the wrapper padding token to preserve the same visual gap at small
viewports.

In `@web/src/components/ui/status-badge.tsx`:
- Around line 35-39: The StatusBadge currently always includes role="status",
making it a polite live region; change the component to make live-region
behavior opt-in by adding a boolean prop (e.g., live or announce, default false)
and only include role="status" (and any aria-live if used) when that prop is
true; update the JSX that renders aria-label={statusLabel} and role="status" so
role is conditional (e.g., role={live ? 'status' : undefined}) and adjust the
component's props/type definition and usages (references: StatusBadge component,
statusLabel) accordingly.

In `@web/src/pages/budget/CostBreakdownChart.tsx`:
- Around line 73-74: The options prop mapping recreates a new array each render
when deptDisabled is true causing unnecessary re-renders of SegmentedControl;
wrap the transformed options in a useMemo that depends on deptDisabled and
DIMENSION_OPTIONS (e.g., const memoizedOptions = useMemo(() => deptDisabled ?
DIMENSION_OPTIONS.map(o => o.value === 'department' ? { ...o, disabled: true } :
o) : DIMENSION_OPTIONS, [deptDisabled, DIMENSION_OPTIONS]) and pass
memoizedOptions to SegmentedControl to avoid allocating a new array on every
render.

In `@web/src/pages/TaskDetailPage.tsx`:
- Around line 38-39: Extract the inline type import to a top-level type import
and simplify the handler signature: add a top import like "import type { WsEvent
} from '@/api/types'" at the top of TaskDetailPage.tsx, then change the handler
signature to use that type (handler: (event: WsEvent) => {
useTasksStore.getState().handleWsEvent(event) }) so the inline import is removed
and the code matches the file's existing import style; keep the call to
useTasksStore.getState().handleWsEvent(event) unchanged.

In `@web/vite.config.ts`:
- Around line 7-16: The vendor chunk for Radix in the VENDOR_CHUNKS constant
currently uses the literal 'radix-ui' which won't match scoped packages like
`@radix-ui/react-`*; update the 'vendor-ui' array in VENDOR_CHUNKS (the entry for
'vendor-ui') to replace 'radix-ui' with the scoped prefix '@radix-ui/' so scoped
Radix modules are included in that chunk (keep the rest of the array unchanged).

In `@web/vitest.config.ts`:
- Line 16: The test.exclude setting currently replaces Vitest's built-in
excludes; change it to extend configDefaults.exclude instead of replacing it:
import configDefaults from Vitest's config export (e.g., import { configDefaults
} from 'vitest/config') and set test.exclude to [...configDefaults.exclude,
'**/e2e/**'] so built-in patterns are preserved while adding the e2e exclusion;
update the variable used in web/vitest.config.ts where test.exclude is defined.

---

Outside diff comments:
In `@web/src/components/layout/SidebarNavItem.tsx`:
- Around line 42-54: The badge element in SidebarNavItem currently applies
aria-live="polite" to a conditionally rendered span so removals (badge -> 0)
won't be announced; update the component to place the live region on a
persistent parent (e.g., the wrapping element in SidebarNavItem) or use the
project's LiveRegion component if available, and move the aria-live and
aria-label logic there (use the badge and label props to construct the message
and still render the visual badge as-is) so both additions and removals of the
badge are reliably announced.

In `@web/src/components/ui/inline-edit.tsx`:
- Around line 128-142: The display-mode button in inline-edit.tsx is missing the
shared focus styling constant FOCUS_RING; update the button's className (the one
using cn(...) where startEditing is wired) to include FOCUS_RING (only when not
disabled) alongside the existing hover and disabled classes so the display
button matches the edit-mode input's focus ring for keyboard users; reference
the button that uses props/locals startEditing, renderDisplay, value,
placeholder, disabled, flashClassName and the FOCUS_RING constant when making
the change.

In `@web/src/components/ui/status-badge.tsx`:
- Around line 20-27: The StatusBadgeProps interface is currently internal but
should be exported for reuse; update the interface declaration for
StatusBadgeProps (used by the StatusBadge component) to be exported so stories,
tests, and other consumers can import the type instead of re-declaring it.
Ensure the exported name remains StatusBadgeProps and keep all existing fields
(status, label, pulse, className, animated) unchanged to preserve the component
contract.

In `@web/src/pages/tasks/TaskListView.tsx`:
- Around line 79-96: The sortable column header buttons (rendered in the mapping
over col) are missing the shared focus-ring utility used elsewhere; update the
button className expression in TaskListView's header rendering so that when
col.sortable is true it includes the FOCUS_RING class (alongside the existing
'cursor-pointer hover:text-foreground'), e.g. add FOCUS_RING to the cn(...) call
for the button that uses col.key, handleSort, sortKey and sortDir so the
ArrowUp/ArrowDown icons remain unchanged but keyboard focus shows the same ring
as AgentSpendingTable's ColumnHeader.

---

Duplicate comments:
In `@web/src/__tests__/hooks/useAutoScroll.test.ts`:
- Around line 23-45: The test misses asserting that invoking scrollToBottom
re-enables auto-scroll after the user scrolled away; update the second test for
useAutoScroll to, after dispatching the scroll event and asserting
isAutoScrolling is false, call result.current.scrollToBottom() (inside act if
needed) and then assert result.current.isAutoScrolling is true and
container.scrollTop equals container.scrollHeight to confirm scrollToBottom
restored auto-scrolling; reference the useAutoScroll hook, the scrollToBottom
method, and the isAutoScrolling property when adding the assertions.

In `@web/src/components/layout/Sidebar.tsx`:
- Around line 117-162: Replace the custom tablet overlay block (the if
(isOverlayMode) branch that checks overlayOpen and renders the backdrop div,
aside role="dialog", FocusScope, SidebarNav and SidebarFooter) with the shared
Drawer component from web/src/components/ui/, letting Drawer handle backdrop,
focus trap and dialog semantics; render <SidebarNav collapsed={false} /> and
<SidebarFooter ... /> as Drawer children, wire Drawer open state to overlayOpen
and close to onOverlayClose (or Drawer onOpenChange), and keep behavior for
toggleCollapse, openCommandPalette (call onOverlayClose?.() before
openCommandPalette), shortcutKey, wsConnected, wsReconnectExhausted, user and
logout so existing props are forwarded unchanged. Ensure you remove the backdrop
div, the aside element with role="dialog" and the FocusScope usage when
switching to Drawer.

In `@web/src/hooks/useAutoScroll.ts`:
- Around line 26-43: The scroll handler currently reads containerRef.current
inside onScroll while the listener is attached to the original element, which
can drift if containerRef.current changes; to fix, capture the current element
into a local const (el) when setting up the effect and use that captured element
inside onScroll (referencing el instead of containerRef.current), attach the
listener to that same captured el, and ensure the cleanup removes the listener
from that same captured el; update references to isAutoScrollingRef and
setIsAutoScrolling/thresholdPx as needed inside the handler so they operate on
the bound element identity in useEffect.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/e2e/fixtures/mock-api.ts`:
- Around line 71-79: The freezeTime helper currently only overrides Date.now()
so calls to new Date() still reflect the real system time; update freezeTime
(the function using page.addInitScript) to also override the global Date
constructor inside the page context: capture the original Date (e.g., const
OriginalDate = Date), compute fixedTime, set Date.now = () => fixedTime, then
replace Date with a function/constructor that delegates to OriginalDate when
called with arguments but returns a new OriginalDate(fixedTime) when called
without arguments, and ensure Date.prototype and static methods (like toString,
parse, UTC) are preserved by assigning Date.prototype = OriginalDate.prototype
and copying needed static properties so existing code like BudgetBurnChart.tsx
that calls new Date().toLocaleDateString() sees the frozen time.

In `@web/e2e/visual/login.spec.ts`:
- Around line 9-13: The test 'login page screenshot' currently captures a
screenshot immediately after page.goto and waitForFonts, which can be flaky;
after navigation (page.goto) and before
expect(page).toHaveScreenshot('login.png'), wait for the page to reach a stable
state by calling page.waitForLoadState('networkidle') (or equivalent) so all
network requests finish and animations settle; update the test function (the
test named "login page screenshot") to await
page.waitForLoadState('networkidle') (and keep waitForFonts) before taking the
screenshot to improve reliability.

In `@web/playwright.config.ts`:
- Around line 26-47: Tests are missing a mobile viewport project, so add a new
Playwright project entry in the projects array (e.g., name 'mobile-chromium')
that uses a mobile device or a mobile viewport (for example devices['Pixel 5']
or viewport { width: 375, height: 812 }) to ensure MobileUnsupportedOverlay and
useBreakpoint (<768px) behaviors are covered; update the projects array
alongside existing entries (desktop-chromium, desktop-sm-chromium,
tablet-chromium, desktop-firefox, desktop-webkit) so CI runs a mobile scenario
for visual/regression testing.

In `@web/src/__tests__/hooks/useAutoScroll.test.ts`:
- Around line 80-87: The test for useAutoScroll should wrap the call to
result.current.scrollToBottom() in act() to match other tests and avoid
warnings: update the null-ref case in the test to call act(() =>
result.current.scrollToBottom()), keeping the existing assertions
(isAutoScrolling) intact; ensure act is imported from your test utils (the same
act used elsewhere in these tests) and reference the useAutoScroll hook,
result.current.scrollToBottom, and isAutoScrolling when making the change.

In `@web/src/__tests__/hooks/useJustUpdated.test.ts`:
- Around line 4-155: Add property-based tests using fast-check for the
useJustUpdated hook to satisfy the repository standard: import fast-check in
web/src/__tests__/hooks/useJustUpdated.test.ts and add fc.property-based tests
that (1) assert TTL boundary invariants for markUpdated + isJustUpdated (given
random ttlMs and elapsed times, isJustUpdated should be true iff elapsed <
ttlMs), (2) assert re-mark resets TTL (markUpdated twice with random intervals
yields still-true if second mark + subsequent elapsed < ttlMs), and (3) assert
independent tracking for multiple IDs (generate arrays of timestamps/IDs and
ensure one ID expiring does not affect others). Use renderHook(() =>
useJustUpdated({ ttlMs })) and vi.useFakeTimers() / vi.advanceTimersByTime
within the property body, referencing markUpdated, isJustUpdated, relativeTime,
and clearAll to drive assertions; fail the property when any invariant is
violated.

In `@web/src/__tests__/hooks/useRovingTabIndex.test.ts`:
- Around line 4-14: Add a property-based test using fast-check in
web/src/__tests__/hooks/useRovingTabIndex.test.ts that generates arbitrary item
counts (e.g., 1–100) and for each run uses the existing makeContainer helper and
a container ref to renderHook(useRovingTabIndex({ containerRef: ref,
orientation: 'vertical' })); assert that result.current.focusedIndex is >= 0 and
< itemCount to ensure the hook maintains a valid focusedIndex for any container
size. Ensure you import fast-check (fc) and clean up the DOM between runs if
needed.

In `@web/src/components/layout/SidebarNavItem.tsx`:
- Around line 42-55: The aria-live on the conditionally rendered badge can cause
screen readers to miss updates; update SidebarNavItem to ensure the live region
is always present by either always rendering the badge element (the span that
currently checks badge !== undefined && badge > 0) and toggling its
visibility/styles when badge is 0 (e.g., hidden or visually hidden), or move
aria-live="polite" to a sibling element that is always rendered; keep the
aria-label logic using label and badge (e.g., `${badge} pending
${label.toLowerCase()}`) intact so announcements remain descriptive.

In `@web/src/components/ui/lazy-code-mirror-editor.tsx`:
- Around line 15-20: The LazyCodeMirrorEditor currently only uses Suspense and
will crash if the dynamic CodeMirrorEditor import fails; wrap the Suspense with
an error boundary (or ensure a higher-level one exists) so chunk-load/network
errors render a fallback UI instead of throwing. Update LazyCodeMirrorEditor to
render an ErrorBoundary (or reuse your app's existing ErrorBoundary component)
that shows an appropriate fallback (e.g., a friendly error card) around the
Suspense+CodeMirrorEditor; reference the LazyCodeMirrorEditor component and the
CodeMirrorEditor import so reviewers can locate and wrap Suspense (currently
returning <Suspense fallback={<SkeletonCard />}>).

In `@web/src/components/ui/metric-card.tsx`:
- Around line 30-32: The count animation hook is always running even when
animateValue is false; update useCountAnimation to accept an options object
(e.g., { enabled?: boolean }) and short-circuit the RAF loop when enabled is
false, then call useCountAnimation(numericValue ?? 0, { enabled: animateValue &&
numericValue !== undefined }) from MetricCard (references: useCountAnimation,
numericValue, animateValue, displayValue, MetricCard) so the hook does nothing
and does not start the RAF loop when animation is disabled.

In `@web/src/hooks/useJustUpdated.ts`:
- Around line 75-79: relativeTime currently returns a formatted string even
after the entry has expired (until GC removes it); make it mirror isJustUpdated
by returning null when the elapsed time since entriesRef.current.get(id) exceeds
the same TTL used by isJustUpdated. Update useJustUpdated's relativeTime to
compute const elapsed = Date.now() - timestamp, compare elapsed against the
shared TTL variable/parameter used by isJustUpdated, return null if elapsed >=
TTL, otherwise return formatRelative(elapsed); reference entriesRef,
relativeTime and isJustUpdated to locate where to apply this change.
- Line 37: The hook computes ttlMs from options but doesn't validate it; update
the logic in useJustUpdated (where ttlMs and DEFAULT_TTL_MS are used) to
validate that ttlMs is a finite number greater than 0 and fallback to
DEFAULT_TTL_MS if not; specifically, after const ttlMs = options?.ttlMs ??
DEFAULT_TTL_MS, replace with a validation that ensures
Number.isFinite(options?.ttlMs) && options.ttlMs > 0, otherwise set ttlMs =
DEFAULT_TTL_MS, so the hook never uses non-finite or non-positive TTL values.

In `@web/src/hooks/useRovingTabIndex.ts`:
- Around line 35-43: When updating item tabindexes in useRovingTabIndex, clamp
focusedIndex against the current items length to avoid leaving all items with
tabindex="-1" when the list shrinks: inside the useEffect that reads
containerRef and queries items via ITEM_SELECTOR, early-return if items.length
=== 0, compute a safeIndex = Math.min(focusedIndex, items.length - 1) (or 0 if
negative) and use safeIndex instead of focusedIndex when deciding which item
gets tabindex="0"; update any state setter (if present) to reflect the clamped
value so the hook stays consistent.

In `@web/src/stores/auth.ts`:
- Around line 150-156: The redirect block using currentPath and
window.location.href always sends users to '/login' and drops the original path;
update it to preserve a return path by capturing the encoded currentPath (unless
it's '/login' or '/setup'), then redirect to '/login?next=<encodedPath>' or
write it to a short-lived key in localStorage/sessionStorage before setting
window.location.href; also ensure the login completion flow (login
handler/component) reads that query param or storage key and navigates back to
the preserved path after successful auth.

---

Outside diff comments:
In `@web/src/__tests__/pages/DashboardPage.test.tsx`:
- Around line 52-54: Remove the unused mock fields wsConnected and wsSetupError
from the test's defaultHookReturn object and any related mock setup in
DashboardPage.test.tsx; if those properties were removed from the actual hook
return type, also update the UseDashboardDataReturn type (and any mock
factories) to reflect the current shape so tests and TS types remain consistent;
search for references to wsConnected and wsSetupError in the test file and
delete or update them accordingly to avoid stale/misleading mocks.

---

Duplicate comments:
In `@web/src/__tests__/integration/ws-dashboard.test.tsx`:
- Around line 70-85: The test hardcodes the expected cap (50) which can drift
from the source of truth; import the exported MAX_ACTIVITIES constant from the
analytics store module and use it in the assertion instead of 50. Update the
test to import MAX_ACTIVITIES (from the module that defines it alongside
updateFromWsEvent / useAnalyticsStore) and replace
expect(activities).toHaveLength(50) with
expect(activities).toHaveLength(MAX_ACTIVITIES); if MAX_ACTIVITIES is not
exported, export it from the analytics store where updateFromWsEvent and
activities are defined.

In `@web/src/hooks/useAutoScroll.ts`:
- Around line 26-43: The scroll listener reads containerRef.current but is
attached to the captured el, which can drift if the ref target changes; fix by
capturing the element at the top of useEffect (const el = containerRef?.current)
and reference that captured el inside onScroll instead of containerRef.current,
and ensure the effect depends on that captured el (add el to the dependency
array) so the listener is rebound and cleaned up against the correct element;
keep existing uses of thresholdPx and isAutoScrollingRef/setIsAutoScrolling in
onScroll.