Repository navigation
Refactor SQLite access into a maintainable persistence layer #260
Description
Context
Requested by @ecarreras after reviewing the amount of inline SQLite access in the bridge.
The problem is not that the project uses explicit SQL. SQLite and hand-written SQL are a good fit for the queue semantics and operational footprint. The maintainability problem is that connection setup, schema compatibility, transactions, queries, row mapping and business decisions are mixed across HTTP handlers, services and domain modules.
Current inventory on main (v0.73.0), excluding tests:
| Module | SQL API calls | Main responsibility currently mixed with SQL |
|---|---|---|
queue.py |
107 | ingestion, deduplication, coalescing, claiming, lifecycle, runtime processes, state, schema compatibility and backfills |
feedback.py |
31 | feedback capture, proposals, rules, enrichment and GitHub-source resolution |
dashboard_data.py |
25 | dashboard read models, filters, metrics and compatibility checks |
backend.py |
17 | webhook monitoring queries and writes inside FastAPI handlers |
mcp.py |
11 | token persistence, authentication and schema compatibility |
observability.py |
9 | process samples and alert lifecycle |
actors.py |
8 | actor backfill plus schema compatibility |
web_push.py |
7 | subscriptions and delivery status |
webhook.py |
5 | delivery/hook persistence |
There are also direct sqlite3.connect() call sites in multiple modules with different defaults. Only some connections enable foreign keys; read-only behavior, timeouts, row factories and transaction modes are not centrally enforced.
Schema evolution is split between:
sql/schema.sql;JobQueue._ensure_columns()and_ensure_indexes();mcp._ensure_schema();- actor backfill schema helpers;
- feature-specific startup backfills.
Issue #190 already covers versioned migrations and should remain the source of truth for that part. This issue is the umbrella for the persistence architecture around it.
Goal
Keep SQL explicit, but isolate it behind a small persistence layer so that:
- application/services express business operations instead of SQL statements;
- transaction boundaries are named and reviewable;
- every connection uses the same operational policy;
- queue concurrency invariants remain protected;
- dashboard queries evolve independently from write workflows;
- schema changes go through versioned migrations rather than feature-local shims.
Non-goals
- Introduce SQLAlchemy or a full ORM.
- Build a generic repository framework.
- Convert every query into a separate
.sqlresource. - Rewrite the queue in one large PR.
- Hide SQLite-specific primitives such as
BEGIN IMMEDIATE, WAL or partial indexes where they are part of the design.
Priority 0 — establish safe foundations
- Complete Introduce versioned SQLite migrations #190: add versioned, idempotent migrations and remove feature-local schema mutation once migrated.
- Add one
Database/connection factory under a persistence package, with explicit read-write and read-only entrypoints. - Centralize
timeout,row_factory,PRAGMA foreign_keys=ON, WAL/busy policy and read-only/query-only behavior. - Add an explicit transaction context supporting deferred and
BEGIN IMMEDIATEtransactions; do not rely on each caller's implicitsqlite3behavior. - Document the transaction contract for ingestion, deduplication, coalescing, claiming and finishing jobs.
- Add concurrency/contract tests before moving queue SQL. They must prove:
- one canonical job per source/event;
- no two workers claim the same job;
- active jobs with the same
work_keystay serialized; - pause state is read inside the claim transaction;
- acknowledgements and job-run state cannot be partially updated.
- Preserve backup/restore as a specialized database operation; it does not need to pass through repositories.
- Make ordinary
JobQueueconstruction validation-only and require active-job checks plus backup/rollback in the explicit migration path — fix: require explicit SQLite migration preflight #283 (merged).
Priority 1 — separate persistence by domain
- Split the current
JobQueuegod object into focused persistence adapters, without changing public behavior initially — refactor: extract queue persistence repositories #277 (merged):-
JobRepository: job lifecycle and row mapping — refactor: extract queue persistence repositories #277 (merged); -
IngestionRepository: receipts, canonical events, deduplication and coalescing — refactor: extract queue persistence repositories #277 (merged); -
AcknowledgementRepository— refactor: extract acknowledgement and state repositories #273 (merged); -
RuntimeRepository: runs, worker heartbeats, runtime processes, worklog/progress/session events — refactor: extract runtime repository #275 (merged); -
StateRepository: bridge/executor state — refactor: extract acknowledgement and state repositories #273 (merged).
-
- Keep orchestration and policy decisions in services. Repositories should persist/query data, not decide GitHub policy or intent — refactor: extract queue persistence repositories #277, refactor: extract webhook and MCP repositories #278, refactor: extract push and observability repositories #279, refactor: extract feedback repository #280 (merged).
- Move webhook hook/delivery/action SQL out of FastAPI handlers into a
WebhookRepository— refactor: extract webhook and MCP repositories #278 (merged). - Move MCP token SQL into an
McpTokenRepository; authentication should consume repository methods rather than issue SQL directly — refactor: extract webhook and MCP repositories #278 (merged). - Move web-push subscription/delivery SQL into a
WebPushRepository— refactor: extract push and observability repositories #279 (merged). - Move observability samples/alerts into an
ObservabilityRepository— refactor: extract push and observability repositories #279 (merged). - Move feedback events/proposals/rules into a
FeedbackRepository; keep LLM classification and GitHub enrichment outside it — refactor: extract feedback repository #280 (merged). - Define typed row mappers/DTOs per domain. Stop spreading
sqlite3.Row, ad-hoc dictionaries and repeated JSON decoding beyond the persistence boundary — refactor: extract queue persistence repositories #277, refactor: extract webhook and MCP repositories #278, refactor: extract push and observability repositories #279, refactor: extract feedback repository #280 (merged).
Priority 1 — create an explicit dashboard read model
- Keep dashboard reads separate from write repositories: use a
DashboardQueries/read-model layer optimized for the UI — refactor: add explicit dashboard read model #284 (merged). - Remove SQL from
backend.py; route handlers should validate authorization/input and call query/command services — refactor: add explicit dashboard read model #284 (merged). - Keep list/detail/metrics queries intentionally denormalized where that avoids N+1 queries — refactor: add explicit dashboard read model #284 (merged).
- Preserve the existing single-query/per-endpoint performance criterion and the indexed dashboard ordering — refactor: add explicit dashboard read model #284 (merged).
- Replace generic
where_clause({column: value})composition with allowlisted filter objects so column names cannot become an accidental injection surface during future reuse — refactor: add explicit dashboard read model #284 (merged). - Keep compatibility reads only until the corresponding migration is guaranteed; then delete
table_exists()/column_exists()branching instead of preserving it indefinitely — refactor: add explicit dashboard read model #284 (merged).
Priority 2 — enforce boundaries and performance
- Add an architecture test preventing new
sqlite3.connect()/.execute()calls in HTTP handlers, CLI commands and domain services — refactor: enforce SQLite persistence boundaries #294 (merged). - Add repository integration tests against real temporary SQLite databases; avoid mocks that only assert SQL strings — feat: add SQLite persistence foundation #267, refactor: extract acknowledgement and state repositories #273, refactor: extract runtime repository #275, refactor: extract queue persistence repositories #277, refactor: extract webhook and MCP repositories #278, refactor: extract push and observability repositories #279, refactor: extract feedback repository #280 (merged).
- Add
EXPLAIN QUERY PLANregression tests for the hot paths — perf: finish SQLite persistence hardening #295 (merged):- runnable job claim — perf: finish SQLite persistence hardening #295 (merged);
- dashboard job ordering/filtering — refactor: add explicit dashboard read model #284 (merged);
- webhook delivery pagination and coverage windows — perf: finish SQLite persistence hardening #295 (merged);
- feedback rule lookup by scope/confidence — perf: finish SQLite persistence hardening #295 (merged);
- active MCP tokens and web-push subscriptions — perf: finish SQLite persistence hardening #295 (merged).
- Record slow/busy database operations with operation names rather than raw SQL or sensitive parameters — perf: finish SQLite persistence hardening #295 (merged).
- Define retention/cleanup ownership for webhook payloads, session events, worklogs, process samples and completed runs — refactor: enforce SQLite persistence boundaries #294 (merged).
- Document how to add a table, migration, repository method and read-model query — refactor: enforce SQLite persistence boundaries #294 (merged).
Suggested delivery slices
- Database foundation + migrations: Introduce versioned SQLite migrations #190, connection policy and transaction helpers.
- Queue write path: ingestion/claim/finish repositories with invariant tests.
- Dashboard read path:
DashboardQueriesand removal of SQL from FastAPI handlers. - Feature stores: webhook, feedback, MCP, push and observability repositories.
- Boundary enforcement: architecture tests, query-plan tests and removal of compatibility shims.
Each slice should be independently deployable and preserve the current schema/API behavior. Avoid a flag-day rewrite.
Acceptance criteria
- No direct SQL remains in FastAPI handlers, CLI commands, policy/classifier code or GitHub integration services — refactor: enforce SQLite persistence boundaries #294 (merged).
- Direct
sqlite3.connect()is restricted to the database package and explicit backup/restore code — refactor: enforce SQLite persistence boundaries #294 (merged). - All write workflows declare their transaction mode and boundary — perf: finish SQLite persistence hardening #295 (merged).
- Fresh databases and upgrades from supported historical schemas pass through the same migration system — fix: require explicit SQLite migration preflight #283 (merged).
- Queue/deduplication/coalescing/locking behavior has regression coverage before and after extraction — feat: add SQLite persistence foundation #267, refactor: extract acknowledgement and state repositories #273, refactor: extract runtime repository #275, refactor: extract queue persistence repositories #277 (merged).
- Dashboard list/detail/metrics endpoints do not regress in query count or query plan — perf: finish SQLite persistence hardening #295 (merged).
- The full test suite passes after every delivery slice — perf: finish SQLite persistence hardening #295 (merged).
Delivery tracking
- Versioned SQLite migrations — feat: add versioned SQLite migrations #263 (merged)
- Database foundation, queue contracts and transaction documentation — feat: add SQLite persistence foundation #267 (merged)
- Acknowledgement and state repositories — refactor: extract acknowledgement and state repositories #273 (merged)
- Runtime repository — refactor: extract runtime repository #275 (merged)
- Job and ingestion repositories — refactor: extract queue persistence repositories #277 (merged)
- Webhook and MCP token repositories — refactor: extract webhook and MCP repositories #278 (merged)
- Web-push and observability repositories — refactor: extract push and observability repositories #279 (merged)
- Feedback repository — refactor: extract feedback repository #280 (merged)
- Explicit migration preflight and non-mutating ordinary startup — fix: require explicit SQLite migration preflight #283 (merged)
- Dashboard read model — refactor: add explicit dashboard read model #284 (merged)
- Persistence boundary enforcement, actor backfill extraction and retention/extension docs — refactor: enforce SQLite persistence boundaries #294 (merged)
- Final query-plan, transaction-boundary and SQLite telemetry hardening — perf: finish SQLite persistence hardening #295 (merged)
Activity
ecarreras commentedon Oct 7, 2026
@giscebot comença a treballar en les tasques de Prioritat 0 si necessites separar-ho amb varies pull-requests segons el que facis ho fas. Un cop feta la pull-request actualitza la descripció de la tasca vinculant la pull-request
giscebot commentedon Oct 7, 2026
Primera entrega de Prioritat 0 oberta: #267.
Inclou la base
Databaseamb connexions read-write/read-only, transaccionsDEFERRED/IMMEDIATE, política comuna per les connexions migrades, contracte documental de les transaccions de cua i tests reals de concurrència/rollback. He mantingut l'extracció de repositoris fora d'aquesta PR perquè sigui una fase revisable i no un flag day.Validació: 472 tests locals i els tres checks de CI en verd (dashboard, Python 3.11 i Python 3.12). També he actualitzat la descripció de la issue amb #263 i #267. Les connexions feature-local que encara queden i l'extracció del SQL de cua continuen com a següents entregues.
ecarreras commentedon Oct 7, 2026
@giscebot què més queda de la prioritat 0 ? t'he dit que actualitzessis la descripció de la issue enallçant les pull-requests i marcat els checks dels items fets
giscebot commentedon Oct 7, 2026
Tens raó: havia actualitzat el tracking, però no els checks de la descripció. Ja està corregit.\n\nDe Prioritat 0 no queda res:\n- #263 va completar les migracions versionades.\n- #267 va completar la base Database RW/RO, la política comuna de connexions, les transaccions DEFERRED/IMMEDIATE, el contracte de cua, les proves de concurrència/rollback i l'excepció explícita de backup/restore.\n\nHe marcat tots aquests ítems i #267 al Delivery tracking. El següent bloc pendent ja és Prioritat 1: extreure repositoris per domini i migrar les connexions feature-local/SQL del dashboard sense canviar comportament.
ecarreras commentedon Oct 7, 2026
@giscebot comença doncs ara amb Priority 1 — separate persistence by domain Recorda que després has d'actualtizar la descripció d'aquesta issue amb la feina feta
giscebot commentedon Oct 7, 2026
Primera entrega de Priority 1 — separate persistence by domain oberta: #273.
Extreu
AcknowledgementRepositoryiStateRepository, afegeix els DTOs tipatsAcknowledgementClaimiExecutorPauseState, i mantéJobQueuecom a façana compatible. Les dues invariants delicades no es parteixen: l'acknowledgement es crea dins la transacció d'ingesta i la pausa es llegeix dins la transaccióBEGIN IMMEDIATEdel claim.Validació: 485 tests locals i els tres checks de CI en verd (dashboard, Python 3.11 i Python 3.12); la PR és
CLEANi mergeable. També he actualitzat la descripció de la issue amb subchecks per adapter i el tracking de #273, que queda sense marcar fins que es faci merge.Encara queden
JobRepository,IngestionRepository,RuntimeRepositoryi els repositoris/read models de la resta de dominis; #273 no afirma tancar la Prioritat 1 ni la issue paraigua.ecarreras commentedon Oct 7, 2026
@giscebot revisa que falta del punt Priority 1 — separate persistence by domain actualiza la descripció i si ja està fet tot continua amb l'altre apartat de prioritat 1
giscebot commentedon Oct 7, 2026
Revisió feta: Priority 1 — separate persistence by domain encara no està complet, així que no he saltat encara al read model del dashboard.
He actualitzat la descripció de la issue:
AcknowledgementRepositoryiStateRepositoryara consten com a completats amb refactor: extract acknowledgement and state repositories #273 fusionada.RuntimeRepositoryqueda vinculat a la nova PR #275, encara sense marcar fins que es fusioni.#275 extreu runs, heartbeats, processos runtime, worklog, events de sessió i progrés fora de
JobQueue, ambRuntimeProcesstipat i transaccionsBEGIN IMMEDIATEpels fluxos read→write. MantéJobQueuecom a façana i no canvia esquema ni API pública.Validació: 521 tests locals; CI dashboard, Python 3.11 i Python 3.12 en verd; PR
CLEANi mergeable. Encara quedenJobRepository,IngestionRepositoryi els repositoris feature-specific abans de donar aquest bloc per acabat.ecarreras commentedon Oct 7, 2026
@giscebot continua amb els punts de Priority 1 — separate persistence by domain fins a completar, si ho creus necessari fas stacked pull request
WaylonSmithersJr commentedon Oct 7, 2026
I found a migration-safety gap while reviewing #260 / #263 against an existing deployment.
JobQueue.__init__()always callsinit(), which calls_initialize_database()andapply_migrations()(queue.py). This happens in ordinary entry points, not onlygab migrate-db—for examplegab status/other CLI commands and backend/webhook queue construction. It does not check for active jobs or take a pre-migration backup.I reproduced this on a temporary SQLite backup of our deployed DB (production was not modified): before construction it had an active
waiting_approvaljob and noschema_migrations/job_commit_statusestables. A singleJobQueue(snapshot_path)using currentmaincreated both tables and recorded migrations 1 and 2 while thewaiting_approvaljob remained active. No backup or quiet-window preflight ran.This means the documented
autoupdateguard (active_jobs_block_migration, backup before migration) can be bypassed by any new-version process that opens the DB before the controlled migration step. The guard is especially relevant for a dashboard/webhook process or a manual CLI command started after package installation. The rollingSCHEMAsnapshot can also change a legacy DB beforeapply_migrations()runs.Could we make ordinary
JobQueueconstruction validate the existing schema/version and fail or defer when migration is required, leaving upgrades to an explicitmigrate-dbpath with the same quiet-queue/backup preflight? Fresh DB initialization can remain a separate permitted path. A regression test with an activewaiting_approvaljob and pending migration would pin this behavior.giscebot commentedon Oct 7, 2026
Completada la implementació de Priority 1 — separate persistence by domain en un stack de quatre PRs:
JobRepositoryiIngestionRepository.WebhookRepositoryiMcpTokenRepository.WebPushRepositoryiObservabilityRepository.FeedbackRepository.Ordre de merge: #277 → #278 → #279 → #280. Les quatre PRs estan
CLEAN, mergeables i amb dashboard/Python 3.11/Python 3.12 en verd. A cada capçalera del stack he executat també la suite local completa: 523 tests.He mantingut classificació/política, transport, autenticació, enriquiment GitHub i LLM fora dels repositoris; els adapters concentren connexions, SQL, transaccions i DTOs. No hi ha canvis d’esquema ni d’API pública.
També he actualitzat la descripció de la issue: #275 consta com a fusionada i tots els ítems nous queden vinculats a la seva PR. Els checks del stack continuen oberts fins que les PRs es fusionin. El següent bloc és l’altre apartat de Priority 1: dashboard read model; aquest stack no afirma tancar la issue paraigua.
ecarreras commentedon Oct 8, 2026
@giscebot revisa la issue I actualiza amb tot el que ja està fet i continua amb el que creguis més important, fes tota una secció i ho fas amb les pull-request que necessitis i stacked de GitHub
giscebot commentedon Oct 8, 2026
Revisió i següent secció completades.
JobQueuenomés valida;migrate-dbbloquejapending/running/waiting_approvalencara que l'executor estigui pausat, crea backup i restaura en cas d'error. Suite local: 526 tests; CI dashboard/Python 3.11/Python 3.12 en verd.DashboardQueries,JobListFiltersallowlisted, cap SQL a FastAPI/CLI/monitor, eliminació dels shimstable_exists()/column_exists(), una sola consulta per la llista i preservació del pla indexat. Suite local del stack: 529 tests; els tres checks de CI en verd.Ordre de merge: #283 → #284. Els checks de la descripció per aquestes dues entregues continuen oberts fins que es fusionin; no he marcat feina pendent com a feta.
ecarreras commentedon Oct 8, 2026
@giscebot actualiza la descripció de la issue amb l'estat actual marcant el que ja estigui fet i continua treballant amb els redactors que toqui segons prioritats
giscebot commentedon Oct 8, 2026
He actualitzat la descripció amb l'estat real: #283 i #284 consten com a fusionades, Priority 0 i els dos blocs de Priority 1 queden marcats, i Priority 2 ara separa els hot paths que encara no tenen cobertura de pla de consulta.
Nova entrega oberta: #294.
Aquesta PR completa el primer tall de Priority 2:
DashboardQueries;ActorBackfillRepositoryi elimina el shim DDL local;JobQueueal paquet de migracions;Validació: 541 tests locals; dashboard, Python 3.11 i Python 3.12 en verd; PR
MERGEABLE. No hi ha canvi d'esquema ni d'API. El següent bloc pendent és completar elsEXPLAIN QUERY PLANdels hot paths i després instrumentar operacions lentes/ocupades amb noms segurs.ecarreras commentedon Oct 8, 2026
@giscebot pots actualitzar la descripció de la issue i revisar què queda pendent
giscebot commentedon Oct 8, 2026
He actualitzat la descripció amb l’estat real després del merge de #294: ara consten com a completats el test de frontera arquitectònica, l’extracció del backfill d’actors, la restricció de
sqlite3.connect(), l’absència de SQL directe fora de persistència/read model, la documentació d’extensió i l’ownership de retenció.Queda pendent, per ordre recomanat:
EXPLAIN QUERY PLANpels hot paths encara descoberts: claim de jobs, paginació/finestra de cobertura de webhooks, lookup de regles de feedback i tokens MCP/subscripcions push actius.Matís: #294 defineix qui és responsable de cada retenció, però no introdueix encara una política automàtica d’eliminació de jobs completats; la documentació ho deixa explícitament com una operació futura i separada. La següent entrega natural és la cobertura de plans de consulta, deixant la instrumentació en una PR pròpia.
ecarreras commentedon Oct 9, 2026
@giscebot ataca ja la última part que quedi per tancar ja tot el refactor
giscebot commentedon Oct 9, 2026
Última entrega del refactor oberta: #295.
Completa els pendents de Priority 2:
Validació: 550 tests locals; CI dashboard, Python 3.11 i Python 3.12 en verd; PR MERGEABLE sobre main. He actualitzat la descripció de #260 vinculant #295 a tots els checks pendents i els he deixat oberts fins al merge. La PR inclou Closes #260, de manera que el merge tancarà el paraigua.
giscebot commentedon Oct 9, 2026
Refactor complet i tracking tancat.
La PR final #295 s’ha fusionat a
mainamb el commitba66d343. Els checks de dashboard, Python 3.11 i Python 3.12 són verds; la validació final documentada és de 550 tests.He actualitzat la descripció de #260: plans de consulta, telemetria SQLite, límits de transacció, regressions del dashboard, suite final i delivery tracking consten ara com a completats amb #295 fusionada. No hi queda cap checklist pendent i la issue resta tancada.