evaluator_model was checked against a hardcoded _ALLOWED_EVALUATOR_MO…

[mohamedzeidan2021]

…DELS

LLMAsJudgeEvaluator.evaluator_model was validated against a hardcoded _ALLOWED_EVALUATOR_MODELS dict (model → regions) in sagemaker/train/constants.py. This PR replaces that with two-step validation against authoritative, service-maintained sources, and removes the dict.

Problem

1. Maintenance toil. Every Bedrock judge-model add/deprecation required hand-editing the dict and cutting an SDK release — and the same list is triplicated across the SDK, Studio UI, and SageMaker Agent Skills.
2. Stale list → deep runtime failures. When a judge model reaches end of life the dict still lists it, so it passes client-side validation, the eval job spins up, and only the in-container Bedrock CreateEvaluationJob call fails ("model version has reached end of life"). Compute is wasted and the error surfaces deep inside a running job instead of failing fast.

This is real today: the supported list still advertises claude-3-5-sonnet-20240620, claude-3-5-haiku, claude-3-5-sonnet-v2, and claude-3-7-sonnet, all of which return ResourceNotFoundException from Bedrock in us-west-2.

Solution — two-step validation

Step 1 — construction (is it a judge-capable model?): fetch the service-maintained list at s3://jumpstart-cache-prod-/fmhMetadata/supported-llmaj-judge-models.json (source of truth for supported judge models) and fail fast if evaluator_model isn't in it.

Step 2 — evaluate() (is it still in service?): the supported list is a superset that can still include end-of-life models, so we call bedrock:GetFoundationModel and fail fast if the model is unavailable in the region or past its endOfLifeTime. The lookup is gated on the caller's IAM permission via a new non-raising caller_can_perform() helper that reuses the existing iam:SimulatePrincipalPolicy caller-check pattern (verify_evaluation_caller_permissions).

Graceful degradation everywhere: we never block on "can't tell." If a source can't be read (missing permission, unreadable list, transient error), the SDK logs an actionable warning with a link to the supported models and continues.

User-facing messaging

Fail-fast errors (block construction / submission):

• Not a supported judge model (Step 1):
  ▎ evaluator_model '' is not a supported LLM-as-Judge model in region ''. Choose one of the supported judge models. See https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported
• Past end-of-life (Step 2):
  ▎ evaluator_model '' has reached end of life in region '' (end-of-life ) and can no longer be used as a judge. Choose a judge model that is in service. See https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported
• Unavailable in region (Step 2):
  ▎ evaluator_model '' is not available in region ''. It may be unsupported in this region or have reached end of life. Choose a judge model that is in service in this region. See https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported

Warnings (degrade, do not block):

• Caller's role is missing the permission (Step 2):
  ▎ Your IAM role does not include the bedrock:GetFoundationModel permission, so the SDK can't check whether the evaluator model '' is still in service or has reached end of life. The evaluation will still run, but it may fail if this model has been retired. Add bedrock:GetFoundationModel to your role to enable this check. See the list of supported judge models: https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported
• Permission can't be confirmed — identity isn't a role, or iam:SimulatePrincipalPolicy unavailable (Step 2):
  ▎ The SDK couldn't confirm your IAM role includes the bedrock:GetFoundationModel permission, so it can't check whether the evaluator model '' is still in service or has reached end of life. The evaluation will still run, but it may fail if this model has been retired. See the list of supported judge models: https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported
• Transient Bedrock error while checking lifecycle (Step 2):
  ▎ The SDK couldn't verify whether the evaluator model '' is still in service right now (a temporary error occurred). The evaluation will still run, but it may fail if this model has been retired. See the list of supported judge models: https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported
• Supported-models list unreadable (Step 1):
  ▎ The SDK couldn't retrieve the list of supported judge models, so it can't confirm '' is a valid judge model. The evaluation will still run, but it may fail if the model isn't supported. See the list of supported judge models: https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported

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]

Claude finished @mohamedzeidan2021's task in 3m 13s —— View job

---

Review: replace hardcoded _ALLOWED_EVALUATOR_MODELS with service-backed validation

Thanks for tackling the stale-list problem — the two-step design (construction = "is it a judge model", evaluate() = "is it still in service") is sound, the graceful-degradation-everywhere policy is applied consistently, the triplicated dict removal is clean, and the new paths are well tested. A few things worth a look before merge.

Note: the inline-comment tool isn't available to me in this run, so findings are below with file:line references instead of anchored review comments.

---

1. (medium) The bedrock:GetFoundationModel permission gate can silently skip the EOL check for callers who do hold the permission

sagemaker-train/.../llm_as_judge_evaluator.py:345

allowed = caller_can_perform(["bedrock:GetFoundationModel"], self.sagemaker_session)

bedrock:GetFoundationModel is a resource-scoped action, and caller_can_perform simulates it without ResourceArns — its own docstring warns that "resource-scoped actions can come back implicitDeny even for a caller who holds them" (iam_role_resolver.py:772-775). So any caller whose grant is scoped to specific foundation-model ARNs (rather than * / AmazonBedrockFullAccess) gets allowed is False → the SDK warns and skips the exact EOL check this PR adds, silently defeating step 2 for the security-conscious users most likely to scope their policies.

Meanwhile, a genuine lack of permission is already handled downstream: get_foundation_model would raise AccessDeniedException, which falls through the ResourceNotFoundException/ValidationException branch into the warn-and-continue path at lines 388-397. So the gate mostly buys false negatives plus 3 extra STS/IAM calls (GetCallerIdentity, GetRole, SimulatePrincipalPolicy) per evaluate().

Suggestion: drop the pre-gate and call get_foundation_model directly, mapping AccessDenied* to the "couldn't confirm permission / can't verify" warning. Same UX, no false negatives, fewer API calls. If you keep the gate, simulate with the proper foundation-model ResourceArns so scoped policies resolve correctly.

2. (medium) Construction now performs synchronous S3 network I/O inside a pydantic validator

sagemaker-train/.../llm_as_judge_evaluator.py:292

_validate_evaluator_model does an S3 GetObject against jumpstart-cache-prod-<region> on every LLMAsJudgeEvaluator(...) construction. This changes construction from a cheap in-memory op into a network- and credential-dependent one:

• adds latency to every instantiation;
• other unit tests that construct the evaluator without patching S3Downloader.read_file will now attempt a real network call each time (they'll degrade with a warning, but slow/flaky in CI);
• step-1 validation only runs when sagemaker_session is explicitly passed — if the session is resolved lazily (e.g. via TrainDefaults.get_sagemaker_session), values.get('sagemaker_session') is None and the check always degrades silently.

Consider caching the per-region list (module-level or lru) and/or documenting that construction now touches S3.

3. (low) Step-1 relies on an assumed external JSON schema, failing open if it's wrong

llm_as_judge_evaluator.py:247-262

_fetch_supported_judge_model_ids assumes {"supported_judge_models": [{"model_id": "..."}]}. If the real file's shape ever differs, the parse yields None → warn → continue, so the SDK silently stops validating any model while appearing to. That's the safe failure direction, but given the whole PR is motivated by a stale/wrong list, a brief note (or a log at debug distinguishing "unreadable" from "unexpected shape") would help future debugging.

4. (nit) Duplicated degradation block

llm_as_judge_evaluator.py:388-397 and 398-407 are byte-for-byte identical warning bodies (except ClientError non-matching branch vs except Exception). Both are reachable, just collapsible. Harmless.

---

Nothing here is a hard blocker — the degradation policy means everything fails open. Finding #1 is the one I'd most want addressed, since it can quietly neutralize the new EOL guard for scoped-permission callers. Tests look thorough for the paths that exist; the main untested gap is that evaluate() actually invokes _check_evaluator_model_lifecycle, but that's minor.
· branch llaj-hardcoding

[github-actions]

Claude finished @mohamedzeidan2021's task in 3m 28s —— View job

---

Review: replace hardcoded _ALLOWED_EVALUATOR_MODELS with service-backed validation

• Read the PR diff (/tmp/pr.diff)
• Examine validators, evaluate(), and helpers in context
• Confirm which prior review findings were addressed
• Assess correctness, tests, performance
• Post review

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

The design is sound and the two-step split (construction = "is it a judge model", evaluate() = "is it still in service") is applied consistently, with graceful degradation throughout. The prior review's main concerns look resolved in the latest commit: the caller_can_perform pre-gate is gone and get_foundation_model is now called directly (prior #1), the two degradation blocks are collapsed into a single except Exception with distinct messages (prior #4), and _fetch_supported_judge_model_ids now logs debug distinguishing "unreadable" from "unexpected shape" (prior #3). A few things remain worth a look.

---

1. (medium) get_foundation_model doesn't accept cross-region inference-profile IDs — step 2 can falsely reject a valid judge model

llm_as_judge_evaluator.py:361 / :368

response = client.get_foundation_model(modelIdentifier=self.evaluator_model)
...
if error_code in ("ResourceNotFoundException", "ValidationException"):
    raise ValueError("... is not available in region ...")

bedrock:GetFoundationModel resolves foundation-model IDs/ARNs only. If evaluator_model is ever an inference-profile-style ID (e.g. us.anthropic.claude-..., which some newer models require for on-demand access in a region), Bedrock returns ValidationException/ResourceNotFoundException, and this code maps that to a hard fail-fast "not available in region" — blocking a model that is actually valid. This is a behavior change from the old dict membership check.

Worth confirming: are all entries in supported-llmaj-judge-models.json guaranteed to be plain FM ids (never profile ids)? If not, step 2 should resolve the profile to its base model id before calling get_foundation_model, or treat ValidationException more conservatively (warn rather than block), since a ValidationException can mean "malformed/unsupported identifier" rather than "retired model".

2. (medium) Construction now performs synchronous S3 GetObject inside a pydantic validator

llm_as_judge_evaluator.py:305 (_fetch_supported_judge_model_ids → S3Downloader.read_file)

Because sagemaker_session is always populated (the @validator('sagemaker_session', always=True) default-session creator in base_evaluator.py:419), every LLMAsJudgeEvaluator(...) now issues an S3 read against jumpstart-cache-prod-<region>. Construction went from a cheap in-memory op to a network- and credential-dependent one on the hot path. Consider a module-level / lru_cache per-region cache of the fetched set so repeated constructions (and tuning sweeps) don't re-fetch, and/or document that construction now touches S3.

3. (low) WARNING-level noise on every degrade

llm_as_judge_evaluator.py:308

The "couldn't retrieve the list of supported judge models" path logs at WARNING on every construction where the list can't be read — which includes any caller lacking read access to the JumpStart cache bucket, and GovCloud/China partitions where the jumpstart-cache-prod-<region> bucket naming differs (so it always degrades there). For those users this fires on every instantiation. Consider debug (or warn-once) for the unreadable-list case to avoid steady-state noise, keeping the fail-fast ValueError as the actionable signal.

4. (nit) Stale comment at the evaluate() call site

llm_as_judge_evaluator.py:1016

# ... Gated on caller permissions — warns and continues if it can't be verified. — the pre-gate was removed; step 2 no longer gates on a permission simulation, it calls Bedrock directly and degrades on AccessDenied. Reword to avoid implying a pre-check that no longer exists (the method docstring at :336-341 already describes the new behavior correctly).

---

Tests: thorough for the new paths — supported/unsupported/degrade routes for step 1, and active / future-EOL / past-EOL / not-found / access-denied / transient for step 2, plus a sentinel test that evaluate() invokes the check. Minor gap: no end-to-end test that a past-EOL ValueError actually propagates out of evaluate() (the invocation test patches the method with a sentinel), but that's low priority.

Nothing here is a hard blocker given the fail-open policy; #1 is the one I'd most want confirmed, since it can turn a valid judge model into a hard construction/submission failure.
· branch llaj-hardcoding