kodadot / nft-gallery

Generative Art Marketplace
https://koda.art
MIT License
643 stars 362 forks source link

Assign bot #2448

Closed vikiival closed 2 years ago

vikiival commented 2 years ago

Would be nice that if someone comment: I will take it it would assign it to the person for 48 hrs

petersopko commented 2 years ago

After checking couple of new PRs this seems to be more relevant than ever. I'm gonna add some specs and try to come up with solution soon, if you guys agree:

  1. Have working bot which assigns task on "/assign" in comments. Posts message with (example) "ASSIGNED - @kodadotuserxyz has been assigned with this issue. Task is locked 🔒 until DD-MM-YYYY HH-MM-SS". If the period expires and there's no linked closing PR open by that time -> task gets unassigned with message "OPEN ISSUE - issue wasn't resolved in locked period and is open to re-assignment!"

  2. Triggering bot again by different user BEFORE the locked period is over will result in "ERROR - task is already assigned to @kodadotuserxyz. Wait until DD-MM-YYYY HH-MM-SS before re-assigning!"

  3. Triggering bot again by different user AFTER the locked period is over but WHILE there's already open PR which is linked and closing said issue will result in "ERROR - task is already being resolved in PR #9999 by @kodadotuserxyz!"

  4. Bot comments on PR that's linked AND it's supposed to be closing assigned issue:

    a) If the PR author matches assignee:

    "SUCCESS - PR #9999 is closing issue #9998 which was assigned to @kodadotxyz. Please wait for review or grab another issue in a meantime! https://github.com/kodadot/nft-gallery/issues/"

    b) If the author just goes rogue and ignores all this:

    "WARNING - This PR is closing issue #9999 which is not assigned to you! Get the issue assigned first, wait until locked period is over or grab another issue https://github.com/kodadot/nft-gallery/issues/ Read more about assigning policy at CONTRIBUTING.MD (perhaps @cryptodamsky could help here? )

    c) If the author gets issue assigned after creating PR (for example newcomers who just read WARNING from b)) -> bot comments with SUCCESS message from a)

  5. Some limit on assigned issues at given time? Perhaps three would be the max. limit (unless PR is already open, that wouldn't be counted) after which bot can comment with "MAX ASSIGNED ISSUES LIMIT REACHED. Work on closing assgined issues before getting another issue assigned!"

Pretty sure I've missed some cases but since I'm gonna be looking into GitHub bots regarding #1477 anyways, might aswell work on this one too. Perhaps work on this one first since it's, wild west out there in PRs :laughing: (sorry it keeps happening to you @prachi00)

This was sort of discussed but not fleshed out at the last all-hands-dev call, some feedback / thoughts would be appreciated.

prachi00 commented 2 years ago

@petersopko makes sense, this would be so amazing and helpful when done 😍 yeah happened to me again today in #2498 😔

yangwao commented 2 years ago

Would be nice that if someone comment: I will take it it would assign it to the person for 48 hrs

72hrs is way too much, on small issues 24-48hours should be bearable. We saw that even some complex issues done under 2 days or sooner.

"/assign"

Can we have something human centric?

All the cases would be probably easier understandable in some diagrams as in text is hard to catch some deadlock cases 👀

If you want to start cracking, let's try with https://autocode.com/ it's super easy to make first prototype :)

Oh try to look on gitcoin issues, they have something already cases handled, yet some of them comes to me as overkill.

vikiival commented 2 years ago

Can we have something human centric?

I take this I'll take this I will take this

small issues 24-48hours should be bearable

If we can parse the $ (dollar) labels I would set 24 hr lock for single $ issue

prachi00 commented 2 years ago

Can we have something human centric?

I take this I'll take this I will take this

small issues 24-48hours should be bearable

If we can parse the $ (dollar) labels I would set 24 hr lock for single $ issue

I feel like 24 hours is too less though for one issue sometimes, so maybe we can also add support for increasing time with another comment?

petersopko commented 2 years ago

72hrs is way too much, on small issues 24-48hours should be bearable. We saw that even some complex issues done under 2 days or sooner.

If we can parse the $ (dollar) labels I would set 24 hr lock for single $ issue

I feel like 24 hours is too less though for one issue sometimes, so maybe we can also add support for increasing time with another comment?

OK, if you guys agree, I'll make the first version with $ for 24hrs, $$ for 48hrs, $$$ for 72 and $$$$ and more for 120hrs, i.e. five days. We can add support if you want, or perhaps, since opening PR related to task could lock it until PR is resolved. As @yangwao mentioned before, PRs should be resolved within short period anyways, therefore this would add the requested 24-48hrs and it's also proving you're working on the issue.

a) add support function for additional 24hrs b) force faster PRs which lock the issue

Can do either of those.

Can we have something human centric?

I take this I'll take this I will take this

OK, these three phrases for start.

All the cases would be probably easier understandable in some diagrams as in text is hard to catch some deadlock cases 👀

Will try to recaputre the specs with something eye-friendly tomorrow.

If you want to start cracking, let's try with https://autocode.com/ it's super easy to make first prototype :)

ah, just read the discord reply, will use autocode with js then.

petersopko commented 2 years ago

~Okay, I've trained and tested the bot yesterday on my repo, needs some refinement with PRs today but it's more or less ready, now just ironing out the logic and messages with you :) Also, I need to write up some readme.md since Autocode does awesome job but the initial deployment seems to stay manual (can't do it from GitHub repo afaik).~

~Here's the promised diagram of bot logic:~

Got it working with kv storage. Tried to use comments as storage for saving states but well...that was a stupid way to do it. Here's the updated diagram:

My First Board (6)

some of the variables/terms in diagram and code:

ignoredUsers - bots and internal team, nothing happens if it's empty anyway, this just stops bot from executing right away

goPhrases = goPhrases: [ 'I take this', 'I take this.', "I'll take this", "I'll take this.", 'I will take this', 'I will take this.', 'i take this', 'i take this.', 'ill take this', 'ill take this.', ] bountyTimes = { $: 24, $$: 48, $$$: 72, $$$$: 96, $$$$$: 120, }

messages:

petersopko commented 2 years ago

@yangwao just saw your discord message related force push, automatized check and possible warning in comments under PR can be added as well.

petersopko commented 2 years ago

Got it working with kv storage. Tried to use comments as storage for saving states but well...that was a stupid way to do it.

Updated my previous comment, it works now. I didn't use Cloudflare KV Workers but built-in Autocode kv util. Hope that's okay. https://docs.autocode.com/building-endpoints/keyvalue-storage/

vikiival commented 2 years ago

built-in Autocode kv util

petersopko commented 2 years ago

I have it working with KV workers after all, since I'm connecting it to #1477, there was a possibility of hitting upper limit with autocode's storage. @yangwao please check out the diagram when you're available, and let me know what you think. I'm working on connection to #1477 right now.

vikiival commented 2 years ago

I'm working on connection to https://github.com/kodadot/nft-gallery/issues/1477 right now.

I would personally leave it for another issue // -> focus to make first working version of the assign bot

petersopko commented 2 years ago

I would personally leave it for another issue // -> focus to make first working version of the assign bot

I've published version which is only affected by #1477 in a small way, basically it's just storing data objects needed for generating LEADERBOARD.md in the future. It's triggered by payout message, otherwise it does nothing. But I can remove it in case you want this fully separated, it just uses the same webhook for created comment on issue.

I was waiting for some feedback on the diagram, however, in case you want to test this in the meantime, I have it working on https://github.com/petersopko/kodabot-test/issues where you can get some issues assigned (for shortened period of time, due to testing), PRs opened etc., and in case you want to install it and have it working for yourself at this stage, it's available here https://autocode.com/app/petersopko/kodabot/

yangwao commented 2 years ago

image

I'll maybe adjust the formatting bit to be way easier to find, so contributor doesn't need it to read whole to find some key information

image

Suggestions

Some debug

https://github.com/petersopko/kodabot-test/issues/9 image

My few coin-cents

petersopko commented 2 years ago

image

I'll maybe adjust the formatting bit to be way easier to find, so contributor doesn't need it to read whole to find some key information

ok

  • I'd remove timezone and keep just UTC for anyone out there to make easier calculations

ok

  • [x] Is there sizeable windows for each bounty and hours? To reflect higher bounty might require sizeable time frame. Found it seems for $$ - 48, $$$ - 72hrs :)

bountyTimes = { $: 24, $$: 48, $$$: 72, $$$$: 96, $$$$$: 120}

  • what happens after the time period expires?

~after time period expires, nothing happens unless somebody else wants to take the issue. basically, if you won't finish in time and there's nobody else who'd like to try, feel free to go on?~ just found out that Autocode has a built in scheduler, so I'll set it to check once every minute if all the assigend issues are within limits.

  • if someone takes its, won't deliver, should not be able take it anymore?

~I can set it that way. My thinking was right now, if you give it a shot, won't deliver, somebody else gives it a shot, won't deliver, then you can do it again. Do you want to keep only one try per person?~ ok, one try per person, discussed in DMs

  • is there limit issues a person have? Like 5 or so? So we focus on delivering PRs not assign race :D

nope, I was waiting for this info, let's make it 5 issues then :)

  • have you thought about making a dumb ranking system based on successful tasks, expired takes, unassigns?
  • did you thought about queue? like if there will be two persons who would like to do it, first who fails, can take over once first would not make it in time, surrender or won't deliver it out to keep continuous demand in tact. With ranking this would could play interesting, like adding higher rank to someone successfully deliver as backup someone else

ranking system can be done but it's tricky. what counts as successful issue completion? If you open PR, is it already successful? That can be some PR opening race as you've mentioned. If not, what if PR review takes longer. Is it unsuccessful/expired?

I'd say, let's track two data points, assigned+merged as successful and assigned+unassigned as unsuccessful. These two points can be counted for every dev and it can be tracked and perhaps added as columns to LEADERBOARD.MD?

~to the queue question, currently, locked period which is set, is stored only in kv workers. I can add queue, but re-assign will only happen on next bot trigger which is webhook for created comment. Therefore, if you are second you will be added to the queue and if somebody tries to assign issue after expiration again, you will be assigned instead. Won't happen automatically. Is this kind of queue ok?~ Autocode scheduler fixes this as well, I'm building queue rn.

Some debug

petersopko/kodabot-test#9

thank you for that, hard to debug this alone and github keeps flagging my fake accounts :D

My few coin-cents

yangwao commented 2 years ago

Offtopic thoughts would be nice (optional) for v2 assign bot. Bot would not assign higher priority issues, i.e. if we have p2 issues, some pool of issues which is significantly bigger than p3+, would not assign for p3+ or make it as a lower bound paid out issue, not sure how to handle this most efficiency.

I get that some p3 are easily solvable, but if projects would have ton of p2 open and people start working p5 probably it's not what they want to see being done on the short-term horizon from perspective of project

But this could be a highly controversial topic, but projects out there would help steer where they want to be if they set some priority labels for example.

yangwao commented 2 years ago

Let's continue to