ReviewOS

also looking at this

stacks/bun-query-builder

fix(browser): stop serialising an exclusion as an inclusion

#1108
Merged glennmichael123 wants to merge fix/1096-browser-not-in into main
2 files +115 -1
9 days ago

Half of #1096. The issue should stay open — see the last section.

BrowserModelQueryBuilder.buildQueryParams matched in and not in in the same branch:

else if ((where.operator === 'in' || where.operator === 'not in') && Array.isArray(where.value)) {
  params.append(`${where.column}[]`, where.value.join(','))
}

So an exclusion and an inclusion went out byte-identically:

callbeforeafter
whereIn('role', ['banned'])role[]=bannedrole[]=banned
whereNotIn('role', ['banned'])role[]=bannedfilter[role][not in]=banned

The server therefore answered a whereNotIn with precisely the rows the caller meant to remove. The idiom is overwhelmingly "exclude banned / deleted / private", so the failure mode is showing the records you meant to hide — with no error, and a request that looks correct in a network panel.

No new wire format

Matching in alone lets not in fall through to the filter[column][operator] branch, which is what the sibling BrowserQueryBuilder has always emitted for not in. So this makes the two builders agree rather than inventing a format — there is a test asserting they now serialise the same query the same way.

Tests

test/browser.not-in-serialisation.test.ts reads the outgoing URL by capturing fetch, rather than asserting on a mock server's interpretation of it — the defect was entirely in what went on the wire, so anything downstream of that is the wrong place to look.

3 of 4 fail without the src change; the fourth pins the inclusion form as unchanged.

What this does not fix

where.boolean is read by neither serialiser, so orWhere still serialises as an AND — the other half of #1096, and also a silent wrong answer.

I left it deliberately. Representing OR needs a wire format this file has no precedent for, and choosing one unilaterally risks emitting params no backend parses, which is not obviously better than today. That is a call about the API contract with whatever server consumes these params.

Both call sites now carry a comment recording the gap so it is not rediscovered as a surprise. Options, when you want it: an or group key (filter[or][col][op]), or making orWhere throw on this builder so a wrong answer becomes a loud one.

Worth noting separately: the file spells two-word operators inconsistently — is not becomes filter[col][is_not], while not in becomes filter[col][not in]. I matched the sibling's existing output rather than normalising, since that key is the wire contract.

Checks

  • typecheck and pickier clean
  • bun test — 2 failures, both pre-existing and reproducing on clean main (stale bin/qbx CLI > version, and the migration locking (#1067) flake that passes in isolation)

🤖 Generated with Claude Code

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.