refactor(core): resolve the descriptor once in check_dataflow - #3083
Open
phil-opp wants to merge 1 commit into
Open
refactor(core): resolve the descriptor once in check_dataflow#3083phil-opp wants to merge 1 commit into
phil-opp wants to merge 1 commit into
Conversation
`check_dataflow` resolved the descriptor three times per call: `check_dataflow_static` resolved it once (validate.rs), then called `check_wiring`, which resolved it a second time, and `check_dataflow` resolved it a third time for the path/runtime existence checks. `resolve_aliases_and_set_defaults` clones the entire node topology and rebuilds a `BTreeMap<NodeId, ResolvedNode>`, so this was O(nodes) wasted work on every `dora check` / `dora run` / `dora start` validation. Resolve once and thread the resolved node map through the checks: - `check_wiring_resolved(&nodes)` holds the wiring loop; `check_wiring` keeps its public signature by resolving then delegating. - `check_dataflow_static_resolved(dataflow, &nodes)` holds the resolution-dependent static checks; `check_dataflow_static` keeps its public signature the same way. - ROS2-config validation is extracted to `validate_ros2_configs` so it still runs on the unresolved descriptor *before* resolution, preserving the original error precedence (a ROS2 misconfig is reported ahead of an alias-resolution error). `check_dataflow` now resolves exactly once and reuses the result. Behavior and error ordering are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SD4dBVimzepwKSh2b9F8aG
Contributor
|
⏳ Waiting to start tests on this pull request - details. |
Collaborator
Author
|
🤖 Automated review by Claude — this is a fully automated review with no human in the loop. I reviewed this diff and found no issues. The refactor resolves the descriptor once and threads the resolved node map through Generated by Claude Code |
phil-opp
marked this pull request as ready for review
August 9, 2026 11:34
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
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.
Issue
check_dataflow(libraries/core/src/descriptor/validate.rs) resolved the descriptor three times on every call:check_dataflow_static→dataflow.resolve_aliases_and_set_defaults()check_dataflow_static→check_wiring(dataflow)→ resolves againcheck_dataflow→ resolves a third time for the path/runtime existence checksresolve_aliases_and_set_defaultsclones the entire node topology and rebuilds aBTreeMap<NodeId, ResolvedNode>. The three resolutions are deterministic and identical, so two of them are pure wasted work — O(nodes) on everydora check/dora run/dora startvalidation.Fix
Resolve once and thread the resolved node map through the checks:
check_wiring_resolved(&nodes)holds the wiring loop;check_wiringkeeps its public signature by resolving then delegating.check_dataflow_static_resolved(dataflow, &nodes)holds the resolution-dependent static checks;check_dataflow_statickeeps its public signature the same way.validate_ros2_configs, still run on the unresolved descriptor before resolution — preserving the original error precedence (a ROS2 misconfig is reported ahead of an alias-resolution error).check_dataflownow resolves exactly once and reuses the result. Public API signatures, behavior, and error ordering are all unchanged.Validation
cargo test -p dora-core --lib descriptor::— 156 passed (includes the fullvalidate::testsandwiring_*suites).cargo fmt --all -- --checkandcargo clippy -p dora-core -- -D warningsclean.🤖 This is a machine-generated pull request opened by Claude Code as part of an automated code-review pass. Please review carefully before merging.
🤖 Generated with Claude Code
Generated by Claude Code