fix: change build_size column type to bigint in analysis and master_image entities#1508
fix: change build_size column type to bigint in analysis and master_image entities#1508maxju wants to merge 1 commit intoPrivateAIM:masterfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
📝 WalkthroughWalkthroughThe pull request updates the Changes
Possibly Related Issues
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I have read the CLA Document and I hereby sign the CLA |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server-core/src/database/domains/analysis/entity.ts`:
- Around line 117-119: Add a bigint-to-number transformer for
AnalysisEntity.build_size (and similarly MasterImageEntity.build_size) to avoid
silent precision loss: create a transformer (e.g., BigIntToNumberTransformer)
used in the Column decorator for the property that converts the DB value
(string/bigint) to a JS number on read, validates it against
Number.MAX_SAFE_INTEGER and throws a clear error if it exceeds that limit, and
converts a JS number to string/bigint on write; update the Column(...) for
build_size to include this transformer and keep the TS type as number | null.
In `@packages/server-core/src/database/migrations/mysql/1775822138823-Default.ts`:
- Around line 15-21: In the down method of the migration (function
down(queryRunner: QueryRunner)), add pre-check queries for both
analysis_entity.build_size and master_images.build_size that SELECT
MAX(build_size) (or COUNT rows > 4294967295) and compare to UINT32_MAX
(4294967295); if any max exceeds that value, throw or reject with a clear error
message indicating rollback is unsafe due to out-of-range build_size values and
abort before executing the ALTER TABLE statements so the ALTERs for
analysis_entity and master_images are never attempted on unsafe data.
In
`@packages/server-core/src/database/migrations/postgres/1775822138823-Default.ts`:
- Around line 15-21: The down() migration should perform preflight range checks
on the build_size columns before altering types: use queryRunner.query to run a
SELECT (e.g. COUNT(*) or EXISTS) against "analysis_entity" and "master_images"
to detect any rows where build_size < -2147483648 OR build_size > 2147483647,
and if any are found throw/raise a clear error (including table name and
offending row count) to abort the migration instead of attempting the ALTER;
place these checks at the start of the down() method before the ALTER TABLE
statements and keep the ALTERs unchanged if checks pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a94666c-2de9-488d-a5b9-dd2600202498
📒 Files selected for processing (4)
packages/server-core/src/database/domains/analysis/entity.tspackages/server-core/src/database/domains/master-image/entity.tspackages/server-core/src/database/migrations/mysql/1775822138823-Default.tspackages/server-core/src/database/migrations/postgres/1775822138823-Default.ts
| type: 'bigint', unsigned: true, nullable: true, default: null, | ||
| }) | ||
| build_size: number | null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/server-core/src/database/domains/analysis/entity.ts | head -130 | tail -30Repository: PrivateAIM/hub
Length of output: 991
🏁 Script executed:
fd -type f -name "entity.ts" | xargs grep -l "master_images" | head -5Repository: PrivateAIM/hub
Length of output: 228
🏁 Script executed:
# Let's search for master_images entity definition
fd -type f -name "*.ts" | xargs grep -l "build_size.*bigint" | head -10Repository: PrivateAIM/hub
Length of output: 228
🏁 Script executed:
# Search for transformer patterns in the codebase to see how bigint is handled
rg "transformer:" packages/server-core/src/database/domains/ -A 5 -B 2 | head -60Repository: PrivateAIM/hub
Length of output: 1544
🏁 Script executed:
cat -n packages/server-core/src/database/domains/master-image/entity.ts | grep -A 3 -B 3 "build_size"Repository: PrivateAIM/hub
Length of output: 325
🏁 Script executed:
# Let's see the full context around bigint columns in both entities
rg "type: 'bigint'" packages/server-core/src/database/domains/ -A 3 -B 3Repository: PrivateAIM/hub
Length of output: 1268
🏁 Script executed:
# Look for any recent changes mentioning bigint or build_size
git log --oneline -20 -- packages/server-core/src/database/domains/master-image/entity.ts packages/server-core/src/database/domains/analysis/entity.ts 2>/dev/null || echo "Git history check"Repository: PrivateAIM/hub
Length of output: 149
🏁 Script executed:
# Get full master-image entity build_size definition with more context
sed -n '40,55p' packages/server-core/src/database/domains/master-image/entity.tsRepository: PrivateAIM/hub
Length of output: 447
🏁 Script executed:
# Get full analysis entity build_size definition with more context
sed -n '110,125p' packages/server-core/src/database/domains/analysis/entity.tsRepository: PrivateAIM/hub
Length of output: 463
🏁 Script executed:
# Check if there's a transformer anywhere for the master_image build_size
rg -A 15 "class MasterImageEntity" packages/server-core/src/database/domains/master-image/entity.ts | grep -A 15 "build_size"Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
# Check if there are other bigint columns with transformers to understand the pattern
rg -B 5 "type: 'bigint'" packages/server-core/src/database/domains/ | grep -A 10 "transformer" | head -40Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
# Look for the full master-image entity file to see all columns
wc -l packages/server-core/src/database/domains/master-image/entity.tsRepository: PrivateAIM/hub
Length of output: 125
🏁 Script executed:
# Get the master-image entity to see the complete context
head -60 packages/server-core/src/database/domains/master-image/entity.tsRepository: PrivateAIM/hub
Length of output: 1623
🏁 Script executed:
# Check for any recent changes in git log for context on what was changed
git diff HEAD~5..HEAD -- packages/server-core/src/database/domains/master-image/entity.ts 2>/dev/null | head -100 || echo "No diff found"Repository: PrivateAIM/hub
Length of output: 40
Add bigint conversion transformer for AnalysisEntity.build_size.
Line 117 uses type: 'bigint' but line 119 has build_size: number | null. TypeScript number safely represents integers only up to Number.MAX_SAFE_INTEGER (2^53 - 1), while SQL bigint extends to 2^63 - 1. This creates a precision risk on retrieval. The MasterImageEntity.build_size has the same issue. Add a transformer to validate against Number.MAX_SAFE_INTEGER:
Proposed fix
`@Column`({
type: 'bigint', unsigned: true, nullable: true, default: null,
+ transformer: {
+ to(value: number | null): number | null {
+ return value;
+ },
+ from(value: string | null): number | null {
+ if (value === null) return null;
+ const parsed = Number(value);
+ if (!Number.isSafeInteger(parsed)) {
+ throw new Error('analysis_entity.build_size exceeds Number.MAX_SAFE_INTEGER');
+ }
+ return parsed;
+ },
+ },
})
build_size: number | null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type: 'bigint', unsigned: true, nullable: true, default: null, | |
| }) | |
| build_size: number | null; | |
| type: 'bigint', unsigned: true, nullable: true, default: null, | |
| transformer: { | |
| to(value: number | null): number | null { | |
| return value; | |
| }, | |
| from(value: string | null): number | null { | |
| if (value === null) return null; | |
| const parsed = Number(value); | |
| if (!Number.isSafeInteger(parsed)) { | |
| throw new Error('analysis_entity.build_size exceeds Number.MAX_SAFE_INTEGER'); | |
| } | |
| return parsed; | |
| }, | |
| }, | |
| }) | |
| build_size: number | null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server-core/src/database/domains/analysis/entity.ts` around lines
117 - 119, Add a bigint-to-number transformer for AnalysisEntity.build_size (and
similarly MasterImageEntity.build_size) to avoid silent precision loss: create a
transformer (e.g., BigIntToNumberTransformer) used in the Column decorator for
the property that converts the DB value (string/bigint) to a JS number on read,
validates it against Number.MAX_SAFE_INTEGER and throws a clear error if it
exceeds that limit, and converts a JS number to string/bigint on write; update
the Column(...) for build_size to include this transformer and keep the TS type
as number | null.
There was a problem hiding this comment.
is this also an issue if no other applications use this db table?
There was a problem hiding this comment.
@maxju, yes, it's still an issue within the application itself, regardless of whether external apps access the table. The core concern is a TypeORM runtime behavior:
When TypeORM reads a bigint column from the database, it returns the value as a JavaScript string — not a number — to avoid precision loss. This means the declared TypeScript type number | null is inaccurate at runtime. Any code in this application that reads build_size and treats it as a number (e.g., arithmetic, JSON serialization, comparisons like build_size > threshold) will silently receive a string instead, which can lead to subtle bugs:
// At runtime, this is actually a string, not a number:
entity.build_size + 1 // "102401" instead of 10241
entity.build_size > 0 // always truthy (non-empty string)The transformer proposed in my review comment fixes both issues in one place: it converts the string TypeORM returns into a number, and optionally validates against Number.MAX_SAFE_INTEGER. Without it, the fix would be incomplete — the column type is bigint in the DB, but the application-level type contract is broken at runtime.
That said, if build_size is only stored/displayed and never used in calculations within this codebase, the practical risk is lower. Would you like me to check how build_size is consumed in the codebase?
| public async down(queryRunner: QueryRunner): Promise<void> { | ||
| await queryRunner.query(` | ||
| ALTER TABLE \`analysis_entity\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL | ||
| `); | ||
| await queryRunner.query(` | ||
| ALTER TABLE \`master_images\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL | ||
| `); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/server-core -name "*1775822138823*" -type fRepository: PrivateAIM/hub
Length of output: 212
🏁 Script executed:
cat -n packages/server-core/src/database/migrations/mysql/1775822138823-Default.tsRepository: PrivateAIM/hub
Length of output: 1078
🏁 Script executed:
# Search for TypeORM QueryRunner usage patterns in the codebase
rg -A 5 "queryRunner\.query\(" packages/server-core/src/database/migrations/ -t ts | head -100Repository: PrivateAIM/hub
Length of output: 10666
🏁 Script executed:
# Look for SELECT query patterns in migration files
rg -B 2 -A 5 "SELECT" packages/server-core/src/database/migrations/ -t ts | head -80Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
# Search for QueryRunner usage patterns more broadly, especially for SELECT/query results
rg "queryRunner\.query" packages/server-core/src -A 3 -B 1 -t ts | grep -A 3 -B 1 "SELECT\|COUNT\|const\|=" | head -100Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
# Check package.json for TypeORM version
cat packages/server-core/package.json | grep -A 2 -B 2 "typeorm"Repository: PrivateAIM/hub
Length of output: 236
🌐 Web query:
TypeORM 0.3.28 QueryRunner query SELECT result format return value
💡 Result:
Web search failed: Server error: no LLM provider could handle the message
🌐 Web query:
MySQL ALTER TABLE MODIFY COLUMN downcast bigint to int out of range error
💡 Result:
Web search failed: Server error: no LLM provider could handle the message
🏁 Script executed:
# Let's check TypeORM usage in the codebase for query result patterns
rg -B 5 -A 5 "await.*queryRunner\.query" packages/server-core/src/database -t ts | head -150Repository: PrivateAIM/hub
Length of output: 17770
🏁 Script executed:
# Look for TypeORM usage patterns with query results
find packages/server-core -name "*.ts" -type f -exec grep -l "queryRunner\.query" {} \; | head -10Repository: PrivateAIM/hub
Length of output: 367
🏁 Script executed:
# Check if TypeORM types are available in node_modules
find packages/server-core/node_modules/typeorm -name "*.d.ts" -path "*/query-runner*" 2>/dev/null | head -5Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
# Check TypeORM version in package-lock.json to understand the exact version
cat packages/server-core/package-lock.json | grep -A 10 '"typeorm"' | head -20Repository: PrivateAIM/hub
Length of output: 128
Guard rollback from bigint to int against out-of-range values.
Lines 17 and 20 will fail on rollback once any build_size exceeds 4,294,967,295 (INT UNSIGNED max). Add explicit pre-checks and fail with a clear error before altering types to prevent database rollback failures.
Proposed fix
public async down(queryRunner: QueryRunner): Promise<void> {
+ const [analysisCount] = await queryRunner.query(`
+ SELECT COUNT(*) AS c
+ FROM \`analysis_entity\`
+ WHERE \`build_size\` IS NOT NULL AND \`build_size\` > 4294967295
+ `);
+ const [masterCount] = await queryRunner.query(`
+ SELECT COUNT(*) AS c
+ FROM \`master_images\`
+ WHERE \`build_size\` IS NOT NULL AND \`build_size\` > 4294967295
+ `);
+ if (Number(analysisCount.c) > 0 || Number(masterCount.c) > 0) {
+ throw new Error('Rollback blocked: build_size exceeds INT UNSIGNED range.');
+ }
+
await queryRunner.query(`
ALTER TABLE \`analysis_entity\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL
`);
await queryRunner.query(`
ALTER TABLE \`master_images\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL
`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public async down(queryRunner: QueryRunner): Promise<void> { | |
| await queryRunner.query(` | |
| ALTER TABLE \`analysis_entity\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL | |
| `); | |
| await queryRunner.query(` | |
| ALTER TABLE \`master_images\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL | |
| `); | |
| public async down(queryRunner: QueryRunner): Promise<void> { | |
| const [analysisCount] = await queryRunner.query(` | |
| SELECT COUNT(*) AS c | |
| FROM \`analysis_entity\` | |
| WHERE \`build_size\` IS NOT NULL AND \`build_size\` > 4294967295 | |
| `); | |
| const [masterCount] = await queryRunner.query(` | |
| SELECT COUNT(*) AS c | |
| FROM \`master_images\` | |
| WHERE \`build_size\` IS NOT NULL AND \`build_size\` > 4294967295 | |
| `); | |
| if (Number(analysisCount.c) > 0 || Number(masterCount.c) > 0) { | |
| throw new Error('Rollback blocked: build_size exceeds INT UNSIGNED range.'); | |
| } | |
| await queryRunner.query(` | |
| ALTER TABLE \`analysis_entity\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL | |
| `); | |
| await queryRunner.query(` | |
| ALTER TABLE \`master_images\` MODIFY COLUMN \`build_size\` int UNSIGNED NULL | |
| `); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server-core/src/database/migrations/mysql/1775822138823-Default.ts`
around lines 15 - 21, In the down method of the migration (function
down(queryRunner: QueryRunner)), add pre-check queries for both
analysis_entity.build_size and master_images.build_size that SELECT
MAX(build_size) (or COUNT rows > 4294967295) and compare to UINT32_MAX
(4294967295); if any max exceeds that value, throw or reject with a clear error
message indicating rollback is unsafe due to out-of-range build_size values and
abort before executing the ALTER TABLE statements so the ALTERs for
analysis_entity and master_images are never attempted on unsafe data.
| public async down(queryRunner: QueryRunner): Promise<void> { | ||
| await queryRunner.query(` | ||
| ALTER TABLE "analysis_entity" ALTER COLUMN "build_size" TYPE integer | ||
| `); | ||
| await queryRunner.query(` | ||
| ALTER TABLE "master_images" ALTER COLUMN "build_size" TYPE integer | ||
| `); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "1775822138823-Default.ts" -type fRepository: PrivateAIM/hub
Length of output: 216
🏁 Script executed:
cat -n ./packages/server-core/src/database/migrations/postgres/1775822138823-Default.tsRepository: PrivateAIM/hub
Length of output: 1030
🏁 Script executed:
rg -A 5 "await queryRunner\.query\(" ./packages/server-core/src/database/migrations/ | head -100Repository: PrivateAIM/hub
Length of output: 11469
🏁 Script executed:
rg "queryRunner\.query.*SELECT" ./packages/server-core/src/database/migrations/ -A 3Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
rg "queryRunner" ./packages/server-core/src --type ts | grep -i "select\|query.*await" | head -20Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
fd "typeorm" ./packages/server-core -t d | head -5Repository: PrivateAIM/hub
Length of output: 40
🏁 Script executed:
cat ./packages/server-core/package.json | grep -A 5 -B 5 "typeorm"Repository: PrivateAIM/hub
Length of output: 399
Add preflight validation before rolling back bigint to integer.
The down() migration may fail if any build_size values in analysis_entity or master_images exceed PostgreSQL's integer bounds (−2147483648 to 2147483647). Add range checks before the type conversions to detect this issue early and abort with a clear error message.
Proposed fix
public async down(queryRunner: QueryRunner): Promise<void> {
+ const [{ c: analysisCount }] = await queryRunner.query(`
+ SELECT COUNT(*)::int AS c
+ FROM "analysis_entity"
+ WHERE "build_size" IS NOT NULL
+ AND ("build_size" > 2147483647 OR "build_size" < -2147483648)
+ `);
+ const [{ c: masterCount }] = await queryRunner.query(`
+ SELECT COUNT(*)::int AS c
+ FROM "master_images"
+ WHERE "build_size" IS NOT NULL
+ AND ("build_size" > 2147483647 OR "build_size" < -2147483648)
+ `);
+ if (analysisCount > 0 || masterCount > 0) {
+ throw new Error('Rollback blocked: build_size exceeds INTEGER range.');
+ }
+
await queryRunner.query(`
ALTER TABLE "analysis_entity" ALTER COLUMN "build_size" TYPE integer
`);
await queryRunner.query(`
ALTER TABLE "master_images" ALTER COLUMN "build_size" TYPE integer
`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public async down(queryRunner: QueryRunner): Promise<void> { | |
| await queryRunner.query(` | |
| ALTER TABLE "analysis_entity" ALTER COLUMN "build_size" TYPE integer | |
| `); | |
| await queryRunner.query(` | |
| ALTER TABLE "master_images" ALTER COLUMN "build_size" TYPE integer | |
| `); | |
| public async down(queryRunner: QueryRunner): Promise<void> { | |
| const [{ c: analysisCount }] = await queryRunner.query(` | |
| SELECT COUNT(*)::int AS c | |
| FROM "analysis_entity" | |
| WHERE "build_size" IS NOT NULL | |
| AND ("build_size" > 2147483647 OR "build_size" < -2147483648) | |
| `); | |
| const [{ c: masterCount }] = await queryRunner.query(` | |
| SELECT COUNT(*)::int AS c | |
| FROM "master_images" | |
| WHERE "build_size" IS NOT NULL | |
| AND ("build_size" > 2147483647 OR "build_size" < -2147483648) | |
| `); | |
| if (analysisCount > 0 || masterCount > 0) { | |
| throw new Error('Rollback blocked: build_size exceeds INTEGER range.'); | |
| } | |
| await queryRunner.query(` | |
| ALTER TABLE "analysis_entity" ALTER COLUMN "build_size" TYPE integer | |
| `); | |
| await queryRunner.query(` | |
| ALTER TABLE "master_images" ALTER COLUMN "build_size" TYPE integer | |
| `); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/server-core/src/database/migrations/postgres/1775822138823-Default.ts`
around lines 15 - 21, The down() migration should perform preflight range checks
on the build_size columns before altering types: use queryRunner.query to run a
SELECT (e.g. COUNT(*) or EXISTS) against "analysis_entity" and "master_images"
to detect any rows where build_size < -2147483648 OR build_size > 2147483647,
and if any are found throw/raise a clear error (including table name and
offending row count) to abort the migration instead of attempting the ALTER;
place these checks at the start of the down() method before the ALTER TABLE
statements and keep the ALTERs unchanged if checks pass.
1 participant
Entity files updated (type: int → bigint):
packages/server-core/src/database/domains/analysis/entity.tspackages/server-core/src/database/domains/master-image/entity.tspackages/server-core/src/database/migrations/postgres/1775822138823-Default.tspackages/server-core/src/database/migrations/mysql/1775822138823-Default.tsSummary by CodeRabbit