johnsturgeon/tgfp-web

Watch

Fork 0

Clean up remaining TGFPNfl → ESPNNfl refactor vestiges #365

New issue

Closed

opened 2026-08-12 21:31:27 +02:00 by johnsturgeon · 0 comments

johnsturgeon commented

2026-08-12 21:31:27 +02:00

Owner

Context

Ahead of swapping the ESPN data source for TheSportsDB, the codebase should
have exactly one NFL-data concern in flight. An audit found that the earlier
TGFPNfl → ESPNNfl refactor is essentially complete — the module, all six
exported classes (ESPNNfl, ESPNNflGame, ESPNNflTeam, ESPNNflOdd,
ESPNNflStanding, ESPNSeasonType) and every import site are cleanly
ESPN-named — but four vestiges remain.

The tgfp-nfl PyPI package itself was already removed from
config/requirements.txt (commit d32f1e4); the library is vendored at
app/espn_nfl/.

Phase 1 — Zero-risk cleanup

No behavior change. Safe to do immediately and independently of the
TheSportsDB work.

1.1 Delete dead method `ESPNNflTeam.tgfp_id()`

File: app/espn_nfl/espn_nfl.py, lines 418–430

def tgfp_id(self, tgfp_teams):
    """
    Args:
        tgfp_teams: list of teams to loop through
    Returns:
        the tgfp_id for the current data_source's team, None if not found
    """
    found_team_id = None
    for team in tgfp_teams:
        if self.id == team.tgfp_nfl_team_id:
            found_team_id = team.id
            break
    return found_team_id

This method has zero callers. Verified across all file types repo-wide
(including Jinja templates), with no getattr anywhere that could hide a
dynamic call.

It is worth deleting beyond simply being dead code: line 427 is the only
place the NFL data-source module reaches into the database schema. It is a
leftover of the old "map data-source team → TGFP team" pattern. Removing it
leaves app/espn_nfl/ with no knowledge of the ORM models at all — which is
precisely the decoupling wanted before swapping providers.

1.2 Rename stale internal identifiers

File	Line	Current	Proposed
`app/espn_nfl/espn_nfl.py`	252	`find_tgfp_nfl_standing_for_team`	`find_standing_for_team`
`app/espn_nfl/espn_nfl.py`	192	docstring: "a list of all TgfpNflTeams"	"a list of all `ESPNNflTeam`"

find_tgfp_nfl_standing_for_team has a single internal caller at
app/espn_nfl/espn_nfl.py:204 and no external references, so the rename is
contained to one file.

Optional, judgment call: app/espn_nfl/__init__.py:1 has the module docstring
"""TGFP NFL Model Objects""". This is ambiguous rather than wrong — TGFP is
the project name — so it may be left as is.

1.3 Fix stale documentation

File: CLAUDE.md

Line 135 — points at app/tgfp_nfl/tgfp_nfl.py; the actual path is
app/espn_nfl/espn_nfl.py.
Line 187 — states "Package tgfp-nfl==6.3.3 wraps API". The code is
vendored in-repo and that dependency has been removed.

Phase 2 — Database column rename (decision required)

This phase is a separate decision and should not be bundled with Phase 1
without agreement.

Two live columns, confirmed present in the database:

game.tgfp_nfl_game_id
team.tgfp_nfl_team_id

Both carry the field description "External TGFP/NFL game id", but they hold
ESPN ids today and would hold TheSportsDB ids after the swap. The name
is already inaccurate and the swap makes it more so.

Reference sites (7)

File	Line(s)
`app/models/game.py`	32
`app/models/team.py`	18
`app/jobs/create_picks.py`	31, 34, 38, 53
`app/jobs/sync_team_records.py`	12
`app/jobs/update_game.py`	31
`app/espn_nfl/espn_nfl.py`	427 — removed by Phase 1.1

Proposed naming

Provider-neutral, so a future data-source change requires no further migration:

tgfp_nfl_game_id → external_game_id
tgfp_nfl_team_id → external_team_id

Update the description= text on both Field(...) definitions to match.

⚠️ Migration constraint

The existing Alembic revisions reference these column names as part of applied
migration history:

alembic/versions/9ff5e4af6bbc_01_initial_schema.py (lines 58, 76, 87, 128)
alembic/versions/1885e311300d_02_added_field_uniqueness.py (lines 25–38)

These files must not be edited. A repo-wide find-and-replace would corrupt
migration history. The rename requires a new migration:

scripts/alembic_generate_migration.sh "rename tgfp_nfl id columns to external ids"

Both columns are indexed and unique, so the generated migration should be
reviewed to confirm the indexes are renamed rather than dropped and recreated.

Timing

Do it now — keeps the TheSportsDB swap a single concern, but touches the
schema when the swap itself may not need to.
Do it with the swap — one migration instead of two, but two concerns in
flight at once.

Acceptance criteria

ESPNNflTeam.tgfp_id() removed
find_tgfp_nfl_standing_for_team renamed, sole caller updated
Stale docstrings corrected (espn_nfl.py:192)
CLAUDE.md lines 135 and 187 corrected
(Phase 2, if approved) columns renamed via a new Alembic migration,
all 6 remaining reference sites updated, indexes preserved
pylint still reports 10.00/10
App boots and serves /ping, /login, and the Discord OAuth flow

Verification

After Phase 1, the only remaining matches should be the database column names
(and the Alembic history that must not be touched):

grep -rniE "tgfp[_-]?nfl" . \
  --exclude-dir=.venv --exclude-dir=.git --exclude-dir=node_modules \
  --exclude-dir=__pycache__ --exclude-dir=.pytest_cache --exclude=tgfp.dump

After Phase 2, that search should return only the two Alembic revision
files listed above.

Notes

tgfp.dump (repo root) contains the old column names. It is an untracked
database dump artifact; no action needed.
Repository has no test suite, so verification is lint plus a manual boot and
login round trip.

## Context Ahead of swapping the ESPN data source for TheSportsDB, the codebase should have exactly one NFL-data concern in flight. An audit found that the earlier `TGFPNfl` → `ESPNNfl` refactor is essentially complete — the module, all six exported classes (`ESPNNfl`, `ESPNNflGame`, `ESPNNflTeam`, `ESPNNflOdd`, `ESPNNflStanding`, `ESPNSeasonType`) and every import site are cleanly ESPN-named — but four vestiges remain. The `tgfp-nfl` PyPI package itself was already removed from `config/requirements.txt` (commit `d32f1e4`); the library is vendored at `app/espn_nfl/`. --- ## Phase 1 — Zero-risk cleanup No behavior change. Safe to do immediately and independently of the TheSportsDB work. ### 1.1 Delete dead method `ESPNNflTeam.tgfp_id()` **File:** `app/espn_nfl/espn_nfl.py`, lines 418–430 ```python def tgfp_id(self, tgfp_teams): """ Args: tgfp_teams: list of teams to loop through Returns: the tgfp_id for the current data_source's team, None if not found """ found_team_id = None for team in tgfp_teams: if self.id == team.tgfp_nfl_team_id: found_team_id = team.id break return found_team_id ``` This method has **zero callers**. Verified across all file types repo-wide (including Jinja templates), with no `getattr` anywhere that could hide a dynamic call. It is worth deleting beyond simply being dead code: line 427 is the **only place the NFL data-source module reaches into the database schema**. It is a leftover of the old "map data-source team → TGFP team" pattern. Removing it leaves `app/espn_nfl/` with no knowledge of the ORM models at all — which is precisely the decoupling wanted before swapping providers. ### 1.2 Rename stale internal identifiers | File | Line | Current | Proposed | |---|---|---|---| | `app/espn_nfl/espn_nfl.py` | 252 | `find_tgfp_nfl_standing_for_team` | `find_standing_for_team` | | `app/espn_nfl/espn_nfl.py` | 192 | docstring: "a list of all TgfpNflTeams" | "a list of all `ESPNNflTeam`" | `find_tgfp_nfl_standing_for_team` has a single internal caller at `app/espn_nfl/espn_nfl.py:204` and no external references, so the rename is contained to one file. Optional, judgment call: `app/espn_nfl/__init__.py:1` has the module docstring `"""TGFP NFL Model Objects"""`. This is ambiguous rather than wrong — TGFP is the project name — so it may be left as is. ### 1.3 Fix stale documentation **File:** `CLAUDE.md` - **Line 135** — points at `app/tgfp_nfl/tgfp_nfl.py`; the actual path is `app/espn_nfl/espn_nfl.py`. - **Line 187** — states "Package `tgfp-nfl==6.3.3` wraps API". The code is vendored in-repo and that dependency has been removed. --- ## Phase 2 — Database column rename (decision required) **This phase is a separate decision and should not be bundled with Phase 1 without agreement.** Two live columns, confirmed present in the database: - `game.tgfp_nfl_game_id` - `team.tgfp_nfl_team_id` Both carry the field description `"External TGFP/NFL game id"`, but they hold **ESPN** ids today and would hold **TheSportsDB** ids after the swap. The name is already inaccurate and the swap makes it more so. ### Reference sites (7) | File | Line(s) | |---|---| | `app/models/game.py` | 32 | | `app/models/team.py` | 18 | | `app/jobs/create_picks.py` | 31, 34, 38, 53 | | `app/jobs/sync_team_records.py` | 12 | | `app/jobs/update_game.py` | 31 | | `app/espn_nfl/espn_nfl.py` | 427 — *removed by Phase 1.1* | ### Proposed naming Provider-neutral, so a future data-source change requires no further migration: - `tgfp_nfl_game_id` → `external_game_id` - `tgfp_nfl_team_id` → `external_team_id` Update the `description=` text on both `Field(...)` definitions to match. ### ⚠️ Migration constraint The existing Alembic revisions reference these column names as part of applied migration history: - `alembic/versions/9ff5e4af6bbc_01_initial_schema.py` (lines 58, 76, 87, 128) - `alembic/versions/1885e311300d_02_added_field_uniqueness.py` (lines 25–38) **These files must not be edited.** A repo-wide find-and-replace would corrupt migration history. The rename requires a **new** migration: ```bash scripts/alembic_generate_migration.sh "rename tgfp_nfl id columns to external ids" ``` Both columns are indexed and unique, so the generated migration should be reviewed to confirm the indexes are renamed rather than dropped and recreated. ### Timing - **Do it now** — keeps the TheSportsDB swap a single concern, but touches the schema when the swap itself may not need to. - **Do it with the swap** — one migration instead of two, but two concerns in flight at once. --- ## Acceptance criteria - [ ] `ESPNNflTeam.tgfp_id()` removed - [ ] `find_tgfp_nfl_standing_for_team` renamed, sole caller updated - [ ] Stale docstrings corrected (`espn_nfl.py:192`) - [ ] `CLAUDE.md` lines 135 and 187 corrected - [ ] *(Phase 2, if approved)* columns renamed via a new Alembic migration, all 6 remaining reference sites updated, indexes preserved - [ ] `pylint` still reports 10.00/10 - [ ] App boots and serves `/ping`, `/login`, and the Discord OAuth flow ## Verification After Phase 1, the only remaining matches should be the database column names (and the Alembic history that must not be touched): ```bash grep -rniE "tgfp[_-]?nfl" . \ --exclude-dir=.venv --exclude-dir=.git --exclude-dir=node_modules \ --exclude-dir=__pycache__ --exclude-dir=.pytest_cache --exclude=tgfp.dump ``` After Phase 2, that search should return **only** the two Alembic revision files listed above. ## Notes - `tgfp.dump` (repo root) contains the old column names. It is an untracked database dump artifact; no action needed. - Repository has no test suite, so verification is lint plus a manual boot and login round trip.

johnsturgeon added reference clean-up-remaining-tgfpnfl-espnnfl-refactor-vestiges-365

2026-08-12 21:33:59 +02:00

johnsturgeon self-assigned this

2026-08-12 21:34:05 +02:00

johnsturgeon closed this issue

2026-08-13 17:03:31 +02:00