Skip to content

QRM conversion & CLI safety: nonexistent Sql.Like, Sql.Raw placeholder corruption, hardcoded db. receiver, destructive migrate remove/squash ordering #317

Description

@DJGosnell

Combined finding from the 2026-07-07 multi-agent deep review (tooling perspective). Items 1–2 were adversarially verified; item 3 is detailed but was not independently re-verified (verification capped per reviewer). All touch src/Quarry.Migration (ChainEmitter + code fixes) and src/Quarry.Tool. Theme: the conversion path can corrupt user code — a working Dapper/ADO.NET/SqlKata call is replaced by output that either doesn't compile or compiles and sends broken SQL.

1 (high, CONFIRMED): Conversion emits a nonexistent Sql.Like API — converted LIKE never compiles

  • ChainEmitter.EmitBinary (ChainEmitter.cs:399-404) unconditionally emits Sql.Like({left}, {right}) for every SQL LIKE. The only Sql class in the repo (src/Quarry/Query/Sql.cs) has no Like member (static class — not extensible), so the output is guaranteed CS0117. Quarry's actual LIKE surface is string.Contains/StartsWith/EndsWith.
  • All three migration paths are affected (Dapper QRM001/002, ADO.NET QRM021/022, SqlKata QRM031/032 — all reuse ChainEmitter). ChainEmitterTests.cs:538/595 assert the broken string (tests never compile emitted code), and Samples/4_DapperMigration/DapperQueries.cs:95 advertises it. Notably the older SqlToChainConverter in Quarry.Analyzers explicitly rejects LIKE — the correct handling was known.
  • Fix: map LIKE patterns to Contains/StartsWith/EndsWith via wildcard analysis of literal patterns; non-literal patterns → Sql.Raw with the actual predicate text or suggestion-only.

2 (high, CONFIRMED): Sql.Raw fallback replaces CASE/CAST/EXISTS with literal placeholder text — silent query corruption via applied code fix

  • EmitRawFallbackExtractSourceText (ChainEmitter.cs:473-484) returns placeholders — "CASE ... END", "CAST(... AS type)", "EXISTS (...)", "{func}(...)" — not the original SQL. The emitted Sql.Raw<bool>("CASE ... END") compiles (the generator renders raw text verbatim, SqlExprRenderer.cs:209-211) and sends the literal placeholder to the database: syntax error or wrong predicate, discovered only at execution.
  • The path is reachable (parser produces those nodes; EmitExpression routes them to the fallback) and routed to fixable QRM002 with a BatchFixer FixAllProvider — the only guards are ChainCode == null and IsSuggestionOnly (set solely for INSERT), so mass unreviewed conversion is realistic. The parallel ADO.NET code fix has the same gap.
  • Fix: thread source spans through SqlNode so the fallback quotes the actual expression text; until then, mark any statement requiring the raw fallback as IsSuggestionOnly = true so the fix never applies it (the INSERT guard shows the pattern).

3 (high, not independently re-verified — validate first): All converters hardcode a db. receiver

  • ChainEmitter (lines 72/136/168), EfCoreConverter.cs:77, SqlKataConverter.cs:77 emit chains rooted at literal db.. Applied fixes splice this into methods whose receiver is a Dapper IDbConnection connection or EF _context — a db local almost never exists → CS0103 on every applied fix. Not documented as a placeholder.
  • Fix: locate an in-scope QuarryContext-typed variable/field via the semantic model and substitute its name; when none exists, make the fix suggestion-only with a TODO comment.
  • Cross-cutting guard for 1–3: add a compile-verification test harness that feeds converter output through a real Quarry compilation — none of these could have shipped with one.

4 (medium): QRM analyzer performance — schema resolution before the cheap gate

  • Each of the four QRM analyzers independently runs SchemaResolver.Resolve, which calls GetDeclaredSymbol on EVERY class declaration in the compilation (SchemaResolver.cs:20-37) — four full symbol walks — before the cheap GetTypeByMetadataName ORM-presence check (DapperMigrationAnalyzer.cs:27-37). A project with zero Dapper/EF/SqlKata references still pays all four. AdoNetMigrationAnalyzer's gate is System.Data.Common.DbCommand (:33-36) — present in effectively every compilation, so it never no-ops.
  • Fix: reorder gates (ORM type check first); share one schema resolution across the four analyzers; give the ADO.NET analyzer a meaningful gate (e.g. require Quarry schemas + textual CommandText/ExecuteReader candidates before semantic work).

5 (medium): CLI hazards

  • quarry migrate remove recursively deletes the newest migration directory — including the hand-written user partial with Before/AfterUpgrade hooks — with no confirmation and no unapplied-in-DB check despite the doc comment saying "unapplied" (MigrateCommands.cs:241-299).
  • quarry migrate squash deletes all old migration directories before generating and writing the new baseline (:644-668 vs :677); a generation/write failure leaves the project with no migrations. --ni skips the confirmation, combining both hazards in CI.
  • quarry convert supports only --from dapper (ConvertCommand.cs:14) while llm.md (lines 15, 313) advertises all four sources; the CLI is also report-only while docs say "emits equivalent chain API code".
  • Fix: write-then-delete ordering for squash; confirmation + unapplied check (or --force) for remove; preserve user partials (move to backup); either wire up the existing EfCore/AdoNet/SqlKata converters in the CLI or fix the docs.

Low (bundled)

  • Scaffold "Accept all ≥80%" applies the threshold only to the current candidate; later low-confidence FKs are auto-accepted.
  • CLI option parser cannot accept values beginning with -; NU1903 vulnerability warnings suppressed.
  • Quarry.Migration references Microsoft.CodeAnalysis.Workspaces with RS1038 suppressed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions