Apply /review-areas -all feedback to AGENTS.md docs

Author: ajcvickers

.claude/agents/builders-reviewer.md

@@ -13,7 +13,7 @@ Read `src/MongoDB.Driver/AGENTS.md` (router) first; then root `AGENTS.md` for bu
 
 ## Review focus
 
-- `Render` correctness — every `*Definition<T>.Render(RenderArgs<T>)` must produce the documented BSON shape. Note that most types render to `BsonDocument`, but `UpdateDefinition<T>.Render` returns `BsonValue` (pipeline updates render as `BsonArray`); `ProjectionDefinition<TSource,TProjection>.Render` returns `RenderedProjectionDefinition<TProjection>` — a wrapper of `BsonDocument` plus the projection-result serializer; and `PipelineDefinition<TInput,TOutput>.Render` returns `RenderedPipelineDefinition<TOutput>` — a wrapper of `IReadOnlyList<BsonDocument>` plus the output serializer. Assert against the wrapped document/list, not the wrapper return value directly. Cast/inspect accordingly when asserting. Tests **must** assert rendered output, not just round-trip.
+- `Render` correctness — every `*Definition<T>.Render(RenderArgs<T>)` must produce the documented BSON shape. Note that most types render to `BsonDocument`, but `UpdateDefinition<T>.Render` returns `BsonValue` (pipeline updates render as `BsonArray`); `ProjectionDefinition<TSource,TProjection>.Render` returns `RenderedProjectionDefinition<TProjection>` — a wrapper of `BsonDocument` plus the projection-result serializer; and `PipelineDefinition<TInput,TOutput>.Render` returns `RenderedPipelineDefinition<TOutput>` — a wrapper of `IReadOnlyList<BsonDocument>` plus the output serializer. Assert against the wrapped document/list, not the wrapper return value directly. Cast/inspect accordingly when asserting. Tests **must** assert rendered BSON (or the wrapped `BsonDocument` / `IReadOnlyList<BsonDocument>` for wrapper return types), not just round-trip.
 - `RenderArgs<TDocument>` — passes the document serializer; using the wrong serializer silently emits wrong BSON.
 - Lambda overload ambiguity — `Expression<Func<T, …>>` overloads are easy to misroute. Type-parameter explicitness in tests reveals this.
 - Implicit conversions — most definition types accept `BsonDocument` and `string` (parsed as JSON — a frequent overload-ambiguity hazard). `Expression<Func<T,…>>` is implicit-convertible specifically on `FilterDefinition<T>`; other definitions accept LINQ expressions only via builder methods. Several non-obvious conversions also exist (`UpdateDefinition<T>` ← `PipelineDefinition<T,T>`; `ProjectionDefinition<TSource,TProjection>` ← `ProjectionDefinition<TSource>`; `FieldDefinition<TDocument>` ← `FieldDefinition<TDocument,TField>`; `PipelineDefinition<TInput,TOutput>` ← four list/array shapes) — see the implicit-conversion bullet list in `src/MongoDB.Driver/AGENTS.md` for the full table. Adding any new implicit conversion risks ambiguity at call sites.

.claude/agents/geojson-reviewer.md

@@ -18,7 +18,7 @@ Read `src/MongoDB.Driver/GeoJsonObjectModel/AGENTS.md` first; then root `AGENTS.
 - Antimeridian-crossing polygons require splitting or careful CRS — driver doesn't validate.
 - CRS specification — driver sets no client-side default (`CoordinateReferenceSystem` is `null` unless supplied); `2dsphere` assumes WGS84 server-side. Projected systems need explicit CRS via `GeoJsonObjectArgs`.
 - 2D vs 3D coordinate types are compile-time choices; 3D altitude is ignored by standard query operators.
-- Two parallel discriminator dispatchers — `GeoJsonObjectSerializer<T>` (any GeoJSON object) and `GeoJsonGeometrySerializer<T>` (geometries only). Adding a new geometry type needs all three of: (1) the new type and its dedicated serializer under `GeoJsonObjectModel/Serializers/`; (2) discriminator-`switch` entries in both `GeoJsonObjectSerializer<T>` and `GeoJsonGeometrySerializer<T>`; and (3) serializer registration for direct-typed call sites — the established pattern is a `[BsonSerializer(typeof(...))]` attribute on the type (matches every existing geometry); `BsonSerializer.RegisterSerializer(...)` at startup is a fallback for types you don't own. A PR that updates the dispatchers but omits step (3) will fail when callers hold the concrete type statically.
+- Two parallel discriminator dispatchers — `GeoJsonObjectSerializer<T>` (any GeoJSON object) and `GeoJsonGeometrySerializer<T>` (geometries only). Adding a new geometry type needs all three of: (1) the new type and its dedicated serializer under `GeoJsonObjectModel/Serializers/`; (2) discriminator-`switch` entries in both `GeoJsonObjectSerializer<T>` and `GeoJsonGeometrySerializer<T>`; and (3) serializer registration for direct-typed call sites — the established pattern is a `[BsonSerializer(typeof(...))]` attribute on the type (matches every existing geometry); `BsonSerializer.RegisterSerializer(...)` at startup is a fallback for types you don't own. A PR that updates the dispatchers but omits step (3) will produce wrong or incomplete serialization at concrete-typed call sites (the framework falls back to convention-based serialization, which may serialize silently and incorrectly rather than throw).
 - `bbox` is optional metadata; `2dsphere` computes its own S2 covering and ignores the user-supplied `bbox`, so an inaccurate `bbox` will not cause spatial-index misses (but may mislead other readers).
 - Integration with `FilterDefinitionBuilder.GeoIntersects` / `GeoWithin` / `Near` / `NearSphere` and `IndexKeysDefinitionBuilder.Geo2DSphere`.
 

.claude/agents/gridfs-reviewer.md

@@ -14,7 +14,7 @@ Read `src/MongoDB.Driver/GridFS/AGENTS.md` first; then root `AGENTS.md` for buil
 ## Review focus
 
 - Generic `TFileId` correctness — non-`ObjectId` IDs need a registered serializer; mismatched serializers silently corrupt `_id` on `.files`.
-- Index creation idempotence — first-write semaphore must remain single-shot per bucket lifetime.
+- Index creation idempotence — the single-shot guarantee is provided by the `_ensureIndexesDone` `bool` flag (the `SemaphoreSlim` only serializes the racing first callers); both must be preserved together when refactoring the path.
 - Stream disposal — upload/download streams returned to the caller require explicit `Dispose` to flush metadata / close cursors.
 - Chunk-size semantics — `ChunkSizeBytes` per-bucket vs per-upload override interaction.
 - Revision handling on the `DownloadAsBytesByName` / `DownloadToStreamByName` / `OpenDownloadStreamByName` family — `0` oldest, `-1` newest, off-by-one common.
@@ -25,7 +25,7 @@ Read `src/MongoDB.Driver/GridFS/AGENTS.md` first; then root `AGENTS.md` for buil
 ## Required checks before approving
 
 1. `dotnet test tests/MongoDB.Driver.Tests/MongoDB.Driver.Tests.csproj -f net10.0 --filter "FullyQualifiedName~GridFS"`.
-2. The JSON-driven GridFS operations under `tests/MongoDB.Driver.Tests/JsonDrivenTests/` and `tests/MongoDB.Driver.Tests/UnifiedTestOperations/` — these exercise transactions / retryable-read coverage for GridFS that the GridFS xUnit suite doesn't. When stream lifecycle or transaction behavior is in flight, a focused second pass with `--filter "FullyQualifiedName~JsonDrivenGridFs|FullyQualifiedName~UnifiedGridFs"` runs just those dispatchers.
+2. The JSON-driven GridFS dispatchers under `tests/MongoDB.Driver.Tests/JsonDrivenTests/` and `tests/MongoDB.Driver.Tests/UnifiedTestOperations/` are exercised **indirectly**, by the unified / JSON spec runners (transactions, retryable reads/writes) whose fixtures reference GridFS operations. They are not a standalone test class with a `FullyQualifiedName~JsonDrivenGridFs` filter that produces direct coverage — when stream lifecycle or transaction behavior is in flight, run the broader spec suites those dispatchers are wired into.
 3. For stream-related changes, dispose-path tests cover both happy and exception cases.
 
 ## Escalate to user (do not auto-approve) when

.claude/agents/search-reviewer.md

@@ -34,3 +34,4 @@ Read `src/MongoDB.Driver/Search/AGENTS.md` first; then root `AGENTS.md` for buil
 - Atlas Search env vars not available — tests cannot be exercised.
 - Index-helper API change (search/vector/auto-embedding).
 - BSON shape change for `$search` or `$vectorSearch`.
+- Auto-embedding flow change — anything affecting `IndexName`, `AutoEmbeddingModelName`, or `EmbeddedScoreMode` on `VectorSearchOptions<T>`, or how a `QueryVector(string text)` is dispatched against an auto-embedding index. These three fields are load-bearing public surface and silently affect query results.

src/MongoDB.Driver.Encryption/AGENTS.md

@@ -49,7 +49,7 @@ libmongocrypt asks managed code for AES / HMAC / random / RSA signing via callba
 - `CsfleSchemaBuilder` — fluent builder for **CSFLE** `$jsonSchema`-style schemas (composes with `AutoEncryptionOptions.SchemaMap`). The duplicate-namespace check is incidental: `Encrypt<T>(CollectionNamespace, Action<EncryptedCollectionBuilder<T>>)` calls `_schemas.Add(...)` on the underlying `Dictionary<string, BsonDocument>`, which throws `ArgumentException` on a duplicate key — there is no explicit validation step beyond that. **Not** the entry point for Queryable Encryption — QE schemas are configured via `AutoEncryptionOptions.EncryptedFieldsMap`.
 - `EncryptionAlgorithm` is a single flat enum — values are not partitioned in the type system, only by usage convention:
   - **CSFLE-only by convention** — `AEAD_AES_256_CBC_HMAC_SHA_512_Deterministic` (equality-queryable, same plaintext → same ciphertext), `AEAD_AES_256_CBC_HMAC_SHA_512_Random` (no queries possible).
-  - **QE-only by convention** — `Indexed` (equality with contention), `Range` (range queries), `TextPreview` (preview), `Unindexed`. Server-version availability (preview vs GA) for each algorithm is a server-side concern; consult the MongoDB server release notes rather than relying on driver-side enum metadata. The "Preview" suffix on `TextPreview` reflects the server's preview status — but `EncryptionAlgorithm` is a **public enum**, so the value itself is SemVer-covered: renaming or removing `TextPreview` (e.g. once the server feature GAs as `Text`) requires an `[Obsolete]` deprecation cycle, not an in-place rename.
+  - **QE-only by convention** — `Indexed` (equality with contention), `Range` (range queries), `TextPreview` (preview), `Unindexed`. Server-version availability (preview vs GA) for each algorithm is a server-side concern; consult the MongoDB server release notes rather than relying on driver-side enum metadata. The "Preview" suffix on `TextPreview` reflects the server's preview status — but `EncryptionAlgorithm` is a **public enum**, so the value itself is SemVer-covered: renaming or removing `TextPreview` (e.g. once the server feature GAs as `Text`) requires an `[Obsolete]` deprecation cycle, not an in-place rename. The migration shape is additive-then-deprecate: introduce a new `Text` enum member alongside `TextPreview`, mark `TextPreview` `[Obsolete]`, and only remove it in a later major version — never reuse the existing enum value, since the integer is part of the on-the-wire contract for any caller that has it baked in.
   Server-side enforcement decides which value is valid in a given context. Confusing CSFLE and QE algorithms is a recurring bug.
 
 ## Mongocryptd vs crypt_shared

src/MongoDB.Driver/AGENTS.md

@@ -43,7 +43,7 @@ User-facing facades, fluent APIs, builders, and option/result records. The Opera
 
 Hub: `Builders<TDocument>` (`Builders.cs`) exposes the six core builder properties — listed alphabetically to match the source: `Filter`, `IndexKeys`, `Projection`, `SetFields`, `Sort`, `Update` — plus the Atlas Search builder hubs (`Search`, `SearchPath`, `SearchScore`, `SearchScoreFunction`, `SearchFacet`, `SearchSpan`). Each property returns a builder that produces an immutable `*Definition<TDocument>`.
 
-The same `*Definition` / `*DefinitionBuilder` pattern is used by `FilterDefinition*`, `UpdateDefinition*`, `ProjectionDefinition*`, `SortDefinition*`, `IndexKeysDefinition*`, `SetFieldDefinitions*`. `PipelineDefinition*` and `PipelineStageDefinition*` follow the same shape, but their helpers are the standalone static classes `PipelineDefinitionBuilder` / `PipelineStageDefinitionBuilder` rather than properties on `Builders<T>`. `FieldDefinition` and `ArrayFilterDefinition` are definition-only — polymorphic subtypes are constructed directly, with no builder hub. For fields specifically, there are two parallel generic shapes: `FieldDefinition<T>` (untyped field) with `StringFieldDefinition<T>` / `ExpressionFieldDefinition<T>` subclasses, and `FieldDefinition<T, TField>` (typed field, used when the projected field's CLR type matters) with `StringFieldDefinition<T, TField>` / `ExpressionFieldDefinition<T, TField>` subclasses. Renders bottom out in `RenderedFieldDefinition` / `RenderedFieldDefinition<TField>`.
+The same `*Definition` / `*DefinitionBuilder` pattern is used by `FilterDefinition*`, `UpdateDefinition*`, `ProjectionDefinition*`, `SortDefinition*`, `IndexKeysDefinition*`, `SetFieldDefinitions*`. **`SetFields` is the odd one out** in that family: the builder is singular (`SetFieldDefinitionsBuilder<T>`) but the definition it produces is plural (`SetFieldDefinitions<T>`, a collection). Don't grep for a non-existent `SetFieldDefinitionBuilder`. `PipelineDefinition*` and `PipelineStageDefinition*` follow the same shape, but their helpers are the standalone static classes `PipelineDefinitionBuilder` / `PipelineStageDefinitionBuilder` rather than properties on `Builders<T>`. `FieldDefinition` and `ArrayFilterDefinition` are definition-only — polymorphic subtypes are constructed directly, with no builder hub. For fields specifically, there are two parallel generic shapes: `FieldDefinition<T>` (untyped field) with `StringFieldDefinition<T>` / `ExpressionFieldDefinition<T>` subclasses, and `FieldDefinition<T, TField>` (typed field, used when the projected field's CLR type matters) with `StringFieldDefinition<T, TField>` / `ExpressionFieldDefinition<T, TField>` subclasses. Renders bottom out in `RenderedFieldDefinition` / `RenderedFieldDefinition<TField>`.
 
 - A `*Definition<T>` is an abstract immutable that knows how to `Render(RenderArgs<T>)` to BSON. Most types render to a `BsonDocument`, with three notable exceptions. `UpdateDefinition<T>.Render` returns the broader `BsonValue` because pipeline-form updates render as a `BsonArray`; cast accordingly when asserting against rendered output. The two-generic `ProjectionDefinition<TSource, TProjection>.Render` returns `RenderedProjectionDefinition<TProjection>` (a wrapper of `BsonDocument` plus the projection-result serializer); only the single-generic `ProjectionDefinition<TSource>.Render` returns a bare `BsonDocument`. `PipelineDefinition<TInput, TOutput>.Render` returns `RenderedPipelineDefinition<TOutput>` (a wrapper of `IReadOnlyList<BsonDocument>` plus the output serializer), not a `BsonDocument`. **Signature outlier:** `ArrayFilterDefinition<TItem>.Render` does *not* take `RenderArgs<T>` — it exposes two overloads, the non-generic `Render(IBsonSerializer itemSerializer, IBsonSerializerRegistry serializerRegistry)` declared on the base `ArrayFilterDefinition` and the typed `Render(IBsonSerializer<TItem> itemSerializer, IBsonSerializerRegistry serializerRegistry)` on `ArrayFilterDefinition<TItem>` (see `ArrayFilterDefinition.cs:46` and `:103`). Don't waste time grepping for a `RenderArgs` overload on it.
 - A `*DefinitionBuilder<T>` constructs concrete definitions via fluent methods.
@@ -58,7 +58,7 @@ The same `*Definition` / `*DefinitionBuilder` pattern is used by `FilterDefiniti
   | `SortDefinition<T>` | ✅ | ✅ | — |
   | `IndexKeysDefinition<T>` | ✅ | ✅ | — |
   | `ArrayFilterDefinition<TItem>` | ✅ | ✅ (JSON) | — |
-  | `FieldDefinition<T>` | ❌ | ✅ (treated as a **literal field name** via `StringFieldDefinition<T>` — *not* JSON-parsed; this is the lone asymmetry vs. the other definitions' string conversions) | — |
+  | `FieldDefinition<T>` | ❌ | ✅ (treated as a **literal field name** via `StringFieldDefinition<T>` — *not* JSON-parsed; this is the asymmetry vs. the other definitions' string conversions, and it applies family-wide — `FieldDefinition<TDocument, TField>` below behaves the same way) | — |
   | `FieldDefinition<TDocument, TField>` | ❌ | ✅ (also a literal field name) | up-cast **to** `FieldDefinition<TDocument>` (drops field-CLR-type parameter) |
   | `PipelineDefinition<TInput, TOutput>` | ❌ | — | `IPipelineStageDefinition[]`, `List<IPipelineStageDefinition>`, `BsonDocument[]`, `List<BsonDocument>` (arrays/lists of `BsonDocument`, **not** a singular `BsonDocument` — wrap a single doc in `new[] { doc }` to use the array conversion) |
   | `SetFieldDefinitions<T>` (plural, the collection type exposed via `Builders<T>.SetFields`; distinct from the singular `SetFieldDefinition<T>`, no `s`) | ❌ | ❌ | — |
@@ -73,7 +73,7 @@ The same `*Definition` / `*DefinitionBuilder` pattern is used by `FilterDefiniti
 
 - `IAggregateFluent<TResult>` / `AggregateFluentBase<TResult>` (public abstract) / `AggregateFluent<TInput,TResult>` (`internal abstract`) — fluent stage chaining over a `PipelineDefinition`.
 - Stage options and result records: `AggregateOptions` (consult `AggregateOptions.cs` for the full list of properties; load-bearing ones include `AllowDiskUse`, `BatchSize`, `Comment`, `Hint`, `MaxTime`, `MaxAwaitTime`, `Collation`, `Let`, `BypassDocumentValidation`, and `TranslationOptions`; **`UseCursor` is `[Obsolete]`** and new code should not propagate it to any new API surface), `AggregateBucket*`, `AggregateFacet*`, `AggregateGraphLookupOptions`, `AggregateLookupOptions`, `AggregateUnwindOptions`. `AggregateHelper` is `internal static` (a render helper), not part of the public surface. `MaxAwaitTime` (here on `AggregateOptions`, mirrored on `ChangeStreamOptions`) is honored only by **tailable / change-stream** cursors (it bounds the server-side wait on `getMore`); it is **inert** for ordinary aggregation cursors — see `AggregateOptions.cs` for the property definition and surrounding context. Don't set it expecting it to bound a regular aggregation.
-- `AggregateExpressionDefinition<TSource, TResult>` is separate — an aggregation-expression abstraction with concrete subclasses `BsonValueAggregateExpressionDefinition<TSource, TResult>`, `ExpressionAggregateExpressionDefinition<TSource, TResult>`, and `DocumentsAggregateExpressionDefinition<TDocument>`. The last is the input shape for `$documents` and is accepted only by the client- and database-level entry points `IMongoClient.Aggregate(...)` / `IMongoDatabase.Aggregate(...)`; there is no collection-level fluent equivalent (collection-level aggregates run against the collection itself). Used by stage builders that take expression arguments, not itself a stage option.
+- `AggregateExpressionDefinition<TSource, TResult>` is separate — an aggregation-expression abstraction with concrete subclasses `BsonValueAggregateExpressionDefinition<TSource, TResult>`, `ExpressionAggregateExpressionDefinition<TSource, TResult>`, and `DocumentsAggregateExpressionDefinition<TDocument>`. The last is the input shape for `$documents` and is consumed via the `PipelineStageDefinitionBuilder.Documents(...)` helper, which produces the first stage on a client- or database-level `Aggregate(...)`; there is no collection-level fluent equivalent (collection-level aggregates run against the collection itself). Used by stage builders that take expression arguments, not itself a stage option.
 - Bottoms out in `Core/Operations/AggregateOperation<T>` and `AggregateToCollectionOperation` (for `$out` / `$merge`).
 
 Boundary with `operations-reviewer`: this layer owns **stage shape and pipeline semantics**. The Operations layer owns **cursor lifecycle, retry, and binding selection**.