Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/deterministic-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ pub mod run_set_priority;
pub mod run_assign_owner;
pub mod run_set_due_date;
pub mod run_set_dependencies;
pub mod run_readiness;
pub mod tests_run;
pub mod workspace_summary;
3 changes: 2 additions & 1 deletion codex-rs/deterministic-core/src/run_readiness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub struct RunReadinessSummary {
/// - Overdue visible run (requires `today` date for comparison) → `"overdue"`
///
/// No timers, notifications, escalations, or state mutations are performed.
#[allow(clippy::too_many_arguments, clippy::collapsible_if)]
pub fn derive_readiness(
status: &str,
is_archived: bool,
Expand All @@ -62,7 +63,7 @@ pub fn derive_readiness(
let reason = if blocked_by_count == 1 {
"blocked by 1 run".to_string()
} else {
format!("blocked by {} runs", blocked_by_count)
format!("blocked by {blocked_by_count} runs")
};
(false, Some(reason))
} else {
Expand Down
162 changes: 160 additions & 2 deletions codex-rs/deterministic-daemon/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ pub fn dispatch(
Method::RunSetDueDate => handle_run_set_due_date(params, store),
// Milestone 21: deterministic run dependency links
Method::RunSetDependencies => handle_run_set_dependencies(params, store),
// Milestone 24: deterministic queue overview
Method::RunsQueueOverview => handle_runs_queue_overview(params, store),
}
}

Expand Down Expand Up @@ -518,7 +520,7 @@ fn handle_runs_list(
run.is_blocking = Some(count > 0);
run.blocking_run_count = Some(count);
run.blocking_reason = if count > 0 {
Some(format!("blocking {} run(s)", count))
Some(format!("blocking {count} run(s)"))
} else {
None
};
Expand Down Expand Up @@ -573,7 +575,7 @@ fn handle_run_get(
let is_blocking = Some(blocking_count > 0);
let blocking_run_count = Some(blocking_count);
let blocking_reason = if blocking_count > 0 {
Some(format!("blocking {} run(s)", blocking_count))
Some(format!("blocking {blocking_count} run(s)"))
} else {
None
};
Expand Down Expand Up @@ -1270,6 +1272,162 @@ fn handle_run_set_dependencies(
Ok((serde_json::to_value(result)?, Some(state)))
}

// ---------------------------------------------------------------------------
// runs.overview (Milestone 24)

#[allow(clippy::collapsible_if)]
/// Handle the runs.overview method - deterministic queue overview.
fn handle_runs_queue_overview(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider simplifying handle_runs_queue_overview by extracting small helpers, cleaning up redundant branches, and either properly tracking or removing unused counters to keep the function shallow and focused.

You can simplify handle_runs_queue_overview without changing behavior by extracting a few helpers and removing redundant branching/state:

  1. Unconditional total_count increment

The if readiness.is_ready { total_count += 1 } else { total_count += 1 } adds unnecessary branching:

// before
if readiness.is_ready {
    ready_count += 1;
    total_count += 1;
} else {
    total_count += 1;
}

// after (same behavior)
if readiness.is_ready {
    ready_count += 1;
}
total_count += 1;
  1. Flatten overdue computation

You can avoid nested if today.is_some() / if let chains by matching both today and due_date at once:

// before
if today.is_some() {
    if let Some(due) = &run.due_date {
        if let Some(t) = today {
            if due.as_str() < t {
                overdue_count += 1;
            }
        }
    }
}

// after (equivalent)
if let (Some(t), Some(due)) = (today, run.due_date.as_deref()) {
    if due < t {
        overdue_count += 1;
    }
}
  1. Status prefix helper

The status prefix logic is inline in the loop; extracting it makes the loop easier to scan and reuse:

// helper
fn status_prefix(status: &str) -> &str {
    if status.starts_with("finalized:") {
        "finalized"
    } else if status.starts_with("awaiting_approval:") {
        "awaiting_approval"
    } else if status.starts_with("pending:") {
        "pending"
    } else {
        status
    }
}

// usage in loop
let status_prefix = status_prefix(&run.status);
*status_counts.entry(status_prefix.to_string()).or_insert(0) += 1;
  1. Archived count either track or drop

archived_count is currently always 0, which is misleading when exposed:

  • If you want to track it, increment inside the loop:
let mut archived_count = 0;

for run in &all_runs {
    let is_archived = run.is_archived.unwrap_or(false);
    if is_archived {
        archived_count += 1;
    }
    // ...
}

let result = RunQueueOverview {
    // ...
    archived_count: if include_archived { Some(archived_count) } else { None },
    // ...
};
  • If you don’t need this metric, remove the local variable and the field from RunQueueOverview to reduce mental overhead.
  1. Optional: small aggregation helper

If the loop keeps growing, consider extracting the per-run aggregation into a small function to keep handle_runs_queue_overview focused on I/O and parameter handling:

struct Aggregates {
    total: usize,
    ready: usize,
    // ... other counters
}

fn accumulate_run(agg: &mut Aggregates, run: &RunSummary, today: Option<&str>) {
    // counting logic here (ready, blocked, overdue, maps, etc.)
}

Then in the handler, you just initialize Aggregates and call accumulate_run in the loop, keeping the top-level function shallow.

params: serde_json::Value,
store: &Store,
) -> Result<(serde_json::Value, Option<RunState>)> {
let p: RunsQueueOverviewParams = serde_json::from_value(params)?;

Comment on lines +1278 to +1285
let include_archived = p.include_archived.unwrap_or(false);
let include_snoozed = p.include_snoozed.unwrap_or(false);
let today = p.today.as_deref();

// Use list_runs with high limit to get all runs for overview
// This uses the existing filter semantics from RunsListParams
let archived_only = false; // We handle filtering ourselves
let all_runs = store.list_runs(
10000, // High limit to get all runs
Comment on lines +1290 to +1294
p.workspace_id.as_deref(),
None, // status_filter - none for overview
include_archived,
archived_only,
None, // label_filter
false, // pinned_only
include_snoozed,
false, // snoozed_only
)?;

// Use run_readiness module to derive readiness
use deterministic_core::run_readiness::derive_readiness;

let mut total_count = 0;
let mut ready_count = 0;
let mut blocked_count = 0;
let mut attention_count = 0;
let mut pinned_count = 0;
let mut snoozed_count = 0;
let mut overdue_count = 0;
let archived_count = 0;
Comment on lines +1314 to +1315

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Archived count is always zero, making archived_count misleading when include_archived is true.

archived_count is initialized but never updated, yet the result always returns Some(0) when include_archived is true. This exposes a bogus count to callers. Either increment archived_count whenever an archived item is included, or update the API/docs so this field is not returned (or is clearly marked) when the value is not meaningful.


Comment on lines +1314 to +1316
let mut priority_counts = std::collections::HashMap::new();
let mut assignee_counts = std::collections::HashMap::new();
let mut status_counts = std::collections::HashMap::new();

for run in &all_runs {
let is_archived = run.is_archived.unwrap_or(false);
let is_snoozed = run.is_snoozed.unwrap_or(false);
let is_pinned = run.is_pinned.unwrap_or(false);
let blocked_by_count = run.blocked_by_count.unwrap_or(0);

// Skip based on filters (list_runs handles SQL-level, we handle in-memory)
if is_snoozed && !include_snoozed {
continue;
}

// Count snoozed
if is_snoozed {
snoozed_count += 1;
}

// Count pinned
if is_pinned {
pinned_count += 1;
}

// Count blocked
if blocked_by_count > 0 {
blocked_count += 1;
}

// Derive readiness using the deterministic function
let readiness = derive_readiness(
&run.status,
is_archived,
is_snoozed,
blocked_by_count,
run.priority,
run.due_date.as_deref(),
is_pinned,
today,
);

// Count ready
if readiness.is_ready {
ready_count += 1;
total_count += 1;
} else {
total_count += 1;
}

// Count needs attention
if readiness.needs_attention {
attention_count += 1;
}

// Count overdue
if today.is_some() {
if let Some(due) = &run.due_date {
if let Some(t) = today {
if due.as_str() < t {
overdue_count += 1;
}
}
}
}
Comment on lines +1372 to +1381

// Priority counts
let priority_str = match run.priority {
deterministic_protocol::RunPriority::Low => "low",
deterministic_protocol::RunPriority::Normal => "normal",
deterministic_protocol::RunPriority::High => "high",
deterministic_protocol::RunPriority::Urgent => "urgent",
};
*priority_counts.entry(priority_str.to_string()).or_insert(0) += 1;

// Assignee counts
let assignee = run.assignee.clone().unwrap_or_else(|| "unassigned".to_string());
*assignee_counts.entry(assignee).or_insert(0) += 1;

// Status counts - use prefix
let status_prefix = if run.status.starts_with("finalized:") {
"finalized"
} else if run.status.starts_with("awaiting_approval:") {
"awaiting_approval"
} else if run.status.starts_with("pending:") {
"pending"
} else {
&run.status
};
*status_counts.entry(status_prefix.to_string()).or_insert(0) += 1;
}

let result = RunQueueOverview {
total_visible: total_count,
ready_count,
blocked_count,
needs_attention_count: attention_count,
pinned_count,
snoozed_count,
overdue_count: if today.is_some() { Some(overdue_count) } else { None },
archived_count: if include_archived { Some(archived_count) } else { None },
by_priority: PriorityCounts {
Comment on lines +1409 to +1418
low: *priority_counts.get("low").unwrap_or(&0),
normal: *priority_counts.get("normal").unwrap_or(&0),
high: *priority_counts.get("high").unwrap_or(&0),
urgent: *priority_counts.get("urgent").unwrap_or(&0),
},
by_assignee: assignee_counts,
by_status: status_counts,
};

Ok((serde_json::to_value(result)?, None))
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
1 change: 1 addition & 0 deletions codex-rs/deterministic-daemon/src/persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,7 @@ impl Store {
///
/// This is used by handlers to compute deterministic blocker-impact summaries (Milestone 23).
/// The map only contains run IDs that appear as blockers (count ≥ 1); absent entries mean zero.
#[allow(clippy::unwrap_used)]
pub fn get_blocker_impact_map(&self) -> Result<std::collections::HashMap<String, usize>> {
let conn = self.conn.lock().unwrap();
let mut stmt = conn.prepare(
Expand Down
8 changes: 8 additions & 0 deletions codex-rs/deterministic-protocol/src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ pub enum Method {
RunSetDueDate,
// Milestone 21: deterministic run dependency links
RunSetDependencies,
// Milestone 24: deterministic queue overview
RunsQueueOverview,
}

impl Method {
Expand Down Expand Up @@ -103,6 +105,8 @@ impl Method {
Self::RunSetDueDate => "run.set_due_date",
// Milestone 21
Self::RunSetDependencies => "run.set_dependencies",
// Milestone 24
Self::RunsQueueOverview => "runs.overview",
}
}

Expand Down Expand Up @@ -153,6 +157,8 @@ impl Method {
"run.set_due_date" => Some(Self::RunSetDueDate),
// Milestone 21
"run.set_dependencies" => Some(Self::RunSetDependencies),
// Milestone 24
"runs.overview" => Some(Self::RunsQueueOverview),
_ => None,
}
}
Expand Down Expand Up @@ -204,6 +210,8 @@ impl Method {
Self::RunSetDueDate,
// Milestone 21
Self::RunSetDependencies,
// Milestone 24
Self::RunsQueueOverview,
]
}
}
Expand Down
71 changes: 71 additions & 0 deletions codex-rs/deterministic-protocol/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,77 @@ pub struct RunsListResult {
pub count: usize,
}

// ---------------------------------------------------------------------------
// runs.overview (Milestone 24)

/// Parameters for requesting a queue overview summary.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct RunsQueueOverviewParams {
/// Filter by workspace ID (optional).
#[serde(default)]
pub workspace_id: Option<String>,
/// When true, archived runs are included in counts (Milestone 13).
/// Default: false (archived runs are excluded from visible counts).
#[serde(default)]
pub include_archived: Option<bool>,
/// When true, snoozed runs are included in counts (Milestone 17).
/// Default: false (snoozed runs are excluded from visible counts).
#[serde(default)]
pub include_snoozed: Option<bool>,
/// ISO date (`YYYY-MM-DD`) for computing overdue counts (Milestone 20).
/// When provided, counts will include overdue status based on this date.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub today: Option<String>,
}

/// Deterministic queue overview summary providing aggregate counts.
///
/// This is a read-only inspection operation that derives summary counts from
/// existing run state without mutating anything.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct RunQueueOverview {
/// Total count of visible (non-archived) runs matching the filter.
pub total_visible: usize,
Comment on lines +947 to +948

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): total_visible docs don't match behavior when include_archived is true.

handle_runs_queue_overview increments total_count for every run from list_runs, including archived ones when include_archived is true. As a result, total_visible is actually "total including archived" in that case. Please either (a) filter out archived runs when computing this value, or (b) update the field name and docs to clarify that archived runs are counted when requested.

/// Count of runs that are ready to work on now.
pub ready_count: usize,
/// Count of runs that are blocked by at least one other run.
pub blocked_count: usize,
/// Count of runs that warrant operator attention.
pub needs_attention_count: usize,
/// Count of runs that are currently pinned.
pub pinned_count: usize,
/// Count of runs that are currently snoozed.
pub snoozed_count: usize,
/// Count of runs with a due date in the past (overdue).
/// Only included when `today` parameter is provided.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub overdue_count: Option<usize>,
/// Count of archived runs (only included when include_archived is true).
#[serde(default, skip_serializing_if = "Option::is_none")]
pub archived_count: Option<usize>,
/// Counts grouped by priority level.
pub by_priority: PriorityCounts,
/// Counts grouped by assignee (key is assignee, value is count).
/// Unassigned runs are stored under key `"unassigned"`.
#[serde(default)]
pub by_assignee: std::collections::HashMap<String, usize>,
/// Counts grouped by status prefix (e.g., "awaiting_approval", "finalized", etc.).
#[serde(default)]
pub by_status: std::collections::HashMap<String, usize>,
Comment on lines +969 to +974
}

/// Priority-level counts for queue overview.
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
#[serde(rename_all = "camelCase")]
pub struct PriorityCounts {
pub low: usize,
pub normal: usize,
pub high: usize,
pub urgent: usize,
}

// ---------------------------------------------------------------------------
// run.get (Milestone 7)
// ---------------------------------------------------------------------------
Expand Down
Loading