Storage¶
One SQLite file, in WAL mode, holding everything that has to survive a
restart. Implemented in src/pr_review_agent/store.py.
Eight tables live here: the watermarks and ETags below, the
queue rows whose claim protocol is described in QUEUE.md,
the ledger the budget governor computes its windows from,
budget_state and budget_policy beside it, the runs a paid review or
description produced, and agent_comments, the ids the agent posted. The
schema is declared in one place — this module — because migration order has to
be a single sequence.
🗄 Why SQLite¶
The topology is one writer on one host. PostgreSQL's multi-client concurrency would be capability paid for and never used, while SQLite's write serialisation actively simplifies the hardest invariant in the system: the budget reservation has to be atomic with the queue claim, and SQLite's single write lock gives that for free where Postgres would need explicit row locking.
It also needs no daemon, no port and no DBA on a locked-down host, and
sqlite3 is in the standard library. Revisit only if the agent ever becomes
multi-host.
WAL mode is on because the later phases (queue, lease, budget governor) read this file while the poller writes it. The write lock stays single-writer either way, which is the property the reservation depends on.
busy_timeout is 5 s, so a connection that meets that lock waits rather than
failing on the spot. Several daemons sharing one file can still exceed it —
the reviews are minutes long and the writes are not staggered — and SQLite
reports the lost wait as a plain OperationalError. is_contention tells
that apart from a genuine fault by its message, because the poll loop skips a
cycle for the first and crashes for the second; see
DAEMON.md.
🧭 Watermarks¶
The poller sees open pull requests and recent comments, not opened events, so
the classifier needs a timestamp below which everything has already been
considered.
Held only in memory, the cost of a restart is not a wasted poll — it is spent allowance:
- for pull requests, the whole open backlog is re-offered as fresh;
- for comments, every old
@claudeis replayed as a new request.
A watermark therefore lives on disk, and only ever moves forward. A restart
that read a stale row, or two cycles settling out of order, must not walk it
backwards and re-admit events already decided. advance_watermark takes the
later of the stored and the offered value and returns whichever is in force.
Watermarks are namespaced by stream (pull_requests, comments) because the
two advance independently: comments are sorted by updated, pull requests by
created. Both comment endpoints share the one comments mark — updated
only ever moves forward, so a single high-water mark cannot hide a comment that
surfaces later on the other endpoint.
They are namespaced by repository as well, so the key is
pull_requests:owner/name rather than pull_requests. One store is how
several daemons share one token budget — see BUDGET.md — and an
unqualified key would be written by whichever repository polled last, leaving
every other one reading its own backlog as already seen and skipping it
permanently.
A database written before the key carried a repository holds unqualified
pull_requests and comments rows. A schema migration cannot rename them: the
repository is named in config.yaml and is not in the database at all. So the
daemon adopts them at startup instead, copying each onto its qualified name
before seeding.
Adoption happens once per store, not once per repository, and the unqualified rows are deleted in the same transaction. The rows describe the one repository that was polling before the upgrade, so a second repository later pointed at that store must not inherit them: it would start from a timestamp it has never polled — possibly weeks back — and enqueue and pay for every open pull request since, which is the cold-start spend bound DAEMON.md exists to hold. Deleting them costs a downgrade its watermark, and that is the cheaper of the two.
The daemon loop is what advances them, to the newest timestamp it saw in a payload rather than to wall-clock now, and only after the enqueue that the timestamp accounts for.
Values are stored as aware UTC ISO-8601 strings. A naive datetime is rejected
at the boundary for the same reason Classifier.since rejects one — a naive
comparison against a GitHub ...Z timestamp raises TypeError at the worst
possible moment.
🏷 ETags¶
Cheaper to lose than a watermark: a missing ETag costs one full GET per
endpoint on the next cycle, not correctness. It lives in the same file because
it has the same shape and the same lifetime.
SqliteStore satisfies the ETagCache protocol, so it drops into Poller
exactly where the in-memory ETagStore goes:
1 2 3 | |
The in-memory ETagStore remains the default. It is what the tests use, and
what a one-shot poll wants.
📋 Schema¶
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 | |
budget_policy is what lets several daemons share one file and therefore one
token budget — see BUDGET.md.
The CHECK holds it to one row because a second row would be a second opinion
about one allowance. It is kept apart from budget_state even though both are
small and scalar: that one is what the circuit breaker learned, this is what
an operator declared, and resetting either must not disturb the other.
runs is the only table holding review content — findings, the
assessment, the withheld paths and, for @claude describe, the description — and therefore the only
one the retention sweep purges. It is written before the publisher is asked,
which is what lets a failed GitHub write be retried without a second review —
see PUBLISHER.md.
content_purged_at is stamped separately from emptying findings, so a run
whose content was deleted after a merge stays distinguishable from a run that
looked and found nothing.
queue.comment_id and queue.comment_source are NULL together for a
pr_opened row: nobody wrote a comment to acknowledge, so the 👀 goes on the
pull request itself.
budget_state holds the circuit breaker's
three scalars, and is a separate table from ledger for a reason worth being
explicit about: ledger rows are tokens genuinely consumed and the rolling
windows sum them, so breaker state kept there would be counted as spend.
Key/value because these are three unrelated values rather than a row of one
thing, and every key absent is what "never tripped" reads back as — which is
how a database written before version 9 adopts it with no backfill.
The ledger is append-only and never pruned, even when review content is purged after a merge: the rolling budget windows are computed from it, so deleting a row would silently hand back allowance that was genuinely spent. The retention split is purge content, retain metrics. See DESIGN.md.
A reservation is identified by its dedupe_key while it is open, and
ledger_open is what says so. Governor.settle matches on the key and the
owner, but those two clauses answer different questions: uniqueness — is
there exactly one open reservation for this trigger? — is the index's job,
and entitlement — is it ours? — is the owner's, which is how a worker whose
lease lapsed learns to discard a result it may no longer post.
Without the index they were the same clause, and wrongly. daemon.supervise
restarts a crashed worker with the same owner, so an attempt that died
holding a reservation and then re-claimed the row wrote a second open row
with the identical pair. One settle matched both, updated both, reported no
single reservation, and the worker threw away a review it had already paid
for — while the crashed attempt's ceiling was overwritten by the retry's
actual usage, under-counting the spend. admit now closes the predecessor as
lost inside
the claim's own transaction, so there is never a second open row to be
ambiguous about.
Two columns it deliberately does not have. There is no state, because a
row is reserved exactly when settled_at IS NULL. There is no expires_at,
because an unsettled reservation stays charged until it ages out of its
rolling window rather than being released when its lease lapses — see
BUDGET.md.
repo and pr_number were absent for the same reason until migration 11
added them, because the pacer
counts one pull request's reviews off the ledger.
reviewed_lines is the one column that exists for something other than the
windows: it is the predictor the
pre-flight estimate fits a
tokens-per-line rate against. It stays NULL on a row that reviewed nothing —
a refusal, or a run recorded before the column existed — so those rows never
enter the fit.
stop_reason is why the run ended, which is a different question from
usage_confidence's how far its recorded cost can be trusted. Without it a
run killed on the wall clock and one whose envelope would not parse are the
same row: both unavailable, both charged the full reservation, and nothing
to say which control bound the run. One of eleven values — completed,
truncated, failed, timeout, engine_error, engine_unavailable,
refused, closed, lost, infrastructure, usage_limit — rather than
free text, because it is read by an operator
and, later, by the circuit breaker, and GROUP BY stop_reason has to mean
something. Rows written before the column existed keep a NULL: nothing
recorded why they stopped, and inventing a reason would put fiction in the
one table that is never pruned.
Three of those values settle at zero, and they are kept apart on purpose:
refused is the pre-flight estimate saying no, engine_unavailable is a
subprocess that never started, and closed is a pull request merged or
closed before the claim reached a checkout. No engine ran in any of the
three, but the operator action differs in each — a limit, a host, or a queue
that is too slow.
🔢 Migrations¶
Schema changes are an ordered list applied on connect, with the file's
PRAGMA user_version recording how many have run. Version 1 is the ETag and
watermark tables; version 2 adds the queue; version 3 adds the ledger; version
4 indexes the ledger by (actor_id, reserved_at) for the per-contributor
budget window; version 5 adds ledger.reviewed_lines; version 6 adds
ledger.stop_reason; version 7 adds queue.comment_id and
queue.comment_source, which is what lets the publisher acknowledge the
comment a mention was written in; version 8 adds runs; version 9 adds
budget_state; version 10 adds budget_policy; version 11 adds
ledger.repo, ledger.pr_number and the (repo, pr_number, reserved_at)
index the pacer reads; version
12 adds runs.publish_outcome; version 13 adds runs.publish_attempts and
runs.publish_failed_at; version 14 adds agent_comments; version 15 adds
the ledger_open unique index; version 16 adds ledger.reviewed_since;
version 17 adds runs.omitted; version 18 adds runs.assessment; version
19 adds queue.command and runs.description, for
@claude describe.
Version 15 cleans up before it constrains, and that order is the whole
migration. A unique index cannot be created over a table that already
violates it, and a store that will not migrate is a daemon that will not
start — so a live database holding duplicate open rows would be bricked by
the index alone. The migration therefore settles the duplicates first: for
every dedupe_key with more than one open row it keeps the newest, which is
the one a running worker may still settle, and closes the older ones at their
full reserved_tokens with stop_reason = 'lost' and usage_confidence =
'unavailable'. Their settled_at is their own reserved_at, because
nothing was learned about those runs after the instant they opened and a
later timestamp would imply otherwise. Both statements are one migration
tuple, so the cleanup and the constraint commit together — a crash between
them cannot leave a database with duplicates and no index.
No allowance is handed back by that cleanup. Those rows were counting their whole reservation while they sat open and they count exactly the same afterwards; what changes is that the charge now says why it exists.
Version 13's counter starts at zero on every row written before it, which is the honest reading: nothing counted those posts. A run carrying a stamp is never offered for publication again, so an operator who has fixed whatever GitHub was refusing — an unlocked pull request, a restored token scope — puts the review back in the queue by clearing it:
1 2 | |
The daemon picks it up the next time a publication item names that run; until
then the findings sit in runs, which is where they have been all along.
Version 14's agent_comments is the agent's memory of what it has said: the
classifier reads it once per poll cycle and drops any comment whose id is
there, which is what stops the agent reviewing its own review
(TRIGGERS.md). It is a table
of its own rather than a column on runs because it has to cover every
comment the publisher posts, whether or not a review is behind it, and
because it is never pruned — a retention sweep that deletes runs rows
must not make the agent forget what it said. It starts empty on an existing
store, which reads the same as a first run: comments posted before the
migration are not known to be the agent's, and mention.neutralise is what
covers them.
Version 11's two columns are NULL on every row written before it, and
that is the reading the pacer is built around: no repository and no pull
request means no history, which is the same answer a pull request nobody has
reviewed gets. A backfill is impossible — the ledger is keyed by
dedupe_key, and the pull request a retired key referred to is not
recoverable from it — so the honest default was the design constraint rather
than an afterthought.
Each migration and its version bump commit together, in one transaction.
That is what lets every ALTER TABLE ADD COLUMN migration — 5 to 7, 11 to
13 and 16 to 19 — be written that way, which
SQLite has no IF NOT EXISTS form for and which fails outright on a second
application.
A crash mid-migration rolls the pair back and the migration is simply
re-applied.
The CREATE statements are still IF NOT EXISTS, but now for one reason
rather than two: a database created before the list existed already carries
version 1's tables at user_version = 0, and has to be able to adopt it.
executescript would be the natural way to run a multi-statement migration
and is deliberately not used — it issues a COMMIT of its own first, which
would split the script from its version bump.
🔐 Write transactions¶
SqliteStore.transaction() runs a block inside one BEGIN IMMEDIATE. The
write lock is taken up front rather than on first write, which is what makes a
read-then-write sequence — the queue's conditional claim
— atomic against another writer.
That is also where the budget reservation goes, through claim()'s admit
hook, so that a claim and the allowance it spends commit together or not at
all.