diff --git a/.opencode/skills/design-patterns/SKILL.md b/.opencode/skills/design-patterns/SKILL.md index 106c4f1..7d1b4de 100644 --- a/.opencode/skills/design-patterns/SKILL.md +++ b/.opencode/skills/design-patterns/SKILL.md @@ -393,3 +393,13 @@ class JsonImporter(Importer): | Class directly calls B, C, D on state change | Observer | | Two functions share the same skeleton, differ in one step | Template Method | | Subsystem is complex and callers need a simple entry point | Facade | + +--- + +## Core Heuristic — Procedural vs OOP + +> **When procedural code requires modifying existing functions to add new variants, OOP is the fix.** + +Procedural code is open to inspection but open to modification too — every new case touches existing logic. +OOP (via Strategy, State, Observer, etc.) closes existing code to modification and opens it to extension through new types. +The smell is always the same: **a place in the codebase that must change every time the domain grows.** diff --git a/.opencode/skills/implementation/SKILL.md b/.opencode/skills/implementation/SKILL.md index 4e741c0..0cc4b7f 100644 --- a/.opencode/skills/implementation/SKILL.md +++ b/.opencode/skills/implementation/SKILL.md @@ -135,7 +135,7 @@ Only write an ADR if the decision is non-obvious or has meaningful trade-offs. R Apply to the stub files just written: - [ ] No class with >2 responsibilities (SOLID-S) -- [ ] No class with >2 instance variables (OC-8) +- [ ] No behavioural class with >2 instance variables (OC-8; dataclasses, Pydantic models, value objects, and TypedDicts are exempt) - [ ] All external deps assigned a Protocol (SOLID-D + Hexagonal) — N/A if no external dependencies identified in scope - [ ] No noun with different meaning across modules (DDD Bounded Context) - [ ] No missing Creational pattern: repeated construction without Factory/Builder @@ -254,7 +254,7 @@ As a software-engineer I declare: * OC-5: one dot per line — AGREE/DISAGREE | file:line * OC-6: no abbreviations — AGREE/DISAGREE | file:line * OC-7: ≤20 lines per function, ≤50 per class — AGREE/DISAGREE | longest: file:line -* OC-8: ≤2 instance variables per class — AGREE/DISAGREE | file:line +* OC-8: ≤2 instance variables per class (behavioural classes only; dataclasses, Pydantic models, value objects, and TypedDicts are exempt) — AGREE/DISAGREE | file:line * OC-9: no getters/setters — AGREE/DISAGREE | file:line * Patterns: no creational smell — AGREE/DISAGREE | file:line * Patterns: no structural smell — AGREE/DISAGREE | file:line diff --git a/.opencode/skills/refactor/SKILL.md b/.opencode/skills/refactor/SKILL.md index fcf27e2..6d84a2e 100644 --- a/.opencode/skills/refactor/SKILL.md +++ b/.opencode/skills/refactor/SKILL.md @@ -291,7 +291,7 @@ Before marking the `@id` complete, verify all of the following. Each failed item | OC-5 | One dot per line | `obj.repo.find(id).name` | | OC-6 | No abbreviations | `usr`, `mgr`, `cfg`, `val`, `tmp` | | OC-7 | Classes ≤ 50 lines, methods ≤ 20 lines | Any method requiring scrolling | -| OC-8 | ≤ 2 instance variables per class | `__init__` with 3+ `self.x =` assignments | +| OC-8 | ≤ 2 instance variables per class *(behavioural classes only; dataclasses, Pydantic models, value objects, and TypedDicts are exempt)* | `__init__` with 3+ `self.x =` assignments in a behavioural class | | OC-9 | No getters/setters | `def get_name(self)` / `def set_name(self, v)` | ### SOLID (Martin 2000) diff --git a/AGENTS.md b/AGENTS.md index 88102af..1223815 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -95,7 +95,7 @@ docs/architecture/ tests/ features// - _test.py ← one per Rule: block, software-engineer-written + _test.py ← one per Rule: block, software-engineer-written unit/ _test.py ← software-engineer-authored extras (no @id traceability) ``` @@ -109,7 +109,7 @@ Tests in `tests/unit/` are software-engineer-authored extras not covered by any ## Test File Layout ``` -tests/features//_test.py +tests/features//_test.py ``` ### Function Naming @@ -177,7 +177,7 @@ uv run task doc-serve - **Function length**: ≤ 20 lines - **Class length**: ≤ 50 lines - **Max nesting**: 2 levels -- **Instance variables**: ≤ 2 per class +- **Instance variables**: ≤ 2 per class *(exception: dataclasses, Pydantic models, value objects, and TypedDicts are exempt — they may carry as many fields as the domain requires)* - **Semantic alignment**: tests must operate at the same abstraction level as the acceptance criteria they cover - **Integration tests**: multi-component features require at least one test in `tests/features/` that exercises the public entry point end-to-end diff --git a/docs/workflow.md b/docs/workflow.md index f63e559..9e24894 100644 --- a/docs/workflow.md +++ b/docs/workflow.md @@ -163,7 +163,9 @@ Each step has a designated agent and a specific deliverable. No step is skipped. │ │ │ ARCHITECTURE SMELL CHECK — hard gate (fix before commit) │ │ [ ] No class with >2 responsibilities (SOLID-S) │ -│ [ ] No class with >2 instance variables (OC-8) │ +│ [ ] No behavioural class with >2 instance variables (OC-8; │ +│ dataclasses, Pydantic models, value objects, TypedDicts │ +│ are exempt) │ │ [ ] All external deps assigned a Protocol (SOLID-D + Hexagonal) │ │ N/A if no external dependencies identified in scope │ │ [ ] No noun with different meaning across planned modules │ @@ -221,6 +223,11 @@ Each step has a designated agent and a specific deliverable. No step is skipped. │ │ │ Load skill refactor — follow its protocol │ │ │ │ │ │ uv run task test-fast after each individual change │ │ │ │ │ │ EXIT: test-fast passes; no smells remain │ │ │ +│ │ ├───────────────────────────────────────────────────────┤ │ │ +│ │ │ SELF-DECLARE │ │ │ +│ │ │ Fill Self-Declaration block in TODO.md │ │ │ +│ │ │ AGREE/DISAGREE per principle with file:line │ │ │ +│ │ │ DISAGREE requires inline justification │ │ │ │ │ └───────────────────────────────────────────────────────┘ │ │ │ │ │ │ │ │ Mark @id completed in TODO.md │ │ @@ -259,7 +266,7 @@ Each step has a designated agent and a specific deliverable. No step is skipped. │ * OC-5: one dot per line — AGREE/DISAGREE | file:line │ │ * OC-6: no abbreviations — AGREE/DISAGREE | file:line │ │ * OC-7: ≤20 lines per function — AGREE/DISAGREE | file:line │ -│ * OC-8: ≤2 instance variables per class — AGREE/DISAGREE | file:line │ +│ * OC-8: ≤2 instance variables per class (behavioural classes only; dataclasses, Pydantic models, value objects, and TypedDicts are exempt) — AGREE/DISAGREE | file:line │ │ * OC-9: no getters/setters — AGREE/DISAGREE | file:line │ │ * Patterns: no creational smell — AGREE/DISAGREE | file:line │ │ * Patterns: no structural smell — AGREE/DISAGREE | file:line │ @@ -471,7 +478,7 @@ Source: docs/features/in-progress/.feature ## Cycle State Test: @id: -Phase: RED | GREEN | REFACTOR +Phase: RED | GREEN | REFACTOR | SELF-DECLARE ## Self-Declaration As a software-engineer I declare: @@ -493,7 +500,7 @@ As a software-engineer I declare: * OC-5: one dot per line — AGREE/DISAGREE | file:line * OC-6: no abbreviations — AGREE/DISAGREE | file:line * OC-7: ≤20 lines per function, ≤50 per class — AGREE/DISAGREE | longest: file:line -* OC-8: ≤2 instance variables per class — AGREE/DISAGREE | file:line +* OC-8: ≤2 instance variables per class (behavioural classes only; dataclasses, Pydantic models, value objects, and TypedDicts are exempt) — AGREE/DISAGREE | file:line * OC-9: no getters/setters — AGREE/DISAGREE | file:line * Patterns: no creational smell — AGREE/DISAGREE | file:line * Patterns: no structural smell — AGREE/DISAGREE | file:line @@ -534,7 +541,7 @@ As a software-engineer I declare: | Function length | ≤ 20 lines | | Class length | ≤ 50 lines | | Max nesting | 2 levels | -| Instance variables per class | ≤ 2 | +| Instance variables per class | ≤ 2 (behavioural classes only; dataclasses, Pydantic models, value objects, TypedDicts are exempt) | | `noqa` comments | 0 | | `type: ignore` comments | 0 | | Orphaned tests | 0 | diff --git a/tests/unit/app_test.py b/tests/unit/app_test.py index 9c52e92..206c51f 100644 --- a/tests/unit/app_test.py +++ b/tests/unit/app_test.py @@ -7,7 +7,6 @@ from app.__main__ import main -@pytest.mark.unit @given(verbosity=st.sampled_from(["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"])) @example(verbosity="INFO") def test_app_main_runs_with_valid_verbosity(verbosity: str) -> None: