|WARNING| [v1] dts: update dts check format script and resolve errors

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/165224

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-10

# DPDK Patch Review

## Patch: dts: update dts check format script and resolve errors

### Summary
This patch updates tool versions in the DTS check-format script (mypy 1.13.0 - 2.1.0, ruff 0.8.1 - 0.15.16, types packages) and fixes formatting/type-hinting issues that became visible with the new versions.

---

## Errors

### 1. **Incorrect type annotations in decorator wrappers** (API design)
**Files:** `dts/api/testpmd/__init__.py`, `dts/framework/remote_session/interactive_shell.py`

The decorator wrappers are annotated to return `Callable[P, "TestPmd"] | Any` (or similar), which is incorrect. A wrapper function should return the same type as the wrapped function, not the class itself.

```python
# Current (incorrect):
def _wrapper(
    self: "TestPmd", *args: P.args, **kwargs: P.kwargs
) -> Callable[P, "TestPmd"] | Any:
```

The return type should match what the decorated function returns. If the original function returns `Self` (the instance), the wrapper should indicate that:

```python
# Correct:
def _wrapper(
    self: "TestPmd", *args: P.args, **kwargs: P.kwargs
) -> R:
    # where R is bound from func: Callable[..., R]
```

Or simply omit the overly-specific type annotation:
```python
def _wrapper(self: "TestPmd", *args: P.args, **kwargs: P.kwargs) -> Any:
```

---

### 2. **Unconventional formatting of assert statements**
**File:** `dts/framework/config/__init__.py` and multiple others

The patch reformats assert statements to wrap the message in parentheses on a continuation line:
```python
assert sut_node_port_peer is not None, (
    f"Invalid SUT node port specified for link port_topology.{link_idx}."
)
```

While syntactically correct, the guideline prefers continuation lines double-indented without extra grouping:
```python
assert sut_node_port_peer is not None, \
        f"Invalid SUT node port specified for link port_topology.{link_idx}."
```

The parenthesized format adds visual noise. Stick to backslash continuation or natural line breaks.

---

### 3. **Inconsistent type narrowing in flag constructors**
**File:** `dts/api/testpmd/types.py`

```python
flag: RSSOffloadTypesFlag = cls(0)
```

This explicit annotation is unnecessary. The assignment `cls(0)` already has type `Self` from the class method return type. The pattern appears three times (`RSSOffloadTypesFlag`, `PacketOffloadFlag`, `RtePTypes`). Remove the redundant type annotation:

```python
flag = cls(0)
```

---

### 4. **Incorrect use of `type(self).none` instead of class name**
**File:** `dts/framework/utils.py`

```python
return (
    f"{self.value}"
    if self == self.none
    else f"{TarCompressionFormat.none.value}.{self.value}"
)
```

The original code used `type(self).none` (dynamic class reference), which is correct for subclasses. Hardcoding `TarCompressionFormat.none` breaks if this code is inherited or moved. Revert to:

```python
else f"{type(self).none.value}.{self.value}"
```

---

### 5. **Unnecessary variable and incorrect type annotation**
**File:** `dts/framework/utils.py`

```python
compression_format: Any = f"w:{compress_format.value}"
with tarfile.open(target_tarball_path, compression_format) as tar:
```

The variable `compression_format` is assigned a string but annotated as `Any`, which defeats the purpose of type hints. The original code directly passed the f-string to `tarfile.open()`, which is clearer:

```python
with tarfile.open(target_tarball_path, f"w:{compress_format.value}") as tar:
```

If the variable is kept, annotate it as `str`:
```python
compression_format: str = f"w:{compress_format.value}"
```

---

## Warnings

### 1. **Overly long lines are now accepted (100 chars)**
**File:** `dts/pyproject.toml` and comments in patch

The DPDK C coding standard specifies lines should be <=100 characters. The patch updates meson/python rules to accept this, which is fine for Python. However, several formatted lines in the patch are still noticeably long (e.g., log messages). Consider splitting excessively long f-strings for readability.

---

### 2. **Inconsistent f-string formatting**
**Files:** `dts/framework/cpu.py`, `dts/tests/TestSuite_cryptodev_throughput.py`

The patch changes:
```python
# Before:
f'{",".join(...)}'
# After:
f"{','.join(...)}"
```

Both are valid. The change from single-quotes to double-quotes is a style preference, not a correctness issue. Ensure this aligns with the project's Black/Ruff configuration.

---

### 3. **Poetry lock version bump from 2.0 - 2.1**
**File:** `dts/poetry.lock`

The lock file metadata changed. Ensure the team has Poetry 2.x installed. If CI or developers use Poetry 1.x, this will cause conflicts. Document the Poetry version requirement if not already done.

---

### 4. **Dependency upgrades without explicit justification**
**Files:** `dts/pyproject.toml`, `dts/poetry.lock`

- mypy: 1.13.0 - 2.1.0 (major version bump)
- ruff: 0.8.1 - 0.15.16 (major version bump)
- types-* packages: updated

Major version bumps of dev tools can introduce breaking changes in linting/type-checking. The commit message says "formatting and type hinting errors that weren't previously visible have now appeared," but doesn't list what changed in the tools. Document why these upgrades are needed (e.g., bug fixes, new features, compatibility).

---

### 5. **Missing release notes**
The patch updates dev tooling and fixes code style, but does not modify user-facing API or functionality. Release notes are not required. This is correct.

---

## Info

### 1. **Good: Consistent application of style fixes**
The patch systematically addresses the same patterns across multiple files (f-string quotes, assert formatting, explicit type annotations). This indicates the fixes are automated or carefully reviewed.

---

### 2. **Consider: Black configuration**
If the project uses Black for Python formatting, many of these changes (f-string quotes, line breaks) should be automated. Ensure `pyproject.toml` has a `[tool.black]` section with `line-length = 100` to match the new standard.

---

### 3. **Tooling dependency sprawl**
The lock file adds many new packages (`ast-serialize`, `librt`, `pathspec`) as dependencies of mypy 2.x. This is expected but increases the supply chain attack surface. Consider periodic audits of transitive dependencies.

---

## Summary

**Must Fix (Errors):**
1. Decorator return type annotations are semantically wrong (`Callable[P, Class]` should be the wrapped function's return type)
2. `type(self).none` - `TarCompressionFormat.none` hardcoding breaks extensibility
3. Remove unnecessary `compression_format` variable or annotate it as `str`, not `Any`

**Should Fix (Warnings):**
1. Revert `flag: RSSOffloadTypesFlag = cls(0)` to `flag = cls(0)` (redundant annotation)
2. Document why mypy/ruff major version bumps are required

**Consider:**
1. Use backslash continuation instead of parenthesized assert messages for consistency with DPDK style
2. Verify Poetry 2.x is documented/required