Skip to content

[Bug] An empty string in acceptable makes a test pass unconditionally under contains, startsWith and partial #164

Description

@addyCooks

Description

checkPass (src/lib/benchmark-match.ts:26) iterates acceptable and, for
three of the five match modes, compares against the empty string using
JavaScript operations that are vacuously true for it:

checkPass([''], 'anything at all', 'contains')    -> passed: true
checkPass([''], 'anything at all', 'startsWith')  -> passed: true
checkPass([''], 'anything at all', 'partial')     -> passed: true

''.includes('') and String.prototype.startsWith('') are true for every
possible input, so a single empty string anywhere in a test's acceptable
array makes that test pass regardless of what the model produced. The test goes
permanently green and green tests are the ones nobody re-reads.

Root Cause

The loop at benchmark-match.ts:42 does no validity check on expected before
dispatching into the mode switch:

for (const expected of acceptable) {
	const expectedProcessed = processText(expected);
	const expectedNormalized = normalizeText(expectedProcessed);

	switch (mode) {
		case 'contains': {
			if (actualNormalized.includes(expectedNormalized)) {   // '' -> always true

Per mode, with expected === '':

Mode Line Result Why
contains :57 always passes includes('') is always true
startsWith :67 always passes startsWith('') is always true
partial :81 always passes actual.startsWith('') is always true
exact :50 safe requires actual to also be empty
semantic :96 mostly safe the '' case needs actual empty, or actual starting with : (the ${expected}: delimiter branch at :105) worth tightening, but not vacuous

Note semantic is the default mode, and the three affected modes are all
opt-in, which limits blast radius. partial in particular already carries a
comment warning that it "can inflate pass rates" this is a second, unflagged
way it does so.

Nothing upstream catches it either. acceptable is optional on BenchmarkTest
(src/types/index.ts:132) and the dataset validation loop at
benchmark.tsx:486 checks only that each test has a prompt or messages it never inspects acceptable. So ["ls", ""] loads without complaint.

(A missing acceptable is handled correctly: test.acceptable || [] at
benchmark.tsx:642 yields an empty array, the loop body never runs, and the
test fails. The bug is specifically an empty string inside the array.)

Environment

  • macOS version: N/A platform-independent (pure string logic)
  • Apple Silicon: N/A
  • Node version: v22.22.3
  • Python version: N/A
  • Nanotune version: 1.7.0 confirmed on main @ 946c488

Steps to Reproduce

  1. Add a test to .nanotune/benchmarks/tests.json with an empty string in
    acceptable and an opt-in match mode:
{
  "id": 99,
  "prompt": "list all files",
  "acceptable": ["ls -la", ""],
  "category": "basic",
  "match": "contains"
}
  1. Run nanotune benchmark.

Test 99 passes no matter what the model returns including empty output, a
refusal, or an unrelated answer.

Realistic paths into this state: a stripped-out placeholder, a hand-edited
tests.json, or a generator/script that emits "" for an answer it could not
fill in.

Expected Behavior

An empty string in acceptable is not treated as a matchable answer. It is
either skipped during matching, or better surfaced at dataset load time as
a dataset error, e.g. Test #99: "acceptable" contains an empty string.

Actual Behavior

The test passes unconditionally under contains, startsWith and partial,
silently inflating the pass rate that the tool exists to report. Nothing in the
output distinguishes it from a genuine match matchedAnswer is returned as
"" and matchType as the mode name.

Logs/Screenshots

checkPass([''], 'anything at all', 'contains')    -> {"passed":true,"matchedAnswer":"","matchType":"contains"}
checkPass([''], 'anything at all', 'startsWith')  -> {"passed":true,"matchedAnswer":"","matchType":"startsWith"}
checkPass([''], 'anything at all', 'partial')     -> {"passed":true,"matchedAnswer":"","matchType":"partial"}
checkPass([''], 'anything at all', 'exact')       -> {"passed":false,"matchedAnswer":null,"matchType":null}
checkPass([''], 'anything at all', 'semantic')    -> {"passed":false,"matchedAnswer":null,"matchType":null}

Additional Context

Suggested fix

  1. Skip empty expected values in the match loop if (expectedNormalized === '') continue;
    at the top of the for body in benchmark-match.ts:42. One line, and it
    covers all five modes uniformly.
  2. Better in addition: reject them at load time so the user learns the dataset
    is wrong rather than quietly getting a free pass. This fits naturally into
    the dataset validation loop at benchmark.tsx:486, or into the Zod dataset
    schema if that lands first.
  3. While in benchmark-match.ts, consider the semantic '' edge in the table
    above an actual response beginning with : matches an empty expected via
    the delimiter branch at :105. Same one-line guard fixes it.

Not a duplicate of #141 (Semantic match mode \n check is unreachable due
to prior normalization
). #141 is about a branch in the semantic case that can
never fire; this is about branches in contains/startsWith/partial that
always fire. Same file, opposite failure, and both are addressed by different
lines though they batch well into one PR.

  • I have searched existing issues to ensure this is not a duplicate
  • I can reproduce this issue consistently
  • This issue occurs with the latest version of nanotune

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions