diff --git a/CHANGELOG.md b/CHANGELOG.md index ac1e73835..c4b9aaa95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - AI Chat: Gemini provider now sends tool schemas with `additionalProperties` stripped and optional fields rewritten from `type: [X, null]` to `type: X, nullable: true`, fixing 400 errors when sending a message with tools enabled. - AI Chat: Gemini provider now round-trips `thoughtSignature` on function calls, fixing the second-round 400 error after a tool runs. +- AI Chat: GitHub Copilot tool registration was failing with "Expected string" schema validation errors. Optional fields now register with `type: "string"` instead of `type: ["string", "null"]` and are excluded from `required`, which Copilot's LSP validator accepts. - MySQL/MariaDB: `BIT(N)` columns now display as decimal numbers (`0`, `1`, `255`) instead of raw bytes that showed up as control characters like `^A` in the data grid. (#1272) - ClickHouse, BigQuery, CloudflareD1, LibSQL, Etcd, and DynamoDB: long-running queries no longer fail at 30 seconds when Settings > Query timeout is set higher. The HTTP transport now uses the configured query timeout plus a 30-second grace, so the server's `max_execution_time` (or equivalent) fires before the client gives up. Setting "No limit" raises the transport ceiling to 1 hour. (#1267) - AI Chat: DeepSeek V4 thinking content (`reasoning_content`) is now captured during streaming and passed back in subsequent turns, fixing 400 errors when using deepseek-v4-pro or deepseek-v4-flash. diff --git a/TablePro/Core/AI/Chat/ChatToolSpec+Copilot.swift b/TablePro/Core/AI/Chat/ChatToolSpec+Copilot.swift index cfdb991f6..53a8de56c 100644 --- a/TablePro/Core/AI/Chat/ChatToolSpec+Copilot.swift +++ b/TablePro/Core/AI/Chat/ChatToolSpec+Copilot.swift @@ -10,15 +10,81 @@ extension ChatToolSpec { CopilotLanguageModelToolInformation( name: name, description: description, - inputSchema: Self.normalizeForCopilot(inputSchema) + inputSchema: Self.sanitizeForCopilot(inputSchema) ) } - private static func normalizeForCopilot(_ schema: JsonValue) -> JsonValue { - guard case .object(var dict) = schema else { return schema } - if dict["required"] == nil { - dict["required"] = .array([]) + /// Copilot's LSP schema validator rejects `type: [X, "null"]` arrays. Every + /// `type` field must be a single string. Convert nullable scalars to plain + /// scalars and drop the property from the parent's `required` array so the + /// model knows it can omit the field. Recurses into nested objects and array + /// items. + static func sanitizeForCopilot(_ schema: JsonValue) -> JsonValue { + switch schema { + case .object(let fields): + return sanitizeObject(fields) + case .array(let items): + return .array(items.map(sanitizeForCopilot)) + default: + return schema } - return .object(dict) + } + + private static func sanitizeObject(_ fields: [String: JsonValue]) -> JsonValue { + var nullableKeys: Set = [] + var rewritten: [String: JsonValue] = [:] + + for (key, value) in fields { + if key == "properties", case .object(let props) = value { + var cleanedProps: [String: JsonValue] = [:] + for (propName, propValue) in props { + let (cleaned, wasNullable) = stripNullableType(propValue) + if wasNullable { + nullableKeys.insert(propName) + } + cleanedProps[propName] = sanitizeForCopilot(cleaned) + } + rewritten[key] = .object(cleanedProps) + } else { + rewritten[key] = sanitizeForCopilot(value) + } + } + + if !nullableKeys.isEmpty, case .array(let required) = rewritten["required"] { + let filtered = required.filter { entry in + if case .string(let name) = entry { return !nullableKeys.contains(name) } + return true + } + rewritten["required"] = .array(filtered) + } + + return .object(rewritten) + } + + /// Rewrites `type: [X, "null"]` to `type: X` on a property schema. + /// Also strips `null` from `enum` arrays if present. + /// Returns the cleaned schema and whether the original was nullable. + private static func stripNullableType(_ schema: JsonValue) -> (JsonValue, Bool) { + guard case .object(var fields) = schema, + case .array(let typeMembers) = fields["type"] + else { + return (schema, false) + } + + let nullCount = typeMembers.filter { $0 == .string("null") }.count + guard nullCount > 0 else { return (schema, false) } + + let nonNull = typeMembers.filter { $0 != .string("null") } + if nonNull.count == 1, let primary = nonNull.first { + fields["type"] = primary + } else { + fields["type"] = .array(nonNull) + } + + if case .array(let enumMembers) = fields["enum"] { + fields["enum"] = .array(enumMembers.filter { $0 != .null }) + } + + return (.object(fields), true) } } diff --git a/TableProTests/Core/AI/CopilotSchemaSanitizationTests.swift b/TableProTests/Core/AI/CopilotSchemaSanitizationTests.swift new file mode 100644 index 000000000..963f4f75e --- /dev/null +++ b/TableProTests/Core/AI/CopilotSchemaSanitizationTests.swift @@ -0,0 +1,165 @@ +// +// CopilotSchemaSanitizationTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("Copilot schema sanitization") +struct CopilotSchemaSanitizationTests { + @Test("Converts type:[X,null] to type:X and drops the field from required") + func rewritesOptionalScalar() { + let input = JsonValue.object([ + "type": .string("object"), + "properties": .object([ + "schema": .object([ + "type": .array([.string("string"), .string("null")]), + "description": .string("optional") + ]) + ]), + "required": .array([.string("schema")]) + ]) + let output = ChatToolSpec.sanitizeForCopilot(input) + guard case .object(let root) = output, + case .object(let props) = root["properties"], + case .object(let schemaField) = props["schema"] else { + Issue.record("expected nested object") + return + } + #expect(schemaField["type"] == .string("string")) + #expect(root["required"] == .array([])) + } + + @Test("Preserves non-nullable scalars in required") + func preservesRequiredFields() { + let input = JsonValue.object([ + "type": .string("object"), + "properties": .object([ + "connection_id": .object([ + "type": .string("string"), + "description": .string("UUID") + ]) + ]), + "required": .array([.string("connection_id")]) + ]) + let output = ChatToolSpec.sanitizeForCopilot(input) + guard case .object(let root) = output else { Issue.record("expected object"); return } + #expect(root["required"] == .array([.string("connection_id")])) + } + + @Test("Strips null from enum when type was nullable") + func stripsNullFromEnum() { + let input = JsonValue.object([ + "type": .string("object"), + "properties": .object([ + "tier": .object([ + "type": .array([.string("string"), .string("null")]), + "enum": .array([.string("a"), .string("b"), .null]) + ]) + ]), + "required": .array([.string("tier")]) + ]) + let output = ChatToolSpec.sanitizeForCopilot(input) + guard case .object(let root) = output, + case .object(let props) = root["properties"], + case .object(let tier) = props["tier"] else { + Issue.record("expected tier object") + return + } + #expect(tier["type"] == .string("string")) + #expect(tier["enum"] == .array([.string("a"), .string("b")])) + } + + @Test("Mixed required and optional drop only the nullable") + func mixedRequiredAndOptional() { + let input = JsonValue.object([ + "type": .string("object"), + "properties": .object([ + "connection_id": .object([ + "type": .string("string") + ]), + "schema": .object([ + "type": .array([.string("string"), .string("null")]) + ]) + ]), + "required": .array([.string("connection_id"), .string("schema")]) + ]) + let output = ChatToolSpec.sanitizeForCopilot(input) + guard case .object(let root) = output, + case .array(let required) = root["required"] else { + Issue.record("expected required array") + return + } + #expect(required == [.string("connection_id")]) + } + + @Test("Recurses into nested object properties") + func recursesIntoNested() { + let input = JsonValue.object([ + "type": .string("object"), + "properties": .object([ + "filter": .object([ + "type": .string("object"), + "properties": .object([ + "name": .object([ + "type": .array([.string("string"), .string("null")]) + ]) + ]), + "required": .array([.string("name")]) + ]) + ]), + "required": .array([.string("filter")]) + ]) + let output = ChatToolSpec.sanitizeForCopilot(input) + guard case .object(let root) = output, + case .object(let props) = root["properties"], + case .object(let filter) = props["filter"], + case .object(let nestedProps) = filter["properties"], + case .object(let nameField) = nestedProps["name"] else { + Issue.record("expected nested name field") + return + } + #expect(nameField["type"] == .string("string")) + #expect(filter["required"] == .array([])) + } + + @Test("Real ChatToolSchemaBuilder output passes through Copilot validator shape") + func realBuilderOutputIsValid() throws { + // Simulates what ListTablesChatTool produces. + let realSchema = ChatToolSchemaBuilder.object( + properties: [ + "connection_id": ChatToolSchemaBuilder.connectionId, + "database": ChatToolSchemaBuilder.string( + description: "Database name. Omit to use current.", + optional: true + ), + "schema": ChatToolSchemaBuilder.schemaName + ] + ) + let sanitized = ChatToolSpec.sanitizeForCopilot(realSchema) + guard case .object(let root) = sanitized, + case .object(let props) = root["properties"] else { + Issue.record("expected sanitized object") + return + } + // Every type field at the property level should be a single string. + for (_, value) in props { + guard case .object(let field) = value else { continue } + if case .array = field["type"] { + Issue.record("type should not be an array after sanitization") + } + } + // Optional fields must be removed from required. + if case .array(let required) = root["required"] { + let names = required.compactMap { val -> String? in + if case .string(let name) = val { return name } + return nil + } + #expect(!names.contains("database")) + #expect(!names.contains("schema")) + #expect(names.contains("connection_id")) + } + } +}