fix: migrate to SQLModel session.exec() and gate regressions (#890)#914
fix: migrate to SQLModel session.exec() and gate regressions (#890)#914
Conversation
Migration plan for all 77 session.execute() call sites across src/ (zndraw, zndraw_auth, zndraw_joblib) to the SOTA session.exec() API, plus a pytest filterwarnings gate to prevent regressions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add the 25 test call sites (4 files) to the spec — with filterwarnings set to error, unmigrated test files would fail the suite too. No @pytest.mark.protected tests in the affected files, so no carve-outs needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Empirical verification shows test fixtures use pure sqlalchemy AsyncSession, so direct session.execute() calls in tests don't emit the SQLModel deprecation warning and aren't blocked by the filterwarnings gate. The 15k warnings observed come entirely from integration tests hitting src/ code via the production uvicorn server (which configures class_=SQLModelAsyncSession). Migrating src/ alone eliminates all of them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per-file atomic task breakdown for the 77 src/ call sites, plus the pyproject.toml filterwarnings gate. Each task is self-contained with exact line numbers, before/after code, test commands, and commit command. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test session factories now create SQLModel sessions (matching production) so the migrated src/ code's session.exec() calls work in tests. Also migrates direct session.execute() calls in test_sweeper and test_registry to session.exec() since those sessions are now SQLModel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
for more information, see https://pre-commit.ci
📝 WalkthroughWalkthroughA repository-wide migration replaces SQLModel async session Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-04-08-sqlmodel-session-exec-migration.md (1)
182-204: Minor: Add language specifier to fenced code block.The expected output block at line 184 is missing a language specifier, which triggers a markdownlint warning.
📝 Suggested fix
Expected output (17 files, summing to 77): -``` +```text src/zndraw/database.py:1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-04-08-sqlmodel-session-exec-migration.md` around lines 182 - 204, The fenced code block showing the "Expected output (17 files, summing to 77):" is missing a language specifier which causes a markdownlint warning; update the opening fence for that block to use a language (e.g., change ``` to ```text) so the block becomes ```text and leave the contents unchanged—locate the fenced block labeled "Expected output (17 files, summing to 77):" and modify its opening fence accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/specs/2026-04-08-sqlmodel-session-exec-migration-design.md`:
- Line 152: The regex in the pattern string 'error:[\s\S]*You probably want to
use .session\.exec:DeprecationWarning' uses an unescaped dot before "session"
which matches any character; update that literal pattern to escape the period so
it reads 'error:[\s\S]*You probably want to use
\.session\.exec:DeprecationWarning' (modify the string containing the regex in
the docs/spec where the pattern is defined).
---
Nitpick comments:
In `@docs/superpowers/plans/2026-04-08-sqlmodel-session-exec-migration.md`:
- Around line 182-204: The fenced code block showing the "Expected output (17
files, summing to 77):" is missing a language specifier which causes a
markdownlint warning; update the opening fence for that block to use a language
(e.g., change ``` to ```text) so the block becomes ```text and leave the
contents unchanged—locate the fenced block labeled "Expected output (17 files,
summing to 77):" and modify its opening fence accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14492ab8-a13a-471b-82e9-79ca2dbb8318
📒 Files selected for processing (25)
docs/superpowers/plans/2026-04-08-sqlmodel-session-exec-migration.mddocs/superpowers/specs/2026-04-08-sqlmodel-session-exec-migration-design.mdpyproject.tomlsrc/zndraw/database.pysrc/zndraw/routes/admin.pysrc/zndraw/routes/bookmarks.pysrc/zndraw/routes/chat.pysrc/zndraw/routes/figures.pysrc/zndraw/routes/frames.pysrc/zndraw/routes/geometries.pysrc/zndraw/routes/presets.pysrc/zndraw/routes/rooms.pysrc/zndraw/routes/screenshots.pysrc/zndraw/routes/selection_groups.pysrc/zndraw_auth/cli_login.pysrc/zndraw_auth/db.pysrc/zndraw_joblib/dependencies.pysrc/zndraw_joblib/registry.pysrc/zndraw_joblib/router.pysrc/zndraw_joblib/sweeper.pytests/zndraw/conftest.pytests/zndraw_auth/conftest.pytests/zndraw_joblib/conftest.pytests/zndraw_joblib/test_registry.pytests/zndraw_joblib/test_sweeper.py
|
|
||
| ```toml | ||
| filterwarnings = [ | ||
| 'error:[\s\S]*You probably want to use .session\.exec:DeprecationWarning', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the exact deprecation message from SQLModel source
# Find the deprecation message in the SQLModel package
python -c "
import sqlmodel.ext.asyncio.session as sess
import inspect
import ast
# Get the source code of the AsyncSession class
source = inspect.getsource(sess.AsyncSession)
print('Searching for deprecated decorator and message...')
print(source[:2000]) # Print first part to see the decorator
" | grep -A 5 -B 5 "deprecated\|session.exec"Repository: zincware/ZnDraw
Length of output: 180
🏁 Script executed:
cat -n docs/superpowers/specs/2026-04-08-sqlmodel-session-exec-migration-design.md | sed -n '145,165p'Repository: zincware/ZnDraw
Length of output: 1356
🌐 Web query:
SQLModel AsyncSession session.exec deprecation warning message
💡 Result:
session.exec(...) is not deprecated in SQLModel. The SQLModel deprecation warning is for session.execute(...), and it tells you to switch to session.exec(...). In the SQLModel source, the deprecation text starts with: “You probably want to use session.exec() instead of session.execute() …” and explains that execute() returns Row objects and you’d typically need .scalars() to get model objects. [1][2]
What to do instead
-
Sync:
heroes = session.exec(select(Hero)).all()
(instead of
session.execute(...).scalars().all()) [1] -
Async:
result = await session.exec(select(Hero)) heroes = result.all()
(instead of
await session.execute(...)+.scalars()) [2]
If you’re seeing a warning that specifically mentions exec being deprecated, it’s likely coming from a different library or from importing the wrong AsyncSession (e.g., SQLAlchemy’s instead of SQLModel’s). The SQLModel release notes also mention adjustments to the session.execute() docstring/deprecation messaging. [3]
Sources:
[1] https://github.com/fastapi/sqlmodel/blob/530c2684/sqlmodel/orm/session.py
[2] https://github.com/fastapi/sqlmodel/blob/530c2684/sqlmodel/ext/asyncio/session.py
[3] https://sqlmodel.tiangolo.com/release-notes/
Escape the period before "session" in the regex pattern.
Line 152 has an unescaped period before session:
'error:[\s\S]*You probably want to use .session\.exec:DeprecationWarning',
^
unescapedIn regex, an unescaped . matches any character, not a literal period. The SQLModel deprecation message contains "You probably want to use session.exec(...)", so the pattern should be:
'error:[\s\S]*You probably want to use \.session\.exec:DeprecationWarning',This ensures the filter matches only the intended warning and prevents false positives from strings like "Xsession" or "asession".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/specs/2026-04-08-sqlmodel-session-exec-migration-design.md`
at line 152, The regex in the pattern string 'error:[\s\S]*You probably want to
use .session\.exec:DeprecationWarning' uses an unescaped dot before "session"
which matches any character; update that literal pattern to escape the period so
it reads 'error:[\s\S]*You probably want to use
\.session\.exec:DeprecationWarning' (modify the string containing the regex in
the docs/spec where the pattern is defined).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #914 +/- ##
==========================================
- Coverage 91.78% 91.77% -0.01%
==========================================
Files 246 246
Lines 22881 22905 +24
==========================================
+ Hits 21001 21022 +21
- Misses 1880 1883 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
session.execute()call sites across 17 files insrc/to SQLModel'ssession.exec()API — the SOTA recommendation per sqlmodel.tiangolo.comDeprecationWarninginstances per test run that were drowning out real warnings in CIpyproject.tomlfilterwarningsgate that escalates any futuresession.execute()reintroduction (on a SQLModel session) to a hard CI failureAsyncSessionimports fromsqlalchemy.ext.asynciotosqlmodel.ext.asyncio.sessionin files that use.exec(), so type annotations match the runtime session typesession.execute()calls accordinglyCloses #890.
Approach
session.exec()returns the same data via.one()/.one_or_none()/.all()instead of.scalar_one()/.scalar_one_or_none()/.scalars().all()update(Task).where(...).values(...)call inrouter.py(optimistic-locking task-claim path) uses SQLModel'sUpdateBaseoverload onexec(), which returns a fully-typedCursorResultwith.rowcounttest_resilience.pyis intentionally untouched — it creates its own sqlalchemy sessions for raw SQL manipulation, which don't go through SQLModel's wrapperTest plan
uv run pytest tests/zndraw_joblib/ tests/zndraw_auth/— 358 passeduv run pytest tests/zndraw/ -m "not integration"— 1072 passed, 1 skippedgrep -rn "session\.execute(" src/ --include="*.py"— returns emptygrep -rn "\.scalars(" src/ --include="*.py"— returns emptyfilterwarningsgate is in effect (anysession.execute()on a SQLModel session now fails the suite)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Documentation
Chores