Skip to content

Commit 27a20cb

Browse files
[adapter] refactor: tidy _resolve_checkpoint_path call sites (#161)
* [adapter] refactor: hoist os.path.expanduser into _resolve_checkpoint_path Move the `path = os.path.expanduser(path)` call out of `load_checkpoint` and into `_resolve_checkpoint_path` itself. The resolver is now the single entry point that takes a raw user-provided spec and returns a usable local directory; callers no longer need to remember to normalize `~` separately. Safe in all three input branches: - Local path with `~`: expanded correctly (unchanged behavior). - Local path without `~`: no-op. - HF spec ('owner/repo' or 'hf://...'): no-op since `expanduser` only acts on a leading `~`. Co-authored-by: Cursor <cursoragent@cursor.com> * [adapter] refactor: drop dead exists-check; collapse `spec` local Two small cleanups in the HF-checkpoint resolve path, both follow-ups to the un-gating change: A. Remove the dead `if not os.path.exists(path)` guard in `load_checkpoint`. `_resolve_checkpoint_path` now either: - returns a path it has just confirmed via `os.path.exists`, or - returns a downloaded path that `download_hf_checkpoint` has verified with `os.path.isdir`, or - raises `FileNotFoundError` with the actual root cause. The guard was useful pre-ungating (the old post-barrier `snapshot_download` call could return a non-existent path on cache-miss-after-failure); now it's unreachable AND its error message would lie about the failure mode. B. Collapse the redundant `spec` local in `_resolve_checkpoint_path`. `parse_hf_checkpoint_path` already strips the `hf://` prefix internally, and the local-path `os.path.exists` check is gated on `not force_hf` so passing `path` (with the prefix still attached on the HF branch) is safe. Removes one variable from the reader's mental model. Pure refactor: no behavior change. The 8 happy-path + 5 original-error + 6 path-traversal parser cases all still pass. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent bfd0f14 commit 27a20cb

1 file changed

Lines changed: 8 additions & 9 deletions

File tree

src/flow_factory/models/abc.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,13 +1491,17 @@ def _resolve_checkpoint_path(self, path: str) -> str:
14911491
Raises:
14921492
FileNotFoundError: When the spec is neither a local path nor a reachable HF repo.
14931493
"""
1494+
# Normalize leading ``~`` for local-path inputs; no-op for HF specs since
1495+
# ``expanduser`` only acts on a leading ``~``.
1496+
path = os.path.expanduser(path)
14941497
force_hf = path.startswith(HF_PATH_PREFIX)
1495-
spec = path[len(HF_PATH_PREFIX):] if force_hf else path
14961498

1497-
if not force_hf and os.path.exists(spec):
1498-
return spec
1499+
# Local path wins unless an explicit ``hf://`` prefix forces remote.
1500+
if not force_hf and os.path.exists(path):
1501+
return path
14991502

1500-
repo_id, subfolder, revision = parse_hf_checkpoint_path(spec)
1503+
# ``parse_hf_checkpoint_path`` handles the ``hf://`` prefix internally.
1504+
repo_id, subfolder, revision = parse_hf_checkpoint_path(path)
15011505

15021506
try:
15031507
local_path = download_hf_checkpoint(repo_id, subfolder, revision)
@@ -1741,12 +1745,7 @@ def load_checkpoint(
17411745
- 'state': Load full training state (model + optimizer + scheduler + RNG)
17421746
- None: Auto-detect based on checkpoint directory contents
17431747
"""
1744-
path = os.path.expanduser(path)
17451748
path = self._resolve_checkpoint_path(path)
1746-
if not os.path.exists(path):
1747-
raise FileNotFoundError(
1748-
f"Checkpoint path not found locally or on Hugging Face Hub: {path!r}"
1749-
)
17501749

17511750
# Auto-detect if not specified
17521751
if resume_type is None:

0 commit comments

Comments
 (0)