Replace BuildMode enum with composable filter logic

Eliminates the BuildMode enum (Full, Incremental, Targets, Subset,
ScopeWithUpstream) and determine_build_mode() function. Each build
constraint (incremental, targets, scope) computes a NodeIdMap32 filter
independently, then filters are intersected when multiple apply.

This fixes the bug where `putup -B build build/putup` built all 98
commands instead of just the 57 needed for the putup binary. The
incremental and target constraints now compose naturally.

Move collect_affected_commands and collect_required_commands from
scheduler.cpp anonymous namespace to pup::graph (dag.hpp/dag.cpp)
where they belong as graph algorithms.

Simplify scheduler public API from 4 build methods to 1:
  build(state, filter=nullptr)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: typeless

include/pup/exec/scheduler.hpp

@@ -89,31 +89,9 @@ class Scheduler final {
     Scheduler(Scheduler&&) noexcept;
     auto operator=(Scheduler&&) noexcept -> Scheduler&;
 
-    /// Build from a dependency graph
+    /// Build commands matching the filter. nullptr = build all.
     [[nodiscard]]
-    auto build(graph::BuildState const& graph) -> Result<BuildStats>;
-
-    /// Build only the commands that depend on changed files
-    [[nodiscard]]
-    auto build_incremental(
-        graph::BuildState const& graph,
-        Vec<StringId> const& changed_files
-    ) -> Result<BuildStats>;
-
-    /// Build only a specific subset of commands
-    [[nodiscard]]
-    auto build_subset(
-        graph::BuildState const& graph,
-        NodeIdMap32 const& command_ids
-    ) -> Result<BuildStats>;
-
-    /// Build specific targets and all required dependencies.
-    /// Uses reverse traversal to find commands needed.
-    [[nodiscard]]
-    auto build_targets(
-        graph::BuildState const& graph,
-        Vec<NodeId> const& target_ids
-    ) -> Result<BuildStats>;
+    auto build(graph::BuildState const& state, NodeIdMap32 const* filter = nullptr) -> Result<BuildStats>;
 
     /// Set callback for job start
     auto on_job_start(JobStartCallback callback) -> void;
@@ -140,14 +118,6 @@ class Scheduler final {
 private:
     std::unique_ptr<Impl> impl_;
 
-    /// Core build loop shared by all public build methods.
-    /// When filter is non-null, only matching commands run.
-    [[nodiscard]]
-    auto run(
-        graph::BuildState const& graph,
-        NodeIdMap32 const* filter
-    ) -> Result<BuildStats>;
-
     /// Execute jobs in parallel using the async event loop
     auto execute_parallel(
         Vec<BuildJob> const& jobs,

include/pup/graph/dag.hpp

@@ -357,6 +357,16 @@ struct BuildState {
 [[nodiscard]]
 auto make_build_state() -> BuildState;
 
+/// Collect all commands affected by the given changed files.
+/// Uses forward traversal: starts at changed inputs, walks forward through outputs.
+[[nodiscard]]
+auto collect_affected_commands(Graph const& graph, Vec<StringId> const& changed_files) -> NodeIdMap32;
+
+/// Collect all commands required to build the given target nodes.
+/// Uses reverse traversal: starts at targets, walks backward through inputs.
+[[nodiscard]]
+auto collect_required_commands(Graph const& graph, Vec<NodeId> const& target_ids) -> NodeIdMap32;
+
 /// Set build root name and clear path cache
 auto set_build_root_name(BuildState& state, std::string_view name) -> void;
 

src/cli/cmd_build.cpp

@@ -1037,40 +1037,19 @@ auto remove_stale_outputs(
     }
 }
 
-/// Build mode precedence (highest to lowest):
-/// 1. Incremental - if old index exists and files changed
-/// 2. ScopeWithUpstream - fresh build with -a and explicit targets
-/// 3. Targets - if specific output targets requested (and not incremental)
-/// 4. Subset - exclude config commands from full build
-/// 5. Full - build everything
-enum class BuildMode {
-    Incremental,
-    ScopeWithUpstream,
-    Targets,
-    Subset,
-    Full,
-};
-
-auto determine_build_mode(
-    bool has_targets,
-    bool use_incremental,
-    bool has_config_cmds,
-    bool scope_with_upstream
-) -> BuildMode
+auto intersect_filters(
+    pup::NodeIdMap32 const& a,
+    pup::NodeIdMap32 const& b,
+    pup::graph::Graph const& graph
+) -> pup::NodeIdMap32
 {
-    if (use_incremental) {
-        return BuildMode::Incremental;
-    }
-    if (scope_with_upstream) {
-        return BuildMode::ScopeWithUpstream;
-    }
-    if (has_targets) {
-        return BuildMode::Targets;
-    }
-    if (has_config_cmds) {
-        return BuildMode::Subset;
+    auto result = pup::NodeIdMap32 {};
+    for (auto id : pup::graph::all_nodes(graph)) {
+        if (a.contains(id) && b.contains(id)) {
+            result.set(id, 1);
+        }
     }
-    return BuildMode::Full;
+    return result;
 }
 
 /// Build a single variant with the given options.
@@ -1359,46 +1338,49 @@ auto build_single_variant(
     }
 
     auto start = pup::SteadyClock::time_point { pup::SteadyClock::now() };
-    auto build_result = pup::Result<pup::exec::BuildStats> {};
-
-    auto scope_with_upstream = opts.include_all_deps && !scopes.empty() && !use_incremental;
-    auto mode = determine_build_mode(
-        !target_node_ids.empty(),
-        use_incremental,
-        !config_cmds.empty(),
-        scope_with_upstream
-    );
 
-    switch (mode) {
-    case BuildMode::Incremental: {
-        build_result = scheduler.build_incremental(bs, changed_files);
-        break;
+    // Composable filter: layer independent concerns, intersect when combined
+    auto filter = pup::NodeIdMap32 {};
+    auto has_filter = false;
+
+    if (use_incremental && !changed_files.empty()) {
+        filter = pup::graph::collect_affected_commands(bs.graph, changed_files);
+        has_filter = true;
     }
-    case BuildMode::ScopeWithUpstream: {
+
+    if (!target_node_ids.empty()) {
+        auto required = pup::graph::collect_required_commands(bs.graph, target_node_ids);
+        if (has_filter) {
+            filter = intersect_filters(filter, required, bs.graph);
+        } else {
+            filter = std::move(required);
+            has_filter = true;
+        }
+    }
+
+    if (opts.include_all_deps && !scopes.empty() && !use_incremental) {
         auto scope_cmds = collect_scope_with_upstream_commands(bs, scopes);
         for (auto const& cfg : config_cmds) {
             scope_cmds.remove(cfg.cmd_id);
         }
-        build_result = scheduler.build_subset(bs, scope_cmds);
-        break;
+        if (has_filter) {
+            filter = intersect_filters(filter, scope_cmds, bs.graph);
+        } else {
+            filter = std::move(scope_cmds);
+            has_filter = true;
+        }
     }
-    case BuildMode::Targets:
-        build_result = scheduler.build_targets(bs, target_node_ids);
-        break;
-    case BuildMode::Subset: {
-        auto non_config_cmds = pup::NodeIdMap32 {};
+
+    if (!config_cmds.empty() && !has_filter) {
         for (auto id : pup::graph::all_nodes(bs.graph)) {
             if (node_id::is_command(id) && !config_cmd_ids.contains(id)) {
-                non_config_cmds.set(id, 1);
+                filter.set(id, 1);
             }
         }
-        build_result = scheduler.build_subset(bs, non_config_cmds);
-        break;
-    }
-    case BuildMode::Full:
-        build_result = scheduler.build(bs);
-        break;
+        has_filter = true;
     }
+
+    auto build_result = scheduler.build(bs, has_filter ? &filter : nullptr);
     auto end = pup::SteadyClock::time_point { pup::SteadyClock::now() };
     auto duration = std::chrono::milliseconds { std::chrono::duration_cast<std::chrono::milliseconds>(end - start) };
 

src/cli/cmd_configure.cpp

@@ -182,7 +182,7 @@ auto configure_single_variant(
         }
     });
 
-    auto build_result = scheduler.build_subset(ctx.graph(), all_commands);
+    auto build_result = scheduler.build(ctx.graph(), &all_commands);
 
     if (!opts.verbose) {
         printf("\n");

src/exec/scheduler.cpp

@@ -329,113 +329,6 @@ auto validate_guard_dependencies(
     return {};
 }
 
-/// Collect all commands required to build the given target nodes.
-/// Uses reverse traversal: starts at targets, walks backward through inputs.
-auto collect_required_commands(
-    graph::Graph const& graph,
-    Vec<NodeId> const& target_ids
-) -> NodeIdMap32
-{
-    auto visited = NodeIdMap32 {};
-    auto commands = NodeIdMap32 {};
-    auto stack = Vec<NodeId> {};
-    for (auto id : target_ids) {
-        stack.push_back(id);
-    }
-
-    while (!stack.empty()) {
-        auto id = stack.back();
-        stack.pop_back();
-
-        if (visited.contains(id)) {
-            continue;
-        }
-        visited.set(id, 1);
-
-        if (node_id::is_command(id) && graph::get_command_node(graph, id)) {
-            commands.set(id, 1);
-        }
-
-        for (auto input_id : graph::get_inputs(graph, id)) {
-            stack.push_back(input_id);
-        }
-
-        for (auto dep_id : graph::get_order_only(graph, id)) {
-            stack.push_back(dep_id);
-        }
-    }
-
-    return commands;
-}
-
-/// Compute all commands affected by the given changed files.
-/// Uses forward traversal: starts at changed inputs, walks forward through outputs.
-auto collect_affected_commands(
-    graph::Graph const& graph,
-    Vec<StringId> const& changed_files
-) -> NodeIdMap32
-{
-    auto& pool = global_pool();
-
-    auto path_to_id = Vec<std::pair<std::string_view, NodeId>> {};
-    for (auto id : graph::all_nodes(graph)) {
-        auto path_id = graph::get_full_path(graph, id);
-        if (!is_empty(path_id)) {
-            path_to_id.emplace_back(pool.get(path_id), id);
-        }
-    }
-    std::sort(path_to_id.begin(), path_to_id.end());
-
-    auto affected = NodeIdMap32 {};
-    auto to_process = Vec<NodeId> {};
-
-    for (auto file_id : changed_files) {
-        auto file_path = pool.get(file_id);
-        auto it = std::lower_bound(path_to_id.begin(), path_to_id.end(), file_path, [](auto const& p, auto const& k) { return p.first < k; });
-        if (it != path_to_id.end() && it->first == file_path) {
-            auto id = it->second;
-            if (!affected.contains(id)) {
-                affected.set(id, 1);
-                to_process.push_back(id);
-            }
-
-            auto const* node = graph::get_file_node(graph, id);
-            if (node && node->type == NodeType::Generated) {
-                for (auto input_id : graph::get_inputs(graph, id)) {
-                    if (!affected.contains(input_id)) {
-                        affected.set(input_id, 1);
-                        to_process.push_back(input_id);
-                    }
-                }
-            }
-        }
-    }
-
-    // Expand to include all dependent commands (including order-only).
-    // get_outputs() excludes sticky edges by design (Tupfile/config dependencies
-    // are parse-time deps, not build-time deps).
-    while (!to_process.empty()) {
-        auto id = NodeId { to_process.back() };
-        to_process.pop_back();
-
-        for (auto dep_id : graph::get_outputs(graph, id)) {
-            if (!affected.contains(dep_id)) {
-                affected.set(dep_id, 1);
-                to_process.push_back(dep_id);
-            }
-        }
-
-        for (auto dep_id : graph::get_order_only_dependents(graph, id)) {
-            if (!affected.contains(dep_id)) {
-                affected.set(dep_id, 1);
-                to_process.push_back(dep_id);
-            }
-        }
-    }
-
-    return affected;
-}
-
 struct JobSlot {
     pup::platform::AsyncProcess process = {};
     HeapBuf stdout_buf = {};
@@ -609,45 +502,7 @@ auto Scheduler::stats() const -> BuildStats
 // Public build API
 // ---------------------------------------------------------------------------
 
-auto Scheduler::build(graph::BuildState const& state) -> Result<BuildStats>
-{
-    return run(state, nullptr);
-}
-
-auto Scheduler::build_incremental(
-    graph::BuildState const& state,
-    Vec<StringId> const& changed_files
-) -> Result<BuildStats>
-{
-    auto affected = collect_affected_commands(state.graph, changed_files);
-    return run(state, &affected);
-}
-
-auto Scheduler::build_subset(
-    graph::BuildState const& state,
-    NodeIdMap32 const& command_ids
-) -> Result<BuildStats>
-{
-    return run(state, &command_ids);
-}
-
-auto Scheduler::build_targets(
-    graph::BuildState const& state,
-    Vec<NodeId> const& target_ids
-) -> Result<BuildStats>
-{
-    auto cmds = collect_required_commands(state.graph, target_ids);
-    return run(state, &cmds);
-}
-
-// ---------------------------------------------------------------------------
-// Private implementation
-// ---------------------------------------------------------------------------
-
-auto Scheduler::run(
-    graph::BuildState const& state,
-    NodeIdMap32 const* filter
-) -> Result<BuildStats>
+auto Scheduler::build(graph::BuildState const& state, NodeIdMap32 const* filter) -> Result<BuildStats>
 {
     impl_->cancelled = false;
     impl_->stats = BuildStats {};