feat: Seal of reliability infrastructure and official criterion - #1795
Open
jcpitre wants to merge 6 commits into
Open
feat: Seal of reliability infrastructure and official criterion#1795jcpitre wants to merge 6 commits into
jcpitre wants to merge 6 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1783
Implements the seal or reliability algorithm as defined in #1761, but uses only
officialas the first criterion implemented.The code is placed in
functions-python/task_executoras theseal_of_reliabilitytask.By default, the task will extract the list of feeds to process. The query to extract the feeds is defined in
functions-python/tasks_executor/src/tasks/seal_of_reliability/context.pyand is currently:The payload also allows work on specific feeds (as listed in the
stable_feed_idsparameter).Note that the filtering (deprecated, development, etc) is also applied to the list of feeds passed in the payload.
From our AI friend:
What this does
Implements the Seal of Reliability evaluation logic and the Official criterion, per #1783. The logic is described in #1761; the
sealcriterionandfeedreliabilitysealtables come from #1760, which is the base branch of this PR.A new
tasks_executortask,update_seal_of_reliability, evaluates every eligible GTFS feed and writes the two seal tables. The source tables (feed,gtfsdataset,validationreport,gtfs_feed_availability_check) are only ever read.The remaining five criteria are #1784 and #1782. The framework here is what they slot into: each is a new evaluator subclass plus whatever data it needs on the context.
How to run it
{ "task": "update_seal_of_reliability", "payload": { "dry_run": true } }dry_rundefaults to true, so an accidental invocation evaluates everything and writes nothing. Other parameters:stable_feed_ids,limit,criteria,batch_size,now. All documented infunctions-python/tasks_executor/README.md, including a local curl walkthrough.No Cloud Scheduler job is defined yet — the task is invoked manually for now. When it is scheduled it needs to run after the daily
check_gtfs_feed_availabilityjob, since the Available criterion (#1784) will read the availability rows recorded for that day.Structure
criteria.pySealCriterionNameenum (all six values, mirroring the DB type) and the 6-month reliability windowcontext.pyFeedSealContext, built in bulk per batchevaluators/base.pyCriterionEvaluatorbase class,RawEvaluationevaluators/official.pystate_machine.pytransition()— steps 2-3 of the logic: failure tracking andgrace_failingseal_updater.pyupdate_seals()— orchestration, thehas_sealroll-up, and persistenceupdate_seal_of_reliability.pyEvaluators are pure functions over a pre-loaded context and never query, which is what keeps the query count proportional to the number of criteria rather than the number of feeds, and what makes the unit tests dataclass-shaped.
Decisions worth a reviewer's attention
A criterion that can't be evaluated is not a failing criterion.
RawEvaluation.failingis tri-state;Nonemeans "no verdict this run" and leaves the stored row untouched. Without this, an outage in an upstream pipeline (no availability check recorded, no validation report yet) would look like every feed failing and would revoke seals across the catalogue once the grace period expired.A grace period protects a seal a feed already holds; it cannot be used to earn one. Granting requires every criterion to pass now; keeping only requires that no failure has been confirmed. Otherwise a feed 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 "14 days to fix it before disqualification".
inactiveandfuturefeeds are deliberately evaluated. Excluding a feed does not make it neutral, it freezes its stored rows — an inactive feed should fail Fresh (future coverage) and lose the seal rather than keep displaying one because we stopped looking at it. Onlydeprecatedanddevelopmentare excluded, alongside anything notpublished.Official has no grace period and no reliability window. It is a point-in-time state check that clears as soon as a feed is flagged official again. The state machine's grace and window logic is generic and ships now, so it is tested against a synthetic criterion that has both — Official alone would leave those branches unexercised for #1784 to discover.
nowis a parameter, not a call to the clock. Runs are replayable and idempotent: re-evaluating for the same instant does not restart a failure streak, and a historical timestamp can be passed for backfills.Both seal tables are written with Core statements against
__table__.feedreliabilityseal.feed_idis simultaneously its primary key and a foreign key tofeed(id), which sqlacodegen maps as joined-table inheritance (class Feedreliabilityseal(Feed), a sibling ofGtfsfeed). Persisting an ORM instance would attempt to insert a new feed. A surrogateidwithUNIQUE (feed_id)in #1760 would remove the quirk if preferred — worth a decision, since it affects the base branch.The report describes changes, not state.
evaluationsholds one entry per criterion whose verdict moved, rather than one per feed per criterion — the latter grows with the catalogue (~424 bytes each, megabytes for a full run) and is whatsealcriterionis for. This followsfailuresincheck_gtfs_feed_availabilityanddispatchedinbackfill_changelog.evaluationsis also excluded from the log line, since Cloud Logging drops aLogEntryover 256 KB.Seal counts are reported as
seals_before_run/seals_after_run/seals_granted/seals_revoked, withbefore + granted - revoked == after. On a dry run,afteris what would be stored.Tests
54 tests, 94% branch coverage of the new package.
test_seal_evaluators.py— the Official criterion (true / false / NULL) and the base class contract. No database.test_seal_state_machine.py— grace-period expiry, streak reset and restart, reliability-window boundaries at day 179 and 180, idempotency. No database.test_seal_updater_db.py— eligibility, context loading, grant / deny / revoke / recover, count balance, against the test DB.test_seal_end_to_end_db.py— seeds feeds, invokestasks_executorwith a hand-builtflask.Request, inspects both tables, modifiesofficial, runs again, and inspects the transitions. Covers dispatch and payload parsing, including thatdry_rundefaults to true through the payload layer.test_seal_updater_db.pyandtest_seal_end_to_end_db.pyneed the local Postgres test database, liketest_check_gtfs_feed_availability_db.py. CI already provides it.Not in this PR
feed_infodate columns they need (Expose feed start and end date from validation report in our DB and/or API #1775)has_seal— worth keeping until Seal of reliability: Implement other criteria evaluation #1784 and Seal of Realiability: Implement the Fresh-continuous-coverage criterion #1782 land, since with only Official implementedhas_sealis effectivelyfeed.officialand does not yet mean "reliable"Summary:
Summarize the changes in the pull request including how it relates to any issues (include the #number, or link them).
Expected behavior:
Explain and/or show screenshots for how you expect the pull request to work in your testing (in case other devices exhibit different behavior).
Testing tips:
Provide tips, procedures and sample files on how to test the feature.
Testers are invited to follow the tips AND to try anything they deem relevant outside the bounds of the testing tips.
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything