clean up

Author: chiang-daniel

app/web_ui/src/lib/ui/error_with_trace.svelte

@@ -7,10 +7,9 @@
   export let error_title: string = "Error"
   export let troubleshooting_steps: string[] = []
 
-  // The generated ErrorWithTrace["trace"] uses the "-Output" variants of the
-  // chat message schemas while the shared `Trace` alias uses the "-Input"
-  // variants. Shapes are render-compatible; cast once here so trace.svelte's
-  // prop typing is satisfied.
+  // The API response and UI component use slightly different TypeScript types
+  // for chat messages (same structure, different type names). Cast once here
+  // to satisfy the Trace component's prop typing.
   $: trace_for_viewer = (error.trace ?? []) as TraceType
 </script>
 

app/web_ui/src/routes/(app)/run/run_page_errors.test.ts

@@ -1,20 +1,6 @@
 import { describe, it, expect } from "vitest"
 import { looks_like_error_with_trace } from "./error_with_trace_detection"
 
-// Phase 4 wires the run page to distinguish three fetch outcomes:
-//   1. 500 with an ErrorWithTrace body -> render <ErrorWithTrace>
-//   2. 500 (or 4xx) with a plain HTTPException body -> FormContainer fallback
-//   3. Network failure / non-JSON body -> FormContainer fallback
-// In all three cases the submit button re-enables because `submitting = false`
-// lives inside a `finally` block.
-//
-// The single decision point driving (1) vs (2)+(3) on the run page is
-// `looks_like_error_with_trace(fetch_error)`. `fetch_error` is whatever
-// openapi-fetch parsed out of the response body (JSON object, string fallback,
-// or {} for empty bodies per `openapi-fetch/dist/index.js`). These tests feed
-// each scenario's realistic `fetch_error` shape into the type guard and assert
-// the routing decision.
-
 describe("looks_like_error_with_trace (type guard)", () => {
   it("returns true for an ErrorWithTrace body with trace present", () => {
     const body = {
@@ -108,33 +94,3 @@ describe("looks_like_error_with_trace (type guard)", () => {
     expect(looks_like_error_with_trace(body)).toBe(false)
   })
 })
-
-// Explicit scenario-level mapping tests. These re-assert scenario-to-branch
-// mapping so future readers see the three run-page outcomes named directly.
-describe("run page error routing scenarios", () => {
-  it("scenario 1: 500 with ErrorWithTrace body -> renders <ErrorWithTrace>", () => {
-    const fetch_error = {
-      message: "The model's output didn't match the expected format.",
-      error_type: "JSONSchemaValidationError",
-      trace: [
-        { role: "system", content: "You are a helpful assistant." },
-        { role: "user", content: "hi" },
-        { role: "assistant", content: "not valid json" },
-      ],
-    }
-    expect(looks_like_error_with_trace(fetch_error)).toBe(true)
-  })
-
-  it("scenario 2: 500 with plain {detail: ...} body -> FormContainer fallback", () => {
-    const fetch_error = { detail: "Task configuration was deleted." }
-    expect(looks_like_error_with_trace(fetch_error)).toBe(false)
-  })
-
-  it("scenario 3: network failure surfaces non-object fetch_error -> FormContainer fallback", () => {
-    // openapi-fetch returns the raw text when the body isn't JSON; a thrown
-    // fetch error at the transport layer never reaches this branch (it bubbles
-    // to the outer catch) but we still guard against it here.
-    expect(looks_like_error_with_trace("")).toBe(false)
-    expect(looks_like_error_with_trace(undefined)).toBe(false)
-  })
-})

libs/core/kiln_ai/adapters/errors.py

@@ -5,7 +5,7 @@
 - `KilnRunError`: the exception thrown by the adapter that carries the partial
   conversation trace across the exception boundary so the API layer can return
   it to the client.
-- `format_user_message`: maps known exceptions to user-friendly text.
+- `format_error_message`: maps known exceptions to user-friendly text.
 """
 
 from __future__ import annotations
@@ -66,11 +66,11 @@ def _safe_str(exc: Exception) -> str:
     return result
 
 
-def format_user_message(exc: Exception) -> str:
+def format_error_message(exc: Exception) -> str:
     """Map an exception to a user-friendly message.
 
-    Known exception types get custom messages. Unknown types fall back to
-    str(exc). Never returns a stack trace or internal details.
+    Known exception types get custom messages. Unknown types get a generic
+    fallback to avoid leaking provider internals to the client.
     """
     try:
         # Order matters: several litellm error classes inherit from each
@@ -117,6 +117,6 @@ def format_user_message(exc: Exception) -> str:
             if "specific output schema" in msg or "didn't meet the schema" in msg:
                 return "The model's output didn't match the task's output schema."
 
-        return _safe_str(exc)
+        return _GENERIC_FALLBACK_MESSAGE
     except Exception:
         return _GENERIC_FALLBACK_MESSAGE

libs/core/kiln_ai/adapters/model_adapters/base_adapter.py

@@ -13,7 +13,7 @@
     MultiturnFormatter,
     get_chat_formatter,
 )
-from kiln_ai.adapters.errors import KilnRunError, format_user_message
+from kiln_ai.adapters.errors import KilnRunError, format_error_message
 from kiln_ai.adapters.ml_model_list import (
     KilnModelProvider,
     StructuredOutputMode,
@@ -287,8 +287,8 @@ async def _run_returning_run_output(
                     for message in parsed_output.trace
                 )
 
-                # Validate reasoning content is present and required
-                # We don't require reasoning when using tools as models tend not to return any on the final turn (both Sonnet and Gemini).
+                # Validate reasoning content is present if required.
+                # Models often skip reasoning on the final turn when tools are involved, so we don't require it then.
                 if (
                     provider.reasoning_capable
                     and (
@@ -343,7 +343,7 @@ async def _run_returning_run_output(
                 except Exception:
                     partial_trace = None
             raise KilnRunError(
-                message=format_user_message(e),
+                message=format_error_message(e),
                 partial_trace=partial_trace,
                 original=e,
             ) from e

libs/core/kiln_ai/adapters/model_adapters/mcp_adapter.py

@@ -1,7 +1,7 @@
 import json
 from typing import Tuple
 
-from kiln_ai.adapters.errors import KilnRunError, format_user_message
+from kiln_ai.adapters.errors import KilnRunError, format_error_message
 from kiln_ai.adapters.model_adapters.base_adapter import AdapterConfig, BaseAdapter
 from kiln_ai.adapters.parsers.json_parser import parse_json_string
 from kiln_ai.adapters.run_output import RunOutput
@@ -214,7 +214,7 @@ async def _run_and_validate_output(
             raise
         except Exception as e:
             raise KilnRunError(
-                message=format_user_message(e),
+                message=format_error_message(e),
                 partial_trace=None,
                 original=e,
             ) from e

libs/core/kiln_ai/adapters/model_adapters/test_base_adapter_errors.py

@@ -5,13 +5,13 @@
 - The partial trace is preserved across the exception boundary
 - Already-wrapped KilnRunErrors pass through unmodified
 - The original exception is accessible via `.original` and `__cause__`
-- `format_user_message` is applied to the wrapped message
+- `format_error_message` is applied to the wrapped message
 """
 
 from __future__ import annotations
 
 from typing import Tuple
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch
 
 import litellm
 import pytest
@@ -161,15 +161,15 @@ async def test_cause_chain_preserved(make_adapter):
     assert ei.value.original is inner
 
 
-async def test_format_user_message_applied_for_known_exception(make_adapter):
+async def test_format_error_message_applied_for_known_exception(make_adapter):
     inner = RuntimeError("Too many turns (11). Stopping iteration...")
     adapter = make_adapter(post_raise=inner)
     with pytest.raises(KilnRunError) as ei:
         await adapter._run_returning_run_output("hello")
     assert str(ei.value) == "The run exceeded the maximum number of turns."
 
 
-async def test_format_user_message_applied_for_litellm_rate_limit(make_adapter):
+async def test_format_error_message_applied_for_litellm_rate_limit(make_adapter):
     try:
         rate_limit = litellm.RateLimitError(
             message="upstream", model="m", llm_provider="openai"
@@ -231,24 +231,3 @@ async def test_messages_to_trace_hook_called(make_adapter):
             await adapter._run_returning_run_output("hello")
     hook.assert_called_once()
     assert ei.value.partial_trace == converted
-
-
-async def test_happy_path_unchanged(make_adapter, base_task):
-    """Sanity check: when `_run` succeeds the adapter still returns normally."""
-    run_output = RunOutput(output="hi", intermediate_outputs={})
-    adapter = make_adapter(return_output=(run_output, Usage()))
-
-    with (
-        patch(
-            "kiln_ai.adapters.model_adapters.base_adapter.model_parser_from_id"
-        ) as mock_parser_from_id,
-        patch.object(type(adapter), "model_provider") as mock_mp,
-    ):
-        mock_parser = MagicMock()
-        mock_parser.parse_output.return_value = run_output
-        mock_parser_from_id.return_value = mock_parser
-        mock_mp.return_value = KilnModelProvider(name="openai", formatter=None)
-
-        task_run, out = await adapter._run_returning_run_output("hello")
-    assert out is run_output
-    assert task_run.output.output == "hi"

libs/core/kiln_ai/adapters/model_adapters/test_mcp_adapter.py

@@ -299,8 +299,7 @@ async def test_mcp_adapter_runtime_failure_wrapped_in_kiln_run_error(
 
     assert isinstance(ei.value.original, RuntimeError)
     assert ei.value.partial_trace is None
-    # format_user_message falls back to str() for unrecognised RuntimeError.
-    assert str(ei.value) == "mcp tool blew up"
+    assert str(ei.value) == "An unexpected error occurred."
     assert ei.value.error_type == "RuntimeError"
 
 
@@ -313,8 +312,7 @@ async def test_mcp_adapter_output_schema_mismatch_wrapped_in_kiln_run_error(
     project_with_local_mcp_server,
     local_mcp_tool_id,
 ):
-    """Post-run output schema validation failures are also runtime failures
-    in scope for Phase 1 and should surface as KilnRunError."""
+    """Post-run output schema validation failures should surface as KilnRunError."""
     from kiln_ai.adapters.errors import KilnRunError
 
     project, _ = project_with_local_mcp_server

libs/core/kiln_ai/adapters/test_errors.py

@@ -9,7 +9,7 @@
 from kiln_ai.adapters.errors import (
     ErrorWithTrace,
     KilnRunError,
-    format_user_message,
+    format_error_message,
 )
 
 
@@ -59,21 +59,21 @@ class TestFormatUserMessage:
     def test_rate_limit_error(self):
         exc = _make_litellm_error(litellm.RateLimitError)
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "Rate limit exceeded. Wait a moment and try again."
         )
 
     def test_authentication_error(self):
         exc = _make_litellm_error(litellm.AuthenticationError)
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "Authentication with the model provider failed. Check your API key."
         )
 
     def test_api_connection_error(self):
         exc = _make_litellm_error(litellm.APIConnectionError)
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "Could not connect to the model provider. Check your network connection."
         )
 
@@ -88,7 +88,7 @@ def test_api_connection_error(self):
     def test_service_unavailable_family(self, cls):
         exc = _make_litellm_error(cls)
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The model provider is currently unavailable. Try again in a moment."
         )
 
@@ -108,7 +108,7 @@ def test_json_schema_validation_error(self):
             )
             Exception.__init__(exc, "schema mismatch")
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The model's output didn't match the expected format."
         )
 
@@ -117,15 +117,15 @@ def test_too_many_turns(self):
             "Too many turns (11). Stopping iteration to avoid using too many tokens."
         )
         assert (
-            format_user_message(exc) == "The run exceeded the maximum number of turns."
+            format_error_message(exc) == "The run exceeded the maximum number of turns."
         )
 
     def test_too_many_tool_calls(self):
         exc = RuntimeError(
             "Too many tool calls (31). Stopping iteration to avoid using too many tokens."
         )
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The run exceeded the maximum number of tool calls in one turn."
         )
 
@@ -134,7 +134,7 @@ def test_tool_not_available(self):
             "A tool named 'foo' was invoked by a model, but was not available."
         )
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The model tried to call a tool that isn't available on this task."
         )
 
@@ -143,7 +143,7 @@ def test_parse_arguments(self):
             "Failed to parse arguments for tool 'foo' (should be JSON): blah"
         )
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The model produced invalid arguments for a tool call."
         )
 
@@ -152,7 +152,7 @@ def test_validate_arguments(self):
             "Failed to validate arguments for tool 'foo'. The arguments didn't match..."
         )
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The model's tool call arguments didn't match the tool's schema."
         )
 
@@ -161,7 +161,7 @@ def test_reasoning_required(self):
             "Reasoning is required for this model, but no reasoning was returned."
         )
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The model should have returned reasoning but didn't."
         )
 
@@ -171,29 +171,28 @@ def test_schema_mismatch_value_error(self):
             "that JSON didn't meet the schema. Search 'Troubleshooting Structured Data Issues' in our docs for more information."
         )
         assert (
-            format_user_message(exc)
+            format_error_message(exc)
             == "The model's output didn't match the task's output schema."
         )
 
-    def test_unknown_exception_falls_back_to_str(self):
+    def test_unknown_exception_uses_generic_message(self):
         exc = KeyError("missing_key")
-        # KeyError's str() wraps in quotes; just assert the key name is present
-        assert "missing_key" in format_user_message(exc)
+        assert format_error_message(exc) == "An unexpected error occurred."
 
-    def test_unknown_exception_type_falls_back_to_str(self):
+    def test_unknown_exception_type_uses_generic_message(self):
         class WeirdError(Exception):
             pass
 
         exc = WeirdError("something odd")
-        assert format_user_message(exc) == "something odd"
+        assert format_error_message(exc) == "An unexpected error occurred."
 
-    def test_runtime_error_without_known_prefix_falls_back(self):
+    def test_runtime_error_without_known_prefix_uses_generic_message(self):
         exc = RuntimeError("something random happened")
-        assert format_user_message(exc) == "something random happened"
+        assert format_error_message(exc) == "An unexpected error occurred."
 
-    def test_value_error_without_schema_hint_falls_back(self):
+    def test_value_error_without_schema_hint_uses_generic_message(self):
         exc = ValueError("just a regular value error")
-        assert format_user_message(exc) == "just a regular value error"
+        assert format_error_message(exc) == "An unexpected error occurred."
 
     def test_survives_broken_str(self):
         class BrokenStrError(Exception):
@@ -202,20 +201,18 @@ def __str__(self):
 
         exc = BrokenStrError("hidden")
         # Should not raise; should return the generic fallback.
-        assert format_user_message(exc) == "An unexpected error occurred."
+        assert format_error_message(exc) == "An unexpected error occurred."
 
-    def test_empty_message_falls_back_to_class_name(self):
+    def test_empty_message_falls_back_to_generic(self):
         exc = RuntimeError("")
-        # RuntimeError with no recognised prefix and empty string → class name
-        # (more useful to the user than a totally generic message).
-        assert format_user_message(exc) == "RuntimeError"
+        assert format_error_message(exc) == "An unexpected error occurred."
 
-    def test_empty_message_custom_class_uses_class_name(self):
+    def test_empty_message_custom_class_uses_generic(self):
         class WeirdError(Exception):
             pass
 
         exc = WeirdError("")
-        assert format_user_message(exc) == "WeirdError"
+        assert format_error_message(exc) == "An unexpected error occurred."
 
 
 class TestKilnRunError: