Skip to content

fix(db): tolerate SQLite optional COLUMN keyword in migration parsing (#8368)#8456

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
joaovictor91123:fix/migration-column-extraction-optional-keyword-8368
Jul 24, 2026
Merged

fix(db): tolerate SQLite optional COLUMN keyword in migration parsing (#8368)#8456
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
joaovictor91123:fix/migration-column-extraction-optional-keyword-8368

Conversation

@joaovictor91123

Copy link
Copy Markdown
Contributor

Summary

  • src/db/migration-column-extraction.ts's extractSchemaEvents powers the CI-gating column-collision check (scripts/check-migrations.ts / db:migrations:check). Its three ALTER TABLE ... COLUMN ... regexes (RENAME/DROP/ADD) all required the literal COLUMN keyword, but SQLite's actual grammar makes it optional for all three forms (ALTER TABLE t ADD x INTEGER, DROP x, RENAME x TO y are all valid, verified directly against real sqlite3). A migration using the terser syntax produced zero SchemaEvents and was silently invisible to collision detection. Made COLUMN optional (\s+(?:COLUMN\s+)?) in all three regexes.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Closes #8368

Validation

  • git diff --check
  • npm run typecheck
  • npx vitest run test/unit/migration-column-extraction.test.ts --coverage --coverage.include="src/db/migration-column-extraction.ts" — all 36 tests pass (33 pre-existing + 3 new), 100% statements/branches/functions/lines on the changed file
  • npx tsx scripts/check-migrations.ts — ran directly against the real 178-file migration corpus: 178 migrations OK — contiguous 0001..0174 ..., no new duplicates, confirming the widened regexes don't misparse any existing migration
  • npm run actionlint
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • This change touches only src/db/migration-column-extraction.ts and its unit test (no UI/MCP/worker/OpenAPI surface touched), so the UI/MCP/workers/OpenAPI-specific checks above were not run locally; they are unaffected by this diff and are still exercised by the full CI gate.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no auth/session/CORS code touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — internal CI-gating parser only, no external API/OpenAPI/MCP surface.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI touched.)
  • Visible UI changes include a UI Evidence section below. (N/A — no visible UI change.)
  • Public docs/changelogs are updated where needed. (N/A — no docs/changelog change needed.)

UI Evidence

N/A — this is a backend/tooling parser change with no visible UI surface.

Notes

  • Only the three regexes' COLUMN handling changed; no other parsing or collision-detection logic touched, per the issue's own scope note. No multi-column ALTER TABLE variant support was added (explicitly out of scope).

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.64%. Comparing base (3c815ea) to head (56e908a).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8456      +/-   ##
==========================================
- Coverage   92.43%   89.64%   -2.79%     
==========================================
  Files         791       98     -693     
  Lines       79317    22838   -56479     
  Branches    23954     3908   -20046     
==========================================
- Hits        73314    20473   -52841     
+ Misses       4866     2187    -2679     
+ Partials     1137      178     -959     
Flag Coverage Δ
shard-1 100.00% <100.00%> (+42.23%) ⬆️
shard-2 ?
shard-3 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/db/migration-column-extraction.ts 100.00% <100.00%> (ø)

... and 693 files with indirect coverage changes

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 24, 2026
@loopover-orb

loopover-orb Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-24 14:11:34 UTC

2 files · 1 AI reviewer · no blockers · readiness 81/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR widens three regexes in `extractSchemaEvents` (RENAME/DROP/ADD COLUMN) to make the `COLUMN` keyword optional, matching SQLite's actual grammar where `ALTER TABLE t ADD x INTEGER` is valid without the keyword. The fix targets the correct layer (the regex source of truth used by the CI-gating collision check), preserves backward compatibility with the explicit-`COLUMN` form via existing passing tests, and adds three new tests exercising the exact terser syntax for all three ALTER forms. The description's claim that the widened regexes were verified against the real 178-file migration corpus with no new false positives is credible given the change is a narrow, non-greedy optionality addition that doesn't touch statement boundaries or the DROP TABLE/CREATE TABLE paths.

Nits — 4 non-blocking
  • The three ternary-like `(?:COLUMN\s+)?` groups are now duplicated three times with no shared helper; consider extracting a small constant/comment noting SQLite's optional-COLUMN grammar rule once instead of restating it implicitly in each regex.
  • Several checklist items (test:workers, build:mcp, ui:lint, ui:typecheck, npm audit) are left unchecked in the PR description; worth confirming they're genuinely out of scope for a pure src/db + unit-test change rather than just skipped.
  • Consider a follow-up regression test with a column literally named `column` (e.g. `ALTER TABLE t ADD column TEXT`) to document how the optional-keyword regex resolves that inherent ambiguity, since SQLite itself treats `COLUMN` as a non-reserved keyword usable as an identifier.
  • The file-level comment block above `extractSchemaEvents` documents assumptions about the corpus (no CREATE TRIGGER, bare snake_case identifiers); a one-line addition noting the optional-COLUMN grammar quirk there would keep that documentation block complete for future maintainers.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8368
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 176 registered-repo PR(s), 81 merged, 5 issue(s).
Contributor context ✅ Confirmed Gittensor contributor joaovictor91123; Gittensor profile; 176 PR(s), 5 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
All three regexes were changed exactly as requested, making COLUMN optional via (?:COLUMN\s+)? while preserving correct capture groups, and new tests cover the COLUMN-absent form for ADD/DROP/RENAME alongside existing passing tests.

Review context
  • Author: joaovictor91123
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, JavaScript, MDX, TypeScript, C++, CSS, Rust
  • Official Gittensor activity: 176 PR(s), 5 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 9adcccb into JSONbored:main Jul 24, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migration column-extraction regexes require the literal COLUMN keyword, but SQLite doesn't

1 participant