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.
This commit is contained in:
@@ -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.
|
- **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.
|
- **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.
|
### Philosophy
|
||||||
- Hard limit: None — let the formatter handle wrapping if needed.
|
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.
|
||||||
- Rationale: 80-char limits cause excessive line continuations that waste tokens.
|
|
||||||
|
### 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)
|
## 11. Structural Dependency Mapping (SDM)
|
||||||
|
|
||||||
|
|||||||
@@ -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 <class>`
|
||||||
|
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 <method> from <Class>`
|
||||||
|
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`
|
||||||
+5
-1
@@ -53,7 +53,8 @@ exclude = [
|
|||||||
|
|
||||||
[tool.ruff.lint]
|
[tool.ruff.lint]
|
||||||
# Enable McCabe complexity check for nesting depth enforcement
|
# 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 style choices that conflict with project's compact style
|
||||||
ignore = [
|
ignore = [
|
||||||
@@ -65,6 +66,9 @@ ignore = [
|
|||||||
"W293", # Blank line contains whitespace
|
"W293", # Blank line contains whitespace
|
||||||
]
|
]
|
||||||
|
|
||||||
|
[tool.ruff.lint.per-file-ignores]
|
||||||
|
"__init__.py" = ["PLR"] # Allow init to export classes
|
||||||
|
|
||||||
[tool.ruff.lint.mccabe]
|
[tool.ruff.lint.mccabe]
|
||||||
max-complexity = 5
|
max-complexity = 5
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user