milestone: richer examples + review-pass fixes

docs/code-review.md

@@ -0,0 +1,61 @@
+# Code review standards
+
+The reviewer's contract for this repo — and a reusable checklist for any
+Python team.
+
+## Layer 0: machines argue about style, humans argue about design
+
+These run in CI; a human review comment about anything they cover is wasted:
+
+| Tool | Standard it enforces |
+|---|---|
+| `ruff check` + `ruff format` | PEP 8, import order, bugbear/simplify/pyupgrade rule packs — each rule documented at [docs.astral.sh/ruff/rules](https://docs.astral.sh/ruff/rules/) |
+| `mypy --strict` | PEP 484 typing, no untyped defs, no implicit Any |
+| `pytest` + coverage | behavior, not just "it imports" |
+| `python -m design_patterns.readme_table --check` | docs can't drift from code |
+
+Worth adding for security-sensitive work: `bandit` (SAST) and `pip-audit`
+(dependency CVEs). Note: bandit flags every `assert` (B101) — in pytest
+tests that's idiomatic, not a finding.
+
+## Layer 1: the written standards behind the tools
+
+- **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)
+
+## Layer 2: what human reviewers actually check
+
+Severity-ordered — block on CRITICAL/HIGH, note MEDIUM:
+
+**CRITICAL**
+- Injection: user input reaching `eval`/`exec`, SQL strings, `subprocess` with `shell=True`
+- Unsafe deserialization: `pickle.loads`/`yaml.load` on data crossing a trust boundary
+- Secrets in code
+
+**HIGH**
+- `assert` as a runtime guard (vanishes under `python -O`)
+- Mutable default arguments; shared mutable module state
+- Swallowed exceptions (`except: pass`), or `except Exception` hiding real errors
+- Resources without context managers; missing cleanup on the error path
+- Thread-safety claims the code doesn't earn (unguarded lazy init, shared caches)
+- Unbounded recursion/loops on user-controlled input
+
+**MEDIUM**
+- Work at import time (I/O, big computation) — see `patterns/python/global_object`
+- `isinstance` traps (`bool` passes `int` checks), `is` vs `==` on sentinels
+- API honesty: docstrings/comments that promise more than the code delivers
+- A design pattern where a language feature suffices — check the catalog's verdict first
+
+## Reference sources for reviewers (MCP)
+
+- **This repo's own MCP server** — `claude mcp add design-patterns -- uv run --directory <repo> python-design-patterns-mcp`. `recommend_pattern` answers "should this be a Singleton?" with python-patterns.guide's verdicts and caveats; `get_pattern` serves the reference implementation to compare against.
+- **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.
+
+## Review etiquette
+
+- Cite the rule or the file, not taste ("B008: mutable default" beats "I don't like this").
+- One approval pass = one severity sweep top-down; don't drip-feed.
+- The author of a change never approves it.

docs/index.md

@@ -3,4 +3,5 @@
 - [How to read this repo](how-to-read-this-repo.md) — the unit 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

patterns/behavioral/interpreter/real_world.py

@@ -18,18 +18,31 @@
 }
 
 
-def safe_eval(formula: str) -> float:
-    """Evaluate arithmetic like '2 * (3 + 4)'; reject everything else."""
-    return _walk(ast.parse(formula, mode="eval").body)
+#: Deeper than any human formula; shallower than the recursion limit, so a
+#: hostile input gets a clean ValueError instead of a RecursionError crash.
+MAX_DEPTH = 50
 
 
-def _walk(node: ast.expr) -> float:
-    if isinstance(node, ast.Constant) and isinstance(node.value, int | float):
+def safe_eval(formula: str) -> float:
+    """Evaluate arithmetic like '2 * (3 + 4)'; reject everything else."""
+    return _walk(ast.parse(formula, mode="eval").body, depth=0)
+
+
+def _walk(node: ast.expr, depth: int) -> float:
+    if depth > MAX_DEPTH:
+        raise ValueError("expression too deeply nested")
+    if (
+        isinstance(node, ast.Constant)
+        and isinstance(node.value, int | float)
+        and not isinstance(node.value, bool)
+        # bool subclasses int, and a *safe* evaluator should not quietly
+        # compute True + 1 -- so it is excluded explicitly.
+    ):
         return float(node.value)
     if isinstance(node, ast.BinOp) and type(node.op) in _BINOPS:
-        return _BINOPS[type(node.op)](_walk(node.left), _walk(node.right))
+        return _BINOPS[type(node.op)](_walk(node.left, depth + 1), _walk(node.right, depth + 1))
     if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.USub):
-        return -_walk(node.operand)
+        return -_walk(node.operand, depth + 1)
     raise ValueError(f"disallowed syntax: {ast.dump(node)[:40]}")
 
 

patterns/behavioral/interpreter/tests/test_interpreter.py

@@ -34,3 +34,13 @@ def test_attack_is_rejected_not_executed(self) -> None:
     def test_names_are_rejected(self) -> None:
         with pytest.raises(ValueError):
             real_world.safe_eval("x + 1")
+
+    def test_bool_constants_are_rejected(self) -> None:
+        # bool subclasses int; a safe evaluator must not compute True + 1.
+        with pytest.raises(ValueError, match="disallowed"):
+            real_world.safe_eval("True + 1")
+
+    def test_hostile_nesting_gets_a_clean_error_not_a_crash(self) -> None:
+        bomb = "1" + " + 1" * 200  # deeper than MAX_DEPTH
+        with pytest.raises(ValueError, match="deeply nested"):
+            real_world.safe_eval(bomb)

patterns/behavioral/mediator/README.md

@@ -29,8 +29,10 @@ and *only* the mediator decides who reacts.
 ## Pythonic solution
 
 The mediator doesn't need a Colleague base class — widgets accept a
-`notify` callable, and the mediator is a small coordinator holding the
-interaction rules in one readable place.
+`notify` callable and hold zero rules. `pythonic.py` scales the idea to a
+checkout form whose rules genuinely tangle (country restricts shipping,
+shipping gates payment and changes the total): one `_recheck` method holds
+every rule, and a country change cascades through the dependent fields.
 
 ## In the wild
 

patterns/behavioral/mediator/pythonic.py

@@ -1,43 +1,81 @@
 """The mediator without a Colleague hierarchy.
 
-Widgets take a ``notify`` callable; the coordinator holds every interaction
-rule in one place and the widgets hold none.
+A checkout form with enough interdependent rules to *justify* a mediator:
+country restricts shipping methods, shipping method gates payment options
+and recomputes the total, and submit is enabled only when the whole set is
+coherent. Widgets know none of it -- every rule lives in one method.
 """
 
 from __future__ import annotations
 
 from collections.abc import Callable
+from dataclasses import dataclass, field
 
+SHIPPING_BY_COUNTRY = {
+    "CA": {"standard": 900, "express": 2400},
+    "US": {"standard": 700, "express": 1900},
+    "DE": {"standard": 1100},  # no express lane
+}
+#: cash-on-delivery is only offered on express shipments
+PAYMENTS_BY_SHIPPING: dict[str, tuple[str, ...]] = {
+    "standard": ("card",),
+    "express": ("card", "cod"),
+}
 
-class TextField:
-    def __init__(self, notify: Callable[[], None]) -> None:
-        self.text = ""
-        self._notify = notify
 
-    def type_text(self, text: str) -> None:
-        self.text = text
-        self._notify()
+@dataclass
+class Field:
+    """A dumb widget: holds a value, reports changes. No rules."""
 
+    notify: Callable[[], None]
+    value: str = ""
 
-class SignupForm:
-    """The mediator: rules in one readable method."""
+    def set(self, value: str) -> None:
+        self.value = value
+        self.notify()
 
-    def __init__(self) -> None:
-        self.username = TextField(self._recheck)
-        self.password = TextField(self._recheck)
-        self.submit_enabled = False
+
+@dataclass
+class CheckoutForm:
+    """The mediator: every cross-field rule, in one readable place."""
+
+    cart_cents: int
+    country: Field = field(init=False)
+    shipping: Field = field(init=False)
+    payment: Field = field(init=False)
+    shipping_options: tuple[str, ...] = ()
+    payment_options: tuple[str, ...] = ()
+    total_cents: int = 0
+    submit_enabled: bool = False
+
+    def __post_init__(self) -> None:
+        self.country = Field(self._recheck)
+        self.shipping = Field(self._recheck)
+        self.payment = Field(self._recheck)
+        self._recheck()
 
     def _recheck(self) -> None:
-        self.submit_enabled = bool(self.username.text) and len(self.password.text) >= 8
+        lanes = SHIPPING_BY_COUNTRY.get(self.country.value, {})
+        self.shipping_options = tuple(lanes)
+        if self.shipping.value not in lanes:
+            self.shipping.value = ""  # country change invalidated the lane
+        self.payment_options = PAYMENTS_BY_SHIPPING.get(self.shipping.value, ())
+        if self.payment.value not in self.payment_options:
+            self.payment.value = ""
+        self.total_cents = self.cart_cents + lanes.get(self.shipping.value, 0)
+        self.submit_enabled = bool(
+            self.country.value and self.shipping.value and self.payment.value
+        )
 
 
 def main() -> None:
-    form = SignupForm()
-    form.username.type_text("ada")
-    form.password.type_text("short")
-    print(f"weak password:  {form.submit_enabled}")
-    form.password.type_text("correcthorse")
-    print(f"valid form:     {form.submit_enabled}")
+    form = CheckoutForm(cart_cents=5000)
+    form.country.set("CA")
+    form.shipping.set("express")
+    form.payment.set("cod")
+    print(f"total {form.total_cents}, submit={form.submit_enabled}")
+    form.country.set("DE")  # express vanishes; dependent fields reset
+    print(f"after DE: shipping={form.shipping.value!r}, submit={form.submit_enabled}")
 
 
 if __name__ == "__main__":

patterns/behavioral/mediator/tests/test_mediator.py

@@ -19,18 +19,39 @@ def test_weak_password_keeps_submit_disabled(self) -> None:
 
 
 class TestPythonic:
-    def test_form_coordination(self) -> None:
-        form = pythonic.SignupForm()
-        form.username.type_text("ada")
-        form.password.type_text("correcthorse")
+    def test_happy_path_enables_submit_and_totals(self) -> None:
+        form = pythonic.CheckoutForm(cart_cents=5000)
+        form.country.set("CA")
+        form.shipping.set("express")
+        form.payment.set("cod")
         assert form.submit_enabled
-
-    def test_widgets_know_no_rules(self) -> None:
-        # A TextField is reusable with any notify callable -- no form coupling.
+        assert form.total_cents == 5000 + 2400
+
+    def test_country_change_cascades_through_dependent_fields(self) -> None:
+        form = pythonic.CheckoutForm(cart_cents=5000)
+        form.country.set("US")
+        form.shipping.set("express")
+        form.payment.set("cod")
+        form.country.set("DE")  # DE has no express -> shipping and payment reset
+        assert form.shipping.value == "" and form.payment.value == ""
+        assert not form.submit_enabled
+        assert form.shipping_options == ("standard",)
+
+    def test_payment_options_follow_shipping_method(self) -> None:
+        form = pythonic.CheckoutForm(cart_cents=1000)
+        form.country.set("CA")
+        form.shipping.set("standard")
+        standard_options: tuple[str, ...] = form.payment_options
+        assert standard_options == ("card",)
+        form.shipping.set("express")
+        express_options: tuple[str, ...] = form.payment_options
+        assert express_options == ("card", "cod")
+
+    def test_widgets_hold_no_rules(self) -> None:
         pings: list[str] = []
-        field = pythonic.TextField(lambda: pings.append("changed"))
-        field.type_text("x")
-        assert pings == ["changed"]
+        widget = pythonic.Field(notify=lambda: pings.append("changed"))
+        widget.set("anything")
+        assert pings == ["changed"]  # reusable with any coordinator
 
 
 class TestRealWorld:

patterns/behavioral/memento/README.md

@@ -8,6 +8,7 @@ symptoms: ["undo", "checkpoint and rollback", "save game", "restore previous sta
 verdict: use-with-care
 caveats:
   - "Immutable state makes the pattern nearly free: a snapshot is just keeping the old object. Design the state to be frozen and mementos fall out."
+  - "pickle.loads executes code while deserializing — only unpickle snapshots your own process produced; use JSON for anything crossing a trust boundary."
   - "Deep-copying big mutable graphs per keystroke is the naive cost; snapshot the smallest state that matters."
 stdlib_sightings: [copy.deepcopy, pickle.dumps, dataclasses.replace]
 ---

patterns/behavioral/memento/real_world.py

@@ -2,6 +2,11 @@
 
 dumps() produces an opaque snapshot; loads() restores an equivalent object
 -- checkpoint/rollback for anything picklable.
+
+SECURITY: ``pickle.loads`` executes code during deserialization. Only ever
+unpickle snapshots your own process produced and stored somewhere untrusted
+input cannot reach (CWE-502). For snapshots that cross a trust boundary,
+serialize explicit state as JSON instead.
 """
 
 from __future__ import annotations
@@ -21,6 +26,7 @@ def checkpoint(game: Game) -> bytes:
 
 
 def rollback(snapshot: bytes) -> Game:
+    # Safe ONLY because `snapshot` came from checkpoint() in this process.
     restored = pickle.loads(snapshot)
     assert isinstance(restored, Game)
     return restored

patterns/creational/abstract_factory/README.md

@@ -29,9 +29,10 @@ factory per family, and client code programmed against the interface.
 ## Pythonic solution
 
 Classes and functions are first-class, so the guide's advice is: accept
-*callables*. `pythonic.py` passes `Decimal` itself as the number factory; the
-"complete" factory bundling several builders is just a small dataclass of
-callables — no abstract base required.
+*callables*. `pythonic.py` renders one sales report through interchangeable
+document families (HTML for the web app, Markdown for the CLI) — each family
+a dataclass of builder callables that belong together, no abstract base
+required.
 
 ## In the wild