Skip to content

Commit 187ede2

Browse files
committed
test: more TDD coverage — parsing, guidance, triage, config
Parsing (llm_response): - parse_json_single_quoted_object_wrapped_in_findings - parse_json_single_quoted_value_with_escaped_apostrophe - parse_json_double_quoted_value_with_apostrophe_unchanged - convert_single_quoted_json_to_double: fix \' in single-quoted string (emit apostrophe in JSON string, not backslash-doublequote) Guidance: - build_review_guidance_no_prose_section_when_rules_none/empty - build_review_guidance_single_prose_rule - build_review_guidance_prose_rule_with_special_chars Triage: - test_triage_options_default_skip_deletion_only_false - test_reason_skip_deletion_only_exact Config: - test_config_default_triage_skip_deletion_only_false - test_config_deserialize_triage_skip_deletion_only_false Made-with: Cursor
1 parent e3b2b84 commit 187ede2

4 files changed

Lines changed: 139 additions & 11 deletions

File tree

src/config.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2184,6 +2184,25 @@ triage_skip_deletion_only: true
21842184
assert!(config.triage_skip_deletion_only);
21852185
}
21862186

2187+
#[test]
2188+
fn test_config_default_triage_skip_deletion_only_false() {
2189+
let config = Config::default();
2190+
assert!(
2191+
!config.triage_skip_deletion_only,
2192+
"default: deletions get review unless explicitly enabled"
2193+
);
2194+
}
2195+
2196+
#[test]
2197+
fn test_config_deserialize_triage_skip_deletion_only_false() {
2198+
let yaml = r#"
2199+
model: claude-sonnet-4-6
2200+
triage_skip_deletion_only: false
2201+
"#;
2202+
let config: Config = serde_yaml::from_str(yaml).unwrap();
2203+
assert!(!config.triage_skip_deletion_only);
2204+
}
2205+
21872206
#[test]
21882207
fn test_default_frontier_role_models_match_requested_pair() {
21892208
let config = Config::default();

src/parsing/llm_response.rs

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -436,28 +436,32 @@ fn convert_single_quoted_json_to_double(s: &str) -> String {
436436
}
437437
if c == '\'' {
438438
// Start of single-quoted string: emit " and copy until unescaped ', escaping " and \.
439+
// Inside single-quoted: \' → one quote in JSON (emit \"); \\ → emit \\; \" → emit \"
439440
out.push('"');
441+
let mut single_escape = false;
440442
for c in chars.by_ref() {
441-
if c == '\\' {
442-
escape_next = true;
443-
out.push(c);
444-
} else if c == '\'' {
445-
if escape_next {
446-
escape_next = false;
447-
out.push('\'');
443+
if single_escape {
444+
single_escape = false;
445+
if c == '\'' {
446+
out.push('\''); // apostrophe in JSON string needs no escape
448447
} else {
449-
out.push('"');
450-
break;
448+
out.push('\\');
449+
out.push(c);
451450
}
451+
} else if c == '\\' {
452+
single_escape = true;
453+
} else if c == '\'' {
454+
out.push('"');
455+
break;
452456
} else if c == '"' {
453457
out.push('\\');
454458
out.push('"');
455459
} else {
456460
out.push(c);
457461
}
458462
}
459-
if escape_next {
460-
escape_next = false;
463+
if single_escape {
464+
out.push('\\');
461465
}
462466
continue;
463467
}
@@ -1349,6 +1353,43 @@ let data = &input;
13491353
assert!(comments[0].content.contains("deprecated"));
13501354
}
13511355

1356+
#[test]
1357+
fn parse_json_single_quoted_object_wrapped_in_findings() {
1358+
// Outer object with "findings" key; inner array uses single quotes — raw bracket span + repair.
1359+
let input = r#"{"findings": [{'line': 2, 'issue': 'Minor bug'}]}"#;
1360+
let file_path = PathBuf::from("src/lib.rs");
1361+
let comments = parse_llm_response(input, &file_path).unwrap();
1362+
assert_eq!(comments.len(), 1);
1363+
assert_eq!(comments[0].line_number, 2);
1364+
assert!(comments[0].content.contains("Minor bug"));
1365+
}
1366+
1367+
#[test]
1368+
fn parse_json_single_quoted_value_with_escaped_apostrophe() {
1369+
// Single-quoted value containing escaped apostrophe (e.g. "don't") — converter preserves it.
1370+
let input = r#"[{'line': 1, 'issue': 'don\'t forget'}]"#;
1371+
let file_path = PathBuf::from("src/lib.rs");
1372+
let comments = parse_llm_response(input, &file_path).unwrap();
1373+
assert_eq!(comments.len(), 1);
1374+
assert_eq!(comments[0].line_number, 1);
1375+
assert!(
1376+
comments[0].content.contains("don't"),
1377+
"content should contain apostrophe: {:?}",
1378+
comments[0].content
1379+
);
1380+
}
1381+
1382+
#[test]
1383+
fn parse_json_double_quoted_value_with_apostrophe_unchanged() {
1384+
// Valid JSON with apostrophe in double-quoted string — no repair; parses as-is.
1385+
let input = r#"[{"line": 3, "issue": "don't use deprecated API"}]"#;
1386+
let file_path = PathBuf::from("src/lib.rs");
1387+
let comments = parse_llm_response(input, &file_path).unwrap();
1388+
assert_eq!(comments.len(), 1);
1389+
assert_eq!(comments[0].line_number, 3);
1390+
assert!(comments[0].content.contains("don't"));
1391+
}
1392+
13521393
// ── Bug: find_json_array uses mismatched brackets ──────────────────
13531394
//
13541395
// `find_json_array` uses `find('[')` (first) + `rfind(']')` (last).

src/review/pipeline/guidance.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,53 @@ mod tests {
115115
assert!(guidance.contains("Always use parameterized queries"));
116116
assert!(guidance.contains("No direct SQL string concatenation"));
117117
}
118+
119+
#[test]
120+
fn build_review_guidance_no_prose_section_when_rules_none() {
121+
let config = config::Config {
122+
review_rules_prose: None,
123+
..config::Config::default()
124+
};
125+
let guidance = build_review_guidance(&config, None).unwrap();
126+
assert!(
127+
!guidance.contains("Custom rules (natural language)"),
128+
"guidance should not include prose section when review_rules_prose is None"
129+
);
130+
}
131+
132+
#[test]
133+
fn build_review_guidance_no_prose_section_when_rules_empty() {
134+
let config = config::Config {
135+
review_rules_prose: Some(vec![]),
136+
..config::Config::default()
137+
};
138+
let guidance = build_review_guidance(&config, None).unwrap();
139+
assert!(
140+
!guidance.contains("Custom rules (natural language)"),
141+
"guidance should not include prose section when review_rules_prose is empty"
142+
);
143+
}
144+
145+
#[test]
146+
fn build_review_guidance_single_prose_rule() {
147+
let config = config::Config {
148+
review_rules_prose: Some(vec!["Single rule.".to_string()]),
149+
..config::Config::default()
150+
};
151+
let guidance = build_review_guidance(&config, None).unwrap();
152+
assert!(guidance.contains("Custom rules (natural language)"));
153+
assert!(guidance.contains("Single rule."));
154+
}
155+
156+
#[test]
157+
fn build_review_guidance_prose_rule_with_special_chars() {
158+
let config = config::Config {
159+
review_rules_prose: Some(vec!["Check for <script> injection in HTML.".to_string()]),
160+
..config::Config::default()
161+
};
162+
let guidance = build_review_guidance(&config, None).unwrap();
163+
assert!(guidance.contains("Custom rules (natural language)"));
164+
assert!(guidance.contains("<script>"));
165+
assert!(guidance.contains("injection"));
166+
}
118167
}

src/review/triage.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,25 @@ mod tests {
695695
);
696696
}
697697

698+
// ── TriageOptions and reason strings ───────────────────────────────────
699+
700+
#[test]
701+
fn test_triage_options_default_skip_deletion_only_false() {
702+
let options = TriageOptions::default();
703+
assert!(
704+
!options.skip_deletion_only,
705+
"default should not skip deletion-only so deletions still get review"
706+
);
707+
}
708+
709+
#[test]
710+
fn test_reason_skip_deletion_only_exact() {
711+
assert_eq!(
712+
TriageResult::SkipDeletionOnly.reason(),
713+
"deletion-only changes"
714+
);
715+
}
716+
698717
// ── #29 optional skip deletion-only ──────────────────────────────────
699718

700719
#[test]

0 commit comments

Comments
 (0)