Merge pull request #204 from PyAutoLabs/claude/hygiene-detail-flag-n1fgdq

feat(hygiene): --detail flag makes the config scan's findings routable

Author: Jammy2211

agents/conductors/hygiene/_hygiene_config.py

@@ -30,6 +30,13 @@
 Stdlib + PyYAML only — never imports the science stack. Emits one `count|summary`
 line; exits non-zero (no output) if PyYAML is absent so the conductor falls back
 gracefully. It is a *surface* signal — the count is "items to review", not bugs.
+
+Both signals come in two views over **one** traversal. `diff_detail` /
+`orphan_detail` return the items themselves; `diff` / `orphan_files` are the
+count view layered on top, so a tally can never disagree with its own listing.
+`--detail` prints the items grouped by the config file (or repo) they belong to
+— the routable form the `/refactor` hand-off needs. Without it the output is the
+single `count|summary` line the conductor's summary table parses, unchanged.
 """
 
 from __future__ import annotations
@@ -93,27 +100,52 @@ def load(path: str):
         return None
 
 
-def diff(root: str, pairs=PAIRS) -> tuple[int, list[str]]:
-    total = 0
-    detail: list[str] = []
+def _counts_by_repo(records) -> dict[str, int]:
+    """`{repo: item count}` over `(repo, …, items)` records, preserving
+    first-seen order. Both detail shapes fit: the repo is first, items last."""
+    counts: dict[str, int] = {}
+    for record in records:
+        repo, items = record[0], record[-1]
+        counts[repo] = counts.get(repo, 0) + len(items)
+    return counts
+
+
+def _summarise(records) -> tuple[int, list[str]]:
+    """The count view of a detail record list: `(total, ["repo:N", …])`."""
+    counts = _counts_by_repo(records)
+    return sum(counts.values()), [f"{repo}:{n}" for repo, n in counts.items()]
+
+
+def diff_detail(root: str, pairs=PAIRS) -> list[tuple[str, str, list[str]]]:
+    """The key-mirror drift itself: one `(workspace repo, config file name,
+    sorted missing key paths)` record per file missing at least one key.
+
+    `diff()` is the count view of this same walk — the skip rules (pair not
+    checked out, no workspace counterpart, unparseable YAML) live here only.
+    """
+    records: list[tuple[str, str, list[str]]] = []
     for lib_rel, ws_rel in pairs:
         lib_dir = os.path.join(root, lib_rel)
         ws_dir = os.path.join(root, ws_rel)
         if not (os.path.isdir(lib_dir) and os.path.isdir(ws_dir)):
             continue
-        missing = 0
-        for lib_yaml in glob.glob(os.path.join(lib_dir, "*.yaml")):
-            ws_yaml = os.path.join(ws_dir, os.path.basename(lib_yaml))
+        repo = ws_rel.split("/")[0]
+        for lib_yaml in sorted(glob.glob(os.path.join(lib_dir, "*.yaml"))):
+            name = os.path.basename(lib_yaml)
+            ws_yaml = os.path.join(ws_dir, name)
             if not os.path.isfile(ws_yaml):
                 continue  # workspace may intentionally not copy this file
             lib_data, ws_data = load(lib_yaml), load(ws_yaml)
             if lib_data is None or ws_data is None:
                 continue
-            missing += len(key_paths(lib_data) - key_paths(ws_data))
-        if missing:
-            total += missing
-            detail.append(f"{ws_rel.split('/')[0]}:{missing}")
-    return total, detail
+            missing = key_paths(lib_data) - key_paths(ws_data)
+            if missing:
+                records.append((repo, name, sorted(missing)))
+    return records
+
+
+def diff(root: str, pairs=PAIRS) -> tuple[int, list[str]]:
+    return _summarise(diff_detail(root, pairs))
 
 
 def _yaml_relpaths(config_dir: str) -> set[str]:
@@ -142,21 +174,22 @@ def _suppressed(relpath: str) -> bool:
     return relpath.split("/")[0] in ORPHAN_OWNERS
 
 
-def orphan_files(root: str, libraries=LIBRARIES, lib_relpaths=None,
-                 owners=ORPHAN_OWNERS) -> tuple[int, list[str]]:
-    """Workspace config files with no library counterpart, after owner-map
-    suppression.
+def orphan_detail(root: str, libraries=LIBRARIES, lib_relpaths=None,
+                  owners=ORPHAN_OWNERS) -> list[tuple[str, list[str]]]:
+    """The orphan files themselves: one `(repo, sorted orphan relpaths)` record
+    per repo holding at least one, after owner-map suppression.
 
     Only repos whose `config/` *mirrors* the library tree (shares ≥1 file with
     the library set) are scanned — that self-scopes to the workspace/tutorial/
     test/assistant repos and excludes organ repos (Brain/Heart/Mind) whose
     `config/` is their own thing, without a hardcoded repo list to go stale.
+
+    `orphan_files()` is the count view of this same walk.
     """
     if lib_relpaths is None:
         lib_relpaths = library_config_relpaths(root, libraries)
     lib_repos = {repo for repo, _ in libraries}
-    total = 0
-    detail: list[str] = []
+    records: list[tuple[str, list[str]]] = []
     for name in sorted(os.listdir(root)):
         if name in lib_repos:
             continue
@@ -169,27 +202,76 @@ def orphan_files(root: str, libraries=LIBRARIES, lib_relpaths=None,
         orphans = {r for r in (rels - lib_relpaths)
                    if not (r.split("/")[0] in owners)}
         if orphans:
-            total += len(orphans)
-            detail.append(f"{name}:{len(orphans)}")
-    return total, detail
+            records.append((name, sorted(orphans)))
+    return records
+
+
+def orphan_files(root: str, libraries=LIBRARIES, lib_relpaths=None,
+                 owners=ORPHAN_OWNERS) -> tuple[int, list[str]]:
+    """Workspace config files with no library counterpart, after owner-map
+    suppression — the count view of `orphan_detail()`."""
+    return _summarise(orphan_detail(root, libraries, lib_relpaths, owners))
+
+
+def _plural(n: int, noun: str) -> str:
+    return f"{n} {noun}" if n == 1 else f"{n} {noun}s"
+
+
+def render_detail(key_records, orphan_records) -> list[str]:
+    """The routable form of both signals: every drifted key path under the
+    workspace config file missing it, every orphan under its repo."""
+    lines: list[str] = []
+    if key_records:
+        lines.append("Library config keys absent downstream, by the workspace file "
+                     "missing them:")
+        for repo, name, keys in key_records:
+            lines.append(f"  {repo}/config/{name} — {_plural(len(keys), 'key')}")
+            lines.extend(f"    - {k}" for k in keys)
+    if orphan_records:
+        if lines:
+            lines.append("")
+        lines.append("Orphan config files (no library ships one at this relative "
+                     "path), by repo:")
+        for repo, orphans in orphan_records:
+            lines.append(f"  {repo}/config — {_plural(len(orphans), 'file')}")
+            lines.extend(f"    - {o}" for o in orphans)
+    return lines
 
 
 def main() -> int:
-    ap = argparse.ArgumentParser()
+    ap = argparse.ArgumentParser(
+        description="Config drift prescan for the hygiene conductor: library "
+                    "config keys absent downstream, and workspace config files "
+                    "with no library counterpart.")
     ap.add_argument("--root", default=os.path.expanduser("~/Code/PyAutoLabs"))
+    ap.add_argument("--detail", action="store_true",
+                    help="list every drifted key path and orphan file, grouped "
+                         "by the config file / repo it belongs to. Default is "
+                         "the single 'count|summary' line the conductor parses.")
     ns = ap.parse_args()
-    keys, key_detail = diff(ns.root)
-    orphans, orphan_detail = orphan_files(ns.root)
+    key_records = diff_detail(ns.root)
+    orphan_records = orphan_detail(ns.root)
+    keys, key_tally = _summarise(key_records)
+    orphans, orphan_tally = _summarise(orphan_records)
     total = keys + orphans
     parts = []
     if keys:
         parts.append(f"{keys} library config keys absent downstream "
-                     f"(review/mirror): {' '.join(key_detail)}")
+                     f"(review/mirror): {' '.join(key_tally)}")
     if orphans:
         parts.append(f"{orphans} orphan config files with no library counterpart "
-                     f"(review/remove): {' '.join(orphan_detail)}")
+                     f"(review/remove): {' '.join(orphan_tally)}")
     summary = "; ".join(parts) or "config in sync (no key drift or orphan files)"
-    print(f"{total}|{summary}")
+    if not ns.detail:
+        print(f"{total}|{summary}")
+        return 0
+    # --detail is the human/routing view: the summary sentence without the
+    # machine `count|` prefix, then the items themselves.
+    print(summary)
+    lines = render_detail(key_records, orphan_records)
+    if lines:
+        print()
+        print("\n".join(lines))
     return 0
 
 

agents/conductors/hygiene/hygiene.sh

@@ -37,7 +37,7 @@
 #   hygiene.sh refs            # dead internal references in workspace prose -> /refactor
 #   hygiene.sh optdeps         # smoke-listed scripts w/ a gated API but no skip guard -> /refactor
 #   hygiene.sh extras          # optional deps declared by a library but missing from the smoke CI install -> /bug
-#   hygiene.sh config          # library config keys missing downstream -> /refactor
+#   hygiene.sh config          # library config keys missing downstream + orphan config files -> /refactor
 #   hygiene.sh artifacts       # tracked leaked outputs/data -> /repo_cleanup
 #   hygiene.sh packaging       # ignored top-level *.egg-info/build dirs -> clean_slate.sh
 #   hygiene.sh <mode> --json   # machine-readable HygieneDecision
@@ -722,6 +722,15 @@ elif [[ "$mode" == "optdeps" ]]; then
 elif [[ "$mode" == "extras" ]]; then
   echo "Optional dependencies the workspace-validation smoke leg never installs (read-only scan):"
   python3 "$HERE/_hygiene_extras.py" --root "$ROOT"
+elif [[ "$mode" == "config" ]]; then
+  echo "Library config keys absent downstream + orphan config files (read-only scan):"
+  python3 "$HERE/_hygiene_config.py" --root "$ROOT" --detail \
+    || echo "config diff unavailable (PyYAML missing?)"
+  echo
+  echo "→ route the mirrors/removals to /refactor; Hygiene never edits source. This is a"
+  echo "  SURFACE signal — judge each item before acting: a workspace may omit a library"
+  echo "  key deliberately, and an orphan file may be read by something the library set"
+  echo "  does not encode (add its owner to ORPHAN_OWNERS rather than deleting it)."
 elif [[ "$mode" == "default" ]]; then
   # 'debris' and 'finding' pre-scans yield directly-actionable counts (perf's
   # timing is deferred here — too slow for the fast scan). Rank across them and

tests/test_hygiene_conductor.py

@@ -431,6 +431,115 @@ def test_orphan_files_skips_non_mirror_repos(tmp_path):
     assert total == 0 and detail == []
 
 
+# --- config --detail: the routable view of both signals. ----------------------
+# The count alone cannot be routed to /refactor — these lock the fact that the
+# key paths and orphan paths are printed, AND that asking for them never moves
+# the default `count|summary` line the conductor's summary table parses.
+
+CONFIG_HELPER = (
+    BRAIN_HOME / "agents" / "conductors" / "hygiene" / "_hygiene_config.py"
+)
+
+
+def _run_config_helper(root, *args):
+    _load_config_helper()  # skips (SystemExit) if PyYAML absent
+    return subprocess.run(
+        [sys.executable, str(CONFIG_HELPER), "--root", str(root), *args],
+        capture_output=True, text=True,
+    )
+
+
+def _drifted_pair(root):
+    """A real PAIRS pair (PyAutoFit <-> autofit_workspace) with nested and
+    top-level key drift across two config files."""
+    _fake_library(root, {
+        "general.yaml": {"output": {"search_internal": 1}, "keep": 2},
+        "logging.yaml": {"total_files_open": 1},
+    })
+    _fake_workspace(root, "autofit_workspace", {
+        "general.yaml": {"output": {}, "keep": 2},   # missing output.search_internal
+        "logging.yaml": {},                          # missing total_files_open
+    })
+
+
+def test_config_detail_groups_drifted_keys_under_the_file_missing_them(tmp_path):
+    _drifted_pair(tmp_path)
+    r = _run_config_helper(tmp_path, "--detail")
+    assert r.returncode == 0, r.stderr
+    # The key paths themselves — the thing the count could not hand over.
+    assert "- output.search_internal" in r.stdout
+    assert "- total_files_open" in r.stdout
+    # ...each under the workspace file it is absent from, not a flat list.
+    general = r.stdout.index("autofit_workspace/config/general.yaml")
+    logging_ = r.stdout.index("autofit_workspace/config/logging.yaml")
+    assert general < r.stdout.index("- output.search_internal") < logging_
+    assert logging_ < r.stdout.index("- total_files_open")
+
+
+def test_config_detail_groups_orphan_files_under_their_repo(tmp_path):
+    """The orphan signal gets the same treatment, owner suppression intact."""
+    _fake_library(tmp_path, {
+        "general.yaml": {"a": 1},
+        "non_linear/GridSearch.yaml": {"grid": 1},
+    })
+    _fake_workspace(tmp_path, "some_workspace", {
+        "general.yaml": {"a": 1},                    # shared -> this IS a mirror
+        "grids.yaml": {"radial_minimum": 1},         # orphan -> named
+        "non_linear/nest.yaml": {"Nautilus": 1},     # orphan -> named
+        "non_linear/GridSearch.yaml": {"grid": 1},   # mirrored -> absent
+        "build/env_vars.yaml": {"X": 1},             # owned -> suppressed
+    })
+    r = _run_config_helper(tmp_path, "--detail")
+    assert r.returncode == 0, r.stderr
+    repo = r.stdout.index("some_workspace/config")
+    assert repo < r.stdout.index("- grids.yaml")
+    assert repo < r.stdout.index("- non_linear/nest.yaml")
+    assert "GridSearch.yaml" not in r.stdout   # has a library counterpart
+    assert "env_vars.yaml" not in r.stdout     # ORPHAN_OWNERS suppression
+
+
+def test_config_default_output_is_still_one_count_summary_line(tmp_path):
+    """The regression guard: `prescan_config` parses `${out%%|*}`, so adding
+    --detail must not add a line, a prefix, or a newline to the default."""
+    _drifted_pair(tmp_path)
+    r = _run_config_helper(tmp_path)
+    assert r.returncode == 0, r.stderr
+    assert r.stdout.splitlines() == [
+        "2|2 library config keys absent downstream (review/mirror): "
+        "autofit_workspace:2"
+    ]
+
+
+def test_config_detail_on_a_clean_tree_reports_in_sync_and_lists_nothing(tmp_path):
+    r = _run_config_helper(tmp_path, "--detail")
+    assert r.returncode == 0, r.stderr
+    assert r.stdout.strip() == "config in sync (no key drift or orphan files)"
+
+
+def test_hygiene_config_mode_hands_over_the_drifted_key_paths(tmp_path):
+    """The whole point: `hygiene config` must surface routable findings, not a
+    tally the operator has to re-derive by importing the module."""
+    _load_config_helper()  # skips (SystemExit) if PyYAML absent
+    _drifted_pair(tmp_path)
+    r = _run(["config"], tmp_path)
+    assert r.returncode == 0, r.stderr
+    assert "output.search_internal" in r.stdout
+    assert "total_files_open" in r.stdout
+    assert "/refactor" in r.stdout
+
+
+def test_hygiene_config_json_row_is_unchanged_by_detail(tmp_path):
+    """The machine surface keeps reading the count line, not the detail."""
+    _load_config_helper()  # skips (SystemExit) if PyYAML absent
+    _drifted_pair(tmp_path)
+    r = _run(["config", "--json"], tmp_path)
+    assert r.returncode == 0, r.stderr
+    row = json.loads(r.stdout)["row"]
+    assert row["mode"] == "config" and row["kind"] == "surface"
+    assert row["count"] == 2
+    assert "output.search_internal" not in row["summary"]
+
+
 def test_help_lists_the_usage_block(tmp_path):
     r = _run(["--help"], tmp_path)
     assert r.returncode == 0