fix(dependency_graph): remove orphaned children in remove_dependency

[jskladan]

When a node is removed, child nodes that have no remaining parents are now recursively removed as well, preventing orphaned nodes from lingering in the graph.

Closes: #1041

[X] PR follows CONTRIBUTING.md guidelines

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bc358d88-d6b0-4cdb-9e0c-64e3c3604996

📥 Commits

Reviewing files that changed from the base of the PR and between f827fd2 and ff21497.

📒 Files selected for processing (2)

• src/fromager/dependency_graph.py
• tests/test_dependency_graph.py

---

📝 Walkthrough

Walkthrough

DependencyGraph.remove_dependency() was rewritten to remove the target node and then iteratively remove any descendant nodes that become orphaned (no remaining parents). The implementation snapshots direct children, removes the deleted node from each child’s parents and from each parent’s children, deletes the node from self.nodes, and enqueues children that now have no parents (excluding ROOT) for further removal until the queue is empty. A local build_graph() helper and multiple tests (test_remove_dependency*) were added to cover leaf removal, cascading orphan removal, shared-child preservation, diamond topologies, mid-graph removals, already-removed targets, and removal of nonexistent nodes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing remove_dependency to remove orphaned children from the graph.
Description check	✅ Passed	The description accurately explains the change: orphaned child nodes are now recursively removed when a parent is deleted, and references issue #1041.
Linked Issues check	✅ Passed	The PR implementation meets all requirements from #1041: it removes orphaned children iteratively via queue-based approach, cleans up stale parent references, and recursively/iteratively removes descendants without remaining parents.
Out of Scope Changes check	✅ Passed	All changes are in-scope: dependency_graph.py implements the orphan-removal fix, and test_dependency_graph.py adds comprehensive test coverage for the new behavior with no unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[rd4398]

This looks good!

[LalatenduMohanty]

@mergify rebase

[mergify]

rebase

✅ Branch has been successfully rebased

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/fromager/dependency_graph.py`:
- Around line 389-392: The recursive call in remove_dependency (the loop over
children checking child.key != ROOT and child.key in self.nodes and not
child.parents) causes repeated global child-edge cleanup and O(n²) behavior
during cascades; fix by avoiding re-entering remove_dependency for each orphan —
instead, when you detect an orphan node (use children, ROOT, self.nodes,
child.parents, child.canonicalized_name, child.version to locate it), remove it
from its direct parents' children lists and add it to a local queue of nodes to
delete, then process that queue iteratively (or mark nodes for bulk deletion) so
global child-edge cleanup runs once per node rather than once per orphan
recursion.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dc241074-4cae-4bfb-b1ea-5676ca7ac3e5

📥 Commits

Reviewing files that changed from the base of the PR and between bc300d4 and 1f8c31c.

📒 Files selected for processing (2)

• src/fromager/dependency_graph.py
• tests/test_dependency_graph.py

[jskladan]

@coderabbitai please re-review this

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@jskladan Sure, I'll re-review the PR now!

[full_review]

[jskladan]

@rd4398 / @LalatenduMohanty - if you could have a look, I addressed the feedback from the Coderabbit, it IMO made sense to make this into a pretty straightforward stack-driven BFS instead of the recursion (funny enough, I've seen the coderabbit implementation after I've made the changes 🥲 didn't know it proposes detailed solutions, but it is virtually the same, only I decided to use dequeue instead of plain list)

[LalatenduMohanty]

@jskladan The BFS approach is the better choice here since it avoids stack depth issues on deep dependency chains and makes the traversal order explicit . You can squash the commits. I can approve the PR after that

[jskladan]

Done. Squashed and rebased.