framework: contributor scaffold + per-rule justifications (Phase 2b.3)#38
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds contributor-facing scaffolding and documentation so new component classes can be created with the framework’s required surface (slots/ClassVars/registry/refdes/etc.) “by construction”, plus regression tests to keep the scaffold output compliant over time.
Changes:
- Introduces
scripts/scaffold_component.pyto generate a new component module, test stub, and update the kind package’s re-exports. - Adds contributor-focused tests that materialize scaffold output into a temp tree and validate syntax/shape/registry + that the generated test stub passes.
- Expands contributor docs (
CONTRIBUTING.md) and design rationale (docs/design-principles.md) to explain the rules and recommend the scaffold workflow.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/scaffold_component.py | New scaffold CLI that renders/writes component + test stub and updates kind __init__.py. |
| tests/contributor/_scaffold_harness.py | Utilities for running the scaffold into a temp root and loading generated modules safely. |
| tests/contributor/test_scaffold_output.py | Static/import-time assertions on scaffold output (syntax, slots, required ClassVars, registry, __init__.py updates, overwrite refusal). |
| tests/contributor/test_scaffold_emits_tests.py | Executes generated test stub functions in-process against generated class output. |
| docs/design-principles.md | Adds “Rules for component-class authors” section with per-rule “Why” justifications and a scaffold pointer. |
| CONTRIBUTING.md | Documents the scaffold as the recommended path and outlines what contributors must fill in post-generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+82
| # CamelCase → snake_case; handles consecutive caps (LM7805 → lm7805). | ||
| s = re.sub(r'(?<!^)(?=[A-Z])', '_', name) |
Comment on lines
+26
to
+29
| Or interactively: | ||
|
|
||
| uv run scripts/scaffold_component.py --interactive | ||
|
|
Comment on lines
+329
to
+341
| @validate_call(config={{'arbitrary_types_allowed': True}}) | ||
| def __init__( | ||
| self, | ||
| domain: GroundDomain = ELECTRICAL, | ||
| *, | ||
| refdes_number: RefdesNumber, | ||
| ) -> None: | ||
| validate_refdes(self.REFDES_PREFIX, refdes_number) | ||
| self._refdes_number = refdes_number | ||
| self._ports = {{ | ||
| {_ports_block(spec)} | ||
| }} | ||
|
|
|
|
||
| def evaluate(self) -> None: | ||
| {_evaluate_body(spec)} | ||
|
|
Comment on lines
+502
to
+504
| has_trailing_comma = stripped.endswith(',') | ||
| sep = '' if has_trailing_comma else ', ' | ||
| new_body = body.rstrip().rstrip(',') + f"{sep} '{name}'," |
Comment on lines
+235
to
+239
| *Why:* A real chip at power-on may be in any state its silicon allows; some latch types come up in a defined state, most don't. A scaffold that initialises every component to a defined state lies about the bench reality — the user's downstream check for "is this in the right state yet?" never sees the indeterminate-at-power-on case the real chip exhibits. | ||
|
|
||
| ### Scaffolding a new component | ||
|
|
||
| `scripts/scaffold_component.py` machine-applies the rules above. See [`CONTRIBUTING.md`](https://github.com/raeq/wirebench/blob/main/CONTRIBUTING.md#adding-a-new-part) for the invocation; the scaffold's output passes every framework rule by construction, so the contributor only fills in part-specific specification (pin logic, `VERIFY` / `GOTCHAS` strings, layout descriptor). |
Comment on lines
+67
to
+68
| `__call__()` shape that drives every OUT pin so the framework's | ||
| *OUT pin must be driven* invariant passes by default. |
Comment on lines
+118
to
+140
| def test_chip_scaffold_constructs_cleanly(tmp_path: Path) -> None: | ||
| """A scaffolded chip with an OUT pin still constructs — `evaluate()` | ||
| drives every OUT pin with a placeholder so the framework's | ||
| OUT-pin-must-be-driven invariant passes by default.""" | ||
| spec = chip_spec() | ||
| paths, component_module, _ = materialise_and_load(spec, tmp_path) | ||
| cls = getattr(component_module, spec.class_name) | ||
| instance = cls(refdes_number=1) | ||
| assert instance.refdes == f"{spec.refdes_prefix}1" | ||
| # OUT pin is driven by evaluate() — exercise that path too. | ||
| instance.evaluate() | ||
| assert instance.ports['out'].value is False # placeholder for Digital OUT | ||
|
|
||
|
|
||
| def test_chip_scaffold_declares_chip_as_base(tmp_path: Path) -> None: | ||
| """The chip kind inherits from `Chip`; the passive kind from | ||
| `Part`. Picking the right base class is the friction the scaffold | ||
| machine-applies.""" | ||
| paths, component_module, _ = materialise_and_load(chip_spec(), tmp_path) | ||
| text = paths['component'].read_text() | ||
| assert 'from framework.chip import Chip' in text | ||
| assert '(Chip)' in text | ||
|
|
|
|
||
| def _class_body(spec: ComponentSpec) -> str: | ||
| base = 'Chip' if spec.kind == 'chip' else 'Part' | ||
| pin_names_tuple = ', '.join(f"'_{p.name}'" for p in spec.pins) |
Owner
Author
|
Thanks — three real bugs, all fixed in 0a4f32b. Details below. High — chip scaffold produced invalid
|
added 2 commits
May 20, 2026 20:44
…hase 2b.3) The reviewer's "every class must obey project-specific rules" critique is a contributor complaint, not a user complaint. A user wires existing parts together; the friction lives in adding a new component class — `__slots__`, all six required ClassVars (REFDES_PREFIX, FOOTPRINT, PIN_NUMBERS, LAYOUT, VERIFY, GOTCHAS), @register, refdes validation, port shape, test stub. Two responses, both keeping the rules: **Scaffold script** — `scripts/scaffold_component.py`. A CLI that takes a brief part spec (name, kind, refdes prefix, footprint, pins, description) and emits a complete component-class file plus a matching test stub. The scaffold's output passes every framework rule by construction; the contributor fills in only the part-specific bits (real `evaluate()` / `__call__()` logic, VERIFY / GOTCHAS strings, layout descriptor). Supports `passive` and `chip` kinds today — the two shapes new contributors reach for first. Also re-exports the new class from the kind's `__init__.py`. **Per-rule physical justifications.** `docs/design-principles.md` gains a *Rules for component-class authors* section that mirrors the project CLAUDE.md's component-class rules with a `Why:` line keyed to a physical referent for each. The CLAUDE.md edits land locally (the file is gitignored per project policy) so Claude sessions see the same `Why:` content during contributor-side work. **CONTRIBUTING.md** documents the scaffold as the recommended path for adding new components, with the manual recipe kept as a fallback. **Tests** in `tests/contributor/`: - `test_scaffold_output.py` — every emitted file `ast.parse`s, declares `__slots__`, contains all six required ClassVars, registers with the framework registry, updates `__init__.py`'s re-exports, constructs cleanly, refuses to overwrite existing files. Covers both passive and chip kinds. - `test_scaffold_emits_tests.py` — the generated test stub passes when run against the generated class (every test function called in-process; unique class names per test to avoid registry collisions). - Helper module loads scaffold modules via `importlib.util.spec_from_file_location` so tmp_path output doesn't pollute the real `components.passives.*` namespace. Suite: 4813 passed (18 new), mypy clean.
…: acronym filenames, Low: phantom --interactive) **High — chip scaffold produced invalid Chip subclasses.** The previous template subclassed `Chip` but only set `_refdes_number` and `_ports` and never called `Chip.__init__`. `Chip.__init__` builds the port map from `Pin` objects, asserts every OUT pin is internally driven by a cell, and validates the drive-type declarations — none of which the scaffold's output supported. Worse, `Chip.__call__` is abstract; the scaffold's class couldn't instantiate at all (`TypeError: abstract method __call__`). Rewritten chip template: - Constructs one `Pin(PinId(n, name), …)` per declared pin. - For each OUT pin, instantiates an `IdleDriver` placeholder cell and wires `cell.ports['out']` to `pin.internal` so the *every OUT pin is internally driven* invariant passes by construction. Contributor replaces each `IdleDriver` with the real behavioural cell (concept cells under `src/components/chips/concepts/`). - Calls `super().__init__(pins=[…], cells=[…])` so the framework's port map, ERC checks, and refdes uniqueness pass cleanly. - Emits a concrete `__call__` mirroring `SN74HC04.__call__` — drives every IN/BIDIR pin, runs `evaluate()`, returns the tuple of OUT-pin values. `tests/contributor/test_scaffold_output.py` now has explicit assertions that: - The chip class constructs without `PartConfigurationError`. - Driving the chip via `__call__()` settles the OUT pin at the `IdleDriver`'s idle value (proves the cell is actually wired). - `super().__init__(` appears in the chip output. **Medium — `_snake_case` produced wrong filenames for acronyms.** The previous regex `(?<!^)(?=[A-Z])` split before every internal capital, turning `LM7806` into `l_m7806.py`, `SN74HC04` into `s_n74_h_c04.py`, and `ATmega328P` into `a_tmega328p.py`. None of those match the wirebench catalogue's existing file-naming convention (`lm7806.py`, `sn74hc04.py`, `atmega328p.py`). Rewritten with two acronym-aware regexes: - `([A-Z][A-Z]+)([A-Z][a-z])` → split an all-caps acronym (≥ 2 caps) from a following CapitalizedWord. The 2-cap floor prevents catastrophic backtracking on `ATmega` (the engine would otherwise back `[A-Z]+` down to a single `A` and match `A_Tmega`). - `([a-z])([A-Z])` → split CamelCase boundaries (`MyChip` → `My_Chip`). Parametrised test now pins five cases: `LM7806`, `SN74HC04`, `ATmega328P`, `MyChip`, `HTTPServer`. **Low — `--interactive` was advertised in the docstring but never implemented.** Removed the broken promise from the module docstring. The CLI takes explicit flags; non-interactive use is the only supported path. Suite: 4820 passed (7 new), mypy clean.
raeq
force-pushed
the
phase-2b-3-contributor-scaffold
branch
from
0a4f32b to
b564b63
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes Phase 2b. The reviewer's "every class must obey project-specific rules" critique was a contributor complaint, not a user complaint — a user wires existing parts together; the friction lives in adding a new component class. Two responses, both keeping the rules:
Scaffold script.
scripts/scaffold_component.py— CLI that takes a brief part spec (name, kind, refdes prefix, footprint, pins, description) and emits a complete component-class file plus a matching test stub. The output passes every framework rule by construction:__slots__declaredREFDES_PREFIX,FOOTPRINT,PIN_NUMBERS,LAYOUT,VERIFY,GOTCHAS@register('ClassName')decoration@validate_call-decorated__init__with refdes validationevaluate()that drives every OUT pin (satisfies the chip OUT-pin invariant)__call__()with hardware-pin-name parameters__str__/__repr__/refdes/refdes_numberproperties__init__.py's__all__Supports
passiveandchipkinds today — the two shapes new contributors reach for first. Other families (connector,diode,transistor,relay,transducer) inherit through dedicated base classes with shapes too varied to template usefully; for those, copy an existing example.Example invocation:
uv run scripts/scaffold_component.py \ --name LM7806 \ --kind chip \ --refdes-prefix U \ --footprint "Package_TO_SOT_THT:TO-220-3_Vertical" \ --pins "vin:in:Analog,gnd:in:Analog,vout:out:Analog" \ --description "6 V linear regulator — TO-220 fixed-output."Per-rule physical justifications.
docs/design-principles.mdgains a Rules for component-class authors section that surfaces aWhy:line — keyed to a physical referent — for each rule the framework imposes on component classes. The CLAUDE.md edits land locally too (the project policy keeps CLAUDE.md gitignored) so Claude sessions see the sameWhy:content during contributor-side work.CONTRIBUTING.md documents the scaffold as the recommended path for adding new components, with the manual recipe kept as a fallback for the unsupported kinds.
Files
scripts/scaffold_component.py— the scaffold CLI (mypy strict-clean).docs/design-principles.md— new Rules for component-class authors section with per-ruleWhy:justifications.CONTRIBUTING.md— documents the scaffold as the recommended path; manual recipe kept as fallback.tests/contributor/_scaffold_harness.py— loads scaffolded modules viaimportlib.util.spec_from_file_locationso the temp-dir output doesn't pollute the realcomponents.passives.*namespace.tests/contributor/test_scaffold_output.py— static + import-time checks: every emitted fileast.parses, declares__slots__, contains all six required ClassVars, registers with the framework registry, updates__init__.py's re-exports, constructs cleanly, refuses to overwrite existing files. Covers both passive and chip kinds.tests/contributor/test_scaffold_emits_tests.py— the generated test stub passes when run against the generated class (every test function called in-process; unique class names per test to avoid registry collisions).Test plan
uv run pytest— 4813 passed (18 new), 16 skippeduv run mypy src/ demos/ scripts/scaffold_component.py— clean