Skip to content

docs(sdks): language-agnostic SDK specification - #162

Open
adnanrhussain wants to merge 6 commits into
mainfrom
ahussain/sdk_spec
Open

docs(sdks): language-agnostic SDK specification#162
adnanrhussain wants to merge 6 commits into
mainfrom
ahussain/sdk_spec

Conversation

@adnanrhussain

Copy link
Copy Markdown
Collaborator

Summary

Formalizes sdks/SPEC.md as the normative blueprint for all Evaluator SDKs — current (TypeScript, Python) and future. The spec is prescriptive: it defines the right contracts, and SDKs are brought into conformance with it.

Structure

  • Design principles (§1) — 11 principles every rule derives from (one canonical name; idiomatic surface / identical core; universal envelope; fail fast/loud/structured; observability never affects results; sensitive data opt-in; declared determinism; units in names; core vs. optional capabilities; executable spec)
  • Canonical contracts (§§2–8) — naming & value forms, config, input validation, result envelope + payload rules, error taxonomy with data-driven retryability, logging, telemetry
  • Optional capabilities (§9) — not required per SDK, but contract-bound when built (batch evaluation)
  • Evaluator registry (§10) — evaluator definitions are shared, language-neutral data; definition schema, model pinning, prompt hygiene, derived inputs
  • Process & lifecycle (§§11–12) — new-evaluator/new-SDK conformance checklist, contract fixtures as the executable spec, governance, spec versioning, compatibility & deprecation policy
  • Appendices — A: known conformance gaps in current SDKs (snapshot 2026-08-18), B: per-SDK conformance matrix, C: open questions, D: changelog

Key decisions

  • One canonical name across languages, chosen to avoid builtins by construction (RequestTimeoutError, InputValidationError/OutputValidationError split)
  • All durations in milliseconds with _ms suffix (retry_after_ms)
  • Result envelope {evaluator, result, metadata} with per-evaluator payload shapes; profile taxonomy deliberately deferred (Q-2)
  • Stability levels (Stable/Experimental) so the spec can move fast without breaking users
  • partner_keypartner_api_key; new telemetry.tracking_key (semantics: Q-8)
  • Hierarchical evaluator IDs (literacy.ela_reading.vocabulary), taxonomy: Q-7

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 18, 2026 22:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new normative, language-agnostic specification document (sdks/SPEC.md) intended to define the canonical contracts that all Evaluator SDKs (current and future) must implement, including configuration, validation, result envelope, errors, logging, telemetry, registry direction, and lifecycle/governance.

Changes:

  • Introduces the initial SDK spec (v0.1.0) with required core contracts (config, validation, results, errors, logging, telemetry).
  • Defines optional capabilities (batch evaluation) with contract requirements when implemented.
  • Establishes direction for a shared evaluator registry + cross-SDK contract fixtures and includes conformance/gap tracking appendices.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread sdks/SPEC.md Outdated
Comment thread sdks/SPEC.md Outdated
Comment thread sdks/SPEC.md Outdated
Comment thread sdks/SPEC.md Outdated
@michael-berger-czi

Copy link
Copy Markdown

Is this spec for our reference as we develop our SDKs, or is this for our end users when they're evaluating the sdks?

@adnanrhussain

Copy link
Copy Markdown
Collaborator Author

Is this spec for our reference as we develop our SDKs, or is this for our end users when they're evaluating the sdks?

@michael-berger-czi - We are currently the primary builders and maintainers of the SDK. Eventually this model may evolve into a community-driven open-source approach, and so I would imagine codifying the spec is helpful for anyone contributing, or as you shared, trying to understand the SDK

@michael-berger-czi michael-berger-czi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that a lot of things will become out of date (like parameters, event fields and output shape), and it will be an ongoing cost to remember to update this document as these low level details change. Recommend focusing this document on high level concerns, but open to discussing if you think otherwise.

Comment thread sdks/SPEC.md
| `google_api_key` | string | none | Required when `default_providers` includes `google` or `model_override.provider` is `google` |
| `openai_api_key` | string | none | Required when `default_providers` includes `openai` or `model_override.provider` is `openai` |
| `anthropic_api_key` | string | none | Required when `default_providers` includes `anthropic` or `model_override.provider` is `anthropic` |
| `partner_api_key` | string | none | Learning Commons–issued partner key; forwarded as auth on telemetry requests, never in event bodies |

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should rename this to learning_commons_api_key or lc_api_key to be explicit. What do folks think?

@czi-fsisenda czi-fsisenda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🚀
Lots of comments, but just because we are covering so much here.
I'm sure this will evolve a lot as use cases we never imagined come up.
Only 2 issues I feel strongly about for this iteration:

  1. Full exception stack trace must be available to sdk users. Exception error messages should be as descriptive as possible - even including sensitive info if necessary. Agree that SDK doesn't log sensitive info or include sensitive info in telemetry.
  2. OutputValidationErrors should retry immediately. No backoff.

Comment thread sdks/SPEC.md

Every rule in this spec derives from one of these. When the spec is silent, decide by principle, then codify the decision here.

1. **One canonical name.** Every public identifier has exactly one canonical name, identical in every SDK and all documentation. Casing (§2.1) is the only permitted transformation; names are chosen to avoid known language builtins by construction (`RequestTimeoutError`, not `TimeoutError`).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had issues with this when implementing the Python SDK. Typescript used ValidationError which conflicted with Pydantics ValidationError to disambiguate them for Python we are using InputValidationError.
I think we can do best effort, but this may not always work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we reserve this recommendation for terms we own directly, like eval names?

Comment thread sdks/SPEC.md

3. **Universal envelope, evaluator-scoped payload.** Every result shares one envelope; the domain payload is defined per evaluator in the registry (§10), under shared invariants (§5.2).

4. **Fail fast, fail loud, fail structured.** Configuration problems surface at construction. Evaluation failures are canonical errors (§6) carrying retryability as data. Every error MUST be diagnosable from the error alone — silent fallbacks, swallowed causes, and generic messages are defects.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

Comment thread sdks/SPEC.md

5. **Observability never affects results.** Telemetry and logging MUST NOT throw, block, delay, or change an evaluation's outcome.

6. **No hidden state.** The full structured model output MUST be surfaced in every result — never empty, never omitted.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No optional fields in output?

Comment thread sdks/SPEC.md

6. **No hidden state.** The full structured model output MUST be surfaced in every result — never empty, never omitted.

7. **Sensitive data is opt-in.** Raw user-supplied input text, and raw provider error strings (which may echo inputs or key fragments), MUST NOT appear in telemetry, logs, or error messages unless the caller explicitly opts in. Model outputs, scores, and reasoning are product data and MAY be logged and reported.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree for telemetry and somewhat for logs, but I think we shouldn't filter error messages.

  • Telemetry: We have a flag for including full inputs in telemetry so should it be MUST NOT unless explicitly and prominently opted-in for telemetry.
  • Logs: We should be careful about logging in general, but can sensitive info show up in debug logs?
  • Error messages: I think error messages should have as much detail as is available including sensitive info. The context for using the SDK is as code owner. The code owner should decide what to do with the error message. They can decide whether to log it, whether to include it in output, when and where it is available. If we sanitize the error, we limit the user's ability to diagnose issues. The CLI tool may need a different policy though.

Comment thread sdks/SPEC.md
| `google_api_key` | string | none | Required when `default_providers` includes `google` or `model_override.provider` is `google` |
| `openai_api_key` | string | none | Required when `default_providers` includes `openai` or `model_override.provider` is `openai` |
| `anthropic_api_key` | string | none | Required when `default_providers` includes `anthropic` or `model_override.provider` is `anthropic` |
| `partner_api_key` | string | none | Learning Commons–issued partner key; forwarded as auth on telemetry requests, never in event bodies |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be learning_commons_api_key since that is what we are using for this.

Comment thread sdks/SPEC.md
- Every normative change adds a changelog line ([Appendix D](#appendix-d-spec-changelog)).
- Open design questions live in [Appendix C](#appendix-c-open-questions) — promoted into the spec or closed with a recorded rationale, never silently dropped.

### 12.2 Versioning

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we state that same eval spec should have same eval version in every SDK?
So we have SDK version that explicitly references language-agnostic SDK spec version and each eval has it's own version based on eval spec?
Do SDK versions have to be similar? e.g. if there's a new major version of a language and we need a new major version of SDK, is that fine?

Comment thread sdks/SPEC.md

### 12.2 Versioning

- The spec is SemVer-versioned, independent of SDK packages. Pre-1.0, minor bumps may break; post-1.0, breaking changes to Stable content require a major bump.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Not clear which spec we are referring to.

Comment thread sdks/SPEC.md

**Deprecation:** a renamed or removed surface keeps a working, warning-emitting alias for at least one minor release of every affected SDK.

**Evaluation-behavior changes** (model or prompt updates) are versioned in the registry and communicated in SDK changelogs even when the API is unchanged — users experience a score shift as a breaking change, whatever SemVer says.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do we mean SDK when we say API here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does eval SemVer drive SDK SemVer? If some eval has a breaking change, even if the SDKs have no other changes, then the SDKs all get a version bump?

Comment thread sdks/SPEC.md

| # | Question | Notes |
|---|---|---|
| Q-1 | Should SDKs read API keys from environment variables as a fallback to explicit config? | UX win for scripts vs. surprise credential pickup in servers; precedence and naming must be canonical if adopted |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Always read keys from explicit config AND config construction can fallback to env variables. That way users can trust config. OR remove all keys from config and always read keys from env variables.

Comment thread sdks/SPEC.md
|---|---|---|
| Q-1 | Should SDKs read API keys from environment variables as a fallback to explicit config? | UX win for scripts vs. surprise credential pickup in servers; precedence and naming must be canonical if adopted |
| Q-2 | Payload shape taxonomy — should evaluator families (QTC, feedback, standards alignment, …) share declared payload profiles? | Deliberately deferred (§5.2); revisit once feedback and math evaluator classes mature |
| Q-3 | Should registry definitions carry a version (prompt/model revision) surfaced in result `metadata`? | Would let users pin/detect evaluation-behavior changes (§12.3) programmatically |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants