trackers-review.md

trackers Package — Python Code Review

PR-01 — Fix ByteTrack no-op when detections.confidence is None

Priority: P0
Size: small
Consensus: Opus + GPT
Files: src/trackers/core/bytetrack/tracker.py, tests under tests/core/

Current behavior

ByteTrackTracker.update() uses zeros as the default confidence:

confidences = detections.confidence if detections.confidence is not None else np.zeros(len(detections))

That routes every no-confidence detection into the low-confidence bucket. ByteTrack only spawns from unmatched high-confidence detections, so a common input such as sv.Detections(xyxy=...) silently returns only tracker_id=-1 or an empty result forever.

Why it matters

All other trackers treat missing confidence as "all detections usable" by defaulting to ones. ByteTrack is the outlier and silently fails on valid supervision.Detections input.

Suggested fix

Use all-ones confidence:

confidences = detections.confidence if detections.confidence is not None else np.ones(len(detections))

Prefer dtype compatible with existing detection arrays if needed.

Suggested tests

• Parametrize no-confidence detections across SORT, ByteTrack, OC-SORT, and BoT-SORT.
• For ByteTrack, call update() for enough frames to pass minimum_consecutive_frames; assert at least one non-negative tracker_id.
• Assert behavior matches explicit confidence=np.ones(...).

PR-02 — Unify lost-track buffer semantics and prevent zero-frame buffers

Priority: P0
Size: small/medium
Consensus: Opus + GPT
Files:

• src/trackers/core/sort/tracker.py
• src/trackers/core/bytetrack/tracker.py
• src/trackers/core/ocsort/tracker.py
• src/trackers/core/botsort/tracker.py
• src/trackers/core/sort/utils.py
• src/trackers/core/bytetrack/utils.py
• src/trackers/core/botsort/utils.py
• possibly src/trackers/core/base.py

Current behavior

All four trackers compute:

int(frame_rate / 30.0 * lost_track_buffer)

This floors to zero for low frame rates or small buffers. SORT, ByteTrack, and BoT-SORT alive helpers then use strict < maximum_frames_without_update, while OC-SORT uses <=.

Example: frame_rate=10, lost_track_buffer=1 gives maximum_frames_without_update == 0, so a track can die immediately.

Why it matters

The parameter name implies "number of missed frames to keep a lost track alive." Current behavior is surprising, inconsistent across trackers, and especially bad for mobile / low-FPS streams.

Suggested fix

• Validate frame_rate > 0.
• Validate lost_track_buffer >= 1.
• Compute once with:

maximum_frames_without_update = max(1, math.ceil(frame_rate / 30.0 * lost_track_buffer))

• Prefer moving this shared calculation into BaseTracker or a small shared helper to prevent future drift.
• Use <= maximum_frames_without_update consistently in alive/prune helpers.
• Update docstrings to say whether the value is a 30-FPS-normalized buffer or an actual missed-frame count after scaling.

Suggested tests

• Parametrize tracker class x lost_track_buffer={1, 2, 5} x frame_rate={10, 15, 30, 60}.
• Create a track, miss exactly maximum_frames_without_update frames, and assert it is still alive.
• Miss one more frame and assert it is pruned.
• Add invalid-argument tests for zero/negative frame rate and buffer.

PR-03 — Replace global class-variable track ID counters with per-instance allocators

Priority: P0
Size: medium
Consensus: Opus + GPT
Files:

• src/trackers/utils/base_tracklet.py
• src/trackers/core/*/tracklet.py
• src/trackers/core/*/tracker.py
• tests/core/test_tracklets.py
• tests/core/test_trackers.py

Current behavior

BaseTracklet.count_id is a class variable. get_next_tracker_id() mutates the class-level counter, and tracker reset() methods reset that global state. Two tracker instances of the same class in one process share and can reset each other's ID allocation.

GPT's repro produced two live SORT tracks with ID 0 after resetting a second tracker instance.

Why it matters

Concurrent trackers are common: multi-camera apps, class-specific trackers, services handling multiple requests, notebooks, and A/B comparisons. Global ID allocation can create collisions and non-local side effects.

Suggested fix

• Move _next_track_id to each tracker instance.
• Add a small instance method such as _allocate_tracker_id().
• Assign IDs in the tracker rather than inside tracklet class methods, or pass an allocated ID into tracklets when they become confirmed.
• Remove Tracklet.count_id = 0 from reset() methods.
• Update tests that currently pin global counter behavior.

Suggested tests

• Instantiate two trackers of the same class.
• Mature a track in tracker A.
• Reset tracker B.
• Mature another track in tracker A.
• Assert tracker A does not reuse an existing live ID.
• Repeat at least for SORT and ByteTrack; ideally parametrize all trackers.

PR-04 — Restore the output-row contract for ByteTrack, BoT-SORT, and OC-SORT

Priority: P1
Size: medium
Consensus: Opus + GPT, with GPT specifically reproing ByteTrack/BoT-SORT mid-confidence drops
Files:

• src/trackers/core/bytetrack/tracker.py
• src/trackers/core/botsort/tracker.py
• src/trackers/core/ocsort/tracker.py
• tests under tests/core/

Current behavior

The advertised API is effectively: return the same detections with tracker_id populated. Current behavior diverges:

• ByteTrack and BoT-SORT drop unmatched detections whose confidence is between high_conf_det_threshold and track_activation_threshold.
• ByteTrack's docstring says detection order may differ from input.
• OC-SORT filters low-confidence detections before association and emits rows from the filtered detections, not the original batch.

GPT repro: with high_conf_det_threshold=0.6, track_activation_threshold=0.7, a detection at confidence 0.65 returned length 0 for both ByteTrack and BoT-SORT.

Why it matters

Callers often expect output arrays to align with input metadata: class_id, masks, custom data, and original detection indices. Dropping or reordering rows makes downstream code silently wrong.

Suggested fix

• Build tracker_ids = np.full(len(input_detections), -1, dtype=int) in original input order.
• Use filtered/high/low subsets only for association and spawn decisions.
• For every matched detection, write its ID into tracker_ids[original_idx].
• Return an indexed/copy result with all original rows and tracker_id populated, unless the library intentionally chooses a breaking API change.
• Gate track spawning separately from output emission: detections below activation threshold should still be returned with tracker_id=-1.

Suggested tests

• For each tracker, assert len(tracker.update(detections)) == len(detections) for empty, low-confidence, mid-confidence, high-confidence, and mixed inputs.
• Assert returned xyxy, confidence, class_id, and custom data preserve original row order.
• Assert unmatched rows receive tracker_id=-1.

PR-05 — Fix OC-SORT ORU double-application of the final observation

Priority: P1
Size: small
Consensus: Opus + GPT
Files: src/trackers/core/ocsort/tracklet.py, tests under tests/core/

Current behavior

OCSORTTracklet._unfreeze_xcycsr() and _unfreeze_xyxy() loop over range(time_gap), so the final virtual observation is exactly the real new observation. update() then calls self.state_estimator.update(bbox) again.

The docstring says interpolation factors should go from 0 to (time_gap - 1) / time_gap, with the caller responsible for the final real update. The code currently reaches factor 1.0.

Why it matters

This biases OC-SORT re-association after missed frames and diverges from the intended Observation-Centric Recovery algorithm.

Suggested fix

Change both interpolation loops to avoid the final real observation, e.g. for i in range(time_gap - 1), and ensure time_gap <= 1 is handled without extra virtual updates.

Suggested tests

• Add a spy/fake Kalman filter or monkeypatch kf.update.
• Create a miss/reacquire path with time_gap=3.
• Assert virtual updates are applied only for intermediate observations and the final real bbox is applied exactly once by update().
• Assert final state matches a hand-computed sequence for a simple box.

PR-06 — Validate eval metric sequence lengths and similarity shapes

Priority: P1
Size: small
Consensus: Opus + GPT
Files:

• src/trackers/eval/clear.py
• src/trackers/eval/hota.py
• src/trackers/eval/identity.py
• tests under tests/eval/

Current behavior

The low-level metric functions iterate with zip(gt_ids, tracker_ids). Mismatched sequence lengths silently truncate to the shorter sequence.

GPT repro: CLEAR reported no false positives for one GT frame and two tracker frames because the trailing tracker frame was ignored.

Why it matters

The higher-level MOT preparation path may align normal MOT inputs, but these metric functions are public enough to be called directly. Silent truncation can inflate published results.

Suggested fix

• Add a shared validation helper for:
  • len(gt_ids) == len(tracker_ids) == len(similarity_scores)
  • for each frame, similarity_scores[t].shape == (len(gt_ids[t]), len(tracker_ids[t]))
• Raise ValueError with actionable messages.

Suggested tests

• Mismatched frame counts raise.
• Mismatched per-frame similarity shape raises.
• Valid empty-frame combinations still pass.

PR-07 — Fix OC-SORT observation history hygiene

Priority: P2
Size: small
Consensus: Opus + GPT
Files: src/trackers/core/ocsort/tracklet.py, tests under tests/core/

Current behavior

• The spawn observation is not recorded in self.observations.
• last_observation and history entries store caller-owned arrays by reference.
• previous_to_last_observation is written but not read.

Why it matters

History aliasing lets external mutation corrupt tracker internals. Missing the initial observation weakens early velocity/history behavior and complicates ORU reasoning.

Suggested fix

• Store defensive copies: self.last_observation = np.asarray(initial_bbox).copy().
• Seed self.observations = {0: self.last_observation.copy()}.
• Copy every updated bbox before saving it.
• Remove previous_to_last_observation unless a real consumer is added.

Suggested tests

• Mutate the source bbox after tracklet construction; internals remain stable.
• Mutate the source bbox after update; history remains stable.
• Assert observation key 0 exists immediately after construction.

PR-08 — Clamp degenerate XCYCSR states before converting to XYXY

Priority: P2
Size: small
Consensus: Opus + GPT
Files:

• src/trackers/utils/converters.py
• src/trackers/utils/state_representations.py
• tests under tests/utils/

Current behavior

xcycsr_to_xyxy() computes:

w = np.sqrt(scale * aspect_ratio)
h = scale / w

Zero or negative scale/aspect can produce NaN/Inf. The current XCYCSRStateEstimator.clamp_velocity() only zeros scale velocity when the next scale would be non-positive; it does not clamp the current state.

Why it matters

The IoU layer now rejects non-finite boxes, which is good, but the tracker can still generate them internally from degenerate Kalman states.

Suggested fix

• Clamp scale and aspect ratio to a small positive epsilon in the converter or before calling it.
• Clamp the actual XCYCSR state in XCYCSRStateEstimator, not only velocity.
• Keep behavior deterministic and documented.

Suggested tests

• xcycsr_to_xyxy() with zero scale, negative scale, zero aspect, and negative aspect returns finite boxes or raises a deliberate ValueError.
• If choosing clamping, assert output width/height are positive finite values.
• Add a state-estimator test proving a degenerate predicted state is repaired before conversion.

PR-09 — Reject duplicate tracker registry IDs

Priority: P2
Size: small
Consensus: Opus; consistent with current extensibility goals
Files: src/trackers/core/base.py, tests/core/test_registration.py

Current behavior

BaseTracker.__init_subclass__() silently overwrites BaseTracker._registry[tracker_id] when a second subclass uses the same tracker_id.

Why it matters

The package advertises tracker auto-registration and extensibility. Silent overwrite makes import order observable and can break CLI discovery.

Suggested fix

Raise ValueError when registering a duplicate tracker_id, unless the exact same class is being registered idempotently.

Suggested tests

• Define two temporary subclasses with the same tracker_id.
• Assert the second registration raises and the original registry entry remains unchanged.

PR-10 — Clean stale package/test directories and fix tox

Priority: P2
Size: small
Consensus: Opus
Files/directories:

• tox.ini
• root-level trackers/
• root-level test/

Current behavior

The repository contains stale root-level trackers/ and test/ directories with only __pycache__ artifacts from the old layout. tox.ini has changedir = test, so tox runs from the stale directory rather than the real tests/ directory. It also installs only pytest, not the project.

Why it matters

This confuses contributors and can make tox validate little or nothing.

Suggested fix

• Delete stale trackers/ and test/ cache-only directories.
• Update tox.ini to run from the repo root.
• Install the package in editable mode or install the needed extras/dependency groups.
• Keep tox aligned with pyproject.toml pytest config.

Suggested tests

• tox -e py310 or one available local tox env discovers and runs the real suite.

PR-11 — Add deterministic assignment tie-breaking

Priority: P2
Size: small/medium
Consensus: Opus
Files: association helpers in src/trackers/core/*/

Current behavior

Unmatched output order is sorted, but equal-cost assignment ties are still left to scipy.optimize.linear_sum_assignment.

Why it matters

Golden tests and cross-language rewrites need deterministic ID sequences when cost matrices contain ties or symmetric boxes.

Suggested fix

Add a tiny deterministic perturbation to cost/similarity matrices before assignment, small enough not to change non-tie decisions. Centralize the helper so all trackers use the same tie-break policy.

Suggested tests

• Construct a symmetric/tied association matrix.
• Assert the selected pairs and resulting tracker IDs are stable.
• Use the same fixture later as a Dart/Rust conformance golden.

PR-12 — Unify minimum_consecutive_frames semantics

Priority: P2
Size: small/medium
Consensus: Opus
Files: src/trackers/core/ocsort/tracklet.py, possibly BoT-SORT and shared tests

Current behavior

OC-SORT has a warmup branch that can grant real IDs before the configured minimum_consecutive_frames threshold, unlike SORT, ByteTrack, and BoT-SORT.

Why it matters

The same parameter name should not have tracker-specific semantics unless the difference is explicit and tested.

Suggested fix

• Decide one package-wide meaning:
  • strict: ID only after the threshold is met, or
  • explicit instant_first_frame_activation style parameter.
• Prefer strict consistency for SORT, ByteTrack, and OC-SORT; BoT-SORT already exposes an explicit first-frame activation option.

Suggested tests

• Parametrize all trackers with minimum_consecutive_frames=3.
• Feed a single stable detection over frames.
• Assert IDs remain -1 until the same frame count for every tracker, except any tracker with an explicitly enabled instant-activation parameter.

PR-13 — Harden OC-SORT empty-array shapes and freeze timing

Priority: P3
Size: small/investigation
Consensus: Opus
Files: src/trackers/core/ocsort/tracker.py, src/trackers/core/ocsort/tracklet.py, tests under tests/core/

Current behavior

• np.array([t.get_state_bbox() for t in self.tracks]) produces shape (0,) when there are no tracks. Current downstream guards happen to avoid failure, but the shape is wrong for a (0, 4) box matrix.
• OC-SORT freeze timing may capture state after one drift step rather than at the moment of the first miss. This needs a test-first investigation.

Why it matters

Shape drift is a common source of future vectorization bugs. Freeze timing affects OC-SORT recovery quality and should be explicit.

Suggested fix

• Use np.empty((0, 4)) and np.empty((0, 2)) in zero-track paths.
• Add a focused freeze-timing test before changing behavior.

Suggested tests

• Empty-tracks/non-empty-detections update path returns a valid result with no accidental (0,) arrays reaching IoU or direction-consistency code.
• Miss/reacquire fixture documents exactly when _frozen_state is captured.

PR-14 — Numerical hardening follow-up

Priority: P3
Size: medium/design
Consensus: Opus
Files:

• src/trackers/utils/kalman_filter.py
• src/trackers/utils/state_representations.py
• tracker defaults/tests

Current behavior / open questions

• Kalman gain uses np.linalg.inv(S); np.linalg.solve is usually preferred.
• Q-block scaling for the XCYCSR model may double-damp one term; verify against the SORT/OC-SORT reference before changing.
• SORT and ByteTrack now default to XYXYStateEstimator, which allows independent corner velocities and can shear boxes through occlusions.
• OC-SORT direction-normalization epsilon is very small relative to float32 noise.

Why it matters

These are not urgent correctness repros, but they affect long-run numerical stability and the rewrite spec.

Suggested fix

Make this a test-first numerical audit. Do not change defaults without a benchmark or golden-test reason.

Suggested tests

• Kalman predict/update golden cases.
• Degenerate covariance / near-singular innovation case.
• Occlusion fixture comparing XYXY vs XCYCSR/XCYCWH drift.

PR-15 — Consolidate association and IoU helper duplication

Priority: P3
Size: medium/design
Consensus: Opus
Files: multiple under src/trackers/core/, src/trackers/eval/, src/trackers/utils/iou.py

Current behavior

Association helpers have diverging signatures across trackers. The package also has multiple IoU surfaces: pluggable tracker IoU, eval box IoU, and older helper semantics.

Why it matters

Divergence makes the code harder to port and easier to regress. A rewrite should follow one association contract.

Suggested fix

• Add a shared association helper that accepts a similarity matrix, a threshold, and tie-break policy.
• Route tracker helpers through it.
• Consider making eval IoU call into the same validated box math where appropriate, while preserving metric-specific behavior.

Suggested tests

Existing association and IoU tests should continue to pass; add only tests for new shared edge cases.

PR-16 — Dedicated BoT-SORT deep review

Priority: P3
Size: review-only first
Consensus: Opus noted limited coverage in this v2 pass
Files: src/trackers/core/botsort/*, tests/core/test_botsort_*

Current status

BoT-SORT was added after the original review and was only spot-checked here. The top shared issues already include BoT-SORT where verified: lost-buffer semantics, global ID counters, and mid-confidence output drops.

Suggested review scope

• CMC numerical stability and failure modes.
• Empty-array shape guards.
• Output-row contract after PR-04.
• Signed-IoU normalization across all association stages.
• Frame requirement/error behavior when CMC is enabled.
• Mobile/rewrite implications of OpenCV-dependent CMC.

Coding-agent handoff format

For each PR, give a coding agent:

1. The PR section from this document.
2. The relevant source paths.
3. The test requirements.
4. The instruction to keep the PR surgical and avoid combining unrelated PRs.

Recommended agent prompt skeleton:

Implement PR-XX from docs/trackers_python_review_v2.md.
Keep the change limited to the files named in that section unless tests show a
tightly coupled fix is required. Add the suggested regression tests. Run the
relevant test subset and report any unrelated baseline failures separately.