feat(rules): bootstrap 3 Python doc families (architecture, ioc, logging)

CLAUDE.md

@@ -39,6 +39,9 @@ Each enforceable guide in `docs/` should have a matching agent in `agents/`. The
 | `go-licensing-guide.md` | `license-assistant` |
 | `agent-command-development-guide.md` | `agent-auditor` + `slash-command-auditor` |
 | `claude-code-skill-writing-guide.md` | `skill-auditor` |
+| `python-architecture-patterns.md` | `python-architecture-assistant` |
+| `python-ioc-guide.md` | `python-architecture-assistant` |
+| `python-logging-guide.md` | `python-quality-assistant` |
 
 Reference-only docs (patterns, setup guides) don't need agents.
 

docs/python-architecture-patterns.md

@@ -12,17 +12,30 @@ The standard pattern for Python services follows this structure:
 
 ## 1. Core Pattern: Constructor Injection
 
-### Protocol Definition
+### RULE python-architecture/constructor-injection-only (MUST)
 
-```python
-from typing import Protocol
+**Owner**: python-architecture-assistant
+**Applies when**: a Python service class receives a dependency (logger, repository, API client, validator, etc.) through a *method parameter* instead of through `__init__`. Related but distinct anti-patterns covered by other rules: dependency from a global module variable (see `python-architecture/main-py-composition-root`); post-construction `self.foo = bar.foo` mutation (see `python-ioc/dependencies-as-private-fields`, which mandates private-field storage that makes mutation socially difficult).
+**Enforcement**: judgment (semantic — requires distinguishing dependency injection from runtime parameter passing. The other two related anti-patterns have their own rules and enforcement paths.)
+**Why**: Mixing dependency injection with runtime parameter passing breaks the contract Python's type system relies on. Constructor injection makes the dep set visible at `__init__`, immutable after construction, and self-documenting at the type signature. Method-level injection means tests have to construct the full dep graph for every call site; global injection means tests are order-dependent. Methods should receive only the data needed to do their job — `create_user(user: User)`, not `create_user(repo, logger, validator, user)`.
+
+#### Bad
 
-class UserRepository(Protocol):
-    def save(self, user: User) -> None: ...
-    def find_by_id(self, user_id: int) -> User | None: ...
+```python
+class UserService:
+    def create_user(
+        self,
+        repo: UserRepository,   # ← dependency, not runtime data
+        logger: Logger,         # ← dependency, not runtime data
+        validator: UserValidator,  # ← dependency, not runtime data
+        user: User,             # ← actual runtime data
+    ) -> None:
+        validator.validate(user)
+        repo.save(user)
+        logger.info(f"Created user {user.id}")
 ```
 
-### Service Implementation
+#### Good
 
 ```python
 class UserService:
@@ -32,24 +45,64 @@ class UserService:
         logger: Logger,
         validator: UserValidator,
     ):
-        self._repo = repo           # Static dependency
-        self._logger = logger       # Static dependency
-        self._validator = validator # Static dependency
+        self._repo = repo
+        self._logger = logger
+        self._validator = validator
 
-    def create_user(self, user: User) -> None:  # Runtime param only
+    def create_user(self, user: User) -> None:   # only runtime data
         self._validator.validate(user)
         self._repo.save(user)
         self._logger.info(f"Created user {user.id}")
 ```
 
-**Key points:**
-- Constructor receives ALL dependencies (static, mockable)
-- Methods receive ONLY runtime data (request params, user input)
-- Dependencies stored as private fields (`self._dep`)
-- Clean separation: deps bound once, methods reusable
+**Key points** (the RULE above shows the canonical Bad/Good shape; below are the orthogonal supporting conventions):
+
+- Constructor receives ALL dependencies (static, mockable); methods receive ONLY runtime data.
+- Dependencies stored as private fields (`self._dep`) — see [`python-ioc/dependencies-as-private-fields`](python-ioc-guide.md).
+- Dependency interfaces use `Protocol` — see [`python-ioc/protocol-not-abc-for-dependencies`](python-ioc-guide.md).
 
 ## 2. main.py Pattern (Composition Root)
 
+### RULE python-architecture/main-py-composition-root (SHOULD)
+
+**Owner**: python-architecture-assistant
+**Applies when**: a Python application instantiates service objects at module level (top of `repository.py`, `handler.py`, etc.) instead of wiring them inside `main()` / `main.py`.
+**Enforcement**: judgment (file-scope check: module-level `Service()` / `Repository()` calls outside `if __name__ == "__main__":` blocks)
+**Why**: Module-level instantiation runs at *import time* — order-dependent, hard to test, impossible to mock for unit tests. The composition root pattern says: configuration parsing, infrastructure setup, and service wiring all happen in one place (`main.py` → `main(argv)`), in a deterministic order, with explicit dependency edges. Tests can then construct their own composition root with mock infrastructure; production constructs the real one. Module-level state collapses this clean separation into spaghetti.
+
+#### Bad
+
+```python
+# user_service.py
+from infrastructure import database, api_client  # imports trigger connection!
+
+repo = SqlUserRepository(database)             # module-level — runs at import
+service = UserService(repo, api_client)         # module-level — runs at import
+```
+
+#### Good
+
+```python
+# main.py — composition root
+def main(argv):
+    logging.basicConfig(...)
+    db_url = os.getenv("DATABASE_URL")
+    api_key = os.getenv("API_KEY")
+
+    database = Database(db_url)               # infrastructure first
+    api_client = ApiClient(api_key)
+
+    repo = SqlUserRepository(database)        # services composed explicitly
+    service = UserService(repo, api_client)
+
+    handler = UserHandler(service)
+    HttpServer(8080, handler).run()
+
+# user_service.py — no module-level instantiation
+class UserService:
+    def __init__(self, repo: UserRepository, api: ApiClient): ...
+```
+
 The `main.py` is the **composition root** where all dependencies are wired together.
 
 ### Structure

docs/python-ioc-guide.md

@@ -53,22 +53,17 @@ class UserValidator(Protocol):
 
 ## Rules
 
-### Use Protocol for Dependency Interfaces
+### RULE python-ioc/protocol-not-abc-for-dependencies (MUST)
 
-**Constraint:** MUST use `Protocol` for defining dependency interfaces unless shared implementation is required.
+**Owner**: python-architecture-assistant
+**Applies when**: a Python service interface (dependency surface, e.g. `UserRepository`, `Logger`) is declared with `abc.ABC` + `@abstractmethod` decorators instead of `typing.Protocol`, AND no shared implementation across concrete types motivates the ABC.
+**Enforcement**: judgment (ast-grep partial: detect `class X(ABC):` declarations that only contain `@abstractmethod` methods — but the "no shared impl needed" trigger is semantic)
+**Why**: Protocol gives you structural typing — any class with the right method shape satisfies it, without inheritance. That's the right primitive for dependency interfaces: mocks don't need to inherit from anything, tests don't fake an `ABC` hierarchy, and you avoid the "every protocol class has both an ABC and a Protocol declaration" duplication that ABC-first projects accrete. ABC is the right primitive when concrete implementations share code via `super()` — that's the actual reason ABCs exist. For dependency interfaces (which are pure contracts), Protocol is shorter, lighter, and friendlier to mocks.
 
-**Rationale:** Protocol enables structural typing and works seamlessly with mocks without inheritance overhead.
+#### Bad
 
-**Examples:**
 ```python
-# [GOOD]
-from typing import Protocol
-
-class UserRepository(Protocol):
-    def save(self, user: User) -> None: ...
-    def find_by_id(self, user_id: int) -> User | None: ...
-
-# [BAD] - Using ABC when no shared implementation needed
+# Using ABC when no shared implementation needed
 from abc import ABC, abstractmethod
 
 class UserRepository(ABC):
@@ -77,6 +72,16 @@ class UserRepository(ABC):
         pass
 ```
 
+#### Good
+
+```python
+from typing import Protocol
+
+class UserRepository(Protocol):
+    def save(self, user: User) -> None: ...
+    def find_by_id(self, user_id: int) -> User | None: ...
+```
+
 **Protocol vs ABC Decision:**
 
 | Need | Use |
@@ -120,23 +125,30 @@ class UserService:
         self._validator = UserValidator()
 ```
 
-### Store Dependencies as Private Fields
+### RULE python-ioc/dependencies-as-private-fields (MUST)
 
-**Constraint:** MUST store injected dependencies as private fields with underscore prefix (`self._repo`).
+**Owner**: python-architecture-assistant
+**Applies when**: a Python service class stores an injected dependency on `self.<name>` (public attribute) instead of `self._<name>` (single-underscore private convention).
+**Enforcement**: judgment (ast-grep first-pass filter: any `assignment` inside `__init__` with left-hand side `self.X` where `X` does not start with `_`. Type-annotation resolution to distinguish service deps from public data fields cannot be done in ast-grep — the agent makes the final call. Overinclusive first-pass is acceptable: most public `self.X` assignments in service classes ARE the smell.)
+**Why**: Public attribute storage invites external mutation — `user_service.repo = MockRepository()` — which breaks the immutability contract that makes constructor injection safe in the first place. Tests that mutate inject-time fields hide behavior that production never exercises. Single-underscore prefix is Python's universal "internal, don't touch" convention; following it forces test code to compose deps the same way production does (through the constructor) and keeps the dep set observable only at `__init__`.
 
-**Rationale:** Prevents external mutation and clearly indicates internal implementation details.
+#### Bad
 
-**Examples:**
 ```python
-# [GOOD]
 class UserService:
     def __init__(self, repo: UserRepository):
-        self._repo = repo  # Private field
+        self.repo = repo                 # public — external code can rebind it
 
-# [BAD]
+# elsewhere:
+user_service.repo = SomeOtherRepo()      # silent contract violation
+```
+
+#### Good
+
+```python
 class UserService:
     def __init__(self, repo: UserRepository):
-        self.repo = repo  # Public field allows external mutation
+        self._repo = repo                # private — single-underscore convention
 ```
 
 ### Never Return Values from Constructors

docs/python-logging-guide.md

@@ -38,44 +38,49 @@ logger = logging.getLogger(__name__)
 
 ## Configuration Rules
 
-### Configure Logging Once at Application Entry Point
+### RULE python-logging/configure-once-in-main (MUST)
 
-**Constraint:** Application code MUST call `logging.basicConfig()` exactly once at startup in `main.py`. Library code MUST NOT call `basicConfig()` or configure handlers.
+**Owner**: python-quality-assistant
+**Applies when**: a Python file in *library code* (modules imported by application code, intended for reuse across applications) calls `logging.basicConfig()` or adds handlers to the root logger. **Exempt**: the application entry point (`main.py` / `__main__.py` / `cli.py`) AND application-private helper modules wired from the entry point exclusively for configuration (e.g. `logging_setup.py` — see the next section), since those are part of the entry-point configuration surface, not library code.
+**Enforcement**: judgment (semantic — distinguishing library code from application-private configuration helpers requires reading the import graph; ast-grep can flag every non-entry-point `logging.basicConfig` call as a first-pass filter, but the agent must rule out the helper-module exemption case)
+**Why**: `basicConfig` is a one-shot global root-logger setup. Calling it from library code produces three failure modes: (1) first import wins, so the library's config silently overrides the application's choice depending on import order; (2) repeated calls add duplicate handlers, doubling every log line; (3) applications can't change log level without code edits in libraries they don't control. Libraries call `logging.getLogger(__name__)` and emit; configuration is the application's responsibility, and the application does it exactly once.
 
-**Rationale:** Multiple configurations cause conflicts, overwrites, and duplicate log entries across modules.
+#### Bad
 
-**Examples:**
 ```python
-# [GOOD] Application entry point
-# main.py
+# mylib/service.py — library configures logging — wrong
 import logging
 
-logging.basicConfig(
-    format='%(asctime)s %(levelname)-8s [%(name)s:%(lineno)d] %(message)s',
-    level=logging.INFO,
-    datefmt='%Y-%m-%d %H:%M:%S'
-)
-
+logging.basicConfig(level=logging.INFO)   # first import wins, overrides app
 logger = logging.getLogger(__name__)
-logger.info("Application started")
 
-# [GOOD] Library module
-# mylib/service.py
+class UserService: ...
+```
+
+#### Good
+
+```python
+# main.py — application entry point, configure once
 import logging
 
-logger = logging.getLogger(__name__)  # Get logger only
+def main():
+    logging.basicConfig(
+        format='%(asctime)s %(levelname)-8s [%(name)s:%(lineno)d] %(message)s',
+        level=logging.INFO,
+    )
+    # ... rest of app wiring ...
 
-class UserService:
-    def process(self):
-        logger.info("Processing user")
+if __name__ == "__main__":
+    main()
 
-# [BAD] Library configuring logging
-# mylib/service.py
-logging.basicConfig(level=logging.INFO)  # Never do this in libraries
+# mylib/service.py — library only gets a logger, never configures
+import logging
+
+logger = logging.getLogger(__name__)
 
-# [BAD] Multiple configurations
-# orders/handler.py
-logging.basicConfig(level=logging.DEBUG)  # Conflicts with main config
+class UserService:
+    def process(self):
+        logger.info("processing user")
 ```
 
 ### Extract Logging Configuration to Dedicated Module
@@ -377,23 +382,28 @@ logger.warning("Failed to connect", connection_error)  # Only logs first arg
 logger.warning("Failed to connect: %s", connection_error)
 ```
 
-### Use Lazy Evaluation for Expensive DEBUG Operations
+### RULE python-logging/lazy-evaluation-for-debug (MUST)
 
-**Constraint:** Code MUST use `%s` formatting (not f-strings) for DEBUG messages with expensive operations.
+**Owner**: python-quality-assistant
+**Applies when**: a Python `logger.debug(...)` / `logger.log(logging.DEBUG, ...)` call passes an f-string whose interpolation calls an expensive function (serializer, JSON dump, network fetch, large-collection traversal) instead of using `%s` placeholders with deferred arguments.
+**Enforcement**: judgment (ast-grep is a first-pass filter only: `call_expression` named `logger.debug` with an `interpreted_string_literal` first argument matching `f"..."`. Trivial f-string interpolations like `logger.debug(f"count={count}")` are NOT violations — the rule targets *expensive* interpolations (serializers, JSON dumps, network fetches, large-collection traversals). The agent makes the expensive-vs-trivial judgment after the mechanical filter.)
+**Why**: F-strings interpolate immediately at function-call time. With `logger.debug(f"...{expensive(x)}...")`, `expensive(x)` runs every call, even when DEBUG is filtered out and the message is discarded. `logger.debug("... %s ...", expensive(x))` defers `expensive(x)` to the logging library, which only evaluates arguments when the level is enabled. In hot paths with DEBUG-by-default-off production, the difference between "free" and "this expensive call runs millions of times for log lines no one sees" is measurable. The `isEnabledFor(logging.DEBUG)` guard is the explicit alternative; `%s` is the implicit-defer pattern that scales.
 
-**Rationale:** `%s` formatting only evaluates arguments if DEBUG level is enabled, avoiding unnecessary computation.
+#### Bad
 
-**Examples:**
 ```python
-# [BAD] F-string with expensive DEBUG operation
+# F-string forces expensive_serialization to run on every call,
+# even when DEBUG is filtered out and the message is discarded
 logger.debug(f"Details: {expensive_serialization(large_object)}")
-# Always evaluates, even when DEBUG disabled
+```
+
+#### Good
 
-# [GOOD] Lazy evaluation with %s
+```python
+# %s placeholder defers evaluation to the logging library — runs only if DEBUG enabled
 logger.debug("Details: %s", expensive_serialization(large_object))
-# Only evaluates if DEBUG enabled
 
-# [GOOD] Explicit guard for expensive operations
+# Explicit guard — same effect, more readable when you also need f-string features
 if logger.isEnabledFor(logging.DEBUG):
     logger.debug(f"Details: {expensive_serialization(large_object)}")
 ```