fix: bind the caller's filter as a single conjunct, and derive keys honestly

CHANGELOG.md

@@ -26,6 +26,28 @@ some query behavior changes (see Breaking changes).
 
 ### Fixed
 
+- **Critical: an ungrouped `Or()` escaped the partition and type predicates.** The caller's
+  filter was appended to the generated `WHERE` clause without being bound as a unit:
+
+  ```sql
+  WHERE FullTypeName = ? AND Partition = ? AND <term1> OR <term2>
+  ```
+
+  `AND` binds tighter than `OR`, so SQL read that as
+  `(FullTypeName = ? AND Partition = ? AND term1) OR (term2)` — every term after the first
+  `Or()` was matched against **the whole table**. A two-term `Or()` returned rows from other
+  partitions, and rows of *other stored types*, which the reader then deserialized as `T` with
+  no error. The same clause is used by `DeleteObjectsAsync`, so an ungrouped `Or()` could
+  **delete rows in other partitions and of other types**, and by `CountObjectsAsync`, which
+  over-counted. The caller's filter is now emitted inside its own parentheses. Losing the
+  `Partition` predicate also cost the partition-prefixed indexes, so this was a large
+  performance regression as well as a correctness one; grouped OR-chains now use the index.
+  Filters already wrapped in `StartGroup()`/`EndGroup()` were unaffected and still are.
+- **LINQ predicates lost their own precedence.** `TychoQueryable` translated `&&` and `||` by
+  emitting their operands flat, so `Where(x => (x.A || x.B) && x.C)` became `A OR B AND C` —
+  read by SQL as `A OR (B AND C)` — and returned rows matching only `A` despite their failing
+  `C`. Each composite boolean node is now emitted in its own group. (`.Where(a).Where(b)`
+  chains were affected the same way when either predicate contained an `||`.)
 - **Data integrity: filter values are now compared in the form the serializer wrote.**
   A filter value was rendered with `ToString()`, which is not how the serializer stores it for
   every type. The clearest case is an enum: both serializers write it as a **number** by
@@ -119,6 +141,15 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4
 
 ### Performance
 
+- **`CountObjectsAsync` no longer counts rows on the client.** It issued
+  `SELECT 1 FROM JsonValue WHERE …` and incremented a counter once per matching row, costing a
+  reader round trip per row. It now issues `SELECT COUNT(*)` and reads the single scalar:
+  **16.0 ms → 6.5 ms** counting a 250,000-row partition (2.5x). The same query backs the
+  pre-count a progress-reporting `ReadObjectsAsync` performs, so progress-enabled reads pay
+  half of what they did. A *filtered* count is still bounded by whether the filtered property
+  is indexed — counting a 1-in-200 selective filter on an unindexed path takes ~79 ms on the
+  same store, essentially all of it the `JSON_EXTRACT` scan.
+
 - **`PRAGMA optimize` on connect and disconnect.** `Connect`/`ConnectAsync` and
   `Disconnect`/`DisconnectAsync`/`Dispose` run SQLite's recommended `PRAGMA optimize`
   (bounded by `analysis_limit = 400`) so the query planner keeps fresh statistics and
@@ -155,6 +186,78 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4
 
 ### Added
 
+- **`AddTypeRegistration<T>()` now detects the id property by convention, as documented.** It
+  previously did nothing of the kind: it recorded no selector, so `WriteObjectAsync(obj)`,
+  `ReadObjectAsync(obj)`, `ObjectExistsAsync(obj)`, `DeleteObjectAsync(obj)` and `GetIdFor(obj)`
+  all threw `TychoException: An id mapping has not been provided`, on a type whose property was
+  literally named `Id`. The property is now found by name — `Id`, then `<TypeName>Id`, matched
+  case-insensitively, and it must be public, readable and non-indexed. When no such property
+  exists the type is still registered but without an id mapping, exactly as before, so
+  registering a key-less type and supplying keys at the call site keeps working.
+- **Strict registration now rejects a key that diverges from the registered id property.**
+  `WriteObjectsAsync(objs, keySelector, …)` takes a key at the call site and overrides the
+  registration. A row written under a key the registration would not produce is unreachable by
+  every by-object overload, and the delete failure is silent — `DeleteObjectAsync(obj)` returns
+  false while the row survives. With `requireTypeRegistration: true` and a type registered by id
+  property, such a write now throws `TychoException` naming both keys. Delegate registrations
+  (`AddTypeRegistrationWithCustomKeySelector`) have no property to compare against and are
+  unaffected, as is everything outside strict mode. The check wraps the selector rather than
+  pre-scanning, so a lazy sequence is still enumerated exactly once.
+- **Filters on the id property are answered from the `Key` column where that is provably
+  correct.** `Filter(Equals, x => x.Id, …)` and `Filter(In, x => x.Id, …)` previously went
+  through `JSON_EXTRACT` and scanned. Under `requireTypeRegistration` with a type registered by
+  id property, they are now emitted against the indexed `Key` column instead:
+
+  | filter on the id property, 250,000 rows | scan | rewritten |
+  |---|---:|---:|
+  | `Equals` | 79.3 ms | **0.0 ms** |
+  | `In`, 100 keys | 101.2 ms | **0.2 ms** |
+
+  Soundness comes from two things together: the write guard above means no row written through
+  this instance can diverge, and rows already in the database are checked once per type with a
+  divergence probe before the rewrite is used (~92 ms on that store, on the first such query
+  only, then cached for the connection). A single divergent row disables the rewrite for that
+  type and the ordinary predicate is emitted, so the worst case is the behaviour that was there
+  before. Negated forms (`NotEquals`, `NotIn`) are deliberately left alone — they cannot use an
+  index either way — as is a null comparison value, since `Key` is `NOT NULL`.
+- **`ReadObjectsByKeysAsync<T>(keys, partition, sort, …)`.** Reads a batch of keys in one round
+  trip. The key set is bound as a **single JSON array** expanded by `JSON_EACH`, not as one
+  parameter per key, so there is no `SQLITE_MAX_VARIABLE_NUMBER` ceiling (999 on older SQLite
+  builds), no chunking for callers to think about, and one prepared statement regardless of
+  batch size. Keys lead the primary key, so each is a primary-key probe. Measured against a
+  loop of `ReadObjectAsync` on a 250,000-row store (best of five, after warm-up):
+
+  | batch | looped `ReadObjectAsync` | `ReadObjectsByKeysAsync` |
+  |------:|------------------------:|-------------------------:|
+  |   200 |                  1.9 ms |                   0.9 ms |
+  |   999 |                 10.6 ms |                   4.5 ms |
+  | 4,949 |                 36.8 ms |                  16.9 ms |
+  |23,784 |                183.2 ms |                  67.3 ms |
+
+  That is 2.1–2.7x end to end. Both figures include deserialization, which is identical between
+  them and dominates what is left — the query alone is 27.5 ms at 23,784 keys. The `JSON_EACH` shape was chosen by
+  measurement: a single `IN (@p0…@pN)` collapses at scale (1,297.7 ms at 23,784 keys, because
+  the statement text and plan grow with the batch), a chunked `IN` is 91.8 ms, and a temp-table
+  join carries ~40 ms of fixed setup. Keys not present are simply absent from the result.
+- **`FilterType.In` and `FilterType.NotIn`.** Set membership as a single atomic term, via new
+  `Filter` overloads taking an `IEnumerable`:
+
+  ```csharp
+  FilterBuilder<Item>.Create().Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47 });
+  ```
+
+  It renders to `<path> IN (…)` through the same numeric `CAST` the scalar comparisons use, so
+  an expression index over the property still serves the query. Being one term, it cannot be
+  mis-grouped the way an `Or()` chain can. Details:
+  - Duplicate values are removed; the caller's order is preserved.
+  - An empty set matches nothing for `In` and everything for `NotIn` — never `IN ()`, which is
+    a syntax error, and never a silently dropped term, which would widen the result set.
+  - A `null` in the set is matched against a missing or null member with `IS NULL`, which SQL's
+    own `IN` would never do. `NotIn` keeps SQL's semantics for rows whose member is null: they
+    are not returned, exactly as `NotEquals` already behaves.
+  - Longer lists are split across several `IN` terms rather than exceeding
+    `SQLITE_MAX_VARIABLE_NUMBER`, which is only 999 on older SQLite builds, so a large set works
+    regardless of which build the host application ships.
 - **`IJsonValueResolver`.** A second optional serializer capability, feature-detected the same
   way, reporting the scalar form a CLR value takes in JSON so filter comparisons are made
   against what was stored. Implemented by `SystemTextJsonSerializer` and
@@ -169,6 +272,21 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4
 
 ### Breaking changes
 
+- **`AddTypeRegistration<T>()` on a type with a conventional id property now supplies a key.**
+  Previously every by-object operation on such a type threw; they now work. Code that caught
+  that exception, or that relied on `WriteObjectsAsync(objs, keySelector)` disagreeing with a
+  conventionally-named `Id` property, changes behaviour — under `requireTypeRegistration` the
+  disagreement is now an error rather than a silently unreachable row.
+
+- **An ungrouped `Or()` now means what it reads as.** Code that (unknowingly) depended on the
+  leaked rows — most plausibly a query written against a single-partition, single-type database
+  where the bug was invisible — returns fewer rows now. This is the fix, not a regression.
+- **Passing a collection to a scalar `FilterType` now throws `ArgumentException`.** Adding the
+  `IEnumerable` overloads changes overload resolution for a collection argument, which
+  previously bound to `object` and was rendered as `ToString()` (`"System.Int32[]"`), matching
+  nothing silently. Use `FilterType.In`. A literal `null` argument also now binds to the new
+  overload, but keeps its old meaning — `Filter(Equals, x => x.Value, null)` is still the
+  null comparison.
 - **Enum, `DateOnly` and `TimeOnly` filter values now compare against their JSON form.** Code
   that worked around the enum mismatch by casting to `(int)` keeps working. Code that relied on
   a string-enum converter's name matching by coincidence also keeps working, and now stays
@@ -215,6 +333,16 @@ index); batch writes **−44%**; database file with three indexes **20.2 → 8.4
 
 ### Notes
 
+- **Serializer choice is the largest remaining lever on read throughput.** Reading a whole
+  250,000-row partition measured 254.7 ms with `SystemTextJsonSerializer` against 358.9 ms with
+  `NewtonsoftJsonSerializer` (~1.4x), because the former implements `IUtf8JsonDeserializer` and
+  receives rows as UTF-8 spans. Deserialization dominates any large read: of the 67.3 ms
+  `ReadObjectsByKeysAsync` takes for 23,784 keys, only 27.5 ms is the query.
+- **Outside strict mode, a filter on the id property still scans.** The `Key`-column rewrite
+  needs the write guard to hold, and that guard only applies under `requireTypeRegistration`.
+  Without it, index the id property or reach those rows through `ReadObjectsByKeysAsync`; both
+  measured 0.0 ms against the 71.6 ms scan.
+
 - Performance guidance: prefer `WriteObjectsAsync` for writing many objects — it is
   ~10× faster and ~6× lower-allocation than looping `WriteObjectAsync`, and
   `withTransaction: true` is faster than `false` for bulk writes.

README.md

@@ -190,8 +190,20 @@ var exists = await db.ObjectExistsAsync<Person>("123");
 
 // Count objects
 var count = await db.CountObjectsAsync<Person>();
+
+// Read many objects by key in one round trip. Prefer this over a loop of ReadObjectAsync:
+// keys lead the primary key, and there is no limit on how many may be passed.
+var people = await db.ReadObjectsByKeysAsync<Person>(new object[] { "id-1", "id-2", "id-3" });
 ```
 
+> **Filtering on the property that is also the Tycho key** (`x => x.Id`) normally goes through
+> `JSON_EXTRACT` and scans, because a write may supply its own key selector and Tycho cannot
+> assume the property still matches the stored key. Under `requireTypeRegistration: true`, with
+> the type registered by id property, that assumption *is* enforced, and `Equals` / `In` filters
+> on the id property are answered from the indexed `Key` column instead — 79.3 ms to 0.0 ms on a
+> 250,000-row store. Otherwise, reach those rows through `ReadObjectAsync` /
+> `ReadObjectsByKeysAsync`, or index the property like any other.
+
 ### Filtering
 
 ```csharp
@@ -210,6 +222,22 @@ var complexFilter = FilterBuilder<Person>
     .And()
     .Filter(FilterType.Contains, x => x.Name, "Doe");
 
+// Set membership: one atomic term, so it needs no grouping
+var inDepartments = FilterBuilder<Person>
+    .Create()
+    .Filter(FilterType.In, x => x.DepartmentId, new[] { 33, 47, 51 });
+
+// Mixing OR with other terms: group the alternatives so the intent is explicit
+var grouped = FilterBuilder<Person>
+    .Create()
+    .StartGroup()
+        .Filter(FilterType.Equals, x => x.DepartmentId, 33)
+        .Or()
+        .Filter(FilterType.Equals, x => x.DepartmentId, 47)
+    .EndGroup()
+    .And()
+    .Filter(FilterType.GreaterThan, x => x.Age, 25);
+
 // Get a single object matching the filter
 var johnDoe = await db.ReadObjectAsync<Person>(filter: complexFilter);
 
@@ -323,6 +351,19 @@ var result = await db.DeleteBlobsAsync("documents");
 Console.WriteLine($"Deleted {result.Count} blobs");
 ```
 
+## Performance notes
+
+- **Reach rows by key through the key APIs.** A filter on the key property scans; see the note
+  under [Basic Querying](#basic-querying). `ReadObjectsByKeysAsync` fetches a whole batch in one
+  round trip and has no limit on batch size.
+- **`SystemTextJsonSerializer` deserializes faster.** It implements `IUtf8JsonDeserializer`, so
+  rows are handed to it as UTF-8 spans and skip an intermediate stream. Reading a whole
+  250,000-row partition measured **254.7 ms** with `SystemTextJsonSerializer` against
+  **358.9 ms** with `NewtonsoftJsonSerializer` — about 1.4x. Deserialization dominates any large
+  read, so this is usually the largest single lever on read throughput.
+- **Index anything you filter or sort on.** An unindexed `JSON_EXTRACT` predicate scans the
+  partition; see below.
+
 ## Indexing
 
 Create indexes to improve query performance:

TychoDB.Benchmarks/Diagnostics.cs

@@ -216,7 +216,7 @@ private static (string Sql, FilterParameters Parameters) BuildSortQuery(Action<S
         configure(sort);
 
         var sb = new StringBuilder(BuildBaseQuery());
-        sort.Build(sb);
+        sort.Build(sb, BuildSerializer());
         sb.AppendLine().AppendLine(Queries.Limit(top));
         return (sb.ToString(), new FilterParameters());
     }