docs(clickhouse): require max+1 numbering and idempotent DDL#3633
Conversation
…ations Codify two rules that came out of the 029/030 ordering incident (TRI-9367 test cloud deploy): 1. ClickHouse migration files must be numbered max(existing)+1. Goose runs in strict mode in the deploy pipeline and refuses to apply a missing version below the current version, which blocks the deploy. Branches that fell behind main need to renumber before merging. 2. DDL must be idempotent (ADD COLUMN IF NOT EXISTS, DROP COLUMN IF EXISTS, CREATE TABLE IF NOT EXISTS, etc.) so a retry or out-of-order apply (e.g. goose up --allow-missing for local recovery) is a no-op rather than an error. Rules go in internal-packages/clickhouse/CLAUDE.md; reviewer-facing summary added to .claude/REVIEW.md as a new 🔴 finding.
|
WalkthroughThis PR documents operational governance rules for ClickHouse migrations managed by Goose. It adds detailed guidance to the ClickHouse package documentation specifying that migration files must be numbered sequentially (starting at current max + 1) and prohibiting downward version changes, since Goose in strict mode enforces this ordering. The documentation also requires migration DDL to be idempotent using patterns like Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@internal-packages/clickhouse/CLAUDE.md`:
- Around line 13-16: The fenced code block that currently begins with ``` is
missing a language tag (MD040); update the opening fence to include a language
identifier (for example ```text or ```console) so the block becomes ```text and
retains the same contents (the goose run error lines), ensuring markdownlint no
longer flags MD040.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 94e61241-4e86-4b59-98f6-3637cd445710
📒 Files selected for processing (2)
.claude/REVIEW.mdinternal-packages/clickhouse/CLAUDE.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: typecheck / typecheck
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
internal-packages/clickhouse/CLAUDE.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-15T17:11:58.596Z
Learning: Before opening a PR with migrations, rebase and renumber migration files if main branch has added migrations during your work to avoid numbering conflicts below the new max.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-15T17:11:58.596Z
Learning: Use `raw_` prefix for input tables where data lands first.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-15T17:11:58.596Z
Learning: Use `_v1`, `_v2` suffixes for table versioning.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-15T17:11:58.596Z
Learning: Use `_mv_v1` suffix for materialized views.
Learnt from: CR
Repo: triggerdotdev/trigger.dev
Timestamp: 2026-05-15T17:11:58.596Z
Learning: Use `_per_day` and `_per_month` suffixes for aggregation tables.
🪛 markdownlint-cli2 (0.22.1)
internal-packages/clickhouse/CLAUDE.md
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
nicktrn
left a comment
nicktrn
left a comment
There was a problem hiding this comment.
Goose runs in strict mode in the cloud deploy pipeline
We should also enforce this in dev to catch early
2 participants
Summary
Codify two rules for ClickHouse migration authors that came out of the 029/030 ordering incident on the TRI-9367 test cloud deploy:
max(existing) + 1, never slot in below the latest. Goose runs in strict mode in the cloud deploy pipeline and refuses to apply a missing version below the current version — slotting a file in below an already-applied number blocks the next deploy.ADD COLUMN IF NOT EXISTS,DROP COLUMN IF EXISTS,CREATE TABLE IF NOT EXISTS, etc.) so a retry or out-of-order apply (goose up --allow-missingfor local recovery, manual fixups) is a no-op rather than an error.Where the rules live
internal-packages/clickhouse/CLAUDE.md— full rules + example for migration authors (and AI agents writing migrations)..claude/REVIEW.md— added a 🔴 finding under "What makes a 🔴 Important finding" so PR reviewers flag either fault as blocking.The existing migration files are left untouched; the idempotency requirement applies going forward.
Test plan
IF NOT EXISTS/IF EXISTSforms