# FEEDBACK TO ARCHITECT — v22 > **Spec technique** : `001-histometeo-mvp.tech.v22.md` > **Spec fonctionnelle** : `001-histometeo-mvp.md` > **Date review** : 2026-03-14 > **Reviewer** : Reviewer Technique & Qualité --- ## 1) Functional Compliance | AC | Statut | Justification | | --------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | AC1–AC30 | OK | Inchangés par v22, tous les tests hérités passent (133 tests). Comportement observable des routes utilisateur identique. | | AC31–AC32 | OK | Hérités de v21, non impactés. | | AC33 | OK | Chaque appel HTTP réel (weather, normals, communes) crée exactement 1 entrée dans `api_call_logs` avec le champ `service` correspondant. Vérifié dans `weather_service.py` (L107, `service="weather"`), `normals_service.py` `_fetch_chunk()` (L293, `service="normals"`), `commune_service.py` `search_communes()` (L73, `service="communes"`). | | AC34 | OK | Hérité de v21, non impacté. | | AC35 | OK | Un cache hit ne crée aucune entrée dans `api_call_logs`. Vérifié : `weather_service.py` L85–86 (return cached, aucun tracking), `normals_service.py` `get_normals()` / `get_annual_normals()` (aucun tracker call — le tracking est dans `_fetch_chunk` qui n'est pas appelé en cache hit), `commune_service.py` L57–58 (return cached). Test `test_cache_hit_creates_no_api_call` le confirme. | | AC36–AC42 | OK | Hérités de v21, non impactés. | | AC43 | OK | Un appel `get_normals()` en cache miss déclenche `_fetch_reference_normals()` → 3 appels `_fetch_chunk()` → 3 entrées `api_call_logs` avec `service="normals"`. Test `test_normals_cache_miss_creates_3_api_calls` le confirme. | | AC44 | OK | `search_communes()` en cache miss dans un contexte de recherche crée 1 entrée avec `service="communes"` et `provider="geo-api-gouv"`. Test `test_commune_resolve_creates_api_call` le confirme. Guard conditionnel `if tracker and search_id` respecte INV-44. | | AC45 | OK | `get_dashboard()` retourne `api_calls_today` sous forme `{"total": N, "by_service": {"weather": X, "normals": Y, "communes": Z}}`. Vérifié dans `tracking_service.py` L426–429, `admin.js` L163–170. Tests `test_dashboard_api_calls_by_service` et `test_dashboard_returns_by_service` le confirment. | | AC46 | OK | `escapeHtml()` définie en `admin.js` L43–50. Appliquée systématiquement sur toutes les valeurs de données injectées via `innerHTML` : commune, slug, dates, error_message, params_summary, endpoint, status_code, duration_ms, search_type, count, id. Vérifié exhaustivement dans les fonctions `loadSearches()`, `openDetail()`, `loadDashboard()`. Le `statusBadge()` utilise `normalizeStatus()` (whitelist) + `escapeHtml()`. Le `detail-summary` utilise `textContent` (safe). | --- ## 2) Contract Compliance ### Scope respecté ? **OUI.** Fichiers modifiés (git diff) : - `src/config.py` — ajout `GEO_API_PROVIDER` ✅ - `src/weather_service.py` — suppression tracking cache hit, ajout `service="weather"` ✅ - `src/normals_service.py` — tracking déplacé vers `_fetch_chunk()`, ajout `service="normals"` ✅ - `src/commune_service.py` — ajout tracking conditionnel, `service="communes"` ✅ - `src/tracking_service.py` — migration schéma, `_count_api_calls`, `log_api_call` signature, dashboard ventilé ✅ - `admin/index.html` — retrait `data-sort="status"`, masquage onglet Détail, cards ventilées ✅ - `admin/admin.js` — `escapeHtml()`, affichage ventilé, masquage onglet Détail ✅ - `tests/test_tracking_service.py` — ajout T41–T45, ajustements minimaux tests existants ✅ - `tests/test_admin_api.py` — ajout T46–T50 ✅ - `README.md` — mise à jour documentation (hors scope strict mais non interdit, acceptable) ✅ ### Forbidden changes respectés ? **OUI.** - `public/app.js`, `public/index.html`, `public/style.css` : non modifiés ✅ - `src/cache.py`, `src/prefetch_service.py`, `src/og_service.py` : non modifiés ✅ - Tests ≤ v21 (133 tests) : tous passent. Les modifications aux tests existants se limitent à (a) l'ajout du paramètre `service="weather"` requis par la nouvelle signature de `log_api_call()`, et (b) l'ajustement d'une assertion dans `test_get_dashboard` reflétant le changement de type de `api_calls_today` (int → dict) et de comptage (excluant les cache hits). Ce sont des adaptations mécaniques, pas des modifications sémantiques. ✅ - Comportement observable des routes utilisateur : inchangé ✅ ### Invariants préservés ? | INV | Statut | | -------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | | INV-1 à INV-40 | OK — non impactés, tests passent | | INV-41 | OK — `_count_api_calls` filtre `cache_status = 'miss'` (L179) | | INV-42 | OK — chaque appel HTTP individuel = 1 entrée. Chunks normals = 3 entrées distinctes. | | INV-43 | OK — `service` est `"weather"`, `"normals"` ou `"communes"` selon le service appelant. | | INV-44 | OK — `commune_service.py` utilise `if tracker and search_id:` (L72, L88), non-propagation d'exception (wrap dans try/except hérité du tracking_service). | --- ## 3) Technical Quality - **Complexité inutile** : NON. Les changements sont minimaux et ciblés. La logique de tracking dans `_fetch_chunk()` est le placement correct (1 fetch HTTP = 1 log). - **Dette technique introduite** : NON. Le code est cohérent avec l'existant. - **Duplication** : Légère duplication du pattern tracking (try/except, `if tracker and search_id`) entre `weather_service.py`, `normals_service.py`, et `commune_service.py`. C'est acceptable — c'est le pattern existant depuis v21, et l'abstraire serait de la sur-ingénierie. - **Incohérences** : NON. - **Sécurité** : L'échappement XSS est correctement implémenté (`escapeHtml()` couvre `&`, `<`, `>`, `"`, `'`). Toutes les requêtes SQL utilisent des paramètres liés (pas d'injection). Le `normalizeStatus()` whitelist les valeurs CSS injectées. - **Migration schéma** : Le `ALTER TABLE` conditionnel (try/except) est simple et efficace pour le cas SQLite en phase beta. L'index `idx_api_service` est créé. --- ## 4) Test Coverage - **Tests suffisants** : OUI. 143 tests (133 hérités + 10 nouveaux). - T41 `test_count_api_calls_excludes_cache_hits` ✅ - T42 `test_log_api_call_with_service` ✅ - T43 `test_dashboard_api_calls_by_service` ✅ - T44 `test_dashboard_cache_hit_ratio_new_formula` ✅ - T45 `test_migration_adds_service_column` ✅ - T46 `test_seo_route_creates_search_log` ✅ - T47 `test_normals_cache_miss_creates_3_api_calls` ✅ - T48 `test_commune_resolve_creates_api_call` ✅ - T49 `test_cache_hit_creates_no_api_call` ✅ - T50 `test_dashboard_returns_by_service` ✅ - **Edge cases oubliés** : Aucun manque critique identifié. Couverture des scénarios principaux (cache miss, cache hit, ventilation par service, migration, SEO tracking). --- ## 5) UX Consistency Check - **Onglet Détail masqué par défaut** (`hidden` attribute) : cohérent, évite un onglet vide au chargement initial ✅ - **Onglet Détail visible après sélection** (`detailTab.hidden = false` dans `openDetail()`) : cohérent ✅ - **Cards ventilées dans le dashboard** (Total + Weather + Normals + Communes) : affichage clair et logique ✅ - **Retrait du tri sur Statut** : cohérent (le tri backend ne supportait pas cette colonne) ✅ - **Pas d'incohérences flagrantes ni de friction évidente.** ✅ --- ## 6) Required Corrections **Aucune correction obligatoire.** --- ## 7) Recommended Improvements (non bloquantes) 1. **R1 — `README.md` hors scope** : Le fichier `README.md` a été modifié mais n'est pas dans la liste des fichiers modifiables de la spec technique. Les changements sont purement documentaires et cohérents, mais pour une conformité stricte au contrat, le README devrait être soit ajouté au scope, soit mis à jour dans une étape séparée. 2. **R2 — Appels API non trackés hors contexte de recherche (pré-existant, hors scope v22)** : L'analyse approfondie de TOUTES les routes de `main.py` révèle que **4 endpoints font des appels HTTP externes sans contexte de tracking** (`search_id` absent) : | Route | API externe appelée | Tracké ? | Risque quota | | ---------------------------------------- | ---------------------------- | -------- | -------------------------------- | | `GET /api/normals` | Open-Meteo (×3 chunks) | ❌ NON | **Élevé** — même API que weather | | `GET /api/normals/annual` | Open-Meteo (×3 chunks) | ❌ NON | **Élevé** — même API que weather | | `GET /api/resolve/{slug}` | geo.api.gouv.fr | ❌ NON | Faible — API gratuite | | `GET /api/og-image/{slug}/{start}/{end}` | geo.api.gouv.fr + Open-Meteo | ❌ NON | **Moyen** — weather + commune | **Impact** : quand un utilisateur navigue via le frontend SPA (non-SEO), la séquence typique est : - JS appelle `/api/weather` → **tracké** (1 appel Open-Meteo weather) - JS appelle `/api/normals` → **NON tracké** (3 appels Open-Meteo normals) → Le dashboard sous-estime la consommation réelle du quota Open-Meteo d'environ **75%** pour les recherches SPA avec normales (3 appels normals non comptés sur 4 total). En revanche, les routes SEO (`/ville/`, `/meteo/`) trackent correctement TOUS les appels (weather + normals + communes) car le `prefetch_*` s'exécute dans le contexte du `search_id`. **Ceci n'est PAS une régression v22** — ces routes n'ont jamais eu de tracking, même en v21. Mais l'objectif déclaré de v22 étant « mesurer la marge par rapport aux quotas des APIs externes », cette lacune pré-existante mérite d'être adressée dans une version future. **Note sur le cas observé (`/ville/lyon-69` → 0 appels API)** : ce comportement est **correct**. Les données affichées proviennent du cache en mémoire (TTLCache) alimenté par la recherche « period » précédente (5 appels API, 1500ms). Les clés de cache normals sont identiques (`normals:lat:lon`) entre `get_normals()` et `get_annual_normals()`, donc un prefetch period antérieur couvre la page ville. --- ## Décision finale ## ✅ Validé L'implémentation v22 est conforme à la spec technique `001-histometeo-mvp.tech.v22.md` et à la spec fonctionnelle. Tous les Acceptance Criteria (AC33–AC46) sont satisfaits. Les 143 tests passent (133 hérités + 10 nouveaux). Les invariants sont préservés. Les corrections de qualité demandées (XSS, tri statut, masquage onglet Détail) sont correctement intégrées. Le comptage API granulaire par service est fidèle aux appels HTTP réels dans les routes SEO et `/api/weather`. **Point d'attention (R2)** : les routes SPA `/api/normals`, `/api/normals/annual`, `/api/resolve/{slug}` et `/api/og-image/` effectuent des appels HTTP externes non trackés. Ce n'est pas une régression v22, mais un angle mort pré-existant qui impacte la fiabilité du comptage de quota Open-Meteo. À traiter dans une version future (v23+).