-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat: per-subtask cost breakdown display #10765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: per-subtask cost breakdown display #10765
Conversation
- Add SubtaskDetail interface with id, name, tokens, cost, status fields - Add buildSubtaskDetails() function to aggregate task costs - Pass childDetails in taskWithAggregatedCosts message - Integrate subtask cost display into TodoListDisplay component - Add fuzzy matching to match todo items with subtask details - Display token count and cost next to matching todo items - Add 16 tests for TodoListDisplay with subtask cost functionality Related to: RooCodeInc#5376
Review complete. All previously flagged issues have been resolved. The new task cost breakdown utilities use integer micros for stable aggregation, which is a solid approach. No new issues found.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| * - Stripping common task prefixes (Subtask N:, ## Task:, Task N:) | ||
| * - Removing trailing ellipsis from truncated strings | ||
| */ | ||
| function normalizeForComparison(str: string): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a bit of bad hack to try and match the task's that already are printed, I need to look into this deeper to find a better way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, jinx!
02c2586 to
e18382d
Compare
|
@roomote give me some feedback on this PR |
Verified the reported issue. The investigation/analysis markdown files ( |
| content: z.string(), | ||
| status: todoStatusSchema, | ||
| // Optional fields for subtask tracking | ||
| subtaskId: z.string().optional(), // ID of the linked subtask (child task) for direct cost/token attribution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added subtaskId to the todo item so we have a stable link to the spawned child task for cost/token attribution when returning to the parent. The todo item id is not stable across status transitions because it’s derived from the todo text + status (see parseMarkdownChecklist() and the hash input at src/core/tools/UpdateTodoListTool.ts:242-245). As a result, using the todo id to reconnect to the subtask would break when the checkbox state changes.
Summary
This PR implements the original feature: display token count and cost per delegated subtask inline in the parent task’s todo list.
It also replaces the unreliable todo↔subtask matching with deterministic ID-based linking, so each delegated subtask can write its tokens + cost back onto the exact todo record it belongs to.
Screenshots
ezgif-34019eabb0f25160.mov
Problem (why deterministic linking is required)
1) Why we can’t rely on
TodoItem.idTodoItem.idis not stable. It’s derived from content + status, so it changes exactly when a todo transitions between[ ]→[-]→[x].The ID is computed here:
id = md5(content + status)inparseMarkdownChecklist:Roo-Code/src/core/tools/UpdateTodoListTool.ts
Lines 229 to 253 in e18382d
match[2] + status:Roo-Code/src/core/tools/UpdateTodoListTool.ts
Lines 242 to 246 in e18382d
This means:
[ ] Write testsand[-] Write testsproduce different idstodo.idwould break during normal execution2) Why we can’t rely on fuzzy matching
The UI previously matched a todo’s
contentto a subtask’snamevia fuzzy string matching, which can collide and is not robust to truncation or user edits.Solution
1) Add a stable linkage field:
TodoItem.subtaskIdWe add
subtaskId?: stringtoTodoItemas a stable foreign key to the delegated child task ID.Roo-Code/packages/types/src/todo.ts
Lines 13 to 23 in e18382d
2) Provider-owned lifecycle hooks (delegation + completion)
We link and update todos at the two existing delegation chokepoints:
Delegation-time: provider knows
child.taskIdimmediatelytodo.subtaskId = childTaskIdand persist into the parent’s message streamRoo-Code/src/core/webview/ClineProvider.ts
Lines 3194 to 3264 in e18382d
Completion-time: provider knows
{ parentTaskId, childTaskId }HistoryItem, compute tokens/cost, find todo bysubtaskId, and write back:todo.tokens = tokensIn + tokensOuttodo.cost = totalCostRoo-Code/src/core/webview/ClineProvider.ts
Lines 3299 to 3389 in e18382d
3) Preserve metadata across subsequent
update_todo_listcallsBecause
update_todo_listreparses markdown and re-materializes todo objects, we preserve metadata fields across updates by exactcontentmatch (duplicates matched in order).preserveTodoMetadata:Roo-Code/src/core/tools/UpdateTodoListTool.ts
Lines 205 to 227 in e18382d
4) UI now renders from the todo record
TodoListDisplaynow:todo.subtaskIdtodo.tokens/todo.cost(with optional fallback to subtaskDetails-by-id)Roo-Code/webview-ui/src/components/chat/TodoListDisplay.tsx
Lines 1 to 151 in e18382d
Flow diagrams
Delegation → completion → todo write-back → UI
Why
subtaskId(and notid) must existflowchart TD A[update_todo_list markdown] --> B[parseMarkdownChecklist] B --> C["id = md5(content + status)"] C --> D{status changes?} D -- yes --> E[id changes] E --> F[link by todo.id breaks] F --> G[Need stable field: todo.subtaskId = childTaskId]Testing
TodoListDisplay.spec.tsxupdateTodoListTool.spec.tsprovider-delegation.spec.tshistory-resume-delegation.spec.tsRelated to: #5376