Revisión de calidad
Objetivo: $ARGUMENTS
Revisa el objetivo en cuanto a calidad de diseño y pruebas. Reporta hallazgos; no edites código.
1. Cargar la rúbrica
Lee ambos antes de mirar el código:
~/.claude/CODING_STANDARDS.mdes la rúbrica. Cada hallazgo cita uno de sus encabezados de sección.- design-vocabulary.md contiene los términos para formular los hallazgos y las pruebas utilizadas para verificarlos a continuación.
Lee también AGENTS.md / CLAUDE.md del repositorio si existen. Si esos archivos indican que el repositorio deliberadamente hace algo que los estándares desaconsejan, menciónalo una vez en el informe en lugar de marcar cada instancia. Un hábito que solo observas en el código (cada módulo lee el entorno, cada prueba hace monkeypatch) no es una elección documentada: el código nuevo que lo sigue sigue siendo un hallazgo.
2. Resolver el objetivo
- Sin argumento: la rama actual contra su base.
git merge-base HEAD origin/main(omaster), luegogit diff <base>...HEADmás cambios no confirmados degit diff HEAD. - Número de PR o URL:
gh pr diff <n>ygh pr view <n> --json baseRefName,headRefName,files. - Rama:
git diff <base>...<branch>. - Ruta, o
.para todo el repositorio: revisa el código existente bajo esa ruta tal como está, sin diff. Esto es modo base de código, a continuación.
3. Leer lo suficiente para juzgar el diseño
Modo cambio (diff, rama, PR)
Un diff por sí solo no puede mostrar si un módulo es profundo. Para cada módulo cambiado, lee el archivo completo y busca sus llamadores y sus pruebas. La profundidad, la propiedad y la duplicación son juicios sobre los sitios de llamada, así que míralos.
Limita los hallazgos a lo que el cambio introduce o empeora. Un problema preexistente en código que el diff toca está dentro del alcance si el cambio era el momento natural para corregirlo; uno en código no tocado no lo está.
Los módulos nuevos reciben revisión a escala de módulo, como en el modo base de código: recorre Tipos y Legibilidad con la misma profundidad que las otras secciones. Para cada módulo nuevo o reescrito sustancialmente, enumera cada función con sus tipos de parámetros y retorno antes de juzgarlo, y anota cada map, dict o tuple sin tipo que acepte, retorne o lea claves, y cada nombre de una sola letra.
Mide la complejidad ciclomática para cada archivo fuente cambiado antes de juzgar Legibilidad, con la herramienta habitual del lenguaje: por ejemplo radon cc -s para Python, gocyclo para Go, la regla complexity de eslint para JavaScript/TypeScript, cognitive_complexity de clippy para Rust, PMD para Java. Ejecútalo con las herramientas propias del repositorio si las tiene, o con un ejecutor único (uvx, npx, go run) si no. Prefiere una herramienta que cuente operandos booleanos de cortocircuito; algunas (C901 de ruff, por ejemplo) los omiten y subestiman las peores funciones. Si no hay herramienta disponible, cuenta los puntos de decisión manualmente como los define la rúbrica. Una función que el diff añade con puntuación superior a 10, o una existente que el diff supera 10 o aumenta más, es candidata. Para una función existente, puntúa también la versión base (escribe git show <base>:<file> en un archivo temporal y mídela) para que el hallazgo pueda decir qué añadió el cambio.
Cuando el diff añada más de unas 150 líneas de código fuente, lanza dos subagentes general-purpose en paralelo mientras revisas las secciones de diseño, dándoles a cada uno el diff, la lista de archivos cambiados y las reglas de verificación del paso 4 para sus secciones: uno para Tipos, Legibilidad y "Inclínate hacia datos inmutables donde valga la pena", el otro para Pruebas, aplicando las comprobaciones de aserción a cada nueva prueba. Verifica lo que cada uno devuelva antes de conservarlo.
Modo base de código (ruta o .)
Todo lo que esté bajo la ruta está dentro del alcance, pero el umbral para un hallazgo se eleva con el tamaño del alcance. Un solo módulo o directorio pequeño (aproximadamente menos de 1500 líneas de código fuente) recibe hallazgos a nivel de módulo, función y nombre: recorre Tipos y Legibilidad con la misma profundidad que las otras secciones. Un paquete o todo el repositorio recibe solo los problemas más grandes: las decisiones de diseño y patrones arquitectónicos que frenan la base de código. Lo mismo se aplica cuando el usuario nombra una característica o herramienta (por ejemplo, "toda la CLI") que resulta ser un solo módulo: juégalo a escala de módulo. Pregunta de cada candidato a escala de paquete o repositorio:
- ¿Cuántos módulos, pruebas o cambios futuros pagan por ello? Un costo confinado a una función es un detalle menor a esta escala.
- ¿Arreglarlo cambiaría cómo se escribe nuevo código en este repositorio? Si la corrección son unas pocas líneas locales sin efecto en los llamadores, déjalo pasar.
- ¿Es un patrón o algo puntual? Las instancias individuales cuentan solo como evidencia de un patrón.
Ejemplos de problemas grandes: un sistema externo sin un módulo propietario, por lo que cada llamador rehace autenticación y paginación; políticas y orquestación fusionadas en los puntos de entrada, por lo que las decisiones solo se pueden probar de extremo a extremo; propiedad del estado dividida entre módulos que cada uno conoce el diseño de almacenamiento; clientes y configuración construidos en tiempo de importación, por lo que nada se puede probar sin el entorno real; un conjunto de pruebas que parchea internos por todas partes porque no existen costuras. Una función larga, una variable mal nombrada, una excepción tragada o una aserción débil no son problemas grandes por sí solos.
Estos problemas abarcan módulos, así que construye un mapa antes de leer archivos en profundidad:
- los módulos (archivos o paquetes) y aproximadamente qué hace cada uno, a partir de
git ls-files <path>y un vistazo a la parte superior de cada módulo - cómo se conectan las piezas principales: qué llama a qué, y cómo se mueven el estado y los datos entre puntos de entrada, módulos y sistemas externos
- cada sistema externo (API, base de datos, cola, formato de archivo, modelo) y cada módulo que habla directamente con él
- cada lugar donde se lee el entorno o la configuración (
os.environ,getenv,process.env, análisis de banderas) - estado mutable a nivel de módulo y efectos de tiempo de importación (clientes, conexiones, cachés, bloqueos construidos en ámbito de módulo)
- los puntos de entrada, y el directorio de pruebas con qué módulos ejercita cada archivo de prueba
grep es suficiente para la mayor parte de esto. Mantén el mapa corto; es un índice de trabajo, no parte del informe.
Si el alcance es pequeño (aproximadamente menos de 3000 líneas de código fuente), revísalo directamente desde el mapa, y en paralelo lanza dos subagentes general-purpose, dándole a cada uno la lista de archivos y las reglas de verificación del paso 4 para sus secciones. Uno obtiene solo las secciones de Tipos, Legibilidad y "Inclínate hacia datos inmutables donde valga la pena" de la rúbrica. El otro obtiene solo la sección de Pruebas y debe aplicar las comprobaciones de aserción a cada prueba. De esa manera esas secciones reciben atención completa en lugar de lo que queda después de las anteriores. Verifica lo que cada uno devuelva antes de conservarlo. De lo contrario, divídelo en áreas de módulos relacionados, cada una con sus pruebas, y asigna cada área a un subagente general-purpose, lanzado en paralelo. Dale a cada uno la ruta de la rúbrica, la ruta del vocabulario, el mapa, su lista de archivos, las reglas de verificación del paso 4 y el umbral de problemas grandes anterior, y dile que no edite nada. Un subagente que solo ve su propia área tiende a desviarse hacia detalles locales, así que pídele los patrones de diseño que ve en su área y la evidencia para cada uno (archivo:línea), no una lista de hallazgos a nivel de línea. Los problemas entre módulos provienen del mapa, así que trabájalos tú mismo, luego combínalos con los patrones de los subagentes, fusiona los que sean la misma decisión subyacente vista desde diferentes áreas, y vuelve a verificar la evidencia para cada uno antes de informar.
4. Revisar contra cada sección
Recorre los estándares en orden: Módulos, Estado, Tipos, Dependencias y configuración, Errores, Legibilidad, Pruebas. Para cada hallazgo candidato, verifícalo antes de conservarlo:
- Módulo superficial o de paso: aplica la prueba de eliminación. Nombra los llamadores y di qué complejidad reaparecería en ellos, o muestra que nada lo haría.
- Abstracción que no se ha ganado su lugar: cuenta los adaptadores en la costura. Un doble de prueba cuenta. Un adaptador sin nada oculto detrás es un hallazgo; dos es una costura real.
- Abstracción faltante: verifica la dirección opuesta con la misma intensidad. Busca una preocupación manejada en línea en tres o más lugares, dentro de un módulo o entre módulos.
- Propietario faltante / preocupación duplicada: enumera cada lugar donde está escrita la preocupación (archivo:línea), no "esto aparece en varios lugares".
- Concepto sin propietario: enumera cada lugar donde se representa, analiza o construye (archivo:línea), y cualquier firma donde parámetros del mismo tipo con significados diferentes estén uno al lado del otro. Los identificadores cuentan como conceptos, no solo datos estructurados. Nombres e IDs de diferentes tipos pasados como el mismo primitivo (un nombre de repositorio fuente y destino, un nombre de rama y una ref, dos tipos de ID) son el caso principal a capturar. Luego di qué tipo de propietario encaja. Si los llamadores mantienen y pasan el concepto, recomienda un tipo para él. Si solo necesitan que se haga algo con él, recomienda ocultarlo dentro del módulo que hace el trabajo. No predetermines un tipo de dominio.
- Restricciones de ordenamiento o claves/rutas/IDs construidas a mano: muestra al llamador haciendo la secuenciación o construcción.
- Estado mutable compartido, efectos en tiempo de carga, configuración leída en profundidad: nombra la variable global o la llamada en tiempo de importación, y la prueba o segunda configuración que bloquea.
- Escrituras instantáneas: muestra la lectura, el paso de larga duración y la reescritura.
- Datos mutables que serían mejores inmutables: repórtalo solo cuando la mutabilidad tenga un costo concreto. Muestra el segundo poseedor que ve un cambio inesperado, la caché o clave que hace insegura, o el cambio hecho lejos de donde se creó el valor. Una estructura mutable con un solo propietario que la cambia a través de su propio código no es un hallazgo. Si la versión inmutable costaría rendimiento, di qué memoizar o compartir, y dónde vive esa caché.
- Estado inválido que se puede representar: nombra el estado que nunca debe ocurrir y muestra el tipo que lo permite (campos opcionales que deben establecerse juntos, combinaciones de booleanos, una cadena con pocos valores permitidos). Enumera las guardas o comprobaciones
Noneque existen solo por culpa de él (archivo:línea). Propón la forma que lo hace imposible, y di qué guardas elimina ese cambio. - Dict o tupla como concepto: enumera las claves leídas y cada función que las lee (archivo:línea), y cada lugar donde se construye el dict. Propón el tipo de registro en el estilo existente del repositorio (un modelo pydantic o dataclass, una interfaz de TypeScript, un struct de Go, lo que sea que el repositorio ya use), que contenga solo los campos que el código usa, y di dónde se analiza la carga útil en él.
- Módulo que posee un sistema externo: verifica si sus funciones comparten una credencial, identidad, URL base o caché a través de globales del módulo o lecturas del entorno por llamada, y si las pruebas parchean varias de sus funciones. Si es así, recomienda una clase de cliente con métodos nombrados por dominio, construida al inicio y pasada, y nombra los parches que el doble reemplazaría.
- Comentarios de documentación faltantes: enumera cada función no trivial sin uno (archivo:línea) y qué necesitaría decir su comentario de documentación que la firma no dice (un fallback, un modo de error, estado que toca, una forma de entrada). Agrúpalos en un hallazgo de Legibilidad por módulo.
- Nombres de una sola letra: repórtalos como un hallazgo de Legibilidad agrupado enumerando cada uno (archivo:línea) con un nombre propuesto. Cuentan hacia el límite de Legibilidad como un hallazgo, nunca como una razón para omitirlos.
- Errores tragados: muestra el camino desde el fallo hasta un resultado que cambia mientras solo una línea de registro lo registra.
- Nombre con varios significados: búscalo con grep y enumera lo que contiene en cada sitio.
- Nombre que oculta su nivel u objeto: muestra la cadena de llamadas o sitios de llamada donde se lee de manera ambigua, y propón el reemplazo.
- Función que hace más de una cosa: intenta escribir su resumen de una oración. Si no puedes, cita la rama, el bloque try o el parámetro de modo que te lo impide.
- Función demasiado compleja: para cada candidato de la medición de complejidad, da su puntuación, luego enumera las fases que ejecuta en orden (filtrar la entrada, obtener, reclamar, validar, construir, crear, registrar, notificar, ...) con sus rangos de línea y la regla de fallo de cada fase. Propón la división como funciones nombradas, una por fase o decisión, con firmas, y di qué hace cada pieza extraída comprobable sin parchear; usualmente las decisiones se convierten en funciones puras y el original se lee como una secuencia corta de pasos nombrados. Un hallazgo por función. Por encima de 20 se clasifica con los hallazgos de diseño; de 11 a 20 es un hallazgo de Legibilidad que no cuenta hacia el límite de Legibilidad. No descartes una división porque la función sea un punto de entrada o porque el repositorio tenga otras más grandes.
- Afirmaciones de prueba: nombra el error del producto que haría fallar la afirmación. Si no puedes, ese es el hallazgo. Para casos parametrizados o basados en tablas, traza cada entrada a través del código y di qué parte de ella alcanza el comportamiento para el cual el caso está nombrado. Un caso cuya entrada distintiva es consumida por una rama anterior no cubre ese comportamiento, aunque exista una prueba para él. Asimismo, verifica que el fixture o doble realmente pueda producir la entrada que desencadenaría cada guarda o ruta de rechazo. Si nunca puede, la guarda no está probada.
- Dobles de prueba: di de qué trata la prueba, luego si el doble se sienta fuera de ese límite o reimplementa lógica del producto.
- Pruebas que parchean internos: señala la decisión que debería extraerse o la dependencia que debería pasarse, ya que esa es la corrección real.
- Costura o puerto propuesto, en la corrección de cualquier hallazgo: dale operaciones nombradas en el dominio (
close_ticket(id)), no verbos de transporte (get/post/put). Un puerto que solo reenvía solicitudes falla la prueba de eliminación, y su doble tendría que reimplementar el enrutamiento. Verifica cada costura propuesta contra la prueba de eliminación antes de informarla.
Dale a Pruebas la misma profundidad que las secciones anteriores, incluso cuando las secciones anteriores produzcan muchos hallazgos. Ejecuta cada comprobación de Pruebas anterior contra cada archivo de prueba: para cada afirmación, nombra el error del producto que la haría fallar. Un comportamiento relevante para la seguridad sin una afirmación fallida (redacción, una comprobación de autenticación, una puerta de aprobación) se clasifica con los hallazgos de diseño.
Descarta cualquier cosa que no puedas hacer concreta. Descarga detalles de estilo que los estándares no cubren. No informes errores de corrección aquí a menos que se sigan directamente de un problema de calidad (por ejemplo, una escritura instantánea que pierde datos); menciónalos en una línea y apunta a /code-review.