Skip to content

Commit eb0accb

Browse files
authored
fix: use leaf graph path for stdio suggestion (#167)
## Technical changes - Replaced the labeled reporter's global `spawn_leaf_count` stdio heuristic with per-leaf evaluation based on graph structure along each `LeafExecutionPath`. - Added `LeafExecutionPath::all_containing_graphs_single_node(...)` to determine whether a leaf is inside only single-node graphs (root + expanded ancestors). - Moved stdio suggestion selection to `new_leaf_execution(...)`, storing the computed suggestion on `LabeledLeafReporter` and using it in `start(...)`. - Removed now-obsolete `count_spawn_leaves(...)` logic and rewrote labeled reporter unit tests to validate graph-path criteria and ancestor behavior. - Added a new e2e fixture `stdio-graph-criteria` that exercises mixed `&&` + expanded graph cases and verifies tty behavior via snapshots. - Updated stdin-inheritance fixture documentation to match the new criteria wording. ## User-facing changes - Commands now suggest inherited stdio when they run in a single-node graph chain, even if a sibling `&&` branch expands into a multi-node graph. - Commands now suggest piped stdio whenever they are in a multi-node graph or nested under any multi-node ancestor graph. - This makes tty behavior for nested `vp run ...` expansions align with execution semantics in mixed sequential/expanded plans. ## Validation - `cargo test -p vite_task labeled_reporter` - `cargo test -p vite_task_bin --test e2e_snapshots -- stdio-graph-criteria` - `cargo test -p vite_task_bin --test e2e_snapshots -- stdin-inheritance`
1 parent bddad36 commit eb0accb

File tree

11 files changed

+319
-143
lines changed

11 files changed

+319
-143
lines changed

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

Lines changed: 131 additions & 142 deletions
Large diffs are not rendered by default.

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,49 @@ impl LeafExecutionPath {
180180
}
181181
unreachable!("LeafExecutionPath: empty path")
182182
}
183+
184+
/// Return true when this leaf is contained in a chain of execution graphs where
185+
/// every graph (root + all nested `Expanded` ancestors) has exactly one node.
186+
///
187+
/// This is used by the labeled reporter to determine whether inherited stdio can
188+
/// be suggested for spawned processes.
189+
///
190+
/// # Panics
191+
///
192+
/// - If the path is empty (indicates a bug in path construction).
193+
/// - If an intermediate path element points to a `Leaf` item instead of
194+
/// `Expanded`.
195+
fn all_containing_graphs_single_node(&self, root_graph: &ExecutionGraph) -> bool {
196+
let Some((last_path_item, parent_path_items)) = self.0.split_last() else {
197+
unreachable!("LeafExecutionPath: empty path")
198+
};
199+
200+
let mut current_graph = root_graph;
201+
if current_graph.node_count() != 1 {
202+
return false;
203+
}
204+
205+
for (depth, path_item) in parent_path_items.iter().enumerate() {
206+
let item = path_item.resolve(current_graph);
207+
match &item.kind {
208+
ExecutionItemKind::Expanded(nested_graph) => {
209+
current_graph = nested_graph;
210+
if current_graph.node_count() != 1 {
211+
return false;
212+
}
213+
}
214+
ExecutionItemKind::Leaf(_) => {
215+
unreachable!(
216+
"LeafExecutionPath: intermediate element at depth {depth} is a Leaf, expected Expanded"
217+
)
218+
}
219+
}
220+
}
221+
222+
// Validate that the final path item resolves in the containing graph.
223+
let _ = last_path_item.resolve(current_graph);
224+
true
225+
}
183226
}
184227

185228
impl std::ops::Index<&LeafExecutionPath> for ExecutionGraph {

crates/vite_task_bin/tests/e2e_snapshots/fixtures/stdin-inheritance/snapshots.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
# Tests stdin inheritance behavior for spawned tasks.
22
#
33
# Stdin is inherited from the parent process only when BOTH conditions are met:
4-
# 1. The reporter suggests inherited stdin (single spawn leaf in the graph, or synthetic execution)
4+
# 1. The reporter suggests inherited stdin (the leaf is in a single-node graph chain,
5+
# or the execution uses PlainReporter for synthetic execution)
56
# 2. Caching is disabled for the task (cache_metadata is None)
67
#
78
# Otherwise, stdin is set to /dev/null.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "stdio-graph-criteria-test",
3+
"private": true
4+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"name": "other"
3+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"tasks": {
3+
"check-tty": {
4+
"command": "check-tty",
5+
"cache": false
6+
},
7+
"bar": {
8+
"command": "check-tty",
9+
"cache": false
10+
},
11+
"foo-nested": {
12+
"command": "vp run bar",
13+
"cache": false
14+
}
15+
}
16+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
packages:
2+
- packages/*
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Tests stdio suggestion criteria based on graph shape along each leaf path.
2+
#
3+
# Rules under test:
4+
# - Suggest piped when the leaf's containing graph has more than one node, or
5+
# any ancestor graph on its path has more than one node.
6+
# - Suggest inherited only when every graph on the path (root + nested Expanded
7+
# graphs) has exactly one node.
8+
9+
[[e2e]]
10+
name = "single-node chains inherit even with multi-node sibling graph"
11+
steps = [
12+
"vp run foo",
13+
]
14+
15+
[[e2e]]
16+
name = "multi-node ancestor forces piped for nested single-node graph"
17+
steps = [
18+
"vp run -r foo-nested",
19+
]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
source: crates/vite_task_bin/tests/e2e_snapshots/main.rs
3+
expression: e2e_outputs
4+
---
5+
> vp run -r foo-nested
6+
~/packages/other$ check-ttycache disabled: no cache config
7+
stdin:not-tty
8+
stdout:not-tty
9+
stderr:not-tty
10+
11+
$ check-ttycache disabled: no cache config
12+
stdin:not-tty
13+
stdout:not-tty
14+
stderr:not-tty
15+
16+
17+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
18+
Vite+ Task RunnerExecution Summary
19+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
20+
21+
Statistics: 2 tasks0 cache hits0 cache misses2 cache disabled
22+
Performance: 0% cache hit rate
23+
24+
Task Details:
25+
────────────────────────────────────────────────
26+
[1] other#bar: ~/packages/other$ check-tty
27+
Cache disabled in task configuration
28+
·······················································
29+
[2] stdio-graph-criteria-test#bar: $ check-tty
30+
Cache disabled in task configuration
31+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
source: crates/vite_task_bin/tests/e2e_snapshots/main.rs
3+
expression: e2e_outputs
4+
---
5+
> vp run foo
6+
$ check-ttycache disabled: no cache config
7+
stdin:tty
8+
stdout:tty
9+
stderr:tty
10+
11+
$ check-ttycache disabled: no cache config
12+
stdin:tty
13+
stdout:tty
14+
stderr:tty
15+
16+
~/packages/other$ check-ttycache disabled: no cache config
17+
stdin:not-tty
18+
stdout:not-tty
19+
stderr:not-tty
20+
21+
$ check-ttycache disabled: no cache config
22+
stdin:not-tty
23+
stdout:not-tty
24+
stderr:not-tty
25+
26+
27+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
28+
Vite+ Task RunnerExecution Summary
29+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
30+
31+
Statistics: 4 tasks0 cache hits0 cache misses4 cache disabled
32+
Performance: 0% cache hit rate
33+
34+
Task Details:
35+
────────────────────────────────────────────────
36+
[1] stdio-graph-criteria-test#foo: $ check-tty
37+
Cache disabled in task configuration
38+
·······················································
39+
[2] stdio-graph-criteria-test#bar: $ check-tty
40+
Cache disabled in task configuration
41+
·······················································
42+
[3] other#check-tty: ~/packages/other$ check-tty
43+
Cache disabled in task configuration
44+
·······················································
45+
[4] stdio-graph-criteria-test#check-tty: $ check-tty
46+
Cache disabled in task configuration
47+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

0 commit comments

Comments
 (0)