Merge main into embedding phase1#3
Closed
sayalikudale wants to merge 112 commits into
Closed
Conversation
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…configuration Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
… and telemetry integration Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…ate empty embeddings Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…oint/health sub-objects Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…orization Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…lthCheckConfig in converter
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…dding controller tests, switching the dab schema for the embedding system to default to false
embeddings endpoint is now permanently fixed to /embed with no user-configurable path option. This removes unnecessary configuration surface since the feature has not been released yet, eliminating the need for backward compatibility. Changes: - Remove path property from dab.draft.schema.json - Remove Path, UserProvidedPath, and EffectivePath from EmbeddingsEndpointOptions - Remove EffectiveEndpointPath from EmbeddingsOptions - Remove path deserialization from EmbeddingsOptionsConverterFactory - Remove --runtime.embeddings.endpoint.path CLI option - Remove path configuration logic from ConfigGenerator - Remove endpoint path validation from RuntimeConfigValidator - Update Startup.cs logging to use DEFAULT_PATH constant - Update all tests to remove path references
## Why make this change? - Closes Azure#3279 The aggregate_records MCP tool always rejects requests without groupby because the orderby schema has "default": "desc", causing LLMs to always include it, which then triggers a validation error requiring groupby. Simple aggregations like SELECT COUNT(*) FROM table had to go through this kind of behaviour and complexity. ## What is this change? Removed "default": "desc" from the orderby input schema so LLMs no longer auto-populate it. Updated ValidateGroupByDependencies to silently ignore orderby when groupby is absent instead of returning an error. Ordering is meaningless without grouping, so the parameter is harmlessly cleared via a ref flag. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests - [x] Manual Tests (VS code GHCP) --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
…om/ajtiwari07/data-api-builder into add-internal-text-embedding-system
…om/ajtiwari07/data-api-builder into add-internal-text-embedding-system
## Why make this change? - Resolves issue Azure#3469 ## What is this change? - `Cli.csproj`: Changes package description and tags to include new MCP endpoint. - `Core.csproj`: Adds authoring requirements based on the following link https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/11557/Publish-as-Microsoft?anchor=publishing-microsoft-packages-on-nuget.org ## How was this tested? - [ ] Integration Tests - [ ] Unit Tests No tests are necessary for this change ## Sample Request(s) N/A
…om/ajtiwari07/data-api-builder into add-internal-text-embedding-system
## Why make this change? - Fixes issue Azure#3455 ## What is this change? This changes the default value of the `patterns.name` property inside `autoentities` so that it uses both the {schema} and {object} in order to allow users to use multiple objects that have the same name but are from different schemas without the need to do additional changes to the config file. This PR is dependent on PR Azure#3468. ## How was this tested? - [X] Integration Tests - [ ] Unit Tests - [X] Manual Tests Used sample below to ensure that different entities with the same object name but different schema name can be created without the need to change the `patterns.name` property. ## Sample Request(s) ``` "AllMagazineObjects": { "patterns": { "include": [ "%.magazines" ] }, "permissions": [ { "role": "anonymous", "actions": [ { "action": "*" } ] } ] } ``` <img width="1060" height="499" alt="image" src="https://github.com/user-attachments/assets/8a8f01ae-a10c-49ba-a3fe-4881c8a2aacd" /> <img width="1075" height="649" alt="image" src="https://github.com/user-attachments/assets/6754b26e-fe24-4f26-adaa-c41c5ee6955f" /> --------- Co-authored-by: Souvik Ghosh <souvikofficial04@gmail.com>
… section is absent (Azure#3450) ## Why make this change? GraphQL aggregation features (`groupBy`, `sum`, `avg`, `min`, `max`, `count`) were silently disabled for users whose config lacked an explicit `runtime.graphql` section, even though `EnableAggregation` defaults to `true`. ## What is this change? **Bug fix: `RuntimeConfig.EnableAggregation` logic inversion** The property used `&&` logic, returning `false` when `Runtime` or `Runtime.GraphQL` was `null`. This is inconsistent with every analogous property (e.g. `IsGraphQLEnabled`) which use `||` logic to treat absence as "use default (enabled)": ```csharp // Before (broken): returns false when runtime.graphql section is absent public bool EnableAggregation => Runtime is not null && Runtime.GraphQL is not null && Runtime.GraphQL.EnableAggregation; // After (fixed): returns true when section is absent, consistent with IsGraphQLEnabled public bool EnableAggregation => Runtime is null || Runtime.GraphQL is null || Runtime.GraphQL.EnableAggregation; ``` **Additional fixes:** - `GraphQLSchemaCreator.OnConfigChanged` now updates `_isAggregationEnabled` on hot-reload (was omitted, causing stale state) - `schemas/dab.draft.schema.json`: added `enable-aggregation` to `runtime.graphql` properties — it was missing despite `additionalProperties: false`, meaning any config explicitly setting the flag would fail schema validation **Test refactoring (per code review feedback):** - Extracted a `LoadConfig` private helper in `RuntimeConfigLoaderTests` to eliminate repeated mock-filesystem setup across all `EnableAggregation` tests - Merged `EnableAggregation_WhenExplicitlyDisabled_ReturnsFalse` and `EnableAggregation_WhenExplicitlyEnabled_ReturnsTrue` into a single parameterized `[DataTestMethod]` (`EnableAggregation_WhenExplicitlySet_ReturnsConfiguredValue`) using `[DataRow(true)]` and `[DataRow(false)]` - Merged `Build_WithMssqlAndAggregationEnabled_AddsGroupByToConnectionType` and `Build_WithPostgreSqlAndAggregationEnabled_DoesNotAddGroupByToConnectionType` into a single parameterized `[DataTestMethod]` (`Build_WithAggregationEnabled_GroupByPresenceMatchesDatabaseSupport`) covering MSSQL, DWSQL (groupBy present), PostgreSQL, and MySQL (groupBy absent) ## How was this tested? - [ ] Integration Tests - [x] Unit Tests - `RuntimeConfigLoaderTests`: `EnableAggregation` defaults to `true` when `runtime` or `runtime.graphql` sections are absent; a single parameterized test (`EnableAggregation_WhenExplicitlySet_ReturnsConfiguredValue`) covers both `true` and `false` explicit values using `[DataRow]`. A shared `LoadConfig` helper eliminates repeated mock-filesystem setup. - `QueryBuilderTests`: a single parameterized test (`Build_WithAggregationEnabled_GroupByPresenceMatchesDatabaseSupport`) covers MSSQL and DWSQL (groupBy present) and PostgreSQL and MySQL (groupBy absent), plus a test confirming groupBy is omitted when aggregation is disabled. ## Sample Request(s) With a config that has no explicit `runtime.graphql` section, the following now works as expected for MSSQL/DWSQL entities: ```graphql { books { groupBy(fields: [price]) { fields { price } aggregations { count sum(field: price) avg(field: price) max(field: price) min(field: price) } } } } ``` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Aniruddh25 <3513779+Aniruddh25@users.noreply.github.com> Co-authored-by: Aniruddh Munde <anmunde@microsoft.com> Co-authored-by: Souvik Ghosh <souvikofficial04@gmail.com>
…om/ajtiwari07/data-api-builder into add-internal-text-embedding-system
….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>
…om/ajtiwari07/data-api-builder into add-internal-text-embedding-system
## 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>
…om/ajtiwari07/data-api-builder into add-internal-text-embedding-system
…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 ajtiwari07#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 ajtiwari07#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>
…al-text-embedding-system # Conflicts: # src/Service/Startup.cs
prshri-msft
added a commit
to prshri-msft/data-api-builder
that referenced
this pull request
May 22, 2026
…descriptions Stage 3.17 added auto-embed indicators to OpenAPI POST body schemas, GraphQL arguments, and MCP describe-entities. Two paths were missed (found in PR review): 1. REST GET query parameters: AddStoredProcedureInputParameters didn't receive the entity's configured ParameterMetadata, so it couldn't tell which params are auto-embed. Threads the config params through the call chain and augments the parameter description with the auto-embed indicator (mirroring the POST body path). 2. MCP custom tool input schema: BuildInputSchema's parameter description didn't surface auto-embed. Now appends the auto-embed indicator to the description so MCP clients (including LLMs) see that DAB will convert the value to an embedding before execution. The schema type stays the multi-type array — runtime validation (ParameterEmbeddingHelper) rejects non-string inputs to auto-embed params with a clear 400, so the schema doesn't need to encode that constraint. AddStoredProcedureInputParameters is changed from private to internal so a focused unit test can validate the description-augmentation logic without bootstrapping the full DI graph + MSSQL fixture used by the existing OpenApi/StoredProcedureGeneration integration tests. Adds one test per fix. Addresses code-review finding ajtiwari07#3.
prshri-msft
added a commit
to prshri-msft/data-api-builder
that referenced
this pull request
May 22, 2026
…descriptions Stage 3.17 added auto-embed indicators to OpenAPI POST body schemas, GraphQL arguments, and MCP describe-entities. Two paths were missed (found in PR review): 1. REST GET query parameters: AddStoredProcedureInputParameters didn't receive the entity's configured ParameterMetadata, so it couldn't tell which params are auto-embed. Threads the config params through the call chain and augments the parameter description with the auto-embed indicator (mirroring the POST body path). 2. MCP custom tool input schema: BuildInputSchema's parameter description didn't surface auto-embed. Now appends the auto-embed indicator to the description so MCP clients (including LLMs) see that DAB will convert the value to an embedding before execution. The schema type stays the multi-type array — runtime validation (ParameterEmbeddingHelper) rejects non-string inputs to auto-embed params with a clear 400, so the schema doesn't need to encode that constraint. AddStoredProcedureInputParameters is changed from private to internal so a focused unit test can validate the description-augmentation logic without bootstrapping the full DI graph + MSSQL fixture used by the existing OpenApi/StoredProcedureGeneration integration tests. Adds one test per fix. Addresses code-review finding ajtiwari07#3.
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.
Why make this change?
What is this change?
Merge origin/main — resolve Startup.cs conflict (keep embedding telemetry meter)
How was this tested?