Skip to content

fix(place-search): bound results by distance, before truncating - #11

Merged
timwangmusic merged 1 commit into
mainfrom
fix/place-search-distance-bound
Aug 30, 2026
Merged

fix(place-search): bound results by distance, before truncating#11
timwangmusic merged 1 commit into
mainfrom
fix/place-search-distance-bound

Conversation

@timwangmusic

Copy link
Copy Markdown

A user in San Francisco searched for a store and got Arizona. Reproduced against this service from (37.7749, -122.4194):

query radius sent in range what came back
Fry's Food and Drug 8000 0 / 10 Phoenix + Tucson, 920–1212 km
Bashas 8000 0 / 10 Kingman → Tucson, 804–1199 km
Fry's Food and Drug 500 0 / 20 same 20 rows as radius=16000

Why

location+radius on legacy Text Search is a ranking bias, not a restriction — the Maps SDK says so in the field's own doc ("prominent results from outside of the search radius may be included") — and params() on that endpoint emits no restriction parameter to send instead. parseTextSearchResponse filtered on place id, geometry, dedupe and ClosedPermanently, never on distance; it didn't even receive the origin.

This was the only place-returning path in the service with neither a distance bound nor a distance sort. Both nearby paths read Redis GEORADIUS, which genuinely is bounded, and SortPlacesByDistance / utils.HaversineDist already existed here — text search just called neither.

The change

Order of operations is the fix. Truncation moves out of the parse loop to after the filter and the sort. Cutting to the limit first leaves the bound with only Google's most prominent rows — the ones that can be a thousand kilometres away — and discards local matches ranked 11th–20th. A page is at most 20 places, so parsing all of it is free.

Bias and bound are now separate numbers. Google receives a narrower bias (PlaceTextSearchBiasRadius, 8 km) than the bound the response is filtered against (PlaceTextSearchDefaultRadius, 80 km). Widening the bias to match would push local results out of the single page the filter has to work with — backwards.

80 km because a text query names a place the user may drive to. MaxSearchRadius (16 km) is the nearby cap — the Redis widening ceiling — and it fails inside one metro (SF→San Jose is ~68 km). 80 km covers a metro end to end and sits an order of magnitude under the nearest wrong-state result measured (804 km). Absent or 0 bounds at the default; no opt-out, because unbounded is the bug.

The zero-origin gate is load-bearing: (0,0) is in the Gulf of Guinea, so filtering against it would drop everything. The HTTP handler already rejects a zero location, so it only covers direct callers and the existing tests.

Also corrects the handler's doc comment, which had recorded the belief that produced this — anchoring a query biases it, it does not bound it.

Verification

go test ./iowrappers/... ./planner/... green, go vet clean, gofmt clean. Five new tests: the Arizona set drops to empty, a cross-metro result survives at 80 km but not at the nearby radius, a local match below the limit survives truncation, survivors come back nearest-first, and a zero origin or zero radius disables the bound. The eleven existing parse tests are unchanged in behaviour — only the call signature moved.

Note: bin/mintpat does not build on main (stale client.NewPAT signature). Pre-existing, verified on the base commit, untouched here.

Deploy note

This is one Gin handler on the VM all four Convex deployments share, with no staged rollback. The app-side bound (offerbee-ai/Offerbee#305) is already independent of it, so this can merge on its own schedule.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FJSjdEEbVdGZrgJqeuJja1

A user in San Francisco searched for a store and got Arizona. Reproduced
against the live service: "Fry's Food and Drug" from (37.7749, -122.4194) with
radius=8000 returns ten Phoenix and Tucson stores, 920-1212km away, and
radius=500 returns the same twenty rows as radius=16000.

The radius was never doing what callers assumed. Legacy Text Search treats
location+radius as a ranking BIAS — the Maps SDK's own doc says "prominent
results from outside of the search radius may be included" — and there is no
restriction parameter on that endpoint. parseTextSearchResponse filtered on
place ID, geometry, dedupe and permanently-closed, and never on distance; it
did not even receive the origin. This is the only place-returning path in the
service with neither a distance bound nor a distance sort, because the two
nearby paths read Redis GEORADIUS, which really is bounded.

So the response is bounded here, and the order of operations is the fix.
Truncation now happens AFTER the filter and the sort, not during the parse
loop: cutting to the limit first leaves the bound with only Google's most
prominent rows — the ones that can be a thousand kilometres away — and discards
the local matches it ranked 11th to 20th. A page is at most 20 places, so
parsing all of it is free.

What Google receives is now a narrower bias (8km) than the bound it is filtered
against (80km default). Widening the bias to match would push local results out
of the single page the filter has to work with, which is backwards.

80km is the bound because a text query names a place the user may drive to:
16km, the nearby cap, fails inside one metro (SF to San Jose is ~68km), while
80km covers a US metro end to end and sits an order of magnitude below the
nearest wrong-state result measured (804km). Absent or 0 bounds at the default;
there is no opt-out, because unbounded is the bug.

The zero-origin gate is load-bearing — (0,0) is in the Gulf of Guinea, so
filtering against it would drop everything. The handler already rejects a zero
location, so it only covers direct callers and the existing tests.

Also corrects the handler's doc comment, which recorded the belief that
produced this: anchoring a query biases it, it does not bound it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FJSjdEEbVdGZrgJqeuJja1
@timwangmusic
timwangmusic merged commit 0e3b983 into main Aug 30, 2026
2 checks passed
@timwangmusic
timwangmusic deleted the fix/place-search-distance-bound branch August 30, 2026 01:12
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.

3 participants