Feat/343 screen intelligence e2e tests#359
Merged
graycyrus merged 3 commits intotinyhumansai:mainfrom Apr 6, 2026
Merged
Conversation
…sai#343 Add layered test coverage proving the full capture → vision → memory pipeline: screenshot save/cleanup disk paths, VisionSummary serde roundtrip, JSON-RPC shape tests for status and vision_recent endpoints. - tests/screen_intelligence_vision_e2e.rs: save_screenshot_to_disk creates a PNG and keep_screenshots=false cleanup removes it; VisionSummary struct serializes/persists/is queryable end-to-end; platform support table + macOS checklist added to module doc - tests/json_rpc_e2e.rs: screen_intelligence_status shape test (platform_supported, session.active, permissions.screen_recording); vision_recent returns empty summaries without an active session - src/openhuman/screen_intelligence/tests.rs: save_screenshot_to_disk unit tests for the write path and the no-image-ref error path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…or handling - Added new fields to track vision persistence count, last persisted key, and last persist error in SessionRuntime and SessionStatus. - Implemented error handling for vision summary persistence, ensuring errors are logged and state is updated accordingly. - Introduced a new method to analyze a frame and persist the summary, improving the vision processing pipeline. - Updated tests to validate the new functionality and ensure proper behavior with mocked vision outputs. This commit improves the robustness of the screen intelligence pipeline by enhancing the tracking and handling of vision summary persistence.
Contributor
📝 WalkthroughWalkthroughThis PR extends the screen intelligence engine with persistent vision analysis capabilities. It introduces synchronous vision frame analysis and memory persistence, adds state tracking for vision persistence metrics, refines session start permission checks, and expands test coverage with unit and E2E tests for the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Test
participant Engine as AccessibilityEngine
participant Vision as Vision Analyzer
participant Memory as UnifiedMemory
participant Disk as Disk I/O
Client->>Engine: analyze_and_persist_frame(frame)
activate Engine
Engine->>Engine: validate config<br/>(local_ai enabled,<br/>provider = ollama)
alt Config Invalid
Engine-->>Client: Err(message)
else Config Valid
rect rgba(100, 150, 200, 0.5)
Note over Engine,Vision: Analysis Phase
Engine->>Vision: analyze_frame_with_vision(frame)
activate Vision
Vision-->>Engine: VisionSummary
deactivate Vision
end
alt Analysis Error
Engine->>Engine: update state.last_error
Engine-->>Client: Err(analysis_error)
else Analysis Success
rect rgba(150, 100, 200, 0.5)
Note over Engine,Memory: Persistence Phase
Engine->>Memory: persist_vision_summary(summary)
activate Memory
Memory->>Disk: write document
activate Disk
Disk-->>Memory: ok/err
deactivate Disk
Memory-->>Engine: Result<key, error>
deactivate Memory
end
alt Persist Error
Engine->>Engine: update state.last_error
Engine->>Engine: vision_persist_count++
Engine-->>Client: Err(persist_error)
else Persist Success
Engine->>Engine: vision_persist_count++
Engine->>Engine: last_vision_persisted_key = key
Engine-->>Client: Ok(VisionSummary)
end
end
end
deactivate Engine
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Brings in Discord server/channel picker (tinyhumansai#349) and autocomplete observability improvements (tinyhumansai#308) from main. Resolves conflict in tests/json_rpc_e2e.rs by keeping both the screen intelligence tests (this branch) and the autocomplete runtime settings test (main). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/screen_intelligence/tests.rs (1)
643-646: Remove redundant imports - already imported at file scope.These imports duplicate lines 6-9 at the top of the file.
♻️ Proposed cleanup
#[test] fn save_screenshot_to_disk_writes_png_to_workspace() { - use base64::{engine::general_purpose::STANDARD as B64, Engine}; - use image::codecs::png::PngEncoder; - use image::{ImageBuffer, Rgb, RgbImage}; - use tempfile::tempdir; - let tmp = tempdir().expect("tempdir");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/screen_intelligence/tests.rs` around lines 643 - 646, Duplicate imports of B64/Engine, PngEncoder, ImageBuffer/Rgb/RgbImage, and tempdir are present inside the test scope and already imported at file scope; remove the redundant inner use statements (those containing base64::{engine::general_purpose::STANDARD as B64, Engine}, image::codecs::png::PngEncoder, image::{ImageBuffer, Rgb, RgbImage}, and tempfile::tempdir) so the test reuses the top-level imports instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/json_rpc_e2e.rs`:
- Around line 916-919: The expect message on the chain calling
recent_result.get("summaries").and_then(Value::as_array).expect(...) is a plain
string containing `{recent_result}` that won't interpolate; change it to use a
formatted panic message (e.g., use format! or panic! to include recent_result)
so the actual recent_result value is shown on failure—update the expect argument
for the summaries extraction to something like expect(&format!("expected
summaries array: {:?}", recent_result)).
---
Nitpick comments:
In `@src/openhuman/screen_intelligence/tests.rs`:
- Around line 643-646: Duplicate imports of B64/Engine, PngEncoder,
ImageBuffer/Rgb/RgbImage, and tempdir are present inside the test scope and
already imported at file scope; remove the redundant inner use statements (those
containing base64::{engine::general_purpose::STANDARD as B64, Engine},
image::codecs::png::PngEncoder, image::{ImageBuffer, Rgb, RgbImage}, and
tempfile::tempdir) so the test reuses the top-level imports instead.
🪄 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: 80a9968a-d314-4505-af57-447d30ccc263
📒 Files selected for processing (6)
src/openhuman/screen_intelligence/engine.rssrc/openhuman/screen_intelligence/helpers.rssrc/openhuman/screen_intelligence/tests.rssrc/openhuman/screen_intelligence/types.rstests/json_rpc_e2e.rstests/screen_intelligence_vision_e2e.rs
Comment on lines
+916
to
+919
| let summaries = recent_result | ||
| .get("summaries") | ||
| .and_then(Value::as_array) | ||
| .expect("expected summaries array: {recent_result}"); |
Contributor
There was a problem hiding this comment.
Fix expect! panic message - variable not interpolated in string literal.
The expect call uses a string literal that won't interpolate {recent_result}. Use a formatted panic message instead.
🐛 Proposed fix
let summaries = recent_result
.get("summaries")
.and_then(Value::as_array)
- .expect("expected summaries array: {recent_result}");
+ .unwrap_or_else(|| panic!("expected summaries array: {recent_result}"));📝 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
| let summaries = recent_result | |
| .get("summaries") | |
| .and_then(Value::as_array) | |
| .expect("expected summaries array: {recent_result}"); | |
| let summaries = recent_result | |
| .get("summaries") | |
| .and_then(Value::as_array) | |
| .unwrap_or_else(|| panic!("expected summaries array: {}", recent_result)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/json_rpc_e2e.rs` around lines 916 - 919, The expect message on the
chain calling
recent_result.get("summaries").and_then(Value::as_array).expect(...) is a plain
string containing `{recent_result}` that won't interpolate; change it to use a
formatted panic message (e.g., use format! or panic! to include recent_result)
so the actual recent_result value is shown on failure—update the expect argument
for the summaries extraction to something like expect(&format!("expected
summaries array: {:?}", recent_result)).
senamakel
approved these changes
Apr 6, 2026
4 tasks
This was referenced Apr 8, 2026
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
3 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.
Summary
parse_vision_summary_output) →persist_vision_summaryinto unified memory.[screen_intelligence]logs and visible failure modes in tests.OPENHUMAN_WORKSPACEisolation).Problem
src/openhuman/screen_intelligence/(capture workers, vision worker,persist_vision_summary), but operators cannot rely on the full chain: permissions, scheduling, model wiring, and memory writes may fail silently or go untested.Solution
persist_vision_summary(unified memory namespace).OPENHUMAN_WORKSPACEisolation; optional manual macOS checklist for real Screen Recording permission where automation cannot grant it in CI.keep_screenshotsand related config.Key files (reference):
src/openhuman/screen_intelligence/engine.rssrc/openhuman/screen_intelligence/helpers.rs(persist_vision_summary)src/openhuman/screen_intelligence/tests.rswith E2E-style coverageSubmission Checklist
app/) and/orcargo test(core) for logic you add or changeapp/test/e2e, mock backend,tests/json_rpc_e2e.rsas appropriate)//////!(Rust), JSDoc or brief file/module headers (TS) on public APIs and non-obvious modulesFeature-specific (issue #343):
Impact
keep_screenshots, etc.); logs must not contain secrets.Related
Branch:
feat/343-screen-intelligence-e2e-testsSummary by CodeRabbit
New Features
Bug Fixes
Tests