Inspect: skip oversized files before reading; never parse plain JSON for TODOs (#1337)

.alfonso/release-notes/next/todo-scan-bounds.md

@@ -0,0 +1 @@
+Background TODO scans no longer read plain JSON, and TODO, metrics, complexity and duplicate scans bound source reads and parsing while reporting oversized files as coverage gaps.

benchmarks/aft-search/slice-descriptors/land-todo-scan-bounds.json

@@ -0,0 +1,8 @@
+{
+  "fixtures": [
+    "harness-goldens"
+  ],
+  "kind": "harness",
+  "slice_class": "non_ranking",
+  "targeted_mechanism": "none"
+}

crates/aft/src/inspect/cache.rs

@@ -200,7 +200,9 @@ impl From<serde_json::Error> for InspectCacheError {
 /// v42: corrected source-buffer freshness, impl scopes, complexity and cycles;
 /// an atomic epoch marker fences raw facts as well as derived findings, and
 /// undecodable dead-code rows remain explicit coverage gaps.
-pub(crate) const TIER2_CONTRIBUTION_CACHE_VERSION: u32 = 42;
+/// v43: source-size admission excludes oversized complexity/duplicate inputs
+/// and reports their coverage gaps instead of reusing unbounded scan results.
+pub(crate) const TIER2_CONTRIBUTION_CACHE_VERSION: u32 = 43;
 // Reserved metadata category: its integer is an epoch, never a scan timestamp.
 const CACHE_VERSION_CATEGORY: &str = "__cache_version__";
 
@@ -3996,7 +3998,7 @@ pub(crate) mod tests {
             decoded.contribution["exports"][0]["is_type_like"].as_bool(),
             Some(true)
         );
-        assert_eq!(TIER2_CONTRIBUTION_CACHE_VERSION, 42);
+        assert_eq!(TIER2_CONTRIBUTION_CACHE_VERSION, 43);
     }
 
     #[test]

crates/aft/src/inspect/scanners/complexity.rs

@@ -8,7 +8,9 @@ use serde_json::{json, Value};
 use tree_sitter::Node;
 
 use super::source_text::{read_source_text, write_skipped_files, SourceText, NOT_VALID_UTF8};
-use crate::cache_freshness::{self, FileFreshness};
+#[cfg(test)]
+use crate::cache_freshness;
+use crate::cache_freshness::FileFreshness;
 use crate::inspect::{
     FileContribution, InspectCategory, InspectJob, InspectResult, InspectScanSuccess,
 };
@@ -347,7 +349,11 @@ pub fn run_complexity_scan(job: &InspectJob) -> InspectResult {
     };
 
     let aggregate = aggregate_file_scans(&job.project_root, &scans, Some(DRILL_DOWN_LIMIT));
-    let scanned_files = scans.iter().map(|scan| scan.path.clone()).collect();
+    let scanned_files = scans
+        .iter()
+        .filter(|scan| scan.contribution.skipped_reason.is_none())
+        .map(|scan| scan.path.clone())
+        .collect();
     let contributions = scans
         .iter()
         .map(file_scan_to_contribution)
@@ -381,7 +387,7 @@ pub(crate) fn aggregate_complexity_contributions_with_limit(
 /// and this scan and is treated as deleted. A file that is not valid UTF-8 is
 /// a per-file skip; other read failures keep failing the scan as before.
 fn scan_file(project_root: &Path, path: &Path) -> Result<Option<FileComplexityScan>, String> {
-    let freshness = match cache_freshness::collect(path) {
+    let (freshness, _permit) = match super::source_text::source_freshness(path) {
         Ok(freshness) => freshness,
         Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
         Err(error) => return Err(format!("freshness failed for {}: {error}", path.display())),
@@ -391,10 +397,15 @@ fn scan_file(project_root: &Path, path: &Path) -> Result<Option<FileComplexitySc
     // whatever their encoding, are never opened as source.
     let analyzable =
         detect_language(path).is_some_and(|language| language_spec(language).is_some());
-    let contribution = if analyzable {
+    let contribution = if freshness.size > super::source_text::MAX_SOURCE_BYTES {
+        unreadable_contribution(project_root, path, super::source_text::TOO_LARGE)
+    } else if analyzable {
         match read_source_text(path) {
             Ok(SourceText::Text(source)) => scan_source(project_root, path, &source),
             Ok(SourceText::NotUtf8) => unreadable_contribution(project_root, path, NOT_VALID_UTF8),
+            Ok(SourceText::TooLarge) => {
+                unreadable_contribution(project_root, path, super::source_text::TOO_LARGE)
+            }
             Ok(SourceText::Vanished) => return Ok(None),
             Err(error) => return Err(format!("read failed for {}: {error}", path.display())),
         }
@@ -668,7 +679,7 @@ fn aggregate_contributions(
         "worst": worst,
         "items": items,
         "drill_down_capped": count > limit,
-        "scanned_files": contributions.len(),
+        "scanned_files": contributions.iter().filter(|contribution| contribution.skipped_reason.is_none()).count(),
         "languages_skipped": languages_skipped.into_iter().collect::<Vec<_>>(),
         "complete": parse_errors.is_empty(),
     });

crates/aft/src/inspect/scanners/duplicates.rs

@@ -214,6 +214,7 @@ pub fn run_duplicates_scan(job: &InspectJob) -> InspectResult {
         .collect::<Vec<_>>();
     let scanned_files = file_scans
         .iter()
+        .filter(|scan| scan.skipped_reason.is_none())
         .map(|scan| scan.path.clone())
         .collect::<Vec<_>>();
 
@@ -230,11 +231,24 @@ pub fn run_duplicates_scan(job: &InspectJob) -> InspectResult {
 /// a per-file skip; other read failures keep failing the scan as before.
 fn scan_file(job: &InspectJob, path: &Path) -> Result<Option<FileScan>, String> {
     let display_path = display_path(&job.project_root, path);
-    let freshness = match cache_freshness::collect(path) {
+    let (freshness, _permit) = match super::source_text::source_freshness(path) {
         Ok(freshness) => freshness,
         Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
         Err(error) => return Err(format!("freshness failed for {}: {error}", path.display())),
     };
+    if freshness.size > super::source_text::MAX_SOURCE_BYTES {
+        return Ok(Some(FileScan {
+            path: path.to_path_buf(),
+            display_path,
+            language_skipped: None,
+            skipped_reason: Some(super::source_text::TOO_LARGE),
+            freshness,
+            line_count: 0,
+            expected_duplicate: false,
+            generated: false,
+            fragments: Vec::new(),
+        }));
+    }
     let Some(lang) = detect_language(path) else {
         return Ok(Some(FileScan {
             path: path.to_path_buf(),
@@ -265,16 +279,24 @@ fn scan_file(job: &InspectJob, path: &Path) -> Result<Option<FileScan>, String>
 
     let source = match read_source_text(path) {
         Ok(SourceText::Text(source)) => source,
-        Ok(SourceText::NotUtf8) => {
+        Ok(skipped @ (SourceText::NotUtf8 | SourceText::TooLarge)) => {
             return Ok(Some(FileScan {
                 path: path.to_path_buf(),
                 display_path,
                 language_skipped: None,
-                skipped_reason: Some(NOT_VALID_UTF8),
+                skipped_reason: Some(if matches!(skipped, SourceText::TooLarge) {
+                    super::source_text::TOO_LARGE
+                } else {
+                    NOT_VALID_UTF8
+                }),
                 freshness,
                 line_count: 0,
                 expected_duplicate: false,
-                generated: crate::inspect::generated::is_generated_file(&job.project_root, path),
+                generated: if matches!(skipped, SourceText::TooLarge) {
+                    false
+                } else {
+                    crate::inspect::generated::is_generated_file(&job.project_root, path)
+                },
                 fragments: Vec::new(),
             }));
         }
@@ -631,7 +653,10 @@ pub(crate) fn aggregate_duplicate_contributions_with_limit(
         .collect();
     let mut aggregate = aggregate_duplicate_occurrences(
         by_hash,
-        parsed.len(),
+        parsed
+            .iter()
+            .filter(|scan| scan.skipped_reason.is_none())
+            .count(),
         line_counts.values().copied().map(u64::from).sum(),
         marker_files,
         languages_skipped,
@@ -796,7 +821,10 @@ fn aggregate_file_scans_with_limit(
 
     aggregate_duplicate_occurrences(
         by_hash,
-        file_scans.len(),
+        file_scans
+            .iter()
+            .filter(|scan| scan.skipped_reason.is_none())
+            .count(),
         file_scans
             .iter()
             .map(|scan| u64::from(scan.line_count))

crates/aft/src/inspect/scanners/metrics.rs

@@ -31,6 +31,7 @@ struct CachedFileMetric {
     path: PathBuf,
     language: &'static str,
     loc: usize,
+    skipped_reason: Option<&'static str>,
 }
 
 #[derive(Debug, Clone)]
@@ -90,18 +91,10 @@ fn run_metrics_scan_with_memo(
     let started = Instant::now();
     memo.reserve_for_scan(job.scope_files.len());
     let progress = crate::run_tool_call::progress::FileProgress::current();
-    let per_file = job
+    let cached_files = job
         .scope_files
         .par_iter()
-        .map(|path| {
-            let cached = memo.get_or_insert_with(path, scan_file);
-            FileMetric {
-                path: cached.path,
-                language: cached.language,
-                symbol_count: cached_symbol_count(path, job),
-                loc: cached.loc,
-            }
-        })
+        .map(|path| memo.get_or_insert_with(path, scan_file))
         .inspect(|_| {
             if let Some(progress) = &progress {
                 progress.finished();
@@ -111,9 +104,29 @@ fn run_metrics_scan_with_memo(
     if job.is_full_project_scope() {
         memo.prune_to_scope(&job.project_root, &job.scope_files);
     }
-    let aggregate = aggregate_metrics(&job.project_root, &per_file);
+    let mut skipped_files = Vec::new();
+    let per_file = cached_files
+        .into_iter()
+        .filter_map(|cached| {
+            if let Some(reason) = cached.skipped_reason {
+                skipped_files.push((
+                    display_path(&job.project_root, &cached.path),
+                    reason.to_string(),
+                ));
+                return None;
+            }
+            Some(FileMetric {
+                symbol_count: cached_symbol_count(&cached.path, job),
+                path: cached.path,
+                language: cached.language,
+                loc: cached.loc,
+            })
+        })
+        .collect::<Vec<_>>();
+    let mut aggregate = aggregate_metrics(&job.project_root, &per_file);
+    super::source_text::write_skipped_files(&mut aggregate, skipped_files, None);
     let success = crate::inspect::InspectScanSuccess {
-        scanned_files: job.scope_files.clone(),
+        scanned_files: per_file.iter().map(|file| file.path.clone()).collect(),
         contributions: Vec::new(),
         aggregate,
     };
@@ -127,16 +140,20 @@ fn metrics_memo() -> &'static Tier1FileMemo<CachedFileMetric> {
 
 fn scan_file(path: &Path) -> (Option<FileFreshness>, CachedFileMetric) {
     let metadata = fs::metadata(path).ok();
-    let bytes = read_file_bytes(path);
+    let _permit =
+        super::source_text::source_scan_permit(metadata.as_ref().map_or(0, fs::Metadata::len));
+    let (bytes, skipped_reason) = read_file_bytes(path);
     let freshness = metadata
         .as_ref()
+        .filter(|_| skipped_reason.is_none())
         .map(|metadata| freshness_from_metadata(metadata, bytes.as_deref()));
     let loc = bytes.as_deref().map(line_count_bytes).unwrap_or_default();
 
     let metric = CachedFileMetric {
         path: path.to_path_buf(),
         language: language_key(path),
         loc,
+        skipped_reason,
     };
     (freshness, metric)
 }
@@ -158,10 +175,17 @@ fn freshness_from_metadata(metadata: &fs::Metadata, bytes: Option<&[u8]>) -> Fil
     }
 }
 
-fn read_file_bytes(path: &Path) -> Option<Vec<u8>> {
-    #[cfg(debug_assertions)]
-    bump_file_read_count(path);
-    crate::project_content::read(path).ok()
+fn read_file_bytes(path: &Path) -> (Option<Vec<u8>>, Option<&'static str>) {
+    use super::source_text::{read_source_bytes, SourceBytes, TOO_LARGE};
+    match read_source_bytes(path) {
+        Ok(SourceBytes::Bytes(bytes)) => {
+            #[cfg(debug_assertions)]
+            bump_file_read_count(path);
+            (Some(bytes), None)
+        }
+        Ok(SourceBytes::TooLarge) => (None, Some(TOO_LARGE)),
+        _ => (None, None),
+    }
 }
 
 fn cached_symbol_count(path: &Path, job: &crate::inspect::InspectJob) -> usize {