feat(index): make the 1 MB max file size configurable via CSP_MAX_FILE_BYTES - #101
Conversation
…E_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
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 20 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request makes the maximum file size limit for indexing overridable via the CSP_MAX_FILE_BYTES environment variable (defaulting to 1 MB) and adds warnings when files are skipped. The feedback suggests caching the parsed environment variable using OnceLock to avoid repeated system calls and lock contention, along with updating the imports accordingly.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR makes the indexing file-size ceiling configurable through
Confidence Score: 4/5The PR should not merge until the public Existing downstream Rust code using Files Needing Attention: crates/csp/src/indexing/create.rs
|
| Filename | Overview |
|---|---|
| crates/csp/src/indexing/files.rs | Adds environment parsing, default fallback, and once-per-process invalid-value diagnostics. |
| crates/csp/src/indexing/create.rs | Applies the configurable limit and reports skipped paths, but breaks public struct-literal callers and prints unescaped repository-controlled filenames. |
| crates/csp/src/indexing/cache_orchestrator.rs | Applies the resolved ceiling to source fingerprint eligibility so disk caches rebuild when the indexed file set changes. |
| crates/csp/src/indexing/file_sizes.rs | Applies the environment-resolved ceiling to bounded lazy reads used for result statistics. |
| crates/csp/src/indexing/index.rs | Updates the internal index-construction call to use environment-based limit resolution. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
E[CSP_MAX_FILE_BYTES] --> R[get_max_file_bytes]
O[CreateIndexOptions override] --> G[Index size gate]
R --> G
R --> F[Cache source fingerprint]
R --> S[Lazy file-size statistics]
G -->|Accepted| I[Chunk and index file]
G -->|Oversized| W[Collect skipped path]
W --> D[stderr warning]
Prompt To Fix All With AI
### Issue 1
crates/csp/src/indexing/create.rs:46
**Public options break callers**
Downstream Rust callers that construct `CreateIndexOptions` with a struct literal will fail to compile because the new required `max_file_bytes` field is missing. The repository's own existing struct literals had to be updated with `max_file_bytes: None`, demonstrating the same compatibility break for external callers. Please provide a backward-compatible construction path before exposing this option.
### Issue 2
crates/csp/src/indexing/create.rs:225
**Paths inject terminal output**
Oversized paths are converted with `display().to_string()` and written to stderr without escaping. A cloned remote repository can therefore use an oversized filename containing ANSI or newline control characters to rewrite terminal output or inject misleading diagnostic lines. Escape control characters before including repository-controlled paths in this warning.
**How this was verified:** Remote repository filenames flow unchanged through file discovery into the new oversized-file warning, which joins and prints their display strings directly to stderr.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(index): make the 1 MB max file size..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Architecture diagram
sequenceDiagram
participant CLI as csp CLI / Library Caller
participant Index as Indexing Orchestrator
participant Limits as File Size Resolver
participant Env as Process Environment
participant FS as Source File System
participant Cache as Cache Fingerprint
participant Store as Index Storage
participant Search as Search and File Stats
participant Stderr as stderr
Note over CLI,Stderr: Configurable per-file size ceiling shared by indexing, caching, and lazy file statistics
CLI->>Index: Create or refresh index
alt Explicit max_file_bytes option
Index->>Limits: Resolve option value
Limits-->>Index: Configured byte ceiling
else No explicit option
Index->>Limits: get_max_file_bytes()
Limits->>Env: Read CSP_MAX_FILE_BYTES
alt Unset
Limits-->>Index: Default 1,000,000 bytes
else Positive integer
Limits-->>Index: Environment byte ceiling
else Malformed or non-positive
Limits->>Stderr: Warn once per process
Limits-->>Index: Default 1,000,000 bytes
end
end
Index->>FS: Walk supported source files
loop Each discovered file
Index->>FS: Read metadata and file size
alt File exceeds resolved ceiling
Index->>Index: Collect skipped path
else File within ceiling
Index->>FS: Read and index file contents
Index->>Store: Write chunks, vectors, and BM25 postings
end
end
opt One or more files were skipped
Index->>Stderr: Report count, first 5 paths, and CSP_MAX_FILE_BYTES hint
end
Index-->>CLI: Index result or no-supported-files error
CLI->>Cache: Resolve source fingerprint
Cache->>Limits: get_max_file_bytes()
Limits-->>Cache: Same effective ceiling
Cache->>FS: Walk and include only files within ceiling
Cache-->>CLI: Cache key covering indexed file set
CLI->>Search: Search result requests file statistics
Search->>Limits: get_max_file_bytes()
Limits-->>Search: Same effective ceiling
Search->>FS: Lazily read local file, bounded by ceiling
alt File remains within ceiling
FS-->>Search: File bytes
Search-->>CLI: UTF-16 character count
else File exceeds ceiling or is not a regular file
Search-->>CLI: No file size result
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- 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)
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an overridable maximum file size limit for indexing (default 1 MB) via the CSP_MAX_FILE_BYTES environment variable or a max_file_bytes option, and adds a stderr warning listing skipped oversized files. The review feedback suggests simplifying the negative number parsing logic to prevent overflow issues and using relative display paths for skipped files to avoid leaking absolute directory structures in the warnings.
- 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)
|
/gemini review |
|
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to override the maximum file size limit for indexing (default 1 MB) via the CSP_MAX_FILE_BYTES environment variable or a configuration option. It also adds stderr warnings for skipped files and escapes control characters in skipped file paths for security. The reviewer suggests supporting a leading plus sign when parsing CSP_MAX_FILE_BYTES to maintain behavioral parity with upstream Python's integer parsing.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
1 participant
Summary
Ports upstream MinishLab/semble#252 (
9bb38fd, closes upstream #250): the 1 MB per-file indexing ceiling is now configurable through theCSP_MAX_FILE_BYTESenvironment variable, and files skipped for exceeding it are reported on stderr instead of disappearing silently.Previously a file larger than the hard-coded 1 MB limit was dropped from the index with no signal, so a large-but-legitimate source file simply never appeared in search results and there was no way to raise the limit.
Changes
crates/csp/src/indexing/files.rs: addDEFAULT_MAX_FILE_BYTES(1 MB),MAX_FILE_BYTES_ENV = "CSP_MAX_FILE_BYTES", andget_max_file_bytes(). The env var is read in bytes; a malformed or non-positive value warns on stderr and falls back to the default.crates/csp/src/indexing/create.rs:create_index_from_pathcollects the paths it skipped for size and prints one stderr warning (count, first 5 paths, and a hint to raiseCSP_MAX_FILE_BYTES) before the "no supported files" error.CreateIndexOptionsgainsmax_file_bytes: Option<u64>, whereNoneresolves the env var.create::MAX_FILE_BYTESstays as a#[deprecated]alias.cache_orchestrator.rs(cache fingerprint) andfile_sizes.rs(lazyfile_charsread) resolve the same ceiling, so the limit stays consistent across indexing, caching, and stats.README.md/README.ko.mddocument the limit and the env var;.please/docs/references/semble.mddrift table updated.Intentional divergence from upstream
Upstream re-warns for every file when the env value is invalid. csp warns once per process instead, so the search path does not flood stderr on every indexed file.
Related issue
Closes #86
Test plan
cargo fmt --all && cargo clippy --all-targets --all-features -- -D warnings && cargo test --workspace— all pass (313 lib + 22 CLI tests)CSP_MAX_FILE_BYTES=100;CSP_MAX_FILE_BYTES=1MBemits the fallback warning; no skips at 5 MBChecklist
mise run test)mise run lint)BREAKING CHANGE:note is includedSummary by cubic
Makes the hard-coded 1 MB per-file indexing ceiling configurable via
CSP_MAX_FILE_BYTES, porting upstream semble #252 and closing #86. Files skipped for exceeding the limit are now reported on stderr instead of disappearing silently, so oversized source files no longer vanish from search results without explanation.Details
DEFAULT_MAX_FILE_BYTES); the env var is parsed once per process, and a malformed or non-positive value warns and falls back to it.CreateIndexOptionsgainsmax_file_bytesand anew()constructor;Noneresolves the env var for library callers.create::MAX_FILE_BYTESis deprecated in favor ofDEFAULT_MAX_FILE_BYTESandget_max_file_bytes(), so existing callers keep working with a compile-time warning.Written for commit 0ab624a. Summary will update on new commits.