A count check that lets ambiguous lookups slip through

A bulk import had a validation step that looked sensible. A spreadsheet cell held a comma-separated list of names. The code looked each name up against a reference table, collected the matching ids, and checked that the number of ids equalled the number of names. If the counts differed, the row was rejected as containing an unknown name.

It passed review and it worked for a long time. It was also wrong in a way that only shows once your lookup gets more generous.

The bug

Here is the shape of it, rewritten as a minimal example:

var names = cell.Split(',').Select(n => n.Trim()).ToList();

var ids = reference
    .Where(r => names.Contains(r.Name, StringComparer.OrdinalIgnoreCase))
    .Select(r => r.Id)
    .ToList();

if (ids.Count != names.Count)
    return Reject("Unrecognised name");

return Accept(ids);

The check asks "did I find as many rows as I was given names?" What we actually care about is "did every name find exactly one row?" Those are different questions, and the first only answers the second when nothing is ambiguous.

Take the input "A, B". Suppose A matches two rows and B matches none. The query returns two ids. Two names, two ids, the counts agree, and the row is accepted. The import then stores one of the ambiguous ids for A and silently drops B. Nothing throws, and nothing is logged. The data is simply wrong.

A surplus on one name pays for a deficit on another. Total-versus-total comparisons can't tell them apart.

Why it stayed hidden

While each name could match at most one row, a duplicate was impossible, so the count check was equivalent to the correct check. The bug was latent rather than absent.

What exposed it was a change to the matching itself. We widened the lookup so a name could match on more than one column, and across more than one group of rows in the reference data. That is a reasonable feature change, but it makes collisions much more likely, and it quietly invalidated an assumption that the validation had been leaning on without anyone writing it down.

The general lesson: when you loosen how a lookup matches, go and find every check that reasons about the result's size. They were written against the old behaviour.

The fix

Resolve each name separately, and reject the row unless every name matches exactly one row:

var resolved = new List<int>();

foreach (var name in names)
{
    var matches = reference
        .Where(r => string.Equals(r.Name, name, StringComparison.OrdinalIgnoreCase))
        .ToList();

    if (matches.Count == 0)
        return Reject($"Unrecognised name: {name}");

    if (matches.Count > 1)
        return Reject($"Ambiguous name: {name}");

    resolved.Add(matches[0].Id);
}

return Accept(resolved);

There are three improvements here, and only the first is the actual bug fix.

• Per-input resolution. Each name has to pass on its own, so one input can no longer cover for another.
• Distinct failure modes. "Not found" and "found more than one" are different problems with different remedies. The person fixing the spreadsheet needs to know which one they have.
• Errors that name the input. The original message said a name was unrecognised without saying which. When the check is per-name, the message can be too.

Testing it

The tests that matter are the ones where the totals agree but the individual results don't. A suite that only covers "all names match" and "one name is missing" will pass against both the broken and the fixed code. These are the cases that distinguish them:

• Two names, where the first matches two rows and the second matches none. Expect rejection.
• Two names, where both match exactly one row each. Expect acceptance with two ids in input order.
• One name matching two rows. Expect an ambiguity error, not a silent pick of the first.
• The same name twice in one cell. Decide deliberately whether that is an error, and write the test for whichever you choose.

Before relying on the fix, confirm that the first case fails against the old implementation. A regression test that never failed proves nothing.

The wider pattern

This isn't specific to spreadsheets. Anywhere you map a list of human-supplied keys onto records, you can write the same bug: tag names, usernames, product codes, permission names. The temptation is to run one query for the whole list and compare sizes, because it is shorter and does one round trip. The size comparison looks like validation, but it only validates the total.

If the one-query form matters for performance, you can keep it. Group the results by the key you matched on, then check that each requested key has exactly one group entry with one row. The point is not the number of queries; it is that the check runs per key.

Takeaways

• A count comparison validates totals, not individuals. Excess on one input can mask a shortfall on another.
• Require exactly one match per input, and report not-found and ambiguous as separate errors.
• When you widen how a lookup matches, audit every check that depends on the size of its result.
• Write tests where the totals agree but the individual outcomes don't, and watch them fail on the old code first.
• Silent data corruption is worse than a rejected row. When in doubt, reject and say why.