Skip to content

Comments

fix poetry add with PEP 735 dependency-groups with include-group#10636

Merged
radoering merged 1 commit intopython-poetry:mainfrom
radoering:fix-add-with-include-group
Dec 7, 2025
Merged

fix poetry add with PEP 735 dependency-groups with include-group#10636
radoering merged 1 commit intopython-poetry:mainfrom
radoering:fix-add-with-include-group

Conversation

@radoering
Copy link
Member

@radoering radoering commented Nov 29, 2025

Pull Request Check List

Resolves: #10632

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

Summary by Sourcery

Fix handling of PEP 735 dependency groups in poetry add when groups contain include-group entries, ensuring dependencies are correctly updated without errors.

Bug Fixes:

  • Ensure poetry add correctly processes dependency groups that include include-group items by tracking them alongside normal dependencies.

Tests:

  • Extend poetry add command tests to cover dependency groups containing include-group entries and verify correct group contents after additions.

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 29, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts how poetry add tracks existing dependencies when using PEP 735 dependency-groups so that entries using include-group are handled safely, and extends tests to cover these cases.

Class diagram for updated dependency tracking in poetry add

classDiagram
    class AddCommand {
        - project_dependency_names list~NormalizedName_or_placeholder~
        + handle() int
        + get_existing_packages_from_input(packages list~str~, section dict~str_Any~, project_dependencies Collection~NormalizedName_or_placeholder~) list~str~
    }

    class NormalizedName {
    }

    class NormalizedName_or_placeholder {
        <<type_alias>>
        + NormalizedName
        + literal_placeholder <include-group>
    }

    class Dependency {
        + create_from_pep_508(spec str) Dependency
        + name NormalizedName
    }

    AddCommand --> Dependency : uses
    AddCommand --> NormalizedName_or_placeholder : tracks
    NormalizedName_or_placeholder --> NormalizedName : includes
Loading

File-Level Changes

Change Details Files
Track dependency-group entries that use include-group with a sentinel so index-based replacement during poetry add works correctly.
  • Change project_dependency_names from an untyped list to `list[NormalizedName
Literal[""]]to support a non-package sentinel value.</li><li>Update the list comprehension that buildsproject_dependency_namesfromgroups_content[group]to insert the sentinel string for non-string (i.e.include-group) entries while still extracting normalized names for PEP 508 strings.</li><li>Add type annotation to get_existing_packages_from_inputso itsproject_dependencies` parameter accepts the same union including the sentinel value.
Extend poetry add tests to cover PEP 735 dependency-groups that contain include-group entries and ensure they are preserved correctly when adding or updating dependencies.
  • Update test_add_to_existing_group to define a dependency-group containing both a regular dependency and an include-group entry and assert that the include-group entry is preserved when a new dependency is added.
  • Update test_add_to_group_with_latest_overwrite_existing similarly to include an include-group entry and verify it remains intact while existing version constraints are updated.
  • Ensure the included group referenced by include-group is present (as an empty list) in the test dependency-groups configuration.
tests/console/commands/test_add.py

Assessment against linked issues

Issue Objective Addressed Explanation
#10632 Modify poetry add so that it correctly handles PEP 735 dependency-groups entries that are inline tables (e.g. {include-group = "lint"}) without crashing, preserving the include-group entries and adding the new dependency.
#10632 Add automated tests covering poetry add behavior with dependency-groups that contain include-group inline table entries to prevent regressions.

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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • Consider extracting the string literal "" into a module-level constant or type alias so that the sentinel is defined in one place and not repeated in both the type hints and logic.
  • The union type NormalizedName | Literal["<include-group>"] is now used in multiple places; introducing a dedicated type alias (e.g., ProjectDependencyName) would improve readability and make future changes to that union easier.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider extracting the string literal "<include-group>" into a module-level constant or type alias so that the sentinel is defined in one place and not repeated in both the type hints and logic.
- The union type `NormalizedName | Literal["<include-group>"]` is now used in multiple places; introducing a dedicated type alias (e.g., `ProjectDependencyName`) would improve readability and make future changes to that union easier.

## Individual Comments

### Comment 1
<location> `tests/console/commands/test_add.py:648-651` </location>
<code_context>
     "cachy (>=0.2.0,<0.3.0)",
+    {include-group = "included"},
 ]
+included = []
 """
     )
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding a test where the included dependency group itself contains dependencies to ensure it isn’t affected by the `add` operation.

With `included = []`, we only check that having an `include-group` entry doesn’t break `poetry add`, but we don’t verify that the included group’s contents are preserved. To strengthen this regression test, populate `included` with a dependency (e.g. `"some-package (>=1.0.0,<2.0.0)"`) and assert that `included` is unchanged after `poetry add`, while `example` is updated. This better protects against regressions where the index-based replacement logic mutates the wrong group.

Suggested implementation:

```python
included = ["some-package (>=1.0.0,<2.0.0)"]
"""

```

```python
    assert "example" in pyproject["dependency-groups"]
    assert pyproject["dependency-groups"]["example"] == [
        "cachy (>=0.2.0,<0.3.0)",
        {"include-group": "included"},
        "pendulum (>=1.4.4,<2.0.0)",
    ]
    # Ensure that the included group's contents are preserved by `poetry add`
    assert pyproject["dependency-groups"]["included"] == [
        "some-package (>=1.0.0,<2.0.0)",
    ]
    if additional_poetry_group:

```
</issue_to_address>

### Comment 2
<location> `tests/console/commands/test_add.py:646-651` </location>
<code_context>
 [dependency-groups]
 example = [
     "cachy (>=0.2.0,<0.3.0)",
+    {include-group = "included"},
 ]
+included = []
</code_context>

<issue_to_address>
**suggestion (testing):** Add a dedicated test case with multiple `include-group` entries in the same dependency group to cover index-based replacement edge cases.

The updated tests only cover a single `{include-group = "included"}` entry at different positions. Because the implementation does index-based replacement, please also add a case where a group contains multiple `include-group` entries (e.g. `[{include-group = "included-a"}, {include-group = "included-b"}, "cachy ..."]`) and verify that `poetry add` updates the concrete dependency while preserving all `include-group` entries.

Suggested implementation:

```python
[dependency-groups]
example = [
    {include-group = "included-a"},
    "cachy (>=0.2.0,<0.3.0)",
    {include-group = "included-b"},
]
included-a = []
included-b = []
"""
    )
    pyproject["dependency-groups"] = groups_content["dependency-groups"]
    assert "example" in pyproject["dependency-groups"]
    assert pyproject["dependency-groups"]["example"] == [
        {"include-group": "included-a"},
        "cachy (>=0.2.0,<0.3.0)",
        {"include-group": "included-b"},
        "pendulum (>=1.4.4,<2.0.0)",
    ]
    if additional_poetry_group:

```

To fully align with your review comment ("Add a dedicated test case"), you may want to:
1. Extract this scenario into its own test function (e.g. `test_add_to_dependency_group_with_multiple_include_groups`) by copying the surrounding test setup/teardown code and using the modified dependency-groups content there.
2. Keep the original single-`include-group` test as-is (revert this change in that original test) and place the multi-`include-group` variant in the new test to ensure both single and multiple include-group scenarios are covered.
3. If the `poetry add` invocation in this test is meant to *update* an existing dependency rather than only add a new one, also assert that the updated constraint for the concrete dependency (e.g. `"cachy (...)"`) is correctly changed while both `{"include-group": "included-a"}` and `{"include-group": "included-b"}` remain present and in the same relative positions.
</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.

@radoering radoering force-pushed the fix-add-with-include-group branch from bf67798 to 8acc4e5 Compare December 7, 2025 05:56
@radoering radoering merged commit 4030e9c into python-poetry:main Dec 7, 2025
54 checks passed
@github-actions
Copy link

github-actions bot commented Jan 7, 2026

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 Jan 7, 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.

Poetry add crashes on PEP 735 dependency-groups with inline tables

1 participant