fix: revert preset reward function deletion from hyperparams dict by mujtaba1747 · Pull Request #6181 · aws/sagemaker-python-sdk

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

mujtaba1747 · 2026-08-13T22:52:18Z

RLVR training needs a reward signal. The old code accepted preset_reward_function on the hyperparameters object, then silently deleted it during processing — leaving jobs to fail downstream (or run without a valid reward).

This PR:

Preserves the preset_reward_function hyperparameter through _process_hyperparameters so it actually reaches the job.
Fails fast, client-side, with a clear message when no reward source is configured, instead of relying on a later server-side failure.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2026-08-13T22:52:52Z

Claude finished @mujtaba1747's task in 1m 40s —— View job

Review: PR #6181 — revert preset reward function deletion from hyperparams dict

Read PR diff
Analyze rlvr_trainer.py changes
Check test coverage & other affected paths
Post review

The change is small and correct. It stops _process_hyperparameters from stripping preset_reward_function, so a Hub-supplied or user-set preset now actually reaches the training job, and it adds a fail-fast guard in train() requiring at least one reward signal. This is a genuine bug fix (previously a preset was silently deleted before submission).

Observations

✅ Correctness

The validation is placed before the compute-type dispatch, so it applies to serverless, SMTJ, and HyperPod paths — good.
getattr(self.hyperparameters, "preset_reward_function", None) safely handles both a missing attribute and self.hyperparameters is None (beta-session fallback). Good.
The error message is clear and actionable, and the docstring Raises: note is accurate.

⚠️ Backward compatibility (minor, likely intended)

train() now raises ValueError when neither custom_reward_function nor preset_reward_function is set. This is stricter than before. It's an improvement (previously such a job would submit with no reward signal and fail server-side), but it is a behavioral change — worth a line in release notes if any consumer relied on Hub defaults populating the preset. If the Hub spec provides a default preset_reward_function, getattr returns it (truthy) and validation passes, so that path is unaffected.

⚠️ Test coverage gap (main feedback)

Only an integ test line was added (test_rlvr_trainer_integration.py:96), which is @pytest.mark.gpu_intensive and won't run in normal CI.
There's no unit test for the new guard. Note that the existing unit tests in tests/unit/train/test_rlvr_trainer.py set self.hyperparameters to a bare Mock(), so getattr(..., "preset_reward_function", None) returns an auto-created (truthy) Mock, which means the new validation passes only by accident and is effectively untested at the unit level.
Suggest adding a unit test that asserts train() raises ValueError when neither reward signal is configured — e.g. a hyperparameters object whose preset_reward_function is None/absent and custom_reward_function=None. This also guards the regression from reappearing:

deftest_train_raises_when_no_reward_signal(self, ...):
mock_hyperparams=Mock(spec=[]) # no preset_reward_function attrmock_hyperparams.to_dict.return_value= {}
mock_finetuning_options.return_value= (mock_hyperparams, "model-arn", False)
trainer=RLVRTrainer(model="test-model", model_package_group="test-group",
training_dataset="s3://bucket/train") # no custom_reward_functionwithpytest.raises(ValueError, match="requires a reward signal"):
trainer.train(wait=False)

Nit

test_rlvr_trainer_integration.py:95 introduces a blank line with trailing whitespace; linters may flag it.

Overall this looks good to merge; the only substantive ask is a unit test for the new validation so it's actually exercised in CI.
• master-rlvr-hyperparam-bugfix

…R cases a reward signal (#6207) * fix(train): give shallow RLVR cases the reward signal RLVR requires RLVRTrainer.train() refuses to submit unless custom_reward_function was passed or hyperparameters.preset_reward_function is set. TestRLVRTrainerSubmission inherits the shared cases from RecipeTrainerCases, which pass neither -- they are about recipe rendering and dataset handling, not reward configuration -- so 14 of the class's 17 tests failed: 12 raising the ValueError, and the two negative cases failing with "rejected, but not for the expected reason" because the reward error preempted the S3 validation error they assert on. Set the preset in a build() override rather than repeating it in each test, and skip it when the test supplies its own custom_reward_function so the three reward-function variants still exercise exactly what they name. "prime_code" is one of the values the recipe's preset_reward_function enum accepts ('', gsm8k, prime_code, prime_math) and is what the deep suite pairs with an ordinary training dataset on this same model. This was not a regression from a later change to sagemaker-train. The guard landed in #6181 on 2026-08-14, five days before the shallow suite merged (#6176), and rlvr_trainer.py is unchanged since. The suite had simply never run in CI: the fast-integ-tests job could not check out fork PR code, and because pull_request_target runs the base branch's workflow it could not have run on #6176 itself either. Verified against us-west-2 in the SDK test account: 14 passed in 94s, each submitting and immediately stopping a real training job. --- X-AI-Prompt: Fix the failing shallow sagemaker-train RLVR integ tests, which were being rejected at submission for a missing reward signal X-AI-Tool: claude-code * ci: run fast-integ-tests in CodeBuild instead of on the runner Replaces the runner-based shallow suite with a CodeBuild invocation, so the suite gates fork PRs -- which is nearly all of them. The job stopped working when actions/checkout began refusing to place fork PR code in a pull_request_target job. That refusal is correct: the runner holds the base repo's GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py and read those credentials out. On a public repo, overriding it with allow-unsafe-pr-checkout would be a live credential-exfiltration path. Guarding the job to same-repo PRs would stop the failure, but 59 of the last 60 merged PRs here are from forks, so that leaves ~2% coverage. This is the real fix: start CodeBuild with source-version-override, exactly as the codestyle-doc-tests, unit-tests and integ-tests jobs already do. The build never sees the runner's token, secrets or default-branch cache, so no same-repo guard is needed. Its own project rather than folding into sagemaker-train-integ-tests, so a shallow failure stays distinguishable from a deep-suite failure and runs concurrently with it rather than queueing behind it. Dropped the upload-artifact step: the JUnit XML no longer exists on the runner, and results are in the CodeBuild logs. Tradeoff recorded in both the workflow comment and the suite README: the pytest selection now lives in createCIShallowIntegBuildSpec in SageMakerMLFPySDKInfraCDK, so changing how the suite is invoked is no longer reviewable in a PR to this repo. Adding a test file under shallow/ is still picked up automatically. The project sagemaker-python-sdk-ci-sagemaker-train-fast-integ-tests is deployed, so the job resolves on merge. --- X-AI-Prompt: Instead of the GitHub runner, run the sagemaker-train shallow integ suite in CodeBuild like the other CI workflows, so fork PRs are gated after actions/checkout began refusing fork PR code in pull_request_target X-AI-Tool: claude-code

fix: rever preset reward function deletion from hyperparams dict
22056fe

mujtaba1747 deployed to auto-approve August 13, 2026 22:52 — with GitHub Actions Active

jam-jee approved these changes Aug 13, 2026
View reviewed changes