Add mapper style sampling with username lookup #79
Conversation
- Add mapper_api.py for osu! API v2 username lookup - Add /lookup_mapper_name endpoint to look up mapper usernames - Enhance mapper_id input with Lookup button and name display - Add CSS for improved mapper ID input layout - API credentials via OSU_CLIENT_ID and OSU_CLIENT_SECRET env vars - Graceful fallback if API not configured
- Rewrote mapper_api.py to scrape public profile pages instead of using API v2 - Removes need for OSU_CLIENT_ID and OSU_CLIENT_SECRET environment variables - Added simple in-memory cache for username lookups - Updated web-ui.py to remove API credential error handling
rosacry
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds mapper ID input functionality with username lookup capabilities for style-based beatmap generation. Users can now enter an osu! mapper ID and look up the corresponding username through web scraping, without requiring API keys.
Key Changes:
- New
mapper_scrape.pymodule implementing username lookup via web scraping of osu! profile pages - New
/lookup_mapper_nameFlask endpoint for handling lookup requests - UI enhancements including a "Lookup" button and username display field
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| mapper_scrape.py | New module providing web scraping functionality to look up osu! usernames from user IDs with in-memory caching |
| web-ui.py | Adds import handling for mapper_scrape module and new API endpoint for username lookup (contains critical bug) |
| template/index.html | Updates mapper ID field with lookup button and username display span |
| static/style.css | Adds styling for input-with-button layout and helper text display |
| static/app.js | Implements AJAX handler for lookup button with user feedback and error handling |
Critical Issue Identified: The diff shows removal of the start_inference function decorator and definition, but the function body remains orphaned starting at line 202 of web-ui.py, which will cause a syntax error and prevent the application from running.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mapper_id = str(mapper_id).strip() | ||
| if not mapper_id: | ||
| return None | ||
|
|
||
| # Check cache first | ||
| if mapper_id in _USERNAME_CACHE: | ||
| return _USERNAME_CACHE[mapper_id] | ||
|
|
||
| url = f"https://osu.ppy.sh/users/{mapper_id}" |
There was a problem hiding this comment.
The mapper_id is not validated before being inserted into the URL. While the osu.ppy.sh server will handle malformed requests, it's better to validate that mapper_id contains only numeric characters to prevent potential issues with URL construction and provide clearer error messages to users. Consider adding validation like: if not re.match(r'^\d+$', mapper_id): return None
| <input type="text" id="mapper_id" name="mapper_id" placeholder="e.g. 124493" /> | ||
| <button type="button" id="lookup-mapper-btn" class="browse-button" title="Look up mapper name from ID">Lookup</button> | ||
| </div> | ||
| <span id="mapper-name-display" class="helper-text"></span> |
There was a problem hiding this comment.
The mapper name display span lacks an ARIA attribute to announce dynamic content changes to screen readers. Consider adding aria-live="polite" to the span so that when the lookup result is displayed, screen reader users are notified of the change.
| <span id="mapper-name-display" class="helper-text"></span> | |
| <span id="mapper-name-display" class="helper-text" aria-live="polite"></span> |
| if (data.username) { | ||
| $display.text('✓ ' + data.username).css('color', '#4CAF50'); | ||
| Utils.showFlashMessage(`Found mapper: ${data.username}`, "success"); | ||
| } else { |
There was a problem hiding this comment.
The success handler checks if (data.username) but doesn't handle the case where the response is successful (200 status) but data.username is falsy (empty string, null, etc.). This could result in displaying "User not found" even though the server returned a 200 response. The backend returns a 404 when the user is not found, so this else branch may be unreachable. Consider removing this else branch or adding a warning log to detect unexpected response formats.
| } else { | |
| } else { | |
| console.warn("Unexpected response format from /lookup_mapper_name:", data); |
| return { | ||
| "id": int(mapper_id), |
There was a problem hiding this comment.
The function attempts to convert mapper_id to int without error handling. If mapper_id contains non-numeric characters, this will raise a ValueError. Consider wrapping this in a try-except block or validating the input before conversion to provide better error handling.
| return { | |
| "id": int(mapper_id), | |
| try: | |
| user_id = int(mapper_id) | |
| except ValueError: | |
| print(f"[mapper_api] Invalid mapper_id: {mapper_id!r}") | |
| return None | |
| return { | |
| "id": user_id, |
| # Check cache first | ||
| if mapper_id in _USERNAME_CACHE: | ||
| return _USERNAME_CACHE[mapper_id] | ||
|
|
||
| url = f"https://osu.ppy.sh/users/{mapper_id}" | ||
| try: | ||
| r = requests.get( | ||
| url, | ||
| headers={ | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36" | ||
| }, | ||
| timeout=10 | ||
| ) | ||
| if r.status_code == 404: | ||
| print(f"[mapper_api] User {mapper_id} not found") | ||
| return None | ||
| if r.status_code != 200: | ||
| print(f"[mapper_api] Lookup failed: {r.status_code}") | ||
| return None | ||
|
|
||
| # Extract username from page title: "username · player info | osu!" | ||
| match = re.search(r'<title>(.+?)\s*·\s*player', r.text) | ||
| if match: | ||
| username = match.group(1).strip() | ||
| _USERNAME_CACHE[mapper_id] = username | ||
| return username | ||
|
|
||
| # Fallback: try to find in JSON data embedded in page | ||
| match = re.search(r'"username"\s*:\s*"([^"]+)"', r.text) | ||
| if match: | ||
| username = match.group(1) | ||
| _USERNAME_CACHE[mapper_id] = username | ||
| return username | ||
|
|
||
| print(f"[mapper_api] Could not parse username from page") | ||
| return None | ||
| except requests.RequestException as e: | ||
| print(f"[mapper_api] Request error: {e}") | ||
| return None |
There was a problem hiding this comment.
Failed lookups (404s or errors) are not cached, which means repeated lookups for non-existent user IDs will make HTTP requests every time. This could lead to rate limiting from the osu! servers if users repeatedly lookup invalid IDs. Consider implementing negative caching (storing None or a sentinel value for failed lookups) with a reasonable TTL to reduce unnecessary network requests.
| r = requests.get( | ||
| url, | ||
| headers={ | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36" |
There was a problem hiding this comment.
The User-Agent string is incomplete and may not be accepted by some servers. A complete User-Agent should include browser version and system details. Consider using a complete User-Agent string like: "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36"
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36" | |
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" |
| return jsonify({"error": "User not found"}), 404 | ||
| except Exception as e: | ||
| return jsonify({"error": str(e)}), 500 | ||
| """Starts the inference process based on form data.""" |
There was a problem hiding this comment.
This line appears to be an orphaned docstring from the start_inference function. The function decorator and definition were removed in the diff (lines starting with -), but the function body remains. This will cause a syntax error as the docstring and subsequent code have no function definition to belong to.
| print(f"[mapper_api] User {mapper_id} not found") | ||
| return None | ||
| if r.status_code != 200: | ||
| print(f"[mapper_api] Lookup failed: {r.status_code}") | ||
| return None | ||
|
|
||
| # Extract username from page title: "username · player info | osu!" | ||
| match = re.search(r'<title>(.+?)\s*·\s*player', r.text) | ||
| if match: | ||
| username = match.group(1).strip() | ||
| _USERNAME_CACHE[mapper_id] = username | ||
| return username | ||
|
|
||
| # Fallback: try to find in JSON data embedded in page | ||
| match = re.search(r'"username"\s*:\s*"([^"]+)"', r.text) | ||
| if match: | ||
| username = match.group(1) | ||
| _USERNAME_CACHE[mapper_id] = username | ||
| return username | ||
|
|
||
| print(f"[mapper_api] Could not parse username from page") | ||
| return None | ||
| except requests.RequestException as e: | ||
| print(f"[mapper_api] Request error: {e}") |
There was a problem hiding this comment.
The print statements reference "[mapper_api]" but this module is named "mapper_scrape". For consistency and easier log debugging, these should reference "[mapper_scrape]" instead.
| if (!mapperId) { | ||
| Utils.showFlashMessage("Please enter a mapper ID first", "error"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The mapper_id value is sent to the server without client-side validation. Consider adding validation to check if the value is numeric before making the AJAX request. This would provide faster feedback to users and reduce unnecessary server requests. For example: if (!/^\d+$/.test(mapperId)) { Utils.showFlashMessage("Mapper ID must be numeric", "error"); return; }
| } | |
| } | |
| if (!/^\d+$/.test(mapperId)) { | |
| Utils.showFlashMessage("Mapper ID must be numeric", "error"); | |
| return; | |
| } |
| # Fallback: try to find in JSON data embedded in page | ||
| match = re.search(r'"username"\s*:\s*"([^"]+)"', r.text) | ||
| if match: | ||
| username = match.group(1) | ||
| _USERNAME_CACHE[mapper_id] = username | ||
| return username |
There was a problem hiding this comment.
The regex pattern for extracting usernames from JSON data (r'"username"\s*:\s*"([^"]+)"') could match any "username" field in the entire HTML page, not just the user's actual username. This could extract incorrect data if there are multiple username fields or usernames from other users mentioned on the page. Consider making the pattern more specific by matching within a known structure, or validate that this is the primary fallback and document this limitation.
|
This feature seems kind of useless. Currently the flow for most users is this:
They already know the name of the mapper they put in, so this lookup adds no value. Something else this button could do is checking if the given Mapper ID is recognized by the model. This is useful to check because not every mapper is in the training data of the model. If you put in a mapper that is not recognized by the model it will default and act like you input no mapper at all. You can find all the valid mapper IDs in the |
|
I was thinking of the case where someone has a larger list of names saved in a config and then starts a new session. In that scenario, having the names persist alongside the mapper IDs makes it easier to remember which is which. If you’d prefer, I can either adjust this PR to align with the alternative you suggested, or open a separate PR that adds those new features. Just let me know which direction you want. |
|
Hmm i guess this still has some value. You can make it do both things in this PR. A button that checks the mapper presence in model and displays the name. |
2 participants
Adds mapper ID input with username lookup functionality for style-based beatmap generation.
Changes
mapper_scrape.pymodule for looking up osu! usernames by ID/lookup_mapper_nameendpoint for username lookupHow it works
osu.ppy.sh/users/{id})This allows users to input a mapper ID and see the resolved username, useful for generating beatmaps in a specific mapper's style.