status-im
/
sourcecred
mirror of https://github.com/status-im/sourcecred.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Discard mentionsAuthorReference 

I added `mentionsAuthorReference` based on an untested hypothesis that
they would be useful. With the passage of time, I've never seen any
evidence that they actually improve cred socres (their impact seems
negligible), and they add complexity.

In the future, "go-fishing" style heuristics like this should not merge
unless they are of clearly demonstrated value. Also, it would be better
to add stuff like this via a standalone plugin rather than in the core
GitHub logic.

Undoes #806.

Test plan: `yarn test`
This commit is contained in:
Dandelion Mané 2019-06-04 23:12:21 +03:00
parent 3179ba841b
commit 4b1763ebc6
9 changed files with 1 additions and 596 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sharness/__snapshots__/example-github-load/data/sourcecred/example-github/graph.json
View File

File diff suppressed because one or more lines are too long

31
src/plugins/github/__snapshots__/createGraph.test.js.snap
View File

@ -1562,37 +1562,6 @@ Array [
"dstIndex": 45,
"srcIndex": 24,
},
Object {
"address": Array [
"sourcecred",
"github",
"MENTIONS_AUTHOR",
"6",
"sourcecred",
"github",
"PULL",
"sourcecred",
"example-github",
"5",
"8",
"sourcecred",
"github",
"COMMENT",
"PULL",
"sourcecred",
"example-github",
"5",
"396430464",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"wchargin",
],
"dstIndex": 23,
"srcIndex": 42,
},
Object {
"address": Array [
"sourcecred",

46
src/plugins/github/__snapshots__/edges.test.js.snap
View File

@ -78,52 +78,6 @@ Object {
}
`;
exports[`plugins/github/edges createEdge works for "mentionsAuthor" 1`] = `
Object {
"addressParts": Array [
"sourcecred",
"github",
"MENTIONS_AUTHOR",
"6",
"sourcecred",
"github",
"ISSUE",
"sourcecred",
"example-github",
"2",
"6",
"sourcecred",
"github",
"ISSUE",
"sourcecred",
"example-github",
"2",
"5",
"sourcecred",
"github",
"USERLIKE",
"USER",
"decentralion",
],
"dstParts": Array [
"sourcecred",
"github",
"ISSUE",
"sourcecred",
"example-github",
"2",
],
"srcParts": Array [
"sourcecred",
"github",
"ISSUE",
"sourcecred",
"example-github",
"2",
],
}
`;
exports[`plugins/github/edges createEdge works for "mergedAs" 1`] = `
Object {
"addressParts": Array [

5
src/plugins/github/createGraph.js
View File

@ -5,7 +5,6 @@ import * as GitNode from "../git/nodes";
import * as N from "./nodes";
import * as R from "./relationalView";
import {createEdge} from "./edges";
import {findMentionsAuthorReferences} from "./heuristics/mentionsAuthorReference";
import {ReactionContent$Values as Reactions} from "./graphqlTypes";
export function createGraph(view: R.RelationalView): Graph {
@ -76,10 +75,6 @@ class GraphCreator {
}
}
}
for (const mentionsAuthorReference of findMentionsAuthorReferences(view)) {
this.graph.addEdge(createEdge.mentionsAuthor(mentionsAuthorReference));
}
}
addNode(addr: N.StructuredAddress) {

17
src/plugins/github/declaration.js
View File

@ -130,22 +130,6 @@ const referencesEdgeType = Object.freeze({
`,
});
const mentionsAuthorEdgeType = Object.freeze({
forwardName: "mentions author of",
backwardName: "has author mentioned by",
defaultWeight: {forwards: 1, backwards: 0},
prefix: E.Prefix.mentionsAuthor,
description: dedent`\
Connects a post that mentions a user to posts in the same thread that
were authored by the mentioned user.
The intuition is that if a post is mentioning an author by name,
their contributions in that thread are probably particularly valuable.
This is an experimental feature and may be removed in a future version of SourceCred.
`,
});
const reactsHeartEdgeType = Object.freeze({
forwardName: "reacted ❤️ to",
backwardName: "got ❤️ from",
@ -201,7 +185,6 @@ const edgeTypes = Object.freeze([
hasParentEdgeType,
mergedAsEdgeType,
referencesEdgeType,
mentionsAuthorEdgeType,
reactsThumbsUpEdgeType,
reactsHeartEdgeType,
reactsHoorayEdgeType,

38
src/plugins/github/edges.js
View File

@ -8,7 +8,6 @@ import {
} from "../../core/graph";
import * as GithubNode from "./nodes";
import * as GitNode from "../git/nodes";
import type {MentionsAuthorReference} from "./heuristics/mentionsAuthorReference";
import {
type ReactionContent,
ReactionContent$Values as Reactions,
@ -20,7 +19,6 @@ export const AUTHORS_TYPE = "AUTHORS";
export const MERGED_AS_TYPE = "MERGED_AS";
export const HAS_PARENT_TYPE = "HAS_PARENT";
export const REFERENCES_TYPE = "REFERENCES";
export const MENTIONS_AUTHOR_TYPE = "MENTIONS_AUTHOR";
export const REACTS_TYPE = "REACTS";
// GitHub tracks its own notion of a commit, which has a particular
// database id, is scoped to a particular repository, and has a canonical url
@ -39,7 +37,6 @@ export const Prefix = Object.freeze({
mergedAs: githubEdgeAddress(MERGED_AS_TYPE),
references: githubEdgeAddress(REFERENCES_TYPE),
hasParent: githubEdgeAddress(HAS_PARENT_TYPE),
mentionsAuthor: githubEdgeAddress(MENTIONS_AUTHOR_TYPE),
reacts: githubEdgeAddress(REACTS_TYPE),
reactsThumbsUp: githubEdgeAddress(REACTS_TYPE, Reactions.THUMBS_UP),
reactsHeart: githubEdgeAddress(REACTS_TYPE, Reactions.HEART),
@ -66,10 +63,6 @@ export type ReferencesAddress = {|
+referrer: GithubNode.TextContentAddress,
+referent: GithubNode.ReferentAddress,
|};
export type MentionsAuthorAddress = {|
+type: typeof MENTIONS_AUTHOR_TYPE,
+reference: MentionsAuthorReference,
|};
export type ReactsAddress = {|
+type: typeof REACTS_TYPE,
+reactionType: ReactionContent,
@ -86,7 +79,6 @@ export type StructuredAddress =
| MergedAsAddress
| HasParentAddress
| ReferencesAddress
| MentionsAuthorAddress
| ReactsAddress
| CorrespondsToCommitAddress;
@ -131,11 +123,6 @@ export const createEdge = Object.freeze({
src: GithubNode.toRaw(referrer),
dst: GithubNode.toRaw(referent),
}),
mentionsAuthor: (reference: MentionsAuthorReference): Edge => ({
address: toRaw({type: MENTIONS_AUTHOR_TYPE, reference}),
src: GithubNode.toRaw(reference.src),
dst: GithubNode.toRaw(reference.dst),
}),
reacts: (
reactionType: ReactionContent,
user: GithubNode.UserlikeAddress,
@ -245,24 +232,6 @@ export function fromRaw(x: RawAddress): StructuredAddress {
): any);
return ({type: REFERENCES_TYPE, referrer, referent}: ReferencesAddress);
}
case MENTIONS_AUTHOR_TYPE: {
const parts = multiLengthDecode(rest, fail);
if (parts.length !== 3) {
throw fail();
}
const [srcParts, dstParts, whoParts] = parts;
const src: GithubNode.TextContentAddress = (GithubNode.fromRaw(
(NodeAddress.fromParts(srcParts): any)
): any);
const dst: GithubNode.TextContentAddress = (GithubNode.fromRaw(
(NodeAddress.fromParts(dstParts): any)
): any);
const who: GithubNode.UserlikeAddress = (GithubNode.fromRaw(
(NodeAddress.fromParts(whoParts): any)
): any);
const reference = {src, dst, who};
return {type: MENTIONS_AUTHOR_TYPE, reference};
}
case REACTS_TYPE: {
const [rawReactionType, ...rest2] = rest;
const reactionType = Reactions[rawReactionType];
@ -311,13 +280,6 @@ export function toRaw(x: StructuredAddress): RawAddress {
...lengthEncode(GithubNode.toRaw(x.referrer)),
...lengthEncode(GithubNode.toRaw(x.referent))
);
case MENTIONS_AUTHOR_TYPE:
return EdgeAddress.append(
Prefix.mentionsAuthor,
...lengthEncode(GithubNode.toRaw(x.reference.src)),
...lengthEncode(GithubNode.toRaw(x.reference.dst)),
...lengthEncode(GithubNode.toRaw(x.reference.who))
);
case REACTS_TYPE:
return EdgeAddress.append(
Prefix.reacts,

6
src/plugins/github/edges.test.js
View File

@ -58,12 +58,6 @@ describe("plugins/github/edges", () => {
createEdge.hasParent(nodeExamples.reviewComment(), nodeExamples.review()),
references: () =>
createEdge.references(nodeExamples.issue(), nodeExamples.pull()),
mentionsAuthor: () =>
createEdge.mentionsAuthor({
src: nodeExamples.issue(),
dst: nodeExamples.issue(),
who: nodeExamples.user(),
}),
reactsHeart: () =>
createEdge.reacts(
Reactions.HEART,

110
src/plugins/github/heuristics/mentionsAuthorReference.js
View File

@ -1,110 +0,0 @@
// @flow
import * as MapUtil from "../../../util/map";
import * as RV from "../relationalView";
import * as N from "../nodes";
// MentionsAuthorReferences are inferred edges between posts. If a post
// contains an @-mention of a user, and that user has authored any posts
// in the same thread, then we create MentionsAuthorReferences from the
// mentioning post to any posts in the same thread that were authored by
// the mentioned author.
//
// A concrete example: Suppose I write a post, and I @-mention you. Then
// my post will have MentionsAuthorReferences pointing to any posts that
// you write. The inference is that because I mentioned you by name, my
// post probably values your posts (either because I am referring to
// content you wrote in a previous post, or because I appreciate that
// you showed up to engage with me after being mentioned)
//
// See #804 for context:
// https://github.com/sourcecred/sourcecred/issues/804
export type MentionsAuthorReference = {|
// The post that generated an mentions author reference (via @-mention)
+src: N.TextContentAddress,
// The post authored by the person mentioned in the src
+dst: N.TextContentAddress,
// The person that was mentioned in src. Storing `who` is necessary
// because in the case of paired authorship, there might be multiple
// pointers from the same src to the same dst
+who: N.UserlikeAddress,
|};
export function* findMentionsAuthorReferences(
view: RV.RelationalView
): Iterator<MentionsAuthorReference> {
function* issuesAndPulls() {
yield* view.issues();
yield* view.pulls();
}
for (const post of issuesAndPulls()) {
const thread = createThread(post);
yield* referencesFromThread(thread);
}
}
// A post in a thread. This is either a top-level post (like an issue or
// pull request) or a child (like a comment).
type ThreadPost = {|
// Address of the post.
+address: N.TextContentAddress,
// List of all authors of the post. Order is undefined (this is a set
// encoded as a list).
+authors: $ReadOnlyArray<N.UserlikeAddress>,
|};
type Thread = {|
// Includes the root post.
// Ordered chronologically.
+posts: $ReadOnlyArray<ThreadPost>,
// Map from raw addresses of userlike kind to an array of all indices
// `i` such that `posts[i]` references the user (in arbitrary order).
// The domain is exactly the set of all users who are referenced in
// the thread at least once.
+userlikeToPostsReferencingThem: Map<N.RawAddress, $ReadOnlyArray<number>>,
|};
function createThread(root: RV.Issue | RV.Pull): Thread {
const posts = [];
const userlikeToPostsReferencingThem = new Map();
function addPost(x: RV.Issue | RV.Pull | RV.Comment) {
const address = x.address();
const authors = Array.from(x.authors()).map((x) => x.address());
const post = {address, authors};
posts.push(post);
for (const referenced of x.references()) {
if (referenced instanceof RV.Userlike) {
const referencedAddress = N.toRaw(referenced.address());
MapUtil.pushValue(
(((userlikeToPostsReferencingThem: Map<
N.RawAddress,
$ReadOnlyArray<number>
>): any): Map<N.RawAddress, number[]>),
referencedAddress,
posts.length - 1
);
}
}
}
addPost(root);
for (const comment of root.comments()) {
addPost(comment);
}
return {posts, userlikeToPostsReferencingThem};
}
function* referencesFromThread(
thread: Thread
): Iterator<MentionsAuthorReference> {
for (const post of thread.posts) {
for (const author of post.authors) {
for (const postIndex of thread.userlikeToPostsReferencingThem.get(
N.toRaw(author)
) || []) {
const thankingAddress = thread.posts[postIndex].address;
yield {src: thankingAddress, dst: post.address, who: author};
}
}
}
}

342
src/plugins/github/heuristics/mentionsAuthorReference.test.js
View File

@ -1,342 +0,0 @@
// @flow
import * as RV from "../relationalView";
import * as N from "../nodes";
import type {Repository} from "../graphqlTypes";
import deepEqual from "lodash.isequal";
import {findMentionsAuthorReferences} from "./mentionsAuthorReference";
describe("plugins/github/heuristics/mentionsAuthorReference", () => {
function exampleRelationalView(): RV.RelationalView {
const view = new RV.RelationalView();
const authors = {
steven: () => ({
__typename: "User",
id: "user:steven",
login: "steven",
url: "https://github.com/steven",
}),
garnet: () => ({
__typename: "User",
id: "user:garnet",
login: "garnet",
url: "https://github.com/garnet",
}),
amethyst: () => ({
__typename: "User",
id: "user:amethyst",
login: "amethyst",
url: "https://github.com/amethyst",
}),
pearl: () => ({
__typename: "User",
id: "user:pearl",
login: "pearl",
url: "https://github.com/pearl",
}),
holo: () => ({
__typename: "User",
id: "user:holo-pearl",
login: "holo-pearl",
url: "https://github.com/holo-pearl",
}),
};
const repoUrl = "https://github.com/my-owner/my-repo";
function issueUrl(number: number): string {
return `${repoUrl}/issues/${String(number)}`;
}
function issueCommentUrl(issueNumber: number, commentNumber: number) {
return `${issueUrl(issueNumber)}#issuecomment-${commentNumber}`;
}
const repository: Repository = {
__typename: "Repository",
id: "repo:my-repo",
createdAt: "2019-05-20T22:52:07Z",
issues: [
{
__typename: "Issue",
id: "issue:1",
url: issueUrl(1),
number: 1,
title: "calling into the void",
body: "hi @amethyst",
author: authors.steven(),
comments: [],
reactions: [],
createdAt: "2019-05-20T22:52:07Z",
},
{
__typename: "Issue",
id: "issue:2",
number: 2,
url: issueUrl(2),
title: "an issue with many types of references",
body: "it is me, @steven\n\nPaired with: @pearl",
author: authors.steven(),
reactions: [],
createdAt: "2019-05-20T22:52:07Z",
comments: [
{
__typename: "IssueComment",
id: "comment:2_1",
url: issueCommentUrl(2, 1),
body: "parry parry thrust @pearl\nparry parry thrust @steven",
author: authors.holo(),
reactions: [],
createdAt: "2019-05-20T22:52:07Z",
},
{
__typename: "IssueComment",
id: "comment:2_2",
url: issueCommentUrl(2, 2),
body: "@holo-pearl: stop!",
author: authors.steven(),
reactions: [],
createdAt: "2019-05-20T22:52:07Z",
},
{
__typename: "IssueComment",
id: "comment:2_3",
url: issueCommentUrl(2, 3),
body: "@amethyst @garnet why aren't you helping",
author: authors.pearl(),
reactions: [],
createdAt: "2019-05-20T22:52:07Z",
},
{
__typename: "IssueComment",
id: "comment:2_4",
url: issueCommentUrl(2, 4),
body: "@amethyst! come quickly, @amethyst!",
author: authors.garnet(),
reactions: [],
createdAt: "2019-05-20T22:52:07Z",
},
{
__typename: "IssueComment",
id: "comment:2_5",
url: issueCommentUrl(2, 5),
body: "i am busy fighting @boomerang-blade guy",
author: authors.amethyst(),
reactions: [],
createdAt: "2019-05-20T22:52:07Z",
},
],
},
],
pullRequests: [
{
__typename: "PullRequest",
id: "pull:3",
url: "https://github.com/my-owner/my-repo/pulls/3",
number: 3,
body: "Self referentially yours: @steven",
title: "a pull request, just because",
mergeCommit: null,
additions: 0,
deletions: 0,
reactions: [],
comments: [],
author: authors.steven(),
reviews: [],
createdAt: "2019-05-20T22:52:07Z",
},
],
url: "https://github.com/my-owner/my-repo",
name: "my-repo",
owner: {
__typename: "Organization",
id: "org:my-owner",
login: "my-owner",
url: "https://github.com/my-owner",
},
defaultBranchRef: null,
};
view.addRepository(repository);
return view;
}
describe("findMentionsAuthorReferences", () => {
// Tracks all expected references, across test cases, so that we can verify
// at the end that no unexpected references are present.
let expectedMentionsAuthorReferences = [];
function expectReferences(name, references) {
expectedMentionsAuthorReferences.push(...references);
const relationalView = exampleRelationalView();
const allMentionsAuthorReferences = Array.from(
findMentionsAuthorReferences(relationalView)
);
it(`has references for ${name}`, () => {
for (const {src, dst, who} of references) {
// For clarity in debugging: Error early if our src/dst/who is
// not actually present in the RelationalView
expect(relationalView.entity(src)).toEqual(expect.anything());
expect(relationalView.entity(dst)).toEqual(expect.anything());
expect(relationalView.entity(who)).toEqual(expect.anything());
// Then verify that the reference is actually present
expect(allMentionsAuthorReferences).toContainEqual({src, dst, who});
}
});
}
const repo = {type: N.REPO_TYPE, owner: "my-owner", name: "my-repo"};
function issueAddress(number: number): N.IssueAddress {
return {type: N.ISSUE_TYPE, repo, number: String(number)};
}
function commentAddress(
issueNum: number,
commentNum: number
): N.CommentAddress {
const id = String(commentNum);
return {type: N.COMMENT_TYPE, parent: issueAddress(issueNum), id};
}
const users = Object.freeze({
pearl: {type: N.USERLIKE_TYPE, subtype: N.USER_SUBTYPE, login: "pearl"},
steven: {type: N.USERLIKE_TYPE, subtype: N.USER_SUBTYPE, login: "steven"},
holoPearl: {
type: N.USERLIKE_TYPE,
subtype: N.USER_SUBTYPE,
login: "holo-pearl",
},
amethyst: {
type: N.USERLIKE_TYPE,
subtype: N.USER_SUBTYPE,
login: "amethyst",
},
garnet: {type: N.USERLIKE_TYPE, subtype: N.USER_SUBTYPE, login: "garnet"},
});
/** The test cases below are organized so that for every
* post in the thread rooted at Issue #2, we expect all
* of the new MentionsAuthorReferences that were created
* as a result of that post (whether because that post is
* the src or the dst). At the end, we verify that every
* MentionsAuthorReference is accounted for. */
// If an author references themself, there will be a self-directed
// MentionsAuthorReference.
expectReferences("issue:2", [
{
src: issueAddress(2),
dst: issueAddress(2),
who: users.steven,
},
]);
// When there was a multi-author, one comment could produce multiple
// edges from the same src to the same dst. The `who` field
// disambiguates.
expectReferences("comment:2_1", [
{
src: commentAddress(2, 1),
dst: issueAddress(2),
who: users.pearl,
},
{
src: commentAddress(2, 1),
dst: issueAddress(2),
who: users.steven,
},
]);
expectReferences("comment:2_2", [
{
src: commentAddress(2, 2),
dst: commentAddress(2, 1),
who: users.holoPearl,
},
// Because Steven self-referenced, that post references all of Steven's
// other posts. (If people use this for gaming, we can reconsider, but
// it should be fairly obvious)
{src: issueAddress(2), dst: commentAddress(2, 2), who: users.steven},
{src: commentAddress(2, 1), dst: commentAddress(2, 2), who: users.steven},
]);
//
expectReferences("comment:2_3", [
{
src: commentAddress(2, 1),
dst: commentAddress(2, 3),
who: users.pearl,
},
]);
// This test case demonstrates that post 2_3, which mentioned @garnet before
// she participated in the thread, still creates a reference once @garnet
// appears.
expectReferences("comment:2_4", [
{
src: commentAddress(2, 3),
dst: commentAddress(2, 4),
who: users.garnet,
},
]);
// This test case demonstrates two interesting properties:
// - comment(2,3) generated two different MentionsAuthorReferences to
// different authors
// - comment(2,4) created two identical MentionsAuthorReferences
// pointing to this one
expectReferences("comment:2_5", [
{
src: commentAddress(2, 3),
dst: commentAddress(2, 5),
who: users.amethyst,
},
{
src: commentAddress(2, 4),
dst: commentAddress(2, 5),
who: users.amethyst,
},
{
src: commentAddress(2, 4),
dst: commentAddress(2, 5),
who: users.amethyst,
},
]);
// Make it very clear that the duplicate reference is, in fact, duplicated.
// (`expectReferences` would locally pass even if the reference were not
// duplicated. The suite would still fail because we compare lengths at the
// end, but having this test makes the failure much easier to see.)
it("the same reference can be created multiple times", () => {
const duplicateReference = {
src: commentAddress(2, 4),
dst: commentAddress(2, 5),
who: users.amethyst,
};
const relationalView = exampleRelationalView();
const allMentionsAuthorReferences = Array.from(
findMentionsAuthorReferences(relationalView)
);
const matching = allMentionsAuthorReferences.filter((x) =>
deepEqual(x, duplicateReference)
);
expect(matching).toHaveLength(2);
});
const pullAddress = {repo, type: N.PULL_TYPE, number: "3"};
// Verify that we check pulls too
expectReferences("pull:3", [
{
src: pullAddress,
dst: pullAddress,
who: users.steven,
},
]);
// Finally, we verify that every reference has been accounted for.
it("has no unexpected references", () => {
const actualMentionsAuthorReferences = Array.from(
findMentionsAuthorReferences(exampleRelationalView())
);
for (const actualRef of actualMentionsAuthorReferences) {
expect(expectedMentionsAuthorReferences).toContainEqual(actualRef);
}
expect(actualMentionsAuthorReferences).toHaveLength(
expectedMentionsAuthorReferences.length
);
});
});
});