From 38bb1231405ab8d87061c9cd973c1d37e4b1a6b5 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 13 Oct 2026 13:25:28 -0500 Subject: [PATCH] fix: stop sending every tool strict; surface provider error reasons Every kof22 turn failed on its first model call with Anthropic 400 invalid_request_error "The compiled grammar is too large ... reduce the number of strict tools". All ten read tools were strict, and the API compiles every strict schema of a request into one grammar. - Tools are not strict by default; hosts opt in with strictToolNames. The kernel validates every tool input (INVALID_INPUT) anyway. - If a provider still refuses strict schemas, the turn retries once without strict tools and logs a warning. - Provider errors carry the API error type, a sanitized message and the request id: logged at WARN on 4xx, request id kept on the usage row, the reason shown to assistant.admin holders and in Test connection. - TurnEngine also catches Errors, so a turn never ends silently. - AnthropicRequestContractTest checks the exact request against the Messages API constraints and a conservative strict-tool budget. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 + docs/DESIGN.md | 29 ++ .../provider/anthropic/AnthropicMapping.java | 53 ++- .../anthropic/AnthropicModelProvider.java | 16 +- .../anthropic/AnthropicModelProviderTest.java | 5 +- .../AnthropicRequestContractTest.java | 443 ++++++++++++++++++ .../resources/fixtures/grammar-too-large.json | 1 + .../qbits/assistant/AssistantQBitConfig.java | 40 ++ .../assistant/engine/ContextBuilder.java | 14 +- .../qbits/assistant/engine/TurnEngine.java | 45 +- .../qbits/assistant/engine/UsageRecorder.java | 17 +- .../provider/ModelProviderException.java | 94 ++++ .../assistant/engine/TurnEngineTest.java | 6 +- 13 files changed, 763 insertions(+), 11 deletions(-) create mode 100644 qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicRequestContractTest.java create mode 100644 qbit-assistant-provider-anthropic/src/test/resources/fixtures/grammar-too-large.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 26f9639..909b711 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ Phase 0/1 of the QQQ Assistant: read-only Q&A and navigation. Requires QQQ 4.1 w - the AI tool kernel (`qqq-backend-core` `actions.ai`, QRun-IO/qqq#1078) - the developer-panel contract (QRun-IO/qqq#1076). +### Fixed (0.1.0-RC.5) +- **Every turn failed with "The AI service could not answer this request."** Anthropic refused the request with 400 `invalid_request_error` "The compiled grammar is too large ... reduce the number of strict tools": all ten read tools were sent with `strict: true`, and the API compiles every strict schema of a request into one grammar. Tools are now **not strict by default**. The kernel already validates every tool input and answers `INVALID_INPUT`. A host can opt individual tools in with `withStrictToolNames(...)`. +- If a provider still refuses the strict schemas, the turn **retries once without strict tools** and logs a warning (`ModelProviderException.isStrictSchemaRejected`). +- Provider errors now carry the **API's own error type, a sanitized message** (one line, capped, key-like tokens redacted) and the **request id**: + - they are logged at WARN for 4xx; + - they are recorded in the `assistant_usage` row's request id; + - they are shown to `assistant.admin` holders in the panel and in Test connection. + Before this, a 400 was logged as "INVALID_REQUEST httpStatus=400" with no reason. +- A turn that hits a JVM `Error` (for example a linkage problem) now ends as `ERROR` with a log line, instead of being swallowed by the executor and ending as an empty `COMPLETE` answer. +- `AnthropicRequestContractTest` checks the exact request against the Messages API constraints and a conservative strict-tool budget. + ### Added (0.1.0-RC.4) - **Provider settings** (`assistant_provider`): type (registered `AssistantProviderType`s; `AnthropicProviderType` in the Anthropic module), model, enabled, default, base URL override, monthly cost cap, max output tokens per request, max concurrent requests, and the last connection test. The default enabled row is the active provider; with no rows the provider registered in code is used, so existing hosts keep working. - **Key handling:** the key is never stored. A row names the environment variable that holds it (`apiKeyEnvName`, allow-listed); `apiKeyStatus` is computed on read (`Set • …abcd`) and dropped on save. A pasted key, an unrelated variable or an unlisted base URL is refused. New config: `withProviderType`, `allowedApiKeyEnvNames`, `allowedBaseUrls`, `withSecretResolver`, `providerTestTimeoutSeconds`. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index cae7a1a..3e4caa9 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -75,3 +75,32 @@ One HTML widget (`assistantHome`) on the Assistant app. It is the only place thi Every section has an empty state; nothing is invented. The nav is the home plus the main tables: Providers, Conversations, Model prices, Policy. Messages, turns, tool calls, feedback and model calls are child lists on a conversation, and model calls also on a provider. Processes are actions on their tables. + +## Strict tools (0.1.0-RC.5) + +**Incident (kof22, 2026-10-13).** Every turn failed on its first model call with 400 `invalid_request_error`: "The compiled grammar is too large, which would cause performance issues. Simplify your tool schemas or reduce the number of strict tools." +- The request sent all ten read tools with `strict: true`. +- Anthropic compiles every strict schema in a request into one grammar. +- The request was inside every documented limit: 20 strict tools, 24 optional parameters, 16 union parameters. The real limit is not published. +- The log line only said `INVALID_REQUEST httpStatus=400`, so the reason was invisible. + +**Decision.** +- **No tool is strict by default.** `AssistantQBitConfig.strictToolNames` opts tools in. +- Strict mode only guaranteed schema-valid input. The kernel already validates every input and returns `INVALID_INPUT` results, and the UI-API tools (below) go through the same validation as the UI. +- If a provider still refuses strict schemas (`ModelProviderException.isStrictSchemaRejected`), the turn retries once with no strict tools and logs a WARN. + +**Budget.** +- `AnthropicRequestContractTest` fails the build when the strict tools of the default request exceed 3 tools, 6 optional parameters or 2 union parameters. These budgets are far below the shape that failed. +- It also checks the documented request rules: + - adaptive thinking without `budget_tokens`, and no sampling parameters; + - a valid effort, and no forced `tool_choice`; + - at most 4 cache breakpoints, with longer TTLs first; + - a user-first, user-last message order; + - strict-schema keywords. + +**Real-API check (dev).** `ANTHROPIC_API_KEY=... ./mvnw verify -Dassistant.live=true -Dtest.excludedGroups= -Dgroups=live -Djacoco.skip=true -pl qbit-assistant-provider-anthropic -am` runs the live golden evals. They send exactly this request shape. + +**Diagnostics.** On a 4xx the provider's error type, sanitized message and request id are: +- logged at WARN; +- recorded as the `assistant_usage` request id; +- shown to `assistant.admin` holders, in the panel's error and in Test connection. diff --git a/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicMapping.java b/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicMapping.java index 5955cc7..743da2c 100644 --- a/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicMapping.java +++ b/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicMapping.java @@ -313,8 +313,51 @@ public class AnthropicMapping /******************************************************************************* - ** Map an SDK error to a provider error. The message holds the status and - ** the API error type only - never request or response bodies. + ** Whether a 400 message says the strict tool schemas are the problem: + ** "The compiled grammar is too large ... reduce the number of strict tools" + ** or "Schema is too complex for compilation". + *******************************************************************************/ + static boolean isStrictSchemaMessage(String message) + { + if(message == null) + { + return (false); + } + String lower = message.toLowerCase(java.util.Locale.ROOT); + return (lower.contains("grammar") || lower.contains("too complex for compilation") || lower.contains("strict tool")); + } + + + + /******************************************************************************* + ** The API's own error message ({"error": {"message": ...}}), or null. The + ** caller sanitizes it; Anthropic's messages name parameters, not content. + *******************************************************************************/ + static String errorMessage(AnthropicServiceException service) + { + try + { + Map body = service.body() == null ? null : service.body().convert(new TypeReference>() {}); + if(body != null && body.get("error") instanceof Map error && error.get("message") instanceof String message) + { + return (message); + } + } + catch(RuntimeException e) + { + ////////////////////////////////////////////// + // not JSON / not the documented error shape // + ////////////////////////////////////////////// + } + return (null); + } + + + + /******************************************************************************* + ** Map an SDK error to a provider error. The message holds the status, the + ** API error type and the API's own (sanitized) error message - never request + ** bodies, prompts or keys. *******************************************************************************/ public static ModelProviderException toProviderException(Throwable throwable) { @@ -348,7 +391,11 @@ public class AnthropicMapping { kind = "overloaded_error".equals(type) ? ModelProviderException.Kind.OVERLOADED : ModelProviderException.Kind.OTHER; } - return (new ModelProviderException(kind, status >= 400 ? status : null, "Anthropic API error" + (status >= 400 ? " (HTTP " + status + ")" : "") + (type == null ? "" : " " + type))); + String apiMessage = errorMessage(service); + String requestId = service.headers() == null ? null : service.headers().values("request-id").stream().findFirst().orElse(null); + ModelProviderException exception = new ModelProviderException(kind, status >= 400 ? status : null, "Anthropic API error" + (status >= 400 ? " (HTTP " + status + ")" : "") + (type == null ? "" : " " + type) + + (apiMessage == null ? "" : ": " + ModelProviderException.sanitizeProviderMessage(apiMessage, ModelProviderException.MAX_PROVIDER_MESSAGE_CHARS))); + return (exception.withProviderDetails(type, apiMessage, requestId).withStrictSchemaRejected(status == 400 && isStrictSchemaMessage(apiMessage))); } if(throwable instanceof AnthropicIoException) { diff --git a/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProvider.java b/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProvider.java index a5ec839..7808034 100644 --- a/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProvider.java +++ b/qbit-assistant-provider-anthropic/src/main/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProvider.java @@ -297,7 +297,21 @@ public class AnthropicModelProvider implements AssistantModelProvider if(!handle.isCancelled()) { ModelProviderException exception = AnthropicMapping.toProviderException(t).withPartialUsage(seen.get()); - LOG.info("Anthropic stream failed", logPair("kind", exception.getKind()), logPair("httpStatus", exception.getHttpStatus()), logPair("model", request.getModel())); + /////////////////////////////////////////////////////////////////// + // a 4xx is a request or account problem an operator must see: // + // log the API's error type, its (sanitized) message and the // + // request id - never the prompt // + /////////////////////////////////////////////////////////////////// + if(exception.getHttpStatus() != null && exception.getHttpStatus() >= 400 && exception.getHttpStatus() < 500) + { + LOG.warn("Anthropic stream failed", logPair("kind", exception.getKind()), logPair("httpStatus", exception.getHttpStatus()), logPair("model", request.getModel()), + logPair("errorType", exception.getProviderErrorType()), logPair("errorMessage", exception.getProviderMessage()), logPair("requestId", exception.getRequestId())); + } + else + { + LOG.info("Anthropic stream failed", logPair("kind", exception.getKind()), logPair("httpStatus", exception.getHttpStatus()), logPair("model", request.getModel()), + logPair("errorType", exception.getProviderErrorType()), logPair("requestId", exception.getRequestId())); + } handle.fail(exception); } } diff --git a/qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProviderTest.java b/qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProviderTest.java index 1036268..0043517 100644 --- a/qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProviderTest.java +++ b/qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicModelProviderTest.java @@ -265,7 +265,10 @@ class AnthropicModelProviderTest { assertThat(e.getKind()).isEqualTo(ModelProviderException.Kind.AUTHENTICATION); assertThat(e.getHttpStatus()).isEqualTo(401); - assertThat(e.getMessage()).doesNotContain(TEST_KEY).doesNotContain("invalid x-api-key"); + assertThat(e.getMessage()).doesNotContain(TEST_KEY).contains("authentication_error"); + assertThat(e.getProviderErrorType()).isEqualTo("authentication_error"); + assertThat(e.getProviderMessage()).isEqualTo("invalid x-api-key"); + assertThat(e.isStrictSchemaRejected()).isFalse(); }); assertThatThrownBy(() -> provider.stream(request(), t -> { }).await(Duration.ofSeconds(10))) diff --git a/qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicRequestContractTest.java b/qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicRequestContractTest.java new file mode 100644 index 0000000..287af88 --- /dev/null +++ b/qbit-assistant-provider-anthropic/src/test/java/com/kingsrook/qbits/assistant/provider/anthropic/AnthropicRequestContractTest.java @@ -0,0 +1,443 @@ +/* + * Copyright 2026 Kingsrook, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.kingsrook.qbits.assistant.provider.anthropic; + + +import java.time.Duration; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import com.anthropic.core.ObjectMappers; +import com.fasterxml.jackson.core.type.TypeReference; +import com.kingsrook.qbits.assistant.AssistantQBitConfig; +import com.kingsrook.qbits.assistant.provider.ModelProviderException; +import com.kingsrook.qbits.assistant.provider.ModelRequest; +import com.kingsrook.qbits.assistant.provider.ModelToolDefinition; +import com.kingsrook.qbits.assistant.provider.fake.ScriptedModelProvider; +import com.kingsrook.qbits.assistant.testing.AssistantTestFixture; +import com.kingsrook.qbits.assistant.testing.TurnRunner; +import com.kingsrook.qqq.backend.core.context.QContext; +import com.kingsrook.qqq.backend.core.utils.JsonUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + + +/******************************************************************************* + ** The exact Messages API request the turn engine sends, checked against the + ** API's documented constraints, plus a conservative budget for strict tools. + ** + ** Why the budget: Anthropic compiles every strict tool schema of a request + ** into one grammar and refuses the request when that grammar is too large + ** (400 invalid_request_error "The compiled grammar is too large ... reduce the + ** number of strict tools"). With all ten read tools strict (0.1.0-RC.3 to + ** RC.4) every kof22 turn failed that way, though the request stayed inside + ** the documented limits (20 strict tools, 24 optional parameters, 16 union + ** parameters). The real limit is not published, so the budget below is far + ** under the shape that failed (10 tools / 12 optional / 3 unions). A real-API + ** check is in docs/DESIGN.md ("Strict tools") for dev use with a key. + *******************************************************************************/ +class AnthropicRequestContractTest +{ + ///////////////////////////////////////////////////////////////////// + // conservative strict-tool budget (see the class comment) // + ///////////////////////////////////////////////////////////////////// + static final int MAX_STRICT_TOOLS = 3; + static final int MAX_STRICT_OPTIONAL_PARAMS = 6; + static final int MAX_STRICT_UNION_PARAMS = 2; + + private static final Set EFFORTS = Set.of("low", "medium", "high", "xhigh", "max"); + private static final Set UNSUPPORTED_STRICT = Set.of("minimum", "maximum", "exclusiveMinimum", "exclusiveMaximum", "multipleOf", "minLength", "maxLength", + "maxItems", "uniqueItems", "oneOf", "not", "if", "then", "else", "patternProperties"); + + + + /******************************************************************************* + ** + *******************************************************************************/ + @AfterEach + void tearDown() + { + QContext.clear(); + } + + + + /******************************************************************************* + ** The request body for the fixture app, as the SDK serializes it. + *******************************************************************************/ + static Map requestBody(AssistantQBitConfig config) throws Exception + { + QContext.init(AssistantTestFixture.build(config), AssistantTestFixture.sessionFor(AssistantTestFixture.ADMIN)); + ModelRequest request = config.getRuntime().getContextBuilder().build(QContext.getQSession(), config.getModels().getMainModel(), config.getModels().getMainEffort(), + List.of(), "How many published posts are there?", Map.of("tableName", "blog_post")); + String json = ObjectMappers.jsonMapper().writeValueAsString(AnthropicMapping.toParams(request)._body()); + return (JsonUtils.toObject(json, new TypeReference>() {})); + } + + + + /******************************************************************************* + ** The default request meets every documented constraint and the budget. + *******************************************************************************/ + @Test + void testDefaultRequestMeetsTheApiContract() throws Exception + { + Map body = requestBody(new AssistantQBitConfig().withModelProvider(new ScriptedModelProvider())); + assertThat(violations(body)).isEmpty(); + assertThat(strictBudgetViolations(body)).isEmpty(); + assertThat(tools(body)).hasSizeGreaterThanOrEqualTo(8).allSatisfy(tool -> assertThat(tool.get("strict")).isNotEqualTo(true)); + } + + + + /******************************************************************************* + ** The RC.3 shape (every tool strict) is recognized as over budget, and a + ** host can still opt a few tools in. + *******************************************************************************/ + @Test + void testStrictBudget() throws Exception + { + AssistantQBitConfig probe = new AssistantQBitConfig().withModelProvider(new ScriptedModelProvider()); + Set all = new LinkedHashSet<>(); + requestBody(probe); + for(ModelToolDefinition tool : probe.getRuntime().getContextBuilder().tools()) + { + all.add(tool.getName()); + } + QContext.clear(); + + Map rc3 = requestBody(new AssistantQBitConfig().withModelProvider(new ScriptedModelProvider()).withStrictToolNames(all)); + assertThat(violations(rc3)).isEmpty(); + assertThat(strictBudgetViolations(rc3)).isNotEmpty(); + QContext.clear(); + + Map few = requestBody(new AssistantQBitConfig().withModelProvider(new ScriptedModelProvider()).withStrictToolNames(Set.of("get_record", "describe_table"))); + assertThat(violations(few)).isEmpty(); + assertThat(strictBudgetViolations(few)).isEmpty(); + assertThat(tools(few).stream().filter(t -> Boolean.TRUE.equals(t.get("strict"))).map(t -> t.get("name"))).containsExactlyInAnyOrder("get_record", "describe_table"); + } + + + + /******************************************************************************* + ** A 400 about the grammar is flagged as a strict-schema rejection, carries + ** the API's message and request id, and never the key. + *******************************************************************************/ + @Test + void testGrammarRejectionIsRecognized() throws Exception + { + try(RecordedAnthropicServer server = new RecordedAnthropicServer()) + { + server.thenError(400, "grammar-too-large.json"); + AnthropicModelProvider provider = AnthropicModelProvider.builder().apiKey("sk-ant-test-0000000000000000000000").baseUrl(server.baseUrl()).maxRetries(0).build(); + assertThatThrownBy(() -> provider.stream(AnthropicModelProviderTest.request(), t -> { }).await(Duration.ofSeconds(10))) + .isInstanceOfSatisfying(ModelProviderException.class, e -> + { + assertThat(e.getKind()).isEqualTo(ModelProviderException.Kind.INVALID_REQUEST); + assertThat(e.isStrictSchemaRejected()).isTrue(); + assertThat(e.getProviderErrorType()).isEqualTo("invalid_request_error"); + assertThat(e.getProviderMessage()).startsWith("The compiled grammar is too large"); + assertThat(e.getMessage()).contains("compiled grammar").doesNotContain("sk-ant-test"); + }); + provider.close(); + } + } + + + + /******************************************************************************* + ** End to end: when the provider refuses strict schemas, the turn retries + ** once without strict tools and answers. + *******************************************************************************/ + @Test + void testTurnRetriesWithoutStrictTools() throws Exception + { + try(RecordedAnthropicServer server = new RecordedAnthropicServer()) + { + server.thenError(400, "grammar-too-large.json").thenStream("text-with-thinking.sse", 0); + AnthropicModelProvider provider = AnthropicModelProvider.builder().apiKey("sk-ant-test-0000000000000000000000").baseUrl(server.baseUrl()).maxRetries(0).build(); + AssistantQBitConfig config = new AssistantQBitConfig().withModelProvider(provider).withStrictToolNames(Set.of("get_record", "count_records", "query_records")); + TurnRunner runner = new TurnRunner(AssistantTestFixture.build(config), config); + + TurnRunner.Outcome outcome = runner.run(AssistantTestFixture.EDITOR, "How many published posts are there?"); + assertThat(outcome.end().get("status")).isEqualTo("COMPLETE"); + assertThat(server.getRequests()).hasSize(2); + assertThat(server.getRequests().get(0).get("body")).contains("\"strict\":true"); + assertThat(server.getRequests().get(1).get("body")).doesNotContain("\"strict\":true"); + provider.close(); + } + } + + + + /******************************************************************************* + ** Documented request constraints (Messages API, claude-sonnet-5-5 family). + *******************************************************************************/ + @SuppressWarnings("unchecked") + static List violations(Map body) + { + List errors = new ArrayList<>(); + if(!(body.get("model") instanceof String model) || !model.matches("claude-[a-z]+-[0-9]+(-[0-9]+)?")) + { + errors.add("model id " + body.get("model")); + } + if(!(body.get("max_tokens") instanceof Number max) || max.intValue() < 1 || max.intValue() > 128_000) + { + errors.add("max_tokens " + body.get("max_tokens")); + } + for(String sampling : List.of("temperature", "top_p", "top_k")) + { + if(body.containsKey(sampling)) + { + errors.add(sampling + " is rejected with thinking on current models"); + } + } + Map thinking = (Map) body.get("thinking"); + if(thinking != null && (!"adaptive".equals(thinking.get("type")) || thinking.containsKey("budget_tokens"))) + { + errors.add("thinking must be adaptive without budget_tokens: " + thinking); + } + Map outputConfig = (Map) body.get("output_config"); + if(outputConfig != null && outputConfig.get("effort") != null && !EFFORTS.contains(outputConfig.get("effort"))) + { + errors.add("effort " + outputConfig.get("effort")); + } + Map toolChoice = (Map) body.get("tool_choice"); + if(toolChoice != null && !"auto".equals(toolChoice.get("type")) && !"none".equals(toolChoice.get("type"))) + { + errors.add("forced tool_choice is rejected: " + toolChoice); + } + + /////////////////////////////////////////////////////////////////// + // cache breakpoints: at most 4, longer TTLs before shorter ones // + /////////////////////////////////////////////////////////////////// + List ttls = new ArrayList<>(); + collectCacheTtls(body.get("tools"), ttls); + collectCacheTtls(body.get("system"), ttls); + collectCacheTtls(body.get("messages"), ttls); + if(ttls.size() > 4) + { + errors.add("more than 4 cache breakpoints: " + ttls); + } + for(int i = 1; i < ttls.size(); i++) + { + if("1h".equals(ttls.get(i)) && "5m".equals(ttls.get(i - 1))) + { + errors.add("a 1h cache entry after a 5m one: " + ttls); + } + } + + List> messages = (List>) body.get("messages"); + if(messages == null || messages.isEmpty() || !"user".equals(messages.get(0).get("role"))) + { + errors.add("messages must start with a user message"); + } + else if(!"user".equals(messages.get(messages.size() - 1).get("role"))) + { + errors.add("no assistant prefill on current models"); + } + for(Map message : messages == null ? List.>of() : messages) + { + for(Map block : (List>) message.get("content")) + { + if("text".equals(block.get("type")) && (!(block.get("text") instanceof String text) || text.isBlank())) + { + errors.add("empty text block"); + } + } + } + + Set names = new LinkedHashSet<>(); + for(Map tool : tools(body)) + { + String name = String.valueOf(tool.get("name")); + if(!name.matches("[a-zA-Z0-9_-]{1,64}") || !names.add(name)) + { + errors.add("tool name " + name); + } + Map schema = (Map) tool.get("input_schema"); + if(schema == null || !"object".equals(schema.get("type"))) + { + errors.add(name + ": input_schema must be an object schema"); + continue; + } + if(Boolean.TRUE.equals(tool.get("strict"))) + { + checkStrictSchema(name, schema, errors); + } + } + return (errors); + } + + + + /******************************************************************************* + ** The strict-tool budget: count, optional parameters and unions across all + ** strict schemas. + *******************************************************************************/ + @SuppressWarnings("unchecked") + static List strictBudgetViolations(Map body) + { + int[] counts = new int[2]; + int strict = 0; + for(Map tool : tools(body)) + { + if(Boolean.TRUE.equals(tool.get("strict"))) + { + strict++; + countOptionalAndUnions(tool.get("input_schema"), counts); + } + } + List errors = new ArrayList<>(); + if(strict > MAX_STRICT_TOOLS) + { + errors.add(strict + " strict tools (budget " + MAX_STRICT_TOOLS + ")"); + } + if(counts[0] > MAX_STRICT_OPTIONAL_PARAMS) + { + errors.add(counts[0] + " optional parameters in strict tools (budget " + MAX_STRICT_OPTIONAL_PARAMS + ")"); + } + if(counts[1] > MAX_STRICT_UNION_PARAMS) + { + errors.add(counts[1] + " union parameters in strict tools (budget " + MAX_STRICT_UNION_PARAMS + ")"); + } + return (errors); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @SuppressWarnings("unchecked") + private static List> tools(Map body) + { + return (body.get("tools") instanceof List list ? (List>) list : List.of()); + } + + + + /******************************************************************************* + ** Strict schemas: additionalProperties false on every object, no + ** unsupported keywords, minItems 0 or 1 only, no recursive $ref. + *******************************************************************************/ + @SuppressWarnings("unchecked") + private static void checkStrictSchema(String path, Object node, List errors) + { + if(node instanceof Map map) + { + Map schema = (Map) map; + if(("object".equals(schema.get("type")) || schema.containsKey("properties")) && !Boolean.FALSE.equals(schema.get("additionalProperties"))) + { + errors.add(path + ": additionalProperties must be false"); + } + for(String keyword : UNSUPPORTED_STRICT) + { + if(schema.containsKey(keyword)) + { + errors.add(path + ": unsupported keyword " + keyword); + } + } + if(schema.get("minItems") instanceof Number minItems && minItems.intValue() > 1) + { + errors.add(path + ": minItems above 1"); + } + if(schema.get("$defs") instanceof Map defs) + { + for(Map.Entry def : defs.entrySet()) + { + if(JsonUtils.toJson(def.getValue()).contains("\"#/$defs/" + def.getKey() + "\"")) + { + errors.add(path + ": recursive $defs/" + def.getKey()); + } + } + } + schema.forEach((key, value) -> checkStrictSchema(path + "/" + key, value, errors)); + } + else if(node instanceof List list) + { + list.forEach(value -> checkStrictSchema(path, value, errors)); + } + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @SuppressWarnings("unchecked") + private static void countOptionalAndUnions(Object node, int[] counts) + { + if(node instanceof Map map) + { + Map schema = (Map) map; + if(schema.get("properties") instanceof Map properties) + { + List required = schema.get("required") instanceof List r ? (List) r : List.of(); + for(Object name : properties.keySet()) + { + if(!required.contains(name)) + { + counts[0]++; + } + } + } + if(schema.get("type") instanceof List || schema.containsKey("anyOf")) + { + counts[1]++; + } + schema.values().forEach(value -> countOptionalAndUnions(value, counts)); + } + else if(node instanceof List list) + { + list.forEach(value -> countOptionalAndUnions(value, counts)); + } + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @SuppressWarnings("unchecked") + private static void collectCacheTtls(Object node, List ttls) + { + if(node instanceof Map map) + { + if(map.get("cache_control") instanceof Map cache) + { + ttls.add(cache.get("ttl") == null ? "5m" : String.valueOf(cache.get("ttl"))); + } + for(Map.Entry entry : map.entrySet()) + { + if(!"cache_control".equals(entry.getKey()) && !"input_schema".equals(entry.getKey())) + { + collectCacheTtls(entry.getValue(), ttls); + } + } + } + else if(node instanceof List list) + { + list.forEach(value -> collectCacheTtls(value, ttls)); + } + } + +} diff --git a/qbit-assistant-provider-anthropic/src/test/resources/fixtures/grammar-too-large.json b/qbit-assistant-provider-anthropic/src/test/resources/fixtures/grammar-too-large.json new file mode 100644 index 0000000..edbb42d --- /dev/null +++ b/qbit-assistant-provider-anthropic/src/test/resources/fixtures/grammar-too-large.json @@ -0,0 +1 @@ +{"type":"error","error":{"type":"invalid_request_error","message":"The compiled grammar is too large, which would cause performance issues. Simplify your tool schemas or reduce the number of strict tools."},"request_id":"req_test_grammar"} diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java index 1c79dbd..cc93a3b 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/AssistantQBitConfig.java @@ -95,6 +95,14 @@ public class AssistantQBitConfig implements QBitConfig private Set hiddenWidgetNames = new LinkedHashSet<>(); private Set allowedWidgetNames = new LinkedHashSet<>(); + + //////////////////////////////////////////////////////////////////////////// + // tools sent with strict: true. Empty by default: Anthropic compiles all // + // strict schemas of a request into one grammar and refuses the request // + // (400 "compiled grammar is too large") with all ten read tools strict. // + // The kernel validates every input anyway (INVALID_INPUT results). // + //////////////////////////////////////////////////////////////////////////// + private Set strictToolNames = new LinkedHashSet<>(); private List customRedactionPatterns = new ArrayList<>(); private Integer killSwitchPollMillis = 2000; private Set allowedApiKeyEnvNames = new LinkedHashSet<>(); @@ -1128,6 +1136,38 @@ public class AssistantQBitConfig implements QBitConfig + /******************************************************************************* + ** Getter for strictToolNames: the tools sent with strict schema adherence + ** (empty by default; see the field comment). + *******************************************************************************/ + public Set getStrictToolNames() + { + return (this.strictToolNames); + } + + + + /******************************************************************************* + ** Setter for strictToolNames + *******************************************************************************/ + public void setStrictToolNames(Set strictToolNames) + { + this.strictToolNames = strictToolNames; + } + + + + /******************************************************************************* + ** Fluent setter for strictToolNames + *******************************************************************************/ + public AssistantQBitConfig withStrictToolNames(Set strictToolNames) + { + this.strictToolNames = strictToolNames; + return (this); + } + + + /******************************************************************************* ** Getter for allowedWidgetNames *******************************************************************************/ diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ContextBuilder.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ContextBuilder.java index f452c05..a795fcf 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ContextBuilder.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/ContextBuilder.java @@ -86,7 +86,7 @@ public class ContextBuilder .withName(definition.getName()) .withDescription(definition.getDescription()) .withInputSchema(definition.getInputSchema()) - .withStrict(!Boolean.FALSE.equals(definition.getStrict()))); + .withStrict(isStrict(definition))); } } return (tools); @@ -94,6 +94,18 @@ public class ContextBuilder + /******************************************************************************* + ** Strict only when the host names the tool in strictToolNames (and the + ** kernel does not opt it out): the provider compiles every strict schema of + ** a request into one grammar, which it refuses when it grows too large. + *******************************************************************************/ + boolean isStrict(AiToolDefinition definition) + { + return (!Boolean.FALSE.equals(definition.getStrict()) && config.getStrictToolNames() != null && config.getStrictToolNames().contains(definition.getName())); + } + + + /******************************************************************************* ** Names of the phase-1 tools. *******************************************************************************/ diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java index b72f2a6..f86c619 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/TurnEngine.java @@ -239,7 +239,7 @@ public class TurnEngine implements Runnable break; } - ModelResponse response = callModel(provider, request); + ModelResponse response = callModelWithStrictFallback(provider, request); stopReason = response.getStopReason(); if(ModelResponse.STOP_CANCELLED.equals(stopReason)) @@ -334,7 +334,15 @@ public class TurnEngine implements Runnable errorText = e.getKind() == ModelProviderException.Kind.TIMEOUT ? "The assistant took too long to answer." : (e.isRetryable() ? "The AI service is busy or unavailable right now. Try again in a moment." : "The AI service could not answer this request."); - LOG.warn("Assistant model call failed", logPair("turnId", turn.getId()), logPair("kind", e.getKind()), logPair("httpStatus", e.getHttpStatus())); + if(!e.isRetryable() && e.getProviderMessage() != null && session.hasPermission(AssistantQBitConfig.ADMIN_PERMISSION)) + { + ////////////////////////////////////////////////////////////// + // administrators see the provider's own (sanitized) reason // + ////////////////////////////////////////////////////////////// + errorText += " The provider said: " + UsageRecorder.errorText(e); + } + LOG.warn("Assistant model call failed", logPair("turnId", turn.getId()), logPair("kind", e.getKind()), logPair("httpStatus", e.getHttpStatus()), + logPair("errorType", e.getProviderErrorType()), logPair("errorMessage", e.getProviderMessage()), logPair("requestId", e.getRequestId())); } catch(InterruptedException e) { @@ -342,8 +350,12 @@ public class TurnEngine implements Runnable status = "CANCELLED"; stopReason = ModelResponse.STOP_CANCELLED; } - catch(Exception e) + catch(Exception | Error e) { + //////////////////////////////////////////////////////////////////// + // Errors too (a missing class, a linkage problem): otherwise the // + // executor swallows them and the turn ends "COMPLETE" and empty // + //////////////////////////////////////////////////////////////////// status = "ERROR"; errorCode = "INTERNAL"; errorText = "Something went wrong while answering. Try again."; @@ -470,6 +482,33 @@ public class TurnEngine implements Runnable + /******************************************************************************* + ** A model call; when the provider refuses the request because of its strict + ** tool schemas, retry once (and for the rest of the turn) with no strict + ** tools. The kernel validates every tool input either way. + *******************************************************************************/ + private ModelResponse callModelWithStrictFallback(AssistantModelProvider provider, ModelRequest request) throws ModelProviderException, InterruptedException + { + try + { + return (callModel(provider, request)); + } + catch(ModelProviderException e) + { + boolean anyStrict = request.getTools().stream().anyMatch(tool -> Boolean.TRUE.equals(tool.getStrict())); + if(!e.isStrictSchemaRejected() || !anyStrict) + { + throw (e); + } + LOG.warn("Assistant provider refused the strict tool schemas; retrying without strict tools", logPair("turnId", turn.getId()), logPair("model", model), + logPair("errorMessage", e.getProviderMessage()), logPair("requestId", e.getRequestId())); + request.getTools().forEach(tool -> tool.setStrict(false)); + return (callModel(provider, request)); + } + } + + + /******************************************************************************* ** One streamed model call. *******************************************************************************/ diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/UsageRecorder.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/UsageRecorder.java index 8c1eb8b..4011a38 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/UsageRecorder.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/engine/UsageRecorder.java @@ -90,7 +90,22 @@ public final class UsageRecorder .withCacheReadTokens(usage == null ? 0L : nz(usage.getCacheReadTokens())) .withCacheWriteTokens(usage == null ? 0L : nz(usage.getCacheWriteTokens())) .withCostMicros(pricing == null ? 0L : pricing.costMicros(usage)) - .withRequestId(response == null ? null : truncate(response.getRequestId(), 100))); + .withRequestId(response != null ? truncate(response.getRequestId(), 100) : (error == null ? null : truncate(error.getRequestId(), 100)))); + } + + + + /******************************************************************************* + ** What an administrator needs to act on a failed call: the provider's error + ** type and its own (sanitized) message when it sent one, else the summary. + *******************************************************************************/ + public static String errorText(ModelProviderException error) + { + if(error.getProviderMessage() != null) + { + return ((error.getProviderErrorType() == null ? "" : error.getProviderErrorType() + ": ") + error.getProviderMessage()); + } + return (error.getMessage()); } diff --git a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/provider/ModelProviderException.java b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/provider/ModelProviderException.java index fedb81b..ce80f13 100644 --- a/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/provider/ModelProviderException.java +++ b/qbit-assistant/src/main/java/com/kingsrook/qbits/assistant/provider/ModelProviderException.java @@ -23,6 +23,8 @@ package com.kingsrook.qbits.assistant.provider; *******************************************************************************/ public class ModelProviderException extends Exception { + public static final int MAX_PROVIDER_MESSAGE_CHARS = 300; + /******************************************************************************* ** Error classes the engine reacts to. *******************************************************************************/ @@ -41,6 +43,10 @@ public class ModelProviderException extends Exception private final Kind kind; private final Integer httpStatus; private ModelUsage partialUsage; + private String providerErrorType; + private String providerMessage; + private String requestId; + private boolean strictSchemaRejected; @@ -106,4 +112,92 @@ public class ModelProviderException extends Exception return (kind == Kind.RATE_LIMITED || kind == Kind.OVERLOADED || kind == Kind.TIMEOUT || kind == Kind.NETWORK || kind == Kind.SERVER); } + + + + /******************************************************************************* + ** The provider's own error type (e.g. invalid_request_error), when it sent one. + *******************************************************************************/ + public String getProviderErrorType() + { + return (providerErrorType); + } + + + + /******************************************************************************* + ** The provider's own error message: one line, capped, with anything that looks + ** like a key removed (see {@link #sanitizeProviderMessage}). Safe to log and + ** to show to administrators; never contains prompt content. + *******************************************************************************/ + public String getProviderMessage() + { + return (providerMessage); + } + + + + /******************************************************************************* + ** The provider's request id, for its support. + *******************************************************************************/ + public String getRequestId() + { + return (requestId); + } + + + + /******************************************************************************* + ** Fluent setter for the provider's error details. + *******************************************************************************/ + public ModelProviderException withProviderDetails(String providerErrorType, String providerMessage, String requestId) + { + this.providerErrorType = sanitizeProviderMessage(providerErrorType, 64); + this.providerMessage = sanitizeProviderMessage(providerMessage, MAX_PROVIDER_MESSAGE_CHARS); + this.requestId = sanitizeProviderMessage(requestId, 100); + return (this); + } + + + + /******************************************************************************* + ** One line, at most max characters, with key-like tokens (sk-..., long + ** base64/hex runs) replaced by [redacted]. Null for blank input. + *******************************************************************************/ + public static String sanitizeProviderMessage(String text, int max) + { + if(text == null || text.isBlank()) + { + return (null); + } + String line = text.replaceAll("[\\p{Cntrl}\\s]+", " ").strip(); + line = line.replaceAll("(?i)\\bsk-[a-z0-9_\\-]{6,}", "[redacted]"); + line = line.replaceAll("[A-Za-z0-9+/_\\-]{40,}", "[redacted]"); + return (line.length() <= max ? line : line.substring(0, max - 1) + "…"); + } + + + + + /******************************************************************************* + ** Whether the provider refused the request because of its strict tool + ** schemas (e.g. Anthropic's "compiled grammar is too large"): the same + ** request without strict tools can succeed. + *******************************************************************************/ + public boolean isStrictSchemaRejected() + { + return (strictSchemaRejected); + } + + + + /******************************************************************************* + ** Fluent setter for strictSchemaRejected + *******************************************************************************/ + public ModelProviderException withStrictSchemaRejected(boolean strictSchemaRejected) + { + this.strictSchemaRejected = strictSchemaRejected; + return (this); + } + } diff --git a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/TurnEngineTest.java b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/TurnEngineTest.java index a7301e4..244c157 100644 --- a/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/TurnEngineTest.java +++ b/qbit-assistant/src/test/java/com/kingsrook/qbits/assistant/engine/TurnEngineTest.java @@ -97,7 +97,11 @@ class TurnEngineTest extends BaseAssistantTest assertThat(first.getSystem().get(0).getText()).contains("qqq_data").contains("untrusted").contains("/app/"); assertThat(first.getSystem().get(1).getCacheHint()).isEqualTo(CacheHint.SHORT); assertThat(first.getTools()).extracting(t -> t.getName()).contains("count_records", "query_records", "navigate").doesNotHaveDuplicates(); - assertThat(first.getTools()).allMatch(t -> Boolean.TRUE.equals(t.getStrict())); + ////////////////////////////////////////////////////////////////////// + // no strict tools by default: Anthropic refused all ten strict // + // ("compiled grammar is too large"); the kernel validates inputs // + ////////////////////////////////////////////////////////////////////// + assertThat(first.getTools()).noneMatch(t -> Boolean.TRUE.equals(t.getStrict())); ModelRequest second = requests.get(1); assertThat(second.getSystem().get(0).getText()).isEqualTo(first.getSystem().get(0).getText()); -- 2.54.0