fix(mapper): keep diagnostics inside the file by colinhacks · Pull Request #20 · colinhacks/reylo

colinhacks · 2026-10-22T19:15:59Z

A .sql file that ends inside a string, a quoted identifier or a block comment lost its types: tsc reported

q.sql(1,1): error TS100025: The content mapper 'reylo' failed to transform this file.
  The content mapper returned an invalid transform response.

and the import became any.

Cause

The engine's lexer reported these errors at the end of the text (offset: b.len()). The analysis turned that into a diagnostic with start equal to the file's length and length: 1, which runs one byte past the end of the file, and TypeScript refuses a transform holding such a diagnostic. With the mapper's range clamped (and no engine change), tsc gives q.sql(1,10): error reylo1: unterminated string literal for select 'a, which confirms the range as the only problem.

Fix

Engine (engine/core/src/lex.rs): each unterminated error now points at the opening quote or the comment's start, as Postgres itself does. This covers a string, an E'' string, a quoted name, a MySQL block comment, and a nested Postgres one. A dollar quote already pointed at its start. The error now underlines what was left open, and its range lies inside the text. The three engine.wasm artifacts are rebuilt.
Mapper (src/mapper.ts, withinFile in src/module-types.ts): every diagnostic the mapper returns is cut at the end of the file, so no engine diagnostic can invalidate a transform again.

Before / after (SQLite, `tsc --runExternalCode`)

File	Before	After
`select 'a`	`q.sql(1,1): error TS100025 …`, import is `any`	`q.sql(1,8): error reylo1: unterminated string literal`
select `a	`q.sql(1,1): error TS100025 …`	`q.sql(1,8): error reylo1: unterminated quoted identifier`
`select 1 /* x`	`q.sql(1,1): error TS100025 …`	`q.sql(1,10): error reylo1: unterminated block comment`

Verified

tests/unterminated.test.ts runs tsc over the three files on SQLite and on Postgres. It checks each error's position and that every module is typed: an @ts-expect-error on a missing property would go unused if the import were any. It also unit-tests withinFile.
engine/core/src/lex.rs tests the offset of every unterminated form.
Gates cargo-build, engine-tests, oracle-tests, wasm, oracle, typecheck, loader-tests (0 skipped) and package all pass.

vercel · 2026-10-22T19:16:04Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
reylo	Ready	Preview	Oct 22, 2026 10:02pm UTC

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

pullfrog

✅ No new issues found.

Reviewed changes

I reviewed both commits. They fix the TS100025 transform rejection for .sql files that end inside an open quote or comment.

Lexer offsets (engine/core/src/lex.rs): unterminated strings, E'' strings, quoted identifiers and both block comment forms now report the offset of their opening token. They used to report b.len() or the scan position, and analysis.rs/mysql.rs then turned that into a length: 1 diagnostic one byte past the end of the file. The old E'…\ case could even land at len + 1. That case is covered now too.
Mapper clamp (src/mapper.ts, withinFile in src/module-types.ts): every diagnostic's range is clamped to Buffer.byteLength(sql). That matches the mapper's positionEncoding: "utf-8", so no future engine diagnostic can invalidate a transform.
Tests (tests/unterminated.test.ts, lexer unit tests): these run real tsc --runExternalCode and use @ts-expect-error to prove each module stays typed. I ran node --test tests/unterminated.test.ts locally: the SQLite case and the withinFile unit test pass, and the Postgres case was skipped because no server was available. No other copy of the old (at byte N) text exists in the oracle or test fixtures.

^{｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}

pullfrog

✅ No new issues found. The push since the last review rebases onto main and rebuilds the engine artifacts.

Reviewed changes

Since the prior review, the branch has been rebased onto main (237e417) and the three engine binaries have been rebuilt.

Rebased the two commits unchanged: git range-diff shows both commits as identical. lex.rs, mapper.ts, module-types.ts and tests/unterminated.test.ts have the same blob hashes as at 5bb6ffa.
Rebuilt packages/{mysql,postgres,sqlite}/engine.wasm on top of #7: these are the only changed blobs. I checked that each binary carries both sets of changes:
- #7's engine code: the odd_pairs message ("takes keys and values in pairs") is in the postgres and sqlite wasm. MySQL never calls odd_pairs, so it is correctly absent there.
- This PR's lexer offsets: I loaded each wasm through Engine and ran analyze on select 1, 'abc, a quoted name, a block comment and an E'' string. Every one returns start: 10 on all three dialects.
- End-to-end test: node --test tests/unterminated.test.ts still passes on SQLite, together with the withinFile unit test. The Postgres case was skipped because no server was available.

^{｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}

pullfrog

ℹ️ The rebase has no code issues. But main gained #15 after this push, and #15 rebuilt packages/postgres/engine.wasm, so that binary will conflict.

Reviewed changes

Since the prior review, the branch was rebased onto cb43f3e (#17, #21), and the wasm rebuild moved into its own commit.

Rebased the lexer and mapper commits. lex.rs, module-types.ts and tests/unterminated.test.ts have the same blobs as at 3620d0b. In mapper.ts the only change is the context from #17. All three diag( call sites still pass size, including the no-project path, which now emits #17's doc comment.
Rebuilt packages/mysql/engine.wasm on top of #21. The new binary is 1683423 bytes. #21's build was 1683085 bytes and the one before it 1678775, so the new binary includes #21's growth. The postgres and sqlite binaries were left alone, which is correct against cb43f3e.
Re-checked the offsets in every committed wasm. I loaded each binary through Engine and ran analyze on each unterminated form: strings, quoted names, E'', $x$ and both block-comment forms. Every one returns start: 10 on all three dialects.

ℹ️ `packages/postgres/engine.wasm` predates #15 on `main`

The branch is based on cb43f3e, but main is now at 0ac13f3 (#15). #15 changed the Postgres engine (infer.rs, postgres.rs, jsonshape.rs) and rebuilt packages/postgres/engine.wasm. This PR's Postgres binary was built without #15. Both sides changed that binary since cb43f3e, so the merge will conflict. Taking either side alone would drop either #15's inference or this PR's lexer offsets.

Technical details

# Postgres engine.wasm must be rebuilt on top of #15

## Affected sites
- packages/postgres/engine.wasm — PR blob `48edfa8` (3788645 bytes) has this PR's lexer offsets but not #15; main's blob `cfb89a4` (3803745 bytes) has #15 but not this PR's lexer offsets.

## Required outcome
- Rebase onto `0ac13f3` and rebuild `packages/postgres/engine.wasm` (mysql and sqlite are unaffected by #15) so the binary carries both #15's `infer.rs`/`postgres.rs`/`jsonshape.rs` changes and the `lex.rs` offsets.
- An `analyze` of `select 1, 'abc` on the rebuilt Postgres wasm should still return a diagnostic with `start: 10`.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}

pullfrog

✅ No new issues found. The rebase onto d450564 clears the packages/postgres/engine.wasm conflict with #15 that the last review flagged.

Reviewed changes

Since the prior review, the branch was rebased onto main at d450564 (#10), and the engine artifacts were rebuilt again.

Rebased the lexer, mapper and test commits. git range-diff shows all three commits as identical patches.
Rebuilt packages/{postgres,sqlite,mysql}/engine.wasm on main. Each binary is main's build plus this PR's lex.rs change:
- The Postgres binary is the same size as main's (3808259 bytes) and only 71 bytes differ. So it carries #15, #18 and #10.
- The SQLite binary is the same size as main's and only 35 bytes differ.
- The MySQL binary is 338 bytes larger than main's, which includes #29. The rebuild before this one added the same 338 bytes on top of #21.
Re-checked the offsets in every committed wasm. I loaded each binary through Engine and ran analyze on every unterminated form. Every one returns start: 10 on all three dialects. As a control, main's Postgres binary still returns start: 14 for select 1, 'abc.

^{｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}

pullfrog

ℹ️ The rebase has no code issues. But main gained #39 after this push. #39 rebuilt packages/postgres/engine.wasm, so that binary conflicts again.

Reviewed changes

Since the prior review, the branch was rebased onto b9b2e05 (#32, #36, #37), and the SQLite engine was rebuilt.

Rebased the lexer, mapper and test commits. git range-diff shows all three commits as identical patches.
Rebuilt packages/sqlite/engine.wasm on top of #32. The new binary is 2446062 bytes and main's at b9b2e05 is 2445740. It includes #32's full-text search functions (bm25, highlight, snippet). The Postgres and MySQL binaries are the same as at 13e2c82. That is correct, because nothing between d450564 and b9b2e05 touched them.
Checked the binaries against their sources. tests/fingerprint.test.ts (added in #36) passes for all three binaries at c928b6f. I also loaded the SQLite binary through Engine and ran analyze on each unterminated form. Every one returns start: 10.

ℹ️ `packages/postgres/engine.wasm` predates #39 on `main`

main is now at 8486b5c (#39). #39 moved the pg_query pins into engine/postgres/vendor/pg_query-source.sh, which the Postgres fingerprint hashes, and rebuilt packages/postgres/engine.wasm. git merge-tree against origin/main reports a binary conflict on that file.

Whichever side the merge keeps, the result is wrong. Keeping main's binary drops this PR's lexer offsets. Keeping this PR's binary fails tests/fingerprint.test.ts: I checked this by putting the PR's lex.rs and Postgres wasm on top of 8486b5c.

Technical details

# Postgres engine.wasm must be rebuilt on top of #39

## Affected sites
- packages/postgres/engine.wasm — PR blob `641b73d` (3808259 bytes) has this PR's `lex.rs` offsets but was built without `engine/postgres/vendor/pg_query-source.sh`; main's blob `bf881e4` (3809553 bytes) has #39 but not the offsets.

## Required outcome
- Rebase onto `8486b5c` and rebuild `packages/postgres/engine.wasm` with `engine/build.sh postgres`. #39 does not affect the SQLite and MySQL binaries.
- `node --experimental-strip-types --test tests/fingerprint.test.ts` passes for all three binaries.
- An `analyze` of `select 1, 'abc` on the rebuilt Postgres wasm still returns a diagnostic with `start: 10`.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}

pullfrog

ℹ️ The rebase onto 8486b5c clears the packages/postgres/engine.wasm conflict with #39. But main gained #33 after this push. #33 rebuilt packages/mysql/engine.wasm, so that binary conflicts now.

Reviewed changes

Since the prior review, the branch was rebased onto 8486b5c (#39 and two docs commits), and the Postgres engine was rebuilt.

Rebased the lexer, mapper and test commits. git range-diff shows all three commits as identical patches.
Rebuilt packages/postgres/engine.wasm on top of #39. The new blob is 039a174. The SQLite and MySQL binaries are the same as at c928b6f. That is correct, because #39 changed only the Postgres engine.
Checked the binaries against their sources. tests/fingerprint.test.ts passes for all three binaries at a980394. I loaded the new Postgres binary through Engine and ran analyze on 'abc, "abc, /* abc, E'ab\ and $x$abc. Every one returns start: 10.

ℹ️ `packages/mysql/engine.wasm` predates #33 on `main`

main is now at 1b16b35, which includes 09da440 (#33). #33 changed the MySQL engine (engine/mysql/src/mysql.rs, the new mysql_derived.rs, mysql_shape.rs) and rebuilt packages/mysql/engine.wasm. git merge-tree HEAD origin/main reports a binary conflict on that file.

Neither side of the conflict is right on its own. main's binary has #33 but not this PR's lexer offsets. This PR's binary has the offsets but was built without #33's sources, so it would fail tests/fingerprint.test.ts after the merge. #33 does not touch engine/core or the path that turns a LexError into a diagnostic, so a rebuild is all this needs.

Technical details

# MySQL engine.wasm must be rebuilt on top of #33

## Affected sites
- packages/mysql/engine.wasm — PR blob `0964cd4` (1687886 bytes) has this PR's `lex.rs` offsets but not #33; main's blob `2c2bb64` (1700223 bytes) has #33 but not the offsets.

## Required outcome
- Rebase onto `origin/main` (`1b16b35`) and rebuild `packages/mysql/engine.wasm` with `engine/build.sh mysql`. #33 does not affect the Postgres and SQLite binaries.
- `node --experimental-strip-types --test tests/fingerprint.test.ts` passes for all three binaries.
- An `analyze` of `select 1, 'abc` on the rebuilt MySQL wasm still returns a diagnostic with `start: 10`.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}

pullfrog

✅ No new issues found. The rebase onto 1b16b35 clears the packages/mysql/engine.wasm conflict with #33 that the last review flagged.

Reviewed changes

Since the prior review, the branch was rebased onto 1b16b35 (#33 and two docs commits), and the MySQL engine was rebuilt.

Rebased the lexer, mapper and test commits. git range-diff shows all three commits as identical patches.
Rebuilt packages/mysql/engine.wasm on top of #33. The new blob is ef78280 at 1700561 bytes. main's binary is 1700223 bytes, so the new one is 338 bytes larger, the same difference every earlier MySQL rebuild added. The Postgres and SQLite binaries are unchanged from a980394. That is correct, because #33 changed only the MySQL engine.
Checked the binaries against their sources. tests/fingerprint.test.ts passes for all three binaries at b9498ca. I loaded the new MySQL binary through Engine and ran analyze on 'abc, `abc and /* abc. Every one returns start: 10.
Checked the merge against the current main. main has since moved to 299f6e6 (#38), which changes only .github/ files. git merge-tree --write-tree HEAD origin/main merges cleanly.

^{｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}

Copilot AI balanced review requested due to automatic review settings October 22, 2026 19:16

Copilot AI reviewed Oct 22, 2026