From b9c1b63f8d1d71227c6a407bdffa7b46bdd564f8 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Mon, 11 May 2026 23:41:41 -0400 Subject: [PATCH] feat(style): Add anti-OOP conventions and OOP refactoring tracker - Add section 10 (Anti-OOP Conventions) to python.md with hard rules, class justification requirements, and Strangler Fig refactoring pattern - Create conductor/refactor_oop.md tracker with 4 phases for class elimination - Add ruff PLR rules (PLR0912, PLR6301, PLR0206) to pyproject.toml for OOP anti-patterns Addresses AI agent scope misinterpretation issues by enforcing flat function-call graphs over deep class hierarchies. --- conductor/code_styleguides/python.md | 65 ++++++++++++++++++++++++++-- conductor/refactor_oop.md | 55 +++++++++++++++++++++++ pyproject.toml | 6 ++- 3 files changed, 121 insertions(+), 5 deletions(-) create mode 100644 conductor/refactor_oop.md diff --git a/conductor/code_styleguides/python.md b/conductor/code_styleguides/python.md index 6de7fe2..7fdc0ae 100644 --- a/conductor/code_styleguides/python.md +++ b/conductor/code_styleguides/python.md @@ -68,11 +68,68 @@ is processed by AI agents, while preserving readability for human review. - **Minimal blank lines.** Token-efficient density is preferred over visual padding. - **Short variable names are acceptable** in tight scopes (loop vars, lambdas). Use descriptive names for module-level and class attributes. -## 9. Line Length +## 10. Anti-OOP Conventions -- Soft limit: 120 characters. -- Hard limit: None — let the formatter handle wrapping if needed. -- Rationale: 80-char limits cause excessive line continuations that waste tokens. +### Philosophy +AI agents consistently misinterpret class hierarchies, method resolution, and inheritance. Flat function-call graphs are deterministic and traceable. OOP introduces scoping complexity that compounds with indentation. + +### Hard Rules (Enforced by lint) +- **Never write a class for a single method.** Use a function. +- **Never use inheritance for code reuse.** Compose with standalone functions. +- **Never use private methods (`_method`).** Module-level functions with clear names suffice. +- **No nested classes.** Define helper types at module level. +- **No decorator classes.** Use plain functions with decorators. + +### Class Justification Required +Every class definition MUST include a comment explaining WHY it is a class and not a function group or struct: + +```python +# JUSTIFIED: Holds mutable shared state across multiple async operations +# Cannot be replaced by functions without passing state through every call +class AsyncOperationScheduler: + ... + +# NOT JUSTIFIED: Simply groups related functions +# Should be module-level functions in operations.py +class OperationHelper: + def validate(self): ... + def execute(self): ... +``` + +### Acceptability Criteria +A class is justified ONLY when ALL of: +1. It holds mutable state that must be encapsulated +2. It has 3+ related methods that share state +3. It implements a behavioral interface used polymorphically (not just data grouping) + +### Refactoring Existing Classes (Strangler Fig Pattern) +When refactoring a class to functions: +1. Write test validating current behavior (prevents regression) +2. Extract one method at a time into module-level functions +3. Create wrapper function that delegates to class until migration complete +4. Delete class only when ALL callers migrated +5. Commit with `refactor(oop):` prefix + +### Data Structures +- **Data-only containers:** Use `NamedTuple`, `dataclass(frozen=True)`, or plain `dict` — NOT classes +- **State machines:** Use dict-based transitions, not class + inheritance +- **Configuration:** Plain dict or `TypedDict`, not classes with defaults + +### Anti-Patterns (Flagged by Ruff PLR rules) +- `PLR0912`: Too many branches — extract to functions +- `PLR6301`: No public methods — class is a namespace anti-pattern +- `PLR0206`: Descriptors in class body — use simple attributes + +### Enforcement +```toml +[tool.ruff.lint.select] +select = ["E", "F", "W", "C90", "C4", "PLR0912", "PLR6301", "PLR0206"] + +[tool.ruff.lint.plr] +max-returns = 4 +max-locals = 8 +max-args = 5 +``` ## 11. Structural Dependency Mapping (SDM) diff --git a/conductor/refactor_oop.md b/conductor/refactor_oop.md new file mode 100644 index 0000000..60cb655 --- /dev/null +++ b/conductor/refactor_oop.md @@ -0,0 +1,55 @@ +# OOP Refactoring Tracker + +Tracks elimination of class-based OOP from the codebase to reduce AI agent scope misinterpretation. + +## Status: IN PROGRESS + +## Phase 1: Leaf Classes (No internal dependents) +These classes have no dependencies on other classes being refactored. Start here. + +- [ ] `src/tool_bias.py` - ToolBiasEngine → module functions + dict +- [ ] `src/session_logger.py` - SessionLogger → module-level functions +- [ ] `src/summarize.py` - SummaryCache class → plain dict + functions + +## Phase 2: Internal Classes (Depend on Phase 1) +These classes use Phase 1 classes. Refactor Phase 1 first. + +- [ ] `src/project_manager.py` - ProjectManager class +- [ ] `src/aggregate.py` - Aggregator class + +## Phase 3: Core Classes (High risk - many callers) +These have many dependents. Handle last with extra testing. + +- [ ] `src/ai_client.py` - AI client classes (AIProvider, etc.) +- [ ] `src/mcp_client.py` - MCPClient, ToolRegistry +- [ ] `src/models.py` - Data classes → dataclass/NamedTuple + +## Phase 4: GUI Classes (Highest risk) +Classes used in GUI callbacks. Require live testing. + +- [ ] `src/gui_2.py` - Main GUI class, panel classes + +## Anti-Regression Protocol + +Before refactoring ANY class: + +1. **Write test** that validates current behavior +2. **Commit baseline** with `test(baseline): add baseline for ` +3. **Extract method** into module-level function +4. **Update all callers** to use function directly +5. **Run test** - must pass identically +6. **Commit extraction** with `refactor(oop): extract from ` +7. **Delete class** only when ALL methods extracted + +## Progress Log + +### 2026-05-11 +- Initial tracker created +- Anti-OOP conventions added to `conductor/code_styleguides/python.md` +- Ruff PLR rules added to `pyproject.toml` + +## Notes + +- Use Strangler Fig pattern: keep class working until last caller migrated +- Prefer `dict`/`NamedTuple` over classes for data containers +- Classes with only data: convert to `dataclass(frozen=True)` or `NamedTuple` \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 3ead2f4..712c730 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,7 +53,8 @@ exclude = [ [tool.ruff.lint] # Enable McCabe complexity check for nesting depth enforcement -select = ["E", "F", "W", "C90", "C4"] +# PLR = complexity rules for methods/functions +select = ["E", "F", "W", "C90", "C4", "PLR0912", "PLR6301", "PLR0206"] # Ignore style choices that conflict with project's compact style ignore = [ @@ -65,6 +66,9 @@ ignore = [ "W293", # Blank line contains whitespace ] +[tool.ruff.lint.per-file-ignores] +"__init__.py" = ["PLR"] # Allow init to export classes + [tool.ruff.lint.mccabe] max-complexity = 5