Split oversized modules and consolidate cascading CSS overrides #7

Open
opened 2026-08-23 22:37:28 +02:00 by kristofferopsahl · 3 comments
Collaborator

Finding

Several files have become change hotspots with unrelated responsibilities:

  • frontend/app.js: 4,693 lines, roughly 272 top-level globals/functions, rendering and state for every surface.
  • frontend/style.css: 2,544 lines, with repeated selectors whose later definitions silently win. .pitch appears repeatedly across the file; band and bench selectors are also duplicated.
  • backend/web/pipeline.py: 1,442 lines; build_players_index() alone spans 348 lines.
  • backend/web/main.py: 1,351 lines, combining app lifecycle, cache state, presentation shaping, simulation helpers, persistence, and routes.

CLAUDE.md already records that reading the first matching CSS rule led to incorrect conclusions because appended overrides hid the binding rule. This is therefore demonstrated maintenance risk, not a stylistic preference.

Suggested direction

  • Establish cohesive module boundaries by domain/surface while preserving current public behaviour.
  • Move shared frontend state and rendering utilities behind explicit module interfaces instead of globals.
  • Consolidate CSS so each base selector has one authoritative rule plus intentional, adjacent breakpoint overrides.
  • Break pipeline stages into named transformations with typed inputs/outputs; split API routers/services from persistence and simulation.
  • Add characterization tests before moving code so this remains a no-functionality-change refactor.

Acceptance criteria

  • No core module remains a catch-all for unrelated domains.
  • Binding CSS rules can be identified without scanning later override blocks.
  • Existing API responses, projections, and rendered behaviour remain unchanged.
## Finding Several files have become change hotspots with unrelated responsibilities: - `frontend/app.js`: 4,693 lines, roughly 272 top-level globals/functions, rendering and state for every surface. - `frontend/style.css`: 2,544 lines, with repeated selectors whose later definitions silently win. `.pitch` appears repeatedly across the file; band and bench selectors are also duplicated. - `backend/web/pipeline.py`: 1,442 lines; `build_players_index()` alone spans 348 lines. - `backend/web/main.py`: 1,351 lines, combining app lifecycle, cache state, presentation shaping, simulation helpers, persistence, and routes. `CLAUDE.md` already records that reading the first matching CSS rule led to incorrect conclusions because appended overrides hid the binding rule. This is therefore demonstrated maintenance risk, not a stylistic preference. ## Suggested direction - Establish cohesive module boundaries by domain/surface while preserving current public behaviour. - Move shared frontend state and rendering utilities behind explicit module interfaces instead of globals. - Consolidate CSS so each base selector has one authoritative rule plus intentional, adjacent breakpoint overrides. - Break pipeline stages into named transformations with typed inputs/outputs; split API routers/services from persistence and simulation. - Add characterization tests before moving code so this remains a no-functionality-change refactor. ## Acceptance criteria - No core module remains a catch-all for unrelated domains. - Binding CSS rules can be identified without scanning later override blocks. - Existing API responses, projections, and rendered behaviour remain unchanged.
Author
Collaborator

This refactor needs a narrower first boundary so it can be completed and reviewed without mixing several unrelated migrations. Which slice should land first?

  • frontend JavaScript modules,
  • CSS consolidation,
  • backend/web/pipeline.py stage extraction, or
  • FastAPI router/service separation?

Please also confirm whether ES modules and multiple browser-loaded JS files are acceptable, and whether this issue should be split into one issue per slice. The current acceptance criterion (“no core module remains a catch-all”) does not define a reviewable stopping point for a single change.

This refactor needs a narrower first boundary so it can be completed and reviewed without mixing several unrelated migrations. Which slice should land first? - frontend JavaScript modules, - CSS consolidation, - `backend/web/pipeline.py` stage extraction, or - FastAPI router/service separation? Please also confirm whether ES modules and multiple browser-loaded JS files are acceptable, and whether this issue should be split into one issue per slice. The current acceptance criterion (“no core module remains a catch-all”) does not define a reviewable stopping point for a single change.
Owner

Agreed that the current acceptance criterion has no stopping point, and the request for a narrower first boundary is right. Answering the three questions, with line counts measured just now.

frontend/app.js          5333
frontend/style.css       2682
backend/web/main.py      2092
backend/web/pipeline.py  1707

Which slice first: backend/web/main.py, and specifically the ODDS AND MARKET endpoints.

Not because it is the largest — app.js is — but because it is the only one of the four with a seam that is already load-bearing. props_load.py, nt_load.py, nt_market.py and backend/odds/ exist and are separate; what sits in main.py is the thin HTTP layer over them, plus a lot that has drifted in. Extracting /api/market, /api/matches and the props endpoints into backend/web/routes/odds.py moves ~400 lines against an interface that is already tested, and nothing else has to move with it.

Explicitly NOT the CSS. .pitch was consolidated on 2026-08-27 (nine rules to one; 15 of 28 declarations never bound) and the result is gated by tests/web/test_css_characterization.py, which renders the live stylesheet against a frozen baseline and compares every element's full computed style across four tabs and twelve widths. That gate exists because the last consolidation nearly shipped a regression invisible at 1600px: the @media (max-width: 1180px) override sat 90 lines above where the merged rule landed, same specificity, so position decided. Further CSS consolidation is high-risk and low-value while the file is already gated; if it happens it should be last, not first.

ES modules: yes, but not in this slice. index.html loads four files with ?v=NNN cache-busting, and that version is enforced by tests/web/test_asset_version.py (digest of all four assets; bumping one without the other fails). Moving to ES modules means either that guard grows to cover a module graph or it is replaced by a bundler — a separate decision with its own failure modes, and it should not ride along with a backend extraction. Splitting app.js is the second slice, not the first.

Yes, split the issue — one per slice, with this as the tracking issue. Concretely:

  1. main.pyroutes/odds.py (this one; ~400 lines, existing tests cover the behaviour)
  2. main.pyroutes/squad.py + routes/players.py
  3. pipeline.py → stage extraction (build / project / warn)
  4. app.js → ES modules, with the asset-version guard rebuilt first
  5. CSS, if still wanted

A reviewable stopping point for each: the characterisation gate must be green with no re-baseline. For the backend slices that means the full suite passes unchanged; for the CSS slice it means test_css_characterization.py passes against the existing baseline. "Re-baseline deliberately when a design change is intended; never to make it pass" is already the rule in CLAUDE.md, and it is what turns "no core module remains a catch-all" into something a reviewer can check.

Agreed that the current acceptance criterion has no stopping point, and the request for a narrower first boundary is right. Answering the three questions, with line counts measured just now. ``` frontend/app.js 5333 frontend/style.css 2682 backend/web/main.py 2092 backend/web/pipeline.py 1707 ``` **Which slice first: `backend/web/main.py`, and specifically the ODDS AND MARKET endpoints.** Not because it is the largest — `app.js` is — but because it is the only one of the four with a seam that is already load-bearing. `props_load.py`, `nt_load.py`, `nt_market.py` and `backend/odds/` exist and are separate; what sits in `main.py` is the thin HTTP layer over them, plus a lot that has drifted in. Extracting `/api/market`, `/api/matches` and the props endpoints into `backend/web/routes/odds.py` moves ~400 lines against an interface that is already tested, and nothing else has to move with it. **Explicitly NOT the CSS.** `.pitch` was consolidated on 2026-08-27 (nine rules to one; 15 of 28 declarations never bound) and the result is gated by `tests/web/test_css_characterization.py`, which renders the live stylesheet against a frozen baseline and compares every element's full computed style across four tabs and twelve widths. That gate exists because the last consolidation nearly shipped a regression invisible at 1600px: the `@media (max-width: 1180px)` override sat 90 lines above where the merged rule landed, same specificity, so position decided. Further CSS consolidation is high-risk and low-value while the file is already gated; if it happens it should be last, not first. **ES modules: yes, but not in this slice.** `index.html` loads four files with `?v=NNN` cache-busting, and that version is enforced by `tests/web/test_asset_version.py` (digest of all four assets; bumping one without the other fails). Moving to ES modules means either that guard grows to cover a module graph or it is replaced by a bundler — a separate decision with its own failure modes, and it should not ride along with a backend extraction. Splitting `app.js` is the second slice, not the first. **Yes, split the issue — one per slice**, with this as the tracking issue. Concretely: 1. `main.py` → `routes/odds.py` (this one; ~400 lines, existing tests cover the behaviour) 2. `main.py` → `routes/squad.py` + `routes/players.py` 3. `pipeline.py` → stage extraction (build / project / warn) 4. `app.js` → ES modules, with the asset-version guard rebuilt first 5. CSS, if still wanted **A reviewable stopping point for each: the characterisation gate must be green with no re-baseline.** For the backend slices that means the full suite passes unchanged; for the CSS slice it means `test_css_characterization.py` passes against the *existing* baseline. "Re-baseline deliberately when a design change is intended; never to make it pass" is already the rule in CLAUDE.md, and it is what turns "no core module remains a catch-all" into something a reviewer can check.
Author
Collaborator

Split into reviewable child issues, in order:

  1. #22 odds/market routes
  2. #23 squad/player routes
  3. #24 pipeline stages
  4. #25 frontend ES modules
  5. #26 remaining CSS consolidation

This issue remains the tracking issue. Each child has a no-rebaseline characterization gate.

Split into reviewable child issues, in order: 1. #22 odds/market routes 2. #23 squad/player routes 3. #24 pipeline stages 4. #25 frontend ES modules 5. #26 remaining CSS consolidation This issue remains the tracking issue. Each child has a no-rebaseline characterization gate.
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
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
solvreven/FPL#7
No description provided.