Skip to content

Feature/updated series#24

Open
Vanshbordia wants to merge 3 commits intomainfrom
feature/updated-series
Open

Feature/updated series#24
Vanshbordia wants to merge 3 commits intomainfrom
feature/updated-series

Conversation

@Vanshbordia
Copy link
Copy Markdown
Owner

@Vanshbordia Vanshbordia commented Nov 26, 2025

Added Performance Module to return Kill Matrix and multi kills and 1vx data

Summary by CodeRabbit

Release Notes

  • New Features

    • Added series.performance() endpoint to retrieve detailed performance statistics including kill matrices, multi-kill counts, clutch events, and economy metrics per player and map.
  • Bug Fixes

    • Enhanced team identification with fallback logic for missing team names and improved team ID resolution in match parsing.
  • Documentation

    • New performance API documentation with structured schemas and usage examples.
    • Updated existing API docs with sample outputs for all code examples.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 26, 2025

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: '+language', 'include', 'exclude', 'focus', 'rules', 'documentation', 'testing', 'style', 'dependencies', 'performance', 'integrations', 'severity', 'messages', 'automation'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

This pull request introduces a new series.performance() API endpoint with comprehensive performance metrics including kill matrices, multi-kill details, clutch events, and per-player statistics. It adds five new data models, a performance parser, documentation with output examples, and extensive test coverage for the new functionality.

Changes

Cohort / File(s) Summary
New Performance API Implementation
src/vlrdevapi/series/performance.py
Introduces new public function performance(series_id, limit, timeout) and internal helpers (_parse_kill_matrix, _parse_player_performances, _parse_detailed_stat_cell, _parse_stat_cell) to fetch and parse per-map performance data including kill matrices, FKFD matrices, OP matrices, and player statistics from series pages.
New Data Models
src/vlrdevapi/series/models.py
Adds five frozen dataclasses: KillMatrixEntry (killer/victim stats), MultiKillDetail (round-level multi-kill events), ClutchDetail (round-level clutch events), PlayerPerformance (aggregated per-player stats with detail lists), and MapPerformance (per-map metrics with kill matrices and player performances).
Series Module Exports
src/vlrdevapi/series/__init__.py
Updates public API to export new models (KillMatrixEntry, PlayerPerformance, MapPerformance) and the new performance function.
Series Metadata Resolution
src/vlrdevapi/series/info.py, src/vlrdevapi/series/matches.py
Adds fallback logic in info.py to use team name as short name when unavailable. Extends matches.py with team ID extraction from page headers, multiple name variation mappings, and enhanced team resolution via canonicalized names and page-derived ID fallbacks.
API Documentation (RST)
docs/source/api/series.rst
Adds Sphinx documentation for new performance function and three new model classes (MultiKillDetail, ClutchDetail, PlayerPerformance); includes usage examples and Top Performers section with performance statistics.
User Documentation - Series APIs
betterdocs/content/docs/api/series/index.mdx, betterdocs/content/docs/api/series/matches.mdx
Updates index to introduce series.performance() endpoint card and expanded quick example with performance data. Adds tab="Code" attributes and output blocks to matches examples.
User Documentation - New Performance Endpoint
betterdocs/content/docs/api/series/performance.mdx
New comprehensive documentation file with Python signature, parameter details, return type, structured schemas for nested data, usage examples (Get performance statistics, Analyze kill matrix, Player performance analysis, First Kill/First Death analysis, Multi-Kill details), error handling, and Related/Source sections.
User Documentation - General Guides
betterdocs/content/docs/getting-started.mdx, betterdocs/content/docs/index.mdx, betterdocs/content/docs/installation.mdx, betterdocs/content/docs/config.mdx, betterdocs/content/docs/rate-limiting.mdx
Adds tab="Code" attributes to code blocks and corresponding Output tabs across multiple documentation sections for enhanced presentation and clarity of example outputs.
Comprehensive Test Suite
tests/lib/test_series.py
Introduces four test classes (TestSeriesPerformance, TestSeriesEdgeCases, TestSeriesPerformanceAdvanced, TestSeriesPerformanceValidation) with 20+ new test methods covering performance endpoint functionality, data structure validation, matrix parsing, player performance extraction, edge cases, parameter handling, and real-API validation scenarios.

Sequence Diagram

sequenceDiagram
    actor User
    participant Client as Client Code
    participant Perf as performance()
    participant Fetcher as fetch_html()
    participant Parser as Parse Helpers
    participant Models as Data Models

    User->>Client: Call performance(series_id)
    Client->>Perf: performance(series_id, limit, timeout)
    
    Perf->>Fetcher: Fetch performance page HTML
    Fetcher-->>Perf: HTML content or error
    
    alt Network error
        Perf-->>Client: Return empty list
    else Success
        Perf->>Perf: Extract team IDs from header
        Perf->>Perf: Build team tag → ID mappings
        Perf->>Perf: Build game ID → map name mappings
        
        loop For each game section
            Perf->>Parser: _parse_kill_matrix(game, matrix_class, team_tag_to_id)
            Parser-->>Perf: list[KillMatrixEntry]
            
            Perf->>Parser: _parse_player_performances(game, team_tag_to_id)
            Parser-->>Perf: list[PlayerPerformance]
            
            Perf->>Models: Create MapPerformance
            Models-->>Perf: MapPerformance instance
        end
        
        Perf-->>Client: list[MapPerformance]
    end
    
    Client-->>User: Performance data
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • src/vlrdevapi/series/performance.py — High logic density with intricate HTML parsing, multiple fallback pathways for team ID resolution, and careful data extraction from complex nested structures. The _parse_kill_matrix, _parse_player_performances, and _parse_detailed_stat_cell functions contain nuanced cell-by-cell parsing with optional data handling.
  • src/vlrdevapi/series/models.py — Five new frozen dataclasses with multiple optional fields and nested structures. Verify immutability, field naming consistency, and alignment with parser expectations.
  • src/vlrdevapi/series/matches.py — Enhanced team resolution logic with multiple fallback mechanisms and canonicalized name mappings. Ensure fallback precedence is correct and doesn't introduce regressions in existing match parsing.
  • tests/lib/test_series.py — Extensive new test suite (20+ methods across 4 test classes) with real-API validation. Verify mock setup, assertion completeness, and edge-case coverage align with parser robustness.

Poem

🐰 A performance parser born this day,
Kill matrices and clutches on display,
Per-player stats in frozen grace,
All maps collected in one place!
With tests aplenty, docs so bright,
The series API shines tonight!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive Title is vague and generic, using non-descriptive term 'updated series' that does not convey meaningful information about the changeset's core addition of a performance module. Revise title to be more specific about the main change, such as 'Add series.performance() module for kill matrix and player performance data' to clearly communicate the primary feature addition.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 97.44% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/updated-series

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (11)
src/vlrdevapi/series/matches.py (1)

100-129: Enhanced metadata mapping with smart name variations.

The implementation effectively handles multiple name formats:

  • Direct team ID lookup via team_id_to_meta
  • Parentheses extraction (e.g., "Guangzhou Huadu Bilibili Gaming(Bilibili Gaming)" → "Bilibili Gaming")
  • All name variations indexed for flexible matching

The use of rfind for parentheses extraction correctly targets the last occurrence, which typically contains the most specific tag.

Optional: Consider handling edge case of duplicate canonical names.

If multiple teams share the same canonical name variation (e.g., both teams have short name "TBD"), the team_meta_lookup dict will map to whichever team was processed last. While the page_team_ids fallback mitigates this, you could optionally add a check to detect and log such collisions for debugging:

# Index all variations
for variation in name_variations:
    canon = canonical(variation)
    if canon:
        if canon in team_meta_lookup and team_meta_lookup[canon] != team_meta_rec:
            # Optional: log collision for debugging
            pass
        team_meta_lookup[canon] = team_meta_rec
betterdocs/content/docs/api/series/performance.mdx (1)

466-468: Misleading variable name in example.

The variable all_maps filters for map_name == "Corrode", which is a specific map, not an aggregate. Consider renaming for clarity:

-all_maps = next((game for game in perf if game.map_name == "Corrode"), None)
+corrode_map = next((game for game in perf if game.map_name == "Corrode"), None)
 
-if all_maps:
-    for player in all_maps.player_performances:
+if corrode_map:
+    for player in corrode_map.player_performances:
docs/source/api/series.rst (1)

89-108: Consider adding documentation for KillMatrixEntry and MapPerformance models.

The Data Models section documents MultiKillDetail, ClutchDetail, and PlayerPerformance, but KillMatrixEntry and MapPerformance are also exported in __all__ and used by the performance() function. Adding their autoclass entries would provide complete API documentation.

KillMatrixEntry
~~~~~~~~~~~~~~~

.. autoclass:: vlrdevapi.series.KillMatrixEntry
   :members:
   :undoc-members:

MapPerformance
~~~~~~~~~~~~~~

.. autoclass:: vlrdevapi.series.MapPerformance
   :members:
   :undoc-members:
src/vlrdevapi/series/__init__.py (1)

3-6: Consider exporting MultiKillDetail and ClutchDetail for full API accessibility.

These models are used in PlayerPerformance fields (e.g., multi_2k_details: list[MultiKillDetail]) and documented in series.rst. Users may need to type-annotate or inspect these objects. Adding them to the exports would provide a complete public API surface.

-from .models import TeamInfo, MapAction, Info, PlayerStats, MapTeamScore, RoundResult, MapPlayers, KillMatrixEntry, PlayerPerformance, MapPerformance
+from .models import TeamInfo, MapAction, Info, PlayerStats, MapTeamScore, RoundResult, MapPlayers, KillMatrixEntry, MultiKillDetail, ClutchDetail, PlayerPerformance, MapPerformance

And update __all__:

     "KillMatrixEntry",
+    "MultiKillDetail",
+    "ClutchDetail",
     "PlayerPerformance",
src/vlrdevapi/series/performance.py (5)

40-43: Silent failure on network errors may hide issues.

Returning an empty list on NetworkError is valid for graceful degradation, but consumers won't know if the series genuinely has no data or if a network issue occurred. Consider logging the error at debug/warning level for observability.

     try:
         html = fetch_html(url, effective_timeout)
     except NetworkError:
+        import logging
+        logging.debug(f"Failed to fetch performance data for series {series_id}")
         return []

75-95: Duplicated team abbreviation handling code.

The logic for handling "TEAM LIQUID" and "DRX" abbreviations is duplicated between the main header parsing (lines 75-95) and the overview page fallback (lines 127-142). Consider extracting this into a helper function.

def _add_team_abbreviations(team_name_upper: str, team_id: int | None, team_tag_to_id: dict[str, int | None]) -> None:
    """Add common abbreviations for known team names."""
    if team_name_upper == "TEAM LIQUID":
        team_tag_to_id["TL"] = team_id
    elif team_name_upper == "DRX":
        team_tag_to_id["DRX"] = team_id

Also applies to: 127-142


232-235: Narrow the exception type for int conversion.

Catching bare Exception obscures potential issues. Since int() only raises ValueError, prefer catching that specific exception. The static analysis also flagged this pattern.

             try:
                 numeric_ids.append((int(g), g))
-            except Exception:
+            except ValueError:
                 continue

275-278: Use specific exception type.

Same as line 234—prefer ValueError over bare Exception for clarity.

         try:
             gid = int(game_id) if game_id and game_id.isdigit() else None
-        except Exception:
+        except ValueError:
             gid = None

438-488: Consider documenting the table-finding strategies.

The four fallback strategies (lines 441-488) for locating the performance table show good defensive programming but make the code complex. A brief comment explaining why multiple strategies are needed (HTML structure variations) would help maintainers.

tests/lib/test_series.py (2)

179-223: Consider marking real API tests as integration tests.

test_performance_real_api_chronicle_match hits the real VLR.gg API without mocking. This could cause flaky tests if the API is unavailable or data changes. Consider marking it with @pytest.mark.integration or @pytest.mark.slow so it can be skipped in fast CI runs.

+    @pytest.mark.integration
     def test_performance_real_api_chronicle_match(self):
         """Test real API performance data for Chronicle FNC vs NRG (series 542272)."""

554-809: All validation tests hit real API—consider adding integration markers.

All tests in TestSeriesPerformanceValidation make real network calls without mocking. While valuable for integration testing, they could cause CI failures due to network issues or data changes on VLR.gg. Consider:

  1. Adding @pytest.mark.integration to the class
  2. Using pytest.mark.skipif for environments without network access
+@pytest.mark.integration
 class TestSeriesPerformanceValidation:
     """Specific data validation tests for performance module."""
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5bbe2a3 and c88e6b9.

📒 Files selected for processing (15)
  • betterdocs/content/docs/api/series/index.mdx (2 hunks)
  • betterdocs/content/docs/api/series/matches.mdx (4 hunks)
  • betterdocs/content/docs/api/series/performance.mdx (1 hunks)
  • betterdocs/content/docs/config.mdx (3 hunks)
  • betterdocs/content/docs/getting-started.mdx (8 hunks)
  • betterdocs/content/docs/index.mdx (4 hunks)
  • betterdocs/content/docs/installation.mdx (3 hunks)
  • betterdocs/content/docs/rate-limiting.mdx (4 hunks)
  • docs/source/api/series.rst (3 hunks)
  • src/vlrdevapi/series/__init__.py (2 hunks)
  • src/vlrdevapi/series/info.py (1 hunks)
  • src/vlrdevapi/series/matches.py (3 hunks)
  • src/vlrdevapi/series/models.py (1 hunks)
  • src/vlrdevapi/series/performance.py (1 hunks)
  • tests/lib/test_series.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/vlrdevapi/series/matches.py (1)
src/vlrdevapi/utils.py (1)
  • extract_id_from_url (98-119)
src/vlrdevapi/series/__init__.py (2)
src/vlrdevapi/series/models.py (10)
  • TeamInfo (10-18)
  • MapAction (22-27)
  • Info (31-47)
  • PlayerStats (51-71)
  • MapTeamScore (75-84)
  • RoundResult (88-97)
  • MapPlayers (101-108)
  • KillMatrixEntry (112-123)
  • PlayerPerformance (143-171)
  • MapPerformance (175-183)
src/vlrdevapi/series/performance.py (1)
  • performance (18-317)
tests/lib/test_series.py (2)
src/vlrdevapi/series/performance.py (1)
  • performance (18-317)
tests/lib/conftest.py (1)
  • mock_fetch_html (26-92)
src/vlrdevapi/series/performance.py (5)
src/vlrdevapi/series/models.py (5)
  • MapPerformance (175-183)
  • KillMatrixEntry (112-123)
  • PlayerPerformance (143-171)
  • MultiKillDetail (127-131)
  • ClutchDetail (135-139)
src/vlrdevapi/config.py (1)
  • get_config (74-76)
src/vlrdevapi/fetcher.py (1)
  • fetch_html (422-443)
src/vlrdevapi/exceptions.py (1)
  • NetworkError (50-52)
src/vlrdevapi/utils.py (3)
  • extract_text (25-34)
  • parse_int (54-64)
  • extract_id_from_url (98-119)
🪛 Ruff (0.14.5)
tests/lib/test_series.py

225-225: Unused method argument: mock_fetch_html

(ARG002)


230-230: Unused method argument: mock_fetch_html

(ARG002)


242-242: Unused method argument: mock_fetch_html

(ARG002)


255-255: Unused method argument: mock_fetch_html

(ARG002)


278-278: Unused method argument: mock_fetch_html

(ARG002)


301-301: Unused method argument: mock_fetch_html

(ARG002)


310-310: Unused method argument: mock_fetch_html

(ARG002)


316-316: Unused method argument: mock_fetch_html

(ARG002)


321-321: Unused method argument: mock_fetch_html

(ARG002)


335-335: Unused method argument: mock_fetch_html

(ARG002)


345-345: Unused method argument: mock_fetch_html

(ARG002)


373-373: Unused method argument: mock_fetch_html

(ARG002)


386-386: Unused method argument: mock_fetch_html

(ARG002)


399-399: Unused method argument: mock_fetch_html

(ARG002)


440-440: Unused method argument: mock_fetch_html

(ARG002)


453-453: Unused method argument: mock_fetch_html

(ARG002)


470-470: Unused method argument: mock_fetch_html

(ARG002)


486-486: Unused method argument: mock_fetch_html

(ARG002)


506-506: Unused method argument: mock_fetch_html

(ARG002)


518-518: Unused method argument: mock_fetch_html

(ARG002)


537-537: Unused method argument: mock_fetch_html

(ARG002)


720-720: f-string without any placeholders

Remove extraneous f prefix

(F541)


806-806: f-string without any placeholders

Remove extraneous f prefix

(F541)

src/vlrdevapi/series/performance.py

179-179: Dictionary key literal "FNC" repeated

Remove repeated key literal "FNC"

(F601)


234-235: try-except-continue detected, consider logging the exception

(S112)


234-234: Do not catch blind exception: Exception

(BLE001)


277-277: Do not catch blind exception: Exception

(BLE001)


600-600: Local variable direct_text is assigned to but never used

Remove assignment to unused variable direct_text

(F841)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Test on Python 3.11
  • GitHub Check: validate-docs
  • GitHub Check: validate-betterdocs
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (23)
src/vlrdevapi/series/info.py (1)

126-130: LGTM! Robust fallback for missing short names.

The fallback logic gracefully handles teams without short tags by using the full team name. This prevents downstream None-related issues and aligns with the broader team resolution strategy enhanced in matches.py.

betterdocs/content/docs/index.mdx (2)

21-36: Documentation improvement looks good.

The addition of tabbed Code/Output blocks enhances the user experience by clearly showing expected results alongside code examples.


104-125: Documentation enhancement approved.

The Output section clearly demonstrates the expected results, making it easier for users to verify their implementation.

betterdocs/content/docs/config.mdx (1)

128-218: Documentation improvements approved.

The addition of Output blocks across all configuration examples provides clear expectations for users and improves the overall documentation quality.

betterdocs/content/docs/getting-started.mdx (1)

33-162: Documentation enhancements look excellent.

The comprehensive addition of Output blocks across all examples (Matches, Search, Events, Rate Limiting) significantly improves the getting-started experience by showing concrete results.

betterdocs/content/docs/installation.mdx (1)

21-60: Documentation updates approved.

The Output blocks clearly demonstrate expected behavior during installation verification and rate limit configuration.

betterdocs/content/docs/rate-limiting.mdx (1)

18-71: Rate limiting documentation improvements approved.

The Output blocks effectively demonstrate the state transitions during rate limit configuration, making the behavior clear to users.

src/vlrdevapi/series/matches.py (2)

75-88: Page-level team ID extraction looks solid.

The extraction logic correctly targets the match header and uses the appropriate selectors (.mod-1 and .mod-2) to maintain team ordering. The use of extract_id_from_url ensures robust ID parsing.


245-255: Robust multi-level fallback strategy.

The fallback logic gracefully handles missing team metadata:

  1. First attempts canonical name lookup in team_meta_lookup
  2. Falls back to ID-based lookup via team_id_to_meta using page_team_ids
  3. Finally uses page_team_ids directly for ID assignment

The ordering assumption (page_team_ids[0] = team1, page_team_ids[1] = team2) is valid because the extraction at lines 79-88 explicitly uses .mod-1 and .mod-2 selectors, ensuring consistent ordering.

betterdocs/content/docs/api/series/matches.mdx (1)

161-371: Comprehensive documentation examples with outputs.

The extensive Output blocks provide realistic examples of map statistics, player performance, and round-by-round results. This significantly enhances the documentation by showing users exactly what to expect from the API.

betterdocs/content/docs/api/series/performance.mdx (1)

17-18: Documentation signature mismatch for timeout parameter.

The signature shows timeout: float = 5.0 but the actual implementation in performance.py defines timeout: float | None = None. The default timeout is applied internally via _config.default_timeout when None is passed.

Update to match the implementation:

-    timeout: float = 5.0
+    timeout: float | None = None

[inconsistent_summary, raise_minor_issue]

betterdocs/content/docs/api/series/index.mdx (2)

21-27: LGTM!

The new Card for series.performance() is well-integrated and the description accurately summarizes the endpoint's capabilities.


54-62: LGTM!

The Quick Example effectively demonstrates the new series.performance() API alongside existing endpoints.

docs/source/api/series.rst (2)

31-35: LGTM!

The performance function is properly documented with autofunction directive.


214-240: LGTM!

The Match Performance Statistics example is comprehensive, demonstrating multi-kill counts, clutch stats, economy metrics, and detailed event information.

src/vlrdevapi/series/models.py (5)

126-131: Consider using field(default_factory=list) for the players_killed field.

In frozen dataclasses, mutable default values like list[str] without default_factory are typically flagged as a concern. However, since players_killed is a required field (no default), this is actually correct. The type annotation list[str] correctly specifies the expected type without providing a mutable default.

LGTM! - The model is correctly defined as an immutable dataclass with a required list field.


134-139: ClutchDetail has identical structure to MultiKillDetail.

Both models share the same fields (round_number, players_killed). This is acceptable for semantic clarity, as clutches and multi-kills are conceptually different events. If future differentiation is needed (e.g., clutch-specific fields like opponents_remaining), having separate classes provides flexibility.


142-171: LGTM!

The PlayerPerformance model is comprehensive and well-structured. Using None defaults for optional stats is appropriate for scraped data where values may be missing.


174-183: LGTM!

The MapPerformance model correctly uses field(default_factory=list) for mutable list fields, avoiding the shared mutable default issue in dataclasses.


111-123: LGTM!

The KillMatrixEntry model cleanly represents kill/death interactions between players with appropriate optional fields for team context and statistics.

src/vlrdevapi/series/performance.py (1)

703-709: LGTM!

Clean and simple utility function with proper reuse of existing helpers.

tests/lib/test_series.py (2)

225-286: LGTM!

Well-structured tests with proper use of the mock_fetch_html fixture. Tests cover structure validation, matrix parsing, and player performance extraction. (Note: Static analysis warnings about unused mock_fetch_html arguments are false positives—pytest fixtures work via side effects.)


307-551: LGTM!

Comprehensive test coverage for advanced performance scenarios including limit/timeout parameters, game ID mapping, matrix structures, and data consistency. Good use of the mock fixture.

Comment on lines +36 to +40
timeout: {
type: 'float',
description: 'Request timeout in seconds',
default: '5.0',
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

TypeTable default value inconsistent with implementation.

The default shown as '5.0' should be 'None' to match the actual function signature.

     timeout: {
       type: 'float',
       description: 'Request timeout in seconds',
-      default: '5.0',
+      default: 'None',
     },
📝 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
timeout: {
type: 'float',
description: 'Request timeout in seconds',
default: '5.0',
},
timeout: {
type: 'float',
description: 'Request timeout in seconds',
default: 'None',
},
🤖 Prompt for AI Agents
In betterdocs/content/docs/api/series/performance.mdx around lines 36 to 40, the
TypeTable shows the timeout default as '5.0' which is inconsistent with the
function signature that uses None as the default; update the default value in
the documentation entry from '5.0' to 'None' so the docs match the implemented
signature, and ensure the type/description remain unchanged.

Comment on lines +149 to +185
common_abbreviations = {
"TL": "TEAM LIQUID",
"DRX": "DRX",
"FNC": "FNC ESPORTS",
"G2": "G2",
"ENCE": "ENCE",
"LOUD": "LOUD",
"PRX": "PAPER REX",
"T1": "T1",
"GEN": "GEN.G",
"LEV": "LEVIATAN",
"KRÜ": "KRÜ ESPORTS",
"KPR": "KATARU",
"ZETA": "ZETA DIVISION",
"TF": "TWO ALPHA",
"FUT": "FUTURO",
"KC": "KEYD STARS",
"INF": "INFINITE ESPORTS",
"GX": "GIANTX",
"NRG": "NRG",
"SEN": "SENPAI ESPORTS",
"ACN": "AVANTARIA CINCO",
"KBM": "KBM PLUG'N PLAY",
"FUR": "FURIA ESPORTS",
"BAD": "BAD NEWS EAGLES",
"C9": "CLOUD9",
"EG": "EVIL GENIUSES",
"OPTC": "OPTC",
"TSM": "TSM",
"100T": "100 THIEVES",
"FNC": "FNC ESPORTS",
"BBL": "BIGBEN LOUNGE",
"GIA": "GIANTX",
"RENA": "RENASCENCE",
"VIT": "VITALITY",
"GL": "GOLDEN GORILLAS",
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Duplicate dictionary key "FNC" at line 179.

The key "FNC" appears at line 152 and again at line 179, both mapping to "FNC ESPORTS". While functionally harmless (same value), this is redundant and was flagged by static analysis. Remove the duplicate.

         "100T": "100 THIEVES",
-        "FNC": "FNC ESPORTS",
         "BBL": "BIGBEN LOUNGE",
📝 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
common_abbreviations = {
"TL": "TEAM LIQUID",
"DRX": "DRX",
"FNC": "FNC ESPORTS",
"G2": "G2",
"ENCE": "ENCE",
"LOUD": "LOUD",
"PRX": "PAPER REX",
"T1": "T1",
"GEN": "GEN.G",
"LEV": "LEVIATAN",
"KRÜ": "KRÜ ESPORTS",
"KPR": "KATARU",
"ZETA": "ZETA DIVISION",
"TF": "TWO ALPHA",
"FUT": "FUTURO",
"KC": "KEYD STARS",
"INF": "INFINITE ESPORTS",
"GX": "GIANTX",
"NRG": "NRG",
"SEN": "SENPAI ESPORTS",
"ACN": "AVANTARIA CINCO",
"KBM": "KBM PLUG'N PLAY",
"FUR": "FURIA ESPORTS",
"BAD": "BAD NEWS EAGLES",
"C9": "CLOUD9",
"EG": "EVIL GENIUSES",
"OPTC": "OPTC",
"TSM": "TSM",
"100T": "100 THIEVES",
"FNC": "FNC ESPORTS",
"BBL": "BIGBEN LOUNGE",
"GIA": "GIANTX",
"RENA": "RENASCENCE",
"VIT": "VITALITY",
"GL": "GOLDEN GORILLAS",
}
common_abbreviations = {
"TL": "TEAM LIQUID",
"DRX": "DRX",
"FNC": "FNC ESPORTS",
"G2": "G2",
"ENCE": "ENCE",
"LOUD": "LOUD",
"PRX": "PAPER REX",
"T1": "T1",
"GEN": "GEN.G",
"LEV": "LEVIATAN",
"KRÜ": "KRÜ ESPORTS",
"KPR": "KATARU",
"ZETA": "ZETA DIVISION",
"TF": "TWO ALPHA",
"FUT": "FUTURO",
"KC": "KEYD STARS",
"INF": "INFINITE ESPORTS",
"GX": "GIANTX",
"NRG": "NRG",
"SEN": "SENPAI ESPORTS",
"ACN": "AVANTARIA CINCO",
"KBM": "KBM PLUG'N PLAY",
"FUR": "FURIA ESPORTS",
"BAD": "BAD NEWS EAGLES",
"C9": "CLOUD9",
"EG": "EVIL GENIUSES",
"OPTC": "OPTC",
"TSM": "TSM",
"100T": "100 THIEVES",
"BBL": "BIGBEN LOUNGE",
"GIA": "GIANTX",
"RENA": "RENASCENCE",
"VIT": "VITALITY",
"GL": "GOLDEN GORILLAS",
}
🧰 Tools
🪛 Ruff (0.14.5)

179-179: Dictionary key literal "FNC" repeated

Remove repeated key literal "FNC"

(F601)

🤖 Prompt for AI Agents
In src/vlrdevapi/series/performance.py around lines 149 to 185 there is a
duplicate dictionary key "FNC" (first at ~line 152 and again at ~line 179)
mapping to the same value "FNC ESPORTS"; remove the redundant second "FNC" entry
(the one at ~line 179) so the dictionary contains a single "FNC" key, preserving
the remaining mappings and formatting.

Comment on lines +414 to +417
if not final_killer_team_id and killer_team_tag and killer_team_tag in team_tag_to_id:
final_killer_team_id = team_tag_to_id[killer_team_tag]
if not final_victim_team_id and victim_team and victim_team in team_tag_to_id:
final_victim_team_id = team_tag_to_id[victim_team]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent casing in team tag lookup.

At line 355, team_tag.upper() is used for the lookup, but at line 416, victim_team is used without .upper(). This inconsistency could cause lookups to fail if the mapping uses uppercase keys (which it does, per lines 76-77).

                     if not final_killer_team_id and killer_team_tag and killer_team_tag in team_tag_to_id:
-                        final_killer_team_id = team_tag_to_id[killer_team_tag]
-                    if not final_victim_team_id and victim_team and victim_team in team_tag_to_id:
-                        final_victim_team_id = team_tag_to_id[victim_team]
+                        final_killer_team_id = team_tag_to_id.get(killer_team_tag.upper())
+                    if not final_victim_team_id and victim_team and victim_team.upper() in team_tag_to_id:
+                        final_victim_team_id = team_tag_to_id.get(victim_team.upper())

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/vlrdevapi/series/performance.py around lines 414 to 417, the code looks
up team IDs using team_tag_to_id but compares with victim_team without
uppercasing, while elsewhere keys were created/queried as .upper(), causing
inconsistent lookups; update the checks and dictionary access to use
victim_team.upper() (and ensure killer_team_tag is also uppercased consistently)
when testing membership and when retrieving from team_tag_to_id so lookups match
the uppercase keys.

Comment on lines +590 to +600
def _parse_detailed_stat_cell(cell: Tag):
"""Parse a stat value and detailed information from a cell with popable contents."""
stat_sq = cell.select_one(".stats-sq")
if not stat_sq:
return None, None

# Extract the numerical value, handling whitespace properly
count = None
# Get the direct text content of the stats-sq element (excluding content from child elements)
# The number is the direct text content in the element
direct_text = None
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused variable and add return type annotation.

direct_text is assigned at line 600 but never used. Also, the function lacks a return type annotation.

-def _parse_detailed_stat_cell(cell: Tag):
+def _parse_detailed_stat_cell(cell: Tag) -> tuple[int | None, list | None]:
     """Parse a stat value and detailed information from a cell with popable contents."""
     stat_sq = cell.select_one(".stats-sq")
     if not stat_sq:
         return None, None
 
     # Extract the numerical value, handling whitespace properly
     count = None
     # Get the direct text content of the stats-sq element (excluding content from child elements)
     # The number is the direct text content in the element
-    direct_text = None
🧰 Tools
🪛 Ruff (0.14.5)

600-600: Local variable direct_text is assigned to but never used

Remove assignment to unused variable direct_text

(F841)

🤖 Prompt for AI Agents
In src/vlrdevapi/series/performance.py around lines 590 to 600, remove the
unused variable `direct_text` (it is assigned but never referenced) and add an
explicit return type annotation to the function signature (e.g., ->
Tuple[Optional[int], Optional[dict]] or the appropriate types used by this
function). Update the function signature to include the typing imports (from
typing import Tuple, Optional) at the top of the file if not already present,
and ensure the implementation still returns the same two values (None, None or
parsed values) after removing the unused assignment.

Comment thread tests/lib/test_series.py
Comment on lines +720 to +725
print(f"✅ All validations passed for game 542272:")
print(f" - Found {len(nrg_players)} NRG players and {len(fnc_players)} FNC players")
print(f" - brawk: {brawk.multi_2k} 2Ks, {brawk.econ} econ, {brawk.clutch_1v1} 1v1s")
print(f" - Kill matrix has {len(all_maps_perf.kill_matrix)} entries")
print(f" - FKFD matrix has {len(all_maps_perf.fkfd_matrix)} entries")
print(f" - OP matrix has {len(all_maps_perf.op_matrix)} entries")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove f-string prefix and consider removing print statements from tests.

Lines 720 and 806 have f-strings without placeholders (static analysis flagged F541). Also, print statements in tests can clutter output; consider using logging.debug or removing them.

-        print(f"✅ All validations passed for game 542272:")
+        print("✅ All validations passed for game 542272:")
         print(f"   - Found {len(nrg_players)} NRG players and {len(fnc_players)} FNC players")
-        print(f"✅ Skuba Corrode validations passed:")
+        print("✅ Skuba Corrode validations passed:")
         print(f"   - 1v2 clutch in round {clutch_1v2_detail.round_number}: {', '.join(clutch_1v2_detail.players_killed)}")
📝 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
print(f"✅ All validations passed for game 542272:")
print(f" - Found {len(nrg_players)} NRG players and {len(fnc_players)} FNC players")
print(f" - brawk: {brawk.multi_2k} 2Ks, {brawk.econ} econ, {brawk.clutch_1v1} 1v1s")
print(f" - Kill matrix has {len(all_maps_perf.kill_matrix)} entries")
print(f" - FKFD matrix has {len(all_maps_perf.fkfd_matrix)} entries")
print(f" - OP matrix has {len(all_maps_perf.op_matrix)} entries")
print("✅ All validations passed for game 542272:")
print(f" - Found {len(nrg_players)} NRG players and {len(fnc_players)} FNC players")
print(f" - brawk: {brawk.multi_2k} 2Ks, {brawk.econ} econ, {brawk.clutch_1v1} 1v1s")
print(f" - Kill matrix has {len(all_maps_perf.kill_matrix)} entries")
print(f" - FKFD matrix has {len(all_maps_perf.fkfd_matrix)} entries")
print(f" - OP matrix has {len(all_maps_perf.op_matrix)} entries")
🧰 Tools
🪛 Ruff (0.14.5)

720-720: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
In tests/lib/test_series.py around lines 720-725 (and also where similar prints
occur around line 806), the test uses f-strings with no placeholders (triggering
F541) and contains plain print() calls that clutter test output; remove the
unnecessary f-string prefix (make them regular strings) or, better, remove the
prints entirely or replace them with logging.debug calls so test output stays
clean; update both occurrences to either use non-f-strings or logging and run
tests to ensure no remaining F541 warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant