refactor(github): split search_github / parse_github_response / enrich_with_comments
search_github returned a normalized List[dict] directly while every
other adapter follows search_X -> dict envelope, parse_X_response ->
list[dict]. The github branch in pipeline._retrieve_stream was the
only one that called search_* and returned (result, {}) without a
parse step. This blocked fixture-driven testing: there was no parse
function to feed a synthetic envelope to.
Split into three:
search_github(...) -> Dict[str, Any]
HTTP fetch only. Returns {"items": [raw items], "context": {core,
from_date, to_date, count}}.
parse_github_response(response) -> List[Dict[str, Any]]
Pure function. Normalizes, date-filters, sorts by relevance.
enrich_with_comments(items, depth, token) -> List[Dict[str, Any]]
Public extraction of the old private _enrich_top_items. Resolves
the token via env / gh CLI fallback so callers don't have to.
Pipeline now does the standard 3-call dance:
response = github.search_github(...)
items = github.parse_github_response(response)
items = github.enrich_with_comments(items, depth=depth, token=token)
Keeping enrich_with_comments in parse_github_response would make parse
impure and force every fixture-driven test to either mock HTTP or
skip enrichment. Splitting it out matches the YouTube adapter's
pattern.
This commit is contained in:
@@ -142,8 +142,14 @@ def search_github(
|
||||
to_date: str,
|
||||
depth: str = "default",
|
||||
token: Optional[str] = None,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""Search GitHub Issues and PRs.
|
||||
) -> Dict[str, Any]:
|
||||
"""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,12 +159,12 @@ 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.
|
||||
"""
|
||||
resolved_token = _resolve_token(token)
|
||||
if not resolved_token:
|
||||
_log("No GitHub token available (set GITHUB_TOKEN or install gh CLI)")
|
||||
return []
|
||||
return {"items": [], "error": "no token"}
|
||||
|
||||
count = DEPTH_LIMITS.get(depth, DEPTH_LIMITS["default"])
|
||||
core = extract_core_subject(topic)
|
||||
@@ -176,12 +182,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 +259,34 @@ def search_github(
|
||||
},
|
||||
})
|
||||
|
||||
# Enrich top items with comments
|
||||
items = _enrich_top_items(items, depth, resolved_token)
|
||||
|
||||
# Date filter
|
||||
filtered = []
|
||||
for item in items:
|
||||
d = item.get("date")
|
||||
if d is None or (from_date <= d <= to_date):
|
||||
filtered.append(item)
|
||||
if from_date and to_date:
|
||||
items = [
|
||||
item for item in items
|
||||
if item.get("date") is None or (from_date <= item["date"] <= to_date)
|
||||
]
|
||||
|
||||
# Sort by relevance
|
||||
filtered.sort(key=lambda x: x.get("relevance", 0), reverse=True)
|
||||
items.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(
|
||||
|
||||
@@ -1007,8 +1007,11 @@ 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"))
|
||||
return result, {}
|
||||
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,
|
||||
|
||||
+119
-13
@@ -76,13 +76,14 @@ 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)
|
||||
|
||||
@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": [
|
||||
@@ -99,9 +100,15 @@ class TestSearchGithub(unittest.TestCase):
|
||||
},
|
||||
],
|
||||
}
|
||||
result = github.search_github("react", "2026-03-01", "2026-03-31")
|
||||
self.assertEqual(len(result), 1)
|
||||
item = result[0]
|
||||
# Search returns raw envelope; parse normalizes.
|
||||
response = github.search_github("react", "2026-03-01", "2026-03-31")
|
||||
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")
|
||||
@@ -117,10 +124,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):
|
||||
"""403 rate limit returns empty list gracefully."""
|
||||
result = github.search_github("react", "2026-03-01", "2026-03-31")
|
||||
self.assertEqual(result, [])
|
||||
def test_rate_limit_returns_empty_envelope(self, mock_token, mock_fetch):
|
||||
"""403 rate limit returns envelope with empty items list."""
|
||||
response = github.search_github("react", "2026-03-01", "2026-03-31")
|
||||
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")
|
||||
@@ -142,9 +150,107 @@ class TestSearchGithub(unittest.TestCase):
|
||||
},
|
||||
],
|
||||
}
|
||||
result = github.search_github("next.js", "2026-03-01", "2026-03-31")
|
||||
self.assertEqual(len(result), 1)
|
||||
self.assertTrue(result[0]["metadata"]["is_pr"])
|
||||
response = github.search_github("next.js", "2026-03-01", "2026-03-31")
|
||||
items = github.parse_github_response(response)
|
||||
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):
|
||||
|
||||
Reference in New Issue
Block a user