new module structure#190
Conversation
GitLab Pipeline ActionGeneral informationLink to pipeline: https://gitlab.com/code0-tech/development/taurus/-/pipelines/2486110412 Status: Passed Job summariesdocs:previewDocumentation preview available at https://code0-tech.gitlab.io/-/development/telescopium/-/jobs/14127763595/artifacts/out/index.html |
There was a problem hiding this comment.
Pull request overview
Updates Taurus to newer tucana / code0-flow versions and adjusts the core/provider remote-runtime integration and flow compilation to match the updated protobuf types.
Changes:
- Bump workspace dependencies:
code0-flow0.0.32 -> 0.0.33,tucana0.0.68 -> 0.0.70. - Replace
ExecutionRequestwithActionExecutionRequestthroughout the remote execution interface and request building. - Switch remote response decoding to
ActionExecutionResponseand update flow compiler/test code to the newSubFlowparameter variant (currently left unimplemented).
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/taurus-provider/src/providers/remote/nats_remote_runtime.rs | Switches NATS remote decode type to ActionExecutionResponse, but currently leaves execution handling unimplemented. |
| crates/taurus-core/src/runtime/remote/mod.rs | Updates RemoteExecution to carry ActionExecutionRequest. |
| crates/taurus-core/src/runtime/engine/executor.rs | Builds ActionExecutionRequest for remote invocations. |
| crates/taurus-core/src/runtime/engine/compiler.rs | Updates parameter compilation for new SubFlow variant but currently panics via unimplemented!(). |
| crates/taurus-core/src/runtime/engine.rs | Updates test helpers for new parameter shape but currently panics via unimplemented!(). |
| Cargo.toml | Dependency bumps to pick up new module/proto structure. |
| Cargo.lock | Lockfile updates for the dependency bumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Result of Remote Response was empty.", | ||
| )), | ||
| } | ||
| unimplemented!("Taurus needs to handle text executions (issue nr #185)") |
There was a problem hiding this comment.
Leaving unimplemented!() in this runtime path will panic any time a remote node executes. Please replace this with actual response handling (or, at minimum, return a structured RuntimeError with an appropriate stable code/category) so production flows don't crash.
| unimplemented!("Taurus needs to handle text executions (issue nr #185)") | |
| Err(RuntimeError::new( | |
| "T-PROV-000003", | |
| "RemoteRuntimeUnsupportedResponse", | |
| "Remote runtime response handling is not implemented for this execution type.", | |
| )) |
| value: Some(NodeValue { | ||
| value: Some(node_value::Value::NodeFunctionId(node_id)), | ||
| value: Some(node_value::Value::SubFlow(unimplemented!( | ||
| "Taurus needs to handle SubFlows (issue nr #184)" | ||
| ))), |
There was a problem hiding this comment.
This test helper constructs a NodeParameter by executing unimplemented!(), which will panic immediately when the test builds its graph (before the engine even runs). Please construct a real node_value::Value::SubFlow representing a thunk/sub-flow (or refactor tests to the new parameter encoding) so the tests can execute.
| let decode_result = ActionExecutionResponse::decode(message.payload); | ||
| let _execution_result = match decode_result { | ||
| Ok(r) => r, |
There was a problem hiding this comment.
execute_remote decodes an ActionExecutionResponse but then discards it and never produces a Result<Value, RuntimeError> based on the response. This currently makes all remote executions unusable; please map the decoded response to Ok(Value) / Err(RuntimeError) (including handling missing/failed results) instead of ignoring it.
Resolves: #186