feat: schema-driven controls + runtime dynamic param schema enrichment#243
feat: schema-driven controls + runtime dynamic param schema enrichment#243streamer45 merged 36 commits intomainfrom
Conversation
Extend the Monitor view ConfigurableNode to render toggle switches and text inputs for parameters marked tunable: true in the node's param_schema, alongside the existing numeric sliders. Changes: - jsonSchema.ts: add path/enum fields to JsonSchemaProperty, add extractToggleConfigs and extractTextConfigs extractors, add path field to SliderConfig - SchemaControls.tsx: new BooleanToggleControl (toggle switch) and TextInputControl (debounced 300ms text input) using ref-based callback pattern and useTuneNode for live updates - ConfigurableNode.tsx: render toggles, sliders, then text inputs from schema extraction - controlProps.ts: add readByPath utility (inverse of buildParamUpdate) for reading nested values via dot-notation paths - InspectorPane.tsx: deduplicate JsonSchemaProperty by importing from jsonSchema.ts, support path field in edit handlers via tuneNodeConfig - Tests for new extractors and readByPath Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| const { tuneNodeConfig } = useTuneNode(sessionId ?? null); | ||
|
|
||
| // Read from atom for live sync | ||
| const paramsKey = sessionId ? `${sessionId}\0${nodeId}` : nodeId; | ||
| const nodeParams = useAtomValue(nodeParamsAtom(paramsKey)); | ||
|
|
||
| // Effective value: atom > props > default | ||
| const effectiveValue = (() => { | ||
| const stored = readByPath(nodeParams, config.path); | ||
| if (typeof stored === 'boolean') return stored; | ||
| const prop = readByPath(params as Record<string, unknown>, config.path); | ||
| if (typeof prop === 'boolean') return prop; | ||
| if (typeof config.schema.default === 'boolean') return config.schema.default; | ||
| return false; | ||
| })(); | ||
|
|
||
| const [checked, setChecked] = useState(effectiveValue); | ||
|
|
||
| // Sync with external changes | ||
| useEffect(() => { | ||
| setChecked(effectiveValue); | ||
| }, [effectiveValue]); | ||
|
|
||
| // Ref pattern: keep tuneNodeConfig ref stable so toggle handler identity | ||
| // doesn't change when sessionId (rarely) changes. | ||
| const tuneRef = useRef(tuneNodeConfig); | ||
| useEffect(() => { | ||
| tuneRef.current = tuneNodeConfig; | ||
| }, [tuneNodeConfig]); | ||
|
|
||
| const handleToggle = useCallback(() => { | ||
| setChecked((prev) => { | ||
| const next = !prev; | ||
| tuneRef.current(nodeId, buildParamUpdate(config.path, next)); | ||
| return next; | ||
| }); | ||
| }, [nodeId, config.path]); |
There was a problem hiding this comment.
🚩 BooleanToggleControl uses useTuneNode directly instead of onParamChange prop pattern
The new BooleanToggleControl and TextInputControl in SchemaControls.tsx use useTuneNode directly to send updates, while the existing NumericSliderControl receives onParamChange as a prop from the parent. This is an intentional design divergence — the new controls bypass the stableOnParamChange → tuneNode flow (which uses writeNodeParam for flat key storage) in favor of tuneNodeConfig (which uses deepMerge + writeNodeParams for nested path support). The tradeoff is that the new controls skip the validateParamValue validation step that stableOnParamChange performs in ui/src/views/MonitorView.tsx:895-905. This means boolean toggles and text inputs won't show validation error toasts if the server-side schema constraints are violated.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
This is an intentional design choice. The toggle/text controls use useTuneNode directly because they need buildParamUpdate + tuneNodeConfig for nested dot-notation paths, which the onParamChange → tuneNode flow didn't support (it creates flat { [param]: value } payloads).
With the fix in 3df959e, stableOnParamChange in MonitorView now detects dot-notation paths and routes them through buildParamUpdate + tuneNodeConfig automatically. This means the toggle/text controls could be refactored to use onParamChange in the future for consistency, but doing so would add prop-threading complexity without functional benefit since both paths produce identical wire payloads.
Re: validation skipping — boolean toggles are inherently constrained (true/false only), and text inputs currently have no server-side schema constraints beyond type. If validateValue gains string-specific rules (e.g. minLength, pattern), the text control should be updated to validate before sending.
Address review findings: - useNumericSlider: accept optional path param, use readByPath for atom readback and pass effectivePath to onParamChange so nested params (e.g. properties.score) produce correct wire payloads. - ConfigurableNode: destructure and forward path from SliderConfig to NumericSliderControl. - MonitorView: stableOnParamChange and handleRightPaneParamChange now detect dot-notation paths and route through buildParamUpdate + tuneNodeConfig instead of flat tuneNode. - InspectorPane: use readByPath for currentValue readback so nested paths display correctly; route path-override params through onParamChange (works in both Monitor and design view) instead of calling tuneNodeConfig directly (which no-ops in design view). Remove now-unused useTuneNode/buildParamUpdate imports. - extractTextConfigs: exclude enum-constrained strings (those need a select/dropdown, not a free-text input). - Tests: add enum exclusion and empty-enum inclusion cases. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
ui/src/views/MonitorView.tsx
Outdated
| if (paramName.includes('.')) { | ||
| tuneNodeConfig(nodeId, buildParamUpdate(paramName, value)); | ||
| } else { | ||
| tuneNode(nodeId, paramName, value); | ||
| } |
There was a problem hiding this comment.
🟡 Validation silently skipped for dot-notation parameter paths
validateParamValue (MonitorView.tsx:476) looks up the schema property via schema?.properties?.[paramKey]. When paramKey is a dot-notation path like "properties.score", the lookup fails because schema properties are keyed by their simple name ("score"), not by dot paths. The function returns null (meaning "valid"), so all dot-path parameter changes bypass validation entirely in both stableOnParamChange and handleRightPaneParamChange.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Acknowledged. This is a pre-existing limitation of validateParamValue — it does a flat schema?.properties?.[paramKey] lookup, which won't find the schema property when paramKey is a dot-notation path like "properties.score". The validation returns null (valid), so dot-path changes bypass validation.
In practice this is low-risk: the server-side node will reject truly invalid values, and the schema property types constrain the control UI (toggles only send booleans, sliders only send numbers within min/max bounds). But it's worth noting as a follow-up improvement — validateParamValue could split the dot-path and walk the schema tree to find the correct JsonSchemaProperty, or the controls could pass the original property key alongside the path for validation lookup.
stableOnParamChange and handleRightPaneParamChange now read the current atom state and deep-merge the partial update before calling tuneNodeConfig, matching the useTuneNode pattern. This prevents shallow-merge from clobbering sibling nested properties (e.g. updating properties.score no longer drops properties.show). Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
| return; | ||
| } | ||
|
|
||
| tuneNode(nodeId, paramName, value); | ||
| // Dot-notation paths (e.g. "properties.show") need buildParamUpdate to | ||
| // produce the correct nested UpdateParams payload. We deep-merge the | ||
| // partial update into the current atom state so sibling nested | ||
| // properties are preserved (e.g. updating properties.score doesn't | ||
| // clobber properties.show). This mirrors useTuneNode's deep-merge | ||
| // pattern. | ||
| if (paramName.includes('.')) { | ||
| const partial = buildParamUpdate(paramName, value); | ||
| const k = nodeKey(selectedSessionId ?? '', nodeId); | ||
| const current = defaultSessionStore.get(nodeParamsAtom(k)); | ||
| tuneNodeConfig(nodeId, deepMerge(current, partial)); | ||
| } else { | ||
| tuneNode(nodeId, paramName, value); | ||
| } | ||
| }, | ||
| [toast, tuneNode] | ||
| [toast, tuneNode, tuneNodeConfig, selectedSessionId] | ||
| ); |
There was a problem hiding this comment.
🚩 Two different tuneNodeConfig implementations with subtly different semantics
The codebase now has two tuneNodeConfig functions with different behavior:
useSession.tuneNodeConfig(useSession.ts:71-93): shallow-merges config into atom viawriteNodeParams, sends config as-is to server.useTuneNode.tuneNodeConfig(useTuneNode.ts:30-58): deep-merges config into atom, sends original partial config to server.
BooleanToggleControl and TextInputControl use the useTuneNode variant (correct for nested params), while stableOnParamChange and handleRightPaneParamChange in MonitorView.tsx use useSession's variant with a manual pre-deep-merge workaround. This creates a semantic split where the same node can receive different server payloads depending on whether the change came from a slider vs. a toggle. Consider unifying the nested-path handling behind a single function to avoid divergence.
(Refers to lines 906-931)
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Fixed in 2a922cf. All dot-notation path updates now go through useTuneNode.tuneNodeConfig (aliased as tuneNodeConfigDeep in MonitorView), which is the single function implementing deep-merge-locally + send-partial-to-server. The useSession.tuneNodeConfig remains for full-config updates (compositor's stableOnConfigChange), which is the correct semantic for that use case (send entire config). The split is now clean: useTuneNode for partial nested updates, useSession for full-config replacements.
- stableOnParamChange and handleRightPaneParamChange now use useTuneNode.tuneNodeConfig (aliased as tuneNodeConfigDeep) for dot-notation paths. This correctly deep-merges locally into the atom and sends only the partial update to the server, matching the pattern used by BooleanToggleControl and TextInputControl. - NumericSliderControl now uses readByPath for the initial prop fallback value so nested paths (e.g. properties.score) resolve correctly instead of falling through to schema.default. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
| const handleToggle = useCallback(() => { | ||
| setChecked((prev) => { | ||
| const next = !prev; | ||
| tuneRef.current(nodeId, buildParamUpdate(config.path, next)); | ||
| return next; | ||
| }); | ||
| }, [nodeId, config.path]); |
There was a problem hiding this comment.
🟡 Side effect inside useState updater causes duplicate WebSocket messages in strict mode
In BooleanToggleControl.handleToggle, tuneRef.current() (which sends a WebSocket message and synchronously writes to a Jotai atom) is called inside a setChecked state updater function. React requires state updaters to be pure. In React 18 strict mode (development), updater functions are invoked twice to detect impurities, causing two identical tunenodeasync WebSocket messages per toggle click. Additionally, synchronously updating external state (Jotai atom via sessionStore.set) from within a useState updater can cause unexpected re-render scheduling.
| const handleToggle = useCallback(() => { | |
| setChecked((prev) => { | |
| const next = !prev; | |
| tuneRef.current(nodeId, buildParamUpdate(config.path, next)); | |
| return next; | |
| }); | |
| }, [nodeId, config.path]); | |
| const handleToggle = useCallback(() => { | |
| const next = !checked; | |
| setChecked(next); | |
| tuneRef.current(nodeId, buildParamUpdate(config.path, next)); | |
| }, [nodeId, config.path, checked]); |
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Fixed in a5a925d. Moved the tuneRef.current() call out of the setChecked updater so it's no longer inside a function that React expects to be pure. The handler now reads checked directly and calls setChecked(next) + tuneRef.current(...) sequentially.
| if (schema?.path) { | ||
| onParamChange(node.id, schema.path, value); | ||
| } else { | ||
| onParamChange(node.id, key, value); | ||
| } |
There was a problem hiding this comment.
🔴 DesignView's writeNodeParam stores dot-notation paths as flat keys, breaking readByPath
The PR modifies shared components (InspectorPane.tsx, useNumericSlider.ts) to read params via readByPath (nested traversal) and to send dot-notation path overrides (e.g., "properties.score") as paramName to onParamChange. MonitorView is correctly updated to handle these via buildParamUpdate + tuneNodeConfigDeep. However, DesignView's handleParamChange at ui/src/hooks/usePipeline.ts:106-107 calls writeNodeParam(nodeId, paramName, value) which does { ...current, [key]: value } — storing the dot-path as a literal flat key (e.g., { "properties.score": 42 }) rather than a nested structure. Since readByPath traverses nested objects (obj.properties.score), the stored value can't be read back, causing the UI to revert to the default after each change. This affects sliders on ConfigurableNode cards and all field types in the InspectorPane when a schema property has a path override.
Prompt for agents
The InspectorPane.handleInputChange sends schema.path (a dot-notation string like "properties.score") as paramName to onParamChange. In MonitorView, this is handled correctly by stableOnParamChange (which uses buildParamUpdate + tuneNodeConfigDeep for dot paths). However, in DesignView, the onParamChange callback is usePipeline.handleParamChange (ui/src/hooks/usePipeline.ts:106) which calls writeNodeParam — a function that stores the paramName as a flat key via { ...current, [key]: value }. Since the PR's read path uses readByPath (nested traversal), the flat key can't be read back.
To fix this, DesignView's handleParamChange in usePipeline.ts needs to detect dot-notation paths and use buildParamUpdate + deepMerge (similar to how MonitorView's stableOnParamChange does it). The same pattern used in MonitorView.tsx:922-925 should be replicated: check if paramName.includes('.'), and if so, build the nested update with buildParamUpdate and deep-merge it into the atom. Also update writeNodeParam or add a new write helper that handles nested paths.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Fixed in a5a925d. usePipeline.handleParamChange now detects dot-notation paths and uses buildParamUpdate + deepMerge + writeNodeParams instead of writeNodeParam (which stores dot-paths as literal flat keys like { "properties.score": 42 }). This ensures readByPath can find the nested value in design view.
…t-paths - BooleanToggleControl.handleToggle: move tuneRef.current() call out of the setChecked updater to avoid duplicate WebSocket messages in React strict mode (updaters must be pure). - usePipeline.handleParamChange: detect dot-notation paths and use buildParamUpdate + deepMerge + writeNodeParams instead of writeNodeParam (which stores dot-paths as literal flat keys). This ensures readByPath can find nested values in design view. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Fix stale closure in handleToggle: use checkedRef to avoid lost toggles when two clicks fire before React re-renders. - Add role="switch" and aria-checked to ToggleTrack for a11y. - Extract dispatchParamUpdate helper into controlProps.ts to deduplicate the dot-path branching in MonitorView (×2) and usePipeline. - Flush text input debounce on blur and unmount so the last typed value is sent instead of silently dropped. - Consolidate duplicate styled components: ConfigurableNode now imports ControlLabel/ControlLabelText/ControlDescription from SchemaControls instead of defining identical copies. - Memoize extractSliderConfigs/extractToggleConfigs/extractTextConfigs with useMemo in ConfigurableNode. - Rename SliderGroup → ControlGroup (now wraps all control types). - Add tests for dispatchParamUpdate. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… keystroke flushDebounce previously closed over `text` state, causing its identity to change on every keystroke. The useEffect cleanup then fired the old flushDebounce (with stale text), breaking the debounce entirely and sending stale values to the server. Fix: read from textRef.current instead of the `text` closure so flushDebounce identity stays stable across keystrokes. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ale closure Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…S message on blur Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
🐛 1 issue in files not directly in the diff
🐛 Shallow merge of atom overrides clobbers nested sibling properties in YAML generation and localStorage (ui/src/hooks/pipelineBuilder.ts:133)
The new nested-path dispatch (dispatchParamUpdate → deepMerge) stores overrides as nested objects in the Jotai atom (e.g. { properties: { score: 4 } }). However, buildPipelineForYaml at ui/src/hooks/pipelineBuilder.ts:133 uses a shallow spread ({ ...params, ...overrides }) to merge base params with atom overrides. This means when node.data.params contains nested siblings like { properties: { score: 0, show: true } } and the atom override is { properties: { score: 4 } }, the shallow spread produces { properties: { score: 4 } } — silently dropping show: true. This affects YAML preview, YAML export (handleExportYaml), and localStorage draft saving (ui/src/hooks/usePipeline.ts:355). The same pattern exists in ui/src/views/MonitorView.tsx:962 for the monitor YAML display. The fix is to replace the shallow spread with deepMerge(node.data.params || {}, overrides || {}).
View 4 additional findings in Devin Review.
Debug
| tuneRef.current(nodeId, buildParamUpdate(config.path, next)); | ||
| }, [nodeId, config.path]); | ||
|
|
||
| const disabled = !sessionId; |
There was a problem hiding this comment.
🚩 Toggle/text controls only functional in Monitor view (disabled in Design view)
Both BooleanToggleControl and TextInputControl set disabled = !sessionId (SchemaControls.tsx:234,346). In the Design view (usePipeline), nodes don't have a sessionId, so all toggle and text controls are disabled. Meanwhile, numeric sliders work in both views because they receive onParamChange as a prop. This means tunable boolean/string params appear on design-view node cards but are non-interactive — only the InspectorPane allows editing them. This appears intentional (the module docstring says "Schema-driven controls for Monitor view node cards") but creates a UX asymmetry with sliders.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Add server-side infrastructure for plugins to declare tunable parameters discovered at runtime (e.g. Slint component properties after .slint compilation). The UI merges these runtime schemas with static per-kind schemas and renders controls automatically — no manual YAML needed. Architecture: - ProcessorNode::runtime_param_schema() hook (default None) - DynamicEngine stores per-instance schemas, cleans on remove - GetRuntimeSchemas query + Session/API forwarding - Pipeline.runtime_schemas field in WS + REST responses - UI deepMergeSchemas() merges runtime into static schema - Native SDK: FFI function pointer + macro trampolines - Host wrapper bridges FFI to ProcessorNode trait Slint plugin: - Discovers properties from ComponentDefinition at init - Maps Bool/Float/Int/String to JSON Schema types - Returns runtime schema with tunable: true and path overrides - Limitation: .slint files assumed static for node lifetime Includes unit tests for deepMergeSchemas and all extractors. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
staging-devin-ai-integration
bot
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Bump NATIVE_PLUGIN_API_VERSION from 3 to 4 to match the new get_runtime_param_schema field added to CNativePluginAPI. Without this, old plugins compiled against v3 would pass the version check but cause UB when the host reads past their struct. - Fix Slint property discovery: use ValueType::Number (not Float/Int) to match the actual slint_interpreter API. - Update C header example to v4 with the new field and struct member. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
| /// `CPixelFormat`, and source node support (`get_source_config`, `tick`). | ||
| pub const NATIVE_PLUGIN_API_VERSION: u32 = 3; | ||
| /// v4: Added `get_runtime_param_schema` for dynamic runtime parameter discovery. | ||
| pub const NATIVE_PLUGIN_API_VERSION: u32 = 4; |
There was a problem hiding this comment.
🚩 API version bump (v3→v4) is a breaking change for all existing native plugins
The native plugin API version was bumped from 3 to 4 (sdks/plugin-sdk/native/src/types.rs:17), and the host performs a strict equality check (crates/plugin-native/src/lib.rs:108). This means all existing v3 native plugins will fail to load with the updated host, even though they don't need the new get_runtime_param_schema field. Since the field is Option<fn ptr> and appended at the end of the struct, an alternative approach would be to keep backward compatibility by accepting v3 plugins (treating the missing field as None). Whether the break is intentional should be confirmed.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
The break is intentional. The user explicitly stated "if it helps simplifying we can still make breaking changes, goal is a clean and future ready architecture." All native plugins in-tree are compiled from the same source tree and will pick up the new version automatically. Backward-compatible version range checks could be added later if needed, but for now strict equality keeps things simple and safe.
| export const deepMergeSchemas = ( | ||
| base: JsonSchema | undefined, | ||
| runtime: JsonSchema | undefined | ||
| ): JsonSchema => { | ||
| if (!runtime) return base ?? {}; | ||
| if (!base) return runtime; | ||
|
|
||
| const baseProps = base.properties ?? {}; | ||
| const runtimeProps = runtime.properties ?? {}; | ||
|
|
||
| return { | ||
| ...base, | ||
| properties: { | ||
| ...baseProps, | ||
| ...runtimeProps, | ||
| }, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
📝 Info: deepMergeSchemas property-level merge semantics are shallow within each property
The deepMergeSchemas function in ui/src/utils/jsonSchema.ts:238-261 does a two-level merge: deep at the properties map level, but shallow (spread) at the individual property level. So { ...baseEntry, ...runtimeEntry } means runtime fields win over base fields with the same key. This is correct for the intended use case (runtime adds tunable: true and path to existing base entries with minimum, maximum, default). The test at jsonSchema.test.ts:289-296 explicitly verifies this behavior. No issue found, just documenting the merge semantics.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
This is intentional — runtime schemas from runtime_param_schema() are self-contained (they include type, tunable, path, and description). In practice, runtime-discovered properties don't exist in the static base schema at all (they're new entries), so there's no overlap to merge at the individual property level. The Slint plugin's discover_properties generates complete property definitions. If a future use case needs property-level deep merge (e.g. adding just tunable: true to an existing static property), that can be added, but for now the simpler approach avoids complexity.
…am_schema FFI Introduce a dedicated CSchemaResult struct with explicit json_schema field instead of repurposing CResult.error_message as a success payload. This eliminates the semantic mismatch where a field named error_message carried success data. Also synchronize API version history comments between the Rust SDK (types.rs) and C header (streamkit_plugin.h) to document v1/v2/v3/v4 additions consistently. Add missing CSourceConfig, CTickResult typedefs and NULL field initializers to the C example plugin. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Change deepMergeSchemas from shallow property merge to property-level deep merge. When a runtime property key collides with a base property key, the merged entry now preserves base fields (minimum, maximum, default) that the runtime entry doesn't override. Also switch toBe to toEqual in merge tests for safety against future cloning, and add test for preserved min/max bounds on collision. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Clarify that runtime_param_schema() is called once after initialize() and cached for the node's lifetime. No refresh mechanism exists; if the underlying configuration changes (e.g. a different .slint file), the node must be re-created. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Consolidate the text input debounce constant alongside PARAM_THROTTLE_MS in the shared timing constants file instead of defining it locally. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Schema-driven controls (toggles, sliders, text inputs) on ConfigurableNode cards are now collapsed by default behind a clickable toggle bar showing the control count. Users expand individual nodes on demand to reveal controls, keeping the pipeline DAG clean and readable. - Add ControlsToggleBar with animated CSS chevron - Controls collapsed by default; per-node expand/collapse state - Accessible: aria-expanded, aria-label on toggle button - nodrag/nopan on toggle to prevent accidental drags - No changes to auto-layout, Inspector pane, or LIVE indicators Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Replace CSS border triangle with inline SVG chevron for precise vertical alignment with label text - Remove e.stopPropagation() so clicking the toggle also selects the node in React Flow, bringing it to front naturally - Add elevateNodesOnSelect to FlowCanvas so selected/expanded nodes always render above neighbors - Adjust toggle bar padding to align with node content Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…me schema arrival
Three bugs identified by Devin Review:
1. Server-side: handle_tune_node_fire_and_forget replaced entire
node.params with the partial UpdateParams payload, destroying
sibling keys (e.g. toggling properties.show lost fps, width,
properties.name). Now deep-merges into existing params.
2. Client-side: writeNodeParams used shallow merge ({ ...current,
...cleaned }), so echo-back of partial nested updates from the
server overwrote locally deep-merged siblings. Now uses deepMerge.
3. MonitorView: topoKey did not include runtime_schemas, so a
RuntimeSchemasUpdated WS event arriving after initial topology
build was silently ignored. Now includes runtime schema keys in
the signature so the topology effect re-runs and merges the
discovered schemas into node data.
Includes unit tests for deep_merge_json (Rust).
Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Move finish_call() after pointer reads in runtime_param_schema FFI wrapper to prevent use-after-free if destroy_instance runs (critical) - Remove unused _lib Arc clone in the same function - Deduplicate defByKind.get(apiNode.kind) lookups in MonitorView - Document writeNodeParam as flat-key-only (vs deep-merge writeNodeParams) - Log warning when RuntimeSchemaUpdate is dropped due to full channel - Add comment explaining topoKey immutability contract for schema content - Extract shared __plugin_get_runtime_param_schema and __plugin_destroy_instance into __plugin_shared_ffi! helper macro Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…annel - Lift controlsExpanded state to a module-level Map<nodeId, boolean> so it survives topology rebuilds (e.g. when runtime schemas arrive) - Fix validateParamValue to resolve dot-paths by walking nested schema properties and use runtime-merged schema instead of static registry - Switch runtime schema subscriber channel from bounded to unbounded (schema discovery is one-per-node, low-frequency; bounded channel risked silently dropping the one-time notification) - Document partial-delta broadcast contract on NodeParamsChanged - Document error_to_c naming inconsistency in shared FFI macro Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
…t/order Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
…indicator - Slint plugin: discover_properties now reads initial property values from the live ComponentInstance and includes them as 'default' in the runtime JSON Schema. Fixes toggles showing incorrect initial state (e.g. clock_running showing OFF when the Slint component starts ON). - Replace repeated per-control LIVE badges with a single pulsing dot on the controls toggle bar. Reduces visual noise when many controls are expanded while still communicating live-tunability. - Remove showLiveIndicator/isTunable props from individual controls (NumericSliderControl, BooleanToggleControl, TextInputControl) and the LiveIndicator wrapper component in SchemaControls. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Slint's interpreter normalizes property identifiers to kebab-case internally (e.g. clock_running → clock-running). discover_properties now converts names back to snake_case so they match the rest of the StreamKit stack (YAML params, JSON UpdateParams, set_properties). This fixes the clock_running toggle defaulting to OFF in Monitor View — the runtime schema was emitting 'clock-running' with default=true but the UI was looking up 'clock_running', falling through to false. Also unifies Stream View and Monitor View controls: - Added schemaToControlConfigs() to convert runtime JSON schemas into ControlConfig entries (same type used by YAML client.controls). - OverlayControls now accepts an optional Pipeline prop and generates schema-driven controls from runtime_schemas, merging them with YAML controls (YAML takes precedence for label/group overrides). - StreamView fetches the live pipeline via REST API and passes it to OverlayControls so both views render from the same source of truth. - Added fetchPipeline() to sessions service. - Added unit tests for schemaToControlConfigs covering all control types, label derivation, enum exclusion, and edge cases. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The clock_running toggle showed OFF in both Monitor and Stream Views because the property was not listed in the YAML params (relying on the Slint component's built-in default of true). Unlike 'show' (which was explicitly set in lower_third's params), clock_running had no value for the UI to read from pipeline params — it fell through to the runtime schema's 'default' field, which requires the Slint plugin to be rebuilt with the initial_value reading code from the previous commit. Fix: - Add clock_running: true to scoreboard node params (immediate fix, same pattern as show: true in lower_third) - Add 'Clock Running' toggle to client.controls so Stream View also renders it (grouped under Scoreboard) This makes the toggle work immediately without requiring a plugin rebuild. The runtime schema fix (discover_properties reading initial values) remains the correct long-term solution for properties that pipeline authors don't explicitly list in YAML. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
validateParamValue previously split dot-notation paramKeys (e.g. "properties.show") and walked the schema hierarchy, which failed for runtime-discovered properties stored as flat keys with a `path` field. Now uses flat-key lookup first, then scans for a property whose `path` field matches the paramKey. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The sync handle_tune_node handler was still using full replacement (node.params = Some(durable_params)) while the async handler used deep_merge_json. This caused server-side pipeline model divergence: sync path lost sibling keys on partial nested updates, async path preserved them. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
… NaN warning - StreamView now reads pipeline (including runtime_schemas) from the Zustand session store instead of a one-shot REST fetch with a 1.5s delay. This eliminates the race where late-arriving schemas would be missed and controls wouldn't appear. - Added SAFETY comment in wrapper.rs documenting the thread-local pointer lifetime constraint for CSchemaResult.json_schema. - Added tracing::warn for NaN/Infinity Slint property values that are silently dropped by serde_json::Number::from_f64. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Add trailing comma on Number match arm (rustfmt). - Remove fetchPipeline from sessions.ts — no longer used since StreamView reads pipeline from the WS-driven session store. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ Redundant double deep-merge in useTuneNode wastes work and obscures intent (ui/src/hooks/useTuneNode.ts:38-40)
In useTuneNode.ts, the tuneNodeConfig callback manually deep-merges config into the current atom value at line 39, then passes the already fully-merged result to writeNodeParams at line 40. However, writeNodeParams (ui/src/stores/sessionAtoms.ts:77-84) internally reads the same atom again and performs its own deepMerge(current, cleaned). The first merge is entirely wasted work — its output is used as the source argument of the second merge, which re-reads the same current value and produces the identical result.
The correct fix is to pass the partial config directly to writeNodeParams and let it handle the single deep-merge, i.e. writeNodeParams(nodeId, config, sessionId). The same redundant pattern also exists in ui/src/hooks/usePipeline.ts:112-114.
View 17 additional findings in Devin Review.
Debug
…peline Both useTuneNode and usePipeline's handleParamChange were manually deep-merging partial configs into the current atom value before passing the result to writeNodeParams, which deep-merges again internally. The first merge was entirely redundant. Now both callers pass the partial config directly to writeNodeParams, which is the single merge point. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
2 participants
Summary
Introduces schema-driven controls for dynamic pipeline nodes (e.g. Slint plugins) with a collapsible UI pattern that balances control access with pipeline DAG readability.
Key changes
Collapsible controls pattern — Schema-driven controls on node cards are collapsed by default behind a "N controls ▸" toggle bar. Expanded state persists across topology rebuilds via a module-level
Map<nodeId, boolean>. Clicking the toggle also selects the node (nostopPropagation), andelevateNodesOnSelectensures expanded nodes render above neighbors.Runtime schema enrichment — Nodes can declare
runtime_param_schema()to expose dynamically discovered properties (e.g. Slint component properties). These are merged with staticparam_schemaviadeepMergeSchemasand surfaced as tunable controls.Slint property name normalization — Slint's interpreter normalizes identifiers to kebab-case internally (
clock_running→clock-running).discover_propertiesnow converts back to snake_case so names match the rest of the StreamKit stack (YAML params, UpdateParams, set_properties).Unified Stream/Monitor View controls — Both views now derive controls from the same WS-driven source of truth:
schemaToControlConfigs()converts runtime JSON schemas intoControlConfigentriesOverlayControlsaccepts an optionalPipelineprop, generates schema-driven controls fromruntime_schemas, and merges with YAMLclient.controls(YAML takes precedence for labels/groups)StreamViewreads pipeline from the Zustand session store (kept current by WS events), eliminating the previous one-shot REST fetch with a 1.5s delay that could miss late-arriving schemasDeep merge for params — Server-side (
websocket_handlers.rs, both sync and async handlers) and client-side (writeNodeParams) now deep-merge partialUpdateParamsinstead of replacing, preventing sibling keys from being destroyed on nested property toggles. Redundant double deep-merge inuseTuneNodeandusePipelinehas been removed —writeNodeParamsis the single merge point.FFI safety —
finish_call()moved after all pointer reads inruntime_param_schemawrapper, with SAFETY comment documenting the thread-local pointer lifetime constraint. Identical FFI implementations extracted into__plugin_shared_ffi!macro.Schema notification reliability —
try_sendfor schema discovery now logs a warning onFullsince schema discovery is a one-time bounded event and a dropped notification leaves the UI permanently stale.Lighter LIVE indicator — Replaced per-control
LIVEbadges with a single pulsing dot on the controls toggle bar.Dot-path validation —
validateParamValuenow resolves runtime-discovered properties stored as flat keys with apathfield, so dot-notation params likeproperties.showare validated correctly.NaN/Infinity warning — Slint plugin now logs a
tracing::warnwhen a property has a NaN/Infinity value thatserde_json::Number::from_f64silently drops.Review & Testing Checklist for Human
video_moq_slint_scoreboard.yml, verifyclock_runningtoggle shows ON by default in both Monitor View and Stream Viewshow) and verify other properties (e.g.home_score,clock_running) are not reset. Test both sync TuneNode and async TuneNodeAsync pathsNotes
schemaToControlConfigsutility generates labels by title-casing snake_case/kebab-case property keys (e.g.clock_running→ "Clock Running"). YAMLclient.controlsentries override these auto-generated labels when they target the same node+property.max-statementswarning on StreamView's main component is pre-existing and not introduced by this PR.topoKeyintentionally uses only runtime schema keys (not content) — relies on the documented immutability contract ofruntime_param_schema(). A comment has been added noting this.Link to Devin session: https://staging.itsdev.in/sessions/deecc42c0d344a9e898d12f85b4dad53
Requested by: @streamer45