Skip to content

feat(i18n): persistir el idioma de la interfaz e inventario reproducible de textos (L1 #66) - #152

Merged
egdev6 merged 5 commits into
egdev6:mainfrom
danielgap:feat/i18n-locale
Oct 20, 2026
Merged

egdev6 merged 5 commits into
egdev6:mainfrom
danielgap:feat/i18n-locale

Conversation

@danielgap

Copy link
Copy Markdown
Contributor

Related issue / Issue relacionada

Refs #66 (la issue sigue abierta hasta L5).

ODD: fase L1 — locale persistido, con las decisiones A y B de L0 aprobadas en #66 (comment).

Scope / Alcance

1. locale: "es" | "en" en AppConfig (src-tauri/src/config.rs, src/lib/types.ts)

Entrada Resultado
Arranque limpio (sin config.json) es
Config antigua sin la clave es; el resto de la config no cambia
"en", "EN", " en " en (se recortan espacios y se pasa a minúsculas una sola vez)
"fr", "en-US", "es_ES", "", " " es
null o tipo no string (42, true, ["en"], {…}) es, conservando el resto de la config

Sobre el matiz 2: con un locale: String normal, un null o un tipo inesperado hacía que serde rechazara todo config.json, y load() volvía a los valores por defecto (cuentas, dock y avisos perdidos). Un deserializador tolerante lee esos casos como vacío y sanitized() los lleva a es. No hay migración: las configs actuales cargan igual. El frontend ya reenvía la config completa en save_config, así que el campo se conserva al guardar desde Ajustes.

2. Inventario reproducible (scripts/i18n-inventory.mjs → docs/i18n-inventory.md)

Sobre el matiz 1: en vez de adjuntar una lista hecha a mano, el inventario se genera desde un commit de git, así que cada path:line se puede contrastar con ese commit.

  • React/TS se parsea con el compilador de TypeScript.
  • Rust pasa por un scanner léxico que ignora comentarios, atributos, macros de log y #[cfg(test)].
  • Lo que entra se decide sobre todo por contexto (texto JSX, atributos visibles, unidades de meters), no por la forma del texto.
  • node scripts/i18n-inventory.mjs --check lo regenera en memoria y falla si el documento está desactualizado. Funciona igual con checkouts CRLF.

Corrección de cifras de L0. Al preparar este PR comprobé el inventario que resumí en #66 y sus referencias de línea no eran fiables (muchas desplazadas varias líneas). Por eso lo rehíce con el script. Cifras nuevas: 803 textos distintos / 984 sitios (React 553/651, Rust 279/333), frente a los ~700/~850 que publiqué. Es un superconjunto deliberado:

  • Incluye las claves de lookup de format.ts ('Semana · todos'…), que son justo el emparejamiento por literal que hay que eliminar.
  • Incluye nombres de marca o plan que no se traducirán.
  • Faltante conocido y documentado: la unidad "req" dentro de una tupla en providers/nan.rs.

3. Plurales (matiz 3) — verificados en main @ 3864613. Los dos casos:

  • src-tauri/src/providers/nine_router.rs:426 — format!("{providers} proveedor{}", if providers == 1 { "" } else { "es" })
  • src/lib/reset-credits.ts:9 — count === 1 ? '1 reset disponible' : \${count} resets disponibles``

Además src/Settings.tsx:1538 (` · ${n + 1} cuentas`) solo aparece con ≥ 2 cuentas, así que no tiene rama singular.

En inglés también son 2 formas (one/other), así que basta con ramificar en el catálogo sin Intl.PluralRules.

Fuera de alcance: catálogos, selector en Ajustes, ids estables Rust→JS y cualquier texto visible (L2/L3). Sobre el matiz 4, es.ts saldrá de los literales actuales listados en el inventario, sin retraducir.

Archivo Cambio
src-tauri/src/config.rs LOCALES, LOCALE_DEFAULT, deserializador tolerante, saneo y 6 tests
src/lib/types.ts Locale y AppConfig.locale
scripts/i18n-inventory.mjs Generador y --check
scripts/i18n-inventory.test.mjs 11 tests (heurística, TS, Rust, CLI y drift, CRLF)
docs/i18n-inventory.md Inventario generado
package.json Añade los tests del generador a npm test

Coordination / Coordinación

  • Para ODD: asignaciones y solapamientos revisados; toma verificada o acuerdo explícito del mantenedor enlazado.

Contribuidor: @danielgap
Evidencia de asignación o acuerdo: #66 asignada a @danielgap ("Adelante! toda tuya!") y L0 aprobado en #66 (comment)

Acceptance and validation / Aceptación y validación

  • Criterios cumplidos: locale persistido con tests primero para es, en, ausente, inválido, null o vacío, y recarga que conserva los datos (S2/S5 de feat: interfaz bilingüe ES/EN con español por defecto #66). Inventario path:line verificable con --check.
  • Comprobaciones ejecutadas y resultados (Windows 10):
    • cargo test: 230/230. Los 6 tests nuevos de config::tests fallaron antes de implementar, con locale en Null.
    • npx vitest run --dir src: 591/591.
    • npx tsc --noEmit: OK.
    • node --test scripts/i18n-inventory.test.mjs: 11/11.
    • node scripts/i18n-inventory.mjs --check: exit 0, también con CRLF.
    • Contraste de líneas: 25/25 filas al azar en 20 archivos coinciden.
  • Comprobaciones no ejecutadas y motivo:
    • npm test completo en Windows: 216/220 en los scripts de Node. Los 4 fallos son EPERM al crear symlinks en setup-signing y provision-tools, sin privilegio de symlink en Windows; ya fallaban en un árbol limpio y no tienen relación con este cambio.
    • Sin prueba manual de la app: L1 no cambia nada visible.
  • Bloqueos pendientes: ninguno para L1.

Add `locale: "es" | "en"` to AppConfig as the first L1 step of the
bilingual UI. Missing keys and clean starts default to `es`; values are
trimmed and lowercased once, and anything outside es|en sanitizes to es.

A null or non-string `locale` is read leniently so it can no longer make
serde reject the whole config.json and drop the user's settings back to
defaults.
…ev6#66)

Generate docs/i18n-inventory.md from a git commit instead of keeping a
hand-made list, so every path:line can be checked against that commit.
React/TS is parsed with the TypeScript compiler; Rust with a lexical
scanner that skips comments, attributes, log macros and #[cfg(test)]
items. Inclusion is decided mostly by context (JSX text, visible
attributes, meter units) rather than by the shape of the string.

`--check` regenerates in memory and fails when the document is stale.
The generator tests run as part of `npm test`.
Copilot AI balanced review requested due to automatic review settings October 20, 2026 08:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

egdev6
egdev6 previously approved these changes Oct 20, 2026

@egdev6 egdev6 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@egdev6 @danielgap — revisé el PR completo contra main @ 3864613. El cambio es sólido: verifiqué que el bug del deserializador tolerante es real (load() en config.rs:554 hace .ok() sobre from_str, así que un locale de tipo inesperado tiraba todo config.json a Default), los 6 tests de config.rs cubren la matriz completa incluyendo roundtrip con preservación de datos, los plurales declarados coinciden con el código (nine_router.rs:426, reset-credits.ts:9, Settings.tsx:1538), y el inventario reproducible cumple la condición path:line de la revisión de L0. También destaco la corrección honesta de las cifras de L0 (803/984 vs ~700/850) con el superconjunto y el gap conocido documentados.

Aprobado, con la petición de resolver estas observaciones menores antes del merge:

  1. Regenerar el inventario justo antes de fusionar. El doc está generado desde 541fc6e (último commit del branch tocando src/), no desde main. Al fusionar, el "último commit tocando src" cambia y --check podría marcar drift en el primer re-run. Regenerar el doc en la merge (o en un commit final del branch) para que el documento quede anclado a un commit verificable desde main.

  2. Test de paridad del contrato de locales. LOCALES/LOCALE_DEFAULT (Rust) y Locale/'es' (TS) duplican el contrato sin nada que los mantenga sincronizados. Para L1 es aceptable como está, pero pediría al menos un comentario cruzado en ambos lados apuntando al otro (Rust ← types.ts ya lo tiene; falta el inverso en config.rs), y dejar apuntado cubrir la paridad en L2 con los catálogos.

  3. Enganchar --check a CI. Ahora el drift del inventario solo se detecta en npm test local. Bastaría añadir node scripts/i18n-inventory.mjs --check al job de CI existente (o dejarlo dentro de npm test, que ya corre en CI — en ese caso basta con confirmarlo aquí).

Ninguna es bloqueante por sí misma; la 1 es la única con efecto real post-merge. Con eso resuelto y las otras 2 anotadas, listo para merge y para arrancar L2.

…ev6#66)

The header recorded the last commit touching src/, so a squash or rebase
merge would change that id and make `--check` fail with no source change.
Record the content-addressed trees of src/ and src-tauri/src/ instead;
they survive history rewrites and are checkable with
`git rev-parse <commit>:src`.
@danielgap

Copy link
Copy Markdown
Contributor Author

@egdev6 gracias por la revisión. Ya están resueltas las tres, más el conflicto con main:

Conflicto. Integré main (0.7.0, #153) con un merge, sin reescribir historia. El único choque era la línea "test" de package.json: me quedé con la de main y volví a añadir el test del generador.

1. Inventario anclado. Tenías razón, y el problema era algo mayor: si la PR entra con squash o rebase, el commit de la cabecera deja de existir en main y --check fallaría siempre, aunque no cambie ni una línea. Regenerar justo antes del merge no lo evitaba. Ahora la cabecera ancla el documento a los árboles de src/ y src-tauri/src/, que dependen solo del contenido y sobreviven a cualquier forma de fusionar:

src/           = 603840f0f5d56ecb2320415ef89a1e9524d0a5f7
src-tauri/src/ = ec5f5eae6ba38b362b1e9d736f46490fb5106496

Se comprueba con git rev-parse <commit>:src. Hay un test que reescribe el commit sin cambiar el contenido y exige que --check siga pasando. Las filas no cambiaron (803 / 984).

2. Contrato de locales. Añadí la referencia inversa en config.rs (LOCALES → src/lib/types.ts › Locale) y anoté que la paridad entrará con test junto a los catálogos de L2.

3. --check en CI. Una aclaración antes de decidir: npm test no ejecuta --check sobre el repo, solo los tests unitarios del generador en repos temporales. Si lo engancho a CI, cualquier PR que mueva una línea de src/ fallará hasta regenerar el inventario. Lo comprobé aquí mismo: el comentario de 2 líneas del punto 2 ya lo desactualizó. Esa carga recaería en todos los contribuidores, no solo en i18n.

Mi propuesta es dejarlo fuera de CI hasta L5. Cada PR de i18n lo regenera y lo verifica con --check, como esta, y en L5 el inventario se congela como evidencia o se retira. Si prefieres el chequeo en CI igualmente (bloqueante, o solo como aviso), lo añado en esta PR. La decisión es tuya.

Comprobaciones tras los cambios: cargo test 230/230, vitest 591/591, generador 11/11, --check OK.

@egdev6 egdev6 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@egdev6 @danielgap — re-revisé los commits 3ee9595 y 7fe3717: las tres observaciones menores quedaron resueltas, y la del inventario con una solución mejor que la que propuse.

  1. Inventario anclado a árboles (punto 1): en vez de regenerar en la merge, el generador ahora ancla el doc a los árboles content-addressed (git rev-parse <sha>:src / :src-tauri/src). El header dice "from the source trees src/ = 603840f…", así que el --check es inmune a commits que no toquen src/ — el drift post-merge desaparece de raíz, no solo se gestiona. El test de drift de i18n-inventory.test.mjs cubre el comportamiento.
  2. Contrato cruzado (punto 2): config.rs ahora referencia types.ts › Locale con la nota de que la paridad se testeará junto a los catálogos de L2. Queda apuntado donde toca.
  3. --check en CI (punto 3): confirmado — npm test incluye el test de drift y corre en CI.

Con esto el PR queda aprobado sin reservas: el deserializador tolerante resuelve el bug real de pérdida de config.json, los 6 tests cubren la matriz completa con roundtrip, los plurales están verificados y el inventario cumple el compromiso path:line de L0 — con la corrección honesta de cifras incluida.

Gracias por la calidad del trabajo, @danielgap: es exactamente el tipo de contribución que hace fácil mantener el proyecto — tests primero, alcance declarado, y hasta la autocorrección de las cifras de L0 documentada en lugar de oculta. Listo para merge, y con esto L2 (catálogos) puede arrancar con la base bien puesta. 👏

@danielgap

Copy link
Copy Markdown
Contributor Author

@egdev6 ¡gracias! Una sola aclaración sobre el punto 3, para que no quede una garantía que no existe: npm test ejecuta los tests unitarios del generador (en repos temporales), pero no ejecuta --check sobre docs/i18n-inventory.md. Hoy CI no detecta si el inventario del repo se desactualiza. La propuesta sigue siendo que cada PR de i18n lo regenere hasta L5; si prefieres el chequeo en CI, lo añado en un PR aparte sin bloquear este merge.

@egdev6

egdev6 commented Oct 20, 2026

Copy link
Copy Markdown
Owner

@egdev6 @danielgap — tienes toda la razón y mi afirmación anterior era incorrecta: los tests de i18n-inventory.test.mjs validan el generador y su --check en repos temporales, nunca el docs/i18n-inventory.md real del repo. Tal como está, CI no detecta un inventario desactualizado. Gracias por la aclaración — mejor dejar la garantía real escrita que una revisión que promete de más.

Propuesta aceptada: abrimos tarea específica y el merge de este PR no se bloquea → #155 recoge el paso node scripts/i18n-inventory.mjs --check en el job de CI actual (un paso, sin workflow nuevo), con la convención de regeneración documentada. Mientras tanto, tu criterio de que cada PR de i18n regenere el inventario sigue aplicando hasta que #155 lo automatice.

Con esto el PR queda listo para merge sin pendientes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants