fix(backend): traite la revue de phyri0s sur la PR #84

Tri non déterministe : `latest_for_site` départage désormais les égalités de
timestamp par `reading_id` décroissant, comme `list_history`. `uq_reading_source`
autorise deux lignes au même `site_id`+`timestamp` quand la `source` diffère, donc
le `LIMIT 1` pouvait renvoyer l'une ou l'autre d'un appel à l'autre.

Tests : trois tests `integration` sur `latest_for_site` (plus récente, égalité de
timestamp, isolation par site). Le test d'égalité échoue sans le correctif ci-dessus.

Duplication : `DataQuality` et le repli vers `critical` sortent dans
`app/services/data_quality.py`, partagé par `stats.py`, `site.py` et `sensor.py`,
qui en portaient trois copies indépendantes. Supprime au passage deux
`# type: ignore[assignment]`.
This commit is contained in:
Johan LEROY
2026-09-18 10:28:04 +02:00
parent d167b64188
commit 5eb74aa64a
6 changed files with 74 additions and 22 deletions
+3 -1
View File
@@ -22,10 +22,12 @@ class ReadingRepository:
return (await self._session.execute(requete)).scalars().all() return (await self._session.execute(requete)).scalars().all()
async def latest_for_site(self, site_id: str) -> Reading | None: async def latest_for_site(self, site_id: str) -> Reading | None:
# Piège : `uq_reading_source` autorise deux lignes au même `site_id`+`timestamp` quand la
# `source` diffère. Sans `reading_id` en départage, le `LIMIT 1` renverrait au hasard.
requete = ( requete = (
select(Reading) select(Reading)
.where(Reading.site_id == site_id) .where(Reading.site_id == site_id)
.order_by(Reading.timestamp.desc()) .order_by(Reading.timestamp.desc(), Reading.reading_id.desc())
.limit(1) .limit(1)
) )
lecture: Reading | None = await self._session.scalar(requete) lecture: Reading | None = await self._session.scalar(requete)
+18
View File
@@ -0,0 +1,18 @@
# Contrainte : `ck_reading_quality` accepte NULL et quatre valeurs seulement, alors que le contrat
# frontend n'a aucune valeur pour l'absence de qualité. `qualite_ou_critique()` replie donc sur
# `critical`, la seule des quatre qui n'induise pas une confiance qu'on n'a pas. `QUALITES_CONNUES`
# reste exposé pour les appelants qui doivent distinguer un `critical` stocké d'un repli.
from typing import Literal, get_args
DataQuality = Literal["good", "partial", "degraded", "critical"]
QUALITES_CONNUES: frozenset[str] = frozenset(get_args(DataQuality))
_PAR_VALEUR: dict[str, DataQuality] = {valeur: valeur for valeur in get_args(DataQuality)}
def qualite_ou_critique(valeur: str | None) -> DataQuality:
if valeur is None:
return "critical"
return _PAR_VALEUR.get(valeur, "critical")
+2 -3
View File
@@ -5,12 +5,11 @@ from typing import Literal
from app.models.energy import Reading, Site from app.models.energy import Reading, Site
from app.repositories.reading import ReadingRepository from app.repositories.reading import ReadingRepository
from app.repositories.site import SiteRepository from app.repositories.site import SiteRepository
from app.services.data_quality import qualite_ou_critique
CapteurStatus = Literal["ok", "failing"] CapteurStatus = Literal["ok", "failing"]
OverallStatus = Literal["ok", "degraded", "critical"] OverallStatus = Literal["ok", "degraded", "critical"]
QUALITES_CONNUES: frozenset[str] = frozenset({"good", "partial", "degraded", "critical"})
RAISON_VERS_CAPTEUR: dict[str, str] = { RAISON_VERS_CAPTEUR: dict[str, str] = {
"consumption_sensor_failure": "consumption", "consumption_sensor_failure": "consumption",
"electrical_sensor_failure": "electrical", "electrical_sensor_failure": "electrical",
@@ -80,7 +79,7 @@ def _sante_site(site: Site, derniere: Reading | None) -> SanteSite:
overall="critical", overall="critical",
) )
qualite = derniere.data_quality if derniere.data_quality in QUALITES_CONNUES else "critical" qualite = qualite_ou_critique(derniere.data_quality)
overall = _overall_depuis_qualite(qualite) overall = _overall_depuis_qualite(qualite)
if overall == "critical": if overall == "critical":
+2 -9
View File
@@ -1,15 +1,11 @@
from collections.abc import Sequence from collections.abc import Sequence
from dataclasses import dataclass from dataclasses import dataclass
from datetime import datetime from datetime import datetime
from typing import Literal
from app.models.energy import Site from app.models.energy import Site
from app.repositories.reading import ReadingRepository from app.repositories.reading import ReadingRepository
from app.repositories.site import SiteRepository from app.repositories.site import SiteRepository
from app.services.data_quality import DataQuality, qualite_ou_critique
DataQuality = Literal["good", "partial", "degraded", "critical"]
QUALITES_CONNUES: frozenset[str] = frozenset({"good", "partial", "degraded", "critical"})
class SiteError(Exception): class SiteError(Exception):
@@ -70,9 +66,6 @@ class SiteService:
data_quality="critical", data_quality="critical",
) )
qualite: DataQuality = "critical"
if derniere.data_quality in QUALITES_CONNUES:
qualite = derniere.data_quality # type: ignore[assignment]
return SiteCurrentReading( return SiteCurrentReading(
timestamp=derniere.timestamp, timestamp=derniere.timestamp,
site_id=site.site_id, site_id=site.site_id,
@@ -85,5 +78,5 @@ class SiteService:
temperature_celsius=derniere.temperature_celsius, temperature_celsius=derniere.temperature_celsius,
humidity_percent=derniere.humidity_percent, humidity_percent=derniere.humidity_percent,
null_reasons=derniere.null_reasons or [], null_reasons=derniere.null_reasons or [],
data_quality=qualite, data_quality=qualite_ou_critique(derniere.data_quality),
) )
+2 -9
View File
@@ -1,14 +1,10 @@
from dataclasses import dataclass from dataclasses import dataclass
from datetime import UTC, datetime from datetime import UTC, datetime
from typing import Literal
from app.models.energy import Reading, Site from app.models.energy import Reading, Site
from app.repositories.reading import ReadingRepository from app.repositories.reading import ReadingRepository
from app.repositories.site import SiteRepository from app.repositories.site import SiteRepository
from app.services.data_quality import QUALITES_CONNUES, DataQuality, qualite_ou_critique
DataQuality = Literal["good", "partial", "degraded", "critical"]
QUALITES_CONNUES: frozenset[str] = frozenset({"good", "partial", "degraded", "critical"})
@dataclass(frozen=True, slots=True) @dataclass(frozen=True, slots=True)
@@ -58,13 +54,10 @@ class StatsService:
@staticmethod @staticmethod
def _resume_site(site: Site, derniere: Reading | None) -> SiteConsumption: def _resume_site(site: Site, derniere: Reading | None) -> SiteConsumption:
capacite = site.capacity_kw or 0 capacite = site.capacity_kw or 0
# Piège : `data_quality` est nul dès qu'un site n'a jamais reçu de lecture, ou que le
# producteur n'a pas su la qualifier. Le contrat frontend n'a pas de valeur pour ce cas,
# `critical` est la seule des quatre qui n'induit pas une confiance qu'on n'a pas.
qualite: DataQuality = "critical" qualite: DataQuality = "critical"
consommation = None consommation = None
if derniere is not None and derniere.data_quality in QUALITES_CONNUES: if derniere is not None and derniere.data_quality in QUALITES_CONNUES:
qualite = derniere.data_quality # type: ignore[assignment] qualite = qualite_ou_critique(derniere.data_quality)
consommation = derniere.consumption_kw consommation = derniere.consumption_kw
charge = ( charge = (
@@ -88,6 +88,53 @@ async def test_latest_by_site_returns_one_row_per_site(session: AsyncSession) ->
assert identifiants == {premier, second} assert identifiants == {premier, second}
async def test_latest_for_site_returns_the_most_recent_reading(session: AsyncSession) -> None:
site = await creer_site(session)
depot = ReadingRepository(session)
await creer_lecture(session, site_id=site.site_id, timestamp=datetime(2026, 9, 1, tzinfo=UTC))
recente = await creer_lecture(
session, site_id=site.site_id, timestamp=datetime(2026, 9, 15, tzinfo=UTC)
)
trouvee = await depot.latest_for_site(site.site_id)
reading_id = trouvee.reading_id if trouvee else None
await session.rollback()
assert reading_id == recente.reading_id
async def test_latest_for_site_breaks_a_timestamp_tie_on_the_last_written_reading(
session: AsyncSession,
) -> None:
site = await creer_site(session)
depot = ReadingRepository(session)
horodatage = datetime(2026, 9, 15, tzinfo=UTC)
await creer_lecture(session, site_id=site.site_id, timestamp=horodatage, source="api_history")
derniere = await creer_lecture(
session, site_id=site.site_id, timestamp=horodatage, source="api_current"
)
trouvee = await depot.latest_for_site(site.site_id)
reading_id = trouvee.reading_id if trouvee else None
await session.rollback()
assert reading_id == derniere.reading_id
async def test_latest_for_site_ignores_the_readings_of_the_other_sites(
session: AsyncSession,
) -> None:
sans_lecture = await creer_site(session)
autre = await creer_site(session)
depot = ReadingRepository(session)
await creer_lecture(session, site_id=autre.site_id)
trouvee = await depot.latest_for_site(sans_lecture.site_id)
await session.rollback()
assert trouvee is None
async def test_list_history_orders_the_readings_by_timestamp_descending( async def test_list_history_orders_the_readings_by_timestamp_descending(
session: AsyncSession, session: AsyncSession,
) -> None: ) -> None: