Currently, it's possible to lock the cred explorer UI via the following
sequence of actions:
1. Press the analyze cred button
2. While it is running, modify the weights
After following this repro, the UI is stuck: it will say "loading"
forever, and the analyze cred button is disabled.
The issue is that loadGraph and runPagerank do not apply their success
(or failure) state transitions if they are pre-empted by another state
change. If a repo change occurs, that's the right behavior: the repo
change puts the state back to `"READY_TO_LOAD_GRAPH"`, which means the
UI is ready to re-load, and showing the PageRank results for the wrong
repo would be very confusing.
However, if an edge evaluator change occurs while loadGraph or
runPagerank is happening, the state is left in the "LOADING" status
(which means the analyze cred button is disabled).
This commit fixes the issue via a refactor: per @wchargin's suggestion,
responsibility for the edge evaluator moves from the state module out to
`credExplorer/App.js`. This dramatically simplifies the state module, as
we no longer need a `Substate` concept: we can simplify the state into a
single sequence of states.
As of the refactor, the bug is impossible.
Test plan: Unit tests have been updated to maintain coverage on the
refactored code. I manually tested that the bug no longer repro's, and
that the cred explorer UI continues to function. I did not add a new
test to protect against regression, because in the new codepath, the bug
is basically impossible.
This commit modifies the edge type so that it has a
`defaultForwardWeight` and `defaultBackwardWeight`, and these defaults
are respected by the `WeightConfig`.
I came up with reasonable-seeming defaults for the Git and GitHub
plugins; these will undoubtably be more methodically tuned in the
future.
Test plan: `yarn test` passes, also opening the cred explorer now has
the specified default weights in the WeightConfig. (Note that the
forward/backward directions are reversed as described in #749.)
This commit introduces a new component, `EdgeTypeConfig`, which is
responsible for configuring the weights for a given edge type. The
config creates two `WeightSlider`s: one for the forward direction, and
one for the backward direction. The `DirectionalitySlider` is no longer
used, and is removed. This fixes#596.
So as to avoid confusion, we now describe every edge with variables, as
in 'α REFERENCES β', and clarify that the weight modifies how cred flows
from β to α. This necessitated the creation of an `EdgeWeightSlider`,
local to the `EdgeTypeConfig`, which sets up a `WeightSlider` with the
necessary greek characters.
The EdgeTypeConfig is tested, so this is continuing progress towards
solving #604.
Test plan: I manually verified that modifying edge weights has the
expected effect on cred scores. Also, some new unit tests are included.
This factors `NodeTypeConfig` out of the `WeightConfig` component. The
scope for a `NodeTypeConfig` is that it configures a single node type.
Right now it just renders a single `WeightSlider`, but I like factoring
out both for consistency with the `EdgeTypeConfig` (see #749) and
because I expect we may want to add more complexity later.
Test plan: The new component has some tests, also I manually tested the
frontend.
* StateTransitionMachine.loadGraph reports success
Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.
Test plan: Unit tests included.
* Add StateTransitionMachine.loadGraphAndRunPagerank
This methods combines `loadGraph` and `runPagerank` into one method
which internally chains the two method. `runPagerank` is only called if
`loadGraph` was successful.
Progress on #586.
Test plan:
The new method has attached unit tests. I implemented the unit tests via
mocking, which seemed quite convenient as the method is basically a
wrapper for chaining two other function calls.
* Combine loadGraph and runPagerank into one button
Resolves#586. The new button is called "Analyze cred".
Test plan: Unit tests, also I tested it manually.
* StateTransitionMachine.loadGraph reports success
Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.
Test plan: Unit tests included.
* Add StateTransitionMachine.loadGraphAndRunPagerank
This methods combines `loadGraph` and `runPagerank` into one method
which internally chains the two method. `runPagerank` is only called if
`loadGraph` was successful.
Progress on #586.
Test plan:
The new method has attached unit tests. I implemented the unit tests via
mocking, which seemed quite convenient as the method is basically a
wrapper for chaining two other function calls.
Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.
Test plan: Unit tests included.
Fixes#732; see that issue for context.
Test plan:
The success case still works (verified that loading
sourcecred/sourcecred works).
I haven't tested the error case, as getting a real RATE_LIMIT_EXCEEDED
form GitHub is time-consuming, and has only happened once in practice.
I'm pretty confident the code works because it's a simple adaptation of
the code that catches other cases.
Summary:
In #715, I used Bash arrays for convenience. Our tests should run under
POSIX `sh` (as on Travis and standard GNU/Linux). This patch
reimplements the check using only POSIX features.
Fixes#752.
Test Plan:
As is, `yarn test --full` passes on GNU/Linux and macOS(+GNU coreutils).
Change the glob from `main.*.js` to `*.js` and note that running the
test emits an error:
```
fatal: multiple main bundles found:
build_output/output_NO_REPOS/static/js/main.6307f660.js
build_output/output_NO_REPOS/static/js/ssr.e92af807.js
```
Change the glob from `main.*.js` to `nope.*.js` and note that running
the test emits an error:
```
fatal: no main bundle found
```
Revert the glob to normal and note that all tests run and pass.
(To run tests, `./test_build_static_site.t --chain-lint --long -v` from
the `sharness/` directory.)
wchargin-branch: posix-bundle-check
Test Plan:
Running `./test_build_static_site.t --long -v` no longer detects the
feedback URL as unsafe. (Prior to this commit, it emitted a message to
this effect.) The build is still broken on Linux for other reasons, but
works on macOS or any other system where `sh` resolves to Bash.
As a regression test, the “potentially unsafe argument” warning has been
made to actually fail the test case. To verify this, remove `:` from the
list of `unusual_chars`, run the test, and note that it fails outright.
wchargin-branch: shell-safe-colon
Test Plan:
Note that `yarn backend; node bin/sourcecred.js help` still works.
Note that `git grep -i oclif` returns no results.
Rejoice.
wchargin-branch: remove-oclif
Summary:
This commit changes the CLI to use the code in `cli` instead of `oclif`.
A subsequent commit will remove the dependency on OClif altogether.
Resolves#580.
Test Plan:
Note that `yarn backend; node bin/sourcecred.js help` works. Note that
the documentation in the README is still correct.
wchargin-branch: cli-replace-oclif
Summary:
This ports the OClif version of `sourcecred load` to the sane CLI
system. The functionality is similar, but the interface has been
changed a bit (mostly simplifications):
- The `SOURCECRED_GITHUB_TOKEN` can only be set by an environment
variable, not by a command-line argument. This is standard practice
because it is more secure: (a) other users on the same system can
see the full command line arguments, but not the environment
variables, and (b) it’s easier to accidentally leak a command line
(e.g., in CI) than a full environment.
- The `SOURCECRED_DIRECTORY` can only be set by an environment
variable, not by a command-line argument. This is mostly just to
simplify the interface, and also because we don’t really have a good
name for the argument: we had previously used `-d`, which is
unclear, but `--sourcecred-directory` is a bit redundant, while
`--directory` is vague and `--sourcecred-directory` is redundant.
This is an easy way out, but we can put the flag for this back in if
it becomes a problem.
- The `--max-old-space-size` argument has been removed in favor of a
fixed value. It’s unlikely that users should need to change it.
If we’re blowing an 8GB heap, we should try to not do that instead
of increasing the heap.
- Loading zero repositories, but specifying an output directory, is
now valid. This is the right thing to do, but OClif got in our way
in the previous implementation.
Test Plan:
Unit tests added, with full coverage; run `yarn unit`.
To try it out, run `yarn backend`, then `node bin/cli.js load --help` to
get started.
I also manually tested that the following invocations work (i.e., they
complete successfully, and `yarn start` shows good data):
- `load sourcecred/sourcecred`
- `load sourcecred/example-git{,hub} --output sourcecred/examples`
These work even when invoked from a different directory.
wchargin-branch: cli-load
Summary:
This includes environment variables to specify the SourceCred directory
and the GitHub token. Parts of this may change once #638 is resolved.
Test Plan:
Unit tests included, with full coverage; run `yarn unit`.
wchargin-branch: cli-common
Summary:
This commit includes a minimal usage of an actual CLI application. It
provides the `help` command and no actual functionality.
Test Plan:
Unit tests added, with full coverage. To see it in action, first run
`yarn backend`, then run `node bin/cli.js help`.
wchargin-branch: cli-beginnings
Summary:
This commit introduces the notion of a `Command`, which is simply a
function that takes command-line arguments and interacts with the real
world. This infrastructure will enable us to write a well-tested CLI.
The `Command` interface is asynchronous because commands like `load`
need to block on promise resolution (for loading GitHub and Git data).
This is annoying for testing, but does not actually appear to be a
problem in practice.
Test Plan:
Unit tests added. See later commits for real-world usage.
wchargin-branch: cli-command-infrastructure
Summary:
Per #580, we aim to remove OClif. To do so, we move the old system to a
directory `oclif`, and will create the new system in the now-vacant
`cli` directory.
Test Plan:
Note that `yarn backend` still builds, that `node bin/sourcecred.js`
still has `help` and `load`, and that `git grep -wc cli` yields only
`yarn.lock:9`.
wchargin-branch: rename-cli-to-oclif
Our serialized RelationalView can get quite large - in the case of
TensorFlow it's over 190MB. This is a problem, as GitHub pages have a
hard cap of 100MB on hosted files.
As a temporary workaround, this commit introduces a method,
`compressByRemovingBody`, which strips away the bodies of every post. In
the longer term, we'll need a solution that scales with larger
repositories, e.g. sharding the relational view into smaller pieces.
Test plan: Unit tests were added. I've manually confirmed that the
newly-generated views are smaller (2.1MB vs 3.3MB), and that the
frontend continues to function.
Summary:
We store the relational view in `view.json.gz` instead of `view.json`,
taking advantage of the isomorphic `pako` library for gzip encoding and
decoding.
Sample space savings (note that post bodies are included; i.e., #747 has
not been applied):
SAVE OLD (B) NEW (B) REPO
89.7% 25326 2617 sourcecred/example-github
82.9% 3257576 555948 sourcecred/sourcecred
85.2% 11287621 1665884 ipfs/js-ipfs
88.0% 20953425 2520358 gitcoinco/web
84.4% 38196825 5951459 ipfs/go-ipfs
84.9% 205770642 31101452 tensorflow/tensorflow
<details>
<summary>Script to generate space savings output</summary>
```shell
savings() {
printf '% 7s % 11s % 11s %s\n' 'SAVE' 'OLD (B)' 'NEW (B)' 'REPO'
for repo; do
file="${SOURCECRED_DIRECTORY}/data/${repo}/github/view.json.gz"
if ! [ -f "${file}" ]; then
printf >&2 'warn: no such file %s\n' "${file}"
continue
fi
script="$(sed -e 's/^ *//' <<EOF
repo = '${repo}'
pre_size = $(<"${file}" gzip -dc | wc -c)
post_size = $(<"${file}" wc -c)
percentage = '%0.1f%%' % (100 * (1 - post_size / pre_size))
p = '% 7s % 11d % 11d %s' % (percentage, pre_size, post_size, repo)
print(p)
EOF
)"
python3 -c "${script}"
done
}
```
</details>
Closes#750.
Test Plan:
Comparing the raw old version with the decompressed new version shows
that they are identical:
```
$ <~/tmp/sourcecred/data/sourcecred/example-github/github/view.json \
> shasum -a 256 -
63853b9d3f918274aafacf5198787e18185a61b9c95faf640a1e61f5d11fa19f -
$ <~/tmp/sourcecred/data/sourcecred/example-github/github/view.json.gz \
> gzip -dc | shasum -a 256
63853b9d3f918274aafacf5198787e18185a61b9c95faf640a1e61f5d11fa19f -
```
Additionally, `yarn test --full` passes, and `yarn start` still loads
data and runs PageRank properly.
wchargin-branch: gzip-relational-view
Summary:
This is a follow-up to #746, wherein we exposed our fixed `env` to the
backend applications. We now extend that environment so that it can also
access the user’s runtime environment—i.e., the native values of
`process.env`.
(This is in contrast to the frontend bundles `main.js` and especially
`ssr.js`, where this is not and should not be the case: the environment
must be fixed at build time.)
Test Plan:
Add to the top of `async run()` in `src/cli/commands/load.js`:
```js
console.log(require("../../app/version").VERSION_SHORT);
console.log(process.env.AT_RUNTIME);
```
Run `yarn backend` and `AT_RUNTIME=wat node bin/sourcecred.js load`.
Ensure that the version number and the string `wat` are both printed.
(Before this patch, the string `undefined` would be printed instead of
`wat`.)
wchargin-branch: backend-extensible-env
The GitHub regex in urlIdParse.js incorrectly disallowed repo names with
underscores and dots. Fixes#721.
To mitigate errors like this in the future, code which uses regexes to
find owners and repos has been modified to all depend on the same regex
pattern.
Test plan:
Unit tests have been updated to include the failure case (they correctly
failed), and then code was updated so that the tests pass again.
Also, I manually verified that loading ipfs/js.ipfs.io no longer fails.
Paired with @wchargin
Test Plan:
Add `console.log(require("../../app/version").VERSION_SHORT);` to the
top of `async run()` in `src/cli/commands/load.js`. Run `yarn backend`
and `node bin/sourcecred.js load`, and note that it prints the current
version number. Before this change, it would have raised an error:
```
Error: gitState: not a string: undefined
at parseGitState (~/git/sourcecred/bin/commands/load.js:1160:64)
```
because the requisite environment variables were not included.
Also, `yarn test --full` passes.
wchargin-branch: backend-env
This commit isolates all of the log-weight behavior in the weight
slider. That slider moves in log space, but the numbers printed and
passed around the WeightConfig code are now always in linear-space.
This should reduce confusion in the UI and for developers.
This commit contains two other improvements: (#588)
- Changes the (log space) range on the sliders from ±10 to ±5
- Change the order from slider, weight, name to name, slider, weight, so
that there is more visual separation between the name and the weight.
Test plan: Changes to the weight slider are tested. Changes to the
WeightConfig aren't (#604) so I manually tested the UI.
PluginAdapters and Node/Edge types are increasingly fundamental to the
cred explorer. Prior to this commit, we had no canonical demo
adapters/types, and we would create ad-hoc and messy adapters whenever
we needed them. This creates unnecessary repetition and lowers test
quality.
This commit creates a canonical demo adapter (loosely themed based on
the wonderful game [Factorio]) and refactors most existing test cases to
use the demo adapters. In particular, the horrible mess of pagerankTable
adapters has been removed.
[Factorio]: https://www.factorio.com/
I left `aggregate.test.js` untouched because I would have needed to
materially re-write the tests to port them over. I added a comment so
that if we ever do re-write those tests, we'll use the new demo
adapters.
Test plan: `yarn test` passes.
This commit factors the weight sliders used for both node and edge
weights into a shared WeightSlider component, and factors out the
direction slider used for edge weights into a DirectionalitySlider.
Both of these components are tested. This is a step towards #604.
Test plan:
The specific behaviors of the sliders are well tested. Since the weight
config as a whole is not tested, I manually verified by messing with the
weights that node weights, edge weights, and edge directionality all
affects the cred distribution as anticipated.
Summary:
We currently load trees and then throw them away later, because we don’t
get useful signal from them. We should consider not doing that. This
will be faster.
Test Plan:
```
$ time node bin/sourcecred.js load tensorflow/tensorflow --plugin git
real 0m33.512s
user 0m35.196s
sys 0m12.489s
```
Also, `yarn test --full` passes.
wchargin-branch: git-deforestation
Adds a link titled "what is this?" that points to my gentle introduction
to cred. Also, move the feedback link to be next to it and get rid of
the prototype disclaimer.
Test plan: Visual inspection, also a test was updated.
Summary:
This fixes a bug where, if the `SOURCECRED_DIRECTORY` environment
variable is set to `foo` but the `-d bar` flag is passed, then the
repository registry will be written under `foo` but the plugin data will
be loaded under `bar`.
Test Plan:
```
$ rm -rf /tmp/good /tmp/bad
$ SOURCECRED_DIRECTORY=/tmp/bad >/dev/null \
> node bin/sourcecred.js load sourcecred/example-github -d /tmp/good
$ [ -d /tmp/bad ]; echo $?
$ find /tmp/good
/tmp/good
/tmp/good/cache
/tmp/good/cache/sourcecred
/tmp/good/cache/sourcecred/example-github
/tmp/good/cache/sourcecred/example-github/github
/tmp/good/cache/sourcecred/example-github/git
/tmp/good/repositoryRegistry.json
/tmp/good/data
/tmp/good/data/sourcecred
/tmp/good/data/sourcecred/example-github
/tmp/good/data/sourcecred/example-github/github
/tmp/good/data/sourcecred/example-github/github/view.json
/tmp/good/data/sourcecred/example-github/git
/tmp/good/data/sourcecred/example-github/git/graph.json
```
wchargin-branch: load-pass-context
Adds a badge, adds a link, slight rewrite of the contributing section.
Test plan: Check the links and badge works.
(The badge topic count is off, we'll see if it is at least directionally
correct over time.)
Summary:
This improves the documentation for the change made in #715.
Test Plan:
Note that `./scripts/build_static_site.sh --help` and `yarn sharness`
pass.
wchargin-branch: feedback-url-help-text
Fixes#696.
Test plan: This is basically a config change, so I manually tested it.
I ran SourceCred on gitcoinco/web, which has two bots,
and verified that the bots are correctly removed from the list of users.
Selecting "Bots" in the dropdown filter shows the two bots. Changing
the user weight does not affect the bots' scores, and changing the bot
weight does affect the bots' scores.
Summary:
We can now set, at build time, a URL to be displayed at the top of the
prototype, encouraging users to provide feedback. If the URL is not
provided, it defaults to the appropriate topic on the SourceCred
Discourse instance.
The result looks like this:
![Screenshot of the feedback URL in the prototype][screenshot]
[screenshot]: https://user-images.githubusercontent.com/4317806/44814824-a238b380-ab92-11e8-88c8-dfbae27ca496.png
Test Plan:
Unit tests added to `yarn sharness-full` and `yarn unit`.
You can run `yarn start` to see the message with the default URL, or
`SOURCECRED_FEEDBACK_URL=http://example.com/ yarn start` to specify a
custom URL.
wchargin-branch: feedback-url
This commit adds a hardcoded list of known bots. Building on #713, it
categorizes those userlikes with the bot subtype. (Note that those users
may not be bots in the GitHub ontology - GitHub doesn't actually have a
clear record of which userlikes are bots.)
Progress towards #696.
Test plan:
Observe the single snapshot change, which demonstrates that @credbot is
now correctly categorized as a bot.
Summary:
As a first pass toward support for analyzing whole organizations, we
allow loading multiple repositories with `sourcecred load`, combining
them into a single relational view and a single Git graph at load time.
Test Plan:
Run
```
node bin/sourcecred.js \
load \
sourcecred/example-git \
sourcecred/example-github \
sourcecred/sourcecred \
--output sourcecred/examples \
;
```
and select `sourcecred/examples` from the web view. Filter “Repository”
nodes, and note that there are three.
Note that loading a single repository without `--output` still works,
that loading a single repository with `--output` still works (respecting
the alias name), and loading not exactly one repository without
`--output` yields an appropriate error message.
Note that `yarn sharness-full` still works.
wchargin-branch: load-combined
Userlikes now have an additional piece of data encoded in their address:
whether they are a USER or a BOT. Userlikes are still handled
identically by the RelationalView, which cuts down on code duplication.
I haven't added ORGANIZATIONs but it will be trivial to do once we're
interested in tracking them.
Note that this is basically the same as how we treat comments: comments
are subtyped to review comments, issue comments, and pull comments.
This is the initial step towards solving #696.
Test plan: Existing unit tests pass (and caught a few bugs during
development!). New test cases were added to the parser. Observe that all
the snapshot changes make sense.
Note: As of this commit, every GitHub userlike is classified as a user,
and the subtypes are not used in the application, so this commit causes
no change in observable behavior.
This commit changes the cred normalization algorithm so that the total
cred of all GitHub user nodes always sums to 1000. For rationale on the
change, see #705.
Fixes#705.
Note that this introduces a new way for PageRank to fail: if the
graph has no GitHub userlike nodes, then PageRank will throw an error
when it attempts to normalize. This will result in a message being
displayed to the user, and a more helpful error being printed to
console. If we need the cred explorer to display graphs that have no
userlike nodes, then we can modify the codepath so that it falls back to
normalizing based on all nodes instead of on the GitHub userlike nodes
specifically.
Test plan: There is an included unit test which verifies that the
new argument gets threaded through the state properly. But this is
mostly a config change, so it's best tested by actually inspecting
the cred explorer. I have done so, and can verify that the behavior is
as expected: the sum of users' cred now sums to 1000, and e.g. modifying
the weight on the repository node doesn't produce drastic changes to
cred scores.
This commit adds the logic for computing scores so that the total score,
summed across all nodes matching a NodePrefix, is a fixed constant.
See #705 for context.
Test plan: The logic is quite simple, and adequate unit tests are
included.
Note to reviewer: There is a spurious whitespace diff in the test file
because the tests for the previous test block were not correctly scoped.
Storing the user's weights in localStore enables a workflow where a
user chooses their preferred weights, and brings those weights with them
across projects and contexts. However, this is the wrong workflow:
actually, a project chooses its weights, and when a user visits a
particular project, they want to sync up with the project's choice.
Giving the user the ability to modify the weights and recalculate is
still important, so that they can propose improvements to the project
maintainer. But implicitly keeping their modified weights, and even
bringing them to other projects the user inspects, is
counter-productive.
This commit removes this dubious feature. (It's a feature we were likely
to drop anyway, as it conflicts with #703.) As an added bonus, this code
is untested, which means the feature is technical debt—so removing it
reduces our technical debt! It also removes at least one known bug.
Test plan: There are no tests. I manually verified that the frontend
still works, and that it no longer persists weights across refresh.
Summary:
By using Git’s magic pathspecs instead of post-processing stream
operations, we reduce the pipeline to a single operation. Git implements
its own version of `grep`, so this should be platform-independent.
Previously, we had needed the `-z` argument to `grep(1)`, which is a GNU
extension.
Fixes#594.
Test Plan:
Ensure that the script passes with no output. Then,
```shell
mkdir -p src/flow-typed/
touch src/foo.js src/flow-typed.js src/flow-typed/foo.js
git add src/foo.js src/flow-typed.js src/flow-typed/foo.js
cd scripts
./ensure-flow.js
```
and ensure that script exits with code 1, empty stdout, and stderr with:
```
../src/flow-typed.js
../src/flow-typed/foo.js
../src/foo.js
```
This verifies that the pathspec is properly excluding the root directory
`flow-typed`, but not files that just happen to have `flow-typed` in
their paths.
wchargin-branch: simplify-ensure-flow
Summary:
This patch adds independent exponential backoff to each individual
GitHub GraphQL query. We remove the fixed `GITHUB_DELAY_MS` delay before
each query in favor of this solution, which requires no additional
configuration (thus resolving a TODO in the process).
We use the NPM module `retry` with its default settings: namely, a
maximum of 10 retries with factor-2 backoff starting at 1000ms.
Empirically, it seems very unlikely that we should require much more
than 2 retries for a query. (See Test Plan for more details.)
This is both a short-term unblocker and a good kind of thing to have in
the long term.
Test Plan:
Note that `yarn test --full` passes, including `fetchGithubRepoTest.sh`.
Consider manual testing as follows.
Add `console.info` statements in `retryGithubFetch`, then load a large
repository like TensorFlow, and observe the output:
```shell
$ node bin/sourcecred.js load --plugin github tensorflow/tensorflow 2>&1 | ts -s '%.s'
0.252566 Fetching repo...
0.258422 Trying...
5.203014 Trying...
[snip]
1244.521197 Trying...
1254.848044 Will retry (n=1)...
1260.893334 Trying...
1271.547368 Trying...
1282.094735 Will retry (n=1)...
1283.349192 Will retry (n=2)...
1289.188728 Trying...
[snip]
1741.026869 Ensuring no more pages...
1742.139978 Creating view...
1752.023697 Stringifying...
1754.697116 Writing...
1754.697772 Done.
```
This took just under half an hour, with 264 queries total, of which:
- 225 queries required 0 retries;
- 38 queries required exactly 1 retry;
- 1 query required exactly 2 retries; and
- 0 queries required 3 or more retries.
wchargin-branch: github-backoff
Dandelion Mané
William Chargin
Claire L
