use std::collections::{BTreeMap, BTreeSet};
use serde::{Deserialize, Serialize};
pub const COMPLETE_THROUGH_PR: u32 = 343;
pub const BUILTIN: &str = include_str!("../tests/fixtures/review/review-corpus.jsonl");
pub fn builtin() -> Result<Corpus, CorpusError> {
Corpus::parse(BUILTIN)
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Verdict {
Real,
False,
}
impl Verdict {
#[must_use]
pub fn as_str(self) -> &'static str {
match self {
Self::Real => "real",
Self::False => "false",
}
}
}
impl std::fmt::Display for Verdict {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.as_str())
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DefectClass {
CleanupGap,
ContractDrift,
ErrorTextDrift,
FalseCompileClaim,
LintConvention,
LossyIdentity,
MissingEvent,
OrderingBug,
PerfContract,
PermissiveConstraint,
ProseClarity,
SilentTruncation,
UxDiagnostic,
VacuousTest,
}
pub const CLASSES: [DefectClass; 14] = [
DefectClass::CleanupGap,
DefectClass::ContractDrift,
DefectClass::ErrorTextDrift,
DefectClass::FalseCompileClaim,
DefectClass::LintConvention,
DefectClass::LossyIdentity,
DefectClass::MissingEvent,
DefectClass::OrderingBug,
DefectClass::PerfContract,
DefectClass::PermissiveConstraint,
DefectClass::ProseClarity,
DefectClass::SilentTruncation,
DefectClass::UxDiagnostic,
DefectClass::VacuousTest,
];
impl DefectClass {
#[must_use]
pub fn as_str(self) -> &'static str {
match self {
Self::CleanupGap => "cleanup-gap",
Self::ContractDrift => "contract-drift",
Self::ErrorTextDrift => "error-text-drift",
Self::FalseCompileClaim => "false-compile-claim",
Self::LintConvention => "lint-convention",
Self::LossyIdentity => "lossy-identity",
Self::MissingEvent => "missing-event",
Self::OrderingBug => "ordering-bug",
Self::PerfContract => "perf-contract",
Self::PermissiveConstraint => "permissive-constraint",
Self::ProseClarity => "prose-clarity",
Self::SilentTruncation => "silent-truncation",
Self::UxDiagnostic => "ux-diagnostic",
Self::VacuousTest => "vacuous-test",
}
}
#[must_use]
pub fn from_token(token: &str) -> Option<Self> {
CLASSES.into_iter().find(|c| c.as_str() == token)
}
}
impl std::fmt::Display for DefectClass {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.as_str())
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct CorpusRow {
pub id: u64,
pub pr: u32,
pub reviewer: String,
pub reviewed_sha: String,
pub path: String,
pub line: u32,
pub verdict: Verdict,
pub defect_class: DefectClass,
pub fix_commit: String,
pub description: String,
pub comment_url: String,
}
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
pub enum CorpusError {
#[error("line {line}: {message}")]
Malformed {
line: usize,
message: String,
},
#[error("line {line}: {field} {message}")]
Invalid {
line: usize,
field: &'static str,
message: String,
},
#[error("line {line}: duplicate comment id {id}, first seen on line {first}")]
DuplicateId {
line: usize,
first: usize,
id: u64,
},
}
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct Corpus {
rows: Vec<CorpusRow>,
}
impl Corpus {
pub fn parse(text: &str) -> Result<Self, CorpusError> {
let mut rows = Vec::new();
let mut seen: BTreeMap<u64, usize> = BTreeMap::new();
for (idx, raw) in text.lines().enumerate() {
let line = idx + 1;
if raw.trim().is_empty() {
continue;
}
let row: CorpusRow = serde_json::from_str(raw).map_err(|e| CorpusError::Malformed {
line,
message: e.to_string(),
})?;
validate(&row, line)?;
if let Some(&first) = seen.get(&row.id) {
return Err(CorpusError::DuplicateId {
line,
first,
id: row.id,
});
}
seen.insert(row.id, line);
rows.push(row);
}
Ok(Self { rows })
}
#[must_use]
pub fn rows(&self) -> &[CorpusRow] {
&self.rows
}
#[must_use]
pub fn len(&self) -> usize {
self.rows.len()
}
#[must_use]
pub fn is_empty(&self) -> bool {
self.rows.is_empty()
}
#[must_use]
pub fn complete_subset(&self) -> Self {
Self {
rows: self
.rows
.iter()
.filter(|r| r.pr <= COMPLETE_THROUGH_PR)
.cloned()
.collect(),
}
}
pub fn with_verdict(&self, verdict: Verdict) -> impl Iterator<Item = &CorpusRow> {
self.rows.iter().filter(move |r| r.verdict == verdict)
}
#[must_use]
pub fn reviewed_shas(&self) -> BTreeSet<&str> {
self.rows.iter().map(|r| r.reviewed_sha.as_str()).collect()
}
#[must_use]
pub fn class_counts(&self) -> BTreeMap<DefectClass, (usize, usize)> {
let mut counts: BTreeMap<DefectClass, (usize, usize)> = BTreeMap::new();
for row in &self.rows {
let entry = counts.entry(row.defect_class).or_insert((0, 0));
match row.verdict {
Verdict::Real => entry.0 += 1,
Verdict::False => entry.1 += 1,
}
}
counts
}
}
fn is_full_sha(s: &str) -> bool {
s.len() == 40 && s.chars().all(|c| c.is_ascii_hexdigit())
}
fn validate(row: &CorpusRow, line: usize) -> Result<(), CorpusError> {
let invalid = |field: &'static str, message: String| CorpusError::Invalid {
line,
field,
message,
};
for (field, value) in [
("id", row.id),
("pr", u64::from(row.pr)),
("line", u64::from(row.line)),
] {
if value == 0 {
return Err(invalid(field, "must be positive, got 0".to_owned()));
}
}
if !is_full_sha(&row.reviewed_sha) {
return Err(invalid(
"reviewed_sha",
format!(
"{:?} is not a 40-character hex sha. It must be the comment's \
`original_commit_id` — the tree the reviewer saw — never the \
merged PR head, which contains the fix commits",
row.reviewed_sha
),
));
}
let looks_like_sha =
row.fix_commit.len() >= 7 && row.fix_commit.chars().all(|c| c.is_ascii_hexdigit());
if !row.fix_commit.is_empty() && !looks_like_sha {
return Err(invalid(
"fix_commit",
format!("{:?} is neither empty nor a hex sha", row.fix_commit),
));
}
for (field, value) in [
("reviewer", &row.reviewer),
("path", &row.path),
("description", &row.description),
("comment_url", &row.comment_url),
] {
if value.trim().is_empty() {
return Err(invalid(field, "must not be blank".to_owned()));
}
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::{
CLASSES, COMPLETE_THROUGH_PR, Corpus, CorpusError, CorpusRow, DefectClass, Verdict,
};
fn row_json(overrides: &[(&str, &str)]) -> String {
let mut fields: Vec<(&str, String)> = vec![
("id", "3788975371".to_owned()),
("pr", "292".to_owned()),
("reviewer", "\"github-copilot\"".to_owned()),
(
"reviewed_sha",
"\"97938e013380d66f44ea0cb587b637d06fda1bbb\"".to_owned(),
),
("path", "\"crates/rto-graph/src/engine_slot.rs\"".to_owned()),
("line", "16".to_owned()),
("verdict", "\"real\"".to_owned()),
("defect_class", "\"contract-drift\"".to_owned()),
("fix_commit", "\"41cb5e9\"".to_owned()),
(
"description",
"\"module doc contradicts the lock\"".to_owned(),
),
("comment_url", "\"https://example.invalid/1\"".to_owned()),
];
for &(key, value) in overrides {
if let Some(slot) = fields.iter_mut().find(|(k, _)| *k == key) {
slot.1 = value.to_owned();
} else {
fields.push((key, value.to_owned()));
}
}
let body: Vec<String> = fields.iter().map(|(k, v)| format!("{k:?}: {v}")).collect();
format!("{{{}}}", body.join(", "))
}
#[test]
fn as_str_matches_the_serialised_form() {
for class in CLASSES {
let serialised = serde_json::to_string(&class).expect("a class serialises");
assert_eq!(
serialised,
format!("{:?}", class.as_str()),
"{class:?}: as_str and the serde rename disagree"
);
assert_eq!(DefectClass::from_token(class.as_str()), Some(class));
}
for verdict in [Verdict::Real, Verdict::False] {
let serialised = serde_json::to_string(&verdict).expect("a verdict serialises");
assert_eq!(serialised, format!("{:?}", verdict.as_str()));
}
assert_eq!(DefectClass::from_token("no-such-class"), None);
}
#[test]
fn every_class_is_listed_exactly_once() {
let tokens: Vec<&str> = CLASSES.iter().map(|c| c.as_str()).collect();
let mut sorted = tokens.clone();
sorted.sort_unstable();
assert_eq!(
tokens, sorted,
"CLASSES is not in token order, so two reports would not line up"
);
let mut deduped = sorted.clone();
deduped.dedup();
assert_eq!(deduped.len(), tokens.len(), "CLASSES repeats a class");
}
#[test]
fn a_well_formed_row_parses() {
let corpus = Corpus::parse(&row_json(&[])).expect("parses");
assert_eq!(corpus.len(), 1);
let row = &corpus.rows()[0];
assert_eq!(row.verdict, Verdict::Real);
assert_eq!(row.defect_class, DefectClass::ContractDrift);
assert_eq!(row.pr, 292);
}
#[test]
fn blank_lines_are_skipped() {
let text = format!("{}\n\n \n", row_json(&[]));
assert_eq!(Corpus::parse(&text).expect("parses").len(), 1);
}
#[test]
fn an_unknown_field_is_refused() {
let err = Corpus::parse(&row_json(&[("severity", "\"high\"")]))
.expect_err("an extra field is not the documented schema");
let CorpusError::Malformed { line, ref message } = err else {
panic!("expected Malformed, got {err:?}");
};
assert_eq!(line, 1);
assert!(message.contains("severity"), "names the field: {message}");
}
#[test]
fn a_missing_field_is_refused() {
let json = row_json(&[]).replace("\"fix_commit\": \"41cb5e9\", ", "");
let err = Corpus::parse(&json).expect_err("a missing field is not the schema");
assert!(
err.to_string().contains("fix_commit"),
"names the field: {err}"
);
}
#[test]
fn an_undocumented_class_or_verdict_is_refused() {
for (field, value) in [
("defect_class", "\"off-by-one\""),
("verdict", "\"probably\""),
] {
let err =
Corpus::parse(&row_json(&[(field, value)])).expect_err("not a documented token");
assert!(
matches!(err, CorpusError::Malformed { .. }),
"{field}: {err:?}"
);
}
}
#[test]
fn a_short_reviewed_sha_is_refused_and_says_why() {
let err = Corpus::parse(&row_json(&[("reviewed_sha", "\"97938e0\"")]))
.expect_err("a short sha is not the review commit");
let text = err.to_string();
assert!(text.contains("reviewed_sha"), "names the field: {text}");
assert!(
text.contains("original_commit_id"),
"says what the field is: {text}"
);
assert!(
text.contains("fix commits"),
"says why the head is wrong: {text}"
);
}
#[test]
fn a_zero_identifier_is_refused() {
for field in ["id", "pr", "line"] {
let err =
Corpus::parse(&row_json(&[(field, "0")])).expect_err("zero is not an identifier");
let CorpusError::Invalid { field: got, .. } = err else {
panic!("expected Invalid, got {err:?}");
};
assert_eq!(got, field);
}
}
#[test]
fn a_fix_commit_that_is_not_a_sha_is_refused_but_blank_is_allowed() {
let ok = Corpus::parse(&row_json(&[("fix_commit", "\"\"")])).expect("blank is allowed");
assert!(ok.rows()[0].fix_commit.is_empty());
let err = Corpus::parse(&row_json(&[("fix_commit", "\"landed in a rework\"")]))
.expect_err("prose is not a sha");
assert!(err.to_string().contains("fix_commit"), "{err}");
}
#[test]
fn a_blank_text_field_is_refused() {
for field in ["reviewer", "path", "description", "comment_url"] {
let err = Corpus::parse(&row_json(&[(field, "\" \"")]))
.expect_err("blank text is not text");
assert!(err.to_string().contains(field), "{field}: {err}");
}
}
#[test]
fn a_duplicate_id_is_refused_and_names_both_lines() {
let text = format!("{}\n{}", row_json(&[]), row_json(&[("pr", "293")]));
let err = Corpus::parse(&text).expect_err("the id repeats");
let CorpusError::DuplicateId { line, first, id } = err else {
panic!("expected DuplicateId, got {err:?}");
};
assert_eq!((line, first, id), (2, 1, 3_788_975_371));
}
#[test]
fn complete_subset_drops_selectively_added_rows() {
let text = format!(
"{}\n{}",
row_json(&[]),
row_json(&[("id", "9999"), ("pr", "352")])
);
let corpus = Corpus::parse(&text).expect("parses");
assert_eq!(corpus.len(), 2);
let complete = corpus.complete_subset();
assert_eq!(complete.len(), 1);
assert!(complete.rows().iter().all(|r| r.pr <= COMPLETE_THROUGH_PR));
}
#[test]
fn class_counts_splits_real_from_false() {
let text = format!(
"{}\n{}\n{}",
row_json(&[]),
row_json(&[
("id", "2"),
("defect_class", "\"false-compile-claim\""),
("verdict", "\"false\"")
]),
row_json(&[
("id", "3"),
("defect_class", "\"false-compile-claim\""),
("verdict", "\"false\"")
]),
);
let counts = Corpus::parse(&text).expect("parses").class_counts();
assert_eq!(counts[&DefectClass::ContractDrift], (1, 0));
assert_eq!(counts[&DefectClass::FalseCompileClaim], (0, 2));
assert_eq!(counts.len(), 2, "absent classes are not invented");
}
#[test]
fn reviewed_shas_are_deduplicated() {
let text = format!("{}\n{}", row_json(&[]), row_json(&[("id", "2")]));
let corpus = Corpus::parse(&text).expect("parses");
assert_eq!(corpus.len(), 2);
assert_eq!(corpus.reviewed_shas().len(), 1);
}
#[test]
fn a_row_round_trips() {
let corpus = Corpus::parse(&row_json(&[])).expect("parses");
let json = serde_json::to_string(&corpus.rows()[0]).expect("serialises");
let back: CorpusRow = serde_json::from_str(&json).expect("deserialises");
assert_eq!(&back, &corpus.rows()[0]);
}
}