[MCP] Bug-fix- aggregate_records always requires groupby

src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs

@@ -88,8 +88,7 @@ public class AggregateRecordsTool : IMcpTool
                         ""orderby"": {
                             ""type"": ""string"",
                             ""enum"": [""asc"", ""desc""],
-                            ""description"": ""Sort grouped results by the aggregated value. Requires groupby."",
-                            ""default"": ""desc""
+                            ""description"": ""Sort direction for grouped results by the aggregated value. Only applies when groupby is provided; ignored otherwise.""
                         },
                         ""having"": {
                             ""type"": ""object"",
@@ -394,23 +393,10 @@ public async Task<CallToolResult> ExecuteAsync(
             // Parse filter
             string? filter = root.TryGetProperty("filter", out JsonElement filterElement) ? filterElement.GetString() : null;
 
-            // Parse orderby
+            // Parse orderby (validation deferred until after groupby is known;
+            // if groupby is absent, orderby is silently ignored per #3279)
             bool userProvidedOrderby = root.TryGetProperty("orderby", out JsonElement orderbyElement) && !string.IsNullOrWhiteSpace(orderbyElement.GetString());
             string orderby = "desc";
-            if (userProvidedOrderby)
-            {
-                string normalizedOrderby = (orderbyElement.GetString() ?? string.Empty).Trim().ToLowerInvariant();
-                if (normalizedOrderby != "asc" && normalizedOrderby != "desc")
-                {
-                    return McpResponseBuilder.BuildErrorResult(
-                        toolName,
-                        "InvalidArguments",
-                        $"Argument 'orderby' must be either 'asc' or 'desc' when provided. Got: '{orderbyElement.GetString()}'.",
-                        logger);
-                }
-
-                orderby = normalizedOrderby;
-            }
 
             // Parse first
             int? first = null;
@@ -443,12 +429,28 @@ public async Task<CallToolResult> ExecuteAsync(
 
             // Validate groupby-dependent parameters
             CallToolResult? dependencyError = ValidateGroupByDependencies(
-                groupby.Count, userProvidedOrderby, first, after, toolName, logger);
+                groupby.Count, ref userProvidedOrderby, first, after, toolName, logger);
             if (dependencyError != null)
             {
                 return dependencyError;
             }
 
+            // Validate orderby value only when groupby is present (orderby is ignored otherwise)
+            if (userProvidedOrderby)
+            {
+                string normalizedOrderby = (orderbyElement.GetString() ?? string.Empty).Trim().ToLowerInvariant();
+                if (normalizedOrderby != "asc" && normalizedOrderby != "desc")
+                {
+                    return McpResponseBuilder.BuildErrorResult(
+                        toolName,
+                        "InvalidArguments",
+                        $"Argument 'orderby' must be either 'asc' or 'desc' when provided. Got: '{orderbyElement.GetString()}'.",
+                        logger);
+                }
+
+                orderby = normalizedOrderby;
+            }
+
             // Parse having clause
             Dictionary<string, double>? havingOperators = null;
             List<double>? havingInValues = null;
@@ -481,24 +483,24 @@ public async Task<CallToolResult> ExecuteAsync(
         }
 
         /// <summary>
-        /// Validates that parameters requiring groupby (orderby, first, after) are only used when groupby is present.
+        /// Validates that parameters requiring groupby (first, after) are only used when groupby is present.
         /// Also validates that 'after' requires 'first'.
+        /// Note: 'orderby' without groupby is silently ignored rather than rejected (see #3279).
         /// </summary>
-        private static CallToolResult? ValidateGroupByDependencies(
+        internal static CallToolResult? ValidateGroupByDependencies(
             int groupbyCount,
-            bool userProvidedOrderby,
+            ref bool userProvidedOrderby,
             int? first,
             string? after,
             string toolName,
             ILogger? logger)
         {
             if (groupbyCount == 0)
             {
-                if (userProvidedOrderby)
-                {
-                    return McpResponseBuilder.BuildErrorResult(toolName, "InvalidArguments",
-                        "The 'orderby' parameter requires 'groupby' to be specified. Sorting applies to grouped aggregation results.", logger);
-                }
+                // Silently ignore orderby when groupby is not provided.
+                // LLMs may send orderby due to schema defaults; this is harmless
+                // since ordering is meaningless without grouping.
+                userProvidedOrderby = false;
 
                 if (first.HasValue)
                 {

src/Service.Tests/Mcp/AggregateRecordsToolTests.cs

@@ -171,8 +171,6 @@ public async Task AggregateRecords_InvalidFieldFunctionCombination_ReturnsInvali
         #region Input Validation Tests - GroupBy Dependencies
 
         [DataTestMethod]
-        [DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"orderby\": \"desc\"}", "groupby",
-            DisplayName = "Orderby without groupby")]
         [DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"having\": {\"gt\": 5}}", "groupby",
             DisplayName = "Having without groupby")]
         [DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"groupby\": [\"title\"], \"orderby\": \"ascending\"}", "'asc' or 'desc'",
@@ -190,6 +188,126 @@ public async Task AggregateRecords_GroupByDependencyViolation_ReturnsInvalidArgu
 
         #endregion
 
+        #region Input Validation Tests - Orderby Without Groupby (Issue #3279)
+
+        /// <summary>
+        /// Verifies that orderby without groupby is silently ignored rather than rejected.
+        /// This is the core fix for https://github.com/Azure/data-api-builder/issues/3279.
+        /// The tool should pass input validation and only fail at metadata resolution (no real DB).
+        /// </summary>
+        [DataTestMethod]
+        [DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"orderby\": \"desc\"}",
+            DisplayName = "count(*) with orderby desc, no groupby")]
+        [DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"orderby\": \"asc\"}",
+            DisplayName = "count(*) with orderby asc, no groupby")]
+        [DataRow("{\"entity\": \"Book\", \"function\": \"avg\", \"field\": \"price\", \"orderby\": \"desc\"}",
+            DisplayName = "avg(price) with orderby desc, no groupby")]
+        [DataRow("{\"entity\": \"Book\", \"function\": \"sum\", \"field\": \"price\", \"orderby\": \"asc\"}",
+            DisplayName = "sum(price) with orderby asc, no groupby")]
+        [DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"orderby\": \"ascending\"}",
+            DisplayName = "Invalid orderby value without groupby is silently ignored")]
+        [DataRow("{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\", \"orderby\": \"garbage\"}",
+            DisplayName = "Nonsense orderby value without groupby is silently ignored")]
+        public async Task AggregateRecords_OrderbyWithoutGroupby_PassesValidation(string json)
+        {
+            IServiceProvider sp = CreateDefaultServiceProvider();
+
+            CallToolResult result = await ExecuteToolAsync(sp, json);
+
+            // The tool may fail at metadata resolution (no real DB), but must NOT fail with InvalidArguments.
+            // If the tool succeeds, that's also acceptable — the test is focused on input validation.
+            if (result.IsError != true)
+            {
+                return;
+            }
+
+            JsonElement content = ParseContent(result);
+            string errorType = content.GetProperty("error").GetProperty("type").GetString()!;
+            Assert.AreNotEqual("InvalidArguments", errorType,
+                $"orderby without groupby must not be rejected as InvalidArguments. Got error type: {errorType}");
+        }
+
+        /// <summary>
+        /// Verifies that simple count(*) without orderby or groupby passes validation.
+        /// </summary>
+        [TestMethod]
+        public async Task AggregateRecords_SimpleCountStar_PassesValidation()
+        {
+            IServiceProvider sp = CreateDefaultServiceProvider();
+
+            CallToolResult result = await ExecuteToolAsync(sp,
+                "{\"entity\": \"Book\", \"function\": \"count\", \"field\": \"*\"}");
+
+            // If the tool succeeds, that's acceptable — the test is focused on input validation.
+            if (result.IsError != true)
+            {
+                return;
+            }
+
+            JsonElement content = ParseContent(result);
+            string errorType = content.GetProperty("error").GetProperty("type").GetString()!;
+            Assert.AreNotEqual("InvalidArguments", errorType,
+                "Simple count(*) must pass input validation.");
+        }
+
+        /// <summary>
+        /// Verifies ValidateGroupByDependencies directly: orderby is silently cleared
+        /// when groupby count is 0.
+        /// </summary>
+        [TestMethod]
+        public void ValidateGroupByDependencies_OrderbyWithoutGroupby_ClearsOrderbyFlag()
+        {
+            bool userProvidedOrderby = true;
+            CallToolResult? result = AggregateRecordsTool.ValidateGroupByDependencies(
+                groupbyCount: 0,
+                userProvidedOrderby: ref userProvidedOrderby,
+                first: null,
+                after: null,
+                toolName: "aggregate_records",
+                logger: null);
+
+            Assert.IsNull(result, "No error should be returned when orderby is provided without groupby.");
+            Assert.IsFalse(userProvidedOrderby, "userProvidedOrderby must be set to false when groupby is absent.");
+        }
+
+        /// <summary>
+        /// Verifies ValidateGroupByDependencies preserves orderby when groupby is present.
+        /// </summary>
+        [TestMethod]
+        public void ValidateGroupByDependencies_OrderbyWithGroupby_PreservesOrderbyFlag()
+        {
+            bool userProvidedOrderby = true;
+            CallToolResult? result = AggregateRecordsTool.ValidateGroupByDependencies(
+                groupbyCount: 2,
+                userProvidedOrderby: ref userProvidedOrderby,
+                first: null,
+                after: null,
+                toolName: "aggregate_records",
+                logger: null);
+
+            Assert.IsNull(result, "No error should be returned when both orderby and groupby are provided.");
+            Assert.IsTrue(userProvidedOrderby, "userProvidedOrderby must remain true when groupby is present.");
+        }
+
+        /// <summary>
+        /// Verifies that the orderby schema property has no default value (fix for #3279).
+        /// </summary>
+        [TestMethod]
+        public void GetToolMetadata_OrderbySchemaHasNoDefault()
+        {
+            AggregateRecordsTool tool = new();
+            Tool metadata = tool.GetToolMetadata();
+
+            JsonElement properties = metadata.InputSchema.GetProperty("properties");
+            JsonElement orderbyProp = properties.GetProperty("orderby");
+
+            Assert.IsFalse(orderbyProp.TryGetProperty("default", out _),
+                "The 'orderby' schema property must not have a default value. " +
+                "A default causes LLMs to always send orderby, which previously forced groupby to be required.");
+        }
+
+        #endregion
+
         #region Input Validation Tests - Having Clause
 
         [DataTestMethod]