Phase 4 — Rewire the scheduler onto the week table #410

New issue

Closed

opened 2026-08-21 17:40:27 +02:00 by johnsturgeon · 0 comments

johnsturgeon commented

2026-08-21 17:40:27 +02:00

Owner

Parent: #406 · Tech debt: create_the_picks runs three times for postseason week 4

Revised 2026-08-22. This is the phase that owns the odds_state
transition, the odds write barrier, and the create_the_picks upsert. The
one-way data rule below is amended — read that first, it now permits
something the original wording forbade.

Why

Jobs currently take a WeekInfo handed to them at schedule time and have no
notion of what state the week is in. This is the phase where the week table
starts driving the schedule instead of sitting beside it:

API (source of truth) -> week table (cached truth) -> drives jobs -> updates other tables

The division of labour, stated once so the tasks below read consistently:

Sync jobs pull from the provider and write observations. Idempotent.
The dispatcher reads state and writes decisions. odds_state is the
first column it owns, and the template for every one after it.
Worker jobs read the DB and act on the outside world (mail, Discord).
Their exactly-once guarantee comes from an effect ledger, not from
APScheduler holding a DateTrigger.

Needs Phases 1–3.

Tasks

Stop pickling WeekInfo into APScheduler args.
schedule_create_picks passes args=[week_info], and APScheduler
serializes job args into the jobstore — so the value is frozen at
schedule time. Pass week_id and load inside the job.

Already observed: moving `WeekInfo` out of `app.models.model_helpers` in
Phase 1 made the stored `create_picks` job unrestorable —
`ModuleNotFoundError: No module named 'app.models.model_helpers'` — and
APScheduler dropped it on startup. Any refactor that moves or renames a
class in a job's args silently deletes that job.

schedule_create_picks looks up the week at run time instead of
binding one into a weekly cron. Today the 6am Wednesday run receives a
week captured at 7am the previous Wednesday. Worth confirming
empirically before and after — the current behavior may be masked by
container restarts re-running schedule_jobs in lifespan.
create_the_picks idempotency guard — skip unless the current week
~~is PENDING. Closes the postseason-week-4 triple-fire, and also covers~~
~~reruns, restarts, and the admin button, which is_skip_week never did.~~
Make create_the_picks an upsert — this is the actual fix, and the
struck guard above is the wrong shape. create_the_picks blind-adds
every game with no existence check, so a second run hits
uq_game_data_source_external_game_id and raises IntegrityError. It is
not idempotent, despite scheduler.py:119 asserting that it is. A guard
makes the second run not happen; an upsert makes it harmless, which
is what the dispatcher needs since it will call sync repeatedly by
design. It also fixes a bug the guard cannot: a kickoff time flexed
mid-week never reaches the DB today, because there is no update path at
all.
Rename it. With the upsert in place it is sync_games_for_week, a
sync job. "Creating the picks page" is not a thing it does — that is the
odds_state transition below. CreatePicksException and
SENTRY_CRON_MONITOR_CREATE_PICKS go with it.
Implement the PRELIMINARY → LOCKED transition. This is a
dispatcher decision, not a job:
* Trigger: policy. Wednesday 6am PT to start, unchanged behaviour.
* Guard: every game in the week has a line (spread IS NOT NULL,
which #408 makes expressible), and now < first_kickoff.
* Guard fails at trigger time: retry each tick. Close to kickoff,
release anyway with the degraded default written explicitly, and
alert. Blocking the entire pool because one game is missing a line is
the wrong failure mode.
The recorded fact is the same shape regardless of which policy produced
it, so replacing Wednesday with an odds-coverage heuristic later is one
function body and no migration.
Add the odds write barrier. Once odds_state == LOCKED, the game
sync must not touch spread or favorite_team_id. Field-level, not
row-level — scores and start_time must keep flowing, because flex
scheduling moves kickoffs after picks open and the pick stays valid when
the clock changes. Shape is upsert_game(session, api_game, locked),
not an early return.
```
This protects the invariant the whole lock exists for: *every pick in a
week was made against the same number.* Without it the upsert above is
actively harmful — someone picking Tuesday and someone picking Saturday
would be scored against different spreads, silently.
```
Skip the odds fetch entirely when LOCKED. tank01_api.py:210,
games unconditionally calls self.odds_data, which loops
/getNFLBettingOdds once per distinct game date — 3–4 calls per
week fetched, for numbers the barrier is about to discard. Needs a split
or a flag on the wrapper; see #411.
Alert on the illegal state combination.
game_state != 'pending' AND odds_state = 'preliminary' means kickoff
happened and the picks page was never released. That is the
season-ending failure for a pick'em pool, and it is undetectable while
"released" and "started" share one axis. With two orthogonal machines it
is one query. Wire it to Sentry or Kuma.
Admin unlock as a guarded setter. Flip odds_state back to
PRELIMINARY, refuse if any PlayerGamePick exists for the week, log
it. No column, no ledger row, no migration — see the note in #408 on why
unlock needs no tracking.
Replace per-game polling with one live-games job. See below — this
is a redesign, not a port.
Switch nag_players to read week state for its trigger. Smallest
job, do it first.
~~Switch create_picks to read week state.~~ Subsumed by the
transition and barrier tasks above.
Reinstate per-week job scheduling at startup. lifespan no longer
calls schedule_jobs (see schedule_jobs no longer runs at startup
in TECH_DEBT), so between Wednesdays a restart comes up with nothing
scheduled. Once jobs read week state this stops needing a WeekInfo
argument and can come back.

The polling redesign

schedule_update_games currently creates one APScheduler job per game,
each polling every 5 minutes from kickoff to kickoff+8h. Replace it with a
single job that fetches all live games for the week whenever the week is in
progress.

This is not a refactor of the existing job — it deletes the per-game model
entirely, and with it three separate problems:

API cost. _update_one_game builds a fresh NflApi per game, and each
one pulls games_data + scores_data + odds_data. A 16-game week costs
roughly 16× the calls one week-scoped fetch needs. On a metered plan that is
the difference between ~5 calls per poll and ~80.
Orphaned jobs. update_a_game removes its own job when the game is
final, but on a missing game it returns early and removes nothing — so a
deleted or re-created game leaves a job misfiring until its end_date
passes. Observed in dev: 14 game_id:* jobs against truncated games,
producing hundreds of "was missed by 6:50:40" warnings per startup and
saturating the executor pool.
Jobstore churn. Nothing per-game accumulates, so there is nothing to
purge between weeks or seasons.

The trigger and the exit condition both come from the week state machine, which
is exactly what it is for: poll while game_state == IN_PROGRESS, stop on the
transition to ALL_FINAL. No per-game bookkeeping either way.

Note that the stored column is what makes the exit an event: the writing
helper compares the freshly computed state against the stored one, and the
branch where they differ is the transition. That is the edge trigger for
the rollups, and it is the strongest argument for storing game_state rather
than computing it on read — a derived value has no previous to compare
against.

Flex scheduling

Kickoff times move, sometimes with under two weeks' notice, and a flexed game
silently invalidates a DateTrigger already sitting in the jobstore. Anything
derived from first_kickoff — nag times, the release deadline, the polling
window — has to be recomputed rather than scheduled once. That is an argument
for the tick-and-reconcile shape over pre-scheduling dated jobs, and it is why
the upsert above matters beyond idempotency.

Watch out

Do the per-job switches as separate PRs. This is the point where a rewrite
quietly becomes a weekend. One job per PR keeps each one revertable.

Adding a job first does not sequence it. Every run_date=now() job in
lifespan lands in the same executor pool and runs concurrently. Phase 1 hit
this: update_all_awards_startup and sync_current_week_startup raced, awards
won, and it blew up on an empty week table. Anything that reads the week table
has to tolerate not finding a row yet, rather than relying on ordering.

Jobs may write to the week table; they may not derive its identity.
~~A column's value must come from the provider, or from the writing job's own~~
~~action. It must never come from a query over the tables that week~~
~~schedules — that is the circularity this whole design exists to avoid.~~

Amended, because the original wording forbids the game_state column in #408.
The circularity worth preventing is Week.current() needing game rows to
answer — that genuinely deadlocks, since games are created for a week. State
is not identity. The rule is:

Week identity — season, season_type, week_no — comes from the
provider. Nothing derives it, ever.
Week state may be projected from tables that week drives, provided it
is recomputed from scratch and never read back into identity.
A decision column comes from the writing job's own action and is never
recomputed at all.

Odds coverage is a guard, not a trigger. Lines move until kickoff; they
never "settle" in a way you could detect and fire on. The trigger is policy,
the guard is completeness. Conflating them produces a release that never fires.

Done when

No APScheduler job carries a serialized WeekInfo in its args.
No APScheduler job is scoped to a single game.
~~create_the_picks run three times in a row produces one week's games.~~
sync_games_for_week run three times in a row produces one week's games and
raises nothing.
With odds_state == LOCKED, a sync run leaves every spread untouched and
still updates scores and kickoff times.
Every job decides whether to run by reading week, not by what it was handed.
A restart on any day of the week comes up fully scheduled.
The illegal-combination query is wired to an alert.

Parent: #406 · Tech debt: *`create_the_picks` runs three times for postseason week 4* > **Revised 2026-08-22.** This is the phase that owns the `odds_state` > transition, the odds write barrier, and the `create_the_picks` upsert. The > one-way data rule below is amended — read that first, it now permits > something the original wording forbade. ## Why Jobs currently take a `WeekInfo` handed to them at schedule time and have no notion of what state the week is in. This is the phase where the week table starts driving the schedule instead of sitting beside it: API (source of truth) -> week table (cached truth) -> drives jobs -> updates other tables The division of labour, stated once so the tasks below read consistently: * **Sync jobs** pull from the provider and write observations. Idempotent. * **The dispatcher** reads state and writes *decisions*. `odds_state` is the first column it owns, and the template for every one after it. * **Worker jobs** read the DB and act on the outside world (mail, Discord). Their exactly-once guarantee comes from an effect ledger, not from APScheduler holding a `DateTrigger`. Needs Phases 1–3. ## Tasks - [x] **Stop pickling `WeekInfo` into APScheduler args.** `schedule_create_picks` passes `args=[week_info]`, and APScheduler serializes job args into the jobstore — so the value is frozen at schedule time. Pass `week_id` and load inside the job. Already observed: moving `WeekInfo` out of `app.models.model_helpers` in Phase 1 made the stored `create_picks` job unrestorable — `ModuleNotFoundError: No module named 'app.models.model_helpers'` — and APScheduler dropped it on startup. Any refactor that moves or renames a class in a job's args silently deletes that job. - [x] **`schedule_create_picks` looks up the week at run time** instead of binding one into a weekly cron. Today the 6am Wednesday run receives a week captured at 7am the *previous* Wednesday. Worth confirming empirically before and after — the current behavior may be masked by container restarts re-running `schedule_jobs` in `lifespan`. - [x] ~~**`create_the_picks` idempotency guard** — skip unless the current week~~ ~~is `PENDING`. Closes the postseason-week-4 triple-fire, and also covers~~ ~~reruns, restarts, and the admin button, which `is_skip_week` never did.~~ - [x] **Make `create_the_picks` an upsert** — this is the actual fix, and the struck guard above is the wrong shape. `create_the_picks` blind-`add`s every game with no existence check, so a second run hits `uq_game_data_source_external_game_id` and raises `IntegrityError`. It is not idempotent, despite `scheduler.py:119` asserting that it is. A guard makes the second run *not happen*; an upsert makes it *harmless*, which is what the dispatcher needs since it will call sync repeatedly by design. It also fixes a bug the guard cannot: a kickoff time flexed mid-week never reaches the DB today, because there is no update path at all. - [ ] **Rename it.** With the upsert in place it is `sync_games_for_week`, a sync job. "Creating the picks page" is not a thing it does — that is the `odds_state` transition below. `CreatePicksException` and `SENTRY_CRON_MONITOR_CREATE_PICKS` go with it. - [ ] **Implement the `PRELIMINARY` → `LOCKED` transition.** This is a dispatcher decision, not a job: * **Trigger:** policy. Wednesday 6am PT to start, unchanged behaviour. * **Guard:** every game in the week has a line (`spread IS NOT NULL`, which #408 makes expressible), and `now < first_kickoff`. * **Guard fails at trigger time:** retry each tick. Close to kickoff, release anyway with the degraded default written explicitly, and alert. Blocking the entire pool because one game is missing a line is the wrong failure mode. The recorded fact is the same shape regardless of which policy produced it, so replacing Wednesday with an odds-coverage heuristic later is one function body and no migration. - [ ] **Add the odds write barrier.** Once `odds_state == LOCKED`, the game sync must not touch `spread` or `favorite_team_id`. **Field-level, not row-level** — scores and `start_time` must keep flowing, because flex scheduling moves kickoffs after picks open and the pick stays valid when the clock changes. Shape is `upsert_game(session, api_game, locked)`, not an early return. This protects the invariant the whole lock exists for: *every pick in a week was made against the same number.* Without it the upsert above is actively harmful — someone picking Tuesday and someone picking Saturday would be scored against different spreads, silently. - [ ] **Skip the odds fetch entirely when `LOCKED`.** `tank01_api.py:210`, `games` unconditionally calls `self.odds_data`, which loops `/getNFLBettingOdds` **once per distinct game date** — 3–4 calls per week fetched, for numbers the barrier is about to discard. Needs a split or a flag on the wrapper; see #411. - [ ] **Alert on the illegal state combination.** `game_state != 'pending' AND odds_state = 'preliminary'` means kickoff happened and the picks page was never released. That is the season-ending failure for a pick'em pool, and it is undetectable while "released" and "started" share one axis. With two orthogonal machines it is one query. Wire it to Sentry or Kuma. - [ ] **Admin unlock as a guarded setter.** Flip `odds_state` back to `PRELIMINARY`, refuse if any `PlayerGamePick` exists for the week, log it. No column, no ledger row, no migration — see the note in #408 on why unlock needs no tracking. - [ ] **Replace per-game polling with one live-games job.** See below — this is a redesign, not a port. - [ ] **Switch `nag_players` to read week state** for its trigger. Smallest job, do it first. - [ ] ~~**Switch `create_picks` to read week state.**~~ Subsumed by the transition and barrier tasks above. - [ ] **Reinstate per-week job scheduling at startup.** `lifespan` no longer calls `schedule_jobs` (see *`schedule_jobs` no longer runs at startup* in TECH_DEBT), so between Wednesdays a restart comes up with nothing scheduled. Once jobs read week state this stops needing a `WeekInfo` argument and can come back. ## The polling redesign `schedule_update_games` currently creates **one APScheduler job per game**, each polling every 5 minutes from kickoff to kickoff+8h. Replace it with a single job that fetches all live games for the week whenever the week is in progress. This is not a refactor of the existing job — it deletes the per-game model entirely, and with it three separate problems: * **API cost.** `_update_one_game` builds a fresh `NflApi` per game, and each one pulls `games_data` + `scores_data` + `odds_data`. A 16-game week costs roughly 16× the calls one week-scoped fetch needs. On a metered plan that is the difference between ~5 calls per poll and ~80. * **Orphaned jobs.** `update_a_game` removes its own job when the game is final, but on a *missing* game it returns early and removes nothing — so a deleted or re-created game leaves a job misfiring until its `end_date` passes. Observed in dev: 14 `game_id:*` jobs against truncated games, producing hundreds of "was missed by 6:50:40" warnings per startup and saturating the executor pool. * **Jobstore churn.** Nothing per-game accumulates, so there is nothing to purge between weeks or seasons. The trigger and the exit condition both come from the week state machine, which is exactly what it is for: poll while `game_state == IN_PROGRESS`, stop on the transition to `ALL_FINAL`. No per-game bookkeeping either way. Note that the stored column is what makes the exit an *event*: the writing helper compares the freshly computed state against the stored one, and the branch where they differ **is** the transition. That is the edge trigger for the rollups, and it is the strongest argument for storing `game_state` rather than computing it on read — a derived value has no previous to compare against. ## Flex scheduling Kickoff times move, sometimes with under two weeks' notice, and a flexed game silently invalidates a `DateTrigger` already sitting in the jobstore. Anything derived from `first_kickoff` — nag times, the release deadline, the polling window — has to be recomputed rather than scheduled once. That is an argument for the tick-and-reconcile shape over pre-scheduling dated jobs, and it is why the upsert above matters beyond idempotency. ## Watch out **Do the per-job switches as separate PRs.** This is the point where a rewrite quietly becomes a weekend. One job per PR keeps each one revertable. **Adding a job first does not sequence it.** Every `run_date=now()` job in `lifespan` lands in the same executor pool and runs concurrently. Phase 1 hit this: `update_all_awards_startup` and `sync_current_week_startup` raced, awards won, and it blew up on an empty week table. Anything that reads the week table has to tolerate not finding a row yet, rather than relying on ordering. **Jobs may write to the `week` table; they may not derive its identity.** ~~A column's value must come from the provider, or from the writing job's own~~ ~~action. It must never come from a query over the tables that `week`~~ ~~schedules — that is the circularity this whole design exists to avoid.~~ Amended, because the original wording forbids the `game_state` column in #408. The circularity worth preventing is `Week.current()` needing game rows to answer — that genuinely deadlocks, since games are created *for* a week. State is not identity. The rule is: * Week **identity** — `season`, `season_type`, `week_no` — comes from the provider. Nothing derives it, ever. * Week **state** may be projected from tables that `week` drives, provided it is recomputed from scratch and never read back into identity. * A **decision** column comes from the writing job's own action and is never recomputed at all. **Odds coverage is a guard, not a trigger.** Lines move until kickoff; they never "settle" in a way you could detect and fire on. The trigger is policy, the guard is completeness. Conflating them produces a release that never fires. ## Done when - No APScheduler job carries a serialized `WeekInfo` in its args. - No APScheduler job is scoped to a single game. - ~~`create_the_picks` run three times in a row produces one week's games.~~ `sync_games_for_week` run three times in a row produces one week's games and raises nothing. - With `odds_state == LOCKED`, a sync run leaves every `spread` untouched and still updates scores and kickoff times. - Every job decides whether to run by reading `week`, not by what it was handed. - A restart on any day of the week comes up fully scheduled. - The illegal-combination query is wired to an alert.

johnsturgeon added a new dependency

2026-08-21 17:41:45 +02:00

#406 Resolve the technical debt issues that remain for the week table issue

johnsturgeon referenced this issue

2026-08-21 17:42:36 +02:00

Resolve the technical debt issues that remain for the week table issue #406

johnsturgeon referenced this issue

2026-08-22 08:02:26 +02:00

Phase 5 — Independent job fixes #411

johnsturgeon referenced this issue

2026-08-22 11:24:24 +02:00

Phase 2 — Week state machine #408