-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add Dojo as a source in cairo-coder #90
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
enitrat
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 on ingester side (minus some missing files to scan)
If you want to have a good evaluation, first what I would do is gather a dataset from the logs we have, filter for dojo-related queries, run the Eval with the cairo-coder program, and compare against a "specific" dojo program to see if there's a substancial difference
this can be for a future PR and we can keep this one specifically for the ingester
| } else if ( | ||
| entry.isFile() && | ||
| path.extname(entry.name) === fileExtensions | ||
| ) { |
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.
this doesnt feel right — how can you compare path.extname (a string) to fileExtensions that is supposed to be an Array ?
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.
ok so actually you need to either rename fileExtensions back to fileExtension OR take an array as input
| } else if ( | ||
| entry.isFile() && | ||
| path.extname(entry.name).toLowerCase() === fileExtension // TODO: handle multiple extensions |
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.
not implemented & formatting is off, run trunk check --fix
| path.extname(entry.name).toLowerCase() === this.config.fileExtension | ||
| ) { |
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.
you shouldn't introduce an extra outer loop here; and rather just do something in the lines of
this.config.fileExtension.includes(path.extname(entry.name).toLowerCase())
| entry.isFile() && | ||
| path.extname(entry.name).toLowerCase() === config.fileExtension |
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.
config.fileExtension.includes(path.extname(entry.name).toLowerCase())| DocumentSource.SCARB_DOCS: "Scarb Documentation. For using the Scarb package manager: building, compiling, generating compilation artifacts, managing dependencies, configuration of Scarb.toml.", | ||
| DocumentSource.STARKNET_JS: "StarknetJS Documentation. For using the StarknetJS library: interacting with Starknet contracts, (calls and transactions), deploying Starknet contracts, front-end APIs, javascript integration examples, guides, tutorials and general JS/TS documentation for starknet.", | ||
| DocumentSource.STARKNET_BLOG: "Starknet Blog Documentation. For latest Starknet updates, announcements, feature releases, ecosystem developments, integration guides, and community updates. Useful for understanding recent Starknet innovations, new tools, partnerships, and protocol enhancements.", | ||
| DocumentSource.DOJO_DOCS: "Dojo Documentation. For building onchain games and autonomous worlds using the Dojo framework: entity component system (ECS) patterns, world contracts, models, systems, events, indexing with Torii (GraphQL API, entity subscriptions, real-time state synchronization), SDKs and client libraries including dojo.js (TypeScript/JavaScript integration, entity queries, world interactions), Rust SDK (torii-client, world state queries, account management, transaction execution, real-time subscriptions), dojo.c (C bindings for native integrations, WASM32 support), dojo.unity (Unity C# SDK with codegen plugin, World Manager), dojo.godot (Godot integration with live testnet demos), Telegram bot SDK (@dojoengine packages), support for Unreal Engine modules, Sozo CLI tool (build, migration, deployment), Katana devnet, game development patterns on Starknet.", |
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.
sounds good; last thing you'd then need to do is to to modify the signature field of the optimized_retrieval_program (which is loaded at runtime, containing the optimized instructions), and add his string at the end
dojo_docs: Dojo Documentation. For ...
enitrat
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
enitrat
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.
actually there's an issue with mapping of online pages, example:
https://book.dojoengine.org/pages/framework/index#what-is-dojo
should be https://book.dojoengine.org/framework#what-is-dojo
| const docsDir = path.join(extractDir, 'docs'); | ||
|
|
||
| logger.info(`Processing documentation files in ${docsDir}`); | ||
| const pages = await processDocFiles(this.config, docsDir); |
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 think if here you just do smth like
for (const page of pages) {
page.name = page.name.replace('pages/', '');
}that would fix it
Objectives
Add an new agent that can help starknet user with their dojo book.
Changement