From c05c9e4277d8526bb0c6bec5fb31db8baacf8019 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 21:47:26 +0000 Subject: [PATCH] feat(hygiene): --detail flag makes the config scan's findings routable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `config` prescan emitted only a `count|summary` line and its `main()` took only `--root`, so the mode named *how many* library config keys were absent downstream but never *which*. The hygiene skill routes config findings onward for repair; recovering the key paths meant importing the module and re-running `diff()` by hand. Split each of the two signals into a detail core plus the existing count wrapper — `diff_detail` / `orphan_detail` return the items, `diff` / `orphan_files` are the count view over the same single traversal, so a tally can never disagree with its own listing. Their `(total, ["repo:N", ...])` returns are unchanged; the tests assert them directly. `--detail` then prints every drifted key path under the workspace config file missing it, and every orphan config file under its repo. Without the flag the output is the same single `count|summary` line, so `prescan_config` (which parses `${out%%|*}`) and the conductor's summary table and `--json` row are untouched. `hygiene config` single-mode now renders that detail block, mirroring the `refs` / `optdeps` / `extras` branches, so the findings are reachable from the documented command surface rather than only by calling the helper. Verified against the live checkouts: the 19 drifted keys resolve to autofit_workspace general.yaml (output.search_internal, test.check_likelihood_function) + logging.yaml (total_files_open); autogalaxy_workspace general.yaml (test.exception_override) + 14 notation.yaml labels; autolens_workspace general.yaml (output.fit_dill). Closes #203 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XvQ4YHkNynTmS4ovHAgbWB --- agents/conductors/hygiene/_hygiene_config.py | 134 +++++++++++++++---- agents/conductors/hygiene/hygiene.sh | 11 +- tests/test_hygiene_conductor.py | 109 +++++++++++++++ 3 files changed, 227 insertions(+), 27 deletions(-) diff --git a/agents/conductors/hygiene/_hygiene_config.py b/agents/conductors/hygiene/_hygiene_config.py index df2d9d9..0ca2a51 100755 --- a/agents/conductors/hygiene/_hygiene_config.py +++ b/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 diff --git a/agents/conductors/hygiene/hygiene.sh b/agents/conductors/hygiene/hygiene.sh index 490fbb9..37513a5 100755 --- a/agents/conductors/hygiene/hygiene.sh +++ b/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 --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 diff --git a/tests/test_hygiene_conductor.py b/tests/test_hygiene_conductor.py index 45c05df..1ec437e 100644 --- a/tests/test_hygiene_conductor.py +++ b/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