Expand plan_versions.diff_unified to mediumtext#102
Conversation
The PUT /api/v1/plans/:id/content endpoint exposes a pre-existing schema limit: diff_unified was 'text' (~64KB on MySQL) while content_markdown is 'mediumtext' (~16MB). A whole-file PUT of any plan whose unified diff exceeds 64KB (easy with multi-thousand-line files) triggers ActiveRecord::ValueTooLong → 500. Reproduced locally with a 5,000-line / 189KB plan and 10 scattered edits. After the migration the same PUT returns 201 in ~390ms. Affects all four PlanVersion write paths (operations_controller, commit_session, plans_controller, replace_content) — schema-level fix benefits them all. Amp-Thread-ID: https://ampcode.com/threads/T-019dda86-f31b-7148-82b5-c5f1a59c4568 Co-authored-by: Amp <amp@ampcode.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7afb476ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def change | ||
| change_column :coplan_plan_versions, :diff_unified, :text, limit: 16.megabytes - 1 |
There was a problem hiding this comment.
change_column is irreversible when used inside change, so rolling back this migration will raise ActiveRecord::IrreversibleMigration instead of restoring the old text definition. That makes deploy/database rollback paths fail when this migration has been applied (the same pattern is duplicated in the engine copy), which is a real operational risk for production rollbacks; use explicit up/down (or reversible) to define how to revert the column size change.
Useful? React with 👍 / 👎.
1 participant
What
Bumps
coplan_plan_versions.diff_unifiedfromtext(≈64KB MySQL limit) tomediumtext(≈16MB), matchingcontent_markdown.Why
Found while smoke-testing the new
PUT /api/v1/plans/:id/contentendpoint (#101) on a 5,000-line / 189KB plan with 10 scattered edits. The whole-file rewrite produced a unified diff > 64KB and the version insert blew up with:This is a pre-existing schema limit affecting all four PlanVersion write paths (
operations_controller,commit_session,plans_controller,replace_content) — the new endpoint just makes it trivially reachable since agents are encouraged to PUT entire files.Verification
After applying the migration locally, the same 5,000-line PUT returns
201in ~390ms withapplied: 10and exact roundtrip oncurrent_content.Full test suite still green:
765 examples, 0 failures.