Fix pre dryrun fail

[dongjiang1989]

Closes: #5

Summary by CodeRabbit

• Bug Fixes

  • More robust snapshot error handling with warnings instead of crashes and safer repo-existence checks.
  • CLI exit remains non-zero on item failures.

• Performance Improvements

  • Smarter transfer scheduling: small files in parallel, large files sequentially with progress logging.
  • Skip state updates when nothing was transferred to avoid needless writes.

• Tests

  • Added tests covering snapshot-failure scenarios across sync directions.

[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: ba497946-ead9-4c7f-a0cd-1f08647d9cb0

📥 Commits

Reviewing files that changed from the base of the PR and between 1ed02f8 and 8047205.

📒 Files selected for processing (2)

• src/adapters/modelscope_adapter.py
• src/sync_engine.py

---

📝 Walkthrough

Walkthrough

This PR hardens the HF↔MS sync engine by catching snapshot fetch errors, validating required snapshots by direction, optimizing transfers with a 100MB small/large file strategy, persisting state only after successful transfers, and adding adapter/workflow safety and tests.

Changes

Snapshot error handling and sync optimization

Layer / File(s)	Summary
Snapshot error handling and validation src/sync_engine.py	HuggingFace snapshot fetching is wrapped in try/except to log warnings instead of crashing. BIDIRECTIONAL and HF_TO_MS modes validate that required snapshots exist, raising RuntimeError with repo details when missing.
Conditional state persistence src/sync_engine.py	Sync state and HF commit tracking are updated only when transfers succeed and produce synced files; per-file SHA lookup falls back to the other platform's snapshot map when entries are missing.
Disk-space-aware transfer execution src/sync_engine.py	Introduces a 100MB threshold to separate small files (parallel transfer via ThreadPoolExecutor) from large files (sequential transfer with progress logs); actions are sorted by size and existing success/failure aggregation is preserved.
Repository safety and workflow CLI improvements src/adapters/modelscope_adapter.py, .github/workflows/sync.yml, src/sync_engine.py	ModelScope adapter explicitly checks repo existence before creating and logs non-fatal warnings on errors. Workflow constructs CLI args using a Bash array to preserve boundaries. CLI main computes an any_failed flag for exit logic.
Error handling test coverage tests/test_sync_engine_extended.py	Three new tests verify bidirectional failure modes when snapshots are unavailable and confirm ms_to_hf can succeed when only the HF snapshot fetch fails.

Sequence Diagram(s)

sequenceDiagram
  participant Workflow as GH_Workflow
  participant SyncEngine
  participant HF_API as HuggingFaceAPI
  participant MS_API as ModelScopeAPI
  participant Executor as ThreadPoolExecutor
  participant Storage

  Workflow->>SyncEngine: invoke sync (python -m src.sync_engine with ARGS)
  SyncEngine->>HF_API: fetch HF snapshot (try/except)
  SyncEngine->>MS_API: fetch MS snapshot
  HF_API-->>SyncEngine: snapshot or error (logged)
  MS_API-->>SyncEngine: snapshot or error (logged)
  SyncEngine->>Executor: schedule small-file transfers (concurrent)
  SyncEngine->>Storage: sequentially transfer large files (one-by-one)
  Executor-->>SyncEngine: transfer results (success/failed)
  Storage-->>SyncEngine: transfer results
  SyncEngine->>Storage: update persisted state (only if files_synced non-empty)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

"A rabbit hops through the sync with care,
Catching failures mid-air,
Large files walk, small ones race,
State updates wait for success's embrace,
Tests hop in to make the path fair." 🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.78% 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 'Fix pre dryrun fail' is vague and does not clearly communicate the scope of changes, which include workflow refactoring, error handling improvements, and transfer strategy optimization.	Consider a more descriptive title like 'Fix sync workflow and snapshot error handling' that better summarizes the primary changes across workflow, adapter, and engine components.

✅ 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 closes issue #5 (sync workflow failure on 2026-05-29) with multiple substantive fixes: word-splitting bug in sync.yml, HF snapshot fetch error handling, runtime guards for missing snapshots, state persistence logic, and large-file transfer optimization.
Out of Scope Changes check	✅ Passed	All changes directly address the reported sync failure: workflow improvements, snapshot error handling, transfer strategy optimization, and state persistence fixes are aligned with resolving the issue.

_{✏️ 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 introduces disk-space awareness by transferring large files (>100MB) sequentially and small files in parallel, improves error handling during snapshot fetching, and adds corresponding test cases. However, several critical issues were identified in the review: the ModelScope HubApi lacks a repo_exists method, which will cause an AttributeError; repository creation does not specify the repo_type for datasets; and updating the sync state can fail to record file hashes if the target snapshot is missing or doesn't yet contain the newly synced files.