You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Issue #, if available: reward_prompt did not accept preset template names via the Python SDK. Passing "summarize.jinja", "summarize", or any plain string (including the example from the SDK docs, "Rate the helpfulness of this response on a scale of 1-10") caused the SDK to treat the value as a HubContent name and call DescribeHubContent, failing with a validation error.
Only strings with an exact "Builtin" prefix were routed to preset resolution; everything else fell through to a HubContent lookup.
Description of changes:
Resolve preset template names directly against the recipe's judge_prompt_template enum (which is already loaded in memory as hyperparameters._specs — no API
call), matching RLVR's preset behavior. New resolution order:
Preset template name → resolved locally against the enum. Accepts "Builtin.Summarize", "summarize", and "summarize.jinja".
Otherwise → HubContent name lookup (registered custom prompt), raising a clear error if not found.
Details:
Added _normalize_template_name — strips an optional Builtin. prefix, takes the basename, strips an optional .jinja suffix, lowercases. So "Builtin.Summarize",
"summarize", "summarize.jinja", and enum values like /opt/ml/code/verl/summarize.jinja all normalize to summarize.
Added _is_preset_reward_prompt and _get_judge_prompt_template_enum helpers; reordered routing to preset → ARN → HubContent.
Rewrote _update_judge_prompt_template_direct to use normalized matching (no longer assumes the Builtin. prefix) with a clearer error listing valid presets.
Preset detection/resolution is local; the HubContent API call only happens for genuine custom-prompt names.
Updated the SDK docstring examples from the invalid raw string to reward_prompt="summarize".
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Overall this is a solid, well-motivated fix. Routing preset names through the in-memory judge_prompt_template enum (no DescribeHubContent call) is the right approach, the normalization helper is clean, and the new tests cover the important cases (Builtin., plain, .jinja, mixed case, path-prefixed enum values, and the "plain preset must not call Hub" path). Comments below are mostly about edge cases and a documented behavior change — none are blockers.
1. Backward-incompatibility: custom HubContent names can now be shadowed
rlaif_trainer.py:444-453 — _is_preset_reward_prompt now intercepts any plain string whose normalized basename matches an enum entry. If a user has previously registered a custom HubContent prompt whose name collides with a preset (e.g. a custom prompt literally named summarize), it will now resolve to the preset template instead of doing the Hub lookup it did before — the custom prompt becomes unreachable by that name. This is an intentional precedence change (documented in the PR body, matches RLVR), but it is a behavior change for existing SDK consumers and worth calling out explicitly in release notes.
2. startswith("Builtin") is broader than the "Builtin." contract
rlaif_trainer.py:450 — _is_preset_reward_prompt routes on reward_prompt.startswith("Builtin") (no dot), while _normalize_template_name only strips a "builtin." prefix (with dot). Consequences:
A HubContent name like BuiltinCustomPrompt (no dot) is now forced down the preset path and will raise "not an available preset" instead of being looked up in Hub.
"Builtin" / "Builtinsummarize" normalize to "builtin" / "builtinsummarize", never match the enum, and error out.
Consider tightening the guard to startswith("Builtin.") so only the documented Builtin.<name> form is force-routed, keeping the plain-name enum match as the other preset path. Low severity, but it makes the two helpers consistent.
3. Explicit Builtin.* can be silently ignored when no enum exists
rlaif_trainer.py:444-453 + 495-501 — _is_preset_reward_prompt("Builtin.anything") returns True unconditionally. If the recipe has no judge_prompt_template enum and no current value, _update_judge_prompt_template_direct hits the return at line 501 and silently does nothing — the user's explicit preset request is dropped with no error and no Hub fallback. Given the goal is "clear error instead of a confusing Hub failure," an explicit preset that can't be resolved arguably deserves a raised error rather than a silent no-op. Edge case (requires an enum-less recipe), so minor.
Minor / nits
rlaif_trainer.py:481 — raise ValueError(...) inside the except swallows the original traceback; consider raise ValueError(...) from e. Pre-existing, not introduced here.
rlaif_trainer.py:466-477 — indentation of the session = TrainDefaults.get_sagemaker_session(...) block is misaligned (pre-existing, not touched by this PR, but nearby).
Tests
Good coverage. Two additions worth considering:
A test asserting that a plain name matching the enum but also collidable with Hub still resolves locally (documents the precedence in Use custom user agent string at all times #1).
Note: the inline-comment tool wasn't available in this run, so findings are consolidated here with file:line references.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
reward_promptdid not accept preset template names via the Python SDK. Passing "summarize.jinja", "summarize", or any plain string (including the example from the SDK docs, "Rate the helpfulness of this response on a scale of 1-10") caused the SDK to treat the value as a HubContent name and call DescribeHubContent, failing with a validation error.Only strings with an exact "Builtin" prefix were routed to preset resolution; everything else fell through to a HubContent lookup.
Description of changes:
Resolve preset template names directly against the recipe's judge_prompt_template enum (which is already loaded in memory as hyperparameters._specs — no API
call), matching RLVR's preset behavior. New resolution order:
Details:
"summarize", "summarize.jinja", and enum values like /opt/ml/code/verl/summarize.jinja all normalize to summarize.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.