Skip to content

Commit 84b1fb0

Browse files
committed
fix(desktop): harden evaluation evidence edges
1 parent 5e41f68 commit 84b1fb0

5 files changed

Lines changed: 187 additions & 15 deletions

File tree

desktop/src/app.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ impl EvaluationWindow {
9999
fn suite_matches_filter(
100100
row: &SkillRow,
101101
latest_result: Option<&EvaluationRunResult>,
102+
latest_graded_result: Option<&EvaluationRunResult>,
102103
filter: SuiteFilter,
103104
) -> bool {
104105
match filter {
@@ -107,7 +108,7 @@ fn suite_matches_filter(
107108
SuiteFilter::Attention => row.quality_level() == QualityLevel::Attention,
108109
SuiteFilter::Missing => row.quality_level() == QualityLevel::Missing,
109110
SuiteFilter::NotRun => latest_result.is_none(),
110-
SuiteFilter::FailedRun => latest_result.is_some_and(|result| {
111+
SuiteFilter::FailedRun => latest_graded_result.is_some_and(|result| {
111112
!result.is_dry_run()
112113
&& result.total_count > 0
113114
&& result.passed_count < result.total_count
@@ -1521,6 +1522,12 @@ impl MasterSkillApp {
15211522
.into_iter()
15221523
.map(|result| (result.slug.clone(), result))
15231524
.collect();
1525+
let latest_graded_results: BTreeMap<_, _> = self
1526+
.traces
1527+
.latest_graded_evaluation_results_by_slug()
1528+
.into_iter()
1529+
.map(|result| (result.slug.clone(), result))
1530+
.collect();
15241531
ui.horizontal_wrapped(|ui| {
15251532
ui.label("Filter");
15261533
for filter in [
@@ -1543,8 +1550,12 @@ impl MasterSkillApp {
15431550
.clone()
15441551
.into_iter()
15451552
.filter(|row| {
1546-
suite_matches_filter(row, latest_results.get(&row.slug), self.suite_filter)
1547-
&& suite_matches_query(row, &self.suite_query)
1553+
suite_matches_filter(
1554+
row,
1555+
latest_results.get(&row.slug),
1556+
latest_graded_results.get(&row.slug),
1557+
self.suite_filter,
1558+
) && suite_matches_query(row, &self.suite_query)
15481559
})
15491560
.collect();
15501561
if rows.is_empty() {
@@ -2507,35 +2518,48 @@ mod tests {
25072518
mode: EvaluationMode::Graded,
25082519
trace_id: 2,
25092520
};
2521+
let later_dry_run = EvaluationRunResult {
2522+
slug: "zhiyi".to_string(),
2523+
passed_count: 0,
2524+
total_count: 10,
2525+
mode: EvaluationMode::DryRun,
2526+
trace_id: 3,
2527+
};
25102528

25112529
assert!(super::suite_matches_filter(
25122530
&ready,
25132531
Some(&passing_run),
2532+
Some(&passing_run),
25142533
super::SuiteFilter::Ready
25152534
));
25162535
assert!(super::suite_matches_filter(
25172536
&attention,
2537+
Some(&later_dry_run),
25182538
Some(&failed_run),
25192539
super::SuiteFilter::Attention
25202540
));
25212541
assert!(super::suite_matches_filter(
25222542
&missing,
25232543
None,
2544+
None,
25242545
super::SuiteFilter::Missing
25252546
));
25262547
assert!(super::suite_matches_filter(
25272548
&missing,
25282549
None,
2550+
None,
25292551
super::SuiteFilter::NotRun
25302552
));
25312553
assert!(super::suite_matches_filter(
25322554
&attention,
2555+
Some(&later_dry_run),
25332556
Some(&failed_run),
25342557
super::SuiteFilter::FailedRun
25352558
));
25362559
assert!(!super::suite_matches_filter(
25372560
&ready,
25382561
Some(&passing_run),
2562+
Some(&passing_run),
25392563
super::SuiteFilter::FailedRun
25402564
));
25412565
}

desktop/src/trace.rs

Lines changed: 128 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,11 @@ impl EvaluationRunHistoryItem {
226226
}
227227

228228
fn pass_rate_basis(&self) -> usize {
229-
(self.passed_count * 10_000)
230-
.checked_div(self.total_count)
231-
.unwrap_or(0)
229+
if self.total_count == 0 {
230+
return 0;
231+
}
232+
let basis = (self.passed_count as u128 * 10_000) / self.total_count as u128;
233+
basis.min(usize::MAX as u128) as usize
232234
}
233235

234236
pub fn matches_filter(&self, filter: EvaluationRunHistoryFilter) -> bool {
@@ -1071,11 +1073,19 @@ fn evaluation_case_statuses(
10711073
) -> Result<Vec<EvaluationCaseStatus>, String> {
10721074
cases
10731075
.iter()
1074-
.map(|case| match EvaluationCaseStatus::from_wire(&case.status) {
1075-
EvaluationCaseStatus::Unknown(status) => Err(format!(
1076-
"invalid fidelity suite: unknown case status {status:?}"
1077-
)),
1078-
status => Ok(status),
1076+
.map(|case| {
1077+
if case.index.checked_add(1).is_none() {
1078+
return Err(format!(
1079+
"invalid fidelity suite: case index {} cannot be displayed one-based",
1080+
case.index
1081+
));
1082+
}
1083+
match EvaluationCaseStatus::from_wire(&case.status) {
1084+
EvaluationCaseStatus::Unknown(status) => Err(format!(
1085+
"invalid fidelity suite: unknown case status {status:?}"
1086+
)),
1087+
status => Ok(status),
1088+
}
10791089
})
10801090
.collect()
10811091
}
@@ -1272,6 +1282,14 @@ fn compare_evaluation_runs(
12721282
}
12731283
}
12741284

1285+
fn signed_usize_delta(current: usize, previous: usize) -> isize {
1286+
if current >= previous {
1287+
current.saturating_sub(previous).min(isize::MAX as usize) as isize
1288+
} else {
1289+
-(previous.saturating_sub(current).min(isize::MAX as usize) as isize)
1290+
}
1291+
}
1292+
12751293
#[derive(Clone, Debug, Default, PartialEq, Eq)]
12761294
pub struct TraceSummary {
12771295
pub total: usize,
@@ -2153,10 +2171,10 @@ impl TraceStore {
21532171
previous_trace_id: previous.trace_id,
21542172
current_failed_count,
21552173
previous_failed_count,
2156-
failed_delta: current_failed_count as isize - previous_failed_count as isize,
2174+
failed_delta: signed_usize_delta(current_failed_count, previous_failed_count),
21572175
current_pass_rate,
21582176
previous_pass_rate,
2159-
pass_rate_delta_points: current_pass_rate as isize - previous_pass_rate as isize,
2177+
pass_rate_delta_points: signed_usize_delta(current_pass_rate, previous_pass_rate),
21602178
action: current.action.clone(),
21612179
});
21622180
}
@@ -3209,6 +3227,47 @@ mod tests {
32093227
assert!(errors[0].message.contains("SKIPPED"));
32103228
}
32113229

3230+
#[test]
3231+
fn rejects_case_index_that_cannot_be_displayed_one_based() {
3232+
let mut store = TraceStore::new(10);
3233+
let run = store.begin_with_action(
3234+
"Running master-huineng fidelity",
3235+
TraceAction::FidelityDryRunSkill {
3236+
slug: "huineng".to_string(),
3237+
},
3238+
Some("python3 scripts/test-fidelity.py --master master-huineng --json"),
3239+
"Queued.",
3240+
);
3241+
store.finish_success_with_detail(
3242+
run,
3243+
"master-huineng fidelity finished",
3244+
format!(
3245+
r#"[{{
3246+
"schema_version": 1,
3247+
"master": "master-huineng",
3248+
"mode": "graded",
3249+
"outcome": "completed",
3250+
"total": 1,
3251+
"passed": 1,
3252+
"failed": 0,
3253+
"results": [
3254+
{{"index": {}, "question": "overflow", "status": "PASS"}}
3255+
]
3256+
}}]"#,
3257+
usize::MAX
3258+
),
3259+
Duration::from_millis(50),
3260+
);
3261+
3262+
let errors = store.records.back().unwrap().evaluation_errors();
3263+
assert_eq!(errors.len(), 1);
3264+
assert_eq!(
3265+
errors[0].kind,
3266+
EvaluationEvidenceErrorKind::MalformedPayload
3267+
);
3268+
assert!(errors[0].message.contains("index"));
3269+
}
3270+
32123271
#[test]
32133272
fn rejects_evaluation_payload_for_wrong_skill_scope() {
32143273
let mut store = TraceStore::new(10);
@@ -4139,6 +4198,24 @@ mod tests {
41394198
);
41404199
}
41414200

4201+
#[test]
4202+
fn pass_rate_calculation_handles_large_legacy_counts() {
4203+
let item = EvaluationRunHistoryItem {
4204+
trace_id: 1,
4205+
scope: "master-huineng".to_string(),
4206+
status: TraceStatus::Succeeded,
4207+
passed_count: usize::MAX,
4208+
total_count: usize::MAX,
4209+
failed_count: 0,
4210+
mode: EvaluationMode::Graded,
4211+
duration_ms: None,
4212+
action: None,
4213+
trend: EvaluationRunTrend::New,
4214+
};
4215+
4216+
assert_eq!(item.pass_rate_percent(), 100);
4217+
}
4218+
41424219
#[test]
41434220
fn annotates_evaluation_run_history_with_scope_trends() {
41444221
let mut store = TraceStore::new(10);
@@ -4808,6 +4885,47 @@ mod tests {
48084885
);
48094886
}
48104887

4888+
#[test]
4889+
fn saturates_large_legacy_regression_deltas() {
4890+
let mut store = TraceStore::new(10);
4891+
let old_run = store.begin_with_action(
4892+
"Running master-huineng fidelity",
4893+
TraceAction::FidelityDryRunSkill {
4894+
slug: "huineng".to_string(),
4895+
},
4896+
Some("legacy fidelity"),
4897+
"Queued.",
4898+
);
4899+
store.finish_success_with_detail(
4900+
old_run,
4901+
"legacy fidelity finished",
4902+
"Testing: master-huineng\nResult: 0/1 passed (0%)",
4903+
Duration::from_millis(50),
4904+
);
4905+
let new_run = store.begin_with_action(
4906+
"Running master-huineng fidelity",
4907+
TraceAction::FidelityDryRunSkill {
4908+
slug: "huineng".to_string(),
4909+
},
4910+
Some("legacy fidelity"),
4911+
"Queued.",
4912+
);
4913+
store.finish_success_with_detail(
4914+
new_run,
4915+
"legacy fidelity finished",
4916+
format!(
4917+
"Testing: master-huineng\nResult: 0/{} passed (0%)",
4918+
usize::MAX
4919+
),
4920+
Duration::from_millis(50),
4921+
);
4922+
4923+
let regressions = store.evaluation_regressions(8);
4924+
4925+
assert_eq!(regressions.len(), 1);
4926+
assert_eq!(regressions[0].failed_delta, isize::MAX);
4927+
}
4928+
48114929
#[test]
48124930
fn persists_trace_history_and_next_record_id() {
48134931
let path = temp_path("trace-history");

docs/superpowers/plans/2026-07-30-desktop-evaluation-contract.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -755,4 +755,3 @@ Record:
755755
- exact verification commands and outcomes;
756756
- backward-compatibility behavior;
757757
- intentionally deferred PR 2 runtime work.
758-

scripts/test-fidelity.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,22 @@ def load_tests(master_dir: Path) -> list[dict]:
9191
):
9292
if line.strip():
9393
try:
94-
tests.append(json.loads(line))
94+
test = json.loads(line)
9595
except json.JSONDecodeError as error:
9696
raise ValueError(
9797
f"Invalid fidelity.jsonl line {line_number}: {error.msg}"
9898
) from error
99+
if not isinstance(test, dict):
100+
raise ValueError(
101+
f"Invalid fidelity.jsonl line {line_number}: expected a JSON object"
102+
)
103+
question = test.get("q")
104+
if not isinstance(question, str) or not question.strip():
105+
raise ValueError(
106+
f"Invalid fidelity.jsonl line {line_number}: "
107+
"expected a non-empty string q"
108+
)
109+
tests.append(test)
99110
return tests
100111

101112

tests/test_fidelity_exit.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,26 @@ def test_invalid_fidelity_file_emits_versioned_error_suite(runner, monkeypatch,
9797
assert "line 2" in suite["error"]
9898

9999

100+
def test_fidelity_case_without_question_emits_versioned_error_suite(
101+
runner, monkeypatch, tmp_path
102+
):
103+
master_dir = tmp_path / "master-broken"
104+
(master_dir / "tests").mkdir(parents=True)
105+
(master_dir / "tests" / "fidelity.jsonl").write_text(
106+
'{"difficulty":"basic"}\n',
107+
encoding="utf-8",
108+
)
109+
monkeypatch.setattr(runner, "PREBUILT_DIR", tmp_path)
110+
111+
suite = runner.run_tests("master-broken", dry_run=True, quiet=True)
112+
113+
assert suite["outcome"] == "error"
114+
assert suite["total"] == 0
115+
assert suite["results"] == []
116+
assert "line 1" in suite["error"]
117+
assert "non-empty string q" in suite["error"]
118+
119+
100120
def test_missing_master_exits_nonzero_with_clean_json_stdout():
101121
result = subprocess.run(
102122
[

0 commit comments

Comments
 (0)