fix(rules): apply max_findings after suppression and report dropped findings (#624)

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **`rules_engine.max_findings` is applied after suppression and truncation is reported (#624).** The cap (default 5) used to be applied inside each scanner, before `ignore.rules` / `cora-ignore`, so suppressed findings consumed slots and pushed real findings out, and the rest were dropped silently. Scanners now run uncapped and the cap is enforced once in post-processing on deterministic findings only (LLM issues are never capped), highest severity first. `cora scan` and `cora review` print `⚠ N more deterministic findings not shown (...)` on stderr when findings are cut, and the `cora scan` summary mentions it; stdout formats are unchanged. `max_findings: 0` or `null` now means unlimited (previously `0` hid every finding).
+
 ## [0.17.2] - 2026-10-09
 
 ### Fixed

docs/configuration.md

@@ -453,9 +453,18 @@ rules_engine:
 | Field | Type | Default | Description |
 |-------|------|---------|-------------|
 | `enabled` | `bool` | `true` | Enable/disable the rule engine |
-| `max_findings` | `int` | `5` | Max findings per scan |
+| `max_findings` | `int` | `5` | Max deterministic findings shown per run (`0` or `null` = unlimited). See below |
 | `index_skip_files` | `[string]` | *(see below)* | Glob patterns for files to skip during index scanning |
 
+#### What `max_findings` caps
+
+`max_findings` caps the **deterministic** findings only (rule engine, secrets, security, and index scanners; the findings that carry a `rule_id`). LLM issues are never capped.
+
+- The cap is applied **after** suppression (`ignore.rules` and inline `cora-ignore:` markers), so suppressed findings never use up slots and hide real ones.
+- Findings are kept highest severity first (Critical, Major, Minor, Info); ties keep their original order. Because the most severe findings survive, the cap never changes the blocking decision for a cap of 1 or more.
+- When findings are cut, `cora scan` and `cora review` print one line on **stderr** (`⚠ N more deterministic findings not shown (rules.max_findings = 5; raise it or set 0/null to show all)`), and the `cora scan` summary mentions it. JSON, SARIF and compact output on stdout are unchanged.
+- `0` or `null` means unlimited. For large diffs that `cora review` splits into chunks, the cap applies per chunk.
+
 Default `index_skip_files` patterns (bundled with cora):
 
 - `*.config.ts`, `*.config.js`, `*.config.mjs` — bundler/build config files

src/commands/review.rs

@@ -254,6 +254,8 @@ pub async fn execute_review(
         }
     };
 
+    warn_dropped(response.dropped_findings, config, opts.quiet);
+
     // 6. Filter by severity if specified
     let min_severity = if let Some(ref sev) = opts.severity {
         Severity::from_str_lossy(sev)
@@ -505,6 +507,7 @@ async fn execute_chunked_review(
     let mut any_error = false;
     let mut any_success = false;
     let mut should_block = false;
+    let mut dropped_findings = 0usize;
 
     for chunk in &chunks {
         if !opts.quiet {
@@ -565,6 +568,7 @@ async fn execute_chunked_review(
                     ));
                 }
 
+                dropped_findings += resp.dropped_findings;
                 all_issues.extend(resp.issues);
                 if resp.should_block {
                     should_block = true;
@@ -629,11 +633,14 @@ async fn execute_chunked_review(
     // Build merged response
     let merged_summary = merged_chunk_summary(&summaries, all_issues.len(), any_error);
 
+    warn_dropped(dropped_findings, config, opts.quiet);
+
     let merged_response = ReviewResponse {
         issues: all_issues,
         summary: merged_summary,
         tokens_used: Some(total_tokens),
         should_block,
+        dropped_findings,
     };
 
     // 6. Filter by severity if specified
@@ -784,6 +791,16 @@ fn compute_exit_code(
     }
 }
 
+/// Print the one-line "findings not shown" warning to stderr (#624).
+fn warn_dropped(dropped: usize, config: &Config, quiet: bool) {
+    if quiet {
+        return;
+    }
+    if let Some(w) = crate::engine::postprocess::dropped_warning(dropped, config) {
+        eprintln!("{}", w.yellow());
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -803,6 +820,7 @@ mod tests {
             summary: String::new(),
             tokens_used: None,
             should_block,
+            dropped_findings: 0,
         }
     }
 

src/commands/scan.rs

@@ -6,7 +6,7 @@ use tracing::debug;
 
 use crate::config::schema::Config;
 use crate::engine::ReviewIssue;
-use crate::engine::postprocess::{Source, postprocess};
+use crate::engine::postprocess::{Source, dropped_warning, postprocess_report};
 use crate::engine::review_store;
 use crate::engine::scanner::{FileEntry, batch_files, format_batch_for_prompt, walk_project};
 use crate::engine::types::TokenUsage;
@@ -127,7 +127,8 @@ pub async fn execute_scan(
     let index_findings = crate::engine::index_scanner::scan_project_index(
         &index_bridge,
         &files,
-        config.rules_config.max_findings,
+        // Uncapped: the cap is enforced after suppression in `postprocess` (#624).
+        usize::MAX,
         &index_skip,
     );
     if !index_findings.is_empty() {
@@ -253,7 +254,10 @@ pub async fn execute_scan(
     //    apply ignore.rules and inline cora-ignore markers. Deterministic
     //    findings survive an LLM failure (#595).
     all_issues.extend(index_findings);
-    let all_issues = finalize_issues(config, &files, all_issues);
+    let (all_issues, dropped) = finalize_issues(config, &files, all_issues);
+    if let Some(w) = dropped_warning(dropped, config) {
+        eprintln!("{}", w.yellow());
+    }
     let issue_count = all_issues.len();
     let min_severity = config.hook.min_severity_level();
     // Ord order is Critical(0) < Major(1) < Minor(2) < Info(3), so "at or above
@@ -265,10 +269,17 @@ pub async fn execute_scan(
         files_scanned: files.len(),
         lines_scanned: total_lines,
         summary: format!(
-            "Scanned {} files ({} lines), found {} issues.",
+            "Scanned {} files ({} lines), found {} issues{}.",
             files.len(),
             total_lines,
-            issue_count
+            issue_count,
+            if dropped > 0 {
+                format!(
+                    " ({dropped} more deterministic findings not shown, rules.max_findings cap)"
+                )
+            } else {
+                String::new()
+            }
         ),
         tokens_used: total_tokens,
         should_block,
@@ -342,8 +353,9 @@ fn finalize_issues(
     config: &Config,
     files: &[FileEntry],
     issues: Vec<ReviewIssue>,
-) -> Vec<ReviewIssue> {
-    postprocess(issues, &Source::Files(files), config)
+) -> (Vec<ReviewIssue>, usize) {
+    let report = postprocess_report(issues, &Source::Files(files), config);
+    (report.issues, report.dropped)
 }
 
 /// Compute a short SHA256 hash of a file's content for incremental scanning.
@@ -428,7 +440,7 @@ mod tests {
     }
 
     fn secret_title() -> String {
-        let out = finalize_issues(&Config::default(), &[secret_file("")], Vec::new());
+        let out = finalize_issues(&Config::default(), &[secret_file("")], Vec::new()).0;
         out.iter()
             .find(|i| i.line == Some(2))
             .unwrap_or_else(|| panic!("no finding on line 2: {:?}", titles(&out)))
@@ -438,7 +450,7 @@ mod tests {
 
     #[test]
     fn finds_hardcoded_secret_without_llm() {
-        let out = finalize_issues(&Config::default(), &[secret_file("")], Vec::new());
+        let out = finalize_issues(&Config::default(), &[secret_file("")], Vec::new()).0;
         assert!(
             out.iter()
                 .any(|i| i.file == "src/app.py" && i.line == Some(2)),
@@ -451,7 +463,7 @@ mod tests {
     fn inline_marker_suppresses_secret() {
         let title = secret_title();
         let file = secret_file(&format!("  # cora-ignore: {title}"));
-        let out = finalize_issues(&Config::default(), &[file], Vec::new());
+        let out = finalize_issues(&Config::default(), &[file], Vec::new()).0;
         assert!(
             !out.iter().any(|i| i.line == Some(2)),
             "got {:?}",
@@ -463,7 +475,7 @@ mod tests {
     fn ignore_rules_suppress_secret() {
         let mut config = Config::default();
         config.ignore.rules = vec![secret_title()];
-        let out = finalize_issues(&config, &[secret_file("")], Vec::new());
+        let out = finalize_issues(&config, &[secret_file("")], Vec::new()).0;
         assert!(
             !out.iter().any(|i| i.line == Some(2)),
             "got {:?}",

src/config/schema.rs

@@ -613,13 +613,25 @@ fn default_max_findings() -> usize {
     5
 }
 
+/// `rules_engine.max_findings`: an explicit `null` means unlimited (stored as 0).
+fn deserialize_max_findings<'de, D>(d: D) -> std::result::Result<usize, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    Ok(Option::<usize>::deserialize(d)?.unwrap_or(0))
+}
+
 /// Rule engine configuration section for `.cora.yaml`.
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
 #[serde(deny_unknown_fields)]
 pub struct RulesSection {
     #[serde(default, skip_serializing_if = "is_default")]
     pub enabled: bool,
-    #[serde(default = "default_max_findings")]
+    /// Cap on deterministic findings shown per run. `0` or `null` = unlimited (#624).
+    #[serde(
+        default = "default_max_findings",
+        deserialize_with = "deserialize_max_findings"
+    )]
     pub max_findings: usize,
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub custom: Vec<crate::engine::rules::types::CustomRule>,

src/engine/cache.rs

@@ -195,6 +195,7 @@ mod tests {
             summary: "Found 1 issue.".to_string(),
             tokens_used: None,
             should_block: false,
+            dropped_findings: 0,
         }
     }
 

src/engine/deterministic.rs

@@ -23,6 +23,9 @@ use crate::engine::index_bridge::IndexBridge;
 use crate::engine::rules::{self, types::RuleFinding};
 use crate::engine::{index_scanner, secrets_scanner, security_scanner};
 
+/// Max findings per family listed in the LLM prompt context.
+const PROMPT_FINDINGS_PER_FAMILY: usize = 20;
+
 /// Findings of every deterministic scanner family, in contract order.
 #[derive(Debug, Default)]
 pub struct DeterministicReport {
@@ -45,11 +48,18 @@ pub fn skip_patterns(config: &Config) -> Vec<String> {
 
 /// Run every deterministic check on a parsed diff.
 pub fn run(chunks: &[FileChunk], config: &Config, bridge: &IndexBridge) -> DeterministicReport {
-    let max = config.rules_config.max_findings;
+    // Scanners run uncapped: `rules_engine.max_findings` is enforced by
+    // `postprocess` AFTER suppression, so suppressed findings never consume
+    // slots and truncation can be reported (#624).
+    let max = usize::MAX;
     let skip = skip_patterns(config);
+    let uncapped_rules = crate::engine::rules::types::RulesConfig {
+        max_findings: 0,
+        ..config.rules_config.clone()
+    };
 
     DeterministicReport {
-        rules: rules::run_rules(chunks, &config.rules_config),
+        rules: rules::run_rules(chunks, &uncapped_rules),
         secrets: secrets_scanner::scan_secrets(chunks, max),
         security: security_scanner::scan_security(chunks, max),
         index_unused: index_scanner::scan_unused_imports(bridge, chunks, max, &skip),
@@ -95,8 +105,18 @@ impl DeterministicReport {
             parts.push(warning);
         }
         for family in self.families() {
-            let text = rules::format_rule_context(family);
+            // Scanners run uncapped (the user-facing cap is applied after
+            // suppression in `postprocess`), so bound what is sent to the model:
+            // families are severity-sorted, keep the worst PROMPT_FINDINGS_PER_FAMILY.
+            let shown = &family[..family.len().min(PROMPT_FINDINGS_PER_FAMILY)];
+            let mut text = rules::format_rule_context(shown);
             if !text.is_empty() {
+                if family.len() > shown.len() {
+                    text.push_str(&format!(
+                        "({} more findings of this kind not listed here)\n",
+                        family.len() - shown.len()
+                    ));
+                }
                 parts.push(text);
             }
         }
@@ -207,6 +227,26 @@ diff --git a/src/api.js b/src/api.js
         );
     }
 
+    #[test]
+    fn prompt_context_is_bounded_per_family() {
+        let report = DeterministicReport {
+            secrets: (1..=(PROMPT_FINDINGS_PER_FAMILY as u32 + 7))
+                .map(|line| RuleFinding {
+                    rule_id: "secrets/x".to_string(),
+                    file: "a.rs".to_string(),
+                    line,
+                    severity: crate::engine::Severity::Critical,
+                    title: "t".to_string(),
+                    body: "b".to_string(),
+                })
+                .collect(),
+            ..Default::default()
+        };
+        let ctx = report.context(None).expect("context");
+        assert_eq!(ctx.matches("secrets/x").count(), PROMPT_FINDINGS_PER_FAMILY);
+        assert!(ctx.contains("(7 more findings of this kind not listed here)"));
+    }
+
     #[test]
     fn context_is_ordered_and_optional() {
         let chunks = parse_diff(DIFF);

src/engine/llm/mod.rs

@@ -145,6 +145,7 @@ fn into_review_response(findings: Findings, usage: Option<&transport::Usage>) ->
         summary: findings.summary.unwrap_or_default(),
         tokens_used: usage.map(usage_to_token_usage),
         should_block: false,
+        dropped_findings: 0,
     }
 }