Skip to content

setup: offer the agent's on-demand camera mode, defaulting to on_demand (#179) - #182

Open
Adityakk9031 wants to merge 3 commits into
robocurve:mainfrom
Adityakk9031:#179
Open

setup: offer the agent's on-demand camera mode, defaulting to on_demand (#179)#182
Adityakk9031 wants to merge 3 commits into
robocurve:mainfrom
Adityakk9031:#179

Conversation

@Adityakk9031

Copy link
Copy Markdown
Contributor

Closes #179.

Problem

-P images=on_demand (#173, #175) allows the agent policy to issue take_pic tool calls for frames on-demand instead of attaching all camera frames to every observation. While this is cheaper and preferable for real rigs, it was only discoverable via the plugin README. inspect-robots setup is where operators configure defaults, but it previously did not prompt for agent policy options.

Solution

  1. Setup Wizard (_setup.py):
    • Added _valid_images_mode() validator ("on_demand" / "always").
    • Gated a prompt on configured_policy == "agent" that suggests on_demand (explaining the token savings vs model responsibility tradeoff) while preserving any existing images configuration in [policy.args] on setup re-runs.
    • Updated _render_config() to render managed [policy.args] blocks alongside [defaults] and [embodiment.args] while preserving any unmanaged policy args.
  2. Documentation:
    • Updated plugins/inspect-robots-agent/README.md to note that inspect-robots setup suggests on_demand while preserving the library default always.
    • Updated docs/guide/cli.md and docs/guide/quickstart.md wizard descriptions.
  3. Tests (tests/test_setup.py):
    • Added unit test test_render_config_renders_policy_args.
    • Added integration tests for prompt defaults (on_demand), explicit selection (always), and config preservation on setup re-runs.

Verification

  • ruff check . — All checks passed.
  • ruff format --check . — All 108 files formatted.
  • pytest tests/test_setup.py105 passed, 16 skipped (POSIX/symlink-gated tests skipped on Windows as expected).

@Adityakk9031

Copy link
Copy Markdown
Contributor Author

@jeqcho have a look

@jeqcho

jeqcho commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Adityakk9031! CI on this branch is green, but it now has merge conflicts with main. Could you rebase and resolve? We'll review right after. Thank you! 🙏

@Adityakk9031

Adityakk9031 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@jeqcho hey conflict are fixed have a look

@jeqcho

jeqcho commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Adityakk9031 — you did resolve the conflicts, but main has moved again since your push on Aug 1 and the branch is now conflicting once more (sorry about the moving target! 🙏). Could you rebase one more time? We'll get a review in promptly after that so it doesn't drift again.

@jeqcho jeqcho left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @Adityakk9031 — this lands exactly what #179 asked for, and the fiddly part (teaching the renderer a second managed section) came out clean.

What I checked:

  • Scope matches the issue. The wizard suggests on_demand; the plugin default stays always, so programmatic and CI callers are untouched. The policy_args_owner scoping (defaults.py:219, cli.py:1420) keeps a carried images key inert if the policy later changes, so the raw carry-through in the renderer is safe.
  • Renderer. The new [policy.args] block in src/inspect_robots/_setup.py (_render_config) mirrors the [embodiment.args] block exactly: managed keys first, unmanaged keys carried raw, multiline \n\n\t escaping, empty-section suppression, and the section correctly excluded from the generic carried loop. No scoping hazards in run_setuppolicy_args and managed_policy_args are always bound before _render_config, and every abort path returns before rendering.
  • Tests. All new branches are covered (default suggestion, explicit always, preservation on re-run, invalid existing value falling back to on_demand, renderer unit test, empty-managed-args edge), which should keep the 100% coverage gate green.
  • Docs. README, cli.md, and quickstart.md all updated per the issue.

Two tiny non-blocking nits:

  • plugins/inspect-robots-agent/README.md line 222: the amended parenthetical runs long compared to the surrounding ~80-col wrap; consider rewrapping.
  • When an existing config has an invalid images value, the wizard silently drops it and suggests on_demand (tested, and a reasonable choice) — a one-line dim note telling the operator their old value was discarded would be a nice touch, but happy to leave it.

One ask before we can merge: the branch is based on an older main and currently conflicts — _setup.py has moved a fair bit since (the wizard grew several sections). Could you rebase onto the latest main? Worth re-running the setup tests after the rebase since the scripted-input sequences assume the current six [defaults] prompts (they do still match today's suggestions). CI can gate it once the rebase is up, and I'm happy to merge then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setup: offer the agent's on-demand camera mode, defaulting to on_demand

2 participants