fix(orion): narrow Buck target discovery for subproject all-added imports#2129
Conversation
…state Fetch logs only for the selected build (SSE while building), replace manual batch fetch with React Query caching, cap CachedLogPanel mounts with LRU, and split index.tsx into useBuildSelection/useLogCache hooks. Scope buildId and logs to CL via atomFamily, sync ?tab=check&build= URL, remember last build per task, and add retry on log fetch errors.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e26a0d508e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
benjamin-747
force-pushed
the
main
branch
3 times, most recently
from
b140ad9 to
14bdf41
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14bdf418e6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| None | ||
| }; | ||
|
|
||
| let base = if all_added { |
There was a problem hiding this comment.
When a normal CL only adds files and is not an all-added subproject import, all_added is true but all_added_subproject is false, so this branch replaces the old graph with an empty Targets. The later collect_impacted_targets(..., EmptyBasePolicy::SelectAll) path treats an empty base as “every target is new”, causing a single added source/test file to build every target in the narrowed subtree or even the whole repo instead of just its affected owners.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4393d4c1d6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…orts Scope discovery to changed subtrees and rk8s subproject roots so all-added CLs build rust_library/rust_binary targets instead of :vendor filegroups or entire third-party graphs. Add graph-based rdeps when uquery fails, platform --config injection, toolchain helper filtering, and optional ORION_BUCK_REMOTE_CACHE. Move troubleshooting docs to orion/docs and extend orion-scheduler VM/image deploy support.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 329ef47163
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| if let Some(target) = diff.targets().find(|t| t.label() == label) | ||
| && (is_buckal_plumbing_target(target) || !is_rust_build_rule(&target.rule_type)) |
There was a problem hiding this comment.
For all-added CLs the graph path is skipped, so owner() is the only way to seed builds; this branch drops any owned target whose rule is not rust_library/rust_binary/rust_test by trying to replace it with Rust targets in the same package. When an added JS/CXX/Python/etc. source is owned by a non-Rust target, there are no Rust targets to push, the seed list becomes empty, and Orion reports no impacted targets instead of building the owner. Limit this rewrite to buckal plumbing targets or keep the original non-Rust label.
Useful? React with 👍 / 👎.
| if !is_rust_build_rule(&target.rule_type) { | ||
| continue; |
There was a problem hiding this comment.
When buck2 uquery rdeps fails, this in-graph fallback is supposed to recover reverse dependencies from the narrowed buck2 targets graph, but it only retains Rust build rules. For a narrowed change whose dependent targets are CXX/Python/JS/etc., those rdeps are present in the graph but are skipped here, so the build set remains just the seeds and misses affected non-Rust dependents. The fallback should only exclude toolchain/plumbing helpers, not all non-Rust rules.
Useful? React with 👍 / 👎.
2 participants
No description provided.