Address review feedback: deduplicate query_type, clean unused imports, fix defaults
- Remove duplicate detect_query_type from query.py (divergent 5-type version); canonical 7-type version lives in query_type.py - Fix reddit.py import to use query_type.detect_query_type - Clean unused STOPWORDS/SYNONYMS/tokenize imports from youtube_yt, instagram, tiktok, scrapecreators_x, bird_x after relevance consolidation - Fix _relevance_filter default from 0.7 to 0.0 (items without relevance should not silently pass the filter) - Remove --dateafter from yt-dlp (returns 0 results for evergreen topics) - Remove restrictSearchableAttributes from HN search (misses Ask/Show HN) - Lower HN points filter from >5 to >2 (avoids filtering niche posts) - Add error logging to select_openai_model HTTP failures - Remove mise.toml and internal planning doc from repo - Update module docstrings to describe current purpose, not migration history - Update tests to import from canonical relevance module
This commit is contained in:
Jeffrey Sperling
@@ -1,85 +0,0 @@
|
||||
# Search Pipeline: Query & Relevance Consolidation
|
||||
|
||||
## Strategy: Single upstream PR
|
||||
|
||||
**Branch**: `refactor/query-relevance-consolidation` -> `mvanhorn/last30days-skill:main`
|
||||
**PR**: https://github.com/mvanhorn/last30days-skill/pull/65
|
||||
|
||||
All changes (refactors + behavior improvements) combined into one upstream PR.
|
||||
Originally planned as two phases, but the search quality improvements are
|
||||
broadly useful, not opinionated — merged into a single contribution.
|
||||
|
||||
---
|
||||
|
||||
## Refactors (commits 1-5)
|
||||
|
||||
### Step 1: New `query.py` — shared query utilities
|
||||
- Consolidate 7 duplicated `_extract_core_subject()` (bird_x, reddit, youtube_yt, tiktok, instagram, bluesky, scrapecreators_x) into one parameterized function
|
||||
- `extract_core_subject(topic, noise=None, max_words=None, strip_suffixes=False)` — platform modules pass their own noise set and options to preserve current behavior
|
||||
- Shared `PREFIXES` list (identical across all 7), shared `NOISE_WORDS` base set
|
||||
- Each platform imports `extract_core_subject` and calls with its own overrides (e.g. bird_x passes `max_words=5, strip_suffixes=True`; youtube keeps tips/tricks/tutorial in its noise exclusion)
|
||||
- Fix reddit.py prefix-loop missing `break` (apply all matching prefixes vs only first)
|
||||
- Skip polymarket.py (too different — handles "last N days", preserves title case)
|
||||
- Tests: `tests/test_query.py`
|
||||
|
||||
### Step 2: New `relevance.py` — shared relevance scoring
|
||||
- Consolidate `_tokenize`, `_compute_relevance`, `STOPWORDS`, `SYNONYMS` from youtube_yt/tiktok/instagram
|
||||
- `token_overlap_relevance(query, text, hashtags=None) -> float` — zero-dep, superset of all three implementations (hashtag substring matching from tiktok/instagram, synonym expansion from youtube)
|
||||
- Unified `SYNONYMS` dict (youtube superset: includes svelte/vue entries missing from tiktok/instagram)
|
||||
- Tests: `tests/test_relevance.py` (migrate from `test_youtube_relevance.py` + new hashtag tests)
|
||||
|
||||
### Step 6: urllib fallback for TikTok/Instagram (independent bug fix)
|
||||
- `tiktok.py`: add `http.get()`/`http.post()` fallback when `_requests is None`
|
||||
- `instagram.py`: same pattern
|
||||
- Copies pattern from reddit.py's existing fallback
|
||||
|
||||
### Step 3: Integrate `query.py` into per-source modules (pure refactor)
|
||||
- `bird_x.py`: replace lines 52-106 with import, call `extract_core_subject(topic, max_words=5, strip_suffixes=True, noise=BIRD_NOISE)`
|
||||
- `reddit.py`: replace `NOISE_WORDS` + `_extract_core_subject` with query import; `expand_reddit_queries` imports from query.py too
|
||||
- `youtube_yt.py`: replace `_extract_core_subject` with import, pass youtube-specific noise set (keeps tips/tricks/tutorial/guide/review)
|
||||
- `tiktok.py`, `instagram.py`, `bluesky.py`: same replacement with their noise sets
|
||||
- Update tests: 12+ test methods across 6 test files reference `module._extract_core_subject()` — either re-export from original modules or update test imports
|
||||
|
||||
### Step 8: Deduplicate relevance code in youtube/tiktok/instagram (pure refactor)
|
||||
- `youtube_yt.py`: remove `STOPWORDS`, `SYNONYMS`, `_tokenize`, `_compute_relevance`; import from `relevance.py`
|
||||
- `tiktok.py`: same
|
||||
- `instagram.py`: same
|
||||
- Update tests: `test_youtube_relevance.py`, `test_tiktok.py`, `test_instagram_sc.py`, `test_scrapecreators_x.py` reference `module._tokenize`/`module._compute_relevance` — re-export or update imports
|
||||
|
||||
### Commit order: 1 → 2 → 6 → 3 → 8
|
||||
|
||||
---
|
||||
|
||||
## Search quality improvements (commits 6-8)
|
||||
|
||||
### Step 4: Replace hardcoded relevance with computed scores
|
||||
- `bird_x.py:471` — `"relevance": 0.7` → `token_overlap_relevance(core_topic, text)`
|
||||
- `reddit.py:223` — `"relevance": 0.7` → `token_overlap_relevance(core, title + " " + selftext)`
|
||||
- `hackernews.py:139-141` — blend: `0.6 * rank_score + 0.4 * token_overlap`
|
||||
|
||||
### Step 5: Platform-specific query optimization
|
||||
- `detect_query_type(topic)` — heuristic classifier (product/concept/opinion/how_to/comparison), added here not Phase 1
|
||||
- `extract_compound_terms(topic)` — detect hyphenated/title-case terms, return quoted
|
||||
- `bird_x.py`: OR-group construction for multi-concept queries, OR-based retry before word-dropping fallback
|
||||
- `reddit.py`: conditional opinion/review suffix only for product/opinion queries (uses `detect_query_type`)
|
||||
- `hackernews.py`: add `numericFilters: points>5`, `restrictSearchableAttributes=title`, use `extract_core_subject()` instead of raw topic
|
||||
- `youtube_yt.py`: add `--dateafter YYYYMMDD` (from_date already in signature)
|
||||
|
||||
### Step 7: Post-retrieval relevance filtering in orchestrator
|
||||
- `last30days.py` (after dedup): filter items with `relevance < 0.3` per source (only when list has >3 items)
|
||||
- Extend fallback guarantee to all sources: keep top 3 by relevance if all filtered
|
||||
- `rerank_with_embeddings()` — optional, env-var gated (`OPENAI_API_KEY` or `GOOGLE_API_KEY`), uses existing `http.py`, graceful fallback to token overlap
|
||||
|
||||
### Commit order: 4 → 5 → 7
|
||||
|
||||
---
|
||||
|
||||
## Status: COMPLETE
|
||||
|
||||
All 8 commits pushed to `refactor/query-relevance-consolidation`. PR #65 updated.
|
||||
|
||||
## Verify
|
||||
|
||||
```bash
|
||||
cd ~/projects/last30days-skill && python3 -m unittest discover -s tests -v
|
||||
```
|
||||
@@ -1835,11 +1835,11 @@ def main():
|
||||
"""Filter items below relevance threshold with minimum-result guarantee."""
|
||||
if len(items) <= 3:
|
||||
return items
|
||||
passed = [i for i in items if getattr(i, 'relevance', 0.7) >= threshold]
|
||||
passed = [i for i in items if getattr(i, 'relevance', 0.0) >= threshold]
|
||||
if not passed:
|
||||
# Keep top 3 by relevance if all filtered
|
||||
print(f"[{source_name} WARNING] All results below relevance {threshold}, keeping top 3", file=sys.stderr)
|
||||
by_rel = sorted(items, key=lambda x: getattr(x, 'relevance', 0.7), reverse=True)
|
||||
by_rel = sorted(items, key=lambda x: getattr(x, 'relevance', 0.0), reverse=True)
|
||||
return by_rel[:3]
|
||||
return passed
|
||||
|
||||
|
||||
@@ -58,7 +58,7 @@ def _extract_core_subject(topic: str) -> str:
|
||||
Aggressively strip question/meta/research words to keep only the
|
||||
core product/concept name (max 5 words).
|
||||
"""
|
||||
from .query import NOISE_WORDS, extract_core_subject
|
||||
from .query import extract_core_subject
|
||||
return extract_core_subject(topic, max_words=5, strip_suffixes=True)
|
||||
|
||||
|
||||
|
||||
@@ -90,13 +90,14 @@ def search_hackernews(
|
||||
core = extract_core_subject(topic)
|
||||
_log(f"Searching for '{core}' (raw: '{topic}', since {from_date}, count={count})")
|
||||
|
||||
# Use relevance-sorted search with minimum engagement filter
|
||||
# Use relevance-sorted search with minimum engagement filter.
|
||||
# NOTE: restrictSearchableAttributes=title omitted intentionally — it would
|
||||
# miss Ask HN/Show HN threads where the topic appears in the body.
|
||||
params = {
|
||||
"query": core,
|
||||
"tags": "story",
|
||||
"numericFilters": f"created_at_i>{from_ts},created_at_i<{to_ts},points>5",
|
||||
"numericFilters": f"created_at_i>{from_ts},created_at_i<{to_ts},points>2",
|
||||
"hitsPerPage": str(count),
|
||||
"restrictSearchableAttributes": "title",
|
||||
}
|
||||
|
||||
from urllib.parse import urlencode
|
||||
|
||||
@@ -31,12 +31,7 @@ DEPTH_CONFIG = {
|
||||
# Max words to keep from each caption
|
||||
CAPTION_MAX_WORDS = 500
|
||||
|
||||
from .relevance import (
|
||||
STOPWORDS,
|
||||
SYNONYMS,
|
||||
token_overlap_relevance as _compute_relevance,
|
||||
tokenize as _tokenize,
|
||||
)
|
||||
from .relevance import token_overlap_relevance as _compute_relevance
|
||||
|
||||
|
||||
def _extract_core_subject(topic: str) -> str:
|
||||
|
||||
@@ -53,6 +53,10 @@ def is_search_capable_model(model_id: str) -> bool:
|
||||
Includes mini variants (same structured extraction quality, lower cost).
|
||||
Excludes: nano (no web_search), gpt-4o-mini (no domain filtering),
|
||||
chat/codex/pro/preview/turbo/search (specialized variants).
|
||||
|
||||
Note: gpt-5 with reasoning effort="minimal" does NOT support web_search
|
||||
(per OpenAI docs). We never set reasoning params — our usage is pure
|
||||
tool invocation + JSON extraction — so gpt-5 is safe to include here.
|
||||
"""
|
||||
model_lower = model_id.lower()
|
||||
|
||||
|
||||
+2
-49
@@ -1,9 +1,5 @@
|
||||
"""Shared query utilities for /last30days search modules.
|
||||
|
||||
Consolidates duplicated _extract_core_subject() logic from bird_x, reddit,
|
||||
youtube_yt, tiktok, instagram, bluesky, and scrapecreators_x into one
|
||||
parameterized function. Each platform calls with its own overrides.
|
||||
"""
|
||||
"""Shared query preprocessing utilities: noise-word stripping, core subject
|
||||
extraction, and compound term detection. Used by all search modules."""
|
||||
|
||||
import re
|
||||
from typing import FrozenSet, List, Optional, Set
|
||||
@@ -99,49 +95,6 @@ def extract_core_subject(
|
||||
return result.rstrip('?!.') if not max_words else (result or topic.lower().strip())
|
||||
|
||||
|
||||
# ---- Query type detection (heuristic, no LLM) ----
|
||||
|
||||
_OPINION_SIGNALS = frozenset({
|
||||
'worth', 'thoughts', 'opinion', 'opinions', 'review', 'reviews',
|
||||
'recommend', 'recommendation', 'recommendations', 'should',
|
||||
'anyone', 'anybody', 'experience', 'experiences',
|
||||
})
|
||||
|
||||
_HOW_TO_SIGNALS = frozenset({
|
||||
'how', 'setup', 'configure', 'install', 'tutorial', 'guide',
|
||||
'step', 'steps', 'instructions',
|
||||
})
|
||||
|
||||
_COMPARISON_SIGNALS = frozenset({
|
||||
'vs', 'versus', 'compared', 'comparison', 'better', 'alternative',
|
||||
'alternatives', 'difference', 'differences',
|
||||
})
|
||||
|
||||
_PRODUCT_SIGNALS = frozenset({
|
||||
'pricing', 'price', 'cost', 'plan', 'plans', 'tier', 'tiers',
|
||||
'buy', 'purchase', 'subscription', 'trial', 'free',
|
||||
})
|
||||
|
||||
|
||||
def detect_query_type(topic: str) -> str:
|
||||
"""Classify query intent without an LLM.
|
||||
|
||||
Returns one of: "product", "concept", "opinion", "how_to", "comparison".
|
||||
Used to adapt per-platform query construction.
|
||||
"""
|
||||
words = set(topic.lower().split())
|
||||
|
||||
if words & _COMPARISON_SIGNALS:
|
||||
return "comparison"
|
||||
if words & _HOW_TO_SIGNALS or topic.lower().startswith("how "):
|
||||
return "how_to"
|
||||
if words & _OPINION_SIGNALS:
|
||||
return "opinion"
|
||||
if words & _PRODUCT_SIGNALS:
|
||||
return "product"
|
||||
return "concept"
|
||||
|
||||
|
||||
def extract_compound_terms(topic: str) -> List[str]:
|
||||
"""Detect multi-word terms that should be quoted in search queries.
|
||||
|
||||
|
||||
@@ -48,7 +48,8 @@ DEPTH_CONFIG = {
|
||||
},
|
||||
}
|
||||
|
||||
from .query import detect_query_type, extract_core_subject as _query_extract
|
||||
from .query import extract_core_subject as _query_extract
|
||||
from .query_type import detect_query_type
|
||||
from .relevance import token_overlap_relevance
|
||||
|
||||
# Reddit-specific noise words (preserves original smaller set)
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
"""Shared relevance scoring for /last30days search modules.
|
||||
"""Shared token-overlap relevance scoring for search result ranking.
|
||||
|
||||
Consolidates duplicated _tokenize, _compute_relevance, STOPWORDS, and SYNONYMS
|
||||
from youtube_yt, tiktok, instagram, and scrapecreators_x into one module.
|
||||
Tokenizes text, expands synonyms, and computes query-to-content overlap ratios.
|
||||
"""
|
||||
|
||||
import re
|
||||
|
||||
@@ -24,12 +24,7 @@ DEPTH_CONFIG = {
|
||||
"deep": {"results_per_page": 40},
|
||||
}
|
||||
|
||||
from .relevance import (
|
||||
STOPWORDS,
|
||||
SYNONYMS,
|
||||
token_overlap_relevance as _compute_relevance,
|
||||
tokenize as _tokenize,
|
||||
)
|
||||
from .relevance import token_overlap_relevance as _compute_relevance
|
||||
|
||||
|
||||
def _extract_core_subject(topic: str) -> str:
|
||||
|
||||
@@ -31,12 +31,7 @@ DEPTH_CONFIG = {
|
||||
# Max words to keep from each caption
|
||||
CAPTION_MAX_WORDS = 500
|
||||
|
||||
from .relevance import (
|
||||
STOPWORDS,
|
||||
SYNONYMS,
|
||||
token_overlap_relevance as _compute_relevance,
|
||||
tokenize as _tokenize,
|
||||
)
|
||||
from .relevance import token_overlap_relevance as _compute_relevance
|
||||
|
||||
|
||||
def _extract_core_subject(topic: str) -> str:
|
||||
|
||||
@@ -35,12 +35,7 @@ TRANSCRIPT_LIMITS = {
|
||||
# Max words to keep from each transcript
|
||||
TRANSCRIPT_MAX_WORDS = 500
|
||||
|
||||
from .relevance import (
|
||||
STOPWORDS,
|
||||
SYNONYMS,
|
||||
token_overlap_relevance as _compute_relevance,
|
||||
tokenize as _tokenize,
|
||||
)
|
||||
from .relevance import token_overlap_relevance as _compute_relevance
|
||||
|
||||
|
||||
def _log(msg: str):
|
||||
@@ -100,16 +95,15 @@ def search_youtube(
|
||||
_log(f"Searching YouTube for '{core_topic}' (since {from_date}, count={count})")
|
||||
|
||||
# yt-dlp search with full metadata (no --flat-playlist so dates are real).
|
||||
# --dateafter helps yt-dlp filter server-side, but Python soft filter
|
||||
# (below) handles the fallback for evergreen topics with 0 recent results.
|
||||
dateafter = from_date.replace("-", "") # YYYYMMDD format for yt-dlp
|
||||
# NOTE: --dateafter intentionally omitted — YouTube search returns
|
||||
# relevance-sorted results and strict date filtering returns 0 for
|
||||
# evergreen topics. Python soft filter (below) handles date filtering.
|
||||
cmd = [
|
||||
"yt-dlp",
|
||||
f"ytsearch{count}:{core_topic}",
|
||||
"--dump-json",
|
||||
"--no-warnings",
|
||||
"--no-download",
|
||||
"--dateafter", dateafter,
|
||||
]
|
||||
|
||||
preexec = os.setsid if hasattr(os, 'setsid') else None
|
||||
|
||||
@@ -8,34 +8,35 @@ from pathlib import Path
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
|
||||
|
||||
from lib import instagram
|
||||
from lib.relevance import tokenize as _tokenize
|
||||
|
||||
|
||||
class TestTokenize(unittest.TestCase):
|
||||
"""Tests for _tokenize()."""
|
||||
"""Tests for tokenize() from relevance module."""
|
||||
|
||||
def test_strips_stopwords(self):
|
||||
tokens = instagram._tokenize("how to use the AI tools")
|
||||
tokens = _tokenize("how to use the AI tools")
|
||||
self.assertNotIn("how", tokens)
|
||||
self.assertNotIn("the", tokens)
|
||||
self.assertNotIn("to", tokens)
|
||||
|
||||
def test_expands_synonyms(self):
|
||||
tokens = instagram._tokenize("ai tools")
|
||||
tokens = _tokenize("ai tools")
|
||||
self.assertTrue("artificial" in tokens or "intelligence" in tokens)
|
||||
|
||||
def test_removes_single_char(self):
|
||||
tokens = instagram._tokenize("a b c python")
|
||||
tokens = _tokenize("a b c python")
|
||||
self.assertNotIn("a", tokens)
|
||||
self.assertNotIn("b", tokens)
|
||||
self.assertIn("python", tokens)
|
||||
|
||||
def test_lowercases(self):
|
||||
tokens = instagram._tokenize("Python REACT")
|
||||
tokens = _tokenize("Python REACT")
|
||||
self.assertIn("python", tokens)
|
||||
self.assertIn("react", tokens)
|
||||
|
||||
def test_strips_punctuation(self):
|
||||
tokens = instagram._tokenize("hello, world!")
|
||||
tokens = _tokenize("hello, world!")
|
||||
self.assertIn("hello", tokens)
|
||||
self.assertIn("world", tokens)
|
||||
|
||||
|
||||
@@ -30,6 +30,12 @@ class TestParseVersion(unittest.TestCase):
|
||||
|
||||
class TestIsSearchCapableModel(unittest.TestCase):
|
||||
def test_gpt5_is_capable(self):
|
||||
"""gpt-5 supports web_search when reasoning is not set to 'minimal'.
|
||||
|
||||
Per OpenAI docs, gpt-5 with reasoning effort="minimal" does NOT
|
||||
support web_search. We never set reasoning params (our usage is
|
||||
tool invocation + JSON extraction only), so gpt-5 is safe here.
|
||||
"""
|
||||
self.assertTrue(models.is_search_capable_model("gpt-5"))
|
||||
|
||||
def test_gpt52_is_capable(self):
|
||||
|
||||
+1
-22
@@ -6,7 +6,7 @@ from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
|
||||
|
||||
from lib.query import NOISE_WORDS, detect_query_type, extract_compound_terms, extract_core_subject
|
||||
from lib.query import NOISE_WORDS, extract_compound_terms, extract_core_subject
|
||||
|
||||
|
||||
class TestExtractCoreSubject(unittest.TestCase):
|
||||
@@ -126,27 +126,6 @@ class TestNoiseWordsCompleteness(unittest.TestCase):
|
||||
self.assertIn(w, NOISE_WORDS)
|
||||
|
||||
|
||||
class TestDetectQueryType(unittest.TestCase):
|
||||
"""Tests for detect_query_type()."""
|
||||
|
||||
def test_comparison(self):
|
||||
self.assertEqual(detect_query_type("React vs Vue"), "comparison")
|
||||
|
||||
def test_how_to(self):
|
||||
self.assertEqual(detect_query_type("how to deploy on Vercel"), "how_to")
|
||||
|
||||
def test_opinion(self):
|
||||
self.assertEqual(detect_query_type("cursor IDE worth it"), "opinion")
|
||||
|
||||
def test_product(self):
|
||||
self.assertEqual(detect_query_type("cursor IDE pricing"), "product")
|
||||
|
||||
def test_concept_default(self):
|
||||
self.assertEqual(detect_query_type("multi-agent reinforcement learning"), "concept")
|
||||
|
||||
def test_how_prefix(self):
|
||||
self.assertEqual(detect_query_type("how does Claude work"), "how_to")
|
||||
|
||||
|
||||
class TestExtractCompoundTerms(unittest.TestCase):
|
||||
"""Tests for extract_compound_terms()."""
|
||||
|
||||
@@ -6,26 +6,27 @@ from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
|
||||
from lib import scrapecreators_x
|
||||
from lib.relevance import tokenize as _tokenize
|
||||
|
||||
|
||||
class TestTokenize(unittest.TestCase):
|
||||
def test_lowercases(self):
|
||||
tokens = scrapecreators_x._tokenize("Claude AI")
|
||||
tokens = _tokenize("Claude AI")
|
||||
self.assertIn("claude", tokens)
|
||||
|
||||
def test_strips_stopwords(self):
|
||||
tokens = scrapecreators_x._tokenize("the best AI tool")
|
||||
tokens = _tokenize("the best AI tool")
|
||||
self.assertNotIn("the", tokens)
|
||||
self.assertIn("best", tokens) # 'best' is not a stopword in tokenizer
|
||||
|
||||
def test_removes_single_char(self):
|
||||
tokens = scrapecreators_x._tokenize("a b cd ef")
|
||||
tokens = _tokenize("a b cd ef")
|
||||
self.assertNotIn("a", tokens)
|
||||
self.assertNotIn("b", tokens)
|
||||
self.assertIn("cd", tokens)
|
||||
|
||||
def test_expands_synonyms(self):
|
||||
tokens = scrapecreators_x._tokenize("ai research")
|
||||
tokens = _tokenize("ai research")
|
||||
self.assertIn("artificial", tokens)
|
||||
self.assertIn("intelligence", tokens)
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ from pathlib import Path
|
||||
# Add lib to path
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent / "scripts"))
|
||||
|
||||
from lib.youtube_yt import _compute_relevance, _tokenize
|
||||
from lib.relevance import token_overlap_relevance as _compute_relevance, tokenize as _tokenize
|
||||
|
||||
|
||||
class TestTokenize(unittest.TestCase):
|
||||
|
||||
Reference in New Issue
Block a user