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

Update Request Enhancement #777

Open
0x4007 opened this issue Sep 16, 2023 · 14 comments
Open

Update Request Enhancement #777

0x4007 opened this issue Sep 16, 2023 · 14 comments

Comments

@0x4007
Copy link
Member

0x4007 commented Sep 16, 2023

Occasionally I see that assignees do not take the update request message seriously and a simple fix would be if the bot passes the reply into ChatGPT to understand if it's a valid update message.

If not, the bot should:

  1. Say something on lines of "this is not a valid update on the task. Please provide a detailed description of any updates not visible on GitHub."
  2. continue the original timer for unassignment.

I just realized that the conversation state might be difficult to track.

Scenarios

  1. Bot asks for update, and assignee replies with a valid update.
  2. Bot asks for update, and assignee replies without a valid update.
  3. Bot asks for update, and assignee replies without a valid update, and then eventually replies with a valid update.

Action Flows

  1. works similar to our current implementation
  2. works as described above.
  3. this is where it gets complicated. at this point, the bot should listen for all of the comments coming in from the assignee. If any of them seem like a valid update, then the bot must extend the timer. Otherwise, keep the original expiry time. Perhaps here is an opportunity for posting metadata in a bot comment to track the expiry time? I also feel like we need to carefully think through the prompt, because if the assignee continues to post on-topic comments and/or questions, then I would also consider them as valid "updates" (they are still working on the task.)

Do you have any updates @seprintour? If you would like to release the bounty back to the DevPool, please comment /stop
Last activity time: Mon Sep 11 2023 18:41:07 GMT+0000 (Coordinated Universal Time)

📌

Originally posted by @seprintour in #743 (comment)

@Keyrxng
Copy link
Member

Keyrxng commented Sep 17, 2023

/start

@ubiquibot
Copy link

ubiquibot bot commented Sep 17, 2023

Too many assigned issues, you have reached your max of 2

@Keyrxng
Copy link
Member

Keyrxng commented Sep 17, 2023

Or would some basic input validation be enough for now?

Min character count: 60
Min word count 5

Lorem Ipsum is simply dummy text of the printing and typesetting industry.

That is 74 characters and you could squeeze that out at least no problem

@0x4007
Copy link
Member Author

0x4007 commented Sep 17, 2023

Or would some basic input validation be enough for now?

This is a nice bandaid actually but I would be more comfortable if we checked real world examples of good and bad updates to define the constants. Appreciate if somebody can help with the research because I cant at the moment

@Keyrxng
Copy link
Member

Keyrxng commented Sep 18, 2023

I will do some actual research into it today and get back to you

@Keyrxng
Copy link
Member

Keyrxng commented Sep 18, 2023

So we do not want to shackle hunters by enforcing a template or structure I get that and given the observations in the table below I want to make the following points:

  • 1 - 6: I think all are decent
  • 7: I couldn't decide if this was good, it's clear to anyone that it fits the bill but is it sufficient to just link the pr?
  • 8: Clear action but no deadline, I mean that falls into the enforcing a layout if a deadline is required.
  • 9 - 14: All good examples of bad comments

I suggest the following:

  • Allow 7, first parse the comment for any linked PRs with an active unmerged PR the bot doesn't comment. Still I ask is it enough to just link the PR though?

  • Enforce a three word minimum as you can fit enough in three words. Number 3 is a good example of succinct and clear, others could be:

    • ["just started work", "haven't started yet", "waiting for review", "waiting for approval"]
  • Forget a character count I don't think this should be enforced if we adopt the word count and the word count is effective enough to get at the very least a bare minimum without forcing hunters to "have to think" lmao.

  • Any comment that is less than 3 words cannot be descriptive enough bar a couple that come to mind but that's the exception

ID Status Comment Link Description Comment
1 Good Issue 736 Clear action Sorry for the delay, I will raise a PR by today end.
2 Good Issue 500 Clear action @ubiquibot waiting for merge
3 Good Issue 593 Clear action In progress #649
4 Good Issue 583 Clear action Adding a comment for the bot: waiting for PR approval and merge.
5 Good Issue 496 Clear action @ubiquibot waiting for clarification in #545
6 Good Issue 192 Clear action There's a linked PR
7 IDK Issue 731 Links related so does the job #732
8 IDK Issue 158 Clear action but no deadline I will push soon
9 Bad Issue 743 Non-descript 📌
10 Bad Issue 192 Non-descript yes
11 Bad Issue 524 Non-descript ???
12 Bad Issue 96 Non-descript .
13 Bad Issue 96 Non-descript Wait my boy, patience
14 Bad Issue 774 Non-descript chill my friend

@0x4007
Copy link
Member Author

0x4007 commented Sep 18, 2023

Thanks a ton for the research! The pull request links are insufficient because this implies that no commits were added in a few days, which means that they stopped working. With this update comment, we want to learn why they stopped working on the code. I agree on everything but 6. Some remarks:

  • 3 should never have happened, the bot needs to be able to read draft pull requests
  • 6 I am assuming they stopped working on the pull request, so that's not a reason that is useful to hear.

Word count no longer seems sufficient because "Wait my boy, patience" is a funny, four word way to not say anything meaningful. Whereas "@ubiquibot waiting for merge" is a four word and meaningful way to say that its the pull request reviewers' fault for not merging in more promptly (although we have another spec for following up with reviewers instead of the assignees in this situation, which will eventually be implemented.)

@Keyrxng
Copy link
Member

Keyrxng commented Sep 18, 2023

I feel then that we'd need some form of NLP if not integrating AI in the first iteration and since we are set against adding in any more packages this could be very tricky.

I think that the best move may be the AI approach from the get-go, it has the capacity to cover all edge cases without adding anything into the build we don't already have.

what we know

  • n words can both provide value and/or be useless
  • a predefined structure is out of the question
  • character count appears more useless than word count in and of it's self

what we need

  • ability to distinguish succinct non-sense from succinct sense

my blockers

  • inferring context based on words without using NLP or AI
  • enforcing a structureless structure so that each comment contains enough info

alternative

Perhaps it may be easier to simply include in the bot comment something like

Do you have any updates @seprintour? If you would like to release the bounty back to the DevPool, please comment /stop
Last activity time: Mon Sep 11 2023 18:41:07 GMT+0000 (Coordinated Universal Time)
Notice: Your update should include what if anything remains to be completed as part of the PR

This leaves out anything that is already part of the commit data which you request as part of the spec and ensures that onlookers know roughly how long before things are complete.

Below is a better looking version I think, what purpose does it serve showing when the last activity time was? Instead replace that with a dig at how to build a good update comment.

Do you have any updates @seprintour? If you would like to release the bounty back to the DevPool, please comment /stop. When providing an update it should include what if anything remains to be completed as part of the PR

@0x4007
Copy link
Member Author

0x4007 commented Sep 18, 2023

I think that the best move may be the AI approach from the get-go, it has the capacity to cover all edge cases without adding anything into the build we don't already have.

Let's do it.

Do you have any updates @user? If you would like to release the bounty back to the DevPool, please comment /stop. When providing an update it should include what if anything remains to be completed as part of the PR

Let's change to:

@user, we haven't seen recent activity on this task. Please update us on your progress. Otherwise you can release it back to the DevPool by commenting /stop.

This leaves out anything that is already part of the commit data which you request as part of the spec and ensures that onlookers know roughly how long before things are complete.

I'm expecting that we'll get "real" updates once the assignees realize that its an AI. Particularly after their first interaction with it, and it replying that it needs a valid update or that they will be unassigned.

@Keyrxng
Copy link
Member

Keyrxng commented Sep 18, 2023

Sweet, I'll take this if you can assign me it.

Particularly after their first interaction with it

Most definitely the fear factor of it looking like it's actively being checked as opposed to an all most sort of fake check-up.

So I'm clear:

  1. Update the bot comment.
  2. Take User's reply and feed to GPT with a decent prompt structure.
  3. If it passes the check then no additional comment required and restart activityTime.
  4. If it fails the check then GPT produced comment scaring them into writing a better update or face consequences such as being unassigned if a maintainer has to step in and request more info?

Should the consequences be listed as part of the AI response, or keep a lil mystery and just leave it at "or face the consequences" 🤣

@0x4007
Copy link
Member Author

0x4007 commented Sep 18, 2023

I updated the spec. Hopefully that offers all the clarity that you need?

@Keyrxng
Copy link
Member

Keyrxng commented Sep 18, 2023

Ah ideal!

this is where it gets complicated.

So I think that we should use something like decideContextGPT here to track conversation state.

Why I think we should use this:

  • because there may be multiple bot update comments (which are easily filtered chronologically yeah but tracking user replies to a question vs providing an update not so much) but also to multiple users
  • We feed it the issue convo object, this time with timestamps as part of the streamlined comments.
  • decideContextGPT currently works with a much much larger input and performs excellent so a similar function will smoke this

GPT will easily be able to cover scenario 3 given the conversation history, names and timestamps.

It could more than likely be done without adding timestamps as well but for extra accuracy I think it would benefit from it

@0x4007
Copy link
Member Author

0x4007 commented Sep 18, 2023

Sounds like a plan @Keyrxng

@Keyrxng
Copy link
Member

Keyrxng commented Sep 18, 2023

Finishing up Blame and this through the week then I'll tackle Emma/Agents at the weekend

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants