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

65536 GitHub Comment Character Limit #19

Closed
0x4007 opened this issue Feb 14, 2024 · 25 comments · Fixed by #21 or #23
Closed

65536 GitHub Comment Character Limit #19

0x4007 opened this issue Feb 14, 2024 · 25 comments · Fixed by #21 or #23
Assignees
@0x4007
Labels
Price: 200 USD Priority: 4 (Urgent) Time: <4 Hours

Comments

@0x4007
Copy link
Member

0x4007 commented Feb 14, 2024 •
edited
Loading

This run seemed to have failed on the transmission step at the very end because according to the logs it even generated the permits, but then failed to post back to the original repository issue comments.

A GitHub comment size is max 65536 characters long. Since the conversation incentives were so long, perhaps it makes sense to truncate the permit comment's metadata (we use HTML comments to comment out an embedded JSON object) or to somehow compress it further.

https://github.com/ubiquibot/comment-incentives/actions/runs/7896867759/job/21551519371


Error: HttpError: Validation Failed: {"resource":"IssueComment","code":"unprocessable","field":"data","message":"Body is too long (maximum is 65536 characters)"}
@0x4007 0x4007 changed the title HTTP Validation Failed 65536 GitHub Comment Character Limit Feb 14, 2024
@0x4007 0x4007 mentioned this issue Feb 14, 2024
Closed
@0x4007 0x4007 mentioned this issue Feb 14, 2024
Closed
@ubiquibot ubiquibot bot mentioned this issue Feb 14, 2024
Closed
@0x4007
Copy link
Member Author

0x4007 commented Feb 15, 2024 •
edited
Loading

@rndquu would you mind handling this? Getting payouts to work is a top priority and this seems like there's many ways to go about handling this.

The vision is to be able to interoperate bot plugins with a standardized comment metadata schema. The problem is that this counts against the character limit. Perhaps we can filter out the _url related properties in the GitHub object embedded in the metadata. GitHub includes a ton of URLs as part of any object entity like a GitHub Issue object or a GitHub Comment object.

Beyond that, I would prefer to not have to compromise for edge cases with SOME permit comments not having metadata. It seems like that will lead to headaches down the line for future tooling if its not consistent.

@0x4007 0x4007 mentioned this issue Feb 16, 2024
Closed
@gitcoindev
Copy link
Contributor

I may be able to fix this today, will try to set up and reproduce on my forked repository.

@0x4007 0x4007 mentioned this issue Feb 16, 2024
Closed
@molecula451
Copy link
Contributor

you think you can have this one @gitcoindev ?

@gitcoindev
Copy link
Contributor

you think you can have this one @gitcoindev ?

Hi @molecula451 I am a bit stuck on the setup on my fork, it seems it needs to be run from org and depends on other config repositories. I tried to run locally from my local .env but in the end it needs github actions. If you feel you can fix this one quickly with QA please go ahead. I will experiment on my setup anyway today but feel free to pick it up -)

@molecula451
Copy link
Contributor

we're all debugging on this one, so that's what im asking i think we're all can come with a good solution

@gitcoindev
Copy link
Contributor

Ah sure. I also see @pavlovcik has been experimenting in parallel. I hope we can get it fixed together somehow.

@0x4007
Copy link
Member Author

0x4007 commented Feb 16, 2024

I'm setting up jest to mock the incoming event object

@0x4007
Copy link
Member Author

0x4007 commented Feb 16, 2024

It is somewhat stable (I discovered a bug!) so I'm working on that here #21

@gitcoindev
Copy link
Contributor

Btw. I am not sure if I can see in the 'matrix' correctly today after I spent quite some time looking at logs -) but for me the root cause seems to be that the comment that is going to be inserted is too large because of another error.

I see that relevance-scoring thrown

Failed to average samples {
  error: TypeError: Cannot read properties of undefined (reading 'map')
      at averageSamples (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/relevance-scoring.ts:170:8)

and in turn it created a cascade of trace dumps from failed generate-erc20-permit-signature.ts . This in total is larger than 65536 and it fails to insert such huge comment.

@gitcoindev
Copy link
Contributor

I also downloaded json and wrote a simple script that calculated comment body lengths:

36
706
6
660
5
51
49
6
660
79
1898
204
69
157
634
46
172
395
195
1261
1526
195
491
714
195
2377
194
16820
120
265
Sum: 30186

So as you can see even the sum of characters in all comments in this issue is less than 65536, which also is in favor of the theory that the new comment to be inserted is too large due to error traces.

@molecula451
Copy link
Contributor

30186? then is almost half of the acceptable character limit

@0x4007
Copy link
Member Author

0x4007 commented Feb 16, 2024 •
edited
Loading

Btw. I am not sure if I can see in the 'matrix' correctly today after I spent quite some time looking at logs -) but for me the root cause seems to be that the comment that is going to be inserted is too large because of another error.

I see that relevance-scoring thrown

Failed to average samples {
  error: TypeError: Cannot read properties of undefined (reading 'map')
      at averageSamples (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/relevance-scoring.ts:170:8)

and in turn it created a cascade of trace dumps from failed generate-erc20-permit-signature.ts . This in total is larger than 65536 and it fails to insert such huge comment.

Yea the bug I identified was that I accidentally coded it to check if the price labels ARRAY exists, instead of checking that the length is zero. That seemed to throw that map error that I would see so commonly in the logs.

In this specific scenario, the test issue has no labels and I think the code was not designed to handle that. Still getting it stable, I'm at permit generation part now (missing private key in my env vars!)

comment-incentives/src/handlers/issue/assignee-scoring.ts

Lines 6 to 75 in 77ba9b0

export async function assigneeScoring({
issue,
source,
view,
}: {
issue: GitHubIssue;
source: GitHubUser[];
view: ContributorView;
}): Promise<UserScoreDetails[]> {
// get the price label
const priceLabels = issue.labels.filter((label) => label.name.startsWith("Price: "));
if (!priceLabels.length) {
console.warn("There are no price labels in this repository.");
return []; // Return an empty array if there are no price labels
}
// get the smallest price label
const sortedPriceLabels = priceLabels.sort((a, b) => {
const priceA = parseFloat(a.name.replace("Price: ", ""));
const priceB = parseFloat(b.name.replace("Price: ", ""));
return priceA - priceB;
});
const smallestPriceLabel = sortedPriceLabels[0];
const priceLabelName = smallestPriceLabel.name;
const priceLabelMatch = priceLabelName.match(/\d+(\.\d+)?/);
const priceLabel = priceLabelMatch?.shift();
if (!priceLabel) {
console.warn("Price label is undefined");
return []; // Return an empty array if the price label is undefined
}
// get the price
const price = new Decimal(priceLabel);
// get the number of assignees
const numberOfAssignees = source.length;
const assigneeRewards = source.map((assignee) => {
// get the assignee multiplier
const assigneeMultiplier = new Decimal(1); // TODO: get the assignee multiplier from the database
// calculate the total
const splitReward = price.div(numberOfAssignees).times(assigneeMultiplier);
// return the total
const details: UserScoreDetails = {
score: splitReward,
view: view,
role: "Assignee",
contribution: "Task",
scoring: {
issueComments: null,
reviewComments: null,
specification: null,
task: price,
},
source: {
issue: issue,
user: assignee,
},
};
return details;
});
return assigneeRewards;
}

which also is in favor of the theory that the new comment to be inserted is too large due to error traces.

This is a good theory!

@0x4007
Copy link
Member Author

0x4007 commented Feb 16, 2024

This is a pretty solid co-op campaign

@0x4007
Copy link
Member Author

0x4007 commented Feb 16, 2024 •
edited
Loading

Hey guys, I got it to work! Can somebody work on generating a new X25519_PRIVATE_KEY for the ubiquibot organization? We need to share it for development purposes.

  1. Generate a new key
  2. Encrypt a new evmPrivateKeyEncrypted (no funds!)
  3. Add the shared test key to the comment-incentives readme.

567419d

@0x4007 0x4007 mentioned this issue Feb 16, 2024
Closed
@0x4007 0x4007 mentioned this issue Feb 16, 2024
Merged
@ubiquibot UbiquiBot
Copy link

ubiquibot bot commented Feb 16, 2024 

+ Evaluating results. Please wait...

@ubiquibot UbiquiBot
Copy link

ubiquibot bot commented Feb 16, 2024

[ 104.2 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueSpecification123
IssueComment677.4
ReviewComment23.8
Conversation Incentives
CommentFormattingRelevanceReward
This run seemed to have failed on the transmission step at the v...
23 
code:
  count: 1
  score: "1"
  words: 0
123
@rndquu would you mind handling this? Getting payouts to work is...
27 
code:
  count: 1
  score: "1"
  words: 1
0.76527
I'm setting up jest to mock the incoming event object...
2.20.5652.2
It is somewhat stable (I discovered a bug!) so I'm working on th...
4.60.624.6
> Btw. I am not sure if I can see in the 'matrix' correctly toda...
23.6 
a:
  count: 1
  score: "1"
  words: 2
code:
  count: 1
  score: "1"
  words: 0
hr:
  count: 1
  score: "1"
  words: 0
0.7823.6
This is a pretty solid co-op campaign...
1.60.8251.6
Hey guys, I got it to [work](https://github.com/ubiquibot/produc...
18.4 
a:
  count: 1
  score: "1"
  words: 1
li:
  count: 3
  score: "3"
  words: 24
code:
  count: 3
  score: "3"
  words: 4
0.8318.4
> code looks good, can you paste us some sort of debugging outpu...
2.4 
a:
  count: 1
  score: "2"
  words: 2
0.652.4
Let's give it a test @molecula451 ...
1.40.361.4

[ 5.5 WXDAI ]

@molecula451
Contributions Overview
ViewContributionCountReward
IssueComment34.2
ReviewComment11.3
Conversation Incentives
CommentFormattingRelevanceReward
you think you can have this one @gitcoindev ?...
0.80.410.8
we're all debugging on this one, so that's what im asking i thin...
2.40.542.4
30186? then is almost half of the acceptable character limit...
10.6851
code looks good, can you paste us some sort of debugging output/...
1.30.681.3

[ 31.95 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
IssueComment531.95
Conversation Incentives
CommentFormattingRelevanceReward
I may be able to fix this today, will try to set up and reproduc...
1.90.71.9
> you think you can have this one @gitcoindev ?

Hi @molecula4...

7.40.617.4
Ah sure. I also see @pavlovcik has been experimenting in paralle...
20.5752
Btw. I am not sure if I can see in the 'matrix' correctly today ...
11.35

code:
  count: 1
  score: "0.25"
  words: 0
0.69511.35
I also downloaded [json](https://api.github.com/repos/ubiquity/u...
9.3 
a:
  count: 1
  score: "0.25"
  words: 1
code:
  count: 1
  score: "0.25"
  words: 0
0.6559.3

@0x4007 0x4007 self-assigned this Feb 16, 2024
@ubiquibot UbiquiBot
Copy link

ubiquibot bot commented Feb 16, 2024

@pavlovcik the deadline is at 2024-02-16T21:20:06.472Z

@0x4007 0x4007 reopened this Feb 16, 2024
@0x4007 0x4007 closed this as completed Feb 16, 2024
@ubiquibot UbiquiBot
Copy link

ubiquibot bot commented Feb 16, 2024 

+ Evaluating results. Please wait...

@0x4007
Copy link
Member Author

0x4007 commented Feb 16, 2024 •
edited
Loading 

+ Evaluating results. Please wait...

Looks like it happened again. I'll use this exact issue to test and push another fix asap!

@molecula451
Copy link
Contributor

btw judging the gitcoindev message i think the character set it's manipulatible say if i post a comment that long loooking like a drop down table the bot could think that perhaps my contribution comment is that helpful because of longer?

need to round well

@molecula451
Copy link
Contributor

indeed it happened

image_2024-02-16_132738180

@0x4007
Copy link
Member Author

0x4007 commented Feb 16, 2024 •
edited
Loading

Here is the full transmission for the broken comment:

1708104501473.html.zip

Some initial observations:

  1. metadata can be minified (not pretty printed)
  2. wordScoreCommentDetails counting every single word in the conversation is pretty insane. We should probably exclude that.
  3. Conclusion is just to remove the metadata of the totals object. That will certainly take out most of the metadata.

@0x4007 0x4007 mentioned this issue Feb 16, 2024
Merged
@ubiquibot-dev UbiquiBot[dev]
Copy link

ubiquibot-dev bot commented Feb 16, 2024

[ 330.3 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueSpecification123
IssueTask1.00200
IssueComment8101.6
IssueComment80
ReviewComment23.8
ReviewComment21.9
Conversation Incentives
CommentFormattingRelevanceReward
This run seemed to have failed on the transmission step at the v...
23 
code:
  count: 1
  score: "1"
  words: 0
123
@rndquu would you mind handling this? Getting payouts to work is...
27 
code:
  count: 1
  score: "1"
  words: 1
0.727
I'm setting up jest to mock the incoming event object...
2.20.572.2
It is somewhat stable (I discovered a bug!) so I'm working on th...
4.60.7154.6
> Btw. I am not sure if I can see in the 'matrix' correctly toda...
23.6 
a:
  count: 1
  score: "1"
  words: 2
code:
  count: 1
  score: "1"
  words: 0
hr:
  count: 1
  score: "1"
  words: 0
0.76523.6
This is a pretty solid co-op campaign...
1.60.761.6
Hey guys, I got it to [work](https://github.com/ubiquibot/produc...
18.4 
a:
  count: 1
  score: "1"
  words: 1
li:
  count: 3
  score: "3"
  words: 24
code:
  count: 3
  score: "3"
  words: 4
0.6218.4
> ```diff > + Evaluating results. Please wait... > ```

Look...

5.6

a:
  count: 1
  score: "1"
  words: 2
code:
  count: 1
  score: "1"
  words: 0
0.6955.6
Here is the full transmission for the broken comment:

[170810...

18.6

a:
  count: 1
  score: "1"
  words: 3
li:
  count: 3
  score: "3"
  words: 47
code:
  count: 2
  score: "2"
  words: 2
hr:
  count: 1
  score: "1"
  words: 0
0.7818.6
@rndquu would you mind handling this? Getting payouts to work is...
- 
code:
  count: 1
  score: "0"
  words: 1
0.7-
I'm setting up jest to mock the incoming event object...
-0.57-
It is somewhat stable (I discovered a bug!) so I'm working on th...
-0.715-
> Btw. I am not sure if I can see in the 'matrix' correctly toda...
- 
a:
  count: 1
  score: "0"
  words: 2
code:
  count: 1
  score: "0"
  words: 0
hr:
  count: 1
  score: "0"
  words: 0
0.765-
This is a pretty solid co-op campaign...
-0.76-
Hey guys, I got it to [work](https://github.com/ubiquibot/produc...
- 
a:
  count: 1
  score: "0"
  words: 1
li:
  count: 3
  score: "0"
  words: 24
code:
  count: 3
  score: "0"
  words: 4
0.62-
> ```diff > + Evaluating results. Please wait... > ```

Look...

-

a:
  count: 1
  score: "0"
  words: 2
code:
  count: 1
  score: "0"
  words: 0
0.695-
Here is the full transmission for the broken comment:

[170810...

-

a:
  count: 1
  score: "0"
  words: 3
li:
  count: 3
  score: "0"
  words: 47
code:
  count: 2
  score: "0"
  words: 2
hr:
  count: 1
  score: "0"
  words: 0
0.78-
> code looks good, can you paste us some sort of debugging outpu...
2.4 
a:
  count: 1
  score: "2"
  words: 2
0.682.4
Let's give it a test @molecula451 ...
1.40.311.4
> code looks good, can you paste us some sort of debugging outpu...
1.2 
a:
  count: 1
  score: "1"
  words: 2
0.681.2
Let's give it a test @molecula451 ...
0.70.310.7

[ 10.7 WXDAI ]

@molecula451
Contributions Overview
ViewContributionCountReward
IssueComment59.1
ReviewComment21.6
Conversation Incentives
CommentFormattingRelevanceReward
you think you can have this one @gitcoindev ?...
0.80.6250.8
we're all debugging on this one, so that's what im asking i thin...
2.40.652.4
30186? then is almost half of the acceptable character limit...
10.811
btw judging the gitcoindev message i think the character set it'...
4.60.84.6
indeed it happened

![image_2024-02-16_132738180](https://git...

0.30.7550.3
code looks good, can you paste us some sort of debugging output/...
1.30.681.3
on my way!...
0.30.330.3

[ 31.95 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
IssueComment531.95
Conversation Incentives
CommentFormattingRelevanceReward
I may be able to fix this today, will try to set up and reproduc...
1.90.661.9
> you think you can have this one @gitcoindev ?

Hi @molecula4...

7.40.6557.4
Ah sure. I also see @pavlovcik has been experimenting in paralle...
20.682
Btw. I am not sure if I can see in the 'matrix' correctly today ...
11.35

code:
  count: 1
  score: "0.25"
  words: 0
0.7711.35
I also downloaded [json](https://api.github.com/repos/ubiquity/u...
9.3 
a:
  count: 1
  score: "0.25"
  words: 1
code:
  count: 1
  score: "0.25"
  words: 0
0.6659.3

@0x4007 0x4007 reopened this Feb 16, 2024
@0x4007 0x4007 closed this as completed Feb 16, 2024
@ubiquibot UbiquiBot
Copy link

ubiquibot bot commented Feb 16, 2024 

+ Evaluating results. Please wait...

@ubiquibot UbiquiBot
Copy link

ubiquibot bot commented Feb 16, 2024

[ 330.3 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueSpecification123
IssueTask1.00200
IssueComment8101.6
IssueComment80
ReviewComment23.8
ReviewComment21.9
Conversation Incentives
CommentFormattingRelevanceReward
This run seemed to have failed on the transmission step at the v...
23 
code:
  count: 1
  score: "1"
  words: 0
123
@rndquu would you mind handling this? Getting payouts to work is...
27 
code:
  count: 1
  score: "1"
  words: 1
0.6727
I'm setting up jest to mock the incoming event object...
2.20.562.2
It is somewhat stable (I discovered a bug!) so I'm working on th...
4.60.634.6
> Btw. I am not sure if I can see in the 'matrix' correctly toda...
23.6 
a:
  count: 1
  score: "1"
  words: 2
code:
  count: 1
  score: "1"
  words: 0
hr:
  count: 1
  score: "1"
  words: 0
0.7123.6
This is a pretty solid co-op campaign...
1.60.751.6
Hey guys, I got it to [work](https://github.com/ubiquibot/produc...
18.4 
a:
  count: 1
  score: "1"
  words: 1
li:
  count: 3
  score: "3"
  words: 24
code:
  count: 3
  score: "3"
  words: 4
0.7418.4
> ```diff > + Evaluating results. Please wait... > ```

Look...

5.6

a:
  count: 1
  score: "1"
  words: 2
code:
  count: 1
  score: "1"
  words: 0
0.85.6
Here is the full transmission for the broken comment:

[170810...

18.6

a:
  count: 1
  score: "1"
  words: 3
li:
  count: 3
  score: "3"
  words: 47
code:
  count: 2
  score: "2"
  words: 2
hr:
  count: 1
  score: "1"
  words: 0
0.7918.6
@rndquu would you mind handling this? Getting payouts to work is...
- 
code:
  count: 1
  score: "0"
  words: 1
0.67-
I'm setting up jest to mock the incoming event object...
-0.56-
It is somewhat stable (I discovered a bug!) so I'm working on th...
-0.63-
> Btw. I am not sure if I can see in the 'matrix' correctly toda...
- 
a:
  count: 1
  score: "0"
  words: 2
code:
  count: 1
  score: "0"
  words: 0
hr:
  count: 1
  score: "0"
  words: 0
0.71-
This is a pretty solid co-op campaign...
-0.75-
Hey guys, I got it to [work](https://github.com/ubiquibot/produc...
- 
a:
  count: 1
  score: "0"
  words: 1
li:
  count: 3
  score: "0"
  words: 24
code:
  count: 3
  score: "0"
  words: 4
0.74-
> ```diff > + Evaluating results. Please wait... > ```

Look...

-

a:
  count: 1
  score: "0"
  words: 2
code:
  count: 1
  score: "0"
  words: 0
0.8-
Here is the full transmission for the broken comment:

[170810...

-

a:
  count: 1
  score: "0"
  words: 3
li:
  count: 3
  score: "0"
  words: 47
code:
  count: 2
  score: "0"
  words: 2
hr:
  count: 1
  score: "0"
  words: 0
0.79-
> code looks good, can you paste us some sort of debugging outpu...
2.4 
a:
  count: 1
  score: "2"
  words: 2
0.672.4
Let's give it a test @molecula451 ...
1.40.361.4
> code looks good, can you paste us some sort of debugging outpu...
1.2 
a:
  count: 1
  score: "1"
  words: 2
0.671.2
Let's give it a test @molecula451 ...
0.70.360.7

[ 10.7 WXDAI ]

@molecula451
Contributions Overview
ViewContributionCountReward
IssueComment59.1
ReviewComment21.6
Conversation Incentives
CommentFormattingRelevanceReward
you think you can have this one @gitcoindev ?...
0.80.470.8
we're all debugging on this one, so that's what im asking i thin...
2.40.562.4
30186? then is almost half of the acceptable character limit...
10.831
btw judging the gitcoindev message i think the character set it'...
4.60.724.6
indeed it happened

![image_2024-02-16_132738180](https://git...

0.30.690.3
code looks good, can you paste us some sort of debugging output/...
1.30.771.3
on my way!...
0.30.340.3

[ 31.95 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
IssueComment531.95
Conversation Incentives
CommentFormattingRelevanceReward
I may be able to fix this today, will try to set up and reproduc...
1.90.561.9
> you think you can have this one @gitcoindev ?

Hi @molecula4...

7.40.537.4
Ah sure. I also see @pavlovcik has been experimenting in paralle...
20.582
Btw. I am not sure if I can see in the 'matrix' correctly today ...
11.35

code:
  count: 1
  score: "0.25"
  words: 0
0.7811.35
I also downloaded [json](https://api.github.com/repos/ubiquity/u...
9.3 
a:
  count: 1
  score: "0.25"
  words: 1
code:
  count: 1
  score: "0.25"
  words: 0
0.789.3

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

@0x4007 0x4007

Labels
Price: 200 USD Priority: 4 (Urgent) Time: <4 Hours
Projects
None yet
Milestone
No milestone
3 participants
@0x4007 @molecula451 @gitcoindev