Skip to content

Mark competitive leaderboard as discontinued - #91

Merged
cesaregarza merged 3 commits into
mainfrom
codex/competition-discontinued-20260910
Sep 10, 2026
Merged

cesaregarza merged 3 commits into
mainfrom
codex/competition-discontinued-20260910

Conversation

@cesaregarza

@cesaregarza cesaregarza commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Mark the competitive leaderboard as discontinued with a prominent notice across competition pages. Replace the daily-update promises in the header, leaderboard caption, and FAQ with historical-standings wording, and label stale data as an archived snapshot.

The upstream Sendou data-ingestion workflow, Run Ranked Update in sendouq_analysis, was already disabled separately. This PR changes frontend messaging; it does not remove the existing snapshot-cache or administrator refresh mechanisms. The schedule cleanup in the commit history refers to the obsolete schedule text.

Update the locked development dependency js-yaml from 3.15.1 to 3.15.2 so the required dependency audit passes without relaxing its threshold.

Validation: 106 frontend tests pass, the production build succeeds, and npm audit reports zero vulnerabilities. Desktop and mobile checks cover the notice on the leaderboard and FAQ with no horizontal overflow or browser runtime errors. Local checks used Node 24.19.0 and CI's npm 12.0.2; hosted CI uses pinned Node 24.15.0 and also runs the backend and container checks.

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown

Code Review: Mark competitive leaderboard as discontinued

Reviewed the diff against main (base 4703745, head 7d171eb). The UI changes themselves are clean and well-scoped, but the PR doesn't fully deliver on what its own commit messages claim — specifically the second commit, "fix: remove obsolete leaderboard refresh schedule," which doesn't appear to remove any schedule.

Findings

1. Backend refresh schedule is still active (src/celery_app/beat.py, ~line 49)
The celery beat entry fetch-ripple-stable-daily still runs tasks.refresh_ripple_snapshots daily at 00:30 UTC. The PR's commit message says this schedule was removed, but it's untouched — the backend will keep recomputing and publishing new ripple/competitive snapshots even though the UI now says the leaderboard is discontinued and "no longer being updated." If discontinuing the schedule was intended, this needs an actual change to beat.py (or the commit message needs correcting).

2. FAQ page contradicts the new "discontinued" messaging (src/react_app/src/components/competition/CompetitionFaq.jsx, ~line 165)
The FAQ still states rankings are "recomputed daily from finalized sendou.ink data at 00:30 UTC." A user reading the new banner ("This leaderboard has been discontinued... Rankings are no longer being updated") and then clicking through to /faq will see directly contradictory information. Worth updating this copy in the same PR for consistency.

3. "Refresh now" / admin refresh flow is still fully functional (src/react_app/src/components/competition/CompetitionLayout.jsx, ~line 244)
The refresh button and queueCompetitionSnapshotRefresh admin action remain wired up. As-is, an admin (or anyone routed through onRefresh) can still trigger a live refresh of the "archived" snapshot, which undercuts the new copy stating "No new tournament results will be added." If the leaderboard is meant to be frozen, this control should probably be disabled or removed alongside the banner.

Overall

All three issues point to the same root cause: this PR changes the copy declaring the leaderboard discontinued, but doesn't touch the mechanisms (scheduled task, refresh button, FAQ description) that the copy claims are now inactive. I'd recommend either wiring up the actual backend/UI changes to match the messaging, or scoping this PR to messaging-only and filing a follow-up for the schedule/refresh removal — as long as the commit message doesn't claim work that isn't there.

No security concerns identified. Test coverage: didn't see new tests, which seems reasonable for a copy-only change, but would be worth adding if the refresh/schedule removal happens in a follow-up.

🤖 Generated with Claude Code

@cesaregarza
cesaregarza marked this pull request as ready for review September 10, 2026 02:39
@cesaregarza
cesaregarza merged commit e0e23c4 into main Sep 10, 2026
13 checks passed
@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown

Code Review: Mark competitive leaderboard as discontinued

Reviewed the cumulative diff (3 commits) against main. This is a small, low-risk, copy/UI-only change — no backend or data-layer changes. Overall looks good; a few notes below.

Code quality

  • CompetitionApp.jsx: placing the discontinuation <aside> banner at the Router level (outside CompetitionRoutes) is the right call — it applies uniformly to /, /learn, /viz, and /faq without duplicating markup in each page component.
  • Copy changes in CompetitionFaq.jsx, CompetitionLayout.jsx, and StableLeaderboardHeader.jsx are consistent with each other and read clearly.

Potential issues

  • stale/"Archived snapshot" badge is time-based, not state-based (CompetitionLayout.jsx:217). The badge only appears when stable?.stale || danger?.stale is true, and that flag is computed server-side purely from data age vs. a 24h threshold (src/fast_api_app/routes/ripple_public.py:770-783), not from any "discontinued" flag. That happens to work today (the data is already >24h old), but it conflates two different concepts: "data hasnt refreshed recently" vs. "this leaderboard will never refresh again." If the backends staleness threshold or caching behavior ever changes, the "Archived snapshot" pill could silently disappear even though the leaderboard is still discontinued, while the header text ("Historical standings from the last published leaderboard update") and the FAQ still say otherwise — a minor inconsistency risk worth a comment or a dedicated flag, though not blocking for this copy-only PR.
  • Minor a11y nit: the new banner uses <aside aria-label="Leaderboard discontinued">. aside maps to the "complementary" landmark role, which is a reasonable choice for a persistent banner, but since this is important status information (not supplementary content), a role="status" or role="note" might be announced more reliably by assistive tech. Not a blocker, just worth considering.

Test coverage

  • No tests were added or updated for the new banner or the copy changes (e.g. CompetitionApp.loader.test.js still does not assert on the discontinuation banner text). Given this repo already has RTL-based tests for these components, a simple screen.getByText(/discontinued/i) assertion in CompetitionApp.loader.test.js would guard against the banner accidentally being removed or the text changing unnoticed. Low priority given the low risk of this change, but easy to add.

Performance / security

  • No performance or security concerns — this is static UI copy with no new data flows, external calls, or user input handling.

Minor / unrelated

  • src/react_app/package-lock.json has an incidental js-yaml bump (3.15.1 → 3.15.2), a transitive dev dependency unrelated to this change. Harmless, but if it was not intentional it is worth double-checking it came from a deliberate npm install/npm ci rather than unexpected lockfile drift.

🤖 Generated with Claude Code

This branch was successfully deployed

1 active deployment
SplatTop 0f82acc0 Deployed Sep 10, 2026 by cesaregarza via helm-dry-run #269
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant