From 4427f8283e01dec715a9ca996ac046022d25f4d3 Mon Sep 17 00:00:00 2001 From: branchseer Date: Tue, 24 Feb 2026 11:16:58 +0800 Subject: [PATCH 1/2] refactor: remove LeafExecutionPath in favor of direct item passing LeafExecutionPath was an indirection layer that built index-pair paths at runtime during graph traversal, only for the reporter to re-traverse the same graph to recover the ExecutionItem it already had. Replace with passing &ExecutionItem and a bool (all_ancestors_single_node) directly. - Remove LeafExecutionPath, ExecutionPathItem, and dead Index impl - Simplify GraphExecutionReporterBuilder::build (no longer needs graph) - Remove Arc from LabeledGraphReporter - Track single-node ancestry incrementally during graph traversal - Drop smallvec dependency from vite_task crate --- Cargo.lock | 1 - crates/vite_task/Cargo.toml | 1 - crates/vite_task/src/session/execute/mod.rs | 55 ++--- .../vite_task/src/session/reporter/labeled.rs | 173 +++------------- crates/vite_task/src/session/reporter/mod.rs | 189 ++---------------- 5 files changed, 79 insertions(+), 340 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 63bdc15d..e800887e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3883,7 +3883,6 @@ dependencies = [ "rustc-hash", "serde", "serde_json", - "smallvec 2.0.0-alpha.12", "thiserror 2.0.18", "tokio", "tracing", diff --git a/crates/vite_task/Cargo.toml b/crates/vite_task/Cargo.toml index 14aaad7c..58f61818 100644 --- a/crates/vite_task/Cargo.toml +++ b/crates/vite_task/Cargo.toml @@ -29,7 +29,6 @@ rusqlite = { workspace = true, features = ["bundled"] } rustc-hash = { workspace = true } serde = { workspace = true, features = ["derive", "rc"] } serde_json = { workspace = true } -smallvec.workspace = true thiserror = { workspace = true } tokio = { workspace = true, features = ["rt-multi-thread", "io-std", "io-util", "macros", "sync"] } tracing = { workspace = true } diff --git a/crates/vite_task/src/session/execute/mod.rs b/crates/vite_task/src/session/execute/mod.rs index ed6b9141..a5ed29ab 100644 --- a/crates/vite_task/src/session/execute/mod.rs +++ b/crates/vite_task/src/session/execute/mod.rs @@ -7,7 +7,8 @@ use futures_util::FutureExt; use tokio::io::AsyncWriteExt as _; use vite_path::AbsolutePath; use vite_task_plan::{ - ExecutionGraph, ExecutionItemKind, LeafExecutionKind, SpawnCommand, SpawnExecution, + ExecutionGraph, ExecutionItem, ExecutionItemKind, LeafExecutionKind, SpawnCommand, + SpawnExecution, }; use self::{ @@ -21,8 +22,8 @@ use super::{ ExecutionError, }, reporter::{ - ExitStatus, GraphExecutionReporter, GraphExecutionReporterBuilder, LeafExecutionPath, - LeafExecutionReporter, StdioSuggestion, + ExitStatus, GraphExecutionReporter, GraphExecutionReporterBuilder, LeafExecutionReporter, + StdioSuggestion, }, }; use crate::{Session, session::execute::spawn::SpawnTrackResult}; @@ -64,16 +65,18 @@ impl ExecutionContext<'_> { /// We compute a topological order and iterate in reverse to get execution order /// (dependencies before dependents). /// - /// The `path_prefix` tracks our position within nested execution graphs. For the - /// root call this is an empty path; for nested `Expanded` items it carries the - /// path so far. + /// `all_ancestors_single_node` tracks whether every graph in the ancestry chain + /// (from the root down to this level) contains exactly one node. The initial call + /// passes `graph.node_count() == 1`; recursive calls AND with the nested graph's + /// node count. + /// /// Leaf-level errors are reported through the reporter and do not abort the graph. /// Cycle detection is handled at plan time, so this function cannot encounter cycles. #[expect(clippy::future_not_send, reason = "uses !Send types internally")] async fn execute_expanded_graph( &mut self, graph: &ExecutionGraph, - path_prefix: &LeafExecutionPath, + all_ancestors_single_node: bool, ) { // `compute_topological_order()` returns nodes in topological order: for every // edge A→B, A appears before B. Since our edges mean "A depends on B", @@ -86,18 +89,18 @@ impl ExecutionContext<'_> { for &node_ix in topo_order.iter().rev() { let task_execution = &graph[node_ix]; - for (item_idx, item) in task_execution.items.iter().enumerate() { - // Build the path for this item by appending to the prefix - let mut item_path = path_prefix.clone(); - item_path.push(node_ix, item_idx); - + for item in &task_execution.items { match &item.kind { - ExecutionItemKind::Leaf(leaf_kind) => { - self.execute_leaf(&item_path, leaf_kind).boxed_local().await; + ExecutionItemKind::Leaf(_) => { + self.execute_leaf(item, all_ancestors_single_node).boxed_local().await; } ExecutionItemKind::Expanded(nested_graph) => { - // Recurse into the nested graph, carrying the path prefix forward. - self.execute_expanded_graph(nested_graph, &item_path).boxed_local().await; + self.execute_expanded_graph( + nested_graph, + all_ancestors_single_node && nested_graph.node_count() == 1, + ) + .boxed_local() + .await; } } } @@ -109,12 +112,12 @@ impl ExecutionContext<'_> { /// Creates a [`LeafExecutionReporter`] from the graph reporter and delegates /// to the appropriate execution method. #[expect(clippy::future_not_send, reason = "uses !Send types internally")] - async fn execute_leaf( - &mut self, - path: &LeafExecutionPath, - leaf_execution_kind: &LeafExecutionKind, - ) { - let mut leaf_reporter = self.reporter.new_leaf_execution(path); + async fn execute_leaf(&mut self, item: &ExecutionItem, all_ancestors_single_node: bool) { + let mut leaf_reporter = self.reporter.new_leaf_execution(item, all_ancestors_single_node); + + let ExecutionItemKind::Leaf(leaf_execution_kind) = &item.kind else { + unreachable!("execute_leaf called with non-leaf ExecutionItem"); + }; match leaf_execution_kind { LeafExecutionKind::InProcess(in_process_execution) => { @@ -401,10 +404,7 @@ impl Session<'_> { } }; - // Wrap the graph in Arc so both the reporter and execution can reference it. - // The reporter clones the Arc internally for display lookups. - let graph = Arc::new(execution_graph); - let mut reporter = builder.build(&graph); + let mut reporter = builder.build(); let mut execution_context = ExecutionContext { reporter: &mut *reporter, @@ -414,7 +414,8 @@ impl Session<'_> { // Execute the graph. Leaf-level errors are reported through the reporter // and do not abort the graph. Cycle detection is handled at plan time. - execution_context.execute_expanded_graph(&graph, &LeafExecutionPath::default()).await; + let all_single_node = execution_graph.node_count() == 1; + execution_context.execute_expanded_graph(&execution_graph, all_single_node).await; // Leaf-level errors and non-zero exit statuses are tracked internally // by the reporter. diff --git a/crates/vite_task/src/session/reporter/labeled.rs b/crates/vite_task/src/session/reporter/labeled.rs index d548db49..881ce78a 100644 --- a/crates/vite_task/src/session/reporter/labeled.rs +++ b/crates/vite_task/src/session/reporter/labeled.rs @@ -12,13 +12,12 @@ use owo_colors::Style; use tokio::io::{AsyncWrite, AsyncWriteExt as _}; use vite_path::AbsolutePath; use vite_str::Str; -use vite_task_plan::{ExecutionGraph, ExecutionItemDisplay, ExecutionItemKind, LeafExecutionKind}; +use vite_task_plan::{ExecutionItem, ExecutionItemDisplay, ExecutionItemKind, LeafExecutionKind}; use super::{ CACHE_MISS_STYLE, COMMAND_STYLE, ColorizeExt, ExitStatus, GraphExecutionReporter, - GraphExecutionReporterBuilder, LeafExecutionPath, LeafExecutionReporter, StdioConfig, - StdioSuggestion, format_command_display, format_command_with_cache_status, - format_error_message, + GraphExecutionReporterBuilder, LeafExecutionReporter, StdioConfig, StdioSuggestion, + format_command_display, format_command_with_cache_status, format_error_message, }; use crate::session::{ cache::format_cache_status_summary, @@ -88,7 +87,7 @@ impl LabeledReporterBuilder { } impl GraphExecutionReporterBuilder for LabeledReporterBuilder { - fn build(self: Box, graph: &Arc) -> Box { + fn build(self: Box) -> Box { let writer = Rc::new(RefCell::new(self.writer)); Box::new(LabeledGraphReporter { shared: Rc::new(RefCell::new(SharedReporterState { @@ -96,7 +95,6 @@ impl GraphExecutionReporterBuilder for LabeledReporterBuilder { stats: ExecutionStats::default(), })), writer, - graph: Arc::clone(graph), workspace_path: self.workspace_path, }) } @@ -109,7 +107,6 @@ impl GraphExecutionReporterBuilder for LabeledReporterBuilder { pub struct LabeledGraphReporter { shared: Rc>, writer: Rc>>, - graph: Arc, workspace_path: Arc, } @@ -120,13 +117,14 @@ pub struct LabeledGraphReporter { and finish() is called once after all leaf reporters are dropped" )] impl GraphExecutionReporter for LabeledGraphReporter { - fn new_leaf_execution(&mut self, path: &LeafExecutionPath) -> Box { - let item = path.resolve_item(&self.graph); + fn new_leaf_execution( + &mut self, + item: &ExecutionItem, + all_ancestors_single_node: bool, + ) -> Box { let display = item.execution_item_display.clone(); let stdio_suggestion = match &item.kind { - ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(_)) - if path.all_containing_graphs_single_node(&self.graph) => - { + ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(_)) if all_ancestors_single_node => { StdioSuggestion::Inherited } _ => StdioSuggestion::Piped, @@ -554,167 +552,60 @@ fn format_summary( #[cfg(test)] mod tests { - use std::sync::Arc; - - use vite_task_plan::{ - ExecutionGraph, ExecutionItem, TaskExecution, execution_graph::ExecutionNodeIndex, - }; - use super::*; use crate::session::{ event::CacheDisabledReason, reporter::{ - LeafExecutionPath, LeafExecutionReporter, StdioSuggestion, - test_fixtures::{expanded_task, in_process_task, spawn_task, test_path}, + LeafExecutionReporter, StdioSuggestion, + test_fixtures::{in_process_task, spawn_task, test_path}, }, }; - fn leaf_path(path_items: &[(usize, usize)]) -> LeafExecutionPath { - let mut path = LeafExecutionPath::default(); - for (node_ix, item_ix) in path_items { - path.push(ExecutionNodeIndex::new(*node_ix), *item_ix); - } - path - } - fn build_labeled_leaf( - graph: ExecutionGraph, - path: &LeafExecutionPath, + item: &ExecutionItem, + all_ancestors_single_node: bool, ) -> Box { - let graph_arc = Arc::new(graph); let builder = Box::new(LabeledReporterBuilder::new(test_path(), Box::new(tokio::io::sink()))); - let mut reporter = builder.build(&graph_arc); - reporter.new_leaf_execution(path) + let mut reporter = builder.build(); + reporter.new_leaf_execution(item, all_ancestors_single_node) } #[expect( clippy::future_not_send, reason = "LeafExecutionReporter futures are !Send in single-threaded reporter tests" )] - async fn suggestion_for_path( - graph: ExecutionGraph, - path: &LeafExecutionPath, + async fn suggestion_for( + item: &ExecutionItem, + all_ancestors_single_node: bool, ) -> StdioSuggestion { - let mut leaf = build_labeled_leaf(graph, path); + let mut leaf = build_labeled_leaf(item, all_ancestors_single_node); let stdio_config = leaf.start(CacheStatus::Disabled(CacheDisabledReason::NoCacheMetadata)).await; stdio_config.suggestion } - fn spawn_item(name: &str) -> ExecutionItem { - let mut task = spawn_task(name); - task.items.pop().expect("spawn_task always has one item") - } - - fn expanded_item(name: &str, nested_graph: ExecutionGraph) -> ExecutionItem { - let mut task = expanded_task(name, nested_graph); - task.items.pop().expect("expanded_task always has one item") - } - - fn task_with_items(name: &str, items: Vec) -> TaskExecution { - let mut task = spawn_task(name); - task.items = items; - task - } - - fn graph_with_mixed_and_siblings() -> ExecutionGraph { - let nested_single_node = ExecutionGraph::from_node_list([spawn_task("bar")]); - let nested_multi_node = - ExecutionGraph::from_node_list([spawn_task("build-a"), spawn_task("build-b")]); - - let root = task_with_items( - "foo", - vec![ - spawn_item("foo-step"), - expanded_item("run-bar", nested_single_node), - expanded_item("run-build-recursive", nested_multi_node), - ], - ); - - ExecutionGraph::from_node_list([root]) - } - #[tokio::test] - async fn labeled_reporter_single_spawn_in_single_node_chain_suggests_inherited() { - let graph = ExecutionGraph::from_node_list([spawn_task("build")]); - let path = leaf_path(&[(0, 0)]); - - assert_eq!(suggestion_for_path(graph, &path).await, StdioSuggestion::Inherited); + async fn spawn_with_all_single_node_ancestors_suggests_inherited() { + let task = spawn_task("build"); + assert_eq!(suggestion_for(&task.items[0], true).await, StdioSuggestion::Inherited); } #[tokio::test] - async fn labeled_reporter_multi_node_root_graph_suggests_piped() { - let graph = ExecutionGraph::from_node_list([spawn_task("build"), spawn_task("test")]); - let path = leaf_path(&[(0, 0)]); - - assert_eq!(suggestion_for_path(graph, &path).await, StdioSuggestion::Piped); + async fn spawn_without_all_single_node_ancestors_suggests_piped() { + let task = spawn_task("build"); + assert_eq!(suggestion_for(&task.items[0], false).await, StdioSuggestion::Piped); } #[tokio::test] - async fn labeled_reporter_nested_single_node_chain_suggests_inherited() { - let nested = ExecutionGraph::from_node_list([spawn_task("nested-build")]); - let graph = ExecutionGraph::from_node_list([expanded_task("expand", nested)]); - let path = leaf_path(&[(0, 0), (0, 0)]); - - assert_eq!(suggestion_for_path(graph, &path).await, StdioSuggestion::Inherited); + async fn in_process_leaf_suggests_piped_even_with_single_node_ancestors() { + let task = in_process_task("echo"); + assert_eq!(suggestion_for(&task.items[0], true).await, StdioSuggestion::Piped); } #[tokio::test] - async fn labeled_reporter_nested_multi_node_graph_suggests_piped() { - let nested = - ExecutionGraph::from_node_list([spawn_task("nested-a"), spawn_task("nested-b")]); - let graph = ExecutionGraph::from_node_list([expanded_task("expand", nested)]); - let path = leaf_path(&[(0, 0), (0, 0)]); - - assert_eq!(suggestion_for_path(graph, &path).await, StdioSuggestion::Piped); - } - - #[tokio::test] - async fn labeled_reporter_multi_node_ancestor_forces_piped_for_nested_leaf() { - let nested = ExecutionGraph::from_node_list([spawn_task("nested-build")]); - let graph = ExecutionGraph::from_node_list([ - expanded_task("expand", nested), - spawn_task("sibling"), - ]); - let path = leaf_path(&[(0, 0), (0, 0)]); - - assert_eq!(suggestion_for_path(graph, &path).await, StdioSuggestion::Piped); - } - - #[tokio::test] - async fn labeled_reporter_keeps_inherited_for_single_node_chains_with_multi_node_and_sibling() { - // Root graph has one node and three sequential `&&` items: - // 1) direct spawn leaf - // 2) expanded graph with one node - // 3) expanded graph with multiple nodes - // - // The first two leaves should still suggest inherited stdio because their - // containing graph chain has only one node at every level. The third should - // suggest piped because its containing graph has multiple nodes. - let path_top_level = leaf_path(&[(0, 0)]); - let path_nested_single = leaf_path(&[(0, 1), (0, 0)]); - let path_nested_multi = leaf_path(&[(0, 2), (0, 0)]); - - assert_eq!( - suggestion_for_path(graph_with_mixed_and_siblings(), &path_top_level).await, - StdioSuggestion::Inherited - ); - assert_eq!( - suggestion_for_path(graph_with_mixed_and_siblings(), &path_nested_single).await, - StdioSuggestion::Inherited - ); - assert_eq!( - suggestion_for_path(graph_with_mixed_and_siblings(), &path_nested_multi).await, - StdioSuggestion::Piped - ); - } - - #[tokio::test] - async fn labeled_reporter_in_process_leaf_suggests_piped() { - let graph = ExecutionGraph::from_node_list([in_process_task("echo")]); - let path = leaf_path(&[(0, 0)]); - - assert_eq!(suggestion_for_path(graph, &path).await, StdioSuggestion::Piped); + async fn in_process_leaf_suggests_piped_without_single_node_ancestors() { + let task = in_process_task("echo"); + assert_eq!(suggestion_for(&task.items[0], false).await, StdioSuggestion::Piped); } } diff --git a/crates/vite_task/src/session/reporter/mod.rs b/crates/vite_task/src/session/reporter/mod.rs index 6ca717d0..7ba25f9b 100644 --- a/crates/vite_task/src/session/reporter/mod.rs +++ b/crates/vite_task/src/session/reporter/mod.rs @@ -2,10 +2,10 @@ //! //! This module provides a typestate-based reporter system with three phases: //! -//! 1. [`GraphExecutionReporterBuilder`] — created before the execution graph is known. -//! Transitions to [`GraphExecutionReporter`] when `build()` is called with the graph. +//! 1. [`GraphExecutionReporterBuilder`] — created before execution begins. +//! Transitions to [`GraphExecutionReporter`] when `build()` is called. //! -//! 2. [`GraphExecutionReporter`] — knows the execution graph. Creates [`LeafExecutionReporter`] +//! 2. [`GraphExecutionReporter`] — creates [`LeafExecutionReporter`] //! instances for individual leaf executions via `new_leaf_execution()`. Finalized with `finish()`. //! //! 3. [`LeafExecutionReporter`] — handles events for a single leaf execution (output streaming, @@ -25,21 +25,15 @@ mod plain; // Re-export the concrete implementations so callers can use `reporter::PlainReporter` // and `reporter::LabeledReporterBuilder` without navigating into child modules. -use std::{ - process::ExitStatus as StdExitStatus, - sync::{Arc, LazyLock}, -}; +use std::{process::ExitStatus as StdExitStatus, sync::LazyLock}; pub use labeled::LabeledReporterBuilder; use owo_colors::{Style, Styled}; pub use plain::PlainReporter; -use smallvec::SmallVec; use tokio::io::AsyncWrite; use vite_path::AbsolutePath; use vite_str::Str; -use vite_task_plan::{ - ExecutionGraph, ExecutionItem, ExecutionItemDisplay, ExecutionItemKind, LeafExecutionKind, -}; +use vite_task_plan::{ExecutionItem, ExecutionItemDisplay}; use super::{ cache::format_cache_status_inline, @@ -101,143 +95,6 @@ pub struct StdioConfig { pub stderr_writer: Box, } -// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ -// Leaf execution path — identifies a leaf within a (potentially nested) execution graph -// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - -/// One step in a [`LeafExecutionPath`]: identifies a specific execution item -/// within a single level of the execution graph. -#[derive(Clone, Copy, Debug)] -struct ExecutionPathItem { - /// The node (task) index in the execution graph at this level. - graph_node_ix: ExecutionNodeIndex, - /// The item index within the task's `items` vector. - task_execution_item_index: usize, -} - -use vite_task_plan::execution_graph::ExecutionNodeIndex; - -impl ExecutionPathItem { - /// Resolve this path item against a graph, returning the [`ExecutionItem`] - /// at the identified position. - fn resolve(self, graph: &ExecutionGraph) -> &ExecutionItem { - &graph[self.graph_node_ix].items[self.task_execution_item_index] - } -} - -/// A path through a (potentially nested) execution graph that identifies a specific -/// leaf execution. -/// -/// Each element in the path represents a step deeper into a nested `Expanded` execution -/// graph. The last element identifies the actual leaf item. -/// -/// For example, a path of `[(node_0, item_1), (node_2, item_0)]` means: -/// - In the root graph, node 0, item 1 (which is an `Expanded` containing a nested graph) -/// - In that nested graph, node 2, item 0 (the actual leaf execution) -/// -/// Uses `SmallVec` with inline capacity of 4 since most execution graphs are shallow -/// (typically 1-2 levels of nesting). -#[derive(Clone, Debug, Default)] -pub struct LeafExecutionPath(SmallVec); - -impl LeafExecutionPath { - /// Append a new step to this path, identifying an item at the given node and item indices. - pub fn push(&mut self, graph_node_ix: ExecutionNodeIndex, task_execution_item_index: usize) { - self.0.push(ExecutionPathItem { graph_node_ix, task_execution_item_index }); - } - - /// Resolve this path against a root execution graph, returning the final - /// [`ExecutionItem`] the path points to. - /// - /// This is the shared traversal logic that walks through nested `Expanded` - /// graphs. Used by: - /// - `Index<&LeafExecutionPath> for ExecutionGraph` — extracts `&LeafExecutionKind` - /// - `new_leaf_execution` in `labeled.rs` — extracts `ExecutionItemDisplay` - /// - /// # Panics - /// - /// - If the path is empty (indicates a bug in path construction). - /// - If an intermediate path element points to a `Leaf` item instead of - /// `Expanded` (only `Expanded` items contain nested graphs to descend into). - fn resolve_item<'a>(&self, root_graph: &'a ExecutionGraph) -> &'a ExecutionItem { - let mut current_graph = root_graph; - let last_depth = self.0.len() - 1; - for (depth, path_item) in self.0.iter().enumerate() { - let item = path_item.resolve(current_graph); - if depth == last_depth { - return item; - } - match &item.kind { - ExecutionItemKind::Expanded(nested_graph) => { - current_graph = nested_graph; - } - ExecutionItemKind::Leaf(_) => { - unreachable!( - "LeafExecutionPath: intermediate element at depth {depth} is a Leaf, expected Expanded" - ) - } - } - } - unreachable!("LeafExecutionPath: empty path") - } - - /// Return true when this leaf is contained in a chain of execution graphs where - /// every graph (root + all nested `Expanded` ancestors) has exactly one node. - /// - /// This is used by the labeled reporter to determine whether inherited stdio can - /// be suggested for spawned processes. - /// - /// # Panics - /// - /// - If the path is empty (indicates a bug in path construction). - /// - If an intermediate path element points to a `Leaf` item instead of - /// `Expanded`. - fn all_containing_graphs_single_node(&self, root_graph: &ExecutionGraph) -> bool { - let Some((last_path_item, parent_path_items)) = self.0.split_last() else { - unreachable!("LeafExecutionPath: empty path") - }; - - let mut current_graph = root_graph; - if current_graph.node_count() != 1 { - return false; - } - - for (depth, path_item) in parent_path_items.iter().enumerate() { - let item = path_item.resolve(current_graph); - match &item.kind { - ExecutionItemKind::Expanded(nested_graph) => { - current_graph = nested_graph; - if current_graph.node_count() != 1 { - return false; - } - } - ExecutionItemKind::Leaf(_) => { - unreachable!( - "LeafExecutionPath: intermediate element at depth {depth} is a Leaf, expected Expanded" - ) - } - } - } - - // Validate that the final path item resolves in the containing graph. - let _ = last_path_item.resolve(current_graph); - true - } -} - -impl std::ops::Index<&LeafExecutionPath> for ExecutionGraph { - type Output = LeafExecutionKind; - - fn index(&self, path: &LeafExecutionPath) -> &Self::Output { - match &path.resolve_item(self).kind { - ExecutionItemKind::Leaf(kind) => kind, - ExecutionItemKind::Expanded(_) => { - unreachable!("LeafExecutionPath: final element is Expanded, expected Leaf") - } - } - } -} - // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ // Typestate traits // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ @@ -245,13 +102,10 @@ impl std::ops::Index<&LeafExecutionPath> for ExecutionGraph { /// Builder for creating a [`GraphExecutionReporter`]. /// /// This is the initial state before the execution graph is known. The `build` method -/// transitions to the next state by providing the graph. +/// transitions to the [`GraphExecutionReporter`] phase. pub trait GraphExecutionReporterBuilder { - /// Create a [`GraphExecutionReporter`] for the given execution graph. - /// - /// The reporter may clone the `Arc` to retain a reference to the graph - /// for looking up display information during execution. - fn build(self: Box, graph: &Arc) -> Box; + /// Create a [`GraphExecutionReporter`]. + fn build(self: Box) -> Box; } /// Reporter for an entire graph execution session. @@ -260,11 +114,17 @@ pub trait GraphExecutionReporterBuilder { /// and finalizes the session with `finish()`. #[async_trait::async_trait(?Send)] pub trait GraphExecutionReporter { - /// Create a new leaf execution reporter for the leaf identified by `path`. + /// Create a new leaf execution reporter for the given execution item. /// - /// The reporter implementation can look up display info (task name, command, cwd) - /// from the execution graph using the path. - fn new_leaf_execution(&mut self, path: &LeafExecutionPath) -> Box; + /// `all_ancestors_single_node` is `true` when every execution graph in + /// the ancestry chain (root + all nested `Expanded` parents) contains + /// exactly one node. The reporter may use this to decide stdio mode + /// (e.g. suggesting inherited stdio for a single spawned process). + fn new_leaf_execution( + &mut self, + item: &ExecutionItem, + all_ancestors_single_node: bool, + ) -> Box; /// Finalize the graph execution session. /// @@ -401,7 +261,7 @@ pub mod test_fixtures { use vite_path::AbsolutePath; use vite_task_graph::display::TaskDisplay; use vite_task_plan::{ - ExecutionGraph, ExecutionItem, ExecutionItemDisplay, ExecutionItemKind, InProcessExecution, + ExecutionItem, ExecutionItemDisplay, ExecutionItemKind, InProcessExecution, LeafExecutionKind, SpawnCommand, SpawnExecution, TaskExecution, }; @@ -472,15 +332,4 @@ pub mod test_fixtures { }], } } - - /// Create a `TaskExecution` with an expanded (nested) subgraph as its item. - pub fn expanded_task(name: &str, nested_graph: ExecutionGraph) -> TaskExecution { - TaskExecution { - task_display: test_task_display(name), - items: vec![ExecutionItem { - execution_item_display: test_display(name), - kind: ExecutionItemKind::Expanded(nested_graph), - }], - } - } } From 6e53425218ccdbe0445150beb942a99434985890 Mon Sep 17 00:00:00 2001 From: branchseer Date: Tue, 24 Feb 2026 11:42:35 +0800 Subject: [PATCH 2/2] refactor: replace unreachable! with compile-time type safety in execute_leaf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass &ExecutionItemDisplay and &LeafExecutionKind separately instead of &ExecutionItem. The caller already matches on Leaf(leaf_kind), so decomposing at the call site eliminates the unreachable! branch — the compiler now enforces that execute_leaf can only receive leaf items. --- crates/vite_task/src/session/execute/mod.rs | 28 +++++---- .../vite_task/src/session/reporter/labeled.rs | 58 ++++++++++++++----- crates/vite_task/src/session/reporter/mod.rs | 7 ++- 3 files changed, 65 insertions(+), 28 deletions(-) diff --git a/crates/vite_task/src/session/execute/mod.rs b/crates/vite_task/src/session/execute/mod.rs index a5ed29ab..54b7c8d3 100644 --- a/crates/vite_task/src/session/execute/mod.rs +++ b/crates/vite_task/src/session/execute/mod.rs @@ -7,7 +7,7 @@ use futures_util::FutureExt; use tokio::io::AsyncWriteExt as _; use vite_path::AbsolutePath; use vite_task_plan::{ - ExecutionGraph, ExecutionItem, ExecutionItemKind, LeafExecutionKind, SpawnCommand, + ExecutionGraph, ExecutionItemDisplay, ExecutionItemKind, LeafExecutionKind, SpawnCommand, SpawnExecution, }; @@ -91,8 +91,14 @@ impl ExecutionContext<'_> { for item in &task_execution.items { match &item.kind { - ExecutionItemKind::Leaf(_) => { - self.execute_leaf(item, all_ancestors_single_node).boxed_local().await; + ExecutionItemKind::Leaf(leaf_kind) => { + self.execute_leaf( + &item.execution_item_display, + leaf_kind, + all_ancestors_single_node, + ) + .boxed_local() + .await; } ExecutionItemKind::Expanded(nested_graph) => { self.execute_expanded_graph( @@ -112,14 +118,16 @@ impl ExecutionContext<'_> { /// Creates a [`LeafExecutionReporter`] from the graph reporter and delegates /// to the appropriate execution method. #[expect(clippy::future_not_send, reason = "uses !Send types internally")] - async fn execute_leaf(&mut self, item: &ExecutionItem, all_ancestors_single_node: bool) { - let mut leaf_reporter = self.reporter.new_leaf_execution(item, all_ancestors_single_node); - - let ExecutionItemKind::Leaf(leaf_execution_kind) = &item.kind else { - unreachable!("execute_leaf called with non-leaf ExecutionItem"); - }; + async fn execute_leaf( + &mut self, + display: &ExecutionItemDisplay, + leaf_kind: &LeafExecutionKind, + all_ancestors_single_node: bool, + ) { + let mut leaf_reporter = + self.reporter.new_leaf_execution(display, leaf_kind, all_ancestors_single_node); - match leaf_execution_kind { + match leaf_kind { LeafExecutionKind::InProcess(in_process_execution) => { // In-process (built-in) commands: caching is disabled, execute synchronously let mut stdio_config = leaf_reporter diff --git a/crates/vite_task/src/session/reporter/labeled.rs b/crates/vite_task/src/session/reporter/labeled.rs index 881ce78a..9101a40d 100644 --- a/crates/vite_task/src/session/reporter/labeled.rs +++ b/crates/vite_task/src/session/reporter/labeled.rs @@ -12,7 +12,7 @@ use owo_colors::Style; use tokio::io::{AsyncWrite, AsyncWriteExt as _}; use vite_path::AbsolutePath; use vite_str::Str; -use vite_task_plan::{ExecutionItem, ExecutionItemDisplay, ExecutionItemKind, LeafExecutionKind}; +use vite_task_plan::{ExecutionItemDisplay, LeafExecutionKind}; use super::{ CACHE_MISS_STYLE, COMMAND_STYLE, ColorizeExt, ExitStatus, GraphExecutionReporter, @@ -119,14 +119,13 @@ pub struct LabeledGraphReporter { impl GraphExecutionReporter for LabeledGraphReporter { fn new_leaf_execution( &mut self, - item: &ExecutionItem, + display: &ExecutionItemDisplay, + leaf_kind: &LeafExecutionKind, all_ancestors_single_node: bool, ) -> Box { - let display = item.execution_item_display.clone(); - let stdio_suggestion = match &item.kind { - ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(_)) if all_ancestors_single_node => { - StdioSuggestion::Inherited - } + let display = display.clone(); + let stdio_suggestion = match leaf_kind { + LeafExecutionKind::Spawn(_) if all_ancestors_single_node => StdioSuggestion::Inherited, _ => StdioSuggestion::Piped, }; @@ -552,6 +551,8 @@ fn format_summary( #[cfg(test)] mod tests { + use vite_task_plan::ExecutionItemKind; + use super::*; use crate::session::{ event::CacheDisabledReason, @@ -561,14 +562,24 @@ mod tests { }, }; + /// Extract the `LeafExecutionKind` from a test fixture item. + /// Panics if the item is not a leaf (test fixtures always produce leaves). + fn leaf_kind(item: &vite_task_plan::ExecutionItem) -> &LeafExecutionKind { + match &item.kind { + ExecutionItemKind::Leaf(kind) => kind, + ExecutionItemKind::Expanded(_) => panic!("test fixture item must be a Leaf"), + } + } + fn build_labeled_leaf( - item: &ExecutionItem, + display: &ExecutionItemDisplay, + leaf_kind: &LeafExecutionKind, all_ancestors_single_node: bool, ) -> Box { let builder = Box::new(LabeledReporterBuilder::new(test_path(), Box::new(tokio::io::sink()))); let mut reporter = builder.build(); - reporter.new_leaf_execution(item, all_ancestors_single_node) + reporter.new_leaf_execution(display, leaf_kind, all_ancestors_single_node) } #[expect( @@ -576,10 +587,11 @@ mod tests { reason = "LeafExecutionReporter futures are !Send in single-threaded reporter tests" )] async fn suggestion_for( - item: &ExecutionItem, + display: &ExecutionItemDisplay, + leaf_kind: &LeafExecutionKind, all_ancestors_single_node: bool, ) -> StdioSuggestion { - let mut leaf = build_labeled_leaf(item, all_ancestors_single_node); + let mut leaf = build_labeled_leaf(display, leaf_kind, all_ancestors_single_node); let stdio_config = leaf.start(CacheStatus::Disabled(CacheDisabledReason::NoCacheMetadata)).await; stdio_config.suggestion @@ -588,24 +600,40 @@ mod tests { #[tokio::test] async fn spawn_with_all_single_node_ancestors_suggests_inherited() { let task = spawn_task("build"); - assert_eq!(suggestion_for(&task.items[0], true).await, StdioSuggestion::Inherited); + let item = &task.items[0]; + assert_eq!( + suggestion_for(&item.execution_item_display, leaf_kind(item), true).await, + StdioSuggestion::Inherited + ); } #[tokio::test] async fn spawn_without_all_single_node_ancestors_suggests_piped() { let task = spawn_task("build"); - assert_eq!(suggestion_for(&task.items[0], false).await, StdioSuggestion::Piped); + let item = &task.items[0]; + assert_eq!( + suggestion_for(&item.execution_item_display, leaf_kind(item), false).await, + StdioSuggestion::Piped + ); } #[tokio::test] async fn in_process_leaf_suggests_piped_even_with_single_node_ancestors() { let task = in_process_task("echo"); - assert_eq!(suggestion_for(&task.items[0], true).await, StdioSuggestion::Piped); + let item = &task.items[0]; + assert_eq!( + suggestion_for(&item.execution_item_display, leaf_kind(item), true).await, + StdioSuggestion::Piped + ); } #[tokio::test] async fn in_process_leaf_suggests_piped_without_single_node_ancestors() { let task = in_process_task("echo"); - assert_eq!(suggestion_for(&task.items[0], false).await, StdioSuggestion::Piped); + let item = &task.items[0]; + assert_eq!( + suggestion_for(&item.execution_item_display, leaf_kind(item), false).await, + StdioSuggestion::Piped + ); } } diff --git a/crates/vite_task/src/session/reporter/mod.rs b/crates/vite_task/src/session/reporter/mod.rs index 7ba25f9b..7c8d2974 100644 --- a/crates/vite_task/src/session/reporter/mod.rs +++ b/crates/vite_task/src/session/reporter/mod.rs @@ -33,7 +33,7 @@ pub use plain::PlainReporter; use tokio::io::AsyncWrite; use vite_path::AbsolutePath; use vite_str::Str; -use vite_task_plan::{ExecutionItem, ExecutionItemDisplay}; +use vite_task_plan::{ExecutionItemDisplay, LeafExecutionKind}; use super::{ cache::format_cache_status_inline, @@ -114,7 +114,7 @@ pub trait GraphExecutionReporterBuilder { /// and finalizes the session with `finish()`. #[async_trait::async_trait(?Send)] pub trait GraphExecutionReporter { - /// Create a new leaf execution reporter for the given execution item. + /// Create a new leaf execution reporter for the given leaf. /// /// `all_ancestors_single_node` is `true` when every execution graph in /// the ancestry chain (root + all nested `Expanded` parents) contains @@ -122,7 +122,8 @@ pub trait GraphExecutionReporter { /// (e.g. suggesting inherited stdio for a single spawned process). fn new_leaf_execution( &mut self, - item: &ExecutionItem, + display: &ExecutionItemDisplay, + leaf_kind: &LeafExecutionKind, all_ancestors_single_node: bool, ) -> Box;