Post comment with summary and details to GitHub issue

[gentlementlegen]

In the v1 of the Ubiquibot, when a result gets evaluated, a recap is posted to the related issue with a summary and details of the rewards as well as the link to the rewards themselves Example: https://github.com/ubiquity/cloudflare-deploy-action/issues/9#issuecomment-2028623754

The same logic should be applied in the v2 version by creating a new Module responsible to post that comment. This module will receive a similar input than the one mentioned here

The module should be:

• possible to enable / disable
• eventually configurable (what data to show / hide)
• coming with tests

[0x4007]

@whilefoo rfc on how we can deal with comment outputs. Perhaps we can have a standard recognized property on the output interface? Then the kernel can decide whether to pass it around or something.

interface PluginOutput {
  comment: string; // html comment
  rewards: Rewards; // { "whilefoo": "500", "token": "0x0" } etc
}

[jordan-ae]

/start

[ubiquibot[bot]]

Deadline	Mon, Apr 1, 7:36 PM UTC
Registered Wallet	0x2F05fD58023B0a95d1866aa0A3b672cEf05945c5

Tips:

• 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.

[gentlementlegen]

This needs https://github.com/ubiquibot/conversation-rewards/pull/7 to be merged first. Also probably needs https://github.com/ubiquibot/permit-generation/pull/2 to be able to generate the permits properly.

[0x4007]

This needs https://github.com/ubiquibot/conversation-rewards/pull/7 to be merged first. Also probably needs https://github.com/ubiquibot/permit-generation/pull/2 to be able to generate the permits properly.

I think you should fork from and overtake that second pull due to us being behind schedule

[whilefoo]

@whilefoo rfc on how we can deal with comment outputs. Perhaps we can have a standard recognized property on the output interface? Then the kernel can decide whether to pass it around or something.

interface PluginOutput {
  comment: string; // html comment
  rewards: Rewards; // { "whilefoo": "500", "token": "0x0" } etc
}

there are a couple of options:

1. we let the conversation-rewards plugin generate and post the comment
2. we put comment as output and then another module is responsible for posting it or let conversation-rewards generate rewards and permit-generation generate permits and a third module that uses output from previous plugins to make a comment and post it
3. we let the conversation-rewards plugin generate the comment and pass it as a standard property like you suggested.

In theory 2. option sounds good to separate concerns but it's another plugin which means another call to github actions thus more latency, so for the sake of speed it'd go with option 1 or 3, but going with these 2 options would mean there will 1 comment for rewards summary and 1 comment for permits. I'm not sure if option 3 is any better than option 1 because the plugin already has a token that has permissions to post comments so passing it to the kernel doesn't make much difference.

[gentlementlegen]

To me 1 is the most straightforward to do for few reasons:

• the comment reward plugin has all the needed data already
• it can import the https://github.com/ubiquibot/permit-generation to generate permits itself
• if it is done this way it can be used as a complete standalone without the kernel

3 might make more sense in terms of architecture however. In such case the kernel should pass down results. It is more of an architecture question. Although, if we ever have other plugins in the flow that have influence on the total incentives, it would make sense to go through the kernel to aggregate the total result.

[0x4007]

there are a couple of options:

1. we let the conversation-rewards plugin generate and post the comment

I think the most pure architecture would be that plugins can NOT inherit (write) authentication (only read if possible) of the kernel. As a consequence, no plugin should be able to post directly any issue. Ideally it should only be the kernel with a direct interface to issues. Plugins should just output comment HTML and the kernel can post it all in a single comment at the end of the webhook event invocation chain.

2. we put comment as output and then another module is responsible for posting it or let conversation-rewards generate rewards and permit-generation generate permits and a third module that uses output from previous plugins to make a comment and post it

Could be interesting to have a dedicated plugin to handle commenting, but because this seems like such an essential capability, I would more carefully consider the pros/cons of including this within the kernel.

• On one hand, I really like the idea of making the kernel as pure and lean as possible
• On the other hand there are practical considerations that if every plugin standard output interface includes comment: and then we don't have the comment capability available (the comment plugin is not included in the config) then why is this a standard output property required on every plugin?

3. we let the conversation-rewards plugin generate the comment and pass it as a standard property like you suggested.

In theory 2. option sounds good to separate concerns but it's another plugin which means another call to github actions thus more latency, so for the sake of speed it'd go with option 1 or 3, but going with these 2 options would mean there will 1 comment for rewards summary and 1 comment for permits. I'm not sure if option 3 is any better than option 1 because the plugin already has a token that has permissions to post comments so passing it to the kernel doesn't make much difference.

I'm not concerned about latency now. Besides we can port any plugin to Cloudflare Workers (or combine several inside a single Worker) when we are ready to fix latency issues. To be honest though, except for setting labels, I have no problem with latency (like with permit generation) so there's no need to overengineer yet. I also love the GitHub Actions logs being available for easy debugging, and the fact that they are super generous with the compute.

Given the entire conversation and all the considerations, I definitely think we should do option 3.

This separates concerns the best, which will allow our plugin ecosystem to grow the fastest.

[gentlementlegen]

I think each plugin should output JSON not html as it is not reliable to parse nor manipulate and requires window instance to be instantiated which is annoying on node based projects.

Having a plugin handling commenting seems quite weird as commenting is done by calling Octokit and the REST api which is already a library by itself, so no need to encapsulate it within another one to do the same thing.

My view on this, is to finalize https://github.com/ubiquibot/permit-generation/issues/5 to import it withing the conversation-reward that will post the comment itself as well, otherwise the architecture will be quite convoluted doing ping pong with everything.

[0x4007]

I think each plugin should output JSON not html as it is not reliable to parse nor manipulate

I know JSON makes things more complicated than it needs to be with serialization/transport. Then we have problems like this https://github.com/ubiquibot/conversation-rewards/issues/18. If the comment property is ONLY meant to be used for GitHub comments, then transporting HTML only is the most sensible. Besides, there's no other metadata thats necessary within the comment property, which is the main point of sending a JSON object (it has multiple properties.)

As a somewhat side note: there should also not be any variables inside of the HTML. We could look it as like server-side-rendering (or in this case, plugin-side-rendering) handle producing the final HTML output and then output it as a single string of html entities.

requires window instance to be instantiated which is annoying on node based projects.

We are currently using mdast in @ubiquibot/comment-incentives to generate virtual DOMs. No window needed.

My view on this, is to finalize https://github.com/ubiquibot/permit-generation/issues/5 to import it withing the conversation-reward that will post the comment itself as well, otherwise the architecture will be quite convoluted doing ping pong with everything.

If we agree that this is considered as technical debt, and that we are accruing this so that we can get back on schedule, ok.

[gentlementlegen]

If you want to manipulate and convey data, HTML really is not made for this. If you want something formatted similarly but made for data we can use XML format. The new comment reward actually does instantiate a DOM through JSDOM to make things way simpler instead of using Regex everywhere which is highly unreliable. But there it makes sense because we are parsing comments from an HTML page content.

Biggest advantage from this is to have the comment reward fully standalone, while easy to integrate with the kernel.

If we do something that handles the comment it means each and every module has to send it there and that module should understand every different content / format we send which would be way easier if the module itself handled its own comments, formatting wise.

[0x4007]

If you want to manipulate and convey data, HTML really is not made for this.

Going back to my "plugin-side-rendering" mention, the data manipulation happens inside of the logic of the plugin. Then when the plugin is finally done with all of its compute, it emits a single string in the comment output property. This string is the final, rendered HTML.

The comment output is not intended to be consumed by other plugins. In most cases, the kernel will concatenate all the comment outputs from every plugin and post a single comment at the end. Plugins will consume the metadata property output which will include raw values to work with. I forget where I originally proposed this, but imagine something like:

type HtmlString = string;
type RequestedPermits = { username: string; amount: string; token: string; }[]

interface PluginOutput {
   metadata: Record<string, any>;
   comment: HtmlString;
   rewards: RequestedPermits;
}

[gentlementlegen]

But then how do we consider the formatting of that output?

Practical case: we want to post a comment when a user queries a wallet. That comment is 'user name': 'wallet 0x0' Kernel calls the comment plugin, saying that it wants a comment to be posted. Should the Kernel send the rendering it wants, should the comment plugin transform the data to HTML?

Then, comment-reward wants to post the results. Should ask the Kernel to call the comment plugin, but then formatting is different. Should the Kernel notify the comment plugin that it wants a different output formatting? Should the Kernel compute beforehand the HTML and send it to the comment plugin?

Might be something I don't grasp there. Because I do understand your use case but it seems to be very deterministic on what is the purpose of the plugins which kinda defeats the purpose of having plugins, looks more like a monolithic approach to me

[0x4007]

But then how do we consider the formatting of that output?

The proposed comment output is intended for ease of composability for MOST situations, basically that the output from one plugin would just be appended to the final rendered comment. For simple plugin configurations (plugins not modifying the output results of others) this seems like straightforward architecture.

However, as we know, there will be situations where a subsequent plugin will consider the results of a previous plugin, which means it would need to change the comment that is rendered.

In this situation the subsequent plugin should clobber the output of the previous plugin. It is now clear to be that this will be a new challenge to express to the kernel that it should ignore the comment output of a previous plugin. I suppose it would be straightforward in the metadata using the plugin ID i.e. { "silences": ["ubiquibot/conversation-rewards"] }.

Kernel calls the comment plugin,

I think that comments should be handled from within the kernel. There should not be a separate comment plugin. Read my explanation here.

Inputs

For completeness of my previous comment:

type PluginId = string; // i.e. `ubiquibot/conversation-rewards` `ubiquibot/assistive-pricing`

interface PluginInput {
   metadata: Record<PluginId, any>;
   context: GitHubEventContext;
}

Notice that we should not pass in the HTML of other plugins. Instead, just the metadata ("computed") values from the previous plugins.

[gentlementlegen]

This can work, but we skyrocket coupling and to me defeat purpose of plugins that should be unaware of each other. If any plugin has to understand the result of a previous plugin, it means these plugins have necessarily to co-exist so basically they become a single plugin with no purpose to split them.

[0x4007]

so basically they become a single plugin with no purpose to split them.

I understand your concern and I would need to put more thought into composability. Maybe the comment proposal is bad after all due to your point. It's only useful with simple plugins, but for more complex ones your point is valid.

I need help to think through how any partner in the future can create new plugins that modify

1. permit reward amounts
2. XP reward amounts

I don't like the idea of having a single monolithic rewards generation plugin that wraps all the ways possible to compute rewards (i.e. time estimation, comment rewards, review rewards etc)

[gentlementlegen]

@pavlovcik To mitigate that that's why inside the comment reward itself I also integrated that Module principle so code is not coupled tightly and easy to build on. There is as usual pros and cons to both approaches (splitting or not) but biggest pro is that comments get evaluated once in the same spot, so we save calls to OpenAPI and speed up the process. Also makes it only one configuration file in one location. We need to think about our best options there.

[ubiquibot[bot]]

! action has an uncaught error

[gentlementlegen]

I realized that to carry this task properly we need to handle flags for comment more delicately as they only indicate if the comment is ISSUE | REVIEW with the level MEMBER | CONTRIBUTOR etc. but doesn't specify if it is from a task, a specification and so on. Tags should be added to the config properly as well.

[0x4007]

I realized that to carry this task properly we need to handle flags for comment more delicately as they only indicate if the comment is ISSUE | REVIEW with the level MEMBER | CONTRIBUTOR etc. but doesn't specify if it is from a task, a specification and so on. Tags should be added to the config properly as well.

I see, so you're suggesting that we must annotate each comment as well in order to properly handle the scoring at the end?

Off hand I think there's:

1. Specification
2. Issue comment
3. Pull request comment (a normal comment on a pull request, not related to posting a "review"
4. Pull request review comment (associated with a "review state" I.e. approved, left comments, requested changes)

I suppose we need a better naming convention for the pull related ones. They are considered as separate entities according to the GitHub api. They require different methods to obtain both types.

[gentlementlegen]

Agreed, I think currently there are 3 possible things to annotate on the comments:

• SPECIFICATION for the issue itself, TASK for the related PR fixing it, or simply COMMENT
• MEMBER or CONTRIBUTOR for the status of the member
• REVIEW or ISSUE for where the comment was added

I think this shall cover all cases.

[0x4007]

Consider calling it "contributor" and "collaborator" as that is how it is presented on the GitHub APIs as I recall.

Also I think you forgot about the "review comments"

[jordan-ae]

/stop

[ubiquibot[bot]]

+ You have been unassigned from the task

[gentlementlegen]

/start

[ubiquibot[bot]]

Deadline	Thu, Apr 18, 7:17 AM UTC
Registered Wallet	0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED

Tips:

• 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.

[ubiquibot[bot]]

+ Evaluating results. Please wait...

[ubiquibot[bot]]

[ 141.5 WXDAI ]

@0x4007

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	9	139.9
Review	Comment	1	1.6

Conversation Incentives

Comment	Formatting	Relevance	Reward
@whilefoo rfc on how we can deal with comment outputs. Perhaps w...	5.7 code: count: 1 score: "1" words: 0	0.62	5.7
> This needs https://github.com/ubiquibot/conversation-rewards/p...	1.7	0.71	1.7
> > there are a couple of options: > > 1. we let the conver...	35.5 li: count: 5 score: "5" words: 142 code: count: 2 score: "2" words: 2	0.65	35.5
> I think each plugin should output JSON not html as it is not r...	20.6 code: count: 4 score: "4" words: 6	0.69	20.6
> If you want to manipulate and convey data, HTML really is not ...	17.9 code: count: 5 score: "5" words: 4	0.65	17.9
> But then how do we consider the formatting of that output? ...	27.1 h3: count: 1 score: "1" words: 1 a: count: 2 score: "2" words: 3 code: count: 3 score: "3" words: 9	0.64	27.1
> so basically they become a single plugin with no purpose to sp...	12.8 li: count: 2 score: "2" words: 6 code: count: 1 score: "1" words: 1	0.6	12.8
> I realized that to carry this task properly we need to handle ...	15.7 li: count: 4 score: "4" words: 39 code: count: 2 score: "2" words: 4	0.75	15.7
Consider calling it "contributor" and "collaborator" as that is ...	2.9	0.69	2.9
> @0x4007 I believe that if I had all the tests in this PR it wi...	1.6	0.2	1.6

[ 616.9 WXDAI ]

@gentlementlegen

Contributions Overview

View	Contribution	Count	Reward
Issue	Specification	1	23.6
Issue	Task	1	400
Issue	Comment	9	180.4
Issue	Comment	9	0
Review	Comment	2	8.6
Review	Comment	2	4.3

Conversation Incentives

Comment	Formatting	Relevance	Reward
In the v1 of the Ubiquibot, when a result gets evaluated, a reca...	23.6 a: count: 1 score: "1" words: 1 li: count: 3 score: "3" words: 14	1	23.6
This needs https://github.com/ubiquibot/conversation-rewards/pul...	6.6	0.54	6.6
To me 1 is the most straightforward to do for few reasons: - th...	25 li: count: 3 score: "3" words: 41	0.7	25
I think each plugin should output JSON not html as it is not rel...	25.8 code: count: 3 score: "3" words: 4	0.75	25.8
If you want to manipulate and convey data, HTML really is not ma...	27.8 a: count: 1 score: "1" words: 1	0.61	27.8
But then how do we consider the formatting of that output? Pr...	31.8	0.66	31.8
This can work, but we skyrocket coupling and to me defeat purpos...	11	0.61	11
@pavlovcik To mitigate that that's why inside the comment reward...	18 code: count: 1 score: "1" words: 1	0.67	18
I realized that to carry this task properly we need to handle fl...	13.6 code: count: 2 score: "2" words: 4	0.7	13.6
Agreed, I think currently there are 3 possible things to annotat...	20.8 li: count: 3 score: "3" words: 47 code: count: 7 score: "7" words: 7	0.71	20.8
This needs https://github.com/ubiquibot/conversation-rewards/pul...	-	0.54	-
To me 1 is the most straightforward to do for few reasons: - th...	- li: count: 3 score: "0" words: 41	0.7	-
I think each plugin should output JSON not html as it is not rel...	- code: count: 3 score: "0" words: 4	0.75	-
If you want to manipulate and convey data, HTML really is not ma...	- a: count: 1 score: "0" words: 1	0.61	-
But then how do we consider the formatting of that output? Pr...	-	0.66	-
This can work, but we skyrocket coupling and to me defeat purpos...	-	0.61	-
@pavlovcik To mitigate that that's why inside the comment reward...	- code: count: 1 score: "0" words: 1	0.67	-
I realized that to carry this task properly we need to handle fl...	- code: count: 2 score: "0" words: 4	0.7	-
Agreed, I think currently there are 3 possible things to annotat...	- li: count: 3 score: "0" words: 47 code: count: 7 score: "0" words: 7	0.71	-
Example of successful comment posting with results: https://git...	3.2	0.34	3.2
@0x4007 I believe that if I had all the tests in this PR it will...	5.4	0.59	5.4
Example of successful comment posting with results: https://git...	1.6	0.34	1.6
@0x4007 I believe that if I had all the tests in this PR it will...	2.7	0.59	2.7

[ 21.3 WXDAI ]

@whilefoo

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	1	21.3

Conversation Incentives

Comment	Formatting	Relevance	Reward
> @whilefoo rfc on how we can deal with comment outputs. Perhaps...	21.3 li: count: 3 score: "3" words: 72 code: count: 1 score: "1" words: 0	0.67	21.3