feat(sessions): add active_camera property for Python control#837
feat(sessions): add active_camera property for Python control#837
Conversation
Add the ability to control which camera a frontend session views through
from Python via `vis.sessions["session_id"].active_camera = "camera_key"`.
Backend:
- Add REST endpoints GET/PUT for /api/rooms/{room}/sessions/{session}/active-camera
- Add ACTIVE_CAMERA_UPDATE socket event for Python -> Frontend sync
- Initialize active_camera to session's own camera on join, cleanup on disconnect
- Add validation: KeyError for missing camera, TypeError for non-Camera geometry
Frontend:
- Handle active_camera:update socket event
- Sync sidebar camera changes to backend via REST PUT
- Fix auth token retrieval (was using wrong localStorage key)
Python API:
- session.active_camera: get/set which camera to view through
- session.camera: now gets/sets the currently active camera geometry
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds per-session "active camera" support: frontend socket handler and store setter, client PUT/GET endpoints to persist session active camera in Redis, server-side session key helpers and events to initialize/cleanup active camera, session manager property for active_camera, and integration tests. Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend App
participant API as Server API
participant Redis as Redis
participant Socket as SocketIO
User->>Frontend: attachToCamera(cameraKey)
Frontend->>Frontend: setAttachedCameraKey(cameraKey)
Frontend->>API: PUT /api/rooms/{room}/sessions/{session}/active-camera {active_camera: cameraKey}
API->>Redis: SET room:{room}:session:{session}:active_camera = cameraKey
API->>Socket: emit ACTIVE_CAMERA_UPDATE to session
Socket->>Frontend: active_camera:update event
Frontend->>Frontend: update store (setAttachedCameraKey)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
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
🤖 Fix all issues with AI agents
In @src/zndraw/api_manager.py:
- Around line 1716-1741: The get_active_camera method's return type is
inaccurate because response.json().get("active_camera") may be None; update the
function signature from def get_active_camera(self, session_id: str) -> str: to
return an optional type (e.g., -> str | None or -> Optional[str]) and import
Optional from typing if needed; also update the docstring "Returns" section to
indicate it can return None and ensure any callers handle a None value
appropriately.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/src/hooks/useSocketManager.tsapp/src/store.tsxsrc/zndraw/api_manager.pysrc/zndraw/app/constants.pysrc/zndraw/app/events.pysrc/zndraw/app/redis_keys.pysrc/zndraw/app/session_routes.pysrc/zndraw/session_manager.pytests/test_active_camera.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: You MUST NEVER@pytest.mark.xfailor similar - all tests must pass!
All default values (e.g.,camera,particles) must be defined exclusively within the Pydantic model. Do not scatter fallback logic throughout the codebase.
Do not perform null checks combined with hardcoded literals for default values. Rely entirely on the schema to populate default values during initialization.
You can not use LUA scripts with Redis!
If sensible, implement collections.abc interfaces for your classes, such as MutableMapping or MutableSequence.
Use numpy style docstrings in Python code.
Docstrings must be concise and to the point.
Use type hints wherever possible in Python. Uselist[int|float] | Noneinstead oft.Optional[t.List[int|float]]!
Imports should always be at the top of the file in Python.
Files:
src/zndraw/app/constants.pysrc/zndraw/api_manager.pytests/test_active_camera.pysrc/zndraw/app/session_routes.pysrc/zndraw/app/redis_keys.pysrc/zndraw/app/events.pysrc/zndraw/session_manager.py
**/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
When designing new tests, read the old tests first to understand the existing patterns. Use
pytest.mark.parametrizeto avoid code duplication. Tests should be very specific and test only one thing. Avoid complex test setups. Each test must be a function, not a method of a class!
Files:
tests/test_active_camera.py
🧬 Code graph analysis (3)
app/src/store.tsx (2)
app/src/components/CameraManager.js (1)
sessionId(18-18)app/src/utils/auth.ts (1)
getToken(152-154)
src/zndraw/app/events.py (2)
src/zndraw/app/redis_keys.py (2)
session_active_camera(345-360)session_id(617-619)src/zndraw/session_manager.py (1)
get_session_camera_key(16-18)
src/zndraw/session_manager.py (3)
src/zndraw/geometries/camera.py (1)
Camera(27-223)src/zndraw/api_manager.py (2)
get_active_camera(1716-1741)set_active_camera(1743-1765)src/zndraw/app/session_routes.py (2)
get_active_camera(145-166)set_active_camera(174-217)
🪛 Ruff (0.14.10)
src/zndraw/session_manager.py
172-172: Avoid specifying long messages outside the exception class
(TRY003)
177-179: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pytest (3.11, ubuntu-latest)
- GitHub Check: pytest (3.12, ubuntu-latest)
- GitHub Check: pytest (3.13, ubuntu-latest)
🔇 Additional comments (25)
src/zndraw/app/constants.py (1)
26-26: LGTM!The new
ACTIVE_CAMERA_UPDATEconstant follows the established naming convention and is consistent with other socket event names in the class.src/zndraw/app/events.py (2)
310-312: LGTM!The active camera key cleanup is correctly placed alongside other session-specific cleanup operations and properly scoped within the
if room_name:block.
813-816: LGTM!The active camera initialization correctly defaults to the session's own camera and is properly placed after
create_session_camera()to ensure the camera geometry exists.src/zndraw/app/redis_keys.py (1)
345-360: LGTM!The new
session_active_cameramethod follows numpy-style docstrings as per coding guidelines and provides clear documentation. The Redis key pattern is hierarchical and well-structured.app/src/hooks/useSocketManager.ts (2)
258-261: LGTM!The handler correctly uses
useAppStore.getState()pattern consistent with other handlers in this hook, and the type annotation for the data parameter is present.
732-732: LGTM!Socket event registration and cleanup are properly paired and follow the established pattern for other event handlers in this hook.
Also applies to: 794-794
src/zndraw/session_manager.py (4)
58-76: LGTM!The
cameragetter now correctly returns the geometry at theactive_camerakey, enabling sessions to view through any camera. The docstring accurately describes the updated behavior.
78-90: LGTM!The
camerasetter correctly writes to theactive_camerageometry key, consistent with the getter behavior.
143-152: LGTM!The
active_cameragetter is clean and delegates to the API correctly. Type hints and numpy-style docstring are present as per coding guidelines.
154-181: LGTM!The
active_camerasetter correctly validates both existence and type before updating. The validation order (existence → type) is correct, and the descriptive error messages aid debugging. The static analysis hints about long exception messages (TRY003) are acceptable here since the messages are concise and provide useful context without justifying custom exception classes.app/src/store.tsx (4)
16-16: LGTM!Import correctly added for
getTokento support auth token retrieval for server sync.
221-221: LGTM!New action declaration aligns with the existing pattern and matches the implementation.
1036-1067: LGTM - Good implementation of server sync.The optimistic update pattern is correct, error handling is graceful, and auth token is properly included when available. The
if (roomId && sessionId)guard appropriately prevents unnecessary requests.
1069-1072: LGTM!Simple direct setter correctly avoids server sync loops when used by the socket event handler. The comment clearly explains the purpose.
src/zndraw/api_manager.py (1)
1743-1765: LGTM!Implementation follows existing patterns. The method correctly delegates to the REST endpoint. Note that validation (KeyError/TypeError for invalid cameras) happens at the
FrontendSession.active_camerasetter level insession_manager.py, not at this REST endpoint.tests/test_active_camera.py (8)
13-23: LGTM!Clean, focused test validating the default active camera behavior.
26-41: LGTM!Well-structured test for the active_camera setter.
44-62: LGTM!Good coverage of the switch-back scenario.
65-85: LGTM!Critical test ensuring per-session isolation of the active_camera property.
88-102: LGTM!Good test verifying server-side persistence across API calls.
105-114: LGTM!Proper error handling test for non-existent geometry keys.
117-129: LGTM!Good type validation test ensuring only Camera geometries can be set as active camera.
132-153: LGTM!Excellent integration test verifying the interaction between
session.camerasetter andactive_camera. This ensures the camera setter updates the currently active camera rather than always the session's own camera.src/zndraw/app/session_routes.py (2)
140-166: LGTM!Simple and correct implementation. The endpoint returns
Nonefor theactive_cameravalue if the Redis key doesn't exist, which is acceptable behavior for this use case.
169-217: This REST endpoint intentionally relies on boundary validation rather than endpoint-level validation.The endpoint stores
active_cameradirectly in Redis without validation. However, this is by design: validation happens at the call site boundaries where the endpoint is called:
- Frontend (store.tsx): Validates with
!camera || camera.type !== "Camera"before calling the endpoint- Python API (session_manager.py): Validates camera existence and type in the
active_camerasetter before calling the endpointOnly authenticated clients (@require_auth) can reach this endpoint, and both trusted code paths validate before calling it. If an invalid key somehow reached Redis, accessing
session.camerawould raise a KeyError.This is a deliberate trusted-client design pattern. No changes needed unless you want defense-in-depth validation at the endpoint level.
| def get_active_camera(self, session_id: str) -> str: | ||
| """Get active camera for a frontend session. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| session_id : str | ||
| Session identifier. | ||
|
|
||
| Returns | ||
| ------- | ||
| str | ||
| Camera geometry key this session is viewing through. | ||
|
|
||
| Raises | ||
| ------ | ||
| requests.HTTPError | ||
| If request fails. | ||
| """ | ||
| headers = self._get_headers() | ||
| response = requests.get( | ||
| f"{self.url}/api/rooms/{self.room}/sessions/{session_id}/active-camera", | ||
| headers=headers, | ||
| timeout=10.0, | ||
| ) | ||
| response.raise_for_status() | ||
| return response.json().get("active_camera") |
There was a problem hiding this comment.
Return type annotation may be inaccurate.
The method returns response.json().get("active_camera") which can be None if the key doesn't exist in Redis. The return type annotation should be str | None to accurately reflect this.
Suggested fix
- def get_active_camera(self, session_id: str) -> str:
+ def get_active_camera(self, session_id: str) -> str | None:
"""Get active camera for a frontend session.
Parameters
----------
session_id : str
Session identifier.
Returns
-------
- str
- Camera geometry key this session is viewing through.
+ str | None
+ Camera geometry key this session is viewing through, or None if not set.
Raises
------
requests.HTTPError
If request fails.
"""🤖 Prompt for AI Agents
In @src/zndraw/api_manager.py around lines 1716 - 1741, The get_active_camera
method's return type is inaccurate because response.json().get("active_camera")
may be None; update the function signature from def get_active_camera(self,
session_id: str) -> str: to return an optional type (e.g., -> str | None or ->
Optional[str]) and import Optional from typing if needed; also update the
docstring "Returns" section to indicate it can return None and ensure any
callers handle a None value appropriately.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #837 +/- ##
========================================
Coverage 79.97% 79.97%
========================================
Files 162 163 +1
Lines 19769 19896 +127
========================================
+ Hits 15810 15912 +102
- Misses 3959 3984 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Ensure that the active camera is always returned from the response.
2 participants
Add the ability to control which camera a frontend session views through from Python via
vis.sessions["session_id"].active_camera = "camera_key".Backend:
Frontend:
Python API:
Summary by CodeRabbit
New Features
Behavior
Tests
✏️ Tip: You can customize this high-level summary in your review settings.