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: d71ac83c-7c71-4766-b6ce-2ef031dc03a6

📥 Commits

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

📒 Files selected for processing (2)

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

---

📝 Walkthrough

Walkthrough

The change updates DependencyGraph.remove_dependency() to delete the target node and then iteratively remove any descendant nodes that become orphaned (have no remaining parents). It 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. Tests were added with a local build_graph() helper and multiple test_remove_dependency* cases covering leaf removal, cascading orphan removal, shared-child preservation, diamond topologies, mid-graph removals, and removing 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 summarizes the main change: removing orphaned children when a dependency node is deleted, which matches the primary objective in the changeset.
Description check	✅ Passed	The description accurately relates to the changeset, explaining that child nodes with no remaining parents are recursively removed to prevent orphaned nodes, matching the code changes.
Linked Issues check	✅ Passed	The PR implements all coding requirements from #1041: removes stale parent references from child nodes, recursively removes children with no remaining parents, and uses iterative processing to avoid full-graph scans.
Out of Scope Changes check	✅ Passed	All changes are scoped to the remove_dependency() functionality and its tests; no unrelated modifications to other parts of the codebase are present.

_{✏️ 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