Back to feed

sledtools/pika branch #93

pika-git-typed-state-1

Type internal forge state

Target branch: master

Merge Commit: 6c8537c002abd532fbb5e7985d66571f1faca6ce

branch: merged tutorial: ready ci: success
Open CI Details

Continuous Integration

CI: success

Compact status on the review page, with full logs on the CI page.

Open CI Details

Latest run #119 success

2 passed

head 11ef4b0e814c33c75878538236cb9c9f390f2dea · queued 2026-03-26 01:33:14 · 2 lane(s)

queued 6s · ran 29s

check-notifications · success check-agent-contracts · success

Summary

This branch replaces stringly-typed forge state throughout the pika-news crate with proper Rust enums from the pika_forge_model crate. Fields like branch_state, ci_status, tutorial_status, and per-lane status that were previously String are now BranchState, ForgeCiStatus, TutorialStatus, and CiLaneStatus respectively. The conversion boundary is pushed to the database edge: raw strings read from SQLite rows are parsed into enums immediately via From impls or dedicated parse_lane_status helpers, and enums are serialized back to &str only when writing to the database or rendering to HTML templates. This eliminates an entire class of typo-driven bugs and makes match exhaustiveness checks enforceable at compile time.

Tutorial Steps

Introduce typed enum imports across the crate

Intent: Add the new enum types from `pika_forge_model` and the internal `CiLaneStatus` to every module that previously relied on raw strings for state representation.

Affected files: crates/pika-news/src/branch_store.rs, crates/pika-news/src/ci.rs, crates/pika-news/src/ci_store.rs, crates/pika-news/src/forge_service.rs, crates/pika-news/src/poller.rs, crates/pika-news/src/web.rs

Evidence
@@ -1,4 +1,5 @@
 use anyhow::Context;
+use pika_forge_model::{BranchState, ForgeCiStatus, TutorialStatus};
@@ -7,6 +7,7 @@
 use crate::ci_manifest::{self, ForgeCiManifest};
+use crate::ci_state::CiLaneStatus;
@@ -3,6 +3,7 @@
 use chrono::{DateTime, SecondsFormat, Utc};
+use pika_forge_model::ForgeCiStatus;
@@ -1,6 +1,9 @@
 use std::sync::Arc;
+use pika_forge_model::BranchState;
+use crate::ci_state::CiLaneStatus;

Every module that touches branch state, CI run status, tutorial status, or lane status now imports the corresponding enum type. branch_store.rs pulls in BranchState, ForgeCiStatus, and TutorialStatus from the shared pika_forge_model crate. ci_store.rs and ci.rs import ForgeCiStatus and the crate-internal CiLaneStatus. forge_service.rs imports both BranchState and CiLaneStatus. web.rs restructures its imports to pull CiLaneStatus from crate::ci_state rather than re-aliasing it from the model crate.

This is pure preparation — no logic changes yet, just making the types available.

Replace String fields with enums in branch store structs

Intent: Change the data-carrying structs `BranchFeedItem`, `BranchDetailRecord`, `BranchLookupRecord`, and `BranchActionTarget` so that state fields are statically typed enums instead of `String`.

Affected files: crates/pika-news/src/branch_store.rs

Evidence
@@ -46,11 +47,11 @@
-    pub state: String,
+    pub state: BranchState,
-    pub tutorial_status: String,
-    pub ci_status: String,
+    pub tutorial_status: TutorialStatus,
+    pub ci_status: ForgeCiStatus,
@@ -61,18 +62,18 @@
-    pub branch_state: String,
+    pub branch_state: BranchState,
-    pub tutorial_status: String,
+    pub tutorial_status: TutorialStatus,
-    pub ci_status: String,
+    pub ci_status: ForgeCiStatus,
@@ -80,7 +81,7 @@
-    pub branch_state: String,
+    pub branch_state: BranchState,
@@ -90,7 +91,7 @@
-    pub branch_state: String,
+    pub branch_state: BranchState,

BranchFeedItem.state becomes BranchState, BranchFeedItem.tutorial_status becomes TutorialStatus, and BranchFeedItem.ci_status becomes ForgeCiStatus. The same pattern is applied to BranchDetailRecord, BranchLookupRecord, and BranchActionTarget. Each struct's branch_state field moves from String to BranchState.

This is the key design decision: the struct is the boundary. Any code that reads one of these structs now gets compile-time guarantees about the valid states a branch can be in.

Replace String fields with enums in CI store structs

Intent: Apply the same treatment to CI-related records: `BranchCiRunRecord`, `BranchCiLaneRecord`, `NightlyFeedItem`, `NightlyRunRecord`, `NightlyLaneRecord`, and internal rerun/recovery source structs.

Affected files: crates/pika-news/src/ci_store.rs

Evidence
@@ -19,7 +20,7 @@
-    pub status: String,
+    pub status: ForgeCiStatus,
@@ -34,7 +35,7 @@
-    pub status: String,
+    pub status: CiLaneStatus,
@@ -72,7 +73,7 @@
-    pub status: String,
+    pub status: ForgeCiStatus,
@@ -84,7 +85,7 @@
-    pub status: String,
+    pub status: ForgeCiStatus,
@@ -100,7 +101,7 @@
-    pub status: String,
+    pub status: CiLaneStatus,
@@ -141,14 +142,14 @@
-    status: String,
+    status: CiLaneStatus,
@@ -166,14 +167,14 @@
-    status: String,
+    status: CiLaneStatus,

Suite-level status fields (BranchCiRunRecord.status, NightlyFeedItem.status, NightlyRunRecord.status) become ForgeCiStatus. Lane-level status fields (BranchCiLaneRecord.status, NightlyLaneRecord.status) become the internal CiLaneStatus enum. Internal helper structs like BranchLaneRerunSource, BranchLaneRecoverySource, NightlyLaneRerunSource, and NightlyLaneRecoverySource also switch their status fields.

Note the intentional distinction: suite status uses the shared ForgeCiStatus enum (which is part of the public API model), while lane status uses the crate-internal CiLaneStatus. This keeps the domain separation clean.

Parse enums at the database read boundary in branch_store

Intent: Convert raw SQLite string columns into typed enums immediately when reading rows, so that all downstream code works with enums exclusively.

Affected files: crates/pika-news/src/branch_store.rs

Evidence
@@ -312,11 +313,11 @@
-                        state: row.get(4)?,
+                        state: BranchState::from(row.get::<_, String>(4)?),
-                        tutorial_status: row.get(7)?,
-                        ci_status: row.get(8)?,
+                        tutorial_status: TutorialStatus::from(row.get::<_, String>(7)?),
+                        ci_status: ForgeCiStatus::from(row.get::<_, String>(8)?),
@@ -381,18 +382,18 @@
-                        branch_state: row.get(5)?,
+                        branch_state: BranchState::from(row.get::<_, String>(5)?),
-                        tutorial_status: row.get(11)?,
+                        tutorial_status: TutorialStatus::from(row.get::<_, String>(11)?),
-                        ci_status: row.get(16)?,
+                        ci_status: ForgeCiStatus::from(row.get::<_, String>(16)?),
@@ -421,7 +422,7 @@
-                        branch_state: row.get(3)?,
+                        branch_state: BranchState::from(row.get::<_, String>(3)?),

The pattern is consistent: row.get::<_, String>(N)? pulls the raw string from SQLite, then an immediate From conversion (BranchState::from(...), TutorialStatus::from(...), ForgeCiStatus::from(...)) produces the enum. This is applied in list_branch_feed, get_branch_detail, lookup_branch, and get_branch_action_target.

The From<String> impls on these enums are defined in the pika_forge_model crate and handle unknown strings gracefully (typically mapping to a default variant), which keeps the migration safe even if the database contains legacy values.

Add parse_lane_status helper and use it in ci_store

Intent: Introduce a dedicated parsing function for lane status strings and use it at every point where lane status is read from SQLite, providing proper error reporting for invalid values.

Affected files: crates/pika-news/src/ci_store.rs

Evidence
@@ -1816,6 +1838,19 @@
+fn parse_lane_status(raw: &str) -> rusqlite::Result<CiLaneStatus> {
+    CiLaneStatus::from_str(raw).map_err(|_| {
+        rusqlite::Error::FromSqlConversionFailure(
+            raw.len(),
+            rusqlite::types::Type::Text,
+            Box::new(std::io::Error::new(
+                std::io::ErrorKind::InvalidData,
+                format!("invalid ci lane status `{raw}`"),
+            )),
+        )
+    })
+}
@@ -1731,7 +1753,7 @@
-                status: row.get(4)?,
+                status: parse_lane_status(&row.get::<_, String>(4)?)?,
@@ -1777,7 +1799,7 @@
-                status: row.get(4)?,
+                status: parse_lane_status(&row.get::<_, String>(4)?)?,

Unlike BranchState and ForgeCiStatus which use From<String> (infallible), lane status parsing uses FromStr (fallible) wrapped in the new parse_lane_status helper. This is stricter: an unrecognized lane status string will surface as a rusqlite::Error::FromSqlConversionFailure rather than silently mapping to a default.

The helper is used in list_branch_ci_run_lanes, list_nightly_run_lanes, and all the rerun/recovery source queries — every place a lane status is read from the database.

Type the finish and aggregate functions in ci_store

Intent: Change the signatures of `finish_branch_ci_lane_run`, `finish_nightly_lane_run`, `aggregate_lane_status`, and `update_target_health_after_lane_finish` to accept and return enum types instead of string slices.

Affected files: crates/pika-news/src/ci_store.rs

Evidence
@@ -1368,10 +1390,10 @@
-        status: &str,
+        status: CiLaneStatus,
@@ -1416,7 +1438,7 @@
-                        status,
+                        status.as_str(),
@@ -1612,10 +1634,10 @@
-        status: &str,
+        status: CiLaneStatus,
@@ -2223,7 +2258,7 @@
-) -> anyhow::Result<String> {
+) -> anyhow::Result<ForgeCiStatus> {
@@ -2246,17 +2281,17 @@
-        "success"
+        ForgeCiStatus::Success
-        "running"
+        ForgeCiStatus::Running
-        "queued"
+        ForgeCiStatus::Queued
-        "failed"
+        ForgeCiStatus::Failed
@@ -2100,7 +2135,7 @@
-    status: &str,
+    status: CiLaneStatus,

finish_branch_ci_lane_run and finish_nightly_lane_run now accept CiLaneStatus instead of &str. Internally they call status.as_str() only at the SQL parameter boundary. The aggregate_lane_status function returns ForgeCiStatus directly, eliminating the String allocation and making the suite-level status updates (update_branch_ci_suite_status, update_nightly_run_status) work with typed comparisons.

update_target_health_after_lane_finish takes CiLaneStatus and uses pattern matching instead of string comparison:

// Before
(value, Some(kind)) if value == CiLaneStatus::Failed.as_str() => { ... }
(value, _) if value == CiLaneStatus::Success.as_str() => { ... }

// After
(CiLaneStatus::Failed, Some(kind)) => { ... }
(CiLaneStatus::Success, _) => { ... }

This is where the exhaustiveness checking benefit becomes most visible.

Type the suite status update logic

Intent: Replace string comparisons in `update_branch_ci_suite_status` and `update_nightly_run_status` with enum matching, serializing to string only when writing the SQL parameter.

Affected files: crates/pika-news/src/ci_store.rs

Evidence
@@ -2319,11 +2354,11 @@
-    let sql = if status == "success" || status == "failed" {
+    let sql = if matches!(status, ForgeCiStatus::Success | ForgeCiStatus::Failed) {
-    } else if status == "queued" {
+    } else if status == ForgeCiStatus::Queued {
@@ -2332,7 +2367,7 @@
-    conn.execute(sql, params![status, suite_id])
+    conn.execute(sql, params![status.as_str(), suite_id])
@@ -2340,11 +2375,11 @@
-    let sql = if status == "success" || status == "failed" {
+    let sql = if matches!(status, ForgeCiStatus::Success | ForgeCiStatus::Failed) {
-    } else if status == "queued" {
+    } else if status == ForgeCiStatus::Queued {
@@ -2353,7 +2388,7 @@
-    conn.execute(sql, params![status, nightly_run_id])
+    conn.execute(sql, params![status.as_str(), nightly_run_id])

Both suite status update functions now use matches! with ForgeCiStatus variants to select which SQL statement to execute (setting finished_at, clearing started_at, etc.). The status.as_str() call happens only inside the params![] macro, which is the database write boundary. This pattern ensures the type system governs all logic decisions while keeping SQLite happy with its text columns.

Type the status comparisons in rerun and recovery guards

Intent: Replace string-based status guards in rerun and recovery operations with pattern matching on the CiLaneStatus enum.

Affected files: crates/pika-news/src/ci_store.rs

Evidence
@@ -579,7 +595,7 @@
-            if matches!(row.status.as_str(), "queued" | "running") {
+            if matches!(row.status, CiLaneStatus::Queued | CiLaneStatus::Running) {
@@ -690,7 +706,7 @@
-            if matches!(row.status.as_str(), "queued" | "running") {
+            if matches!(row.status, CiLaneStatus::Queued | CiLaneStatus::Running) {
@@ -788,7 +804,7 @@
-            if !matches!(row.status.as_str(), "queued" | "running") {
+            if !matches!(row.status, CiLaneStatus::Queued | CiLaneStatus::Running) {
@@ -848,7 +864,10 @@
-            if !matches!(row.status.as_str(), "queued" | "running" | "failed") {
+            if !matches!(
+                row.status,
+                CiLaneStatus::Queued | CiLaneStatus::Running | CiLaneStatus::Failed
+            ) {

The rerun and recovery guard conditions — which check whether a lane is in a valid state for the requested operation — now use enum variants. This applies to branch lane reruns, nightly lane reruns, branch/nightly lane cancellation, and branch/nightly lane requeue operations.

The key safety improvement: if a new CiLaneStatus variant is added in the future, any matches! expression that doesn't account for it will still compile (since matches! isn't exhaustive), but the typed comparisons in the struct fields will force developers to handle the new variant wherever destructuring or exhaustive matching is used.

Type the CI suite creation paths

Intent: Use ForgeCiStatus enum values when creating new CI suites (branch and nightly), converting to string only at the SQL insertion point.

Affected files: crates/pika-news/src/ci_store.rs

Evidence
@@ -264,7 +265,11 @@
-            let suite_status = if lanes.is_empty() { "success" } else { "queued" };
+            let suite_status = if lanes.is_empty() {
+                ForgeCiStatus::Success
+            } else {
+                ForgeCiStatus::Queued
+            };
@@ -289,7 +294,7 @@
-                    suite_status,
+                    suite_status.as_str(),
@@ -484,7 +489,11 @@
-            let status = if lanes.is_empty() { "success" } else { "queued" };
+            let status = if lanes.is_empty() {
+                ForgeCiStatus::Success
+            } else {
+                ForgeCiStatus::Queued
+            };
@@ -493,7 +502,14 @@
-                params![repo_id, source_ref, source_head_sha, status, summary, scheduled_for],
+                params![
+                    repo_id,
+                    source_ref,
+                    source_head_sha,
+                    status.as_str(),
+                    summary,
+                    scheduled_for
+                ],

When a new branch CI suite or nightly run is created, the initial status is determined as ForgeCiStatus::Success (for empty lane sets) or ForgeCiStatus::Queued (when lanes are pending). The enum value is passed through the creation logic and only serialized to a string via .as_str() in the params![] macro at the SQL INSERT statement.

Update the CI executor to use CiLaneStatus

Intent: Change the branch and nightly job execution functions in the CI runner to construct CiLaneStatus enum values instead of string literals.

Affected files: crates/pika-news/src/ci.rs

Evidence
@@ -426,7 +427,11 @@
-            let status = if exec.success { "success" } else { "failed" };
+            let status = if exec.success {
+                CiLaneStatus::Success
+            } else {
+                CiLaneStatus::Failed
+            };
@@ -452,8 +457,12 @@
-            match store.finish_branch_ci_lane_run(job.lane_run_id, job.claim_token, "failed", &log)
+            match store.finish_branch_ci_lane_run(
+                job.lane_run_id,
+                job.claim_token,
+                CiLaneStatus::Failed,
+                &log,
+            )
@@ -512,7 +521,11 @@
-            let status = if exec.success { "success" } else { "failed" };
+            let status = if exec.success {
+                CiLaneStatus::Success
+            } else {
+                CiLaneStatus::Failed
+            };
@@ -535,7 +548,12 @@
-            match store.finish_nightly_lane_run(job.lane_run_id, job.claim_token, "failed", &log) {
+            match store.finish_nightly_lane_run(
+                job.lane_run_id,
+                job.claim_token,
+                CiLaneStatus::Failed,
+                &log,
+            ) {

Both execute_branch_job and execute_nightly_job now construct CiLaneStatus::Success or CiLaneStatus::Failed based on the execution result, and pass these enum values to the store's finish_*_lane_run methods. The error-path fallback (CI runner infrastructure errors) also uses CiLaneStatus::Failed directly rather than the string literal "failed".

Update forge_service to use typed enums for state checks and return values

Intent: Replace string comparisons for branch state guards and string literals for lane mutation results with the appropriate enum types.

Affected files: crates/pika-news/src/forge_service.rs

Evidence
@@ -52,13 +55,13 @@
-    pub(crate) lane_status: &'static str,
+    pub(crate) lane_status: CiLaneStatus,
@@ -179,7 +182,7 @@
-        if target.branch_state != "open" {
+        if target.branch_state != BranchState::Open {
@@ -245,7 +248,7 @@
-        if target.branch_state != "open" {
+        if target.branch_state != BranchState::Open {
@@ -366,7 +369,7 @@
-                    lane_status: "failed",
+                    lane_status: CiLaneStatus::Failed,
@@ -392,7 +395,7 @@
-                    lane_status: "queued",
+                    lane_status: CiLaneStatus::Queued,

The BranchLaneMutationResult and NightlyLaneMutationResult structs change their lane_status field from &'static str to CiLaneStatus. The merge and close guards now compare against BranchState::Open instead of the string "open". Lane cancel results report CiLaneStatus::Failed, and requeue results report CiLaneStatus::Queued.

Update web layer to serialize enums at the template boundary

Intent: Convert typed enum values back to strings only when populating HTML template view structs, keeping all internal web logic typed.

Affected files: crates/pika-news/src/web.rs

Evidence
@@ -1435,10 +1435,10 @@
-        state: item.state,
+        state: item.state.as_str().to_string(),
-        tutorial_status: item.tutorial_status,
-        ci_status: item.ci_status,
+        tutorial_status: item.tutorial_status.as_str().to_string(),
+        ci_status: item.ci_status.as_str().to_string(),
@@ -1515,10 +1515,10 @@
-        branch_state: record.branch_state.clone(),
+        branch_state: record.branch_state.as_str().to_string(),
-        tutorial_status: record.tutorial_status,
-        ci_status: record.ci_status,
+        tutorial_status: record.tutorial_status.as_str().to_string(),
+        ci_status: record.ci_status.as_str().to_string(),
@@ -1591,20 +1591,20 @@
-        matches!(run.status.as_str(), "queued" | "running")
+        matches!(run.status, ForgeCiStatus::Queued | ForgeCiStatus::Running)
-                .any(|lane| matches!(lane.status.as_str(), "queued" | "running"))
+                .any(|lane| matches!(lane.status, CiLaneStatus::Queued | CiLaneStatus::Running))
@@ -1626,16 +1626,16 @@
-                .filter(|lane| lane.status == "failed")
+                .filter(|lane| lane.status == CiLaneStatus::Failed)

The web layer is the serialization boundary. Internal logic like branch_ci_runs_are_active and nightly_run_is_active now use enum matching (ForgeCiStatus::Queued | ForgeCiStatus::Running) instead of string comparisons. The ci_lane_counts function and failed-lane filters compare against CiLaneStatus::Failed.

Template view structs still expect String fields (since the HTML templates render text), so the conversion .as_str().to_string() happens exactly at the point where domain data is mapped into template data — in map_feed_item, map_nightly_feed_item, render_detail_template_with_notices, render_branch_ci_live_html_at, and render_branch_ci_summary_html_at.

Update all test assertions to use typed enums

Intent: Replace string literal comparisons in test assertions with enum variants, validating that the typed API works end-to-end.

Affected files: crates/pika-news/src/branch_store.rs, crates/pika-news/src/ci.rs, crates/pika-news/src/poller.rs

Evidence
@@ -931,10 +933,10 @@
-            .any(|item| item.branch_id == first.branch_id && item.state == "closed"));
+            .any(|item| item.branch_id == first.branch_id && item.state == BranchState::Closed));
@@ -1034,7 +1034,7 @@
-        assert_eq!(runs[0].status, "queued");
+        assert_eq!(runs[0].status, ForgeCiStatus::Queued);
@@ -1371,24 +1371,34 @@
-        assert_eq!(runs[0].status, "running");
+        assert_eq!(runs[0].status, ForgeCiStatus::Running);
-        assert_eq!(runs[0].lanes[0].status, "failed");
-        assert_eq!(runs[0].lanes[1].status, "running");
+        assert_eq!(runs[0].lanes[0].status, CiLaneStatus::Failed);
+        assert_eq!(runs[0].lanes[1].status, CiLaneStatus::Running);
@@ -927,9 +946,12 @@
-        assert_eq!(runs[0].status, "success");
+        assert_eq!(runs[0].status, pika_forge_model::ForgeCiStatus::Success);
-        assert!(runs[0].lanes.iter().all(|lane| lane.status == "success"));
+        assert!(runs[0].lanes.iter().all(|lane| lane.status == CiLaneStatus::Success));
@@ -287,7 +288,7 @@
-        assert_eq!(finished[0].lanes[0].status, "success");
+        assert_eq!(finished[0].lanes[0].status, CiLaneStatus::Success);

Every test assertion that previously compared against string literals like "open", "closed", "merged", "queued", "running", "success", or "failed" is updated to use the corresponding enum variant. The test calls to finish_branch_ci_lane_run and finish_nightly_lane_run also pass CiLaneStatus::Success or CiLaneStatus::Failed instead of string arguments.

This is a large mechanical change (dozens of test sites) but it validates that the entire read-write cycle works correctly: enums go into store methods, are persisted as strings in SQLite, are read back and parsed into enums, and can be compared in assertions.

Diff