Files

T

Tianyi Cui 3a88912a22 docs: adopt dependencies-over-hand-rolling policy from NIH audit

A repo-wide Not Invented Here audit (ten parallel domain surveys covering
every package group, scripts/, native/, vendor/ edges, python/, test
infrastructure, and CI) asked of each hand-rolled surface whether a
maintained external package or Node builtin deletes it with a net win.

Policy: new implemented process note records that a dependency which
genuinely deletes owned code is a preferred simplification (bar: net
deletion, health, boundary fit, settled seams stay); root AGENTS.md
carries the one-line rule and dsh-find-simplifications now surveys for
hand-rolled-where-a-dependency-exists candidates.

Findings, all bilingual from birth:
- proposed/simplification: eventsource-parser for llm-deepseek SSE,
  node:timers/promises for three hand-rolled sleeps, turndown (or
  minimal 'entities') for tool-web HTML->markdown, gate-script
  consolidation onto mdast/parseArgs/globSync
- proposed/testing: execa + parseArgs + loadEnvFile + vi.waitFor for
  hand-rolled test subprocess plumbing
- proposed/process: pnpm/action-setup for symmetric CI caching
- proposed/feature: evaluate landstrip before building a Windows
  sandbox launcher
- rejected/simplification: ~30 swap verdicts recorded (vscode-jsonrpc,
  p-retry, Ajv, write-file-atomic, msw, hono, better-sqlite3, wireit,
  landstrip-for-linux, YAML consolidation, ...) so the survey is not
  re-litigated from scratch

Also drops the stale prompt/ entry from the AGENTS.md layout map
(workspace instructions live in packages/context/workspace-context).

2026-07-26 15:54:36 +08:00

3.5 KiB

Raw Blame History

Agent Note: Replace tool-web's regex HTML-to-markdown converter with turndown

Status: proposed

English | 中文

Problem

packages/web/tool-web/src/html.ts (~86 lines, ~40 lines of dedicated tests) converts fetched HTML to markdown with regexes: strip script/style/noscript/comments, convert <a>/<h1-6>/<li>, decode numeric entities plus a 12-entry named-entity table, collapse whitespace. The module's own JSDoc says "A richer converter can replace it without changing the seam or tool schema", and the README's Known Limitations documents it as "a minimal regex converter, not an HTML parser — tables, images, and nested formatting are lost." The web capability seam note assigns HTML→markdown to this package as presentation, so the swap point is exactly here. The converter's output is model-visible on every fetched HTML page; no keyless snapshot currently exercises web_fetch, so no expected outputs pin it.

Proposal

Replace htmlToMarkdown with turndown (new TurndownService().turndown(html)), optionally with turndown-plugin-gfm for tables. The consumer switch in fetch.ts and the status-header/truncation-footer formatting stay. Wrap the call in try/catch falling back to the raw text path: the regex version could never throw; turndown on pathological HTML could. Delete html.ts and its conversion tests; keep tests for the fallback and the surrounding formatting. Update the README's Known Limitations to drop the regex-converter caveat.

If the "deliberately minimal fallback" stance is preferred instead, a minimal variant still deletes the worst part: replace the entity-decoding third of the file (~30 lines: decodeEntities, NAMED_ENTITIES, safeFromCodePoint) with the zero-dependency entities package (already in the lockfile transitively), erasing the documented "about a dozen entities" limitation at near-zero risk.

Alternatives considered

@mozilla/readability + a DOM. Solves a different problem (content extraction, not conversion) and drags a heavier DOM dependency; the seam only asks for markdown rendering of whatever the fetch returned.
Keep the regex converter. It was an explicit v1 placeholder per its own JSDoc; keeping it means model-visible quality (tables, images, nested formatting) stays lost for the cost of maintaining bespoke entity tables.
The minimal entities-only variant. Kept in the proposal as the fallback position; it deletes less but avoids the dependency-weight question entirely.

Acceptance criteria

web_fetch renders tables/nested formatting via turndown (or, minimal variant: decodes all named entities), with the README limitation updated.
Unit tests cover the fallback path; pnpm run test passes for the package.
A keyless snapshot exercising web_fetch markdown rendering is added per testing policy (the missing snapshot coverage is part of the change, and it pins the new output).

Risks

Model-visible output changes on every fetched HTML page — transcript drift is acceptable pre-release, and nothing currently pins the old output.
Dependency weight: turndown's one dependency (@mixmark-io/domino) is a ~200 KB DOM that would enter the single-file-executable closure if tool-web ships in it (single-exe note); the minimal entities variant avoids this if closure size is the deciding factor.

3.5 KiB Raw Blame History