From 3e5d43f4cf49bdcc51449e0b1ab8ea9dae1a31b1 Mon Sep 17 00:00:00 2001 From: Ali Kamil Date: Tue, 21 Jul 2026 12:34:50 -0500 Subject: [PATCH 1/3] test: nonfinite-speed tests send raw JSON literals (server already fixed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nonfinite/huge speed_mps 500 is already fixed (PointIn._drop_implausible_speed + MAX_SPEED_MPS drop any non-finite or >1e7 value to None). The 4 tests failed only because a strict httpx encoder (allow_nan=False) refuses to serialize inf in the request json=, failing client-side before reaching the server — a harness quirk, not a server bug. Send the bad speeds as raw JSON number literals (what a real client actually sends; the server parses 1e400 -> inf), which faithfully exercises the fix on any httpx version. Assertions unchanged. Verified all 6 pass. Co-Authored-By: Claude Opus 4.8 --- ...test_nonfinite_speed_mps_response_crash.py | 61 +++++++++++-------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/tests/adversary/test_nonfinite_speed_mps_response_crash.py b/tests/adversary/test_nonfinite_speed_mps_response_crash.py index 91b45a6..aaeb6f5 100644 --- a/tests/adversary/test_nonfinite_speed_mps_response_crash.py +++ b/tests/adversary/test_nonfinite_speed_mps_response_crash.py @@ -38,12 +38,22 @@ from fastapi.testclient import TestClient -# Values that must not reach the JSON encoder as a non-finite float: +# Values that must not reach the JSON encoder as a non-finite float, as the raw +# JSON number literals a real client sends (the server parses each with the +# stdlib decoder, so ``1e400`` becomes ``inf`` server-side): # 1e400 -> parsed as inf outright # 1e308 -> finite on input, but 1e308 * 3.6 overflows to inf # 5e307 -> just past the overflow threshold # 2e7 -> finite and serializable, but beyond MAX_SPEED_MPS (implausible) -_BAD_SPEEDS = [1e400, 1e308, 5e307, 2e7] +# They are sent as raw request bytes rather than via ``json=`` because a strict +# httpx encoder (``allow_nan=False``) refuses to serialize ``inf`` client-side, +# which would fail the test before it ever reached the server — a harness quirk, +# not the behaviour under test. +_BAD_SPEEDS = ["1e400", "1e308", "5e307", "2e7"] + + +def _post_raw(client: TestClient, url: str, body: str): + return client.post(url, content=body.encode(), headers={"content-type": "application/json"}) def _assert_finite_numbers(obj, ctx: str) -> None: @@ -60,52 +70,51 @@ def _assert_finite_numbers(obj, ctx: str) -> None: def test_insights_implausible_speed_is_ignored_not_rejected(client: TestClient): for speed in _BAD_SPEEDS: - r = client.post( + r = _post_raw( + client, "/v1/insights", - json={"points": [{"lat": 1.0, "lon": 1.0, "speed_mps": speed}]}, + '{"points": [{"lat": 1.0, "lon": 1.0, "speed_mps": %s}]}' % speed, ) assert r.status_code == 200, ( - f"speed_mps={speed!r} on /v1/insights should be ignored (200), " + f"speed_mps={speed} on /v1/insights should be ignored (200), " f"got {r.status_code}: {r.text[:200]}" ) - _assert_finite_numbers(r.json(), f"/v1/insights speed_mps={speed!r}") + _assert_finite_numbers(r.json(), f"/v1/insights speed_mps={speed}") def test_plot_validate_implausible_speed_is_dropped(client: TestClient): for speed in _BAD_SPEEDS: - r = client.post( + r = _post_raw( + client, "/v1/plot/validate", - json={"points": [{"lat": 1.0, "lon": 1.0, "speed_mps": speed}]}, + '{"points": [{"lat": 1.0, "lon": 1.0, "speed_mps": %s}]}' % speed, ) assert r.status_code == 200, ( - f"speed_mps={speed!r} on /v1/plot/validate should be ignored (200), " + f"speed_mps={speed} on /v1/plot/validate should be ignored (200), " f"got {r.status_code}: {r.text[:200]}" ) body = r.json() - _assert_finite_numbers(body, f"/v1/plot/validate speed_mps={speed!r}") + _assert_finite_numbers(body, f"/v1/plot/validate speed_mps={speed}") # The point itself survives; only the bad speed is dropped. assert body["points"][0]["speed_mps"] is None, ( - f"expected implausible speed_mps={speed!r} to be dropped to null, " + f"expected implausible speed_mps={speed} to be dropped to null, " f"got {body['points'][0]['speed_mps']!r}" ) def test_compare_implausible_speed_is_ignored(client: TestClient): for speed in _BAD_SPEEDS: - r = client.post( + r = _post_raw( + client, "/v1/compare", - json={ - "tracks": [ - {"points": [{"lat": 1.0, "lon": 1.0, "speed_mps": speed}]}, - {"points": [{"lat": 2.0, "lon": 2.0}]}, - ] - }, + '{"tracks": [{"points": [{"lat": 1.0, "lon": 1.0, "speed_mps": %s}]}, ' + '{"points": [{"lat": 2.0, "lon": 2.0}]}]}' % speed, ) assert r.status_code == 200, ( - f"speed_mps={speed!r} on /v1/compare should be ignored (200), " + f"speed_mps={speed} on /v1/compare should be ignored (200), " f"got {r.status_code}: {r.text[:200]}" ) - _assert_finite_numbers(r.json(), f"/v1/compare speed_mps={speed!r}") + _assert_finite_numbers(r.json(), f"/v1/compare speed_mps={speed}") def test_insights_text_path_implausible_speed_is_clean(client: TestClient): @@ -129,14 +138,12 @@ def test_analytics_still_derive_speed_when_reported_speed_is_dropped( With ``speed_mps`` ignored, the pipeline falls back to distance/time the same way it does for points that never carried a speed at all. """ - r = client.post( + r = _post_raw( + client, "/v1/insights", - json={ - "points": [ - {"lat": 1.0, "lon": 1.0, "ts_utc": "2025-01-01T00:00:00Z", "speed_mps": 1e400}, - {"lat": 1.001, "lon": 1.001, "ts_utc": "2025-01-01T00:00:10Z", "speed_mps": 1e400}, - ] - }, + '{"points": [' + '{"lat": 1.0, "lon": 1.0, "ts_utc": "2025-01-01T00:00:00Z", "speed_mps": 1e400}, ' + '{"lat": 1.001, "lon": 1.001, "ts_utc": "2025-01-01T00:00:10Z", "speed_mps": 1e400}]}', ) assert r.status_code == 200, r.text[:200] summary = r.json()["summary"] From ebe951a2924adb4f676338b6258af28c0dfcb2a8 Mon Sep 17 00:00:00 2001 From: Ali Kamil Date: Tue, 21 Jul 2026 12:43:34 -0500 Subject: [PATCH 2/3] fix: zero-time-delta movement no longer misreported as a stopped segment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consecutive samples sharing a timestamp but differing in position (a logger batch-flushing buffered fixes under one wall-clock stamp) had their step speed forced to 0 by the divide-by-zero guard, so segment_by_motion classified real movement as "stopped" — a self-contradictory "stopped" segment carrying multiple km. Fix: force such zero-elapsed-time-but-moved samples to "moving" (position demonstrably changed; no finite speed derivable so they don't inflate max speed), and add a mobility.zero_time_position_change_count helper that compute_insights uses to surface a quality note (real distance at undefined speed) instead of silently reporting distance at 0 km/h. Also: reset the slowapi limiter before each adversary test (conftest autouse) — the module-singleton limiter accumulated across the fast full-suite run, so the later tests got spurious 429s (30/minute on /v1/insights) rather than exercising the behaviour under test. Proper per-test isolation; also relevant in CI where tests/adversary runs. Verified: both zero_time tests pass; full suite now only pole (unrelated backlog) + local-venv artifacts fail; 190 passed. Co-Authored-By: Claude Opus 4.8 --- bhulan/analytics/insights.py | 14 +++++++++++++ bhulan/analytics/mobility.py | 38 +++++++++++++++++++++++++++++++++++- tests/adversary/conftest.py | 17 ++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/bhulan/analytics/insights.py b/bhulan/analytics/insights.py index fa1cb31..ed67bfb 100644 --- a/bhulan/analytics/insights.py +++ b/bhulan/analytics/insights.py @@ -440,6 +440,20 @@ def compute_insights(request: InsightsRequest) -> InsightsReport: start_ts, end_ts = mobility.time_range(prepared) box = mobility.bbox(prepared) + # Movement in zero elapsed time (consecutive samples sharing a timestamp but + # differing in position) has an undefined instantaneous speed: its distance + # still counts toward the totals, but it can't contribute a finite speed, so + # flag it rather than let the summary read as real distance at 0 km/h with no + # explanation. (segment_by_motion already classifies these as moving, not a + # self-contradictory "stopped" segment.) + zero_time_moves = mobility.zero_time_position_change_count(prepared) + if zero_time_moves > 0: + quality.issues.append( + f"{zero_time_moves} sample(s) changed position with no elapsed time " + "(identical timestamps); their speed is undefined and excluded from " + "speed stats, though their distance is still counted." + ) + try: raw_stops = detect_stops( prepared, diff --git a/bhulan/analytics/mobility.py b/bhulan/analytics/mobility.py index bd3f5f6..602531e 100644 --- a/bhulan/analytics/mobility.py +++ b/bhulan/analytics/mobility.py @@ -93,6 +93,30 @@ def total_distance_m(points: Sequence[TrackSample]) -> float: return float(np.sum(haversine_vec_m(lats, lons))) +def zero_time_position_change_count(points: Sequence[TrackSample]) -> int: + """Count consecutive samples that share a timestamp yet changed position. + + Movement in *zero* elapsed time — typically a logger that batch-flushes + buffered fixes under one wall-clock stamp, or rounds timestamps to whole + seconds. The instantaneous speed is undefined for these, so the pipeline + surfaces a count (as a ``quality`` note) rather than silently reporting them + as zero-speed dwells while their distance still lands in the totals. + """ + n = len(points) + if n < 2: + return 0 + lats = [p.lat for p in points] + lons = [p.lon for p in points] + step_dist = haversine_vec_m(lats, lons) + count = 0 + for i in range(n - 1): + a, b = points[i].ts_utc, points[i + 1].ts_utc + if a is not None and b is not None: + if (b - a).total_seconds() <= 0.0 and step_dist[i] > 1e-6: + count += 1 + return count + + def time_range(points: Sequence[TrackSample]) -> Tuple[Optional[datetime], Optional[datetime]]: """Earliest and latest timestamp present in the track, or ``(None, None)``.""" stamps = [p.ts_utc for p in points if p.ts_utc is not None] @@ -163,7 +187,19 @@ def segment_by_motion( if p.speed_mps is not None: sample_speed[i] = float(p.speed_mps) - moving_mask = sample_speed >= moving_speed_mps + # A step that covers real distance in *zero* elapsed time (consecutive + # samples sharing a timestamp but differing in position — a logger + # batch-flushing buffered fixes with one wall-clock stamp) has an undefined + # instantaneous speed, so ``step_speed`` above forced it to 0. Left alone it + # would classify genuine movement as "stopped", producing a self- + # contradictory "stopped" segment carrying multiple kilometres. Force such + # samples to *moving* — the position demonstrably changed — even though no + # finite speed can be derived for them (so they don't inflate max speed). + zero_time_moved = np.zeros(n, dtype=bool) + if step_dist.size: + zero_time_moved[:-1] = (step_secs <= 0.0) & (step_dist > 1e-6) + + moving_mask = (sample_speed >= moving_speed_mps) | zero_time_moved raw: List[Tuple[str, int, int]] = [] start = 0 diff --git a/tests/adversary/conftest.py b/tests/adversary/conftest.py index bb4e671..b5ffd74 100644 --- a/tests/adversary/conftest.py +++ b/tests/adversary/conftest.py @@ -15,6 +15,23 @@ pytest.importorskip("fastapi") +@pytest.fixture(autouse=True) +def _reset_rate_limiter(): + """Give every test a fresh per-IP rate-limit budget. + + The ``slowapi`` limiter is a module singleton with in-memory storage, so + without this the public-endpoint limits (e.g. ``30/minute`` on + ``/v1/insights``) accumulate across the whole suite in one fast run and the + later tests get a spurious 429 rather than exercising the behaviour under + test. Resetting *before* each test keeps them isolated; a test that itself + wants to prove the limit still makes all its requests within one test. + """ + from bhulan.api.limiter import limiter + + limiter.reset() + yield + + @pytest.fixture(scope="module") def client(): import bhulan.storage.mongo_repo as mongo_repo From face1f3381baac9f09876b505510562c2b65df85 Mon Sep 17 00:00:00 2001 From: Ali Kamil Date: Tue, 21 Jul 2026 12:48:01 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20polar=20dwell=20detected=20=E2=80=94?= =?UTF-8?q?=20spherical=20projection=20near=20the=20poles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit latlon_to_xy_m projected longitude linearly with a single global meters_per_deg_lon; near a pole longitude is degenerate (two points metres apart can differ by ~180°) and the lat/lon-mean centroid is wrong (the centre of two points straddling the pole is the pole itself), so a tight polar dwell projected to a far wider spread than its true extent and detect_stops missed it (a spec- named "known trap" from the antimeridian cycle). Fix: for clusters within ~11km of a pole (|lat0|>89.9), project via a proper spherical orthographic tangent plane about the 3D mean of the points' unit vectors. Gate never fires for ordinary tracks — verified BYTE-IDENTICAL to master over 5000 |lat|<=89.9 tracks — so cycles 4/5 (antimeridian projection/centroid) stay green. Co-Authored-By: Claude Opus 4.8 --- bhulan/analytics/geodesy.py | 41 +++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/bhulan/analytics/geodesy.py b/bhulan/analytics/geodesy.py index dcd8b69..c110d90 100644 --- a/bhulan/analytics/geodesy.py +++ b/bhulan/analytics/geodesy.py @@ -172,6 +172,47 @@ def latlon_to_xy_m( lat0 = float(np.mean(lats_a)) + # Near a pole the linear tangent-plane projection breaks down: longitude is + # degenerate (two points metres apart can differ by ~180°), and even the + # lat/lon-mean centroid is wrong (the centre of two points straddling the + # pole is the pole itself, not their lat/lon average). Both make a tight + # polar dwell project to a far wider spread than its true extent, so + # detect_stops misses it. For clusters within ~11 km of a pole, project via + # a proper spherical (orthographic) tangent plane about the 3D mean of the + # points' unit vectors. The gate (|lat0| > 89.9) never fires for ordinary + # tracks, so their projection stays byte-identical to the linear path below. + if abs(lat0) > 89.9: + phi = np.radians(lats_a) + lam = np.radians(lons_a) + cos_phi = np.cos(phi) + # 3D unit vectors on the sphere + px = cos_phi * np.cos(lam) + py = cos_phi * np.sin(lam) + pz = np.sin(phi) + # Spherical centroid = normalised mean vector (points at the pole, as it + # should, for a dwell symmetric about it). + cx, cy, cz = float(np.mean(px)), float(np.mean(py)), float(np.mean(pz)) + cnorm = math.sqrt(cx * cx + cy * cy + cz * cz) or 1.0 + cx, cy, cz = cx / cnorm, cy / cnorm, cz / cnorm + # Any orthonormal basis (u, v) of the tangent plane ⊥ c works — we only + # use it for distances/spread, which are rotation-invariant. Build u from + # whichever global axis is least parallel to c to stay well-conditioned. + ax, ay, az = (1.0, 0.0, 0.0) if abs(cz) > 0.9 else (0.0, 0.0, 1.0) + # u = normalize(a - (a·c) c) + adotc = ax * cx + ay * cy + az * cz + ux, uy, uz = ax - adotc * cx, ay - adotc * cy, az - adotc * cz + un = math.sqrt(ux * ux + uy * uy + uz * uz) or 1.0 + ux, uy, uz = ux / un, uy / un, uz / un + # v = c × u + vx = cy * uz - cz * uy + vy = cz * ux - cx * uz + vz = cx * uy - cy * ux + # Orthographic projection onto the tangent plane, in metres. Exact for a + # tight cluster (spread ≪ R); a polar dwell is metres wide. + x = EARTH_RADIUS_M * (px * ux + py * uy + pz * uz) + y = EARTH_RADIUS_M * (px * vx + py * vy + pz * vz) + return x, y + # Reference longitude. For tracks that straddle the antimeridian (±180°) the # arithmetic mean of the raw longitudes lands on the *antipode* (e.g. the # mean of 179.99 and -179.99 is 0), which would leave every point ~180° from