Add binary file detection and large file handling

[simonw]

pypi-changelog seems very slow at rendering diff for some packages that contain MB+ binary files

Summary

Enhanced the wheel contents viewer to properly detect and handle binary files and large text files, preventing them from being displayed in diffs which improves performance and readability.

Key Changes

• Binary file detection: Added BINARY_EXTENSIONS set containing common binary file extensions (.so, .pyd, .dll, .png, .zip, .whl, etc.)
• Binary content detection: Implemented looksLikeBinary() function that checks for null bytes in the first 8KB of file content as a reliable binary indicator
• Filename-based filtering: Added isBinaryFilename() function to quickly identify binary files by extension before attempting to read them
• Large file handling: Introduced MAX_FILE_SIZE_FOR_DIFF constant (512KB) to skip files exceeding this threshold, displaying a message with file size instead
• Enhanced diff filtering: Updated diff generation logic in two locations to handle both binary files and large files using startsWith() checks instead of exact equality, allowing proper handling of the new "[Large file: XXkB]" format

Implementation Details

• Binary detection uses a two-pronged approach: fast extension-based filtering followed by content-based detection for edge cases
• Large files are identified after content is loaded, with their size displayed in KB for user awareness
• The diff generation now gracefully skips both binary and large files with appropriate messaging

https://claude.ai/code/session_01Ngz7FcMyqbhzvCoVH7xPjm

[claude]

Claude finished @simonw's task —— View job

---

PR Review Complete ✅

I've thoroughly reviewed the pull request for binary file detection and large file handling improvements. Here's my comprehensive feedback:

Todo List

• Read and analyze the modified file (pypi-changelog.html)
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage considerations
• Provide comprehensive feedback

---

Code Quality and Best Practices ✅

Excellent implementation overall! The code follows JavaScript best practices:

• Well-structured constants: BINARY_EXTENSIONS set and MAX_FILE_SIZE_FOR_DIFF constant are properly defined
• Clear function separation: Binary detection logic is properly separated into isBinaryFilename() and looksLikeBinary() functions
• Consistent naming: Function and variable names are descriptive and follow camelCase conventions
• DRY principle: The binary/large file detection logic is reused in both showDiff() and showArbitraryDiff() functions

Performance Considerations ✅

Strong performance optimizations implemented:

1. Fast path optimization (lines 353-359): Extension-based binary detection happens before file content is read, preventing unnecessary processing
2. Early content sampling (lines 361-365): Only checks first 8KB for binary detection instead of entire file
3. Size-based filtering (lines 386-388): Large files (>512KB) are excluded from diff generation to prevent UI slowdowns
4. Efficient skipping logic (lines 497-504, 666-673): Binary and large files are properly handled in diff generation with simple string comparisons

Potential Issues Found ⚠️

Minor Bug - Incomplete String Matching

Location: Lines 497, 666

const oldSkip = oldContent.startsWith('[Binary file]') || oldContent.startsWith('[Large file');
//                                                                                    ^^^^ Missing closing bracket

Issue: The second startsWith() check is missing the closing bracket ], so it will match "[Large file ABC]" but not the exact format "[Large file: 123KB]"

Fix needed: Should be oldContent.startsWith('[Large file:') or oldContent.startsWith('[Large file]')

Logic Consistency

The binary detection approach is sound, but there's a slight inconsistency in string matching patterns. Consider standardizing to use startsWith('[Large file:') for better specificity.

Security Considerations ✅

No security concerns identified:

• File extension checking uses a safe allowlist approach
• Binary detection uses standard null-byte checking
• No dynamic code execution or eval usage
• Content handling is properly escaped with escapeHtml()
• No direct file system access (works with zip contents only)

Test Coverage Considerations 📋

Recommended test scenarios (no existing tests found in the file):

1. Binary detection accuracy:

   • Test various binary extensions from BINARY_EXTENSIONS
   • Test content-based binary detection with null bytes
   • Test edge cases (files with binary extensions but text content)

2. Large file handling:

   • Test files exactly at 512KB threshold
   • Test size display formatting
   • Test behavior with very large files

3. Diff generation:

   • Test diff skipping for binary files
   • Test diff skipping for large files
   • Test mixed scenarios (some binary, some text files)

4. Error handling:

   • Test malformed zip files
   • Test network failures when fetching wheels

Suggestions for Improvement 🚀

1. Extract magic numbers: Consider defining BINARY_DETECTION_SAMPLE_SIZE = 8192 as a constant

2. Enhanced error handling: The catch (e) block in fetchWheelContents() (line 392) could provide more specific error information

3. Performance monitoring: Consider adding optional timing logs for large wheel processing

Summary

This is a high-quality implementation that effectively addresses the performance issues with large binary files in diff rendering. The two-tiered binary detection approach (filename + content) is robust, and the large file handling prevents UI performance degradation.

The only issue requiring attention is the minor string matching bug on lines 497 and 666. Otherwise, this PR significantly improves the user experience and application performance.

Recommendation: Approve after fixing the string matching bug ✅

[simonw]

pypdfium2 no longer hangs.

[cloudflare-workers-and-pages]

Deploying tools with    Cloudflare Pages

Latest commit:	bd2a5d2
Status:	⚡️  Build in progress...

View logs

[claude]

Claude finished @simonw's task —— View job

---

PR Review Complete ✅

I've thoroughly reviewed the pull request for binary file detection and large file handling improvements. Here's my comprehensive feedback:

Todo List

• Read and analyze the modified file (pypi-changelog.html)
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage considerations
• Provide comprehensive feedback

---

Code Quality and Best Practices ✅

Excellent implementation overall! The code follows JavaScript best practices:

• Well-structured constants: BINARY_EXTENSIONS set and MAX_FILE_SIZE_FOR_DIFF constant are properly defined
• Clear function separation: Binary detection logic is properly separated into isBinaryFilename() and looksLikeBinary() functions
• Consistent naming: Function and variable names are descriptive and follow camelCase conventions
• DRY principle: The binary/large file detection logic is reused in both showDiff() and showArbitraryDiff() functions

Performance Considerations ✅

Strong performance optimizations implemented:

1. Fast path optimization (lines 353-359): Extension-based binary detection happens before file content is read, preventing unnecessary processing
2. Early content sampling (lines 361-365): Only checks first 8KB for binary detection instead of entire file
3. Size-based filtering (lines 386-388): Large files (>512KB) are excluded from diff generation to prevent UI slowdowns
4. Efficient skipping logic (lines 497-504, 666-673): Binary and large files are properly handled in diff generation with simple string comparisons

Potential Issues Found ⚠️

Minor Bug - Incomplete String Matching

Location: Lines 497, 666

const oldSkip = oldContent.startsWith('[Binary file]') || oldContent.startsWith('[Large file:');
//                                                                                    ^^^^ Missing closing bracket

Issue: The second startsWith() check is missing the closing bracket ], so it will match "[Large file ABC]" but not the exact format "[Large file: 123KB]"

Fix needed: Should be oldContent.startsWith('[Large file:') or oldContent.startsWith('[Large file]')

Logic Consistency

The binary detection approach is sound, but there's a slight inconsistency in string matching patterns. Consider standardizing to use startsWith('[Large file:') for better specificity.

Security Considerations ✅

No security concerns identified:

• File extension checking uses a safe allowlist approach
• Binary detection uses standard null-byte checking
• No dynamic code execution or eval usage
• Content handling is properly escaped with escapeHtml()
• No direct file system access (works with zip contents only)

Test Coverage Considerations 📋

Recommended test scenarios (no existing tests found in the file):

1. Binary detection accuracy:

   • Test various binary extensions from BINARY_EXTENSIONS
   • Test content-based binary detection with null bytes
   • Test edge cases (files with binary extensions but text content)

2. Large file handling:

   • Test files exactly at 512KB threshold
   • Test size display formatting
   • Test behavior with very large files

3. Diff generation:

   • Test diff skipping for binary files
   • Test diff skipping for large files
   • Test mixed scenarios (some binary, some text files)

4. Error handling:

   • Test malformed zip files
   • Test network failures when fetching wheels

Suggestions for Improvement 🚀

1. Extract magic numbers: Consider defining BINARY_DETECTION_SAMPLE_SIZE = 8192 as a constant

2. Enhanced error handling: The catch (e) block in fetchWheelContents() (line 392) could provide more specific error information

3. Performance monitoring: Consider adding optional timing logs for large wheel processing

Summary

This is a high-quality implementation that effectively addresses the performance issues with large binary files in diff rendering. The two-tiered binary detection approach (filename + content) is robust, and the large file handling prevents UI performance degradation.

The only issue requiring attention is the minor string matching bug on lines 497 and 666. Otherwise, this PR significantly improves the user experience and application performance.

Recommendation: Approve after fixing the string matching bug ✅