Skip to content

feat(stats): promote defensive metrics from raw_stats into the typed model - #29

Merged
YonatanHen merged 5 commits into
masterfrom
dev/defender-metrics
Sep 6, 2026
Merged

feat(stats): promote defensive metrics from raw_stats into the typed model#29
YonatanHen merged 5 commits into
masterfrom
dev/defender-metrics

Conversation

@YonatanHen

@YonatanHen YonatanHen commented Sep 6, 2026

Copy link
Copy Markdown
Owner

What this does

Sofascore's defensive columns were always scraped and stored in every competition entry's raw_stats — they were never promoted into the typed Stats model. Because METRIC_FIELDS is derived from Stats, they could not be sorted, filtered, or queried. This was a representation gap, not a data gap, so no re-fetch was required.

Promotes 11 counts and 2 derived rates. METRIC_FIELDS goes 39 → 52.
Counts tackles, tackles_won, interceptions, clearances, blocks, aerial_duels_won, aerial_lost, ball_recoveries, dribbled_past, errors_lead_to_goal, errors_lead_to_shot
Rates tackles_won_pct, aerial_duels_won_pct

Implements Layer 1 of docs/superpowers/specs/2026-07-11-defender-representation-design.md, which is included here (it was previously untracked).

Rates are recomputed, never averaged

aggregate_stats re-derives both rates from the summed counts. Averaging per-competition percentages is wrong, and this is pinned by a test: 90% over 10 tackles and 50% over 90 tackles average to 70%, but the true combined rate is 54%.

One mapping in domain/defensive_stats.py serves both the live fetch and the backfill, so the two paths cannot drift.

Scoring is untouched

ScoringEngine reads Stats fields explicitly and references none of the new ones, so s_final is unchanged and Mathematical_Specification.md needed no edit. Folding defence into scoring is Layer 3 of the design doc and still needs sign-off.

Migration — action required after merge

backend/scripts/DB/backfill_defensive_stats.py re-reads existing raw_stats into the new typed fields. It has been run against the local dev DB (all 1256 player_stats docs). Any other environment needs it run once, or defensive filters return empty/partial results with no error:

cd backend && .venv/Scripts/python scripts/DB/backfill_defensive_stats.py --dry-run
cd backend && .venv/Scripts/python scripts/DB/backfill_defensive_stats.py

Stop any running fetch job first — the script rewrites each doc's whole competitions array.

Review fixes included

Two /code-review passes ran; every finding was addressed.

  • Backfill skipped 21 docs. Docs whose entries had all-zero defensive columns were never written, so they carried no defensive keys at all. Mongo's $gte does not match a missing field, so those players vanished from defensive filters via get_players, while the stats_view path returned them (Python defaults them to 0). One query, two answers. Now every scanned doc is written: all 1256 match a tackles >= 0 filter, previously 1235.
  • aerial_lost had no METRIC_OPTIONS entry, so it could not be filtered from the UI. Added, plus a test comparing METRIC_OPTIONS against METRIC_FIELDS so this drift fails the suite.
  • That mirror test broke in the backend container. docker-compose mounts only ./backend, so the relative path to players.ts resolved outside the mount. It now skips when the file is absent.
  • CI could not catch the drift. The backend job is gated on a backend/** paths filter, so a PR touching only players.ts skipped it entirely. frontend/src/api/players.ts is now in that filter.
  • The backfill's --dry-run claimed writes had happened. It printed "every scanned doc was written" unconditionally. Dry-run and real runs now report separately.

Also fixed

The README documented the pre-overhaul S_final in both Core Features and the architecture diagram — stale since PR #28. Unrelated to this feature, corrected here because the new bullet references S_final.

Known limitations, deliberately not addressed

  • tackles_won_pct and aerial_duels_won_pct are the first ratio metrics in the allowlist and have no minimum-volume guard. Sorting by them puts a player with 1 tackle won from 1 (100%) above a defender at 200/300 (66.7%). The counts are unaffected. Worth deciding separately whether to add a volume threshold.
  • Nothing enforces the backfill has run. Once merged, the Rankings dropdown offers the new metrics against every environment; a DB that has not had the script run returns empty results with no error.
  • CLAUDE.md has drifted: its domain/ tree omits the new defensive_stats.py. Left for /init.

Verification

  • 168 backend tests pass (154 on master + 14 new)
  • ruff check and ruff format --check clean
  • tsc --noEmit clean
  • DB snapshot taken before the migration: backend/scripts/snapshots/pre-defender-metrics.json

🤖 Generated with Claude Code

https://claude.ai/code/session_01SKAXmoCoJ3gAu4yyD3zkCL

YonatanHen and others added 5 commits September 6, 2026 12:07
@YonatanHen @claude
chore(claude): allow pytest and ruff, scope the PR doc hook correctly
43f4f90 
Carried from dev/chatbot-agent because it is tooling, not that feature.
The PreToolUse gate and the PostToolUse doc hook both had an `if` of
Bash(gh pr create*), which does not parse as a permission rule and so
matched every Bash call. Uses the documented Bash(gh pr create *) prefix
form now, and the doc prompt re-checks tool_input.command itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SKAXmoCoJ3gAu4yyD3zkCL
@YonatanHen @claude
feat(stats): promote defensive metrics from raw_stats into the typed …
24be3dc 
…model

Sofascore's defensive columns were always scraped and stored in each
competition entry's raw_stats, but never promoted into Stats. Because
METRIC_FIELDS is derived from Stats, they could not be sorted, filtered
or queried. This is a representation gap, not a data gap: no re-fetch.

Adds 11 counts (tackles, tackles_won, interceptions, clearances, blocks,
aerial_duels_won, aerial_lost, ball_recoveries, dribbled_past,
errors_lead_to_goal, errors_lead_to_shot) and 2 rates. METRIC_FIELDS goes
from 39 to 52.

- Rates are derived from counts, never read from Sofascore's own
  percentage columns, and aggregate_stats recomputes them from the summed
  counts. Averaging them would be wrong: 90% over 10 tackles and 50% over
  90 average to 70%, but the true combined rate is 54%. A test pins this.
- One mapping in domain/defensive_stats.py serves both the live fetch and
  the backfill, so the two paths cannot drift.
- Backfill applied: 1235 of 1256 player_stats docs, 1394 entries. The 21
  untouched docs have all-zero defensive columns.
- Scores are untouched. ScoringEngine does not read these fields, so
  s_final is unchanged and the math spec needs no update. Folding defence
  into s_final is Layer 3 of the design doc and still needs sign-off.
- Replaces the hand-listed field maps in _stats_to_dict/_stats_from_dict
  with asdict/fields. Stats fields were duplicated in five places; the
  serialization pair silently dropped new fields and StatsOut silently
  ignored them. Tests now guard both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SKAXmoCoJ3gAu4yyD3zkCL
@YonatanHen @claude
docs: document the defensive metrics and correct the stale S_final fo…
342e70a 
…rmula

The Defensive Metrics bullet is the user-visible half of the previous
commit: 13 metrics reachable through METRIC_FIELDS and selectable in the
Rankings metric picker. It says plainly that scoring is untouched, so it
cannot be misread as a scoring change.

The S_final correction is unrelated to this feature and pre-existing: the
README still described the pre-overhaul formula, in both the Core
Features bullet and the architecture diagram, after PR #28 replaced it.
Fixed here because the new bullet refers to S_final and would otherwise
sit under a wrong definition of it.

Mathematical_Specification.md deliberately unchanged: ScoringEngine reads
none of the new fields, Defensive_DF is still CS x 4, and nothing in that
document became inaccurate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SKAXmoCoJ3gAu4yyD3zkCL
@YonatanHen @claude
fix: address code review — backfill coverage, restored PR gate, front…
9ed4b82 
…end mirror

Restores the PreToolUse /code-review gate. Commit 43f4f90 carried the
settings file over from dev/chatbot-agent and deleted that whole block
while its message claimed only to rescope the doc hook. Merging it would
have removed the code-review gate from master as a side effect of a
metrics PR. Both hooks now use the Bash(gh pr create:*) permission-prefix
form, matching the allow rules in the same file; Bash(gh pr create *)
would not have matched a bare `gh pr create` with no flags.

Backfill now writes every scanned doc, not only changed ones. 21 docs had
all-zero defensive columns, so they were skipped and kept no defensive
keys at all. Mongo's $gte does not match a missing field, so those
players disappeared from any defensive filter through get_players, while
the stats_view path returned them because _stats_from_dict defaults them
to 0 in Python. One query, two answers. Re-run: all 1256 docs now match
a tackles >= 0 filter, previously 1235.

aerial_lost was typed and allowlisted but had no METRIC_OPTIONS entry, so
it could not be filtered from the UI, and the spec wrongly claimed the
fields were mirrored. Added, and the defensive block moved out of the
middle of the goalkeeping group. A new test compares METRIC_OPTIONS
against METRIC_FIELDS so this drift fails the suite next time.

Also documents that fetches must be stopped before running the backfill,
which rewrites each doc's whole competitions array.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SKAXmoCoJ3gAu4yyD3zkCL
@YonatanHen @claude
fix: second review pass — container-safe test, CI filter, honest dry-run
1df1464 
- The frontend mirror test read frontend/src/api/players.ts by relative
  path. docker-compose mounts only ./backend at /app, so in the container
  that path resolved outside the mount and the test raised
  FileNotFoundError. It now skips when the file is absent, so a
  backend-only checkout or container run is unaffected.
- The backend CI job is gated on a backend/** paths filter, so a PR
  touching only players.ts skipped it and the mirror test never ran —
  exactly the drift it exists to catch. players.ts is now in that filter.
- The backfill printed "every scanned doc was written" unconditionally,
  including under --dry-run when nothing was written. That is the same
  kind of false reassurance the change was meant to remove. Dry-run and
  real runs now report separately, with their own written counter.
- The METRIC_OPTIONS extractor now asserts it parsed something, so a
  formatting change fails as "could not parse" instead of a misleading
  field mismatch, and it accepts digits in metric names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SKAXmoCoJ3gAu4yyD3zkCL
@YonatanHen
YonatanHen merged commit 4490844 into master Sep 6, 2026
5 checks passed
@YonatanHen
YonatanHen deleted the dev/defender-metrics branch September 6, 2026 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@YonatanHen