feat: UI redesign, player journey timeline, and live clan indicators - #2
Open
ionutcnu wants to merge 2 commits into
Open
feat: UI redesign, player journey timeline, and live clan indicators#2ionutcnu wants to merge 2 commits into
ionutcnu wants to merge 2 commits into
Conversation
Complete UI overhaul with WoT theme, animated landing page, admin and monitoring layouts. Add player journey timeline with Tomato.gg API, live current clan lookup in battle report rows and tooltips via WG API.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
wot-clan-watcher | 453227f | Feb 22 2026, 11:24 PM |
📝 WalkthroughSummary by CodeRabbit
WalkthroughExports and config simplified for Cloudflare, new dependencies (exceljs, esbuild), numerous UI additions (radar, smoke, spark, ticker, player journey), multiple API endpoints for player/clan data and stats, extended Wargaming API client and storage queries, enriched metadata, and assorted styling/animation updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PlayerHistory as /api/player-history
participant TomatoGG as Tomato.gg
participant WGAPI as Wargaming API
participant DB as Database
Client->>PlayerHistory: GET ?playerId=123
PlayerHistory->>WGAPI: getPlayerCurrentClan(123)
PlayerHistory->>TomatoGG: fetch history
TomatoGG-->>PlayerHistory: tomatoHistory
alt tomato history available
PlayerHistory->>Client: ok({ history, source: 'tomato', currentClan })
else wg memberhistory available
PlayerHistory->>WGAPI: getMemberHistory(123)
WGAPI-->>PlayerHistory: wgHistory
PlayerHistory->>Client: ok({ history, source: 'wg', currentClan })
else fallback to DB
PlayerHistory->>DB: getPlayerHistory(123)
DB-->>PlayerHistory: dbHistory
PlayerHistory->>Client: ok({ history, source: 'db', currentClan })
end
sequenceDiagram
participant ManualResults as ManualCheckResults
participant PlayerCurrentClans as /api/player-current-clans
participant ClanEmblem as /api/clan-emblem
participant BattleReport as BattleReport component
ManualResults->>PlayerCurrentClans: fetch current clans for accountIds
PlayerCurrentClans-->>ManualResults: map(accountId→clanTag)
ManualResults->>ClanEmblem: fetch emblems for clanIds
ClanEmblem-->>ManualResults: map(clanId→emblemUrl)
ManualResults->>BattleReport: render with currentClans + playerSources + emblems
BattleReport-->>ManualResults: UI with destination/source indicators
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 46
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/components/home/clan-search-panel.tsx (1)
70-89:⚠️ Potential issue | 🟡 MinorSelected state is visually communicated but invisible to screen readers.
The selected item only signals its state via CSS class changes. With
role="button", screen readers have no way to know an item is currently selected. Addaria-pressedto bridge the gap.♿ Proposed fix
<motion.div key={clan.clan_id} role="button" tabIndex={0} + aria-pressed={selectedClan?.clan_id === clan.clan_id} onClick={() => setSelectedClan(clan)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/home/clan-search-panel.tsx` around lines 70 - 89, The clickable clan item uses visual styles only and lacks an accessible state for screen readers; update the motion.div (the interactive element that calls setSelectedClan and compares selectedClan?.clan_id to clan.clan_id) to include an appropriate ARIA state such as aria-pressed={selectedClan?.clan_id === clan.clan_id} (or aria-selected if you prefer list semantics) and ensure the onKeyDown/onClick handlers remain unchanged so assistive tech receives the selected-state change.src/hooks/use-clan-history.ts (1)
110-131:⚠️ Potential issue | 🟡 MinorIncomplete time-format migration —
join_clan/leave_clanstill usestoLocaleString().Line 100 (role_change) was updated to
toLocaleTimeString('en-GB', ...)but line 121 was not. This meansClanHistoryEvent.timeholds a full date-time string (e.g."2/22/2026, 15:45:00") for join/leave events while role-change events store"15:45", producing inconsistent UI rendering.🐛 Proposed fix
- time: new Date(timestamp * 1000).toLocaleString(), + time: new Date(timestamp * 1000).toLocaleTimeString('en-GB', { hour: '2-digit', minute: '2-digit' }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-clan-history.ts` around lines 110 - 131, join_clan/leave_clan events are still using new Date(timestamp * 1000).toLocaleString() causing ClanHistoryEvent.time to be a full datetime while role_change uses toLocaleTimeString('en-GB', ...); update the events.push creation for join_clan/leave_clan (the block handling item.subtype === 'join_clan' || 'leave_clan') to set time using new Date(timestamp * 1000).toLocaleTimeString('en-GB', { hour: '2-digit', minute: '2-digit' }) so the time format matches the role_change events and UI rendering is consistent.src/components/ui/card.tsx (1)
63-70:⚠️ Potential issue | 🟡 MinorCardTitle element changed from
div→h3— breaking change, but safe in current codebase.All existing usages are compatible with the h3 element; no nested heading violations found. Consumers directly nesting CardTitle inside other headings would produce invalid HTML, but this pattern doesn't exist in the current codebase. This remains a semantic breaking change for external consumers or future usage patterns, but poses no immediate risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/card.tsx` around lines 63 - 70, CardTitle was changed from a div to an h3 which is a semantic/breaking change for consumers; either revert CardTitle back to the original div element to preserve backward compatibility (restore previous behavior in the CardTitle component) or keep the h3 but treat it as a breaking change: add a clear migration note in the component docs/CHANGELOG and bump the package major version, and update any related stories/tests that assert element type for CardTitle so consumers are aware of the change.src/app/layout.tsx (1)
98-101:⚠️ Potential issue | 🟠 MajorMissing Subresource Integrity (SRI) on CDN script.
Loading
particles.jsfrom jsdelivr without anintegrityattribute. If the CDN is compromised, arbitrary JS runs on your domain. Add an SRI hash:🛡️ Proposed fix
<Script src="https://cdn.jsdelivr.net/npm/particles.js@2.0.0/particles.min.js" strategy="beforeInteractive" + integrity="sha256-..." // generate via: shasum -b -a 256 particles.min.js | xxd -r -p | base64 + crossOrigin="anonymous" />Generate the hash with:
curl -s https://cdn.jsdelivr.net/npm/particles.js@2.0.0/particles.min.js | openssl dgst -sha384 -binary | openssl base64 -A🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/layout.tsx` around lines 98 - 101, The Script tag loading particles.js in layout.tsx lacks Subresource Integrity; update the <Script ... /> usage (the Script component that imports "https://cdn.jsdelivr.net/npm/particles.js@2.0.0/particles.min.js") to include an integrity attribute with the SHA-384 SRI hash you generate and add crossorigin="anonymous" so the browser can verify the file (i.e., add integrity="sha384-<your-generated-base64-hash>" and crossorigin="anonymous" to that Script element).src/components/ui/player-card-tooltip.tsx (1)
30-75:⚠️ Potential issue | 🟠 MajorStale data if
accountIdprop changes without remount.
fetchedis never reset whenaccountIdchanges. If the parent reuses this component instance with a different player,fetchDatareturns immediately at line 31, showing the previous player's stats/clan.Proposed fix — reset state on accountId change
Add an effect to reset when
accountIdchanges:+ import { useEffect } from 'react'; ... const [fetched, setFetched] = useState(false); + + useEffect(() => { + setStats(null); + setLiveClan(undefined); + setFetched(false); + }, [accountId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/player-card-tooltip.tsx` around lines 30 - 75, When accountId changes the component never resets fetched, causing fetchData to return early and show stale stats/clan; add a useEffect that watches accountId and resets local state (call setFetched(false), setStats(null), setLiveClan(undefined) and optionally setLoading(false)), then optionally rehydrate from statsCache/clanCache (use statsCache.has(accountId) / clanCache.get(accountId) to setStats/setLiveClan immediately) so fetchData can run for the new accountId; reference the existing fetchData, fetched, accountId, statsCache, clanCache, setFetched, setStats, and setLiveClan identifiers when implementing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@open-next.config.ts`:
- Around line 3-7: The config uses placeholder 'dummy' bindings which disable
Incremental Static Regeneration and tag revalidation; update the
defineCloudflareConfig call to replace incrementalCache and tagCache with real
Cloudflare-backed implementations (for example use r2IncrementalCache from
`@opennextjs/cloudflare` for incrementalCache and a KV-backed tag cache) and set
queue to a real Durable Queue binding so ISR/tag revalidation and background
jobs work in production; locate defineCloudflareConfig and change the
incrementalCache, tagCache, and queue entries accordingly, importing
r2IncrementalCache from `@opennextjs/cloudflare` and wiring the proper binding
names.
In `@public/icons/site.webmanifest`:
- Around line 1-20: Add a "start_url" property to the manifest to point to the
app root (e.g., "/") so the PWA always launches at the intended entry point, and
update each item under "icons" (the entries referencing
"android-chrome-192x192.png" and "android-chrome-512x512.png") to include a
"purpose": "any maskable" field so Android Chrome can use adaptive/maskable
icons; ensure both icon objects include the new "purpose" key and the top-level
JSON includes the new "start_url".
In `@src/app/api/clan-history/route.ts`:
- Line 74: The current time formatting uses new Date(timestamp *
1000).toLocaleTimeString('en-GB', ...) which can vary by runtime ICU support;
replace it with a deterministic formatter that builds an "HH:MM" string from the
Date object instead (use Date.getUTCHours()/getHours() depending on existing
timezone intent and padStart to ensure two digits) in the same place where the
code creates the time field (the expression using new Date(timestamp * 1000)
that sets time). This ensures consistent output across runtimes without relying
on toLocaleTimeString.
In `@src/app/api/player-current-clans/route.ts`:
- Line 12: The accountIds array built from raw currently retains duplicates and
forwards them to the WG API; modify the parsing step that creates the accountIds
constant (the code using raw.split(...).map(...).filter(...)) to deduplicate
entries while preserving order (e.g., use a Set or Array.from(new Set(...)) on
the parsed numeric IDs or filter by first index) so only unique, positive
integers are sent to the WG API.
In `@src/app/api/player-history/route.ts`:
- Around line 16-18: The unofficial Tomato.gg fetch (the fetch call that assigns
to res) can break silently; update the route handler so any non-OK response or
thrown error from the
`fetch("https://api.tomato.gg/api/player/clan-history-unofficial/EU/${accountId}")`
call is logged and reported before falling back to Tier 2/3: catch fetch errors
and check res.ok, then log a clear warning including accountId, HTTP status, and
error message/stack, and emit a monitoring event/metric (e.g., via your existing
metrics/sentry helper) indicating a Tier‑1 fallback so the breakage is visible.
- Around line 31-35: The code appends 'Z' to since/until before parsing which
breaks when tomato.gg already returns timezone-suffixed ISO strings; update the
parsing in route.ts (where joinTs and leaveTs are computed and events are
pushed) to either (a) avoid appending 'Z' and use new Date(since)/new
Date(until) directly, or (b) only append 'Z' when the string lacks any timezone
indicator (e.g., no trailing 'Z' and no ±HH:MM); ensure joinTs and leaveTs
remain numeric (use getTime()/1000) and guard against NaN before pushing events.
In `@src/app/globals.css`:
- Around line 247-263: Rename the CSS keyframes to kebab-case to satisfy
stylelint: change smokeLayer1, smokeLayer2, smokeLayer3 (and radarSpin,
tickerScroll, blipFade) to smoke-layer-1, smoke-layer-2, smoke-layer-3 (and
radar-spin, ticker-scroll, blip-fade) in globals.css; then update all references
to these identifiers in components that use inline styles or classNames (e.g.,
manual-check-progress.tsx, live-ticker.tsx) so they point to the new kebab-case
names; run the provided ripgrep command to find and verify all JS/TS/TSX/CSS
usages and adjust any animation names passed as strings or in styled components
accordingly.
- Around line 226-245: The `@keyframes` dust uses an invalid random() call in the
100% keyframe (translateX(calc((random() - 0.5) * 100px))) causing browsers to
drop the horizontal drift; replace that expression with a CSS custom property
(e.g., translateX(var(--dust-drift))) in the `@keyframes` dust and keep
.animate-dust as the animation class, then set --dust-drift per dust element
from JS/TSX (for example in the component that renders the dust elements) using
a runtime Math.random-derived value formatted as pixels so each element gets its
own horizontal offset.
In `@src/app/layout.tsx`:
- Around line 19-22: The APP_URL constant currently falls back to
'https://clanspy.win' which makes metadataBase use production URLs when
NEXT_PUBLIC_APP_URL is unset; update the APP_URL assignment to choose an
environment-appropriate default (e.g., use 'http://localhost:3000' when
process.env.NODE_ENV !== 'production' and only default to the production host in
production), and add a warning log when NEXT_PUBLIC_APP_URL is missing to alert
developers; modify the APP_URL constant and ensure the exported metadata object
(metadata.metadataBase) uses the new APP_URL logic and include the log call near
the APP_URL initialization.
In `@src/app/page.tsx`:
- Line 136: The inline style on the JSX div is redundant with the Tailwind
classes; remove the style prop (style={{ position: 'relative', zIndex: 10 }})
from the element that already has className="container mx-auto px-4 py-8
max-w-7xl relative z-10" so positioning and z-index are only controlled by
Tailwind classes and the markup is not duplicated.
- Around line 53-63: The effect that syncs clanSearch to the URL is write-only
because clanSearch is initialized empty; fix by reading the initial ?q param
when initializing the state that holds clanSearch (so the component's initial
value comes from new URLSearchParams(window.location.search).get('q') or
similar) and then keep the existing useEffect to update the URL on changes;
update references to the state name (clanSearch) and the updater to ensure the
value is used, and remove the redundant "typeof window === 'undefined'" guard
inside the useEffect since this is a 'use client' component and window is
available on mount.
In `@src/components/home/clan-search-panel.tsx`:
- Around line 93-104: The action buttons are hidden when searchResults becomes
empty even though selectedClan may still be set; update the rendering so the
buttons are shown based on selectedClan (not searchResults.length). Move or
re-render the <div className="flex gap-2 pt-2"> containing the Button elements
(which call onScan and onHistory and use loading and historyLoading) outside of
the block gated by searchResults.length > 0, or change its conditional to render
when selectedClan is truthy so users can always trigger onScan/onHistory for the
current selectedClan.
In `@src/components/home/features-section.tsx`:
- Around line 8-27: The features array is a static constant and should be moved
out of the component to avoid re-creating it on every render: lift the const
features (including its objects and references to Target, ScrollText, BarChart3
and the comingSoon flags) to module scope above the component that renders them,
keep any existing imports for the icons, and leave the component to simply
reference the top-level features variable (no other logic changes needed).
In `@src/components/home/recent-changes-panel.tsx`:
- Around line 56-58: The list item in the RecentChangesPanel uses an unstable
index key (key={index}) for displayChanges, which can cause incorrect DOM reuse
when the array is filtered or reordered; update the key on the motion.li to a
stable composite using the change object (e.g., combine change.account_id and
change.timestamp) so each list item has a unique, consistent key (use the
displayChanges.map callback's change inside the RecentChangesPanel component to
form the composite key).
- Around line 82-91: The tomato.gg link in recent-changes-panel.tsx places the
region "EU" before the player segment; update the anchor href so the region is a
trailing path segment after the encoded player identifier — construct the URL
using change.player.account_name and change.player.account_id as the middle
segment (encoded), followed by "/EU" (e.g.,
https://tomato.gg/stats/{encodedName}={id}/EU) so it matches the format used in
battle-report.tsx and manual-check-results.tsx; modify the href expression in
the anchor that uses change.player.account_name and change.player.account_id
accordingly.
In `@src/components/layout/header.tsx`:
- Around line 56-61: The catch block on the signup check fetch in the useEffect
(the fetch('/api/signup-check') chain that calls setSignupEnabled) is swallowing
errors; update the catch to log the error (e.g., console.warn or processLogger)
including the error object/message and context ("signup-check fetch failed") and
then still call setSignupEnabled(false) so failures are visible in dev while
preserving existing behavior.
- Around line 173-176: The Bell icon in the header (component file header.tsx,
element Bell next to displayName) is given an aria-label but isn't interactive;
either make it keyboard-focusable/actionable by wrapping the Bell in a <button>
and move the aria-label to that button (and handle onClick/aria-pressed or
tooltip as needed), or if it's purely decorative set aria-hidden="true" on the
Bell and remove the aria-label; update CSS/classes on the new button (or Bell)
to preserve styling and ensure keyboard focus styles are present.
- Around line 73-80: Remove the redundant userName variable and use displayName
when deriving initials: delete the userName declaration and call
getInitials(displayName) to compute initials; keep displayName
(session?.user?.name || session?.user?.email || '') and initials, and ensure any
other references to userName are updated to use displayName so the fallback
logic remains consistent.
In `@src/components/monitoring/battle-report.tsx`:
- Around line 152-153: The PlayerCardTooltip for "left" players is currently
passed player.destinationTag which causes the tooltip to briefly show the
destination clan while liveClan is undefined; update the props to pass the
monitored clan tag (the variable/prop used elsewhere for the left-column
context, e.g., clanTag or the component/state representing the monitored clan)
instead of player.destinationTag so the tooltip header reflects the clan they
left and matches the joined-side behavior; adjust the PlayerCardTooltip
invocation (accountId, accountName, clanTag) accordingly.
In `@src/components/monitoring/manual-check-progress.tsx`:
- Around line 22-24: Clamp the incoming percent to the 0–100 range inside
ManualCheckProgress and use that clamped value for both computing filledSegs and
for the displayed percentage; specifically create a clampedPercent =
Math.min(100, Math.max(0, percent)) and replace uses of percent when calculating
filledSegs and when rendering Math.round(percent) so the UI never shows values
outside 0–100 while still using the same prop name percent elsewhere.
In `@src/components/monitoring/manual-check-results.tsx`:
- Around line 36-44: The playerSources map is being rebuilt on every render;
wrap its computation in React's useMemo so it's only recalculated when
results.results changes. Replace the top-level const playerSources assignment
with a useMemo that returns the same Record based on iterating results.results
(referencing playerSources and results.results in manual-check-results.tsx),
e.g., useMemo(() => { ...build map... }, [results.results]); ensure you import
useMemo from React and keep the same shape { tag, name } for each player key.
- Around line 23-32: The effect using useEffect depends on results.results which
is an unstable array reference and causes repeated fetches; fix it by deriving a
stable dependency key (e.g., compute a deduplicated list of leaver IDs from
results.results using a Set, sort them and join into a string like leaverIdsKey)
and use that key in the dependency array instead of results.results; inside the
effect compute allLeaverIds from results.results (deduplicated), bail out if
empty, then call fetch(...) and setCurrentClans with the response as before.
In `@src/components/monitoring/monitored-clans-table.tsx`:
- Around line 117-122: The inline style on the avatar div (the element with
className "w-8 h-8 rounded shrink-0 flex items-center justify-center text-[10px]
font-bold" that renders {clan.tag.slice(0, 2)}) hardcodes colors `#1c1812`,
`#3a3020`, and `#CC8800`; replace these inline values with theme-driven tokens
instead (either Tailwind utility classes or CSS variables referenced from your
Tailwind config/theme). Update the JSX to use a class (e.g., bg-[theme-bg-clan],
border-[theme-border-clan], text-[theme-accent-clan] or CSS vars like
var(--clan-bg)) and add corresponding tokens in tailwind.config.js or your
global CSS variables so the colors follow the app theme.
- Around line 34-41: The effect currently depends on the clans array reference
and will refire whenever the parent passes a new array instance; stabilize the
dependency by deriving a stable key from clan IDs (e.g., compute an ids string
or sorted id array) and use that as the dependency instead of clans: inside the
component compute a stable idsKey from clans (using map+sort+join or useMemo)
and then in the useEffect use idsKey (and guard against an empty idsKey) to call
fetch and setEmblems; update references to clans -> idsKey in the useEffect and
keep setEmblems/fetch logic unchanged.
In `@src/components/ui/live-ticker.tsx`:
- Around line 75-84: The useEffect that calls fetch('/api/changes?days=7')
should be updated to use an AbortController and to stop silently swallowing
errors: create an AbortController inside the effect, pass controller.signal to
fetch, check for aborted errors in the catch, and call setChanges only if the
request wasn't aborted; return a cleanup function that calls controller.abort()
to cancel on unmount; also replace the empty catch with proper handling (e.g.,
log the error via console.error or a provided logger) so failures are visible.
In `@src/components/ui/modern-background.tsx`:
- Around line 14-28: The absolute gradient overlay div (the element with
className "absolute inset-0" in modern-background.tsx) is missing
pointer-events-none and aria-hidden attributes; update that div to add className
or utility to include pointer-events-none so it doesn't capture pointer events,
and add aria-hidden="true" to mark it as decorative for assistive technologies
(keep existing backgroundImage style unchanged).
- Around line 40-57: The noise/grain overlay div (comment "Noise/Grain overlay")
and the vignette div (comment "Vignette") are decorative but not hidden from the
accessibility tree; add aria-hidden="true" to both elements in the
ModernBackground component (the two divs with className "absolute inset-0 ...")
so assistive tech ignores them, preserving existing props (zIndex,
backgroundImage, background) and className values.
In `@src/components/ui/player-journey-timeline.tsx`:
- Around line 54-77: In buildStays, the findIndex used to locate a matching
leave pairs any later leave regardless of clan, causing mis-pairing; change the
predicate in events.findIndex((e, idx) => idx > i && e.type === 'leave') to also
require e.clan_tag === ev.clan_tag so leaveIdx/leaveEv is the next leave for the
same clan_tag; keep the subsequent logic that computes durationMs, durationDays,
isCurrent, and advancing i (i = leaveEv ? leaveIdx + 1 : events.length) the same
so stays remain consistent.
- Around line 262-271: The tomato.gg link in the PlayerJourneyTimeline component
builds the URL with "EU" as a prefix; change the href construction so the region
suffix comes after the player identifier. Locate the anchor that currently uses
href={`https://tomato.gg/stats/EU/${encodeURIComponent(player.name)}=${player.id}`}
and replace it with the correct format, e.g.
href={`https://tomato.gg/stats/${encodeURIComponent(player.name)}=${player.id}/EU`},
keeping the same encodeURIComponent(player.name) and player.id usage and
preserving target/rel/className/style and the ExternalLink element.
- Around line 253-255: Replace the raw <img> for the clan emblem with Next.js's
Image component: add "import Image from 'next/image';" at the top, then in the
JSX replace the <img src={currentClan.emblemUrl} ... /> usage with <Image
src={currentClan.emblemUrl} alt={`[${currentClan.tag}]`} width={32} height={32}
className="w-8 h-8" /> (or appropriate width/height matching w-8/h-8); if the
emblem domain is not configured in next.config.js remotePatterns, add the prop
unoptimized to Image to avoid build errors. Ensure you reference
currentClan?.emblemUrl and keep the conditional rendering around currentClan to
avoid null refs.
- Around line 220-238: The modal container motion.div with key="panel" is
missing dialog semantics and focus trapping; add role="dialog",
aria-modal="true" and aria-labelledby pointing to the h2's id (add
id="journey-title" to the existing <h2>), move focus into the modal when setOpen
becomes true and restore focus on close, and implement a focus trap (either
integrate focus-trap-react around the panel or add a useEffect that captures
Tab/Shift-Tab to cycle focus among focusable elements inside the panel and
prevents tabbing out) while keeping the existing onClick stopPropagation and the
backdrop click closing via setOpen(false).
In `@src/components/ui/radar-scan.tsx`:
- Around line 72-79: The radar blip pulsing CSS isn't being disabled when
reduced motion is requested; update the BLIPS mapping render in the RadarScan
component so each <div className="radar-blip"> receives an inline style that
sets animation: 'none' when shouldReduceMotion is true (e.g., style={{ top:
b.top, left: b.left, animationDelay: shouldReduceMotion ? undefined : b.delay,
animation: shouldReduceMotion ? 'none' : undefined }}), keeping existing
top/left and preserving animationDelay only when motion is allowed.
In `@src/components/ui/smoke-effect.tsx`:
- Line 99: The per-frame draw uses Math.random() causing each particle's gray
value to jitter; add a fixed gray property to the Particle interface and assign
it once in createParticle (e.g., p.gray = 15 + Math.random() * 10 when creating
the particle), then in the draw loop replace the per-frame const gray = 15 +
Math.random() * 10 with const gray = p.gray (or use p.gray directly) so the
smoke color stays stable per particle.
- Line 34: The line setting ctx.globalCompositeOperation = 'screen' is redundant
because resizeCanvas (which assigns canvas.width/height) resets the 2D context
and animate() reassigns the composite operation each frame; remove this dead
assignment from the initialization block (or, if intended to persist, move it to
run after resizeCanvas and before the first draw) — look for the canvas context
variable ctx, the resizeCanvas function, and the animate() function where
globalCompositeOperation is set to 'source-over' and then 'screen'.
- Around line 93-109: In the particle draw loop (the block that sets
ctx!.shadowBlur and calls ctx!.createRadialGradient), remove the two shadowBlur
assignments (ctx!.shadowBlur = 40 and ctx!.shadowBlur = 0) and stop relying on
shadowBlur for softness; instead, reuse a precomputed radial sprite/gradient
rather than creating a new CanvasGradient per particle per frame: implement a
small cache (e.g., gradientCache or offscreenSpriteCache) keyed by quantized
size/opacity/gray and generate the CanvasGradient or offscreen canvas once when
missing, then use that cached gradient/sprite in place of
createRadialGradient(p.x, p.y, ...) for each particle; update the code paths
that currently call createRadialGradient and the shadowBlur lines (the snippet
with createRadialGradient and ctx!.shadowBlur) to use the cached resource.
In `@src/components/ui/spark-canvas.tsx`:
- Around line 73-77: The per-particle use of ctx!.shadowBlur / ctx!.shadowColor
inside the draw loop (where each particle p is drawn with ctx!.fillRect) is
causing costly compositing state changes; instead either 1) batch particles by
color inside the SparkCanvas draw routine: group particles p by p.color, for
each color set ctx.shadowBlur and ctx.shadowColor once, draw all group
rectangles with ctx.fillRect, then reset shadowBlur after the group, or 2)
render all sparks to an offscreen canvas (offscreenCtx) without shadows, then
composite that canvas to the main ctx once using a single blur via ctx.filter =
'blur(8px)' (or CSS filter) and drawImage; update the code paths around the
current ctx!.shadowBlur / ctx!.shadowColor / ctx!.fillRect sequence to implement
one of these two approaches.
- Around line 18-95: The effect runs a continuous full-screen animation
(functions animate, createParticle, and the useEffect using canvasRef) without
honoring users' prefers-reduced-motion setting or any prop to disable it; add a
check at the top of the useEffect that uses
window.matchMedia('(prefers-reduced-motion: reduce)') (and accept an optional
prop like reducedMotion or a disableAnimation boolean) to short-circuit before
creating particles/requestAnimationFrame when reduced motion is requested or the
prop is true, and when toggling from animated -> reduced ensure you cancel the
running rafId, clear particles, reset canvas state (ctx.globalAlpha,
shadowBlur), and remove the resize listener so the animation fully stops and is
not started again.
In `@src/components/ui/status-badge.tsx`:
- Around line 5-10: The StatusBadgeProps declares a variant prop that's never
used; either wire it into the component to control styling or remove it
entirely. To keep the prop, update the StatusBadge function signature to accept
variant, add logic in StatusBadge to map variant ('active' | 'inactive') to
different className/style (e.g., green vs gray background and icon/color
adjustments) and use that className in the returned JSX; to remove it, delete
variant from StatusBadgeProps and the destructured props in the StatusBadge
declaration and adjust any callers if necessary. Ensure the chosen approach
updates both the interface (StatusBadgeProps) and the StatusBadge component
implementation so the prop is no longer declared-but-unused.
In `@src/components/ui/video-smoke.tsx`:
- Around line 18-30: The background <video> in src/components/ui/video-smoke.tsx
currently always autoPlays and ignores users' prefers-reduced-motion setting;
update it to respect that preference by either (1) adding Tailwind's
motion-reduce utilities to the video element’s className to hide/disable it
under prefers-reduced-motion (e.g., motion-reduce:opacity-0 or
motion-reduce:hidden) or (2) add a ref (e.g., videoRef) and in a useEffect check
window.matchMedia('(prefers-reduced-motion: reduce)') to pause the video and
remove autoPlay when matched; change the <video> element (and any autoPlay/loop
settings) accordingly so the video does not play when reduced-motion is
requested.
- Line 18: The <video> element in the VideoSmoke component
(src/components/ui/video-smoke.tsx) is currently loading eagerly; add the
attribute preload="none" to the <video> tag to prevent the browser from
downloading the video until playback is requested (keeping it decorative and
below-the-fold). Locate the JSX <video> element in the VideoSmoke component and
add preload="none" to its props; ensure the attribute is present alongside
existing props so behavior is deferred.
In `@src/hooks/use-bulk-import.ts`:
- Around line 37-43: The current cast of worksheet.getSheetValues() to
(string|number|boolean|null)[][] allows rich-text and complex ExcelJS cell types
to be coerced into "[object Object]" and then pushed into clanTags; instead,
change the handling in the import loop (the jsonData variable and the for-loop
using row/value and clanTags) to treat sheet values with their real ExcelJS
union type (e.g., unknown[] or CellValue[][]), explicitly detect and handle
rich-text objects (CellRichTextValue with richText array by concatenating each
piece's .text), handle hyperlink/formula/date cases to extract a plain string,
and skip or reject non-primitive/invalid cell types so the header filter and
clanTags only receive cleaned string clan names rather than "[object Object]".
In `@src/lib/auth.ts`:
- Line 9: The current fallback for secret in src/lib/auth.ts uses a hardcoded
placeholder (secret / BETTER_AUTH_SECRET) which is unsafe; remove the silent
fallback and instead require BETTER_AUTH_SECRET at startup: read
process.env.BETTER_AUTH_SECRET into a const and if it's missing, throw an Error
(or process.exit) when running in production (process.env.NODE_ENV ===
'production'); for non-production you may allow the placeholder but log a clear
warning so developers know it's not suitable for prod. Ensure the exported
secret value (secret) is set only from the validated BETTER_AUTH_SECRET.
In `@src/lib/storage.ts`:
- Around line 138-146: The SELECT currently uses four correlated subqueries on
table `changes` (producing `dest_tag`, `dest_name`, `src_tag`, `src_name`) which
causes O(N×4) scans; replace those four sub-SELECTs with two self-JOINs (or two
lateral subselects) that each fetch both tag and name for the matching row (one
join for the next 'join' after c.timestamp to produce dest_tag/dest_name, one
join for the last 'leave' before c.timestamp to produce src_tag/src_name), and
ensure the predicates (player_id, type, timestamp comparisons) are pushed into
the JOIN conditions; also add/ensure a composite index on (player_id, type,
timestamp) to make the lookups efficient.
In `@src/lib/wargaming-api.ts`:
- Around line 105-133: Both getPlayerCurrentClan and getPlayerCurrentClans call
makeRequest without error handling; match the pattern used by
getClanEmblems/getClanRatings by wrapping the makeRequest call in a try/catch,
log the error (e.g., this.logger.error or processLogger) and return a safe
fallback: for getPlayerCurrentClan return null on error, and for
getPlayerCurrentClans return an empty Record (or map with all ids mapped to null
if you prefer consistency). Update the functions getPlayerCurrentClan and
getPlayerCurrentClans to catch exceptions from makeRequest and return those safe
fallbacks.
- Around line 125-133: Wrap the body of getPlayerCurrentClans in a try/catch
similar to getClanMembersStats/getClanRatings: call this.makeRequest inside try,
build the result as before, and in catch log the error with context (include the
accountIds and the caught error) and then rethrow (or convert to a clear API
error) so callers get consistent behavior; optionally mirror the 100-ID batching
logic used in getClanMembersStats by chunking accountIds and aggregating results
if you want defensive protection against oversized requests.
In `@src/types/clan.ts`:
- Around line 28-37: The destination and source types omit clan_id while the
sibling clan type includes {clan_id, tag, name}; update the destination and
source shapes in the Clan types to also include clan_id (optional if it may be
missing at write time) so they match the clan shape, or if omission is
intentional add a clarifying comment explaining why clan_id is excluded;
reference the destination and source properties and the clan type when making
the change.
---
Outside diff comments:
In `@src/app/layout.tsx`:
- Around line 98-101: The Script tag loading particles.js in layout.tsx lacks
Subresource Integrity; update the <Script ... /> usage (the Script component
that imports "https://cdn.jsdelivr.net/npm/particles.js@2.0.0/particles.min.js")
to include an integrity attribute with the SHA-384 SRI hash you generate and add
crossorigin="anonymous" so the browser can verify the file (i.e., add
integrity="sha384-<your-generated-base64-hash>" and crossorigin="anonymous" to
that Script element).
In `@src/components/home/clan-search-panel.tsx`:
- Around line 70-89: The clickable clan item uses visual styles only and lacks
an accessible state for screen readers; update the motion.div (the interactive
element that calls setSelectedClan and compares selectedClan?.clan_id to
clan.clan_id) to include an appropriate ARIA state such as
aria-pressed={selectedClan?.clan_id === clan.clan_id} (or aria-selected if you
prefer list semantics) and ensure the onKeyDown/onClick handlers remain
unchanged so assistive tech receives the selected-state change.
In `@src/components/ui/card.tsx`:
- Around line 63-70: CardTitle was changed from a div to an h3 which is a
semantic/breaking change for consumers; either revert CardTitle back to the
original div element to preserve backward compatibility (restore previous
behavior in the CardTitle component) or keep the h3 but treat it as a breaking
change: add a clear migration note in the component docs/CHANGELOG and bump the
package major version, and update any related stories/tests that assert element
type for CardTitle so consumers are aware of the change.
In `@src/components/ui/player-card-tooltip.tsx`:
- Around line 30-75: When accountId changes the component never resets fetched,
causing fetchData to return early and show stale stats/clan; add a useEffect
that watches accountId and resets local state (call setFetched(false),
setStats(null), setLiveClan(undefined) and optionally setLoading(false)), then
optionally rehydrate from statsCache/clanCache (use statsCache.has(accountId) /
clanCache.get(accountId) to setStats/setLiveClan immediately) so fetchData can
run for the new accountId; reference the existing fetchData, fetched, accountId,
statsCache, clanCache, setFetched, setStats, and setLiveClan identifiers when
implementing.
In `@src/hooks/use-clan-history.ts`:
- Around line 110-131: join_clan/leave_clan events are still using new
Date(timestamp * 1000).toLocaleString() causing ClanHistoryEvent.time to be a
full datetime while role_change uses toLocaleTimeString('en-GB', ...); update
the events.push creation for join_clan/leave_clan (the block handling
item.subtype === 'join_clan' || 'leave_clan') to set time using new
Date(timestamp * 1000).toLocaleTimeString('en-GB', { hour: '2-digit', minute:
'2-digit' }) so the time format matches the role_change events and UI rendering
is consistent.
Comment on lines
+3
to
+7
| export default defineCloudflareConfig({ | ||
| incrementalCache: 'dummy', | ||
| tagCache: 'dummy', | ||
| queue: 'dummy', | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
'dummy' stubs disable all caching and ISR.
This is fine if the app has no incremental static regeneration or tag-based revalidation needs. If those features are ever used, these must be replaced with real bindings (e.g., r2IncrementalCache, a KV-backed tag cache). The recommended production alternative for incrementalCache is r2IncrementalCache from @opennextjs/cloudflare.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@open-next.config.ts` around lines 3 - 7, The config uses placeholder 'dummy'
bindings which disable Incremental Static Regeneration and tag revalidation;
update the defineCloudflareConfig call to replace incrementalCache and tagCache
with real Cloudflare-backed implementations (for example use r2IncrementalCache
from `@opennextjs/cloudflare` for incrementalCache and a KV-backed tag cache) and
set queue to a real Durable Queue binding so ISR/tag revalidation and background
jobs work in production; locate defineCloudflareConfig and change the
incrementalCache, tagCache, and queue entries accordingly, importing
r2IncrementalCache from `@opennextjs/cloudflare` and wiring the proper binding
names.
| timestamp, | ||
| date: new Date(timestamp * 1000).toISOString().split('T')[0], | ||
| time: new Date(timestamp * 1000).toLocaleString(), | ||
| time: new Date(timestamp * 1000).toLocaleTimeString('en-GB', { hour: '2-digit', minute: '2-digit' }), |
There was a problem hiding this comment.
Server-side toLocaleTimeString may produce inconsistent output.
toLocaleTimeString('en-GB', ...) depends on the runtime's ICU data. On Cloudflare Workers or minimal Node.js builds, locale support can be partial or absent, leading to unexpected formats. Since date on line 73 already uses the locale-independent toISOString(), consider formatting time deterministically too (e.g., pad hours/minutes manually from the Date object, or use a lightweight formatter).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/clan-history/route.ts` at line 74, The current time formatting
uses new Date(timestamp * 1000).toLocaleTimeString('en-GB', ...) which can vary
by runtime ICU support; replace it with a deterministic formatter that builds an
"HH:MM" string from the Date object instead (use Date.getUTCHours()/getHours()
depending on existing timezone intent and padStart to ensure two digits) in the
same place where the code creates the time field (the expression using new
Date(timestamp * 1000) that sets time). This ensures consistent output across
runtimes without relying on toLocaleTimeString.
Comment on lines
+16
to
+18
| const res = await fetch( | ||
| `https://api.tomato.gg/api/player/clan-history-unofficial/EU/${accountId}`, | ||
| { headers: { 'User-Agent': 'Mozilla/5.0' }, signal: AbortSignal.timeout(8000) } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Undocumented/unofficial Tomato.gg endpoint — no stability guarantees.
The path clan-history-unofficial signals this is not a public contract. A schema change or removal breaks Tier 1 silently (falls through to Tier 2/3), but until that is detected users get stale or partial data with no indicator. Add monitoring or at least log the fallback when Tier 1 fails, so breakage is visible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/player-history/route.ts` around lines 16 - 18, The unofficial
Tomato.gg fetch (the fetch call that assigns to res) can break silently; update
the route handler so any non-OK response or thrown error from the
`fetch("https://api.tomato.gg/api/player/clan-history-unofficial/EU/${accountId}")`
call is logged and reported before falling back to Tier 2/3: catch fetch errors
and check res.ok, then log a clear warning including accountId, HTTP status, and
error message/stack, and emit a monitoring event/metric (e.g., via your existing
metrics/sentry helper) indicating a Tier‑1 fallback so the breakage is visible.
Comment on lines
+226
to
+245
| @keyframes dust { | ||
| 0% { | ||
| transform: translateY(100vh) translateX(0) scale(1); | ||
| opacity: 0; | ||
| } | ||
| 10% { | ||
| opacity: 0.3; | ||
| } | ||
| 90% { | ||
| opacity: 0.1; | ||
| } | ||
| 100% { | ||
| transform: translateY(-20vh) translateX(calc((random() - 0.5) * 100px)) scale(1.5); | ||
| opacity: 0; | ||
| } | ||
| } | ||
|
|
||
| .animate-dust { | ||
| animation: dust linear infinite; | ||
| } |
There was a problem hiding this comment.
random() is not a valid CSS function — dust animation is broken.
Line 238: calc((random() - 0.5) * 100px) will be ignored by every browser. CSS has no random() function (it's only a CSS WG proposal). The translateX component in the 100% keyframe will be silently dropped, so all dust particles will drift straight up with no horizontal variation.
Use a CSS custom property set per-element from JS/TSX instead (e.g., --drift: 30px) or remove the horizontal offset entirely.
🐛 Proposed fix — use a custom property
100% {
- transform: translateY(-20vh) translateX(calc((random() - 0.5) * 100px)) scale(1.5);
+ transform: translateY(-20vh) translateX(var(--dust-drift, 0px)) scale(1.5);
opacity: 0;
}Then set --dust-drift per element in the component:
style={{ '--dust-drift': `${(Math.random() - 0.5) * 100}px` } as React.CSSProperties}🧰 Tools
🪛 Stylelint (17.3.0)
[error] 238-238: Expected math.random instead of random (scss/no-global-function-names)
(scss/no-global-function-names)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/globals.css` around lines 226 - 245, The `@keyframes` dust uses an
invalid random() call in the 100% keyframe (translateX(calc((random() - 0.5) *
100px))) causing browsers to drop the horizontal drift; replace that expression
with a CSS custom property (e.g., translateX(var(--dust-drift))) in the
`@keyframes` dust and keep .animate-dust as the animation class, then set
--dust-drift per dust element from JS/TSX (for example in the component that
renders the dust elements) using a runtime Math.random-derived value formatted
as pixels so each element gets its own horizontal offset.
Comment on lines
+37
to
+43
| const jsonData = worksheet.getSheetValues() as (string | number | boolean | null)[][]; | ||
|
|
||
| const clanTags: string[] = []; | ||
| for (let i = 0; i < jsonData.length; i++) { | ||
| const row = jsonData[i]; | ||
| if (Array.isArray(row) && row[0]) { | ||
| const value = String(row[0]).trim(); | ||
| if (Array.isArray(row) && row[1]) { | ||
| const value = String(row[1]).trim(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/hooks/use-bulk-import.ts | head -60Repository: ionutcnu/Watcher
Length of output: 2388
🏁 Script executed:
fd package.json | xargs cat | jq '.dependencies, .devDependencies | select(. != null)' | grep -i excelRepository: ionutcnu/Watcher
Length of output: 82
🏁 Script executed:
fd node_modules/exceljs/package.json | head -1 | xargs cat | jq '.version'Repository: ionutcnu/Watcher
Length of output: 42
🏁 Script executed:
find node_modules/exceljs -name '*.d.ts' | head -5Repository: ionutcnu/Watcher
Length of output: 115
🌐 Web query:
ExcelJS CellValue type union definition getSheetValues() return type
💡 Result:
ExcelJS.CellValue union (from index.d.ts)
export type CellValue =
| null
| number
| string
| boolean
| Date
| undefined
| CellErrorValue
| CellRichTextValue
| CellHyperlinkValue
| CellFormulaValue
| CellSharedFormulaValue;[1]
worksheet.getSheetValues() return type
In the ExcelJS implementation, getSheetValues() builds and returns a sparse array where the index is the 1-based row number, and each entry is row.values (which is itself a sparse, 1-based array of cell values). [2]
So the practical TypeScript type is:
// practical (matches runtime behavior: sparse, 1-based)
type SheetValues = Array<ExcelJS.CellValue[] | undefined>;If you want to be even more accurate about sparsity inside rows:
type SheetValues = Array<Array<ExcelJS.CellValue | undefined> | undefined>;Note: Some older/incorrect typings you may see as CellValue[][] miss the undefined + sparsity aspect implied by the implementation. [2]
Sources: [1] [2]
🌐 Web query:
ExcelJS richText cell value type definition
💡 Result:
In ExcelJS’ TypeScript defs, a rich-text cell value is:
export interface RichText {
text: string;
font?: Partial<Font>;
}
export interface CellRichTextValue {
richText: RichText[];
}
export type CellValue =
| null
| number
| string
| boolean
| Date
| undefined
| CellErrorValue
| CellRichTextValue
| CellHyperlinkValue
| CellFormulaValue
| CellSharedFormulaValue;So you set a cell to rich text by assigning a CellRichTextValue (i.e. { richText: RichText[] }). Example usage is also shown in the project README. [1] [2]
Sources: ExcelJS index.d.ts [1], ExcelJS README (Rich Text section) [2].
Type cast allows rich-text and complex cell values to silently become "[object Object]" strings.
ExcelJS's CellValue union includes Date, CellRichTextValue ({richText: RichText[]}), CellErrorValue, CellHyperlinkValue, CellFormulaValue, and others—far wider than the declared (string | number | boolean | null)[][]. Casting to this narrow type suppresses TypeScript errors; at runtime, String({richText:[...]}) produces "[object Object]", which fails the header filter but still gets pushed into clanTags as an invalid clan name, silently corrupting the import.
Handle rich-text cells explicitly:
🛡️ Proposed fix
- const jsonData = worksheet.getSheetValues() as (string | number | boolean | null)[][];
+ const jsonData = worksheet.getSheetValues() as unknown[][];
const clanTags: string[] = [];
for (let i = 0; i < jsonData.length; i++) {
const row = jsonData[i];
- if (Array.isArray(row) && row[1]) {
- const value = String(row[1]).trim();
+ if (!Array.isArray(row)) continue;
+ const cell = row[1];
+ if (cell == null) continue;
+ const value = (
+ typeof cell === 'object' && cell !== null && 'richText' in cell
+ ? (cell as { richText: { text: string }[] }).richText.map(r => r.text).join('')
+ : String(cell)
+ ).trim();
+ if (value && !value.toLowerCase().match(/^(clan|tag|name|guild)s?$/)) {
clanTags.push(value);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/use-bulk-import.ts` around lines 37 - 43, The current cast of
worksheet.getSheetValues() to (string|number|boolean|null)[][] allows rich-text
and complex ExcelJS cell types to be coerced into "[object Object]" and then
pushed into clanTags; instead, change the handling in the import loop (the
jsonData variable and the for-loop using row/value and clanTags) to treat sheet
values with their real ExcelJS union type (e.g., unknown[] or CellValue[][]),
explicitly detect and handle rich-text objects (CellRichTextValue with richText
array by concatenating each piece's .text), handle hyperlink/formula/date cases
to extract a plain string, and skip or reject non-primitive/invalid cell types
so the header filter and clanTags only receive cleaned string clan names rather
than "[object Object]".
Comment on lines
+28
to
+37
| /** For leave events: the next clan the player joined (within monitored clans) */ | ||
| destination?: { | ||
| tag: string; | ||
| name: string; | ||
| }; | ||
| /** For join events: the clan the player came from (within monitored clans) */ | ||
| source?: { | ||
| tag: string; | ||
| name: string; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
destination/source omit clan_id unlike the existing clan shape.
The sibling clan field carries { clan_id, tag, name }, but destination and source only have { tag, name }. If clan_id is ever needed for direct API lookups from these fields, it'll require a separate resolution step. If this is intentional (e.g., clan_id not available at write time), a comment here would prevent future confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/clan.ts` around lines 28 - 37, The destination and source types
omit clan_id while the sibling clan type includes {clan_id, tag, name}; update
the destination and source shapes in the Clan types to also include clan_id
(optional if it may be missing at write time) so they match the clan shape, or
if omission is intentional add a clarifying comment explaining why clan_id is
excluded; reference the destination and source properties and the clan type when
making the change.
Comment on lines
+31
to
+35
| const joinTs = Math.floor(new Date(since + 'Z').getTime() / 1000); | ||
| events.push({ type: 'join', clan_tag: tag, clan_name: name, timestamp: joinTs, date: since.split('T')[0], source: 'tomato' }); | ||
| if (until) { | ||
| const leaveTs = Math.floor(new Date(until + 'Z').getTime() / 1000); | ||
| events.push({ type: 'leave', clan_tag: tag, clan_name: name, timestamp: leaveTs, date: until.split('T')[0], source: 'tomato' }); |
There was a problem hiding this comment.
since + 'Z' will produce an invalid date if tomato.gg returns timestamps that already carry timezone info.
If the API returns "2023-01-15T10:30:00Z" (already UTC), appending 'Z' yields "2023-01-15T10:30:00ZZ" — new Date() returns NaN, and Math.floor(NaN / 1000) silently becomes NaN, corrupting the event timestamps for the entire response.
🔧 Proposed fix — parse only if no timezone suffix is present
- const joinTs = Math.floor(new Date(since + 'Z').getTime() / 1000);
+ const joinTs = Math.floor(new Date(/[Z+\-]\d*$/.test(since) ? since : since + 'Z').getTime() / 1000);
events.push({ type: 'join', clan_tag: tag, clan_name: name, timestamp: joinTs, date: since.split('T')[0], source: 'tomato' });
if (until) {
- const leaveTs = Math.floor(new Date(until + 'Z').getTime() / 1000);
+ const leaveTs = Math.floor(new Date(/[Z+\-]\d*$/.test(until) ? until : until + 'Z').getTime() / 1000);Alternatively, just always call new Date(since) if the format is consistently ISO-8601 from the API — verify the actual response shape first.
What date/time format does the tomato.gg unofficial clan history API return for the "since" and "until" fields?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/player-history/route.ts` around lines 31 - 35, The code appends
'Z' to since/until before parsing which breaks when tomato.gg already returns
timezone-suffixed ISO strings; update the parsing in route.ts (where joinTs and
leaveTs are computed and events are pushed) to either (a) avoid appending 'Z'
and use new Date(since)/new Date(until) directly, or (b) only append 'Z' when
the string lacks any timezone indicator (e.g., no trailing 'Z' and no ±HH:MM);
ensure joinTs and leaveTs remain numeric (use getTime()/1000) and guard against
NaN before pushing events.
Comment on lines
+82
to
+91
| <a | ||
| href={`https://tomato.gg/stats/EU/${encodeURIComponent(change.player.account_name)}=${change.player.account_id}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| title="View on tomato.gg" | ||
| className="shrink-0 transition-colors hover:text-[#FF8C00]" | ||
| style={{ color: '#7a6a4a' }} | ||
| > | ||
| <ExternalLink className="w-3 h-3" /> | ||
| </a> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine recent-changes-panel.tsx around lines 82-91
fd recent-changes-panel.tsx --type fRepository: ionutcnu/Watcher
Length of output: 104
🏁 Script executed:
# Then locate battle-report.tsx to check the URL format there
fd battle-report.tsx --type fRepository: ionutcnu/Watcher
Length of output: 103
🏁 Script executed:
# Search for all tomato.gg URL patterns in the codebase
rg "tomato\.gg" -A 2 -B 2Repository: ionutcnu/Watcher
Length of output: 6931
Fix tomato.gg URL format — EU/ should be a path suffix, not prefix.
The URL pattern uses tomato.gg/stats/{name}-{id}/EU or tomato.gg/stats/{name}={id}/EU — the region code is a suffix segment. This URL puts EU before the player name:
https://tomato.gg/stats/EU/${name}=${id} ← broken
https://tomato.gg/stats/${name}=${id}/EU ← correct
The battle-report.tsx and manual-check-results.tsx components use the correct format. Apply the same pattern here:
- href={`https://tomato.gg/stats/EU/${encodeURIComponent(change.player.account_name)}=${change.player.account_id}`}
+ href={`https://tomato.gg/stats/${encodeURIComponent(change.player.account_name)}=${change.player.account_id}/EU`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a | |
| href={`https://tomato.gg/stats/EU/${encodeURIComponent(change.player.account_name)}=${change.player.account_id}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| title="View on tomato.gg" | |
| className="shrink-0 transition-colors hover:text-[#FF8C00]" | |
| style={{ color: '#7a6a4a' }} | |
| > | |
| <ExternalLink className="w-3 h-3" /> | |
| </a> | |
| <a | |
| href={`https://tomato.gg/stats/${encodeURIComponent(change.player.account_name)}=${change.player.account_id}/EU`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| title="View on tomato.gg" | |
| className="shrink-0 transition-colors hover:text-[`#FF8C00`]" | |
| style={{ color: '#7a6a4a' }} | |
| > | |
| <ExternalLink className="w-3 h-3" /> | |
| </a> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/home/recent-changes-panel.tsx` around lines 82 - 91, The
tomato.gg link in recent-changes-panel.tsx places the region "EU" before the
player segment; update the anchor href so the region is a trailing path segment
after the encoded player identifier — construct the URL using
change.player.account_name and change.player.account_id as the middle segment
(encoded), followed by "/EU" (e.g.,
https://tomato.gg/stats/{encodedName}={id}/EU) so it matches the format used in
battle-report.tsx and manual-check-results.tsx; modify the href expression in
the anchor that uses change.player.account_name and change.player.account_id
accordingly.
Comment on lines
+152
to
+153
| <PlayerCardTooltip accountId={player.account_id} accountName={player.account_name} clanTag={player.destinationTag}> | ||
| <div className="flex items-center gap-1.5"> |
There was a problem hiding this comment.
PlayerCardTooltip for "left" players receives destinationTag as clanTag — misleading initial state.
PlayerCardTooltip falls back to clanTag when liveClan is still undefined (pre-fetch). Passing player.destinationTag means the tooltip header will briefly show the destination clan (where the player went) rather than the clan they left — which is the context of the "Left" column. Pass clanTag (the monitored clan) to match the surrounding context, consistent with how the joined-side tooltip is handled (line 72).
- <PlayerCardTooltip accountId={player.account_id} accountName={player.account_name} clanTag={player.destinationTag}>
+ <PlayerCardTooltip accountId={player.account_id} accountName={player.account_name} clanTag={clanTag}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <PlayerCardTooltip accountId={player.account_id} accountName={player.account_name} clanTag={player.destinationTag}> | |
| <div className="flex items-center gap-1.5"> | |
| <PlayerCardTooltip accountId={player.account_id} accountName={player.account_name} clanTag={clanTag}> | |
| <div className="flex items-center gap-1.5"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/monitoring/battle-report.tsx` around lines 152 - 153, The
PlayerCardTooltip for "left" players is currently passed player.destinationTag
which causes the tooltip to briefly show the destination clan while liveClan is
undefined; update the props to pass the monitored clan tag (the variable/prop
used elsewhere for the left-column context, e.g., clanTag or the component/state
representing the monitored clan) instead of player.destinationTag so the tooltip
header reflects the clan they left and matches the joined-side behavior; adjust
the PlayerCardTooltip invocation (accountId, accountName, clanTag) accordingly.
Comment on lines
+220
to
+238
| <motion.div | ||
| key="backdrop" | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={{ duration: 0.15 }} | ||
| className="fixed inset-0 flex items-center justify-center p-4" | ||
| style={{ background: 'rgba(0,0,0,0.88)', zIndex: 300 }} | ||
| onClick={() => setOpen(false)} | ||
| > | ||
| <motion.div | ||
| key="panel" | ||
| initial={{ opacity: 0, scale: 0.97, y: 16 }} | ||
| animate={{ opacity: 1, scale: 1, y: 0 }} | ||
| exit={{ opacity: 0, scale: 0.97, y: 16 }} | ||
| transition={{ duration: 0.18 }} | ||
| className="w-full max-w-5xl max-h-[80vh] flex flex-col rounded-xl overflow-hidden" | ||
| style={{ background: '#0d0b09', border: '1px solid #2a2418' }} | ||
| onClick={e => e.stopPropagation()} |
There was a problem hiding this comment.
Modal is missing accessibility semantics and a focus trap.
The panel div (line 230) has no role="dialog", aria-modal="true", or aria-labelledby. Without these, screen readers don't announce it as a dialog and don't restrict browsing to its content. Keyboard users can also tab out of the modal into the background.
🛡️ Proposed minimal fix
<motion.div
key="panel"
...
className="w-full max-w-5xl max-h-[80vh] flex flex-col rounded-xl overflow-hidden"
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="journey-title"
style={{ background: '#0d0b09', border: '1px solid `#2a2418`' }}
onClick={e => e.stopPropagation()}
>Then add id="journey-title" to the <h2> on line 256. A full focus trap (e.g., focus-trap-react or a custom useEffect that constrains Tab/Shift-Tab) should also be added.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/player-journey-timeline.tsx` around lines 220 - 238, The
modal container motion.div with key="panel" is missing dialog semantics and
focus trapping; add role="dialog", aria-modal="true" and aria-labelledby
pointing to the h2's id (add id="journey-title" to the existing <h2>), move
focus into the modal when setOpen becomes true and restore focus on close, and
implement a focus trap (either integrate focus-trap-react around the panel or
add a useEffect that captures Tab/Shift-Tab to cycle focus among focusable
elements inside the panel and prevents tabbing out) while keeping the existing
onClick stopPropagation and the backdrop click closing via setOpen(false).
Comment on lines
+138
to
+146
| `SELECT | ||
| c.type, c.player_id, c.player_name, c.clan_id, c.clan_tag, c.clan_name, c.timestamp, c.date, | ||
| (SELECT d.clan_tag FROM changes d WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp ORDER BY d.timestamp ASC LIMIT 1) AS dest_tag, | ||
| (SELECT d.clan_name FROM changes d WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp ORDER BY d.timestamp ASC LIMIT 1) AS dest_name, | ||
| (SELECT d.clan_tag FROM changes d WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp ORDER BY d.timestamp DESC LIMIT 1) AS src_tag, | ||
| (SELECT d.clan_name FROM changes d WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp ORDER BY d.timestamp DESC LIMIT 1) AS src_name | ||
| FROM changes c | ||
| WHERE c.timestamp >= ? | ||
| ORDER BY c.timestamp DESC` |
There was a problem hiding this comment.
Four correlated subqueries — severe O(N×4) scan overhead on the changes table.
Each of the four added sub-SELECTs re-scans changes per row in the outer query. With any meaningful volume of data this becomes very expensive. dest_tag/dest_name also repeat the identical predicate twice — they should be a single lookup.
Replace with two self-JOINs (or a pair of CTEs):
⚡ Proposed refactor — two lateral-style sub-selects via JOIN
const stmt = db.prepare(
`SELECT
c.type, c.player_id, c.player_name, c.clan_id, c.clan_tag, c.clan_name, c.timestamp, c.date,
- (SELECT d.clan_tag FROM changes d WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp ORDER BY d.timestamp ASC LIMIT 1) AS dest_tag,
- (SELECT d.clan_name FROM changes d WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp ORDER BY d.timestamp ASC LIMIT 1) AS dest_name,
- (SELECT d.clan_tag FROM changes d WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp ORDER BY d.timestamp DESC LIMIT 1) AS src_tag,
- (SELECT d.clan_name FROM changes d WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp ORDER BY d.timestamp DESC LIMIT 1) AS src_name
+ dest.clan_tag AS dest_tag,
+ dest.clan_name AS dest_name,
+ src.clan_tag AS src_tag,
+ src.clan_name AS src_name
FROM changes c
+ LEFT JOIN changes dest ON dest.rowid = (
+ SELECT d.rowid FROM changes d
+ WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp
+ ORDER BY d.timestamp ASC LIMIT 1
+ )
+ LEFT JOIN changes src ON src.rowid = (
+ SELECT d.rowid FROM changes d
+ WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp
+ ORDER BY d.timestamp DESC LIMIT 1
+ )
WHERE c.timestamp >= ?
ORDER BY c.timestamp DESC`
).bind(cutoffTimestamp);This cuts the correlated subquery count from 4 to 2. A composite index on (player_id, type, timestamp) will make both inner lookups efficient.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| `SELECT | |
| c.type, c.player_id, c.player_name, c.clan_id, c.clan_tag, c.clan_name, c.timestamp, c.date, | |
| (SELECT d.clan_tag FROM changes d WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp ORDER BY d.timestamp ASC LIMIT 1) AS dest_tag, | |
| (SELECT d.clan_name FROM changes d WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp ORDER BY d.timestamp ASC LIMIT 1) AS dest_name, | |
| (SELECT d.clan_tag FROM changes d WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp ORDER BY d.timestamp DESC LIMIT 1) AS src_tag, | |
| (SELECT d.clan_name FROM changes d WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp ORDER BY d.timestamp DESC LIMIT 1) AS src_name | |
| FROM changes c | |
| WHERE c.timestamp >= ? | |
| ORDER BY c.timestamp DESC` | |
| `SELECT | |
| c.type, c.player_id, c.player_name, c.clan_id, c.clan_tag, c.clan_name, c.timestamp, c.date, | |
| dest.clan_tag AS dest_tag, | |
| dest.clan_name AS dest_name, | |
| src.clan_tag AS src_tag, | |
| src.clan_name AS src_name | |
| FROM changes c | |
| LEFT JOIN changes dest ON dest.rowid = ( | |
| SELECT d.rowid FROM changes d | |
| WHERE d.type = 'join' AND d.player_id = c.player_id AND d.timestamp > c.timestamp | |
| ORDER BY d.timestamp ASC LIMIT 1 | |
| ) | |
| LEFT JOIN changes src ON src.rowid = ( | |
| SELECT d.rowid FROM changes d | |
| WHERE d.type = 'leave' AND d.player_id = c.player_id AND d.timestamp < c.timestamp | |
| ORDER BY d.timestamp DESC LIMIT 1 | |
| ) | |
| WHERE c.timestamp >= ? | |
| ORDER BY c.timestamp DESC` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/storage.ts` around lines 138 - 146, The SELECT currently uses four
correlated subqueries on table `changes` (producing `dest_tag`, `dest_name`,
`src_tag`, `src_name`) which causes O(N×4) scans; replace those four sub-SELECTs
with two self-JOINs (or two lateral subselects) that each fetch both tag and
name for the matching row (one join for the next 'join' after c.timestamp to
produce dest_tag/dest_name, one join for the last 'leave' before c.timestamp to
produce src_tag/src_name), and ensure the predicates (player_id, type, timestamp
comparisons) are pushed into the JOIN conditions; also add/ensure a composite
index on (player_id, type, timestamp) to make the lookups efficient.
State resets, stable deps, rich-text cell handling, fail-fast auth secret, WG API error handling, time format consistency, dialog semantics, manifest start_url, percent clamping, and deduped account IDs.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/hooks/use-clan-history.ts (3)
50-76:⚠️ Potential issue | 🟠 MajorBoth fetch calls lack
response.okguards — HTTP errors silently swallowed.Non-2xx responses (rate-limits, 500s) are never thrown, so the
catchblock is bypassed. For the newsfeed call, the error body is parsed as data; for the player-names call,playerNamesfalls back to{}and every player renders asPlayer_{id}with no visible failure.🛡️ Proposed fix
const response = await fetch(`/api/clan-newsfeed?clanId=${selectedClan.clan_id}`); + if (!response.ok) throw new Error(`clan-newsfeed: ${response.status}`); const rawData = await response.json();const namesResponse = await fetch('/api/get-player-names', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ accountIds: Array.from(allAccountIds) }) }); + if (!namesResponse.ok) throw new Error(`get-player-names: ${namesResponse.status}`); const namesResult = await namesResponse.json();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-clan-history.ts` around lines 50 - 76, The two fetch calls in use-clan-history.ts (the initial response for `/api/clan-newsfeed` and the namesResponse for `/api/get-player-names`) need explicit response.ok checks before parsing JSON: after awaiting `response` and `namesResponse` verify `response.ok` / `namesResponse.ok` and throw a descriptive Error (include status and statusText or body text) if not ok so the catch block runs; only call `await response.json()` / `await namesResponse.json()` when the response is ok and ensure `playerNames` still defaults to {} on failure. Reference `response`, `rawData`, `namesResponse`, `namesResult`, `playerNames`, and `allAccountIds` when adding these guards.
45-49:⚠️ Potential issue | 🟡 MinorStale events visible across clan switches and on load failure.
allEventsis never cleared at the start ofloadClanHistory. Previous clan's events remain visible while loading, and persist indefinitely on fetch failure (the catch block at line 140 doesn't clear state).🛡️ Proposed fix
const loadClanHistory = useCallback(async () => { if (!selectedClan) return; setHistoryLoading(true); + setAllEvents([]); + setFilteredEvents([]); try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-clan-history.ts` around lines 45 - 49, The bug is that previous clan events remain visible because allEvents is never cleared; update the loadClanHistory function to clear event state at start and on failure: call setAllEvents([]) (or the equivalent state updater) immediately when loadClanHistory begins (before/when setHistoryLoading(true)) so UI doesn't show stale events, and also call setAllEvents([]) inside the catch block (alongside any error handling and setHistoryLoading(false)) to ensure failed fetches don't leave old events displayed; reference the loadClanHistory function, the allEvents state and its setter (setAllEvents) and the existing catch block to make these changes.
188-189:⚠️ Potential issue | 🟡 Minor
URL.revokeObjectURLimmediately afterclick()is not guaranteed safe.
click()queues the download asynchronously; revoking the object URL synchronously before the browser services the download request may cause failure in some environments. Deferring the revocation is safer.🛡️ Proposed fix
- a.click(); - URL.revokeObjectURL(url); + a.click(); + setTimeout(() => URL.revokeObjectURL(url), 100);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-clan-history.ts` around lines 188 - 189, The code calls a.click() then immediately URL.revokeObjectURL(url), which can revoke the blob before the browser begins the download; change this to revoke the object URL asynchronously (e.g., schedule URL.revokeObjectURL(url) in a setTimeout callback or revoke it from a download-complete handler) so the download request can be serviced first; update the call site where a.click() and URL.revokeObjectURL(url) occur (referencing the anchor variable "a" and the "url" object) to defer revocation.src/components/monitoring/manual-check-results.tsx (2)
176-176: 🧹 Nitpick | 🔵 TrivialUse a stable key instead of array index.
- {clanResult.leavers.map((leaver, index) => ( - <tr key={index} className="hover:bg-border/30"> + {clanResult.leavers.map((leaver) => ( + <tr key={leaver.player.account_id} className="hover:bg-border/30">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/monitoring/manual-check-results.tsx` at line 176, The table row uses the array index as key (key={index}) which is unstable; in ManualCheckResults (manual-check-results.tsx) replace the index key with a stable unique identifier from the row object (e.g., use item.id, resultId, checkId, or a deterministic composite like `${item.id || item.name}-${item.timestamp}`) so React can correctly track rows during re-renders and list mutations.
184-188:⚠️ Potential issue | 🟡 Minor
leaver.timeignored — timestamp re-derived inline.
ClanCheckResultalready exposes a pre-formattedtimefield (same asdateat line 185), but the detailed table re-derives the time from the raw Unix timestamp. This can produce a different format than the rest of the UI and wastes aDateallocation per row.<div className="text-xs text-text-tertiary"> - {new Date(leaver.timestamp * 1000).toLocaleTimeString('en-GB', { hour: '2-digit', minute: '2-digit' })} + {leaver.time} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/monitoring/manual-check-results.tsx` around lines 184 - 188, The detailed table is re-deriving a time string from leaver.timestamp instead of using the preformatted field on the model, causing inconsistent formatting and extra Date allocations; update the table cell that currently renders new Date(leaver.timestamp * 1000).toLocaleTimeString(...) to use the provided leaver.time (from the ClanCheckResult) so the component (manual-check-results) uses the canonical preformatted time and avoids creating a Date per row.src/components/ui/player-card-tooltip.tsx (1)
30-81:⚠️ Potential issue | 🔴 CriticalRace condition: stale in-flight fetch overwrites new player's state.
When
accountIdchanges, theuseEffectat line 30 resetsstats/liveClan/fetchedbut there is no mechanism to cancel or discard the previousfetchDatacall that is still in flight. BecausesetStats/setLiveClanare stable references, the old fetch completes and overwrites the new player's state.Example sequence:
- Hover player A →
fetchDatastarts- Hover player B →
useEffectresets state, newfetchDatastarts- Player A's fetch resolves →
setStats(playerAStats),setLiveClan(playerATag)clobbers player B's viewFix: track a fetch-generation counter in a
refand bail out of thethenhandlers when stale.🐛 Proposed fix
+ const fetchGenRef = useRef(0); + useEffect(() => { + fetchGenRef.current += 1; setStats(null); setLiveClan(undefined); setFetched(false); }, [accountId]); const fetchData = useCallback(async () => { if (fetched) return; + const gen = fetchGenRef.current; if (statsCache.has(accountId)) { setStats(statsCache.get(accountId) ?? null); setLiveClan(clanCache.has(accountId) ? (clanCache.get(accountId) ?? null) : undefined); setFetched(true); return; } setLoading(true); try { const [statsRes, clanRes] = await Promise.allSettled([...]); + if (gen !== fetchGenRef.current) return; // stale — discard // ... rest of state updates🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/player-card-tooltip.tsx` around lines 30 - 81, The fetchData flow has a race where an in-flight fetch can overwrite state after accountId changes; modify the component to track a generation id in a useRef (e.g., fetchGenRef) which you increment inside the useEffect that resets state for a new accountId, capture the current generation at the start of fetchData, and before any state or cache writes (including setStats, setLiveClan, statsCache.set, clanCache.set, setFetched, setLoading) bail out if the captured generation does not match fetchGenRef.current; keep the existing cache logic but only apply results when the generation matches to ensure stale responses cannot clobber the new player view.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/icons/site.webmanifest`:
- Around line 1-23: The manifest is missing explicit "scope" (and optionally
"id"), so add a "scope" property (e.g., "/") to the JSON and optionally add an
"id" property (e.g., "/?app=clan-watcher" or a stable string) to
site.webmanifest; update the object that currently contains "name",
"short_name", "start_url", "display", etc., by inserting "scope":
"<desired-scope>" and "id": "<stable-id>" so the browser uses the explicit
scope/id instead of deriving them from start_url.
In `@src/app/api/player-history/route.ts`:
- Around line 45-53: The GET handler currently lacks authentication and exposes
player history; modify the exported GET function to call the existing withAuth
middleware/utility (same used by player-current-clans) before processing the
request, aborting with an unauthorized response if withAuth indicates no valid
session; then only parse playerId and proceed to DB reads (keeping badRequest
checks) after auth succeeds — reference the GET function and the withAuth helper
to find where to insert the auth check and early return on failure.
In `@src/components/home/features-section.tsx`:
- Around line 32-34: The heading hierarchy is incorrect: the component renders
an h2 ("Key Features") followed by h4 elements for each feature title; update
those feature title elements to h3 so the structure becomes h2 → h3 and
satisfies WCAG 1.3.1. Locate the feature title JSX (the h4 elements in the
FeaturesSection component in src/components/home/features-section.tsx —
including the repeated block around lines 58–65) and change the tag from h4 to
h3, preserving the existing classes, styles, and content.
- Around line 58-65: The "Coming Soon" badge is currently inside the <h4> that
renders feature.title so screen readers will read it as part of the heading;
update the JSX in features-section.tsx so the conditional rendering of
feature.comingSoon moves the <span> out of the <h4> (render it immediately after
the <h4>) or, if you prefer to keep it inline, add aria-hidden="true" to the
<span> and render a visually-hidden companion label for screen readers; locate
the <h4> that references feature.title and the conditional span for
feature.comingSoon and adjust their placement/attributes accordingly.
- Line 46: The gradient top border div (className on the absolute div) currently
has sharp corners; add the rounded-t-xl utility to its className (i.e., change
the absolute gradient div's className to include "rounded-t-xl") so its top
corners match the card; also ensure the parent motion.div (the card wrapper) has
"overflow-hidden" and the intended "rounded-xl" so the gradient is clipped and
corners are consistent.
In `@src/components/monitoring/manual-check-progress.tsx`:
- Around line 111-113: The BLIPS mapping currently always renders animated
.radar-blip elements even when shouldReduceMotion is true; update the rendering
in the component (where BLIPS is mapped) to respect shouldReduceMotion by either
not rendering the blips at all when shouldReduceMotion is true or by disabling
their animation (e.g., removing animationDelay and setting
animationPlayState/animation: 'none') for each blip. Locate the BLIPS map block
and use the shouldReduceMotion flag to gate rendering or adjust the inline style
so blips are inert when shouldReduceMotion is true.
- Around line 46-57: Extract the repeated font-family string into a module-level
constant (e.g., FONT_FAMILY = "'Oswald', 'Roboto Condensed', sans-serif") at the
top of src/components/monitoring/manual-check-progress.tsx and replace all
inline uses of style={{ fontFamily: "...", ... }} in this component (including
the two span elements shown and the ~4 other occurrences) to use that constant
(e.g., style={{ fontFamily: FONT_FAMILY, color: ... }}); ensure the constant is
exported or kept local as appropriate and update every occurrence so there are
no remaining literal font-family strings.
In `@src/components/monitoring/manual-check-results.tsx`:
- Around line 40-46: The effect fetching /api/player-current-clans can race and
setCurrentClans with stale data; modify the useEffect that depends on
results.success and leaverKey to create an AbortController, pass
controller.signal to fetch(`/api/player-current-clans?accountIds=${leaverKey}`),
and return a cleanup that calls controller.abort(); in the promise handlers
check for abort (or catch AbortError) before calling setCurrentClans and avoid
swallowing non-abort errors so real failures can be observed. Ensure references
to useEffect, setCurrentClans, leaverKey, and the fetch URL are updated
accordingly.
In `@src/components/ui/player-card-tooltip.tsx`:
- Around line 64-74: The fetch failure case for clanRes isn't caching a value
which causes live clan to be silently disabled for the session; update the clan
fetch handling around clanRes, clanCache, setLiveClan and setFetched so failures
are explicitly cached (use clanCache.set(accountId, null) when clanRes is not
fulfilled or the response is not ok) and call setLiveClan(null) /
setFetched(true) accordingly; this makes clan failure symmetric with statsCache
behavior and prevents the tooltip from permanently suppressing retries or
leaving clanCache undefined.
In `@src/components/ui/player-journey-timeline.tsx`:
- Around line 191-208: The useEffect fetching history can have in-flight
requests overwrite state for a new player; wrap the fetch in an AbortController
inside the useEffect, pass controller.signal to fetch, and in the cleanup call
controller.abort() to cancel previous requests; update the .catch handler to
ignore abort errors (or check error.name === 'AbortError') so setEvents,
setCurrentClan, setDataSource, setError and setLoading are not invoked for
aborted requests (refer to the useEffect block, fetch call, and state setters
setEvents/setCurrentClan/setDataSource/setError/setLoading).
- Around line 362-364: Replace the unstable index key used in the visible.map
rendering (key={idx}) with a stable composite key derived from the stay object
to avoid reconciliation bugs; update the StayNode instantiation inside
visible.map to use a string like
`${stay.tag}-${stay.joinDate}-${reversed.indexOf(stay)}` (or similar unique
combination of stay.tag and stay.joinDate with reversed.indexOf(stay) as a
tiebreaker) so keys remain stable even if events/stays are filtered or
reordered.
In `@src/components/ui/video-smoke.tsx`:
- Around line 19-23: The video element in VideoSmoke
(src/components/ui/video-smoke.tsx) includes a redundant preload="none" while
already using autoPlay; remove the preload="none" attribute from the JSX for the
video element (where attributes autoPlay, loop, muted, playsInline are set) so
autoplay behavior isn't contradicted or misleading, and verify no other logic
depends on that attribute.
In `@src/hooks/use-bulk-import.ts`:
- Around line 36-37: The code accesses workbook.worksheets[0] without checking
for an empty worksheets array, so worksheet may be undefined and calling
worksheet.getSheetValues() will throw; update the logic in useBulkImport to
guard workbook.worksheets (e.g., check workbook.worksheets.length > 0 or if
(!worksheet) ) before calling getSheetValues(), and handle the empty case by
returning early or throwing a clear error / triggering the user-facing toast
with a specific message like "Uploaded Excel file contains no worksheets" so
jsonData is only computed when worksheet is defined (referencing worksheet,
workbook.worksheets, jsonData, and getSheetValues to locate the change).
In `@src/lib/auth.ts`:
- Line 9: The current secret initialization uses nullish coalescing
(process.env.BETTER_AUTH_SECRET ?? ...) which treats an empty string as a valid
secret; change it to a truthiness check so empty-string values fall through to
the IIFE. Update the secret assignment in src/lib/auth.ts (the secret property
initialization) to use a logical OR or explicit Boolean check (e.g.,
process.env.BETTER_AUTH_SECRET || (() => { if (process.env.NODE_ENV ===
'production') throw new Error('BETTER_AUTH_SECRET is required in production');
return 'dev-placeholder'; })()) so production still throws and local dev gets
the placeholder while empty strings are rejected.
In `@src/lib/wargaming-api.ts`:
- Around line 233-249: getClanEmblems currently sends all clanIds at once and
swallows failures; change it to batch clanIds into chunks of max 100 (same
approach as getClanMembersStats), call this.makeRequest('/wgn/clans/info/', {
clan_id: chunk.join(','), fields: 'emblems' }) for each chunk, merge responses
into the final Record<number,string|null> (using the same
entry?.emblems?.x64?.portal fallback logic), and do not silently swallow
errors—propagate or rethrow the caught error so callers can handle it instead of
returning an empty object.
---
Outside diff comments:
In `@src/components/monitoring/manual-check-results.tsx`:
- Line 176: The table row uses the array index as key (key={index}) which is
unstable; in ManualCheckResults (manual-check-results.tsx) replace the index key
with a stable unique identifier from the row object (e.g., use item.id,
resultId, checkId, or a deterministic composite like `${item.id ||
item.name}-${item.timestamp}`) so React can correctly track rows during
re-renders and list mutations.
- Around line 184-188: The detailed table is re-deriving a time string from
leaver.timestamp instead of using the preformatted field on the model, causing
inconsistent formatting and extra Date allocations; update the table cell that
currently renders new Date(leaver.timestamp * 1000).toLocaleTimeString(...) to
use the provided leaver.time (from the ClanCheckResult) so the component
(manual-check-results) uses the canonical preformatted time and avoids creating
a Date per row.
In `@src/components/ui/player-card-tooltip.tsx`:
- Around line 30-81: The fetchData flow has a race where an in-flight fetch can
overwrite state after accountId changes; modify the component to track a
generation id in a useRef (e.g., fetchGenRef) which you increment inside the
useEffect that resets state for a new accountId, capture the current generation
at the start of fetchData, and before any state or cache writes (including
setStats, setLiveClan, statsCache.set, clanCache.set, setFetched, setLoading)
bail out if the captured generation does not match fetchGenRef.current; keep the
existing cache logic but only apply results when the generation matches to
ensure stale responses cannot clobber the new player view.
In `@src/hooks/use-clan-history.ts`:
- Around line 50-76: The two fetch calls in use-clan-history.ts (the initial
response for `/api/clan-newsfeed` and the namesResponse for
`/api/get-player-names`) need explicit response.ok checks before parsing JSON:
after awaiting `response` and `namesResponse` verify `response.ok` /
`namesResponse.ok` and throw a descriptive Error (include status and statusText
or body text) if not ok so the catch block runs; only call `await
response.json()` / `await namesResponse.json()` when the response is ok and
ensure `playerNames` still defaults to {} on failure. Reference `response`,
`rawData`, `namesResponse`, `namesResult`, `playerNames`, and `allAccountIds`
when adding these guards.
- Around line 45-49: The bug is that previous clan events remain visible because
allEvents is never cleared; update the loadClanHistory function to clear event
state at start and on failure: call setAllEvents([]) (or the equivalent state
updater) immediately when loadClanHistory begins (before/when
setHistoryLoading(true)) so UI doesn't show stale events, and also call
setAllEvents([]) inside the catch block (alongside any error handling and
setHistoryLoading(false)) to ensure failed fetches don't leave old events
displayed; reference the loadClanHistory function, the allEvents state and its
setter (setAllEvents) and the existing catch block to make these changes.
- Around line 188-189: The code calls a.click() then immediately
URL.revokeObjectURL(url), which can revoke the blob before the browser begins
the download; change this to revoke the object URL asynchronously (e.g.,
schedule URL.revokeObjectURL(url) in a setTimeout callback or revoke it from a
download-complete handler) so the download request can be serviced first; update
the call site where a.click() and URL.revokeObjectURL(url) occur (referencing
the anchor variable "a" and the "url" object) to defer revocation.
---
Duplicate comments:
In `@public/icons/site.webmanifest`:
- Around line 5-18: The reviewer noted this is a duplicate comment; update the
PR by removing the redundant review note or mark it as resolved and ensure the
manifest entries remain unchanged—specifically keep "start_url": "/" and the
"purpose": "any maskable" values for the icon objects in site.webmanifest so no
code changes are made to those keys.
In `@src/app/api/player-history/route.ts`:
- Around line 35-40: The code appends 'Z' to since/until which can produce
invalid strings and NaN timestamps (variables joinTs/leaveTs) and also calls
since.split without null checks; change the logic in route handler that builds
events so you (1) only normalize by appending 'Z' when the timestamp is present
and lacks any timezone indicator (check with a regex for trailing Z or
[+/-]HH:MM), (2) guard against null/undefined since/until before splitting or
parsing, (3) create Date objects from the normalized string and check
isNaN(date.getTime()) and skip or handle the event if the date is invalid, and
(4) keep the same event shapes pushed to events ({ type, clan_tag, clan_name,
timestamp, date, source }) using date.toISOString().split('T')[0] for the date
portion to avoid manual splitting of the original raw value.
In `@src/components/home/recent-changes-panel.tsx`:
- Around line 82-91: The tomato.gg link in the RecentChangesPanel anchor (the
href built in recent-changes-panel.tsx) incorrectly prefixes the URL with the
region ("EU/...") and causes 404s; update the href construction to place the
region suffix after the player identifier (i.e., build URL like
https://tomato.gg/stats/{player_identifier}{region} or match the pattern used in
player-journey-timeline.tsx where the region is appended after
encodeURIComponent(change.player.account_name)=change.player.account_id), so
modify the string template in the anchor that references
change.player.account_name and change.player.account_id to append the region at
the end instead of at the start.
In `@src/components/monitoring/manual-check-progress.tsx`:
- Line 171: The displayed percentage uses the raw prop percent and can go
outside 0–100; clamp it (same way filledSegs does) before rendering or lift the
clamped value into a named variable (e.g., clampedPercent) and reuse it in the
JSX where Math.round(percent) is shown; ensure you apply Math.round to the
clamped value (Math.round(clampedPercent)) so the UI never shows values below 0%
or above 100% and remains consistent with filledSegs.
In `@src/components/monitoring/manual-check-results.tsx`:
- Around line 23-25: leaverKey currently accumulates account IDs with
duplicates; change its construction to deduplicate IDs before joining: collect
IDs from (results.results ?? []).flatMap(r => (r.leavers ?? []).map(l =>
l.player.account_id)), feed them into a Set to remove duplicates, then join(',')
to build the query string. Update the expression that defines leaverKey so it
uses the Set of IDs (from the mapped account_id values) instead of joining the
raw array.
In `@src/components/ui/player-journey-timeline.tsx`:
- Around line 265-267: The href URL for the anchor in
player-journey-timeline.tsx is built with the region ("EU") as a prefix segment
which produces a broken link; update the href construction that uses
encodeURIComponent(player.name) and player.id so the region is appended as a
trailing path segment (e.g. .../stats/{encodedName}={id}/EU) and keep
target="_blank" unchanged—locate the <a ... href={...}> that builds the
tomato.gg link and reorder the segments accordingly.
- Around line 230-241: The dialog rendered by the motion.div (role="dialog",
aria-modal="true") lacks a focus trap so keyboard users can Tab out; fix by
wrapping the dialog node with a focus trap (e.g., FocusTrap from
focus-trap-react) or implement a useEffect in the PlayerJourneyTimeline
component that on open moves focus into the dialog and listens for Tab /
Shift+Tab to cycle focus inside the modal, restoring focus on close; target the
motion.div element (key="panel") or its container, ensure the first focusable
element receives focus on mount, prevent background content from receiving focus
while open, and clean up listeners on unmount.
- Around line 54-77: The pairing logic in buildStays incorrectly matches any
subsequent leave regardless of clan; replace the findIndex call so it searches
for a leave event that also matches the join's clan (e.g. match e.type ===
'leave' && e.clan_tag === ev.clan_tag (or compare clan_name if tag may be
missing)) to ensure a join is paired with its corresponding leave; keep the same
calculation of durationMs/durationDays and the same i advancement (i = leaveIdx
? leaveIdx + 1 : events.length) but use the new leaveIdx from the clan-aware
search so stays reflect correct clan pairing.
- Line 257: Replace the raw <img> in the PlayerJourneyTimeline component with
Next.js's Image: import Image from 'next/image' (ensure the import exists at
top), then render <Image src={currentClan.emblemUrl}
alt={`[${currentClan.tag}]`} width={32} height={32} className="w-8 h-8" />; if
the emblem host isn't configured in next.config.remotePatterns, add the prop
unoptimized to Image to avoid build/runtime errors. Locate the occurrence using
currentClan.emblemUrl/currentClan.tag in player-journey-timeline.tsx and update
accordingly.
In `@src/components/ui/video-smoke.tsx`:
- Around line 18-24: The video element is still autoplaying for users who prefer
reduced motion; update the <video> element (the video tag with the className
prop) to respect prefers-reduced-motion by adding the Tailwind utility
"motion-reduce:hidden" to its className (so it is hidden when motion reduction
is requested) and optionally gate autoplay/loop by checking a
prefers-reduced-motion media query in the component before setting autoPlay/loop
attributes (modify the video element and any surrounding render logic that sets
autoPlay/loop).
In `@src/hooks/use-bulk-import.ts`:
- Around line 37-46: The code is incorrectly checking for 'text' on rawCell and
casting sheet values to primitive types, causing rich-text cells to become
"[object Object]"; update the logic in use-bulk-import.ts (around jsonData, row,
rawCell, clanTags) to stop the incorrect cast of worksheet.getSheetValues(),
detect CellRichTextValue by checking for 'richText' and extracting joined
richText elements' plainText, and still fall back to handling CellHyperlinkValue
('text' property) or primitives; ensure the runtime type checks handle objects
properly before casting so richText values produce the intended trimmed string
for clanTags.
Comment on lines
+1
to
+23
| { | ||
| "name": "WoT Clan Watcher", | ||
| "short_name": "Clan Watcher", | ||
| "description": "World of Tanks clan tracker that monitors clans and player analytics.", | ||
| "start_url": "/", | ||
| "icons": [ | ||
| { | ||
| "src": "/icons/android-chrome-192x192.png", | ||
| "sizes": "192x192", | ||
| "type": "image/png", | ||
| "purpose": "any maskable" | ||
| }, | ||
| { | ||
| "src": "/icons/android-chrome-512x512.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png", | ||
| "purpose": "any maskable" | ||
| } | ||
| ], | ||
| "theme_color": "#0b0d10", | ||
| "background_color": "#0b0d10", | ||
| "display": "standalone" | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding explicit "scope" (and optionally "id").
Without scope, browsers fall back to deriving it from start_url, so there's no functional bug here. However, to avoid issues with scope determination in this way, it's recommended to explicitly specify scope in your manifest file. Additionally, the id property lets you explicitly define the identifier used for your application, removing the dependency on start_url or the location of the manifest, making it possible to update them in the future.
✨ Proposed additions
"start_url": "/",
+ "scope": "/",
+ "id": "/",
"icons": [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| "name": "WoT Clan Watcher", | |
| "short_name": "Clan Watcher", | |
| "description": "World of Tanks clan tracker that monitors clans and player analytics.", | |
| "start_url": "/", | |
| "icons": [ | |
| { | |
| "src": "/icons/android-chrome-192x192.png", | |
| "sizes": "192x192", | |
| "type": "image/png", | |
| "purpose": "any maskable" | |
| }, | |
| { | |
| "src": "/icons/android-chrome-512x512.png", | |
| "sizes": "512x512", | |
| "type": "image/png", | |
| "purpose": "any maskable" | |
| } | |
| ], | |
| "theme_color": "#0b0d10", | |
| "background_color": "#0b0d10", | |
| "display": "standalone" | |
| } | |
| { | |
| "name": "WoT Clan Watcher", | |
| "short_name": "Clan Watcher", | |
| "description": "World of Tanks clan tracker that monitors clans and player analytics.", | |
| "start_url": "/", | |
| "scope": "/", | |
| "id": "/", | |
| "icons": [ | |
| { | |
| "src": "/icons/android-chrome-192x192.png", | |
| "sizes": "192x192", | |
| "type": "image/png", | |
| "purpose": "any maskable" | |
| }, | |
| { | |
| "src": "/icons/android-chrome-512x512.png", | |
| "sizes": "512x512", | |
| "type": "image/png", | |
| "purpose": "any maskable" | |
| } | |
| ], | |
| "theme_color": "#0b0d10", | |
| "background_color": "#0b0d10", | |
| "display": "standalone" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@public/icons/site.webmanifest` around lines 1 - 23, The manifest is missing
explicit "scope" (and optionally "id"), so add a "scope" property (e.g., "/") to
the JSON and optionally add an "id" property (e.g., "/?app=clan-watcher" or a
stable string) to site.webmanifest; update the object that currently contains
"name", "short_name", "start_url", "display", etc., by inserting "scope":
"<desired-scope>" and "id": "<stable-id>" so the browser uses the explicit
scope/id instead of deriving them from start_url.
Comment on lines
+45
to
+53
| export async function GET(request: NextRequest) { | ||
| try { | ||
| const url = new URL(request.url); | ||
| const playerIdParam = url.searchParams.get('playerId'); | ||
| const playerId = parseInt(playerIdParam || '', 10); | ||
|
|
||
| if (isNaN(playerId) || playerId <= 0) { | ||
| return badRequest('Invalid playerId'); | ||
| } |
There was a problem hiding this comment.
No authentication — route exposes DB clan history without access control.
Every other monitoring route (e.g., player-current-clans) calls withAuth. This route skips it entirely, meaning any unauthenticated client can query tracked clan history for any player ID from the DB.
🔒 Proposed fix
export async function GET(request: NextRequest) {
try {
+ const auth = await withAuth(request);
+ if (auth.error) return auth.error;
+
const url = new URL(request.url);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function GET(request: NextRequest) { | |
| try { | |
| const url = new URL(request.url); | |
| const playerIdParam = url.searchParams.get('playerId'); | |
| const playerId = parseInt(playerIdParam || '', 10); | |
| if (isNaN(playerId) || playerId <= 0) { | |
| return badRequest('Invalid playerId'); | |
| } | |
| export async function GET(request: NextRequest) { | |
| try { | |
| const auth = await withAuth(request); | |
| if (auth.error) return auth.error; | |
| const url = new URL(request.url); | |
| const playerIdParam = url.searchParams.get('playerId'); | |
| const playerId = parseInt(playerIdParam || '', 10); | |
| if (isNaN(playerId) || playerId <= 0) { | |
| return badRequest('Invalid playerId'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/player-history/route.ts` around lines 45 - 53, The GET handler
currently lacks authentication and exposes player history; modify the exported
GET function to call the existing withAuth middleware/utility (same used by
player-current-clans) before processing the request, aborting with an
unauthorized response if withAuth indicates no valid session; then only parse
playerId and proceed to DB reads (keeping badRequest checks) after auth succeeds
— reference the GET function and the withAuth helper to find where to insert the
auth check and early return on failure.
Comment on lines
+32
to
+34
| <h2 className="text-3xl md:text-4xl font-bold text-center mb-12" style={{ fontFamily: "'Oswald', 'Roboto Condensed', sans-serif" }}> | ||
| Key Features | ||
| </h2> |
There was a problem hiding this comment.
Heading hierarchy skips h3 — WCAG 1.3.1.
h2 ("Key Features") is followed directly by h4 (feature titles), skipping a level. Change the feature title heading to h3.
♿ Proposed fix
- <h4 className="text-xl font-bold text-[`#FF8C00`] mb-3 flex items-center justify-center gap-2">
+ <h3 className="text-xl font-bold text-[`#FF8C00`] mb-3 flex items-center justify-center gap-2">
{feature.title}
{feature.comingSoon && (
<span className="text-xs font-semibold px-2 py-1 bg-gradient-to-r from-[`#FF8C00`] to-[`#CC5500`] text-white rounded-full uppercase tracking-wide">
Coming Soon
</span>
)}
- </h4>
+ </h3>Also applies to: 58-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/home/features-section.tsx` around lines 32 - 34, The heading
hierarchy is incorrect: the component renders an h2 ("Key Features") followed by
h4 elements for each feature title; update those feature title elements to h3 so
the structure becomes h2 → h3 and satisfies WCAG 1.3.1. Locate the feature title
JSX (the h4 elements in the FeaturesSection component in
src/components/home/features-section.tsx — including the repeated block around
lines 58–65) and change the tag from h4 to h3, preserving the existing classes,
styles, and content.
| className="relative group" | ||
| > | ||
| {/* Camo Top Border */} | ||
| <div className="absolute top-0 left-0 right-0 h-1 bg-gradient-to-r from-[#FF8C00] via-[#CC5500] to-[#FF8C00]" /> |
There was a problem hiding this comment.
Gradient top border will have square corners against a rounded card.
The parent motion.div has no overflow-hidden and no border-radius, and the absolute gradient strip has no rounded-t-xl, so its top corners will be sharp while the card below has rounded-xl. Add rounded-t-xl to the border div.
🎨 Proposed fix
- <div className="absolute top-0 left-0 right-0 h-1 bg-gradient-to-r from-[`#FF8C00`] via-[`#CC5500`] to-[`#FF8C00`]" />
+ <div className="absolute top-0 left-0 right-0 h-1 rounded-t-xl bg-gradient-to-r from-[`#FF8C00`] via-[`#CC5500`] to-[`#FF8C00`]" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div className="absolute top-0 left-0 right-0 h-1 bg-gradient-to-r from-[#FF8C00] via-[#CC5500] to-[#FF8C00]" /> | |
| <div className="absolute top-0 left-0 right-0 h-1 rounded-t-xl bg-gradient-to-r from-[`#FF8C00`] via-[`#CC5500`] to-[`#FF8C00`]" /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/home/features-section.tsx` at line 46, The gradient top border
div (className on the absolute div) currently has sharp corners; add the
rounded-t-xl utility to its className (i.e., change the absolute gradient div's
className to include "rounded-t-xl") so its top corners match the card; also
ensure the parent motion.div (the card wrapper) has "overflow-hidden" and the
intended "rounded-xl" so the gradient is clipped and corners are consistent.
Comment on lines
+58
to
+65
| <h4 className="text-xl font-bold text-[#FF8C00] mb-3 flex items-center justify-center gap-2"> | ||
| {feature.title} | ||
| {feature.comingSoon && ( | ||
| <span className="text-xs font-semibold px-2 py-1 bg-gradient-to-r from-[#FF8C00] to-[#CC5500] text-white rounded-full uppercase tracking-wide"> | ||
| Coming Soon | ||
| </span> | ||
| )} | ||
| </h4> |
There was a problem hiding this comment.
"Coming Soon" badge is part of the heading text for screen readers.
Placing the badge <span> inside the <h3> means assistive technologies read the badge text as part of the heading. Consider rendering the badge outside the heading element (or adding aria-hidden="true" on the span and a visually-hidden companion label for SR users).
♿ Proposed fix
- <h3 className="text-xl font-bold text-[`#FF8C00`] mb-3 flex items-center justify-center gap-2">
+ <div className="mb-3 flex items-center justify-center gap-2">
+ <h3 className="text-xl font-bold text-[`#FF8C00`]">
{feature.title}
- {feature.comingSoon && (
- <span className="text-xs font-semibold px-2 py-1 bg-gradient-to-r from-[`#FF8C00`] to-[`#CC5500`] text-white rounded-full uppercase tracking-wide">
- Coming Soon
- </span>
- )}
</h3>
+ {feature.comingSoon && (
+ <span aria-label="Coming Soon" className="text-xs font-semibold px-2 py-1 bg-gradient-to-r from-[`#FF8C00`] to-[`#CC5500`] text-white rounded-full uppercase tracking-wide">
+ Coming Soon
+ </span>
+ )}
+ </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/home/features-section.tsx` around lines 58 - 65, The "Coming
Soon" badge is currently inside the <h4> that renders feature.title so screen
readers will read it as part of the heading; update the JSX in
features-section.tsx so the conditional rendering of feature.comingSoon moves
the <span> out of the <h4> (render it immediately after the <h4>) or, if you
prefer to keep it inline, add aria-hidden="true" to the <span> and render a
visually-hidden companion label for screen readers; locate the <h4> that
references feature.title and the conditional span for feature.comingSoon and
adjust their placement/attributes accordingly.
Comment on lines
+362
to
+364
| {visible.map((stay, idx) => ( | ||
| <StayNode key={idx} stay={stay} index={reversed.indexOf(stay)} /> | ||
| ))} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
key={idx} is the map index, not a stable identity.
When showAll toggles, the visible slice changes length. React uses keys to reconcile: idx within visible will be the same for the first 10 items regardless of showAll, so toggling doesn't cause wrong reconciliation here. However, if events (and thus stays) ever get filtered or reordered, index-keyed nodes will re-render incorrectly.
Use a stable composite key derived from the stay's content:
✨ Proposed fix
- <StayNode key={idx} stay={stay} index={reversed.indexOf(stay)} />
+ <StayNode key={`${stay.tag}-${stay.joinDate}`} stay={stay} index={reversed.indexOf(stay)} />Note: the same clan can appear multiple times in a player's history, so if stay.tag + stay.joinDate could collide, append the reversed.indexOf(stay) as a tiebreaker: `${stay.tag}-${stay.joinDate}-${reversed.indexOf(stay)}`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/player-journey-timeline.tsx` around lines 362 - 364,
Replace the unstable index key used in the visible.map rendering (key={idx})
with a stable composite key derived from the stay object to avoid reconciliation
bugs; update the StayNode instantiation inside visible.map to use a string like
`${stay.tag}-${stay.joinDate}-${reversed.indexOf(stay)}` (or similar unique
combination of stay.tag and stay.joinDate with reversed.indexOf(stay) as a
tiebreaker) so keys remain stable even if events/stays are filtered or
reordered.
Comment on lines
+19
to
+23
| autoPlay | ||
| loop | ||
| muted | ||
| playsInline | ||
| preload="none" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
HTML spec preload attribute autoplay video element browser behavior
💡 Result:
preloadis only a hint: The HTML Standard definespreloadas a hint about how much buffering is advisable, and explicitly says it may be ignored (e.g., due to user preferences or connectivity constraints). It also notes thatpreloadcan be changed dynamically. [1]autoplayoverridespreload(fetching-wise): The spec states that theautoplayattribute can overridepreload, because if the media is going to play, it necessarily must buffer first regardless of the preload hint. [1]autoplayis not guaranteed to actually start playback: The spec definesautoplayas “automatically begin playback … as soon as it can do so without stopping,” but also notes user agents don’t have to support autoplay and should honor user preferences. [2]- Modern browsers enforce autoplay policies (common real-world outcome):
- Chrome: muted autoplay is always allowed, but autoplay with sound is allowed only under certain conditions (user interaction, sufficient “media engagement,” install/PWA, etc.). [3]
- In general, autoplay attempts can be blocked by policy; the practical way to detect this is to attempt playback and handle failure (commonly via the
play()promise). [4]
Net effect: setting preload="none" does not reliably prevent network fetching if autoplay is present (or if the browser decides otherwise), and setting autoplay does not ensure playback will start (it may be blocked, often unless muted / user-initiated). [1][2][3][4]
Sources:
[1] WHATWG HTML Standard — Media elements (preload hint; may be ignored; autoplay can override preload)
[2] WHATWG HTML Standard — Media elements (autoplay definition; UA may honor user preferences / not support autoplay)
[3] Chrome for Developers — Autoplay policy in Chrome
[4] MDN — Autoplay guide for media and Web Audio APIs
Remove redundant preload="none" with autoPlay.
Per the HTML spec, preload is an advisory hint that browsers may ignore. When autoPlay is present, the spec explicitly allows autoplay to override preload—if the media will autoplay, it must buffer first regardless of the preload hint. The attribute provides no actual benefit here and is misleading.
🛠️ Proposed fix
<video
autoPlay
loop
muted
playsInline
- preload="none"
className="absolute bottom-0 left-0 w-full h-full object-cover opacity-30"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| autoPlay | |
| loop | |
| muted | |
| playsInline | |
| preload="none" | |
| <video | |
| autoPlay | |
| loop | |
| muted | |
| playsInline | |
| className="absolute bottom-0 left-0 w-full h-full object-cover opacity-30" | |
| style={{ | |
| mixBlendMode: 'screen', | |
| filter: 'contrast(0.9) brightness(0.5) blur(1px)', | |
| }} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/video-smoke.tsx` around lines 19 - 23, The video element in
VideoSmoke (src/components/ui/video-smoke.tsx) includes a redundant
preload="none" while already using autoPlay; remove the preload="none" attribute
from the JSX for the video element (where attributes autoPlay, loop, muted,
playsInline are set) so autoplay behavior isn't contradicted or misleading, and
verify no other logic depends on that attribute.
Comment on lines
+36
to
+37
| const worksheet = workbook.worksheets[0]; | ||
| const jsonData = worksheet.getSheetValues() as (string | number | boolean | null)[][]; |
There was a problem hiding this comment.
Missing null guard on worksheets[0].
If the uploaded file has no worksheets, workbook.worksheets[0] is undefined and .getSheetValues() throws, producing the generic "Failed to process Excel file" toast — no actionable feedback for the user.
🛡️ Proposed fix
const worksheet = workbook.worksheets[0];
+ if (!worksheet) {
+ toast.error('Excel file contains no worksheets.');
+ return;
+ }
const jsonData = worksheet.getSheetValues() as unknown[][];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const worksheet = workbook.worksheets[0]; | |
| const jsonData = worksheet.getSheetValues() as (string | number | boolean | null)[][]; | |
| const worksheet = workbook.worksheets[0]; | |
| if (!worksheet) { | |
| toast.error('Excel file contains no worksheets.'); | |
| return; | |
| } | |
| const jsonData = worksheet.getSheetValues() as unknown[][]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/use-bulk-import.ts` around lines 36 - 37, The code accesses
workbook.worksheets[0] without checking for an empty worksheets array, so
worksheet may be undefined and calling worksheet.getSheetValues() will throw;
update the logic in useBulkImport to guard workbook.worksheets (e.g., check
workbook.worksheets.length > 0 or if (!worksheet) ) before calling
getSheetValues(), and handle the empty case by returning early or throwing a
clear error / triggering the user-facing toast with a specific message like
"Uploaded Excel file contains no worksheets" so jsonData is only computed when
worksheet is defined (referencing worksheet, workbook.worksheets, jsonData, and
getSheetValues to locate the change).
| // Better Auth configuration with D1 database using Kysely | ||
| export const auth = betterAuth({ | ||
| baseURL: process.env.NEXT_PUBLIC_APP_URL || "http://localhost:3000", | ||
| secret: process.env.BETTER_AUTH_SECRET ?? (() => { if (process.env.NODE_ENV === 'production') throw new Error('BETTER_AUTH_SECRET is required in production'); return 'dev-placeholder'; })(), |
There was a problem hiding this comment.
?? does not guard against BETTER_AUTH_SECRET="".
Nullish coalescing only short-circuits on null/undefined. An empty-string secret bypasses the IIFE entirely and silently becomes the session-signing key — every session token is then forgeable with a known (empty) secret.
Replace ?? with a truthiness check:
🔒 Proposed fix
- secret: process.env.BETTER_AUTH_SECRET ?? (() => { if (process.env.NODE_ENV === 'production') throw new Error('BETTER_AUTH_SECRET is required in production'); return 'dev-placeholder'; })(),
+ secret: (() => {
+ const s = process.env.BETTER_AUTH_SECRET;
+ if (!s) {
+ if (process.env.NODE_ENV === 'production') {
+ throw new Error('BETTER_AUTH_SECRET is required in production');
+ }
+ return 'dev-placeholder';
+ }
+ return s;
+ })(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| secret: process.env.BETTER_AUTH_SECRET ?? (() => { if (process.env.NODE_ENV === 'production') throw new Error('BETTER_AUTH_SECRET is required in production'); return 'dev-placeholder'; })(), | |
| secret: (() => { | |
| const s = process.env.BETTER_AUTH_SECRET; | |
| if (!s) { | |
| if (process.env.NODE_ENV === 'production') { | |
| throw new Error('BETTER_AUTH_SECRET is required in production'); | |
| } | |
| return 'dev-placeholder'; | |
| } | |
| return s; | |
| })(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/auth.ts` at line 9, The current secret initialization uses nullish
coalescing (process.env.BETTER_AUTH_SECRET ?? ...) which treats an empty string
as a valid secret; change it to a truthiness check so empty-string values fall
through to the IIFE. Update the secret assignment in src/lib/auth.ts (the secret
property initialization) to use a logical OR or explicit Boolean check (e.g.,
process.env.BETTER_AUTH_SECRET || (() => { if (process.env.NODE_ENV ===
'production') throw new Error('BETTER_AUTH_SECRET is required in production');
return 'dev-placeholder'; })()) so production still throws and local dev gets
the placeholder while empty strings are rejected.
Comment on lines
+233
to
+249
| async getClanEmblems(clanIds: number[]): Promise<Record<number, string | null>> { | ||
| if (clanIds.length === 0) return {}; | ||
| try { | ||
| const data = await this.makeRequest('/wgn/clans/info/', { | ||
| clan_id: clanIds.join(','), | ||
| fields: 'emblems', | ||
| }); | ||
| const result: Record<number, string | null> = {}; | ||
| for (const id of clanIds) { | ||
| const entry = data?.[id.toString()]; | ||
| result[id] = entry?.emblems?.x64?.portal ?? entry?.emblems?.x32?.portal ?? null; | ||
| } | ||
| return result; | ||
| } catch { | ||
| return {}; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Find all call sites of getClanEmblems and getClanRatings
rg -nP --type=ts '\b(getClanEmblems|getClanRatings)\s*\(' -B2 -A2Repository: ionutcnu/Watcher
Length of output: 948
🏁 Script executed:
# Find the getClanMembersStats implementation to see how batching is done
rg -A30 'getClanMembersStats' src/lib/wargaming-api.tsRepository: ionutcnu/Watcher
Length of output: 1221
🏁 Script executed:
# Search for any comments/documentation about the 100-ID constraint
rg -i '100|batch|constraint|limit' src/lib/wargaming-api.tsRepository: ionutcnu/Watcher
Length of output: 496
🏁 Script executed:
# Get the overall structure of the file to understand context
wc -l src/lib/wargaming-api.ts && head -50 src/lib/wargaming-api.tsRepository: ionutcnu/Watcher
Length of output: 1130
🏁 Script executed:
# Search for getClanRatings usage more broadly
rg 'getClanRatings' -B3 -A3Repository: ionutcnu/Watcher
Length of output: 452
🏁 Script executed:
# Check the clan-emblem route to see what clanIds are passed
cat -n src/app/api/clan-emblem/route.tsRepository: ionutcnu/Watcher
Length of output: 1126
🏁 Script executed:
# Search for any WGN endpoint batch limits mentioned in comments
rg -i 'wgn|batch|100' src/lib/wargaming-api.ts | head -20Repository: ionutcnu/Watcher
Length of output: 586
🏁 Script executed:
# Check if there are any other endpoints that explicitly mention batch limits
rg -i 'batch|limit|100' src/lib/wargaming-api.ts | grep -i batchRepository: ionutcnu/Watcher
Length of output: 237
🏁 Script executed:
# Search for getClanRatings usage across entire codebase
rg 'getClanRatings' -B5 -A5Repository: ionutcnu/Watcher
Length of output: 762
🏁 Script executed:
# Check if there are other potential callers of getClanEmblems
rg 'getClanEmblems' -B5 -A5Repository: ionutcnu/Watcher
Length of output: 1372
🏁 Script executed:
# Look at the full getClanRatings implementation
sed -n '214,231p' src/lib/wargaming-api.tsRepository: ionutcnu/Watcher
Length of output: 703
🏁 Script executed:
# Search for any API documentation or comments about /wot/clanratings endpoints
rg -i 'clanratings|wot/clanratings' -B2 -A2Repository: ionutcnu/Watcher
Length of output: 6180
🏁 Script executed:
# Check if there's a README or docs mentioning API constraints
find . -name "README*" -o -name "*.md" | head -5 | xargs grep -l "100\|batch\|limit" 2>/dev/null || echo "No matches found"Repository: ionutcnu/Watcher
Length of output: 71
🏁 Script executed:
# Find the clan-ratings API route
find . -path "*/api/clan-ratings*" -type fRepository: ionutcnu/Watcher
Length of output: 95
🏁 Script executed:
# If not found, search for clan-ratings route
rg 'clan-ratings' -B5 -A10Repository: ionutcnu/Watcher
Length of output: 1027
🏁 Script executed:
# Check monitoring page more carefully for how many IDs are sent
rg -A20 'clan-ratings\?clan_ids' src/app/monitoring/page.tsxRepository: ionutcnu/Watcher
Length of output: 757
🏁 Script executed:
# Check the clan-ratings route to see if it validates input
cat -n src/app/api/clan-ratings/route.tsRepository: ionutcnu/Watcher
Length of output: 1978
getClanEmblems lacks self-contained batching; consider adding defensive limits.
The /wgn/clans/info/ endpoint likely shares the same 100-ID-per-request constraint as other WOT endpoints. getClanEmblems lacks built-in batching and relies on the route handler (/api/clan-emblem) to enforce the limit. If this method is called directly from other code paths, inputs >100 IDs would fail silently in the catch block. getClanMembersStats batches explicitly and is more resilient.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/wargaming-api.ts` around lines 233 - 249, getClanEmblems currently
sends all clanIds at once and swallows failures; change it to batch clanIds into
chunks of max 100 (same approach as getClanMembersStats), call
this.makeRequest('/wgn/clans/info/', { clan_id: chunk.join(','), fields:
'emblems' }) for each chunk, merge responses into the final
Record<number,string|null> (using the same entry?.emblems?.x64?.portal fallback
logic), and do not silently swallow errors—propagate or rethrow the caught error
so callers can handle it instead of returning an empty object.
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.
Complete UI overhaul with WoT theme, animated landing page, admin and monitoring layouts. Add player journey timeline with Tomato.gg API, live current clan lookup in battle report rows and tooltips via WG API.