Skip to content

feat: place text-search with durable cache insert + unified type classification - #449

Merged
tim-eternos merged 8 commits into
masterfrom
feat/place-text-search
Jul 31, 2026
Merged

feat: place text-search with durable cache insert + unified type classification#449
tim-eternos merged 8 commits into
masterfrom
feat/place-text-search

Conversation

@timwangmusic

Copy link
Copy Markdown
Owner

Description

Adds free-text place search with durable cache insertion, closing the gap where the legacy Nearby Search never returns certain places at all (verified live: Konjoe Burger, a food-hall stall at State Street Market, is absent from the cache even immediately after a fresh cold search — downtown Los Altos has only 7 cached eateries within 2 km).

Consumed by OfferBee's best-card page: when a user can't find a place in the nearby list, they explicitly search, pick the right result, and that place is inserted into the geo cache — immediately answering "which card should I use here" and making the place durably visible in category results for everyone.

Solution

1. Expanded type→category reverse map, one rule everywhere (9b6d167)
GetPlaceCategory grows from an 18-type switch to a 43-entry placeTypeToCategory table (meal_delivery→Eatery, convenience_store and the *_store family→Shopping, tourist_attraction/stadium/movie_theater→Visit, drugstore/beauty_salon→Wellness, …). ReclassifyForCategory (the category read filter) now keys on the same map as the cleanup migration — one rule for "does this place belong in this category".

The change is monotone: the map is a strict superset of the searched types (pinned by TestGetPlaceTypesSubsetOfCategoryMap), so strictly more cached places become visible on category reads, never fewer. No data migration needed. GetPlaceTypes — the searched subset that shapes outbound Google queries — is byte-identical, so no new ?type= values can reach the legacy API (the #446/#447 incident vector stays closed; TestGetPlaceCategoryKeysAreGoogleTypes structurally guards the map against non-legacy values, and fast_food_restaurant/food_court remain deliberately refused).

2. Text search + candidate stash + confirm-insert (2a2ecdb, a23d808)

  • POST /v1/place-search: legacy Text Search with query+location+radius only — never a type filter (that's the incident vector) — parsed WITHOUT the zero-ratings filter that hides exactly the new/obscure places this feature targets. Every candidate is stashed server-side (30-min TTL).
  • POST /v1/place-search/confirm: takes only {placeId}, resolves it against the stash (fabricated data can never reach the cache), derives the category server-side from Google's primary type — unmapped types are refused with 422 and nothing is written — enriches via one best-effort Place Details call (hours/URL/photo), never clobbers a previously-cached record's real hours or photo, writes through the audited SetPlacesAddGeoLocations, and read-back-verifies.
  • Anti-pollution is structural: the confirm response's category and the Redis bucket are two reads of the same map with the same key — a mismatch is unrepresentable.

3. Docs (d6b71da, a272365, 2a61462)
README API docs incl. a Visibility note (bucket membership is immediate; category-read visibility in a cold cell takes until the second read, because the freshness-marker path replaces rather than unions cached members — pre-existing behavior, follow-up filed; never stamp MapsLastSearchTime to force it). The reclassify-buckets runbook now carries the complete production residue audit: 27 records = 5 newly removable (shoe_store×2, furniture_store, hardware_store, stadium) + 1 legitimized (night_club — Cain's Ballroom) + 21 residue. Dry-run counts WILL change vs the old 145/0/12/4/3 baseline because the map changed — expected, not a red flag. Do not apply=true without reviewing the new dry-run output.

Deploy ordering (matters)

OfferBee's reward-table change (already merged on their side as C1) must deploy before this does. The expanded map means category reads immediately start emitting re-tagged locationType values (meal_delivery, convenience_store, …); without the reward-table entries those fall back to base rate. Order: OfferBee C1 → this PR → dry-run the cleanup per the updated runbook → OfferBee's UI PR.

New endpoints share the existing global 100 req/hr/IP rate limiter with the category search (single Convex egress IP) — the client gates search behind an explicit tap, but 429s on nearby reads are the metric to watch after deploy. Confirm costs 1 Text Search + 1 Place Details billed call per user action.

Testing

  • Integration testing on Heroku staging — not done; needs a deploy + the smoke steps in the README Visibility note (verify via ZSCORE, expect up to two category reads in a cold cell)
  • Added new unit tests

~60 new tests across 6 files (map/reclassify pins incl. a monotonicity property test and mutation-verified guards, text-search parser rules, insert-flow truth table on miniredis incl. nothing-written-on-422, handler auth/validation incl. an end-to-end 422 with a real unmapped type, hermetic wire-shape marshal tests). go build -v . clean, go test -v ./... 6/6 packages, go vet clean, suite also green under -shuffle=on.

Checks

  • Have you removed commented code?
  • Have you used gofmt to format your code? (gofmt -l empty)

tim-eternos and others added 8 commits July 30, 2026 16:59
GetPlaceCategory was a switch covering only the 18 types the categories
actively search for. Expand it into a package-level placeTypeToCategory
map with 25 more entries so Google primary types (tourist_attraction,
grocery_or_supermarket, drugstore, movie_theater, etc.) classify
correctly, while keeping the no-default-category invariant intact
(GetPlaceCategory("") still returns ("", false)) and leaving
GetPlaceTypes - the searched subset - untouched.

Unify ReclassifyForCategory onto the same map: it now keeps a place
when GetPlaceCategory(primary) == cat instead of scanning
GetPlaceTypes(cat), giving one classification rule shared by the
nearby-search write path, the bucket-cleanup migration, and the
merchant-endpoint read filter. Because the map is a strict superset of
the old searched-types union, this is provably monotonic: it can only
keep more places than before, never fewer (pinned by
TestReclassifyForCategoryKeepsAllFormerlySearchedTypes).

Extend test/place_category_test.go with the 25 new known-type cases,
8 new unmapped-type refusals used by the future text-search 422 path,
a structural guard that every mapped key is a real Places API type
(TestGetPlaceCategoryKeysAreGoogleTypes), and the searched-subset
round-trip test. Update test/redis_client_mocks/bucket_cleanup_test.go:
meal_delivery/night_club rows now keep their verdict for the correct
reason (positively mapped to Eatery, not unmapped), plus four new
truth-table rows covering types that only just became mapped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
Builds the iowrappers-layer machinery for free-text place search: Google
Text Search -> parsed POI.Place candidates -> stashed server-side by place
ID with a 30-minute TTL -> user-confirmed insert into the shared Redis geo
cache. No HTTP wiring; that is a later task against PoiSearcher.TextSearchPlaces
and PoiSearcher.AddSearchedPlaceToCache.

- MapsClient.TextSearchPlaces sets only Query/Location/Radius on the Google
  request (no Type, no OpenNow) since an unenforceable Type filter is what
  previously let hotels get cached as eateries.
- parseTextSearchResponse (pure) skips empty PlaceID, {0,0} geometry,
  CLOSED_PERMANENTLY, and dedupes by PlaceID, but deliberately keeps
  zero-rating results (unlike parsePlacesSearchResponse) since those are
  exactly the new/obscure places this feature exists for. A blank business
  status is treated as Operational rather than invisible.
- PoiSearcher.AddSearchedPlaceToCache refuses entirely (no write at all)
  when the candidate's primary Google type has no POI.PlaceCategory mapping,
  and otherwise best-effort enriches via Place Details, restores any
  previously-cached real opening hours, writes through
  SetPlacesAddGeoLocations, and reads back via CachedPlaces to fail loudly
  on a silent Redis write failure.
- RedisClient gains SetPlaceSearchCandidate/PlaceSearchCandidate stash
  methods beside setPlace/getPlace, under their own key prefix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
Two Important findings from Task 2 review, both local to text_search.go:

1. A lean re-confirm of an already-cached place could silently overwrite
   a real Photo.Reference with the zero value: restoreCachedDetails (shared
   with the nearby-search write path, left untouched) restores URL/Summary/
   FormattedAddress/Address/Hours but not Photo. Gap-fill the photo locally
   from the cached record after restoreCachedDetails runs. Also fold a
   Details-sourced photo into the place when the candidate had none -
   "photos" is already a requested detailed_search_fields entry, so the
   confirm path was paying for it and throwing it away.

2. The Place Details enrich closure passed the caller's raw context
   through to the Google call. The maps SDK's HTTP client has no timeout
   of its own and an inbound request context carries no deadline by
   default, so a hung call could park one of the 5 shared apiSemaphore
   slots indefinitely, starving every other Google call process-wide.
   Extracted the closure into newPlaceDetailsEnricher (bounded by
   GoogleMapsSearchTimeout before acquiring the semaphore) so the fix is
   independently testable with a stub search function instead of a hang
   harness or a real Google client.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
Wires Task 2's iowrappers text-search machinery to two authenticated
endpoints: POST /v1/place-search (free-text Google search, returns the
{"results": [...]} envelope an external Convex client expects) and
POST /v1/place-search/confirm (inserts a candidate into the shared
cache, returns a single {"place","category","alreadyCached"} object).

Both handlers mirror getNearbyPlaces/getNearbyPlacesByCategory's style
exactly: auth first, ShouldBindJSON -> 400, the same zero-location
rejection (proven by test to short-circuit before any Google call),
and the same searchContext construction. confirmSearchedPlace maps the
two iowrappers sentinel errors to 404 candidate_expired and 422
unsupported_place_type (extracting the quoted primary type via
strconv.Unquote, the exact inverse of the %q the sentinel was built
with).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
Add README coverage for the new POST /v1/place-search and
/v1/place-search/confirm endpoints (request/response shapes, error
codes, auth, and the server-side-category safety design). Update
docs/migrations/reclassify-buckets.md to reflect that meal_delivery
and night_club now positively map to Eatery instead of falling out as
unmapped residue, that movie_theater/stadium/hardware_store are newly
removable when found in the wrong bucket, and that the cleanup rule is
now unified with the read filter (ReclassifyForCategory) on the same
primary-type map while still deliberately diverging from the write
rule. Fix the RemoveMisclassifiedPlacesFromCategoryBuckets docstring's
stale meal_delivery/night_club "unmapped" examples (comment-only, no
logic change).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
Replace the unsubstantiated movie_theater residue claim and the "~24"
estimate in reclassify-buckets.md with the controller's authoritative
production audit: all 27 records in placeIDs:eatery by primary type,
correctly splitting into 5 removable (shoe_store x2, furniture_store,
hardware_store, stadium), 1 legitimized (night_club), and 21 still
unmapped residue. movie_theater is now labeled as a rule/test-fixture
example only, never an observed production record. Note that the
original itemization's 27-vs-20 mismatch was from eliding 7 records,
not an error in the total.

Fix the false "unlike nearby search" location-requirement comparison
in README.md and the matching planner.go comment: both
getNearbyPlaces/getNearbyPlacesByCategory already enforce the same
zero-location rejection, so there is no such asymmetry.

Split the ReclassifyForCategory docstring's inaccurate merged bullet
in POI/categories.go into two correct ones: no Types keeps a place
unchanged, but a primary type that is present-and-unmapped drops it
(same as the pre-unification behavior) — comment-only, no logic
change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
The cases map covered Eatery/Shopping/Lodging/Wellness but omitted
Visit, the one category where GetPlaceTypes had grown most and where
silently widening the searched-types list would go unguarded. Add the
Visit row (Park/AmusementPark/Gallery/Museum) so the suite fails if a
fifth type is ever added to GetPlaceTypes(Visit).

Verified RED: temporarily added LocationTypeTouristAttraction to
GetPlaceTypes(Visit) and reran the test, which failed with
"got 5 place types ... want 4" before the mutation was reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
…number

- README: document cold-cell visibility for the confirm endpoint —
  cache membership is immediate (verify via the confirm response or
  ZSCORE placeIDs:<category> <placeID>), but appearance in
  /v1/nearby-places-by-category needs a warm cell or a second read
  after a cold search replaces (not unions) the cached bucket. Never
  manually stamp MapsLastSearchTime to force it. Docs only, no code.
- README: fix radius wording from "when zero or larger" (reads as
  "always") to "when zero or larger than that maximum".
- iowrappers/text_search.go: add json tags to AddSearchedPlaceResult
  (place/category/alreadyCached) to match its sibling
  PlaceSearchCandidate; POI.Place itself stays untagged so this
  doesn't change every endpoint's wire shape at once. Added a
  hermetic marshal test pinning the exact top-level key sets and the
  nested place object's capitalized field names the Convex client
  depends on.
- planner/planner.go: use iowrappers.PlaceTextSearchMaxResults
  instead of a literal 20 for the search handler's limit cap so the
  two constants can't drift; the default-10 literal is unchanged.
- docs/migrations/reclassify-buckets.md: clarify "These 4 records —
  5 counting both shoe_store instances" to "These 4 primary types —
  5 records, counting both shoe_store instances" since the count is
  over primary types, not records.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DjdGnZM6cUBeg6jWwoQfU3
@timwangmusic
timwangmusic requested a review from tim-eternos July 31, 2026 04:40
@tim-eternos
tim-eternos merged commit a96807a into master Jul 31, 2026
4 checks passed
@tim-eternos
tim-eternos deleted the feat/place-text-search branch July 31, 2026 04:40
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