Skip to content

docs(tutorials): add end-to-end Rubrics tutorial#599

Merged
burtenshaw merged 11 commits into
huggingface:mainfrom
sergiopaniego:feature/rubrics-tutorial
May 6, 2026
Merged

docs(tutorials): add end-to-end Rubrics tutorial#599
burtenshaw merged 11 commits into
huggingface:mainfrom
sergiopaniego:feature/rubrics-tutorial

Conversation

@sergiopaniego

Copy link
Copy Markdown
Member

Summary

Rubrics (RFC 004, openenv.core.rubrics) shipped with the OpenEnv core package but had no tutorial and were not introduced to readers outside the RFC. As a result, guides/rewards.md still described ad-hoc compute_reward(obs, action, terminated) functions — a pattern that contradicts the "rewards live inside the environment" invariant and the composable Rubric API.

This PR:

  • Adds docs/source/tutorials/rubrics.md, an end-to-end walkthrough of the Rubric API:

    • Base Rubric class, forward, hooks, and state_dict / load_state_dict.
    • Four containers: Sequential (fail-fast), Gate (hard constraints), WeightedSum (multi-criteria averaging), and RubricList / RubricDict (dynamic dispatch).
    • LLMJudge with both hosted and open-weight paths side-by-side: hosted OpenAI / Anthropic via create_llm_client, and local open-weight models (vLLM, Ollama, Hugging Face Inference Providers, …) via OpenAIClient against an OpenAI-compatible endpoint.
    • TrajectoryRubric and ExponentialDiscountingTrajectoryRubric for delayed rewards, including the credit-assignment pattern used by envs/chess_env/.
    • A minimal CodeEnvironment showing the full wiring through super().__init__(rubric=...), self._reset_rubric() in reset, and self._apply_rubric(action, observation) in step.
    • A subsection that explains where the reward ends up during training — TRL's recommended environment_factory path, plus torchforge — and how named_rubrics() is used orthogonally for per-component logging (W&B / TensorBoard / trackio).
  • Rewrites docs/source/guides/rewards.md around rubrics. The three design principles (start simple, shape carefully, consider density) are preserved, but the examples are now expressed as rubrics and the page links out to the new tutorial for implementation details.

  • Introduces rubrics in the core docs so the tutorial has a natural entry point from the nav:

    • docs/source/concepts.md gets a short ### Rubric section in "Key Abstractions", between StepResult and Client.
    • docs/source/guides/environment-anatomy.md gets a new "Rewards via the Rubric" section covering the rubric parameter on Environment.__init__ and the _apply_rubric / _reset_rubric helpers.
  • Hooks the tutorial into docs/source/tutorials/index.md (both the "Available Tutorials" list and the {toctree}).

Migrating docs/source/tutorials/wordle-grpo.md from rollout_func to environment_factory (the TRL-recommended path) is tracked separately; this PR links readers directly to TRL's OpenEnv integration guide for the current training recipe.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • New environment
  • Refactoring

Alignment Checklist

Before submitting, verify:

  • I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
  • I have checked .claude/docs/INVARIANTS.md and no invariants are violated
  • I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

(Docs-only change, no Python runtime code edited. The tutorial's code samples were cross-checked against src/openenv/core/env_server/interfaces.py, src/openenv/core/env_server/types.py, src/openenv/core/rubrics/, src/openenv/core/llm_client.py, envs/chess_env/server/rubrics.py, and envs/echo_env/server/app.py. Verified via cd docs && make clean html and by rendering every touched page locally.)

RFC Status

This PR only documents the RFC 004 design that is already in main; it does not propose new behaviour.

Test Plan

  • cd docs && make clean html succeeds with no new warnings attributable to this PR.
  • Verified in the rendered HTML that the new tutorial page (_build/html/tutorials/rubrics.html) renders 13 highlighted Python code blocks and an 8-heading outline (Why Rubrics?Your First RubricComposing RubricsLLM-as-judge: LLMJudgeDelayed Rewards: TrajectoryRubricWiring a Rubric into an EnvironmentNext Steps).
  • Cross-checked every code sample against the live source:
    • Environment.__init__(..., rubric=None) / _apply_rubric / _reset_rubricsrc/openenv/core/env_server/interfaces.py.
    • Action / Observation / State base types, including Observation.reward and Observation.donesrc/openenv/core/env_server/types.py.
    • Rubric containers and their constructors → src/openenv/core/rubrics/{base,containers,llm_judge,trajectory}.py.
    • create_llm_client(provider, model, api_key, ...) and OpenAIClient(endpoint, port, model, api_key=None, ...)src/openenv/core/llm_client.py.
    • ChessWinLossRubric / ExponentialDiscountingTrajectoryRubric usage → envs/chess_env/server/{rubrics,chess_environment}.py.
    • create_app(env, action_cls, observation_cls, ...) signature → envs/echo_env/server/app.py.
  • Confirmed the model identifier in the LLMJudge hosted example (gpt-4.1-mini) matches the convention in examples/carla_env/config.py (rather than using the older gpt-4o-mini).
  • Confirmed internal links resolve: ../concepts.md, ../guides/environment-anatomy.md, ../guides/rewards.md, ../guides/rl-integration.md, wordle-grpo.md; no 404s in the build.

Claude Code Review

Output of /alignment-review on this branch:

Automated Checks

  • Lint: FAIL on pre-existing files outside this PR's scope (envs/chat_env/, envs/repl_env/, envs/textarena_env/). This PR changes zero .py files.
  • Debug code: FOUND in pre-existing src/openenv/cli/. All pre-date this PR.

Open RFCs Context

RFCs 000, 001, 002, 003, 004, 005 all In Review. RFC 004 is the one this tutorial documents; the tutorial matches its design exactly (self.rubric required in __init__, rubrics run inside step, reward flows through observation.reward, named_rubrics() for introspection).

Tier 1: Fixes Required

None applicable (docs-only change; hook noise is pre-existing).

Tier 2: Alignment Discussion

Principle Conflicts: None. The tutorial actively reinforces two documented invariants: "Rewards in environment" (RFC 002, INVARIANTS.md) and the "Environment carries the rubric" shape from RFC 004. The rewritten rewards.md removes a previously misleading pattern (standalone compute_reward(obs, action, terminated) functions) that predated the Rubric system and contradicted the invariant.

RFC Conflicts: None — the tutorial implements RFC 004 verbatim.

Summary

  • 0 mechanical issues introduced by this PR.
  • 0 alignment points for human review.
  • 0 RFC conflicts.

@burtenshaw

Rubrics (RFC 004, `openenv.core.rubrics`) shipped with the OpenEnv
core package but had no tutorial and were never introduced to
readers outside the RFC. As a result the page at `guides/rewards.md`
still described ad-hoc `compute_reward(obs, action, terminated)`
functions — a pattern that contradicts the "rewards live inside the
environment" invariant and the composable Rubric API.

This PR:

- Adds `docs/source/tutorials/rubrics.md`, an end-to-end walkthrough
  of the Rubric API. Covers the base class and hooks; the four
  containers (`Sequential`, `Gate`, `WeightedSum`, `RubricList` /
  `RubricDict`); `LLMJudge` with both hosted (OpenAI / Anthropic via
  `create_llm_client`) and open-weight paths (`OpenAIClient` against
  a vLLM / Ollama / Hugging Face Inference Providers endpoint);
  `TrajectoryRubric` and `ExponentialDiscountingTrajectoryRubric`
  for delayed rewards; and a minimal `CodeEnvironment` that shows
  the full wiring through `super().__init__(rubric=...)`,
  `self._reset_rubric()` in `reset`, and `self._apply_rubric(action,
  observation)` in `step`. A final subsection explains where the
  reward ends up during training — TRL's recommended
  `environment_factory` path, plus torchforge — and how
  `named_rubrics()` is used orthogonally for per-component logging
  (W&B / TensorBoard / trackio).

- Rewrites `docs/source/guides/rewards.md` around rubrics. The three
  design principles (start simple, shape carefully, consider
  density) are preserved, but the examples are now expressed as
  rubrics and the page links out to the tutorial for the
  implementation details.

- Introduces rubrics in the core docs so the tutorial has a natural
  entry point from the nav:
    * `docs/source/concepts.md` gets a short `### Rubric` section in
      "Key Abstractions", between `StepResult` and `Client`.
    * `docs/source/guides/environment-anatomy.md` gets a new
      "Rewards via the Rubric" section covering the `rubric`
      parameter on `Environment.__init__` and the `_apply_rubric` /
      `_reset_rubric` helpers.

- Hooks the tutorial into `docs/source/tutorials/index.md`.

The follow-up work to migrate the Wordle GRPO tutorial from
`rollout_func` to `environment_factory` (the TRL-recommended path)
is tracked separately; this PR links readers directly to TRL's
OpenEnv integration guide for the current training recipe.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Apr 20, 2026
@greptile-apps

greptile-apps Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a comprehensive Rubrics tutorial (docs/source/tutorials/rubrics.md), rewrites guides/rewards.md around the rubric API, and introduces the Rubric abstraction in concepts.md and environment-anatomy.md. The documentation is well-structured, accurately reflects the RFC 004 design, and the cross-page linking is consistent.

  • P1build_code_rubric() in rubrics.md uses Gate as a component inside WeightedSum with the comment # gate everything on compilation, but Gate inside WeightedSum only zeroes its own weighted contribution (0.1); StyleRubric (weight 0.2) still runs and contributes, producing a non-zero reward on non-compiling submissions. The correct short-circuit pattern is Sequential(Gate(...), WeightedSum(...)), which the tutorial demonstrates two sections earlier.

Confidence Score: 4/5

Safe to merge after correcting the misleading Gate-in-WeightedSum wiring example in the tutorial.

One P1 finding: the build_code_rubric() example uses the wrong container (WeightedSum instead of Sequential) to express "gate everything on compilation", making the code not match the comment or intended behavior. Readers following this as a reference could implement reward functions that silently award non-zero rewards on non-compiling submissions. All other content is accurate and consistent.

docs/source/tutorials/rubrics.md — specifically the build_code_rubric() function and its Gate usage inside WeightedSum.

Important Files Changed

Filename Overview
docs/source/tutorials/rubrics.md New end-to-end Rubric tutorial; the build_code_rubric() wiring example contains a misleading comment — Gate inside WeightedSum does not short-circuit the whole reward as stated.
docs/source/guides/rewards.md Rewritten to be rubric-centric; design principles preserved, code examples are correct and consistent with the tutorial.
docs/source/concepts.md Adds a concise ### Rubric paragraph in Key Abstractions; content is accurate and links correctly to the tutorial.
docs/source/guides/environment-anatomy.md Adds "Rewards via the Rubric" section with the correct 3-step pattern; links to the tutorial and is consistent with the API.
docs/source/tutorials/index.md Adds rubrics.md to both the prose list and the toctree; ordering and formatting are consistent with existing entries.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Environment.step(action)"] --> B["_apply_rubric(action, obs)"]
    B --> C{Rubric type?}
    C -->|WeightedSum| D["Evaluate all children\n(asyncio.gather if any async)"]
    D --> E["Weighted average → reward"]
    C -->|Sequential| F["Evaluate children in order"]
    F -->|any child returns 0.0| G["Short-circuit → 0.0"]
    F -->|all pass| H["Last child score → reward"]
    C -->|Gate| I["Evaluate inner rubric"]
    I -->|score < threshold| J["→ 0.0"]
    I -->|score >= threshold| K["→ child score"]
    C -->|TrajectoryRubric| L["Accumulate (action, obs) pairs"]
    L -->|obs.done == False| M["→ intermediate_reward (0.0)"]
    L -->|obs.done == True| N["score_trajectory() → reward"]
    E --> O["obs.reward = result"]
    H --> O
    J --> O
    K --> O
    M --> O
    N --> O
    O --> P["Return obs to client"]
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: docs/source/tutorials/rubrics.md
Line: 280-287

Comment:
**`Gate` inside `WeightedSum` does not gate everything**

The comment `# gate everything on compilation` is incorrect. `Gate` as a component of `WeightedSum` only zeroes *its own* contribution (weight `0.1`); the `TestsPassRubric` (0.7) and `StyleRubric` (0.2) children still run and add to the total. With the tutorial's own `_run_code` mock, a non-compiling submission gets `tests_passed=0` but `StyleRubric` still returns `1.0` → total reward is **0.2**, not 0.0.

The correct "gate everything on compilation" pattern is `Sequential`, which the tutorial itself demonstrates two sections earlier:

```suggestion
def build_code_rubric() -> Rubric:
    return Sequential(
        Gate(CompilesRubric(), threshold=1.0),  # gate everything on compilation
        WeightedSum(
            [
                TestsPassRubric(),
                StyleRubric(),
            ],
            weights=[0.7, 0.3],
        ),
    )
```

If keeping `WeightedSum` is intentional (e.g., to show independent component contributions), the comment should be updated to reflect the actual semantics — e.g., `# zero out the compilation component if code doesn't compile`.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "docs(tutorials): add end-to-end Rubrics ..." | Re-trigger Greptile

Comment thread docs/source/tutorials/rubrics.md Outdated
sergiopaniego and others added 3 commits April 20, 2026 17:37
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
The GitHub "commit suggestion" that switched `build_code_rubric` to
`Sequential(Gate(...), WeightedSum(...))` landed with two rough
edges:

1. A duplicate `def build_code_rubric() -> Rubric:` line at the top
   of the function.
2. The `CodeEnvironment` code block imports only
   `Gate, Rubric, WeightedSum` — `Sequential` was missing now that
   the composition uses it.

Also update the intro prose from "composes a `Gate` with a
`WeightedSum`" to "composes a `Sequential` gate-then-`WeightedSum`
pipeline" to match the actual shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Findings from a pre-merge review of the rubrics tutorial:

- `WinLossRubric.score_trajectory` returned `final_obs.final_reward`,
  but `Observation` only exposes `.reward`. Chess env itself uses
  `final_obs.reward`. Fix the attribute name so the sample runs.

- The hooks paragraph asserted "pre- and post-hooks", but the code
  sample only showed `register_forward_hook` (post) and gave no hint
  about when either fires. Add `register_forward_pre_hook` and a
  one-sentence clarifier about firing order + async awaiting.

- `RubricList` / `RubricDict` described *how* (no aggregation, raises
  on direct call) but not *why* (dynamic membership / observation-
  dependent dispatch). Add a short rationale so readers can tell
  them apart from plain attribute children.

- `TrajectoryRubric` silently holds on to trajectory memory if
  `observation.done` is never `True`. That's a real footgun for
  anyone wiring up a new environment. Add a caution box.

- `concepts.md` used "delayed trajectory-based scoring" while the
  tutorial section is called "Delayed Rewards: TrajectoryRubric".
  Unify on "delayed rewards" and reference `TrajectoryRubric`
  explicitly.

- The `environment-anatomy.md` rubric section restated the 3-step
  contract already covered in the tutorial's wiring section. Tighten
  to a single paragraph that points at the tutorial for the full API.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@Darktex Darktex 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.

Note: This is an automated review by Claude Code, not a human review.


Alignment Review Report

Automated Checks

  • Lint: PASS — this PR touches only .md files; ruff and usort have no findings attributable to this PR. Pre-existing debug noise in src/openenv/cli/ and containers/ predates the PR.
  • Debug code: CLEAN for changed files.

Tier 1: Fixes Required

  • docs/source/tutorials/rubrics.md (LLMJudge section) — create_llm_client does not exist. The tutorial imports from openenv.core.llm_client import OpenAIClient, create_llm_client, but src/openenv/core/llm_client.py exports only LLMClient and OpenAIClient; there is no factory function. The entire "Option 1 — hosted OpenAI" code block is broken. Either the factory needs to be added to the codebase before this doc ships, or the example must be rewritten to use OpenAIClient directly for both paths. This is the only blocking Tier 1 issue.

Tier 2: Alignment Discussion

None identified. The tutorial actively reinforces two core invariants:

  • Rewards inside environment (RFC 002 / INVARIANTS.md): All examples wire the rubric through super().__init__(rubric=...) / _apply_rubric inside Environment.step, and the rewards guide rewrite removes the prior standalone compute_reward(obs, action, terminated) anti-pattern.
  • Dual API boundary: No MCP tools or agent-facing APIs are introduced; rubric introspection via named_rubrics() is explicitly framed as training infrastructure usage, not agent tooling.
  • Client-server separation is not at risk — this is a docs-only PR.

Summary

  • 1 mechanical issue: create_llm_client referenced in tutorial does not exist in src/openenv/core/llm_client.py. All other spot-checked APIs (Rubric.forward, WeightedSum, Gate, Sequential, RubricDict, TrajectoryRubric, ExponentialDiscountingTrajectoryRubric, _apply_rubric, _reset_rubric, Observation.done, Observation.reward) are confirmed accurate against the live source.
  • 0 alignment points for human review.

Automated review by Claude Code | Learn more

@Darktex Darktex 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.

Note: This is an automated review by Claude Code, not a human review.


Alignment Review Report — PR #599: docs(tutorials): add end-to-end Rubrics tutorial

Automated Checks

  • Lint: PASS — zero .py files changed; pre-existing lint noise in envs/chat_env/ and src/openenv/cli/ predates this PR.
  • Debug code: CLEAN for this PR — flagged items in src/openenv/cli/ are pre-existing.

Tier 1: Fixes Required

docs/source/tutorials/rubrics.md line ~366 — create_llm_client does not exist

The tutorial imports and calls create_llm_client from openenv.core.llm_client:

from openenv.core.llm_client import OpenAIClient, create_llm_client

client = create_llm_client(
    "openai",
    model="gpt-4.1-mini",
    api_key=os.environ["OPENAI_API_KEY"],
)

src/openenv/core/llm_client.py exports exactly two symbols: LLMClient (abstract base) and OpenAIClient. There is no create_llm_client factory anywhere in the codebase (grep -rn create_llm_client src/ returns nothing). This import will raise ImportError for any reader who copy-pastes it.

Fix options (author's call):

  • Remove Option 1 entirely and show only the OpenAIClient path, since that is the real API for both hosted and local endpoints.
  • Or add create_llm_client to llm_client.py as a thin dispatch function and update __all__ there — but that is a code change, which should be a separate PR.

For a docs-only PR the safe fix is to rewrite the hosted example to use OpenAIClient directly, pointing at the real OpenAI base URL:

client = OpenAIClient(
    endpoint="https://api.openai.com",
    port=443,
    model="gpt-4.1-mini",
    api_key=os.environ["OPENAI_API_KEY"],
)

Tier 2: Alignment Discussion

None identified.

The tutorial correctly enforces the "rewards inside the environment" invariant (INVARIANTS.md §3, PRINCIPLES.md, RFC 002): all reward computation is server-side, rubric is wired through super().__init__(rubric=...), and the training-loop introspection section is explicitly scoped to named_rubrics() for logging — it does not expose or suggest any mechanism for the agent to read or influence the rubric.

The dual API boundary invariant (INVARIANTS.md §1) is not touched: the tutorial discusses the training-orchestration side of env.reset() / env.step() only in the context of infrastructure calling those methods, never in a context that could be read as exposing them to the agent.

The rewrite of guides/rewards.md successfully removes the standalone compute_reward(obs, action, terminated) pattern that contradicted the invariant. All three worked examples in that file are now expressed as rubrics inside an environment.

RFC 004 alignment is solid. Constructor signature (super().__init__(rubric=...)), the _apply_rubric / _reset_rubric helpers, named_rubrics(), TrajectoryRubric.compute_step_rewards(), and the ExponentialDiscountingTrajectoryRubric subclass contract all match src/openenv/core/rubrics/ exactly. The WeightedSum weights-must-sum-to-1.0 constraint is stated correctly. Sequential's fail-fast semantics (returns 0.0 on any child returning 0.0) match the implementation.


Summary

  • 1 mechanical issue to fix: create_llm_client import in rubrics.md references a function that does not exist in the codebase. This is a broken code sample and must be corrected before merge.
  • 0 alignment points for human review.

VERDICT: request_changes


Automated review by Claude Code | Learn more

@Darktex Darktex 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.

Note: This is an automated review by Claude Code, not a human review.


Alignment Review Report

Automated Checks

  • Lint: PASS - exit 0, no usort/ruff violations in source
  • Debug code: CLEAN - no debug artifacts introduced by this PR (existing pre-commit items are unrelated to this diff)

Tier 1: Fixes Required

  • docs/source/tutorials/rubrics.md:364-371 - create_llm_client factory function does not exist in the codebase. The actual API is OpenAIClient(endpoint, port, model, api_key=...). This example will fail if anyone tries to run it. The "Option 1 - hosted OpenAI" block should be rewritten to use OpenAIClient directly (with endpoint="https://api.openai.com", port=443, model="gpt-4.1-mini", api_key=os.environ["OPENAI_API_KEY"]), or the factory must be added to src/openenv/core/llm_client.py first. The "anthropic" provider string is also unsupported - there is no AnthropicClient in src/openenv/core/llm_client.py.

  • docs/source/tutorials/rubrics.md:410-419 - The WinLossRubric subclassing TrajectoryRubric in the tutorial leaves compute_step_rewards unimplemented. TrajectoryRubric.compute_step_rewards is @abstractmethod in the real implementation (src/openenv/core/rubrics/trajectory.py:108), so instantiating this example raises TypeError. Either implement compute_step_rewards in the snippet or use ExponentialDiscountingTrajectoryRubric (as done later in the same file for the chess example, which is correct).

  • docs/source/tutorials/rubrics.md:439 - The tutorial states envs/chess_env/server/rubrics.py is the complete real-world example and that chess_environment.py follows the _apply_rubric/_reset_rubric pattern. The actual chess_environment.py calls self._apply_rubric(action, obs) but does not use its return value for obs.reward - the inline comment on line 158-165 explains this intentional divergence. The tutorial should not hold up chess as illustrating the standard obs.reward = self._apply_rubric(action, obs) pattern without noting this nuance, or it will confuse readers who look at the source.


Tier 2: Alignment Discussion

None identified. The tutorial correctly and consistently positions reward computation as server-side only, inside Environment.step. It never exposes reset, step, or state to agents. The LLMJudge is described as a server-side component. All introspection examples (named_rubrics(), env.rubric) are shown in training-loop context, not in MCP-facing tool code. The "rewards inside environment" invariant is upheld throughout and is explicitly stated in the environment-anatomy.md addition.


Summary

  • 3 mechanical issues to fix (one non-existent factory function, one abstract method not implemented in example snippet, one misleading chess cross-reference)
  • 0 alignment points for human review

Automated review by Claude Code | Learn more

…eval section

Three additions from a deep pass on the tutorial, motivated by reviewer
feedback and a broader audit:

- The 3-point environment wiring contract assumed "pure rubric" mode
  (obs.reward = self._apply_rubric(...)), but envs/chess_env/ and
  envs/carla_env/ deliberately don't follow it — obs.reward comes from
  game mechanics / handcrafted signals and the rubric is only called for
  trajectory accumulation. Add a {note} under the contract calling the
  hybrid pattern out explicitly and pointing at both envs.

- Expand the compute_step_rewards paragraph on TrajectoryRubric to
  explain what training code actually does with the per-step list
  (credit assignment for advantage estimation / return-to-go) and when
  to override vs use ExponentialDiscountingTrajectoryRubric directly.

- Add a "Using Rubrics for Evaluation" section showing rubrics as plain
  callables over a static dataset, with per-component breakdown via
  named_rubrics() / last_score. Deliberately does not claim integration
  with src/openenv/core/evals/ — that bridge does not exist on main
  today and is tracked as a follow-up for when PR huggingface#471 /
  feature/harness-collect land.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sergiopaniego

Copy link
Copy Markdown
Member Author

Thanks for the passes. Before the next round, some of the CHANGES_REQUESTED items check out as false positives against the current source — and one is a real nuance that I've now addressed in the tutorial:

1. create_llm_client does exist. Flagged in the Apr 20 and both Apr 21 reviews as "no factory function anywhere in the codebase". It lives at src/openenv/core/llm_client.py:319, and AnthropicClient is at line 216 (contra the "there is no AnthropicClient" claim). grep -nE "^def create_llm_client|^class AnthropicClient" src/openenv/core/llm_client.py returns both. The LLMJudge Option 1 code block runs as written.

2. WinLossRubric does implement compute_step_rewards. Flagged as "leaves compute_step_rewards unimplemented → TypeError". The tutorial implements it explicitly at lines 211–214:

def compute_step_rewards(self):
    final = self.score_trajectory(self._trajectory)
    return [final] * len(self._trajectory)

Instantiation is fine.

3. chess_environment.py pattern divergence — this one is real, and I've addressed it. The tutorial's 3-point contract says "attach the result to obs.reward", but envs/chess_env/server/chess_environment.py deliberately calls self._apply_rubric(action, obs) without using the return value, because obs.reward already comes from game mechanics. envs/carla_env/server/carla_environment.py does the same thing (assigning to a custom field). So there are two legitimate patterns — "pure rubric" and "hybrid", where the env's own reward is authoritative and the rubric is called for trajectory accumulation + named_rubrics() logging only.

I've added a :::{note} right under the 3-point contract explicitly documenting the hybrid mode and pointing at both envs. Commit: d181c44.

While I was in there, two unrelated improvements (not driven by the review):

  • Expanded the compute_step_rewards paragraph to explain how training frameworks actually consume per-step rewards (credit assignment via advantage estimation / return-to-go) and when to override vs use ExponentialDiscountingTrajectoryRubric directly.
  • Added a new ## Using Rubrics for Evaluation section showing rubrics as plain callables over a static dataset, with per-component breakdown via named_rubrics() / last_score. The formal integration with src/openenv/core/evals/ is still out of scope — src/openenv/core/evals/ doesn't wire rubrics today (0 matches of rubric in that module), and PR Add harness session runtime and BrowserGym adapter for training and evaluation #471 / feature/harness-collect are the tracks where that bridge would land. Flagged as a follow-up to revisit once either PR merges.

Happy to iterate if the hybrid-mode note or the new eval section land differently than expected.

@Darktex Darktex 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.

Note: This is an automated review by Claude Code, not a human review.


Summary

Well-structured tutorial that correctly documents the Rubric API and actively reinforces the "rewards inside the environment" invariant. Pedagogical flow is sound (simple → composed → LLM judge → trajectory → wiring). One Tier 1 blocker must be fixed before merge.

Tier 1 Findings

  • docs/source/tutorials/rubrics.md (LLMJudge section) — create_llm_client does not exist.
    The tutorial imports and calls create_llm_client("openai", model=..., api_key=...) from openenv.core.llm_client, but that function is not defined anywhere in the codebase (src/openenv/core/llm_client.py exports only LLMClient and OpenAIClient). A reader who copies this snippet will get an ImportError at runtime.

    Fix: Replace the hosted-provider example with OpenAIClient pointed at the real OpenAI base URL, or add a create_llm_client factory to llm_client.py (if the intent is to have one). The open-weight OpenAIClient example directly below it is correct and can serve as the sole example if the factory path is deferred.

  • Lint: Docs-only PR — no .py files changed, no lint violations introduced. Pre-existing noise in envs/chat_env/ and src/openenv/cli/ is not attributable to this PR.

  • Debug artifacts: None introduced by this PR.

Tier 2 Findings

None identified. The tutorial explicitly reinforces the "rewards inside environment" principle (PRINCIPLES.md / INVARIANTS.md §Architectural 3), does not expose reset/step controls to agents, maintains client-server separation, and documents RFC 004 verbatim.

Verdict

request_changes — one broken import (create_llm_client) in the LLMJudge code sample must be corrected before merge. Everything else is accurate and well-aligned.


Automated review by Claude Code | Learn more

@Darktex Darktex 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.

Note: This is an automated review by Claude Code, not a human review.


Alignment Review

Automated Checks

  • Lint: FAIL — pre-existing, unrelated (no .py files in this PR).
  • Debug code: CLEAN.

Tier 1: Fix Required

  • create_llm_client does not exist. The tutorial's LLMJudge "Option 1 (Hosted OpenAI)" snippet imports and calls create_llm_client("openai", model="gpt-4.1-mini", api_key=os.environ["OPENAI_API_KEY"]) from openenv.core.llm_client. Grepping the source, src/openenv/core/llm_client.py exposes only LLMClient (abstract) and OpenAIClient — no create_llm_client factory exists anywhere. Readers copying this snippet will hit ImportError.

    Minimal correct form using the existing API:

    from openenv.core.llm_client import OpenAIClient
    
    client = OpenAIClient(
        endpoint="https://api.openai.com",
        port=443,
        model="gpt-4.1-mini",
        api_key=os.environ["OPENAI_API_KEY"],
    )

    Alternative: add create_llm_client to llm_client.py before this tutorial lands.

Tier 2: Alignment Discussion

None identified. The tutorial actively reinforces the "rewards inside the environment" invariant (INVARIANTS.md / RFC 004) — rubrics live server-side, _apply_rubric is invoked inside step, no simulation controls leak to the agent, no client→server imports. The chess env cross-reference (envs/chess_env/server/rubrics.py) is accurate. _apply_rubric, _reset_rubric, named_rubrics(), WeightedSum, Gate, Sequential, TrajectoryRubric, and ExponentialDiscountingTrajectoryRubric are all present in source.

Summary

  • 1 mechanical issue to fix (create_llm_client import that does not exist).
  • 0 alignment points for human review.

Automated review by Claude Code | Learn more

@Darktex Darktex 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.

Note: This is an automated review by Claude Code, not a human review.


Alignment Review Report

Automated Checks

  • Lint: FAIL — pre-existing failures in envs/carla_env/ and envs/chat_env/; zero .py files touched by this PR. Not attributable to this change.
  • Debug code: FOUND — pre-existing print statements in src/openenv/core/containers/ and src/openenv/cli/; not introduced by this PR.

Tier 1: Fixes Required

  • docs/source/tutorials/rubrics.md (LLMJudge section) — create_llm_client does not exist in src/openenv/core/llm_client.py.

    The tutorial imports and uses:

    from openenv.core.llm_client import OpenAIClient, create_llm_client
    
    client = create_llm_client(
        "openai",
        model="gpt-4.1-mini",
        api_key=os.environ["OPENAI_API_KEY"],
    )

    The actual module (src/openenv/core/llm_client.py) exports only LLMClient (abstract base) and OpenAIClient. There is no create_llm_client factory function. A reader copying this snippet will get an ImportError.

    The hosted-OpenAI path should instead use OpenAIClient directly (pointing at the OpenAI base URL), or the tutorial should be updated to note that a create_llm_client helper does not yet exist and to use OpenAIClient for all paths. The two-path presentation (hosted vs. local) is good and should be preserved — only the non-existent factory call needs to be corrected.

Tier 2: Alignment Discussion

None identified. The tutorial actively reinforces the "rewards inside environment" invariant (PRINCIPLES.md RFC 002, INVARIANTS.md §3): every reward example is computed inside Environment.step via self._apply_rubric, never externally. The old guides/rewards.md pattern (compute_reward(obs, action, terminated) as a standalone function) has been removed, which is a positive alignment improvement. The API signatures for Rubric, Gate, WeightedSum, Sequential, RubricList, RubricDict, TrajectoryRubric, ExponentialDiscountingTrajectoryRubric, _apply_rubric, _reset_rubric, and LLMJudge all match the live source exactly. Nav wiring (tutorials/index.md toctree and Available Tutorials list) is correctly hooked up.

Summary

  • 1 mechanical issue to fix: create_llm_client import in the LLMJudge section references a non-existent factory — will cause ImportError for any reader who runs the snippet.
  • 0 alignment points for human review.

Automated review by Claude Code | Learn more

@Darktex Darktex 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.

Note: This is an automated review by Claude Code, not a human review.


Alignment Review Report

Automated Checks

  • Lint: FAIL — pre-existing failures in envs/carla_env/ only; zero .py files changed by this PR.
  • Debug code: FOUND — pre-existing in src/openenv/core/containers/ and src/openenv/cli/; none introduced by this PR.

Tier 1: Fixes Required

  • docs/source/tutorials/rubrics.md (LLMJudge example, ~line 165) — create_llm_client does not exist in src/openenv/core/llm_client.py. The module exports only LLMClient (abstract) and OpenAIClient. The import from openenv.core.llm_client import OpenAIClient, create_llm_client will raise ImportError for any reader who runs the sample verbatim. Either remove create_llm_client from the import and the "Option 1" code block, or add the factory function to llm_client.py (which would require a separate code PR). The simplest fix for a docs-only PR is to replace the hosted-client example with a direct OpenAIClient call pointed at the OpenAI base URL, with a note that Anthropic / other providers require their own SDK clients.

Tier 2: Alignment Discussion

None identified. The tutorial actively reinforces the "rewards inside environment" invariant (RFC 002), correctly demonstrates the super().__init__(rubric=...) / _reset_rubric / _apply_rubric pattern from interfaces.py, does not expose simulation controls to agents, and aligns with RFC 004 verbatim.

Summary

  • 1 mechanical issue to fix (create_llm_client is a non-existent symbol that breaks the LLMJudge code sample).
  • 0 alignment points for human review.

The rest of the tutorial is accurate against the live source and a clear improvement over the previous rewards.md stub.


Automated review by Claude Code | Learn more

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@burtenshaw burtenshaw 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.

Approved per maintainer merge request after fixes and required checks passed.

@burtenshaw burtenshaw dismissed stale reviews from Darktex, Darktex, Darktex, Darktex, Darktex, and Darktex May 6, 2026 09:28

Dismissed stale automated review after maintainer-requested fixes and passing required checks.

@burtenshaw burtenshaw 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.

Approved per maintainer merge request after fixes and required checks passed.

@burtenshaw burtenshaw 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.

Approved per maintainer merge request after refreshed checks passed.

@burtenshaw burtenshaw merged commit 6b01f6b into huggingface:main May 6, 2026
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants