⚡ Skip image type detection when SDK provides explicit type

[Robdel12]

Summary

• Client SDK now sends type field ('base64' or 'file-path') with screenshot requests
• Server uses explicit type when provided, skipping O(n) detection entirely
• Fallback detection optimized with length check (strings > 2KB cannot be file paths)
• Fixed looksLikeFilePath to not false-positive on JPEG base64 (which starts with /9j/)

Problem

For large full-page screenshots (multi-megabyte base64 strings), the detectImageInputType function was running expensive O(n) regex validation on every screenshot. This caused noticeable slowdowns, especially for content-heavy pages like blog indexes and changelogs.

Solution

1. Explicit type from SDK (zero cost): The SDK already knows whether it's sending base64 or a file path, so it now tells the server explicitly
2. Optimized fallback: For backwards compatibility with old clients, detection now bails early on large strings (file paths are always < 2KB)

Test plan

• All existing tests pass
• Verified detection still works correctly for file paths and base64
• Backwards compatible - old clients without type field still work

[claude]

Pull Request Review

Overview

This PR introduces a smart performance optimization by having the client SDK explicitly declare the image type ('base64' or 'file-path'), eliminating expensive O(n) regex validation on large screenshot buffers. The optimization is well-designed with proper fallback for backwards compatibility.

---

✅ Strengths

1. Performance Optimization

• The fast-path approach (explicit type from SDK) completely eliminates detection overhead
• Length-based early bailout (2KB threshold in looksLikeFilePath, 1KB threshold in detectImageInputType) is smart and makes detection O(1) for large base64 strings
• The performance gains will be most noticeable for large full-page screenshots (multi-megabyte base64)

2. Backwards Compatibility

• Fallback detection ensures old clients without type field continue working
• The optimized detection still handles edge cases properly

3. Code Quality

• Clear comments explaining the optimization strategy
• JPEG base64 false-positive bug fix (/9j/ pattern) is a valuable improvement
• Logical ordering of checks (data URI → file patterns → length check → full validation)

---

⚠️ Issues & Concerns

1. Critical: Missing Test Coverage for New Functionality

The new type parameter is added to the client SDK and flows through the entire stack, but there are no tests verifying:

• Client SDK sends the correct type value for file paths vs base64
• Server handlers respect the explicit type parameter
• Backwards compatibility when type is undefined/null

Test files to update:

• tests/server/routers/screenshot.test.js (line 129-142): Add test cases with explicit type: 'base64' and type: 'file-path'
• tests/server/handlers/tdd-handler.test.js: Verify handler uses explicit type when provided
• tests/server/handlers/api-handler.test.js: Same verification needed
• tests/utils/image-input-detector.test.js: Add tests for large strings (>1000 chars) to verify the length optimization works

2. Inconsistent Length Thresholds

The codebase uses different thresholds without clear justification:

• looksLikeFilePath: str.length > 2000 → returns false
• detectImageInputType: str.length > 1000 → assumes base64

Why this matters: A string of 1500 characters would:

1. Pass through looksLikeFilePath early bailout (1500 < 2000)
2. Not trigger the length optimization in detectImageInputType (1500 > 1000)
3. Still run expensive isBase64 validation

Recommendation: Use consistent threshold (1000 for both) or add a comment explaining why they differ.

3. Potential Edge Case: Malformed type Parameter

The code uses: const inputType = type || detectImageInputType(image);

This means type: 'invalid-value' would pass through to file reading logic without validation.

Recommendation: Validate the type parameter or document that invalid types are intentionally handled downstream.

---

🔒 Security Considerations

No new security concerns - The changes don't introduce vulnerabilities:

• File path validation remains in place
• No new user input parsing beyond the type parameter
• Explicit type actually reduces attack surface by skipping regex validation

---

📊 Performance Considerations

Excellent performance improvement:

• Large screenshots (>1KB): O(n) → O(1) when type is explicit
• Large screenshots (>1KB): O(n) → O(1) even with fallback detection (length check)
• Small strings (<1KB): Minimal change, still runs full validation

Real-world impact:

• Multi-megabyte screenshots: Could save 10-100ms per screenshot
• High-frequency test suites: Savings multiply across hundreds of screenshots

---

🎯 Recommendations Summary

Must fix before merge:

1. Add test coverage for the new type parameter throughout the stack
2. Add tests for the length optimization (large strings)
3. Add test for JPEG base64 false-positive fix

Should fix before merge:
4. Align length thresholds (1000 vs 2000) or document reasoning
5. Add input validation for the type parameter

Nice to have:
6. Add JSDoc documentation for the new parameter
7. Add a benchmark test demonstrating the performance improvement

---

🏁 Conclusion

This is a high-quality optimization that addresses a real performance bottleneck with minimal code changes. The implementation is sound and backwards-compatible. However, the lack of test coverage for the core feature is a significant gap that must be addressed before merging.

With the recommended test additions and minor tweaks, this PR will be ready to ship.

Suggested next steps:

1. Add the missing test cases
2. Address the threshold inconsistency
3. Run the full test suite to ensure no regressions
4. Merge with confidence ✨