Skip to content

feat(rollout): surface approver interventions to policy via observati… - #217

Open
Adityakk9031 wants to merge 2 commits into
robocurve:mainfrom
Adityakk9031:feature/187-surface-approver-interventions
Open

feat(rollout): surface approver interventions to policy via observati…#217
Adityakk9031 wants to merge 2 commits into
robocurve:mainfrom
Adityakk9031:feature/187-surface-approver-interventions

Conversation

@Adityakk9031

Copy link
Copy Markdown
Contributor

Description

Fixes #187 by surfacing safety approver interventions (such as clamped and delta_clamped rewrites) directly to policies in observation.extra["approvals"].

Previously, when an approver rewritten an action, the modification was recorded into the trial's event stream in EvalLog, but never surfaced to policy.act(). This meant LLM/VLA policies had to infer clamps from proprioceptive drift alone, often misattributing undershoot to dynamics/stiction and re-issuing the same out-of-bounds action.

Proposed Changes

  • Core (inspect_robots):

    • In rollout.py, track approval events in trial store under _APPROVALS_KEY.
    • Pass the accumulated list of approval intervention records ({"t": t, "detail": detail}) into obs.extra["approvals"] for every step when calling controller.next_action(...).
    • Added unit test test_rollout_surfaces_approvals_in_observation_extra in tests/test_rollout_hardening.py.
  • Agent Policy (inspect_robots-agent):

    • In _observation_content(), render observation.extra.get("approvals") as a prompt text line (e.g. approver: 3 step(s) modified (clamped, delta_clamped).).
    • Allows LLM agent models to recognize safety gate interventions and adjust motion/targets rather than retrying blindly.
    • Added unit test test_observation_content_renders_approver_interventions in plugins/inspect-robots-agent/tests/test_policy_e2e.py.

Verification

  • Ran test suite for core: pytest tests/ (861 passed).
  • Verified agent policy observation content rendering tests.

@Adityakk9031

Copy link
Copy Markdown
Contributor Author

@aris-zhu and @jeqcho have a look

@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 for taking this on — the mechanism is the right shape (rollout mirrors approval events into the store, snapshots them into extra before next_action, agent policy renders a line), the per-trial store created at src/inspect_robots/rollout.py:192 means nothing leaks across trials/epochs, and both new tests are well targeted (nice touch asserting the first inference sees [] in tests/test_rollout_hardening.py::test_rollout_surfaces_approvals_in_observation_extra). Both touched test files pass locally (152 passed). A few things need addressing before merge:

1. The rendered approvals line grows unboundedly — plugins/inspect-robots-agent/src/inspect_robots_agent/policy.py:922-929

_approvals_line joins one detail string per intervention with no aggregation. In exactly the scenario #187 describes — a policy repeatedly issuing out-of-bounds actions against a ClampApprover — a few hundred clamped steps produce approver: 400 step(s) modified (clamped, clamped, clamped, …) with hundreds of repeated tokens, injected into every subsequent observation. Please aggregate by flag, e.g. collections.Counter over the details, rendering something like approver: 400 step(s) modified (clamped ×398, delta_clamped ×2). — and extend test_observation_content_renders_approver_interventions to cover duplicate flags.

2. Cumulative-since-reset semantics diverge from the issue and dilute the signal — src/inspect_robots/rollout.py:220

#187 proposes surfacing interventions "from the steps since the last act()". The PR instead passes the full trial history on every step. Beyond size, this weakens the actionable signal: what the model needs to know is "was my latest motion clamped?", and under cumulative semantics that is drowned out once early interventions accumulate (the line eventually reads the same whether or not the most recent chunk was touched). The rollout already tracks inference boundaries via prev_inferences / _INFER_KEY (lines 219, 232), so windowing is cheap — e.g. record the approvals-list length at each inference and pass only the tail since the previous one. If you have a deliberate reason to prefer cumulative (t fields do let a sophisticated policy diff), please say so on the issue and document the chosen semantics; but for the agent-policy rendering, "since your last decision" is the more useful contract.

3. Missing CHANGELOG entry

CONTRIBUTING.md (step 4) asks for a CHANGELOG.md entry under "Unreleased". This is a policy-visible contract addition, so it belongs under "Added", referencing #187/#217 in the style of the existing entries.

4. Document the newly reserved extra key — src/inspect_robots/types.py:32-35

The Observation docstring currently says the rollout "injects the current step into the policy-facing observation's extra["env_step"] and reserves that key; embodiments should not set it." Since rollout.py:223 now also unconditionally sets (and would silently overwrite) extra["approvals"], please extend that docstring to reserve "approvals" the same way, including its shape (list of {"t": int, "detail": str | None}) and the fact that it is always present, possibly empty.

Minor (optional):

  • approvals = list(store.get(_APPROVALS_KEY, [])) at rollout.py:220 shallow-copies the list, but the entry dicts are shared by reference across every observation for the rest of the trial (and with the store). A policy that mutates an entry would corrupt what later inferences see. Copying the dicts (or storing immutable tuples) would harden this cheaply.
  • _approvals_line's defensive isinstance checks are welcome given extra is an open channel; no change needed there.

Nothing here is structural — the core wiring is sound and matches the issue's intent. Happy to re-review once the rendering aggregation, windowing decision, changelog, and docstring are in. Thanks again for the contribution! 🙏

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.

Surface approver interventions to the policy via observation.extra

2 participants