Conversation
- Add Ruby client SDK in clients/ruby/ with full HTTP API support - Implement auto-discovery of TDD server via .vizzly/server.json - Zero external dependencies (Ruby stdlib only) - Comprehensive test suite (unit + integration tests) - Add CI workflow to test Ruby 3.0-3.3 - Add release workflow with Claude AI changelog generation - Remove unused variant parameter from JS client - Update build-your-own-sdk.md with accurate API contract
Pull Request Review: Ruby Client SDKOverviewThis is an excellent addition to the Vizzly ecosystem! The Ruby client is well-architected, thoroughly tested, and follows Ruby best practices. The AI-powered release workflow is innovative and well-designed. ✅ StrengthsCode Quality
Testing
Release Workflow
🔍 Issues & RecommendationsCritical1. Workflow Security: Hardcoded Commit Push (.github/workflows/release-ruby-client.yml:131) The workflow pushes directly to main without protection. This bypasses any branch protection rules. Recommendation: Add a check to verify the release was triggered from main branch, or use a pull request workflow instead. 2. Version Parsing Edge Case (.github/workflows/release-ruby-client.yml:51) Bash array splitting may fail with non-standard versions. Doesn't validate version format or handle pre-release versions. Recommendation: Add version format validation before processing. High Priority3. Missing Error Handling in Changelog Step (.github/workflows/release-ruby-client.yml:68-104) The Claude Code action might fail, but the workflow continues without checking if CHANGELOG-RELEASE.md was created. Recommendation: Add validation step with fallback content if changelog generation fails. 4. HTTP Connection Not Closed (clients/ruby/lib/vizzly.rb:62-70) The Net::HTTP connection is created but never explicitly closed, which could lead to resource leaks. Recommendation: Use Net::HTTP.start with a block form to ensure connection cleanup. 5. Potential Race Condition in Integration Tests (clients/ruby/test/integration_test.rb:82-87) Polling for server readiness uses fixed retry count (3 seconds total). On slower CI systems, this might not be enough. Recommendation: Increase timeout to 10 seconds or add actual server health check. Medium Priority6. Inconsistent Error Re-raising Logic (clients/ruby/lib/vizzly.rb:107-109) The error re-raising check uses string matching which is brittle. Recommendation: Use a custom VisualDiffError exception class for cleaner error handling. 7. Missing HTTPS Support (clients/ruby/lib/vizzly.rb:61-62) The client doesn't handle HTTPS URLs properly. Recommendation: Add http.use_ssl support based on uri.scheme. 8. Module-Level State Not Thread-Safe (clients/ruby/lib/vizzly.rb:220-252) The shared client instance is not thread-safe in multi-threaded test environments. Recommendation: Use thread-local storage or document the limitation. Low Priority9. Magic Numbers (clients/ruby/lib/vizzly.rb:88, 207) Port 47392 is hardcoded. Extract to DEFAULT_TDD_PORT constant. 10. Gemspec Metadata Consider adding bug_tracker_uri and documentation_uri for better RubyGems integration. 🎯 SummaryThis is high-quality, production-ready code. The implementation shows strong understanding of Ruby idioms, excellent test coverage, and thoughtful error handling. Recommended Actions Before Merge
The other issues are minor improvements that can be addressed in follow-up PRs. Recommendation: Approve with minor changes 🚀 |
Summary
Adds a fully-featured Ruby client SDK for Vizzly with auto-discovery, comprehensive testing, and AI-powered release workflow.
What's Included
Ruby Client SDK (
clients/ruby/).vizzly/server.jsonCI/CD Integration
AI-Powered Release Workflow
workflow_dispatchtrigger with version bump (patch/minor/major)ruby/vX.X.X)Documentation
build-your-own-sdk.mdwith accurate API contractJS Client Cleanup
variantparameter from APIUsage
Release Process
Trigger via GitHub Actions:
Testing
Files Changed
clients/ruby/- New Ruby client implementation.github/workflows/ci.yml- Added Ruby client CI.github/workflows/release-ruby-client.yml- New release workflowsrc/client/index.js- Removed unused variant parameter🚀 Ready for first release!