feat: support Nx namedInputs for global invalidation#53
Merged
EladBezalel merged 8 commits intomainfrom Mar 30, 2026
Merged
Conversation
…terns
Parse namedInputs from nx.json to support two key behaviors:
1. Global invalidation: workspace-root patterns (e.g., babel.config.json,
patches/*) mark all projects as affected when matched
2. Negation patterns: files matching patterns like !{projectRoot}/**/*.figma.tsx
are excluded from triggering affected status
Resolves named input references recursively (e.g., default → sharedGlobals → ciInputs).
Closes #50
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughResolves Nx Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as find_affected_internal()
participant Resolver as named_inputs::resolve_from_nx_json()
participant NxFile as nx.json
participant GlobalCheck as named_inputs::check_global_invalidation()
participant Negation as named_inputs::filter_negated_files()
participant Analyzer as remaining affected analysis
Caller->>Resolver: resolve_from_nx_json(cwd)
Resolver->>NxFile: read & deserialize namedInputs
NxFile-->>Resolver: resolved patterns
Resolver-->>Caller: ResolvedNamedInputs
Caller->>GlobalCheck: check_global_invalidation(changed_files)
alt global pattern matched
GlobalCheck-->>Caller: Some(trigger_file)
Caller->>Analyzer: short-circuit — mark ALL projects affected (GlobalInvalidation)
else no global match
GlobalCheck-->>Caller: None
Caller->>Negation: filter_negated_files(changed_files, projects)
Negation-->>Caller: filtered_changed_files
Caller->>Analyzer: proceed with standard analysis(filtered_changed_files)
Analyzer-->>Caller: affected projects
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/core.rs (1)
80-81: Remove unnecessary.clone()onall_projects.The
all_projectsvector is already owned and can be moved directly intoaffected_projectswithout cloning.♻️ Suggested fix
return Ok(AffectedResult { - affected_projects: all_projects.clone(), + affected_projects: all_projects, report: if generate_report {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core.rs` around lines 80 - 81, Return value construction is cloning all_projects unnecessarily: instead of building AffectedResult with affected_projects: all_projects.clone(), move the owned all_projects into affected_projects (i.e., remove the .clone()). Update the return in the code that constructs AffectedResult so it uses affected_projects: all_projects to avoid the extra allocation while preserving types for AffectedResult and all_projects.tests/integration_test.rs (1)
3051-3053: Consider canonicalizing the temp directory path for consistency.Other test helpers in this file (e.g.,
test_js_to_ts_extension_resolutionat line 1685) canonicalize the temp directory path to handle symlinks (e.g.,/var→/private/varon macOS). This ensures path consistency with the resolver's canonicalized output.While the current tests may pass because all operations are relative to the temp directory, canonicalizing would align with the established pattern.
♻️ Suggested fix
fn root(&self) -> &std::path::Path { - self.dir.path() + // Note: Consider storing canonicalized path in struct initialization + // to match other test helpers in this file + self.dir.path() }Or store the canonicalized path during construction:
struct TempNxRepo { dir: TempDir, root: PathBuf, // canonicalized } impl TempNxRepo { fn new(nx_json: &str) -> Self { let dir = TempDir::new().unwrap(); let root = dir.path().canonicalize().unwrap_or_else(|_| dir.path().to_path_buf()); // ... rest of init using `root` ... Self { dir, root } } fn root(&self) -> &Path { &self.root } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration_test.rs` around lines 3051 - 3053, The TempNxRepo::root method currently returns self.dir.path() which may differ from the resolver's canonicalized paths; modify TempNxRepo to store a canonicalized root PathBuf during construction (e.g., in new) by calling dir.path().canonicalize() with a fallback to dir.path().to_path_buf() on error, update all initialization to use that canonicalized root, and change root(&self) to return &self.root so tests use the canonicalized temp directory consistently.src/named_inputs.rs (2)
208-223: Consider pre-compiling negation patterns for better performance.Currently,
Pattern::new(suffix)is called on every invocation ofis_negated. For large monorepos with many changed files and multiple negation suffixes, this repeated compilation could become a performance concern.Consider storing pre-compiled
Patternobjects innegation_suffixes(similar toglobal_patterns) instead of raw strings.♻️ Suggested approach
pub struct ResolvedNamedInputs { /// Glob patterns for workspace-root files that invalidate all projects pub global_patterns: Vec<Pattern>, - /// Negation glob patterns for project-root files to exclude - /// Stored as (pattern_suffix) where the caller replaces {projectRoot} with actual root - pub negation_suffixes: Vec<String>, + /// Pre-compiled negation glob patterns for project-root files to exclude + pub negation_patterns: Vec<Pattern>, }Then in
is_negated:- for suffix in &self.negation_suffixes { - match Pattern::new(suffix) { - Ok(pat) => { - if pat.matches_with(relative_str, opts) { + for pat in &self.negation_patterns { + if pat.matches_with(relative_str, opts) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/named_inputs.rs` around lines 208 - 223, The loop in is_negated repeatedly calls Pattern::new(suffix) which compiles patterns per-file; change negation_suffixes from Vec<String> to Vec<Pattern> (or a new struct holding compiled patterns) and compile them once where negation_suffixes are populated (handle and log any Pattern::new errors there, similar to global_patterns), then in is_negated iterate the precompiled patterns and call pat.matches_with(relative_str, opts) (remove Pattern::new from is_negated and update references to negation_suffixes accordingly).
83-85: Unnecessary pattern verification.The
let _ = pat.matches_with("test", opts);call doesn't serve a meaningful purpose.Pattern::newalready validates the glob syntax, and this test match against a hardcoded string doesn't verify anything useful about whether the pattern will work correctly with actual file paths.♻️ Suggested fix
Ok(pat) => { debug!("Global pattern: {}", suffix); - // Verify the pattern actually works with our options - let _ = pat.matches_with("test", opts); global_patterns.push(pat); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/named_inputs.rs` around lines 83 - 85, Remove the redundant verification call that does a test match on the pattern: delete the `let _ = pat.matches_with("test", opts);` line in the code where `pat` is created before `global_patterns.push(pat)` (in src/named_inputs.rs). `Pattern::new` already validates syntax so simply push `pat` onto `global_patterns` without the extra `matches_with` invocation; ensure no other logic depends on the discarded result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/core.rs`:
- Around line 80-81: Return value construction is cloning all_projects
unnecessarily: instead of building AffectedResult with affected_projects:
all_projects.clone(), move the owned all_projects into affected_projects (i.e.,
remove the .clone()). Update the return in the code that constructs
AffectedResult so it uses affected_projects: all_projects to avoid the extra
allocation while preserving types for AffectedResult and all_projects.
In `@src/named_inputs.rs`:
- Around line 208-223: The loop in is_negated repeatedly calls
Pattern::new(suffix) which compiles patterns per-file; change negation_suffixes
from Vec<String> to Vec<Pattern> (or a new struct holding compiled patterns) and
compile them once where negation_suffixes are populated (handle and log any
Pattern::new errors there, similar to global_patterns), then in is_negated
iterate the precompiled patterns and call pat.matches_with(relative_str, opts)
(remove Pattern::new from is_negated and update references to negation_suffixes
accordingly).
- Around line 83-85: Remove the redundant verification call that does a test
match on the pattern: delete the `let _ = pat.matches_with("test", opts);` line
in the code where `pat` is created before `global_patterns.push(pat)` (in
src/named_inputs.rs). `Pattern::new` already validates syntax so simply push
`pat` onto `global_patterns` without the extra `matches_with` invocation; ensure
no other logic depends on the discarded result.
In `@tests/integration_test.rs`:
- Around line 3051-3053: The TempNxRepo::root method currently returns
self.dir.path() which may differ from the resolver's canonicalized paths; modify
TempNxRepo to store a canonicalized root PathBuf during construction (e.g., in
new) by calling dir.path().canonicalize() with a fallback to
dir.path().to_path_buf() on error, update all initialization to use that
canonicalized root, and change root(&self) to return &self.root so tests use the
canonicalized temp directory consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dfed4c1f-58bb-4490-9281-2cd2d9e4c9c1
📒 Files selected for processing (7)
src/core.rssrc/lib.rssrc/main.rssrc/named_inputs.rssrc/report.rssrc/types.rstests/integration_test.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/integration_test.rs">
<violation number="1" location="tests/integration_test.rs:2980">
P2: Check git command exit status in test setup helpers; `.output().unwrap()` can hide failed git operations and make tests flaky/misleading.</violation>
</file>
<file name="src/core.rs">
<violation number="1" location="src/core.rs:108">
P1: Negation patterns for `!{projectRoot}/...` are matched against `source_root`, which can break exclusion when `sourceRoot` differs from Nx `projectRoot`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
📦 Preview Release AvailableA preview release has been published for commit fe29537. Installationnpm install https://github.com/frontops-dev/domino/releases/download/pr-53-fe29537/front-ops-domino-1.1.1.tgzRunning the previewnpx https://github.com/frontops-dev/domino/releases/download/pr-53-fe29537/front-ops-domino-1.1.1.tgz affectedDetails |
Move global invalidation check and negation filtering into dedicated functions in named_inputs.rs, keeping core.rs focused on orchestration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/named_inputs.rs (2)
209-224: Consider pre-compiling negation patterns to avoid repeated compilation.
Pattern::new(suffix)is called on every invocation ofis_negated. With many changed files and multiple project roots, this could become a hot path. Since the suffixes are known at resolution time, pre-compiling them inresolve_from_nx_json(similar toglobal_patterns) would improve performance.Sketch of change
pub struct ResolvedNamedInputs { pub global_patterns: Vec<Pattern>, - pub negation_suffixes: Vec<String>, + pub negation_patterns: Vec<Pattern>, }Then in
resolve_from_nx_json, compile patterns when adding tonegation_suffixes, and updateis_negatedto iterate pre-compiled patterns directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/named_inputs.rs` around lines 209 - 224, The negation patterns are being recompiled on every is_negated call (Pattern::new(suffix)), which is wasteful; precompile them during resolution instead—modify resolve_from_nx_json to store compiled Pattern instances (similar to how global_patterns is populated) into negation_suffixes (or rename to negation_patterns) and change is_negated to iterate the precompiled Pattern objects directly (use the existing Pattern.matches_with call) and remove Pattern::new from that hot path; keep the existing error handling when compiling in resolve_from_nx_json so invalid patterns are warned once at resolution time.
81-86: Remove dead code: the verification call has no effect.The
pat.matches_with("test", opts)result is discarded, and the pattern is pushed unconditionally regardless.Pattern::new()already validates glob syntax—this line adds no additional verification.Suggested fix
match Pattern::new(suffix) { Ok(pat) => { debug!("Global pattern: {}", suffix); - // Verify the pattern actually works with our options - let _ = pat.matches_with("test", opts); global_patterns.push(pat); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/named_inputs.rs` around lines 81 - 86, Remove the dead verification call to pat.matches_with("test", opts) in the Pattern::new(suffix) match arm: the result is discarded and Pattern::new already validates syntax, so delete the line that invokes pat.matches_with and leave the pattern push (global_patterns.push(pat)) intact; update the match arm around Pattern::new(suffix) to no longer call pat.matches_with while preserving debug!("Global pattern: {}", suffix) and error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/named_inputs.rs`:
- Around line 209-224: The negation patterns are being recompiled on every
is_negated call (Pattern::new(suffix)), which is wasteful; precompile them
during resolution instead—modify resolve_from_nx_json to store compiled Pattern
instances (similar to how global_patterns is populated) into negation_suffixes
(or rename to negation_patterns) and change is_negated to iterate the
precompiled Pattern objects directly (use the existing Pattern.matches_with
call) and remove Pattern::new from that hot path; keep the existing error
handling when compiling in resolve_from_nx_json so invalid patterns are warned
once at resolution time.
- Around line 81-86: Remove the dead verification call to
pat.matches_with("test", opts) in the Pattern::new(suffix) match arm: the result
is discarded and Pattern::new already validates syntax, so delete the line that
invokes pat.matches_with and leave the pattern push (global_patterns.push(pat))
intact; update the match arm around Pattern::new(suffix) to no longer call
pat.matches_with while preserving debug!("Global pattern: {}", suffix) and error
handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28c76f61-e5de-48e8-af90-7d8f6530388c
📒 Files selected for processing (2)
src/core.rssrc/named_inputs.rs
Add `root` field to Project struct to distinguish project directory
from source root. Negation patterns like `!{projectRoot}/**/*.figma.tsx`
now correctly match against the project root directory, not sourceRoot
which may point to a subdirectory like `libs/my-lib/src`.
Also assert git command exit status in test helpers to catch failures
early instead of silently ignoring them.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib.rs">
<violation number="1" location="src/lib.rs:56">
P1: `Project.root` is being populated from `source_root`, which is semantically different and can break `{projectRoot}`-based matching for N-API callers.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib.rs`:
- Around line 53-57: The conversion currently overwrites Project.root with
project.source_root when building the NapiProject (see the struct fields name,
root, source_root), which loses the original root value used by JS callers
(discover_projects() -> find_affected()); change the conversion to preserve and
copy Project.root into the NapiProject.root (e.g., use
PathBuf::from(&project.root) or clone the existing root field if present)
instead of always assigning source_root, ensuring NapiProject keeps both root
and source_root distinct and forwarded correctly.
In `@src/workspace/nx.rs`:
- Around line 239-245: The code sets project.root to PathBuf::from(&name) but Nx
workspace.json stores the directory in each project's "root" property; update
the NxProjectJson struct to include a root: Option<String> (or String) field,
deserialize it, and when building Project (the place that currently does let
root = PathBuf::from(&name) and projects.push(Project { name, root, source_root,
... })) replace that assignment to use the parsed nx_json.root
(PathBuf::from(root_str)) when present, falling back to PathBuf::from(&name) if
the root property is absent; adjust any references to NxProjectJson and Project
construction accordingly.
In `@tests/integration_test.rs`:
- Around line 3023-3046: The namedInputs fixture never exercises root !=
sourceRoot because both project.json files only set "name" so discovery infers
root == sourceRoot; update the lib-a project.json write (the string passed to
fs::write for root.join("libs/lib-a/project.json") in tests/integration_test.rs)
to include a "sourceRoot": "libs/lib-a/src" field while keeping the project root
directory as "libs/lib-a" and leave the existing src/index.ts file in place;
this ensures the namedInputs test exercises the case where projectRoot !=
sourceRoot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 774fa2d3-a925-477d-ae17-e8e69a2c392f
📒 Files selected for processing (10)
src/core.rssrc/lib.rssrc/named_inputs.rssrc/semantic/resolve_options.rssrc/types.rssrc/utils.rssrc/workspace/nx.rssrc/workspace/rush.rssrc/workspace/workspaces.rstests/integration_test.rs
✅ Files skipped from review due to trivial changes (1)
- src/semantic/resolve_options.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/named_inputs.rs
- Add `root` field to NapiProject so JS callers preserve the distinction between project root and sourceRoot across the N-API boundary - Parse `root` property from workspace.json project entries instead of assuming the key name is the filesystem path - Add integration test exercising root != sourceRoot for negation patterns - Set explicit sourceRoot in lib-a fixture to test the divergent case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integration_test.rs (1)
2989-3002: Consider consolidating with existinggit_inhelper.This
run_gitfunction is nearly identical to the existinggit_inhelper at lines 1664-1678. The only difference is the panic message format. Consider reusinggit_ininTempNxRepoto reduce duplication.♻️ Suggested consolidation
impl TempNxRepo { fn new(nx_json: &str) -> Self { let dir = TempDir::new().unwrap(); let root = dir.path(); // Init git - run_git(root, &["init", "-q"]); - run_git(root, &["config", "user.email", "test@example.com"]); + git_in(root, &["init", "-q"]); + git_in(root, &["config", "user.email", "test@example.com"]); // ... etcThen remove the
run_gitfunction entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration_test.rs` around lines 2989 - 3002, The new run_git function duplicates existing git_in; remove run_git and update callers (e.g., in TempNxRepo tests) to call the existing git_in helper instead to avoid duplication. Locate the run_git definition and replace its usages with git_in (preserving argument order and current_dir semantics) and then delete the run_git function; ensure any panic/message expectations rely on git_in's behavior or adjust calls to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration_test.rs`:
- Around line 3263-3293: The test
test_named_inputs_negation_with_root_differs_from_source_root is placing the
changed file outside sourceRoot so attribution logic in
get_package_names_by_path (in src/utils.rs / ProjectIndex) will not assign it to
lib-a and the test will pass for the wrong reason; change the
repo.change_and_commit call to create the file under the package source root
(e.g., "libs/lib-a/src/Button.figma.tsx") so the file is attributed to lib-a and
the negation pattern ({projectRoot}/**/*.figma.tsx) is actually exercised, and
update the test comments accordingly to reflect that we’re verifying negation
against project root while the file is inside sourceRoot.
---
Nitpick comments:
In `@tests/integration_test.rs`:
- Around line 2989-3002: The new run_git function duplicates existing git_in;
remove run_git and update callers (e.g., in TempNxRepo tests) to call the
existing git_in helper instead to avoid duplication. Locate the run_git
definition and replace its usages with git_in (preserving argument order and
current_dir semantics) and then delete the run_git function; ensure any
panic/message expectations rely on git_in's behavior or adjust calls to match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8048f796-9be8-4e31-ba85-fe93500986e1
📒 Files selected for processing (3)
src/lib.rssrc/workspace/nx.rstests/integration_test.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/workspace/nx.rs
…leanup
- Pre-compile negation patterns during resolution instead of on every
is_negated call (perf improvement for large monorepos)
- Remove dead pat.matches_with("test", opts) verification call
- Remove unnecessary all_projects.clone()
- Fix test_named_inputs_negation_with_root_differs_from_source_root: move
changed file inside sourceRoot so it's properly attributed to the project
(previously passed for the wrong reason — file was unowned, not negated)
- Remove duplicate run_git helper, reuse existing git_in
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/integration_test.rs">
<violation number="1" location="tests/integration_test.rs:3267">
P2: This test no longer verifies the intended projectRoot-vs-sourceRoot distinction because the changed file is now inside `sourceRoot`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Add missing `head`, `root` fields and close truncated assert from merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib.rs (1)
31-39:⚠️ Potential issue | 🟠 MajorRegenerate TypeScript declarations—
NapiProjectinterface is missing therootfield.The
NapiProjectRust struct (src/lib.rs:33) includespub root: Option<String>, but the TypeScript declaration inindex.d.ts(lines 23-29) is missing the correspondingroot?: stringfield. Sinceindex.d.tsis auto-generated by NAPI-RS, runyarn buildto regenerate the type declarations and keep them in sync with the Rust implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib.rs` around lines 31 - 39, The TypeScript declarations are out of sync: the Rust struct NapiProject contains pub root: Option<String> but index.d.ts is missing root?: string; update the generated types by running the build that emits NAPI-RS declarations (e.g., run yarn build) so index.d.ts is regenerated to include root?: string, then verify the NapiProject interface in index.d.ts matches the Rust fields and commit the updated declaration.
🧹 Nitpick comments (2)
src/named_inputs.rs (2)
183-186: Minor docstring inconsistency: says "source root" but means "project root".The docstring says "
project_rootis the project's source root" but the parameter represents the project root directory (where project.json lives), not the source root. The calling code infilter_negated_filescorrectly passesp.root, notp.source_root.📝 Suggested docstring fix
/// Check if a changed file should be excluded by negation patterns. /// `file_path` should be relative to workspace root. - /// `project_root` is the project's source root (relative to workspace root). + /// `project_root` is the project's root directory (relative to workspace root). pub fn is_negated(&self, file_path: &Path, project_root: &Path) -> bool {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/named_inputs.rs` around lines 183 - 186, Docstring for is_negated incorrectly calls project_root the "source root"; update the comment to say project_root is the project's root directory (where project.json lives) to match how filter_negated_files passes p.root. Edit the doc comment above pub fn is_negated(&self, file_path: &Path, project_root: &Path) to replace "source root" with "project root (directory where project.json lives)" and ensure any example wording matches the usage in filter_negated_files that passes p.root rather than p.source_root.
74-79: Workspace-root negations are explicitly not supported.The code logs that
!{workspaceRoot}/...patterns are skipped. This is a reasonable limitation to document, but consider whether this should be awarn!instead ofdebug!to make it more visible to users who might expect these patterns to work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/named_inputs.rs` around lines 74 - 79, The negation-handling branch that matches when negated.strip_prefix("{workspaceRoot}/") currently logs with debug! and should be more visible; change the call from debug!(...) to warn!(...) for the branch that logs "Negation pattern (workspace-root): !{} — skipping (not yet supported)" so users see that workspace-root negations (the negated variable `negated` matching strip_prefix("{workspaceRoot}/")) are skipped; keep the same message and suffix interpolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib.rs`:
- Around line 31-39: The TypeScript declarations are out of sync: the Rust
struct NapiProject contains pub root: Option<String> but index.d.ts is missing
root?: string; update the generated types by running the build that emits
NAPI-RS declarations (e.g., run yarn build) so index.d.ts is regenerated to
include root?: string, then verify the NapiProject interface in index.d.ts
matches the Rust fields and commit the updated declaration.
---
Nitpick comments:
In `@src/named_inputs.rs`:
- Around line 183-186: Docstring for is_negated incorrectly calls project_root
the "source root"; update the comment to say project_root is the project's root
directory (where project.json lives) to match how filter_negated_files passes
p.root. Edit the doc comment above pub fn is_negated(&self, file_path: &Path,
project_root: &Path) to replace "source root" with "project root (directory
where project.json lives)" and ensure any example wording matches the usage in
filter_negated_files that passes p.root rather than p.source_root.
- Around line 74-79: The negation-handling branch that matches when
negated.strip_prefix("{workspaceRoot}/") currently logs with debug! and should
be more visible; change the call from debug!(...) to warn!(...) for the branch
that logs "Negation pattern (workspace-root): !{} — skipping (not yet
supported)" so users see that workspace-root negations (the negated variable
`negated` matching strip_prefix("{workspaceRoot}/")) are skipped; keep the same
message and suffix interpolation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c71a656-07f8-4911-9370-32668e732e03
📒 Files selected for processing (5)
src/core.rssrc/lib.rssrc/named_inputs.rssrc/types.rstests/integration_test.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/integration_test.rs
- src/core.rs
Collaborator
Remaining nitpicks from reviewMost review comments have been addressed. Two minor items remain:
|
- Fix docstring: project_root is the project's root directory, not source root - Regenerate index.d.ts to include missing root, head, and lockfileStrategy fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@EladBezalel addressed your comment |
EladBezalel
approved these changes
Mar 30, 2026
Collaborator
EladBezalel
left a comment
EladBezalel
left a comment
There was a problem hiding this comment.
All review comments addressed. LGTM.
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
2 participants
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
namedInputsfromnx.jsonand resolve named references recursively (e.g.,default→sharedGlobals→ciInputs)babel.config.json,patches/*) trigger global invalidation — all projects are marked affected!{projectRoot}/**/*.figma.tsx) exclude matching files from triggering affected statusnamedInputsis absent fromnx.jsonCloses #50
Changes
src/named_inputs.rs(new): Parses and resolves namedInputs config from nx.json, classifies patterns into global (workspaceRoot) and negation (projectRoot) categoriessrc/core.rs: Integrates namedInputs early in the pipeline — global pattern check short-circuits, negation filter removes excluded filessrc/types.rs: AddedGlobalInvalidationcause variant for reportssrc/report.rs: Handles new cause variant in graph and detail HTML viewstests/integration_test.rs: 6 new integration tests covering global invalidation, negation, recursive resolution, glob wildcards, and no-config fallbackTesting
cargo test --lib— 177 tests)cargo clippy --all-targets)cargo fmt --all)babel.config.jsonchange → 1662 affected (global invalidation)patches/*.patchchange → 1662 affected (glob wildcard)build/crxmake/common.pychange → 1662 affected (deep glob)ci/utils.Jenkinsfilechange → 1662 affected (exact match).figma.tsxfile change → 0 affected (negation pattern).figma.tsx+ normal → 1 affected (negation + normal coexist)Summary by CodeRabbit
New Features
Tests