Skip to content

Commit eac1a0d

Browse files
haasonsaashaasonsaas
committed
refactor: split review session helpers
Made-with: Cursor
1 parent cdd6cae commit eac1a0d

File tree

4 files changed

+179
-118
lines changed

4 files changed

+179
-118
lines changed

‎TODO.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@
101101
- [x] `src/review/pipeline/context/symbols.rs`: split symbol search, snippet selection, and fallback behavior.
102102
- [x] `src/review/pipeline/context/related.rs`: separate related-file discovery from ranking/selection.
103103
- [x] `src/review/pipeline/guidance.rs`: carve guidance assembly, repo-support guidance, and prompt-facing formatting.
104-
- [ ] `src/review/pipeline/session.rs`: split session construction from runtime state transitions.
104+
- [x] `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.
106106
- [ ] `src/review/pipeline/file_context/sources.rs`: split repo sources, symbol sources, and supplemental context sources.
107107
- [ ] `src/review/pipeline/comments.rs`: separate comment assembly, filtering, and metadata stamping.

‎src/review/pipeline/session.rs‎

Lines changed: 7 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1+
#[path = "session/build.rs"]
2+
mod build;
3+
#[path = "session/semantic.rs"]
4+
mod semantic;
5+
16
use anyhow::Result;
27
use std::collections::HashMap;
38
use std::path::PathBuf;
4-
use tracing::{info, warn};
59

610
use crate::core;
711

8-
use super::context::build_symbol_index;
9-
use super::repo_support::{detect_instruction_files, gather_git_log};
1012
use super::services::PipelineServices;
1113
use super::types::ProgressCallback;
14+
use build::build_review_session;
1215

1316
pub(super) struct ReviewSession {
1417
pub diffs: Vec<core::UnifiedDiff>,
@@ -30,119 +33,6 @@ impl ReviewSession {
3033
services: &PipelineServices,
3134
on_progress: Option<ProgressCallback>,
3235
) -> Result<Self> {
33-
let diffs = core::DiffParser::parse_unified_diff(diff_content)?;
34-
info!("Parsed {} file diffs", diffs.len());
35-
36-
if let Some(limit) = services.config.file_change_limit {
37-
if limit > 0 && diffs.len() > limit {
38-
anyhow::bail!(
39-
"Diff contains {} files, exceeding file_change_limit of {}. \
40-
Increase the limit or split the review.",
41-
diffs.len(),
42-
limit
43-
);
44-
}
45-
}
46-
47-
let source_files: HashMap<PathBuf, String> = diffs
48-
.iter()
49-
.filter_map(|diff| {
50-
std::fs::read_to_string(services.repo_path.join(&diff.file_path))
51-
.ok()
52-
.map(|content| (diff.file_path.clone(), content))
53-
})
54-
.collect();
55-
56-
let git_log_output = gather_git_log(&services.repo_path);
57-
let convention_json = services
58-
.convention_store_path
59-
.as_ref()
60-
.and_then(|path| std::fs::read_to_string(path).ok());
61-
62-
let enhanced_ctx = core::build_enhanced_context(
63-
&diffs,
64-
&source_files,
65-
git_log_output.as_deref(),
66-
None,
67-
convention_json.as_deref(),
68-
None,
69-
);
70-
let enhanced_guidance = core::generate_enhanced_guidance(&enhanced_ctx, "rs");
71-
if !enhanced_guidance.is_empty() {
72-
info!(
73-
"Enhanced guidance generated ({} chars)",
74-
enhanced_guidance.len()
75-
);
76-
}
77-
78-
let auto_instructions = if services.config.auto_detect_instructions
79-
&& services.config.review_instructions.is_none()
80-
{
81-
let detected = detect_instruction_files(&services.repo_path);
82-
if detected.is_empty() {
83-
None
84-
} else {
85-
Some(
86-
detected
87-
.iter()
88-
.map(|(name, content)| format!("# From {}\n{}", name, content))
89-
.collect::<Vec<_>>()
90-
.join("\n\n"),
91-
)
92-
}
93-
} else {
94-
None
95-
};
96-
97-
let symbol_index = build_symbol_index(&services.config, &services.repo_path);
98-
99-
let semantic_index = if services.config.semantic_rag {
100-
let index_path = core::default_index_path(&services.repo_path);
101-
let changed_files = diffs
102-
.iter()
103-
.map(|diff| diff.file_path.clone())
104-
.collect::<Vec<_>>();
105-
match core::refresh_semantic_index(
106-
&services.repo_path,
107-
&index_path,
108-
services.embedding_adapter.as_deref(),
109-
&changed_files,
110-
|path| services.config.should_exclude(path),
111-
services.config.semantic_rag_max_files,
112-
)
113-
.await
114-
{
115-
Ok(index) => Some(index),
116-
Err(error) => {
117-
warn!("Semantic index refresh failed: {}", error);
118-
None
119-
}
120-
}
121-
} else {
122-
None
123-
};
124-
125-
let semantic_feedback_store = if services.config.semantic_feedback {
126-
let path = core::default_semantic_feedback_path(&services.config.feedback_path);
127-
let mut store = core::load_semantic_feedback_store(&path);
128-
core::align_semantic_feedback_store(&mut store, services.embedding_adapter.as_deref());
129-
Some(store)
130-
} else {
131-
None
132-
};
133-
134-
Ok(Self {
135-
files_total: diffs.len(),
136-
diffs,
137-
source_files,
138-
on_progress,
139-
enhanced_ctx,
140-
enhanced_guidance,
141-
auto_instructions,
142-
symbol_index,
143-
semantic_index,
144-
semantic_feedback_store,
145-
verification_context: HashMap::new(),
146-
})
36+
build_review_session(diff_content, services, on_progress).await
14737
}
14838
}

‎src/review/pipeline/session/build.rs‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
use anyhow::Result;
2+
use std::collections::HashMap;
3+
use std::path::PathBuf;
4+
use tracing::info;
5+
6+
use crate::core;
7+
8+
use super::super::context::build_symbol_index;
9+
use super::super::repo_support::{detect_instruction_files, gather_git_log};
10+
use super::super::services::PipelineServices;
11+
use super::super::types::ProgressCallback;
12+
use super::semantic::{build_semantic_index, load_semantic_feedback_store};
13+
use super::ReviewSession;
14+
15+
pub(super) async fn build_review_session(
16+
diff_content: &str,
17+
services: &PipelineServices,
18+
on_progress: Option<ProgressCallback>,
19+
) -> Result<ReviewSession> {
20+
let diffs = parse_review_diffs(diff_content, services)?;
21+
let source_files = load_source_files(&diffs, services);
22+
23+
let enhanced_ctx = build_enhanced_context(&diffs, &source_files, services);
24+
let enhanced_guidance = core::generate_enhanced_guidance(&enhanced_ctx, "rs");
25+
if !enhanced_guidance.is_empty() {
26+
info!(
27+
"Enhanced guidance generated ({} chars)",
28+
enhanced_guidance.len()
29+
);
30+
}
31+
32+
Ok(ReviewSession {
33+
files_total: diffs.len(),
34+
symbol_index: build_symbol_index(&services.config, &services.repo_path),
35+
semantic_index: build_semantic_index(&diffs, services).await,
36+
semantic_feedback_store: load_semantic_feedback_store(services),
37+
auto_instructions: detect_auto_instructions(services),
38+
diffs,
39+
source_files,
40+
on_progress,
41+
enhanced_ctx,
42+
enhanced_guidance,
43+
verification_context: HashMap::new(),
44+
})
45+
}
46+
47+
fn parse_review_diffs(
48+
diff_content: &str,
49+
services: &PipelineServices,
50+
) -> Result<Vec<core::UnifiedDiff>> {
51+
let diffs = core::DiffParser::parse_unified_diff(diff_content)?;
52+
info!("Parsed {} file diffs", diffs.len());
53+
54+
if let Some(limit) = services.config.file_change_limit {
55+
if limit > 0 && diffs.len() > limit {
56+
anyhow::bail!(
57+
"Diff contains {} files, exceeding file_change_limit of {}. \
58+
Increase the limit or split the review.",
59+
diffs.len(),
60+
limit
61+
);
62+
}
63+
}
64+
65+
Ok(diffs)
66+
}
67+
68+
fn load_source_files(
69+
diffs: &[core::UnifiedDiff],
70+
services: &PipelineServices,
71+
) -> HashMap<PathBuf, String> {
72+
diffs
73+
.iter()
74+
.filter_map(|diff| {
75+
std::fs::read_to_string(services.repo_path.join(&diff.file_path))
76+
.ok()
77+
.map(|content| (diff.file_path.clone(), content))
78+
})
79+
.collect()
80+
}
81+
82+
fn build_enhanced_context(
83+
diffs: &[core::UnifiedDiff],
84+
source_files: &HashMap<PathBuf, String>,
85+
services: &PipelineServices,
86+
) -> crate::core::enhanced_review::EnhancedReviewContext {
87+
let git_log_output = gather_git_log(&services.repo_path);
88+
let convention_json = services
89+
.convention_store_path
90+
.as_ref()
91+
.and_then(|path| std::fs::read_to_string(path).ok());
92+
93+
core::build_enhanced_context(
94+
diffs,
95+
source_files,
96+
git_log_output.as_deref(),
97+
None,
98+
convention_json.as_deref(),
99+
None,
100+
)
101+
}
102+
103+
fn detect_auto_instructions(services: &PipelineServices) -> Option<String> {
104+
if !(services.config.auto_detect_instructions && services.config.review_instructions.is_none())
105+
{
106+
return None;
107+
}
108+
109+
let detected = detect_instruction_files(&services.repo_path);
110+
if detected.is_empty() {
111+
None
112+
} else {
113+
Some(
114+
detected
115+
.iter()
116+
.map(|(name, content)| format!("# From {}\n{}", name, content))
117+
.collect::<Vec<_>>()
118+
.join("\n\n"),
119+
)
120+
}
121+
}

‎src/review/pipeline/session/semantic.rs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
use tracing::warn;
2+
3+
use crate::core;
4+
5+
use super::super::services::PipelineServices;
6+
7+
pub(super) async fn build_semantic_index(
8+
diffs: &[core::UnifiedDiff],
9+
services: &PipelineServices,
10+
) -> Option<core::semantic::SemanticIndex> {
11+
if !services.config.semantic_rag {
12+
return None;
13+
}
14+
15+
let index_path = core::default_index_path(&services.repo_path);
16+
let changed_files = diffs
17+
.iter()
18+
.map(|diff| diff.file_path.clone())
19+
.collect::<Vec<_>>();
20+
21+
match core::refresh_semantic_index(
22+
&services.repo_path,
23+
&index_path,
24+
services.embedding_adapter.as_deref(),
25+
&changed_files,
26+
|path| services.config.should_exclude(path),
27+
services.config.semantic_rag_max_files,
28+
)
29+
.await
30+
{
31+
Ok(index) => Some(index),
32+
Err(error) => {
33+
warn!("Semantic index refresh failed: {}", error);
34+
None
35+
}
36+
}
37+
}
38+
39+
pub(super) fn load_semantic_feedback_store(
40+
services: &PipelineServices,
41+
) -> Option<core::SemanticFeedbackStore> {
42+
if !services.config.semantic_feedback {
43+
return None;
44+
}
45+
46+
let path = core::default_semantic_feedback_path(&services.config.feedback_path);
47+
let mut store = core::load_semantic_feedback_store(&path);
48+
core::align_semantic_feedback_store(&mut store, services.embedding_adapter.as_deref());
49+
Some(store)
50+
}

0 commit comments

Comments
 (0)