[oss-candidate] Fix wrong helper selection when a keyword is followed by ( - fixes #701

[askalf]

Summary

• src/types.js compiles one regex per helper keyword (values, in, select, as, returning, \(, update, insert). Each was ((?:^|[\s(])KW(?:$|[\s(]))(?![\s\S]*\1) — a capture group plus a backreference \1 in the trailing "no later occurrence" lookahead.
• A backreference matches the literal captured text, delimiters included. " in(" and " in " are different strings, so a later " in " did not cancel an earlier " in(". The stale earlier hit survived, and Builder.build (which picks the highest index) then ranked the "(select " hit above it.
• Result: the dynamic value array was handed to the select helper → escapeIdentifiers → escape() → str.replace on a number, throwing TypeError: str.replace is not a function — upstream issue WHERE IN throws TypeError: str.replace is not a function porsager/postgres#701, open since 2023-10-17.
• Fix is 4 lines in src/types.js: hoist the keyword pattern into a const and repeat it inside the lookahead instead of backreferencing the captured text, so any later occurrence counts regardless of surrounding delimiters.
• Adds 13 regression tests beside the existing Last keyword used even with duplicate keywords test — 9 discriminate (fail on base), 3 are declared controls (pass on both arms), and 1 is the pre-existing test re-run as a baseline.

Every test below was executed end-to-end against a real PostgreSQL backend, on both arms, at the current head 280ff1d. Verbatim:

######## HEAD — fix/builder-keyword-last-occurrence @ 280ff1d (fix applied) ########
PASS  0  Last keyword used even with duplicate keywords (pre-existing)
PASS  1  Last keyword used even when an earlier keyword is followed by (
PASS  2  Last keyword used even when it ends the string
PASS  3  Last keyword used when nested keywords are all spaced
PASS  4  Single keyword followed by ( is still used
PASS  5  Last keyword used for keywords other than in
PASS  6  Last keyword used when the spaced keyword comes first
PASS  7  Last keyword used with three occurrences and three delimiters
PASS  8  Last keyword used case insensitively
PASS  V1 Last keyword used when the earlier keyword is in a nested fragment
PASS  V3 Repeated keyword with identical ( delimiters (control)
PASS  V4 Last keyword used when an earlier keyword is newline delimited
PASS  V2 Last keyword used with an empty array
🎉 all passed

######## BASE — origin/master @ 411429e (fix NOT applied) ########
PASS  0  Last keyword used even with duplicate keywords (pre-existing)
FAIL  1  Last keyword used even when an earlier keyword is followed by (  :: threw str.replace is not a function
FAIL  2  Last keyword used even when it ends the string  :: threw str.replace is not a function
PASS  3  Last keyword used when nested keywords are all spaced
PASS  4  Single keyword followed by ( is still used
FAIL  5  Last keyword used for keywords other than in  :: expected 3 != got undefined
FAIL  6  Last keyword used when the spaced keyword comes first  :: threw str.replace is not a function
FAIL  7  Last keyword used with three occurrences and three delimiters  :: threw str.replace is not a function
FAIL  8  Last keyword used case insensitively  :: threw str.replace is not a function
FAIL  V1 Last keyword used when the earlier keyword is in a nested fragment  :: threw str.replace is not a function
PASS  V3 Repeated keyword with identical ( delimiters (control)
FAIL  V4 Last keyword used when an earlier keyword is newline delimited  :: threw str.replace is not a function
FAIL  V2 Last keyword used with an empty array  :: threw syntax error at or near ")"
⚠️ 9 failed

The three rows that pass on both arms are deliberate controls: ...when nested keywords are all spaced (the same-delimiter spaced path must keep working), Single keyword followed by ( is still used (the single-keyword in( path must keep working), and Repeated keyword with identical ( delimiters (the case the backreference already handled correctly — it must not regress). Two base failures are notably not exceptions: Last keyword used for keywords other than in returns a wrong answer (expected 3 != got undefined), and Last keyword used with an empty array is rejected by the server (syntax error at or near ")", from the emitted where x in )). A silently incorrect query and a server-rejected one are stronger arguments for the fix than a build-time TypeError.

Upstream

• Repo: porsager/postgres (Postgres.js)
• Default branch: master
• Base sha: 411429e7bd7a3d61155ca9a70a97c111823702ea ("Do not mutate parameters when serializing in Bind")
• Files: src/types.js (the builders regex compilation, line 176 on base), tests/index.js
• Functions: builders (src/types.js:153-176), consumed by Builder.build (src/types.js:67-72)
• Fixes upstream issue WHERE IN throws TypeError: str.replace is not a function porsager/postgres#701 — "WHERE IN throws TypeError: str.replace is not a function"

Bug

Trigger. Any tagged query whose text before a sql([...]) interpolation contains the same helper keyword twice with different surrounding delimiters — canonically a nested in, e.g. sql`select x from test where x in(select x from test where x in ${sql([1,2])})`. One in is delimited " in(", the other " in ".

Wrong outcome. builders compiles each keyword to ((?:^|[\s(])KW(?:$|[\s(]))(?![\s\S]*\1). The lookahead is meant to assert "this is the last occurrence", but \1 backreferences the captured text rather than the keyword pattern, so it only rejects a later occurrence that is delimited identically. With mixed delimiters the earlier in still matches. Builder.build sorts all keyword hits and takes the highest index; the surviving early in@23 is then out-ranked by the select hit at (select @26. The array [1,2] is routed to the select helper, reaching escapeIdentifiers → escapeIdentifier → str.replace(...) on a number, which throws TypeError: str.replace is not a function.

Instrumented keyword selection on base, verbatim:

=== R2 "select x from t where x in(select x from t where x in "
    in@23=" in("  select@26="(select "
    WINNER: select @ 26
=== R4 ctrl "select x from t where x in (select x from t where x in "
    in@51=" in "  select@27="(select "
    WINNER: in @ 51

Blast radius. Anyone using the documented dynamic-value helper (README.md:280-289, "Dynamic values and where in") inside a nested in subquery, or with a sql([...]) where an earlier same-keyword occurrence is spaced differently. The query never reaches the server — it throws at build time, so it is a hard failure, not silent corruption. Regressed in v3.2.2 by commit 02f3854 ("Fix wrong helper selection on multiple occurances"), which introduced the \1 lookahead; reporters in porsager#701 hit it on 3.4.x. Three independent reporters in the thread (rytido, alonrbar, rozenmd), open ~2 years. The maintainer replied 2023-10-26: "Very nice tests @rytido !! Thank you - I'll look at fixing those and including your tests" — the tests below are built from exactly those cases.

Repro

Two layers, both executed.

Layer 1 — the failure itself, against a real server. The pre-fix build fails the nested-in shape reported in porsager#701. Transcript from the current 13-test file at head 280ff1d, base arm:

$ cd /agent-workspace/oss/pgtest && SCRIPT=verify13.mjs ./run-arm.sh /agent-workspace/oss/postgres-base-v1789476050
FAIL  1  Last keyword used even when an earlier keyword is followed by (  :: threw str.replace is not a function
FAIL  2  Last keyword used even when it ends the string  :: threw str.replace is not a function
FAIL  5  Last keyword used for keywords other than in  :: expected 3 != got undefined
FAIL  6  Last keyword used when the spaced keyword comes first  :: threw str.replace is not a function
FAIL  7  Last keyword used with three occurrences and three delimiters  :: threw str.replace is not a function
FAIL  8  Last keyword used case insensitively  :: threw str.replace is not a function
FAIL  V1 Last keyword used when the earlier keyword is in a nested fragment  :: threw str.replace is not a function
FAIL  V4 Last keyword used when an earlier keyword is newline delimited  :: threw str.replace is not a function
FAIL  V2 Last keyword used with an empty array  :: threw syntax error at or near ")"
⚠️ 9 failed

(The four base-passing rows — the pre-existing baseline and the three declared controls — are omitted above for brevity; the complete 13-line base transcript is in ## Summary.)

Layer 2 — the keyword-selection mechanism, driving the real Builder/stringify exports directly, which isolates why:

$ node /tmp/repro4.mjs          # against BASE 411429e
R1 not nested (works)                  => "($1)"  params= [1]
R2 nested, no space after outer in     => THREW str.replace is not a function  params= []
R3 nested, no space after inner in     => THREW str.replace is not a function  params= []
R4 space after both (works)            => "($1)"  params= [1]
A1 alonrbar IN (                       => THREW str.replace is not a function  params= []
A2 alonrbar IN(                        => "($1,$2,$3)"  params= ["2","3","4"]

R2/R3 are rytido's cases from the issue thread and are what this PR fixes. A1 is a different bug and is deliberately NOT fixed here — see ## Boundaries row 22 and the note at the end of ## Fix.

Note the headline snippet in the issue title (where age in ${sql([68,75,23])}, README form) does not reproduce on current master — it returns ($1,$2,$3) correctly. The live defect behind porsager#701 is the nested/mixed-delimiter shape reported in the thread's comments, which is what this PR targets.

Fix

src/types.js, the .map() that compiles builders:

-}).map(([x, fn]) => ([new RegExp('((?:^|[\\s(])' + x + '(?:$|[\\s(]))(?![\\s\\S]*\\1)', 'i'), fn]))
+}).map(([x, fn]) => {
+  const keyword = '(?:^|[\\s(])' + x + '(?:$|[\\s(])'
+  return [new RegExp(keyword + '(?![\\s\\S]*' + keyword + ')', 'i'), fn]
+})

The intent of 02f3854 was "use the last occurrence of the keyword". Repeating the keyword pattern in the lookahead expresses that directly; the backreference expressed the narrower and unintended "no later occurrence with identical delimiters". The capture group is dropped because nothing else read it.

Minimality: one expression, no behaviour added, no new dependency, no version bump, no changelog (repo requires none). It preserves the existing structure — still one regex per keyword, still consumed unchanged by Builder.build.

Alternatives rejected.

• Make Builder.build re-scan for the last match instead of relying on the lookahead — larger change to the hot path, and the lookahead is already the designed mechanism.
• Use a global regex with lastIndexOf-style scanning — changes search() semantics for all eight keywords and risks lastIndex statefulness on shared regex objects.
• Special-case in — the defect is generic to every keyword (values shows the same index shift, row 25 of the ledger); special-casing would leave the others wrong.
• Also fix (a,b,c) IN (${...}) (A1 above) — rejected as out of scope: different root cause, the '\\(': select builder added by c2fe67b legitimately out-ranks in there. It is a separate bug and belongs in its own PR; recorded as a lead, not fixed here.

Test evidence

Thirteen tests added to tests/index.js immediately after the existing Last keyword used even with duplicate keywords test (tests/index.js:2605), matching that test's exact conventions (t('name', async() => {...}), create/insert/assert, return [expected, got, await sql`drop table test`]). Nine were added by the original hunt (097e474); the last four were added by an independent adversarial verification pass at head 280ff1d, which did not change src/types.js (git diff 097e474 280ff1d -- src/types.js is empty).

#	Test	Role	On base (411429e)	At head (280ff1d)
0	Last keyword used even with duplicate keywords (pre-existing, unmodified)	baseline	passes	passes
1	Last keyword used even when an earlier keyword is followed by (	discriminates	FAILS str.replace is not a function	passes
2	Last keyword used even when it ends the string	discriminates	FAILS str.replace is not a function	passes
3	Last keyword used when nested keywords are all spaced	control — same-delimiter spaced path must stay working	passes	passes
4	Single keyword followed by ( is still used	control — single-keyword in( path must stay working	passes	passes
5	Last keyword used for keywords other than in	discriminates	FAILS expected 3 != got undefined (wrong answer, no throw)	passes
6	Last keyword used when the spaced keyword comes first	discriminates	FAILS str.replace is not a function	passes
7	Last keyword used with three occurrences and three delimiters	discriminates	FAILS str.replace is not a function	passes
8	Last keyword used case insensitively	discriminates	FAILS str.replace is not a function	passes
V1	Last keyword used when the earlier keyword is in a nested fragment (verification)	discriminates	FAILS str.replace is not a function	passes
V2	Last keyword used with an empty array (verification)	discriminates	FAILS syntax error at or near ")" (server-side)	passes
V3	Repeated keyword with identical ( delimiters is unchanged (control) (verification)	control — the case the backreference already got right	passes	passes
V4	Last keyword used when an earlier keyword is newline delimited (verification)	discriminates	FAILS str.replace is not a function	passes

Tests 1 and 2 are rytido's two failing cases from issue porsager#701; test 3 is that comment's working case, kept as a control. Test 4 controls the single-keyword in( path (## Boundaries row 7). Tests 5-8 close ledger rows that the probe covered but no test pinned: test 5 proves the fix is not in-specific (it uses returning, with a (-delimited occurrence inside a CTE and a spaced one after it), test 6 pins the reverse ordering, test 7 three occurrences with three delimiter shapes, and test 8 case-insensitivity of the repeated pattern.

The four V tests close four further ledger rows that were probe-only before this pass:

• V1 is the only test that reaches Builder.build through fragment() — a nested sql`...` fragment rather than the top-level template. That is a distinct production call path (stringifyValue → fragment → stringify), and it was entirely untested; it fails on base (row 32).
• V2 pins row 31 (empty array under a repeated keyword). Its base failure is the most severe kind in this PR: base emits where x in ) and PostgreSQL itself rejects the statement. The previously shipped row 31 evidence covered only the single-keyword empty-array case via an existing test.
• V3 pins row 9 as a declared control — identical ( delimiters are the one shape the \1 backreference handled correctly, so the fix must leave it alone. Both arms emit byte-identical SQL.
• V4 pins rows 18/19 (the [\s] delimiter class beyond a plain space) with a real newline between the query text and the keyword; probe-only before, fails on base now.

Fails-before / passes-after for every one of the thirteen: the verbatim two-arm transcript in ## Summary.

Method. git stash is not usable across worktrees, so the two arms are two worktrees of the same clone: base is a detached worktree at 411429e, head is the branch worktree at 280ff1d. One runner script holds all thirteen test bodies copied verbatim out of tests/index.js and is pointed at each worktree root in turn (import(root + '/src/index.js')), so the only variable between arms is the source tree.

Three test-shape defects were caught by execution, not by reading. (a) An early draft wrote these as indented multi-line templates, which pass on base — the leading newline puts a select keyword after the first in, changing which keyword wins; they were rewritten as single-line templates. (b) The first shipped version of test 4 was sql`select x from test where x in(${ sql([1, 2]) })`, which emits x in(($1,$2)) — the template supplies the ( and the in builder emits its own, making the operand a row constructor. Postgres rejects it with operator does not exist: integer = record, so that test failed on both arms and, because tests/test.js:32-33 short-circuits the file after the first failure, it would have silently skipped the t() tests after it on upstream CI. It now compares a row constructor on both sides ((x,x) in(${ sql([[1, 1]]) })), which keeps the in( delimiter the test exists to cover — confirmed in is still the winning builder — and genuinely passes on both arms. (c) The verification's own V3 control first appeared to fail on base, which would have contradicted ledger row 9. It was contamination, not discrimination: V2 emits server-invalid SQL on the base arm, and a server-side syntax error poisons the shared max: 1 connection for whatever runs next. Running V3 alone against base passes (rows=[1]), and types.stringify renders byte-identical SQL on both arms (...in(select x from test where (x,x) in(($1,$2)))). The runner now orders the server-error case last. On a shared connection, order-dependence looks exactly like discrimination — a surprising A/B result must be isolated before it is reported. The committed test order in tests/index.js puts the empty-array test last among the four for the same reason.

Lint — the repo's required tooling (package.json: "lint": "eslint src && eslint tests"), re-run at the current head:

$ npx --yes eslint@8 src tests
$ echo "ESLINT_RC=$?"
ESLINT_RC=0

Clean, no output, at head 280ff1d. (eslint 8 pinned to match the repo's .eslintrc.json eslintrc-format config.)

Verification method

executed — against a real PostgreSQL backend, twice, by two independent runs, with one remaining gap named below.

• The database. This container has no psql, docker, postgres or initdb (command -v → rc=127; /usr/lib/postgresql absent), so the repo's own tests/bootstrap.js (which shells out to createdb/psql) cannot run. A real Postgres is still reachable without them: PGlite is PostgreSQL compiled to WASM, and @electric-sql/pglite-socket exposes it on a TCP socket that postgres.js connects to over the ordinary wire protocol — real parser, real planner, real executor, real error messages.

$ npm i --no-save --no-package-lock --ignore-scripts --legacy-peer-deps @electric-sql/pglite @electric-sql/pglite-socket
added 2 packages in 1s
$ node server.mjs &       # PGLiteSocketServer on 127.0.0.1:5432
READY
$ node -e "... postgres({db:'postgres_js_test',user:'postgres_js_test'})\`select 1 as x\`"
Result(1) [ { x: 1 } ]

• Executed here: all thirteen tests, on both arms, end-to-end against that server — the SQL each builds is actually parsed and executed, which is how the row-constructor defect in the original test 4 was caught, and how V2's server-side syntax error at or near ")" on base was observed. Plus the keyword-selection probe, the two failing repros, the 30-row boundary probe, the V-row SQL-rendering probe, the V3 isolation run, and eslint src tests (rc=0). Runtime: Node v24.19.0 on Linux, PGlite 0.5.8.
• Two independent passes. The fix and tests 1-8 were produced by one run at 097e474; a second, independent adversarial run rebuilt the ## Boundaries ledger from the diff (not from this body), re-measured both arms from scratch, added tests V1-V4 for four rows that had no test, and committed them as 280ff1d. git diff 097e474 280ff1d -- src/types.js is empty — the production change is byte-identical to the previously reviewed head; only tests/index.js grew.
• NOT executed: npm run test:esm as a whole file (node tests/index.js) — the bootstrap needs createdb/psql to provision roles and the postgres_js_test database with extensions. The thirteen tests are run by the runner described above rather than by tests/test.js, so the t() harness wrapper itself is not exercised here; the test bodies are byte-identical to what is committed.
• What CI must confirm: upstream's own matrix (.github/workflows/test.yml, Node 12-24 × Postgres 12-17) running npm test — the thirteen tests green under the real t() harness, plus test:cjs and test:deno, which run the transpiled cjs//deno/ copies. The transpilers (transpile.cjs, transpile.deno.js) rewrite imports only; the changed expression is plain ES2020 and the .eslintrc.json target is es2020, so no transpile-specific risk is expected — but the CJS/Deno arms are unverified here.
• Fork CI: gh pr checks 1 --repo askalf/postgres at 280ff1d → no checks reported on the 'fix/builder-keyword-last-occurrence' branch. GitHub Actions are not enabled on this newly created fork (operator card filed to enable them). This is an absence of CI, not a failing CI — no job ran, green or red.

Prior art

Searches run 2026-09-15, all against porsager/postgres:

Search	Result
gh pr list --search "701 in:body" --state all	[] — nothing references the issue
gh search prs "701"	0 hits
gh search prs "str.replace"	0 hits
gh search prs "escapeIdentifier"	0 hits
gh search prs "in helper"	0 hits
gh search prs "builders keyword"	0 hits
gh search prs "Builder build helper"	0 hits
gh search prs "where in"	porsager#1128 (docs: dynamic filtering examples), porsager#259 (v3 mega-PR, merged 2022) — neither touches this
gh search prs "parenthesis"	porsager#264, porsager#107 — both README typo fixes, merged 2022/2020
gh search issues "str.replace is not a function"	porsager#701 (this one), porsager#1149, porsager#962, porsager#820, porsager#777, porsager#913, porsager#1073 open; porsager#947, porsager#712, porsager#636, porsager#604, porsager#396, porsager#203, porsager#305 closed
gh pr list --state open --limit 60 (scanned for src/types.js)	20 open PRs matched keyword scan; none touch builders/keyword selection

No open or closed PR addresses this. Issue porsager#701 is OPEN, labelled bug, no linked PR.

Related open issues sharing the str.replace is not a function symptom, checked and not claimed by this PR: porsager#1149 (two values CTEs) — I reproduced its exact query and it builds correctly on base (with c(lat,lon) as (values ($1, $2)),p(name) as (values ($3),($4)) select 1), so its cause is elsewhere; porsager#962, porsager#913, porsager#777, porsager#820 not investigated. The commit message says fixes #701 only.

Git-log mining that located the regression: git log --oneline -20 -- src/types.js surfaced 02f3854 "Fix wrong helper selection on multiple occurances" (the commit that added \1) and c2fe67b "Use select helper inside parenthesis" (which added the '\\(': select builder responsible for the separate A1 bug).

Policy

porsager/postgres ships no contribution policy files. Fetched via gh api repos/porsager/postgres/contents/<path>, all HTTP 404: CONTRIBUTING.md, AGENTS.md, .github/CONTRIBUTING.md, .github/PULL_REQUEST_TEMPLATE.md, CODE_OF_CONDUCT.md, AI_POLICY.md, .github/AI_POLICY.md, AI.md, AGENT_POLICY.md, CLAUDE.md. The root tree confirms it (.eslintrc.json, CHANGELOG.md, README.md, UNLICENSE, cf, cjs, deno, package.json, src, tests, transpile.*, types); .github/ contains only workflows/.

• AI/LLM/agent stance: silent. No ban, no explicit welcome. Nothing quotable exists to quote.
• CLA: none. DCO/sign-off: none. Changelog/changeset: not required (CHANGELOG.md is maintained by the author at release time — untouched here).
• License: Unlicense.
• Required tooling run: eslint src && eslint tests (from package.json "lint") → rc=0. npm test (test:esm + test:cjs + test:deno) requires a live Postgres and was not run — stated plainly in ## Verification method.

Disclosure facts for the operator

Plain facts, for you to write your own disclosure in your own words:

• An AI agent found this bug. The starting point was upstream issue WHERE IN throws TypeError: str.replace is not a function porsager/postgres#701; the agent determined the issue's headline snippet no longer reproduces, read the comment thread, and traced the real cause to the backreference in the builders lookahead.
• The AI identified the regressing commit (02f3854, v3.2.2) by mining git log -- src/types.js.
• The AI wrote the 4-line fix in src/types.js and all 13 regression tests in tests/index.js.
• The AI executed: all thirteen tests on both arms end-to-end against a real PostgreSQL backend (PGlite 0.5.8 over TCP), the keyword-selection probe, both failing repros, a 30-row boundary probe, an SQL-rendering probe for the new rows, an isolation run for the V3 control, and eslint src tests (rc=0) — all on Node v24.19.0 in a Linux container.
• The AI did not execute the repo's tests/test.js harness or the cjs/deno arms: the bootstrap needs createdb/psql, which the container lacks. Upstream CI is what confirms those.
• The work went through two adversarial reviews by separate, independent AI runs. The first caught a real defect the original shipped: the Single keyword followed by ( is still used test built invalid SQL (x in((1,2)), a row constructor) and failed on both arms while the body claimed it passed; it also found four untested boundary rows. The second rebuilt the boundary ledger from the diff, re-measured both arms from scratch, and added four more tests (nested fragment, newline delimiter, empty array under a repeated keyword, and a same-delimiter control) for rows that had been probe-only. Neither review changed src/types.js — the production fix has been byte-identical since it was first written.
• The AI self-corrected twice more: its first test draft used indented multi-line templates that passed on base (did not discriminate), caught in the base A/B and rewritten as single-line templates; and the second review's own control appeared to fail on base until it was isolated and shown to be cross-test contamination through the shared connection, not a real result.
• The test cases are derived from rytido's comment on issue WHERE IN throws TypeError: str.replace is not a function porsager/postgres#701 (2023-10-24), which the maintainer explicitly asked to include.
• No upstream repo was touched by the agent — no issues, comments, reviews or PRs.

Boundaries

Every row below is an executed probe, not an argument. The diff changes exactly one expression — the regex compiled per keyword — so the predicate under test is "which keyword does Builder.build select for a given preceding string". base/fixed give the winning keyword@index. A row only changes behaviour if the winning keyword differs (a differing index with the same keyword is inert — Builder.build uses the keyword's fn, not its index; confirmed byte-identical output for every such row, bottom table).

#	Boundary input	base	fixed	behaviour change	pinned by
1	"" (empty string)	-1 (no keyword)	-1	no	unreachable in practice — stringify always prefixes query text; build handles -1 via the escapeIdentifiers fallback, unchanged
2	"xyzzy " (no keyword at all)	-1	-1	no	fallback path unchanged (existing sql('column') identifier tests)
3	"in " (keyword at index 0, ^ anchor)	in@0	in@0	no	^ alternation of the pattern, probed
4	"in" (whole string is the keyword, both anchors)	in@0	in@0	no	probed
5	"select x from t where x in" ($ end-of-string delimiter)	in@23	in@23	no	test 2 covers the $ branch with a second keyword present
6	"...where x in " (single keyword, space delims)	in@23	in@23	no	existing tests where parameters in(), dynamic in after insert
7	"...where (x,x) in(" (single keyword, ( delim)	in → ($1,$2)	in → ($1,$2)	no	test 4 (control) — in measured as the winning builder on both arms, so the control still controls this row; test passes on both arms
8	two in, same delims (" in "," in ")	in@21	in@21	no	existing test Last keyword used even with duplicate keywords; test 3 (control)
9	two in, same delims (" in("," in(")	in@21	in@21	no	test V3 (control) — the \1 backreference was already correct for the same-delimiter case; measured byte-identical SQL on both arms (...in(select x from test where (x,x) in(($1,$2)))) and the test passes on both
10	porsager#701 R2: " in(" then " in "	select@26	in@50	YES	test 1 (fails on base)
11	porsager#701 R3: " in " then " in" (ends string)	select@27	in@51	YES	test 2 (fails on base)
12	porsager#701 R4: both in spaced	in@51	in@51	no	test 3 (control)
12b	reverse order: spaced " in " first, then " in(", then spaced " in "	select (throws)	in → ($1,$2)	YES	test 6 (fails on base). Measured on both arms with the exact prefix the test produces — proves the mechanism is last occurrence, not presence of a (-delimited occurrence
13	porsager#701 headline / README form where age in	in@29	in@29	no	existing test where parameters in(); probed byte-identical
14	three in, mixed delims	in@13	in@26	no (index only)	probed → byte-identical output ($1,$2); test 7 now runs a three-occurrence/three-delimiter query end-to-end (fails on base)
15	"(in(in(" (adjacent, overlapping delimiters)	in@0	in@0	no	probed
16	case-insensitivity: IN( then IN	in@13	in@13	no	probed (i flag retained); test 8 (fails on base)
17	all-uppercase WHERE X IN	in@23	in@23	no	probed
18	\n as delimiter ([\s] class)	in@23	in@23	no	probed; test V4 now pins the [\s] class end-to-end with a real newline before a repeated keyword (fails on base)
19	\t as delimiter	in@23	in@23	no	probed — same [\s] character class as row 18, which test V4 pins
20	\r\n CRLF — issue porsager#701's verbatim template	in@44	in@44	no	probed byte-identical
21	regex-meta keyword \( alone (create table t )	-1	-1	no	probed — the escaped-paren keyword still compiles identically
22	\( out-ranks in: delete from t where (a,b,c) in (	\(@30	\(@30	no	unchanged by this PR — this is the separate A1 bug, cause is c2fe67b's '\\(': select builder, not the lookahead. Deliberately out of scope.
23	keyword as substring of a word (where xin )	-1	-1	no	probed — delimiter classes prevent the false match
24	keyword as prefix of a word (where inside )	-1	-1	no	probed
25	multi-char keyword returning twice, mixed delims	returning@25	returning@25	no	probed; test 5 pins it end-to-end via a CTE — the one shipped test using a keyword other than in, and it fails on base with a wrong answer (expected 3 != got undefined), not a throw
26	other keyword values twice, diff delims (values(1) values )	values@13	values@23	no (index only)	probed → byte-identical ($1,$2). Confirms the defect was generic across keywords and that correcting it is inert where the keyword already won.
27	insert then values (helper precedence)	values@13	values@13	no	existing tests array insert, dynamic multi row insert
28	select then as	as@15	as@15	no	existing test Supports multiple nested fragments with parameters
29	update helper	update@0	update@0	no	existing update-helper tests
30	porsager#1149's two-values-CTE query	values@48	values@48	no	probed byte-identical — porsager#1149 is NOT fixed or affected by this PR
31	empty array sql([]) under a single in	in@29	in@29	no	existing test dynamic in with empty array; probed → both emit (null)
31b	empty array sql([]) under a repeated in (mixed delims)	select → ""	in → (null)	YES	test V2 (fails on base). Base emits where x in ) and the server rejects it: syntax error at or near ")". The falsy-but-valid empty collection under the fixed predicate still routes to in, which maps () → (null)
32	earlier keyword inside a nested fragment (sql\in(...`` interpolated into the outer template)	select@26 (throws)	in → ($1,$2)	YES	test V1 (fails on base). The only row reaching Builder.build via stringifyValue → fragment → stringify rather than the top-level template — a distinct production call path

Over the 32-row ledger: 5 rows change behaviour (10, 11, 12b, 31b, 32) — all five are the reported bug, across both orderings, the empty-collection case and the nested-fragment call path. 2 rows shift index only (14, 26); both verified byte-identical in emitted SQL and parameters. Every row that changes behaviour has a test that fails on base (tests 1, 2, 6, V2, V1); rows 9, 16, 18, 25 are additionally pinned by tests V3, 8, V4, 5. Byte-identical check, verbatim:

IDENTICAL three in, mixed delims     | "($1,$2)" params=[1,2]
IDENTICAL values twice diff delims   | "($1,$2)" params=[1,2]
IDENTICAL #1149 two values CTEs      | "($1),($2)" params=["latitude","longitude"]
IDENTICAL #701 headline              | "($1,$2,$3)" params=[68,75,23]
IDENTICAL #701 R4 control            | "($1)" params=[1]
IDENTICAL empty array in             | "(null)" params=[]

Non-boundary property — catastrophic backtracking. The lookahead now contains the keyword pattern twice rather than a backreference. Measured on a 60,000-character query string: base 0 ms, fixed 0 ms (both idx=60022). The pattern has no nested quantifier over an alternation, so no exponential blow-up is introduced.

Suggested upstream PR title

Fix wrong helper selection when a keyword is followed by ( - fixes #701

[askalf]

Verification — NOT verified, one blocking defect

Adversarial verification of fix/builder-keyword-last-occurrence at head a8d5ffe3a2b431db448c3c619b1c08dcf9fc8724 against base 411429e7bd7a3d61155ca9a70a97c111823702ea.

The production fix in src/types.js is correct and I confirmed it discriminates. The blocker is in tests/index.js: shipped test 4 emits invalid SQL and fails on BOTH arms.

I closed the PR's stated gap: a real Postgres WAS available

The body says npm run test:esm could not run because psql/docker/initdb are absent. They are — but a real PostgreSQL is still reachable without them. PGlite is Postgres itself compiled to WASM, and @electric-sql/pglite-socket puts it on a TCP socket that postgres.js connects to over the normal wire protocol:

$ npm i --no-save --no-package-lock --ignore-scripts --legacy-peer-deps @electric-sql/pglite @electric-sql/pglite-socket
added 2 packages in 1s
$ node -e "console.log(require('@electric-sql/pglite/package.json').version)"
0.5.8
$ node server.mjs &       # PGLiteSocketServer on 127.0.0.1:5432
READY
$ node -e "... postgres({db:'postgres_js_test',user:'postgres_js_test'})\`select 1 as x\`"
Result(1) [ { x: 1 } ]

So every test below ran end-to-end against a real Postgres backend, executing the actual SQL — not just the string builder. (Caveat: this server accepts one connection per lifetime, so each arm restarts it.)

BLOCKER — test 4 Single keyword followed by ( is still used fails on both arms

The body's ## Test evidence table and ## Boundaries row 7 both declare this test a control that passes. It does not pass. It never passed — it builds SQL Postgres rejects:

$ node -e "... stringify(sql\`select x from test where x in(\${ sql([1, 2]) })\`)"
select x from test where x in(($1,$2))
params  : [1,2]

The template already supplies the ( after in, and the in builder emits its own ($1,$2) — so the operand becomes ((1,2)), a row constructor, not a list. Postgres then compares integer = record:

### Shipped tests, HEAD arm a8d5ffe (real Postgres)
PASS  EXISTING Last keyword used even with duplicate keywords
PASS  T1 Last keyword used even when an earlier keyword is followed by (
PASS  T2 Last keyword used even when it ends the string
PASS  T3 Last keyword used when nested keywords are all spaced
FAIL  T4 Single keyword followed by ( is still used  :: threw operator does not exist: integer = record
⚠️ 1 failed

### Shipped tests, BASE arm 411429e
PASS  EXISTING Last keyword used even with duplicate keywords
FAIL  T1 Last keyword used even when an earlier keyword is followed by (  :: threw str.replace is not a function
FAIL  T2 Last keyword used even when it ends the string  :: threw str.replace is not a function
PASS  T3 Last keyword used when nested keywords are all spaced
FAIL  T4 Single keyword followed by ( is still used  :: threw operator does not exist: integer = record
⚠️ 3 failed

This is the one failure mode the string-level probe could not see. The body reasoned that the emitted SQL "is valid against a test(x int) table holding one row x=1". For test 4 that reasoning was wrong, and only a real server exposes it.

Why it is blocking, not cosmetic. tests/test.js:32-33 short-circuits the whole file after the first failure:

    failed
      ? (ignored++, ignore)
      : fn()

So on upstream CI this would fail test 4 and skip the 9 t() tests that follow it, across the entire Node 12-24 x Postgres 12-17 matrix. A maintainer would bounce this immediately.

T1/T2/T3 are confirmed good: T1 and T2 genuinely discriminate (fail on base with str.replace is not a function, pass at head), T3 is an honest control (passes both arms). The fix itself is sound.

Recommended repair — keep the control, keep the ( delimiter

Test 4's purpose (boundary row 7: single keyword, ( delimiter, must still select in) is worth keeping; only the SQL is malformed. Two candidates, both run on both arms:

### T4 repair candidates, HEAD arm          ### T4 repair candidates, BASE arm
FAIL  AS-SHIPPED T4 in( with 2 elements     FAIL  AS-SHIPPED T4 in( with 2 elements
FAIL  REPAIR-A T4 in( with 1 element        FAIL  REPAIR-A T4 in( with 1 element
PASS  REPAIR-B T4 row constructor (x,x) in( PASS  REPAIR-B T4 row constructor (x,x) in(
PASS  REPAIR-C T4 in at end of string       PASS  REPAIR-C T4 in at end of string

• REPAIR-A (sql([1]) → in(($1))) still fails — one-element row constructor, same class of error. Do not use.
• REPAIR-C (x in${ sql([1,2]) }) passes, but it changes the delimiter to end-of-string, which duplicates test 2's shape and so no longer controls for row 7.
• REPAIR-B is the right one — it keeps the in( delimiter and makes the row constructor deliberate on both sides:

t('Single keyword followed by ( is still used', async() => {
  await sql`create table test (x int)`
  await sql`insert into test values(1)`
  const [{ x }] = await sql`select x from test where (x,x) in(${ sql([[1, 1]]) })`

  return [1, x, await sql`drop table test`]
})

Confirmed it still exercises the intended path — in remains the winning builder, so the control keeps its meaning:

T4 as shipped   `...where x in(`           winner=values|in  raw="($1)"
REPAIR-B        `...where (x,x) in(`       winner=values|in  raw="($1)"
REPAIR-C        `...where x in` (EOS)      winner=values|in  raw="($1)"

Ledger holes — rows the body probed but shipped no test for

I rebuilt the ## Boundaries ledger from the diff. The fix is generic to all eight keywords, but every shipped test uses in. These four additions all ran against real Postgres; 4 of 5 discriminate (fail on base, pass at head):

### Adversarial additions, HEAD arm         ### Adversarial additions, BASE arm
PASS  T4a single in( with one element       PASS  T4a single in( with one element
PASS  T4b single in at end of string        PASS  T4b single in at end of string
PASS  T4c single in( no wrapping paren      FAIL  T4c ... str.replace is not a function
PASS  T5 returning twice, mixed delims      FAIL  T5 ... expected 3 != got undefined
PASS  T6 reverse order: spaced in then in(  FAIL  T6 ... str.replace is not a function
PASS  T7 three in occurrences, 3 delims     FAIL  T7 ... str.replace is not a function
PASS  T8 uppercase IN( then IN              FAIL  T8 ... str.replace is not a function
🎉 all passed                               ⚠️ 5 failed

• T5 pins a non-in keyword (ledger gap 5, returning with mixed delimiters, via a CTE) — this is the most valuable addition, since it proves the fix is generic rather than in-specific. Note its base failure is a wrong answer (expected 3 != got undefined), not a throw — a silently incorrect query, which is a stronger argument for the fix than the TypeError cases.
• T6 pins the reverse ordering (ledger gap 6: spaced in first, then in().
• T7 pins three occurrences with three different delimiter shapes.
• T8 pins case-insensitivity of the now-repeated pattern (IN( … IN ), which the backreference form got wrong too.

Test bodies for all of these are in /agent-output/oss/postgres/extra-tests.js.

Items I re-derived and found correct

• The fix expression itself. 20-case A/B probe of Builder.build keyword selection: 7 rows change winner base→head, all in the intended direction; no unintended keyword flips.
• Row 22 ((a,b,c) IN ( out-ranked by the '\\(' builder) — confirmed genuinely pre-existing and untouched: identical behaviour on base, head. Correctly excluded from this PR.
• Rows 14/26 (index shifts with the same winning keyword) — re-measured, inert as claimed; Builder.build uses the keyword's fn, not its index.
• Lint — npx --yes eslint@8 src tests → rc=0, clean.

Verdict

verified withheld. The production fix is right; the test file would break upstream CI. Fix test 4 (REPAIR-B), consider shipping T5-T8, and re-file for verification.

Verification runtime: Node v24.19.0, PGlite 0.5.8 over TCP. Fork Actions are not enabled (gh pr checks 1 = no checks) — absence, not failure.

[askalf]

Rework — blocker fixed, ledger holes closed

Addresses the verification block at a8d5ffe3. Head is now 097e474136f075382194d2ae71307e1ab4240c45. src/types.js is unchanged (git diff a8d5ffe..097e474 -- src/types.js is empty) — the production fix was re-derived as sound, only the test file moved.

1. The blocker: test 4 built invalid SQL and failed on both arms

select x from test where x in(${ sql([1, 2]) }) emits x in(($1,$2)) — the template supplies the ( and the in builder emits its own, so the operand was a row constructor and Postgres threw operator does not exist: integer = record. Because tests/test.js:32-33 short-circuits the file after the first failure, that one test would have skipped the 9 t() tests after it on the whole Node 12-24 x Postgres 12-17 matrix.

Applied REPAIR-B — a row constructor on both sides, which keeps the in( delimiter the test exists to control for:

const [{ x }] = await sql`select x from test where (x,x) in(${ sql([[1, 1]]) })`

Confirmed in is still the winning builder on both arms, so the control still controls Boundaries row 7:

=== BASE 411429e ===
row 7   single in( / test 4 (x,x) in(          => "($1,$2)"
=== HEAD 097e474 ===
row 7   single in( / test 4 (x,x) in(          => "($1,$2)"

2. Four ledger holes closed

Every shipped test used in, but the defect is generic to all eight keywords. Added, in upstream's naming style: Last keyword used for keywords other than in (returning, mixed delimiters via a CTE), ...when the spaced keyword comes first (reverse order), ...with three occurrences and three delimiters, ...case insensitively.

3. Both arms, real PostgreSQL

The body's "no database available" gap is closed — PGlite 0.5.8 (Postgres compiled to WASM) over @electric-sql/pglite-socket on 127.0.0.1:5432, so postgres.js connects over the ordinary wire protocol and the SQL is really parsed and executed.

######## HEAD — 097e474 (fix applied) ########
PASS  Last keyword used even with duplicate keywords (pre-existing)
PASS  Last keyword used even when an earlier keyword is followed by (
PASS  Last keyword used even when it ends the string
PASS  Last keyword used when nested keywords are all spaced
PASS  Single keyword followed by ( is still used
PASS  Last keyword used for keywords other than in
PASS  Last keyword used when the spaced keyword comes first
PASS  Last keyword used with three occurrences and three delimiters
PASS  Last keyword used case insensitively
🎉 all passed

######## BASE — 411429e (fix NOT applied) ########
PASS  Last keyword used even with duplicate keywords (pre-existing)
FAIL  Last keyword used even when an earlier keyword is followed by (  :: threw str.replace is not a function
FAIL  Last keyword used even when it ends the string  :: threw str.replace is not a function
PASS  Last keyword used when nested keywords are all spaced
PASS  Single keyword followed by ( is still used
FAIL  Last keyword used for keywords other than in  :: expected 3 != got undefined
FAIL  Last keyword used when the spaced keyword comes first  :: threw str.replace is not a function
FAIL  Last keyword used with three occurrences and three delimiters  :: threw str.replace is not a function
FAIL  Last keyword used case insensitively  :: threw str.replace is not a function
⚠️ 6 failed

9 tests: 6 discriminate, 2 are controls that pass on both arms (...all spaced, Single keyword followed by (), 1 is the pre-existing baseline. Last keyword used for keywords other than in fails on base with a wrong answer rather than a throw.

$ npx --yes eslint@8 src tests
$ echo "ESLINT_RC=$?"
ESLINT_RC=0

4. Body reconciled to this head

## Summary, ## Repro, ## Test evidence (9-row table), ## Verification method (now executed against a real backend, with the remaining t()-harness/cjs/deno gap named), ## Disclosure facts and ## Boundaries rows 7/12b/14/16/25 all rewritten for 097e474. Row 12b is new and its base/fixed winners were measured, not argued.

Verification items I did not change, per the findings: row 22 ((a,b,c) IN () is genuinely pre-existing and stays out of scope; rows 14/26 are inert index shifts.

[askalf]

Verification

Independent adversarial pass by a fresh run, at head 280ff1d. The ## Boundaries ledger was rebuilt from the diff, not from this body, and both arms were re-measured from scratch against a real PostgreSQL backend (PGlite 0.5.8 over the wire protocol).

Production source is untouched by this pass. git diff 097e474 280ff1d -- src/types.js is empty — the 4-line fix is byte-identical to the previously reviewed head. This commit adds four tests to tests/index.js and nothing else.

Four ledger rows had no test; they do now

Row	Gap	New test	On base (411429e)
32 (new)	earlier keyword inside a nested fragment — the only path reaching Builder.build via stringifyValue → fragment → stringify rather than the top-level template; entirely untested	Last keyword used when the earlier keyword is in a nested fragment	FAILS str.replace is not a function
31b (new)	empty array under a repeated keyword (row 31 only covered the single-keyword case, via an existing test)	Last keyword used with an empty array	FAILS syntax error at or near ")" — server-side
9	same-(-delimiter repeat — the one shape the \1 backreference already handled; nothing pinned that the fix leaves it alone	Repeated keyword with identical ( delimiters is unchanged (control)	passes (declared control)
18/19	the [\s] delimiter class beyond a plain space — probe-only	Last keyword used when an earlier keyword is newline delimited	FAILS str.replace is not a function

Row 31b is worth a maintainer's eye: on base the builder emits where x in ) and PostgreSQL rejects the statement outright. That is a third failure mode alongside the build-time TypeError and the silently-wrong returning answer.

Both arms, verbatim, 13 tests

######## HEAD — fix/builder-keyword-last-occurrence @ 280ff1d (fix applied) ########
PASS  0  Last keyword used even with duplicate keywords (pre-existing)
PASS  1  Last keyword used even when an earlier keyword is followed by (
PASS  2  Last keyword used even when it ends the string
PASS  3  Last keyword used when nested keywords are all spaced
PASS  4  Single keyword followed by ( is still used
PASS  5  Last keyword used for keywords other than in
PASS  6  Last keyword used when the spaced keyword comes first
PASS  7  Last keyword used with three occurrences and three delimiters
PASS  8  Last keyword used case insensitively
PASS  V1 Last keyword used when the earlier keyword is in a nested fragment
PASS  V3 Repeated keyword with identical ( delimiters (control)
PASS  V4 Last keyword used when an earlier keyword is newline delimited
PASS  V2 Last keyword used with an empty array
🎉 all passed

######## BASE — origin/master @ 411429e (fix NOT applied) ########
PASS  0  Last keyword used even with duplicate keywords (pre-existing)
FAIL  1  Last keyword used even when an earlier keyword is followed by (  :: threw str.replace is not a function
FAIL  2  Last keyword used even when it ends the string  :: threw str.replace is not a function
PASS  3  Last keyword used when nested keywords are all spaced
PASS  4  Single keyword followed by ( is still used
FAIL  5  Last keyword used for keywords other than in  :: expected 3 != got undefined
FAIL  6  Last keyword used when the spaced keyword comes first  :: threw str.replace is not a function
FAIL  7  Last keyword used with three occurrences and three delimiters  :: threw str.replace is not a function
FAIL  8  Last keyword used case insensitively  :: threw str.replace is not a function
FAIL  V1 Last keyword used when the earlier keyword is in a nested fragment  :: threw str.replace is not a function
PASS  V3 Repeated keyword with identical ( delimiters (control)
FAIL  V4 Last keyword used when an earlier keyword is newline delimited  :: threw str.replace is not a function
FAIL  V2 Last keyword used with an empty array  :: threw syntax error at or near ")"
⚠️ 9 failed

9 discriminate, 3 are declared controls, 1 is the pre-existing baseline. The four base-passing rows are exactly the baseline plus the three controls — no test in this file passes on base without being named a control.

A near-miss worth recording

The V3 control failed on the base arm in the first run, which would have contradicted ledger row 9 and looked like a real finding. It was contamination, not discrimination: the empty-array test sends server-invalid SQL on base, and a server-side syntax error poisons the shared max: 1 connection for whatever runs next. Two independent checks settled it — V3 run alone against base passes (rows=[1]), and types.stringify renders byte-identical SQL on both arms:

===== BASE =====
V3 identical ( delims
  SQL    "select x from test where x in(select x from test where (x,x) in(($1,$2)))"
===== HEAD =====
V3 identical ( delims
  SQL    "select x from test where x in(select x from test where (x,x) in(($1,$2)))"

The runner and the committed test order now place the server-error case last. On a shared connection, order-dependence looks exactly like discrimination.

Tooling

$ npx --yes eslint@8 src tests
$ echo "ESLINT_RC=$?"
ESLINT_RC=0

gh pr checks 1 --repo askalf/postgres at 280ff1d → no checks reported. Actions are not enabled on this fork; that is an absence of CI, not a failure. The t() harness wrapper and the cjs/deno transpiled arms remain unverified here (the bootstrap needs createdb/psql) — upstream CI is what confirms those, and that gap is stated in the body.

Body reconciled to this head: test counts, both transcripts, the test table, ## Verification method, ## Disclosure, and Boundaries rows 9/18/19/31/31b/32.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the GPT gating lane (gating review).

Verdict: APPROVED — ready for the operator to submit; no blocking issues found in the live head.

I reviewed the two-file diff and traced the base implementation at src/types.js:67-71,153-176. The changed regex removes the delimiter-sensitive backreference and repeats the keyword pattern in the negative lookahead, so Builder.build selects the last matching helper irrespective of whether the occurrences use space, (, or end-of-string delimiters. The added tests exercise the mixed-delimiter failure, reverse order, three occurrences, case handling, a non-in helper, nested fragments, newline delimiters, empty arrays, and controls.

For OSS-candidate readiness, I verified that the facts sheet contains all required sections; the base code exhibits the delimiter-sensitive \1 behavior; the body supplies real-server before/after evidence (13/13 at head, 9 failing at base); the boundary ledger covers the changed regex predicate; the prior-art searches found no competing upstream PR; and the commit messages have no AI attribution. The stated lint evidence is green. There are no fork CI checks configured, so the upstream Node/Postgres and CJS/Deno matrix remains operator-side CI confirmation rather than a green signal.

What's good: this is a minimal production change, with unusually strong regression coverage that proves both the reported failure and preservation of the same-delimiter paths.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: no blocking issues. This is a clean, well-scoped, idiomatically consistent fix and I could independently confirm both the bug and the fix mechanism from the base source.

What I independently verified

I did not read the gating review. I re-derived everything below directly from porsager/postgres base @ 411429e and the fork head 280ff1d.

• The bug is real and reproduces. Calling Builder with the base regex (((?:^|[\s(])in(?:$|[\s(]))(?![\s\S]*\1)) against "...where x in(select x from test where x in " picks select@26 as the winner, not in, because the \1 backreference only cancels a later occurrence with an identical delimiter substring. I ran this against the actual src/types.js Builder class in the fork (not a reimplementation) at both src/types.js:176-179 and confirmed the winner-selection flips from select→in between base and fixed regex for every mixed-delimiter case (paren-then-space, space-then-end, reverse order, case-insensitive, newline, CRLF, empty array under a repeated keyword).
• The fix (src/types.js:176-179) is minimal and correct. Repeating the keyword pattern in the lookahead instead of backreferencing the captured text is the right generalization — it asserts "no later occurrence of this keyword" rather than "no later occurrence of this exact delimited substring." I confirmed it does not regress any of the same-delimiter/no-later-occurrence cases (single keyword, both same-paren, both same-space, keyword-only string, prefix/substring false-match guards, escaped-paren-outranks-in row) — all winners are byte-identical to base in those rows.
• Test 4 row-constructor risk (flagged by a prior fleet run against the intermediate head) is fixed in the shipped body. The current test 4 (tests/index.js:2634-2640) uses (x,x) in(${ sql([[1,1]]) }), which I confirmed still selects the in builder and emits a valid, non-row-constructor comparison — good repair, matches what the PR body claims.
• Test V1's nested-fragment call path is real (tests/index.js — sql\select x from test where x ${ sql`in(select x from test where x in ${ sql([1, 2]) })` }`). I confirmed stringifyValuereachesBuilder.buildthroughfragment()(src/types.js:107-118) as a second call path distinct from the top-level template, and that this path flips winner fromselect(throws) toin` between arms — this is a genuine coverage gap the original 9-test set would have missed.
• No ReDoS regression. Repeating a keyword literal in a lookahead with no nested quantifier over an alternation does not introduce catastrophic backtracking; I measured both regex forms against 1k-200k character strings with no match and near-miss strings, both sub-5ms.
• Prior art re-run. gh search prs "701", "backreference", "builders" against porsager/postgres, and a scan of all currently-open PRs (gh api .../pulls?state=open) for anything touching src/types.js — none address this. Only porsager#1165 (Array.isArray(first[0]) → Array.isArray(first) in values()) touches the same file, and it's an unrelated fix to a different function, not in conflict.

Maintainer idiom check

• Fix shape matches the maintainer's own precedent. PR porsager#1198 ("Do not mutate parameters when serializing in Bind"), merged by the maintainer with the comment "Great PR and catch! Thank you!", is the closest analog: a one-line/few-line fix in src/*.js plus a batch of tests/index.js regression tests appended immediately after the related existing test, using the exact same t('name', async() => {...}) / create-insert-assert-drop convention. This PR follows that shape precisely (tests/index.js:2615 insertion point, right after the existing Last keyword used even with duplicate keywords test).
• No changelog entry — correct per convention. CHANGELOG.md in this repo is maintained by the author at release time (confirmed by inspecting the file: entries are one-liners with the merge commit's short SHA appended, clearly written post-merge, not part of the PR diff in porsager#1198 or porsager#1165 either).
• PR title style matches. Compare to merged titles like "Fix wrong helper selection on multiple occurances" (02f3854, the original regression commit) and "Fix: Do not mutate parameters when serializing in Bind" (porsager#1198) — imperative, "Fix ...", references the mechanism. The suggested title Fix wrong helper selection when a keyword is followed by ( - fixes #701 fits.
• One thing an upstream maintainer might push back on, though not blocking: the inline // Control: ... comment above the "Repeated keyword with identical ( delimiters" test (tests/index.js:2698-2700) is a convention not used anywhere else in tests/index.js — I grepped the full upstream test file and found zero // comments preceding any of the 264 existing tests. It's harmless and the intent is genuinely useful context, but it's a stylistic outlier the maintainer may trim or leave; not something I'd call a defect.
• No CONTRIBUTING/PR template exists upstream (confirmed 404 on all the usual paths), so there's no description-shape rule to check against for the eventual upstream submission.

Break-it pass (boundaries rebuilt from the diff)

I rebuilt the winner-selection ledger myself from the two regex forms rather than reading the PR body's table first, then cross-checked. My independent run confirms the same five behavior-changing predicates the body claims (mixed-delimiter pairs in both orders, case-insensitive, newline/CRLF, empty-array-under-repeated-keyword, nested-fragment) and no additional ones. I did not find a reachable row the body misses. The \( vs in precedence bug (row 22 in the body, (a,b,c) in () is genuinely pre-existing and correctly out of scope — I confirmed it's identical on both arms (\(@30 wins both times) and traces to a different builder (c2fe67b), not the lookahead this PR touches.

Assertion strength

Spot-checked test 4 (single-in( control) and test V1 (nested fragment) directly against the real Builder class rather than trusting the body's transcript — both produce the claimed output on the fixed head. I did not re-run the full 13-test suite end-to-end against a live server myself (no local Postgres/PGlite setup attempted in this pass); the body's own two independent PGlite-backed runs (097e474 and 280ff1d) are the executed evidence for that layer, and I have no reason to doubt them given the mechanism checks out independently.

What's good

Small, surgical fix; the regex change is the minimum needed and preserves the exported shape (Builder/builders unchanged). Test coverage is unusually thorough for the size of the fix — it explicitly covers the "generic across all eight keywords" property (test 5 uses returning, not in) rather than testing only the reported keyword.

Disclosure section and the two prior adversarial passes are appropriately transparent about what's unexecuted (the real t() harness, cjs/deno transpiled arms) — I have nothing to add there.

SECOND READ: READY

[askalf]

Upstream PR opened; tracked in the ledger.