Tracking: 2026-07 multi-perspective deep review findings (performance, generator, tests, tooling)

[DJGosnell]

Master tracking issue for the 2026-07-07 multi-agent deep review of the entire Quarry system (runtime, generated code, source generator, tests, benchmarks, analyzers, migration tooling, CLI). Eight reviewer agents covered fundamentally different perspectives; every critical/high finding was independently adversarially verified (attempted refutation with code evidence; several reproduced end-to-end against the built generator or the published nupkg).

Review verdict summary

Perspective	Grade	Perspective	Grade
Runtime hot-path performance	A-	Runtime API & public surface	B+
Generated code quality	B+	Test suite	B+
Benchmark methodology	B+	Generator architecture	B-
Generator warm-build performance	C+	Tooling (analyzers/convert/CLI)	C-

Headline: the runtime hot path genuinely earns the Dapper-competitive claim (one carrier allocation per chain, static SQL, typed-ordinal readers, [UnsafeAccessor] extraction — verified against real generated output). The critical defects cluster in the conditional-mask model (the one place the fail-loud philosophy breaks), the generator's incremental-caching seams, and the tooling periphery.

Reviewer note: the "stale generated entities on schema edit at design time" finding was assessed as not an issue for this project — Quarry is operated as a build-time generator (per-edit regeneration would be too slow on large codebases). The underlying schema-parse structure work is folded into #309 as build-time efficiency.

Tracked issues

Correctness (fix first)

• Conditional clause mask model: unconditional Limit/Offset/Distinct + runtime crash on else-if / multi-clause branches #307 — Conditional clause mask model (critical ×2, verified w/ repro): conditional Limit/Offset/Distinct silently applied unconditionally (silent wrong results); else-if chains / multi-clause branches dispatch null! SQL at runtime with no compile-time signal. ChainAnalyzer / SqlAssembler / TransitionBodyEmitter / CarrierEmitter.
• Runtime hot-path fixes: IN-cache hash collision, per-row NavigationList allocation, OpId gating, ConfigureAwait, RawSql mapper caching #308 — Runtime hot-path fixes (high ×2 verified + nits): collidable IN-list SQL cache hash (wrong-SQL reuse reachable — lengths (16,900)/(85,41) collide), per-row NavigationList allocation on entity fetch, unconditional insert OpId, missing ConfigureAwait(false) on raw-SQL paths, per-row mapper instantiation in RawSql readers.
• Display-class prediction robustness: stale UnsafeAccessorType names on cross-partial ordinal shifts; generic containing types/methods unguarded (runtime TypeLoadException) #310 — Display-class prediction robustness (high ×2, verified w/ TypeLoadException repro): stale [UnsafeAccessorType] names after cross-partial ordinal shifts (in-place mutation + equality exclusion); no guard for generic containing types/methods.
• Diagnostic pipeline reliability: bind-stage QRY900 drain-discarded, deferred-diagnostic registry silently drops unregistered IDs (3rd occurrence), ThreadStatic trace/cancellation hazards #311 — Diagnostic pipeline reliability (high ×2, verified): bind-stage QRY900 errors drained-and-discarded; deferred-diagnostic registry silently drops unregistered IDs (third occurrence — make the miss path fail loud); ThreadStatic trace/cancellation hazards.
• Migration model duplication drift: diverged builders + SnapshotCompiler whitelist silently degrade migrate add/diff to an empty baseline #313 — Migration model duplication drift (high, verified — live bug): diverged builder APIs + SnapshotCompiler whitelist silently degrade migrate add/diff to diffing against an empty baseline when snapshots contain column defaults/collation/charset.

Performance (the paramount goal)

• Generator warm-build performance: incremental caching seams defeat per-edit granularity (Stage 2.5/emission CompilationProvider combines, schema-parse walks, discovery predicate, Collect barrier) #309 — Generator warm-build performance (1 critical-class + 3 high, verified): Stage 2.5 and emission Combine(CompilationProvider) defeat caching (full supplemental-compilation rebuild + dataflow analysis every run), O(entities × trees) schema-parse walks, over-broad discovery predicate, Stage 4 Collect barrier vs provably file-local chains, syntax-node retention; no incrementality tests/benchmarks exist to catch any of it.
• Benchmark methodology: throughput bypasses the carrier path (stale QRY032 claim), cold-start measures warm caches, PreparedQuery/streaming/large-result/concurrency unmeasured #315 — Benchmark methodology (high ×2 verified): ThroughputBenchmarks bypasses the carrier path on a stale QRY032 justification; ColdStartBenchmarks measures warm static caches while docs cite it as cold-start evidence; PreparedQuery / ToAsyncEnumerable / large result sets / parallel throughput / incremental generator rebuild unmeasured.

API & feature gaps

• Transaction support: no BeginTransaction API, DbCommand.Transaction never assigned, dead defaultIsolation parameter #312 — Transaction support (high, verified): no BeginTransaction API, DbCommand.Transaction never assigned (chain queries incompatible with explicit transactions on SqlClient/MySqlConnector), dead defaultIsolation constructor parameter.
• Runtime API polish: inconsistent exception wrapping, dead TypeMappingRegistry, Quarry.Index collision, joined-builder GroupBy/Having gap, silent unloaded NavigationList, doc drift #319 — Runtime API polish (medium/low bundle): inconsistent exception wrapping (SELECT execution errors bypass QuarryQueryException), dead exception types and TypeMappingRegistry, Quarry.Index/System.Index collision, joined builders missing GroupBy/Having/SingleOrDefault, silent-empty unloaded NavigationList enumeration, IntelliSense/doc examples teaching nonexistent API shapes.

Guardrails

• Test suite guardrails: hollow incremental-caching tests, no perf regression gate, concurrency insurance, row-order flakes, manifest goldens, streaming/cancellation #314 — Test suite guardrails (high ×2 verified): IncrementalCachingTests hollow on three axes (anonymous-type fixtures → QRY032 path; same-Compilation re-run; vacuous assertions); no perf regression gate anywhere (post-merge dashboard only, no thresholds); concurrency insurance suite; 526 latent row-order flakes; manifest goldens unenforced; streaming/cancellation untested.

Tooling

• QRA analyzers & code fixes: CodeFixes nupkg packs to lib/ (fixes dead for consumers), QRA402 fires on every query, QRA201 fix breaks code, dead rules, analyzer perf #316 — QRA analyzers & code fixes (high ×3 verified + 1 to validate): published CodeFixes nupkg packs to lib/ (all QRA fixes dead for consumers), QRA402 fires up to 3× on every solitary query with O(N²) re-discovery, QRA201 fix produces non-compiling code and changes join semantics, four rules string-match SQL shapes their input can never produce, analyzer perf discipline.
• QRM conversion & CLI safety: nonexistent Sql.Like, Sql.Raw placeholder corruption, hardcoded db. receiver, destructive migrate remove/squash ordering #317 — QRM conversion & CLI safety (high ×2 verified + 1 to validate): nonexistent Sql.Like API (LIKE conversion never compiles), Sql.Raw placeholder text silently corrupts queries via applied code fixes, hardcoded db. receiver, destructive migrate remove/squash ordering, convert CLI supports only dapper while docs advertise four.

Maintainability

• Generator IR maintainability: RawCallSite copy-method divergence drops CTE metadata (bug), AnalyzeChainGroup god-method, decentralized dialect logic with permissive defaults #318 — Generator IR refactors (contains one bug): RawCallSite copy-method divergence silently drops CTE metadata (fix first); ~50-param god-object; 1,170-line AnalyzeChainGroup; dialect logic across ~50 sites with permissive switch defaults. Sequence after Conditional clause mask model: unconditional Limit/Offset/Distinct + runtime crash on else-if / multi-clause branches #307 (both churn ChainAnalyzer).

Suggested ordering

1. Conditional clause mask model: unconditional Limit/Offset/Distinct + runtime crash on else-if / multi-clause branches #307 (silent wrong results / crashes on documented flagship pattern) and the two quick wins in Runtime hot-path fixes: IN-cache hash collision, per-row NavigationList allocation, OpId gating, ConfigureAwait, RawSql mapper caching #308 (IN-cache exact-length check, NavigationList singleton).
2. Migration model duplication drift: diverged builders + SnapshotCompiler whitelist silently degrade migrate add/diff to an empty baseline #313 immediate CLI bug fix + Diagnostic pipeline reliability: bind-stage QRY900 drain-discarded, deferred-diagnostic registry silently drops unregistered IDs (3rd occurrence), ThreadStatic trace/cancellation hazards #311 registry fail-loud (small changes, close silent-failure classes).
3. Test suite guardrails: hollow incremental-caching tests, no perf regression gate, concurrency insurance, row-order flakes, manifest goldens, streaming/cancellation #314 incremental-caching test rewrite + perf gate — the guardrails that catch regressions from everything else.
4. Transaction support: no BeginTransaction API, DbCommand.Transaction never assigned, dead defaultIsolation parameter #312 transactions (adoption blocker), Display-class prediction robustness: stale UnsafeAccessorType names on cross-partial ordinal shifts; generic containing types/methods unguarded (runtime TypeLoadException) #310, Generator warm-build performance: incremental caching seams defeat per-edit granularity (Stage 2.5/emission CompilationProvider combines, schema-parse walks, discovery predicate, Collect barrier) #309.
5. QRA analyzers & code fixes: CodeFixes nupkg packs to lib/ (fixes dead for consumers), QRA402 fires on every query, QRA201 fix breaks code, dead rules, analyzer perf #316/QRM conversion & CLI safety: nonexistent Sql.Like, Sql.Raw placeholder corruption, hardcoded db. receiver, destructive migrate remove/squash ordering #317 tooling pass, Benchmark methodology: throughput bypasses the carrier path (stale QRY032 claim), cold-start measures warm caches, PreparedQuery/streaming/large-result/concurrency unmeasured #315 benchmark corrections, Generator IR maintainability: RawCallSite copy-method divergence drops CTE metadata (bug), AnalyzeChainGroup god-method, decentralized dialect logic with permissive defaults #318 refactors, Runtime API polish: inconsistent exception wrapping, dead TypeMappingRegistry, Quarry.Index collision, joined-builder GroupBy/Having gap, silent unloaded NavigationList, doc drift #319 polish.

Cross-cutting themes to keep in mind

1. Recurring structural patterns generated repeated bugs: the stringly-typed deferred-diagnostic registry (3 occurrences), hand-maintained copy methods (CTE drop), duplicated migration models (live CLI bug), permissive dialect switch defaults. Prefer fixes that kill the class (fail-loud miss paths, records/with, single-source models) over instance fixes.
2. Mutable state excluded from equality on cached pipeline objects appears in three issues (Generator warm-build performance: incremental caching seams defeat per-edit granularity (Stage 2.5/emission CompilationProvider combines, schema-parse walks, discovery predicate, Collect barrier) #309, Display-class prediction robustness: stale UnsafeAccessorType names on cross-partial ordinal shifts; generic containing types/methods unguarded (runtime TypeLoadException) #310, Diagnostic pipeline reliability: bind-stage QRY900 drain-discarded, deferred-diagnostic registry silently drops unregistered IDs (3rd occurrence), ThreadStatic trace/cancellation hazards #311) — one modeling convention fixes all three.
3. The fail-loud philosophy holds everywhere except Conditional clause mask model: unconditional Limit/Offset/Distinct + runtime crash on else-if / multi-clause branches #307 — restoring it there is the highest-leverage correctness work in the repo.