fix(api): reject non-object params for audio::mixer in compile_dag#514
Conversation
When audio::mixer params is present but not a JSON object, compile_dag now returns an error instead of silently skipping num_inputs injection. Closes #472 Signed-off-by: streamkit-devin <devin@streamkit.dev>
🤖 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}
| if def.kind == "audio::mixer" && mode != EngineMode::Dynamic { | ||
| if let Some(ref p) = params { | ||
| if !p.is_object() { | ||
| return Err(format!( | ||
| "audio::mixer params must be an object, got {}", | ||
| value_type_name(p), | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
📝 Info: The new params-shape rejection is limited to DAG mixer auto-injection
This check runs only for audio::mixer nodes in DAG pipelines when mode != EngineMode::Dynamic, immediately before injecting num_inputs. That means dynamic DAG pipelines and all steps: pipelines preserve their previous compile-time behavior; dynamic audio::mixer params are still left to the node factory/engine path rather than being normalized here. This appears consistent with the existing dynamic-mode test and with AudioMixerConfig needing num_inputs only for stateless/oneshot pre-created pins (crates/nodes/src/audio/filters/mixer.rs:63-64).
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| Ok((name, Node { kind: def.kind, params, state: None })) | ||
| }) | ||
| .collect(); | ||
| .collect::<Result<IndexMap<_, _>, _>>()?; |
There was a problem hiding this comment.
📝 Info: Collecting into a Result preserves successful pipeline construction semantics
Changing the node-map construction from collecting bare (name, Node) pairs to collecting Result<(name, Node), String> only introduces early return on the new validation failure. The connection list and incoming counts are fully built before this point, and user_nodes.into_iter() is still consumed exactly once, so successful compilations keep the same node ordering and connection data as before.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #514 +/- ##
=======================================
Coverage 79.41% 79.41%
=======================================
Files 232 232
Lines 66904 66951 +47
Branches 1822 1908 +86
=======================================
+ Hits 53129 53171 +42
- Misses 13769 13774 +5
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Apply the non-object params rejection in all engine modes, not just non-dynamic. Dynamic pipelines now also fail at compile time instead of deferring to the node factory. - Include the node name in the error message for easier debugging with multiple mixer nodes. Signed-off-by: streamkit-devin <devin@streamkit.dev>
- Merge the two adjacent audio::mixer if-blocks into a single branch with the mode-gated injection nested inside. - Add a test pinning the fail-fast behavior for Some(Value::Null) params. Signed-off-by: streamkit-devin <devin@streamkit.dev>
2 participants
Summary
compile_dagnow returnsResult::Errwhen anyaudio::mixernode hasparamsthat is not a JSON object, in all engine modes — not just oneshot. Dynamic pipelines previously deferred this to the node factory; now they fail at compile time too."audio::mixer node 'my_mixer' params must be an object, got string").Some(Value::Null).Closes #472
Review & Validation
cargo test -p streamkit-api -- compiler::testspasses all 6 compiler testsNotes
Some(Value::Null)params (fromparams: nullor bareparams:in YAML) now errors as "got null" instead of silently skipping injection. This is intentional — null params is invalid for a mixer config and should surface early. A dedicated test pins this contract.audio::mixer-only. Generalizing to all node kinds is tracked in Validate params is an object for all node kinds in compile_dag #524.Link to Devin session: https://staging.itsdev.in/sessions/dd23569c931b41529fd43a49b50a35aa
Requested by: @streamer45
Devin Review
c65b88a(HEAD is839ad23)