fix: using confirmed status to compute probation state

api/src/shared/common/seal_criteria.py

@@ -17,6 +17,7 @@
 from typing import Dict, Final, Iterable, Optional, Tuple, TypeAlias
 
 from shared.common.error_handling import raise_internal_http_error, unknown_seal_criterion
+from shared.database_gen.sqlacodegen_models import SealCriterion as SealCriterionOrm
 
 # A criterion named by its `SealCriterionName` value, as it arrives in a payload and as it
 # is stored in `seal_criterion.criterion`.
@@ -129,20 +130,24 @@ class CriterionPhase(str, Enum):
 )
 
 
-def is_serving_probation(
-    criterion: str | SealCriterionName,
-    probation_start: Optional[datetime],
-    observed_status: str | CriterionStatus | None,
-) -> bool:
-    """Whether `criterion` is currently serving probation.
+def is_in_grace_period(row: SealCriterionOrm) -> bool:
+    """Whether the criterion is holding an unconfirmed failure inside its grace period."""
+    if grace_period_for(row.criterion) is None:
+        return False
+    return (
+        row.probation_start is None
+        and row.confirmed_status == CriterionStatus.PASS
+        and row.first_observed_failure_at is not None
+    )
 
-    Probation is a clean-run requirement, so a criterion whose most recent check observed a failure
-    is not serving it: that failure restarts the clock, and the criterion is reported as failing
-    rather than as recovering. Exempt criteria never serve probation whatever the row says.
+
+def is_serving_probation(row: SealCriterionOrm) -> bool:
+    """
+    Whether the criterion is serving probation, which starts only once it stops failing.
     """
-    if probation_start is None or probation_period_for(criterion) is None:
+    if row.probation_start is None or probation_period_for(row.criterion) is None:
         return False
-    return observed_status != CriterionStatus.FAIL
+    return row.confirmed_status == CriterionStatus.PASS
 
 
 def roll_up_on_probation(

api/src/shared/db_models/feed_reliability_report_impl.py

@@ -17,7 +17,7 @@ def _seal_status_of(criterion_rows: list[SealCriterionOrm]) -> str:
     return roll_up_seal_status(
         (
             CriterionStatus(row.confirmed_status),
-            is_serving_probation(row.criterion, row.probation_start, row.observed_status),
+            is_serving_probation(row),
         )
         for row in criterion_rows
     ).value

api/src/shared/db_models/feed_reliability_summary_impl.py

@@ -64,10 +64,7 @@ def from_orm(cls, feed: FeedOrm | None) -> FeedReliabilitySummary | None:
             return None
 
         evaluated_ats = [c.evaluated_at for c in feed.seal_criteria if c.evaluated_at is not None]
-        serving_probation = {
-            c.criterion: is_serving_probation(c.criterion, c.probation_start, c.observed_status)
-            for c in feed.seal_criteria
-        }
+        serving_probation = {c.criterion: is_serving_probation(c) for c in feed.seal_criteria}
         on_probation = roll_up_on_probation(
             (CriterionStatus(c.confirmed_status), serving_probation[c.criterion]) for c in feed.seal_criteria
         )

api/src/shared/db_models/reliability_criterion_impl.py

@@ -5,6 +5,7 @@
     CriterionStatus,
     SealCriterionName,
     grace_period_for,
+    is_in_grace_period,
     is_serving_probation,
     probation_period_for,
     resolve_criterion,
@@ -17,12 +18,10 @@
 # shared with the nightly job so the two cannot drift apart.
 #
 # `status` serves `confirmed_status`, the debounced verdict, which is the one `has_seal` is rolled
-# up from - so a client can always explain the seal it is shown. `observed_status`, the raw daily
-# check, never reaches the response on its own: the one thing it says that the debounced value does
-# not - a failure running right now - is carried by `in_grace_period` instead. This is the shape
-# specified in the #1789 spike; #1799 shipped `observed_status` here by mistake, which reported a
-# criterion inside its grace period as `fail` and could report a criterion that is the sole reason
-# a seal was withdrawn as `unknown`.
+# up from - so a client can always explain the seal it is shown. `observed_status` never reaches
+# the response, directly or through a derived flag: it belongs to the last run, so a badge built on
+# it would flicker on a night that reached no verdict. This is the shape specified in the #1789
+# spike; #1799 shipped `observed_status` here by mistake.
 
 
 class ReliabilityCriterionImpl(ReliabilityCriterion):
@@ -84,22 +83,10 @@ def from_orm(cls, criterion_row: SealCriterionOrm | None) -> ReliabilityCriterio
         # authority on which criteria serve them.
         grace_period = grace_period_for(criterion)
         probation_period = probation_period_for(criterion)
-        on_probation = is_serving_probation(criterion, criterion_row.probation_start, criterion_row.observed_status)
+        on_probation = is_serving_probation(criterion_row)
+        in_grace_period = is_in_grace_period(criterion_row)
         probation_start = criterion_row.probation_start if on_probation else None
 
-        # A failing check still inside its grace period is not yet counting against the seal: the
-        # daily check reads `fail` but the debounced status is still `pass`. Both columns are read
-        # here rather than the served `status`, which is the debounced one and so is `pass` in
-        # exactly this case - that is what makes `in_grace_period` true only alongside `pass`, as
-        # #1789 specifies. Grace does not apply during probation: a failure then restarts the
-        # probation clock outright, so there is nothing left for grace to protect.
-        in_grace_period = (
-            grace_period is not None
-            and criterion_row.observed_status == CriterionStatus.FAIL
-            and criterion_row.confirmed_status == CriterionStatus.PASS
-            and criterion_row.probation_start is None
-        )
-
         return cls(
             criterion=criterion.value,
             status=status,

api/tests/integration/test_search_api.py

@@ -775,17 +775,14 @@ def test_search_reports_probation_from_the_view(client: TestClient, mocker):
         assert response_body.results[0].reliability_seal.probation_ends_at is not None
 
 
-def test_search_ignores_probation_for_a_criterion_observed_failing(client: TestClient, mocker):
-    """An observed failure restarts the probation clock, so the view must not roll it up.
-
-    This is the same rule `is_serving_probation` applies to the feed-detail and report endpoints;
-    search reads the materialized view instead, so the two roll-ups have to agree.
-    """
+def test_search_ignores_probation_for_a_withdrawn_criterion(client: TestClient, mocker):
+    """A withdrawn criterion's frozen `probation_start` must not roll up, in the view or the API."""
     _grant_seal_filter(mocker)
     feed_stable_id = TEST_GTFS_FEED_STABLE_IDS[0]
     probation_start = datetime.now(timezone.utc) - timedelta(days=20)
 
-    with _feed_with_seal(feed_stable_id, [_criterion("available", "fail", probation_start)]):
+    withdrawn = _criterion("fresh_coverage", "not_applicable", probation_start, confirmed_status="not_applicable")
+    with _feed_with_seal(feed_stable_id, [withdrawn]):
         response = _search(client, [("limit", 100), ("has_seal", "true")])
 
         assert response.status_code == 200

api/tests/unittest/models/test_feed_reliability_summary_impl.py

@@ -140,15 +140,53 @@ def test_probation_rolls_up_over_non_exempt_criteria(self):
         assert result.on_probation is True
         assert result.probation_ends_at == later + PROBATION_PERIOD
 
-    def test_probation_ignored_for_a_criterion_observed_failing(self):
-        """A criterion failing its latest check is failing, not recovering, so it rolls up as such."""
+    def test_probation_not_served_while_a_criterion_is_still_failing(self):
+        """A criterion sitting on a confirmed failure is failing, not waiting out a clean run."""
         feed = make_feed(
+            has_seal=False,
             criteria=[
                 make_criterion(
                     SealCriterionName.AVAILABLE,
                     observed_status="fail",
+                    confirmed_status="fail",
+                    probation_start=NOW - timedelta(days=20),
+                )
+            ],
+        )
+        result = FeedReliabilitySummaryImpl.from_orm(feed)
+
+        assert result.on_probation is False
+        assert result.probation_ends_at is None
+
+    def test_probation_not_served_when_the_latest_run_reached_no_verdict(self):
+        """`unknown` over a confirmed failure must not read as the recovery probation counts from."""
+        feed = make_feed(
+            has_seal=False,
+            criteria=[
+                make_criterion(
+                    SealCriterionName.AVAILABLE,
+                    observed_status="unknown",
+                    confirmed_status="fail",
                     probation_start=NOW - timedelta(days=20),
                 )
+            ],
+        )
+        result = FeedReliabilitySummaryImpl.from_orm(feed)
+
+        assert result.on_probation is False
+        assert result.probation_ends_at is None
+
+    def test_probation_ignored_for_a_withdrawn_criterion(self):
+        """`not_applicable` freezes `probation_start` rather than clearing it, so it must not roll up."""
+        feed = make_feed(
+            criteria=[
+                make_criterion(
+                    SealCriterionName.FRESH_COVERAGE,
+                    observed_status="not_applicable",
+                    confirmed_status="not_applicable",
+                    probation_start=NOW - timedelta(days=20),
+                ),
+                make_criterion(SealCriterionName.AVAILABLE),
             ]
         )
         result = FeedReliabilitySummaryImpl.from_orm(feed)

api/tests/unittest/models/test_reliability_criterion_impl.py

@@ -184,27 +184,57 @@ def test_passing_while_on_probation(self):
         assert result.on_probation is True
         assert result.probation_ends_at == probation_start + PROBATION_PERIOD
 
-    def test_observed_failure_ends_probation(self):
-        """An observed failure restarts the probation clock, so the row is not serving probation.
-
-        Grace does not step in either: a failure during probation has nothing to protect.
-        """
+    def test_a_confirmed_failure_is_not_serving_probation(self):
+        """A criterion still failing has not begun its clean run, whatever `probation_start` says."""
         row = make_row(
             criterion=SealCriterionName.AVAILABLE,
             observed_status="fail",
-            confirmed_status="pass",
-            first_observed_failure_at=NOW - timedelta(days=2),
+            confirmed_status="fail",
+            first_observed_failure_at=NOW - timedelta(days=20),
             last_observed_failure_at=NOW,
-            probation_start=NOW - timedelta(days=1),
+            last_confirmed_failure_at=NOW,
+            probation_start=NOW + timedelta(days=1),
         )
         result = ReliabilityCriterionImpl.from_orm(row)
 
-        assert result.status == CriterionStatus.PASS.value
+        assert result.status == CriterionStatus.FAIL.value
         assert result.on_probation is False
         assert result.probation_ends_at is None
         assert result.in_grace_period is False
         assert result.grace_period_ends_at is None
 
+    def test_an_unevaluable_run_over_a_confirmed_failure_is_not_probation(self):
+        """`unknown` on top of a confirmed failure is still a failure, not the recovery."""
+        row = make_row(
+            criterion=SealCriterionName.AVAILABLE,
+            observed_status="unknown",
+            confirmed_status="fail",
+            first_observed_failure_at=NOW - timedelta(days=20),
+            last_observed_failure_at=NOW - timedelta(days=1),
+            last_confirmed_failure_at=NOW - timedelta(days=1),
+            probation_start=NOW,
+        )
+        result = ReliabilityCriterionImpl.from_orm(row)
+
+        assert result.status == CriterionStatus.FAIL.value
+        assert result.on_probation is False
+        assert result.probation_ends_at is None
+
+    def test_an_unevaluable_run_does_not_clear_the_grace_period(self):
+        """A night that reached no verdict leaves the streak, and its countdown, standing."""
+        first_failure = NOW - timedelta(days=10)
+        row = make_row(
+            observed_status="unknown",
+            confirmed_status="pass",
+            first_observed_failure_at=first_failure,
+            last_observed_failure_at=NOW - timedelta(days=1),
+        )
+        result = ReliabilityCriterionImpl.from_orm(row)
+
+        assert result.status == CriterionStatus.PASS.value
+        assert result.in_grace_period is True
+        assert result.grace_period_ends_at == first_failure + GRACE_PERIODS[SealCriterionName.COMPLIANT]
+
     def test_official_and_stable_serve_no_probation(self):
         """`official` and `stable` are exempt, even if a probation_start is somehow stored."""
         for criterion in (SealCriterionName.OFFICIAL, SealCriterionName.STABLE):

liquibase/materialized_views/feed_search.sql

@@ -115,10 +115,9 @@ LEFT JOIN (
 -- Seal of Reliability. FeedReliabilitySeal owns the overall outcome; the per-criterion rows
 -- contribute the last evaluation time and the probation roll-up, so the search result can carry
 -- the same summary the feed-detail endpoint embeds without a second lookup.
--- Probation is aggregated over the criteria that are actually serving it: 'official' and 'stable'
--- are point-in-time state checks and are exempt, so a stray probation_start on one of them must not
--- make the feed look like it is waiting out six months; and a criterion whose latest check observed
--- a failure is failing rather than recovering, since that failure restarts the probation clock.
+-- Probation is aggregated over the criteria actually serving it: 'official' and 'stable' are
+-- exempt, and probation starts only once a criterion stops failing, so it has to be a confirmed
+-- pass. Mirrors `seal_criteria.is_serving_probation`.
 LEFT JOIN (
     SELECT
         FeedReliabilitySeal.feed_id,
@@ -130,7 +129,7 @@ LEFT JOIN (
             WHEN bool_or(SealCriterion.confirmed_status = 'fail') THEN NULL
             ELSE MAX(SealCriterion.probation_start) FILTER (
                 WHERE SealCriterion.criterion NOT IN ('official', 'stable')
-                  AND SealCriterion.observed_status <> 'fail'
+                  AND SealCriterion.confirmed_status = 'pass'
             )
         END AS seal_latest_probation_start
     FROM feed_reliability_seal AS FeedReliabilitySeal