Repository navigation
feat(m27): deterministic run triage buckets and queue ordering - #49
Merged
Merged
Conversation
- Add run_staleness module with derive_staleness function - Add RunStaleness enum (fresh/aging/stale) - Add staleness fields to RunSummary: age_days, is_stale, staleness_reason, staleness_bucket - Add staleness filters to RunsListParams: stale_only, fresh_only, sort_by_staleness, today - Handler derives staleness from updated_at timestamp against reference date - Staleness rules: fresh (<=3 days), aging (4-7 days), stale (>7 days) - No backend autonomy added - purely deterministic derived fields
- Add RunTriageBucket enum with buckets: critical, attention, ready, blocked, deferred, done - Add run_triage.rs core module with derive_triage() function - Wire triage derivation into handle_run_get and handle_runs_list - Add triage_bucket_filter and sort_by_triage to runs.list - Add triage_bucket/triage_reason to run.get result - Add staleness and triage fields to RunSummary - Sort by triage bucket rank: critical > attention > ready > blocked > deferred > done No backend autonomy, timers, or background behavior.
Reviewer's GuideImplements deterministic staleness and triage derivation for runs, exposes the new buckets and reasons on run summaries and run.get, and wires them into runs.list filtering and sorting. Sequence diagram for runs.list with deterministic staleness and triagesequenceDiagram
actor Client
participant Daemon as DeterministicDaemonHandlers
participant Store
participant Staleness as RunStalenessModule
participant Triage as RunTriageModule
Client->>Daemon: handle_runs_list(RunsListParams)
Daemon->>Store: query_runs(params)
Store-->>Daemon: Vec RunSummary (no staleness/triage)
Daemon->>Daemon: compute reference_date (Utc::now)
loop for each run in runs
Daemon->>Staleness: derive_staleness(run.updated_at, reference_date)
Staleness-->>Daemon: RunStalenessSummary
Daemon->>Daemon: set age_days, is_stale, staleness_reason, staleness_bucket
end
loop for each run in runs
Daemon->>Triage: derive_triage(status, is_archived, is_snoozed, is_blocked, priority, due_date, staleness_bucket, reference_date)
Triage-->>Daemon: RunTriageSummary
Daemon->>Daemon: set triage_bucket, triage_reason
end
alt triage_bucket_filter set
Daemon->>Daemon: retain runs where triage_bucket == filter
end
alt stale_only
Daemon->>Daemon: retain runs where is_stale == true
end
alt fresh_only
Daemon->>Daemon: retain runs where is_stale == false
end
alt sort_by_triage
Daemon->>Daemon: sort runs by RunTriageBucket.sort_key()
end
alt sort_by_staleness
Daemon->>Daemon: sort runs by age_days descending
end
Daemon-->>Client: RunsListResult (runs with staleness and triage)
Sequence diagram for run.get with staleness and triage derivationsequenceDiagram
actor Client
participant Daemon as DeterministicDaemonHandlers
participant Store
participant Staleness as RunStalenessModule
participant Triage as RunTriageModule
Client->>Daemon: handle_run_get(run_id)
Daemon->>Store: load_run_state(run_id)
Store-->>Daemon: RunState
Daemon->>Daemon: compute reference_date (Utc::now)
Daemon->>Daemon: is_archived = archive_metadata.is_some()
Daemon->>Staleness: derive_staleness(state.updated_at, reference_date)
Staleness-->>Daemon: RunStalenessSummary
Daemon->>Triage: derive_triage(state.status, is_archived, snoozed, has_blockers, priority, due_date, Some(staleness_bucket), reference_date)
Triage-->>Daemon: RunTriageSummary
Daemon->>Daemon: triage_bucket = Some(triage_bucket)
Daemon->>Daemon: triage_reason = Some(triage_reason)
Daemon-->>Client: RunGetResult (includes triage_bucket, triage_reason)
Class diagram for new staleness and triage types and updated run API modelsclassDiagram
class RunSummary {
+effort: Option~RunEffort~
+age_days: Option~u32~
+is_stale: Option~bool~
+staleness_reason: Option~String~
+staleness_bucket: Option~RunStaleness~
+triage_bucket: Option~RunTriageBucket~
+triage_reason: Option~String~
+created_at: String
+updated_at: String
}
class RunsListParams {
+workspace_id: String
+status_filter: Option~String~
+limit: Option~u32~
+offset: Option~u32~
+sort_by_effort: Option~bool~
+stale_only: Option~bool~
+fresh_only: Option~bool~
+sort_by_staleness: Option~bool~
+today: Option~String~
+triage_bucket_filter: Option~RunTriageBucket~
+sort_by_triage: Option~bool~
}
class RunGetResult {
+run_state: RunState
+pending_approvals: Vec~Approval~
+blocking_run_ids: Vec~String~
+blocking_run_count: Option~u32~
+blocking_reason: Option~String~
+effort: Option~RunEffort~
+triage_bucket: Option~RunTriageBucket~
+triage_reason: Option~String~
}
class RunStaleness {
<<enumeration>>
Fresh
Aging
Stale
+as_str() str
+parse(s: &str) Option~RunStaleness~
+sort_key() i64
}
class RunTriageBucket {
<<enumeration>>
Critical
Attention
Ready
Blocked
Deferred
Done
+sort_key() i64
}
class RunStalenessSummary {
+age_days: usize
+is_stale: bool
+staleness_reason: String
+staleness_bucket: RunStaleness
}
class RunTriageSummary {
+triage_bucket: RunTriageBucket
+triage_reason: String
}
class RunStalenessModule {
+derive_staleness(updated_at: &str, reference_date: &str) RunStalenessSummary
-compute_age_days(updated_at: &str, reference_date: &str) usize
}
class RunTriageModule {
+derive_triage(status: &str, is_archived: bool, is_snoozed: bool, is_blocked: bool, priority: &str, due_date: Option~&str~, staleness_bucket: Option~RunStaleness~, reference_date: &str) RunTriageSummary
-is_past_due(due_date: &str, reference_date: &str) bool
}
RunSummary --> RunStaleness
RunSummary --> RunTriageBucket
RunGetResult --> RunTriageBucket
RunsListParams --> RunTriageBucket
RunStalenessSummary --> RunStaleness
RunTriageSummary --> RunTriageBucket
RunStalenessModule --> RunStalenessSummary
RunTriageModule --> RunTriageSummary
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The new
RunsListParams.todayreference date is never used inhandle_runs_list(the handler always usesUtc::now()), which makes results time-dependent and harder to control in callers/tests; consider wiringtodaythrough toderive_staleness/derive_triagewhen provided and only falling back toUtc::now()if it isNone. - In
handle_runs_list, bothsort_by_triageandsort_by_stalenesscan be enabled, but sinceVec::sort_byis not stable the later sort completely overrides the earlier one; if you expect combined ordering, consider either enforcing mutual exclusivity or expressing a single composite sort key.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `RunsListParams.today` reference date is never used in `handle_runs_list` (the handler always uses `Utc::now()`), which makes results time-dependent and harder to control in callers/tests; consider wiring `today` through to `derive_staleness`/`derive_triage` when provided and only falling back to `Utc::now()` if it is `None`.
- In `handle_runs_list`, both `sort_by_triage` and `sort_by_staleness` can be enabled, but since `Vec::sort_by` is not stable the later sort completely overrides the earlier one; if you expect combined ordering, consider either enforcing mutual exclusivity or expressing a single composite sort key.
## Individual Comments
### Comment 1
<location path="codex-rs/deterministic-daemon/src/handlers.rs" line_range="595-596" />
<code_context>
+ }
+ // Milestone 27: sort_by_triage — deterministic bucket ordering:
+ // critical → attention → ready → blocked → deferred → done.
+ if p.sort_by_triage.unwrap_or(false) {
+ runs.sort_by(|a, b| {
+ let a_rank = a.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999);
+ let b_rank = b.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999);
</code_context>
<issue_to_address>
**issue (bug_risk):** Triage sorting currently orders lowest-priority buckets first instead of highest-priority first.
`RunTriageBucket::sort_key` assigns higher numbers to higher-priority buckets (Critical=5 … Done=0), but the current `a_rank.cmp(&b_rank)` sorts ascending, so Done appears before Critical. To match the documented order (critical → attention → ready → blocked → deferred → done), sort by descending rank (e.g., `b_rank.cmp(&a_rank)`) or invert the sort_key mapping so lower values represent higher priority and keep the ascending sort.
</issue_to_address>
### Comment 2
<location path="codex-rs/deterministic-daemon/src/handlers.rs" line_range="555-557" />
<code_context>
});
}
+ // Milestone 26: derive staleness for each run.
+ let reference_date = chrono::Utc::now().format("%Y-%m-%d").to_string();
+ for run in &mut runs {
+ let staleness = deterministic_core::run_staleness::derive_staleness(
+ &run.updated_at,
+ &reference_date,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The staleness reference date ignores the `today` parameter from `RunsListParams`, reducing determinism and making the API field effectively unused.
`RunsListParams::today` is never used because `handle_runs_list` always sets `reference_date` from `Utc::now()`. This makes behavior time-dependent and ignores the API parameter. Use the supplied `today` when present, falling back to current UTC otherwise, e.g.:
```rust
let reference_date = p
.today
.clone()
.unwrap_or_else(|| chrono::Utc::now().format("%Y-%m-%d").to_string());
```
```suggestion
// Milestone 26: derive staleness for each run, using the supplied `today` override when present.
let reference_date = p
.today
.clone()
.unwrap_or_else(|| Utc::now().format("%Y-%m-%d").to_string());
for run in &mut runs {
```
</issue_to_address>
### Comment 3
<location path="codex-rs/deterministic-daemon/src/handlers.rs" line_range="574-560" />
<code_context>
+ run.is_archived.unwrap_or(false),
+ run.is_snoozed.unwrap_or(false),
+ run.is_blocked.unwrap_or(false),
+ &run.priority.to_string(),
+ run.due_date.as_deref(),
+ run.staleness_bucket,
+ &reference_date,
+ );
+ run.triage_bucket = Some(triage.triage_bucket);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `priority.to_string()` and string equality for triage logic couples behavior to the `Display` format.
Here `derive_triage` is given `&run.priority.to_string()`, and its rules rely on `priority == "urgent"`. That makes behavior depend on the `Display` implementation, which is meant for humans and may change (capitalization, localization, etc.), silently breaking triage. Prefer passing the priority enum/internal discriminant, or a dedicated stable API like `priority.as_str()` intended for programmatic comparison.
Suggested implementation:
```rust
let triage = deterministic_core::run_triage::derive_triage(
&run.status,
run.is_archived.unwrap_or(false),
run.is_snoozed.unwrap_or(false),
run.is_blocked.unwrap_or(false),
run.priority.as_str(),
run.due_date.as_deref(),
run.staleness_bucket,
&reference_date,
);
```
To fully implement this change, you will also need to:
1. Ensure the `priority` type has a stable programmatic API:
- Add an `as_str(&self) -> &'static str` method (or equivalent) on the priority enum/type, returning stable identifiers such as `"urgent"`, `"high"`, etc.
2. Adjust the `deterministic_core::run_triage::derive_triage` signature to accept the new type if it currently expects `&str` from `to_string()`:
- If it currently takes `priority: &str`, you can keep that and just pass `priority.as_str()` as done above.
- If you decide to take the enum directly instead (e.g., `priority: Priority` or `&Priority`), update the call site here accordingly (e.g., pass `&run.priority`) and update the triage logic to match on the enum variants instead of string equality.
3. Update any other call sites of `derive_triage` that currently pass `priority.to_string()` to use the new stable API as well, for consistency.
</issue_to_address>
### Comment 4
<location path="codex-rs/deterministic-daemon/src/handlers.rs" line_range="556" />
<code_context>
});
}
+ // Milestone 26: derive staleness for each run.
+ let reference_date = chrono::Utc::now().format("%Y-%m-%d").to_string();
+ for run in &mut runs {
+ let staleness = deterministic_core::run_staleness::derive_staleness(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring shared date resolution, run traversal, filtering, and sorting into small helpers to keep the handlers simpler and more cohesive.
You can keep all the new functionality but reduce complexity by:
### 1. Resolve `reference_date` once and share it
Centralize the "params or now" logic and use it in both handlers:
```rust
fn resolve_reference_date(param_today: Option<String>) -> String {
if let Some(today) = param_today {
today
} else {
Utc::now().format("%Y-%m-%d").to_string()
}
}
```
Then:
```rust
fn handle_runs_list(...) -> Result<...> {
// ...
let reference_date = resolve_reference_date(p.today.clone());
// use reference_date below
}
```
```rust
fn handle_run_get(...) -> Result<...> {
// ...
let reference_date = resolve_reference_date(None); // or a param if added later
let staleness = derive_staleness(&state.updated_at, &reference_date);
let triage = derive_triage(
&state.status,
is_archived,
state.snooze_metadata.is_some(),
!state.blocked_by_run_ids.is_empty(),
&state.priority.to_string(),
state.due_date.as_deref(),
Some(staleness.staleness_bucket),
&reference_date,
);
// ...
}
```
This removes duplicated `Utc::now()` logic and makes the reference date behavior explicit.
### 2. Combine the two passes over `runs`
You can derive staleness and triage in a single loop, preserving the same semantics:
```rust
let reference_date = resolve_reference_date(p.today.clone());
for run in &mut runs {
let staleness = derive_staleness(&run.updated_at, &reference_date);
run.age_days = Some(staleness.age_days as u32);
run.is_stale = Some(staleness.is_stale);
run.staleness_reason = Some(staleness.staleness_reason);
run.staleness_bucket = Some(staleness.staleness_bucket);
let triage = derive_triage(
&run.status,
run.is_archived.unwrap_or(false),
run.is_snoozed.unwrap_or(false),
run.is_blocked.unwrap_or(false),
&run.priority.to_string(),
run.due_date.as_deref(),
run.staleness_bucket,
&reference_date,
);
run.triage_bucket = Some(triage.triage_bucket);
run.triage_reason = Some(triage.triage_reason);
}
```
This keeps the same data on each run but avoids two tightly-coupled traversals.
### 3. Encode filter interactions in a single predicate
You can keep current behavior (including the “both stale_only and fresh_only → no results” edge case) but make it easier to reason about:
```rust
fn runs_filter_predicate(
run: &RunSummary,
triage_bucket_filter: &Option<RunTriageBucket>,
stale_only: bool,
fresh_only: bool,
) -> bool {
if let Some(bucket) = triage_bucket_filter {
if run.triage_bucket.as_ref() != Some(bucket) {
return false;
}
}
let is_stale = run.is_stale.unwrap_or(false);
if stale_only && !is_stale {
return false;
}
if fresh_only && is_stale {
return false;
}
true
}
```
Then in `handle_runs_list`:
```rust
let stale_only = p.stale_only.unwrap_or(false);
let fresh_only = p.fresh_only.unwrap_or(false);
runs.retain(|r| runs_filter_predicate(
r,
&p.triage_bucket_filter,
stale_only,
fresh_only,
));
```
This puts all filter composition in one place instead of scattered `retain` calls.
### 4. Encapsulate staleness sort semantics
Instead of inlining the `age_days` comparison in the handler, hide the “oldest first” rule behind a small helper so the handler doesn’t re-encode domain semantics:
```rust
fn staleness_sort_key(age_days: Option<u32>) -> (bool, u32) {
// `None` goes last, higher age_days = more urgent
match age_days {
Some(days) => (false, std::u32::MAX - days),
None => (true, 0),
}
}
```
Then:
```rust
if p.sort_by_staleness.unwrap_or(false) {
runs.sort_by_key(|r| staleness_sort_key(r.age_days));
}
```
Similarly, you could wrap the triage sort in a helper to keep the handler focused on orchestration rather than ranking details:
```rust
fn triage_sort_key(bucket: Option<RunTriageBucket>) -> u32 {
bucket.map(RunTriageBucket::sort_key).unwrap_or(999)
}
if p.sort_by_triage.unwrap_or(false) {
runs.sort_by_key(|r| triage_sort_key(r.triage_bucket));
}
```
These changes keep all existing behavior but make the staleness/triage logic more cohesive and easier to modify later.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+595
to
+596
| if p.sort_by_triage.unwrap_or(false) { | ||
| runs.sort_by(|a, b| { |
There was a problem hiding this comment.
issue (bug_risk): Triage sorting currently orders lowest-priority buckets first instead of highest-priority first.
RunTriageBucket::sort_key assigns higher numbers to higher-priority buckets (Critical=5 … Done=0), but the current a_rank.cmp(&b_rank) sorts ascending, so Done appears before Critical. To match the documented order (critical → attention → ready → blocked → deferred → done), sort by descending rank (e.g., b_rank.cmp(&a_rank)) or invert the sort_key mapping so lower values represent higher priority and keep the ascending sort.
Comment on lines
+555
to
+557
| // Milestone 26: derive staleness for each run. | ||
| let reference_date = chrono::Utc::now().format("%Y-%m-%d").to_string(); | ||
| for run in &mut runs { |
There was a problem hiding this comment.
suggestion (bug_risk): The staleness reference date ignores the today parameter from RunsListParams, reducing determinism and making the API field effectively unused.
RunsListParams::today is never used because handle_runs_list always sets reference_date from Utc::now(). This makes behavior time-dependent and ignores the API parameter. Use the supplied today when present, falling back to current UTC otherwise, e.g.:
let reference_date = p
.today
.clone()
.unwrap_or_else(|| chrono::Utc::now().format("%Y-%m-%d").to_string());
Suggested change
| // Milestone 26: derive staleness for each run. | |
| let reference_date = chrono::Utc::now().format("%Y-%m-%d").to_string(); | |
| for run in &mut runs { | |
| // Milestone 26: derive staleness for each run, using the supplied `today` override when present. | |
| let reference_date = p | |
| .today | |
| .clone() | |
| .unwrap_or_else(|| Utc::now().format("%Y-%m-%d").to_string()); | |
| for run in &mut runs { |
| for run in &mut runs { | ||
| let staleness = deterministic_core::run_staleness::derive_staleness( | ||
| &run.updated_at, | ||
| &reference_date, |
There was a problem hiding this comment.
suggestion (bug_risk): Using priority.to_string() and string equality for triage logic couples behavior to the Display format.
Here derive_triage is given &run.priority.to_string(), and its rules rely on priority == "urgent". That makes behavior depend on the Display implementation, which is meant for humans and may change (capitalization, localization, etc.), silently breaking triage. Prefer passing the priority enum/internal discriminant, or a dedicated stable API like priority.as_str() intended for programmatic comparison.
Suggested implementation:
let triage = deterministic_core::run_triage::derive_triage(
&run.status,
run.is_archived.unwrap_or(false),
run.is_snoozed.unwrap_or(false),
run.is_blocked.unwrap_or(false),
run.priority.as_str(),
run.due_date.as_deref(),
run.staleness_bucket,
&reference_date,
);To fully implement this change, you will also need to:
- Ensure the
prioritytype has a stable programmatic API:- Add an
as_str(&self) -> &'static strmethod (or equivalent) on the priority enum/type, returning stable identifiers such as"urgent","high", etc.
- Add an
- Adjust the
deterministic_core::run_triage::derive_triagesignature to accept the new type if it currently expects&strfromto_string():- If it currently takes
priority: &str, you can keep that and just passpriority.as_str()as done above. - If you decide to take the enum directly instead (e.g.,
priority: Priorityor&Priority), update the call site here accordingly (e.g., pass&run.priority) and update the triage logic to match on the enum variants instead of string equality.
- If it currently takes
- Update any other call sites of
derive_triagethat currently passpriority.to_string()to use the new stable API as well, for consistency.
| }); | ||
| } | ||
| // Milestone 26: derive staleness for each run. | ||
| let reference_date = chrono::Utc::now().format("%Y-%m-%d").to_string(); |
There was a problem hiding this comment.
issue (complexity): Consider refactoring shared date resolution, run traversal, filtering, and sorting into small helpers to keep the handlers simpler and more cohesive.
You can keep all the new functionality but reduce complexity by:
1. Resolve reference_date once and share it
Centralize the "params or now" logic and use it in both handlers:
fn resolve_reference_date(param_today: Option<String>) -> String {
if let Some(today) = param_today {
today
} else {
Utc::now().format("%Y-%m-%d").to_string()
}
}Then:
fn handle_runs_list(...) -> Result<...> {
// ...
let reference_date = resolve_reference_date(p.today.clone());
// use reference_date below
}fn handle_run_get(...) -> Result<...> {
// ...
let reference_date = resolve_reference_date(None); // or a param if added later
let staleness = derive_staleness(&state.updated_at, &reference_date);
let triage = derive_triage(
&state.status,
is_archived,
state.snooze_metadata.is_some(),
!state.blocked_by_run_ids.is_empty(),
&state.priority.to_string(),
state.due_date.as_deref(),
Some(staleness.staleness_bucket),
&reference_date,
);
// ...
}This removes duplicated Utc::now() logic and makes the reference date behavior explicit.
2. Combine the two passes over runs
You can derive staleness and triage in a single loop, preserving the same semantics:
let reference_date = resolve_reference_date(p.today.clone());
for run in &mut runs {
let staleness = derive_staleness(&run.updated_at, &reference_date);
run.age_days = Some(staleness.age_days as u32);
run.is_stale = Some(staleness.is_stale);
run.staleness_reason = Some(staleness.staleness_reason);
run.staleness_bucket = Some(staleness.staleness_bucket);
let triage = derive_triage(
&run.status,
run.is_archived.unwrap_or(false),
run.is_snoozed.unwrap_or(false),
run.is_blocked.unwrap_or(false),
&run.priority.to_string(),
run.due_date.as_deref(),
run.staleness_bucket,
&reference_date,
);
run.triage_bucket = Some(triage.triage_bucket);
run.triage_reason = Some(triage.triage_reason);
}This keeps the same data on each run but avoids two tightly-coupled traversals.
3. Encode filter interactions in a single predicate
You can keep current behavior (including the “both stale_only and fresh_only → no results” edge case) but make it easier to reason about:
fn runs_filter_predicate(
run: &RunSummary,
triage_bucket_filter: &Option<RunTriageBucket>,
stale_only: bool,
fresh_only: bool,
) -> bool {
if let Some(bucket) = triage_bucket_filter {
if run.triage_bucket.as_ref() != Some(bucket) {
return false;
}
}
let is_stale = run.is_stale.unwrap_or(false);
if stale_only && !is_stale {
return false;
}
if fresh_only && is_stale {
return false;
}
true
}Then in handle_runs_list:
let stale_only = p.stale_only.unwrap_or(false);
let fresh_only = p.fresh_only.unwrap_or(false);
runs.retain(|r| runs_filter_predicate(
r,
&p.triage_bucket_filter,
stale_only,
fresh_only,
));This puts all filter composition in one place instead of scattered retain calls.
4. Encapsulate staleness sort semantics
Instead of inlining the age_days comparison in the handler, hide the “oldest first” rule behind a small helper so the handler doesn’t re-encode domain semantics:
fn staleness_sort_key(age_days: Option<u32>) -> (bool, u32) {
// `None` goes last, higher age_days = more urgent
match age_days {
Some(days) => (false, std::u32::MAX - days),
None => (true, 0),
}
}Then:
if p.sort_by_staleness.unwrap_or(false) {
runs.sort_by_key(|r| staleness_sort_key(r.age_days));
}Similarly, you could wrap the triage sort in a helper to keep the handler focused on orchestration rather than ranking details:
fn triage_sort_key(bucket: Option<RunTriageBucket>) -> u32 {
bucket.map(RunTriageBucket::sort_key).unwrap_or(999)
}
if p.sort_by_triage.unwrap_or(false) {
runs.sort_by_key(|r| triage_sort_key(r.triage_bucket));
}These changes keep all existing behavior but make the staleness/triage logic more cohesive and easier to modify later.
There was a problem hiding this comment.
Pull request overview
Adds deterministic, derived staleness + triage metadata to runs, and exposes new runs.list filtering/sorting capabilities to support queue triage ordering in the deterministic daemon/protocol.
Changes:
- Extend protocol types with staleness + triage fields (
RunStaleness,RunTriageBucket, newRunsListParamsflags, newRunSummary/RunGetResultfields). - Add deterministic-core modules to derive staleness and triage (
run_staleness.rs,run_triage.rs). - Wire derivations into daemon handlers for
runs.listandrun.get, including new filters and sorts.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| codex-rs/deterministic-protocol/src/types.rs | Adds wire types/fields for staleness + triage, plus list params for filtering/sorting. |
| codex-rs/deterministic-daemon/src/persistence.rs | Initializes new RunSummary staleness/triage fields to None (handler-populated). |
| codex-rs/deterministic-daemon/src/handlers.rs | Derives staleness/triage in runs.list and run.get, adds filters and new sort options. |
| codex-rs/deterministic-core/src/run_triage.rs | Implements deterministic triage bucket derivation (new module + unit tests). |
| codex-rs/deterministic-core/src/run_staleness.rs | Implements deterministic staleness derivation (new module + unit tests). |
| codex-rs/deterministic-core/src/lib.rs | Exposes the new core modules. |
You can also share your feedback on Copilot code review. Take the survey.
| runs.sort_by(|a, b| { | ||
| let a_rank = a.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999); | ||
| let b_rank = b.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999); | ||
| a_rank.cmp(&b_rank) |
| runs.sort_by(|a, b| { | ||
| let a_rank = a.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999); | ||
| let b_rank = b.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999); | ||
| a_rank.cmp(&b_rank) |
Comment on lines
+45
to
+53
| pub fn derive_triage( | ||
| status: &str, | ||
| is_archived: bool, | ||
| is_snoozed: bool, | ||
| is_blocked: bool, | ||
| priority: &str, | ||
| due_date: Option<&str>, | ||
| staleness_bucket: Option<RunStaleness>, | ||
| reference_date: &str, |
Comment on lines
+106
to
+115
| // 5. attention: urgent OR needs_attention (status indicates attention needed) | ||
| if priority == "urgent" || status.contains("attention") || status.contains("waiting") { | ||
| return RunTriageSummary { | ||
| triage_bucket: RunTriageBucket::Attention, | ||
| triage_reason: if priority == "urgent" { | ||
| "urgent priority".to_string() | ||
| } else { | ||
| "needs attention".to_string() | ||
| }, | ||
| }; |
Comment on lines
+42
to
+108
| // Try to parse dates and compute age | ||
| let age_days = compute_age_days(updated_at, reference_date); | ||
|
|
||
| // Determine staleness bucket and reason | ||
| let (staleness_bucket, staleness_reason, is_stale) = if age_days <= 3 { | ||
| ( | ||
| RunStaleness::Fresh, | ||
| format!("updated {age_days} day(s) ago"), | ||
| false, | ||
| ) | ||
| } else if age_days <= 7 { | ||
| ( | ||
| RunStaleness::Aging, | ||
| format!("updated {age_days} days ago"), | ||
| false, | ||
| ) | ||
| } else { | ||
| ( | ||
| RunStaleness::Stale, | ||
| format!("stale: {age_days} days since update"), | ||
| true, | ||
| ) | ||
| }; | ||
|
|
||
| RunStalenessSummary { | ||
| age_days, | ||
| is_stale, | ||
| staleness_reason, | ||
| staleness_bucket, | ||
| } | ||
| } | ||
|
|
||
| /// Compute the number of days between updated_at and reference_date. | ||
| /// | ||
| /// Both should be ISO 8601 format: | ||
| /// - `updated_at`: "2024-01-15T10:30:00Z" (full timestamp) | ||
| /// - `reference_date`: "2024-01-18" (date only) | ||
| /// | ||
| /// Returns 0 if parsing fails. | ||
| fn compute_age_days(updated_at: &str, reference_date: &str) -> usize { | ||
| // Parse the reference date (YYYY-MM-DD) | ||
| let ref_date = match chrono::NaiveDate::parse_from_str(reference_date, "%Y-%m-%d") { | ||
| Ok(d) => d, | ||
| Err(_) => return 0, | ||
| }; | ||
|
|
||
| // Parse the updated_at timestamp - extract just the date part | ||
| let updated_date_str = if let Some(d) = updated_at.split('T').next() { | ||
| d | ||
| } else { | ||
| return 0; | ||
| }; | ||
|
|
||
| let updated_date = match chrono::NaiveDate::parse_from_str(updated_date_str, "%Y-%m-%d") { | ||
| Ok(d) => d, | ||
| Err(_) => return 0, | ||
| }; | ||
|
|
||
| // Compute difference in days | ||
| let diff = ref_date.signed_duration_since(updated_date); | ||
| let days = diff.num_days(); | ||
|
|
||
| // Return 0 for negative (future dates) or invalid | ||
| if days < 0 { | ||
| 0 | ||
| } else { | ||
| days as usize |
Comment on lines
+1990
to
+1991
| /// - deferred: snoozed, archived, or otherwise not active | ||
| /// - done: finalized runs |
Comment on lines
+582
to
+605
| // Milestone 27: triage_bucket_filter — keep only runs in the requested bucket. | ||
| if let Some(ref bucket_filter) = p.triage_bucket_filter { | ||
| runs.retain(|r| r.triage_bucket.as_ref() == Some(bucket_filter)); | ||
| } | ||
| // Milestone 27: stale_only and fresh_only filters. | ||
| if p.stale_only.unwrap_or(false) { | ||
| runs.retain(|r| r.is_stale.unwrap_or(false)); | ||
| } | ||
| if p.fresh_only.unwrap_or(false) { | ||
| runs.retain(|r| !r.is_stale.unwrap_or(true)); | ||
| } | ||
| // Milestone 27: sort_by_triage — deterministic bucket ordering: | ||
| // critical → attention → ready → blocked → deferred → done. | ||
| if p.sort_by_triage.unwrap_or(false) { | ||
| runs.sort_by(|a, b| { | ||
| let a_rank = a.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999); | ||
| let b_rank = b.triage_bucket.map(RunTriageBucket::sort_key).unwrap_or(999); | ||
| a_rank.cmp(&b_rank) | ||
| }); | ||
| } | ||
| // Milestone 27: sort_by_staleness — oldest first (higher age_days = more urgent). | ||
| if p.sort_by_staleness.unwrap_or(false) { | ||
| runs.sort_by(|a, b| b.age_days.cmp(&a.age_days)); | ||
| } |
Comment on lines
+555
to
+561
| // Milestone 26: derive staleness for each run. | ||
| let reference_date = chrono::Utc::now().format("%Y-%m-%d").to_string(); | ||
| for run in &mut runs { | ||
| let staleness = deterministic_core::run_staleness::derive_staleness( | ||
| &run.updated_at, | ||
| &reference_date, | ||
| ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RunTriageBucketenum with buckets:critical,attention,ready,blocked,deferred,donerun_triage.rscore module with deterministicderive_triage()functionhandle_run_getandhandle_runs_listtriage_bucket_filterandsort_by_triagetoruns.listtriage_bucket/triage_reasontorun.getresultRunSummaryTriage Precedence
done: archived OR finalizeddeferred: snoozedblocked: has blockerscritical: urgent + (overdue OR stale)attention: urgent OR needs_attentionready: not blocked, not urgent, not stale, not snoozeddeferred: fallbackTest Plan
Summary by Sourcery
Introduce deterministic staleness and triage classification for runs and expose them through list and get APIs.
New Features:
Enhancements: