Conversation
…e error instead of panicing
…nto major-rework
…rate was referncing core lib
There was a problem hiding this comment.
Pull request overview
This PR refactors the Taurus execution engine into a new taurus-core crate, adds execution tracing and new reference-resolution behavior, and introduces a JSON-driven execution test suite that runs in CI.
Changes:
- Extract core execution/context/runtime logic into
crates/core(taurus-core) and update the Taurus service to consume it. - Add execution tracing infrastructure (trace frames + renderer) and expand reference handling (flow input, inputType, reference paths).
- Add an “execution suite” binary (
crates/tests) with example flows and wire it into the GitHub Actions workflow.
Reviewed changes
Copilot reviewed 33 out of 38 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| taurus/src/context/executor.rs | Removed old executor implementation (superseded by taurus-core). |
| taurus/src/context/context.rs | Removed old context implementation (superseded by taurus-core). |
| crates/core/src/lib.rs | New library root exporting context, debug, and runtime. |
| crates/core/src/context/mod.rs | Context module wiring for the new core crate. |
| crates/core/src/context/argument.rs | Updated runtime error import path for arguments. |
| crates/core/src/context/context.rs | New context with flow input, inputType storage, and reference path traversal. |
| crates/core/src/context/executor.rs | New executor with eager/lazy thunk evaluation and optional trace rendering. |
| crates/core/src/context/macros.rs | Updated macros to use runtime::error::RuntimeError. |
| crates/core/src/context/registry.rs | FunctionStore::default() now auto-populates all runtime functions. |
| crates/core/src/context/signal.rs | Updated signal to use the new runtime error type. |
| crates/core/src/debug/mod.rs | New debug module exports. |
| crates/core/src/debug/trace.rs | Data model for execution trace frames/edges/outcomes. |
| crates/core/src/debug/tracer.rs | Trace collector implementation (Tracer). |
| crates/core/src/debug/render.rs | Human-readable trace renderer. |
| crates/core/src/runtime/mod.rs | Runtime module root. |
| crates/core/src/runtime/error/mod.rs | Runtime error type now exposes name and message publicly. |
| crates/core/src/runtime/functions/mod.rs | Central function collection used to populate the registry. |
| crates/core/src/runtime/functions/array.rs | Updated list APIs to support lazy predicate/transform thunks and inputType iteration. |
| crates/core/src/runtime/functions/boolean.rs | Updated imports to use new runtime error module. |
| crates/core/src/runtime/functions/control.rs | Adds runtime trace labels for branching. |
| crates/core/src/runtime/functions/http.rs | Updates headers handling from list to struct. |
| crates/core/src/runtime/functions/number.rs | Fixes random range panic and adds tests for invalid ranges. |
| crates/core/src/runtime/functions/object.rs | New object helper functions (+ unit tests). |
| crates/core/src/runtime/functions/text.rs | Updated imports to use new runtime error module. |
| crates/core/Cargo.toml | Introduces taurus-core crate and its dependencies. |
| crates/core/.gitignore | Ignores core crate build output. |
| crates/taurus/src/config/mod.rs | Adds a configuration module for the Taurus service. |
| crates/taurus/src/main.rs | Switches Taurus service to use taurus-core executor/registry and enables tracing. |
| crates/taurus/Cargo.toml | Moves Taurus crate to workspace version/edition and depends on taurus-core. |
| crates/tests/Cargo.toml | Adds a tests binary crate for the execution suite. |
| crates/tests/src/main.rs | JSON-driven execution suite runner for validation flows. |
| crates/tests/README.md | Documentation for running/adding flows to the suite. |
| crates/tests/flows/01_return_object.json | Example flow file (response object). |
| crates/tests/flows/02_return_flow_input.json | Example flow file (flow input reference). |
| crates/tests/flows/03_for_each.json | Example flow file (inputType reference via for_each). |
| Cargo.toml | Expands workspace members and adds workspace deps (incl. taurus-core). |
| Cargo.lock | Dependency lockfile updates (workspace + tucana bump). |
| .github/workflows/build-and-test.yml | Runs both cargo test and the new execution suite in CI. |
Comments suppressed due to low confidence (8)
crates/core/src/runtime/functions/array.rs:188
filterinserts anInputTypeinto theContexteach iteration, but several early-return paths don't clear it (e.g.,Err(e) => return Signal::Failure(e)and returningothersignals). This leaves staleinput_typesentries that can affect subsequent executions. Consider clearinginput_typeon all exit paths (RAII guard /defer-style helper).
crates/core/src/runtime/functions/array.rs:453for_eachhas two issues here: (1) the error message says "map expects..." even though this isfor_each, and (2) if the thunk returns a non-Successsignal, the function returns early without callingctx.clear_input_type(input_type), leaving stale context state. Consider fixing the message and ensuring cleanup happens before any early return.
crates/core/src/runtime/functions/array.rs:724sortpopulates twoInputTypeentries (input_index0 and 1) but never clears them on the success path (and only clearsinput_type, notinput_type_next, on error paths). This can leak stale values into later reference lookups. Ensure both input types are cleared before returning, regardless of success/failure.
crates/core/src/runtime/functions/number.rs:389- The
random_numberfunction registers asstd::number::random_number, but this error message mentionsstd::math::randomand has a grammar typo ("then" -> "than"). Consider updating the message to reference the correct function name and fix the wording to make debugging clearer.
crates/taurus/src/main.rs:31 handle_messagealways enables tracing (with_trace = true), which will print a potentially large per-execution trace to stdout for every flow. This can significantly increase log volume and CPU in production. Consider gating tracing behind a config/env flag (e.g., only in development) or using structured logging instead of unconditionalprintln!.
crates/core/src/runtime/functions/array.rs:740sortruns the comparator thunk inside asort_byclosure but always returnsOrdering::Equal, then performs a secondsort_byusing a sequentialoutvector unrelated to the compared elements. This violatessort_by's comparator requirements (total, consistent ordering) and can yield incorrect / non-deterministic results. Consider computing the comparator result for the actual(a,b)pair and returningLess/Equal/Greaterdirectly from the firstsort_by(capturing failures to return after the sort), rather than sorting twice.
crates/core/src/runtime/functions/array.rs:834sort_reversehas the same issue assort: the firstsort_byexecutes the comparator thunk but always returnsOrdering::Equal, and the secondsort_byuses a sequentialoutvector that doesn't correspond to the compared elements. This can produce incorrect / non-deterministic ordering and violatessort_bycomparator expectations. Consider returning the ordering derived from the thunk directly within the comparator closure (capturing failures to handle after sorting).
crates/core/src/runtime/functions/array.rs:818sort_reversealso never clearsinput_type/input_type_nexton the success path, and on error paths only clearsinput_type. This can leak stale comparison operands into laterinputTypereferences. Ensure both entries are cleared before returning, regardless of outcome.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Raphael Götz <52959657+raphael-goetz@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR restructures Taurus into a workspace with a new taurus-core crate, adds an execution test-suite crate with exported flow fixtures, and introduces new reference/tracing logic in the executor and runtime functions (including support for new reference value targets like flow input and input-type references).
Changes:
- Introduces
taurus-core(context/executor/runtime/debug modules) and migrates Taurus to use it. - Adds a standalone execution suite (
crates/tests) that runs exported flow fixtures in CI. - Updates runtime functions to support new reference behaviors (e.g.,
InputType) and improves debugging/tracing; fixes invalid random ranges.
Reviewed changes
Copilot reviewed 33 out of 38 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| taurus/src/context/executor.rs | Removed old executor implementation (migrated into taurus-core). |
| taurus/src/context/context.rs | Removed old context implementation (migrated into taurus-core). |
| crates/tests/src/main.rs | Adds execution-suite runner for JSON flow fixtures. |
| crates/tests/flows/03_for_each.json | Adds a flow fixture exercising for_each + inputType references. |
| crates/tests/flows/02_return_flow_input.json | Adds a flow fixture exercising flowInput references. |
| crates/tests/flows/01_return_object.json | Adds a flow fixture for a simple response-returning chain. |
| crates/tests/README.md | Documents how to run/extend the execution-suite. |
| crates/tests/Cargo.toml | Adds tests crate dependencies and workspace integration. |
| crates/taurus/src/main.rs | Switches Taurus binary to taurus-core and enables tracing in execution. |
| crates/taurus/src/config/mod.rs | Introduces a config module for Taurus startup configuration. |
| crates/taurus/Cargo.toml | Uses workspace version/edition and adds dependency on taurus-core. |
| crates/core/src/runtime/mod.rs | Adds runtime module boundary for functions and errors. |
| crates/core/src/runtime/functions/text.rs | Updates imports to use runtime::error::RuntimeError. |
| crates/core/src/runtime/functions/object.rs | Adds new object stdlib functions + unit tests. |
| crates/core/src/runtime/functions/number.rs | Updates error import; changes random range handling + adds tests. |
| crates/core/src/runtime/functions/mod.rs | Adds runtime function collection entry-point. |
| crates/core/src/runtime/functions/http.rs | Changes HTTP headers representation to Struct and updates validation. |
| crates/core/src/runtime/functions/control.rs | Adds runtime trace labels for branch selection. |
| crates/core/src/runtime/functions/boolean.rs | Updates imports to use runtime::error::RuntimeError. |
| crates/core/src/runtime/functions/array.rs | Adds lazy thunk support for list ops; adds InputType injection and tracing. |
| crates/core/src/runtime/error/mod.rs | Makes RuntimeError.name/message public for suite output. |
| crates/core/src/lib.rs | Exposes context, debug, and runtime modules. |
| crates/core/src/debug/tracer.rs | Adds tracer implementation collecting execution frames. |
| crates/core/src/debug/trace.rs | Adds trace data model (frames/edges/outcomes). |
| crates/core/src/debug/render.rs | Adds ANSI tree renderer for execution traces. |
| crates/core/src/debug/mod.rs | Exposes debug submodules. |
| crates/core/src/context/signal.rs | Switches to runtime::error::RuntimeError. |
| crates/core/src/context/registry.rs | FunctionStore::default() now populates via runtime::functions::collect(). |
| crates/core/src/context/mod.rs | Adds context module re-exports. |
| crates/core/src/context/macros.rs | Updates macros to use runtime::error::RuntimeError. |
| crates/core/src/context/executor.rs | New executor: reference resolution, thunk handling, and optional trace generation. |
| crates/core/src/context/context.rs | New context: stores results, flow input, input types, and supports reference paths. |
| crates/core/src/context/argument.rs | Switches to runtime::error::RuntimeError. |
| crates/core/Cargo.toml | Introduces taurus-core crate manifest. |
| crates/core/.gitignore | Ignores crate-local build artifacts. |
| Cargo.toml | Converts repo into workspace; bumps tucana and adds shared deps. |
| Cargo.lock | Updates lockfile for new workspace crates and dependency bump. |
| .github/workflows/build-and-test.yml | Runs cargo test and the new execution-suite in CI. |
Comments suppressed due to low confidence (12)
crates/core/src/runtime/functions/array.rs:409
- The argument-shape error message in
for_eachsaysmap expects ..., which is misleading when debugging failures. Update the message to referencefor_eachand its expected arg names/modes.
crates/core/src/runtime/functions/array.rs:184 std::list::filtersets anInputTypeentry in the context, but several early-return paths skipctx.clear_input_type(input_type)(e.g., when the predicate returns non-bool or whenrunreturns Failure/Return/Respond/Stop). This can leave stale input_type values in the context for subsequent executions.
crates/core/src/runtime/functions/array.rs:716sortexecutes the comparator thunk inside asort_byclosure but always returnsOrdering::Equal. The standard library is free to call the comparator any number of times and in any order, so the collectedsignals/outsequence will be nondeterministic and may not match the comparisons performed by the secondsort_by, producing incorrect ordering. Consider performing a single sort where each comparison calls the comparator thunk and returnsOrderingdirectly (and propagate errors deterministically, e.g., by using a custom stable sort implementation that can short-circuit on failure).
crates/core/src/runtime/functions/array.rs:679- The argument-shape error message in
sortsaysmap expects ..., which is misleading when debugging. Update the message to referencesortand the comparator arg modes (array: eager, comparator: lazy thunk).
crates/core/src/runtime/functions/number.rs:396 rand::random_range(min..=max)uses an inclusive range, but the existing test asserts the result is< max(and callers often expect a half-open range for floats). This can make tests flaky (e.g., returning exactlymax) and may be a behavior change from the previous integer-based implementation. Consider using a half-open range (min..max) for floats (handlingmin==maxseparately), or update tests/contract to allow<= max.
crates/core/src/runtime/functions/array.rs:433std::list::for_eachreturns early on Failure/Return/Respond/Stop without clearing the insertedInputTypeentry, so the context can retain staleinput_typevalues. Clear the input_type on all exit paths (e.g., via a scope guard /defer-style helper).
crates/core/src/runtime/functions/number.rs:389- The
randomerror message has a few issues: grammar uses "then" instead of "than", and it referencesstd::math::randomeven though this handler is registered asstd::number::random_number. Consider correcting the wording to avoid confusing users.
crates/core/src/runtime/functions/number.rs:811 - Test name has typos:
test_random_range_fist_bigger_then_secondshould be...first_bigger_than_second("fist"/"then"). Renaming improves readability and searchability.
crates/core/src/runtime/functions/array.rs:810 sort_reversehas the same nondeterminism issue assort: it runs the comparator thunk inside asort_bythat always returnsOrdering::Equal, so the number/order of thunk executions (and thusout) is unspecified and can lead to incorrect ordering in the subsequent sort. Consider a single deterministic comparison path (custom sort) that calls the comparator and returnsOrderingdirectly.
crates/core/src/runtime/functions/array.rs:740sortandsort_reverseinsert twoInputTypeentries (input_index0 and 1), but cleanup only ever clearsinput_type(index 0) and never clearsinput_type_next(index 1), and there is no cleanup on the success path. This will leave stale input_type values inContextafter sorting.
crates/taurus/src/main.rs:31handle_messagealways enables tracing (with_trace=true), which will print a full execution trace for every flow execution. This is likely too noisy/expensive for normal operation; wire this to configuration (env var / log level) so trace rendering is only enabled when debugging.
crates/core/src/runtime/functions/array.rs:773- The argument-shape error message in
sort_reversesaysmap expects ..., which is misleading. Update it to referencesort_reverseand its expected args (array eager, comparator lazy thunk).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| pub fn get_current_node_id(&mut self) -> i64 { |
| pub fn run_tests(&self) { | ||
| for case in self.cases.clone() { | ||
| match case.run() { | ||
| CaseResult::Success => print_success(&case), | ||
| CaseResult::Failure(input, result) => print_failure(&case, &input, result), | ||
| } | ||
| } | ||
| } |
| pub fn execute(&self, start_node_id: i64, ctx: &mut Context, with_trace: bool) -> Signal { | ||
| let mut tracer = Tracer::new(); | ||
|
|
||
| let (signal, _root_frame) = self.execute_call(start_node_id, ctx, &mut tracer); | ||
|
|
||
| if with_trace && let Some(run) = tracer.take_run() { | ||
| println!("{}", crate::debug::render::render_trace(&run)); | ||
| } |
| { | ||
| "name": "02_return_object", | ||
| "description": "This flow expects the same output value as the input", |
| let node = match self.nodes.get(&node_id) { | ||
| Some(n) => n.clone(), | ||
| None => { | ||
| let err = | ||
| RuntimeError::simple("NodeNotFound", format!("Node {} not found", node_id)); | ||
| return (Signal::Failure(err), 0); | ||
| } | ||
| }; | ||
|
|
||
| let entry = match self.functions.get(node.runtime_function_id.as_str()) { | ||
| Some(e) => e, | ||
| None => { | ||
| let err = RuntimeError::simple( | ||
| "FunctionNotFound", | ||
| format!("Function {} not found", node.runtime_function_id), | ||
| ); | ||
| return (Signal::Failure(err), 0); | ||
| } |
| if let Kind::StructValue(struct_value) = &kind { | ||
| match struct_value.fields.get(part) { | ||
| Some(x) => { | ||
| curr = x.clone(); | ||
| } | ||
| None => return ContextResult::NotFound, | ||
| } |
| taurus_core::context::signal::Signal::Success(value) => { | ||
| let json = to_json_value(value); | ||
| if json == input.clone().expected_result { | ||
| return CaseResult::Success; | ||
| } else { | ||
| return CaseResult::Failure(input, json); | ||
| } | ||
| } | ||
| taurus_core::context::signal::Signal::Return(value) => { | ||
| let json = to_json_value(value); | ||
| if json == input.clone().expected_result { | ||
| return CaseResult::Success; | ||
| } else { | ||
| return CaseResult::Failure(input, json); | ||
| } | ||
| } | ||
| taurus_core::context::signal::Signal::Respond(value) => { | ||
| let json = to_json_value(value); | ||
| if json == input.clone().expected_result { | ||
| return CaseResult::Success; | ||
| } else { | ||
| return CaseResult::Failure(input, json); | ||
| } | ||
| } |
| "name": "03_for_each", | ||
| "description": "This flow validates the functionality of the refernece type 'input_type'", | ||
| "inputs": [ |
| "flow": { | ||
| "starting_node_id": "1", | ||
| "node_functions": [ | ||
| { |
| )); | ||
|
|
||
| let edge_child_prefix = format!("{}{}", prefix, if last { " " } else { "│ " }); | ||
| render_frame(*child_id, by_id, &edge_child_prefix, true, out); |
Resolves: #97, #98, #113, #117