Skip to content

fix(thrift): use standard XOR zigzag decode to fix numeric message offset#201

Merged
EdamAme-x merged 1 commit into
mainfrom
fix/numeric-message-offset
Jul 6, 2026
Merged

fix(thrift): use standard XOR zigzag decode to fix numeric message offset#201
EdamAme-x merged 1 commit into
mainfrom
fix/numeric-message-offset

Conversation

@EdamAme-x

Copy link
Copy Markdown
Member

Summary

  • Fixed TMoreCompactProtocol.decodeZigZag() which used a multiplication-based formula (n >> 1) * (sign) instead of the standard XOR formula (n >> 1) ^ -(n & 1). The old formula was off by +1 for every negative value (odd encoded integers).
  • Fixed the type-16 string-ID delta decoder which used the same broken formula. Since lastStringId accumulates across the entire parse, each negative delta error compounded — causing numeric-only message text (e.g. "100") to arrive as "101", "102", or "103".
  • The offset grew with sending speed because more operations in a single sync batch meant more type-16 string fields with negative deltas, accumulating a larger error.
  • Added regression tests for decodeZigZag covering positive values, negative values, and round-trip with standard zigzag encoding.

Closes #200

Test plan

  • deno test packages/linejs/base/thrift/readwrite/tmc.test.ts passes (3 tests)
  • deno check passes
  • deno fmt passes
  • Manual verification: send numeric-only messages ("100", "200") at various speeds and confirm message.text matches exactly

🤖 Generated with Claude Code

Co-authored-by: pchrphhee pchrphhee@users.noreply.github.com

The multiplication-based zigzag formula `(n >> 1) * (sign)` produced
an off-by-one for every negative value (odd encoded integers), causing
numeric-only message text to arrive with a +1…+3 offset that grew
with batch size.  Switching both `decodeZigZag` and the type-16
string-ID delta to the canonical `(n >> 1) ^ -(n & 1)` fixes the
accumulation error.

Closes #200

Co-authored-by: pchrphhee <pchrphhee@users.noreply.github.com>
@EdamAme-x

Copy link
Copy Markdown
Member Author

thanks you @pchrphhee

@EdamAme-x EdamAme-x merged commit 09090f5 into main Jul 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: numeric-only messages received with +1 to +3 offset, worse when sent quickly (v3.1.5)

1 participant