fix(persona): retain observations when a digest is truncated - #145
fix(persona): retain observations when a digest is truncated#145YellowSnnowmann wants to merge 2 commits into
Conversation
A dense window (e.g. a Codex session full of corrections/directives) distils into many facet observations, and the 4 K output-token cap was sized for "one small JSON object". The model ran out of output mid-array and emitted a well-formed prefix with no closing `]` (observed `EOF while parsing a list` at column ~15-19 K). Two independent bugs made that lossy: - `digest_window` collapsed any parse failure to `Ok(Vec::new())`, so a truncated response was indistinguishable from a genuinely-empty digest. - `digest_and_fold` commits the window cursor for ANY `Ok` result, so the truncated window was marked done and its observations were dropped for good, never retried. Fix: - Raise `DIGEST_MAX_OUTPUT_TOKENS` 4_096 -> 16_384 so dense digests fit, and document the coupling to `WINDOW_CHARS`. - Introduce `DigestError` (a distinct, retryable failure type) and return `Err` for an unparseable/truncated response instead of a fake-empty `Ok`. It flows through the caller's existing hard-failure arm, which already `continue`s without committing the cursor, so the window is re-attempted next run. A genuinely-empty digest still returns `Ok(vec![])` and commits, since re-running would only reproduce it. Tests: flip the bad-JSON case (now a non-committable `Err`), add a truncated-array case, and a pipeline test asserting a truncated window leaves its cursor absent and is re-processed on the next run. Closes #5510
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe digest token limit increases to 16,384. Provider and parsing failures now remain retryable and prevent cursor commits. Valid empty digests still commit. Tests cover truncated responses and successful retries. ChangesPersona digest retry handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: 🔵 Low · up to Malformed digests will now be retried instead of silently losing observations, but a persistently failing window could repeat provider work, cost, and transcript transmission across runs. The impact is bounded per run and the PR is mergeable with owner awareness of this follow-up risk. Possibly related issues
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0287 · 32,798 in / 8,928 out · 27,931 cached (85%) · z-ai/glm-5.2
critique: $0.0114 · 9,978 in / 3,849 out · 8,564 cached (86%) · z-ai/glm-5.2
security: $0.0079 · 9,894 in / 2,404 out · 8,494 cached (86%) · z-ai/glm-5.2
tests: $0.0055 · 5,825 in / 1,737 out · 4,872 cached (84%) · z-ai/glm-5.2
description: $0.0023 · 6,256 in / 351 out · 5,225 cached (84%) · z-ai/glm-5.2
Summary
Bulk coding-session ingest silently dropped observations on dense sessions.
DIGEST_MAX_OUTPUT_TOKENS = 4096truncated digest responses mid-JSON (EOF while parsing a list), the parse failure soft-failed toOk(vec![]), and the caller committed the window cursor anyway — so the window was marked done with zero observations captured, unrecoverably.This PR:
DIGEST_MAX_OUTPUT_TOKENS4096 → 16384 (documented inline; dense Codex digests exceed 4096, matching the observed ~15–19k-column truncations).DigestErrorso a truncated/unparseable digest returnsErrinstead of a committable empty result. The caller's existing hard-failure arm then skips the cursor commit, so a truncated window is retried on the next ingest run rather than lost. Genuinely-empty digests still commit as before.Addresses tinyhumansai/openhuman#5510, and the digest half of tinyhumansai/openhuman#5509. The other half of #5509 (the OpenHuman-side ingest RPC timeout budget) is a separate OpenHuman PR that will also bump this submodule pointer once this merges.
API Or Behavior Changes
digest_window/digest_sessionnow propagate a truncated/unparseable digest asErr(previously a silent emptyOk). Public return types are unchanged (anyhow::Result). A truncated window no longer commits its cursor.Tests
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo build --all-targetscargo testUpdated
distill_tests.rs(bad/truncated JSON now asserts a non-committableErr, notOk+empty) and addedpipeline_tests.rscoverage proving a truncated window does not commit its cursor and is re-processed on a later run.Documentation
None needed — the token-cap rationale is documented inline at the constant.
Summary by CodeRabbit