search

ubiquity / ubiquibot

Putting the 'A' in 'DAO'
https://github.com/marketplace/ubiquibot
MIT License
17 stars 60 forks source link

/review #748

Closed Keyrxng closed 10 months ago

Keyrxng commented 1 year ago

Resolves #746

invoked via /review

The implemented prompts differ from the original spec but after experimenting with variations, I believe the current setup provides uniform formatting and covers all requirements of the spec.

netlify[bot] commented 1 year ago

Deploy Preview for ubiquibot-staging ready!

Name Link
Latest commit f1400cc00b500ac260d6c53c6fb60c6c858e52d7
Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/650bf1bdb191150008e68d8d
Deploy Preview https://deploy-preview-748--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.

Keyrxng commented 1 year ago

Interesting prompts. Seems like a lot of experimentation was put into them. Understanding purely via code review is tough for that but I would love to see QA examples to see those prompts in action.

I've deleted the majority from https://github.com/Keyrxng/UbiquityBotTesting/pull/41#issuecomment-1712961314 especially the really terrible responses but you can still sort of the see the progression.

If this is mandatory I'll spam my issue with various versions and I'll comment the prompt scheme I used for it below, just lmk

0x4007 commented 1 year ago

I just need to see the latest when it is ready for review. Posting all and highlighting favorites should be fine

Keyrxng commented 1 year ago

I just need to see the latest when it is ready for review. Posting all and highlighting favorites should be fine

https://github.com/Keyrxng/UbiquityBotTesting/pull/41#issuecomment-1713538151

Added some examples using the spec prompt and showing you what the context call response object looks like

0x4007 commented 1 year ago

Here are the actionable changes made in the code:

This is meant to list what changes must be done from where the author currently is at to achieve the specification (if they have not already)

It looks like it is just describing what work was done in the pull request.

The results of your prompt seems to have a lot of noise and not a lot of signal. If I'm understanding your results correctly, I think that my prompt returns a more clear answer.

Keyrxng commented 1 year ago

Rework https://github.com/Keyrxng/UbiquityBotTesting/pull/41#issuecomment-1715300983

How's this looking mate?

0x4007 commented 12 months ago

It could be interesting to plug this into CI on push so that it won't pass until the bot thinks it passed. Until we implement automatically closing pull requests due to poor performance though, we should not automatically invoke the /review.

0xcodercrane commented 12 months ago

It could be interesting to plug this into CI on push so that it won't pass until the bot thinks it passed. Until we implement automatically closing pull requests due to poor performance though, we should not automatically invoke the /review.

good point. we can create an ai-review ci/cd running on each pull request.

0x4007 commented 11 months ago

I'm really looking forward for this one to get merged in because I think it will be able to help us merge the other pull requests faster. @Keyrxng any updates?

By the way, https://github.com/Keyrxng/UbiquityBotTesting/pull/41#issuecomment-1719534993 is my favorite review result.

Keyrxng commented 11 months ago

any updates?

I felt that I was waiting for you guys but I will stop work on Blame and just ensure everything is operating inline with the favourite you mentioned which I'm sure it is. So long as it is my work with this for now is done unless there are further requests. Will let you know in a couple hours.

Keyrxng commented 11 months ago

So the response is short and sweet if it's meeting the spec while when it's failing it's going into depth with things and suggesting how to meet the spec or what is missing from the current implementation.

Done my best to remove any type of overview or listings, I think that ratio you mentioned is better in the more recent stuff than the one you said was your favourite so far.

Keyrxng commented 11 months ago

What would be so ideal is if it could be tested on closed prs or something as that amount of real data would be so much better to work with than my shitty gpt-assisted test issues 🤣 Hope it's looking better tho, lmk and I'll wrap this up tomorrow.

0x4007 commented 11 months ago

Does it make sense to eventually turn all the AI functions into -background functions?

https://docs.netlify.com/functions/background-functions/

@0xcodercrane rfc

0xcodercrane commented 11 months ago

Does it make sense to eventually turn all the AI functions into -background functions?

https://docs.netlify.com/functions/background-functions/

@0xcodercrane rfc

The background function is a new feature of netlify and I think it's worth to take the 15 mins execution timeout benefit of that feature. but my concern from their docs is the pricing plan. they just say it may not be available on all the pricing plans.

Keyrxng commented 11 months ago

Okay I'm back in the game

All good to pull things from this pr and start with a clean commit history from the most recent dev branch, seems cleaner and I can avoid dealing with all of the conflicts?