Fail Claude Code runs that end with a degenerate final message

[knechtionscoding]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Claude Code can finish a run cleanly — subtype: success, is_error: false,
stop_reason: end_turn, terminal_reason: completed — after doing substantial
real work, and still emit markup fragments instead of an answer as its final
message. A Slack-facing agent did exactly this in production, ending a
41-turn run with:

`stale=False</li>
</ul>
</section
</section>

The completion gate in kelos-capture only inspects the termination fields, so
this passes, the Task is recorded as Succeeded, and the fragments are reported
verbatim.

This adds a content predicate. A result is classified ResultIncomplete when
all of the following hold:

1. it would otherwise be ResultCompleted (an already-failing run is not
   second-guessed),
2. num_turns >= 10 — the agent did substantial work, and
3. the trimmed final message is under 200 characters.

ResultIncomplete is reused rather than adding a new status, because it already
makes kelos-capture exit non-zero. The Job's existing unconditional
backoffLimit: 1 turns that into a rerun, and isJobFailed waits for the
JobFailed condition rather than job.Status.Failed > 0, so the Task stays
Running across the retry. No new retry machinery is needed.

kelos-capture also emits a degenerate: true output key on the failing
attempt (run() emits the marker block even when the exit code is 1), so
downstream consumers can distinguish this failure from any other without
duplicating the heuristic.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

The predicate measures characters, not tokens. usage.output_tokens on the
result line appears to be cumulative over the session, not per-final-turn, so a
run with 40 tool calls carries a large output_tokens even when the final
message is junk — a token-based floor would silently never fire. result is
unambiguously the final message, so measuring its length sidesteps the question.
Length is counted in runes, not bytes, so the floor means the same thing for a
non-ASCII answer as for an ASCII one.

The degenerate marker is gated on the classification, not the length
floor. IsDegenerateOutput() is independent of Status(), so an explicit
error result (subtype: error_max_turns) whose short error text trips the same
floor would otherwise be labelled degenerate. That is a different failure, and
mislabelling it would hide the real reason from downstream consumers, so the
marker is only set when the result is ResultIncomplete and degenerate.

This is not provider-side truncation. FireRouter was probed directly against
deepseek-v4-flash-0731: max_tokens: 8 returns stop_reason: max_tokens and
max_tokens: 512 returns end_turn. stop_reason is forwarded faithfully, so
the existing max_tokens guard already catches genuine truncation. The garbled
run came back as a real clean end_turn; content is the only remaining signal.

A false positive is cheap. If only the first attempt is misjudged, it costs
one retry and is invisible to the user — the Task stays Running and Slack
shows only the progress message. If both attempts are misjudged, the Task goes
Failed and Slack posts the error header with results["response"] still
attached, so the user still gets their answer. A false positive never loses an
answer, which is the argument for erring toward detection.

The thresholds (10 turns, 200 chars) are deliberately conservative and not
yet calibrated. There is no metric for final-message length today; the
distribution has to come from task.Status.Results["response"] on completed
Tasks. kelos_task_output_tokens_total is labeled by spawner as of c2faf5f1,
so the token distribution per spawner is already queryable, but per the note
above tokens are the wrong measure here. Intent is to ship conservative and
tighten once the distribution is known.

Backward compatibility: older Claude Code versions that omit num_turns leave
it at zero, so the predicate never fires for them — matching how Status()
already tolerates empty reason fields. There is an explicit test for this.

Tests cover the real garbled sample after a high turn count, a legitimate brief
answer after two turns ("Fixed in v0.472." — the case the predicate must not
break, since the general spawner answers casually all the time), a long
coherent answer after many turns, the charFloor and turnFloor boundaries,
the missing-num_turns case, and that degenerate: true reaches the captured
outputs on a failing attempt.

make test and make verify pass.

Does this PR introduce a user-facing change?

Claude Code runs that terminate cleanly but return a final message too short to be an answer after many turns are now treated as failures rather than successes, so the Task's existing retry reruns the agent instead of reporting garbled output. `kelos-capture` emits a `degenerate: true` result key on such an attempt.

[cubic-dev-ai]

All reported issues were addressed across 7 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[gjkim42]

/kelos claude-review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-claude-reviewer-pull-request-review-e0adcfe11cc0 has succeeded. ✅

[kelos-bot]

🤖 Kelos Claude Reviewer Agent @gjkim42

Review Summary

Verdict: APPROVE
Overall correctness: patch is correct
Scope: Classifies a clean-terminating Claude Code run as ResultIncomplete when it took ≥10 turns but ended with a <200-char final message, so kelos-capture exits non-zero (triggering the Job's existing retry) and emits a degenerate: true output key on the failing attempt.

Findings Overview

Priority	Count	File:Line	Summary
P0	0	—	none
P1	0	—	none
P2	2	internal/capture/usage.go:112	degenerate marker and "degenerate final message" headline also fire for runs incomplete for unrelated reasons (stop_reason=tool_use/max_tokens)
P2		docs/reference.md:1096	New user-facing degenerate output/results key missing from the status.outputs/status.results reference table
P3	0	—	none

Findings

Correctness / Documentation accuracy

• [P2] internal/capture/usage.go:112 (also internal/claudecode/result.go:114) — The gate status == ResultIncomplete && completion.IsDegenerateOutput() does not implement the adjacent comment's claim ("Marked only when degeneracy is what makes the run incomplete") or the PR description's rule ("it would otherwise be ResultCompleted"). A result that is incomplete for an unrelated reason — e.g. stop_reason: "tool_use" with num_turns: 41 and the short text "Starting the next tool" (the existing incomplete-result fixture plus a num_turns field, which current Claude Code always emits) — trips the length floor too, so it gets degenerate: true and its diagnostic changes from Claude Code run incomplete (stop_reason=tool_use) to Claude Code returned a degenerate final message (stop_reason=tool_use, degenerate_output=...). That is exactly the mislabeling the PR's second reviewer note argues against for error results, applied to the other-incomplete class, and it also rewrites the headline for genuine max_tokens truncation. Suggested fix: require clean termination fields inside IsDegenerateOutput() ((TerminalReason == "" || TerminalReason == "completed") && (StopReason == "" || StopReason == "end_turn")), or compute the status without the degenerate check and compare to ResultCompleted. The test "degenerate output alongside an explicit stop reason" in internal/claudecode/result_test.go pins the current behavior and would need updating with the fix.

Documentation completeness

• [P2] docs/reference.md:1096 — The release note announces a new user-facing result key (kelos-capture emits a degenerate: true result key), but the canonical status.outputs list (branch, commit, base-branch, pr, cost-usd, input-tokens, output-tokens) and the status.results row on the next line are not updated. docs/agent-image-interface.md covers the mechanism, but a user inspecting task.status.results should find the key in the reference table. (The list also omits the pre-existing response key — not introduced by this PR, but worth fixing in the same edit.)

Suggestions (optional)

• [P3] internal/claudecode/result.go:15 — The "a false positive is cheap" argument assumes misjudgment is per-attempt random, but this predicate is deterministic on content: a question whose correct answer is legitimately short after ≥10 turns (e.g. an operational check ending in "All 47 pods are healthy.") will fail both attempts identically — the Task always goes Failed, agent cost doubles, and Slack shows the error header alongside the correct answer. The tests cover a brief answer at 2 turns but not this class. Since the thresholds are explicitly uncalibrated, consider calibrating against the Results["response"] length distribution soon after merge, and/or narrowing the predicate to fragment-shaped content (e.g. unbalanced markup) rather than length alone.

Key takeaways

• The core mechanism is sound and well-tested: the retry path (backoffLimit: 1 + isJobFailed waiting on the JobFailed condition), results being read from the latest pod only at the terminal transition (so a successful retry cannot inherit degenerate: true), and the Slack failure path attaching results["response"] all check out in the existing code.
• The one behavioral gap is that the degenerate marker's stated "only when degeneracy is the cause" contract is not enforced for the non-degenerate incomplete class (tool_use/max_tokens), which dilutes the key's meaning for the downstream consumers it was added for.