fix: handle missing files gracefully in remove_real_and_linked_file (#297)

[bhumitschaudhry]

Summary

Fixes #297

The remove_real_and_linked_file() helper had two bugs that caused crashes when delete_original=True was used during model splitting:

1. targetpath uninitialized — targetpath was only assigned inside an if block, causing UnboundLocalError when the input was a regular file (not a symlink).
2. Missing file crash — FileNotFoundError was not caught, so deleting an already-removed file would crash mid-split, leaving users with partial splits and missing source shards.
3. Type mismatch — os.path.realpath() returns str but callers pass Path objects, breaking the equality comparison.

Changes

air_llm/airllm/utils.py:

• Normalize to_delete via os.fspath() so Path objects compare correctly against str realpath
• Initialize targetpath = None before the conditional
• Catch FileNotFoundError on os.remove() and return gracefully
• Add os.path.exists() guard before removing the symlink target
• Add docstring explaining the function's behavior

air_llm/tests/test_delete_original.py (new):

• Unit tests for regular files, Path inputs, missing files, and (Linux/macOS) symlinks

Root Cause

The original code:

def remove_real_and_linked_file(to_delete):
    if (os.path.realpath(to_delete) != to_delete):
        targetpath = os.path.realpath(to_delete)
    os.remove(to_delete)
    if (targetpath):
         os.remove(targetpath)

Had three bugs:

• targetpath was undefined when to_delete was a regular file → UnboundLocalError
• Path objects compared against str realpath → symlink detection broken on Path inputs
• No error handling for missing files → crash mid-split

Test Results

Ran 8 tests in 0.038s — OK (skipped=3, symlink tests skip on Windows)

Why this matters

delete_original=True is specifically for low-disk environments. A crash after deleting original shards leaves users with a partial split, missing source files, and no clear recovery path except re-downloading very large model artifacts.

[Copilot]

Pull request overview

This PR addresses crashes when delete_original=True triggers remove_real_and_linked_file() during model splitting, making deletion resilient to Path inputs and missing files, and adding unit coverage for the helper.

Changes:

• Harden remove_real_and_linked_file() against Path vs str comparisons and missing-file deletions.
• Add a unittest module covering regular files, Path inputs, missing files, and symlink scenarios.
• Add an additional “standalone” test script under air_llm/tests/.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
air_llm/airllm/utils.py	Makes deletion helper safer for Path inputs and missing files; adds docstring describing behavior.
air_llm/tests/test_delete_original.py	Adds unit tests for regular files, missing files, and symlink behavior.
air_llm/tests/test_delete_original_standalone.py	Adds a standalone script/test module duplicating the helper and validating behavior in isolation.

Suppressed comments (4)

air_llm/tests/test_delete_original.py:81

• This symlink test should be skipped on Windows for the same reason as the other symlink test (symlink creation often fails without elevated privileges).

    def test_symlink_with_path_object(self):
        """Symlink removal works with Path input too."""

air_llm/tests/test_delete_original.py:103

• Creating a broken symlink is also Windows-hostile (privilege/dev-mode dependent). Skipping this test on Windows avoids CI failures unrelated to the actual behavior under test.

    def test_broken_symlink_does_not_crash(self):
        """Deleting a symlink whose target is already gone should not raise."""
        link = os.path.join(self.tmpdir, "broken-link.bin")

air_llm/tests/test_delete_original_standalone.py:112

• Same as above: the skip condition should include macOS (Darwin) if the intent is Linux/macOS.

    @unittest.skipUnless(platform.system() == "Linux", "symlink test requires Linux/macOS")
    def test_symlink_with_path_object(self):

air_llm/tests/test_delete_original_standalone.py:123

• Same as above: include macOS (Darwin) in the skip condition to match the message and intended coverage.

    @unittest.skipUnless(platform.system() == "Linux", "symlink test requires Linux/macOS")
    def test_broken_symlink_does_not_crash(self):

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.