Repository navigation
feature: Add uses to /maps endpoint - #473
Merged
Merged
Conversation
Contributor
Reviewer's GuideAdds CSV-backed, typed Sequence diagram for CSV-backed map uses in the maps endpointsequenceDiagram
participant Client
participant MapsRoute
participant parse_maps_csv
participant maps_csv
participant Map
Client->>MapsRoute: GET /maps
MapsRoute->>parse_maps_csv: parse_maps_csv()
parse_maps_csv->>maps_csv: read_csv_file("maps")
maps_csv-->>parse_maps_csv: map rows with uses
parse_maps_csv-->>MapsRoute: maps with uses
MapsRoute->>Map: validate response
Map-->>Client: maps including uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="app/domain/parsers/maps.py" line_range="28" />
<code_context>
"name": map_dict["name"],
"screenshot": get_static_url_maps(map_dict["key"]),
"gamemodes": map_dict["gamemodes"].split(","),
+ "uses": map_dict["uses"].split(","),
"location": map_dict["location"],
"country_code": map_dict.get("country_code") or None,
</code_context>
<issue_to_address>
**Old instances overwrite the new map shape**
When old- and new-version API instances overlap during deployment, and an old instance writes to the shared maps cache after a new instance has written its response, an old-version API instance can write its map list to the shared `/maps` cache key after a new instance has populated it. Its parser omits `uses`, so clients then receive responses without the new field until a new-version write or cache expiry.
Prevent old-version instances from writing the new-version cache entry, for example by versioning the cache key or ensuring old instances are drained before refreshing it.
</issue_to_address>
### Comment 2
<location path="app/api/models/maps.py" line_range="24" />
<code_context>
...,
description="Main gamemodes on which the map is playable",
)
+ uses: list[MapUse] = Field(
+ ...,
+ min_length=1,
</code_context>
<issue_to_address>
**Cached maps lack classifications**
When a pre-deployment `/maps` proxy-cache entry is still valid, fastAPI never runs for a cache hit, so the cached JSON omits `uses` and clients receive map entries in the old shape without the new classifications.
Purge or invalidate the `/maps` proxy cache on deployment.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: app/domain/parsers/maps.py:28, app/api/models/maps.py:24
| "name": map_dict["name"], | ||
| "screenshot": get_static_url_maps(map_dict["key"]), | ||
| "gamemodes": map_dict["gamemodes"].split(","), | ||
| "uses": map_dict["uses"].split(","), |
Contributor
There was a problem hiding this comment.
🟡 Medium · Old instances overwrite the new map shape
When old- and new-version API instances overlap during deployment, and an old instance writes to the shared maps cache after a new instance has written its response, an old-version API instance can write its map list to the shared /maps cache key after a new instance has populated it. Its parser omits uses, so clients then receive responses without the new field until a new-version write or cache expiry.
Prevent old-version instances from writing the new-version cache entry, for example by versioning the cache key or ensuring old instances are drained before refreshing it.
Prompt for AI agents
In `app/domain/parsers/maps.py` at line 28:
**Old instances overwrite the new map shape**
When old- and new-version API instances overlap during deployment, and an old instance writes to the shared maps cache after a new instance has written its response, an old-version API instance can write its map list to the shared `/maps` cache key after a new instance has populated it. Its parser omits `uses`, so clients then receive responses without the new field until a new-version write or cache expiry.
Prevent old-version instances from writing the new-version cache entry, for example by versioning the cache key or ensuring old instances are drained before refreshing it.| ..., | ||
| description="Main gamemodes on which the map is playable", | ||
| ) | ||
| uses: list[MapUse] = Field( |
Contributor
There was a problem hiding this comment.
🟡 Medium · Cached maps lack classifications
When a pre-deployment /maps proxy-cache entry is still valid, fastAPI never runs for a cache hit, so the cached JSON omits uses and clients receive map entries in the old shape without the new classifications.
Purge or invalidate the /maps proxy cache on deployment.
Prompt for AI agents
In `app/api/models/maps.py` at line 24:
**Cached maps lack classifications**
When a pre-deployment `/maps` proxy-cache entry is still valid, fastAPI never runs for a cache hit, so the cached JSON omits `uses` and clients receive map entries in the old shape without the new classifications.
Purge or invalidate the `/maps` proxy cache on deployment.
|
Owner
|
@RealGatt Thank you for the initiative, good for me ! |
TeKrop
pushed a commit
that referenced
this pull request
Oct 17, 2026
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
2 participants
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.
I constantly had to do manual sorting for valid maps within my applications, figured this was a better system.
Watchpoint: Grimsvotn was also misnamed, this PR fixes that.
Add a
usesarray to each map returned by/mapsso clients can distinguish Standard, Stadium, Arcade, Workshop, and Training maps. The field supports multiple values, such as["standard", "stadium"]for Busan.Classifications are stored in the maps CSV and exposed through the parser and a typed response field. Assault and Clash maps are classified as Arcade-only, Wuxing University as Stadium-only, Workshop maps as Workshop, and Practice Range as Training. Existing map fields and gamemode filtering are preserved.
All 59 existing maps were reviewed against the Overwatch Wiki map list,
Validation:
Deployment note: existing reverse-proxy cache entries may continue returning the previous response shape until they expire or are refreshed.
Summary by Sourcery
Add map-use classifications to
/mapsso clients can distinguish maps by supported play context.New Features:
usesclassification for every map returned by the/mapsendpoint, covering standard, Stadium, Arcade, Workshop, and Training usage, including maps with multiple classifications.Bug Fixes:
Enhancements:
Deployment:
/mapsresponse shape until refreshed or expired.Tests:
/mapsresponse.