Skip to content

Cut Google Maps spend: reverse-geocode cache, Details mask trim, confirm skip, closure persistence - #450

Merged
tim-eternos merged 2 commits into
masterfrom
perf/maps-cost-free-wins
Aug 2, 2026
Merged

Cut Google Maps spend: reverse-geocode cache, Details mask trim, confirm skip, closure persistence#450
tim-eternos merged 2 commits into
masterfrom
perf/maps-cost-free-wins

Conversation

@timwangmusic

Copy link
Copy Markdown
Owner

The no-wire-change batch from the 2026-08-01 geo cost/freshness audit. Four cost cuts plus one zero-cost freshness fix:

1. Reverse-geocode cache (the big one)

Every nearby scan buys a reverse-geocode via processLocation — and on a warm place cache it was the only Google call the scan made, so it was ~100% of warm-scan spend. Results are now cached per ~8 km search cell (geocode:reverse:<cell>, 30-day expiry per Geocoding caching terms). The RedisClient.ReverseGeocode stub is now a real implementation; processLocation, PoiSearcher.ReverseGeocode, and the /v1/reverse-geocoding handler all route through it.

2. Details field-mask trim

detailed_search_fields drops name and user_ratings_total: no Details consumer reads either (both arrive free with every Nearby/Text Search result), and user_ratings_total alone pulls every Details call into the Atmosphere billing tier. The AddUserRatingsTotal migration passes its own single-field list and is unaffected. editorial_summary stays (trip planner reads it) — note that means the Atmosphere tier still applies until that's revisited. Pinned by TestDetailedSearchFieldsMask.

3. Confirm skips redundant Details

/v1/place-search/confirm bought a full max-tier Place Details call even when the cached record's details were current — data restoreCachedDetails restores anyway. It now applies the same placeDetailsAreCurrent rule the nearby path already trusts. This also makes Offerbee's wallet-change re-confirm of manual rows free in the common case.

4. Photos-only mask in stale-photo recovery

photos_client.go requested the entire Details mask to recover one photo reference; now ["photos"].

5. Closures are persisted (freshness fix, zero cost)

Cold searches filtered non-OPERATIONAL results before the cache write, discarding the closure signal entirely — a permanently closed place kept its OPERATIONAL record and cache membership forever. The write now includes closures (new persistAndFilterSearchResults owns write + response filter); the response-side behavior is unchanged, and the existing read-side Operational filter retires closed places from cache serves.

Testing

  • TDD throughout: new suites reverse_geocode_cache_test.go (round-trip, cell scoping, expiry, cache-first searcher + processLocation), add_searched_place_currency_test.go (skip-when-current, buy-when-stale, buy-when-uncached), closure_persistence_test.go (persist + response exclusion + read-path retirement), config_yaml_test.go (mask pin).
  • go vet ./... clean; full go test ./... green (7 packages).
  • The photos one-liner has no unit test — that call site has no stub seam; covered by build + the narrowed literal.

Deliberately not in this PR: hoursKnown wire flag (API change), text-search caching + timeout salvage (larger), MapsLastSearchTime dead-field purge (Redis ops).

🤖 Generated with Claude Code

https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3

tim-eternos and others added 2 commits August 1, 2026 15:04
… redundant confirm Details, persist closures

Four API-cost fixes plus one freshness fix that costs nothing, from the
2026-08-01 geo cost/freshness audit. No wire-contract changes.

- Reverse-geocode results are now cached in Redis per ~8 km search cell
  (30-day expiry, matching Geocoding caching terms). This was the ONLY
  Google call a warm nearby scan made — processLocation buys it on
  every request — so a warm scan's Google spend drops to zero. The
  RedisClient.ReverseGeocode stub is now real; processLocation and the
  reverse-geocoding endpoint route through the cached path.

- detailed_search_fields drops name and user_ratings_total: no Details
  consumer reads either (both arrive free with every Nearby/Text Search
  result), and user_ratings_total alone pulls every Details call into
  the Atmosphere billing tier. The AddUserRatingsTotal migration passes
  its own field list and is unaffected. Pinned by
  TestDetailedSearchFieldsMask.

- place-search confirm skips its Place Details call when the cached
  record's details are current (same placeDetailsAreCurrent rule the
  nearby path trusts) — re-confirming a place we already hold was a
  full-price max-tier call for data restoreCachedDetails restores
  anyway.

- Stale-photo recovery requests only the photos field instead of the
  full Details mask.

- Cold searches now PERSIST permanently-closed places instead of
  filtering them out before the cache write. The old order discarded
  the closure signal entirely: the stale record kept OPERATIONAL status
  and cache membership forever. The response-side filter is unchanged
  (closures never reach callers); the read-side Operational filter now
  retires them from cache serves at zero extra API cost.

Not included (deliberately): the hoursKnown wire flag (API change),
text-search caching and timeout salvage (larger changes), and the
MapsLastSearchTime dead-field purge (Redis ops, not code).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
…es flag

Now that closures are persisted in the cache, the old contract was one
forgotten field away from breaking planners: filtering only happened
when a caller remembered BusinessStatus: POI.Operational, so a
zero-value PlaceSearchRequest would serve permanently-closed places
from both cache reads and cold-search responses.

The zero value is now safe: non-Operational places are always filtered
unless the caller explicitly opts in with IncludeClosedPlaces. All
three production call sites (matcher, both planner handlers) already
requested Operational filtering, so their behavior is unchanged — the
field they set is simply gone. The RemovePlaces migration test opts in:
it verifies raw bucket contents and its fixtures carry no Status.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
@timwangmusic

Copy link
Copy Markdown
Owner Author

Added a fail-safe flag on top of the closure-persistence change: PlaceSearchRequest.BusinessStatus is replaced by IncludeClosedPlaces (zero value = non-Operational places always filtered, from cache reads and cold-search responses alike). Previously a caller had to remember BusinessStatus: POI.Operational to get filtering — with closures now actually present in the cache, a forgotten field would have served closed places to planners. All three production call sites already requested filtering, so behavior is unchanged; only the failure mode is gone.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3

@tim-eternos tim-eternos left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@tim-eternos
tim-eternos merged commit 3a22b81 into master Aug 2, 2026
4 checks passed
@tim-eternos
tim-eternos deleted the perf/maps-cost-free-wins branch August 2, 2026 00:24
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.

2 participants