Summary:
The internal `UpdateResult` structure now lists IDs of objects whose
typename has been queried. This list is expected to be empty for now.
Test Plan:
Unit tests added.
wchargin-branch: mirror-typename-updateresult
Summary:
The internal `QueryPlan` structure now lists IDs of objects whose
typename is to be queried. This list is expected to be empty for now.
Test Plan:
Unit tests included.
wchargin-branch: mirror-typename-queryplan
Summary:
Fields in a GraphQL schema may now declare themselves as “unfaithful”,
indicating that the server is known to return responses with incorrect
types for that field. Future changes will teach the Mirror module to
query these fields more carefully, handling such incorrect responses
robustly. See the [tracking issue] and [project initiative] for details.
[project initiative]: https://discourse.sourcecred.io/t/graphql-mirror-fidelity-awareness/275
[tracking issue]: https://github.com/sourcecred/sourcecred/issues/998
This change is source-compatible and data-incompatible: the APIs are
backward-compatible, but the schema representation (JSON serialized
form) has changed, and so the `Mirror` constructor will reject old
caches.
Test Plan:
Unit tests included.
wchargin-branch: schema-fidelity
Using a required type of before and after completion weight is a simple
way to start minting Cred on Initiatives. It sets expectations by having
both states defined in a version controlled file.
Eventually all plugins are expected to use the ReferenceDetector. This
commit composes the Github and Discourse detectors we can create, for now
exposing it as unused to `createPluginGraphs`.
Adding parameters to Project.
Assumes we're loading an InitiativesDirectory. We're not including the local path here, as this is environment dependent. It should be passed as an ENV or CLI parameter instead.
Summary:
Because `fromObject` does not mutate its input, it’s safe to accept
read-only inputs. This is needed for parts of the CredRank Markov
process graph implementation.
Test Plan:
Typing test added.
wchargin-branch: maputil-readonly-input
Summary:
Most of the blacklisted reactions helpfully link to their original
source, but some don’t. This patch adds the missing links.
Test Plan:
The following command now prints a URL on every line:
```
<src/plugins/github/blacklistedObjectIds.js \
awk '/^[^ ]/ { p = 0 }; p { gsub(".*// ", ""); print }; /reactions/ { p = 1 }'
```
wchargin-branch: blacklist-urls
This commit makes several small improvements to the distributionToCred
module:
- We rename the output `FullTimelineCred` data structure to
`TimelineCredScores`, which is more descriptive
- We re-organize that data structure so that rather than being an array
of `{interval, cred}` objects, it has an `intervals` property and a
`intervalCredScores` property, both of which are arrays. This will make
downstream usage cleaner.
- An unused variable is removed.
- We document invariants about the TimelineCredScores data type.
- We mark the TimelineCredScores data type opaque, so that clients
recieving a TimelineCredScores can trust that the invariants are
maintained.
Test plan:
- The rename is robustly tested by `yarn flow`.
- That the refactor lands without changing existing semantics is
robustly tested by `yarn test --full`, since we snapshot a full cred
load; thus we know that cred scores haven't changed. (Also, we have
existing unit tests).
- The newly documented invariants aren't robustly tested by the test
code, but it's easy to see that they hold by reading the algorithm.
Helper functions intended to be used in succession by loadDirectory.
Only `_validateUrl` provides helfpul error messages. It's the caller's
responsiblity to do this first.
Helper functions intended to be used in succession by `loadDirectory`.
Only `_validatePath` provides helfpul error messages. It's the caller's
responsiblity to do this first.
Introduces dependency `globby` for globbing with a Promises API.
The private function `_initiativeFileId` will be used as a helper to
load a directory. The public function `initiativeFileURL` will be used
to add links to the remote file in the node description.
This commit modifies the api declaration file so that it includes
`core.weights` and `core.weightedGraph`. These are both important
modules for interfacing with Cred (loading a WeightedGraph, and defining
custom weights), so it's natural for them to be included in the API.
Test plan: Change is a trivial extension of the pattern in api.js. `yarn
flow` passes, and I believe no further testing is _required_; however, I
will use this in an Observable notebook prior to merging.
Based on [forum discussion][1], Initiatives should be tracked in files.
The main issue with storing the existing Initiative type as JSON in a
file, is there's no natural NodeAddress or URL for a file-based tracker.
This type resolves that by using the file name within a directory as a
unique reference and requiring a remoteUrl for referencing. (See #1640)
[1]: https://discourse.sourcecred.io/t/576
"Trackers" were an idea to let Initiatives be aware of the medium that
declares it. Such as a Discourse topic or file.
With Discourse in mind this was really useful. We could add an automatic
contribution edge, enhance reference detection to "upgrade" a URL pointing
to that Topic to resolve to the Initiative instead, etc.
Using files as the only source of Initiatives this becomes less relevant.
So in the interest of reducing complexity, we'll remove tracker awareness.
Previously we were relying on the `Initiative.tracker` to define the
address of an Initiative. Based on feedback at #1643 we want to remove
trackers. So we'll need a replacement ID.
This will enforce a new InitiativeId convention. As well as how to derive
a NodeAddressT from it. In a follow-up we'll remove the tracker concept.
Currently similar code to read/write Compatible JSON files is copy
pasted across the code. This takes some common practices and provides
a generic utility for it.
Correct Flow type usage can't be detected if the JSON type is opaque
though. GraphJSON is an example of this, so removed the opaque for a
smoke test.
There was a fair amount of copy-pasted lines in these tests. Which is mostly a
good thing, because ESlint and Flow provide good errors when you're using the
variables wrong. But in terms of readability wasn't great.
In upcoming PRs we'll add more to these test files. So I thought it was good to
improve this first.
This version:
- Still copy-pastes to get good ESlint and Flow errors.
- Doesn't repeat itself when it's not for better errors or readability.
- Uses a "spyBuilder", for readability in spite of prettier trying to collapse lines.
- Makes sure duplicates are exactly duplicates, easier to edit in IDEs.
example: `[githubDeclaration, "githubDeclaration"]`
instead of `[fakeGithubDec, "fake-github-dec"]`
Test plan: `yarn test`
Reviewer note: recommend looking at the split diff on Github, not the unified one.
Summary:
Upcoming changes will add support for field-level fidelity annotations
(see #998), at which point the `Reaction.user` field will be marked
unfaithful. This patch will surface that behavior change.
Test Plan:
The newly snapshotted query is valid, and returns a reaction whose
`user` property lists typename and ID.
wchargin-branch: mirror-snapshot-reaction
This commit moves a lot of code and algorithms for computing timeline
cred scores into `core/algorithm`. The `TimelineCred` module hasn't been
moved, because it isn't clean enough for core -- it has dependencies on
analysis and types, for example.
This is another material step towards consolidating all of the
SourceCred algorithm logic into `core/algorithm`, although there's still
more to be done.
Test plan: It's just a code reorg; `yarn test` is sufficient.
This commit modifies the timelinePagerank module so that it no longer
takes in node/edge types. Instead, the timelinePagerank just takes a
WeightedGraph and uses weights from that WeightedGraph. This is a key
part of decoupling the core cred computation logic from the plugin
logic, as described in #1557.
I also modified the timelinePagerank module's immediate dependencies
(the weightEvaluator module) to do the same. Since the weight evaluators
now have a simpler contract (no overriding, etc), the unit tests have
been simplified.
Test plan: It's a simple refactor, so `yarn test` should be sufficient.
As a bit of added caution, I manually tested changing weights in the
frontend, and verified that cred updates as expected.
This was the last usage of strings as tokens. Other than the edges of
the system, like the cli and bin code which read the arguments.
Meaning the tokens should now always be validated.
Closes#1626
As part of my cleanup to make it easy to document and re-implement the
SourceCred algorithm, I want a place in core where we can consolidate
the js implementation. I'm renaming `core/attribution` to
`core/algorithm` to make this clearer.
Test plan: It's just a rename. `yarn test` passing is sufficient to
assure us of correctness.
TimelineCred has a `reduceSize` method which discards cred for most
nodes, keeping only cred for the top nodes of each type across all time.
I've wanted to remove this for a while, because it is a bad fit for the
kind of experimentation we're starting to do with showing the top nodes
for recent activity periods. Since the recent nodes haven't had much
time to accumulate cred, they are almost all discarded by reduceSize. As
an added inducement, I want to get rid of reduceSize for #1557 because
it requires type information.
`reduceSize` still serves one function, which is enabling the frontend
to load faster because it only loads a smaller amount of data which is
discoverable in the UI. However, it doesn't make sense to discard most
of the data just for a fast UI load--we can later make another data
structure which is tuned for the needs of the frontend, and have that
data structure include only summary statistics.
This will make the cred.json file much larger for large repos, e.g. I
expect that loading tensorflow/tensorflow would now go over the 100MB
hard cap for GitHub pages. However, right now we're prioritizing using
SourceCred for medium-small projects (e.g. SourceCred itself, and maybe
Maker), so this isn't a current concern.
Test plan: `yarn test --full` passes.
This commit fixes an issue introduced in #1625, which caused the
`state.test.js` file to print some unhandled console errors when
running `yarn unit`.
First, this commit changes the file so that it properly errors if any
unexpected console errors are printed. Then it fixes the erroring tests.
Test plan: `yarn test` passes; `yarn unit --watch legacy/state.test.js`
no longer prints any error messages to console.
One of several cleanup commits. See #1629.
The previously created loadContext (see #1622) function will be used
instead and renamed to be load.
We're removing significant amounts of test code as well. The individual
components that make up the replacement function already provide the
needed coverage, like `dataDirectory.test.js` and `loadContext.test.js`.
The integration provided by this function is also covered through the
sharness `test_load_example_github.t`.
Follows the general outline of #1586.
It uses a new trick of aliasing external module functions as
private properties. This makes the spyOn / mock tests more robust,
while fitting in the composition responsibility.
Note: this doesn't include a WeightedGraph.overrideWeights step.
Because overriding weights isn't related to plugins, this will be
handled as a separate feature, later in the load pipeline.
Similar to CachedProject, we're using an opaque PluginGraphs return
type. Because only PluginLoaders can add the semantic of this being
all plugin graphs, and to use this semantic in future functions.
Note, the return type is a CachedProject. See #1586 for discussion.
Having this type allows us to create new functions with a semantic
of requiring the project is mirrored into cache. It is opaque,
because only the "all plugins" semantic which PluginsLoaders has
could know when mirroring of a Project has been completed.
Additionally MirrorEnv is not a strict type. We're expecting this
to be a subset of parameters. We'll use Flow to ensure we only use
the ones we need from it.
This commit modifies the frontend so that it now pulls plugin
declarations from disk, rather than from the TimelineCred. This will
allow us to decouple the TimelineCred from the PluginDeclarations, which
is another step towards #1557.
As proof that the frontend no longer gets plugins from the TimelineCred,
I removed the public `plugins()` method on TimelineCred.
Test plan: The frontend is somewhat sketchily tested. `yarn test`
passing is good, manual inspection of the frontend is also necessary;
I've done this.
This builds on #1623 and is another step towards separating cred
computation from plugin declarations, as described in #1557. Basically,
this will allow the frontend to get plugin declarations even if the
TimelineCred computation never saw them.
This commit modifies `api/load`, and adds a new facility to
`DataDirectory` for saving the PluginDeclarations (which will be used by
@Beanow's in-flight refactor of `api/load`).
Test plan: See included unit tests, also try loading a project and
inspect the newlys saved file.
Similar to PluginLoaders, this accepts an interface like
TimelineCred.compute. During a load, we have the added
responsiblity of using the TaskReporter.
This lets us mock the concrete TimelineCred.compute, while testing
just the extra functionality. The responsiblity to compose this
with the concrete TimelineCred.compute lies with LoadContext.
Updating the mirror and using the mirror data should be separated.
As unified reference detection requires the mirror of all plugins are
updated. Upcoming refactors to the load system will solidify this.
While this refactor is ongoing, we will ignore the fetchGithubRepo
return value, using it as a mirror update only. And use this new
function to extract the data as needed in other loading steps.
Summary:
The `options` argument was introduced during the EAV table refactor and
dropped once that was complete, so we can remove the docs.
Test Plan:
None.
wchargin-branch: mirror-extract-no-options
Summary:
Previously, `_nontransactionallyRegisterObject` differed from its
counterpart `registerObject` in two ways: the former does not enter a
transaction, but it also requires a function to format an error message.
The value provided by `registerObject` to the helper is a suitable
default, so we can use it to decouple the transaction semantics from the
error message.
Test Plan:
Existing tests suffice, retaining full coverage.
wchargin-branch: mirror-default-typename-message
This commit adds a simple method for (de-)serializing arrays of
PluginDeclarations. This will allow us to save PluginDeclarations for
consumption by the frontend, without having them bundled with cred
results in TimelineCred. Thus, we can simplify and clean up TimelineCred
as described in #1557.
Test plan: Inspect unit tests and snapshots; `yarn test` passes.
This is the first of several commits to create the PluginLoaders
abstraction. Using this allows us to define "for all plugins"
semantics, while keeping each underlying plugin interface flexible.
Delegating to a CacheProvider instance, will limit the number
of places where we need to handle filesystem details. It will
also allow a mock, or in-memory cache to be provided.
