refactor: consolidate test infrastructure and document conventions

docs/TESTING.md

@@ -0,0 +1,237 @@
+# Testing Guide
+
+How to run, write, and maintain tests in the socratic-study-mentor workspace.
+
+## Running Tests
+
+The workspace has two packages with independent test suites:
+
+| Package | Tests | Path |
+|---------|-------|------|
+| studyctl | 293 | `packages/studyctl/tests/` |
+| agent-session-tools | 357 | `packages/agent-session-tools/tests/` |
+
+### Prerequisites
+
+Before running tests, install all workspace packages:
+
+```bash
+uv sync --all-packages
+```
+
+Plain `uv sync` only installs root deps — you will get `ModuleNotFoundError` on `click`, `typer`, etc. if you skip this.
+
+### Invocation Paths
+
+**From workspace root (recommended):**
+
+```bash
+uv run pytest                          # both packages, 650 tests
+uv run pytest packages/studyctl/tests/ # studyctl only
+uv run pytest packages/agent-session-tools/tests/  # agent-session-tools only
+```
+
+The root `pyproject.toml` sets `--import-mode=importlib` which is **load-bearing** — it prevents namespace conflicts between the two packages. Do not remove or override this.
+
+**From inside a package directory:**
+
+```bash
+cd packages/studyctl
+uv run pytest                          # uses studyctl's own pyproject.toml config
+```
+
+Note: when running from a member directory, the root config's `--import-mode=importlib` is NOT applied (the member's own `addopts` take precedence). This is fine for focused development but the workspace root is the authoritative invocation path.
+
+### Useful Options
+
+```bash
+uv run pytest -x                       # stop on first failure
+uv run pytest -k "test_review"         # run tests matching a pattern
+uv run pytest -m "not integration"     # skip integration tests
+uv run pytest --tb=long                # verbose tracebacks
+uv run pytest -v                       # show individual test names
+```
+
+## Why No conftest.py in studyctl
+
+This is the single most important architectural constraint in the test suite.
+
+**The problem:** When pytest collects both packages from the workspace root, it loads conftest files from both `packages/agent-session-tools/tests/conftest.py` and (if it existed) `packages/studyctl/tests/conftest.py`. These conftest files register as pytest plugins via pluggy. With `--import-mode=importlib`, the two conftest modules would occupy the same plugin namespace, causing a registration conflict.
+
+**The solution:** agent-session-tools has a conftest (it was here first and holds shared fixtures like `temp_db`, `migrated_db`, `populated_db`). studyctl does NOT have a conftest. All studyctl fixtures are either:
+
+1. **Inlined** in each test file (the current pattern for most tests)
+2. **Imported from `_helpers.py`** — a plain Python module with factory functions (not pytest fixtures)
+
+**Never create `packages/studyctl/tests/conftest.py`.** If you need shared utilities for studyctl tests, add them to `packages/studyctl/tests/_helpers.py`.
+
+## Fixture Patterns
+
+### agent-session-tools: Use conftest
+
+Shared fixtures live in `packages/agent-session-tools/tests/conftest.py`:
+
+- `temp_db` — temporary SQLite DB with base schema, yields `(conn, db_path)`
+- `migrated_db` — same as `temp_db` but with all migrations applied
+- `populated_db` — `migrated_db` with a sample session and message inserted
+- `temp_config_dir` — temporary config directory structure
+- `sample_session_data` — dict with canonical session fields
+- `sample_message_data` — dict with canonical message fields
+
+Use these directly in your test function signatures:
+
+```python
+def test_export_creates_session(migrated_db, tmp_path):
+    conn, db_path = migrated_db
+    # conn has all migrations applied, ready for exporter testing
+```
+
+### studyctl: Use _helpers.py or Inline
+
+Import factory functions from `_helpers.py` and wrap them in `@pytest.fixture`:
+
+```python
+import pytest
+from _helpers import make_review_db, make_isolated_config
+
+
+@pytest.fixture()
+def review_db(tmp_path):
+    return make_review_db(tmp_path)
+
+
+@pytest.fixture(autouse=True)
+def isolated_config(tmp_path, monkeypatch):
+    return make_isolated_config(tmp_path, monkeypatch)
+
+
+def test_something(review_db):
+    # review_db is a Path to a temp SQLite file with review tables
+    ...
+```
+
+**Available helpers:**
+
+| Function | Returns | What it does |
+|----------|---------|--------------|
+| `make_review_db(tmp_path)` | `Path` | Creates a temp SQLite DB with review schema (WAL mode) |
+| `make_isolated_config(tmp_path, monkeypatch)` | `Path` | Redirects `CONFIG_DIR` and `_CONFIG_PATH` to temp dir |
+
+For simple, one-off fixtures, just inline them in the test file. Only use `_helpers.py` when the pattern recurs across multiple files.
+
+## Mock Conventions
+
+Two tools, each for a specific purpose:
+
+### `monkeypatch` — for attributes and environment
+
+Use `monkeypatch.setattr` to replace module-level constants and object attributes. Use `monkeypatch.setenv` for environment variables. These auto-revert when the test ends.
+
+```python
+def test_custom_config_dir(monkeypatch, tmp_path):
+    monkeypatch.setattr("studyctl.settings.CONFIG_DIR", tmp_path)
+    monkeypatch.setenv("STUDYCTL_CONFIG", str(tmp_path / "config.yaml"))
+    # settings module now reads from tmp_path
+```
+
+### `unittest.mock.patch` — for callables
+
+Use `patch` when you need to replace a function AND verify it was called. The `MagicMock`/`AsyncMock` objects provide `.assert_called_once_with()`, `.call_args`, etc.
+
+```python
+from unittest.mock import patch
+
+def test_pypi_check_offline(tmp_path):
+    with patch("studyctl.doctor.updates._fetch_pypi_version", return_value="2.1.0"):
+        result = check_update_available()
+        assert result.status == "pass"
+```
+
+For async code, use `AsyncMock`:
+
+```python
+from unittest.mock import AsyncMock, patch
+
+@patch("studyctl.content.notebooklm_client.asyncio.sleep", new_callable=AsyncMock)
+async def test_rate_limit(mock_sleep):
+    await generate_with_rate_limit()
+    mock_sleep.assert_called()
+```
+
+### When to use which
+
+| Scenario | Tool |
+|----------|------|
+| Replace a module constant (`CONFIG_DIR`, `DEFAULT_DB`) | `monkeypatch.setattr` |
+| Set/override an env var | `monkeypatch.setenv` |
+| Replace a function to control its return value | `patch` (context manager) |
+| Replace a function AND verify call arguments | `patch` (decorator or context manager) |
+| Replace an async function | `patch` with `new_callable=AsyncMock` |
+| Replace `shutil.which` for tool detection | `patch` |
+| Replace `subprocess.run` for command isolation | `patch` |
+
+## Test Markers
+
+### `@pytest.mark.integration`
+
+For tests that require external infrastructure (tmux, network, real databases). These are excluded from fast local runs:
+
+```python
+import pytest
+
+pytestmark = pytest.mark.integration  # marks entire module
+
+# Or per-test:
+@pytest.mark.integration
+def test_tmux_session_lifecycle():
+    ...
+```
+
+Run integration tests explicitly:
+
+```bash
+uv run pytest -m integration           # only integration tests
+uv run pytest -m "not integration"     # skip integration tests
+```
+
+### `pytest.importorskip` — for optional dependencies
+
+Tests that require optional packages use `importorskip` at module level. The test file is collected normally but all tests in it are **skipped** at runtime if the dependency is missing:
+
+```python
+# At the top of the file, before any other imports from the optional package:
+pytest.importorskip("pymupdf")
+pytest.importorskip("fastapi")
+```
+
+Currently skipped groups: `pymupdf` (content splitter), `notebooklm` (notebooklm client), `fastapi` (web app/artefacts), `mcp` (MCP tools).
+
+## Adding a New Test
+
+### For studyctl
+
+1. Create `packages/studyctl/tests/test_<module>.py`
+2. Import helpers if needed: `from _helpers import make_review_db`
+3. Define fixtures inline or wrap helpers in `@pytest.fixture`
+4. If testing optional-dep code, add `pytest.importorskip("package")` at module level
+5. Do NOT create a conftest.py
+
+### For agent-session-tools
+
+1. Create `packages/agent-session-tools/tests/test_<module>.py`
+2. Use conftest fixtures (`temp_db`, `migrated_db`, etc.) directly in test signatures
+3. For exporter tests, use `migrated_db` — it has all migrations applied
+4. Add new shared fixtures to conftest.py if they'll be used by 3+ test files
+
+### Naming
+
+- Test files: `test_<module_name>.py`
+- Test classes: `Test<Feature>` (optional — flat functions are fine)
+- Test functions: `test_<behaviour_under_test>`
+- Fixtures: descriptive nouns (`review_db`, `isolated_config`, `projects_dir`)
+
+## Known TODOs
+
+- **Coverage configuration** — `pytest-cov` is in dev deps but studyctl has no `[tool.coverage.run]` config. Agent-session-tools does.
+- **`addopts` split** — running from workspace root uses `--import-mode=importlib` only. Running from a member directory uses that member's `addopts`. The root path is authoritative.
+- **Worktree tests** — 8 test files in `.claude/worktrees/feat+live-session-dashboard/` need merging (blocked on 5 source modules landing in main first).

docs/brainstorms/2026-04-01-test-consolidation-tdd-foundation-brainstorm.md

@@ -0,0 +1,103 @@
+# Test Consolidation & TDD Foundation
+
+**Date**: 2026-04-01
+**Status**: Decided
+**Next**: `/ce:plan` for cleanup work; TDD learning via fresh project
+
+---
+
+## What We're Building
+
+A consolidated, well-documented test suite for socratic-study-mentor, plus a learning path for TDD/pytest skills.
+
+**NOT a framework from scratch** — extracting and documenting patterns that already exist in 650 tests across two packages.
+
+## Why This Approach
+
+### The Problem
+
+- **650 tests exist** (293 studyctl + 357 agent-session-tools) but the user can't maintain them — they were AI-generated
+- **Near-zero pytest experience** — fixtures, parametrize, markers, conftest patterns are unfamiliar territory
+- **Inconsistent patterns**: 3 different mocking styles, 7 copy-pasted `migrated_db` fixtures, no pytest config for studyctl
+- **AuDHD users waiting** for the app — can't pause features for a learning sabbatical
+- **conftest prohibition** in studyctl/tests (pluggy conflict with agent-session-tools) forces inline fixtures
+
+### Rejected Approaches
+
+1. **"Build comprehensive test framework first"** — Rejected. This is Big Design Up Front for tests. YAGNI applies to test infrastructure. You can't design fixtures for features that don't exist yet.
+
+2. **"TDD bootcamp on this codebase"** — Rejected for now. Retrofitting TDD onto 650 AI-generated tests is the hardest way to learn. Better to learn TDD on a fresh project where every decision is yours.
+
+3. **"Document conventions, then apply"** — Rejected as primary approach. Risks the same "framework first" trap. Conventions emerge from practice.
+
+### Chosen Approach: Three-Part Split
+
+1. **Cleanup (this session)**: Mechanical consolidation — dedup fixtures, merge worktree tests, add pytest config, write TESTING.md. Claude does the work with heavy teaching commentary.
+
+2. **Feature work continues**: New features get properly tested by Claude, with inline teaching moments explaining every test pattern used. User learns by active observation.
+
+3. **TDD from scratch (future)**: User starts a new project using TDD from day one. Fresh codebase, full ownership, concepts grounded by patterns observed here.
+
+## Key Decisions
+
+| Decision | Choice | Rationale |
+|----------|--------|-----------|
+| Learn TDD on this codebase? | No — fresh project later | Retrofitting is the hardest way to learn |
+| Build test framework first? | No — extract from existing | YAGNI; emergent architecture over BDUF |
+| Include worktree tests? | Yes | 7 new files with useful patterns (tmux integration, session state, parking lot) |
+| Who does the cleanup? | Claude, with teaching commentary | User learns by watching, ships features in parallel |
+| conftest for studyctl? | Still no — use test helpers module instead | Pluggy conflict is real; a `tests/_helpers.py` can share fixtures without conftest |
+| Mock convention? | `monkeypatch` for attrs/env, `patch` for functions | Document when to use which; standardise across both packages |
+
+## Current State Audit
+
+### What works
+- 650 tests, 0 collection errors (after `uv sync --all-packages`)
+- agent-session-tools has clean conftest with 5 shared fixtures
+- `--import-mode=importlib` at root prevents pluggy conflicts
+- `pytest.importorskip()` pattern for optional deps is clean
+- Exporter tests use real fake filesystems (good integration pattern)
+
+### What needs fixing
+- 7 identical `migrated_db` fixtures across exporter tests → move to conftest
+- studyctl has no `[tool.pytest.ini_options]` in its pyproject.toml
+- `pytest.mark.live` declared but never used → either use it or remove it
+- 3 mocking styles with no convention for when to use which
+- 7 worktree test files need merging with conventions applied
+- No TESTING.md documenting patterns for contributors
+
+### What's intentional (don't change)
+- No conftest.py in studyctl/tests (pluggy conflict)
+- Inline fixtures in studyctl tests (consequence of above)
+- `pytest.importorskip()` at module level for optional deps
+- `--import-mode=importlib` (load-bearing for workspace)
+
+## Cleanup Scope
+
+### Phase 1: Mechanical (Claude does this)
+- [ ] Deduplicate `migrated_db` fixture into agent-session-tools conftest
+- [ ] Add `[tool.pytest.ini_options]` to studyctl's pyproject.toml
+- [ ] Resolve `pytest.mark.live` — use it or remove it
+- [ ] Merge 7 worktree test files into main tree
+- [ ] Create `packages/studyctl/tests/_helpers.py` for shared fixture functions (not conftest)
+- [ ] Write `docs/TESTING.md` with conventions
+
+### Phase 2: Ongoing (with features)
+- [ ] Every new feature gets TDD-style tests with teaching commentary
+- [ ] Teaching moments saved to Obsidian for reference
+
+### Phase 3: User's learning path (separate)
+- [ ] Start fresh project using TDD from scratch
+- [ ] Study-Mentor session on pytest/TDD fundamentals
+
+## Open Questions
+
+*None — all resolved during brainstorming dialogue.*
+
+## Success Criteria
+
+1. All 650+ tests pass from workspace root with `uv run pytest`
+2. Zero duplicated fixtures
+3. TESTING.md exists with clear conventions for: fixture patterns, mock conventions, test tiers, how to run tests
+4. User can read any test file and understand what it does (teaching moments bridge the gap)
+5. New features get properly tested with patterns explained inline

packages/agent-session-tools/pyproject.toml

@@ -122,7 +122,7 @@ testpaths = ["tests"]
 pythonpath = ["src"]
 addopts = "-v --tb=short"
 markers = [
-    "live: marks tests that require live database (deselect with '-m \"not live\"')",
+    "integration: requires external infrastructure (tmux, real DB, network)",
 ]
 
 [tool.coverage.run]

packages/agent-session-tools/tests/conftest.py

@@ -6,6 +6,8 @@
 
 import pytest
 
+from agent_session_tools.migrations import migrate
+
 
 @pytest.fixture
 def temp_db():
@@ -29,6 +31,14 @@ def temp_db():
     db_path.unlink(missing_ok=True)
 
 
+@pytest.fixture
+def migrated_db(temp_db):
+    """Return a temp_db with all migrations applied so exporter columns exist."""
+    conn, db_path = temp_db
+    migrate(conn)
+    return conn, db_path
+
+
 @pytest.fixture
 def temp_config_dir(tmp_path):
     """Create a temporary config directory."""

packages/agent-session-tools/tests/test_exporter_aider.py

@@ -5,22 +5,13 @@
 import pytest
 
 from agent_session_tools.exporters.aider import AiderExporter
-from agent_session_tools.migrations import migrate
 
 
 # ---------------------------------------------------------------------------
 # Fixtures
 # ---------------------------------------------------------------------------
 
 
-@pytest.fixture()
-def migrated_db(temp_db):
-    """Return a (conn, db_path) tuple with all migrations applied."""
-    conn, db_path = temp_db
-    migrate(conn)
-    return conn, db_path
-
-
 @pytest.fixture()
 def aider_tree(tmp_path: Path) -> Path:
     """Create a fake project directory containing an Aider history file.