Skip to content

fix: use set with backtracking for tree circular dependency detection#10811

Merged
radoering merged 2 commits into
python-poetry:mainfrom
dimbleby:fix/show-tree-circular-detection
Mar 31, 2026
Merged

fix: use set with backtracking for tree circular dependency detection#10811
radoering merged 2 commits into
python-poetry:mainfrom
dimbleby:fix/show-tree-circular-detection

Conversation

@dimbleby

@dimbleby dimbleby commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

packages_in_tree was a shared mutable list — siblings' subtrees would pollute each other, causing false circular dependency warnings. Changed to a set (correct data structure for membership checks) with backtracking (discard after recursion) to isolate sibling branches.

Pull Request Check List

Resolves: #issue-number-here

  • Added tests for changed code.
  • Updated documentation for changed code.

Summary by Sourcery

Fix circular dependency detection in the poetry show --tree command to avoid false positives when dependencies are shared between sibling nodes.

Bug Fixes:

  • Prevent shared dependencies between sibling packages in the tree view from being incorrectly reported as circular dependencies.

Tests:

  • Add a regression test to ensure shared dependencies between sibling nodes are not flagged as circular in the tree output.

@sourcery-ai

sourcery-ai Bot commented Mar 30, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors the dependency tree display to track visited packages using a backtracking set instead of a shared list, preventing false circular dependency warnings, and adds a regression test for shared sibling dependencies.

Class diagram for tree display methods using a backtracking set

classDiagram
    class ShowCommand {
        +display_package_tree(io, package, dependency, installed_packages)
        -_display_tree(io, dependency, installed_packages, packages_in_tree, previous_tree_bar, level) void
        -_write_tree_line(io, line) void
    }

    class IO {
        +output Output
    }

    class Output {
        +supports_utf8() bool
    }

    class Dependency {
        +name NormalizedName
    }

    class Package {
        +name NormalizedName
    }

    class NormalizedName

    ShowCommand --> IO : uses
    ShowCommand --> Dependency : traverses
    ShowCommand --> Package : root package
    ShowCommand --> NormalizedName : tracks in packages_in_tree

    ShowCommand : -packages_in_tree set~NormalizedName~
    ShowCommand : _display_tree uses backtracking on packages_in_tree
    ShowCommand : circular detection via membership in packages_in_tree
Loading

File-Level Changes

Change Details Files
Fix circular-dependency detection in tree view by using a backtracking set of visited packages instead of a shared mutable list.
  • Initialize packages_in_tree as a set of the root package and first dependency when starting the tree display.
  • Change _display_tree to accept and work with a set of visited package names instead of a list.
  • Remove per-iteration aliasing of the visited collection to avoid shared state between sibling branches.
  • Check circular dependencies via set membership on packages_in_tree.
  • On descending into a dependency, add its name to packages_in_tree before recursion and discard it immediately after returning to implement backtracking.
src/poetry/console/commands/show.py
Add regression test to ensure shared dependencies between sibling nodes are not marked as circular.
  • Construct a small package graph where two sibling dependencies share a common child dependency.
  • Mock lock data to reflect the constructed graph in the test environment.
  • Execute poetry show --tree and assert that the output does not contain a circular dependency warning for the shared dependency.
tests/console/commands/test_show.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The manual add/discard backtracking on packages_in_tree assumes there are no early returns or exceptions in the recursive branch; consider wrapping the recursion in a try/finally or using a helper/context manager to ensure the set is always restored correctly if the function gains additional exit paths later.
  • Now that packages_in_tree is a set, it might be clearer to rename it to something like ancestors or visited_ancestors to better convey that it tracks the current ancestor chain rather than an ordered tree path.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The manual add/discard backtracking on `packages_in_tree` assumes there are no early returns or exceptions in the recursive branch; consider wrapping the recursion in a `try`/`finally` or using a helper/context manager to ensure the set is always restored correctly if the function gains additional exit paths later.
- Now that `packages_in_tree` is a `set`, it might be clearer to rename it to something like `ancestors` or `visited_ancestors` to better convey that it tracks the current ancestor chain rather than an ordered tree path.

## Individual Comments

### Comment 1
<location path="src/poetry/console/commands/show.py" line_range="623" />
<code_context>
                     level + 1,
                 )

+                packages_in_tree.discard(dependency.name)
+
     def _write_tree_line(self, io: IO, line: str) -> None:
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard the add/discard pair with a try/finally to avoid leaking state on errors.

Since `packages_in_tree` is shared and mutated in place across recursive calls, any exception in `_display_tree` will bypass the `discard`, leaving stale entries in the set and causing later branches to be misidentified as circular. Please wrap the recursive call in a `try/finally` so the `discard` always runs:

```python
packages_in_tree.add(dependency.name)
try:
    self._display_tree(
        io,
        dependency,
        installed_packages,
        packages_in_tree,
        tree_bar,
        level + 1,
    )
finally:
    packages_in_tree.discard(dependency.name)
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/poetry/console/commands/show.py Outdated
@dosubot

dosubot Bot commented Mar 30, 2026

Copy link
Copy Markdown

Related Documentation

1 document(s) may need updating based on files changed in this PR:

Python Poetry

CHANGELOG /poetry/blob/main/CHANGELOG.md — ⏳ Awaiting Merge

Note: You must be authenticated to accept/decline updates.

How did I do? Any feedback?  Join Discord

dimbleby and others added 2 commits March 31, 2026 18:27
packages_in_tree was a shared mutable list — siblings' subtrees would
pollute each other, causing false circular dependency warnings.
Changed to a set (correct data structure for membership checks) with
backtracking (discard after recursion) to isolate sibling branches.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@radoering radoering force-pushed the fix/show-tree-circular-detection branch from d93db23 to 4febe90 Compare March 31, 2026 16:27
@radoering radoering enabled auto-merge (squash) March 31, 2026 16:28
@radoering radoering merged commit e541800 into python-poetry:main Mar 31, 2026
54 checks passed
@dimbleby dimbleby deleted the fix/show-tree-circular-detection branch March 31, 2026 16:48
@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants