Skip to content

Commit 53c52e7

Browse files
authored
feat: sub-agent intelligence quality — headline tail-keep + bounded stall detection (T2) (#242)
* feat: sub-agent headline tail-keep + bounded stall detection (T2 intelligence quality) Headline channel (B1): - extractSummaryInfo keeps the TAIL of over-cap answers (leading …): verdicts, recommendations, and next actions live at the end of a report — the old head cut dropped exactly the conclusions - truncateTailWithLen helper; original rune count and summary_truncated semantics unchanged; delegate_tasks description states the new contract - legacy head-keep pins updated to the tail-keep contract Stall detection (B2): - hints escalate instead of resetting: per-fingerprint milestone doubles (3→6→12→…), later hints marked 'again' — a hint-resistant loop gets progressively fewer interruptions instead of 27 identical hints - bg_status/bg_output polling is bounded rather than exempt: 3× threshold (9 identical polls) fires a poll-specific hint (check bg_list, read bg_output, move on) — a pinned dead job id can no longer burn the whole iteration budget silently Pinned RED-first in internal/loop/stall_escalation_test.go and cmd/odek/headline_tail_test.go. internal/loop and full cmd/odek green under -race. * fix: close review findings — partial-marker detection under tail-keep, milestone lifecycle - PartialSummaryReason matches budget markers with Contains instead of HasPrefix: tail-kept over-cap summaries lose their leading bytes, so a budget-killed run was misclassified as success (HIGH, judge round 2) - toolStallMilestones reset at run start and dropped alongside fingerprints on error/eviction — long-lived engines no longer inherit escalated milestones across runs (MAJOR, judge round 1) - bg-poll hints use the milestone mechanism with an accurate poll count instead of doubling the counter (reported 18 polls after 9) internal/loop and full cmd/odek green under -race.
1 parent b5fd90c commit 53c52e7

9 files changed

Lines changed: 307 additions & 32 deletions

‎cmd/odek/headline_tail_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package main
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/BackendStack21/odek/internal/session"
8+
)
9+
10+
// ── B1: sub-agent headline keeps the TAIL ────────────────────────────────
11+
//
12+
// The old head-keep truncation chopped exactly the part that matters: a
13+
// verdict/recommendation lives at the END of a report, so a 2048-rune
14+
// head-cut dropped conclusions while keeping boilerplate. The headline cut
15+
// must keep the tail (verdicts, file lists, next actions) with a leading
16+
// ellipsis marking the trim.
17+
18+
func TestExtractSummaryInfo_KeepsTail(t *testing.T) {
19+
verdict := "VERDICT: MERGE — the patch is correct and complete."
20+
filler := strings.Repeat("Analysis prose. ", 200) // > 2048 runes
21+
content := filler + "\n\n" + verdict
22+
23+
msgs := []session.Message{{Role: "assistant", Content: content}}
24+
got, total, truncated := extractSummaryInfo(msgs)
25+
26+
if !truncated {
27+
t.Fatal("expected truncated=true for over-cap content")
28+
}
29+
if total != len([]rune(content)) {
30+
t.Errorf("total rune count = %d, want %d", total, len([]rune(content)))
31+
}
32+
if !strings.Contains(got, verdict) {
33+
t.Errorf("tail-keep headline lost the verdict — head-keep truncation drops conclusions:\n%.300s", got)
34+
}
35+
if !strings.HasPrefix(got, "…") {
36+
t.Errorf("tail-keep headline must mark the cut with a LEADING ellipsis, got: %.50s", got)
37+
}
38+
runes := len([]rune(got))
39+
if runes > subagentHeadlineMaxRunes+1 { // +1 for the ellipsis rune
40+
t.Errorf("headline = %d runes, want ≤ %d (+ellipsis)", runes, subagentHeadlineMaxRunes)
41+
}
42+
}
43+
44+
func TestExtractSummaryInfo_ShortContentUntouched(t *testing.T) {
45+
msgs := []session.Message{{Role: "assistant", Content: "short answer"}}
46+
got, total, truncated := extractSummaryInfo(msgs)
47+
if truncated || total != 12 || got != "short answer" {
48+
t.Errorf("short content must pass through untouched, got %q (total=%d, truncated=%v)", got, total, truncated)
49+
}
50+
}
51+
52+
// The parent-facing description must describe the new semantics.
53+
func TestSubagentDescription_TailKeepSemantics(t *testing.T) {
54+
desc := (&delegateTasksTool{}).Description()
55+
if strings.Contains(desc, "a trailing … means it was cut") {
56+
t.Error("description still claims head-keep semantics ('a trailing … means it was cut')")
57+
}
58+
if !strings.Contains(desc, "leading …") {
59+
t.Error("description must state the tail-keep contract ('a leading … marks the cut; the end of the answer is kept')")
60+
}
61+
}

‎cmd/odek/subagent.go‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,12 +1364,15 @@ const subagentHeadlineMaxRunes = 2048
13641364

13651365
// extractSummaryInfo returns the child's final answer cut to the headline
13661366
// cap, the ORIGINAL rune count, and whether it was cut (C — the parent
1367-
// render turns this into a visible truncation marker). The bulk channel is
1368-
// the artifact protocol; the headline is a status summary.
1367+
// render turns this into a visible truncation marker). The cut keeps the
1368+
// TAIL of the answer with a leading ellipsis: verdicts, recommendations,
1369+
// and next actions live at the end of a report — a head cut dropped
1370+
// exactly the conclusions. The bulk channel is the artifact protocol; the
1371+
// headline is a status summary.
13691372
func extractSummaryInfo(messages []session.Message) (string, int, bool) {
13701373
for i := len(messages) - 1; i >= 0; i-- {
13711374
if messages[i].Role == "assistant" && messages[i].Content != "" {
1372-
s, total := truncateWithLen(messages[i].Content, subagentHeadlineMaxRunes)
1375+
s, total := truncateTailWithLen(messages[i].Content, subagentHeadlineMaxRunes)
13731376
return s, total, total > subagentHeadlineMaxRunes
13741377
}
13751378
}
@@ -1394,6 +1397,20 @@ func truncateWithLen(s string, n int) (string, int) {
13941397
return string(runes[:n]) + "…", len(runes)
13951398
}
13961399

1400+
// truncateTailWithLen keeps the LAST n runes of s (prepending "…") and
1401+
// reports the ORIGINAL rune count. Used by the sub-agent headline: the end
1402+
// of a report carries the verdict; the start is boilerplate.
1403+
func truncateTailWithLen(s string, n int) (string, int) {
1404+
runes := []rune(s)
1405+
if n <= 0 {
1406+
return "…", len(runes)
1407+
}
1408+
if len(runes) <= n {
1409+
return s, len(runes)
1410+
}
1411+
return "…" + string(runes[len(runes)-n:]), len(runes)
1412+
}
1413+
13971414
func extractFilesChanged(messages []session.Message) []string {
13981415
var files []string
13991416
seen := make(map[string]bool)

‎cmd/odek/subagent_contract_test.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,8 @@ func TestExtractSummary_AssistantWithToolCallsOnly(t *testing.T) {
823823
}
824824

825825
// Updated for the headline cap (SUBAGENT_RESULT_ARTIFACTS_PLAN.md):
826-
// extractSummary now carries 2048 runes instead of the old 500-rune cut.
826+
// extractSummary carries 2048 runes; since the tail-keep change the cut
827+
// preserves the END of the answer (verdicts) with a leading ellipsis.
827828
func TestExtractSummary_TruncatesLongOutput(t *testing.T) {
828829
longContent := strings.Repeat("a", 3000)
829830
msgs := []session.Message{
@@ -833,8 +834,11 @@ func TestExtractSummary_TruncatesLongOutput(t *testing.T) {
833834
if len([]rune(summary)) > subagentHeadlineMaxRunes+1 { // cap + ellipsis
834835
t.Errorf("extractSummary too long: %d runes (max %d)", len([]rune(summary)), subagentHeadlineMaxRunes+1)
835836
}
836-
if !strings.HasSuffix(summary, "…") {
837-
t.Errorf("truncated summary should end with '…', got: %q", summary)
837+
if !strings.HasPrefix(summary, "…") {
838+
t.Errorf("truncated summary should start with '…' (tail-keep), got: %q", summary)
839+
}
840+
if !strings.HasSuffix(summary, longContent[len(longContent)-10:]) {
841+
t.Errorf("tail-keep summary must preserve the answer END, got: %q", summary)
838842
}
839843
}
840844

‎cmd/odek/subagent_delivery_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestDelegateTasksDescription_ArtifactChannel(t *testing.T) {
3232
"32 KB",
3333
"artifact_read(id)",
3434
"flat file in your artifact dir",
35-
"trailing … means it was cut",
35+
"leading … marks the cut",
3636
} {
3737
if !strings.Contains(desc, want) {
3838
t.Errorf("Description() missing %q — the parent LLM cannot learn the artifact protocol", want)

‎cmd/odek/subagent_result_render_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@ func TestExtractSummary_HeadlineCap(t *testing.T) {
2828
if n := len([]rune(got)); n != subagentHeadlineMaxRunes+1 { // cap + ellipsis
2929
t.Errorf("headline cap: got %d runes, want %d (cap + ellipsis)", n, subagentHeadlineMaxRunes+1)
3030
}
31-
if !strings.HasPrefix(got, long[:subagentHeadlineMaxRunes]) {
32-
t.Error("headline must preserve the answer prefix")
31+
if !strings.HasPrefix(got, "…") {
32+
t.Error("tail-keep headline must mark the cut with a leading ellipsis")
33+
}
34+
if !strings.HasSuffix(got, long[len(long)-subagentHeadlineMaxRunes:]) {
35+
t.Error("tail-keep headline must preserve the answer END")
3336
}
3437
}
3538

‎cmd/odek/subagent_tool.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func (t *delegateTasksTool) Description() string {
207207
return `Spawn sub-agent processes for independent sub-tasks. Each child has a fresh context — put everything it needs in goal/context. Children never prompt for approvals (denials are listed). Trust never increases downward. Depth is capped.
208208
209209
Result delivery — two channels per sub-agent:
210-
- Headline: the sub-agent's final answer, capped at ~2000 characters. Treat it as a status summary; a trailing … means it was cut.
210+
- Headline: the sub-agent's final answer, capped at ~2000 characters keeping the END (verdicts, next actions) — a leading … marks the cut.
211211
- Artifacts: validated file deliverables (id, type, size, one-line summary). text/* ≤32 KB is inlined; fetch the rest with artifact_read(id). For artifact-heavy work, set guidance to write a flat file in your artifact dir and keep the final answer to a short headline.`
212212
}
213213

‎internal/loop/budget_hints.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,14 @@ func intBudgetBand(max, remaining int64, exhausted bool, t int, label string) st
242242
// set status/partial_reason without string-matching the markers themselves.
243243
func PartialSummaryReason(final string) (string, bool) {
244244
switch {
245-
case strings.HasPrefix(final, budgetSummaryMarker):
245+
// Contains, not HasPrefix: the sub-agent headline is tail-kept, so an
246+
// over-cap partial summary loses its leading bytes — the marker must
247+
// still be detected anywhere in the text (markers are unique sentinels).
248+
case strings.Contains(final, budgetSummaryMarker):
246249
return "iteration_budget", true
247-
case strings.HasPrefix(final, execBudgetSummaryMarker):
250+
case strings.Contains(final, execBudgetSummaryMarker):
248251
return "execution_budget", true
249-
case strings.HasPrefix(final, timeBudgetSummaryMarker):
252+
case strings.Contains(final, timeBudgetSummaryMarker):
250253
return "time_budget", true
251254
}
252255
return "", false

‎internal/loop/loop.go‎

Lines changed: 98 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ func formatLongToolRuntime(ms int64) string {
5050
return fmt.Sprintf("%dm%02ds", sec/60, sec%60)
5151
}
5252

53-
// dropStallFingerprints removes stall-map entries for one tool name so a
54-
// failing sibling cannot wipe a looping successful call's streak.
5553
func dropStallFingerprints(m map[string]int, toolName string) {
5654
if m == nil {
5755
return
@@ -64,6 +62,21 @@ func dropStallFingerprints(m map[string]int, toolName string) {
6462
}
6563
}
6664

65+
// dropStallMilestones mirrors dropStallFingerprints for the milestone map,
66+
// so a reset fingerprint re-arms at the base threshold instead of inheriting
67+
// an escalated milestone from earlier runs.
68+
func dropStallMilestones(m map[string]int, toolName string) {
69+
if m == nil {
70+
return
71+
}
72+
prefix := toolName + "\x00"
73+
for k := range m {
74+
if strings.HasPrefix(k, prefix) {
75+
delete(m, k)
76+
}
77+
}
78+
}
79+
6780
// evictLowestStallCount drops one fingerprint with the lowest repeat count
6881
// (key tie-break) so the 64-entry bound does not wipe the whole map.
6982
func evictLowestStallCount(m map[string]int) {
@@ -549,9 +562,10 @@ type Engine struct {
549562
compactMu sync.Mutex
550563
digestGen uint64
551564
digestCancel context.CancelFunc
552-
digestInFlight bool // a summarizer side call is running; new trims debounce instead of canceling
553-
digestDirty bool // drops arrived while the side call was in flight; refetch after it lands
554-
digestCallStart int // pendingDropped index where the in-flight call's input begins
565+
digestInFlight bool // a summarizer side call is running; new trims debounce instead of canceling
566+
digestDirty bool // drops arrived while the side call was in flight; refetch after it lands
567+
digestCallStart int // pendingDropped index where the in-flight call's input begins
568+
toolStallMilestones map[string]int // next stall-hint threshold per fingerprint (doubles per fire)
555569
pendingDropped []session.Message
556570
pendingDroppedCovered int // prefix length of pendingDropped the ready digest actually summarized
557571
pendingDigest string
@@ -2493,6 +2507,7 @@ func (e *Engine) runLoop(ctx context.Context, in []session.Message) (answer stri
24932507
e.maxConsecutiveToolErrors = make(map[string]int)
24942508
// Reset per-session repeated-call (stall) tracking
24952509
e.toolRepeatCounts = nil
2510+
e.toolStallMilestones = nil
24962511
// Reset the run's mutation ledger and completion-nudge state.
24972512
e.externalChargeMu.Lock()
24982513
e.externalReserved = 0
@@ -3508,6 +3523,9 @@ func (e *Engine) runLoop(ctx context.Context, in []session.Message) (answer stri
35083523
const (
35093524
errThreshold = 3 // consecutive errors before intervention
35103525
stallThreshold = 3 // consecutive identical successful calls before intervention
3526+
// bgPollStallThreshold: polling is legitimate (results change), so
3527+
// poll fingerprints get a 3× raised threshold before the hint.
3528+
bgPollStallThreshold = 3 * stallThreshold
35113529
)
35123530
var corrections []string
35133531
for idx, tc := range result.ToolCalls {
@@ -3527,6 +3545,7 @@ func (e *Engine) runLoop(ctx context.Context, in []session.Message) (answer stri
35273545
// this tool's fingerprints only so a looping sibling still
35283546
// accumulates toward the stall threshold.
35293547
dropStallFingerprints(e.toolRepeatCounts, toolName)
3548+
dropStallMilestones(e.toolStallMilestones, toolName)
35303549
} else {
35313550
e.maxConsecutiveToolErrors[toolName] = 0
35323551

@@ -3540,22 +3559,56 @@ func (e *Engine) runLoop(ctx context.Context, in []session.Message) (answer stri
35403559
// polling exists.
35413560
fp := toolName + "\x00" + tc.Function.Arguments
35423561
if isBGPollTool(toolName) {
3543-
// Background-job polling tools: leave the counters
3544-
// untouched — identical polling is normal, AND
3545-
// interleaved ordinary-call repetition must still
3546-
// count toward the threshold.
3562+
// Background-job polling tools: legitimate repetition, but
3563+
// BOUNDED — after 3× the normal threshold of identical
3564+
// polls (a pinned dead job id), fire the same hint. Count
3565+
// in the same fingerprint map with a raised milestone.
3566+
if e.toolRepeatCounts == nil {
3567+
e.toolRepeatCounts = make(map[string]int)
3568+
}
3569+
if _, ok := e.toolRepeatCounts[fp]; !ok && len(e.toolRepeatCounts) >= 64 {
3570+
evictLowestStallCount(e.toolRepeatCounts)
3571+
delete(e.toolStallMilestones, fp)
3572+
}
3573+
e.toolRepeatCounts[fp]++
3574+
milestone, mok := e.toolStallMilestones[fp]
3575+
if !mok {
3576+
milestone = bgPollStallThreshold
3577+
}
3578+
if e.toolRepeatCounts[fp] >= milestone {
3579+
pm := e.toolRepeatCounts[fp]
3580+
corrections = append(corrections, fmt.Sprintf(
3581+
"⚠️ You polled %q with identical arguments %d times with no state change%s. The job may be stuck or the id wrong: check bg_list for valid ids, read bg_output for the job's progress, or move on.",
3582+
toolName, pm, againSuffix(mok)))
3583+
e.emitSignal(SignalEvent{
3584+
Type: "tool_recovery",
3585+
Tool: toolName,
3586+
Detail: fmt.Sprintf("repeated identical poll (%dx%s)", pm, againDetail(mok)),
3587+
})
3588+
// Escalate via milestone, keep the count accurate.
3589+
if e.toolStallMilestones == nil {
3590+
e.toolStallMilestones = make(map[string]int)
3591+
}
3592+
e.toolStallMilestones[fp] = milestone * 2
3593+
}
35473594
} else {
35483595
if e.toolRepeatCounts == nil {
35493596
e.toolRepeatCounts = make(map[string]int)
35503597
}
35513598
if _, ok := e.toolRepeatCounts[fp]; !ok && len(e.toolRepeatCounts) >= 64 {
35523599
evictLowestStallCount(e.toolRepeatCounts)
3600+
delete(e.toolStallMilestones, fp)
35533601
}
35543602
e.toolRepeatCounts[fp]++
3555-
if e.toolRepeatCounts[fp] >= stallThreshold {
3603+
milestone, ok := e.toolStallMilestones[fp]
3604+
if !ok {
3605+
milestone = stallThreshold
3606+
}
3607+
if e.toolRepeatCounts[fp] >= milestone {
3608+
warned := e.toolRepeatCounts[fp] >= stallThreshold*2
35563609
correction := fmt.Sprintf(
3557-
"⚠️ You called %q with identical arguments %d times (possibly interleaved with other calls) with no new information. Change approach: vary the arguments, switch to a different tool, or move on to the next step — repeating the same call will not produce a different result.",
3558-
toolName, e.toolRepeatCounts[fp])
3610+
"⚠️ You called %q with identical arguments %d times (possibly interleaved with other calls) with no new information%s. Change approach: vary the arguments, switch to a different tool, or move on to the next step — repeating the same call will not produce a different result.",
3611+
toolName, e.toolRepeatCounts[fp], againSuffix(warned))
35593612
if e.planStore != nil {
35603613
if state, ok := e.planStore.Snapshot(); ok {
35613614
cls, _ := classifyToolCall(toolName, tc.Function.Arguments)
@@ -3578,12 +3631,16 @@ func (e *Engine) runLoop(ctx context.Context, in []session.Message) (answer stri
35783631
e.emitSignal(SignalEvent{
35793632
Type: "tool_recovery",
35803633
Tool: toolName,
3581-
Detail: fmt.Sprintf("repeated identical call (%dx, possibly interleaved)", e.toolRepeatCounts[fp]),
3634+
Detail: fmt.Sprintf("repeated identical call (%dx%s, possibly interleaved)", e.toolRepeatCounts[fp], againDetail(warned)),
35823635
})
3583-
// Reset this fingerprint's counter after injecting
3584-
// the suggestion (same semantics as the error-recovery
3585-
// counter).
3586-
e.toolRepeatCounts[fp] = 0
3636+
// Escalate, don't reset: the hint interval doubles
3637+
// (3 → 6 → 12 → …) so a hint-resistant loop gets
3638+
// progressively fewer interruptions while later hints
3639+
// say "again" — the loop is persistent, not correcting.
3640+
if e.toolStallMilestones == nil {
3641+
e.toolStallMilestones = make(map[string]int)
3642+
}
3643+
e.toolStallMilestones[fp] = milestone * 2
35873644
}
35883645
}
35893646
}
@@ -3795,12 +3852,34 @@ func (e *Engine) appendRemainingPlan(ctx context.Context, messages []session.Mes
37953852
}
37963853

37973854
// isBGPollTool reports whether the tool is a read-only background-job
3798-
// polling tool exempt from stall detection (identical arguments are the
3799-
// normal polling pattern; the result changes between calls).
3855+
// polling tool. Poll fingerprints get an elevated stall threshold rather
3856+
// than a total exemption (a pinned dead job id must not poll forever).
38003857
func isBGPollTool(name string) bool {
38013858
return name == "bg_status" || name == "bg_output"
38023859
}
38033860

3861+
// isPow2 reports whether n is a positive power of two.
3862+
func isPow2(n int) bool {
3863+
return n > 0 && n&(n-1) == 0
3864+
}
3865+
3866+
// againSuffix marks an escalated (repeat) stall hint so the model can tell
3867+
// it apart from the first one — the loop is persistent, not self-correcting.
3868+
func againSuffix(repeat bool) string {
3869+
if repeat {
3870+
return " again"
3871+
}
3872+
return ""
3873+
}
3874+
3875+
// againDetail is the signal-detail counterpart of againSuffix.
3876+
func againDetail(repeat bool) string {
3877+
if repeat {
3878+
return ", again"
3879+
}
3880+
return ""
3881+
}
3882+
38043883
// lastUserMessage returns the content of the most recent user message.
38053884
func lastUserMessage(messages []session.Message) string {
38063885
for i := len(messages) - 1; i >= 0; i-- {

0 commit comments

Comments
 (0)