test(db): seed per-test sessions.db from one migrated template - #23
Merged
Merged
Conversation
studyloop.history._connection._connect creates the sessions database from SCHEMA_FILE and runs every migration the first time any caller opens a path that does not exist. Fourteen test modules point STUDYLOOP_DB at a fresh tmp_path / "sessions.db" per test, so each test's first write — usually `plan new` indexing its document — pays that bootstrap: measured here at ~59 ms and a 1 MB, 75-table, WAL file (user_version 48), 241 times across those modules (test_plan_application 49, test_now_plan_guidance 38, test_plan_application_mutations 33, test_cli_plan_seam 29, ...). On a slow runner disk it is the first step to stall: CI run 35350636500 timed out two `plan close` seam tests at 60 s each inside `plan new` creating the database, before `plan close` ran; the module runs in 2.5 s locally. The fix this pins is not a shared live database — test_plan_application's fixture (council review 1, F6) needs "no history" to be a fact about the test, not about what a shared file holds. Each test keeps its own file at its own path; only the bootstrap is shared: one template built once per process through the production _connect, copied per test. Five tests pin the helper: the template has the schema and user_version a genuine first _connect produces; seeding gives distinct files, sets STUDYLOOP_DB, and a first connect on a seeded path never logs "Created sessions DB"; the template is built once (a rebuild stub must not be called); template and copy are a single checkpointed file with no -wal/-shm sidecar; and a lint over tests/ refuses the retired setenv(tmp_path / "sessions.db") pattern so it cannot come back. RED: the module fails at collection (no _sessions_db_template helper). The pyright: ignore[reportMissingImports] tags on the two import lines are the RED pattern used since item 3b; GREEN strips them.
…REEN Every test that pointed STUDYLOOP_DB at a fresh tmp_path / "sessions.db" paid the production bootstrap on its first write: history._connection ._connect created the file from SCHEMA_FILE, ran all 48 migrations and converted it to WAL — ~59 ms and a 1 MB, 75-table file, 241 times across the 20 modules that set the variable (measured by counting the "Created sessions DB" log line). On a slow runner disk that is the first step to stall: CI run 35350636500 timed out two `plan close` seam tests at 60 s each inside `plan new` creating the database, before `plan close` ran. tests/_sessions_db_template.py builds one template per process through the real _connect (under a MonkeyPatch context that also points STUDYLOOP_CONFIG at an absent file, so a config's own session_db key — which outranks the env var by design — cannot route the build elsewhere), closes the connection so SQLite checkpoints away the -wal/-shm sidecars, and seed_sessions_db(path, monkeypatch) copies that one file to the test's own path and sets STUDYLOOP_DB. The 14 modules that used the fresh path swap the setenv line for that call; the 6 that build their own database (drifted FTS, scope fixtures, records.connect) are unchanged. Not a shared live database: test_plan_application's fixture (council review 1, F6) needs "no history" to be a fact about the test, not about what a shared file holds. Each test still gets its own file; only the bootstrap is shared, and the guard test pins that a write to one test's copy is invisible from another's. Measured on the same 20-file set: 37.57 s -> 22.84 s, 241 bootstraps -> 1; test_cli_plan_seam.py alone 2.50 s -> 0.97 s. Guard 5/5 (template schema and user_version equal a genuine fresh bootstrap; seeded first connect never logs "Created sessions DB"; built once; single checkpointed file; lint over tests/ refuses the retired pattern). test_mcp_next_action.py, which imports isolate_now_world, 10/10. ruff, format, pyright clean. Full suite 30 failed / 5117 passed / 14 errors; the 44 failed+errored ids are byte-identical to the item-4 control's committed environmental set (run - control = empty, control - run = empty).
There was a problem hiding this comment.
🔵 Needs a closer look
Reinitialize the per-database context identity after template copies and add the requested regression assertion.
Pull request overview
This pull request reduces test startup time by copying a migrated SQLite template into isolated per-test database paths.
Changes:
- Adds shared template creation and seeding helpers.
- Updates 14 test modules to use seeded databases.
- Adds schema, WAL, isolation, and retired-pattern regression guards.
File summaries
| File | Summary |
|---|---|
packages/studyloop/tests/test_web_plans_seam.py |
Uses seeded test database. |
packages/studyloop/tests/test_sessions_db_template.py |
Validates template correctness and isolation. |
packages/studyloop/tests/test_session_start_purpose.py |
Uses seeded test database. |
packages/studyloop/tests/test_plan_recording_failures.py |
Uses seeded test database. |
packages/studyloop/tests/test_plan_journey_combined.py |
Uses seeded test database. |
packages/studyloop/tests/test_plan_intent_snapshots.py |
Uses seeded test database. |
packages/studyloop/tests/test_plan_guidance.py |
Uses seeded test database. |
packages/studyloop/tests/test_plan_application.py |
Uses seeded test database. |
packages/studyloop/tests/test_plan_application_mutations.py |
Uses seeded test database. |
packages/studyloop/tests/test_now_plan_guidance.py |
Uses seeded test database. |
packages/studyloop/tests/test_mcp_plan_tools.py |
Uses seeded test database. |
packages/studyloop/tests/test_mcp_plan_record_seam.py |
Uses seeded test database. |
packages/studyloop/tests/test_cli_plan_seam.py |
Uses seeded test database. |
packages/studyloop/tests/test_cli_doctor.py |
Uses seeded test database. |
packages/studyloop/tests/test_agent_prompt_contract.py |
Uses seeded test database. |
packages/studyloop/tests/_sessions_db_template.py |
Moderate issue: reinitialize per-database context_access_state.instance after copying and add a distinct-instance regression assertion. |
Review details
Suppressed comments (1)
packages/studyloop/tests/_sessions_db_template.py:77
- Copying the migrated file also copies the singleton
context_access_state.instancecreated by migration 38. A genuine bootstrap assigns a fresh UUID per database, but every seeded test DB now reports the template's same instance, so replication/retention code can treat otherwise independent test databases as one logical node; the compaction path explicitly discards this singleton for the same reason. Reinitialize this per-database identity after the copy and add a regression assertion that two seeded databases have different instances.
shutil.copyfile(template_path(), destination)
- Files reviewed: 16/16 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 18, 2026
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
Every test that pointed
STUDYLOOP_DBat a freshtmp_path / "sessions.db"paid the production bootstrap on its first write —history._connection._connectcreated the file fromSCHEMA_FILE, ran all 48 migrations and converted it to WAL: ~59 ms and a 1 MB, 75-table file, 241 times across the 20 modules that set the variable. On a slow runner disk that is the first step to stall: CI run 35350636500 timed out twoplan closeseam tests at 60 s each insideplan newcreating the database, beforeplan closeran.tests/_sessions_db_template.pybuilds one template per process through the real_connectandseed_sessions_db(path, monkeypatch)copies that single checkpointed file to the test's own path and setsSTUDYLOOP_DB. The 14 modules that used the fresh path swap one line; the 6 that build their own database are unchanged.Not a shared live database:
test_plan_application.py(council review 1, F6) needs "no history" to be a fact about the test. Each test still gets its own file; only the bootstrap is shared, and the guard pins that a write to one copy is invisible from another.Tested
tests/test_sessions_db_template.py5/5: template schema +user_versionequal a genuine fresh bootstrap; a seeded first connect never logs "Created sessions DB"; built once; single file, no-wal/-shm; a lint overtests/refuses the retired pattern.test_cli_plan_seam.pyalone 2.50 s → 0.97 s.Merge
Local fast-forward once green (RED
0d9c3185→ GREENa5b9f903; the programme's receipts cite SHAs), as with #20 and #22.