Repository navigation
fix(db): make alembic check clean on SQLite - #883
Merged
Merged
Conversation
… on SQLite; ci-doctor runs alembic check (#869) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…(sa.column())
sa.desc("created_at") wraps a text label, so Alembic still treated the index
as an expression and SQLite still reported it as changed. SM026's expression
test now reads an ordered column as a column, so the index is no longer
listed as unverifiable.
Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
… ordering
make_include_object now skips, on SQLite only, an index pair whose only
difference is an ordered column written as sa.text("col DESC") or
sa.desc("col"): SQLite reflects it as a plain column, so autogenerate wrote a
drop and re-create and alembic check failed (GH #869). A different column list,
a uniqueness change or a real expression is still reported. Guard by Codex;
moved into its own module so migrations.py keeps its docstrings.
Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Never suppress a partial-index comparison (Codex): the SQLite ordering guard compares columns only, so a changed WHERE predicate would have been hidden. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Codex Review Summary: ✅ CompletedThis 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. |
Deploying simple-module-python with
|
| Latest commit: |
0395adb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4735c515.simple-module-python.pages.dev |
| Branch Preview URL: | https://fix-issue-869-mixed-expressi.simple-module-python.pages.dev |
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
Closes #869.
On a SQLite database migrated to heads,
alembic checkfailed, and every SQLiterevision --autogeneratewrote the same two false operations into the new revision. CI never saw it, because its onlyalembic checkran on Postgres.file_storagebrowse index:sa.text("created_at DESC"), which SQLite reflects as a plain column. It is nowsa.desc(sa.column("created_at")).sa.desc("created_at")with a string is not enough: SQLAlchemy wraps it as a text label, and Alembic still treats that as an expression.users_access_token.expires_at: it was declaredTIMESTAMPAwarewhile its migration createdDateTime(timezone=True), and SQLite reported a type change. It now usesAwareDateTime(), the repo convention. It still reads back aware UTC, and every writer already passes UTC.simple_module_db/_sqlite_index_compare.py):make_include_objectskips an index pair whose only difference is an ordered column written as text or assa.desc("col").lower(email), and any partial index.env.py.desc()/asc()as a column, so the browse index is no longer listed as unverifiable. A labelled or cast expression still counts as an expression.make ci-doctornow runsalembic checkagainst its throwaway SQLite database.Design:
docs/superpowers/specs/2026-11-07-sqlite-alembic-check-design.md. Plan:docs/superpowers/plans/2026-11-07-sqlite-alembic-check.md.Verification
make lintgreen.make test-py: 8,235 passed, 28 skipped, with Redis.make test-js: 900 passed.make ci-doctor:alembic checkon SQLite reported no new upgrade operations; doctor gave 0 errors.make migrations-roundtrip-pgon a scratch database: no new upgrade operations.migrations.pykeeps its docstrings, and extended it to cover the label form.Test plan
alembic upgrade headsthenalembic checkreports no new operations.doctor(now runsalembic check) andmigrations-roundtrip-pg.https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4