Skip to content

Conversation

ienugr
Copy link
Contributor

@ienugr ienugr commented Aug 8, 2025

GitHub Comment Testing Summary

Successful Tests:

  • Normal comment generation (6 violations)
  • SQL truncation (33 violations)
  • Multiple files handling
  • Error handling for oversized content

Large File Tests:

  • File: 60,000+ lines
  • Violations: 30,003 issues
  • Comment size: 1,165,116 characters (18x over limit)
  • Error caught: 'Comment body is too large'

Analysis:

  • The fix is working for normal use cases
  • Error handling prevents GitHub API failures
  • Summary mode logic may need adjustment for extreme cases

ienugr added 2 commits August 7, 2025 15:45
- 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
- Add normal comment generation test (001_demo_normal_comment.sql)
- Add SQL truncation test (002_demo_sql_truncation.sql)
- Add GitHub Actions workflow for testing
- Add documentation explaining the demo

This allows testing of the GitHub comment size limit fix
in a real PR environment to capture screenshots for documentation.
Copy link

netlify bot commented Aug 8, 2025

👷 Deploy request for squawkhq pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 852bcb2

@ienugr
Copy link
Contributor Author

ienugr commented Aug 8, 2025

Squawk Report

🚒 6 violations across 1 file(s)


migrations/001_demo_normal_comment.sql

-- Demo Migration 001: Normal GitHub Comment Generation
-- This migration demonstrates typical violations that should generate
-- a normal GitHub comment with full SQL content

BEGIN;

-- Adding NOT NULL field without default (violation)
ALTER TABLE users ADD COLUMN email varchar(255) NOT NULL;

-- Adding foreign key constraint (violation)  
ALTER TABLE posts ADD CONSTRAINT fk_posts_user_id 
    FOREIGN KEY (user_id) REFERENCES users(id);

-- Changing column type (violation)
ALTER TABLE products ALTER COLUMN price TYPE decimal(10,2);

-- Adding field with default value (violation)
ALTER TABLE users ADD COLUMN status integer DEFAULT 1;

COMMIT;

🚒 Rule Violations (6)

warning[adding-required-field]: Adding a new column that is `NOT NULL` and has no default value to an existing table effectively makes it required.
 --> migrations/001_demo_normal_comment.sql:8:19
  |
8 | ALTER TABLE users ADD COLUMN email varchar(255) NOT NULL;
  |                   --------------------------------------
  |
  = help: Make the field nullable or add a non-VOLATILE DEFAULT
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
 --> migrations/001_demo_normal_comment.sql:8:36
  |
8 | ALTER TABLE users ADD COLUMN email varchar(255) NOT NULL;
  |                                    ------------
  |
  = help: Use a `TEXT` field with a `CHECK` constraint.
warning[constraint-missing-not-valid]: By default new constraints require a table scan and block writes to the table while that scan occurs.
  --> migrations/001_demo_normal_comment.sql:11:19
   |
11 |   ALTER TABLE posts ADD CONSTRAINT fk_posts_user_id 
   |  ___________________-
12 | |     FOREIGN KEY (user_id) REFERENCES users(id);
   | |______________________________________________-
   |
   = help: Use `NOT VALID` with a later `VALIDATE CONSTRAINT` call.
warning[adding-foreign-key-constraint]: Adding a foreign key constraint requires a table scan and a `SHARE ROW EXCLUSIVE` lock on both tables, which blocks writes to each table.
  --> migrations/001_demo_normal_comment.sql:11:23
   |
11 |   ALTER TABLE posts ADD CONSTRAINT fk_posts_user_id 
   |  _______________________-
12 | |     FOREIGN KEY (user_id) REFERENCES users(id);
   | |______________________________________________-
   |
   = help: Add `NOT VALID` to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
warning[changing-column-type]: Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.
  --> migrations/001_demo_normal_comment.sql:15:41
   |
15 | ALTER TABLE products ALTER COLUMN price TYPE decimal(10,2);
   |                                         ------------------
   |
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/001_demo_normal_comment.sql:18:37
   |
18 | ALTER TABLE users ADD COLUMN status integer DEFAULT 1;
   |                                     -------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.

📚 More info on rules

⚡️ Powered by Squawk (2.21.1), a linter for PostgreSQL, focused on migrations

@ienugr
Copy link
Contributor Author

ienugr commented Aug 8, 2025

Squawk Report

🚒 33 violations across 1 file(s)


migrations/002_demo_sql_truncation.sql

-- Demo Migration 002: SQL Truncation Test
-- This migration has 60+ lines to test the SQL truncation feature
-- Expected: SQL content truncated at 50 lines with truncation notice

BEGIN;

-- Adding NOT NULL field (violation at the beginning)
ALTER TABLE users ADD COLUMN phone varchar(20) NOT NULL;

-- Generate enough content to trigger truncation
CREATE TABLE demo_table_1 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_2 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_3 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_4 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_5 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_6 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP

-- ... (17 more lines truncated for brevity)

⚠️ Note: SQL content has been truncated for display purposes. The full analysis was performed on the complete file.

🚒 Rule Violations (33)

warning[adding-required-field]: Adding a new column that is `NOT NULL` and has no default value to an existing table effectively makes it required.
 --> migrations/002_demo_sql_truncation.sql:8:19
  |
8 | ALTER TABLE users ADD COLUMN phone varchar(20) NOT NULL;
  |                   -------------------------------------
  |
  = help: Make the field nullable or add a non-VOLATILE DEFAULT
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
 --> migrations/002_demo_sql_truncation.sql:8:36
  |
8 | ALTER TABLE users ADD COLUMN phone varchar(20) NOT NULL;
  |                                    -----------
  |
  = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:12:8
   |
12 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:12:8
   |
12 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:13:10
   |
13 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:14:16
   |
14 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:15:16
   |
15 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:19:8
   |
19 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:19:8
   |
19 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:20:10
   |
20 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:21:16
   |
21 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:22:16
   |
22 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:26:8
   |
26 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:26:8
   |
26 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:27:10
   |
27 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:28:16
   |
28 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:29:16
   |
29 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:33:8
   |
33 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:33:8
   |
33 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:34:10
   |
34 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:35:16
   |
35 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:36:16
   |
36 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:40:8
   |
40 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:40:8
   |
40 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:41:10
   |
41 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:42:16
   |
42 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:43:16
   |
43 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:47:8
   |
47 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:47:8
   |
47 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:48:10
   |
48 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:49:16
   |
49 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:50:16
   |
50 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[changing-column-type]: Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.
  --> migrations/002_demo_sql_truncation.sql:65:47
   |
65 | ALTER TABLE products ALTER COLUMN description TYPE text;
   |                                               ---------
   |

📚 More info on rules

⚡️ Powered by Squawk (2.21.1), a linter for PostgreSQL, focused on migrations

@ienugr
Copy link
Contributor Author

ienugr commented Aug 8, 2025

Squawk Report

🚒 39 violations across 2 file(s)


migrations/001_demo_normal_comment.sql

-- Demo Migration 001: Normal GitHub Comment Generation
-- This migration demonstrates typical violations that should generate
-- a normal GitHub comment with full SQL content

BEGIN;

-- Adding NOT NULL field without default (violation)
ALTER TABLE users ADD COLUMN email varchar(255) NOT NULL;

-- Adding foreign key constraint (violation)  
ALTER TABLE posts ADD CONSTRAINT fk_posts_user_id 
    FOREIGN KEY (user_id) REFERENCES users(id);

-- Changing column type (violation)
ALTER TABLE products ALTER COLUMN price TYPE decimal(10,2);

-- Adding field with default value (violation)
ALTER TABLE users ADD COLUMN status integer DEFAULT 1;

COMMIT;

🚒 Rule Violations (6)

warning[adding-required-field]: Adding a new column that is `NOT NULL` and has no default value to an existing table effectively makes it required.
 --> migrations/001_demo_normal_comment.sql:8:19
  |
8 | ALTER TABLE users ADD COLUMN email varchar(255) NOT NULL;
  |                   --------------------------------------
  |
  = help: Make the field nullable or add a non-VOLATILE DEFAULT
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
 --> migrations/001_demo_normal_comment.sql:8:36
  |
8 | ALTER TABLE users ADD COLUMN email varchar(255) NOT NULL;
  |                                    ------------
  |
  = help: Use a `TEXT` field with a `CHECK` constraint.
warning[constraint-missing-not-valid]: By default new constraints require a table scan and block writes to the table while that scan occurs.
  --> migrations/001_demo_normal_comment.sql:11:19
   |
11 |   ALTER TABLE posts ADD CONSTRAINT fk_posts_user_id 
   |  ___________________-
12 | |     FOREIGN KEY (user_id) REFERENCES users(id);
   | |______________________________________________-
   |
   = help: Use `NOT VALID` with a later `VALIDATE CONSTRAINT` call.
warning[adding-foreign-key-constraint]: Adding a foreign key constraint requires a table scan and a `SHARE ROW EXCLUSIVE` lock on both tables, which blocks writes to each table.
  --> migrations/001_demo_normal_comment.sql:11:23
   |
11 |   ALTER TABLE posts ADD CONSTRAINT fk_posts_user_id 
   |  _______________________-
12 | |     FOREIGN KEY (user_id) REFERENCES users(id);
   | |______________________________________________-
   |
   = help: Add `NOT VALID` to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
warning[changing-column-type]: Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.
  --> migrations/001_demo_normal_comment.sql:15:41
   |
15 | ALTER TABLE products ALTER COLUMN price TYPE decimal(10,2);
   |                                         ------------------
   |
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/001_demo_normal_comment.sql:18:37
   |
18 | ALTER TABLE users ADD COLUMN status integer DEFAULT 1;
   |                                     -------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.

migrations/002_demo_sql_truncation.sql

-- Demo Migration 002: SQL Truncation Test
-- This migration has 60+ lines to test the SQL truncation feature
-- Expected: SQL content truncated at 50 lines with truncation notice

BEGIN;

-- Adding NOT NULL field (violation at the beginning)
ALTER TABLE users ADD COLUMN phone varchar(20) NOT NULL;

-- Generate enough content to trigger truncation
CREATE TABLE demo_table_1 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_2 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_3 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_4 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_5 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP
);

CREATE TABLE demo_table_6 (
    id SERIAL PRIMARY KEY,
    name VARCHAR(100),
    created_at TIMESTAMP DEFAULT NOW(),
    updated_at TIMESTAMP

-- ... (17 more lines truncated for brevity)

⚠️ Note: SQL content has been truncated for display purposes. The full analysis was performed on the complete file.

🚒 Rule Violations (33)

warning[adding-required-field]: Adding a new column that is `NOT NULL` and has no default value to an existing table effectively makes it required.
 --> migrations/002_demo_sql_truncation.sql:8:19
  |
8 | ALTER TABLE users ADD COLUMN phone varchar(20) NOT NULL;
  |                   -------------------------------------
  |
  = help: Make the field nullable or add a non-VOLATILE DEFAULT
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
 --> migrations/002_demo_sql_truncation.sql:8:36
  |
8 | ALTER TABLE users ADD COLUMN phone varchar(20) NOT NULL;
  |                                    -----------
  |
  = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:12:8
   |
12 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:12:8
   |
12 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:13:10
   |
13 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:14:16
   |
14 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:15:16
   |
15 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:19:8
   |
19 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:19:8
   |
19 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:20:10
   |
20 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:21:16
   |
21 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:22:16
   |
22 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:26:8
   |
26 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:26:8
   |
26 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:27:10
   |
27 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:28:16
   |
28 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:29:16
   |
29 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:33:8
   |
33 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:33:8
   |
33 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:34:10
   |
34 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:35:16
   |
35 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:36:16
   |
36 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:40:8
   |
40 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:40:8
   |
40 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:41:10
   |
41 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:42:16
   |
42 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:43:16
   |
43 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitting the max `int` limit.
  --> migrations/002_demo_sql_truncation.sql:47:8
   |
47 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use 64-bit integer values instead to prevent hitting this limit.
warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
  --> migrations/002_demo_sql_truncation.sql:47:8
   |
47 |     id SERIAL PRIMARY KEY,
   |        ------
   |
   = help: Use an `IDENTITY` column instead.
warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
  --> migrations/002_demo_sql_truncation.sql:48:10
   |
48 |     name VARCHAR(100),
   |          ------------
   |
   = help: Use a `TEXT` field with a `CHECK` constraint.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:49:16
   |
49 |     created_at TIMESTAMP DEFAULT NOW(),
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[prefer-timestamp-tz]: When Postgres stores a datetime in a `timestamp` field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time.
  --> migrations/002_demo_sql_truncation.sql:50:16
   |
50 |     updated_at TIMESTAMP
   |                ---------
   |
   = help: Use timestamptz instead of timestamp for your column type.
warning[changing-column-type]: Changing a column type requires an `ACCESS EXCLUSIVE` lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.
  --> migrations/002_demo_sql_truncation.sql:65:47
   |
65 | ALTER TABLE products ALTER COLUMN description TYPE text;
   |                                               ---------
   |

📚 More info on rules

⚡️ Powered by Squawk (2.21.1), a linter for PostgreSQL, focused on migrations

@ienugr
Copy link
Contributor Author

ienugr commented Aug 8, 2025

GitHub Comment Testing Summary

Successful Tests:

  • Normal comment generation (6 violations)
  • SQL truncation (33 violations)
  • Multiple files handling
  • Error handling for oversized content

Large File Tests:

  • File: 60,000+ lines
  • Violations: 30,003 issues
  • Comment size: 1,165,116 characters (18x over limit)
  • Error caught: 'Comment body is too large'

Analysis:

  • The fix is working for normal use cases
  • Error handling prevents GitHub API failures
  • Summary mode logic may need adjustment for extreme cases

@ienugr ienugr closed this Aug 8, 2025
kodiakhq bot pushed a commit that referenced this pull request Sep 2, 2025
## 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
- Added `GITHUB_COMMENT_MAX_SIZE` constant (65,000 chars) with safety margin
- Pre-check comment size before API calls to provide better error messages

### 2. Smart Comment Truncation
- Limit SQL preview to 50 lines per file with truncation notice
- Preserve all violation information while reducing content size

### 3. Summary Mode Fallback
- For very large reports, switch to summary mode that omits SQL content
- Display file statistics, line counts, and violation details
- Clear notice about content omission

### 4. Enhanced Error Handling
- New `CommentTooLarge` error variant with descriptive messages
- Better user feedback when size limits are exceeded

## Changes Made

- **`crates/squawk/src/github.rs`**: Main comment generation logic with size checking and fallback modes
- **`crates/squawk_github/src/app.rs`**: Enhanced error handling for comment size limits
- **`crates/squawk_github/src/lib.rs`**: New error variant for size-related failures

## Testing

### Successful Tests

#### Normal comment generation (6 violations)

<img width="790" height="1090" alt="image" src="https://github.com/user-attachments/assets/54246721-3902-4f2b-898c-ce915f89d97d" />

Reference: #608 (comment)

#### SQL truncation (33 violations)

<img width="863" height="651" alt="image" src="https://github.com/user-attachments/assets/fd191aaa-deb4-4967-b213-0ffade69c98c" />

Reference: #608 (comment)

#### Multiple files handling

<img width="853" height="1003" alt="image" src="https://github.com/user-attachments/assets/356271a8-4754-4ae7-b57c-f0adc208ed64" />

Reference: #608 (comment)

#### Error handling for oversized content

```
Error: Upload to GitHub failed

Caused by:
    Comment size error: Comment body is too large (1165116 characters). GitHub API limit is 65,536 characters.
```

Large File Tests:
- File: 60,000+ lines
- Violations: 30,003 issues
- Comment size: 1,165,116 characters (18x over limit)
- Error caught: 'Comment body is too large'

Analysis:
- The fix is working for normal use cases
- Error handling prevents GitHub API failures
- Summary mode logic may need adjustment for extreme cases

All existing tests continue to pass.

Fixes #603
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant