Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions frontend/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ export function currentCategoryMeta() {
return metadata[els.category.value];
}

// True when switching between these two seasons changes the category's default
// threshold — an ongoing season (DYNAMIC_THRESHOLDS) against a finalized one
// (DEFAULT_THRESHOLDS), in either direction. Two finalized seasons share one
// static default, so the user's value stays. Reads the default each season
// carries in /api/metadata, so it follows whatever config.py resolved.
export function seasonSwitchChangesThresholdDefault(fromSeason, toSeason) {
const defaults = currentCategoryMeta().default_thresholds;
return defaults[fromSeason] !== defaults[toSeason];
}

// True for whatever season is still in progress this calendar year, false
// for every finalized/historical one — same Calendar Year comparison
// config.py's resolve_default_threshold() routes the static-vs-dynamic
Expand Down
4 changes: 2 additions & 2 deletions frontend/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ export async function updatePlayerPool() {
// reassigns .max/.value, checked in priority order below:
//
// 1. resetThresholdOnNextRange is still true — a Category switch or an
// ongoing→historical Season switch is still pending. A Category switch
// ongoing↔historical Season switch is still pending. A Category switch
// already ran resetThresholdToCategoryDefault() synchronously (a rough
// placeholder — it can only grow .max just far enough to fit the new
// default off whatever pending data was already on hand, not this
Expand Down Expand Up @@ -873,7 +873,7 @@ export function updateThresholdRange() {
resetThresholdOnNextRange = false;
} else if (Number(els.threshold.value) > max) {
// Every other Season/Position change (Position always; Season only
// when it isn't an ongoing→historical transition — main.js's season
// when it isn't an ongoing↔historical transition — main.js's season
// "change" handler already forces resetThresholdOnNextRange for that
// case via resetThresholdToCategoryDefault(), so this branch won't even
// run for it) — keep the user's value if it still fits; otherwise fall
Expand Down
29 changes: 17 additions & 12 deletions frontend/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
setPlayerPool,
setAppliedFilters,
preloadLogos,
isOngoingSeason,
seasonSwitchChangesThresholdDefault,
currentCategoryMeta,
metadata,
} from "./data.js";
Expand Down Expand Up @@ -66,8 +66,9 @@ import {

// Tracks whatever Season value was last settled (either by the user picking
// one, or a category switch's own season-select rebuild) so the season
// "change" handler below can tell an ongoing→historical transition apart
// from every other one — a plain els.season.value read inside that handler
// "change" handler below can tell a switch that changes the threshold default
// (ongoing↔historical) apart from every other one — a plain
// els.season.value read inside that handler
// only ever sees the *new* value, never what it was a moment ago. Synced
// in three places: right after the initial load, at the end of a category
// switch (which can silently rebuild/reset the season <select>), and at the
Expand Down Expand Up @@ -220,14 +221,18 @@ function attachEvents() {
});

els.season.addEventListener("change", () => {
// Ongoing → historical: the ongoing season's threshold sits on a scale
// shaped by however much of the season has actually been played, not a
// comparable number for a finalized season — carrying it over would
// either read as an oddly low cutoff or get silently clamped to an
// arbitrary pool max, never the historical default anyone actually
// configured. Historical → historical shares one static default across
// every finalized season (DEFAULT_THRESHOLDS), so there's nothing
// meaningful to snap to — leave the user's value alone.
// Ongoing ↔ historical, in either direction: the ongoing season's
// threshold sits on a scale shaped by however much of the season has
// actually been played, not a comparable number for a finalized season —
// carrying it over would either read as an oddly low cutoff or get
// silently clamped to an arbitrary pool max, never the default anyone
// actually configured. Going back to the ongoing season is the same
// problem mirrored: the historical 230/300 only got clamped back to the
// dynamic default for as long as the ongoing season's pool max sat below
// it, which stopped being true once enough weeks were played. Historical →
// historical shares one static default across every finalized season
// (DEFAULT_THRESHOLDS), so there's nothing meaningful to snap to — leave
// the user's value alone.
//
// Deliberately does NOT call resetThresholdToCategoryDefault() here the
// way the Category handler above does — only sets the flag and lets
Expand All @@ -245,7 +250,7 @@ function attachEvents() {
// worse sin than a brief stale value), but a Season switch has no such
// units mismatch, so there's nothing lost by just waiting the beat for
// real data instead of flashing a wrong guess first.
if (isOngoingSeason(previousSeasonValue) && !isOngoingSeason(els.season.value)) {
if (seasonSwitchChangesThresholdDefault(previousSeasonValue, els.season.value)) {
setResetThresholdOnNextRange(true);
}
previousSeasonValue = els.season.value;
Expand Down
12 changes: 9 additions & 3 deletions playbook/architecture/THRESHOLDS.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ derived from `value`/`max`, so reassigning `.max` visibly moves the handle even
untouched — the driving rule behind every case below is "never reassign `.max` unless `.value` is
also genuinely changing." `updatePendingThresholdMax()` checks two triggers, in priority order:

1. **`resetThresholdOnNextRange` is still `true`** — a Category switch or an ongoing→historical Season
switch is pending. A Category switch (`main.js`'s category `"change"` handler) still calls
1. **`resetThresholdOnNextRange` is still `true`** — a Category switch or an ongoing↔historical Season
switch (either direction) is pending. A Category switch (`main.js`'s category `"change"` handler) still calls
`resetThresholdToCategoryDefault()` synchronously first, for instant feedback — leaving the
outgoing category's number on screen, even briefly, is actively misleading since the two
categories' threshold fields are entirely different units (PR Opp vs Non Spike PB Snaps). That
function can only grow `.max` just far enough to fit the new default off whatever pending data
happens to already be on hand (the *outgoing* slice's, not the incoming one's real numbers) — a
rough placeholder, corrected here once real data lands. A Season switch crossing ongoing→historical
rough placeholder, corrected here once real data lands. A Season switch crossing ongoing↔historical
has no such units mismatch, so it *skips* that synchronous call entirely (only sets the flag) and
leaves both `.value` and `.max` untouched until this function runs with real fetched data. Either
way, this branch applies the real default + real max together, atomically, in one write, the moment
Expand All @@ -65,6 +65,12 @@ also genuinely changing." `updatePendingThresholdMax()` checks two triggers, in
moving, arbitrary target), instead of the number actually configured as sensible for it via
`DYNAMIC_THRESHOLDS`/`DEFAULT_THRESHOLDS`.

The "reset pending" trigger covers both directions on purpose (`seasonSwitchChangesThresholdDefault()` in
`data.js`, true when the two seasons carry different `default_thresholds`). Going from a finalized season
back to the ongoing one used to rely on trigger 2 alone: the historical 230/300 only fell back to the
dynamic default while the ongoing season's pool max stayed below it. Once enough weeks were played (2026
Week 7: ED 256, DI 244, OL 304) the old value fit again and the threshold stayed put.

Neither trigger fires for a no-op swap (e.g. one historical season to another, sharing
`DEFAULT_THRESHOLDS`' one static value) — the very first version of this function recomputed `.max`
unconditionally on *every* pending swap, which visibly slid the bar for that case even though the
Expand Down
4 changes: 2 additions & 2 deletions playbook/tests/TEST_FILTERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

- **Kind:** browser (Playwright). The union rule lives inside `render()`, so the tests call the real `render()` on synthetic rows and read the trace it hands to `Plotly.react` (wrapped for the call, then restored). A label counts as dimmed when its text colour carries `DIM_OPACITY.label` from `config.js`.
- **Needs:** Chromium; the tests skip without it. The page must have finished loading so the real metadata exists, which the fixture waits for. Its own `page` fixture.
- **Covers:** Teams alone; Players alone; Teams and Players as a union (an intersection would highlight nobody); nothing selected dims everyone; a selection never changes who is plotted; the threshold stays a hard gate even for a selected team or player.
- **Covers:** Teams alone; Players alone; Teams and Players as a union (an intersection would highlight nobody); nothing selected dims everyone; a selection never changes who is plotted; the threshold stays a hard gate even for a selected team or player. A Season switch resets the threshold to the new season's default in both directions between an ongoing and a finalized season (`seasonSwitchChangesThresholdDefault()`), and not between two finalized seasons; the Season `"change"` handler calls that rule (checked from source).
- **Not here:** Pinned Players invalidation → `TEST_PINNED_IMPACT.md`. The Players search pool and fuzzy matching have no test file yet.
- **Known gaps:** the tests depend on how `render()` builds its trace, because the rule is not a separate function. If it is ever extracted, test it directly. Dimming is read from the text colour only, not from marker or logo opacity. The Players search pool staying live (INVARIANTS 4) is not covered.
- **Known gaps:** the tests depend on how `render()` builds its trace, because the rule is not a separate function. If it is ever extracted, test it directly. Dimming is read from the text colour only, not from marker or logo opacity. The Players search pool staying live (INVARIANTS 4) is not covered. The Season switch is tested as a rule plus a source check; no test drives the real dropdown and reads the threshold box afterwards.
49 changes: 49 additions & 0 deletions tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,52 @@ def test_threshold_is_a_hard_gate_even_for_a_selected_player_or_team(page):

assert "E. Low" not in plotted
assert "E. Low" not in highlighted


# Switching between an ongoing season and a finalized one changes the default threshold; two finalized seasons share one.
SEASON_SWITCH_JS = """
async () => {
const d = await import('/data.js');
const cat = d.currentCategoryMeta();
const before = cat.default_thresholds;
cat.default_thresholds = { 2024: 230, 2025: 230, 2026: 80 };

try {
const changes = (from, to) => d.seasonSwitchChangesThresholdDefault(from, to);
return {
ongoingToHistorical: changes('2026', '2025'),
historicalToOngoing: changes('2025', '2026'),
historicalToHistorical: changes('2024', '2025'),
sameSeason: changes('2026', '2026'),
};
} finally {
cat.default_thresholds = before;
}
}
"""


def test_a_season_switch_resets_the_threshold_in_both_directions(page):
result = page.evaluate(SEASON_SWITCH_JS)

assert result["ongoingToHistorical"] is True
assert result["historicalToOngoing"] is True


def test_two_finalized_seasons_keep_the_users_threshold(page):
result = page.evaluate(SEASON_SWITCH_JS)

assert result["historicalToHistorical"] is False
assert result["sameSeason"] is False


def test_the_season_change_handler_asks_that_rule_and_not_only_the_leaving_direction():
# The handler is wired inside attachEvents(), so this is checked from source.
from pathlib import Path

source = (
Path(__file__).resolve().parent.parent / "frontend" / "main.js"
).read_text(encoding="utf-8")

assert "seasonSwitchChangesThresholdDefault(previousSeasonValue" in source
assert "isOngoingSeason(previousSeasonValue)" not in source
Loading