Skip to content

loop: cycle 16 — ✅ regression + acceptance PASS · 1 findings · reach 56.4% - #73

Merged
rslayer merged 2 commits into
masterfrom
loop/cycle-16
Jul 23, 2026
Merged

loop: cycle 16 — ✅ regression + acceptance PASS · 1 findings · reach 56.4%#73
rslayer merged 2 commits into
masterfrom
loop/cycle-16

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Autonomous SDLC cycle 16 against spec/spec.md.

  • Eval (regression): 198 passed / 0 failed — ✅ regression + acceptance PASS
  • Findings surviving refutation: 1 (each failing adversary test = one finding)
  • Adversary reach: 56.4% of bhulan/ has been attacked — gaps in reports/adversary/coverage-gaps.md (a targeting aid, not a KPI)

Ranked findings — you decide

Triage — cycle 16 surviving findings

poetry run pytest tests/adversary/ -q → 1 failed, 65 passed. One
distinct defect survives this cycle.

rank severity finding (test file::test) unauth? blast radius why it matters
1 high test_merge_nearby_stops_anchor_cap_radius_still_balloons.py::test_merged_stop_radius_m_balloons_far_beyond_stop_radius_m yes — /v1/insights, ordinary documented InsightsOptions fields (merge_stops_within_m, stop_radius_m), no special payload shape correctness / silently-wrong-answer An everyday "one popular spot dominates a merged group" track (ten short dwells on one side of an anchor, one on the other, each individually within stop_radius_m of the anchor) is merged into a single reported stop whose radius_m (86m) is a 71% overshoot of the configured stop_radius_m (50m) — with no error or quality.issues entry, so a consumer trusting radius_m <= stop_radius_m per ADR 0013 is silently misled.

High detail

#1 — Anchor-based merge cap bounds the wrong point, so radius_m can approach
2x stop_radius_m (high).
ADR 0013's fix to merge_nearby_stops
(bhulan/analytics/stops.py) caps merge admission by requiring each
candidate member to lie within stop_radius_m of the group's anchor (its
first member), and the ADR's stated guarantee is the stronger claim that
"every reported merged stop is a genuine cluster: radius_m <= stop_radius_m
... so a downstream 'was the vehicle parked here' consumer can trust the
location and spread" (matching spec.md AC1: "each reported centroid lies
within stop_radius_m of its own members"). Those are two different
guarantees: bounding every member's distance to the anchor only constrains
membership to a disk of radius stop_radius_m centred on the anchor — it says
nothing about where the sample_count-weighted centroid of an asymmetric
group within that disk ends up. When ten dwells sit ~49m east of the anchor
and one sits ~49m west (each individually admissible under the cap), the
weighted centroid is pulled toward the heavy east side, and the correctly
computed enclosing radius_m from that centroid to the lone west outlier
legitimately reports ~86m — approaching the disk's full diameter
(2 * stop_radius_m) rather than staying within it. This is not a contrived
tie-breaking edge case: a single popular location with occasional stops on
its far side is the ordinary shape of any frequently-visited-place track
(e.g., a loading dock approached from two directions), it is reachable
unauthenticated with a single POST /v1/insights call using only documented
options, and it reproduces exactly the "unbounded blob" symptom ADR 0013 was
written to eliminate — just capped at ~2x stop_radius_m instead of
unbounded. Ranked high rather than critical because it is a correctness
defect with a bounded (not unbounded) overshoot and no availability/crash
impact, unlike prior cycles' critical findings (e.g. cycle 12's unauthenticated
recursion-crash DoS).


Triage ranks only; nothing is auto-promoted. Pick the findings worth fixing and they become the next cycle's spec. Refuted false positives: see reports/adversary/refuted-16.md.

The build proposed the code changes; you decide. Merge to keep or close to discard. This loop never merges to master and never deploys. bhulan is a public demo — review carefully.

@rslayer

rslayer commented Jul 22, 2026

Copy link
Copy Markdown
Owner

⚠️ Do not merge as-is — the cycle-16 finding test is self-contradictory (holding for an owner decision).

test_merged_stop_radius_m_balloons_far_beyond_stop_radius_m asserts both len(stops) == 1 (all 12 dwells merge) and radius_m <= 60. These are mutually exclusive: the input spans 98 m east–west (10 dwells at +49 m, one at 0, one at −49 m), so if they all merge the weighted centroid lands ~36.8 m east and the honest radius_m is ~85.8 m — no correct code can report ≤60 m while keeping them as one stop. To get radius ≤50 the group must split (len>1), which fails the len==1 assertion.

So merging this PR adds a permanently-failing, unsatisfiable test to master → CI red forever.

The underlying finding is real though: ADR 0013 claims radius_m <= stop_radius_m, but the O(1) anchor-disk cap only bounds each member to stop_radius_m of the anchor, so an asymmetric group's weighted centroid drifts and the reported radius can approach 2 × stop_radius_m. Resolving it is an owner product decision, because every path has a real trade-off:

  1. Enforce the guarantee (split ballooning groups). Honors ADR 0013, but changes which stops merge (halves effective merge reach) and can't be done at full reach in O(1) — a naive per-admit radius recompute reintroduces the cycle-9/11 O(n²) DoS. Requires rewriting this test's assertion to the achievable invariant (per-stop radius_m <= stop_radius_m, len is whatever falls out).
  2. Accept the overshoot, correct the docs. Weaken ADR 0013 to state the true guarantee (member-to-anchor ≤ stop_radius_m; merged radius up to ~2×). No behavior change, no DoS risk — but a consumer trusting radius_m is still misled for asymmetric groups.

Recommend re-triaging this finding and picking (1) or (2); I can implement either once you decide. Left unmerged.

Resolves the cycle-16 finding via Option 2 (owner decision): ADR 0013's claim
that "every reported merged stop is a genuine cluster: radius_m <=
stop_radius_m" was FALSE. The anchor cap bounds MEMBERSHIP to a disk of radius
stop_radius_m around the anchor — diameter 2x — while radius_m is measured from
the sample_count-weighted centroid. An asymmetric group (ten dwells 49m east of
the anchor, one 49m west) centres 36.8m east and reports radius_m ~86m against
stop_radius_m=50.

Why not enforce the tighter bound: doing so at full merge reach needs a
per-admission recompute against the moving centroid — the O(n^2) blow-up ADR
0011 exists to avoid — and the only O(1) alternative (capping members to ~half
the stop radius) would fragment exactly the large-site dwells a big
stop_radius_m is for. A warehouse, DC, or port is legitimately one "place" and
callers size stop_radius_m (up to 10km) to match; splitting those to flatter a
radius bound is the worse answer.

So: report the true spread, and make the overshoot impossible to miss.
- ADR 0013: withdraw the false guarantee, state the real one (membership within
  stop_radius_m of the anchor), with the measured counter-example and rationale.
- merge_nearby_stops docstring + inline comment: same correction.
- StopOut.radius_m: schema description now says it is the true spread, not a
  bound, and that merged stops can reach ~2x stop_radius_m.
- compute_insights: emit a quality issue when a merged stop's radius exceeds
  the configured stop_radius_m — the finding's real complaint was silence.
  Only fires when merging is enabled; verified silent otherwise.

The cycle-16 test asserted len(stops)==1 AND radius_m<=60, which are mutually
exclusive (the input spans 98m, so merging it all gives ~86m; getting <=50m
requires splitting). No correct implementation satisfies both, so it now
asserts the achievable contract: bounded by the anchor-disk diameter, and
flagged whenever it exceeds stop_radius_m.

205 passed; ruff, mypy, openapi + frontend codegen all green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rslayer
rslayer merged commit 500b55f into master Jul 23, 2026
2 checks passed
@rslayer
rslayer deleted the loop/cycle-16 branch July 23, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant