Overwritting someone elses reviews.

[dpc]

Sometimes people give a negative review to a crate, and we conciously would like to overwrite them.

Current example. Me and other crev user reviewed smallvec and gave it a negative review, because it was ridden with unsafe and had a history of unsoundness. Then, I submitted a fuzzer to smallvec, and I think with a fuzzer we can have more confidence, and this crate does not deserve a negative rating anymore. At very least I don't want it to fail in my own crate verify output.

So I'd like to overwrite the other persons (which I still trust) review in this case. I could ask them to change their review, but that's not the point.

I am thinking, that a review could have an optional overrule/overwrite/override section with a list of reviews of other other people that should be ignored for some reason, because this review says they no longer apply, were a mistake, etc.

We could identify other reviews by:

• by id of their author
• digest
• signature

The id is the longest, but is the easiest for the humans to understand and use, so I think I'll stick with it.

For other users my override would only apply if they trust me more than then a person I'm trying to override here (more or equal maybe?).

Should be fairly easy to implement, though I could use some second opinion.

[programmerjake]

maybe also have the overriding condition be dependent on if the overridden reviewer trusts the overrider? We want to avoid override wars, so, unless special circumstances apply (very high level of trust, trust relationship between overrider and overridee, etc.), I don't think overrides should apply for cases of equal trust.

[programmerjake]

also would be nice to have local overrides that aren't published.

[BatmanAoD]

Who would be affected by an override/overwrite?

"Overwriting" sounds like changing someone else's review globally, which surely isn't what you mean. "Override" sounds like it could somehow affect the trust status of the review globally (i.e. even for users who don't have a trust relationship with the overrider), but that the review itself would still exist.

Conversely, "ignore" sounds like it would not affect anyone else, even if they have a trust relationship with the user doing the ignoring.

I'm guessing you mean something between these two extremes, i.e., ignoring a review would break transitivity, so that users who trust your reviews would not automatically have any trust in that specific review. Maybe "hide" would be a better term?

Another way of thinking about this is that you can trust someone, but disagree with them on a certain issue. In this case, you trust the reviewer in general (and used to agree), but believe the situation has changed and no longer agree with their (or your old) assessment. So this sounds like a mechanism for more granular levels of trust. Maybe it should be possible to just set trust levels for particular reviews directly, which would handle this case as well as the opposite (where you agree with a specific review but don't trust the reviewer in general).

[dpc]

@BatmanAoD

Another way of thinking about this is that you can trust someone, but disagree with them on a certain issue. In this case, you trust the reviewer in general (and used to agree), but believe the situation has changed and no longer agree with their (or your old) assessment.

This is exactly the motivation. I still trust the other reviewer (otherwise I would just change their trust level to none and be done with it), but eg. can not wait for them to re-review, reconsider, or just in this one case / in my case I want an exception.

"Overwriting" sounds like changing someone else's review globally,

Right now I think about "just ignore this one particular review".

We want to avoid override wars, so

I am not really concerned about overriding wars. If people have deep disagreements that they can't resolve, they can just stop trusting each other, and possibly other people should too. :D . This feature is purely for tactical, one time exceptions from otherwise mutually respecting parties.

@programmerjake

also would be nice to have local overrides that aren't published.

Good idea. I wonder if it should be "don't publish" or just "don't get into effect for other people".

Also, you just reminded me that I'd like this ignore (I think it's more of an ignore than override), to apply only to a specific version of a review. If the ignored author redid their review, it would require redoing current review to ignore it again.

[BatmanAoD]

Another interesting question this brings up is whether reviews should be set to auto-expire at some point, e.g. on some kind of SemVer bump.

If your and the other reviewer's original reviews had been restricted to the current minor version, the newly fuzzed version would have been implicitly "unreviewed" by both of you.

[dpc]

@BatmanAoD Reviews are version specific right now. New version is unreviewed by definition. Previous reviews can be used as a base for differential reviews, etc. but that's about it.

[BatmanAoD]

That makes a lot of sense (and, prior to this issue, is how I assumed it worked), but in that case I don't understand how the smallvec issue arose. Did the smallvec version number not get bumped after you submitted the fuzzer?

[dpc]

@BatmanAoD Not yet. There is no new release. Technically the actual code people use is the same. I just have more confidence in it than before, because my fuzzer did not catch anything.

[dpc]

I was thinking, how could I achieve the same effect without introducing a completely new, one-off feature.

First, this field does not really fit well in package review. It seems more of a trust-level exception.

Maybe trust proofs could have conditional, or exception, or something. But that makes the trust set context-dependent, which is not great.

[uberjay]

In my opinion this would be better solved in other ways. Perhaps by including an additional age-based weight to a review's trust would help? I think this specific problem is a bit of a degenerate case. (both because of the infancy of cargo crev and the size of the reviewer pool.)

I guess what I'm trying to say is, maybe this doesn't even need to be solved?

EDIT: I'm also hoping this doesn't need to be solved, since managing a cryptographic web of trust is already a pretty hard problem to solve. Adding additional, more complex ways of overriding the trust calculations isn't going to make the UX better and easier to understand. 😉

[dpc]

@uberjay I agree that this sort of stuff would have a tendency to get solved by just aggregate volume.

I very like the idea of age-based logic here. If someone with higher trust / closer in the WoT is saying something is OK and that review was also much more recent, then it is a good indicator that probably that person knew better.

This way if you disagree with something, you can always write your own review, that indicates that "you knew better". Howerver, for this to be effective crev should make sure that any contradicting information during the review was taken into account.

Example: when writting a review for crate x crev should do a final check and double-check that the author is aware of any existing issues already reported for x and not listed in the new review, and any reviews of x with ratings different then the one given and so on.

[kornelski]

I think some form of this is needed. People will make mistakes and make wrong reviews. Currently the only way to ignore a wrong review is to distrust the author, but distrusting a person for one wrong review is a nuclear option, a creates a "too big to fail" problem.

Non-code review websites have a "Helpful/Unhelpful" buttons for reviews. Such thing could be exposed in a crev GUI as well.

[kornelski]

Referring to a review is a tricky problem. If it's identified by content/signature, then small edit of the review will make it dodge other people's metadata about it. If it's identified by (crev id, package, version), then metadata will still apply even if someone flips review from negative to positive.

So I think reviews should be identified by (crev id of the reviewer, package, version, rating). This way metadata won't apply anymore if someone changes the rating, but disagreement with a negative review will remain as long as the review is negative.

[kornelski]

So I suggest having a vote proof type. You can vote yes/no for a review. Votes will be weighed by trust of who cast the vote. Reviews that cross some threshold of downvotes will be ignored.

[dpc]

Related: #329