Files
last30days-skill/docs/plans/2026-03-07-review-open-issues-and-prs-triage-plan.md
T
Matt Van Horn 8f7fb5a7fe fix(release): v2.9.5 - remove re-introduced Save Research section
PR merges on March 7 (PR #48 Xiaohongshu, upstream merge) regressed
SKILL.md by re-introducing the "Save Research to Documents" section
that v2.9.4 removed. Those branches were forked before v2.9.4 and
brought the old content back via merge resolution.

Fixes: remove save section, restore --save-dir flag on bash command,
update agent mode line, add tool-call guard to STOP instruction.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-08 11:55:35 -07:00

12 KiB

title, type, status, date
title type status date
Triage Open Issues and PRs refactor active 2026-03-07

Triage Open Issues and PRs - last30days-skill

Consolidated review and action plan for all 6 open items (2 issues, 4 PRs) as of 2026-03-07.


1. PR #52 - Fix missing metadata files in skill upload bundle

Author: 04cb | Size: +0/-1 | Fixes: #46 File changed: .clawhubignore (removes *.json glob)

What it does

The *.json pattern in .clawhubignore was excluding .claude-plugin/marketplace.json and .claude-plugin/plugin.json - the metadata files required for ClawHub skill upload. This caused the "Zip file contains path with invalid characters" error reported in #46.

Risk analysis

  • Removing *.json also includes test fixtures (fixtures/*.json) and vendored package.json in the bundle. None are sensitive or harmful.
  • No package-lock.json, skills-lock.json, or credential files exist as .json in the repo.
  • One-line change with zero production code impact.

Recommendation: MERGE IMMEDIATELY

  • Merge PR #52
  • Close issue #46 (auto-closes via "Fixes #46")
  • Verify upload works post-merge

2. Issue #46 - "Claude will not upload as it says thread is invalid"

Author: johnuppard | Error: "Zip file contains path with invalid characters"

Recommendation: CLOSES WITH PR #52

No standalone action needed. PR #52 is the fix. After merge, comment on #46 confirming resolution and ask reporter to verify.


3. PR #50 - test: add tests for entity_extract module

Author: mark-c4r | Size: +167/-0 (tests only)

What it does

Adds tests/test_entity_extract.py with 23 test cases covering all 4 public/private functions in scripts/lib/entity_extract.py:

Function Cases Coverage
_extract_x_handles 8 author handles, @mentions, generic filtering, case normalization, frequency ranking, edge cases
_extract_x_hashtags 5 basic extraction, multiple tags, frequency ranking, short tag filtering, empty input
_extract_subreddits 6 field extraction, cross-refs in comments, frequency ranking, r/ stripping
extract_entities 4 integration, max limits, empty inputs, return key validation

Code quality assessment

  • All function signatures match the actual module implementation exactly
  • All 23 tests validated against the real module - they pass
  • Import pattern matches existing tests (sys.path.insert + from lib import)
  • Uses unittest.TestCase consistent with all other test files
  • No external dependencies, no API calls, no mocking needed (pure functions)
  • Test data uses inline dicts matching real Reddit/X response structure

Recommendation: MERGE AFTER LOCAL VERIFICATION

  • Pull branch and run python3 -m pytest tests/test_entity_extract.py -v (23/23 passed)
  • Run full suite python3 -m pytest tests/ (317/320 passed - 3 pre-existing failures in test_models.py, unrelated)
  • Merge

This is a clean, well-structured community contribution that adds coverage to a previously untested module.


4. PR #48 - feat: add Xiaohongshu source + Reddit public fallback

Author: YJLi-new | Size: +523/-75 | Files: 5 modified

What it does

Two features bundled in one PR:

A. Xiaohongshu (Little Red Book) source:

  • New scripts/lib/xiaohongshu_api.py module for searching via xiaohongshu-mcp HTTP API
  • Source alias: --search xiaohongshu or --search xhs
  • Health/login availability checks in env.py
  • Handles Chinese numeric suffixes (wan/yi) in engagement parsing
  • Default API base: http://host.docker.internal:18060 (Docker-hosted)

B. Reddit public JSON fallback:

  • search_reddit_public() added to openai_reddit.py
  • Uses reddit.com/search/.json endpoint (no API key required)
  • Multiple query strategy (topic, core subject, quoted core)
  • Engagement-based relevance heuristic (60% score, 40% comments)
  • Supplemental retries correctly gated - only fire when OpenAI auth is present

Architecture compliance

Pattern Status
Three-function module pattern (search/parse/enrich) Follows existing patterns
DEPTH_CONFIG with quick/default/deep Present
_log() gated on stderr.isatty() Present
env.py availability check Present with retry logic
ThreadPoolExecutor dispatch Correctly wired with +1 worker
Render/UI integration Consistent with existing sources

Issues found

  1. Health check duplication - is_xiaohongshu_available() in env.py AND search_feeds() both probe login status. Redundant but harmless.
  2. from_date/to_date accepted but unused for Xiaohongshu - relies on API-side time bucketing. Acceptable given API limitation.
  3. Over-defensive setdefault() in normalization (id and source_domain already set by search_feeds). Harmless.
  4. Error message format inconsistency between OpenAI and public Reddit paths. Minor.
  5. get_available_sources() docstring not updated to reflect Reddit always being available now.
  6. No tests included for new Xiaohongshu module or Reddit public fallback.

Security

  • No hardcoded credentials
  • xsec_token comes from API response (not user input)
  • Docker host.docker.internal default is safe for containerized environments
  • All URLs use https:// for public Xiaohongshu

Recommendation: MERGE WITH CONDITIONS

The Reddit public fallback alone makes this worth merging - it makes Reddit work with zero API keys. Xiaohongshu adds value for Chinese-market research.

Before merge:

  • Run full test suite to confirm no regressions (292/297 pass, 5 pre-existing)
  • Verify Xiaohongshu gracefully skips when API is unavailable (returns False, no crash)
  • Resolved merge conflicts with ScrapeCreators Reddit (main). Priority: ScrapeCreators -> OpenAI -> public fallback
  • Merged to main and pushed
  • Test Reddit public fallback locally: unset OPENAI_API_KEY, run python3 scripts/last30days.py "test topic" --search reddit --emit=compact
  • Update get_available_sources() docstring to reflect Reddit always-available change

After merge (follow-up):

  • Add tests/test_xiaohongshu.py with fixture-based tests
  • Add tests/test_reddit_public.py for the fallback path
  • Consider extracting health check duplication

5. PR #47 - feat: add Apify as unified API provider

Author: lapolazzati | Size: +1484/-73 | Files: 6 new + orchestrator changes

What it does

Adds Apify as a single-token alternative (APIFY_API_TOKEN) covering Reddit, X, TikTok, and Instagram. Existing per-source keys take priority; Apify is a transparent fallback.

New modules:

  • apify_client.py (163 lines) - shared HTTP client for Apify run-sync API
  • apify_reddit.py (195 lines) - Reddit via trudax/reddit-scraper actor
  • apify_x.py (221 lines) - X via apidojo/tweet-scraper actor
  • apify_tiktok.py (284 lines) - TikTok via clockworks/tiktok-scraper actor
  • apify_instagram.py (288 lines) - Instagram via apify/instagram-reel-scraper actor

Source routing priority:

Reddit:    OpenAI -> Apify -> None
X:         Bird -> xAI -> Apify -> None
TikTok:    ScrapeCreators -> Apify -> None
Instagram: ScrapeCreators -> Apify -> None

Critical issues

  1. Actor ID mismatch (BLOCKER): Code uses different Apify actors than documented in README and plan.md.

    Source In Code In Docs
    Reddit trudax/reddit-scraper automation-lab/reddit-scraper
    X apidojo/tweet-scraper scraper_one/x-posts-search
    TikTok clockworks/tiktok-scraper epctex/tiktok-search-scraper
    Instagram apify/instagram-reel-scraper matches

    Users following README instructions will hit wrong actors.

  2. No tests (BLOCKER): 1484 lines of new code with zero test coverage. Each Apify module has its own date parsing, relevance scoring, and response normalization - all untested.

  3. Significant code duplication: apify_tiktok.py and apify_instagram.py share nearly identical:

    • _tokenize() (7 lines)
    • _compute_relevance() (14 lines)
    • STOPWORDS set
    • SYNONYMS dict
    • _extract_core_subject() (~20 lines)
  4. Version mismatch: README says v2.9 but git history shows v2.9.4 on main. Version should be higher.

  5. plan.md included in commit - implementation planning doc shouldn't be in the final merge.

What's good

  • Source routing logic in env.py is clean and backward-compatible
  • Orchestrator dispatch uses consistent _source parameter pattern
  • Existing source paths are completely untouched
  • Timeout scaling is appropriate (90s quick, 150s default, 240s deep)
  • Token handling follows security best practices (parameter passing, Bearer auth)

Recommendation: REQUEST CHANGES

This PR is too large and has too many issues for a clean merge. Request the contributor to:

Must fix before any merge:

  • Resolve actor ID mismatches (verify which actors actually work, update code OR docs)
  • Add unit tests for all 5 new modules (at minimum: response normalization, date parsing)
  • Fix version number
  • Remove plan.md from commit

Should fix:

  • Extract shared code from apify_tiktok/apify_instagram into apify_common.py
  • Consider splitting into 2 PRs:
    • PR A: apify_client.py + env.py routing (foundation)
    • PR B: Individual source modules + tests (features)

Status: [x] Review posted requesting changes (2026-03-07)

Comment template for PR:

Thanks for this contribution - the single-token approach is a great idea for simplifying setup. A few things need fixing before we can merge:

  1. The Apify actor IDs in the code don't match the README/plan docs. Can you verify which actors are correct and align code + docs?
  2. We need test coverage for the new modules - at minimum normalization tests with sample actor responses.
  3. There's significant duplication between apify_tiktok.py and apify_instagram.py (_tokenize, _compute_relevance, STOPWORDS, SYNONYMS, _extract_core_subject). Could you extract shared code to an apify_common.py?
  4. Version in README should be higher than v2.9.4 (current main).
  5. Please remove plan.md from the commit.

6. Issue #45 - Add support for Gemini CLI

Author: alexferrari88 | Request: "Would it be possible to add support for Gemini CLI?"

Assessment

This is a feature request to support Gemini CLI as a runtime alongside Claude Code and Codex. Key considerations:

  • Scope: The skill is currently packaged for Claude Code (SKILL.md format) and Codex (~/.codex/skills/). Gemini CLI uses a different skill/extension format.
  • Effort: Would require understanding Gemini CLI's plugin system, creating a compatible manifest, and potentially adapting the Python execution model.
  • Priority: Low - Claude Code and Codex are the primary targets and where the user base is.
  • Community: If the requester wants to contribute, they'd be best positioned to understand Gemini CLI's requirements.

Recommendation: ACKNOWLEDGE AND BACKLOG

  • Respond with comment acknowledging and inviting contribution
  • Add a help wanted label
  • Keep open as a backlog item

Priority Summary

Priority Item Action Risk
1 PR #52 (metadata fix) Merge now DONE None
2 Issue #46 (upload error) Auto-closes with PR #52 DONE None
3 PR #50 (entity tests) Run tests, merge DONE None
4 PR #48 (Xiaohongshu + Reddit fallback) Test locally, merge with conditions DONE Low
5 Issue #45 (Gemini CLI) Acknowledge, backlog, label DONE None
6 PR #47 (Apify unified) Request changes DONE (awaiting contributor) Medium-High