Summary:
This change is motivated by the behavior of loops, but applies more
generally to edges. Previously, a loop would induce two contributions
with the same contributor, but possibly different weights: one with the
to-weight of the edge and one with the fro-weight. For one, this is
annoying to downstream clients, who would like to use the contributor as
a superkey. But it is also somewhat strange that a single contributor
could have two different weights.
The same applies to edges in general: every edge induces two
contributions with the same contributor, of type `NEIGHBOR`.
As of this commit, we replace `NEIGHBOR` with `IN_EDGE` and `OUT_EDGE`,
one of each induced by each edge. This has the effect that a contributor
maps to at most one contribution.
Test Plan:
Existing unit tests updated.
wchargin-branch: separate-in-out-edge-contributions
Summary:
When we convert a graph to a Markov chain, each cell in the transition
matrix is a sum of edge weights from the given `src` to the given `dst`,
plus the synthetic self-loop needed for stability. Performing this sum
loses information: given the transition matrix, a client cannot
determine how much a particular edge contributed to the score of a node
without redoing the relevant computations. In this commit, we expose the
structure of these contributions (i.e., edges and synthetic loops).
This changes the API of `graphToMarkovChain.js`, but it does not change
the resulting Markov chains. It also does not change the API of
`pagerank.js`. In particular, clients of `pagerank.js` will not have
access to the contributions structure that we have just created.
Test Plan:
Existing unit tests have been updated to use the new API, and pass
without change. An additional test is added for a newly exposed
function, even though this function is also tested extensively as part
of later downstream tests.
In one snapshot, one value changes from `0.25` to `0.25 + 1.7e-16`. The
other values in the enclosing distribution do not change, so I think
that it is more likely that this is due to floating-point instability
than an actual bug. (I’m not sure where exactly I commuted or associated
an operation, but it’s quite possible that I may have done so). To
compensate, I added an additional check that the values in the
stationary distribution sum to `1.0` within `1e-9` tolerance; this check
passes.
wchargin-branch: expose-contributions
Previously, when expanding a node in the cred explorer, it would display
the neighboring nodes, but not any information about the edges linking
to that node. If the same node was reached by multiple edges, this
information was not communicated to the user.
As of this commit, it now concisely communicates what kind of edge was
connecting the chosen node to its adjacencies. There's a new `edgeVerb`
method that plugin adapters must implement, which gives a
direction-based verb descriptiong of the edge, e.g. "authors" or "is
authored by".
Test plan:
Unit tests added to the PagerankTable tests, and hand inspection.
Paired with @wchargin
Summary:
This commit adds sliders for each node and edge type (hard-coded for
now), and hooks them up to the cred explorer so that re-running PageRank
uses the newly induced edge evaluator.
Paired with @decentralion.
Test Plan:
We will add tests later. We promise! In the meantime, the results that
appear when you drag a slider and re-run PageRank seem appropriate. For
instance, changing the “Git blob” node type from `0.0` to `-10.0`
results in the Git blobs not dominating the whole view.
wchargin-branch: configurable-weights-ui
Summary:
PageRank wants an _edge evaluator_: a function mapping an edge to its
to-weight and fro-weight. This commit provides functions for creating
edge evaluators based on the high-level, coarse notions of node and edge
types. We use [the formulation described in a comment on #476][1].
Paired with @decentralion.
[1]: https://github.com/sourcecred/sourcecred/issues/476#issuecomment-402576435
Test Plan:
None. We will add tests later. We promise!
wchargin-branch: edge-weight-generators
Summary:
As we add sliders for adjusting the PageRank parameters, we trigger a
bunch of unneeded renders on `PagerankTable`. As `PagerankTable` is a
pure component, we can [mark it and its children as such][1] to see notable
performance improvements: the `Array.from` and `sort` in its `render`
method are showing up on the flamegraph.
[1]: https://reactjs.org/docs/react-api.html#reactpurecomponent
Paired with @decentralion.
Test Plan:
Unit tests pass, whereas if we instead implement `shouldComponentUpdate`
by `return false` then the interaction tests fail. Also, `yarn start`
seems to behave as expected as we switch among different graphs.
wchargin-branch: pure-pageranktable
Summary:
To verify that the correct graph is loaded, we display the graph’s node
and edge count in the UI. As previously implemented, this would be
recomputed on every change, requiring iteration over all nodes and edges
of the graph. On my machine (T440s) and the SourceCred graph, this
induced an ~80ms performance floor for any operations that caused the
app to re-render, which is noticeable.
This commit retains the useful information, but computes it only at
graph load time.
Paired with @decentralion.
Test Plan:
Note that the behavior is unchanged.
wchargin-branch: cache-graph-size
Rewrites the README to be a lot more concrete about what SourceCred is
doing, and to give more up-to-date information overall.
Test plan: The README includes a small code block with instructions for
turning on a local copy of the Cred Explorer. I ran this on my machine
(inside /tmp) and it worked locally.
Summary:
See #432. This commit switches the GitHub Markdown parser from matching
regular expressions against the raw post body to matching the same
regular expressions against the semantic text of a Markdown document.
See test cases for `parseReferences` and `parseMarkdown` for more
details.
There are no changes to snapshots or cached GitHub data because the
Markdown in the example repository is simple enough to be properly
parsed by a simple approach that treats everything as literal text.
The change to the “finds numeric references in a multiline string” test
case is to strip leading indentation from the string in question. As
previously written, the test had four spaces of leading indentation on
each line, causing the Markdown parser to interpret it as a code block,
in turn causing our logic to (correctly) omit it entirely. The revised
version of the test has no leading indentation.
Test Plan:
Light unit tests included; these tests are intended to verify that the
actual Markdown logic from `parseMarkdown` is being used, and that file
has more extensive tests. Additionally, `yarn travis --full` passes.
wchargin-branch: github-parse-markdown
Summary:
This commit exposes a function of type `(string): string[]` to
encapsulate the whole Markdown pipeline, from parsing to AST
transformation to text node extraction. Clients of this module do not
need to know about `commonmark`.
Test Plan:
A single comprehensive test case has been added.
wchargin-branch: text-blocks
Summary:
The regular expressions used to detect GitHub references were of the
form `/(?:\W|^)(things)(?:\W|$)/gm`, where the outer non-capturing
groups were intended to enforce a word boundary constraint. However,
this caused reference detection in strings like `"#1 #2"` to fail,
because the putative matches would be `"#1 "` and `" #2"`, but these
matches overlap, and the JavaScript RegExp API (like most such APIs)
finds only non-overlapping matches. Therefore, in a string of
space-separated references of the same kind, only every other reference
would be detected.
A solution is to use a positive lookahead instead of the second
non-capturing group: i.e., `/(?:\W|$)/` becomes `/(?=\W|$)/`. (Ideally,
the first non-capturing group would just be a lookbehind, but JavaScript
doesn’t support those.)
In some cases, using `\b` is a simpler solution. But this does not work
in all cases: for instance, it works for repo-numeric references, but
does not work for numeric references, because the presence of the hash
means that there cannot be an immediately preceding word boundary. For
consistency, I opted to use the lookahead solution in all cases, but any
solution that passes tests is okay with me.
Test Plan:
Regression tests added. They fail before this patch and pass with it.
wchargin-branch: fix-space-separated-github-references
Summary:
The `^` metacharacter only matches the start of a non-initial line when
the `m` (“multiline”) flag is set on the RegExp. This flag was set only
for the GitHub URL RegExp.
Test Plan:
Regression tests added. They fail before this patch and pass with it.
wchargin-branch: fix-sol-github-references
Test plan:
`git grep -i v3` only shows incidental hits in longer strings
`yarn travis --full` passes
`yarn backend` works
`yarn build` works
`yarn start` works
`node bin/sourcecred.js start` works
`node bin/sourcecred.js load sourcecred example-github` works
Paired with @wchargin
Summary:
The bridge introduced in #448 has now served its purpose, and may be
deconstructed. This implements the first part of the last step of the
plan described in that pull request.
Paired with @decentralion.
Test Plan:
After `yarn backend && yarn build`:
- `node bin/sourcecred.js start` works, and
- `yarn start` works, and
- `yarn travis --full` works.
wchargin-branch: demolish-bridge
Test plan:
`node bin/sourcecred.js load sourcecred example-github` works
`yarn start` works
`node bin/sourcecred.js start-v3` works
`yarn travis --full` passes
Paired with @wchargin
Summary:
This could also be moved into the bridge directory, but this way is
marginally easier, and it doesn’t really matter in the end.
Test Plan:
`yarn backend` followed by `node bin/sourcecredV3.js start-v3` works.
wchargin-branch: start-v3
The `load` command replaces `plugin-load`. By default, it loads data for
all plugins, and does so in parallel using execDependencyGraph. If
passed the optional `--plugin` flag, then it will load data just for
that plugin.
As an implementation detail, when loading all plugins, load calls itself
with the plugin flag set.
Usage:
`node bin/sourcecred.js load repoOwner repoName`
Test plan:
Tested by hand; I blew away my SourceCred directory and then loaded the
example-github repository.
This integrates the PageRank table from #466 into the v3 cred explorer
app, bringing the v3 frontend to better-than-parity with v1!
Test plan:
Some unit tests were included, and running `yarn start` and inspecting
the App reveals that it is working correctly. Loading a PageRank result
and then changing the repository no longer triggers a crash :).
Paired with @wchargin
Ports #265 to the v3 branch, along with some tweaks:
- Only display log score, and normalize them by adding 10 (so that most
are non-negative)
- Change colors to a soothing green
- Improve display, e.g. make overflowing node description text wrap
within the row
Implements most of the tests requested in #269
Test plan: Many unit tests added
Paired with @wchargin
Summary:
This enables plugins to specify different semantic types of nodes, along
with human-readable names. This will be used, for instance, in the cred
explorer, where users may filter to one of these node types.
Paired with @decentralion.
Test Plan:
Flow passes.
wchargin-branch: plugin-node-types
Summary:
This presents a human-readable name for a plugin. It’s not yet used
anywhere.
Paired with @decentralion.
Test Plan:
Flow passes.
wchargin-branch: plugin-name
Test plan:
Run the following commands:
```
node bin/sourcecredV3.js load-plugin-v3 sourcecred example-github --plugin=git
node bin/sourcecredV3.js load-plugin-v3 sourcecred example-github --plugin=github
yarn start
```
Then, navigate in-browser to the v3 cred explorer and load data for
`sourcecred/example-github`. The following messages are printed to
console:
```
GitHub: Loaded graph: 31 nodes, 73 edges.
Git: Loaded graph: 15 nodes, 19 edges.
Combined: Loaded graph: 44 nodes, 92 edges.
```
Paired with @wchargin
Summary:
This enables grabbing the GitHub relational view from disk and
converting it to a graph on the client.
Paired with @decentralion.
Test Plan:
For testing, information about the GitHub graph is printed when you
click the “Load data” button in the UI. Do so.
wchargin-branch: github-plugin-adapter
Summary:
Text input boxes for repository owner and name now appear. “Loading the
data” consists of logging the attempt to the console.
Test Plan:
Run `yarn start`, and note that the inputs are keyed against the same
local store key as their V1 equivalents. Note that clicking “Load data”
prints a message to the console.
Paired with @decentralion.
wchargin-branch: v3-load-data-ui
In GitHub, you can make cross repo references. For example,
sourcecred/sourcecred#459 is one such reference. This commit adds
support for detecting those references and adding them to the GitHub
graph.
Test plan:
See attached unit tests.
This commit adds `loadGitData`, which clones the git repository for a
given repo and saves the corresponding git graph. It also adds that
method to the `loadPlugin` command, so that the following command now
works:
```
$ node bin/sourcecredV3.js load-plugin-v3 sourcecred example-git --plugin=git
```
After running that command, the correct file is present:
```
$ du -sh tmp/sourcecred/data/sourcecred/example-git/git/graph.json
28K /home/dandelion/tmp/sourcecred/data/sourcecred/example-git/git/graph.json
```
The command takes:
| repository | time (s) |
:------------------------- | ----------:
| `sourcecred/example-git` | 1 |
| `sourcecred/sourcecred` | 5 |
| `ipfs/js-ipfs` | 18 |
| `ipfs/go-ipfs` | ∞ (OOM) |
Summary:
Pending the resolution of brigand/babel-plugin-flow-react-proptypes#201,
we’re removing this plugin from our build, because it results in
incorrect code generation. We’ll be happy to add it back if the bug is
fixed.
Test Plan:
Fingers crossed.
wchargin-branch: remove-bpfrpt
This module exposes a method, `pagerank`, which is a convenient entry
point for taking a `Graph` and returning a `PagerankResult`. This
obviates the need for `src/v1/app/credExplorer/basicPagerank.js`.
Test plan: Unit tests included.
I'm planning to make a `pagerank.js` module that is a clean entry point
for all the graph-pagerank-related code, so it will be cleaner to expose all
the default options there.
Test plan: travis
Paired with @wchargin
If `findStationaryDistribution` is passed `0` as `maxIterations`, then
it should return the initial distribution.
Test plan: see new unit test
Paired with @wchargin
This commit enables paired authorship on GitHub authored entities.
If the entity has the string "Paired with" in the body, followed by a
username reference, that entity will be recorded as having dual
authorship, with the nominal author and the paired-with author being
treated identically in the relational view and the graph.
If there's a need to pair with more than one author, the "Paired with"
signifier may be repeated. The regex matcher is forgiving of
capitalizing the P or W, and an optional colon may be added immediately
after the word "with".
Note that the code assumes that every `TextContentEntity` is also an
`AuthoredEntity`. If that changes, it will cause a type error and we'll
need to refine the code somewhat.
As implemented, it is impossible for the same user to author a post
multiple time; if this is textually suggested (e.g. by a paired-with
reference to the post's nominal author), the extra paired-with
references are silently ignored. Also, having a paired-with reference
suppresses the basic reference (although it is possible to have a post
that is paired with someone, and additionally references them).
Test plan:
Tests have been updated, and the behavior of the parser is extensively
tested. For an end-to-end demonstration, I've also added a unit test in
the relational view that verifies that sourcecred/example-github#10 has
two authors. You can also see that the graph snapshot has updated to
include additional authorship edges (and that corresponding reference
edges have disappeared).
Closes#218
Summary:
This implements Step 3 of the plan described in #448.
Test Plan:
Run `yarn start` and navigate to `/v3` (by clicking the nav link).
wchargin-branch: branch-v3
Summary:
The bridge now lets you select any version of the app that you want, as
long as that’s V1, because that’s the only version that exists. We’ll
add a V3 version shortly.
This implements Step 2 of the plan described in #448.
Test Plan:
In `yarn start` and `node bin/sourcecred.js start`, note that navigating
to `/` redirects to `/v1`, and that the cred explorer works.
wchargin-branch: bridge-select
Summary:
Our build system doesn’t make it easy to have two separate React
applications, which we would like to have for the V1 and V3 branches.
Instead, we’ll implement a bridge to maintain compatibility.
The plan looks like this:
1. Change the app from pointing to V1 to pointing to a bridge
2. Move the router into the bridge and move the V1 app from the `/`
route to the `/v1` route (e.g., `/v1/explorer`)
3. Add a V3 app under the `/v3` route
4. ???
5. Delete the V1 app and remove it from the bridge
6. Delete the bridge and move the V3 app from the `/v3` route to `/`
This commit implements Step 1.
Test Plan:
To verify that the bridge is in fact showing, apply
```diff
diff --git a/src/bridge/app/index.js b/src/bridge/app/index.js
index 379e289..72e784c 100644
--- a/src/bridge/app/index.js
+++ b/src/bridge/app/index.js
@@ -9,5 +9,11 @@ const root = document.getElementById("root");
if (root == null) {
throw new Error("Unable to find root element!");
}
-ReactDOM.render(<V1App />, root);
+ReactDOM.render(
+ <React.Fragment>
+ <h1>Hello</h1>
+ <V1App />
+ </React.Fragment>,
+ root
+);
registerServiceWorker();
```
and say “hello” back to the app.
wchargin-branch: bridge
Summary:
This provides a command-line entry point `load-plugin-v3` (which will
become `load-plugin` eventually), which fetches the GitHub data via
GraphQL and saves the resulting `RelationalStore` to disk.
A change to the Babel config is needed to prevent runtime errors of the
form `_callee7` is not defined, where `_callee7` is a gensym that is
appears exactly once in the source (in use position, not definition
position). I’m not sure exactly what is causing the error or why this
config change fixes it. But while this patch may be fragile, I don’t
think that it’s likely to subtly break anything, so I’m okay with
pushing it for now and dealing with any resulting breakage as it arises.
Paired with @decentralion.
Test Plan:
Run `yarn backend`, then run something like:
```
node bin/sourcecredV3.js load-plugin-v3 \
sourcecred example-github --plugin github
```
Inspect results in `SOURCECRED_DIR/data/OWNER/NAME/github/view.json`,
where `SOURCECRED_DIR` is `/tmp/sourcecred` by default, and `OWNER` and
`NAME` are the repository owner and name.
This example repository takes about 1.1 seconds to run. The SourceCred
repository takes about 45 seconds.
wchargin-branch: cli-load-plugin
Summary:
Due to oclif’s structure, this entry point shares its `commands`
directory with that of the V1 entry point. We’ll therefore add commands
like `start-v3` as we go.
Test Plan:
`yarn backend` works, and `node bin/sourcecredV3.js start` launches the
V1 server.
wchargin-branch: v3-cli
This adds additions and deletions to the v3 Pull data model, and also
uses them in the pull descriptions.
It's basically a port of #340 to v3.
Test plan: Snapshots
This adds methods for serializing the GitHub RelationalView.
We have not put in the work to ensure that these methods generate
canonical data. Getting the issues in a different order, or finding
references in a different order, can change the JSON output even if the
resulting repositories are equivalent.
@decentralion think it's not worth putting in the effort, since we may
switch to a SQL database soon anyway.
Test plan: travis
Paired with @wchargin
Now that we want to implement RelationalView de/serialization, we need a
way to construct one without adding data to it.
Now that we're allowing `addData` to be called explicitly, we also want
to make sure it's idempotent, which necessitated a small change to
reference handling. A new test verifies idempotency.
Test plan: travis
Paired with @wchargin
Summary:
This makes progress on #432. We’d like to look for GitHub references
only within each text node of the Markdown AST. But there are two
complications:
- Text nodes split across formatting, and it’s valid for someone to
write `*Paired* with @decentralion, but *tested* independently`, or
`**Closes** #12345`, or something.
- Sometimes contiguous blocks of text expand to multiple text nodes,
because of how CommonMark approaches smart punctuation. For
instance: the document `It's got "punctuation" and stuff!` has eight
text nodes ([demo][1]).
In this commit, we introduce functions `deformat` and `coalesceText` to
solve these problems. (They go together because `coalesceText` is useful
for testing `deformat`.)
[1]: https://spec.commonmark.org/dingus/?text=It%27s%20got%20%22punctuation%22%20and%20stuff!
wchargin-branch: markdown-deformat
Summary:
We plan to use this to more intelligently extract references from GitHub
text content. See #432.
Test Plan:
In a Node shell, running
```js
const cm = require("commonmark");
var parser = new cm.Parser();
var ast = parser.parse("Hello\nworld");
var html = new cm.HtmlRenderer({softbreak: " "}).render(ast);
console.log(html);
```
prints `<p>Hello world</p>`.
wchargin-branch: commonmark
This commit adds a `description` method that takes a GitHub entity, and
returns a description of that entity. Based on the work in #261.
In contrast to the implementation in #261:
- It won't crash on entities without an author (although we don't have a
test case for this; see #389).
- It handles multi-authors reasonably (although we can't test that, as
we haven't implemented multi-authorship yet; see #218).
Test plan:
Inspect snapshot to see some examples.
William Chargin
Dandelion Mané
