|WARNING| [v2] 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/165699

_AI Code Review_

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

# DPDK DTS Patch Review

## Summary

This patch updates dependency versions in the DTS (DPDK Test Suite) tooling and fixes resulting formatting/type hinting issues. The changes are primarily mechanical fixes to satisfy updated linter and type checker versions.

---

## Findings

### **Errors**

None identified.

### **Warnings**

#### 1. **Type Hint Return Type Error in Decorators**

**Files:** `dts/api/testpmd/__init__.py` (lines 79, 105, 135), `dts/framework/remote_session/interactive_shell.py` (line 53)

**Issue:** The wrapper functions in these decorators return `Callable[Concatenate["TestPmd", P], Any]` or similar, but they should return the actual return type `R` (or `Any`) of the wrapped function, not a callable type.

**Why it matters:** The type signature is incorrect. When you call a decorated function, you get back the function's return value, not another callable. This will cause type checking failures for callers.

**Suggested fix:**

```python
# In dts/api/testpmd/__init__.py
@functools.wraps(func)
def _wrapper(self: "TestPmd", *args: P.args, **kwargs: P.kwargs) -> R:
    if self.ports_started:
        self._logger.debug("Ports need to be stopped to continue.")
        self.stop_all_ports()
    return func(self, *args, **kwargs)

# In dts/framework/remote_session/interactive_shell.py
def _wrapper(self: "InteractiveShell", *args: P.args, **kwargs: P.kwargs) -> R | None:
    if self._ssh_channel.active:
        return func(self, *args, **kwargs)
    return None
```

The return type should be `R` (the wrapped function's return type), not `Callable[...]`.

---

#### 2. **Type Annotation Pattern in `filter_cores` Decorator**

**File:** `dts/framework/context.py` (line 111)

**Issue:** The decorator signature is confusing:
```python
def decorator(func: type["TestProtocol"]) -> Callable[P, type["TestProtocol"]]:
```

The parameter `func` should be a callable (the function being decorated), not `type["TestProtocol"]`. The return type `Callable[P, type["TestProtocol"]]` also appears incorrect--it should return the same callable type as the input or use proper ParamSpec syntax.

**Why it matters:** This type signature does not match how Python decorators work. `func` is a function, not a type object. Callers and type checkers will be confused.

**Suggested fix:**

```python
def decorator(func: Callable[P, Any]) -> Callable[P, Any]:
    @functools.wraps(func)
    def wrapper(*args: P.args, **kwargs: P.kwargs) -> Any:
        # ... implementation ...
```

Or if the return type is known to be a TestProtocol instance:

```python
def decorator(func: Callable[P, "TestProtocol"]) -> Callable[P, "TestProtocol"]:
```

---

#### 3. **Hardcoded Tarball Mode String Cast**

**File:** `dts/framework/utils.py` (lines 216-217)

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

**Issue:** The `cast()` tells the type checker that the runtime string is one of the literal types in `Tar_modes`, but there is no runtime validation. If `compress_format.value` is an unexpected value (e.g., due to a bug or enum addition), `tarfile.open()` will fail at runtime with a confusing error.

**Why it matters:** The code assumes `compress_format.value` always produces a valid tarfile mode, but the cast is purely for type checking--it does not enforce correctness.

**Suggested fix:**

Add a runtime assertion or validation before the cast:

```python
mode_str = f"w:{compress_format.value}"
assert mode_str in ("w:gz", "w:bz2", "w:xz", "w:tar"), \
    f"Invalid tarball mode: {mode_str}"
tarball_mode = cast(Tar_modes, mode_str)
with tarfile.open(target_tarball_path, tarball_mode) as tar:
```

Or use a mapping:

```python
MODE_MAP = {
    TarCompressionFormat.gzip: "w:gz",
    TarCompressionFormat.bzip2: "w:bz2",
    # ...
}
tarball_mode = MODE_MAP[compress_format]
```

---

#### 4. **Dependency Version Bumps Without Release Notes**

**Files:** `dts/pyproject.toml`, `dts/poetry.lock`

**Issue:** The patch updates `mypy` from `^1.13.0` to `^2.1.0`, `ruff` from `^0.8.1` to `^0.15.16`, and `types-paramiko`/`types-pyyaml` versions. These are significant version jumps (mypy 2.x is a major release with new mypyc runtime dependencies). However, there are no release notes documenting these changes or explaining any new tool behaviors/requirements.

**Why it matters:** DTS contributors need to know that the tooling has changed and may need to install new dependencies (`ast-serialize`, `librt`) or adjust their local environments. The lack of documentation may cause confusion.

**Suggested fix:** Add a note in the appropriate release notes file (if one exists for DTS) or in the commit message explaining the tool updates and any new setup requirements.

---

### **Info**

#### 1. **Mechanical Formatting Changes**

The patch includes many formatting fixes (line breaks, f-string formatting, operator spacing) that are purely mechanical cleanups required by the updated `ruff` version. These are appropriate and improve consistency.

#### 2. **Type Annotation Additions**

Explicit type annotations added to flag variables in `dts/api/testpmd/types.py` (lines 282, 963, 1171) are improvements for clarity and type checking correctness.

---

## Conclusion

The patch is mostly correct. The primary concerns are:

1. **Decorator return type annotations** are incorrect and need to return `R` (the wrapped function's return type), not `Callable[...]`.
2. **The `filter_cores` decorator** has a confusing type signature that does not match decorator semantics.
3. **The `cast()` in tarball mode selection** lacks runtime validation.
4. **Missing release notes** for significant dependency updates.

These are **Warnings** rather than errors because the code will compile and likely function correctly at runtime, but the type annotations are misleading and the cast is a latent bug risk.