AI review of dependency-groups

@sirosen, I ran a review (see https://iscinumpy.dev/post/claude-code-reviews/), below. Would you like me to fix any of these? We should add 3.15 to CI/classifiers since it's in RC phase, for sure.

🤖 AI text below 🤖

Review complete. I read the whole implementation, verified suspicious behaviors by running them, and checked the CLIs. The core resolver (cycle detection, caching, normalization) is sound — I traced the ancestor-tracking logic and could not construct a false-positive or false-negative cycle, including diamonds, self-includes, and resolver reuse after errors. The findings are all at the edges:

Bugs

1. Non-string include-group values produce a cryptic error — _implementation.py:161
The value of {include-group = ...} is never validated as a string. A TOML file with {include-group = 5} fails deep inside _normalize_name with TypeError: expected string or bytes-like object, got 'int', and the lint CLI dutifully reports that unhelpful message. A str check at parse time with a clear message ("include-group value must be a string") would fix both the API and lint output.

2. pip-install-dependency-groups crashes on an empty resolution — _pip_wrapper.py:58
If the requested groups resolve to zero requirements (e.g. empty = []), it runs pip install with no arguments, which fails with a CalledProcessError traceback. Verified. It should skip the pip call (arguably print "nothing to install") when resolved is empty.

3. The dependency-groups CLI shows raw tracebacks for user errors — __main__.py:55
dependency-groups nonexistent prints a full traceback ending in LookupError. Same for invalid TOML or a missing pyproject file (this last one applies to all three CLIs). The lint and pip-install CLIs catch LookupError/ValueError/TypeError and print tidy messages; the main CLI should too.

4. Type hints accept Mapping items but the implementation requires dict — _implementation.py:157
The signatures say Sequence[str | Mapping[str, str]], but _parse_group checks isinstance(item, dict), so a MappingProxyType({"include-group": "b"}) is rejected as "Invalid dependency group item" (verified). Either check Mapping or tighten the hints to dict.

Smaller issues

• __main__.main() takes no argv parameter while _lint_dependency_groups.main and _pip_wrapper.main both accept argv for testability. Worth aligning.
• --list ignores -o/--output and prints group names space-delimited (print(*keys)), which is ambiguous if a name contains a space. Newline-delimited output routed through the same output logic would be more consistent with the rest of the command.
• _toml_compat.py catches ImportError on the first import but ModuleNotFoundError on the second — harmless, but inconsistent for no reason.

Simplifications

• _normalize_group_names maintains both original_names (normalized → list of originals) and normalized_to_original (normalized → last original); the second is derivable from the first (names[0] after the duplicate check, since duplicates raise). One structure suffices.
• _parse_group and _resolve both end with "assign to cache dict, then read it back" (self._parsed_groups[group] = tuple(elements); return self._parsed_groups[group]); assigning a local and returning it is simpler.

Modernization

• Python 3.9 is past end-of-life (October 2025). requires-python = ">=3.9" plus the 3.9 classifier, tox env, and mypy-py39 could be dropped; that would also unlock dataclass(slots=True) and match-free but cleaner typing. This is a maintainer-policy call for a PyPA library, so I'd raise it with sirosen rather than just doing it.
• DependencyGroupInclude could be @dataclasses.dataclass(frozen=True) — it's a value type that ends up in cached tuples, so immutability fits (and it becomes hashable).
• Python 3.15 is in beta; CI/tox could add it to the test matrix alongside the existing 3.14 coverage.

No performance concerns: the resolver caches parses and resolutions, the 100-deep include chain in the test suite resolves fine, and data sizes here are trivially small.

Read the referenced areas in _implementation.py, _pip_wrapper.py, and main.py, then confirm with the maintainer which finding is in scope. Check the relevant CLI entry points and existing behavior described in the issue. Done means the selected user-facing error or empty-resolution case behaves clearly without affecting the other resolver behavior.