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

QA 2 #4

Open
wants to merge 109 commits into
base: development
Choose a base branch
from
Open

QA 2 #4

wants to merge 109 commits into from

Conversation

kingsley-einstein
Copy link
Owner

Resolves #46

ubiquity-os bot and others added 30 commits October 31, 2024 20:22
@kingsley-einstein kingsley-einstein marked this pull request as draft December 24, 2024 19:35
@kingsley-einstein kingsley-einstein marked this pull request as ready for review December 24, 2024 19:35
@kingsley-einstein
Copy link
Owner Author

/scanContributions

@ubiquity-os-kernel-personal
Copy link

{
  "gentlementlegen": {},
  "Keyrxng": {},
  "0x4007": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1,
    "pull_request.commented": 3,
    "pull_request_review.changes_requested": 1,
    "pull_request_review_comment.created": 1
  },
  "ubiquity-os[bot]": {},
  "rndquu": {},
  "kingsley-einstein": {
    "pull_request.commented": 5,
    "pull_request.assigned": 1,
    "pull_request.cross-referenced": 1,
    "pull_request.convert_to_draft": 1,
    "pull_request.ready_for_review": 1,
    "+1": 1,
    "heart": 1,
    "hooray": 1
  },
  "whck6": {},
  "ubiquity-os-kernel-personal[bot]": {
    "pull_request.commented": 4
  },
  "EresDev": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1
  },
  "caller": "scanContributions"
}

@kingsley-einstein
Copy link
Owner Author

/scanContributions

@ubiquity-os-kernel-personal
Copy link

{
  "gentlementlegen": {},
  "Keyrxng": {},
  "0x4007": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1,
    "pull_request.commented": 3,
    "pull_request_review.changes_requested": 1,
    "pull_request_review_comment.created": 1
  },
  "ubiquity-os[bot]": {},
  "rndquu": {},
  "kingsley-einstein": {
    "pull_request.commented": 6,
    "pull_request.assigned": 1,
    "pull_request.cross-referenced": 1,
    "pull_request.convert_to_draft": 1,
    "pull_request.ready_for_review": 1,
    "pull_request.labeled": 1,
    "+1": 1,
    "heart": 1,
    "hooray": 1
  },
  "whck6": {},
  "ubiquity-os-kernel-personal[bot]": {
    "pull_request.commented": 5
  },
  "EresDev": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1
  },
  "caller": "scanContributions"
}

@0x4007
Copy link

0x4007 commented Dec 25, 2024

@gentlementlegen @whilefoo do you think this is ready for use in a rewards scheme?

I feel like there should be config added in but I'm not sure how it would be tested because generated rewards currently only happen in text-conversation-rewards.

@gentlementlegen
Copy link

It could be usable but not sure how to intend to communicate or handle rewards since this is its own plugin. What's the flow, in your mind?

@0x4007
Copy link

0x4007 commented Dec 25, 2024

We are supposed to change the kernel architecture to support "payment permit requests"

Each plugin then should be able to return a permit request

The kernel should post the combined sum of permits when the issue is closed as complete.

@gentlementlegen
Copy link

It seems to mix the kernel and plugins within the same codebase, transforming the kernel into a monolith doing everything. I think it would make more sense to enable the "passing data" to other plugins functionality so this one runs first then passes down the output to text-conversation-rewards. We also moved the permit generation to its own plugin as an endpoint so any plugin can just request a plugin generation which sounds simpler.

@kingsley-einstein
Copy link
Owner Author

/scanContributions

@0x4007
Copy link

0x4007 commented Dec 26, 2024

We also moved the permit generation to its own plugin as an endpoint so any plugin can just request a plugin generation which sounds simpler.

The main point is that we need to make this a separate plugin and it needs to be able to generate permits so whats the next steps looking like? Seems that when we accept this we still cant use it.

@gentlementlegen
Copy link

Right now, to generate permits, you need to include the permits npm package. If we deploy the endpoint, the plugin should just call that endpoint and get the results.

ubiquity-os/permit-generation#96

@kingsley-einstein
Copy link
Owner Author

/scanContributions

@ubiquity-os-kernel-personal
Copy link

{
  "gentlementlegen": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1,
    "pull_request.commented": 3
  },
  "Keyrxng": {},
  "0x4007": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1,
    "pull_request.commented": 6,
    "pull_request_review.changes_requested": 1,
    "pull_request_review_comment.created": 1
  },
  "ubiquity-os[bot]": {},
  "rndquu": {},
  "kingsley-einstein": {
    "pull_request.commented": 8,
    "pull_request.assigned": 1,
    "pull_request.cross-referenced": 1,
    "pull_request.convert_to_draft": 1,
    "pull_request.ready_for_review": 1,
    "pull_request.labeled": 1,
    "+1": 1,
    "heart": 1,
    "hooray": 1
  },
  "whck6": {},
  "ubiquity-os-kernel-personal[bot]": {
    "pull_request.commented": 6
  },
  "EresDev": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1
  },
  "MentleGentlemen": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1
  },
  "whilefoo": {
    "pull_request.mentioned": 1,
    "pull_request.subscribed": 1
  },
  "caller": "scanContributions"
}

@kingsley-einstein
Copy link
Owner Author

@0x4007
What do you think?

@whilefoo
Copy link

whilefoo commented Jan 1, 2025

We also moved the permit generation to its own plugin as an endpoint so any plugin can just request a plugin generation which sounds simpler.

Since we plan on aggregating rewards then the plugin should tell the kernel the reward and the kernel will store this information somewhere and the permit will be generated when the user cashes out on pay.ubq.fi, unless we leave plugins the ability to bypass our system and generate "raw" permits if they wish

@0x4007
Copy link

0x4007 commented Jan 1, 2025

Alright so do we accept this as is and not be able to use it or do you guys have clear instructions of changes that should be implemented so we can use this after accepting?

@gentlementlegen
Copy link

gentlementlegen commented Jan 8, 2025

For the required changes:

  • this should probably listen to "issue.closed" events
  • the result should be a JSON that is sent back to the kernel
  • it seems that all the workflow files are empty? (or is it a GitHub display problem)
  • bun should be used in favor of yarn

I think we could reimplement plugin communication within the kernel so that the output is this one is sent to text-conversation-rewards to take this into account for the final result. Because we care about only one unified permit with all the results in the end, correct?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be deleted.


export async function returnDataToKernel(repoToken: string, stateId: string, output: object, eventType = "return-data-to-ubiquity-os-kernel") {
export async function returnDataToKernel(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obsolete and would be handled by the SDK.

@@ -9,7 +9,7 @@ import { Logs } from "@ubiquity-dao/ubiquibot-logger";
*
* ubiquity:listeners: ["issue_comment.created", ...]
*/
export type SupportedEventsU = "issue_comment.created" | "issue_comment.deleted" | "issue_comment.edited";
export type SupportedEventsU = "issue_comment.created" | "issue_comment.edited";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to listen to these events but rather issue closed.

@@ -23,7 +23,7 @@ export const pluginSettingsSchema = T.Object(
configurableResponse: T.String(),
customStringsUrl: T.Optional(T.String()),
},
{ default: { configurableResponse: "Hello, world!" } }
{ default: { configurableResponse: "This is the UbiquityOS contributions scanner." } }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed.

compatibility_date = "2024-05-23"
node_compat = true
compatibility_date = "2024-11-27"
compatibility_flags = ["nodejs_compat"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrangler should not be needed as this probably should run as an Action.

@whilefoo
Copy link

whilefoo commented Jan 8, 2025

I think we could reimplement plugin communication within the kernel so that the output is this one is sent to text-conversation-rewards to take this into account for the final result. Because we care about only one unified permit with all the results in the end, correct?

We had some problems with KV, so I don't know if it's wise to enable plugin communication without fixing the problem (I can't find the issue about this). Alternatively we could make this as part of a module inside text-conversation-rewards

@gentlementlegen
Copy link

@whilefoo the problem was our quota. I think we can optimize it by checking if plugins are in chains and if so not storing results in KV, which was a proposal from @0x4007 but in the end we ended up removing it altogether because we actually never used it. Maybe here would be a first good use case?

Otherwise yes as you say it could be a module there, we were trying splitting things because this plugin is becoming a huge monolith piling up functionalities.

@0x4007
Copy link

0x4007 commented Jan 24, 2025

@gentlementlegen you should import this into text-conversation-rewards to use this. Perhaps you can create a new task and handle it? Otherwise is this ready to accept?

@whilefoo
Copy link

I can move the logic into text-conversation-rewards as a module and make a PR

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

Successfully merging this pull request may close these issues.

Generalized "GitHub Webhook + Contributor Role -> Rewards" No Config v1
4 participants