fix: make manual-qa help self-sufficient and slim routine help paths#1168
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
thymikee
left a comment
There was a problem hiding this comment.
Independent adversarial review, run against origin/fix/help-manual-qa-diet (ab256ef9) in a scratch worktree (build, tests, typecheck, lint, format:check, layering; byte-size claims reproduced via buildUsageText()/buildCommandUsageText() on both origin/main and the branch).
Blocking
1. pnpm format:check fails on src/cli/parser/cli-help.ts — this is a live CI failure, not a pre-existing one.
CI's "Lint & Format" job runs pnpm lint (passes) and pnpm format:check (fails) — see .github/workflows/ci.yml:86,89. Reproduced locally: oxfmt --check flags src/cli/parser/cli-help.ts. The offending hunk is the new renderAlignedSection code around cli-help.ts:1183:
const columnWidth = Math.max(...items.map((item) => Math.min(item.label.length, ALIGN_CAP))) + 2;
oxfmt --write collapses this back onto one line (it's currently fine as one line but something nearby trips the formatter — running pnpm format locally fixes it in one pass). The PR's own test evidence only lists pnpm lint, not pnpm format:check, which is presumably how this slipped through. Trivial fix, but as written this PR does not pass CI.
Verified sound (no changes needed)
Axis 1 — discovery-critical line. Default app loop: agent-device open <app> -> agent-device snapshot -i -> agent-device press/fill/click <target> --settle -> ... in AGENT_START_LINES (cli-help.ts:73) is byte-for-byte unchanged from origin/main, still the second line of "Agent Starting Point:", untouched by this diff. Confirmed by diffing cli-help.ts on both refs directly (not just trusting the PR description).
Axis 2 — command shapes, verified against real command definitions and real shell behavior:
open com.example.app --relaunch/open https://example.com/deep-link— both matchopen's actual usage (open [appOrUrl] [url] ... [--relaunch]).press @e12 --settle,fill @e13 "qa@example.com" --settle,close— matchpress/fill/closeusage exactly.press 'label="Follow"'— verifiedtryParseSelectorChain('label="Follow"')parses correctly, and confirmed in real zsh that this quoting survives.wait text "Order placed" 3000— traced throughsrc/core/wait-positionals.ts:parseWaitPositionals: for thetextkind, the single trailing numeric positional istimeoutMs(notquietMs— that only applies towait stable). Ran it directly:parseWaitPositionals(["text","Order placed","3000"])→{kind:"text", text:"Order placed", timeoutMs:3000}. The example is correct.- The apostrophe-quoting hazard line is real and correctly stated: reproduced in zsh that naively typing
press 'label="Don't leave"'breaks withunmatched "— exactly the failure mode the PR's own evidence section describes, so the added guidance line is warranted and accurate.
No wrong shapes found. This is the axis most likely to actively mislead a small model if wrong, and it checks out.
Axis 3 — cut content. Cross-checked each removed AGENT_QUICKSTART_LINES/EXAMPLE_LINES entry against help workflow, help react-native, help remote, and per-command help:
- Anti-pattern (snapshot | grep), Android IME capture, clipboard/paste limits, "back --system", raw-coordinate fallback + verification, sparse/AX recovery, remote lease/proxy details, batch JSON step format, "verification must name expected text/selector" — all still present verbatim-in-spirit inside
help workflow(lines ~194–333) orhelp react-native, which are exactly where the PR's routing sends agents for anything beyond the routine loop. - Real, minor loss (not blocking): the pinned-ref line (
cli-help.ts:85) dropped "Pinned refs get exact staleness warnings instead of the coarse tree-changed one" and thesettle.refsGenerationalternate-source mention. Diffed againstorigin/mainand confirmed this detail was only ever in bare help — it has no home inhelp workflowor elsewhere post-PR. It's an explanatory nicety (why pinning helps), not a procedural instruction, so I'd call this a nit, not a blocker. EXAMPLE_LINEScuts (macOS open, react-devtools, cdp) are all still covered by their own dedicated topics (help macos,help react-devtools,help cdp).
Axis 4 — react-native rerouting. help react-native's own body (cli-help.ts:691-730, untouched by this PR) still fully covers Metro/Re.Pack reload, Expo Go, dev-server port reachability, and LogBox/RedBox overlay handling itself — the routing change only touches which topic handles generic navigation questions, not RN-specific hazards. Grepped skills/agent-device/SKILL.md: it already lists help manual-qa first (for "scripted/manual QA, acceptance checks, checklist execution") and help workflow last ("fallback reference for general app driving or mixed tasks") — this file is untouched by the PR and was apparently already fixed by an earlier PR, so there's no stale routing left pointing routine QA at help workflow. All three routing surfaces this PR touches (react-native, physical-device, shared footer) consistently point routine QA to manual-qa.
Axis 5 — renderAlignedSection cap. Reasoned through the algorithm: columnWidth = min(max-actual-label-len, 26) + 2; rows with label.length <= 26 pad to columnWidth, longer rows fall back to a fixed 2-space gap and don't stretch the column. Verified in real output (node bin/agent-device.mjs help) that short rows (back, home, etc.) stay tightly aligned at a consistent column, and the 55-char tv-remote press|longpress... row renders with just a 2-space gap without disturbing anything else. Confirmed the test suite only asserts \s{2,} (grepped both test files), never an exact column width, so this doesn't risk breaking on unrelated label-length changes elsewhere in the registry.
Axis 6 — tests, typecheck, lint, layering. In a scratch worktree off origin/fix/help-manual-qa-diet: pnpm build clean; all 3 targeted test files (cli-help-topics.test.ts, cli-help-command-usage.test.ts, cli-help.test.ts) pass, 75/75; pnpm typecheck clean; pnpm lint (oxlint) clean; pnpm check:layering clean (729 files, DAG OK). gh pr checks 1168: Coverage fails on the known pre-existing runner-xctestrun.test.ts packageVersion fixture mismatch (0.19.0 fixture vs 0.19.1 repo version) — reproduces on main, unrelated to this PR, matches your stated known-flake list. Lint & Format fails — see Blocking #1 above, this one is real. All other checks (Typecheck, Integration Tests, Smoke Tests, Layering Guard, Bundle Size, etc.) pass.
Axis 7 — byte-size claims. Reproduced every number in the PR's table by calling buildUsageText() / buildCommandUsageText(topic) directly via node --experimental-strip-types on both origin/main and the branch:
| Surface | main (before) | branch (after) | PR claimed |
|---|---|---|---|
| bare help | 16,261 B | 11,734 B | 16,261 → 11,734 ✓ |
| manual-qa | 2,351 B | 3,014 B | 2,351 → 3,014 ✓ |
| react-native | 7,280 B | 7,652 B | 7,280 → 7,652 ✓ |
| workflow | 31,708 B | 31,793 B | 31,708 → 31,793 ✓ |
| Every number matches exactly, byte for byte. |
Minor nits (non-blocking)
- The shared
Related:footer (buildHelpTopicUsageText,cli-help.ts:1234) is appended to every topic uniformly, so viewinghelp manual-qaitself now prints a self-referentialagent-device help manual-qa routine QA loop with concrete command shapesline pointing at the page you're already reading. This is a pre-existing pattern (the same thing already happens forhelp workflowonmain), not introduced by this PR, so not a regression — but worth a follow-up to special-case the footer per-topic. - Minor content loss noted under Axis 3 (pinned-ref staleness-warning nuance).
Verdict
Needs changes — one real blocker: run pnpm format on cli-help.ts and repush so pnpm format:check (and hence the "Lint & Format" CI job) passes. Everything else — the discovery-critical line, command-shape correctness, content-cut coverage, RN routing, the alignment fix, and every byte-size claim — checks out under direct verification. Once formatting is fixed this is approve-worthy.
|
Both review findings addressed in 7235698:
Updated sizes: bare help unchanged at 11,734 B; |
July 2026 benchmarks (gpt-5.4-mini/haiku/sonnet driving the CLI) showed agents burning 14-41KB of tool output on help before a routine QA flow: help react-native routed "generic navigation, selectors, refs, verification" to help workflow (31.7KB), one run read the bare `agent-device help` mid-task (16.3KB), and several fell back to per-command help because manual-qa lacked concrete command shapes. - manual-qa gets a "Command shapes" block covering the full routine QA loop (open --relaunch, deep-link open, snapshot -i, press/fill --settle, wait text, close) plus a quoting note for apostrophe/quote labels, so agents don't need to read help workflow or per-command help for a normal pass. - react-native's routing block no longer sends routine QA to help workflow; it now points routine flows at manual-qa and reserves workflow for deep exploration/debugging. - physical-device and the shared topic-footer routing also point at manual-qa alongside workflow. - Trimmed the bare `agent-device help` output: cut Agent Quickstart lines that duplicated topic-specific detail already owned by react-native/workflow/remote/web/tv, dropped 3 redundant Examples, and fixed an alignment bug where one long usage string (tv-remote) forced padding whitespace onto every other Commands: row. The "Default app loop" line (shown to flip Haiku from 0/3 to 3/3 in earlier benchmarks) stays intact and early.
…rkflow - pnpm format:check failed on the new renderAlignedSection columnWidth line; reformatted with pnpm format. - Reviewer noted the only content genuinely lost from the bare-help Quickstart trim was the "pinned refs get exact staleness warnings" nuance; restored it as one line in help workflow's Snapshots and refs section (its natural owner), keeping the bare-help line short.
7235698 to
3eaa15d
Compare
|
Evidence
July 2026 benchmarks of gpt-5.4-mini/haiku/sonnet driving the CLI showed agents spending 14-41KB of tool output on help before a routine QA flow:
help manual-qa+help react-nativebefore a QA flow spent 14-41KB of tool output on help.help workflow(31.7KB) becausehelp react-nativerouted "generic navigation, selectors, refs, verification" there.agent-device help(16.3KB) mid-task.help open/press/fill/close) becausemanual-qalacked concrete command shapes.relaunchcommand because no shape showedopen <app> --relaunch.label="Don't leave".Changes
help manual-qanow has a compact "Command shapes" block covering the whole routine QA loop (open <bundleId> --relaunch,open <url>deep link,snapshot -i,press @eN --settle,press 'label="Text"',fill @eN "text" --settle,wait text "..." <ms>,close), plus one line on apostrophe/quote-label quoting hazards (prefer@reffrom the latest snapshot/settle output). No per-command help reads should be needed for a normal pass.help react-nativeno longer routes routine QA tohelp workflow. The routing block now sends routine flows tohelp manual-qaand reserveshelp workflowfor deep exploration/debugging of navigation/selector/ref edge cases.agent-device help(buildUsageText) is trimmed:Agent Quickstartlines that duplicated detail already owned by dedicated topics (React Native Metro/Expo recovery, Android IME capture, direct-proxy flags, sparse/AX recovery, the full web command sequence, back/system-back wording) — kept short pointer lines tohelp react-native/help remote/help tv/help webinstead.Examples:lines (macOS open, react-devtools, cdp — none pinned by tests, all covered by their own topics).renderAlignedSection: one long usage string (tv-remote press|longpress <button> [--duration-ms <ms>], 55 chars) forced every otherCommands:row to pad out to that width — pure whitespace, no content. Capped the alignment column at 26 chars; longer labels fall back to a plain 2-space gap (still satisfies every\s{2,}regex in the test suite).Agent Starting Point:.physical-deviceand the shared per-topic "Related:" footer (buildHelpTopicUsageText) now also point athelp manual-qa, not justhelp workflow.Byte sizes (via
buildUsageText()/buildCommandUsageText(topic), UTF-8)agent-device helphelp manual-qahelp react-nativehelp workflow(unchanged content + new footer line)Bare help landed at 11.7KB rather than the ~8KB target. The floor is the auto-generated
Commands:catalog (5.5KB after the alignment fix, down from 7.2KB — the 1.7KB alignment-padding fix was the only safe cut available there). That catalog is derived from the command-descriptor registry (src/commands/**), not owned bycli-help.ts; shortening the ~65 per-command summary strings to hit 8KB would be a separate, broader change touching many files well outside a "help text diet," so I left it alone and documented the shortfall here instead of quietly missing the target.Test evidence
test/skillgym/suites/agent-device-smoke-suite.ts: grepped for any direct assertions oncli-help.tscontent (cli-help,buildUsageText,HELP_TOPICS, literal'help') — only one match, aforbiddenOutputs: [plannedCommand('help')]check on the agent's own planned output, unrelated to help text wording. Not run (LLM bench, per instructions).scripts/help-conformance-bench.mjs: loads help text viacliHelp(['--help'])/cliHelp(['help', docId])with no hardcoded string assertions on content — only scores the LLM's planned commands. Not run (LLM bench).cli-help.ts:src/__tests__/cli-help.test.ts(21 tests, all abouthelp workflow/per-command help, untouched by this change) andsrc/cli/parser/__tests__/cli-help-topics.test.ts(bare-help + all topic assertions) andsrc/cli/parser/__tests__/cli-help-command-usage.test.ts. Updatedcli-help-topics.test.ts's "usage includes agent workflows..." test to drop assertions on the ~12 Quickstart lines that were deliberately cut/moved into topics, and added assertions for the new manual-qa pointer and TV/web pointer lines. All 75 tests across the three files pass.pnpm build,pnpm lint,pnpm typecheck,pnpm check:layeringall pass clean.vitest run --project unit-core(3350 tests): 7 unrelated failures, none in files touched by this PR (daemon-client, runtime-hints, apple/core index, runner-client/xctestrun). Re-ran those 5 files in isolation: 219/220 passed; the 1 remaining failure is a pre-existing stalepackageVersionfixture (expects "0.19.0", repo is now on 0.19.1) unrelated to help text.Deliberately not trimmed
Commands:catalog itself (5.5KB) — see byte-size note above.help workflow's body content — untouched; only the shared "Related:" footer gained a line pointing tomanual-qa.Agent Workflows:topic-list descriptions — already compact (1.3KB for 13 topics) and heavily pinned by tests verbatim; not worth the churn for marginal savings.