Fix GitHub comment upload failure for large SQL linting reports#605
Merged
kodiakhq[bot] merged 5 commits intosbdchd:masterfrom Sep 2, 2025
Merged
Conversation
👷 Deploy request for squawkhq pending review.Visit the deploys page to approve it
|
sbdchd
requested changes
Aug 16, 2025
Owner
sbdchd
left a comment
There was a problem hiding this comment.
Thanks for the PR! Left some minor comments!
- Add size checking to prevent 422 errors when comment exceeds GitHub's 65,536 character limit - Implement smart comment truncation with SQL preview limited to 50 lines - Add fallback to summary mode for very large reports that omits SQL content but preserves all violation information - Include clear notices when content is truncated or summarized - Add comprehensive tests for comment size handling Fixes sbdchd#603
…rings - Extract shared comment formatting logic into format_comment() function - Remove duplicate markdown template from get_comment_body() and get_summary_comment_body() - Improve code maintainability by centralizing template structure - Update test snapshots to reflect cleaner output (removed extra blank line) - Fix truncate_sql_if_needed() function signature to match actual usage
757daf2 to
81be829
Compare
Contributor
Author
|
Hi @sbdchd, thanks for the feedback, I have updated the PR, kindly review it again. Thanks 🙏 |
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes issue #603 where Squawk fails to upload large linting reports to GitHub with a 422 "Unprocessable Entity" error.
Problem
When performing linting on large SQL migrations (14,000+ lines across multiple files), Squawk generates GitHub comments that exceed the API's 65,536 character limit, causing upload failures.
Solution
1. Size Limit Enforcement
GITHUB_COMMENT_MAX_SIZEconstant (65,000 chars) with safety margin2. Smart Comment Truncation
3. Summary Mode Fallback
4. Enhanced Error Handling
CommentTooLargeerror variant with descriptive messagesChanges Made
crates/squawk/src/github.rs: Main comment generation logic with size checking and fallback modescrates/squawk_github/src/app.rs: Enhanced error handling for comment size limitscrates/squawk_github/src/lib.rs: New error variant for size-related failuresTesting
Successful Tests
Normal comment generation (6 violations)
Reference: #608 (comment)
SQL truncation (33 violations)
Reference: #608 (comment)
Multiple files handling
Reference: #608 (comment)
Error handling for oversized content
Large File Tests:
Analysis:
All existing tests continue to pass.
Fixes #603