Skip to content

Commit d24f80c

Browse files
committed
Match plan section labels to heading starts only
Body prose that mentioned steps or risks was treated as a section start, so a five-section plan could lose its real Ordered steps heading and salvage as incomplete.
1 parent 7b1333b commit d24f80c

2 files changed

Lines changed: 122 additions & 7 deletions

File tree

src/subagent/index.test.ts

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,83 @@ describe("sub-agent stop helpers", () => {
311311
"src/subagent/report.ts",
312312
].join("\n");
313313

314+
const STEPS_IN_AC_BODY_PLAN_ENVELOPE = [
315+
"## Summary",
316+
"Plan for the salvage gate.",
317+
"",
318+
"## Findings",
319+
"### Files / paths",
320+
"src/subagent/report.ts",
321+
"",
322+
"### Acceptance criteria",
323+
"The worker completes the salvage steps.",
324+
"",
325+
"### Non-goals",
326+
"Do not finish CL-6946.",
327+
"",
328+
"### Risks",
329+
"A headings-only complete would auto-dispatch builder on a stub.",
330+
"",
331+
"### Ordered steps",
332+
"Add hasPlanFindings, then wire evaluateSubAgentStop.",
333+
"",
334+
"## Blockers",
335+
"None.",
336+
"",
337+
"## Paths",
338+
"src/subagent/report.ts",
339+
].join("\n");
340+
341+
const RISKS_IN_AC_BODY_PLAN_ENVELOPE = [
342+
"## Summary",
343+
"Plan for the salvage gate.",
344+
"",
345+
"## Findings",
346+
"### Files / paths",
347+
"src/subagent/report.ts",
348+
"",
349+
"### Acceptance criteria",
350+
"The worker mitigates residual risks.",
351+
"",
352+
"### Non-goals",
353+
"Do not finish CL-6946.",
354+
"",
355+
"### Risks",
356+
"A headings-only complete would auto-dispatch builder on a stub.",
357+
"",
358+
"### Ordered steps",
359+
"Add hasPlanFindings, then wire evaluateSubAgentStop.",
360+
"",
361+
"## Blockers",
362+
"None.",
363+
"",
364+
"## Paths",
365+
"src/subagent/report.ts",
366+
].join("\n");
367+
368+
const NUMBERED_COUNSEL_PLAN_ENVELOPE = [
369+
"## Summary",
370+
"Plan for the salvage gate.",
371+
"",
372+
"## Findings",
373+
"1. Files / paths to touch",
374+
" src/subagent/report.ts",
375+
"2. Acceptance criteria",
376+
" The worker completes the salvage steps.",
377+
"3. Non-goals",
378+
" Do not finish CL-6946.",
379+
"4. Risks and open questions",
380+
" A headings-only complete would auto-dispatch builder on a stub.",
381+
"5. Ordered steps",
382+
" Add hasPlanFindings, then wire evaluateSubAgentStop.",
383+
"",
384+
"## Blockers",
385+
"None.",
386+
"",
387+
"## Paths",
388+
"src/subagent/report.ts",
389+
].join("\n");
390+
314391
test("evaluateSubAgentStop returns incomplete-report for Summary-only tool-less narration after tools", () => {
315392
expect(
316393
evaluateSubAgentStop({
@@ -443,6 +520,18 @@ describe("sub-agent stop helpers", () => {
443520
expect(hasPlanFindings(PASS_PLAN_ENVELOPE)).toBe(true);
444521
});
445522

523+
test("hasPlanFindings stays true when an earlier section body uses the word steps", () => {
524+
expect(hasPlanFindings(STEPS_IN_AC_BODY_PLAN_ENVELOPE)).toBe(true);
525+
});
526+
527+
test("hasPlanFindings stays true when an earlier section body uses the word risks", () => {
528+
expect(hasPlanFindings(RISKS_IN_AC_BODY_PLAN_ENVELOPE)).toBe(true);
529+
});
530+
531+
test("hasPlanFindings is true for counsel numbered labels with following-line substance", () => {
532+
expect(hasPlanFindings(NUMBERED_COUNSEL_PLAN_ENVELOPE)).toBe(true);
533+
});
534+
446535
test("hasReportEnvelope stays heading-presence only on headings-only text", () => {
447536
expect(hasReportEnvelope(HEADINGS_ONLY_ENVELOPE)).toBe(true);
448537
expect(hasPlanFindings(HEADINGS_ONLY_ENVELOPE)).toBe(false);
@@ -500,6 +589,36 @@ describe("sub-agent stop helpers", () => {
500589
).toBe("complete");
501590
});
502591

592+
test("evaluateSubAgentStop completes a five-section plan with steps in an earlier body", () => {
593+
expect(
594+
evaluateSubAgentStop({
595+
hasToolCalls: false,
596+
requirePlanSubstance: true,
597+
lastAssistantText: STEPS_IN_AC_BODY_PLAN_ENVELOPE,
598+
}),
599+
).toBe("complete");
600+
});
601+
602+
test("evaluateSubAgentStop completes a five-section plan with risks in an earlier body", () => {
603+
expect(
604+
evaluateSubAgentStop({
605+
hasToolCalls: false,
606+
requirePlanSubstance: true,
607+
lastAssistantText: RISKS_IN_AC_BODY_PLAN_ENVELOPE,
608+
}),
609+
).toBe("complete");
610+
});
611+
612+
test("evaluateSubAgentStop completes counsel numbered labels with following-line substance", () => {
613+
expect(
614+
evaluateSubAgentStop({
615+
hasToolCalls: false,
616+
requirePlanSubstance: true,
617+
lastAssistantText: NUMBERED_COUNSEL_PLAN_ENVELOPE,
618+
}),
619+
).toBe("complete");
620+
});
621+
503622
test("evaluateSubAgentStop does not stop for many unique reads while still calling tools", () => {
504623
let thrash = EMPTY_THRASH_STATE;
505624
for (let i = 0; i < 200; i++) {

src/subagent/report.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,13 +155,9 @@ function classifyPlanSectionLine(
155155
if (stripped.length === 0) return null;
156156
for (const section of PLAN_FINDING_SECTIONS) {
157157
const match = section.match.exec(stripped);
158-
if (match === null || match.index === undefined) continue;
159-
// Only treat a line as a section start when the label is the line's heading,
160-
// not a later mention in an outline sentence.
161-
if (match.index > 0 && /[,;]/.test(stripped.slice(0, match.index))) {
162-
continue;
163-
}
164-
const after = stripped.slice(match.index + match[0].length);
158+
// Labels start the heading or numbered item, not a later word in body prose.
159+
if (match === null || match.index !== 0) continue;
160+
const after = stripped.slice(match[0].length);
165161
const separated = after.match(/^\s*[:.\-]\s+(\S.*)$/);
166162
return { id: section.id, rest: separated?.[1] ?? "" };
167163
}

0 commit comments

Comments
 (0)