feat(studio): family lettering, tints, and the rack as a signal path - #3187
Conversation
13f574f to
72210c5
Compare
e47c566 to
b2b1d5f
Compare
terencecho
left a comment
There was a problem hiding this comment.
Verdict: LGTM — clean design, empirical claims hold, CI failure is stack-wide pre-existing drift confirmed on parent PR #3186.
Verified claims (file:line at head SHA b2b1d5f)
- "Two faces of type, four families on the sans, serif on non-linear, mono on smart." —
packages/studio/src/components/editor/propertyPanelFxFamily.ts:34-40(FX_FAMILY_TYPE): filterfont-light uppercase tracking-[0.14em], dynamicsfont-bold uppercase tracking-tight, nonlinearfont-serif italic, timefont-extralight uppercase tracking-[0.24em], smartfont-mono font-medium. Applied atpropertyPanelFxNodeRow.tsx:319,propertyPanelFxCarveModule.tsx:219,propertyPanelFxEqModule.tsx:151. - "Smart = carve + Tone EQ + leveller." —
propertyPanelFxFamily.ts:66(fromCarve || fromEq || fromLeveller). Carve/EQ modules hard-setdata-fx-family="smart"(FxCarveModule.tsx:210,FxEqModule.tsx:141); leveller-tagged nodes flowing through the generic row pick it up automatically. Note: no standalone leveller module exists yet inpackages/studio/src/components/editor/, sofromLevellersupport is currently exercised only via the helper path — not a defect. - "Tint = family hue × registry-position lightness step, 62-76% at 32% sat." —
propertyPanelFxFamily.ts:80-82((62 + step * 14).toFixed(1)%, 32% sat, per-family hue fromFAMILY_HUE). - "IN/OUT terminals + rack-shown numbering + preset run brackets." —
propertyPanelFxSection.tsx:415-419(IN),:497-500(OUT); positions counted over shown modules (showCarve ? 1 : 0) + eqIds.lengthskip) at:358-362;runsgrouping consecutivefromPresetat:335-345; preset wrapper at:483-493.
OLD-assumption counterfactuals (5 new tests)
- "draws the rack as a signal path" — falsified by OLD no-IN/OUT-terminals + node headers with no
hf-fx-node-index. Also falsified by numbering offchain.nodesindex (would emit "01"/"02" instead of "02"/"03", because a carve card wouldn't consume slot 01). - "draws a preset's nodes as one thing" — falsified by OLD flat
handBuilt.map: no[data-fx-preset]wrapper exists,run.querySelectorreturns null. - "brackets only nodes it sits next to" — falsified by naive "group by
fromPresetregardless of adjacency": would produce 1 wrapper with bothhighpass+lowpass, not 2 separate wrappers. - "letters each family differently" — falsified by OLD single
font-semiboldon.hf-fx-node-name:[data-fx-family]attribute missing (0-length query), andfont-serifcount is 0 not 1. - "tints two filters apart, same hue" — falsified by OLD absent
borderLeftColor: both.style.borderLeftColorare"",.not.toBefails.
Cross-PR coupling
- Family definition: this PR's
FxFamily = "filter"|"dynamics"|"nonlinear"|"time"|"smart"is notHF_AUDIO_FX_PRESET_FAMILIESfrom #3177 (voice|repair|character|space). Different taxonomy — module families derive fromHF_AUDIO_FX.group(packages/core/src/audioFx.ts:75), not from preset family. No conflict, no shared source. - Signal-path direction: derived from
chain.nodesarray position viahandBuilt→positionsmap (propertyPanelFxSection.tsx:358-362). No new ordering field. HfAudioFxNode: no new fields. Family logic reads existingtype/fromCarve/fromEq/fromLeveller(audioFx.ts:793-840). Parse/serialize unchanged; no allow-list work needed.- Colorblind discriminator: non-color discriminator is the lettering itself (sans-weight/case/tracking, serif on non-linear, monospace on smart) — explicitly reasoned in
propertyPanelFxFamily.tsdocstring: "the name already carries the family in its lettering, and colouring it too would fight the panel's own tokens." Tint is a within-family sibling step, not a family discriminant. Accessibility OK.
Nits (non-blocking)
fxFamilyOffallback to"filter"for any unrecognized group is quiet — ifHfAudioFxGroupever gains a value not enumerated in the ternary atpropertyPanelFxFamily.ts:69, new modules would silently letter as filter. Current union is closed; asatisfies/assertNeverwould be belt-and-braces.plans/audio-fx-ux/README.mdis on the recurring oxfmt drift list; the new section appended here doesn't clear the file's drift (pre-existing).
CI status
- Preflight (lint + format) FAILURE = 9 files, all in the recurring stack-wide drift set (
README.md,plans/audio-automation-lanes/SPEC.md,plans/audio-fx-presets.md,plans/audio-fx-ux/README.md,plans/automation-lanes-shared-rows.md,plans/automation-time-selection-design.md,skills/hyperframes-audio/SKILL.md,.../references/attributes.md,.../references/fx-registry.md). Identical failure list on parent PR #3186 — pre-existing, not this-PR signal.oxlintclean ("0 warnings, 0 errors"). preview-regressionFAILURE = cascade from Preflight (Preview parity skipped → gate check fails). Not a real signal.- Regression, Player perf, WIP: all SUCCESS.
— Review by tai (pr-review)
miga-heygen
left a comment
There was a problem hiding this comment.
Review: feat(studio): family lettering, tints, and the rack as a signal path — #3187
Verdict: LGTM with one design-record question
Signal-path framing works. IN/OUT terminals present, two-digit zero-padded node numbering accounts for carve and EQ modules so the count is sequential. Preset run bracketing wraps consecutive nodes in a dashed-border container — reordering breaks the bracket (tested explicitly).
Family tinting is correctly derived from registry data. fxFamilyOf() reads the group field with correct overrides for composite nodes (fromCarve/fromEq/fromLeveller → "smart"). fxFamilyTint() steps lightness from 62% to 76% at 32% saturation to differentiate same-family modules.
Tests are thorough: signal path terminals + numbering, preset grouping, bracket splitting on reorder, per-family unique lettering, same-family tint differentiation.
One question for the author: Three of five family hues diverge significantly from the design record in #3176 (plans/audio-fx-presets.md):
| Family | Design record | This PR | Delta |
|---|---|---|---|
| Nonlinear | #FF6B5C (coral, hue 6) |
hue 310 (magenta) | 55° |
| Time | #B98CFF (violet, hue 264) |
hue 165 (cyan) | 99° |
| Smart | #3CE6AC (teal, hue 160) |
hue 95 (yellow-green) | 65° |
Filter (blue) and Dynamics (orange) match. If the other three are intentional departures (likely iteration), the design record should be updated; if not, three values in FAMILY_HUE need correction. Not blocking — the hues work visually regardless — but the design record is the kind of document people will reference later.
Review by Miga
🤖 Generated with Claude Code
b2b1d5f to
95772df
Compare
72210c5 to
d9da574
Compare
95772df to
94c0291
Compare
d9da574 to
fa461bf
Compare
94c0291 to
a7e8e7f
Compare
fa461bf to
9afd02d
Compare
a7e8e7f to
ffbc329
Compare
9afd02d to
d2934c2
Compare
# Conflicts: # packages/studio/src/components/editor/propertyPanelAudioFxGroup.test.tsx # packages/studio/src/components/editor/propertyPanelAudioFxGroup.tsx # packages/studio/src/components/editor/propertyPanelFxSection.tsx # skills-manifest.json # skills/hyperframes-audio/SKILL.md # skills/hyperframes-audio/references/attributes.md # skills/hyperframes-audio/scripts/carve.mjs
A rack of eight modules is eight lines of text, and reading it should not mean reading eight names. Four families told apart by weight, case and tracking; the serif spent on the one family that generates signal rather than shaping it; monospace for the measuring modules, because what they show IS a readout.
The schematic, translated to one column: IN and OUT terminals, every step numbered over what the rack shows rather than the chain, and a preset's consecutive nodes bracketed as the one thing that was added.
🤖 Generated with Claude Code