feat: Plugin football #2461
feat: Plugin football #2461
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @suleigolden! Welcome to the elizaOS community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now an elizaOS contributor!
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces a new football plugin ( Changes
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (7)
packages/plugin-football/src/types.ts (1)
1-10: Enhance type definitions for better type safetyConsider making the types more specific:
+export type MatchStatus = 'SCHEDULED' | 'LIVE' | 'FINISHED' | 'POSTPONED' | 'CANCELLED'; + +export type MatchEvent = { + type: 'GOAL' | 'YELLOW_CARD' | 'RED_CARD' | 'SUBSTITUTION'; + minute: number; + team: string; + player: string; +}; + export type MatchData = { league: string; matches: Array<{ homeTeam: string; awayTeam: string; - score: string; - status: string; - events: string[]; + score: { home: number; away: number }; + status: MatchStatus; + events: MatchEvent[]; }>; };packages/plugin-football/tsup.config.ts (1)
8-8: Fix misleading comment about module formatThe comment suggests targeting CommonJS but the format is set to ESM.
- format: ["esm"], // Ensure you're targeting CommonJS + format: ["esm"], // Output as ECMAScript modulespackages/plugin-football/src/actions/fetchStandingsAction.ts (1)
53-95: Update example responses to be more realisticThe example responses contain hard-coded team positions that might become outdated.
Consider using more generic examples that focus on the response format rather than specific team positions.
packages/plugin-football/src/tests/fetch-standings-action.test.ts (2)
40-90: Enhance test coverage for standings data validation.The successful fetch test case could be more comprehensive:
- Add assertions for the structure of the standings data
- Test edge cases like empty standings or malformed team data
it("should fetch standings successfully", async () => { global.fetch = vi.fn(() => Promise.resolve({ ok: true, json: () => Promise.resolve({ standings: [ { table: [ { position: 1, team: { name: "Manchester City" }, points: 45, + played: 20, + won: 14, + draw: 3, + lost: 3, + goalsFor: 45, + goalsAgainst: 21, }, ], }, ], }), }) ) as any; const result = await fetchStandingsAction.handler( mockRuntime, {} as Memory, {} as State ); expect(result).toEqual({ standings: [ { table: [ { position: 1, team: { name: "Manchester City" }, points: 45, + played: 20, + won: 14, + draw: 3, + lost: 3, + goalsFor: 45, + goalsAgainst: 21, }, ], }, ], }); + // Validate data structure + expect(result.standings[0].table[0]).toHaveProperty('played'); + expect(result.standings[0].table[0]).toHaveProperty('won'); + expect(result.standings[0].table[0]).toHaveProperty('lost'); }); +it("should handle empty standings gracefully", async () => { + global.fetch = vi.fn(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ standings: [] }) + }) + ) as any; + + const result = await fetchStandingsAction.handler( + mockRuntime, + {} as Memory, + {} as State + ); + expect(result.standings).toEqual([]); +});
92-106: Enhance error handling test coverage.The error handling test should verify the error logging functionality and test different error scenarios.
it("should handle fetch errors gracefully", async () => { + const errorSpy = vi.spyOn(elizaLogger, 'error'); global.fetch = vi.fn(() => Promise.resolve({ ok: false, - statusText: "Internal Server Error", + status: 500, + statusText: "Internal Server Error" }) ) as any; const result = await fetchStandingsAction.handler( mockRuntime, {} as Memory, {} as State ); expect(result).toBe(false); + expect(errorSpy).toHaveBeenCalledWith( + "Error fetching standings data:", + "Internal Server Error" + ); }); +it("should handle network errors", async () => { + global.fetch = vi.fn(() => Promise.reject(new Error("Network error"))); + + const result = await fetchStandingsAction.handler( + mockRuntime, + {} as Memory, + {} as State + ); + expect(result).toBe(false); +});packages/plugin-football/src/tests/match-action.test.ts (1)
27-38: Extract mock data to a shared fixture file.The mock response structure is duplicated across test files. Consider extracting it to a shared fixture.
+// __fixtures__/mockResponses.ts +export const mockMatchData = { + matches: [ + { + homeTeam: { name: "Chelsea" }, + awayTeam: { name: "Arsenal" }, + score: { + fullTime: { homeTeam: 1, awayTeam: 2 }, + }, + }, + ], +}; -const mockResponse = { - matches: [ - { - homeTeam: { name: "Chelsea" }, - awayTeam: { name: "Arsenal" }, - score: { - fullTime: { homeTeam: 1, awayTeam: 2 }, - }, - }, - ], -}; +import { mockMatchData } from '../__fixtures__/mockResponses'; +const mockResponse = mockMatchData;packages/plugin-football/package.json (1)
2-3: Use semantic versioning formatThe version
0.1.8+build.1uses a non-standard format. Consider using standard semver format like0.1.8.- "version": "0.1.8+build.1", + "version": "0.1.8"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
agent/package.json(2 hunks)agent/src/index.ts(3 hunks)packages/plugin-football/.npmignore(1 hunks)packages/plugin-football/README.md(1 hunks)packages/plugin-football/eslint.config.mjs(1 hunks)packages/plugin-football/package.json(1 hunks)packages/plugin-football/src/actions/fetchMatchAction.ts(1 hunks)packages/plugin-football/src/actions/fetchStandingsAction.ts(1 hunks)packages/plugin-football/src/index.ts(1 hunks)packages/plugin-football/src/tests/fetch-standings-action.test.ts(1 hunks)packages/plugin-football/src/tests/match-action.test.ts(1 hunks)packages/plugin-football/src/types.ts(1 hunks)packages/plugin-football/tsconfig.json(1 hunks)packages/plugin-football/tsup.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/plugin-football/eslint.config.mjs
- packages/plugin-football/tsconfig.json
- packages/plugin-football/.npmignore
🧰 Additional context used
🪛 LanguageTool
packages/plugin-football/README.md
[misspelling] ~125-~125: This word is normally spelled as one.
Context: ... 4. Goal System Improvements - Multi-step goal planning - Goal dependency tra...
(EN_COMPOUNDS_MULTI_STEP)
🪛 Markdownlint (0.37.0)
packages/plugin-football/README.md
37-37: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
38-38: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
39-39: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
40-40: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (1)
agent/package.json (1)
101-101: LGTM!The plugin dependency is correctly added following the workspace pattern used for other plugins.
…ity and validation updates - Added 5-second timeout using AbortController for fetchMatchAction. - Implemented response validation to ensure data structure matches expected types. - Improved error handling with detailed logs for better debugging. - Removed .ts extensions from imports in index.ts to prevent production build issues. - Cleaned up tsup.config.ts by removing unused externals (@reflink/reflink and @node-llama-cpp). - Guarded against invalid API responses to enhance type safety.
|
@suleigolden please give access to push to your branch / resolve conflicts |
Head branch was pushed to by a user without write access
Relates to
No specific issue or ticket linked.
Risks
Low Risk:
plugin-footballpackage, specifically the fetching of football data and its corresponding tests.Background
What does this PR do?
This PR introduces the
plugin-football, which integrates with the Football-Data.org API to provide live football match data and league standings. It includes:fetchMatchAction: Retrieves live match scores, teams, and key events.fetchStandingsAction: Retrieves league standings for a specific competition.fetchMatchActionandfetchStandingsActionusing Vitest.What kind of change is this?
Documentation changes needed?
The documentation should include:
How to configure the Football-Data.org API key in the
.envfile.Examples of how to use
fetchMatchActionandfetchStandingsAction.My changes do not require a change to the project documentation.
Testing
Where should a reviewer start?
fetchMatchActionimplementation inpackages/plugin-football/src/actions/fetchMatchAction.ts.fetchStandingsActionimplementation inpackages/plugin-football/src/actions/fetchStandingsAction.ts.packages/plugin-football/src/tests/match-action.test.ts.Detailed testing steps
Setup:
.envfile contains the correctFOOTBALL_API_KEY.pnpm install.Run Tests:
Manual Verification:
fetchMatchActionto fetch live match data from the Football-Data.org API.fetchStandingsActionto retrieve league standings and verify the output.Screenshots
Before
N/A (New feature)
After
N/A (New feature)
Deploy Notes
No special deployment steps are required. Ensure the
.envfile includes theFOOTBALL_API_KEY.Database changes
None.
Deployment instructions
FOOTBALL_API_KEYto the.envfile in the deployment environment.Discord username
N/A