Skip to content

Fix OpenRewardSpec omitting task‑scoped tools during rollout binding (fixes #5727) - #5729

Merged
adithya-s-k merged 5 commits into
huggingface:mainfrom
rycerzes:feat/openreward-example
May 25, 2026
Merged

Fix OpenRewardSpec omitting task‑scoped tools during rollout binding (fixes #5727)#5729
adithya-s-k merged 5 commits into
huggingface:mainfrom
rycerzes:feat/openreward-example

Conversation

@rycerzes

@rycerzes rycerzes commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix bug where OpenRewardSpec omitted task‑scoped tools during rollout binding.

Changes

  • Core fix (addresses OpenReward binding only discovers shared /tools, misses task-specific /task_tools #5727)trl/experimental/openreward/_spec.py now correctly discovers and binds task‑specific tools
    • Previously, only shared tools (from GET /{env_name}/tools) were bound.
    • This PR adds a discover_task_tools flag (default True) that opens a short‑lived session and calls session.list_tools() (which returns the union of shared and task‑specific tools).
    • If discovery fails, it falls back to the shared‑only list (non‑breaking).
    • The new task_tools_discovery_index parameter allows manual control over which task index is used for the probe.
  • Supporting changes
    • New helper _tool_specs_from_session_list_tools() to normalise the SDK output.
    • Extended docstrings to explain the discovery behaviour.
    • Updated tests (tests/experimental/_openreward_echo_env.py, tests/experimental/test_openreward.py) to verify that task‑scoped tools are bound and that disabling discovery works as expected.

Related Issues

Fixes #5727

Testing

  • Verified that a GRPO trainer now sees and can call both echo and hint task‑scoped tools.
  • Added unit tests to validate the discovery fallback and the configurable discover_task_tools=False behaviour.

CC: @adithya-s-k


Note

Medium Risk
Changes tool discovery/binding logic to open probe sessions and merge tool specs, which could affect rollout initialization behavior and performance or expose differing tool schemas across tasks.

Overview
Fixes OpenRewardSpec tool binding to include task-scoped ORS tools by optionally probing session.list_tools() (ORS /task_tools) during spec initialization and merging discovered ToolSpecs by name, with fallback to shared-only environment.list_tools() on errors.

Adds discover_task_tools (default True) and task_tools_discovery_index to control/limit probing (including multi-index probing when indices is provided), and updates the experimental echo env + tests to validate binding of a session-only hint tool and the opt-out behavior.

Reviewed by Cursor Bugbot for commit 8f3d02c. Bugbot is set up for automated code reviews on this repo. Configure here.

- `HintParams`, and `list_task_tools` method to expose the hint tool during task tool discovery.
- update `OpenRewardSpec` to support task-specific tool discovery
@rycerzes
rycerzes force-pushed the feat/openreward-example branch from a0f3fe8 to 6c7d590 Compare May 11, 2026 14:37
@rycerzes rycerzes changed the title Add OpenReward GRPO notebook and fix task‑specific tool binding (fixes #5727) Fix OpenRewardSpec omitting task‑scoped tools during rollout binding (fixes #5727) May 11, 2026
@rycerzes rycerzes closed this May 11, 2026
@rycerzes
rycerzes deleted the feat/openreward-example branch May 11, 2026 14:43
@rycerzes
rycerzes restored the feat/openreward-example branch May 11, 2026 14:43
@rycerzes rycerzes reopened this May 11, 2026
Comment thread trl/experimental/openreward/_spec.py
Comment thread trl/experimental/openreward/_spec.py
Comment thread trl/experimental/openreward/_spec.py
- refactor task tools probe indices
Comment thread trl/experimental/openreward/_spec.py
@rycerzes
rycerzes force-pushed the feat/openreward-example branch from 5de4652 to f48d157 Compare May 11, 2026 17:09

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f48d157. Configure here.

Comment thread trl/experimental/openreward/_spec.py
@adithya-s-k

Copy link
Copy Markdown
Collaborator

Amazing work @rycerzes 🙌 — nice catch on the /tools vs /task_tools distinction, that's a detail I missed when reading the SDK. Repro in #5727 is super clean too. The default-on discover_task_tools with a graceful shared-only fallback is exactly the right shape.

Two small suggestions if you want to tighten it before merge (otherwise this LGTM):

  1. When indices= is set and task_tools_discovery_index isn't, discovery opens one probe session per distinct index — could be O(N) HTTP sessions if someone passes a large indices= list. Might be worth defaulting to single-probe (index 0) and gating the multi-probe behavior behind an explicit flag.
  2. The discovery-failure warning logs the raw exception, and the probe session is opened with secrets= (which includes the API key). Low-risk, but if the SDK ever embeds request headers in error reprs the key could surface in logs — narrower exception handling or scrubbing the message would be defensive.

Neither is blocking. Thanks for cleaning this up so quickly 🚀

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@rycerzes

Copy link
Copy Markdown
Contributor Author

Thanks for the review @adithya-s-k!

  1. Multi-probe is intentional since the ORS contract makes list_task_tools() per-task, so tool sets can genuinely differ across indices. Single-probe would silently miss tools in heterogeneous rollouts. Users with uniform tasks can already opt in via task_tools_discovery_index=.
  2. The SDK doesn't embed secrets in exception reprs today, so we're holding off on defensive scrubbing for now, easy to revisit if it ever becomes real.

I have fixed the failing lint issue, the other tests failing is pre-existing and is not introduced by this branch. All good to merge as-is. 🙌

@adithya-s-k

Copy link
Copy Markdown
Collaborator

@qgallouedec , have a look at this and let me know your thoughts

@adithya-s-k
adithya-s-k merged commit 9e80cab into huggingface:main May 25, 2026
5 of 6 checks passed
adithya-s-k added a commit to adithya-s-k/trl that referenced this pull request Jun 11, 2026
Train on Harbor agentic task suites with GRPOTrainer via environment_factory.
HarborSpec maps one task suite to the three trainer slots (train_dataset /
environment_factory / reward_funcs), mirroring the OpenReward integration
(huggingface#5752, huggingface#5729, huggingface#5696). HarborEnv wraps a Harbor sandbox + verifier; the base
agent (harness) is pluggable — built-in `bash`, plus `jupyter` and
`terminal_notes` example harnesses (folder-per-harness, each with a README).

HarborEnv follows Harbor's *external agent* pattern (the policy drives the loop
and tool methods exec into the sandbox); Harbor's *installed agents* are not
supported, since RL needs the trainer to drive generation and capture the
policy's tokens/log-probs, which an opaque in-container agent can't expose.

- trl/experimental/harbor: HarborEnv (+ HarborBashEnv) and HarborSpec
- examples/scripts/harbor: data_agent.py + harnesses/
- docs/source/harbor.md (+ toctree, example_overview entries)
- tests/experimental/test_harbor.py (require_harbor + is_harbor_available)
- pyproject: add `harbor` extra; relax vllm cap to >=0.22.0 (0.19 pins
  transformers<5, which breaks environment_factory; it needs transformers>=5.2)

E2B's from_dockerfile build honors RUN but silently drops COPY'd build-context
files, so HarborEnv replicates the Dockerfile's COPY directives at runtime
(upload as the sandbox user, mv into place as root) — healthchecks that run
those files (e.g. a data-pull hook) then work. Verified end-to-end: gpt-4.1
over the bash harness lands reward=1.0 on a data-agent task.
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.

OpenReward binding only discovers shared /tools, misses task-specific /task_tools

3 participants