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

Time "Top Up" Limits #51

Open
0x4007 opened this issue Nov 26, 2024 · 22 comments · May be fixed by #79
Open

Time "Top Up" Limits #51

0x4007 opened this issue Nov 26, 2024 · 22 comments · May be fixed by #79

Comments

@0x4007
Copy link
Member

0x4007 commented Nov 26, 2024

A quick idea that can use improvement:

Assignee can restart the disqualification timer by continuing to try. There's many clear examples of the task not being the right fit for the assignee and they try i.e. for months on projects that should be doable in a day.

We could consider doing limits on time limit "top ups." So for example:

Priority 1 - 5 top ups
Priority 2 - 4 top ups 
Priority 3 - 3 top ups 
Priority 4 - 2 top ups 
Priority 5 - 1 top up. 

I suspect that priority 5 tasks will border on eliminating most even capable assignees but once the task is complete even if just slightly after deadline we can always retroactively assign credit.


  • We should be able to toggle this behavior in the config.
  • If it is enabled the message should clearly explain the amount of remaining top ups.

I foresee a bit of complexity around the logic related to dealing with the "time limits" for follow up/disqualify and the top up counter.

  • If the limits are 7/14 as they are now, then the first top up should occur after 14 days on a priority 1 normal project.
  • priority 5 top up would be after 14 / 5 = 2.8 days.
  • Check the assignment timestamp and use that as a point of reference to calculate the current top up counter/state. i.e. if the task was assigned 35 days ago, and the clock speed is 14 days, then 35/14=2.5 meaning we are still at top up number 2.

Example Context

Logic:

14 days base timer / priority 4 = 3.5 days. 
this means that top up counter increments every 3.5 days. 
priority 4 has 2 additional top ups. 

1 November is the original granted 3.5 day time limit. 
after 3.5 days pass, the first top up is spent. 
after another 3.5 days pass, the second top up is spent. 

3.5 + 3.5 + 3.5 = 10.5 days has elapsed 

on the next attempt to follow up disqualification occurs because no more top ups remain. 

Remarks:

I wonder if the original time that we grant should be based on the Time: label because I see problems with tasks that are set to Time: <1 Week and Priority: 5 (Emergency) they would only be able to get 5.6 days to work on it.

Copy link
Contributor

ubiquity-os-beta bot commented Dec 4, 2024

Note

The following contributors may be suitable for this task:

Keyrxng

78% Match ubiquity-os-marketplace/command-start-stop#2
78% Match ubiquity/ubiquibot#681

ishowvel

78% Match ubiquity-os-marketplace/daemon-disqualifier#26

@gentlementlegen
Copy link
Member

@0x4007 Wouldn't it be simpler to use a "quota" or "credits" for reminders?

Based on what you describe:

  • priority 1 gets 5 credits
  • priority 2 gets 4 credits
  • etc...

So basically you would be allowed to get n reminders total for a given task, each of the reminders using 1 credit?

Maybe I didn't understand properly, but given the specification, consider the following example:

  • a user has 7 days to fulfill the task
  • the user takes 6 days to finish it, and changes it to "ready for review"
  • no review happens within 3.5 days, a reminder should be sent, but instead the user gets disqualified right away?

Is this correct?

@0x4007
Copy link
Member Author

0x4007 commented Jan 8, 2025

Is this correct?

Yes.

In practice the good talent gets their reviews processed pretty quickly. The ones who aren't cut out for the job end up being much slower from reviewers losing patience with them. But perhaps your proposal might be more fair.

The concept of "credits" seem to imply that storage is required. My strategy is just a calculation based on elapsed time.

You might be able to parse the entire timeline every time to see if there are exceptions to the timing though, like for slow reviews and such, but it makes the logic more complex.

@gentlementlegen
Copy link
Member

I think your idea makes more sense because with mine, a Priority 1 with a 1 Week deadline would get 5 reminders thus a month of extra time at most, which is too much for a one week tasks obviously.

Check the assignment timestamp and use that as a point of reference to calculate the current top up counter/state. i.e. if the task was assigned 35 days ago, and the clock speed is 14 days, then 35/14=2.5 meaning we are still at top up number 2.

However wouldn't this mean that it would also be the case with your spec? I know the deadlines are not meant to be strictly exact, but the gap between a week and a month seems too big.

Maybe we could make it as simple as one reminder allowed only? Would that be too harsh? If we handle slow reviews I think it would help us keeping deadline tighter.

@0x4007
Copy link
Member Author

0x4007 commented Jan 8, 2025

but the gap between a week and a month seems too big.

Of course in an ideal world everybody processes their tasks with utmost priority and speed. But the reality is that even our core team members spend several weeks on, for example, a one week task.

After monitoring these behaviors for some years I confidently came up with this design for the specification.

I think it's an acceptable starting point and we can iterate from there to try and coax better performance out of the team. However the main business problem I want to solve with this implementation is to disqualify the outside contributors who fall under this category:

There's many clear examples of the task not being the right fit for the assignee and they try i.e. for months on projects that should be doable in a day.

@gentlementlegen
Copy link
Member

Yes I understand the problem you are trying to solve. The one I am thinking of is when users take a 1h task, and they keep pushing for many days, so no reminder or blame is sent although that task should have been finished quite quickly. I've seen 15 minutes tasks take a week.

Let's try implementing your idea and see how it goes, it just feels like more pressure is put on high priority tasks which are currently mostly handled by the core team, when I think what you're targeting are low level priority tasks done by outside contributors.

@0x4007
Copy link
Member Author

0x4007 commented Jan 8, 2025

I would like to try the current proposed strategy first but we can potentially have a static amount of top ups too with an alternative configuration (for example, 5 for every priority level) which would make things easier for the higher priority tasks.

@0x4007
Copy link
Member Author

0x4007 commented Jan 21, 2025

/ask based on the specification please provide some typescript code for the developer to start this implementation with

1 similar comment
@gentlementlegen
Copy link
Member

/ask based on the specification please provide some typescript code for the developer to start this implementation with

Copy link
Contributor

Based on the specification, here's a TypeScript code snippet to help you start implementing the contribution scanning and top-up management system. This implementation leverages Octokit for interacting with GitHub APIs, integrates with the UbiquityOS SDK and Logger for ecosystem compatibility, manages environment variables using dotenv, and utilizes Luxon for date and time manipulation. Additionally, ensure that your project is set up with ESLint and Prettier for code quality and formatting standards.

// src/handlers/manageTopUps.ts

import { Octokit } from "@octokit/rest";
import { DateTime } from "luxon";
import { config } from "dotenv";
import { UbiquityOS, Logger } from "ubiquity-os-sdk";

// Load environment variables from .env file
config();

// Initialize Octokit with GitHub token
const octokit = new Octokit({
  auth: process.env.GITHUB_TOKEN,
});

// Initialize UbiquityOS SDK and logger
const ubiquity = new UbiquityOS();
const logger = new Logger("TopUpManager");

// Define priority to top-up mapping
const priorityTopUps: Record<number, number> = {
  1: 5,
  2: 4,
  3: 3,
  4: 2,
  5: 1,
};

// Define base timer in days
const BASE_TIMER_DAYS = 14;

// Interface for Issue Labels
interface Label {
  name: string;
}

// Function to determine priority based on labels
function getPriority(labels: Label[]): number | null {
  for (const label of labels) {
    const match = label.name.match(/Priority\s+(\d)/i);
    if (match) {
      return parseInt(match[1], 10);
    }
  }
  return null;
}

// Function to manage top-ups and disqualify assignees if necessary
export async function manageTopUps(owner: string, repo: string, issueNumber: number): Promise<void> {
  try {
    // Fetch issue details to get labels and assignee
    const { data: issue } = await octokit.issues.get({
      owner,
      repo,
      issue_number: issueNumber,
    });

    // Determine the priority of the issue
    const priority = getPriority(issue.labels as Label[]);
    if (priority === null || !(priority in priorityTopUps)) {
      logger.error(`Issue #${issueNumber} does not have a valid priority label.`);
      return;
    }

    const totalTopUps = priorityTopUps[priority];
    const assignee = issue.assignee?.login;

    if (!assignee) {
      logger.warn(`Issue #${issueNumber} does not have an assignee.`);
      return;
    }

    // Fetch issue timeline events
    const events = await octokit.issues.listEventsForTimeline({
      owner,
      repo,
      issue_number: issueNumber,
      per_page: 100,
    });

    // Get the assignment timestamp
    const assignmentEvent = events.data.find(event => event.event === "assigned" && event.assignee?.login === assignee);
    const assignmentDate = assignmentEvent ? DateTime.fromJSDate(new Date(assignmentEvent.created_at)) : DateTime.fromJSDate(new Date(issue.created_at));

    // Calculate elapsed time in days
    const now = DateTime.now();
    const elapsedDays = now.diff(assignmentDate, 'days').days;
    const elapsedTimers = Math.floor(elapsedDays / BASE_TIMER_DAYS);

    // Determine used top-ups
    const usedTopUps = Math.min(elapsedTimers, totalTopUps);

    if (usedTopUps >= totalTopUps) {
      // Disqualify the assignee
      await ubiquity.disqualifyAssignee(owner, repo, issueNumber, assignee);
      logger.info(`Assignee @${assignee} for issue #${issueNumber} has been disqualified due to exceeding top-ups.`);
    } else {
      // Notify the assignee about remaining top-ups
      const remainingTopUps = totalTopUps - usedTopUps;
      await ubiquity.notifyAssignee(owner, repo, issueNumber, assignee, remainingTopUps);
      logger.info(`Assignee @${assignee} for issue #${issueNumber} has ${remainingTopUps} top-ups remaining.`);
    }
  } catch (error) {
    logger.error(`Error managing top-ups for issue #${issueNumber}:`, { error });
    throw error;
  }
}

Explanation

  1. Imports and Initialization:

    • Octokit: Used for interacting with GitHub APIs.
    • Luxon: Utilized for robust date and time manipulation.
    • dotenv: Manages environment variables securely.
    • UbiquityOS SDK and Logger: Facilitates integration with the UbiquityOS ecosystem and provides logging capabilities.
  2. Configuration:

    • Environment variables are loaded from a .env file to manage sensitive information like GitHub tokens.
  3. Priority Mapping:

    • A priorityTopUps object maps each priority level to its corresponding number of top-ups. For example, a priority 1 issue has 5 top-ups.
  4. Utility Functions:

    • getPriority(labels: Label[]): Parses issue labels to determine the priority. It looks for labels matching the pattern Priority X where X is a number between 1 and 5.
  5. Main Function - manageTopUps:

    • Fetching Issue Details: Retrieves the issue to obtain labels and assignee information.
    • Determining Priority: Uses getPriority to ascertain the issue's priority level.
    • Fetching Timeline Events: Obtains the timeline of events related to the issue to find out when the issue was assigned.
    • Calculating Elapsed Time: Determines how many base timers (14 days each) have elapsed since the assignment.
    • Managing Top-Ups:
      • If the number of used top-ups meets or exceeds the total allowed based on priority, the assignee is disqualified.
      • Otherwise, the assignee is notified about the remaining top-ups.
    • Error Handling: Logs and throws errors encountered during the process to ensure visibility and ease of debugging.
  6. Integration with UbiquityOS SDK:

    • The functions disqualifyAssignee and notifyAssignee should be implemented within the UbiquityOS SDK to handle the respective actions.

Next Steps

  1. Implement UbiquityOS SDK Methods:

    • disqualifyAssignee: Define how an assignee is disqualified within your system.
    • notifyAssignee: Set up notifications to inform assignees about their remaining top-ups.
  2. Scheduling:

    • Integrate this function with a scheduler (e.g., GitHub Actions, cron jobs) to run periodically and manage top-ups automatically.
  3. Testing:

    • Develop comprehensive tests using Jest, leveraging Mock Service Worker (msw) to mock GitHub API responses. Ensure that edge cases are covered.
  4. Code Quality:

    • Ensure that ESLint and Prettier are properly configured to maintain code standards and formatting.
  5. Environment Variables:

    • Securely store and manage all necessary environment variables, such as GITHUB_TOKEN, in a .env file or your deployment environment's secret manager.
  6. Documentation:

    • Document the functionality, usage, and configuration steps to aid future developers and maintainers.

Example Usage

Assuming you have set up the necessary methods in the UbiquityOS SDK, you can invoke the manageTopUps function as follows:

import { manageTopUps } from "./handlers/manageTopUps";

// Example: Manage top-ups for issue #51 in the 'ubiquity-os-marketplace/daemon-disqualifier' repository
manageTopUps("ubiquity-os-marketplace", "daemon-disqualifier", 51)
  .then(() => {
    console.log("Top-up management completed successfully.");
  })
  .catch((error) => {
    console.error("An error occurred during top-up management:", error);
  });

This setup provides a robust foundation for managing top-ups based on issue priorities and elapsed time, ensuring that assignees are fairly evaluated and managed within the UbiquityOS ecosystem.

@gentlementlegen
Copy link
Member

I am convinced that the top-up calculation can be made much simpler, because in the end it is just a multiplier for the deadline, no need to calculate all the in-between steps, more likely just the "final final" deadline,

@0x4007
Copy link
Member Author

0x4007 commented Jan 21, 2025

no need to calculate all the in-between steps

What about the reminders? If that is handled by another set of logic then okay!

@ManyRios

This comment has been minimized.

Copy link
Contributor

[!CAUTION]

Copy link
Contributor

Caution

You are not assigned to this task

@gentlementlegen
Copy link
Member

/start

Copy link
Contributor

Warning! This task was created over 72 days ago. Please confirm that this issue specification is accurate before starting.
Beneficiary 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED

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.

Copy link
Contributor

ubiquity-os-beta bot commented Feb 6, 2025

Important

  • Be sure to link a pull-request before the first reminder to avoid disqualification.
  • Reminders will be sent every 1 day and 18 hours if there is no activity.
  • Assignees will be disqualified after 3 days and 12 hours of inactivity.

@gentlementlegen gentlementlegen linked a pull request Feb 6, 2025 that will close this issue
@gentlementlegen
Copy link
Member

@0x4007 Currently, if a reminder is sent, we do not send anymore reminders until the final deadline, or if user activity has happened in between. I guess this should be change to remind many times due to the top-up system?

@0x4007
Copy link
Member Author

0x4007 commented Feb 10, 2025

Yeah I think each top up should incur a reminder of sorts. I'm wondering if it's annoying to interject and say something like

"You used 3 of 5 top ups!" Etc

@gentlementlegen
Copy link
Member

Currently I just mention it in the reminder "Top ups remaining: 3" but that can be tweaked.

@0x4007
Copy link
Member Author

0x4007 commented Feb 10, 2025

I think there is value in saying both numbers to take advantage of the psychological principle of loss aversion.

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

Successfully merging a pull request may close this issue.

3 participants