Feat/event teams players - #45
Merged
Merged
Conversation
Add a `players` list to each `Team` returned by `event.teams()`, containing each player's `name` and `id` parsed from the event page. The field is optional and defaults to `[]` when roster data is unavailable. Introduce a new `TeamPlayer` model exposed from the `_event.teams` submodule, and update documentation and changelog accordingly.
📝 WalkthroughWalkthroughThe release adds ChangesEvent API updates
Release and official documentation updates
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
| Filename | Overview |
|---|---|
| src/vlrdevapi/_event/teams/parser.py | Parses player anchors from event team cards and attaches validated player IDs and names to each team. |
| src/vlrdevapi/_event/teams/models.py | Adds the TeamPlayer model and an independently allocated default player list to Team. |
| tests/event/teams/test_parser.py | Adds roster coverage, but the tests still depend on an ignored fixture absent from clean checkouts. |
| official-docs/components/search.tsx | Switches search creation to zbsearch, which is now declared as a direct application dependency. |
| official-docs/package.json | Declares zbsearch directly and updates the documentation application's dependency versions. |
Reviews (3): Last reviewed commit: "docs: correct stage parameter in teams e..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+8
to
+10
| return HTMLParser( | ||
| load_fixture("event", "2682_vct-2026-americas-kickoff", "overview.html") | ||
| ) |
There was a problem hiding this comment.
When the event-team tests run in a clean checkout, _fixture() requests an HTML file under the ignored and untracked tests/test_html directory, causing load_fixture to raise FileNotFoundError before the roster assertions execute.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/event/teams/test_parser.py
Line: 8-10
Comment:
**Ignored fixture breaks roster tests**
When the event-team tests run in a clean checkout, `_fixture()` requests an HTML file under the ignored and untracked `tests/test_html` directory, causing `load_fixture` to raise `FileNotFoundError` before the roster assertions execute.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| } from 'fumadocs-ui/components/dialog/search'; | ||
| import { useDocsSearch } from 'fumadocs-core/search/client'; | ||
| import { create } from '@orama/orama'; | ||
| import { create } from 'zbsearch'; |
There was a problem hiding this comment.
The application now imports zbsearch directly, but package.json does not declare it and the lockfile installs it only through another package. This couples the docs build to Fumadocs' private dependency graph, so a valid dependency refresh that stops hoisting zbsearch will leave this import unresolved.
Prompt To Fix With AI
This is a comment left during a code review.
Path: official-docs/components/search.tsx
Line: 14
Comment:
**Search relies on transitive dependency**
The application now imports `zbsearch` directly, but `package.json` does not declare it and the lockfile installs it only through another package. This couples the docs build to Fumadocs' private dependency graph, so a valid dependency refresh that stops hoisting `zbsearch` will leave this import unresolved.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Deploying vlrdevapi with
|
| Latest commit: |
8af8cae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7fb2f7bb.vlrdevapi.pages.dev |
| Branch Preview URL: | https://feat-event-teams-players.vlrdevapi.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@official-docs/package.json`:
- Around line 38-41: Add zbsearch to the runtime dependencies in
official-docs/package.json to match the import in the search component, then
regenerate the package manager lockfile so the dependency is fully recorded.
In `@src/vlrdevapi/_event/namespace.py`:
- Around line 102-104: Update the Team.players documentation to state that it is
always-present and may be an empty list when roster data is unavailable. Apply
this wording at src/vlrdevapi/_event/namespace.py lines 102-104 and 245, and
src/vlrdevapi/_event/teams/namespace.py lines 46-48; replace any “optional”
wording accordingly.
In `@src/vlrdevapi/_event/teams/parser.py`:
- Around line 93-100: Update the player parsing logic around the
href-to-player_id conversion and item.text call to reject non-positive IDs and
blank names before constructing TeamPlayer. Return None when player_id is less
than or equal to zero or when the stripped name is empty, while preserving the
existing handling of malformed href values.
In `@tests/event/teams/test_parser.py`:
- Around line 35-39: Update test_parse_team_no_players_defaults_empty to use a
fixture containing a minimal team card with no .event-team-players-item
elements, then assert the parsed team’s players value equals an empty list.
Ensure the test verifies that specific playerless team rather than only checking
the existing fixture teams’ types.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 38984d9d-4203-478a-8fb6-fec724836b8a
⛔ Files ignored due to path filters (2)
official-docs/package-lock.jsonis excluded by!**/package-lock.jsonuv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
CHANGELOG.mddocs-py/docs/changelog.mdofficial-docs/app/(home)/changelog/page.tsxofficial-docs/components/analytics.tsxofficial-docs/components/home/terminal-demo.tsxofficial-docs/components/search.tsxofficial-docs/content/docs/reference/events/teams.mdxofficial-docs/package.jsonpyproject.tomlsrc/vlrdevapi/__init__.pysrc/vlrdevapi/_event/namespace.pysrc/vlrdevapi/_event/teams/__init__.pysrc/vlrdevapi/_event/teams/models.pysrc/vlrdevapi/_event/teams/namespace.pysrc/vlrdevapi/_event/teams/parser.pytests/event/teams/test_filtering.pytests/event/teams/test_parser.py
Comment on lines
+93
to
+100
| try: | ||
| id_str = href.split("/player/")[1].split("/")[0] | ||
| player_id = int(id_str) | ||
| except (ValueError, IndexError): | ||
| return None | ||
|
|
||
| name = item.text(strip=True) | ||
| return TeamPlayer(id=player_id, name=name) No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject invalid player IDs and blank player names.
int() accepts 0 and negative values. item.text(strip=True) can also be empty. Skip these entries before creating TeamPlayer, so the parser preserves its positive-ID and non-empty-name contract.
Proposed fix
try:
id_str = href.split("/player/")[1].split("/")[0]
player_id = int(id_str)
except (ValueError, IndexError):
return None
name = item.text(strip=True)
+ if player_id <= 0 or not name:
+ return None
return TeamPlayer(id=player_id, name=name)📝 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
| try: | |
| id_str = href.split("/player/")[1].split("/")[0] | |
| player_id = int(id_str) | |
| except (ValueError, IndexError): | |
| return None | |
| name = item.text(strip=True) | |
| return TeamPlayer(id=player_id, name=name) | |
| try: | |
| id_str = href.split("/player/")[1].split("/")[0] | |
| player_id = int(id_str) | |
| except (ValueError, IndexError): | |
| return None | |
| name = item.text(strip=True) | |
| if player_id <= 0 or not name: | |
| return None | |
| return TeamPlayer(id=player_id, name=name) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vlrdevapi/_event/teams/parser.py` around lines 93 - 100, Update the
player parsing logic around the href-to-player_id conversion and item.text call
to reject non-positive IDs and blank names before constructing TeamPlayer.
Return None when player_id is less than or equal to zero or when the stripped
name is empty, while preserving the existing handling of malformed href values.
Comment on lines
+35
to
+39
| def test_parse_team_no_players_defaults_empty(): | ||
| teams = parse_teams(_fixture()) | ||
| # players is optional; even if a team somehow lacks any, it should default to []. | ||
| for team in teams: | ||
| assert isinstance(team.players, list) No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test an actual team without player elements.
This test passes even when every fixture team has players. Add a minimal team card with no .event-team-players-item elements and assert that parsing returns players == [].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/event/teams/test_parser.py` around lines 35 - 39, Update
test_parse_team_no_players_defaults_empty to use a fixture containing a minimal
team card with no .event-team-players-item elements, then assert the parsed
team’s players value equals an empty list. Ensure the test verifies that
specific playerless team rather than only checking the existing fixture teams’
types.
Update documentation and changelog to state that the `players` field on each `Team` is always present, defaulting to an empty list when roster data is unavailable. Also add `zbsearch` dependency to the official-docs package.
Events list dates are rendered without a year on vlr.gg. These now use a sentinel year of 2019 instead of the current year, and any parsed date with year < 2020 indicates the source omitted the year. Also, datetime.strptime calls that parse day-of-month without a year now supply an explicit year derived from context (reference date, page-header year, or sentinel) to avoid DeprecationWarning and future breakage in Python 3.15.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vlrdevapi/commons/timezone.py (1)
245-261: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftResolve the displayed date year from the local timestamp.
local_date.replace(year=canonical.year)can assign the UTC year to a local date on the other side of December 31/January 1. That makes the offset span nearly a year andtimezone(offset)rejects it, so timezone detection falls back to_system_iana_timezone(). Compute the year around the stored UTC timestamp, and try adjacent candidate dates if the offset is invalid. Add a December/January regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vlrdevapi/commons/timezone.py` around lines 245 - 261, The displayed date parsing in the timezone-detection flow must resolve the year relative to the stored UTC timestamp rather than always using canonical.year. Update the surrounding date-resolution logic and its caller to try the relevant adjacent-year candidate dates, selecting one whose offset is valid before falling back to _system_iana_timezone(); preserve existing parsing behavior for dates that include a year. Add a regression test covering a December/January boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@official-docs/app/`(home)/changelog/page.tsx:
- Around line 10-12: Update the version rendering in the changelog timeline and
mobile heading around the `v{v.version}` expressions so the `Unreleased` entry
is displayed without the `v` prefix while released versions retain it. Use the
existing version value to conditionally omit the prefix only for `Unreleased`.
In `@src/vlrdevapi/_event/list/models.py`:
- Around line 25-34: Update the start_date and end_date field descriptions in
src/vlrdevapi/_event/list/models.py#L25-L34 to state the null-safe condition
value is not None and value.year < 2020; update
official-docs/content/docs/reference/events/list.mdx#L118-L125 with the same
condition so both documentation sites share one sentinel-year contract.
In `@src/vlrdevapi/commons/datetime.py`:
- Around line 117-119: Update the sentinel-year checks in the date parsing logic
to treat every parsed year below 2020 as an omitted year, using the documented
full.year >= 2020 boundary rather than checking only _UNKNOWN_YEAR (2019); apply
this consistently to both affected start/end handling paths.
- Around line 8-12: Change _UNKNOWN_YEAR in datetime.py to a leap year below
2020, then update its adjacent comments and all related model descriptions or
documentation that identify 2019 as the unknown year so they consistently
reference the new sentinel.
---
Outside diff comments:
In `@src/vlrdevapi/commons/timezone.py`:
- Around line 245-261: The displayed date parsing in the timezone-detection flow
must resolve the year relative to the stored UTC timestamp rather than always
using canonical.year. Update the surrounding date-resolution logic and its
caller to try the relevant adjacent-year candidate dates, selecting one whose
offset is valid before falling back to _system_iana_timezone(); preserve
existing parsing behavior for dates that include a year. Add a regression test
covering a December/January boundary.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c406ea6c-c961-4d80-848b-0d36eff5f18d
⛔ Files ignored due to path filters (1)
official-docs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
CHANGELOG.mddocs-py/docs/changelog.mdofficial-docs/app/(home)/changelog/page.tsxofficial-docs/content/docs/reference/events/list.mdxofficial-docs/content/docs/reference/events/teams.mdxofficial-docs/package.jsonsrc/vlrdevapi/_event/info/parser.pysrc/vlrdevapi/_event/list/models.pysrc/vlrdevapi/_event/namespace.pysrc/vlrdevapi/_event/stages/parser.pysrc/vlrdevapi/_event/teams/models.pysrc/vlrdevapi/_event/teams/namespace.pysrc/vlrdevapi/commons/datetime.pysrc/vlrdevapi/commons/timezone.pytests/commons/test_timezone.pytests/event/teams/test_parser.pytests/helpers/expected_from_html.pytests/team/stats/test_parser.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/vlrdevapi/_event/teams/namespace.py
- official-docs/package.json
- src/vlrdevapi/_event/teams/models.py
- official-docs/content/docs/reference/events/teams.mdx
- src/vlrdevapi/_event/namespace.py
- tests/event/teams/test_parser.py
Comment on lines
+10
to
+12
| { | ||
| version: 'Unreleased', | ||
| date: 'In development', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render Unreleased without the v prefix.
The new value is displayed through v{v.version} at Line 190. The page will show vUnreleased in the timeline and mobile heading.
Proposed fix
- v{v.version}
+ {v.version === 'Unreleased' ? v.version : `v${v.version}`}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@official-docs/app/`(home)/changelog/page.tsx around lines 10 - 12, Update the
version rendering in the changelog timeline and mobile heading around the
`v{v.version}` expressions so the `Unreleased` entry is displayed without the
`v` prefix while released versions retain it. Use the existing version value to
conditionally omit the prefix only for `Unreleased`.
Comment on lines
+25
to
+34
| start_date: date | None = Field( | ||
| default=None, | ||
| description="Event start date. Year is 2019 when the listing omitted it " | ||
| "(year < 2020 means the year is not present).", | ||
| ) | ||
| end_date: date | None = Field( | ||
| default=None, | ||
| description="Event end date. Year is 2019 when the listing omitted it " | ||
| "(year < 2020 means the year is not present).", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use one null-safe sentinel-year contract.
Both fields are nullable. The documented condition must check for None before reading .year.
src/vlrdevapi/_event/list/models.py#L25-L34: update both field descriptions to usevalue is not None and value.year < 2020.official-docs/content/docs/reference/events/list.mdx#L118-L125: document the same null-safe condition.
📍 Affects 2 files
src/vlrdevapi/_event/list/models.py#L25-L34(this comment)official-docs/content/docs/reference/events/list.mdx#L118-L125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vlrdevapi/_event/list/models.py` around lines 25 - 34, Update the
start_date and end_date field descriptions in
src/vlrdevapi/_event/list/models.py#L25-L34 to state the null-safe condition
value is not None and value.year < 2020; update
official-docs/content/docs/reference/events/list.mdx#L118-L125 with the same
condition so both documentation sites share one sentinel-year contract.
Comment on lines
+8
to
+12
| # Sentinel year used when a vlr.gg page omits the year from a date | ||
| # (e.g. ``"Sep 14"``). Any parsed date with ``year < 2020`` means the | ||
| # source did not specify a year; Valorant launched in 2020, so no real | ||
| # data can carry the sentinel year 2019. | ||
| _UNKNOWN_YEAR = 2019 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
python - <<'PY'
from datetime import datetime
for year in (2019, 2000):
try:
print(year, datetime.strptime(f"Feb 29 {year}", "%b %d %Y").date())
except ValueError:
print(year, "invalid")
PYRepository: Vanshbordia/vlrdevapi
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files matching datetime.py:"
fd 'datetime\.py$' . || true
echo
echo "Relevant date handling references:"
rg -n "_UNKNOWN_YEAR|_parse_list_(date|partial_date)|strptime|Feb 29|2019|2000" . --glob '*.py' --glob '*.md' --glob '*.txt' | head -120 || trueRepository: Vanshbordia/vlrdevapi
Length of output: 4559
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("src/vlrdevapi/commons/datetime.py")
if not path.exists():
print("missing src/vlrdevapi/commons/datetime.py")
raise SystemExit(0)
source = path.read_text()
tree = ast.parse(source, filename=str(path))
consts = [
(name, const.value if isinstance(const, ast.Constant) else None)
for name, const in ast.walk(tree)
if isinstance(const, ast.Assign) and len(const.targets) == 1
]
assigns = []
for node in tree.body:
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == "_UNKNOWN_YEAR":
assigns.append((node.lineno, ast.get_docstring(tree), ast.unparse(node.value) if hasattr(ast, 'unparse') else "<assign>"))
print("Assignments:", assigns)
for node in ast.walk(tree):
if isinstance(node, ast.Constant) and node.value in (2019, 2000):
print(f"constant {node.value} at line {node.lineno}")
# Simple string scan for parsing order around listed references.
lines = source.splitlines()
for line_no in [1, 8, 160, 165, 175, 185, 190]:
print(f"{line_no}: {lines[line_no-1]}")
PYRepository: Vanshbordia/vlrdevapi
Length of output: 326
Use a leap-year sentinel for yearless dates.
_UNKNOWN_YEAR = 2019 cannot represent Feb 29 because Feb 29 2019 is invalid for datetime.strptime. Use a leap year below 2020 and update the documentation and model descriptions that use 2019 as the unknown year.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vlrdevapi/commons/datetime.py` around lines 8 - 12, Change _UNKNOWN_YEAR
in datetime.py to a leap year below 2020, then update its adjacent comments and
all related model descriptions or documentation that identify 2019 as the
unknown year so they consistently reference the new sentinel.
Comment on lines
+117
to
+119
| date is returned for both start and end. When a side has no year, the | ||
| sentinel year :data:`_UNKNOWN_YEAR` (2019) is used; a parsed date with | ||
| ``year < 2020`` means the source omitted the year. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the sentinel check with the documented contract.
The documentation states that every year below 2020 means “year omitted”. The condition recognizes only 2019. An explicit 2018 value therefore bypasses contextual year resolution.
Use full.year >= 2020, or narrow the documentation to exactly 2019.
Suggested correction
-if full is not None and full.year != _UNKNOWN_YEAR:
+if full is not None and full.year >= 2020:Also applies to: 220-224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vlrdevapi/commons/datetime.py` around lines 117 - 119, Update the
sentinel-year checks in the date parsing logic to treat every parsed year below
2020 as an omitted year, using the documented full.year >= 2020 boundary rather
than checking only _UNKNOWN_YEAR (2019); apply this consistently to both
affected start/end handling paths.
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.
Summary by CodeRabbit
New Features
TeamPlayermodel.Bug Fixes
Documentation
Improvements