Skip to content

Read-only research commands open the deployment database read-write, and migrate it #610

Description

@eaitbrahim

Found while reviewing #609 (keel research). Pre-existing, not introduced there — but #609
widens the surface and advertises it in a new docs page, which is what makes it worth fixing.

The behaviour

keel research significance --from deployment, keel research factors and
keel research independence are read-only questions. They reach their data through
_open_repo (keel/commands/_common.py:128-131):

def _open_repo(ctx: click.Context) -> Repository:
    conn = connect(ctx.obj["db_path"])
    migrate(conn)
    return Repository(conn)

migrate commits unconditionally (keel/data/db.py:595). So a question about the database
opens it read-write, and — when the running binary is newer than the file — migrates its
schema as a side effect of being asked.

Why it is worth a seam rather than a shrug

Two reasons, neither of which was true before #609.

  1. The same file already does it the other way, on the same tables.
    keel research pooled-review reads every profile with
    sqlite3.connect(f"file:{db}?mode=ro", uri=True) and is pinned by a test that hashes the
    fixture .db files before and after a run. Two commands in keel/commands/research.py
    read orders/trade_outcomes with opposite safety postures, and nothing explains why to
    someone reading only one of them.

  2. The docs page now invites the read. docs/research-toolkit.md tells an operator these
    commands answer questions about their deployment. The 2026-09-30 pooled review (The 2026-09-30 n=100 pooled review is underpowered: n_eff is about 39, and it can only detect a 20-point edge #427) is
    the exact scenario — pointing analysis at live ledgers, possibly while the agent is
    running.

WAL makes this survivable in practice, and this is not a live-trading defect. It is a sharp
edge that got sharper.

The ask

A read-only repository seam — _open_repo_ro(ctx), or an _open_repo(ctx, *, read_only=True)
that connects mode=ro and skips migrate — and the read-only research commands moved onto
it. pooled-review already shows the shape.

Two things to decide rather than assume:

  • What a read-only command should do against a database that needs migrating. Refusing
    with "this database predates the running binary; run a command that writes, or upgrade" is
    probably right, and it is an operator error rather than an evidence refusal (keel research: thirteen evidence modules with no front door #601's
    distinction) — but that is a call, not an obvious default.
  • How wide to take it. Other read-only commands (insights, status, pnl, activity)
    have the same shape. Doing all of them at once is a bigger change than this issue needs;
    doing only research/ leaves the inconsistency, just smaller. My inclination is the seam
    plus research/ here, and the rest as follow-up once the seam has a shape.

A pin belongs with it: hash the database file before and after, the way
test_pooled_review_reads_are_read_only already does in
tests/commands/test_research_commands.py.

Not urgent

Documented as a hazard in docs/research-toolkit.md § "Which of these open your database
read-write" in the meantime, with the advice to copy the database first if it matters.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions