feat(v4): add bbox query param to /v4/places#90
Conversation
Adds an optional `bbox=min_lon,min_lat,max_lon,max_lat` parameter to `GET /v4/places`, following the GeoJSON / Leaflet convention. When present, only places whose coordinates fall inside the bbox are returned. The parameter composes cleanly with the existing `fields`, `updated_since`, `include_deleted` and `limit` filters. Invalid input (non-numeric values, out-of-range coordinates, inverted corners, wrong arity) is rejected with 400 invalid_input. Use case: mobile / low-bandwidth clients that want only the places visible in the current map viewport, instead of downloading the full worldwide dataset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds bounding-box filtering to the Places API. A new optional ChangesBbox Query Parameter Feature
Sequence DiagramsequenceDiagram
participant Client
participant PlacesHandler
participant BboxParser
participant Database
participant FilterEngine
Client->>PlacesHandler: GET /v4/places?bbox=min_lon,min_lat,max_lon,max_lat
PlacesHandler->>BboxParser: parse_bbox(bbox_string)
BboxParser->>BboxParser: validate format, ranges, corner order
BboxParser-->>PlacesHandler: BoundingBox
PlacesHandler->>Database: select_updated_since (without LIMIT)
Database-->>PlacesHandler: all places from offset
PlacesHandler->>FilterEngine: filter by bbox (lat, lon)
FilterEngine-->>PlacesHandler: places within bounds
PlacesHandler->>PlacesHandler: apply limit via truncation
PlacesHandler-->>Client: filtered and limited places
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/rest/v4/places.rs`:
- Around line 151-153: Don't truncate elements or submissions individually
(remove the truncate calls around elements and submissions) because that drops
candidates before final chronological ordering; instead, after assembling
elements and submissions (respecting include_pending), merge them into a single
collection, sort that combined collection by the shared timestamp (e.g.,
created_at) in the desired chronological order, then apply args.limit to
truncate the merged result, and finally map/split the selected items back to the
expected output shape; apply this change for the blocks that reference elements,
submissions and args.limit (including the second truncate at the later block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06f790d3-4e10-4d05-9b4c-f22e25167e46
📒 Files selected for processing (2)
docs/rest/v4/places.mdsrc/rest/v4/places.rs
CodeRabbit on PR teambtcmap#90 spotted that the per-collection truncate at lines 151-153 (elements) and 176-178 (submissions) could drop legitimate candidates from one source before they were merged with the other, returning the wrong final top-N when `include_pending=true` was combined with a bbox. Removed both per-source truncates; the existing `.take(limit)` on the merged + chronologically-sorted result is now the only place the caller's `limit` is honoured. SQL `LIMIT` is still pushed down to `select_updated_since` when no bbox is active (no in-memory filter step that could discard rows), so the dataset-wide hot path stays unchanged. All 349 tests still pass; fmt + clippy clean.
Summary
Adds an optional
bbox=min_lon,min_lat,max_lon,max_latparameter toGET /v4/places, following the GeoJSON / Leaflet convention (south-west corner first, longitude first). When present, only places whose coordinates fall inside the bbox are returned.The parameter composes cleanly with the existing
fields,updated_since,include_deletedandlimitfilters.Example
Validation
Invalid input is rejected with
400 invalid_input:min_lat > max_latormin_lon > max_lon)Tests
Five new unit tests under
rest::v4::places::testcover the happy path and each validation branch. Full suite passes locally:cargo fmt --checkandcargo clippy -- -D warningsare both clean.Why
Today
/v4/placesreturns the entire ~28k worldwide dataset (~5 MB even with slim?fields=), and the workaround for the web client at btcmap.org is to downloadcdn.static.btcmap.org/api/v4/places.jsononce and then delta-sync via?updated_since=. That works well for browsers that can keep a large localforage cache, but it isn't a great fit for mobile apps — caches get evicted aggressively and users in low-bandwidth areas pay the full cold-sync cost on every reinstall.A native viewport bbox filter is a much bigger win for those clients. Prompted by an open-source mobile-wallet maintainer (Lightning Piggy) needing viewport queries on low-bandwidth devices.
Files
src/rest/v4/places.rs— newbboxquery arg,parse_bboxhelper, in-memory filter afterselect_updated_since, five unit testsdocs/rest/v4/places.md— parameter table entry + worked exampleCo-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation