From 8ff2497b637fe592c23311c57108a787914a1f21 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 22 Sep 2026 17:59:21 -0400 Subject: [PATCH 1/5] Report a media href that reaches its file only under case folding A media href is now resolved against the folder the way a companion .lift-ranges already is, so a Windows-authored folder read on a case-sensitive filesystem no longer reports as missing-media files that are right there. Unlike a companion, whose spelling only sil-lift reads, a media href is read by whatever serves the folder afterwards -- a web export, an APK build -- so the fold is reported rather than resolved silently: a new media-href-mismatch warning says the file is here but a case-sensitive host will not find it, and missing-media narrows to no file under any spelling. Resolution reads the directory rather than probing the href's spelling, so the verdict is the same on a case-folding filesystem as on a case-sensitive one; an exact probe answers for a name it can never report, since SDD.PNG stats true against an on-disk sdd.png. Every component folds, not just the last. The conventional audio/pictures subfolder is sil-lift's own guess, so a folder spelling it another way resolves without being reported, while a folder the href itself misspells is one rename however many references cross it, and is reported once with no entry. A href reaching outside the folder is probed as written, since which directory to fold in would be a guess. check-media reports the same mismatches and exits 1 for them, and no longer calls a misspelled file orphaned as well as missing: it compares folded relative paths rather than resolved ones, which is what the library now means by a href naming a file. --no-check-media suppresses both media codes, since the media it exists for lives outside the folder, where no spelling can be verified at all. missing_media() is replaced by check_media(), which classifies each reference rather than answering one boolean, and returns only the references that did not resolve cleanly. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 +- docs/en/guides/cli.md | 13 +- docs/en/guides/folder-media.md | 9 +- docs/en/guides/validate.md | 14 +- src/sil_lift/__init__.py | 2 + src/sil_lift/_cli.py | 64 ++++++--- src/sil_lift/_model.py | 134 ++++++++++++++++-- src/sil_lift/_validate.py | 68 ++++++++- tests/corpus/PROVENANCE.md | 23 +-- .../media-href-mismatch/Audio/word.wav | 0 .../media-href-mismatch.lift | 34 +++++ .../media-href-mismatch/pictures/other.png | 0 .../media-href-mismatch/pictures/sdd.png | 0 .../{ => nfd-range-ids}/nfd-range-ids.lift | 0 .../nfd-range-ids.lift-ranges | 0 tests/test_cli.py | 30 +++- tests/test_ranges_folder.py | 70 ++++++++- tests/test_validate.py | 28 +++- 18 files changed, 429 insertions(+), 64 deletions(-) create mode 100644 tests/corpus/negative/media-href-mismatch/Audio/word.wav create mode 100644 tests/corpus/negative/media-href-mismatch/media-href-mismatch.lift create mode 100644 tests/corpus/negative/media-href-mismatch/pictures/other.png create mode 100644 tests/corpus/negative/media-href-mismatch/pictures/sdd.png rename tests/corpus/negative/{ => nfd-range-ids}/nfd-range-ids.lift (100%) rename tests/corpus/negative/{ => nfd-range-ids}/nfd-range-ids.lift-ranges (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 959c836..b576f7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ releases may contain breaking changes. - LIFT-folder handling: `RangesFile` (standalone `.lift-ranges` documents, same fidelity guarantees), automatic companion discovery/tracking on load (`Lexicon.ranges_files`), `save()` writes companions together, - `all_ranges()` merged view, `media_refs()` / `missing_media()` helpers, + `all_ranges()` merged view, `media_refs()` / `check_media()` helpers, build-from-scratch helpers `Lexicon.add_ranges_file()` / `RangesFile.add_range()` / `Range.add_element()` (`save()` writes and header-references a new companion beside the `.lift`); vendored @@ -71,7 +71,7 @@ releases may contain breaking changes. file, entry, and line it concerns. RELAX NG layer with two documented departures from strict validation (invalid `file://` hrefs downgraded to `uri-not-rfc` warnings; legal interleaving not falsely flagged); vendored - ranges schema over companions; and twelve semantic checks, one `Problem` + ranges schema over companions; and thirteen semantic checks, one `Problem` code each (with missing-id opt-in via `require_ids`). Every code is described in `docs/en/guides/validate.md`. - Canonical sort: `Lexicon.sort()` / `RangesFile.sort()` (entries by diff --git a/docs/en/guides/cli.md b/docs/en/guides/cli.md index 341a7a9..09e7b56 100644 --- a/docs/en/guides/cli.md +++ b/docs/en/guides/cli.md @@ -8,12 +8,21 @@ sil-lift validate PATH [--format {text,json}] [--strict] [--no-check-media] [--r sil-lift stats PATH [--format {text,json}] entry/sense/language counts (streaming; any size) sil-lift sort PATH [-o OUT] canonically sorted, diff-ready copy (default: in place) -sil-lift check-media PATH missing and orphaned media report; exit 1 if missing +sil-lift check-media PATH missing, misspelled, and orphaned media report; exit 1 if missing or misspelled sil-lift export PATH [-o OUT] [--langs L] [--tsv] one row per leaf sense (subsenses flattened) to CSV/TSV (streaming) ``` -`--format json` writes a single JSON object to stdout (and nothing else) for CI/automation consumption; see the schema in the example below. `--strict` treats warnings as errors, exiting 1 if any are found — use it to gate a build on no warnings at all rather than on errors alone. `--no-check-media` skips the filesystem media-presence check (suppressing `missing-media` findings), which is useful when validating a freshly generated export whose audio/photo files live elsewhere rather than in the same folder. `--require-ids` additionally fails (a `missing-id` error) on any entry lacking a `guid` or sense lacking an `id` — stricter than LIFT, for workflows that re-import by a stable id. Passing `-` as the path reads the document from stdin (a piped document has no folder, so its companion `.lift-ranges` and media are not resolved). `stats` likewise takes `--format json`, emitting the counts as a single JSON object. +`validate`'s options: + +- `--format json` writes a single JSON object to stdout (and nothing else) for CI/automation consumption; see the schema in the example below. +- `--strict` treats warnings as errors, exiting 1 if any are found — use it to gate a build on no warnings at all rather than on errors alone. +- `--no-check-media` skips the filesystem media-presence check, suppressing `missing-media` and `media-href-mismatch` findings. Useful when validating a freshly generated export whose audio/photo files live elsewhere rather than in the same folder. +- `--require-ids` additionally fails (a `missing-id` error) on any entry lacking a `guid` or sense lacking an `id` — stricter than LIFT, for workflows that re-import by a stable id. + +Passing `-` as the path reads the document from stdin. A piped document has no folder, so its companion `.lift-ranges` and media are not resolved. + +`stats` likewise takes `--format json`, emitting the counts as a single JSON object. !!! note `validate`'s exit codes and `--format json` schema are a supported automation interface: both are covered by tests and change only under SemVer. diff --git a/docs/en/guides/folder-media.md b/docs/en/guides/folder-media.md index 4e03341..145d8ea 100644 --- a/docs/en/guides/folder-media.md +++ b/docs/en/guides/folder-media.md @@ -41,11 +41,18 @@ Pass `resolve_ranges=False` to `load()` to skip companion discovery. for ref in lex.media_refs(): # every and print(ref.kind, ref.href, ref.entry_id) -lex.missing_media() # refs whose files don't exist +lex.check_media() # refs that don't resolve cleanly ``` Resolution follows the conventional layout: a relative href is checked as given (backslashes normalized — WeSay writes `pictures\photo with space.png`) and under `audio/` (for pronunciation media) or `pictures/` (for illustrations). Remote/absolute hrefs can't be checked and are skipped. +`check_media()` reports only the references that did not resolve cleanly, one `MediaResolution` each: + +- `status="missing"` — no file answered the href under any spelling. +- `status="mismatch"` — one did, but only under case folding or NFC; `found` names the file on disk. + +A href that spells its file exactly is not reported. Unlike a companion name, which folds silently, a media href is also read by whatever serves the folder afterwards, so the fold is reported as [`media-href-mismatch`](validate.md#problem-codes). + ## Other folder contents A LIFT folder often holds files sil-lift doesn't model — writing-system LDML under `WritingSystems/`, The Combine's speaker consent audio/image files under `consent/`, and the like; `load()`/`save()` leave these untouched, and [`Lexicon.save_zip()`](lift-export-interop.md) carries them through verbatim when packaging the folder. diff --git a/docs/en/guides/validate.md b/docs/en/guides/validate.md index 729ae05..a178856 100644 --- a/docs/en/guides/validate.md +++ b/docs/en/guides/validate.md @@ -24,11 +24,11 @@ Each `Problem` carries `level` (`"error"`/`"warning"`), a stable `code`, `messag 1. **RELAX NG** against the LIFT 0.13 grammar (vendored from lift-standard — a byte-identical copy committed into this package). 2. **Ranges schema** — this project's `lift-ranges-0.13.rng` — over every tracked `.lift-ranges` companion, addressed to the companion rather than the `.lift`. -3. **Semantic checks** the grammar cannot express — twelve of them, one code each. +3. **Semantic checks** the grammar cannot express — thirteen of them, one code each. ## Problem codes -Every finding carries one of these, whichever layer produced it — `schema` and `uri-not-rfc` come from the schema layers, the other twelve are semantic checks. The strings are a supported interface; `--strict` promotes every warning to an error. +Every finding carries one of these, whichever layer produced it — `schema` and `uri-not-rfc` come from the schema layers, the other thirteen are semantic checks. The strings are a supported interface; `--strict` promotes every warning to an error. | code | level | what it flags | | ------------------------ | ------- | -------------------------------------------------------------------------- | @@ -38,6 +38,7 @@ Every finding carries one of these, whichever layer produced it — `schema` and | `duplicate-form-lang` | warning | two forms in one multitext sharing a language | | `duplicate-guid` | error | a guid reused among entries, or among one document's ranges/range-elements | | `form-missing-lang` | error | a `
` or `` without the `lang` the schema requires | +| `media-href-mismatch` | warning | a media href that reaches its file only under case folding or NFC | | `missing-id` | error | opt-in via `require_ids`: an entry without a guid, a sense without an id | | `missing-media` | warning | a referenced audio or picture file not on disk | | `normalization-mismatch` | warning | a name that reaches the id it refers to only under NFC | @@ -51,7 +52,14 @@ All three layers work from the document serialized as it stands, so one that can A companion name matching several files loads none of them: the ranges they define go absent until all but one is renamed or removed. -The three companion-folder codes (`ambiguous-ranges-file`, `dangling-ranges-href`, and `unreadable-ranges-file`) are reported only when companion discovery ran. Loading with `resolve_ranges=False` puts companions out of scope, so none of them is reported; attaching one afterwards with `add_ranges_file()` does not bring them back, since it never reads the folder these codes report on. `missing-media` is unaffected: media is never resolved into the model, so nothing was opted out of. +The three companion-folder codes (`ambiguous-ranges-file`, `dangling-ranges-href`, and `unreadable-ranges-file`) are reported only when companion discovery ran. Loading with `resolve_ranges=False` puts companions out of scope, so none of them is reported; attaching one afterwards with `add_ranges_file()` does not bring them back, since it never reads the folder these codes report on. `missing-media` and `media-href-mismatch` are unaffected: media is never resolved into the model, so nothing was opted out of. + +`missing-media` and `media-href-mismatch` divide the media check between them: the first means no file answered the href under any spelling, the second that one did but the href does not name it exactly. The second is the portability finding — the file is here, and a case-sensitive host serving this folder will not find it. + +Two rules govern what `media-href-mismatch` reports: + +- A misspelled *folder* is one rename however many references cross it, so it is reported once and carries no entry. A misspelled *filename* is reported per reference, addressed to the entry that wrote it. +- The conventional `audio/`/`pictures/` subfolder is sil-lift's own guess rather than something the document wrote, so a folder spelling it another way resolves without being reported. ## Real-world FieldWorks (FLEx) output diff --git a/src/sil_lift/__init__.py b/src/sil_lift/__init__.py index e8aaa4f..d86c110 100644 --- a/src/sil_lift/__init__.py +++ b/src/sil_lift/__init__.py @@ -21,6 +21,7 @@ GrammaticalInfo, Lexicon, MediaRef, + MediaResolution, Note, Pronunciation, RangesChanges, @@ -62,6 +63,7 @@ "LiftWriteError", "LiftWriter", "MediaRef", + "MediaResolution", "Multitext", "Note", "Problem", diff --git a/src/sil_lift/_cli.py b/src/sil_lift/_cli.py index 1dcd3c1..6f5b6f7 100644 --- a/src/sil_lift/_cli.py +++ b/src/sil_lift/_cli.py @@ -25,9 +25,9 @@ from ._canonical import canonicalize from ._errors import LiftError -from ._model import Lexicon, _normalize_href +from ._model import Lexicon, _fold, _folded_entries, _normalize_href from ._stream import open_reader -from ._validate import iter_problems +from ._validate import iter_problems, media_mismatch_groups if TYPE_CHECKING: from collections.abc import Sequence @@ -39,6 +39,9 @@ __all__ = ["main"] +#: What --no-check-media suppresses: everything the filesystem media check says. +_MEDIA_CODES = frozenset({"missing-media", "media-href-mismatch"}) + def _problem_json(problem: Problem) -> dict[str, object]: return { @@ -68,7 +71,7 @@ def _collect_problems(args: argparse.Namespace) -> list[Problem]: return [ problem for problem in problems - if not (args.no_check_media and problem.code == "missing-media") + if not (args.no_check_media and problem.code in _MEDIA_CODES) ] @@ -153,26 +156,42 @@ def _cmd_sort(args: argparse.Namespace) -> int: def _cmd_check_media(args: argparse.Namespace) -> int: lexicon = Lexicon.load(args.path) - missing = lexicon.missing_media() - for ref in missing: - owner = ref.entry_id or ref.entry_guid or "?" - print(f"missing {ref.kind:12s} {ref.href!r} (entry {owner})") - - referenced: set[Path] = set() + resolutions = lexicon.check_media() + missing = [item for item in resolutions if item.status == "missing"] + for item in missing: + owner = item.ref.entry_id or item.ref.entry_guid or "?" + print(f"missing {item.ref.kind:12s} {item.ref.href!r} (entry {owner})") + directories, files = media_mismatch_groups(resolutions) + # Spellings differing only in normalization render identically. + for written, on_disk in directories: + print(f"mismatch {'folder':12s} {written!a} is {on_disk!a} on disk") + for item, written, on_disk in files: + owner = item.ref.entry_id or item.ref.entry_guid or "?" + print(f"mismatch {item.ref.kind:12s} {written!a} is {on_disk!a} on disk (entry {owner})") + + referenced: set[str] = set() base = lexicon.path.parent if lexicon.path is not None else Path(args.path).parent for ref in lexicon.media_refs(): relative = _normalize_href(ref.href) if relative is None: # remote/absolute hrefs can't confirm a local file continue - referenced.add((base / relative).resolve()) subfolder = "audio" if ref.kind == "media" else "pictures" - referenced.add((base / subfolder / relative).resolve()) + referenced.add(_folded_key(relative)) + referenced.add(_folded_key(Path(subfolder) / relative)) + # Folded, not resolved: a href reaches its file under the same folding + # check_media() uses, so a file it names inexactly is in use, not orphaned. + listings: dict[Path, dict[str, list[Path]]] = {} + media_folders = [ + path + for name in ("audio", "pictures") + for path in _folded_entries(base, listings).get(name, ()) + if path.is_dir() + ] orphans = [ file - for folder in ("audio", "pictures") - if (base / folder).is_dir() - for file in sorted((base / folder).rglob("*")) - if file.is_file() and file.resolve() not in referenced + for folder in media_folders + for file in sorted(folder.rglob("*")) + if file.is_file() and _folded_key(file.relative_to(base)) not in referenced ] for file in orphans: print(f"orphaned {file.relative_to(base)} (no media/illustration references it)") @@ -181,8 +200,14 @@ def _cmd_check_media(args: argparse.Namespace) -> int: "note: WeSay-style audio writing systems reference files from form " "text, which this check does not follow" ) - print(f"{len(missing)} missing, {len(orphans)} orphaned") - return 1 if missing else 0 + mismatched = len(directories) + len(files) + print(f"{len(missing)} missing, {mismatched} mismatched, {len(orphans)} orphaned") + return 1 if missing or mismatched else 0 + + +def _folded_key(relative: Path) -> str: + """A relative path reduced to what a case-folding filesystem treats as one path.""" + return "/".join(_fold(part) for part in relative.parts) def _leaf_senses(entry: Entry) -> list[Sense]: @@ -340,7 +365,10 @@ def main(argv: Sequence[str] | None = None) -> int: validate.add_argument( "--no-check-media", action="store_true", - help="skip the filesystem media-presence check (suppresses missing-media findings)", + help=( + "skip the filesystem media-presence check " + "(suppresses missing-media and media-href-mismatch findings)" + ), ) validate.add_argument( "--require-ids", diff --git a/src/sil_lift/_model.py b/src/sil_lift/_model.py index dca3e53..82d8903 100644 --- a/src/sil_lift/_model.py +++ b/src/sil_lift/_model.py @@ -40,6 +40,7 @@ "GrammaticalInfo", "Lexicon", "MediaRef", + "MediaResolution", "Note", "Pronunciation", "RangesChanges", @@ -275,6 +276,21 @@ class MediaRef: sense_id: str | None = None # set for illustrations (they live on senses) +@dataclass(slots=True) +class MediaResolution: + """A media reference that did not resolve cleanly against the LIFT folder. + + ``missing`` means no file answered the href under any spelling; + ``mismatch`` means one did, but only under case folding or NFC, and + ``found`` is the file it named. A href that names its file exactly is not + reported at all -- see :meth:`Lexicon.check_media`. + """ + + ref: MediaRef + status: Literal["missing", "mismatch"] + found: Path | None = None + + @dataclass(slots=True) class RangesChanges: """What differs between a companion ``.lift-ranges`` and the file it was read from. @@ -541,6 +557,92 @@ def _existing_file(candidate: Path, listings: dict[Path, dict[str, list[Path]]]) return matches[0] if len(matches) == 1 else None +def _folded_entries( + folder: Path, listings: dict[Path, dict[str, list[Path]]] +) -> dict[str, list[Path]]: + """Everything in ``folder``, keyed by folded name; one read per folder. + + Unlike :func:`_folded_matches` this never probes an exact spelling first, + because an exact probe on a case-folding filesystem answers for a name it + cannot report: ``SDD.PNG`` stats true against an on-disk ``sdd.png`` and + the real spelling is never seen. Listing is also the cheaper half of the + trade, since a document's media probes the same few folders over and over. + """ + if folder not in listings: + entries: dict[str, list[Path]] = {} + try: + for path in folder.iterdir(): + entries.setdefault(_fold(path.name), []).append(path) + except OSError: + pass # missing or unreadable folder: nothing resolves out of it + listings[folder] = entries + return listings[folder] + + +def _media_matches( + base: Path, relative: Path, listings: dict[Path, dict[str, list[Path]]] +) -> list[Path]: + """Every file under ``base`` that ``relative`` names, folding each component. + + Component by component, because a folder spelled another way hides + everything under it: a Windows-authored ``Pictures\\`` read on a + case-sensitive filesystem would otherwise take every href across it with + it. + + A href reaching outside the folder (``..``) is probed as written and not + folded: which directory to search would be a guess. + """ + parts = relative.parts + if not parts: + return [] + if not _foldable(parts): + candidate = base / relative + try: + return [candidate] if candidate.is_file() else [] + except OSError: + return [] + current = [base] + for part in parts[:-1]: + current = [ + match + for folder in current + for match in _folded_entries(folder, listings).get(_fold(part), ()) + if match.is_dir() + ] + if not current: + return [] + return [ + match + for folder in current + for match in _folded_entries(folder, listings).get(_fold(parts[-1]), ()) + if match.is_file() + ] + + +def _foldable(parts: tuple[str, ...]) -> bool: + """Whether every component of a relative path names something to look up.""" + return not any(part in (".", "..") for part in parts) + + +def _href_components(href: str, found: Path) -> list[tuple[str, str]]: + """The href's own components paired with the on-disk names they reached. + + Only what the href wrote: the conventional ``audio/``/``pictures/`` + subfolder is sil-lift's guess, so its spelling is nobody's defect and it is + left out of the comparison. + """ + relative = _normalize_href(href) + if relative is None or not _foldable(relative.parts): + return [] + written = relative.parts + return list(zip(written, found.parts[-len(written) :], strict=True)) + + +def _spelled_exactly(href: str, found: Path) -> bool: + """Whether ``href`` names ``found`` byte for byte, not merely under folding.""" + return all(written == on_disk for written, on_disk in _href_components(href, found)) + + class _Candidate(NamedTuple): """A place a companion may be found, and what sent :meth:`Lexicon.load` there.""" @@ -1060,9 +1162,9 @@ def iter_problems(self, *, require_ids: bool = False) -> Iterator[Problem]: only when companion discovery ran: a lexicon loaded with ``resolve_ranges=False`` put companions out of scope, so none is reported for it, and attaching one afterwards with - :meth:`add_ranges_file` does not reinstate them. ``missing-media`` is - unaffected: media is never resolved into the model, so nothing was - opted out of. + :meth:`add_ranges_file` does not reinstate them. ``missing-media`` and + ``media-href-mismatch`` are unaffected: media is never resolved into + the model, so nothing was opted out of. """ from ._validate import iter_lexicon_problems @@ -1126,26 +1228,38 @@ def media_refs(self) -> Iterator[MediaRef]: illustration.href, "illustration", entry.id, entry.guid, sense.id ) - def missing_media(self) -> list[MediaRef]: - """Media references whose files don't exist in the LIFT folder layout. + def check_media(self) -> list[MediaResolution]: + """Media references that don't resolve cleanly in the LIFT folder layout. A relative href is checked as given (backslashes normalized) and under the conventional subfolder (``audio/`` for media, ``pictures/`` for illustrations). Remote/absolute hrefs can't be checked and are skipped. + + A reference whose href names its file exactly is not reported. One that + reaches a file only under case folding or NFC is a ``mismatch``: the + file is here, but a case-sensitive host serving this folder will not + find it. One that reaches nothing is ``missing``. """ if self.path is None: return [] base = self.path.parent subfolder = {"media": "audio", "illustration": "pictures"} - missing = [] + listings: dict[Path, dict[str, list[Path]]] = {} + unresolved = [] for ref in self.media_refs(): relative = _normalize_href(ref.href) if relative is None: continue - candidates = [base / relative, base / subfolder[ref.kind] / relative] - if not any(candidate.is_file() for candidate in candidates): - missing.append(ref) - return missing + matches = [ + match + for candidate in (relative, Path(subfolder[ref.kind]) / relative) + for match in _media_matches(base, candidate, listings) + ] + if not matches: + unresolved.append(MediaResolution(ref, "missing")) + elif not any(_spelled_exactly(ref.href, match) for match in matches): + unresolved.append(MediaResolution(ref, "mismatch", matches[0])) + return unresolved def find(self, *, id: str | None = None, guid: str | None = None) -> Entry | None: """The first entry matching the given id and/or guid, or None. diff --git a/src/sil_lift/_validate.py b/src/sil_lift/_validate.py index eb7b9da..fd0f174 100644 --- a/src/sil_lift/_validate.py +++ b/src/sil_lift/_validate.py @@ -47,6 +47,7 @@ Lexicon, _existing_file, _folded_matches, + _href_components, _normalize_href, _ranges_candidates, _same_file, @@ -55,9 +56,10 @@ if TYPE_CHECKING: import os - from collections.abc import Collection, Iterator + from collections.abc import Collection, Iterable, Iterator from ._header import Range + from ._model import MediaResolution __all__ = ["Problem", "iter_problems", "validate_file"] @@ -321,6 +323,35 @@ def _form_shape_problems( ) +def media_mismatch_groups( + resolutions: Iterable[MediaResolution], +) -> tuple[list[tuple[str, str]], list[tuple[MediaResolution, str, str]]]: + """Media spelling mismatches split into folder findings and file findings. + + A folder the hrefs misspell is one rename however many references cross it, + so it is reported once and carries no entry; a misspelled filename is one + rename each, addressed to the entry that wrote it. Without the split, a + folder authored as ``Pictures\\`` would report once per media reference in + the document. + + Shared with the CLI's ``check-media``, which groups the same way. + """ + directories: dict[tuple[str, str], None] = {} + files: list[tuple[MediaResolution, str, str]] = [] + for resolution in resolutions: + if resolution.status != "mismatch" or resolution.found is None: + continue + components = _href_components(resolution.ref.href, resolution.found) + for index, (written, on_disk) in enumerate(components): + if written == on_disk: + continue + if index == len(components) - 1: + files.append((resolution, written, on_disk)) + else: + directories[(written, on_disk)] = None + return list(directories), files + + def _semantic_problems( lexicon: Lexicon, entry_lines: list[tuple[int | None, str | None, str | None]], @@ -673,13 +704,38 @@ def range_named(name: str) -> str | None: file=ranges_paths.get(range_id) or file, ) - # Missing media files. - for media_ref in lexicon.missing_media(): + # Media files that are absent, or here under another spelling. + resolutions = lexicon.check_media() + for resolution in resolutions: + if resolution.status != "missing": + continue yield Problem( "warning", "missing-media", - f"{media_ref.kind} file not found: {media_ref.href!r}", + f"{resolution.ref.kind} file not found: {resolution.ref.href!r}", + file=file, + entry_id=resolution.ref.entry_id, + guid=resolution.ref.entry_guid, + ) + directories, files = media_mismatch_groups(resolutions) + # The two spellings can render identically, so name them by code point. + for written, on_disk in directories: + yield Problem( + "warning", + "media-href-mismatch", + f"media hrefs name folder {written!a}, which is {on_disk!a} on disk; they " + "match only under case folding or Unicode normalization, so a " + "case-sensitive host will not find any file under it", + file=file, + ) + for resolution, written, on_disk in files: + yield Problem( + "warning", + "media-href-mismatch", + f"{resolution.ref.kind} href names {written!a}, which is {on_disk!a} on disk; " + "they match only under case folding or Unicode normalization, so a " + "case-sensitive host will not find it", file=file, - entry_id=media_ref.entry_id, - guid=media_ref.entry_guid, + entry_id=resolution.ref.entry_id, + guid=resolution.ref.entry_guid, ) diff --git a/tests/corpus/PROVENANCE.md b/tests/corpus/PROVENANCE.md index 826dc91..572dfd3 100644 --- a/tests/corpus/PROVENANCE.md +++ b/tests/corpus/PROVENANCE.md @@ -71,7 +71,7 @@ that script; committed so tests don't depend on regeneration. `pictures/cultural law.png` (space in filename). The file has a UTF-8 BOM and tab-indented attribute-per-line formatting — a byte-fidelity edge case. Upstream `Moma.WeSayConfig` not taken (not LIFT). Primary fixture for - media_refs()/missing_media(). + media_refs()/check_media(). ## misc/sample.lift @@ -184,7 +184,7 @@ What did match only after normalizing is still reported, as a `normalization-mismatch` warning per range-element id: 2 parent links and 80 grammatical-info values reach 5 ids in Sango, one of them under both aliases of the part-of-speech list, so 6 warnings — and none in the other fixtures. -`negative/nfd-range-ids.lift` is the hand-authored version of the same shape. +`negative/nfd-range-ids/` is the hand-authored version of the same shape. ## generated/ — synthetic large files (not committed) @@ -193,15 +193,16 @@ git-ignored, regenerated on demand. ## negative/ — invalid fixtures (hand-authored) -Each file carries an XML comment documenting its defect and the expected -Problem code: `duplicate-guid`, `dangling-ref`, `range-parent`, -`undefined-range-value` (2 warnings \+ a clean control entry), -`duplicate-form-lang` (the Schematron-only rule), `schema-invalid` -(structural), `missing-media/` (a folder fixture), `flex-quirks` -(URI quirks that must yield warnings, never schema errors), and -`nfd-range-ids` (a `.lift` \+ `.lift-ranges` pair carrying FLEx's -historical normalization asymmetry: NFD ids, NFC references, and one -parent that dangles in every normalization). +One defect class per fixture, each carrying an XML comment that names the +defect and the Problem code it must yield. Some hold several instances of +their defect, and some a clean control entry beside them, pinning a check +against over-firing as well as under-firing. + +A defect contained in one document is a single `.lift`. A defect that needs +more than the document gets a subfolder named for it, holding the `.lift` and +everything its check reads. Any media files in those folders are empty +placeholders rather than fetched content: nothing ever reads their bytes. + `schema-invalid.lift` and `flex-quirks.lift` are raw-RNG-invalid (the latter only under libxml2's anyURI check) and appear in the corpus test's expected-invalid list. diff --git a/tests/corpus/negative/media-href-mismatch/Audio/word.wav b/tests/corpus/negative/media-href-mismatch/Audio/word.wav new file mode 100644 index 0000000..e69de29 diff --git a/tests/corpus/negative/media-href-mismatch/media-href-mismatch.lift b/tests/corpus/negative/media-href-mismatch/media-href-mismatch.lift new file mode 100644 index 0000000..8e1feb4 --- /dev/null +++ b/tests/corpus/negative/media-href-mismatch/media-href-mismatch.lift @@ -0,0 +1,34 @@ + + + + +one + + + + + +
two
 + + + + + +
three
 + +
three
+ + + +three + + + diff --git a/tests/corpus/negative/media-href-mismatch/pictures/other.png b/tests/corpus/negative/media-href-mismatch/pictures/other.png new file mode 100644 index 0000000..e69de29 diff --git a/tests/corpus/negative/media-href-mismatch/pictures/sdd.png b/tests/corpus/negative/media-href-mismatch/pictures/sdd.png new file mode 100644 index 0000000..e69de29 diff --git a/tests/corpus/negative/nfd-range-ids.lift b/tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift similarity index 100% rename from tests/corpus/negative/nfd-range-ids.lift rename to tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift diff --git a/tests/corpus/negative/nfd-range-ids.lift-ranges b/tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift-ranges similarity index 100% rename from tests/corpus/negative/nfd-range-ids.lift-ranges rename to tests/corpus/negative/nfd-range-ids/nfd-range-ids.lift-ranges diff --git a/tests/test_cli.py b/tests/test_cli.py index 1712365..2806ea4 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -78,6 +78,32 @@ def test_validate_no_check_media(capsys: pytest.CaptureFixture[str]) -> None: assert "[missing-media]" not in capsys.readouterr().out +def test_validate_no_check_media_covers_spelling_findings( + capsys: pytest.CaptureFixture[str], +) -> None: + # The flag is for media that lives outside the folder; a spelling verdict + # on files it was told not to check would contradict that. + path = CORPUS_DIR / "negative" / "media-href-mismatch" / "media-href-mismatch.lift" + assert main(["validate", str(path)]) == 0 + assert "[media-href-mismatch]" in capsys.readouterr().out + assert main(["validate", str(path), "--no-check-media"]) == 0 + assert "[media-href-mismatch]" not in capsys.readouterr().out + + +def test_check_media_flags_mismatch_without_calling_the_file_orphaned( + capsys: pytest.CaptureFixture[str], +) -> None: + path = CORPUS_DIR / "negative" / "media-href-mismatch" / "media-href-mismatch.lift" + assert main(["check-media", str(path)]) == 1 + out = capsys.readouterr().out + assert "0 missing, 2 mismatched, 0 orphaned" in out + # The files are named inexactly, not unnamed: reporting them orphaned as + # well as unresolved is the second half of the same bug. + assert "no media/illustration references it" not in out + assert "'SDD.PNG' is 'sdd.png' on disk" in out + assert "'Pictures' is 'pictures' on disk" in out + + def test_validate_require_ids(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: path = tmp_path / "noid.lift" path.write_bytes( @@ -189,7 +215,7 @@ def test_check_media_absolute_href_does_not_mark_local_file_referenced( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: # An absolute href (FLEx-style dangling path) must not mark a folder file - # as referenced — mirrors missing_media(), which skips non-relative hrefs. + # as referenced — mirrors check_media(), which skips non-relative hrefs. audio = tmp_path / "audio" audio.mkdir() wav = audio / "one.wav" @@ -305,7 +331,7 @@ def test_export_filename_with_space(tmp_path: Path) -> None: def test_validate_text_output_survives_a_cp1252_stdout(monkeypatch: pytest.MonkeyPatch) -> None: raw = _redirect(monkeypatch, "stdout", "cp1252") - assert main(["validate", str(CORPUS_DIR / "negative" / "nfd-range-ids.lift")]) == 1 + assert main(["validate", str(CORPUS_DIR / "negative" / "nfd-range-ids" / "nfd-range-ids.lift")]) == 1 sys.stdout.flush() out = raw.getvalue().decode("utf-8") assert unicodedata.normalize("NFD", "Órfão") in out # the id cp1252 cannot hold diff --git a/tests/test_ranges_folder.py b/tests/test_ranges_folder.py index 3a56d23..8ca52af 100644 --- a/tests/test_ranges_folder.py +++ b/tests/test_ranges_folder.py @@ -199,31 +199,87 @@ def test_ranges_schema_is_loadable_and_spec_faithful() -> None: ) -def test_media_refs_and_missing_media_on_moma_folder() -> None: +def test_media_refs_and_check_media_on_moma_folder() -> None: lexicon = sil_lift.load(CORPUS_DIR / "folder" / "Moma" / "Moma.lift") refs = list(lexicon.media_refs()) assert {r.href for r in refs} == {"pictures\\cultural law.png", "pictures\\sdd.png"} assert all(r.kind == "illustration" for r in refs) assert all(r.entry_id for r in refs) - assert lexicon.missing_media() == [] + # Every href spells its file exactly, so nothing is reported at all. + assert lexicon.check_media() == [] -def test_missing_media_on_all_flex_fields() -> None: +def test_check_media_on_all_flex_fields() -> None: # The corpus deliberately omits the upstream filler media (PROVENANCE.md), # so these references must be reported missing. lexicon = sil_lift.load(CORPUS_DIR / "flex" / "AllFLExFields" / "AllFLExFields.lift") - missing = {(r.kind, r.href) for r in lexicon.missing_media()} + missing = {(r.ref.kind, r.ref.href) for r in lexicon.check_media() if r.status == "missing"} assert ("media", "Kalimba.mp3") in missing assert ("illustration", "Desert.jpg") in missing -def test_missing_media_flags_broken_ref(tmp_path: Path) -> None: +def test_check_media_flags_broken_ref(tmp_path: Path) -> None: src = CORPUS_DIR / "folder" / "Moma" shutil.copytree(src, tmp_path / "Moma") lexicon = sil_lift.load(tmp_path / "Moma" / "Moma.lift") (tmp_path / "Moma" / "pictures" / "sdd.png").unlink() - missing = lexicon.missing_media() - assert [r.href for r in missing] == ["pictures\\sdd.png"] + assert [(r.ref.href, r.status) for r in lexicon.check_media()] == [ + ("pictures\\sdd.png", "missing") + ] + + +def _write_lift_with_illustration(folder: Path, href: str) -> Path: + """A minimal loadable .lift whose one sense illustrates ``href``.""" + folder.mkdir(parents=True, exist_ok=True) + path = folder / "media.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + f'\n' + "\n" + "\n", + encoding="utf-8", + ) + return path + + +def test_check_media_reports_a_normalization_only_match(tmp_path: Path) -> None: + # Not a corpus fixture: every checkout filesystem preserves case in a + # filename, but HFS+ rewrites normalization, so a committed NFD/NFC pair + # has a premise the checkout itself could alter. + name_nfc = unicodedata.normalize("NFC", "café.png") + name_nfd = unicodedata.normalize("NFD", "café.png") + assert name_nfc != name_nfd + (tmp_path / "pictures").mkdir() + (tmp_path / "pictures" / name_nfd).write_bytes(b"") + path = _write_lift_with_illustration(tmp_path, f"pictures/{name_nfc}") + resolutions = sil_lift.load(path).check_media() + assert [r.status for r in resolutions] == ["mismatch"] + assert resolutions[0].found is not None + assert resolutions[0].found.name == name_nfd + + +def test_check_media_reports_a_href_that_several_files_fold_onto(tmp_path: Path) -> None: + if not _case_sensitive(tmp_path): + pytest.skip("needs a case-sensitive filesystem to hold both spellings") + (tmp_path / "pictures").mkdir() + for name in ("SDD.PNG", "sdd.png"): + (tmp_path / "pictures" / name).write_bytes(b"") + # Matching neither exactly is what makes it ambiguous; it is still one + # defect with one fix, so it reports as an ordinary mismatch. + path = _write_lift_with_illustration(tmp_path, "pictures/Sdd.png") + assert [r.status for r in sil_lift.load(path).check_media()] == ["mismatch"] + + +def test_check_media_resolves_an_exactly_spelled_href_above_the_folder(tmp_path: Path) -> None: + # ".." names no directory to search, so it is probed as written rather than + # folded -- and must keep resolving. + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "one.png").write_bytes(b"") + path = _write_lift_with_illustration(tmp_path / "lex", "../shared/one.png") + assert sil_lift.load(path).check_media() == [] def _write_case_variant_pair(folder: Path, lift_name: str, ranges_name: str) -> Path: diff --git a/tests/test_validate.py b/tests/test_validate.py index a307fa5..9d1ca45 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -167,7 +167,7 @@ def test_header_range_id_reaches_a_companion_id_in_another_normalization() -> No def test_nfd_ids_warn_once_and_still_flag_the_real_dangling_parent(tmp_path: Path) -> None: - path = NEGATIVE_DIR / "nfd-range-ids.lift" + path = NEGATIVE_DIR / "nfd-range-ids" / "nfd-range-ids.lift" problems = problems_for(path) assert codes(problems) == {("error", "range-parent"), ("warning", "normalization-mismatch")} (dangling,) = [p for p in problems if p.code == "range-parent"] @@ -184,7 +184,7 @@ def test_nfd_ids_warn_once_and_still_flag_the_real_dangling_parent(tmp_path: Pat # Normalization belongs to the comparison only: the mixed forms survive. sil_lift.Lexicon.load(path).save(tmp_path / path.name) for name in (path.name, "nfd-range-ids.lift-ranges"): - assert (tmp_path / name).read_bytes() == (NEGATIVE_DIR / name).read_bytes(), name + assert (tmp_path / name).read_bytes() == (path.parent / name).read_bytes(), name def test_one_id_referenced_in_two_spellings_still_warns_once() -> None: @@ -342,6 +342,30 @@ def test_missing_media_folder_fixture() -> None: assert any("gone.png" in m for m in hrefs) +def test_media_href_mismatch_folder_fixture() -> None: + # No skipif: the whole point of resolving against the directory listing is + # that a case-folding host reaches the same verdict as a case-sensitive one. + problems = problems_for(NEGATIVE_DIR / "media-href-mismatch" / "media-href-mismatch.lift") + assert not [p for p in problems if p.code == "missing-media"], "every file is present" + mismatches = [p for p in problems if p.code == "media-href-mismatch"] + assert all(p.level == "warning" for p in mismatches) + # The misspelled folder is one rename, so it reports once and names no + # entry; the misspelled filename is addressed to the entry that wrote it. + folder = [p for p in mismatches if p.entry_id is None] + assert len(folder) == 1 + assert "'Pictures'" in folder[0].message and "'pictures'" in folder[0].message + files = [p for p in mismatches if p.entry_id is not None] + assert [(p.entry_id, "'SDD.PNG'" in p.message) for p in files] == [("one", True)] + + +def test_media_href_mismatch_leaves_the_conventional_subfolder_alone() -> None: + # Entry "three" writes a bare "word.wav" that only resolves because the + # guessed audio/ folds onto the on-disk Audio/. That component is sil-lift's + # own, so its spelling is nobody's defect and must never be reported. + lexicon = sil_lift.load(NEGATIVE_DIR / "media-href-mismatch" / "media-href-mismatch.lift") + assert [r.ref.entry_id for r in lexicon.check_media()] == ["one", "two"] + + def test_flex_uri_quirks_warn_but_never_error() -> None: problems = problems_for(NEGATIVE_DIR / "flex-quirks.lift") assert problems, "the quirky URIs must be reported" From 33b763e5ff70d8e3594828c9ecef15ddc92c95e5 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 23 Sep 2026 12:28:14 -0400 Subject: [PATCH 2/5] Report orphans by file identity, and a folder mismatch per folder check-media built its referenced set from the paths the hrefs spell rather than the files they reach, so a href crossing a dot segment -- audio/sub/../one.wav against an on-disk audio/one.wav -- matched nothing the folder walk found and printed as orphaned though it resolves. The set now holds the files media resolution returns, compared by identity, which also spares the command from re-deriving rules the model owns. A folder mismatch was keyed by the one component that differs, so A/Foo and B/Foo collapsed into one finding although each is its own rename. The key is the whole path the href reached, which the message now names too. Co-Authored-By: Claude Opus 5 (1M context) --- src/sil_lift/_cli.py | 22 +++++++++------------- src/sil_lift/_validate.py | 8 +++++++- tests/test_cli.py | 25 ++++++++++++++++++++++++- tests/test_validate.py | 25 +++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/sil_lift/_cli.py b/src/sil_lift/_cli.py index 6f5b6f7..8ca6bb4 100644 --- a/src/sil_lift/_cli.py +++ b/src/sil_lift/_cli.py @@ -25,7 +25,7 @@ from ._canonical import canonicalize from ._errors import LiftError -from ._model import Lexicon, _fold, _folded_entries, _normalize_href +from ._model import Lexicon, _folded_entries, _media_matches, _normalize_href from ._stream import open_reader from ._validate import iter_problems, media_mismatch_groups @@ -169,18 +169,19 @@ def _cmd_check_media(args: argparse.Namespace) -> int: owner = item.ref.entry_id or item.ref.entry_guid or "?" print(f"mismatch {item.ref.kind:12s} {written!a} is {on_disk!a} on disk (entry {owner})") - referenced: set[str] = set() + # The files the hrefs reach, under the folding check_media() uses, rather + # than the paths they spell: a file named inexactly is in use, not orphaned. + referenced: set[Path] = set() base = lexicon.path.parent if lexicon.path is not None else Path(args.path).parent + listings: dict[Path, dict[str, list[Path]]] = {} for ref in lexicon.media_refs(): relative = _normalize_href(ref.href) if relative is None: # remote/absolute hrefs can't confirm a local file continue subfolder = "audio" if ref.kind == "media" else "pictures" - referenced.add(_folded_key(relative)) - referenced.add(_folded_key(Path(subfolder) / relative)) - # Folded, not resolved: a href reaches its file under the same folding - # check_media() uses, so a file it names inexactly is in use, not orphaned. - listings: dict[Path, dict[str, list[Path]]] = {} + for candidate in (relative, Path(subfolder) / relative): + for match in _media_matches(base, candidate, listings): + referenced.add(match.resolve()) media_folders = [ path for name in ("audio", "pictures") @@ -191,7 +192,7 @@ def _cmd_check_media(args: argparse.Namespace) -> int: file for folder in media_folders for file in sorted(folder.rglob("*")) - if file.is_file() and _folded_key(file.relative_to(base)) not in referenced + if file.is_file() and file.resolve() not in referenced ] for file in orphans: print(f"orphaned {file.relative_to(base)} (no media/illustration references it)") @@ -205,11 +206,6 @@ def _cmd_check_media(args: argparse.Namespace) -> int: return 1 if missing or mismatched else 0 -def _folded_key(relative: Path) -> str: - """A relative path reduced to what a case-folding filesystem treats as one path.""" - return "/".join(_fold(part) for part in relative.parts) - - def _leaf_senses(entry: Entry) -> list[Sense]: """The entry's senses that carry content, document order. diff --git a/src/sil_lift/_validate.py b/src/sil_lift/_validate.py index fd0f174..960eb8a 100644 --- a/src/sil_lift/_validate.py +++ b/src/sil_lift/_validate.py @@ -334,6 +334,9 @@ def media_mismatch_groups( folder authored as ``Pictures\\`` would report once per media reference in the document. + A folder is keyed by its whole path rather than the one component that + differs, since ``A/Foo`` and ``B/Foo`` are two renames, not one. + Shared with the CLI's ``check-media``, which groups the same way. """ directories: dict[tuple[str, str], None] = {} @@ -348,7 +351,10 @@ def media_mismatch_groups( if index == len(components) - 1: files.append((resolution, written, on_disk)) else: - directories[(written, on_disk)] = None + reached = components[: index + 1] + directories[ + ("/".join(part for part, _ in reached), "/".join(part for _, part in reached)) + ] = None return list(directories), files diff --git a/tests/test_cli.py b/tests/test_cli.py index 2806ea4..d293786 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -90,6 +90,28 @@ def test_validate_no_check_media_covers_spelling_findings( assert "[media-href-mismatch]" not in capsys.readouterr().out +def test_check_media_follows_a_dot_segment_href_to_its_file( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # "audio/sub/../one.wav" and the "audio/one.wav" the folder walk finds are + # one file: the href resolves, so nothing is missing and nothing orphaned. + (tmp_path / "audio" / "sub").mkdir(parents=True) + (tmp_path / "audio" / "one.wav").write_bytes(b"") + path = tmp_path / "dots.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + "\n" + "\n", + encoding="utf-8", + ) + assert main(["check-media", str(path)]) == 0 + assert "0 missing, 0 mismatched, 0 orphaned" in capsys.readouterr().out + + def test_check_media_flags_mismatch_without_calling_the_file_orphaned( capsys: pytest.CaptureFixture[str], ) -> None: @@ -331,7 +353,8 @@ def test_export_filename_with_space(tmp_path: Path) -> None: def test_validate_text_output_survives_a_cp1252_stdout(monkeypatch: pytest.MonkeyPatch) -> None: raw = _redirect(monkeypatch, "stdout", "cp1252") - assert main(["validate", str(CORPUS_DIR / "negative" / "nfd-range-ids" / "nfd-range-ids.lift")]) == 1 + path = CORPUS_DIR / "negative" / "nfd-range-ids" / "nfd-range-ids.lift" + assert main(["validate", str(path)]) == 1 sys.stdout.flush() out = raw.getvalue().decode("utf-8") assert unicodedata.normalize("NFD", "Órfão") in out # the id cp1252 cannot hold diff --git a/tests/test_validate.py b/tests/test_validate.py index 9d1ca45..81d9d45 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -358,6 +358,31 @@ def test_media_href_mismatch_folder_fixture() -> None: assert [(p.entry_id, "'SDD.PNG'" in p.message) for p in files] == [("one", True)] +def test_media_href_mismatch_reports_each_misspelled_folder_separately(tmp_path: Path) -> None: + # One misspelled component under two parents is two renames, so keying the + # finding on the component alone would undercount the work. + for parent in ("a", "b"): + (tmp_path / parent / "foo").mkdir(parents=True) + (tmp_path / parent / "foo" / f"{parent}.png").write_bytes(b"") + path = tmp_path / "two.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + '\n' + "\n" + "\n", + encoding="utf-8", + ) + problems = problems_for(path) + mismatches = [p for p in problems if p.code == "media-href-mismatch"] + assert len(mismatches) == 2 + assert any("'a/Foo'" in p.message for p in mismatches) + assert any("'b/Foo'" in p.message for p in mismatches) + + def test_media_href_mismatch_leaves_the_conventional_subfolder_alone() -> None: # Entry "three" writes a bare "word.wav" that only resolves because the # guessed audio/ folds onto the on-disk Audio/. That component is sil-lift's From 4a4f2630c05b260c1acc5484f1663a2333845a09 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 23 Sep 2026 12:43:55 -0400 Subject: [PATCH 3/5] Key a folder mismatch by the folder, not the path written to it A folder mismatch was keyed by the whole path an href spells, so one folder reached through two spellings of its parent -- A/Foo and a/Foo over a single a/foo -- reported twice although both name the one rename. The key is now the on-disk path the href reaches, together with the component it spells that folder with: enough to tell a/foo from b/foo, and no longer split by an ancestor that has nothing to do with either. The message names that component and the path it reaches rather than the whole written path, so it identifies a real folder instead of one href's way of writing its way down to it. Co-Authored-By: Claude Opus 5 (1M context) --- src/sil_lift/_validate.py | 9 ++++----- tests/corpus/PROVENANCE.md | 2 +- tests/test_validate.py | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/sil_lift/_validate.py b/src/sil_lift/_validate.py index 960eb8a..9490169 100644 --- a/src/sil_lift/_validate.py +++ b/src/sil_lift/_validate.py @@ -334,8 +334,9 @@ def media_mismatch_groups( folder authored as ``Pictures\\`` would report once per media reference in the document. - A folder is keyed by its whole path rather than the one component that - differs, since ``A/Foo`` and ``B/Foo`` are two renames, not one. + A folder finding is keyed by the on-disk path it reaches and the component + the href spells it with: ``a/foo`` and ``b/foo`` are two renames, while one + folder reached through two spellings of its parent is still one. Shared with the CLI's ``check-media``, which groups the same way. """ @@ -352,9 +353,7 @@ def media_mismatch_groups( files.append((resolution, written, on_disk)) else: reached = components[: index + 1] - directories[ - ("/".join(part for part, _ in reached), "/".join(part for _, part in reached)) - ] = None + directories[(written, "/".join(part for _, part in reached))] = None return list(directories), files diff --git a/tests/corpus/PROVENANCE.md b/tests/corpus/PROVENANCE.md index 572dfd3..b5f0558 100644 --- a/tests/corpus/PROVENANCE.md +++ b/tests/corpus/PROVENANCE.md @@ -71,7 +71,7 @@ that script; committed so tests don't depend on regeneration. `pictures/cultural law.png` (space in filename). The file has a UTF-8 BOM and tab-indented attribute-per-line formatting — a byte-fidelity edge case. Upstream `Moma.WeSayConfig` not taken (not LIFT). Primary fixture for - media_refs()/check_media(). + `media_refs()`/`check_media()`. ## misc/sample.lift diff --git a/tests/test_validate.py b/tests/test_validate.py index 81d9d45..ed7298c 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -379,8 +379,37 @@ def test_media_href_mismatch_reports_each_misspelled_folder_separately(tmp_path: problems = problems_for(path) mismatches = [p for p in problems if p.code == "media-href-mismatch"] assert len(mismatches) == 2 - assert any("'a/Foo'" in p.message for p in mismatches) - assert any("'b/Foo'" in p.message for p in mismatches) + assert any("'a/foo'" in p.message for p in mismatches) + assert any("'b/foo'" in p.message for p in mismatches) + + +def test_media_href_mismatch_reports_one_folder_once_however_hrefs_reach_it( + tmp_path: Path, +) -> None: + # One folder is one rename however inconsistently the hrefs spell the way + # down to it: 'A/Foo' and 'a/Foo' reach the same 'a/foo' and report once. + (tmp_path / "a" / "foo").mkdir(parents=True) + for name in ("one.png", "two.png"): + (tmp_path / "a" / "foo" / name).write_bytes(b"") + path = tmp_path / "nested.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + '\n' + "\n" + "\n", + encoding="utf-8", + ) + problems = problems_for(path) + mismatches = [p for p in problems if p.code == "media-href-mismatch"] + # Two real defects: the parent written 'A', and 'a/foo' written 'Foo'. + assert sorted(p.message.split(";")[0] for p in mismatches) == [ + "media hrefs name folder 'A', which is 'a' on disk", + "media hrefs name folder 'Foo', which is 'a/foo' on disk", + ] def test_media_href_mismatch_leaves_the_conventional_subfolder_alone() -> None: From 8ef22e66b84081a6e1d26740e0f9c64a93bf15b6 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 23 Sep 2026 13:19:21 -0400 Subject: [PATCH 4/5] Identify a mismatched folder by the file that reached it A folder finding was keyed by a path aligned against the href, which describes a/foo and pictures/a/foo identically: the conventional subfolder that separates them is the lookup's, and the document never wrote it. Two such folders merged into one finding, and the survivor named the wrong path. The key is now the directory the resolved file sits in, together with the component the href spells it with, so one folder written two ways still reports twice. Folder paths are reported relative to the folder holding the .lift, which is what lets the message tell pictures/a/foo from a/foo. Every path that was already unambiguous reads exactly as before. Co-Authored-By: Claude Opus 5 (1M context) --- src/sil_lift/_cli.py | 4 ++-- src/sil_lift/_validate.py | 28 ++++++++++++++++++---------- tests/test_validate.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/sil_lift/_cli.py b/src/sil_lift/_cli.py index 8ca6bb4..e4e2127 100644 --- a/src/sil_lift/_cli.py +++ b/src/sil_lift/_cli.py @@ -156,12 +156,13 @@ def _cmd_sort(args: argparse.Namespace) -> int: def _cmd_check_media(args: argparse.Namespace) -> int: lexicon = Lexicon.load(args.path) + base = lexicon.path.parent if lexicon.path is not None else Path(args.path).parent resolutions = lexicon.check_media() missing = [item for item in resolutions if item.status == "missing"] for item in missing: owner = item.ref.entry_id or item.ref.entry_guid or "?" print(f"missing {item.ref.kind:12s} {item.ref.href!r} (entry {owner})") - directories, files = media_mismatch_groups(resolutions) + directories, files = media_mismatch_groups(resolutions, base) # Spellings differing only in normalization render identically. for written, on_disk in directories: print(f"mismatch {'folder':12s} {written!a} is {on_disk!a} on disk") @@ -172,7 +173,6 @@ def _cmd_check_media(args: argparse.Namespace) -> int: # The files the hrefs reach, under the folding check_media() uses, rather # than the paths they spell: a file named inexactly is in use, not orphaned. referenced: set[Path] = set() - base = lexicon.path.parent if lexicon.path is not None else Path(args.path).parent listings: dict[Path, dict[str, list[Path]]] = {} for ref in lexicon.media_refs(): relative = _normalize_href(ref.href) diff --git a/src/sil_lift/_validate.py b/src/sil_lift/_validate.py index 9490169..5af34bb 100644 --- a/src/sil_lift/_validate.py +++ b/src/sil_lift/_validate.py @@ -324,7 +324,7 @@ def _form_shape_problems( def media_mismatch_groups( - resolutions: Iterable[MediaResolution], + resolutions: Iterable[MediaResolution], base: Path ) -> tuple[list[tuple[str, str]], list[tuple[MediaResolution, str, str]]]: """Media spelling mismatches split into folder findings and file findings. @@ -334,13 +334,15 @@ def media_mismatch_groups( folder authored as ``Pictures\\`` would report once per media reference in the document. - A folder finding is keyed by the on-disk path it reaches and the component - the href spells it with: ``a/foo`` and ``b/foo`` are two renames, while one - folder reached through two spellings of its parent is still one. + A folder is identified by the file that reached it, not by what the href + spells. Names taken from the href alone cannot tell ``a/foo`` from + ``pictures/a/foo``: what separates them is the conventional subfolder, + which the lookup supplied and the document never wrote. Paths are reported + relative to ``base``, the folder holding the ``.lift``. Shared with the CLI's ``check-media``, which groups the same way. """ - directories: dict[tuple[str, str], None] = {} + directories: dict[tuple[Path, str], tuple[str, str]] = {} files: list[tuple[MediaResolution, str, str]] = [] for resolution in resolutions: if resolution.status != "mismatch" or resolution.found is None: @@ -351,10 +353,14 @@ def media_mismatch_groups( continue if index == len(components) - 1: files.append((resolution, written, on_disk)) - else: - reached = components[: index + 1] - directories[(written, "/".join(part for _, part in reached))] = None - return list(directories), files + continue + # As many levels above the file as there are components after this + # one -- the directory this component actually named. + folder = resolution.found.parents[len(components) - index - 2] + directories.setdefault( + (folder, written), (written, folder.relative_to(base).as_posix()) + ) + return list(directories.values()), files def _semantic_problems( @@ -722,7 +728,9 @@ def range_named(name: str) -> str | None: entry_id=resolution.ref.entry_id, guid=resolution.ref.entry_guid, ) - directories, files = media_mismatch_groups(resolutions) + # A pathless lexicon resolves no media at all, so there is nothing to group. + media_base = lexicon.path.parent if lexicon.path is not None else Path() + directories, files = media_mismatch_groups(resolutions, media_base) # The two spellings can render identically, so name them by code point. for written, on_disk in directories: yield Problem( diff --git a/tests/test_validate.py b/tests/test_validate.py index ed7298c..aa1bd0a 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -383,6 +383,35 @@ def test_media_href_mismatch_reports_each_misspelled_folder_separately(tmp_path: assert any("'b/foo'" in p.message for p in mismatches) +def test_media_href_mismatch_tells_apart_two_folders_one_href_could_name( + tmp_path: Path, +) -> None: + # The conventional pictures/ lookup reaches a folder the direct one cannot, + # and both answer to "a/Foo": what the href writes cannot tell them apart, + # only the file each one reached. + (tmp_path / "a" / "foo").mkdir(parents=True) + (tmp_path / "pictures" / "a" / "foo").mkdir(parents=True) + (tmp_path / "a" / "foo" / "one.png").write_bytes(b"") + (tmp_path / "pictures" / "a" / "foo" / "two.png").write_bytes(b"") + path = tmp_path / "roots.lift" + path.write_text( + '\n' + '\n' + '\n' + '
one
\n' + '\n' + '\n' + "\n" + "\n", + encoding="utf-8", + ) + mismatches = [p for p in problems_for(path) if p.code == "media-href-mismatch"] + assert sorted(p.message.split(";")[0] for p in mismatches) == [ + "media hrefs name folder 'Foo', which is 'a/foo' on disk", + "media hrefs name folder 'Foo', which is 'pictures/a/foo' on disk", + ] + + def test_media_href_mismatch_reports_one_folder_once_however_hrefs_reach_it( tmp_path: Path, ) -> None: From cd98d5dc11d26c1e83daffdf12f28b57dd6d643d Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 8 Oct 2026 15:01:44 -0400 Subject: [PATCH 5/5] Harden media resolution against empty hrefs and stat failures An href of "" or "." has no components, so appending it to the conventional subfolder named the subfolder itself; a regular file called pictures or audio then matched, and comparing the href's spelling against it raised ValueError. check_media() now reports such an href missing before any probe, and check-media's orphan scan skips it. The folder walk in _media_matches stats entries through helpers that treat any OSError as "not there": before Python 3.14, pathlib lets a PermissionError escape is_file()/is_dir(). Folder listings are sorted, so which of several folding matches is reported, and the folder a mismatch is grouped under, is the same on every filesystem. CONTRIBUTING now asks for a PROVENANCE.md entry only for fetched fixtures, matching the rule that file states for hand-authored negative/ fixtures. The CLI guide's exit codes count misspelled media as a finding. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 9 +++++---- docs/en/guides/cli.md | 2 +- src/sil_lift/_cli.py | 2 +- src/sil_lift/_model.py | 34 +++++++++++++++++++++++++++++++--- tests/test_ranges_folder.py | 9 +++++++++ 5 files changed, 47 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 02fa940..961e7a1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,10 +44,11 @@ The fidelity tests assert that saving writes back the **exact bytes** of `tests/corpus/`, or `tests/tools/xslt/`. Even a trailing-newline tweak breaks the suite. `.gitattributes` and `.editorconfig` list exceptions so git and editors leave these files alone — don't remove them. -- Adding a fixture requires an entry in `tests/corpus/PROVENANCE.md`: source - URL, commit SHA, fetch date, license. Hand-authored fixtures (e.g. under - `tests/corpus/negative/`) carry an XML comment documenting the defect and the - expected validator finding. +- Adding a fetched fixture requires an entry in `tests/corpus/PROVENANCE.md`: + source URL, commit SHA, fetch date, license. Hand-authored fixtures under + `tests/corpus/negative/` follow the rule in that file's `negative/` section + instead of getting an entry each, and carry an XML comment documenting the + defect and the expected validator finding. - The migrated `spec-examples/0.13/` files are generated by `tests/tools/migrate_corpus.py`; regenerate rather than edit. diff --git a/docs/en/guides/cli.md b/docs/en/guides/cli.md index cb11367..b9c709e 100644 --- a/docs/en/guides/cli.md +++ b/docs/en/guides/cli.md @@ -81,4 +81,4 @@ $ sil-lift export dictionary.lift --langs en,fr -o dictionary.csv All output is UTF-8, on every platform and whether it goes to a console, a pipe, or a `>` redirect — never the locale encoding (cp1252 on Windows, ASCII under a C/POSIX locale), which cannot represent LIFT content. `sil-lift export dictionary.lift > dictionary.csv` therefore writes exactly the bytes `-o dictionary.csv` writes, CRLF row terminators included. -Exit codes: `0` success (warnings do not fail the run unless `--strict`), `1` findings (validation errors / missing media / warnings under `--strict`, not counting codes given to `--allow`), `2` an I/O failure at either end — input that cannot be read, or output that cannot be written (a reader like `head` closing the pipe, a full disk). +Exit codes: `0` success (warnings do not fail the run unless `--strict`), `1` findings (validation errors / missing or misspelled media / warnings under `--strict`, not counting codes given to `--allow`), `2` an I/O failure at either end — input that cannot be read, or output that cannot be written (a reader like `head` closing the pipe, a full disk). diff --git a/src/sil_lift/_cli.py b/src/sil_lift/_cli.py index 8a3ad8c..a239abe 100644 --- a/src/sil_lift/_cli.py +++ b/src/sil_lift/_cli.py @@ -191,7 +191,7 @@ def _cmd_check_media(args: argparse.Namespace) -> int: listings: dict[Path, dict[str, list[Path]]] = {} for ref in lexicon.media_refs(): relative = _normalize_href(ref.href) - if relative is None: # remote/absolute hrefs can't confirm a local file + if relative is None or not relative.parts: # remote/absolute/empty: no local file continue subfolder = "audio" if ref.kind == "media" else "pictures" for candidate in (relative, Path(subfolder) / relative): diff --git a/src/sil_lift/_model.py b/src/sil_lift/_model.py index 82d8903..a8da5b1 100644 --- a/src/sil_lift/_model.py +++ b/src/sil_lift/_model.py @@ -571,7 +571,9 @@ def _folded_entries( if folder not in listings: entries: dict[str, list[Path]] = {} try: - for path in folder.iterdir(): + # Sorted, so which of several folding matches gets reported does + # not depend on the order a filesystem happens to list them in. + for path in sorted(folder.iterdir()): entries.setdefault(_fold(path.name), []).append(path) except OSError: pass # missing or unreadable folder: nothing resolves out of it @@ -607,7 +609,7 @@ def _media_matches( match for folder in current for match in _folded_entries(folder, listings).get(_fold(part), ()) - if match.is_dir() + if _is_dir(match) ] if not current: return [] @@ -615,10 +617,31 @@ def _media_matches( match for folder in current for match in _folded_entries(folder, listings).get(_fold(parts[-1]), ()) - if match.is_file() + if _is_file(match) ] +def _is_file(path: Path) -> bool: + """``path.is_file()``, False where stat fails for any reason. + + Before Python 3.14 pathlib swallows only a few errnos, so a + ``PermissionError`` would otherwise escape a listable but unsearchable + folder. + """ + try: + return path.is_file() + except OSError: + return False + + +def _is_dir(path: Path) -> bool: + """``path.is_dir()``, False where stat fails for any reason.""" + try: + return path.is_dir() + except OSError: + return False + + def _foldable(parts: tuple[str, ...]) -> bool: """Whether every component of a relative path names something to look up.""" return not any(part in (".", "..") for part in parts) @@ -1250,6 +1273,11 @@ def check_media(self) -> list[MediaResolution]: relative = _normalize_href(ref.href) if relative is None: continue + if not relative.parts: + # "" or ".": names no file, and appending it to the + # conventional subfolder would name the subfolder itself. + unresolved.append(MediaResolution(ref, "missing")) + continue matches = [ match for candidate in (relative, Path(subfolder[ref.kind]) / relative) diff --git a/tests/test_ranges_folder.py b/tests/test_ranges_folder.py index 8ca52af..80b7c8f 100644 --- a/tests/test_ranges_folder.py +++ b/tests/test_ranges_folder.py @@ -273,6 +273,15 @@ def test_check_media_reports_a_href_that_several_files_fold_onto(tmp_path: Path) assert [r.status for r in sil_lift.load(path).check_media()] == ["mismatch"] +@pytest.mark.parametrize("href", ["", "."]) +def test_check_media_reports_an_empty_href_missing(tmp_path: Path, href: str) -> None: + # A file named like the conventional subfolder is what an empty href + # appended to it would reach. + (tmp_path / "pictures").write_bytes(b"") + path = _write_lift_with_illustration(tmp_path, href) + assert [r.status for r in sil_lift.load(path).check_media()] == ["missing"] + + def test_check_media_resolves_an_exactly_spelled_href_above_the_folder(tmp_path: Path) -> None: # ".." names no directory to search, so it is probed as written rather than # folded -- and must keep resolving.