Agent: split agent.py into a package with a toolkit per AWS service

.claude/skills/check/rules.py

@@ -6,15 +6,16 @@
 
 Errors (exit 1):
   - a top-level import that isn't the standard library, boto3 or botocore (optional packages load lazily)
-  - an import of another analyzer (shared code goes in _kit/), except a companion importing the analyzer it
-    builds on (COMPANIONS: s3_explorer.py on s3.py, agent.py on lambda_functions.py); an import of a module a few
-    analyzers share by any other analyzer (SHARED: _bedrock.py, for bedrock_kb.py, bedrock_chat.py and agent.py);
-    a shared module importing an analyzer;
+  - an import of another analyzer (shared code goes in _kit/), except a companion importing the analyzers it
+    builds on (COMPANIONS: s3_explorer.py on s3.py, the agent/ package on lambda_functions.py); an import of a
+    module a few analyzers share by any other analyzer (SHARED: _bedrock.py, for bedrock_kb.py, bedrock_chat.py and
+    agent/); a shared module importing an analyzer;
     and _kit/ importing any of them
   - an analyzer defining a name that _kit/ has, or the shared module it imports (a copy of a shared helper: import
     it from there instead); and a shared module defining a name that _kit/ has
   - no `from __future__ import annotations`
-  - the five numbered `# N. ...` section banners missing or out of order (not in _kit/ or a shared module)
+  - the five numbered `# N. ...` section banners missing or out of order (not in _kit/, a shared module or an
+    analyzer that is a package, like agent/: one concern per module there, and one of them has the View)
   - an AWS operation that isn't read-only (Put*, Delete*, Create*, ...) - nothing may write
   - a public View method without @_friendly_errors (a companion's window: @_window_errors) or a docstring
     (help() lists the docstring's first line), or one that prints / displays directly instead of building blocks
@@ -41,14 +42,16 @@
 ANALYZERS = ROOT / "src" / PACKAGE
 KIT = ANALYZERS / "_kit"  # the code the analyzers share: checked like them, but it has no sections or View
 ALLOWED_TOP_LEVEL = {"boto3", "botocore"}
-# A companion builds on one analyzer: it imports that analyzer (lazily, so it still imports alone), its UI class
-# is a <Service>View or <Service>Explorer, and its AWS calls are checked against that analyzer's services. The agent
-# builds on lambda_functions: its tools read through LambdaAnalyzer.
-COMPANIONS = {"s3_explorer": "s3", "agent": "lambda_functions"}
+# A companion builds on analyzers: it imports them (lazily, so it still imports alone), its UI class is a
+# <Service>View or <Service>Explorer, and its AWS calls are checked against those analyzers' services. The agent
+# builds on every analyzer one of its toolkits reads through (agent/toolkits/: LambdaToolkit on LambdaAnalyzer).
+COMPANIONS = {"s3_explorer": ("s3",), "agent": ("lambda_functions",)}
 # Code a few analyzers share that the others must not load: a module of the package beside them -> the analyzers
 # that import it. It's checked like _kit (no sections or View), imports _kit and never an analyzer, and the
 # analyzers that import it don't define its names again.
 SHARED = {"_bedrock": {"bedrock_kb", "bedrock_chat", "agent"}}  # the agent shows its answers with _bedrock's _Answer
+# An analyzer can be a package (agent/): its modules are checked like an analyzer's file, as one analyzer.
+PACKAGES = {p.parent.name for p in ANALYZERS.glob("*/__init__.py") if not p.parent.name.startswith("_")}
 NEWER_STDLIB = {"compression", "annotationlib", "tomllib", "zoneinfo", "graphlib"}  # stdlib on newer Pythons
 READ_PREFIXES = ("Get", "List", "Describe", "Head", "Scan", "Query", "Select", "BatchGet", "Lookup", "Search",
                  "Retrieve")
@@ -113,16 +116,22 @@ def _module_root(node: ast.Import | ast.ImportFrom) -> list[str]:
     return [alias.name.split(".")[0] for alias in node.names]
 
 
-def _analyzers_imported(node: ast.Import | ast.ImportFrom) -> list[str]:
+def _analyzers_imported(node: ast.Import | ast.ImportFrom, inside: tuple[str, ...] = ()) -> list[str]:
     """The modules an import loads, by their name in the package: `import s3`, `from .s3 import x`,
-    `from aws_analyzer import s3` and `import aws_analyzer.s3` all give ["s3"]."""
+    `from aws_analyzer import s3` and `import aws_analyzer.s3` all give ["s3"]. `inside` is the package the
+    importing file is in, below aws_analyzer (("agent", "toolkits") for agent/toolkits/x.py): a relative import is
+    read from there, so `from ...lambda_functions import x` gives ["lambda_functions"] and `from ..budget import x`
+    gives ["agent"]."""
     if isinstance(node, ast.Import):
         return [alias.name.split(".")[1] if alias.name.startswith(f"{PACKAGE}.") else alias.name.split(".")[0]
                 for alias in node.names]
     module = node.module or ""
-    if node.level and module:  # from .s3 import x
-        return [module.split(".")[0]]
-    if node.level or module == PACKAGE:  # from . import s3, from aws_analyzer import s3
+    if node.level:  # from .s3 import x, from . import s3, from ..budget import x
+        base = list(inside[:max(len(inside) - (node.level - 1), 0)])
+        if module:
+            return [(base + module.split("."))[0]]
+        return [(base + [alias.name])[0] for alias in node.names]
+    if module == PACKAGE:  # from aws_analyzer import s3
         return [alias.name for alias in node.names]
     if module.startswith(f"{PACKAGE}."):  # from aws_analyzer.s3 import x
         return [module.split(".")[1]]
@@ -195,11 +204,13 @@ def check_file(path: Path, report: Report, readme: str, apis: dict[str, set[str]
     except SyntaxError as exc:
         report.error(f"{rel}:{exc.lineno}", f"doesn't parse: {exc.msg}")
         return
-    in_kit = path.parent.name == KIT.name
-    is_shared = not in_kit and path.stem in SHARED
-    parent = None if in_kit else COMPANIONS.get(path.stem)
-    uses = set() if in_kit else {module for module, users in SHARED.items() if path.stem in users}
-    siblings = {p.stem for p in ANALYZERS.glob("*.py")} - {path.stem, parent, "__init__"} - uses
+    owner, inside = _owner(path)
+    in_kit = owner == KIT.name
+    in_package = owner in PACKAGES
+    is_shared = not in_kit and not in_package and path.stem in SHARED
+    parents = () if in_kit else COMPANIONS.get(owner, ())
+    uses = set() if in_kit else {module for module, users in SHARED.items() if owner in users}
+    siblings = ({p.stem for p in ANALYZERS.glob("*.py")} | PACKAGES) - {owner, "__init__", *parents} - uses
     kit = kit_names() if kit is None else kit
     shared = shared_names() if shared is None else shared
 
@@ -216,7 +227,7 @@ def check_file(path: Path, report: Report, readme: str, apis: dict[str, set[str]
         if not isinstance(node, (ast.Import, ast.ImportFrom)):
             continue
         where = f"{rel}:{node.lineno}"
-        for name in sorted(set(_analyzers_imported(node)) & siblings):
+        for name in sorted(set(_analyzers_imported(node, inside)) & siblings):
             if in_kit:
                 report.error(where, f"_kit imports {name}; _kit loads with every analyzer, so it never imports an "
                                     "analyzer or a module some of them share")
@@ -235,7 +246,7 @@ def check_file(path: Path, report: Report, readme: str, apis: dict[str, set[str]
             if id(node) in top_level and not _is_stdlib(name) and name not in ALLOWED_TOP_LEVEL:
                 report.error(where, f"imports {name} at import time; only boto3 + stdlib may load there "
                                     f"(import it inside the function via _require({name!r}, ...))")
-            elif (id(node) not in top_level and not _is_stdlib(name) and name != parent
+            elif (id(node) not in top_level and not _is_stdlib(name) and name not in parents
                   and name not in ALLOWED_TOP_LEVEL | {"IPython"}):
                 report.warn(where, f"imports {name} directly; use _require({name!r}, purpose) so a missing "
                                    "package becomes a note that says what to pip install")
@@ -253,16 +264,16 @@ def check_file(path: Path, report: Report, readme: str, apis: dict[str, set[str]
 
     # Section banners 1..5 in order.
     banners = [int(m.group(1)) for m in re.finditer(r"^# (\d+)\. ", source, re.MULTILINE)]
-    if banners != [1, 2, 3, 4, 5] and not in_kit and not is_shared:
+    if banners != [1, 2, 3, 4, 5] and not in_kit and not is_shared and not in_package:
         report.error(rel, f"section banners are {banners or 'missing'}; expected # 1. .. # 5. in order "
                           "(Helpers, Data models, Pure analysis, <Service>Analyzer, <Service>View)")
 
     # View methods: decorated, documented, render through blocks.
-    ui_suffixes = ("View", "Explorer") if parent else ("View",)
-    guards = {"_friendly_errors", "_window_errors"} if parent else {"_friendly_errors"}  # a window: _kit's guard
+    ui_suffixes = ("View", "Explorer") if parents else ("View",)
+    guards = {"_friendly_errors", "_window_errors"} if parents else {"_friendly_errors"}  # a window: _kit's guard
     view = next((n for n in tree.body if isinstance(n, ast.ClassDef) and n.name.endswith(ui_suffixes)), None)
     if view is None:
-        if not in_kit and not is_shared:
+        if not in_kit and not is_shared and not in_package:  # a package's View is in one of its modules
             report.error(rel, "no <Service>View class")
     else:
         for member in view.body:
@@ -298,8 +309,9 @@ def check_file(path: Path, report: Report, readme: str, apis: dict[str, set[str]
 
     # AWS operations: read-only, and each one's permission documented in README.md.
     services = _client_services(tree)
-    if parent and (ANALYZERS / f"{parent}.py").exists():  # it calls AWS through the analyzer's clients
-        services |= _client_services(ast.parse((ANALYZERS / f"{parent}.py").read_text(encoding="utf-8")))
+    for parent in parents:  # it calls AWS through the analyzers' clients
+        if (ANALYZERS / f"{parent}.py").exists():
+            services |= _client_services(ast.parse((ANALYZERS / f"{parent}.py").read_text(encoding="utf-8")))
     ops = _operations(services)
     defined = {n.name for n in ast.walk(tree) if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))}
     # Strings that are keyword values or compared with something (engine="converse", kind == "scan") are labels,
@@ -337,6 +349,30 @@ def check_file(path: Path, report: Report, readme: str, apis: dict[str, set[str]
             report.warn(where, f"{name} needs `{' or '.join(a for a in actions if a)}`, which README.md's IAM permissions don't list")
 
 
+def _owner(path: Path) -> tuple[str, tuple[str, ...]]:
+    """(the analyzer a file belongs to, the package it's in below aws_analyzer): s3.py -> ("s3", ()),
+    _kit/fmt.py -> ("_kit", ("_kit",)), agent/toolkits/lambda_tools.py -> ("agent", ("agent", "toolkits"))."""
+    try:
+        parts = path.resolve().relative_to(ANALYZERS.resolve()).parts
+    except ValueError:  # a file outside the package: checked as an analyzer of its own
+        return path.stem, ()
+    return (parts[0], parts[:-1]) if len(parts) > 1 else (path.stem, ())
+
+
+def package_files(package: str) -> list[Path]:
+    """Every module of an analyzer that is a package (agent/), its __init__.py included."""
+    return sorted((ANALYZERS / package).rglob("*.py"))
+
+
+def check_package(package: str, report: Report) -> None:
+    """What a package must have as a whole: a <Service>View class in one of its modules."""
+    for path in package_files(package):
+        tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
+        if any(isinstance(n, ast.ClassDef) and n.name.endswith("View") for n in tree.body):
+            return
+    report.error(f"src/{PACKAGE}/{package}/", "no <Service>View class in any of its modules")
+
+
 def _client_services(tree: ast.AST) -> set[str]:
     """The services a file creates clients for: session.client("s3", ...) -> {"s3"}."""
     return {n.args[0].value for n in ast.walk(tree) if isinstance(n, ast.Call)
@@ -354,17 +390,21 @@ def _documented(action: str, readme: str) -> bool:
 def main() -> int:
     parser = argparse.ArgumentParser(description=(__doc__ or "").split("\n\n")[0])
     parser.add_argument("files", nargs="*", type=Path,
-                        help="files to check (default: src/aws_analyzer/*.py and src/aws_analyzer/_kit/*.py)")
+                        help="files to check (default: src/aws_analyzer/*.py, its packages and _kit/*.py)")
     parser.add_argument("--apis", action="store_true", help="list the AWS operations each analyzer calls")
     args = parser.parse_args()
     readme_path = ROOT / "README.md"
     readme = readme_path.read_text(encoding="utf-8") if readme_path.exists() else ""
     report, apis = Report(), {}
     files = [f.resolve() for f in args.files] or sorted(
         p for p in [*ANALYZERS.glob("*.py"), *KIT.glob("*.py")] if p.name != "__init__.py")
+    if not args.files:
+        files += [path for package in sorted(PACKAGES) for path in package_files(package)]
     kit, shared = kit_names(), shared_names()
     for path in files:
         check_file(path, report, readme, apis, kit, shared)
+    for package in sorted(PACKAGES) if not args.files else ():
+        check_package(package, report)
     for label, items in (("error", report.errors), ("warning", report.warnings)):
         for item in items:
             print(f"{label}: {item}")
@@ -374,9 +414,16 @@ def main() -> int:
             for name in sorted(names):
                 actions = [iam_action(c) or "(none needed)" for c in name.split(" | ")]
                 print(f"  {name:<52} IAM: {' | '.join(actions)}")
-    in_kit = sum(path.parent.name == KIT.name for path in files)
-    in_shared = sum(path.parent.name != KIT.name and path.stem in SHARED for path in files)
-    modules = f"{len(files) - in_kit - in_shared} analyzers" + (f", {in_shared} shared module" if in_shared else "")
+    owners = [_owner(path)[0] for path in files]
+    in_kit = owners.count(KIT.name)
+    in_shared = sum(owner in SHARED for owner in owners)
+    packages = sorted({owner for owner in owners if owner in PACKAGES})
+    in_packages = sum(owner in PACKAGES for owner in owners)
+    modules = f"{len(files) - in_kit - in_shared - in_packages + len(packages)} analyzers"
+    if packages:
+        modules += f" ({', '.join(f'{p}/' for p in packages)} as a package of {in_packages} modules)"
+    if in_shared:
+        modules += f", {in_shared} shared module"
     print(f"\nrules: {modules} and {in_kit} _kit modules, {len(report.errors)} errors, "
           f"{len(report.warnings)} warnings")
     return 1 if report.errors else 0

.claude/skills/check/run.py

@@ -110,7 +110,10 @@ def main() -> int:
 
     # imports: each module of the package in a fresh interpreter, boto3 only.
     failures = []
-    for path in sorted(p for p in (ROOT / "src" / "aws_analyzer").glob("*.py") if p.name != "__init__.py"):
+    package = ROOT / "src" / "aws_analyzer"
+    modules = [p for p in package.glob("*.py") if p.name != "__init__.py"]  # and a package's own: agent/
+    modules += [p.parent for p in package.glob("*/__init__.py") if not p.parent.name.startswith("_")]
+    for path in sorted(modules):
         code = f"MODULE = 'aws_analyzer.{path.stem}'\n{BOTO3_ONLY}"
         proc = subprocess.run([python, "-c", code], cwd=ROOT / "src", capture_output=True, text=True)
         if proc.returncode:

.claude/skills/check/snapshot.py

@@ -280,7 +280,8 @@ def worker(root: Path, source: str, service: str, out: Path, work: Path) -> int:
     with mock_aws():
         kwargs = seed() or {}
         mod = _module(root, service)
-        ours = {k: v for k, v in vars(mod).items() if isinstance(v, type) and v.__module__ == mod.__name__}
+        ours = {k: v for k, v in vars(mod).items()  # a package's classes are in its modules (agent.view, ...)
+                if isinstance(v, type) and (v.__module__ == mod.__name__ or v.__module__.startswith(f"{mod.__name__}."))}
         view_cls = next(v for k, v in ours.items() if k.endswith("View"))  # not the _BaseView it imports
         core_cls = next(v for k, v in ours.items() if k.endswith("Analyzer"))
         core = core_cls(**kwargs) if "session" in kwargs else core_cls(region=REGION, **kwargs)
@@ -308,20 +309,22 @@ def names_worker(root: Path, out: Path) -> int:
     module has, and the attributes of each class of ours it has."""
     folder, prefix = _layout(root)
     modules = [path for path in sorted(folder.glob("*.py")) if not path.stem.startswith("_")]
-    ours = {prefix + path.stem for path in modules}
+    modules += [path for path in sorted(folder.glob("*/__init__.py")) if not path.parent.name.startswith("_")]
+    names = {path: path.parent.name if path.name == "__init__.py" else path.stem for path in modules}  # agent/
+    ours = {prefix + name for name in names.values()}
 
     def is_ours(obj) -> bool:
         module = getattr(obj, "__module__", "") or ""
         return module in ours or (bool(prefix) and module.startswith(prefix))
 
     report: dict[str, dict] = {}
     for path in modules:
-        mod = _module(root, path.stem)
+        mod = _module(root, names[path])
         present = sorted(n for n in dir(mod) if not (n.startswith("__") and n.endswith("__")))
         classes = {n: sorted(a for a in dir(getattr(mod, n)) if not (a.startswith("__") and a.endswith("__")))
                    for n in present if isinstance(getattr(mod, n), type) and is_ours(getattr(mod, n))}
-        report[path.stem] = {"defined": [n for n in _defined(path) if n in present], "present": present,
-                             "classes": classes}
+        report[names[path]] = {"defined": [n for n in _defined(path) if n in present], "present": present,
+                               "classes": classes}
     (out / "names.json").write_text(json.dumps(report, indent=1, sort_keys=True), encoding="utf-8")
     return 0
 

.claude/skills/demo/demo.py

@@ -2044,9 +2044,10 @@ def main() -> int:
     args = parser.parse_args()
 
     package = ROOT / "src" / "aws_analyzer"
-    if args.service.startswith("_") or not (package / f"{args.service}.py").exists():
-        names = ", ".join(sorted(p.stem for p in package.glob("*.py") if not p.stem.startswith("_")))
-        parser.error(f"no src/aws_analyzer/{args.service}.py (have: {names})")
+    modules = sorted({p.stem for p in package.glob("*.py")} | {p.parent.name for p in package.glob("*/__init__.py")})
+    modules = [name for name in modules if not name.startswith("_")]  # a file, or a package like agent/
+    if args.service not in modules:
+        parser.error(f"no src/aws_analyzer/{args.service}.py (have: {', '.join(modules)})")
 
     mock = None
     extra: dict = {}
@@ -2070,7 +2071,8 @@ def main() -> int:
 
     try:
         mod = importlib.import_module(f"aws_analyzer.{args.service}")
-        ours = {k: v for k, v in vars(mod).items() if isinstance(v, type) and v.__module__ == mod.__name__}
+        ours = {k: v for k, v in vars(mod).items()  # a package's classes are in its modules (agent.view, ...)
+                if isinstance(v, type) and (v.__module__ == mod.__name__ or v.__module__.startswith(f"{mod.__name__}."))}
         view_cls = next(v for k, v in ours.items() if k.endswith("View"))  # not the _BaseView it imports
         core_cls = next(v for k, v in ours.items() if k.endswith("Analyzer"))
         kwargs = {"region": args.region, "profile": args.profile} if args.live else {"region": "us-east-1"}