From b344a87de237dc58667c2d7f6af051155d5dbbe0 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 22:14:55 +0900 Subject: [PATCH 1/3] feat(index): make the 1 MB max file size configurable via CSP_MAX_FILE_BYTES and warn about skipped files (#86) Parity with upstream semble #252 (9bb38fd, closes MinishLab/semble#250): - `indexing::files::get_max_file_bytes()` resolves `CSP_MAX_FILE_BYTES` (bytes); a malformed or non-positive value warns on stderr (once per process) and falls back to `DEFAULT_MAX_FILE_BYTES` (1 MB). - `create_index_from_path` collects the paths skipped for size and prints one stderr warning naming the count and the first five paths, instead of skipping silently. `CreateIndexOptions.max_file_bytes` lets library callers pin the limit; `None` resolves the env var. - The cache fingerprint and the lazy `file_chars` read use the same resolved ceiling, so they keep tracking exactly the files indexed. - `create::MAX_FILE_BYTES` stays as a deprecated alias of the default. - README.md / README.ko.md document the limit and the env var. Closes #86 --- .please/docs/references/semble.md | 11 ++-- README.ko.md | 2 + README.md | 2 + crates/csp/src/indexing/cache_orchestrator.rs | 7 +- crates/csp/src/indexing/create.rs | 46 ++++++++++++- crates/csp/src/indexing/create/tests.rs | 47 +++++++++++++ crates/csp/src/indexing/file_sizes.rs | 13 ++-- crates/csp/src/indexing/files.rs | 66 ++++++++++++++++++- crates/csp/src/indexing/index.rs | 1 + 9 files changed, 179 insertions(+), 16 deletions(-) diff --git a/.please/docs/references/semble.md b/.please/docs/references/semble.md index 3c1fcc1..fd350fb 100644 --- a/.please/docs/references/semble.md +++ b/.please/docs/references/semble.md @@ -165,8 +165,11 @@ Same contract as semble `tokens.py`: - Content-type partition: `DOC_LANGUAGES`, `CONFIG_LANGUAGES`, `DATA_LANGUAGES`; code = all minus those. `get_extensions(types, extra)` inverts the map; the **`extra`** param (custom extensions) is a small Rust-side API addition. -- File gating: `MAX_FILE_BYTES = 1_000_000` (in `create.rs`); empty/whitespace and too-new - (mtime) files are skipped (`FileStatus`). +- File gating: `DEFAULT_MAX_FILE_BYTES = 1_000_000`, overridable per process via + `CSP_MAX_FILE_BYTES` (`get_max_file_bytes()` — upstream `SEMBLE_MAX_FILE_BYTES`, #252; a + malformed/non-positive value warns once and falls back). `create_index_from_path` collects the + paths skipped for size and prints one stderr warning (count + first 5 paths). Empty/whitespace + and too-new (mtime) files are skipped (`FileStatus`). ### 4.6 `indexing/dense.rs` — Model2Vec embeddings (real + stub) @@ -316,8 +319,8 @@ Ported faithfully (`LazyLock` for the static patterns, `RefCell` a local source root is read lazily per returned result with a memo (misses memoized too), where upstream `_compute_file_sizes` runs eagerly over every indexed file in `SembleIndex.__init__`. Git sources still capture eagerly at clone time (the temp checkout is gone by search time). - Because the read now happens inside a live search, it is bounded by `MAX_FILE_BYTES` (the same - ceiling the indexer applies) — upstream, running at construction time, has no such bound. + Because the read now happens inside a live search, it is bounded by `get_max_file_bytes()` (the + same ceiling the indexer applies) — upstream, running at construction time, has no such bound. Decoding matches upstream `read_file_text` (`errors="replace"`) and the csp indexer (`String::from_utf8_lossy`), so a non-UTF-8 file that got indexed still gets sized. diff --git a/README.ko.md b/README.ko.md index 2fc7437..ffa053d 100644 --- a/README.ko.md +++ b/README.ko.md @@ -407,6 +407,8 @@ csp find-related src/auth.ts 42 ./my-project `csp search`나 `csp find-related`를 `--index` 없이 실행하면, `csp`는 소스와 콘텐츠 선택을 키로 하여 글로벌 캐시 `~/.csp/index/`에 자동으로 인덱싱·캐시합니다. 다음 실행 때 캐시를 재사용하며, 소스 파일이 바뀌면 콘텐츠 해시로 자동 무효화되므로 수동으로 다시 인덱싱할 필요가 없습니다. 무효화된 캐시는 증분으로 다시 만들어집니다. 내용이 바뀐 파일만 다시 청킹·임베딩하고, 바뀌지 않은 파일은 기존 청크·벡터·BM25 포스팅을 그대로 유지합니다. `--index <경로>`를 지정하면 그 경로를 그대로 사용하고 자동 캐시를 우회합니다. `csp index -o <경로>`는 명시적 영속화 전용(`-o` 필수)이며 자동 캐시와는 독립적입니다. +인덱스를 가볍게 유지하기 위해 1 MB보다 큰 파일은 인덱싱에서 제외됩니다. 제외된 파일은 인덱싱 시점에 stderr 경고로 앞쪽 몇 개의 경로와 함께 보고됩니다. 생성된 대용량 문서나 수집 문서를 다뤄야 한다면 `CSP_MAX_FILE_BYTES` 환경 변수(바이트 단위)로 이 한도를 올리거나 내릴 수 있습니다. 값이 잘못됐거나 0 이하이면 경고 후 기본값 1 MB로 돌아갑니다. +
토큰 절약량 보기 diff --git a/README.md b/README.md index 0dfbd70..ce8d146 100644 --- a/README.md +++ b/README.md @@ -407,6 +407,8 @@ csp find-related src/auth.ts 42 ./my-project When you run `csp search` or `csp find-related` **without** `--index`, `csp` automatically indexes and caches the source in a global cache at `~/.csp/index/`, keyed by the source and content selection. The cache is reused on the next run and invalidated automatically when the source files change (by content hash), so you do not need to reindex manually. A stale cache is rebuilt incrementally: only files whose content changed are re-chunked and re-embedded, and unchanged files keep their existing chunks, vectors, and BM25 postings. Passing `--index ` uses that exact path instead and bypasses the auto-cache. `csp index -o ` is for explicit persistence only (`-o` is required) and is independent of the auto-cache. +Files larger than 1 MB are skipped during indexing to keep index builds lean. Skipped files are reported as a warning on stderr at index time, naming the first few paths. If you work with large generated or ingested documents, you can raise (or lower) this limit with the `CSP_MAX_FILE_BYTES` environment variable (in bytes); a malformed or non-positive value warns and falls back to the 1 MB default. +
Savings diff --git a/crates/csp/src/indexing/cache_orchestrator.rs b/crates/csp/src/indexing/cache_orchestrator.rs index 3747eba..6624486 100644 --- a/crates/csp/src/indexing/cache_orchestrator.rs +++ b/crates/csp/src/indexing/cache_orchestrator.rs @@ -9,11 +9,11 @@ use crate::chunking::source::DESIRED_CHUNK_LENGTH_CHARS; use crate::indexing::cache::{ compute_content_hash_from_paths, ensure_cache_dir, resolve_cache_dir, CacheLocation, }; -use crate::indexing::create::MAX_FILE_BYTES; use crate::indexing::dense::SelectableBasicBackend; use crate::indexing::dense::DEFAULT_MODEL_NAME; use crate::indexing::file_walker::walk_files; use crate::indexing::files::get_extensions; +use crate::indexing::files::get_max_file_bytes; use crate::indexing::index::{ normalize_content, parse_manifest, read_chunks, CspIndex, IndexManifest, LoadOptions, INDEX_SCHEMA_VERSION, @@ -42,11 +42,14 @@ fn collect_source_paths(root: &Path, content: &[ContentType]) -> Vec<(String, Pa let resolved = get_extensions(content, None); let ext_refs: Vec<&str> = resolved.iter().map(String::as_str).collect(); let mut files = Vec::new(); + // Same ceiling as `create_index_from_path`, so the fingerprint tracks + // exactly the files that get indexed. + let max_file_bytes = get_max_file_bytes(); for file_path in walk_files(root, &ext_refs, &[]) { let Ok(meta) = std::fs::metadata(&file_path) else { continue; }; - if meta.len() > MAX_FILE_BYTES { + if meta.len() > max_file_bytes { continue; } let rel = file_path.strip_prefix(root).unwrap_or(&file_path); diff --git a/crates/csp/src/indexing/create.rs b/crates/csp/src/indexing/create.rs index dabb83b..07cac89 100644 --- a/crates/csp/src/indexing/create.rs +++ b/crates/csp/src/indexing/create.rs @@ -15,14 +15,21 @@ use crate::chunking::source::chunk_source; use crate::indexing::cache::sha256_hex; use crate::indexing::dense::{embed_chunk_refs, Model, SelectableBasicBackend}; use crate::indexing::file_walker::walk_files; -use crate::indexing::files::{detect_language, get_extensions}; +use crate::indexing::files::{ + detect_language, get_extensions, get_max_file_bytes, DEFAULT_MAX_FILE_BYTES, MAX_FILE_BYTES_ENV, +}; use crate::indexing::sparse::{enrich_for_bm25, Bm25Index}; use crate::indexing::types::{make_chunk_id, FileManifest, FileManifestEntry, PreviousIndex}; use crate::tokens::tokenize; use crate::types::{Chunk, ContentType}; /// 1 MB max file size to read and index. -pub const MAX_FILE_BYTES: u64 = 1_000_000; +#[deprecated( + since = "0.1.10", + note = "use `indexing::files::DEFAULT_MAX_FILE_BYTES` or `get_max_file_bytes()` \ + (the limit is now overridable via `CSP_MAX_FILE_BYTES`)" +)] +pub const MAX_FILE_BYTES: u64 = DEFAULT_MAX_FILE_BYTES; /// Options for [`create_index_from_path`]. pub struct CreateIndexOptions<'a> { @@ -33,6 +40,10 @@ pub struct CreateIndexOptions<'a> { pub content: Option>, /// When set, chunk file paths are stored relative to this root. pub display_root: Option, + /// Max file size (bytes) to read and index; larger files are skipped with a + /// warning. `None` resolves `CSP_MAX_FILE_BYTES` (default 1 MB) via + /// [`get_max_file_bytes`]. + pub max_file_bytes: Option, } /// Result of [`create_index_from_path`]. @@ -45,6 +56,26 @@ pub struct CreateIndexResult { pub files: FileManifest, } +/// The warning for files skipped for exceeding `max_file_bytes` — the count +/// plus the first five paths — or `None` when nothing was skipped. Port of +/// upstream `_warn_skipped_large` (semble #252): a silent skip left unexplained +/// gaps in results. +pub(crate) fn skipped_large_warning(skipped: &[String], max_file_bytes: u64) -> Option { + if skipped.is_empty() { + return None; + } + let shown = &skipped[..skipped.len().min(5)]; + Some(format!( + "Skipped {} file(s) exceeding the maximum file size of {} bytes \ + (raise {} to include them): {}{}", + skipped.len(), + max_file_bytes, + MAX_FILE_BYTES_ENV, + shown.join(", "), + if skipped.len() > 5 { " ..." } else { "" }, + )) +} + /// Replace a file's BM25 postings: remove its old slots (if any), then add its /// new ones. fn reindex_file( @@ -169,6 +200,7 @@ pub fn create_index_from_path( .unwrap_or_else(|| vec![ContentType::Code]); let resolved = get_extensions(&content, options.extensions.as_deref()); let ext_refs: Vec<&str> = resolved.iter().map(String::as_str).collect(); + let max_file_bytes = options.max_file_bytes.unwrap_or_else(get_max_file_bytes); let (mut bm25_index, previous_files, mut previous_chunks, mut previous_vectors) = open_previous(previous); @@ -181,6 +213,7 @@ pub fn create_index_from_path( let mut vectors: Vec>> = Vec::new(); let mut fresh_rows: Vec = Vec::new(); let mut files = FileManifest::new(); + let mut skipped_large: Vec = Vec::new(); for file_path in walk_files(path, &ext_refs, &[]) { let language = detect_language(&file_path.to_string_lossy()); @@ -188,7 +221,8 @@ pub fn create_index_from_path( Ok(meta) => meta.len(), Err(_) => continue, }; - if size > MAX_FILE_BYTES { + if size > max_file_bytes { + skipped_large.push(file_path.display().to_string()); continue; } let Ok(bytes) = std::fs::read(&file_path) else { @@ -248,6 +282,12 @@ pub fn create_index_from_path( } } + // Warn before the empty check so a tree of only oversized files explains + // itself rather than just reporting "no supported files". + if let Some(warning) = skipped_large_warning(&skipped_large, max_file_bytes) { + eprintln!("csp: {warning}"); + } + if chunks.is_empty() { return Err(format!( "No supported files found under {}.", diff --git a/crates/csp/src/indexing/create/tests.rs b/crates/csp/src/indexing/create/tests.rs index efd4e45..2dee527 100644 --- a/crates/csp/src/indexing/create/tests.rs +++ b/crates/csp/src/indexing/create/tests.rs @@ -9,6 +9,7 @@ fn opts(model: &Model, display_root: Option) -> CreateIndexOptions<'_> extensions: None, content: None, display_root, + max_file_bytes: None, } } @@ -53,6 +54,7 @@ fn respects_extensions_override() { extensions: Some(vec![".txt".to_string()]), content: Some(vec![ContentType::Docs]), display_root: Some(dir.path().to_path_buf()), + max_file_bytes: None, }; let result = create_index_from_path(dir.path(), &options, None).unwrap(); assert_eq!(result.chunks.len(), 1); @@ -76,6 +78,51 @@ fn skips_files_over_max_bytes() { assert!(!paths.contains(&"big.ts")); } +#[test] +fn honors_configured_max_file_bytes() { + let dir = tempdir().unwrap(); + std::fs::write(dir.path().join("mid.ts"), "a".repeat(200)).unwrap(); + std::fs::write(dir.path().join("small.ts"), "export const x = 1\n").unwrap(); + let model = make_stub_model(4); + let indexed = |limit: u64| -> Vec { + let options = CreateIndexOptions { + max_file_bytes: Some(limit), + ..opts(&model, Some(dir.path().to_path_buf())) + }; + let result = create_index_from_path(dir.path(), &options, None).unwrap(); + result.chunks.iter().map(|c| c.file_path.clone()).collect() + }; + + // A limit under the default still gates: 200 bytes > 100. + let paths = indexed(100); + assert!(paths.iter().any(|p| p == "small.ts")); + assert!(!paths.iter().any(|p| p == "mid.ts")); + // Raising it lets the same file through. + assert!(indexed(1_000).iter().any(|p| p == "mid.ts")); +} + +#[test] +fn skipped_large_warning_names_first_five_paths_and_count() { + assert_eq!(skipped_large_warning(&[], 1_000_000), None); + + let two = vec!["a.ts".to_string(), "b.ts".to_string()]; + let msg = skipped_large_warning(&two, 1_000_000).unwrap(); + assert_eq!( + msg, + "Skipped 2 file(s) exceeding the maximum file size of 1000000 bytes \ + (raise CSP_MAX_FILE_BYTES to include them): a.ts, b.ts" + ); + + let seven: Vec = (1..=7).map(|i| format!("f{i}.ts")).collect(); + let msg = skipped_large_warning(&seven, 500).unwrap(); + assert!(msg.starts_with("Skipped 7 file(s) exceeding the maximum file size of 500 bytes")); + assert!( + msg.ends_with("f1.ts, f2.ts, f3.ts, f4.ts, f5.ts ..."), + "{msg}" + ); + assert!(!msg.contains("f6.ts")); +} + #[test] fn descends_into_subdirectories() { let dir = tempdir().unwrap(); diff --git a/crates/csp/src/indexing/file_sizes.rs b/crates/csp/src/indexing/file_sizes.rs index 9b64433..2c7a108 100644 --- a/crates/csp/src/indexing/file_sizes.rs +++ b/crates/csp/src/indexing/file_sizes.rs @@ -11,7 +11,7 @@ use std::io::Read as _; use std::path::{Path, PathBuf}; use std::sync::Mutex; -use crate::indexing::create::MAX_FILE_BYTES; +use crate::indexing::files::get_max_file_bytes; /// UTF-16 character counts per repo-relative file path, resolved eagerly /// (captured) or lazily (read from a local root on demand). @@ -97,7 +97,7 @@ impl FileSizes { /// the file walker never follows symlinks, and a path that has since become a /// symlink, FIFO, or device must not be able to redirect or stall the read. /// -/// The read is bounded by [`MAX_FILE_BYTES`], the same ceiling +/// The read is bounded by [`get_max_file_bytes`], the same ceiling /// `create_index_from_path` applies — lazily, this runs inside a live search, /// so a file that has grown past the indexing limit since it was chunked must /// not be slurped whole on the query path. Decoding is lossy, matching the @@ -137,12 +137,13 @@ pub(crate) fn read_file_chars(root: &Path, file_path: &str) -> Option { // a local writer who already controls the indexed tree. let file = std::fs::File::open(&canonical).ok()?; let meta = file.metadata().ok()?; - if !meta.is_file() || meta.len() > MAX_FILE_BYTES { + let max_file_bytes = get_max_file_bytes(); + if !meta.is_file() || meta.len() > max_file_bytes { return None; } let mut bytes = Vec::with_capacity(meta.len() as usize); - file.take(MAX_FILE_BYTES + 1).read_to_end(&mut bytes).ok()?; - if bytes.len() as u64 > MAX_FILE_BYTES { + file.take(max_file_bytes + 1).read_to_end(&mut bytes).ok()?; + if bytes.len() as u64 > max_file_bytes { return None; } Some(String::from_utf8_lossy(&bytes).encode_utf16().count() as u64) @@ -260,7 +261,7 @@ mod tests { #[test] fn lazy_skips_files_larger_than_the_indexing_ceiling() { let root = tempdir().unwrap(); - let big = vec![b'a'; MAX_FILE_BYTES as usize + 1]; + let big = vec![b'a'; get_max_file_bytes() as usize + 1]; std::fs::write(root.path().join("grown.ts"), &big).unwrap(); let sizes = FileSizes::lazy(root.path().to_path_buf()); diff --git a/crates/csp/src/indexing/files.rs b/crates/csp/src/indexing/files.rs index d286ca6..dd165e9 100644 --- a/crates/csp/src/indexing/files.rs +++ b/crates/csp/src/indexing/files.rs @@ -2,10 +2,55 @@ //! `src/indexing/files.ts` (← semble `index/files.py`). use std::collections::{BTreeSet, HashMap, HashSet}; -use std::sync::LazyLock; +use std::sync::{LazyLock, Once}; use crate::types::ContentType; +/// Default (1 MB) max file size to read and index. Overridable per process via +/// [`MAX_FILE_BYTES_ENV`] — see [`get_max_file_bytes`]. +pub const DEFAULT_MAX_FILE_BYTES: u64 = 1_000_000; + +/// Environment variable overriding [`DEFAULT_MAX_FILE_BYTES`], in bytes +/// (upstream `SEMBLE_MAX_FILE_BYTES`). +pub const MAX_FILE_BYTES_ENV: &str = "CSP_MAX_FILE_BYTES"; + +/// Parse a raw [`MAX_FILE_BYTES_ENV`] value. `None` (unset) is the default; +/// a malformed or non-positive value is `Err` carrying the warning to print +/// before falling back to the default, mirroring upstream `get_max_file_bytes`. +pub(crate) fn parse_max_file_bytes(raw: Option<&str>) -> Result { + let Some(raw) = raw else { + return Ok(DEFAULT_MAX_FILE_BYTES); + }; + match raw.trim().parse::() { + Ok(value) if value > 0 => Ok(value as u64), + Ok(_) => Err(format!( + "{MAX_FILE_BYTES_ENV} must be positive, using the default of \ + {DEFAULT_MAX_FILE_BYTES} bytes" + )), + Err(_) => Err(format!( + "Invalid {MAX_FILE_BYTES_ENV} {raw:?}, using the default of \ + {DEFAULT_MAX_FILE_BYTES} bytes" + )), + } +} + +/// The maximum file size to read and index: [`MAX_FILE_BYTES_ENV`] when set, +/// else [`DEFAULT_MAX_FILE_BYTES`]. A malformed or non-positive override warns +/// on stderr (once per process — this also runs per result on the search path) +/// and falls back to the default rather than aborting indexing. +pub fn get_max_file_bytes() -> u64 { + static WARNED: Once = Once::new(); + let raw = std::env::var_os(MAX_FILE_BYTES_ENV); + let raw = raw.as_deref().map(|s| s.to_string_lossy()); + match parse_max_file_bytes(raw.as_deref()) { + Ok(value) => value, + Err(warning) => { + WARNED.call_once(|| eprintln!("csp: {warning}")); + DEFAULT_MAX_FILE_BYTES + } + } +} + /// Extension (including the leading dot, lowercase) → tree-sitter language name. /// Transcribed verbatim from the upstream `EXTENSION_TO_LANGUAGE`. pub const EXTENSION_TO_LANGUAGE: &[(&str, &str)] = &[ @@ -524,6 +569,25 @@ pub fn get_extensions(types: &[ContentType], extra: Option<&[String]>) -> Vec Date: Fri, 4 Sep 2026 23:11:04 +0900 Subject: [PATCH 2/3] chore(index): apply AI code review suggestions - Cache the parsed CSP_MAX_FILE_BYTES in a OnceLock so the env var is read and parsed once per process (gemini-code-assist) - Add CreateIndexOptions::new(model) so callers using struct-update syntax stay source-compatible with future option fields (greptile) - Escape control characters in repository-controlled paths before printing them in stderr diagnostics (greptile, cubic) - Accept the full unsigned range for CSP_MAX_FILE_BYTES while keeping the "must be positive" message for negative values; make the lazy file_chars read cap overflow-safe (cubic) - Point the skipped-file hint at `max_file_bytes` instead of the env var when a caller pinned the limit explicitly (cubic) --- crates/csp/src/indexing/create.rs | 59 ++++++++++++++++++++++--- crates/csp/src/indexing/create/tests.rs | 27 ++++++++--- crates/csp/src/indexing/file_sizes.rs | 4 +- crates/csp/src/indexing/files.rs | 53 ++++++++++++++-------- 4 files changed, 110 insertions(+), 33 deletions(-) diff --git a/crates/csp/src/indexing/create.rs b/crates/csp/src/indexing/create.rs index 07cac89..84ba151 100644 --- a/crates/csp/src/indexing/create.rs +++ b/crates/csp/src/indexing/create.rs @@ -46,6 +46,24 @@ pub struct CreateIndexOptions<'a> { pub max_file_bytes: Option, } +impl<'a> CreateIndexOptions<'a> { + /// Options for `model` with every other field at its default: code-only + /// content, no extra extensions, chunk paths as walked, and the size limit + /// resolved from `CSP_MAX_FILE_BYTES`. Prefer + /// `CreateIndexOptions { extensions: .., ..CreateIndexOptions::new(model) }` + /// over a bare struct literal so a future option field does not break + /// callers. + pub fn new(model: &'a Model) -> Self { + Self { + model, + extensions: None, + content: None, + display_root: None, + max_file_bytes: None, + } + } +} + /// Result of [`create_index_from_path`]. #[derive(Debug)] pub struct CreateIndexResult { @@ -56,21 +74,46 @@ pub struct CreateIndexResult { pub files: FileManifest, } +/// A repository-controlled path rendered for a stderr diagnostic, with control +/// characters (ANSI escapes, newlines) escaped so a hostile file name in a +/// cloned repo cannot rewrite the terminal or forge diagnostic lines. +fn escape_control(path: &str) -> String { + let mut out = String::with_capacity(path.len()); + for c in path.chars() { + if c.is_control() { + out.extend(c.escape_default()); + } else { + out.push(c); + } + } + out +} + /// The warning for files skipped for exceeding `max_file_bytes` — the count /// plus the first five paths — or `None` when nothing was skipped. Port of /// upstream `_warn_skipped_large` (semble #252): a silent skip left unexplained -/// gaps in results. -pub(crate) fn skipped_large_warning(skipped: &[String], max_file_bytes: u64) -> Option { +/// gaps in results. `from_env` selects the hint: the env var when the limit +/// was resolved from it, else the `max_file_bytes` option the caller pinned. +pub(crate) fn skipped_large_warning( + skipped: &[String], + max_file_bytes: u64, + from_env: bool, +) -> Option { if skipped.is_empty() { return None; } - let shown = &skipped[..skipped.len().min(5)]; + let shown: Vec = skipped.iter().take(5).map(|p| escape_control(p)).collect(); + let knob = if from_env { + MAX_FILE_BYTES_ENV + } else { + "max_file_bytes" + }; Some(format!( "Skipped {} file(s) exceeding the maximum file size of {} bytes \ (raise {} to include them): {}{}", skipped.len(), max_file_bytes, - MAX_FILE_BYTES_ENV, + knob, shown.join(", "), if skipped.len() > 5 { " ..." } else { "" }, )) @@ -239,7 +282,7 @@ pub fn create_index_from_path( eprintln!( "csp: skipping {}: its display path collides with an already indexed file \ (non-UTF-8 file name)", - file_path.display() + escape_control(&file_path.display().to_string()) ); continue; } @@ -284,7 +327,11 @@ pub fn create_index_from_path( // Warn before the empty check so a tree of only oversized files explains // itself rather than just reporting "no supported files". - if let Some(warning) = skipped_large_warning(&skipped_large, max_file_bytes) { + if let Some(warning) = skipped_large_warning( + &skipped_large, + max_file_bytes, + options.max_file_bytes.is_none(), + ) { eprintln!("csp: {warning}"); } diff --git a/crates/csp/src/indexing/create/tests.rs b/crates/csp/src/indexing/create/tests.rs index 2dee527..c4f9e60 100644 --- a/crates/csp/src/indexing/create/tests.rs +++ b/crates/csp/src/indexing/create/tests.rs @@ -5,11 +5,8 @@ use tempfile::tempdir; fn opts(model: &Model, display_root: Option) -> CreateIndexOptions<'_> { CreateIndexOptions { - model, - extensions: None, - content: None, display_root, - max_file_bytes: None, + ..CreateIndexOptions::new(model) } } @@ -103,10 +100,10 @@ fn honors_configured_max_file_bytes() { #[test] fn skipped_large_warning_names_first_five_paths_and_count() { - assert_eq!(skipped_large_warning(&[], 1_000_000), None); + assert_eq!(skipped_large_warning(&[], 1_000_000, true), None); let two = vec!["a.ts".to_string(), "b.ts".to_string()]; - let msg = skipped_large_warning(&two, 1_000_000).unwrap(); + let msg = skipped_large_warning(&two, 1_000_000, true).unwrap(); assert_eq!( msg, "Skipped 2 file(s) exceeding the maximum file size of 1000000 bytes \ @@ -114,13 +111,29 @@ fn skipped_large_warning_names_first_five_paths_and_count() { ); let seven: Vec = (1..=7).map(|i| format!("f{i}.ts")).collect(); - let msg = skipped_large_warning(&seven, 500).unwrap(); + let msg = skipped_large_warning(&seven, 500, true).unwrap(); assert!(msg.starts_with("Skipped 7 file(s) exceeding the maximum file size of 500 bytes")); assert!( msg.ends_with("f1.ts, f2.ts, f3.ts, f4.ts, f5.ts ..."), "{msg}" ); assert!(!msg.contains("f6.ts")); + + // A caller-pinned limit points at the option, not the env var. + let msg = skipped_large_warning(&two, 100, false).unwrap(); + assert!( + msg.contains("(raise max_file_bytes to include them)"), + "{msg}" + ); + assert!(!msg.contains("CSP_MAX_FILE_BYTES")); +} + +#[test] +fn skipped_large_warning_escapes_control_characters_in_paths() { + let hostile = vec!["evil\x1b[31m\nfake: ok.ts".to_string()]; + let msg = skipped_large_warning(&hostile, 10, true).unwrap(); + assert!(msg.ends_with("evil\\u{1b}[31m\\nfake: ok.ts"), "{msg}"); + assert!(!msg.contains('\n')); } #[test] diff --git a/crates/csp/src/indexing/file_sizes.rs b/crates/csp/src/indexing/file_sizes.rs index 2c7a108..96135a7 100644 --- a/crates/csp/src/indexing/file_sizes.rs +++ b/crates/csp/src/indexing/file_sizes.rs @@ -142,7 +142,9 @@ pub(crate) fn read_file_chars(root: &Path, file_path: &str) -> Option { return None; } let mut bytes = Vec::with_capacity(meta.len() as usize); - file.take(max_file_bytes + 1).read_to_end(&mut bytes).ok()?; + file.take(max_file_bytes.saturating_add(1)) + .read_to_end(&mut bytes) + .ok()?; if bytes.len() as u64 > max_file_bytes { return None; } diff --git a/crates/csp/src/indexing/files.rs b/crates/csp/src/indexing/files.rs index dd165e9..84ad266 100644 --- a/crates/csp/src/indexing/files.rs +++ b/crates/csp/src/indexing/files.rs @@ -2,7 +2,7 @@ //! `src/indexing/files.ts` (← semble `index/files.py`). use std::collections::{BTreeSet, HashMap, HashSet}; -use std::sync::{LazyLock, Once}; +use std::sync::{LazyLock, OnceLock}; use crate::types::ContentType; @@ -21,12 +21,12 @@ pub(crate) fn parse_max_file_bytes(raw: Option<&str>) -> Result { let Some(raw) = raw else { return Ok(DEFAULT_MAX_FILE_BYTES); }; - match raw.trim().parse::() { - Ok(value) if value > 0 => Ok(value as u64), - Ok(_) => Err(format!( - "{MAX_FILE_BYTES_ENV} must be positive, using the default of \ - {DEFAULT_MAX_FILE_BYTES} bytes" - )), + let trimmed = raw.trim(); + match trimmed.parse::() { + Ok(value) if value > 0 => Ok(value), + // Zero, or a negative integer (which only parses as signed). + Ok(_) => Err(non_positive_warning()), + Err(_) if trimmed.parse::().is_ok() => Err(non_positive_warning()), Err(_) => Err(format!( "Invalid {MAX_FILE_BYTES_ENV} {raw:?}, using the default of \ {DEFAULT_MAX_FILE_BYTES} bytes" @@ -34,21 +34,31 @@ pub(crate) fn parse_max_file_bytes(raw: Option<&str>) -> Result { } } +fn non_positive_warning() -> String { + format!( + "{MAX_FILE_BYTES_ENV} must be positive, using the default of \ + {DEFAULT_MAX_FILE_BYTES} bytes" + ) +} + /// The maximum file size to read and index: [`MAX_FILE_BYTES_ENV`] when set, -/// else [`DEFAULT_MAX_FILE_BYTES`]. A malformed or non-positive override warns -/// on stderr (once per process — this also runs per result on the search path) -/// and falls back to the default rather than aborting indexing. +/// else [`DEFAULT_MAX_FILE_BYTES`]. The variable is read and parsed once per +/// process (this also runs per result on the search path); a malformed or +/// non-positive override warns on stderr that one time and falls back to the +/// default rather than aborting indexing. pub fn get_max_file_bytes() -> u64 { - static WARNED: Once = Once::new(); - let raw = std::env::var_os(MAX_FILE_BYTES_ENV); - let raw = raw.as_deref().map(|s| s.to_string_lossy()); - match parse_max_file_bytes(raw.as_deref()) { - Ok(value) => value, - Err(warning) => { - WARNED.call_once(|| eprintln!("csp: {warning}")); - DEFAULT_MAX_FILE_BYTES + static MAX_FILE_BYTES: OnceLock = OnceLock::new(); + *MAX_FILE_BYTES.get_or_init(|| { + let raw = std::env::var_os(MAX_FILE_BYTES_ENV); + let raw = raw.as_deref().map(|s| s.to_string_lossy()); + match parse_max_file_bytes(raw.as_deref()) { + Ok(value) => value, + Err(warning) => { + eprintln!("csp: {warning}"); + DEFAULT_MAX_FILE_BYTES + } } - } + }) } /// Extension (including the leading dot, lowercase) → tree-sitter language name. @@ -574,6 +584,11 @@ mod tests { assert_eq!(parse_max_file_bytes(None), Ok(DEFAULT_MAX_FILE_BYTES)); assert_eq!(parse_max_file_bytes(Some("2000000")), Ok(2_000_000)); assert_eq!(parse_max_file_bytes(Some(" 512 ")), Ok(512)); + // The full unsigned range is valid, not just what fits in i64. + assert_eq!( + parse_max_file_bytes(Some("18446744073709551615")), + Ok(u64::MAX) + ); } #[test] From 0ab624a850a90275b704dbd2908491fc372f8cca Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 4 Sep 2026 23:16:28 +0900 Subject: [PATCH 3/3] chore(index): apply AI code review suggestions (round 2) - Detect a negative CSP_MAX_FILE_BYTES of any magnitude by shape ("-" + digits) instead of an i64 parse, keeping "-abc" malformed (gemini-code-assist) - Record skipped-for-size paths relative to the display root, matching the indexed chunk paths (gemini-code-assist) --- crates/csp/src/indexing/create.rs | 2 +- crates/csp/src/indexing/files.rs | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/csp/src/indexing/create.rs b/crates/csp/src/indexing/create.rs index 84ba151..751c972 100644 --- a/crates/csp/src/indexing/create.rs +++ b/crates/csp/src/indexing/create.rs @@ -265,7 +265,7 @@ pub fn create_index_from_path( Err(_) => continue, }; if size > max_file_bytes { - skipped_large.push(file_path.display().to_string()); + skipped_large.push(display_path(&file_path, options.display_root.as_deref())); continue; } let Ok(bytes) = std::fs::read(&file_path) else { diff --git a/crates/csp/src/indexing/files.rs b/crates/csp/src/indexing/files.rs index 84ad266..9dbd887 100644 --- a/crates/csp/src/indexing/files.rs +++ b/crates/csp/src/indexing/files.rs @@ -22,11 +22,15 @@ pub(crate) fn parse_max_file_bytes(raw: Option<&str>) -> Result { return Ok(DEFAULT_MAX_FILE_BYTES); }; let trimmed = raw.trim(); + // A negative integer of any magnitude is non-positive, like upstream's + // arbitrary-precision `int()`; `-abc` stays malformed. + let negative_integer = trimmed + .strip_prefix('-') + .is_some_and(|digits| !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit())); match trimmed.parse::() { Ok(value) if value > 0 => Ok(value), - // Zero, or a negative integer (which only parses as signed). Ok(_) => Err(non_positive_warning()), - Err(_) if trimmed.parse::().is_ok() => Err(non_positive_warning()), + Err(_) if negative_integer => Err(non_positive_warning()), Err(_) => Err(format!( "Invalid {MAX_FILE_BYTES_ENV} {raw:?}, using the default of \ {DEFAULT_MAX_FILE_BYTES} bytes" @@ -601,6 +605,11 @@ mod tests { assert!(err.contains("CSP_MAX_FILE_BYTES must be positive"), "{err}"); let err = parse_max_file_bytes(Some("-5")).unwrap_err(); assert!(err.contains("must be positive"), "{err}"); + // Any magnitude of negative integer, but not a malformed negative. + let err = parse_max_file_bytes(Some("-99999999999999999999")).unwrap_err(); + assert!(err.contains("must be positive"), "{err}"); + let err = parse_max_file_bytes(Some("-abc")).unwrap_err(); + assert!(err.contains("Invalid CSP_MAX_FILE_BYTES"), "{err}"); } #[test]