* fix(vision): validate dataset video paths before training
* fix(vision): remove redundant warnings import, add pytest tests for #5085
* fix(trainer): auto-validate video paths in UnslothVisionDataCollator on first batch (#5085)
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* refactor(vision): use str.removeprefix instead of slicing (Datta0 review)
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* style: replace em dashes with hyphens in error message and docstring
* Update unsloth/models/vision.py
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Split: keep only 1 file(s)
* fix(vision): broaden video-path validation to all collator inputs
- check_dataset_for_missing_videos now accepts every example shape that
UnslothVisionDataCollator forwards to process_vision_info:
dict rows with "messages"/"conversations"/"prompt"/"completion", and raw
message-list rows. Earlier logic only handled {"messages": [...]}, so
conversations/prompt/completion datasets silently skipped validation and
raw message-list rows crashed on list.get.
- Guard against non-dict message entries; a bare string inside a message
list no longer raises AttributeError.
- Decode file:// URIs via urllib so percent-encoded paths, absolute
Windows URIs (file:///C:/...) and host-qualified URIs
(file://localhost/abs/path) map back to their real filesystem path.
- Expose an optional "checked" set so callers can reuse dedup state across
invocations.
- Docstring warns that passing a streaming IterableDataset consumes the
iterator.
UnslothVisionDataCollator now validates every batch (not just batch 0)
and applies formatting_func before validation, matching the base
collator's own ordering so formatter-generated video paths are also
checked. The already-checked set is shared across batches, so per-batch
cost stays proportional to newly seen paths.
* fix(vision): robust URI + scheme handling in video-path validator
- _local_path_from_video_value now treats anything with a "://" prefix
as a URI and validates only file:// URIs. This prevents false
FileNotFoundError on remote schemes that were silently passed through
before (s3://, gs://, hf://, ftp://, az://, ...).
- Non-localhost file authorities (e.g. file://nas-server/share/clip.mp4)
are now skipped instead of being stripped and validated against the
local filesystem; RFC 8089 only permits empty host or "localhost" for
local files.
- Drop the explicit unquote call: urllib.request.url2pathname already
unquotes, so the previous url2pathname(unquote(path)) double-decoded
any filename with a literal percent (e.g. a file named "clip%20.mp4").
- Remove the Windows drive-letter strip block; nturl2path.url2pathname
handles "/C:/foo" -> "C:\\foo" itself, leaving nothing for the guard to
match on either OS.
- Return None when the resolved path is empty (bare "file://" or
"file://hostname") so the caller skips it instead of reporting a
blank " - " entry in the error message.
- Add a runtime guard in check_dataset_for_missing_videos that warns and
returns early when handed a datasets.IterableDataset, matching the
docstring contract and preventing silent iterator exhaustion.
Windows native paths like "C:/path/x.mp4" stay valid because the
scheme check uses the "://" substring (not urlparse's single-letter
scheme surface).
* tests: consolidate video-path validation coverage into one file
New coverage for tests/test_video_path_validation.py:
- every-batch validation with cross-batch dedup (replaces the old
first-batch-only assertion which no longer matches the implementation).
- all collator-supported input shapes: messages, conversations,
prompt/completion, raw-message-list rows; non-dict message entries.
- file:// URI robustness: percent-encoded paths, localhost netloc,
non-localhost netloc skipped, bare / hostname-only URIs skipped,
double-encoded filenames single-unquote correctly.
- non-file remote schemes (s3, gs, hf, ftp, az) skipped without raising.
- Windows-style absolute path not mistaken for a URI scheme.
- formatting_func applied before validation inside the collator wrapper.
- IterableDataset runtime guard warns and returns without consuming.
The pre-existing test_collator_validates_only_once assertion has been
replaced by test_collator_validates_every_batch / dedupes_across_batches
because the wrapper now validates every batch.
The session-scoped AST fallback fixture was extended to extract the
helper functions that the rewritten check_dataset_for_missing_videos
depends on, so the Windows/no-triton code path still loads the module
surface.
* Fix CI iter 1
* Omit release-desktop.yml from the PR diff
The workflow file landed on origin/main after the PR branched off; our
fork-scoped push token cannot touch .github/workflows/**. Drop it from
this branch so the PR diff stays within the author's authorisable
surface. The file remains on origin/main and will return after this PR
merges upstream.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* fix(vision): cache only validated paths in checked set
Missing paths were added to the dedup cache before the existence check,
so a caller that caught FileNotFoundError and retried with the same
collator/checked set would silently skip the bad path on the second call.
Only add a path to checked after os.path.isfile confirms it exists, so
missing paths are re-validated on every call until they are fixed.
* fix(ci): resolve two CI failures introduced by this PR
- Add __all__ to models/__init__.py so the HOISTED-IMPORT-UNUSED linter
check passes for check_dataset_for_missing_videos
- Replace Dataset.from_list() in test helpers with plain list literals;
the CI environment mocks datasets with a MockDataset that only has
from_dict, but check_dataset_for_missing_videos accepts any iterable
so no Dataset wrapper is needed
- Guard test_iterable_dataset_warns_and_skips with pytest.importorskip
so it skips cleanly when the real datasets package is unavailable
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* fix(ci): correct HOISTED-IMPORT-UNUSED without breaking wildcard exports
Adding __all__ to models/__init__.py was too aggressive - it restricted
from .models import * in _gpu_init.py to only check_dataset_for_missing_videos,
hiding FastLanguageModel, FastVisionModel etc and breaking
test_fast_model_class_surface_under_spoof.
Instead: remove __all__, and add an explicit named import in _gpu_init.py
so the linter sees the symbol consumed in the re-export chain.
* Deduplicate missing video paths and skip data URIs in validator
check_dataset_for_missing_videos appended a path to the missing list on every
occurrence, so a path referenced by multiple rows was reported N times and the
error header read the wrong count. Track missing paths in a per-call set so each
is listed once, kept separate from the checked cache so retries still re-check
missing files. This restores the dedup behaviour the docstring promises and the
existing test_duplicate_paths_deduplicated test asserts.
Also skip data: URIs in _local_path_from_video_value so inline base64 payloads
are not flagged as missing files.
Add tests for the data URI case, warn-only dedup, and real integration against
the unsloth_zoo UnslothVisionDataCollator base (verifying validation gates the
base call, formatting_func is applied once and restored even when the base
raises).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* fix(trainer): declare __slots__ on UnslothVisionDataCollator subclass
* fix(ci): hoist check_dataset_for_missing_videos to trainer module level; guard IterableDataset skip
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* fix(vision): accept tuple message content in video path validator
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten comments and docstrings for PR #5136
Comment-only pass over the new video path validation code: shorten the
collator and validator docstrings, collapse multi-line inline comments,
and reduce test docstrings to one-liners. No code changes; verified with
comment_tools.py check --strip-docstrings (3/3 code unchanged) and the
full test suite (35 passed).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Daniel Han <danielhanchen@gmail.com>
Leo Borcherding