Repository navigation
perf(python): compact repeated Ruff and SQLFluff descriptions - #4645
Conversation
📊 Automated PR Analysis
SummaryCompacts Ruff and SQLFluff lint output by hoisting a rule's description into its 'Top rules' summary line when every occurrence shares the same message, leaving per-location detail lines with just the rule code; distinct messages and rules outside the summary cap still keep their descriptions attached to each location. Adds seven regression tests (including a real Ruff JSON fixture) and makes Ruff's tied summary ordering deterministic, with no changes to dependencies, workflows, or runner routi… Review Checklist
Linked issues: #3920 Analyzed automatically by wshm · This is an automated analysis, not a human review. |
Protocol-zero-0
left a comment
There was a problem hiding this comment.
Reviewed exact head 265e479426275cff1b093e0f3dbe5c0e5dca4416 against parent e542dca0bdcc262fa035dcf7fc33910cdcc8cdaf. No blocking issue found in the repeated-description change.
Independent Linux verification with Ruff 0.17.0 and SQLFluff 4.3.0:
- Real repeated diagnostics are described once in the visible rule summary, with every displayed location retaining its code. Different F401 import names and RF01 references remain attached to their respective locations. Native/direct/lint exit codes match for the exercised diagnostic runs; clean and fatal controls are unchanged between parent and head.
- Sixteen additional JSON-stream cases cover identical and mixed messages, an empty description, whitespace normalization, Unicode, the existing long-description truncation, more than ten rules, and the fifty-detail limit. Descriptions outside the visible summary remain beside their locations. Eight separate launches produce identical tied-rule output.
- Real sixty-file runs through each direct runner retain the fifty-detail cap and the omitted-count marker. Following the recovery hints retrieves all sixty locations and messages: full per-location text for Ruff, and all sixty original JSON violation records for SQLFluff. Explicit-format and help controls also match the parent.
cargo test --locked --lib 'cmds::python::': 96 passed.cargo fmt --all --checkandcargo clippy --workspace --all-targets --locked -- -D warningspassed.
This verifies the filter change on Linux. I did not rerun the full workspace suite, other platforms, or integration with the separate pending Ruff routing/output-format patches.
Ruff now orders tied files and per-file rules by name, and both filters truncate a long shared description in Top rules to 100 characters. Neither was covered: a count-only sort or an untruncated row passed the suite. Add real Ruff 0.16.6 and SQLFluff 4.3.0 captures that reach both. Drop the README claim that Ruff's recovery log keeps full messages: its lines are truncated like the detail lines, and recovery depends on the configured mode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
KuSh
left a comment
There was a problem hiding this comment.
Round 1 — APPROVED
Claim: when every occurrence of a rule has the same description, Ruff and SQLFluff print it once on that rule's visible Top rules row and keep only location and code on the detail lines; Ruff's tied orderings become deterministic.
Scope: accept.
Ran: real Ruff 0.16.6 and SQLFluff 4.3.0 on real Python scripts and DuckDB SQL files, develop e542dca0b vs this head vs this head merged onto develop 5687d0d84: 2-file, 4/10-file and 60/80-file corpora (14 to 4,382 diagnostics), clean runs, fatal runs (--dialect NOPE, a missing path), explicit --format yaml / --output-format=concise, rtk lint, rtk pipe -f ruff-check|sqlfluff-lint. Invariants: under the hook (rtk rewrite → rtk ruff|sqlfluff), over-cap with accounting (shown + +N more = total on all 13 rows; the rtk recall hints return all 32/32 hidden Ruff locations with their messages and all 60/60 SQLFluff records), every decision, exit codes equal on every row. Savings against native text 46.8% on the 2-file SQLFluff run (develop 35.5%) up to 99.8%, vs the 20% floor.
Pins: 15 decisions, 15 mutated, 8 survived the suite: 4 reachable ones (name order of tied Ruff files and of per-file rules; the 100-character truncation of a shared description, both filters) are now killed by the fixup below; 4 are unreachable (trimming before comparing, and the empty-description guard, in both filters: none of the ~11,000 real diagnostics has surrounding whitespace or an empty message).
Matrix: executed this round; pin check: executed this round.
CI @265e47942: all green (15 jobs, Linux/macOS/Windows). @d12a28410: all green (15 jobs).
Blocks merge (0)
Filed as follow-ups
- #4700 Lint filters shorten paths to a basename, so same-named files in different directories become indistinguishable — pre-existing, outside this PR's hunks (
compact_path).
Checked and correct — no need to re-verify
- A description moves to
Top rulesonly when every occurrence of that code carries it (F401 import names, RF01/RF03 references stay per location), and only for rows actually printed; a rule past the 10-row cap keeps it on each location. - No location loses its message: on every row, each printed location has its message on its own line or, identical, on its rule's row (checked against the raw JSON).
- Ruff output is byte-identical across 8 runs on a corpus full of ties; develop gave 8 different outputs.
- Fixture
ruff_check_repeated_raw.jsonand the savings test are real Ruff output and pass on the merge with develop.
Next
Fixup d12a28410 pushed, code-reviewed: three tests on real Ruff and SQLFluff captures that pin the tie order and the truncation of a shared description, and the README sentence about Ruff's recovery log dropped (that log truncates messages at 100 characters like the detail lines). Approving.
Threads: 0 open.
Summary
sqlfluff linton small corpora #3920 consistently in Ruff and SQLFluff: place a rule's description in its visibleTop rulesrow when all occurrences have that same description, leaving the rule code with every detail location.first at/ worst-first details, and Ruff's per-location recovery representation.Fixes #3920.
This chooses the shared-summary option discussed in the issue and the merged #253 review, rather than diverging the two filters.
Test plan
cargo fmt --all --checkand standalone proxy formatting.cargo clippy --workspace --all-targets --locked -- -D warnings.cargo test --locked --lib 'cmds::python::': 96 passed.cargo test --workspace --locked: 4,967 passed, 8 ignored.cargo test --no-default-features --locked: 4,507 passed, 8 ignored.cargo test --workspace --locked -- --ignored: the 8 ignored tests passed separately.git diff --check; two self-review passes. The two identical-description regressions failed on original source before implementation.rtk lint, andrtk pipeon identical inputs. Checked codes/messages/coordinates and direct/lint exit-code parity (1for diagnostics, not a fatal failure).Real before/after results
Same four-file fixtures, tool versions and environment; original
developise542dca0bdcc262fa035dcf7fc33910cdcc8cdaf.These are this fixture's command-output bytes, not tokenizer counts or model bills. Against the injected JSON, the corresponding reductions are 88.6% and 91.0%.
Release-process comparison on macOS arm64: five warmups, then 40 interleaved launches per variant/filter, identical real JSON on stdin. Median
rtk pipetimes were Ruff 4.88 → 4.88 ms, SQLFluff 4.50 → 4.69 ms. This includes process launch, CLI, filtering and output capture, not isolated filter timings or a cross-platform hook-startup guarantee. Both executable hashes and original/patched output shapes were verified.Raw native captures, fixture inputs, recovery checks, performance samples and replay instructions
Before/after screenshots and recording
The images and 12.88-second recording show a labeled viewer of the actual captured CLI output, not product UI or mocked linter output. Same 1440×1200 viewport, no clipped output; the recording was decoded and sought at four positions. Evidence is on an independent branch and is not included in this code diff.
SQLFluff — before and after
Before:
After:
Ruff — before and after
Before:
After:
Watch the real-output before/after recording
Scope and verification limits
pipefilter already lacks a standalone recovery hint. This patch does not change that; full raw recovery was verified through the direct runner.