ubiquibot / comment-incentives

0 stars 10 forks source link

`TypeError: counter.plus is not a function` #31

Closed 0x4007 closed 3 months ago

0x4007 commented 4 months ago

There's an issue I've never seen before. Please research a solution before we fund this issue.

TypeError: counter.plus is not a function
    at CommentScoring._calculateWordScores (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/comment-scoring-rubric.ts:164:25)
    at CommentScoring.computeWordScore (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/comment-scoring-rubric.ts:255:35)
    at perUserCommentScoring (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/per-user-comment-scoring.ts:10:19)
    at <anonymous> (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/all-comment-scoring.ts:43:9)
    at Array.forEach (<anonymous>)
    at <anonymous> (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/all-comment-scoring.ts:39:13)
    at Array.flatMap (<anonymous>)
    at allCommentScoring (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/all-comment-scoring.ts:25:6)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at commentsScoring (/home/runner/work/comment-incentives/comment-incentives/src/handlers/issue/evaluate-comments.ts:28:40)
Error: TypeError: counter.plus is not a function

Source https://github.com/ubiquibot/comment-incentives/actions/runs/8002349026/job/21855360540#step:5:262 Related https://github.com/ubiquity/nft-rewards/issues/3#issuecomment-1959049060

molecula451 commented 3 months ago

relates to the decimal.js package

https://github.com/ubiquibot/comment-incentives/blob/385a90e6797d493d334a2bab8a8d40de4bd4d021/src/handlers/issue/comment-scoring-rubric.ts#L164

Keyrxng commented 3 months ago

It looks like it's failing for the word "constructor" as far as I can tell, using the rpc-racer as a base for testing. I'm just running a script calling aggregateAndScoreContributions(..args) but I'm unfamiliar with the specifics of this part of the codebase

image

Keyrxng commented 3 months ago

this fails in and of itself

 const wordScoreCommentDetails: { [key: string]: Decimal } = {};
  const words = ["function", "was", "async", "but", "the", "constructor", "isn", "t", "I", "was"];
  const ZERO = new Decimal(0);
  const roleWordScore = new Decimal(0.1);

  for (const word of words) {
    let counter = wordScoreCommentDetails[word] || ZERO;
    counter = counter.plus(roleWordScore);

    wordScoreCommentDetails[word] = counter;
  }

the word constructor can't be used as a key or it's touching the object constructor

const wordScoreCommentDetails: { [key: string]: Decimal } = {};
  const words = ["function", "was", "async", "but", "the", "constructor", "isn", "t", "I", "was"];
  const ZERO = new Decimal(0);

  for (const word of words) {
    let counter = wordScoreCommentDetails[word] || ZERO;
    wordScoreCommentDetails[word] = counter;
  }

  console.log("scoringDetails: ", wordScoreCommentDetails);
scoringDetails:  {
  function: 0,
  was: 0,
  async: 0,
  but: 0,
  the: 0,
  constructor: [Function: Object],
  isn: 0,
  t: 0,
  I: 0
}
Keyrxng commented 3 months ago

_getWordsNotInDisabledElements() seems like the only sort of 'blacklist' but it's for elements as opposed to literal strings, have I missed in it the codebase or is there a need to add something like it for literals?

0x4007 commented 3 months ago

No there is no blacklist for words. Great find @Keyrxng!

Keyrxng commented 3 months ago

No there is no blacklist for words. Great find @Keyrxng!

What's the approach then just wrap it in a try catch and log the failed word(s) or implement something a bit more robust?

It might be an idea to exclude reserved words like ["constructor", "toString", "prototype", etc] but it'll essentially be the same as try, catch, log wouldn't it and would need manually updated if more are found

0x4007 commented 3 months ago

You can dynamically generate a blacklist based on all of the properties of the scoringDetails object.

It's not the right approach but we are going to have a brand new codebase to replace all this soon enough.

The alternative would be to design some type of encoding schema that will guarantee no collisions with the built in object properties (base64?) or alternatively just using some type of different data type to handle these words.

ubiquibot[bot] commented 3 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 3 months ago

[ 66 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueSpecification141.8
IssueComment218
ReviewComment16.2
Conversation Incentives
CommentFormattingRelevanceReward
There's an issue I've never seen before. Please research a solut...
41.8
code:
  count: 1
  score: "1"
  words: 0
141.8
No there is no blacklist for words. Great find @Keyrxng!...
20.742
You can dynamically generate a blacklist based on all of the pro...
16
code:
  count: 1
  score: "1"
  words: 1
0.67516
You can definitely include `my.ts` as a jest test. Just name it ...
6.2
code:
  count: 1
  score: "2"
  words: 2
0.626.2

[ 11.6 WXDAI ]

@molecula451
Contributions Overview
ViewContributionCountReward
IssueComment13.2
ReviewComment18.4
Conversation Incentives
CommentFormattingRelevanceReward
relates to the [decimal.js](https://www.npmjs.com/package/decima...
3.2
a:
  count: 1
  score: "1"
  words: 2
0.833.2
the only viable way i'd see testing this in a production like en...
8.40.558.4

[ 75 WXDAI ]

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueTask175
IssueComment40
Conversation Incentives
CommentFormattingRelevanceReward
It looks like it's failing for the word "constructor" as far as ...
-
li:
  count: 2
  score: "0"
  words: 8
code:
  count: 1
  score: "0"
  words: 2
0.78-
this fails in and of itself ```ts const wordScoreCommentDet...
-
code:
  count: 3
  score: "0"
  words: 0
0.62-
``_getWordsNotInDisabledElements()`` seems like the only sort of...
-
code:
  count: 1
  score: "0"
  words: 1
0.56-
> No there is no blacklist for words. Great find @Keyrxng! Wh...
-0.71-
molecula451 commented 3 months ago

working https://github.com/ubiquibot/comment-incentives/actions/runs/8288619639, nice hack! @Keyrxng

Keyrxng commented 3 months ago

I'm full of them đŸ˜‚