Skip to content

fix(hltb): fetch metadata when a HowLongToBeat ID is set by hand - #4235

Merged
gantoine merged 3 commits into
rommapp:masterfrom
sdornan:claude/github-issue-2926-validation-4a3669
Aug 19, 2026
Merged

fix(hltb): fetch metadata when a HowLongToBeat ID is set by hand#4235
gantoine merged 3 commits into
rommapp:masterfrom
sdornan:claude/github-issue-2926-validation-4a3669

Conversation

@sdornan

@sdornan sdornan commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Description

Manually entering a HowLongToBeat ID saved the ID but never fetched anything, leaving the How Long to Beat tab empty.

update_rom has a block that refetches from each metadata provider whose ID changed, covering Flashpoint, LaunchBox, RA, MobyGames, ScreenScraper and IGDB. HLTB was absent from it, and HLTBHandler had no get_rom_by_id at all, so hltb_metadata only ever got populated when the client also sent raw_hltb_metadata, which the edit form does not do for a hand-typed ID. Both v1 and v2 expose the field, so both were affected.

This adds the missing handler method and wires it in alongside the others.

On scraping, since that is the reason this was previously closed

HLTB exposes no by-ID API. I re-checked rather than taking it on trust: POST /api/bleed with searchTerms: ["7169"] returns count: 0 (it is a text search over names), and /api/game/7169, /api/games/7169, /api/bleed/game/7169, /api/game?id=7169, /api/gamedetail/7169 and /api/game/detail/7169 all 404. So the record is read from the __NEXT_DATA__ payload the game page already ships to the browser.

Worth weighing against what the feature already rests on: backend/utils/update_hltb_api_url.py discovers the rotating search endpoint by fetching the homepage, regex-matching a _app-<hash>.js script tag, downloading that minified webpack bundle and regex-matching the minified JavaScript for the URL. The whole search path depends on it. Relative to that, this adds a dependency on the __NEXT_DATA__ tag and one pageProps path. The field names (comp_main, review_score, and so on) are the same ones the search API returns, so that surface is shared rather than additive.

I also checked the Next.js data route (/_next/data/{buildId}/game/{id}.json). It works and returns 6.4KB of JSON rather than 30KB of HTML, but buildId rotates on every deploy and can only be discovered by parsing __NEXT_DATA__ out of an HTML page, so it costs an extra request and a staleness path to arrive at the same object. Not worth it.

Two implementation details that are not obvious:

  • /game/{id} is requested directly rather than /game?id={id}. The latter answers with a 301, and the shared httpx client is built without follow_redirects, so the query form would have silently returned nothing.
  • The game page dates a release in full ("1996-02-27") where search returns just the year (2008), so _release_year normalizes both.

Failing loudly

A format change would otherwise degrade to a saved ID and an empty tab, which is byte-for-byte the symptom being fixed here and would get re-reported as this same issue. So an unreadable page raises 502 naming the cause, while a 404 or an empty record list stays benign. This includes a game_id presence check: build_hltb_game defaults every absent field, so a rename upstream would otherwise have silently returned no match forever.

The trade-off is that a genuine upstream rewrite fails the whole edit submit rather than half-succeeding. That felt like the right side to err on for a scrape-backed path, but happy to flip it.

Tests

12 new tests. hltb_game_page_example.json is a real payload captured from howlongtobeat.com/game/7169, trimmed to the fields the parser actually reads, and asserted through the full path down to the exact extracted metadata dict. It keeps our own parsing honest without touching the network in CI; it cannot detect an upstream change on its own.

I confirmed the endpoint tests are not vacuous by reverting the wiring: they fail with assert get_rom_by_id_mock.called is False, reproducing the reported bug. test_update_raw_hltb_metadata needed the handler pinned, as it now traverses the fetch path and would otherwise reach the network.

Also refactored while here: the 30-line dict to HLTBGame construction is now shared between the search and by-ID paths rather than duplicated, and the cover-URL build was already repeated three times.

Verified against live HowLongToBeat: a game with full data (id 7169), a sparse one with only two recorded times (id 68168), and an unknown ID, which returns cleanly rather than raising. Full backend suite: 2953 passed, 0 failed. trunk check clean.

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

Screenshots (if applicable)

N/A, backend only.


AI assistance disclosure

Written with Claude Code (Claude Opus 5): diagnosis, implementation, and tests. The absence of a by-ID API endpoint was established by probing the live service rather than assumed, and the parsing was verified end to end against live HowLongToBeat. Reviewed by me before submitting.

Fixes #2926

`update_rom` fetches from every metadata provider whose ID changed, except
HLTB, which had no by-ID lookup at all. A hand-entered ID was stored and the
How Long to Beat tab stayed empty.

HLTB's API only exposes search, so `get_rom_by_id` reads the record from the
`__NEXT_DATA__` payload the game page already ships. The canonical
`/game/{id}` path is requested directly, as `/game?id=` answers with a
redirect the shared httpx client does not follow, and the game page dates a
release in full where search returns just the year.

A page RomM can no longer read raises 502 naming the cause instead of
reporting a game with no times, since that failure is otherwise
indistinguishable from the bug this fixes.

Fixes rommapp#2926

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 19, 2026 01:20
@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fetches HowLongToBeat metadata when a ROM's HLTB ID is manually changed and extracts game data from the page's Next.js hydration payload.

  • Adds an HLTB by-ID page lookup, metadata normalization, and shared game/cover builders.
  • Wires changed HLTB IDs into the ROM update flow and clears metadata when the ID is removed.
  • Adds endpoint, parser, upstream-error, sparse-record, disabled-provider, and captured-payload tests.

Confidence Score: 4/5

The uncaught HLTB transport-error path should be fixed before merging because ordinary upstream connection failures can turn ROM edits into generic server errors.

The new direct page request handles HTTP statuses, connection refusal, and read timeout, but other routine httpx request failures escape through update_rom instead of preserving the provider's explicit service-unavailable behavior.

Files Needing Attention: backend/handler/metadata/hltb_handler.py

Important Files Changed

Filename Overview
backend/endpoints/roms/init.py Adds the missing HLTB changed-ID lookup and metadata merge to update_rom.
backend/handler/metadata/hltb_handler.py Adds by-ID HTML-page extraction and shared normalization helpers, but routine uncaught httpx transport failures can turn manual edits into generic 500 responses.
backend/tests/endpoints/roms/test_rom.py Verifies changed HLTB IDs invoke the handler, merge metadata, and remain persisted when no match is returned.
backend/tests/handler/metadata/test_hltb_handler.py Covers successful parsing, sparse payloads, disabled handling, unknown IDs, status errors, and malformed page formats, but not broader transport errors.
backend/tests/handler/metadata/hltb_game_page_example.json Provides a trimmed captured hydration payload for deterministic parser coverage.

Fix all with Greploop Fix All in Claude Code

Prompt To Fix All With AI
### Issue 1
backend/handler/metadata/hltb_handler.py:729
**Transport failures escape as 500s**

When the new game-page request raises an ordinary httpx transport error other than `ConnectError` or `ReadTimeout`, the exception escapes `update_rom`, causing a generic HTTP 500 instead of the provider's actionable HTTP 503 response.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(hltb): fetch metadata when a HowLong..." | Re-trigger Greptile

Comment thread backend/handler/metadata/hltb_handler.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a backend gap where manually setting a HowLongToBeat (HLTB) ID would persist the ID but not fetch HLTB metadata, leaving the HLTB tab empty. The PR adds a by-ID fetch path to the HLTB handler (via parsing the game page hydration payload) and wires it into the update_rom metadata refresh flow, with new unit and endpoint tests to prevent regression.

Changes:

  • Add HLTBHandler.get_rom_by_id() that fetches /game/{id} and parses __NEXT_DATA__ for the game record.
  • Update update_rom to refetch HLTB metadata when hltb_id changes, consistent with other metadata providers.
  • Add unit and endpoint tests, plus a captured payload fixture, to cover the new by-ID fetch behavior and failure modes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
backend/handler/metadata/hltb_handler.py Adds by-ID lookup by parsing Next.js hydration payload, plus shared builders for game and cover URL.
backend/endpoints/roms/init.py Wires HLTB into the existing “refetch provider metadata when ID changes” logic in update_rom.
backend/tests/endpoints/roms/test_rom.py Adds endpoint coverage ensuring update_rom triggers HLTB by-ID fetch when hltb_id is set/changed.
backend/tests/handler/metadata/test_hltb_handler.py Adds focused unit tests for by-ID parsing, error handling, and fixture-based parsing validation.
backend/tests/handler/metadata/hltb_game_page_example.json Adds a trimmed real-world __NEXT_DATA__ fixture used to validate the parser without network access.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread backend/handler/metadata/hltb_handler.py Outdated
Comment thread backend/handler/metadata/hltb_handler.py Outdated
sdornan and others added 2 commits August 18, 2026 20:30
`ConnectTimeout` is the likely failure when HLTB is slow or unreachable, and
it is not a subclass of `ConnectError`, so it escaped `update_rom` as a bare
500 rather than the actionable 503 the other providers give.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two ways the 502 could fire on a page RomM can still read:

The hydration tag was matched by exact attribute order, so a CSP nonce or a
reordered attribute would have been reported as a rewrite. The id alone
identifies it.

`raise_for_status` lets a 3xx through, so a hop HLTB added later would reach
the parser as a page with no payload. Redirects are now followed; the client
validates every hop against SSRF, redirects included.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gantoine
gantoine merged commit cd02cc5 into rommapp:master Aug 19, 2026
6 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.

[Bug] Manually adding How Long to Beat id does not fetch metadata properly

3 participants