Skip to content

fix(parser): keep anchors when a stop token's text is stripped - #640

Merged
lukealonso merged 4 commits into
local-inference-lab:dev/jovian-judgementfrom
logprobz:fix/scanner-stop-stripped-drop
Sep 4, 2026
Merged

lukealonso merged 4 commits into
local-inference-lab:dev/jovian-judgementfrom
logprobz:fix/scanner-stop-stripped-drop

Conversation

@logprobz

@logprobz logprobz commented Sep 4, 2026 •

Copy link
Copy Markdown

Problem

Serving can strip a stop token's text from delta_text while retaining its ID in delta_token_ids. The non-streaming path likewise passes output.text together with model_output_token_ids to the parser.

When the retained stop ID is an auto-assigned DROP_TERMINAL, TokenIDScanner._recover_holdback_text cannot match the reconstructed text. Its fallback, _rebuild_from_anchors, resolves anchors right to left. A missing trailing DROP can therefore bind a structural anchor such as <tool_call> to a literal lookalike earlier in the text. With one text token followed by a stripped stop ID, the fallback can also lose the text token count.

Fix

When reconstructed text does not match, _recover_holdback_text retries without trailing DROP terminals. If the retained items match the suffix of delta_text, it returns them unchanged and defers the DROPs with zero prefix token counts. A later scan() or flush_pending() still emits the deferred terminals.

The previous find check is replaced by endswith. Detokenizer holdback can only precede the current token reconstruction, so a valid reconstruction must be a suffix. A middle match now follows the existing anchor-rebuild path instead of silently discarding trailing text.

Compatibility and side effects

  • This is a generic TokenIDScanner change and affects every parser-engine configuration with auto-DROP special tokens.
  • A stripped trailing DROP is now retained internally until the next scan() or flush_pending(). StreamingParserEngine.finish() always flushes scanner state.
  • Direct internal scanner callers must also resolve or flush pending items. Existing callers and tests do so.
  • Multiple trailing DROPs keep their original order, and retained text-token counts remain attached to the retained text chunks.
  • This PR is recommended to land before or with fix(parser): keep GLM tool delimiters inside argument values #639. The combined parser-engine suite passes.

Related work and duplicate check

This is the focused scanner portion split from closed PR #540. The GLM delimiter-state change is #639.

Open-PR searches found no upstream PR implementing the stripped trailing DROP recovery. This is not a duplicate of vllm-project/vllm#54971, which changes GLM argument-key parsing and does not touch TokenIDScanner.

Tests

Pytest validation used Python 3.12 in /opt/venv inside the existing rtx-station/glm53-jovian:lp15 image. Source and temporary test dependencies were mounted read-only. Lint validation used the repository-pinned Ruff version through uvx on the host.

/opt/venv/bin/python -m pytest \
  tests/parser/engine/test_token_id_scanner.py::TestStopStrippedTrailingDrop -q
/opt/venv/bin/python -m pytest tests/parser/engine/test_token_id_scanner.py -q
/opt/venv/bin/python -m pytest tests/parser/engine -q
uvx ruff@0.14.0 check \
  tests/parser/engine/test_token_id_scanner.py \
  vllm/parser/engine/token_id_scanner.py
uvx ruff@0.14.0 format --check \
  tests/parser/engine/test_token_id_scanner.py \
  vllm/parser/engine/token_id_scanner.py

Results:

  • Both original regression tests fail on the test-only commit and pass with the fix.
  • Four review-added regressions cover holdback prefixes, multiple trailing DROPs, next-delta resolution, and suffix-only reconstruction.
  • Complete token-scanner file: 36 passed.
  • tests/parser/engine: 3,797 passed.
  • Ruff lint, Ruff format, and git diff --check: passed.
  • This PR and fix(parser): keep GLM tool delimiters inside argument values #639 combined: 3,806 parser-engine tests passed.

Model and serving validation

No live model-generation evaluation was run. This is a deterministic scanner-level regression: delta_text omits the trailing stop text while delta_token_ids retains its ID. Unit and parser-engine tests cover that exact input shape and verify anchor ordering and token-count preservation.

AI assistance and human review

Claude Code assisted with implementation and tests. OpenAI Codex performed an independent line-by-line diff review and validation. The human submitter reviewed and understands every changed line.

logprobz and others added 2 commits September 4, 2026 05:26
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: cd6d341c-d774-47e7-ac1a-5b9d02f8b672

📥 Commits

Reviewing files that changed from the base of the PR and between a50ebee and e2656b9.

📒 Files selected for processing (2)
  • tests/parser/engine/test_token_id_scanner.py
  • vllm/parser/engine/token_id_scanner.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

logprobz and others added 2 commits September 4, 2026 07:27
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
@lukealonso
lukealonso merged commit 5c90fc8 into local-inference-lab:dev/jovian-judgement Sep 4, 2026
1 check 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.

2 participants