fix: stop re-searching Google Maps on every nearby-search request

[timwangmusic]

Problem

Production logged the number of results from redis is 0 on nearly every request, each followed by a full Google Maps fan-out. The cache wasn't warming — it was being bypassed. Two defects compounded.

The newer categories were never wired like Visit. EncodeNearbySearchRedisKey appended a price segment for Eatery only, so Shopping/Lodging/Wellness got one price-agnostic bucket — but Get/SetMapsLastSearchTime special-cased only PlaceCategoryVisit when dropping that segment. Those three ended up with a price-scoped marker over a price-agnostic bucket. Visit had neither segment, Eatery had both; the newer categories were the only ones where the two disagreed.

Worse, the marker was keyed on country:admin1:city while the bucket it guards is a geo index read from the caller's exact coordinates. Trip planning searches from a city centroid, so the two roughly agreed. The merchant endpoint — the only caller of the new categories — searches from the user's precise location (planner.go:1402-1405). A suburban user read "Shopping in Los Angeles is fresh" against a 498-member bucket with nothing inside 16 km. These categories are 30–120× sparser than the old ones (Shopping 498 / Wellness 319 / Lodging 123 vs Visit 14,674 / Eatery 8,589), so they hit it constantly.

A zero-result read could never be suppressed. The gate required len(savedPlaces) > 0, so the marker was only ever consulted alongside a non-empty result. Every such request fell through to Google, re-stamped the marker, and repeated on the very next request — forever rather than once.

Separately, the eatery price split cost three identical Google searches per area: Google omits price_level for most places (so they collapsed into level0) and only accepts a price filter at level ≥ 3, so levels 0/1/2 issued identical requests yet each read back a fifth of the data — while solver.go:527 was already filtering on price after the read.

Changes

• One price-agnostic bucket per category, mirroring placeIDs:visit. EncodeNearbySearchRedisKey(cat) drops the level parameter rather than ignoring it, so the compiler walked every call site.
• The marker is scoped to the external search, keyed on an ~8 km location cell matching ColdStartSearchRadius: <cell>:<category>, plus :pricey<N> for eatery levels 3–4 — the only levels where PriceyEatery makes Google apply a real price filter at 4× radius. Levels 0–2 share one field. Visit needing no price segment now falls out of the rule instead of being an exception. Brand markers get the same treatment; they had the identical bug.
• Empty results are honoured with a 24 h window instead of 14 days. The stamp moves below the searchErr return — required, since a failed search would otherwise silence retries for a day.
• Place Details is no longer re-bought. The placesToUpdate hook was stubbed with a literal // placeholder for filtering places that do no need updates; it's now populated from a pipelined cache lookup, filtered before the DetailsLimit cap so the budget goes to places we lack, and gated on LastUpdatedAt (90 d) so records can't freeze. restoreCachedDetails fills gaps only — the write path is a blind upsert, so a skipped place would otherwise overwrite the record it was skipped for.
• Read amplification: one pipelined fetch instead of a GET per member (reusing the existing getPlacesPipelined), a GEORADIUS COUNT bound, no more req.Radius write-back. The union/merge-sort path and AllPriceLevels are deleted with the price split.
• removePlace ZREMmed "placeIDs:eatery:"+priceLevel — missing the level prefix writes used — and never touched the three newer buckets, so it deleted records and orphaned their geo members (~328 recorded in the runbook). It now iterates POI.AllPlaceCategories through the shared encoder. GetPlaceCountByCategory had the same bug, which is why /stats/places always reported zero eateries.

Migration — run BEFORE deploying

GET /v1/migrate/union-eatery-buckets, dry-run unless apply=true. Runbook: docs/migrations/collapse-eatery-buckets.md.

It's additive and invisible to the running code; deploying first would point every eatery read at a key that doesn't exist yet. AGGREGATE MIN is mandatory — a GEO member's score is its 52-bit geohash, and the SUM that redis.ZStore.Aggregate defaults to would add the scores of any place present in two buckets and relocate it into the ocean.

Marker fields also re-key, so expect one cold search per occupied cell per category after deploy. Geo buckets are untouched, so reads return the full member set immediately.

Verification

gofmt -l . clean, go vet ./... clean, go test ./... -count=1 passing.

Also driven against local dev data that happened to be in the exact pre-migration state:

Check	Result
Collapsed key before migration	absent → reads returned 0 at every price level (the deploy-order hazard, live)
Union dry run → apply	expected_after=111 → target_after=111, exact match
Coordinate integrity	111/111 checked, 0 drifted; 50 m search finds the sample at 0.1 m
Read across price levels	49 places at every level, identical (was 0)
Shopping marker	written at level 0, hits at all five levels — asymmetry gone
Eatery marker	levels 0–2 hit, 3–4 miss — pricey searches keep their own marker
Marker 22 km away	miss, as intended

New tests cover the invariant that broke (bucket key identical across all price levels), marker collapse and pricey separation, cell sharing at ~2 km and splitting at ~20 km, the freshness gate including the empty-result case, AGGREGATE MIN preserving coordinates, migration re-runnability, removePlace clearing every bucket, and the Details skip (zero calls when all cached, refresh when stale, nil lookup unchanged, no record degradation, budget spent on unstored).

Notes for review

• One deviation from plan. Opening hours and formatted address were intended as Details-completeness signals but are unusable: CreatePlace backfills every blank weekday, so hours are never empty and the check would always pass; and the Nearby response carries its own FormattedAddress. URL has exactly one source — a Details result — so it's the whole list (detailsSourcedFields).
• One planned test is absent. "Maps error → marker not stamped" is enforced by statement ordering; asserting it needs a stubbable maps client, and PoiSearcher.mapsClient is the concrete *MapsClient. The existing SearchClient interface is the natural seam — left as follow-up rather than faked.
• Out of scope, still live: redis_client.go hot-spins when req.Radius == 0 (0 * 2 == 0, and MinNumResults > 0 means the break never fires). Reachable unauthenticated via getOptimalPlan, which writes a 400 but never returns. I touched that loop for COUNT and pipelining but deliberately left the termination bug alone.
• Unrelated finding while reading auth for end-to-end testing: PATs are stored in plaintext as Redis key names despite the Hash/tokenHash naming — NewPAT sets Hash: token and keys on pat_hash:<token>, and validation looks up the raw Authorization value. Worth its own issue.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WHNC45T5vRTKuEmifJANCG

[tim-eternos]

lgtm