Skip to content

Commit a00f4f0

Browse files
haasonsaashaasonsaas
committed
refactor: split review context related helpers
Made-with: Cursor
1 parent 328b71f commit a00f4f0

5 files changed

Lines changed: 144 additions & 110 deletions

File tree

‎TODO.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@
9999
- [x] `src/review/pipeline/execution/responses/validation.rs`: separate schema validation from per-comment sanitization.
100100
- [x] `src/review/pipeline/prepare/runner.rs`: split per-diff orchestration, pre-analysis/triage decisions, and progress updates.
101101
- [x] `src/review/pipeline/context/symbols.rs`: split symbol search, snippet selection, and fallback behavior.
102-
- [ ] `src/review/pipeline/context/related.rs`: separate related-file discovery from ranking/selection.
102+
- [x] `src/review/pipeline/context/related.rs`: separate related-file discovery from ranking/selection.
103103
- [ ] `src/review/pipeline/guidance.rs`: carve guidance assembly, repo-support guidance, and prompt-facing formatting.
104104
- [ ] `src/review/pipeline/session.rs`: split session construction from runtime state transitions.
105105
- [ ] `src/review/pipeline/services.rs`: separate service wiring from optional feature initialization.

‎src/review/pipeline/context/related.rs‎

Lines changed: 8 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1,109 +1,8 @@
1-
use std::path::{Path, PathBuf};
2-
3-
use crate::core;
4-
5-
pub(in crate::review::pipeline) fn gather_related_file_context(
6-
index: &core::SymbolIndex,
7-
file_path: &Path,
8-
repo_path: &Path,
9-
) -> Vec<core::LLMContextChunk> {
10-
let mut chunks: Vec<core::LLMContextChunk> = Vec::new();
11-
12-
let callers = index.reverse_deps(file_path);
13-
for caller_path in callers.iter().take(3) {
14-
if let Some(summary) = index.file_summary(caller_path) {
15-
let truncated: String = if summary.len() > 2000 {
16-
let mut end = 2000;
17-
while end > 0 && !summary.is_char_boundary(end) {
18-
end -= 1;
19-
}
20-
format!("{}...[truncated]", &summary[..end])
21-
} else {
22-
summary.to_string()
23-
};
24-
chunks.push(
25-
core::LLMContextChunk::reference(
26-
caller_path.clone(),
27-
format!("[Caller/dependent file]\n{}", truncated),
28-
)
29-
.with_provenance(core::ContextProvenance::ReverseDependencySummary),
30-
);
31-
}
32-
}
33-
34-
let test_files = find_test_files(file_path, repo_path);
35-
for test_path in test_files.iter().take(2) {
36-
let relative: &Path = test_path.strip_prefix(repo_path).unwrap_or(test_path);
37-
if let Ok(content) = std::fs::read_to_string(test_path) {
38-
let snippet: String = content.lines().take(60).collect::<Vec<_>>().join("\n");
39-
if !snippet.is_empty() {
40-
chunks.push(
41-
core::LLMContextChunk::reference(
42-
relative.to_path_buf(),
43-
format!("[Test file]\n{}", snippet),
44-
)
45-
.with_provenance(core::ContextProvenance::RelatedTestFile),
46-
);
47-
}
48-
}
49-
}
50-
51-
chunks
52-
}
53-
54-
fn find_test_files(file_path: &Path, repo_path: &Path) -> Vec<PathBuf> {
55-
let stem = match file_path.file_stem().and_then(|s| s.to_str()) {
56-
Some(s) => s.to_string(),
57-
None => return Vec::new(),
58-
};
59-
let ext = file_path.extension().and_then(|e| e.to_str()).unwrap_or("");
60-
let parent = file_path.parent().unwrap_or(Path::new(""));
61-
62-
let candidates: Vec<PathBuf> = vec![
63-
repo_path
64-
.join(parent)
65-
.join(format!("test_{}.{}", stem, ext)),
66-
repo_path
67-
.join(parent)
68-
.join(format!("{}_test.{}", stem, ext)),
69-
repo_path
70-
.join(parent)
71-
.join(format!("{}.test.{}", stem, ext)),
72-
repo_path
73-
.join(parent)
74-
.join(format!("{}.spec.{}", stem, ext)),
75-
repo_path
76-
.join(parent)
77-
.join("tests")
78-
.join(format!("{}.{}", stem, ext)),
79-
repo_path
80-
.join(parent)
81-
.join("__tests__")
82-
.join(format!("{}.{}", stem, ext)),
83-
];
84-
85-
candidates
86-
.into_iter()
87-
.filter(|path: &PathBuf| path.is_file())
88-
.collect()
89-
}
90-
91-
#[cfg(test)]
92-
mod tests {
93-
#[test]
94-
fn test_utf8_safe_truncation() {
95-
let prefix = "a".repeat(1999);
96-
let value = format!("{}€rest", prefix);
97-
assert!(value.len() > 2000);
98-
99-
let mut end = 2000;
100-
while end > 0 && !value.is_char_boundary(end) {
101-
end -= 1;
102-
}
103-
let truncated = &value[..end];
104-
105-
assert_eq!(end, 1999);
106-
assert!(truncated.starts_with("aaa"));
107-
assert!(!truncated.contains('€'));
108-
}
109-
}
1+
#[path = "related/callers.rs"]
2+
mod callers;
3+
#[path = "related/run.rs"]
4+
mod run;
5+
#[path = "related/test_files.rs"]
6+
mod test_files;
7+
8+
pub(in crate::review::pipeline) use run::gather_related_file_context;

‎src/review/pipeline/context/related/callers.rs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
use std::path::Path;
2+
3+
use crate::core;
4+
5+
pub(super) fn build_caller_context_chunks(
6+
index: &core::SymbolIndex,
7+
file_path: &Path,
8+
) -> Vec<core::LLMContextChunk> {
9+
index
10+
.reverse_deps(file_path)
11+
.iter()
12+
.take(3)
13+
.filter_map(|caller_path| {
14+
let summary = index.file_summary(caller_path)?;
15+
Some(
16+
core::LLMContextChunk::reference(
17+
caller_path.clone(),
18+
format!("[Caller/dependent file]\n{}", truncate_summary(summary)),
19+
)
20+
.with_provenance(core::ContextProvenance::ReverseDependencySummary),
21+
)
22+
})
23+
.collect()
24+
}
25+
26+
fn truncate_summary(summary: &str) -> String {
27+
if summary.len() <= 2000 {
28+
return summary.to_string();
29+
}
30+
31+
let mut end = 2000;
32+
while end > 0 && !summary.is_char_boundary(end) {
33+
end -= 1;
34+
}
35+
format!("{}...[truncated]", &summary[..end])
36+
}
37+
38+
#[cfg(test)]
39+
mod tests {
40+
use super::truncate_summary;
41+
42+
#[test]
43+
fn test_utf8_safe_truncation() {
44+
let prefix = "a".repeat(1999);
45+
let value = format!("{}€rest", prefix);
46+
assert!(value.len() > 2000);
47+
48+
let truncated = truncate_summary(&value);
49+
assert!(truncated.starts_with("aaa"));
50+
assert!(!truncated.contains('€'));
51+
assert!(truncated.ends_with("...[truncated]"));
52+
}
53+
}

‎src/review/pipeline/context/related/run.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
use std::path::Path;
2+
3+
use crate::core;
4+
5+
use super::callers::build_caller_context_chunks;
6+
use super::test_files::build_related_test_chunks;
7+
8+
pub(in crate::review::pipeline) fn gather_related_file_context(
9+
index: &core::SymbolIndex,
10+
file_path: &Path,
11+
repo_path: &Path,
12+
) -> Vec<core::LLMContextChunk> {
13+
let mut chunks = build_caller_context_chunks(index, file_path);
14+
chunks.extend(build_related_test_chunks(file_path, repo_path));
15+
chunks
16+
}

‎src/review/pipeline/context/related/test_files.rs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
use std::path::{Path, PathBuf};
2+
3+
use crate::core;
4+
5+
pub(super) fn build_related_test_chunks(
6+
file_path: &Path,
7+
repo_path: &Path,
8+
) -> Vec<core::LLMContextChunk> {
9+
find_test_files(file_path, repo_path)
10+
.iter()
11+
.take(2)
12+
.filter_map(|test_path| build_test_chunk(test_path, repo_path))
13+
.collect()
14+
}
15+
16+
fn build_test_chunk(test_path: &Path, repo_path: &Path) -> Option<core::LLMContextChunk> {
17+
let relative = test_path.strip_prefix(repo_path).unwrap_or(test_path);
18+
let content = std::fs::read_to_string(test_path).ok()?;
19+
let snippet = content.lines().take(60).collect::<Vec<_>>().join("\n");
20+
if snippet.is_empty() {
21+
return None;
22+
}
23+
24+
Some(
25+
core::LLMContextChunk::reference(
26+
relative.to_path_buf(),
27+
format!("[Test file]\n{}", snippet),
28+
)
29+
.with_provenance(core::ContextProvenance::RelatedTestFile),
30+
)
31+
}
32+
33+
fn find_test_files(file_path: &Path, repo_path: &Path) -> Vec<PathBuf> {
34+
let stem = match file_path.file_stem().and_then(|s| s.to_str()) {
35+
Some(s) => s.to_string(),
36+
None => return Vec::new(),
37+
};
38+
let ext = file_path.extension().and_then(|e| e.to_str()).unwrap_or("");
39+
let parent = file_path.parent().unwrap_or(Path::new(""));
40+
41+
vec![
42+
repo_path
43+
.join(parent)
44+
.join(format!("test_{}.{}", stem, ext)),
45+
repo_path
46+
.join(parent)
47+
.join(format!("{}_test.{}", stem, ext)),
48+
repo_path
49+
.join(parent)
50+
.join(format!("{}.test.{}", stem, ext)),
51+
repo_path
52+
.join(parent)
53+
.join(format!("{}.spec.{}", stem, ext)),
54+
repo_path
55+
.join(parent)
56+
.join("tests")
57+
.join(format!("{}.{}", stem, ext)),
58+
repo_path
59+
.join(parent)
60+
.join("__tests__")
61+
.join(format!("{}.{}", stem, ext)),
62+
]
63+
.into_iter()
64+
.filter(|path| path.is_file())
65+
.collect()
66+
}

0 commit comments

Comments
 (0)