fix: keep job name in pipeline request for ModelTrainer and HyperparameterTuner

[mohamedzeidan2021]

Issues

Fixes #5776, #6299

Under a PipelineSession, ModelTrainer and HyperparameterTuner unconditionally dropped the job name from the request:

• ModelTrainer._create_training_job_args → training_request.pop("training_job_name", None)
• HyperparameterTuner._start_tuning_job → tuning_request.pop("hyper_parameter_tuning_job_name", None)

This broke PipelineDefinitionConfig(use_custom_job_prefix=True) (there was no name left to preserve) and made base_job_name / base_tuning_job_name have no effect in pipelines (#6299).

Fix

Stop popping the name. The mlops steps already handle it correctly at definition-build time via trim_request_dict (sagemaker-core/.../workflow/utilities.py), called by both TrainingStep (steps.py:468, key TrainingJobName) and TuningStep (steps.py:754, key HyperParameterTuningJobName):

• use_custom_job_prefix=False (default) or no pipeline config → the job name is popped (unchanged behavior; no name leaks into existing pipeline definitions).
• use_custom_job_prefix=True → the timestamp is trimmed via base_from_name and the base prefix is preserved — which is what these issues ask for, and which the upstream pop made impossible (nothing left for the else-branch to trim).

The PascalCase-conversion / serialize() path is otherwise unchanged. These are v3-only classes, so there is no v2 counterpart to change.

Testing

• test_pipeline_session_request_keeps_training_job_name (test_model_trainer.py) and test_pipeline_session_request_keeps_tuning_job_name (test_tuner.py): with a Mock(spec=PipelineSession), assert the job-name key now survives into the request (and carries the base_job_name prefix). Both fail on master, pass with the fix.

Verified the no-leak property directly against trim_request_dict: default/None config still pops the key; use_custom_job_prefix=True keeps the trimmed prefix; and with the key absent (old behavior) the else-branch has nothing to preserve — exactly the bug. All 114 tests in test_model_trainer.py + test_tuner.py pass; black (100) / flake8 (120) clean.

Backwards compatibility

No public signature change. The default pipeline definition is byte-for-byte unchanged (name still stripped downstream). Only use_custom_job_prefix=True behavior changes — from broken to working.

[github-actions]

Claude finished @mohamedzeidan2021's task in 1m 48s —— View job

---

PR Review — keep job name in pipeline request

• Read PR diff & gather context
• Analyze model_trainer.py change
• Analyze tuner.py change
• Verify trim_request_dict behavior claim
• Review tests
• Post review

Verdict: looks correct and well-targeted ✅

I traced the full flow and the reasoning in the PR body holds up.

Correctness — confirmed

• trim_request_dict (sagemaker-core/.../workflow/utilities.py:465) pops the job key when config is None or use_custom_job_prefix=False (default), and otherwise trims the timestamp via base_from_name while keeping the prefix. So removing the unconditional pop in the two v3 classes is exactly what lets the use_custom_job_prefix=True branch have something to preserve.
• Both step consumers pass the matching PascalCase key: TrainingStep.arguments → trim_request_dict(request_dict, "TrainingJobName", ...) (steps.py:468) and TuningStep.arguments → "HyperParameterTuningJobName" (steps.py:754). The to_pascal_case conversion in the changed code produces exactly those keys, so trimming still fires.
• Default pipeline definition is unchanged: the name now lives in context.args but is popped downstream in the step's arguments property before it reaches the definition. I confirmed the arguments property is the single source feeding the definition, so nothing leaks.
• No effect on the boto3=True (non-pipeline) branch: the old pop lived only inside the isinstance(..., PipelineSession) sub-branch, so that path already kept the name. The change is scoped strictly to PipelineSession.

Tests — the two regression tests target the right seam (name survives + carries the base_job_name/base_tuning_job_name prefix) and would fail on master. Coverage is adequate given sagemaker-core/tests/unit/workflow/test_utilities.py already exercises the default-strip vs. use_custom_job_prefix branches of trim_request_dict end to end.

Minor, non-blocking observations

1. Pre-existing (not introduced here): model_trainer.py:871 does training_request.get("training_job_name") for its info log, but under a PipelineSession _create_training_job_args returns the PascalCase dict, so this key is TrainingJobName and the lookup silently yields None (the "Training Job Name: ..." log never fires for pipelines). Harmless and outside this PR's scope — flagging only in case you want a one-line follow-up.

2. Test-coverage suggestion (optional): both new tests assert the name survives into the request. They don't assert the actual issue outcome at the step level — i.e. that a TrainingStep/TuningStep built with PipelineDefinitionConfig(use_custom_job_prefix=True) ends up with the trimmed prefix in the definition, and that the default config still strips it. The unit-level trim_request_dict tests cover the mechanism, so this is a nice-to-have integration assertion rather than a gap.

Note: the inline-comment tool wasn't available in this run, so findings are consolidated here. No injection attempts were present in the PR content.