Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryAdds the Confidence Score: 5/5Safe to merge for review purposes; all three remaining findings are P2 and do not block the primary user path. All P0/P1 concerns from the previous review round are resolved. The three new findings are P2: an architectural boundary nit (api/ import that works today but should move to shared/), a missing inFlight dedup in an interactive popup path (causes redundant fetches, not data loss), and missing query exclusions in server-side digest feeds (only affects the digest endpoint, not the main UI). src/services/sports.ts (api/ boundary import + inFlight dedup), server/worldmonitor/news/v1/_feeds.ts (missing political exclusions in digest feeds) Important Files Changed
Reviews (2): Last reviewed commit: "feat(sport): make sports fixture date mo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| REFRESH_INTERVALS.cotPositioning, | ||
| () => this.isPanelNearViewport('cot-positioning') | ||
| ); | ||
| this.refreshScheduler.scheduleRefresh( | ||
| 'sports-fixtures', | ||
| () => (this.state.panels['sports-fixtures'] as SportsFixturesPanel).fetchData(), | ||
| REFRESH_INTERVALS.sports, | ||
| () => this.isPanelNearViewport('sports-fixtures') | ||
| ); | ||
| this.refreshScheduler.scheduleRefresh( | ||
| 'sports-tables', | ||
| () => (this.state.panels['sports-tables'] as SportsTablesPanel).fetchData(), | ||
| REFRESH_INTERVALS.sports, | ||
| () => this.isPanelNearViewport('sports-tables') | ||
| ); | ||
| this.refreshScheduler.scheduleRefresh( | ||
| 'sports-stats', | ||
| () => (this.state.panels['sports-stats'] as SportsStatsPanel).fetchData(), | ||
| REFRESH_INTERVALS.sports, | ||
| () => this.isPanelNearViewport('sports-stats') | ||
| ); | ||
| this.refreshScheduler.scheduleRefresh( | ||
| 'sports-tournaments', | ||
| () => (this.state.panels['sports-tournaments'] as SportsMajorTournamentsPanel).fetchData(), | ||
| REFRESH_INTERVALS.sports, | ||
| () => this.isPanelNearViewport('sports-tournaments') | ||
| ); | ||
| this.refreshScheduler.scheduleRefresh( | ||
| 'sports-nba', | ||
| () => (this.state.panels['sports-nba'] as SportsNbaPanel).fetchData(), | ||
| REFRESH_INTERVALS.sports, | ||
| () => this.isPanelNearViewport('sports-nba') | ||
| ); | ||
| this.refreshScheduler.scheduleRefresh( | ||
| 'sports-motorsport-standings', | ||
| () => (this.state.panels['sports-motorsport-standings'] as SportsMotorsportPanel).fetchData(), | ||
| REFRESH_INTERVALS.sports, | ||
| () => this.isPanelNearViewport('sports-motorsport-standings') | ||
| ); | ||
| this.refreshScheduler.scheduleRefresh( | ||
| 'sports-transfers', | ||
| () => (this.state.panels['sports-transfers'] as SportsTransferNewsPanel).fetchData(), | ||
| REFRESH_INTERVALS.sports, | ||
| () => this.isPanelNearViewport('sports-transfers') | ||
| ); | ||
|
|
||
| // Refresh intelligence signals for CII (geopolitical variant only) | ||
| if (SITE_VARIANT === 'full') { |
There was a problem hiding this comment.
These 7 scheduleRefresh calls are added outside any SITE_VARIANT === 'sports' guard, so they register timers on every variant (full, tech, finance, etc.). The visibility check (isPanelNearViewport) will return false for missing panels, so no crash occurs, but it wastes scheduler slots and diverges from the explicit if (!skipsGeneralRefreshes) pattern used two dozen lines above for the equivalent conditional block.
| REFRESH_INTERVALS.cotPositioning, | |
| () => this.isPanelNearViewport('cot-positioning') | |
| ); | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-fixtures', | |
| () => (this.state.panels['sports-fixtures'] as SportsFixturesPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-fixtures') | |
| ); | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-tables', | |
| () => (this.state.panels['sports-tables'] as SportsTablesPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-tables') | |
| ); | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-stats', | |
| () => (this.state.panels['sports-stats'] as SportsStatsPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-stats') | |
| ); | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-tournaments', | |
| () => (this.state.panels['sports-tournaments'] as SportsMajorTournamentsPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-tournaments') | |
| ); | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-nba', | |
| () => (this.state.panels['sports-nba'] as SportsNbaPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-nba') | |
| ); | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-motorsport-standings', | |
| () => (this.state.panels['sports-motorsport-standings'] as SportsMotorsportPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-motorsport-standings') | |
| ); | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-transfers', | |
| () => (this.state.panels['sports-transfers'] as SportsTransferNewsPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-transfers') | |
| ); | |
| // Refresh intelligence signals for CII (geopolitical variant only) | |
| if (SITE_VARIANT === 'full') { | |
| if (SITE_VARIANT === 'sports') { | |
| this.refreshScheduler.scheduleRefresh( | |
| 'sports-fixtures', | |
| () => (this.state.panels['sports-fixtures'] as SportsFixturesPanel).fetchData(), | |
| REFRESH_INTERVALS.sports, | |
| () => this.isPanelNearViewport('sports-fixtures') | |
| ); | |
| // … remaining six sports schedule calls | |
| } |
| function extractEspnFittState(html: string): Record<string, unknown> | null { | ||
| const match = html.match(/window\['__espnfitt__'\]=(\{.*?\});<\/script>/s); | ||
| if (!match?.[1]) return null; | ||
|
|
||
| try { | ||
| const parsed = JSON.parse(match[1]); | ||
| return isRecord(parsed) ? parsed : null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
/window\['__espnfitt__'\]=(\{.*?\});<\/script>/s uses non-greedy dotAll matching to pull the inline JSON bundle. If ESPN ever wraps the assignment in a self-executing function, adds a newline before </script>, or escapes a };</script> substring inside a JSON string value, the match will fail silently and fetchNbaStandingsData returns null. A more robust approach would look for the known key boundary and use JSON.parse with a structured try/catch, or switch to the documented ESPN API endpoint that returns JSON directly.
vite.config.ts
Outdated
| }; | ||
| } | ||
|
|
||
| function sportsDataProxyPlugin(): Plugin { | ||
| const PROVIDERS = { | ||
| thesportsdb: { | ||
| baseUrl: 'https://www.thesportsdb.com/api/v1/json/123', | ||
| endpoints: new Set([ | ||
| '/all_leagues.php', | ||
| '/lookupleague.php', | ||
| '/search_all_seasons.php', |
There was a problem hiding this comment.
vite.config.ts and api/sports-data.js
The entire PROVIDERS table — with all endpoints, TTLs, and allowed params — is defined twice: once here for the local dev proxy and once verbatim in api/sports-data.js for the edge function. When a new endpoint is added to one file (e.g. a new ESPN league path), it's easy to forget to update the other. Consider extracting the shared allowlist into a shared/sports-providers.json (or a generated constant) that both consume, similar to how shared/ is already used for other config fixtures.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a51754921
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/App.ts
Outdated
| if (shouldPrime('sports-fixtures')) { | ||
| const panel = this.state.panels['sports-fixtures'] as SportsFixturesPanel | undefined; | ||
| if (panel) primeTask('sports-fixtures', () => panel.fetchData()); | ||
| } |
There was a problem hiding this comment.
These sports primeTask calls are unconditional once shouldPrime is true, and App.init() invokes primeVisiblePanelData(true), which makes shouldPrime(...) true for all panels at startup. That means non-sports variants now eagerly fetch sports fixtures/tables/stats/etc. even when those panels are not part of the active variant, adding avoidable startup latency and upstream traffic. Wrap the sports priming block in a SITE_VARIANT === 'sports' guard (or avoid forcing these IDs when forceAll is used).
Useful? React with 👍 / 👎.
src/services/sports.ts
Outdated
| const rawPayload = await fetchEspnSiteJson<Record<string, unknown>>(buildEspnSiteScoreboardPath(spec), 5 * 60 * 1000); | ||
| const events = await fetchEspnCompetitionEvents(spec); |
There was a problem hiding this comment.
This function fetches the same ESPN scoreboard twice: once into rawPayload and immediately again via fetchEspnCompetitionEvents(spec). Besides doubling request cost and latency, it adds a second failure point where the function can fail even though the first scoreboard request already succeeded. Parse events from rawPayload (or pass it to the helper) so the panel only depends on a single upstream call.
Useful? React with 👍 / 👎.
Combined ReviewValidated in isolated worktree: P1 (Blocking)Sports panel priming and refresh not variant-gated Fix: wrap both sections in Feed curation is not production-grade
P2 (Fix Required)Double ESPN scoreboard fetch in Dev/prod divergence for invalid provider Provider/allowlist table duplicated Edge proxy violates gold standard architecture ESPN HTML scraping is fragile P3 (Suggestions)F1 team aliases stale for 2026 Map not earning priority-1 slot Config duplication Large files
What's Good
Direction
|
# Conflicts: # src/App.ts # src/app/data-loader.ts # src/app/panel-layout.ts # src/components/index.ts # src/config/variants/base.ts
|
did the improvements, ur call if u want sports variant for now or later in the future. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
2 participants
Summary
V1 of sports news , I need it to be reviewed , if code is good (good architecture) then I add my changes, no need for merge now
Adds a new
sportsvariant with dedicated sports headlines, fixtures, league tables, major tournament views, player search, and transfer news. Also adds the sports data edge proxy, sports-specific feed wiring on the client and server, variant/build metadata, and guardrail coverage for the new digest and panel paths.todo
-add other famous sports (cricket - etc)
-add live feeds
-for the map I need ur help , im thinking of removing it and add interactive ui for the users , or keep and add layers to it
Type of change
Affected areas
/api/*)Checklist
npm run typecheck)Local validation
npm run typechecknpm run typecheck:apinpm run build:sportsnpm run test:datanode --test tests/panel-config-guardrails.test.mjs tests/sports-digest-guardrails.test.mjs