NVIDIA / NVIDIA/TensorRT-LLM

[Bug]: ruff-legacy pre-commit hook reports all baseline violations as regressions on Windows

Open Beginner friendly
#17,743 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Windows
Dominant language
Python
Stars
14.7k
Forks
2.8k
Avg merge
2d 23h
Merged PRs (30d)
489

Description

Summary

On Windows, the ruff-legacy pre-commit hook reports every violation already
recorded in ruff-legacy-baseline.json as a new regression. It fails on files
that have not been touched at all, so the hook blocks every commit that
includes a legacy file and gives Windows contributors no way to distinguish a
real regression from the noise.

Reproduction

On a Windows checkout, with a clean working tree:

$ git status --porcelain          # empty
$ python scripts/legacy_utils.py lint-precommit \
      tensorrt_llm/serve/tool_parser/base_tool_parser.py \
      tests/unittest/llmapi/apps/test_tool_parsers.py

New violations (28 regressions vs baseline):
  tensorrt_llm\serve\tool_parser\base_tool_parser.py:46  D212  Multi-line docstring summary should start at the first line
  tensorrt_llm\serve\tool_parser\base_tool_parser.py:93  D205  1 blank line required between summary line and description
  ...
  tests\unittest\llmapi\apps\test_tool_parsers.py:2072  D202  No blank lines allowed after function docstring (found 1)

Both files are unmodified, and the reported counts match the baseline exactly:
base_tool_parser.py has D212: 6 in the baseline and six D212 violations in
the working tree.

Cause

normalize_path in scripts/legacy_utils.py builds the lookup key with
str(p.relative_to(repo_root)), which uses the platform separator:

https://github.com/NVIDIA/TensorRT-LLM/blob/main/scripts/legacy_utils.py#L124-L132

On Windows that yields tensorrt_llm\serve\tool_parser\base_tool_parser.py,
while ruff-legacy-baseline.json stores forward-slash keys. baseline.get(filepath, {})
in compare_against_baseline therefore always misses, every rule falls back to
a baseline count of 0, and every current violation counts as a regression.

The same normalized key feeds current_counts, checked_files,
identify_new_violations and the auto-fix report, so a single fix in
normalize_path covers all of them.

Suggested fix

Return a POSIX path, matching how the baseline is stored:

return p.relative_to(repo_root).as_posix()

This is a no-op on Linux and macOS, where as_posix() already matches str().

Environment
  • Windows 11, Git Bash / PowerShell
  • TensorRT-LLM main at f8c7f55b2b

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in scripts/legacy_utils.py at normalize_path, then read how its result is used by compare_against_baseline and the related violation-reporting paths. Make the normalized key match the forward-slash paths in ruff-legacy-baseline.json, then rerun the provided Windows reproduction and confirm unchanged files no longer report baseline violations as regressions.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
90/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.