fix(db): tolerate SQLite's optional COLUMN keyword in ALTER TABLE parsing (#8368)#8457
Conversation
…sing (JSONbored#8368) extractSchemaEvents' three ADD/DROP/RENAME regexes all required the literal COLUMN keyword, but SQLite's actual ALTER TABLE grammar makes it optional for all three forms -- ALTER TABLE t ADD x INTEGER, DROP x, and RENAME x TO y are all valid without it. A migration using the terser syntax produced zero SchemaEvents, invisible to detectColumnCollisions and silently defeating the CI gate this parser backs. Makes COLUMN optional in all three regexes; no other parsing or collision-detection logic changes. Adds test cases for the COLUMN-absent form of all three statement types alongside the existing COLUMN-present tests.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8457 +/- ##
===========================================
+ Coverage 59.66% 89.64% +29.98%
===========================================
Files 791 98 -693
Lines 79319 22838 -56481
Branches 23952 3908 -20044
===========================================
- Hits 47325 20473 -26852
+ Misses 28250 2187 -26063
+ Partials 3744 178 -3566
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Caution 🛑 LoopOver review result - reject/close recommendedReview updated: 2026-07-24 14:18:26 UTC
Review summary Nits — 3 non-blocking
Why this is blocked
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
LoopOver is closing this pull request on the maintainer's behalf (conflicts with the base branch — resolve and open a fresh PR; No linked issue detected). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Summary
extractSchemaEvents' three regexes (renameColumnMatch,dropColumnMatch,addColumnMatch) all required the literalCOLUMNkeyword, but SQLite's actualALTER TABLEgrammar makes it optional for all three forms —ALTER TABLE t ADD x INTEGER,ALTER TABLE t DROP x, andALTER TABLE t RENAME x TO yare all valid statements that omit it (verified directly against realsqlite3).SchemaEvents for that statement — completely invisible todetectColumnCollisions, silently defeating the CI gate this parser powers (db:migrations:check).\s+COLUMN\s+to\s+(?:COLUMN\s+)?in all three regexes so both forms are correctly detected. No other parsing or collision-detection logic changed.COLUMN-absent form of all three statement types, alongside the existingCOLUMN-present tests (all of which continue to pass unmodified).Closes #8368
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/db/migration-column-extraction.ts(pure regex/parsing logic, no UI/MCP/workers/OpenAPI surface) plus its test file. Verified vianpx vitest run test/unit/migration-column-extraction.test.ts: all 36 tests pass, including the 3 newCOLUMN-absent cases. Branch coverage on the changed conditions (verified directly againstcoverage/lcov.info): theif (renameColumnMatch)check shows both arms hit (BRDA:170,35,0,3/BRDA:170,35,1,517),if (dropColumnMatch)showsBRDA:179,36,0,64/BRDA:179,36,1,453, andif (addColumnMatch)showsBRDA:182,37,0,189/BRDA:182,37,1,264— every branch this diff touches is exercised on both sides.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots. (N/A — no visible UI changes.)Notes
ALTER TABLEvariants (e.g. multi-column operations) were added, and no other parsing/collision-detection logic changed.