Cap merge_nearby_stops by stop_radius_m — a merged stop is one place - #62
Merged
Conversation
A stop is one place: points clustered within stop_radius_m of a centre. Cycles 7-9 made the merge transitive but UNBOUNDED, so a chain of pairwise-close stops could collapse into one implausibly wide "stop" (a slow walk/drift). Cap it: a stop joins the current group only while within stop_radius_m of the group's fixed anchor (first member); else it starts a new group. Tied to the caller's stop_radius_m (the stop radius WINS over a looser merge_stops_within_m), O(1) per stop (no cycle-9 O(n^2) reintroduction), threaded from InsightsOptions. Verified (local A/B): all merge tests green; ZERO regressions (full suite); cycles 1-10 green; O(n) (2x/doubling to 8000 stops); BYTE-IDENTICAL when the cap never trips (0-diff over 2000 within-radius merges). Test changes (semantic — please review): the 4 pure-geometry merge tests set stop_radius_m so their chain legitimately fits one stop (assertions unchanged, still verify single-linkage/centroid/radius); the perf test now asserts the drift chain SPLITS under the cap (still O(n)); heavy_dwell is reframed from "asserts the buggy wide merge" to "asserts every stop stays bounded"; and a new test_merge_nearby_stops_span_cap_splits_walk.py asserts a progressive walk splits into bounded stops. ADR 0013. NEXT: progressive-movement rejection in detect_stops (a walk currently reports phantom stops even without merging). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rslayer
added a commit
that referenced
this pull request
Jul 21, 2026
A stop is one place: points clustered around a common centre. detect_stops only checked spread ≤ radius, so a slow walk/crawl lasting longer than min_duration was chopped into radius-sized chunks and each reported as a phantom stop (a ~190m walk -> 2 stops). Reject a cluster that progressively translates: measure the drift between its first- and second-half centroids; a dwell's halves coincide (jitter averages out), a walk's drift apart. Reject when drift > 0.5 x stop_radius_m (tunable). Clusters < 4 samples are treated as dwells so brief legit stops are never dropped. Verified: slow walk -> 0 stops, jittery dwell -> 1 stop; NO new failures on master (still the 20 known-backlog baseline); O(n) on a 16k-sample walk. ADR 0014. Independent of the merge cap (PR #62): this is the PRIMARY fix (walks never reach the merge). Note for compose: #62's cap-geometry tests that route a walk through /v1/insights should move to unit-level merge_nearby_stops calls so this filter doesn't pre-empt them. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…-> unit level With detect_stops rejecting progressive movement (ADR 0014), a co-linear chain routed through /v1/insights is rejected before the merge, so merge geometry can only be exercised by calling merge_nearby_stops directly. Converted the three overlapping cap tests (chain_centroid_radius, radius_m, span_cap_splits) from endpoint tests to unit-level calls against merge_nearby_stops with constructed Stop objects. Assertions preserved/strengthened; API reachability of the merge is covered elsewhere. Full suite: 19 fails = known backlog + venv only (one fewer than baseline — heavy_dwell now passes with the cap); O(n) preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Cap the transitive merge by
stop_radius_m— a merged stop is one placeImplements the confirmed decision (stop radius wins): a merged "stop" can never be wider than one stop's radius, so a slow GPS walk/drift no longer masquerades as a single wide stop.
Product change (
stops.py+insights.py):merge_nearby_stopsgainsstop_radius_m(threaded fromoptions.stop_radius_m). A stop joins the current merged group only while withinstop_radius_mof the group's fixed anchor (first member) — O(1) per stop, no reintroduction of the cycle-9 O(n²). Beyond that, it starts a new group.merge_stops_within_mstill gates whether neighbours are close enough to consider, butstop_radius_mbounds the result. ADR 0013.Verified by local A/B (not implemented via the loop — see note)
Six existing merge tests set
stop_radius_m = 5withmerge = 45and asserted an 80–175 m chain fully merges — the exact behaviour the cap changes. I updated them conservatively:stop_radius_mconfig was raised so the chain legitimately fits one stop — assertions unchanged, so they still verify what they did before.radius_m ≤ stop_radius_m)" — a stronger assertion.test_merge_nearby_stops_span_cap_splits_walk.py: a progressive walk must split into bounded stops.Why this came as a direct PR, not a loop cycle
The loop's cycle-11 run was degraded (build work reached no branch; adversary/robustness hit max-turns), and the cap requires rewriting contradictory tests — which the loop is forbidden from doing (it may not change test assertions). So I implemented it directly and verified hard. You can still run the loop's adversary against this branch.
Next: progressive-movement rejection in
detect_stops— a continuous walk currently reports phantom stops even without merging (the deeper half of your "a walk isn't a stop" point).