Iteration 39: LLM ergonomics — help text, hints & recipes#41
Merged
Conversation
- Add output format examples to --help for 10 key commands (tabs, navigate, eval, dom, console, network, perf, screenshot, cookies, snapshot) - Document default limits/sort in network (20, duration desc), console (50, timestamp desc), perf (20, duration desc) - Expand --wait-text/--wait-selector/--no-daemon help descriptions - Add output examples, troubleshooting, and workflow patterns to llm-help - Add actionable hints to error messages: element-not-found suggests dom --count, tab-not-found suggests ff-rdp tabs, URL rejection mentions --allow-unsafe-urls, timeouts suggest --wait-timeout - Add "hint" JSON field on zero-result output for cookies, console, network - Add fallback/fallback_method to meta when a11y/sources fall back to JS eval - Add interaction, error handling, and cross-command workflow recipes - Pre/post dogfooding sessions documented (sessions 34 & 35) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds richer CLI help text and recipes, inserts actionable hints into error messages and empty-result JSON envelopes, records JS-eval fallback usage in output metadata, and adds dogfooding/iteration docs. No public API signatures or runtime control flow semantics were changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (3)
crates/ff-rdp-cli/src/commands/network.rs (1)
126-175: Add focused tests for empty-hint decision paths.Please add tests for: (1) truly empty capture, and (2) filtered-empty with captured events, so hint text stays accurate over time.
As per coding guidelines "
**/*.rs: Make Rust code unit testable and add tests when feasible; add e2e tests for all commands/subcommands."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ff-rdp-cli/src/commands/network.rs` around lines 126 - 175, Write unit tests in crates/ff-rdp-cli/src/commands/network.rs to cover the two empty-hint decision paths: (1) truly empty capture (results == vec![]) and (2) filtered-empty where results contains events but the applied OutputControls filtering/sorting/fields results in no shown items. In each test, construct a CLI-like context and meta, call the same code paths that compute empty_hint and produce the final envelope (exercise the use_detail branch via OutputPipeline::from_cli + envelope_with_truncation and the summary branch via build_network_summary + output::envelope), then assert the produced JSON envelope contains a "hint" field with the expected message for the truly empty case and that the filtered-empty case either preserves or omits the hint as intended (use the same logic around empty_hint, envelope.as_object_mut, and insertion). Name tests e.g. test_empty_capture_shows_hint and test_filtered_empty_keeps_accurate_hint and keep them deterministic by controlling InputControls/cli so apply_limit/apply_fields produce the desired empty vs non-empty outcomes.crates/ff-rdp-cli/src/commands/sources.rs (1)
115-121: Add a unit-test seam for fallback metadata emission.This introduces a user-visible output branch (
meta.fallback,meta.fallback_method) but it isn’t directly asserted in tests. Consider extracting meta construction into a small pure helper and adding tests for bothused_js_fallback = true/false.As per coding guidelines: "Make Rust code unit testable and add tests when feasible; add e2e tests for all commands/subcommands".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ff-rdp-cli/src/commands/sources.rs` around lines 115 - 121, Extract the inline meta construction into a small pure helper (e.g., fn construct_meta(host: &str, port: u16, used_js_fallback: bool) -> serde_json::Value) and replace the current block that builds `meta` with a call to that helper; the helper should produce {"host", "port"} and conditionally insert "fallback" and "fallback_method" when `used_js_fallback` is true (matching the current keys "fallback" and "fallback_method" / value "js-eval"). Add unit tests for the helper that assert the JSON contains the fallback fields when `used_js_fallback = true` and does not contain them when false, and use the helper from the command implementation so behavior is covered by unit tests.crates/ff-rdp-cli/src/commands/a11y.rs (1)
62-73: Consider adding direct tests for fallback metadata.This adds user-visible metadata fields in one branch; extracting meta assembly into a helper would make
fallback/fallback_methodeasy to unit test.As per coding guidelines: "Make Rust code unit testable and add tests when feasible; add e2e tests for all commands/subcommands".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/ff-rdp-cli/src/commands/a11y.rs` around lines 62 - 73, Extract the JSON metadata assembly into a small helper function (e.g., build_meta or assemble_meta) that accepts the base fields (host, port, depth, max_chars) and a used_js_fallback flag and returns a serde_json::Value (or serde_json::Map) with the "fallback" and "fallback_method" keys added when used_js_fallback is true; update the current code to call that helper. Add unit tests that call the helper directly to assert that when used_js_fallback is true the returned meta contains "fallback": true and "fallback_method": "js-eval", and when false those keys are absent. Ensure tests reference the helper (assemble_meta/build_meta) so they don’t depend on CLI plumbing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/ff-rdp-cli/src/commands/network.rs`:
- Around line 126-132: The empty_hint currently uses results.is_empty() which
triggers when filters remove all items, producing a misleading "No network
events captured" message; change the condition to check the pre-filtered
collection (the original captured events variable used before applying
--filter/--method) instead of results so the hint only appears when there truly
were no captured events, and apply the same change at the other two occurrences
referenced (the blocks around lines 155-161 and 170-175).
In `@crates/ff-rdp-cli/src/commands/recipes.rs`:
- Around line 155-156: The recipe title currently promises "check status" but
the shown command only extracts links (ff-rdp dom "a[href]" --attrs --jq
'[.results[].href]'); either update the recipe title/text to only say "Extract
all links" or add an additional command step that takes the extracted URLs and
performs an HTTP status check (for example, iterate the jq output and run a
status probe with curl or an HTTP client) so the docs match the behaviour;
update the recipe description and any example command string near ff-rdp dom
"a[href]" --attrs --jq '[.results[].href]' to reflect the chosen fix.
In `@crates/ff-rdp-cli/src/tab_target.rs`:
- Around line 29-33: The current numeric tab-selection branch constructs a range
"1–{count}" even when count == 0, producing confusing messages like "1–0 tabs
available"; before using n and count in the existing conditional (the branch
returning Err(AppError::User(...)) that references n and count), add a guard for
count == 0 and return a clear error such as "no tabs available; use `ff-rdp
tabs` to list available tabs" (or similar) so numeric --tab values are handled
separately when the tabs list is empty; update the code in tab_target.rs around
the existing n/count check to perform this special-case early return.
In `@kb/iterations/iteration-39-llm-ergonomics.md`:
- Around line 35-37: The fenced code block that contains the example response
(the block starting with "Output: {\"results\": ...}") is missing a language
tag; update that fenced block in kb/iterations/iteration-39-llm-ergonomics.md by
adding a language specifier (preferably json) after the opening backticks so the
block becomes ```json to satisfy MD040 and improve readability.
- Line 29: Replace inline code-style paths with Obsidian wikilinks: find
occurrences of the filename string dogfooding-session-34.md (and the other
inline path referenced near line 88) and change them to Obsidian-style
[[dogfooding-session-34]] links; ensure the file retains YAML frontmatter and
that all cross-references in the markdown use [[wikilinks]] instead of inline
code paths for Obsidian compatibility.
---
Nitpick comments:
In `@crates/ff-rdp-cli/src/commands/a11y.rs`:
- Around line 62-73: Extract the JSON metadata assembly into a small helper
function (e.g., build_meta or assemble_meta) that accepts the base fields (host,
port, depth, max_chars) and a used_js_fallback flag and returns a
serde_json::Value (or serde_json::Map) with the "fallback" and "fallback_method"
keys added when used_js_fallback is true; update the current code to call that
helper. Add unit tests that call the helper directly to assert that when
used_js_fallback is true the returned meta contains "fallback": true and
"fallback_method": "js-eval", and when false those keys are absent. Ensure tests
reference the helper (assemble_meta/build_meta) so they don’t depend on CLI
plumbing.
In `@crates/ff-rdp-cli/src/commands/network.rs`:
- Around line 126-175: Write unit tests in
crates/ff-rdp-cli/src/commands/network.rs to cover the two empty-hint decision
paths: (1) truly empty capture (results == vec![]) and (2) filtered-empty where
results contains events but the applied OutputControls filtering/sorting/fields
results in no shown items. In each test, construct a CLI-like context and meta,
call the same code paths that compute empty_hint and produce the final envelope
(exercise the use_detail branch via OutputPipeline::from_cli +
envelope_with_truncation and the summary branch via build_network_summary +
output::envelope), then assert the produced JSON envelope contains a "hint"
field with the expected message for the truly empty case and that the
filtered-empty case either preserves or omits the hint as intended (use the same
logic around empty_hint, envelope.as_object_mut, and insertion). Name tests e.g.
test_empty_capture_shows_hint and test_filtered_empty_keeps_accurate_hint and
keep them deterministic by controlling InputControls/cli so
apply_limit/apply_fields produce the desired empty vs non-empty outcomes.
In `@crates/ff-rdp-cli/src/commands/sources.rs`:
- Around line 115-121: Extract the inline meta construction into a small pure
helper (e.g., fn construct_meta(host: &str, port: u16, used_js_fallback: bool)
-> serde_json::Value) and replace the current block that builds `meta` with a
call to that helper; the helper should produce {"host", "port"} and
conditionally insert "fallback" and "fallback_method" when `used_js_fallback` is
true (matching the current keys "fallback" and "fallback_method" / value
"js-eval"). Add unit tests for the helper that assert the JSON contains the
fallback fields when `used_js_fallback = true` and does not contain them when
false, and use the helper from the command implementation so behavior is covered
by unit tests.
🪄 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: 4f341d5c-52ba-42aa-b8d0-7a152bac93f6
📒 Files selected for processing (17)
crates/ff-rdp-cli/src/cli/args.rscrates/ff-rdp-cli/src/commands/a11y.rscrates/ff-rdp-cli/src/commands/click.rscrates/ff-rdp-cli/src/commands/console.rscrates/ff-rdp-cli/src/commands/cookies.rscrates/ff-rdp-cli/src/commands/llm_help.rscrates/ff-rdp-cli/src/commands/navigate.rscrates/ff-rdp-cli/src/commands/network.rscrates/ff-rdp-cli/src/commands/recipes.rscrates/ff-rdp-cli/src/commands/sources.rscrates/ff-rdp-cli/src/commands/type_text.rscrates/ff-rdp-cli/src/commands/url_validation.rscrates/ff-rdp-cli/src/commands/wait.rscrates/ff-rdp-cli/src/tab_target.rskb/dogfooding/dogfooding-session-34.mdkb/dogfooding/dogfooding-session-35.mdkb/iterations/iteration-39-llm-ergonomics.md
There was a problem hiding this comment.
Pull request overview
This PR improves ff-rdp’s “LLM ergonomics” by expanding CLI help text with output examples and defaults, adding actionable hints to errors/empty outputs, surfacing fallback metadata, and documenting common multi-command workflows (“recipes”) plus updated LLM reference help.
Changes:
- Expanded
--helpoutput for key commands (output schema examples, defaults, clarified wait flags /--no-daemon). - Added actionable hints to common errors/timeouts and added
"hint"fields for zero-result outputs (cookies/console/network), plus standardized fallback metadata fora11y/sources. - Added/updated “recipes” and
llm-helpdocumentation with troubleshooting and workflow patterns; added iteration + dogfooding KB entries.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| kb/iterations/iteration-39-llm-ergonomics.md | Iteration 39 plan/writeup (help text, hints, recipes, acceptance criteria). |
| kb/dogfooding/dogfooding-session-34.md | Pre-implementation dogfooding findings for LLM ergonomics. |
| kb/dogfooding/dogfooding-session-35.md | Post-implementation verification notes for the ergonomics changes. |
| crates/ff-rdp-cli/src/cli/args.rs | Adds/expands clap long_about help including output examples/defaults. |
| crates/ff-rdp-cli/src/tab_target.rs | Improves “tab not found” errors with actionable ff-rdp tabs/launch hints. |
| crates/ff-rdp-cli/src/commands/url_validation.rs | Mentions --allow-unsafe-urls in scheme rejection errors. |
| crates/ff-rdp-cli/src/commands/wait.rs | Adds --wait-timeout suggestion to wait timeout error message. |
| crates/ff-rdp-cli/src/commands/navigate.rs | Adds --wait-timeout suggestion to navigate wait timeout error message. |
| crates/ff-rdp-cli/src/commands/click.rs | Adds selector-debugging hint to “Element not found” JS exception. |
| crates/ff-rdp-cli/src/commands/type_text.rs | Adds selector-debugging hint to “Element not found” JS exception. |
| crates/ff-rdp-cli/src/commands/cookies.rs | Adds zero-result "hint" field (and consent banner note) when no cookies found. |
| crates/ff-rdp-cli/src/commands/console.rs | Adds zero-result "hint" field when no console messages captured. |
| crates/ff-rdp-cli/src/commands/network.rs | Adds zero-result "hint" field; refactors envelope creation to allow hint insertion. |
| crates/ff-rdp-cli/src/commands/a11y.rs | Adds structured fallback metadata when JS eval fallback is used. |
| crates/ff-rdp-cli/src/commands/sources.rs | Adds structured fallback metadata when JS eval fallback is used. |
| crates/ff-rdp-cli/src/commands/recipes.rs | Adds workflow “recipes” sections (interaction, error handling, cross-command). |
| crates/ff-rdp-cli/src/commands/llm_help.rs | Expands LLM reference with output examples, troubleshooting, workflows + test coverage update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| let empty_hint = if results.is_empty() { | ||
| Some(json!( | ||
| "No network events captured. Events are buffered by the daemon; navigate first with: ff-rdp navigate <url> --with-network, or use --follow to stream events in real time." |
There was a problem hiding this comment.
The JSON hint text here always says events are buffered by the daemon, but results.is_empty() can happen in direct mode (--no-daemon) too. This can mislead users who are connected directly.
Consider making the hint conditional on ctx.via_daemon (daemon-buffer wording vs. “page may have loaded before subscribing”) or rephrasing it to be accurate in both modes.
Suggested change
| "No network events captured. Events are buffered by the daemon; navigate first with: ff-rdp navigate <url> --with-network, or use --follow to stream events in real time." | |
| "No network events captured. The page may have loaded before subscribing; navigate first with: ff-rdp navigate <url> --with-network, or use --follow to stream events in real time." |
| // The non-timed drain_network_events() stops on idle, so timeout is never reached. | ||
| let summary = build_network_summary(&results, false); | ||
| let envelope = output::envelope(&summary, 1, &meta); | ||
| let mut envelope = output::envelope(&summary, 1, &meta); |
There was a problem hiding this comment.
In summary mode, the envelope is built with total set to 1, which makes .total meaningless for network (it should represent the number of captured requests). It also prevents the “total==0” convention from working consistently for zero-result hints.
Consider setting total to results.len() (or the pre-filter count, if that’s the intended semantic) so summary/detail modes report consistent totals.
Suggested change
| let mut envelope = output::envelope(&summary, 1, &meta); | |
| let mut envelope = output::envelope(&summary, results.len(), &meta); |
| "hint".to_string(), | ||
| json!( | ||
| "No cookies found. The page may not set cookies, or try navigating first. \ | ||
| If a consent banner is present, accept it or use `ff-rdp launch --auto-consent`." |
There was a problem hiding this comment.
This hint suggests ff-rdp launch --auto-consent, but --auto-consent requires either --profile or --temp-profile (see launch validation). As written, following the hint will error.
Adjust the suggested command to include --temp-profile (and optionally --headless) or mention the requirement explicitly.
Suggested change
| If a consent banner is present, accept it or use `ff-rdp launch --auto-consent`." | |
| If a consent banner is present, accept it or use `ff-rdp launch --temp-profile --auto-consent`." |
| affect which requests Firefox records. | ||
|
|
||
| Default: 20 results, sorted by duration (slowest first). | ||
| Output (summary mode): {\"results\": {\"summary\": {...}, \"top_slowest\": [...]}, \"total\": N, \"meta\": {...}} |
There was a problem hiding this comment.
The network command’s documented summary output shape here (results.summary / top_slowest) doesn’t match what build_network_summary() actually returns (total_requests, total_transfer_bytes, slowest, etc.).
Please update the example JSON in long_about to reflect the real summary keys so --help output stays accurate.
Suggested change
| Output (summary mode): {\"results\": {\"summary\": {...}, \"top_slowest\": [...]}, \"total\": N, \"meta\": {...}} | |
| Output (summary mode): {\"results\": {\"total_requests\": N, \"total_transfer_bytes\": N, \"slowest\": [...]}, \"total\": N, \"meta\": {...}} |
| ff-rdp perf compare https://old.example.com https://new.example.com --label "Before,After" | ||
|
|
||
| Extract all links and check status: | ||
| ff-rdp dom "a[href]" --attrs --jq '[.results[].href]'"# |
There was a problem hiding this comment.
The recipes output ends with an extra " before the raw-string terminator ("#). This will print a stray quote at the end of the last command line, making the recipe easy to copy/paste incorrectly.
Remove the extra quote so the final recipe line ends cleanly and the raw string terminator is not part of the displayed text.
Suggested change
| ff-rdp dom "a[href]" --attrs --jq '[.results[].href]'"# | |
| ff-rdp dom "a[href]" --attrs --jq '[.results[].href]'# |
| @@ -317,6 +317,68 @@ All commands return JSON by default with envelope: `{"results": ..., "total": N, | |||
| When results are truncated: `{"results": ..., "total": N, "truncated": true, "hint": "showing 20 of 84, use --all for complete list", "meta": {...}}` | |||
| Use `--jq` to filter: operates on `.results` automatically (implies --detail mode). | |||
There was a problem hiding this comment.
The --jq behavior is described as “operates on .results automatically”, but OutputPipeline::finalize() applies the jq filter to the full {results,total,meta,...} envelope. This mismatch will cause users/LLMs to write incorrect filters.
Update this line (or the pipeline behavior) so the documentation matches reality; currently filters need to reference .results explicitly.
Suggested change
| Use `--jq` to filter: operates on `.results` automatically (implies --detail mode). | |
| Use `--jq` to filter the full JSON envelope; reference `.results` explicitly when filtering results (implies --detail mode). |
| {"results": [{"url": "https://...", "title": "Page Title", "actor": "server1.conn0.tab1", "selected": true}], "total": 1, "meta": {"host": "localhost", "port": 6000}} | ||
|
|
||
| ### network output (summary mode, default) | ||
| {"results": {"summary": {"total_requests": 42, "total_duration_ms": 3200}, "top_slowest": [...]}, "total": 42, "meta": {...}} |
There was a problem hiding this comment.
The network “summary mode” example here doesn’t match the actual network command output:
build_network_summary()returns keys liketotal_requestsandslowest, not a nested{summary: ..., top_slowest: ...}structure.- The example also shows
"total": 42, but the current summary-mode implementation builds the envelope withtotal = 1.
Please update the example JSON to match the real schema (or adjust the command to make total consistent with request count).
Suggested change
| {"results": {"summary": {"total_requests": 42, "total_duration_ms": 3200}, "top_slowest": [...]}, "total": 42, "meta": {...}} | |
| {"results": {"total_requests": 42, "total_duration_ms": 3200, "slowest": [...]}, "total": 1, "meta": {...}} |
- network: condition empty_hint on watcher_was_empty, add filter-specific hint, use via_daemon for context-aware message, fix summary total to results.len() - cookies: include --temp-profile in --auto-consent hint - args/llm_help: fix network summary output example keys, clarify --jq docs - recipes: fix "Extract all links" title (no status check) - tab_target: handle count==0 edge case for numeric tab selector - KB: use wikilinks, add json language tag to fenced code block - fmt: fix let-chain formatting in a11y.rs and sources.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Apr 15, 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
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
--helpfor 10 key commands, documented default limits/sort orders, expanded--wait-text/--wait-selector/--no-daemondescriptionsdom --count, tab-not-found suggestsff-rdp tabs, URL rejection mentions--allow-unsafe-urls, timeouts suggest--wait-timeout"hint"field added when cookies, console, or network return empty resultsa11yandsourcesnow add"fallback": true, "fallback_method": "js-eval"to output metadata when falling back to JS evalllm-helpcommandTest plan
cargo fmt— cleancargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace— all 249+ tests passff-rdp tabs --helpshows output formatff-rdp click "nonexistent"shows hint in errorff-rdp cookieswith 0 results shows hint in JSON🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation