# Implementation Plans
Generated by the improve skill on 2026-07-23, against commit `c7622fd`.
Execute in the order below unless dependencies say otherwise. Each executor:
read the plan fully before starting, honor its STOP conditions, and update
your row when done.
## Execution order & status
| 025 | Preserve whitespace inside `<pre>`/`<code>` during post-processing (issue #24) | P1 | S | — | **MERGED** `39f4f10` (`46642eb`..`efab09f`) — split-and-map pass, not the reference commit's placeholder swap (see plan for why); corpus unchanged |
| 026 | Two pre-existing paths that delete code-listing content (`replace_brs`, comment channel) | P1 | M | 025 | TODO — both reproduced end-to-end, byte-identical at HEAD (not 025 regressions) |
| 024 | Escape link/image/media destinations in markdown output (injection) | P1 | M | — | **COMMITTED** `cb98bc3` on branch `advisor/remaining-plans` (unmerged) — review folded in media (iframe/video/audio) + footnote-def sinks the plan missed |
| 001 | Fix byline lowercase-slice panic (remote DoS) | P1 | S | — | **MERGED** `00de913` |
| 002 | Make the Mozilla test suite assert and run by default | P1 | M | — | **MERGED** `6d197cc` — surfaced true baseline 119/130; README was stale at 122/130 |
| 003 | Add CI + clean fmt/clippy baseline | P1 | S | — | **MERGED** `59d5b25`,`6060fba`,`89e1b52`,`bcd8657` (+ `4430e24` post-merge fmt) |
| 004 | Document sanitization contract + opt-in sanitizer | P1 | M | 002 | **MERGED** `d3c817f` — REVISE round closed a scheme-filter bypass; plan spec corrected |
| 010 | Fix broken README example + compile README as doctests | P2 | S | — | **MERGED** `a57130f` (import fix),`335397e` (doctests) — used `#[cfg(doctest)]`, not a crate-doc include; see note below |
| 011 | Stop double-escaping `&` in standardized image URLs | P2 | S | — | **MERGED** `9355750` |
| 019 | Apply entity-aware escape fix to math.rs + code_blocks.rs | P2 | S | 011 | **MERGED** `08b9088` — helper extracted to new `src/elements/escaping.rs`, not `mod.rs` |
| 020 | Wire up or remove the 4 public options that are never read | P1 | M | — | **MERGED** `2eb21aa`,`dfd56f3` — `max_elems_to_parse` enforced; other 3 deprecated, not removed |
| 021 | Fix `unwrap_nav_wrappers` deleting entire article bodies | **P0** | S | — | **MERGED** `3b8f337` — Option A (function deleted); corpus unchanged at 119/130 |
| 022 | Remove `<script>`/`<style>` at DOM level (`</script >` bypass) | **P0** | M | — | **MERGED** `e1789cf`,`0eb0a65` — DOM removal + serializer denylist; no extra parse; corpus unchanged at 119/130 |
| 023 | Bound markdown table column widths (panic + 265× blowup) | P1 | S | — | **MERGED** `f392a7b` — cap 200; measured 293× → 2.01×; zero existing expectations changed |
| 007 | Hoist per-call regex compilation into Lazy statics | P2 | S | — (coordinate with 006) | **MERGED** `13204b8` — Mozilla corpus 39.7s → 25.6s |
| 005 | NodeId element identity (kill O(N·K) scans) + NaN-safe sort | P2 | M | 002 | **MERGED** `98c6fdf`,`08cf31f` — corpus byte-identical to baseline, deterministic over 3 runs |
| 008 | Bound DOM recursion depth (stack-overflow DoS) | P2 | M | 002 (recommended) | **MERGED** `d522421` — limit is **256, not the planned 512** (measured; see note below) |
| 016 | Complete is_probably_readerable (visibility + unlikely-candidates) | P2 | S-M | 002 | **MERGED** `d840b82` — corpus agreement 128/130 → **130/130**; used `div > br`, not the planned `div > p` |
| 006 | Consolidate element removal onto the DOM path | P2 | L | 002 (mandatory) | **BLOCKED** — STOP condition hit; work preserved on branch `refactor/dom-only-element-removal` (`c1568da`), NOT merged. See below. |
| 012 | Migrate once_cell → std LazyLock, declare MSRV 1.80 | P3 | M | 003 | **COMMITTED** `cafb70e` on branch `advisor/remaining-plans` (unmerged) — MSRV declared **1.83** not 1.80: resolved tree needs 1.83 (ICU4X 2.x via url/idna), verified with `cargo metadata`; not toolchain-verified |
| 013 | Dependency hygiene (v_htmlescape pin, thiserror 2.x, dev advisory) | P3 | S | — | **COMMITTED** `b112b2a` on branch `advisor/remaining-plans` (unmerged) — v_htmlescape `=0.15.8`→`0.15`, thiserror `1.0`→`2.0`; crossbeam already done by 003 |
| 014 | Document dev workflow (CONTRIBUTING) + CLAUDE.md | P3 | S | best after 002+003 | **COMMITTED** `c4f0842` — CONTRIBUTING.md Development section added; CLAUDE.md left untracked (owner's file), reconciled scope |
| 015 | Split the 2127-line metadata.rs module | P3 | M | after 001 and 007 land | **COMMITTED** `9f91e8d` on branch `advisor/remaining-plans` (unmerged) — pure move, byte-verified; byline.rs 1187 lines (test-extraction follow-up noted); Step 3 utils migration skipped (shared helpers) |
| 009 | Collapse serialize→re-parse round-trips into one tree pass | P3 | L | 002 + 006 (mandatory) | TODO |
| 017 | SPIKE: LLM-ready-Markdown entry point design | P3 | M | — | TODO |
| 018 | SPIKE: CLI binary design + feature-gated prototype | P3 | M | 017 (soft) | TODO |
one-line rationale).
## Merge state
As of 2026-08-04, plans 001, 002, 003, 004 and 011 are **merged into `main`**
(9 commits ahead of `c7622fd`; nothing pushed). Backup tag `pre-merge-backup`
points at the pre-merge base — `git reset --hard pre-merge-backup` reverts the
whole stack.
All three CI gates are green on the merged tree: `cargo fmt --check`,
`cargo clippy --all-targets -- -D warnings`, and `cargo test`
(168 lib + 117 markdown + 2 asserting Mozilla + 24 doctests).
Two consequences for the remaining plans:
- **New baseline**: plans still stamped `Planned at c7622fd` will now show
drift on files touched by the merge. Re-verify their "Current state"
excerpts before dispatching; the ones most affected are 005, 008, 009
(`element_to_html` now takes a `sanitize: bool` param from plan 004) and
010 (README + `lib.rs` gained a Security section).
- **The blockers are gone**: 005 and 010 were previously unrunnable against a
clean base. They can now be executed normally.
## Repo conventions for executors (READ BEFORE DISPATCHING)
- An **untracked `CLAUDE.md`** exists in the main working tree (the owner keeps
it untracked deliberately). Because it is untracked, it is INVISIBLE to
executor worktrees (which only see committed files) and subagents will
distrust it if merely relayed. **Inline the relevant CLAUDE.md conventions
directly into every executor prompt.** The load-bearing ones:
- Commits: single-line conventional commit, `type(scope): what changed`.
**No body. No `Co-Authored-By` trailer** (this overrides the harness
default). Describe the change, not the process/finding counts.
- No `unwrap()`/`expect()` in library code (fine in tests). No `unsafe`
without a `// SAFETY:` comment. No decorative/redundant comments.
- `thiserror` for lib errors; prefer `&str` over `String` in params; `///`
docs on public items.
- Plan 014 (create CLAUDE.md) is now **partially superseded** — a CLAUDE.md
already exists (untracked). If 014 is executed later, scope it to reconcile
with the existing file and CONTRIBUTING, and keep CLAUDE.md untracked unless
the owner says otherwise.
## Plan 006 STOP: the removal RULES are unsafe once removal actually works
Executed 2026-08-05. The refactor itself succeeded — the unified
`remove_unwanted_dom` pass replaced all three string-level removers, every unit
test was migrated to the public path, lib (208) and markdown (117) suites passed,
and Mozilla **metadata** was byte-identical to baseline. It then failed the
Mozilla **content** assertion:
```
2 case(s) regressed:
aclu: extracted content is empty (expected ~11328)
mercurial: extracted content is empty (expected ~23191)
```
That is plan 021's bug reappearing from the other direction, and it was caught
only by the unconditional `actual_len > 0` assertion added in `3b8f337`.
**Cause.** The keyword sets match as substrings against `class + " " + id` of
`div, span, aside, section, ul, ol`, and the article containers on those two
pages match:
| mercurial | `section evolve-shared-mutable-history` | `share` inside "**share**d" |
| aclu | `panel-display panel-two-col-sidebar-right-mix` | `sidebar` (a real layout token) |
The DOM pass detaches the container **and everything inside it**. The regex
removers never did: `<tag[^>]*?>.*?</tag>` stopped at the first closing tag, so
they only ever deleted a fragment. **The rule set was never safe — it was only
survivable because the matcher was broken.** Correcting the mechanism exposes
that.
**Diagnostic run** (whole-token matching, `\b(share|social|...)\b`): fixes
mercurial, does **not** fix aclu, because `sidebar` is a genuine word inside
`panel-two-col-sidebar-right-mix`. So this is not a matching-strategy bug; the
rules themselves need re-deriving.
**Why stopped rather than fixed.** Plan 006 scope says explicitly: "do not
'improve' thresholds while consolidating; behaviour changes must come only from
WHERE removal happens, not from new rules." Re-deriving the keyword rules is a
design decision, and its STOP condition covers exactly this case.
**Recommended follow-up plan (006b), before 006 can land:**
1. Do not remove a keyword-matched container if it holds substantial text or is
an ancestor of the top candidate. `should_remove_dom_node` already weighs link
density and text length — route keyword removal through it instead of
detaching unconditionally.
2. Match whole class tokens (split on whitespace), not raw substrings, so
"shared" stops matching `share`. Necessary but not sufficient on its own.
3. Keep the unconditional tag list (form, iframe, nav, …) as-is; it caused no
regressions.
4. Re-run the corpus expecting 119/130 metadata and zero empty-content cases.
## Deviation from plan 016's spec: `div > br`, not `div > p`
Plan 016 said to include "any `div` with a direct `<p>` child", calling it the
simple form of Mozilla's rule. Mozilla actually selects `div > br` and adds
those divs' parents; there is no `div > p` rule. Measured against the
`readerable` flags in the 130 expected-metadata files:
| stub (before this plan) | 2 | 0 |
| `div > p` (as planned) | 1 | **2** |
| `div > br` (as Mozilla) | **0** | **0** |
`div > p` also double-counts, scoring a div's text and its paragraphs' text
separately, which is what produced the two new false positives. Shipped
`div > br`.
Two further notes from executing 016:
- The plan's Step 3 test 1 expected long paragraphs inside
`<div class="comment">` to be non-readerable. They are not: the check reads
each scored node's **own** class and id and never walks ancestors, exactly as
the plan's own Mozilla restatement says. Tests were rewritten to put the class
on the scored element, and a test now pins the per-node behaviour explicitly
because the opposite is the intuitive assumption.
- Every check was mutation-tested by deleting it and confirming a named test
fails. The first drafts of the `li > p` and `div > br` tests passed with their
check removed, i.e. they proved nothing, and were rewritten.
## Deviation from plan 008's spec: MAX_DOM_DEPTH is 256, not 512
Plan 008 specified `MAX_DOM_DEPTH = 512`. That value does not actually prevent
the overflow it exists to prevent. Measured on the markdown converter, the
walker with the largest stack frames, in a debug build on a default libtest
thread stack:
| 512 | **stack overflow, abort** |
| 448 | **stack overflow, abort** |
| 384 | survives |
| 256 | survives |
Shipped 256: ~2.5x headroom over real-world nesting (well under 100 levels) and
~1.5x margin below the observed cliff, since frame sizes shift with build
profile, platform and future edits to `convert_element_inner`.
Also of note from executing 008: the depth guard stops the *abort*, but a
document with tens of thousands of elements is still very slow (measured
through `parse()`: 1,000 nested divs 2.5s, 5,000 divs 43s, 10,000 divs >70s).
That cost is the regex cleanup passes scaling superlinearly in element count,
not recursion, and it is the pre-existing finding already noted in SECURITY.md.
Plan 006 is the fix.
## Deviation from plan 010's spec
Plan 010 Step 2 specified `#![doc = include_str!("../README.md")]`, which pulls
the README into the crate docs. That was written when README and `lib.rs` docs
overlapped heavily. They no longer do: the docs rewrite (`a57130f`) moved
Security and the algorithm description into `lib.rs` only, and left badges,
install, benchmarks and repo-relative links (`CONTRIBUTING.md`, `SECURITY.md`)
in the README only. Including it would have duplicated the usage examples and
put broken relative links on docs.rs.
Used instead:
```rust
#[cfg(doctest)]
#[doc = include_str!("../README.md")]
struct ReadmeDoctests;
```
Same guarantee (every README `rust` fence compiles and runs under
`cargo test`), no crate-doc pollution. Plan 010's done-criterion naming the
`#![doc = ...]` form is therefore intentionally unmet.
Also note the plan's suggestion to hide setup lines with the `#` prefix: that
works in rustdoc but **not** on GitHub or crates.io, which render `# fn main()`
literally. The snippets were made self-contained instead.
## Dependency notes
- **002 is the keystone**: the Mozilla corpus currently asserts nothing, so
every behavior-changing plan (005, 006, 008, 009, 016) lists it as a
prerequisite regression net. Land it first (001 is independent and can go
in parallel).
- **003 (CI)** will be red until its own fmt/clippy cleanup steps run — that
is by design. Merge 002 first if possible so CI gates the asserting suite
from day one.
- **007 vs 006**: 006 deletes several functions 007 would optimize. Either
order works; 007 is cheap enough that wasted work is acceptable, and its
drift check tells the executor to skip deleted functions.
- **009** must come last among the refactors: it needs 006's DOM-only
cleaning and 002's content assertions, and it touches the same functions
as 004/005/008.
- **015** moves code that 001 and 007 edit — schedule it after both to avoid
merge pain.
- **004, 005, 008** all touch `element_to_html`'s signature. Whichever lands
later must reconcile (each plan carries a note for this).
## Recorded findings without their own plan yet (post-merge bug hunt, 2026-08-04)
All reproduced by the bug-hunt agent against `4430e24`. Listed so they aren't
re-discovered; each needs a plan before execution.
- **Markdown injection** (`src/markdown/converter.rs:131-135` → `rules/images.rs:8`,
`rules/links.rs:29`): `alt`, `href` and `title` go into `![]()`/`[]()` raw.
`escape_markdown` (`rules/text.rs:84-93`) deliberately skips `[`/`]`.
Reproduced: `alt="](javascript:alert(1)) [pwn"` yields an attacker-controlled
link destination in `markdown_content`. **CLOSED** by plan 024 (`cb98bc3`):
structural escaping of `[`/`]`/`(`/`)`/titles is always on; `javascript:`/
`vbscript:`/non-image `data:` scheme-dropping is gated on `sanitize_urls`
(wired from `sanitize_content`). The bug-hunt review extended the fix to the
media sinks (`iframe`/`video`/`audio`, `rules/media.rs`) and footnote
definitions (`converter.rs`), which had the same raw-destination breakout.
- **`pick_best_srcset` splits on `,` first** (`src/elements/images.rs:115`):
corrupts any CDN URL containing a comma (imgix/Cloudinary `w=800,q=75`).
Reproduced: `src` rewritten to the broken relative path `q=75/a.jpg`. MED.
- **`element_to_html` drops attribute namespace prefixes**
(`src/content_extractor.rs:980` emits `name.local` only): produces duplicate
`href` attributes on SVG `<a xlink:href … href …>` and mangles `xmlns:xlink`.
Parser-differential risk. Also makes the `"xlink:href"` arm at `:976` dead
code. MED.
- **`link_density_modifier` makes the `partial_cmp().unwrap()` panic reachable**
(`src/content_extractor.rs:229`, value applied at `src/scoring.rs:129`): NaN or
±∞ passed through the public builder panics extraction. **CLOSED** in
`98c6fdf` (sort now uses `total_cmp`, so the panic is unreachable) and
`08cf31f` (builder ignores non-finite values). MED.
- **`IMG_TAG_RE` truncates at `>` inside an attribute value**
(`src/elements/images.rs:11`): deletes a partial tag and leaks text.
Reproduced: `<img … zz="a>b">` → `before b"> after`. LOW.
- **Void-element end-tag alternation** (`src/post_processor.rs:279-287`):
`<input>…</input>` patterns swallow content up to a stray `</input>` planted in
a comment (comment bodies are emitted verbatim at `content_extractor.rs:1004`).
LOW; plan 022 removed the comment vector **only when `sanitize_content` is on**
— comments are still emitted verbatim on the default path.
- **`cleaner::parse_element` can build a reversed byte range**
(`src/cleaner.rs:411`): `&html[opening_end+1..closing_start]` with no ordering
check. Latent — not currently reachable, becomes live if `replace_brs` is ever
exported or fed unescaped input. LOW.
- **Mozilla content assertion is weak** (`tests/mozilla_test_suite.rs`): the
±100% band plus integer division means `expected_len <= 1` admits
`actual_len == 0`. This is why plan 021's bug shipped undetected. **Partly
closed in `3b8f337`**: an unconditional `actual_len > 0` check now runs outside
the divergence allowlist. It is a forward guard only — no corpus case was empty
under plan 021's bug, so it would not have caught that one. Still open:
tightening the band from ±100% to ±15-20%. LOW (test quality).
## Findings considered and rejected
- **ReDoS in the regex patterns**: not applicable — the `regex` crate is a
linear-time finite-automaton engine; no backtracking engine is used.
- **"Fix the 8 failing Mozilla test pages"**: rejected as a goal — README
explicitly frames these divergences as intentional improvements
(better byline/excerpt choices). A `strict_mozilla_compat` flag was
considered and deferred for lack of any demand signal; do not chase
byte-parity.
- **RUSTSEC-2026-0204 (crossbeam-epoch)** as a security finding: dev-only
path (criterion), unreachable from the published library. Handled as a
lockfile hygiene item inside plans 003/013.
- **Unused dependencies**: none exist — all Cargo.toml deps verified in use
(bitflags in constants.rs, serde on Article, serde_json for JSON-LD, etc.).
- **Missing unit-test coverage per module**: not a finding — every src module
has `#[cfg(test)]` tests (154 total); the gap was integration-level only
(plan 002).
- **Public API reference docs**: not a finding — public items are broadly
documented and docs.rs is populated.
- **`utils.rs` as a junk drawer**: rejected as its own finding — it is a
cohesive byline/text-helper module; its byline half migrates naturally in
plan 015.
- **WASM/npm distribution** (direction): grounded (the repo benchmarks
against `@mozilla/readability` and the pure-function API is WASM-friendly)
but deferred — bundle-size risk from scraper/html5ever/regex needs its own
spike, and the maintainer hasn't signaled browser-side demand. Revisit
after 017/018 outcomes.
- **True streaming/incremental parsing**: no in-repo demand signal, large
architectural change — not planned.
- **NaN-panic sort in find_best_candidate**: not rejected, but folded into
plan 005 (same function) rather than its own plan.
- **No-op `fix_relative_urls_in_html` dead code**: folded into plan 006.
- **README "122/130" claim unenforceable**: folded into plans 002 (enforce)
and 010 (reproduce command).