-
Notifications
You must be signed in to change notification settings - Fork 164
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
Adding a Documentation Generation Util #1404
base: dev
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
a6ba4cd
to
2dd7ba8
Compare
2dd7ba8
to
eefe927
Compare
99442f6
to
99aaf3b
Compare
@aarondr77 the tests are failing, but I haven't added any tests yet. |
99aaf3b
to
8ee2b16
Compare
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.
Awesome work on this. I love a lot of the utilities that you built + how you were thinking about the user flow.
A bit of cleanup that I think we should do and then we can get this merged!
mito-ai/src/utils/notebook.tsx
Outdated
const fetchPromise = requestAPI('mito_ai/completion', { | ||
method: 'POST', | ||
body: JSON.stringify({ | ||
messages: [{role: 'user', content: "Write markdown documentation for the following code:\n" + combinedCode}] |
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.
It would be awesome to think a bit more about what documentation would be useful to generate here. Right now, the code that we're generating is super verbose. It is explaining the details of every single line of code in the selected cells (at least in my usage), but I don't think that really makes for good documentation.
I think we can generate a better prompt that will lead to more useful results. You can see an example of a the other prompts we use in src/prompts/BasicPrompt.tsx
.
Want to try refining this prompt to get better results?
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.
@aarondr77 I agree, it would be better to refine this before we get this merged. I will work on it.
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.
@aarondr77 I think I am late in addressing this review. Because the BasicPrompt.tsx
has been moved or renamed, Either way I improved my prompt a bit.
mito-ai/src/utils/notebook.tsx
Outdated
import { requestAPI } from './handler'; | ||
import { ServerError, TimeoutError, ConnectionError, UnknownError, OpenAIError } from './errors'; | ||
|
||
const LOADING_MARKDOWN = '> *`⏳ Generating documentation... please wait`*'; |
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.
Maybe we should just extend this to say that it might take up 30 seconds since it can be pretty slow and it might seem broken to users.
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.
You have a point there, but if we say 30 seconds, that is a guarantee, can we give that guarantee?
Because this guarantee would depend on what underneath service guarantees us. Any idea about it?
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 know my code has the time out, but that was actually a naive timeout I added without any reasoning.
writeToCell(newCell.model, aiMessage); | ||
NotebookActions.renderAllMarkdown(notebook.content); | ||
} else { | ||
console.error("New cell not found"); |
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.
Maybe we should make this function return the new cell id if it was created and return undefined it it was not. Then we don't need to go search for the loading cell right after creating on lines 189-190
@aarondr77 what is the best practice to deal with the yarn.lock file? Do we commit it? |
8ee2b16
to
eacd306
Compare
56f13f4
to
8107552
Compare
Description
This function generates the code documentation in markdown format.
Testing
TODO
Documentation
TODO