Skip to content

Pass unset rollback options through to migrator (remove forced batch=1)#1406

Merged
hwbrzzl merged 2 commits intomasterfrom
copilot/fix-batch-setting-migrate-rollback
Mar 11, 2026
Merged

Pass unset rollback options through to migrator (remove forced batch=1)#1406
hwbrzzl merged 2 commits intomasterfrom
copilot/fix-batch-setting-migrate-rollback

Conversation

Copy link
Contributor

Copilot AI commented Mar 10, 2026 •
edited by coderabbitai bot
Loading

migrate:rollback was overriding unset options by forcing batch=1, which prevented the migrator from applying its default rollback selection logic. When both step and batch are zero, the migrator should resolve the last batch internally.

  • Behavior fix in rollback command

    • Removed the command-level defaulting that converted (step=0, batch=0) into (0,1).
    • The command now forwards user input as-is to Migrator.Rollback(step, batch).

  • Test alignment with intended default semantics

    • Updated rollback command tests to expect Rollback(0, 0) for:
      • default invocation path
      • error path with default options
// before
if step == 0 && batch == 0 {
    batch = 1
}
_ = r.migrator.Rollback(step, batch)

// after
_ = r.migrator.Rollback(step, batch)
Original prompt

This section details on the original issue you should resolve

<issue_title>Incorrect batch setting in MigrateRollbackCommand</issue_title>
<issue_description>The batch setting in migrate_rollback_command.go#L70 is incorrect.

The batch should not be set to 1. Otherwise, the batch will always be one when rolling back the migration. The batch should be zero and then passed to the migration module. The migration module should get the last batch when both the step and the batch are zero.</issue_description>

Comments on the Issue (you are @copilot in this section)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Summary by CodeRabbit

  • Bug Fixes
    • Updated migration rollback command parameter handling to correctly process default scenarios when neither step nor batch options are specified.

@hwbrzzl
Fix migrate rollback default batch handling
72b13af 
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix incorrect batch setting in MigrateRollbackCommand Pass unset rollback options through to migrator (remove forced batch=1) Mar 10, 2026
@codecov
Copy link

codecov bot commented Mar 11, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.48%. Comparing base (628ed45) to head (72b13af).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1406   +/-   ##
=======================================
  Coverage   72.48%   72.48%           
=======================================
  Files         292      292           
  Lines       19577    19577           
=======================================
  Hits        14191    14191           
  Misses       4807     4807           
  Partials      579      579

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hwbrzzl hwbrzzl marked this pull request as ready for review March 11, 2026 06:59
@hwbrzzl hwbrzzl requested a review from a team as a code owner March 11, 2026 06:59
@hwbrzzl
Copy link
Contributor

hwbrzzl commented Mar 11, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 11, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d381d941-0259-4e17-824c-f9ffd7e495b8

📥 Commits

Reviewing files that changed from the base of the PR and between 628ed45 and 72b13af.

📒 Files selected for processing (2)
  • database/console/migration/migrate_rollback_command.go
  • database/console/migration/migrate_rollback_command_test.go
💤 Files with no reviewable changes (1)
  • database/console/migration/migrate_rollback_command.go
📝 Walkthrough

Walkthrough

Removed logic that defaulted batch to 1 in the rollback command when neither step nor batch options were provided. The command now passes (0, 0) to the migration module, allowing it to determine the last batch for rollback operations.

Changes

Cohort / File(s) Summary
Rollback batch defaulting fix
database/console/migration/migrate_rollback_command.go, database/console/migration/migrate_rollback_command_test.go		 Removed incorrect batch=1 default; now passes (0, 0) to migration module to query the last batch. Test expectations updated from Rollback(0, 1) to Rollback(0, 0) across default and error scenarios.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A batch that was wrongly set to one,
Now floats as zeros, migration's undone!
The module will find what it truly needs,
The last batch blooms like our garden seeds. 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Pass unset rollback options through to migrator (remove forced batch=1)' directly and clearly summarizes the main change: removing forced batch=1 defaulting and passing values through as-is.
Linked Issues check ✅ Passed The PR successfully addresses issue #912 by removing the batch=1 defaulting logic and allowing (0, 0) to pass through to the migrator, enabling internal batch resolution.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the migration rollback command and its tests, with no unrelated modifications present.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch copilot/fix-batch-setting-migrate-rollback

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

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

This was referenced Mar 11, 2026
Closed
Merged
@hwbrzzl hwbrzzl merged commit 5457bef into master Mar 11, 2026
16 of 17 checks passed
@hwbrzzl hwbrzzl deleted the copilot/fix-batch-setting-migrate-rollback branch March 11, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@hwbrzzl hwbrzzl

Copilot code review Copilot

Labels

🚀 Review Ready

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect batch setting in MigrateRollbackCommand

2 participants

@hwbrzzl