Add --skip/--skip-only flags; make group skip trim its dependency subtree#338
Merged
Conversation
…subtree `scope doctor run` could only select groups via `--only`; there was no way to deselect a group and prune its dependency subtree. Separately, a group's YAML `skip` config was resolved at execution time and only no-op'd that single node, so its dependencies ran anyway even though the group didn't. - Add `--skip`/`--skip-only` flags: `--skip` removes a group and any dependency not also required by another included group; `--skip-only` removes just the named group and keeps its dependencies. - Resolve a group's own `skip` config at planning time (in `resolve_skips`) so it trims its dependency subtree the same way `--skip` does, while still reporting the group as skipped in the run summary. - Fix the debug graph print to show group names instead of bare node indices (`Config::NodeIndexLabel` -> `Config::EdgeNoLabel`). - Guard `--skip-only` against force-adding dependencies of a group that was never going to run in the first place, and preserve `GroupReport` entries for planning-time-skipped groups. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…, close QA gaps compute_group_order took 4 positional args (2 same-typed BTreeSets), and resolve_skips returned a bare 3-tuple of same-typed sets — both easy to transpose silently. build_dependency_graph returned an unnamed tuple and used continue-based exclusion loops. - compute_group_order now takes a single GroupOrderParams struct. - build_dependency_graph returns a named DependencyGraph struct and filters excluded groups declaratively instead of via continue. - resolve_skips returns a named SkipResolution struct instead of a tuple. - Extracted wire_start_node/traversal_order_from to remove duplicated graph-traversal logic between reachable_group_names and compute_group_order. - Closed two gaps found by coverage/mutation testing: execute_command had no test, and the skip_subtree/skip_only short-circuit guard in resolve_skips had a surviving mutant (no test proved a group's own skip command is skipped once already excluded via --skip/--skip-only). - Tightened the skipped-group debug_assert in RunGroups::execute to also cover skipped_group, not just succeeded/failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RunGroups::execute ran everything in all_paths first, then looped over skipped_groups (a BTreeSet, alphabetical) and printed "Group skipped" warnings afterward — so skipped groups always appeared at the end of the run instead of where they'd naturally sit in the topological order. Thread the full (unpruned) candidate order through SkipResolution into RunGroups as `full_order`, and merge execute()/run_path() into a single pass over it: report a skip in its natural position, silently drop anything transitively pruned as an exclusive dependency, and otherwise run normally (preserving the existing skip_remaining cascade-on-failure behavior unchanged). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three real mutants survived a mutation-testing pass on this branch, all in untested warning/error paths rather than core logic: - should_skip_group's run_command error path (SkipSpec::Command) was never exercised, so an Err was indistinguishable from "don't skip". Added a unit test asserting the error propagates. - warn_on_unknown_group's condition could be deleted or inverted with no test noticing. Added E2E tests (matching the existing stdout-substring pattern used throughout this suite) asserting the warning fires for an unknown --skip/--skip-only name. - The "Could not find any tasks to execute" guard's `&&` could become `||` with no test noticing, which would spuriously warn whenever something was skipped but other groups still ran. Added an E2E test covering the mixed case (everything runnable was explicitly skipped). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rubberduck203
marked this pull request as ready for review
July 14, 2026 12:14
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.
Summary
--skip <name>and--skip-only <name>toscope doctor run.--skipremoves a group along with any dependency not also required by another included group (a shared dependency survives);--skip-onlyremoves just the named group and keeps its dependencies running.skipconfig (skip: trueorskip: { command }) is now resolved at planning time instead of execution time, so it trims its dependency subtree the same way--skipdoes, rather than just no-op'ing that one node. The group is still reported as skipped in the run summary.Config::NodeIndexLabel→Config::EdgeNoLabel).--skip-onlyagainst force-adding dependencies of a group that was never going to run in the first place, and preservesGroupReportentries for planning-time-skipped groups (parity with the old per-group skip check).Test plan
cargo build,cargo test(161 unit + 33 integration tests pass)cargo fmt --check,cargo clippy -- -D warningscleanskip-with-deps,group-skip-boolean-trims-dependency) for--skip,--skip-only, and config-basedskip, confirming console output and the-vvdebug graph dump (now shows group names)docs/docs/commands/doctor.md,docs/docs/models/ScopeDoctorGroup.mdx