refactor(core)!: separar acceso destructivo, mover el umbral a la aplicación y añadir CI - #19
Merged
Merged
Conversation
Ruff se estaba ejecutando con su conjunto por defecto, que cambia entre versiones: un CI que dependa de él se rompe solo el día que alguien actualiza la herramienta. Se fija un select explícito (E, F, I, UP, PIE, SIM) en pyproject. Con esas reglas había 140 avisos. Este commit corrige los mecánicos: anotaciones PEP 585/604, orden de imports, placeholders innecesarios e imports sin usar. Los prompts quedan exentos de E501 mediante per-file-ignores: son datos, y partir una línea de prompt para que quepa en 88 columnas cambia el texto que recibe el modelo.
…licación Cierra tres problemas de diseño abiertos en el propio repositorio. #12 — clean_all() colgaba de IRepository, así que cualquier consumidor que solo necesitara leer o guardar recibía además la capacidad de borrar la base de datos entera. Se extrae a IAdminRepository, que ya existía pero no se estaba usando. Los adaptadores concretos implementan ambos puertos; los consumidores dependen del estrecho. IngestService recibe el puerto de administración de forma opcional y clean() falla con un mensaje claro si no se le inyectó: la ingesta no necesita permisos destructivos para hacer su trabajo. #14 — el umbral de similitud vivía como estado del adaptador de Supabase, que lo aplicaba en silencio por encima de lo que pidiera quien llamaba. Pasa a viajar en SearchOptions desde la capa de aplicación, de modo que el mismo repositorio puede atender una consulta estricta y otra permisiva sin reconfigurarse. El adaptador conserva solo una constante de último recurso, no una política. #18 — docling_chunker importaba docling y transformers en la cabecera del módulo, así que el extra opcional 'ingestion' dejaba de serlo en cuanto algo importaba el paquete. Las importaciones pasan a __init__, con un ImportError explicativo. Se corrige además un 'except:' desnudo en mongo_repository que se tragaba cualquier excepción, incluida KeyboardInterrupt, y convertía un fallo de serialización en un document_id mal tipado que solo se manifestaba más tarde como un join que no casaba nada. 13 tests nuevos cubren los tres límites. BREAKING CHANGE: IRepository ya no expone clean_all(). Quien la necesite debe depender de IAdminRepository. SupabaseRepository ya no acepta 'threshold' en el constructor.
- LICENSE (MIT): sin ella nadie podía reutilizar legalmente el código de un repositorio cuyo propósito declarado es enseñar el patrón. - CI con ruff y pytest sobre Python 3.10 y 3.12. Se instala sin el extra 'ingestion' a propósito, para que el CI verifique que el proyecto se importa y se testea sin las dependencias pesadas. - Se elimina debug_db.py, un script de depuración suelto en la raíz. - Insignias de CI y licencia en el README.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
El filtro apuntaba a 'main', pero la rama por defecto de este repositorio es 'dev-fran', así que el workflow no llegaba a ejecutarse en ninguna PR. El disparador de pull_request pasa a no filtrar por rama: una PR debe validarse apunte a donde apunte.
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.
Closes #12
Closes #14
Closes #18
Resumen
Cierra las tres issues abiertas del repositorio, añade la licencia y el CI que faltaban, y sube la cobertura de 22 a 35 tests.
#12 —
clean_all()colgaba deIRepositoryCualquier consumidor que solo necesitara leer o guardar recibía además la capacidad de borrar la base de datos entera.
IAdminRepositoryya existía en el repositorio pero no se estaba usando.IRepositorypierdeclean_all().IngestServicerecibeadmin_repositoryde forma opcional, yclean()falla con un mensaje claro si no se inyectó. La ingesta no necesita permisos destructivos para hacer su trabajo, así que no se le conceden por defecto.#14 — el umbral vivía en infraestructura
SupabaseRepositoryguardaba el umbral como estado y lo aplicaba por encima de lo que pidiera quien llamaba, en silencio. AdemásRAGServicenunca lo pasaba, así queSearchOptions.thresholdexistía sin usarse.Ahora el umbral viaja en
SearchOptionsdesde la capa de aplicación hasta el adaptador. El mismo repositorio puede atender una consulta estricta y otra permisiva sin reconfigurarse. El adaptador conserva soloDEFAULT_SEMANTIC_THRESHOLDcomo constante de último recurso, no como política.#18 — el extra
ingestionno era opcionaldocling_chunkerimportabadoclingytransformersen la cabecera del módulo, de modo que importar el paquete fallaba sin el extra instalado. Las importaciones pasan a__init__, con unImportErrorque dice qué instalar.Extra: un
except:desnudo que era un fallo realEn
mongo_repository, la conversión dedocument_idaObjectIdestaba envuelta enexcept: pass. Se tragaba cualquier excepción —incluidaKeyboardInterrupt— y convertía un fallo de serialización en undocument_idmal tipado que solo se manifestaba mucho más tarde, como un join que no casaba nada. Ahora capturaInvalidIdyTypeError, y avisa.Cambios de infraestructura
LICENSE.github/workflows/ci.ymlpyproject.tomldebug_db.pyEl CI instala sin el extra
ingestiona propósito: así verifica de forma continua que el proyecto se importa y se testea sin las dependencias pesadas, que es justo lo que pide la #18.Plan de prueba
ruff check src/ tests/— All checks passed!ruff format --check— 37 archivos ya formateadospytest -q— 35 pasados (antes 22)tests/test_repository_boundaries.py, que fallan si alguien devuelveclean_allaIRepository, si el umbral deja de llegar al adaptador, o sidoclingvuelve a importarse en la cabeceraBREAKING CHANGE
IRepositoryya no exponeclean_all(): quien la necesite debe depender deIAdminRepository.SupabaseRepositoryya no aceptathresholden el constructor.