-
Notifications
You must be signed in to change notification settings - Fork 20
feat: Accept agenticConfiguration prop to accommodate agentic behaviors. #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
ab0d5e6 to
e3e3a48
Compare
db9679b to
d0be897
Compare
d0be897 to
d0a2bb6
Compare
MohammedMaaz
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.
Overall looks good, just some minor comments from my side.
| const { options } = messageHistoryItem; | ||
|
|
||
| return ( | ||
| <Fragment key={index}> |
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.
Any reason for using Fragment here, since there's only 1 child?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall, TBH. Could have been just to cleanly wrap it and have the key there instead of mixed in with the div that has a bunch of classes.
| // @ts-ignore | ||
| (ref.current as ReactChatbotWebComponent).setAgenticConfiguration(props.agenticConfiguration); | ||
| } | ||
| }, [props]); |
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.
Can we be more precise here and only include desired props?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure we can. But I'd like to keep the pattern we have when working with WebComponents for now so as not to add complexity to this PR.
| (globalThis as any).fetch ||= jest.fn(); | ||
| const global = globalThis as typeof globalThis & { fetch: any }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the following might be a more simpler equivalent of this, while also avoiding the use of any. Correct me if I'm wrong though.
| (globalThis as any).fetch ||= jest.fn(); | |
| const global = globalThis as typeof globalThis & { fetch: any }; | |
| const global = { ...globalThis, fetch: globalThis.fetch ?? jest.fn() } |
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.
So unfortunately, we'll need to keep the original way of writing it.
In the context of jest, globalThis is essentially the global namespace (sort of like window in the browser context). As such, on line 148 we're mocking fetch in that global namespace.
With the change you've suggested, we're defining a new object called global and that's where we are providing the mock of fetch. So the problem there is that the global namespace doesn't get the fetch mock- only this new object which we've defined called global.
Furthermore, the only reason I originally defined global on line 149 was to be able to avoid casting it to that type below- it looked really messy to have to type:
const mockFetch = jest.spyOn(globalThis as typeof globalThis & { fetch: any }, "fetch").mockResolvedValue({
status: 200,
json: jest.fn().mockResolvedValue({ event: "mock-event" })
});
53c78cd to
c31ad26
Compare
This PR enables adding an agentic configuration to react-chatbot. Requests to agentic web services occur on every message by the user, and behavior specified in the configuration allows the chatbot to react to the web service response in ways desired by the developer.
The Agentic Web Service
The agentic web service should send back a response in the form of:
The chatbot can then parse at least the
eventproperty to determine follow-up behavior.The Chatbot Response Handler
Based on the web service response, the chatbot can take action in 2 ways, all specified in a callback handler:
User Actions
If defined in the agentic callback handler, the chatbot may then make actions available for the user to select. These come in 3 forms, and may be a combination of all of them:
Configuration
This is all done by adding an
agenticConfigurationto either the component oruseChathook: