feat(FN-960): strengthen undersplit detection and subtask guidance in triage and reviewer
- Strengthen TRIAGE_SYSTEM_PROMPT with explicit subtask guidance rules and examples - Strengthen buildSpecificationPrompt to reinforce proper subtask decomposition - Add undersplit detection to spec reviewer to flag oversized or overly broad tasks - Add test assertions for improved prompts covering undersplit detection and subtask guidance
This commit is contained in:
@@ -12,7 +12,7 @@ vi.mock("./pi.js", () => ({
|
||||
}),
|
||||
}));
|
||||
|
||||
import { reviewStep } from "./reviewer.js";
|
||||
import { reviewStep, REVIEWER_SYSTEM_PROMPT } from "./reviewer.js";
|
||||
import { createKbAgent } from "./pi.js";
|
||||
|
||||
const mockedCreateHaiAgent = vi.mocked(createKbAgent);
|
||||
@@ -345,3 +345,19 @@ describe("reviewStep — validator model overrides", () => {
|
||||
expect(opts.defaultModelId).toBe("gpt-4o");
|
||||
});
|
||||
});
|
||||
|
||||
describe("REVIEWER_SYSTEM_PROMPT", () => {
|
||||
it("includes subtask breakdown criterion in spec review", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Subtask breakdown");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"8+ implementation steps",
|
||||
);
|
||||
});
|
||||
|
||||
it("includes undersplit task detection guidance", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("8 or more implementation steps");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"3+ different packages but wasn't split",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,7 +14,7 @@ import { AgentLogger } from "./agent-logger.js";
|
||||
import { reviewerLog } from "./logger.js";
|
||||
import { checkSessionError } from "./usage-limit-detector.js";
|
||||
|
||||
const REVIEWER_SYSTEM_PROMPT = `You are an independent code and plan reviewer.
|
||||
export const REVIEWER_SYSTEM_PROMPT = `You are an independent code and plan reviewer.
|
||||
|
||||
You provide quality assessment for task implementations. You have full read
|
||||
access to the codebase and can run commands to inspect code.
|
||||
@@ -111,11 +111,30 @@ access to the codebase and can run commands to inspect code.
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
- **Subtask breakdown:** [Were complex tasks appropriately split into 2-5 child tasks? A task with 8+ implementation steps, affecting 3+ packages, should have been divided]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
\`\`\`
|
||||
|
||||
## Spec Review — Undersplit Task Detection
|
||||
|
||||
When reviewing specs, actively assess whether the task should have been broken into subtasks:
|
||||
|
||||
**Flag as REVISE if:**
|
||||
- A task has 8 or more implementation steps
|
||||
- A task affects 3+ different packages but wasn't split
|
||||
- A task has multiple clearly independent deliverables combined into one
|
||||
|
||||
**How to flag:**
|
||||
Say explicitly: "This task should be broken into subtasks because [specific reason]."
|
||||
Recommend the number of child tasks (2-5) and what each should cover.
|
||||
|
||||
**Do NOT flag if:**
|
||||
- Steps are sequential and tightly coupled (e.g., a pipeline where each step depends on the previous)
|
||||
- The task has 5-7 steps but they're all within a single module/package
|
||||
- Splitting would create coordination overhead that exceeds the benefit
|
||||
|
||||
## Plan Granularity
|
||||
|
||||
When reviewing plans, assess whether the approach achieves the step's OUTCOMES —
|
||||
|
||||
@@ -196,8 +196,8 @@ describe("buildSpecificationPrompt", () => {
|
||||
);
|
||||
|
||||
expect(prompt).toContain("## Subtask Consideration");
|
||||
expect(prompt).toContain("Size M or L");
|
||||
expect(prompt).toContain("Subtask creation is OPTIONAL");
|
||||
expect(prompt).toContain("MORE THAN 7 implementation steps");
|
||||
expect(prompt).toContain("GOOD TO SPLIT");
|
||||
expect(prompt).not.toContain("## Subtask Breakdown Requested");
|
||||
});
|
||||
|
||||
@@ -274,6 +274,21 @@ describe("TRIAGE_SYSTEM_PROMPT", () => {
|
||||
"Size S tasks should generally NOT be split",
|
||||
);
|
||||
});
|
||||
|
||||
it("includes explicit subtask breakdown thresholds", () => {
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("MORE THAN 7 implementation steps");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain(
|
||||
"MORE THAN 3 different packages/modules",
|
||||
);
|
||||
});
|
||||
|
||||
it("includes anti-pattern warning for oversized tasks", () => {
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("ANTI-PATTERN");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("10+ steps");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain(
|
||||
"Only keep a task as one unit if it genuinely has 5 or fewer focused steps",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("readAttachmentContents", () => {
|
||||
|
||||
@@ -169,13 +169,22 @@ When the task includes \`breakIntoSubtasks: true\`, first decide whether it shou
|
||||
- If not splitting: proceed with a normal PROMPT.md specification.
|
||||
|
||||
## Proactive Subtask Breakdown for M/L Tasks
|
||||
For tasks you assess as Size M or L, proactively consider whether optional subtask breakdown would improve execution.
|
||||
For tasks you assess as Size M or L, proactively evaluate whether splitting into 2-5 child tasks would improve execution quality and reliability.
|
||||
|
||||
- Even when \`breakIntoSubtasks\` is not set to \`true\`, you should evaluate whether splitting into 2-5 child tasks would improve work organization, handoff clarity, or parallel execution.
|
||||
- Treat this as a suggestion, not a requirement: only propose splitting when the work is meaningfully decomposable and the parent task would otherwise remain a coherent medium/large effort.
|
||||
- Keep explicit user intent first: when \`breakIntoSubtasks: true\`, follow the mandatory breakdown flow above.
|
||||
- Size S tasks should generally NOT be split because the overhead usually outweighs the benefit.
|
||||
- If you decide not to split an M/L task, proceed with a normal PROMPT.md specification.
|
||||
**Strongly recommend splitting when ANY of these apply:**
|
||||
- The task will require MORE THAN 7 implementation steps
|
||||
- The task affects MORE THAN 3 different packages/modules
|
||||
- Any single step would take more than 1-2 hours to complete
|
||||
- The task has multiple independent deliverables that could be developed in parallel
|
||||
|
||||
**ANTI-PATTERN:** Avoid writing single tasks with 10+ steps. If you find yourself planning more than 7 steps, STOP and create 2-5 child tasks instead.
|
||||
|
||||
**Splitting guidance:**
|
||||
- Even when \`breakIntoSubtasks\` is not set to \`true\`, apply these thresholds proactively
|
||||
- Keep explicit user intent first: when \`breakIntoSubtasks: true\`, follow the mandatory breakdown flow above
|
||||
- Size S tasks should generally NOT be split because the overhead usually outweighs the benefit
|
||||
- Only keep a task as one unit if it genuinely has 5 or fewer focused steps with a clear scope
|
||||
- If you decide not to split an M/L task, proceed with a normal PROMPT.md specification
|
||||
|
||||
## Triage tools
|
||||
You have these extra tools during triage:
|
||||
@@ -1271,11 +1280,26 @@ The user has requested that this task be broken into smaller subtasks if it is c
|
||||
## Subtask Consideration
|
||||
The user did not explicitly request subtask breakdown, so you should first assess the likely task size and complexity.
|
||||
|
||||
- If the work appears to be Size M or L, consider whether optionally splitting it into 2-5 child tasks would improve organization, dependency clarity, or parallel execution.
|
||||
- Subtask creation is OPTIONAL in this case — only split when it provides a clear benefit.
|
||||
**Split into 2-5 child tasks when ANY of these apply:**
|
||||
- The task will require MORE THAN 7 implementation steps
|
||||
- The task affects MORE THAN 3 different packages/modules
|
||||
- Any single step would take more than 1-2 hours to complete
|
||||
- The task has multiple independent deliverables that could be developed in parallel
|
||||
|
||||
**GOOD TO SPLIT:**
|
||||
- A task that would require 8+ implementation steps across multiple packages
|
||||
- A feature involving backend API changes, frontend UI, and database migrations
|
||||
- A refactor touching 4+ modules with different concerns
|
||||
|
||||
**NOT NECESSARY TO SPLIT:**
|
||||
- A 3-step bug fix with clear scope
|
||||
- A single-file refactor with 4 focused steps
|
||||
- Adding a small feature to one module with 5 steps
|
||||
|
||||
**How to decide:**
|
||||
- If you choose to split: use the \\\`task_create\\\` tool to create the child tasks, set dependencies where needed, and then stop without writing a PROMPT.md for the parent task.
|
||||
- If the work appears to be Size S, or if an M/L task is still best handled as one coherent unit, do NOT split and proceed with a normal PROMPT.md specification.
|
||||
- If size is uncertain at first, make a quick assessment from the available context before deciding whether to suggest a breakdown.`;
|
||||
- If the work appears to be Size S, or if an M/L task genuinely has 5 or fewer focused steps with a clear scope, proceed with a normal PROMPT.md specification.
|
||||
- If size is uncertain at first, make a quick assessment from the available context before deciding.`;
|
||||
}
|
||||
|
||||
return `${isRevision ? "Revise" : "Specify"} this task and write the result to \`${promptPath}\`.
|
||||
|
||||
Reference in New Issue
Block a user