fix(analytics): stop /report and /duplicates hanging, and scope the ownership panels (#13602)

autobot-backend/api/codebase_analytics/duplicate_detector.py

@@ -26,7 +26,7 @@
 from dataclasses import dataclass, field
 from datetime import datetime, timezone
 from pathlib import Path
-from typing import Any, Dict, List, Set, Tuple
+from typing import Any, Callable, Dict, List, Set, Tuple
 
 from autobot_shared.logging_manager import get_logger
 
@@ -48,6 +48,7 @@
     SKIP_DIRS,
     TS_EXTENSIONS,
     VUE_EXTENSIONS,
+    is_skipped_path,
 )
 
 logger = get_logger(__name__)
@@ -104,6 +105,11 @@
 LSH_SIMILARITY_THRESHOLD = 0.5  # Minimum similarity for LSH candidate pairs
 LSH_ENABLED = True  # Set False to disable LSH and use O(n^2) fallback
 
+# Issue #13602: how often the long phases consult the cancel token. Checking
+# every item would cost more than the work between checks; every 500 bounds the
+# abandonment window to well under a second while staying free in the hot loop.
+CANCEL_CHECK_INTERVAL = 500
+
 
 # =============================================================================
 # Data Models
@@ -437,6 +443,7 @@ def _build_minhash(token_set: Set[str], num_perm: int = LSH_NUM_PERMUTATIONS) ->
 def _build_minhash_signatures(
     blocks: List[CodeBlock],
     num_perm: int,
+    cancelled: "Callable[[], bool] | None" = None,
 ) -> Dict[int, MinHash]:
     """
     Build MinHash signatures for all code blocks.
@@ -446,12 +453,19 @@ def _build_minhash_signatures(
     Args:
         blocks: List of code blocks
         num_perm: Number of MinHash permutations
+        cancelled: #13602 — polled so an abandoned scan stops here. This is the
+            phase that dominates the runtime, and it had no cancellation point
+            at all, so a caller that had already timed out still waited on one
+            of eight shared executor threads until it finished.
 
     Returns:
         Dictionary mapping block index to MinHash signature
     """
     minhashes: Dict[int, MinHash] = {}
     for idx, block in enumerate(blocks):
+        if cancelled is not None and idx % CANCEL_CHECK_INTERVAL == 0 and cancelled():
+            logger.info("MinHash signature build cancelled at %d/%d blocks", idx, len(blocks))
+            return minhashes
         token_set = block.token_set if block.token_set else set(block.normalized_content.split())
         if token_set:
             minhashes[idx] = _build_minhash(token_set, num_perm)
@@ -535,6 +549,7 @@ def _find_lsh_candidates(
     blocks: List[CodeBlock],
     threshold: float = LSH_SIMILARITY_THRESHOLD,
     num_perm: int = LSH_NUM_PERMUTATIONS,
+    cancelled: "Callable[[], bool] | None" = None,
 ) -> List[Tuple[int, int, float]]:
     """
     Find candidate duplicate pairs using Locality-Sensitive Hashing.
@@ -575,7 +590,9 @@ def _find_lsh_candidates(
     )
 
     # Phase 1: Build MinHash signatures - O(n) (Issue #665: uses helper)
-    minhashes = _build_minhash_signatures(blocks, num_perm)
+    minhashes = _build_minhash_signatures(blocks, num_perm, cancelled=cancelled)
+    if cancelled is not None and cancelled():
+        return []
 
     # Phase 2: Create LSH index - O(n) (Issue #665: uses helper)
     lsh = _build_lsh_index(minhashes, threshold, num_perm)
@@ -662,9 +679,14 @@ def _cancelled(self) -> bool:
         return self._cancel_token is not None and self._cancel_token.is_set()
 
     def _should_skip_path(self, path: Path) -> bool:
-        """Check if path should be skipped."""
-        path_parts = set(path.parts)
-        return bool(path_parts & SKIP_DIRS)
+        """Check if path should be skipped, relative to this scan's root.
+
+        #13602: this compared SKIP_DIRS against the ABSOLUTE path, so scanning a
+        root that itself sits under `.worktrees/` would have classified every
+        file below it as skippable. The method had no callers, so nothing broke
+        — it was a trap armed for whoever wired it in next.
+        """
+        return is_skipped_path(path, self.project_root)
 
     def _get_files_to_scan(self) -> List[Path]:
         """Get list of files to scan for duplicates.
@@ -709,9 +731,23 @@ def _get_files_to_scan(self) -> List[Path]:
         return files
 
     def _extract_all_blocks(self, files: List[Path]) -> List[CodeBlock]:
-        """Extract code blocks from all files (Issue #560: extracted)."""
+        """Extract code blocks from all files (Issue #560: extracted).
+
+        #13602: honours the cancel token. #12779 added the token but consulted
+        it only inside the os.walk loop — 0.14 s of a run whose remaining phases
+        measured over six minutes on a single tree. So a scan the caller had
+        abandoned still ran to completion holding one of eight executor threads,
+        which is the starvation that makes unrelated endpoints hang.
+        """
         all_blocks: List[CodeBlock] = []
-        for file_path in files:
+        for index, file_path in enumerate(files):
+            if index % CANCEL_CHECK_INTERVAL == 0 and self._cancelled():
+                logger.info(
+                    "Block extraction cancelled after %d/%d files",
+                    index,
+                    len(files),
+                )
+                return all_blocks
             try:
                 content = file_path.read_text(encoding="utf-8", errors="ignore")
                 blocks = _extract_code_blocks(str(file_path), content)
@@ -895,6 +931,7 @@ def _find_similar_duplicates(
                 all_blocks,
                 threshold=LSH_SIMILARITY_THRESHOLD,
                 num_perm=LSH_NUM_PERMUTATIONS,
+                cancelled=self._cancelled,
             )
 
             return self._process_lsh_candidates(all_blocks, lsh_candidates, seen_pairs)
@@ -944,6 +981,12 @@ def run_analysis(self) -> DuplicateAnalysis:
         # Extract all code blocks
         all_blocks = self._extract_all_blocks(files)
         logger.info("Extracted %d code blocks", len(all_blocks))
+        if self._cancelled():
+            # #13602: abandoned before the expensive similarity phases. Returning
+            # an empty analysis is honest — the caller has already stopped
+            # waiting — and it frees the executor thread the next request needs.
+            logger.info("Duplicate detection abandoned before similarity search")
+            return self._build_duplicate_analysis([], len(files))
 
         # Group blocks by hash for exact matches
         hash_groups: Dict[str, List[CodeBlock]] = defaultdict(list)
@@ -1198,6 +1241,12 @@ async def run_analysis_async(self) -> DuplicateAnalysis:
         # Extract all code blocks
         all_blocks = self._extract_all_blocks(files)
         logger.info("Extracted %d code blocks", len(all_blocks))
+        if self._cancelled():
+            # #13602: abandoned before the expensive similarity phases. Returning
+            # an empty analysis is honest — the caller has already stopped
+            # waiting — and it frees the executor thread the next request needs.
+            logger.info("Duplicate detection abandoned before similarity search")
+            return self._build_duplicate_analysis([], len(files))
 
         # Issue #665: Use helper to collect duplicates
         duplicates = await self._collect_all_duplicates(all_blocks)

autobot-backend/api/codebase_analytics/duplicate_scan_cancellation_12779_test.py

@@ -31,7 +31,13 @@ def _tree(root: Path) -> None:
     """Build a source tree with excluded dirs that must never be descended."""
     (root / "pkg").mkdir(parents=True)
     (root / "pkg" / "real.py").write_text("x = 1\n", encoding="utf-8")
-    for skipped in ("node_modules", ".git", "venv"):
+    # #13602: `.worktrees` and `.claude/worktrees` were NOT in SKIP_DIRS, despite
+    # _get_files_to_scan's own docstring claiming `.worktrees/` was pruned. On a
+    # real checkout that was 160,105 of 167,072 code files — the repo scanned ~26
+    # times over. This test passed throughout, because _tree() never built one.
+    (root / ".claude" / "worktrees" / "wt2" / "pkg").mkdir(parents=True)
+    (root / ".claude" / "worktrees" / "wt2" / "pkg" / "noise.py").write_text("z = 3\n", encoding="utf-8")
+    for skipped in ("node_modules", ".git", "venv", ".worktrees"):
         deep = root / skipped / "a" / "b" / "c"
         deep.mkdir(parents=True)
         (deep / "noise.py").write_text("y = 2\n", encoding="utf-8")
@@ -137,3 +143,187 @@ def test_lock_is_released_for_the_next_caller(self, monkeypatch):
         ep._duplicate_scan_lock.release()
         assert ep._duplicate_scan_lock.acquire(blocking=False)
         ep._duplicate_scan_lock.release()
+
+
+class TestScanRootInsideASkippedDirectory:
+    """#13602: adding `.worktrees` to SKIP_DIRS arms a trap.
+
+    Five call sites tested `SKIP_DIRS & set(path.parts)` on an ABSOLUTE path,
+    which asks whether a skip name appears anywhere in it — including in the
+    scan root. CI and development both run from inside `.worktrees/`, so the
+    moment the name was added, scanning from such a root would classify every
+    file below it as skippable and report an empty codebase. No error, no log,
+    just zero results — indistinguishable from a clean repo.
+
+    The root-relative check (#7128b's pattern) is what makes the name safe to
+    add at all.
+    """
+
+    def test_a_root_inside_worktrees_still_finds_its_own_files(self, tmp_path):
+        root = tmp_path / ".worktrees" / "issue-13602"
+        _tree(root)
+        found = DuplicateCodeDetector(project_root=str(root))._get_files_to_scan()
+        names = {p.name for p in found}
+        assert "real.py" in names, "a scan rooted inside .worktrees must not erase itself"
+        assert "noise.py" not in names, "nested skip dirs must still be pruned"
+
+    def test_should_skip_path_is_relative_to_the_scan_root(self, tmp_path):
+        """The dead method wired in (#13602). It had no callers, so its absolute
+        comparison was a trap armed for whoever wired it in next."""
+        root = tmp_path / ".worktrees" / "issue-13602"
+        (root / "pkg").mkdir(parents=True)
+        detector = DuplicateCodeDetector(project_root=str(root))
+
+        assert not detector._should_skip_path(root / "pkg" / "real.py")
+        assert detector._should_skip_path(root / ".worktrees" / "nested" / "x.py")
+        assert detector._should_skip_path(root / "node_modules" / "y.py")
+
+
+class TestCancellationReachesTheExpensivePhases:
+    """#13602: the token was consulted in exactly one place — the os.walk loop.
+
+    That loop measured 0.14s of a run whose remaining phases took over six
+    minutes on a single tree (105,441 blocks, still building MinHash signatures
+    at 6m15s). So a scan the caller had abandoned still ran to completion,
+    holding one of eight shared executor threads. That starvation is what made
+    unrelated endpoints hang.
+
+    The review of #14014 found these checks shipped with NO test — the whole
+    detector half could be deleted and the suite stayed green. These are that
+    test: each asserts the phase stops, not merely that a check exists.
+    """
+
+    def _blocks(self, count):
+        from api.codebase_analytics.duplicate_detector import CodeBlock
+
+        return [
+            CodeBlock(
+                file_path=f"/tmp/f{i}.py",
+                start_line=1,
+                end_line=5,
+                content=f"def f{i}(): pass",
+                normalized_content=f"def f{i} pass",
+                content_hash=f"h{i}",
+                line_count=5,
+                token_set={f"tok{i}", "def", "pass"},
+            )
+            for i in range(count)
+        ]
+
+    def test_block_extraction_stops_when_cancelled(self, tmp_path, monkeypatch):
+        import api.codebase_analytics.duplicate_detector as det
+
+        files = []
+        for i in range(det.CANCEL_CHECK_INTERVAL * 3):
+            f = tmp_path / f"m{i}.py"
+            f.write_text("x = 1\n", encoding="utf-8")
+            files.append(f)
+
+        token = threading.Event()
+        token.set()
+        detector = det.DuplicateCodeDetector(project_root=str(tmp_path), cancel_token=token)
+
+        read_count = {"n": 0}
+        real_read = Path.read_text
+
+        def _counting_read(self, *args, **kwargs):
+            read_count["n"] += 1
+            return real_read(self, *args, **kwargs)
+
+        monkeypatch.setattr(Path, "read_text", _counting_read)
+        detector._extract_all_blocks(files)
+
+        assert read_count["n"] < len(files), "extraction read every file despite being cancelled"
+
+    def test_minhash_signature_build_stops_when_cancelled(self):
+        """This is the phase that dominates the runtime, and it had no
+        cancellation point at all."""
+        import api.codebase_analytics.duplicate_detector as det
+
+        pytest.importorskip("datasketch")
+        blocks = self._blocks(det.CANCEL_CHECK_INTERVAL * 3)
+
+        signatures = det._build_minhash_signatures(blocks, num_perm=16, cancelled=lambda: True)
+
+        assert len(signatures) < len(blocks), "signature build processed every block despite cancellation"
+
+    def test_minhash_build_completes_when_not_cancelled(self):
+        """The case that must stay working — a cancellation check that always
+        fires would pass the test above while breaking every real scan."""
+        import api.codebase_analytics.duplicate_detector as det
+
+        pytest.importorskip("datasketch")
+        blocks = self._blocks(10)
+
+        signatures = det._build_minhash_signatures(blocks, num_perm=16, cancelled=lambda: False)
+
+        assert len(signatures) == len(blocks)
+
+    def test_the_lsh_phase_is_given_the_token(self, tmp_path, monkeypatch):
+        """Threading the predicate through is the wiring; without it the checks
+        inside are unreachable."""
+        import api.codebase_analytics.duplicate_detector as det
+
+        seen: dict[str, object] = {}
+
+        def _spy(blocks, threshold=0.5, num_perm=128, cancelled=None):
+            seen["cancelled"] = cancelled
+            return []
+
+        monkeypatch.setattr(det, "_find_lsh_candidates", _spy)
+        monkeypatch.setattr(det, "LSH_AVAILABLE", True)
+        detector = det.DuplicateCodeDetector(project_root=str(tmp_path), cancel_token=threading.Event())
+        detector._find_similar_duplicates(self._blocks(20), set())
+
+        assert seen["cancelled"] is not None, "the LSH phase was never given the cancel predicate"
+        assert seen["cancelled"]() is False
+        detector._cancel_token.set()
+        assert seen["cancelled"]() is True, "the predicate must reflect the live token"
+
+    def test_a_scan_cancelled_after_the_walk_skips_the_similarity_phases(self, tmp_path, monkeypatch):
+        """The phase boundary. Without it, a token set during extraction still
+        let the six-minute similarity search run to completion."""
+        import api.codebase_analytics.duplicate_detector as det
+
+        (tmp_path / "a.py").write_text("def a(): pass\n", encoding="utf-8")
+        token = threading.Event()
+        detector = det.DuplicateCodeDetector(project_root=str(tmp_path), cancel_token=token)
+
+        called = {"similar": False}
+
+        def _never(*_args, **_kwargs):
+            called["similar"] = True
+            return []
+
+        monkeypatch.setattr(detector, "_find_similar_duplicates", _never)
+        monkeypatch.setattr(detector, "_extract_all_blocks", lambda files: (token.set(), [])[1])
+
+        result = detector.run_analysis()
+
+        assert not called["similar"], "the similarity search ran on an abandoned scan"
+        assert result.total_duplicates == 0
+
+    @pytest.mark.asyncio
+    async def test_the_async_path_has_the_same_boundary(self, tmp_path, monkeypatch):
+        """`run_analysis` and `run_analysis_async` are separate implementations
+        and the check had to be added to both. Guarding only the sync one would
+        leave the semantic-analysis path running abandoned scans to completion."""
+        import api.codebase_analytics.duplicate_detector as det
+
+        (tmp_path / "a.py").write_text("def a(): pass\n", encoding="utf-8")
+        token = threading.Event()
+        detector = det.DuplicateCodeDetector(project_root=str(tmp_path), cancel_token=token)
+
+        called = {"collected": False}
+
+        async def _never(*_args, **_kwargs):
+            called["collected"] = True
+            return []
+
+        monkeypatch.setattr(detector, "_collect_all_duplicates", _never)
+        monkeypatch.setattr(detector, "_extract_all_blocks", lambda files: (token.set(), [])[1])
+
+        result = await detector.run_analysis_async()
+
+        assert not called["collected"], "the async path ran the expensive phases on an abandoned scan"
+        assert result.total_duplicates == 0