fix(core): merge the alembic heads left by #1440 and #1444 - #1447
Merged
Conversation
Both PRs added a migration off v5o6b7s8d9e0 and merged separately, so main had two heads (w6k7i8n9d0a1 and x7d8e9f0a1b2). Alembic refuses `upgrade head` with two heads, so every fresh database on main failed to initialize. Each PR was green because the suite builds schemas with create_all and stamps them; nothing ran the real upgrade against the merged graph. The first thing that did was the #1398 confirmation eval, which failed at `bm mcp` startup. - y8f9a0b1c2d3 is a no-op merge revision with both heads as parents. A merge, not a re-parent, so databases already stamped at either head still receive the other branch's changes. - tests/test_migration_graph.py asserts one head and that every revision is on the path from base to it. Verified failing on a92b1ac (names both heads) and passing here. - `just doctor` on this branch initializes a fresh database through `upgrade head` and passes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Summary
Main has two Alembic heads:
w6k7i8n9d0a1(#1444, search_index row-kind uniqueness) andx7d8e9f0a1b2(#1440, viaw6r7e8a9d0y1). Both revisev5o6b7s8d9e0. Alembic refusesupgrade headwith two heads, so every fresh database on main fails to initialize withMultiple head revisions are present.Each PR was green on its own because the test suite builds schemas with
create_alland stamps them; nothing runs the real upgrade against the merged graph (the #1445 gap). The #1398 confirmation eval was the first thing to do so, and died atbm mcpstartup.Fix
y8f9a0b1c2d3is a no-op merge revision with both heads as parents. A merge rather than a re-parent, so a database already stamped at either head still receives the other branch's changes.tests/test_migration_graph.pyasserts the graph has one head and every revision is on the path from base to it. Fails ona92b1acbnaming both heads; passes here.Verification
just doctoron this branch: fresh database initialized throughupgrade head, all checks pass.just fast-checkclean.Part of #1438. Related: #1445.
🤖 Generated with Claude Code
https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp