Skip to content

Commit 65ee170

Browse files
RonnyPfannschmidtclaude
RonnyPfannschmidt
and
claude
committed
docs(design): commit refactoring design writeups into history
Add the design/ writeup chain (decisions + per-step docs 01-07) that guides the stacked refactoring branches. Exclude design/ from blacken-docs since the docs use elided pseudo-code signatures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 64d28b8 commit 65ee170

10 files changed

Lines changed: 1270 additions & 0 deletions

‎.pre-commit-config.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ repos:
3434
hooks:
3535
- id: blacken-docs
3636
additional_dependencies: [black==24.2.0]
37+
exclude: ^design/
3738
- repo: local
3839
hooks:
3940
- id: rst

‎design/01-module-reorganization.md‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# 01 — Module reorganization
2+
3+
**Status:** Foundation step (behavior-preserving move)
4+
**Depends on:** nothing
5+
**Next:** [02-configuration-objects.md](02-configuration-objects.md)
6+
7+
## Problem
8+
9+
On `main`, hook machinery is concentrated in:
10+
11+
| File | Contents today |
12+
|------|----------------|
13+
| [`src/pluggy/_hooks.py`](../src/pluggy/_hooks.py) | TypedDicts, markers, callers, HookImpl, HookSpec, helpers |
14+
| [`src/pluggy/_callers.py`](../src/pluggy/_callers.py) | `_multicall`, old-style wrapper adapter |
15+
16+
Later steps (Protocols, CompletionHook, typed impls) need clear module
17+
boundaries. try-claude already performed this split.
18+
19+
## Goals
20+
21+
- Split by role into focused modules (try-claude layout, clearer names OK).
22+
- Preserve import paths via re-exports from `_hooks.py` / shims.
23+
- Zero behavior change in this step.
24+
25+
## Non-goals
26+
27+
- New types, CompletionHook, Protocol callers (docs 02–05).
28+
- Copying anything from `reiterate-claude`.
29+
30+
## Target design (chosen)
31+
32+
Cross-check summary:
33+
34+
| Branch | Layout | Notes |
35+
|--------|--------|-------|
36+
| `try-claude` | `_hook_config`, `_hook_markers`, `_hook_callers` (callers+impls), `_callers` | Good role split; `_hook_callers` too large; `_hook_*` + `_callers` naming clash |
37+
| `reiterate-claude` | `_config`, `_decorators`, `_caller`, `_implementation`, `_execution` | Cleaner names; **do not copy its logic** |
38+
| **This step** | reiterate **names** + current `main` **content** | Move-only |
39+
40+
```text
41+
src/pluggy/
42+
_config.py # HookspecOpts, HookimplOpts, normalize_hookimpl_opts
43+
_decorators.py # markers, HookSpec, varnames
44+
_caller.py # HookCaller, HookRelay, _SubsetHookCaller
45+
_implementation.py # HookImpl + _Plugin / _HookImplFunction
46+
_execution.py # _multicall + wrapper helpers
47+
_hooks.py # re-exports (compat)
48+
_callers.py # thin re-export of _execution (compat)
49+
```
50+
51+
Name mapping from try-claude: see [DECISIONS.md](DECISIONS.md) D1. For this
52+
step, move **current `main` code** only — do not yet port try-claude’s new
53+
types/Protocols.
54+
55+
## Reference branch / files
56+
57+
```bash
58+
# Layout inspiration only — content for step 01 is current main
59+
git show try-claude:src/pluggy/_hook_config.py
60+
git show try-claude:src/pluggy/_hook_markers.py
61+
git show try-claude:src/pluggy/_hook_callers.py
62+
git show try-claude:src/pluggy/_callers.py
63+
git show try-claude:src/pluggy/_hooks.py
64+
```
65+
66+
## Implementation steps
67+
68+
1. Create modules; cut-and-paste from `main`.
69+
2. Fix relative imports; avoid cycles (`TYPE_CHECKING` where needed).
70+
3. `_hooks.py` re-exports prior public/internal names.
71+
4. Thin-shim or delete `_callers.py` after updating internal imports to
72+
`_execution`.
73+
5. Verify:
74+
75+
```bash
76+
uv run pytest
77+
uv run pre-commit run -a
78+
```
79+
80+
Commit message:
81+
82+
```text
83+
chore(structure): split hook modules into _config, _decorators, _caller, _implementation, _execution
84+
```
85+
86+
## Public API / back-compat
87+
88+
- `from pluggy import ...` unchanged.
89+
- `from pluggy._hooks import ...` unchanged via re-exports.
90+
91+
## Tests
92+
93+
- No new semantic tests; suite must stay green.
94+
95+
## Done when
96+
97+
- [ ] Role modules exist; `_hooks.py` is re-export layer.
98+
- [ ] Move-only diff (no intentional behavior change).
99+
- [ ] pytest + pre-commit green.

‎design/02-configuration-objects.md‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# 02 — Configuration objects (TypedDicts removed)
2+
3+
**Status:** Replace live options encoding
4+
**Depends on:** [01-module-reorganization.md](01-module-reorganization.md)
5+
**Next:** [03-markers-attach-config.md](03-markers-attach-config.md)
6+
7+
## Problem
8+
9+
On `main`, options are `HookspecOpts` / `HookimplOpts` TypedDicts — unvalidated
10+
dicts copied around as the live representation. try-claude introduced proper
11+
configuration classes; those are the API.
12+
13+
## Goals
14+
15+
- Add `HookspecConfiguration` and `HookimplConfiguration` (`__slots__`,
16+
`Final`, validation).
17+
- **Remove TypedDicts from the live and public API surface.**
18+
- Provide a **narrow pytest support shim** that accepts mapping/dict-shaped
19+
options and converts them to configuration objects (pytest migration only).
20+
- Export configuration classes from `pluggy`.
21+
22+
## Non-goals
23+
24+
- Keeping TypedDicts “for compatibility” as a dual API ([DECISIONS.md](DECISIONS.md) D4).
25+
- Marker attachment (doc 03) — can land in the same PR if cleaner, but
26+
conceptually next.
27+
- `create_hookimpl` body (needs NormalImpl/WrapperImpl — doc 04); may stub
28+
or add method signature with a TODO.
29+
30+
## Target design
31+
32+
```python
33+
# src/pluggy/_config.py
34+
35+
class HookspecConfiguration:
36+
__slots__ = ("firstresult", "historic", "warn_on_impl", "warn_on_impl_args")
37+
firstresult: Final[bool]
38+
historic: Final[bool]
39+
warn_on_impl: Final[Warning | None]
40+
warn_on_impl_args: Final[Mapping[str, Warning] | None]
41+
42+
def __init__(self, firstresult=False, historic=False, ...):
43+
if historic and firstresult:
44+
raise ValueError("cannot have a historic firstresult hook")
45+
...
46+
47+
class HookimplConfiguration:
48+
__slots__ = ("wrapper", "hookwrapper", "optionalhook",
49+
"tryfirst", "trylast", "specname")
50+
...
51+
# create_hookimpl added in doc 04
52+
```
53+
54+
Pytest support shim (name flexible — e.g. `_pytest_compat.py` or helpers in
55+
`_config.py` marked clearly):
56+
57+
```python
58+
def hookspec_config_from_mapping(opts: Mapping[str, Any]) -> HookspecConfiguration:
59+
"""Pytest/support only: accept legacy dict-shaped options."""
60+
return HookspecConfiguration(
61+
firstresult=opts.get("firstresult", False),
62+
historic=opts.get("historic", False),
63+
warn_on_impl=opts.get("warn_on_impl"),
64+
warn_on_impl_args=opts.get("warn_on_impl_args"),
65+
)
66+
67+
def hookimpl_config_from_mapping(opts: Mapping[str, Any]) -> HookimplConfiguration:
68+
...
69+
```
70+
71+
Do **not** keep `TypedDict` classes in `__all__`. Do not document dicts as
72+
the options API. Markers take kwargs that construct configuration objects
73+
directly (doc 03).
74+
75+
## Reference branch / files
76+
77+
```bash
78+
git show try-claude:src/pluggy/_hook_config.py
79+
```
80+
81+
Port classes from try-claude; then **delete** the TypedDict definitions from
82+
the live module (try still kept them — we go further per D4). Isolate mapping
83+
helpers as the only dict-facing surface.
84+
85+
## Implementation steps
86+
87+
### Step 2.1 — Add configuration classes
88+
89+
Port from try-claude `_hook_config.py` into `_config.py`.
90+
91+
### Step 2.2 — Remove TypedDicts from live path
92+
93+
1. Delete `HookspecOpts` / `HookimplOpts` TypedDict definitions (or move to a
94+
private pytest-shim module if pytest tests in-tree still need the names
95+
during transition — prefer deletion + mapping helpers).
96+
2. Update all internal imports/usages to configuration classes.
97+
3. Update `__init__.py` exports: add configuration classes; drop TypedDict
98+
exports.
99+
100+
### Step 2.3 — Pytest support shim
101+
102+
1. Add `hookspec_config_from_mapping` / `hookimpl_config_from_mapping`.
103+
2. Document in docstring: for pytest/support migration only, not the public
104+
options API.
105+
3. If pytest (downstream) needs a temporary import path, keep it private
106+
(`pluggy._config` or `pluggy._pytest_compat`), not in `__all__`.
107+
108+
### Step 2.4 — Tests
109+
110+
- Validation (`historic` + `firstresult`).
111+
- Mapping shim round-trip.
112+
- No remaining tests that treat TypedDicts as the primary API.
113+
114+
```bash
115+
uv run pytest
116+
uv run pre-commit run -a
117+
```
118+
119+
Commit message:
120+
121+
```text
122+
feat(config): replace TypedDict options with Hook*Configuration classes
123+
```
124+
125+
## Public API / back-compat
126+
127+
| Before | After |
128+
|--------|-------|
129+
| `HookspecOpts` / `HookimplOpts` TypedDicts | **Removed** from public API |
130+
| dict attrs on marked functions | configuration objects (doc 03) |
131+
| — | `HookspecConfiguration` / `HookimplConfiguration` exported |
132+
| — | private mapping shim for pytest support |
133+
134+
Marker **kwargs** (`firstresult=`, `wrapper=`, …) stay — they construct
135+
objects, they are not TypedDict literals.
136+
137+
## Tests to add/update
138+
139+
| File | Coverage |
140+
|------|----------|
141+
| `testing/test_configuration.py` | Class validation; mapping shim |
142+
| Anything importing TypedDicts | Migrate or delete |
143+
144+
## Done when
145+
146+
- [ ] Configuration classes are the only live options encoding.
147+
- [ ] TypedDicts gone from public `__all__` / docs path.
148+
- [ ] Pytest mapping shim exists and is clearly non-API.
149+
- [ ] pytest + pre-commit green.

‎design/03-markers-attach-config.md‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# 03 — Markers attach configuration objects
2+
3+
**Status:** Markers emit new types only
4+
**Depends on:** [02-configuration-objects.md](02-configuration-objects.md)
5+
**Next:** [04-hookimpl-wrapper-types.md](04-hookimpl-wrapper-types.md)
6+
**Also unlocks:** [06-project-spec.md](06-project-spec.md)
7+
8+
## Problem
9+
10+
Markers on `main` attach dicts. After doc 02, configuration classes exist;
11+
markers and discovery must use them exclusively.
12+
13+
## Goals
14+
15+
- `HookspecMarker` / `HookimplMarker` attach `HookspecConfiguration` /
16+
`HookimplConfiguration` as `<project>_spec` / `<project>_impl`.
17+
- `HookSpec` stores `config: HookspecConfiguration`.
18+
- Manager parsing reads configuration objects only (no dict branch except
19+
calling the pytest mapping shim if an explicit compat path is required).
20+
21+
## Non-goals
22+
23+
- ProjectSpec on markers (doc 06) — optional to combine.
24+
- Impl subclass factory (doc 04).
25+
26+
## Target design
27+
28+
```python
29+
def setattr_hookspec_opts(func: _F) -> _F:
30+
config = HookspecConfiguration(
31+
firstresult=firstresult,
32+
historic=historic,
33+
warn_on_impl=warn_on_impl,
34+
warn_on_impl_args=warn_on_impl_args,
35+
)
36+
setattr(func, self.project_name + "_spec", config)
37+
return func
38+
39+
40+
def setattr_hookimpl_opts(func: _F) -> _F:
41+
config = HookimplConfiguration(...)
42+
setattr(func, self.project_name + "_impl", config)
43+
return func
44+
```
45+
46+
Decorator kwargs unchanged. Attribute **value type** is configuration object.
47+
48+
```mermaid
49+
sequenceDiagram
50+
participant Dev
51+
participant Marker as HookimplMarker
52+
participant Func as function
53+
participant PM as PluginManager
54+
Dev->>Marker: "@hookimpl(tryfirst=True)"
55+
Marker->>Func: "project_impl = HookimplConfiguration"
56+
Dev->>PM: register(plugin)
57+
PM->>Func: getattr config
58+
Note over PM: no dict path
59+
```
60+
61+
## Reference branch / files
62+
63+
```bash
64+
git show try-claude:src/pluggy/_hook_markers.py
65+
git show try-claude:src/pluggy/_manager.py
66+
```
67+
68+
## Implementation steps
69+
70+
1. Update markers to construct/attach configuration objects.
71+
2. Update `HookSpec` and manager discovery to use `.firstresult` etc.
72+
3. Remove any `normalize_hookimpl_opts` dict mutation on the live path.
73+
4. Tests: attached attr is configuration instance; historic+firstresult
74+
raises at decoration time.
75+
76+
```bash
77+
uv run pytest && uv run pre-commit run -a
78+
```
79+
80+
Commit message:
81+
82+
```text
83+
feat(decorators): attach Hook*Configuration objects on marked functions
84+
```
85+
86+
## Public API / back-compat
87+
88+
- Decorator kwargs stable.
89+
- Private attribute payload is now a configuration object (not a dict).
90+
- Callers that poked dict attrs must use the pytest mapping shim or migrate.
91+
92+
## Tests
93+
94+
| File | Coverage |
95+
|------|----------|
96+
| `testing/test_configuration.py` | Attachment + validation |
97+
| Registration / marker tests | Adapt off dict assumptions |
98+
99+
## Done when
100+
101+
- [ ] No marker or manager live path attaches/reads TypedDicts.
102+
- [ ] Suite green.

0 commit comments

Comments
 (0)