Merged
Conversation
branchseer
changed the title
branchseer
force-pushed
the
02-13-feat_new_reporter_implementation
branch
2 times, most recently
from
153ff9b to
0340b6c
Compare
Extract the query planning logic from ExecutionPlan::plan into a new plan_query method that accepts QueryPlanRequest and returns Result<ExecutionGraph, Error> directly. Session::plan_from_cli now returns ExecutionGraph instead of ExecutionPlan, and callers wrap it via the new ExecutionPlan::from_execution_graph constructor. Also narrows RunCommand::into_plan_request to into_query_plan_request, since it always produced a Query variant.
Replace the flat Reporter trait with ExecutionEvent/ExecutionId dispatch with a three-phase typestate hierarchy: GraphExecutionReporterBuilder → GraphExecutionReporter → LeafExecutionReporter. Split into PlainReporter (standalone, for single-leaf synthetic execution) and LabeledReporter family (graph-aware, with summary aggregation and shared state via Rc<RefCell>). Extract execute_spawn as a free function reusable from both graph execution and execute_synthetic. Simplify plan_synthetic to return SpawnExecution directly. Remove ExecutionEvent, ExecutionEventKind, ExecutionId, and the CycleDetected cache variant (cycles are now graph-level errors via ExecutionAborted).
Add StdinSuggestion enum to LeafExecutionReporter trait so reporters can suggest whether child processes should inherit stdin or use /dev/null. PlainReporter always suggests inherited stdin (single synthetic execution). LabeledReporter suggests inherited only when the graph has exactly one spawn leaf (computed via count_spawn_leaves at build time). The actual stdin is inherited only when BOTH conditions hold: - Reporter suggests Inherited - Task has no cache metadata (caching disabled) This prevents non-deterministic user input from corrupting cached output while allowing interactive stdin for single uncached tasks. Includes 15 unit tests for stdin suggestion logic and 3 E2E snapshot tests verifying stdin inheritance/null behavior end-to-end.
MSYS cat from Git for Windows crashes with MapViewOfFileEx errors when stdin is set to Stdio::null(), causing CI failures on Windows. Replace with a Node.js read-stdin tool that handles null stdin gracefully.
Executions no longer abort the entire graph on infrastructure errors
(cache lookup, spawn, or cache update failures). All tasks run to
completion; failures are tracked by the reporter and reflected in the
final exit code.
- Replace ExecutionAborted with SpawnOutcome { CacheHit, Spawned, Failed }
- Make execute_leaf infallible (returns ())
- execute_expanded_graph returns Option<Str> for cycle errors only
- Remove first_error abort banner from labeled reporter
- Update exit code logic to account for infrastructure errors
- Update cycle dependency snapshot for new error format
Use petgraph::acyclic::Acyclic<DiGraph> for the ExecutionGraph type alias, providing type-level acyclicity guarantees. Cycles are now detected when building the execution plan rather than during graph execution. - Add CycleDependencyDetected variant to TaskPlanErrorKind - Replace StableGraph + toposort with Acyclic::nodes_iter() for execution order - Simplify ExecutionAborted to a unit struct (no error message field) - Remove error parameter from GraphExecutionReporter::finish() - Move cache initialization before reporter construction in execute_graph - Add plan snapshot test for cycle dependency errors - Update E2E snapshots for new error format and execution order
…ruct Replace the petgraph::acyclic::Acyclic type alias with a custom ExecutionGraph struct that owns a DiGraph and a pre-computed toposort. This eliminates the RefCell-based !Sync issue that required arc_with_non_send_sync suppressions. - Add try_from_graph() for validated construction with CycleError - Add inner() for access to the underlying DiGraph - Add toposort() returning cached &[ExecutionNodeIndex] - Add from_node_list() for building edge-free test graphs - Implement Deref<Target = DiGraph> for transparent read access - Remove petgraph::data::Build import and arc_with_non_send_sync expects - Replace Index<ExecutionPathItem> impl with ExecutionPathItem::resolve()
Move PlainReporter and LabeledReporter family from reporter.rs into reporter/plain.rs and reporter/labeled.rs. Shared types, traits, and display helpers remain in reporter/mod.rs with pub use re-exports.
branchseer
force-pushed
the
02-13-feat_new_reporter_implementation
branch
from
4a9dc5c to
ccee3b7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.