fix(marketguard): stop the API stalling on every snapshot refresh

[Pankraz01]

Problem

The public API went unresponsive for seconds at a time whenever a market snapshot expired — which an uptime monitor records as the service being offline. Two independent causes, both reproduced and measured:

1. The Hypixel refresh only ever ran inside a client request.

LowestBinService.get_lowest_bins() refreshed on demand: the first caller after the 60s cache TTL expired paid for paginating the entire auction house, and the auction decode ran directly on the event loop:

for auction in auction_snapshot.auctions:      # ~100k auctions
    resolved_item = resolve_auction_item(auction)   # base64 + gzip + NBT parse

Benchmarked on real-shaped payloads:

auctions	item size	event loop blocked
80,000	~420 b64 chars	4.5s
80,000	~520 b64 chars	6.8s
80,000	~656 b64 chars	10.3s

With WEB_CONCURRENCY=1 (kept deliberately, per DEPLOY.md, for the process-local rate limiter) that froze everything for the duration — including /api/v2/lowestbin and /api/v1/bazaar themselves, which is what the uptime monitor polls. On top of that, the service's asyncio.Lock was held across the whole refresh, so concurrent callers queued behind it even before the decode started. This is the cause of the observed availability drop.

2. /api/v1/ready reported healthy snapshots as degraded.

Readiness used cache_ttl_seconds (60s) as its freshness bound. Hypixel only regenerates these datasets about once per minute, so a perfectly normal snapshot was routinely just over the line and returned HTTP 206. Not what the monitor in this deployment watches, but wrong on its own terms and fixed here too.

Changes

• app/marketguard_api/refresher.py (new) — background refresher keeping the Lowest BIN and Bazaar snapshots inside their TTL on a jittered timer. Requests read a warm snapshot from MariaDB instead of triggering an upstream fetch. Backs off to a shorter retry interval on upstream failure, and hands refreshing back to the request path on shutdown so nothing is left unrefreshed.
• service.py — auction decode moved to a worker thread (asyncio.to_thread); read path made lock-free via double-checked locking so a running refresh no longer queues up client requests; per-snapshot metadata now travels with the snapshot it belongs to rather than living in mutable service state, so a concurrent refresh cannot pair a price with another snapshot's auctioneer UUID.
• routes.py — readiness grace window, so a normally-aged snapshot reports 200.
• main.py / app/main.py — refreshers started and stopped by the app lifespan, in both the standalone api mode and the combined platform mode.
• tests/test_marketguard_api.py — also repairs two player-query tests that had been failing on main since before this branch (see the second commit); they were deterministic test bugs, not infrastructure.

New settings, all with safe defaults and documented in .env.example, README.md and DEPLOY.md:

setting	default	purpose
MARKETGUARD_BACKGROUND_REFRESH_ENABLED	true	refresh off the request path
MARKETGUARD_BACKGROUND_REFRESH_INTERVAL_SECONDS	0 (= cache TTL)	refresh cadence
MARKETGUARD_BACKGROUND_REFRESH_RETRY_SECONDS	15	retry after a failed refresh
MARKETGUARD_READINESS_FRESH_GRACE_SECONDS	120	readiness tolerance

Verification

End-to-end against a real Uvicorn server with an out-of-process probe (an in-process probe cannot measure an event-loop stall — it gets stalled too), 90,000 synthetic auctions, steady client traffic:

	probe healthy	timeouts	worst response
before	110/112 (98.2%)	1	5.00s (probe timeout = recorded down)
after	146/146 (100%)	0	0.07s

10 new tests. The key regression test asserts on thread identity rather than timing, so it is deterministic — verified to fail when the to_thread hand-off is reverted:

assert observed["decode_thread_id"] != loop_thread_id
E       assert 139947550937216 != 139947550937216

Full suite: 123 passed, 0 failed.

Deployment note

No migration needed. After deploy, docker compose logs scamscreener-api should show Started MarketGuard lowestbin background refresher and the same for bazaar.

Monitoring: /api/v2/lowestbin and /api/v1/bazaar are now served from the stored snapshot and no longer block on an upstream fetch, so they are safe to poll directly. /api/v1/ready is also available — treat both 200 and 206 as up there, since 206 means "serving slightly stale data" and only 503 means no usable snapshot. /api/v1/health is intentionally 403 from public networks and must not be monitored.

🤖 Generated with Claude Code

https://claude.ai/code/session_0187ZiUfeTQFKtLB4K6JFGWa

[Pankraz01]

CI: Server Security / test-and-audit is red, but not because of this PR

main is red with the exact same two failures, so this is not a regression from these changes:

run	commit	result
#41 (main, push)	87bb1b2	2 failed, 111 passed
#42 (this PR)	605aa9a	2 failed, 121 passed

Same two tests in both, and the 10 new tests added here all pass:

• test_players_query_returns_teapot_for_invalid_server_hypixel_api_key — MarketGuardStorageError: Failed to initialize MarketGuard MariaDB schema.
• test_players_query_coalesces_identical_cache_misses_and_tracks_efficiency — assert [419, 419, 419] == [200, 200, 200]

Not a flake and not infrastructure — both are deterministic test bugs:

1. The first test passes no MarketGuard services to create_app, so register_marketguard_routes constructs a real MarketGuardStorage, which tries to connect to MariaDB on 127.0.0.1:3306. The workflow runs pytest with no database service, so schema init fails. Every other test in the file passes a _MemoryMarketGuardStorage instead.
2. The second expects 200 but configures no Hypixel API key, and players_query returns 419 when hypixel_api_key is empty — so it asserts against behaviour the route no longer has.

Proposed patch (verified locally: 123 passed, 0 failed)

Not applied here, to keep this PR scoped to the availability bug. Happy to push it here or open a separate PR — whichever you prefer.

--- a/tests/test_marketguard_api.py
+++ b/tests/test_marketguard_api.py
@@ -1046,9 +1046,12 @@ def test_players_query_returns_teapot_for_invalid_server_hypixel_api_key(tmp_pat
         return httpx.Response(403, json={"success": False, "cause": "Invalid API key"})
 
     settings = _marketguard_settings(hypixel_api_key="invalid-hypixel-key")
+    marketguard_service, marketguard_bazaar_service = _noop_marketguard_services(settings)
     app = create_app(
         training_hub_settings=_training_hub_settings(tmp_path),
         marketguard_settings=settings,
+        marketguard_service=marketguard_service,
+        marketguard_bazaar_service=marketguard_bazaar_service,
         marketguard_player_service=_marketguard_player_service(
             settings,
             _hypixel_handler,
@@ -1141,7 +1144,7 @@ def test_players_query_coalesces_identical_cache_misses_and_tracks_efficiency()
         async def aclose(self) -> None:
             return None
 
-    settings = _marketguard_settings(players_rate_limit_per_minute=3)
+    settings = _marketguard_settings(players_rate_limit_per_minute=3, hypixel_api_key="test-key")
     marketguard_service, marketguard_bazaar_service = _noop_marketguard_services(settings)
     counting_player_service = _CountingPlayerService()
     app = create_marketguard_app(

Worth fixing soon either way: while CI is red by default, a real regression in a future PR is easy to miss.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15a5aa19cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Disable inline refresh before the first attempt

When the first scheduled refresh after a restart fails while MariaDB already has an expired-but-still-servable snapshot, this line is never reached, so inline refresh remains enabled. LowestBinService._load_snapshot and BazaarService.get_bazaar then skip their nonblocking stale-data branches, and concurrent cache misses perform serialized upstream requests before falling back; during a Hypixel outage this recreates long request stalls until a background attempt succeeds. Disable inline refresh when the refresher starts rather than only after a successful refresh.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Bound the full readiness window by the stale cutoff

For allowed settings such as a 300-second cache TTL, 300-second stale cutoff, and 300-second grace, this validation passes even though readiness_fresh_seconds becomes 600 seconds. At snapshot age 400, /api/v1/ready therefore returns 200 while the data services refuse to serve that snapshot after an upstream failure and return 503. Compare cache_ttl_seconds + readiness_fresh_grace_seconds against the stale cutoff, or cap the readiness threshold, so unusable data is never reported as healthy.

Useful? React with 👍 / 👎.