Feat configurable agent as lib#6
Conversation
…t usage - Rename package to @eggai/configurable-agent and add exports["./lib"] - Add src/lib/index.ts as the public library surface (runAgent, prepareMessages, types) - Enable declaration emit in tsconfig.build.json for TypeScript consumers - Add openai-compatible provider to ModelProvider enum and buildModel() - Add apiKey field to model config schema - Accumulate inputTokens/outputTokens across steps and emit in final event - Add @vitest/coverage-v8 for coverage reporting
…nsumers - Add "default" condition to ./lib exports map so CJS loaders (qualops) can resolve the package without ERR_PACKAGE_PATH_NOT_EXPORTED - Re-export jsonSchema from 'ai' via lib/index.ts so consumers don't need a direct ai dependency to wrap plain JSON Schema objects
Captures x-ratelimit-* headers from the finish-step stream part and emits a rate_limit_tokens error event when remaining tokens fall below 5% of the limit. Passes any partial response text as partialContent so callers can attempt recovery. Also propagates partial content on stream_error.
Extract step-completion error detection into a pure exported function `diagnoseStep` so new error conditions can be added in one place without touching the loop control flow. Add `finishReason === 'length'` detection as `max_tokens_reached` error alongside the existing TPM rate-limit check. Both conditions attach `partialContent` when text was accumulated before the failure so callers can attempt partial recovery. Restructure tests: five fast `diagnoseStep` unit tests (no mock model) plus three integration tests that exercise the full loop path.
QualOps Code Quality AnalysisStatus: ✅ PASSED - No issues found Summary
No issues found in the analyzed code. 📊 Full ReportPowered by QualOps |
5588f9a to
64e57d4
Compare
9eae18e to
14631e0
Compare
- Remove private:true to allow npm publishing - Add npm-publish.yml: provenance-based publish (no token needed) triggered by merging release/vX.Y.Z branches, matching qualops release process - Add create-release-pr.yml: bumps version, updates CHANGELOG, opens release PR - Add changelog job to CI: required for PRs touching src/, tests/, package.json - Add CHANGELOG.md with initial 0.1.0 entry
| stopReason: string; | ||
| steps: number; | ||
| truncated: boolean; | ||
| usage?: { inputTokens: number; outputTokens: number }; |
There was a problem hiding this comment.
Great idea. Should this be a mandatory field?
| provider: ModelProvider, | ||
| name: z.string().min(1), | ||
| baseUrl: z.string().url().optional(), | ||
| apiKey: z.string().optional(), |
There was a problem hiding this comment.
I don't think the api key should be part of the config, because that would make the whole config a secret.
| "@biomejs/biome": "^1.9.0", | ||
| "@types/node": "^22.10.0", | ||
| "@types/shell-quote": "^1.7.5", | ||
| "@vitest/coverage-v8": "^4.1.4", |
Review: provider-specific handling in
|
| Provider | Token rate-limit headers | Result |
|---|---|---|
| OpenAI | x-ratelimit-{limit,remaining,reset}-tokens |
works |
| Anthropic | anthropic-ratelimit-tokens-{limit,remaining,reset} (+ separate input-tokens/output-tokens buckets); reset is an RFC-3339 timestamp, not "59s" |
never matches |
none (429 + RetryInfo) |
never matches | |
| Ollama | none | never matches |
| openai-compatible (Mistral/Groq/Together/…) | header names vary per backend | mostly never matches |
diagnoseStep isn't even passed the provider, so it can't branch. The message text is OpenAI-semantics-specific too: "input consumed X/Y tokens, leaving only Z for output" assumes OpenAI's single TPM bucket — Anthropic tracks input and output limits separately, so that framing is wrong there, and resets in ${resetTokens} would print a raw ISO timestamp.
Correctness issue (provider-independent)
Even on OpenAI, the rate-limit branch fires at the wrong time and severity. The headers reflect remaining budget after a successful response. So when remaining/limit < 0.05, the loop emits an error and returns — aborting the loop and discarding a completed, successful step. Being near the TPM ceiling isn't a failure of this request; it's a warning about the next one. The added integration test encodes this: a successful stream (finishReason: stop) gets turned into a fatal rate_limit_tokens error. The 5% threshold is also arbitrary and a false-positive risk for small buckets.
The provider-agnostic place to detect rate limiting is the actual 429, which the AI SDK surfaces uniformly as APICallError (.statusCode, .responseHeaders, .isRetryable). Right now it just falls into the catch as agent_failed.
What's good
finishReason === 'length'→max_tokens_reachedis correct and genuinely provider-agnostic (the SDK normalizes every provider's stop reason). Keep this.partialContenton stream errors andusageaccumulation are provider-neutral — good additions.openai-compatibleprovider +apiKeyschema are reasonable. Minor:requiredEnvVarForreturnsnullso a key-requiring endpoint gets no validation, andapiKeyin the config file invites plaintext secrets (env fallback already exists).
Suggested direction
- Preferred: drop proactive header parsing for rate limits; keep the
finishReason==='length'diagnosis; handle real rate limits by catchingAPICallError/429 from the SDK and emitting a typedrate_limiterror there — uniform across all five providers, fires only on actual failure. - If you want pre-emptive "budget low" signaling: thread
config.model.providerintodiagnoseStep, add a per-provider header map (incl. Anthropic's two-bucket scheme), normalize the reset format, and emit it as a non-fatal warning that doesn't abort a successful step. - At minimum, name it for what it is (OpenAI) so it doesn't read as general rate-limit handling.
The tests would have caught this if any rate-limit case used Anthropic-shaped headers; currently they all use the OpenAI names, so the gap is invisible in CI.
27682b3 to
fb8aa9c
Compare
Allows configurable agent to be added as a package dependency to other projects