Skip to content

Conversation

@gmegidish
Copy link
Member

@gmegidish gmegidish commented Nov 27, 2025

Summary by CodeRabbit

  • New Features

    • Added a --format flag to the dump UI command to select output format (parsed or raw).
    • UI dump supports a raw output mode so devices can return unparsed source data for inspection.
  • Bug Fixes / Improvements

    • Improved error reporting consistency when performing UI dumps.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a --format flag to the dump UI CLI and routes requests to either a new raw-source path or the existing parsed-elements path. Devices and WDA client gain DumpSourceRaw/GetSourceRaw methods; command and CLI layers pass and return the new format and raw data, with improved error wrapping.

Changes

Cohort / File(s) Summary
CLI flag binding
cli/dump.go
Adds package-level dumpUIFormat and a --format flag for the dump ui subcommand; forwards format into DumpUIRequest.
Command layer routing
commands/dump.go
Adds Format string to DumpUIRequest and RawData interface{} to DumpUIResponse; makes Elements json:"elements,omitempty"; conditional branch: Format=="raw"DumpSourceRaw() populates RawData, else DumpSource() populates Elements; error wrapping updated to use %w.
Device interface
devices/common.go
Adds DumpSourceRaw() (interface{}, error) to ControllableDevice interface.
Device implementations
devices/android.go, devices/ios.go, devices/simulator.go
Implement DumpSourceRaw() for Android, iOS, and Simulator; Android’s DumpSource() now delegates to DumpSourceRaw() and parses the returned raw string.
WDA client
devices/wda/source.go
Adds GetSourceRaw() (interface{}, error) to fetch raw WDA /source output; GetSourceElements() now uses GetSourceRaw(); improved error wrapping (%w) and adjusted logging location.
sequenceDiagram
    participant User
    participant CLI as CLI (cli/dump.go)
    participant Cmd as Command (commands/dump.go)
    participant Device as Device (android/ios/simulator)
    participant WDA as WDA Client (devices/wda/source.go)

    User->>CLI: dump ui --format raw
    CLI->>CLI: parse flags
    CLI->>Cmd: DumpUI(DumpUIRequest{Format:"raw"})
    alt Format == "raw"
        Cmd->>Device: DumpSourceRaw()
        Device->>WDA: GetSourceRaw()
        WDA-->>Device: raw interface{}
        Device-->>Cmd: raw interface{}
        Cmd->>Cmd: set RawData
    else Format != "raw"
        Cmd->>Device: DumpSource()
        Device->>Device: parse to ScreenElements
        Device-->>Cmd: []ScreenElement
        Cmd->>Cmd: set Elements
    end
    Cmd-->>CLI: DumpUIResponse
    CLI-->>User: output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review commands/dump.go conditional paths and error messages for consistent wrapping and return shapes.
  • Verify ControllableDevice interface change and all implementations (android.go, ios.go, simulator.go) compile and correctly delegate.
  • Inspect devices/wda/source.go new GetSourceRaw() and downstream marshaling/unmarshaling/error wrapping in GetSourceElements().
  • Confirm JSON serialization implications of Elements omitempty and addition of RawData in DumpUIResponse.

Poem

🐰 A little flag hops through the code,
Raw streams tumble down the node.
CLI to device, WDA hums along,
Elements parsed or raw and strong.
Hop, parse, return — a rabbit’s song! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: support raw view tree ui dumps' accurately summarizes the main change: adding support for raw UI/view tree dump functionality across multiple device types and CLI interfaces.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e2203be and a343c4a.

📒 Files selected for processing (1)
  • devices/wda/source.go (1 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
cli/dump.go (1)

46-46: Clarify the default format behavior in help text.

The help text states "Default: json" but when the format is empty, the code returns parsed Elements (not a "json" format per se). Consider updating the description to clarify the actual behavior:

-	dumpUICmd.Flags().StringVar(&dumpUIFormat, "format", "", "Output format: 'raw' for unprocessed tree from agent (Default: json)")
+	dumpUICmd.Flags().StringVar(&dumpUIFormat, "format", "", "Output format: 'raw' for unprocessed tree from agent (default: parsed elements)")
devices/wda/source.go (1)

113-113: Consider log verbosity for large UI trees.

The verbose logging of the entire WDA source response could produce very large output for complex UIs. If this becomes problematic, consider truncating or summarizing the logged data.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 00b5bfb and e2203be.

📒 Files selected for processing (7)
  • cli/dump.go (2 hunks)
  • commands/dump.go (1 hunks)
  • devices/android.go (2 hunks)
  • devices/common.go (1 hunks)
  • devices/ios.go (1 hunks)
  • devices/simulator.go (1 hunks)
  • devices/wda/source.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
devices/common.go (2)
devices/android_display_test.go (1)
  • name (41-45)
commands/screenshot.go (1)
  • DeviceID (16-21)
devices/android.go (2)
devices/common.go (1)
  • ScreenElement (38-38)
types/screen.go (1)
  • ScreenElement (10-19)
commands/dump.go (3)
devices/common.go (2)
  • ScreenElement (38-38)
  • StartAgentConfig (31-33)
types/screen.go (1)
  • ScreenElement (10-19)
commands/commands.go (3)
  • CommandResponse (11-15)
  • FindDeviceOrAutoSelect (64-97)
  • NewErrorResponse (26-31)
cli/dump.go (1)
commands/dump.go (1)
  • DumpUIRequest (9-12)
devices/wda/source.go (4)
devices/wda/types.go (1)
  • WdaClient (9-13)
devices/common.go (1)
  • ScreenElement (38-38)
types/screen.go (1)
  • ScreenElement (10-19)
utils/logger.go (1)
  • Verbose (19-23)
⏰ 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). (2)
  • GitHub Check: lint
  • GitHub Check: Analyze (go)
🔇 Additional comments (12)
devices/simulator.go (1)

775-777: LGTM!

The DumpSourceRaw() implementation correctly delegates to the WDA client and follows the same pattern as the existing DumpSource() method.

devices/ios.go (1)

665-667: LGTM!

The DumpSourceRaw() implementation is consistent with the simulator device and correctly delegates to the WDA client.

devices/common.go (1)

68-68: LGTM!

The interface extension properly adds DumpSourceRaw() to support raw format output. Using interface{} as the return type is appropriate since Android returns XML strings while iOS/Simulator returns JSON structures.

devices/android.go (2)

957-961: Good improvement on error wrapping.

Using %w for error wrapping enables proper error unwrapping with errors.Is() and errors.As().


983-1015: LGTM!

The refactoring cleanly separates raw data retrieval (DumpSourceRaw) from parsing (DumpSource). The type assertion with a clear error message handles the interface{} to string conversion appropriately.

devices/wda/source.go (3)

84-84: Good improvement on error wrapping.

Using %w enables proper error chain inspection.


90-105: LGTM!

The GetSourceRaw() method correctly extracts the raw value from the WDA response and handles missing "value" field appropriately.


107-128: LGTM!

The refactoring to use GetSourceRaw() is clean, and the improved error wrapping with %w is consistent throughout.

commands/dump.go (4)

11-11: LGTM!

The Format field addition enables the raw dump functionality.


16-17: Good use of omitempty.

The omitempty tags on both Elements and RawData ensure the JSON response only includes the relevant field based on the requested format.


25-31: Good error handling improvements.

Error wrapping with %w and including device ID in error messages improves debuggability.


34-56: LGTM!

The conditional logic cleanly separates raw and parsed dump paths. The implementation correctly handles the format selection.

@gmegidish gmegidish merged commit 3ded3bd into main Nov 29, 2025
7 of 8 checks passed
@gmegidish gmegidish deleted the feat-support-raw-ui-dumps branch November 29, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants