Skip to content

Use duplicate_repo for sandbox Spaces#249

Merged
lewtun merged 2 commits intomainfrom
codex/fix-hf-duplicate-repo
May 8, 2026
Merged

Use duplicate_repo for sandbox Spaces#249
lewtun merged 2 commits intomainfrom
codex/fix-hf-duplicate-repo

Conversation

@lewtun
Copy link
Copy Markdown
Member

@lewtun lewtun commented May 8, 2026

Summary

  • Replace deprecated HfApi.duplicate_space with HfApi.duplicate_repo(..., repo_type="space") for sandbox Space creation.
  • Map sandbox hardware and sleep-time options to the new space_hardware and space_sleep_time arguments.
  • Update the sandbox unit-test fakes and assertions to cover the new Hub API call shape.

Tests

  • uv run pytest tests/unit/test_sandbox_private_spaces.py
  • uv run ruff check agent/tools/sandbox_client.py tests/unit/test_sandbox_private_spaces.py
  • uv run ruff format --check agent/tools/sandbox_client.py tests/unit/test_sandbox_private_spaces.py
  • uv run ruff check --exclude train_smollm2.py .
  • uv run ruff format --check --exclude train_smollm2.py .

Note: the exact repo-wide uv run ruff check . and uv run ruff format --check . commands are currently blocked locally by unrelated untracked train_smollm2.py formatting/lint issues.

Co-authored-by: OpenAI Codex <codex@openai.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

Claude finished @lewtun's task in 2m 25s —— View job


Claude PR Review

No blocking issues — 1 P1, 1 P2.

This is a tightly scoped migration from the deprecated HfApi.duplicate_space to the unified HfApi.duplicate_repo(..., repo_type="space"), with the hardware/sleep_time kwargs renamed to their space-namespaced variants. Production code, unit-test fakes, and the PEP-723 inline-script dep pin (huggingface_hub>=1.12.0) are all updated consistently. pyproject.toml already requires huggingface-hub>=1.12.0 and uv.lock is on 1.12.0, so the runtime version is in sync.

I confirmed there are no remaining duplicate_space references and no other call sites use the old hardware=/sleep_time= kwargs.


P1 — Hub kwarg shape isn't actually exercised by the tests

agent/tools/sandbox_client.py:615-625 calls api.duplicate_repo(repo_type="space", space_hardware=..., space_sleep_time=...), but the four updated unit-test fakes at tests/unit/test_sandbox_private_spaces.py:25, 71, 111, 158 all use def duplicate_repo(self, **kwargs) — which silently accepts any kwarg name. If the real huggingface_hub 1.12.0 method names them differently (e.g. without the space_ prefix, or expects them inside a nested dict), the unit tests will still pass while the live call 400s.

This is the same mock-shape risk that existed pre-PR; I'd just suggest one of the following before merging, given it was the central reason for the change:

  • Run uv run python -c "import inspect, huggingface_hub; print(inspect.signature(huggingface_hub.HfApi.duplicate_repo))" and paste it in the PR body, or
  • Do one manual sandbox creation against the real Hub, or
  • Tighten the fake's signature to def duplicate_repo(self, *, from_id, to_id, repo_type, private, space_hardware, space_sleep_time=None) so a typo in the production kwarg name fails tests.

Cite: agent/tools/sandbox_client.py:615-625, tests/unit/test_sandbox_private_spaces.py:25,71,111,158.

P2 — Inline-script dep header jumped from 0.20.01.12.0

agent/tools/sandbox_client.py:4 bumps the PEP-723 dep range across a major version. This is fine — it matches the project pin — but anyone who has been running this file standalone via uv run agent/tools/sandbox_client.py will now resolve a major-version-newer huggingface_hub. Worth a one-line note in the release notes if there is one.


· branch codex/fix-hf-duplicate-repo

Co-authored-by: OpenAI Codex <codex@openai.com>
@lewtun
Copy link
Copy Markdown
Member Author

lewtun commented May 8, 2026

Addressed the P1 in c6e6942 by tightening the sandbox FakeApi.duplicate_repo methods to keyword-only signatures matching the Hub API shape, so old or misspelled kwargs fail tests instead of being swallowed by **kwargs.

Also verified the installed Hub signature locally:

(self, from_id: 'str', to_id: 'str | None' = None, *, repo_type: 'str | None' = None, private: 'bool | None' = None, visibility: 'RepoVisibility_T | None' = None, token: 'bool | str | None' = None, exist_ok: 'bool' = False, space_hardware: 'SpaceHardware | None' = None, space_storage: 'SpaceStorage | None' = None, space_sleep_time: 'int | None' = None, space_secrets: 'list[dict[str, str]] | None' = None, space_variables: 'list[dict[str, str]] | None' = None, space_volumes: 'list[Volume] | None' = None) -> 'RepoUrl'

P2 is noted; I do not see a release-notes file in this repo, and the PR body already calls out the standalone PEP-723 header bump.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

Claude finished @lewtun's task in 1m 7s —— View job


Claude PR Review (re-review of c6e6942)

LGTM.

  • Read updated diff vs origin/main
  • Check agent/tools/sandbox_client.py call site
  • Check tightened test fakes
  • Verify P1 fix from prior review
  • Post review

P1 from the prior review is addressed. The four test fakes at tests/unit/test_sandbox_private_spaces.py:47, 110, 159, 223 are now strict keyword-only signatures (*, from_id, to_id, repo_type, private, space_hardware, space_sleep_time=None), so a renamed/typo'd kwarg in agent/tools/sandbox_client.py:615-625 would raise TypeError in tests instead of being silently swallowed. Three of the four tests also assert repo_type == "space" on the captured call. The kwarg names (from_id, to_id, repo_type, private, space_hardware, space_sleep_time) match the huggingface_hub 1.12.0 signature you pasted in the thread.

P2 (PEP-723 dep header 0.20.01.12.0) — accepted as noted; no release notes file exists in this repo.

Confirmed there are no remaining duplicate_space references in the tree.

--- · branch codex/fix-hf-duplicate-repo

@lewtun lewtun merged commit d684c1a into main May 8, 2026
3 checks passed
@lewtun lewtun deleted the codex/fix-hf-duplicate-repo branch May 8, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant