Skip to content

CI: run on PRs; verify the refresh pipeline still runs and doesn't silently change any derived data #233

Description

@alhart2015

Problem

There is no CI in this repo today (no .github/workflows/), so nothing automatically verifies a PR before merge. For most code that's a gap; for the refresh pipeline it's a real risk, because a subtle change to scoring/projection/MC/SPoE math can silently shift every derived number the dashboard shows without breaking a single existing test. CLAUDE.md already codifies "Run refresh before merge" as a manual convention -- this issue is about automating both halves of that:

  1. The refresh pipeline still runs end-to-end (no crash / no missing output).
  2. A PR doesn't change any derived data unless it means to -- and when it does, the change is explicit and reviewable.

What already exists (build on this, don't start from scratch)

  • src/fantasy_baseball/web/refresh_pipeline.py -- run_full_refresh() -> RefreshRun().run() writes ~18 outputs via write_cache(CacheKey.*) (STANDINGS, PROJECTIONS, STANDINGS_BREAKDOWN, RANKINGS, ROSTER, OPP_ROSTERS, LINEUP_OPTIMAL, ROSTER_AUDIT, STASH, LEVERAGE, MONTE_CARLO, SPOE, PROBABLE_STARTERS, POSITIONS, TRANSACTIONS, TRANSACTION_ANALYZER, PENDING_MOVES, plus STREAK_SCORES). That set is the "data surface" we care about.
  • tests/test_web/test_refresh_pipeline.py + tests/test_web/_refresh_fixture.py already run the whole pipeline offline: Yahoo/FanGraphs/draft_value are mocked with canned fixture dicts and writes go to a fake_redis kv. So a deterministic, network-free full refresh is already achievable in a test.
  • Gap: that test asserts only shapes ("every expected key is written", "standings has 12 teams with these stat keys"). It does not assert the actual values, so a math change that keeps the shape sails through.

Proposed approach

Tier 1 -- stand up CI on PRs (foundational)

GitHub Actions workflow triggered on pull_request that runs the end-of-effort gates from CLAUDE.md:

  • pytest -n auto (includes the existing offline refresh integration test -> catches "pipeline broke")
  • mypy, ruff check ., ruff format --check ., vulture

This alone delivers goal #1: if a change makes run_full_refresh crash or stop writing an expected key, the existing shape test fails in CI.

Tier 2 -- value-level golden snapshot of the refresh outputs (the real ask)

Extend the existing fixture-driven refresh test into a golden/snapshot test:

  • After a full offline run_full_refresh against the committed fixture, dump each CacheKey payload to a canonical serialization (sorted keys, fixed float formatting) and compare against committed golden files under e.g. tests/test_web/refresh_golden/.
  • Any diff fails CI -> goal feat: player comparison on Players page #2. One golden file per CacheKey (readable diffs) is probably better than one giant blob.

Handling intentional data changes intelligently

The point is not to detect intent automatically -- it's to make a data change explicit, visible, and reviewed instead of silent:

  • Goldens are checked in. A PR that legitimately changes outputs regenerates them (pytest --update-refresh-golden flag or python scripts/update_refresh_golden.py) and commits the new goldens in the same PR. The golden diff then shows the reviewer exactly which numbers moved and by how much.
  • Separate the two failure modes so they read differently in CI:
    • pipeline broke (crash / missing key) -> hard fail, never expected.
    • data changed -> fails only if the goldens weren't updated in the same diff. If they were, it passes and (optionally) posts a summary of what moved.
  • Optional belt-and-suspenders: a data-change PR label (or a required checkbox in the PR template) that a reviewer must apply when goldens change, so "the numbers moved" is always a conscious sign-off, not a rubber stamp.

Determinism requirements (the hard part -- must be nailed for goldens to be stable)

Goldens are only viable if the offline refresh is byte-reproducible. Audit and pin:

  • Monte Carlo: simulation.run_monte_carlo / run_ros_monte_carlo must run under a fixed seed in the fixture (verify a seed knob exists or add one). MONTE_CARLO/SPOE are the most likely sources of nondeterministic drift.
  • Clock: effective_date / any date.today() in the pipeline must be injectable and frozen in the fixture.
  • Projection inputs: pinned by the fixture already (canned blended rows) -- keep them frozen.
  • Serialization: canonicalize (sorted keys, stable float repr) so diffs are signal, not float-format noise. Consider whether MC should be exact-match under a seed (preferred) vs numeric tolerance (invites slow drift).
  • Sweep for other nondeterminism leaking into payloads: dict/set ordering, wall-clock timestamps, UUIDs.

Open questions

  • Fixture-mode injection: the existing test monkeypatches; is that the seam we want, or should RefreshRun grow explicit dependency injection (Yahoo session/league, projection source, kv store) for a cleaner offline entry point?
  • Golden granularity: per-CacheKey files vs one snapshot.
  • CI runtime budget: a full-iteration MC on the fixture may be slow; a reduced-iteration MC config for CI is fine (goldens are for that CI config) as long as it's deterministic.
  • Scope of the "pipeline runs" guarantee: the offline test covers the compute half (frozen inputs -> outputs). The fetch half (live Yahoo OAuth + FanGraphs + MLB statsapi) can't run in CI without creds; a lighter smoke test that feeds recorded raw API payloads through the parsers would catch schema/parse breakage -- worth a follow-up, but Tier 1+2 above is the priority.

Out of scope

  • Deploy/CD automation (Render). This is PR-gating CI only.
  • Testing the live fetch layer against real Yahoo/FanGraphs/MLB endpoints.

Context

Spun out while merging #231 (issue #227). Complements the manual "Run refresh before merge" rule in CLAUDE.md by turning it into an automated gate.

Metadata

Metadata

Assignees

No one assigned

    Labels

    backlogMigrated from TODO.mdinfrastructureInfra / productionization

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions