migration fix for calendar aligned

[akshaydeo]

Summary

Replaces GORM's Migrator.HasColumn / AddColumn calls in the migrate_calendar_aligned migration with raw SQL, and adds doc comments to all legacy budget migration helpers. The GORM migrator approach was unreliable across dialects; the new hasColumn helper queries pragma_table_info (SQLite) or information_schema.columns (all other databases) directly, and column addition is done via a plain ALTER TABLE ... ADD COLUMN statement.

Changes

• Added a hasColumn helper that checks for a column's existence using dialect-aware raw SQL instead of the GORM migrator API.
• Replaced mig.HasColumn / mig.AddColumn calls in migrateCalendarAlignedToBudgetsAndRateLimitsTable with hasColumn and raw ALTER TABLE statements for both governance_budgets.calendar_aligned and governance_rate_limits.calendar_aligned.
• Added doc comments to all legacy budget migration types and helper functions (legacyBudgetVirtualKey, legacyBudgetVirtualKeyProviderConfig, legacyBudgetTeam, sqliteColumnInfo, legacyBudgetColumnModel, currentBudgetOwnerModel, quoteSQLiteIdentifier, sqliteTableColumns, sqliteTableHasColumn).

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

go test ./framework/configstore/...

Run against both a SQLite and a supported relational database to confirm the calendar_aligned column is added correctly on a fresh migration and that re-running the migration is idempotent (column already exists case is skipped without error).

Screenshots/Recordings

N/A

Breaking changes

• Yes
• No

Related issues

N/A

Security considerations

No auth, secrets, or PII implications. Raw SQL identifiers in the new helper use parameterised queries, so there is no SQL injection risk.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Refactor
  • Added robust migration helpers to detect and add missing columns across different databases, improving cross-engine reliability.
  • Updated migration steps to apply missing-column changes using direct schema statements for safer upgrades; existing data-prefill behavior is unchanged.

Walkthrough

Refactors SQLite migration helpers in framework/configstore/migrations.go: reorders legacy budget models and docs, adds a generic hasColumn(tx, table, column) helper, and updates migrateCalendarAlignedToBudgetsAndRateLimitsTable to use hasColumn and raw ALTER TABLE statements to add missing calendar_aligned columns.

Changes

SQLite Migration Helpers Refactoring

Layer / File(s)	Summary
Legacy budget models organization and helper documentation framework/configstore/migrations.go	Reorders legacyBudgetVirtualKey, legacyBudgetVirtualKeyProviderConfig, legacyBudgetTeam, and sqliteColumnInfo with adjacent TableName() methods; adds a comment for currentBudgetOwnerModel and documentation comments for quoteSQLiteIdentifier and sqliteTableHasColumn.
New generic hasColumn helper framework/configstore/migrations.go	Adds hasColumn(tx, table, column) which checks column existence via SQLite PRAGMA table_info when using sqlite dialect and via information_schema.columns for other dialects.
Calendar-aligned migration update framework/configstore/migrations.go	Modifies migrateCalendarAlignedToBudgetsAndRateLimitsTable to call hasColumn and, when missing, add calendar_aligned to governance_budgets and governance_rate_limits using raw ALTER TABLE ... ADD COLUMN ... SQL instead of GORM migrator HasColumn/AddColumn.

Possibly Related PRs

• maximhq/bifrost#3434: Changes budget CalendarAligned handling related to virtual key derivation.

Suggested Reviewers

• danpiths
• roroghost17

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble schemas with careful paws,
Reorder structs and mend the flaws,
I ask, "hasColumn?" — then ALTER with glee,
A rabbit's patch for SQLite's tree. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'migration fix for calendar aligned' is vague and non-descriptive, using generic phrasing that doesn't convey the specific technical change.	Consider a more specific title like 'Replace GORM migrator with dialect-aware SQL for calendar_aligned column checks' to better communicate the actual change.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description is comprehensive and well-structured, covering all major template sections with detailed explanations of changes, testing approach, and security considerations.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 05-17-migration_fix_for_calendar_aligned

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[akshaydeo]

• migration fix for calendar aligned #3553 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 5/5

Safe to merge — the new hasColumn helper is correct for both SQLite and non-SQLite dialects, errors are properly propagated, and the raw ALTER TABLE statements are straightforward.

The change is narrowly scoped to one migration function. All previously identified concerns (silent error suppression, missing schema filter in the information_schema query, duplicate doc comments) have been addressed in this revision. The hasColumn implementation uses parameterised queries throughout, current_schema() is supported by PostgreSQL and MySQL, and pragma_table_info(?) with bound parameters is valid on modern SQLite. No new correctness issues were found.

No files require special attention.

Important Files Changed

Filename	Overview
framework/configstore/migrations.go	Replaces GORM migrator API with a dialect-aware hasColumn helper (proper error propagation, current_schema() schema filter for non-SQLite), updates migrateCalendarAlignedToBudgetsAndRateLimitsTable to use raw ALTER TABLE, and adds doc comments to all legacy budget helpers.

_{Reviews (3): Last reviewed commit: "Apply suggestions from code review" | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@framework/configstore/migrations.go`:
- Around line 252-267: The hasColumn function currently swallows errors from the
schema introspection query (tx.Raw(...).Scan(&count).Error) and returns false,
which is ambiguous; change hasColumn(tx *gorm.DB, table, column string) to
return (bool, error), propagate and return the actual error from
tx.Raw(...).Scan(&count).Error instead of returning false, and update any
callers of hasColumn to handle the error (abort migration or surface the error)
rather than treating a false as “column missing”; keep the existing Dialector
switch and query construction (including the sqlite and default queries) but
ensure errors are returned up the call chain.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb927236-3ab9-4328-8aac-abda49b8057e

📥 Commits

Reviewing files that changed from the base of the PR and between 44b56a9 and 8871cc5.

📒 Files selected for processing (1)

• framework/configstore/migrations.go

[coderabbitai]

🧹 Nitpick comments (1)

framework/configstore/migrations.go (1)

252-263: ⚡ Quick win

Reuse sqliteTableHasColumn in the SQLite branch.

This adds a second SQLite schema-introspection path next to sqliteTableHasColumn, which makes future fixes easy to miss in one place.

♻️ Suggested simplification

 func hasColumn(tx *gorm.DB, table, column string) (bool, error) {
+	switch tx.Dialector.Name() {
+	case "sqlite":
+		return sqliteTableHasColumn(tx, table, column)
+	}
+
 	var count int64
-	var q string
-	switch tx.Dialector.Name() {
-	case "sqlite":
-		q = `SELECT COUNT(*) FROM pragma_table_info(?) WHERE name = ?`
-	default:
-		q = `SELECT COUNT(*) FROM information_schema.columns WHERE table_name = ? AND column_name = ? AND table_schema = current_schema()`
-	}
+	q := `SELECT COUNT(*) FROM information_schema.columns WHERE table_name = ? AND column_name = ? AND table_schema = current_schema()`
 	if err := tx.Raw(q, table, column).Scan(&count).Error; err != nil {
 		return false, err
 	}
 	return count > 0, nil
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/configstore/migrations.go` around lines 252 - 263, The SQLite
branch in hasColumn duplicates schema-introspection logic instead of reusing the
existing sqliteTableHasColumn helper; replace the sqlite-specific query path in
hasColumn with a call to sqliteTableHasColumn(tx, table, column) and return its
(bool, error) result so errors are preserved and there is a single canonical
implementation to maintain; ensure the call matches sqliteTableHasColumn's
signature and that the default (non-sqlite) branch keeps the existing
information_schema query and error propagation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@framework/configstore/migrations.go`:
- Around line 252-263: The SQLite branch in hasColumn duplicates
schema-introspection logic instead of reusing the existing sqliteTableHasColumn
helper; replace the sqlite-specific query path in hasColumn with a call to
sqliteTableHasColumn(tx, table, column) and return its (bool, error) result so
errors are preserved and there is a single canonical implementation to maintain;
ensure the call matches sqliteTableHasColumn's signature and that the default
(non-sqlite) branch keeps the existing information_schema query and error
propagation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe47c60d-3d13-4d5d-a5fe-4ad5c03f7eff

📥 Commits

Reviewing files that changed from the base of the PR and between 99e4d37 and 806a322.

📒 Files selected for processing (1)

• framework/configstore/migrations.go

[akshaydeo]

Merge activity

• May 17, 6:17 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 17, 6:17 PM UTC: @akshaydeo merged this pull request with Graphite.