Skip to content

Replace byte-period detection with token-based per-chunk detection #66

Description

@weselben

Derived from: #54, #55, #57, #62, #64

Work:

  • Replace the prototype byte-period detection in internal/streaming/repetition_guard_stream.go with token-based detection.
  • Use the latest pure-Go tiktoken port (chose during this work).
  • Run detection per chunk on the rolling token stream. No holdback queue. No straddling-event rewrite.
  • Trigger on N consecutive repeats of any pattern length up to max_pattern_size.
  • Remove queuedEvent, rewriteEventAtCut, and releaseSafeEvents.
  • Keep the package-internal detectRun helper or replace it with a token equivalent.
  • Update unit tests in internal/streaming/repetition_guard_stream_test.go.
  • Brown-test it against the producer in /tmp/stream-repetition-brown/.

Done when: go build ./... passes; go test ./internal/streaming/... passes including the new token-based cases; the brown test shows truncation + [DONE] from a real HTTP SSE loop source.

Activity

  1. weselben commented on Sep 1, 2026

    @weselben
    OwnerAuthor

    The #65 research tuned the detection defaults — fold these into this task:

    • limit = 3 (not 2)
    • maxUnit = 32 chars (not 256)
    • minRunBytes = 96
    • Run the suffix scan on a rolling 4096-char tail every 128 chars (Z-array exact-cycle, oh-my-pi trick)
      Full heuristics + skip list live on Add obvious-code and tool-call skip heuristic #71.
  2. weselben commented on Sep 1, 2026

    @weselben
    OwnerAuthor

    Scope update from #73: max pattern length is config-driven, not hardcoded. Read the per-request effective value (model override → global STREAM_REPETITION_MAX_PATTERN → 8) and pass it into the detector alongside the limit. Everything else in this task is unchanged.

  3. weselben commented on Sep 1, 2026

    @weselben
    OwnerAuthor

    Repo-conformant notes (from review):

    • Mirror internal/streaming/slowdown_stream.go: io.ReadCloser wrap, closeOnce on the source, ctx-aware cancellation. Reuse parseDataLine, nextEventBoundary, donePayload, lfEventBoundary from observed_sse_stream.go (already exported in package).
    • Replace bytes.Buffer output queue with streamBuffer (from stream_buffer.go) — already pooled, non-concurrent FIFO, the right shape. Minimal diff.
    • Drop the queuedEvent / rewriteEventAtCut / releaseSafeEvents machinery entirely — eager per-chunk means no holdback.
    • Detector: rolling token tail of last limit * maxPattern tokens. limit=3, maxPattern=8 defaults (from Legitimate-repetition heuristics: what counts as 'obvious code'? #65 research); both come from the per-request config (r.repetitionLimit, r.repetitionMaxPattern).
    • Skip heuristics come from Add obvious-code and tool-call skip heuristic #71 — apply them here.

    Test style: internal/streaming/observed_sse_stream_test.go uses table-driven testing (no testify) with a recordingReadCloser and strings.NewReader. Use the same shape — TestRepetitionGuardStream_DisabledPassthrough, TestRepetitionGuardStream_TokenStutter, TestRepetitionGuardStream_ChainLoop, TestRepetitionGuardStream_CleanStream, TestDetectRun table-driven. Brown-test-style producer moves to #72.

    Minimal-diff goal: repetition_guard_stream.go shrinks vs the prototype; slowdown_stream.go untouched; observed_sse_stream.go untouched; stream_buffer.go reused as-is.

    This runs on 400+ machines in production — every code path (active, off, fallback, skip) covered by a unit test before merge.

  4. self-assigned this
    on Sep 1, 2026
  5. weselben commented on Sep 1, 2026

    @weselben
    OwnerAuthor

    Working on this. Order: TokenCounter interface + lazy loader first (#67 deliverable, in internal/streaming/tokenizer.go), then the guard refactor to consume it (#66). Byte-period stays as the fallback implementation behind the same interface per #64. Committing incrementally on feat/stream-repetition-canceller; this ticket closes when the guard is token-based and tests cover all paths.

  6. weselben commented on Sep 2, 2026

    @weselben
    OwnerAuthor

    Resolved on feat/stream-repetition-canceller (commit 9707690).

    What changed:

    • internal/streaming/repetition_guard_stream.go rewritten: per-chunk token detection via a rolling token tail (detectTokenRun / tokenTailRepeats); byte-period detector kept only as fallback.
    • Eager trigger, no holdback queue, no straddling-event rewrite: events pass through before inspection; on trigger the upstream is closed and a synthetic data: [DONE] is appended.
    • internal/streaming/tokenizer.go + tokenizer_test.go: TokenCounter over github.com/ron2111/omnitoken v0.1.7 (pure-Go tiktoken port).

    Evidence:

    • go test ./internal/streaming/... green (TestRepetitionGuardStream_TokenStutter, _ChainLoop, _FallbackStutter, _TriggerBeforeUpstreamEOF, TestTokenTailRepeats, TestClampGuardParams).
    • Brown e2e over real HTTP SSE (worktree .tmp/stream-repetition-brown): guard triggered on gpt-4o tokenizer, upstream cut after 17 events, client saw 4 loop phrases, output ends with [DONE].

    🤖 Written by Kimi Code (AI agent) from the stream-repetition-canceller worktree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions