SOBotics / Higgs

A generic dashboard for viewing and providing feedback to SOBotics bots.
GNU General Public License v3.0
4 stars 0 forks source link

Review queue for Higgs #34

Closed rjrudman closed 6 years ago

rjrudman commented 6 years ago

We need a way for Higgs to be configured to mark whether or not a post has been reviewed or not. A straight-forward way would be to have the bots configure how many reviews are required to mark the report as reviewed or not.

The problems arise when conflicting feedback is given. Do we want functionality to extend the number of reviews required if conflicting feedback is given? Also, what about feedback which is different, but not necessarily conflicting?

How do we handle this with other bots, and what would you all like to be able to configure?

double-fault commented 6 years ago

IMHO, everything should be configurable by bot owners, including conflicting feedback. Maybe keep a field asking whether conflicting feedback should be put into review, and how many more feedbacks should be there to overrule the conflicting feedback.

Also, it'd be nice to add a way to clear your feedback from the report page; in cases where you've made a mistake, clearing the feedback will easily put it out of the queue.

double-fault commented 6 years ago

Maybe even allow admins to clear feedback if they believe it is invalid?

rjrudman commented 6 years ago

Good point about clearing feedback. I've added an issue here.

As for the configuration, I'm also interested in discussing the architecture of the configuration. I'd like it to be pretty flexible, but I also don't want to over complicate things for people writing bots

double-fault commented 6 years ago

I don't see how adding customisation for conflicting feedbacks is going to complicate things a lot, as it will only add 2 fields. Still, if you want to keep it even simpler, maybe provide defaults which owners can change based on their choice? This will keep the model flexible for those who require customisation, and will also be fast for those in a hurry.

jdd-software commented 6 years ago

This is some information only related to HD (comments) just 1 use case:

Comments are subjective there is no exact line when a comments instead of being no longer need becomes offensive. In general SE policy is much tougher then meta policy.

With this in mind conflicting feedback is no problem, it's just an indication of the subjectivity, hence there is no direct need for additional review nor is there a need for admin to change feedback from a user.

What is useful is to have as much feedback as possible, to be able to filter out interesting comments that admin can select and add to future feeds.

I like a lot the review interface used by sentinel if a user could in a similar interface filter on (no-feedback (yes/no/both), conflicting feedback (yes/no/both), detection score (min-max), user (select)) the user could add additional feedback to comments of interest.

Normally a user could just review comments that have no feedback but to me it is more interesting to for example filter on all low scoring comments, see current feedback and then also give my own. (trying to find low-scoring offensive comments that are good to add to feed).

adeak commented 6 years ago

There's also the aspect of bots that take action on main, such as Natty's autocomment. Perhaps there should be an option to submit these kinds of actions to additional scrutiny, for instance no autocomment should go unreviewed and maybe even no autocomment should have conflicting feedback (say, conflicting feedback should be resolved by admins).

rjrudman commented 6 years ago

Okay, based on the above discussion, here's my proposal. Currently, we have a dashboard setup for admins (and in the future, the owners of the bots), which allow us to configure feedback types. Here's what we currently have:

{
  "botId": 0,
  "feedbackTypes": [
    {
      "name": "tp",
      "colour": "green",
      "icon": "+",
      "isActionable": true,
      "isEnabled": true
    },
    {
      "name": "fp",
      "colour": "red",
      "icon": "-",
      "isActionable": true,
      "isEnabled": true
    },
  ]
}

Here's what I propose:

{
  "botId": 0,
  "feedbackTypes": [
    {
      "name": "tp",
      "colour": "green",
      "icon": "+",
      "isEnabled": true,
      // Whether or not this is counted in the field below (requiredToBeActioned)
      "isActionable": true
    },
    {
      "name": "fp",
      "colour": "red",
      "icon": "-",
      "isEnabled": true,
      "isActionable": true
    }],
    "conflicts": [ 
      { 
        f1: "tp",
        // f2 here is optional. Passing in null means that *any* tp feedback raises a conflict. 
        // Useful in conjunction with 'requiresAdmin', for cases like auto-flagging receiving a fp.
        f2: "fp" 
        // If we hit a conflict, we now require 5 feedbacks to mark the report as 'actioned'
        "requiredToBeActioned": 5,
        // If we hit this conflict, an admin/bot owner should be alerted. If someone changes
        // their feedback to resolve the conflict, the alert will be retracted
        "requiresAdmin": true
      } 
    ],

    // How many *total* feedbacks are required to mark the report as 'actioned'.
    "requiredToBeActioned": 3 
}

For the most part, bots won't need to be aware about this kind of information. Since this can be setup per bot, we can apply these rules to all reports sent by the bot.

However, bots are able to override the requiredToBeActioned and conflicts values on a per-report basis. This accounts for @adeak's situation where an automated comment/flag needs more scrutiny. If they provide a value for conflicts, it needs to include all conflict rules. Passing an empty array, for example, would disable conflicts for this particular post.

double-fault commented 6 years ago

I notice that for conflicts the f1 and f2 is specified. I'm assuming these are the two feedbacks which should trigger a conflict.

This could be very useful for bots like HeatDetector which has a ton of different feedbacks, and not any two different feedbacks (example, skp) should trigger a conflict. But for some bots, the feedback types are usually very different and IMO it would be unnecessary to have all possible conflicts specified.

Instead, why can't we do it the other way around? All feedbacks by default trigger a conflict, and for bots like HeatDetector some feedback combinations can be excluded from all conflicts.

rjrudman commented 6 years ago

Hmm. You raise a good point about that situation. The request may become quite verbose when needing to specify multiple conflicts.

However, even if all feedbacks are conflicting with each other, some may be worse than others (for example, requiring someone to intervene). I think we'd need a way to specifically mark which conflicts are worse than others.

I have two possible solutions:

  1. Instead of f1 and f2, we could have a feedbacks field which is an array of all feedbacks which conflict. For example:
conflicts: [
    {
        feedbacks: ['a', 'b', 'c'],
        requiresAdmin: true
    }, 
    {
        feedbacks: ['a', 'd', 'e'],
        requiresAdmin: false
    }
]

Might cut down on the verbosity. Though it does add a bit more complicated error detection/logic (what happens if a feedback pair is specified multiple times?).

  1. Higgs automatically generate my proposal, with all feedbacks conflicting with eachother. This would allow us to still keep the structure (avoiding 'negative' fields is more clear, at least to me. "noConflicts": {} is a bit more unintuitive than "conflicts": {}). At the same time, it would cut down on setup time by automatically choosing the more likely scenario. A further benefit of this is... if we find out that in general bots don't usually have feedbacks conflicting.. it's easy for us to change how the defaults are generated, without having to change the API.
double-fault commented 6 years ago

@rjrudman I believe that we have to reach a line between flexibility and simplicity. This is how I think the conflicting feedbacks should be handled (somewhat similar to your proposal two).

Conflicting feedbacks should be enabled by default, meaning that any two different feedbacks should be thought of as conflicting. Admins should be notified unless disabled to be on the safe side. A bot should be able to disable this setting, preferably through a simple boolean or a checkbox on the bot settings page. If this is disabled, all possible conflicts can be included in a conflicts array like you have mentioned.

The negative thing is a bit more unintuitive, I agree, but we can always make it better by naming it better, say something like, conflictException?

Overall, here is what it should look like:

conflictException: {
    feedbacks: ['a', 'b']
}

And, if all feedback conflicts are disabled by default, you should be able to introduce conflicting feedbacks like you've said:

conflicts: [
   [ {
        feedbacks: ['a', 'b', 'c'],
        requiresAdmin: true
    }, 
    {
        feedbacks: ['a', 'd', 'e'],
        requiresAdmin: false
    }
]]

Overall, this would mean that the settings would usually be one time from the bot settings page. Also, by enabling all feedbacks as conflicting and alerting admins by default, you are on the safe side that no report with conflicting feedbacks which should have been handled in a different way has gone unnoticed. Admins can disable this if they wish.

According to the same logic, if a feedback pair is specified two times, assume the feedback as conflicting; do not put it in the conflict exceptions.

Also, if none of the conflicts or conflictException arrays have been specified, just go with the bot setting defaults.

rjrudman commented 6 years ago

@Fortunate-MAN After thinking about it, I like your proposed approach here. It'll also nicely handle cases where new feedback types are added (they'll be conflicting by default). Unless anyone raises a concern otherwise in the next day or so, I'll be implementing it this way

double-fault commented 6 years ago

@rjrudman perfect, thank you!