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

Reviewer Incentives #60

Comments

@0x4007
Copy link
Member

0x4007 commented Nov 29, 2024

Reviews always seem to be slower than new pulls being submitted. Perhaps with better incentive design we can speed up this operational problem.

My original spec was dense but provides a lot of context on the problem. I asked Claude to rewrite it to be more clear, but in case you want more context, please scroll to the bottom.

Jump to this comment to see final output amounts from a large pull.

Claude Rewrite

Base Calculation

interface ReviewReward {
  baseRate: number; // $1 per 100 lines
  conclusiveReviewCredit: number; // $25 flat rate
  excludedFiles: string[]; // Files marked with linguist-generated
}

function calculateReviewReward(linesChanged: number): number {
  return linesChanged / 100;
}

Core Components

1. Minimum Review Credit

  • Amount: $25 (conclusiveReviewCredit)
  • Conditions:
    • One-time payment per pull request
    • Requires conclusive review (approval or request changes)
    • Comments-only reviews do not qualify

2. Line Change Calculation

  • Formula: (additions + deletions - generated_files) / 100
  • Example:
function calculateNetLines(diff: {
  additions: number;
  deletions: number;
  generatedFiles: number;
}): number {
  return (diff.additions + diff.deletions - diff.generatedFiles) / 100;
}

3. Multiple Review Handling

  • Track line changes between review points
  • Only credit newly reviewed lines
  • Use commit hashes as review checkpoints

Example Scenarios

Single Complete Review

const example1 = {
  additions: 8406,
  deletions: 189,
  generatedFiles: 697,
  isConclusive: true
};

const reward = calculateNetLines(example1) + (example1.isConclusive ? 25 : 0);
// $78.98 + $25 = $103.98

Incremental Reviews

interface IncrementalReview {
  startCommit: string;
  endCommit: string;
  additions: number;
  deletions: number;
  generatedFiles: number;
}

function calculateIncrementalReward(reviews: IncrementalReview[]): number {
  return reviews.reduce((total, review) => 
    total + calculateNetLines(review), 0);
}

Implementation Notes

  1. File Exclusions

    • Use .gitattributes with linguist-generated
    • Common exclusions:
      • yarn.lock
      • Generated documentation
      • Build artifacts
  2. Diff Comparison

    • Recommendation: Use three-dot diff (...)
    • Rationale: More accurate representation of changes specific to PR
  3. Review Tracking

    • Store commit hashes as checkpoints
    • Calculate incremental diffs between reviews
    • Track review conclusiveness status
@0x4007
Copy link
Member Author

0x4007 commented Nov 29, 2024

Source Specification

uusd.ubq.fi#3

Taking this large pull as an example: https://github.com/ubiquity/uusd.ubq.fi/pull/3/files

+8406 
−189 
net line changes = 8595
minus yarn lockfile (697) = 7898
7898/100=$78.98 
We can exclude the lockfile diff and other common generated files line changes count using linguist-generated

I think a minimum credit of $25 could be nice because even taking the time to see a tightly scoped pull of only a few lines requires context switching and thinking about the changes in relation to the codebase, also while checking CI status and conversation state. We could add this minimum ONE TIME if the reviewer left a conclusive (approval or request changes, not just commented) so the full review would yield $103.98. We could also refer to it as a conclusiveReviewCredit: 25

Handling Multiple Reviews

Problem: pulls may be reviewed multiple times by a team member. How do we handle this dynamically (no hard numbers) so that the incentives scale with small and large pulls accurately?

Solution: we track exactly which lines the reviewer is reviewing and credit only for changed lines reviewed.

Example:

Second example, starting from the middle of the commits:

Footnotes

  1. I'm not sure whether to use the three dot diff or two dot diff (... or ..). three implies from our development, two I believe implies from their base development. If their base branch is our of date from ours, this will be a higher line change count with two dot diff. Two dot diff would yield $73.39 in this scenario.

@0x4007
Copy link
Member Author

0x4007 commented Nov 29, 2024

A last consideration is the premium for priority level. Similar to our other rewards, we should grant higher rewards for higher priority projects.

Multiplying the entire output doesn't seem right, because $25 * 4 (priority 4: urgent) = $100 just for leaving a rejection or approval which seems excessive.

In example 2 above, as this was a priority 4 task: $92.46 * 4 = $369.84 + $25 = $394.84 in review rewards for me.

Basically the same for rndquu as they approved the last commit at the time of writing.

whilefoo last reviewed ubiquity/uusd.ubq.fi@7e705c1 so

7,112 additions and 91 deletions.
697 generated
= 6,506 / 100 
= $65.06
* 4 = $260.24

zugdev ubiquity/uusd.ubq.fi@f07a96b

8,406 additions and 189 deletions.
697 generated
= $78.98
* 4 = $315.92

This does not include the conversation rewards so we might need to normalize them to be the same amount for every contributor. And review rewards only for collaborators.

@kingsley-einstein
Copy link

/start

@kingsley-einstein
Copy link

/help

Copy link

Available Commands

Command Description Example
/help List all available commands. /help
/allow Allows the user to modify the given label. /allow @user1 label
/ask Ask any question about the repository, issue or pull request /ask
/query Returns the user's wallet, access, and multiplier information. /query @UbiquityOS
/start Assign yourself and/or others to the issue/task. /start
/stop Unassign yourself from the issue/task. /stop
/wallet Register your wallet address for payments. /wallet ubq.eth

@kingsley-einstein
Copy link

/start

Copy link

Deadline Fri, Dec 6, 4:11 PM UTC
Beneficiary 0xb69DB7b7B3aD64d53126DCD1f4D5fBDaea4fF578

Tip

  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@kingsley-einstein
Copy link

@0x4007

Could you please create a repository that I could target with this?

@Keyrxng
Copy link
Member

Keyrxng commented Nov 30, 2024

I agree this would be great and solve a problem that's existed for some time now, particularly the last couple months although pace seems to be improving rapidly these day in my experience.

I started on https://github.com/ubiquity-os/ubiquity-os-plugin-installer ~3 weeks ago and it will be tied away in the next week so I am free to pick this up and rush it through if needed, lmk ty.

Copy link

@kingsley-einstein, this task has been idle for a while. Please provide an update.

@kingsley-einstein
Copy link

@kingsley-einstein, this task has been idle for a while. Please provide an update.

On it

@0x4007
Copy link
Member Author

0x4007 commented Dec 2, 2024

@kingsley-einstein the reason why it follows up is because you aren't working on it. You should be working on it to avoid the follow ups.

Copy link

@kingsley-einstein, this task has been idle for a while. Please provide an update.

@kingsley-einstein
Copy link

@kingsley-einstein, this task has been idle for a while. Please provide an update.

I'll push in a moment

Copy link

@kingsley-einstein, this task has been idle for a while. Please provide an update.

@kingsley-einstein
Copy link

@kingsley-einstein, this task has been idle for a while. Please provide an update.

Currently working on this

@surafeldev
Copy link

/help

Copy link

Available Commands

Command Description Example
/help List all available commands. /help
/allow Allows the user to modify the given label type. /allow @user1 label
/ask Ask any question about the repository, issue or pull request /ask
/query Returns the user's wallet, access, and multiplier information. /query @UbiquityOS
/start Assign yourself and/or others to the issue/task. /start
/stop Unassign yourself from the issue/task. /stop
/wallet Register your wallet address for payments. /wallet ubq.eth

@surafeldev
Copy link

/help

Copy link

Available Commands

Command Description Example
/help List all available commands. /help
/allow Allows the user to modify the given label type. /allow @user1 label
/ask Ask any question about the repository, issue or pull request /ask
/query Returns the user's wallet, access, and multiplier information. /query @UbiquityOS
/start Assign yourself and/or others to the issue/task. /start
/stop Unassign yourself from the issue/task. /stop
/wallet Register your wallet address for payments. /wallet ubq.eth

@surafeldev
Copy link

/query

Copy link

Failed to run command-query-user.
error: missing required argument 'username'

Usage: program [options] [command]

Options:
  -h, --help                   display help for command

Commands:
  /query [options] <username>
  help [command]               display help for command

@surafeldev
Copy link

/query @surafeldev

Copy link

Property Value
Wallet 0x0013f4217f6a8B48A92f7EA5d811A5F8D8364B93

@surafeldev
Copy link

/wallet 0xB13260bfEe08DcA208F2ECc735171B21763EaaF6

Copy link

+ Successfully registered wallet address

@surafeldev
Copy link

/query @surafeldev

Copy link

Property Value
Wallet 0x0013f4217f6a8B48A92f7EA5d811A5F8D8364B93

@surafeldev
Copy link

/start

Copy link

Deadline Mon, Dec 16, 8:30 PM UTC
Beneficiary 0x0013f4217f6a8B48A92f7EA5d811A5F8D8364B93

Tip

  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@surafeldev
Copy link

can I change the registered wallet, It seems like it can`t be changed by re registering my wallet address again ?

@ishowvel
Copy link

The spec basically gives a reward to the diff of the commits combined between each review

if this were to be a separate plugin wouldn't the reward infrastructure have to be implemented first?

@0x4007
Copy link
Member Author

0x4007 commented Dec 10, 2024

can I change the registered wallet, It seems like it can`t be changed by re registering my wallet address again ?

This seems like it might be related to that one problem a long time ago with the shifted rows in the database @whilefoo @gentlementlegen any ideas?

Also @ishowvel that is an accurate assessment so I think for the short term it would make sense for @gentlementlegen to implement a module inside of the text-conversation-rewards plugin and then later we can move it out to its own plugin once we support "permit requests" from all plugins to the kernel and the kernel generating them all in one shot.

@gentlementlegen
Copy link
Member

I think this pull-request should help re-registering a different wallet to an account.

@ishowvel
Copy link

/start

Copy link

! This issue is already assigned. Please choose another unassigned task.

@whilefoo
Copy link

Maybe this can be done after this task but I think we should also incentivise fast reviews, either by increasing the reward for fast reviews or decreasing reward for slow reviews.
This should also take into account that PRs with higher priority will be prioritized so slow reviews for lower priority tasks wouldn't be penalized, and also by taking size of the PR into account as bigger PRs will take longer to review than smaller PRs

@0x4007
Copy link
Member Author

0x4007 commented Dec 10, 2024

Maybe this can be done after this task but I think we should also incentivise fast reviews, either by increasing the reward for fast reviews or decreasing reward for slow reviews.

I remember thinking of this in the past but I couldn't find my old proposals. It would be helpful if you put together a proposal with the math formula as thats the most important part to figure out.

Copy link

@surafeldev, this task has been idle for a while. Please provide an update.

Copy link

Passed the disqualification threshold and no activity is detected, removing assignees: @surafeldev.

@ishowvel
Copy link

/start

Copy link

Deadline Fri, Dec 20, 12:27 PM UTC
Beneficiary 0x340D8d2bd82dEb4f485623453c9F6ad307e6B027

Tip

  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@surafeldev
Copy link

I have been researching and find out how should it be done, I have started working on it.

@ishowvel
Copy link

@0x4007
image
what should the metadata look like, i am a bit hazy on this so i am hoping you could help

@0x4007
Copy link
Member Author

0x4007 commented Dec 15, 2024

That looks fine for now, we can keep iterating to find something good. It's kind of a minor detail.

I think the ID is not useful to show so you can exclude it.

@ishowvel
Copy link

ishowvel commented Dec 15, 2024

The current sumRewards function gets away with choosing the least of two values to limit rewards for each type of reward because theres only one type of reward

by adding this pr we now have multiple types of rewards

25.006 (review diff + review base)
0.1 (review comment)

this would become 6.1 which would go over the limit, was this behavior intended?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment