Skip to content

Commit facfe8f

Browse files
committed
refactor: implement Index<LeafExecutionPath> for ExecutionGraph
Add Index<&LeafExecutionPath> for ExecutionGraph with Output = LeafExecutionKind, providing direct indexing into nested execution graphs to retrieve leaf execution kinds. - Extract shared graph traversal into LeafExecutionPath::resolve_item() - Remove Option from display fields in labeled reporter (was unreachable dead code) - Add explicit Index<NodeIndex<Ix>> on AcyclicGraph to prevent shadowing through Deref
1 parent c5ee519 commit facfe8f

3 files changed

Lines changed: 64 additions & 62 deletions

File tree

crates/vite_task/src/session/reporter/labeled.rs

Lines changed: 21 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use vite_task_plan::{ExecutionGraph, ExecutionItemDisplay, ExecutionItemKind, Le
1717
use super::{
1818
CACHE_MISS_STYLE, COMMAND_STYLE, ColorizeExt, ExitStatus, GraphExecutionReporter,
1919
GraphExecutionReporterBuilder, LeafExecutionPath, LeafExecutionReporter, StdioConfig,
20-
StdioSuggestion, format_cache_hit_message, format_command_display,
21-
format_command_with_cache_status, format_error_message,
20+
StdioSuggestion, format_command_display, format_command_with_cache_status,
21+
format_error_message,
2222
};
2323
use crate::session::{
2424
cache::format_cache_status_summary,
@@ -28,9 +28,7 @@ use crate::session::{
2828
/// Information tracked for each leaf execution, used in the final summary.
2929
#[derive(Debug)]
3030
struct ExecutionInfo {
31-
/// Display info for this execution. `None` for displayless executions
32-
/// (e.g., synthetics reached via nested expansion).
33-
display: Option<ExecutionItemDisplay>,
31+
display: ExecutionItemDisplay,
3432
/// Cache status, determined at `start()`.
3533
cache_status: CacheStatus,
3634
/// Exit status from the process. `None` means no process was spawned (cache hit or in-process).
@@ -146,8 +144,7 @@ pub struct LabeledGraphReporter {
146144
)]
147145
impl GraphExecutionReporter for LabeledGraphReporter {
148146
fn new_leaf_execution(&mut self, path: &LeafExecutionPath) -> Box<dyn LeafExecutionReporter> {
149-
// Look up display info from the graph using the path
150-
let display = path.resolve_display(&self.graph).cloned();
147+
let display = path.resolve_item(&self.graph).execution_item_display.clone();
151148
Box::new(LabeledLeafReporter {
152149
shared: Rc::clone(&self.shared),
153150
writer: Rc::clone(&self.writer),
@@ -164,16 +161,8 @@ impl GraphExecutionReporter for LabeledGraphReporter {
164161
let (summary_buf, result) = {
165162
let shared = self.shared.borrow();
166163

167-
// Print summary.
168-
// Special case: single execution without display info (e.g., synthetic via
169-
// nested expansion) → skip summary since there's nothing meaningful to show.
170-
let is_single_displayless =
171-
shared.executions.len() == 1 && shared.executions[0].display.is_none();
172-
let summary_buf = if is_single_displayless {
173-
None
174-
} else {
175-
Some(format_summary(&shared.executions, &shared.stats, &self.workspace_path))
176-
};
164+
let summary_buf =
165+
Some(format_summary(&shared.executions, &shared.stats, &self.workspace_path));
177166

178167
// Determine exit code based on failed tasks and infrastructure errors:
179168
// - Infrastructure errors (cache lookup, spawn failure) have error_message set
@@ -231,7 +220,7 @@ struct LabeledLeafReporter {
231220
shared: Rc<RefCell<SharedReporterState>>,
232221
writer: Rc<RefCell<Box<dyn AsyncWrite + Unpin>>>,
233222
/// Display info for this execution, looked up from the graph via the path.
234-
display: Option<ExecutionItemDisplay>,
223+
display: ExecutionItemDisplay,
235224
workspace_path: Arc<AbsolutePath>,
236225
/// Whether `start()` has been called. Used to determine if stats should be updated
237226
/// in `finish()` and whether to push an `ExecutionInfo` entry.
@@ -281,16 +270,14 @@ impl LeafExecutionReporter for LabeledLeafReporter {
281270

282271
// Format command line with cache status (sync), then write asynchronously.
283272
// The shared borrow to read cache_status is brief and dropped before the await.
284-
if let Some(ref display) = self.display {
285-
let line = {
286-
let shared = self.shared.borrow();
287-
let cache_status = &shared.executions.last().unwrap().cache_status;
288-
format_command_with_cache_status(display, &self.workspace_path, cache_status)
289-
};
290-
let mut writer = self.writer.borrow_mut();
291-
let _ = writer.write_all(line.as_bytes()).await;
292-
let _ = writer.flush().await;
293-
}
273+
let line = {
274+
let shared = self.shared.borrow();
275+
let cache_status = &shared.executions.last().unwrap().cache_status;
276+
format_command_with_cache_status(&self.display, &self.workspace_path, cache_status)
277+
};
278+
let mut writer = self.writer.borrow_mut();
279+
let _ = writer.write_all(line.as_bytes()).await;
280+
let _ = writer.flush().await;
294281

295282
StdioConfig {
296283
suggestion,
@@ -352,12 +339,6 @@ impl LeafExecutionReporter for LabeledLeafReporter {
352339
buf.extend_from_slice(format_error_message(message).as_bytes());
353340
}
354341

355-
// For executions without display info (synthetics via nested expansion) that are
356-
// cache hits, print the cache hit message
357-
if self.started && self.display.is_none() && self.is_cache_hit {
358-
buf.extend_from_slice(format_cache_hit_message().as_bytes());
359-
}
360-
361342
// Add a trailing newline after each task's output for readability.
362343
// Skip if start() was never called (e.g. cache lookup failure) — there's
363344
// no task output to separate.
@@ -516,10 +497,7 @@ fn format_summary(
516497
);
517498

518499
for (idx, exec) in executions.iter().enumerate() {
519-
// Skip executions without display info (they have nothing to show in the summary)
520-
let Some(ref display) = exec.display else {
521-
continue;
522-
};
500+
let display = &exec.display;
523501

524502
let task_display = &display.task_display;
525503

@@ -677,13 +655,16 @@ mod tests {
677655
/// Build a `LabeledGraphReporter` for the given graph and return a leaf reporter
678656
/// for the first node's first item.
679657
fn build_labeled_leaf(graph: ExecutionGraph) -> Box<dyn LeafExecutionReporter> {
658+
use vite_task_plan::execution_graph::ExecutionNodeIndex;
659+
680660
let graph_arc = Arc::new(graph);
681661
let builder =
682662
Box::new(LabeledReporterBuilder::new(test_path(), Box::new(tokio::io::sink())));
683663
let mut reporter = builder.build(&graph_arc);
684664

685-
// Create a leaf reporter for the first node
686-
let path = LeafExecutionPath::default();
665+
// Create a leaf reporter for the first node's first item
666+
let mut path = LeafExecutionPath::default();
667+
path.push(ExecutionNodeIndex::new(0), 0);
687668
reporter.new_leaf_execution(&path)
688669
}
689670

crates/vite_task/src/session/reporter/mod.rs

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ use smallvec::SmallVec;
3737
use tokio::io::AsyncWrite;
3838
use vite_path::AbsolutePath;
3939
use vite_str::Str;
40-
use vite_task_plan::{ExecutionGraph, ExecutionItem, ExecutionItemDisplay, ExecutionItemKind};
40+
use vite_task_plan::{
41+
ExecutionGraph, ExecutionItem, ExecutionItemDisplay, ExecutionItemKind, LeafExecutionKind,
42+
};
4143

4244
use super::{
4345
cache::format_cache_status_inline,
@@ -144,44 +146,52 @@ impl LeafExecutionPath {
144146
self.0.push(ExecutionPathItem { graph_node_ix, task_execution_item_index });
145147
}
146148

147-
/// Look up the [`ExecutionItemDisplay`] for the leaf identified by this path,
148-
/// traversing through nested `Expanded` graphs as needed.
149+
/// Resolve this path against a root execution graph, returning the final
150+
/// [`ExecutionItem`] the path points to.
149151
///
150-
/// Returns `None` if the path is empty.
152+
/// This is the shared traversal logic that walks through nested `Expanded`
153+
/// graphs. Used by:
154+
/// - `Index<&LeafExecutionPath> for ExecutionGraph` — extracts `&LeafExecutionKind`
155+
/// - `new_leaf_execution` in `labeled.rs` — extracts `ExecutionItemDisplay`
151156
///
152157
/// # Panics
153158
///
154-
/// Panics if an intermediate path element does not point to an `Expanded` item,
155-
/// which indicates a bug in path construction.
156-
fn resolve_display<'a>(
157-
&self,
158-
root_graph: &'a ExecutionGraph,
159-
) -> Option<&'a ExecutionItemDisplay> {
159+
/// - If the path is empty (indicates a bug in path construction).
160+
/// - If an intermediate path element points to a `Leaf` item instead of
161+
/// `Expanded` (only `Expanded` items contain nested graphs to descend into).
162+
fn resolve_item<'a>(&self, root_graph: &'a ExecutionGraph) -> &'a ExecutionItem {
160163
let mut current_graph = root_graph;
164+
let last_depth = self.0.len() - 1;
161165
for (depth, path_item) in self.0.iter().enumerate() {
162166
let item = path_item.resolve(current_graph);
163-
let is_last = depth == self.0.len() - 1;
164-
if is_last {
165-
// Last element — return the display info regardless of Leaf/Expanded
166-
return Some(&item.execution_item_display);
167+
if depth == last_depth {
168+
return item;
167169
}
168-
// Intermediate element — must be Expanded so we can descend into it
169170
match &item.kind {
170171
ExecutionItemKind::Expanded(nested_graph) => {
171172
current_graph = nested_graph;
172173
}
173174
ExecutionItemKind::Leaf(_) => {
174-
// A Leaf in the middle of the path means the execution engine
175-
// pushed a Leaf node as an intermediate step, which is a bug —
176-
// only Expanded items can contain nested graphs to descend into.
177175
unreachable!(
178176
"LeafExecutionPath: intermediate element at depth {depth} is a Leaf, expected Expanded"
179177
)
180178
}
181179
}
182180
}
183-
// Empty path
184-
None
181+
unreachable!("LeafExecutionPath: empty path")
182+
}
183+
}
184+
185+
impl std::ops::Index<&LeafExecutionPath> for ExecutionGraph {
186+
type Output = LeafExecutionKind;
187+
188+
fn index(&self, path: &LeafExecutionPath) -> &Self::Output {
189+
match &path.resolve_item(self).kind {
190+
ExecutionItemKind::Leaf(kind) => kind,
191+
ExecutionItemKind::Expanded(_) => {
192+
unreachable!("LeafExecutionPath: final element is Expanded, expected Leaf")
193+
}
194+
}
185195
}
186196
}
187197

crates/vite_task_plan/src/execution_graph.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::ops::Deref;
1+
use std::ops::{Deref, Index};
22

33
use petgraph::{
44
graph::{DefaultIx, DiGraph, EdgeIndex, IndexType, NodeIndex},
@@ -134,7 +134,7 @@ impl<N, Ix: IndexType> Default for AcyclicGraph<N, Ix> {
134134
}
135135

136136
/// Deref to the inner `DiGraph` so that read-only graph operations
137-
/// (`node_count()`, `node_weights()`, `node_indices()`, indexing by `NodeIndex`, etc.)
137+
/// (`node_count()`, `node_weights()`, `node_indices()`, etc.)
138138
/// work transparently on `AcyclicGraph`.
139139
impl<N, Ix: IndexType> Deref for AcyclicGraph<N, Ix> {
140140
type Target = DiGraph<N, (), Ix>;
@@ -144,6 +144,17 @@ impl<N, Ix: IndexType> Deref for AcyclicGraph<N, Ix> {
144144
}
145145
}
146146

147+
/// Explicit `NodeIndex` indexing so that `graph[node_ix]` continues to work
148+
/// even when downstream crates add their own `Index` impls on `AcyclicGraph`
149+
/// (a direct `Index` impl shadows any `Index` that would be found through `Deref`).
150+
impl<N, Ix: IndexType> Index<NodeIndex<Ix>> for AcyclicGraph<N, Ix> {
151+
type Output = N;
152+
153+
fn index(&self, index: NodeIndex<Ix>) -> &Self::Output {
154+
&self.graph[index]
155+
}
156+
}
157+
147158
/// The execution graph type alias, specialized for task execution.
148159
pub type ExecutionGraph = AcyclicGraph<TaskExecution, ExecutionIx>;
149160

0 commit comments

Comments
 (0)