♻️ Refactor TddService into functional decomposition

[Robdel12]

Summary

The TddService was doing way too much - 1,769 lines handling signature generation, baseline downloading, hotspot management, screenshot comparison, result aggregation, and baseline acceptance. Testing it required 7 vi.mock calls just to get started.

This PR breaks it down into focused, pure functions that are actually testable without all that mocking overhead.

What changed

New modules in src/tdd/:

• core/signature.js - Pure functions for signature/filename generation (this is the critical cloud sync point)
• core/hotspot-coverage.js - Pure math functions for hotspot coverage calculation
• metadata/baseline-metadata.js - Baseline metadata file I/O
• metadata/hotspot-metadata.js - Hotspot metadata file I/O
• services/baseline-manager.js - Local baseline CRUD operations
• services/comparison-service.js - Honeydiff wrapper and comparison result builders
• services/result-service.js - Result aggregation and summary calculation
• tdd-service.js - Thin orchestration layer that composes all these modules

New tests in tests/tdd/:

• 126 pure function tests with zero mocking - they test actual logic
• 9 real integration tests using actual filesystem and the real honeydiff binary
• All tests use temp directories that clean up after themselves

Backwards compatibility:

• src/services/tdd-service.js re-exports from the new location
• Plugins and existing code continue to work unchanged

Why this matters

The old approach made testing painful. Now:

• Pure functions can be tested without mocks
• Integration tests use real filesystem and real honeydiff
• Contract tests still ensure signature parity with cloud

Test plan

• All 910 tests pass
• New pure function tests (126 tests) work without mocking
• Integration tests use real temp dirs and real honeydiff
• Existing TddService tests still pass (backwards compat)
• Contract tests for signature parity still pass

[claude]

Code Review - PR #125

Overall Assessment

Verdict: ✅ APPROVED with minor suggestions

This is an excellent refactoring that dramatically improves code quality, testability, and maintainability. The transformation from a 1,769-line monolithic service into a functional decomposition with pure functions is well-executed.

✅ Strengths

1. Excellent Functional Decomposition

• Breaking TddService into focused modules (core/, metadata/, services/) creates clear separation of concerns
• Pure functions in core/signature.js and core/hotspot-coverage.js are highly testable without mocks
• Clean orchestration pattern in the new tdd-service.js

2. Outstanding Test Coverage

• 126 pure function tests with NO mocking - huge win for test reliability
• 9 real integration tests using actual filesystem and honeydiff binary
• 1,886 lines of new tests vs 1,763 lines of deleted code

3. Backwards Compatibility

• Re-export strategy in src/services/tdd-service.js ensures existing code continues to work
• Zero breaking changes for consumers

4. Critical Path Protection

• Signature/filename generation contract with cloud is preserved
• Contract tests remain in place to verify parity

5. Code Quality

• Consistent project style (let usage, naming conventions)
• Clear JSDoc comments
• Proper error handling

🔍 Minor Suggestions

1. Missing Error Logging (Minor) - src/tdd/metadata/baseline-metadata.js:28

• The catch block swallows errors without logging
• Suggestion: Add output.debug() to help troubleshooting

2. Error Documentation (Low) - src/tdd/services/comparison-service.js

• Add @throws documentation to compareImages JSDoc for honeydiff errors

3. Hotspot Loading Behavior (Question) - src/tdd/tdd-service.js:606-616

• If hotspotData exists but lacks a screenshot, never reloads from disk
• Likely intentional but worth confirming

🛡️ Security: ✅ All Checks Passed

• Path validation, screenshot sanitization, and property validation preserved
• No new file system operations without proper safety checks

⚡ Performance: ✅ Neutral to Positive

• Negligible function call overhead
• Smaller functions easier for V8 to optimize
• Pure functions enable future memoization

📋 Complete Checklist

• ✅ Code quality: Excellent
• ✅ No critical bugs detected
• ✅ Performance: No regressions
• ✅ Security: All checks maintained
• ✅ Test coverage: Outstanding
• ✅ Backwards compatibility: Preserved
• ✅ Contract tests: Passing

🎯 Recommendations

Before merge:

1. Consider adding debug logging to metadata parse errors
2. Add @throws documentation to compareImages

After merge:
3. Document new module structure in CLAUDE.md

Future opportunities:

• Memoization of signature generation
• Worker thread parallelization
• Easier unit testing of new features

🎉 Conclusion

This is a model refactoring that:

• Dramatically improves testability (126 mock-free tests)
• Maintains 100% backwards compatibility
• Preserves critical cloud sync contracts
• Follows functional programming best practices
• Sets a great example for future work

Production-ready with only minor documentation improvements needed. Great work!