Skip to content

docs: add Google-style docstrings to detection.py and tracker.py#78

Open
SiddharthRiot wants to merge 5 commits into
Devnil434:mainfrom
SiddharthRiot:fix/docstrings-detection-tracker
Open

docs: add Google-style docstrings to detection.py and tracker.py#78
SiddharthRiot wants to merge 5 commits into
Devnil434:mainfrom
SiddharthRiot:fix/docstrings-detection-tracker

Conversation

@SiddharthRiot
Copy link
Copy Markdown
Contributor

@SiddharthRiot SiddharthRiot commented May 16, 2026

Closes #4

Changes

  • Added Google-style docstrings to all public methods in services/detection/detection.py and services/tracking/tracker.py
  • All docstrings include Args, Returns, and Example sections
  • Fixed undefined names (DetectionFrameDetectionFrameSchema, DetectionDetectionSchema)
  • Fixed incorrect imports and removed unused variables
  • ruff check passes with no errors

Testing

python -m ruff check services/detection/detection.py services/tracking/tracker.py
All checks passed!

Summary by CodeRabbit

  • Improvements

    • Detection now returns structured per-object results (bounding box, confidence, class); per-detection zone membership is no longer included.
    • Visualization simplified to bounding boxes, label+confidence, centroid dot, and a HUD counter; polygon/zone overlays removed.
    • CLI demo reads actual stream width before logging.
  • Documentation

    • Clarified and expanded docs and docstrings across detection, tracking, and memory components.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

Detector.detect() now returns a schema-based DetectionFrameSchema (DetectionSchema items with BoundingBox, confidence, class_id). Visualization and CLI demo simplified to draw bounding boxes, label+confidence, centroid dots, and a HUD; zone membership is computed but not returned. Tracker and Memory receive docstring/comment-only edits.

Changes

Detection Output Schema and Visualization Refactoring

Layer / File(s) Summary
Detection output schema and Detector interface
services/detection/detection.py
Detector.detect() now returns DetectionFrameSchema with per-detection DetectionSchema containing BoundingBox(x1, y1, x2, y2), confidence, and class_id; zones computed from centroids are no longer stored; module and class docstrings updated.
Detection visualization and CLI integration
services/detection/detection.py
draw_detections() now accepts DetectionFrameSchema and renders bounding boxes, label+confidence text, centroid dots, and a HUD counter; polygon/zone overlays and scene-graph/LLM prompt printing removed; main() reads CAP_PROP_FRAME_WIDTH explicitly and displays annotated frames.

Documentation and Non-functional Edits (Tracker & Memory)

Layer / File(s) Summary
Memory module docstring reflow
services/memory/memory.py
Reflowed docstrings for internal Redis key helpers and record/update helpers; no behavior or public-signature changes.
Tracker class documentation and interfaces
services/tracking/tracker.py
Expanded Google-style docstrings for Tracker.__init__, update(), drain_lifecycle_events(), _emit_lifecycle(), _cosine_similarity(), and main(); removed/simplified divider/inline comments; sys.path and CLI/window strings standardized; no runtime/signature changes.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Detector
  participant Renderer
  participant Display

  Client->>Detector: frame (np.ndarray), frame_id
  Detector-->>Client: DetectionFrameSchema (detections with BoundingBox, confidence, class_id)
  Client->>Renderer: draw_detections(frame, det_frame)
  Renderer-->>Display: annotated frame (bboxes, labels, centroid dots, HUD)
  Display->>Client: cv2.imshow shown to user
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Devnil434/Eagle#103: Overlaps with Redis/memory docstring updates in services/memory/memory.py.

Poem

I hop through frames with boxes clear and bright,
I dot the centers, label each small sight,
Docstrings polished, comments put in line,
Tracks keep steady — visuals lean and fine,
🐇 I render, count, and watch each bounding light.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning PR title mentions adding docstrings to detection.py and tracker.py, which aligns with the main objective, but the actual changes extend beyond docstrings to include significant refactoring of detection logic and schema types. Update the title to reflect all substantial changes, such as 'refactor: update detection schema and add docstrings' or clarify scope in the PR description.
Linked Issues check ⚠️ Warning The PR adds Google-style docstrings to public methods in detection.py and tracker.py and passes ruff checks, meeting issue #4 requirements. However, detection.py changes include significant refactoring (DetectionFrame → DetectionFrameSchema, removal of zone overlays) beyond docstring scope. Clarify whether the detection.py refactoring is intentional or scope creep. If intentional, update issue #4 requirements or link additional issues; if unintentional, revert detection.py to docstring-only changes.
Out of Scope Changes check ⚠️ Warning detection.py contains significant refactoring beyond docstrings: DetectionFrame → DetectionFrameSchema schema type changes, draw_detections function signature updates, zone overlay removal, and LLM prompt removal. These changes are not addressed in issue #4. Separate docstring-only changes in detection.py from refactoring changes into distinct PRs, or update issue #4 to explicitly include the refactoring scope.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
services/detection/detection.py (1)

31-33: ⚡ Quick win

Remove stale zone-membership work from detect() and align the docstrings with the actual return contract.

get_zones_for_point(cx, cy) is executed per detection and discarded, so this adds avoidable work in a hot loop. At the same time, the docstrings still claim zone memberships are part of returned detections, which no longer matches the constructed DetectionSchema.

Proposed cleanup
-from services.detection.zones import DEFAULT_ZONES, get_zones_for_point
+from services.detection.zones import DEFAULT_ZONES
@@
-    DetectionFrameSchema objects with bounding boxes, labels, confidence
-    scores, and zone memberships.
+    DetectionFrameSchema objects with bounding boxes, labels, and confidence
+    scores.
@@
-            DetectionFrameSchema with all detected objects and zone memberships.
+            DetectionFrameSchema with all detected objects.
@@
-            _ = [z.name for z in get_zones_for_point(cx, cy)]
-
             detections.append(DetectionSchema(

Also applies to: 71-72, 92-93

🤖 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/detection/detection.py` around lines 31 - 33, The detect() function
still calls get_zones_for_point(cx, cy) per detection and mentions zone
memberships in its docstring even though returned objects are
DetectionSchema/DetectionFrameSchema without zone data; remove the per-detection
get_zones_for_point call and any unused zone-handling variables from detect(),
update the detect() and related docstrings to reflect that detections no longer
include zone memberships (only bounding boxes, labels, and confidence), and
ensure DetectionFrameSchema/DetectionSchema construction matches the docstrings;
search for and remove the same stale calls/comments around the other noted
locations (lines ~71-72 and ~92-93) so the implementation and docs are
consistent.
🤖 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 317-318: The docstring for the cosine similarity function
(cosine_similarity) incorrectly states the return range as [0, 1]; update it to
reflect the true range of cosine similarity: "Float in [-1, 1] representing
similarity; 0.0 if either norm is zero." Modify the docstring text in tracker.py
(the cosine_similarity function) to use [-1, 1] and keep the note about the
zero-norm early return.

---

Nitpick comments:
In `@services/detection/detection.py`:
- Around line 31-33: The detect() function still calls get_zones_for_point(cx,
cy) per detection and mentions zone memberships in its docstring even though
returned objects are DetectionSchema/DetectionFrameSchema without zone data;
remove the per-detection get_zones_for_point call and any unused zone-handling
variables from detect(), update the detect() and related docstrings to reflect
that detections no longer include zone memberships (only bounding boxes, labels,
and confidence), and ensure DetectionFrameSchema/DetectionSchema construction
matches the docstrings; search for and remove the same stale calls/comments
around the other noted locations (lines ~71-72 and ~92-93) so the implementation
and docs are consistent.
🪄 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: 5306bbbb-8977-4efd-ac30-11a09042a5f0

📥 Commits

Reviewing files that changed from the base of the PR and between 29a47a7 and 8b9277f.

📒 Files selected for processing (2)
  • services/detection/detection.py
  • services/tracking/tracker.py

Comment on lines +317 to +318
Float in [0, 1] representing similarity; 0.0 if either norm is zero.
"""
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix incorrect cosine similarity return range in docstring.

Line 317 says similarity is in [0, 1], but cosine similarity is typically in [-1, 1] (except zero-norm early return). Please update the docstring range to match the implementation.

Suggested docstring fix
-            Float in [0, 1] representing similarity; 0.0 if either norm is zero.
+            Float in [-1, 1] representing similarity; 0.0 if either norm is zero.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Float in [0, 1] representing similarity; 0.0 if either norm is zero.
"""
Float in [-1, 1] representing similarity; 0.0 if either norm is zero.
"""
🤖 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 317 - 318, The docstring for the
cosine similarity function (cosine_similarity) incorrectly states the return
range as [0, 1]; update it to reflect the true range of cosine similarity:
"Float in [-1, 1] representing similarity; 0.0 if either norm is zero." Modify
the docstring text in tracker.py (the cosine_similarity function) to use [-1, 1]
and keep the note about the zero-norm early return.

@SiddharthRiot
Copy link
Copy Markdown
Contributor Author

Hey @Devnil434, the CI failure is not related to my changes. The lint check passes successfully. the test job fails due to missing pytest-cov dependency in GitHub actions workflow (--cov flag is unrecognized). This is a pre-existing issue in the repo. happy to fix the workflow if needed, just let me know!

@kunal-9090
Copy link
Copy Markdown

I checked this PR and prepared a rebased repair branch because the current PR branch is conflicting with main. The original docstring/code refactor overlapped with upstream changes, so I kept current main behavior as the source of truth and retained the safe CI/import cleanup needed by the current workflows.\n\nRepair branch: https://github.com/kunal-9090/Eagle/tree/codex/fix-pr-78-rebase\n\nWhat changed in the repair branch:\n- rebased the PR branch onto current upstream main\n- resolved conflicts in services/detection/detection.py and services/tracking/tracker.py by preserving current upstream behavior\n- stopped eager tracking imports in services/init.py so focused memory tests do not import optional tracking deps\n- corrected tracker settings import to rom libs.config.settings import settings\n\nLocal verification: python -m ruff check services/memory/ libs/schemas/memory.py services/init.py services/tracking/tracker.py passes. Direct push to the contributor fork was blocked with 403 for kunal-9090, so I pushed the repair branch to my fork and linked it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Write docstrings for detector.py and tracker.py

3 participants