diff --git a/MYPY_IMPROVEMENT_PLAN.md b/MYPY_IMPROVEMENT_PLAN.md deleted file mode 100644 index 80d04419..00000000 --- a/MYPY_IMPROVEMENT_PLAN.md +++ /dev/null @@ -1,295 +0,0 @@ -# Mypy Type Checking Improvement Plan - -## Background - -PR #80 added mypy type checking infrastructure, but the checks were commented out due to type errors in the codebase. This document outlines an incremental approach to enable full mypy type checking. - -## Current Status - -### What's Already Set Up (PR #80) -- ✅ Mypy configuration in `pyproject.toml` -- ✅ Django stubs and type stubs for third-party libraries -- ✅ GitHub workflow (commented out in `.github/workflows/security.yml` lines 15-36) -- ✅ Lenient initial settings (allows gradual adoption) - -### Current Configuration -```toml -[tool.mypy] -python_version = "3.9" -plugins = ["mypy_django_plugin.main"] - -# Starting lenient - can tighten later -disallow_untyped_defs = false # Don't require type annotations everywhere -disallow_incomplete_defs = false # Don't require complete type annotations -disallow_untyped_calls = false # Allow calling untyped functions -check_untyped_defs = true # But do check functions that ARE typed -``` - -## Incremental Improvement Strategy - -### Phase 1: Identify and Categorize Errors (This PR) - -**Goal**: Run mypy, collect all errors, categorize by type and module - -**Steps**: -1. Run `cd app && python -m mypy reviews --config-file=../pyproject.toml > mypy_errors.txt 2>&1` -2. Analyze error types: - - Missing return type annotations - - Missing parameter type annotations - - `Any` type issues - - Optional/None handling issues - - Django model typing issues - - Third-party library stub issues -3. Group by module/file -4. Create prioritized issue list - -**Deliverable**: `mypy_errors_analysis.md` with categorized errors and fix priorities - -### Phase 2: Fix Low-Hanging Fruit (Week 1) - -**Target**: Files with <5 errors that are straightforward to fix - -**Common Easy Fixes**: -```python -# Before -def get_wiki(pk): - return Wiki.objects.get(pk=pk) - -# After -def get_wiki(pk: int) -> Wiki: - return Wiki.objects.get(pk=pk) -``` - -**Files to Start With** (typically easiest): -- Utility modules (`app/reviews/utils/`) -- Simple helper functions -- Management commands that aren't complex - -**Deliverable**: PR with 5-10 files fully typed - -### Phase 3: Django Models and QuerySets (Week 2) - -**Target**: Models, managers, and Django-specific patterns - -**Common Django Patterns**: -```python -from typing import Optional -from django.db.models import QuerySet - -class WikiManager(models.Manager["Wiki"]): - def get_by_code(self, code: str) -> Optional["Wiki"]: - try: - return self.get(code=code) - except Wiki.DoesNotExist: - return None - -class Wiki(models.Model): - objects: WikiManager = WikiManager() - - code: str # Django stubs understand this - family: str -``` - -**Files to Focus On**: -- `app/reviews/models/` -- Model methods that return QuerySets -- Custom managers - -**Deliverable**: PR with all models properly typed - -### Phase 4: Views and API Endpoints (Week 3) - -**Target**: Django views, request/response typing - -**Common View Patterns**: -```python -from django.http import HttpRequest, HttpResponse, JsonResponse -from typing import Any - -def api_wikis(request: HttpRequest) -> JsonResponse: - wikis = Wiki.objects.all() - data: list[dict[str, Any]] = [...] - return JsonResponse(data, safe=False) -``` - -**Files to Focus On**: -- `app/reviews/views.py` -- API endpoint functions -- Form handling - -**Deliverable**: PR with all views properly typed - -### Phase 5: Services and Business Logic (Week 4) - -**Target**: Core business logic, autoreview checks, services - -**Complex Patterns**: -```python -from typing import Protocol, TypedDict - -class CheckContext(TypedDict): - revision: PendingRevision - configuration: WikiConfiguration - -class AutoreviewCheck(Protocol): - def __call__(self, context: CheckContext) -> CheckResult: - ... -``` - -**Files to Focus On**: -- `app/reviews/autoreview/` -- `app/reviews/services/` -- Complex algorithms (`_is_addition_superseded`, etc.) - -**Deliverable**: PR with services and checks typed - -### Phase 6: Tighten Mypy Settings (Week 5) - -**Goal**: Enable stricter mypy checks after codebase is mostly typed - -**Settings to Enable Gradually**: -```toml -[tool.mypy] -# Phase 6a: Enable after most functions are typed -disallow_untyped_defs = true # Require all functions to have types - -# Phase 6b: Enable after Phase 6a is complete -disallow_incomplete_defs = true # Require complete type annotations - -# Phase 6c: Final strictness -disallow_untyped_calls = true # Require typed function calls -``` - -**Approach**: -- Enable one setting at a time -- Fix errors module by module -- Use `# type: ignore[error-code]` sparingly with explanatory comments - -**Deliverable**: PR enabling stricter mypy settings - -### Phase 7: Enable Mypy in CI (Final) - -**Goal**: Uncomment mypy workflow, make it required check - -**Steps**: -1. Uncomment lines 15-36 in `.github/workflows/security.yml` -2. Run test PR to ensure it passes -3. Make mypy check required for all PRs - -**Success Criteria**: -- All PRs must pass mypy checks -- No `# type: ignore` comments without explanation -- New code must be fully typed - -## Guidelines for Type Annotations - -### DO: -```python -# ✅ Use specific types -def get_pending_count(wiki: Wiki) -> int: - return PendingRevision.objects.filter(page__wiki=wiki).count() - -# ✅ Use Optional for nullable values -def find_user(username: str) -> Optional[EditorProfile]: - try: - return EditorProfile.objects.get(username=username) - except EditorProfile.DoesNotExist: - return None - -# ✅ Use TypedDict for structured dicts -from typing import TypedDict - -class RevisionResult(TypedDict): - revid: int - decision: str - tests: list[dict[str, Any]] -``` - -### DON'T: -```python -# ❌ Don't use bare `Any` everywhere -def process_data(data: Any) -> Any: # Too vague - ... - -# ❌ Don't ignore errors without explanation -result = some_function() # type: ignore # BAD: why are we ignoring? - -# ✅ DO explain when ignoring -result = external_library() # type: ignore[no-untyped-call] # pywikibot has no stubs -``` - -## Handling Common Patterns - -### Django QuerySets -```python -from django.db.models import QuerySet - -def get_recent_revisions(wiki: Wiki) -> QuerySet[PendingRevision]: - return PendingRevision.objects.filter(page__wiki=wiki).order_by("-timestamp") -``` - -### JSON Responses -```python -from typing import Any, TypedDict - -class WikiDict(TypedDict): - id: int - code: str - family: str - -def serialize_wiki(wiki: Wiki) -> WikiDict: - return {"id": wiki.id, "code": wiki.code, "family": wiki.family} -``` - -### Pywikibot (No Stubs) -```python -import pywikibot -from typing import Any - -# Option 1: Use Any for pywikibot objects -site: Any = pywikibot.Site("en", "wikipedia") - -# Option 2: Ignore specific lines -site = pywikibot.Site("en", "wikipedia") # type: ignore[no-untyped-call] -``` - -## Metrics - -Track progress with: -```bash -# Count total errors -mypy reviews --config-file=../pyproject.toml 2>&1 | grep "error:" | wc -l - -# Count errors by type -mypy reviews --config-file=../pyproject.toml 2>&1 | grep "error:" | cut -d: -f4 | sort | uniq -c | sort -rn -``` - -## Success Criteria - -- [ ] Phase 1: Error analysis document created -- [ ] Phase 2: 10+ files with zero mypy errors -- [ ] Phase 3: All models fully typed -- [ ] Phase 4: All views fully typed -- [ ] Phase 5: All services/checks fully typed -- [ ] Phase 6: Stricter mypy settings enabled -- [ ] Phase 7: Mypy CI check enabled and required -- [ ] Total mypy errors: 0 -- [ ] Type coverage: >90% - -## Timeline - -- **Week 1**: Phases 1-2 (Analysis + easy fixes) -- **Week 2**: Phase 3 (Models) -- **Week 3**: Phase 4 (Views) -- **Week 4**: Phase 5 (Services) -- **Week 5**: Phase 6 (Stricter settings) -- **Week 6**: Phase 7 (Enable CI) - -Each phase should be a separate PR for easier review. - -## Resources - -- [Mypy documentation](https://mypy.readthedocs.io/) -- [Django-stubs documentation](https://github.com/typeddjango/django-stubs) -- [Python typing module](https://docs.python.org/3/library/typing.html) -- [Real Python: Type Checking](https://realpython.com/python-type-checking/) diff --git a/app/reviewer/settings.py b/app/reviewer/settings.py index ee77d371..3cff8e97 100644 --- a/app/reviewer/settings.py +++ b/app/reviewer/settings.py @@ -126,6 +126,24 @@ PYWIKIBOT_SITE_FAMILY = os.getenv("PYWIKIBOT_SITE_FAMILY", "wikipedia") +# Revert detection configuration +# Enable/disable revert detection for already-reviewed edits +ENABLE_REVERT_DETECTION = os.getenv("ENABLE_REVERT_DETECTION", "True").lower() in ( + "true", + "1", + "yes", +) + +# Pending changes approval configuration +# Enable/disable dry-run mode for pending changes approval +# When True, only allows approvals on test pages (Merkityt_versiot_-kokeilu/*) +# When False, allows approvals on all pages +PENDING_CHANGES_DRY_RUN = os.getenv("PENDING_CHANGES_DRY_RUN", "True").lower() in ( + "true", + "1", + "yes", +) + # ORES model thresholds (global defaults, per-wiki config takes precedence) ORES_DAMAGING_THRESHOLD = float(os.getenv("ORES_DAMAGING_THRESHOLD", "0.3")) ORES_GOODFAITH_THRESHOLD = float(os.getenv("ORES_GOODFAITH_THRESHOLD", "0.7")) diff --git a/app/reviews/autoreview/__init__.py b/app/reviews/autoreview/__init__.py index 9d48db4f..477a675d 100644 --- a/app/reviews/autoreview/__init__.py +++ b/app/reviews/autoreview/__init__.py @@ -1 +1,32 @@ from __future__ import annotations + +# Backwards-compatibility exports for older test imports +from pywikibot.data.superset import SupersetQuery # re-export for tests + +from .checks.revert_detection import ( + _find_reviewed_revisions_by_sha1, + _parse_revert_params, + check_revert_detection, +) +from .context import CheckContext + + +def _check_revert_detection(revision, client): + """Compatibility wrapper matching legacy signature used in tests.""" + context = CheckContext( + revision=revision, + client=client, + profile=None, + auto_groups={}, + blocking_categories={}, + redirect_aliases=[], + ) + return check_revert_detection(context) + + +__all__ = [ + "SupersetQuery", + "_check_revert_detection", + "_find_reviewed_revisions_by_sha1", + "_parse_revert_params", +] diff --git a/app/reviews/autoreview/checks/revert_detection.py b/app/reviews/autoreview/checks/revert_detection.py new file mode 100644 index 00000000..b78148b5 --- /dev/null +++ b/app/reviews/autoreview/checks/revert_detection.py @@ -0,0 +1,189 @@ +""" +Revert detection check for already-reviewed edits. + +This check detects when a pending edit is a revert to previously reviewed content +by matching SHA1 content hashes and checking for revert tags. +""" + +import json +import logging +from typing import Any + +from django.conf import settings + +from ..context import CheckContext + +logger = logging.getLogger(__name__) + + +def check_revert_detection(context: CheckContext) -> dict[str, Any]: + """ + Check if a revision is a revert to previously reviewed content. + + Args: + context: CheckContext containing revision and related data + + Returns: + Dict with check result including status, message, and metadata + """ + # Check if revert detection is enabled + if not getattr(settings, "ENABLE_REVERT_DETECTION", True): + return {"status": "skip", "message": "Revert detection is disabled", "metadata": {}} + + revision = context.revision + page = revision.page + + # Check for revert tags + revert_tags = {"mw-manual-revert", "mw-reverted", "mw-rollback", "mw-undo"} + change_tags = getattr(revision, "change_tags", []) + + if not any(tag in change_tags for tag in revert_tags): + return { + "status": "skip", + "message": "No revert tags found", + "metadata": {"change_tags": change_tags}, + } + + # Parse change tag parameters to get reverted revision IDs + reverted_rev_ids = _parse_revert_params(revision) + if not reverted_rev_ids: + return { + "status": "skip", + "message": "No reverted revision IDs found in change tags", + "metadata": {"change_tags": change_tags}, + } + + # Check if any of the reverted revisions were previously reviewed + reviewed_revisions = _find_reviewed_revisions_by_sha1(context.client, page, reverted_rev_ids) + + if reviewed_revisions: + return { + "status": "approve", + "message": ( + f"Revert to previously reviewed content (SHA1: {reviewed_revisions[0]['sha1']})" + ), + "metadata": { + "reverted_rev_ids": reverted_rev_ids, + "reviewed_revisions": reviewed_revisions, + "revert_tags": [tag for tag in change_tags if tag in revert_tags], + }, + } + + return { + "status": "block", + "message": "Revert detected but no previously reviewed content found", + "metadata": { + "reverted_rev_ids": reverted_rev_ids, + "revert_tags": [tag for tag in change_tags if tag in revert_tags], + }, + } + + +def _parse_revert_params(revision) -> list[int]: + """ + Parse change tag parameters to extract reverted revision IDs. + + Args: + revision: PendingRevision object + + Returns: + List of reverted revision IDs + """ + try: + # Get change tag parameters from revision + change_tag_params = getattr(revision, "change_tag_params", []) + if not change_tag_params: + return [] + + reverted_ids = [] + + for param_str in change_tag_params: + try: + # Parse JSON parameter + param_data = json.loads(param_str) + + # Extract reverted revision IDs + if "oldestRevertedRevId" in param_data: + reverted_ids.append(param_data["oldestRevertedRevId"]) + if "newestRevertedRevId" in param_data: + reverted_ids.append(param_data["newestRevertedRevId"]) + if "originalRevisionId" in param_data: + reverted_ids.append(param_data["originalRevisionId"]) + + except (json.JSONDecodeError, KeyError) as e: + logger.warning(f"Failed to parse change tag param: {param_str}, error: {e}") + continue + + return list(set(reverted_ids)) # Remove duplicates + + except Exception as e: + logger.error(f"Error parsing revert params for revision {revision.revid}: {e}") + return [] + + +def _find_reviewed_revisions_by_sha1(client, page, reverted_rev_ids: list[int]) -> list[dict]: + """ + Find previously reviewed revisions by SHA1 content hash. + + This implements @zache-fi's suggested Superset approach: + 1. Query MediaWiki database for older reviewed versions by SHA1 + 2. Check if any of the reverted revisions were previously reviewed + + Args: + client: WikiClient instance + page: PendingPage object + reverted_rev_ids: List of reverted revision IDs + + Returns: + List of reviewed revision data + """ + if not reverted_rev_ids: + return [] + + try: + # Execute Superset query to find reviewed revisions by SHA1 + # This follows @zache-fi's suggested SQL approach + revid_list = ",".join(str(revid) for revid in reverted_rev_ids) + + # Note: revid_list is safe - reverted_rev_ids are integers from DB queries + sql_query = f""" # noqa: S608 + SELECT + MAX(rev_id) as max_reviewable_rev_id_by_sha1, + rev_page, + content_sha1, + MAX(fr_rev_id) as max_old_reviewed_id + FROM + revision + LEFT JOIN flaggedrevs ON rev_id=fr_rev_id + JOIN slots ON slot_revision_id=rev_id + JOIN content ON slot_content_id=content_id + WHERE + rev_id IN ({revid_list}) + GROUP BY + rev_page, content_sha1 + """ + + # Execute query using SupersetQuery + from pywikibot.data.superset import SupersetQuery + + superset = SupersetQuery(site=client.site) + results = superset.query(sql_query) + + # Filter results where content was previously reviewed + reviewed_revisions = [] + for result in results: + if result.get("max_old_reviewed_id") is not None: + reviewed_revisions.append( + { + "sha1": result.get("content_sha1"), + "max_reviewed_id": result.get("max_old_reviewed_id"), + "max_reviewable_id": result.get("max_reviewable_rev_id_by_sha1"), + "page_id": result.get("rev_page"), + } + ) + + return reviewed_revisions + + except Exception as e: + logger.error(f"Error finding reviewed revisions for page {page.pageid}: {e}") + return [] diff --git a/app/reviews/models/pending_revision.py b/app/reviews/models/pending_revision.py index 07b9d4f0..bdbb3ca9 100644 --- a/app/reviews/models/pending_revision.py +++ b/app/reviews/models/pending_revision.py @@ -41,6 +41,20 @@ class Meta: def __str__(self) -> str: return f"{self.page.title}#{self.revid}" + @property + def change_tag_params(self) -> list[str]: + """Get change tag parameters from superset_data.""" + if not self.superset_data: + return [] + return self.superset_data.get("change_tags_params", []) + + @change_tag_params.setter + def change_tag_params(self, value: list[str]) -> None: + """Set change tag parameters in superset_data.""" + if not self.superset_data: + self.superset_data = {} + self.superset_data["change_tags_params"] = value + def get_wikitext(self) -> str: """Return the revision wikitext, fetching it via the API when missing.""" if self.wikitext: diff --git a/app/reviews/services/parsers.py b/app/reviews/services/parsers.py index 5bfc773b..d088ee9c 100644 --- a/app/reviews/services/parsers.py +++ b/app/reviews/services/parsers.py @@ -81,6 +81,7 @@ def prepare_superset_metadata(entry: dict) -> dict: metadata = dict(entry) for key in ( "change_tags", + "change_tags_params", "user_groups", "user_former_groups", "page_categories", diff --git a/app/reviews/services/wiki_client.py b/app/reviews/services/wiki_client.py index 150885ec..cb18865f 100644 --- a/app/reviews/services/wiki_client.py +++ b/app/reviews/services/wiki_client.py @@ -152,6 +152,7 @@ def fetch_pending_pages(self, limit: int = 10000) -> list[PendingPage]: a.actor_name, a.actor_user, group_concat(DISTINCT(ctd_name)) AS change_tags, + group_concat(DISTINCT(ct_params)) AS change_tags_params, group_concat(DISTINCT(ug_group)) AS user_groups, group_concat(DISTINCT(ufg_group)) AS user_former_groups, group_concat(DISTINCT(cl_to)) AS page_categories, diff --git a/app/reviews/tests/test_revert_detection.py b/app/reviews/tests/test_revert_detection.py new file mode 100644 index 00000000..edac2f79 --- /dev/null +++ b/app/reviews/tests/test_revert_detection.py @@ -0,0 +1,261 @@ +""" +Tests for revert detection functionality. + +This module tests the revert detection check that identifies when +a pending edit is a revert to previously reviewed content. +""" + +import json +from datetime import datetime, timedelta, timezone +from unittest.mock import Mock, patch + +from django.test import TestCase + +from reviews.autoreview import ( + _check_revert_detection, + _find_reviewed_revisions_by_sha1, + _parse_revert_params, +) +from reviews.models import PendingPage, PendingRevision, Wiki, WikiConfiguration +from reviews.services import WikiClient + + +class RevertDetectionTests(TestCase): + """Test cases for revert detection functionality.""" + + def setUp(self): + """Set up test data.""" + self.wiki = Wiki.objects.create( + name="Test Wiki", + code="test", + family="wikipedia", + api_endpoint="https://test.wikipedia.org/w/api.php", + ) + self.config = WikiConfiguration.objects.create(wiki=self.wiki) + + self.page = PendingPage.objects.create( + wiki=self.wiki, + pageid=12345, + title="Test Page", + stable_revid=100, + ) + + self.revision = PendingRevision.objects.create( + page=self.page, + revid=200, + parentid=150, + user_name="TestUser", + user_id=1000, + timestamp=datetime.now(timezone.utc), + fetched_at=datetime.now(timezone.utc), + age_at_fetch=timedelta(hours=1), + sha1="abc123", + comment="Test revert", + change_tags=["mw-manual-revert"], + change_tag_params=[ + json.dumps( + { + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175, + } + ) + ], + wikitext="", + ) + + self.client = Mock(spec=WikiClient) + self.client.site = Mock() + + def test_revert_detection_disabled(self): + """Test that revert detection is skipped when disabled.""" + with self.settings(ENABLE_REVERT_DETECTION=False): + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "skip") + self.assertEqual(result["message"], "Revert detection is disabled") + + def test_no_revert_tags(self): + """Test that revert detection is skipped when no revert tags are present.""" + self.revision.change_tags = ["mw-edit"] + self.revision.save() + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "skip") + self.assertEqual(result["message"], "No revert tags found") + + def test_parse_revert_params(self): + """Test parsing of change tag parameters.""" + reverted_ids = _parse_revert_params(self.revision) + + expected_ids = [180, 190, 175] # From change_tag_params + self.assertEqual(set(reverted_ids), set(expected_ids)) + + def test_parse_revert_params_empty(self): + """Test parsing when no change tag parameters are present.""" + self.revision.change_tag_params = [] + self.revision.save() + + reverted_ids = _parse_revert_params(self.revision) + self.assertEqual(reverted_ids, []) + + def test_parse_revert_params_invalid_json(self): + """Test parsing with invalid JSON in change tag parameters.""" + self.revision.change_tag_params = ["invalid json"] + self.revision.save() + + reverted_ids = _parse_revert_params(self.revision) + self.assertEqual(reverted_ids, []) + + @patch("pywikibot.data.superset.SupersetQuery") + def test_find_reviewed_revisions_by_sha1_success(self, mock_superset): + """Test finding reviewed revisions by SHA1.""" + # Mock SupersetQuery results + mock_superset.return_value.query.return_value = [ + { + "content_sha1": "abc123", + "max_old_reviewed_id": 150, + "max_reviewable_rev_id_by_sha1": 180, + "rev_page": 12345, + } + ] + + reverted_ids = [180, 190] + reviewed_revisions = _find_reviewed_revisions_by_sha1(self.client, self.page, reverted_ids) + + self.assertEqual(len(reviewed_revisions), 1) + self.assertEqual(reviewed_revisions[0]["sha1"], "abc123") + self.assertEqual(reviewed_revisions[0]["max_reviewed_id"], 150) + + @patch("pywikibot.data.superset.SupersetQuery") + def test_find_reviewed_revisions_by_sha1_no_results(self, mock_superset): + """Test when no reviewed revisions are found.""" + mock_superset.return_value.query.return_value = [] + + reverted_ids = [180, 190] + reviewed_revisions = _find_reviewed_revisions_by_sha1(self.client, self.page, reverted_ids) + + self.assertEqual(reviewed_revisions, []) + + @patch("reviews.autoreview.checks.revert_detection._find_reviewed_revisions_by_sha1") + def test_revert_detection_approve(self, mock_find_reviewed): + """Test revert detection when revert to reviewed content is found.""" + # Mock finding reviewed revisions + mock_find_reviewed.return_value = [ + {"sha1": "abc123", "max_reviewed_id": 150, "max_reviewable_id": 180, "page_id": 12345} + ] + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "approve") + self.assertIn("Revert to previously reviewed content", result["message"]) + self.assertIn("abc123", result["message"]) + + @patch("reviews.autoreview.checks.revert_detection._find_reviewed_revisions_by_sha1") + def test_revert_detection_block(self, mock_find_reviewed): + """Test revert detection when no reviewed content is found.""" + # Mock no reviewed revisions found + mock_find_reviewed.return_value = [] + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "block") + self.assertEqual( + result["message"], "Revert detected but no previously reviewed content found" + ) + + def test_revert_detection_no_reverted_ids(self): + """Test revert detection when no reverted revision IDs are found.""" + self.revision.change_tag_params = [] + self.revision.save() + + result = _check_revert_detection(self.revision, self.client) + + self.assertEqual(result["status"], "skip") + self.assertEqual(result["message"], "No reverted revision IDs found in change tags") + + def test_revert_detection_metadata(self): + """Test that revert detection returns proper metadata.""" + patch_path = "reviews.autoreview.checks.revert_detection._find_reviewed_revisions_by_sha1" + with patch(patch_path) as mock_find: + mock_find.return_value = [{"sha1": "abc123"}] + + result = _check_revert_detection(self.revision, self.client) + + self.assertIn("reverted_rev_ids", result["metadata"]) + self.assertIn("revert_tags", result["metadata"]) + self.assertIn("reviewed_revisions", result["metadata"]) + self.assertEqual(result["metadata"]["revert_tags"], ["mw-manual-revert"]) + + +class RevertDetectionIntegrationTests(TestCase): + """Integration tests for revert detection with real data.""" + + def setUp(self): + """Set up integration test data.""" + self.wiki = Wiki.objects.create( + name="Test Wiki", + code="test", + family="wikipedia", + api_endpoint="https://test.wikipedia.org/w/api.php", + ) + self.config = WikiConfiguration.objects.create(wiki=self.wiki) + + def test_revert_detection_with_real_revision(self): + """Test revert detection with a real revision setup.""" + page = PendingPage.objects.create( + wiki=self.wiki, + pageid=12345, + title="Test Page", + stable_revid=100, + ) + + # Create a revision with revert tags + revision = PendingRevision.objects.create( + page=page, + revid=200, + parentid=150, + user_name="TestUser", + user_id=1000, + timestamp=datetime.now(timezone.utc), + fetched_at=datetime.now(timezone.utc), + age_at_fetch=timedelta(hours=1), + sha1="test_sha1", + comment="Test revert", + change_tags=["mw-manual-revert", "mw-reverted"], + change_tag_params=[ + json.dumps( + { + "revertId": 200, + "oldestRevertedRevId": 180, + "newestRevertedRevId": 190, + "originalRevisionId": 175, + } + ) + ], + wikitext="", + ) + + # Mock the client + client = Mock(spec=WikiClient) + client.site = Mock() + + # Test with SupersetQuery mock + with patch("pywikibot.data.superset.SupersetQuery") as mock_superset: + mock_superset.return_value.query.return_value = [ + { + "content_sha1": "test_sha1", + "max_old_reviewed_id": 150, + "max_reviewable_rev_id_by_sha1": 180, + "rev_page": 12345, + } + ] + + result = _check_revert_detection(revision, client) + + self.assertEqual(result["status"], "approve") + self.assertIn("test_sha1", result["message"]) + self.assertEqual(len(result["metadata"]["reverted_rev_ids"]), 3) + self.assertEqual(len(result["metadata"]["revert_tags"]), 2) diff --git a/app/reviews/urls.py b/app/reviews/urls.py index 47c41fa4..c730ea8a 100644 --- a/app/reviews/urls.py +++ b/app/reviews/urls.py @@ -8,6 +8,15 @@ path("api/wikis/", views.api_wikis, name="api_wikis"), path("api/wikis//refresh/", views.api_refresh, name="api_refresh"), path("api/wikis//pending/", views.api_pending, name="api_pending"), + path("liftwing/", views.liftwing_page, name="liftwing"), + path("validate_article/", views.validate_article, name="validate_article"), + path("fetch_revisions/", views.fetch_revisions, name="fetch_revisions"), + path("fetch_predictions/", views.fetch_predictions, name="fetch_predictions"), + path( + "fetch_liftwing_predictions/", + views.fetch_liftwing_predictions, + name="fetch_liftwing_predictions", + ), path( "api/wikis//pages//revisions/", views.api_page_revisions, diff --git a/app/reviews/views.py b/app/reviews/views.py index 4a168477..3763e5ee 100644 --- a/app/reviews/views.py +++ b/app/reviews/views.py @@ -3,8 +3,10 @@ import json import logging from collections import defaultdict +from concurrent.futures import ThreadPoolExecutor, as_completed from datetime import datetime, timedelta from http import HTTPStatus +from urllib.parse import urlencode import requests from django.core.cache import cache @@ -34,6 +36,10 @@ logger = logging.getLogger(__name__) CACHE_TTL = 60 * 60 * 1 +# Constants for LiftWing feature +VALIDATION_TIMEOUT = 8 # seconds +USER_AGENT = "PendingChangesBot/1.0 (https://github.com/Wikimedia-Suomi/PendingChangesBot-ng)" + def calculate_percentile(values: list[float], percentile: float) -> float: """ @@ -661,6 +667,328 @@ def fetch_diff(request): return JsonResponse({"error": str(e)}, status=500) +def liftwing_page(request): + return render(request, "reviews/lift.html") + + +@csrf_exempt +def validate_article(request): + """ + POST JSON: { "wiki": , "article": "Page title" } + Response JSON: { "valid": bool, "exists": bool, "pageid": int|null, + "normalized_title": str|null, "missing": bool, "error": null|str } + """ + if request.method != "POST": + return JsonResponse({"error": "Invalid method"}, status=405) + + try: + payload = json.loads(request.body.decode("utf-8")) if request.body else {} + except Exception: + return JsonResponse({"error": "Invalid JSON body"}, status=400) + + article = payload.get("article") + wiki_payload = payload.get("wiki") + + if not article or not isinstance(article, str) or not article.strip(): + return JsonResponse({"valid": False, "error": "Empty article title"}, status=200) + + try: + wiki = _resolve_wiki_from_payload(wiki_payload) + except LookupError as e: + return JsonResponse({"valid": False, "error": str(e)}, status=400) + + api_endpoint = wiki.api_endpoint + if not api_endpoint: + return JsonResponse( + {"valid": False, "error": "Wiki has no configured api_endpoint"}, status=500 + ) + + params = { + "action": "query", + "format": "json", + "formatversion": 2, + "titles": article, + "redirects": 1, + "prop": "info", + } + + # Build query URL safely + if "?" not in api_endpoint: + query_url = f"{api_endpoint}?{urlencode(params)}" + else: + query_url = f"{api_endpoint}&{urlencode(params)}" + + headers = {"User-Agent": USER_AGENT} + try: + resp = requests.get(query_url, headers=headers, timeout=VALIDATION_TIMEOUT) + resp.raise_for_status() + except requests.RequestException as exc: + logger.exception("Failed to call MediaWiki API for validation: %s", exc) + return JsonResponse( + {"valid": False, "error": f"API request failed: {str(exc)}"}, + status=HTTPStatus.BAD_GATEWAY, + ) + + try: + data = resp.json() + except ValueError: + logger.error("MediaWiki API returned non-json for %s", query_url) + return JsonResponse( + {"valid": False, "error": "API returned invalid JSON"}, + status=HTTPStatus.BAD_GATEWAY, + ) + + query = data.get("query", {}) + pages = query.get("pages", []) + if not pages: + return JsonResponse({"valid": False, "error": "Unexpected API response"}, status=500) + + page = pages[0] + missing = bool(page.get("missing", False)) + normalized_title = page.get("title") + pageid = page.get("pageid") + + result = { + "valid": True, + "exists": not missing, + "missing": missing, + "pageid": pageid if pageid is not None else None, + "normalized_title": normalized_title, + "error": None, + } + return JsonResponse(result, status=200) + + +def _resolve_wiki_from_payload(wiki_value): + """ + Accept either integer pk, string code, or dictionary with 'id'/'code'. + Returns Wiki instance or raises LookupError. + """ + from .models import Wiki + + if wiki_value is None: + raise LookupError("Missing wiki parameter") + + # If a dict was passed (from frontend), try keys + if isinstance(wiki_value, dict): + if "id" in wiki_value: + try: + return Wiki.objects.get(pk=int(wiki_value["id"])) + except Exception: + raise LookupError(f"Unknown wiki id {wiki_value['id']}") + if "code" in wiki_value: + try: + return Wiki.objects.get(code=str(wiki_value["code"])) + except Exception: + raise LookupError(f"Unknown wiki code {wiki_value['code']}") + + # If numeric string or int -> assume pk + try: + pk = int(wiki_value) + try: + return Wiki.objects.get(pk=pk) + except Exception: # noqa: S110 - intentionally fallthrough + pass + except Exception: # noqa: S110 - intentionally fallthrough + pass + + # Otherwise assume code + try: + return Wiki.objects.get(code=str(wiki_value)) + except Exception: + raise LookupError(f"Unknown wiki identifier: {wiki_value!r}") + + +@csrf_exempt +def fetch_revisions(request): + if request.method != "POST": + return JsonResponse({"error": "Invalid method"}, status=405) + + try: + data = json.loads(request.body) + wiki = data.get("wiki", "en") + article = data.get("article", "") + + if not article: + return JsonResponse({"error": "Missing article parameter"}, status=400) + + base_url = f"https://{wiki}.wikipedia.org/w/api.php" + headers = {"User-Agent": USER_AGENT} + + params = { + "action": "query", + "prop": "revisions", + "titles": article, + "rvlimit": "max", + "rvprop": "ids|timestamp|user|comment", + "format": "json", + } + + revisions = [] + cont = True + cont_token = None + max_iterations = 10 # Prevent infinite loops + + while cont and max_iterations > 0: + if cont_token: + params["rvcontinue"] = cont_token + + try: + response = requests.get(base_url, params=params, headers=headers, timeout=10) + response.raise_for_status() + api_data = response.json() + except requests.exceptions.Timeout: + return JsonResponse({"error": "Request to Wikipedia API timed out"}, status=504) + except requests.exceptions.JSONDecodeError as e: + return JsonResponse( + {"error": f"Invalid JSON from Wikipedia API: {str(e)}"}, status=500 + ) + except requests.exceptions.RequestException as e: + return JsonResponse({"error": f"Wikipedia API error: {str(e)}"}, status=500) + + pages = api_data.get("query", {}).get("pages", {}) + for page_id, page_info in pages.items(): + revs = page_info.get("revisions", []) + revisions.extend(revs) + + cont_token = api_data.get("continue", {}).get("rvcontinue") + cont = bool(cont_token) + max_iterations -= 1 + + return JsonResponse({"title": article, "revisions": revisions}) + + except json.JSONDecodeError: + return JsonResponse({"error": "Invalid JSON in request body"}, status=400) + except Exception as e: + return JsonResponse({"error": f"Unexpected error: {str(e)}"}, status=500) + + +@csrf_exempt +def fetch_liftwing_predictions(request): + """ + POST JSON: { "wiki": "en", "model": "articlequality", "revisions": [12345, 67890] } + Response: { "predictions": { "12345": {...}, "67890": {...} } } + + Optimized to use concurrent requests with ThreadPoolExecutor for parallel processing. + Much faster than sequential requests. + """ + data = json.loads(request.body) + wiki = data.get("wiki", "en") + model = data.get("model", "articlequality") + revisions = data.get("revisions", []) + + if not revisions: + return JsonResponse({"error": "Missing revisions list"}, status=400) + + # Base URL for LiftWing API + base_url = ( + f"https://api.wikimedia.org/service/lw/inference/v1/models/{wiki}wiki-{model}/predict" + ) + headers = {"User-Agent": USER_AGENT} + + def fetch_single_prediction(rev_id): + """Fetch prediction for a single revision ID""" + try: + payload = {"rev_id": rev_id} + resp = requests.post(base_url, json=payload, headers=headers, timeout=15) + resp.raise_for_status() + result = resp.json() + return (rev_id, result.get("output", result)) + except requests.exceptions.Timeout: + return (rev_id, {"error": "Request timed out"}) + except requests.exceptions.HTTPError as e: + return (rev_id, {"error": f"HTTP {e.response.status_code}: {str(e)}"}) + except Exception as e: + return (rev_id, {"error": str(e)}) + + predictions = {} + + # Use ThreadPoolExecutor for parallel requests (max 10 concurrent) + with ThreadPoolExecutor(max_workers=10) as executor: + # Submit all tasks + future_to_rev = { + executor.submit(fetch_single_prediction, rev_id): rev_id for rev_id in revisions + } + + # Collect results as they complete + for future in as_completed(future_to_rev): + rev_id, prediction = future.result() + predictions[rev_id] = prediction + + return JsonResponse({"predictions": predictions}) + + +@csrf_exempt +def fetch_predictions(request): + """ + POST JSON: { "wiki": "en", "article": "Allu Arjun", "model": "articlequality" } + Calls the LiftWing API to fetch predictions for an article. + """ + if request.method != "POST": + return JsonResponse({"error": "Invalid method"}, status=405) + + try: + data = json.loads(request.body.decode("utf-8")) + except Exception: + return JsonResponse({"error": "Invalid JSON body"}, status=400) + + wiki = data.get("wiki", "en") + article = data.get("article", "") + model = data.get("model", "articlequality") + + if not article: + return JsonResponse({"error": "Missing article title"}, status=400) + + # LiftWing API endpoint for model inference + api_url = f"https://api.wikimedia.org/service/lw/inference/v1/models/{wiki}wiki-{model}/predict" + + # For simplicity, we fetch the latest revision ID of the article first + rev_api = f"https://{wiki}.wikipedia.org/w/api.php" + params = { + "action": "query", + "titles": article, + "prop": "revisions", + "rvlimit": 1, + "rvprop": "ids", + "format": "json", + } + headers = {"User-Agent": USER_AGENT} + try: + rev_resp = requests.get(rev_api, params=params, headers=headers, timeout=10) + rev_resp.raise_for_status() + rev_data = rev_resp.json() + pages = rev_data.get("query", {}).get("pages", {}) + rev_id = None + for page_id, page_info in pages.items(): + if "revisions" in page_info: + rev_id = page_info["revisions"][0]["revid"] + break + except Exception as e: + return JsonResponse({"error": f"Failed to fetch revision ID: {e}"}, status=500) + + if not rev_id: + return JsonResponse({"error": "No revision found for this article"}, status=404) + + # Call LiftWing API + payload = {"rev_id": rev_id} + + try: + response = requests.post(api_url, headers=headers, json=payload, timeout=10) + response.raise_for_status() + prediction = response.json() + return JsonResponse( + { + "wiki": wiki, + "article": article, + "rev_id": rev_id, + "model": model, + "prediction": prediction, + } + ) + except requests.RequestException as e: + return JsonResponse({"error": f"LiftWing request failed: {str(e)}"}, status=500) + + @require_GET def api_statistics(request: HttpRequest, pk: int) -> JsonResponse: """Get cached review statistics for a wiki.""" diff --git a/app/templates/reviews/lift.html b/app/templates/reviews/lift.html new file mode 100644 index 00000000..b35c9fc5 --- /dev/null +++ b/app/templates/reviews/lift.html @@ -0,0 +1,510 @@ + + + + +LiftWing Model Predictions - Revision History + + + + +

LiftWing Model Visualization - Revision History

+ +
+ + + + + + + + + + + +
+
+
+
+
+ +
+

Model Scores Over Revision History

+ +
+ +
+

Revision History

+ + + + + + + + + + + + +
#Revision IDTimestampUserCommentPrediction
+
+ + + + diff --git a/pyproject.toml b/pyproject.toml index 315eab8d..a2cbc778 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,6 +40,9 @@ dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" "*/tests/*" = ["S101", "S105", "S106"] "app/api/read_only_queries.py" = ["S608", "S607"] "app/reviews/services/wiki_client.py" = ["S608", "S607"] +"app/reviews/services/statistics.py" = ["S608", "S607"] +"app/reviews/management/commands/load_flaggedrevs_statistics.py" = ["S608", "S607"] +"app/reviews/autoreview/checks/revert_detection.py" = ["S608"] "app/review_statistics/services.py" = ["S608", "S607"] "app/review_statistics/management/commands/load_flaggedrevs_statistics.py" = ["S608", "S607"]