Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ tests/
```bash
uv run pytest # All tests
uv run pytest tests/ --ignore=tests/e2e -m "not lifecycle" # The CI gate (every non-e2e, non-real-LLM test)
# ~6.5 min locally. If it is much slower on your
# machine, see "Suite speed" below.
uv run pytest tests/core/ # Core module tests
scripts/lifecycle --mode cli|all # Real-LLM lifecycle tests (run locally before a PR)
# api/web exit 3 — not implemented (#948, #1068)
Expand All @@ -168,6 +170,34 @@ cd web-ui && npm test
cd web-ui && npm run build
```

#### Suite speed (#979)

The suite used to be fsync-bound: `_init_database` builds ~18 tables plus
indexes and switches to WAL, and that cost **~1273 ms per test** on a real
filesystem versus 2.7 ms on tmpfs. It ran once per test, which was most of the
wall clock — measured at ~910 s, and reportedly ~4 h on a slower WSL2 disk.

`tests/conftest.py` now builds that schema **once per session** and copies the
file (0.1 ms) into place per test. `_init_database` output is byte-identical
across builds, so the copy is exactly equivalent; the template is produced by
calling the real function, so a schema change or `SCHEMA_VERSION` bump cannot
leave it stale. Full suite: **~910 s → ~378 s**, same results, and CI benefits
too. `tests/core/test_workspace_db_template_979.py` pins the equivalence.

If your machine is still I/O-bound, put pytest's temp dirs on tmpfs:

```bash
mkdir -p /dev/shm/pytest-cf
uv run pytest tests/ --ignore=tests/e2e -m "not lifecycle" --basetemp=/dev/shm/pytest-cf
```

Opt-in on purpose — a real filesystem is what CI runs, and is the more faithful
environment. `/dev/shm` is RAM-backed (the run must fit) and Linux-only. It
composes with the ambient-workspace guard: `pytest_configure` registers
`--basetemp` as an isolated root. With the template fix this now buys nothing
here (378 s on disk vs 428 s on tmpfs before the fix); it is the lever only
for disks where the gap is still large.

### Golden Path CLI
```bash
# Workspace
Expand Down
83 changes: 83 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,89 @@ def openai_api_key(mock_env) -> str:
return key


# ---------------------------------------------------------------------------
# Workspace schema template (#979)
# ---------------------------------------------------------------------------
#
# Building the workspace schema costs ~1273ms per call on a real filesystem and
# ~2.7ms on tmpfs. That 470× difference is fsync: ~18 CREATE TABLEs plus
# indexes plus a WAL-mode switch, committed and flushed, once per test. It was
# most of the wall clock of a full local run (measured: ~910s on disk vs 428s
# with a tmpfs basetemp, identical results).
#
# `_init_database` is deterministic — two builds are byte-identical, with no
# -wal/-shm sidecars — so building it ONCE per session and copying the file
# (0.1ms) is exactly equivalent, not merely similar.
#
# Drift is structurally impossible: the template is produced by calling the
# real `_init_database`, so a schema change or SCHEMA_VERSION bump is picked up
# automatically. `tests/core/test_workspace_db_template_979.py` asserts the
# copy stays byte-identical to a real build.
#
# Test-only. No production code is touched, and durability semantics outside
# the test session are unchanged.

_REAL_INIT_DATABASE = _workspace_module._init_database


@pytest.fixture(scope="session")
def real_init_database():
"""The unpatched schema builder, for tests that must compare against it."""
return _REAL_INIT_DATABASE


@pytest.fixture(scope="session", autouse=True)
def _workspace_db_template(tmp_path_factory):
"""Build the workspace schema at most once per session; copy it per test.

Built lazily on first use, not at session start: a run that never creates a
workspace (``pytest -k one_unrelated_test``) should not pay the ~1.3s build
it will never benefit from.
"""
import shutil
import threading

template_lock = threading.Lock()
template_holder: list[Path] = []

def _template() -> Path:
with template_lock:
if not template_holder:
path = tmp_path_factory.mktemp("cf-db-template") / "state.db"
_REAL_INIT_DATABASE(path)
template_holder.append(path)
return template_holder[0]

def _copy_template(db_path):
# An EXISTING database is a migration, not a build: `_init_database`
# is `CREATE TABLE IF NOT EXISTS` plus ALTER TABLE steps, so copying
# over it would discard the caller's data and skip every migration —
# making migration tests pass for the wrong reason, because the
# template already contains the column the migration was meant to add.
# Only a new file may be templated. The hot path is unaffected:
# create_or_load_workspace builds at a fresh temp path.
if Path(db_path).exists():
return _REAL_INIT_DATABASE(db_path)

# Let sqlite create the file so the mode is whatever sqlite would have
# given it (0644 base, not open()'s 0666 — see
# test_state_db_permissions_match_a_plain_sqlite_create), then
# overwrite the contents. Opening 'wb' on an existing file leaves its
# mode alone, so permissions stay faithful by construction rather than
# by recomputing a umask formula.
import sqlite3

sqlite3.connect(db_path).close()
with open(_template(), "rb") as src, open(db_path, "wb") as dst:
shutil.copyfileobj(src, dst)

_workspace_module._init_database = _copy_template
try:
yield _template
finally:
_workspace_module._init_database = _REAL_INIT_DATABASE


# Markers for test organization
def pytest_configure(config):
"""Configure pytest with custom markers."""
Expand Down
205 changes: 205 additions & 0 deletions tests/core/test_workspace_db_template_979.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
"""The per-test workspace schema build is the suite's cost centre (issue #979).

Rebuilding ~18 tables plus indexes per test costs ~1273ms on a real filesystem
(2.7ms on tmpfs) — that difference is fsync, and it is most of the wall clock
of a full local run. Copying a template built once per session costs 0.1ms.

The optimisation is only safe because ``_init_database`` is deterministic, so
these tests pin the property the speedup rests on rather than the speedup
itself. If a schema change ever made the output non-reproducible, the copy
would stop being equivalent and this file is what says so.
"""

from __future__ import annotations

import hashlib
import sqlite3
from pathlib import Path

import pytest

pytestmark = pytest.mark.v2


def _digest(path: Path) -> str:
return hashlib.md5(path.read_bytes()).hexdigest()


class TestTheSchemaBuildIsReproducible:
"""The premise: two builds are byte-identical, so a copy is equivalent."""

def test_two_builds_are_byte_identical(self, tmp_path, real_init_database):
a, b = tmp_path / "a.sqlite", tmp_path / "b.sqlite"
real_init_database(a)
real_init_database(b)
assert _digest(a) == _digest(b)

def test_no_wal_sidecars_are_left_behind(self, tmp_path, real_init_database):
"""A single-file copy is only complete if -wal/-shm are checkpointed."""
real_init_database(tmp_path / "db.sqlite")
assert sorted(p.name for p in tmp_path.iterdir()) == ["db.sqlite"]


class TestTheTemplateMatchesARealBuild:
"""The guarantee: what tests get is what the real code path produces."""

def test_the_copy_is_byte_identical_to_a_real_build(
self, tmp_path, real_init_database
):
from codeframe.core import workspace as ws

real = tmp_path / "real.sqlite"
real_init_database(real)

copied = tmp_path / "copied.sqlite"
ws._init_database(copied) # the session-patched version

assert _digest(copied) == _digest(real), (
"the template has diverged from _init_database — the speedup is "
"no longer equivalent to the real schema build"
)

def test_the_copy_carries_the_current_schema_version(self, tmp_path):
"""A SCHEMA_VERSION bump must not leave the template stamped behind."""
from codeframe.core import workspace as ws

db = tmp_path / "db.sqlite"
ws._init_database(db)
conn = sqlite3.connect(db)
try:
assert conn.execute("PRAGMA user_version").fetchone()[0] == ws.SCHEMA_VERSION
finally:
conn.close()

def test_the_copy_is_in_wal_mode(self, tmp_path):
from codeframe.core import workspace as ws

db = tmp_path / "db.sqlite"
ws._init_database(db)
conn = sqlite3.connect(db)
try:
assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "wal"
finally:
conn.close()

@pytest.mark.parametrize("umask_value", [0o022, 0o002, 0o007])
def test_the_copy_has_the_mode_sqlite_would_have_given_it(
self, tmp_path, umask_value
):
"""A file copy carries the SOURCE's mode — that is the trap here.

The template is built once under the session's umask, so copying its
permission bits would hand every later test the wrong mode. Compared
against a reference database sqlite creates itself, not a formula:
sqlite uses an 0644 base, not open()'s 0666.
"""
import os

from codeframe.core import workspace as ws

old = os.umask(umask_value)
try:
reference = tmp_path / "reference.db"
sqlite3.connect(reference).close()
expected = reference.stat().st_mode & 0o777

db = tmp_path / "db.sqlite"
ws._init_database(db)
actual = db.stat().st_mode & 0o777
finally:
os.umask(old)

assert actual == expected, f"mode {actual:o}, sqlite would give {expected:o}"

def test_a_workspace_built_on_the_copy_works_end_to_end(self, tmp_path):
"""The point of the schema is that tasks round-trip through it."""
from codeframe.core import tasks
from codeframe.core.tasks import TaskStatus
from codeframe.core.workspace import create_or_load_workspace

repo = tmp_path / "repo"
repo.mkdir()
workspace = create_or_load_workspace(repo)
task = tasks.create(workspace, title="T", description="d")
tasks.update_status(workspace, task.id, TaskStatus.READY)

assert tasks.get(workspace, task.id).status == TaskStatus.READY


class TestAnExistingDatabaseIsAMigration:
"""`_init_database` on an existing file migrates it — it does not rebuild.

The template must only ever serve a *new* database. Copying over an
existing one discards its contents and skips every ALTER TABLE, which
makes migration tests pass for the wrong reason: the template already has
the column the migration was supposed to add.
"""

def test_an_existing_database_keeps_its_data(self, tmp_path):
from codeframe.core import workspace as ws

db = tmp_path / "state.db"
conn = sqlite3.connect(db)
conn.execute("CREATE TABLE legacy (id TEXT PRIMARY KEY)")
conn.execute("INSERT INTO legacy VALUES ('keep-me')")
conn.commit()
conn.close()

ws._init_database(db)

conn = sqlite3.connect(db)
try:
assert conn.execute("SELECT id FROM legacy").fetchall() == [("keep-me",)]
finally:
conn.close()

def test_the_alter_table_migration_actually_runs(self, tmp_path):
"""The concrete case: a pre-created_by blockers table gets the column."""
from codeframe.core import workspace as ws

db = tmp_path / "state.db"
conn = sqlite3.connect(db)
conn.execute("""
CREATE TABLE blockers (
id TEXT PRIMARY KEY,
workspace_id TEXT NOT NULL,
task_id TEXT,
question TEXT NOT NULL,
answer TEXT,
status TEXT NOT NULL DEFAULT 'OPEN',
created_at TEXT NOT NULL,
answered_at TEXT
)
""")
conn.execute(
"INSERT INTO blockers (id, workspace_id, question, status, created_at) "
"VALUES ('b1', 'w1', 'q?', 'OPEN', '2026-01-01')"
)
conn.commit()
conn.close()

ws._init_database(db)

conn = sqlite3.connect(db)
try:
columns = {r[1] for r in conn.execute("PRAGMA table_info(blockers)")}
assert "created_by" in columns
# Migrated, not replaced — a rebuild would have dropped the row.
assert conn.execute("SELECT id FROM blockers").fetchall() == [("b1",)]
finally:
conn.close()


class TestPerTestPatchingStillWins:
"""A test that swaps _init_database itself must not be broken by this."""

def test_monkeypatch_restores_the_session_template_version(self, monkeypatch, tmp_path):
from codeframe.core import workspace as ws

patched = ws._init_database
monkeypatch.setattr(ws, "_init_database", lambda p: None)
assert ws._init_database is not patched
monkeypatch.undo()
assert ws._init_database is patched, (
"undo must restore the template version, not the original"
)