ci: fix pre dryrun fail

[dongjiang1989]

Closes #7

Summary by CodeRabbit

• Bug Fixes

  • Clearer permission error messages for repository upload failures.
  • Safer handling when snapshots are missing to avoid unexpected failures.

• Improvements

  • Disk-aware file transfer strategy: small files transferred in parallel, large files sequentially with progress reporting.
  • More robust sync-state updates to prevent incorrect state when snapshots are absent.

• Chores

  • Updated model repository reference and CI sync invocation.

• Tests

  • Added tests covering snapshot failure scenarios and directional sync behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 82ffe693-abf8-45fb-96f7-5761f81c20e4

📥 Commits

Reviewing files that changed from the base of the PR and between 726bb3d and eb128f8.

📒 Files selected for processing (1)

• src/adapters/modelscope_adapter.py

---

📝 Walkthrough

Walkthrough

The PR refactors workflow CLI args into a bash array, surfaces adapter permission errors, validates required snapshots per sync direction, adds disk-aware transfer scheduling, conditions state persistence on actual transfers, and adds tests for snapshot-failure scenarios.

Changes

Sync Workflow Robustness

Layer / File(s)	Summary
Workflow argument array construction .github/workflows/sync.yml	The "Run sync" step refactors argument construction from string concatenation to a bash array, conditionally appending --dry-run, --direction, and --target flags and invoking with "${ARGS[@]}" for proper argument boundary preservation.
ModelScope adapter error handling src/adapters/modelscope_adapter.py	upload_file and _upload_via_hf now detect authorization failures (HTTP 401/403, "does not exist") and raise PermissionError with explicit messaging. create_repo_if_needed replaces exception-based existence detection with explicit repo_exists checks and creates only when missing.
Sync engine snapshot validation src/sync_engine.py	HuggingFace snapshot fetching wraps in try/except and logs warnings on failure. BIDIRECTIONAL mode raises RuntimeError if either snapshot is missing, HF_TO_MS raises if HuggingFace snapshot is missing, and MS_TO_HF validates the ModelScope snapshot presence.
File transfer scheduling optimization src/sync_engine.py	Transfer execution implements disk-space-aware scheduling: small files (≤100MB) transfer in parallel using a bounded thread pool, large files transfer sequentially with indexed progress logging, and docstring documents the strategy.
Sync engine state persistence and fallbacks src/sync_engine.py	State updates occur only when result.files_synced is non-empty. Per-file SHA lookups fall back to the alternate snapshot's file map when the primary map lacks the entry. HF last_synced_commit is set only when hf_snapshot exists. CLI exit behavior uses an explicit any_failed variable.
Snapshot failure tests and model configuration tests/test_sync_engine_extended.py, config/sync_config.yaml	Three new tests validate bidirectional and unidirectional sync behavior when snapshots fail: both sides failing marks FAILED, one side failing marks FAILED with error message, and HF failure does not prevent MS_TO_HF success. The qwen2.5-7b-instruct model's ms_repo_id is updated to a new repository path.

Sequence Diagram

sequenceDiagram
  participant Workflow
  participant SyncEngine
  participant HuggingFace
  participant ModelScope
  Workflow->>SyncEngine: invoke python -m src.sync_engine
  SyncEngine->>HuggingFace: fetch snapshot (try/except)
  SyncEngine->>ModelScope: fetch snapshot
  SyncEngine->>SyncEngine: schedule transfers (small parallel, large sequential)
  SyncEngine->>ModelScope: upload files via ModelScopeAdapter

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nudge the args into a row,
I sniff for perms where uploads go,
I sort the files, small quick, big slow,
I only mark the state when bytes did flow,
Hopping home with tests aglow.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'ci: fix pre dryrun fail' is vague and does not clearly communicate the specific changes made in the pull request.	Provide a more descriptive title that clearly explains the specific issue being fixed, such as 'ci: improve snapshot handling and error resilience in sync workflow' or similar.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR addresses issue #7 (sync failure on 2026-05-29) with multiple robustness improvements including snapshot handling, error handling, and performance optimizations.
Out of Scope Changes check	✅ Passed	All changes appear scoped to fixing sync failures: workflow improvements, adapter error handling, sync engine robustness, config updates, and related tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-pre-dryrun-fail

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request updates the sync configuration, improves error handling for ModelScope uploads, introduces disk-space-aware sequential transfers for large files, and adds robust snapshot validation checks. The code review highlights a critical runtime AttributeError due to the use of non-existent repo_exists and create_repo methods on ModelScope's HubApi. Additionally, the reviewer suggests converting exception messages to lowercase for case-insensitive matching and ensuring both snapshots are valid during HF_TO_MS syncs to prevent redundant file transfers.