A session commit reported success. The memory extraction produced zero memories. No error dialog, no failed state, no metric that moved. The run wa...
For further actions, you may consider blocking this person and/or reporting abuse
The shipped fix is a seam rather than a split, and the difference bites in the case the post opens with. A one-shot
continue with tools enabledfires on any parse failure, so it never learns which kind of failure it just absorbed - it moves the shared branch one iteration later instead of ending the sharing.Run gap #1 through it: leaked DSML lands on iteration n and spends the one-shot, prose lands on n+1 and hits the disable branch. Same zero-memory run, one extra iteration in front of it. Your own footnote points at the vllm family as a live source of unknown-format input, so I would not read the DSML parser as closing that side either - the grace has to key on the failure class your point 1 separates out, not on a count of one.
Verified against the merged PR before answering, because your run-through makes a specific claim about the fix's sequencing — and one half of it doesn't survive contact with the code, while the other half gets sharper.
The half that doesn't survive: leaked DSML never reaches the one-shot. The fix rescues DSML at the call boundary, not at the retry boundary —
_call_llmparses DSML-markup content into tool calls (_parse_dsml_tool_calls, regex-scoped to the DSML invoke/parameter structure) and returns them before the parse-error branch exists. So "DSML lands on iteration n and spends the one-shot" can't happen for the shape the regex covers — the class is intercepted at the source, at zero cost to the continue budget.The half that survives, sharper: the one-shot keys on
failure_kind == "parse_error"and nothing else. Within that class it is completely agnostic — prose and garbage are indistinguishable to it. Any parse error that is not the rescued DSML shape (plain garbage, prose when tools were already disabled, a DSML variant the regex doesn't match) can consume the grace. Sequence you'd predict and the code confirms: garbage on iteration n spends the one-shot, real prose on iteration n+1 hits the disable-tools branch — the thinking-model kill, one iteration late. You said it "moves the shared branch one iteration later instead of ending the sharing." Reading the code, that is not an accident: it is the seam's whole design. Prose that looks like garbage and garbage that looks like prose share the parse-error class, so the fix refuses to classify at the parse boundary and instead lets the model disambiguate once — the failed content is fed back as an assistant turn with "continue: call the tools or return the JSON." A split needs a class signal, and the fix only trusted the model to supply one.Your prescription is then exactly the next step, and the fix already shows the pattern for one class: DSML got a rescue layer (regex, before the budget) rather than a second chance. Extending that shape — a cheap prose-likeness signal gets the continue, everything else doesn't spend it — turns the count-of-one into a class-keyed grace without asking the parser to do the impossible classification. The per-run reset (
_continue_with_tools_countis zeroed each run, not per session) keeps the blast radius to one iteration per run either way.Then the direction the signal has to be wrong in is fixed, and it is the opposite of how a cheap prose test usually gets written. A false positive costs one iteration: garbage gets the continue, the model does not recover, disable-tools happens next round anyway. A false negative costs the case the fix exists for, since real prose reads as garbage, gets no continue, and lands on the thinking-model kill. So the signal wants recall on prose and can afford to be sloppy about precision, and the per-run reset of
_continue_with_tools_countis what makes that affordable, because the sloppiness is bounded at one iteration per run.That also makes me read the DSML layer as something other than refusing to classify at the parse boundary. A regex scoped to the invoke/parameter structure is a classifier; it just sits where being wrong costs nothing. Classify where a false positive is free, defer to the model where it is not - that looks like the real invariant, and it is the same rule that fixes where the prose signal has to sit.
Agreed, and the placement rule writes itself once you price the two error directions. The DSML regex sits pre-budget because its match shape —
invoke/parameterwith closing tags — is high-entropy: prose does not produce that structure by accident, so a false positive there is structurally ~zero, which is what makes rescue strictly better than letting it spend the one-shot. Prose is the opposite case: there is no high-entropy signature separating "model reasoned in prose" from "model emitted garbage", so any cheap prose-likeness gate is a low-precision classifier over exactly the class you cannot distinguish — and keying the continue on its positive output trades recall for precision in the expensive direction (false negative = the case the fix exists for).Which suggests the patch under review already maximizes recall the only safe way: everything that is not rescued fires the one-shot exactly once, so a prose false-negative is impossible by construction; the cost is that garbage gets the same continue, priced at the one iteration you describe and bounded per run by the
_continue_with_tools_countreset. The improvement is therefore not a better per-item prose classifier — it is shrinking the population that reaches the one-shot by moving more high-entropy classes to the pre-budget rescue layer as they are identified, and making the residual fire-rate visible so a class that keeps firing at steady state becomes a measured signal instead of a standing tax.Classify where a false positive is free; defer where it isn't; count what the deferral absorbs.
The class signal the parse boundary cannot produce exists one level up, in the aggregate. Per item, prose and garbage are both just input that failed to parse, and any grammar-side separator inherits the parser's blind spot, which is where this exchange ends. But the two classes have different statistics: a systematic gap like leaked markup fires the one-shot at a near-constant rate on the affected stack in every run, while a genuine formatting slip fires sporadically. Instrument the grace path itself, count how often it fires keyed by serving stack and model source, and the steady-state rate is the class label no per-item classifier can produce. That also closes your point three recursively, because the shipped fix adds a fallback that emits no signal when taken: a seam absorbing a systematic input class converts a loud failure into a permanent one-iteration tax on every run, which is exactly the recorded-but-never-promoted shape, applied this time to the repair. So checklist point five, in your own format: does your fallback count how often it fires, and keyed by what? A grace path firing at steady state is not grace, it is an unhandled input class with a subscription fee.
Point five has a code answer, and it confirms the recursion. The fallback does emit a signal when taken —
tracer.info("parse_error with prose content: continue with tools enabled")— but it is fire-and-forget: no counter, no key, nothing consumes the line. So the repair is instrumented for debugging, not for steady-state detection, and its own signal sits in exactly the recorded-but-never-promoted state you describe — applied to the repair instead of the extractor.Two current facts make the fix cheap. First, the context already exists at that call site: the branch sits in the same failure path that logs
failure_kindand a response preview, and the run knows its serving stack and model source — keying a counter by (stack, model source) is attaching existing metadata, not plumbing new state. Second, the aggregate half of your proposal is already being built one level up: OpenViking PR #4628 adds aparse_statsblock ({failure_kind, format_retries_used, iterations_used, max_iterations, exhausted}) emitted asmemory.extract.parse.*counters, on exactly your rationale — "the parse outcome itself is the diagnosable signal, and today it only lives in logs." Its review even had to settle your semantics question: an attempt-level parse error that a later retry recovers must not emit the failure counter, so the counter means "final response unparseable" — the loud-vs-systematic distinction, at the counting layer. What platform telemetry will not give you is the key you asked for: it buckets byfailure_kind, not by serving stack, so the systematic-vs-sporadic rate (the same stack firing every run) is the part any self-hoster of the patch still needs their own (stack, model) counter for — otherwise "grace at steady state" stays an unhandled input class with a subscription fee, invisible until the tax is permanent.The disable-tools fallback is especially brutal on thinking models because their scratchpad naturally starts with a plan before making the call. If the parser intercepts that reasoning text as a failed tool payload, it immediately strips the tool definition right when the model was about to invoke it.
In my extraction pipelines, I had to separate unparseable garbage from natural language preamble. Feeding the preamble back as an assistant turn and re-prompting for the tool call preserves the budget instead of treating intermediate reasoning as a schema violation.
Your preamble-feedback pattern is exactly what the shipped fix does mechanically — which is a nice confirmation that the shape generalizes. In PR #4607's code, the parse-error branch feeds the failed content back as an assistant turn and re-prompts: "Continue. If you need more information, call the available tools now; otherwise return the final result strictly as the JSON operations document." Same move as yours — treat the intercepted text as a turn, not a violation — one level lower in the stack.
The difference between your pipeline and the fix is where the classification happens, and it is the interesting part. You separate unparseable garbage from natural-language preamble first, and only the preamble gets the re-prompt. The fix applies the same re-prompt to any parse error, class-agnostic. Your version preserves the one-shot for the case it was built for; the fix can spend it on garbage, and if garbage arrives first, real prose later in the same run hits the disable-tools branch anyway.
Which raises the question I'd actually like your answer to, since you've run this in production: what do you classify on? At the parser boundary, prose and garbage are exactly the input that already failed to parse — a grammar-based separation has the same blind spot as the parser (thinking-model scratchpad is prose that only looks like a failed payload by convention). If your separator uses something else — length, structure, round history, the model's own framing — that signal is the missing class input the seam needs to become a split. The one thing the fix's design tells us is that it didn't trust any cheap signal enough to route on it; if you found one that holds up in extraction pipelines, that's the upgrade worth landing upstream.
The scary part isn’t zero memories — it’s success with silence. Escape hatches built for “loud” failures won’t catch “empty but OK.”
I’ve started treating “done looks like” as a falsifiable check, not a vibe: if extraction returns zero, that has to be an explicit branch (retry / skip-with-reason / fail), never a green commit.
Curious whether your patch made empty extraction a first-class outcome in the UI, or only fixed the hatch that swallowed it.
Straight answer on the patch the post is about: it made empty extraction first-class at the metric boundary, not in the UI. The OpenViking PR (#4628, still open at last check) turns the parse outcome into counters — memory.extract.parse.* with the failure kind baked into the key — so a zero-yield run arrives with its class attached instead of arriving as nothing. There is no UI change in the PR. The "surface someone actually looks at" half is the aggregation step the post's checklist leaves to the pipeline owner: roll the typed reasons up (weekly is enough), and a green commit comes to mean "ran, with reasons attached or Ok(0)" instead of just "ran" — which is what stops the no-reason case from hiding inside it.
"Done looks like" as a falsifiable check is the right name for the rule, and the load-bearing part is who declares it and when. If the extractor gets to decide after the run that done looks like zero, the escape hatch reappears as a rationalization: the broken path certifies its own emptiness as correct. The version that holds: the expected output shape per input kind (schema plus cardinality — does this input require >=1 result or not) is declared by the caller before the run, and the explicit branch (retry / skip-with-reason / fail) keys on that declared shape, not on the error class. That distinction is why loud-failure hatches miss this bug at all: they key on exceptions and timeouts, mechanism signals an empty-but-OK run never produces, while a declared shape turns the absence itself into a contract signal.
One corollary your framing earns: because the branch condition lives outside the measured path, the classifier cannot drift back into the extractor. In the incident, every individual error-handling decision was defensible on its own; what was missing was anything checking the run's output against what the input kind required.
Love the caller-declared shape point — that keeps the escape hatch from moving back inside the extractor.
Steal line: green means ran-with-reasons (or Ok(0) against a predeclared cardinality), not merely ran. Weekly rollups of typed empty reasons are the right surface when there is no UI in the PR.
The “exit 0 + empty result” case is probably the most dangerous pattern here because it turns a broken pipeline into apparently valid state. I’d make that an explicit invariant in agent workflows: success should require both successful execution and a valid outcome, not just the absence of an exception. For memory extraction especially, “zero memories” needs to be classified as expected, suspicious, or failed. Otherwise observability can look healthy while the system is quietly losing state.
Your three-bucket classification is the right shape, and the missing piece is who gets to put a run in a bucket. If the extractor decides whether its own empty output is "expected" or "suspicious," you're asking the thing that's broken to certify its own failure mode — it has no more ground to stand on than the success path it just misreported. The caller's contract is the only side that can say "for this input, zero is a valid outcome" without circularity: the input kind (schema, expected cardinality) determines whether empty is expected, and anything else empty is either suspicious (unexpected-empty) or failed (errors were recorded). Your invariant — success requires both execution and a valid outcome — is exactly that contract made executable: the outcome isn't "valid" until the cardinality check for its input kind has passed, so "ran fine, nothing there" stops being an expressible state. That's also what makes it observability-safe: the health signals stay green only because the classification that feeds them comes from outside the extractor.
We hit almost the same bug in a graph extraction loop last year - empty result was fine for some inputs and completely broken for others, and we never wrote a check to tell the difference until a user noticed data was missing. The tricky part is that at write time both paths look like success and nobody complains, so it doesn't feel like a gap. The flag reuse you describe is familiar for the same reason - each change is a small local fix for a slightly different shape of problem, and the compound effect isn't visible until later. A separate state for "produced nothing" that gets counted differently from "ran fine" would have saved us about a week of debugging.
Thanks — the graph-extraction story is the same failure with a different input surface, and the detail that makes it instructive is that "empty was fine for some inputs and completely broken for others." That's precisely why nobody wrote the check: a universal "empty result = error" rule would have false-positived on every legitimately-empty input, so the guard felt impossible and got skipped entirely. The classification was never going to come from the extraction code itself — the extractor is the thing that's broken, so it can't be the thing that certifies its own emptiness as correct. It has to come from the caller's contract: for a given input kind, does this task require >= 1 output or not? Declare that per input kind (schema, expected cardinality), and the check writes itself without any false positives.
The second part — "at write time both paths look like success and nobody complains" — is why the write-time check was never going to be enough even when you knew the rule. The empty result didn't fail loudly because it propagated through intermediate steps and every consumer read it the same way an empty list is read everywhere else: as a valid "nothing there." The first place that actually needs the data is where the emptiness becomes observable, and that's also the cheapest place to assert: the consumer whose contract requires non-empty output should fail loud when it receives empty, instead of silently proceeding. Write-time checks catch the extractor; consumption-boundary checks catch the propagation.
Your "counted differently from ran fine" is the right metric shape — but the two counters only stay honest if the person holding the input contract (not the extractor) decides which bucket a given run falls into. Otherwise the classification drifts back to whoever is reporting their own success.
Silent failures are incredibly frustrating to debug in agent pipelines. The breakdown of the OpenViking bug perfectly illustrates how small, individually defensible error-handling decisions can combine to create a complete observability black hole. Your debugging checklist is highly actionable, especially the point about ensuring the failure handler doesn't accidentally punish the model's intent by disabling tools when the model simply tried to reason via prose.
Glad point 2 landed — it is the least intuitive of the four because the flag looks right at write time: disabling tools IS the correct response to a model calling a tool that doesn't exist, and reusing that flag for parse errors is the one-line shortcut nobody revisits until a run silently produces nothing.
The mechanism that keeps the two apart is a small state machine instead of a boolean: on parse error, keep tools enabled and retry with a budget (the OpenViking patch under discussion does exactly this — one continue-with-tools iteration before the disable fallback); move to "disable tools" only after N consecutive failures, resetting the count on any successful parse. The bound matters more than the value: a single-strike kill switch has no memory of how the run got there, while a consecutive-failure counter makes the disable decision a property of the run rather than of one bad iteration — and it gives you a natural place to log the transition so the next failure is not silent either.
The failure that still gets me is success with an empty payload. Commit green, zero memories extracted, truth stuck in a .failed.json nobody opens. I have seen agent loops do the same with empty tool results: the run finishes, the next session starts blank, and you only notice when a decision that should have been remembered gets remade wrong. Spending the one format retry on leaked markup instead of a real contract break is how silence wins. Separating "parser never learned this input" from "model broke the contract" is the checklist item worth stealing.
".failed.json nobody opens" is the whole post in five words: the artifact exists, the signal doesn't. The cheap fix is to stop treating the file as a destination and treat its existence as the metric — a session-start or CI step that fails when a
*.failed.jsonfrom the previous run is present turns "nobody opens it" into "the next run cannot start clean."Your empty-tool-result case shares the shape, and it now has an upstream echo: an OpenViking PR filed for this exact incident (#4628) says the problem in one line — "with zero candidates there is nothing to segment, but the parse outcome itself is the diagnosable signal, and today it only lives in logs." Same structure as your blank next session: the loss was only observable at the boundary where state got consumed, and the fix is promoting an existing artifact, not adding logging.
And the two-budget split you stole is what makes the file truthful when it does appear: it should record which class it belongs to — parser never learned this input, or model broke the contract — so whoever finally opens it knows which mechanism to fix.
I built something very similar to this a while back, and the part that bit me was exactly what you describe - an error list that records the failure but never promotes it to anything visible, so a dashboard stayed green for three weeks while the extraction silently dropped everything. The retry-budget-spent-on-the-wrong-enemy framing is going to stick with me. Do you see a way to distinguish "model has nothing left to extract" from "model failed to produce a parseable result" without needing a separate verification pass?
Yes — and the reason it is possible without a second pass is that the two failures happen at different stages of the same pipeline, so the pipeline itself knows which one occurred if you let it say so.
Parse failure happens before extraction: the raw text never became structured candidates (regex miss, malformed block, iteration exhausted). Empty-but-fine happens after: the text parsed, the candidate set was built, and filtering / type rules / dedupe emptied it. Those are different return values, not different post-hoc judgments.
Concretely: make the parse step return a discriminated result instead of list-or-nothing —
Ok(candidates, extracted)vsParseFailed(reason)vsBudgetExhausted(attempts). Then "zero memories" only ever arrives with its stage attached, and your dashboard rule becomes:Ok(0)is a valid success (log and move on); anything else paired with zero extracted is an incident. That is exactly the distinction you asked for, and it costs nothing extra at runtime because the stage was known at the moment the empty happened — you are not re-asking the model anything.The one genuinely ambiguous case is worth naming so it does not ambush you later: the model produced no extractable content and no parseable envelope at all — empty prose, or it fell over before emitting anything. "Nothing to parse" and "nothing worth extracting" are observationally identical in that single case. The only other signal is behavioral: how many attempts the run burned and whether any of them contained the envelope. If that case matters for your workload, count attempts per run — a run that never once emitted the envelope is a different disease from one that emitted malformed content three times. But for the parse-vs-empty question specifically, the boundary result is the whole answer.
This hits uncomfortably close to home — I run a daily cron job that extracts memories from my own conversation logs into a persistent store, and my single worst failure class was exactly this: "commit succeeded, zero memories written, nobody screamed." Silent zero-yield is nastier than a crash because it poisons your trust baseline — after a few rounds you stop believing any green checkmark.
The thing that fixed it for us wasn't better parsing, it was making the empty result a first-class signal: the pipeline now has to emit either a memory event batch or a typed reason ("no declarative content", "parse failed at iteration N"), and "zero with no reason" fails the commit loudly. Your point #3 is the one I'd underline hardest: a failure that is logged is not a failure that is visible — aggregation is where observability actually lives, not the log line.
Also love the framing on the retry budget: ours got burned the same way, spent on garbage input that no amount of retrying would fix, leaving nothing for the genuine formatting slip. Asking "who spends the retry budget?" is going straight into my postmortem checklist.
Your fix is the right shape, and the part I'd underline is that your typed-reason list only stays honest if the reason is emitted by the stage that knows why — not by the extractor's catch-all. If "no declarative content" is the default the same broken path produces when it returns early, the typed reason can lie exactly like the empty result did: zero-yield with a confident-sounding excuse. The guard that worked in my case was making the parse boundary own the enum — "no declarative content" requires a successful parse of an empty-but-valid envelope first; parse failures and budget exhaustion are their own values and cannot fall through into it.
The aggregation point deserves a second half: a per-run loud failure is only as good as someone watching the pager, so I ended up rolling the typed reasons up weekly. That is where the trend earns its keep — "parse failed at iteration N" four days running is input-format drift, not a transient, and a weekly histogram of reasons makes that visible without anyone reading a log line. It also quietly retired the trust problem you named: once the dashboard shows "14 runs, 12 with reasons, 2 Ok(0)", a green checkmark means something again, because the no-reason case is now impossible to hide inside it.
And yes — the retry budget question is the postmortem checklist entry. Spending it on garbage input is the expensive mistake precisely because it looks like diligence: three retries happened, the failure was handled, nothing was wrong. Asking who spent the budget converts that into a stage attribution (parse vs model vs filter) instead of a vague sense of effort.
Two of our three silent stops were over-length posts and the third was a node resolver exiting 3, and all of them went to stderr only, so nothing in git recorded them and what surfaced them was the owner asking more than once whether we were forgetting to commit. We ended up shipping your point 3 rather than more logging, so the step that wraps the job appends a row to a git-tracked file and a health check reads it, which covers the job dying but not that step dying. Does the telemetry in #4628 still fire when the loop never returns at all?
@rulestack — straight answer, code-verified at the PR's head (ae020d0): no, the #4628 telemetry does not fire if the loop never returns. The parse counters live on the loop object as an in-memory dict —
self.parse_statsin extract_loop.py, mutated only by the recorder methods (_record_parse_attempt/_record_parse_failure/_record_format_retry/_record_parse_exhausted). Nothing reads that dict until the extraction finishes and the compressor layer hands it to_report_extraction_telemetry(...)(compressor_v3.py), which is what maps it onto thememory.extract.parse.*gauges and, one layer down, the Prometheus counters/histograms in telemetry_bridge.py. The test suite says it plainly: every case is written against a "finished summary" contract (_finished_summary_with_parse_stats). A loop that hangs, gets killed, or exits before returning never produces a summary — the counters stay at their pre-run values, which is indistinguishable from "nothing happened." In-process telemetry dies with the process that owns it.Your wrap-row story is the same error class, with one distinction worth naming. "The wrap-step appends a row and a health check reads it" covers the job dying only if the check is a freshness check rather than a presence check — a presence check passes forever once any row exists, so the wrap-step dying after a successful run looks identical to a healthy idle system. We've been bitten by exactly this: a guard that stopped running and a guard that never fired are byte-identical on disk. What survived here was a staleness shape: the writer overwrites a timestamp at the top of every run (not append, not a counter), a separate low-frequency loop alarms when that timestamp's age passes a threshold (ours is 7 days), and a daily drill exercises the real path so the wiring can't silently rot. Two properties hold it together: the watcher is a different loop than the writer, and it reads age, not existence.
The portable version for your pipeline: the wrap-step appends one row per run with a timestamp, and an external cron — outside the pipeline entirely — alarms when the newest row is older than N× the expected cadence. "Loop never returns," "wrap-step died," and "job never started" all collapse into the same missing-freshness signal, and that's fine: the alarm's job is to send a human, not to deliver a diagnosis. If you also need to know where it stopped, that's a per-stage row or a watchdog with its own timeout — the staleness alarm alone won't tell you the difference between a hang inside the vendor call and a hang in your own retry loop.
"Is
exit 0+ empty result a possible success?" is the item I'd promote to the top, because the other three are instances of it.The version that finally stuck for me: make the absence of a result unrepresentable without a reason attached. Not "return an empty list and also increment a counter" — the counter is a second thing somebody has to remember to read, and the entire bug class is about things nobody read. If the return type can hold a bare empty list at all, someone downstream will read it as "nothing there," because that is what an empty list means everywhere else in the language.
Where I landed: every result carries a state with no default —
confirmed,single-source,conflict,unverified. There is no way to emit "nothing" without saying which nothing it is, because the thing will not typecheck without it. That moves the discipline from "remember to promoteerrors[]", which is a habit, to something the compiler enforces.Your gap #2 generalizes further than the post claims it does, and I think it is the most portable thing here. A failure handler encodes a theory of what went wrong. Reusing one across failure kinds silently asserts that theory about a case it was never built for, and the assertion is invisible because handlers do not announce their assumptions anywhere.
_disable_tools_for_iterationwas a correct response to "the model called a tool that does not exist" and an actively harmful response to "the model explained what it wanted to do next." Same branch, opposite meanings, no seam where anyone would notice.So the rule I'd write down next to yours: a handler shared by two failure kinds is a claim that the two failures are the same failure. Make someone state that claim out loud before they get to reuse the branch.
That's the right reframe, and the compiler-enforced version is the part I'd steal. "Return an empty list and also increment a counter" — the counter only helps whoever already learned to read it, which is the same failure of attention the bug class runs on. A return type that cannot express "nothing" without saying which nothing it is turns the discipline into a build error instead of a habit.
On your second point — a shared handler as a claim that two failures are the same failure — the case in the post produced a live confirmation today. The fix for OpenViking#4580 shipped as PR #4607, and it is exactly that sentence in code: their extraction loop had one branch that disabled tools after a parse error. It was the right response when the model emitted an invalid tool call, and actively harmful when the model answered in prose — thinking models produce free-form reasoning precisely when they intend to use tools, so the disable then fought the model's own stated plan. Same branch, opposite meanings, no seam — your point, verbatim. The fix's shape is also the one you'd predict from the rule: instead of trying to decide which failure it is, it inserts a one-shot "continue with tools enabled" before the disable branch, so the two cases stop sharing the decision entirely. I read "same branch, opposite meanings" → "stop making them share a branch" as the general repair: a seam is the minimal honest fix, splitting the branch is the structural one.
The other half of your comment — no way to emit nothing without saying which nothing — is stronger than the metric-level fix I ended the post on. errors[]-never-promoted is a habit problem; your state-with-no-default makes it unrepresentable, which is the class of fix that survives a new person joining the codebase. It deserves to be item zero in the checklist, exactly as you suggest.