Migrate old instructions column - #981
Open
zetter-rpf wants to merge 2 commits into
Open
Conversation
Previously, projects created before instruction_steps existed only had their instructions stored in the legacy text column, so Project#instructions had to fall back to it at read time. This change adds a data migration that copies the legacy instructions value into instruction_steps for any row where instruction_steps is still null, so every project can be read from instruction_steps alone. This is a step toward removing the instructions column entirely.
Previously, Project#instructions fell back to the legacy text column for rows written before instruction_steps existed, and the setter kept writing that column too. Now that instruction_steps has been backfilled for every project, that fallback is unnecessary and would break once the column is dropped. This change marks instructions as an ignored column and simplifies the instructions accessor and mutator to delegate directly to instruction_steps, removing the last runtime dependency on the legacy column so it can be safely dropped in a follow-up migration.
Test coverage93.43% line coverage reported by SimpleCov. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates legacy projects.instructions (text) into the newer projects.instruction_steps (jsonb) and updates the Project model to stop reading/writing the legacy column, preparing for dropping it in a later deploy.
Changes:
- Add a data migration to backfill
instruction_stepsfrom the legacyinstructionscolumn. - Update
Projectto ignore the legacyinstructionscolumn and makeinstructions/instructions=delegate toinstruction_steps. - Simplify the
Project#instructionsmodel spec to only validate delegation toinstruction_steps.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/models/project.rb | Ignores the legacy instructions column and delegates instructions accessors to instruction_steps. |
| db/migrate/20260825144345_backfill_instruction_steps_from_instructions.rb | Backfills instruction_steps from instructions for existing rows. |
| db/schema.rb | Updates schema version after adding the migration. |
| spec/models/project_spec.rb | Updates model tests to reflect the new delegation-only behavior. |
Suppressed comments (1)
db/migrate/20260825144345_backfill_instruction_steps_from_instructions.rb:13
downis currently a no-op, so rolling back this migration would report success while leaving data changed. If the backfill can’t be safely reversed, it should explicitly raiseActiveRecord::IrreversibleMigrationto avoid a misleading rollback.
def down
end
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+5
to
+9
| execute <<~SQL.squish | ||
| UPDATE projects | ||
| SET instruction_steps = to_jsonb(instructions) | ||
| WHERE instruction_steps IS NULL AND instructions IS NOT NULL | ||
| SQL |
Contributor
Author
There was a problem hiding this comment.
I'm going to be bold and try to run this as I expect the migration to run within a few seconds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status
What's changed?
This is tidy up after introducing instruction Steps to migrate all data into a single column.
By adding
instructionsto ignored columns tests will fail if anything is accessing it and we can later drop the old column in another deploy.Note that instruction steps is a JSONB column, that might contain an array of instruction objects, or a single string. Currently editor-ui handles these different formats. We could later do another migration to wrap the strings into an array which would allow us to simplify the frontend.