-
Notifications
You must be signed in to change notification settings - Fork 233
feat(nx-mcp): make nx_project_details more token efficient by default #2905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
bb17622 to
0ba631b
Compare
|
View your CI Pipeline Execution ↗ for commit f859ffb
☁️ Nx Cloud last updated this comment at |
| for (const target of targets) { | ||
| if (targets.some((t) => t !== target && t.startsWith(target))) { | ||
| rootTargets.add(target); | ||
|
|
||
| const atomizedTargets = targets.filter( | ||
| (t) => t.startsWith(target) && t !== target, | ||
| ); | ||
| atomizedTargetsMap.set(target, atomizedTargets); | ||
| targetsToExclude.push(...atomizedTargets); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic incorrectly identifies root targets when target names overlap. If a targetGroup contains ['test', 'test-ci', 'test-ci--Test1'], the code will:
- Mark
testas a root target with['test-ci', 'test-ci--Test1']as atomized targets - Also mark
test-cias a root target with['test-ci--Test1']as atomized target - Add
test-ci--Test1totargetsToExcludetwice
This causes incorrect filtering and display. Fix by enforcing the double-dash pattern:
for (const target of targets) {
const atomizedTargets = targets.filter(
(t) => t !== target && t.startsWith(`${target}--`)
);
if (atomizedTargets.length > 0) {
rootTargets.add(target);
atomizedTargetsMap.set(target, atomizedTargets);
targetsToExclude.push(...atomizedTargets);
}
}| for (const target of targets) { | |
| if (targets.some((t) => t !== target && t.startsWith(target))) { | |
| rootTargets.add(target); | |
| const atomizedTargets = targets.filter( | |
| (t) => t.startsWith(target) && t !== target, | |
| ); | |
| atomizedTargetsMap.set(target, atomizedTargets); | |
| targetsToExclude.push(...atomizedTargets); | |
| } | |
| for (const target of targets) { | |
| const atomizedTargets = targets.filter( | |
| (t) => t !== target && t.startsWith(`${target}--`), | |
| ); | |
| if (atomizedTargets.length > 0) { | |
| rootTargets.add(target); | |
| atomizedTargetsMap.set(target, atomizedTargets); | |
| targetsToExclude.push(...atomizedTargets); | |
| } | |
| } |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Co-authored-by: MaxKless <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nx Cloud is proposing a fix for your failed CI:
This change fixes the detectAtomizedTargets function to only exclude targets that actually follow the atomized naming pattern (rootTarget--*). The previous implementation incorrectly filtered out all targets in a targetGroup except the root, which caused nx:run-script targets like deploy and custom-hello (from package.json scripts) to disappear from the compressed output. With this fix, only true atomized targets are excluded while maintaining the PR's token-efficiency improvements.
We verified this fix by re-running nx-mcp-e2e:e2e-ci--src/nx-project-details-compressed-targets.test.ts.
diff --git a/libs/shared/llm-context/src/lib/project-graph.ts b/libs/shared/llm-context/src/lib/project-graph.ts
index db0802ae..dc370c7b 100644
--- a/libs/shared/llm-context/src/lib/project-graph.ts
+++ b/libs/shared/llm-context/src/lib/project-graph.ts
@@ -31,7 +31,10 @@ export function detectAtomizedTargets(targetGroups: Record<string, string[]>): {
// The group name is the root target name (from metadata)
const rootTarget = groupName;
- const atomizedTargets = targets.filter((t) => t !== rootTarget);
+ // Only consider targets atomized if they follow the pattern: rootTarget--*
+ const atomizedTargets = targets.filter(
+ (t) => t !== rootTarget && t.startsWith(`${rootTarget}--`),
+ );
if (atomizedTargets.length > 0) {
rootTargets.add(rootTarget);
Or Apply changes locally with:
npx nx-cloud apply-locally Pi6Q-3USi
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
No description provided.