fix(slackbot): avoid corrupting final delivery when the streamed-char offset lands mid-token#342
Closed
axis-guzus wants to merge 1 commit into
Conversation
… offset lands mid-token `continuationText` blind-slices the canonical `result_text` at `slackbot_streamed_answer_chars`. That offset counts the *source* characters streamed live, which is a different string-space than `result_text` — markdown normalization, whitespace reflow, and post-hoc answer revisions all change lengths, so the two drift. When the offset is misaligned the `text.slice(offset)` lands mid-token and drops characters (observed in production as e.g. "market cap" rendered as "marketountries", "defillama.com" as "vama.com"). We only have the count here, not the streamed string, so the prefix can't be verified — but we can refuse to cut mid-token: only treat the offset as a split point when it lands on a whitespace boundary in `result_text`; otherwise post the full answer (re-showing the already-streamed prefix is strictly preferable to emitting corrupted text). The aligned suffix-continuation path is unchanged. Adds a regression test; existing final-delivery tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
|
We've merged the Rust rewrite in #344, meaning the old API and Slackbot services are deprecated. As such we're closing this PR - if you think this is a mistake, please reopen the PR and ping me. |
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
Agent answers delivered to Slack are intermittently corrupted — characters are dropped or words merge (observed in production: "market cap" → "marketountries", "defillama.com" → "vama.com", "authenticate" → "thenticate"). The content is correct server-side; only the Slack render is mangled.
Root cause
continuationText(services/slackbot/src/centaur/final-delivery.ts) blind-slices the canonicalresult_textatslackbot_streamed_answer_chars:That offset counts the source characters streamed live (the per-delta source-char accumulator in
agent-session.ts/codex-session.ts), which is a different string-space thanresult_text:slack_codex_canonical_answer_correctionwhen the canonical answer length differs from what was streamed.When the offset is misaligned,
text.slice(offset)lands mid-token and drops characters. The offset is accumulated server-side withmax(...), so overshoot is sticky.Fix
continuationTextonly has the offset count, not the streamed string, so it can't verify the prefix. This guards the slice: only treat the offset as a split point when it lands on a whitespace boundary inresult_text; when it's misaligned, fall back to posting the full answer (re-showing the already-streamed prefix is strictly preferable to emitting corrupted text). The aligned suffix-continuation path (live delivery cut off at a clean boundary) is unchanged.Tests
Adds a regression test (offset lands mid-token → the full answer is posted, not a corrupted slice). The existing "posts only the missing suffix when live delivery was cut off" test still passes.
Note
A more complete fix would plumb the streamed prefix text through (or compute the continuation server-side, where both strings are available) so the split point can be verified exactly rather than heuristically. Happy to follow up if you'd prefer that direction — this change is a minimal, safe guard against the user-visible corruption.