Skip to content

fix(ingest): keep the word boundary a styled space carries - #3588

Open
L4XB wants to merge 1 commit into
Graphify-Labs:v8from
L4XB:fix/ingest-markdown-whitespace
Open

L4XB wants to merge 1 commit into
Graphify-Labs:v8from
L4XB:fix/ingest-markdown-whitespace

Conversation

@L4XB

@L4XB L4XB commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

The bug

_html_to_markdown is what turns a fetched page into the markdown that gets ingested,
searched and linked in the graph. markdownify runs an inline element's text through
chomp(), which lifts the surrounding whitespace out of the text and then returns '' once
nothing is left, so an element holding only whitespace disappears together with its
whitespace.

Measured through _html_to_markdown itself, on v8:

page markup before after
Hello<b> </b>world Helloworld Hello world
Hello<em> </em>world Helloworld Hello world
Hello<a href="…"> </a>world Helloworld Hello world
<b>First</b><b> </b><b>Last</b> **First****Last** **First** **Last**

Ten of the eleven inline tags lose the boundary; only <u> survives.

The last row is what makes this ordinary rather than an edge case: an editor that emits one
element per styled run puts the space between two bold words in an element of its own, so any
page that bolds a phrase mid-sentence can produce it. Two words then enter the graph as one
token sequence, and neither word's query matches it.

The change

markdownify(html, ...) is MarkdownConverter(...).convert(html), so this swaps in a
subclass that wraps the conversion function markdownify resolves for a tag: a whitespace-only
element returns its text unchanged, everything else takes the normal path. One hook covers all
fourteen inline tags instead of overriding each of them.

The ImportError fallback is deliberately untouched. The converter class is built inside an
lru_cached factory whose from markdownify import MarkdownConverter raises the same
ImportError the caller already catches, so an install without the pdf/all extra still
takes the tag strip. It is also built once rather than per call.

heading_style="ATX", bullets="-" and strip=["img"] still reach the converter — that is
what three of the control cases pin.

Tests

tests/test_ingest_html_markdown.py is new, 18 cases behind
pytest.importorskip("markdownify") so an install without the extra skips rather than fails:

  • the whitespace-only element across ten tags, including the anchor
  • the two-styled-runs case, asserting the exact markdown
  • six controls: bold, emphasis and code with real content, plus the ATX heading, the -
    bullet and the stripped <img>
  • script and style text still never reaches the output

11 of the 18 fail on v8; the 7 that pass are the controls.

tests/test_ingest.py + tests/test_ingest_html_markdown.py
  + tests/test_markdown_code_spans.py                       37 passed
ruff check (repo config)                                    All checks passed

markdownify runs an inline element's text through chomp(), which lifts the
surrounding whitespace out and then returns an empty string once nothing is
left, so an element holding only whitespace disappears together with its
whitespace:

  Hello<b> </b>world              ->  Helloworld
  <b>First</b><b> </b><b>Last</b> ->  **First****Last**

The second shape is what an editor emitting one element per styled run
produces for a space between two bold words, so an ingested page can lose
word boundaries anywhere a phrase is styled mid-sentence. Whatever is
searched and linked in the graph then carries the merged word.

Wrap the conversion function markdownify resolves per tag, which covers every
inline tag at once, and keep the ImportError fallback intact: the converter is
built lazily so an install without markdownify still takes the tag strip.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. 1 change(s) tested, no difference found (not proven).


Graphify review — findings

Fixes HTML-to-markdown conversion swallowing the whitespace of inline elements that contain only whitespace, which previously ran adjacent words together (Hello<b> </b>worldHelloworld). _html_to_markdown now converts through a cached MarkdownConverter subclass that returns a whitespace-only inline element's text verbatim instead of chomping it away, while keeping the existing ATX/bullet/image-strip options and the ImportError fallback to a basic tag strip.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 230 functions depend on the 32 functions this change touches.

Health — this change adds coupling hotspots:

  • new: dispatch_command() — 2 callers, 125 callees
  • new: ingest() — 3 callers, 8 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees
  • new: test_lessons_artifact_cannot_be_globbed_back_into_memory() — 0 callers, 6 callees

Verification — 230 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 56 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

3 of 284 test file(s) selected (1%) via static blast radius.

  • tests/test_ingest.py — impact
  • tests/test_ingest_html_markdown.py — impact, changed-test
  • tests/test_reflect.py — impact

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

No difference found (not proven): No behavior difference found in \_html\_to\_markdown (not a proof).

The verifier ran both versions of \_html\_to\_markdown on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

· 4 more finding(s) on lines outside this diff (see the check run).

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.

1 participant