diff --git a/.claude/commands/new-pattern.md b/.claude/commands/new-pattern.md index 7fdab6f..5211221 100644 --- a/.claude/commands/new-pattern.md +++ b/.claude/commands/new-pattern.md @@ -1,17 +1,26 @@ --- -description: Scaffold a new pattern unit under patterns// +description: Scaffold a new pattern module under patterns// argument-hint: / "Pattern Name" --- -Scaffold a new pattern unit for $ARGUMENTS. +Scaffold a new pattern module for $ARGUMENTS. 1. Validate the group is one of: principle, python, creational, structural, behavioral, modern. Refuse anything else. -2. Create `patterns///` with the exact template from CLAUDE.md: - README.md (frontmatter with `id: /`, all schema keys present, - `verdict:` left as `use-with-care` with a `TODO` caveat), empty-but-importable - `__init__.py`, and stub `naive.py`, `pythonic.py`, `real_world.py` each with a - typed `main() -> None` and script guard, plus `tests/test_.py` with one - failing `test_todo` marked `xfail(reason="unit not yet written")`. -3. Run `make check` and report the result. Do not write the actual pattern content — +2. Create `patterns///` with the exact module template from CLAUDE.md: + - `README.md` — frontmatter with `id: /`, all schema keys present, + `verdict:` left as `use-with-care` with a `TODO` caveat; a ~10-line front door + mapping the folders. + - `__init__.py` re-exporting from `pattern/`; `pattern/__init__.py` + + `pattern/.py` with a typed stub. + - `docs/fundamentals.md`, `docs/implementation.md`, `docs/examples.md` — each a + heading plus a `TODO` line naming what belongs there (classic-form contrast in + fundamentals; never use the word "naive"). + - `examples/__init__.py` and `examples/demo/` (`__init__.py`, `__main__.py` with a + typed `main() -> None` + script guard that imports from `...pattern`). + - `tests/test_.py` with one failing `test_todo` marked + `xfail(reason="unit not yet written")`. +3. Run `make check` and report the result. Note: the catalog loader will fail the + unit until the docs files, a runnable example importing `pattern/`, and real + tests exist — that is the point. Do not write the actual pattern content — scaffolding only. diff --git a/docs/code-review.md b/.github/code-review.md similarity index 62% rename from docs/code-review.md rename to .github/code-review.md index 3d301aa..b5714c9 100644 --- a/docs/code-review.md +++ b/.github/code-review.md @@ -23,7 +23,7 @@ tests that's idiomatic, not a finding. - **PEP 8** (style) · **PEP 257** (docstrings) · **PEP 20** (design sensibility) - **Google Python Style Guide** — the most common team-level extension - This repo's own bar: [CLAUDE.md](../CLAUDE.md) (unit template, frontmatter - schema) and [verdicts.md](verdicts.md) + schema) and [docs/verdicts.md](../docs/verdicts.md) ## Layer 2: what human reviewers actually check @@ -54,6 +54,36 @@ Severity-ordered — block on CRITICAL/HIGH, note MEDIUM: - **Context7 MCP** — current library/framework docs, for "is this the right API usage?" questions. - **python-patterns.guide** — the prose authority behind this catalog's verdicts. +## House rules (settled during the v2 migration, enforced in review) + +- Name-keyed registries and caretakers refuse silent duplicates: `ValueError` + unless `replace=True`. Ordered collections where repeats are meaningful — + chains, signals — append freely; `functools.singledispatch`'s own overwrite + behavior is inherited, noted not fought. +- `ParamSpec` typing only where a wrapper callable is returned + (`structural/decorator` is the precedent); registries that return callables + unchanged use plain identity typing. +- Never use `None` as a cache sentinel — a factory may legitimately return + `None` and it must still cache once (`LazyProxy._MISSING` precedent). + Immutability guards recurse into containers: a tuple holding a list is + mutable where it counts (`InternPool` precedent). +- Every unit's `examples/` must genuinely build on its `pattern/` package — + an AST import check enforces the import; reviewers judge token imports + (an annotation-only import that vanishes at runtime does not count). +- Frontmatter stays stable; deliberate caveat improvements are allowed and + called out in review. + +## Mutation discipline + +Reading a diff is not verification. For any load-bearing claim — a shutdown +discipline, a durability promise, a security guard, an "import does no work" +assertion — apply the mutation that would falsify it (swap the operator, +collapse the branch, make the write non-atomic) and confirm the suite fails. +During the v2 migration this caught four real defects that reading alone did +not: an untestable shutdown switch, a sqlite adapter that never committed, a +fixture that erased its own evidence, and a runtime-erased annotation import. +If the mutant survives, the finding is the missing test, not the mutation. + ## Review etiquette - Cite the rule or the file, not taste ("B008: mutable default" beats "I don't like this"). diff --git a/AGENTS.md b/AGENTS.md index 6f5c045..c1fc42b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,17 +1,17 @@ # Agent instructions — python-design-patterns Companion catalog to [python-patterns.guide](https://python-patterns.guide/): every design -pattern as runnable, tested, typed Python — plus an MCP server (`src/design_patterns_mcp/`) -that serves the catalog to agents. +pattern as a self-contained, tested, typed Python module — plus an MCP server +(`src/design_patterns/mcp/`) that serves the catalog to agents. ## Layout -- `patterns///` — one directory per pattern ("unit"). Groups: +- `patterns///` — one module per pattern ("unit"). Groups: `principle`, `python`, `creational`, `structural`, `behavioral`, `modern`. -- `src/design_patterns/` — catalog loader (frontmatter → typed `Pattern` objects). -- `src/design_patterns_mcp/` — FastMCP server (tools, resources, prompts, sandbox). -- Legacy flat dirs (`behavioral/`, `combos/`, `creational/`, `structural/`) are - pre-migration code: excluded from lint, deleted as units absorb them. Do not add to them. +- `src/design_patterns/` — catalog loader (frontmatter → typed `Pattern` objects, + strict structure validation in CI). +- `src/design_patterns/mcp/` — MCP server on the mcp 2.x SDK (`MCPServer`, not + FastMCP): tools, resources, prompts, sandbox. ## Pattern unit template @@ -19,19 +19,37 @@ Every unit has exactly this shape (scaffold one with `/new-pattern`): ``` patterns/// -├── README.md # YAML frontmatter + prose -├── __init__.py -├── naive.py # the literal 1994/Java-style translation -├── pythonic.py # what you actually write in Python -├── real_world.py # the pattern as it appears in the stdlib -└── tests/test_.py +├── README.md # YAML frontmatter + ~10-line front door: problem, verdict, folder map +├── __init__.py # public API — re-exports from pattern/ +├── pattern/ # the pattern as importable, typed library code +│ ├── __init__.py +│ └── .py # named for what it provides (chain.py, decorators.py, …) +├── docs/ +│ ├── fundamentals.md # intent, participants, mechanism, when/when-not, +│ │ # the classic (GoF) form as an annotated listing — never call it "naive" +│ ├── implementation.md# introducing it into a real system: smell, steps, idioms, pitfalls +│ └── examples.md # cited EXTERNAL usages: stdlib, OSS, articles +├── examples/ # runnable mini-projects that import pattern/ +│ ├── __init__.py +│ └── / # realistic domain, no Foo/Bar; __main__.py + modules +└── tests/ # isolated: test_.py + test_.py ``` -- Each `.py` variant is import-safe (no side effects at import) and has a - `main() -> None` demo runnable as a script (`if __name__ == "__main__": main()`). -- Tests import the variants and assert behavior — never just "it runs". +- Everything import-safe (no side effects at import); mini-projects run via + `uv run python -m patterns...examples.`. +- Tests assert behavior, never just "it runs"; load-bearing claims get the + mutation treatment (`.github/code-review.md`). +- Examples must genuinely build on `pattern/` — the loader and a catalog test + enforce the structure and the import; reviewers reject token imports. - Full type hints; `mypy --strict` must pass. +## House rules + +- Name-keyed registries refuse silent duplicates (`ValueError` unless + `replace=True`); ordered collections append freely. +- `ParamSpec` only where a wrapper callable is returned; identity typing otherwise. +- Never `None` as a cache sentinel; immutability guards recurse into containers. + ## Frontmatter schema (the MCP server indexes this — keep it valid) ```yaml @@ -47,14 +65,15 @@ stdlib_sightings: [functools.wraps, contextlib.contextmanager] ``` Verdicts: `pythonic` = use it as shown; `use-with-care` = valid but has sharp edges -(caveats say which); `prefer-alternative` = the naive form exists for study, the -pythonic file shows what to write instead (e.g. Singleton → module global, -Visitor → singledispatch). See `docs/verdicts.md`. +(caveats say which); `prefer-alternative` = the classic form exists for study in +docs/fundamentals.md, `pattern/` exports the alternative to write instead +(e.g. Singleton → module global, Visitor → singledispatch). See `docs/verdicts.md`. ## Workflow - Branches: `main ← staging ← feat/`. PRs target `staging`. Never push to `main`. -- Gate before any PR: `make check` (ruff lint+format, mypy --strict, pytest+cov) green. +- Gate before any PR: `make check` (ruff lint+format, mypy --strict, pytest+cov, + readme-table drift) green. - Commit style: `: ` (`feat`, `fix`, `chore`, `docs`, `refactor`). - Toolchain is uv only — no pip/poetry. `make install` to set up. @@ -63,5 +82,6 @@ Visitor → singledispatch). See `docs/verdicts.md`. - Lead with the problem, not the pattern name's history. - Say plainly when Python makes the pattern unnecessary — that honesty is the point of the repo. Cite the guide chapter when one exists. -- naive.py mirrors the GoF book faithfully, even when un-Pythonic (that's its job); - pythonic.py is idiomatic; real_world.py points at real stdlib usage. +- The classic form in fundamentals.md mirrors the GoF book faithfully, even when + un-Pythonic (that's its job); `pattern/` is idiomatic; examples.md points at + real external usage. diff --git a/CLAUDE.md b/CLAUDE.md index 9eb6141..18876e5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,17 +1,17 @@ # python-design-patterns Companion catalog to [python-patterns.guide](https://python-patterns.guide/): every design -pattern as runnable, tested, typed Python — plus an MCP server (`src/design_patterns_mcp/`) -that serves the catalog to agents. +pattern as a self-contained, tested, typed Python module — plus an MCP server +(`src/design_patterns/mcp/`) that serves the catalog to agents. ## Layout -- `patterns///` — one directory per pattern ("unit"). Groups: +- `patterns///` — one module per pattern ("unit"). Groups: `principle`, `python`, `creational`, `structural`, `behavioral`, `modern`. -- `src/design_patterns/` — catalog loader (frontmatter → typed `Pattern` objects). -- `src/design_patterns_mcp/` — FastMCP server (tools, resources, prompts, sandbox). -- Legacy flat dirs (`behavioral/`, `combos/`, `creational/`, `structural/`) are - pre-migration code: excluded from lint, deleted as units absorb them. Do not add to them. +- `src/design_patterns/` — catalog loader (frontmatter → typed `Pattern` objects, + strict structure validation in CI). +- `src/design_patterns/mcp/` — MCP server on the mcp 2.x SDK (`MCPServer`, not + FastMCP): tools, resources, prompts, sandbox. ## Pattern unit template @@ -19,19 +19,37 @@ Every unit has exactly this shape (scaffold one with `/new-pattern`): ``` patterns/// -├── README.md # YAML frontmatter + prose -├── __init__.py -├── naive.py # the literal 1994/Java-style translation -├── pythonic.py # what you actually write in Python -├── real_world.py # the pattern as it appears in the stdlib -└── tests/test_.py +├── README.md # YAML frontmatter + ~10-line front door: problem, verdict, folder map +├── __init__.py # public API — re-exports from pattern/ +├── pattern/ # the pattern as importable, typed library code +│ ├── __init__.py +│ └── .py # named for what it provides (chain.py, decorators.py, …) +├── docs/ +│ ├── fundamentals.md # intent, participants, mechanism, when/when-not, +│ │ # the classic (GoF) form as an annotated listing — never call it "naive" +│ ├── implementation.md# introducing it into a real system: smell, steps, idioms, pitfalls +│ └── examples.md # cited EXTERNAL usages: stdlib, OSS, articles +├── examples/ # runnable mini-projects that import pattern/ +│ ├── __init__.py +│ └── / # realistic domain, no Foo/Bar; __main__.py + modules +└── tests/ # isolated: test_.py + test_.py ``` -- Each `.py` variant is import-safe (no side effects at import) and has a - `main() -> None` demo runnable as a script (`if __name__ == "__main__": main()`). -- Tests import the variants and assert behavior — never just "it runs". +- Everything import-safe (no side effects at import); mini-projects run via + `uv run python -m patterns...examples.`. +- Tests assert behavior, never just "it runs"; load-bearing claims get the + mutation treatment (`.github/code-review.md`). +- Examples must genuinely build on `pattern/` — the loader and a catalog test + enforce the structure and the import; reviewers reject token imports. - Full type hints; `mypy --strict` must pass. +## House rules + +- Name-keyed registries refuse silent duplicates (`ValueError` unless + `replace=True`); ordered collections append freely. +- `ParamSpec` only where a wrapper callable is returned; identity typing otherwise. +- Never `None` as a cache sentinel; immutability guards recurse into containers. + ## Frontmatter schema (the MCP server indexes this — keep it valid) ```yaml @@ -47,14 +65,15 @@ stdlib_sightings: [functools.wraps, contextlib.contextmanager] ``` Verdicts: `pythonic` = use it as shown; `use-with-care` = valid but has sharp edges -(caveats say which); `prefer-alternative` = the naive form exists for study, the -pythonic file shows what to write instead (e.g. Singleton → module global, -Visitor → singledispatch). See `docs/verdicts.md`. +(caveats say which); `prefer-alternative` = the classic form exists for study in +docs/fundamentals.md, `pattern/` exports the alternative to write instead +(e.g. Singleton → module global, Visitor → singledispatch). See `docs/verdicts.md`. ## Workflow - Branches: `main ← staging ← feat/`. PRs target `staging`. Never push to `main`. -- Gate before any PR: `make check` (ruff lint+format, mypy --strict, pytest+cov) green. +- Gate before any PR: `make check` (ruff lint+format, mypy --strict, pytest+cov, + readme-table drift) green. - Commit style: `: ` (`feat`, `fix`, `chore`, `docs`, `refactor`). - Toolchain is uv only — no pip/poetry. `make install` to set up. @@ -63,5 +82,6 @@ Visitor → singledispatch). See `docs/verdicts.md`. - Lead with the problem, not the pattern name's history. - Say plainly when Python makes the pattern unnecessary — that honesty is the point of the repo. Cite the guide chapter when one exists. -- naive.py mirrors the GoF book faithfully, even when un-Pythonic (that's its job); - pythonic.py is idiomatic; real_world.py points at real stdlib usage. +- The classic form in fundamentals.md mirrors the GoF book faithfully, even when + un-Pythonic (that's its job); `pattern/` is idiomatic; examples.md points at + real external usage. diff --git a/README.md b/README.md index 533c959..c847d70 100644 --- a/README.md +++ b/README.md @@ -1,24 +1,29 @@ # Python Design Patterns Look up any design pattern and see what a fluent Python developer would -*actually* write — the classic GoF form, the pythonic form, and where the -standard library already does it — with an honest verdict when the right -answer is "don't". All 23 Gang of Four patterns plus Python-native and -modern ones, every example typed, tested, and runnable. +*actually* write — the classic GoF form contrasted with the Python form, +importable reference code, and a mini-project that puts it to work — with an +honest verdict when the right answer is "don't". All 23 Gang of Four patterns +plus Python-native and modern ones, every unit typed, tested, and runnable. ## Use it -Each pattern is a folder — read them in this order: +Each pattern is a self-contained module: ``` patterns/structural/decorator/ -├── README.md # the problem, the trade-offs, the verdict -├── naive.py # the classic 1994 translation -├── pythonic.py # what you actually write in Python -└── real_world.py # where the stdlib already does this +├── README.md # the problem, the verdict, the map +├── pattern/ # the pattern as importable, typed code +├── docs/ # fundamentals · implementation · cited external examples +├── examples/ # runnable mini-projects that use pattern/ +└── tests/ # behavioral tests for both ``` -Run any example: `uv run python -m patterns.structural.decorator.pythonic` +```python +from patterns.structural.decorator import retry, logged +``` + +Run any mini-project: `uv run python -m patterns.structural.decorator.examples.resilient_client` Give it to your agents (MCP server with search, runnable examples, and pattern recommendations): diff --git a/docs/contributing.md b/docs/contributing.md index 691d628..6a9d26d 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -12,13 +12,20 @@ takes only reviewed milestone merges. CI (3.11/3.12/3.13) must pass. 2. Fill the frontmatter — every key; `id` must equal `/`; pick the verdict per [verdicts.md](verdicts.md). The catalog loader validates this in CI and fails loudly. -3. Write the three variants (see [how-to-read-this-repo.md](how-to-read-this-repo.md) - for what each is for) and behavioral tests for all of them. -4. `make check` — ruff, mypy --strict, pytest must all pass. +3. Build the module (see [how-to-read-this-repo.md](how-to-read-this-repo.md) + for what each part is for): `pattern/` (the importable code), the three + `docs/` files, at least one `examples//` mini-project that + genuinely imports `pattern/`, and behavioral tests for both. +4. `make check` — ruff, mypy --strict, pytest must all pass. The loader + rejects a unit missing any part of the template, and a catalog test + rejects an example that never imports its own pattern package. 5. `make readme` — regenerate the catalog table (CI rejects a stale one). ## Quality bar - Full type hints; import-safe modules (no side effects at import). -- Tests assert behavior, not "it runs". -- Prose: one page, problem-first, no UML, no history lessons. +- Tests assert behavior, not "it runs" — and load-bearing claims get the + mutation treatment (see [.github/code-review.md](../.github/code-review.md)). +- Mini-projects use realistic domains, no Foo/Bar. +- Prose: one page, problem-first, no UML, no history lessons. The classic + (GoF) form lives in `docs/fundamentals.md` as an annotated listing. diff --git a/docs/how-to-read-this-repo.md b/docs/how-to-read-this-repo.md index df48409..2a865ce 100644 --- a/docs/how-to-read-this-repo.md +++ b/docs/how-to-read-this-repo.md @@ -1,22 +1,35 @@ # How to read this repo -Every pattern lives in `patterns///` with the same five parts: +Every pattern is a self-contained module in `patterns///`: -| File | Job | +| Part | Job | |---|---| -| `README.md` | YAML frontmatter (machine-readable metadata) + one page of prose: problem → naive → pythonic → in the wild → verdict | -| `naive.py` | The Gang-of-Four/Java translation, faithfully — even where it looks silly in Python. It exists so you can diff it against `pythonic.py` and *see* what the language absorbs. | -| `pythonic.py` | What a fluent Python developer writes for the same problem. When the pattern collapses into a language feature, this file shows the collapse and names it. | -| `real_world.py` | A small program using the stdlib's own embodiment of the pattern. | -| `tests/` | Behavioral tests for all three variants. | +| `README.md` | YAML frontmatter (machine-readable metadata) + a ten-line front door: problem, verdict, map of the folders below | +| `pattern/` | The pattern itself as importable, typed library code — what you `from patterns.. import` | +| `docs/fundamentals.md` | What the pattern *is*: intent, participants, mechanism, when/when-not — including the classic (GoF/Java) form as an annotated listing, diffed against the Python form | +| `docs/implementation.md` | How to introduce the pattern into a real system: the smell, the steps, the idioms, the pitfalls | +| `docs/examples.md` | Cited external usages — stdlib, major OSS, articles — the extra resources to pull during design and review | +| `examples//` | A runnable mini-project that imports `pattern/` and puts it to work in a realistic domain (more can be added over time) | +| `tests/` | Behavioral tests for the pattern code and each mini-project | + +## Usage contract + +```python +from patterns.behavioral.chain_of_responsibility import Chain +``` + +```bash +uv run python -m patterns.behavioral.chain_of_responsibility.examples.ticket_escalation +``` ## Where to start - Reading for education: start with `principle/composition_over_inheritance`, - then any pattern whose *symptom* you recognize (the frontmatter lists them). + then any pattern whose *symptom* you recognize (the frontmatter lists them); + read `docs/fundamentals.md` first, the code second. - Solving a problem now: search the catalog through the [MCP server](mcp.md) - (`recommend_pattern`) or skim the README table's "problem it solves" column. -- Every example runs: `uv run python -m patterns...`. + (`recommend_pattern`) or skim the README table's "problem it solves" column, + then go straight to that unit's `docs/implementation.md`. ## Groups diff --git a/docs/index.md b/docs/index.md index 208cff9..c4dcea0 100644 --- a/docs/index.md +++ b/docs/index.md @@ -1,7 +1,6 @@ # Documentation -- [How to read this repo](how-to-read-this-repo.md) — the unit anatomy and where to start +- [How to read this repo](how-to-read-this-repo.md) — the module anatomy and where to start - [Verdicts](verdicts.md) — what ✅ / ⚠️ / 🔄 mean, and who decides - [MCP server](mcp.md) — connect agents to the catalog -- [Code review standards](code-review.md) — the reviewer's contract and severity checklist - [Contributing](contributing.md) — adding or improving a pattern unit diff --git a/docs/mcp.md b/docs/mcp.md index 04fd6bb..fa59d44 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -29,25 +29,23 @@ python-design-patterns-mcp --http --host 127.0.0.1 --port 8734 | Tool | What it does | |---|---| | `list_patterns(group?, verdict?)` | Catalog listing, filterable | -| `get_pattern(pattern_id, variant?)` | Full prose and metadata (`variant` served only pre-module-shape units; the current catalog has none) | +| `get_pattern(pattern_id)` | One pattern's metadata and README prose | | `search_patterns(query, limit?)` | BM25 full-text search over names, aliases, problems, symptoms, prose | | `get_pattern_docs(pattern_id, doc)` | A pattern's teaching doc: `fundamentals`, `implementation`, or `examples` | | `list_examples(pattern_id)` | A pattern's runnable mini-projects | -| `run_example(pattern_id, example=...)` | Executes a mini-project in a sandboxed subprocess; returns real stdout (`variant=` remains for pre-module-shape units only) | +| `run_example(pattern_id, example)` | Executes a mini-project in a sandboxed subprocess; returns real stdout | | `read_source(pattern_id)` | A pattern's own implementation (`pattern/` package) | | `recommend_pattern(problem_statement, limit?)` | Ranked candidates with caveats; `prefer-alternative` verdicts tell you what to write instead | Every pattern follows three access levels: scan docs (`get_pattern_docs`) → run a use case (`list_examples` + `run_example`) → -read the source (`read_source`). Units predating the module shape would -expose flat variant files instead; the current catalog has none. +read the source (`read_source`). ## Resources - `catalog://index` — the whole catalog as JSON - `pattern:///` — one pattern's prose - `pattern:////docs/` — one pattern's teaching doc -- `pattern:////` — a pre-module-shape unit's variant source (none in the current catalog) ## Prompts @@ -55,8 +53,7 @@ expose flat variant files instead; the current catalog has none. ## Sandbox contract -`run_example` executes only files resolved from the catalog index — the -`(id, variant)` / `(id, example)` pair is a dictionary lookup, never joined -into a path. The +`run_example` executes only packages resolved from the catalog index — the +`(id, example)` pair is a dictionary lookup, never joined into a path. The subprocess runs `python -I` in a temp cwd with a scrubbed environment, a 10s timeout, and 64KB output caps. There is no arbitrary-code-execution tool. diff --git a/docs/verdicts.md b/docs/verdicts.md index 2ffe7bc..958a855 100644 --- a/docs/verdicts.md +++ b/docs/verdicts.md @@ -5,9 +5,9 @@ Every unit's frontmatter carries one verdict — the catalog's honest answer to | Verdict | Meaning | |---|---| -| ✅ `pythonic` | Use it as shown in `pythonic.py`; the pattern (in its Python form) is what we'd genuinely recommend. | +| ✅ `pythonic` | Use it as shown in the unit's `pattern/` package; the pattern (in its Python form) is what we'd genuinely recommend. | | ⚠️ `use-with-care` | Legitimate uses exist, but each caveat in the frontmatter names a concrete failure mode. Read them first. | -| 🔄 `prefer-alternative` | The honest answer is usually "don't". The naive form exists for study; `pythonic.py` shows what to write instead (e.g. Singleton → module global, Visitor → `functools.singledispatch`). | +| 🔄 `prefer-alternative` | The honest answer is usually "don't". The classic form appears in `docs/fundamentals.md` for study; the unit's `pattern/` package exports the alternative to write instead (e.g. Singleton → module global, Visitor → `functools.singledispatch`). | Where [python-patterns.guide](https://python-patterns.guide/) has a chapter, its verdict wins and the unit links it. Where it doesn't, we reason from the diff --git a/pyproject.toml b/pyproject.toml index f2643ce..6646e48 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "python-design-patterns" -version = "1.0.0" +version = "2.0.0" description = "Design patterns as importable Python modules — docs, runnable mini-projects, and an MCP server for agents." readme = "README.md" license = { file = "LICENSE" } diff --git a/src/design_patterns/catalog.py b/src/design_patterns/catalog.py index 1743be5..eb69c12 100644 --- a/src/design_patterns/catalog.py +++ b/src/design_patterns/catalog.py @@ -15,14 +15,15 @@ from typing import Literal, get_args Verdict = Literal["pythonic", "use-with-care", "prefer-alternative"] -VariantName = Literal["naive", "pythonic", "real_world"] -Shape = Literal["module", "legacy"] DocName = Literal["fundamentals", "implementation", "examples"] VERDICTS: tuple[str, ...] = get_args(Verdict) -VARIANTS: tuple[str, ...] = get_args(VariantName) DOC_NAMES: tuple[str, ...] = get_args(DocName) +# Pre-v2 units shipped flat naive/pythonic/real_world files; their presence in +# a unit today is stale debris and fails validation. +_RETIRED_VARIANT_FILES = ("naive.py", "pythonic.py", "real_world.py") + _REQUIRED_KEYS = frozenset({"id", "name", "guide_url", "problem", "symptoms", "verdict", "caveats"}) @@ -54,23 +55,8 @@ def group(self) -> str: def slug(self) -> str: return self.id.split("/", 1)[1] - @property - def shape(self) -> Shape: - """``module`` units keep code in ``pattern/``; ``legacy`` units ship flat variant files. - - Any module-shape marker (``pattern/``, ``docs/``, ``examples/``) claims - the unit for strict validation, so a half-migration fails CI loudly - instead of quietly loading as legacy. - """ - markers = ("pattern", "docs", "examples") - return "module" if any((self.path / m).is_dir() for m in markers) else "legacy" - - def variants(self) -> dict[str, Path]: - """The flat example files a legacy unit ships (empty for module units).""" - return {v: self.path / f"{v}.py" for v in VARIANTS if (self.path / f"{v}.py").is_file()} - def docs(self) -> dict[str, Path]: - """The unit's teaching docs (fundamentals/implementation/examples), if present.""" + """The unit's teaching docs (fundamentals/implementation/examples).""" return { d: self.path / "docs" / f"{d}.md" for d in DOC_NAMES @@ -89,7 +75,7 @@ def examples(self) -> dict[str, Path]: } def sources(self) -> dict[str, Path]: - """The pattern's own code: ``pattern/*.py``, keyed by filename (module units).""" + """The pattern's own code: ``pattern/*.py``, keyed by filename.""" pattern_dir = self.path / "pattern" if not pattern_dir.is_dir(): return {} @@ -164,14 +150,9 @@ def _parse_pattern(readme: Path, root: Path) -> Pattern: def _validate_shape(pattern: Pattern, readme: Path) -> None: - """Module-shape units get strict structural validation; legacy units keep the old rule.""" - if pattern.shape == "legacy": - if not pattern.variants(): - raise CatalogError(f"{readme}: unit ships no naive/pythonic/real_world example") - return - + """Every unit must ship the complete module shape: pattern/ docs/ examples/ tests/.""" unit = pattern.path - if stale := sorted(pattern.variants()): + if stale := sorted(f for f in _RETIRED_VARIANT_FILES if (unit / f).is_file()): raise CatalogError(f"{readme}: module unit still ships legacy variant files: {stale}") missing_docs = [d for d in DOC_NAMES if not (unit / "docs" / f"{d}.md").is_file()] if missing_docs: @@ -209,13 +190,11 @@ def ids(self) -> tuple[str, ...]: return tuple(p.id for p in self.patterns) def to_json(self) -> str: - """The ``catalog://index`` payload: everything except prose and paths.""" + """The ``catalog://index`` payload: metadata plus docs/examples listings.""" entries = [] for p in self.patterns: entry = asdict(p) del entry["prose"], entry["path"] - entry["shape"] = p.shape - entry["variants"] = sorted(p.variants()) entry["docs"] = sorted(p.docs()) entry["examples"] = sorted(p.examples()) entries.append(entry) diff --git a/src/design_patterns/mcp/sandbox.py b/src/design_patterns/mcp/sandbox.py index bd0acf1..ffb9ffa 100644 --- a/src/design_patterns/mcp/sandbox.py +++ b/src/design_patterns/mcp/sandbox.py @@ -1,8 +1,8 @@ -"""Sandboxed execution of catalog example files -- and nothing else. +"""Sandboxed execution of catalog example packages -- and nothing else. The contract: only paths resolved from the catalog index are runnable. -The (id, variant) / (id, example) pair is looked up, never joined into a -path, so there is no traversal and no arbitrary-file execution surface. +The (id, example) pair is looked up, never joined into a path, so there is +no traversal and no arbitrary-file execution surface. """ from __future__ import annotations @@ -60,29 +60,14 @@ def _run_module(repo_root: str, module: str) -> RunResult: ) -def run_example(catalog: Catalog, pattern_id: str, variant: str) -> RunResult: - """Execute one vendored legacy example file in a subprocess and capture its output.""" - pattern = catalog.get(pattern_id) # KeyError for unknown ids -- by design - variants = pattern.variants() - if variant not in variants: - raise KeyError(f"{pattern_id} has no variant {variant!r} (has: {sorted(variants)})") - path = variants[variant] # resolved by the catalog, never by the caller - if not path.is_file(): # a real check, not an assert: survives python -O - raise FileNotFoundError(f"catalog names {path} but it does not exist") - - repo_root = pattern.path.parents[2] - module = f"patterns.{pattern.group}.{pattern.slug}.{variant}" - return _run_module(str(repo_root), module) - - def run_example_package(catalog: Catalog, pattern_id: str, example: str) -> RunResult: - """Execute a module-shape unit's ``examples/`` mini-project package.""" + """Execute a unit's ``examples/`` mini-project package in a subprocess.""" pattern = catalog.get(pattern_id) # KeyError for unknown ids -- by design examples = pattern.examples() if example not in examples: raise KeyError(f"{pattern_id} has no example {example!r} (has: {sorted(examples)})") path = examples[example] # resolved by the catalog, never by the caller - if not (path / "__main__.py").is_file(): + if not (path / "__main__.py").is_file(): # a real check, not an assert: survives python -O raise FileNotFoundError(f"catalog names {path} but it has no __main__.py") repo_root = pattern.path.parents[2] diff --git a/src/design_patterns/mcp/server.py b/src/design_patterns/mcp/server.py index eec4a34..95aa70f 100644 --- a/src/design_patterns/mcp/server.py +++ b/src/design_patterns/mcp/server.py @@ -11,10 +11,9 @@ from typing import Any from mcp.server import MCPServer -from mcp.server.mcpserver.exceptions import ResourceError, ToolError +from mcp.server.mcpserver.exceptions import ResourceError from design_patterns.catalog import DOC_NAMES, Catalog, Pattern, load_catalog -from design_patterns.mcp.sandbox import run_example as _run_example from design_patterns.mcp.sandbox import run_example_package as _run_example_package from design_patterns.mcp.search import SearchIndex @@ -40,10 +39,8 @@ def get_index() -> SearchIndex: "'prefer-alternative' tell you what to write instead. Every unit " "offers three access levels: get_pattern_docs " "(fundamentals/implementation/examples), then list_examples + " - "run_example(example=...) for runnable mini-projects, then read_source " - "(the pattern/ package). Units predating the module shape would ship " - "flat variant files via get_pattern(variant=...) and " - "run_example(variant=...); the current catalog has none." + "run_example for runnable mini-projects, then read_source " + "(the pattern/ package)." ), ) @@ -57,32 +54,18 @@ def _summary(pattern: Pattern) -> dict[str, Any]: } -def _detail(pattern: Pattern, include_source: str | None) -> dict[str, Any]: - detail: dict[str, Any] = { +def _detail(pattern: Pattern) -> dict[str, Any]: + return { **_summary(pattern), - "shape": pattern.shape, "aliases": list(pattern.aliases), "guide_url": pattern.guide_url, "symptoms": list(pattern.symptoms), "caveats": list(pattern.caveats), "stdlib_sightings": list(pattern.stdlib_sightings), - "variants": sorted(pattern.variants()), + "docs": sorted(pattern.docs()), + "examples": sorted(pattern.examples()), "prose": pattern.prose, } - if pattern.shape == "module": - detail["docs"] = sorted(pattern.docs()) - detail["examples"] = sorted(pattern.examples()) - detail["note"] = ( - "module-shape unit: variants are legacy-only. Read docs via " - "get_pattern_docs, run mini-projects via list_examples + " - "run_example(example=...), read code via read_source." - ) - return detail - if include_source: - variants = pattern.variants() - wanted = sorted(variants) if include_source == "all" else [include_source] - detail["source"] = {name: variants[name].read_text() for name in wanted if name in variants} - return detail @mcp.tool() @@ -99,12 +82,11 @@ def list_patterns(group: str | None = None, verdict: str | None = None) -> list[ @mcp.tool() -def get_pattern(pattern_id: str, variant: str | None = None) -> dict[str, Any]: - """Fetch one pattern's full documentation. pattern_id is '/' - (e.g. 'structural/decorator'). For legacy units, variant ('naive', - 'pythonic', 'real_world', or 'all') includes that flat file's source; - module-shape units serve code via read_source instead.""" - return _detail(_get(pattern_id), variant) +def get_pattern(pattern_id: str) -> dict[str, Any]: + """Fetch one pattern's metadata and README prose. pattern_id is + '/' (e.g. 'structural/decorator'). Teaching docs come from + get_pattern_docs; code comes from read_source.""" + return _detail(_get(pattern_id)) @mcp.tool() @@ -122,34 +104,12 @@ def _get(pattern_id: str) -> Pattern: raise ValueError(f"unknown pattern {pattern_id!r}; known ids: {known}") from None -def _require_module_shape(pattern: Pattern) -> Pattern: - if pattern.shape != "module": - raise ToolError( - f"{pattern.id} is not yet migrated to the module shape " - "(no pattern/, docs/, examples/); use get_pattern with a variant instead" - ) - return pattern - - @mcp.tool() -def run_example( - pattern_id: str, variant: str | None = None, example: str | None = None -) -> dict[str, Any]: - """Execute one of a pattern's vendored examples in a sandboxed subprocess - and return its real output. For migrated (module-shape) patterns pass - example=; for legacy patterns pass - variant='naive'|'pythonic'|'real_world'. Exactly one of the two.""" - # Legacy-shape support (the variant= arm below and its relatives) is kept - # deliberately: every real unit is module-shape now, but the loader - # contract still admits legacy units and the synthetic test fixtures - # exercise these paths. Removal is a wrap-phase decision, tracked there. - if (variant is None) == (example is None): - raise ValueError("pass exactly one of 'variant' (legacy) or 'example' (module-shape)") - if example is not None: - result = _run_example_package(get_catalog(), pattern_id, example) - else: - assert variant is not None - result = _run_example(get_catalog(), pattern_id, variant) +def run_example(pattern_id: str, example: str) -> dict[str, Any]: + """Execute one of a pattern's mini-projects (example=) in a sandboxed subprocess and return its real output.""" + _get(pattern_id) # helpful unknown-id error before the sandbox's KeyError + result = _run_example_package(get_catalog(), pattern_id, example) return { "exit_code": result.exit_code, "stdout": result.stdout, @@ -160,10 +120,10 @@ def run_example( @mcp.tool() def get_pattern_docs(pattern_id: str, doc: str) -> str: - """Read one of a migrated pattern's teaching docs: 'fundamentals' (intent, + """Read one of a pattern's teaching docs: 'fundamentals' (intent, participants, mechanism, classic-form contrast), 'implementation' (how to introduce it into a real system), or 'examples' (cited external usages).""" - pattern = _require_module_shape(_get(pattern_id)) + pattern = _get(pattern_id) docs = pattern.docs() if doc not in docs: raise ValueError(f"doc must be one of {sorted(DOC_NAMES)}; {pattern_id} has {sorted(docs)}") @@ -172,9 +132,9 @@ def get_pattern_docs(pattern_id: str, doc: str) -> str: @mcp.tool() def list_examples(pattern_id: str) -> list[dict[str, Any]]: - """List a migrated pattern's runnable mini-projects (examples/); + """List a pattern's runnable mini-projects (examples/); run one with run_example(pattern_id, example=).""" - pattern = _require_module_shape(_get(pattern_id)) + pattern = _get(pattern_id) return [ { "name": name, @@ -187,9 +147,9 @@ def list_examples(pattern_id: str) -> list[dict[str, Any]]: @mcp.tool() def read_source(pattern_id: str) -> dict[str, str]: - """Read a migrated pattern's own implementation: every file in its - pattern/ package, keyed by filename.""" - pattern = _require_module_shape(_get(pattern_id)) + """Read a pattern's own implementation: every file in its pattern/ + package, keyed by filename.""" + pattern = _get(pattern_id) return {name: path.read_text(encoding="utf-8") for name, path in pattern.sources().items()} @@ -197,7 +157,7 @@ def read_source(pattern_id: str) -> dict[str, str]: def recommend_pattern(problem_statement: str, limit: int = 3) -> list[dict[str, Any]]: """Describe a design problem in plain words; get ranked candidate patterns, each with its caveats and verdict attached. A 'prefer-alternative' verdict - means the pythonic variant shows what to write instead.""" + means the unit's docs show what to write instead.""" recommendations = [] for hit in get_index().search(problem_statement, limit): p = hit.pattern @@ -208,14 +168,10 @@ def recommend_pattern(problem_statement: str, limit: int = 3) -> list[dict[str, "stdlib_sightings": list(p.stdlib_sightings), } if p.verdict == "prefer-alternative": - where = ( - f"read get_pattern_docs({p.id!r}, 'fundamentals') and read_source({p.id!r})" - if p.shape == "module" - else "see this unit's pythonic.py" - ) rec["note"] = ( f"The guide's honest answer is usually not {p.name}: " - f"{where} for what to write instead." + f"read get_pattern_docs({p.id!r}, 'fundamentals') and " + f"read_source({p.id!r}) for what to write instead." ) recommendations.append(rec) return recommendations @@ -223,7 +179,7 @@ def recommend_pattern(problem_statement: str, limit: int = 3) -> list[dict[str, @mcp.resource("catalog://index") def catalog_index() -> str: - """The whole catalog as JSON: every pattern's metadata and variants.""" + """The whole catalog as JSON: every pattern's metadata, docs, and examples.""" return get_catalog().to_json() @@ -233,33 +189,13 @@ def pattern_doc(group: str, slug: str) -> str: return _get(f"{group}/{slug}").prose -@mcp.resource("pattern://{group}/{slug}/{variant}") -def pattern_source(group: str, slug: str, variant: str) -> str: - """One legacy pattern's example source (naive | pythonic | real_world).""" - pattern = _get(f"{group}/{slug}") - variants = pattern.variants() - if variant not in variants: - hint = ( - "module-shape unit: use pattern:///docs/ or the read_source tool" - if pattern.shape == "module" - else f"has: {sorted(variants)}" - ) - raise ResourceError(f"{pattern.id} has no variant {variant!r} ({hint})") - return variants[variant].read_text() - - @mcp.resource("pattern://{group}/{slug}/docs/{doc}") def pattern_docs_resource(group: str, slug: str, doc: str) -> str: - """One migrated pattern's teaching doc (fundamentals | implementation | examples).""" + """One pattern's teaching doc (fundamentals | implementation | examples).""" pattern = _get(f"{group}/{slug}") docs = pattern.docs() if doc not in docs: - hint = ( - f"has: {sorted(docs)}" - if pattern.shape == "module" - else "unit not yet migrated to the module shape; use get_pattern instead" - ) - raise ResourceError(f"{pattern.id} has no doc {doc!r} ({hint})") + raise ResourceError(f"{pattern.id} has no doc {doc!r} (has: {sorted(docs)})") return docs[doc].read_text(encoding="utf-8") @@ -270,8 +206,6 @@ def refactor_toward(pattern_id: str, code: str) -> str: caveats = "\n".join(f"- {c}" for c in pattern.caveats) reference = ( f"read_source({pattern.id!r}) and get_pattern_docs({pattern.id!r}, 'implementation')" - if pattern.shape == "module" - else "this unit's pythonic.py file" ) return ( f"Refactor the following code toward the {pattern.name} pattern " @@ -287,8 +221,6 @@ def explain_pattern(pattern_id: str, audience: str = "an intermediate Python dev pattern = _get(pattern_id) contrast = ( f"the classic-form vs Python contrast in get_pattern_docs({pattern.id!r}, 'fundamentals')" - if pattern.shape == "module" - else "the contrast between this unit's classic-form and pythonic example files" ) return ( f"Explain the {pattern.name} pattern to {audience}. Problem it solves: " diff --git a/tests/conftest.py b/tests/conftest.py index cabd0df..937173f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,8 +1,7 @@ -"""Shared fixtures: a synthetic module-shape unit for engine tests. +"""Shared fixtures: a synthetic pattern unit for engine tests. -The real catalog's module-shape pilot is being built alongside this code, so -engine tests use a self-contained synthetic unit instead of assuming any real -unit's API. +Engine tests use a self-contained synthetic unit instead of assuming any real +unit's API, so unit-level refactors never ripple into the engine suite. """ from pathlib import Path @@ -43,47 +42,9 @@ def write_module_unit(root: Path) -> Path: return unit -def write_legacy_unit(root: Path) -> Path: - """Build ``/creational/oldthing`` as a pre-migration legacy-shape unit.""" - unit = root / "creational" / "oldthing" - unit.mkdir(parents=True) - for pkg in (root, root / "creational", unit): - init = pkg / "__init__.py" - if not init.exists(): - init.write_text("") - (unit / "README.md").write_text( - "---\n" - "id: creational/oldthing\nname: Oldthing\nguide_url: null\n" - 'problem: "Build an old thing."\nsymptoms: ["old thing needed"]\n' - "verdict: prefer-alternative\ncaveats: []\n" - "---\n\n# Oldthing\n" - ) - for variant in ("naive", "pythonic", "real_world"): - (unit / f"{variant}.py").write_text( - f'def main() -> None:\n print("{variant} oldthing runs")\n\n\n' - 'if __name__ == "__main__":\n main()\n' - ) - tests = unit / "tests" - tests.mkdir() - (tests / "test_oldthing.py").write_text("def test_ok() -> None:\n assert True\n") - return unit - - @pytest.fixture def module_catalog(tmp_path: Path) -> Catalog: """A catalog whose ``patterns/`` root holds one synthetic module-shape unit.""" root = tmp_path / "patterns" write_module_unit(root) return load_catalog(root) - - -@pytest.fixture -def legacy_catalog(tmp_path: Path) -> Catalog: - """A catalog holding one synthetic legacy-shape unit. - - Real units migrate to the module shape group by group, so tests of the - legacy behavior must not depend on any real unit staying legacy. - """ - root = tmp_path / "patterns" - write_legacy_unit(root) - return load_catalog(root) diff --git a/tests/test_catalog.py b/tests/test_catalog.py index 84c3306..c5684b5 100644 --- a/tests/test_catalog.py +++ b/tests/test_catalog.py @@ -7,7 +7,6 @@ from design_patterns.catalog import ( VERDICTS, - Catalog, CatalogError, find_patterns_root, load_catalog, @@ -20,21 +19,6 @@ def test_loads_all_units(self) -> None: assert len(catalog.patterns) == 32 assert "structural/decorator" in catalog.ids() - def test_real_catalog_is_fully_module_shape(self) -> None: - # The migration is complete: every real unit is module-shape. A unit - # regressing to legacy shape must fail here, not silently downgrade. - shapes = {p.shape for p in load_catalog().patterns} - assert shapes == {"module"} - - def test_legacy_loader_branch_stays_covered_by_the_synthetic_unit( - self, legacy_catalog: Catalog - ) -> None: - # No real unit is legacy any more; this pins that the loader's legacy - # branch (and the tests that rely on it) still have a living subject. - (pattern,) = legacy_catalog.patterns - assert pattern.shape == "legacy" - assert sorted(pattern.variants()) == ["naive", "pythonic", "real_world"] - def test_every_module_example_builds_on_its_own_pattern_package(self) -> None: # The mini-projects exist to show the pattern in practice: each one # must import its unit's pattern/ package, not reimplement the idea. @@ -43,8 +27,6 @@ def test_every_module_example_builds_on_its_own_pattern_package(self) -> None: import ast for pattern in load_catalog().patterns: - if pattern.shape != "module": - continue group, slug = pattern.id.split("/") absolute = f"patterns.{group}.{slug}.pattern" for name, path in pattern.examples().items(): @@ -69,16 +51,13 @@ def test_every_module_example_builds_on_its_own_pattern_package(self) -> None: f"{pattern.id} example {name!r} never imports its own pattern package" ) - def test_every_unit_ships_its_shape_completely(self) -> None: + def test_every_unit_ships_the_full_module_shape(self) -> None: for pattern in load_catalog().patterns: - if pattern.shape == "module": - assert sorted(pattern.docs()) == ["examples", "fundamentals", "implementation"], ( - pattern.id - ) - assert pattern.examples(), pattern.id - assert pattern.sources(), pattern.id - else: - assert sorted(pattern.variants()) == ["naive", "pythonic", "real_world"], pattern.id + assert sorted(pattern.docs()) == ["examples", "fundamentals", "implementation"], ( + pattern.id + ) + assert pattern.examples(), pattern.id + assert pattern.sources(), pattern.id def test_verdicts_are_from_the_vocabulary(self) -> None: for pattern in load_catalog().patterns: @@ -96,17 +75,11 @@ def test_get_unknown_id_raises(self) -> None: def test_index_json_round_trips(self) -> None: entries = json.loads(load_catalog().to_json()) assert len(entries) == 32 - assert all({"id", "name", "problem", "verdict", "variants"} <= e.keys() for e in entries) + keys = {"id", "name", "problem", "verdict", "docs", "examples"} + assert all(keys <= e.keys() for e in entries) assert not any("prose" in e or "path" in e for e in entries) -def _write_unit(root: Path, group: str, slug: str, frontmatter: str, body: str = "# x") -> None: - unit = root / group / slug - unit.mkdir(parents=True) - (unit / "README.md").write_text(f"---\n{frontmatter}\n---\n\n{body}\n") - (unit / "pythonic.py").write_text("def main() -> None: ...\n") - - GOOD = """\ id: creational/thing name: Thing @@ -119,22 +92,24 @@ def _write_unit(root: Path, group: str, slug: str, frontmatter: str, body: str = class TestValidation: def test_minimal_valid_unit_loads(self, tmp_path: Path) -> None: - _write_unit(tmp_path, "creational", "thing", GOOD) + _write_module_unit(tmp_path, "creational", "thing", GOOD) catalog = load_catalog(tmp_path) assert catalog.get("creational/thing").verdict == "pythonic" def test_missing_key_fails(self, tmp_path: Path) -> None: - _write_unit(tmp_path, "creational", "thing", GOOD.replace('problem: "Build a thing."', "")) + _write_module_unit( + tmp_path, "creational", "thing", GOOD.replace('problem: "Build a thing."', "") + ) with pytest.raises(CatalogError, match="missing frontmatter keys"): load_catalog(tmp_path) def test_id_directory_mismatch_fails(self, tmp_path: Path) -> None: - _write_unit(tmp_path, "creational", "other", GOOD) + _write_module_unit(tmp_path, "creational", "other", GOOD) with pytest.raises(CatalogError, match="!= directory"): load_catalog(tmp_path) def test_unknown_verdict_fails(self, tmp_path: Path) -> None: - _write_unit(tmp_path, "creational", "thing", GOOD.replace("pythonic", "amazing")) + _write_module_unit(tmp_path, "creational", "thing", GOOD.replace("pythonic", "amazing")) with pytest.raises(CatalogError, match="verdict"): load_catalog(tmp_path) @@ -145,12 +120,6 @@ def test_no_frontmatter_fails(self, tmp_path: Path) -> None: with pytest.raises(CatalogError, match="frontmatter"): load_catalog(tmp_path) - def test_unit_without_examples_fails(self, tmp_path: Path) -> None: - _write_unit(tmp_path, "creational", "thing", GOOD) - (tmp_path / "creational" / "thing" / "pythonic.py").unlink() - with pytest.raises(CatalogError, match="ships no"): - load_catalog(tmp_path) - def test_empty_tree_fails(self, tmp_path: Path) -> None: with pytest.raises(CatalogError, match="no pattern units"): load_catalog(tmp_path) @@ -183,11 +152,9 @@ class TestModuleShapeValidation: def test_valid_module_unit_loads_with_shape_fields(self, tmp_path: Path) -> None: _write_module_unit(tmp_path, "creational", "thing", GOOD) pattern = load_catalog(tmp_path).get("creational/thing") - assert pattern.shape == "module" assert sorted(pattern.docs()) == ["examples", "fundamentals", "implementation"] assert sorted(pattern.examples()) == ["demo"] assert sorted(pattern.sources()) == ["__init__.py", "thing.py"] - assert pattern.variants() == {} def test_missing_doc_fails(self, tmp_path: Path) -> None: unit = _write_module_unit(tmp_path, "creational", "thing", GOOD) @@ -225,9 +192,8 @@ def test_stale_legacy_variant_files_fail(self, tmp_path: Path) -> None: with pytest.raises(CatalogError, match="legacy variant files"): load_catalog(tmp_path) - def test_half_migration_is_claimed_and_fails_loudly(self, tmp_path: Path) -> None: - # docs/ alone marks the unit module-shape; strict validation then - # demands the rest instead of silently classifying it legacy. + def test_partial_unit_fails_loudly(self, tmp_path: Path) -> None: + # A unit with only some of the template present must fail validation. unit = tmp_path / "creational" / "thing" (unit / "docs").mkdir(parents=True) (unit / "README.md").write_text(f"---\n{GOOD}\n---\n\n# x\n") @@ -240,10 +206,9 @@ def test_examples_dir_needs_init(self, tmp_path: Path) -> None: with pytest.raises(CatalogError, match=r"examples/ is not a package"): load_catalog(tmp_path) - def test_index_json_carries_shape(self, tmp_path: Path) -> None: + def test_index_json_carries_docs_and_examples(self, tmp_path: Path) -> None: _write_module_unit(tmp_path, "creational", "thing", GOOD) entries = json.loads(load_catalog(tmp_path).to_json()) - assert entries[0]["shape"] == "module" assert entries[0]["examples"] == ["demo"] assert entries[0]["docs"] == ["examples", "fundamentals", "implementation"] diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 541036e..28d4504 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -27,20 +27,15 @@ async def test_list_patterns_filters(self) -> None: ids = [p["id"] for p in result.structured_content["result"]] assert len(ids) == 5 and all(i.startswith("creational/") for i in ids) - async def test_get_pattern_with_source( - self, legacy_catalog: Catalog, monkeypatch: pytest.MonkeyPatch - ) -> None: - # Variant source is a legacy-shape feature; every real unit migrates, - # so this runs against the synthetic legacy unit. - monkeypatch.setattr(server_module, "get_catalog", lambda: legacy_catalog) + async def test_get_pattern_lists_docs_and_examples(self) -> None: async with Client(mcp) as client: - result = await client.call_tool( - "get_pattern", {"pattern_id": "creational/oldthing", "variant": "pythonic"} - ) + result = await client.call_tool("get_pattern", {"pattern_id": "structural/decorator"}) assert result.structured_content is not None detail = result.structured_content - assert detail["verdict"] == "prefer-alternative" - assert "pythonic oldthing runs" in detail["source"]["pythonic"] + assert detail["verdict"] == "pythonic" + assert detail["docs"] == ["examples", "fundamentals", "implementation"] + assert "resilient_client" in detail["examples"] + assert detail["prose"] async def test_get_pattern_unknown_id_names_the_catalog(self) -> None: async with Client(mcp) as client: @@ -85,23 +80,16 @@ async def test_recommend_attaches_caveats(self) -> None: assert singleton["caveats"] -class TestRecommendNoteByShape: - """The prefer-alternative note is pinned per shape, via synthetic units.""" +class TestRecommendNote: + """The prefer-alternative note is pinned via the synthetic unit.""" - @staticmethod - def _point_at( - catalog: Catalog, - monkeypatch: pytest.MonkeyPatch, + async def test_note_names_the_docs_and_source_tools( + self, module_catalog: Catalog, monkeypatch: pytest.MonkeyPatch ) -> None: from design_patterns.mcp.search import SearchIndex - monkeypatch.setattr(server_module, "get_catalog", lambda: catalog) - monkeypatch.setattr(server_module, "get_index", lambda: SearchIndex(catalog)) - - async def test_module_unit_note_names_the_module_tools( - self, module_catalog: Catalog, monkeypatch: pytest.MonkeyPatch - ) -> None: - self._point_at(module_catalog, monkeypatch) + monkeypatch.setattr(server_module, "get_catalog", lambda: module_catalog) + monkeypatch.setattr(server_module, "get_index", lambda: SearchIndex(module_catalog)) async with Client(mcp) as client: result = await client.call_tool( "recommend_pattern", {"problem_statement": "thing needed"} @@ -111,22 +99,9 @@ async def test_module_unit_note_names_the_module_tools( assert rec["id"] == "creational/thing" assert "get_pattern_docs" in rec["note"] and "read_source" in rec["note"] - async def test_legacy_unit_note_points_at_pythonic_file( - self, legacy_catalog: Catalog, monkeypatch: pytest.MonkeyPatch - ) -> None: - self._point_at(legacy_catalog, monkeypatch) - async with Client(mcp) as client: - result = await client.call_tool( - "recommend_pattern", {"problem_statement": "old thing needed"} - ) - assert result.structured_content is not None - rec = result.structured_content["result"][0] - assert rec["id"] == "creational/oldthing" - assert "pythonic.py" in rec["note"] - class TestModuleShapeTools: - """The three new access levels, against a synthetic migrated unit.""" + """The three access levels, against the synthetic unit.""" @pytest.fixture(autouse=True) def _use_module_catalog(self, module_catalog: Catalog, monkeypatch: pytest.MonkeyPatch) -> None: @@ -164,11 +139,6 @@ async def test_run_example_by_package(self) -> None: run = result.structured_content assert run["exit_code"] == 0 and "built a thing" in run["stdout"] - async def test_run_example_requires_exactly_one_selector(self) -> None: - async with Client(mcp) as client: - result = await client.call_tool("run_example", {"pattern_id": "creational/thing"}) - assert result.is_error - async def test_read_source_returns_pattern_package(self) -> None: async with Client(mcp) as client: result = await client.call_tool("read_source", {"pattern_id": "creational/thing"}) @@ -183,59 +153,12 @@ async def test_docs_resource(self) -> None: assert isinstance(first, TextResourceContents) assert "implementation of Thing" in first.text - async def test_variant_resource_error_text_reaches_client(self) -> None: - # A module-shape unit refusing a legacy variant read must explain itself. - async with Client(mcp) as client: - with pytest.raises(Exception, match="module-shape unit"): - await client.read_resource("pattern://creational/thing/naive") - - -class TestLegacyShapeErrors: - """Module-shape tools refuse un-migrated units with a clear message, not a crash. - - Uses a synthetic legacy unit: every real unit migrates to the module shape, - so no real id can be relied on to stay legacy. - """ - - @pytest.fixture(autouse=True) - def _use_legacy_catalog(self, legacy_catalog: Catalog, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(server_module, "get_catalog", lambda: legacy_catalog) - - async def test_get_pattern_docs_on_legacy_unit(self) -> None: - async with Client(mcp) as client: - result = await client.call_tool( - "get_pattern_docs", {"pattern_id": "creational/oldthing", "doc": "fundamentals"} - ) - assert result.is_error - assert "not yet migrated" in str(result.content[0]) - - async def test_list_examples_on_legacy_unit(self) -> None: - async with Client(mcp) as client: - result = await client.call_tool("list_examples", {"pattern_id": "creational/oldthing"}) - assert result.is_error - - async def test_read_source_on_legacy_unit(self) -> None: - async with Client(mcp) as client: - result = await client.call_tool("read_source", {"pattern_id": "creational/oldthing"}) - assert result.is_error - - async def test_run_example_legacy_variant_dispatch(self) -> None: - # The variant= arm is what every un-migrated unit still relies on. - async with Client(mcp) as client: - result = await client.call_tool( - "run_example", {"pattern_id": "creational/oldthing", "variant": "pythonic"} - ) - assert result.structured_content is not None - run = result.structured_content - assert run["exit_code"] == 0 and not run["timed_out"] - assert "pythonic oldthing runs" in run["stdout"] - - async def test_docs_resource_error_text_reaches_client(self) -> None: + async def test_unknown_doc_resource_error_text_reaches_client(self) -> None: # ResourceError (not ValueError) is required for the hint to survive # the SDK's template wrapper — this pins that the text gets through. async with Client(mcp) as client: - with pytest.raises(Exception, match="not yet migrated"): - await client.read_resource("pattern://creational/oldthing/docs/fundamentals") + with pytest.raises(Exception, match="has no doc"): + await client.read_resource("pattern://creational/thing/docs/naive") class TestResources: @@ -258,16 +181,6 @@ async def test_pattern_doc_and_docs_templates(self) -> None: assert isinstance(first_fund, TextResourceContents) assert "# Iterator — fundamentals" in first_fund.text - async def test_legacy_variant_source_template( - self, legacy_catalog: Catalog, monkeypatch: pytest.MonkeyPatch - ) -> None: - monkeypatch.setattr(server_module, "get_catalog", lambda: legacy_catalog) - async with Client(mcp) as client: - src = await client.read_resource("pattern://creational/oldthing/pythonic") - first_src = src.contents[0] - assert isinstance(first_src, TextResourceContents) - assert "pythonic oldthing runs" in first_src.text - class TestPrompts: async def test_prompts_are_listed_and_render(self) -> None: diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index 5ca3246..62f97c1 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -1,43 +1,16 @@ -"""Sandbox contract: only catalog files run; timeouts and bad ids refuse.""" +"""Sandbox contract: only catalog example packages run; bad ids and names refuse.""" + +from pathlib import Path import pytest +from tests.conftest import write_module_unit from design_patterns.catalog import Catalog, load_catalog -from design_patterns.mcp.sandbox import run_example, run_example_package +from design_patterns.mcp.sandbox import run_example_package CATALOG = load_catalog() -class TestSandbox: - def test_runs_a_legacy_variant(self, legacy_catalog: Catalog) -> None: - # Legacy variants are a synthetic-fixture concern: every real unit - # migrates to the module shape. - result = run_example(legacy_catalog, "creational/oldthing", "pythonic") - assert result.exit_code == 0 - assert "pythonic oldthing runs" in result.stdout - assert not result.timed_out - - def test_unknown_pattern_id_is_refused(self, legacy_catalog: Catalog) -> None: - with pytest.raises(KeyError): - run_example(legacy_catalog, "../../etc/passwd", "naive") - - def test_unknown_variant_is_refused(self, legacy_catalog: Catalog) -> None: - # Against a unit that HAS variants, so the refusal is a real selection - # miss, not the empty-variants degenerate case. - with pytest.raises(KeyError, match="no variant"): - run_example(legacy_catalog, "creational/oldthing", "__init__") - - def test_traversal_shaped_variant_is_refused(self, legacy_catalog: Catalog) -> None: - with pytest.raises(KeyError): - run_example(legacy_catalog, "creational/oldthing", "../../../tmp/evil") - - def test_failing_example_reports_not_raises(self, legacy_catalog: Catalog) -> None: - # every current example exits 0; simulate by checking the API shape - result = run_example(legacy_catalog, "creational/oldthing", "real_world") - assert isinstance(result.exit_code, int) - assert isinstance(result.stderr, str) - - class TestPackageSandbox: def test_runs_a_module_example_that_imports_the_pattern(self, module_catalog: Catalog) -> None: result = run_example_package(module_catalog, "creational/thing", "demo") @@ -57,9 +30,17 @@ def test_unknown_pattern_id_is_refused(self, module_catalog: Catalog) -> None: with pytest.raises(KeyError): run_example_package(module_catalog, "../../etc/passwd", "demo") - def test_legacy_unit_has_no_packages(self, legacy_catalog: Catalog) -> None: - with pytest.raises(KeyError, match="no example"): - run_example_package(legacy_catalog, "creational/oldthing", "pythonic") + def test_failing_example_reports_not_raises(self, tmp_path: Path) -> None: + # A crashing demo must come back as a RunResult, not an exception. + root = tmp_path / "patterns" + unit = write_module_unit(root) + (unit / "examples" / "demo" / "__main__.py").write_text( + "import sys\n\nprint('about to fail')\nsys.exit(3)\n" + ) + result = run_example_package(load_catalog(root), "creational/thing", "demo") + assert result.exit_code == 3 + assert "about to fail" in result.stdout + assert not result.timed_out def test_runs_the_real_pilot_unit(self) -> None: # The migrated unit itself, through the python -I -m path CI must cover. @@ -71,19 +52,18 @@ def test_runs_the_real_pilot_unit(self) -> None: assert not result.timed_out -def _every_module_example() -> list[tuple[str, str]]: +def _every_example() -> list[tuple[str, str]]: return [ (pattern.id, example) for pattern in CATALOG.patterns - if pattern.shape == "module" for example in sorted(pattern.examples()) ] class TestEveryExampleRuns: - """Demo rot check: every module unit's every example runs in the sandbox.""" + """Demo rot check: every unit's every example runs in the sandbox.""" - @pytest.mark.parametrize(("pattern_id", "example"), _every_module_example()) + @pytest.mark.parametrize(("pattern_id", "example"), _every_example()) def test_example_exits_cleanly(self, pattern_id: str, example: str) -> None: result = run_example_package(CATALOG, pattern_id, example) assert result.exit_code == 0, f"{pattern_id}/{example}: {result.stderr}"