-
Notifications
You must be signed in to change notification settings - Fork 7
Add @ocap/kernel-agents package
#668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
0f7fb26 to
bbc62fa
Compare
rekmarks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great to see your sojourn in agent-land come together in such a well-conceived package. See various comments in-line. We should be able to get this merged in no time!
| /** | ||
| * Test constants for E2E tests | ||
| */ | ||
| export const DEFAULT_MODEL = 'llama3.1:latest'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, how did we choose this and often do we expect to change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fit for the test setup: available in ollama, relatively small
Fit for purpose: trained for tool use, trained to produce json outputs
In practice the prompts should be optimized to the models; the test suite is biased toward this choice of model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the stuff in here be exported from the language service package? Or perhaps repo-tools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe later. The suite.test.ts tests that the local ollama server is running and prepared correctly (i.e. the model is pulled), which is a sort of liveness assurance above and beyond what the language service package promises. Putting the constants in repo-tools graduates them to a scope that exceeds current need.
This suggestion does point to broader question of separating the code and the data for the agent framework; for example, prompts are.... probably code? But also model dependent. And models are.... probably data? But we are using them to determine flow control of our program.
I think leaving these under the kernel-agents/test/ directory is the tidiest choice for the moment.
| return { | ||
| task: async ( | ||
| query: string, | ||
| { invocationBudget = 10 }: { invocationBudget?: number } = {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the number of times the agent can invoke capabilities before the task throws. You can imagine that by the 10th call to a tool without completing the task, this particular approach is probably not going to succeed.
rekmarks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I meant to request changes in my previous review)
d219945 to
f748c0d
Compare
rekmarks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Let's leave remaining comments unresolved. Some of it is documentation that should arguably be committed, but at least we can leave it easily accessible for Those Who Follow (i.e. ourselves, next week).
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
6437b85 to
84cbf32
Compare
| logger?.info('results:', results); | ||
| const didEnd = results.find((capability) => capability.name === 'end'); | ||
| if (didEnd) { | ||
| logger?.info('exit invocation with result:', didEnd.result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ); | ||
| expect(result).toBeDefined(); | ||
| expect(countSpy).toHaveBeenCalled(); | ||
| expect(result).includes(word.length.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Invalid Matcher Usage in Tests
The tests incorrectly use .includes() directly on the expect() object, which isn't a valid Vitest/Jest matcher. This means the assertions aren't checking what's intended.
Additional Locations (1)
This PR contains two minor fixes for the recently added kernel agents package (#668). - Two test expectations at the end of agent end to ends tests were no ops - The agent's task method was logging to the base logger instead of the taskLogger
This PR introduces a new
@ocap/kernel-agentspackage that provides capability-enabled AI agents.Changes
New package
@ocap/kernel-agentsmakeAgentfactory for producing capability-augmented AI agents from aLanguageModeland aCapabilityRecord@ocap/kernel-language-model-serviceLanguageModel.sampleto return{ stream, abort }, which allows downstream consumers to stop the LLM from generating more tokens (useful when you know the generation is invalid midstream)sampleinterfaceNote to Reviewers
See
README.mdfor local e2e test setup.Note
Adds a new capability-driven agent package and changes LanguageModel.sample to return an abortable stream ({ stream, abort }).
@ocap/kernel-agentsmakeAgentwith defaultendcapability and capability invocation flow.messages,prompt, example transcripts/capabilities) and incremental JSON streaming parser.@ocap/kernel-language-model-service)LanguageModel.samplenow returns{ stream, abort }(abortable streaming).typesentry.packages/kernel-agentsto TS project refs and Vitest coverage thresholds; workspace wiring in lockfile.Written by Cursor Bugbot for commit 84cbf32. This will update automatically on new commits. Configure here.