feat: territory search by bounding box or GeoJSON polygon - #6
Merged
Conversation
Address search could filter by a circle (lat/lng/radius) and by structured
fields, but not by a shape. A territory is rarely a circle: a sales area, a
delivery zone, a canvassing route and a map viewport are all polygons or
rectangles, and approximating one with a radius drags in everything in the
corners.
GET /addresses?bbox=-83.1,39.9,-82.9,40.1
GET /addresses?polygon={"type":"Polygon","coordinates":[[...]]}
Both compose with the existing filters and with text search, so a query can be
"Barendt, inside this neighbourhood" rather than one or the other.
bbox is longitude-first, matching GeoJSON, Leaflet, MapLibre and PostGIS, so a
caller can pass a viewport straight through. Latitude-first is the common
mistake and it is silent -- a swapped pair still parses and just returns
nothing -- so the bounds are validated and a bad box is a 400 naming what is
wrong, not an empty result. The same reasoning applies to the polygon: it is
checked for type, ring closure and coordinate ranges here, because
ST_GeomFromGeoJSON raises on malformed input and a caller's typo should not
surface as a 500.
Both predicates are index-assisted. && against the GIST index on geom is exact
containment for point geometry, and ST_Intersects shortlists on the same index
before testing candidates. Verified by plan: Bitmap Index Scan for both.
Tests assert the rectangle actually restricts (101 of 600 rows) and that every
returned point is inside it, that a polygon excludes points its own bounding
box includes, that territory filters compose with text search without widening
the result, and that a reversed or malformed box is rejected rather than
returned empty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S2nhUJ4Zdsxf7cP9DapsY3
gameplan generates its client from this file with openapi-typescript, so a parameter missing here is a parameter that repo's client cannot send. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S2nhUJ4Zdsxf7cP9DapsY3
**NaN walked past every bbox guard.** ParseFloat accepts "NaN", and every comparison against NaN is false, so all three validation blocks passed and the value reached ST_MakeEnvelope -- producing an empty 200 or a 500, which is the exact silent failure the validation exists to stop. Non-finite values are now rejected before the range checks rather than by them. **The bbox filter was not exact.** The comment claimed && on point geometry is exact containment. It is not: && compares BOX2DF values, which are float4 and rounded outward, so at Ohio longitudes one ULP is about 0.7 m of slack. Verified directly -- a point at lng -82.8999995 against a box ending at -82.9 gives `&& = true` while `ST_Intersects = false`. A client tiling a territory into adjacent boxes double-counted every address within 0.7 m of a shared edge; a test now seeds a point on such an edge and asserts it appears in exactly one of two neighbouring boxes. With && restored it appears in both. Now ST_Intersects, matching what county_service.go already does, still index- assisted. **Only the exterior ring was validated.** A Polygon whose hole is short or unclosed passed and was rejected by GEOS inside ST_Intersects, surfacing as a 500 on a request that was merely wrong -- the outcome the validator exists to prevent. Every ring is checked now, and errors name which one. **A wrong geometry type produced an unhelpful error.** Decoding type and coordinates together made a Point fail on its coordinates with "cannot unmarshal number into .coordinates.0 of type [][]float64", telling the caller nothing about the real problem. The type is read first. **Nothing bounded polygon complexity.** An exact point-in-polygon test runs per candidate row, so cost scales with vertices as well as rows, and a shape traced off a map can carry tens of thousands. Capped at 1000 positions. **Two tests were weaker than they looked.** The polygon test compared totals and cleared by a single row -- the band's true bounding box is slightly wider than the rectangle it was compared against, so any fixture change would have flipped it into a failure blaming the polygon. It now asserts on the points actually excluded. And the index test EXPLAINed hand-copied SQL, so a change making the real predicate non-indexable would have left it passing against its own stale duplicate; both predicates are now exported constants the test formats. One thing deliberately not claimed: a latitude-first pair whose numbers are both legal coordinates cannot be detected, and the validator no longer pretends to. It reports the range and names the convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S2nhUJ4Zdsxf7cP9DapsY3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Address search could filter by a circle and by structured fields, but not by a
shape. A territory is rarely a circle — a sales area, a delivery zone, a
canvassing route, a map viewport are all rectangles or polygons, and
approximating one with a radius drags in everything in the corners.
Both compose with the existing filters and with text search, so a query can be
"Barendt, inside this neighbourhood" rather than one or the other.
Bad input is rejected, not silently widened
bboxis longitude-first, matching GeoJSON, Leaflet, MapLibre and PostGIS, soa viewport can be passed straight through. Latitude-first is the common mistake
and it is silent — a swapped pair inside Ohio's range still parses and just
returns nothing. So the bounds are validated and a bad box is a 400 naming what
is wrong.
Same reasoning for the polygon: type, ring closure and coordinate ranges are
checked before it reaches PostGIS, because
ST_GeomFromGeoJSONraises onmalformed input and a caller's typo should not surface as a 500 that reads like
a server fault.
Index-assisted
&&against the GIST index ongeomis exact containment for point geometry —a point either is inside the rectangle or is not — so no recheck is needed.
ST_Intersectsshortlists on the same index, then tests candidates exactly.Confirmed by plan, with
enable_seqscanoff so the fixture's size cannot maskthe answer:
Bitmap Index Scanfor both.Tests
The rectangle actually restricts (101 of 600 rows) and every returned point is
inside it. A polygon excludes points its own bounding box includes — which
needed off-diagonal points seeded, since the fixture's addresses lie on a
perfect diagonal and any band around it selects the same set as its box.
Territory filters compose with text search without widening the result.
Reversed, truncated and out-of-range boxes are all rejected.
One thing the index test surfaced: the shared test fixture never built the GIST
index on
geomthat production has, so any spatial plan assertion against itwas meaningless. It builds it now.
🤖 Generated with Claude Code
https://claude.ai/code/session_01S2nhUJ4Zdsxf7cP9DapsY3