Phase 2 — Week state machine #408

Closed
opened 2026-08-21 17:39:26 +02:00 by johnsturgeon · 0 comments
Owner

Parent: #406 · Tech debt: Week has no state machine

Revised 2026-08-22 after a design pass. The week carries two state
machines, not one. game_state is an observation, projected from game
rows. odds_state is a decision, made and recorded by the dispatcher.
Strikethrough below is superseded text, kept so the reasoning is traceable.

Why

app/models/week_state.py defines WeekState (pending / pregame /
in_progress / all_final) and nothing imports it. The game_state column
was pulled back out of Week before shipping, because a column no job writes
would have sat at pending on every row and lied.

This phase is independent of Phase 3 and 4. It only needs Phase 1 far enough
along that week rows exist.

Tasks

  • Add an in-memory SQLite session fixture to tests/conftest.py.
    Week has no foreign keys, so it needs no seed data — this is the
    cheapest possible entry point into the "no test database" problem, and
    everything below leans on it.

  • Add GameStatus as a StrEnum in app/models/game_status.py,
    alongside season.py and week_state.py. Members SCHEDULED,
    IN_PROGRESS, FINAL, POSTPONED, SUSPENDED; lowercase values, to
    match WeekState. Game.game_status stays annotated str -- a
    StrEnum member is a str, so it stores with no conversion and
    compares equal to what comes back off a loaded row.

    What this replaces is ESPN's vocabulary. `tank01_api.py:29` maps
    Tank01's `"0"`-`"4"` onto `STATUS_SCHEDULED` / `STATUS_FINAL` strings,
    and `tests/test_tank01_api.py:13` says why out loud -- "ESPN-style
    strings ... because models/game.py compares against" them. ESPN is
    gone; the strings outlived it.
    
    **One map, not two.** `_GAME_STATUS_TYPES` stays in `tank01_api.py` and
    only changes what it maps *to* (`GameStatus.FINAL`, not
    `"STATUS_FINAL"`), so `NflApiGame.game_status` is already a
    `GameStatus` and the sync job assigns it with no translation of its
    own. The dict's keys are Tank01's and stay provider-local; its values
    are ours and are shared. A second map in the sync job would be an
    identity map.
    
    This is the same split `season.py` already documents -- *"these ints
    are what the DB stores and what every API wrapper normalizes to, so the
    mapping belongs with the models rather than on whichever provider is
    current."* `game_status` never got the same treatment.
    
    The wrapper will import from `app.models` for the first time.
    `test_integrity` only forbids the reverse direction, so this is
    allowed -- but import the submodule
    (`from app.models.game_status import GameStatus`), not the package: a
    bare `app.models` registers every SQLModel table to get one enum.
    
    Do it in this phase and it is a rename, not a data migration: `game` is
    still empty in production. `SeasonType` stays an `IntEnum` because
    season type is genuinely *ordered* -- `Week.current()` sorts on it.
    Game status has no order, so it takes the `WeekState` treatment
    instead.
    
  • Write week_state_from_statuses() as a pure function, with tests.
    No DB, no wiring, nothing imports it yet.

  • Add game_state back to Week plus a migration, defaulting to
    pending. Annotate the column str, not WeekState — a native
    Postgres enum makes every future state an ALTER TYPE on a live table.

  • Add odds_state to Week, same migration, defaulting to
    preliminary. New OddsState StrEnum in app/models/week_state.py:
    PRELIMINARYLOCKED, one direction. This single fact is both "odds
    are frozen" and "the picks page is released" — they are one moment with
    two names, see the design notes.

  • Make Game.spread and Game.favorite_team_id nullable, same
    migration. Today a game with no odds is written as home −0.5, byte for
    byte identical to a genuine pick'em, so "does every game have a line" is
    not an answerable question — and that is precisely the guard on the
    LOCKED transition. Null means "no odds yet"; the transition is what
    guarantees non-null afterwards. game is still empty in production, so
    this is one revision with no backfill. Blast radius is
    app/templates/picks.j2:48, app/jobs/create_picks.py, and one
    comparison at app/models/game.py:83.

  • Give it a writer. sync_current_week (or update_a_game) computes
    the state from the provider payload and writes it.

  • Give game_state a writer. One shared helper, called by every job
    that writes game rows, inside the same transaction as the write.
    Not sync_current_week — that job only ever sees what the provider says
    the current week is, and never touches a game row. Not a standalone
    projection job either: a separate job is exactly where drift comes from.
    Committing the projection alongside the rows it summarises takes the
    drift window to zero rather than to one tick.

  • Delete _games_exist_and_all_games_are_final() in
    app/jobs/award_update_all.py — defined, called from nowhere, and very
    nearly the pending / all_final half of this logic. Do not leave a
    third copy.

Design notes

Two kinds of column, opposite failure modes. Both live on week, and the
obvious tidy-up on either one breaks it. Comment them individually.

game_state odds_state
kind materialised projection latched decision
source fold over game rows the dispatcher's own action
recomputed every write, from scratch never
written by sync dispatcher
the bug forgetting to recompute (silent staleness) recomputing it (destroys information)

game_state drifting is self-healing and benign — the next write corrects it,
and the worst case is one extra poll or a picker link appearing a beat late.
odds_state is not derivable from anything: no query over any table can tell
you whether you released the picks page.

Storing game_state does not violate the one-way rule — but #410 states
that rule in a form that forbids it outright. That paragraph is being amended
there: week identity comes from the provider and nothing derives it; week
state may be projected from tables that week drives, provided it is
recomputed from scratch and never read back into identity. The circularity the
rule exists to prevent is Week.current() needing games to answer, which would
deadlock, because games are created for a week. State is not identity.

Why odds_state is one fact and not two. An earlier draft of this had
odds_locked and picks_released as separate facts on the theory that odds
"settle" at some observable moment you might want to freeze before releasing.
They do not — lines move on injury news right up to kickoff. The freeze is an
act of the pool, not an observation of the market, and there is no reason to
perform it earlier than the moment you show people the numbers they are
picking against. One column.

That also means odds coverage is a guard, not a trigger. The trigger is
policy (today: Wednesday 6am PT). The guard is "every game has a line, and
now < first_kickoff". See #410.

odds_state needs no unlock tracking. Going back to PRELIMINARY is only
legal before any PlayerGamePick exists for the week, and in that region it
has no consequences to record: nothing downstream depends on the old spreads,
so unlock-then-relock is indistinguishable from having locked correctly the
first time. The end state is the whole truth. The one thing that looks like it
needs history — not re-announcing "picks are open" on the second lock — is
covered by the effect ledger, not by this column.

Pure function, not a query method.
Two reasons: there is no seedable test
database, so a pure function is the only version testable today; and it has two
callers on opposite sides of the one-way data rule — the live updater feeds it
statuses from the API payload, a backfill would feed it statuses from game
rows. The Week table still never derives itself from tables it drives.

Both of those reasons are gone. The first task in this issue builds the test
database. And under the sync/dispatch split there is only ever one caller: the
live updater writes its game row and stops, and the projection reads rows.
Nothing feeds statuses in from an API payload any more.

The pure function survives regardless, as the inner layer — it is the only
part with branching worth testing exhaustively, and the test table below is
still the specification. What changes is that it is no longer the public API.
The wrapper that loads rows and folds them is three lines, and that is what
callers use.

POSTPONED and SUSPENDED are skipped, not folded. Tank01 has five
status codes, not three (0 scheduled, 1 in progress, 2 final,
3 postponed, 4 suspended). The old string comparison quietly ignored the
last two: is_final, is_pregame and is_in_progress in
app/models/game.py are three independent equality checks, so a postponed
game answered False to all three and belonged to no bucket. A week
containing one would never reach ALL_FINAL -- polling would never stop and
the rollups would never fire.

Filter them out before the fold, and the fold itself stays a three-branch
function over games that are actually going to happen:

playable = [s for s in statuses if s not in _WILL_NOT_RESOLVE]

Do not implement this as "count postponed as final" instead. Any version
that counts terminal-vs-not sees "one POSTPONED, fifteen SCHEDULED" as a
mix and returns IN_PROGRESS for a week where nothing has kicked off --
and postponements are usually announced days ahead, so that is an ordinary
Tuesday, not an edge case. Under #410 it would start score polling early and,
once nag_players reads week state, stop nagging players who have not picked.
Skipping gets it right; folding does not.

This is provisional. What actually happens to a postponed or suspended
game -- rescheduled into this week, moved to another, abandoned -- is
unresolved and wants its own issue. A rescheduled game reappears under status
0 with a new kickoff, which is a problem for
Game.get_first_game_of_the_week and the nag times rather than for this fold.

Knowingly unhandled: if every game in a week is postponed the filtered list
is empty and this returns PENDING. Not worth a branch.

in_progress means "started but not finished", not "a game is live right
now".
A week with thirteen finals and a Monday nighter yet to kick off is
neither pregame nor all_final. A naive any(status == IN_PROGRESS) gets
that case wrong — put it in the docstring, it is the assumption a future reader
will make incorrectly.

It also must not be a method on Week.
As of Phase 1,
app/models/game.py imports Week at module level —
Game.current_season_distinct_week_infos calls Week.current(session) — so
the dependency arrow inside app/models currently runs game -> week.
Putting the state calculation on Week as a query method (the obvious shape:
Week.recompute_state(session), reading Game rows) closes that into
game -> week -> game. That is not a style problem; it breaks
import app.models outright, and it will surface as an unrelated-looking
ImportError somewhere far from the change.
A module-level function taking a sequence of status strings has no such
problem: it imports nothing from game, and callers pass the statuses in.

The conclusion holds; the import argument does not survive #414.
app/models/game.py:162 is the only use of Week in that file, and it
sits inside current_season_distinct_week_infos itself. Once #414 moves that
method onto Week and rewrites it to read the week table, game.py drops
its Week import and there is no import edge between the two modules in
either direction — at which point week -> game would be perfectly legal.

The reason that does survive is the one #414 is itself establishing: Week
answers from the week table alone.
A method on Week that reads Game
re-muddies that the moment #414 has cleaned it, and reads as "the week derives
itself from games" even when it is computed on read and stores nothing. The
fold belongs on Game — which after #409 has week_id and can do it as a
one-column filter — or in the job.

Test cases

Statuses are GameStatus members, not the ESPN-era STATUS_* strings.

input expected
[] PENDING
all FINAL ALL_FINAL
all SCHEDULED PREGAME
one IN_PROGRESS, rest scheduled IN_PROGRESS
finals + scheduled, none live IN_PROGRESS
single-game week, final ALL_FINAL
all FINAL plus one POSTPONED ALL_FINAL
all FINAL plus one SUSPENDED ALL_FINAL
all SCHEDULED plus one POSTPONED PREGAME
finals + scheduled + one POSTPONED IN_PROGRESS
one IN_PROGRESS, rest POSTPONED IN_PROGRESS

Done when

  • Every row of the table above passes.
  • game_state on a live week reflects reality without anyone running a job by hand.
  • odds_state exists and defaults to preliminary. Nothing writes it in this
    phase — its writer is the dispatcher, in #410.
  • Game.spread is nullable, and a game with no odds in the payload is stored
    as null rather than as home −0.5.
Parent: #406 · Tech debt: *`Week` has no state machine* > **Revised 2026-08-22 after a design pass.** The week carries **two** state > machines, not one. `game_state` is an *observation*, projected from `game` > rows. `odds_state` is a *decision*, made and recorded by the dispatcher. > Strikethrough below is superseded text, kept so the reasoning is traceable. ## Why `app/models/week_state.py` defines `WeekState` (`pending` / `pregame` / `in_progress` / `all_final`) and nothing imports it. The `game_state` column was pulled back out of `Week` before shipping, because a column no job writes would have sat at `pending` on every row and lied. This phase is independent of Phase 3 and 4. It only needs Phase 1 far enough along that `week` rows exist. ## Tasks - [ ] **Add an in-memory SQLite `session` fixture to `tests/conftest.py`.** `Week` has no foreign keys, so it needs no seed data — this is the cheapest possible entry point into the "no test database" problem, and everything below leans on it. - [ ] **Add `GameStatus` as a `StrEnum`** in `app/models/game_status.py`, alongside `season.py` and `week_state.py`. Members `SCHEDULED`, `IN_PROGRESS`, `FINAL`, `POSTPONED`, `SUSPENDED`; lowercase values, to match `WeekState`. `Game.game_status` stays annotated `str` -- a `StrEnum` member *is* a `str`, so it stores with no conversion and compares equal to what comes back off a loaded row. What this replaces is ESPN's vocabulary. `tank01_api.py:29` maps Tank01's `"0"`-`"4"` onto `STATUS_SCHEDULED` / `STATUS_FINAL` strings, and `tests/test_tank01_api.py:13` says why out loud -- "ESPN-style strings ... because models/game.py compares against" them. ESPN is gone; the strings outlived it. **One map, not two.** `_GAME_STATUS_TYPES` stays in `tank01_api.py` and only changes what it maps *to* (`GameStatus.FINAL`, not `"STATUS_FINAL"`), so `NflApiGame.game_status` is already a `GameStatus` and the sync job assigns it with no translation of its own. The dict's keys are Tank01's and stay provider-local; its values are ours and are shared. A second map in the sync job would be an identity map. This is the same split `season.py` already documents -- *"these ints are what the DB stores and what every API wrapper normalizes to, so the mapping belongs with the models rather than on whichever provider is current."* `game_status` never got the same treatment. The wrapper will import from `app.models` for the first time. `test_integrity` only forbids the reverse direction, so this is allowed -- but import the submodule (`from app.models.game_status import GameStatus`), not the package: a bare `app.models` registers every SQLModel table to get one enum. Do it in this phase and it is a rename, not a data migration: `game` is still empty in production. `SeasonType` stays an `IntEnum` because season type is genuinely *ordered* -- `Week.current()` sorts on it. Game status has no order, so it takes the `WeekState` treatment instead. - [ ] **Write `week_state_from_statuses()` as a pure function, with tests.** No DB, no wiring, nothing imports it yet. - [ ] **Add `game_state` back to `Week`** plus a migration, defaulting to `pending`. Annotate the column `str`, not `WeekState` — a native Postgres enum makes every future state an `ALTER TYPE` on a live table. - [ ] **Add `odds_state` to `Week`**, same migration, defaulting to `preliminary`. New `OddsState` StrEnum in `app/models/week_state.py`: `PRELIMINARY` → `LOCKED`, one direction. This single fact is both "odds are frozen" and "the picks page is released" — they are one moment with two names, see the design notes. - [ ] **Make `Game.spread` and `Game.favorite_team_id` nullable**, same migration. Today a game with no odds is written as home −0.5, byte for byte identical to a genuine pick'em, so "does every game have a line" is not an answerable question — and that is precisely the guard on the `LOCKED` transition. Null means "no odds yet"; the transition is what guarantees non-null afterwards. `game` is still empty in production, so this is one revision with no backfill. Blast radius is `app/templates/picks.j2:48`, `app/jobs/create_picks.py`, and one comparison at `app/models/game.py:83`. - [ ] ~~**Give it a writer.** `sync_current_week` (or `update_a_game`) computes~~ ~~the state from the provider payload and writes it.~~ - [ ] **Give `game_state` a writer.** One shared helper, called by every job that writes `game` rows, **inside the same transaction as the write**. Not `sync_current_week` — that job only ever sees what the provider says the current week is, and never touches a game row. Not a standalone projection job either: a separate job is exactly where drift comes from. Committing the projection alongside the rows it summarises takes the drift window to zero rather than to one tick. - [ ] **Delete `_games_exist_and_all_games_are_final()`** in `app/jobs/award_update_all.py` — defined, called from nowhere, and very nearly the `pending` / `all_final` half of this logic. Do not leave a third copy. ## Design notes **Two kinds of column, opposite failure modes.** Both live on `week`, and the obvious tidy-up on either one breaks it. Comment them individually. | | `game_state` | `odds_state` | |---|---|---| | kind | materialised projection | latched decision | | source | fold over `game` rows | the dispatcher's own action | | recomputed | every write, from scratch | **never** | | written by | sync | dispatcher | | the bug | forgetting to recompute (silent staleness) | recomputing it (destroys information) | `game_state` drifting is self-healing and benign — the next write corrects it, and the worst case is one extra poll or a picker link appearing a beat late. `odds_state` is not derivable from anything: no query over any table can tell you whether you released the picks page. **Storing `game_state` does not violate the one-way rule** — but #410 states that rule in a form that forbids it outright. That paragraph is being amended there: week *identity* comes from the provider and nothing derives it; week *state* may be projected from tables that week drives, provided it is recomputed from scratch and never read back into identity. The circularity the rule exists to prevent is `Week.current()` needing games to answer, which would deadlock, because games are created *for* a week. State is not identity. **Why `odds_state` is one fact and not two.** An earlier draft of this had `odds_locked` and `picks_released` as separate facts on the theory that odds "settle" at some observable moment you might want to freeze before releasing. They do not — lines move on injury news right up to kickoff. The freeze is an act of the pool, not an observation of the market, and there is no reason to perform it earlier than the moment you show people the numbers they are picking against. One column. That also means odds *coverage* is a **guard**, not a trigger. The trigger is policy (today: Wednesday 6am PT). The guard is "every game has a line, and `now < first_kickoff`". See #410. **`odds_state` needs no unlock tracking.** Going back to `PRELIMINARY` is only legal before any `PlayerGamePick` exists for the week, and in that region it has no consequences to record: nothing downstream depends on the old spreads, so unlock-then-relock is indistinguishable from having locked correctly the first time. The end state is the whole truth. The one thing that looks like it needs history — not re-announcing "picks are open" on the second lock — is covered by the effect ledger, not by this column. **Pure function, not a query method.** ~~Two reasons: there is no seedable test~~ ~~database, so a pure function is the only version testable today; and it has two~~ ~~callers on opposite sides of the one-way data rule — the live updater feeds it~~ ~~statuses from the API payload, a backfill would feed it statuses from `game`~~ ~~rows. The Week table still never derives itself from tables it drives.~~ Both of those reasons are gone. The first task in this issue builds the test database. And under the sync/dispatch split there is only ever one caller: the live updater writes its game row and stops, and the projection reads rows. Nothing feeds statuses in from an API payload any more. The pure function survives regardless, as the **inner** layer — it is the only part with branching worth testing exhaustively, and the test table below is still the specification. What changes is that it is no longer the public API. The wrapper that loads rows and folds them is three lines, and that is what callers use. **`POSTPONED` and `SUSPENDED` are skipped, not folded.** Tank01 has five status codes, not three (`0` scheduled, `1` in progress, `2` final, `3` postponed, `4` suspended). The old string comparison quietly ignored the last two: `is_final`, `is_pregame` and `is_in_progress` in `app/models/game.py` are three independent equality checks, so a postponed game answered `False` to all three and belonged to no bucket. A week containing one would never reach `ALL_FINAL` -- polling would never stop and the rollups would never fire. Filter them out before the fold, and the fold itself stays a three-branch function over games that are actually going to happen: ```python playable = [s for s in statuses if s not in _WILL_NOT_RESOLVE] ``` Do **not** implement this as "count postponed as final" instead. Any version that counts terminal-vs-not sees "one `POSTPONED`, fifteen `SCHEDULED`" as a mix and returns `IN_PROGRESS` for a week where nothing has kicked off -- and postponements are usually announced days ahead, so that is an ordinary Tuesday, not an edge case. Under #410 it would start score polling early and, once `nag_players` reads week state, stop nagging players who have not picked. Skipping gets it right; folding does not. **This is provisional.** What actually happens to a postponed or suspended game -- rescheduled into this week, moved to another, abandoned -- is unresolved and wants its own issue. A rescheduled game reappears under status `0` with a new kickoff, which is a problem for `Game.get_first_game_of_the_week` and the nag times rather than for this fold. Knowingly unhandled: if *every* game in a week is postponed the filtered list is empty and this returns `PENDING`. Not worth a branch. **`in_progress` means "started but not finished", not "a game is live right now".** A week with thirteen finals and a Monday nighter yet to kick off is neither `pregame` nor `all_final`. A naive `any(status == IN_PROGRESS)` gets that case wrong — put it in the docstring, it is the assumption a future reader will make incorrectly. **It also must not be a method on `Week`.** ~~As of Phase 1,~~ ~~`app/models/game.py` imports `Week` at module level —~~ ~~`Game.current_season_distinct_week_infos` calls `Week.current(session)` — so~~ ~~the dependency arrow inside `app/models` currently runs `game -> week`.~~ ~~Putting the state calculation on `Week` as a query method (the obvious shape:~~ ~~`Week.recompute_state(session)`, reading `Game` rows) closes that into~~ ~~`game -> week -> game`. That is not a style problem; it breaks~~ ~~`import app.models` outright, and it will surface as an unrelated-looking~~ ~~ImportError somewhere far from the change.~~ ~~A module-level function taking a sequence of status strings has no such~~ ~~problem: it imports nothing from `game`, and callers pass the statuses in.~~ The conclusion holds; the import argument does not survive #414. `app/models/game.py:162` is the **only** use of `Week` in that file, and it sits inside `current_season_distinct_week_infos` itself. Once #414 moves that method onto `Week` and rewrites it to read the `week` table, `game.py` drops its `Week` import and there is no import edge between the two modules in either direction — at which point `week -> game` would be perfectly legal. The reason that does survive is the one #414 is itself establishing: **`Week` answers from the `week` table alone.** A method on `Week` that reads `Game` re-muddies that the moment #414 has cleaned it, and reads as "the week derives itself from games" even when it is computed on read and stores nothing. The fold belongs on `Game` — which after #409 has `week_id` and can do it as a one-column filter — or in the job. ## Test cases Statuses are `GameStatus` members, not the ESPN-era `STATUS_*` strings. | input | expected | |---|---| | `[]` | `PENDING` | | all `FINAL` | `ALL_FINAL` | | all `SCHEDULED` | `PREGAME` | | one `IN_PROGRESS`, rest scheduled | `IN_PROGRESS` | | finals + scheduled, none live | `IN_PROGRESS` | | single-game week, final | `ALL_FINAL` | | all `FINAL` plus one `POSTPONED` | `ALL_FINAL` | | all `FINAL` plus one `SUSPENDED` | `ALL_FINAL` | | all `SCHEDULED` plus one `POSTPONED` | **`PREGAME`** | | finals + scheduled + one `POSTPONED` | `IN_PROGRESS` | | one `IN_PROGRESS`, rest `POSTPONED` | `IN_PROGRESS` | ## Done when - Every row of the table above passes. - `game_state` on a live week reflects reality without anyone running a job by hand. - `odds_state` exists and defaults to `preliminary`. Nothing writes it in this phase — its writer is the dispatcher, in #410. - `Game.spread` is nullable, and a game with no odds in the payload is stored as null rather than as home −0.5.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
johnsturgeon/tgfp-web#408
No description provided.