Skip to content

Commit f39a145

Browse files
committed
fix(memory): describe task update deltas
1 parent c6dfb3c commit f39a145

3 files changed

Lines changed: 179 additions & 14 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: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl std::fmt::Display for TaskPriority {
101101
}
102102
}
103103

104-
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, utoipa::ToSchema)]
104+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, utoipa::ToSchema)]
105105
pub struct TaskSubtask {
106106
pub title: String,
107107
pub completed: bool,
@@ -350,9 +350,21 @@ impl TaskStore {
350350
}
351351

352352
pub async fn update(&self, task_number: i64, input: UpdateTaskInput) -> Result<Option<Task>> {
353+
Ok(self
354+
.update_with_previous(task_number, input)
355+
.await?
356+
.map(|(_, updated)| updated))
357+
}
358+
359+
pub async fn update_with_previous(
360+
&self,
361+
task_number: i64,
362+
input: UpdateTaskInput,
363+
) -> Result<Option<(Task, Task)>> {
353364
let Some(current) = self.get_by_number(task_number).await? else {
354365
return Ok(None);
355366
};
367+
let previous = current.clone();
356368

357369
if let Some(next_status) = input.status
358370
&& !can_transition(current.status, next_status)
@@ -452,7 +464,10 @@ impl TaskStore {
452464
.await
453465
.context("failed to update task")?;
454466

455-
self.get_by_number(task_number).await
467+
Ok(self
468+
.get_by_number(task_number)
469+
.await?
470+
.map(|updated| (previous, updated)))
456471
}
457472

458473
pub async fn delete(&self, task_number: i64) -> Result<bool> {
@@ -869,6 +884,33 @@ mod tests {
869884
);
870885
}
871886

887+
#[tokio::test]
888+
async fn update_with_previous_returns_applied_snapshot_pair() {
889+
let store = setup_store().await;
890+
let created = store
891+
.create(self_assigned_input("old title", TaskStatus::Backlog))
892+
.await
893+
.expect("task should be created");
894+
895+
let (previous, updated) = store
896+
.update_with_previous(
897+
created.task_number,
898+
UpdateTaskInput {
899+
title: Some("new title".to_string()),
900+
priority: Some(TaskPriority::High),
901+
..Default::default()
902+
},
903+
)
904+
.await
905+
.expect("update should succeed")
906+
.expect("task should exist");
907+
908+
assert_eq!(previous.title, "old title");
909+
assert_eq!(previous.priority, TaskPriority::Medium);
910+
assert_eq!(updated.title, "new title");
911+
assert_eq!(updated.priority, TaskPriority::High);
912+
}
913+
872914
#[tokio::test]
873915
async fn global_task_numbers_are_unique_across_agents() {
874916
let store = setup_store().await;

src/tools/task_update.rs

Lines changed: 133 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Task update tool for branch and worker processes.
22
3-
use crate::tasks::{TaskPriority, TaskStatus, TaskStore, TaskSubtask, UpdateTaskInput};
3+
use crate::tasks::{Task, TaskPriority, TaskStatus, TaskStore, TaskSubtask, UpdateTaskInput};
44
use crate::{AgentId, WorkerId};
55
use rig::completion::ToolDefinition;
66
use rig::tool::Tool;
@@ -213,9 +213,9 @@ impl Tool for TaskUpdateTool {
213213
),
214214
};
215215

216-
let updated = self
216+
let (previous, updated) = self
217217
.task_store
218-
.update(
218+
.update_with_previous(
219219
task_number,
220220
UpdateTaskInput {
221221
title: args.title,
@@ -236,14 +236,9 @@ impl Tool for TaskUpdateTool {
236236
.ok_or_else(|| TaskUpdateError(format!("task #{} not found", task_number)))?;
237237

238238
if let Some(working_memory) = &self.working_memory {
239+
let summary = task_update_memory_summary(&previous, &updated);
239240
working_memory
240-
.emit(
241-
crate::memory::WorkingMemoryEventType::TaskUpdate,
242-
format!(
243-
"Task #{} updated to {}",
244-
updated.task_number, updated.status
245-
),
246-
)
241+
.emit(crate::memory::WorkingMemoryEventType::TaskUpdate, summary)
247242
.importance(0.4)
248243
.record();
249244
}
@@ -256,3 +251,131 @@ impl Tool for TaskUpdateTool {
256251
})
257252
}
258253
}
254+
255+
fn task_update_memory_summary(previous: &Task, updated: &Task) -> String {
256+
let mut changes = Vec::new();
257+
258+
if previous.status != updated.status {
259+
changes.push(format!("status {} -> {}", previous.status, updated.status));
260+
}
261+
if previous.priority != updated.priority {
262+
changes.push(format!(
263+
"priority {} -> {}",
264+
previous.priority, updated.priority
265+
));
266+
}
267+
if previous.title != updated.title {
268+
changes.push("title".to_string());
269+
}
270+
if previous.description != updated.description {
271+
changes.push("description".to_string());
272+
}
273+
if previous.subtasks != updated.subtasks {
274+
changes.push("subtasks".to_string());
275+
}
276+
if previous.metadata != updated.metadata {
277+
changes.push("metadata".to_string());
278+
}
279+
if previous.worker_id != updated.worker_id {
280+
changes.push(match (&previous.worker_id, &updated.worker_id) {
281+
(None, Some(_)) => "worker assigned".to_string(),
282+
(Some(_), None) => "worker unassigned".to_string(),
283+
_ => "worker binding".to_string(),
284+
});
285+
}
286+
if previous.approved_by != updated.approved_by {
287+
changes.push("approval".to_string());
288+
}
289+
if previous.assigned_agent_id != updated.assigned_agent_id {
290+
changes.push("assignment".to_string());
291+
}
292+
293+
if changes.is_empty() {
294+
return format!("Task #{} updated", updated.task_number);
295+
}
296+
297+
if changes.len() == 1 && previous.status != updated.status {
298+
return format!(
299+
"Task #{} updated to {}",
300+
updated.task_number, updated.status
301+
);
302+
}
303+
304+
format!(
305+
"Task #{} updated: {}",
306+
updated.task_number,
307+
changes.join(", ")
308+
)
309+
}
310+
311+
#[cfg(test)]
312+
mod tests {
313+
use super::task_update_memory_summary;
314+
use crate::tasks::{Task, TaskPriority, TaskStatus, TaskSubtask};
315+
316+
fn task_fixture() -> Task {
317+
Task {
318+
id: "task-id".to_string(),
319+
task_number: 7,
320+
title: "Original title".to_string(),
321+
description: Some("Original description".to_string()),
322+
status: TaskStatus::Backlog,
323+
priority: TaskPriority::Medium,
324+
owner_agent_id: "agent".to_string(),
325+
assigned_agent_id: "agent".to_string(),
326+
subtasks: Vec::new(),
327+
metadata: serde_json::json!({}),
328+
source_memory_id: None,
329+
worker_id: None,
330+
created_by: "branch".to_string(),
331+
approved_at: None,
332+
approved_by: None,
333+
created_at: "2026-04-19T00:00:00Z".to_string(),
334+
updated_at: "2026-04-19T00:00:00Z".to_string(),
335+
completed_at: None,
336+
}
337+
}
338+
339+
#[test]
340+
fn task_update_memory_summary_preserves_status_update_wording() {
341+
let previous = task_fixture();
342+
let mut updated = previous.clone();
343+
updated.status = TaskStatus::Ready;
344+
345+
assert_eq!(
346+
task_update_memory_summary(&previous, &updated),
347+
"Task #7 updated to ready"
348+
);
349+
}
350+
351+
#[test]
352+
fn task_update_memory_summary_names_non_status_changes() {
353+
let previous = task_fixture();
354+
let mut updated = previous.clone();
355+
updated.title = "New title".to_string();
356+
updated.description = Some("New description".to_string());
357+
updated.priority = TaskPriority::High;
358+
updated.subtasks = vec![TaskSubtask {
359+
title: "Check output".to_string(),
360+
completed: true,
361+
}];
362+
updated.metadata = serde_json::json!({"source": "review"});
363+
updated.worker_id = Some("worker-1".to_string());
364+
updated.approved_by = Some("victor".to_string());
365+
366+
assert_eq!(
367+
task_update_memory_summary(&previous, &updated),
368+
"Task #7 updated: priority medium -> high, title, description, subtasks, metadata, worker assigned, approval"
369+
);
370+
}
371+
372+
#[test]
373+
fn task_update_memory_summary_handles_no_actual_delta() {
374+
let previous = task_fixture();
375+
376+
assert_eq!(
377+
task_update_memory_summary(&previous, &previous),
378+
"Task #7 updated"
379+
);
380+
}
381+
}

0 commit comments

Comments
 (0)