Skip to content

feat(core): name new Alembic revisions by sequential schema version - #1658

Merged
phernandez merged 5 commits into
mainfrom
sequential-alembic-revision-ids
Oct 6, 2026
Merged

phernandez merged 5 commits into
mainfrom
sequential-alembic-revision-ids

Conversation

@phernandez

@phernandez phernandez commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Why

basicmachines-co/basic-memory-cloud#2355 moves tenant migrations out of band, and each tenant records an ordered schema version. Random hex revision IDs only say "different". A sequential number says which schema is newer.

What

  • Every revision after z9a0b1c2d3e4 is named by its schema version, zero-padded to four digits: "0040", "0041", and so on, so migration files sort in version order. Each one revises the previous one.
  • migrations.schema_version(revision) returns the number of revisions applied up to and including that revision. z9a0b1c2d3e4 is 39. For numbered revisions it equals int(revision) ("0040" is 40). It raises ValueError for unknown IDs, including prefixes such as "z9a" that Alembic would otherwise resolve.
  • migrations.next_revision_id(script) returns head + 1 and raises if the graph has more than one head.
  • env.py sets the revision ID through Alembic's process_revision_directives hook (assign_sequential_revision_ids), in both the online and offline context.configure calls. alembic.ini sets revision_environment = true, because without it a plain alembic revision (no --autogenerate) never runs env.py (alembic/command.py) and writes a random ID. Both just migration and hand-written revisions therefore get the next number.
  • LAST_UNNUMBERED_REVISION = "z9a0b1c2d3e4" marks where numbering starts. The older IDs are not all hex; many were hand-picked.
  • Two open PRs off the same head both generate the same ID. A probe with two 0040 files shows Alembic warns Revision 0040 is present more than once and reports heads ['0040', '0040'], so the second PR fails the one-head test. The rule, now in AGENTS.md and the just migration comment, is to renumber your revision to the next ID and point its down_revision at the new head. Never add a merge revision once numbered revisions start; the tail test requires a linear chain. The one-head test and next_revision_id errors now give that advice instead of "add a merge revision".

Verification

  • Probe run against the current Alembic: alembic revision --autogenerate on a temp database generated 0040_probe_padded_id.py with Revision ID: 0040, Revises: z9a0b1c2d3e4. A plain alembic revision -m ... also generated 0040. Without revision_environment it generated e1245b8670f9. Probe files were deleted.
  • tests/test_migration_graph.py covers the hook: it assigns the next version on a real MigrationContext and fails fast without a script directory. It also covers the graph: one head, a contiguous, linear tail of exactly-four-digit IDs after z9a0b1c2d3e4, helper values, unknown IDs, a forked graph, and that alembic.ini keeps revision_environment on.
  • just lint and just typecheck pass. tests/test_migration_graph.py + tests/test_alembic_env.py: 19 passed; alembic/migrations.py at 100% coverage.

Note: future migration modules start with a digit, so tests must load them with importlib.import_module rather than from basic_memory.alembic.versions import ....

🤖 Generated with Claude Code

https://claude.ai/code/session_01D36TJNafZfpQh99hu9JkTF

Revisions after z9a0b1c2d3e4 are named "40", "41", ... instead of random hex IDs,
so a schema version orders at a glance (cloud tenants report which one they run).

- migrations.schema_version(revision) counts the revisions a revision applies
  (z9a0b1c2d3e4 is 39) and raises ValueError for unknown or prefix-only IDs.
- migrations.next_revision_id(script) returns head + 1 and refuses a graph with
  more than one head.
- env.py passes assign_sequential_revision_ids as process_revision_directives, so
  `just migration` (alembic revision --autogenerate) assigns the ID.
- Tests: one head, numeric contiguous linear tail after z9a0b1c2d3e4, helper
  values, unknown-ID and forked-graph errors, and the hook itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D36TJNafZfpQh99hu9JkTF
Signed-off-by: phernandez <paul@basicmachines.co>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T16:36:11.245853Z 8ad0230 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

phernandez and others added 3 commits October 6, 2026 11:03
New revisions are named "0040", "0041", ... so migration files sort in version
order in a directory listing. schema_version still counts the graph, so "0040"
is 40. The graph test now requires numbered revisions to be exactly four digits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D36TJNafZfpQh99hu9JkTF
Signed-off-by: phernandez <paul@basicmachines.co>
`alembic revision` without --autogenerate only runs env.py when
revision_environment is true (alembic/command.py), so hand-written revisions
skipped assign_sequential_revision_ids and got a random ID. Set
revision_environment = true in alembic.ini; a probe run of
`alembic revision -m ...` now generates 0040 revising z9a0b1c2d3e4.

Rename LAST_HEX_REVISION to LAST_UNNUMBERED_REVISION: 21 of the 39 existing
IDs are not hex (z9a0b1c2d3e4 included) and many were hand-picked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D36TJNafZfpQh99hu9JkTF
Signed-off-by: phernandez <paul@basicmachines.co>
…vision

Two open PRs off the same head both generate the next ID ("0040"). A probe
with two 0040 files shows Alembic warns "Revision 0040 is present more than
once" and reports heads ['0040', '0040'], so the second PR fails the one-head
test.

The fix is to renumber your revision to the next ID and point its
down_revision at the new head. A merge revision is not allowed once numbered
revisions start (the tail test requires a linear chain). Document this in
AGENTS.md and the justfile migration recipe, and replace the "join them with
a merge revision" advice in the one-head test and next_revision_id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D36TJNafZfpQh99hu9JkTF
Signed-off-by: phernandez <paul@basicmachines.co>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29e2df7161

ℹ️ 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".

Comment thread src/basic_memory/alembic/migrations.py Outdated
raise RuntimeError("alembic revision context has no script directory")

for directive in directives:
directive.rev_id = next_revision_id(context.script)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject revisions that do not extend the sole head

When a developer uses the supported alembic revision --head <older-revision> --splice path, this hook assigns the next global schema ID without validating the generated directive's parent. alembic revision --help documents --head as selecting the base revision and --splice as allowing a non-head base; reproducing this with --head y8f9a0b1c2d3 --splice creates revision 0040 with down_revision = 'y8f9a0b1c2d3', leaving both 0040 and z9a0b1c2d3e4 as heads and violating the required linear numbered tail. Validate that each directive extends the sole current head and fail before writing the file.

AGENTS.md reference: AGENTS.md:L291-L296

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 8ad0230.

Probe before the fix: alembic revision -m x --head y8f9a0b1c2d3 --splice wrote 0040 revising y8f9a0b1c2d3, and alembic heads listed 0040 and z9a0b1c2d3e4.

assign_sequential_revision_ids now resolves each directive's head and raises ValueError before Alembic writes the file unless it is the single current head. The error tells you to build on the head. The CLI passes head=None when --head is omitted, and ScriptDirectory.generate_revision reads that as "head", so the hook resolves it the same way.

Probes after the fix: the splice is refused with no file written; plain alembic revision and --autogenerate still produce 0040 on z9a0b1c2d3e4. New tests cover the splice (y8f9a0b1c2d3) and base cases, and the accepted None / "head" / explicit-head cases. just lint, just typecheck and the migration tests (23) pass.

…n the head

`alembic revision --head y8f9a0b1c2d3 --splice` got the next ID anyway: a probe
wrote 0040 revising y8f9a0b1c2d3 and left two heads. assign_sequential_revision_ids
now resolves each directive's head and raises before the file is written unless
it is the single current head. The CLI passes head=None when --head is omitted,
which Alembic reads as "head", so the hook resolves it the same way.

Probes: plain and --autogenerate revisions still produce 0040 on
z9a0b1c2d3e4; the splice is refused and writes no file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D36TJNafZfpQh99hu9JkTF
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez
phernandez merged commit 3417094 into main Oct 6, 2026
43 checks passed
@phernandez
phernandez deleted the sequential-alembic-revision-ids branch October 6, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant