Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

AI: Check for Duplicate Specs #608

Merged
merged 46 commits into from
Aug 27, 2023

Conversation

kamaalsultan
Copy link
Contributor

@netlify
Copy link

netlify bot commented Aug 11, 2023

Deploy Preview for ubiquibot-staging ready!

Name Link
🔨 Latest commit 89e6705
🔍 Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/64eb4da23246d20008fdf8ac
😎 Deploy Preview https://deploy-preview-608--ubiquibot-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

.env.example Show resolved Hide resolved
src/handlers/issue/pre.ts Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
.env.example Outdated Show resolved Hide resolved
src/helpers/similarity.ts Outdated Show resolved Hide resolved
@0x4007
Copy link
Member

0x4007 commented Aug 12, 2023

Great reviews, team!

@0x4007
Copy link
Member

0x4007 commented Aug 12, 2023

@ByteBallet please be sure to "Resolve conversation" for each after you implement requested changes, and have all of your questions answered.

@kamaalsultan
Copy link
Contributor Author

@ByteBallet please be sure to "Resolve conversation" for each after you implement requested changes, and have all of your questions answered.

Yes, sure.

src/handlers/issue/pre.ts Outdated Show resolved Hide resolved
@0x4007
Copy link
Member

0x4007 commented Aug 16, 2023

I was thinking about the rate limit problem. What if we "preserve state" by passing in the progress to the database. For example:

{
	checking: { url: "ubiquity/ubiquibot/520", startTime: "1234123412" },
	queue: [
		{ pending: false, similarity: 0.7, issueUrl: "ubiquity/ubiquibot/510" },
		{ pending: true, issueUrl: "ubiquity/ubiquibot/511" },
		{ pending: true, issueUrl: "ubiquity/ubiquibot/512" },
	],
}

If we hit rate limits, this job can continue the next time an issue is posted?

I want to experiment with the current implementation to see how obtrusive the rate limits are. If they are a problem I would be more motivated to include these capabilities to deal with rate limits.

(Sorry for the sloppy example I wrote this on my phone.)

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Aug 16, 2023

this job can continue the next time an issue is posted?

But the problem is we can also hit rate limit while checking the next issue. If this process is repeated, I think we will have a lot of delay.

@0x4007
Copy link
Member

0x4007 commented Aug 16, 2023

But the problem is we can also hit rate limit while checking the next issue. If this process is repeated, I think we will have a lot of delay.

In the original specification I explicitly stated that slow is fine. Even if it takes a day its still immensely helpful for repositories with hundreds of open issues and community contributors opening new ones.

that passes the issue specification to ChatGPT (asynchronously/slowly is fine)

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Aug 16, 2023

In the original specification I explicitly stated that slow is fine. Even if it takes a day its still immensely helpful for repositories with hundreds of open issues and community contributors opening new ones.

https://github.com/ayaka14732/ChatGPTAPIFree
I was looking for a new method and found this project on github.
If you dislike it, I am considering this way.
image
This is the standard mechanism to avoid rate limit in OpenAI.
https://platform.openai.com/docs/guides/rate-limits/error-mitigation
@pavlovcik rfc

@0x4007
Copy link
Member

0x4007 commented Aug 16, 2023

Exponential backoff is pretty standard. We can use it.

@kamaalsultan
Copy link
Contributor Author

@web4er
Did you test with the latest changes?
As I modified a bit on prompt for measuring similarity.
6f86e24
It might be working fine on local but it will cause error on netlify as it limits time of serverless function.

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Aug 22, 2023

ubiquibot/staging#141
QA for deployed branch on Netlify.
It worked but I think increasing time limit will be necessary further...
@pavlovcik @web4er

@kamaalsultan
Copy link
Contributor Author

I am not sure if it is necessary to increase time limit for more than 10 seconds as it seems working fine...

@web4er
Copy link
Contributor

web4er commented Aug 22, 2023

Did you test with the latest changes?

I have tested it at 93818b4 that was latest when I was doing it.

I think the point of concern in the QA was that you are always expecting a number from AI, and if it responds with some text, you are parsing any number from the answer as a similarity.

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Aug 22, 2023

I have tested it at 93818b4 that was latest when I was doing it.

I think the point of concern in the QA was that you are always expecting a number from AI, and if it responds with some text, you are parsing any number from the answer as a similarity.

Since we don't want 100% correctness from AI, I think it's reasonable to do like that. It's just the problem that we concern more on number of % or bot's answering rate. And in my case, I didn't experience the bot answering with number that is irrelevant to the similarity. I made some changes (revert string literal) after 93818b4 and you will also see no problem there. ^^

@0x4007
Copy link
Member

0x4007 commented Aug 22, 2023

@web4er

Did you test with the latest changes?

As I modified a bit on prompt for measuring similarity.

6f86e24

It might be working fine on local but it will cause error on netlify as it limits time of serverless function.

You shouldn't have undone the string literal. Also consider the prompt I wrote because I got really good results for the keywords in my limited testing: #488 (comment)

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Aug 22, 2023

I couldn't use string literal as I used the prompt from .env file. First, I used exactly same prompt from yours but gpt did not give me satisfactory result. It always gave me words from the prompt, not content. (e.g. It always gave me "important" as important word)

@kamaalsultan
Copy link
Contributor Author

Also consider the prompt I wrote because I got really good results for the keywords in my limited testing: #488 (comment)

If we want, we can just change prompts from env variable.

@kamaalsultan
Copy link
Contributor Author

What do you think about it? @pavlovcik

@Draeieg
Copy link
Contributor

Draeieg commented Aug 25, 2023

as I understand it currently we have 2 options for QA on previews

  1. get rugged by temporarily pay for the premium servers
  2. wait until Similar Issue Detection: Minimize Runtime #652

or 3rd option do local testing like @web4er and trust me bro it will run fine once we figure a solution

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Aug 25, 2023

Or just try again on https://github.com/ubiquibot/staging/
QA for ubiquibot/staging#157

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Aug 25, 2023

@0x4007
Copy link
Member

0x4007 commented Aug 25, 2023

  1. get rugged by temporarily pay for the premium servers

I gave you card info to set it up its fine for now.

@kamaalsultan
Copy link
Contributor Author

I gave you card info to set it up its fine for now.

I guess it is not needed for QA.

src/handlers/processors.ts Outdated Show resolved Hide resolved
src/handlers/processors.ts Outdated Show resolved Hide resolved
@0xcodercrane 0xcodercrane merged commit 5ef5f44 into ubiquity:development Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

AI: Check for Duplicate Specs
6 participants