Skip to content

refactor(db): split monolithic db.py into per-entity package - #62

Open
tachyon-beep wants to merge 2 commits into
mainfrom
claude/refactor-db-module-gcSGR
Open

refactor(db): split monolithic db.py into per-entity package#62
tachyon-beep wants to merge 2 commits into
mainfrom
claude/refactor-db-module-gcSGR

Conversation

@tachyon-beep

Copy link
Copy Markdown
Collaborator

Summary

keisei/db.py had grown to 1,158 lines covering DDL for 21 tables, eight v1→v8 migrations, and read/write helpers for ≥12 entity families. Touching one entity meant scrolling past every other. This PR splits the module into a keisei.db package where each entity owns its DDL and helpers.

  • keisei/db/__init__.pySCHEMA_VERSION, init_db, wal_checkpoint, public re-exports
  • keisei/db/_connection.py_connect (the shared SQLite handle factory)
  • keisei/db/_migrations.py_migrate_v*_to_v*, _MIGRATIONS registry, apply_migrations runner
  • 12 entity submodules (metrics, snapshots, training_state, league, head_to_head, historical, gauntlet, tournament, game_features, style_profiles, showcase, tournament_queue) — each contributes a DDL constant plus its read/write helpers

init_db concatenates each submodule's DDL in the original executescript order so the resulting schema is byte-equivalent. The migration chain is unchanged (SCHEMA_VERSION = 8).

Also folds two pre-existing per-table modules into the package and updates 11 importers:

  • keisei/showcase/db_ops.pykeisei/db/showcase.py
  • keisei/training/tournament_queue.pykeisei/db/tournament_queue.py

The public API is re-exported from keisei.db, so all 50+ existing from keisei.db import … sites work unchanged.

Test plan

  • uv run pytest tests/test_db.py tests/test_db_edge_cases.py tests/test_db_league_schema.py tests/test_db_style_schema.py — 95 passed
  • uv run pytest tests/test_showcase_db.py tests/test_showcase_runner.py tests/test_server_showcase.py — 37 passed
  • uv run pytest tests/test_tournament_queue.py tests/test_tournament_worker.py tests/test_tournament_dispatcher.py tests/test_tournament_sidecar_integration.py — 55 passed
  • Full suite: 1547 passed; the 3 failures (test_amp.py/test_pytorch_amp_pipeline.py GradScaler — CUDA absent; test_historical_gauntletshogi_gym extension not built; test_psutil_available_returns_values — psutil cold-call returns None) are pre-existing environment issues, verified to fail on the pre-refactor tree.
  • Smoke check: python -c "from keisei.db import init_db, _connect, write_metrics, read_league_data; from keisei.db.showcase import queue_match; from keisei.db.tournament_queue import enqueue_pairings" resolves.
  • Lint: ruff check on touched files reports 14 errors (all pre-existing); pre-refactor baseline was 17 across the same code, so this PR net-reduces lint debt.

Generated by Claude Code

The 1,158-line db.py held DDL for 21 tables, eight v1→v8 migrations, and
read/write helpers for ≥12 entity families. Touching one entity required
scrolling past the others. Splits the module into a keisei.db package:

- keisei.db.{_connection,_migrations} hold the shared connection helper
  and the migration registry (single source of truth, unchanged chain).
- Per-entity submodules (metrics, snapshots, training_state, league,
  head_to_head, historical, gauntlet, tournament, game_features,
  style_profiles, showcase, tournament_queue) own their CREATE TABLE
  DDL plus the read/write helpers for those tables.
- keisei.db.__init__ orchestrates init_db by concatenating each
  submodule's DDL constant in the original executescript order, then
  calls _migrations.apply_migrations. SCHEMA_VERSION = 8 is unchanged.
- Public API (init_db, _connect, write_metrics, etc.) is re-exported
  from keisei.db so all 50+ existing import sites keep working.

Also folds keisei.showcase.db_ops -> keisei.db.showcase and
keisei.training.tournament_queue -> keisei.db.tournament_queue (their
DDL was already in the same monolith), and updates 11 importers.

Resulting schema is byte-equivalent: all 118 db-targeted tests, 37
showcase tests, and 55 tournament-queue tests pass. Net-reduces ruff
debt across the touched files (17 -> 14 errors).
Copilot AI review requested due to automatic review settings May 5, 2026 06:40
Follow-up to e7441c0: those three files were intended to be removed in
the same commit but a git-stash round-trip during verification dropped
the staged deletions. Their replacements already live under keisei/db/.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the database layer by introducing a keisei.db package with per-entity modules, while updating showcase and tournament-queue import sites to use the new package layout.

Changes:

  • Added modular keisei.db submodules for schema DDL, migrations, shared connection setup, and entity-specific read/write helpers.
  • Updated training, showcase, server, and test imports to use keisei.db.showcase and keisei.db.tournament_queue.
  • Kept the root keisei.db API surface re-exported through keisei/db/__init__.py.

Key review findings:

  • mark_pairing_done() can accept stale completions after a pairing has been reset/reclaimed, which makes crash-recovery unsafe.
  • write_showcase_move() can move showcase_games.total_ply backwards on duplicate/retried move writes.
  • The old monolithic keisei/db.py still exists, so the refactor currently leaves duplicate DB implementations in-tree.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_tournament_worker.py Updates test imports to keisei.db.tournament_queue.
tests/test_tournament_sidecar_integration.py Updates sidecar integration test import path.
tests/test_tournament_queue.py Updates queue test imports to new DB module.
tests/test_tournament_dispatcher.py Updates dispatcher tests to new queue module path.
tests/test_showcase_runner.py Updates showcase runner tests to new showcase DB module.
tests/test_showcase_db.py Updates showcase DB tests to new module path.
tests/test_server_showcase.py Updates server showcase tests to new module path.
keisei/training/tournament_runner.py Switches worker code to keisei.db.tournament_queue.
keisei/training/tournament_dispatcher.py Switches dispatcher queue imports to new DB package.
keisei/training/katago_loop.py Updates tournament-queue import path and import ordering.
keisei/showcase/runner.py Switches showcase runner DB helpers to keisei.db.showcase.
keisei/server/app.py Switches showcase API helpers to keisei.db.showcase.
keisei/db/training_state.py Extracts training-state table DDL and helpers.
keisei/db/tournament.py Extracts tournament stats table helpers.
keisei/db/tournament_queue.py Extracts tournament queue + worker heartbeat logic.
keisei/db/style_profiles.py Extracts style-profile storage helpers.
keisei/db/snapshots.py Extracts game snapshot storage/query helpers.
keisei/db/showcase.py Extracts showcase queue/game/move/heartbeat helpers.
keisei/db/metrics.py Extracts metrics table DDL and read/write helpers.
keisei/db/league.py Extracts league tables and read helpers.
keisei/db/historical.py Extracts historical library table DDL.
keisei/db/head_to_head.py Extracts head-to-head table and backfill helpers.
keisei/db/gauntlet.py Extracts gauntlet results table DDL.
keisei/db/game_features.py Extracts game-features table and queries.
keisei/db/_migrations.py Extracts migration registry and runner.
keisei/db/_connection.py Extracts shared SQLite connection helper.
keisei/db/init.py Re-exports public DB API and builds schema from submodules.
Comments suppressed due to low confidence (2)

keisei/db/showcase.py:213

  • write_showcase_move() makes duplicate move writes a no-op with INSERT OR IGNORE, but the following UPDATE showcase_games SET total_ply = ? still runs unconditionally. If an older ply is retried after later moves have already been stored, total_ply is moved backwards and the dashboard will report an incorrect game length.
    keisei/db/tournament_queue.py:246
  • This terminal update is keyed only by id. After reset_stale_playing() requeues a supposedly dead claim, a late write from the old worker can still flip the row to done/failed, even if another worker has already reclaimed it. The recovery path needs to verify the row is still in playing for the same worker before accepting the completion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread keisei/db/__init__.py
Comment on lines +1 to +12
"""SQLite database layer — schema, migrations, read/write helpers.

This is a per-entity package: each table family lives in its own submodule
(``metrics``, ``snapshots``, ``training_state``, ``league``, ``head_to_head``,
``historical``, ``gauntlet``, ``tournament``, ``game_features``, ``style_profiles``,
``showcase``, ``tournament_queue``). Each submodule contributes a ``DDL`` constant
and the read/write helpers for its tables. ``init_db`` concatenates the DDL
blocks in their original order and runs them as a single ``executescript``;
the migration registry stays in :mod:`keisei.db._migrations`.

The public read/write API is re-exported from this package, so every existing
``from keisei.db import …`` site keeps working.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale — keisei/db.py was deleted in the follow-up commit 4f0136b. The reviewer was looking at the first commit (e7441c0), which staged the new package but lost the deletion when a git stash round-trip dropped it; the second commit on this PR removes the file. The latest tree contains only keisei/db/.


Generated by Claude Code

Comment thread keisei/db/metrics.py
timestamp TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%SZ', 'now'))
);
CREATE INDEX IF NOT EXISTS idx_metrics_epoch ON metrics(epoch);
CREATE INDEX IF NOT EXISTS idx_metrics_id ON metrics(id);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct observation, but out of scope for this PR. The idx_metrics_id index existed in the pre-refactor db.py (line 216) and the agreed plan for this refactor explicitly preserves byte-equivalent schema — split only, no behaviour changes. Removing the redundant index is a separate cleanup; happy to follow up with a one-line PR if you want it tracked.


Generated by Claude Code

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.

3 participants