Implement Claude provider and foundation scaffold#2
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @ryaker, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the essential groundwork for the Zora autonomous AI agent, focusing on establishing a robust and extensible foundation. It introduces the core TypeScript type system, a flexible TOML-based configuration engine with validation, and a comprehensive test infrastructure. Crucially, it also integrates the first functional LLM provider, Claude, demonstrating the agent's multi-provider architecture. This foundational work ensures that subsequent feature development can proceed on a stable and well-defined platform, without yet implementing the full agentic execution loop or tool functionalities. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Code Review
This is an impressive foundational PR. It establishes a solid, well-architected base for the Zora project. The use of TypeScript with strict settings, dependency injection for testability, a clear type system, and a robust configuration engine are all excellent choices. The Claude provider implementation is particularly well-designed with its lazy-loading of the SDK and resilient error handling. The test coverage for the new components is also very thorough. I have a few suggestions for minor improvements regarding documentation consistency, algorithmic efficiency in one of the validation functions, a small refactoring in the Claude provider, and an incomplete test case. Overall, this is a high-quality submission that sets the project up for success.
| ## Foundation scaffold: type system, config engine, and test infrastructure | ||
|
|
||
| > Tier 1, Items 1–3 of the [WSJF Implementation Plan](specs/v5/IMPLEMENTATION_PLAN.md) | ||
| > 35 files changed · 3,630 insertions · 62 tests passing · clean `tsc --noEmit` |
There was a problem hiding this comment.
There are a couple of minor inconsistencies in the statistics mentioned here and in other documentation files:
- This file states "62 tests passing", while the main PR body mentions "70 passing tests".
- The toolchain is described as using "TypeScript 5.7", but
package.jsonspecifies^5.7.0andpackage-lock.jsonhas resolved this to version5.9.3.
Aligning these details across the documentation would improve clarity and prevent confusion.
| const names = config.providers.map((p) => p.name); | ||
| const dupes = names.filter((n, i) => names.indexOf(n) !== i); | ||
| if (dupes.length > 0) { | ||
| errors.push(`Duplicate provider names: ${[...new Set(dupes)].join(', ')}`); | ||
| } |
There was a problem hiding this comment.
The current implementation for finding duplicate provider names uses filter with indexOf, which has a time complexity of O(n²). While this is fine for a small number of providers, a more efficient O(n) approach using a Set would be more scalable and is generally better practice. The same applies to the duplicate rank check on lines 175-179.
A suggested refactoring for finding duplicate names:
const seenNames = new Set<string>();
const duplicateNames = new Set<string>();
for (const provider of config.providers) {
if (seenNames.has(provider.name)) {
duplicateNames.add(provider.name);
} else {
seenNames.add(provider.name);
}
}
if (duplicateNames.size > 0) {
errors.push(`Duplicate provider names: ${[...duplicateNames].join(', ')}`);
}| if (options.queryFn) { | ||
| this._queryFn = options.queryFn; | ||
| } else { | ||
| // Lazy import to avoid requiring the SDK at construction time | ||
| // This will be resolved at first execute() call | ||
| this._queryFn = null as unknown as QueryFn; | ||
| } |
There was a problem hiding this comment.
The lazy initialization of _queryFn can be simplified to avoid type assertions and as any. By changing the property declaration on line 138 to private _queryFn: QueryFn | null;, you can simplify the constructor logic and the _resolveQueryFn method for better type safety and readability.
this._queryFn = options.queryFn ?? null;| it('aborts an active query', async () => { | ||
| const mockQuery = new MockSDKQuery([]); | ||
| const queryFn = vi.fn(() => mockQuery); | ||
| const provider = new ClaudeProvider({ config, queryFn }); | ||
|
|
||
| const task = makeTask({ jobId: 'job-to-abort' }); | ||
|
|
||
| // Start execution but don't await completion yet | ||
| const iterator = provider.execute(task); | ||
| const firstEventPromise = iterator.next(); | ||
|
|
||
| await provider.abort('job-to-abort'); | ||
|
|
||
| const result = await firstEventPromise; | ||
| expect(result.done).toBe(false); // Should have yielded 'done' or 'error' in finally block | ||
|
|
||
| // In our implementation, finally block yields 'done' or similar if not aborted? | ||
| // Wait, execute() yields done after loop. | ||
| // If we abort, the SDK query should stop yielding. | ||
| }); |
There was a problem hiding this comment.
This test case for aborting a query appears to be incomplete. It calls provider.abort() but doesn't have any assertions to verify that the query was actually aborted or that the execute generator behaved as expected after the abort signal. It would be great to complete this test to ensure the abort functionality works correctly. For example, you could assert that the abort method on the mock query was called.
User description
Summary
This PR implements Tier 1, Items 1–4 of the Zora WSJF Implementation Plan. It establishes the project foundation (type system, config engine, test infrastructure) and provides the first functional LLM provider implementation using the Claude Agent SDK.
Changes
1. Project Scaffolding
tsconfig.json,vitest.config.ts).2. Core Type System (
src/types.ts)LLMProvider,AuthStatus,QuotaStatus,AgentEvent,TaskContext, etc.3. Config Engine (
src/config/)smol-toml.4. Claude Provider (
src/providers/claude-provider.ts)ClaudeProviderbacked by@anthropic-ai/claude-agent-sdk.execute()as anAsyncGenerator<AgentEvent>for streaming.abort()support.queryFnfor clean unit testing.5. Documentation
POLICY_PRESETS.mdandWEB_ONBOARDING_SPEC.md.README.mdand installation guides.Testing
70 passing tests across 4 suites:
defaults.test.ts(27): Config validation and defaults.loader.test.ts(12): TOML loading and merging.mock-provider.test.ts(23): Provider interface compliance.claude-provider.test.ts(8): Claude-specific logic, error handling, and event mapping.Clean
tsc --noEmit(npm run lint).Verified on macOS (darwin).
All tests passing
Linting clean
Documentation updated
CodeAnt-AI Description
Add Claude LLM provider, TOML config parsing, and unit tests
What Changed
Impact
✅ Can load TOML configs✅ Clearer config validation errors✅ Claude provider available for local Mac-session authentication💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.