feat: widget inspector queries, timeline aliases, frame analysis fixes - #8
Conversation
- WidgetInspectorQueryService for generic inspector queries - Per-frame shader jank tracking - navigation_stack replaced with widget_inspector_query export - Various docs, formatting, and test improvements
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR expands Flutter VM-service profiling with attach discovery, timeline commands, Flutter attach/drive support, widget-inspector queries, persisted memory snapshots, improved widget-tree filtering, MCP tools, tests, and updated documentation. ChangesFlutter profiling workflows
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant VMServiceDiscovery
participant WidgetInspectorQueryService
participant FlutterApp
CLI->>VMServiceDiscovery: Resolve VM-service URI
VMServiceDiscovery-->>CLI: Return active app URI
CLI->>WidgetInspectorQueryService: Query inspector method
WidgetInspectorQueryService->>FlutterApp: Call inspector service extension
FlutterApp-->>WidgetInspectorQueryService: Return JSON payload
WidgetInspectorQueryService-->>CLI: Return decoded query result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart (1)
55-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract WebSocket URI normalization into a shared helper.
The same 4-line URI normalization (
http→ws,https→wss, append/ws) is repeated inrun()of_FrameTimingCommand(lines 55–58),MemorySnapshotCommand(lines 244–247),WidgetTreeCommand(lines 481–484),InspectorQueryCommand(lines 611–614),ScreenshotCommand(lines 708–711), andDebugDumpCommand(lines 780–783). Adding anormalizeWsUri(String uri)method to theVmServiceDiscoverymixin would centralize this and prevent drift.♻️ Proposed refactor
Add to
VmServiceDiscoverymixin:String normalizeWsUri(String uri) { final wsUri = uri .replaceFirst('http://', 'ws://') .replaceFirst('https://', 'wss://'); return wsUri.endsWith('/ws') ? wsUri : '$wsUri/ws'; }Then replace each 4-line block with
final cleanWs = normalizeWsUri(vmServiceUri);.🤖 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 `@packages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart` around lines 55 - 58, Add normalizeWsUri(String uri) to the VmServiceDiscovery mixin, preserving the existing HTTP/HTTPS-to-WebSocket conversion and /ws suffix handling. Replace the duplicated URI normalization blocks in _FrameTimingCommand.run, MemorySnapshotCommand, WidgetTreeCommand, InspectorQueryCommand, ScreenshotCommand, and DebugDumpCommand with calls to normalizeWsUri(vmServiceUri).
🤖 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 `@packages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dart`:
- Around line 199-206: The attach CLI’s skip-dtd flag is permanently true and
cannot opt into DTD, unlike the MCP path. Update the skip-dtd option in the
attach command configuration to be negatable so --no-skip-dtd is accepted and
enableDtd can become true, keeping the CLI behavior aligned with MCP.
In `@packages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart`:
- Around line 649-662: Move the duplicated _resolveMainIsolate logic from
InspectorQueryCommand, ScreenshotCommand, and DebugDumpCommand into the
VmServiceDiscovery mixin as a shared resolveMainIsolate(VM vm) method,
preserving its isolate filtering and error handling. Remove each private copy
and update all call sites to invoke resolveMainIsolate(vm).
In `@packages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dart`:
- Around line 966-977: Document the private _toInspectorRpc helper, describing
its VM-service extension-name mapping and supported get/set/profile/debug
prefixes. Update the mapping to explicitly handle all supported inspector
methods, including structuredErrors and isWidgetTreeReady, and reject
unsupported names with a clear error instead of returning an invalid RPC string.
In `@packages/devtools_profiler_cli/lib/src/mcp/tools/flutter_tools.dart`:
- Around line 61-134: Extract the duplicated timeline tool configuration from
profileTimelineTool and timelineTool into a shared factory function,
parameterized by name and title, while preserving their descriptions, schemas,
and annotations. Consider extending the factory to profileFrameProfileTool so
all tools sharing this input/output contract remain aligned, then instantiate
each tool through the factory.
In `@packages/devtools_profiler_core/lib/src/capture/profile_runner.dart`:
- Around line 279-281: Wrap the warning message passed to
sessionController.addWarning in adjacent string literals, following the
formatting used for similar messages such as the profileAttach progress message,
while preserving the exact text and keeping each line near 80 characters.
In `@packages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dart`:
- Around line 64-74: Add a concise documentation comment immediately above the
private `_decodeResult` helper explaining that VM-service inspector responses
may be JSON-encoded strings or already-decoded raw values, and that the fallback
preserves raw values when decoding is unnecessary or fails.
- Around line 39-48: Expand the documentation for WidgetInspectorQuery.query to
describe the isolateId, method, and args parameters, the
WidgetInspectorQueryResult it returns, and that it throws StateError when the
isolate does not expose the requested method; use prose rather than tag-style
documentation.
In `@packages/devtools_profiler_core/test/flutter_widget_tree_test.dart`:
- Around line 135-151: Add a brief /// doc comment immediately above the private
_FakeWidgetTreeVmService class explaining that it fakes the VM service for the
root widget tree extension and validates the expected profiler RPC method; leave
the existing callServiceExtension assertion intact.
In `@skills/devtools-profiler-local/SKILL.md`:
- Around line 22-26: Update the command guidance to replace the unrecognized
top-level `frame-profile` reference with the documented `flutter:frame-profile`
command, while retaining `timeline` as the top-level alias; adjust the affected
guidance near the Flutter app workflow.
- Around line 273-274: Replace the stale flutter:route-stack workflow in the
navigation tree guidance with the new flutter:inspector query, updating the
surrounding usage text so it accurately documents the replacement
navigation-inspection surface.
---
Outside diff comments:
In `@packages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart`:
- Around line 55-58: Add normalizeWsUri(String uri) to the VmServiceDiscovery
mixin, preserving the existing HTTP/HTTPS-to-WebSocket conversion and /ws suffix
handling. Replace the duplicated URI normalization blocks in
_FrameTimingCommand.run, MemorySnapshotCommand, WidgetTreeCommand,
InspectorQueryCommand, ScreenshotCommand, and DebugDumpCommand with calls to
normalizeWsUri(vmServiceUri).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8fd8ef97-8581-4694-a6c6-8cd1937b73af
📒 Files selected for processing (37)
README.mdpackages/devtools_profiler_cli/CHANGELOG.mdpackages/devtools_profiler_cli/README.mdpackages/devtools_profiler_cli/lib/src/cli.dartpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/lib/src/cli/commands/discover_command.dartpackages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dartpackages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dartpackages/devtools_profiler_cli/lib/src/cli/options.dartpackages/devtools_profiler_cli/lib/src/mcp/server.dartpackages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dartpackages/devtools_profiler_cli/lib/src/mcp/tools/capture_tools.dartpackages/devtools_profiler_cli/lib/src/mcp/tools/flutter_tools.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_core/.pubignorepackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/README.mdpackages/devtools_profiler_core/lib/devtools_profiler_core.dartpackages/devtools_profiler_core/lib/src/capture/profile_attach_request.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dartpackages/devtools_profiler_core/lib/src/flutter/debug_dump.dartpackages/devtools_profiler_core/lib/src/flutter/frame_analysis.dartpackages/devtools_profiler_core/lib/src/flutter/navigation_stack.dartpackages/devtools_profiler_core/lib/src/flutter/screenshot.dartpackages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dartpackages/devtools_profiler_core/lib/src/flutter/widget_tree.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/animation_screen.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/home_screen.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/list_scroll_screen.dartpackages/devtools_profiler_core/test/flutter_widget_tree_test.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/widget_inspector_query_test.dartpackages/devtools_region_profiler/README.mdskills/devtools-profiler-local/SKILL.md
💤 Files with no reviewable changes (2)
- packages/devtools_profiler_core/lib/src/flutter/navigation_stack.dart
- packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: CI / Format and Analyze: feat: widget inspector queries, timeline aliases, frame analysis fixes
Conclusion: failure
##[group]Run dart analyze .
�[36;1mdart analyze .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.11.4/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Analyzing ....
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:1:8 - Target of URI doesn't exist: 'package:flutter/material.dart'. Try creating the file referenced by the URI, or try using a URI for a file that does exist. - uri_does_not_exist
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:10:3 - Undefined name 'WidgetsFlutterBinding'. Try correcting the name to one that is defined, or defining the name. - undefined_identifier
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:11:3 - The function 'runApp' isn't defined. Try importing the library that defines 'runApp', correcting the name to the name of an existing function, or defining a function named 'runApp'. - undefined_function
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:14:31 - Classes can only extend other classes. Try specifying a different superclass, or removing the extends clause. - extends_non_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:15:32 - No associated named super constructor parameter. Try changing the name to the name of an existing named super constructor parameter, or creating such named parameter. - super_formal_parameter_without_associated_named
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:3 - Undefined class 'Widget'. Try changing the name to the name of an existing class, or creating a class with the name 'Widget'. - undefined_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:16 - Undefined class 'BuildContext'. Try changing the name to the name of an existing class, or ...
GitHub Actions: CI / 1_Format and Analyze.txt: feat: widget inspector queries, timeline aliases, frame analysis fixes
Conclusion: failure
##[group]Run dart analyze .
�[36;1mdart analyze .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.11.4/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Analyzing ....
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:1:8 - Target of URI doesn't exist: 'package:flutter/material.dart'. Try creating the file referenced by the URI, or try using a URI for a file that does exist. - uri_does_not_exist
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:10:3 - Undefined name 'WidgetsFlutterBinding'. Try correcting the name to one that is defined, or defining the name. - undefined_identifier
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:11:3 - The function 'runApp' isn't defined. Try importing the library that defines 'runApp', correcting the name to the name of an existing function, or defining a function named 'runApp'. - undefined_function
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:14:31 - Classes can only extend other classes. Try specifying a different superclass, or removing the extends clause. - extends_non_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:15:32 - No associated named super constructor parameter. Try changing the name to the name of an existing named super constructor parameter, or creating such named parameter. - super_formal_parameter_without_associated_named
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:3 - Undefined class 'Widget'. Try changing the name to the name of an existing class, or creating a class with the name 'Widget'. - undefined_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:16 - Undefined class 'BuildContext'. Try changing the name to the name of an existing class, or ...
🧰 Additional context used
📓 Path-based instructions (4)
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Follow idiomatic Dart and keep code easy to scan in split-screen views
Prefer multi-line strings over string concatenation for large text blocks, command output fixtures, JSON examples, and terminal snapshots
Keep lines near 80 characters where practical. Long identifiers and URLs may exceed that when wrapping would make the code harder to read
Use records for short-lived grouped return values instead of introducing one-off classes
Use patterns,if-case, and switch expressions when they make parsing or dispatch logic clearer
Use class modifiers such assealed,final,base, andinterfacewhen they describe the intended inheritance boundary
Use digit separators for large numeric literals, for example timeouts, sample counts, and byte sizes
Use wildcard variables for intentionally unused callback parameters
Use null-aware collection elements when conditionally including nullable values in list or map literals
Use dot shorthands only when the inferred type is obvious from context
Use///documentation comments for public APIs
Start doc comments with a short, single-sentence summary
Put a blank line after the first sentence when adding more detail in doc comments
Avoid repeating information that is already obvious from the declaration in documentation comments
Start method comments with third-person verbs, such as 'Returns', 'Starts', or 'Captures'
Start non-boolean property comments with a noun phrase
Start boolean property comments with 'Whether'
Use square brackets for in-scope identifiers, such as [ProfileRunRequest], [Duration], and [StateError]
Explain parameters, return values, and exceptions in prose rather than using tag-style documentation
Prefer fenced Markdown code blocks for examples in documentation comments
Keep Markdown simple; avoid HTML in documentation comments
Files:
packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/home_screen.dartpackages/devtools_profiler_core/lib/devtools_profiler_core.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/list_scroll_screen.dartpackages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dartpackages/devtools_profiler_cli/lib/src/cli/commands/discover_command.dartpackages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dartpackages/devtools_profiler_core/test/widget_inspector_query_test.dartpackages/devtools_profiler_core/lib/src/flutter/screenshot.dartpackages/devtools_profiler_core/lib/src/capture/profile_attach_request.dartpackages/devtools_profiler_cli/lib/src/mcp/tools/capture_tools.dartpackages/devtools_profiler_cli/lib/src/mcp/server.dartpackages/devtools_profiler_cli/lib/src/cli/options.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/animation_screen.dartpackages/devtools_profiler_cli/lib/src/cli.dartpackages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/flutter_widget_tree_test.dartpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_core/lib/src/flutter/debug_dump.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_cli/lib/src/mcp/tools/flutter_tools.dartpackages/devtools_profiler_core/lib/src/flutter/widget_tree.dartpackages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dartpackages/devtools_profiler_core/lib/src/flutter/frame_analysis.dartpackages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dartpackages/devtools_profiler_cli/test/cli_test.dart
packages/devtools_profiler_*/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Consider documenting private helpers when they encode profiler behavior, artifact contracts, protocol semantics, or VM-service assumptions
Files:
packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/home_screen.dartpackages/devtools_profiler_core/lib/devtools_profiler_core.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/list_scroll_screen.dartpackages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dartpackages/devtools_profiler_cli/lib/src/cli/commands/discover_command.dartpackages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dartpackages/devtools_profiler_core/test/widget_inspector_query_test.dartpackages/devtools_profiler_core/lib/src/flutter/screenshot.dartpackages/devtools_profiler_core/lib/src/capture/profile_attach_request.dartpackages/devtools_profiler_cli/lib/src/mcp/tools/capture_tools.dartpackages/devtools_profiler_cli/lib/src/mcp/server.dartpackages/devtools_profiler_cli/lib/src/cli/options.dartpackages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/animation_screen.dartpackages/devtools_profiler_cli/lib/src/cli.dartpackages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/flutter_widget_tree_test.dartpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_core/lib/src/flutter/debug_dump.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_cli/lib/src/mcp/tools/flutter_tools.dartpackages/devtools_profiler_core/lib/src/flutter/widget_tree.dartpackages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dartpackages/devtools_profiler_core/lib/src/flutter/frame_analysis.dartpackages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dartpackages/devtools_profiler_cli/test/cli_test.dart
packages/*/README.md
📄 CodeRabbit inference engine (AGENTS.md)
Package READMEs should explain how that package is used and how it fits into the profiler system. Prefer examples that agents can execute directly from the CLI
Files:
packages/devtools_region_profiler/README.mdpackages/devtools_profiler_core/README.mdpackages/devtools_profiler_cli/README.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep the root
README.mdend-user focused
Files:
README.md
🪛 GitHub Actions: CI / 1_Format and Analyze.txt
packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/home_screen.dart
[error] 1-1: Target of URI doesn't exist: 'package:flutter/material.dart'. (uri_does_not_exist)
[error] 7-7: Undefined class 'Widget'. (undefined_class)
[error] 7-7: Undefined class 'BuildContext'. (undefined_class)
[error] 10-10: The method 'Scaffold' isn't defined for the type 'HomeScreen'. (undefined_method)
[warning] 7-7: The method doesn't override an inherited method. (override_on_non_overriding_member)
packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/list_scroll_screen.dart
[error] 1-1: Target of URI doesn't exist: 'package:flutter/material.dart'. (uri_does_not_exist)
[error] 18-18: Undefined name 'Colors'. (undefined_identifier)
[error] 31-31: Undefined name 'ScaffoldMessenger'. (undefined_identifier)
[warning] 7-7: The method doesn't override an inherited method. (override_on_non_overriding_member)
packages/devtools_profiler_core/test/widget_inspector_query_test.dart
[warning] 1-1: Unused import: 'dart:io'. (unused_import)
packages/devtools_profiler_core/lib/src/flutter/screenshot.dart
[warning] 1-1: Unused import: 'dart:io'. (unused_import)
packages/devtools_profiler_core/lib/src/capture/profile_attach_request.dart
[warning] 1-1: Unused import: 'dart:io'. (unused_import)
packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/animation_screen.dart
[error] 1-1: Target of URI doesn't exist: 'package:flutter/material.dart'. (uri_does_not_exist)
[error] 8-8: Undefined class 'State'. (undefined_class)
[error] 11-11: Mixin can only be applied to class. (mixin_with_non_class_superclass)
[error] 12-12: Classes can only mix in mixins and classes. (mixin_of_non_class)
[error] 13-13: Undefined class 'AnimationController'. (undefined_class)
[error] 28-28: Undefined name 'Curves'. (undefined_identifier)
[warning] 8-8: The method doesn't override an inherited method. (override_on_non_overriding_member)
[warning] 36-36: The method doesn't override an inherited method. (override_on_non_overriding_member)
packages/devtools_profiler_cli/test/mcp_server_test.dart
[warning] 1-1: Unused import: 'dart:io'. (unused_import)
packages/devtools_profiler_core/test/profile_runner_test.dart
[warning] 1-1: Unused import: 'dart:io'. (unused_import)
packages/devtools_profiler_core/test/flutter_widget_tree_test.dart
[warning] 1-1: Unused import: 'dart:io'. (unused_import)
packages/devtools_profiler_cli/test/cli_test.dart
[warning] 1-1: Unused import: 'dart:io'. (unused_import)
🪛 GitHub Actions: CI / Format and Analyze
packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/home_screen.dart
[error] 1-1: dart analyze (uri_does_not_exist): Target of URI doesn't exist: 'package:flutter/material.dart'.
packages/devtools_profiler_core/test/widget_inspector_query_test.dart
[warning] 1-1: dart analyze (unused_import): Unused import: 'dart:io'. Try removing the import directive.
packages/devtools_profiler_core/lib/src/flutter/screenshot.dart
[warning] 1-1: dart analyze (unused_import): Unused import: 'dart:io'. Try removing the import directive.
packages/devtools_profiler_core/lib/src/capture/profile_attach_request.dart
[warning] 1-1: dart analyze (unused_import): Unused import: 'dart:io'. Try removing the import directive.
packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/animation_screen.dart
[error] 1-1: dart analyze (uri_does_not_exist): Target of URI doesn't exist: 'package:flutter/material.dart'.
[error] 4-5: dart analyze (extends_non_class / super_formal_parameter_without_associated_named): Invalid class inheritance/super constructor due to missing Flutter base classes.
[error] 11-15: dart analyze: Flutter/animation types used but undefined (e.g., 'State', 'AnimationController', 'Animation').
[warning] 8-8: dart analyze (override_on_non_overriding_member): The method doesn't override an inherited method.
packages/devtools_profiler_cli/test/mcp_server_test.dart
[warning] 1-1: dart analyze (unused_import): Unused import: 'dart:io'. Try removing the import directive.
packages/devtools_profiler_core/test/profile_runner_test.dart
[warning] 1-1: dart analyze (unused_import): Unused import: 'dart:io'. Try removing the import directive.
packages/devtools_profiler_core/test/flutter_widget_tree_test.dart
[warning] 1-1: dart analyze (unused_import): Unused import: 'dart:io'. Try removing the import directive.
packages/devtools_profiler_cli/test/cli_test.dart
[warning] 1-1: dart analyze (unused_import): Unused import: 'dart:io'. Try removing the import directive.
🪛 LanguageTool
packages/devtools_profiler_core/CHANGELOG.md
[style] ~17-~17: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... helpers for CLI and agent workflows. - Added WidgetInspectorQueryService for calli...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
packages/devtools_profiler_cli/CHANGELOG.md
[style] ~18-~18: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... alias for the same analysis surface. - Added flutter attach launch support for who...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...for Flutter widget tree inspection. - Addedflutter:inspector` for generic Flutter...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (49)
README.md (1)
115-117: LGTM!Also applies to: 739-754
packages/devtools_profiler_cli/CHANGELOG.md (1)
13-25: LGTM!Also applies to: 39-40, 68-69
packages/devtools_profiler_cli/README.md (1)
138-141: LGTM!Also applies to: 158-163, 173-196, 208-221, 448-451, 465-468
skills/devtools-profiler-local/SKILL.md (1)
226-262: LGTM!packages/devtools_profiler_core/.pubignore (1)
1-1: LGTM!packages/devtools_profiler_core/CHANGELOG.md (1)
3-29: LGTM!packages/devtools_profiler_core/README.md (2)
145-150: LGTM!
223-230: LGTM!packages/devtools_profiler_core/lib/src/flutter/debug_dump.dart (3)
6-6: LGTM!
15-16: LGTM!
18-21: LGTM!packages/devtools_region_profiler/README.md (1)
176-179: LGTM!packages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dart (1)
9-15: LGTM!Also applies to: 180-180, 225-253
packages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dart (1)
5-8: LGTM!Also applies to: 22-22
packages/devtools_profiler_cli/lib/src/cli/options.dart (1)
143-162: LGTM!packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/list_scroll_screen.dart (1)
31-33: LGTM!packages/devtools_profiler_cli/lib/src/mcp/tools/capture_tools.dart (1)
109-110: LGTM!Also applies to: 121-122, 165-165
packages/devtools_profiler_cli/lib/src/mcp/tools/flutter_tools.dart (1)
218-261: LGTM!Also applies to: 279-284
packages/devtools_profiler_core/lib/src/capture/profile_attach_request.dart (1)
13-13: LGTM!Also applies to: 32-32
packages/devtools_profiler_core/lib/src/capture/profile_runner.dart (2)
36-38: LGTM!
541-542: LGTM!packages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dart (4)
93-116: LGTM!
781-791: LGTM! Good catch fixing the summary path to honorprojectOnlylike the non-summary path.
820-831: 🎯 Functional CorrectnessConfirm
groupNameis a valid/needed argument for these inspector RPCs.Flutter's
WidgetInspectorServiceextensions (e.g.getSelectedWidget,getParentChain,getProperties) consumeargandobjectGroup, notgroupName. Sending an extragroupNamekey is likely ignored by the framework, but please confirm it isn't a mistaken stand-in forobjectGroup(which is already sent) or a dead argument that should be removed, particularly for less common inspector methods where argument handling may differ.
1502-1503: LGTM!packages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dart (1)
5-29: LGTM!packages/devtools_profiler_core/test/widget_inspector_query_test.dart (2)
1-86: LGTM! Good coverage of both the decode-success and extension-unavailable paths.
1-6: 🩺 Stability & AvailabilityPipeline reports an unused
dart:ioimport that isn't visible in this file's shown imports.CI flagged
unused_import: 'dart:io'for this file, but the provided imports (lines 1-5) don't includedart:io. Please confirm whether this warning is stale or whether the import still needs to be removed from the committed file.Source: Pipeline failures
packages/devtools_profiler_cli/lib/src/mcp/server.dart (1)
71-78: LGTM!packages/devtools_profiler_cli/test/mcp_server_test.dart (2)
42-45: LGTM!
235-235: LGTM!Also applies to: 246-268
packages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart (2)
190-203: LGTM!Also applies to: 470-470
363-365: 🎯 Functional Correctness
flutter testdoesn't use this terminal-mode path. Therun-only guard is intentional, andflutter testis handled separately via--enable-vmservice.> Likely an incorrect or invalid review comment.packages/devtools_profiler_cli/lib/src/cli/commands/discover_command.dart (1)
49-53: LGTM!packages/devtools_profiler_cli/test/cli_test.dart (1)
6-7: LGTM!Also applies to: 77-125, 176-225, 276-301, 327-330, 351-354, 375-378, 570-570, 579-602
packages/devtools_profiler_core/test/profile_runner_test.dart (1)
38-40: LGTM!Also applies to: 108-125, 127-155, 472-474
packages/devtools_profiler_core/test/flutter_widget_tree_test.dart (1)
36-83: LGTM!packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/animation_screen.dart (1)
25-32: LGTM!packages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart (3)
4-195: LGTM!
240-428: LGTM!
504-640: LGTM!packages/devtools_profiler_core/lib/src/flutter/frame_analysis.dart (2)
261-307: Per-frame shader jank tracking is well-designed.The
shaderJankInCurrentFrameflag correctly correlates shader compilation events with janky frames: it's set when a shader event exceeds 1 ms, read into the janky-frame event map, and reset after every frame event to prevent cross-frame leakage. This is a meaningful improvement over a globalshaderEvents.any()check.
343-355: LGTM!packages/devtools_profiler_core/lib/src/flutter/widget_tree.dart (2)
155-170: Flattening framework wrappers is the right approach forprojectOnlymode.Hoisting children of framework widgets into the parent (instead of dropping them) correctly preserves project widgets nested inside framework wrappers. The recursive
_parseNodecall ensures nested framework wrappers are also flattened before their children are hoisted.
179-203: Expanded framework widget list and_heuristic removal look correct.Removing
name.startsWith('_')correctly stops treating private project widgets as framework widgets. The added names (MaterialApp,Scaffold,Navigator, etc.) are all common framework wrappers that should be flattened in project-only mode.packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/screens/home_screen.dart (1)
11-11: LGTM!packages/devtools_profiler_core/lib/src/flutter/screenshot.dart (1)
43-59: LGTM!packages/devtools_profiler_cli/lib/src/cli.dart (1)
52-56: LGTM!packages/devtools_profiler_core/lib/devtools_profiler_core.dart (1)
57-57: 🎯 Functional CorrectnessNo remaining
navigation_stack.dartreferences.> Likely an incorrect or invalid review comment.
| String _toInspectorRpc(String method) { | ||
| if (method.startsWith('ext.flutter.')) { | ||
| return method; | ||
| } | ||
| if (method.startsWith('get') || method.startsWith('set')) { | ||
| return 'ext.flutter.inspector.$method'; | ||
| } | ||
| if (method.startsWith('profile') || method.startsWith('debug')) { | ||
| return 'ext.flutter.$method'; | ||
| } | ||
| return method; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Document this RPC-name mapping helper.
_toInspectorRpc encodes VM-service/protocol assumptions about how CLI-facing method names map to actual ext.flutter.* extension names, but has no doc comment. Also note the prefix rules (get*/set*, profile*/debug*) don't cover every real inspector extension (e.g. boolean toggles like structuredErrors or isWidgetTreeReady fall through unchanged), so unsupported method names would silently produce an invalid RPC string rather than a clear error.
As per path instructions, "Consider documenting private helpers when they encode profiler behavior, artifact contracts, protocol semantics, or VM-service assumptions."
🤖 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 `@packages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dart` around lines
966 - 977, Document the private _toInspectorRpc helper, describing its
VM-service extension-name mapping and supported get/set/profile/debug prefixes.
Update the mapping to explicitly handle all supported inspector methods,
including structuredErrors and isWidgetTreeReady, and reject unsupported names
with a clear error instead of returning an invalid RPC string.
Source: Path instructions
| final Tool profileTimelineTool = Tool( | ||
| name: 'profile_timeline', | ||
| title: 'Profile Timeline', | ||
| description: | ||
| 'Profile VM timeline frame timing and detect jank from a running ' | ||
| 'Flutter application. Connects to the VM service URI, samples frame ' | ||
| 'events from the VM timeline for the requested duration, and returns ' | ||
| 'frame timing metrics including P90, P99, max frame times, jank ' | ||
| 'percentage, and phase breakdowns (build, layout, paint). Use ' | ||
| 'profile_discover_apps to find available VM service URIs.', | ||
| inputSchema: Schema.object( | ||
| properties: { | ||
| 'vmServiceUri': Schema.string( | ||
| description: | ||
| 'The VM service WebSocket URI (e.g. ' | ||
| 'ws://127.0.0.1:8181/abc123/ws).', | ||
| ), | ||
| 'durationSeconds': Schema.int( | ||
| description: 'Duration to profile frames in seconds (default: 5).', | ||
| ), | ||
| }, | ||
| required: ['vmServiceUri'], | ||
| additionalProperties: false, | ||
| ), | ||
| outputSchema: Schema.object( | ||
| description: 'Frame timing analysis results.', | ||
| additionalProperties: true, | ||
| ), | ||
| annotations: ToolAnnotations( | ||
| destructiveHint: false, | ||
| idempotentHint: false, | ||
| openWorldHint: false, | ||
| readOnlyHint: true, | ||
| title: 'Profile Timeline', | ||
| ), | ||
| ); | ||
|
|
||
| final Tool timelineTool = Tool( | ||
| name: 'timeline', | ||
| title: 'Timeline', | ||
| description: | ||
| 'Profile VM timeline frame timing and detect jank from a running ' | ||
| 'Flutter application. Connects to the VM service URI, samples frame ' | ||
| 'events from the VM timeline for the requested duration, and returns ' | ||
| 'frame timing metrics including P90, P99, max frame times, jank ' | ||
| 'percentage, and phase breakdowns (build, layout, paint). Use ' | ||
| 'profile_discover_apps to find available VM service URIs.', | ||
| inputSchema: Schema.object( | ||
| properties: { | ||
| 'vmServiceUri': Schema.string( | ||
| description: | ||
| 'The VM service WebSocket URI (e.g. ' | ||
| 'ws://127.0.0.1:8181/abc123/ws).', | ||
| ), | ||
| 'durationSeconds': Schema.int( | ||
| description: 'Duration to profile frames in seconds (default: 5).', | ||
| ), | ||
| }, | ||
| required: ['vmServiceUri'], | ||
| additionalProperties: false, | ||
| ), | ||
| outputSchema: Schema.object( | ||
| description: 'Frame timing analysis results.', | ||
| additionalProperties: true, | ||
| ), | ||
| annotations: ToolAnnotations( | ||
| destructiveHint: false, | ||
| idempotentHint: false, | ||
| openWorldHint: false, | ||
| readOnlyHint: true, | ||
| title: 'Timeline', | ||
| ), | ||
| ); | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether profileFrameProfileTool duplicates the new timeline tools.
sed -n '24,60p' packages/devtools_profiler_cli/lib/src/mcp/tools/flutter_tools.dartRepository: kingwill101/devtools-profiler
Length of output: 1426
Extract a shared factory for the timeline tools
profileTimelineTool and timelineTool are identical apart from name and title, so the repeated description/schema/annotations should be built in one helper. profileFrameProfileTool uses the same input/output contract too, so it may be worth folding that into the same factory if these are meant to stay aligned.
🤖 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 `@packages/devtools_profiler_cli/lib/src/mcp/tools/flutter_tools.dart` around
lines 61 - 134, Extract the duplicated timeline tool configuration from
profileTimelineTool and timelineTool into a shared factory function,
parameterized by name and title, while preserving their descriptions, schemas,
and annotations. Consider extending the factory to profileFrameProfileTool so
all tools sharing this input/output contract remain aligned, then instantiate
each tool through the factory.
- Extract and into VmServiceDiscovery mixin to eliminate duplicated URI normalization (7x) and isolate resolution (3x) across flutter commands - Make negatable in attach CLI so opts into DTD - Document with supported method name patterns - Add doc comments to and - Document test helper - Wrap long warning string in adjacent literals for readability - Fix SKILL.md: replace top-level ref with - Fix SKILL.md: replace stale with
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dart (1)
966-989: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMap
isWidgetTreeReadyandstructuredErrorsexplicitly. They’re listed as supported inspector methods, but_toInspectorRpcreturns them unchanged instead of routing them toext.flutter.inspector.*. Either add special handling or remove them from the supported-method examples.🤖 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 `@packages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dart` around lines 966 - 989, Update _toInspectorRpc to explicitly route isWidgetTreeReady and structuredErrors to ext.flutter.inspector.<method>, or remove them from the documented supported-method examples; retain the existing mappings for get/set and profile/debug methods.
🤖 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 `@skills/devtools-profiler-local/SKILL.md`:
- Around line 26-27: Update the command list in the initial guidance to use the
actual Flutter command names, replacing “memory-snapshot” with
“flutter:memory-snapshot” and “widget-tree” with “flutter:widget-tree”; retain
the existing “timeline” or “flutter:frame-profile” alternatives.
---
Outside diff comments:
In `@packages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dart`:
- Around line 966-989: Update _toInspectorRpc to explicitly route
isWidgetTreeReady and structuredErrors to ext.flutter.inspector.<method>, or
remove them from the documented supported-method examples; retain the existing
mappings for get/set and profile/debug methods.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bba23540-f41f-45f6-a8a4-eafaec2a4d8d
📒 Files selected for processing (8)
packages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dartpackages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dartpackages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dartpackages/devtools_profiler_core/test/flutter_widget_tree_test.dartskills/devtools-profiler-local/SKILL.md
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: CI / 1_Format and Analyze.txt: feat: widget inspector queries, timeline aliases, frame analysis fixes
Conclusion: failure
##[group]Run dart analyze .
�[36;1mdart analyze .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.11.4/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Analyzing ....
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:1:8 - Target of URI doesn't exist: 'package:flutter/material.dart'. Try creating the file referenced by the URI, or try using a URI for a file that does exist. - uri_does_not_exist
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:10:3 - Undefined name 'WidgetsFlutterBinding'. Try correcting the name to one that is defined, or defining the name. - undefined_identifier
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:11:3 - The function 'runApp' isn't defined. Try importing the library that defines 'runApp', correcting the name to the name of an existing function, or defining a function named 'runApp'. - undefined_function
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:14:31 - Classes can only extend other classes. Try specifying a different superclass, or removing the extends clause. - extends_non_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:15:32 - No associated named super constructor parameter. Try changing the name to the name of an existing named super constructor parameter, or creating such named parameter. - super_formal_parameter_without_associated_named
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:3 - Undefined class 'Widget'. Try changing the name to the name of an existing class, or creating a class with the name 'Widget'. - undefined_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:16 - Undefined class 'BuildContext'. Try changing the name to the name of an existing class, or ...
GitHub Actions: CI / Format and Analyze: feat: widget inspector queries, timeline aliases, frame analysis fixes
Conclusion: failure
##[group]Run dart analyze .
�[36;1mdart analyze .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.11.4/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Analyzing ....
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:1:8 - Target of URI doesn't exist: 'package:flutter/material.dart'. Try creating the file referenced by the URI, or try using a URI for a file that does exist. - uri_does_not_exist
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:10:3 - Undefined name 'WidgetsFlutterBinding'. Try correcting the name to one that is defined, or defining the name. - undefined_identifier
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:11:3 - The function 'runApp' isn't defined. Try importing the library that defines 'runApp', correcting the name to the name of an existing function, or defining a function named 'runApp'. - undefined_function
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:14:31 - Classes can only extend other classes. Try specifying a different superclass, or removing the extends clause. - extends_non_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:15:32 - No associated named super constructor parameter. Try changing the name to the name of an existing named super constructor parameter, or creating such named parameter. - super_formal_parameter_without_associated_named
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:3 - Undefined class 'Widget'. Try changing the name to the name of an existing class, or creating a class with the name 'Widget'. - undefined_class
error - packages/devtools_profiler_core/test/fixtures/profiled_flutter_app/lib/main.dart:18:16 - Undefined class 'BuildContext'. Try changing the name to the name of an existing class, or ...
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Follow idiomatic Dart and keep code easy to scan in split-screen views
Prefer multi-line strings over string concatenation for large text blocks, command output fixtures, JSON examples, and terminal snapshots
Keep lines near 80 characters where practical. Long identifiers and URLs may exceed that when wrapping would make the code harder to read
Use records for short-lived grouped return values instead of introducing one-off classes
Use patterns,if-case, and switch expressions when they make parsing or dispatch logic clearer
Use class modifiers such assealed,final,base, andinterfacewhen they describe the intended inheritance boundary
Use digit separators for large numeric literals, for example timeouts, sample counts, and byte sizes
Use wildcard variables for intentionally unused callback parameters
Use null-aware collection elements when conditionally including nullable values in list or map literals
Use dot shorthands only when the inferred type is obvious from context
Use///documentation comments for public APIs
Start doc comments with a short, single-sentence summary
Put a blank line after the first sentence when adding more detail in doc comments
Avoid repeating information that is already obvious from the declaration in documentation comments
Start method comments with third-person verbs, such as 'Returns', 'Starts', or 'Captures'
Start non-boolean property comments with a noun phrase
Start boolean property comments with 'Whether'
Use square brackets for in-scope identifiers, such as [ProfileRunRequest], [Duration], and [StateError]
Explain parameters, return values, and exceptions in prose rather than using tag-style documentation
Prefer fenced Markdown code blocks for examples in documentation comments
Keep Markdown simple; avoid HTML in documentation comments
Files:
packages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dartpackages/devtools_profiler_core/test/flutter_widget_tree_test.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dartpackages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dartpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart
packages/devtools_profiler_*/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Consider documenting private helpers when they encode profiler behavior, artifact contracts, protocol semantics, or VM-service assumptions
Files:
packages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dartpackages/devtools_profiler_core/test/flutter_widget_tree_test.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dartpackages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dartpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart
🔇 Additional comments (15)
skills/devtools-profiler-local/SKILL.md (4)
227-233: LGTM!
241-263: LGTM!
277-280: LGTM!
421-421: LGTM!packages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dart (1)
199-206: LGTM!Also applies to: 225-253
packages/devtools_profiler_cli/lib/src/cli/commands/vm_service_discovery.dart (1)
48-69: LGTM!The
normalizeWsUriandresolveMainIsolatehelpers are well-documented and correctly consolidated into the mixin. Note thatresolveMainIsolateis more lenient than the previous private copies it replaces: it falls back to the first system isolate when no non-system isolates exist, rather than throwing. This is a reasonable resilience improvement, but commands relying on Flutter extensions should be aware that a system-isolate fallback may not have those extensions registered.packages/devtools_profiler_core/lib/src/capture/profile_runner.dart (1)
36-38: LGTM!Also applies to: 280-284
packages/devtools_profiler_cli/lib/src/cli/commands/flutter_commands.dart (1)
4-46: LGTM!The
_FrameTimingCommandbase class effectively eliminates duplication across frame/timeline commands, and the consolidation ofnormalizeWsUri/resolveMainIsolateinto the mixin addresses the previous triplication concern. TheInspectorQueryCommandimplementation with_methodToRpctranslation and the memory snapshot persistence flow are well-structured.Also applies to: 55-60, 130-185, 230-239, 255-268, 316-407, 460-472, 514-605, 614-618, 664-669, 724-729, 798-798
packages/devtools_profiler_core/test/flutter_widget_tree_test.dart (1)
36-83: LGTM!Also applies to: 134-154
packages/devtools_profiler_core/lib/src/flutter/widget_inspector_query.dart (3)
1-28: LGTM!
39-70: LGTM!
72-86: LGTM!packages/devtools_profiler_cli/lib/src/mcp/tool_handlers.dart (3)
96-114: LGTM!
785-785: LGTM!
801-839: LGTM!
| `timeline` (or `flutter:frame-profile`), `memory-snapshot`, or | ||
| `widget-tree`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the actual Flutter command names.
memory-snapshot and widget-tree are documented later as flutter:memory-snapshot and flutter:widget-tree (Lines [254] and [268]). Agents following this “First Response” guidance may otherwise suggest unrecognized commands.
Proposed fix
- `memory-snapshot`, or
- `widget-tree`.
+ `flutter:memory-snapshot`, or
+ `flutter:widget-tree`.📝 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.
| `timeline` (or `flutter:frame-profile`), `memory-snapshot`, or | |
| `widget-tree`. | |
| `timeline` (or `flutter:frame-profile`), `flutter:memory-snapshot`, or | |
| `flutter:widget-tree`. |
🤖 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 `@skills/devtools-profiler-local/SKILL.md` around lines 26 - 27, Update the
command list in the initial guidance to use the actual Flutter command names,
replacing “memory-snapshot” with “flutter:memory-snapshot” and “widget-tree”
with “flutter:widget-tree”; retain the existing “timeline” or
“flutter:frame-profile” alternatives.
These files require package:flutter which is not available during workspace-level dart analyze. They are only used by integration tests that run with the Flutter SDK.
Summary
19 commits of work across
devtools_profiler_coreanddevtools_profiler_cli, adding Flutter widget inspector query capabilities, timeline aliases, attach/drive launch improvements, and frame analysis fixes.Changes
devtools_profiler_coreWidgetInspectorQueryServicefor calling generic Flutter inspector service extensions (getSelectedWidget,getProperties,getChildren,getParentChain,getDetailsSubtree,getLayoutExplorerNode)navigation_stackexport withwidget_inspector_query; route stack support removed upstreamdevtools_profiler_clitimelineas a top-level CLI command andprofile_timelineas an MCP tool alias for the frame timing analysis surfaceflutter:inspector— New command for generic Flutter widget inspector queriesflutter attachlaunches — Profile already-running Flutter appsflutter drivelaunches — Profile Flutter drive test runsattachfinds a running VM service when no URI is providedChangelogs
devtools_profiler_cli0.4.0 section already comprehensive; no changes neededdevtools_profiler_core0.4.0 section updated with missing entries (inspector queries, per-frame shader jank, export changes)Documentation
Summary by CodeRabbit
timeline,flutter:timeline, andflutter:frame-profile, plus matching MCP timeline tools.flutter:inspector --method ....flutter attachandflutter drive, including VM-service auto-discovery when a single app is running.ws/wss) and attach now defaults to a bounded 15s session.