lablup / lablup/mlxcel

refactor(cli): dedupe the --estimate-memory preflight block shared by serve and generate

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

Description

## Summary

The `--estimate-memory` preflight warn/abort block is duplicated between `serve` and `generate`, differing only in the trailing remediation hint. Move it into a helper in `memory_estimate.rs` that takes the hint as a parameter.

## Background

Both commands print the same WARNING string and run the same fits/force logic; only the remediation hint at the end differs (`--ctx-size` / `--max-batch-size` for serve, `--max-tokens` for generate). The shared home already exists and both callers already import from it.

## Proposed Solution

Add a helper in `src/execution/memory_estimate.rs` that takes the remediation hint as a parameter, and call it from both commands. Same shape as the already-merged `format_bytes` dedup (#1626).

## Implementation Notes

- `src/commands/serve.rs:342-359` vs `src/commands/generate.rs:348-365` (confirmed): identical `if !estimate.fits { if force_memory { eprintln!("WARNING: --estimate-memory preflight says this load is over budget by {}. Continuing because --force was set." ...) } }` block. The only difference is `args.force_memory` (serve) vs `args.generation.force_memory` (generate) and the trailing hint.
- Shared home already imported by both: `src/execution/memory_estimate.rs` (`:485`, `:1396`, `:1412`).

## Acceptance Criteria

- [ ] One implementation; both commands print exactly what they printed before.

---

## Original Suggestion

### Title: refactor(cli): dedupe the --estimate-memory preflight block shared by serve and generate

The `--estimate-memory` preflight warn/abort block is duplicated byte-for-byte between `serve` and `generate`, differing only in the trailing remediation hint.

## Evidence

- `src/commands/serve.rs:342-359` vs `src/commands/generate.rs:348-365` — identical including the `"WARNING: --estimate-memory preflight says this load is over budget by {} ... Continuing because --force was set."` string; only the hint differs (`--ctx-size`/`--max-batch-size` vs `--max-tokens`)
- Shared home already exists: `src/execution/memory_estimate.rs` (see `:485`, `:1396`, `:1412`), which both callers already import from

## Suggested fix

Move the block into a helper in `memory_estimate.rs` taking the remediation hint as a parameter — the same shape as the already-merged `format_bytes` dedup (#1626).

## Acceptance criteria

- [ ] One implementation; both commands print exactly what they printed before

Contributor guide

Open the contributing guide

Research direction

Start with the duplicated preflight blocks in src/commands/serve.rs:342-359 and src/commands/generate.rs:348-365, then inspect src/execution/memory_estimate.rs as the shared home. Extract one helper that accepts the remediation hint and call it from both commands. Done means one implementation remains and both commands print exactly as before.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.