Skip to content

fix(failure): read a trailing context limit after a multi-byte character - #141

Open
shaurya703 wants to merge 1 commit into
tinyhumansai:mainfrom
shaurya703:fix/context-limit-multibyte-before-number
Open

shaurya703 wants to merge 1 commit into
tinyhumansai:mainfrom
shaurya703:fix/context-limit-multibyte-before-number

Conversation

@shaurya703

Copy link
Copy Markdown
Contributor

Summary

parse_context_limit_from_error panics when a multi-byte character comes right before the number in the "<N> maximum context length" (Mistral) shape.

Bug. Reproduction on current main:

parse_context_limit_from_error("too large for model with\u{a0}32768 maximum context length");
// panicked at crates/tinyinference-llm/src/failure/context_limit.rs:138:25
// (byte index is not a char boundary)

The same happens when a CJK character precedes the number ("…模型32768 maximum context length"). This runs on provider overflow error bodies through model::discover::record_overflow_error, so a provider error with that wording crashes the caller instead of recording the window.

Root cause. trailing_number finds the last non-digit character with rfind and starts the digit run at index + 1. That is one byte past the start of the character, so a no-break space (2 bytes) or CJK character (3 bytes) leaves the slice start inside it.

Fix. Walk char_indices() from the end and start at index + character.len_utf8().

Related issue

None. Found while reading the overflow-error parsers.

API or behavior changes

None. Messages that used to panic now return the stated limit.

Validation

  • cargo fmt --all -- --check: clean
  • cargo clippy -p tinyinference-llm --all-targets --all-features -- -D warnings: clean
  • cargo test -p tinyinference-llm --all-features: all pass (873 lib tests). The new test panics without the fix.

Tests

failure::context_limit::tests::parses_trailing_limit_after_a_multibyte_character covers a no-break space and a CJK character before the number, and expects Some(32_768) for both.

Documentation

Not needed.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

parse_context_limit_from_error falls back to the number right before
"maximum context length" (the Mistral shape). trailing_number found the
character before the digits with rfind and started the digits at
index + 1, which lands inside that character when it is multi-byte. A
message such as "too large for model with\u{a0}32768 maximum context
length" or one with a CJK character before the number panicked on the
slice instead of returning the limit.

Step past the whole preceding character with its UTF-8 length.
@tinysweeper

tinysweeper Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 9bdca33c07d5b0c4b1d33795054dde5ac16a52c7

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 2 of 2 files.
Pull request purpose (title): fix(failure): read a…

Diagnostics and supporting details

Full summary

Pull request purpose (title): fix(failure): read a trailing context limit after a multi-byte character. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

`trailing_number` in the context-limit parser replaces its `rfind`-based byte offset with a reverse `char_indices()` scan that advances by the matched character's `len_utf8()`, preserving the `map_or(0, ...)` fallback for all-digit/comma input and leaving the downstream `trim_start_matches(',')`/parse handling unchanged. Messages that previously panicked on a non-char-boundary slice now return the stated limit. A new unit test in the sibling tests file exercises a no-break space and a CJK character before the number, both expected to parse as `Some(32_768)`.

Features

  • Modified — Panic-free trailing context-limit parsing after multi-byte characters: Provider overflow error bodies with a no-break space or CJK character before the limit number no longer panic the caller (via `model::discover::record_overflow_error`) and instead yield the parsed window; behavior is unchanged for other inputs, including all-digit/all-comma strings. (crates/tinyinference-llm/src/failure/context_limit.rs fn first_number(text: &str, window: usize) -> Option<u64> {, crates/tinyinference-llm/src/failure/context_limit.rs)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: It noted the tests feed ASCII-lowercased text, while the code still handles arbitrary multi-byte input.
  • Lane summary: The change fixes a real byte-boundary panic in `trailing_number`: `rfind` returns a byte index of the matched non-digit character's start, so `index + 1` could land inside a multi-byte character and panic on slicing. Replacing it with `char_indices().rev().find(...)` and adding `character.len_utf8()` is correct, the `map_or(0, ...)` case (all digits/commas) is preserved, and the two new tests exercise the no-break-space and CJK cases. Note both tests feed `lower` (ASCII-lowercased) text, so the multibyte character preceding the digits is the no-break space / CJK run — the code itself still handles arbitrary multibyte correctly. _The code index is behind this pull request (indexed at `b3a7c5fb6ca2`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The security lane found that `begin` always lands on a char boundary at or after the last non-digit, keeping the slice valid and the downstream trim/parse handling consistent, and introduced no security-relevant sink or untrusted-input issue.
  • Lane summary: The change rewrites `trailing_number` to step past a whole multi-byte character instead of one byte, fixing a real panic (a no-break space or CJK character before the limit would slice inside a char boundary). The reverse `char_indices` scan, `map_or(0, ...)` fallback, and subsequent `trim_start_matches(',')`/`parse_grouped` handling are consistent: `begin` always lands on a char boundary at or after the last non-digit, so the slice is valid. The two new tests exercise exactly the fixed inputs and are unit tests in the sibling `*_tests.rs` file. No security-relevant sink, untrusted-input path, or repository-rule violation is introduced. _The code index is behind this pull request (indexed at `b3a7c5fb6ca2`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The tests lane reported nothing reportable, noting the new test exercises inputs that panicked before the change and parse correctly now.
  • Lane summary: The change fixes a real panic: `rfind` returns a byte index of the non-digit character, so `index + 1` could land inside a multi-byte character and `trimmed[begin..]` would panic. The new `char_indices` walk adds `len_utf8()`, and the new test exercises both a no-break space and a CJK character — inputs that panicked before and parse correctly now. I found nothing reportable. _The code index is behind this pull request (indexed at `b3a7c5fb6ca2`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane reported no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `b3a7c5fb6ca2`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The description lane found the description matches the diff, explaining the panic, the `char_indices()`/`len_utf8()` fix, and the added test, with no unmentioned functional changes.
  • Lane summary: The description matches the diff: it explains the byte-index panic in `trailing_number`, the fix using `char_indices()` and `len_utf8()`, and the added test covering a no-break space and a CJK character, with no unmentioned functional changes. One wording matter: the test is named `parses_trailing_limit_after_a_multibyte_character`, which is accurate for a trailing number even though the character is leading it — not a defect. There are no reportable code defects in the changed lines. _The code index is behind this pull request (indexed at `b3a7c5fb6ca2`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000096
  • Tokens: 53638 input · 1621 output · 8704 cached · 150 embedding
  • Updated: 2026-10-26T06:25:56Z
Head State Pass summary
9bdca33c07d5 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-26T06:25:56Z)

tinysweeper 0.1.2

@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4f2c1293-d5ea-4af2-a8cf-c26fc7426047
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0001

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant