From 951e7cc22cf948abd883b01e02d9af861c17e71e Mon Sep 17 00:00:00 2001 From: cka-y Date: Tue, 4 Aug 2026 11:14:23 -0400 Subject: [PATCH 01/10] feat: seal of reliability schema --- liquibase/changelog.xml | 2 ++ liquibase/changes/feat_1760.sql | 64 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 liquibase/changes/feat_1760.sql diff --git a/liquibase/changelog.xml b/liquibase/changelog.xml index 1b82f3e88..1408c7220 100644 --- a/liquibase/changelog.xml +++ b/liquibase/changelog.xml @@ -125,6 +125,8 @@ + + diff --git a/liquibase/changes/feat_1760.sql b/liquibase/changes/feat_1760.sql new file mode 100644 index 000000000..52140de1b --- /dev/null +++ b/liquibase/changes/feat_1760.sql @@ -0,0 +1,64 @@ +-- Add Seal of Reliability tables (issue #1760). +-- These tables denormalize data already present in gtfs_feed_availability_check, +-- validationreport, and gtfsdataset. Following the existing pattern (e.g. +-- gtfsfeed.latest_dataset_id): raw tables remain the append-only source of truth, +-- while the seal tables are a nightly-computed cache that lets the API answer +-- "does this feed have the seal?" in a single row lookup with no joins. +-- See #1761 for the usage of these tables. + +-- One row per feed; owns the overall seal outcome. +CREATE TABLE IF NOT EXISTS feed_reliability_seal ( + feed_id VARCHAR(255) PRIMARY KEY, + + has_seal BOOLEAN NOT NULL DEFAULT FALSE, + seal_earned_at TIMESTAMPTZ, + seal_lost_at TIMESTAMPTZ, + + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + CONSTRAINT feed_reliability_seal_feed_id_fkey + FOREIGN KEY (feed_id) + REFERENCES gtfsfeed(id) + ON DELETE CASCADE +); + +CREATE TYPE seal_criterion_name AS ENUM ( + 'official', + 'stable', + 'available', + 'compliant', + 'fresh_coverage', + 'fresh_continuous' +); + +-- One row per feed per criterion; owns all per-criterion state. +-- raw_* columns reflect the instantaneous state at the last evaluation, with no grace applied. +-- grace_* columns reflect the debounced state: a failure is only confirmed after its grace period expires. +-- The seal logic is driven exclusively by grace_* columns; raw_* columns are for monitoring and audit. +CREATE TABLE IF NOT EXISTS seal_criterion ( + feed_id VARCHAR(255) NOT NULL, + criterion seal_criterion_name NOT NULL, + + -- Current state + raw_failing BOOLEAN, -- NULL = not yet evaluated; TRUE = failing at last check, no grace applied + grace_failing BOOLEAN, -- NULL = not yet evaluated; TRUE = failure has persisted beyond the grace period + + -- Evaluation tracking + evaluated_at TIMESTAMPTZ, + + -- Failure tracking + first_raw_failure_at TIMESTAMPTZ, -- start of current raw failure streak; cleared on recovery + last_raw_failure_at TIMESTAMPTZ, -- end of last raw failure streak; never cleared + last_grace_failure_at TIMESTAMPTZ, -- end of the grace period failure; never cleared + + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + PRIMARY KEY (feed_id, criterion), + + CONSTRAINT seal_criterion_feed_id_fkey + FOREIGN KEY (feed_id) + REFERENCES gtfsfeed(id) + ON DELETE CASCADE +); From 21b3b6be1f23bd49cb603ed43c2aa7ac2ccaae9c Mon Sep 17 00:00:00 2001 From: cka-y Date: Tue, 4 Aug 2026 11:20:23 -0400 Subject: [PATCH 02/10] fix: naming consistency --- liquibase/changes/feat_1760.sql | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/liquibase/changes/feat_1760.sql b/liquibase/changes/feat_1760.sql index 52140de1b..ee6fca200 100644 --- a/liquibase/changes/feat_1760.sql +++ b/liquibase/changes/feat_1760.sql @@ -1,26 +1,21 @@ -- Add Seal of Reliability tables (issue #1760). -- These tables denormalize data already present in gtfs_feed_availability_check, --- validationreport, and gtfsdataset. Following the existing pattern (e.g. --- gtfsfeed.latest_dataset_id): raw tables remain the append-only source of truth, +-- ValidationReport, and GTFSDataset. Following the existing pattern (e.g. +-- GTFSFeed.latest_dataset_id): raw tables remain the append-only source of truth, -- while the seal tables are a nightly-computed cache that lets the API answer -- "does this feed have the seal?" in a single row lookup with no joins. -- See #1761 for the usage of these tables. -- One row per feed; owns the overall seal outcome. -CREATE TABLE IF NOT EXISTS feed_reliability_seal ( - feed_id VARCHAR(255) PRIMARY KEY, +CREATE TABLE IF NOT EXISTS FeedReliabilitySeal ( + feed_id VARCHAR(255) PRIMARY KEY REFERENCES GTFSFeed(id) ON DELETE CASCADE, has_seal BOOLEAN NOT NULL DEFAULT FALSE, seal_earned_at TIMESTAMPTZ, seal_lost_at TIMESTAMPTZ, created_at TIMESTAMPTZ NOT NULL DEFAULT now(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), - - CONSTRAINT feed_reliability_seal_feed_id_fkey - FOREIGN KEY (feed_id) - REFERENCES gtfsfeed(id) - ON DELETE CASCADE + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() ); CREATE TYPE seal_criterion_name AS ENUM ( @@ -36,8 +31,8 @@ CREATE TYPE seal_criterion_name AS ENUM ( -- raw_* columns reflect the instantaneous state at the last evaluation, with no grace applied. -- grace_* columns reflect the debounced state: a failure is only confirmed after its grace period expires. -- The seal logic is driven exclusively by grace_* columns; raw_* columns are for monitoring and audit. -CREATE TABLE IF NOT EXISTS seal_criterion ( - feed_id VARCHAR(255) NOT NULL, +CREATE TABLE IF NOT EXISTS SealCriterion ( + feed_id VARCHAR(255) NOT NULL REFERENCES GTFSFeed(id) ON DELETE CASCADE, criterion seal_criterion_name NOT NULL, -- Current state @@ -55,10 +50,5 @@ CREATE TABLE IF NOT EXISTS seal_criterion ( created_at TIMESTAMPTZ NOT NULL DEFAULT now(), updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), - PRIMARY KEY (feed_id, criterion), - - CONSTRAINT seal_criterion_feed_id_fkey - FOREIGN KEY (feed_id) - REFERENCES gtfsfeed(id) - ON DELETE CASCADE + PRIMARY KEY (feed_id, criterion) ); From 180494e0be016e4c9efcf53316b10d5639e9afca Mon Sep 17 00:00:00 2001 From: jcpitre Date: Wed, 5 Aug 2026 09:39:29 -0400 Subject: [PATCH 03/10] Indexed seal of reliability tables with Feed instead of GtfsFeed. --- liquibase/changes/feat_1760.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/liquibase/changes/feat_1760.sql b/liquibase/changes/feat_1760.sql index ee6fca200..1c6c84fc1 100644 --- a/liquibase/changes/feat_1760.sql +++ b/liquibase/changes/feat_1760.sql @@ -8,7 +8,7 @@ -- One row per feed; owns the overall seal outcome. CREATE TABLE IF NOT EXISTS FeedReliabilitySeal ( - feed_id VARCHAR(255) PRIMARY KEY REFERENCES GTFSFeed(id) ON DELETE CASCADE, + feed_id VARCHAR(255) PRIMARY KEY REFERENCES Feed(id) ON DELETE CASCADE, has_seal BOOLEAN NOT NULL DEFAULT FALSE, seal_earned_at TIMESTAMPTZ, @@ -32,7 +32,7 @@ CREATE TYPE seal_criterion_name AS ENUM ( -- grace_* columns reflect the debounced state: a failure is only confirmed after its grace period expires. -- The seal logic is driven exclusively by grace_* columns; raw_* columns are for monitoring and audit. CREATE TABLE IF NOT EXISTS SealCriterion ( - feed_id VARCHAR(255) NOT NULL REFERENCES GTFSFeed(id) ON DELETE CASCADE, + feed_id VARCHAR(255) NOT NULL REFERENCES Feed(id) ON DELETE CASCADE, criterion seal_criterion_name NOT NULL, -- Current state From db0362d952cd5f121b5bed45efdff74672f5658f Mon Sep 17 00:00:00 2001 From: jcpitre Date: Wed, 5 Aug 2026 17:43:08 -0400 Subject: [PATCH 04/10] Seal of reliability first commit --- functions-python/tasks_executor/README.md | 75 ++++ functions-python/tasks_executor/src/main.py | 18 + .../src/tasks/seal_of_reliability/__init__.py | 0 .../src/tasks/seal_of_reliability/context.py | 122 ++++++ .../src/tasks/seal_of_reliability/criteria.py | 49 +++ .../evaluators/__init__.py | 40 ++ .../seal_of_reliability/evaluators/base.py | 63 +++ .../evaluators/official.py | 40 ++ .../tasks/seal_of_reliability/seal_updater.py | 413 ++++++++++++++++++ .../seal_of_reliability/state_machine.py | 115 +++++ .../update_seal_of_reliability.py | 68 +++ .../test_seal_end_to_end_db.py | 302 +++++++++++++ .../test_seal_evaluators.py | 90 ++++ .../test_seal_state_machine.py | 191 ++++++++ .../test_seal_updater_db.py | 350 +++++++++++++++ 15 files changed, 1936 insertions(+) create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/__init__.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/context.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py create mode 100644 functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py create mode 100644 functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py create mode 100644 functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py create mode 100644 functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py create mode 100644 functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py diff --git a/functions-python/tasks_executor/README.md b/functions-python/tasks_executor/README.md index 420b02178..2cf2e392d 100644 --- a/functions-python/tasks_executor/README.md +++ b/functions-python/tasks_executor/README.md @@ -406,3 +406,78 @@ Only **comparable** datasets are considered: a dataset must have a `downloaded_a | `pairs_dispatched` | Pairs dispatched (or, in `dry_run`, that would be dispatched) | | `dispatched` | (dry-run only) list of `{feed_stable_id, base_dataset_stable_id, new_dataset_stable_id}` | + +### update_seal_of_reliability + +Evaluates the implemented Seal of Reliability criteria (issue #1761) for every eligible GTFS +feed and updates the `sealcriterion` and `feedreliabilityseal` tables. Reads the source +tables and never modifies them. + +Only the **Official** criterion is implemented (issue #1783), so `has_seal` currently means +`feed.official IS TRUE`. The remaining five criteria — Stable, Available, Compliant and the +two Fresh checks — are tracked by #1784 and #1782; `seal_criterion_name` in the database +already declares all six values, so adding one needs no schema change. + +Eligible feeds are GTFS, `operational_status = published`, and `status NOT IN (deprecated, +development)`. `inactive` and `future` feeds are deliberately included: skipping a feed +freezes its stored rows rather than making it neutral. + +```json +{ + "task": "update_seal_of_reliability", + "payload": { + "dry_run": true, + "stable_feed_ids": ["mdb-1210"] + } +} +``` + +| Parameter | Type | Default | Description | +|---|---|---|---| +| `dry_run` | bool | `true` | Evaluate every feed and return the report without writing anything | +| `stable_feed_ids` | list[str] \| null | `null` | Evaluate only these feeds; unknown ids raise. When set, `evaluations` covers every criterion of those feeds instead of only the ones whose verdict moved | +| `limit` | int \| null | `null` | Cap the number of feeds evaluated | +| `criteria` | list[str] \| null | `null` | Evaluate only these criteria. Only `official` is implemented so far; naming a criterion that has no evaluator yet raises. A subset of the implemented criteria skips the `has_seal` roll-up, since the ones not evaluated cannot be judged | +| `batch_size` | int | `200` | Feeds loaded per query batch | +| `now` | str \| null | `null` | ISO timestamp to evaluate against, for replays and backfills. Defaults to the current UTC time | + +**Response fields**: + +| Field | Description | +|---|---| +| `total_feeds` | Feeds evaluated | +| `criteria` | The criteria evaluated in this run | +| `partial_run` | True when `criteria` was a subset, meaning `has_seal` was not recalculated | +| `criterion_rows_written` | `sealcriterion` rows inserted or updated (`0` on a dry run) | +| `not_evaluable` | Criterion evaluations skipped because an input was missing | +| `seals_before_run` | Feeds that held the seal before this run | +| `seals_after_run` | Feeds holding it afterwards — on a dry run, what *would* be stored | +| `seals_granted` / `seals_revoked` | Transitions in this run. `before + granted - revoked == after` | +| `revoked_stable_ids` | Feeds that lost the seal in this run | +| `first_evaluations` | Criteria evaluated for the first time (no stored row yet) | +| `evaluations` | The notable outcomes: one entry per feed and criterion whose verdict moved, with `raw_failing`, `grace_failing`, `previously_grace_failing` and `reason`. A first evaluation appears only if it landed on a failure; the rest are counted by `first_evaluations`. When `stable_feed_ids` is set, every criterion of those feeds is included whether or not it moved | + +> Note: no Cloud Scheduler job is defined for this task yet — invoke it manually. Once +> scheduled it should run after the daily `check_gtfs_feed_availability` job, since the +> Available criterion reads the availability rows recorded for the day. + +#### Running it locally + +Start Postgres and the function, then post to it: + +```shell +docker compose --env-file ./config/.env.local up -d --force-recreate +scripts/function-python-run.sh --function_name tasks_executor --no_install_venv +``` + +```shell +curl -s -X POST http://localhost:8080 -H "Content-Type: application/json" \ + -d '{"task":"update_seal_of_reliability","payload":{"dry_run":true,"stable_feed_ids":["mdb-1210"]}}' \ + | python3 -m json.tool +``` + +`Accept: text/csv` returns a single summary row (the top-level report fields), not one row +per evaluation — the converter flattens the returned dict, and `evaluations` lands in it as +a single stringified cell. Use the JSON response for per-criterion detail. + +Nothing about this task needs GCP credentials — only `FEEDS_DATABASE_URL`. diff --git a/functions-python/tasks_executor/src/main.py b/functions-python/tasks_executor/src/main.py index 943590341..d8ddeed71 100644 --- a/functions-python/tasks_executor/src/main.py +++ b/functions-python/tasks_executor/src/main.py @@ -76,6 +76,9 @@ notifications_dispatch_monitor_handler, ) from tasks.changelog.backfill_changelog import backfill_changelog_handler +from tasks.seal_of_reliability.update_seal_of_reliability import ( + update_seal_of_reliability_handler, +) init_logger() LIST_COMMAND: Final[str] = "list" @@ -255,6 +258,21 @@ ), "handler": backfill_changelog_handler, }, + "update_seal_of_reliability": { + "description": ( + "Evaluates the implemented Seal of Reliability criteria for every eligible " + "GTFS feed and updates sealcriterion and feedreliabilityseal. Only the " + "Official criterion is implemented so far (see #1784 and #1782 for the rest). " + "Reads the source tables and never modifies them. " + "Parameters: dry_run (default true), stable_feed_ids (default null; when set, " + "`evaluations` covers every criterion of those feeds), limit (default null), " + "criteria (default null " + "meaning every implemented criterion; a partial set skips the has_seal " + "roll-up), batch_size (default 200), now (ISO timestamp, default current " + "UTC time)." + ), + "handler": update_seal_of_reliability_handler, + }, } diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/__init__.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/context.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/context.py new file mode 100644 index 000000000..9b909189d --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/context.py @@ -0,0 +1,122 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Feed eligibility query and bulk loading of everything the evaluators need. + +The evaluators are pure functions over a `FeedSealContext`, so every DB read for a batch of +feeds happens here, in a fixed number of queries regardless of batch size. + +Only Official is implemented, so the context currently carries just the feed row. Each new +criterion adds the fields it needs here plus one bulk query to populate them: the latest +dataset for Compliant and Fresh, the day's availability rows for Available, the full +dataset coverage history for Fresh continuous coverage. +""" + +from dataclasses import dataclass +from datetime import datetime +from typing import Dict, Optional, Sequence + +from sqlalchemy.orm import Session + +from shared.database_gen.sqlacodegen_models import Feed, Gtfsfeed + + +@dataclass +class FeedSealContext: + """Everything the evaluators need for one feed. + + Built by `build_contexts`. Evaluators read from this and never query. + """ + + feed_id: str + # The evaluation timestamp. Passed in rather than read from the clock so evaluators + # stay pure and a run can be replayed for any point in time. + now: datetime + stable_id: Optional[str] = None + + # Feed-level flags + official: Optional[bool] = None + + +def get_seal_feeds_query( + db_session: Session, stable_feed_ids: Optional[Sequence[str]] = None +): + """Return a query for the feeds the seal applies to. + + Eligibility is defined here and nowhere else, so a full run and a one-feed run exercise + the same predicate. `inactive` and `future` feeds are deliberately included: skipping a + feed does not make it neutral, it freezes its stored rows, and once Fresh (future + coverage) exists an inactive feed should fail it rather than keep displaying a seal. + """ + query = db_session.query(Gtfsfeed).filter( + Feed.data_type == "gtfs", + Feed.status.notin_(["deprecated", "development"]), + Feed.operational_status == "published", + ) + if stable_feed_ids is not None: + query = query.filter(Feed.stable_id.in_(list(stable_feed_ids))) + return query + + +def build_contexts( + db_session: Session, feeds: Sequence[Gtfsfeed], now: datetime +) -> Dict[str, FeedSealContext]: + """Load everything the evaluators need for `feeds`, in a fixed number of queries. + + Args: + db_session: SQLAlchemy session. Unused while Official is the only criterion, since + everything it needs is already on the feed row, but kept in the signature + because every further criterion needs it. + feeds: The batch of feeds to load, from `get_seal_feeds_query`. + now: The evaluation timestamp. + + Returns: + feed_id -> FeedSealContext. + + How to add a criterion's data. Two kinds: + + 1. Already on the selected feed row (`official`, `created_at`, `seasonal`, + `is_producer_url_unstable`). Add the field to FeedSealContext and read it off `feed` + below. No query, no cost. + + 2. Needs its own query. Add the field, then a module-level `_load_*` helper that takes + the whole batch and returns a dict keyed by feed_id, and call it once here. Keeping + the query per batch rather than per feed is what holds the query count proportional + to the number of criteria instead of the number of feeds. For example, Available + (issue #1784) would add: + + def _load_availability_today(db_session, feed_ids, day_start) -> Dict[str, bool]: + '''feed_id -> whether any availability check succeeded since day_start. + Feeds absent from the result had no check at all, which the criterion reads + as "not evaluable" rather than "failing".''' + + called once as `availability = _load_availability_today(...)` and consumed per feed + as `availability_success_today=availability.get(feed.id, False)`. + """ + return { + feed.id: FeedSealContext( + feed_id=feed.id, + now=now, + stable_id=feed.stable_id, + official=feed.official, + ) + for feed in feeds + } + + +def batched(items: Sequence, size: int): + """Yield successive slices of `items` of at most `size` elements.""" + for start in range(0, len(items), size): + yield items[start : start + size] diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py new file mode 100644 index 000000000..c06bf245b --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py @@ -0,0 +1,49 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Seal of Reliability policy values. + +Everything here is the published definition of the seal, so it lives in code rather than +in DB config: changing any of these values changes which feeds qualify, and that should +go through code review, tests and a deploy. + +Each criterion's grace period belongs with the criterion, as a class attribute on its +evaluator. Only Official is implemented so far and it has none. +""" + +from datetime import timedelta +from enum import Enum +from typing import Final + + +class SealCriterionName(str, Enum): + """The six seal criteria. Values match the seal_criterion_name DB enum. + + All six are listed even though only Official has an evaluator (see #1784 and #1782), + so that the enum stays a faithful mirror of the database type. + """ + + OFFICIAL = "official" + STABLE = "stable" + AVAILABLE = "available" + COMPLIANT = "compliant" + FRESH_COVERAGE = "fresh_coverage" + FRESH_CONTINUOUS = "fresh_continuous" + + +# A criterion that fails past its grace period stays failing until it has gone this long +# without another confirmed failure. It is the default for new evaluators; Official is +# exempt because it is a point-in-time state check (see OfficialEvaluator). +RELIABILITY_WINDOW: Final[timedelta] = timedelta(days=180) diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py new file mode 100644 index 000000000..a8851ec7f --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py @@ -0,0 +1,40 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""The seal criterion evaluators. + +`EVALUATORS` is the registry the job iterates. Only Official is implemented so far +(issue #1783); the remaining criteria are tracked by #1784 and #1782. Adding one means a +new subclass, an entry here, and whatever fields it needs on `FeedSealContext`. + +`seal_criterion_name` in the database already declares all six values, so a criterion can +be added without a schema change. +""" + +from typing import Final, List + +from tasks.seal_of_reliability.evaluators.base import CriterionEvaluator, RawEvaluation +from tasks.seal_of_reliability.evaluators.official import OfficialEvaluator + +EVALUATORS: Final[List[CriterionEvaluator]] = [ + OfficialEvaluator(), +] + +__all__ = [ + "EVALUATORS", + "CriterionEvaluator", + "OfficialEvaluator", + "RawEvaluation", +] diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py new file mode 100644 index 000000000..2b0495f91 --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py @@ -0,0 +1,63 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Base class for the per-criterion evaluators.""" + +from dataclasses import dataclass +from datetime import timedelta +from typing import Optional, Tuple + +from tasks.seal_of_reliability.context import FeedSealContext +from tasks.seal_of_reliability.criteria import RELIABILITY_WINDOW, SealCriterionName + + +@dataclass(frozen=True) +class RawEvaluation: + """One criterion's verdict for one feed, with no grace applied. + + `failing` is tri-state: True and False are verdicts, None means the criterion could not + be evaluated this run (a missing availability check, a criterion that does not apply to + this feed) and the stored state must be left alone. + """ + + criterion: SealCriterionName + failing: Optional[bool] + reason: str + + +class CriterionEvaluator: + """Evaluates one criterion against a pre-loaded feed context. + + Subclasses set `name` and, where they differ from the defaults, `grace_period` and + `reliability_window`, then implement `_evaluate`. They never touch the database: all + the data they need is on the context, loaded in bulk by `context.build_contexts`. + """ + + name: SealCriterionName = None + grace_period: Optional[timedelta] = None + reliability_window: Optional[timedelta] = RELIABILITY_WINDOW + + def evaluate(self, ctx: FeedSealContext) -> RawEvaluation: + """Evaluate the criterion and label the result with this evaluator's name. + + Labelling happens here rather than in the subclasses so an evaluator cannot + disagree with its own `name`. + """ + failing, reason = self._evaluate(ctx) + return RawEvaluation(criterion=self.name, failing=failing, reason=reason) + + def _evaluate(self, ctx: FeedSealContext) -> Tuple[Optional[bool], str]: + """Return (failing, reason) for this feed. Implemented by subclasses.""" + raise NotImplementedError("Subclasses should implement this method.") diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py new file mode 100644 index 000000000..63f0152f8 --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py @@ -0,0 +1,40 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Official criterion: the feed is flagged official.""" + +from typing import Optional, Tuple + +from tasks.seal_of_reliability.context import FeedSealContext +from tasks.seal_of_reliability.criteria import SealCriterionName +from tasks.seal_of_reliability.evaluators.base import CriterionEvaluator + + +class OfficialEvaluator(CriterionEvaluator): + """`feed.official IS TRUE`. + + A point-in-time state check: official at the time of reviewing the dataset, with no + 6-month check. Both the grace period and the reliability window are None, so the + criterion clears as soon as the feed is flagged official again. + """ + + name = SealCriterionName.OFFICIAL + grace_period = None + reliability_window = None + + def _evaluate(self, ctx: FeedSealContext) -> Tuple[Optional[bool], str]: + if ctx.official: + return False, "feed is official" + return True, f"feed.official is {ctx.official!r}, expected True" diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py new file mode 100644 index 000000000..892c2183e --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py @@ -0,0 +1,413 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Nightly Seal of Reliability evaluation (issue #1761). + +Reads feed, dataset, validation report and availability data; writes only sealcriterion +and feedreliabilityseal. The raw source tables are never modified. + +Both seal tables are written with Core statements against `__table__` rather than through +ORM objects. `feedreliabilityseal.feed_id` is both its primary key and a foreign key to +feed(id), which sqlacodegen maps as joined-table inheritance +(`class Feedreliabilityseal(Feed)`, a sibling of Gtfsfeed in the polymorphic hierarchy), so +persisting an ORM instance would try to insert a new feed. Core statements address the +table directly and are unaffected. +""" + +import logging +import time +from datetime import datetime, timezone +from typing import Dict, List, Optional, Sequence, Tuple + +from sqlalchemy import select +from sqlalchemy.dialects.postgresql import insert +from sqlalchemy.orm import Session + +from shared.database.database import with_db_session +from shared.database_gen.sqlacodegen_models import Feedreliabilityseal, Sealcriterion + +from tasks.seal_of_reliability.context import ( + batched, + build_contexts, + get_seal_feeds_query, +) +from tasks.seal_of_reliability.criteria import SealCriterionName +from tasks.seal_of_reliability.evaluators import EVALUATORS +from tasks.seal_of_reliability.state_machine import SealCriterionState, transition + +DEFAULT_BATCH_SIZE: int = 200 + +SEAL_TABLE = Feedreliabilityseal.__table__ +CRITERION_TABLE = Sealcriterion.__table__ + + +def _resolve_evaluators(criteria: Optional[Sequence[str]]) -> List: + """Return the evaluators to run, optionally filtered to `criteria`.""" + if criteria is None: + return list(EVALUATORS) + wanted = {str(name) for name in criteria} + known = {evaluator.name.value for evaluator in EVALUATORS} + unknown = sorted(wanted - known) + if unknown: + raise ValueError( + f"Unknown criteria: {unknown}. Known criteria: {sorted(known)}" + ) + return [evaluator for evaluator in EVALUATORS if evaluator.name.value in wanted] + + +def _load_previous_states( + db_session: Session, feed_ids: Sequence[str] +) -> Dict[Tuple[str, str], SealCriterionState]: + """Map (feed_id, criterion) -> stored state for a batch of feeds.""" + if not feed_ids: + return {} + rows = db_session.execute( + select(CRITERION_TABLE).where(CRITERION_TABLE.c.feed_id.in_(list(feed_ids))) + ).all() + return { + (row.feed_id, row.criterion): SealCriterionState( + feed_id=row.feed_id, + criterion=SealCriterionName(row.criterion), + raw_failing=row.raw_failing, + grace_failing=row.grace_failing, + evaluated_at=row.evaluated_at, + first_raw_failure_at=row.first_raw_failure_at, + last_raw_failure_at=row.last_raw_failure_at, + last_grace_failure_at=row.last_grace_failure_at, + ) + for row in rows + } + + +def _load_previous_seals( + db_session: Session, feed_ids: Sequence[str] +) -> Dict[str, bool]: + """Map feed_id -> stored has_seal, for feeds that already have a seal row.""" + if not feed_ids: + return {} + rows = db_session.execute( + select(SEAL_TABLE.c.feed_id, SEAL_TABLE.c.has_seal).where( + SEAL_TABLE.c.feed_id.in_(list(feed_ids)) + ) + ).all() + return {row.feed_id: bool(row.has_seal) for row in rows} + + +def _roll_up_has_seal( + states: Dict[str, SealCriterionState], + evaluator_count: int, + currently_held: bool, +) -> bool: + """True only when every criterion is explicitly not failing. + + `grace_failing is False` rather than `not grace_failing`: a criterion that has never + been evaluated is NULL, and NULL must not be read as a pass. A feed missing a row for + any criterion cannot qualify either. + + A grace period protects a seal the feed already holds; it cannot be used to earn one. + Without that distinction a feed being evaluated for the first time while already + failing would be handed the seal for the length of the grace period, which is the + opposite of what "14 days to fix it before disqualification" means. So granting + requires every criterion to pass right now, while keeping only requires that no + failure has been confirmed. + """ + if len(states) < evaluator_count: + return False + if not all(state.grace_failing is False for state in states.values()): + return False + if currently_held: + return True + return all(state.raw_failing is False for state in states.values()) + + +def _is_notable( + previous: Optional[SealCriterionState], state: Optional[SealCriterionState] +) -> bool: + """True when this evaluation moved a criterion's verdict. + + A first evaluation is not notable on its own: the initial run over the whole catalogue + would otherwise report every feed, which is exactly the unbounded payload this is meant + to avoid. The `first_evaluations` count covers that case instead. A first evaluation + that lands on a failure *is* reported, since that is the actionable half. + """ + if state is None: + return False + if previous is None: + return bool(state.raw_failing) or bool(state.grace_failing) + return ( + state.raw_failing != previous.raw_failing + or state.grace_failing != previous.grace_failing + ) + + +def _upsert_criteria( + db_session: Session, states: Sequence[SealCriterionState], now: datetime +) -> None: + """Insert or update sealcriterion rows for the given states.""" + if not states: + return + payload = [ + { + "feed_id": state.feed_id, + "criterion": state.criterion.value, + "raw_failing": state.raw_failing, + "grace_failing": state.grace_failing, + "evaluated_at": state.evaluated_at, + "first_raw_failure_at": state.first_raw_failure_at, + "last_raw_failure_at": state.last_raw_failure_at, + "last_grace_failure_at": state.last_grace_failure_at, + "updated_at": now, + } + for state in states + ] + statement = insert(CRITERION_TABLE).values(payload) + db_session.execute( + statement.on_conflict_do_update( + index_elements=[CRITERION_TABLE.c.feed_id, CRITERION_TABLE.c.criterion], + set_={ + "raw_failing": statement.excluded.raw_failing, + "grace_failing": statement.excluded.grace_failing, + "evaluated_at": statement.excluded.evaluated_at, + "first_raw_failure_at": statement.excluded.first_raw_failure_at, + "last_raw_failure_at": statement.excluded.last_raw_failure_at, + "last_grace_failure_at": statement.excluded.last_grace_failure_at, + "updated_at": statement.excluded.updated_at, + }, + ) + ) + + +def _upsert_seals(db_session: Session, outcomes: Sequence[dict], now: datetime) -> None: + """Insert or update feedreliabilityseal rows. + + seal_earned_at and seal_lost_at are written only when has_seal actually changes, so + they record transitions rather than the time of the last evaluation. + """ + for outcome in outcomes: + row = { + "feed_id": outcome["feed_id"], + "has_seal": outcome["has_seal"], + "updated_at": now, + } + if outcome["granted"]: + row["seal_earned_at"] = now + elif outcome["revoked"]: + row["seal_lost_at"] = now + + statement = insert(SEAL_TABLE).values(**row) + update_set = { + "has_seal": statement.excluded.has_seal, + "updated_at": statement.excluded.updated_at, + } + if "seal_earned_at" in row: + update_set["seal_earned_at"] = statement.excluded.seal_earned_at + if "seal_lost_at" in row: + update_set["seal_lost_at"] = statement.excluded.seal_lost_at + db_session.execute( + statement.on_conflict_do_update( + index_elements=[SEAL_TABLE.c.feed_id], set_=update_set + ) + ) + + +@with_db_session +def update_seals( + db_session: Session, + dry_run: bool = True, + stable_feed_ids: Optional[Sequence[str]] = None, + limit: Optional[int] = None, + criteria: Optional[Sequence[str]] = None, + batch_size: int = DEFAULT_BATCH_SIZE, + now: Optional[datetime] = None, +) -> dict: + """Evaluate the seal criteria for every eligible feed and store the result. + + Every feed is evaluated before anything is written, so a dry run exercises exactly the + same code path as a real run. + + Args: + db_session: SQLAlchemy session, injected by @with_db_session. + dry_run: Evaluate and report without writing. Default True. + stable_feed_ids: Evaluate only these feeds. Unknown ids raise. When given, + `evaluations` reports every criterion of those feeds rather than only the + ones whose verdict moved. + limit: Cap the number of feeds evaluated. + criteria: Evaluate only these criteria. A partial set skips the has_seal roll-up, + since the criteria that were not evaluated cannot be judged. + batch_size: Feeds loaded per query batch. + now: Evaluation timestamp. Defaults to the current UTC time. + + Returns: + A report dict. + """ + started = time.monotonic() + now = now or datetime.now(timezone.utc) + evaluators = _resolve_evaluators(criteria) + partial_run = len(evaluators) < len(EVALUATORS) + + query = get_seal_feeds_query(db_session, stable_feed_ids=stable_feed_ids) + if limit is not None: + query = query.limit(limit) + feeds = query.all() + + if stable_feed_ids is not None: + missing = sorted(set(stable_feed_ids) - {feed.stable_id for feed in feeds}) + if missing: + raise ValueError(f"stable_feed_ids not found: {missing}") + + logging.info( + "Evaluating %d criterion/criteria for %d feed(s) (dry_run=%s, now=%s).", + len(evaluators), + len(feeds), + dry_run, + now.isoformat(), + ) + + all_states: List[SealCriterionState] = [] + outcomes: List[dict] = [] + evaluations: List[dict] = [] + not_evaluable = 0 + first_evaluations = 0 + + for batch in batched(feeds, batch_size): + batch_ids = [feed.id for feed in batch] + contexts = build_contexts(db_session, batch, now) + previous_states = _load_previous_states(db_session, batch_ids) + previous_seals = _load_previous_seals(db_session, batch_ids) + + for feed in batch: + ctx = contexts[feed.id] + feed_states: Dict[str, SealCriterionState] = {} + + for evaluator in evaluators: + raw = evaluator.evaluate(ctx) + previous = previous_states.get((feed.id, evaluator.name.value)) + state = transition( + prev=previous, + raw=raw, + grace_period=evaluator.grace_period, + reliability_window=evaluator.reliability_window, + now=now, + feed_id=feed.id, + ) + if raw.failing is None: + not_evaluable += 1 + if state is not None: + feed_states[evaluator.name.value] = state + if state is not previous: + all_states.append(state) + if previous is None and state is not None: + first_evaluations += 1 + + # `evaluations` reports the notable outcomes, not one entry per + # evaluation: a criterion whose verdict moved, or every criterion of a feed + # the caller named explicitly. One entry per feed per criterion would grow + # with the catalogue (~424 bytes each, so megabytes for a full run) and is + # what the sealcriterion table is for. This matches `failures` in + # check_gtfs_feed_availability and `dispatched` in backfill_changelog. + named = stable_feed_ids is not None + if named or _is_notable(previous, state): + evaluations.append( + { + "stable_id": ctx.stable_id, + "criterion": evaluator.name.value, + "raw_failing": raw.failing, + "grace_failing": ( + state.grace_failing if state is not None else None + ), + "previously_grace_failing": ( + previous.grace_failing if previous is not None else None + ), + "reason": raw.reason, + } + ) + + if partial_run: + continue + + # Merge in any stored criteria this run did not produce a new state for, so the + # roll-up sees the full set even when an evaluator returned "not evaluable". + merged = { + criterion: state + for (owner_id, criterion), state in previous_states.items() + if owner_id == feed.id + } + merged.update(feed_states) + + had_seal = previous_seals.get(feed.id) + has_seal = _roll_up_has_seal( + merged, len(EVALUATORS), currently_held=bool(had_seal) + ) + outcomes.append( + { + "feed_id": feed.id, + "stable_id": ctx.stable_id, + "had_seal": bool(had_seal), + "has_seal": has_seal, + # A first evaluation is a grant if it passes, but it is not a loss if + # it fails: nothing was held, so nothing was lost. Only these two + # flags stamp seal_earned_at / seal_lost_at. + "granted": has_seal and not had_seal, + "revoked": bool(had_seal) and not has_seal, + } + ) + + revoked = [outcome for outcome in outcomes if outcome["revoked"]] + granted = [outcome for outcome in outcomes if outcome["granted"]] + if not dry_run: + for state_batch in batched(all_states, batch_size): + _upsert_criteria(db_session, state_batch, now) + db_session.commit() + for outcome_batch in batched(outcomes, batch_size): + _upsert_seals(db_session, outcome_batch, now) + db_session.commit() + + report = { + "message": ( + f"{'Dry run: evaluated' if dry_run else 'Updated'} {len(feeds)} feed(s) " + f"across {len(evaluators)} criterion/criteria." + ), + "dry_run": dry_run, + "evaluated_at": now.isoformat(), + "total_feeds": len(feeds), + "criteria": [evaluator.name.value for evaluator in evaluators], + "partial_run": partial_run, + "criterion_rows_written": 0 if dry_run else len(all_states), + "not_evaluable": not_evaluable, + "first_evaluations": first_evaluations, + # seals_before_run + seals_granted - seals_revoked == seals_after_run. + # On a dry run the "after" figure is what would be stored, not what is. + "seals_before_run": sum(1 for outcome in outcomes if outcome["had_seal"]), + "seals_after_run": sum(1 for outcome in outcomes if outcome["has_seal"]), + "seals_granted": len(granted), + "seals_revoked": len(revoked), + "revoked_stable_ids": [outcome["stable_id"] for outcome in revoked], + "elapsed_seconds": round(time.monotonic() - started, 2), + } + if partial_run: + report["note"] = ( + "Partial criteria run: has_seal was not recalculated because the criteria " + "that were not evaluated cannot be judged." + ) + report["evaluations"] = evaluations + + # Log without `evaluations`: Cloud Logging drops a LogEntry over 256 KB and an entry is + # ~424 bytes, so a run naming a few hundred feeds would lose the whole log entry. The + # counts belong in logs; `evaluations` is for the caller reading the response. + logging.info( + "Task completed: %s", + {key: value for key, value in report.items() if key != "evaluations"}, + ) + return report diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py new file mode 100644 index 000000000..3de18db8f --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py @@ -0,0 +1,115 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Generic per-criterion state machine for the Seal of Reliability. + +Steps 2 and 3 of the nightly job: failure tracking and `grace_failing`. This is the only +place that knows about grace periods and the reliability window, and it is +criterion-agnostic — the caller passes the values from the evaluator. `now` is a parameter +rather than a call to `datetime.now()` so runs are replayable and idempotent. +""" + +from dataclasses import dataclass, replace +from datetime import datetime, timedelta +from typing import Optional + +from tasks.seal_of_reliability.criteria import SealCriterionName +from tasks.seal_of_reliability.evaluators.base import RawEvaluation + + +@dataclass(frozen=True) +class SealCriterionState: + """One row of the sealcriterion table. + + raw_* fields describe the instantaneous state at the last evaluation, with no grace + applied. grace_failing is the debounced state that drives the seal outcome. + """ + + feed_id: str + criterion: SealCriterionName + raw_failing: Optional[bool] = None + grace_failing: Optional[bool] = None + evaluated_at: Optional[datetime] = None + first_raw_failure_at: Optional[datetime] = None + last_raw_failure_at: Optional[datetime] = None + last_grace_failure_at: Optional[datetime] = None + + +def transition( + prev: Optional[SealCriterionState], + raw: RawEvaluation, + grace_period: Optional[timedelta], + reliability_window: Optional[timedelta], + now: datetime, + feed_id: Optional[str] = None, +) -> Optional[SealCriterionState]: + """Apply one evaluation to a criterion's stored state. + + Returns the new state, or `prev` unchanged when the criterion was not evaluable + (`raw.failing is None`) — a missing input must never be read as a failure, otherwise an + upstream outage would revoke seals across the catalogue. + + Args: + prev: The stored state, or None if this criterion has never been evaluated. + raw: The evaluator's verdict for this run. + grace_period: How long a failure streak may last before it is confirmed. + None means a failure is confirmed immediately. + reliability_window: How long a confirmed failure keeps the criterion failing. + None means the criterion reflects the current state only, with no memory. + now: The evaluation timestamp. + feed_id: Required when `prev` is None, to build the first state. + """ + if raw.failing is None: + return prev + + resolved_feed_id = prev.feed_id if prev is not None else feed_id + if resolved_feed_id is None: + raise ValueError("feed_id is required when there is no previous state") + + base = prev or SealCriterionState(feed_id=resolved_feed_id, criterion=raw.criterion) + + if raw.failing: + # first_raw_failure_at marks the start of the current streak. It is cleared on + # recovery, so it is only None here at the start of a new streak. + first_raw_failure_at = base.first_raw_failure_at or now + last_raw_failure_at = now + last_grace_failure_at = base.last_grace_failure_at + if grace_period is None or now - first_raw_failure_at >= grace_period: + last_grace_failure_at = now + else: + # The streak ended, so the grace period resets. last_raw_failure_at and + # last_grace_failure_at are history and are never cleared. + first_raw_failure_at = None + last_raw_failure_at = base.last_raw_failure_at + last_grace_failure_at = base.last_grace_failure_at + + if reliability_window is None: + # No memory: the criterion tracks the current state and clears on recovery. + grace_failing = raw.failing + else: + grace_failing = ( + last_grace_failure_at is not None + and last_grace_failure_at >= now - reliability_window + ) + + return replace( + base, + raw_failing=raw.failing, + grace_failing=grace_failing, + evaluated_at=now, + first_raw_failure_at=first_raw_failure_at, + last_raw_failure_at=last_raw_failure_at, + last_grace_failure_at=last_grace_failure_at, + ) diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py new file mode 100644 index 000000000..731c2f71a --- /dev/null +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py @@ -0,0 +1,68 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Task entry point for the nightly Seal of Reliability evaluation (issue #1761).""" + +from datetime import datetime + +from tasks.seal_of_reliability.seal_updater import DEFAULT_BATCH_SIZE, update_seals + + +def get_parameters(payload: dict): + """Read the task parameters from the payload, applying defaults.""" + now = payload.get("now") + return ( + payload.get("dry_run", True), + payload.get("stable_feed_ids", None), + payload.get("limit", None), + payload.get("criteria", None), + payload.get("batch_size", DEFAULT_BATCH_SIZE), + datetime.fromisoformat(now) if now else None, + ) + + +def update_seal_of_reliability_handler(payload: dict) -> dict: + """ + Handler for the nightly Seal of Reliability evaluation. + + Payload parameters: + dry_run (bool): Evaluate every feed and return the report without writing. + Default: True. + stable_feed_ids (list[str] | None): Evaluate only these feeds. Unknown ids raise. + When set, `evaluations` covers every criterion of those feeds. + Default: None (all eligible feeds). + limit (int | None): Cap the number of feeds evaluated. Default: no limit. + criteria (list[str] | None): Evaluate only these criteria. A partial set skips the + has_seal roll-up. Default: None (every implemented criterion). + batch_size (int): Feeds loaded per query batch. Default: 200. + now (str | None): ISO timestamp to evaluate against, for replays and backfills. + Default: current UTC time. + """ + ( + dry_run, + stable_feed_ids, + limit, + criteria, + batch_size, + now, + ) = get_parameters(payload) + return update_seals( + dry_run=dry_run, + stable_feed_ids=stable_feed_ids, + limit=limit, + criteria=criteria, + batch_size=batch_size, + now=now, + ) diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py new file mode 100644 index 000000000..df50d2f47 --- /dev/null +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py @@ -0,0 +1,302 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""End-to-end test of the Seal of Reliability task through the tasks_executor entry point. + +Unlike test_seal_updater_db.py, which calls `update_seals` directly, this drives the +function the way Cloud Scheduler does: a JSON payload posted to `tasks_executor`, dispatched +by name through the registry in main.py. That covers the layers the direct tests skip — +payload parsing, defaults, and the task being reachable by its registered name. + +The task resolves its own session from FEEDS_DATABASE_URL rather than taking a `db_url`, so +the environment is pointed at the test database for the duration of each test and the +Database singleton is reset around it. + +These runs are unnamed, so they evaluate every eligible feed in the test database — which +includes the fixtures seeded by conftest.pytest_sessionstart. Assertions are therefore +scoped to this module's own PREFIX, and the report counts are checked as invariants and +deltas rather than absolutes. +""" + +import os +import unittest +from datetime import datetime, timedelta, timezone +from unittest.mock import MagicMock, patch + +import flask +from main import tasks_executor +from sqlalchemy import delete, select + +from shared.database.database import with_db_session +from shared.database_gen.sqlacodegen_models import ( + Feed, + Feedreliabilityseal, + Gtfsfeed, + Sealcriterion, +) +from test_shared.test_utils.database_utils import default_db_url, reset_database_class + +NOW = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc) +PREFIX = "seal_e2e_" + +OFFICIAL = f"{PREFIX}official" +NOT_OFFICIAL = f"{PREFIX}not_official" +UNKNOWN_OFFICIAL = f"{PREFIX}unknown_official" +DEPRECATED = f"{PREFIX}deprecated" +UNPUBLISHED = f"{PREFIX}unpublished" + + +def _seed(db_session, feed_id, official=True, status="active", operational="published"): + db_session.add( + Gtfsfeed( + id=feed_id, + stable_id=feed_id, + data_type="gtfs", + status=status, + operational_status=operational, + official=official, + created_at=NOW - timedelta(days=400), + producer_url=f"https://example.com/{feed_id}.zip", + ) + ) + db_session.flush() + + +def _cleanup(db_session): + """Deleting the parent Feed cascades to gtfsfeed and both seal tables.""" + db_session.execute(delete(Feed).where(Feed.stable_id.like(f"{PREFIX}%"))) + db_session.commit() + + +class TestSealTaskEndToEnd(unittest.TestCase): + """Seed, run through the entry point, inspect, modify, run again, inspect.""" + + @with_db_session(db_url=default_db_url) + def setUp(self, db_session): + _cleanup(db_session) + _seed(db_session, OFFICIAL) + _seed(db_session, NOT_OFFICIAL, official=False) + _seed(db_session, UNKNOWN_OFFICIAL, official=None) + _seed(db_session, DEPRECATED, status="deprecated") + _seed(db_session, UNPUBLISHED, operational="unpublished") + db_session.commit() + self.app = flask.Flask(__name__) + + @with_db_session(db_url=default_db_url) + def tearDown(self, db_session): + _cleanup(db_session) + reset_database_class() + + def run_task(self, payload: dict) -> dict: + """Invoke tasks_executor with a hand-built request, as Cloud Scheduler would. + + FEEDS_DATABASE_URL is pointed at the test database because the handler resolves its + own session; without this the task would run against the local development DB. + """ + request = MagicMock(spec=flask.Request) + request.get_json.return_value = { + "task": "update_seal_of_reliability", + "payload": payload, + } + request.headers = {} + + reset_database_class() + with patch.dict(os.environ, {"FEEDS_DATABASE_URL": default_db_url}): + with self.app.app_context(): + response = tasks_executor(request) + reset_database_class() + + self.assertEqual(response.status_code, 200, response.get_data(as_text=True)) + return response.get_json() + + @staticmethod + def ours(report: dict) -> list: + """The report's evaluations for this module's feeds only. + + Unnamed runs also cover the conftest fixtures, so filtering keeps these assertions + independent of what else lives in the test database. + """ + return [ + row for row in report["evaluations"] if row["stable_id"].startswith(PREFIX) + ] + + @staticmethod + @with_db_session(db_url=default_db_url) + def seal_state(db_session): + """stable_id -> (has_seal, earned_at set?, lost_at set?) for the seeded feeds.""" + seal = Feedreliabilityseal.__table__ + rows = db_session.execute( + select( + Feed.stable_id, + seal.c.has_seal, + seal.c.seal_earned_at, + seal.c.seal_lost_at, + ) + .join(seal, seal.c.feed_id == Feed.id) + .where(Feed.stable_id.like(f"{PREFIX}%")) + ).all() + return { + row.stable_id: ( + row.has_seal, + row.seal_earned_at is not None, + row.seal_lost_at is not None, + ) + for row in rows + } + + @staticmethod + @with_db_session(db_url=default_db_url) + def criterion_state(db_session): + """stable_id -> the sealcriterion row, for the seeded feeds.""" + criterion = Sealcriterion.__table__ + rows = db_session.execute( + select(Feed.stable_id, criterion) + .join(criterion, criterion.c.feed_id == Feed.id) + .where(Feed.stable_id.like(f"{PREFIX}%")) + ).all() + return {row.stable_id: row for row in rows} + + @staticmethod + @with_db_session(db_url=default_db_url) + def set_official(stable_id, official, db_session): + db_session.execute( + Feed.__table__.update() + .where(Feed.__table__.c.stable_id == stable_id) + .values(official=official) + ) + db_session.commit() + + def test_dry_run_through_the_entry_point_writes_nothing(self): + report = self.run_task({"dry_run": True, "now": NOW.isoformat()}) + self.assertTrue(report["dry_run"]) + self.assertGreaterEqual(report["total_feeds"], 3) + self.assertEqual(report["criterion_rows_written"], 0) + self.assertEqual(self.seal_state(), {}, "nothing written for our feeds") + self.assertEqual(self.criterion_state(), {}) + + def test_dry_run_is_the_default_when_the_payload_omits_it(self): + """The safety default has to survive the payload layer, not just the function.""" + report = self.run_task({"now": NOW.isoformat()}) + self.assertTrue(report["dry_run"]) + self.assertEqual(self.seal_state(), {}) + + def test_two_runs_with_a_change_in_between(self): + # --- first run: writes the initial state + first = self.run_task({"dry_run": False, "now": NOW.isoformat()}) + self.assertFalse(first["dry_run"]) + self.assertGreaterEqual(first["criterion_rows_written"], 3) + self.assertEqual(first["seals_revoked"], 0, "nothing was held beforehand") + self.assertEqual( + {row["stable_id"] for row in self.ours(first)}, + {NOT_OFFICIAL, UNKNOWN_OFFICIAL}, + "a first evaluation is reported only when it lands on a failure", + ) + + # --- inspect the database + self.assertEqual( + self.seal_state(), + { + OFFICIAL: (True, True, False), + NOT_OFFICIAL: (False, False, False), + UNKNOWN_OFFICIAL: (False, False, False), + }, + "only the official feed earned it; the others were never granted or lost", + ) + criteria = self.criterion_state() + self.assertNotIn(DEPRECATED, criteria) + self.assertNotIn(UNPUBLISHED, criteria) + self.assertFalse(criteria[OFFICIAL].grace_failing) + self.assertIsNone(criteria[OFFICIAL].first_raw_failure_at) + self.assertTrue(criteria[NOT_OFFICIAL].grace_failing) + self.assertEqual(criteria[NOT_OFFICIAL].first_raw_failure_at, NOW) + + # --- modify the database: revoke one, recover another + self.set_official(OFFICIAL, False) + self.set_official(NOT_OFFICIAL, True) + + # --- second run + later = NOW + timedelta(days=1) + second = self.run_task({"dry_run": False, "now": later.isoformat()}) + self.assertEqual(second["first_evaluations"], 0, "every feed already had a row") + self.assertEqual(second["seals_granted"], 1) + self.assertEqual(second["seals_revoked"], 1) + self.assertIn(OFFICIAL, second["revoked_stable_ids"]) + self.assertEqual( + second["seals_before_run"] + + second["seals_granted"] + - second["seals_revoked"], + second["seals_after_run"], + "before + granted - revoked == after", + ) + + moved = {row["stable_id"]: row for row in self.ours(second)} + self.assertEqual(set(moved), {OFFICIAL, NOT_OFFICIAL}, "only these two moved") + self.assertTrue(moved[OFFICIAL]["grace_failing"]) + self.assertFalse(moved[OFFICIAL]["previously_grace_failing"]) + self.assertFalse(moved[NOT_OFFICIAL]["grace_failing"]) + self.assertTrue(moved[NOT_OFFICIAL]["previously_grace_failing"]) + + # --- inspect again: both transitions are recorded, history is preserved + self.assertEqual( + self.seal_state(), + { + OFFICIAL: (False, True, True), + NOT_OFFICIAL: (True, True, False), + UNKNOWN_OFFICIAL: (False, False, False), + }, + "the revoked feed keeps its earned_at; the recovered one gains earned_at", + ) + criteria = self.criterion_state() + self.assertEqual(criteria[OFFICIAL].first_raw_failure_at, later) + self.assertIsNone( + criteria[NOT_OFFICIAL].first_raw_failure_at, "the streak ended" + ) + self.assertEqual( + criteria[NOT_OFFICIAL].last_raw_failure_at, + NOW, + "history is never cleared, so the old failure time survives recovery", + ) + + def test_third_run_with_no_change_is_a_no_op(self): + self.run_task({"dry_run": False, "now": NOW.isoformat()}) + before = self.criterion_state() + + later = NOW + timedelta(days=2) + report = self.run_task({"dry_run": False, "now": later.isoformat()}) + self.assertEqual(self.ours(report), [], "none of our feeds moved") + self.assertEqual(report["first_evaluations"], 0) + self.assertEqual(report["seals_before_run"], report["seals_after_run"]) + + after = self.criterion_state() + for stable_id, row in after.items(): + with self.subTest(stable_id=stable_id): + self.assertEqual( + row.first_raw_failure_at, + before[stable_id].first_raw_failure_at, + "a re-evaluation must not restart a failure streak", + ) + self.assertEqual(row.evaluated_at, later, "but it is re-evaluated") + + def test_unknown_task_name_is_rejected(self): + request = MagicMock(spec=flask.Request) + request.get_json.return_value = {"task": "no_such_seal_task", "payload": {}} + request.headers = {} + with self.app.app_context(): + response = tasks_executor(request) + self.assertEqual(response.status_code, 400) + + +if __name__ == "__main__": + unittest.main() diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py new file mode 100644 index 000000000..276a13dac --- /dev/null +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py @@ -0,0 +1,90 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Unit tests for the seal criterion evaluators. No database.""" + +import unittest +from datetime import datetime, timezone + +from tasks.seal_of_reliability.context import FeedSealContext +from tasks.seal_of_reliability.criteria import SealCriterionName +from tasks.seal_of_reliability.evaluators import ( + EVALUATORS, + CriterionEvaluator, + OfficialEvaluator, +) + +NOW = datetime(2026, 6, 1, tzinfo=timezone.utc) + + +def _ctx(**overrides) -> FeedSealContext: + defaults = {"feed_id": "feed-1", "now": NOW, "stable_id": "mdb-1"} + defaults.update(overrides) + return FeedSealContext(**defaults) + + +class TestBaseClass(unittest.TestCase): + def test_subclass_must_implement_evaluate(self): + class Incomplete(CriterionEvaluator): + name = SealCriterionName.OFFICIAL + + with self.assertRaises(NotImplementedError): + Incomplete().evaluate(_ctx()) + + def test_result_is_labelled_with_the_evaluator_name(self): + for evaluator in EVALUATORS: + with self.subTest(criterion=evaluator.name): + result = evaluator.evaluate(_ctx(official=True)) + self.assertEqual(result.criterion, evaluator.name) + + def test_every_result_carries_a_reason(self): + for evaluator in EVALUATORS: + with self.subTest(criterion=evaluator.name): + self.assertTrue(evaluator.evaluate(_ctx()).reason) + + def test_registry_has_no_duplicate_criteria(self): + names = [evaluator.name for evaluator in EVALUATORS] + self.assertEqual(sorted(names), sorted(set(names))) + + def test_registry_only_uses_known_criteria(self): + """Every registered evaluator must map onto a value of the DB enum.""" + for evaluator in EVALUATORS: + with self.subTest(criterion=evaluator.name): + self.assertIn(evaluator.name, set(SealCriterionName)) + + +class TestOfficial(unittest.TestCase): + def test_official_feed_passes(self): + self.assertFalse(OfficialEvaluator().evaluate(_ctx(official=True)).failing) + + def test_non_official_feed_fails(self): + self.assertTrue(OfficialEvaluator().evaluate(_ctx(official=False)).failing) + + def test_unknown_official_flag_fails(self): + """NULL is not an endorsement: only an explicit True passes.""" + self.assertTrue(OfficialEvaluator().evaluate(_ctx(official=None)).failing) + + def test_has_no_grace_or_window(self): + """A point-in-time check: it clears as soon as the feed is official again.""" + self.assertIsNone(OfficialEvaluator.grace_period) + self.assertIsNone(OfficialEvaluator.reliability_window) + + def test_reason_names_the_offending_value(self): + result = OfficialEvaluator().evaluate(_ctx(official=None)) + self.assertIn("None", result.reason) + + +if __name__ == "__main__": + unittest.main() diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py new file mode 100644 index 000000000..e87d0ec76 --- /dev/null +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py @@ -0,0 +1,191 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Unit tests for the generic seal criterion state machine. No database.""" + +import unittest +from datetime import datetime, timedelta, timezone + +from tasks.seal_of_reliability.criteria import RELIABILITY_WINDOW, SealCriterionName +from tasks.seal_of_reliability.evaluators.base import RawEvaluation +from tasks.seal_of_reliability.state_machine import SealCriterionState, transition + +DAY_ZERO = datetime(2026, 1, 1, tzinfo=timezone.utc) +FEED_ID = "feed-1" + +# Official, the first criterion implemented, has neither a grace period nor a reliability +# window, so it exercises almost none of the state machine. These tests drive a synthetic +# criterion that has both. +GRACE = timedelta(days=14) + + +def _raw(failing, criterion=SealCriterionName.AVAILABLE): + return RawEvaluation(criterion=criterion, failing=failing, reason="test") + + +def _run(days, grace_period=GRACE, reliability_window=RELIABILITY_WINDOW, state=None): + """Apply one verdict per entry in `days`: (day offset, failing).""" + for offset, failing in days: + state = transition( + prev=state, + raw=_raw(failing), + grace_period=grace_period, + reliability_window=reliability_window, + now=DAY_ZERO + timedelta(days=offset), + feed_id=FEED_ID, + ) + return state + + +class TestNotEvaluable(unittest.TestCase): + def test_none_verdict_leaves_state_untouched(self): + """A criterion that cannot be evaluated must not change anything.""" + state = _run([(0, True)]) + unchanged = transition( + prev=state, + raw=_raw(None), + grace_period=GRACE, + reliability_window=RELIABILITY_WINDOW, + now=DAY_ZERO + timedelta(days=1), + ) + self.assertIs(unchanged, state) + + def test_none_verdict_with_no_previous_state_stays_none(self): + self.assertIsNone( + transition( + prev=None, + raw=_raw(None), + grace_period=GRACE, + reliability_window=RELIABILITY_WINDOW, + now=DAY_ZERO, + feed_id=FEED_ID, + ) + ) + + def test_missing_feed_id_without_previous_state_raises(self): + with self.assertRaises(ValueError): + transition( + prev=None, + raw=_raw(True), + grace_period=GRACE, + reliability_window=RELIABILITY_WINDOW, + now=DAY_ZERO, + ) + + +class TestGracePeriod(unittest.TestCase): + def test_first_failure_is_not_confirmed(self): + state = _run([(0, True)]) + self.assertTrue(state.raw_failing) + self.assertFalse(state.grace_failing) + self.assertEqual(state.first_raw_failure_at, DAY_ZERO) + self.assertEqual(state.last_raw_failure_at, DAY_ZERO) + self.assertIsNone(state.last_grace_failure_at) + + def test_failure_within_grace_is_not_confirmed(self): + state = _run([(day, True) for day in range(0, 14)]) + self.assertTrue(state.raw_failing) + self.assertFalse(state.grace_failing) + self.assertIsNone(state.last_grace_failure_at) + + def test_failure_at_grace_expiry_is_confirmed(self): + state = _run([(day, True) for day in range(0, 15)]) + self.assertTrue(state.grace_failing) + self.assertEqual(state.last_grace_failure_at, DAY_ZERO + timedelta(days=14)) + + def test_recovery_within_grace_resets_the_streak(self): + state = _run([(0, True), (1, True), (2, False)]) + self.assertFalse(state.raw_failing) + self.assertFalse(state.grace_failing) + self.assertIsNone(state.first_raw_failure_at) + # History is kept. + self.assertEqual(state.last_raw_failure_at, DAY_ZERO + timedelta(days=1)) + + def test_streak_restarts_after_recovery(self): + """A new streak gets a full grace period, it does not resume the old one.""" + days = [(day, True) for day in range(0, 10)] + days += [(10, False)] + days += [(day, True) for day in range(11, 20)] + state = _run(days) + self.assertTrue(state.raw_failing) + self.assertFalse(state.grace_failing) + self.assertEqual(state.first_raw_failure_at, DAY_ZERO + timedelta(days=11)) + + def test_no_grace_period_confirms_immediately(self): + state = _run([(0, True)], grace_period=None) + self.assertTrue(state.grace_failing) + self.assertEqual(state.last_grace_failure_at, DAY_ZERO) + + +class TestReliabilityWindow(unittest.TestCase): + def test_confirmed_failure_persists_after_recovery(self): + days = [(day, True) for day in range(0, 15)] + [(15, False)] + state = _run(days) + self.assertFalse(state.raw_failing) + self.assertTrue( + state.grace_failing, "a confirmed failure must hold through the window" + ) + + def test_criterion_clears_once_the_window_passes(self): + days = [(day, True) for day in range(0, 15)] + days += [(15, False), (15 + RELIABILITY_WINDOW.days, False)] + self.assertFalse(_run(days).grace_failing) + + def test_criterion_still_failing_just_inside_the_window(self): + days = [(day, True) for day in range(0, 15)] + days += [(15, False), (14 + RELIABILITY_WINDOW.days, False)] + self.assertTrue(_run(days).grace_failing) + + def test_no_window_tracks_current_state_only(self): + """Official's shape: no grace, no window, clears as soon as the feed recovers.""" + state = _run( + [(0, True), (1, False)], grace_period=None, reliability_window=None + ) + self.assertFalse(state.grace_failing) + self.assertIsNotNone(state.last_grace_failure_at, "history is still recorded") + + +class TestIdempotency(unittest.TestCase): + def test_same_timestamp_twice_produces_the_same_state(self): + """A re-run for the same instant must not compound a failure streak.""" + once = _run([(0, True), (1, True)]) + twice = _run([(1, True)], state=once) + self.assertEqual(once.first_raw_failure_at, twice.first_raw_failure_at) + self.assertEqual(once.last_raw_failure_at, twice.last_raw_failure_at) + self.assertEqual(once.grace_failing, twice.grace_failing) + + def test_state_is_not_mutated_in_place(self): + state = _run([(0, True)]) + _run([(1, True)], state=state) + self.assertEqual(state.last_raw_failure_at, DAY_ZERO) + + +class TestStateShape(unittest.TestCase): + def test_state_carries_feed_id_and_criterion(self): + state = _run([(0, True)]) + self.assertEqual(state.feed_id, FEED_ID) + self.assertEqual(state.criterion, SealCriterionName.AVAILABLE) + + def test_state_defaults_are_all_none(self): + state = SealCriterionState( + feed_id=FEED_ID, criterion=SealCriterionName.OFFICIAL + ) + self.assertIsNone(state.raw_failing) + self.assertIsNone(state.grace_failing) + self.assertIsNone(state.evaluated_at) + + +if __name__ == "__main__": + unittest.main() diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py new file mode 100644 index 000000000..712ad7674 --- /dev/null +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py @@ -0,0 +1,350 @@ +# +# MobilityData 2026 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Integration tests for the seal context loader and orchestrator, against the test DB.""" + +import unittest +from datetime import datetime, timedelta, timezone + +from tasks.seal_of_reliability.context import build_contexts, get_seal_feeds_query +from tasks.seal_of_reliability.criteria import SealCriterionName +from tasks.seal_of_reliability.seal_updater import update_seals +from sqlalchemy import delete, select + +from shared.database.database import with_db_session +from shared.database_gen.sqlacodegen_models import ( + Feed, + Feedreliabilityseal, + Gtfsfeed, + Sealcriterion, +) +from test_shared.test_utils.database_utils import default_db_url + +NOW = datetime(2026, 6, 1, 12, 0, tzinfo=timezone.utc) +PREFIX = "seal_test_" + +OFFICIAL = f"{PREFIX}official" +NOT_OFFICIAL = f"{PREFIX}not_official" +UNKNOWN_OFFICIAL = f"{PREFIX}unknown_official" +DEPRECATED = f"{PREFIX}deprecated" +UNPUBLISHED = f"{PREFIX}unpublished" +INACTIVE = f"{PREFIX}inactive" + + +def _seed_feed( + db_session, + feed_id: str, + official=True, + status="active", + operational_status="published", +): + """Insert one GTFS feed.""" + db_session.add( + Gtfsfeed( + id=feed_id, + stable_id=feed_id, + data_type="gtfs", + status=status, + operational_status=operational_status, + official=official, + created_at=NOW - timedelta(days=400), + producer_url=f"https://example.com/{feed_id}.zip", + ) + ) + db_session.flush() + + +def _set_official(db_session, feed_id: str, official): + db_session.execute( + Feed.__table__.update() + .where(Feed.__table__.c.id == feed_id) + .values(official=official) + ) + db_session.commit() + + +def _cleanup(db_session): + """Remove every row this module created. + + Deleting from `feed` rather than `gtfsfeed` is deliberate: Gtfsfeed is a joined-table + subclass of Feed, so deleting the subclass leaves the parent row behind and the next + insert collides on feed_pkey. Deleting the parent cascades to gtfsfeed and to both seal + tables, all of which are ON DELETE CASCADE. + """ + db_session.execute(delete(Feed).where(Feed.stable_id.like(f"{PREFIX}%"))) + db_session.commit() + + +class SealDbTestCase(unittest.TestCase): + """Seeds the feeds below before each test and removes them afterwards.""" + + @with_db_session(db_url=default_db_url) + def setUp(self, db_session): + _cleanup(db_session) + _seed_feed(db_session, OFFICIAL) + _seed_feed(db_session, NOT_OFFICIAL, official=False) + _seed_feed(db_session, UNKNOWN_OFFICIAL, official=None) + _seed_feed(db_session, DEPRECATED, status="deprecated") + _seed_feed(db_session, UNPUBLISHED, operational_status="unpublished") + _seed_feed(db_session, INACTIVE, status="inactive") + db_session.commit() + + @with_db_session(db_url=default_db_url) + def tearDown(self, db_session): + _cleanup(db_session) + + @staticmethod + @with_db_session(db_url=default_db_url) + def criterion_rows(feed_id, db_session): + table = Sealcriterion.__table__ + return { + row.criterion: row + for row in db_session.execute( + select(table).where(table.c.feed_id == feed_id) + ).all() + } + + @staticmethod + @with_db_session(db_url=default_db_url) + def seal_row(feed_id, db_session): + table = Feedreliabilityseal.__table__ + return db_session.execute( + select(table).where(table.c.feed_id == feed_id) + ).first() + + @staticmethod + @with_db_session(db_url=default_db_url) + def set_official(feed_id, official, db_session): + _set_official(db_session, feed_id, official) + + +class TestEligibilityQuery(SealDbTestCase): + @with_db_session(db_url=default_db_url) + def test_excludes_deprecated_and_unpublished_but_keeps_inactive(self, db_session): + found = { + feed.stable_id + for feed in get_seal_feeds_query(db_session).all() + if feed.stable_id and feed.stable_id.startswith(PREFIX) + } + self.assertIn(OFFICIAL, found) + self.assertIn(INACTIVE, found, "inactive feeds must be evaluated, not frozen") + self.assertNotIn(DEPRECATED, found) + self.assertNotIn(UNPUBLISHED, found) + + @with_db_session(db_url=default_db_url) + def test_stable_feed_ids_narrows_the_same_query(self, db_session): + feeds = get_seal_feeds_query(db_session, stable_feed_ids=[OFFICIAL]).all() + self.assertEqual([feed.stable_id for feed in feeds], [OFFICIAL]) + + +class TestBuildContexts(SealDbTestCase): + @with_db_session(db_url=default_db_url) + def test_loads_the_fields_the_evaluators_need(self, db_session): + feeds = get_seal_feeds_query(db_session, stable_feed_ids=[OFFICIAL]).all() + ctx = build_contexts(db_session, feeds, NOW)[feeds[0].id] + self.assertEqual(ctx.stable_id, OFFICIAL) + self.assertTrue(ctx.official) + self.assertEqual(ctx.now, NOW) + + @with_db_session(db_url=default_db_url) + def test_builds_one_context_per_feed(self, db_session): + feeds = get_seal_feeds_query( + db_session, stable_feed_ids=[OFFICIAL, NOT_OFFICIAL] + ).all() + contexts = build_contexts(db_session, feeds, NOW) + self.assertEqual(len(contexts), 2) + self.assertEqual({ctx.official for ctx in contexts.values()}, {True, False}) + + +class TestUpdateSeals(SealDbTestCase): + def test_dry_run_writes_nothing(self): + report = update_seals(dry_run=True, stable_feed_ids=[OFFICIAL], now=NOW) + self.assertTrue(report["dry_run"]) + self.assertEqual(report["total_feeds"], 1) + self.assertEqual(report["criterion_rows_written"], 0) + self.assertEqual(self.criterion_rows(OFFICIAL), {}) + self.assertIsNone(self.seal_row(OFFICIAL)) + + def test_dry_run_counts_are_prospective(self): + """Nothing is held yet, so `after` describes what a real run would store.""" + report = update_seals(dry_run=True, now=NOW) + self.assertEqual(report["seals_before_run"], 0) + self.assertEqual(report["seals_after_run"], 2, "the two official feeds") + self.assertEqual(report["seals_granted"], 2) + self.assertEqual(report["seals_revoked"], 0) + self.assertIsNone(self.seal_row(OFFICIAL), "still a dry run") + + def test_seal_counts_balance(self): + """before + granted - revoked == after, across a grant then a revocation.""" + first = update_seals(dry_run=False, now=NOW) + self.assertEqual(first["seals_before_run"], 0) + self.assertEqual(first["seals_after_run"], 2) + + self.set_official(OFFICIAL, False) + second = update_seals(dry_run=False, now=NOW) + self.assertEqual(second["seals_before_run"], 2, "two were held going in") + self.assertEqual(second["seals_revoked"], 1) + self.assertEqual(second["seals_granted"], 0) + self.assertEqual(second["seals_after_run"], 1) + self.assertEqual( + second["seals_before_run"] + + second["seals_granted"] + - second["seals_revoked"], + second["seals_after_run"], + ) + + def test_dry_run_reports_rows_for_a_named_feed(self): + report = update_seals(dry_run=True, stable_feed_ids=[NOT_OFFICIAL], now=NOW) + self.assertEqual( + [row["criterion"] for row in report["evaluations"]], + [SealCriterionName.OFFICIAL.value], + ) + self.assertTrue(report["evaluations"][0]["raw_failing"]) + self.assertTrue(report["evaluations"][0]["reason"]) + self.assertIsNone(report["evaluations"][0]["previously_grace_failing"]) + + def test_named_feed_reports_a_row_even_when_nothing_moved(self): + """An explicit feed list is the debugging path: report all of its criteria.""" + update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) + report = update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) + self.assertEqual(len(report["evaluations"]), 1) + self.assertFalse(report["evaluations"][0]["grace_failing"]) + self.assertFalse(report["evaluations"][0]["previously_grace_failing"]) + + def test_unnamed_run_reports_only_criteria_that_moved(self): + """A passing feed evaluated twice contributes no entry the second time.""" + first = update_seals(dry_run=False, now=NOW) + self.assertTrue( + any(row["stable_id"] == NOT_OFFICIAL for row in first["evaluations"]), + "a first evaluation that lands on a failure is reported", + ) + self.assertFalse( + any(row["stable_id"] == OFFICIAL for row in first["evaluations"]), + "a first evaluation that passes is covered by first_evaluations", + ) + self.assertGreaterEqual(first["first_evaluations"], 2) + + steady = update_seals(dry_run=False, now=NOW) + self.assertEqual( + steady["evaluations"], [], "nothing moved, so nothing to report" + ) + self.assertEqual(steady["first_evaluations"], 0) + + def test_a_criterion_that_flips_is_reported_with_its_previous_value(self): + update_seals(dry_run=False, now=NOW) + self.set_official(OFFICIAL, False) + report = update_seals(dry_run=False, now=NOW) + + moved = [row for row in report["evaluations"] if row["stable_id"] == OFFICIAL] + self.assertEqual(len(moved), 1) + self.assertTrue(moved[0]["grace_failing"]) + self.assertFalse(moved[0]["previously_grace_failing"]) + + def test_official_feed_earns_the_seal(self): + update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) + row = self.criterion_rows(OFFICIAL)[SealCriterionName.OFFICIAL.value] + self.assertFalse(row.raw_failing) + self.assertFalse(row.grace_failing) + self.assertEqual(row.evaluated_at, NOW) + self.assertIsNone(row.first_raw_failure_at) + + seal = self.seal_row(OFFICIAL) + self.assertTrue(seal.has_seal) + self.assertEqual(seal.seal_earned_at, NOW) + self.assertIsNone(seal.seal_lost_at) + + def test_failing_official_denies_the_seal_immediately(self): + """Official has no grace period, so one failure is confirmed at once.""" + update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) + row = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] + self.assertTrue(row.raw_failing) + self.assertTrue(row.grace_failing) + self.assertEqual(row.first_raw_failure_at, NOW) + self.assertEqual(row.last_grace_failure_at, NOW) + self.assertFalse(self.seal_row(NOT_OFFICIAL).has_seal) + + def test_unknown_official_flag_denies_the_seal(self): + update_seals(dry_run=False, stable_feed_ids=[UNKNOWN_OFFICIAL], now=NOW) + self.assertFalse(self.seal_row(UNKNOWN_OFFICIAL).has_seal) + + def test_rerun_is_idempotent(self): + update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) + first = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] + update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) + second = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] + self.assertEqual(first.first_raw_failure_at, second.first_raw_failure_at) + self.assertEqual(first.last_grace_failure_at, second.last_grace_failure_at) + + def test_losing_official_status_revokes_the_seal(self): + update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) + self.assertTrue(self.seal_row(OFFICIAL).has_seal) + + self.set_official(OFFICIAL, False) + later = NOW + timedelta(days=1) + update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=later) + seal = self.seal_row(OFFICIAL) + self.assertFalse(seal.has_seal) + self.assertEqual(seal.seal_lost_at, later) + self.assertEqual(seal.seal_earned_at, NOW, "the earlier grant is preserved") + + def test_regaining_official_status_clears_the_criterion(self): + """No reliability window, so recovery is immediate rather than 6 months later.""" + update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) + self.assertFalse(self.seal_row(NOT_OFFICIAL).has_seal) + + self.set_official(NOT_OFFICIAL, True) + later = NOW + timedelta(days=1) + update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=later) + row = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] + self.assertFalse(row.grace_failing) + self.assertIsNone(row.first_raw_failure_at, "the streak is cleared") + self.assertEqual( + row.last_grace_failure_at, NOW, "the failure is still on record" + ) + seal = self.seal_row(NOT_OFFICIAL) + self.assertTrue(seal.has_seal) + self.assertEqual(seal.seal_earned_at, later) + + def test_partial_criteria_run_skips_the_roll_up(self): + """Named explicitly, `official` is still the whole registry, so not partial.""" + report = update_seals( + dry_run=False, + stable_feed_ids=[OFFICIAL], + criteria=[SealCriterionName.OFFICIAL.value], + now=NOW, + ) + self.assertFalse(report["partial_run"]) + self.assertTrue(self.seal_row(OFFICIAL).has_seal) + + def test_unknown_criterion_raises(self): + with self.assertRaises(ValueError): + update_seals(criteria=["not_a_criterion"], now=NOW) + + def test_criterion_without_an_evaluator_raises(self): + """`stable` is a valid DB enum value but has no evaluator yet (#1784).""" + with self.assertRaises(ValueError): + update_seals(criteria=[SealCriterionName.STABLE.value], now=NOW) + + def test_unknown_stable_feed_id_raises(self): + with self.assertRaises(ValueError): + update_seals(stable_feed_ids=[f"{PREFIX}does_not_exist"], now=NOW) + + def test_limit_caps_the_feeds_evaluated(self): + report = update_seals(dry_run=True, limit=2, now=NOW) + self.assertLessEqual(report["total_feeds"], 2) + + +if __name__ == "__main__": + unittest.main() From 9480323a3ca8a6cb3539c9db5708fb63ee68c1cf Mon Sep 17 00:00:00 2001 From: jcpitre Date: Thu, 6 Aug 2026 10:09:22 -0400 Subject: [PATCH 05/10] Cascade seal rows on feed delete --- api/src/shared/database/database.py | 1 + .../cascade_delete/test_cascade_delete.py | 51 +++++++++++++++++++ .../test_seal_updater_db.py | 11 ++-- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/api/src/shared/database/database.py b/api/src/shared/database/database.py index 6652d108c..8ddb4e95e 100644 --- a/api/src/shared/database/database.py +++ b/api/src/shared/database/database.py @@ -77,6 +77,7 @@ def configure_polymorphic_mappers(): Feed.redirectingids, # redirectingid_source_id_fkey Feed.redirectingids_, # redirectingid_target_id_fkey Feed.feed_license_changes, + Feed.sealcriteria, # sealcriterion_feed_id_fkey ], Gtfsfeed: [Gtfsfeed.gtfs_dataset_changelogs], Gbfsfeed: [ diff --git a/api/tests/integration/cascade_delete/test_cascade_delete.py b/api/tests/integration/cascade_delete/test_cascade_delete.py index dee5cd89a..8b8532a3d 100644 --- a/api/tests/integration/cascade_delete/test_cascade_delete.py +++ b/api/tests/integration/cascade_delete/test_cascade_delete.py @@ -26,6 +26,7 @@ Gtfsfeed, Geopolygon, FeedLicenseChange, + Sealcriterion, ) from sqlalchemy import text @@ -207,6 +208,56 @@ def test_delete_feed_cascadeto_feed_license_changes(test_database): ) +def test_delete_feed_cascadeto_sealcriterion(test_database): + """Deleting a feed removes its per-criterion seal state (issue #1760). + + This exercises the `Feed.sealcriteria` entry in `cascade_entities`: without it, + SQLAlchemy would try to NULL `sealcriterion.feed_id`, which is NOT NULL and part of the + composite primary key, instead of letting ON DELETE CASCADE do the work. + """ + + with test_database.start_db_session() as session: + feed = Feed(id="f1") + seal_criterion = Sealcriterion(feed_id="f1", criterion="official") + session.add_all([feed, seal_criterion]) + session.commit() + + delete_and_assert( + session, + [ + "SELECT COUNT(*) FROM feed where id = 'f1'", + "SELECT COUNT(*) FROM sealcriterion where feed_id = 'f1'", + ], + feed, + ) + + +def test_delete_feed_cascadeto_feedreliabilityseal(test_database): + """Deleting a feed removes its overall seal row (issue #1760). + + The row is inserted with raw SQL rather than the ORM: `feedreliabilityseal.feed_id` is + both its primary key and a foreign key to `feed.id`, so sqlacodegen maps the table as a + joined-table subclass of Feed. Adding a `Feedreliabilityseal()` instance would therefore + try to insert a second feed row rather than a child row. + """ + + with test_database.start_db_session() as session: + feed = Feed(id="f1") + session.add(feed) + session.commit() + session.execute(text("INSERT INTO feedreliabilityseal (feed_id) VALUES ('f1')")) + session.commit() + + delete_and_assert( + session, + [ + "SELECT COUNT(*) FROM feed where id = 'f1'", + "SELECT COUNT(*) FROM feedreliabilityseal where feed_id = 'f1'", + ], + feed, + ) + + def test_delete_gbfsfeed_cascadeto_gbfsversion_cascadeto_gbfsendpoint_cascadeto_gbfsendpointhttpaccesslog( test_database, ): diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py index 712ad7674..d48edc181 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py @@ -42,6 +42,11 @@ UNPUBLISHED = f"{PREFIX}unpublished" INACTIVE = f"{PREFIX}inactive" +# The eligible feeds this module seeds. Runs that assert exact counts must be scoped to +# these: an unnamed run also covers the fixtures seeded by conftest.pytest_sessionstart and +# any seal rows another test left behind, so the totals would not be deterministic. +OURS = [OFFICIAL, NOT_OFFICIAL, UNKNOWN_OFFICIAL, INACTIVE] + def _seed_feed( db_session, @@ -179,7 +184,7 @@ def test_dry_run_writes_nothing(self): def test_dry_run_counts_are_prospective(self): """Nothing is held yet, so `after` describes what a real run would store.""" - report = update_seals(dry_run=True, now=NOW) + report = update_seals(dry_run=True, stable_feed_ids=OURS, now=NOW) self.assertEqual(report["seals_before_run"], 0) self.assertEqual(report["seals_after_run"], 2, "the two official feeds") self.assertEqual(report["seals_granted"], 2) @@ -188,12 +193,12 @@ def test_dry_run_counts_are_prospective(self): def test_seal_counts_balance(self): """before + granted - revoked == after, across a grant then a revocation.""" - first = update_seals(dry_run=False, now=NOW) + first = update_seals(dry_run=False, stable_feed_ids=OURS, now=NOW) self.assertEqual(first["seals_before_run"], 0) self.assertEqual(first["seals_after_run"], 2) self.set_official(OFFICIAL, False) - second = update_seals(dry_run=False, now=NOW) + second = update_seals(dry_run=False, stable_feed_ids=OURS, now=NOW) self.assertEqual(second["seals_before_run"], 2, "two were held going in") self.assertEqual(second["seals_revoked"], 1) self.assertEqual(second["seals_granted"], 0) From b193c4c5b4f722325e38e3848bf5af3b1c87fda9 Mon Sep 17 00:00:00 2001 From: jcpitre Date: Tue, 11 Aug 2026 23:49:35 -0400 Subject: [PATCH 06/10] rework the reliability algorithm around probation and positive criterion status --- functions-python/tasks_executor/README.md | 11 +- .../src/tasks/seal_of_reliability/criteria.py | 13 +- .../evaluators/__init__.py | 7 +- .../seal_of_reliability/evaluators/base.py | 37 +-- .../evaluators/official.py | 11 +- .../tasks/seal_of_reliability/seal_updater.py | 125 +++++----- .../seal_of_reliability/state_machine.py | 178 ++++++++++---- .../test_seal_end_to_end_db.py | 26 +- .../test_seal_evaluators.py | 14 +- .../test_seal_state_machine.py | 224 +++++++++++------- .../test_seal_updater_db.py | 52 ++-- liquibase/changelog.xml | 2 + liquibase/changes/feat_1783.sql | 43 ++++ 13 files changed, 492 insertions(+), 251 deletions(-) create mode 100644 liquibase/changes/feat_1783.sql diff --git a/functions-python/tasks_executor/README.md b/functions-python/tasks_executor/README.md index 2cf2e392d..6994aac40 100644 --- a/functions-python/tasks_executor/README.md +++ b/functions-python/tasks_executor/README.md @@ -422,6 +422,15 @@ Eligible feeds are GTFS, `operational_status = published`, and `status NOT IN (d development)`. `inactive` and `future` feeds are deliberately included: skipping a feed freezes its stored rows rather than making it neutral. +Each criterion carries two independent pieces of state. `confirmed_pass` is the debounced +status: an observed failure only flips it once it outlasts the criterion's grace period, and +recovery clears it the same day. `probation_start` is the penalty served afterwards — 180 +days with no observed failure, counted from the day the criterion recovered, restarted by +any observed failure while it is running. A feed holds the seal when every criterion that +has ever produced a verdict is a confirmed pass and not on probation, so probation is a +penalty rather than an entry requirement: a feed that has never had a confirmed failure can +hold the seal from its first evaluation. + ```json { "task": "update_seal_of_reliability", @@ -455,7 +464,7 @@ freezes its stored rows rather than making it neutral. | `seals_granted` / `seals_revoked` | Transitions in this run. `before + granted - revoked == after` | | `revoked_stable_ids` | Feeds that lost the seal in this run | | `first_evaluations` | Criteria evaluated for the first time (no stored row yet) | -| `evaluations` | The notable outcomes: one entry per feed and criterion whose verdict moved, with `raw_failing`, `grace_failing`, `previously_grace_failing` and `reason`. A first evaluation appears only if it landed on a failure; the rest are counted by `first_evaluations`. When `stable_feed_ids` is set, every criterion of those feeds is included whether or not it moved | +| `evaluations` | The notable outcomes: one entry per feed and criterion whose verdict moved, with `observed_pass`, `confirmed_pass`, `previously_confirmed_pass`, `on_probation` and `reason`. A first evaluation appears only if it landed on a failure; the rest are counted by `first_evaluations`. When `stable_feed_ids` is set, every criterion of those feeds is included whether or not it moved | > Note: no Cloud Scheduler job is defined for this task yet — invoke it manually. Once > scheduled it should run after the daily `check_gtfs_feed_availability` job, since the diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py index c06bf245b..68c8f7dc6 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py @@ -43,7 +43,12 @@ class SealCriterionName(str, Enum): FRESH_CONTINUOUS = "fresh_continuous" -# A criterion that fails past its grace period stays failing until it has gone this long -# without another confirmed failure. It is the default for new evaluators; Official is -# exempt because it is a point-in-time state check (see OfficialEvaluator). -RELIABILITY_WINDOW: Final[timedelta] = timedelta(days=180) +# A criterion that recovers from a confirmed failure is put on probation: it must then go +# this long with no observed failure before it can contribute to the seal again. It is the +# default for new evaluators; Official is exempt because it is a point-in-time state check +# (see OfficialEvaluator). +# +# Probation is a penalty, not an entry requirement. It is opened by a recovery, and a first +# evaluation that passes is not a recovery, so a feed that has never had a confirmed failure +# can hold the seal from its very first evaluation. +PROBATION_PERIOD: Final[timedelta] = timedelta(days=180) diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py index a8851ec7f..3ddabbe94 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/__init__.py @@ -25,7 +25,10 @@ from typing import Final, List -from tasks.seal_of_reliability.evaluators.base import CriterionEvaluator, RawEvaluation +from tasks.seal_of_reliability.evaluators.base import ( + CriterionEvaluator, + CriterionObservation, +) from tasks.seal_of_reliability.evaluators.official import OfficialEvaluator EVALUATORS: Final[List[CriterionEvaluator]] = [ @@ -35,6 +38,6 @@ __all__ = [ "EVALUATORS", "CriterionEvaluator", + "CriterionObservation", "OfficialEvaluator", - "RawEvaluation", ] diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py index 2b0495f91..f11aef120 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/base.py @@ -20,20 +20,23 @@ from typing import Optional, Tuple from tasks.seal_of_reliability.context import FeedSealContext -from tasks.seal_of_reliability.criteria import RELIABILITY_WINDOW, SealCriterionName +from tasks.seal_of_reliability.criteria import PROBATION_PERIOD, SealCriterionName @dataclass(frozen=True) -class RawEvaluation: - """One criterion's verdict for one feed, with no grace applied. +class CriterionObservation: + """One criterion's own check for one feed, with no debouncing. - `failing` is tri-state: True and False are verdicts, None means the criterion could not - be evaluated this run (a missing availability check, a criterion that does not apply to - this feed) and the stored state must be left alone. + `observed_pass` is tri-state: True and False are verdicts, None means the criterion + could not be evaluated this run (a missing availability check, a criterion that does not + apply to this feed) and the stored state must be left alone. + + Stated positively, so an evaluator returns the same sense as the check it reads + (`success = TRUE`, `total_error = 0`) with no inversion in between. """ criterion: SealCriterionName - failing: Optional[bool] + observed_pass: Optional[bool] reason: str @@ -41,23 +44,29 @@ class CriterionEvaluator: """Evaluates one criterion against a pre-loaded feed context. Subclasses set `name` and, where they differ from the defaults, `grace_period` and - `reliability_window`, then implement `_evaluate`. They never touch the database: all - the data they need is on the context, loaded in bulk by `context.build_contexts`. + `probation_period`, then implement `_evaluate`. They never touch the database: all the + data they need is on the context, loaded in bulk by `context.build_contexts`. + + `grace_period` holds a passing status while an observed failure is still young. + `probation_period` is how long the criterion must go with no observed failure after + recovering from a confirmed failure. None on either means the criterion does not use it. """ name: SealCriterionName = None grace_period: Optional[timedelta] = None - reliability_window: Optional[timedelta] = RELIABILITY_WINDOW + probation_period: Optional[timedelta] = PROBATION_PERIOD - def evaluate(self, ctx: FeedSealContext) -> RawEvaluation: + def evaluate(self, ctx: FeedSealContext) -> CriterionObservation: """Evaluate the criterion and label the result with this evaluator's name. Labelling happens here rather than in the subclasses so an evaluator cannot disagree with its own `name`. """ - failing, reason = self._evaluate(ctx) - return RawEvaluation(criterion=self.name, failing=failing, reason=reason) + observed_pass, reason = self._evaluate(ctx) + return CriterionObservation( + criterion=self.name, observed_pass=observed_pass, reason=reason + ) def _evaluate(self, ctx: FeedSealContext) -> Tuple[Optional[bool], str]: - """Return (failing, reason) for this feed. Implemented by subclasses.""" + """Return (observed_pass, reason) for this feed. Implemented by subclasses.""" raise NotImplementedError("Subclasses should implement this method.") diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py index 63f0152f8..be5e07a24 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py @@ -26,15 +26,16 @@ class OfficialEvaluator(CriterionEvaluator): """`feed.official IS TRUE`. A point-in-time state check: official at the time of reviewing the dataset, with no - 6-month check. Both the grace period and the reliability window are None, so the - criterion clears as soon as the feed is flagged official again. + 6-month check. Both the grace period and the probation period are None, so the criterion + fails the same day the flag is lost and clears the same day it comes back, taking the + seal with it in both directions. """ name = SealCriterionName.OFFICIAL grace_period = None - reliability_window = None + probation_period = None def _evaluate(self, ctx: FeedSealContext) -> Tuple[Optional[bool], str]: if ctx.official: - return False, "feed is official" - return True, f"feed.official is {ctx.official!r}, expected True" + return True, "feed is official" + return False, f"feed.official is {ctx.official!r}, expected True" diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py index 892c2183e..05bffdb39 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py @@ -80,12 +80,13 @@ def _load_previous_states( (row.feed_id, row.criterion): SealCriterionState( feed_id=row.feed_id, criterion=SealCriterionName(row.criterion), - raw_failing=row.raw_failing, - grace_failing=row.grace_failing, + observed_pass=row.observed_pass, + confirmed_pass=row.confirmed_pass, evaluated_at=row.evaluated_at, - first_raw_failure_at=row.first_raw_failure_at, - last_raw_failure_at=row.last_raw_failure_at, - last_grace_failure_at=row.last_grace_failure_at, + first_observed_failure_at=row.first_observed_failure_at, + last_observed_failure_at=row.last_observed_failure_at, + last_confirmed_failure_at=row.last_confirmed_failure_at, + probation_start=row.probation_start, ) for row in rows } @@ -105,31 +106,33 @@ def _load_previous_seals( return {row.feed_id: bool(row.has_seal) for row in rows} -def _roll_up_has_seal( - states: Dict[str, SealCriterionState], - evaluator_count: int, - currently_held: bool, -) -> bool: - """True only when every criterion is explicitly not failing. - - `grace_failing is False` rather than `not grace_failing`: a criterion that has never - been evaluated is NULL, and NULL must not be read as a pass. A feed missing a row for - any criterion cannot qualify either. - - A grace period protects a seal the feed already holds; it cannot be used to earn one. - Without that distinction a feed being evaluated for the first time while already - failing would be handed the seal for the length of the grace period, which is the - opposite of what "14 days to fix it before disqualification" means. So granting - requires every criterion to pass right now, while keeping only requires that no - failure has been confirmed. +def _roll_up_has_seal(states: Dict[str, SealCriterionState]) -> bool: + """True when every criterion in service is a confirmed pass and not on probation. + + A criterion is *in service* once it has produced a verdict at any point + (`observed_pass is not None`). One that never has is skipped rather than counted as a + failure, which is what lets the seal be computed before every criterion has a data + source: a criterion whose source only starts collecting later sits at NULL until then + and simply is not part of the roll-up. + + That waiver is self-limiting because `transition` never writes NULL back, so a + criterion can only be skipped before its first verdict ever. Once it has produced one it + stays in the roll-up with its last verdict, and an upstream outage freezes it rather + than quietly waiving it. + + The non-empty guard stops the roll-up being vacuously true: "every criterion in service + qualifies" holds trivially for a feed nothing has ever been measured on. + + `confirmed_pass is True` rather than a bare truthiness test is defensive; within the + in-service set it cannot be NULL, since the two booleans are always written together. """ - if len(states) < evaluator_count: + in_service = [state for state in states.values() if state.observed_pass is not None] + if not in_service: return False - if not all(state.grace_failing is False for state in states.values()): - return False - if currently_held: - return True - return all(state.raw_failing is False for state in states.values()) + return all( + state.confirmed_pass is True and state.probation_start is None + for state in in_service + ) def _is_notable( @@ -145,10 +148,10 @@ def _is_notable( if state is None: return False if previous is None: - return bool(state.raw_failing) or bool(state.grace_failing) + return state.observed_pass is False or state.confirmed_pass is False return ( - state.raw_failing != previous.raw_failing - or state.grace_failing != previous.grace_failing + state.observed_pass != previous.observed_pass + or state.confirmed_pass != previous.confirmed_pass ) @@ -162,12 +165,13 @@ def _upsert_criteria( { "feed_id": state.feed_id, "criterion": state.criterion.value, - "raw_failing": state.raw_failing, - "grace_failing": state.grace_failing, + "observed_pass": state.observed_pass, + "confirmed_pass": state.confirmed_pass, "evaluated_at": state.evaluated_at, - "first_raw_failure_at": state.first_raw_failure_at, - "last_raw_failure_at": state.last_raw_failure_at, - "last_grace_failure_at": state.last_grace_failure_at, + "first_observed_failure_at": state.first_observed_failure_at, + "last_observed_failure_at": state.last_observed_failure_at, + "last_confirmed_failure_at": state.last_confirmed_failure_at, + "probation_start": state.probation_start, "updated_at": now, } for state in states @@ -177,12 +181,13 @@ def _upsert_criteria( statement.on_conflict_do_update( index_elements=[CRITERION_TABLE.c.feed_id, CRITERION_TABLE.c.criterion], set_={ - "raw_failing": statement.excluded.raw_failing, - "grace_failing": statement.excluded.grace_failing, + "observed_pass": statement.excluded.observed_pass, + "confirmed_pass": statement.excluded.confirmed_pass, "evaluated_at": statement.excluded.evaluated_at, - "first_raw_failure_at": statement.excluded.first_raw_failure_at, - "last_raw_failure_at": statement.excluded.last_raw_failure_at, - "last_grace_failure_at": statement.excluded.last_grace_failure_at, + "first_observed_failure_at": statement.excluded.first_observed_failure_at, + "last_observed_failure_at": statement.excluded.last_observed_failure_at, + "last_confirmed_failure_at": statement.excluded.last_confirmed_failure_at, + "probation_start": statement.excluded.probation_start, "updated_at": statement.excluded.updated_at, }, ) @@ -192,6 +197,11 @@ def _upsert_criteria( def _upsert_seals(db_session: Session, outcomes: Sequence[dict], now: datetime) -> None: """Insert or update feedreliabilityseal rows. + A row is written for every evaluated feed, whether or not it qualifies. That matters + beyond bookkeeping: `created_at` on this row is what a criterion measuring "how long + have we been tracking this feed" reads, so a feed that does not qualify still needs one, + or such a criterion could never start counting and the feed could never come to qualify. + seal_earned_at and seal_lost_at are written only when has_seal actually changes, so they record transitions rather than the time of the last evaluation. """ @@ -292,17 +302,17 @@ def update_seals( feed_states: Dict[str, SealCriterionState] = {} for evaluator in evaluators: - raw = evaluator.evaluate(ctx) + observation = evaluator.evaluate(ctx) previous = previous_states.get((feed.id, evaluator.name.value)) state = transition( prev=previous, - raw=raw, + observation=observation, grace_period=evaluator.grace_period, - reliability_window=evaluator.reliability_window, + probation_period=evaluator.probation_period, now=now, feed_id=feed.id, ) - if raw.failing is None: + if observation.observed_pass is None: not_evaluable += 1 if state is not None: feed_states[evaluator.name.value] = state @@ -323,14 +333,21 @@ def update_seals( { "stable_id": ctx.stable_id, "criterion": evaluator.name.value, - "raw_failing": raw.failing, - "grace_failing": ( - state.grace_failing if state is not None else None + "observed_pass": observation.observed_pass, + "confirmed_pass": ( + state.confirmed_pass if state is not None else None ), - "previously_grace_failing": ( - previous.grace_failing if previous is not None else None + "previously_confirmed_pass": ( + previous.confirmed_pass + if previous is not None + else None ), - "reason": raw.reason, + "on_probation": ( + state.probation_start is not None + if state is not None + else None + ), + "reason": observation.reason, } ) @@ -346,10 +363,10 @@ def update_seals( } merged.update(feed_states) + # A feed with no seal row yet is treated as not holding one, so a first run can + # grant the seal but can never withdraw one: nothing was held to lose. had_seal = previous_seals.get(feed.id) - has_seal = _roll_up_has_seal( - merged, len(EVALUATORS), currently_held=bool(had_seal) - ) + has_seal = _roll_up_has_seal(merged) outcomes.append( { "feed_id": feed.id, diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py index 3de18db8f..b877d7014 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py @@ -15,101 +15,177 @@ # """Generic per-criterion state machine for the Seal of Reliability. -Steps 2 and 3 of the nightly job: failure tracking and `grace_failing`. This is the only -place that knows about grace periods and the reliability window, and it is -criterion-agnostic — the caller passes the values from the evaluator. `now` is a parameter -rather than a call to `datetime.now()` so runs are replayable and idempotent. +Steps 2 to 4 of the nightly job: observed failure tracking, the confirmed status, and +probation. This is the only place that knows about grace periods and probation, and it is +criterion-agnostic — the caller passes both from the evaluator. `now` is a parameter rather +than a call to `datetime.now()` so runs are replayable and idempotent. + +Two pieces of state are tracked per feed per criterion and they are independent: + +* `confirmed_pass` — does the criterion pass right now, debounced by its grace period. +* `probation_start` — the penalty it serves after a confirmed failure. + +The seal (step 5, in seal_updater) requires every criterion in service to be a confirmed +pass and not on probation. """ from dataclasses import dataclass, replace -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from typing import Optional from tasks.seal_of_reliability.criteria import SealCriterionName -from tasks.seal_of_reliability.evaluators.base import RawEvaluation +from tasks.seal_of_reliability.evaluators.base import CriterionObservation @dataclass(frozen=True) class SealCriterionState: """One row of the sealcriterion table. - raw_* fields describe the instantaneous state at the last evaluation, with no grace - applied. grace_failing is the debounced state that drives the seal outcome. + Both booleans are stored positively: TRUE means the criterion passed. `observed_pass` + is the instantaneous check, `confirmed_pass` the debounced status that drives the seal. + NULL on either means the criterion has never produced a verdict for this feed, and the + job never writes NULL back — see `transition`. + + The failure timestamps stay negative on purpose: they record events that really are + failures. Booleans describe state, timestamps record events. """ feed_id: str criterion: SealCriterionName - raw_failing: Optional[bool] = None - grace_failing: Optional[bool] = None + observed_pass: Optional[bool] = None + confirmed_pass: Optional[bool] = None evaluated_at: Optional[datetime] = None - first_raw_failure_at: Optional[datetime] = None - last_raw_failure_at: Optional[datetime] = None - last_grace_failure_at: Optional[datetime] = None + first_observed_failure_at: Optional[datetime] = None + last_observed_failure_at: Optional[datetime] = None + last_confirmed_failure_at: Optional[datetime] = None + probation_start: Optional[datetime] = None + + +def _next_day_start(moment: datetime) -> datetime: + """The start of the day after `moment`, in UTC. + + Probation is stamped at day granularity for two reasons: two runs on the same day + produce the same start, and stamping *tomorrow* on every failing day leaves the start on + the day the criterion was repaired once the streak ends. + """ + if moment.tzinfo is not None: + moment = moment.astimezone(timezone.utc) + return moment.replace(hour=0, minute=0, second=0, microsecond=0) + timedelta(days=1) + + +def _probation_start( + base: SealCriterionState, + observed_pass: bool, + confirmed_pass: bool, + probation_period: Optional[timedelta], + now: datetime, +) -> Optional[datetime]: + """Step 4: probation is a penalty, not an entry requirement. + + Two rules put a criterion on probation, and both are recoveries: + + * recovery from a confirmed failure, in all circumstances; + * recovery from an observed failure while already on probation. + + A first evaluation that passes is not a recovery, so it opens nothing. An observed + failure absorbed by the grace period costs a criterion that is not on probation nothing, + while the same failure during probation restarts the whole count. + """ + if probation_period is None: + return None + + if not observed_pass: + if base.probation_start is not None or not confirmed_pass: + return _next_day_start(now) + # Not on probation and this failure is still inside its grace period: the track + # record survives the blip. + return base.probation_start + + if ( + base.probation_start is not None + and now >= base.probation_start + probation_period + ): + return None + return base.probation_start def transition( prev: Optional[SealCriterionState], - raw: RawEvaluation, + observation: CriterionObservation, grace_period: Optional[timedelta], - reliability_window: Optional[timedelta], + probation_period: Optional[timedelta], now: datetime, feed_id: Optional[str] = None, ) -> Optional[SealCriterionState]: - """Apply one evaluation to a criterion's stored state. + """Apply one observation to a criterion's stored state. Returns the new state, or `prev` unchanged when the criterion was not evaluable - (`raw.failing is None`) — a missing input must never be read as a failure, otherwise an - upstream outage would revoke seals across the catalogue. + (`observation.observed_pass is None`) — a missing input must never be read as a failure, + otherwise an upstream outage would withdraw seals across the catalogue. That is also + what keeps `observed_pass IS NULL` meaning "never evaluated" rather than "not evaluable + last night", which step 5 relies on to decide what is in service. Args: prev: The stored state, or None if this criterion has never been evaluated. - raw: The evaluator's verdict for this run. - grace_period: How long a failure streak may last before it is confirmed. - None means a failure is confirmed immediately. - reliability_window: How long a confirmed failure keeps the criterion failing. - None means the criterion reflects the current state only, with no memory. + observation: The evaluator's verdict for this run. + grace_period: How long an observed failure streak may last before the status flips. + None means the status flips on the first failing day. + probation_period: How long the criterion must go with no observed failure after + recovering from a confirmed failure. None means the criterion has no probation. now: The evaluation timestamp. feed_id: Required when `prev` is None, to build the first state. """ - if raw.failing is None: + if observation.observed_pass is None: return prev resolved_feed_id = prev.feed_id if prev is not None else feed_id if resolved_feed_id is None: raise ValueError("feed_id is required when there is no previous state") - base = prev or SealCriterionState(feed_id=resolved_feed_id, criterion=raw.criterion) + base = prev or SealCriterionState( + feed_id=resolved_feed_id, criterion=observation.criterion + ) - if raw.failing: - # first_raw_failure_at marks the start of the current streak. It is cleared on - # recovery, so it is only None here at the start of a new streak. - first_raw_failure_at = base.first_raw_failure_at or now - last_raw_failure_at = now - last_grace_failure_at = base.last_grace_failure_at - if grace_period is None or now - first_raw_failure_at >= grace_period: - last_grace_failure_at = now - else: - # The streak ended, so the grace period resets. last_raw_failure_at and - # last_grace_failure_at are history and are never cleared. - first_raw_failure_at = None - last_raw_failure_at = base.last_raw_failure_at - last_grace_failure_at = base.last_grace_failure_at - - if reliability_window is None: - # No memory: the criterion tracks the current state and clears on recovery. - grace_failing = raw.failing + # A criterion that has never produced a verdict gets no grace period. The grace period + # holds a pass the criterion has actually earned, and one we have never seen pass has + # nothing to hold — otherwise a feed that is broken the first time we look at it would + # be reported as passing for up to a month on evidence we do not have. + first_evaluation = base.observed_pass is None + + if observation.observed_pass: + # The streak is over, so the grace period resets. The two `last_*` timestamps are + # history and are never cleared. + first_observed_failure_at = None + last_observed_failure_at = base.last_observed_failure_at + last_confirmed_failure_at = base.last_confirmed_failure_at + confirmed_pass = True else: - grace_failing = ( - last_grace_failure_at is not None - and last_grace_failure_at >= now - reliability_window + # first_observed_failure_at marks the start of the current streak. It is cleared on + # recovery, so it is only None here at the start of a new streak. + first_observed_failure_at = base.first_observed_failure_at or now + last_observed_failure_at = now + confirmed_pass = ( + grace_period is not None + and not first_evaluation + and now - first_observed_failure_at < grace_period + ) + last_confirmed_failure_at = ( + base.last_confirmed_failure_at if confirmed_pass else now ) return replace( base, - raw_failing=raw.failing, - grace_failing=grace_failing, + observed_pass=observation.observed_pass, + confirmed_pass=confirmed_pass, evaluated_at=now, - first_raw_failure_at=first_raw_failure_at, - last_raw_failure_at=last_raw_failure_at, - last_grace_failure_at=last_grace_failure_at, + first_observed_failure_at=first_observed_failure_at, + last_observed_failure_at=last_observed_failure_at, + last_confirmed_failure_at=last_confirmed_failure_at, + probation_start=_probation_start( + base=base, + observed_pass=observation.observed_pass, + confirmed_pass=confirmed_pass, + probation_period=probation_period, + now=now, + ), ) diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py index df50d2f47..538c173f8 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py @@ -217,10 +217,10 @@ def test_two_runs_with_a_change_in_between(self): criteria = self.criterion_state() self.assertNotIn(DEPRECATED, criteria) self.assertNotIn(UNPUBLISHED, criteria) - self.assertFalse(criteria[OFFICIAL].grace_failing) - self.assertIsNone(criteria[OFFICIAL].first_raw_failure_at) - self.assertTrue(criteria[NOT_OFFICIAL].grace_failing) - self.assertEqual(criteria[NOT_OFFICIAL].first_raw_failure_at, NOW) + self.assertTrue(criteria[OFFICIAL].confirmed_pass) + self.assertIsNone(criteria[OFFICIAL].first_observed_failure_at) + self.assertFalse(criteria[NOT_OFFICIAL].confirmed_pass) + self.assertEqual(criteria[NOT_OFFICIAL].first_observed_failure_at, NOW) # --- modify the database: revoke one, recover another self.set_official(OFFICIAL, False) @@ -243,10 +243,10 @@ def test_two_runs_with_a_change_in_between(self): moved = {row["stable_id"]: row for row in self.ours(second)} self.assertEqual(set(moved), {OFFICIAL, NOT_OFFICIAL}, "only these two moved") - self.assertTrue(moved[OFFICIAL]["grace_failing"]) - self.assertFalse(moved[OFFICIAL]["previously_grace_failing"]) - self.assertFalse(moved[NOT_OFFICIAL]["grace_failing"]) - self.assertTrue(moved[NOT_OFFICIAL]["previously_grace_failing"]) + self.assertFalse(moved[OFFICIAL]["confirmed_pass"]) + self.assertTrue(moved[OFFICIAL]["previously_confirmed_pass"]) + self.assertTrue(moved[NOT_OFFICIAL]["confirmed_pass"]) + self.assertFalse(moved[NOT_OFFICIAL]["previously_confirmed_pass"]) # --- inspect again: both transitions are recorded, history is preserved self.assertEqual( @@ -259,12 +259,12 @@ def test_two_runs_with_a_change_in_between(self): "the revoked feed keeps its earned_at; the recovered one gains earned_at", ) criteria = self.criterion_state() - self.assertEqual(criteria[OFFICIAL].first_raw_failure_at, later) + self.assertEqual(criteria[OFFICIAL].first_observed_failure_at, later) self.assertIsNone( - criteria[NOT_OFFICIAL].first_raw_failure_at, "the streak ended" + criteria[NOT_OFFICIAL].first_observed_failure_at, "the streak ended" ) self.assertEqual( - criteria[NOT_OFFICIAL].last_raw_failure_at, + criteria[NOT_OFFICIAL].last_observed_failure_at, NOW, "history is never cleared, so the old failure time survives recovery", ) @@ -283,8 +283,8 @@ def test_third_run_with_no_change_is_a_no_op(self): for stable_id, row in after.items(): with self.subTest(stable_id=stable_id): self.assertEqual( - row.first_raw_failure_at, - before[stable_id].first_raw_failure_at, + row.first_observed_failure_at, + before[stable_id].first_observed_failure_at, "a re-evaluation must not restart a failure streak", ) self.assertEqual(row.evaluated_at, later, "but it is re-evaluated") diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py index 276a13dac..0d897ab53 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_evaluators.py @@ -67,19 +67,23 @@ def test_registry_only_uses_known_criteria(self): class TestOfficial(unittest.TestCase): def test_official_feed_passes(self): - self.assertFalse(OfficialEvaluator().evaluate(_ctx(official=True)).failing) + self.assertTrue(OfficialEvaluator().evaluate(_ctx(official=True)).observed_pass) def test_non_official_feed_fails(self): - self.assertTrue(OfficialEvaluator().evaluate(_ctx(official=False)).failing) + self.assertFalse( + OfficialEvaluator().evaluate(_ctx(official=False)).observed_pass + ) def test_unknown_official_flag_fails(self): """NULL is not an endorsement: only an explicit True passes.""" - self.assertTrue(OfficialEvaluator().evaluate(_ctx(official=None)).failing) + self.assertFalse( + OfficialEvaluator().evaluate(_ctx(official=None)).observed_pass + ) - def test_has_no_grace_or_window(self): + def test_has_no_grace_or_probation(self): """A point-in-time check: it clears as soon as the feed is official again.""" self.assertIsNone(OfficialEvaluator.grace_period) - self.assertIsNone(OfficialEvaluator.reliability_window) + self.assertIsNone(OfficialEvaluator.probation_period) def test_reason_names_the_offending_value(self): result = OfficialEvaluator().evaluate(_ctx(official=None)) diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py index e87d0ec76..1da7836ac 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py @@ -18,57 +18,69 @@ import unittest from datetime import datetime, timedelta, timezone -from tasks.seal_of_reliability.criteria import RELIABILITY_WINDOW, SealCriterionName -from tasks.seal_of_reliability.evaluators.base import RawEvaluation +from tasks.seal_of_reliability.criteria import PROBATION_PERIOD, SealCriterionName +from tasks.seal_of_reliability.evaluators.base import CriterionObservation from tasks.seal_of_reliability.state_machine import SealCriterionState, transition DAY_ZERO = datetime(2026, 1, 1, tzinfo=timezone.utc) FEED_ID = "feed-1" -# Official, the first criterion implemented, has neither a grace period nor a reliability -# window, so it exercises almost none of the state machine. These tests drive a synthetic -# criterion that has both. +# Official, the first criterion implemented, has neither a grace period nor probation, so it +# exercises almost none of the state machine. These tests drive a synthetic criterion that +# has both. GRACE = timedelta(days=14) -def _raw(failing, criterion=SealCriterionName.AVAILABLE): - return RawEvaluation(criterion=criterion, failing=failing, reason="test") +def _day(offset: int) -> datetime: + return DAY_ZERO + timedelta(days=offset) -def _run(days, grace_period=GRACE, reliability_window=RELIABILITY_WINDOW, state=None): - """Apply one verdict per entry in `days`: (day offset, failing).""" - for offset, failing in days: +def _observation(observed_pass, criterion=SealCriterionName.AVAILABLE): + return CriterionObservation( + criterion=criterion, observed_pass=observed_pass, reason="test" + ) + + +def _run(days, grace_period=GRACE, probation_period=PROBATION_PERIOD, state=None): + """Apply one verdict per entry in `days`: (day offset, observed_pass).""" + for offset, observed_pass in days: state = transition( prev=state, - raw=_raw(failing), + observation=_observation(observed_pass), grace_period=grace_period, - reliability_window=reliability_window, - now=DAY_ZERO + timedelta(days=offset), + probation_period=probation_period, + now=_day(offset), feed_id=FEED_ID, ) return state +def _failing(start, end): + """Observed failures on every day in [start, end).""" + return [(day, False) for day in range(start, end)] + + class TestNotEvaluable(unittest.TestCase): def test_none_verdict_leaves_state_untouched(self): """A criterion that cannot be evaluated must not change anything.""" - state = _run([(0, True)]) + state = _run([(0, False)]) unchanged = transition( prev=state, - raw=_raw(None), + observation=_observation(None), grace_period=GRACE, - reliability_window=RELIABILITY_WINDOW, - now=DAY_ZERO + timedelta(days=1), + probation_period=PROBATION_PERIOD, + now=_day(1), ) self.assertIs(unchanged, state) def test_none_verdict_with_no_previous_state_stays_none(self): + """No row is created, so `observed_pass IS NULL` keeps meaning never evaluated.""" self.assertIsNone( transition( prev=None, - raw=_raw(None), + observation=_observation(None), grace_period=GRACE, - reliability_window=RELIABILITY_WINDOW, + probation_period=PROBATION_PERIOD, now=DAY_ZERO, feed_id=FEED_ID, ) @@ -78,103 +90,148 @@ def test_missing_feed_id_without_previous_state_raises(self): with self.assertRaises(ValueError): transition( prev=None, - raw=_raw(True), + observation=_observation(False), grace_period=GRACE, - reliability_window=RELIABILITY_WINDOW, + probation_period=PROBATION_PERIOD, now=DAY_ZERO, ) class TestGracePeriod(unittest.TestCase): - def test_first_failure_is_not_confirmed(self): + def test_first_evaluation_passes_immediately(self): + """No observation period to serve: a passing first verdict is a confirmed pass.""" state = _run([(0, True)]) - self.assertTrue(state.raw_failing) - self.assertFalse(state.grace_failing) - self.assertEqual(state.first_raw_failure_at, DAY_ZERO) - self.assertEqual(state.last_raw_failure_at, DAY_ZERO) - self.assertIsNone(state.last_grace_failure_at) + self.assertTrue(state.observed_pass) + self.assertTrue(state.confirmed_pass) + self.assertIsNone(state.first_observed_failure_at) + self.assertIsNone(state.probation_start) + + def test_first_evaluation_gets_no_grace_period(self): + """A criterion never seen to pass has no earned pass for the grace period to hold.""" + state = _run([(0, False)]) + self.assertFalse(state.observed_pass) + self.assertFalse(state.confirmed_pass) + self.assertEqual(state.last_confirmed_failure_at, DAY_ZERO) def test_failure_within_grace_is_not_confirmed(self): - state = _run([(day, True) for day in range(0, 14)]) - self.assertTrue(state.raw_failing) - self.assertFalse(state.grace_failing) - self.assertIsNone(state.last_grace_failure_at) + state = _run([(0, True)] + _failing(1, 14)) + self.assertFalse(state.observed_pass) + self.assertTrue(state.confirmed_pass, "still inside the grace period") + self.assertIsNone(state.last_confirmed_failure_at) def test_failure_at_grace_expiry_is_confirmed(self): - state = _run([(day, True) for day in range(0, 15)]) - self.assertTrue(state.grace_failing) - self.assertEqual(state.last_grace_failure_at, DAY_ZERO + timedelta(days=14)) - - def test_recovery_within_grace_resets_the_streak(self): - state = _run([(0, True), (1, True), (2, False)]) - self.assertFalse(state.raw_failing) - self.assertFalse(state.grace_failing) - self.assertIsNone(state.first_raw_failure_at) - # History is kept. - self.assertEqual(state.last_raw_failure_at, DAY_ZERO + timedelta(days=1)) + state = _run([(0, True)] + _failing(1, 16)) + self.assertFalse(state.confirmed_pass) + self.assertEqual(state.last_confirmed_failure_at, _day(15)) + + def test_recovery_is_immediate(self): + """The first day with an observed pass clears the status, with no wait.""" + state = _run([(0, True)] + _failing(1, 20) + [(20, True)]) + self.assertTrue(state.confirmed_pass) + self.assertIsNone(state.first_observed_failure_at, "the streak is cleared") + self.assertEqual(state.last_observed_failure_at, _day(19), "history is kept") + self.assertEqual(state.last_confirmed_failure_at, _day(19)) def test_streak_restarts_after_recovery(self): """A new streak gets a full grace period, it does not resume the old one.""" - days = [(day, True) for day in range(0, 10)] - days += [(10, False)] - days += [(day, True) for day in range(11, 20)] + days = [(0, True)] + _failing(1, 10) + [(10, True)] + _failing(11, 20) state = _run(days) - self.assertTrue(state.raw_failing) - self.assertFalse(state.grace_failing) - self.assertEqual(state.first_raw_failure_at, DAY_ZERO + timedelta(days=11)) + self.assertFalse(state.observed_pass) + self.assertTrue(state.confirmed_pass) + self.assertEqual(state.first_observed_failure_at, _day(11)) def test_no_grace_period_confirms_immediately(self): - state = _run([(0, True)], grace_period=None) - self.assertTrue(state.grace_failing) - self.assertEqual(state.last_grace_failure_at, DAY_ZERO) + state = _run([(0, True), (1, False)], grace_period=None) + self.assertFalse(state.confirmed_pass) + self.assertEqual(state.last_confirmed_failure_at, _day(1)) + + +class TestProbation(unittest.TestCase): + def test_a_clean_first_evaluation_opens_no_probation(self): + """Probation is a penalty, not an entry requirement.""" + self.assertIsNone(_run([(0, True)]).probation_start) + self.assertIsNone(_run([(0, True), (1, True), (2, True)]).probation_start) + + def test_a_blip_inside_grace_does_not_open_probation(self): + """Off probation, a failure absorbed by the grace period costs nothing.""" + state = _run([(0, True), (1, False), (2, True)]) + self.assertTrue(state.confirmed_pass) + self.assertIsNone(state.probation_start) + + def test_recovery_from_a_confirmed_failure_opens_probation(self): + state = _run([(0, True)] + _failing(1, 16) + [(16, True)]) + self.assertTrue(state.confirmed_pass, "the status clears the same day") + self.assertEqual( + state.probation_start, _day(16), "but probation runs from the repair day" + ) + def test_probation_starts_the_day_after_the_last_failure(self): + """Stamped at tomorrow every failing day, so it lands on the day of repair.""" + state = _run([(0, True)] + _failing(1, 16)) + self.assertEqual(state.probation_start, _day(16)) -class TestReliabilityWindow(unittest.TestCase): - def test_confirmed_failure_persists_after_recovery(self): - days = [(day, True) for day in range(0, 15)] + [(15, False)] - state = _run(days) - self.assertFalse(state.raw_failing) - self.assertTrue( - state.grace_failing, "a confirmed failure must hold through the window" - ) + def test_probation_ends_after_the_full_period(self): + days = [(0, True)] + _failing(1, 16) + [(16, True)] + served = _run(days + [(16 + PROBATION_PERIOD.days, True)]) + self.assertIsNone(served.probation_start) - def test_criterion_clears_once_the_window_passes(self): - days = [(day, True) for day in range(0, 15)] - days += [(15, False), (15 + RELIABILITY_WINDOW.days, False)] - self.assertFalse(_run(days).grace_failing) + def test_probation_still_open_one_day_short(self): + days = [(0, True)] + _failing(1, 16) + [(16, True)] + state = _run(days + [(15 + PROBATION_PERIOD.days, True)]) + self.assertEqual(state.probation_start, _day(16)) - def test_criterion_still_failing_just_inside_the_window(self): - days = [(day, True) for day in range(0, 15)] - days += [(15, False), (14 + RELIABILITY_WINDOW.days, False)] - self.assertTrue(_run(days).grace_failing) + def test_an_observed_failure_during_probation_restarts_it(self): + """Confirmed or not: during probation any failure costs the whole count.""" + days = [(0, True)] + _failing(1, 16) + [(16, True), (100, False), (101, True)] + state = _run(days) + self.assertTrue(state.confirmed_pass, "the blip stayed inside the grace period") + self.assertEqual(state.probation_start, _day(101)) - def test_no_window_tracks_current_state_only(self): - """Official's shape: no grace, no window, clears as soon as the feed recovers.""" + def test_a_criterion_without_probation_never_gets_one(self): + """Official's shape: it clears outright as soon as the feed recovers.""" state = _run( - [(0, True), (1, False)], grace_period=None, reliability_window=None + [(0, True)] + _failing(1, 30) + [(30, True)], + grace_period=None, + probation_period=None, + ) + self.assertTrue(state.confirmed_pass) + self.assertIsNone(state.probation_start) + + def test_probation_uses_day_granularity(self): + """A run at any hour stamps the same start, so the period is not shifted.""" + midday = _day(1) + timedelta(hours=13, minutes=27) + state = transition( + prev=None, + observation=_observation(False), + grace_period=None, + probation_period=PROBATION_PERIOD, + now=midday, + feed_id=FEED_ID, ) - self.assertFalse(state.grace_failing) - self.assertIsNotNone(state.last_grace_failure_at, "history is still recorded") + self.assertEqual(state.probation_start, _day(2)) class TestIdempotency(unittest.TestCase): def test_same_timestamp_twice_produces_the_same_state(self): """A re-run for the same instant must not compound a failure streak.""" - once = _run([(0, True), (1, True)]) - twice = _run([(1, True)], state=once) - self.assertEqual(once.first_raw_failure_at, twice.first_raw_failure_at) - self.assertEqual(once.last_raw_failure_at, twice.last_raw_failure_at) - self.assertEqual(once.grace_failing, twice.grace_failing) + once = _run([(0, True), (1, False), (2, False)]) + twice = _run([(2, False)], state=once) + self.assertEqual( + once.first_observed_failure_at, twice.first_observed_failure_at + ) + self.assertEqual(once.last_observed_failure_at, twice.last_observed_failure_at) + self.assertEqual(once.confirmed_pass, twice.confirmed_pass) + self.assertEqual(once.probation_start, twice.probation_start) def test_state_is_not_mutated_in_place(self): - state = _run([(0, True)]) - _run([(1, True)], state=state) - self.assertEqual(state.last_raw_failure_at, DAY_ZERO) + state = _run([(0, False)]) + _run([(1, False)], state=state) + self.assertEqual(state.last_observed_failure_at, DAY_ZERO) class TestStateShape(unittest.TestCase): def test_state_carries_feed_id_and_criterion(self): - state = _run([(0, True)]) + state = _run([(0, False)]) self.assertEqual(state.feed_id, FEED_ID) self.assertEqual(state.criterion, SealCriterionName.AVAILABLE) @@ -182,9 +239,10 @@ def test_state_defaults_are_all_none(self): state = SealCriterionState( feed_id=FEED_ID, criterion=SealCriterionName.OFFICIAL ) - self.assertIsNone(state.raw_failing) - self.assertIsNone(state.grace_failing) + self.assertIsNone(state.observed_pass) + self.assertIsNone(state.confirmed_pass) self.assertIsNone(state.evaluated_at) + self.assertIsNone(state.probation_start) if __name__ == "__main__": diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py index d48edc181..f94a78e1a 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py @@ -216,17 +216,17 @@ def test_dry_run_reports_rows_for_a_named_feed(self): [row["criterion"] for row in report["evaluations"]], [SealCriterionName.OFFICIAL.value], ) - self.assertTrue(report["evaluations"][0]["raw_failing"]) + self.assertFalse(report["evaluations"][0]["observed_pass"]) self.assertTrue(report["evaluations"][0]["reason"]) - self.assertIsNone(report["evaluations"][0]["previously_grace_failing"]) + self.assertIsNone(report["evaluations"][0]["previously_confirmed_pass"]) def test_named_feed_reports_a_row_even_when_nothing_moved(self): """An explicit feed list is the debugging path: report all of its criteria.""" update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) report = update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) self.assertEqual(len(report["evaluations"]), 1) - self.assertFalse(report["evaluations"][0]["grace_failing"]) - self.assertFalse(report["evaluations"][0]["previously_grace_failing"]) + self.assertTrue(report["evaluations"][0]["confirmed_pass"]) + self.assertTrue(report["evaluations"][0]["previously_confirmed_pass"]) def test_unnamed_run_reports_only_criteria_that_moved(self): """A passing feed evaluated twice contributes no entry the second time.""" @@ -254,16 +254,17 @@ def test_a_criterion_that_flips_is_reported_with_its_previous_value(self): moved = [row for row in report["evaluations"] if row["stable_id"] == OFFICIAL] self.assertEqual(len(moved), 1) - self.assertTrue(moved[0]["grace_failing"]) - self.assertFalse(moved[0]["previously_grace_failing"]) + self.assertFalse(moved[0]["confirmed_pass"]) + self.assertTrue(moved[0]["previously_confirmed_pass"]) def test_official_feed_earns_the_seal(self): update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) row = self.criterion_rows(OFFICIAL)[SealCriterionName.OFFICIAL.value] - self.assertFalse(row.raw_failing) - self.assertFalse(row.grace_failing) + self.assertTrue(row.observed_pass) + self.assertTrue(row.confirmed_pass) self.assertEqual(row.evaluated_at, NOW) - self.assertIsNone(row.first_raw_failure_at) + self.assertIsNone(row.first_observed_failure_at) + self.assertIsNone(row.probation_start, "a clean first run opens no probation") seal = self.seal_row(OFFICIAL) self.assertTrue(seal.has_seal) @@ -274,12 +275,20 @@ def test_failing_official_denies_the_seal_immediately(self): """Official has no grace period, so one failure is confirmed at once.""" update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) row = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] - self.assertTrue(row.raw_failing) - self.assertTrue(row.grace_failing) - self.assertEqual(row.first_raw_failure_at, NOW) - self.assertEqual(row.last_grace_failure_at, NOW) + self.assertFalse(row.observed_pass) + self.assertFalse(row.confirmed_pass) + self.assertEqual(row.first_observed_failure_at, NOW) + self.assertEqual(row.last_confirmed_failure_at, NOW) self.assertFalse(self.seal_row(NOT_OFFICIAL).has_seal) + def test_a_feed_that_never_qualifies_still_gets_a_seal_row(self): + """`created_at` on that row is what a tracking-age criterion measures against.""" + update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) + seal = self.seal_row(NOT_OFFICIAL) + self.assertIsNotNone(seal, "a row is written whether or not the feed qualifies") + self.assertFalse(seal.has_seal) + self.assertIsNone(seal.seal_lost_at, "nothing was held, so nothing was lost") + def test_unknown_official_flag_denies_the_seal(self): update_seals(dry_run=False, stable_feed_ids=[UNKNOWN_OFFICIAL], now=NOW) self.assertFalse(self.seal_row(UNKNOWN_OFFICIAL).has_seal) @@ -289,8 +298,12 @@ def test_rerun_is_idempotent(self): first = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) second = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] - self.assertEqual(first.first_raw_failure_at, second.first_raw_failure_at) - self.assertEqual(first.last_grace_failure_at, second.last_grace_failure_at) + self.assertEqual( + first.first_observed_failure_at, second.first_observed_failure_at + ) + self.assertEqual( + first.last_confirmed_failure_at, second.last_confirmed_failure_at + ) def test_losing_official_status_revokes_the_seal(self): update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) @@ -305,7 +318,7 @@ def test_losing_official_status_revokes_the_seal(self): self.assertEqual(seal.seal_earned_at, NOW, "the earlier grant is preserved") def test_regaining_official_status_clears_the_criterion(self): - """No reliability window, so recovery is immediate rather than 6 months later.""" + """Official has no probation, so recovery restores the seal the same day.""" update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=NOW) self.assertFalse(self.seal_row(NOT_OFFICIAL).has_seal) @@ -313,10 +326,11 @@ def test_regaining_official_status_clears_the_criterion(self): later = NOW + timedelta(days=1) update_seals(dry_run=False, stable_feed_ids=[NOT_OFFICIAL], now=later) row = self.criterion_rows(NOT_OFFICIAL)[SealCriterionName.OFFICIAL.value] - self.assertFalse(row.grace_failing) - self.assertIsNone(row.first_raw_failure_at, "the streak is cleared") + self.assertTrue(row.confirmed_pass) + self.assertIsNone(row.first_observed_failure_at, "the streak is cleared") + self.assertIsNone(row.probation_start, "Official serves no probation") self.assertEqual( - row.last_grace_failure_at, NOW, "the failure is still on record" + row.last_confirmed_failure_at, NOW, "the failure is still on record" ) seal = self.seal_row(NOT_OFFICIAL) self.assertTrue(seal.has_seal) diff --git a/liquibase/changelog.xml b/liquibase/changelog.xml index 1408c7220..f04ba7ae4 100644 --- a/liquibase/changelog.xml +++ b/liquibase/changelog.xml @@ -127,6 +127,8 @@ + + diff --git a/liquibase/changes/feat_1783.sql b/liquibase/changes/feat_1783.sql new file mode 100644 index 000000000..b0a0166a7 --- /dev/null +++ b/liquibase/changes/feat_1783.sql @@ -0,0 +1,43 @@ +-- Issue #1783: revised Seal of Reliability algorithm (spec in #1761). +-- Adjusts the tables added by feat_1760.sql. Nothing reads them yet, so this is safe to +-- apply as a rename rather than an additive migration. + +-- 1. The two status booleans flip to positive polarity and are renamed. +-- `has_seal` is already positive and every criterion check is written positively +-- (`success = TRUE`, `total_error = 0`), so negative status columns forced an inversion +-- at both ends. `raw` also described how a value was produced rather than what it means +-- and had no opposite pole, which left `grace` on a different axis; observed -> +-- confirmed is one axis, and the grace period is the distance along it. +ALTER TABLE SealCriterion RENAME COLUMN raw_failing TO observed_pass; +ALTER TABLE SealCriterion RENAME COLUMN grace_failing TO confirmed_pass; + +-- The failure timestamps keep their negative sense on purpose: they record events that +-- really are failures, and the grace period is measured from the start of a bad streak. +-- Booleans describe state, timestamps record events. +ALTER TABLE SealCriterion RENAME COLUMN first_raw_failure_at TO first_observed_failure_at; +ALTER TABLE SealCriterion RENAME COLUMN last_raw_failure_at TO last_observed_failure_at; +ALTER TABLE SealCriterion RENAME COLUMN last_grace_failure_at TO last_confirmed_failure_at; + +-- These are inversions, not just renames: TRUE meant failing and now means passing. +-- NULL keeps its meaning of "not yet evaluated" and must stay NULL. +UPDATE SealCriterion SET observed_pass = NOT observed_pass WHERE observed_pass IS NOT NULL; +UPDATE SealCriterion SET confirmed_pass = NOT confirmed_pass WHERE confirmed_pass IS NOT NULL; + +COMMENT ON COLUMN SealCriterion.observed_pass IS + 'The criterion''s own check on this day, with no debouncing. NULL = never evaluated.'; +COMMENT ON COLUMN SealCriterion.confirmed_pass IS + 'The debounced status. FALSE once an observed failure outlasts the grace period, or ' + 'immediately for a criterion with no grace period. NULL = never evaluated.'; + +-- 2. probation_start replaces the reliability window, which used to be an implicit lookback +-- over last_grace_failure_at. Probation is a penalty rather than an entry requirement: a +-- criterion is put on it by a recovery, never by a first evaluation, so a feed that has +-- never had a confirmed failure can hold the seal from its first evaluation. +ALTER TABLE SealCriterion ADD COLUMN IF NOT EXISTS probation_start TIMESTAMPTZ; + +COMMENT ON COLUMN SealCriterion.probation_start IS + 'Start of the 180-day stretch with no observed failure that this criterion must ' + 'complete before it can contribute to the seal again. Started by a recovery from a ' + 'confirmed failure, or by a recovery from an observed failure while already on ' + 'probation. NULL = not on probation: nothing has gone wrong yet, probation was served ' + 'out, or the criterion has no probation at all.'; From b88b37953ff4dd7e974e2296a7d0e146698e7584 Mon Sep 17 00:00:00 2001 From: jcpitre Date: Wed, 12 Aug 2026 09:42:06 -0400 Subject: [PATCH 07/10] Updated comments and doc --- functions-python/tasks_executor/README.md | 21 +++---------------- functions-python/tasks_executor/src/main.py | 3 +-- .../src/tasks/seal_of_reliability/criteria.py | 6 +++--- .../evaluators/official.py | 6 +++--- .../seal_of_reliability/state_machine.py | 7 +++++-- .../test_seal_state_machine.py | 2 +- 6 files changed, 16 insertions(+), 29 deletions(-) diff --git a/functions-python/tasks_executor/README.md b/functions-python/tasks_executor/README.md index 6994aac40..37bdb2bdd 100644 --- a/functions-python/tasks_executor/README.md +++ b/functions-python/tasks_executor/README.md @@ -413,24 +413,13 @@ Evaluates the implemented Seal of Reliability criteria (issue #1761) for every e feed and updates the `sealcriterion` and `feedreliabilityseal` tables. Reads the source tables and never modifies them. -Only the **Official** criterion is implemented (issue #1783), so `has_seal` currently means -`feed.official IS TRUE`. The remaining five criteria — Stable, Available, Compliant and the -two Fresh checks — are tracked by #1784 and #1782; `seal_criterion_name` in the database -already declares all six values, so adding one needs no schema change. +`seal_criterion_name` in the database declares all six criteria, so adding an evaluator +needs no schema change. The criteria still to be implemented are tracked by #1784 and #1782. Eligible feeds are GTFS, `operational_status = published`, and `status NOT IN (deprecated, development)`. `inactive` and `future` feeds are deliberately included: skipping a feed freezes its stored rows rather than making it neutral. -Each criterion carries two independent pieces of state. `confirmed_pass` is the debounced -status: an observed failure only flips it once it outlasts the criterion's grace period, and -recovery clears it the same day. `probation_start` is the penalty served afterwards — 180 -days with no observed failure, counted from the day the criterion recovered, restarted by -any observed failure while it is running. A feed holds the seal when every criterion that -has ever produced a verdict is a confirmed pass and not on probation, so probation is a -penalty rather than an entry requirement: a feed that has never had a confirmed failure can -hold the seal from its first evaluation. - ```json { "task": "update_seal_of_reliability", @@ -446,7 +435,7 @@ hold the seal from its first evaluation. | `dry_run` | bool | `true` | Evaluate every feed and return the report without writing anything | | `stable_feed_ids` | list[str] \| null | `null` | Evaluate only these feeds; unknown ids raise. When set, `evaluations` covers every criterion of those feeds instead of only the ones whose verdict moved | | `limit` | int \| null | `null` | Cap the number of feeds evaluated | -| `criteria` | list[str] \| null | `null` | Evaluate only these criteria. Only `official` is implemented so far; naming a criterion that has no evaluator yet raises. A subset of the implemented criteria skips the `has_seal` roll-up, since the ones not evaluated cannot be judged | +| `criteria` | list[str] \| null | `null` | Evaluate only these criteria. Naming a criterion that has no evaluator yet raises. A subset of the implemented criteria skips the `has_seal` roll-up, since the ones not evaluated cannot be judged | | `batch_size` | int | `200` | Feeds loaded per query batch | | `now` | str \| null | `null` | ISO timestamp to evaluate against, for replays and backfills. Defaults to the current UTC time | @@ -466,10 +455,6 @@ hold the seal from its first evaluation. | `first_evaluations` | Criteria evaluated for the first time (no stored row yet) | | `evaluations` | The notable outcomes: one entry per feed and criterion whose verdict moved, with `observed_pass`, `confirmed_pass`, `previously_confirmed_pass`, `on_probation` and `reason`. A first evaluation appears only if it landed on a failure; the rest are counted by `first_evaluations`. When `stable_feed_ids` is set, every criterion of those feeds is included whether or not it moved | -> Note: no Cloud Scheduler job is defined for this task yet — invoke it manually. Once -> scheduled it should run after the daily `check_gtfs_feed_availability` job, since the -> Available criterion reads the availability rows recorded for the day. - #### Running it locally Start Postgres and the function, then post to it: diff --git a/functions-python/tasks_executor/src/main.py b/functions-python/tasks_executor/src/main.py index d8ddeed71..082e12f03 100644 --- a/functions-python/tasks_executor/src/main.py +++ b/functions-python/tasks_executor/src/main.py @@ -261,8 +261,7 @@ "update_seal_of_reliability": { "description": ( "Evaluates the implemented Seal of Reliability criteria for every eligible " - "GTFS feed and updates sealcriterion and feedreliabilityseal. Only the " - "Official criterion is implemented so far (see #1784 and #1782 for the rest). " + "GTFS feed and updates sealcriterion and feedreliabilityseal. " "Reads the source tables and never modifies them. " "Parameters: dry_run (default true), stable_feed_ids (default null; when set, " "`evaluations` covers every criterion of those feeds), limit (default null), " diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py index 68c8f7dc6..1ba4393de 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/criteria.py @@ -48,7 +48,7 @@ class SealCriterionName(str, Enum): # default for new evaluators; Official is exempt because it is a point-in-time state check # (see OfficialEvaluator). # -# Probation is a penalty, not an entry requirement. It is opened by a recovery, and a first -# evaluation that passes is not a recovery, so a feed that has never had a confirmed failure -# can hold the seal from its very first evaluation. +# Probation is opened only by a recovery. A first evaluation that passes is not a recovery, +# so a feed that has never had a confirmed failure never serves probation at all and can +# hold the seal from its very first evaluation. PROBATION_PERIOD: Final[timedelta] = timedelta(days=180) diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py index be5e07a24..2401c6f36 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/evaluators/official.py @@ -36,6 +36,6 @@ class OfficialEvaluator(CriterionEvaluator): probation_period = None def _evaluate(self, ctx: FeedSealContext) -> Tuple[Optional[bool], str]: - if ctx.official: - return True, "feed is official" - return False, f"feed.official is {ctx.official!r}, expected True" + # `is True` rather than a truthiness test: NULL is not an endorsement, and it is a + # verdict of "not official" rather than "not evaluable". + return ctx.official is True, f"feed.official is {ctx.official!r}" diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py index b877d7014..de14d4446 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/state_machine.py @@ -23,7 +23,7 @@ Two pieces of state are tracked per feed per criterion and they are independent: * `confirmed_pass` — does the criterion pass right now, debounced by its grace period. -* `probation_start` — the penalty it serves after a confirmed failure. +* `probation_start` — the stretch it must serve after recovering from a confirmed failure. The seal (step 5, in seal_updater) requires every criterion in service to be a confirmed pass and not on probation. @@ -80,7 +80,7 @@ def _probation_start( probation_period: Optional[timedelta], now: datetime, ) -> Optional[datetime]: - """Step 4: probation is a penalty, not an entry requirement. + """Step 4: probation is only ever opened by a recovery. Two rules put a criterion on probation, and both are recoveries: @@ -101,6 +101,9 @@ def _probation_start( # record survives the blip. return base.probation_start + # The check passed today. If the criterion was serving probation and the whole stretch + # has now gone by since it started, it has served it: clear it. Otherwise leave the start + # exactly where it is — a passing day never moves it, it only brings the end closer. if ( base.probation_start is not None and now >= base.probation_start + probation_period diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py index 1da7836ac..d2e86fc0e 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py @@ -148,7 +148,7 @@ def test_no_grace_period_confirms_immediately(self): class TestProbation(unittest.TestCase): def test_a_clean_first_evaluation_opens_no_probation(self): - """Probation is a penalty, not an entry requirement.""" + """Probation is opened only by a recovery, and this is not one.""" self.assertIsNone(_run([(0, True)]).probation_start) self.assertIsNone(_run([(0, True), (1, True), (2, True)]).probation_start) From f9a1c7276e3030898cc174afd281dad6a4c3f51a Mon Sep 17 00:00:00 2001 From: jcpitre Date: Wed, 12 Aug 2026 11:59:57 -0400 Subject: [PATCH 08/10] Updated comments and tests --- .../test_seal_state_machine.py | 39 +++ .../test_seal_updater_db.py | 264 +++++++++++++++++- liquibase/changes/feat_1783.sql | 41 +-- 3 files changed, 313 insertions(+), 31 deletions(-) diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py index d2e86fc0e..1f73350ac 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_state_machine.py @@ -180,6 +180,45 @@ def test_probation_still_open_one_day_short(self): state = _run(days + [(15 + PROBATION_PERIOD.days, True)]) self.assertEqual(state.probation_start, _day(16)) + def test_a_failure_the_day_before_probation_ends_restarts_the_whole_period(self): + """Nearly served is not served: the count goes back to zero, not to one day left.""" + # Probation runs from D16, so it would have ended on D196. + served = [(0, True)] + _failing(1, 16) + [(16, True)] + + state = _run(served + [(195, False), (196, True)]) + self.assertEqual( + state.probation_start, _day(196), "restarted on the repair day" + ) + + self.assertIsNotNone( + _run(served + [(195, False), (196, True), (375, True)]).probation_start, + "the original D196 end date is gone", + ) + self.assertIsNone( + _run(served + [(195, False), (196, True), (376, True)]).probation_start, + "it ends 180 days after the restart instead", + ) + + def test_a_failure_on_the_last_day_of_probation_restarts_it(self): + """Expiry is only ever checked on a passing day, so a failure that day wins.""" + served = [(0, True)] + _failing(1, 16) + [(16, True)] + state = _run(served + [(196, False)]) + self.assertEqual(state.probation_start, _day(197)) + + def test_two_runs_on_the_same_failing_day_stamp_the_same_start(self): + """The `+1 day` is stamped from the UTC day, so the hour of the run cannot shift it.""" + state = None + for moment in (_day(1) + timedelta(hours=2), _day(1) + timedelta(hours=23)): + state = transition( + prev=state, + observation=_observation(False), + grace_period=None, + probation_period=PROBATION_PERIOD, + now=moment, + feed_id=FEED_ID, + ) + self.assertEqual(state.probation_start, _day(2)) + def test_an_observed_failure_during_probation_restarts_it(self): """Confirmed or not: during probation any failure costs the whole count.""" days = [(0, True)] + _failing(1, 16) + [(16, True), (100, False), (101, True)] diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py index f94a78e1a..f4a4c5499 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py @@ -17,9 +17,11 @@ import unittest from datetime import datetime, timedelta, timezone +from unittest.mock import patch from tasks.seal_of_reliability.context import build_contexts, get_seal_feeds_query -from tasks.seal_of_reliability.criteria import SealCriterionName +from tasks.seal_of_reliability.criteria import PROBATION_PERIOD, SealCriterionName +from tasks.seal_of_reliability.evaluators import CriterionEvaluator, OfficialEvaluator from tasks.seal_of_reliability.seal_updater import update_seals from sqlalchemy import delete, select @@ -48,6 +50,56 @@ OURS = [OFFICIAL, NOT_OFFICIAL, UNKNOWN_OFFICIAL, INACTIVE] +class _StandInEvaluator(CriterionEvaluator): + """A criterion with a grace period and probation, for the probation tests below. + + Official is the only real evaluator and has neither, so without a stand-in nothing + exercises `probation_start` against a database: it would never be persisted, reloaded, + or seen by the `has_seal` roll-up with a value other than NULL. + + It reads `official` so a test can drive it with the existing `set_official` helper, but + unlike Official it debounces failures and serves probation afterwards. It borrows the + `available` enum value, which has no evaluator of its own yet (#1784). + """ + + name = SealCriterionName.AVAILABLE + grace_period = timedelta(days=14) + + def _evaluate(self, ctx): + return ctx.official is True, f"stand-in, feed.official is {ctx.official!r}" + + +# Patched over the registry so the roll-up sees a criterion that can be on probation. +WITH_PROBATION = [OfficialEvaluator(), _StandInEvaluator()] + + +DARK_FROM = NOW + timedelta(days=2) + + +class _GoesDarkEvaluator(CriterionEvaluator): + """A criterion that loses its upstream input partway through, standing in for #1784. + + It returns no verdict from `DARK_FROM` onwards, keyed on the clock rather than on + `official` so that a test can drive it and Official in opposite directions at the same + moment. No grace period, so its verdicts land immediately and the tests are about what + happens once it goes quiet. + """ + + name = SealCriterionName.COMPLIANT + grace_period = None + + def _evaluate(self, ctx): + if ctx.now >= DARK_FROM: + return None, "stand-in has no input this run" + return ctx.official is True, f"stand-in, feed.official is {ctx.official!r}" + + +# Official is kept in the registry on purpose: it is passing by the time the stand-in goes +# dark, so if the stand-in's frozen row were dropped from the roll-up the seal would come +# straight back. Without it, the empty-in_service guard would mask that. +GOES_DARK = [OfficialEvaluator(), _GoesDarkEvaluator()] + + def _seed_feed( db_session, feed_id: str, @@ -365,5 +417,215 @@ def test_limit_caps_the_feeds_evaluated(self): self.assertLessEqual(report["total_feeds"], 2) +@patch("tasks.seal_of_reliability.seal_updater.EVALUATORS", WITH_PROBATION) +class TestProbation(SealDbTestCase): + """Probation persisted and rolled up, driven by `_StandInEvaluator`. + + Timestamps are written out rather than derived so the assertions do not restate the + implementation they are checking. + """ + + STREAK_STARTS = datetime(2026, 6, 2, 12, 0, tzinfo=timezone.utc) + CONFIRMED_AT = datetime(2026, 6, 16, 12, 0, tzinfo=timezone.utc) # 14 days later + REPAIRED_AT = datetime(2026, 6, 17, 12, 0, tzinfo=timezone.utc) + PROBATION_FROM = datetime( + 2026, 6, 17, 0, 0, tzinfo=timezone.utc + ) # the day of repair + + def stand_in(self): + return self.criterion_rows(OFFICIAL)[SealCriterionName.AVAILABLE.value] + + def run_at(self, moment): + update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=moment) + + def test_a_clean_run_puts_nothing_on_probation(self): + self.run_at(NOW) + self.assertIsNone(self.stand_in().probation_start) + self.assertTrue(self.seal_row(OFFICIAL).has_seal) + + def test_a_failure_inside_the_grace_period_starts_no_probation(self): + self.run_at(NOW) + self.set_official(OFFICIAL, False) + self.run_at(self.STREAK_STARTS) + + row = self.stand_in() + self.assertFalse(row.observed_pass) + self.assertTrue(row.confirmed_pass, "still inside its grace period") + self.assertIsNone(row.probation_start) + + def test_probation_starts_on_the_day_of_repair(self): + self.run_at(NOW) + self.set_official(OFFICIAL, False) + self.run_at(self.STREAK_STARTS) + self.run_at(self.CONFIRMED_AT) + + row = self.stand_in() + self.assertFalse(row.confirmed_pass, "the streak outlasted the grace period") + self.assertEqual(row.probation_start, self.PROBATION_FROM) + + def test_probation_withholds_the_seal_while_every_criterion_passes(self): + self.run_at(NOW) + self.set_official(OFFICIAL, False) + self.run_at(self.STREAK_STARTS) + self.run_at(self.CONFIRMED_AT) + self.set_official(OFFICIAL, True) + self.run_at(self.REPAIRED_AT) + + rows = self.criterion_rows(OFFICIAL) + self.assertTrue(rows[SealCriterionName.OFFICIAL.value].confirmed_pass) + self.assertTrue(rows[SealCriterionName.AVAILABLE.value].confirmed_pass) + self.assertEqual( + rows[SealCriterionName.AVAILABLE.value].probation_start, + self.PROBATION_FROM, + "recovery clears the status but starts probation", + ) + self.assertFalse( + self.seal_row(OFFICIAL).has_seal, + "both criteria pass, so only the open probation can be withholding it", + ) + + def test_the_seal_returns_once_probation_is_served(self): + self.run_at(NOW) + self.set_official(OFFICIAL, False) + self.run_at(self.STREAK_STARTS) + self.run_at(self.CONFIRMED_AT) + self.set_official(OFFICIAL, True) + self.run_at(self.REPAIRED_AT) + + served_at = self.PROBATION_FROM + PROBATION_PERIOD + self.run_at(served_at) + + self.assertIsNone(self.stand_in().probation_start) + seal = self.seal_row(OFFICIAL) + self.assertTrue(seal.has_seal) + self.assertEqual(seal.seal_earned_at, served_at) + + +class TestCriterionBroughtIntoServiceLate(SealDbTestCase): + """A criterion whose evaluator arrives after the feed already has rows. + + This is the `available` case from #1761: its source only starts collecting on some date, + so until then it has no row at all and the roll-up has to carry on without it. Each test + runs once with Official alone, then again with the stand-in registered. + """ + + LATER = NOW + timedelta(days=1) + + def run_at(self, moment, evaluators): + with patch("tasks.seal_of_reliability.seal_updater.EVALUATORS", evaluators): + update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=moment) + + def test_a_criterion_with_no_row_does_not_deny_the_seal(self): + """The waiver itself: an unevaluated criterion is skipped, not counted as failing.""" + self.run_at(NOW, [OfficialEvaluator()]) + + self.assertEqual( + set(self.criterion_rows(OFFICIAL)), + {SealCriterionName.OFFICIAL.value}, + "the stand-in has no row yet", + ) + self.assertTrue(self.seal_row(OFFICIAL).has_seal) + + def test_a_passing_newcomer_joins_without_disturbing_the_seal(self): + self.run_at(NOW, [OfficialEvaluator()]) + earned_at = self.seal_row(OFFICIAL).seal_earned_at + + self.run_at(self.LATER, WITH_PROBATION) + + row = self.criterion_rows(OFFICIAL)[SealCriterionName.AVAILABLE.value] + self.assertTrue(row.confirmed_pass) + self.assertIsNone(row.probation_start, "a first verdict is not a recovery") + seal = self.seal_row(OFFICIAL) + self.assertTrue(seal.has_seal) + self.assertEqual( + seal.seal_earned_at, + earned_at, + "the seal was never withdrawn and re-granted", + ) + self.assertIsNone(seal.seal_lost_at) + + def test_a_newcomer_whose_first_verdict_fails_gets_no_grace_period(self): + """`first_evaluation` denies the grace period: nothing has been earned to hold. + + The stand-in does have a 14-day grace period, and it is deliberately not applied + here — otherwise this feed would keep its seal for a fortnight on a criterion we + have never once seen pass. So the failure is a confirmed failure on day one. + """ + self.run_at(NOW, [OfficialEvaluator()]) + self.assertTrue(self.seal_row(OFFICIAL).has_seal) + + self.set_official(OFFICIAL, False) + self.run_at(self.LATER, WITH_PROBATION) + + row = self.criterion_rows(OFFICIAL)[SealCriterionName.AVAILABLE.value] + self.assertFalse(row.observed_pass) + self.assertFalse( + row.confirmed_pass, "a confirmed failure on its very first verdict" + ) + self.assertEqual(row.first_observed_failure_at, self.LATER) + self.assertEqual(row.last_confirmed_failure_at, self.LATER) + + seal = self.seal_row(OFFICIAL) + self.assertFalse(seal.has_seal) + self.assertEqual(seal.seal_lost_at, self.LATER) + + +@patch("tasks.seal_of_reliability.seal_updater.EVALUATORS", GOES_DARK) +class TestCriterionThatStopsBeingEvaluable(SealDbTestCase): + """A criterion that produced a verdict once and then loses its input. + + The roll-up skips criteria that have *never* produced a verdict, so the safety of that + rule rests entirely on a criterion never falling back into that state once it has one. + """ + + FAILED_AT = NOW + timedelta(days=1) + + def stand_in(self): + return self.criterion_rows(OFFICIAL).get(SealCriterionName.COMPLIANT.value) + + def run_at(self, moment): + update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=moment) + + def test_going_dark_freezes_the_verdict_and_keeps_the_seal_withheld(self): + # Both criteria pass, then both fail and the seal goes. + self.run_at(NOW) + self.assertTrue(self.seal_row(OFFICIAL).has_seal) + + self.set_official(OFFICIAL, False) + self.run_at(self.FAILED_AT) + self.assertFalse(self.seal_row(OFFICIAL).has_seal) + + # Official recovers, but the stand-in has lost its input. Its stored failure must + # stand and keep withholding the seal. + self.set_official(OFFICIAL, True) + self.run_at(DARK_FROM) + + rows = self.criterion_rows(OFFICIAL) + self.assertTrue( + rows[SealCriterionName.OFFICIAL.value].confirmed_pass, + "the only other criterion is passing again", + ) + frozen = rows[SealCriterionName.COMPLIANT.value] + self.assertFalse(frozen.observed_pass, "the last verdict stands") + self.assertFalse(frozen.confirmed_pass) + self.assertEqual( + frozen.evaluated_at, self.FAILED_AT, "not re-stamped without a verdict" + ) + self.assertFalse( + self.seal_row(OFFICIAL).has_seal, + "going dark must not hand the seal back", + ) + + def test_going_dark_before_any_verdict_writes_no_row(self): + """The other half: with no verdict ever, there is nothing to hold in service.""" + self.run_at(DARK_FROM) + + self.assertIsNone(self.stand_in(), "no row is written without a verdict") + self.assertTrue( + self.seal_row(OFFICIAL).has_seal, + "and the criterion is skipped rather than denying the seal", + ) + + if __name__ == "__main__": unittest.main() diff --git a/liquibase/changes/feat_1783.sql b/liquibase/changes/feat_1783.sql index b0a0166a7..3b51a0a05 100644 --- a/liquibase/changes/feat_1783.sql +++ b/liquibase/changes/feat_1783.sql @@ -1,43 +1,24 @@ --- Issue #1783: revised Seal of Reliability algorithm (spec in #1761). --- Adjusts the tables added by feat_1760.sql. Nothing reads them yet, so this is safe to --- apply as a rename rather than an additive migration. +-- Issue #1783: Seal of Reliability algorithm (spec in #1761). Adjusts the tables added by +-- feat_1760.sql. Nothing reads them yet, so this renames in place. --- 1. The two status booleans flip to positive polarity and are renamed. --- `has_seal` is already positive and every criterion check is written positively --- (`success = TRUE`, `total_error = 0`), so negative status columns forced an inversion --- at both ends. `raw` also described how a value was produced rather than what it means --- and had no opposite pole, which left `grace` on a different axis; observed -> --- confirmed is one axis, and the grace period is the distance along it. +-- The two status booleans become positive: TRUE now means the criterion passed. ALTER TABLE SealCriterion RENAME COLUMN raw_failing TO observed_pass; ALTER TABLE SealCriterion RENAME COLUMN grace_failing TO confirmed_pass; +UPDATE SealCriterion SET observed_pass = NOT observed_pass WHERE observed_pass IS NOT NULL; +UPDATE SealCriterion SET confirmed_pass = NOT confirmed_pass WHERE confirmed_pass IS NOT NULL; --- The failure timestamps keep their negative sense on purpose: they record events that --- really are failures, and the grace period is measured from the start of a bad streak. --- Booleans describe state, timestamps record events. +-- The timestamps stay negative: they record events that really are failures. ALTER TABLE SealCriterion RENAME COLUMN first_raw_failure_at TO first_observed_failure_at; ALTER TABLE SealCriterion RENAME COLUMN last_raw_failure_at TO last_observed_failure_at; ALTER TABLE SealCriterion RENAME COLUMN last_grace_failure_at TO last_confirmed_failure_at; --- These are inversions, not just renames: TRUE meant failing and now means passing. --- NULL keeps its meaning of "not yet evaluated" and must stay NULL. -UPDATE SealCriterion SET observed_pass = NOT observed_pass WHERE observed_pass IS NOT NULL; -UPDATE SealCriterion SET confirmed_pass = NOT confirmed_pass WHERE confirmed_pass IS NOT NULL; +ALTER TABLE SealCriterion ADD COLUMN IF NOT EXISTS probation_start TIMESTAMPTZ; COMMENT ON COLUMN SealCriterion.observed_pass IS 'The criterion''s own check on this day, with no debouncing. NULL = never evaluated.'; COMMENT ON COLUMN SealCriterion.confirmed_pass IS - 'The debounced status. FALSE once an observed failure outlasts the grace period, or ' - 'immediately for a criterion with no grace period. NULL = never evaluated.'; - --- 2. probation_start replaces the reliability window, which used to be an implicit lookback --- over last_grace_failure_at. Probation is a penalty rather than an entry requirement: a --- criterion is put on it by a recovery, never by a first evaluation, so a feed that has --- never had a confirmed failure can hold the seal from its first evaluation. -ALTER TABLE SealCriterion ADD COLUMN IF NOT EXISTS probation_start TIMESTAMPTZ; - + 'The debounced status, FALSE once an observed failure outlasts the grace period. ' + 'NULL = never evaluated.'; COMMENT ON COLUMN SealCriterion.probation_start IS - 'Start of the 180-day stretch with no observed failure that this criterion must ' - 'complete before it can contribute to the seal again. Started by a recovery from a ' - 'confirmed failure, or by a recovery from an observed failure while already on ' - 'probation. NULL = not on probation: nothing has gone wrong yet, probation was served ' - 'out, or the criterion has no probation at all.'; + 'Start of the 180 days with no observed failure a criterion serves after recovering ' + 'from a confirmed failure. NULL = not on probation.'; From faa2cd094e91c65c60fee0dad057b61d44863ee0 Mon Sep 17 00:00:00 2001 From: jcpitre Date: Wed, 12 Aug 2026 14:36:55 -0400 Subject: [PATCH 09/10] Capped returned json size. --- functions-python/tasks_executor/README.md | 10 ++- .../tasks/seal_of_reliability/seal_updater.py | 80 ++++++++++++++++--- .../update_seal_of_reliability.py | 15 +++- .../test_seal_updater_db.py | 59 +++++++++++++- 4 files changed, 143 insertions(+), 21 deletions(-) diff --git a/functions-python/tasks_executor/README.md b/functions-python/tasks_executor/README.md index 37bdb2bdd..7e0c61768 100644 --- a/functions-python/tasks_executor/README.md +++ b/functions-python/tasks_executor/README.md @@ -433,10 +433,11 @@ freezes its stored rows rather than making it neutral. | Parameter | Type | Default | Description | |---|---|---|---| | `dry_run` | bool | `true` | Evaluate every feed and return the report without writing anything | -| `stable_feed_ids` | list[str] \| null | `null` | Evaluate only these feeds; unknown ids raise. When set, `evaluations` covers every criterion of those feeds instead of only the ones whose verdict moved | +| `stable_feed_ids` | list[str] \| null | `null` | Evaluate only these feeds. Ids that are unknown or not eligible are skipped with a logged warning naming them; it raises only when *none* of them can be evaluated. When set, `evaluations` covers every criterion of those feeds instead of only the ones whose verdict moved | | `limit` | int \| null | `null` | Cap the number of feeds evaluated | | `criteria` | list[str] \| null | `null` | Evaluate only these criteria. Naming a criterion that has no evaluator yet raises. A subset of the implemented criteria skips the `has_seal` roll-up, since the ones not evaluated cannot be judged | -| `batch_size` | int | `200` | Feeds loaded per query batch | +| `batch_size` | int | `200` | Feeds loaded per query batch. Every eligible feed is still evaluated — this only sizes the queries | +| `max_reported_evaluations` | int | `50` | Cap on the `evaluations` list in the response. Everything is still evaluated and written; `evaluations_omitted` says how many entries were left out | | `now` | str \| null | `null` | ISO timestamp to evaluate against, for replays and backfills. Defaults to the current UTC time | **Response fields**: @@ -451,9 +452,10 @@ freezes its stored rows rather than making it neutral. | `seals_before_run` | Feeds that held the seal before this run | | `seals_after_run` | Feeds holding it afterwards — on a dry run, what *would* be stored | | `seals_granted` / `seals_revoked` | Transitions in this run. `before + granted - revoked == after` | -| `revoked_stable_ids` | Feeds that lost the seal in this run | +| `granted_stable_ids` / `revoked_stable_ids` | The feeds behind `seals_granted` / `seals_revoked` — the two transitions written to `feedreliabilityseal` in this run | | `first_evaluations` | Criteria evaluated for the first time (no stored row yet) | -| `evaluations` | The notable outcomes: one entry per feed and criterion whose verdict moved, with `observed_pass`, `confirmed_pass`, `previously_confirmed_pass`, `on_probation` and `reason`. A first evaluation appears only if it landed on a failure; the rest are counted by `first_evaluations`. When `stable_feed_ids` is set, every criterion of those feeds is included whether or not it moved | +| `evaluations` | The notable outcomes, capped at `max_reported_evaluations`: one entry per feed and criterion whose verdict moved, with `observed_pass`, `confirmed_pass`, `previously_confirmed_pass`, `on_probation` and `reason`. A first evaluation appears only if it landed on a failure; the rest are counted by `first_evaluations`. When `stable_feed_ids` is set, every criterion of those feeds is included whether or not it moved | +| `evaluations_omitted` | Entries left out of `evaluations` by the cap. `sealcriterion` holds every verdict regardless | #### Running it locally diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py index 05bffdb39..1b0412834 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py @@ -29,14 +29,18 @@ import logging import time from datetime import datetime, timezone -from typing import Dict, List, Optional, Sequence, Tuple +from typing import Dict, List, Optional, Sequence, Set, Tuple from sqlalchemy import select from sqlalchemy.dialects.postgresql import insert from sqlalchemy.orm import Session from shared.database.database import with_db_session -from shared.database_gen.sqlacodegen_models import Feedreliabilityseal, Sealcriterion +from shared.database_gen.sqlacodegen_models import ( + Feed, + Feedreliabilityseal, + Sealcriterion, +) from tasks.seal_of_reliability.context import ( batched, @@ -49,6 +53,9 @@ DEFAULT_BATCH_SIZE: int = 200 +# Limit the number of evaluation reported to so the return does not get gigantic. +DEFAULT_MAX_REPORTED_EVALUATIONS: int = 50 + SEAL_TABLE = Feedreliabilityseal.__table__ CRITERION_TABLE = Sealcriterion.__table__ @@ -67,6 +74,43 @@ def _resolve_evaluators(criteria: Optional[Sequence[str]]) -> List: return [evaluator for evaluator in EVALUATORS if evaluator.name.value in wanted] +def _validate_requested_feed_ids( + db_session: Session, + requested: Sequence[str], + evaluated: Set[str], +) -> None: + """Warn about requested feeds that will not be evaluated, or raise if that is all of them.""" + unusable = sorted(set(requested) - evaluated) + if not unusable: + return # every requested feed is being evaluated, so there is nothing to report + + # One extra query, and only now that something has already been dropped. + existing = { + row.stable_id + for row in db_session.execute( + select(Feed.stable_id).where(Feed.stable_id.in_(unusable)) + ).all() + } + unknown = sorted(set(unusable) - existing) + + problems = [] + if unknown: + problems.append(f"not found: {unknown}") + if existing: + problems.append( + f"not eligible for the seal: {sorted(existing)} (must be gtfs, " + "operational_status=published, and status not in (deprecated, development))" + ) + summary = "; ".join(problems) + + if not evaluated: + # The run did nothing at all, which a log line is too quiet for. + raise ValueError(f"no requested feed can be evaluated — {summary}") + + # Only a warning, so one stale id does not cost a run over fifty feeds. + logging.warning("Skipping %d of the requested feed(s): %s", len(unusable), summary) + + def _load_previous_states( db_session: Session, feed_ids: Sequence[str] ) -> Dict[Tuple[str, str], SealCriterionState]: @@ -241,6 +285,7 @@ def update_seals( criteria: Optional[Sequence[str]] = None, batch_size: int = DEFAULT_BATCH_SIZE, now: Optional[datetime] = None, + max_reported_evaluations: int = DEFAULT_MAX_REPORTED_EVALUATIONS, ) -> dict: """Evaluate the seal criteria for every eligible feed and store the result. @@ -250,14 +295,17 @@ def update_seals( Args: db_session: SQLAlchemy session, injected by @with_db_session. dry_run: Evaluate and report without writing. Default True. - stable_feed_ids: Evaluate only these feeds. Unknown ids raise. When given, - `evaluations` reports every criterion of those feeds rather than only the - ones whose verdict moved. + stable_feed_ids: Evaluate only these feeds. Unknown or ineligible ids are skipped + with a logged warning; it raises only if none of them can be evaluated. When + given, `evaluations` reports every criterion of those feeds rather than only + the ones whose verdict moved. limit: Cap the number of feeds evaluated. criteria: Evaluate only these criteria. A partial set skips the has_seal roll-up, since the criteria that were not evaluated cannot be judged. batch_size: Feeds loaded per query batch. now: Evaluation timestamp. Defaults to the current UTC time. + max_reported_evaluations: Cap on the `evaluations` list in the report. The count dropped is + always returned as `evaluations_omitted`. Returns: A report dict. @@ -273,9 +321,9 @@ def update_seals( feeds = query.all() if stable_feed_ids is not None: - missing = sorted(set(stable_feed_ids) - {feed.stable_id for feed in feeds}) - if missing: - raise ValueError(f"stable_feed_ids not found: {missing}") + _validate_requested_feed_ids( + db_session, stable_feed_ids, {feed.stable_id for feed in feeds} + ) logging.info( "Evaluating %d criterion/criteria for %d feed(s) (dry_run=%s, now=%s).", @@ -410,6 +458,9 @@ def update_seals( "seals_after_run": sum(1 for outcome in outcomes if outcome["has_seal"]), "seals_granted": len(granted), "seals_revoked": len(revoked), + # The two transitions in feedreliabilityseal, by feed. Counts alone cannot say + # which feed moved, and that is the first thing anyone asks of a run. + "granted_stable_ids": [outcome["stable_id"] for outcome in granted], "revoked_stable_ids": [outcome["stable_id"] for outcome in revoked], "elapsed_seconds": round(time.monotonic() - started, 2), } @@ -418,11 +469,14 @@ def update_seals( "Partial criteria run: has_seal was not recalculated because the criteria " "that were not evaluated cannot be judged." ) - report["evaluations"] = evaluations - - # Log without `evaluations`: Cloud Logging drops a LogEntry over 256 KB and an entry is - # ~424 bytes, so a run naming a few hundred feeds would lose the whole log entry. The - # counts belong in logs; `evaluations` is for the caller reading the response. + # A sample, not the record: sealcriterion holds every verdict. The omitted count is + # always present so a truncated list can never be mistaken for the whole story. + report["evaluations"] = evaluations[:max_reported_evaluations] + report["evaluations_omitted"] = max(0, len(evaluations) - max_reported_evaluations) + + # Log without `evaluations`: Cloud Logging drops a LogEntry over 256 KB, so a run + # naming a few hundred feeds would lose the whole log entry. The counts belong in logs; + # `evaluations` is for the caller reading the response. logging.info( "Task completed: %s", {key: value for key, value in report.items() if key != "evaluations"}, diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py index 731c2f71a..f581e5a07 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py @@ -17,7 +17,11 @@ from datetime import datetime -from tasks.seal_of_reliability.seal_updater import DEFAULT_BATCH_SIZE, update_seals +from tasks.seal_of_reliability.seal_updater import ( + DEFAULT_BATCH_SIZE, + DEFAULT_MAX_REPORTED_EVALUATIONS, + update_seals, +) def get_parameters(payload: dict): @@ -30,6 +34,7 @@ def get_parameters(payload: dict): payload.get("criteria", None), payload.get("batch_size", DEFAULT_BATCH_SIZE), datetime.fromisoformat(now) if now else None, + payload.get("max_reported_evaluations", DEFAULT_MAX_REPORTED_EVALUATIONS), ) @@ -46,9 +51,13 @@ def update_seal_of_reliability_handler(payload: dict) -> dict: limit (int | None): Cap the number of feeds evaluated. Default: no limit. criteria (list[str] | None): Evaluate only these criteria. A partial set skips the has_seal roll-up. Default: None (every implemented criterion). - batch_size (int): Feeds loaded per query batch. Default: 200. + batch_size (int): Feeds loaded per query batch. Every eligible feed is still + evaluated; this only sizes the queries. Default: 200. now (str | None): ISO timestamp to evaluate against, for replays and backfills. Default: current UTC time. + max_reported_evaluations (int): Cap on the `evaluations` list in the response. Everything is + still evaluated and written; `evaluations_omitted` reports how many + entries were left out. Default: 50. """ ( dry_run, @@ -57,6 +66,7 @@ def update_seal_of_reliability_handler(payload: dict) -> dict: criteria, batch_size, now, + max_reported_evaluations, ) = get_parameters(payload) return update_seals( dry_run=dry_run, @@ -65,4 +75,5 @@ def update_seal_of_reliability_handler(payload: dict) -> dict: criteria=criteria, batch_size=batch_size, now=now, + max_reported_evaluations=max_reported_evaluations, ) diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py index f4a4c5499..4cf69096d 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py @@ -243,6 +243,20 @@ def test_dry_run_counts_are_prospective(self): self.assertEqual(report["seals_revoked"], 0) self.assertIsNone(self.seal_row(OFFICIAL), "still a dry run") + def test_both_seal_transitions_are_reported_by_feed(self): + """Counts say how many moved; these say which, for each direction.""" + first = update_seals(dry_run=False, stable_feed_ids=OURS, now=NOW) + self.assertEqual( + sorted(first["granted_stable_ids"]), sorted([OFFICIAL, INACTIVE]) + ) + self.assertEqual(first["revoked_stable_ids"], []) + + self.set_official(OFFICIAL, False) + later = NOW + timedelta(days=1) + second = update_seals(dry_run=False, stable_feed_ids=OURS, now=later) + self.assertEqual(second["granted_stable_ids"], []) + self.assertEqual(second["revoked_stable_ids"], [OFFICIAL]) + def test_seal_counts_balance(self): """before + granted - revoked == after, across a grant then a revocation.""" first = update_seals(dry_run=False, stable_feed_ids=OURS, now=NOW) @@ -408,14 +422,55 @@ def test_criterion_without_an_evaluator_raises(self): with self.assertRaises(ValueError): update_seals(criteria=[SealCriterionName.STABLE.value], now=NOW) - def test_unknown_stable_feed_id_raises(self): - with self.assertRaises(ValueError): + def test_a_run_with_no_usable_feed_raises(self): + """Nothing was evaluated, so a report saying so would be too quiet.""" + with self.assertRaises(ValueError) as caught: update_seals(stable_feed_ids=[f"{PREFIX}does_not_exist"], now=NOW) + self.assertIn("not found", str(caught.exception)) + + def test_an_ineligible_feed_says_so_rather_than_not_found(self): + """A filtered-out feed is in the database, so "not found" would send you hunting.""" + with self.assertRaises(ValueError) as caught: + update_seals(stable_feed_ids=[DEPRECATED], now=NOW) + message = str(caught.exception) + self.assertIn("not eligible", message) + self.assertIn(DEPRECATED, message) + self.assertNotIn("not found", message) + + def test_unusable_feeds_are_dropped_rather_than_costing_the_run(self): + """One stale id must not cost a run over the feeds that are fine.""" + with self.assertLogs(level="WARNING") as logs: + report = update_seals( + dry_run=False, + stable_feed_ids=[OFFICIAL, DEPRECATED, f"{PREFIX}does_not_exist"], + now=NOW, + ) + self.assertEqual(report["total_feeds"], 1, "the eligible feed still ran") + self.assertTrue(self.seal_row(OFFICIAL).has_seal) + + warning = "\n".join(logs.output) + self.assertIn(DEPRECATED, warning) + self.assertIn(f"{PREFIX}does_not_exist", warning) def test_limit_caps_the_feeds_evaluated(self): report = update_seals(dry_run=True, limit=2, now=NOW) self.assertLessEqual(report["total_feeds"], 2) + def test_evaluations_are_capped_without_capping_the_run(self): + """The list is a sample; sealcriterion is the record.""" + report = update_seals( + dry_run=False, stable_feed_ids=OURS, now=NOW, max_reported_evaluations=2 + ) + self.assertEqual(len(report["evaluations"]), 2) + self.assertEqual(report["evaluations_omitted"], 2, "4 feeds, 1 criterion each") + self.assertEqual(report["total_feeds"], 4, "every feed was still evaluated") + self.assertEqual(len(self.criterion_rows(OFFICIAL)), 1, "and still written") + + def test_evaluations_omitted_is_zero_when_nothing_was_dropped(self): + report = update_seals(dry_run=True, stable_feed_ids=[OFFICIAL], now=NOW) + self.assertEqual(len(report["evaluations"]), 1) + self.assertEqual(report["evaluations_omitted"], 0) + @patch("tasks.seal_of_reliability.seal_updater.EVALUATORS", WITH_PROBATION) class TestProbation(SealDbTestCase): From 727d0ca96f3065b86424e2d2581b89af46e36a6f Mon Sep 17 00:00:00 2001 From: jcpitre Date: Wed, 12 Aug 2026 15:09:16 -0400 Subject: [PATCH 10/10] Refactored json return. --- functions-python/tasks_executor/README.md | 12 +- functions-python/tasks_executor/src/main.py | 2 +- .../tasks/seal_of_reliability/seal_updater.py | 131 ++++++++++-------- .../update_seal_of_reliability.py | 12 +- .../test_seal_end_to_end_db.py | 26 ++-- .../test_seal_updater_db.py | 73 +++++----- 6 files changed, 141 insertions(+), 115 deletions(-) diff --git a/functions-python/tasks_executor/README.md b/functions-python/tasks_executor/README.md index 7e0c61768..1ad81f3fa 100644 --- a/functions-python/tasks_executor/README.md +++ b/functions-python/tasks_executor/README.md @@ -433,11 +433,11 @@ freezes its stored rows rather than making it neutral. | Parameter | Type | Default | Description | |---|---|---|---| | `dry_run` | bool | `true` | Evaluate every feed and return the report without writing anything | -| `stable_feed_ids` | list[str] \| null | `null` | Evaluate only these feeds. Ids that are unknown or not eligible are skipped with a logged warning naming them; it raises only when *none* of them can be evaluated. When set, `evaluations` covers every criterion of those feeds instead of only the ones whose verdict moved | +| `stable_feed_ids` | list[str] \| null | `null` | Evaluate only these feeds. Ids that are unknown or not eligible are skipped with a logged warning naming them; it raises only when *none* of them can be evaluated. When set, `feeds` covers every one of those feeds instead of only the ones that moved | | `limit` | int \| null | `null` | Cap the number of feeds evaluated | | `criteria` | list[str] \| null | `null` | Evaluate only these criteria. Naming a criterion that has no evaluator yet raises. A subset of the implemented criteria skips the `has_seal` roll-up, since the ones not evaluated cannot be judged | | `batch_size` | int | `200` | Feeds loaded per query batch. Every eligible feed is still evaluated — this only sizes the queries | -| `max_reported_evaluations` | int | `50` | Cap on the `evaluations` list in the response. Everything is still evaluated and written; `evaluations_omitted` says how many entries were left out | +| `max_reported_feeds` | int | `50` | Cap on the `feeds` list in the response. Everything is still evaluated and written; `feeds_omitted` says how many entries were left out | | `now` | str \| null | `null` | ISO timestamp to evaluate against, for replays and backfills. Defaults to the current UTC time | **Response fields**: @@ -454,8 +454,8 @@ freezes its stored rows rather than making it neutral. | `seals_granted` / `seals_revoked` | Transitions in this run. `before + granted - revoked == after` | | `granted_stable_ids` / `revoked_stable_ids` | The feeds behind `seals_granted` / `seals_revoked` — the two transitions written to `feedreliabilityseal` in this run | | `first_evaluations` | Criteria evaluated for the first time (no stored row yet) | -| `evaluations` | The notable outcomes, capped at `max_reported_evaluations`: one entry per feed and criterion whose verdict moved, with `observed_pass`, `confirmed_pass`, `previously_confirmed_pass`, `on_probation` and `reason`. A first evaluation appears only if it landed on a failure; the rest are counted by `first_evaluations`. When `stable_feed_ids` is set, every criterion of those feeds is included whether or not it moved | -| `evaluations_omitted` | Entries left out of `evaluations` by the cap. `sealcriterion` holds every verdict regardless | +| `feeds` | One entry per reported feed, capped at `max_reported_feeds`: `stable_id`, its `feedreliabilityseal` state (`had_seal`, `has_seal`), and a nested `criteria` list holding every criterion of that feed with `observed_pass`, `confirmed_pass`, `previously_confirmed_pass`, `on_probation` and `reason`. A feed is reported when it was named in `stable_feed_ids`, when one of its criteria moved, or when its seal changed — so a quiet nightly run returns an empty list | +| `feeds_omitted` | Feeds left out of `feeds` by the cap. `sealcriterion` and `feedreliabilityseal` hold everything regardless | #### Running it locally @@ -473,7 +473,7 @@ curl -s -X POST http://localhost:8080 -H "Content-Type: application/json" \ ``` `Accept: text/csv` returns a single summary row (the top-level report fields), not one row -per evaluation — the converter flattens the returned dict, and `evaluations` lands in it as -a single stringified cell. Use the JSON response for per-criterion detail. +per feed — the converter flattens the returned dict, and `feeds` lands in it as a single +stringified cell. Use the JSON response for per-feed and per-criterion detail. Nothing about this task needs GCP credentials — only `FEEDS_DATABASE_URL`. diff --git a/functions-python/tasks_executor/src/main.py b/functions-python/tasks_executor/src/main.py index 082e12f03..00a91913c 100644 --- a/functions-python/tasks_executor/src/main.py +++ b/functions-python/tasks_executor/src/main.py @@ -264,7 +264,7 @@ "GTFS feed and updates sealcriterion and feedreliabilityseal. " "Reads the source tables and never modifies them. " "Parameters: dry_run (default true), stable_feed_ids (default null; when set, " - "`evaluations` covers every criterion of those feeds), limit (default null), " + "`feeds` covers every one of those feeds), limit (default null), " "criteria (default null " "meaning every implemented criterion; a partial set skips the has_seal " "roll-up), batch_size (default 200), now (ISO timestamp, default current " diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py index 1b0412834..10661e47e 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/seal_updater.py @@ -53,8 +53,8 @@ DEFAULT_BATCH_SIZE: int = 200 -# Limit the number of evaluation reported to so the return does not get gigantic. -DEFAULT_MAX_REPORTED_EVALUATIONS: int = 50 +# Limit the number of feeds reported so the return does not get gigantic. +DEFAULT_MAX_REPORTED_FEEDS: int = 50 SEAL_TABLE = Feedreliabilityseal.__table__ CRITERION_TABLE = Sealcriterion.__table__ @@ -285,7 +285,7 @@ def update_seals( criteria: Optional[Sequence[str]] = None, batch_size: int = DEFAULT_BATCH_SIZE, now: Optional[datetime] = None, - max_reported_evaluations: int = DEFAULT_MAX_REPORTED_EVALUATIONS, + max_reported_feeds: int = DEFAULT_MAX_REPORTED_FEEDS, ) -> dict: """Evaluate the seal criteria for every eligible feed and store the result. @@ -304,8 +304,8 @@ def update_seals( since the criteria that were not evaluated cannot be judged. batch_size: Feeds loaded per query batch. now: Evaluation timestamp. Defaults to the current UTC time. - max_reported_evaluations: Cap on the `evaluations` list in the report. The count dropped is - always returned as `evaluations_omitted`. + max_reported_feeds: Cap on the `feeds` list in the report. The count dropped is + always returned as `feeds_omitted`. Returns: A report dict. @@ -335,9 +335,10 @@ def update_seals( all_states: List[SealCriterionState] = [] outcomes: List[dict] = [] - evaluations: List[dict] = [] + feed_reports: List[dict] = [] not_evaluable = 0 first_evaluations = 0 + is_feed_list_provided = stable_feed_ids is not None for batch in batched(feeds, batch_size): batch_ids = [feed.id for feed in batch] @@ -348,6 +349,8 @@ def update_seals( for feed in batch: ctx = contexts[feed.id] feed_states: Dict[str, SealCriterionState] = {} + criteria_report: List[dict] = [] + anything_moved = False for evaluator in evaluators: observation = evaluator.evaluate(ctx) @@ -369,37 +372,34 @@ def update_seals( if previous is None and state is not None: first_evaluations += 1 - # `evaluations` reports the notable outcomes, not one entry per - # evaluation: a criterion whose verdict moved, or every criterion of a feed - # the caller named explicitly. One entry per feed per criterion would grow - # with the catalogue (~424 bytes each, so megabytes for a full run) and is - # what the sealcriterion table is for. This matches `failures` in - # check_gtfs_feed_availability and `dispatched` in backfill_changelog. - named = stable_feed_ids is not None - if named or _is_notable(previous, state): - evaluations.append( - { - "stable_id": ctx.stable_id, - "criterion": evaluator.name.value, - "observed_pass": observation.observed_pass, - "confirmed_pass": ( - state.confirmed_pass if state is not None else None - ), - "previously_confirmed_pass": ( - previous.confirmed_pass - if previous is not None - else None - ), - "on_probation": ( - state.probation_start is not None - if state is not None - else None - ), - "reason": observation.reason, - } - ) + if _is_notable(previous, state): + anything_moved = True + + criteria_report.append( + { + "criterion": evaluator.name.value, + "observed_pass": observation.observed_pass, + "confirmed_pass": ( + state.confirmed_pass if state is not None else None + ), + "previously_confirmed_pass": ( + previous.confirmed_pass if previous is not None else None + ), + "on_probation": ( + state.probation_start is not None + if state is not None + else None + ), + "reason": observation.reason, + } + ) if partial_run: + # No roll-up on a partial run, so there is no seal state to report. + if is_feed_list_provided or anything_moved: + feed_reports.append( + {"stable_id": ctx.stable_id, "criteria": criteria_report} + ) continue # Merge in any stored criteria this run did not produce a new state for, so the @@ -415,19 +415,36 @@ def update_seals( # grant the seal but can never withdraw one: nothing was held to lose. had_seal = previous_seals.get(feed.id) has_seal = _roll_up_has_seal(merged) - outcomes.append( - { - "feed_id": feed.id, - "stable_id": ctx.stable_id, - "had_seal": bool(had_seal), - "has_seal": has_seal, - # A first evaluation is a grant if it passes, but it is not a loss if - # it fails: nothing was held, so nothing was lost. Only these two - # flags stamp seal_earned_at / seal_lost_at. - "granted": has_seal and not had_seal, - "revoked": bool(had_seal) and not has_seal, - } - ) + outcome = { + "feed_id": feed.id, + "stable_id": ctx.stable_id, + "had_seal": bool(had_seal), + "has_seal": has_seal, + # A first evaluation is a grant if it passes, but it is not a loss if + # it fails: nothing was held, so nothing was lost. Only these two + # flags stamp seal_earned_at / seal_lost_at. + "granted": has_seal and not had_seal, + "revoked": bool(had_seal) and not has_seal, + } + outcomes.append(outcome) + + # A feed is reported when the caller asked for it by name, when one of its + # criteria moved, or when its seal changed. A run over a quiet catalogue with no + # feed list therefore reports nothing, which keeps a nightly response small. + if ( + is_feed_list_provided + or anything_moved + or outcome["granted"] + or outcome["revoked"] + ): + feed_reports.append( + { + "stable_id": ctx.stable_id, + "had_seal": outcome["had_seal"], + "has_seal": has_seal, + "criteria": criteria_report, + } + ) revoked = [outcome for outcome in outcomes if outcome["revoked"]] granted = [outcome for outcome in outcomes if outcome["granted"]] @@ -469,16 +486,16 @@ def update_seals( "Partial criteria run: has_seal was not recalculated because the criteria " "that were not evaluated cannot be judged." ) - # A sample, not the record: sealcriterion holds every verdict. The omitted count is - # always present so a truncated list can never be mistaken for the whole story. - report["evaluations"] = evaluations[:max_reported_evaluations] - report["evaluations_omitted"] = max(0, len(evaluations) - max_reported_evaluations) - - # Log without `evaluations`: Cloud Logging drops a LogEntry over 256 KB, so a run - # naming a few hundred feeds would lose the whole log entry. The counts belong in logs; - # `evaluations` is for the caller reading the response. + # A sample, not the record: the two seal tables hold every verdict. The omitted count + # is always present so a truncated list can never be mistaken for the whole story. + report["feeds"] = feed_reports[:max_reported_feeds] + report["feeds_omitted"] = max(0, len(feed_reports) - max_reported_feeds) + + # Log without `feeds`: Cloud Logging drops a LogEntry over 256 KB, so a run naming a + # few hundred feeds would lose the whole log entry. The counts belong in logs; `feeds` + # is for the caller reading the response. logging.info( "Task completed: %s", - {key: value for key, value in report.items() if key != "evaluations"}, + {key: value for key, value in report.items() if key != "feeds"}, ) return report diff --git a/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py b/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py index f581e5a07..64eeef6c6 100644 --- a/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py +++ b/functions-python/tasks_executor/src/tasks/seal_of_reliability/update_seal_of_reliability.py @@ -19,7 +19,7 @@ from tasks.seal_of_reliability.seal_updater import ( DEFAULT_BATCH_SIZE, - DEFAULT_MAX_REPORTED_EVALUATIONS, + DEFAULT_MAX_REPORTED_FEEDS, update_seals, ) @@ -34,7 +34,7 @@ def get_parameters(payload: dict): payload.get("criteria", None), payload.get("batch_size", DEFAULT_BATCH_SIZE), datetime.fromisoformat(now) if now else None, - payload.get("max_reported_evaluations", DEFAULT_MAX_REPORTED_EVALUATIONS), + payload.get("max_reported_feeds", DEFAULT_MAX_REPORTED_FEEDS), ) @@ -55,8 +55,8 @@ def update_seal_of_reliability_handler(payload: dict) -> dict: evaluated; this only sizes the queries. Default: 200. now (str | None): ISO timestamp to evaluate against, for replays and backfills. Default: current UTC time. - max_reported_evaluations (int): Cap on the `evaluations` list in the response. Everything is - still evaluated and written; `evaluations_omitted` reports how many + max_reported_feeds (int): Cap on the `feeds` list in the response. Everything is + still evaluated and written; `feeds_omitted` reports how many entries were left out. Default: 50. """ ( @@ -66,7 +66,7 @@ def update_seal_of_reliability_handler(payload: dict) -> dict: criteria, batch_size, now, - max_reported_evaluations, + max_reported_feeds, ) = get_parameters(payload) return update_seals( dry_run=dry_run, @@ -75,5 +75,5 @@ def update_seal_of_reliability_handler(payload: dict) -> dict: criteria=criteria, batch_size=batch_size, now=now, - max_reported_evaluations=max_reported_evaluations, + max_reported_feeds=max_reported_feeds, ) diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py index 538c173f8..437ca5d76 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_end_to_end_db.py @@ -123,14 +123,12 @@ def run_task(self, payload: dict) -> dict: @staticmethod def ours(report: dict) -> list: - """The report's evaluations for this module's feeds only. + """The report's feed entries for this module's feeds only. Unnamed runs also cover the conftest fixtures, so filtering keeps these assertions independent of what else lives in the test database. """ - return [ - row for row in report["evaluations"] if row["stable_id"].startswith(PREFIX) - ] + return [row for row in report["feeds"] if row["stable_id"].startswith(PREFIX)] @staticmethod @with_db_session(db_url=default_db_url) @@ -200,8 +198,8 @@ def test_two_runs_with_a_change_in_between(self): self.assertEqual(first["seals_revoked"], 0, "nothing was held beforehand") self.assertEqual( {row["stable_id"] for row in self.ours(first)}, - {NOT_OFFICIAL, UNKNOWN_OFFICIAL}, - "a first evaluation is reported only when it lands on a failure", + {OFFICIAL, NOT_OFFICIAL, UNKNOWN_OFFICIAL}, + "the two failures moved a criterion; the official feed gained the seal", ) # --- inspect the database @@ -243,10 +241,18 @@ def test_two_runs_with_a_change_in_between(self): moved = {row["stable_id"]: row for row in self.ours(second)} self.assertEqual(set(moved), {OFFICIAL, NOT_OFFICIAL}, "only these two moved") - self.assertFalse(moved[OFFICIAL]["confirmed_pass"]) - self.assertTrue(moved[OFFICIAL]["previously_confirmed_pass"]) - self.assertTrue(moved[NOT_OFFICIAL]["confirmed_pass"]) - self.assertFalse(moved[NOT_OFFICIAL]["previously_confirmed_pass"]) + + self.assertTrue(moved[OFFICIAL]["had_seal"]) + self.assertFalse(moved[OFFICIAL]["has_seal"]) + self.assertFalse(moved[OFFICIAL]["criteria"][0]["confirmed_pass"]) + self.assertTrue(moved[OFFICIAL]["criteria"][0]["previously_confirmed_pass"]) + + self.assertFalse(moved[NOT_OFFICIAL]["had_seal"]) + self.assertTrue(moved[NOT_OFFICIAL]["has_seal"]) + self.assertTrue(moved[NOT_OFFICIAL]["criteria"][0]["confirmed_pass"]) + self.assertFalse( + moved[NOT_OFFICIAL]["criteria"][0]["previously_confirmed_pass"] + ) # --- inspect again: both transitions are recorded, history is preserved self.assertEqual( diff --git a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py index 4cf69096d..ca3b7d8a8 100644 --- a/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py +++ b/functions-python/tasks_executor/tests/tasks/seal_of_reliability/test_seal_updater_db.py @@ -276,52 +276,55 @@ def test_seal_counts_balance(self): second["seals_after_run"], ) - def test_dry_run_reports_rows_for_a_named_feed(self): + def test_a_reported_feed_carries_its_seal_state_and_its_criteria(self): report = update_seals(dry_run=True, stable_feed_ids=[NOT_OFFICIAL], now=NOW) + self.assertEqual(len(report["feeds"]), 1) + + feed = report["feeds"][0] + self.assertEqual(feed["stable_id"], NOT_OFFICIAL) + self.assertFalse(feed["had_seal"]) + self.assertFalse(feed["has_seal"]) self.assertEqual( - [row["criterion"] for row in report["evaluations"]], + [row["criterion"] for row in feed["criteria"]], [SealCriterionName.OFFICIAL.value], ) - self.assertFalse(report["evaluations"][0]["observed_pass"]) - self.assertTrue(report["evaluations"][0]["reason"]) - self.assertIsNone(report["evaluations"][0]["previously_confirmed_pass"]) + self.assertFalse(feed["criteria"][0]["observed_pass"]) + self.assertTrue(feed["criteria"][0]["reason"]) + self.assertIsNone(feed["criteria"][0]["previously_confirmed_pass"]) - def test_named_feed_reports_a_row_even_when_nothing_moved(self): - """An explicit feed list is the debugging path: report all of its criteria.""" + def test_named_feed_is_reported_even_when_nothing_moved(self): + """An explicit feed list is the debugging path: report it either way.""" update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) report = update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) - self.assertEqual(len(report["evaluations"]), 1) - self.assertTrue(report["evaluations"][0]["confirmed_pass"]) - self.assertTrue(report["evaluations"][0]["previously_confirmed_pass"]) - def test_unnamed_run_reports_only_criteria_that_moved(self): - """A passing feed evaluated twice contributes no entry the second time.""" + feed = report["feeds"][0] + self.assertTrue(feed["had_seal"]) + self.assertTrue(feed["has_seal"]) + self.assertTrue(feed["criteria"][0]["confirmed_pass"]) + self.assertTrue(feed["criteria"][0]["previously_confirmed_pass"]) + + def test_unnamed_run_reports_only_feeds_that_moved(self): first = update_seals(dry_run=False, now=NOW) - self.assertTrue( - any(row["stable_id"] == NOT_OFFICIAL for row in first["evaluations"]), - "a first evaluation that lands on a failure is reported", - ) - self.assertFalse( - any(row["stable_id"] == OFFICIAL for row in first["evaluations"]), - "a first evaluation that passes is covered by first_evaluations", - ) + reported = {row["stable_id"] for row in first["feeds"]} + self.assertIn(NOT_OFFICIAL, reported, "its criterion landed on a failure") + self.assertIn(OFFICIAL, reported, "it was granted the seal") self.assertGreaterEqual(first["first_evaluations"], 2) steady = update_seals(dry_run=False, now=NOW) - self.assertEqual( - steady["evaluations"], [], "nothing moved, so nothing to report" - ) + self.assertEqual(steady["feeds"], [], "nothing moved, so nothing to report") self.assertEqual(steady["first_evaluations"], 0) - def test_a_criterion_that_flips_is_reported_with_its_previous_value(self): + def test_a_feed_that_flips_is_reported_with_the_previous_verdict(self): update_seals(dry_run=False, now=NOW) self.set_official(OFFICIAL, False) report = update_seals(dry_run=False, now=NOW) - moved = [row for row in report["evaluations"] if row["stable_id"] == OFFICIAL] + moved = [row for row in report["feeds"] if row["stable_id"] == OFFICIAL] self.assertEqual(len(moved), 1) - self.assertFalse(moved[0]["confirmed_pass"]) - self.assertTrue(moved[0]["previously_confirmed_pass"]) + self.assertTrue(moved[0]["had_seal"]) + self.assertFalse(moved[0]["has_seal"]) + self.assertFalse(moved[0]["criteria"][0]["confirmed_pass"]) + self.assertTrue(moved[0]["criteria"][0]["previously_confirmed_pass"]) def test_official_feed_earns_the_seal(self): update_seals(dry_run=False, stable_feed_ids=[OFFICIAL], now=NOW) @@ -456,20 +459,20 @@ def test_limit_caps_the_feeds_evaluated(self): report = update_seals(dry_run=True, limit=2, now=NOW) self.assertLessEqual(report["total_feeds"], 2) - def test_evaluations_are_capped_without_capping_the_run(self): - """The list is a sample; sealcriterion is the record.""" + def test_the_feed_list_is_capped_without_capping_the_run(self): + """The list is a sample; the two seal tables are the record.""" report = update_seals( - dry_run=False, stable_feed_ids=OURS, now=NOW, max_reported_evaluations=2 + dry_run=False, stable_feed_ids=OURS, now=NOW, max_reported_feeds=2 ) - self.assertEqual(len(report["evaluations"]), 2) - self.assertEqual(report["evaluations_omitted"], 2, "4 feeds, 1 criterion each") + self.assertEqual(len(report["feeds"]), 2) + self.assertEqual(report["feeds_omitted"], 2, "4 feeds requested") self.assertEqual(report["total_feeds"], 4, "every feed was still evaluated") self.assertEqual(len(self.criterion_rows(OFFICIAL)), 1, "and still written") - def test_evaluations_omitted_is_zero_when_nothing_was_dropped(self): + def test_feeds_omitted_is_zero_when_nothing_was_dropped(self): report = update_seals(dry_run=True, stable_feed_ids=[OFFICIAL], now=NOW) - self.assertEqual(len(report["evaluations"]), 1) - self.assertEqual(report["evaluations_omitted"], 0) + self.assertEqual(len(report["feeds"]), 1) + self.assertEqual(report["feeds_omitted"], 0) @patch("tasks.seal_of_reliability.seal_updater.EVALUATORS", WITH_PROBATION)