Skip to content

Commit 29c8282

Browse files
committed
feat(llm): add negative examples and subject specificity validator
- System prompt: BAD/GOOD example pairs for subject quality guidance - CommitValidator rule 6: generic verb+noun detection (e.g. "update code", "improve things") triggers retry with specificity instruction - is_generic_subject() checks subjects <=4 words for vague patterns
1 parent 4bab02c commit 29c8282

2 files changed

Lines changed: 161 additions & 8 deletions

File tree

src/services/llm/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,19 @@ Rules:
2020
- Subject: imperative, specific, lowercase start, no trailing period, max 72 chars total first line.
2121
- Body: 1-3 sentences about WHY for non-trivial changes, else null.
2222
- Do not list files changed.
23+
- If the change is purely syntactic (collapsing nested blocks, reformatting, reordering imports) with identical behavior, use "style" — never describe it as fixing a bug or adding a feature.
24+
- The SUGGESTED TYPE is a hint. Override it if the diff clearly shows a different type.
25+
- Never copy labels, field names, or evidence tags from the prompt into your output. The breaking_change value must describe the actual change in plain English.
26+
- If public APIs are both added and removed, this is an API replacement (refactor), not a new feature.
27+
28+
Examples:
29+
GOOD: "add evidence-based commit validation with retry" — one specific thing
30+
GOOD: "replace path-only grouping with diff-shape fingerprinting" — names the concrete change
31+
BAD: "update code and improve things" — too vague
32+
BAD: "refactor code for better performance and add validation" — two concerns in one subject
33+
34+
1. Diff adds `pub fn new_api()` and removes `pub fn old_api()` → type: "refactor", breaking_change: "removed `old_api()`, use `new_api()` instead"
35+
2. Diff only changes whitespace/indentation → type: "style", body: null, breaking_change: null
2336
2437
Output ONLY valid JSON (nullable fields use null, not the string "null"):
2538
{"type":"<type>","scope":null,"subject":"<subject>","body":null,"breaking_change":null}

src/services/sanitizer.rs

Lines changed: 148 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -288,14 +288,14 @@ impl CommitSanitizer {
288288
}
289289

290290
// Apply lowercase to subject if enabled (for plain text, lowercase after the colon)
291-
if format.lowercase_subject {
292-
if let Some(colon_pos) = cleaned.find(": ") {
293-
let (prefix, rest) = cleaned.split_at(colon_pos + 2);
294-
let mut chars = rest.chars();
295-
if let Some(first) = chars.next() {
296-
let lowered: String = first.to_lowercase().chain(chars).collect();
297-
cleaned = format!("{}{}", prefix, lowered);
298-
}
291+
if format.lowercase_subject
292+
&& let Some(colon_pos) = cleaned.find(": ")
293+
{
294+
let (prefix, rest) = cleaned.split_at(colon_pos + 2);
295+
let mut chars = rest.chars();
296+
if let Some(first) = chars.next() {
297+
let lowered: String = first.to_lowercase().chain(chars).collect();
298+
cleaned = format!("{}{}", prefix, lowered);
299299
}
300300
}
301301

@@ -332,4 +332,144 @@ impl CommitSanitizer {
332332

333333
Ok(())
334334
}
335+
336+
/// Try to parse raw LLM output as structured JSON without sanitizing.
337+
/// Used by the post-generation validator to inspect the LLM's raw intent.
338+
pub fn parse_structured(raw: &str) -> Option<StructuredCommit> {
339+
Self::try_parse_json(raw).ok()
340+
}
341+
}
342+
343+
/// Post-generation evidence-based validation.
344+
///
345+
/// Checks if a structured commit message is consistent with the evidence flags
346+
/// computed from code analysis. Returns a list of violations that can be used
347+
/// to construct a corrective re-prompt.
348+
pub struct CommitValidator;
349+
350+
impl CommitValidator {
351+
/// Validate a structured commit against evidence flags.
352+
/// Returns violations as human-readable correction instructions.
353+
pub fn validate(
354+
commit: &StructuredCommit,
355+
has_bug_evidence: bool,
356+
is_mechanical: bool,
357+
public_api_removed_count: usize,
358+
is_dependency_only: bool,
359+
) -> Vec<String> {
360+
let mut violations = Vec::new();
361+
let commit_type = commit.commit_type.to_lowercase();
362+
363+
// Rule 1: type=fix requires bug evidence
364+
if commit_type == "fix" && !has_bug_evidence {
365+
violations.push(
366+
"Type is \"fix\" but no bug-fix comments were found in the diff. \
367+
Use \"refactor\" instead."
368+
.to_string(),
369+
);
370+
}
371+
372+
// Rule 2: breaking_change must be set when public API removed
373+
if commit.breaking_change.is_none() && public_api_removed_count > 0 {
374+
violations.push(
375+
"Public APIs were removed but breaking_change is null. \
376+
Describe what was removed in plain English."
377+
.to_string(),
378+
);
379+
}
380+
381+
// Rule 3: breaking_change must not copy internal field names
382+
if let Some(ref bc) = commit.breaking_change {
383+
let lower = bc.to_lowercase();
384+
if lower.contains("public_api_removed")
385+
|| lower.contains("bug_evidence")
386+
|| lower.contains("mechanical_transform")
387+
|| lower.contains("dependency_only")
388+
{
389+
violations.push(
390+
"The breaking_change field contains internal label names. \
391+
Describe the actual API change in plain English."
392+
.to_string(),
393+
);
394+
}
395+
}
396+
397+
// Rule 4: mechanical transform cannot be feat or fix
398+
if is_mechanical && matches!(commit_type.as_str(), "feat" | "fix") {
399+
violations.push(
400+
"Change is a mechanical/formatting transform but type is \"feat\"/\"fix\". \
401+
Use \"style\" or \"refactor\"."
402+
.to_string(),
403+
);
404+
}
405+
406+
// Rule 5: dependency-only should be chore
407+
if is_dependency_only && commit_type != "chore" {
408+
violations.push(
409+
"All changes are in dependency/config files but type is not \"chore\".".to_string(),
410+
);
411+
}
412+
413+
// Rule 6: subject too generic (vague verb + vague noun)
414+
if Self::is_generic_subject(&commit.subject) {
415+
violations.push(
416+
"Subject is too generic — name the specific API, function, or module changed. \
417+
Avoid vague verbs like \"update\", \"improve\", \"change\"."
418+
.to_string(),
419+
);
420+
}
421+
422+
violations
423+
}
424+
425+
/// Check if a subject is too generic (vague verb + vague noun without specifics).
426+
///
427+
/// Flags subjects like "update code", "improve things", "change functionality"
428+
/// but allows "update dependency versions", "improve error messages in validator".
429+
fn is_generic_subject(subject: &str) -> bool {
430+
const GENERIC_VERBS: &[&str] = &["update", "improve", "change", "modify", "enhance"];
431+
const GENERIC_NOUNS: &[&str] = &[
432+
"code",
433+
"things",
434+
"stuff",
435+
"functionality",
436+
"logic",
437+
"implementation",
438+
"behavior",
439+
"performance",
440+
"handling",
441+
"processing",
442+
];
443+
444+
let words: Vec<&str> = subject.split_whitespace().collect();
445+
if words.len() > 4 {
446+
// Longer subjects are usually specific enough
447+
return false;
448+
}
449+
450+
let lower: Vec<String> = words.iter().map(|w| w.to_lowercase()).collect();
451+
452+
// Check if starts with generic verb and contains a generic noun
453+
if let Some(first) = lower.first()
454+
&& GENERIC_VERBS.contains(&first.as_str())
455+
{
456+
return lower[1..]
457+
.iter()
458+
.any(|w| GENERIC_NOUNS.contains(&w.as_str()));
459+
}
460+
461+
false
462+
}
463+
464+
/// Format violations as a CORRECTIONS section for a retry prompt.
465+
pub fn format_corrections(violations: &[String]) -> String {
466+
let mut section =
467+
String::from("\nCORRECTIONS (your previous output had these errors — fix them):\n");
468+
for v in violations {
469+
section.push_str("- ");
470+
section.push_str(v);
471+
section.push('\n');
472+
}
473+
section
474+
}
335475
}

0 commit comments

Comments
 (0)