# FEEDBACK TO ARCHITECT — v24 > **Spec technique reviewée** : `001-histometeo-mvp.tech.v24.md` > **Spec fonctionnelle source** : `001-histometeo-mvp.md` > **Feedback intégré** : `feedback-to-architect-001-v23.md` (R3, R4, R5) > **Date review** : 2026-03-14 --- ## 1) Functional Compliance ### Acceptance Criteria hérités | AC | Statut | Justification | | -------- | ------ | ------------------------------------------------------------------------------------------------- | | AC1–AC51 | ✅ OK | Inchangés. Les 152 tests hérités (≤ v23) passent tous — aucune régression fonctionnelle détectée. | ### Acceptance Criteria v24 | AC | Statut | Justification | | ---------------------------------------------------------------------------------------------------------- | ------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | | AC52 — `TTLCache.get()` incrémente `hit_count` accessible via `snapshot()` | ✅ OK | `cache.py` L32 : `item[2] = hit_count + 1` uniquement sur cache hit (après le guard expiration). Test T60 (`test_ttlcache_hit_count`) vérifie 3 hits → `snapshot()[0]["hit_count"] == 3`. | | AC53 — `snapshot()` retourne `key`, `created_at`, `expires_at`, `hit_count` sans la valeur | ✅ OK | `cache.py` L49–58 : le dict construit ne contient que les 4 champs. Test T61 (`test_ttlcache_snapshot`) vérifie l'absence de `"value"`. | | AC54 — `set()` initialise `created_at` au timestamp courant et `hit_count` à 0 | ✅ OK | `cache.py` L39 : `[created_at, expires_at, 0, value]`. Test T60 vérifie le reset à 0 après `set()` sur clé existante. | | AC55 — `GET /admin/api/cache` retourne 5 sections avec `entries`, `total`, `max_entries`, `ttl_seconds` | ✅ OK | `main.py` L796–838 : endpoint `admin_cache()` avec helper `_section()`. Test T63 (`test_admin_cache_endpoint`) vérifie la structure complète des 5 sections. | | AC56 — Onglet "Cache" affiche Service, Clé, Mise en cache, Expiration, TTL restant, Réutilisations | ✅ OK | `admin/index.html` L412–460 : section `cache-view` avec les 6 colonnes. `admin/admin.js` L144–185 : `loadCache()` peuple le tableau avec `formatUnixTimestamp()` et `formatRemainingTtl()`. | | AC57 — Badge bleu "cache" quand `status == 'success'` ET `total_api_calls == 0` | ✅ OK | `admin/admin.js` L114–117 : condition `item.status === "success" && (item.total_api_calls ?? 0) === 0`. CSS `.status-cache` en `admin/index.html` L161–166. Test T66 (`test_search_list_cache_badge_data`). | | AC58 — `fetchWeatherForCommune()` transmet `commune` et `slug` à `/api/weather` | ✅ OK | `app.js` L1330–1340 : `params.set("commune", commune.nom)` et construction slug via `generateSlug()`. Test T67 (`test_weather_receives_commune_slug`) vérifie les valeurs en base. | | AC59 — UUID `search_id` généré via `crypto.randomUUID()` et transmis aux 3 endpoints | ✅ OK | `app.js` L1376–1384 : `createClientSearchId()` avec fallback gracieux. Propagé à `fetchWeatherForCommune`, `fetchNormals`, `fetchAnnualNormals` (params conditionnels `if (searchId)`). | | AC60 — `/api/normals` et `/api/normals/annual` acceptent `search_id` optionnel et propagent via ContextVar | ✅ OK | `main.py` L300–310 et L335–341 : param `search_id: str | None = Query(default=None)`, validation + `set_current_search_id()`+`finally: set_current_search_id(None)`. Tests T68 et T69 vérifient la propagation. | | AC61 — `search_id` non conforme ignoré silencieusement | ✅ OK | `main.py` L179–182 : `_validate_search_id()` retourne `None` si regex ne match pas. Pattern UUID v4 strict (`_UUID_V4_PATTERN`). Test T70 (`test_normals_rejects_invalid_search_id`) vérifie `search_id = NULL`. | | AC62 — `total_api_calls` inclut weather + normals avec même `search_id` | ✅ OK | Test T71 (`test_search_total_includes_normals`) : weather (1 call) + normals (3 calls) avec même UUID → `total_api_calls == 4`. Aucune modification de `tracking_service.py` nécessaire — SQL `WHERE search_id = ?`. | | AC63 — Vue détail affiche les appels weather ET normals | ✅ OK | Conséquence directe de AC60/AC62 : `get_search_detail()` exécute `WHERE search_id = ?` qui retourne désormais toutes les entrées. `admin.js` `openDetail()` itère `data.api_calls` sans filtrage par service. | --- ## 2) Contract Compliance ### Scope respecté ? ✅ **OUI** — Les fichiers modifiés dans le working tree correspondent exactement au périmètre contractuel : | Fichier | Modifié ? | Conforme au scope ? | | ------------------------- | --------- | ------------------------------------------------------------------------------------------------- | | `src/cache.py` | ✅ | ✅ Tuple → liste, `hit_count`, `created_at`, `snapshot()` | | `src/main.py` | ✅ | ✅ Endpoint `/admin/api/cache`, param `search_id` sur normals, UUID validation | | `public/app.js` | ✅ | ✅ `commune`/`slug` transmis, `createClientSearchId()`, `generateSlug()`, propagation `search_id` | | `admin/admin.js` | ✅ | ✅ Badge "cache", onglet Cache, `loadCache()`, formatage timestamps | | `admin/index.html` | ✅ | ✅ CSS `.status-cache`, section `cache-view`, bouton tab | | `tests/test_cache.py` | ✅ | ✅ T60–T62 (hit_count, snapshot, expired exclusion) | | `tests/test_admin_api.py` | ✅ | ✅ T63–T66 (endpoint cache, auth, entries, badge data) | | `tests/test_api.py` | ✅ | ✅ T67–T71 (commune/slug, search_id propagation, validation, total) | ### Forbidden changes respectés ? ✅ **OUI** — Vérification exhaustive via `git diff --name-only` : - `src/weather_service.py` — **aucune modification** - `src/commune_service.py` — **aucune modification** - `src/normals_service.py` — **aucune modification** - `src/tracking_service.py` — **aucune modification** - `src/config.py` — **aucune modification** - `src/og_service.py` — **aucune modification** - `src/prefetch_service.py` — **aucune modification** - `public/index.html` — **aucune modification** - `public/style.css` — **aucune modification** - Tests existants hérités (≤ v23, 152 tests) — **tous passent sans modification sémantique** ### Invariants préservés ? | Invariant | Statut | Note | | ------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------ | | INV-1 à INV-47 | ✅ | Aucune régression — confirmé par la suite de 152 tests hérités | | INV-48 — `get()` incrémente hit_count uniquement sur cache hit | ✅ | Le guard `if expires_at <= now` + `return None` précède l'incrémentation. Entrées absentes retournent `None` sans effet. | | INV-49 — `snapshot()` ne retourne jamais les valeurs | ✅ | Le dict construit exclut `_value`. Assertion `"value" not in item` dans T61. | | INV-50 — `set()` remet `hit_count` à 0 | ✅ | `[created_at, expires_at, 0, value]`. Vérifié par T60 (`snap_after_reset[0]["hit_count"] == 0`). | | INV-51 — `search_id` optionnel sur normals | ✅ | `Query(default=None)` sur les deux endpoints. Comportement identique à v23 si absent. | | INV-52 — Validation UUID v4 strict côté serveur | ✅ | `_UUID_V4_PATTERN` vérifie la position du `4` (version) et `[89ab]` (variant). `re.IGNORECASE` pour casse mixte. | | INV-53 — Badge "cache" seulement si `status === 'success'` ET `total_api_calls === 0` | ✅ | Condition exacte dans `admin.js` L114–117 avec `?? 0` pour null safety. | | INV-54 — `/admin/api/cache` protégé par auth HTTP Basic | ✅ | Route enregistrée sur `admin_router` qui a `dependencies=[Depends(verify_admin)]`. Test T64. | | INV-55 — `snapshot()` capte un instantané sans garantie transactionnelle | ✅ | Le lock `self._lock` est bref (O(n), n ≤ 500 entrées). Cohérent avec la section risques R3 de la spec. | ### Done When | # | Critère | Statut | Vérification | | ------- | ------------------------------------------------------------- | ------ | ---------------------------------------------------------------------- | | D33–D73 | 152 tests hérités passent | ✅ | 164 tests passent au total (152 hérités + 12 nouveaux) | | D74 | `snapshot()` retourne key, created_at, expires_at, hit_count | ✅ | T61 | | D75 | `get()` incrémente `hit_count` | ✅ | T60 | | D76 | `GET /admin/api/cache` — 5 caches | ✅ | T63 + T65 | | D77 | Onglet Cache avec tableau 6 colonnes | ✅ | HTML + JS vérifiés | | D78 | Badge "cache" dans la liste | ✅ | Condition JS + CSS + T66 | | D79 | `commune` et `slug` transmis à `/api/weather` | ✅ | T67 | | D80 | UUID `search_id` avant chaque recherche | ✅ | `createClientSearchId()` + propagation à 3 endpoints | | D81 | `/api/normals` et `/api/normals/annual` acceptent `search_id` | ✅ | T68 + T69 | | D82 | `total_api_calls` inclut weather + normals | ✅ | T71 (asserté `== 4`) | | D83 | Vue détail affiche weather ET normals | ✅ | SQL `WHERE search_id = ?` dans `get_search_detail()` capture les deux. | | D84 | Tous les tests passent | ✅ | 164 passed, 0 failed | --- ## 3) Technical Quality ### Complexité inutile ? **Non.** L'implémentation est minimale et suit fidèlement la spec : - `TTLCache` : le changement tuple → liste est la solution la plus simple pour la mutabilité de `hit_count`. Pas de dataclass, pas de NamedTuple — cohérent avec le style existant. - `_validate_search_id()` : une seule fonction regex, 4 lignes. Pas de surcharge. - `_section()` : helper local dans l'endpoint cache — pas de nouvelle abstraction inutile. - `createClientSearchId()` : fallback `undefined` sans polyfill UUID — exactement comme prescrit. ### Dette technique introduite ? **Non.** Aucune dette nouvelle identifiée. ### Duplication ? **Non.** Le pattern `validated_id = _validate_search_id(search_id)` + `set_current_search_id()` + `finally` apparaît sur `/api/normals` et `/api/normals/annual`, mais c'est le même pattern déjà utilisé dans `/api/weather`. Factoriser serait de la sur-ingénierie pour 3 occurrences. ### Incohérences ? **Non.** --- ## 4) Test Coverage ### Tests suffisants ? ✅ **OUI** — 12 nouveaux tests couvrent les 3 axes v24 : | Test | Fichier | Vérifie | | ---- | ------------------- | -------------------------------------------------- | | T60 | `test_cache.py` | `hit_count` incrémentés + reset à 0 par `set()` | | T61 | `test_cache.py` | `snapshot()` structure correcte, sans valeur | | T62 | `test_cache.py` | Entrées expirées exclues du snapshot | | T63 | `test_admin_api.py` | Endpoint `/admin/api/cache` — structure 5 sections | | T64 | `test_admin_api.py` | Auth required (401) | | T65 | `test_admin_api.py` | Cache reflète l'entrée après un appel weather | | T66 | `test_admin_api.py` | Données compatibles badge cache | | T67 | `test_api.py` | commune/slug transmis et stockés en base | | T68 | `test_api.py` | `/api/normals` propage search_id | | T69 | `test_api.py` | `/api/normals/annual` propage search_id | | T70 | `test_api.py` | UUID invalide → `search_id = NULL` | | T71 | `test_api.py` | total_api_calls = weather + normals (4) | ### Edge cases oubliés ? **Aucun edge case majeur oublié.** Les cas suivants sont couverts implicitement ou explicitement : - Cache vide → T63 retourne `total: 0` par défaut - UUID majuscules → `re.IGNORECASE` dans le pattern - Navigateur sans `crypto.randomUUID()` → fallback `undefined`, pas de `search_id` envoyé - Snapshot après expiration → T62 - `hit_count` reset après `set()` → T60 --- ## 5) UX Consistency Check ### Incohérences flagrantes ? **Non.** L'interface admin est cohérente : - Le badge "cache" utilise un bleu doux (`#0e6ba8` / `#e8f4fd`) distinct des badges success/error/pending - L'onglet Cache s'intègre dans la navigation existante (Récentes / Détail / Synthèse / **Cache**) - Le bouton "Rafraîchir" dans l'onglet Cache est positionné en toolbar en haut à droite - Les timestamps sont convertis en dates locales lisibles (`toLocaleString()`) - Le TTL restant est affiché en format humain (`2h 15min`) ### Comportement inattendu ? **Non.** ### Friction évidente ? **Non.** --- ## 6) Required Corrections **Aucune correction obligatoire.** --- ## 7) Recommended Improvements (non bloquantes) | # | Suggestion | Priorité | | --- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | | R1 | **Town page search_id orphelin** — `loadTownPage()` crée un `searchId` transmis à `fetchAnnualNormals()`, mais aucun appel `/api/weather` ne crée de `search_logs` correspondant. Les `api_call_logs` auront un `search_id` pointant vers rien. Impact nul mais inutile. Suggestion : ne pas passer de `searchId` dans le cas town page. | Basse | | R2 | **`data/tracking.db` tracké dans git** — Ce fichier binaire apparaît dans les diffs. Considérer l'ajout dans `.gitignore`. Problème pré-existant, pas introduit par v24. | Basse | --- ## 🧭 Décision finale ### ✅ Validé L'implémentation v24 est **conforme** à la spec technique dans ses 3 axes : 1. **Observabilité cache** — `TTLCache` augmenté avec `hit_count`, `created_at`, `snapshot()`. Endpoint admin `/admin/api/cache` protégé. Onglet Cache fonctionnel dans l'admin. 2. **Badge "cache"** — Indicateur visuel correct (condition `status === 'success' && total_api_calls === 0`), cohérent avec le design system existant. 3. **Attribution API SPA** — R3 (commune/slug transmis), R4 (search_id propagé via UUID client), R5 (vue détail complète) sont tous résolus. Le `total_api_calls` reflète désormais le coût réel (weather + normals). **Aucune correction obligatoire.** Les 164 tests passent (152 hérités + 12 nouveaux). Le scope est strictement respecté, aucun fichier interdit n'a été modifié, tous les invariants sont préservés.