feat: update IO and improve types, update discord example #64
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@ponderingdemocritus has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces a comprehensive refactoring of the input/output handler system across multiple files. The primary changes involve renaming Changes
Sequence DiagramsequenceDiagram
participant Client
participant Orchestrator
participant IOHandler
Client->>Orchestrator: registerIOHandler
Orchestrator->>IOHandler: Store handler with execute method
Client->>Orchestrator: dispatchToInput/Output
Orchestrator->>IOHandler: Call execute method
IOHandler-->>Orchestrator: Return processed data
Orchestrator-->>Client: Provide result
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
packages/core/src/core/processors/research-processor.ts (1)
Line range hint
205-213: Fix type error inbuildHandlerSchemaPart.The pipeline failure indicates that
outputSchemais not defined in theIOHandlertype. This needs to be fixed to match the new type system.Update the
IOHandlerinterface in the types file:interface IOHandler { name: string; role: HandlerRole; + outputSchema?: z.ZodSchema; execute: (data: any) => Promise<any>; }🧰 Tools
🪛 GitHub Actions: CI
[error] 205-205: Property 'outputSchema' does not exist on type 'IOHandler'
examples/example-api.ts (1)
Line range hint
96-112: Add error handling for GitHub API requests.The handler should handle potential API failures, rate limits, and invalid responses.
execute: async (payload) => { const { repo } = payload as { repo: string }; const response = await fetch( `https://api.github.com/repos/${repo}/issues` ); + if (!response.ok) { + throw new Error( + `GitHub API request failed: ${response.status} ${response.statusText}` + ); + } const issues = await response.json(); return issues; },
🧹 Nitpick comments (11)
examples/example-discord.ts (2)
23-23: Consider making the log level configurable.
Currently,LogLevel.DEBUGis hard-coded. For production scenarios, shifting this to an environment variable or config file might be beneficial.- const loglevel = LogLevel.DEBUG; + const loglevel = process.env.LOG_LEVEL + ? LogLevel[process.env.LOG_LEVEL.toUpperCase()] || LogLevel.DEBUG + : LogLevel.DEBUG;
32-33: Caution about purging data.
Purging the vector database on startup is handy for development, but be sure to remove or conditionally execute this in production to avoid unintended data loss.packages/core/src/core/types/index.ts (3)
536-557: InputIOHandler structure.
Consider adding type parameters toexecuteandsubscribefor stronger typing instead of(data: any) => .... This will help avoid runtime type confusion.-export interface InputIOHandler extends BaseIOHandler { +export interface InputIOHandler<T = any> extends BaseIOHandler { role: HandlerRole.INPUT; - execute?: (data: any) => Promise<unknown>; + execute?: (data: T) => Promise<unknown>; - subscribe?: (onData: (data: any) => void) => () => void; + subscribe?: (onData: (data: T) => void) => () => void; }
560-585: OutputIOHandler with enforced output schema.
Terrific for type-safety. Again, consider parameterizingexecuteto match the schema type.
587-609: ActionIOHandler with optional outputSchema.
If actions require input validation, consider renamingoutputSchemato something likeactionSchemaorparamSchemafor clarity. Otherwise, this looks great.examples/example-basic.ts (2)
89-93: Renaming from “handler” to “execute” alongside “outputSchema.”
The changes are consistent with the new IOHandler architecture. Consider specifying stricter types fordatainexecuterather thanany, ensuring alignment with the Zod schema.
Line range hint
115-128: Renaming to “execute” and “outputSchema” for GraphQL fetch.
Same suggestion: ensure the data inexecutematches the shape enforced byoutputSchema.packages/core/src/core/io/discord.ts (1)
125-134: LGTM! Good type safety improvements.The addition of generic type parameter and proper schema validation enhances type safety.
Consider adding type assertion to ensure T extends MessageData:
- public createMessageOutput<T>(): IOHandler { + public createMessageOutput<T extends MessageData>(): IOHandler {packages/core/src/core/chain-of-thought.ts (1)
1086-1087: LGTM! Improved type safety and property validation.The changes correctly implement:
- Type casting to
OutputIOHandler- Validation of required properties
- Standardized naming using
outputSchemaHowever, we could make the error message more descriptive.
Consider improving the error message to be more specific:
- return `No handler registered for action type "${action.type}" try again`; + return `No valid handler registered for action type "${action.type}". Handler must implement 'execute' and 'outputSchema' properties.`;Also applies to: 1092-1095
examples/example-twitter.ts (2)
Line range hint
138-155: LGTM! Well-structured output handler with proper validation.The schema correctly validates tweet content for ASCII characters. Consider adding a length validation to ensure tweets don't exceed Twitter's 280-character limit.
outputSchema: z .object({ content: z .string() + .max(280, "Tweet content must not exceed 280 characters") .regex( /^[\x20-\x7E]*$/, "No emojis or non-ASCII characters allowed" ), })
Line range hint
167-182: LGTM! Well-structured reply handler with proper validation.Consider adding the same length validation as suggested for the tweet handler.
outputSchema: z .object({ - content: z.string(), + content: z + .string() + .max(280, "Tweet content must not exceed 280 characters"), inReplyTo: z .string() .optional() .describe("The tweet ID to reply to, if any"), })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
examples/example-api.ts(5 hunks)examples/example-basic.ts(3 hunks)examples/example-discord.ts(3 hunks)examples/example-goal.ts(3 hunks)examples/example-server.ts(1 hunks)examples/example-twitter.ts(4 hunks)packages/core/src/core/chain-of-thought.ts(4 hunks)packages/core/src/core/io/discord.ts(5 hunks)packages/core/src/core/orchestrator.ts(8 hunks)packages/core/src/core/processors/message-processor.ts(4 hunks)packages/core/src/core/processors/research-processor.ts(1 hunks)packages/core/src/core/types/index.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
packages/core/src/core/processors/research-processor.ts
[error] 205-205: Property 'outputSchema' does not exist on type 'IOHandler'
🔇 Additional comments (38)
examples/example-discord.ts (16)
2-4: Documentation looks clear.
No issues found here.
8-8: Import statement confirmed.
No issues found.
20-20: Added Discord.js Message import.
Correct usage, no issues found.
38-38: Sample model usage acknowledged.
No issues found.
42-42: No issues with message processor comment.
49-49: Connecting to MongoDB.
No issues found.
58-58: Clearing tasks.
No concerns.
61-61: Orchestrator creation.
No concerns.
74-74: Discord client initialization.
No concerns.
79-79: Passing loglevel into Discord client.
No concerns.
82-93: Possible mismatch with comment about mentions.
The comment says “On mention, it pipes data,” but the actual logic streams all incoming messages without mention checks. Please verify if this behavior matches your desired design.
98-102: Output handler registration verified.
Looks good. Valid approach to hooking Discord’s message output.
110-113: Console logs for debugging are fine.
No issues found.
115-115: Graceful shutdown comment acknowledged.
No issues found.
119-121: Removing streaming IO handler
This ensures the subscription (and resources) are properly cleaned up. Good approach.
122-122: Verify “discord_reply” handler name.
core.removeIOHandler("discord_reply")assumes the output handler is registered under that name. Confirm thatdiscord.createMessageOutput()uses"discord_reply"internally or rename as needed.packages/core/src/core/types/index.ts (3)
512-525: Expanded documentation for I/O operations.
No functional issues found; the new doc block clearly explains how to register IO handlers.
528-534: BaseIOHandler introduced.
This is a straightforward interface that standardizes thenameproperty. Looks good.
610-611: Union type for IOHandler.
This unifies all handler types. Good approach.packages/core/src/core/io/discord.ts (3)
24-32: LGTM! Well-structured schema definition.The zod schema is clear, well-documented with descriptions, and properly handles optional fields.
100-111: LGTM! Clean implementation of stream cleanup.The method properly handles the cleanup of message listeners and includes appropriate logging.
136-160: LGTM! Improved error handling and response structure.The method now provides detailed error information and consistent response structure.
packages/core/src/core/processors/message-processor.ts (4)
3-9: LGTM! Type imports are well-organized.The addition of
ActionIOHandlerandOutputIOHandlertypes improves type safety.
53-54: LGTM! Type definitions are more specific.Using
OutputIOHandler[]andActionIOHandler[]instead ofIOHandler[]provides better type safety.
106-110: LGTM! Clear guidelines for message replies.The new thinking block provides clear criteria for when to reply to messages:
- When mentioned in the message
- When able to provide deep help
- Never respond to self-messages
66-66: Consider safer type handling instead of non-null assertions.Using
!assertion operator withoutputSchemacould lead to runtime errors if the schema is undefined. Consider adding a type guard or providing a default schema.Also applies to: 72-72
examples/example-goal.ts (1)
107-115: LGTM! Consistent naming conventions.The renaming of
handlertoexecuteandschematooutputSchemaimproves consistency across the codebase.Also applies to: 137-150
packages/core/src/core/orchestrator.ts (2)
81-81: LGTM! Improved subscription management.The addition of the
unsubscribersmap and proper cleanup inremoveIOHandlerprevents memory leaks and ensures proper resource management.Also applies to: 98-111, 124-134
143-143: LGTM! Consistent handler execution checks.The changes standardize how handler execution is checked and error messages are consistent across different dispatch methods.
Also applies to: 157-162, 182-189, 254-254, 265-265, 537-538, 544-544
packages/core/src/core/chain-of-thought.ts (2)
9-9: LGTM! Type safety improvement.The addition of
OutputIOHandlertype import enhances type safety for output handlers.
1150-1160: LGTM! Robust output handling implementation.The changes effectively:
- Implement type-safe output handling
- Filter outputs based on schema availability
- Generate schema documentation correctly
The implementation is clean and aligns well with the type system.
Also applies to: 1228-1228
examples/example-twitter.ts (2)
Line range hint
95-115: LGTM! Clean implementation of the Twitter mentions input handler.The handler correctly processes mentions and properly structures the output data.
Line range hint
121-132: LGTM! Clean implementation of the consciousness thoughts input handler.The handler correctly processes autonomous thoughts and handles empty cases.
examples/example-server.ts (2)
78-85: LGTM! Clean implementation of the user chat input handler.The schema correctly validates user input and the handler properly passes through the payload.
Line range hint
90-101: LGTM! Clean implementation of the chat reply output handler.The schema correctly validates the reply structure and the handler properly processes the message.
examples/example-api.ts (3)
Line range hint
117-157: LGTM! Well-implemented universal API call handler.The handler properly validates input, makes the request, and handles both JSON and text responses with proper error handling.
Line range hint
166-176: LGTM! Clean implementation of the user chat input handler.The schema correctly validates user input and the handler properly passes through the payload.
Line range hint
181-196: LGTM! Clean implementation of the UI chat reply output handler.The schema correctly validates the reply structure and the handler properly processes the message.
packages/core/src/core/io/discord.ts
Outdated
| if (message.author.displayName == "DeepLoaf") { | ||
| console.log("Skipping message from DeepLoaf"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the user filter configurable.
The hardcoded check for "DeepLoaf" should be made configurable to improve maintainability and flexibility.
Consider this implementation:
- if (message.author.displayName == "DeepLoaf") {
- console.log("Skipping message from DeepLoaf");
- return;
- }
+ const ignoredUsers = this.config?.ignoredUsers ?? [];
+ if (ignoredUsers.includes(message.author.displayName)) {
+ this.logger.debug("DiscordClient", `Skipping message from ignored user: ${message.author.displayName}`);
+ return;
+ }Add the configuration type:
interface DiscordClientConfig {
ignoredUsers?: string[];
}| // @ts-ignore - no idea why this is chucking error and i cannot be bothered to fix it | ||
| const sentMessage = await channel.send(data.content); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-ignore by fixing the type assertion.
The @ts-ignore comment should be replaced with proper type handling.
Try this approach:
- // @ts-ignore - no idea why this is chucking error and i cannot be bothered to fix it
- const sentMessage = await channel.send(data.content);
+ const sentMessage = await (channel as TextChannel).send(data.content);Add the import:
import { TextChannel } from "discord.js";
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
examples/example-discord.ts (3)
74-81: Move bot name to environment variables.The bot name "DeepLoaf" is hardcoded. Consider moving it to environment variables for better configurability.
const discord = new DiscordClient( { discord_token: env.DISCORD_TOKEN, - discord_bot_name: "DeepLoaf", + discord_bot_name: env.DISCORD_BOT_NAME, }, loglevel );
83-98: Add type safety to the onData callback.The
onDatacallback lacks proper typing, which could lead to runtime errors.core.registerIOHandler({ name: "discord_stream", role: HandlerRole.INPUT, - subscribe: (onData) => { + subscribe: (onData: (data: { content: string; channelId: string; sentBy?: string }) => void) => { discord.startMessageStream((incomingMessage: Message) => { onData(incomingMessage); }); return () => { discord.stopMessageStream(); }; }, });
Line range hint
116-129: Add error handling to cleanup process.Consider wrapping the cleanup operations in try-catch blocks to ensure graceful shutdown even if individual cleanup steps fail.
process.on("SIGINT", async () => { console.log(chalk.yellow("\n\nShutting down...")); - // If we want to stop the streaming IO handler: - core.removeIOHandler("discord_stream"); - - // Also remove any other handlers or do cleanup - core.removeIOHandler("discord_reply"); - rl.close(); + try { + // If we want to stop the streaming IO handler: + await core.removeIOHandler("discord_stream"); + + // Also remove any other handlers or do cleanup + await core.removeIOHandler("discord_reply"); + rl.close(); + } catch (error) { + console.error(chalk.red("Error during cleanup:"), error); + } console.log(chalk.green("✅ Shutdown complete")); process.exit(0); });packages/core/src/core/io/discord.ts (6)
Line range hint
1-33: Use zod schema to derive MessageData interface.Consider using zod's type inference to ensure the interface and schema stay in sync.
-export interface MessageData { - content: string; - channelId: string; - conversationId?: string; - sendBy?: string; -} export const messageSchema = z.object({ content: z.string().describe("The content of the message"), channelId: z.string().describe("The channel ID where the message is sent"), sendBy: z.string().optional().describe("The user ID of the sender"), conversationId: z .string() .optional() .describe("The conversation ID (if applicable)"), }); + +export type MessageData = z.infer<typeof messageSchema>;
Line range hint
38-72: Enhance error handling during initialization.Consider adding retry logic for login and proper cleanup if initialization fails.
constructor( private credentials: DiscordCredentials, logLevel: LogLevel = LogLevel.INFO ) { + if (!credentials.discord_token || !credentials.discord_bot_name) { + throw new Error("Missing required credentials"); + } + this.client = new Client({ intents: [ GatewayIntentBits.Guilds, GatewayIntentBits.GuildMessages, GatewayIntentBits.MessageContent, GatewayIntentBits.GuildMembers, GatewayIntentBits.DirectMessages, GatewayIntentBits.DirectMessageTyping, GatewayIntentBits.DirectMessageReactions, ], partials: [Partials.Channel], }); this.logger = new Logger({ level: logLevel, enableColors: true, enableTimestamp: true, }); this.client.on(Events.ClientReady, () => { this.logger.info("DiscordClient", "Initialized successfully"); }); - this.client.login(this.credentials.discord_token).catch((error) => { - this.logger.error("DiscordClient", "Failed to login", { error }); - console.error("Login error:", error); - }); + const maxRetries = 3; + let retries = 0; + + const attemptLogin = async () => { + try { + await this.client.login(this.credentials.discord_token); + } catch (error) { + this.logger.error("DiscordClient", "Failed to login", { error }); + if (retries < maxRetries) { + retries++; + this.logger.info("DiscordClient", `Retrying login (${retries}/${maxRetries})...`); + await new Promise(resolve => setTimeout(resolve, 1000 * retries)); + return attemptLogin(); + } + throw error; + } + }; + + attemptLogin().catch((error) => { + this.logger.error("DiscordClient", "Failed to initialize after retries", { error }); + throw error; + }); }
74-116: Improve message stream handling and type safety.Consider adding more robust message filtering and proper typing for the message stream.
- public startMessageStream(onData: (data: any) => void) { + public startMessageStream(onData: (data: Omit<MessageData, 'conversationId'>) => void) { this.logger.info("DiscordClient", "Starting message stream..."); this.messageListener = (message: Message) => { - if ( - message.author.displayName == this.credentials.discord_bot_name - ) { - console.log( - `Skipping message from ${this.credentials.discord_bot_name}` - ); + if (this.shouldSkipMessage(message)) { + this.logger.debug("DiscordClient", "Skipping message", { + author: message.author.displayName, + reason: "bot message" + }); return; } onData({ content: message.content, channelId: message.channelId, sentBy: message.author?.id, }); }; this.client.on(Events.MessageCreate, this.messageListener); } + private shouldSkipMessage(message: Message): boolean { + return ( + message.author.displayName === this.credentials.discord_bot_name || + message.author.bot || + message.content.trim().length === 0 + ); + }
121-125: Ensure thorough cleanup in destroy method.Consider adding checks and proper error handling during cleanup.
public destroy() { + try { this.stopMessageStream(); + this.client.removeAllListeners(); this.client.destroy(); this.logger.info("DiscordClient", "Client destroyed"); + } catch (error) { + this.logger.error("DiscordClient", "Error during cleanup", { error }); + throw error; + } }
130-139: Improve type safety in createMessageOutput.Consider constraining the generic type and adding runtime type checking.
- public createMessageOutput<T>(): IOHandler { + public createMessageOutput<T extends MessageData>(): IOHandler { return { role: HandlerRole.OUTPUT, name: "discord_message", execute: async (data: T) => { + const result = messageSchema.safeParse(data); + if (!result.success) { + throw new Error(`Invalid message data: ${result.error}`); + } return await this.sendMessage(data as MessageData); }, outputSchema: messageSchema, }; }
Line range hint
141-211: Improve error handling in sendMessage method.Consider adding more specific error types and better error messages.
+ private class DiscordError extends Error { + constructor(message: string, public readonly code?: string) { + super(message); + this.name = 'DiscordError'; + } + } private async sendMessage(data: MessageData): Promise<{ success: boolean; messageId?: string; content?: string; error?: string; }> { try { this.logger.info("DiscordClient.sendMessage", "Sending message", { data, }); if (env.DRY_RUN) { this.logger.info( "DiscordClient.sendMessage", "Dry run enabled", { data, } ); return { success: true, messageId: "DRY_RUN", content: "DRY_RUN", - error: "DRY_RUN", + error: undefined, }; } if (!data?.channelId || !data?.content) { - return { - success: false, - error: "Channel ID and content are required", - }; + throw new DiscordError('Channel ID and content are required', 'MISSING_REQUIRED_FIELDS'); } const channel = this.client.channels.cache.get(data?.channelId); if (!channel?.isTextBased()) { - const error = new Error( + throw new DiscordError( `Invalid or unsupported channel: ${data.channelId}` + 'INVALID_CHANNEL' ); - this.logger.error( - "DiscordClient.sendMessage", - "Error sending message", - { - error, - } - ); - throw error; } const sentMessage = await channel.send(data.content); return { success: true, messageId: sentMessage.id, content: data.content, error: undefined, }; } catch (error) { this.logger.error( "DiscordClient.sendMessage", "Error sending message", { error, } ); return { success: false, - error: error instanceof Error ? error.message : "Unknown error", + error: error instanceof DiscordError + ? `${error.message} (${error.code})` + : error instanceof Error + ? error.message + : "Unknown error", }; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/example-discord.ts(3 hunks)packages/core/src/core/io/discord.ts(5 hunks)packages/core/src/core/processors/research-processor.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/core/processors/research-processor.ts
🔇 Additional comments (3)
examples/example-discord.ts (2)
1-21: LGTM! Clear documentation and well-organized imports.The documentation clearly explains the transition to streaming IOHandler, and the imports are appropriately organized.
99-114: LGTM! Clear structure and well-documented setup.The output handler registration and console setup are well-organized with clear comments explaining each section.
packages/core/src/core/io/discord.ts (1)
189-191: Remove @ts-ignore by fixing the type assertion.The @ts-ignore comment should be replaced with proper type handling.
Summary by CodeRabbit
Release Notes
New Features
Improvements
handlertoexecute).outputSchema.Changes
Refactoring