Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions src/core/task-persistence/taskMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,40 @@ export async function taskMetadata({
workspace,
}: TaskMetadataOptions) {
const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)

// Handle edge case where there are no messages at all
if (!messages || messages.length === 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider refactoring to eliminate code duplication here. The historyItem object is created twice - once in this early return and once at the end of the function. You could restructure this by:

  1. Using a boolean flag like const hasMessages = messages && messages.length > 0
  2. Pre-calculating all values (taskName, timestamp, tokenUsage, taskDirSize) based on message availability
  3. Creating the historyItem object only once at the end

This would improve maintainability and follow DRY principles.

const historyItem: HistoryItem = {
id: taskId,
number: taskNumber,
ts: Date.now(),
task: `Task #${taskNumber} (No messages)`,
tokensIn: 0,
tokensOut: 0,
cacheWrites: 0,
cacheReads: 0,
totalCost: 0,
size: 0,
workspace,
}
return {
historyItem,
tokenUsage: {
totalTokensIn: 0,
totalTokensOut: 0,
totalCacheWrites: 0,
totalCacheReads: 0,
totalCost: 0,
contextTokens: 0,
},
}
}

const taskMessage = messages[0] // First message is always the task say.

const lastRelevantMessage =
messages[findLastIndex(messages, (m) => !(m.ask === "resume_task" || m.ask === "resume_completed_task"))]
messages[findLastIndex(messages, (m) => !(m.ask === "resume_task" || m.ask === "resume_completed_task"))] ||
messages[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to use taskMessage here:

Suggested change
messages[0]
taskMessage


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tokenUsage object is missing the contextTokens field. Based on the TokenUsage type definition, it should include:

tokenUsage: {
    totalTokensIn: 0,
    totalTokensOut: 0,
    totalCacheWrites: 0,
    totalCacheReads: 0,
    totalCost: 0,
    contextTokens: 0,  // <- Missing field
}

This ensures type consistency with the rest of the codebase.

let taskDirSize = taskSizeCache.get<number>(taskDir)

Expand All @@ -45,11 +75,17 @@ export async function taskMetadata({

const tokenUsage = getApiMetrics(combineApiRequests(combineCommandSequences(messages.slice(1))))

// Ensure task name is never blank - provide fallback names
let taskName = taskMessage.text?.trim() || ""
if (!taskName) {
taskName = `Task #${taskNumber} (Incomplete)`
}

const historyItem: HistoryItem = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to restructure the function to use a single object creation pattern:

// Determine message availability upfront
const hasMessages = messages && messages.length > 0

// Pre-calculate all values based on availability
let taskName: string
let timestamp: number
let tokenUsage: ReturnType<typeof getApiMetrics>
let taskDirSize: number

if (!hasMessages) {
    // Handle no messages case
} else {
    // Handle messages case
}

// Create historyItem once with pre-calculated values

This eliminates the duplicate object creation and makes the code more maintainable.

id: taskId,
number: taskNumber,
ts: lastRelevantMessage.ts,
task: taskMessage.text ?? "",
task: taskName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this work better here? That way taskName is not reassigned unnecessarily.

Suggested change
task: taskName,
task: taskMessage.text?.trim() || `Task #${taskNumber} (Incomplete)`,

tokensIn: tokenUsage.totalTokensIn,
tokensOut: tokenUsage.totalTokensOut,
cacheWrites: tokenUsage.totalCacheWrites,
Expand Down