Skip to content

Commit 94b72c3

Browse files
committed
fix(memory): describe task update deltas
1 parent 8fb07b8 commit 94b72c3

3 files changed

Lines changed: 162 additions & 10 deletions

File tree

docs/design-docs/working-memory-triage.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ Findings from CodeRabbit review + bug reports. Tracking resolution before merge.
5353
- [x] **R15 — UTF-8 panic on topic truncation** (`src/memory/working.rs:739`)
5454
Byte-index slice at 80 can split multibyte chars. **Fixed:** `floor_char_boundary(80)`.
5555

56-
- [ ] **R16 — Task update event always says "status change"** (`src/tools/task_update.rs:246`)
57-
Every update emits `"updated to <status>"` even for title/description edits. Compute actual delta.
56+
- [x] **R16 — Task update event always says "status change"** (`src/tools/task_update.rs:246`)
57+
Every update emits `"updated to <status>"` even for title/description edits. **Fixed in this slice:** task-update working-memory events now compare the before/after task record and name the actual changed fields, while preserving the existing status-only wording.
5858

5959
## Live Observations (from prompt inspect, March 19)
6060

src/tasks/store.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ impl std::fmt::Display for TaskPriority {
103103
}
104104
}
105105

106-
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, utoipa::ToSchema)]
106+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, utoipa::ToSchema)]
107107
pub struct TaskSubtask {
108108
pub title: String,
109109
pub completed: bool,
@@ -163,6 +163,7 @@ pub struct UpdateTaskInput {
163163

164164
#[derive(Debug, Clone)]
165165
pub struct TaskUpdateResult {
166+
pub previous_task: Task,
166167
pub previous_status: TaskStatus,
167168
pub task: Task,
168169
}
@@ -403,6 +404,7 @@ impl TaskStore {
403404
};
404405

405406
let current = task_from_row(row)?;
407+
let previous_task = current.clone();
406408
let previous_status = current.status;
407409
let task = Self::update_current_in_tx(&mut tx, task_number, current, input).await?;
408410

@@ -411,6 +413,7 @@ impl TaskStore {
411413
.context("failed to commit task update transaction")?;
412414

413415
Ok(Some(TaskUpdateResult {
416+
previous_task,
414417
previous_status,
415418
task,
416419
}))
@@ -458,6 +461,7 @@ impl TaskStore {
458461
};
459462

460463
let current = task_from_row(row)?;
464+
let previous_task = current.clone();
461465
let previous_status = current.status;
462466
let task = Self::update_current_in_tx(&mut tx, task_number, current, input).await?;
463467

@@ -467,6 +471,7 @@ impl TaskStore {
467471

468472
Ok(WorkerTaskUpdateResult::Updated(Box::new(
469473
TaskUpdateResult {
474+
previous_task,
470475
previous_status,
471476
task,
472477
},
@@ -1086,6 +1091,33 @@ mod tests {
10861091
);
10871092
}
10881093

1094+
#[tokio::test]
1095+
async fn update_result_returns_applied_snapshot_pair() {
1096+
let store = setup_store().await;
1097+
let created = store
1098+
.create(self_assigned_input("old title", TaskStatus::Backlog))
1099+
.await
1100+
.expect("task should be created");
1101+
1102+
let result = store
1103+
.update_with_status_transition(
1104+
created.task_number,
1105+
UpdateTaskInput {
1106+
title: Some("new title".to_string()),
1107+
priority: Some(TaskPriority::High),
1108+
..Default::default()
1109+
},
1110+
)
1111+
.await
1112+
.expect("update should succeed")
1113+
.expect("task should exist");
1114+
1115+
assert_eq!(result.previous_task.title, "old title");
1116+
assert_eq!(result.previous_task.priority, TaskPriority::Medium);
1117+
assert_eq!(result.task.title, "new title");
1118+
assert_eq!(result.task.priority, TaskPriority::High);
1119+
}
1120+
10891121
#[tokio::test]
10901122
async fn global_task_numbers_are_unique_across_agents() {
10911123
let store = setup_store().await;

src/tools/task_update.rs

Lines changed: 127 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Task update tool for branch and worker processes.
22
33
use crate::tasks::{
4-
TaskPriority, TaskStatus, TaskStore, TaskSubtask, UpdateTaskInput, WorkerTaskUpdateResult,
4+
Task, TaskPriority, TaskStatus, TaskStore, TaskSubtask, UpdateTaskInput, WorkerTaskUpdateResult,
55
};
66
use crate::{AgentId, WorkerId};
77
use rig::completion::ToolDefinition;
@@ -235,6 +235,7 @@ impl Tool for TaskUpdateTool {
235235
}
236236
},
237237
};
238+
let previous_task = update_result.previous_task;
238239
let previous_status = update_result.previous_status;
239240
let updated = update_result.task;
240241

@@ -250,10 +251,7 @@ impl Tool for TaskUpdateTool {
250251
} else {
251252
(
252253
crate::memory::WorkingMemoryEventType::TaskUpdate,
253-
format!(
254-
"Task #{} updated to {}",
255-
updated.task_number, updated.status
256-
),
254+
task_update_memory_summary(&previous_task, &updated),
257255
0.4,
258256
)
259257
};
@@ -272,13 +270,69 @@ impl Tool for TaskUpdateTool {
272270
}
273271
}
274272

273+
fn task_update_memory_summary(previous: &Task, updated: &Task) -> String {
274+
let mut changes = Vec::new();
275+
276+
if previous.status != updated.status {
277+
changes.push(format!("status {} -> {}", previous.status, updated.status));
278+
}
279+
if previous.priority != updated.priority {
280+
changes.push(format!(
281+
"priority {} -> {}",
282+
previous.priority, updated.priority
283+
));
284+
}
285+
if previous.title != updated.title {
286+
changes.push("title".to_string());
287+
}
288+
if previous.description != updated.description {
289+
changes.push("description".to_string());
290+
}
291+
if previous.subtasks != updated.subtasks {
292+
changes.push("subtasks".to_string());
293+
}
294+
if previous.metadata != updated.metadata {
295+
changes.push("metadata".to_string());
296+
}
297+
if previous.worker_id != updated.worker_id {
298+
changes.push(match (&previous.worker_id, &updated.worker_id) {
299+
(None, Some(_)) => "worker assigned".to_string(),
300+
(Some(_), None) => "worker unassigned".to_string(),
301+
_ => "worker binding".to_string(),
302+
});
303+
}
304+
if previous.approved_by != updated.approved_by {
305+
changes.push("approval".to_string());
306+
}
307+
if previous.assigned_agent_id != updated.assigned_agent_id {
308+
changes.push("assignment".to_string());
309+
}
310+
311+
if changes.is_empty() {
312+
return format!("Task #{} updated", updated.task_number);
313+
}
314+
315+
if changes.len() == 1 && previous.status != updated.status {
316+
return format!(
317+
"Task #{} updated to {}",
318+
updated.task_number, updated.status
319+
);
320+
}
321+
322+
format!(
323+
"Task #{} updated: {}",
324+
updated.task_number,
325+
changes.join(", ")
326+
)
327+
}
328+
275329
#[cfg(test)]
276330
mod tests {
277331
use super::*;
278-
279332
use crate::memory::working::WorkingMemoryEvent;
280333
use crate::memory::{WorkingMemoryEventType, WorkingMemoryStore};
281334
use crate::tasks::store::setup_test_store;
335+
use crate::tasks::{Task, TaskPriority, TaskStatus, TaskSubtask};
282336
use chrono_tz::Tz;
283337
use sqlx::sqlite::SqlitePoolOptions;
284338
use std::time::Duration;
@@ -313,6 +367,72 @@ mod tests {
313367
.expect("timed out waiting for working memory event")
314368
}
315369

370+
fn task_fixture() -> Task {
371+
Task {
372+
id: "task-id".to_string(),
373+
task_number: 7,
374+
title: "Original title".to_string(),
375+
description: Some("Original description".to_string()),
376+
status: TaskStatus::Backlog,
377+
priority: TaskPriority::Medium,
378+
owner_agent_id: "agent".to_string(),
379+
assigned_agent_id: "agent".to_string(),
380+
subtasks: Vec::new(),
381+
metadata: serde_json::json!({}),
382+
source_memory_id: None,
383+
worker_id: None,
384+
created_by: "branch".to_string(),
385+
approved_at: None,
386+
approved_by: None,
387+
created_at: "2026-04-19T00:00:00Z".to_string(),
388+
updated_at: "2026-04-19T00:00:00Z".to_string(),
389+
completed_at: None,
390+
}
391+
}
392+
393+
#[test]
394+
fn task_update_memory_summary_preserves_status_update_wording() {
395+
let previous = task_fixture();
396+
let mut updated = previous.clone();
397+
updated.status = TaskStatus::Ready;
398+
399+
assert_eq!(
400+
task_update_memory_summary(&previous, &updated),
401+
"Task #7 updated to ready"
402+
);
403+
}
404+
405+
#[test]
406+
fn task_update_memory_summary_names_non_status_changes() {
407+
let previous = task_fixture();
408+
let mut updated = previous.clone();
409+
updated.title = "New title".to_string();
410+
updated.description = Some("New description".to_string());
411+
updated.priority = TaskPriority::High;
412+
updated.subtasks = vec![TaskSubtask {
413+
title: "Check output".to_string(),
414+
completed: true,
415+
}];
416+
updated.metadata = serde_json::json!({"source": "review"});
417+
updated.worker_id = Some("worker-1".to_string());
418+
updated.approved_by = Some("victor".to_string());
419+
420+
assert_eq!(
421+
task_update_memory_summary(&previous, &updated),
422+
"Task #7 updated: priority medium -> high, title, description, subtasks, metadata, worker assigned, approval"
423+
);
424+
}
425+
426+
#[test]
427+
fn task_update_memory_summary_handles_no_actual_delta() {
428+
let previous = task_fixture();
429+
430+
assert_eq!(
431+
task_update_memory_summary(&previous, &previous),
432+
"Task #7 updated"
433+
);
434+
}
435+
316436
#[tokio::test]
317437
async fn task_update_emits_outcome_for_done_status() {
318438
let task_store = Arc::new(setup_test_store().await);
@@ -409,7 +529,7 @@ mod tests {
409529
assert_eq!(event.event_type, WorkingMemoryEventType::TaskUpdate);
410530
assert_eq!(
411531
event.summary,
412-
format!("Task #{} updated to done", created.task_number)
532+
format!("Task #{} updated: title", created.task_number)
413533
);
414534
}
415535

0 commit comments

Comments
 (0)