feat(sdk,sdkx): pod-runtime errdefs/telemetry convergence [skip-tag:sdkx]#47
Merged
Conversation
errdefs:
- new BudgetExceeded classification (Is/Wrap/Fmt + 429 mapping) for
sandbox-host token/cost/quota refusals, distinct from upstream
RateLimit responses.
- new PolicyDenied classification (Is/Wrap/Fmt + 403 mapping) for
tool allow-list / network egress / RBAC refusals, distinct from
upstream Forbidden responses.
telemetry:
- new attrs.go pinning the canonical OpenTelemetry attribute names
(pod.id, agent.id, run.id, engine.kind, llm.tokens.*, kanban.*,
...) so producers across sdk/* stop drifting on key spellings.
- new RecordRunSummary helper that emits a short-lived
"engine.run.summary" span carrying RunID / status / token / cost /
latency in one shot. Stays leaf-package-clean by taking int64
counts instead of model.TokenUsage.
Both packages remain zero-dependency on the rest of sdk/* — they are
the L0 base every other layer can safely import.
Made-with: Cursor
Walk every "origin error" — fmt.Errorf calls that synthesise a brand
new error rather than wrap an existing one — across sdk/* and replace
them with the matching errdefs constructor so HTTPStatus mapping,
errors.As classification, and downstream pod-runtime policy hooks all
see consistent categories. Wrapping calls (fmt.Errorf with %w) are
left untouched: errdefs classification survives errors.Unwrap, so the
inner constructor is enough.
Mapping applied:
- errdefs.Validationf (bad input / required field / invalid format)
sdk/agent/run.go nil engine / empty Agent.ID / nil board
sdk/history/tools.go missing conversation ID
sdk/knowledge/generator.go nil llm
sdk/knowledge/deprecated.go missing dataset_id / name
sdk/knowledge/backend/fs/vec.go vec payload corruption
sdk/knowledge/backend/fs/atomic.go nil workspace
sdk/workspace/local.go empty / dash-prefixed git url
- errdefs.NotAvailablef (configured dependency missing or shut down)
sdk/history/tools.go summary store not available
sdk/kanban/tools.go no kanban instance available (x2)
sdk/kanban/scheduler.go scheduler stopped
- errdefs.Forbiddenf (refusal of a destructive / privileged action)
sdk/workspace/{mem,local}.go refusing to remove workspace root
sdk/workflow is intentionally skipped — the package is deprecated and
slated for removal in v0.3.0; touching its origin errors would just
churn code on its way out.
Made-with: Cursor
Follow-up to 1b2eebf. The previous commit only covered the trivially matchable subset — bare `fmt.Errorf("...")` calls with no format arguments. This commit handles the remaining "origin error" variant: `fmt.Errorf("...%s/%T...", args)` calls that synthesize a fresh error without wrapping anything. Wrapped errors (`fmt.Errorf("...: %w", err)`) are intentionally left alone: errdefs classifications travel through the Unwrap chain, so as long as the inner error is classified the outer wrap is transparent to `errors.As` / `errdefs.IsXxx`. Re-classifying every wrap site would add noise without changing observable behaviour. Notable picks - sdk/graph/runner/internal/executor: max iteration → BudgetExceeded (the first real consumer of the new BudgetExceeded code added in 21cb584). Parallel branch lookup miss → NotFound; merge variable / channel races → Conflict. - sdk/graph/port: missing required input/output port → Validation. - sdk/script/bindings: every type-check / required-field error in the three bridges + llm_marshal switches to Validation. The script side needs these to surface as 400-class so dashboards can split out user-script errors from runtime / provider faults. - sdk/engine/subjects: stream-delta payload decode errors → Validation. - sdk/history/summary_store: node deleted / not found → NotFound. - sdk/recall/extractor: facts JSON parse failure → Validation. - sdk/workspace/mem: "is a directory" misuse → Validation. Out of scope (deferred to a separate boundary-normalization pass) - Standard-library origin errors that are wrapped without classification (os.IsNotExist, json.UnmarshalTypeError, sql.ErrNoRows, ...). These need per-call-site decisions and will land package-by-package. - Deprecated packages (sdk/workflow, sdk/llm/deprecated.go, sdk/knowledge/deprecated.go). Verified - go build ./... in sdk, voice, sdkx - go test ./... in sdk, voice, sdkx Made-with: Cursor
Pod-level retry, circuit-breaking, fail-fast and SLO accounting are all
written as policies over errdefs predicates (IsRateLimit, IsTimeout,
IsNotAvailable, ...). For those decisions to fire correctly, the two
biggest sources of "raw" upstream errors that flow through the SDK have
to be translated *at the boundary* — otherwise everything looks like
"some error" and pods fall back to coarse all-or-nothing behaviour.
This commit covers the two boundary classes that justify the work today:
1. LLM provider faults (every external call goes through here).
2. context cancellation / deadline (every long-running path threads
a ctx through hot loops).
Other boundaries (fs ENOENT, json parse, gzip) are deliberately
deferred — pods do not auto-decide on those.
Changes
-------
sdk/errdefs (helper)
- FromContext(err) maps ctx.Err() → Timeout / Aborted / passthrough,
with documented semantics so providers and runtimes can fold it into
any error path without inventing local conventions.
- HasClassification(err) (extracted from a private helper) lets
adapters short-circuit double-classification so a pre-marked error
(e.g. ctx already wrapped into Timeout) is never demoted by a later
pass through ClassifyProviderError.
sdk/llm (helpers)
- ClassifyProviderError(provider, err) wraps an SDK-returned provider
error into Unauthorized / Forbidden / RateLimit / Validation /
NotAvailable based on the existing ErrorCategory taxonomy. The wrap
preserves the original error chain so callers that need richer signals
(vendor-specific fields, fallback decisions keyed on ErrorCategory)
can still errors.As against the inner type. Pre-classified errors are
returned untouched.
- ClassifyHTTPStatus(provider, code, body) does the same for providers
that drive HTTP themselves (ollama, custom REST adapters): it
produces the same errdefs vocabulary keyed off a raw status code.
sdkx/llm/{openai,anthropic,bytedance,ollama}
- All Generate / GenerateStream paths route the provider error through
ClassifyProviderError; HTTP-driven paths in ollama additionally use
ClassifyHTTPStatus for non-2xx responses.
- Stream readers (the `s.err = stream.Err()` patterns) classify on the
way out so consumers polling .Err() get the same categorisation as
the synchronous path.
- Stream readers also normalise baseCtx.Err() via errdefs.FromContext
so a deadline mid-stream surfaces as IsTimeout, not as a raw stdlib
error that downstream policies cannot interpret.
- qwen / azure / deepseek / minimax are thin wrappers over the above
and inherit the classification automatically.
sdk (ctx call sites)
- Hot-path ctx.Err() returns now go through errdefs.FromContext:
graph executor (parallel / retry), event/memory (publish blocking),
history/compactor (Compact / Archive / Clear / Shutdown), knowledge
retrieval (fs + retrieval recall loops), knowledge/reloader,
knowledge/service rebuild, recall/memory await, sdk/workspace exec,
sdk/script/{jsrt,luart} script-cancelled paths.
- Deprecated paths (sdk/knowledge/deprecated.go) skipped to keep the
diff focused.
Out of scope (deferred until pods need them)
- Standard-library origin error normalisation (os.IsNotExist on
workspace already lives behind ErrNotFound which is errdefs-marked,
so history → workspace chains already classify correctly; finer fs /
json / gzip translations will land per-call-site when pod policies
start consuming them).
Tests
- errdefs: pin FromContext mapping (DeadlineExceeded → Timeout,
Canceled → Aborted), identity preservation (errors.Is still matches
the original sentinel), nil passthrough, no-double-classification
guard, unknown-cause passthrough.
- sdk/llm: pin ClassifyProviderError mapping for every category +
errors.Is chain preservation + nil passthrough + pre-classified
short-circuit; ClassifyHTTPStatus mapping for every documented
status code.
Verified
- go build ./... and go test ./... in sdk, voice, sdkx (all green).
Made-with: Cursor
…ttr-key drift
The Attr* constants in sdk/telemetry/attrs.go were declared in the L0
commit but the producers were still emitting the same keys as raw
string literals. Two costs followed:
- Renaming a constant did not propagate; the test in attrs_test.go
only pinned the constants, not the call sites.
- Several call sites had drifted from the canonical schema, so
dashboards needed two separate filters to capture the same logical
dimension.
This commit closes both gaps for the keys we already publish. New keys
are out of scope (see follow-up commit for ConversationID / DatasetID
/ ErrorMessage).
Schema corrections (BREAKING for any dashboard / alert that still
filters on the old key — none in tree, kanban not yet released to
external consumers):
kanban.target_agent_id → kanban.target.agent.id (Submit span)
target_agent_id → kanban.target.agent.id (tasks metric)
kanban.scheduler.agent_id → agent.id (scheduler span; same dimension as the rest of the SDK)
agent_id → agent.id (scheduler logs)
card_id, card → kanban.card.id (scheduler / board / kanban logs)
tool → tool.name (tool panic recover log)
Producer adoptions (no observable change, just constant references):
sdk/tool/registry.go tool.name × 7
sdk/graph/runner/.../executor.go graph.name / run.id / node.id × 14
sdk/graph/node/llmnode/llmnode.go node.id × 1
sdk/kanban/{kanban,scheduler,board}.go kanban.target.agent.id /
kanban.card.id / agent.id × 9
Out of scope
- conversation_id / dataset_id / error.message — call sites exist
(sdk/history, sdk/recall, sdk/knowledge, ...) but the corresponding
Attr* constants do not. Adding the constants and the call-site
rewrites in the same commit would mix "schema migration" with
"schema extension"; they ship as a follow-up.
- "schedule_id", "node.type", "status" and similar internal keys are
intentionally left as string literals — they are package-local
dimensions that no other producer needs to match against.
Verified
- go build ./... and go test ./... in sdk (all green; no test asserted
on the migrated literals, so the schema rename is observable only
via attrs_test.go which is unchanged).
Made-with: Cursor
…and adopt
Three call-site dimensions were emitted across multiple packages with
no shared constant, so each producer chose its own spelling:
- conversation context: sdk/history (compactor / archive) used
"conversation_id" snake_case;
- knowledge dataset: sdk/graph/node/knowledgenode used "dataset_id";
- error text: ~36 sites across sdk/{history,kanban,graph,
tool,knowledge,recall,llm} used "error".
All three are cross-package dimensions the future sdk/pod controller
needs to filter/aggregate on (per-conversation budgets, per-dataset
SLO, per-class error counts), so they must converge on canonical
keys before producers multiply further.
Constants added (sdk/telemetry/attrs.go):
AttrConversationID = "conversation.id"
AttrDatasetID = "dataset.id"
AttrErrorMessage = "error.message"
attrs_test.go was extended in lockstep so any future rename surfaces
as a test failure rather than silent dashboard drift.
Producer migrations:
history/compactor.go conversation_id × 5, error × 11
history/archive.go conversation_id × 2
history/store.go error × 1
kanban/{kanban,board,scheduler,metrics}.go error × 10
graph/runner/.../executor.go error × 2
graph/node/llmnode/llmnode.go error × 3 (also node_id → AttrNodeID)
graph/node/knowledgenode/knowledgenode.go dataset_id × 2 + error × 1
knowledge/deprecated.go error × 1
recall/jobs.go error × 4
llm/fallback.go error × 4
tool/registry.go error × 1
Notes
- Error key normalisation is "error" → "error.message" on log records
and span events. AttrErrorMessage is documented to align *seman-
tically* with OTel's exception.message but to live under the shorter
error.message key, because flowcraft does not otherwise emit the
exception-event triple (type / message / stacktrace) — a single
canonical name is enough and matches existing log queries.
- JSON struct tags ("error" in event payload structs, "conversation_id"
in archive records, etc.) are intentionally left untouched: those
are wire-format schemas owned by sdk/event consumers, not telemetry
attribute keys, and migrating them is a separate breaking change.
- Internal package-local keys ("raw", "schedule_id", "card", "fork_node",
"panic", "status") are not promoted to constants — no other package
needs to filter on them.
Verified: go build ./... + go test ./... in sdk, sdkx, voice all green.
Made-with: Cursor
… errdefs
Provider-error → errdefs translation lived in sdk/llm/classifier.go,
which forced peer capabilities (sdkx/embedding/*, future sdkx/rerank,
…) to import sdk/llm purely to reuse a function whose body has nothing
to do with chat completion: HTTP status codes, the keyword table, and
HTTPStatusCoder probing are common to every external SaaS API the SDK
talks to.
Restructured along the actual responsibility boundary.
sdk/errdefs/http.go (new)
- ProviderCategory enum + ProviderXxx constants — the finer-grained
dimension that distinguishes 401/403 (Auth) from 402 (Billing) and
ContextOverflow from generic Permanent. Both detail buckets matter
for cooldown / retry decisions but collapse to the same errdefs
marker on the wire (Unauthorized / Forbidden / Validation).
- HTTPStatusCoder interface — exported here so SDK error types can be
matched without callers importing sdk/llm just for the interface.
- ClassifyProvider(err) ProviderCategory — structured-status-first
dispatcher with the keyword table (context-overflow keywords are
checked before HTTP code matching so "http 400: maximum context
length exceeded" wins as ContextOverflow, not Permanent).
- ClassifyProviderError(provider, err) error — wraps a provider error
in an errdefs marker via the table above. nil passthrough; existing
classification preserved (no double-wrapping of pre-classified
errors like FromContext output).
- ClassifyHTTPStatus(provider, code, body) error — same mapping but
for callers that drive HTTP themselves (ollama / custom REST).
- ProviderCategoryFromHTTPCode — exported so consumers that just have
a bare status code can map it to a category without paying for the
keyword scan.
- collectErrorMessages — moved verbatim; walks both Unwrap() error
and Unwrap() []error so phrases hidden in joined / wrapped chains
still match.
sdk/llm/classifier.go (trimmed ~70%)
The file kept only what is genuinely LLM-fallback domain:
- ErrorCategory = errdefs.ProviderCategory (alias) so fallback.go
reads in fallback vocabulary.
- CategoryXxx constants aliased to ProviderXxx so call sites and
tests need no migration.
- CategoryString / ShouldFallback / CooldownMultiplier — three free
functions (alias types cannot have methods); these encode the
fallback chain's policy (when to give up, breaker hold per class,
metric label).
- ClassifyError → errdefs.ClassifyProvider — domain-named alias.
Removed (the intent of the refactor):
- llm.ClassifyProviderError / llm.ClassifyHTTPStatus / llm.HTTPStatusCoder.
Re-exporting them as thin shims would defeat the point — peers
would still pull sdk/llm just to call the shim. External SDK
boundaries MUST call errdefs.ClassifyProvider* directly.
sdkx/llm/{openai,anthropic,bytedance,ollama}.go + their stream.go
- 16 call sites switched: llm.ClassifyProviderError →
errdefs.ClassifyProviderError; llm.ClassifyHTTPStatus →
errdefs.ClassifyHTTPStatus. No new imports needed; the affected
files already import errdefs for FromContext / Validation.
sdk/llm/fallback.go + sdk/llm/errors.go
- cat.ShouldFallback() / cat.CooldownMultiplier() / cat.String()
rewritten as the equivalent free-function calls (ShouldFallback(cat),
CooldownMultiplier(cat), CategoryString(cat)) since alias types
cannot carry methods. Behaviour unchanged; only the call shape
changed.
Tests
- sdk/errdefs/http_test.go (new): pins ClassifyProvider, mapping to
errdefs markers, nil passthrough, classification preservation,
ClassifyHTTPStatus, and ProviderCategoryFromHTTPCode. 6 test
functions, ~150 cases.
- sdk/llm/classifier_test.go (rewritten): scope narrowed to the
fallback decision primitives (CategoryString, ShouldFallback,
CooldownMultiplier, IsPermanentError) plus an alias-identity guard
and a one-row dispatcher smoke. The full classification surface is
no longer covered here — it is owned by errdefs/http_test.go.
Verified
- go build ./... + go test ./... in sdk, sdkx, voice all green.
Made-with: Cursor
LLM providers in sdkx/llm/* were emitting their dispatch / stream spans
with hand-typed attribute strings:
attribute.String("llm.provider", "openai") // hand-typed
attribute.String("llm.model", c.model) // matches AttrLLMModel
attribute.Int64("llm.input_tokens", …) // SCHEMA DRIFT
attribute.Int64("llm.output_tokens", …) // SCHEMA DRIFT
Two costs followed:
- llm.input_tokens / llm.output_tokens drifted from the canonical
sdk/telemetry attrs.go schema (AttrLLMInputTokens =
"llm.tokens.input", AttrLLMOutputTokens = "llm.tokens.output").
Dashboards summing tokens across providers had to handle two
different keys, and any future package that emits via the canonical
constants would not appear in those rollups at all.
- llm.provider had no central constant, so peer capabilities (the
future sdk/pod controller doing per-provider rate-limit / cost
splits, embedding rerank fan-out by provider, …) had no shared key
to filter on.
Constant added (sdk/telemetry/attrs.go)
AttrLLMProvider = "llm.provider"
pinned in attrs_test.go alongside the rest of the Attr* set.
Producers migrated
sdkx/llm/openai/{openai,stream}.go 4 spans, 6 attribute sites
sdkx/llm/anthropic/{anthropic,stream}.go 4 spans, 8 attribute sites
sdkx/llm/bytedance/{bytedance,stream}.go 4 spans, 6 attribute sites
sdkx/llm/ollama/{ollama,stream}.go 4 spans, 6 attribute sites
All four providers now emit:
AttrLLMProvider ("llm.provider") — was hand-typed
AttrLLMModel ("llm.model") — was hand-typed
AttrLLMInputTokens ("llm.tokens.input") — was llm.input_tokens
AttrLLMOutputTokens ("llm.tokens.output") — was llm.output_tokens
The token-key rename is a BREAKING change for any dashboard / alert
that filtered on the old llm.input_tokens / llm.output_tokens. None
in tree; the canonical schema in sdk/telemetry/attrs.go has been the
source of truth since it landed in the L0 commit, and producers in
sdk/* (sdk/llm fallback metrics) were already on the new keys.
azure / deepseek / minimax / qwen are thin wrappers around the openai
client and inherit the migration without further changes.
sdkx/knowledge/watcher: error key normalisation
watcher.go was the only telemetry call site outside sdk/* still
emitting the legacy "error" key. Switched to the canonical
AttrErrorMessage so dashboards filtering by "error.message exists"
pick up watcher diagnostics uniformly with the rest of the SDK.
Verified
- sdk/telemetry/attrs_test.go pins the new AttrLLMProvider constant.
- go build ./... + go test ./... in sdk, sdkx, voice all green.
Made-with: Cursor
…roviderError
The two embedding providers that talk to an external SaaS API
(sdkx/embedding/openai, sdkx/embedding/bytedance) were returning raw
fmt.Errorf("…: %w") wrapped errors on every failure path, leaving the
pod controller blind to whether a failure was Auth (rotate creds),
RateLimit (back off), Validation (input too large) or NotAvailable
(retry now). The chat-completion peers in sdkx/llm/* were already on
the unified path via errdefs.ClassifyProviderError; embedding is the
last cross-provider boundary that did not surface the same signals.
Producer changes
sdkx/embedding/openai/openai.go
Embed err → errdefs.ClassifyProviderError("openai", err)
EmbedBatch err → errdefs.ClassifyProviderError("openai", err)
"empty response" → errdefs.NotAvailablef
"expected N got M" → errdefs.NotAvailablef
sdkx/embedding/bytedance/bytedance.go
embedStandard err → errdefs.ClassifyProviderError("bytedance", err)
embedBatchStandard err → errdefs.ClassifyProviderError("bytedance", err)
embedMultimodal err → errdefs.ClassifyProviderError("bytedance", err)
"empty response" → errdefs.NotAvailablef
"expected N got M" → errdefs.NotAvailablef
embedBatchMultimodal: kept fmt.Errorf("…: %w") because it wraps
embedMultimodal's already-classified output and %w preserves the
inner errdefs marker through errors.As (verified in errdefs tests).
sdkx/embedding/{azure,qwen} are thin wrappers around the openai
client and inherit the migration without changes.
Why errdefs and not sdk/llm
ClassifyProviderError lives in sdk/errdefs (since the previous
commit) precisely so peer capabilities like embedding don't have to
pull sdk/llm into their import set just to reuse the HTTP-status /
keyword classifier. The function body is provider-agnostic; only
the LLM-fallback chain (FallbackLLM cooldown + ShouldFallback)
needs the LLM-domain ErrorCategory wrapper.
Verified
- go build ./... + go test ./... in sdk, sdkx, voice all green.
Made-with: Cursor
…NotAvailable
The SQL-backed retrieval indexes (sdkx/retrieval/sqlite,
sdkx/retrieval/postgres) and the SQLite recall job queue
(sdkx/recall/jobqueue/sqlite) returned raw fmt.Errorf wraps on
backend setup failure (pragma exec, namespace DDL, schema migration).
A pod that opens its index/queue at startup had no behavioural signal
to distinguish "the SQLite file is locked / Postgres is down" from
"some user-facing input is malformed", forcing every consumer to
string-match the error or treat all errors as fatal.
Producer changes
sdkx/retrieval/sqlite/index.go
Open.pragma loop → errdefs.NotAvailable(...)
ensureNS DDL exec → errdefs.NotAvailable(...)
sdkx/retrieval/postgres/index.go
ensureNS DDL exec → errdefs.NotAvailable(...)
sdkx/recall/jobqueue/sqlite/queue.go
Open.pragma loop → errdefs.NotAvailable(...)
migrate stmts loop → errdefs.NotAvailable(...)
(added errdefs import)
Why NotAvailable and not Internal
- These failures are about an external dependency that is currently
unreachable / misconfigured / locked, not a bug in our code path.
- pod policies that handle backend outages (back off, switch to a
fallback index, surface a 503) all key on errdefs.IsNotAvailable.
- Internal would route the same error into the "panic-class, page
someone" bucket, which is wrong for a transient lock or a missing
Postgres role.
Out of scope
- Per-row Exec / Query failures inside read/write paths are NOT
promoted to NotAvailable in this commit. Those are mostly already
bubbled raw and need a more careful triage (constraint violations
→ Conflict, missing rows → NotFound) than a sweep can give them;
scheduled as a follow-up once sdk/pod's policy surface is concrete
enough to validate the choices.
- sdkx/workspace/objstore stays untouched: the upper layer
sdk/workspace already classifies its own errors via sentinel
ErrNotFound; the backend errors here are caught and re-classified
one level up.
- sdkx/extract: tool-call return shape, never reaches pod retry
policy (see earlier sdkx-gap analysis).
Verified: go test ./retrieval/... ./recall/... in sdkx all green;
no consumer changes needed (NotAvailable preserves the wrapped chain
so existing error.Error() output is unchanged).
Made-with: Cursor
After hoisting the provider-error → errdefs translation into
errdefs/http.go, classifier.go and errors.go in sdk/llm held only an
LLM-domain alias bridge (ErrorCategory, CategoryXxx, CategoryString,
ShouldFallback, CooldownMultiplier, ClassifyError) plus two sentinels
(ErrAllProvidersOpen / ErrAllProvidersFailed) and a single-line
predicate (IsPermanentError) — all of them consumed only by
fallback.go and one test file. Fragmenting that into three production
files made the alias bridge feel like a public API surface, but
nothing outside sdk/llm referenced any of those symbols.
Inlined into fallback.go
- ErrAllProvidersOpen / ErrAllProvidersFailed sentinels
- shouldFallback — chain-stop policy (was ShouldFallback)
- cooldownMultiplier — per-category breaker hold (was CooldownMultiplier)
- categoryLabel — metric / log token (was CategoryString)
Call sites switched to errdefs.ProviderCategory and
errdefs.ClassifyProvider directly; the LLM-domain ErrorCategory
alias and the CategoryXxx constants were the only reason the bridge
existed and have been removed.
Deleted
- sdk/llm/classifier.go
- sdk/llm/errors.go
- sdk/llm/classifier_test.go
- IsPermanentError — zero callers anywhere in the tree; downstream
code that needs the "should I retry this" answer should branch on
the errdefs.IsXxx predicates (IsValidation / IsRateLimit / ...)
rather than borrow the LLM-fallback "permanent" lens.
- ErrorCategory / CategoryXxx public constants — replaced internally
by errdefs.ProviderCategory / errdefs.ProviderXxx; no peer code
referenced them, so removing them avoids an alias surface that
promised LLM-specific semantics it did not actually carry.
Tests
fallback_test.go absorbed the three policy tests (shouldFallback,
cooldownMultiplier, categoryLabel), now expressed in terms of the
unexported helpers and errdefs.ProviderCategory directly. The
alias-identity guard is gone with the alias itself. The single-row
ClassifyError dispatch smoke and TestIsPermanentError were dropped
along with the symbols they covered; the full classification surface
remains pinned in errdefs/http_test.go so coverage is unchanged.
Net effect: sdk/llm is now one production file (fallback.go) plus
one test file (fallback_test.go), each fully self-contained.
Verified: go build ./... + go test ./... in sdk, sdkx, voice all green.
Made-with: Cursor
…P 503)
ErrAllProvidersOpen and ErrAllProvidersFailed used to leave fallback.go
as plain stdlib errors, so by the time pod / api.HTTPStatus inspected
them they fell through to the default 500. Semantically a fallback
chain that has run out is the textbook "service is temporarily
unavailable, retry later" case (NotAvailable / 503), not an internal
bug — every breaker eventually re-closes and every upstream eventually
recovers.
Per the errdefs convention (sdk/errdefs/errors.go top doc) sentinels
and behavioural classification are orthogonal: the sentinel stays a
plain errors.New so callers keep doing identity checks via
errors.Is(err, ErrAllProvidersOpen), and the classification gets
layered on at the return sites via errdefs.NotAvailable so the same
value also satisfies errdefs.IsNotAvailable.
The wrap is performed by two new package-private helpers,
allOpenError() and allFailedError(lastErr), and NOT in the var block:
pre-wrapping the sentinel once in the var block would yield a single
shared errNotAvailable value and break errors.Is identity for any
caller that kept the plain pointer reference. The doc comment on the
var block spells this trade-off out so the next person doesn't try to
"simplify" by hoisting the wrap.
allFailedError uses fmt.Errorf("%w: %w", ...) — both unwrap branches
remain reachable, so errors.Is(err, ErrAllProvidersFailed) AND
errors.Is(err, lastErr) both keep working. This matters because pod
logs and tests want to see the underlying provider error chain even
after the chain-level wrapper is on top.
Test: TestFallbackLLM_TerminalErrors_AreNotAvailable pins all three
properties together (sentinel identity, lastErr identity, NotAvailable
classification → 503), so a future regression that breaks any of them
fails loudly. The pre-existing TestFallbackLLM_AllFail /
TestFallbackLLM_AllBreakersOpen / TestFallbackLLM_AllBreakersOpen_Stream
cases stay as-is and confirm the wrap did not break sentinel identity
on the original call paths.
Verified: go test ./... in sdk, sdkx, voice all green.
Made-with: Cursor
The four-line telemetry.Warn call inside the JSONMode extract-error branch was left at the parent's indentation depth during the earlier AttrErrorMessage migration (commit on the telemetry-attribute pass). gofmt-clean as-is, but visually misleading — the call sits inside an "if extractErr != nil" body and should be indented one more level. Pure whitespace; no behavioural change. Made-with: Cursor
lIang70
changed the title
lIang70
changed the title
lIang70
changed the title
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is the second pass of the pod-runtime convergence: it pushes the
sdk/errdefsandsdk/telemetrycontracts all the way through every external boundary in the SDK and through the four sdkx capability modules that share those boundaries. The architectural goal is that pod / api can decide HTTP status, retryability, and dashboard category by inspecting the error and the span attributes alone — without per-package switch statements.The 13 commits split into five small, ordered groups.
1. Pod-runtime primitives in errdefs / telemetry
feat(sdk/errdefs,sdk/telemetry): add pod-runtime base primitives— introduces theBudgetExceededandPolicyDeniedmarkers (so a sandbox policy refusal stops being indistinguishable from an upstream Forbidden),errdefs.FromContextfor ctx.Err() normalisation (DeadlineExceeded → Timeout, Canceled → Aborted), and the first wave oftelemetry.AttrXxxconstants.refactor(sdk): classify origin errors via errdefs+refactor(sdk): classify formatted origin errors via errdefs— sweeps every package insdk/that returns boundary errors so they go out with the right marker (NotFound / Validation / NotAvailable / Internal / ...). The two commits are split by call shape only (errors.Newvsfmt.Errorf); the categories were reviewed package by package.feat(sdk/errdefs,sdk/llm): pod-grade boundary error normalization— wires the same machinery into the LLM provider boundary (the host-facing surface ofsdk/llm, before the per-provider adapters in sdkx).2. Telemetry attribute convergence
refactor(sdk/telemetry): adopt AttrXxx constants at producers + fix attr-key drift— kills schema drift likekanban.target_agent_id(snake) vskanban.target.agent.id(dotted), and migrates existing producers off ad-hoc string literals to theAttrXxxconstants.feat(sdk/telemetry): add ConversationID/DatasetID/ErrorMessage attrs and adopt— fills the three remaining gaps and switchessdk/history,sdk/graph/node/knowledgenode, plus all error-logging sites insdk/{knowledge,recall,kanban,llm,graph,...}to the constants. After this commit no producer insdk/writes a raw attr-key string.feat(sdkx/llm,sdk/telemetry): converge LLM provider telemetry on AttrXxx— same job for the four sdkx LLM providers (openai / anthropic / bytedance / ollama), plus token-name schema unification (llm.input_tokens→llm.tokens.inputetc.) and the newAttrLLMProviderconstant.3. Hoist provider-error classification out of sdk/llm
refactor(sdk/errdefs,sdk/llm): hoist provider-error classification to errdefs— moves the generic HTTP-status / keyword / SDK-error → errdefs dispatcher (HTTPStatusCoder,ClassifyProviderError,ClassifyHTTPStatus,ProviderCategory*,ClassifyProvider) down tosdk/errdefs/http.go. The original location insdk/llmwas a layering bug: every external-provider boundary in the workspace (LLM, embedding, future rerank/...) needs the same dispatcher, but sdkx/embedding was importingsdk/llmpurely to reach it. After this commit, peers consumeerrdefs.ClassifyProviderErrordirectly.refactor(sdk/llm): inline classifier into fallback, drop alias bridge— once the dispatcher moved out,sdk/llm/classifier.goandsdk/llm/errors.gowere just an LLM-named alias bridge over the new errdefs symbols. Both files are deleted; the small amount of FallbackLLM-specific policy (shouldFallback,cooldownMultiplier,categoryLabel) is inlined intosdk/llm/fallback.goas unexported helpers.sdk/llmis now one production file plus one test file, both fully self-contained.feat(sdk/llm): classify chain-terminal sentinels as NotAvailable (HTTP 503)— wrapsErrAllProvidersOpen/ErrAllProvidersFailedwitherrdefs.NotAvailableat the return sites (not in the var block — that would breakerrors.Isidentity for callers comparing against the plain sentinel pointers). Result: identity, last-err identity, and classification all hold simultaneously, and pod / api emit 503 on a fully-exhausted chain instead of falling through to 500.4. sdkx peer adoption
feat(sdkx/embedding): normalize provider errors via errdefs.ClassifyProviderError— switches the openai / azure / qwen / bytedance embedding adapters to the hoisted dispatcher and removes thesdk/llmimport that only existed for namespace reasons. Plainfmt.Errorffailure paths (empty response, count mismatch) are wrapped witherrdefs.NotAvailablefso they reach api as 503.feat(sdkx/retrieval,sdkx/recall): classify storage backend errors as NotAvailable— wraps the SQLPRAGMA/ DDL / migrationfmt.Errorfcalls insdkx/retrieval/{sqlite,postgres}andsdkx/recall/jobqueue/sqlitewitherrdefs.NotAvailable, so a storage outage surfaces as 503 (retryable) rather than the default 500.5. Style fix-ups
style(sdk/graph/node/llmnode): fix indentation on json_mode warn block— pure whitespace, no behavioural change.Auto-tag policy after merge
.github/workflows/auto-tag.ymlruns on every push tomainthat touchessdk/**,sdkx/**, orvoice/**, bumps the patch on each affected module, and reads per-module skip markers from the head commit message.sdk/→ let auto-tag bump it; the workflow will cutsdk/v0.2.3against the merge commit. No marker needed.sdkx/→ suppressed via[skip-tag:sdkx]in the PR title. GitHub puts the PR title into the body of the merge commit it creates, and the workflow readsgit log -1 --pretty=%B(full message, including body), so the marker takes effect. The sdkx changes here are mechanical adaptations to the sdk-side errdefs hoist; they will ship in a later sdkx release on its own cadence.voice/→ not touched in this PR; auto-tag will see no diff undervoice/and skip it on its own.Test plan
go build ./...green insdk/,sdkx/,voice/against this branch.go test ./...green insdk/,sdkx/,voice/(no FAIL lines).TestFallbackLLM_TerminalErrors_AreNotAvailabletable test.sdk/errdefs/http_test.go.gofmtclean (the style commit in this PR fixed the only outstanding instance).sdk/v0.2.3and that nosdkx/v0.2.2tag was created.