feat(vnext): recognize bounded relation query sites - #186
Conversation
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/vnext/query-site.ts">
<violation number="1" location="src/vnext/query-site.ts:1388">
P2: Malformed `NATURAL LEFT(x)` sequences can now produce relation completions instead of failing closed because `(` clears compound NATURAL join prefixes. Preserve the function-call recovery only for non-compound prefixes; mark `natural-*` prefixes ambiguous when `JOIN` is absent.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if (code === 40) { | ||
| punctuationFrame.joinPrefix = null; | ||
| } else { | ||
| markUnavailable(punctuationFrame, "ambiguous-query-site"); | ||
| } |
There was a problem hiding this comment.
P2: Malformed NATURAL LEFT(x) sequences can now produce relation completions instead of failing closed because ( clears compound NATURAL join prefixes. Preserve the function-call recovery only for non-compound prefixes; mark natural-* prefixes ambiguous when JOIN is absent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/vnext/query-site.ts, line 1388:
<comment>Malformed `NATURAL LEFT(x)` sequences can now produce relation completions instead of failing closed because `(` clears compound NATURAL join prefixes. Preserve the function-call recovery only for non-compound prefixes; mark `natural-*` prefixes ambiguous when `JOIN` is absent.</comment>
<file context>
@@ -1253,12 +1381,15 @@ export function recognizeSqlRelationQuerySite(
punctuationFrame.joinPrefix !== null
) {
- punctuationFrame.joinPrefix = null;
+ if (code === 40) {
+ punctuationFrame.joinPrefix = null;
+ } else {
</file context>
| if (code === 40) { | |
| punctuationFrame.joinPrefix = null; | |
| } else { | |
| markUnavailable(punctuationFrame, "ambiguous-query-site"); | |
| } | |
| if ( | |
| code === 40 && | |
| !punctuationFrame.joinPrefix.startsWith("natural-") | |
| ) { | |
| punctuationFrame.joinPrefix = null; | |
| } else { | |
| markUnavailable(punctuationFrame, "ambiguous-query-site"); | |
| } |
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
## Summary - extract the streaming bounded SQL lexer from the relation-site state machine into one package-private module - preserve the exact PostgreSQL, DuckDB, BigQuery, and Dremio lexical profiles, UTF-16 offsets, embedded-region barriers, quote/comment behavior, one-token pushback, and 16,384-lexeme ceiling - keep query-site keyword, comment-cursor, region, and resource semantics local to the consumer - translate generic lexer resource evidence through an exhaustive package-owned map - add direct boundary tests without exposing tokens or lexer APIs from the package This is a zero-semantics prerequisite for the separate bounded CTE layout/visibility recognizer. The recognizers will initially use separate streaming traversals over the same lexical implementation; any traversal fusion remains benchmark-driven. ## Performance An initial broader helper extraction caused a reproducible Vite SSR namespace-call regression on the hot path. The boundary was narrowed before commit. Stable means at the exact head are back at the PR #186 baseline: - exact 10 KiB statement: about 0.56–0.59 ms - 1,000 classified aliases: about 0.36–0.38 ms - 1,000 authenticated `USING` columns: about 0.21 ms The lexer remains streaming and does not allocate a token tape. ## Verification - 1,543 tests passed plus 1 expected failure - changed coverage: 97.05% statements, 95.79% branches, 100% functions, 97.04% lines - bounded lexer coverage: 98.92% statements/lines, 98.78% branches, 100% functions - repository coverage: 95.34% statements, 92.05% branches, 96.15% functions, 95.33% lines - source, test, loose-optional, and demo typechecks pass - repository oxlint, test-integrity, and diff checks pass - browser, package, worker-placement, demo, and benchmark gates pass - 20,000 deterministic differential lexer comparisons passed across dialects, masked regions, subranges, UTF-16, comments, quotes, punctuation, and pushback - independent SQL/API and concurrency/performance reviewers approved exact commit `ed079df0f68aadaf8957a7ca5e6c497e91b74860` Part of #169. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Refactored vNext to share a streaming bounded SQL lexer and the embedded-region lookup, and switched `query-site` to use them. Behavior is unchanged across PostgreSQL, DuckDB, BigQuery, and Dremio; this unblocks the bounded CTE recognizer in #169. - **Refactors** - Moved the lexer into package-private `src/vnext/bounded-sql-lexer.ts`. - Centralized `findSqlEmbeddedRegionAtOrAfter` in `src/vnext/source.ts` and reused it in the lexer and `query-site` (with tests). - Preserves lexical profiles, UTF-16 offsets, embedded-region barriers, quote/comment rules, one-token pushback, and the 16,384-lexeme cap. - Kept consumer-specific semantics in `query-site`; mapped lexer resource signals to local `query-site` resources. - Removed duplicated lexer and region-lookup code from `src/vnext/query-site.ts` and wired it to the shared modules. <sup>Written for commit 76190f9. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/187?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
Summary
SELECTrelation-site recognizerFROM, qualified prefixes, aliases, joins, same-depth commas, nested queries, and dialect-ownedNATURALjoinsUSING(identifier [, identifier ...])grammar before crossing a join constraintONuntil a parser-backed or separately specified expression recognizer can prove its boundaryUSINGclause exitsSafety and performance
USINGcolumns: about 0.21 ms meanUSINGconstraintVerification
01cd8d29bf99a7c07f6a72ca746da328847c6dfbPart of #169.