Add screenshot generation to build process

[simonw]

• Install shot-scraper dependency in build.sh
• Create generate_screenshots.py to capture 1024x768 0.9 quality JPEG screenshots
• Screenshot filenames include SHA256 hash of HTML content (e.g., dot.{hash}.jpeg)
• Skip screenshot generation if file with correct hash already exists
• Update build_colophon.py to display screenshots floated right with max-width 50%
• Screenshots saved in screenshots/ directory
• Excludes index.html and colophon.html from screenshot generation

🤖 Generated with Claude Code

[claude]

Claude finished @simonw's task —— View job

---

PR Review: Screenshot Generation Feature

Reviewing this pull request for code quality, best practices, potential bugs, performance considerations, security concerns, and test coverage.

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze build.sh changes
• Review generate_screenshots.py implementation
• Examine build_colophon.py modifications
• Check for security concerns
• Assess performance implications
• Review test coverage
• Provide comprehensive feedback

---

Review Feedback

✅ Strengths

1. Smart Caching Strategy

• The hash-based filename approach ({name}.{hash}.jpeg) in generate_screenshots.py:46-50 is excellent for cache invalidation
• Skip logic at generate_screenshots.py:52-55 prevents unnecessary regeneration
• Efficient file change detection using SHA256 hashing

2. Error Handling

• Good subprocess error handling in generate_screenshots.py:74-77 with detailed error output
• Proper dependency checking with shot-scraper --version validation
• Graceful handling of missing files in build_colophon.py:22-37

3. Code Structure

• Clean separation of concerns between screenshot generation and display logic
• Consistent hash calculation functions in both files (though duplicated)
• Proper use of pathlib for cross-platform compatibility

⚠️ Issues & Improvements

1. Code Duplication (Medium Priority)

# Duplicated in both generate_screenshots.py:9-15 and build_colophon.py:11-17
def get_file_hash(file_path):
    """Calculate SHA256 hash of file content."""

Recommendation: Extract to a shared utility module to follow DRY principle.

2. Security Considerations (Low Risk)

• subprocess.run() calls use shell=False (good)
• File paths are properly validated through pathlib
• No user input directly passed to shell commands
• Minor concern: No explicit validation that screenshot files are actual images

3. Performance Considerations

• Screenshots generated synchronously - could be slow for many files
• 1024x768 resolution and 90% quality are reasonable defaults
• Suggestion: Consider adding progress indicators for large batches

4. Missing Edge Cases

# generate_screenshots.py:25-28 - What if glob returns no files?
html_files = [
    f for f in Path(".").glob("*.html")
    if f.name not in ["index.html", "colophon.html"]
]

Recommendation: Add validation for empty results.

5. Error Recovery

• Screenshot generation continues even if individual files fail (good)
• However, no cleanup of partial/corrupted screenshot files
• Suggestion: Validate screenshot file integrity after creation

🔧 Technical Details

Build Process Integration:

• Proper integration in build.sh:11,17-18 with shot-scraper dependency
• Logical placement in build sequence (after links, before colophon)

CSS & Display:

• Responsive design with max-width: 50% and proper float clearing
• Good mobile considerations in media queries
• Border and border-radius provide nice visual polish

📋 Missing Test Coverage

The PR lacks tests for:

• Screenshot generation functionality
• Hash calculation consistency
• Error handling scenarios
• Integration with build_colophon.py

Recommendation: Add tests to tests/ directory covering core functionality.

🚀 Performance Impact

• Build time: Will increase due to screenshot generation, but caching mitigates repeated runs
• Storage: JPEG at 90% quality is a good balance
• Network: Screenshots will improve user experience on the colophon page

Overall Assessment

This is a solid implementation with good caching strategy and error handling. The main areas for improvement are:

1. Eliminate code duplication by extracting shared utilities
2. Add comprehensive tests for the new functionality
3. Consider async screenshot generation for better performance at scale

The security posture is good, and the feature adds genuine value to the site. The implementation follows Python best practices and integrates well with the existing build system.

Recommendation: ✅ Approve with suggested improvements

---

[cloudflare-workers-and-pages]

Deploying tools with    Cloudflare Pages

Latest commit:	8438a0e
Status:	✅  Deploy successful!
Preview URL:	https://a0495d04.tools-b1q.pages.dev
Branch Preview URL:	https://claude-add-screenshot-genera.tools-b1q.pages.dev

View logs