Redis backed Caching of Internal Embeddings#2
Open
ajtiwari07 wants to merge 21 commits into
Open
Conversation
….15.3 (Azure#3488) ## Why make this change? Bump `OpenTelemetry.Exporter.OpenTelemetryProtocol` to pick up patch fixes from 1.15.3. ## What is this change? - Updated `OpenTelemetry.Exporter.OpenTelemetryProtocol` from `1.13.0` → `1.15.3` in `src/Directory.Packages.props` (centrally managed versions) ## How was this tested? - [ ] Integration Tests - [ ] Unit Tests ## Sample Request(s) N/A — dependency version bump only. > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `www.nuget.org` > - Triggering command: `/home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/Azure/data-api-builder/settings/copilot/coding_agent) (admins only) > > </details> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Co-authored-by: Ruben Cerna <rcernaserna@microsoft.com>
prshri-msft
added a commit
to prshri-msft/data-api-builder
that referenced
this pull request
May 1, 2026
…nale Addresses two PR review comments from JimRoberts-MS on PR ajtiwari07#1: Comment ajtiwari07#2 (ParameterEmbeddingHelper.cs:157): "is there a limitation that only one parameter can be configured for embedding at a time? I think we'd want to do batching here to avoid multiple sequential waits on the api" Comment ajtiwari07#1 (RuntimeConfigValidator.cs:507): "can this limitation about default values be clarified a bit more?" Changes: 1. ParameterEmbeddingHelper.cs — refactor to use TryEmbedBatchAsync (addresses ajtiwari07#2): Restructured the substitution loop into 3 phases: - COLLECT — validate each embed param value, gather (paramName, text) pairs (preserves per-param error specificity for type/null checks) - BATCH — single TryEmbedBatchAsync call instead of N sequential TryEmbedAsync calls (saves ~(N-1) × API_LATENCY on cache miss) - SUBSTITUTE — write each returned vector back into resolvedParams Behavior preserved: - Single-embed-param path is equivalent (batch of 1) - All error status codes unchanged (400 for bad input, 500 for service failure, 503 for missing service) - In-place mutation contract on resolvedParams unchanged - G9 float format preserved Defensive checks added: - Length mismatch between requests and returned embeddings → 500 - Null/empty embedding for any individual param → 500 Type validation extracted into private ExtractTextValue(paramName, value) helper to flatten the nested if/else in the main loop. Verified with 16 manual test cases across REST POST/GET and GraphQL, covering single-embed and multi-embed sprocs, positive and negative inputs. 2. ParameterEmbeddingHelper.cs — fix misleading internal comment (related to ajtiwari07#1's surrounding context): The previous comment claimed "DAB's existing required-param validation handles missing required params later" — but DAB's request validation for sprocs only checks for extra fields (not missing ones). The actual mechanism that catches missing required params is the SQL Server error "expects parameter X, which was not supplied", parsed by MsSqlDbExceptionParser into a 400 DatabaseInputError. Updated the comment to describe what actually happens. 3. RuntimeConfigValidator.cs — expand embed/default rule rationale (addresses ajtiwari07#1): Replaced the one-line rationale for "Rule 3: embed:true with a default value is not supported" with a multi-paragraph explanation that: - Leads with the conceptual UX point: an embed param represents user input (typically a search query); defaulting it would mean the server fabricates and embeds a query the user never typed. That isn't a sensible fallback. - Notes that defaults on non-embed params of the same sproc remain supported (rule only fires for embed: true params). - Briefly documents why even setting aside the UX concern, supporting embed-defaults would be non-trivial (GraphQL schema literal-baking has no VECTOR type; REST/MCP defaults would be re-embedded every request; embedding-at-startup couples startup to provider availability). - Documents the current observed behavior when a client forgets to supply an embed param (verified empirically): explicit null/empty → 400 BadRequest from the helper; field omitted → 400 DatabaseInputError from SQL via MsSqlDbExceptionParser. Both produce a clear, actionable client error. - Notes that the rule can be lifted later if a real use case emerges. No behavior changes beyond the batching itself. Validator rule unchanged; helper logic for missing-value handling unchanged; error message text unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Why make this change? Closes Azure#3274 - MCP Server returns "Method not found: logging/setLevel" error when clients send the standard MCP logging/setLevel request. Closes Azure#3275 - Control output in MCP stdio mode (default to `LogLevel.None`, redirect/suppress console output). ## What is this change? ### MCP `logging/setLevel` Handler - Added handler for `logging/setLevel` JSON-RPC method in `McpStdioServer.cs` - Implemented `DynamicLogLevelProvider` with `ILogLevelController` interface to allow MCP to update log levels dynamically - Added `IsCliOverridden` and `IsConfigOverridden` properties to enforce precedence rules ### Log Level Precedence System **Precedence (highest to lowest):** 1. **CLI `--LogLevel` flag** - cannot be changed by MCP 2. **Config `runtime.telemetry.log-level`** - cannot be changed by MCP 3. **MCP `logging/setLevel`** - only works if neither CLI nor config set a level 4. Default (LogLevel.None for MCP stdio mode) If CLI or config set a level, MCP requests are accepted but silently ignored (no error returned per MCP spec). ### Early Config Reading for MCP Mode - Added `TryGetLogLevelFromConfig()` in `Program.cs` to read config file early (before host build) - This ensures config log level is detected before Console redirect decision - Console redirect for MCP stdio mode now respects config log level ### CLI Log Level Handling - Added `Utils.CliLogLevel` property to track the parsed `--LogLevel` value - CLI's `CustomLoggerProvider` now respects the `--LogLevel` value for its own logging ### Config Helpers - Added `HasExplicitLogLevel()` helper to `RuntimeConfig` to correctly detect when config actually pins a log level - This properly handles null values in telemetry section (null values don't count as explicit override) ## How was this tested? - [x] Unit Tests (`DynamicLogLevelProviderTests` - 5 tests) - [x] Manual Testing ### Manual Test 1: No override (MCP can change level) 1. Start MCP server without `--LogLevel` and without config `log-level` 2. MCP sends `logging/setLevel` with `level: info` 3. Result: Log level changes to info ### Manual Test 2: CLI override (MCP blocked) 1. Start MCP server with `--LogLevel Warning` 2. MCP sends `logging/setLevel` with `level: info` 3. Result: Log level stays at Warning, MCP request accepted silently ### Manual Test 3: Config override (MCP blocked) 1. Add `"telemetry": { "log-level": { "default": "Warning" } }` to config 2. Start MCP server without `--LogLevel` 3. MCP sends `logging/setLevel` with `level: info` 5. Result: Log level stays at Warning, MCP request accepted silently ### Manual Test 4: Config with null values (MCP can change level) 1. Add `"telemetry": { "log-level": { "default": null } }` to config 2. Start MCP server without `--LogLevel` 3. MCP sends `logging/setLevel` with `level: info` 4. Result: Log level changes to info (null values don't count as override) ## Sample Request(s) MCP client sends: ```json { "jsonrpc": "2.0", "id": 1, "method": "logging/setLevel", "params": { "level": "info" } } ``` Server responds with empty result (success per MCP spec) and updates log level if no CLI/config override is active. --------- Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com>
prshri-msft
added a commit
to prshri-msft/data-api-builder
that referenced
this pull request
May 4, 2026
Adds the first formal automated test coverage for Phase 3 production code. New file: src/Service.Tests/UnitTests/ParameterEmbeddingHelperTests.cs 28 tests covering ParameterEmbeddingHelper.SubstituteEmbedParametersAsync, organized into 7 #region blocks (matching the EmbeddingServiceTests.cs "#region Batch Embedding Tests" pattern): #region No-Op Cases (4 tests) - NullConfigParams_ReturnsImmediately_NoServiceCall - NoEmbedParams_ReturnsImmediately_NoServiceCall - EmbedParamsConfiguredButNoneSupplied_ReturnsAfterCollect_NoServiceCall - EmptyConfigParamsList_ReturnsImmediately_NoServiceCall #region Service Availability (2 tests) - NullService_WithEmbedParams_Throws503 (defense-in-depth) - NullService_WithoutEmbedParams_NoThrow (backward compat) #region Input Type Validation (8 tests) - PlainString_AcceptsAndEmbeds - JsonElementString_AcceptsAndEmbeds - JsonElementNumber_Throws400 - JsonElementBoolean_Throws400 - JsonElementArray_Throws400 - JsonElementObject_Throws400 - JsonElementNull_Throws400AsEmpty - NonStringNonJsonElementValue_Throws400 #region Empty And Whitespace Validation (3 tests) - EmptyString_Throws400 - WhitespaceString_Throws400 - NullValue_Throws400 #region Batching Behavior (5 tests) — covers Jim review comment ajtiwari07#2 - SingleEmbedParam_CallsBatchOnce_NotSequential - MultipleEmbedParams_CallsBatchOnce_NotSequential ← key batching guarantee - MixedEmbedAndNonEmbed_OnlyEmbedTextsBatched - MultipleEmbedParams_OrderPreserved_BatchTextsMatchConfigOrder - PartiallySuppliedEmbedParams_BatchesSubsetOnly #region Batch Result Handling (4 tests) - BatchSuccess_VectorsSubstitutedInResolvedParams - BatchFailure_Throws500_WithAllParamNames - BatchLengthMismatch_Throws500 - IndividualEmbeddingEmpty_Throws500_NamingFailedParam #region Output Format And Cancellation (2 tests) - VectorJson_UsesG9AndInvariantCulture ← validates locale-independent G9 float serialization - CancellationToken_ForwardedToEmbeddingService Implementation notes -------------------- - Mocking pattern: Mock<IEmbeddingService> (Strict). Each test sets up the expected batch call and verifies post-conditions on resolvedParams. No database, no DI container — pure unit tests. - Helper factories at top of class (EmbedParam, NormalParam, JsonElementFrom, SetupBatch, VerifyBatchedExactlyOnce) keep individual test bodies focused on the behavior being tested. - JsonElement construction uses JsonDocument.Parse(...).RootElement.Clone() so the parsed element survives after the source document is disposed. - Float values used in expected-output assertions are powers of 1/2 (0.5, 0.25, 0.125) which are exactly representable in binary float and round-trip through G9 to the same string representation. The "G9 + InvariantCulture" test specifically uses non-exact values (0.1f, -0.2f, 0.0001234567f) and asserts on parsed-back values rather than exact strings to verify precision and locale independence. - `#nullable enable` directive at the top of the file is required because the helper signature uses IDictionary<string, object?> and the test project is set to `<Nullable>disable</Nullable>` globally. Per-file enable is the smallest scoped change that lets us match the helper's signature without introducing project-wide nullable warnings. Test results ------------ - dotnet build src/Service.Tests/Azure.DataApiBuilder.Service.Tests.csproj -c Release → Build succeeded. 0 Warning(s). 0 Error(s). - dotnet test --filter "FullyQualifiedName~ParameterEmbeddingHelperTests" → Total tests: 28. Passed: 28. Failed: 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…re#3318) ## Why make this change? - Closes issue Azure#3262 The logger for the Startup class is not initialized properly, since this logger is special due to the nature of the Startup class it needs to be continuously updated as DAB initializes. This causes two problems: - Some logs appear even when LogLevel is set to some value that would impede those logs to appear. - Some logs don't appear at all, even when LogLevel is set to a value that should allow them to be logged. - Closes issue Azure#3256 & Azure#3255 The CLI logger still outputs some logs even when the LogLevel is set to `none`. It is expected that if the LogLevel set is `none` or some other level that shouldn't output the `information` level, the logs will not appear. ## What is this change? Important Note: These changes currently only allow us to change the LogLevel from the CLI with the `default` namespace in the config file. An task was created to solve this issue: Azure#3451 In order to solve issue Azure#3262: - We removed the LogBuffer from the services inside of `Startup.cs`, this is necessary since we wanted each class to have its own LogBuffer so that we are able to tell from which logger the logs are being outputted. - Then, we also correctly initialized the `Startup` logger by changing the method that it was using to initialize the logger, it now uses `CreateLoggerFactoryForHostedAndNonHostedScenario` which checks if there are any LogLevel namespaces from the config file that can be applicable for the specific logger. It is important to note that there are multiple places where the logs are flushed in order to cover for the cases in which an exception is found and causes DAB to end abruptly, and when we there is an IsLateConfigured scenario. - We also changed the logger for the LogBuffer in all the missing places where it creates logs before the logger is able to properly initialize to add those logs to the LogBuffer and only flush them after the loggers are initialized. In order to solve issue Azure#3256 & Azure#3255: - We changed the CLI so that we add all the logs go to a single global LogBuffer that is created inside the `StartOptions.cs` until it is able to deserialize the RuntimeConfig and find which level to set the `LogLevel` in order to flush all the logs. - This is something that we only want to happen when we use the `dab start` command, which is why we only make this change in the `StartOptions.cs` file, on the function `TryStartEngineWithOptions` inside of `ConfigGenerator.cs`, and a few functions from `Utils.cs` and `ConfigMerger.cs` that are used inside the `TryStartEngine` function. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests ## Sample Request(s) - dab start --LogLevel none - dab start --LogLevel error --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
## Why make this change? Closes Azure#3267 ## What is this change? Alters the validation logic in the following way. Is top-level config with data-source-files? (we call this a `Root` config file) ├── YES │ ├── Has datasource? → ValidateEntityPresence (same rules as non-root) │ ├── No datasource but has entities/autoentities? → ERROR │ └── No datasource, no entities → VALID (children provide everything) │ └── For each child → ValidateNonRootConfig(child, filename) │ └── NO (standalone or child config) ├── No datasource? → ERROR: "data source is required" └── Has datasource → ValidateEntityPresence Note: A top-level config file without any children data-source files is NOT considered a root. And an intermediary config file, ie: is a child, that also has child configs is NOT a root. Only a top-level config with children configs is a Root. #### ValidateEntityPresence Count resolved autoentities from AutoentityResolutionCounts total = manual entities + resolved autoentities total == 0? → ERROR: "No entities found" total > 0 but autoentities discovered nothing? → WARN: "Autoentities configured but none discovered" No double messaging. If total is 0, only the error is recorded, not the warning. ## How was this tested? ### Truth table — top-level config Variables (`1` = present / non-empty, `0` = absent / empty): - **DSF** — `data-source-files` present - **DS** — `data-source` present - **E** — manual `entities` count > 0 - **AE** — `autoentities` count > 0 (presence, *not* resolved count) Path is determined by `IsRootConfig = (DSF == 1) && !IsChildConfig`. | # | DSF | DS | E | AE | AE resolved | Path | Expected | Test | |---|:---:|:--:|:-:|:--:|:-----------:|------|----------|------| | 1 | 0 | 0 | 0 | 0 | — | Non-root | **Error**: "data source is required" | `TestNonRootWithNoDataSourceProducesError` | | 2 | 0 | 0 | 0 | 1 | — | Non-root | **Error**: "data source is required" | _covered by #1 — DS check fires first_ | | 3 | 0 | 0 | 1 | 0 | — | Non-root | **Error**: "data source is required" | _covered by #1_ | | 4 | 0 | 0 | 1 | 1 | — | Non-root | **Error**: "data source is required" | _covered by #1_ | | 5 | 0 | 1 | 0 | 0 | — | Non-root | **Error**: "No entities found" | `TestNonRootWithDataSourceAndNoEntitiesProducesError` | | 6a | 0 | 1 | 0 | 1 | 0 | Non-root | **Error**: "No entities found" | `TestNonRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` | | 6b | 0 | 1 | 0 | 1 | >0 | Non-root | **Valid** | `TestNonRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` | | 7 | 0 | 1 | 1 | 0 | — | Non-root | **Valid** | `TestNonRootWithDataSourceAndEntitiesIsValid` | | 8a | 0 | 1 | 1 | 1 | 0 | Non-root | **Valid** + **Warn** | `TestNonRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` | | 8b | 0 | 1 | 1 | 1 | >0 | Non-root | **Valid** | _covered by #7 / #6b combined_ | | 9 | 1 | 0 | 0 | 0 | — | Root | **Valid** (children carry the load) | `TestRootWithNoDataSourceAndNoEntitiesIsValid`, `TestRootConfigWithNoDataSourceAndNoEntitiesParses` | | 10 | 1 | 0 | 0 | 1 | — | Root | **Error**: "must not define entities or autoentities" | `TestRootWithNoDataSourceButAutoentitiesProducesError` | | 11 | 1 | 0 | 1 | 0 | — | Root | **Error**: "must not define entities" | `TestRootWithNoDataSourceButEntitiesProducesError` | | 12 | 1 | 0 | 1 | 1 | — | Root | **Error** | _covered by #11_ | | 13 | 1 | 1 | 0 | 0 | — | Root (with own DS) | **Error**: "No entities found" | `TestRootWithDataSourceAndNoEntitiesProducesError` | | 14a | 1 | 1 | 0 | 1 | 0 | Root (with own DS) | **Error**: "No entities found" | `TestRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` | | 14b | 1 | 1 | 0 | 1 | >0 | Root (with own DS) | **Valid** | `TestRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` | | 15 | 1 | 1 | 1 | 0 | — | Root (with own DS) | **Valid** | `TestRootWithDataSourceAndEntitiesIsValid` | | 16a | 1 | 1 | 1 | 1 | 0 | Root (with own DS) | **Valid** + **Warn** | `TestRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` | | 16b | 1 | 1 | 1 | 1 | >0 | Root (with own DS) | **Valid** | _covered by Azure#15 / #14b combined_ | ### Truth table — child config (validated when iterating `root.ChildConfigs`) Children are always treated as non-root regardless of their own `data-source-files`. | # | DS | E | AE | AE resolved | Expected | Test | |---|:--:|:-:|:--:|:-----------:|----------|------| | C1 | 0 | 0 | 0 | — | **Error** naming the child file: "data source is required" | `TestChildWithNoDataSourceProducesNamedError` | | C2 | 0 | * | * | — | **Error** naming the child file: "data source is required" | _covered by C1_ | | C3 | 1 | 0 | 0 | — | **Error** naming the child file: "No entities found" | `TestChildWithDataSourceAndNoEntitiesProducesNamedError` | | C4a | 1 | 0 | 1 | 0 | **Error** naming the child file: "No entities found" | `TestChildWithDataSourceAndAutoentitiesResolvingZeroProducesNamedError` | | C4b | 1 | 0 | 1 | >0 | **Valid** | _covered by C5 (resolved entities behave the same as manual entities)_ | | C5 | 1 | 1 | 0 | — | **Valid** | _implicitly via `TestRootWithDataSourceAndEntitiesIsValid` setup_ | | C6a | 1 | 1 | 1 | 0 | **Valid** + **Warn** naming the child file | `TestChildWithEntitiesAndAutoentitiesResolvingZeroLogsNamedWarning` | | C6b | 1 | 1 | 1 | >0 | **Valid** | _covered by C5_ | ### Other scenarios | Scenario | Expected | Test | |----------|----------|------| | Connection-string error gates entity validation (no entity error fires when DB unreachable) | `IsConfigValid == false` due to connection error only | `TestValidateNonRootZeroEntitiesWithInvalidConnectionString` | | Config with no entities parses cleanly (constructor no longer throws) and `IsConfigValid` returns false without throwing | parse OK, validate fails | `TestValidateConfigWithNoEntitiesProducesCleanError` _(modified)_ | | Root parses successfully without a data source | parse OK, `IsRootConfig == true` | `TestRootConfigWithNoDataSourceAndNoEntitiesParses` | | Non-root with DS and no entities parses successfully | parse OK, `IsRootConfig == false` | `TestNonRootConfigWithDataSourceAndNoEntitiesParses` | | Autoentities present but resolve to nothing — must not crash, must not double-message with "No entities found" | no crash; only "No entities found" if total = 0 | `ValidateAutoentitiesConfiguration` _(modified to `isValidateOnly: true`)_ | New tests: `TestRootConfigWithNoDataSourceAndNoEntitiesParses` Root config (has data-source-files) without datasource parses OK `TestNonRootConfigWithDataSourceAndNoEntitiesParses` Non-root config with datasource + no entities parses OK (validation catches it later) `TestNonRootWithDataSourceAndNoEntitiesProducesError` Calls ValidateDataSourceAndEntityPresence directly, error recorded `TestNonRootWithNoDataSourceProducesError` No datasource, error with "data source is required" `TestNonRootWithDataSourceAndEntitiesIsValid` Datasource + entities, no errors `TestRootWithNoDataSourceAndNoEntitiesIsValid` Root with child, no own datasource, valid `TestRootWithNoDataSourceButEntitiesProducesError` Root with entities but no datasource, error `TestRootWithDataSourceAndEntitiesIsValid` Root with own datasource + entities, valid `TestChildWithDataSourceAndNoEntitiesProducesNamedError` Child with no entities, error names the child file `TestChildWithNoDataSourceProducesNamedError` Child with no datasource, error names the child file `TestNonRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` Non-root with only autoentities that resolve to 0 `TestNonRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` Non-root with only autoentities resolving > 0 entities `TestNonRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` Non-root with manual entities + autoentities resolving 0 `TestRootWithNoDataSourceButAutoentitiesProducesError` Root with no datasource but autoentities defined `TestRootWithDataSourceAndNoEntitiesProducesError` Root with own datasource and zero entities/autoentities `TestRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` Root with own datasource and autoentities resolving 0 `TestRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` Root with own datasource and autoentities resolving > 0 `TestRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` Root with own datasource, manual entities, and autoentities resolving 0 `TestChildWithDataSourceAndAutoentitiesResolvingZeroProducesNamedError` Child with autoentities-only resolving 0 `TestChildWithEntitiesAndAutoentitiesResolvingZeroLogsNamedWarning` Child with manual entities + autoentities resolving 0 Modified tests: `TestValidateConfigWithNoEntitiesProducesCleanError` Replaced main's version (expected parse failure) with ours: parse succeeds, IsConfigValid returns false `ValidateAutoentitiesConfiguration` Changed to isValidateOnly: true, asserts no crashes instead of zero errors --------- Co-authored-by: Anusha Kolan <anushakolan10@gmail.com>
## Why make this change? Enables MCP clients (like MCP Inspector, Claude Desktop, VS Code Copilot) to receive real-time log output via MCP `notifications/message`. Related: Azure#3274 (depends on PR Azure#3419) ## What is this change? When `logging/setLevel` is called with a level other than "none", logs are sent to MCP clients as JSON-RPC notifications: ```json { "jsonrpc": "2.0", "method": "notifications/message", "params": { "level": "info", "logger": "Azure.DataApiBuilder.Service.Startup", "data": "Starting Data API builder..." } } ``` ### New files: - `McpLogNotificationWriter.cs` - Writes logs as MCP notifications to stdout - `McpLogger.cs` / `McpLoggerProvider.cs` - ILogger implementation for .NET logging pipeline - `McpLogNotificationTests.cs` - Unit tests (8 tests) ### Modified files: - `Program.cs` - Registers `McpNotificationWriter` and `McpLoggerProvider` for MCP mode - `McpStdioServer.cs` - Enables notifications when `logging/setLevel` is called ## How was this tested? - Unit tests: 6 tests covering level mapping, enable/disable, JSON format - Manual testing with MCP Inspector: verified notifications appear when `logging/setLevel` is sent ## Note This PR targets `dev/anushakolan/set-log-level` (PR Azure#3419) as it depends on the `logging/setLevel` implementation.
## Why make this change?
Fixes the format of the OData filter in JWT string claims.
## What is this change?
In `AuthorizationResolver` we now escape embedded single quotes in claim
values by doubling them, before we wrap the value in single quotes for
OData substitution. This conforms to the OData 4.01 ABNF rule for string
literals (Section 7: Literal Data Values).
Policy: `@item.col1 eq @claims.userId`
Claim `userId` value: `alice' or 1 eq 1 or '`
| | Resulting OData predicate |
| --- | --- |
| Before | `col1 eq 'alice' or 1 eq 1 or ''` <- injects `or 1 eq 1`,
bypassing row-level auth |
| After | `col1 eq 'alice'' or 1 eq 1 or '''` <- attacker payload
contained inside a single string literal |
## How was this tested?
New parameterized test
`DbPolicy_StringClaim_SingleQuotesEscaped_PreventsODataInjection` in
`src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs`
covers:
- Active OR-predicate injection attempt is neutralized.
- Legitimate apostrophe-bearing value (e.g. `O'Brien`) is safely
escaped.
- Value composed solely of single quotes is fully escaped.
- Value with no single quotes is unchanged aside from the enclosing
quotes (no regression).
## Sample Request(s)
```json
{
"entities": {
"Note": {
"source": "dbo.Notes",
"permissions": [
{
"role": "authenticated",
"actions": [
{
"action": "read",
"policy": { "database": "@item.ownerId eq @claims.userId" }
}
]
}
]
}
}
}
```
Reproduction - `userId` claim value of `alice' or 1 eq 1 or '`:
```http
GET /api/Note HTTP/1.1
Authorization: Bearer <jwt-with-crafted-userId-claim>
X-MS-API-ROLE: authenticated
```
- Before fix: the engine emitted `WHERE ownerId = 'alice' or 1 eq 1 or
''`, returning rows owned by other users.
- After fix: the engine emits `WHERE ownerId = 'alice'' or 1 eq 1 or
'''`, which compares against the literal string `alice' or 1 eq 1 or '`
and returns no unauthorized rows.
Co-authored-by: Souvik Ghosh <souvikofficial04@gmail.com>
Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
…ent (Azure#3475) ## Why make this change? `nuget/README.md` (the README shown on the NuGet package page) was minimal and stale — it described only REST and GraphQL, omitted the MCP endpoint entirely and other content present in the root `README.md`. ## What is this change? Rewrote `nuget/README.md` to match the quality and completeness of the root `README.md`, adapted for the NuGet context: - **Badges** — docs link, license - **About** — updated to cover REST, GraphQL, and MCP endpoints - **Features** — added MCP section: DML tools, custom tools via stored procedures, configurable path (`/mcp` default), AI client compatibility (GitHub Copilot, Azure AI Foundry) - **Resources** — aligned with root README; links use absolute GitHub URLs ## How was this tested? - [ ] Integration Tests - [ ] Unit Tests Documentation-only change; no code logic modified. ## Sample Request(s) N/A --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Co-authored-by: Ruben Cerna <rcernaserna@microsoft.com> Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
|
|
||
| if (string.IsNullOrWhiteSpace(level2Options.ConnectionString)) | ||
| { | ||
| throw new Exception("Embeddings Azure Managed Redis L2 cache requires a valid connection-string."); |
There was a problem hiding this comment.
for log messages and such not sure if it would be less confusing to refer to it as 'Remote' cache or something in case people are running outside of Azure
Owner
Author
There was a problem hiding this comment.
Technically yes, I was hoping it is our value prop going into L2 caching. Will Modify it.
| .WithBackplane(new RedisBackplane(new RedisBackplaneOptions | ||
| { | ||
| ConnectionMultiplexerFactory = async () => await connectionMultiplexerTask | ||
| })); |
There was a problem hiding this comment.
for this particular api do we not need to .Apply() or something to signal that we're done configuring?
Owner
Author
There was a problem hiding this comment.
Nope, FusionCacheBuilder is itself registered in the DI container.
JimRoberts-MS
approved these changes
May 11, 2026
**Summary** This PR adds configurable text chunking capabilities to the embeddings API, enabling automatic text segmentation before embedding generation. This feature supports both single-text and multi-document batch processing with runtime configuration and query parameter overrides. **Changes** Configuration Added EmbeddingsChunkingOptions.cs - Configuration model for chunking behavior Enabled (bool) - Enable/disable chunking SizeChars (int) - Chunk size in characters (default: 800) OverlapChars (int) - Overlap between chunks (default: 100) EffectiveSizeChars property ensures minimum valid chunk size Modified EmbeddingsOptions.cs - Added Chunking property and IsChunkingEnabled helper Removed EmbeddingsCacheOptions.cs - Simplified configuration by removing unused cache feature **API Enhancements** Modified Controllers/EmbeddingController.cs Auto-detects request type (single text vs. document array) Implements overlapping text chunking algorithm Supports query parameter overrides: $chunking.enabled, $chunking.size-chars, $chunking.overlap-chars Returns multiple embeddings per document when chunking is enabled Added Models/EmbedDocumentRequest.cs - Request model for document arrays Added Models/EmbedDocumentResponse.cs - Response model with chunked embeddings Schema ([schemas](vscode-file://vscode-app/c:/Users/ajtiwari/AppData/Local/Programs/Microsoft%20VS%20Code/07ff9d6178/resources/app/out/vs/code/electron-browser/workbench/workbench.html)) Modified dab.draft.schema.json - Added chunking configuration schema with validation rules Testing ([UnitTests](vscode-file://vscode-app/c:/Users/ajtiwari/AppData/Local/Programs/Microsoft%20VS%20Code/07ff9d6178/resources/app/out/vs/code/electron-browser/workbench/workbench.html)) Added EmbeddingsChunkingOptionsTests.cs (13 tests) - Configuration validation Added ChunkTextTests.cs (21 tests) - Chunking algorithm validation including edge cases Modified EmbeddingControllerTests.cs (+18 tests) - API endpoint tests for chunking and document arrays Total Test Coverage: 72 tests (48 existing + 24 new) - All passing## Why make this change? **Testing** All 72 unit tests passing Edge cases covered: empty text, very small chunks, overlap larger than chunk size, Unicode text Query parameter parsing validated Backward compatibility verified **Breaking Changes** None - This is a backward-compatible addition. Existing single-text requests continue to work without modification. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com> Co-authored-by: roberto.perez <robertopc@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Souvik Ghosh <souvikofficial04@gmail.com> Co-authored-by: Aniruddh Munde <anmunde@microsoft.com> Co-authored-by: sayalikudale <68876274+sayalikudale@users.noreply.github.com> Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com> Co-authored-by: Anusha Kolan <anushakolan10@gmail.com> Co-authored-by: Sayali Kudale <sayalikudale@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jim Roberts <JimRoberts-MS@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Embeddings-Specific Cache Configuration
EmbeddingsCacheOptionsclass for embeddings-specific cache settings2. Level 2 (L2) Distributed Redis Cache Support
EmbeddingsCacheLevel2Optionsclass for Redis-specific configuration3. Enhanced Configuration Schema
cachesection underembeddings4. Dedicated FusionCache Instance for Embeddings
5. Comprehensive Test Coverage
EmbeddingsCacheOptionsclass (205 lines)EmbeddingsCacheLevel2Optionsclass (64 lines)