feat: implement linear frame interpolation logic and unit tests#82
Merged
Devnil434 merged 7 commits intoMay 20, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements trajectory interpolation for missed detections: a new sequenceDiagram
participant Detector
participant Tracker.update
participant _interpolate_trajectory
participant TrackStore
Detector->>Tracker.update: detection (frame_id, bbox/center)
Tracker.update->>TrackStore: fetch previous track (last_seen_frame, last_pos)
alt gap_frames > 0 and <= max_interpolation_gap
Tracker.update->>_interpolate_trajectory: last_pos, new_pos, gap_frames, start_frame_id
_interpolate_trajectory->>Tracker.update: interpolated points (frame_id,x,y[,w,h],interpolated=True)
Tracker.update->>TrackStore: merge interpolated points + current point, truncate to MAX_TRAJECTORY_LEN
else
Tracker.update->>TrackStore: append current point only
end
ChangesTrajectory interpolation for missed detections
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_tracker.py (1)
491-515: ⚡ Quick winStrengthen interpolation test assertions for
w/hand full step sequence.
test_interpolate_trajectory_successcurrently checks only the first interpolated(x, y)point. Add assertions forw,h, and at least the last interpolated point to lock down the full linear progression contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tracker.py` around lines 491 - 515, Update test_interpolate_trajectory_success to assert the full linear progression: verify that each interpolated dict in the returned list has interpolated==True and correct sequential frame_id starting at start_frame, assert both "w" and "h" values for the first and last interpolated entries (matching expected linear interpolation between last_pos and new_pos), and add an assertion for the last interpolated point's "x" and "y" to confirm the final step (e.g., result[-1]["x"], result[-1]["y"], result[-1]["w"], result[-1]["h"] match the expected values). Use the existing _interpolate_trajectory inputs and compute expected per-step deltas to validate the entire sequence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/tracking/tracker.py`:
- Around line 366-396: Tracker.update is never calling the new helper
_interpolate_trajectory so missed-frame gaps are not filled; modify
Tracker.update to detect a gap between the last stored point for a track and the
incoming new_pos (compute gap_frames as new_pos['frame_id'] -
last_point['frame_id'] - 1), call _interpolate_trajectory(last_point, new_pos,
gap_frames, last_point['frame_id'] + 1) when gap_frames > 0, extend the track's
history with the returned interpolated points before appending the current
new_pos (preserving w/h if present), and ensure the same interpolated
flag/rounding behavior is used to keep consistency with _interpolate_trajectory.
---
Nitpick comments:
In `@tests/test_tracker.py`:
- Around line 491-515: Update test_interpolate_trajectory_success to assert the
full linear progression: verify that each interpolated dict in the returned list
has interpolated==True and correct sequential frame_id starting at start_frame,
assert both "w" and "h" values for the first and last interpolated entries
(matching expected linear interpolation between last_pos and new_pos), and add
an assertion for the last interpolated point's "x" and "y" to confirm the final
step (e.g., result[-1]["x"], result[-1]["y"], result[-1]["w"], result[-1]["h"]
match the expected values). Use the existing _interpolate_trajectory inputs and
compute expected per-step deltas to validate the entire sequence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 204a55a7-3af6-4e8b-8ac2-bc48b68b2d3a
📒 Files selected for processing (2)
services/tracking/tracker.pytests/test_tracker.py
…engthen test assertions
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/tracking/tracker.py`:
- Around line 194-195: Protect the access to prev.trajectory[-1] to avoid
IndexError by checking if prev.trajectory is truthy/non-empty before indexing;
if empty, set last_pos to the current center (use {"x": cx, "y": cy}) or skip
interpolation logic that uses last_pos. Update the block where last_pos and
new_pos are created (references: prev, prev.trajectory, last_pos, new_pos, cx,
cy) to perform this guard and ensure downstream interpolation code can handle
the fallback path.
- Line 190: The interpolation gap limit should be an explicit Tracker attribute
rather than reading from an uninitialized self.config; add a parameter/attribute
named max_interpolation_gap (default 10) to Tracker's initializer (e.g.,
__init__) and replace uses of self.config.get("max_interpolation_gap", 10) with
self.max_interpolation_gap (or self._max_interpolation_gap if you prefer private
naming) so interpolation logic in methods like the one setting max_gap refers
directly to the Tracker attribute and is discoverable/configurable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 804a9e1a-2129-4d63-93cc-a8e8c5e6e422
📒 Files selected for processing (2)
services/tracking/tracker.pytests/test_tracker.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_tracker.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
services/tracking/tracker.py (1)
64-93:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
max_interpolation_gapattribute is missing from__init__.Line 188 references
self.max_interpolation_gap, but this attribute is never initialized in__init__. This will raiseAttributeErrorat runtime when the interpolation logic executes.🐛 Proposed fix
def __init__( self, fps: float = FPS_DEFAULT, max_age: int = 30, # frames before a lost track is marked DEAD n_init: int = 3, # frames before a track is CONFIRMED max_cosine_distance: float = 0.4, camera_id: str = "cam_01", event_logger: TrackEventLogger | None = None, reid_similarity_threshold: float = 0.85, + max_interpolation_gap: int = 10, ) -> None: self.fps = fps self.camera_id = camera_id self.max_age = max_age # NEW self.REID_SIMILARITY_THRESHOLD = reid_similarity_threshold + self.max_interpolation_gap = max_interpolation_gap🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/tracking/tracker.py` around lines 64 - 93, The constructor __init__ is missing initialization for self.max_interpolation_gap which is referenced later (causing AttributeError); add a new parameter (e.g., max_interpolation_gap: int = <sensible default>) to __init__'s signature and assign it (self.max_interpolation_gap = max_interpolation_gap) alongside other instance attributes so the interpolation logic can safely read it; update any callers if needed to pass the value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/tracking/tracker.py`:
- Around line 209-218: The instantiation of TrajectoryPoint in tracker.py passes
interpolated and optional w/h which are not defined on the TrajectoryPoint
pydantic model, causing TypeError; either update the TrajectoryPoint schema in
libs/schemas/tracking.py to include interpolated: bool (default False) and
optional w: Optional[float] and h: Optional[float] (or appropriate numeric
types) so the constructor accepts these keys, or remove those extra kwargs from
the list comprehension in tracker.py (the TrajectoryPoint(...) call) so only x,
y, and frame_id are passed; pick the approach consistent with downstream code
(if interpolation metadata is needed, add the fields to TrajectoryPoint and make
them optional with sensible defaults).
---
Duplicate comments:
In `@services/tracking/tracker.py`:
- Around line 64-93: The constructor __init__ is missing initialization for
self.max_interpolation_gap which is referenced later (causing AttributeError);
add a new parameter (e.g., max_interpolation_gap: int = <sensible default>) to
__init__'s signature and assign it (self.max_interpolation_gap =
max_interpolation_gap) alongside other instance attributes so the interpolation
logic can safely read it; update any callers if needed to pass the value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e08814c2-c0a8-441c-8384-2a7e209aed30
📒 Files selected for processing (3)
services/memory/memory.pyservices/tracking/tracker.pytests/test_tracker.py
✅ Files skipped from review due to trivial changes (1)
- services/memory/memory.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_tracker.py
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/tracking/tracker.py (1)
190-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude occluded frames in dwell-time accumulation.
dwell_framesonly increments by1, so a track that disappears for 3 frames and then reappears still advances dwell by a single frame. That under-reports occupancy instead of preserving continuity across occlusions.Suggested fix
prev = self._active_tracks.get(tid) - dwell_frames = (prev.dwell_time_frames + 1) if prev else 1 + gap_frames = max(0, self._frame_id - prev.last_seen_frame - 1) if prev else 0 + dwell_frames = (prev.dwell_time_frames + gap_frames + 1) if prev else 1 dwell_secs = dwell_frames / self.fps # ── Trajectory ──────────────────────────────────────────────── prev_traj = prev.trajectory if prev else [] - gap_frames = 0 - - if prev is not None: - gap_frames = max(0, self._frame_id - prev.last_seen_frame - 1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/tracking/tracker.py` around lines 190 - 199, The dwell-time calc currently only adds 1 on reappearance and ignores occluded frames; update the logic so that when prev exists you compute gap_frames = max(0, self._frame_id - prev.last_seen_frame - 1) and add that gap to dwell_frames (i.e., dwell_frames = prev.dwell_time_frames + 1 + gap_frames) so occluded frames are counted toward occupancy; adjust any subsequent uses that assume dwell_frames to use this new value (references: variables prev, dwell_frames, gap_frames, prev.last_seen_frame, prev.dwell_time_frames, self._frame_id, self.fps).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/tracking/tracker.py`:
- Around line 417-425: The _interpolate_trajectory function is defined after the
module-level entry point so main() can call Tracker.update which references it,
causing a NameError; move the entire _interpolate_trajectory function definition
so it appears before the if __name__ == "__main__": guard (i.e., above the
main() call) so that _interpolate_trajectory is defined when Tracker.update
runs, or alternatively hoist its definition near other helper functions used by
Tracker.update.
---
Outside diff comments:
In `@services/tracking/tracker.py`:
- Around line 190-199: The dwell-time calc currently only adds 1 on reappearance
and ignores occluded frames; update the logic so that when prev exists you
compute gap_frames = max(0, self._frame_id - prev.last_seen_frame - 1) and add
that gap to dwell_frames (i.e., dwell_frames = prev.dwell_time_frames + 1 +
gap_frames) so occluded frames are counted toward occupancy; adjust any
subsequent uses that assume dwell_frames to use this new value (references:
variables prev, dwell_frames, gap_frames, prev.last_seen_frame,
prev.dwell_time_frames, self._frame_id, self.fps).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c753bcf9-215b-4454-bc7e-e1f9d6c688c7
📒 Files selected for processing (2)
libs/schemas/tracking.pyservices/tracking/tracker.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
services/tracking/tracker.py (2)
424-454: 💤 Low valueInterpolation logic is correct; consider moving function definition earlier in the file.
The linear interpolation math is sound and correctly generates
gap_framesintermediate points with proper frame_id sequencing. However, defining_interpolate_trajectoryaftermain()(line 369) doesn't follow typical Python conventions. Consider moving this helper function to appear before theTrackerclass or immediately after it, where helper functions are more discoverable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/tracking/tracker.py` around lines 424 - 454, The helper function _interpolate_trajectory is defined after main() which breaks common readability conventions; move its definition to appear before the Tracker class or immediately after the Tracker class (but before main()) so helper functions are discoverable; ensure any references to _interpolate_trajectory (e.g., from Tracker methods) still resolve and update any nearby docstrings/comments if needed, and run tests to confirm no import/forward-reference issues arise.
211-214: 💤 Low valueConsider logging a warning if
prev.trajectoryis unexpectedly empty.The fallback to current center coordinates will interpolate from the current position to itself over the gap, which produces unintuitive results. Since this edge case shouldn't occur in normal operation (a track with
gap_frames > 0should always have a non-empty trajectory), consider logging a warning or skipping interpolation entirely when this condition is detected.Alternative approach
if prev is not None and 0 < gap_frames <= max_gap: - if prev.trajectory: + if prev.trajectory: last_pos = {"x": prev.trajectory[-1].x, "y": prev.trajectory[-1].y} else: - last_pos = {"x": cx, "y": cy} + logger.warning(f"Track #{tid} has gap but empty trajectory; skipping interpolation") + # Skip interpolation by not setting last_pos + prev.trajectory = [] # Ensure we skip the interpolation block🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/tracking/tracker.py` around lines 211 - 214, Detect when prev.trajectory is empty before falling back to last_pos and either log a warning or skip interpolation: in the block that currently sets last_pos from prev.trajectory or falls back to {"x": cx, "y": cy}, add a conditional that if prev.trajectory is empty (len(prev.trajectory) == 0) emits a warning via the module/logger used in tracker.py including identifying context (e.g., track id or frame index and gap_frames) and then either set a flag to skip interpolation for this gap or continue without interpolating, rather than silently using the current center; keep references to prev.trajectory, last_pos, cx, cy and gap_frames so the change is localized and easy to find.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@services/tracking/tracker.py`:
- Around line 424-454: The helper function _interpolate_trajectory is defined
after main() which breaks common readability conventions; move its definition to
appear before the Tracker class or immediately after the Tracker class (but
before main()) so helper functions are discoverable; ensure any references to
_interpolate_trajectory (e.g., from Tracker methods) still resolve and update
any nearby docstrings/comments if needed, and run tests to confirm no
import/forward-reference issues arise.
- Around line 211-214: Detect when prev.trajectory is empty before falling back
to last_pos and either log a warning or skip interpolation: in the block that
currently sets last_pos from prev.trajectory or falls back to {"x": cx, "y":
cy}, add a conditional that if prev.trajectory is empty (len(prev.trajectory) ==
0) emits a warning via the module/logger used in tracker.py including
identifying context (e.g., track id or frame index and gap_frames) and then
either set a flag to skip interpolation for this gap or continue without
interpolating, rather than silently using the current center; keep references to
prev.trajectory, last_pos, cx, cy and gap_frames so the change is localized and
easy to find.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df50f261-d981-48be-94b9-368daab4c456
📒 Files selected for processing (1)
services/tracking/tracker.py
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
2 participants
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
Resolves #68
This PR implements linear frame interpolation for trajectories when a tracked object experiences temporary occlusions or missed YOLO detections. By generating smooth intermediate bounding box steps and maintaining timeline continuity, this enhancement ensures that downstream metric engines correctly calculate cumulative dwell times and prevents visual snapping on front-end dashboards.
🛠️ Changes Implemented
1. Core Logic (
services/tracking/tracker.py)_interpolate_trajectory()to calculate uniform coordinate and bounding box dimension deltas over a dynamic frame gap sequence."interpolated": Trueflags to all synthetic sequence points to guarantee tracking data telemetry transparency.2decimal places) to all calculated positions (x, y, w, h) for consistent data schemas and dashboard safety.[]ifgap_frames <= 0.2. Test Coverage (
tests/test_tracker.py)Added a comprehensive suite of
pytestcases to lock down correctness and prevent future regression:test_interpolate_trajectory_success: Validates accurate linear coordinate steps, exact index incrementation, and proper metadata injection across standard multi-frame occlusions.test_interpolate_trajectory_no_gap: Assures defensive short-circuiting against zero or negative frame gap arguments.test_interpolate_trajectory_no_movement: Simulates static occlusion cases to verify coordinate locks function flawlessly during stationary dwell accumulations.🧪 Verification & Testing Results
All tests run locally inside an isolated virtual environment are passing successfully: