Skip to content

fix(wikipedia): fix search arg name + add random and trending commands#231

Merged
jackwener merged 2 commits intojackwener:mainfrom
Astro-Han:worktree-fix-wikipedia-search
Mar 22, 2026
Merged

fix(wikipedia): fix search arg name + add random and trending commands#231
jackwener merged 2 commits intojackwener:mainfrom
Astro-Han:worktree-fix-wikipedia-search

Conversation

@Astro-Han
Copy link
Copy Markdown
Contributor

@Astro-Han Astro-Han commented Mar 22, 2026

Summary

  • fix: search.ts referenced args.keyword but the argument is defined as query, causing search to always send an empty query
  • feat: add random command — get a random article summary via REST API
  • feat: add trending command — most-read articles (yesterday's data for availability)

All three commands are PUBLIC strategy, no browser required, reuse the existing wikiFetch utility.

Test plan

  • opencli wikipedia search "TypeScript" --limit 3 returns results (was broken before)
  • opencli wikipedia random returns a random article
  • opencli wikipedia trending --limit 5 returns most-read articles
  • opencli wikipedia random --lang zh works with non-English languages
  • npm run typecheck passes

- fix: search.ts referenced `args.keyword` but the argument is defined
  as `query`, causing the search term to always be undefined
- feat: add `random` command (random article summary via REST API)
- feat: add `trending` command (most-read articles, yesterday's data)

All commands are PUBLIC strategy, no browser required, reuse wikiFetch.
@Astro-Han Astro-Han force-pushed the worktree-fix-wikipedia-search branch from df93bd1 to f6a8214 Compare March 22, 2026 03:40
@Astro-Han
Copy link
Copy Markdown
Contributor Author

Note: Wikipedia API commands verified via npm run typecheck (pass) and npx vitest run src/ (301/301 pass). Manual API testing skipped due to network restrictions in my environment (Node.js fetch times out to en.wikipedia.org). CI runners should verify the API calls end-to-end.

Copy link
Copy Markdown
Owner

@jackwener jackwener left a comment

Choose a reason for hiding this comment

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

PR #231 Review

总体来说这个 PR 改得不错,Bug fix 是正确的,两个新命令也遵循了现有模式。以下是一些改进建议:


✅ 做得好的

  1. Bug Fix 正确args.keywordargs.query 修复了 search 始终发送空 query 的问题
  2. 遵循现有模式random.tstrending.tssummary.ts 结构一致,都用 wikiFetch + Strategy.PUBLIC
  3. PR 描述清晰 — test plan 列了具体验证命令

⚠️ 建议改进

1. random.tssummary.ts 有明显重复

random.ts 的返回结构和类型定义与 summary.ts 基本一致(title, description, extract, url),连 REST API 路径都在同一系列 (/api/rest_v1/page/...)。

建议提取一个共享的 response 类型到 utils.ts

// utils.ts
export interface WikiSummary {
  title?: string;
  description?: string;
  extract?: string;
  content_urls?: { desktop?: { page?: string } };
}

这样 summary.tsrandom.ts 都可以复用,避免两处维护同样的类型。

2. trending.ts 日期计算:86400_000 不适用于所有时区

const d = new Date(Date.now() - 86400_000);

在 DST(夏令时)切换日,一天可能是 23 或 25 小时。虽然这里用的是 getUTC* 所以实际上是安全的(UTC 没有 DST),但建议加一行注释说明为什么用 UTC 是正确的,避免未来维护者误改为本地时间方法:

// Use UTC to avoid DST issues — Wikipedia API expects UTC dates
const d = new Date(Date.now() - 86400_000);

3. extract 截断长度 300 可以抽为常量

random.tssummary.ts 里都硬编码了 .slice(0, 300)trending.ts 里 description 用了 .slice(0, 80)。建议统一为常量:

// utils.ts
export const EXTRACT_MAX_LEN = 300;
export const DESC_MAX_LEN = 80;

4. 缺少文档更新

现在 docs/adapters/browser/wikipedia.md 只记录了 searchsummary 两个命令。新增的 randomtrending 也需要加上文档。

5. 缺少单元测试

PR 只做了手动验证。建议至少补一个对 trending.ts 日期路径拼接的单元测试,因为日期格式化是最容易出 bug 的地方。


📋 总结

维度 评价
Bug Fix ✅ 正确
新功能 ✅ 遵循现有模式
代码复用 ⚠️ random 和 summary 类型可提取
文档 ⚠️ 缺少 random/trending 文档
测试 ⚠️ 缺少单元测试

建议:补上文档和共享类型后可合并。测试和常量提取为可选改进。

- Extract WikiSummary, WikiMostReadArticle types to utils.ts
- Extract EXTRACT_MAX_LEN/DESC_MAX_LEN constants
- Add formatSummaryRow() helper to eliminate duplicate mapping in
  summary.ts and random.ts
- Update docs with random and trending command examples
@jackwener jackwener merged commit 1d56dd7 into jackwener:main Mar 22, 2026
12 checks passed
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