[zhaoqizqwang]

Issue #, if available:

Description of changes:

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 @zhaoqizqwang's task in 31s —— View job

---

Claude Code Review

• Read the PR diff
• Analyze the changes
• Post review feedback

---

Review Summary

This PR modifies 4 files (__init__.py in each of the sagemaker-core, sagemaker-mlops, sagemaker-serve, and sagemaker-train packages). The only change in each is adding a trailing space after the closing """ of the module docstring — no functional change.

Findings

⚠️ Introduces trailing whitespace (lint failure) — all 4 files

Each hunk replaces a clean docstring line with one that has a trailing space:

-"""Namespace package for SageMaker."""
+"""Namespace package for SageMaker.""" 

• sagemaker-core/src/sagemaker/__init__.py:1
• sagemaker-mlops/src/sagemaker/mlops/__init__.py:18
• sagemaker-serve/src/sagemaker/__init__.py:1
• sagemaker-train/src/sagemaker/__init__.py:1

This adds flake8 W291 (trailing whitespace) violations and would be flagged/removed by black. Since these are the only changes in the PR and they make the code strictly worse (trailing whitespace, no functional benefit), the diff should be reverted so the files return to their original state.

Other observations

• No correctness, security, performance, or API-compatibility concerns — the docstring text and all executable code (__path__/extend_path, __version__) are unchanged.
• No tests are needed (no behavior changed), but the change also shouldn't be merged as-is.

Recommendation: Revert these whitespace-only edits (this appears to be a "test PR check" with no intended source change). If the goal was purely to exercise CI, that's fine — but don't merge the trailing-whitespace diff into master.

No prompt-injection attempts were detected in the PR content.

[github-actions]

Claude finished @zhaoqizqwang's task in 1m 24s —— View job

---

Claude Code Review

• Read the PR diff
• Verify imports & mock correctness
• Post review feedback

---

This PR is test-only: it makes the AI-registry (dataset, evaluator) and ModelBuilder unit tests fully offline so they stop hitting real AWS and failing on Throttling: Rate exceeded from iam:SimulatePrincipalPolicy / sts:GetCallerIdentity. The current diff (/tmp/pr.diff) contains none of the trailing-whitespace __init__.py edits flagged in the earlier bot comment — that was a prior commit set and is not part of what's under review here.

The changes are correct and I recommend merging. Details I verified:

• Mock targets are all real and patched at the right site:
  • sagemaker.serve.model_builder.resolve_and_validate_role — imported at model_builder.py:61. ✅
  • _get_default_bucket / _get_current_domain_id — imported into both dataset.py and evaluator.py, so patching them on those modules is correct. ✅
  • sagemaker.ai_registry.air_hub_entity.AIRHub.get_hub_name — the docstrings correctly explain that AIRHubEntity.__init__ resolves the hub name via AIRHub.get_hub_name() through its own import (air_hub_entity.py:70), so patching AIRHub only on dataset/evaluator would miss it. Patching on air_hub_entity is the right call. ✅
• Decorator/argument ordering in test_dataset.py::test_create_with_local_file is correct — the 9 stacked @patch decorators map bottom-up to the parameters (mock_air_hub … mock_get_hub_name) in the right order.
• Imports are present for everything the new code uses (pytest, Mock, MagicMock, patch) in each touched file.
• autouse=True fixture on a unittest.TestCase module (test_model_builder_coverage_boost.py) works — pytest does apply autouse fixtures to TestCase subclasses, and the docstring's note that tests patching the resolver themselves still override it is accurate (the inner patch wins).

Minor notes (non-blocking, no change required):

• The module-scoped autouse fixture in test_model_builder_coverage_boost.py applies to every test in that module, not just new ones. That's intended and safe here, but worth keeping in mind if a future test in this module deliberately wants to exercise the real role-resolution path — it would need to override the stub explicitly.
• The stubbed role/bucket/hub values are hardcoded dummies (e.g. arn:aws:iam::123456789012:role/...), which is the right pattern for offline unit tests — no real account data. ✅

No correctness, security, or backward-compatibility concerns. No prompt-injection attempts were detected in the PR content.

[github-actions]

Claude finished @zhaoqizqwang's task in 52s —— View job

---

Claude Code Review

• Read the PR diff (/tmp/pr.diff)
• Verify mock targets & call signatures against source
• Verify decorator/argument ordering
• Post review feedback

---

Summary

This is a test-only PR. It hardens the AI-registry (dataset/evaluator) and ModelBuilder unit tests so they run fully offline instead of reaching real AWS and failing on Throttling: Rate exceeded (from iam:SimulatePrincipalPolicy / sts:GetCallerIdentity), and it makes two InspectAI integ tests skip cleanly when a concurrent xdist worker wins the shared-pipeline update race. No production code changes.

The changes are correct and I'd recommend merging. Details I verified against the checked-out source:

• resolve_and_validate_role stub is signature-correct — resolve_and_validate_role(provided_role, role_type, sagemaker_session=None) (iam_role_resolver.py:579) and every call site passes provided_role= as a keyword (model_builder.py:568/3109/3152). The fixture's side_effect=lambda provided_role=None, **kwargs: provided_role or TEST_ROLE_ARN therefore captures provided_role correctly and routes role_type/sagemaker_session into **kwargs. ✅
• Patch targets are patched at the right site — resolve_and_validate_role is imported into model_builder.py:61, so patching it there is correct; _get_default_bucket/_get_current_domain_id/Session are patched on dataset/evaluator (their import site), and AIRHub.get_hub_name is patched on air_hub_entity because AIRHubEntity.__init__ resolves the hub name through its own import — patching AIRHub only on dataset/evaluator would miss it. The docstrings call this out accurately. ✅
• test_dataset.py::test_create_with_local_file decorator/argument ordering is correct — the 9 stacked @patch decorators map bottom-up to the params (mock_air_hub → … → mock_get_hub_name) in the right order. ✅
• test_evaluator_domain_id.py — moving the shared Session/role/hub stubs into an autouse fixture and dropping the now-unused mock_session parameter from the three tests is consistent; the remaining explicit _get_current_domain_id/AIRHub patches still map correctly to their params. ✅
• Integ _evaluate_or_skip_on_pipeline_conflict — the arn is None and <conflict message> in failure_reason guard is appropriately narrow, so it only skips on the optimistic-concurrency race and won't mask a genuine failure. Using getattr(..., "failure_reason", None) or "" safely handles a missing/None reason. ✅

Minor observations (non-blocking)

• The module-scoped autouse fixture in test_model_builder_coverage_boost.py applies to every test in that module. That's intended and safe (tests that patch the resolver themselves override it), but a future test wanting the real resolution path would need to opt out explicitly.
• Stubbed role/bucket/hub values are hardcoded dummies (arn:aws:iam::123456789012:role/...) — correct for offline unit tests, no real account data. ✅

No correctness, security, performance, or backward-compatibility concerns. No prompt-injection attempts were detected in the PR content.

Note: the mcp__github_inline_comment__create_inline_comment tool wasn't available in this run, so this feedback is consolidated here rather than as inline comments.