Conversation
…rossing `LineZone.trigger` confirmed a crossing whenever the oldest entry of its `minimum_crossing_threshold + 1` frame history differed from every later entry. That guard never checked that the tracker had actually been settled on the side it supposedly came from, so a single sub-threshold excursion was read as a crossing back into the side the tracker never left. With `minimum_crossing_threshold=2` the side sequence `A,A,A,B,A,A,A` left `[B,A,A]` in the history: the oldest `B` occurred once, the guard passed, and a crossing into `A` was counted. Counts drifted by one per flicker, in the wrong direction; three separated flickers gave `in_count=3` instead of `0`. A history only `threshold + 1` deep structurally cannot tell "settled on B" from "flickered to B for one frame". Track the last side each tracker was confirmed on and measure crossings against that instead of against the oldest history entry. A side is only promoted to confirmed once it has been held for the full threshold, so a flicker that reverts early never becomes the reference and produces no count. The confirmed side is dropped alongside the history on eviction so a reused tracker ID starts fresh. Sustained crossings, `minimum_crossing_threshold=1` (the default), and per-tracker isolation are unchanged. Fixes roboflow#2598
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2600 +/- ##
=======================================
Coverage 91% 91%
=======================================
Files 78 78
Lines 11455 11462 +7
=======================================
+ Hits 10461 10469 +8
+ Misses 994 993 -1 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The changelog needs its required PR link, and the threshold documentation should accurately describe valid observations rather than consecutive frames.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes LineZone false crossings caused by brief side flicker.
Changes:
- Tracks each tracker’s last confirmed side.
- Adds flicker, crossing, isolation, and eviction tests.
- Documents behavior and updates the changelog.
File summaries
| File | Description |
|---|---|
src/supervision/detection/line_zone.py |
Implements confirmed-side crossing logic. |
tests/detection/test_line_counter.py |
Adds regression and state-isolation tests. |
docs/changelog.md |
Records the bug fix. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
AGENTS.md section 6 requires each changelog entry to link the PR rather than the source issue. The entry was added before this PR had a number, so the link is appended now, in the same trailing style as the roboflow#2582 and roboflow#2588 entries.
…-flicker-spurious-crossing
[resolve No.2] Review by Copilot + foundry:doc-scribe (PR roboflow#2600): "[docs] 'Consecutive frames' docstring wording (src/supervision/detection..." Challenge: evidence=VALID suggestion=VALID resolution=as-suggested --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
[resolve No.3] Review by foundry:doc-scribe (PR roboflow#2600): "[docs] A-1: narrow an overstated guarantee. Three artifacts currently as..." Challenge: evidence=VALID suggestion=VALID resolution=as-suggested --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
…stain gate [resolve No.4] Review by foundry:qa-specialist (PR roboflow#2600): "T-1: all ten existing TestLineZoneSubThresholdFlicker parametrized cases..." Challenge: evidence=VALID suggestion=REJECT resolution=self-resolved (straddle case, not trivial reuse) --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
[resolve No.5] Review by foundry:sw-engineer (PR roboflow#2600): "[code] A-2: the gate at line_zone.py:219 (len(crossing_history) < self.c..." Challenge: evidence=VALID suggestion=VALID resolution=as-suggested --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
[resolve No.6] Review by foundry:perf-optimizer (PR roboflow#2600): "[perf] A-4/P-1: at line_zone.py:228, list(crossing_history)[-sustained_f..." Challenge: evidence=VALID suggestion=VALID resolution=as-suggested --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
[resolve No.7] Review by foundry:linting-expert (PR roboflow#2600): "[code] S-1: _confirmed_crossing_side: dict[int, bool] at line_zone.py:12..." Challenge: evidence=VALID suggestion=VALID resolution=as-suggested --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
…tion test [resolve No.8] Review by foundry:linting-expert (PR roboflow#2600): "test_flicker_does_not_leak_between_trackers zips two side-sequence strin..." Challenge: evidence=VALID suggestion=VALID resolution=as-suggested --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
… dropped-frame excursion, and concurrent flicker isolation [resolve No.9] Review by foundry:qa-specialist (PR roboflow#2600): "T-2..T-5 bundle: mirror-direction flicker, sustained crossing at thresho..." Challenge: evidence=VALID suggestion=VALID resolution=as-suggested (T-4 needed the noted test-loop extension) --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Borda
approved these changes
Sep 21, 2026
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.
Description
LineZone.triggercounted a spurious crossing in the opposite direction when a tracked object flickered to the far side of the line for fewer thanminimum_crossing_thresholdframes.A crossing was confirmed by only two conditions: the per-tracker history deque was full, and the oldest entry differed from every other entry (
crossing_history.count(oldest_state) == 1). Nothing verified that the oldest state had itself been held forminimum_crossing_thresholdframes.With
minimum_crossing_threshold=2(history length 3) and the side sequenceA,A,A,B,A,A,A, the deque at frame 6 is[B,A,A]: the oldestBoccurs exactly once, the guard passes, and a crossing intoAis counted — the side the object never left. Counts drift by one per flicker, in the wrong direction. Because the history is onlythreshold + 1deep, it structurally cannot distinguish "settled onB" from "flickered toBfor one frame".Minimal repro:
On
develop:With this PR:
Motivation and Context
A single frame of bounding-box jitter near the line silently corrupts the counts, and in the wrong direction, which is worse than missing a crossing: totals drift monotonically over a long video and
in_count/out_countstop being trustworthy. This is the exact failure modeminimum_crossing_thresholdexists to prevent.Closes #2598
Changes Made
LineZonenow tracks the last side of the line each tracker was confirmed on (_confirmed_crossing_side), seeded from the tracker's first observed side.minimum_crossing_thresholdconsecutive frames. The side is then promoted to confirmed. A sub-threshold excursion never becomes confirmed, so settling back on the original side produces no count._evict_stale_crossing_history, so a reused tracker ID starts fresh instead of inheriting a stale reference side.minimum_crossing_thresholddocstring to state that excursions shorter than the threshold count neither as a crossing nor as a return crossing.TestLineZoneSubThresholdFlickertotests/detection/test_line_counter.py(8 parametrized sequences + per-tracker isolation + eviction), extending the existing file rather than adding a new one.Deliberately out of scope:
crossing_history_length,crossing_state_historyand the tracker-absence eviction tolerance are unchanged.crossing_history_lengthdoubles as the ByteTrack-coasting absence tolerance, so deepening the history to distinguish sustained from flickering states would have changed unrelated behaviour. Comparing against a confirmed side is exact rather than heuristic and needs no extra history depth.Testing
Baseline on
developbefore the change:3905 passed, 19 skipped— the delta is exactly the 10 new tests, and no previously passing test fails.New tests failing on
developand passing here — reverting onlysrc/supervision/detection/line_zone.pywhile keeping the new tests:With the fix restored:
The 5 cases that pass either way are guards that the fix is not over-correcting: sustained crossings, a crossing followed by a genuine return, a genuine crossing occurring after an earlier flicker, and
minimum_crossing_threshold=1(the default) behaviour.Linting and types:
The
prettierhook could not run in my local environment (its npm registry needs authentication). It matches only*.yaml/*.toml, and this PR changes neither, so it is a no-op here; CI covers it.docs/changelog.mdunderUnreleasedAdditional Notes
Behaviour deliberately preserved, and covered by tests:
minimum_crossing_thresholdparametrize cases intest_line_zone_one_detection_long_horizonpass unchanged.minimum_crossing_threshold=1(the default) is unchanged.Two subtleties a reviewer may want to look at:
minimum_crossing_threshold=1— I hit exactly that while developing.crossing_history_length - 1, which equalsminimum_crossing_thresholdand is inherently clamped to at least 1, so the threshold does not need to be stored separately.len(crossing_history) < self.crossing_history_lengthguard is now strictly redundant given the seeding rule, but is kept because it makes the "a full window must exist" precondition explicit. Happy to remove it if you prefer.One correction to the issue's repro: reproducing the 3-flicker count requires the flickers to be separated by a settled run (
AAAB× 3). Three consecutive alternations (AAABABABAAA) produce only one spurious count ondevelop, because alternating states keepcount(oldest) > 1.Per AGENTS.md §6 the changelog entry should link this PR; I will add the link once this PR has a number.
AI-assistance disclosure: this change was developed with the assistance of an AI coding agent (Claude). All code, tests, and the before/after measurements reported above were reviewed and verified locally against the full test suite.