🐛 Fix local TDD dimension mismatch handling (#222)

[Robdel12]

Summary

Local TDD was auto-overwriting the baseline when honeydiff threw a dimension mismatch.

This changes that path to keep the comparison reviewable instead of silently replacing the baseline.

History / Context

• Reported in Better handling for changed dimensions #222.
• The local overwrite behavior was introduced in b7cf403 (2025-10-22, multi-variant local TDD).
• It was carried forward during the TDD service refactor in b4c579a (2025-12-13).
• Cloud behavior does not do this overwrite fallback; comparison failures are treated as failures in processing paths.
• Honeydiff only throws Image dimensions don't match for width mismatches; variable height is supported.

What Changed

• src/tdd/tdd-service.js
• In the dimension mismatch catch path:
• Stop creating/replacing local baseline files.
• Stop updating baseline metadata.
• Build a failed comparison instead (status: failed) so it remains reviewable in the TDD UI.
• Keep baseline/current paths for side-by-side review and set diff to null.
• Add reason: 'dimension-mismatch' and preserve the error message.

Why This Keeps Compare Behavior Aligned

• The honeydiff compare call and options are unchanged.
• This only changes local fallback behavior after a compare error.
• Height behavior is unchanged (same-width/different-height still compares).

Tests

• npm run lint
• node --test tests/tdd/tdd-service.test.js
• node --test tests/server/handlers/tdd-handler.test.js

User Impact

• No more silent baseline overwrite on width mismatch.
• Teams can inspect baseline vs current and explicitly accept/reject in local TDD.

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - Approved

19 comparisons, no changes detected.

View build

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{fix/tdd-dimension-mismatch-handling-222 · e48334fe}

[claude]

Code Review

Summary

This PR fixes a critical bug where dimension mismatches in local TDD mode were silently overwriting baselines instead of presenting them as reviewable failures. The fix aligns local behavior with cloud behavior and improves the developer experience.

Code Quality ✅

Excellent refactoring:

• The code restructuring (moving effectiveThreshold and effectiveMinClusterSize outside the try block) is clean and necessary for the new error handling path.
• Proper use of the buildFailedComparison helper maintains consistency with other comparison paths.
• Clear, descriptive logging that aids debugging.

Good defensive programming:

• The mock honeydiff result ({ isDifferent: true, diffClusters: [] }) is sensible for a dimension mismatch scenario.
• Setting diffPath: null is correct since no diff can be generated for dimension mismatches.

Potential Issues 🔍

1. Missing properties from buildFailedComparison (Minor)

Looking at buildFailedComparison in src/tdd/services/comparison-service.js:186-232, it expects honeydiffResult to have these properties:

• diffPercentage (line 197)
• diffPixels (line 198)
• totalPixels (line 200)
• aaPixelsIgnored (line 201)
• aaPercentage (line 202)
• boundingBox (line 203)
• heightDiff (line 204)
• intensityStats (line 205)

Your mock only provides isDifferent and diffClusters. This could lead to:

• diffPercentage and diffCount being undefined in the comparison result
• Missing metrics in the TDD UI

Suggested fix (src/tdd/tdd-service.js:1432-1435):

honeydiffResult: {
  isDifferent: true,
  diffClusters: [],
  diffPercentage: 0,
  diffPixels: 0,
  totalPixels: 0,
  aaPixelsIgnored: 0,
  aaPercentage: 0,
  boundingBox: null,
  heightDiff: 0,
  intensityStats: null,
},

This would make the comparison result more complete and prevent potential UI rendering issues.

2. Test coverage is thorough ✅

The test update is excellent:

• Verifies baseline is NOT saved
• Verifies metadata is NOT saved
• Checks status, reason, and error message
• Validates debug logging

Performance ✅

No performance concerns. The refactoring doesn't add any new operations.

Security ✅

No security concerns. This is purely comparison logic.

Test Coverage ✅

The test update comprehensively covers the new behavior. Nice work tracking the baselineSaved and metadataSaved flags to ensure no side effects occur.

Architecture Alignment ✅

Excellent alignment:

• Local TDD now matches cloud behavior (no auto-overwrite on comparison failures)
• The comparison remains reviewable with side-by-side baseline/current views
• Clear error messaging (reason: 'dimension-mismatch')

This is exactly the right approach per CLAUDE.md's philosophy: "Avoid over-engineering. Only make changes that are directly requested or clearly necessary."

Recommendations

1. Consider adding complete honeydiffResult mock (see suggestion above) to prevent undefined properties in the UI.
2. Optional: Add an integration test that verifies the TDD UI can render dimension mismatch comparisons without errors (though this may be covered by reporter visual tests).

Verdict: ✅ Approve with minor suggestion

This is a well-executed bug fix that solves a real user pain point (#222). The code is clean, well-tested, and properly documented. The only minor concern is ensuring all comparison result properties are populated to prevent potential UI issues.

Great work! The refactoring is thoughtful and the test coverage is solid.