From 2db8b7a3a3827282a17584b6224daa01ab427f55 Mon Sep 17 00:00:00 2001 From: Original Gary <276612211+OpenGaryBot@users.noreply.github.com> Date: Thu, 14 May 2026 07:36:29 +1000 Subject: [PATCH 1/2] fix: root-cause CI failures inherited from sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #38 merged with 5 failing tests. Each had a real root cause now fixed, verified locally before push. 1. tree-sitter-language-pack 1.6.3 breaks 3 bash detector tests 1.6.3 (released 2026-04-23) is a deliberate breaking refactor by upstream (kreuzberg-dev): the public module moved from `tree_sitter_language_pack` to `_native`, removing the top-level package that desloppify's parser bootstrap imports. ImportError gets caught by PARSE_INIT_ERRORS in analysis/unused_imports.py and returns empty findings, masking the regression as test assertion failures rather than import failures. Verified: on a clean Python 3.12 venv with 1.6.3, all 3 bash tests fail with `[] == ['helpers']` etc. Pin to 1.6.2 → all 5 bash tests pass. Fix: cap at <1.6.3 in pyproject.toml (treesitter + full extras) and update test_treesitter_language_pack_is_capped_below_incompatible_release to assert the new cap. 2. test_successful_dedicated_install_rovodev: missing _read_local_docs_file mock OP's update_skill/cmd.py prefers local docs/ files over the download path (sensible: local-first when running from a checkout). Test mocked only `_download`, so _read_local_docs_file('SKILL.md') and ('ROVODEV.md') returned real local content (the actual ROVODEV.md shipping with the package), bypassing the mock. The test class already had this mock pattern on test_download_failure and test_bad_content; the three "successful_install" tests were inconsistent. Added @patch on `_read_local_docs_file, return_value=None` to all three to consistently exercise the download path the tests intend to test. Verified all 25 tests in the file pass. 3. test_show_structural_loads_medium_confidence_matches: scope.py regression The sync's per-file restoration of OP versions reverted scope.py's load_matches to a stale variant that routes everything through build_work_queue, which applies standalone_threshold filtering (medium structural < high threshold → filtered). Upstream's evolved load_matches has an explicit "show should surface persisted matching issues even when a detector has a higher standalone confidence threshold" path that calls build_issue_items directly with forced_ids, bypassing the filter. OP's only difference in scope.py was a docstring example mentioning advocacy_language — not load-bearing code. Upstream's load_matches handles advocacy scopes identically. Fix: take upstream's scope.py + revert engine/work_queue.py to upstream (no longer needs build_work_queue_for_visibility export, which only OP's old scope.py used). Verified all 42 tests in test_cmd_show.py pass. Excluded from this PR: ~30 additional latent failures visible in Python 3.12 local runs that don't appear in CI on Python 3.11. Separate triage. --- desloppify/app/commands/show/scope.py | 74 ++++++++++++------- desloppify/engine/work_queue.py | 4 - .../test_transitive_modules_update_skill.py | 9 ++- .../test_pyproject_optional_dependencies.py | 2 +- pyproject.toml | 12 ++- 5 files changed, 63 insertions(+), 38 deletions(-) diff --git a/desloppify/app/commands/show/scope.py b/desloppify/app/commands/show/scope.py index b2ad8f459..4c84bf553 100644 --- a/desloppify/app/commands/show/scope.py +++ b/desloppify/app/commands/show/scope.py @@ -15,6 +15,8 @@ QueueBuildOptions, build_work_queue, ) +from desloppify.engine._work_queue.helpers import scope_matches +from desloppify.engine._work_queue.ranking import build_issue_items @dataclass(frozen=True) @@ -197,39 +199,55 @@ def load_matches( status_filter: str, chronic: bool, ) -> list[dict[str, Any]]: - """Load matching issues from the ranked queue. + """Load matching issues for an exploratory show query. - When a scope is provided, search both execution and backlog partitions - so scoped queries (e.g. ``show advocacy_language``) surface findings - even when a different lifecycle phase (like initial review) owns the - execution partition. + Unlike execution queues, show should surface persisted matching issues even + when a detector has a higher standalone confidence threshold. """ - from desloppify.engine.work_queue import build_work_queue_for_visibility, QueueVisibility - - base_opts = QueueBuildOptions( - count=None, + issue_map = state.get("work_items") or state.get("issues", {}) + if not isinstance(issue_map, dict) or not issue_map: + queue = build_work_queue( + state, + options=QueueBuildOptions( + count=None, + scope=scope, + status=status_filter, + include_subjective=False, + chronic=chronic, + ), + ) + return [item for item in queue.get("items", []) if item.get("kind") == "issue"] + return build_issue_items( + state, + scan_path=state.get("scan_path"), + status_filter=status_filter, scope=scope, - status=status_filter, - include_subjective=False, chronic=chronic, + forced_ids=_matching_issue_ids_for_scope(state, scope), ) - queue = build_work_queue(state, options=base_opts) - matches = [ - item for item in queue.get("items", []) if item.get("kind") == "issue" - ] - # When scoped and execution partition returned nothing, check backlog. - if not matches and scope and status_filter == "open": - backlog_queue = build_work_queue_for_visibility( - state, - options=base_opts, - visibility=QueueVisibility.BACKLOG, - ) - matches = [ - item - for item in backlog_queue.get("items", []) - if item.get("kind") == "issue" - ] - return matches + + +def _matching_issue_ids_for_scope( + state: StateModel, + scope: str | None, +) -> set[str]: + """Return persisted IDs matching a show scope, bypassing queue thresholds.""" + issue_map = state.get("work_items") or state.get("issues", {}) + if not isinstance(issue_map, dict): + return set() + if not scope: + return {issue_id for issue_id in issue_map if isinstance(issue_id, str)} + + matched: set[str] = set() + for issue_id, issue in issue_map.items(): + if not isinstance(issue_id, str) or not isinstance(issue, dict): + continue + item = dict(issue) + item["id"] = issue_id + item.setdefault("kind", "issue") + if scope_matches(item, scope): + matched.add(issue_id) + return matched def resolve_noise( diff --git a/desloppify/engine/work_queue.py b/desloppify/engine/work_queue.py index 7ad4ed9e8..ea2ab07ad 100644 --- a/desloppify/engine/work_queue.py +++ b/desloppify/engine/work_queue.py @@ -9,11 +9,9 @@ from desloppify.engine._work_queue.core import ( QueueBuildOptions, WorkQueueResult, - _build_work_queue_with_visibility as build_work_queue_for_visibility, build_work_queue, ) from desloppify.engine._work_queue.issues import list_open_review_issues -from desloppify.engine._work_queue.models import QueueVisibility from desloppify.engine._work_queue.ranking import group_queue_items from desloppify.engine._work_queue.synthetic_workflow import ( build_deferred_disposition_item, @@ -21,11 +19,9 @@ __all__ = [ "QueueBuildOptions", - "QueueVisibility", "WorkQueueResult", "build_deferred_disposition_item", "build_work_queue", - "build_work_queue_for_visibility", "group_queue_items", "list_open_review_issues", ] diff --git a/desloppify/tests/commands/test_transitive_modules_update_skill.py b/desloppify/tests/commands/test_transitive_modules_update_skill.py index 3e506b6a0..41a259693 100644 --- a/desloppify/tests/commands/test_transitive_modules_update_skill.py +++ b/desloppify/tests/commands/test_transitive_modules_update_skill.py @@ -209,8 +209,9 @@ def test_bad_content(self, mock_download, _mock_local, _mock_colorize, capsys): assert "doesn't look like a skill document" in out @patch("desloppify.app.commands.update_skill.colorize", side_effect=lambda t, _c: t) + @patch("desloppify.app.commands.update_skill.cmd._read_local_docs_file", return_value=None) @patch("desloppify.app.commands.update_skill._download") - def test_successful_dedicated_install(self, mock_download, _mock_colorize, capsys, tmp_path): + def test_successful_dedicated_install(self, mock_download, _mock_local, _mock_colorize, capsys, tmp_path): skill_content = "# Skill\n\nContent" mock_download.side_effect = lambda f: { "SKILL.md": skill_content, @@ -230,9 +231,10 @@ def test_successful_dedicated_install(self, mock_download, _mock_colorize, capsy assert "Updated" in out @patch("desloppify.app.commands.update_skill.colorize", side_effect=lambda t, _c: t) + @patch("desloppify.app.commands.update_skill.cmd._read_local_docs_file", return_value=None) @patch("desloppify.app.commands.update_skill._download") def test_successful_dedicated_install_rovodev( - self, mock_download, _mock_colorize, capsys, tmp_path + self, mock_download, _mock_local, _mock_colorize, capsys, tmp_path ): """Per-project `update-skill rovodev` writes the dedicated `.rovodev/...` file.""" skill_content = "# Skill\n\nContent" @@ -257,8 +259,9 @@ def test_successful_dedicated_install_rovodev( assert "Updated" in out @patch("desloppify.app.commands.update_skill.colorize", side_effect=lambda t, _c: t) + @patch("desloppify.app.commands.update_skill.cmd._read_local_docs_file", return_value=None) @patch("desloppify.app.commands.update_skill._download") - def test_successful_shared_install(self, mock_download, _mock_colorize, capsys, tmp_path): + def test_successful_shared_install(self, mock_download, _mock_local, _mock_colorize, capsys, tmp_path): """Non-dedicated install (e.g. windsurf) replaces section in existing file.""" skill_content = "# Skill\n\nContent" mock_download.side_effect = lambda f: { diff --git a/desloppify/tests/core/test_pyproject_optional_dependencies.py b/desloppify/tests/core/test_pyproject_optional_dependencies.py index 5e0ed763d..03b0d7792 100644 --- a/desloppify/tests/core/test_pyproject_optional_dependencies.py +++ b/desloppify/tests/core/test_pyproject_optional_dependencies.py @@ -62,4 +62,4 @@ def test_treesitter_language_pack_is_capped_below_incompatible_release() -> None if str(spec).startswith("tree-sitter-language-pack") ] - assert language_pack_specs == ["tree-sitter-language-pack>=0.3,<1.8"] + assert language_pack_specs == ["tree-sitter-language-pack>=0.3,<1.6.3"] diff --git a/pyproject.toml b/pyproject.toml index a54e1b99f..b7e04dbc4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,11 @@ Issues = "https://github.com/peteromallet/desloppify/issues" [project.optional-dependencies] treesitter = [ "tree-sitter>=0.21", - "tree-sitter-language-pack>=0.3,<1.8", + # 1.6.3 was a breaking refactor by upstream: the public module moved from + # `tree_sitter_language_pack` to `_native`, removing the top-level package + # that desloppify's parser bootstrap imports. Pin out until the API is + # restored (or desloppify migrates to the new import path). + "tree-sitter-language-pack>=0.3,<1.6.3", ] csharp-xml = ["defusedxml>=0.7.0"] python-security = ["bandit>=1.7.8"] @@ -45,7 +49,11 @@ plan-yaml = ["PyYAML>=6.0"] full = [ "defusedxml>=0.7.0", "tree-sitter>=0.21", - "tree-sitter-language-pack>=0.3,<1.8", + # 1.6.3 was a breaking refactor by upstream: the public module moved from + # `tree_sitter_language_pack` to `_native`, removing the top-level package + # that desloppify's parser bootstrap imports. Pin out until the API is + # restored (or desloppify migrates to the new import path). + "tree-sitter-language-pack>=0.3,<1.6.3", "bandit>=1.7.8", "Pillow>=9.0.0", "PyYAML>=6.0", From be045637adbc656c65a2b54c437d4d8f7eda99fa Mon Sep 17 00:00:00 2001 From: Original Gary <276612211+OpenGaryBot@users.noreply.github.com> Date: Thu, 14 May 2026 07:41:19 +1000 Subject: [PATCH 2/2] fix: skip bash_unused_imports tests when tree-sitter unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests-core runs via `make tests` → `install-ci-tools` which installs the core package without [full] extras (no tree-sitter-language-pack). The new test_bash_unused_imports.py module (came in via the sync from upstream) lacked the skipif guard that every other test_treesitter module uses: pytestmark = pytest.mark.skipif( not is_available(), reason="tree-sitter-language-pack not installed", ) Without the guard, the parser-init catch in detect_unused_imports returns empty findings on missing parser, and the test assertions fail with "assert [] == ['helpers']" — exactly the symptom the previous commit's 1.6.3 pin fixed for tests-full but couldn't fix for tests-core (parser not installed at all there). Verified locally: tests still pass when tree-sitter is installed; will skip cleanly when it isn't. --- .../tests/lang/common/test_bash_unused_imports.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/desloppify/tests/lang/common/test_bash_unused_imports.py b/desloppify/tests/lang/common/test_bash_unused_imports.py index c74dee941..cdc9991b3 100644 --- a/desloppify/tests/lang/common/test_bash_unused_imports.py +++ b/desloppify/tests/lang/common/test_bash_unused_imports.py @@ -4,6 +4,16 @@ import textwrap +import pytest + +from desloppify.languages._framework.treesitter import is_available + +# Skip all tests if tree-sitter-language-pack is not installed. +# Matches the guard pattern used by other test_treesitter modules. +pytestmark = pytest.mark.skipif( + not is_available(), reason="tree-sitter-language-pack not installed" +) + def _detect(tmp_path, contents: str): from desloppify.languages._framework.treesitter.analysis.unused_imports import (