Skip to content

Commit 05f8a6c

Browse files
Route RunStatus-sensitive branches through the enum, not literals
batch.rs, comparison.rs, dashboard.rs, alerts.rs, and pages.rs were all matching on bare string literals ("queued" / "running" / "completed" etc.) when deciding how to bucket runs. That made it easy for a typo or a new status value to silently fall through to the "_ =>" default and flip terminal/in-flight classification. Parse incoming strings into RunStatus once and match on the enum. The compiler now flags missing variants, and new RunStatus variants will force us to visit every decision point. Domain-specific composites outside the enum (e.g. batch.rs's "failed_partial") are left as string constants with an inline comment explaining why. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent 7465f46 commit 05f8a6c

5 files changed

Lines changed: 66 additions & 45 deletions

File tree

src/routes/pages.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -497,10 +497,13 @@ pub async fn batch_detail(
497497
})
498498
.collect();
499499
let has_pending_runs = batch_runs.iter().any(|run| {
500-
run.status != "completed"
501-
&& run.status != "failed"
502-
&& run.status != "failed_partial"
503-
&& run.status != "cancelled"
500+
// A run is "pending" if it is still in-flight. We additionally treat
501+
// "failed_partial" (a composite batch-level state set by
502+
// services::batch, not a RunStatus) as terminal.
503+
let is_terminal = RunStatus::parse(&run.status)
504+
.map(RunStatus::is_terminal)
505+
.unwrap_or(false);
506+
!is_terminal && run.status != "failed_partial"
504507
});
505508

506509
Ok(Html(
@@ -792,7 +795,9 @@ pub async fn scanner_index(State(state): State<AppState>) -> AppResult<Html<Stri
792795
.collect();
793796

794797
let has_scan_running = has_latest_scan_run
795-
&& (latest_scan_run_status == "running" || latest_scan_run_status == "queued");
798+
&& RunStatus::parse(&latest_scan_run_status)
799+
.map(RunStatus::is_in_flight)
800+
.unwrap_or(false);
796801

797802
let html = ScannerTemplate {
798803
has_latest_scan_run,

src/services/alerts.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::{app_state::AppState, models::AlertRule};
1+
use crate::{app_state::AppState, models::AlertRule, status::RunStatus};
22
use anyhow::Result;
33
use chrono::Utc;
44

@@ -198,7 +198,13 @@ async fn build_snapshot(state: &AppState, ticker: &str) -> Result<TickerAlertSna
198198
}
199199

200200
fn should_skip_status(status: &str) -> bool {
201-
matches!(status, "queued" | "running" | "no_data")
201+
if status == "no_data" {
202+
return true;
203+
}
204+
matches!(
205+
RunStatus::parse(status),
206+
Some(RunStatus::Queued | RunStatus::Running)
207+
)
202208
}
203209

204210
fn is_decision_downgrade(previous: Option<&str>, current: &str) -> bool {
@@ -230,10 +236,12 @@ fn classify_decision(
230236
latest_score: Option<f64>,
231237
evidence_freshness: &str,
232238
) -> String {
233-
match latest_status {
234-
"no_data" => "no_coverage".to_string(),
235-
"queued" | "running" => "researching".to_string(),
236-
"failed" | "cancelled" => "attention".to_string(),
239+
if latest_status == "no_data" {
240+
return "no_coverage".to_string();
241+
}
242+
match RunStatus::parse(latest_status) {
243+
Some(RunStatus::Queued | RunStatus::Running) => "researching".to_string(),
244+
Some(RunStatus::Failed | RunStatus::Cancelled) => "attention".to_string(),
237245
_ => match latest_score {
238246
Some(score) if score >= 8.0 && evidence_freshness == "fresh" => {
239247
"high_conviction".to_string()

src/services/batch.rs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::{app_state::AppState, models::BatchJobRunWithDetails};
1+
use crate::{app_state::AppState, models::BatchJobRunWithDetails, status::RunStatus};
22
use anyhow::Result;
33

44
pub async fn sync_batch_jobs_for_run(state: &AppState, run_id: &str) -> Result<()> {
@@ -14,7 +14,7 @@ async fn sync_single_batch_job(state: &AppState, batch_job_id: &str) -> Result<(
1414
if batch_job_runs.is_empty() {
1515
state
1616
.db
17-
.update_batch_job_status(batch_job_id, "queued")
17+
.update_batch_job_status(batch_job_id, RunStatus::Queued.as_str())
1818
.await?;
1919
return Ok(());
2020
}
@@ -25,26 +25,30 @@ async fn sync_single_batch_job(state: &AppState, batch_job_id: &str) -> Result<(
2525
let mut has_queued = false;
2626

2727
for batch_job_run in &batch_job_runs {
28+
// Any status the DB returns that we don't recognise (e.g. a future
29+
// "retrying" state) is treated as in-flight so we don't prematurely
30+
// finalise the batch.
2831
let status = batch_job_run
2932
.run
3033
.as_ref()
31-
.map(|run| run.status.as_str())
32-
.unwrap_or("queued");
34+
.and_then(|run| RunStatus::parse(&run.status))
35+
.unwrap_or(RunStatus::Queued);
3336
match status {
34-
"completed" => completed_count += 1,
35-
"failed" | "cancelled" => failed_count += 1,
36-
"queued" => has_queued = true,
37-
_ => has_running = true,
37+
RunStatus::Completed => completed_count += 1,
38+
RunStatus::Failed | RunStatus::Cancelled => failed_count += 1,
39+
RunStatus::Queued => has_queued = true,
40+
RunStatus::Running => has_running = true,
3841
}
3942
}
4043

4144
let all_terminal = completed_count + failed_count == batch_job_runs.len();
4245
if all_terminal {
4346
let status = if failed_count == 0 {
44-
"completed"
47+
RunStatus::Completed.as_str()
4548
} else if completed_count == 0 {
46-
"failed"
49+
RunStatus::Failed.as_str()
4750
} else {
51+
// Domain-specific composite status that doesn't map to RunStatus.
4852
"failed_partial"
4953
};
5054
let summary = build_batch_summary(&batch_job_runs, completed_count, failed_count);
@@ -54,15 +58,15 @@ async fn sync_single_batch_job(state: &AppState, batch_job_id: &str) -> Result<(
5458
.await?;
5559
} else {
5660
let status = if has_running {
57-
"running"
61+
RunStatus::Running
5862
} else if has_queued {
59-
"queued"
63+
RunStatus::Queued
6064
} else {
61-
"running"
65+
RunStatus::Running
6266
};
6367
state
6468
.db
65-
.update_batch_job_status(batch_job_id, status)
69+
.update_batch_job_status(batch_job_id, status.as_str())
6670
.await?;
6771
}
6872
Ok(())

src/services/comparison.rs

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::{app_state::AppState, models::ComparisonRunWithDetails};
1+
use crate::{app_state::AppState, models::ComparisonRunWithDetails, status::RunStatus};
22
use anyhow::Result;
33

44
pub async fn sync_comparisons_for_run(state: &AppState, run_id: &str) -> Result<()> {
@@ -14,7 +14,7 @@ async fn sync_single_comparison(state: &AppState, comparison_id: &str) -> Result
1414
if comparison_runs.is_empty() {
1515
state
1616
.db
17-
.update_comparison_status(comparison_id, "queued")
17+
.update_comparison_status(comparison_id, RunStatus::Queued.as_str())
1818
.await?;
1919
return Ok(());
2020
}
@@ -28,13 +28,13 @@ async fn sync_single_comparison(state: &AppState, comparison_id: &str) -> Result
2828
let status = comparison_run
2929
.run
3030
.as_ref()
31-
.map(|run| run.status.as_str())
32-
.unwrap_or("queued");
31+
.and_then(|run| RunStatus::parse(&run.status))
32+
.unwrap_or(RunStatus::Queued);
3333
match status {
34-
"completed" => completed_count += 1,
35-
"failed" | "cancelled" => failed_count += 1,
36-
"queued" => has_queued = true,
37-
_ => has_running = true,
34+
RunStatus::Completed => completed_count += 1,
35+
RunStatus::Failed | RunStatus::Cancelled => failed_count += 1,
36+
RunStatus::Queued => has_queued = true,
37+
RunStatus::Running => has_running = true,
3838
}
3939
}
4040

@@ -48,15 +48,15 @@ async fn sync_single_comparison(state: &AppState, comparison_id: &str) -> Result
4848
.await?;
4949
} else {
5050
let status = if has_running {
51-
"running"
51+
RunStatus::Running
5252
} else if has_queued {
53-
"queued"
53+
RunStatus::Queued
5454
} else {
55-
"running"
55+
RunStatus::Running
5656
};
5757
state
5858
.db
59-
.update_comparison_status(comparison_id, status)
59+
.update_comparison_status(comparison_id, status.as_str())
6060
.await?;
6161
}
6262

@@ -69,10 +69,11 @@ fn build_terminal_rollup(
6969
failed_count: usize,
7070
) -> (&'static str, Option<String>, String) {
7171
let status = if failed_count == 0 {
72-
"completed"
72+
RunStatus::Completed.as_str()
7373
} else if completed_count == 0 {
74-
"failed"
74+
RunStatus::Failed.as_str()
7575
} else {
76+
// Domain-specific composite status outside the RunStatus enum.
7677
"failed_partial"
7778
};
7879

@@ -83,7 +84,7 @@ fn build_terminal_rollup(
8384
.run
8485
.as_ref()
8586
.map(|run| run.status.as_str())
86-
.unwrap_or("queued");
87+
.unwrap_or(RunStatus::Queued.as_str());
8788
let run_summary = comparison_run
8889
.run
8990
.as_ref()
@@ -108,7 +109,7 @@ fn build_terminal_rollup(
108109
.run
109110
.as_ref()
110111
.map(|run| run.status.as_str())
111-
.unwrap_or("queued");
112+
.unwrap_or(RunStatus::Queued.as_str());
112113
let run_summary = comparison_run
113114
.run
114115
.as_ref()

src/services/dashboard.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::{
22
app_state::AppState,
33
models::{DashboardResponse, DashboardTickerRow},
44
services::alerts::{self, TickerAlertSnapshot},
5+
status::RunStatus,
56
};
67
use anyhow::{anyhow, Result};
78
use chrono::Utc;
@@ -144,10 +145,12 @@ fn classify_decision(
144145
latest_score: Option<f64>,
145146
evidence_freshness: &str,
146147
) -> String {
147-
match latest_status {
148-
"no_data" => "no_coverage".to_string(),
149-
"queued" | "running" => "researching".to_string(),
150-
"failed" | "cancelled" => "attention".to_string(),
148+
if latest_status == "no_data" {
149+
return "no_coverage".to_string();
150+
}
151+
match RunStatus::parse(latest_status) {
152+
Some(RunStatus::Queued | RunStatus::Running) => "researching".to_string(),
153+
Some(RunStatus::Failed | RunStatus::Cancelled) => "attention".to_string(),
151154
_ => match latest_score {
152155
Some(score) if score >= 8.0 && evidence_freshness == "fresh" => {
153156
"high_conviction".to_string()

0 commit comments

Comments
 (0)