Skip to content

fix(screenscraper): skip name search after notgame hash lookup#3417

Merged
gantoine merged 4 commits into
rommapp:masterfrom
Spinnich:fix/screenscraper-skip-name-search-after-notgame
May 24, 2026
Merged

fix(screenscraper): skip name search after notgame hash lookup#3417
gantoine merged 4 commits into
rommapp:masterfrom
Spinnich:fix/screenscraper-skip-name-search-after-notgame

Conversation

@Spinnich
Copy link
Copy Markdown
Contributor

@Spinnich Spinnich commented May 23, 2026

Description
When jeuInfos.php returns a notgame entry (BIOS files, ZZZ hacks, unlicensed dumps, etc.), RomM had two related bugs:

  1. Notgame stored as a valid match — if the entry had a real numeric ID, lookup_rom() passed it straight to build_ss_game(), storing it as a legitimate SS match.
  2. Pointless name search — if the entry had a falsy ID (e.g. "0"), lookup_rom() returned SSRom(ss_id=None) and scan_handler fell through to get_rom(), issuing a jeuRecherche.php fuzzy name search that always returned nothing useful (SS's own notgame flag means no valid match will ever be found by name). This wastes the user's daily API quota.

Fix:
Bug #3415

  • Adds NOTGAME_NAME_PREFIX constant and _is_notgame() helper (checks both the notgame: "true" API field and ZZZ(NOTGAME) name prefix)
  • lookup_rom() now detects notgame entries before calling build_ss_game() and returns SSRom(ss_id=None, notgame=True)
  • scan_handler guards the get_rom() fallback: if lookup_rom() returned notgame=True, the jeuRecherche.php call is skipped entirely
  • _search_rom() now filters out notgame entries from name search results (defensive fix for the name search path)
  • Adds notgame: NotRequired[bool] field to SSRom

Checklist

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

🤖 Drafted with Claude Code

Spinnich and others added 3 commits May 23, 2026 17:01
When jeuInfos.php returned a notgame entry (BIOS files, ZZZ hacks, etc.),
lookup_rom() had no notgame check, leading to two bugs:
- notgame entries with a real ID were stored as valid SS matches
- notgame entries with a falsy ID fell through to a jeuRecherche.php name
  search that always returned nothing (pointless quota usage)

Adds _is_notgame() and NOTGAME_NAME_PREFIX, returns SSRom(notgame=True)
from lookup_rom() on a notgame hit, and guards the get_rom() fallback in
scan_handler so the name search is skipped entirely. Also adds the missing
notgame filter to _search_rom() so ZZZ entries can't match by name either.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous commit added a notgame skip-name-search check in scan_handler
but used a different key ("notgame") than what lookup_rom returned
("not_game"), so the fallback was never actually skipped. Align both on
SSRom.not_game, pop the internal flag before returning the SSRom to the
rest of the scan pipeline, and rename the helper to _is_not_game for
consistency with the field name.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Instead of smuggling an internal control flag through the SSRom dict,
lookup_rom now returns (SSRom, is_not_game: bool). scan_handler unpacks
the tuple and short-circuits the name-search fallback when either an
ss_id matched or the hash lookup flagged the entry as notgame.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gantoine gantoine requested a review from Copilot May 24, 2026 00:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes ScreenScraper “notgame” handling during scans so BIOS/“ZZZ(NOTGAME)” entries do not get treated as valid matches and do not trigger the fallback fuzzy name search (saving API quota).

Changes:

  • Add _is_not_game() detection and use it to ignore notgame responses from both hash lookup and name search results.
  • Change SSHandler.lookup_rom() to return (SSRom, is_not_game) and update scan flow to skip the filename-search fallback when is_not_game is true.
  • Add unit tests covering lookup_rom() returning the notgame flag.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
backend/handler/metadata/ss_handler.py Adds notgame detection, filters notgame results, and returns an explicit notgame indicator from hash lookup.
backend/handler/scan_handler.py Skips ScreenScraper name-search fallback when hash lookup indicates a notgame match.
backend/tests/handler/metadata/test_ss_handler.py Updates notgame helper tests and adds new async tests for lookup_rom notgame signaling.
Comments suppressed due to low confidence (2)

backend/tests/handler/metadata/test_ss_handler.py:365

  • ScreenScraperService.get_game_info is an async method, but these tests patch it with patch.object(..., return_value=...), which replaces it with a non-awaitable MagicMock. Since lookup_rom() does await self.ss_service.get_game_info(...), this will raise TypeError: object dict can't be used in 'await'. Patch with new_callable=AsyncMock (or set the attribute to an AsyncMock(return_value=...)) so the awaited call works.
        assert result == url

    def test_does_not_duplicate_existing_credentials(self):
        """Pre-existing ssid/sspassword on the URL are replaced, not duplicated."""

backend/tests/handler/metadata/test_ss_handler.py:382

  • Same issue as above: get_game_info is awaited inside lookup_rom(), but it is patched with a plain MagicMock via patch.object(..., return_value=...). Use AsyncMock so the awaited call returns notgame_response without raising a TypeError.
    def test_handles_stripped_url_from_extract_media(self):
        """A URL that's already had ssid/sspassword stripped (the storage form)
        gets credentials re-attached cleanly, with dev creds and other params
        left intact."""

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gantoine gantoine merged commit 1240902 into rommapp:master May 24, 2026
5 checks passed
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.

3 participants