mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-24 16:02:55 +08:00
fix(cli): remove planning-context gate from review suggestion trigger
The review follow-up suggestion was gated behind hasPlanningContext, requiring a prior plan_exit or plan handover prefix before triggering. This was overly restrictive — any first implementation turn by the code agent should prompt a review suggestion. Also excludes orchestrator turns and removes task from review tools since orchestrator is no longer a trigger.
This commit is contained in:
@@ -76,7 +76,7 @@ export namespace SessionPrompt {
|
||||
)
|
||||
}
|
||||
|
||||
const reviewTools = new Set(["edit", "write", "multiedit", "apply_patch", "task"]) // kilocode_change
|
||||
const reviewTools = new Set(["edit", "write", "multiedit", "apply_patch"]) // kilocode_change
|
||||
|
||||
// kilocode_change start - ask review follow-up only after first implementation turn per session
|
||||
function reviewTurns(messages: MessageV2.WithParts[]) {
|
||||
@@ -100,7 +100,7 @@ export namespace SessionPrompt {
|
||||
}
|
||||
|
||||
function isImplementationTurn(input: { user: MessageV2.User; turn: MessageV2.WithParts[] }) {
|
||||
if (!["code", "orchestrator"].includes(input.user.agent)) return false
|
||||
if (!["code"].includes(input.user.agent)) return false
|
||||
|
||||
const hasPlanExit = input.turn.some((msg) =>
|
||||
msg.parts.some((part) => part.type === "tool" && part.tool === "plan_exit" && part.state.status === "completed"),
|
||||
@@ -112,32 +112,6 @@ export namespace SessionPrompt {
|
||||
)
|
||||
}
|
||||
|
||||
function hasPlanningContext(input: { turns: ReturnType<typeof reviewTurns>; messages: MessageV2.WithParts[] }) {
|
||||
// Same-session plan → code: a prior turn contains a completed plan_exit
|
||||
const priorHasPlanExit = input.turns
|
||||
.slice(0, -1)
|
||||
.some((t) =>
|
||||
t.turn.some((msg) =>
|
||||
msg.parts.some((p) => p.type === "tool" && p.tool === "plan_exit" && p.state.status === "completed"),
|
||||
),
|
||||
)
|
||||
if (priorHasPlanExit) return true
|
||||
|
||||
// Cross-session handover: first user message starts with the plan prefix
|
||||
const first = input.messages.find((m) => m.info.role === "user")
|
||||
if (first) {
|
||||
const text = first.parts
|
||||
.filter((p): p is MessageV2.TextPart => p.type === "text" && !p.synthetic)
|
||||
.map((p) => p.text)
|
||||
.join("\n")
|
||||
.trimStart()
|
||||
if (text.startsWith(PlanFollowup.PLAN_PREFIX)) return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
// kilocode_change end
|
||||
|
||||
// kilocode_change start - share review follow-up trigger logic with tests
|
||||
export function shouldAskReviewFollowup(input: { messages: MessageV2.WithParts[]; abort: AbortSignal }) {
|
||||
if (input.abort.aborted) return false
|
||||
@@ -151,8 +125,6 @@ export namespace SessionPrompt {
|
||||
const alreadyImplemented = turns.slice(0, -1).some(isImplementationTurn)
|
||||
if (alreadyImplemented) return false
|
||||
|
||||
if (!hasPlanningContext({ turns, messages: input.messages })) return false
|
||||
|
||||
return true
|
||||
}
|
||||
// kilocode_change end
|
||||
|
||||
@@ -409,13 +409,13 @@ async function seedHandoverSession() {
|
||||
}
|
||||
|
||||
describe("review follow-up detection", () => {
|
||||
test("does not trigger without plan context even with implementation tool", () =>
|
||||
test("triggers for code agent with implementation tool", () =>
|
||||
withInstance(async () => {
|
||||
const messages = await seed({
|
||||
agent: "code",
|
||||
tools: [{ tool: "edit" }],
|
||||
})
|
||||
expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false)
|
||||
expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(true)
|
||||
}))
|
||||
|
||||
test("does not trigger for orchestrator turns without plan context", () =>
|
||||
@@ -435,6 +435,132 @@ describe("review follow-up detection", () => {
|
||||
expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false)
|
||||
}))
|
||||
|
||||
test("does not trigger for orchestrator even with plan context", () =>
|
||||
withInstance(async () => {
|
||||
const session = await Session.create({})
|
||||
|
||||
// Turn 1: plan turn that ends with plan_exit
|
||||
const planUser = await Session.updateMessage({
|
||||
id: Identifier.ascending("message"),
|
||||
role: "user",
|
||||
sessionID: session.id,
|
||||
time: { created: Date.now() },
|
||||
agent: "code",
|
||||
model,
|
||||
})
|
||||
await Session.updatePart({
|
||||
id: Identifier.ascending("part"),
|
||||
messageID: planUser.id,
|
||||
sessionID: session.id,
|
||||
type: "text",
|
||||
text: "Plan the feature",
|
||||
})
|
||||
|
||||
const planAssistant: MessageV2.Assistant = {
|
||||
id: Identifier.ascending("message"),
|
||||
role: "assistant",
|
||||
sessionID: session.id,
|
||||
time: { created: Date.now() },
|
||||
parentID: planUser.id,
|
||||
modelID: model.modelID,
|
||||
providerID: model.providerID,
|
||||
mode: "code",
|
||||
agent: "code",
|
||||
path: {
|
||||
cwd: Instance.directory,
|
||||
root: Instance.worktree,
|
||||
},
|
||||
cost: 0,
|
||||
tokens: {
|
||||
total: 0,
|
||||
input: 0,
|
||||
output: 0,
|
||||
reasoning: 0,
|
||||
cache: { read: 0, write: 0 },
|
||||
},
|
||||
finish: "end_turn",
|
||||
}
|
||||
await Session.updateMessage(planAssistant)
|
||||
await Session.updatePart({
|
||||
id: Identifier.ascending("part"),
|
||||
messageID: planAssistant.id,
|
||||
sessionID: session.id,
|
||||
type: "tool",
|
||||
callID: Identifier.ascending("tool"),
|
||||
tool: "plan_exit",
|
||||
state: {
|
||||
status: "completed",
|
||||
input: {},
|
||||
output: "ok",
|
||||
title: "plan_exit",
|
||||
metadata: {},
|
||||
time: { start: Date.now(), end: Date.now() },
|
||||
},
|
||||
} satisfies MessageV2.ToolPart)
|
||||
|
||||
// Turn 2: orchestrator turn with task tool
|
||||
const orchUser = await Session.updateMessage({
|
||||
id: Identifier.ascending("message"),
|
||||
role: "user",
|
||||
sessionID: session.id,
|
||||
time: { created: Date.now() },
|
||||
agent: "orchestrator",
|
||||
model,
|
||||
})
|
||||
await Session.updatePart({
|
||||
id: Identifier.ascending("part"),
|
||||
messageID: orchUser.id,
|
||||
sessionID: session.id,
|
||||
type: "text",
|
||||
text: "Implement it",
|
||||
})
|
||||
|
||||
const orchAssistant: MessageV2.Assistant = {
|
||||
id: Identifier.ascending("message"),
|
||||
role: "assistant",
|
||||
sessionID: session.id,
|
||||
time: { created: Date.now() },
|
||||
parentID: orchUser.id,
|
||||
modelID: model.modelID,
|
||||
providerID: model.providerID,
|
||||
mode: "orchestrator",
|
||||
agent: "orchestrator",
|
||||
path: {
|
||||
cwd: Instance.directory,
|
||||
root: Instance.worktree,
|
||||
},
|
||||
cost: 0,
|
||||
tokens: {
|
||||
total: 0,
|
||||
input: 0,
|
||||
output: 0,
|
||||
reasoning: 0,
|
||||
cache: { read: 0, write: 0 },
|
||||
},
|
||||
finish: "end_turn",
|
||||
}
|
||||
await Session.updateMessage(orchAssistant)
|
||||
await Session.updatePart({
|
||||
id: Identifier.ascending("part"),
|
||||
messageID: orchAssistant.id,
|
||||
sessionID: session.id,
|
||||
type: "tool",
|
||||
callID: Identifier.ascending("tool"),
|
||||
tool: "task",
|
||||
state: {
|
||||
status: "completed",
|
||||
input: {},
|
||||
output: "ok",
|
||||
title: "task",
|
||||
metadata: {},
|
||||
time: { start: Date.now(), end: Date.now() },
|
||||
},
|
||||
} satisfies MessageV2.ToolPart)
|
||||
|
||||
const messages = await Session.messages({ sessionID: session.id })
|
||||
expect(SessionPrompt.shouldAskReviewFollowup({ messages, abort: AbortSignal.any([]) })).toBe(false)
|
||||
}))
|
||||
|
||||
test("does not trigger for read-only turns", () =>
|
||||
withInstance(async () => {
|
||||
const messages = await seed({
|
||||
|
||||
Reference in New Issue
Block a user