detect_stops: reject progressive movement — a walk is not a stop - #63
Merged
rslayer merged 1 commit intoJul 21, 2026
Merged
Conversation
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>
rslayer
added a commit
that referenced
this pull request
Jul 21, 2026
rslayer
added a commit
that referenced
this pull request
Jul 21, 2026
…-> 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>
rslayer
added a commit
that referenced
this pull request
Jul 21, 2026
…rejection (#62) * cap merge_nearby_stops by stop_radius_m — a merged stop is one place 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> * reconcile cap with progressive-rejection (#63): merge-geometry tests -> 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> --------- 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.
detect_stops: reject progressive movement — a walk is not a stopImplements the deeper half of "a walk isn't a stop." A stop is one place — points clustered around a common centre.
detect_stopsonly checked spread ≤ radius, so a slow walk/crawl lasting longer thanmin_stop_minuteswas chopped into radius-sized chunks, each reported as a phantom stop (a steady ~190 m walk → 2 "stops"). (Fast movement was already filtered by duration.)Fix: a cluster is a stop only if it isn't progressively translating — measured by the drift between its first- and second-half centroids. A dwell's halves coincide (random jitter averages out); a walk's drift apart by ~the distance travelled. Reject when drift >
0.5 × stop_radius_m(tunable constant). Clusters < 4 samples are treated as dwells so brief legitimate stops are never dropped. ADR 0014.Verified (local A/B)
Relationship to the cap (#62) — please read
This is the primary fix; the merge cap (#62) is defence-in-depth. Once walks are rejected at detection, they never reach the merge, so the cap rarely binds. They compose, with one caveat: #62 raised
stop_radius_min a few cap-geometry tests to force a chain to merge — but a raised radius makes that chain read as one progressive cluster, which this filter then rejects. So if you take both, those cap-geometry tests should be converted to callmerge_nearby_stopsdirectly (unit level) rather than routing a walk through/v1/insights. I kept this PR independent of #62 (branched from master) so you can review/merge them in either order; I'll reconcile the overlap once you've decided on #62.Tunable to note
_PROGRESSIVE_DRIFT_FRACTION = 0.5is a threshold worth your eye — it's the dwell-vs-walk sensitivity. 0.5 cleanly separates a directional radius-fill from jitter; lower rejects more aggressively. Could become anInsightsOptionsknob if you want per-request control.