Skip to content

Commit e5b2b82

Browse files
haasonsaasclaude
andcommitted
fix: address Devin review feedback — triage false positives and embedding default
- `#` comment prefix: exclude Rust attributes (#[...], #![...]), C preprocessor directives (#include, #define, etc.) from comment detection - `generated/` match: use boundary-aware checks (starts_with or /generated/) to avoid false positives on paths like `user_generated/` - Embedding model default: fall back to primary model instead of hardcoding OpenAI's text-embedding-3-small, respecting self-hosted provider support - 4 new tests for the triage fixes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a05f873 commit e5b2b82

File tree

2 files changed

+68
-8
lines changed

2 files changed

+68
-8
lines changed

src/config.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -913,10 +913,7 @@ impl Config {
913913
ModelRole::Primary => &self.model,
914914
ModelRole::Weak => self.model_weak.as_deref().unwrap_or(&self.model),
915915
ModelRole::Reasoning => self.model_reasoning.as_deref().unwrap_or(&self.model),
916-
ModelRole::Embedding => self
917-
.model_embedding
918-
.as_deref()
919-
.unwrap_or("text-embedding-3-small"),
916+
ModelRole::Embedding => self.model_embedding.as_deref().unwrap_or(&self.model),
920917
}
921918
}
922919

@@ -1633,9 +1630,10 @@ mod tests {
16331630
model_embedding: None,
16341631
..Config::default()
16351632
};
1633+
// Falls back to primary model when no embedding model configured
16361634
assert_eq!(
16371635
config.model_for_role(ModelRole::Embedding),
1638-
"text-embedding-3-small"
1636+
"claude-sonnet-4-6"
16391637
);
16401638
}
16411639

src/review/triage.rs

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,13 @@ const LOCK_FILES: &[&str] = &[
4141
];
4242

4343
/// Comment line prefixes (after trimming leading whitespace).
44-
const COMMENT_PREFIXES: &[&str] = &["//", "#", "/*", "*/", "* ", "--", "<!--", "\"\"\"", "'''"];
44+
const COMMENT_PREFIXES: &[&str] = &["//", "# ", "/*", "*/", "* ", "--", "<!--", "\"\"\"", "'''"];
45+
46+
/// Patterns that start with `#` but are NOT comments (Rust attributes, C preprocessor, etc.).
47+
const HASH_NON_COMMENT_PREFIXES: &[&str] = &[
48+
"#[", "#!", "#include", "#define", "#ifdef", "#ifndef", "#endif", "#pragma", "#undef", "#elif",
49+
"#else", "#if ", "#error", "#warning", "#line",
50+
];
4551

4652
/// Classify a diff using fast heuristics (no LLM call).
4753
///
@@ -108,8 +114,10 @@ fn is_generated_file(path: &str) -> bool {
108114
// Path contains marker segments
109115
if path.contains(".generated.")
110116
|| path.contains(".g.")
111-
|| path.contains("_generated/")
112-
|| path.contains("generated/")
117+
|| path.starts_with("_generated/")
118+
|| path.contains("/_generated/")
119+
|| path.starts_with("generated/")
120+
|| path.contains("/generated/")
113121
{
114122
return true;
115123
}
@@ -171,6 +179,14 @@ fn is_comment_line(content: &str) -> bool {
171179
// Blank lines in a comment-only change are fine
172180
return true;
173181
}
182+
// Handle `#` lines: exclude Rust attributes, C preprocessor, etc.
183+
if trimmed.starts_with('#')
184+
&& HASH_NON_COMMENT_PREFIXES
185+
.iter()
186+
.any(|p| trimmed.starts_with(p))
187+
{
188+
return false;
189+
}
174190
COMMENT_PREFIXES
175191
.iter()
176192
.any(|prefix| trimmed.starts_with(prefix))
@@ -638,6 +654,52 @@ mod tests {
638654
assert_eq!(triage_diff(&diff), TriageResult::NeedsReview);
639655
}
640656

657+
#[test]
658+
fn test_comment_only_does_not_match_rust_attributes() {
659+
let diff = make_diff(
660+
"src/lib.rs",
661+
vec![
662+
make_line(1, ChangeType::Added, "#[derive(Debug, Clone, Serialize)]"),
663+
make_line(
664+
2,
665+
ChangeType::Added,
666+
"#[serde(rename_all = \"snake_case\")]",
667+
),
668+
],
669+
);
670+
assert_eq!(triage_diff(&diff), TriageResult::NeedsReview);
671+
}
672+
673+
#[test]
674+
fn test_comment_only_does_not_match_c_preprocessor() {
675+
let diff = make_diff(
676+
"src/main.c",
677+
vec![
678+
make_line(1, ChangeType::Added, "#include <stdio.h>"),
679+
make_line(2, ChangeType::Added, "#define MAX_SIZE 100"),
680+
],
681+
);
682+
assert_eq!(triage_diff(&diff), TriageResult::NeedsReview);
683+
}
684+
685+
#[test]
686+
fn test_comment_only_does_not_match_rust_inner_attributes() {
687+
let diff = make_diff(
688+
"src/lib.rs",
689+
vec![make_line(1, ChangeType::Added, "#![allow(unused)]")],
690+
);
691+
assert_eq!(triage_diff(&diff), TriageResult::NeedsReview);
692+
}
693+
694+
#[test]
695+
fn test_generated_does_not_match_user_generated_dir() {
696+
let diff = make_diff(
697+
"src/user_generated/profile.rs",
698+
vec![make_line(1, ChangeType::Added, "pub struct Profile {}")],
699+
);
700+
assert_eq!(triage_diff(&diff), TriageResult::NeedsReview);
701+
}
702+
641703
// ── Edge cases ───────────────────────────────────────────────────────
642704

643705
#[test]

0 commit comments

Comments
 (0)