fix: Remove fallback subtasks in parseSubtasksFromText to properly throw errors on invalid input
This commit is contained in:
@@ -873,79 +873,86 @@ Note on dependencies: Subtasks can depend on other subtasks with lower IDs. Use
|
||||
* @param {number} expectedCount - Expected number of subtasks
|
||||
* @param {number} parentTaskId - Parent task ID
|
||||
* @returns {Array} Parsed subtasks
|
||||
* @throws {Error} If parsing fails or JSON is invalid
|
||||
*/
|
||||
function parseSubtasksFromText(text, startId, expectedCount, parentTaskId) {
|
||||
// Set default values for optional parameters
|
||||
startId = startId || 1;
|
||||
expectedCount = expectedCount || 2; // Default to 2 subtasks if not specified
|
||||
|
||||
// Handle empty text case
|
||||
if (!text || text.trim() === '') {
|
||||
throw new Error('Empty text provided, cannot parse subtasks');
|
||||
}
|
||||
|
||||
// Locate JSON array in the text
|
||||
const jsonStartIndex = text.indexOf('[');
|
||||
const jsonEndIndex = text.lastIndexOf(']');
|
||||
|
||||
// If no valid JSON array found, throw error
|
||||
if (
|
||||
jsonStartIndex === -1 ||
|
||||
jsonEndIndex === -1 ||
|
||||
jsonEndIndex < jsonStartIndex
|
||||
) {
|
||||
throw new Error('Could not locate valid JSON array in the response');
|
||||
}
|
||||
|
||||
// Extract and parse the JSON
|
||||
const jsonText = text.substring(jsonStartIndex, jsonEndIndex + 1);
|
||||
let subtasks;
|
||||
|
||||
try {
|
||||
// Locate JSON array in the text
|
||||
const jsonStartIndex = text.indexOf('[');
|
||||
const jsonEndIndex = text.lastIndexOf(']');
|
||||
subtasks = JSON.parse(jsonText);
|
||||
} catch (parseError) {
|
||||
throw new Error(`Failed to parse JSON: ${parseError.message}`);
|
||||
}
|
||||
|
||||
if (
|
||||
jsonStartIndex === -1 ||
|
||||
jsonEndIndex === -1 ||
|
||||
jsonEndIndex < jsonStartIndex
|
||||
) {
|
||||
throw new Error('Could not locate valid JSON array in the response');
|
||||
}
|
||||
// Validate array
|
||||
if (!Array.isArray(subtasks)) {
|
||||
throw new Error('Parsed content is not an array');
|
||||
}
|
||||
|
||||
// Extract and parse the JSON
|
||||
const jsonText = text.substring(jsonStartIndex, jsonEndIndex + 1);
|
||||
let subtasks = JSON.parse(jsonText);
|
||||
// Log warning if count doesn't match expected
|
||||
if (expectedCount && subtasks.length !== expectedCount) {
|
||||
log(
|
||||
'warn',
|
||||
`Expected ${expectedCount} subtasks, but parsed ${subtasks.length}`
|
||||
);
|
||||
}
|
||||
|
||||
// Validate
|
||||
if (!Array.isArray(subtasks)) {
|
||||
throw new Error('Parsed content is not an array');
|
||||
}
|
||||
|
||||
// Set default values for optional parameters
|
||||
startId = startId || 1;
|
||||
expectedCount = expectedCount || subtasks.length;
|
||||
|
||||
// Log warning if count doesn't match expected
|
||||
if (expectedCount && subtasks.length !== expectedCount) {
|
||||
// Normalize subtask IDs if they don't match
|
||||
subtasks = subtasks.map((subtask, index) => {
|
||||
// Assign the correct ID if it doesn't match
|
||||
if (!subtask.id || subtask.id !== startId + index) {
|
||||
log(
|
||||
'warn',
|
||||
`Expected ${expectedCount} subtasks, but parsed ${subtasks.length}`
|
||||
`Correcting subtask ID from ${subtask.id || 'undefined'} to ${startId + index}`
|
||||
);
|
||||
subtask.id = startId + index;
|
||||
}
|
||||
|
||||
// Normalize subtask IDs if they don't match
|
||||
subtasks = subtasks.map((subtask, index) => {
|
||||
// Assign the correct ID if it doesn't match
|
||||
if (!subtask.id || subtask.id !== startId + index) {
|
||||
log(
|
||||
'warn',
|
||||
`Correcting subtask ID from ${subtask.id || 'undefined'} to ${startId + index}`
|
||||
);
|
||||
subtask.id = startId + index;
|
||||
}
|
||||
// Convert dependencies to numbers if they are strings
|
||||
if (subtask.dependencies && Array.isArray(subtask.dependencies)) {
|
||||
subtask.dependencies = subtask.dependencies.map((dep) => {
|
||||
return typeof dep === 'string' ? parseInt(dep, 10) : dep;
|
||||
});
|
||||
} else {
|
||||
subtask.dependencies = [];
|
||||
}
|
||||
|
||||
// Convert dependencies to numbers if they are strings
|
||||
if (subtask.dependencies && Array.isArray(subtask.dependencies)) {
|
||||
subtask.dependencies = subtask.dependencies.map((dep) => {
|
||||
return typeof dep === 'string' ? parseInt(dep, 10) : dep;
|
||||
});
|
||||
} else {
|
||||
subtask.dependencies = [];
|
||||
}
|
||||
// Ensure status is 'pending'
|
||||
subtask.status = 'pending';
|
||||
|
||||
// Ensure status is 'pending'
|
||||
subtask.status = 'pending';
|
||||
// Add parentTaskId if provided
|
||||
if (parentTaskId) {
|
||||
subtask.parentTaskId = parentTaskId;
|
||||
}
|
||||
|
||||
// Add parentTaskId if provided
|
||||
if (parentTaskId) {
|
||||
subtask.parentTaskId = parentTaskId;
|
||||
}
|
||||
return subtask;
|
||||
});
|
||||
|
||||
return subtask;
|
||||
});
|
||||
|
||||
return subtasks;
|
||||
} catch (error) {
|
||||
log('error', `Error parsing subtasks: ${error.message}`);
|
||||
// Re-throw the error to be handled by the caller
|
||||
throw error;
|
||||
}
|
||||
return subtasks;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -196,29 +196,12 @@ These subtasks will help you implement the parent task efficiently.`;
|
||||
expect(result[2].dependencies).toEqual([1, 2]);
|
||||
});
|
||||
|
||||
test('should create fallback subtasks for empty text', () => {
|
||||
test('should throw an error for empty text', () => {
|
||||
const emptyText = '';
|
||||
|
||||
const result = parseSubtasksFromText(emptyText, 1, 2, 5);
|
||||
|
||||
// Verify fallback subtasks structure
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toMatchObject({
|
||||
id: 1,
|
||||
title: 'Subtask 1',
|
||||
description: 'Auto-generated fallback subtask',
|
||||
status: 'pending',
|
||||
dependencies: [],
|
||||
parentTaskId: 5
|
||||
});
|
||||
expect(result[1]).toMatchObject({
|
||||
id: 2,
|
||||
title: 'Subtask 2',
|
||||
description: 'Auto-generated fallback subtask',
|
||||
status: 'pending',
|
||||
dependencies: [],
|
||||
parentTaskId: 5
|
||||
});
|
||||
expect(() => parseSubtasksFromText(emptyText, 1, 2, 5)).toThrow(
|
||||
'Empty text provided, cannot parse subtasks'
|
||||
);
|
||||
});
|
||||
|
||||
test('should normalize subtask IDs', () => {
|
||||
@@ -272,29 +255,12 @@ These subtasks will help you implement the parent task efficiently.`;
|
||||
expect(typeof result[1].dependencies[0]).toBe('number');
|
||||
});
|
||||
|
||||
test('should create fallback subtasks for invalid JSON', () => {
|
||||
test('should throw an error for invalid JSON', () => {
|
||||
const text = `This is not valid JSON and cannot be parsed`;
|
||||
|
||||
const result = parseSubtasksFromText(text, 1, 2, 5);
|
||||
|
||||
// Verify fallback subtasks structure
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toMatchObject({
|
||||
id: 1,
|
||||
title: 'Subtask 1',
|
||||
description: 'Auto-generated fallback subtask',
|
||||
status: 'pending',
|
||||
dependencies: [],
|
||||
parentTaskId: 5
|
||||
});
|
||||
expect(result[1]).toMatchObject({
|
||||
id: 2,
|
||||
title: 'Subtask 2',
|
||||
description: 'Auto-generated fallback subtask',
|
||||
status: 'pending',
|
||||
dependencies: [],
|
||||
parentTaskId: 5
|
||||
});
|
||||
expect(() => parseSubtasksFromText(text, 1, 2, 5)).toThrow(
|
||||
'Could not locate valid JSON array in the response'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user