perf(python): compact repeated Ruff and SQLFluff descriptions

src/cmds/python/README.md

@@ -7,6 +7,10 @@
 - `pytest_cmd.rs` uses a state machine text parser (no JSON available from pytest)
 - `ruff_cmd.rs` uses JSON for check mode (`--output-format=json`) and text filtering for format mode
 - `sqlfluff_cmd.rs` injects `--format json` for lint mode and groups violations by rule and file; other subcommands pass through capped
+- Ruff and SQLFluff list identical descriptions once beside a visible top rule.
+  Locations retain the rule code; differing messages and rules outside the
+  summary cap keep their descriptions beside each location. Ruff recovery logs
+  retain the full per-location messages.
 - `pip_cmd.rs` auto-detects `uv` as a pip alternative and routes accordingly
 - `uv_cmd.rs` preserves `uv run` environment semantics: the program's own output passes through on success (bounded, with a tee hint), diagnostics are extracted on failure
 - `python -m pytest` and `python3 -m mypy` are rewritten by the hook registry to `rtk pytest` / `rtk mypy`

src/cmds/python/ruff_cmd.rs

@@ -7,7 +7,7 @@ use crate::core::truncate::CAP_WARNINGS;
 use crate::core::utils::{resolved_command, truncate};
 use anyhow::Result;
 use serde::Deserialize;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 /// A tool with a list like this also belongs in `core::tracking::SUBCOMMAND_ROUTERS`,
 /// which keeps the subcommand in its telemetry label.
@@ -157,9 +157,16 @@ pub fn filter_ruff_check_json(output: &str) -> String {
     let total_files = unique_files.len();
 
     // Group by rule code
-    let mut by_rule: HashMap<String, usize> = HashMap::new();
+    let mut by_rule: HashMap<&str, (usize, Option<&str>)> = HashMap::new();
     for diag in &diagnostics {
-        *by_rule.entry(diag.code.clone()).or_insert(0) += 1;
+        let message = diag.message.trim();
+        let entry = by_rule.entry(&diag.code).or_insert((0, Some(message)));
+        entry.0 += 1;
+        // F401, for example, includes the imported name. Never replace
+        // different messages with the first message seen for that code.
+        if entry.1 != Some(message) {
+            entry.1 = None;
+        }
     }
 
     // Group by file
@@ -169,7 +176,7 @@ pub fn filter_ruff_check_json(output: &str) -> String {
     }
 
     let mut file_counts: Vec<_> = by_file.iter().collect();
-    file_counts.sort_by(|a, b| b.1.cmp(a.1));
+    file_counts.sort_by_key(|(file, count)| (std::cmp::Reverse(**count), **file));
 
     // Build output
     let mut result = String::new();
@@ -184,15 +191,24 @@ pub fn filter_ruff_check_json(output: &str) -> String {
     result.push('\n');
 
     // Show top rules
-    let mut rule_counts: Vec<_> = by_rule.iter().collect();
-    rule_counts.sort_by(|a, b| b.1.cmp(a.1));
+    let mut rule_counts: Vec<_> = by_rule
+        .iter()
+        .map(|(&code, &(count, message))| (code, count, message))
+        .collect();
+    rule_counts.sort_by_key(|&(code, count, _)| (std::cmp::Reverse(count), code));
 
     const MAX_RUFF_RULES: usize = CAP_WARNINGS;
     const MAX_RUFF_FILES: usize = CAP_WARNINGS;
+    let mut described_rules = HashSet::new();
     if !rule_counts.is_empty() {
         result.push_str("Top rules:\n");
-        for (rule, count) in rule_counts.iter().take(MAX_RUFF_RULES) {
-            result.push_str(&format!("  {} ({}x)\n", rule, count));
+        for (rule, count, message) in rule_counts.iter().take(MAX_RUFF_RULES) {
+            result.push_str(&format!("  {} ({}x)", rule, count));
+            if let Some(message) = message.filter(|s| !s.is_empty()) {
+                result.push_str(&format!(": {}", truncate(message, 100)));
+                described_rules.insert(*rule);
+            }
+            result.push('\n');
         }
         result.push('\n');
     }
@@ -210,7 +226,7 @@ pub fn filter_ruff_check_json(output: &str) -> String {
         }
 
         let mut file_rule_counts: Vec<_> = file_rules.iter().collect();
-        file_rule_counts.sort_by(|a, b| b.1.cmp(a.1));
+        file_rule_counts.sort_by_key(|(rule, count)| (std::cmp::Reverse(**count), *rule));
 
         for (rule, count) in file_rule_counts.iter().take(3) {
             result.push_str(&format!("    {} ({})\n", rule, count));
@@ -240,8 +256,22 @@ pub fn filter_ruff_check_json(output: &str) -> String {
         .collect();
 
     result.push_str("\nViolations:\n");
-    for line in violation_lines.iter().take(MAX_VIOLATIONS) {
-        result.push_str(line);
+    for (diag, line) in diagnostics
+        .iter()
+        .zip(&violation_lines)
+        .take(MAX_VIOLATIONS)
+    {
+        if described_rules.contains(diag.code.as_str()) {
+            result.push_str(&format!(
+                "  {}:{}:{} {}\n",
+                compact_path(&diag.filename),
+                diag.location.row,
+                diag.location.column,
+                diag.code,
+            ));
+        } else {
+            result.push_str(line);
+        }
     }
     if violation_lines.len() > MAX_VIOLATIONS {
         result.push_str(&format!(
@@ -434,6 +464,81 @@ mod tests {
         assert!(result.contains("No issues found"));
     }
 
+    #[test]
+    fn repeated_descriptions_are_listed_once_in_visible_top_rules() {
+        let input = r#"[
+            {"code":"F401","message":"`os` imported but unused","location":{"row":1,"column":8},"filename":"/project/src/a.py","fix":null},
+            {"code":"F401","message":"`os` imported but unused","location":{"row":2,"column":8},"filename":"/project/src/b.py","fix":null}
+        ]"#;
+        let out = filter_ruff_check_json(input);
+        let (summary, detail) = out.split_once("Violations:\n").expect("detail section");
+        assert!(
+            summary.contains("F401 (2x): `os` imported but unused"),
+            "{out}"
+        );
+        assert_eq!(out.matches("`os` imported but unused").count(), 1, "{out}");
+        assert!(detail.contains("src/a.py:1:8 F401"));
+        assert!(detail.contains("src/b.py:2:8 F401"));
+    }
+
+    #[test]
+    fn distinct_messages_for_one_rule_stay_attached_to_their_locations() {
+        let input = r#"[
+            {"code":"F401","message":"`os` imported but unused","location":{"row":1,"column":8},"filename":"/project/src/a.py","fix":null},
+            {"code":"F401","message":"`sys` imported but unused","location":{"row":2,"column":8},"filename":"/project/src/a.py","fix":null}
+        ]"#;
+        let out = filter_ruff_check_json(input);
+        assert!(out.contains("src/a.py:1:8 F401 `os` imported but unused"));
+        assert!(out.contains("src/a.py:2:8 F401 `sys` imported but unused"));
+    }
+
+    #[test]
+    fn descriptions_for_rules_outside_the_summary_cap_stay_in_details() {
+        let diagnostics: Vec<_> = (0..=CAP_WARNINGS)
+            .map(|i| {
+                serde_json::json!({
+                    "code": format!("RULE{i:02}"), "message": format!("Distinct diagnostic {i}"),
+                    "location": {"row": i + 1, "column": 1}, "filename": "src/a.py", "fix": null,
+                })
+            })
+            .collect();
+        let out =
+            filter_ruff_check_json(&serde_json::to_string(&diagnostics).expect("fixture JSON"));
+        let detail = out.split_once("Violations:\n").expect("detail section").1;
+        assert!(detail.contains("RULE10 Distinct diagnostic 10"), "{out}");
+    }
+
+    #[test]
+    fn real_repeated_diagnostics_keep_locations_and_reduce_output() {
+        // Captured from Ruff 0.16.6:
+        // `ruff check --isolated --select F401,E701 --output-format=json src/four.py`
+        // where four.py imports os and has two one-line if statements. Only
+        // the machine-specific filename prefix was normalized to /fixture.
+        let input = include_str!("../../../tests/fixtures/ruff_check_repeated_raw.json");
+        let out = filter_ruff_check_json(input);
+        assert_eq!(
+            out.matches("Multiple statements on one line (colon)")
+                .count(),
+            1
+        );
+        assert_eq!(out.matches("`os` imported but unused").count(), 1);
+        for location in [
+            "src/four.py:1:8 F401",
+            "src/four.py:3:8 E701",
+            "src/four.py:4:9 E701",
+        ] {
+            assert!(out.contains(location), "{out}");
+        }
+        assert!(out.contains("3 issues in 1 files (1 fixable)"), "{out}");
+        assert!(
+            out.len() * 100 <= input.len() * 80,
+            "must save at least 20%"
+        );
+        for _ in 0..10 {
+            assert_eq!(filter_ruff_check_json(input), out);
+        }
+    }
+
     #[test]
     fn test_filter_ruff_check_with_issues() {
         let output = r#"[

src/cmds/python/sqlfluff_cmd.rs

@@ -11,7 +11,7 @@ use crate::core::truncate::CAP_WARNINGS;
 use crate::core::utils::{resolved_command, truncate};
 use anyhow::Result;
 use serde::Deserialize;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 #[derive(Debug, Deserialize)]
 struct SqlfluffViolation {
@@ -50,6 +50,13 @@ struct SqlfluffFile {
     violations: Vec<SqlfluffViolation>,
 }
 
+struct RuleSummary<'a> {
+    count: usize,
+    path: &'a str,
+    line: Option<u64>,
+    description: Option<&'a str>,
+}
+
 /// How rtk should invoke sqlfluff, and how to read the result back.
 ///
 /// Both entry points - `rtk sqlfluff ...` and `rtk lint sqlfluff ...` - build
@@ -205,27 +212,35 @@ fn render_lint_json(output: &str, exit_code: i32) -> String {
     // Group by rule over the raw sqlfluff JSON order (`parsed`, pre-sort), so
     // "first at" is the earliest occurrence in the original output - not the
     // first after the worst-first file sort above.
-    let mut by_rule: HashMap<&str, (usize, &str, Option<u64>)> = HashMap::new();
+    let mut by_rule: HashMap<&str, RuleSummary<'_>> = HashMap::new();
     for file in &parsed {
         for v in &file.violations {
-            let entry =
-                by_rule
-                    .entry(v.code.as_str())
-                    .or_insert((0, file.filepath.as_str(), v.line()));
-            entry.0 += 1;
-            if entry.2.is_none() && v.line().is_some() {
-                entry.1 = file.filepath.as_str();
-                entry.2 = v.line();
+            let description = v.description.trim();
+            let entry = by_rule.entry(v.code.as_str()).or_insert(RuleSummary {
+                count: 0,
+                path: &file.filepath,
+                line: v.line(),
+                description: Some(description),
+            });
+            entry.count += 1;
+            if entry.line.is_none() && v.line().is_some() {
+                entry.path = &file.filepath;
+                entry.line = v.line();
+            }
+            // One code may name several diagnostics (for example RF01 embeds
+            // a column name). Only identical descriptions can move to the key.
+            if entry.description != Some(description) {
+                entry.description = None;
             }
         }
     }
-    let mut rule_counts: Vec<(&str, usize, &str, Option<u64>)> = by_rule
+    let mut rule_counts: Vec<_> = by_rule
         .iter()
-        .map(|(code, (count, path, line))| (*code, *count, *path, *line))
+        .map(|(&code, summary)| (code, summary))
         .collect();
     // Most frequent rule first; ties break alphabetically by rule code so
     // identical input always yields an identical report.
-    rule_counts.sort_by_key(|r| (std::cmp::Reverse(r.1), r.0));
+    rule_counts.sort_by_key(|(code, summary)| (std::cmp::Reverse(summary.count), *code));
 
     // Build compact output
     let mut result = String::new();
@@ -241,13 +256,19 @@ fn render_lint_json(output: &str, exit_code: i32) -> String {
 
     // Top rules
     result.push_str("Top rules:\n");
-    for (rule, count, path, line) in rule_counts.iter().take(MAX_REPORTED_RULES) {
+    let mut described_rules = HashSet::new();
+    for (rule, summary) in rule_counts.iter().take(MAX_REPORTED_RULES) {
         result.push_str(&format!(
-            "  {} ({}x, first at {})\n",
+            "  {} ({}x, first at {})",
             rule,
-            count,
-            sample_location(path, *line)
+            summary.count,
+            sample_location(summary.path, summary.line)
         ));
+        if let Some(description) = summary.description.filter(|s| !s.is_empty()) {
+            result.push_str(&format!(": {}", truncate(description, 100)));
+            described_rules.insert(*rule);
+        }
+        result.push('\n');
     }
     if rule_counts.len() > MAX_REPORTED_RULES {
         result.push_str(&format!(
@@ -321,11 +342,16 @@ fn render_lint_json(output: &str, exit_code: i32) -> String {
                 break 'detail;
             }
             detail.push_str(&format!(
-                "  {} {} {}\n",
+                "  {} {}",
                 location(&path, v.line(), v.col()),
                 v.code,
-                truncate(v.description.trim(), 100),
             ));
+            // A description outside the visible Top rules cap is not a key
+            // the reader has seen; keep it next to every affected location.
+            if !described_rules.contains(v.code.as_str()) {
+                detail.push_str(&format!(" {}", truncate(v.description.trim(), 100)));
+            }
+            detail.push('\n');
             rendered += 1;
         }
     }
@@ -430,6 +456,49 @@ mod tests {
     /// coordinates, no `fixes` key, 51 files whose worst one is emitted last.
     const V2_JSON: &str = include_str!("../../../tests/fixtures/sqlfluff_lint_v2_raw.json");
 
+    #[test]
+    fn repeated_descriptions_are_listed_once_in_visible_top_rules() {
+        let out = filter_sqlfluff_lint_json(V2_JSON);
+        let message = "Query produces an unknown number of result columns.";
+        let (summary, detail) = out.split_once("Violations:\n").expect("detail section");
+        assert!(summary.contains(&format!("): {message}")), "{out}");
+        assert_eq!(out.matches(message).count(), 1, "{out}");
+        assert!(!detail.contains(message), "{out}");
+        assert!(
+            detail.contains("AM04"),
+            "locations must keep their rule code"
+        );
+    }
+
+    #[test]
+    fn distinct_messages_for_one_rule_stay_attached_to_their_locations() {
+        let input = r#"[{"filepath":"models/a.sql","violations":[
+            {"code":"RF01","description":"Reference 'customer' not found.","line_no":1,"line_pos":3},
+            {"code":"RF01","description":"Reference 'order' not found.","line_no":2,"line_pos":4}
+        ]}]"#;
+        let out = filter_sqlfluff_lint_json(input);
+        assert!(out.contains("models/a.sql:1:3 RF01 Reference 'customer' not found."));
+        assert!(out.contains("models/a.sql:2:4 RF01 Reference 'order' not found."));
+    }
+
+    #[test]
+    fn descriptions_for_rules_outside_the_summary_cap_stay_in_details() {
+        let violations: Vec<_> = (0..=MAX_REPORTED_RULES)
+            .map(|i| {
+                serde_json::json!({
+                    "code": format!("RULE{i:02}"),
+                    "description": format!("Distinct diagnostic {i}"),
+                    "line_no": i + 1,
+                })
+            })
+            .collect();
+        let input = serde_json::json!([{"filepath":"models/a.sql","violations":violations}]);
+        let out = filter_sqlfluff_lint_json(&input.to_string());
+        let detail = out.split_once("Violations:\n").expect("detail section").1;
+        assert!(detail.contains("RULE10 Distinct diagnostic 10"), "{out}");
+        assert!(out.contains("+1 more rules"), "{out}");
+    }
+
     // ── exit-code semantics ─────────────────────────────────────────────────
     //
     // sqlfluff exits 1 whenever it finds violations. That is the normal case,