fix(observer): emit <skip_summary/> on skip instead of empty response#3068
Open
funrun11 wants to merge 1 commit into
Open
fix(observer): emit <skip_summary/> on skip instead of empty response#3068funrun11 wants to merge 1 commit into
funrun11 wants to merge 1 commit into
Conversation
The SDK observation prompt (src/sdk/prompts.ts) tells the observer to return an empty response when a tool use should be skipped, and several mode skip_guidance fields repeat "return an empty response" / "no output necessary". But the output classifier treats a non-XML / empty reply as an invalid output (idle/prose). Three consecutive invalid outputs poison the SDK session, which is killed and respawned — a self-sustaining loop on quiet or low-signal sessions that drops all captured work and stores zero observations. The parser and the server-side providers (Gemini/OpenRouter/Claude) already standardize on a self-closing <skip_summary/> tag (src/sdk/parser.ts, src/sdk/output-classifier.ts, src/server/generation/providers/shared/prompt-builder.ts). This aligns the SDK observation path and the mode skip_guidance with that same contract, so a skip is emitted as a recognized, valid output instead of an invalid one that trips the poison threshold. Refs thedotmack#3032, thedotmack#3042. Complementary to the parser-side guards in thedotmack#3059 / thedotmack#2943 (fix the contract at the source rather than tolerating the empty reply). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Greptile SummaryThis PR aligns observer skip output with the existing XML skip contract. The main changes are:
Confidence Score: 5/5Safe to merge: the changes are limited to prompt guidance and mode configuration text that now matches the existing parser/classifier contract. The modified files consistently replace empty skip-output instructions with the recognized skip tag, and no runtime parsing or threshold logic was changed.
What T-Rex did
Reviews (1): Last reviewed commit: "fix(observer): emit <skip_summary/> on s..." | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On quiet / low-signal sessions the SDK observer poison-loops and stores zero observations:
Reproduced on 13.8.0 and 13.8.1 (Linux + Windows). Worker restart and version bump do not fix it.
Root cause — contract drift
src/sdk/prompts.tsinstructs the observer:and several mode
skip_guidancefields repeat "return an empty response only" / "No output necessary if skipping."But
src/sdk/output-classifier.tsclassifies an empty/non-XML reply asidle/prose→ invalid output, andResponseProcessor.tspoisons the session after 3 consecutive invalids. So a correct skip is scored as an invalid output and walks the session straight to the poison threshold.Meanwhile the parser and the server-side providers already standardize on a self-closing
<skip_summary/>tag:src/sdk/parser.ts—<skip_summary(?:\s+reason="([^"]*)")?\s*/>src/sdk/output-classifier.ts— treats<skip_summary/>as validxmlsrc/server/generation/providers/shared/prompt-builder.ts— "return a single self-closing<skip_summary />"Fix
Align the SDK observation path and the mode
skip_guidancewith that existing contract: on skip, emit<skip_summary reason="..."/>(a recognized, valid output) instead of an empty response. No parser/threshold changes.src/sdk/prompts.ts— central skip instructionplugin/modes/code.json,meme-tokens.json,law-study.json,law-study--chill.json,email-investigation.json—skip_guidanceTesting
Patched a local install, restarted the worker, ran a quiet file-reading session. Before: poison loop, 0 observations. After: skips emit
<skip_summary reason="..."/>(stored as valid,consecutiveInvalidOutputsstays 0), substantive reads distill into<observation>blocks and sync to chroma. No poison events.Notes
Refs #3032, #3042. Complementary to the parser-side guards in #3059 / #2943 — this fixes the contract at the source (don't emit the empty reply) rather than tolerating it downstream. The two approaches compose.