Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/commands/benchmark.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ With sampling enabled, `--samples <n>` runs each test n times and records the pe
nanotune benchmark --temperature 0.8 --samples 5
```

`--temperature`, `--seed`, and `--samples` are rejected outright if they can't be parsed, rather than falling back to the default. A typo like `--samples 5x` fails with a message naming the flag, so a run never reports a score under settings you didn't ask for.
Every numeric flag — the sampling ones above and `--threads`, `--gpu-layers`, `--ctx-size`, `--batch-size`, `--max-tokens` and `--timeout` — is rejected outright if it can't be parsed, rather than falling back to the default or being truncated. A typo like `--samples 5x` or `--ctx-size 4096x` fails immediately with a message naming the flag, before the model is resolved and `llama-server` is started, so a run never reports a score under settings you didn't ask for.

## Examples

Expand Down
2 changes: 2 additions & 0 deletions docs/commands/chat.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ nanotune chat

Hardware flags are the same as [`nanotune benchmark`](benchmark.md), and a `--preset` overrides the individual flags.

Every numeric flag is rejected outright if it can't be parsed, rather than falling back to the default or being truncated: `--ctx-size 4096x` fails with a message naming the flag instead of quietly starting the server at 4096.

## Per-Turn Stats

Each assistant reply is followed by a dim stat line:
Expand Down
90 changes: 25 additions & 65 deletions src/commands/benchmark.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
buildMessages,
formatConversationForJudge,
getTestDisplayPrompt,
resolveBenchmarkFlags,
resolveSamplingOptions,
summarizeSamples,
} from '../lib/benchmark-utils.js';
Expand All @@ -37,9 +38,7 @@ import {
chatCompletion,
checkLlamaCppInstalled,
exportModel,
type GenerateOptions,
installLlamaCpp,
type ServerOptions,
startLlamaServer,
stopLlamaServer,
} from '../lib/llama-cpp.js';
Expand All @@ -50,8 +49,6 @@ import {
} from '../lib/model-cache.js';
import {assertSupportedPlatform} from '../lib/platform.js';
import {
BENCHMARK_PRESETS,
type BenchmarkPreset,
type BenchmarkResult,
type BenchmarkTest,
type BenchmarkTestResult,
Expand Down Expand Up @@ -329,6 +326,30 @@ export function BenchmarkCommand({options}: Props) {
return;
}

// The llama-server flags get the same treatment, for the same
// reason: unchecked, a typo'd --ctx-size is either truncated
// ("4096x" quietly becomes 4096) or reaches llama-server as the
// literal argument "NaN" — minutes from now, after the download.
const flags = resolveBenchmarkFlags(
{
preset: options.preset,
threads: options.threads,
gpuLayers: options.gpuLayers,
ctxSize: options.ctxSize,
batchSize: options.batchSize,
cpuOnly: options.cpuOnly,
maxTokens: options.maxTokens,
timeout: options.timeout,
},
sampling,
);
if (flags.errors.length > 0) {
setError(flags.errors[0]);
setStatus('error');
return;
}
const {serverOptions, generateOptions, timeout} = flags;

// Find model
let modelPath: string | null;
if (options.base) {
Expand Down Expand Up @@ -472,64 +493,6 @@ export function BenchmarkCommand({options}: Props) {
}
}

let serverOptions: ServerOptions;
let generateOptions: GenerateOptions;

if (options.preset) {
// Validate preset
const validPresets: BenchmarkPreset[] = [
'low',
'medium',
'high',
'ultra',
];
if (!validPresets.includes(options.preset as BenchmarkPreset)) {
setError(
`Invalid preset: ${options.preset}. Valid presets: ${validPresets.join(', ')}`,
);
setStatus('error');
return;
}

// Apply preset configuration
const preset = BENCHMARK_PRESETS[options.preset as BenchmarkPreset];
serverOptions = {
threads: preset.threads,
gpuLayers: preset.gpuLayers,
ctxSize: preset.ctxSize,
batchSize: preset.batchSize,
cpuOnly: preset.gpuLayers === 0,
};
generateOptions = {
maxTokens: preset.maxTokens,
temperature: sampling.temperature,
seed: sampling.seed,
};
} else {
serverOptions = {
threads: options.threads
? Number.parseInt(options.threads, 10)
: undefined,
gpuLayers: options.gpuLayers
? Number.parseInt(options.gpuLayers, 10)
: undefined,
ctxSize: options.ctxSize
? Number.parseInt(options.ctxSize, 10)
: 4096,
batchSize: options.batchSize
? Number.parseInt(options.batchSize, 10)
: 2048,
cpuOnly: options.cpuOnly,
};
generateOptions = {
maxTokens: options.maxTokens
? Number.parseInt(options.maxTokens, 10)
: 50,
temperature: sampling.temperature,
seed: sampling.seed,
};
}

// Check if any tests use llm-judge and load judge config if needed
const hasJudgeTests = tests.some(t => t.match === 'llm-judge');
let judgeConfig: JudgeProviderConfig | null = null;
Expand All @@ -547,9 +510,6 @@ export function BenchmarkCommand({options}: Props) {

// Run benchmarks
setStatus('running');
const timeout = options.timeout
? Number.parseInt(options.timeout, 10)
: 30000;

const failures: BenchmarkResult['failures'] = [];
const allResults: BenchmarkTestResult[] = [];
Expand Down
19 changes: 15 additions & 4 deletions src/commands/chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export function ChatCommand({options}: Props) {

// Memoize so the values are stable across renders — otherwise the startup
// effect would tear down and restart the llama-server on every state change.
const serverOptions = useMemo(
const server = useMemo(
() =>
buildServerOptions({
preset,
Expand All @@ -156,10 +156,11 @@ export function ChatCommand({options}: Props) {
}),
[preset, threads, gpuLayers, ctxSize, batchSize, cpuOnly],
);
const generateOptions = useMemo(
const generate = useMemo(
() => buildGenerateOptions({preset, maxTokens, temperature, topP, seed}),
[preset, maxTokens, temperature, topP, seed],
);
const generateOptions = generate.options;

const appendTurn = useCallback((turn: Omit<DisplayTurn, 'id'>) => {
setDisplayTurns(prev => [...prev, {id: turnIdRef.current++, ...turn}]);
Expand All @@ -171,6 +172,16 @@ export function ChatCommand({options}: Props) {

const startup = async () => {
try {
// Reject a mistyped numeric flag before anything else happens —
// unchecked, it reaches llama-server as the literal argument "NaN"
// (or as a null field in the completion body) instead of failing here.
const flagErrors = [...server.errors, ...generate.errors];
if (flagErrors.length > 0) {
setError(flagErrors[0]);
setStatus('error');
return;
}

if (!configExists()) {
setError('Not a Nanotune project. Run `nanotune init` first.');
setStatus('error');
Expand Down Expand Up @@ -207,7 +218,7 @@ export function ChatCommand({options}: Props) {
}
setModelLabel(modelPath.split('/').pop() ?? modelPath);

const handle = await startLlamaServer(modelPath, serverOptions);
const handle = await startLlamaServer(modelPath, server.options);
if (cancelled) {
await stopLlamaServer(handle);
return;
Expand All @@ -232,7 +243,7 @@ export function ChatCommand({options}: Props) {
void stopLlamaServer(handle);
}
};
}, [modelArg, systemArg, serverOptions]);
}, [modelArg, systemArg, server, generate]);

// Backstop: if the process exits unexpectedly (SIGINT etc.), make sure
// the server child is killed. The useEffect cleanup handles graceful exit.
Expand Down
37 changes: 36 additions & 1 deletion src/commands/commands.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Text } from "ink";
import { render } from "ink-testing-library";
import { useKeyInput } from "../components/index.js";
import { loadTrainingData } from "../lib/data.js";
import { streamPreview } from "./chat.js";
import { ChatCommand, streamPreview } from "./chat.js";
import { DataExportCommand } from "./data/export.js";
import { DataImportCommand } from "./data/import.js";
import { DataListCommand } from "./data/list.js";
Expand Down Expand Up @@ -386,3 +386,38 @@ test.serial("DataExportCommand with --eval exports the validation set", async (t
teardown();
}
});

// ── numeric flags are rejected before any server work ─────────────────

test.serial("ChatCommand rejects a non-numeric flag before touching the model", async (t) => {
try {
// No project and no model here: getting the flag error rather than
// "Not a Nanotune project" proves the check runs before anything else,
// so "NaN" never reaches llama-server's argv.
setupEmptyDir();
const output = await renderCommand(
<ChatCommand options={{ gpuLayers: "abc" }} />,
"--gpu-layers",
);

t.true(output.includes("Invalid value for --gpu-layers"));
t.false(output.includes("Not a Nanotune project"));
} finally {
teardown();
}
});

test.serial("ChatCommand rejects a partially parseable flag", async (t) => {
try {
setupEmptyDir();
const output = await renderCommand(
<ChatCommand options={{ ctxSize: "4096x" }} />,
"--ctx-size",
);

// Number.parseInt would have accepted this as 4096.
t.true(output.includes("Invalid value for --ctx-size"));
} finally {
teardown();
}
});
Loading
Loading