feat(api): historial persistente y GET /history con paginación (#102) - #104
Merged
Conversation
feat(bench): add benchmarks for database connection and write performance feat(bench): measure impact of connection management on SQLite performance
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#102 · Historial: persistir análisis y exponer
GET /historycon paginaciónCloses #102
Hasta aquí la API era sin estado: cada petición se atendía con lo que traía
dentro y no dejaba rastro, así que reiniciar el proceso no perdía nada porque no
había nada que perder. R9 rompe eso —el usuario tiene que poder volver a ver un
análisis de ayer— y con ello aparece la primera escritura a disco del backend.
Qué incluye
backend/api/history.py(nuevo): el almacén, SQLite detrás de tresfunciones (
record,queryy el gestor de conexión). Lo que protege de uncambio de requisitos no es elegir el almacén más flexible sino aislarlo:
cambiar de motor es reescribir este fichero sin tocar endpoints ni sus tests.
backend/api/app.py: registro tras/analyzey tras/tools/{name}/execute, y la rutaGET /history. Una petición rechazada por404 o 422 no se registra: no es una ejecución, y ensuciaría el historial
con intentos fallidos del formulario.
backend/api/schemas.py:HistoryEntry,HistoryPagey los enumsHistoryKind/Origin, que viven aquí con el resto del contrato para que elconjunto cerrado de valores salga publicado en OpenAPI.
backend/config/settings.pyy.gitignore:history_dbapunta avar/y no adata/.data/está versionado —datasets y splits congelados,que no deben cambiar nunca— y esto es estado que cambia en cada petición;
juntarlos acaba en un
git add data/que commitea la base de datos.tests/api/test_history.py(22 tests) y el aislamiento entests/conftest.py.spikes/bench_historial_*.py: los dos bancos de prueba que sostienen lascifras de abajo, con sus resultados y conclusiones en la cabecera.
La decisión de fondo: se guardan análisis, no invocaciones
Al pie de la letra, «registrar cada ejecución de herramienta» haría que un solo
POST /analyzedejara cinco filas, una por señal, y la pantalla resultantesería una lista de
detect_clickbait_lexicalrepetido sin el titular por ningúnsitio. Esa granularidad ya existe donde sirve:
log_tool_invocationregistracada invocación con parámetros y duración. Son dos registros con dos públicos.
Se guarda la respuesta completa, no sólo el veredicto: reejecutar al abrir la
entrada costaría ~20 s en frío y las señales remotas no son deterministas, así
que el «resultado anterior» podría salir distinto. Un historial que cambia lo que
dice no es un historial.
Lo que salió de medir en vez de suponer
Una revisión externa señaló cinco puntos de rendimiento. Medirlos descartó tres,
corrigió uno y destapó otro que no estaba en la lista.
Path.mkdir(parents=True, exist_ok=True)sobre directorio existenteCREATE TABLE IF NOT EXISTSsobre tabla existente_conectar()+INSERT+commitEjecutar el esquema y el
mkdiren cada conexión cuesta el 0,005 % y el0,004 % de lo que envuelven: se quedan, porque hacen que el sistema funcione
recién clonado el repo sin ningún paso de instalación, y cachearlos en una
variable de módulo rompería los tests con
tmp_path. El timeout que seproponía añadir ya existía:
sqlite3.connectlo trae en 5 s por defecto, medidoen 5,01 s antes de
database is locked.La conexión sin cerrar sí era real, aunque no por el motivo que se le
atribuía. En
sqlite3, elwithde una conexión gestiona la transacción y nola cierra:
gc.collect()close()No es la fuga lineal que llevaría a
Too many open files, pero el atasco crece ygc.collect()lo devuelve siempre a cero: son conexiones esperando al recolectorde ciclos. El código era correcto por accidente, apoyado en un detalle de
CPython que PyPy no comparte.
_conectarpasa a gestor de contexto propio quecierra en un
finally, confirmando antes de cerrar — al revés se perdería laescritura, porque cerrar con una transacción pendiente la deshace.
WAL: la recomendación de manual, descartada por medición
journal_modesynchronousAl cerrar la última conexión a una base en modo WAL, SQLite ejecuta un
checkpoint completo; con «una conexión por operación» eso ocurre en cada
escritura. WAL rinde cuando las conexiones se mantienen abiertas, que es justo lo
que este diseño no hace: son dos decisiones acopladas, y quedarse con media de
cada una es peor que con cualquiera entera.
Los 0,47 ms de
synchronous=OFFprueban que esos ~164 ms son todofsync.No se toca: un historial que se pierde al cortarse la luz no es un historial.
Un fallo que sólo apareció mirando
var/Añadir el registro a
/analyzeconvirtió, sin avisar, todos los tests de esaruta en escritores del historial real: una corrida de la suite dejaba cuatro
entradas «Un titular» en
var/history.db. El aislamiento va en un fixtureautousedetests/conftest.pyy no en el fichero que prueba el historial,porque quien contamina no es quien lo prueba: lo hace cualquier test que llame a
un endpoint que registre, incluidos los que aún no existen.
Notas
límite; las columnas
tool,verdictystatusya existen para no tener quemigrar cuando llegue.
origines hoy siempreapi:formychatestán declarados porque elprototipo los distingue y añadir la columna después obligaría a migrar, pero no
hay quien los emita hasta que existan la SPA y el agente.
statusyverdictvan como cadena y no como enum, a propósito: son datosleídos de disco que pudo escribir otra versión del código, y un enum sobre eso
hace que el día que cambie un valor las filas antiguas dejen de validar y
GET /historydevuelva un 500 por una entrada de hace meses. El significado destatusademás difiere entre tipos —en un análisis es «alguna señal funcionó»,en una herramienta es «no falló»— y se replantea en [api] Historial: filtros y retención (R9.4, R9.5) #103.
fsyncse midieron sobre el disco virtual de WSL2, dondeatraviesa hasta el anfitrión Windows; en Linux nativo son décimas de ms. Queda
apuntado para volver a medirlo al contenerizar (H4) y decidir entonces si la
escritura debe salir de la ruta de respuesta.
HistoryEntry(**fila)sigue asumiendo que las claves del almacén casan conlos campos del contrato. El
SELECTnombra las columnas para que la salidadel módulo sea una declaración deliberada y no el reflejo de la tabla, pero
cerrar el acoplamiento del todo exigiría construir el modelo campo a campo. La
red hoy está en los tests, no en el código.