fix: using confirmed status to compute probation state - #1843
Merged
Merged
Conversation
cka-y
marked this pull request as ready for review
September 15, 2026 19:32
davidgamez
approved these changes
Sep 15, 2026
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.
Summary:
on_probationandin_grace_periodwere derived fromobserved_status. That column is a property of the last run, not of the criterion: a run that reaches no verdict (unknown) leaves the debounced state untouched by design, so any flag built onobserved_statusmoves on a night nobody could measure.Three concrete bugs came out of that:
unknownon top of a confirmed failure reportedon_probation: true. Probation is the clean run served after recovering, and a criterion still sitting on a confirmed failure never recovered.observed_status != 'fail'was true forunknown, so the criterion read as recovering.not_applicable) criterion rolled up into feed-level probation. The nightly job deliberately freezesprobation_startrather than clearing it, in case the criterion applies again.observed_status <> 'fail'matched it, so a feed whose only probation sat on a criterion that no longer applies reportedon_probation: truewith a bogusprobation_ends_at— on the feed-detail/report endpoints and infeedsearch.in_grace_periodflickered off on a blind night. It requiredobserved_status = 'fail', so anunknownrun mid-grace dropped the at-risk flag and nulledgrace_period_ends_at, even though the failure streak and its countdown were still running.Both flags now read only the debounced columns —
confirmed_status,first_observed_failure_at,probation_start:liquibase/materialized_views/feed_search.sqlcarries the same predicate for search, so it moves with it:observed_status <> 'fail'becomesconfirmed_status = 'pass'.Related: #1842.
Expected behavior:
Response shapes are unchanged.
has_sealandseal_statusare unchanged too —roll_up_seal_statusonly consultson_probationin itsconfirmed is PASS and on_probationterm, and already excludesnot_applicablecriteria, so neither bug could reach the seal decision. What changes is what gets reported:unknownover a confirmed failureon_probationtruefalsenot_applicablewith a frozenprobation_starton_probation/probation_ends_attrue/ setfalse/nullnot_applicablewith a frozenprobation_startreliability_seal.on_probationtruefalseunknownduring a grace periodin_grace_period/grace_period_ends_atfalse/nulltrue/ setThe feed-level
on_probationwas already correct for the first row (roll_up_on_probationcancels on any confirmed failure) — that guard is now redundant rather than load-bearing.Deploy note: the
rebuild-feed-search-mvchangeset isrunOnChange="true"and drops/recreatesfeedsearch, so the view is rebuilt on the next Liquibase run. No new changeset file.Testing tips:
Unit tests, no DB needed:
New cases:
test_an_unevaluable_run_over_a_confirmed_failure_is_not_probationtest_an_unevaluable_run_does_not_clear_the_grace_periodtest_probation_ignored_for_a_withdrawn_criterion(criterion + feed level)test_search_ignores_probation_for_a_withdrawn_criterion— the one that exercises the changed SQL; it needs the test DB and a refreshedfeedsearch, and fails against the old predicate.Two existing tests were rewritten rather than just re-asserted: they built rows the nightly job cannot produce (
observed=fail+confirmed=pass+probation_startset —within_gracerequiresnot was_on_probation, so that combination is unreachable). They now use reachable rows.To check by hand, pick a feed with a criterion on probation, set its latest
seal_criterionrow toobserved_status='unknown'withconfirmed_status='fail', and confirmGET /v1/gtfs_feeds/{id}reports that criterion asfailwithon_probation: false.Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anythingGenerated with Claude Code