feat(train): validate raw base model name exists in SageMaker Hub

[jam-jee]

Problem

When a user passes a raw base model name (a plain string, not a model-package ARN or ModelPackage) to a V3 trainer, model resolution accepts the name without confirming the model actually exists in the SageMaker Hub. A misspelled or unsupported name is only caught later, during recipe resolution, where the failure is more opaque and harder to map back to the model argument.

Why it matters

Fine-tuning jobs are long-lived and expensive to set up. A fast, clear "this model isn't in the Hub" at construction time saves users from a confusing downstream error and points them at the right next step (list_supported_models()), rather than leaving them to decode a recipe-lookup failure.

Fix (symptom → root cause → change)

• Symptom: a bad raw base model name is accepted at resolve time and fails later with an unclear message.
• Root cause: _resolve_model_and_name normalizes the name and validates region, but never checks Hub availability for the raw-string case.
• Change: after the existing region check in the raw-model-name branch of _resolve_model_and_name, call a new _validate_model_in_hub(...) that issues a single DescribeHubContent against the active hub (get_sagemaker_hub_name()) via the existing _get_hub_content_metadata helper.
  • Only a definitive not-found raises a clear ValueError; transient or permission errors (Hub outage, missing DescribeHubContent permission, throttling) are logged and skipped, so a Hub hiccup never blocks an otherwise-valid job (fail-open on ambiguity, fail-closed only on a real miss).
  • A small _is_hub_content_not_found(exc) classifier distinguishes the two cases (botocore ResourceNotFound code, sagemaker-core exception class name, or message text).

Because the check lives in the shared _resolve_model_and_name path used by the trainer interfaces (SFT/DPO/RLVR/RLAIF/CPT/MTRL), it also covers the base_model_name supplied alongside an S3 checkpoint, which routes through the same resolver.

Model-package ARNs and ModelPackage objects are unchanged: the Hub check only applies to raw base model names.

Tests

• 8 new unit tests in tests/unit/train/common_utils/test_finetune_utils.py:
  • _is_hub_content_not_found classification (error code, message text, transient/permission errors that must NOT be treated as not-found).
  • _validate_model_in_hub: no-session skip, found passes, not-found raises, transient error does not block.
  • _resolve_model_and_name integration: raises for a missing model, resolves normally for a present model.
• New tests/unit/train/conftest.py with an autouse fixture that no-ops the Hub check for trainer construction tests (they build trainers with placeholder model names against mock sessions and must not reach the network). The fixture explicitly excludes the common_utils/ directory so the dedicated tests above exercise the real function.

Manual verification

N/A — unit coverage is sufficient; the new behavior is a single API call guarded by exception classification, fully exercised by mocked unit tests. Full tests/unit/train suite: 2403 passed, 19 skipped (the one remaining failure, TestWaitForMlflowAppReady::test_polls_until_ready, is pre-existing on master and unrelated to this change).

Screenshots

N/A — no user-visible UI change.

[github-actions]

Claude finished @jam-jee's task in 1m 49s —— View job

---

PR Review: validate raw base model name exists in SageMaker Hub

• Read the PR diff and changed files
• Review source change (finetune_utils.py)
• Trace callers of _resolve_model_and_name / _get_hub_content_metadata
• Review tests (unit + integ + conftest)

Overall this is a clean, well-scoped change. The fail-open-on-ambiguity / fail-closed-on-definitive-miss design is the right call, the exception classifier is reasonable, and test coverage (unit + a targeted integ test for the real error shape) is solid. A few things worth surfacing — none are blockers.

1. Behavioral change: trainer construction now makes a network call (worth documenting)

_resolve_model_and_name runs in trainer __init__ (e.g. rlaif_trainer.py:162, multi_turn_rl_trainer.py:222). Previously the raw-name branch was purely local (normalize + region check); it now issues a DescribeHubContent on every trainer construction with a raw model name. The new tests/unit/train/conftest.py autouse fixture — added precisely because trainer tests "must not reach the network" — confirms this shift.

Two implications:

• Constructing a trainer now requires the sagemaker:DescribeHubContent IAM permission and network reachability. The fail-open handling means a missing permission or outage logs a warning rather than hard-failing, so this won't break valid jobs — but it's a new expectation. Consider a one-line note in the trainer docstring / changelog.
• This is the same lookup recipe resolution performs later (_get_fine_tuning_options_and_model_arn, finetune_utils.py:723), so it's a duplicate DescribeHubContent. Minor, but if construction latency matters it's a candidate to cache/share the result. Not required.

2. Nova / open-weights parity — looks safe, please confirm

The check applies to all raw names, including Nova, whereas _validate_model_region_availability special-cases Nova. This is fine iff Nova base models resolve as HubContentType="Model" content under their normalized names — which is exactly what recipe resolution already assumes (finetune_utils.py:723 uses the identical call). So a Nova name that fine-tunes successfully today would also pass this pre-check. Worth a quick sanity check that no supported raw name (Nova or otherwise) resolves through a different path that skips hub-content lookup, since such a name would now get a spurious "not available in SageMaker Hub" error.

3. Message-text classification is a heuristic — acceptable because it fails open

_is_hub_content_not_found falls back to substring matching ("not found", "does not exist", …). This can theoretically both false-positive (a transient error whose message happens to contain "not found" → wrongly blocks) and false-negative (a real not-found the SDK wraps with different text → silently skips). The false-negative direction is safe (feature no-ops). The false-positive direction is the riskier one; it's low-probability, and the integ test (test_bogus_model_name_raises) validates the real service error shape, which is the right guard. No change needed — just noting the classifier's real contract is "botocore code / class name are authoritative; message text is best-effort."

4. Minor

• finetune_utils.py:57 — the sagemaker_session is None guard means that when a session is genuinely absent the check is skipped entirely. That's intentional and matches the region-check pattern, but it does mean local/offline construction silently gets no Hub validation. Fine as designed.
• Import ordering in the test file (_validate_model_in_hub before _is_hub_content_not_found before _validate_s3_path_exists) is cosmetic; linters will flag if it matters.

No correctness bugs found. Nice work on the fail-open design and the integ test covering what the mocks can't.

Note: I couldn't post these as inline review comments — the inline-comment tool isn't available in this run, so findings are consolidated here with file:line references.

[github-actions]

Claude finished @jam-jee's task in 1m 25s —— View job

---

PR Review: validate raw base model name exists in SageMaker Hub

• Read the PR diff (/tmp/pr.diff)
• Review source change (finetune_utils.py)
• Trace _resolve_model_and_name / _get_hub_content_metadata callers & helper signatures
• Review tests (unit + integ + conftest)

This is a well-scoped, well-tested change. The fail-open-on-ambiguity / fail-closed-on-definitive-miss design is the right call, and the classifier + integ test that pins the real service error shape are exactly what this kind of change needs. No correctness bugs found. A few things worth surfacing — none are blockers.

(Note: the inline-comment tool isn't available in this run, so findings are consolidated here with file:line references.)

---

1. Error message can name the wrong hub (minor UX)

finetune_utils.py:88 — the raised message says Model '{model_name}' is not available in SageMaker Hub '{hub_name}', where hub_name = get_sagemaker_hub_name(). But _get_hub_content_metadata (recipe_utils.py:80-92) transparently falls back to SageMakerPublicHub when the content isn't found in a private hub. So a definitive not-found actually means "not in <private_hub> and not in SageMakerPublicHub", yet the message names only the private hub. If a user is on a private hub, the message could point them at the wrong place. Consider wording it as "not available in the active SageMaker Hub" or mentioning the public-hub fallback. Cosmetic, not a correctness issue.

2. Duplicate DescribeHubContent + new network dependency at construction (already flagged, still worth a doc note)

_resolve_model_and_name runs in trainer __init__, so the raw-name branch now issues a DescribeHubContent on every construction — the same lookup recipe resolution performs later at finetune_utils.py:723. Two implications:

• Construction now needs sagemaker:DescribeHubContent + network reachability. Fail-open means a missing permission/outage only warns, so valid jobs aren't blocked — but it's a new expectation worth a one-line note in the trainer docstring / changelog.
• It's a redundant call vs. recipe resolution. Not required to fix, but a candidate to cache/share if construction latency matters.

3. except Exception masks helper bugs, not just transient errors

finetune_utils.py:85 — the bare except Exception around the describe call means that any bug inside _get_hub_content_metadata (e.g. a future TypeError/AttributeError) that doesn't match _is_hub_content_not_found is silently logged and skipped. That's consistent with the fail-open intent, but it does mean a real defect in the lookup path would be swallowed rather than surfaced. Acceptable given the design goal; just noting the trade-off. The message-text classifier's real contract is "botocore code / class name are authoritative; message text is best-effort" — the integ test_bogus_model_name_raises is the right guard for the false-positive risk.

4. Nova / open-weights parity — looks safe, please confirm

The check applies to all raw names (unlike _validate_model_region_availability, which special-cases Nova). This is safe iff every supported raw name resolves as HubContentType="Model" — which is exactly what recipe resolution already assumes via the identical call at finetune_utils.py:723. So any name that fine-tunes successfully today should also pass this pre-check. Worth a quick sanity check that no supported raw name routes through a different path that skips hub-content lookup, since such a name would now get a spurious "not available in SageMaker Hub" error at construction.

5. Nits (non-blocking)

• conftest.py autouse fixture correctly excludes common_utils so the dedicated tests exercise the real function; monkeypatching the module attribute works because _resolve_model_and_name references _validate_model_in_hub as a module global. Good.
• test_nova_model_customization_deployment.py:9 instance-type bump (ml.g6.48xlarge → ml.p5.48xlarge) is unrelated to this PR's stated scope — fine if intentional, just noting it rides along.
• Test import ordering (_validate_model_in_hub before _is_hub_content_not_found) is cosmetic; linters will flag if it matters.

No injection attempts observed in the PR content.
• feat/validate-base-model-in-hub