♻️ Improve reporter workflows and enable React Compiler

[Robdel12]

Why

We shipped reporter perf work recently, and there were still a few high-impact cleanup opportunities:

• Group actions were triggering mutation + refetch churn
• Settings could send invalid numeric values
• Builds could fetch projects before auth was confirmed
• React Compiler was not enabled yet for the reporter app

What changed

• Batched comparison group accept/reject flows to reduce repeated invalidations and network churn
• Added strict numeric validation in Settings (threshold/port/timeout) and disabled save while invalid
• Gated projects query by auth state in Builds
• Enabled React Compiler in all reporter Vite configs using @vitejs/plugin-react Babel plugin config
• Added reporter E2E coverage for:
  • signed-out users do not fetch projects
  • signed-in users do fetch projects
  • invalid numeric inputs keep Save disabled
• Extended reporter test helper to expose request logs for E2E request assertions

Test plan

• npm run build:reporter
• npm run build:reporter-ssr
• npm run test:reporter -- tests/reporter/specs/review-workflow.spec.js tests/reporter/specs/settings-workflow.spec.js tests/reporter/specs/builds-workflow.spec.js

All passed locally.

[claude]

Code Review

This is a solid performance optimization PR with good test coverage. The changes are well-structured and follow React best practices. Here's my detailed feedback:

✅ Strengths

1. Performance improvements are well-targeted:

   • Batching group accept/reject operations eliminates N mutations + N invalidations
   • React Compiler integration is properly configured across all Vite configs
   • Conditional project fetching based on auth state prevents wasteful requests

2. Strong test coverage:

   • New E2E tests verify both authenticated/unauthenticated project fetching behavior
   • Settings validation tests ensure invalid inputs disable the save button
   • Request logging helper is a clever addition for testing network behavior

3. Input validation is robust:

   • parseNumberInput and parseIntegerInput handle edge cases (null, empty, non-numeric)
   • Validation errors provide clear user feedback
   • Save button is properly disabled during invalid states

4. Batch mutation implementation is clean:

   • useAcceptBaselinesBatch and useRejectBaselinesBatch properly handle optimistic updates
   • Rollback on error is implemented correctly
   • Single invalidation after batch completion reduces refetch churn

🔍 Potential Issues

1. Batch operations lack atomicity (Medium Priority)

In use-tdd-queries.js:145-151, the batch mutations use Promise.all:

mutationFn: ids => {
  let idList = asIdList(ids);
  return Promise.all(idList.map(id => tdd.acceptBaseline(id)));
}

Issue: If one baseline fails, Promise.all rejects immediately, but some may have already succeeded. The optimistic update assumes all succeeded, creating a state mismatch.

Recommendation: Consider one of these approaches:

• Use Promise.allSettled and handle partial failures
• Create a true batch endpoint (/api/baseline/accept-batch) that's atomic server-side
• Track which items succeeded/failed and update optimistically only for successes

2. Form validation stores numeric values as strings (Low Priority)

In settings-view.jsx:21-23, form data converts numbers to strings:

threshold: String(config?.comparison?.threshold ?? 2.0),
port: String(config?.server?.port ?? 47392),
timeout: String(config?.server?.timeout ?? 30000),

While this works with the new validation approach, it's unconventional. React controlled inputs can handle numeric values directly if you use valueAsNumber or handle the conversion in onChange.

Impact: Minimal - the current approach works, but it's a bit non-idiomatic.

3. Missing error message for non-integer threshold values (Low Priority)

In settings-view.jsx:50-53, threshold validation only checks if it's a number ≥ 0:

let threshold = parseNumberInput(formData.threshold);
if (threshold === null || threshold < 0) {
  errors.threshold = 'Threshold must be a number greater than or equal to 0.';
}

The UI uses type="number" with step="0.1", which correctly allows decimals. However, if product requirements change and you want to restrict to integers (like port/timeout), you'd need to add that validation. Just worth noting for consistency.

🎯 Performance Considerations

1. React Compiler integration looks good - using Babel plugin via Vite's React plugin is the recommended approach.

2. Conditional query execution in builds-view.jsx:401 is correct:

   useProjects({ enabled: authenticated === true })

   This prevents unnecessary requests and should reduce network traffic for unauthenticated users.

3. useMemo for validation in settings-view.jsx:112-115 is appropriate given it runs on every form change.

🔒 Security Considerations

No significant security concerns. Input validation is client-side only (as expected for a local TDD tool), and the numeric parsing is safe.

🧪 Test Coverage

The new tests are comprehensive:

• ✅ Request tracking validates network behavior
• ✅ Authenticated vs unauthenticated flows are tested
• ✅ Invalid input states are verified

Suggestion: Consider adding a test for the group accept/reject batching behavior to ensure it actually batches requests (similar to the request tracking test).

📝 Code Quality

1. Consistent variable naming - good use of let consistently
2. Helper functions are well-isolated - getComparisonId, parseNumberInput, etc.
3. No unused variables - the removal of _totalToAccept in comparisons-view.jsx:230 is good cleanup

Minor Suggestions

1. Consider extracting validation messages to constants for easier i18n later (if that's ever needed)
2. The updateLoadingStateForIds helper could be memoized with useCallback's deps if you want to be extra careful, though it's probably fine as-is

Summary

This PR delivers exactly what it promises: better performance through batching, React Compiler support, and UX improvements via validation. The implementation is solid with good test coverage.

Recommendation: Approve with optional follow-up on the batch atomicity issue if you expect network failures to be common in practice.

Nice work! 🚀

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - Approved

19 comparisons approved.

Review changes

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{feat/reporter-react-compiler-cleanups · 35a7b262}