Public tribe-name leaderboard tab - #4746
Merged
Merged
Conversation
Adds a third "Tribes" tab to the leaderboard modal, ranking purchased bot
tribe names by rolling 30-day player reach.
GET /leaderboard/tribes is 1-based, 50 per page, capped at page 2, and
carries no total or hasMore — so fetchTribeLeaderboard() requests page 1
and only asks for page 2 when page 1 came back exactly full, then appends.
A page-2 failure keeps page 1 rather than erroring the whole tab. No pager
UI: with a two-page cap and a board that starts well under one page, it
would be ceremony.
Unlike LeaderboardClanTable there are no sortable columns. Its client-side
sort would silently sort only the loaded rows, and the server-supplied rank
is the point of the board — rows render entry.rank, not index + 1, so page
2 keeps ranks 51+.
Window bounds are plain strings, and formatWindowDate builds a Date from
the parsed parts rather than new Date(str): a date-only string parses as
UTC midnight, so the clan modal's toLocaleDateString pattern renders the
PREVIOUS day west of UTC ("Jun 26" for 2026-06-27 in America/Los_Angeles).
It also rejects rollover (2026-13-45 would silently become next February)
and returns null on anything unparseable so the caption drops instead of
showing "Invalid Date" — the same tolerance boostExpiresAt needed.
The buyer renders as a secondary line under the tribe name via
<player-name>, which already owns the username-or-publicId fallback and the
verified badge. An Owner column would push Player Reach, the metric the
board ranks by, off screen on mobile. Note that player-name ignores
nameClass unless onNameClick is also set, so the name links to the profile
modal via the same openFromLeaderboard handoff the ranked tab uses.
playerReach is impressions, not distinct people: copy says "Appeared in
games with N players", never "seen by N people".
Deploy after infra#474 — until it ships the tab renders its error state.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a tribes leaderboard API contract and paginated fetcher, a LitElement tribe table with loading and error states, and a new Tribes tab in the leaderboard modal with translations and tests. ChangesTribe leaderboard
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant LeaderboardModal
participant LeaderboardTribeTable
participant Api
Player->>LeaderboardModal: select Tribes tab
LeaderboardModal->>LeaderboardTribeTable: ensureLoaded()
LeaderboardTribeTable->>Api: fetchTribeLeaderboard()
Api-->>LeaderboardTribeTable: tribe leaderboard response
LeaderboardTribeTable-->>LeaderboardModal: render tribe rows
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/client/LeaderboardModal.test.ts (1)
68-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep API pagination logic out of this modal mock.
The modal test’s fetch stub always returns an empty page, so this duplicate pagination implementation never loads page 2. It can also drift from
TRIBE_LEADERBOARD_PAGE_SIZEand the real response validation. Return a small fixed response here and keep pagination coverage intests/client/ApiTribeLeaderboard.test.ts.Proposed simplification
- fetchTribeLeaderboard: vi.fn(async () => { - const load = async (page: number) => { - const url = new URL(`${getApiBase()}/leaderboard/tribes`); - url.searchParams.set("page", String(page)); - const res = await fetch(url.toString(), { - headers: { Accept: "application/json" }, - }); - return res.ok ? res.json() : false; - }; - const first = await load(1); - if (first === false || first.tribes.length < 50) return first; - const second = await load(2); - if (second === false) return first; - return { ...first, tribes: [...first.tribes, ...second.tribes] }; - }), + fetchTribeLeaderboard: vi.fn(async () => ({ + windowDays: 30, + start: "2026-06-27", + end: "2026-07-27", + tribes: [], + })),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/client/LeaderboardModal.test.ts` around lines 68 - 84, Replace the duplicated pagination logic inside the fetchTribeLeaderboard mock with a small fixed leaderboard response suitable for the modal test. Remove the local load function and page 2 handling; keep pagination and response-validation coverage in ApiTribeLeaderboard tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/lang/en.json`:
- Line 935: Update the tribes tooltip translation keys around
tribes_games_tooltip and the related players tooltip to use ICU pluralization
compatible with IntlMessageFormat, producing singular text when count is 1 and
plural text otherwise. Add matching pluralized keys to every other language file
that already defines these translations, preserving each locale’s existing
wording.
---
Nitpick comments:
In `@tests/client/LeaderboardModal.test.ts`:
- Around line 68-84: Replace the duplicated pagination logic inside the
fetchTribeLeaderboard mock with a small fixed leaderboard response suitable for
the modal test. Remove the local load function and page 2 handling; keep
pagination and response-validation coverage in ApiTribeLeaderboard tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a4c69b68-46af-465b-bc41-ecdd423621b6
📒 Files selected for processing (9)
resources/lang/en.jsonsrc/client/Api.tssrc/client/LeaderboardModal.tssrc/client/components/leaderboard/LeaderboardTribeTable.tssrc/core/ApiSchemas.tstests/ApiSchemas.test.tstests/client/ApiTribeLeaderboard.test.tstests/client/LeaderboardModal.test.tstests/client/LeaderboardTribeTable.test.ts
| "ratio": "Ratio", | ||
| "refresh_time": "Refreshed every 1 hour", | ||
| "title": "Leaderboard", | ||
| "tribes_games_tooltip": "Featured in {count} games", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate files =="
git ls-files | rg '(^|/)resources/lang/en\.json$|translateText|ICU|plural|intl-messageformat' || true
echo
echo "== context around reported keys =="
if [ -f resources/lang/en.json ]; then
nl -ba resources/lang/en.json | sed -n '925,945p'
fi
echo
echo "== usages of reported keys =="
rg -n '"tribes_games_tooltip"|"tribes_reach_value_tooltip"|tribes_games_tooltip|tribes_reach_value_tooltip' . -S || true
echo
echo "== search translation implementation snippets =="
rg -n "translateText|messageformat|Intl|plural|{.*count" . -S --glob '!node_modules' --glob '!dist' --glob '!build' | head -200 || trueRepository: openfrontio/OpenFrontIO
Length of output: 280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "-- language file keys around reported ranges --"
awk 'NR>=925 && NR<=945 { printf "%6d %s\n", NR, $0 }' resources/lang/en.json
echo
echo "-- usages of reported keys --"
grep -RInE 'tribes_games_tooltip|tribes_reach_value_tooltip' . || true
echo
echo "-- translation implementation snippets --"
grep -RInE 'function translateText|const .*(translate|translation)|Intl\.|messageformat|plural|{.*count|interpolat' . \
--exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build \
| head -200 || true
echo
echo "-- deterministic probe for simple interpolations without plural handler --"
python3 - <<'PY'
text = "Featured in {count} games"
for count in [1, 2, 10]:
print(count, text.replace("{count}", str(count)))
PYRepository: openfrontio/OpenFrontIO
Length of output: 26730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "-- Translator/Utils locations --"
grep -RInE 'export .*translateText|function translateText|new IntlMessageFormat|formatValue|IntlMessageFormat' src tests --include='*.ts' --include='*.tsx' | head -80 || true
echo
echo "-- translation system test around plural behavior --"
awk 'NR>=1 && NR<=220 { printf "%6d %s\n", NR, $0 }' tests/client/LeaderboardModal.test.ts 2>/dev/null || true
awk 'NR>=1 && NR<=220 { printf "%6d %s\n", NR, $0 }' tests/Translationsystem.test.ts 2>/dev/null || true
echo
echo "-- package intl-messageformat dependency and availability check --"
if [ -f package.json ]; then jq '.dependencies, .devDependencies' package.json | grep -E '(intl-messageformat|`@formatjs/icu-messageformat-parser`)' -C1 || true; fi
if [ -f package-lock.json ]; then jq '.packages["node_modules/intl-messageformat"].version, .packages["node_modules/@formatjs/icu-messageformat-parser"].version' package-lock.json || true; fi
node - <<'JS'
try {
const intl = require('intl-messageformat');
console.log('intl-messageformat loaded', intl.IntlMessageFormat.version || '(no version)');
const formatter = new intl.IntlMessageFormat('{count, plural, one {# game} other {# games}}');
for (const n of [1, 2, 10]) console.log(n, formatter.format({ count: n }));
} catch (error) {
console.log('intl-messageformat unavailable:', error.message);
}
JSRepository: openfrontio/OpenFrontIO
Length of output: 10092
Use plurals for the tribe tooltip counts.
count === 1 currently renders as “1 games” / “1 players”. Use ICU pluralization with IntlMessageFormat, and add the same plural keys to the other language files that already use plural forms.
Also applies to: 940
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/lang/en.json` at line 935, Update the tribes tooltip translation
keys around tribes_games_tooltip and the related players tooltip to use ICU
pluralization compatible with IntlMessageFormat, producing singular text when
count is 1 and plural text otherwise. Add matching pluralized keys to every
other language file that already defines these translations, preserving each
locale’s existing wording.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Drop the "one player who saw the same name in three games counts three times" sentence; the tooltip now just states what the number is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a third Tribes tab to the leaderboard modal, ranking purchased bot tribe names by rolling 30-day player reach. Implements the infra handoff for
GET /leaderboard/tribes.Important
Merge/deploy after infra#474. Until that endpoint ships, the tab renders its error state (verified — it's contained to the tab and doesn't break the modal).
Separately, the handoff is explicit that the board was not meant to launch alongside names — soft-launch names, let inventory build, then launch as a "Season 1" moment. The tab is currently unconditional; say the word and I'll gate it.
What's here
src/core/ApiSchemas.tsTribeLeaderboardResponseSchemasrc/client/Api.tsfetchTribeLeaderboard()src/client/components/leaderboard/LeaderboardTribeTable.ts<leaderboard-tribe-table>src/client/LeaderboardModal.tsTAB_KEYSresources/lang/en.jsonleaderboard_modal.*keysDesign notes
Paging. The response has no
totalorhasMore, sofetchTribeLeaderboard()requests page 1 and only asks for page 2 when page 1 came back exactly 50 long, then appends. A page-2 failure keeps page 1 rather than erroring the tab. No pager UI — with a two-page cap, and a board that starts well under one page, it'd be ceremony.No sortable columns, unlike
LeaderboardClanTable. Its client-side sort would silently sort only the loaded rows, and the server-suppliedrankis the point of the board. Rows renderentry.rank, notindex + 1, so page 2 keeps ranks 51+.Dates don't go through
new Date(str). A date-only string parses as UTC midnight, so the clan modal'snew Date(x).toLocaleDateString()pattern renders the previous day west of UTC. Confirmed by patching the naive version back in — it fails underTZ=America/Los_Angeleswithexpected 'Jun 26' to be 'Jun 27'.formatWindowDatebuilds from the parsed parts, rejects rollover (2026-13-45would silently become next February), and returns null on anything unreadable so the caption drops instead of showing "Invalid Date" — the same toleranceboostExpiresAtneeded, which is also why the window bounds are plainz.string().Owner renders as a secondary line under the tribe name via
<player-name>, which already owns theusername ?? publicIdfallback and the verified badge. An Owner column would push Player Reach — the metric the board ranks by — off screen on mobile. Noteplayer-nameignoresnameClassunlessonNameClickis also set (the no-click branch renders a<copy-button>chip), so the name links to the profile modal via the sameopenFromLeaderboardhandoff the ranked tab uses.Wording is a product rule:
playerReachis impressions, not distinct people — one player who saw a name in three games counts three times. Copy says "Appeared in games with N players" / "Featured in N games", never "seen by N people". Encoded in the tooltips and in comments on the schema.Verification
Full suite (2395 + 287) passing,
tsc/eslint/prettierclean.Driven in the running app against stubbed routes:
pages requested: ["1","2"], 62 rows from 50 + 12Rolling 30-day window (Jun 27 – Jul 27)nullusername falling back to the public id, bare-name claim showing the verified badge, and a long name truncating rather than wrappingColumn widths are tuned so Player Reach stays on screen at mobile widths rather than behind the horizontal scroll the clans tab has.
🤖 Generated with Claude Code