fix(migrations): use numeric ordering to allow 3-digit migrations

[Lutherwaves]

Closes #197.

Problem

Migration files are named <id>__<description>.yaml. The runner derives each migration id by parsing the numeric prefix and gates application on a numeric high-water mark (apply when id > mark). But it chose run order with sort.Strings(keys) — a lexical sort.

Lexical and numeric order agree only while every id has the same digit width. The first 3-digit migration (100__…) sorts before 10__…..99__… lexically (the char after 100 is _ > any digit). So on a fresh database (mark = 0) the runner:

1. applies 01..09 (mark → 9),
2. applies 100, 101 next in lexical order (mark → 101),
3. reaches 10..99, whose ids are now below the mark, and silently skips all of them — no error, recorded as success.

Incrementally migrated databases (mark already ≥ 99) are unaffected, so this only bites fresh environments: new dev setups, CI, ephemeral test databases.

Fix

Order keys by their parsed numeric id — the same value the apply gate uses — with a lexical tie-break for equal ids and a lexical fallback for unparseable prefixes (so ordering stays deterministic; a bad id is still surfaced by the existing per-migration check). migrationID is extracted as the single parse helper shared by both the ordering and the gate, so they can no longer disagree.

Tests

storage/migration_internal_test.go:

• TestSortMigrationKeysOrdersNumericallyNotLexically — mixed 1/2/3-digit ids incl. a tie on 100; asserts 1,2,9,10,99,100,100 order (would fail under sort.Strings).
• TestSortMigrationKeysFallsBackToLexicalForUnparseablePrefix — non-numeric prefixes sort deterministically without panicking.

go build ./..., go vet ./storage/, and go test ./storage/ all pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed migration execution ordering to prioritize numeric IDs extracted from migration filenames, with lexical ordering as deterministic fallback for ties or non-numeric prefixes.

• Tests

  • Added comprehensive test coverage for numeric-based migration ordering logic and fallback behavior.

[coderabbitai]

Warning

Rate limit exceeded

@Lutherwaves has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 36 minutes and 53 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 512737c8-9d4d-4e80-9c59-05bb6e0846fc

📥 Commits

Reviewing files that changed from the base of the PR and between 946e881 and f6eff91.

📒 Files selected for processing (2)

• storage/migration.go
• storage/migration_internal_test.go

📝 Walkthrough

Walkthrough

Migration execution now orders by parsed numeric ID rather than lexical filename sort. Helper functions extract numeric prefixes and sort deterministically by id with lexical tie-break. Integration into runMigrations applies this sorting before the apply-gate check. Comprehensive tests validate numeric ordering and graceful fallback to lexical ordering.

Changes

Numeric-aware migration ordering

Layer / File(s)	Summary
Numeric ID extraction and sorting implementation storage/migration.go	migrationID parses the numeric prefix from filenames; sortMigrationKeys orders keys by that parsed id with stable lexical fallback. Imports updated to use slices instead of sort.
Apply sorting in runMigrations loop storage/migration.go	Collected migration keys are sorted via sortMigrationKeys, and each iteration computes migrationId using the same parsing logic before the apply-gate check, eliminating duplication.
Sorting behavior validation storage/migration_internal_test.go	Tests verify numeric-id ordering across mixed digit widths (including identical id ties) and deterministic lexical fallback when prefixes are unparseable, ensuring no panics.

---

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	Title exceeds 50-character requirement at 65 characters; however, it is clear, descriptive, and directly summarizes the main change (numeric ordering for migrations).	Reduce title to under 50 characters, e.g., 'fix(migrations): use numeric ordering for migrations' (48 chars).
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Changes fully implement the primary objective from #197: numeric-order sorting via extracted migrationID helper, deterministic lexical tie-break, and shared parse logic between ordering and gating prevent silent migration skipping.
Out of Scope Changes check	✅ Passed	All changes are scoped to migration ordering; no unrelated modifications to other systems or extraneous refactoring detected.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/numeric-migration-sort

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[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 `@storage/migration_internal_test.go`:
- Around line 116-123: The helper function slicesIndex is redundant—replace all
calls to slicesIndex(s, v) with slices.Index(s, v) from the standard library and
add the "slices" import; then remove the slicesIndex function entirely. Locate
usages in tests (any references to slicesIndex) and update them to slices.Index,
update the import block to include "slices", and delete the slicesIndex helper
function to avoid duplicate boilerplate.

🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: d48fbc4b-56b1-451d-9a47-870f7e281670

📥 Commits

Reviewing files that changed from the base of the PR and between 1323ff6 and 946e881.

📒 Files selected for processing (2)

• storage/migration.go
• storage/migration_internal_test.go