# FEEDBACK TO ARCHITECT — 001-histometeo-mvp **Date** : 2025-03-11 **Reviewer** : Reviewer Technique & Qualité **Sources** : `001-histometeo-mvp.md` (spec fonctionnelle), `001-histometeo-mvp.tech.md` (spec technique), code implémenté --- ## 1) Functional Compliance | AC | Statut | Justification | | ---- | ------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | AC1 | ⚠️ KO partiel | L'auto-complétion fonctionne, mais les **tests unitaires du service commune sont SKIPPED** (7/20 tests non exécutés) → non vérifiable automatiquement. | | AC2 | ✅ OK | Le flow complet commune → dates → tableau fonctionne côté intégration. | | AC3 | ❌ KO | Les descriptions WMO sont **sans accents** ("Ciel degage" au lieu de "Ciel dégagé", "Bruine legere" au lieu de "Bruine légère"). INV-5 violé (interface intégralement en français). De plus, **typo** : "Brouillard givrante" au lieu de "Brouillard givrant" (spec §3.6). | | AC4 | ❌ KO | **Bug critique dans `formatDisplayDateTime`** (`app.js` L97) : l'offset UTC est hardcodé à `+01:00` (CET hiver). En été (CEST), Paris est à `+02:00`. Toutes les heures d'été sont décalées d'1 heure. INV-2 violé. | | AC5 | ✅ OK | Validation période > 31 jours fonctionnelle (test passe). | | AC6 | ✅ OK | Validation date future fonctionnelle (test passe). | | AC7 | ✅ OK | Validation date < 1940 fonctionnelle (test passe). | | AC8 | ⚠️ KO partiel | Messages d'erreur présents mais **sans accents** ("Le parametre de recherche…", "Reessayez dans quelques instants."). La spec §3.5 montre des messages avec accents ("Le paramètre de recherche…", "Réessayez…"). INV-5 violé. | | AC9 | ✅ OK | Note de transparence visible en permanence, contenu conforme. | | AC10 | ✅ OK | CSS responsive mobile-first, breakpoint 768px, table scrollable. Conforme. | | AC11 | ✅ OK | Aucune inscription requise. | --- ## 2) Contract Compliance ### Scope respecté ? ✅ Oui — Seuls les fichiers autorisés ont été créés/modifiés (`public/`, `src/`, `tests/`, fichiers racine). ### Forbidden changes respectés ? ✅ Oui — `docs/`, `.github/`, `README.md` non modifiés par l'implémentation. ### Invariants préservés ? | Invariant | Statut | Détail | | --------- | ------ | --------------------------------------------------------------------------------------------------------- | | INV-1 | ✅ OK | Aucune persistance (pas de DB, pas de localStorage). | | INV-2 | ❌ KO | Bug timezone `+01:00` hardcodé dans `app.js`. Les heures d'été sont fausses. | | INV-3 | ✅ OK | Limite 31 jours validée côté back et front. | | INV-4 | ✅ OK | Aucune clé API requise. | | INV-5 | ❌ KO | Tous les accents français sont absents : descriptions WMO, messages d'erreur backend, messages front-end. | | INV-6 | ✅ OK | Page unique, pas de routeur. | --- ## 3) Technical Quality ### Complexité inutile ? Non. L'architecture est sobre et conforme au design minimal proposé. ### Dette technique introduite ? Oui, plusieurs éléments : 1. **Faille XSS dans `renderRows()`** (`app.js` L179-187) — Les données API sont injectées via `tr.innerHTML` sans échappement. Si l'API externe (Open-Meteo ou geo.api.gouv.fr) retournait du contenu malveillant dans un champ texte, il y aurait une injection XSS. Il faut utiliser `document.createElement`/`textContent` pour chaque cellule, ou un helper d'échappement HTML. 2. **Crash sur valeurs `null`** dans `_normalize_payload()` (`weather_service.py` L159-169) — Si `temperature_2m[idx]` est `null` (documented dans la spec §5 "Pièges fréquents" pour les données anciennes), `float(None)` lève `TypeError` → erreur 500 non catchée. Seul `weather_code: null` est géré. 3. **`httpx.AsyncClient` jamais fermé** — Les instances `self.client` dans `CommuneService` et `WeatherService` ne sont jamais `close()`-ées. En production long-running, cela peut causer des fuites de connexions. Un `lifespan` handler FastAPI serait approprié. ### Duplication ? Mineure : les classes `FakeResponse` et `FakeClient` sont dupliquées entre `test_commune_service.py` et `test_weather_service.py`. Pourrait être dans `conftest.py`, mais non bloquant. ### Incohérences ? - La validation `end_date < start_date` dans `_validate_dates` est testée **après** la vérification `start_date < MIN_HISTORICAL_DATE`. Si `start=1939-01-01&end=1938-01-01`, l'erreur retournée sera "antérieure au 1er janvier 1940" et non "date de fin avant date de début". C'est acceptable mais à noter. --- ## 4) Test Coverage ### Tests suffisants ? ❌ **Non** — 7 tests sur 20 sont **SKIPPED** et ne s'exécutent pas. **Cause** : `pytest-asyncio` est installé (v0.25.3 dans `requirements.txt`) mais **aucun fichier de configuration** (`pyproject.toml`, `pytest.ini`, `setup.cfg`) ne déclare `asyncio_mode`. Avec pytest-asyncio >= 0.21, le mode par défaut est `strict` et nécessite une configuration explicite, sinon les markers `@pytest.mark.asyncio` ne sont pas reconnus. Les tests skippés couvrent : - `test_commune_service.py` : transformation coordonnées, réponse vide, erreur upstream (3 tests) - `test_weather_service.py` : normalisation + WMO, `null` WMO, erreur upstream, longueurs incohérentes (4 tests) Ce sont les **tests unitaires de la logique métier core** — leur non-exécution vide de sens la stratégie de tests §6. ### Edge cases oubliés ? - Pas de test pour les caractères accentués dans la recherche commune (mentionné dans §6 "Edge cases critiques" : `"saint-étienne"`, `"île-de-france"`) - Pas de test de validation des coordonnées (`lat`/`lon` hors range) - Pas de test pour `weather_code: null` au niveau API intégration (seulement au niveau service, mais ce test est SKIPPED) - Pas de test pour les valeurs `null` dans `temperature_2m` / `precipitation` etc. --- ## 5) UX Consistency Check ### Incohérences flagrantes ? - **Absence totale d'accents** dans l'interface (titres HTML, messages, descriptions WMO). L'application semble être en français "sans diacritiques". C'est une incohérence UX forte pour une app ciblant des utilisateurs francophones. ### Comportement inattendu ? - **Bug timezone été** : un utilisateur consultant la météo du 15 juillet 2024 verra toutes les heures décalées d'1h (ex: 14h00 affiché au lieu de 15h00). Bug silencieux et trompeur. ### Friction évidente ? - Pas de friction notable au-delà des bugs identifiés. Le flow est fluide. --- ## 6) Required Corrections | # | Fichier | Correction | Sévérité | Routage | | --- | ----------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------ | ----------- | | C1 | `tests/` + racine | **Ajouter `pyproject.toml`** (ou `pytest.ini`) avec `[tool.pytest.ini_options] asyncio_mode = "auto"` pour que les 7 tests async s'exécutent réellement. | **Bloquant** | Développeur | | C2 | `public/app.js` L97 | **Corriger `formatDisplayDateTime`** : ne pas hardcoder l'offset UTC. Utiliser directement `new Date(isoLocalTime)` en traitant le timestamp comme une heure locale Europe/Paris (puisque Open-Meteo retourne déjà en heure locale avec le paramètre `timezone`), ou parser sans offset et formater directement les composants de la string ISO. | **Bloquant** | Développeur | | C3 | `public/app.js` L179-187 | **Corriger la faille XSS** dans `renderRows()` : remplacer `innerHTML` par la création d'éléments DOM avec `textContent` pour chaque cellule. | **Bloquant** | Développeur | | C4 | `src/weather_service.py` L21-49 | **Rétablir les accents** dans `WMO_DESCRIPTIONS` conformément à la table §3.6 de la spec technique. Corriger aussi la typo "Brouillard givrante" → "Brouillard givrant". | **Bloquant** | Développeur | | C5 | `src/weather_service.py`, `src/commune_service.py`, `src/main.py` | **Rétablir les accents** dans tous les messages d'erreur conformément à la spec §3.5 (ex: "paramètre", "Réessayez", "données", "antérieure", "postérieure", "limitée"). | **Bloquant** | Développeur | | C6 | `src/weather_service.py` L159-169 | **Gérer les valeurs `null`** dans `_normalize_payload` pour `temperature_2m`, `precipitation`, `relative_humidity_2m`, `wind_speed_10m` (pas seulement `weather_code`). Un `null` doit donner une valeur par défaut (ex: `None` ou `"—"`) sans crash. | **Bloquant** | Développeur | | C7 | `public/index.html` | **Rétablir les accents** dans les textes statiques HTML (titre, labels, note de transparence) conformément à la spec et INV-5. | **Bloquant** | Développeur | --- ## 7) Recommended Improvements (non bloquantes) | # | Suggestion | | --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | | R1 | Fermer proprement les `httpx.AsyncClient` via un `lifespan` handler FastAPI (`async with httpx.AsyncClient() as client`). | | R2 | Factoriser `FakeResponse`/`FakeClient` dans `conftest.py` pour éviter la duplication entre les deux fichiers de test service. | | R3 | Ajouter les edge cases manquants listés en §6 de la spec technique (caractères accentués, coordonnées hors range, `null` sur valeurs numériques). | | R4 | Les messages côté frontend (`app.js`) sont aussi sans accents ("Saisissez au moins 2 caracteres", "Aucune commune trouvee…") — à harmoniser avec le backend. | --- ## 🧭 Décision finale ### ❌ Rejeté — corrections obligatoires **Justification** : 1. **7/20 tests ne s'exécutent pas** — la couverture de test est trompeuse. Les services core (commune, weather) n'ont aucun test validé. 2. **Bug timezone critique** (AC4 / INV-2) — les heures d'été sont toutes fausses, ce qui est le coeur fonctionnel de l'application. 3. **Faille XSS** — injection HTML possible via `innerHTML`. 4. **INV-5 violé** — l'absence systématique d'accents français dans toute l'application (WMO, erreurs, UI) viole l'invariant d'interface intégralement en français. 5. **Crash silencieux** sur données `null` (valeurs numériques) non géré. ### Routage - **C1, C2, C3, C4, C5, C6, C7** → retour **Développeur** pour corrections. - Aucun retour Architecte nécessaire (spec technique cohérente). - Aucun retour Rédacteur nécessaire (spec fonctionnelle cohérente).