-
Notifications
You must be signed in to change notification settings - Fork 130
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
feat(api): add langchain agent approach (#5) #29
Conversation
@@ -1,3 +1,4 @@ | |||
export * from './approach.js'; | |||
export * from './ask-retrieve-then-read.js'; | |||
export * from './ask-read-retrieve-read.js'; | |||
export * from './chat-read-retrieve-read.js'; |
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.
certainly confusing file names....Is this a convention?
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.
Yeah it's not the best, it's basically "<chat_or_ask_api>-<approach_name>" but confusing for the "ask" ones.
Initially I wanted to put them in their own folders like chat/read-retrieve-read.ts
and ask/read-retrieve-read.ts
, but then it would be the class names that would be confusing 😅
If you have a better idea... (naming is even worse on the Python sample BTW)
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.
Wouldn't then (if we do / maybe ApiName be a super class to be extended?
like class Chat {}
class ReadRetrieve extends Chat {
constructor () {
super();
}
}
Or they share no commonalities at all?
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.
They already implement either the ChatApproach
or AskApproach
interface (in approach.ts
) and all derive from BaseApproach
class.
But for example it would result in having two ReadRetrieveRead
class to be imported in the plugins/approaches.ts
file (so we'll have to discriminate anyways), and having identical class name is not good for searchability of the code :/
I'll try to think of something!
Note: to be rebased or merged for keeping individual commits as steps (some are useful for feedbacks)