Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Underpinnings for Revert Risk instead of ORES scores. #4476

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,12 @@ class MwQueryResult {

@Serializable
class OresResult {
private val damaging: OresItem? = null
private val goodfaith: OresItem? = null

private val revertrisklanguageagnostic: OresItem? = null
// TODO: articlequality
// TODO: draftquality
val damagingProb: Float
get() = damaging?.trueProb ?: 0f
val goodfaithProb: Float
get() = goodfaith?.trueProb ?: 0f

val revertRisk: Float
get() = revertrisklanguageagnostic?.trueProb ?: 0f
}

@Serializable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,13 @@ class ArticleEditDetailsFragment : Fragment(), WatchlistExpiryDialog.Callback, M
}
}

private val sequentialTooltipRunnable = Runnable {
private val revertRiskTooltipRunnable = Runnable {
if (!isAdded) {
return@Runnable
}
sendPatrollerExperienceEvent("impression", "pt_tooltip")
val balloon = FeedbackUtil.getTooltip(requireContext(), getString(R.string.patroller_diff_tooltip_one), autoDismiss = true, showDismissButton = true, dismissButtonText = R.string.image_recommendation_tooltip_next, countNum = 1, countTotal = 2)
balloon.showAlignBottom(binding.oresDamagingButton)
balloon.relayShowAlignBottom(FeedbackUtil.getTooltip(requireContext(), getString(R.string.patroller_diff_tooltip_two), autoDismiss = true, showDismissButton = true, countNum = 2, countTotal = 2), binding.oresGoodFaithButton)
FeedbackUtil.getTooltip(requireContext(), getString(R.string.patroller_diff_tooltip_one), autoDismiss = true, showDismissButton = true)
.showAlignBottom(binding.revertRiskButton)
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
Expand Down Expand Up @@ -270,7 +269,7 @@ class ArticleEditDetailsFragment : Fragment(), WatchlistExpiryDialog.Callback, M

override fun onDestroyView() {
binding.appBarLayout.removeOnOffsetChangedListener(actionBarOffsetChangedListener)
binding.root.removeCallbacks(sequentialTooltipRunnable)
binding.root.removeCallbacks(revertRiskTooltipRunnable)
_binding = null
super.onDestroyView()
}
Expand Down Expand Up @@ -429,11 +428,11 @@ class ArticleEditDetailsFragment : Fragment(), WatchlistExpiryDialog.Callback, M

private fun maybeShowOneTimeSequentialRecentEditsTooltips() {
if (Prefs.showOneTimeSequentialRecentEditsDiffTooltip && viewModel.fromRecentEdits &&
binding.oresDamagingButton.isVisible && binding.oresGoodFaithButton.isVisible &&
binding.revertRiskButton.isVisible &&
parentFragment == FragmentUtil.getAncestor(this, SuggestedEditsCardsFragment::class.java)?.topBaseChild()) {
Prefs.showOneTimeSequentialRecentEditsDiffTooltip = false
binding.root.removeCallbacks(sequentialTooltipRunnable)
binding.root.postDelayed(sequentialTooltipRunnable, 500)
binding.root.removeCallbacks(revertRiskTooltipRunnable)
binding.root.postDelayed(revertRiskTooltipRunnable, 500)
}
}

Expand Down Expand Up @@ -487,22 +486,18 @@ class ArticleEditDetailsFragment : Fragment(), WatchlistExpiryDialog.Callback, M
}
binding.overlayRevisionFromTimestamp.text = binding.revisionFromTimestamp.text

binding.oresDamagingButton.isVisible = false
binding.oresGoodFaithButton.isVisible = false
binding.revertRiskButton.isVisible = false

viewModel.revisionTo?.let {
binding.usernameToButton.text = it.user
binding.revisionToTimestamp.text = DateUtil.getTimeAndDateString(requireContext(), it.timeStamp)
binding.overlayRevisionToTimestamp.text = binding.revisionToTimestamp.text
binding.revisionToEditComment.text = StringUtil.fromHtml(it.parsedcomment.trim())

if (it.ores != null && viewModel.fromRecentEdits) {
binding.oresDamagingButton.isVisible = true
binding.oresDamagingButton.text = getString(R.string.edit_damage, ((it.ores?.damagingProb ?: 0f) * 100f).toInt().toString().plus("%"))
binding.oresDamagingButton.setOnClickListener(openQualityAndIntentFiltersPage)
binding.oresGoodFaithButton.isVisible = true
binding.oresGoodFaithButton.text = getString(R.string.edit_intent, ((it.ores?.goodfaithProb ?: 0f) * 100f).toInt().toString().plus("%"))
binding.oresGoodFaithButton.setOnClickListener(openQualityAndIntentFiltersPage)
if (it.ores != null) {
binding.revertRiskButton.isVisible = true
binding.revertRiskButton.text = getString(R.string.edit_revert_risk, ((it.ores?.revertRisk ?: 0f) * 100f).toInt().toString().plus("%"))
binding.revertRiskButton.setOnClickListener(openQualityAndIntentFiltersPage)

maybeShowOneTimeSequentialRecentEditsTooltips()
}
Expand All @@ -522,11 +517,8 @@ class ArticleEditDetailsFragment : Fragment(), WatchlistExpiryDialog.Callback, M
}

private val openQualityAndIntentFiltersPage = View.OnClickListener { view ->
if (view.id == R.id.oresDamagingButton) {
sendPatrollerExperienceEvent("quality_click", "pt_edit")
} else {
sendPatrollerExperienceEvent("intent_click", "pt_edit")
}
sendPatrollerExperienceEvent("revert_risk_click", "pt_edit")
// TODO: correct url:
UriUtil.visitInExternalBrowser(requireContext(), Uri.parse(getString(R.string.quality_and_intent_filters_url)))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,11 @@ class SuggestedEditsRecentEditsFilterActivity : BaseActivity() {
SuggestedEditsRecentEditsFilterTypes.BOT_EDITS_GROUP.forEach {
filterListWithHeaders.add(Filter(FILTER_TYPE_CATEGORY, it.id))
}
// TODO: rename to "revert risk":
filterListWithHeaders.add(getString(R.string.patroller_tasks_filters_contribution_quality_header))
SuggestedEditsRecentEditsFilterTypes.DAMAGING_GROUP.forEach {
filterListWithHeaders.add(Filter(FILTER_TYPE_CATEGORY, it.id, true))
}
filterListWithHeaders.add(getString(R.string.patroller_tasks_filters_user_intent_header))
SuggestedEditsRecentEditsFilterTypes.GOODFAITH_GROUP.forEach {
filterListWithHeaders.add(Filter(FILTER_TYPE_CATEGORY, it.id, true))
}
filterListWithHeaders.add(getString(R.string.patroller_tasks_filters_significance_header))
SuggestedEditsRecentEditsFilterTypes.MINOR_EDITS_GROUP.forEach {
filterListWithHeaders.add(Filter(FILTER_TYPE_CATEGORY, it.id))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,7 @@ enum class SuggestedEditsRecentEditsFilterTypes constructor(val id: String,
DAMAGING_LIKELY_PROBLEMS("damagingLikelyProblems", "0.629|0.944",
R.string.patroller_tasks_filters_contribution_quality_likely_problems, R.string.patroller_tasks_filters_contribution_quality_likely_problems_desc),
DAMAGING_VERY_LIKELY_PROBLEMS("damagingBad", "0.944|1",
R.string.patroller_tasks_filters_contribution_quality_bad, R.string.patroller_tasks_filters_contribution_quality_bad_desc),
// Only check the goodfaith: { true } score
GOODFAITH_GOOD("goodfaithGood", "0.75|1",
R.string.patroller_tasks_filters_user_intent_good, R.string.patroller_tasks_filters_user_intent_good_desc),
GOODFAITH_MAY_PROBLEMS("goodfaithMayProblems", "0.647|0.75",
R.string.patroller_tasks_filters_user_intent_may_problems, R.string.patroller_tasks_filters_user_intent_may_problems_desc),
GOODFAITH_LIKELY_PROBLEMS("goodfaithLikelyProblems", "0.25|0.647",
R.string.patroller_tasks_filters_user_intent_likely_problems, R.string.patroller_tasks_filters_user_intent_likely_problems_desc),
GOODFAITH_VERY_LIKELY_PROBLEMS("goodfaithBad", "0|0.25",
R.string.patroller_tasks_filters_user_intent_bad, R.string.patroller_tasks_filters_user_intent_bad_desc);
R.string.patroller_tasks_filters_contribution_quality_bad, R.string.patroller_tasks_filters_contribution_quality_bad_desc);

override fun code(): Int {
// This enumeration is not marshalled so tying declaration order to presentation order is
Expand All @@ -68,8 +59,8 @@ enum class SuggestedEditsRecentEditsFilterTypes constructor(val id: String,
val LATEST_REVISIONS_GROUP = listOf(LATEST_REVISION, NOT_LATEST_REVISION)
val USER_REGISTRATION_GROUP = listOf(UNREGISTERED, REGISTERED)
val USER_EXPERIENCE_GROUP = listOf(NEWCOMERS, LEARNERS, EXPERIENCED_USERS)
// TODO: rename to "revert risk"?
val DAMAGING_GROUP = listOf(DAMAGING_GOOD, DAMAGING_MAY_PROBLEMS, DAMAGING_LIKELY_PROBLEMS, DAMAGING_VERY_LIKELY_PROBLEMS)
val GOODFAITH_GROUP = listOf(GOODFAITH_GOOD, GOODFAITH_MAY_PROBLEMS, GOODFAITH_LIKELY_PROBLEMS, GOODFAITH_VERY_LIKELY_PROBLEMS)

// Multiple choice
val DEFAULT_FILTER_USER_STATUS = setOf(UNREGISTERED, NEWCOMERS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ class SuggestedEditsRecentEditsViewModel : ViewModel() {

val allRecentChanges = response.query?.recentChanges.orEmpty()

// Filtering Ores damaging and goodfaith
val recentChanges = filterOresScores(filterOresScores(allRecentChanges, true), false)
// Filtering by revert risk
val recentChanges = filterOresScores(allRecentChanges)

// Get usernames
val usernames = recentChanges.filter { !it.anon }.map { it.user }.distinct().filter {
Expand Down Expand Up @@ -155,8 +155,7 @@ class SuggestedEditsRecentEditsViewModel : ViewModel() {
// Ores related: default is empty
val findSelectedOres = Prefs.recentEditsIncludedTypeCodes.subtract(findSelectedUserStatus.toSet())
.filter { code ->
SuggestedEditsRecentEditsFilterTypes.GOODFAITH_GROUP.map { it.id }.contains(code) ||
SuggestedEditsRecentEditsFilterTypes.DAMAGING_GROUP.map { it.id }.contains(code)
SuggestedEditsRecentEditsFilterTypes.DAMAGING_GROUP.map { it.id }.contains(code)
}

// Find the remaining selected filters
Expand Down Expand Up @@ -198,8 +197,7 @@ class SuggestedEditsRecentEditsViewModel : ViewModel() {
}

if (includedTypesCodes.any { code ->
SuggestedEditsRecentEditsFilterTypes.GOODFAITH_GROUP.map { it.id }.contains(code) ||
SuggestedEditsRecentEditsFilterTypes.DAMAGING_GROUP.map { it.id }.contains(code) }) {
SuggestedEditsRecentEditsFilterTypes.DAMAGING_GROUP.map { it.id }.contains(code) }) {
list.add("oresreview")
}

Expand Down Expand Up @@ -269,9 +267,8 @@ class SuggestedEditsRecentEditsViewModel : ViewModel() {
return recentChanges
}

private fun filterOresScores(recentChanges: List<MwQueryResult.RecentChange>, isDamagingGroup: Boolean): List<MwQueryResult.RecentChange> {
val filterGroupSet = if (isDamagingGroup) SuggestedEditsRecentEditsFilterTypes.DAMAGING_GROUP.map { it.id }
else SuggestedEditsRecentEditsFilterTypes.GOODFAITH_GROUP.map { it.id }
private fun filterOresScores(recentChanges: List<MwQueryResult.RecentChange>): List<MwQueryResult.RecentChange> {
val filterGroupSet = SuggestedEditsRecentEditsFilterTypes.DAMAGING_GROUP.map { it.id }

if (Prefs.recentEditsIncludedTypeCodes.any { code -> filterGroupSet.contains(code) }) {
val findOresFilters = Prefs.recentEditsIncludedTypeCodes
Expand All @@ -283,7 +280,7 @@ class SuggestedEditsRecentEditsViewModel : ViewModel() {

return recentChanges.filter { it.ores != null }.filter {
val scoreRanges = findOresFilters.map { type -> type.value }
val oresScore = if (isDamagingGroup) it.ores?.damagingProb ?: 0f else it.ores?.goodfaithProb ?: 0f
val oresScore = it.ores?.revertRisk ?: 0f
scoreRanges.forEach { range ->
val scoreRangeArray = range.split("|")
val inScoreRange = oresScore >= scoreRangeArray.first().toFloat() && oresScore <= scoreRangeArray.last().toFloat()
Expand Down
15 changes: 3 additions & 12 deletions app/src/main/res/layout/fragment_article_edit_details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@
android:layout_marginStart="16dp"
android:layout_marginTop="6dp"
android:layout_marginBottom="8dp"
app:constraint_referenced_ids="oresDamagingButton,oresGoodFaithButton,diffCharacterCountView"
app:constraint_referenced_ids="revertRiskButton,diffCharacterCountView"
app:flow_horizontalBias="0"
app:flow_horizontalGap="8dp"
app:flow_horizontalStyle="packed"
Expand All @@ -263,22 +263,13 @@
app:layout_constraintTop_toBottomOf="@id/thanksDivider" />

<com.google.android.material.button.MaterialButton
android:id="@+id/oresDamagingButton"
android:id="@+id/revertRiskButton"
style="@style/App.Button.Secondary"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:textColor="?attr/secondary_color"
android:layout_marginStart="16dp"
tools:text="Damaging: 50%"/>

<com.google.android.material.button.MaterialButton
android:id="@+id/oresGoodFaithButton"
style="@style/App.Button.Secondary"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:textColor="?attr/secondary_color"
android:layout_marginStart="16dp"
tools:text="Goodfaith: 50%"/>
tools:text="Revert risk: 50%"/>

<TextView
android:id="@+id/diffCharacterCountView"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values-qq/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@
<string name="edit_warn">Title of the screen of publishing a warn message to a user talk page.</string>
<string name="edit_damage">Label of the edit damage in the article edit detail page. %s will be replaced by the percentage of edit damage.</string>
<string name="edit_intent">Label of the edit intent in the article edit detail page. %s will be replaced by the percentage of edit intent.</string>
<string name="edit_revert_risk">Label that displays the calculated revert risk of a certain revision. %s will be replaced by the percentage of revert risk.</string>
<plurals name="edit_diff_bytes">
<item quantity="one">Text for the revisions difference in bytes. %s will be replaced by the positive one or negative one.</item>
<item quantity="other">Text for the revisions difference in bytes. %s will be replaced by the number of bytes.</item>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@
<string name="edit_warn">Warn</string>
<string name="edit_damage">Damage %s</string>
<string name="edit_intent">Intent %s</string>
<string name="edit_revert_risk">Revert risk: %s</string>
<plurals name="edit_diff_bytes">
<item quantity="one">%s byte</item>
<item quantity="other">%s bytes</item>
Expand Down
Loading