Migrate AgentRunner to Agent Workflow (Python) #502
Conversation
🦋 Changeset detectedLatest commit: b60618a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis pull request documents and implements a migration from the legacy AgentRunner to the new Agent Workflow. It introduces a new changeset file, refactors and removes outdated chat engine and multiagent code, and streamlines template installation and workflow creation. Updates span both backend Python workflows and frontend chat UI components, including restructuring API endpoints, models, and error handling. A new dependency on zod is added to support schema validations in the Next.js code. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatEndpoint
participant WorkflowCreator
participant StreamHandler
Client->>ChatEndpoint: POST /chat_request (chat data)
ChatEndpoint->>WorkflowCreator: create_workflow(params)
WorkflowCreator-->>ChatEndpoint: Workflow instance
ChatEndpoint->>StreamHandler: Initialize stream (with callbacks)
StreamHandler-->>ChatEndpoint: Process workflow events
ChatEndpoint->>Client: Return streaming response via vercel_stream
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)`templates/**`: For files under the `templates` folder, do n...
⏰ Context from checks skipped due to timeout of 90000ms (42)
🔇 Additional comments (3)
✨ Finishing Touches
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
Documentation and Community
|
templates/types/streaming/fastapi/app/api/routers/vercel_response.py
Outdated
Show resolved
Hide resolved
templates/components/multiagent/python/app/api/routers/vercel_response.py
Outdated
Show resolved
Hide resolved
leehuwuj
force-pushed
the
lee/agent-workflows
branch
from
ecfd352 to
5a230be
Compare
leehuwuj
force-pushed
the
lee/agent-workflows
branch
from
b58913e to
7e23d77
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
questions/datasources.ts (1)
22-27: LGTM! Consider using an early return pattern for better readability.The conditional check for excluding the "No datasource" option in FastAPI is correct and aligns with the E2E test configuration.
Consider refactoring to use an early return pattern for better readability:
if (selectedDataSource === undefined || selectedDataSource.length === 0) { - if (framework !== "fastapi") { - choices.push({ - title: "No datasource", - value: "none", - }); - } + if (framework === "fastapi") { + choices.push({ + title: + process.platform !== "linux" + ? "Use an example PDF" + : "Use an example PDF (you can add your own data files later)", + value: "exampleFile", + }); + return choices; + } + + choices.push({ + title: "No datasource", + value: "none", + });templates/types/streaming/fastapi/app/api/callbacks/source_nodes.py (2)
25-30: Simplify nested conditional checks.
You can merge the nestedifs into a single condition for readability.Below is an optional refactor:
-def _is_retrieval_result_event(self, event: Any) -> bool: - if isinstance(event, ToolCallResult): - if event.tool_name == "query_index": - return True - return False +def _is_retrieval_result_event(self, event: Any) -> bool: + return ( + isinstance(event, ToolCallResult) + and event.tool_name == "query_index" + )🧰 Tools
🪛 Ruff (0.8.2)
26-27: Use a single
ifstatement instead of nestedifstatements(SIM102)
31-59: Conditional URL assignment is clear and robust.
Checks forpipeline_id, private file handling, and fallback URL are well-structured. Consider logging or flagging when no URL can be inferred, if that is a concern for debugging or monitoring.templates/types/streaming/nextjs/app/components/ui/chat/tools/chat-tools.tsx (1)
23-26: Consider adding type safety for annotations.The type assertion
as MessageAnnotation[]could be unsafe.Consider adding runtime type checking:
- const annotations = message.annotations as MessageAnnotation[] | undefined; + const annotations = message.annotations?.map(annotation => { + if (!isMessageAnnotation(annotation)) { + console.warn('Invalid annotation type detected'); + return null; + } + return annotation; + }).filter((a): a is MessageAnnotation => a !== null);templates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsx (2)
219-246: Consider stricter validation forraw_output.
Currently, thez.custom<WeatherData>()call does not fully validate the shape ofWeatherData. This could lead to potential runtime errors if theraw_outputdoes not conform to theWeatherDatainterface.You may consider defining a dedicated Zod schema that matches your
WeatherDatainterface to ensure complete, type-safe validation. For example:- raw_output: z.custom<WeatherData>(), + raw_output: z.object({ + latitude: z.number(), + longitude: z.number(), + // ... define remaining fields as needed + }),
248-295: Enhance handling of grouped events and errors.
While grouping events bytool_idworks for a single event, it may be worth handling subsequent events (e.g., separated output events). Also, consider displaying a dedicated error state ifis_erroris present.If you anticipate multiple events for the same
tool_id, extend the grouping logic to capture and merge subsequent “output” events. Moreover, you can add a fallback UI foris_errorto inform the user about invalid or failed requests.templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx (3)
12-31: Validateraw_outputmore thoroughly if possible.
Your schema usesz.record(z.unknown())forraw_output. If you know the structure ofraw_output, consider using a stricter schema to avoid potential runtime type issues.
38-85: Enhance feedback for potential errors.
While this component neatly groups annotation events, it shows no distinct handling for error cases (e.g.,is_error). If partial or incorrect data is returned, you might want to inform the user or display an appropriate fallback instead of a mere loading state.
90-120: Consider optional chaining fornode.node.
When creatingSourceNode[], ensure you have a guarantee thatnode.nodeis always defined. Otherwise, optional chaining(node.node?.id_?)can prevent runtime errors if the structure is unexpectedly missing.templates/types/streaming/nextjs/app/components/ui/chat/custom/deep-research-card.tsx (1)
148-155: LGTM! Good use of memoization.The
useMemohook effectively prevents unnecessary state recalculations. The annotation filtering is clear and type-safe.Consider destructuring
annotationsfrommessageto make the dependency array more explicit:- }, [message.annotations]); + const { annotations } = message; + }, [annotations]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.changeset/cool-cars-promise.md(1 hunks).github/workflows/e2e.yml(1 hunks)helpers/python.ts(0 hunks)questions/datasources.ts(1 hunks)templates/components/engines/python/chat/engine.py(0 hunks)templates/components/engines/python/chat/node_postprocessors.py(0 hunks)templates/components/multiagent/python/app/api/routers/chat.py(2 hunks)templates/components/multiagent/python/app/api/routers/vercel_response.py(0 hunks)templates/components/multiagent/python/app/workflows/function_calling_agent.py(0 hunks)templates/types/streaming/fastapi/app/api/callbacks/source_nodes.py(1 hunks)templates/types/streaming/fastapi/app/api/routers/chat.py(3 hunks)templates/types/streaming/fastapi/app/api/routers/events.py(1 hunks)templates/types/streaming/fastapi/app/api/routers/models.py(1 hunks)templates/types/streaming/fastapi/app/api/routers/query.py(2 hunks)templates/types/streaming/fastapi/app/api/routers/vercel_response.py(2 hunks)templates/types/streaming/fastapi/app/engine/engine.py(3 hunks)templates/types/streaming/nextjs/app/components/ui/chat/chat-message-content.tsx(1 hunks)templates/types/streaming/nextjs/app/components/ui/chat/custom/deep-research-card.tsx(2 hunks)templates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx(2 hunks)templates/types/streaming/nextjs/app/components/ui/chat/tools/chat-tools.tsx(2 hunks)templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx(1 hunks)templates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsx(2 hunks)templates/types/streaming/nextjs/package.json(2 hunks)
💤 Files with no reviewable changes (5)
- templates/components/engines/python/chat/node_postprocessors.py
- templates/components/multiagent/python/app/api/routers/vercel_response.py
- templates/components/engines/python/chat/engine.py
- templates/components/multiagent/python/app/workflows/function_calling_agent.py
- helpers/python.ts
✅ Files skipped from review due to trivial changes (2)
- templates/types/streaming/fastapi/app/api/routers/events.py
- .changeset/cool-cars-promise.md
🧰 Additional context used
📓 Path-based instructions (1)
`templates/**`: For files under the `templates` folder, do n...
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
templates/types/streaming/fastapi/app/api/routers/query.pytemplates/components/multiagent/python/app/api/routers/chat.pytemplates/types/streaming/fastapi/app/engine/engine.pytemplates/types/streaming/nextjs/app/components/ui/chat/tools/chat-tools.tsxtemplates/types/streaming/nextjs/package.jsontemplates/types/streaming/nextjs/app/components/ui/chat/chat-message-content.tsxtemplates/types/streaming/fastapi/app/api/routers/chat.pytemplates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsxtemplates/types/streaming/nextjs/app/components/ui/chat/custom/deep-research-card.tsxtemplates/types/streaming/fastapi/app/api/routers/models.pytemplates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsxtemplates/types/streaming/fastapi/app/api/callbacks/source_nodes.pytemplates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsxtemplates/types/streaming/fastapi/app/api/routers/vercel_response.py
🪛 Ruff (0.8.2)
templates/types/streaming/fastapi/app/api/callbacks/source_nodes.py
26-27: Use a single if statement instead of nested if statements
(SIM102)
🔇 Additional comments (31)
.github/workflows/e2e.yml (1)
22-22:❓ Verification inconclusive
LGTM! The change aligns with the FastAPI data source configuration.
The removal of the
--no-filesoption from Python E2E tests is consistent with the changes inquestions/datasources.tswhere the "No datasource" option is excluded for FastAPI framework.Let's verify that this change doesn't affect other frameworks:
🏁 Script executed:
#!/bin/bash # Description: Verify that the `--no-files` option is still available for non-FastAPI frameworks. # Test: Search for the `--no-files` option in the codebase. rg -A 5 $'--no-files'Length of output: 462
LGTM! The removal of the
--no-filesoption from the Python E2E tests aligns with the FastAPI datasource configuration changes inquestions/datasources.ts—excluding the "No datasource" option as intended. However, the initial verification command encountered an error due to incorrect argument handling. To ensure that non‑FastAPI frameworks still use the--no-filesoption, please re-run the verification with the corrected command:#!/bin/bash # Verify that the '--no-files' option is still available for non-FastAPI frameworks. rg --fixed-strings -- '--no-files'Once confirmed that the
--no-filesoption remains for non‑FastAPI cases, this change can be fully approved.templates/types/streaming/fastapi/app/api/routers/query.py (1)
6-6: LGTM! Type safety improvement.The explicit type annotation for the
responsevariable enhances code clarity and type safety.Also applies to: 29-29
templates/components/multiagent/python/app/api/routers/chat.py (1)
8-8: LGTM! Enhanced URL handling for source nodes.The addition of
AddNodeUrlcallback improves the chat functionality by processing URLs for source nodes.Also applies to: 49-49
templates/types/streaming/fastapi/app/engine/engine.py (2)
31-32: LGTM! Added validation for tools.The runtime check ensures at least one tool is provided, preventing potential issues.
34-38: LGTM! Successful migration to AgentWorkflow.The migration from
AgentRunnertoAgentWorkflowis implemented correctly, maintaining the required functionality.templates/types/streaming/fastapi/app/api/routers/models.py (1)
106-112: LGTM! Improved annotations validation.The changes enhance data integrity by:
- Using Optional[List[Annotation]] for better type clarity
- Adding validation to ensure only valid Annotation instances are included
templates/types/streaming/fastapi/app/api/callbacks/source_nodes.py (3)
1-11: Module import organization appears fine.
No immediate issues found with the imports or logger setup.
12-23: Solid event handling logic.
The mainrunmethod is straightforward, checking the event type and updating node metadata. It looks correct and maintainable.
60-62: Factory method is clean and minimal.
No adjustments needed.templates/types/streaming/fastapi/app/api/routers/chat.py (4)
5-17: Imports and references updated properly.
The new imports for callbacks and engine are correct. Removal of legacy references seems aligned with the new workflow.
42-55: Streaming response design is cohesive.
ReturningStreamHandler.from_default(...).vercel_stream()with the specified callbacks is a clean solution. The callback chain is well-defined.
78-89: Non-streaming endpoint logic matches the updated engine usage.
Extracting the content fromAgentOutputor serializing theresponseworks well. No direct issues seen.
92-92: Returning result in correct format.
Wrapping the content in anAssistantroleMessageis consistent with the schema.templates/types/streaming/fastapi/app/api/routers/vercel_response.py (4)
1-4: Async and type imports align with the streaming context.
No concerns here.
26-32: Constructor injection for the stream handler is sound.
This design improves testability and decoupling compared to passing multiple parameters.
33-63: Well-structured async generator for streaming events.
Catchesasyncio.CancelledErrorand logs other exceptions properly. Ensure you’ve tested scenarios where partial data is yielded or the request is cancelled mid-stream.
64-103: Focused text streaming logic.
The_stream_textmethod correctly handles different event result types. Theconvert_text,convert_data, andconvert_errorclass methods provide clean separation of concerns.templates/types/streaming/nextjs/package.json (2)
41-41:✅ Verification successful
Consider updating zod to the latest version.
The current version (3.24.2) might be outdated.
🌐 Web query:
What is the latest stable version of zod?💡 Result:
The latest stable version of Zod is 3.24.2, released 12 days ago[1][4][5]. This version includes features like asynchronous validation support in
z.custom()and various dependency updates[4].For context:
- Zod follows semantic versioning with regular updates
- The package receives frequent maintenance (e.g., 3.24.2 addresses security patches in dependencies)[4]
- Canary builds like
3.24.0-canaryexist for testing but aren't considered stable[1]You can install the latest stable version using:
npm install [email protected]Citations:
- 1: https://npm.io/package/zod
- 2: https://zod.dev/CHANGELOG
- 3: https://www.npmjs.com/package/zod/v/3.5.0
- 4: https://github.com/colinhacks/zod/releases
- 5: https://www.npmjs.com/package/zod
- 6: https://github.com/zenseact/zod
Zod version is current and requires no update.
The dependency"zod": "^3.24.2"intemplates/types/streaming/nextjs/package.jsonis already set to the latest stable release (3.24.2). No changes are necessary.
20-20:❓ Verification inconclusive
Verify the version compatibility of @llamaindex/chat-ui.
The update from 0.0.14 to 0.1.0 is a major version change that might introduce breaking changes.
Also applies to: 40-41
🌐 Web query:
What are the breaking changes in @llamaindex/chat-ui version 0.1.0 compared to 0.0.14?💡 Result:
The provided search results do not contain specific information about breaking changes in
@llamaindex/chat-uiversions0.1.0vs0.0.14. The latest available documentation (source 7) shows@llamaindex/chat-uiat version0.0.15as of February 2025, with no mention of av0.1.xrelease in any search results.For accurate information about breaking changes between these specific versions (
v0.1.xvsv0.0.x), you would need to consult:
- The official GitHub repository's release notes for
create-llama- The npm registry's version history for
@llamaindex/chat-ui- The project's changelog files (like those shown for other packages in source 2)
The search results only show general chat UI improvements up to February 2025 (source 8), but none specifically addressing these version comparisons for
@llamaindex/chat-ui.Citations:
- 1: https://docs.llamaindex.ai/en/v0.10.22/CHANGELOG/
- 2: https://github.com/run-llama/create-llama/blob/main/CHANGELOG.md
- 3: https://www.protecto.ai/blog/llamaindex-0-11-major-update-new-features-improvements/
- 4: [Bug]: MistralAI as_chat_engine() broken after upgrade to llama-index-llms-mistralai 0.1.5 llama_index#11926
- 5: https://docs.llamaindex.ai/en/v0.10.17/changes/changelog.html
- 6: https://blog.langchain.dev/the-new-langchain-architecture-langchain-core-v0-1-langchain-community-and-a-path-to-langchain-v0-1/
- 7: https://www.npmjs.com/package/@llamaindex/chat-ui
- 8: https://www.llamaindex.ai/blog/the-latest-updates-to-llamacloud
- 9: https://docs.llamaindex.ai/en/stable/CHANGELOG/
- 10: [Bug]: Breaking changes makes it hard to work with llama_index llama_index#10747
Action Required: Verify the @llamaindex/chat-ui version update
The dependency update in
templates/types/streaming/nextjs/package.jsonbumps@llamaindex/chat-uifrom0.0.14to0.1.0, which is a major version change. However, available documentation currently lists the latest stable version as0.0.15(with no explicit release notes for0.1.x). This raises concerns about potential, undocumented breaking changes. Please double-check the following:
- Confirm that the version update to
0.1.0is intentional and supported by corresponding changelog or release notes (for example, by reviewing the official GitHub repository or the project's changelog).- Ensure that any breaking changes in this update have been addressed in the codebase.
- Note that the same change applies to lines 40–41.
templates/types/streaming/nextjs/app/components/ui/chat/tools/chat-tools.tsx (1)
13-17: LGTM! Good use of hooks to reduce prop drilling.The change from prop-based message passing to using the
useChatMessagehook improves code maintainability and follows React best practices.templates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx (3)
396-415: LGTM! Good use of zod for runtime type validation.The schema provides strong type safety for the artifact tool data structure.
268-276: Review sandbox iframe security settings.The current sandbox permissions might be too permissive.
Consider restricting the sandbox permissions further:
- sandbox="allow-forms allow-scripts allow-same-origin" + sandbox="allow-scripts"Also, verify if all these permissions are necessary for your use case.
419-450: LGTM! Well-structured component with proper error handling.The component effectively uses hooks and includes proper error handling for schema validation.
templates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsx (1)
1-4: Imports look good.
These newly introduced imports are all used appropriately in this file. No issues found.templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx (2)
1-10: Imports are appropriate and consistent.
No issues found in these new imports; they are all utilized in this module.
33-36: Type definition is straightforward.
TheGroupedIndexQuerytype is simple and clear, no issues here.templates/types/streaming/nextjs/app/components/ui/chat/chat-message-content.tsx (2)
1-1: New imports align with the updated component logic.
The newly added imports (WeatherToolComponent,RetrieverComponent) are used in the refactored layout. No issues found.Also applies to: 3-3, 5-5, 6-6
9-23: Refactored static rendering approach looks clean.
Laying out each child component systematically within<ChatMessage.Content>simplifies the structure. If you need conditional rendering (e.g., to hide unused components), consider optional checks.templates/types/streaming/nextjs/app/components/ui/chat/custom/deep-research-card.tsx (3)
3-3: LGTM! Good use of hooks and utility functions.The change from prop-based to hook-based message handling improves component reusability and state management. The
getCustomAnnotationutility function nicely encapsulates the annotation filtering logic.Also applies to: 146-146
56-58: LGTM! Clean interface definition.The simplified props interface correctly reflects the component's dependencies after moving to hook-based message handling.
157-157: LGTM! Clean conditional rendering.The early return pattern with null check is a clear and effective way to handle the case when no deep research events are present.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
templates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsx (3)
174-221: Enhance accessibility for weather information display.Consider the following accessibility improvements:
- Add ARIA labels for weather icons
- Add semantic HTML elements for temperature and weather status
- Add proper color contrast for text on the blue background
Apply this diff to improve accessibility:
-<div className="bg-[#61B9F2] rounded-2xl shadow-xl p-5 space-y-4 text-white w-fit"> +<div className="bg-[#61B9F2] rounded-2xl shadow-xl p-5 space-y-4 text-white w-fit" role="region" aria-label="Weather Information"> <div className="flex justify-between"> <div className="space-y-2"> - <div className="text-xl">{currentDayString}</div> + <time className="text-xl" dateTime={data.current.time}>{currentDayString}</time> <div className="text-5xl font-semibold flex gap-4"> - <span> + <span role="text" aria-label={`Temperature: ${data.current.temperature_2m} ${data.current_units.temperature_2m}`}> {data.current.temperature_2m}{" "} {data.current_units.temperature_2m} </span> - {weatherCodeDisplayMap[data.current.weather_code].icon} + <span role="img" aria-label={weatherCodeDisplayMap[data.current.weather_code].status}> + {weatherCodeDisplayMap[data.current.weather_code].icon} + </span> </div> </div> - <span className="text-xl"> + <span className="text-xl" role="text" aria-label={`Weather status: ${weatherCodeDisplayMap[data.current.weather_code].status}`}> {weatherCodeDisplayMap[data.current.weather_code].status} </span> </div>
224-241: Consider strengthening type safety in the schema definition.The schema allows for optional fields and nested optionals which could lead to runtime type checking complexity.
Consider flattening the optional nesting:
- tool_output: z.optional( - z - .object({ - content: z.string(), - tool_name: z.string(), - raw_input: z.record(z.unknown()), - raw_output: z.custom<WeatherData>(), - is_error: z.boolean().optional(), - }) - .optional(), - ), + tool_output: z.object({ + content: z.string(), + tool_name: z.string(), + raw_input: z.record(z.unknown()), + raw_output: z.custom<WeatherData>(), + is_error: z.boolean().optional(), + }).optional(),
277-289: Consider extracting loading state into a separate component.The loading state JSX could be extracted into a reusable component to improve maintainability and reusability.
Consider creating a
LoadingStatecomponent:function LoadingState({ location }: { location: string }) { return ( <ChatEvents data={[{ title: `Loading weather information for ${location}...` }]} showLoading={true} /> ); }templates/types/streaming/nextjs/app/components/ui/chat/custom/deep-research-card.tsx (3)
184-206: Extract accordion item into a separate component for better maintainability.Consider extracting the accordion item into a separate component to improve code organization and reusability.
Apply this diff to extract the component:
+function QuestionAccordionItem({ question }: { question: QuestionState }) { + return ( + <AccordionItem + value={question.id} + className="border rounded-lg [&[data-state=open]>div]:rounded-b-none" + > + <AccordionTrigger className="hover:bg-accent hover:no-underline py-3 px-3 gap-2"> + <div className="flex items-center gap-2 w-full"> + <div className="flex-shrink-0"> + {stateIcon[question.state]} + </div> + <span className="font-medium text-left flex-1"> + {question.question} + </span> + </div> + </AccordionTrigger> + {question.answer && ( + <AccordionContent className="border-t px-3 py-3"> + <Markdown content={question.answer} /> + </AccordionContent> + )} + </AccordionItem> + ); +} {state.analyze.questions.length > 0 && ( <Accordion type="single" collapsible className="space-y-2"> - {state.analyze.questions.map((question: QuestionState) => ( - <AccordionItem - key={question.id} - value={question.id} - className="border rounded-lg [&[data-state=open]>div]:rounded-b-none" - > - <AccordionTrigger className="hover:bg-accent hover:no-underline py-3 px-3 gap-2"> - <div className="flex items-center gap-2 w-full"> - <div className="flex-shrink-0"> - {stateIcon[question.state]} - </div> - <span className="font-medium text-left flex-1"> - {question.question} - </span> - </div> - </AccordionTrigger> - {question.answer && ( - <AccordionContent className="border-t px-3 py-3"> - <Markdown content={question.answer} /> - </AccordionContent> - )} - </AccordionItem> - ))} + {state.analyze.questions.map((question: QuestionState) => ( + <QuestionAccordionItem key={question.id} question={question} /> + ))} </Accordion> )}
148-155: Consider memoizing the event filtering separately.The
useMemohook is handling both filtering and state transformation, which could be split for better performance.Consider separating the concerns:
const deepResearchEvents = useMemo(() => getCustomAnnotation<DeepResearchEvent>( message.annotations, (annotation) => annotation?.type === "deep_research_event", ), [message.annotations] ); const state = useMemo(() => deepResearchEvents.length ? deepResearchEventsToState(deepResearchEvents) : null, [deepResearchEvents] );
182-208: Consider extracting question list into a separate component.The question list rendering logic is complex and could benefit from being extracted into a separate component.
Consider creating a
QuestionListcomponent to improve maintainability:function QuestionList({ questions }: { questions: QuestionState[] }) { return ( <Accordion type="single" collapsible className="space-y-2"> {questions.map((question) => ( <QuestionItem key={question.id} question={question} /> ))} </Accordion> ); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
templates/types/streaming/nextjs/app/components/ui/chat/custom/deep-research-card.tsx(2 hunks)templates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx(2 hunks)templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx(1 hunks)templates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`templates/**`: For files under the `templates` folder, do n...
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsxtemplates/types/streaming/nextjs/app/components/ui/chat/custom/deep-research-card.tsxtemplates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsxtemplates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx
⏰ Context from checks skipped due to timeout of 90000ms (42)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
🔇 Additional comments (3)
templates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx (1)
396-452: LGTM! Well-structured schema and efficient component implementation.The code demonstrates good practices:
- Well-defined schema using Zod for type safety
- Efficient annotation processing with
getCustomAnnotation- Performance optimization using
useMemo- Proper null checks and error handling
templates/types/streaming/nextjs/app/components/ui/chat/tools/weather-card.tsx (2)
224-301: LGTM! Well-implemented weather tool component.The code demonstrates good practices:
- Well-defined schema using Zod for type safety
- Efficient annotation processing with
getCustomAnnotation- Performance optimization using
useMemo- Proper loading state handling
174-221: LGTM! Well-structured weather card implementation.The component effectively separates concerns between data and presentation, with clear weather code mappings and comprehensive display of current and forecast weather data.
templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/tools/query-index.tsx
Outdated
Show resolved
Hide resolved
| @@ -11,13 +11,14 @@ | |||
| from app.engine.tools.query_engine import get_query_engine_tool | |||
|
|
|||
|
|
|||
| def get_chat_engine(params=None, event_handlers=None, **kwargs): | |||
| def get_engine(params=None, **kwargs): | |||
There was a problem hiding this comment.
how about we name this create_workflow and we use only one unified chat.py?
There was a problem hiding this comment.
move SourceNodesEvent from multiagents events.py to deep research (then we can remove the file)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
templates/components/multiagent/python/app/api/routers/chat.py(0 hunks)templates/types/streaming/fastapi/app/api/routers/chat.py(2 hunks)templates/types/streaming/fastapi/app/workflows/__init__.py(1 hunks)templates/types/streaming/fastapi/app/workflows/agent.py(3 hunks)
💤 Files with no reviewable changes (1)
- templates/components/multiagent/python/app/api/routers/chat.py
🧰 Additional context used
📓 Path-based instructions (1)
`templates/**`: For files under the `templates` folder, do n...
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
templates/types/streaming/fastapi/app/workflows/agent.pytemplates/types/streaming/fastapi/app/api/routers/chat.pytemplates/types/streaming/fastapi/app/workflows/__init__.py
🪛 Ruff (0.8.2)
templates/types/streaming/fastapi/app/workflows/__init__.py
1-1: .agent.create_workflow imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🪛 GitHub Actions: Lint on push or pull request
templates/types/streaming/fastapi/app/workflows/__init__.py
[error] 1-1: Unused import: '.agent.create_workflow' imported but unused; consider removing, adding to __all__, or using a redundant alias.
⏰ Context from checks skipped due to timeout of 90000ms (42)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
🔇 Additional comments (11)
templates/types/streaming/fastapi/app/workflows/agent.py (5)
4-4: No issues detected on the import.The import statement is valid, as
AgentWorkflowis used later in this file.
13-15: Good usage of default parameters.Using
params = {}whenparamsisNoneavoids mutable default arguments and ensures safe initialization.
20-20: PossibleTypeErrorcheck.
IndexConfig(**params)can raise aTypeErrorifparamscontains invalid keys. Ifparamscan vary widely, consider additional validation or error handling for unexpected arguments.
30-31: Valid constraint for tool availability.Raising a
RuntimeErrorensures that the workflow has at least one tool, preventing misconfiguration.
33-34: Migration toAgentWorkflowappears correct.Using
AgentWorkflow.from_tools_or_functionsproperly integrates the new approach with the existing LLM settings and system prompt.templates/types/streaming/fastapi/app/api/routers/chat.py (6)
5-8: Imports for new callbacks look appropriate.These callbacks (
LlamaCloudFileDownload,SuggestNextQuestions,StreamHandler,AddNodeUrl) are referenced later for streaming and post-processing.
13-13: Consistent workflow import.Importing
create_workflowaligns with the migrated architecture defined inapp.workflows.
28-28: Inclusion of agent messages.Using
include_agent_messages=Trueinget_history_messages()preserves a complete conversation context for the workflow.
34-37: Passingfiltersinto the workflow.Ensures query-level customization in
create_workflow. Confirm thatfiltersis consumed correctly through**kwargsin the workflow.
39-43: Streaming responses from the workflow.Invoking
workflow.run(..., stream=True)is consistent with a streaming-based approach for incremental user feedback.
44-51: Integration withStreamHandler.Returning
vercel_stream()properly composes streaming output with all defined callbacks, enhancing the chat experience.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
templates/components/agents/python/financial_report/app/workflows/events.py (1)
36-46: Add return type annotation to to_response methodThe
to_responsemethod is missing a return type annotation unlike the similar method in theAgentRunEventclass.- def to_response(self): + def to_response(self) -> dict:templates/components/agents/python/financial_report/app/workflows/tools.py (1)
22-25: Consider Adding Method Documentation or Clarifyingacall.The abstract method name
acallis unconventional. If feasible, consider renaming or adding a docstring to clarify how this differs from a standardcall().
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
helpers/python.ts(0 hunks)templates/components/agents/python/financial_report/app/workflows/events.py(1 hunks)templates/components/agents/python/financial_report/app/workflows/tools.py(1 hunks)
💤 Files with no reviewable changes (1)
- helpers/python.ts
🧰 Additional context used
📓 Path-based instructions (1)
`templates/**`: For files under the `templates` folder, do n...
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
templates/components/agents/python/financial_report/app/workflows/events.pytemplates/components/agents/python/financial_report/app/workflows/tools.py
🪛 Ruff (0.8.2)
templates/components/agents/python/financial_report/app/workflows/tools.py
151-151: Function definition does not bind loop variable i
(B023)
⏰ Context from checks skipped due to timeout of 90000ms (42)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
🔇 Additional comments (10)
templates/components/agents/python/financial_report/app/workflows/events.py (5)
1-8: Imports look appropriate for the functionalityThe imports include necessary modules for type annotations, enum support, and components from llama_index. The structure follows standard Python import organization (stdlib first, then third-party, then local).
10-13: Clean enum implementationThe
AgentRunEventTypeenum clearly defines the two types of agent run events with appropriate string values. This approach provides type safety while maintaining readable string values for serialization.
15-20: Well-structured event class with proper type annotationsThe
AgentRunEventclass correctly extends the baseEventclass and includes appropriate type annotations for all attributes. Default values for optional fields are properly provided.
21-31: Response formatting method looks goodThe
to_responsemethod creates a structured dictionary that follows a consistent format for agent events. The nested structure with type and data fields will help with client-side processing.
33-35: Source nodes event class has appropriate structureThe
SourceNodesEventclass properly extends the baseEventclass and includes type annotation for the nodes list.templates/components/agents/python/financial_report/app/workflows/tools.py (5)
1-20: Imports and Setup Look Consistent.The imports and logger setup are well-structured. No issues are apparent, and all imported modules are later referenced. Good job keeping dependencies concise.
28-59: Pydantic Model and Methods Are Well-Structured.Your usage of
BaseModelalong with helper methods (has_tool_calls,tool_name, etc.) looks consistent. Assertions protect against misusage, and thefull_responsecoroutine handles streaming responses properly. Overall, this is clean and maintainable.
61-90: Async Chat Flow Is Correct.The generator logic and the handling of tool calls versus normal chat responses appear correct. The function offers a neat way to distinguish if an immediate tool call is needed while still returning workable streams.
160-198: Sufficient Error Handling.The code correctly treats exceptions by returning error details in a
ChatMessageand logging them. This design is reasonable for gracefully reporting tool errors back to the chat context.
200-231: Generator Logic Is Clear.The
_tool_call_generatorfunction effectively handles and streams both tool call indicators and chat chunks. Good job ensuring that tool calls are distinguished before yielding chat messages.
templates/components/agents/python/financial_report/app/workflows/tools.py
Outdated
Show resolved
Hide resolved
leehuwuj
force-pushed
the
lee/agent-workflows
branch
from
a72abfb to
9fd6d0c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
templates/components/agents/python/deep_research/app/workflows/models.py (1)
48-60: Consider adding a return type annotation toto_response.
It might be helpful to declaredef to_response(self) -> dict:for clearer type-checking and readability.class SourceNodesEvent(Event): nodes: List[NodeWithScore] - def to_response(self): + def to_response(self) -> dict: return { "type": "sources", "data": { "nodes": [ SourceNodes.from_source_node(node).model_dump() for node in self.nodes ] }, }templates/components/agents/python/financial_report/app/workflows/events.py (2)
15-31: Optional: Specify return type into_response.
Currently, it returns a dictionary but lacks an explicit type annotation. This can help with clarity in typed codebases.
33-45: Align with best practice: Add a return type forto_response.
Similar to other classes, it would be helpful to specify the return type.class SourceNodesEvent(Event): nodes: List[NodeWithScore] - def to_response(self): + def to_response(self) -> dict: return { "type": "sources", "data": { "nodes": [ SourceNodes.from_source_node(node).model_dump() for node in self.nodes ] }, }templates/components/agents/python/form_filling/app/workflows/events.py (2)
15-31: Add optional return type.
For better static analysis, consider explicitly returningdictfromto_response.
33-45: Duplicate logic found across modules.
SourceNodesEventis repeated in multiple files. Consider deduplicating in a shared module, if feasible, to reduce maintenance overhead. Also, adding-> dictforto_responseis recommended.templates/components/agents/python/financial_report/app/workflows/tools.py (1)
28-59: Consider graceful handling for a missing generator.
Currently, there is anassert self.generator is not None. Ifgeneratoris unexpectedlyNone, this triggers an assertion error rather than a user-friendly exception.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
helpers/python.ts(0 hunks)templates/components/agents/python/deep_research/app/workflows/deep_research.py(1 hunks)templates/components/agents/python/deep_research/app/workflows/models.py(2 hunks)templates/components/agents/python/financial_report/app/workflows/events.py(1 hunks)templates/components/agents/python/financial_report/app/workflows/tools.py(1 hunks)templates/components/agents/python/form_filling/app/workflows/events.py(1 hunks)templates/components/agents/python/form_filling/app/workflows/tools.py(1 hunks)templates/types/streaming/fastapi/app/api/routers/chat.py(2 hunks)templates/types/streaming/fastapi/app/workflows/__init__.py(1 hunks)
💤 Files with no reviewable changes (1)
- helpers/python.ts
✅ Files skipped from review due to trivial changes (1)
- templates/components/agents/python/deep_research/app/workflows/deep_research.py
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/types/streaming/fastapi/app/workflows/init.py
🧰 Additional context used
📓 Path-based instructions (1)
`templates/**`: For files under the `templates` folder, do n...
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
templates/components/agents/python/deep_research/app/workflows/models.pytemplates/components/agents/python/financial_report/app/workflows/events.pytemplates/components/agents/python/form_filling/app/workflows/events.pytemplates/types/streaming/fastapi/app/api/routers/chat.pytemplates/components/agents/python/financial_report/app/workflows/tools.pytemplates/components/agents/python/form_filling/app/workflows/tools.py
🪛 Ruff (0.8.2)
templates/components/agents/python/financial_report/app/workflows/tools.py
151-151: Function definition does not bind loop variable i
(B023)
templates/components/agents/python/form_filling/app/workflows/tools.py
151-151: Function definition does not bind loop variable i
(B023)
⏰ Context from checks skipped due to timeout of 90000ms (42)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, express, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, express, --no-files)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, express, --no-files)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, express, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (18, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
🔇 Additional comments (18)
templates/types/streaming/fastapi/app/api/routers/chat.py (8)
5-8: Appropriate callback implementation for the new workflow architecture.The new imports introduce a modular approach with separated concerns through different callback classes, which aligns well with the Agent Workflow migration.
13-13: Correctly migrated from engine to workflow pattern.The import change from engine-related imports to
create_workflowappropriately reflects the architectural migration described in the PR.
28-28: Enhanced message retrieval with agent messages.Including agent messages in the history provides a more complete context for the workflow, improving conversation coherence and agent performance.
34-37: Clean workflow creation implementation.The workflow creation is concise and properly configured with the necessary parameters and filters.
39-43: Workflow run implementation follows best practices.The workflow run method properly uses the user message and chat history with appropriate streaming configuration.
44-51: Well-structured stream handler with callbacks.The StreamHandler implementation with separate callback modules follows good separation of concerns and will make the code more maintainable.
60-90: Consider removing the non-streaming endpoint.Based on the previous comments from team members, this non-streaming endpoint may no longer be needed and was only kept for reference in README.md.
Do you still need this endpoint? Previous comments from @leehuwuj indicated: "I think this endpoint is no longer usage (we just have a note for using it in the README.md file)." And @marcusschiesser asked if the multiagent folder could be removed as a result.
53-57: Error logging and handling improved.The error handling has been updated with more specific error messages that reflect the new context of chat operations.
templates/components/agents/python/deep_research/app/workflows/models.py (1)
7-7: No issues with the new import.
Well-structured import forSourceNodes.templates/components/agents/python/financial_report/app/workflows/events.py (1)
10-13: LGTM for enum definitions.
TheAgentRunEventTypeenum is straightforward and clear.templates/components/agents/python/form_filling/app/workflows/events.py (1)
10-13: Well-defined enum.
Enum values are concise and descriptive.templates/components/agents/python/financial_report/app/workflows/tools.py (5)
22-26: Abstract class and method naming look good.
TheContextAwareToolapproach is clear, ensuring all derived tools implementacall.
61-90: Overall logic is clear.
This asynchronous method properly differentiates between a tool-call and a final streaming response.
92-158: Prevent KeyError in single tool call code path.
Directly indexingtools_by_name[tool_calls[0].tool_name]may raise a KeyError if the tool doesn't exist. Consistently using the.get()approach as in the multi-call path can prevent unexpected crashes.if len(tool_calls) == 1: - return [ - await call_tool( - ctx, - tools_by_name[tool_calls[0].tool_name], - tool_calls[0], - ... - ) - ] + tool = tools_by_name.get(tool_calls[0].tool_name) + if not tool: + return [ + ChatMessage( + role=MessageRole.ASSISTANT, + content=f"Tool {tool_calls[0].tool_name} does not exist", + ) + ] + return [ + await call_tool( + ctx, + tool, + tool_calls[0], + ... + ) + ]🧰 Tools
🪛 Ruff (0.8.2)
151-151: Function definition does not bind loop variable
i(B023)
160-198: Appropriate use of context for tools.
Injecting context forContextAwareToolis a good design choice; error handling is properly handled in theexceptblock.
200-231: Descriptive asynchronous generator.
The_tool_call_generatoryields clear signals regarding tool calls vs. streamed text.templates/components/agents/python/form_filling/app/workflows/tools.py (2)
171-177: Confirm tool argument consistency for context-aware tools.
When callingacallon aContextAwareTool, the code injects actxparameter. Verify that all tools implementingContextAwareToolaccept the same signature (acall(ctx, ...)) and handlectxappropriately. Otherwise, this could cause unexpected runtime failures.
200-231: Validate final chunk usage in_tool_call_generator.
The generator logic yields a boolean indicator before streaming chunks, then yields a final chunk after the loop. Ensure that downstream consumers properly handle this final chunk and consider adding checks forNoneto prevent potential attribute errors if the LLM yieldsNone.
templates/components/agents/python/form_filling/app/workflows/events.py
Outdated
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Refactor
Chores