Skip to content
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: Add AI interaction for text rewrite #66

Merged
merged 11 commits into from
Jun 8, 2023
Merged

Conversation

gjreda
Copy link
Collaborator

@gjreda gjreda commented Jun 1, 2023

Adds initial "rewrite" capability to the sidecar. For a first pass, we're only focusing on the ability to rewrite a highlighted block of text in a more concise manner.

After trying some of the model options mentioned in #57, I decided to move forward with OpenAI for now. Ultimately, it felt like there's A LOT of noise around the other various model alternatives and I was starting to feel like I had fallen down a deep dark rabbit hole. When a clear winner emerges for us as a better option, I don't think swapping over to it will be too bad.

Another note is that I've opted to use GPT-3.5 rather than davinci. I felt that GPT-3.5 was producing better, more human-like responses. It also costs 1/10th of the price for now.

Example usage

❯ poetry run python python/main.py rewrite --text "In the cycling world, power meters are the typical way to objectively measure performance. Your speed is dependent on a lot of factors, like wind, road surface, and elevation. One's heart rate is largely a function of genetics, but also things like temperature. The amount of power you are outputting to the pedals though, is a direct measure of how much work you're doing, regardless of wind, elevation, your body weight, etc."

[{"index": 0, "text": "Power meters are the standard method for objectively measuring cycling performance, as they directly measure the amount of work being done regardless of external factors such as wind, elevation, and body weight."}]

Note that I'm passing index as a key rather than id to maintain consistency with how the OpenAI response refers to these fields. I think we might want to reserve what they call ID for later, when we eventually get to chat interactions.

@gjreda gjreda changed the title feat: Add AI interaction for text rewrite (#60) feat: Add AI interaction for text rewrite Jun 1, 2023
@hammer
Copy link
Contributor

hammer commented Jun 2, 2023

One thought I had on this PR that I forgot to mention in our call: we should probably add a configuration UX to allow users to specify their OPENAI_API_KEY, and possibly to configure other aspects of the app. cc @cguedes

@gjreda
Copy link
Collaborator Author

gjreda commented Jun 2, 2023

Ran a couple of examples using the ML scholarly papers we've been testing against and results still seem pretty good.

Moving this into "ready for review" as I think it makes sense to merge in the initial backend work as one PR and then add in the front-end interaction once we have the UI ready. cc @cguedes @sehyod Merging this in shouldn't cause any issues since it's not called anywhere.

❯ poetry run python python/main.py rewrite --text "A common misconception about overfitting is that it is caused by noise, like training examples labeled with the wrong class. This can indeed aggravate overfitting, by making the learner draw a capricious frontier to keep those examples on what it thinks is the right side. But severe overfitting can occur even in the absence of noise. For instance, suppose we learn a Boolean classifier that is just the disjunction of the examples labeled “true” in the training set. (In other words, the classi- fier is a Boolean formula in disjunctive normal form, where each term is the conjunction of the feature values of one specific training example). This classifier gets all the train- ing examples right and every positive test example wrong, regardless of whether the training data is noisy or not." | jq
[
  {
    "index": 0,
    "text": "Overfitting is not solely caused by noise, as it can occur even without it. For example, a Boolean classifier that is just the disjunction of the examples labeled \"true\" in the training set can lead to severe overfitting, getting all training examples right but every positive test example wrong."
  }
]

@gjreda gjreda requested review from hammer, cguedes and sehyod June 2, 2023 03:22
@gjreda gjreda marked this pull request as ready for review June 2, 2023 03:23
@cguedes cguedes mentioned this pull request Jun 2, 2023
@cguedes
Copy link
Collaborator

cguedes commented Jun 2, 2023

One thought I had on this PR that I forgot to mention in our call: we should probably add a configuration UX to allow users to specify their OPENAI_API_KEY, and possibly to configure other aspects of the app. cc @cguedes

Created issue for that new feature: #76

cguedes
cguedes previously approved these changes Jun 2, 2023
@cguedes cguedes linked an issue Jun 2, 2023 that may be closed by this pull request
cguedes
cguedes previously approved these changes Jun 5, 2023
Copy link
Collaborator

@cguedes cguedes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: I've only checked the code. I didn't had an API key to test.

@gjreda gjreda requested a review from cguedes June 5, 2023 18:45
cguedes
cguedes previously approved these changes Jun 6, 2023
@cguedes
Copy link
Collaborator

cguedes commented Jun 7, 2023

Adjusted the "AI View" to adopt the new sidecar API for rewrite.

image

@cguedes cguedes self-requested a review June 7, 2023 12:55
cguedes
cguedes previously approved these changes Jun 7, 2023
python/sidecar/interact.py Outdated Show resolved Hide resolved
src/panels/AIPanel.tsx Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@hammer
Copy link
Contributor

hammer commented Jun 7, 2023

I have put my OpenAI API key into the environment but I'm still not seeing output when running this PR. It says "Processing..." when I hit "REWRITE" but no text is returned.

How can I get more diagnostics to figure out what's going on?

@gjreda gjreda requested review from hammer, cguedes and sehyod June 7, 2023 17:13
@hammer
Copy link
Contributor

hammer commented Jun 7, 2023

I was able to get this working on a fresh checkout. The AI is alive!!!

@hammer
Copy link
Contributor

hammer commented Jun 8, 2023

What’s blocking this from going in?

@sehyod
Copy link
Collaborator

sehyod commented Jun 8, 2023

The missing piece here is actually rewriting the selected text with what the AI returns. I think @cguedes was working on it but he is off today. I think we could merge this as is and add the actual rewrite action in a follow up PR

@hammer
Copy link
Contributor

hammer commented Jun 8, 2023

Yeah I think there are UX issues to work through on how to take the text returned by the model and incorporate it into the document. Let's just get this in and open up a follow-on issue for what to do with the model-generated text.

@gjreda
Copy link
Collaborator Author

gjreda commented Jun 8, 2023

Only thing blocking from my perspective is an approving review. I can't merge without it.

src/panels/AIPanel.tsx Outdated Show resolved Hide resolved
@gjreda gjreda merged commit 7fbdf7e into main Jun 8, 2023
8 checks passed
@gjreda gjreda deleted the 60-ai-interactions-rewrite branch June 8, 2023 16:58
sehyod pushed a commit that referenced this pull request Jun 9, 2023
* feat: Add AI interaction for text rewrite (#60)

* linter fix

* Fix unit test reading OPENAI_API_KEY with .get

* Adjust AI View to use rewrite sidecar command

* Use `self.n_options` instead of hard-coded 1

* Create useCallablePromise and improve code

* remove unused dependencies

---------

Co-authored-by: Carlos Guedes <cguedes@gmail.com>
Co-authored-by: Carlos Guedes <cguedes@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AI interactions: rewrite
4 participants