Skip to content

Runtime API polish: inconsistent exception wrapping, dead TypeMappingRegistry, Quarry.Index collision, joined-builder GroupBy/Having gap, silent unloaded NavigationList, doc drift #319

Description

@DJGosnell

Combined finding from the 2026-07-07 multi-agent deep review (runtime-API perspective, medium/low severity bundle). Individually small; grouped as one polish pass over src/Quarry public surface + docs. The review's overall verdict on the API was strongly positive (zero-cost schema DSL, fail-loud stubs, institutional-memory comments) — these are the residue items.

1: Exception taxonomy is partially dead and inconsistently applied

  • QuarryConnectionException and QuarryMappingException are defined and unit-tested but never thrown by any runtime or generator code; connection-open failures propagate raw from DbConnection.OpenAsync (QuarryContext.cs:119).
  • Inconsistent wrapping inside QueryExecutor: non-query (:270-283) and scalar (:217-253) paths wrap failures in QuarryQueryException with SQL attached, but on every fetch path (:32, 65, 97, 131, 172, 301) ExecuteReaderAsync sits outside the try — only row-materialization errors are wrapped. A SELECT execution error surfaces as a raw provider exception while the same error in an UPDATE surfaces as QuarryQueryException; catch (QuarryQueryException e) { log(e.Sql); } per the docs silently misses all SELECT failures.
  • Fix: wrap fetch-path ExecuteReaderAsync consistently; either wire up or delete the two dead exception types; update llm.md's exception section to match reality.

2: TypeMappingRegistry is dead code from the removed runtime-fallback path

  • Class doc references NormalizeParameterValue (does not exist anywhere in src) and "the fallback execution path (QRY001 queries)" — the carrier-only architecture has no fallback. TryConvert/TryConfigureParameter are called only by tests. Meanwhile TypeMapping<TCustom,TDb>'s constructor (TypeMapping.cs:30) performs a static ConcurrentDictionary TryAdd on every instantiation — including every generated-carrier mapper — populating a registry nothing reads.
  • Fix: delete the registry + constructor side effect (small perf win), or document a real purpose.

3: Unloaded NavigationList is silently empty under enumeration but throws on indexer

  • GetEnumerator() returns empty and Count returns 0 for unloaded navigations, while the indexer throws (NavigationList.cs:25, 33-41, 76-79). foreach (var order in user.Orders) on an entity fetched without a join silently yields nothing — indistinguishable from "no orders", and exactly the mistake EF migrants make (no lazy loading). The one place in the library where misuse produces plausible wrong data instead of an exception.
  • Fix: make enumeration/Count of an unloaded list throw the same InvalidOperationException as the indexer (consistent fail-loud), or provide an explicit .OrEmpty() escape. Coordinate with the NavigationList singleton change in the hot-path issue (both touch this class).

4: Quarry.Index collides with System.Index

  • public Index IX_Email => Index(Email).Unique(); is ambiguous (CS0104) with implicit using System; — the project's own sample needs using Index = Quarry.Index; (UserSchema.cs:2). Guaranteed first-day compile error for any user declaring an index, with a non-obvious fix.
  • Fix: rename the type (e.g. TableIndex/SchemaIndex) with [Obsolete] alias for a release, or document the alias prominently in the schema article and analyzer-assist it.

5: Joined builders missing GroupBy/Having and ExecuteFetchSingleOrDefaultAsync

  • Grep over all arities of IJoinedQueryBuilder.g.cs (2–6, projected and unprojected): zero occurrences of GroupBy/Having/SingleOrDefault. The common report shape "JOIN then GROUP BY a joined column" is inexpressible in the typed API — users must fall back to RawSqlAsync or CTEs, and nothing in the docs flags the gap. The missing SingleOrDefault terminal breaks terminal-set uniformity when refactoring a single-table chain into a join.
  • Fix: extend the T4 templates + generator support for joined GroupBy/Having/SingleOrDefault (feature work — size it), or document the limitation explicitly with the CTE workaround.

6: Shipped docs teach API shapes that do not exist

  • QuarryContextAttribute's <example> (IntelliSense on the first attribute every user applies) shows public partial QueryBuilder<User> Users { get; } — wrong type AND wrong member kind (property vs method), the exact top footgun llm.md warns about; <remarks> repeats it (QuarryContextAttribute.cs:10, 22).
  • Root llm.md:49 and src/Quarry.Generator/README.md:278 document parameterless HasOne<UserSchema>() — only HasOne<T>(string foreignKeyPropertyName) exists (Schema.cs:174); the documented snippet is a compile error.
  • Fix: correct both; add a docs-snippet compile test if cheap (the manifest/testing infra could compile doc snippets).

Low (bundled)

  • Use-after-dispose undetected: a disposed context silently reopens its connection — add an ObjectDisposedException check.
  • Sub-second defaultTimeout truncates to CommandTimeout = 0 (infinite) — round up or throw.
  • SqlDialectFactory is vestigial public surface (identity function + enum-mirroring properties) — obsolete/remove.
  • XML doc hygiene: shipped docs reference internal session files and unresolvable crefs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    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