Update pydemo1#104
Conversation
📝 WalkthroughWalkthroughThe PyDemo workflow now launches directly, uses redesigned timer and exit controls, broadcasts stored eye captures through websocket messages, exports and tests the new Python helper, and selects ChangesPyDemo workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ModuleSelector
participant run
participant enterDemo
participant PythonWorkflow
ModuleSelector->>run: select et-ws-pydemo1 and start
run->>enterDemo: create demo state
enterDemo->>PythonWorkflow: load runtime and execute workflow
sequenceDiagram
participant saveEyeCapture
participant eye_capture_stored_json
participant capture_broadcast_json
participant WebSocket
saveEyeCapture->>eye_capture_stored_json: pass agent_id and filename
eye_capture_stored_json->>capture_broadcast_json: build stored-capture message
eye_capture_stored_json-->>saveEyeCapture: return JSON
saveEyeCapture->>WebSocket: send broadcast JSON
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene Coverage |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Jul 23, 2026 2:09a.m. | Review ↗ |
| C & C++ | |
Jul 23, 2026 2:09a.m. | Review ↗ |
| Docker | |
Jul 23, 2026 2:09a.m. | Review ↗ |
| Java | |
Jul 23, 2026 2:09a.m. | Review ↗ |
| JavaScript | |
Jul 23, 2026 2:09a.m. | Review ↗ |
| Python | |
Jul 23, 2026 2:09a.m. | Review ↗ |
| Rust | |
Jul 23, 2026 2:09a.m. | Review ↗ |
| Secrets | |
Jul 23, 2026 2:09a.m. | Review ↗ |
| Code coverage | |
Jul 23, 2026 2:35a.m. | Review ↗ |
Code Coverage Summary
| Language | Line Coverage (New Code) | Line Coverage (Overall) |
|---|---|---|
| Aggregate | 100% |
57.9% [▲ up 0.1% from main] |
| Python | 100% |
85.3% [▲ up 0.1% from main] |
| Rust | - | 55.7% [▲ up 0.1% 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 |
|---|---|
| ErrorProne | 2 high |
🟢 Metrics -22 complexity · 0 duplication
Metric Results Complexity -22 Duplication 0
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✅ All modified and coverable lines are covered by tests. 📢 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/ws-modules/pydemo1/pkg/et_ws_pydemo1.js (2)
637-658: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBroadcast failure after a successful upload gets misreported as "eye capture failed".
If
state.client?.send?.(...)throws after the storagefetchalready succeeded, the sharedcatchlogseye capture failedand emitseye_capture_error_json, even though the image was stored correctly.🔧 Proposed fix
state.client?.send?.(py.eye_capture_stored_json(agentId, filename)); - log(`eye capture saved to storage and broadcast: ${filename} (${bytes.length} bytes)`); + log(`eye capture saved to storage: ${filename} (${bytes.length} bytes)`); + try { + state.client?.send?.(py.eye_capture_stored_json(agentId, filename)); + } catch (broadcastError) { + log(`eye capture broadcast failed (capture already stored): ${String(broadcastError)}`); + }🤖 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/pydemo1/pkg/et_ws_pydemo1.js` around lines 637 - 658, Update saveEyeCapture so failures from state.client?.send?.(...) after a successful storage upload are handled separately from capture and upload failures. Preserve the success log and avoid emitting eye_capture_error_json for broadcast-only errors; only report capture or storage failures as eye capture failures.
37-48: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRace:
stop()clearsruntimebefore the async back()-navigation cleanup completes.
window.history.back()firespopstateasynchronously, butstop()unconditionally setsruntime = nullright after calling it — before the pendingexitDemo(state)(triggered by thatpopstate) actually restores the DOM. Ifrun()is invoked again in that window,if (runtime) return;no longer guards it (runtime is already null), so a new demo session is created while the oldexitDemocleanup is still pending. When that staleexitDemoeventually fires, it replacesdocument.bodywith the old demo'spreviousBodyNodessnapshot, silently wiping out the brand-new demo overlay that was just built.🔧 Proposed fix
export function stop() { if (!runtime) return; const state = runtime; state.stopped = true; if (state.activeDemo && window.history.state?.pydemo1) { window.history.back(); + // exitDemo() runs asynchronously via the pending popstate and clears `runtime` itself. } else { exitDemo(state); + runtime = null; } - runtime = null; setMainStatus("pydemo1 stopped."); }Also applies to: 23-35, 1069-1071
🤖 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/pydemo1/pkg/et_ws_pydemo1.js` around lines 37 - 48, Update stop() so runtime remains set while asynchronous history.back() cleanup is pending, preventing run() from starting a new session before exitDemo(state) completes. Clear runtime only after the popstate-triggered cleanup finishes, while preserving immediate cleanup for the non-navigation path; ensure stale exitDemo cannot overwrite a newly created demo.
🤖 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.
Outside diff comments:
In `@services/ws-modules/pydemo1/pkg/et_ws_pydemo1.js`:
- Around line 637-658: Update saveEyeCapture so failures from
state.client?.send?.(...) after a successful storage upload are handled
separately from capture and upload failures. Preserve the success log and avoid
emitting eye_capture_error_json for broadcast-only errors; only report capture
or storage failures as eye capture failures.
- Around line 37-48: Update stop() so runtime remains set while asynchronous
history.back() cleanup is pending, preventing run() from starting a new session
before exitDemo(state) completes. Clear runtime only after the
popstate-triggered cleanup finishes, while preserving immediate cleanup for the
non-navigation path; ensure stale exitDemo cannot overwrite a newly created
demo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cfda6b99-4303-4278-a941-4628936e51a9
📒 Files selected for processing (5)
services/ws-modules/pydemo1/pkg/et_ws_pydemo1.jsservices/ws-modules/pydemo1/pydemo1/__init__.pyservices/ws-modules/pydemo1/pydemo1/demo.pyservices/ws-modules/pydemo1/tests/test_demo.pyservices/ws-server/static/app.js
2 participants
Summary by CodeRabbit
New Features
Bug Fixes