Merge pull request #438 from iliaal/refactor/github-search-parse-split

skills/last30days/scripts/lib/github.py

@@ -62,6 +62,17 @@ def _resolve_token(token: Optional[str] = None) -> Optional[str]:
     return None
 
 
+def resolve_token(token: Optional[str] = None) -> Optional[str]:
+    """Public alias for ``_resolve_token``.
+
+    The pipeline calls this once before ``search_github`` and
+    ``enrich_with_comments`` so the ``gh auth token`` subprocess fallback
+    only fires once per query when ``GITHUB_TOKEN`` is unset, instead of
+    twice (once per call site).
+    """
+    return _resolve_token(token)
+
+
 def _fetch_json(
     url: str,
     token: Optional[str] = None,
@@ -142,8 +153,14 @@ def search_github(
     to_date: str,
     depth: str = "default",
     token: Optional[str] = None,
-) -> List[Dict[str, Any]]:
+) -> Dict[str, Any]:
-    """Search GitHub Issues and PRs.
+    """Search GitHub Issues and PRs (HTTP fetch only).
+
+    Returns a raw envelope shaped like every other adapter's ``search_X``:
+    ``{"items": [raw GitHub API items], "context": {core, from_date,
+    to_date, count}}``. Normalization, date filtering, and sorting move
+    to ``parse_github_response``; comment enrichment moves to
+    ``enrich_with_comments``.
 
     Args:
         topic: Search topic
@@ -153,15 +170,23 @@ def search_github(
         token: Optional GitHub token (falls back to env/gh CLI)
 
     Returns:
-        List of normalized item dicts. Empty list on any failure.
+        Dict envelope. Empty ``items`` list on any failure.
     """
+    count = DEPTH_LIMITS.get(depth, DEPTH_LIMITS["default"])
+    core = extract_core_subject(topic)
     resolved_token = _resolve_token(token)
     if not resolved_token:
         _log("No GitHub token available (set GITHUB_TOKEN or install gh CLI)")
-        return []
+        return {
-
+            "items": [],
-    count = DEPTH_LIMITS.get(depth, DEPTH_LIMITS["default"])
+            "error": "no token",
-    core = extract_core_subject(topic)
+            "context": {
+                "core": core,
+                "from_date": from_date,
+                "to_date": to_date,
+                "count": count,
+            },
+        }
     _log(f"Searching for '{core}' (raw: '{topic}', since {from_date}, count={count})")
 
     # Build search query with date filter
@@ -176,12 +201,41 @@ def search_github(
 
     data = _fetch_json(url, token=resolved_token, timeout=30)
     if not data:
-        return []
+        return {"items": [], "context": {"core": core, "from_date": from_date,
+                                          "to_date": to_date, "count": count}}
 
     raw_items = data.get("items", [])
     _log(f"Found {len(raw_items)} issues/PRs")
 
-    items = []
+    return {
+        "items": raw_items,
+        "context": {
+            "core": core,
+            "from_date": from_date,
+            "to_date": to_date,
+            "count": count,
+        },
+    }
+
+
+def parse_github_response(response: Dict[str, Any]) -> List[Dict[str, Any]]:
+    """Normalize a ``search_github`` envelope into the skill's item shape.
+
+    Pure function: no I/O, no token, no enrichment. Applies the date
+    filter using the search context and sorts by relevance.
+    """
+    if not isinstance(response, dict):
+        return []
+    raw_items = response.get("items") or []
+    if not isinstance(raw_items, list):
+        return []
+    context = response.get("context") or {}
+    core = context.get("core") or ""
+    from_date = context.get("from_date") or ""
+    to_date = context.get("to_date") or ""
+    count = context.get("count") or DEPTH_LIMITS["default"]
+
+    items: List[Dict[str, Any]] = []
     for i, item in enumerate(raw_items[:count]):
         html_url = item.get("html_url", "")
         repo = _parse_repo_from_url(html_url)
@@ -224,20 +278,34 @@ def search_github(
             },
         })
 
-    # Enrich top items with comments
-    items = _enrich_top_items(items, depth, resolved_token)
-
     # Date filter
-    filtered = []
+    if from_date and to_date:
-    for item in items:
+        items = [
-        d = item.get("date")
+            item for item in items
-        if d is None or (from_date <= d <= to_date):
+            if item.get("date") is None or (from_date <= item["date"] <= to_date)
-            filtered.append(item)
+        ]
 
-    # Sort by relevance
+    items.sort(key=lambda x: x.get("relevance", 0), reverse=True)
-    filtered.sort(key=lambda x: x.get("relevance", 0), reverse=True)
+    return items
 
-    return filtered
+
+def enrich_with_comments(
+    items: List[Dict[str, Any]],
+    depth: str = "default",
+    token: Optional[str] = None,
+) -> List[Dict[str, Any]]:
+    """Fetch top comments for top-K items by reactions and attach to metadata.
+
+    Mutates and returns ``items``. Resolves ``token`` via env/gh CLI when
+    not supplied, matching ``search_github``'s fallback chain.
+    """
+    if not items:
+        return items
+    resolved_token = _resolve_token(token)
+    if not resolved_token:
+        _log("No GitHub token available for comment enrichment")
+        return items
+    return _enrich_top_items(items, depth, resolved_token)
 
 
 def _enrich_top_items(

skills/last30days/scripts/lib/pipeline.py

@@ -1007,8 +1007,14 @@ def _retrieve_stream(
         result = polymarket.search_polymarket(subquery.search_query, from_date, to_date, depth=depth)
         return polymarket.parse_polymarket_response(result, topic=subquery.search_query), {}
     if source == "github":
-        result = github.search_github(subquery.search_query, from_date, to_date, depth=depth, token=config.get("GITHUB_TOKEN"))
+        # Resolve once at the pipeline boundary so search and enrich
-        return result, {}
+        # share the result; otherwise each call would re-run the env
+        # lookup and gh-CLI subprocess fallback (up to 5s timeout each).
+        token = github.resolve_token(config.get("GITHUB_TOKEN"))
+        response = github.search_github(subquery.search_query, from_date, to_date, depth=depth, token=token)
+        items = github.parse_github_response(response)
+        items = github.enrich_with_comments(items, depth=depth, token=token)
+        return items, {}
     if source == "pinterest":
         result = pinterest.search_pinterest(
             subquery.search_query, from_date, to_date,

tests/test_github.py

@@ -73,13 +73,29 @@ class TestParseDate(unittest.TestCase):
 class TestSearchGithub(unittest.TestCase):
     @patch.dict("os.environ", {}, clear=True)
     @patch("subprocess.run", side_effect=FileNotFoundError)
-    def test_no_token_returns_empty(self, mock_run):
+    def test_no_token_returns_empty_envelope(self, mock_run):
         result = github.search_github("react", "2026-03-01", "2026-03-31", token=None)
-        self.assertEqual(result, [])
+        self.assertEqual(result.get("items", []), [])
+        self.assertIn("error", result)
+        # Envelope shape must match the fetch-failure path: context is
+        # always present so callers (and parse_github_response) can read
+        # diagnostic fields without branching on which failure mode hit.
+        self.assertIn("context", result)
+        self.assertEqual(result["context"]["from_date"], "2026-03-01")
+        self.assertEqual(result["context"]["to_date"], "2026-03-31")
+
+    def test_resolve_token_public_alias(self):
+        """resolve_token is the public entry point pipeline uses; _resolve_token stays
+        private. Both should return the same value for the same input."""
+        self.assertEqual(
+            github.resolve_token("explicit-token"),
+            github._resolve_token("explicit-token"),
+        )
+        self.assertEqual(github.resolve_token("explicit-token"), "explicit-token")
 
     @patch.object(github, "_fetch_json")
     @patch.object(github, "_resolve_token", return_value="test-token")
-    def test_search_returns_items(self, mock_token, mock_fetch):
+    def test_search_returns_raw_envelope(self, mock_token, mock_fetch):
         mock_fetch.return_value = {
             "total_count": 1,
             "items": [
@@ -96,9 +112,15 @@ class TestSearchGithub(unittest.TestCase):
                 },
             ],
         }
-        result = github.search_github("react", "2026-03-01", "2026-03-31")
+        # Search returns raw envelope; parse normalizes.
-        self.assertEqual(len(result), 1)
+        response = github.search_github("react", "2026-03-01", "2026-03-31")
-        item = result[0]
+        self.assertEqual(len(response["items"]), 1)
+        self.assertEqual(response["items"][0]["title"], "React Server Components bug")
+        self.assertEqual(response["context"]["from_date"], "2026-03-01")
+
+        items = github.parse_github_response(response)
+        self.assertEqual(len(items), 1)
+        item = items[0]
         self.assertEqual(item["source"], "github")
         self.assertEqual(item["container"], "facebook/react")
         self.assertEqual(item["title"], "React Server Components bug")
@@ -114,10 +136,11 @@ class TestSearchGithub(unittest.TestCase):
 
     @patch.object(github, "_fetch_json", return_value=None)
     @patch.object(github, "_resolve_token", return_value="test-token")
-    def test_rate_limit_returns_empty(self, mock_token, mock_fetch):
+    def test_rate_limit_returns_empty_envelope(self, mock_token, mock_fetch):
-        """403 rate limit returns empty list gracefully."""
+        """403 rate limit returns envelope with empty items list."""
-        result = github.search_github("react", "2026-03-01", "2026-03-31")
+        response = github.search_github("react", "2026-03-01", "2026-03-31")
-        self.assertEqual(result, [])
+        self.assertEqual(response["items"], [])
+        self.assertEqual(github.parse_github_response(response), [])
 
     @patch.object(github, "_fetch_json")
     @patch.object(github, "_resolve_token", return_value="test-token")
@@ -139,9 +162,107 @@ class TestSearchGithub(unittest.TestCase):
                 },
             ],
         }
-        result = github.search_github("next.js", "2026-03-01", "2026-03-31")
+        response = github.search_github("next.js", "2026-03-01", "2026-03-31")
-        self.assertEqual(len(result), 1)
+        items = github.parse_github_response(response)
-        self.assertTrue(result[0]["metadata"]["is_pr"])
+        self.assertEqual(len(items), 1)
+        self.assertTrue(items[0]["metadata"]["is_pr"])
+
+
+class TestParseGithubResponse(unittest.TestCase):
+    """Fixture-driven parse tests: feed a synthetic search_github envelope to
+    parse_github_response and assert normalized output.
+
+    This contract (search returns dict envelope, parse turns it into a list)
+    matches every other source adapter. Before this refactor, search_github
+    returned a bare list and there was no parse step, blocking fixture tests.
+    """
+
+    _RAW_ENVELOPE = {
+        "items": [
+            {
+                "html_url": "https://github.com/facebook/react/issues/42",
+                "title": "React Server Components bug",
+                "body": "There is a bug when using RSC with streaming...",
+                "created_at": "2026-03-15T10:00:00Z",
+                "state": "open",
+                "comments": 12,
+                "reactions": {"total_count": 8},
+                "labels": [{"name": "bug"}, {"name": "rsc"}],
+                "user": {"login": "testuser"},
+            },
+            {
+                "html_url": "https://github.com/vercel/next.js/pull/99",
+                "title": "Add streaming support",
+                "body": "This PR adds...",
+                "created_at": "2026-03-20T10:00:00Z",
+                "state": "open",
+                "comments": 5,
+                "reactions": {"total_count": 3},
+                "labels": [],
+                "user": {"login": "dev"},
+                "pull_request": {"url": "..."},
+            },
+        ],
+        "context": {
+            "core": "react",
+            "from_date": "2026-03-01",
+            "to_date": "2026-03-31",
+            "count": 25,
+        },
+    }
+
+    def test_normalizes_items(self):
+        items = github.parse_github_response(self._RAW_ENVELOPE)
+        self.assertEqual(len(items), 2)
+        by_url = {i["url"]: i for i in items}
+        issue = by_url["https://github.com/facebook/react/issues/42"]
+        self.assertEqual(issue["source"], "github")
+        self.assertEqual(issue["container"], "facebook/react")
+        self.assertEqual(issue["title"], "React Server Components bug")
+        self.assertEqual(issue["date"], "2026-03-15")
+        self.assertEqual(issue["author"], "testuser")
+        self.assertEqual(issue["engagement"]["reactions"], 8)
+        self.assertEqual(issue["engagement"]["comments"], 12)
+        self.assertFalse(issue["metadata"]["is_pr"])
+
+    def test_detects_pr(self):
+        items = github.parse_github_response(self._RAW_ENVELOPE)
+        pr = next(i for i in items if "/pull/" in i["url"])
+        self.assertTrue(pr["metadata"]["is_pr"])
+
+    def test_date_filter_drops_outside_window(self):
+        envelope = {
+            "items": [
+                {
+                    "html_url": "https://github.com/foo/bar/issues/1",
+                    "title": "Too old",
+                    "created_at": "2026-01-15T10:00:00Z",
+                    "comments": 0, "reactions": {"total_count": 0},
+                    "labels": [], "user": {"login": "x"},
+                },
+                {
+                    "html_url": "https://github.com/foo/bar/issues/2",
+                    "title": "In window",
+                    "created_at": "2026-03-15T10:00:00Z",
+                    "comments": 0, "reactions": {"total_count": 0},
+                    "labels": [], "user": {"login": "x"},
+                },
+            ],
+            "context": {"core": "foo", "from_date": "2026-03-01",
+                        "to_date": "2026-03-31", "count": 25},
+        }
+        items = github.parse_github_response(envelope)
+        self.assertEqual(len(items), 1)
+        self.assertEqual(items[0]["title"], "In window")
+
+    def test_sorts_by_relevance(self):
+        items = github.parse_github_response(self._RAW_ENVELOPE)
+        scores = [i.get("relevance", 0) for i in items]
+        self.assertEqual(scores, sorted(scores, reverse=True))
+
+    def test_empty_envelope(self):
+        self.assertEqual(github.parse_github_response({"items": []}), [])
+        self.assertEqual(github.parse_github_response({}), [])
 
 
 class TestComputeRelevance(unittest.TestCase):