Skip to content

test(db): seed per-test sessions.db from one migrated template - #23

Merged
NetDevAutomate merged 2 commits into
mainfrom
fix/seam-test-db-bootstrap
Sep 18, 2026
Merged

NetDevAutomate merged 2 commits into
mainfrom
fix/seam-test-db-bootstrap

Conversation

@NetDevAutomate

Copy link
Copy Markdown
Owner

Summary

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. 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 and seed_sessions_db(path, monkeypatch) copies that single checkpointed file to the test's own path and sets STUDYLOOP_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

  • Guard tests/test_sessions_db_template.py 5/5: template schema + user_version equal a genuine fresh bootstrap; a seeded first connect never logs "Created sessions DB"; built once; single file, no -wal/-shm; a lint over tests/ refuses the retired pattern.
  • 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.
  • 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 = ∅, control − run = ∅). ruff / format / pyright clean; all pre-commit hooks on both commits.

Merge

Local fast-forward once green (RED 0d9c3185 → GREEN a5b9f903; the programme's receipts cite SHAs), as with #20 and #22.

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).
Copilot AI lite review requested due to automatic review settings September 18, 2026 16:57

Copilot AI 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.

🔵 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.instance created 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.

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.

2 participants