Skip to content

fix(web): add WebChannel read fallback + docs and tests + stable skil…#215

Closed
adeyemijosh wants to merge 2 commits intoPanniantong:mainfrom
adeyemijosh:fix/web-channel-read-docs-test
Closed

fix(web): add WebChannel read fallback + docs and tests + stable skil…#215
adeyemijosh wants to merge 2 commits intoPanniantong:mainfrom
adeyemijosh:fix/web-channel-read-docs-test

Conversation

@adeyemijosh
Copy link
Copy Markdown

Changes

  • WebChannel.read(): Added URL read method with schema normalization, 10s timeout, status checks, error handling, and content truncation (>10k chars).
  • Test coverage: Added test_web_channel_can_handle_and_check to verify URL handling and health check.
  • Dev docs: Updated README.md and CONTRIBUTING.md with pip install -e . + pytest -q setup steps and ModuleNotFoundError fix hint.
  • Robustness: Improved _install_skill() fallback reading (importlib.resources → local file) with UTF-8 encoding.

Validation

  • python -m pip install -e .
  • python -m pytest -q --maxfail=1
  • 74 passed

Type

fix / enhancement to core WebChannel + docs + tests

Copy link
Copy Markdown
Owner

@Panniantong Panniantong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感谢贡献!fallback 机制和 encoding 修复的方向都对,但 read() 的实现与项目设计不符,需要修改:

必须修复:

  1. read() 应使用 Jina Reader,而非直接 requests.get()

    check() 的描述是「通过 Jina Reader 读取任意网页」,但新实现的 read() 完全绕开了 Jina,直接抓取原始 HTML。这会返回未处理的 HTML 给 agent,与项目定位不符。请改为:

    def read(self, url: str) -> str:
        if not url.startswith(('http://', 'https://')):
            url = 'https://' + url
        jina_url = f'https://r.jina.ai/{url}'
        resp = requests.get(jina_url, timeout=30)
        resp.raise_for_status()
        return resp.text
  2. read() 方法缺少测试:新增的功能没有对应的单元测试(应 mock requests.get 测试正常和异常路径)。

  3. README.md 的修改位置不合适:在 README 顶部新增「开发快速启动」节会打断现有文档结构,且内容与 CONTRIBUTING.md 重复。建议这部分内容移入 CONTRIBUTING.md 或删除。

可保留的改动(很好):

  • _install_skill() 的 try/except fallback 机制 ✅
  • 写文件时统一加 encoding="utf-8"
  • CONTRIBUTING.md 补充 pytest 步骤 ✅

修复以上三条后请重新请求 review!

Copy link
Copy Markdown
Author

@adeyemijosh adeyemijosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three requested fixes are now complete:

  1. ✅ read() now uses Jina Reader API (https://r.jina.ai/)
  2. ✅ Added comprehensive unit tests with mocking (normal + abnormal paths)
  3. ✅ Removed duplicate README.md section

All 79 tests pass. Ready for review!

@adeyemijosh adeyemijosh requested a review from Panniantong March 31, 2026 07:56
Panniantong added a commit that referenced this pull request Mar 31, 2026
- WebChannel.read(): reads any URL via Jina Reader (r.jina.ai), returns Markdown
- _install_skill(): add fallback from importlib.resources to Path(__file__) for editable installs
- Explicit UTF-8 encoding on all file read/write operations

Inspired by PR #215, implemented correctly (Jina instead of raw requests).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Panniantong added a commit that referenced this pull request Mar 31, 2026
- WebChannel.read(): reads any URL via Jina Reader (r.jina.ai), returns Markdown
- _install_skill(): add fallback from importlib.resources to Path(__file__) for editable installs
- Explicit UTF-8 encoding on all file read/write operations

Inspired by PR #215, implemented correctly (Jina instead of raw requests).

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants