johnsturgeon/tgfp-web

Watch

Fork 0

Keep polling a game the live window gave up on #443

Merged

johnsturgeon merged 2 commits from a-game-that-misses-its-live-window-can-never-reach-final-442 into main

2026-09-02 07:54:46 +02:00

johnsturgeon commented

2026-09-01 19:09:36 +02:00

Owner

The bug

_in_live_window measures from the scheduled kickoff, and rule 7 was the only
caller of update_scores_current_week. Once that window shut, a game could
never reach FINAL -- so the week never reached ALL_FINAL, rule 5 re-fetched
its schedule every 45 minutes forever, and rules 8 and 9 never fired for it.

Two ways in:

Bootstrapping into the past. A week whose games were already played gets
rows written with insert_game's SCHEDULED default, and the window is
already behind it. Production hit this on the Aug 2026 rebuild: preseason
week 4, all 16 games SCHEDULED, week stuck at PREGAME, no
update_scores_current_week row in jobstate at all.
A delayed kickoff. Games slip for weather and for the game before them,
and the provider does not move start_time when they do. A 90-minute delay
plus a 3.5-hour game finishes outside a 5-hour tail measured from the
scheduled start.

The fix

LIVE_LEAD / LIVE_TAIL are hoisted out of _in_live_window so
_needs_catch_up is expressed against the same window rather than a second
copy of it. Rule 7b sits below the live poll and fires every 30 minutes while
any game is unsettled and past the tail.

Cancelled games are excluded. A postponed game never settles, and would be the
one case where this rule could not falsify its own condition.

The two clauses share a JobState row, so a live game's 5-minute poll also
satisfies the slow one -- no double dispatch on a day with both a live game and
an abandoned one.

Tests

test_scores_are_not_polled_long_after_kickoff is deleted, not adjusted.
It asserted "a game stuck un-final must not be polled for ever", which is
precisely the behaviour this fixes. Replaced by four tests: the abandoned game
is polled, at the slow cadence, not at the fast one, and a cancelled game does
not hold it open.

test_an_unfinished_game_does_not_move_the_watermark seeded its live game 9
hours past kickoff, which the catch-up now claims. Moved inside the window with
a fresh poll, so the watermark is still what the test turns on.

Mutation-tested, five of five caught: rule deleted (2 tests), is_cancelled
guard dropped (1), catch-up at the live cadence (1), catch-up claiming final
games (6), live window losing its tail bound (2).

Two things that surfaced from doing that, both fixed here: the first
cancelled-game test passed for the wrong reason -- a week holding only a
postponed game reads as NO_GAMES, so rule 1 answered before rule 7b was
reached -- and a docstring claimed the clause order mattered, which it does not,
since the two clauses are an OR over the same job.

It backfills on deploy

No one-off needed. The rule is level-triggered on the current state, so the
first tick after deploy sees 16 unsettled games past the tail and no
update_scores_current_week row, and dispatches. Verified against a seeded
replica of production as it stood on 2026-08-31.

Rule 5 may take the first tick or two while its 45-minute window is open, but
it self-limits and rule 7b follows within a couple of minutes.

resolves #442

## The bug `_in_live_window` measures from the scheduled kickoff, and rule 7 was the only caller of `update_scores_current_week`. Once that window shut, a game could never reach `FINAL` -- so the week never reached `ALL_FINAL`, rule 5 re-fetched its schedule every 45 minutes forever, and rules 8 and 9 never fired for it. Two ways in: - **Bootstrapping into the past.** A week whose games were already played gets rows written with `insert_game`'s `SCHEDULED` default, and the window is already behind it. Production hit this on the Aug 2026 rebuild: preseason week 4, all 16 games `SCHEDULED`, week stuck at `PREGAME`, no `update_scores_current_week` row in `jobstate` at all. - **A delayed kickoff.** Games slip for weather and for the game before them, and the provider does not move `start_time` when they do. A 90-minute delay plus a 3.5-hour game finishes outside a 5-hour tail measured from the scheduled start. ## The fix `LIVE_LEAD` / `LIVE_TAIL` are hoisted out of `_in_live_window` so `_needs_catch_up` is expressed against the same window rather than a second copy of it. Rule 7b sits below the live poll and fires every 30 minutes while any game is unsettled and past the tail. Cancelled games are excluded. A postponed game never settles, and would be the one case where this rule could not falsify its own condition. The two clauses share a `JobState` row, so a live game's 5-minute poll also satisfies the slow one -- no double dispatch on a day with both a live game and an abandoned one. ## Tests `test_scores_are_not_polled_long_after_kickoff` is **deleted**, not adjusted. It asserted "a game stuck un-final must not be polled for ever", which is precisely the behaviour this fixes. Replaced by four tests: the abandoned game is polled, at the slow cadence, not at the fast one, and a cancelled game does not hold it open. `test_an_unfinished_game_does_not_move_the_watermark` seeded its live game 9 hours past kickoff, which the catch-up now claims. Moved inside the window with a fresh poll, so the watermark is still what the test turns on. Mutation-tested, five of five caught: rule deleted (2 tests), `is_cancelled` guard dropped (1), catch-up at the live cadence (1), catch-up claiming final games (6), live window losing its tail bound (2). Two things that surfaced from doing that, both fixed here: the first cancelled-game test passed for the wrong reason -- a week holding only a postponed game reads as `NO_GAMES`, so rule 1 answered before rule 7b was reached -- and a docstring claimed the clause order mattered, which it does not, since the two clauses are an OR over the same job. ## It backfills on deploy No one-off needed. The rule is level-triggered on the current state, so the first tick after deploy sees 16 unsettled games past the tail and no `update_scores_current_week` row, and dispatches. Verified against a seeded replica of production as it stood on 2026-08-31. Rule 5 may take the first tick or two while its 45-minute window is open, but it self-limits and rule 7b follows within a couple of minutes. resolves #442

johnsturgeon added 1 commit

2026-09-01 19:09:37 +02:00

Keep polling a game the live window gave up on

Claude Code Review / claude-review (pull_request) Successful in 2m2s

Details

Python FastAPI Jinja Linting / build (pull_request) Successful in 25s

Details

Tests / pytest (pull_request) Successful in 12s

Details

80ec7a9bf3

`_in_live_window` measures from the scheduled kickoff, and rule 7 was the only
caller of the scores job -- so once that window shut, a game could never reach
FINAL. The week then never reached ALL_FINAL, rule 5 re-fetched its schedule
every 45 minutes indefinitely, and rules 8 and 9 never fired for it.

Two ways in. A week whose games were played before the rows existed is inserted
with the SCHEDULED default and the window already behind it -- production hit
this on the Aug 2026 rebuild, with all 16 preseason week 4 games stuck and no
`update_scores_current_week` row in `jobstate` at all. The other is an ordinary
delayed kickoff: games slip for weather and for the game before them, and the
provider does not move `start_time` when they do.

Rule 7b polls every 30 minutes while any game is unsettled and past the tail.
Cancelled games are excluded -- a postponed game never settles, and would be
the one case where the rule could not falsify its own condition.

`test_scores_are_not_polled_long_after_kickoff` is deleted rather than
adjusted: it asserted the behaviour this fixes.

Code by John. Tests by Claude Opus 5, directed and reviewed by John.

resolves #442

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

forgejo-actions commented

2026-09-01 19:11:41 +02:00

This is a tight, well-scoped change. I've read app/jobs/dispatcher.py, the Game/GameStatus models, and the full test file to check the surrounding rules and helpers.

The change correctly closes a real gap: a game whose scheduled kickoff has slipped out of the 5-hour live tail (delayed game, or a week bootstrapped after its games were already played) could never be re-polled, so it never reached FINAL and every rule below rule 7 stalled for that week. Hoisting LIVE_LEAD/LIVE_TAIL so rule 7b shares the exact window boundary is the right way to keep the two clauses from drifting, and the tests are unusually thorough. No blocking issues.

Worth fixing

Nothing blocking — the items below are judgment calls.

Nits

Unbounded poll/log for a game the provider never finalizes (app/jobs/dispatcher.py:145). Rule 7b's condition is only falsified when a game reaches FINAL or becomes cancelled (POSTPONED/SUSPENDED). If the provider stops returning a settling status for some other reason — a dropped/renamed external_game_id, a game left IN_PROGRESS in the feed — 7b dispatches update_scores and emits a Sentry info every 30 minutes indefinitely, with no escalation. This is the deliberate trade you made against the old "give up" behaviour, and cancelled games are the escape hatch, so it's defensible; just flagging that the failure mode is silent-but-forever rather than eventually-alerting.
SUSPENDED is bundled into the catch-up exclusion (app/jobs/dispatcher.py:199, via Game.is_cancelled at app/models/game.py:89). The docstring justifies the exclusion with "a postponed game never settles," which is true for POSTPONED but not for SUSPENDED — a suspended game can resume the next day and go FINAL, and because it resumes well past the 5-hour tail, neither rule 7 nor 7b will ever poll it to pick up that final. This matches the codebase's existing treatment of SUSPENDED as terminal (_get_current_game_state, player_game_pick), so it's pre-existing and out of scope for this PR — but the rule's stated rationale doesn't actually cover the SUSPENDED half of is_cancelled. If suspended-then-resumed is a real scenario you care about, it'd need separate handling; if not, the docstring slightly overstates the guarantee.
Boundary between rule 7 and 7b isn't directly tested. The tests exercise 1-minute-before, 9-hours-after, etc., but not now == utc_start_time + LIVE_TAIL (7 owns it via <=) vs. just past it (7b owns it via >). The description says mutation testing caught the tail-bound loss, so this is covered indirectly — an explicit boundary case would just make the handoff self-documenting.

Test coverage, docstrings, and the version bump (2.20.1 → 2.20.2 in both pyproject.toml and uv.lock) all look correct.

This is a tight, well-scoped change. I've read `app/jobs/dispatcher.py`, the `Game`/`GameStatus` models, and the full test file to check the surrounding rules and helpers. --- The change correctly closes a real gap: a game whose scheduled kickoff has slipped out of the 5-hour live tail (delayed game, or a week bootstrapped after its games were already played) could never be re-polled, so it never reached `FINAL` and every rule below rule 7 stalled for that week. Hoisting `LIVE_LEAD`/`LIVE_TAIL` so rule 7b shares the exact window boundary is the right way to keep the two clauses from drifting, and the tests are unusually thorough. No blocking issues. ## Worth fixing Nothing blocking — the items below are judgment calls. ## Nits - **Unbounded poll/log for a game the provider never finalizes** (`app/jobs/dispatcher.py:145`). Rule 7b's condition is only falsified when a game reaches `FINAL` or becomes cancelled (`POSTPONED`/`SUSPENDED`). If the provider stops returning a settling status for some other reason — a dropped/renamed `external_game_id`, a game left `IN_PROGRESS` in the feed — 7b dispatches `update_scores` and emits a Sentry `info` every 30 minutes indefinitely, with no escalation. This is the deliberate trade you made against the old "give up" behaviour, and cancelled games are the escape hatch, so it's defensible; just flagging that the failure mode is silent-but-forever rather than eventually-alerting. - **`SUSPENDED` is bundled into the catch-up exclusion** (`app/jobs/dispatcher.py:199`, via `Game.is_cancelled` at `app/models/game.py:89`). The docstring justifies the exclusion with "a postponed game never settles," which is true for `POSTPONED` but not for `SUSPENDED` — a suspended game can resume the next day and go `FINAL`, and because it resumes well past the 5-hour tail, neither rule 7 nor 7b will ever poll it to pick up that final. This matches the codebase's existing treatment of `SUSPENDED` as terminal (`_get_current_game_state`, `player_game_pick`), so it's pre-existing and out of scope for this PR — but the rule's stated rationale doesn't actually cover the `SUSPENDED` half of `is_cancelled`. If suspended-then-resumed is a real scenario you care about, it'd need separate handling; if not, the docstring slightly overstates the guarantee. - **Boundary between rule 7 and 7b isn't directly tested.** The tests exercise 1-minute-before, 9-hours-after, etc., but not `now == utc_start_time + LIVE_TAIL` (7 owns it via `<=`) vs. just past it (7b owns it via `>`). The description says mutation testing caught the tail-bound loss, so this is covered indirectly — an explicit boundary case would just make the handoff self-documenting. Test coverage, docstrings, and the version bump (`2.20.1` → `2.20.2` in both `pyproject.toml` and `uv.lock`) all look correct.

johnsturgeon added 1 commit

2026-09-01 19:16:21 +02:00

Pin the production scenario the catch-up rule was written for

Claude Code Review / claude-review (pull_request) Successful in 1m49s

Details

Python FastAPI Jinja Linting / build (pull_request) Successful in 24s

Details

Tests / pytest (pull_request) Successful in 12s

Details

662f081746

The existing tests each isolate one condition -- past the tail, the slow
cadence, a cancelled game. None of them asserts that the situation which
produced #442 actually resolves.

This one is the 2026-08-31 rebuild as it stood: preseason week 4 created days
after its games were played, sixteen rows on `insert_game`'s SCHEDULED default,
and `jobstate` holding only the five jobs that had managed to run. One tick.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

johnsturgeon merged commit a13a113dc4 into main

2026-09-02 07:54:46 +02:00

johnsturgeon referenced this pull request from a commit

2026-09-02 07:54:47 +02:00

Merge pull request 'Keep polling a game the live window gave up on' (#443) from a-game-that-misses-its-live-window-can-never-reach-final-442 into main