review classifier: truncate an over-long reason instead of dropping the verdict
An answer whose reason ran past 500 characters was rejected whole, so the verdict was lost and the one error row turned the review into no_verdict. The reason is cut to the cap, ending in an ellipsis; verdict and certainty are kept.
This commit is contained in:
parent
c42321ff0e
commit
3f5f348270
3 changed files with 34 additions and 19 deletions
|
|
@ -235,7 +235,8 @@ The model answers each classification in JSON. The answer holds:
|
|||
|
||||
- a **verdict**, one of _match_, _no match_ or _not applicable to this PR_
|
||||
- a **certainty** from 0 to 100, how sure the model is of that verdict
|
||||
- a **reason**, at most 500 characters
|
||||
- a **reason**; one over 500 characters is cut to 500, ending in `…`, and the
|
||||
verdict and certainty are kept
|
||||
|
||||
An answer:
|
||||
|
||||
|
|
|
|||
|
|
@ -18,9 +18,9 @@ forge and holds no forge credential.
|
|||
- One `claude --print` per classification, with no session, no tools
|
||||
(`--tools ""`), no MCP server, its own empty directory and
|
||||
`ANTHROPIC_API_KEY` in its environment only.
|
||||
- An answer is a JSON object with a verdict, a certainty and a reason of at
|
||||
most 500 characters. A failed run, a timeout or an answer that doesn't
|
||||
validate leaves that row without a verdict, and it doesn't count.
|
||||
- An answer is a JSON object with a verdict, a certainty and a reason; a
|
||||
reason over 500 characters is cut to 500, ending in `…`. A failed run, a
|
||||
timeout or an answer that doesn't validate leaves that row without a verdict, and it doesn't count.
|
||||
- A run with no API key, or with no classification answered, fails as a
|
||||
whole: the caller sees an error, never a verdict.
|
||||
|
||||
|
|
|
|||
|
|
@ -1,13 +1,15 @@
|
|||
//! Reading the model's answer to one classification.
|
||||
//!
|
||||
//! An answer that does not parse, names an unknown verdict, carries a
|
||||
//! certainty outside 0..=100 or a reason over the cap is an error, and the
|
||||
//! row it belongs to is excluded rather than guessed at.
|
||||
//! An answer that does not parse, names an unknown verdict or carries a
|
||||
//! certainty outside 0..=100 is an error, and the row it belongs to is
|
||||
//! excluded rather than guessed at. A reason over the cap is not an error: it
|
||||
//! is cut to the cap, so the verdict and certainty survive.
|
||||
|
||||
use serde::Deserialize;
|
||||
use swarm_queue_client::review_classifier::Verdict;
|
||||
|
||||
/// The longest reason, in characters, an answer may carry.
|
||||
/// The longest reason, in characters, an answer carries; a longer one is
|
||||
/// truncated to this, ending in `…`.
|
||||
pub const REASON_MAX_CHARS: usize = 500;
|
||||
|
||||
/// One validated answer.
|
||||
|
|
@ -56,19 +58,23 @@ pub fn parse(text: &str) -> Result<Answer, String> {
|
|||
raw.certainty
|
||||
)
|
||||
})?;
|
||||
let length = raw.reason.chars().count();
|
||||
if length > REASON_MAX_CHARS {
|
||||
return Err(format!(
|
||||
"the answer's reason is {length} characters, over the {REASON_MAX_CHARS}-character cap"
|
||||
));
|
||||
}
|
||||
Ok(Answer {
|
||||
verdict,
|
||||
certainty,
|
||||
reason: raw.reason,
|
||||
reason: cap_reason(raw.reason),
|
||||
})
|
||||
}
|
||||
|
||||
/// `reason` cut to `REASON_MAX_CHARS` characters, the last being `…`, if longer.
|
||||
fn cap_reason(reason: String) -> String {
|
||||
if reason.chars().count() <= REASON_MAX_CHARS {
|
||||
return reason;
|
||||
}
|
||||
let mut cut: String = reason.chars().take(REASON_MAX_CHARS - 1).collect();
|
||||
cut.push('…');
|
||||
cut
|
||||
}
|
||||
|
||||
/// `text` without a surrounding code fence, with or without a language tag.
|
||||
fn unfence(text: &str) -> &str {
|
||||
let Some(inner) = text.strip_prefix("```").and_then(|t| t.strip_suffix("```")) else {
|
||||
|
|
@ -119,16 +125,24 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn an_oversized_reason_is_an_error_and_one_at_the_cap_is_not() {
|
||||
fn an_oversized_reason_is_truncated_and_one_at_the_cap_is_kept() {
|
||||
let answer = |n: usize| {
|
||||
format!(
|
||||
r#"{{"verdict":"match","certainty":90,"reason":"{}"}}"#,
|
||||
"é".repeat(n)
|
||||
)
|
||||
};
|
||||
let e = parse(&answer(REASON_MAX_CHARS + 1)).expect_err("over the cap");
|
||||
assert!(e.contains("501 characters"), "{e}");
|
||||
// Characters, not bytes: `é` is two bytes.
|
||||
assert!(parse(&answer(REASON_MAX_CHARS)).is_ok());
|
||||
let at_cap = parse(&answer(REASON_MAX_CHARS)).expect("at the cap");
|
||||
assert_eq!(at_cap.reason, "é".repeat(REASON_MAX_CHARS));
|
||||
|
||||
let long = parse(&answer(600)).expect("over the cap still parses");
|
||||
assert_eq!(long.verdict, Verdict::Match);
|
||||
assert_eq!(long.certainty, 90);
|
||||
assert_eq!(long.reason.chars().count(), REASON_MAX_CHARS);
|
||||
assert_eq!(
|
||||
long.reason,
|
||||
format!("{}…", "é".repeat(REASON_MAX_CHARS - 1))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue