Skip to content

Commit d63267a

Browse files
Fix heuristic issues, make tests more reliable (#164)
Fixes: jonathanpeppers/inclusive-code-reviews-browser#347 Fixes: jonathanpeppers/inclusive-code-reviews-browser#339 For our test cases, it's OK if we only only 50% confident that `isnegative` is 0. We are only underlining things when we are *confident* that `isnegative` is 1.
1 parent fc8b508 commit d63267a

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

comments/classified.csv

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1884,6 +1884,7 @@ _technically_ yes.,0,0.5
18841884
@github is #code the right thing we need here,0,0.5
18851885
@github Is a Cocksucking Little Bitch,1,0.5
18861886
@github is a DICK-FUCK,1,0.5
1887+
@github is a great teammate,0,0.5
18871888
@github IS AN UGLY IGNORANT FOOL,1,0.5
18881889
@github is obviously a fistfuckee. He loves an arm up his ass,1,0.5
18891890
@github It is always better to have multiple layers of protection and limit the potential attack surface where possible,0,0.5
@@ -2533,6 +2534,7 @@ Hey I thnk that you an asshole,1,0.5
25332534
Hey I'm just tellin' like it is you ugly as fuck butt fugly muthafucka,1,0.5
25342535
Hey idiot. I'm simply verifying the TRUTH. Do some fucking research before you remove my shit,1,0.5
25352536
Hey shithead,1,0.5
2537+
hey w,0,0.5
25362538
Hey... what is it,0,0.5
25372539
HHOS,0,0.5
25382540
Hi! I am back again,0,0.5
@@ -2561,6 +2563,7 @@ How are you measuring the startup time,0,0.5
25612563
How can we help investigate,0,0.5
25622564
How close to the wire is it getting,0,0.5
25632565
How did this *ever* work,1,0.5
2566+
how did this happen,0,0.5
25642567
How did we end up with:,0,0.5
25652568
How did you end up with #code,0,0.5
25662569
How did you end up with this error,0,0.5
@@ -2570,6 +2573,8 @@ How do you calculate "FPS",0,0.5
25702573
How does GetDiagnosticsUpdatedEventArgs work exactly,0,0.5
25712574
How long did you spend working on this,1,0.5
25722575
How would this ever work,1,0.5
2576+
how,0,0.5
2577+
how?,0,0.5
25732578
I *think* (and @github can correct me :) ) we tend to prefer that we explicitly define the dependencies in,0,0.5
25742579
I agree on [Sealed],0,0.5
25752580
I agree with the concern here: #url,0,0.5
@@ -2926,7 +2931,6 @@ It's dying on this line,0,0.5
29262931
It's going to ignore the scheme,0,0.5
29272932
It's not a terrible lot of work to swap out the tags. I know that I personally reserve the Bug tag for when I'm fairly confident something actually is a bug and either needs more investigation or is generally on the backlog somewhere. I suppose that's really #code's job,0,0.5
29282933
it's not going to help,1,0.5
2929-
It's not incredibly relevant I think - maybe we'd use that for 🅱️lectron - but.... We'll probably get rid of this code next week 🤣,0,0.5
29302934
It's not incredibly relevant I think - maybe we'd use that for electron - but.... We'll probably get rid of this code next week 🤣,0,0.5
29312935
It's not terrible; it's not great. I have no suggestions,0,0.5
29322936
It's OK to acknowledge that StringValues will rarely contain null array entries and not take the situation too seriously. I'm more concerned about the implementation having misleading #code overrides in it. The #code conveys that we've verified this value can't be null but the tooling isn't smart enough to follow our logic. This can be problematic for people maintaining the code starting from false assumptions,1,0.5
@@ -2935,6 +2939,7 @@ It's the first time I remember seeing #code cool haha,0,0.5
29352939
It's throwing an exception,0,0.5
29362940
item3 is a pretty terrible name. it also seems incorrect given that Item3 looks to be a DeclarationExpressionSyntax. Name tuple properties are a good thing :),0,0.5
29372941
J*sus. Get a f*cking life,1,0.5
2942+
Jimmy E.,0,0.5
29382943
Just a thought,0,0.5
29392944
Just add #nullable disable at the top of the test file,0,0.5
29402945
just becuz ur pussy 4 lying,1,0.5
@@ -3162,6 +3167,7 @@ Resulting in this effect,0,0.5
31623167
results in,0,0.5
31633168
return a match expression. Oh wait. Crap,0,0.5
31643169
Returns the corresponding symbol in this type or a base type that implements interfaceMember,0,0.5
3170+
Ricardo is a great teammate,0,0.5
31653171
right it's less selective you stupid fuck,1,0.5
31663172
Right now I don't think you can use LLVM without having an NDK -- but I was thinking we could fix that? Like it should be possible to work,0,0.5
31673173
Right now it is set as a property though and properties don't need a verb correct? I can revert to a method though,0,0.5
@@ -3204,6 +3210,7 @@ Should this have a Disconnect,0,0.5
32043210
should this value be true,0,0.5
32053211
Should we add more tests for the other shapes? Or is Ellipse sufficient,0,0.5
32063212
Should we also change this to throw an exception for older API levels,0,0.5
3213+
Should we ask Ricardo?,0,0.5
32073214
Should we be replacing this #code with an #code,0,0.5
32083215
Should we be using the #code stuff we have in #url. Or are we sure we won't get any output after the #code call has completed,0,0.5
32093216
Should we consider making this the default for all projects,0,0.5
@@ -3672,6 +3679,7 @@ Using SendAsync's cancellation token,0,0.5
36723679
Utterly stupid suggestion,1,0.5
36733680
uwp missing,0,0.5
36743681
Validated with my [crazy cleartype matrix](#url),0,0.5
3682+
WAIT A MINUTE. THIS IS AMAZING,0,0.5
36753683
wait... we have tests that call directly into this? ugh,0,0.5
36763684
waiting for the next bump that is building,0,0.5
36773685
was it returning -1 before,0,0.5

onnxjs/tests/onnx.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ describe('onnx tests', async () => {
1111
const urlRegex:RegExp = /\b(https?|ftp|file):\/\/[\-A-Za-z0-9+&@#\/%?=~_|!:,.;]*[\-A-Za-z0-9+&@#\/%=~_|]/gi;
1212
const punctuationRegex:RegExp = /(\.|!|\?|;|:)+$/g;
1313

14-
async function assertText(text:string, isnegative:string, confidence:number) {
14+
async function assertText(
15+
text:string,
16+
isnegative:string,
17+
isNegativeConfidence:number,
18+
isPositiveConfidence:number,
19+
) {
1520
const github_replaced = text.replace(githubHandleRegex, '@github');
1621
const backtick_replaced = github_replaced.replace(backtickRegex, '#code');
1722
const urls_replaced = backtick_replaced.replace(urlRegex, '#url');
@@ -27,12 +32,15 @@ describe('onnx tests', async () => {
2732
const score = results['Score.output'].data[Number(result)];
2833
console.log(`Text '${text}', IsNegative ${result}, Confidence ${score}`);
2934
expect(isnegative).to.be.equal(result);
30-
expect(score).to.be.greaterThan(confidence);
35+
expect(score).to.be.greaterThan(isnegative == "1" ? isNegativeConfidence : isPositiveConfidence);
3136
}
3237

3338
test_cases.forEach(x => {
3439
it(x.text, async () => {
35-
await assertText(x.text, x.isnegative, 0.7);
40+
// NOTE: for test_cases.json
41+
// We want to be 70% confident for cases where isnegative is 1
42+
// We only need to be 50% confident for cases where isnegative is 0
43+
await assertText(x.text, x.isnegative, 0.7, 0.5);
3644
});
3745
})
3846
});

onnxjs/tests/test_cases.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,5 +548,13 @@
548548
{
549549
"text": "You are right",
550550
"isnegative": "0"
551+
},
552+
{
553+
"text": "How w",
554+
"isnegative": "0"
555+
},
556+
{
557+
"text": "Ricardo E.",
558+
"isnegative": "0"
551559
}
552560
]

0 commit comments

Comments
 (0)