Sub E: Add-effect issue template + CONTRIBUTING + scaffolder - #14
Conversation
Adds // SCAFFOLDER:INSERT_MOD / _EXPORT / _PAGE / _DISPATCH and # SCAFFOLDER:INSERT_REGISTRY marker comments at the insertion points the gen-effect.mjs scaffolder patches against. No runtime behavior change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
One-command scaffolder that generates the 5 per-effect files (Rust kernel, TS class, catalog page, Tier2 test, Tier3a spec) and patches 5 sentinel markers in pre-existing files. Templates mirror Sub D's Gain artifacts 1:1, including the renderEffectPage helper pattern for catalog pages and test.fixme for Tier3a stubs (so CI signals red until a real spec replaces it). CLI supports --class <PascalName> for effects with audio acronyms (EQ3Band, FFTWindow, LFO) whose naive kebab→Pascal split would produce Eq3Band / FftWindow / Lfo. scripts/gen-effect.test.mjs covers valid/invalid names, --class override, --force overwrite, missing markers, and patch idempotency against a tmp root; 13/13 pass under `node --test`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
YAML issue form that forces every new-effect request to declare the spec in machine-readable form before submission: effect name, summary, category, reference source + license, parameter list, preset list (≥3), channel model, null-test tolerance, edge cases. Optional "Class name override" field pairs with the scaffolder's --class flag for acronymed effects. Closing markdown block codifies the 8-step worker AI workflow inline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Repositions CONTRIBUTING.md from maintainer-oriented Vercel setup notes to contributor-facing onboarding. Sections: Ways to contribute, Reference source allow-list (interim placeholder until Sub F lands LICENSE-THIRDPARTY), the 8-step workflow a worker AI follows to land an effect, Common pitfalls (renderEffectPage lifecycle, AudioParam length handling, alloc/dealloc size discipline, state-first ABI, PascalCase acronyms), Review expectations, Commit conventions. Vercel maintainer notes preserved at the bottom as §7. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds two root scripts: `gen:effect` (invokes the scaffolder) and `test:scaffolder` (runs the self-test under node --test). CI's wasm-build job picks up `vp run test:scaffolder` so marker drift or template regressions fail the build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 957184165c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!/^[a-z][a-z0-9-]*$/.test(name)) | ||
| fail(2, `name must match /^[a-z][a-z0-9-]*$/, got "${name}"`); |
There was a problem hiding this comment.
The name regex currently accepts values like foo--bar and foo-, but deriveNames() then splits on - and unconditionally calls s[0].toUpperCase(), which crashes with TypeError for empty segments. In CLI mode this bypasses the intended ScaffoldError path and hard-crashes the scaffolder (node scripts/gen-effect.mjs foo--bar). Please reject empty kebab segments during validation (or guard the PascalCase conversion) so invalid names fail cleanly with exit code 2.
Useful? React with 👍 / 👎.
The prior regex /^[a-z][a-z0-9-]*$/ accepted `foo--bar`, `foo-`, and
`-foo`, which then tripped `deriveNames()`'s `s.split("-").map(s =>
s[0].toUpperCase() + s.slice(1))` at the empty segment — `undefined`
has no `.toUpperCase()`, surfacing as a raw TypeError that bypasses
ScaffoldError and the intended exit 2 path.
Tightens the pattern to require at least one [a-z0-9] in each
hyphen-delimited segment. Adds a regression test covering the five
shapes that used to crash.
Reported by @codex on #14.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66831db0da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two codex findings on 66831db: P2: `snake` is interpolated verbatim into `pub mod <snake>;` (Rust) and `import * as <snake> from ...` (TS). A name like `for` passes the kebab regex but lands as `import * as for ...` and breaks the whole tree. Blocks the known reserved-identifier set (Rust keywords + TS/JS reserved words that are all lowercase and thus reachable through the regex) up front with exit 2. P3: the add-effect.yml `Effect name` field still documented the old `/^[a-z][a-z0-9-]*$/`. Updates the description to the current non-empty-segment regex and calls out the reserved-ident constraint so contributors don't file issues with names the scaffolder would refuse. Adds a regression test covering ten reserved names. Reported by @codex on #14. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review Addressed both findings on 66831db in f6d719c: P2 (reserved idents): Added a P3 (form regex drift): Updated the
|
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Encodes the "add a new effect" workflow that Sub D proved into three artifacts so future effects can be landed by worker AIs without asking for DSP/architectural clarifications.
.github/ISSUE_TEMPLATE/add-effect.yml— GitHub issue form with required fields (name, summary, category, reference + license, parameters, ≥3 presets, channel model, tolerance, edge cases) + optional "Class name override" for acronymed effects.scripts/gen-effect.mjs— one-command scaffolder (vp run gen:effect <name> [--class <Pascal>] [--force]) that writes 5 new files (Rust kernel, TS class, catalog page, Tier2 test, Tier3a spec) and patches 5 sentinel markers in pre-existing files (effects/mod.rs,effects/index.ts,examples/main.ts,worklet/processor.ts,gen-golden/effects.py). Templates mirror Sub D's Gain 1:1, including therenderEffectPage()helper andtest.fixme(...)for Tier3a (stays red until a real spec replaces it).scripts/gen-effect.test.mjs— 13 self-tests covering valid/invalid names,--classoverride,--force, missing markers, and patch idempotency. Wired into CI viavp run test:scaffolderin the wasm-build job.CONTRIBUTING.md— rewritten around an 8-step worker workflow, reference allow-list (interim placeholder until Sub F), common pitfalls (renderEffectPage lifecycle, alloc/dealloc size discipline, state-first ABI, PascalCase acronym →--classflag).Scope changes vs. Issue #6
Per discussion before implementation, issue #6 body was updated as the SSoT before any code landed. Dropped from scope:
MANIFEST.toml+manifest-update.yml— no downstream consumer exists; deferred until a real reader (README generator, drift-detection CI, etc.) lands..github/ISSUE_TEMPLATE/config.yml— Discussions not enabled.Links
Definition of done
scripts/gen-effect.mjshandles happy path +--class+--force+ invalid names + missing markersscripts/gen-effect.test.mjsruns in CI (13/13 pass locally)CONTRIBUTING.mdrewritten per §6.4 of the updated issue bodypackage.jsonhasgen:effect+test:scaffoldertest:scaffoldervp check/vp run smoke/vp run test:tier2/vp run test:scaffolderall green locallyvp run test:tier3agreen in CI (not run locally — Playwright is slow)Preview
Vercel preview should attach on push; the catalog is unchanged from Sub D so the preview is primarily a sanity check that the existing effects (passthrough, gain) still render.
Test plan
vp run gen:effect <some-effect>on a branch to confirm the scaffolder produces files that match CONTRIBUTING.md §3's expectations