DEV Community

Jiwon Yoon
Jiwon Yoon

Posted on

Clear the Lineup — doesNotEqual was always true for single-select survey answers in Formbricks

Summer Bug Smash: Clear the Lineup 🐛🛹

This is a submission for DEV's Summer Bug Smash: Clear the Lineup.

Project Overview

Formbricks is an open-source survey and experience-management platform. Its packages/surveys package ships the client-side survey runner, and inside it, packages/surveys/src/lib/logic.ts is the module that decides, for every question and every "skip logic" / branching rule a survey author configures, whether a given condition (equals, doesNotEqual, contains, isEmpty, and friends) is true for a respondent's answer. This is pure, unglamorous logic — but it's load-bearing: it's what routes respondents through the correct sequence of questions.

I picked up issue #8527 for the "Clear the Lineup" track, which reports that doesNotEqual conditions weren't behaving correctly for single-choice questions.

Bug Fix or Performance Improvement

The doesNotEqual operator is supposed to be the exact logical negation of equals. For most answer shapes it was. But for MultipleChoiceSingle answers, the survey runtime sometimes represents the selected value as a single-element array rather than a bare string (this happens via getLeftOperandValue, e.g. when a choice answer is merged with an "other" option path). To handle that shape, both equals and doesNotEqual had a special-case clause that unwraps a one-element array and compares its only entry to the right-hand value.

equals's fallback:

return (
  (Array.isArray(leftValue) &&
    leftValue.length === 1 &&
    typeof rightValue === "string" &&
    leftValue.includes(rightValue)) ||
  leftValue === rightValue
);
Enter fullscreen mode Exit fullscreen mode

doesNotEqual's fallback (before the fix):

return (
  (Array.isArray(leftValue) &&
    leftValue.length === 1 &&
    typeof rightValue === "string" &&
    !leftValue.includes(rightValue)) ||
  leftValue !== rightValue
);
Enter fullscreen mode Exit fullscreen mode

At a glance this looks like a faithful "negate every sub-expression" mirror of equals. It isn't. The array-includes clause was correctly inverted (!leftValue.includes(rightValue)), but the second disjunct, leftValue !== rightValue, is not the negation of the whole equals expression — it's the negation of only the plain-comparison branch.

Here's the JavaScript gotcha that makes this a real bug and not just a stylistic quibble: when leftValue is an array like ["Option 1"] and rightValue is the string "Option 1" (or a choice id like "opt1"), leftValue !== rightValue is always true — an array and a string are never ===, regardless of contents, so the !== comparison can never be false for this input shape. Because the whole fallback is an OR of two clauses, one clause that's unconditionally true makes the entire expression unconditionally true. The array-includes clause's correct, careful negation gets swallowed by the second clause and never has a chance to matter. In other words: doesNotEqual was a tautology whenever the left value was a single-element array — it returned true no matter what the respondent actually selected.

The practical impact: any survey using skip logic or branching with "does not equal" against a single-choice question would silently misroute every respondent, since the condition was always satisfied. This is the kind of bug that's invisible in casual manual testing — you only notice it if you specifically test the branch that's supposed to be excluded, which is exactly the path people tend not to click through.

Code

PR: https://github.com/jiwonyoon-dev/formbricks/pull/1 (merged)

A transparency note on where this PR lives: the upstream formbricks/formbricks repository currently restricts opening pull requests to existing collaborators, so I couldn't submit this fix upstream directly. The fix is a merged PR on my fork instead — the challenge rules explicitly accept fork merges — and the branch is ready to upstream the moment the maintainers open contributions or pick it up from issue #8527.

The fix replaces the piecewise-negated fallback with a literal De Morgan negation of the entire equals expression, guaranteeing equals and doesNotEqual can never agree on the same inputs:

-        return (
+        return !(
           (Array.isArray(leftValue) &&
             leftValue.length === 1 &&
             typeof rightValue === "string" &&
-            !leftValue.includes(rightValue)) ||
-          leftValue !== rightValue
+            leftValue.includes(rightValue)) ||
+          leftValue === rightValue
         );
Enter fullscreen mode Exit fullscreen mode

And the regression test added to packages/surveys/src/lib/logic.test.ts, in the "Edge Cases" describe block right after the existing "evaluates multiple choice conditions for equals/doesNotEqual" test, reusing the same mockSurvey / mockVariablesData fixtures already in scope:

test("evaluates doesNotEqual for single-selection answers stored as an array (#8527)", () => {
  // Regression test: doesNotEqual must be the exact negation of equals. Previously, when the
  // left value was a single-element array (e.g. a MultipleChoiceSingle answer merged with
  // "other" options) matching a static right value, doesNotEqual incorrectly returned true.
  ...
  // The single selected answer matches the compared option, so doesNotEqual must be false.
  expect(
    evaluateLogic(
      singleSelectSurvey,
      { singleSelectQ: ["Option 1"] },
      mockVariablesData,
      doesNotEqualMatchingCondition,
      "default"
    )
  ).toBe(false);

  // The single selected answer does not match the compared option, so doesNotEqual must be true.
  expect(
    evaluateLogic(
      singleSelectSurvey,
      { singleSelectQ: ["Option 1"] },
      mockVariablesData,
      doesNotEqualNonMatchingCondition,
      "default"
    )
  ).toBe(true);

  // Plain string comparisons must remain unaffected by the fix.
  expect(
    evaluateLogic(mockSurvey, mockData, mockVariablesData, doesNotEqualStringCondition, "default")
  ).toBe(false);
  expect(
    evaluateLogic(mockSurvey, mockData, mockVariablesData, doesNotEqualStringMismatchCondition, "default")
  ).toBe(true);
});
Enter fullscreen mode Exit fullscreen mode

The commit follows the conventional format already used across the repo's last 15 commits on main: fix(surveys): make doesNotEqual the exact negation of equals for single-select answers (#8527).

My Improvements

I'll be upfront: I worked through this with an AI coding agent (Claude) doing the hands-on triage, reproduction, and patch drafting, while I reviewed and directed each step. That's the honest shape of how this fix came together, and I think it's a fair way to work a bug like this — the agent is good at mechanically tracing through boolean algebra and running the test suite over and over; I was there to sanity-check the reasoning and make the call on scope.

Before the fix, we added the regression test above to the unmodified logic.ts and ran it:

npx vitest run src/lib/logic.test.ts
Enter fullscreen mode Exit fullscreen mode
AssertionError: expected true to be false // Object.is equality
- Expected: false
+ Received: true
  at src/lib/logic.test.ts:1436:9
Enter fullscreen mode Exit fullscreen mode

That's the exact tautology from the triage, caught red-handed: doesNotEqual returned true for a matching single-select answer.

After the fix, the same focused run passed 42/42, including all three assertions in the new regression test (single-select array matching → false, single-select array not matching → true, plain-string match/mismatch → unaffected). Running the full package suite, npx vitest run in packages/surveys, gave 26 test files / 651 tests, all passing — no regressions elsewhere. npx eslint src/lib/logic.ts src/lib/logic.test.ts --ext .ts produced zero output (clean).

I also verified the change algebraically, not just empirically: when the array-single-element-and-string condition is false, both the old and new fallback reduce to the same thing (leftValue !== rightValue vs. the negation of leftValue === rightValue — identical). The only input shape whose behavior changes is the tautology case itself, which is exactly the invariant we wanted: equals and doesNotEqual can never both be true, or both be false, for the same inputs.

Scope decisions and honest limitations. I deliberately left the PictureSelection/Date special-case branches inside doesNotEqual untouched, even though they don't have a symmetric branch on the equals side — that asymmetry predates this issue and fixing it isn't part of a minimal, reviewable patch for #8527. I also ran tsc --noEmit for awareness and saw only pre-existing, unrelated module-resolution errors from @formbricks/survey-ui and @formbricks/i18n-utils — artifacts of only packages/surveys being pnpm-installed with --filter rather than a full monorepo install. Neither touched file appears in that error list, but I want to note the gap honestly rather than pretend the build was 100% clean end-to-end; I didn't attempt a full monorepo install to clear it since it's environmental noise, not something the code change caused. No e2e/integration tests exist for this pure-function logic module in this package — unit tests in the 1500+-line logic.test.ts are the established convention here, so that's what I extended.

Lesson worth keeping. The generalizable gotcha is: someArray !== someString is always true in JavaScript, because they're different types and can never be triple-equal. Any fallback shaped like (specialCase) || leftValue !== rightValue is a landmine the moment leftValue can be an array — the "obviously true when types differ" clause silently overrides whatever careful special-casing you did in the first disjunct. When you're negating a multi-clause boolean expression, negate the whole expression (!(...)), don't negate each clause independently and OR them back together — those are not equivalent once more than one clause is in play. That's the one-line rule that would have prevented this bug from ever shipping.

Top comments (0)