lablup / lablup/mlxcel

fix(bench): speculative_bench panics instead of erroring when --prompt fails to tokenize

Open Beginner friendly
#1,667 0 comments 0 reactions 0 assignees View on GitHub
priority:low status:ready type:bug
Dominant language
Rust
Stars
467
Forks
54
Avg merge
4h 25m
Merged PRs (30d)
310

Description

## Summary

`speculative_bench` panics instead of returning an error when the user-supplied `--prompt` fails to tokenize, while the sibling bench binary handles the same situation with a proper diagnostic. Make it return a readable error.

## Background

`encode_prompt` ends in `.expect("tokenizer.encode must succeed on a valid utf-8 prompt")`, and the prompt it encodes comes straight from the `--prompt` CLI argument. A prompt the tokenizer rejects therefore aborts with a panic backtrace rather than a diagnostic. The reference implementation two files over, `bench_decode.rs`, already does this correctly. This is the same class of defect as the previously fixed `--target` unwrap panic in this binary (#1242).

## Proposed Solution

Make `encode_prompt` return `Result`, mapping the tokenizer error into a message that names `--prompt`, mirroring `tokenize_prompt` in `bench_decode.rs`. Both call sites already sit inside `-> Result<...>` functions, so only a `?` is needed at each.

## Implementation Notes

- `src/bin/speculative_bench.rs:343` defines `encode_prompt(...) -> Vec` ending in `.expect(...)`; the `--prompt` field is at line 150; call sites are at lines 369 and 536.
- `src/bin/bench_decode.rs:152` defines `tokenize_prompt(...) -> Result>` using `anyhow::anyhow!("tokenization failed: {err}")`, the pattern to follow.

## Acceptance Criteria

- [ ] A prompt the tokenizer rejects produces a readable error naming `--prompt`, not a panic backtrace
- [ ] Normal prompts behave exactly as before

---

## Original Suggestion

### Title: fix(bench): speculative_bench panics when --prompt fails to tokenize

`speculative_bench` panics instead of returning an error when the user-supplied `--prompt` fails to tokenize, while the sibling bench binary handles the same situation with a proper diagnostic.

## Evidence

- `src/bin/speculative_bench.rs:343-351` — `encode_prompt` ends in `.expect("tokenizer.encode must succeed on a valid utf-8 prompt")`; the prompt comes straight from the `--prompt` CLI arg (field at line 150)
- Reference implementation two files over: `src/bin/bench_decode.rs:152-159` uses `tokenize_prompt(...) -> Result>` with `anyhow!("tokenization failed: {err}")`
- Both call sites (lines 369 and 536) are already inside `-> Result<...>` functions, so only a `?` is needed

This is the same class as the previously fixed `--target` unwrap panic in this binary (#1242).

## Suggested fix

Make `encode_prompt` return `Result`, mapping the tokenizer error into a message that names `--prompt`, mirroring `bench_decode.rs`.

## Acceptance criteria

- [ ] A prompt the tokenizer rejects produces a readable error naming `--prompt`, not a panic backtrace
- [ ] Normal prompts behave exactly as before

Contributor guide

Open the contributing guide

Research direction

Start with src/bin/speculative_bench.rs:343 and compare encode_prompt with tokenize_prompt in src/bin/bench_decode.rs:152. Check both call sites at lines 369 and 536, then exercise a rejected and a normal --prompt. Done means the rejected prompt reports a readable error naming --prompt without a panic, while normal prompts behave as before.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.