feat(stats): wire token-savings telemetry into search and find_related#82
feat(stats): wire token-savings telemetry into search and find_related#82amondnet wants to merge 1 commit into
Conversation
`save_search_stats` was defined and tested but never called — no code path recorded to `~/.csp/savings.jsonl`, so `csp savings` always reported zero. Wire it in at the CLI/MCP app boundary (keeping `CspIndex::search` a pure, side-effect-free library call) and apply the semble#206 snippet accounting. - CspIndex gains `file_sizes` (repo-relative path → UTF-16 char count), captured at build time in `from_path`, carried through `from_git`'s re-root, and recomputed in `load_from_disk` when the source root is a still-present local directory (mirrors semble reading sizes off `root`). - `save_search_stats` takes `max_snippet_lines` so recorded `snippet_chars` reflects what the caller actually received (semble#206): `None` → full chunk, `Some(0)` → 0, `Some(n)` → first n lines. - Recording happens in `search_output` / `find_related_output` (CLI) and `search_tool` / `find_related_tool` (MCP), each behind an injected `Option<&Path>` stats file so tests redirect telemetry off the real `~/.csp/savings.jsonl`. The MCP server and CLI dispatch default to the real file; tests pass a temp file or `None`. Closes #81.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 15 |
| Duplication | 8 |
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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 5 files
Architecture diagram
sequenceDiagram
participant CLI as CLI Dispatch
participant MCP as MCP Server
participant Stats as save_search_stats()
participant Index as CspIndex
participant Disk as Source Tree on Disk
participant File as ~/.csp/savings.jsonl
Note over CLI,MCP: Search / find-related entry points
CLI->>Index: search(query, options)
Index->>Disk: read_from_path(root)
Disk-->>Index: file_sizes (UTF-16 char counts)
Index-->>CLI: results
CLI->>Stats: save_search_stats(stats_file, results, CallType::Search, index.file_sizes, max_snippet_lines)
alt max_snippet_lines is None
Stats->>Stats: snippet_chars = utf16_len(content) (full chunk)
else max_snippet_lines is Some(0)
Stats->>Stats: snippet_chars = 0
else max_snippet_lines is Some(n)
Stats->>Stats: snippet_chars = utf16_len(first n lines)
end
Stats->>File: Append JSONL record
File-->>Stats: (swallow I/O errors)
Note over MCP,Index: MCP tools with injected stats_file
MCP->>Index: search_tool(cache, ..., stats_file)
Index-->>MCP: results
alt stats_file is Some
MCP->>Stats: save_search_stats(stats_file, results, CallType::Search, index.file_sizes, max_snippet_lines)
Stats->>File: Append JSONL record
end
Note over Index,Disk: File sizes lifecycle
alt from_path() (local directory)
Index->>Disk: compute_file_sizes(root, chunks)
Disk-->>Index: HashMap<path, utf16_char_count>
else from_git() (remote URL)
Index->>Index: clone from_path computes sizes
Index->>Index: clone file_sizes before re-rooting
else load_from_disk() (cached manifest)
alt root is a present local directory
Index->>Disk: compute_file_sizes(root, chunks)
Disk-->>Index: HashMap<path, utf16_char_count>
else root is git URL or missing
Index->>Index: file_sizes = empty (file_chars = 0)
end
end
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![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
이번 풀 리퀘스트는 검색 및 관련 청크 찾기 기능에 토큰 절약 텔레메트리(통계 기록) 기능을 추가하는 변경사항을 담고 있습니다. 인덱스 빌드 시 파일 크기를 캡처하고, 검색 결과 출력 시 실제 전달된 문자 수를 계산하여 통계 파일에 기록하도록 구현되었습니다. 코드 리뷰 결과, compute_file_sizes 함수에서 상위 디렉터리 참조(..)나 절대 경로를 검증하지 않아 발생할 수 있는 경로 탐색(Path Traversal) 취약점(HIGH)과, delivered_chars 함수에서 불필요한 메모리 할당으로 인한 성능 저하 우려(MEDIUM)가 지적되었습니다. 두 피드백 모두 타당하며 제시된 코드로 수정할 것을 권장합니다.
| fn compute_file_sizes(root: &Path, chunks: &[Chunk]) -> HashMap<String, u64> { | ||
| let mut sizes: HashMap<String, u64> = HashMap::new(); | ||
| for chunk in chunks { | ||
| if sizes.contains_key(&chunk.file_path) { | ||
| continue; | ||
| } | ||
| if let Ok(text) = std::fs::read_to_string(root.join(&chunk.file_path)) { | ||
| sizes.insert(chunk.file_path.clone(), text.encode_utf16().count() as u64); | ||
| } | ||
| } | ||
| sizes | ||
| } |
There was a problem hiding this comment.
[HIGH] compute_file_sizes 함수의 경로 탐색(Path Traversal) 취약점 방지
Symptom: compute_file_sizes 함수에서 root.join(&chunk.file_path)를 사용하여 파일을 읽을 때, chunk.file_path가 상위 디렉토리 참조(..)나 절대 경로를 포함하고 있는지 검증하지 않고 그대로 파일 시스템에서 읽고 있습니다.
Source: OWASP — Path Traversal (Input Validation)
Consequence: 악의적으로 조작된 인덱스 파일(chunks.json)을 로드할 경우, 공격자가 지정한 임의의 시스템 파일(예: /etc/passwd)을 읽으려고 시도하는 경로 탐색(Path Traversal) 취약점이 발생할 수 있습니다.
Remedy: chunk.file_path가 절대 경로이거나 상위 디렉토리 참조(..)를 포함하는지 검증하여, 안전하지 않은 경로인 경우 파일 읽기를 건너뛰도록 제한합니다.
fn compute_file_sizes(root: &Path, chunks: &[Chunk]) -> HashMap<String, u64> {
let mut sizes: HashMap<String, u64> = HashMap::new();
for chunk in chunks {
if sizes.contains_key(&chunk.file_path) {
continue;
}
let path = Path::new(&chunk.file_path);
if path.is_absolute() || path.components().any(|c| matches!(c, std::path::Component::ParentDir | std::path::Component::RootDir)) {
continue;
}
if let Ok(text) = std::fs::read_to_string(root.join(path)) {
sizes.insert(chunk.file_path.clone(), text.encode_utf16().count() as u64);
}
}
sizes
}| fn delivered_chars(content: &str, max_snippet_lines: Option<usize>) -> u64 { | ||
| match max_snippet_lines { | ||
| None => utf16_len(content), | ||
| Some(0) => 0, | ||
| Some(n) => { | ||
| let snippet = content.lines().take(n).collect::<Vec<_>>().join("\n"); | ||
| utf16_len(&snippet) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[MEDIUM] delivered_chars 함수의 불필요한 메모리 할당 개선
Symptom: delivered_chars 함수에서 max_snippet_lines가 Some(n)일 때, 첫 n줄의 UTF-16 길이를 계산하기 위해 content.lines().take(n).collect::<Vec<_>>().join("\n")을 사용하여 새로운 Vec과 String을 매번 할당하고 있습니다.
Source: McConnell — Code Complete (Ch. 26: Code-Tuning Techniques)
Consequence: 검색 결과가 많거나 검색 빈도가 높을 때 불필요한 메모리 할당과 복사가 발생하여 성능 저하 및 가비지 컬렉션(또는 메모리 해제) 부하를 유발할 수 있습니다.
Remedy: Vec과 String을 할당하지 않고, 반복자를 통해 각 라인의 UTF-16 길이와 줄바꿈 문자 개수만 합산하도록 변경하여 메모리 할당을 제거합니다.
fn delivered_chars(content: &str, max_snippet_lines: Option<usize>) -> u64 {
match max_snippet_lines {
None => utf16_len(content),
Some(0) => 0,
Some(n) => {
let mut total_len = 0;
let mut count = 0;
for line in content.lines().take(n) {
total_len += utf16_len(line);
count += 1;
}
if count > 0 {
total_len += (count - 1) as u64;
}
total_len
}
}
}
|
Greptile SummaryThis PR wires the previously inert
Confidence Score: 4/5Safe to merge; the only concrete issue is that The core telemetry wiring, the
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI as csp CLI / MCP Tool
participant dispatch as dispatch_with_stats(stats_file)
participant search as search_output / find_related_output
participant index as CspIndex
participant stats as save_search_stats
participant file as savings.jsonl
User->>CLI: csp search "query"
CLI->>dispatch: "dispatch_with_stats(Search{...}, stats_file)"
dispatch->>index: load_index() → CspIndex (with file_sizes)
index-->>dispatch: "CspIndex {chunks, file_sizes}"
dispatch->>search: "search_output(&idx, query, top_k, max_lines, Some(stats_file))"
search->>index: index.search(query, options)
index-->>search: "Vec<SearchResult>"
search->>stats: save_search_stats(stats_file, results, Search, file_sizes, max_lines)
stats->>stats: compute snippet_chars via delivered_chars()
stats->>stats: dedup file paths → sum file_chars
stats->>file: append JSONL record
search-->>dispatch: JSON string
dispatch-->>User: stdout
Note over index: file_sizes populated at from_path() / from_git() / load_from_disk()
Note over stats: Best-effort: I/O errors are swallowed
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI as csp CLI / MCP Tool
participant dispatch as dispatch_with_stats(stats_file)
participant search as search_output / find_related_output
participant index as CspIndex
participant stats as save_search_stats
participant file as savings.jsonl
User->>CLI: csp search "query"
CLI->>dispatch: "dispatch_with_stats(Search{...}, stats_file)"
dispatch->>index: load_index() → CspIndex (with file_sizes)
index-->>dispatch: "CspIndex {chunks, file_sizes}"
dispatch->>search: "search_output(&idx, query, top_k, max_lines, Some(stats_file))"
search->>index: index.search(query, options)
index-->>search: "Vec<SearchResult>"
search->>stats: save_search_stats(stats_file, results, Search, file_sizes, max_lines)
stats->>stats: compute snippet_chars via delivered_chars()
stats->>stats: dedup file paths → sum file_chars
stats->>file: append JSONL record
search-->>dispatch: JSON string
dispatch-->>User: stdout
Note over index: file_sizes populated at from_path() / from_git() / load_from_disk()
Note over stats: Best-effort: I/O errors are swallowed
|
1 participant
Summary
save_search_statswas defined and unit-tested but never called in any code path — nothing recorded to~/.csp/savings.jsonl, socsp savingsalways reported zero. This wires it in at the CLI/MCP app boundary and applies the semble#206 snippet accounting deferred from #75.Closes #81.
What changed
CspIndex.file_sizes(repo-relative path → UTF-16 char count) captured at build time infrom_path, carried throughfrom_git's re-root, and recomputed inload_from_diskwhen the source root is a still-present local directory (mirrors semble reading sizes offroot). Empty when source files aren't available (e.g. a cached git index) →file_charsis simply 0.save_search_statstakesmax_snippet_linesso recordedsnippet_charsreflects what the caller actually received (semble#206):None→ full chunk,Some(0)→ 0,Some(n)→ first n lines.search_output/find_related_output(CLI) andsearch_tool/find_related_tool(MCP), each behind an injectedOption<&Path>stats file.CspIndex::searchstays a pure, side-effect-free library call — telemetry lives at the app boundary (issue savings telemetry is not wired into the search flow (blocks semble#206) #81 permits either).Testability / no pollution
Every recording site takes an injected
Option<&Path>. The MCP server holds an injectablestats_filefield (Nonein tests); CLI dispatch is split intodispatch_with_stats(command, stats_file). Tests redirect telemetry to a temp file or disable it, so the developer's real~/.csp/savings.jsonlis never touched.Testing
cargo fmt --all && cargo clippy --all-targets --all-features -- -D warnings && cargo test --workspace— 273 lib + 22 CLI tests pass (4 network-gated ignored).Stacking
Stacked on #75 (
amondnet/max-snippet-lines) — base will retarget tomainonce #75 merges.🤖 Generated with Claude Code
Summary by cubic
Record token-savings telemetry for
searchandfind-relatedin the CLI and MCP socsp savingsshows real data. Snippet accounting now respectsmax_snippet_lines.save_search_statsinto CLI (search_output,find_related_output) and MCP (search_tool,find_related_tool) behind an optional stats file; defaults to~/.csp/savings.jsonl.CspIndex.file_sizes(repo-relative → UTF-16 char count), computed infrom_path, preserved throughfrom_git, and recomputed inload_from_diskwhen the local root exists; empty when sources aren’t available.save_search_statsto acceptmax_snippet_linesand countsnippet_charsfor only the delivered lines (None = full, 0 = none, n = first n lines).CspIndex::searchside-effect-free; tests injectNoneor a temp file to avoid touching the real stats file. Fixes savings telemetry is not wired into the search flow (blocks semble#206) #81.Written for commit 306b1a2. Summary will update on new commits.