From 202f82117154ec5c0826019d9d3c437a9b2511ab Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 12:21:21 -0400 Subject: [PATCH 1/2] Discover nested source-package members so the alias-drift guard sees them discover_source_package_dirs globbed only */*/source_package.yaml, so nested members (such as chronicle#335's rc_XX members) were invisible to find_alias_map_drift and the default-bundle coverage check: removing a nested member's alias was not reported (review of #335, finding 1). Search recursively instead. On main this finds the same 333 packages as before (none missing, none unmapped); on #335's branch the review measured 367, exactly the alias targets. Also correct the _is_missing_object_diagnostic docstring, which still said incidental mentions stay a fetch error (review r2 of #336). Tests: two new cases (nested members discovered; an unmapped nested member reported), both red under the old two-level glob via an in-memory mutant. Co-Authored-By: Claude Opus 5.5 --- .../335-nested-package-discovery.fixed.md | 7 ++++++ chronicle/artifacts.py | 5 +++-- chronicle/source_package.py | 10 +++++---- tests/test_source_package_alias_drift.py | 22 +++++++++++++++++++ 4 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 changelog.d/335-nested-package-discovery.fixed.md diff --git a/changelog.d/335-nested-package-discovery.fixed.md b/changelog.d/335-nested-package-discovery.fixed.md new file mode 100644 index 00000000..601532e2 --- /dev/null +++ b/changelog.d/335-nested-package-discovery.fixed.md @@ -0,0 +1,7 @@ +Make source-package discovery find nested package members, such as +`stats_nz/census_2023_family_type/rc_01`, not just `/` +directories. The alias-drift guard and the default-bundle coverage check rely on +discovery, so a nested member that lost its alias is now reported instead of +being silently left out. Also correct the `verify-raw` diagnostics docstring: an +error line that merely mentions a missing-object marker still counts as +missing. diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index 9b5b5444..b195b1d8 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -1019,8 +1019,9 @@ def _is_missing_object_diagnostic(stderr: str) -> bool: Colour codes are removed first, because Wrangler wraps words in them. A marker counts only as a whole phrase, and only on a line that holds nothing but markers (such as ``NoSuchKey: specified key does not exist``) or is an - error line, so unrelated text that merely mentions a marker stays a fetch - error. + error line. A marker on a line that reports no error stays a fetch error, + but an error line that merely mentions a marker still counts as missing: + both are failures, so neither can produce ``verified``. """ for raw_line in _ANSI_ESCAPE.sub("", stderr).lower().splitlines(): line = " ".join(raw_line.split()) diff --git a/chronicle/source_package.py b/chronicle/source_package.py index c45fde35..58c02b2b 100644 --- a/chronicle/source_package.py +++ b/chronicle/source_package.py @@ -2259,18 +2259,20 @@ def source_package_root() -> Path: def discover_source_package_dirs(root: Path | None = None) -> list[Path]: - """Discover ``/`` directories under a packages root. + """Discover every directory holding a source package under a packages root. Returns repo-relative package paths (e.g. ``usda_snap/fy69_to_current``), - matching the shape of the alias-map values, sorted for determinism. This - is the authoring surface a complete bundle must cover. + matching the shape of the alias-map values, sorted for determinism. Nested + members (e.g. ``stats_nz/census_2023_family_type/rc_01``) are found too, so + the alias-drift guard sees every package a bundle could build. This is the + authoring surface a complete bundle must cover. """ packages_root = root if root is not None else source_package_root() if not packages_root.exists(): return [] discovered = { manifest.parent.relative_to(packages_root) - for manifest in packages_root.glob(f"*/*/{SOURCE_PACKAGE_FILENAME}") + for manifest in packages_root.rglob(SOURCE_PACKAGE_FILENAME) } return sorted(discovered, key=str) diff --git a/tests/test_source_package_alias_drift.py b/tests/test_source_package_alias_drift.py index bd26c21c..d4c4019c 100644 --- a/tests/test_source_package_alias_drift.py +++ b/tests/test_source_package_alias_drift.py @@ -113,6 +113,28 @@ def test_discover_source_package_dirs_scans_given_root(tmp_path): assert discovered == {"alpha/one", "beta/two"} +def test_discover_source_package_dirs_finds_nested_members(tmp_path): + """A package family's nested members are discovered alongside its root.""" + _write_package(tmp_path, "alpha/family") + _write_package(tmp_path, "alpha/family/rc_01") + _write_package(tmp_path, "alpha/family/rc_02") + + discovered = {str(path) for path in discover_source_package_dirs(root=tmp_path)} + assert discovered == {"alpha/family", "alpha/family/rc_01", "alpha/family/rc_02"} + + +def test_find_alias_map_drift_flags_an_unmapped_nested_member(tmp_path): + """Dropping a nested member's alias is reported, not silently skipped.""" + _write_package(tmp_path, "alpha/family") + _write_package(tmp_path, "alpha/family/rc_01") + aliases = {"alpha-family": type(tmp_path)("alpha/family")} + + missing_dirs, unmapped_dirs = find_alias_map_drift(root=tmp_path, aliases=aliases) + + assert missing_dirs == [] + assert [str(path) for path in unmapped_dirs] == ["alpha/family/rc_01"] + + def test_find_alias_map_drift_flags_unmapped_directory(tmp_path): """An on-disk package with no alias entry is reported as unmapped.""" _write_package(tmp_path, "alpha/mapped") From 866d921173c8641009bed227752ec1f515ae50b1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 14:34:54 -0400 Subject: [PATCH 2/2] Follow symlinked package directories during discovery Path.rglob does not follow directory symlinks by default, so the recursive discovery from the previous commit lost a case the old two-level glob covered: a symlinked package with no alias escaped the drift guard (review finding on #338). Discovery now walks with os.walk(followlinks=True) and reports each package under its link path. It skips only a directory whose real path is already in its own ancestry, so a cycle ends the walk while two links to one package are both reported, as under the old glob. Tests: a symlinked package and a symlinked family with a nested member are found and flagged when unaliased; a directory and a link to it are both reported; a cycle terminates. Each fails under its own in-memory mutant (plain rglob; a global visited set; no cycle check). On the real tree discovery still equals the old glob and the 333 alias targets. Co-Authored-By: Claude Opus 5.5 --- .../335-nested-package-discovery.fixed.md | 4 +- chronicle/source_package.py | 21 +++++++-- tests/test_source_package_alias_drift.py | 43 +++++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/changelog.d/335-nested-package-discovery.fixed.md b/changelog.d/335-nested-package-discovery.fixed.md index 601532e2..f39b139a 100644 --- a/changelog.d/335-nested-package-discovery.fixed.md +++ b/changelog.d/335-nested-package-discovery.fixed.md @@ -2,6 +2,8 @@ Make source-package discovery find nested package members, such as `stats_nz/census_2023_family_type/rc_01`, not just `/` directories. The alias-drift guard and the default-bundle coverage check rely on discovery, so a nested member that lost its alias is now reported instead of -being silently left out. Also correct the `verify-raw` diagnostics docstring: an +being silently left out. Symlinked directories are followed at every depth +(the old two-level glob followed them at its two levels), and a link back into +its own ancestry ends the walk. Also correct the `verify-raw` diagnostics docstring: an error line that merely mentions a missing-object marker still counts as missing. diff --git a/chronicle/source_package.py b/chronicle/source_package.py index 58c02b2b..3859040f 100644 --- a/chronicle/source_package.py +++ b/chronicle/source_package.py @@ -2266,14 +2266,27 @@ def discover_source_package_dirs(root: Path | None = None) -> list[Path]: members (e.g. ``stats_nz/census_2023_family_type/rc_01``) are found too, so the alias-drift guard sees every package a bundle could build. This is the authoring surface a complete bundle must cover. + + Symlinked directories are followed at every depth and reported under their + link path, as the earlier two-level glob did at its two levels; + ``Path.rglob`` does not follow them by default. Only a link back into its + own ancestry is not entered, so a cycle ends the walk instead of recursing + forever. """ packages_root = root if root is not None else source_package_root() if not packages_root.exists(): return [] - discovered = { - manifest.parent.relative_to(packages_root) - for manifest in packages_root.rglob(SOURCE_PACKAGE_FILENAME) - } + discovered: set[Path] = set() + ancestry: dict[str, frozenset[str]] = {} + for dirpath, dirnames, filenames in os.walk(packages_root, followlinks=True): + real = os.path.realpath(dirpath) + above = ancestry.get(os.path.dirname(dirpath), frozenset()) + if real in above: + dirnames[:] = [] + continue + ancestry[dirpath] = above | {real} + if SOURCE_PACKAGE_FILENAME in filenames: + discovered.add(Path(dirpath).relative_to(packages_root)) return sorted(discovered, key=str) diff --git a/tests/test_source_package_alias_drift.py b/tests/test_source_package_alias_drift.py index d4c4019c..613be10b 100644 --- a/tests/test_source_package_alias_drift.py +++ b/tests/test_source_package_alias_drift.py @@ -135,6 +135,49 @@ def test_find_alias_map_drift_flags_an_unmapped_nested_member(tmp_path): assert [str(path) for path in unmapped_dirs] == ["alpha/family/rc_01"] +def test_discover_source_package_dirs_follows_symlinked_packages(tmp_path): + """A symlinked package or family is found under its link path, nested members included.""" + root = tmp_path / "packages" + outside = tmp_path / "outside" + _write_package(outside, "linked_package") + _write_package(outside, "linked_family/rc_01") + (root / "alpha").mkdir(parents=True) + (root / "alpha" / "linked").symlink_to( + outside / "linked_package", target_is_directory=True + ) + (root / "beta").symlink_to(outside / "linked_family", target_is_directory=True) + + discovered = {str(path) for path in discover_source_package_dirs(root=root)} + assert discovered == {"alpha/linked", "beta/rc_01"} + + missing_dirs, unmapped_dirs = find_alias_map_drift(root=root, aliases={}) + assert missing_dirs == [] + assert [str(path) for path in unmapped_dirs] == ["alpha/linked", "beta/rc_01"] + + +def test_discover_source_package_dirs_reports_each_link_to_one_package(tmp_path): + """A directory and a link to it are separate discoveries, as under the old glob.""" + _write_package(tmp_path, "alpha/real") + (tmp_path / "beta").mkdir() + (tmp_path / "beta" / "alias").symlink_to( + tmp_path / "alpha" / "real", target_is_directory=True + ) + + discovered = {str(path) for path in discover_source_package_dirs(root=tmp_path)} + assert discovered == {"alpha/real", "beta/alias"} + + +def test_discover_source_package_dirs_stops_at_a_symlink_cycle(tmp_path): + """A link back into its own ancestry ends the walk instead of recursing forever.""" + _write_package(tmp_path, "alpha/family") + (tmp_path / "alpha" / "family" / "loop").symlink_to( + tmp_path / "alpha", target_is_directory=True + ) + + discovered = {str(path) for path in discover_source_package_dirs(root=tmp_path)} + assert discovered == {"alpha/family"} + + def test_find_alias_map_drift_flags_unmapped_directory(tmp_path): """An on-disk package with no alias entry is reported as unmapped.""" _write_package(tmp_path, "alpha/mapped")