Add picture viewer module#103
Conversation
📝 WalkthroughWalkthroughAdds a WASM pic-viewer that receives pyeye1 capture broadcasts, validates storage URLs, renders images to a canvas, and reports displayed images. It also adds browser tests, workspace tasks, WebSocket identity configuration, and CI coverage collection. ChangesPic viewer capture flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PyEye1
participant WebSocket
participant PicViewer
participant Canvas
PyEye1->>WebSocket: Broadcast stored capture
WebSocket->>PicViewer: Deliver capture notification
PicViewer->>Canvas: Decode and draw image
PicViewer->>WebSocket: Report displayed image
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/coverage.yamlTraceback (most recent call last): config/ast-grep/rules/doc-summary-ends-with-period.yamlTraceback (most recent call last): Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene Coverage  |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Jul 23, 2026 1:24a.m. | Review ↗ |
| C & C++ | |
Jul 23, 2026 1:24a.m. | Review ↗ |
| Docker | |
Jul 23, 2026 1:24a.m. | Review ↗ |
| Java | |
Jul 23, 2026 1:24a.m. | Review ↗ |
| JavaScript | |
Jul 23, 2026 1:24a.m. | Review ↗ |
| Python | |
Jul 23, 2026 1:24a.m. | Review ↗ |
| Rust | |
Jul 23, 2026 1:24a.m. | Review ↗ |
| Secrets | |
Jul 23, 2026 1:24a.m. | Review ↗ |
| Code coverage |  |
Jul 23, 2026 1:52a.m. | Review ↗ |
Code Coverage Summary
| Language | Line Coverage (New Code) | Line Coverage (Overall) |
|---|---|---|
| Aggregate | 26.7% |
57.8% [▼ down 1.4% from main] |
| Python | 100% |
85.2% |
| Rust | 24.7% |
55.6% [▼ down 1.5% from main] |
➟ Additional coverage metrics may have been reported. See full coverage report ↗
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Documentation | 1 minor |
| Security | 1 high |
| Complexity | 1 medium 2 minor |
🟢 Metrics 0 complexity · 6 duplication
Metric Results Complexity 0 Duplication 6
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/ws-modules/pic-viewer/src/lib.rs`:
- Around line 227-231: Update show_image so image.decode() races against a
bounded timer, returning an error when the deadline expires instead of awaiting
indefinitely. Preserve successful decoding and propagate fetch/decode failures,
allowing the caller to regain control after timeout.
- Around line 181-187: Update queue_capture_notification to bound the pending
VecDeque and retain only the newest capture when the queue reaches its limit.
After parsing a valid notification, discard stale queued captures as needed
before enqueueing the new notification, preserving the existing behavior for
invalid values.
- Around line 207-220: Update the URL validation in the capture-notification
parsing flow around message.get("url") so it accepts only canonical storage
paths, not arbitrary strings with a "/storage/" prefix. Parse and normalize the
path, reject traversal or any non-canonical segments that resolve outside
storage, and construct or retain the URL only after validation so browser
navigation remains confined to storage.
- Around line 12-16: Move the clippy::future_not_send and clippy::single_call_fn
expectations from the crate-level attribute to the specific async functions or
wait_for_* helpers that trigger them. Preserve the existing justification text
and ensure each expectation is scoped only to the item requiring it.
In `@services/ws-modules/pyeye1/pyeye1/eye_detection.py`:
- Around line 171-176: Update the run-workflow test double used by
test_run_workflow so platform.save_eye_capture returns a fixture filename
instead of None. Assert that the successful capture path emits a broadcast
envelope containing that filename, while preserving the existing error-path
behavior.
🪄 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 Plus
Run ID: c2a5717e-27ea-4147-992b-664f75b6627f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.github/workflows/coverage.yaml.mise/config.coverage.toml.mise/config.rust.toml.mise/config.tomlCargo.tomlconfig/ast-grep/rules/doc-summary-ends-with-period.yamlservices/ws-modules/pic-viewer/Cargo.tomlservices/ws-modules/pic-viewer/build.rsservices/ws-modules/pic-viewer/src/lib.rsservices/ws-modules/pic-viewer/tests/parse.rsservices/ws-modules/pic-viewer/tests/show_image.rsservices/ws-modules/pyeye1/pkg/et_ws_pyeye1.jsservices/ws-modules/pyeye1/pyeye1/eye_detection.pyservices/ws-wasm-agent/src/lib.rs
| #![expect( | ||
| clippy::future_not_send, | ||
| clippy::single_call_fn, | ||
| reason = "browser WASM module: JsFuture is !Send; module-local helpers like wait_for_* are single-use by design" | ||
| )] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Scope the Clippy expectations to the items that need them. The crate-level #[expect] is broader than necessary; move each one to the specific async function or helper it applies to, keeping the existing justification.
🤖 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/ws-modules/pic-viewer/src/lib.rs` around lines 12 - 16, Move the
clippy::future_not_send and clippy::single_call_fn expectations from the
crate-level attribute to the specific async functions or wait_for_* helpers that
trigger them. Preserve the existing justification text and ensure each
expectation is scoped only to the item requiring it.
| fn queue_capture_notification(pending: &Rc<RefCell<VecDeque<CaptureNotification>>>, value: &JsValue) { | ||
| let Some(data) = value.as_string() else { | ||
| return; | ||
| }; | ||
| if let Some(notification) = parse_capture_notification(&data) { | ||
| pending.borrow_mut().push_back(notification); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the pending capture queue.
Any connected peer can send captures faster than decoding and drawing completes, so VecDeque grows without
limit and can exhaust the viewer's WASM heap. Cap it and drop or coalesce stale captures; showing the newest
image is the appropriate behavior here.
🤖 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/ws-modules/pic-viewer/src/lib.rs` around lines 181 - 187, Update
queue_capture_notification to bound the pending VecDeque and retain only the
newest capture when the queue reaches its limit. After parsing a valid
notification, discard stale queued captures as needed before enqueueing the new
notification, preserving the existing behavior for invalid values.
| let url = message.get("url").and_then(serde_json::Value::as_str)?; | ||
| if !url.starts_with("/storage/") { | ||
| return None; | ||
| } | ||
| let filename = message | ||
| .get("filename") | ||
| .and_then(serde_json::Value::as_str) | ||
| .unwrap_or("") | ||
| .to_string(); | ||
| Some(CaptureNotification { | ||
| from_agent_id, | ||
| filename, | ||
| url: url.to_string(), | ||
| }) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate a canonical storage path, not a prefix.
/storage/../private.png passes starts_with("/storage/"), but the browser resolves it outside storage when
assigned to image.src. A peer can therefore steer this authenticated browser to arbitrary same-origin GET
routes. Derive the URL from validated path components, or parse and require canonical storage segments.
🤖 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/ws-modules/pic-viewer/src/lib.rs` around lines 207 - 220, Update the
URL validation in the capture-notification parsing flow around
message.get("url") so it accepts only canonical storage paths, not arbitrary
strings with a "/storage/" prefix. Parse and normalize the path, reject
traversal or any non-canonical segments that resolve outside storage, and
construct or retain the URL only after validation so browser navigation remains
confined to storage.
| pub async fn show_image(url: &str) -> Result<(), JsValue> { | ||
| let image = HtmlImageElement::new()?; | ||
| image.set_src(url); | ||
| // decode() resolves once the image is fetched and ready to draw, and rejects on a failed fetch/decode. | ||
| let _decoded = JsFuture::from(image.decode()).await?; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Put a deadline on image decoding.
A stalled storage response leaves image.decode() pending indefinitely. The outer loop cannot observe
stop(), disconnect, or process later captures while this await is blocked. Race loading against a bounded
timer and return an error on timeout.
🤖 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/ws-modules/pic-viewer/src/lib.rs` around lines 227 - 231, Update
show_image so image.decode() races against a bounded timer, returning an error
when the deadline expires instead of awaiting indefinitely. Preserve successful
decoding and propagate fetch/decode failures, allowing the caller to regain
control after timeout.
| filename = await platform.save_eye_capture() | ||
| except Exception as exc: | ||
| platform.log(f"eye capture failed: {exc}") | ||
| platform.send_event(eye_capture_error_event_json(str(exc))) | ||
| return | ||
| platform.send_event(capture_broadcast_json(str(platform.agent_id()), str(filename))) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the workflow test double for the returned filename.
services/ws-modules/pyeye1/tests/test_run_workflow.py:138-139 still returns None; this path
stringifies it and emits a capture broadcast with "filename": "None". Return a fixture filename and assert
the resulting broadcast envelope.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 172-172: Do not catch blind exception: Exception
(BLE001)
🤖 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/ws-modules/pyeye1/pyeye1/eye_detection.py` around lines 171 - 176,
Update the run-workflow test double used by test_run_workflow so
platform.save_eye_capture returns a fixture filename instead of None. Assert
that the successful capture path emits a broadcast envelope containing that
filename, while preserving the existing error-path behavior.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests