# FEEDBACK TO ARCHITECT — 001-histometeo-mvp (v2)

**Date** : 2025-03-11
**Reviewer** : Reviewer Technique & Qualité
**Sources** : `001-histometeo-mvp.md` (spec fonctionnelle), `001-histometeo-mvp.tech.v2.md` (spec technique v2), `feedback-to-architect-001.md` (feedback précédent), code implémenté

---

## 1) Functional Compliance

| AC   | Statut | Justification                                                                                                                                               |
| ---- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------- |
| AC1  | ✅ OK  | Auto-complétion fonctionnelle. 4 tests unitaires du service commune passent (dont test caractères accentués `"saint-étienne"`).                             |
| AC2  | ✅ OK  | Flow complet commune → dates → résumé journalier + tableau horaire fonctionne. Structure `{data, daily_summary}` conforme.                                  |
| AC3  | ✅ OK  | Tableau horaire avec colonnes date/heure, température, précipitations, humidité, vent, conditions (icône + description). Valeurs `null` affichées en `"—"`. |
| AC4  | ✅ OK  | `formatDisplayDateTime` corrigé : parsing direct de la string ISO sans offset UTC hardcodé. Heures d'été correctes.                                         |
| AC5  | ✅ OK  | Validation période > 31 jours fonctionnelle (test API passe).                                                                                               |
| AC6  | ✅ OK  | Validation date future fonctionnelle (test API passe).                                                                                                      |
| AC7  | ✅ OK  | Validation date < 1940 fonctionnelle (test API passe).                                                                                                      |
| AC8  | ✅ OK  | Messages d'erreur explicites, non génériques, **avec accents** dans tous les messages backend et frontend.                                                  |
| AC9  | ✅ OK  | Note de transparence visible en permanence dans la section `#info`, contenu conforme.                                                                       |
| AC10 | ✅ OK  | CSS responsive mobile-first, breakpoint 768px, tables scrollables. 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).

**Remarque** : `README.md` a été mis à jour avec les fonctionnalités v2 (icônes, résumé journalier). La spec technique stipule que le README "ne sera mis à jour qu'après implémentation complète validée". L'implémentation étant complète et les tests passant, c'est une anticipation mineure, non bloquante.

### Forbidden changes respectés ?

✅ Oui — `docs/specs/`, `.github/` 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, pas de fichier).                                                                                                                          |
| INV-2     | ✅ OK        | `formatDisplayDateTime` parse la string ISO directement via `split("T")`. Pas d'offset UTC hardcodé.                                                                                          |
| INV-3     | ✅ OK        | Limite 31 jours validée côté backend (`_validate_dates`) et côté frontend (`isDateRangeValid`).                                                                                               |
| INV-4     | ✅ OK        | Aucune clé API requise.                                                                                                                                                                       |
| INV-5     | ⚠️ KO mineur | Accents rétablis dans toute l'interface **sauf** le nom de l'application : `<title>HistoMeteo</title>` et `<h1>HistoMeteo</h1>` alors que la spec et le titre FastAPI utilisent "HistoMétéo". |
| INV-6     | ✅ OK        | Page unique, pas de routeur.                                                                                                                                                                  |
| INV-7     | ✅ OK        | Aucun `innerHTML` avec données dynamiques. `textContent` utilisé systématiquement dans `renderRows`, `renderDailySummary`, `showSuggestions`, `showGlobalError`.                              |

---

## 3) Technical Quality

### Complexité inutile ?

Non. L'architecture reste sobre et conforme au design minimal.

### Dette technique introduite ?

Non. Les éléments identifiés dans le feedback v1 ont été corrigés :

- Faille XSS éliminée (C3 ✅)
- Clients httpx fermés proprement via `lifespan` (R1 ✅)
- Crash `null` résolu (C6 ✅)

### Duplication ?

Aucune. `FakeResponse` / `FakeClient` correctement factorisés dans `conftest.py` (R2 ✅).

### Incohérences ?

1. **Nom de l'application** : `FastAPI(title="HistoMétéo")` avec accents, mais `<title>HistoMeteo</title>` et `<h1>HistoMeteo</h1>` sans accents. Inconsistance mineure.

2. **Typographie française** : espace manquant avant le deux-points dans `Période disponible:` et `Données:` (convention typographique française : espace insécable avant `:`). Non bloquant mais perfectible.

3. **Ambiguïté période 31 jours** : la validation backend `(end_date - start_date).days > MAX_PERIOD_DAYS` autorise un écart de 31 jours (soit 32 jours calendaires de données). "31 jours maximum" pourrait signifier 31 jours calendaires de données (écart de 30 jours). L'interprétation actuelle est raisonnable mais non testée dans le cas limite exact.

### Qualité du code

- Structure de code claire et lisible
- Séparation des responsabilités respectée (config, cache, services, API)
- Helpers `_to_float_or_none` / `_to_int_or_none` bien factorisés
- Méthode `_public_row` isole la projection des champs exposés
- `defaultdict` + `Counter` pour le résumé journalier : choix simple et approprié
- Algorithme de condition dominante (mode + tie-break par code WMO le plus élevé) conforme à la spec

---

## 4) Test Coverage

### Tests suffisants ?

**23/23 tests PASSED, 0 SKIPPED** — correction C1 (`pyproject.toml` avec `asyncio_mode = "auto"`) effective.

| Fichier                   | Tests | Couverture                                                                                                                                    |
| ------------------------- | ----- | --------------------------------------------------------------------------------------------------------------------------------------------- |
| `test_cache.py`           | 5     | Insert/get, expiration, FIFO eviction, clé manquante, thread safety ✅                                                                        |
| `test_commune_service.py` | 4     | Transformation coordonnées, réponse vide, erreur upstream, accents ✅                                                                         |
| `test_weather_service.py` | 6     | Normalisation + WMO + icônes, null WMO fallback, erreur upstream, longueurs incohérentes, nulls numériques, résumé multi-jours + tie-break ✅ |
| `test_api.py`             | 8     | Communes OK/court/manquant, Weather OK + daily_summary + icônes, période trop longue, date future, date < 1940, fin < début ✅                |

### Edge cases couverts vs spec §6

| Edge case spécifié                                              | Couvert ?                                      |
| --------------------------------------------------------------- | ---------------------------------------------- |
| Caractères accentués commune (`"saint-étienne"`)                | ✅ Oui                                         |
| `weather_code: null` → icône `❓` + description fallback        | ✅ Oui                                         |
| `temperature_2m: null` → `None` dans la réponse                 | ✅ Oui                                         |
| Résumé journalier toutes températures `null` → `temp_min: null` | ✅ Oui (second jour du test multi-day)         |
| Résumé égalité codes WMO → code le plus élevé                   | ✅ Oui (second jour : codes 3 et 61, 61 gagne) |
| Résumé un seul code WMO sur 24h → condition dominante           | ⚠️ Non testé explicitement                     |
| Période exactement 31 jours (limite inclusive)                  | ⚠️ Non testé (cas limite absent)               |

### Edge cases manquants (non bloquants)

1. **Période de 31 jours exactement** : pas de test pour le cas `(end_date - start_date).days == 31` → vérifie que c'est bien accepté (ni 30 ni 32).
2. **Validation coordonnées invalides** : pas de test unitaire ni API pour `lat=100` ou `lon=200` → vérifie que le message d'erreur est correct.
3. **Résumé journalier avec tous weather codes `null`** : le deuxième jour du test multi-day a des weather codes valides (3, 61). Pas de test où une journée entière n'a que des `weather_code: None` → devrait donner `DEFAULT_WMO` ("❓", "Conditions non disponibles").

---

## 5) UX Consistency Check

### Incohérences flagrantes ?

- Nom de l'application sans accent dans le `<title>` et `<h1>` alors que le reste de l'interface a des accents. Visuellement mineur (le nom propre "HistoMeteo" reste lisible).

### Comportement inattendu ?

Aucun. Le bug timezone été (C2) est corrigé. Les heures s'affichent correctement quelle que soit la saison.

### Friction évidente ?

Aucune. Le flow est fluide : saisie → suggestions → sélection → dates → recherche → résumé + tableau.

---

## 6) Required Corrections

Aucune correction bloquante.

---

## 7) Recommended Improvements (non bloquantes)

| #   | Suggestion                                                                                                                                                                                  |
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| R1  | **Accents nom de l'application** : remplacer `<title>HistoMeteo</title>` et `<h1>HistoMeteo</h1>` par `HistoMétéo` pour une cohérence complète avec INV-5 et le titre FastAPI.              |
| R2  | **Typographie française** : ajouter une espace avant les deux-points dans `Période disponible :` (`index.html`) et `Données :` (footer).                                                    |
| R3  | **Tests edge cases manquants** : ajouter (a) test période 31 jours exactement, (b) test coordonnées hors range, (c) test résumé journalier avec tous `weather_code: None` sur un jour.      |
| R4  | **Deprecation pytest-asyncio** : ajouter `asyncio_default_fixture_loop_scope = "function"` dans `pyproject.toml` pour éliminer le `PytestDeprecationWarning` sur le scope de la event loop. |
| R5  | **Cohérence Dockerfile** : l'image Docker utilise `python:3.12-slim` tandis que l'environnement local est en Python 3.14. Aligner sur la même version majeure pour éviter des divergences.  |

---

## 🧭 Décision finale

### ✅ Validé

**Justification** :

1. **Les 11 critères d'acceptation (AC1–AC11) sont tous satisfaits**.
2. **Les 7 corrections bloquantes (C1–C7) du feedback précédent sont toutes résolues** :
   - C1 : `pyproject.toml` avec `asyncio_mode = "auto"` → 23/23 tests passent, 0 skipped
   - C2 : `formatDisplayDateTime` corrigé, plus d'offset hardcodé
   - C3 : `innerHTML` remplacé par `textContent` partout (XSS éliminé)
   - C4 : Accents WMO rétablis, typo "givrant" corrigée
   - C5 : Accents messages d'erreur backend rétablis
   - C6 : Valeurs `null` numériques gérées via `_to_float_or_none` / `_to_int_or_none`
   - C7 : Accents textes HTML statiques rétablis
3. **Les 4 améliorations recommandées (R1–R4) sont intégrées** :
   - R1 : `lifespan` handler ferme les clients httpx au shutdown
   - R2 : `FakeResponse`/`FakeClient` factorisés dans `conftest.py`
   - R3 : Edge cases ajoutés (accents commune, nulls numériques, résumé multi-jours)
   - R4 : Accents messages frontend corrigés
4. **Icônes météo** : table WMO enrichie avec emoji, champ `icon` dans la réponse API, affichage dans les colonnes Conditions (horaire + résumé).
5. **Résumé journalier** : calcul backend (`_compute_daily_summary`) correct, retourné dans `daily_summary`, affiché dans la section `#daily-summary` au-dessus du tableau horaire.
6. **Tous les 7 invariants sont préservés** (INV-5 avec réserve mineure sur le nom de l'application).
7. **Qualité technique** : code propre, pas de dette technique introduite, bonne séparation des responsabilités.
8. **23/23 tests passent**, couverture adéquate pour le MVP.

Les 5 améliorations recommandées (R1–R5) sont toutes non bloquantes et peuvent être traitées dans une itération ultérieure.

### Routage

Aucun retour nécessaire — implémentation conforme aux spécifications fonctionnelle et technique.