db : le cycle de vie d'une recommandation entre en base (#164) #180

Merged
lenaic merged 1 commit from olivier/164-recommandation-cycle into develop 2026-09-08 08:16:41 +00:00
Member

Ferme #164. cycle.rapprocher, écrit au #154, attendait de savoir où écrire : voici la table.

Le numéro n'est pas celui du ticket

Le #164 demande une 0017_zone_or_recommandation_cycle.sql. 0017 est prise par le #36 (référence de comparaison des prévisions) et 0018 par le #168 (libellé et état des alertes), toutes deux fusionnées dans develop pendant que le #164 attendait. Le fichier est donc la 0019, même contenu, même intention. La 0018 annonçait d'ailleurs le rendez-vous : « resolue_a vient avec le cycle de vie des recommandations au #164 ».

Ce que la migration fait

Trois colonnes sur public.recommandation, et la 0014 n'est pas retouchée :

Colonne Forme Pourquoi
statut text not null default 'active' contraint à active / resolue. Le défaut vaut aussi pour les lignes déjà chargées : la 0014 n'enregistre que des émissions
resolue_a timestamptz nullable l'instant que le #153 demande. Contrairement à alerte au #168, la colonne arrive avec ce qui la remplit — cycle.Resolution le produit déjà
action text not null la consigne, qui sépare une recommandation d'une alerte. libelle porte le constat et laisse l'exploitant sans quoi faire

L'unicité de la 0014 tombe. unique (site_id, horodatage, regle_id, regle_version) laissait coexister deux lignes actives pour le même site et la même règle — il suffit que l'horodatage diffère, c'est-à-dire à chaque heure — et faisait de la version une part de l'identité, alors que le #153 pose l'inverse : « recaler un seuil ne doit ni fermer ni rouvrir ce qui est en cours ». À sa place, un index unique partiel (site_id, regle_id) where statut = 'active'. Partiel, parce que l'historique des résolues doit pouvoir compter plusieurs lignes pour le même couple.

Le rejeu idempotent d'une journée que l'ancienne contrainte servait est perdu, et c'est assumé : il appartient désormais au job, pas au schéma.

Le critère 5, tranché : default '' puis drop default

Un add column ... not null sans défaut réussit sur une table vide et échoue dès la première ligne présente. Les migrations s'appliquent au démarrage du serveur, donc à un moment que personne ne choisit.

La table est vide partout aujourd'hui — rien n'y écrit : le chargement de la zone or ne remplit que mesure, qualite_jour et alerte, et l'API sert des fixtures. Mais une migration qui ne marche que sur une table vide est une migration qui tombe le jour où elle ne l'est plus. D'où les deux gestes : le défaut rend l'ajout sûr quelle que soit l'histoire de la base, son retrait fait qu'une insertion qui oublie l'action échoue au lieu d'écrire une consigne vide.

Le nullable a été écarté pour la raison qui vaut déjà côté moteur, où regle.py refuse d'émettre : « une recommandation qui ne dit pas quoi faire n'est pas une recommandation ».

Une contrainte de plus que le ticket ne demandait

check ((statut = 'resolue') = (resolue_a is not null)). L'état et son instant ne peuvent pas se contredire — c'est l'invariant pour lequel resolue_a existe. Signalé ici parce que le #164 ne l'énumérait pas.

Éprouvé sur PostgreSQL, pas seulement lu

Deux bases d'essai jetables sur ev-postgres, supprimées à la fin ; ni enervision_preprod ni enervision_prod touchées.

  • base neuve : 0001 → 0019 d'affilée, sans erreur ;
  • base arrêtée à 0018, portant déjà une ligne de recommandation : 0019 seule passe, et la ligne préexistante ressort en statut = active, resolue_a nul, action = '' — le cas exact du critère 5 ;
  • rejeu de la 0019 une seconde fois sur les deux bases : rien que des NOTICE, aucune erreur ;
  • retour arrière joué tel qu'il est écrit en fin de fichier : la table revient aux huit colonnes de la 0014 et retrouve recommandation_site_id_horodatage_regle_id_regle_version_key — ce qui prouve au passage que le nom écrit dans le drop constraint est bien celui que PostgreSQL fabrique. La 0019 se rejoue ensuite par-dessus.

Les gardes ont été poussées, et non supposées :

Insertion Verdict
une première active acceptée
une seconde active, même site + même règle refuséerecommandation_active
même site + même règle, version différente refusée — recaler un seuil ne rouvre rien
une active sur une autre règle acceptée
sans action refuséenot null
statut inconnu refuséerecommandation_statut
résolue sans resolue_a, ou active avec un resolue_a refuséesrecommandation_resolue_a
deux résolues, même site + même règle acceptées — c'est l'historique

Côté tests

  • tests/unit/db/test_migration_recommandation_cycle.py, 14 cas. Dont une garde de dérive avec le moteur : la 0019 est le « où écrire » que cycle.rapprocher attend, et rien d'autre ne surveille que les deux se parlent des mêmes champs — le job n'existe pas encore. Et une garde sur le nom de la contrainte retirée, recomposé depuis la clause de la 0014 plutôt que recopié : le jour où quelqu'un touche à cette clause, le drop constraint if exists ne trouverait plus rien et ne le dirait pas.
  • La 0019 rejoint la liste NOUVEAUX de test_migrations_zone_or.py (critère 4) plutôt que de recopier les deux garde-fous génériques — sens de retour écrit, rejeu sans effet de bord.

pytest tests/unit : 932 passés, 1 sauté. ruff check et ruff format --check propres.

Hors périmètre, comme le ticket le pose

Brancher cycle.rapprocher sur cette persistance — lecture des actives, écriture des ouvertures et des résolutions depuis le service recommendations — reste à faire dans un ticket séparé. Ni changement d'API, ni de tableau de bord.

Ferme #164. `cycle.rapprocher`, écrit au #154, attendait de savoir où écrire : voici la table. ## Le numéro n'est pas celui du ticket Le #164 demande une `0017_zone_or_recommandation_cycle.sql`. **0017 est prise par le #36** (référence de comparaison des prévisions) et **0018 par le #168** (libellé et état des alertes), toutes deux fusionnées dans `develop` pendant que le #164 attendait. Le fichier est donc la **0019**, même contenu, même intention. La 0018 annonçait d'ailleurs le rendez-vous : « `resolue_a` vient avec le cycle de vie des recommandations au #164 ». ## Ce que la migration fait Trois colonnes sur `public.recommandation`, et la 0014 n'est pas retouchée : | Colonne | Forme | Pourquoi | |---|---|---| | `statut` | `text not null default 'active'` | contraint à `active` / `resolue`. Le défaut vaut aussi pour les lignes déjà chargées : la 0014 n'enregistre que des émissions | | `resolue_a` | `timestamptz` nullable | l'instant que le #153 demande. Contrairement à `alerte` au #168, la colonne arrive avec ce qui la remplit — `cycle.Resolution` le produit déjà | | `action` | `text not null` | la consigne, qui sépare une recommandation d'une alerte. `libelle` porte le constat et laisse l'exploitant sans quoi faire | **L'unicité de la 0014 tombe.** `unique (site_id, horodatage, regle_id, regle_version)` laissait coexister deux lignes *actives* pour le même site et la même règle — il suffit que l'horodatage diffère, c'est-à-dire à chaque heure — et faisait de la version une part de l'identité, alors que le #153 pose l'inverse : « recaler un seuil ne doit ni fermer ni rouvrir ce qui est en cours ». À sa place, un **index unique partiel** `(site_id, regle_id) where statut = 'active'`. Partiel, parce que l'historique des résolues doit pouvoir compter plusieurs lignes pour le même couple. Le rejeu idempotent d'une journée que l'ancienne contrainte servait est perdu, et c'est assumé : il appartient désormais au job, pas au schéma. ## Le critère 5, tranché : `default ''` puis `drop default` Un `add column ... not null` sans défaut réussit sur une table vide et **échoue dès la première ligne présente**. Les migrations s'appliquent au démarrage du serveur, donc à un moment que personne ne choisit. La table est vide partout aujourd'hui — rien n'y écrit : le chargement de la zone or ne remplit que `mesure`, `qualite_jour` et `alerte`, et l'API sert des fixtures. Mais une migration qui ne marche que sur une table vide est une migration qui tombe le jour où elle ne l'est plus. D'où les deux gestes : le défaut rend l'ajout sûr quelle que soit l'histoire de la base, son retrait fait qu'une insertion qui **oublie** l'action échoue au lieu d'écrire une consigne vide. Le nullable a été écarté pour la raison qui vaut déjà côté moteur, où `regle.py` refuse d'émettre : « une recommandation qui ne dit pas quoi faire n'est pas une recommandation ». ## Une contrainte de plus que le ticket ne demandait `check ((statut = 'resolue') = (resolue_a is not null))`. L'état et son instant ne peuvent pas se contredire — c'est l'invariant pour lequel `resolue_a` existe. Signalé ici parce que le #164 ne l'énumérait pas. ## Éprouvé sur PostgreSQL, pas seulement lu Deux bases d'essai jetables sur `ev-postgres`, supprimées à la fin ; ni `enervision_preprod` ni `enervision_prod` touchées. - **base neuve** : 0001 → 0019 d'affilée, sans erreur ; - **base arrêtée à 0018, portant déjà une ligne de recommandation** : 0019 seule passe, et la ligne préexistante ressort en `statut = active`, `resolue_a` nul, `action = ''` — le cas exact du critère 5 ; - **rejeu** de la 0019 une seconde fois sur les deux bases : rien que des `NOTICE`, aucune erreur ; - **retour arrière** joué tel qu'il est écrit en fin de fichier : la table revient aux huit colonnes de la 0014 et retrouve `recommandation_site_id_horodatage_regle_id_regle_version_key` — ce qui prouve au passage que le nom écrit dans le `drop constraint` est bien celui que PostgreSQL fabrique. La 0019 se rejoue ensuite par-dessus. Les gardes ont été poussées, et non supposées : | Insertion | Verdict | |---|---| | une première active | acceptée | | une seconde active, même site + même règle | **refusée** — `recommandation_active` | | même site + même règle, **version différente** | **refusée** — recaler un seuil ne rouvre rien | | une active sur une autre règle | acceptée | | sans `action` | **refusée** — `not null` | | `statut` inconnu | **refusée** — `recommandation_statut` | | résolue sans `resolue_a`, ou active **avec** un `resolue_a` | **refusées** — `recommandation_resolue_a` | | deux résolues, même site + même règle | acceptées — c'est l'historique | ## Côté tests - `tests/unit/db/test_migration_recommandation_cycle.py`, 14 cas. Dont une **garde de dérive** avec le moteur : la 0019 est le « où écrire » que `cycle.rapprocher` attend, et rien d'autre ne surveille que les deux se parlent des mêmes champs — le job n'existe pas encore. Et une garde sur le nom de la contrainte retirée, **recomposé** depuis la clause de la 0014 plutôt que recopié : le jour où quelqu'un touche à cette clause, le `drop constraint if exists` ne trouverait plus rien **et ne le dirait pas**. - La 0019 rejoint la liste `NOUVEAUX` de `test_migrations_zone_or.py` (critère 4) plutôt que de recopier les deux garde-fous génériques — sens de retour écrit, rejeu sans effet de bord. `pytest tests/unit` : 932 passés, 1 sauté. `ruff check` et `ruff format --check` propres. ## Hors périmètre, comme le ticket le pose Brancher `cycle.rapprocher` sur cette persistance — lecture des actives, écriture des ouvertures et des résolutions depuis le service `recommendations` — reste à faire dans un ticket séparé. Ni changement d'API, ni de tableau de bord.
db: le cycle de vie d'une recommandation entre en base (#164)
All checks were successful
Intégration / Contrôles statiques du dépôt (pull_request) Successful in 8s
Intégration / Workflows — lint et audit de sécurité (pull_request) Successful in 25s
Intégration / Tableau de bord — dépendances, tests et construction (pull_request) Successful in 36s
Intégration / Python — qualité, tests et dépendances (pull_request) Successful in 5m21s
ab69e1344e
`cycle.rapprocher`, écrit au #154, attendait de savoir où écrire. La 0019
ajoute à `public.recommandation` les trois colonnes du #153 : `statut`
(active/résolue, défaut active), `resolue_a`, et `action` — la consigne, qui
sépare une recommandation d'une alerte et que la 0014 avait oubliée. La 0014
n'est pas retouchée.

L'unicité (site, horodatage, règle, version) tombe : elle laissait coexister
deux lignes actives pour le même site et la même règle, et faisait de la
version une part de l'identité, alors que le #153 pose l'inverse — recaler un
seuil ne ferme ni ne rouvre ce qui est en cours. À sa place, un index unique
partiel sur (site_id, regle_id) où statut = 'active'.

`action text not null` devant des lignes déjà écrites (critère 5) : `default ''`
puis `drop default`. Le défaut rend l'ajout sûr quelle que soit l'histoire de
la base — sans lui, la migration échoue dès la première ligne présente, au
démarrage du serveur ; son retrait fait qu'une insertion qui oublie l'action
échoue au lieu d'écrire une consigne vide. La table est vide partout
aujourd'hui : le chargement de la zone or ne remplit que `mesure`,
`qualite_jour` et `alerte`, et l'API sert des fixtures.

Une contrainte de plus que le ticket ne demandait : `(statut = 'resolue') =
(resolue_a is not null)`. L'état et son instant ne peuvent pas se contredire,
et c'est l'invariant que la colonne existe pour tenir.

0017 était pris par le #36 et 0018 par le #168 pendant que le #164 attendait :
le fichier est donc la 0019, et non la 0017 du ticket.

Le fichier rejoint la liste `NOUVEAUX` de test_migrations_zone_or.py
(critère 4) et porte ses propres cas dans
tests/unit/db/test_migration_recommandation_cycle.py, dont la garde de dérive
avec `cycle.Resolution` et `Recommandation` du moteur.
olivier requested review from lenaic 2026-09-08 08:08:31 +00:00
lenaic approved these changes 2026-09-08 08:14:59 +00:00
Dismissed
lenaic left a comment

Approuvée. La migration est juste et l'index unique partiel est ce que le #153 demande : l'ancienne unicité laissait coexister une ligne active par heure pour le même site et la même règle, et faisait de la version une part de l'identité alors que le ticket pose l'inverse.

J'ai vérifié le nom de la contrainte en base plutôt que de le déduire, parce qu'un drop constraint if exists qui ne trouve rien ne dit rien :

recommandation_site_id_horodatage_regle_id_regle_version_key
  | UNIQUE (site_id, horodatage, regle_id, regle_version)

Il tombe juste. Le default '' puis drop default est le bon geste, et ton commentaire dit pourquoi mieux que je ne l'aurais fait.

Deux points bloquants, et ils sont chez moi

Ta migration casse la #175, ouverte en parallèle. Ce n'est pas un reproche : c'est à elle de s'aligner, pas à toi de reculer.

Le on conflict de la #175 visait la contrainte que tu supprimes. PostgreSQL rend 42P10, l'insertion entière échoue, à chaque passe horaire, et ça ne se voit que dans recommandations.log.

action devient not null sans défaut, et la #175 ne l'écrivait pas. C'est exactement ce que ton commentaire annonce vouloir : « une insertion future qui oublie l'action échoue au lieu d'écrire une consigne vide ». Elle a attrapé la mienne.

Les deux sont corrigés dans la #175, qui prend ta migration dans sa branche pour pouvoir s'y croiser. Elle en profite pour lire statut au lieu d'une fenêtre de 26 heures, et pour ÉCRIRE les résolutions au lieu de les compter : sans statut, a_resoudre partait à la poubelle et une recommandation restait ouverte pour toujours.

Une correction factuelle

Ta demande dit « la table est vide partout aujourd'hui ». Elle contient une ligne depuis que le job de la #175 a tourné sur le serveur ce matin :

SITE003 | r2 v1.0.0 | Data Center Marseille, charge > 85 % depuis 3 h

Ça ne change rien à ta migration, le default '' la traite correctement, et c'est bien pour ça qu'il est là. Mais le critère 5 n'est plus théorique : il s'applique pour de vrai au prochain déploiement.

Ordre de fusion

Celle-ci d'abord, la #175 ensuite. La #175 référence statut et action : fusionnée avant, elle casserait sur des colonnes absentes.

Approuvée. La migration est juste et l'index unique partiel est ce que le #153 demande : l'ancienne unicité laissait coexister une ligne active par heure pour le même site et la même règle, et faisait de la version une part de l'identité alors que le ticket pose l'inverse. J'ai vérifié le nom de la contrainte en base plutôt que de le déduire, parce qu'un `drop constraint if exists` qui ne trouve rien ne dit rien : ``` recommandation_site_id_horodatage_regle_id_regle_version_key | UNIQUE (site_id, horodatage, regle_id, regle_version) ``` Il tombe juste. Le `default ''` puis `drop default` est le bon geste, et ton commentaire dit pourquoi mieux que je ne l'aurais fait. ## Deux points bloquants, et ils sont chez moi Ta migration casse la #175, ouverte en parallèle. Ce n'est pas un reproche : c'est à elle de s'aligner, pas à toi de reculer. **Le `on conflict` de la #175 visait la contrainte que tu supprimes.** PostgreSQL rend 42P10, l'insertion entière échoue, à chaque passe horaire, et ça ne se voit que dans `recommandations.log`. **`action` devient `not null` sans défaut, et la #175 ne l'écrivait pas.** C'est exactement ce que ton commentaire annonce vouloir : « une insertion future qui oublie l'action échoue au lieu d'écrire une consigne vide ». Elle a attrapé la mienne. Les deux sont corrigés dans la #175, qui prend ta migration dans sa branche pour pouvoir s'y croiser. Elle en profite pour lire `statut` au lieu d'une fenêtre de 26 heures, et pour ÉCRIRE les résolutions au lieu de les compter : sans `statut`, `a_resoudre` partait à la poubelle et une recommandation restait ouverte pour toujours. ## Une correction factuelle Ta demande dit « la table est vide partout aujourd'hui ». Elle contient **une ligne** depuis que le job de la #175 a tourné sur le serveur ce matin : ``` SITE003 | r2 v1.0.0 | Data Center Marseille, charge > 85 % depuis 3 h ``` Ça ne change rien à ta migration, le `default ''` la traite correctement, et c'est bien pour ça qu'il est là. Mais le critère 5 n'est plus théorique : il s'applique pour de vrai au prochain déploiement. ## Ordre de fusion **Celle-ci d'abord, la #175 ensuite.** La #175 référence `statut` et `action` : fusionnée avant, elle casserait sur des colonnes absentes.
lenaic approved these changes 2026-09-08 08:16:37 +00:00
lenaic merged commit f010b1d5ce into develop 2026-09-08 08:16:41 +00:00
lenaic deleted branch olivier/164-recommandation-cycle 2026-09-08 08:16:41 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
g2/enervision!180
No description provided.