Repository navigation
fix(html2md): preserve visible labels and literal preformatted text - #1591
lanyue-llk wants to merge 1 commit into
Conversation
Spider Bench A/B✅ All benchmark browser service runs completed; results are informational. Public HEAD: 184 / 240 rows; 38 / 48 sites produced rows; 10 unexpected empty sites. ✅ Deterministic fixture contract is clean. Common ancestor Public 48-site run · informational
Public-site content, timing, and memory are noisy. This single A/B run reports evidence only; site outcome counts explain missing rows without treating them as a deterministic regression. CPU and memory timelinesThese are bounded 40-point views of the same complete process-tree samples used by the HTML report. CPU uses 100% per occupied logical core; memory lines are RSS first and PSS second when complete PSS samples are available. Basexychart-beta
title "Base CPU"
x-axis "Elapsed seconds" [1.036, 1.536, 2.537, 4.039, 5.041, 6.543, 7.545, 9.046, 10.047, 11.551, 12.552, 14.055, 15.058, 16.059, 16.56, 17.562, 19.065, 20.066, 21.568, 22.571, 24.073, 25.075, 26.577, 27.578, 29.08, 30.081, 31.584, 32.583, 34.085, 35.088, 36.588, 37.589, 39.09, 40.091, 41.591, 42.592, 44.094, 44.595]
y-axis "CPU percent" 0 --> 200
line [13.99, 129.84, 117.84, 153.72, 119.87, 139.75, 141.87, 78.01, 103.79, 107.75, 115.76, 3.99, 2, 0, 5.99, 83.85, 91.94, 43.92, 95.97, 101.78, 133.86, 105.7, 135.82, 109.86, 59.92, 117.87, 107.71, 98.17, 71.95, 65.85, 128.01, 133.92, 19.97, 9.99, 100.06, 13.99, 127.86, 173.64]
xychart-beta
title "Base memory: RSS then PSS"
x-axis "Elapsed seconds" [0.036, 1.036, 1.536, 2.537, 4.039, 5.041, 6.543, 7.545, 9.046, 10.047, 11.551, 12.552, 14.055, 15.058, 16.059, 16.56, 17.562, 19.065, 20.066, 21.568, 22.571, 24.073, 25.075, 26.577, 27.578, 29.08, 30.081, 31.584, 32.583, 34.085, 35.088, 36.588, 37.589, 39.09, 40.091, 41.591, 42.592, 44.094, 44.595, 44.908]
y-axis "Memory MiB" 0 --> 500
line [13.16, 73.45, 103.04, 104, 141.19, 156.29, 135.59, 161.41, 148.74, 142.86, 130.08, 131.96, 118.44, 119.73, 120.27, 119.07, 124.65, 148.75, 122.84, 153.5, 126.75, 143.46, 271.6, 188.21, 216.43, 142.98, 163.12, 154.55, 138.33, 129.28, 139.64, 239.11, 223.5, 149.98, 131.52, 131.75, 129.53, 159.68, 168.76, 0]
line [9.79, 69.93, 98.62, 100.37, 135.86, 152.7, 131.9, 155.28, 145.19, 139.4, 126.52, 128.25, 114.87, 116.14, 116.69, 115.49, 121.2, 145.34, 119.48, 149.92, 123.19, 140.24, 268.14, 184.68, 212.97, 140.12, 155.71, 151.01, 134.63, 125.66, 136.03, 235.53, 219.97, 146.31, 127.9, 127.35, 125.96, 156.14, 166.13, 0]
HEADxychart-beta
title "HEAD CPU"
x-axis "Elapsed seconds" [1.037, 2.539, 3.541, 4.543, 5.044, 5.544, 6.045, 7.548, 8.549, 10.052, 11.054, 12.056, 13.557, 14.559, 16.06, 17.06, 18.062, 18.563, 19.564, 21.066, 22.067, 23.068, 24.57, 25.571, 27.074, 28.075, 29.577, 30.578, 32.08, 33.08, 34.082, 35.583, 36.585, 38.086, 39.087, 40.589, 41.591, 43.093]
y-axis "CPU percent" 0 --> 200
line [9.99, 109.79, 107.75, 149.63, 129.76, 131.96, 127.85, 119.76, 121.91, 69.85, 113.74, 83.85, 144.07, 13.97, 14, 2, 0, 19.96, 93.96, 119.7, 131.94, 105.84, 115.95, 123.93, 47.93, 109.88, 139.67, 47.92, 69.87, 90.02, 9.97, 143.86, 63.89, 4, 1.99, 107.98, 79.96, 109.86]
xychart-beta
title "HEAD memory: RSS then PSS"
x-axis "Elapsed seconds" [0.037, 1.037, 2.539, 3.541, 4.543, 5.044, 5.544, 6.045, 7.548, 8.549, 10.052, 11.054, 12.056, 13.557, 14.559, 16.06, 17.06, 18.062, 18.563, 19.564, 21.066, 22.067, 23.068, 24.57, 25.571, 27.074, 28.075, 29.577, 30.578, 32.08, 33.08, 34.082, 35.583, 36.585, 38.086, 39.087, 40.589, 41.591, 43.093, 44.054]
y-axis "Memory MiB" 0 --> 500
line [13.37, 69.88, 111.73, 122.49, 124.73, 150.12, 145.66, 139.99, 139.61, 134.99, 151.74, 158.75, 141.94, 147.38, 137.36, 117.42, 121.29, 120, 125.29, 162.24, 138.21, 147.26, 268.63, 202.88, 204.29, 142.39, 180.25, 142.3, 134.23, 139.23, 132.68, 127.63, 188.9, 147.11, 126.37, 126.43, 159.36, 138.79, 160.76, 0]
line [10.23, 66.29, 108.23, 118.97, 121.19, 147.14, 142.16, 136.47, 136.44, 131.46, 148.21, 156.07, 138.38, 141.31, 133.8, 113.93, 117.72, 116.31, 121.62, 158.45, 134.6, 143.72, 265.03, 199.41, 200.63, 139.21, 176.89, 137.99, 130.48, 135.73, 129.11, 123.97, 186.07, 143.37, 122.63, 122.68, 155.64, 135.04, 156.89, 0]
Deterministic fixture · required contractThe fixture exercises 15 routes: 8 are expected to emit rows and 7 intentionally exercise empty, loading, or timeout behavior.
Full HTML, JSON, CSV, logs, and page snapshots: workflow run and full The public-web A/B is informational. The exact HEAD fixture contract runs as a separate required CI check; timing and public-site content deltas remain non-blocking. |
Sequential Navigation Soak A/B✅ HEAD completed the 200-navigation resilience and memory observation. One browser process, one target, and one CDP session navigate CSDN → SegmentFault → Huaban → example.com repeatedly. This run issued 200 Common ancestor Session resilience
A public-page failure is reported separately from an unrecoverable session. The soak requires all navigation attempts, zero failed recovery, zero lifecycle/network ordering violations, and complete resource evidence. Process-tree memory
HEAD memory by 50-navigation quarter
Raw reports and per-navigation boundary samples: workflow run and full Public-site timing and memory are observational. Use the A/B deltas and trend shape as evidence, not as a deterministic performance threshold. |
CI Regression ReportSource CI run · source state at render:
Release regression — ✅ HEAD/base failures 0/0; raw binary +0.002787%Package and image size
Startup latency and PSS
Concurrency matrix
Frontend differential — ✅ 1,020/1,020 cases matched; 0 issues
Agent episodes · Moli vs Chromium — ✅ 8/8 Moli episodes passed; 0 failures
Runtime and CDP session contracts — ✅ 26 contract cases; 0 failures
CDP smoke — ✅ 52/52 groups passed; 621 scenariosWorkers: 4 · cumulative group time: 204.39 s · failed groups: 0
WebMainBench · 545 pages — ✅ 545/545 completed; 0 unexpected failures
Frozen HTML · Linux · Diagnostics: All artifact fields are parsed by the trusted default-branch renderer; missing or invalid inputs remain visible as unavailable. |
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复核提交 266117486817，未发现本次改动新增的明确阻断问题。
保留 pre 的字面文本，同时把链接目的地址放到代码块外，能兼顾代码复制和目录下载链接；aria-label 只补空链接，不覆盖已有文字，这两个处理方向合理。媒体地址沿现有 writer 输出，新增 URL 过滤也没有放行 javascript/vbscript 一类主动执行地址。
实际将该提交的 moli-html2md 原码编译为 WASI，运行了 13 个独立 DOM 场景，并对父提交做了相同对照：当前提交 13 项断言全部通过，其中 7 项输出相对父提交变化。覆盖空/非空链接标签、表格、块级子节点、pre 中的类型链接和目录、媒体与 picture、危险 URL、换行保真。这是转换器实测，不是完整仓库测试。
2661174 to
829f198
Compare
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
This PR does three things in moli-html2md: an empty <a> gets its aria-label as the link text, <video>/<audio>/<source>/<track> emit their src/poster URLs as links, and anchors inside <pre> are collected and printed as a link list after the closing fence.
Assessment: request changes. The individual mechanisms are small and the tests are deterministic. The problems are the scope, a URL policy that differs between code paths, duplicate link output, and heavy overlap with the open html2md stack #1334-#1336.
PR description vs code
- The stated problem,
String (/types/String)appearing inside a fenced block, does not appear to reproduce inmoli-html2mdonmain(if it came from another conversion path, please name it).raw_nodeonmainalready collects only text, so<pre>content was already literal; links inside<pre>were simply dropped. What the PR actually adds is the trailing link list. The fenced part of the newpreformatted_*tests already passes onmain; only the appended[label](href)lines exercise new behaviour. Please restate the problem as "destinations inside<pre>were dropped". - The title says "visible labels", but
aria-labelis an accessible name, not a visible label. - Media URLs are a third feature that the title does not mention.
- The Browser Eval note says
image-01("responsive image sources") passed. Nothing in this diff touchesimg/srcset(mainhas no srcset handling), so that case does not show anything this PR changed. - The body says fixtures cover "inline code". No test covers a link inside inline
<code>, and such links are still dropped (inline == trueskipsEndPreLink).
Overlap and conflicts with the open stack #1334-#1336 (refs/review/pr-1334..1336)
- Media, duplicated. #1334 adds
moli-html2md/src/media.rs. It hassafe_url(the same control-stripping, scheme-allowlist logic as this PR'sresource_url, plus adata:image/exception for images),sources()(directsrcplus child<source>with dedup), atitle-basedlabel(), andMachine::mediahandlingiframe|video|audio. This PR re-implements the same thing more narrowly, as a second private function inmachine.rswith different rules: no dedup, hard-coded English labels, separate<source>visits, and posters labelled "Video poster" where #1334 uses an empty alt. Merging both gives two URL-safety policies and two media renderers. One will silently shadow the other'svideo|audiomatch arm, or the merge will conflict at the same match position. - Accessible-name fallback, duplicated. #1334's
content::fallback_text(aria-label, then title, then svg<title>) applies to any element whose subtree has no readable content, including<a>. This PR adds an anchor-onlyaria-labelpath through a newWriter::end_link(serial, fallback)parameter and a secondlast_link_contentcounter. Both answer "what text does an empty link get" and would conflict inend_link, which #1335 also edits to addflush_pending_text(). - Link-safety filter. #1334 applies
safe_urlto<a href>itself. This PR leaves top-level<a href="javascript:...">unfiltered but filters the same URL inside<pre>and in media elements. raw_node/<pre>traversal. #1335 adds early returns inraw_nodefor form controls,role=buttonand script/style. #1336 adds anchor-target emission forpre. Both edit the same hunk where this PR insertsEndPreLink. The conflicts are mechanical, but the ordering matters: a control-skipping early return placed before theacheck could change which links get collected.- Recommendation: rebase this onto the stack, or split it. Keep only the
<pre>link list here, and reuse #1334'smedia::safe_urlandmedia::sourcesand itsfallback_textinstead of adding parallel implementations. If #1334-#1336 are being abandoned, say so explicitly and close them, so the repo does not end up with two competing designs.
Other
- The
moli-html2md/README.md"Supported output" list (around line 77) does not mention media resource links or the after-fence link list. - CI: the Spider Bench A/B comment is neutral (184/240 on both sides). I could not get the check rollup (the GitHub API timed out). I also could not compile locally (no linker in this sandbox), so the findings below come from reading the code at head, not from running it.
Verdict: changes requested (posted as a comment because GitHub does not allow requesting changes on your own PR).
Automated review by Claude Code.
|
Superseded by the existing #1334–#1337 stack. That stack already owns accessible fallback text, shared destination safety, media/resource extraction and links inside literal preformatted content, with broader tests and consistent semantics. #1592 has now been rebased directly onto #1337, so no open PR depends on this branch. Closing this duplicate avoids merging a second URL policy and a second media implementation. |
1 participant
Problem
HTML-to-Markdown conversion could lose visible control labels and declared media addresses. Inside a
preblock, appending a link destination also changed copyable source text: a linkedStringtype becameString (/types/String)inside the code sample.Change
Preserve visible labels, whitespace, and media URLs already present in the supplied DOM. Keep
precontent literal even when the page omits a nestedcodeelement. Named links found insidepreremain available after the closing fence, using the existing link writer, without altering the fenced text.The converter reads the supplied DOM only. It performs no additional fetches.
data:andblob:values remain excluded because they are run-local or potentially unbounded, while ordinary portable resource URLs are retained.Reproduce and verify
Convert this fragment:
The fenced source must remain
let value: String = String::new();; the named destination appears after the fence. Additional fixtures cover empty-link labels, directory listings, audio/video/source/track/poster URLs, unsafe script URLs, and inline code.cargo test --locked -p moli-html2mdThe media fixtures correspond to Web Fetch's embedded-media contract: return resource addresses already exposed by the page. No per-site or per-case branch is added. CI validates the exact PR branch.
Dependencies
Independent change targeting
main; no other open PR is required.Browser Eval integration reproduction
Build this branch, register the exact executable and SHA-256 as
moli-candidate, then run the frozen embedded-media cases through Browser Eval's certified native-fetch entry point:The exact Linux x86-64 artifact for
829f1989da7bpassed all three frozen cases: responsive image sources (image-01), multiple video sources (video-01), and video poster/resource addresses (video-03). The observed call times were 6.62 s, 8.60 s, and 7.05 s. These cases exercise the public resource-address contract through the same adapter and scorer used by the 430-task report.