solvreven/FPL

Watch

Fork 0

Connect or retire the unused depth-chart refusal mechanism #4

New issue

Closed

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

kristofferopsahl commented

2026-08-23 22:37:28 +02:00

Collaborator

Finding

backend/web/depth.py documents and implements a decision-critical refusal for players whose historical minutes no longer describe the current depth chart. apply_depth_refusal() mutates provenance to depth-chart-changed, and backend/web/provenance.py classifies that source as untrusted.

However, repository-wide reference analysis finds no caller of either apply_depth_refusal() or blockers_for() outside their definitions. No tests exercise the module. The mechanism therefore looks complete in isolation but cannot affect production projections—the exact disconnected-wiring failure described in CLAUDE.md.

Related clearly unused definitions found by the same pass:

backend/web/tm_parse.py:232 render_incomplete() (superseded by the structural page_is_rendered() guard)
backend/engine.py:186 project_gameweek() (no repository caller; confirm whether it is intended public API before removal)

Suggested direction

Decide whether the depth refusal is still part of the intended model.
If yes, wire it at the point where per-gameweek availability and provenance are both available, and add a prove-the-wire test that fails if the call/consumer is removed.
If no, remove the module and provenance branch so dead safeguards cannot be mistaken for active protection.
Separately confirm and remove or document the other unused definitions.

Acceptance criteria

The repository contains no decision-critical guard that has no production path.
Tests prove the caller-to-consumer connection, not only helper logic.
Remaining public utility functions have an explicit supported use or repository caller.

## Finding `backend/web/depth.py` documents and implements a decision-critical refusal for players whose historical minutes no longer describe the current depth chart. `apply_depth_refusal()` mutates provenance to `depth-chart-changed`, and `backend/web/provenance.py` classifies that source as untrusted. However, repository-wide reference analysis finds no caller of either `apply_depth_refusal()` or `blockers_for()` outside their definitions. No tests exercise the module. The mechanism therefore looks complete in isolation but cannot affect production projections—the exact disconnected-wiring failure described in `CLAUDE.md`. Related clearly unused definitions found by the same pass: - `backend/web/tm_parse.py:232` `render_incomplete()` (superseded by the structural `page_is_rendered()` guard) - `backend/engine.py:186` `project_gameweek()` (no repository caller; confirm whether it is intended public API before removal) ## Suggested direction - Decide whether the depth refusal is still part of the intended model. - If yes, wire it at the point where per-gameweek availability and provenance are both available, and add a prove-the-wire test that fails if the call/consumer is removed. - If no, remove the module and provenance branch so dead safeguards cannot be mistaken for active protection. - Separately confirm and remove or document the other unused definitions. ## Acceptance criteria - The repository contains no decision-critical guard that has no production path. - Tests prove the caller-to-consumer connection, not only helper logic. - Remaining public utility functions have an explicit supported use or repository caller.

kristofferopsahl added the

cleanup

reliability

priority: high

labels

2026-08-23 22:37:28 +02:00

kristofferopsahl commented

2026-08-30 17:46:13 +02:00

Author

Collaborator

I can implement this once the intended model behavior is decided. Please confirm:

Should the depth-chart refusal remain part of the production model, or should backend/web/depth.py and its depth-chart-changed provenance branch be removed?
If it remains, at which projection boundary should it run: before per-GW availability is applied, or after availability/provenance has been assembled?
Are engine.project_gameweek() and tm_parse.render_incomplete() supported external APIs, or may they be removed when repository callers are absent?

The first answer determines whether the safe change is wiring or deletion; inferring it risks changing projection policy.

I can implement this once the intended model behavior is decided. Please confirm: 1. Should the depth-chart refusal remain part of the production model, or should `backend/web/depth.py` and its `depth-chart-changed` provenance branch be removed? 2. If it remains, at which projection boundary should it run: before per-GW availability is applied, or after availability/provenance has been assembled? 3. Are `engine.project_gameweek()` and `tm_parse.render_incomplete()` supported external APIs, or may they be removed when repository callers are absent? The first answer determines whether the safe change is wiring or deletion; inferring it risks changing projection policy.

solvreven commented

2026-08-30 19:42:00 +02:00

Owner

Answering all three, with a measurement rather than a preference.

First, a correction to the premise this issue inherited from CLAUDE.md. That file says depth.py is unwired because "FPL's four position buckets cannot express a depth chart — Saliba is rank 2 among Arsenal defenders and Mosquera rank 7". That describes reassigning minutes. This module explicitly refuses to do that, and says so at length in its own docstring: knowing Saliba is out tells you those minutes are available, not who gets them. Its only output is a refusal plus the reason. So the stated blocker does not apply to what the code actually does.

1. Keep it — but as a BADGE, not a provenance refusal.

Measured against the live index for GW3, blockers_for fires on 11 players of 623 (1.8%):

Calafiori     Arsenal        DEF  start=0.57  blocked by: Saliba
Hincapie      Arsenal        DEF  start=0.41  blocked by: Saliba
Schar         Newcastle      DEF  start=0.15  blocked by: Burn
Livramento    Newcastle      DEF  start=0.32  blocked by: Burn
A.Murphy      Newcastle      DEF  start=0.00  blocked by: Burn
Adams         Bournemouth    MID  start=0.52  blocked by: Kroupi.Jr
Kluivert      Bournemouth    MID  start=0.47  blocked by: Kroupi.Jr
Cook          Bournemouth    MID  start=0.47  blocked by: Kroupi.Jr
Hutchinson    Nott'm Forest  MID  start=0.44  blocked by: I.Sangare
Dominguez     Nott'm Forest  MID  start=0.38  blocked by: I.Sangare
Hudson-Odoi   Nott'm Forest  MID  start=0.35  blocked by: I.Sangare

The rate is fine; the accuracy is position-dependent, and that is the finding. Calafiori and Hincapié behind Saliba are exactly the case the module was written for — three centre-backs, one out. The six MID rows are the coarse-bucket problem in the open: FPL's "MID" spans a defensive midfielder and two wingers, so one absent midfielder refuses three players whose roles it says nothing about. DEF is narrower (CB/FB) and does better, though Livramento at right-back blocked by Burn is still weak.

So roughly half the refusals would be wrong, and a refusal is not free — it drops the player from the candidate pool. Wiring it as designed buys one real signal and five false ones.

A badge does not have that asymmetry. "Measured behind Saliba, who is out" is true for every row above, including the weak ones — the reader can judge Livramento themselves — and it costs nothing when wrong. It is also this repo's own rule for prior-derived values: render the row, suppress the number, mark the cell, not remove the row.

Concretely: keep blockers_for, keep depth_blocked_by and the DEPTH-CHANGED badge, and drop the start_source = "depth-chart-changed" write. That line is what turns it into a refusal. SOURCE should then go, and the USER-override carve-out with it (a badge does not need to yield to an override; it is a statement of fact about where the number came from).

Re-open the refusal question if detailed position data ever arrives — but note Understat was probed 2026-07-31 and its position is coarse groups (Saliba D S, Mosquera D S, identical), so that is not the source.

2. Boundary: after availability, before provenance is consumed. It reads availability_by_gw for the blocker, so availability must already be assembled; and it writes a badge that the projection and the UI both read. In build_players_index, immediately after availability_by_gw is populated.

3. Both may be removed. Neither is a supported external API — there is no external consumer of this repo.

tm_parse.render_incomplete() — delete. It is a blocklist ("tm_spinner", "Loading ...") superseded by the allowlist page_is_rendered, which is what scrape_transfermarkt.py actually calls to set RENDER_INCOMPLETE. CLAUDE.md is explicit that a blocklist only ever catches yesterday's failure; leaving it beside the allowlist that replaced it invites someone to call the wrong one.
engine.project_gameweek() — delete. Zero callers across backend/, scripts/ and tests/. The live path is project_all → project_fixture. It also carries p_assist: float = 0.8 as a default, which is exactly the kind of unowned constant this repo keeps finding in dead code.

Answering all three, with a measurement rather than a preference. **First, a correction to the premise this issue inherited from CLAUDE.md.** That file says `depth.py` is unwired because "FPL's four position buckets cannot express a depth chart — Saliba is rank 2 among Arsenal defenders and Mosquera rank 7". That describes *reassigning* minutes. **This module explicitly refuses to do that**, and says so at length in its own docstring: knowing Saliba is out tells you those minutes are available, not who gets them. Its only output is a refusal plus the reason. So the stated blocker does not apply to what the code actually does. **1. Keep it — but as a BADGE, not a provenance refusal.** Measured against the live index for GW3, `blockers_for` fires on **11 players of 623 (1.8%)**: ``` Calafiori Arsenal DEF start=0.57 blocked by: Saliba Hincapie Arsenal DEF start=0.41 blocked by: Saliba Schar Newcastle DEF start=0.15 blocked by: Burn Livramento Newcastle DEF start=0.32 blocked by: Burn A.Murphy Newcastle DEF start=0.00 blocked by: Burn Adams Bournemouth MID start=0.52 blocked by: Kroupi.Jr Kluivert Bournemouth MID start=0.47 blocked by: Kroupi.Jr Cook Bournemouth MID start=0.47 blocked by: Kroupi.Jr Hutchinson Nott'm Forest MID start=0.44 blocked by: I.Sangare Dominguez Nott'm Forest MID start=0.38 blocked by: I.Sangare Hudson-Odoi Nott'm Forest MID start=0.35 blocked by: I.Sangare ``` The rate is fine; the **accuracy is position-dependent**, and that is the finding. Calafiori and Hincapié behind Saliba are exactly the case the module was written for — three centre-backs, one out. The six `MID` rows are the coarse-bucket problem in the open: FPL's "MID" spans a defensive midfielder and two wingers, so one absent midfielder refuses three players whose roles it says nothing about. `DEF` is narrower (CB/FB) and does better, though Livramento at right-back blocked by Burn is still weak. So roughly **half the refusals would be wrong**, and a refusal is not free — it drops the player from the candidate pool. Wiring it as designed buys one real signal and five false ones. A badge does not have that asymmetry. "Measured behind Saliba, who is out" is true for every row above, including the weak ones — the reader can judge Livramento themselves — and it costs nothing when wrong. It is also this repo's own rule for prior-derived values: *render the row, suppress the number, mark the cell*, not remove the row. **Concretely:** keep `blockers_for`, keep `depth_blocked_by` and the `DEPTH-CHANGED` badge, and **drop the `start_source = "depth-chart-changed"` write**. That line is what turns it into a refusal. `SOURCE` should then go, and the `USER`-override carve-out with it (a badge does not need to yield to an override; it is a statement of fact about where the number came from). Re-open the refusal question if detailed position data ever arrives — but note Understat was probed 2026-07-31 and its `position` is coarse groups (Saliba `D S`, Mosquera `D S`, identical), so that is not the source. **2. Boundary: after availability, before provenance is consumed.** It reads `availability_by_gw` for the blocker, so availability must already be assembled; and it writes a badge that the projection and the UI both read. In `build_players_index`, immediately after `availability_by_gw` is populated. **3. Both may be removed. Neither is a supported external API — there is no external consumer of this repo.** - `tm_parse.render_incomplete()` — **delete.** It is a *blocklist* ("tm_spinner", "Loading ...") superseded by the allowlist `page_is_rendered`, which is what `scrape_transfermarkt.py` actually calls to set `RENDER_INCOMPLETE`. CLAUDE.md is explicit that a blocklist only ever catches yesterday's failure; leaving it beside the allowlist that replaced it invites someone to call the wrong one. - `engine.project_gameweek()` — **delete.** Zero callers across `backend/`, `scripts/` and `tests/`. The live path is `project_all` → `project_fixture`. It also carries `p_assist: float = 0.8` as a default, which is exactly the kind of unowned constant this repo keeps finding in dead code.

solvreven referenced this issue from a commit

2026-08-30 19:43:23 +02:00

docs: correct why depth.py is unwired — the stated blocker was the wrong one

solvreven referenced this issue

2026-08-30 19:53:57 +02:00

The season stops being hardcoded, the chores run themselves, and Norsk Tipping drives the market #19

kristofferopsahl referenced this issue from a pull request that will close it,

2026-08-30 21:30:45 +02:00

Wire depth-chart changes as badges #21

kristofferopsahl closed this issue

2026-08-30 21:30:46 +02:00