Feat: Added automatic determination of task number based on complexity (#884)
- Added 'defaultNumTasks: 10' to default config, now used in 'parse-prd' - Adjusted 'parse-prd' and 'expand-task' to: - Accept a 'numTasks' value of 0 - Updated tool and command descriptions - Updated prompts to 'an appropriate number of' when value is 0 - Updated 'README-task-master.md' and 'command-reference.md' docs - Added more tests for: 'parse-prd', 'expand-task' and 'config-manager' Co-authored-by: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com>
This commit is contained in:
Geoff Hammond
@@ -136,6 +136,7 @@ const DEFAULT_CONFIG = {
|
||||
global: {
|
||||
logLevel: 'info',
|
||||
debug: false,
|
||||
defaultNumTasks: 10,
|
||||
defaultSubtasks: 5,
|
||||
defaultPriority: 'medium',
|
||||
projectName: 'Task Master',
|
||||
@@ -738,5 +739,116 @@ describe('getAllProviders', () => {
|
||||
|
||||
// Add tests for getParametersForRole if needed
|
||||
|
||||
// --- defaultNumTasks Tests ---
|
||||
describe('Configuration Getters', () => {
|
||||
test('getDefaultNumTasks should return default value when config is valid', () => {
|
||||
// Arrange: Mock fs.readFileSync to return valid config when called with the expected path
|
||||
fsReadFileSyncSpy.mockImplementation((filePath) => {
|
||||
if (filePath === MOCK_CONFIG_PATH) {
|
||||
return JSON.stringify({
|
||||
global: {
|
||||
defaultNumTasks: 15
|
||||
}
|
||||
});
|
||||
}
|
||||
throw new Error(`Unexpected fs.readFileSync call: ${filePath}`);
|
||||
});
|
||||
fsExistsSyncSpy.mockReturnValue(true);
|
||||
|
||||
// Force reload to clear cache
|
||||
configManager.getConfig(MOCK_PROJECT_ROOT, true);
|
||||
|
||||
// Act: Call getDefaultNumTasks with explicit root
|
||||
const result = configManager.getDefaultNumTasks(MOCK_PROJECT_ROOT);
|
||||
|
||||
// Assert
|
||||
expect(result).toBe(15);
|
||||
});
|
||||
|
||||
test('getDefaultNumTasks should return fallback when config value is invalid', () => {
|
||||
// Arrange: Mock fs.readFileSync to return invalid config
|
||||
fsReadFileSyncSpy.mockImplementation((filePath) => {
|
||||
if (filePath === MOCK_CONFIG_PATH) {
|
||||
return JSON.stringify({
|
||||
global: {
|
||||
defaultNumTasks: 'invalid'
|
||||
}
|
||||
});
|
||||
}
|
||||
throw new Error(`Unexpected fs.readFileSync call: ${filePath}`);
|
||||
});
|
||||
fsExistsSyncSpy.mockReturnValue(true);
|
||||
|
||||
// Force reload to clear cache
|
||||
configManager.getConfig(MOCK_PROJECT_ROOT, true);
|
||||
|
||||
// Act: Call getDefaultNumTasks with explicit root
|
||||
const result = configManager.getDefaultNumTasks(MOCK_PROJECT_ROOT);
|
||||
|
||||
// Assert
|
||||
expect(result).toBe(10); // Should fallback to DEFAULTS.global.defaultNumTasks
|
||||
});
|
||||
|
||||
test('getDefaultNumTasks should return fallback when config value is missing', () => {
|
||||
// Arrange: Mock fs.readFileSync to return config without defaultNumTasks
|
||||
fsReadFileSyncSpy.mockImplementation((filePath) => {
|
||||
if (filePath === MOCK_CONFIG_PATH) {
|
||||
return JSON.stringify({
|
||||
global: {}
|
||||
});
|
||||
}
|
||||
throw new Error(`Unexpected fs.readFileSync call: ${filePath}`);
|
||||
});
|
||||
fsExistsSyncSpy.mockReturnValue(true);
|
||||
|
||||
// Force reload to clear cache
|
||||
configManager.getConfig(MOCK_PROJECT_ROOT, true);
|
||||
|
||||
// Act: Call getDefaultNumTasks with explicit root
|
||||
const result = configManager.getDefaultNumTasks(MOCK_PROJECT_ROOT);
|
||||
|
||||
// Assert
|
||||
expect(result).toBe(10); // Should fallback to DEFAULTS.global.defaultNumTasks
|
||||
});
|
||||
|
||||
test('getDefaultNumTasks should handle non-existent config file', () => {
|
||||
// Arrange: Mock file not existing
|
||||
fsExistsSyncSpy.mockReturnValue(false);
|
||||
|
||||
// Force reload to clear cache
|
||||
configManager.getConfig(MOCK_PROJECT_ROOT, true);
|
||||
|
||||
// Act: Call getDefaultNumTasks with explicit root
|
||||
const result = configManager.getDefaultNumTasks(MOCK_PROJECT_ROOT);
|
||||
|
||||
// Assert
|
||||
expect(result).toBe(10); // Should fallback to DEFAULTS.global.defaultNumTasks
|
||||
});
|
||||
|
||||
test('getDefaultNumTasks should accept explicit project root', () => {
|
||||
// Arrange: Mock fs.readFileSync to return valid config
|
||||
fsReadFileSyncSpy.mockImplementation((filePath) => {
|
||||
if (filePath === MOCK_CONFIG_PATH) {
|
||||
return JSON.stringify({
|
||||
global: {
|
||||
defaultNumTasks: 20
|
||||
}
|
||||
});
|
||||
}
|
||||
throw new Error(`Unexpected fs.readFileSync call: ${filePath}`);
|
||||
});
|
||||
fsExistsSyncSpy.mockReturnValue(true);
|
||||
|
||||
// Force reload to clear cache
|
||||
configManager.getConfig(MOCK_PROJECT_ROOT, true);
|
||||
|
||||
// Act: Call getDefaultNumTasks with explicit project root
|
||||
const result = configManager.getDefaultNumTasks(MOCK_PROJECT_ROOT);
|
||||
|
||||
// Assert
|
||||
expect(result).toBe(20);
|
||||
});
|
||||
});
|
||||
|
||||
// Note: Tests for setMainModel, setResearchModel were removed as the functions were removed in the implementation.
|
||||
// If similar setter functions exist, add tests for them following the writeConfig pattern.
|
||||
|
||||
@@ -122,7 +122,8 @@ jest.unstable_mockModule(
|
||||
'../../../../../scripts/modules/config-manager.js',
|
||||
() => ({
|
||||
getDefaultSubtasks: jest.fn(() => 3),
|
||||
getDebugFlag: jest.fn(() => false)
|
||||
getDebugFlag: jest.fn(() => false),
|
||||
getDefaultNumTasks: jest.fn(() => 10)
|
||||
})
|
||||
);
|
||||
|
||||
@@ -199,6 +200,10 @@ const generateTaskFiles = (
|
||||
)
|
||||
).default;
|
||||
|
||||
const { getDefaultSubtasks } = await import(
|
||||
'../../../../../scripts/modules/config-manager.js'
|
||||
);
|
||||
|
||||
// Import the module under test
|
||||
const { default: expandTask } = await import(
|
||||
'../../../../../scripts/modules/task-manager/expand-task.js'
|
||||
@@ -946,4 +951,120 @@ describe('expandTask', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Dynamic Subtask Generation', () => {
|
||||
const tasksPath = 'tasks/tasks.json';
|
||||
const taskId = 1;
|
||||
const context = { session: null, mcpLog: null };
|
||||
|
||||
beforeEach(() => {
|
||||
// Reset all mocks
|
||||
jest.clearAllMocks();
|
||||
|
||||
// Setup default mocks
|
||||
readJSON.mockReturnValue({
|
||||
tasks: [
|
||||
{
|
||||
id: 1,
|
||||
title: 'Test Task',
|
||||
description: 'A test task',
|
||||
status: 'pending',
|
||||
subtasks: []
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
findTaskById.mockReturnValue({
|
||||
id: 1,
|
||||
title: 'Test Task',
|
||||
description: 'A test task',
|
||||
status: 'pending',
|
||||
subtasks: []
|
||||
});
|
||||
|
||||
findProjectRoot.mockReturnValue('/mock/project/root');
|
||||
});
|
||||
|
||||
test('should accept 0 as valid numSubtasks value for dynamic generation', async () => {
|
||||
// Act - Call with numSubtasks=0 (should not throw error)
|
||||
const result = await expandTask(
|
||||
tasksPath,
|
||||
taskId,
|
||||
0,
|
||||
false,
|
||||
'',
|
||||
context,
|
||||
false
|
||||
);
|
||||
|
||||
// Assert - Should complete successfully
|
||||
expect(result).toBeDefined();
|
||||
expect(generateTextService).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('should use dynamic prompting when numSubtasks is 0', async () => {
|
||||
// Act
|
||||
await expandTask(tasksPath, taskId, 0, false, '', context, false);
|
||||
|
||||
// Assert - Verify generateTextService was called
|
||||
expect(generateTextService).toHaveBeenCalled();
|
||||
|
||||
// Get the call arguments to verify the system prompt
|
||||
const callArgs = generateTextService.mock.calls[0][0];
|
||||
expect(callArgs.systemPrompt).toContain(
|
||||
'an appropriate number of specific subtasks'
|
||||
);
|
||||
});
|
||||
|
||||
test('should use specific count prompting when numSubtasks is positive', async () => {
|
||||
// Act
|
||||
await expandTask(tasksPath, taskId, 5, false, '', context, false);
|
||||
|
||||
// Assert - Verify generateTextService was called
|
||||
expect(generateTextService).toHaveBeenCalled();
|
||||
|
||||
// Get the call arguments to verify the system prompt
|
||||
const callArgs = generateTextService.mock.calls[0][0];
|
||||
expect(callArgs.systemPrompt).toContain('5 specific subtasks');
|
||||
});
|
||||
|
||||
test('should reject negative numSubtasks values and fallback to default', async () => {
|
||||
// Mock getDefaultSubtasks to return a specific value
|
||||
getDefaultSubtasks.mockReturnValue(4);
|
||||
|
||||
// Act
|
||||
await expandTask(tasksPath, taskId, -3, false, '', context, false);
|
||||
|
||||
// Assert - Should use default value instead of negative
|
||||
expect(generateTextService).toHaveBeenCalled();
|
||||
const callArgs = generateTextService.mock.calls[0][0];
|
||||
expect(callArgs.systemPrompt).toContain('4 specific subtasks');
|
||||
});
|
||||
|
||||
test('should use getDefaultSubtasks when numSubtasks is undefined', async () => {
|
||||
// Mock getDefaultSubtasks to return a specific value
|
||||
getDefaultSubtasks.mockReturnValue(6);
|
||||
|
||||
// Act - Call without specifying numSubtasks (undefined)
|
||||
await expandTask(tasksPath, taskId, undefined, false, '', context, false);
|
||||
|
||||
// Assert - Should use default value
|
||||
expect(generateTextService).toHaveBeenCalled();
|
||||
const callArgs = generateTextService.mock.calls[0][0];
|
||||
expect(callArgs.systemPrompt).toContain('6 specific subtasks');
|
||||
});
|
||||
|
||||
test('should use getDefaultSubtasks when numSubtasks is null', async () => {
|
||||
// Mock getDefaultSubtasks to return a specific value
|
||||
getDefaultSubtasks.mockReturnValue(7);
|
||||
|
||||
// Act - Call with null numSubtasks
|
||||
await expandTask(tasksPath, taskId, null, false, '', context, false);
|
||||
|
||||
// Assert - Should use default value
|
||||
expect(generateTextService).toHaveBeenCalled();
|
||||
const callArgs = generateTextService.mock.calls[0][0];
|
||||
expect(callArgs.systemPrompt).toContain('7 specific subtasks');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -47,7 +47,8 @@ jest.unstable_mockModule('../../../../../scripts/modules/ui.js', () => ({
|
||||
jest.unstable_mockModule(
|
||||
'../../../../../scripts/modules/config-manager.js',
|
||||
() => ({
|
||||
getDebugFlag: jest.fn(() => false)
|
||||
getDebugFlag: jest.fn(() => false),
|
||||
getDefaultNumTasks: jest.fn(() => 10)
|
||||
})
|
||||
);
|
||||
|
||||
@@ -94,13 +95,15 @@ jest.unstable_mockModule('path', () => ({
|
||||
}));
|
||||
|
||||
// Import the mocked modules
|
||||
const { readJSON, writeJSON, log, promptYesNo } = await import(
|
||||
const { readJSON, promptYesNo } = await import(
|
||||
'../../../../../scripts/modules/utils.js'
|
||||
);
|
||||
|
||||
const { generateObjectService } = await import(
|
||||
'../../../../../scripts/modules/ai-services-unified.js'
|
||||
);
|
||||
|
||||
// Note: getDefaultNumTasks validation happens at CLI/MCP level, not in the main parse-prd module
|
||||
const generateTaskFiles = (
|
||||
await import(
|
||||
'../../../../../scripts/modules/task-manager/generate-task-files.js'
|
||||
@@ -433,4 +436,123 @@ describe('parsePRD', () => {
|
||||
// Verify prompt was NOT called with append flag
|
||||
expect(promptYesNo).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('Dynamic Task Generation', () => {
|
||||
test('should use dynamic prompting when numTasks is 0', async () => {
|
||||
// Setup mocks to simulate normal conditions (no existing output file)
|
||||
fs.default.existsSync.mockImplementation((p) => {
|
||||
if (p === 'tasks/tasks.json') return false; // Output file doesn't exist
|
||||
if (p === 'tasks') return true; // Directory exists
|
||||
return false;
|
||||
});
|
||||
|
||||
// Call the function with numTasks=0 for dynamic generation
|
||||
await parsePRD('path/to/prd.txt', 'tasks/tasks.json', 0);
|
||||
|
||||
// Verify generateObjectService was called
|
||||
expect(generateObjectService).toHaveBeenCalled();
|
||||
|
||||
// Get the call arguments to verify the prompt
|
||||
const callArgs = generateObjectService.mock.calls[0][0];
|
||||
expect(callArgs.prompt).toContain('an appropriate number of');
|
||||
expect(callArgs.prompt).not.toContain('approximately 0');
|
||||
});
|
||||
|
||||
test('should use specific count prompting when numTasks is positive', async () => {
|
||||
// Setup mocks to simulate normal conditions (no existing output file)
|
||||
fs.default.existsSync.mockImplementation((p) => {
|
||||
if (p === 'tasks/tasks.json') return false; // Output file doesn't exist
|
||||
if (p === 'tasks') return true; // Directory exists
|
||||
return false;
|
||||
});
|
||||
|
||||
// Call the function with specific numTasks
|
||||
await parsePRD('path/to/prd.txt', 'tasks/tasks.json', 5);
|
||||
|
||||
// Verify generateObjectService was called
|
||||
expect(generateObjectService).toHaveBeenCalled();
|
||||
|
||||
// Get the call arguments to verify the prompt
|
||||
const callArgs = generateObjectService.mock.calls[0][0];
|
||||
expect(callArgs.prompt).toContain('approximately 5');
|
||||
expect(callArgs.prompt).not.toContain('an appropriate number of');
|
||||
});
|
||||
|
||||
test('should accept 0 as valid numTasks value', async () => {
|
||||
// Setup mocks to simulate normal conditions (no existing output file)
|
||||
fs.default.existsSync.mockImplementation((p) => {
|
||||
if (p === 'tasks/tasks.json') return false; // Output file doesn't exist
|
||||
if (p === 'tasks') return true; // Directory exists
|
||||
return false;
|
||||
});
|
||||
|
||||
// Call the function with numTasks=0 - should not throw error
|
||||
const result = await parsePRD('path/to/prd.txt', 'tasks/tasks.json', 0);
|
||||
|
||||
// Verify it completed successfully
|
||||
expect(result).toEqual({
|
||||
success: true,
|
||||
tasksPath: 'tasks/tasks.json',
|
||||
telemetryData: {}
|
||||
});
|
||||
});
|
||||
|
||||
test('should use dynamic prompting when numTasks is negative (no validation in main module)', async () => {
|
||||
// Setup mocks to simulate normal conditions (no existing output file)
|
||||
fs.default.existsSync.mockImplementation((p) => {
|
||||
if (p === 'tasks/tasks.json') return false; // Output file doesn't exist
|
||||
if (p === 'tasks') return true; // Directory exists
|
||||
return false;
|
||||
});
|
||||
|
||||
// Call the function with negative numTasks
|
||||
// Note: The main parse-prd.js module doesn't validate numTasks - validation happens at CLI/MCP level
|
||||
await parsePRD('path/to/prd.txt', 'tasks/tasks.json', -5);
|
||||
|
||||
// Verify generateObjectService was called
|
||||
expect(generateObjectService).toHaveBeenCalled();
|
||||
const callArgs = generateObjectService.mock.calls[0][0];
|
||||
// Negative values are treated as <= 0, so should use dynamic prompting
|
||||
expect(callArgs.prompt).toContain('an appropriate number of');
|
||||
expect(callArgs.prompt).not.toContain('approximately -5');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Configuration Integration', () => {
|
||||
test('should use dynamic prompting when numTasks is null', async () => {
|
||||
// Setup mocks to simulate normal conditions (no existing output file)
|
||||
fs.default.existsSync.mockImplementation((p) => {
|
||||
if (p === 'tasks/tasks.json') return false; // Output file doesn't exist
|
||||
if (p === 'tasks') return true; // Directory exists
|
||||
return false;
|
||||
});
|
||||
|
||||
// Call the function with null numTasks
|
||||
await parsePRD('path/to/prd.txt', 'tasks/tasks.json', null);
|
||||
|
||||
// Verify generateObjectService was called with dynamic prompting
|
||||
expect(generateObjectService).toHaveBeenCalled();
|
||||
const callArgs = generateObjectService.mock.calls[0][0];
|
||||
expect(callArgs.prompt).toContain('an appropriate number of');
|
||||
});
|
||||
|
||||
test('should use dynamic prompting when numTasks is invalid string', async () => {
|
||||
// Setup mocks to simulate normal conditions (no existing output file)
|
||||
fs.default.existsSync.mockImplementation((p) => {
|
||||
if (p === 'tasks/tasks.json') return false; // Output file doesn't exist
|
||||
if (p === 'tasks') return true; // Directory exists
|
||||
return false;
|
||||
});
|
||||
|
||||
// Call the function with invalid numTasks (string that's not a number)
|
||||
await parsePRD('path/to/prd.txt', 'tasks/tasks.json', 'invalid');
|
||||
|
||||
// Verify generateObjectService was called with dynamic prompting
|
||||
// Note: The main module doesn't validate - it just uses the value as-is
|
||||
// Since 'invalid' > 0 is false, it uses dynamic prompting
|
||||
expect(generateObjectService).toHaveBeenCalled();
|
||||
const callArgs = generateObjectService.mock.calls[0][0];
|
||||
expect(callArgs.prompt).toContain('an appropriate number of');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user