Skip to content

Commit 89dd9af

Browse files
karstenskytclaude
andauthored
fix(tracking): ADR-028 RC2 + RC3 + RC5 -- three orientation corrections -- silly-kicks 4.71.0 (PR-S139, ADR-051) (#188)
PR 3 of 5. Re-materialize trigger, no forced VAEP retrain, C4 count unchanged (32). No new ADR: RC2/RC3/RC5 are all recorded in ADR-051. *** DO NOT TAG v4.71.0. *** This version is committed and traceable but never published. It corrects the serving geometry that GkCompletionModel's bundled weights were fitted against; releasing it alone introduces a train/serve skew that does not exist today. PR 4 retrains those weights and bumps to 4.72.0, which is the version that ships. RC2 -- _gk_geometry wrote frame coordinates into action-LTR quantities _tracking_gk_xy and _tracking_ball_xy sampled positions from the linked frame and returned them unreprojected, while their own sibling _tracking_gk_xy_detected had always applied the ADR-028 point reflection. The two halves failed differently, and the distinction is operational. In _tracking_gk_xy the goal-area clamp (x <= 16.5) is an own-half predicate in ACTION-LTR coords, so against a raw away-team frame x it rejects a correctly placed keeper (action-LTR x=5 IS frame x=100) and the goal kick fell through to the rule-point prior -- a systematic loss of the tracking tier, not a visibly bad coordinate. The clamp now runs strictly AFTER the reflection. _tracking_ball_xy has NO clamp, so nothing caught a mis-projected ball: restart coordinates moved by up to a full pitch length (spec 2.2 measured 101.24 m GS / 99.58 m IDSSE). The flip is computed ONCE per call and threaded into all five helper call sites, so a call emits at most one OrientationUnresolvedWarning instead of five and the redundant per-helper groupby+merge is gone. RC3 -- the space-creation OBSO multiplier was applied unrotated compute_space_created builds its multiplier from the attack-LTR transition/EPV grids and applies it to a frame-LTR pitch-control surface, which EXCHANGED the two emitted columns for away actions: max |base.created - mirrored.denied | = 4.44e-16 (swapped pair agreed) max |base.created - mirrored.created| = 1.20688 (like-for-like did not) DEVIATION FROM THE PLAN, deliberate: the plan said to reflect the finished obso_multiplier. That product also contains a ball-anchored distance_weight computed in FRAME coords which must never be mirrored -- the rule the opponent branch already documented. Reflecting the two GRIDS instead is correct, and additionally fixes the opponent multiplier for free because it is constructed as a flip of the same artifacts. Reflecting the product would have been 98.7% wrong (the ball Gaussian relocates 19.29,10.97 -> 85.71,57.03) AND would have left the opponent multiplier uncorrected. compute_space_created and _compute_space_creation_for_action gain a keyword-only attacks_rtl (default False = byte-identical for existing callers). The flag comes from the FRAMES via acting_team_attacks_rtl; home_team_id stays in both signatures and stays UNREAD (D3 retires it by disuse, not removal). space_creation_xfns delegates to add_space_creation and inherits the fix. RC5 -- the next-event destination proxy ignored team_id _next_event_start borrows the next action's start_x/start_y, guarded only on game_id/period_id. SPADL is per-ACTING-team LTR, so a cross-team next event describes the same physical point in the opposite convention. Measured: a shared point the opponent records as (45.0, 20.0) is (60.0, 48.0) in the anchor's frame -- 15 m x, 28 m y. An UNATTESTED team id never decides: ids_differ is NA-safe-both-present, so an NA on either side leaves the coordinate untouched (ADR-027: 'cannot tell' must not become 'reflect'). This is action-vs-action, not frame-vs-action, so the mirror registry is structurally blind to it; dedicated tests are the only guard. Fixed here rather than in PR 4 on ordering grounds -- PR 4 retrains, and a retrain must run against final geometry. Also fixed (carried, unrelated to any RC) - SECURITY.md advertised the long-dead 3.x support line. - TODO.md's TF-19 row carried a 424-char verbatim duplication that added an extra table cell via one unescaped pipe; it now matches its 16 peers at 6 unescaped pipes, both legitimate escapes preserved. From the pre-commit review - resolve_restart_geometry's docstring promised it was warning-free so the resolve_gk_geometry shim 'can never leak a warning onto the frozen compute_xt_gk path'. RC2 makes that false; the docstring now says so. The warning is intended -- unoriented frames mean the reprojection could not happen, which is the condition RC2 exists to make audible. - CLAUDE.md and ADR-051 claimed CI escalates OrientationUnresolvedWarning 'with an opt-out list of ZERO'. Verified FALSE (pyproject escalates SyntheticEPV / IgnoredSurfaceInputs / MissingFeatureContract only). That was intent stated as fact; both corrected. - transition_grid is read as orientation-neutral by add_obso and as attack-LTR by compute_space_created. The divergence pre-dates RC3 (the opponent branch always flipped it); now documented at both ends instead of silent. - ADR-051's absolute '14 strict xfails' became a running count (14 at PR 1, 10 after PR 3). Every new test was proven by planting a regression, not by passing: 2/3 ball tests and 1/6 cross-team tests fail when the reflection is neutered, with the same-team and home-row controls correctly still passing. Verified: 6317 passed / 0 failed / 10 xfailed; ruff check + format clean; pyright 0 errors, run BARE over the whole repo including tests/. Claude-Session: https://claude.ai/code/session_01GHLeED1UusyqEw5LMpfxHS Co-authored-by: Claude <noreply@anthropic.com>
1 parent 641dadf commit 89dd9af

15 files changed

Lines changed: 693 additions & 58 deletions

CHANGELOG.md

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,111 @@ All notable changes to silly-kicks will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [4.71.0] — NOT RELEASED (ships within 4.72.0 alongside PR 4)
9+
10+
> **Do not tag `v4.71.0`.** This version is committed and traceable but deliberately never published
11+
> to PyPI. PR 3 corrects the serving geometry that `GkCompletionModel`'s bundled weights were fitted
12+
> against; releasing it alone would introduce a train/serve skew that does not exist today. PR 4
13+
> retrains those weights and bumps to 4.72.0, which is the version that ships. See ADR-051 and
14+
> `docs/superpowers/plans/2026-07-29-adr028-orientation-defect-class.md`.
15+
16+
### Fixed — ADR-028 RC2 + RC3: two per-action orientation corrections (PR-S139, ADR-051)
17+
18+
PR 3 of 5. **Re-materialize trigger, no forced VAEP retrain.** C4 count unchanged (32). No new ADR —
19+
RC2 and RC3 are both recorded in ADR-051.
20+
21+
**RC2 — `_gk_geometry` wrote frame coordinates into action-LTR quantities.** `_tracking_gk_xy` and
22+
`_tracking_ball_xy` sampled positions from the linked frame and returned them unreprojected, while
23+
their own sibling `_tracking_gk_xy_detected` had always applied the ADR-028 point reflection. Both now
24+
reproject.
25+
26+
**The two halves failed differently, and the distinction is operational.** In `_tracking_gk_xy` the
27+
failure was a *systematic loss of the tracking tier* rather than a wild coordinate: the goal-area
28+
clamp (`gx <= 16.5`) is an own-half predicate in *action-LTR* coords, so applied to a raw away-team
29+
frame x it rejects a correctly-placed keeper (action-LTR x=5 is frame x=100) and the goal kick fell
30+
through to the rule-point fallback. The clamp is now applied strictly *after* the reflection, and the
31+
ordering is commented in place so it is not "tidied" back.
32+
33+
`_tracking_ball_xy` has **no clamp**, so there was nothing to catch a mis-projected ball: restart
34+
origins and destinations moved by up to a full pitch length — spec §2.2 measured a maximum of
35+
**101.24 m** (GS) / **99.58 m** (IDSSE). A consumer reading `enriched_start_x` saw a plausible
36+
on-pitch coordinate at the wrong end.
37+
38+
**RC3 — the space-creation OBSO multiplier was applied unrotated.** `compute_space_created` builds its
39+
multiplier from the attack-LTR `transition_grid`/`epv_grid` and applies it to a frame-LTR
40+
pitch-control surface. For an away action the two conventions are a 180° point reflection apart, and
41+
the measured consequence was that the two emitted columns were **exchanged**:
42+
43+
```
44+
max |base.created - mirrored.denied | = 4.44e-16 <- the SWAPPED pair agreed to float noise
45+
max |base.created - mirrored.created| = 1.20688 <- while like-for-like did not
46+
```
47+
48+
The fix point-reflects the two **grids** (both axes), not the finished multiplier — the multiplier
49+
also contains a ball-anchored `distance_weight` computed in frame coords, which must never be
50+
mirrored. That is the rule the opponent-perspective branch had already followed and documented.
51+
Reflecting at the grid seam also corrects the opponent multiplier for free, since it is constructed as
52+
a flip of the same artifacts.
53+
54+
`compute_space_created` and `_compute_space_creation_for_action` gain a keyword-only
55+
`attacks_rtl: bool = False`. The flip is computed once per call from the **frames**
56+
(`acting_team_attacks_rtl`) and threaded in; `home_team_id` remains in both signatures and remains
57+
unread, because it encodes team identity rather than attacking direction (ADR-051 D1). D3 retires that
58+
parameter by disuse, not by removal. `space_creation_xfns` delegates to `add_space_creation` and
59+
inherits the fix — it is not a second seam.
60+
61+
**Downstream — measured away-row change rates** (spec §2.2; one-match point estimates per provider,
62+
not corpus rates):
63+
64+
| Surface | GS 10502 | IDSSE DFL-MAT-J03WMX |
65+
|---|---|---|
66+
| `xt_gk` composite | 19.0% | 0% |
67+
| `gk_completion` | 17.4% | 0% |
68+
| `space_creation` (both columns) | 47.4%, max 0.140 m² | 60.0%, max 0.880 m² |
69+
| restart `enriched_start_x` | 2.55%, max **101.24 m** | 1.11%, max **99.58 m** |
70+
71+
The IDSSE zeroes are not an absence of the defect — that rate is governed entirely by ADR-024
72+
native-origin trust, which keeps IDSSE on the native tier where RC2's imputation ladder is never
73+
reached.
74+
75+
The changed surface is wider than the two `enriched_*` pairs: **all 8 `add_restart_coordinates`
76+
columns** move, including the `*_coord_source` / `*_coord_confidence` provenance, so a consumer
77+
filtering on confidence (as that function's own docstring example does) will see rows appear and
78+
disappear. Home rows are unaffected throughout.
79+
80+
Three strict xfail markers are deleted (13 → 10); their pre-fix magnitudes (0.125, 7.0 m, 1.207) are
81+
retained in the tolerance rationales as the signatures a regression would have to reproduce.
82+
83+
**One documented contract is amended, not silently broken.** `resolve_restart_geometry`'s docstring
84+
promised it was warning-free, so the `resolve_gk_geometry` shim "can never leak a warning onto the
85+
frozen `compute_xt_gk` path". Resolving orientation requires `acting_team_attacks_rtl`, which emits
86+
`OrientationUnresolvedWarning` when nothing resolves — so that promise is now false and the docstring
87+
says so. The warning is intended: unoriented frames mean the re-projection could not happen, which is
88+
the exact condition RC2 exists to make audible. The flag is computed **once per call** and threaded
89+
into all five helper call sites, so a call emits at most one such warning instead of five (and the
90+
redundant per-helper groupby+merge is gone).
91+
92+
**RC5 — the next-event destination proxy ignored `team_id`.** `_next_event_start` borrows the next
93+
action's `start_x`/`start_y` as a destination proxy, guarded only on `game_id`/`period_id`. SPADL is
94+
per-**acting-team** LTR, so when the next action belongs to the other team the borrowed coordinate
95+
describes the same physical point in the opposite convention — a 180° point reflection away.
96+
Measured: a shared point the opponent records as `(45.0, 20.0)` is `(60.0, 48.0)` in the anchor's own
97+
frame (15 m x, 28 m y). It now reflects on a cross-team borrow.
98+
99+
This is **action-vs-action**, not frame-vs-action, so the mirror registry is structurally blind to it
100+
and dedicated tests are the only guard. An **unattested team id never decides**`ids_differ` is
101+
NA-safe-both-present, so an NA on either side leaves the coordinate untouched rather than reflecting
102+
it (the ADR-027 rule that "cannot tell" must not become "reflect"). Found during this PR's review and
103+
fixed here rather than in PR 4 on ordering grounds: PR 4 retrains `GkCompletionModel`, and a retrain
104+
must run against final geometry.
105+
106+
### Fixed — two carried defects unrelated to any RC
107+
108+
- **`SECURITY.md`** advertised `3.x` as the supported line, stale since 4.0.0 (2026-05-30).
109+
- **`TODO.md`'s TF-19 On-Deck row** carried a 424-character span duplicated verbatim, adding one extra
110+
table cell via a single unescaped `|`. The row now matches its 16 peers at 6 unescaped pipes, with
111+
both legitimate `\|` escapes preserved.
112+
8113
## [4.70.0] — 2026-07-29
9114

10115
### Fixed — ADR-028 RC1: the cover-shadow passer was never reprojected (PR-S138, ADR-051)

0 commit comments

Comments
 (0)