Re-write the Tank API so that it is a smart python wrapper of the actual API #427

Closed
opened 2026-08-25 15:16:06 +02:00 by johnsturgeon · 1 comment
Owner
No description provided.
Author
Owner

Provider boundary: shaping Tank01 data into pool data

Design notes, not a plan. This records why the provider boundary is being
re-cut and which direction the pieces should face, so each step can be scoped
on its own later without re-deriving the reasoning.

Deliberately does not enumerate the steps. Work is scoped one piece at a time,
as each becomes obvious; every piece is expected to be non-breaking and tested
on its own. Nothing here commits to an order or a finish line.


The problem

app/tank01_api/tank01_api.py fetches Tank01 payloads and shapes them into
NflApiGame, which the ingest job then copies into Game. That copy
(_game_from_api_game) is close to a 1:1 field mapping, which reads as though
the intermediate class is not paying for itself.

A boring mapper is not the problem. The problem is that NflApiGame was shaped
to resemble Game rather than to resemble Tank01, so we maintain two types that
must change together and get the benefit of neither.

The mapping is also not truly 1:1: NflApiGame carries Tank01 team-id
strings, while Game carries FK ints. Identity resolution across a system
boundary is exactly what this layer is for — it is just doing less than it
appears to.

The target shape

Four responsibilities:

  1. Transport — auth, retries, endpoints, per-week memo. Returns payloads,
    decides nothing.
  2. Provider schema (DTO) — a typed mirror of what the provider said, in
    the provider's vocabulary. Dumb on purpose. gameStatusCode stays a string
    called gameStatusCode.
  3. Mapper — pure functions, provider vocabulary → pool vocabulary. The only
    code that speaks both languages.
  4. Domain / persistenceGame, and the upsert that writes it.

The industry name for the whole boundary is an anti-corruption layer; the middle
two pieces are the DTO and the mapper.

Three tiers of type fall out of that:

  1. Provider DTO — Tank01's vocabulary. gameStatusCode: str, homeTeamSpread: "+-2.5".
  2. Pool value objects — our vocabulary, but not persistence: no table, no
    primary key, no FKs, no session. Line, GameFacts, and today's NflApiScore.
  3. tgfp modelsGame, Team, Week. Tables, FKs, ids.

Tier 2 is what lets mappers be pure. Going DTO → Game directly means every
mapper needs FK lookups, so every mapper needs a session, so every mapper test
needs a database. GameFacts is the mapper's output; resolving it into a row is
the persistence step's job.

The test for whether the seam is in the right place

Ask which change forces which edit:

  • Tank01 renames a field → the DTO and the mapper change, nothing else.
  • The pool changes how it picks a favorite → domain logic changes, no
    provider file is touched
    .

The second one currently fails. Median-across-books and favorite selection are
pool rules living in tank01_api.py. Switching providers would mean carrying
them across, which is the outcome the boundary exists to prevent. We already ran
that migration once (ESPN → Tank01), which is the evidence that the boundary
earns its keep here rather than being ceremony.

Where the code lives

app/
  tank01_api/          # the only code that knows Tank01 exists
    client.py          # transport: auth, retries, endpoints, per-week memo
    schemas.py         # Pydantic mirrors of Tank01 payloads, verbatim

  ingest/              # provider -> pool. write path only.
    facts.py           # tier 2: GameFacts, KickoffFacts, Line -- frozen, inert
    mappers.py         # pure: schemas -> facts
    games.py           # persistence: facts -> Game rows (takes a session)

  models/              # tier 3 + shared pool vocabulary  (unchanged)
    game.py  team.py  week.py  player.py ...        # SQLModel tables
    game_status.py  season.py  week_state.py        # shared vocabulary
    week_info.py  data_source.py

  jobs/                # orchestration: decides *what* to fetch, and when
    sync_games_for_current_week.py ...

Tier 2 lands in two places

app/models/ already holds tier-2 objects — WeekInfo is a plain dataclass
with no table, and GameStatus / SeasonType / OddsState are enums. That is
the right home for shared vocabulary: things routers, templates, jobs and
mappers all speak.

GameFacts is different. It is meaningful only on the write path — produced by
a mapper, consumed by an upsert two lines later, seen by nothing else. That
belongs in app/ingest/facts.py.

The line: shared vocabulary → models/. Ingest-only fact bundles → ingest/.

Dependency direction

jobs ──> ingest ──> tank01_api
  └────────┴──> models
  • tank01_api/ imports nothing from app/models/. It speaks Tank01 and HTTP.
  • ingest/ imports both — schemas from the provider, enums and tables from
    models. It is the only layer allowed to.
  • jobs/ owns policy: reads odds_state, decides whether odds are worth
    fetching, calls the right mapper.

The first rule is mechanically checkable, which is what makes it worth stating:

grep -rn "from app.models" app/tank01_api/

Empty means the seam is clean. Today it fails by exactly one line —
tank01_api.py imports GameStatus, because the provider wrapper currently does
the "2"FINAL translation. Move that into the mapper and the import goes
away on its own.

The data path, end to end

One week of games, top to bottom.

app/tank01_api/schemas.py — provider vocabulary, verbatim.

class Tank01ScheduleGame(BaseModel):
    gameID: str
    teamIDHome: str
    teamIDAway: str
    gameTime_epoch: str
    espnID: str
    season: str

app/tank01_api/client.py — memoized, week-pinned, returns DTOs and nothing else.

class Tank01Client:
    def __init__(self, season: int, week_no: int, season_type: int): ...

    @cached_property
    def schedule(self) -> list[Tank01ScheduleGame]: ...      # 1 call
    @cached_property
    def scores(self) -> dict[str, Tank01ScoreRow]: ...       # 1 call
    @cached_property
    def odds(self) -> dict[str, Tank01GameOdds]: ...         # ~3 calls

app/ingest/facts.py — tier 2. Inert. Carries Tank01 team-id strings, not FKs.

@dataclass(frozen=True)
class Line:
    spread: float
    home_is_favorite: bool

@dataclass(frozen=True)
class GameFacts:
    external_game_id: str
    home_team_ext_id: str
    away_team_ext_id: str
    start_time: datetime
    status: GameStatus
    home_points: int
    away_points: int
    season: int
    season_type: int
    week_no: int
    event_id: int
    line: Line | None          # None = odds unknown, not 0.5

app/ingest/mappers.py — pure, no session, testable from a dict.

def to_line(odds: Tank01GameOdds | None) -> Line | None: ...

def to_game_facts(
    schedule: Tank01ScheduleGame,
    score: Tank01ScoreRow,
    odds: Tank01GameOdds | None,
) -> GameFacts: ...

app/ingest/games.py — the only step that touches the DB. ext-id → FK happens here.

def upsert_games(session: Session, week: Week, facts: list[GameFacts]) -> None:
    for f in facts:
        game = ...  # select by external_game_id, or construct
        game.start_time = f.start_time
        game.game_status = f.status
        game.home_team_score = f.home_points
        game.road_team_score = f.away_points
        if f.line is not None:                    # locked week: leave the line alone
            game.spread = f.line.spread
            game.favorite_team_id = (
                home.id if f.line.home_is_favorite else away.id
            )
        session.add(game)

app/jobs/sync_games_for_current_week.py — policy. The job decides what is
worth fetching.

client = Tank01Client(season=w.season, week_no=w.week_no, season_type=w.season_type)
odds = client.odds if w.odds_state != OddsState.LOCKED else {}
facts = [
    to_game_facts(s, client.scores[s.gameID], odds.get(s.gameID))
    for s in client.schedule
]
upsert_games(session, w, facts)

Two things in that last block carry the whole design.

odds = client.odds if ... else {} must be hoisted out of the comprehension.
Referencing client.odds inside the loop would trigger the cached_property and
spend the ~3 calls even on a locked week — the exact "a flag can only skip
writes" trap already documented in sync_scores_for_current_week.py. That one
line is the whole of #425.

And line=None flows all the way to upsert_games, where it means do not touch
those columns
. No fabricated 0.5 anywhere in the path, and nothing can overwrite
a locked line, because the value that would have overwritten it was never
constructed.

What moves where

Run this sort over NflApiGame and nothing is left, which is why what remains is
GameFacts: a container with no behavior.

The test is does it need to know what Tank01 said?

→ Mapper (yes, it does):

  • _parse_spread turning "+-2.5" into -2.5
  • dropping unreadable books, and the Sentry log that goes with it
  • median across N sportsbooks
  • sign → which side is favored
  • abs() + round-half-up
  • ""0 for blank points on unstarted games
  • epoch string → UTC datetime
  • "2"GameStatus.FINAL
  • int(espnID)

→ Stays a property, downstream (reads only normalized fields):

is_pregame, is_final, winning_team. These know nothing about Tank01 — they
read status and two ints. They are domain predicates, and they already exist on
Game (game.py:84, :89, :137), duplicated with NflApiGame today. The
duplication disappears on its own once NflApiGame does.

→ Persistence (needs the DB):

home_team, away_team, favored_team. Not computation — resolution.
ext-id → FK, in upsert_games, which has the session.

→ Deleted, not moved:

The pick'em fallback — no odds → spread = 0.5, home favored. That exists only
because "unknown" was not representable. line: Line | None makes it
representable, so the workaround evaporates rather than relocating.

Caching and fetch policy

Splitting mappers out does not cost the per-week caching, because caching is a
transport concern and stays in the client. The seam is already visible in
today's class:

  • Fetch + memo (stays): games_data, scores_data, teams_data,
    odds_data, current_infocached_property over an HTTP call, pinned by
    the constructor.
  • Shaping (moves out): games, live_scores, teams, find_teams.

Two different kinds of "smart", and they belong in different places:

  • Memoizing within a scope — "already fetched this week's schedule." The
    client's job. Keep it.
  • Deciding what is worth fetching — "odds are locked, do not fetch odds."
    Policy. Depends on odds_state, which is a pool concept the client has no
    business knowing. The job's job.

Today NflApi.games makes the second decision by always touching odds_data,
which is the whole of #425.

Pulling mappers out does not multiply calls: two mappers needing the same
schedule hit the same memo on the same client instance. Cache the fetch, not
the translation.

One scope note: cached_property means the memo lives as long as the object,
which is correct here because a client is already constructed per job run. Keep
doing that. The failure mode would be a module-level singleton, which turns a
per-run cache into a stale-forever cache.

Worked example: odds

Odds is the least 1:1 thing in the system. There is no odds table; OddsState
in week_state.py is an enum. Odds resolve to exactly two columns on Game:

favorite_team_id: Optional[int]   # FK
spread: Optional[float]           # "null until odds arrive"

So the shape of the work is:

~3 date-scoped calls → N games × M sportsbooks × a string like "+-2.5"
→ one float and one FK.

Parsing "+-2.5" is provider handling; median-across-books and favorite
selection are pool rules. Both land in the mapper, for different reasons — the
first because only the mapper should know Tank01's string forms, the second
because no provider file should encode how the pool picks a favorite.

On DTO scope: model the fields we consume and let Pydantic ignore the rest. A
faithful mirror of every Tank01 field is not needed, because drift is already
covered by the live contract tests and the weekly api-drift workflow. Strict
validation on the fields we use, loose on everything else.

GameFacts is roughly NflApiGame today

Field-for-field it is close to a rename. Three differences, and they are the point:

  1. NflApiGame holds a live client. self._data_source is stashed in
    __init__, so home_team / away_team / favored_team can spend HTTP calls
    on access. GameFacts holds ext-id strings and resolves nothing. That is what
    makes it inert: construct it, and everything it will ever say is decided.
  2. NflApiGame cannot say "I do not know." No odds → spread = 0.5.
    GameFacts.line is Line | None.
  3. NflApiGame computes; GameFacts carries. Parsing, median, sign and
    rounding happen in __init__ today. Under the split the mapper does that and
    hands over the result — a container, not a calculator. Same data, different
    author: NflApiGame is built by the client, GameFacts by a mapper outside it.

The rest of the delta is small: spread + _favored_team_id collapse into
line, _data_source disappears, the team ids become plain strings, and
game_date drops out entirely — it never escapes app/tank01_api/, existing
only as the key for the /getNFLBettingOdds fetch loop, which makes it a
transport detail.

NflApiScore is already a proper tier-2 object — inert, no client handle, no
session. It is the template. NflApiGame is the one that drifted.

Approach

Build the pieces in place inside app/tank01_api/tank01_api.py first, then
migrate them to their own modules once they exist and are covered. Growing the
new shape next to the old one keeps every step non-breaking; moving files is a
separate, mechanical step that can happen when the contents have settled.

How this relates to #425

#425 wants the
odds fetch split from the games fetch, because a locked week re-fetches the
schedule all week (flex scheduling) and pays ~3 extra /getNFLBettingOdds calls
every time, on a metered plan.

It is awkward to fix today because the only way to say "skip odds" through
NflApiGame.__init__ is odds_data=None, which silently resolves to
spread=0.5 / home favored — and the ingest job then writes that over the
locked line.

Under the shape above, #425 stops being a flag or a second return type. "Schedule
only" is just a mapper with a narrower signature (to_kickoff_facts), and there
is no odds parameter to pass None to. The awkwardness is this design question
surfacing as a bug.

Noticed along the way

  • Game.spread and Game.favorite_team_id are both Optional with
    default=None. But the comment in test_game_without_odds_defaults_to_home_favorite
    justifies the 0.5 fallback by asserting both are non-nullable. That is stale:
    "odds unknown" is already representable, so the fabricated pick'em is a
    workaround for a constraint that no longer exists. Worth resolving on its own.
  • NflApi.games has exactly one production consumer,
    sync_games_for_current_week.py. Nothing else can be affected by how it is split.
  • find_game was test-only surface and has been removed; tests now use a local
    game_by_id helper.
# Provider boundary: shaping Tank01 data into pool data Design notes, not a plan. This records *why* the provider boundary is being re-cut and *which direction* the pieces should face, so each step can be scoped on its own later without re-deriving the reasoning. Deliberately does not enumerate the steps. Work is scoped one piece at a time, as each becomes obvious; every piece is expected to be non-breaking and tested on its own. Nothing here commits to an order or a finish line. --- ## The problem `app/tank01_api/tank01_api.py` fetches Tank01 payloads and shapes them into `NflApiGame`, which the ingest job then copies into `Game`. That copy (`_game_from_api_game`) is close to a 1:1 field mapping, which reads as though the intermediate class is not paying for itself. A boring mapper is not the problem. The problem is that `NflApiGame` was shaped to resemble `Game` rather than to resemble Tank01, so we maintain two types that must change together and get the benefit of neither. The mapping is also not truly 1:1: `NflApiGame` carries Tank01 team-id *strings*, while `Game` carries FK *ints*. Identity resolution across a system boundary is exactly what this layer is for — it is just doing less than it appears to. ## The target shape Four responsibilities: 1. **Transport** — auth, retries, endpoints, per-week memo. Returns payloads, decides nothing. 2. **Provider schema (DTO)** — a typed mirror of what the provider *said*, in the provider's vocabulary. Dumb on purpose. `gameStatusCode` stays a string called `gameStatusCode`. 3. **Mapper** — pure functions, provider vocabulary → pool vocabulary. The only code that speaks both languages. 4. **Domain / persistence** — `Game`, and the upsert that writes it. The industry name for the whole boundary is an anti-corruption layer; the middle two pieces are the DTO and the mapper. Three tiers of type fall out of that: 1. **Provider DTO** — Tank01's vocabulary. `gameStatusCode: str`, `homeTeamSpread: "+-2.5"`. 2. **Pool value objects** — our vocabulary, but not persistence: no table, no primary key, no FKs, no session. `Line`, `GameFacts`, and today's `NflApiScore`. 3. **tgfp models** — `Game`, `Team`, `Week`. Tables, FKs, ids. Tier 2 is what lets mappers be pure. Going DTO → `Game` directly means every mapper needs FK lookups, so every mapper needs a session, so every mapper test needs a database. `GameFacts` is the mapper's output; resolving it into a row is the persistence step's job. ## The test for whether the seam is in the right place Ask which change forces which edit: - Tank01 renames a field → the DTO and the mapper change, **nothing else**. - The pool changes how it picks a favorite → domain logic changes, **no provider file is touched**. The second one currently fails. Median-across-books and favorite selection are *pool rules* living in `tank01_api.py`. Switching providers would mean carrying them across, which is the outcome the boundary exists to prevent. We already ran that migration once (ESPN → Tank01), which is the evidence that the boundary earns its keep here rather than being ceremony. ## Where the code lives ``` app/ tank01_api/ # the only code that knows Tank01 exists client.py # transport: auth, retries, endpoints, per-week memo schemas.py # Pydantic mirrors of Tank01 payloads, verbatim ingest/ # provider -> pool. write path only. facts.py # tier 2: GameFacts, KickoffFacts, Line -- frozen, inert mappers.py # pure: schemas -> facts games.py # persistence: facts -> Game rows (takes a session) models/ # tier 3 + shared pool vocabulary (unchanged) game.py team.py week.py player.py ... # SQLModel tables game_status.py season.py week_state.py # shared vocabulary week_info.py data_source.py jobs/ # orchestration: decides *what* to fetch, and when sync_games_for_current_week.py ... ``` ### Tier 2 lands in two places `app/models/` already holds tier-2 objects — `WeekInfo` is a plain dataclass with no table, and `GameStatus` / `SeasonType` / `OddsState` are enums. That is the right home for **shared vocabulary**: things routers, templates, jobs and mappers all speak. `GameFacts` is different. It is meaningful only on the write path — produced by a mapper, consumed by an upsert two lines later, seen by nothing else. That belongs in `app/ingest/facts.py`. The line: **shared vocabulary → `models/`. Ingest-only fact bundles → `ingest/`.** ### Dependency direction ``` jobs ──> ingest ──> tank01_api └────────┴──> models ``` - `tank01_api/` imports **nothing** from `app/models/`. It speaks Tank01 and HTTP. - `ingest/` imports both — schemas from the provider, enums and tables from models. It is the only layer allowed to. - `jobs/` owns policy: reads `odds_state`, decides whether odds are worth fetching, calls the right mapper. The first rule is mechanically checkable, which is what makes it worth stating: ```bash grep -rn "from app.models" app/tank01_api/ ``` Empty means the seam is clean. Today it fails by exactly one line — `tank01_api.py` imports `GameStatus`, because the provider wrapper currently does the `"2"` → `FINAL` translation. Move that into the mapper and the import goes away on its own. ## The data path, end to end One week of games, top to bottom. **`app/tank01_api/schemas.py`** — provider vocabulary, verbatim. ```python class Tank01ScheduleGame(BaseModel): gameID: str teamIDHome: str teamIDAway: str gameTime_epoch: str espnID: str season: str ``` **`app/tank01_api/client.py`** — memoized, week-pinned, returns DTOs and nothing else. ```python class Tank01Client: def __init__(self, season: int, week_no: int, season_type: int): ... @cached_property def schedule(self) -> list[Tank01ScheduleGame]: ... # 1 call @cached_property def scores(self) -> dict[str, Tank01ScoreRow]: ... # 1 call @cached_property def odds(self) -> dict[str, Tank01GameOdds]: ... # ~3 calls ``` **`app/ingest/facts.py`** — tier 2. Inert. Carries Tank01 team-id *strings*, not FKs. ```python @dataclass(frozen=True) class Line: spread: float home_is_favorite: bool @dataclass(frozen=True) class GameFacts: external_game_id: str home_team_ext_id: str away_team_ext_id: str start_time: datetime status: GameStatus home_points: int away_points: int season: int season_type: int week_no: int event_id: int line: Line | None # None = odds unknown, not 0.5 ``` **`app/ingest/mappers.py`** — pure, no session, testable from a dict. ```python def to_line(odds: Tank01GameOdds | None) -> Line | None: ... def to_game_facts( schedule: Tank01ScheduleGame, score: Tank01ScoreRow, odds: Tank01GameOdds | None, ) -> GameFacts: ... ``` **`app/ingest/games.py`** — the only step that touches the DB. ext-id → FK happens here. ```python def upsert_games(session: Session, week: Week, facts: list[GameFacts]) -> None: for f in facts: game = ... # select by external_game_id, or construct game.start_time = f.start_time game.game_status = f.status game.home_team_score = f.home_points game.road_team_score = f.away_points if f.line is not None: # locked week: leave the line alone game.spread = f.line.spread game.favorite_team_id = ( home.id if f.line.home_is_favorite else away.id ) session.add(game) ``` **`app/jobs/sync_games_for_current_week.py`** — policy. The job decides what is worth fetching. ```python client = Tank01Client(season=w.season, week_no=w.week_no, season_type=w.season_type) odds = client.odds if w.odds_state != OddsState.LOCKED else {} facts = [ to_game_facts(s, client.scores[s.gameID], odds.get(s.gameID)) for s in client.schedule ] upsert_games(session, w, facts) ``` Two things in that last block carry the whole design. `odds = client.odds if ... else {}` **must** be hoisted out of the comprehension. Referencing `client.odds` inside the loop would trigger the `cached_property` and spend the ~3 calls even on a locked week — the exact "a flag can only skip *writes*" trap already documented in `sync_scores_for_current_week.py`. That one line is the whole of #425. And `line=None` flows all the way to `upsert_games`, where it means *do not touch those columns*. No fabricated 0.5 anywhere in the path, and nothing can overwrite a locked line, because the value that would have overwritten it was never constructed. ## What moves where Run this sort over `NflApiGame` and nothing is left, which is why what remains is `GameFacts`: a container with no behavior. The test is **does it need to know what Tank01 said?** **→ Mapper** (yes, it does): - `_parse_spread` turning `"+-2.5"` into `-2.5` - dropping unreadable books, and the Sentry log that goes with it - median across N sportsbooks - sign → which side is favored - `abs()` + round-half-up - `""` → `0` for blank points on unstarted games - epoch string → UTC datetime - `"2"` → `GameStatus.FINAL` - `int(espnID)` **→ Stays a property, downstream** (reads only normalized fields): `is_pregame`, `is_final`, `winning_team`. These know nothing about Tank01 — they read `status` and two ints. They are domain predicates, and they already exist on `Game` (`game.py:84`, `:89`, `:137`), duplicated with `NflApiGame` today. The duplication disappears on its own once `NflApiGame` does. **→ Persistence** (needs the DB): `home_team`, `away_team`, `favored_team`. Not computation — *resolution*. ext-id → FK, in `upsert_games`, which has the session. **→ Deleted, not moved:** The pick'em fallback — no odds → `spread = 0.5`, home favored. That exists only because "unknown" was not representable. `line: Line | None` makes it representable, so the workaround evaporates rather than relocating. ## Caching and fetch policy Splitting mappers out does not cost the per-week caching, because caching is a transport concern and stays in the client. The seam is already visible in today's class: - **Fetch + memo (stays):** `games_data`, `scores_data`, `teams_data`, `odds_data`, `current_info` — `cached_property` over an HTTP call, pinned by the constructor. - **Shaping (moves out):** `games`, `live_scores`, `teams`, `find_teams`. Two different kinds of "smart", and they belong in different places: - **Memoizing within a scope** — "already fetched this week's schedule." The client's job. Keep it. - **Deciding what is worth fetching** — "odds are locked, do not fetch odds." Policy. Depends on `odds_state`, which is a pool concept the client has no business knowing. The job's job. Today `NflApi.games` makes the second decision by always touching `odds_data`, which is the whole of #425. Pulling mappers out does not multiply calls: two mappers needing the same schedule hit the same memo on the same client instance. **Cache the fetch, not the translation.** One scope note: `cached_property` means the memo lives as long as the object, which is correct here because a client is already constructed per job run. Keep doing that. The failure mode would be a module-level singleton, which turns a per-run cache into a stale-forever cache. ## Worked example: odds Odds is the least 1:1 thing in the system. There is no odds table; `OddsState` in `week_state.py` is an enum. Odds resolve to exactly two columns on `Game`: ``` favorite_team_id: Optional[int] # FK spread: Optional[float] # "null until odds arrive" ``` So the shape of the work is: **~3 date-scoped calls → N games × M sportsbooks × a string like `"+-2.5"` → one float and one FK.** Parsing `"+-2.5"` is provider handling; median-across-books and favorite selection are pool rules. Both land in the mapper, for different reasons — the first because only the mapper should know Tank01's string forms, the second because no provider file should encode how the pool picks a favorite. On DTO scope: model the fields we consume and let Pydantic ignore the rest. A faithful mirror of every Tank01 field is not needed, because drift is already covered by the `live` contract tests and the weekly `api-drift` workflow. Strict validation on the fields we use, loose on everything else. ## GameFacts is roughly NflApiGame today Field-for-field it is close to a rename. Three differences, and they are the point: 1. **`NflApiGame` holds a live client.** `self._data_source` is stashed in `__init__`, so `home_team` / `away_team` / `favored_team` can spend HTTP calls on access. `GameFacts` holds ext-id strings and resolves nothing. That is what makes it inert: construct it, and everything it will ever say is decided. 2. **`NflApiGame` cannot say "I do not know."** No odds → `spread = 0.5`. `GameFacts.line` is `Line | None`. 3. **`NflApiGame` computes; `GameFacts` carries.** Parsing, median, sign and rounding happen in `__init__` today. Under the split the mapper does that and hands over the result — a container, not a calculator. Same data, different author: `NflApiGame` is built by the client, `GameFacts` by a mapper outside it. The rest of the delta is small: `spread` + `_favored_team_id` collapse into `line`, `_data_source` disappears, the team ids become plain strings, and `game_date` drops out entirely — it never escapes `app/tank01_api/`, existing only as the key for the `/getNFLBettingOdds` fetch loop, which makes it a transport detail. `NflApiScore` is already a proper tier-2 object — inert, no client handle, no session. It is the template. `NflApiGame` is the one that drifted. ## Approach Build the pieces *in place* inside `app/tank01_api/tank01_api.py` first, then migrate them to their own modules once they exist and are covered. Growing the new shape next to the old one keeps every step non-breaking; moving files is a separate, mechanical step that can happen when the contents have settled. ## How this relates to #425 [#425](https://forgejo.sturgeon.me/johnsturgeon/tgfp-web/issues/425) wants the odds fetch split from the games fetch, because a locked week re-fetches the schedule all week (flex scheduling) and pays ~3 extra `/getNFLBettingOdds` calls every time, on a metered plan. It is awkward to fix today because the only way to say "skip odds" through `NflApiGame.__init__` is `odds_data=None`, which silently resolves to `spread=0.5` / home favored — and the ingest job then writes that over the locked line. Under the shape above, #425 stops being a flag or a second return type. "Schedule only" is just a mapper with a narrower signature (`to_kickoff_facts`), and there is no odds parameter to pass `None` to. The awkwardness is this design question surfacing as a bug. ## Noticed along the way - `Game.spread` and `Game.favorite_team_id` are both `Optional` with `default=None`. But the comment in `test_game_without_odds_defaults_to_home_favorite` justifies the 0.5 fallback by asserting both are non-nullable. That is stale: "odds unknown" is already representable, so the fabricated pick'em is a workaround for a constraint that no longer exists. Worth resolving on its own. - `NflApi.games` has exactly one production consumer, `sync_games_for_current_week.py`. Nothing else can be affected by how it is split. - `find_game` was test-only surface and has been removed; tests now use a local `game_by_id` helper.
Sign in to join this conversation.
No milestone
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.

Dependencies

No dependencies set

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