-
Notifications
You must be signed in to change notification settings - Fork 954
feat: improve menu/agents relationship in dojo, improve 404 handling #781
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
base: main
Are you sure you want to change the base?
Conversation
Addresses issues where feature IDs were not correctly matched, leading to incorrect page rendering. Implements middleware to validate integration and feature IDs, returning a custom 404 page when either is invalid. Adds a new client component for feature layouts to handle rendering the content or code/readme viewers based on the URL parameters. Also removes trailing slashes from agent URLs.
Refactors middleware to set headers indicating 404 status, allowing the `FeatureLayout` component to trigger a `notFound()` call. This centralizes 404 handling and removes redundant checks in the component. Also sets `dynamicParams = false` to return 404 for invalid integration IDs.
Removes the dedicated file for integration features data. This data is now sourced directly from the `menuIntegrations` array, streamlining feature availability checks and eliminating data duplication.
Centralizes integration configurations into a single source of truth within `integrations.ts`. Removes redundant integration definitions from `menu.ts` and utilizes the centralized configuration. Updates middleware to leverage the single source of truth for validation.
Removes the separate integrations.ts file and merges its content into menu.ts, which serves as the single source of truth for integration configurations. This change streamlines the codebase and ensures consistency in integration definitions used by the UI menu, middleware, and agents.
Introduces a `defineAgents` helper function to ensure type safety when defining agent configurations. This ensures that the integration ID matches one defined in `menu.ts` and that the agent keys are a subset of the features defined for that integration. This prevents runtime errors and improves code maintainability by providing compile-time validation of agent configurations.
Improves type safety and reduces boilerplate in agent integrations by introducing a `mapAgents` utility function. This simplifies the configuration of agents that follow a consistent pattern, especially when multiple agents share the same base URL and only differ in their path. The change also updates the agent integrations to use a simple object with async functions rather than the previous `defineAgents` helper. Fixes #Implement Feature ID Matching and 404
Ensures that the integration IDs defined in `menu.ts` have corresponding entries in the `agentsIntegrations` map. Also enforces that all features defined for each integration are present in the returned object, preventing runtime errors due to missing agent implementations.
Ensures that agent integrations maps have exactly the expected features, preventing missing or extra features. This change improves type safety by leveraging TypeScript's type system to validate the agent integrations configuration. It introduces an `ExactRecord` type that verifies that a given type has only the keys specified in another type, preventing unexpected or missing features in agent definitions. The "backend_tool_rendering" feature was temporarily disabled to ensure type correctness given current agent implementations. This feature may be re-enabled in future commits after corresponding agent implementations have been updated. Fixes #implement-feature-id-matching-and-404
Updates the `mapAgents` function to correctly remove the readonly modifier from the mapped type, ensuring it matches the expected `AgentsMap` type. Removes the `ExactRecord` type as it's no longer needed due to the improved `mapAgents` type definition, leading to a simpler and more maintainable code.
Enables the 'backend_tool_rendering' feature for LangGraph and CrewAI, and 'agentic_chat_reasoning' for the Autonomous Tools integration by uncommenting them in the menu configuration. The context indicates that this change is related to implementing feature ID matching and addressing 404 errors.
brandonmcconnell
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.
This PR adds some better type checks and ensures that the agentsIntegrations features listed per integration in apps/dojo/src/agents.ts match what is in apps/dojo/src/menu.ts.
Otherwise, it complains, as it did in each of these three cases.
—
attn @tylerslaton @maxkorp @mme @ranst91
| "server-starter-all-features": async () => | ||
| mapAgents( | ||
| (path) => new ServerStarterAllFeaturesAgent({ url: `${envVars.serverStarterAllFeaturesUrl}/${path}` }), | ||
| { | ||
| agentic_chat: "agentic_chat", | ||
| // TODO: Add agent for agentic_chat_reasoning | ||
| // agentic_chat_reasoning: "agentic_chat_reasoning", | ||
| backend_tool_rendering: "backend_tool_rendering", | ||
| human_in_the_loop: "human_in_the_loop", | ||
| agentic_generative_ui: "agentic_generative_ui", | ||
| tool_based_generative_ui: "tool_based_generative_ui", | ||
| shared_state: "shared_state", | ||
| predictive_state_updates: "predictive_state_updates", | ||
| } | ||
| ), |
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.
"agentic_chat_reasoning" was missing here but is expected in the list of agents for "server-starter-all-features" in apps/dojo/src/menu.ts.
Lines 194 to 207 in 69320c9
| { | |
| id: "server-starter-all-features", | |
| name: "Server Starter (All Features)", | |
| features: [ | |
| "agentic_chat", | |
| "backend_tool_rendering", | |
| "human_in_the_loop", | |
| "agentic_chat_reasoning", | |
| "agentic_generative_ui", | |
| "predictive_state_updates", | |
| "shared_state", | |
| "tool_based_generative_ui", | |
| ], | |
| }, |
| crewai: async () => | ||
| mapAgents( | ||
| (path) => new CrewAIAgent({ url: `${envVars.crewAiUrl}/${path}` }), | ||
| { | ||
| agentic_chat: "agentic_chat", | ||
| // TODO: Add agent for backend_tool_rendering | ||
| // backend_tool_rendering: "backend_tool_rendering", | ||
| human_in_the_loop: "human_in_the_loop", | ||
| tool_based_generative_ui: "tool_based_generative_ui", | ||
| agentic_generative_ui: "agentic_generative_ui", | ||
| shared_state: "shared_state", | ||
| predictive_state_updates: "predictive_state_updates", | ||
| } | ||
| ), |
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.
"backend_tool_rendering" was missing here but is expected in the list of agents for "crewai" in apps/dojo/src/menu.ts.
Lines 160 to 172 in 69320c9
| { | |
| id: "crewai", | |
| name: "CrewAI", | |
| features: [ | |
| "agentic_chat", | |
| "backend_tool_rendering", | |
| "human_in_the_loop", | |
| "agentic_generative_ui", | |
| "predictive_state_updates", | |
| "shared_state", | |
| "tool_based_generative_ui", | |
| ], | |
| }, |
| "langgraph-typescript": async () => | ||
| mapAgents( | ||
| (graphId) => new LangGraphAgent({ deploymentUrl: envVars.langgraphTypescriptUrl, graphId }), | ||
| { | ||
| agentic_chat: "agentic_chat", | ||
| // TODO: Add agent for backend_tool_rendering | ||
| // backend_tool_rendering: "backend_tool_rendering", | ||
| agentic_generative_ui: "agentic_generative_ui", | ||
| human_in_the_loop: "human_in_the_loop", | ||
| predictive_state_updates: "predictive_state_updates", | ||
| shared_state: "shared_state", | ||
| tool_based_generative_ui: "tool_based_generative_ui", | ||
| subgraphs: "subgraphs", | ||
| } | ||
| ), |
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.
"backend_tool_rendering" was missing here but is expected in the list of agents for "langgraph-typescript" in apps/dojo/src/menu.ts.
Lines 34 to 46 in 69320c9
| id: "langgraph-typescript", | |
| name: "LangGraph (Typescript)", | |
| features: [ | |
| "agentic_chat", | |
| "backend_tool_rendering", | |
| "human_in_the_loop", | |
| "agentic_generative_ui", | |
| "predictive_state_updates", | |
| "shared_state", | |
| "tool_based_generative_ui", | |
| "subgraphs", | |
| ], | |
| }, |
brandonmcconnell
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.
@ranst91 I wanted to make sure this PR properly preserves your recent work adding LangChain and doesn't impose any issues like showing the menu item(s) for it prematurely.
Two comments here to address, below.
_
cc @tylerslaton @maxkorp @mme
| langchain: async () => | ||
| mapAgents( | ||
| new LangChainAgent({ | ||
| // TODO: @ranst91 - can you add param types here? | ||
| // @ts-expect-error - TODO: add types | ||
| chainFn: async ({ messages, tools, threadId }) => { | ||
| const { ChatOpenAI } = await import("@langchain/openai"); | ||
| const chatOpenAI = new ChatOpenAI({ model: "gpt-4o" }); | ||
| const model = chatOpenAI.bindTools(tools, { | ||
| strict: true, | ||
| }); | ||
| return model.stream(messages, { tools, metadata: { conversation_id: threadId } }); | ||
| }, | ||
| }), | ||
| { | ||
| agentic_chat: "", | ||
| tool_based_generative_ui: "", | ||
| } | ||
| ), |
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.
Could you add types for the three untyped object parameters here?
{ messages, tools, threadId }| { | ||
| id: "langchain", | ||
| name: "LangChain", | ||
| features: [ | ||
| "agentic_chat", | ||
| "tool_based_generative_ui", | ||
| ], | ||
| }, |
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.
Adding this block for langchain to menu.ts will display it in the menu. I want to make sure we are clear to including them, or did you want them to stay unlisted for now?
If you want the agents in agents.ts but not have them appear in the menu, I'll just need to make a small tweak to my TS type checks to permit additional integrations in agents.ts that don't exist in menu.ts, and then we can remove/comment them here until we're ready.
Ensure we return a 404 code for any pages that do not exist, whether integrations or features. This PR achieves that using Next.js middleware.
This PR also improves how we handle type checks in
agents.tsandmenu.tsto consolidate some of the redundancy in our multiple "sources of truth" and ensures that bothmenu.tsalways satisfiesagents.ts.