feat: add adaptive zone anomaly baselines using Welford's algorithm#91
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR implements adaptive per-zone anomaly detection using Welford's online algorithm. The ChangesAdaptive Per-Zone Anomaly Baseline
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as MemoryService<br/>handle_lifecycle_event
participant Lost as MemoryService<br/>_handle_lost
participant Baseline as ZoneBaseline
participant Redis
participant LogWriter as _append_event<br/>& _update_record
Client->>Handler: LOST frame event
Handler->>Lost: LOST TrackEvent
Lost->>Lost: zone_anomalous = False
loop each zone in event.zones_present
Lost->>Baseline: is_anomalous(dwell)
Baseline->>Redis: load zone:{name}:stats
Baseline-->>Lost: anomaly boolean
Lost->>Baseline: update(dwell)
Baseline->>Redis: save updated stats
end
Lost->>Redis: query track record
Lost->>LogWriter: _update_record with anomalous flag
Lost->>LogWriter: _append_event with anomalous flag
LogWriter->>Redis: persist anomalous in record & event log
Lost-->>Handler: (global_id, zone_anomalous)
Handler->>Client: event log updated
sequenceDiagram
participant Client
participant API as GET /zones/{name}/stats
participant Dependency as _get_redis
participant Baseline as ZoneBaseline
participant Redis
Client->>API: GET /zones/lobby/stats
API->>Dependency: _get_redis(request)
Dependency->>Redis: retrieve app.state.redis
alt Redis available
Dependency-->>API: redis_client
else Redis unavailable
Dependency-->>API: HTTPException(503)
end
API->>Baseline: ZoneBaseline(redis, "lobby")
Baseline->>Redis: load zone:lobby:stats
Baseline-->>API: stats loaded
API->>Baseline: get_stats()
Baseline-->>API: {count, mean, variance, std, m2}
API-->>Client: ZoneStatsResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The changes span a new service module (Welford algorithm with Redis persistence), integration into an existing core service (MemoryService), new API endpoint, schema update, and comprehensive tests. The anomaly detection logic requires careful review of the Welford implementation, merge-conflict markers in memory.py, state mutation ordering (detect-before-update), and Redis persistence semantics. The variety of implementation roles across baseline service, integration, API, and tests, combined with stateful computation and persistence, demands heterogeneous reasoning per area. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
dilshadalikhan2004
force-pushed
the
feat/adaptive-zone-baseline
branch
from
dbf8e21 to
288c929
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/memory/memory.py (1)
215-236:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResolve both unresolved merge-conflict blocks in
services/memory/memory.py.The merge-conflict markers at lines 217–236 and 252–435 leave this module unparsable, blocking Python syntax validation. When resolving, retain the
anomalousfield in both_update_record()and_append_event()to preserve anomaly detection across the LOST event path.Merge-conflict markers verified present at:
- Lines 217–236 in
_update_record()- Lines 252–435 in
_append_event()🤖 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/memory/memory.py` around lines 215 - 236, Remove the Git conflict markers and merge the two variants so the file parses: in _update_record() keep the compact update and include the "anomalous" key (set from the anomalous parameter) along with state, last_seen_frame, last_seen_ms, dwell_time_seconds, and zones_present; likewise resolve the conflict in _append_event() to ensure the event-record update path for LOST events preserves and writes the "anomalous" field into the stored record (use the same key name "anomalous"), and remove any leftover <<<<<<<, =======, and >>>>>>> markers so the module is syntactically valid.
🤖 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 `@apps/backend/main.py`:
- Around line 26-35: Resolve the merge conflict by removing the Git conflict
markers and creating a single FastAPI app instance: import zones_router from
apps.backend.routes.zones, keep the FastAPI() instantiation as the single `app`,
and ensure `app.state.redis` is initialized (using the redis client currently
aliased as `redis_sync` or whichever Redis client you prefer) before including
`zones_router` so routes that use `request.app.state.redis` work; register the
router (`app.include_router(zones_router)`) and remove the
duplicate/contradictory blocks so there is only one `app` definition and no
leftover conflict markers.
In `@services/memory/memory.py`:
- Around line 186-190: The loop over event.zones_present can process the same
zone multiple times per event, skewing Redis baselines; before creating
ZoneBaseline and calling baseline.is_anomalous / baseline.update, deduplicate
the zones in event.zones_present (while preserving order if needed) and iterate
over the unique list instead (e.g., use an order-preserving dedupe like
dict.fromkeys or a set if order doesn't matter) so each zone's baseline is
learned/checked exactly once and zone_anomalous is set appropriately.
- Around line 197-198: _handle_dead currently calls _update_record(event,
TrackState.DEAD.value) which uses the default anomalous=False and clears any
prior anomaly flag; change the call to preserve the anomaly bit by passing the
existing anomalous state (e.g., _update_record(event, TrackState.DEAD.value,
anomalous=event.anomalous)) or, if the TrackLifecycleEvent lacks that field,
fetch the stored record via _get_record(track_id) and pass its anomalous value
into _update_record so DEAD transitions do not clear the anomalous flag.
---
Outside diff comments:
In `@services/memory/memory.py`:
- Around line 215-236: Remove the Git conflict markers and merge the two
variants so the file parses: in _update_record() keep the compact update and
include the "anomalous" key (set from the anomalous parameter) along with state,
last_seen_frame, last_seen_ms, dwell_time_seconds, and zones_present; likewise
resolve the conflict in _append_event() to ensure the event-record update path
for LOST events preserves and writes the "anomalous" field into the stored
record (use the same key name "anomalous"), and remove any leftover <<<<<<<,
=======, and >>>>>>> markers so the module is syntactically valid.
🪄 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: c7a95ee3-4ae6-4471-a8e2-ed962e33d512
📒 Files selected for processing (6)
apps/backend/main.pyapps/backend/routes/zones.pylibs/schemas/tracking.pyservices/memory/baseline.pyservices/memory/memory.pytests/test_baseline.py
| <<<<<<< HEAD | ||
| import redis as redis_sync | ||
| from fastapi import FastAPI, Query | ||
| from fastapi.responses import Response | ||
| from prometheus_client import generate_latest | ||
| ======= | ||
| from apps.backend.routes.zones import router as zones_router | ||
|
|
||
| app = FastAPI() | ||
| >>>>>>> 4d99088 (feat: adaptive anomaly baseline per zone using Welford's algorithm) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd apps/backend && wc -l main.py && head -100 main.pyRepository: Devnil434/Eagle
Length of output: 3019
🏁 Script executed:
cd apps/backend && sed -n '35,90p' main.py | cat -nRepository: Devnil434/Eagle
Length of output: 2270
🏁 Script executed:
cd apps/backend && find . -name "zones.py" -type f && cat -n apps/backend/routes/zones.pyRepository: Devnil434/Eagle
Length of output: 156
🏁 Script executed:
cd apps/backend && cat -n routes/zones.pyRepository: Devnil434/Eagle
Length of output: 1679
Resolve this merge conflict and keep a single live app instance.
These conflict markers at lines 26–35 and 41–83 make the file invalid Python. The unresolved split also decides whether zones_router is included and whether app.state.redis is initialized—both required because zones_router depends on request.app.state.redis (see apps/backend/routes/zones.py:26).
🤖 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 `@apps/backend/main.py` around lines 26 - 35, Resolve the merge conflict by
removing the Git conflict markers and creating a single FastAPI app instance:
import zones_router from apps.backend.routes.zones, keep the FastAPI()
instantiation as the single `app`, and ensure `app.state.redis` is initialized
(using the redis client currently aliased as `redis_sync` or whichever Redis
client you prefer) before including `zones_router` so routes that use
`request.app.state.redis` work; register the router
(`app.include_router(zones_router)`) and remove the duplicate/contradictory
blocks so there is only one `app` definition and no leftover conflict markers.
| for zone in event.zones_present: | ||
| baseline = ZoneBaseline(self._r, zone) | ||
| if baseline.is_anomalous(event.dwell_time_seconds): | ||
| zone_anomalous = True | ||
| baseline.update(event.dwell_time_seconds) |
There was a problem hiding this comment.
Deduplicate zones_present before learning.
If the same zone appears twice in one LOST event, this loop records the same dwell twice and permanently skews that zone's Redis baseline.
♻️ Proposed fix
- for zone in event.zones_present:
+ for zone in dict.fromkeys(event.zones_present):
baseline = ZoneBaseline(self._r, zone)
if baseline.is_anomalous(event.dwell_time_seconds):
zone_anomalous = True
baseline.update(event.dwell_time_seconds)🤖 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/memory/memory.py` around lines 186 - 190, The loop over
event.zones_present can process the same zone multiple times per event, skewing
Redis baselines; before creating ZoneBaseline and calling baseline.is_anomalous
/ baseline.update, deduplicate the zones in event.zones_present (while
preserving order if needed) and iterate over the unique list instead (e.g., use
an order-preserving dedupe like dict.fromkeys or a set if order doesn't matter)
so each zone's baseline is learned/checked exactly once and zone_anomalous is
set appropriately.
| def _handle_dead(self, event: TrackLifecycleEvent) -> None: | ||
| self._update_record(event, TrackState.DEAD.value) |
There was a problem hiding this comment.
Preserve the anomaly bit on DEAD transitions.
_handle_dead() uses the default anomalous=False, so a track flagged on LOST is rewritten as non-anomalous when it later expires.
💡 Suggested shape after resolving the conflict
- def _update_record(self, event: TrackLifecycleEvent, state: str, anomalous: bool = False) -> None:
+ def _update_record(
+ self,
+ event: TrackLifecycleEvent,
+ state: str,
+ anomalous: bool | None = None,
+ ) -> None:
record = self._load_record(event.camera_id, event.track_id) or {}
record.update({
"state": state,
"last_seen_frame": event.frame_id,
"last_seen_ms": event.timestamp_ms,
"dwell_time_seconds": event.dwell_time_seconds,
"zones_present": event.zones_present,
- "anomalous": anomalous,
+ "anomalous": record.get("anomalous", False) if anomalous is None else anomalous,
})🤖 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/memory/memory.py` around lines 197 - 198, _handle_dead currently
calls _update_record(event, TrackState.DEAD.value) which uses the default
anomalous=False and clears any prior anomaly flag; change the call to preserve
the anomaly bit by passing the existing anomalous state (e.g.,
_update_record(event, TrackState.DEAD.value, anomalous=event.anomalous)) or, if
the TrackLifecycleEvent lacks that field, fetch the stored record via
_get_record(track_id) and pass its anomalous value into _update_record so DEAD
transitions do not clear the anomalous flag.
8b54c59 to
3ad8991
Compare
Note on CI failuresThe services/memory/init.py → pipeline.py → action_classifier.py ...references settings fields that don't exist in This PR's own tests (tests/test_baseline.py) — 15/15 pass. |
dilshadalikhan2004
force-pushed
the
feat/adaptive-zone-baseline
branch
from
3ad8991 to
69fc14d
Compare
Update: All CI checks now passingPreviously flagged test_memory.py failures have been resolved as part of this PR:
Final Test Results
Closes #72 |
2 participants
Summary
Implements adaptive per-zone dwell anomaly baselines using Welford’s online algorithm with Redis-backed persistence.
Features Added
Online running statistics for dwell times:
Adaptive anomaly detection:
dwell > mean + 2.5 * stdRedis persistence under:
zone:{name}:statsZone statistics API endpoint:
GET /zones/{name}/statsAnomaly flag propagation into tracking events
Unit tests validating statistical correctness and persistence
Technical Notes
Test Status
Known Pre-existing Failures (Unrelated to This PR)
These failures already exist on the base branch and are unrelated to the adaptive baseline implementation:
test_tracker.py11 failures caused by:
SyntaxError: 'return' outside functionservices/tracking/tracker.pyline 234test_scene_graph.pyultralyticsdependency/environment setupFixes #72
Summary by CodeRabbit
Release Notes
New Features
Tests