feat: add orchestrator cost aggregation and hierarchy visualization #8082
Conversation
- Add utility functions for building task trees and calculating aggregated costs - Implement AggregatedCostDisplay component with collapsible breakdown - Update TaskHeader to show aggregated costs for top-level orchestrator tasks - Add HierarchicalTaskItem component for tree-based task display - Update HistoryView to support hierarchical task visualization with toggle - Add comprehensive tests for task hierarchy utilities - Add translation keys for new UI elements This implementation addresses Issue #5376 by: 1. Computing aggregated costs client-side from existing rootTaskId/parentTaskId fields 2. Showing collapsible cost breakdown in TaskHeader for orchestrator tasks 3. Rendering task history as an indented tree structure 4. Maintaining real-time updates as subtasks complete 5. Preserving existing behavior for non-orchestrator tasks
dosubot
bot
added
size:XL
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| visited.add(taskId) | ||
|
|
||
| tasks.forEach((task) => { | ||
| if (task.parentTaskId === taskId) { |
There was a problem hiding this comment.
Is this intentional? The function adds tasks to the descendants array before checking if the taskId has been visited. This could lead to incorrect behavior in circular reference scenarios. Consider checking visited status before adding:
| if (task.parentTaskId === taskId) { | |
| function collectDescendants(taskId: string) { | |
| if (visited.has(taskId)) return | |
| visited.add(taskId) | |
| tasks.forEach((task) => { | |
| if (task.parentTaskId === taskId && !visited.has(task.id)) { | |
| descendants.push(task) | |
| collectDescendants(task.id) | |
| } | |
| }) | |
| } |
| "tokens": "Tokens", | ||
| "cache": "Cache", | ||
| "apiCost": "API Cost", | ||
| "aggregatedCost": "Total Cost", |
There was a problem hiding this comment.
These new translation keys need to be added to all other locale files (ca, de, es, fr, hi, id, it, ja, ko, nl, pl, pt-BR, ru, tr, vi, zh-CN, zh-TW) to prevent missing translation errors for non-English users. Could we ensure all locales are updated?
| </div> | ||
|
|
||
| {/* Render children recursively */} | ||
| {isExpanded && hasChildren && ( |
There was a problem hiding this comment.
For deeply nested task hierarchies with many items, this recursive rendering without virtualization could cause performance issues. Have you considered implementing virtualization for the tree view, perhaps using react-window or similar?
| const descendants = getTaskDescendants(circularTasks, "task-a") | ||
| // task-a has task-b as a child, and task-b has task-a as a child (circular) | ||
| // The visited set prevents infinite loop, but both are found as descendants | ||
| expect(descendants).toHaveLength(2) |
There was a problem hiding this comment.
This test expectation seems incorrect. In a circular reference between task-a and task-b, neither should be a descendant of the other. The current implementation returns 2 items which doesn't make logical sense. Should we reconsider the expected behavior here?
| depth: number | ||
| } | ||
|
|
||
| export const AggregatedCostDisplay: React.FC<AggregatedCostDisplayProps> = ({ currentTask, className }) => { |
There was a problem hiding this comment.
Could we wrap this component with React.memo to prevent unnecessary re-renders when parent components update? This would improve performance, especially when dealing with large task histories:
| export const AggregatedCostDisplay: React.FC<AggregatedCostDisplayProps> = ({ currentTask, className }) => { | |
| export const AggregatedCostDisplay = React.memo<AggregatedCostDisplayProps>(({ currentTask, className }) => { |
| ? "font-semibold text-vscode-foreground" | ||
| : "text-vscode-descriptionForeground", | ||
| )} | ||
| style={{ paddingLeft: `${item.depth * 12}px` }}> |
There was a problem hiding this comment.
Consider extracting this magic number to a constant for better maintainability:
| style={{ paddingLeft: `${item.depth * 12}px` }}> | |
| const INDENTATION_PER_LEVEL = 12; | |
| // ... | |
| style={{ paddingLeft: `${item.depth * INDENTATION_PER_LEVEL}px` }} |
hannesrudolph
added
the
Issue/PR - Triage
Summary
This PR implements hierarchy visualization and aggregated costs for Orchestrator mode tasks, addressing Issue #5376.
Changes
Core Features
Implementation Details
New Components
AggregatedCostDisplay: Displays aggregated costs with collapsible breakdownHierarchicalTaskItem: Renders tasks in tree structure with expand/collapseModified Components
TaskHeader: Conditionally shows aggregated costs for orchestrator tasksHistoryView: Added toggle for flat/hierarchical view modesUtility Functions
buildTaskTree: Constructs tree from flat task list using parent/child relationshipscalculateAggregatedCost: Computes total cost for task and all descendantsgetTaskDescendants: Retrieves all subtasks for a given parentisTopLevelOrchestrator: Checks if task is orchestrator without parentTechnical Approach
rootTaskId,parentTaskId, andtotalCostfieldsTesting
UI/UX
Screenshots
The implementation provides:
Related Issue
Closes #5376
Testing Instructions
Checklist
Important
Adds task hierarchy visualization and cost aggregation for orchestrator tasks with real-time updates and hierarchical view toggle.
TaskHeaderandAggregatedCostDisplay.HistoryView.AggregatedCostDisplaycomponent for cost breakdown.HierarchicalTaskItemfor rendering tasks in a tree structure.TaskHeaderto show aggregated costs conditionally.HistoryViewto toggle between flat and hierarchical views.buildTaskTree,calculateAggregatedCost,getTaskDescendants,isTopLevelOrchestratorintaskHierarchy.ts.taskHierarchy.spec.ts.This description was created by
for 40b74b9. You can customize this summary. It will automatically update as commits are pushed.