Skip to content

Replace html2text (GPL-3.0) with markdownify (MIT)#586

Closed
mrummuka wants to merge 4 commits into
Graphify-Labs:v5from
mrummuka:replace-html2text-with-markdownify
Closed

Replace html2text (GPL-3.0) with markdownify (MIT)#586
mrummuka wants to merge 4 commits into
Graphify-Labs:v5from
mrummuka:replace-html2text-with-markdownify

Conversation

@mrummuka

Copy link
Copy Markdown

Summary

Swaps the optional HTML→Markdown converter used by URL ingestion from html2text (GPL-3.0) to markdownify (MIT). Aligns the pdf and all extras with the project's MIT license and removes a copyleft dependency that affected anyone redistributing or embedding graphify.

Why

html2text is the only GPL-licensed dependency in the project. Its presence in the pdf/all extras creates copyleft obligations that conflict with the project's MIT license and impact downstream redistribution.

Changes

  • graphify/ingest.py: _html_to_markdown() now uses markdownify with heading_style=ATX, bullets='-', and strip=['img']. Script/style blocks are pre-stripped (with content) via regex because markdownify's strip= removes tags but keeps their inner text — preventing CSS/JS from leaking into the emitted markdown (small security improvement).
  • pyproject.toml: html2textmarkdownify in both pdf and all extras.
  • graphify/skill.md and skills/graphify/skill.md: user-facing description updated.
  • CHANGELOG.md: Unreleased entry added.

Behaviour preserved:

  • ATX headings (# Title)
  • Inline links kept
  • Images dropped (matches prior ignore_images=True)
  • No body-width wrapping
  • Regex-strip fallback path retained when markdownify is unavailable

Tests

Adds tests/test_html_to_markdown.py with 12 tests:

  • 9 conversion tests (paragraphs, ATX headings, links, image removal, script/style stripping, bullet lists, no wrapping, empty input, malformed HTML)
  • Fallback path test that forces ImportError
  • End-to-end _fetch_webpage integration test
  • Regression guard that fails if html2text reappears in shipped code or pyproject.toml
Baseline: 293 passed
After:    305 passed (293 + 12 new), 0 failed, 0 regressions

License posture

Dep Before After
HTML→MD converter html2text (GPL-3.0) markdownify (MIT)

Closes the only copyleft dependency in the project.

@mrummuka
mrummuka changed the base branch from v4 to v5 April 28, 2026 09:33
Adds 12 tests covering paragraphs, headings (ATX), links, image
removal, script/style stripping, bullet lists, line wrapping, empty
input, malformed HTML, the regex-strip fallback path, an end-to-end
_fetch_webpage smoke test, and a regression guard ensuring the
GPL-3.0 'html2text' dependency does not creep back into shipped code
or pyproject.toml.

Three tests intentionally fail against the current html2text-based
implementation; they will turn green when html2text is replaced with
markdownify (MIT) in the following commit.
Swap the optional HTML→Markdown converter used by URL ingestion from
html2text to markdownify. Aligns the 'pdf' and 'all' extras with the
project's MIT license and removes a copyleft dependency that affected
anyone redistributing or embedding graphify.

Behaviour preserved:
- Headings rendered ATX style (# Title)
- Links kept inline
- Images dropped (matches prior ignore_images=True)
- No body-width wrapping
- Regex-strip fallback path retained when markdownify is unavailable

Script/style blocks are now pre-stripped (with content) before
conversion: markdownify's strip= removes tags but preserves their
inner text, which previously leaked CSS/JS into the output.

All 305 tests pass (293 existing + 12 new).
- CHANGELOG: add Unreleased entry documenting the license-motivated swap
- skill.md (both copies): update user-facing description from
  'converted to markdown via html2text' to '...via markdownify'
@mrummuka
mrummuka force-pushed the replace-html2text-with-markdownify branch from 0b48f08 to 3709454 Compare April 28, 2026 09:38
@Qodo-Free-For-OSS

Copy link
Copy Markdown

Hi, tests/test_html_to_markdown.py asserts markdownify-specific output even when markdownify is not installed, so the test suite fails on a base install before exercising the intended regex-strip fallback behavior.

Severity: action required | Category: reliability

How to fix: Gate tests on markdownify

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

The new tests in tests/test_html_to_markdown.py assume markdownify is installed and assert markdown-specific output. However, markdownify is only an optional dependency (pdf/all extras). When it isn’t installed, _html_to_markdown() falls back to regex stripping and returns plain text, causing these tests to fail on a base install.

Issue Context

CI currently installs -e ".[mcp,pdf,watch]", so CI passes, but developers/packagers running pip install -e . (or consumers running tests without extras) will see failures.

Fix Focus Areas

Choose one:

  • Make markdownify a required dependency, OR

  • Mark markdownify-dependent tests with a conditional skip (e.g., pytest.importorskip("markdownify")) and keep only fallback assertions unconditionally, OR

  • Split tests into two groups: converter-present vs fallback.

  • tests/test_html_to_markdown.py[13-69]

  • pyproject.toml[44-53]

  • .github/workflows/ci.yml[24-31]

We noticed a couple of other issues in this PR as well - happy to share if helpful.


Found by Qodo code review

Reviewer (PR Graphify-Labs#586) noted that the new tests asserted markdownify-specific
output unconditionally, breaking 'pip install -e .' (no extras) test runs.
markdownify is only declared in the 'pdf'/'all' extras; on a base install
the converter falls back to regex-strip.

- Add a 'requires_markdownify' skip marker (importlib.util.find_spec, no
  side effects) and decorate the 5 tests that assert markdownify-shape
  output: ATX headings, link syntax, bullet lists, no-wrap, end-to-end
  _fetch_webpage rendering.
- Keep all fallback/regression tests unconditional.
- Add 'test_fallback_basic_text_extraction' so base installs still get
  end-to-end coverage of the regex-strip path on a realistic HTML mix.

Validation:
  with extras:   13 passed (was 12)
  without:        8 passed, 5 skipped, 0 failed
  full suite (with extras):    454 passed
  full suite (without extras): 449 passed, 5 skipped
@mrummuka

Copy link
Copy Markdown
Author

Thanks — fixed in b48a0a5. Used the skip-marker option: gated the 5 markdownify-shape assertions behind @requires_markdownify (using importlib.util.find_spec for a side-effect-free check), kept all fallback/regression tests unconditional, and added a test_fallback_basic_text_extraction smoke test so the regex-strip path gets end-to-end coverage on base installs.

Verified:

pip install -e ".[pdf]": 454 passed (full suite)
pip install -e .: 449 passed, 5 skipped, 0 failed (full suite)
Yes, please share the other issues — happy to address them in this PR.

safishamsi added a commit that referenced this pull request May 1, 2026
…ne comments, query boost, cache race, markdownify, content hash

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Collaborator

The requested changes have been implemented in 3fdae8f. html2text (GPL-3.0) replaced with markdownify (MIT) in both ingest.py and pyproject.toml. Script/style blocks pre-stripped before conversion. Shipped in v0.6.2.

@safishamsi safishamsi closed this May 1, 2026
matzls pushed a commit to matzls/graphify that referenced this pull request May 10, 2026
…bs#638 Graphify-Labs#589 Graphify-Labs#586 Graphify-Labs#593: kimi thinking, manifest, inline comments, query boost, cache race, markdownify, content hash

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

3 participants