Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .codeyam/state/step-highwater.json

This file was deleted.

11 changes: 11 additions & 0 deletions Sources/AppCore/AskCoachView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,17 @@ public struct AskCoachView: View {
reply = CoachReply(intent: .general,
text: "Your AI coach key was rejected. Reconnect it in Settings, then ask again.",
mood: .concerned)
} catch CoachError.rateLimited {
// Previously fell through to the connection copy, which sent the
// user to check their wifi over a provider-side throttle.
reply = CoachReply(intent: .general,
text: "Your AI provider is rate limiting requests right now. Give it a minute and ask again.",
mood: .concerned)
} catch CoachError.upstream(let message) {
// A provider-side configuration fault. Saying "check your
// connection" here sent the user chasing a network problem that
// did not exist, so state what actually happened.
reply = CoachReply(intent: .general, text: message, mood: .concerned)
} catch {
reply = CoachReply(intent: .general,
text: "I couldn't reach Buddy just now. Check your connection and try again.",
Expand Down
40 changes: 39 additions & 1 deletion Sources/AppCore/Coach/RemoteCoach.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ public enum CoachError: Error, Equatable {
case rateLimited
case server
case network
/// The backend reached the provider and the provider refused for a reason the
/// user can act on (an unavailable model, an exhausted token budget). Carries
/// the backend's own message: collapsing these into `.server` is what made a
/// misconfigured provider read as "check your connection" in the chat, which
/// pointed the user at the one thing that was not wrong.
case upstream(String)
}

/// Calls the backend coach proxy. Stateless; safe to construct per request.
Expand Down Expand Up @@ -245,6 +251,31 @@ public struct RemoteCoach {
let safetyFlag: Bool
}

/// The backend's error envelope: `{ "error": "<code>", "message": "<why>" }`.
private struct ErrorBody: Decodable {
let error: String
let message: String?
}

/// The actionable reason from a non-2xx body, when the backend supplied one.
/// Only codes the user can actually do something about are surfaced; a bare
/// `upstream_error` stays a generic failure so a real outage still reads as one.
static func errorMessage(from data: Data) -> String? {
guard let body = try? JSONDecoder().decode(ErrorBody.self, from: data) else { return nil }
let actionable: Set<String> = [
"model_unavailable",
"token_budget_exhausted",
// An exhausted balance is the single most likely real-world failure
// and the one the user can actually fix, so it must never be
// flattened into a retry or connection message.
"insufficient_quota",
]
guard actionable.contains(body.error), let message = body.message, !message.isEmpty else {
return nil
}
return message
}

/// The context actually put on the wire. The journal is projected down to its
/// bounded recent slice here — at the single point where the payload leaves
/// the device — so an unbounded diary can never push `loadHistory` out of the
Expand Down Expand Up @@ -291,7 +322,14 @@ public struct RemoteCoach {
case 200: break
case 401: throw CoachError.invalidKey // only a rejected key surfaces to the user; a 400 (bad request) is our bug, not theirs
case 429: throw CoachError.rateLimited
default: throw CoachError.server
default:
// The backend distinguishes a provider misconfiguration (unavailable
// model, exhausted token budget) from an outage. Carry that reason
// through rather than flattening it into a generic failure.
if let message = Self.errorMessage(from: data) {
throw CoachError.upstream(message)
}
throw CoachError.server
}

guard let decoded = try? JSONDecoder().decode(ResponseBody.self, from: data) else {
Expand Down
65 changes: 65 additions & 0 deletions Tests/AppCoreTests/CoachErrorSurfacingTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import XCTest
@testable import AppCore

// How a failed coach call reaches the user.
//
// Regression: a working OpenAI key produced "I couldn't reach Buddy just now.
// Check your connection and try again." The key was valid and the network was
// fine — the backend had rejected the request for a provider-side reason — but
// every non-401 collapsed into `.server`, so the chat pointed the user at the
// one thing that was not wrong. These pin the reason surviving the trip.
final class CoachErrorSurfacingTests: XCTestCase {

private func body(_ json: String) -> Data { Data(json.utf8) }

func testActionableUpstreamReasonSurvives() {
let data = body("""
{"error":"model_unavailable","message":"OpenAI rejected the request: model \\"gpt-5\\" may be unavailable to this key."}
""")
XCTAssertEqual(RemoteCoach.errorMessage(from: data),
"OpenAI rejected the request: model \"gpt-5\" may be unavailable to this key.")
}

/// The real cause of the reported outage: an OpenAI account with no credits.
/// The user can fix this in a minute, but only if they are told what it is.
func testOutOfCreditsIsSurfaced() {
let data = body("""
{"error":"insufficient_quota","message":"Your OpenAI account is out of credits, so it declined the request. Add credits to your OpenAI account and ask again."}
""")
let message = RemoteCoach.errorMessage(from: data)
XCTAssertNotNil(message)
XCTAssertTrue(message!.contains("out of credits"))
// The two messages this used to be mistaken for.
XCTAssertFalse(message!.contains("connection"))
XCTAssertFalse(message!.contains("try again shortly"))
}

func testExhaustedBudgetIsSurfaced() {
let data = body("""
{"error":"token_budget_exhausted","message":"OpenAI used its entire token budget before answering."}
""")
XCTAssertEqual(RemoteCoach.errorMessage(from: data),
"OpenAI used its entire token budget before answering.")
}

/// A genuine outage stays generic — the caller falls back to the offline
/// coach rather than showing the user backend jargon they cannot act on.
func testGenericUpstreamErrorIsNotSurfaced() {
XCTAssertNil(RemoteCoach.errorMessage(from: body(#"{"error":"upstream_error"}"#)))
XCTAssertNil(RemoteCoach.errorMessage(from: body(#"{"error":"no_text"}"#)))
}

func testMalformedOrEmptyBodiesAreIgnored() {
XCTAssertNil(RemoteCoach.errorMessage(from: body("not json")))
XCTAssertNil(RemoteCoach.errorMessage(from: Data()))
XCTAssertNil(RemoteCoach.errorMessage(from: body(#"{"error":"model_unavailable"}"#)))
XCTAssertNil(RemoteCoach.errorMessage(from: body(#"{"error":"model_unavailable","message":""}"#)))
}

/// The error the chat renders must not be the connection copy.
func testUpstreamErrorIsDistinctFromNetworkError() {
XCTAssertNotEqual(CoachError.upstream("model unavailable"), CoachError.server)
XCTAssertNotEqual(CoachError.upstream("model unavailable"), CoachError.network)
XCTAssertEqual(CoachError.upstream("same"), CoachError.upstream("same"))
}
}
125 changes: 115 additions & 10 deletions api/_lib/llm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,90 @@ function parseJsonReply(text: string): LlmResult {
}
}

/** Map any provider's HTTP status onto the status the app already handles. */
function throwForStatus(status: number | undefined, provider: LlmProvider): never {
/**
* Map any provider's HTTP status onto the status the app already handles.
*
* `detail` carries the provider's own error text when we have it. Without it a
* misconfigured model id and a genuine outage both surfaced as "could not be
* reached", which is what made a broken OpenAI key impossible to diagnose from
* the app: the user saw a network-sounding message for a config bug.
*/
function throwForStatus(
status: number | undefined,
provider: LlmProvider,
detail?: ProviderError,
): never {
const name = provider === "anthropic" ? "Anthropic" : provider === "openai" ? "OpenAI" : "Gemini";
const suffix = detail?.message ? ` ${detail.message}` : "";
if (status === 401 || status === 403) {
throw new LlmError(401, "invalid_key", `That API key was rejected by ${name}.`);
}
if (status === 429) {
// An empty balance is NOT a transient limit and never clears on its own.
// 402 (not 429) so the app parses the body instead of routing it to the
// retry path — "try again shortly" is unactionable when the account is dry.
if (isQuotaExhausted(detail)) {
throw new LlmError(
402,
"insufficient_quota",
`Your ${name} account is out of credits, so it declined the request. Add credits to your ${name} account and ask again.`,
);
}
throw new LlmError(429, "rate_limited", `Your ${name} account is rate limited. Try again shortly.`);
}
throw new LlmError(502, "upstream_error", `${name} could not be reached.`);
// 400/404 are OUR bug (unknown model, malformed request), not an outage, and
// not something the user can retry past. Say so, and pass the provider's
// reason through so the cause is visible without reproducing locally.
if (status === 400 || status === 404) {
throw new LlmError(
502,
"model_unavailable",
`${name} rejected the request: model "${modelFor(provider)}" may be unavailable to this key.${suffix}`,
);
}
throw new LlmError(502, "upstream_error", `${name} could not be reached.${suffix}`);
}

/** A provider's error body, normalized to the bits we route on. */
interface ProviderError {
message?: string;
/** Provider's own classifier, e.g. "insufficient_quota". */
type?: string;
/** Provider's own code, e.g. "credit_balance_exhausted". */
code?: string;
}

/** Best-effort extraction of a provider's error from its response body. */
async function errorDetail(response: Response): Promise<ProviderError | undefined> {
const body = (await response.json().catch(() => null)) as
| { error?: { message?: string; type?: string; code?: string } | string }
| null;
if (!body) return undefined;
const err = body.error;
if (typeof err === "string") return { message: err.slice(0, 300) };
if (!err) return undefined;
return {
message: typeof err.message === "string" ? err.message.slice(0, 300) : undefined,
type: typeof err.type === "string" ? err.type : undefined,
code: typeof err.code === "string" ? err.code : undefined,
};
}

/**
* Whether a 429 is an exhausted balance rather than a burst limit.
*
* These are opposite problems wearing the same status code: a burst limit clears
* on its own in seconds, an empty balance never does. Telling someone with no
* credits to "try again shortly" is advice that cannot work, so they get routed
* apart here.
*/
function isQuotaExhausted(detail?: ProviderError): boolean {
const needles = ["insufficient_quota", "credit_balance_exhausted", "billing_hard_limit_reached"];
const haystack = `${detail?.type ?? ""} ${detail?.code ?? ""}`.toLowerCase();
if (needles.some((n) => haystack.includes(n))) return true;
// Gemini/others don't use OpenAI's type/code vocabulary; fall back to prose.
const message = (detail?.message ?? "").toLowerCase();
return message.includes("no credits remaining") || message.includes("exceeded your current quota");
}

// MARK: - Anthropic
Expand All @@ -182,7 +256,10 @@ async function completeAnthropic(apiKey: string, request: LlmRequest): Promise<L
return parseJsonReply(textBlock.text);
} catch (err) {
if (err instanceof LlmError) throw err;
throwForStatus((err as { status?: number }).status, "anthropic");
// The SDK surfaces the provider's error body on `.error`; pass it through so
// an exhausted balance is told apart from a burst limit here too.
const e = err as { status?: number; error?: { error?: ProviderError } };
throwForStatus(e.status, "anthropic", e.error?.error);
}
}

Expand All @@ -192,6 +269,19 @@ async function completeAnthropic(apiKey: string, request: LlmRequest): Promise<L
// fetch rather than the SDK: it is one request shape, and every extra dependency
// is weight in a serverless bundle that cold-starts on each user's key.

/**
* Headroom for the reply itself, on top of whatever the model spends thinking.
*
* `max_completion_tokens` is a budget for reasoning tokens AND visible output.
* On a reasoning model the thinking is invoiced against the same allowance, so a
* budget sized for a 2-4 sentence answer (1024) can be consumed entirely before
* a single visible character is emitted — the call then returns `content: ""`
* with `finish_reason: "length"`, which read as "no text" and surfaced to the
* user as a connection failure. The reply is small; the thinking is not, so this
* is sized for the thinking.
*/
const OPENAI_MIN_COMPLETION_TOKENS = 16000;

async function completeOpenAI(apiKey: string, request: LlmRequest): Promise<LlmResult> {
let response: Response;
try {
Expand All @@ -203,7 +293,7 @@ async function completeOpenAI(apiKey: string, request: LlmRequest): Promise<LlmR
},
body: JSON.stringify({
model: MODELS.openai,
max_completion_tokens: request.maxTokens ?? 1024,
max_completion_tokens: Math.max(request.maxTokens ?? 1024, OPENAI_MIN_COMPLETION_TOKENS),
messages: [
{ role: "system", content: request.system },
...request.messages.map((t) => ({ role: t.role, content: t.content })),
Expand All @@ -218,15 +308,30 @@ async function completeOpenAI(apiKey: string, request: LlmRequest): Promise<LlmR
throw new LlmError(502, "upstream_error", "OpenAI could not be reached.");
}

if (!response.ok) throwForStatus(response.status, "openai");
if (!response.ok) throwForStatus(response.status, "openai", await errorDetail(response));

const body = (await response.json().catch(() => null)) as {
choices?: Array<{ message?: { content?: string | null; refusal?: string | null } }>;
choices?: Array<{
finish_reason?: string;
message?: { content?: string | null; refusal?: string | null };
}>;
} | null;
const choice = body?.choices?.[0]?.message;
const first = body?.choices?.[0];
const choice = first?.message;
// A strict-schema refusal comes back in its own field, with content null.
if (choice?.refusal) return { kind: "refusal" };
if (typeof choice?.content !== "string" || !choice.content) return { kind: "empty" };
if (typeof choice?.content !== "string" || !choice.content) {
// Empty output because the budget ran out is a fixable configuration fault,
// not an upstream outage — name it so it can't hide behind "no text" again.
if (first?.finish_reason === "length") {
throw new LlmError(
502,
"token_budget_exhausted",
`OpenAI used its entire token budget before answering (model "${MODELS.openai}"). Raise max_completion_tokens.`,
);
}
return { kind: "empty" };
}
return parseJsonReply(choice.content);
}

Expand Down Expand Up @@ -274,7 +379,7 @@ async function completeGemini(apiKey: string, request: LlmRequest): Promise<LlmR
throw new LlmError(502, "upstream_error", "Gemini could not be reached.");
}

if (!response.ok) throwForStatus(response.status, "gemini");
if (!response.ok) throwForStatus(response.status, "gemini", await errorDetail(response));

const body = (await response.json().catch(() => null)) as {
candidates?: Array<{ finishReason?: string; content?: { parts?: Array<{ text?: string }> } }>;
Expand Down
Loading
Loading