Fable review of 4.0rc1

[simonw]

Final review before shipping a stable 4.0 release - very important to spot any last minute things that would be a breaking change if we fix them later

Initial review document: https://github.com/simonw/sqlite-utils/blob/0c369a447eeaf39084f0d14a45b3eeb7eacb631b/fable-review-4.0rc1.md

Then we fixed a lot of stuff.

Claude-Session: https://claude.ai/code/session_01UnLnhsH25Nnv7LHhekUfPd

---

📚 Documentation preview 📚: https://sqlite-utils--767.org.readthedocs.build/en/767/

[codecov]

Codecov Report

❌ Patch coverage is 90.13158% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.16%. Comparing base (79117b9) to head (7a015cf).

Files with missing lines	Patch %	Lines
sqlite_utils/db.py	84.37%	15 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #767      +/-   ##
==========================================
- Coverage   95.34%   95.16%   -0.19%     
==========================================
  Files           9        9              
  Lines        3330     3431     +101     
==========================================
+ Hits         3175     3265      +90     
- Misses        155      166      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[simonw]

I'm not 100% confident in the design where changes are rolled back if you didn't commit them at the end of a with db: block.

[simonw]

That makes me think maybe we should have a db.begin() and db.commit() and db.rollback() method, since it's a bit ugly in the documentation to suddenly start showing raw db.execute("begin") strings and then use methods on db.conn.

[simonw]

Let's introduce our own exception for here, it's weird to use RuntimeError.

[simonw]

What does "not supported" mean, will it actively break things or just behave in weird ways?

[simonw]

The empirical answer is worse than "behaves in weird ways" — it breaks silently, in the most dangerous way possible, on the most basic operation. Under Python 3.13, with a connection passed to Database() in either mode:

• autocommit=True: db["t"].insert({"id": 1}) appears to succeed — the row is readable in-process — but the connection is left inside an open transaction, a second connection can't even see the table, and when the connection closes, everything vanishes. No error at any point. The mechanism: atomic() runs a real BEGIN, but Python documents conn.commit() as a no-op on autocommit=True connections, so the commit at the end of every write method silently does nothing.
• autocommit=False: identical symptoms via a different route — the connection is always inside an implicit transaction, so atomic() permanently takes its savepoint branch and never commits anything. Insert looks fine in-process; after close, the table doesn't exist.

So "not supported" currently means: every write the library makes is silently discarded at close. Nothing raises, nothing warns. That's the same silent-data-loss family as the delete_where bug — except here it's the entire API, and the docs sentence we wrote ("not supported") badly understates it.

[simonw]

Given that answer I said:

In that case, do an experiment now (don't commit any code) which sees if there is a minimal change we can make that results in our entire test suite passing for connections that use the new autocommit setting

And Fable did that and said:

The working tree is clean, so I can experiment freely and git checkout at the end. Protocol: (1) force all internally-created connections to autocommit=True via an env var and bypass the new guard, (2) baseline the damage under Python 3.13, (3) apply the smallest fix I hypothesize — atomic() and commit()/rollback() issuing literal SQL statements, which behave identically in every mode — and (4) count and categorize what still fails. Then the same for autocommit=False.

Baseline: 276 failures under autocommit=True with no mitigation. Now the hypothesized minimal fix — literal transaction statements, which mean the same thing in every connection mode:

Experiment complete. Capturing the diff summary and reverting everything:

Experiment done, everything reverted (tree clean, suite back to 1118 green on the committed code). Here's what it found:

Answer: yes — for autocommit=True, near-total compatibility is a ~20-line change

Baseline: forcing every internally-created connection to autocommit=True breaks the library catastrophically, as expected: 276 of 1134 tests fail (every atomic() commit is a silent no-op, so transactions pile up open and everything downstream rots).

The minimal fix — replace mode-dependent connection method calls with literal SQL statements, which mean the same thing in every mode:

• In atomic()'s top-level branch: conn.rollback() → execute("ROLLBACK"), conn.commit() → execute("COMMIT") (the BEGIN was already a statement; the savepoint branch was already pure SQL).
• In db.commit()/db.rollback(): literal COMMIT/ROLLBACK, guarded by in_transaction (a bare COMMIT with no transaction errors, unlike the no-op method).
• One call-site cleanup: the CLI bulk command still used with db.conn: — switching it to db.atomic() (which it should arguably use anyway for consistency).

Result: 276 → 3 failures, and two of those are the constructor-guard tests themselves, which the experiment deliberately bypasses — so really one genuine incompatibility: test_database_context_manager, which pins that raw db.execute() writes roll back at close. Under autocommit=True there are no implicit transactions — raw writes commit instantly — so that documented semantic simply doesn't exist in that mode. That's not a bug to fix; it's the honest behavioral delta that would need documenting. Also notable: my new savepoint test only passed after switching it from db.conn.rollback() to db.rollback() — a nice confirmation that the methods you asked for this morning are exactly the mode-agnostic idiom.

Crucially, the fix is backwards-compatible: with the literal-statement changes in place and no autocommit forcing, the full suite passes untouched (1118/1118) in legacy mode. So this change could ship in 4.x at any time without breaking anyone, and it would even fix the tracer blind spot (transaction statements become visible if routed through execute()).

autocommit=False is a different story: 266 failures even with the fix. As predicted, its always-inside-an-implicit-transaction model breaks atomic()'s in_transaction sniffing at the root — supporting it needs a real redesign (explicit nesting-depth tracking instead of state sniffing), not a patch.

Implication for the 5.0 speculation: the door is even more open than I guessed. When demand arrives, autocommit=True support is a minor-release-sized change (literal statements + relax the guard for True only + document the no-implicit-rollback delta + sweep the remaining four with db.conn: sites in cli.py). Only autocommit=False — or changing your own default — carries major-release weight. Nothing worth doing before stable 4.0, but the experiment's conclusion is worth remembering when the first "support my autocommit connection" issue gets filed.

[simonw]

I'm going to implement this and do an RC which works in both modes.

[simonw]

f7ff3e2

[simonw]

I don't like wrapped text like this in docs.

[simonw]

Look at the 10 lines of docs/python-api.rst following 151 - these have been text-wrapped, but the style for this project's documentation is NOT to text-wrap RST. Fix that by unwrapping the block, and find any other instances in the RST files that have been wrapped like that but should not have been.

[simonw]

That worked: 7a015cf

[simonw]

I said to Claude Code:

Use an Opus subagent to find the transcript of. this conversation in your ~/.claude/projects/ folder and then count the number of prompts from me in that

It came back with:

The Opus subagent found the transcript at /root/.claude/projects/-home-user-sqlite-utils/e526d56a-f6a0-5b9e-ae51-beac4a240161.jsonl and counted 38 prompts from you (including the one asking for this count).

How it got there: of 425 user-role entries in the JSONL, 380 are tool results stored under the user role, 3 are task notifications, 1 is an image-paste annotation, 2 are [Request interrupted by user] markers, and 1 is a truncated fragment — leaving 38 genuine human messages. It verified the classification by quoting the first and last five, which check out ("Final review before shipping a stable 4.0 release…" through this very request).

The one judgment call: your interrupted "That one failing test: is there a design change we can make to `sqlite" was re-typed in full 22 seconds later — the agent counted that as one logical prompt since the fragment is a pure prefix of the retyped version. If you'd count the interrupted fragment separately, the answer is 39. (Your other interruption, during the CI-step discussion, was two genuinely distinct messages and both are in the 38.)