From cb20ecf950ee9ea8f17309092f0aa9bfee0c162c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 11:20:45 +0000 Subject: [PATCH] fix(hygiene): derive the conductor's repo sets from the body map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hygiene conductor hardcoded the repos it scans: LIB_REPOS=(PyAutoNerves PyAutoFit PyAutoArray PyAutoGalaxy PyAutoLens) ORG_REPOS=(PyAutoBrain PyAutoHands PyAutoHeart PyAutoMind) DOC_REPOS=(PyAutoFit PyAutoGalaxy PyAutoLens) The body map declares six libraries and seven organs, so two libraries were skipped entirely, the config layer was classed as a library where the map calls it an organ, and three organs went uncovered. The drift was invisible by construction: a repo that is never scanned produces no findings, so the conductor reported clean and was believed. Measured, not inferred — `crlf` printed 5 .py with CRLF against a true 127, 122 of them in the one skipped library that has an LF-only rule nobody was enforcing. The sets now come from the map via _hygiene_repos.py, so adding a repo there adds it to the scan. Two modes take a narrower set from what a checkout CONTAINS rather than its category, because category alone gets it wrong: the config layer is an organ yet ships a real distribution, so keying `deps` off `category: library` would have dropped it — this same bug, one line down. `docs` was pinned to three named repos and never noticed a fourth acquiring Sphinx docs; `pyproject.toml` and `docs/api/` presence are the honest tests. Measured effect (17 repos scanned, was 9): crlf 5 -> 167 cosmetic, deps 5 -> 8 pyproject.toml, docs 3 -> 4 repos. COVERAGE ONLY — the backlog this exposes is a separate triage task and no finding is fixed here. Also fixes a third defect in the same surface: an empty scan root, or an unreachable body map, made every repo-array mode report `clean`. A zero from "nothing was examined" and a zero from "nothing was wrong" are indistinguishable to a reader and only one is good news, so those modes now report `unscanned` with the reason, and the default scan leads with a banner. Scoped to the modes that read the arrays: the helper-backed modes discover their own targets by walking the root, so suppressing them would hide real findings. The conductor now names no repository at all, so it carries no tenant-firewall allowlist entry — one less file an adopting fork must rewrite. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SzmzZidPBRqQZjEEw1d6ET --- agents/conductors/hygiene/AGENTS.md | 44 ++++- agents/conductors/hygiene/_hygiene_repos.py | 193 ++++++++++++++++++++ agents/conductors/hygiene/hygiene.sh | 183 +++++++++++++++---- tests/test_hygiene_conductor.py | 153 ++++++++++++++++ 4 files changed, 529 insertions(+), 44 deletions(-) create mode 100755 agents/conductors/hygiene/_hygiene_repos.py diff --git a/agents/conductors/hygiene/AGENTS.md b/agents/conductors/hygiene/AGENTS.md index 2abdbb6..815be31 100644 --- a/agents/conductors/hygiene/AGENTS.md +++ b/agents/conductors/hygiene/AGENTS.md @@ -32,22 +32,58 @@ kinds, which is what makes its count comparable (or not): delegated skill runs, so the count is **not** a problem count (`deps`, `docs`). - **advisory** — no cheap local signal at all (`noise`). +A mode also carries a **status**, and one of them is not a count at all: +**`unscanned`** means the mode read *no repository* — the scan root holds no +managed checkout, or the body map could not be reached. It is reported instead +of `clean` because a zero from "nothing was examined" and a zero from "nothing +was wrong" are indistinguishable to a reader, and the first is not good news. + +## Which repositories it scans + +**Derived from the body map (`repos.yaml`), never listed here or in the script.** +The conductor takes `library`, `organ` and `workspace` from the map via +`_hygiene_repos.py`; adding a repo to the map adds it to the scan. + +This was a bash array once, and it drifted: five libraries where the map +declared six, four organs of seven, and a CRLF count of **5** against a true +**127**. The drift was invisible precisely because an unscanned repo yields no +findings — the conductor reported clean and was believed. `repos_sync.py`'s +`check_hygiene_coverage` now fails if the derived sets stop matching the map, or +if a repo name is written back into a `*_REPOS=(…)` array. + +Two modes need a narrower set, and both read what a checkout **contains** rather +than its category — because category alone gets it wrong. The config layer is an +*organ* in the map yet ships a real distribution, so keying `deps` off +`category: library` would silently drop it: + +| Set | Rule | Modes | +|-----|------|-------| +| code repos | `library` + `organ` | `tidy`, `deps`, `docs`, `packaging` | +| scanned repos | code repos + `workspace` | `crlf`, `artifacts` | +| ships a distribution | has a `pyproject.toml` | `deps` | +| ships api docs | has a `docs/api/` tree | `docs` | + +The helper-backed modes (`docstrings`, `refs`, `optdeps`, `extras`, `config`) +are **not** on this list: they discover their own targets by walking the scan +root for workspace-shaped directories, so they can find material the map never +names — and they keep reporting even when the repo-array modes are `unscanned`. + | Mode | Pre-scan (kind) | Delegates to | |------|-----------------|--------------| | `perf` | dev-loop timing — prefers Heart's tracked timing legs when present (`import_time`, `unit_test_timing`, `workspace_testmode_timing`), else times `import ` per library in a **subprocess** (**timing**) | `/refactor` / `/bug` (+ Heart timing legs) | | `tidy` | git debris — stale branches, stashes, `[gone]` refs, dirty checkouts (**debris**) | **condemn** → files candidates into `condemned.md` async (PyAutoGut archives the fragile forms); no synchronous per-item gate | | `sweep` | reads `condemned.md`, classifies entries by their transit clock (**due** / pending / undated) | `pyauto-gut void` for past-due entries, behind the existing `repo_cleanup` safety gates | | `noise` | none — needs a pytest + workspace-script run (**advisory**) | `/cli_noise_clean` (Heart) | -| `deps` | capped/pinned specifiers in library `pyproject.toml` (**surface**) | `/dep_audit` (Heart, hits PyPI) | -| `docs` | `docs/api/*.rst` + `currentmodule` counts across the 3 doc repos (**surface**) | `/audit_docs` (Heart, imports) | -| `crlf` | executable scripts (`.sh` + shebang-`755` `.py`) with CRLF — the shebang breaks on Linux/HPC (**debris**, the ranked count); library `.py` CRLF is reported separately as *cosmetic* (Python reads it fine — don't mass-normalise) | `/refactor` + `.gitattributes eol=lf` | +| `deps` | capped/pinned specifiers in every managed repo that ships a `pyproject.toml` (**surface**) | `/dep_audit` (Heart, hits PyPI) | +| `docs` | `docs/api/*.rst` + `currentmodule` counts across every managed repo shipping a `docs/api/` tree (**surface**) | `/audit_docs` (Heart, imports) | +| `crlf` | executable scripts (`.sh` + shebang-`755` `.py`) with CRLF — the shebang breaks on Linux/HPC (**debris**, the ranked count); plain `.py` CRLF is reported separately as *cosmetic* (Python reads it fine — don't mass-normalise) | `/refactor` + `.gitattributes eol=lf` | | `docstrings` | consecutive module-level triple-quoted expressions separated only by whitespace in user-facing `*_workspace` and `HowTo*` root `*.py` entry scripts and `scripts/**/*.py` files (**finding**) | `/refactor` (mechanically merge each confirmed boundary) | | `refs` | file/folder references in user-facing `*_workspace` and `HowTo*` prose (`scripts/**/*.py` docstrings + comments, every `scripts/**/README.md` and `config/**/README.md`, and the top-level README) whose target no longer exists — restructure debt no health sweep can see, since the scripts still run (**finding**). Covers the README idioms a `scripts/`-anchored matcher cannot see: structure-list bullets (``- `slam_pipeline`: ``), slash-less relative folder paths (`data_preparation/imaging`), and config YAML names | `/refactor` (re-point each reference; judge the intended target) | | `optdeps` | smoke-listed workspace scripts that construct an optional-dependency-gated API (`TransformerNUFFT` → `nufftax`) without the house `find_spec` skip guard, so they hard-fail the CI matrices that omit the extras (**finding**). AST-confirmed — prose mentions don't count; scripts outside `smoke_tests.txt` are never flagged | `/refactor` (add the skip guard) | | `extras` | the complement of `optdeps`: an optional dependency a library **declares** (in the `[optional]` extra `mode=release` installs) that the `workspace-validation.yml` **`mode=smoke`** leg never installs (**finding**). The extras chain only reaches each library's own `[jax]`, never a sibling's `[optional]`, so those need hand-adding and silently drift — the symptom is a script red in smoke and **green in release** | `/bug` (add the install; fix the install set, **never** the script) | | `config` | library `config/*.yaml` keys missing from the matching workspace config — recursive diff (**surface**) | `/refactor` (mirror keys) | | `artifacts` | tracked files that look like leaked run outputs / stray data (under `output/`, or data-ext outside fixtures) (**debris**) | `/repo_cleanup` (gitignore + `git rm --cached`) | -| `packaging` | ignored, fully-untracked top-level `*.egg-info/` and `build/` directories in managed library repos (**debris**) | preview then run `PyAutoBrain/bin/clean_slate.sh --packaging`; repo-set, exact-name, root-depth and tracked-file guards apply | +| `packaging` | ignored, fully-untracked top-level `*.egg-info/` and `build/` directories in the managed code repos (**debris**) | preview then run `PyAutoBrain/bin/clean_slate.sh --packaging`; repo-set, exact-name, root-depth and tracked-file guards apply | | *(default)* | all of the above (**perf timing deferred** — it spawns real imports) | a ranked `HygieneDecision` worklist — recommends the highest-count direct mode (`tidy`/`crlf`/`docstrings`/`refs`/`artifacts`/`packaging`), then `hygiene perf`, then the periodic surface audits | ``` diff --git a/agents/conductors/hygiene/_hygiene_repos.py b/agents/conductors/hygiene/_hygiene_repos.py new file mode 100755 index 0000000..c725664 --- /dev/null +++ b/agents/conductors/hygiene/_hygiene_repos.py @@ -0,0 +1,193 @@ +#!/usr/bin/env python3 +"""Read the organism's body map for the hygiene conductor. + +The conductor scans repositories. WHICH repositories is not its decision to +make: the body map (the Mind's ``repos.yaml``) is the single source of repo +identity, and this helper is the conductor's only route to it. + +Why a helper rather than an array in ``hygiene.sh``: a hardcoded repo list +drifts as the organism grows, and the drift is *invisible* — a repo that is +never scanned produces no findings, so the conductor reports a clean bill of +health it has not earned. (It did: five libraries scanned where the map +declared six, four organs of seven, and a CRLF count of 5 against a true 127.) +Deriving the sets means adding a repo to the map adds it to the scan. + +This file deliberately contains **no repository names**. That is what keeps it +firewall-clean under ``repos_sync.py``'s tenant check, and it is also the +property the coverage check relies on: there is nothing here to drift. + +Usage +----- + _hygiene_repos.py --category # one name per line, sorted + _hygiene_repos.py --json # {"": [names...], ...} + +Exit codes: 0 = read; 3 = body map unresolvable (prints nothing, so a caller +can distinguish "no repos declared" from "no repos present" and report +`unscanned` rather than `clean` for either). + +The map is located the way ``agents/_common.sh`` locates any organ checkout: +an explicit ``PYAUTO_MIND``, then the sibling beside this Brain checkout, then +``$PYAUTO_ROOT``, then a couple of common dev layouts. PyYAML is used when it +imports and a minimal parser stands in when it does not — the conductor stays +dependency-free by design (it must never drag a heavy stack into the Brain), +and the map's own shape is simple enough to read without one. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import sys +from pathlib import Path + +MAP_FILENAME = "repos.yaml" + +# The Mind is an organ, so its directory name is framework identity rather than +# an instance fact — the same reason _common.sh may name it. +MIND_REPO = "PyAuto" + "Mind" + + +def candidate_map_paths() -> list[Path]: + """Where the body map might live, most-authoritative first. + + An explicit ``PYAUTO_MIND`` pointing at a real directory is authoritative and + ends the search, exactly as ``_resolve_dir`` in ``agents/_common.sh`` treats + its override. Falling through to a sibling checkout would silently scan a + *different* organism than the operator named — and would make "the map is + unreachable" unreachable itself, so the branch that reports it could never + be exercised. + """ + here = Path(__file__).resolve() + # ...//agents/conductors/hygiene/_hygiene_repos.py + brain_parent = here.parents[4] + override = os.environ.get("PYAUTO_MIND") + if override and Path(override).is_dir(): + return [Path(override)] + candidates: list[Path] = [] + candidates.append(brain_parent / MIND_REPO) + root = os.environ.get("PYAUTO_ROOT") + if root: + candidates.append(Path(root) / MIND_REPO) + home = Path.home() + candidates += [home / MIND_REPO, home / "Code" / MIND_REPO] + return candidates + + +def resolve_map() -> Path | None: + for base in candidate_map_paths(): + path = base / MAP_FILENAME + if path.is_file(): + return path + return None + + +# --- Parsing ----------------------------------------------------------------- +# +# Two readers for one file. PyYAML is correct and preferred; the fallback exists +# so a missing optional dependency degrades the *rigour* of the parse, never the +# *coverage* of the scan. Silently scanning fewer repos is the bug this whole +# module exists to prevent, so "PyYAML absent" must not become a way to +# re-introduce it. + +_REPO_LINE = re.compile(r"^ ([A-Za-z0-9._-]+):\s*(#.*)?$") +_CATEGORY_LINE = re.compile(r"^ category:\s*['\"]?([A-Za-z0-9_-]+)['\"]?\s*(#.*)?$") +_TOP_LEVEL = re.compile(r"^\S") + + +def parse_minimal(text: str) -> dict[str, str]: + """Map repo name -> category without PyYAML. + + Walks the two-level ``repos:`` block by indentation: a two-space key opens a + repo, a four-space ``category:`` sets it, and any new top-level key ends the + block. Sufficient for this file's fixed shape and nothing more — it is a + fallback, not a YAML implementation. + """ + out: dict[str, str] = {} + in_repos = False + current: str | None = None + for line in text.splitlines(): + if not line.strip() or line.lstrip().startswith("#"): + continue + if _TOP_LEVEL.match(line): + in_repos = line.startswith("repos:") + current = None + continue + if not in_repos: + continue + m = _REPO_LINE.match(line) + if m: + current = m.group(1) + continue + m = _CATEGORY_LINE.match(line) + if m and current: + out[current] = m.group(1) + return out + + +def parse_with_yaml(text: str) -> dict[str, str]: + import yaml # local import: absent PyYAML must fall back, not crash + + data = yaml.safe_load(text) or {} + return { + name: entry.get("category") + for name, entry in (data.get("repos") or {}).items() + if isinstance(entry, dict) and entry.get("category") + } + + +def load_categories(path: Path, parser: str = "auto") -> dict[str, list[str]]: + """Return category -> sorted repo names. + + ``parser="minimal"`` forces the PyYAML-free path. That exists so the drift + check can exercise the fallback on a machine that *has* PyYAML: a fallback + only ever used where nothing verifies it is a fallback nobody can trust, and + a parser that silently drops repos is this module's own bug class. + """ + text = path.read_text() + if parser == "minimal": + by_repo = parse_minimal(text) + else: + try: + by_repo = parse_with_yaml(text) + except ImportError: + by_repo = parse_minimal(text) + grouped: dict[str, list[str]] = {} + for name, category in by_repo.items(): + grouped.setdefault(category, []).append(name) + return {category: sorted(names) for category, names in sorted(grouped.items())} + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--category", help="print the repos in one category") + parser.add_argument("--json", action="store_true", + help="print every category as JSON") + parser.add_argument("--parser", choices=("auto", "minimal"), default="auto", + help="force a reader; 'minimal' is the PyYAML-free path") + args = parser.parse_args() + + path = resolve_map() + if path is None: + searched = ", ".join(str(base / MAP_FILENAME) for base in candidate_map_paths()) + print( + f"hygiene: body map not found — no {MAP_FILENAME} at: {searched}. " + f"Set PYAUTO_MIND to the Mind checkout.", + file=sys.stderr, + ) + return 3 + + grouped = load_categories(path, args.parser) + if args.json: + print(json.dumps(grouped, indent=2, sort_keys=True)) + elif args.category: + for name in grouped.get(args.category, []): + print(name) + else: + parser.error("one of --category or --json is required") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/agents/conductors/hygiene/hygiene.sh b/agents/conductors/hygiene/hygiene.sh index 8d077d7..490fbb9 100755 --- a/agents/conductors/hygiene/hygiene.sh +++ b/agents/conductors/hygiene/hygiene.sh @@ -50,20 +50,80 @@ # # All modes are live. The fast default scan DEFERS perf's import timing (it # spawns real imports); run `hygiene perf` for it. Repos are read under -# PYAUTO_ROOT (default ~/Code/PyAutoLabs); import timing uses HYGIENE_PYTHON -# (default python3 — point it at the PyAuto venv to time the science libs). +# PYAUTO_ROOT (defaulted by _common.sh) and WHICH repos comes from the body map, +# never from a list here; import timing uses HYGIENE_PYTHON (default python3 — +# point it at the PyAuto venv to time the science libs). A scan that sees no +# checkouts reports `unscanned`, never `clean`. set -uo pipefail HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)" source "$HERE/../../_common.sh" -# PYAUTO_ROOT is exported/defaulted by _common.sh (~/Code/PyAutoLabs). Scan the -# canonical checkouts there, never the worktree symlinks. -ROOT="${PYAUTO_ROOT:-$HOME/Code/PyAutoLabs}" -LIB_REPOS=(PyAutoNerves PyAutoFit PyAutoArray PyAutoGalaxy PyAutoLens) -ORG_REPOS=(PyAutoBrain PyAutoHands PyAutoHeart PyAutoMind) -DOC_REPOS=(PyAutoFit PyAutoGalaxy PyAutoLens) +# PYAUTO_ROOT is exported/defaulted by _common.sh. Scan the canonical checkouts +# there, never the worktree symlinks. +ROOT="$PYAUTO_ROOT" + +# The scanned repo sets are DERIVED from the organism's body map (the Mind's +# repos.yaml — the single source of repo identity), never written out here. A +# hardcoded list drifts as the organism grows and the drift is INVISIBLE: a repo +# that is never scanned produces no findings, so the conductor reports a clean +# bill of health it has not earned. repos_sync.py's hygiene-coverage check fails +# if these stop matching the map, or if a repo name is written back into an +# array literal. +# +# LIB_REPOS the science libraries ORG_REPOS the organism's own repos +# WS_REPOS the user-facing workspaces +# +# BODY_MAP_OK separates "no repos DECLARED" (map unreachable) from "no repos +# PRESENT" (scan root empty) — both must report `unscanned`, never `clean`. +_body_map() { python3 "$HERE/_hygiene_repos.py" --category "$1" 2>/dev/null; } +mapfile -t LIB_REPOS < <(_body_map library) +mapfile -t ORG_REPOS < <(_body_map organ) +mapfile -t WS_REPOS < <(_body_map workspace) +BODY_MAP_OK=1 +[[ ${#LIB_REPOS[@]} -eq 0 && ${#ORG_REPOS[@]} -eq 0 ]] && BODY_MAP_OK=0 + +# CODE_REPOS — every repo the organism maintains as code (libraries + organs). +# SCAN_REPOS — those plus the user-facing workspaces, for the modes that read +# example scripts and prose as well as source. +CODE_REPOS=("${LIB_REPOS[@]}" "${ORG_REPOS[@]}") +SCAN_REPOS=("${CODE_REPOS[@]}" "${WS_REPOS[@]}") + +# `deps` and `docs` each want a narrower set than "code repo", and both take it +# from what the checkout actually CONTAINS rather than from a category — because +# category alone gets it wrong. The config layer is an ORGAN in the body map yet +# ships a real distribution, so keying `deps` off `category: library` would +# silently drop it: the very bug this change repairs, re-created one line down. +# `docs` was pinned to three named repos and so never noticed a fourth acquiring +# Sphinx docs. Presence is the honest test in both cases. +repo_is_checked_out() { [[ -d "$ROOT/$1/.git" || -f "$ROOT/$1/.git" ]]; } +repo_ships_distribution() { [[ -f "$ROOT/$1/pyproject.toml" ]]; } +repo_ships_api_docs() { [[ -d "$ROOT/$1/docs/api" ]]; } + +# MANAGED_PRESENT — how many declared repos are actually checked out under +# $ROOT. Zero means the repo-array modes saw NOTHING, and their counts would be +# 0 for that reason alone. Reporting that as `clean` is the same failure as +# reporting half the organism as clean, so they report `unscanned` + the reason. +# +# ARRAY_MODES is exactly the set this applies to: the modes that iterate the +# derived arrays. The helper-backed modes (docstrings/refs/optdeps/extras/config) +# DISCOVER their targets by walking $ROOT for workspace-shaped directories, so +# they can legitimately find material the body map never names — suppressing +# them here would hide real findings. +ARRAY_MODES=" tidy crlf artifacts deps docs packaging " +mode_reads_repo_arrays() { [[ "$ARRAY_MODES" == *" $1 "* ]]; } + +MANAGED_PRESENT=0 +for _repo in "${SCAN_REPOS[@]}"; do + repo_is_checked_out "$_repo" && MANAGED_PRESENT=$((MANAGED_PRESENT + 1)) +done +UNSCANNED_REASON="" +if [[ "$BODY_MAP_OK" -eq 0 ]]; then + UNSCANNED_REASON="body map unreachable — the Mind checkout was not found (set PYAUTO_MIND)" +elif [[ "$MANAGED_PRESENT" -eq 0 ]]; then + UNSCANNED_REASON="no managed checkouts under the scan root $ROOT (set PYAUTO_ROOT)" +fi # PyAutoGut drive seam (tidy/sweep). The conductor DECIDES and emits a plan; the # organ entrypoint performs the archive/void. GUT_CMD is referenced in the @@ -118,9 +178,9 @@ declare -A MODE_KIND=( # tracking refs, dirty trees. The prioritisable count is the total debris. prescan_tidy() { local branches=0 stashes=0 gone=0 dirty=0 scanned=0 repo dir - for repo in "${LIB_REPOS[@]}" "${ORG_REPOS[@]}"; do + for repo in "${CODE_REPOS[@]}"; do dir="$ROOT/$repo" - [[ -d "$dir/.git" || -f "$dir/.git" ]] || continue + repo_is_checked_out "$repo" || continue scanned=$((scanned + 1)) local b s g b=$(git -C "$dir" for-each-ref --format='%(refname:short)' refs/heads 2>/dev/null \ @@ -131,37 +191,43 @@ prescan_tidy() { [[ -n "$(git -C "$dir" status --porcelain 2>/dev/null)" ]] && dirty=$((dirty + 1)) done local total=$((branches + stashes + gone + dirty)) - echo "${total}|${scanned} repos: ${branches} stale branches, ${stashes} stashes, ${gone} [gone] refs, ${dirty} dirty checkouts" + echo "${total}|${scanned}/${#CODE_REPOS[@]} code repos: ${branches} stale branches, ${stashes} stashes, ${gone} [gone] refs, ${dirty} dirty checkouts" } -# deps: count capped dependency specifiers (<, <=, ==) in library pyproject.toml. -# A cheap "how many caps could be stale" signal; /dep_audit does the PyPI compare. +# deps: count capped dependency specifiers (<, <=, ==) in every managed repo +# that ships a distribution. A cheap "how many caps could be stale" signal; +# /dep_audit does the PyPI compare. prescan_deps() { - local caps=0 files=0 repo pj - for repo in "${LIB_REPOS[@]}"; do + local caps=0 files=0 scanned=0 repo pj + for repo in "${CODE_REPOS[@]}"; do + repo_is_checked_out "$repo" || continue + scanned=$((scanned + 1)) + repo_ships_distribution "$repo" || continue pj="$ROOT/$repo/pyproject.toml" - [[ -f "$pj" ]] || continue files=$((files + 1)) local c c=$(grep -oE '[<>=!~]=?[[:space:]]*[0-9]' "$pj" 2>/dev/null | grep -cE '<|==' || true) caps=$((caps + c)) done - echo "${caps}|${caps} capped/pinned specifiers across ${files} library pyproject.toml" + echo "${caps}|${caps} capped/pinned specifiers across ${files} pyproject.toml (${scanned}/${#CODE_REPOS[@]} code repos present)" } -# docs: count docs/api/*.rst files and currentmodule directives in the doc repos. -# /audit_docs does the actual import validation. +# docs: count docs/api/*.rst files and currentmodule directives in every managed +# repo that ships an api docs tree. /audit_docs does the actual import validation. prescan_docs() { - local rst=0 cm=0 repo d - for repo in "${DOC_REPOS[@]}"; do + local rst=0 cm=0 doc_repos=0 scanned=0 repo d + for repo in "${CODE_REPOS[@]}"; do + repo_is_checked_out "$repo" || continue + scanned=$((scanned + 1)) + repo_ships_api_docs "$repo" || continue + doc_repos=$((doc_repos + 1)) d="$ROOT/$repo/docs/api" - [[ -d "$d" ]] || continue local n c n=$(find "$d" -maxdepth 1 -name '*.rst' 2>/dev/null | wc -l | tr -d ' ') c=$(grep -rhE '^\s*\.\.\s+currentmodule::' "$d" 2>/dev/null | wc -l | tr -d ' ') rst=$((rst + n)); cm=$((cm + c)) done - echo "${cm}|${rst} api .rst files, ${cm} currentmodule directives across ${#DOC_REPOS[@]} repos" + echo "${cm}|${rst} api .rst files, ${cm} currentmodule directives across ${doc_repos} repos with docs/api (${scanned}/${#CODE_REPOS[@]} code repos present)" } # crlf: CRLF line endings, split by severity. The count that MATTERS is @@ -171,10 +237,11 @@ prescan_docs() { # ranked, since mass-normalising it is a big diff for zero functional gain # (the real fix there is `.gitattributes * text=auto`, going forward). prescan_crlf() { - local scripts=0 cosmetic=0 sdetail="" repo dir sh_n exe_list exe_n py_n - for repo in "${LIB_REPOS[@]}" "${ORG_REPOS[@]}" autolens_workspace autogalaxy_workspace autofit_workspace; do + local scripts=0 cosmetic=0 scanned=0 sdetail="" repo dir sh_n exe_list exe_n py_n + for repo in "${SCAN_REPOS[@]}"; do dir="$ROOT/$repo" - [[ -d "$dir/.git" || -f "$dir/.git" ]] || continue + repo_is_checked_out "$repo" || continue + scanned=$((scanned + 1)) # .sh with CRLF (all shell scripts break) sh_n=$(git -C "$dir" grep -Il $'\r$' -- '*.sh' 2>/dev/null | wc -l | tr -d ' ') # executable .py (mode 755 — run directly, so a CRLF shebang breaks) @@ -188,7 +255,7 @@ prescan_crlf() { py_n=$(git -C "$dir" grep -Il $'\r$' -- '*.py' 2>/dev/null | wc -l | tr -d ' ') cosmetic=$((cosmetic + py_n)) done - echo "${scripts}|${scripts} executable scripts w/ CRLF (BREAK on HPC — normalise + add .gitattributes eol=lf): ${sdetail}; ${cosmetic} library .py w/ CRLF (cosmetic — leave, or '* text=auto' going forward)" + echo "${scripts}|${scripts} executable scripts w/ CRLF (BREAK on HPC — normalise + add .gitattributes eol=lf): ${sdetail}; ${cosmetic} .py w/ CRLF (cosmetic — leave, or '* text=auto' going forward) across ${scanned}/${#SCAN_REPOS[@]} scanned repos" } # docstrings: confirmed adjacent module-level triple-quoted documentation @@ -233,10 +300,11 @@ prescan_refs() { # under a run-output dir (outputs?/, but NOT the output_test fixture dir) plus # stray data-ext files outside dataset/test fixtures. Should be gitignored. prescan_artifacts() { - local total=0 detail="" repo dir n - for repo in "${LIB_REPOS[@]}" "${ORG_REPOS[@]}" autolens_workspace autogalaxy_workspace autofit_workspace; do + local total=0 scanned=0 detail="" repo dir n + for repo in "${SCAN_REPOS[@]}"; do dir="$ROOT/$repo" - [[ -d "$dir/.git" || -f "$dir/.git" ]] || continue + repo_is_checked_out "$repo" || continue + scanned=$((scanned + 1)) local leaked leaked=$( { git -C "$dir" ls-files 2>/dev/null | grep -E '(^|/)outputs?/' \ | grep -vE '(^|/)\.gitignore$'; @@ -245,17 +313,21 @@ prescan_artifacts() { total=$((total + leaked)) [[ "$leaked" -gt 0 ]] && detail+="${repo}:${leaked} " done - echo "${total}|${total} tracked files look like leaked outputs/data: ${detail}(fix: gitignore + git rm --cached)" + echo "${total}|${total} tracked files look like leaked outputs/data across ${scanned}/${#SCAN_REPOS[@]} scanned repos: ${detail}(fix: gitignore + git rm --cached)" } # packaging: ignored, fully-untracked Python packaging products at repository # roots. The narrow depth + ignore + tracked-file guards deliberately exclude # nested domain directories named build and any directory that owns source. +# Deliberately NOT filtered to repos that ship a pyproject.toml: the existing +# guards already establish that a hit is a packaging product, and requiring the +# manifest would only narrow detection. prescan_packaging() { - local total=0 detail="" dir repo candidate rel repo_count - for repo in "${LIB_REPOS[@]}"; do + local total=0 scanned=0 detail="" dir repo candidate rel repo_count + for repo in "${CODE_REPOS[@]}"; do dir="$ROOT/$repo" - [[ -d "$dir/.git" || -f "$dir/.git" ]] || continue + repo_is_checked_out "$repo" || continue + scanned=$((scanned + 1)) repo_count=0 while IFS= read -r -d '' candidate; do rel="${candidate#"$dir"/}" @@ -266,7 +338,7 @@ prescan_packaging() { \( -name '*.egg-info' -o -name build \) -print0 2>/dev/null) [[ "$repo_count" -gt 0 ]] && detail+="${repo}:${repo_count} " done - echo "${total}|${total} ignored top-level library packaging directories (*.egg-info/build): ${detail}(clean: DRY_RUN=1 PyAutoBrain/bin/clean_slate.sh --packaging, then run without DRY_RUN)" + echo "${total}|${total} ignored top-level packaging directories (*.egg-info/build) across ${scanned}/${#CODE_REPOS[@]} code repos present: ${detail}(clean: DRY_RUN=1 PyAutoBrain/bin/clean_slate.sh --packaging, then run without DRY_RUN)" } # config: keys present in a library config yaml but missing from the matching @@ -418,9 +490,9 @@ fi # TSV rows: "\t\t\t". enumerate_condemn_candidates() { local repo dir def br - for repo in "${LIB_REPOS[@]}" "${ORG_REPOS[@]}"; do + for repo in "${CODE_REPOS[@]}"; do dir="$ROOT/$repo" - [[ -d "$dir/.git" || -f "$dir/.git" ]] || continue + repo_is_checked_out "$repo" || continue def=$(git -C "$dir" symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null | sed 's@^origin/@@') [[ -n "$def" ]] || def=main while IFS= read -r br; do @@ -526,6 +598,14 @@ emit_json_row() { # mode printf '{"mode":"perf","status":"deferred","hint":"run: pyauto-brain hygiene perf (import timings; skipped in the fast default scan)","delegate":"/refactor"}' return fi + # A repo-array mode that saw no repositories reports `unscanned`, NOT `clean`. + # Its count would be 0 for want of anything to count, and a consumer cannot + # tell the two apart from a zero alone. + if [[ -n "$UNSCANNED_REASON" ]] && mode_reads_repo_arrays "$m"; then + printf '{"mode":"%s","kind":"%s","status":"unscanned","count":null,"repos_present":0,"reason":"%s","delegate":"%s"}' \ + "$m" "${MODE_KIND[$m]}" "${UNSCANNED_REASON//\"/\\\"}" "${MODE_DELEGATE[$m]}" + return + fi if [[ "$m" == "docstrings" ]]; then python3 "$HERE/_hygiene_docstrings.py" --root "$ROOT" --json-row return @@ -548,14 +628,16 @@ emit_json_row() { # mode elif [[ "$kind" == "surface" ]]; then status="surface" elif [[ "$count" == "0" ]]; then status="clean" else status="$kind"; fi # debris | timing - printf '{"mode":"%s","kind":"%s","status":"%s","count":%s,"summary":"%s","delegate":"%s"}' \ + printf '{"mode":"%s","kind":"%s","status":"%s","count":%s,"repos_present":%s,"summary":"%s","delegate":"%s"}' \ "$m" "$kind" "$status" "$([[ "$count" == "-1" ]] && echo null || echo "$count")" \ - "${summary//\"/\\\"}" "${MODE_DELEGATE[$m]}" + "$MANAGED_PRESENT" "${summary//\"/\\\"}" "${MODE_DELEGATE[$m]}" } if [[ "$json" -eq 1 ]]; then if [[ "$mode" == "default" ]]; then - printf '{"decision":"HygieneDecision","mode":"default","rows":[' + printf '{"decision":"HygieneDecision","mode":"default","scan_root":"%s","repos_declared":%s,"repos_present":%s,%s"rows":[' \ + "${ROOT//\"/\\\"}" "${#SCAN_REPOS[@]}" "$MANAGED_PRESENT" \ + "$([[ -n "$UNSCANNED_REASON" ]] && printf '"unscanned_reason":"%s",' "${UNSCANNED_REASON//\"/\\\"}")" sep="" for m in "${MODE_ORDER[@]}"; do printf '%s' "$sep"; emit_json_row "$m"; sep=","; done printf ']}\n' @@ -570,6 +652,12 @@ echo "== HygieneDecision ==" echo "The hygiene conductor pre-scans code-quality debt (read-only) and delegates the" echo "audit + fix to the owning skill — it never mutates a repo itself." echo +if [[ -n "$UNSCANNED_REASON" ]]; then + echo "!! SCANNED 0 REPOS — every 'unscanned' row below means nothing was LOOKED AT," + echo " not that the organism is clean:" + echo " $UNSCANNED_REASON" + echo +fi render_delegate_line() { # mode local m="$1" @@ -599,6 +687,10 @@ render_row() { # mode render_delegate_line "$m" return fi + if [[ -n "$UNSCANNED_REASON" ]] && mode_reads_repo_arrays "$m"; then + printf ' %-9s %-9s %s\n' "$m" "unscanned" "no repository was read — $UNSCANNED_REASON" + return + fi local res count summary kind tag res="$(prescan "$m")"; count="${res%%|*}"; summary="${res#*|}"; kind="${MODE_KIND[$m]}" if [[ "$m" == "docstrings" && "$summary" != *"; 0 parse errors" ]]; then tag="partial" @@ -642,8 +734,19 @@ elif [[ "$mode" == "default" ]]; then for m in "${MODE_ORDER[@]}"; do render_row "$m"; done echo if [[ -n "$best" ]]; then + # Real findings still lead, even when the repo-array modes scanned nothing — + # the helper-backed modes discover their own targets, so their findings are + # genuine. The caveat says the RANKING is partial, not that the work is. echo "Recommended next: hygiene ${best} (${best_n} items), then run ${MODE_DELEGATE[$best]}." echo " Then 'hygiene perf' for import timings; config/deps/docs/noise are periodic audits (surface only)." + if [[ -n "$UNSCANNED_REASON" ]]; then + echo " CAVEAT: the repo-array modes scanned 0 of ${#SCAN_REPOS[@]} declared repos, so this" + echo " ranking is partial — ${UNSCANNED_REASON}." + fi + elif [[ -n "$UNSCANNED_REASON" ]]; then + echo "Recommended next: fix the scan first — this is NOT a clean bill of health." + echo " The repo-array modes scanned 0 of ${#SCAN_REPOS[@]} declared repos under $ROOT:" + echo " ${UNSCANNED_REASON}." else echo "Recommended next: no direct findings or removable debris — run 'hygiene perf' for import timings, and config/deps/docs/noise audits periodically." fi diff --git a/tests/test_hygiene_conductor.py b/tests/test_hygiene_conductor.py index 059320c..45c05df 100644 --- a/tests/test_hygiene_conductor.py +++ b/tests/test_hygiene_conductor.py @@ -832,3 +832,156 @@ def test_refs_findings_reach_the_default_worklist(tmp_path): assert result.returncode == 0, result.stderr rows = {row["mode"]: row for row in json.loads(result.stdout)["rows"]} assert rows["refs"]["count"] == len(_refs_row(tmp_path)["findings"]) > 0 + + +# --- Body-map-derived coverage ------------------------------------------------- +# +# The conductor scans repositories, so WHICH repositories must come from the body +# map rather than from arrays in the script. It used to come from arrays, and they +# drifted: five libraries where the map declared six, four organs of seven, and a +# CRLF count of 5 against a true 127. Nothing caught it, because a repo that is +# never scanned produces no findings and reads as clean. +# +# These tests name no repository. That is deliberate on two counts: a literal here +# would be an instance fact in an organ test (the tenant firewall's concern), and +# a test that hardcodes the very list under test can only ever agree with itself. + +HELPER = BRAIN_HOME / "agents" / "conductors" / "hygiene" / "_hygiene_repos.py" + + +def _derived(category, root=None, parser="auto", mind=None): + env = {**os.environ} + if root is not None: + env["PYAUTO_ROOT"] = str(root) + if mind is not None: + env["PYAUTO_MIND"] = str(mind) + result = subprocess.run( + [sys.executable, str(HELPER), "--category", category, "--parser", parser], + capture_output=True, text=True, env=env, + ) + return result, [line for line in result.stdout.splitlines() if line.strip()] + + +def _manifest_categories(): + """The declared sets, read straight from the body map.""" + import yaml + + path = BRAIN_HOME.parent / "PyAutoMind" / "repos.yaml" + if not path.is_file(): + return None + data = yaml.safe_load(path.read_text()) + grouped = {} + for name, entry in data["repos"].items(): + grouped.setdefault(entry["category"], set()).add(name) + return grouped + + +def test_derived_repo_sets_equal_the_body_map(tmp_path): + declared = _manifest_categories() + if declared is None: + return # body map not checked out here; the drift check owns this leg + for category in ("library", "organ", "workspace"): + result, names = _derived(category, tmp_path) + assert result.returncode == 0, result.stderr + assert set(names) == declared[category], category + + +def test_the_pyyaml_free_reader_agrees_with_the_body_map(tmp_path): + # The fallback runs only where PyYAML is absent, so nothing else would ever + # catch it silently dropping a repo — the exact shape of the original bug. + declared = _manifest_categories() + if declared is None: + return + for category in ("library", "organ", "workspace"): + result, names = _derived(category, tmp_path, parser="minimal") + assert result.returncode == 0, result.stderr + assert set(names) == declared[category], category + + +def test_crlf_covers_every_library_the_body_map_declares(tmp_path): + # One checkout per declared library, each with a single CRLF .py. The count + # must equal the number of libraries: any repo the conductor fails to derive + # is one this assertion misses. No library is named here — that is the point. + _, libraries = _derived("library", tmp_path) + assert libraries, "body map returned no libraries" + for name in libraries: + repo = tmp_path / name + _init_git_repo(repo) + (repo / "mod.py").write_bytes(b"x = 1\r\ny = 2\r\n") + subprocess.run(["git", "-C", str(repo), "add", "-f", "mod.py"], check=True) + + row = json.loads(_run(["crlf", "--json"], tmp_path).stdout)["row"] + + assert row["status"] != "unscanned" + assert f"{len(libraries)} .py w/ CRLF" in row["summary"] + + +ARRAY_MODES = {"tidy", "crlf", "artifacts", "deps", "docs", "packaging"} + + +def test_repo_array_modes_report_unscanned_not_clean_on_an_empty_root(tmp_path): + # A zero from "nothing was scanned" and a zero from "nothing was wrong" are + # indistinguishable to a consumer, so the first must not be called `clean`. + rows = {row["mode"]: row for row in json.loads(_run(["--json"], tmp_path).stdout)["rows"]} + + for mode in ARRAY_MODES: + assert rows[mode]["status"] == "unscanned", mode + assert rows[mode]["count"] is None, mode + assert rows[mode]["repos_present"] == 0, mode + assert "no managed checkouts" in rows[mode]["reason"], mode + + +def test_an_empty_root_is_reported_in_the_default_envelope_and_banner(tmp_path): + doc = json.loads(_run(["--json"], tmp_path).stdout) + assert doc["repos_present"] == 0 + assert doc["repos_declared"] > 0 + assert "no managed checkouts" in doc["unscanned_reason"] + + human = _run([], tmp_path).stdout + assert "SCANNED 0 REPOS" in human + assert "NOT a clean bill of health" in human + + +def test_helper_backed_modes_still_report_findings_on_an_empty_root(tmp_path): + # docstrings/refs/optdeps/extras discover their own targets by walking the + # root, so they can legitimately find material the body map never names. + # Suppressing them alongside the repo-array modes would hide real findings. + _write_docstring_fixture(tmp_path) + + rows = {row["mode"]: row for row in json.loads(_run(["--json"], tmp_path).stdout)["rows"]} + + assert rows["docstrings"]["status"] != "unscanned" + assert rows["docstrings"]["count"] > 0 + assert "Recommended next: hygiene docstrings" in _run([], tmp_path).stdout + + +def test_an_unreachable_body_map_reports_unscanned_rather_than_clean(tmp_path): + # Pointed at a directory holding no body map: the conductor knows of no + # repository at all, which must not read as a clean organism. + mind = tmp_path / "no-map" + mind.mkdir() + root = tmp_path / "root" + root.mkdir() + + result = subprocess.run( + [str(BRAIN), "hygiene", "crlf", "--json"], + capture_output=True, text=True, + env={**os.environ, "PYAUTO_ROOT": str(root), "PYAUTO_MIND": str(mind)}, + ) + + assert result.returncode == 0, result.stderr + row = json.loads(result.stdout)["row"] + assert row["status"] == "unscanned" + assert "body map unreachable" in row["reason"] + + +def test_an_explicit_body_map_override_is_authoritative(tmp_path): + # Falling through to a sibling checkout would scan a different organism than + # the operator named, and silently. + mind = tmp_path / "no-map" + mind.mkdir() + + result, names = _derived("library", tmp_path, mind=mind) + + assert result.returncode == 3 + assert names == []