lablup / lablup/mlxcel

fix(cli): reject whitespace-only prompts in embed to match rerank

Open Beginner friendly
#1,664 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

`mlxcel embed -p " "` is accepted while `mlxcel rerank -d " "` is rejected. The two commands' otherwise line-for-line identical input validation diverges on whitespace-only strings. Bring embed in line with rerank.

## Background

Both `run_embed` and `run_rerank` guard against empty inputs early, with matching "nothing to embed/rerank" and "image file does not exist" wording. The empty check differs: embed tests `String::is_empty`, so a whitespace-only prompt passes, while rerank tests `d.trim().is_empty()`, so it is rejected.

## Proposed Solution

Adopt the `trim().is_empty()` check in `embed.rs` so whitespace-only prompts fail with the same style of error as rerank. Add one unit test following the `src/commands/*_tests.rs` convention. Also rename embed's misleading binding: it is named `empty` but holds an index, producing `bail!("prompt {empty} is empty")`.

## Implementation Notes

- `src/commands/embed.rs:135` uses `args.prompts.iter().position(String::is_empty)` and binds the index as `empty`.
- `src/commands/rerank.rs:108` uses `args.documents.iter().position(|d| d.trim().is_empty())` and binds it as `index`.
- The `src/commands/` directory already has a `_tests.rs` convention to follow.

## Acceptance Criteria

- [ ] `mlxcel embed` rejects whitespace-only prompts with the same style of error as `rerank`
- [ ] A test pins the behavior

---

## Original Suggestion

### Title: fix(cli): embed accepts whitespace-only prompts while rerank rejects them

`mlxcel embed -p " "` is accepted while `mlxcel rerank -d " "` is rejected: the two commands' otherwise line-for-line identical input validation differs on whitespace-only strings.

## Evidence

- `src/commands/embed.rs:135-137` — checks `.position(String::is_empty)` (whitespace passes)
- `src/commands/rerank.rs:108-110` — checks `.position(|d| d.trim().is_empty())` (whitespace rejected)
- The surrounding `run_*` prologues are otherwise identical (same "nothing to embed/rerank" and "image file does not exist" wording)

Minor bonus in the same lines: embed's binding is named `empty` but holds an index (`bail!("prompt {empty} is empty")`), which reads oddly.

## Suggested fix

Adopt the `trim().is_empty()` check in `embed.rs` (and rename the index binding), plus one unit test; `src/commands/` already has a `_tests.rs` convention to follow.

## Acceptance criteria

- [ ] `mlxcel embed` rejects whitespace-only prompts with the same style of error as `rerank`
- [ ] A test pins the behavior

Contributor guide

Open the contributing guide

Research direction

Start with the validation at src/commands/embed.rs:135 and compare it with src/commands/rerank.rs:108. Follow the src/commands/*_tests.rs convention to add a unit test for whitespace-only prompts, and verify that mlxcel embed rejects them with the expected error style and that the index binding is clear.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.