feat: add scene graph representation layer for surveillance reasoning#89
Merged
Devnil434 merged 6 commits intoMay 19, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Pydantic graph schemas, implements a NetworkX-backed SceneGraph that builds from tracked frames and serializes to an LLM-ready edge list, updates the prompt builder to accept SceneGraph + event_description, and replaces tests with a suite validating graph construction and prompt serialization. ChangesScene Graph Refactoring: Schema-Driven Representation with NetworkX
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/schemas/graph.py (1)
28-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce
GraphEdgedistance invariants at the schema boundary.
GraphEdgecurrently allows invalid states (e.g., negativedistance_px, ordistance_pxon non-NEARedges). Validating this in-model prevents bad graph data from propagating downstream.Suggested fix
from pydantic import BaseModel +from pydantic import model_validator ... class GraphEdge(BaseModel): source: str # node id target: str # node id edge_type: EdgeType distance_px: Optional[float] = None # for NEAR edges + + `@model_validator`(mode="after") + def validate_distance_rules(self): + if self.distance_px is not None and self.distance_px < 0: + raise ValueError("distance_px must be non-negative") + if self.edge_type != EdgeType.NEAR and self.distance_px is not None: + raise ValueError("distance_px is only valid for NEAR edges") + return self🤖 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 `@libs/schemas/graph.py` around lines 28 - 33, GraphEdge allows invalid states; add validation inside the GraphEdge Pydantic model to enforce that distance_px, when provided, is non-negative, that distance_px is only set for edges with edge_type == EdgeType.NEAR, and that NEAR edges must have a non-null distance_px; implement this via a validator/root_validator on GraphEdge that inspects edge_type and distance_px and raises ValueError with a clear message when invariants are violated (referencing GraphEdge.distance_px and EdgeType.NEAR).
🧹 Nitpick comments (1)
services/reasoning/scene_graph.py (1)
16-17: ⚡ Quick winUse
MAX_PROMPT_TOKENSin truncation logic instead of a hardcoded literal.Line 99 hardcodes
200, so updatingMAX_PROMPT_TOKENShas no effect. This is an easy drift bug to prevent.Suggested fix
- if len(words) > 200: - serialized = " ".join(words[:200]) + "\n[...truncated]" + word_budget = int(self.MAX_PROMPT_TOKENS / 1.3) + if len(words) > word_budget: + serialized = " ".join(words[:word_budget]) + "\n[...truncated]"Also applies to: 99-100
🤖 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/reasoning/scene_graph.py` around lines 16 - 17, The truncation logic in scene_graph.py currently uses a hardcoded literal 200 instead of the module-level constant MAX_PROMPT_TOKENS, causing changes to the constant to have no effect; update the truncation code (the function that trims prompt/token length where the literal 200 is used) to reference MAX_PROMPT_TOKENS instead, and replace any other occurrences at the nearby truncation logic (the two occurrences noted) so all token-limit checks use the single MAX_PROMPT_TOKENS symbol.
🤖 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/reasoning/scene_graph.py`:
- Around line 39-43: The code in from_tracked_frame is assuming each
zone/object/other item has an "id" and will KeyError on malformed items; update
all places that access ["id"] (eg. the loops that call
sg.add_node(GraphNode(id=zone["id"], ...)) and similar lines at 52-67) to
defensively fetch the id (e.g., use .get("id") or "id" in item) and skip or log
the item when id is missing before calling GraphNode or sg.add_node/sg.add_edge;
ensure you apply the same check for zones, objects and any other tracked_frame
entries referenced in from_tracked_frame so malformed entries don’t crash the
graph build.
In `@tests/test_scene_graph.py`:
- Around line 132-137: The test_empty_graph_serialization currently asserts that
"?" is not in the prompt but the SceneGraph.to_prompt_str uses the arrow marker
"→" for edges; update the assertion in test_empty_graph_serialization to check
that the actual edge marker "→" is not present in the serialized prompt (e.g.,
assert "→" not in prompt) so the test fails if edge serialization appears in an
empty SceneGraph.
---
Outside diff comments:
In `@libs/schemas/graph.py`:
- Around line 28-33: GraphEdge allows invalid states; add validation inside the
GraphEdge Pydantic model to enforce that distance_px, when provided, is
non-negative, that distance_px is only set for edges with edge_type ==
EdgeType.NEAR, and that NEAR edges must have a non-null distance_px; implement
this via a validator/root_validator on GraphEdge that inspects edge_type and
distance_px and raises ValueError with a clear message when invariants are
violated (referencing GraphEdge.distance_px and EdgeType.NEAR).
---
Nitpick comments:
In `@services/reasoning/scene_graph.py`:
- Around line 16-17: The truncation logic in scene_graph.py currently uses a
hardcoded literal 200 instead of the module-level constant MAX_PROMPT_TOKENS,
causing changes to the constant to have no effect; update the truncation code
(the function that trims prompt/token length where the literal 200 is used) to
reference MAX_PROMPT_TOKENS instead, and replace any other occurrences at the
nearby truncation logic (the two occurrences noted) so all token-limit checks
use the single MAX_PROMPT_TOKENS symbol.
🪄 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: 2b565d06-5992-4c48-bd40-deecb7df946d
📒 Files selected for processing (7)
libs/schemas/graph.pyservices/__init__.pyservices/reasoning/__init__.pyservices/reasoning/prompts.pyservices/reasoning/scene_graph.pytests/__init__.pytests/test_scene_graph.py
💤 Files with no reviewable changes (2)
- services/init.py
- services/reasoning/init.py
Comment on lines
+132
to
137
| def test_empty_graph_serialization(self): | ||
| sg = SceneGraph(timestamp=0.0) | ||
| prompt = sg.to_prompt_str() | ||
| assert "t=0.0s" in prompt | ||
| assert "?" not in prompt # no edges | ||
|
|
There was a problem hiding this comment.
Fix empty-graph assertion to check the actual edge marker.
Line 136 checks for "?", but edge lines use →. This test can pass even if edge serialization regresses.
Suggested fix
- assert "?" not in prompt # no edges
+ assert "→" not in prompt # no edges🤖 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_scene_graph.py` around lines 132 - 137, The
test_empty_graph_serialization currently asserts that "?" is not in the prompt
but the SceneGraph.to_prompt_str uses the arrow marker "→" for edges; update the
assertion in test_empty_graph_serialization to check that the actual edge marker
"→" is not present in the serialized prompt (e.g., assert "→" not in prompt) so
the test fails if edge serialization appears in an empty SceneGraph.
- Add GraphNode/GraphEdge Pydantic schemas (libs/schemas/graph.py) - Implement SceneGraph with NetworkX MultiDiGraph (services/reasoning/scene_graph.py) - Integrate scene graph into LLM prompt builder (services/reasoning/prompts.py) - Add comprehensive unit tests — 11/11 passing Closes Devnil434#69
- Defensively handle missing IDs in from_tracked_frame() to avoid KeyError - Fix empty-graph serialization test to check for '→' instead of '?'
Contributor
Author
|
I will review the errors thoroughly and get back to you. |
Contributor
Author
|
Apologies for the frequent updates, it seems like I might be spamming the PR a bit with these commits. Would it be okay if I focus on resolving the remaining issues locally first, and then push a single comprehensive update so that CodeRabbitAI can verify the fixes all at once? Let me know if that works! |
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.
Closes #69
Summary by CodeRabbit
New Features
Refactor
Tests