⚡ Singleton SSE connection + incremental updates for TDD dashboard

[Robdel12]

Summary

Follow-up to #213 (v0.29.2). Addresses two remaining performance issues in the TDD dashboard's real-time update system:

• Singleton SSE connection — useReportData() was called in 4 components, each creating its own EventSource to /api/events (4 concurrent connections + 4 file watchers). Moved SSE management to a SSEProvider React context that wraps the app once. Components now read connection state via useSSEState() context consumer.
• Incremental updates — The server was re-sending the entire report-data.json on every file change. Now it diffs against the last-sent data per connection and sends only comparisonUpdate, comparisonRemoved, and summaryUpdate events. For a 139-screenshot suite, this reduces per-event payload from ~50-200KB (full array) to ~0.5-2KB (single comparison).
• Fallback preserved — Initial connection still receives full reportData for complete state. Polling fallback activates when SSE is disconnected.

Files changed

File	Change
src/reporter/src/providers/sse-provider.jsx	New — singleton context managing one EventSource + all incremental event handlers
src/reporter/src/hooks/use-sse.js	Replaced 100-line per-component hook with 13-line context consumer
src/reporter/src/hooks/queries/use-tdd-queries.js	useReportDataSSE() → useSSEState()
src/reporter/src/main.jsx	Added <SSEProvider> wrapper inside <QueryClientProvider>
src/server/routers/events.js	Added diffing logic for incremental SSE events

Test plan

• npm run build:reporter — reporter builds without errors
• npm run lint + npm run format:check — clean
• npm test — all 1882 tests pass
• Manual: npm pack, install in vizzly e2e repo, run full suite (139 screenshots) — dashboard loads and updates correctly
• Verify single EventSource connection in browser DevTools Network tab (filter by EventStream)
• Watch SSE events — after initial full payload, subsequent events should be small patches

Closes #213

[claude]

PR Review: ⚡ Singleton SSE connection + incremental updates for TDD dashboard

Summary

This PR successfully addresses critical performance issues in the TDD dashboard's real-time update system. The changes are well-architected and align with React best practices. Overall, this is a strong improvement that reduces network overhead significantly.

Strengths ✅

Architecture & Design

• Excellent use of React Context: Moving SSE management to a singleton provider is the correct pattern. This eliminates the 4x redundant connections issue cleanly.
• Smart incremental diffing: The server-side diffing logic (src/server/routers/events.js:111-143) is well-structured and will dramatically reduce payload sizes (50-200KB → 0.5-2KB).
• Proper separation of concerns: The provider handles connection lifecycle, while components consume state via a simple hook.
• Backward compatibility: Fallback to full reportData when no previous data exists prevents edge cases.

Code Quality

• Clean refactoring: The migration from useReportDataSSE() to useSSEState() is minimal and clear.
• Good error handling: Silent catch blocks for JSON parsing are appropriate here (src/reporter/src/providers/sse-provider.jsx:64-66).
• Proper cleanup: Connection cleanup on unmount and error paths is correctly implemented (lines 142-152).

Issues & Recommendations 🔍

Critical: Missing Test Coverage for New Features ⚠️

The incremental update events (comparisonUpdate, comparisonRemoved, summaryUpdate) have zero test coverage. The existing tests/server/routers/events.test.js only tests the old reportData event.

Recommendation: Add tests for:

it('sends comparisonUpdate for changed comparisons', async () => {
  // Initial data with comparison A
  // Update file with changed comparison A
  // Assert comparisonUpdate event sent with only changed fields
});

it('sends comparisonRemoved when comparison is deleted', async () => {
  // Initial data with comparison A
  // Update file without comparison A  
  // Assert comparisonRemoved event with { id: 'A' }
});

it('sends summaryUpdate when summary fields change', async () => {
  // Initial data with total: 10
  // Update file with total: 11
  // Assert summaryUpdate event sent
});

it('sends no events when nothing changed', async () => {
  // Initial data
  // Update file with identical data
  // Assert sendIncrementalUpdates returns false
});

Potential Bug: Race Condition in Comparison Updates

In sse-provider.jsx:70-89, the comparisonUpdate handler has a potential race condition:

let idx = comparisons.findIndex(c => c.id === comparison.id);
if (idx >= 0) {
  comparisons = comparisons.map((c, i) =>
    i === idx ? { ...c, ...comparison } : c  // ⚠️ Shallow merge
  );
}

Issue: If comparison from the event only contains changed fields (as intended for incremental updates), this will overwrite the existing comparison object. If the server sends { id: 'foo', status: 'failed' } but omits other fields like name, diffPercentage, etc., those fields will be lost.

Fix: You need to ensure the server sends complete comparison objects in comparisonUpdate events, not just deltas. Looking at events.js:123, it does send newComp which should be complete, so this is likely fine—but worth verifying.

Recommendation: Add a comment clarifying that comparisonUpdate events contain the full comparison object, not a partial delta.

Performance: Inefficient Comparison Diffing

In events.js:80-87, the comparisonChanged function only checks 5 fields:

const comparisonChanged = (oldComp, newComp) => {
  return (
    oldComp.status !== newComp.status ||
    oldComp.diffPercentage !== newComp.diffPercentage ||
    oldComp.userAction !== newComp.userAction ||
    oldComp.timestamp !== newComp.timestamp ||
    oldComp.name !== newComp.name
  );
};

Concerns:

1. What if comparisons have other fields? If the comparison object has fields like browser, viewport, baseline, diff, etc., changes to those won't trigger updates.
2. Floating-point precision: diffPercentage comparison using !== might have precision issues.

Recommendation:

• Use a more robust deep comparison (e.g., JSON.stringify(oldComp) !== JSON.stringify(newComp))
• OR document explicitly which fields are tracked and why others are ignored
• OR use a shallow object comparison utility

Code Quality: Summary Diffing Uses Stringify

In events.js:101-105:

const summaryChanged = (oldData, newData) => {
  let oldSummary = extractSummary(oldData);
  let newSummary = extractSummary(newData);
  return JSON.stringify(oldSummary) !== JSON.stringify(newSummary);
};

Concerns:

• JSON.stringify() is reliable but inefficient for large objects
• Order-dependent (though probably fine here since data comes from same source)
• baseline metadata is included in summary, which might be large

Recommendation: Consider a shallow comparison or explicit field checks if performance matters here.

Minor: Inconsistent Error Handling

In sse-provider.jsx:124-137, connection errors trigger exponential backoff reconnection, which is great. However:

eventSource.onerror = () => {
  // ...
  setState(SSE_STATE.ERROR);
  setError(new Error('SSE connection failed'));  // Generic message

Issue: The error is generic and doesn't capture the actual failure reason. EventSource's onerror doesn't provide error details by spec, but you could inspect eventSource.readyState for more context.

Recommendation: Consider logging more diagnostic info (connection state, retry attempt) to help debug issues in production.

Documentation: Missing JSDoc for New Functions

The new diffing functions in events.js lack JSDoc comments:

• buildComparisonMap (line 68)
• comparisonChanged (line 80)
• extractSummary (line 93)
• summaryChanged (line 101)
• sendIncrementalUpdates (line 111)

Recommendation: Add JSDoc for consistency with the rest of the file.

Nitpick: React Context Default Value is Misleading

In sse-provider.jsx:12-15:

export let SSEContext = createContext({
  state: SSE_STATE.DISCONNECTED,
  error: null,
});

This default value will never be used because the context is always wrapped by <SSEProvider>. Consider using null or undefined as the default to catch accidental usage outside the provider.

export let SSEContext = createContext(null);

Then in useSSEState():

export function useSSEState() {
  const context = useContext(SSEContext);
  if (!context) {
    throw new Error('useSSEState must be used within SSEProvider');
  }
  return context;
}

Security Considerations 🔒

No security concerns identified. The PR doesn't introduce any new attack vectors:

• JSON parsing has try-catch protection
• No user input is processed
• SSE is read-only (no commands from client)

Performance Impact 📊

Excellent performance improvement:

• Reduces concurrent connections from 4 to 1
• Reduces payload size by ~98% (50-200KB → 0.5-2KB) for incremental updates
• No performance regressions identified

Minor optimization opportunity: Consider adding a check to skip diffing entirely if JSON.stringify(oldData) === JSON.stringify(newData) before building maps.

Manual Testing Checklist (from PR description)

The PR author should verify:

• Single EventSource connection in DevTools Network tab
• SSE events after initial load show small payloads (not full report data)

Final Verdict ✅

Approve with recommendations. The core implementation is solid and solves the stated performance problems effectively. The main gaps are:

1. Test coverage for incremental events (critical for confidence)
2. Clarification on comparison diffing logic (document which fields matter)
3. Minor code quality improvements (JSDoc, error messages)

Once tests are added for the new event types, this is ready to merge.

---

Files reviewed:

• src/reporter/src/providers/sse-provider.jsx:1-160
• src/reporter/src/hooks/use-sse.js:1-13
• src/reporter/src/hooks/queries/use-tdd-queries.js:17-18
• src/reporter/src/main.jsx:19-25
• src/server/routers/events.js:65-182
• tests/server/routers/events.test.js:1-358

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - 3 changes need review

Status	Count
Passed	13
Changed	3
Auto-approved	16

Changes needing review (3)

fullscreen-viewer · Firefox · 1920×1080 · 5.8% diff

viewer-zoomed-100 · Firefox · 1920×1080 · 0.5% diff

fullscreen-viewer · Firefox · 375×667 · 197.0% diff

Review changes

CLI TUI - Processing...

Build in progress...

---

_{fix/tdd-sse-singleton-incremental · 7a511b29}