Skip to content

fix(rag): convert html to text in one linear pass - #1969

Open
murdore wants to merge 1 commit into
releasefrom
fix/codeql-rag-html-text
Open

murdore wants to merge 1 commit into
releasefrom
fix/codeql-rag-html-text

Conversation

@murdore

@murdore murdore commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Replaces the chained regex replacements that turn HTML into text in three RAG paths, and in one docs build script, with a single left-to-right scanner. CodeQL alerts addressed: #157-#160 and #166 (src/lib/rag/chunking/htmlChunker.ts), #161-#164 (src/lib/rag/document/loaders.ts), #150 and #156 (src/lib/rag/chunkers/HTMLChunker.ts), #206 (docs-site/scripts/build-llms-txt.ts). Open alerts #151 (htmlChunker.ts) and #152 (loaders.ts), both js/bad-tag-filter on the same </script> expressions, were not in my list; they are on the code this removes.

This is text conversion, not output sanitization. The result is plain text for retrieval and chunking. It is not meant to be written back into a page, and the existing entity decoding still runs afterwards and is allowed to turn &lt;b&gt; into <b>. Nothing here makes the output safe to render as HTML.

Whether the alerts close is decided by CodeQL after this PR runs, not by me. CodeQL was not available locally. What I checked is that no .replace(/<...>/...) tag chain is left in the four flagged files (grep), not what CodeQL will say about the new code.

What was wrong (behaviour, not only the static-analysis shape)

Each caller chained global replacements (script, style, comment, block end, tag). That had three real defects:

  1. Removing one construct could join the text on either side into a new tag for the next replacement: <scr<!---->ipt>.
  2. The script end-tag match missed </script > and </SCRIPT\t>, so a script body could be indexed as page text.
  3. The lazy [\s\S]*? bodies rescan the rest of the input from every unclosed <script, <style or <!--, which is quadratic. In the reversal run below the old web loader took 36,138 ms and 22,027 ms on two 400 KB documents.

What changes for a caller

No public API or type change; removeHtmlMarkup in src/lib/utils/htmlText.ts is imported by the three callers only and is not exported from any entry point (grep), so docs/api is not regenerated. The WebLoader, the legacy HTMLChunker (rag/chunking/htmlChunker.ts) and the createChunker("html") chunker (rag/chunkers/HTMLChunker.ts) call it with their own replacement text and line-break tag lists, so each keeps its previous spacing.

The scanner reads the input once. Text is copied only from outside removed spans, so a removal cannot assemble a tag, and the output never matches <[^>]+>. A < survives only when no > follows it, or as the empty <>.

Behaviour that differs, all on malformed or hostile input:

  • </script >, </SCRIPT\t> and </style x="y"> now close their element.
  • A tag that only begins with the word, such as <scripty>, is an ordinary tag, not a script element (<scripty>x</script>y now gives xy, was y).
  • A tag assembled by removing something inside it is not treated as that tag: a <scr<!---->ipt>BODY</script> z gives a ipt>BODY z, was a BODY z.
  • The createChunker("html") chunker removes a whole comment even when it holds > (<!-- a > b -->); it used to leave residue such as --> behind.
  • A bare < that is not a tag now ends at the first > after it. The loader used to turn </p> into a newline first and then let the bare < run on to a later >: 5 < 6 is true</p>after<b>x</b> gave 5 x, now 5 afterx. Both lose the <...> span; the new one keeps more text.
  • An element with no end tag still loses only its start tag, as before.

docs-site/scripts/build-llms-txt.ts gets a local removeTagSpans with the same result as replace(/<[^>]+>/g, ""). Its output is unchanged. The remaining JSX-component regexes in that function (<Tabs, <TabItem, <[A-Z]...) are untouched.

Tests

New suite test/continuous-test-suite-rag-html-markup.ts (10 cases), wired as pnpm run test:rag-html-markup and into the credential-free CI step. It takes everything from dist/index.js: WebLoader on an owned local HTTP document, the legacy HTMLChunker, and createChunker("html"). Every case runs all three paths and names the paths that are wrong; assertion messages carry path names only, never converted text.

  • Six cases pin assembled tags (script pair, comment), spaced or attributed end tags, an unclosed start tag before a closed element, name boundaries, and a comment holding >.
  • One case keeps an ordinary page's text exactly.
  • Three cases convert hostile documents (50,000 to 75,000 unclosed openers, about 400 KB) and require each path to return the exact expected text within 2,000 ms (scaled by the harness).

Per CLAUDE.md rule 15 there is no import from src/lib/. test/continuous-test-suite-rag-entity-decoding.ts is unchanged.

What I ran

All on the committed tree; machine load average was between about 40 and 240 during these runs.

  • pnpm run test:rag-html-markup: 10 of 10 pass, 0.08 to 0.19 s. pnpm exec tsx test/continuous-test-suite-rag-entity-decoding.ts: 13 of 13 pass.
  • Reversal (fails without the fix): I swapped the three compiled modules under dist/ for the pre-change sources (parent commit, transpiled as ES2022 modules), ran the suite, then restored them from backups and verified sha256 on all three (all matched). This is a swap of compiled modules, not a rebuild. Result: exit 1, 0 passed, 10 failed, each reported as a failure, not a skip. Per path: assembled script pair, spaced end tags, unclosed-then-closed, name boundary and the ordinary page failed on web loader, legacy chunker and createChunker("html"); the assembled comment case on web loader and legacy chunker; the comment holding > on createChunker("html") only; two hostile cases exceeded 2,000 ms on all three paths. The third hostile case (unclosed <script> start tags) failed with fetch failed while the old conversion blocked the process; I did not isolate that cause. After restoring, 10 of 10 pass again. Run time with the old code was 104.9 s.
  • Fuzz (fixed seed, scanner run directly on the TypeScript source): 300,000 random strings built from 41 pieces (brackets, comment, script and style openers and closers, <br>, </p>), each through three option sets (900,000 conversions): 0 outputs contained a <...> span. With empty replacements, on the same 300,000 strings, 0 outputs were changed by a second pass and 0 were not a subsequence of the input. A further 300,000 strings from a tag-only alphabet (no -, !, so no comments) matched replace(/<[^>]+>/g, "") exactly.
  • Linear work, measured by counting characters touched: I instrumented the string primitives the scanner uses (indexOf, lastIndexOf, slice, startsWith, toLowerCase, charCodeAt, charAt) and ran 41 hostile families (unclosed or mismatched script, style and comment openers, </scripty, <br plus spaces, assembled tags, long names, lone < and >) under two option sets, with the repeat count doubling from 12,500 to 200,000. Work per input character was constant across the sizes in every row, the highest being 9.00 (the <> family), and the largest growth for one doubling of the input was 2.00. The regex whitespace loop in the <br probe is not counted, but it only steps over characters the removed tag then consumes. This is the evidence for linear time; it does not depend on machine load.
  • Wall-clock cross-check, 38 families under 3 option sets (114 runs) at 50,000 to 800,000 repeats: none timed out, 0 outputs contained a <...> span, the slowest conversion was 8.2 s (the <script> x n plus <style> x n document, about 12 million characters, on the loaded machine). Timings were noisy under load; one row's 16x ratio read 109 because its 50,000 point was a warm-up outlier (698 ms at 100,000 and 4,054 ms at 800,000 for the same row), so I did not use wall-clock ratios as proof.
  • Fidelity against the old regex chains (each caller's own old chain reproduced from the parent commit): 100,000 ordinary fragments (text, entities, nested tags with attributes, void tags, script and style closed by the exact end tag, comments without > inside) gave 0 differences on all three paths. Comments holding >: 0 of 20,000 differ on the loader and legacy chunker, 9,209 of 20,000 differ on createChunker("html") (the residue case above). Fragments with a bare <: 1,497, 859 and 750 of 20,000 differ (loader, legacy, factory); I read the example above, not every case. 100,000 malformed mixes differ in 16,112, 9,333 and 10,563 cases, as intended; I read one sample, not all; no output contained a <...> span in the bare-< and malformed sets.
  • removeTagSpans against the regex: 0 differences over all 4,450 Markdown files under docs/ (14,196,910 characters) and over 300,000 random strings.
  • Pre-commit hooks passed: catalog codegen check, check (svelte-check 0 errors, tsc --noEmit --strict) and validate:all. svelte-check printed a config load error for landing/svelte.config.js (missing @sveltejs/adapter-vercel in this worktree) and reported 0 errors.

Not covered

  • CodeQL was not run, and I have not dismissed any alert.
  • Not run: pnpm test, the rest of the suites, and test/continuous-test-suite-rag.ts (it imports dotenv/config, and I was told not to read .env). Hosted CI has not run. pnpm run check:docs-api was not run by me; no exported type changed, and the pre-push hook runs it.
  • Other regexes in these files (WebLoader.extractMainContent, the legacy chunker's tag splitting) are untouched; I did not measure them and no open alert points at them.
  • No committed test for build-llms-txt.ts, because its output is identical; the evidence is the corpus and fuzz comparison above. This touches docs-site/, so the non-required Docs-site Artifacts check will run; search-index.json is not produced by this script and I did not regenerate it.
  • The suite is not added to the test:unit aggregate (one line per file was the brief); it runs in the CI step.
  • Entity decoding, htmlToMarkdown.ts and markupSniff.ts are unchanged.

Summary by CodeRabbit

  • Bug Fixes

    • Improved HTML-to-text conversion across web loading and chunking, including handling of comments, scripts, styles, malformed tags, and line breaks.
    • Preserved surrounding text and literal empty <> sequences when removing markup from generated text.
  • Tests

    • Added offline coverage for HTML markup conversion, including hostile input patterns and processing-time checks.

The WebLoader, the legacy HTMLChunker and the factory html chunker turned
HTML into text with a chain of global regex replacements (script, style,
comment, block end, generic tag). Each removal could join the text on either
side of it into a new tag for the next replacement, the end-tag match missed
`</script >`, and the lazy bodies rescanned the whole rest of the input from
every unclosed `<script`, `<style` or `<!--`, which is quadratic.

One shared scanner (utils/htmlText.ts) now reads the input once, left to
right. Text is only copied from outside removed spans, script and style end
tags may carry whitespace or attributes, and every search either consumes
what it scans or is answered from an index computed up front. The llms-txt
build script gets the same treatment for its tag strip.

This is text conversion, not output sanitization. Entity decoding is
unchanged and still happens exactly once, after markup removal.
@github-actions

Copy link
Copy Markdown
Contributor

✅ Single Commit Policy - COMPLIANT

Status: Policy requirements met • 1 commit • Valid format • Ready for merge

📊 View validation details

📝 Commit Details

  • Hash: 49c568b7e8c74ec18eafd2ba9da8eb10af832566
  • Message: fix(rag): convert html to text in one linear pass
  • Author: Sachin Sharma

✅ Validation Results

  • Single commit requirement met
  • No merge commits in branch
  • Semantic commit message format verified
  • Ready for squash merge to release branch

🤖 Automated validation by NeuroLink Single Commit Enforcement

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: juspay/neurolink/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 558a4316-ed3d-4a07-b850-7d5124ac2a8d

📥 Commits

Reviewing files that changed from the base of the PR and between 6fe6cb4 and 49c568b.


📒 Files selected for processing (8)
  • .github/workflows/ci.yml
  • docs-site/scripts/build-llms-txt.ts
  • package.json
  • src/lib/rag/chunkers/HTMLChunker.ts
  • src/lib/rag/chunking/htmlChunker.ts
  • src/lib/rag/document/loaders.ts
  • src/lib/utils/htmlText.ts
  • test/continuous-test-suite-rag-html-markup.ts

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

The change adds a shared HTML markup removal utility, updates RAG and documentation text conversion, and adds an offline test suite that runs in CI.

Changes

HTML Markup Text Conversion

Layer / File(s) Summary
Markup scanning utility
src/lib/utils/htmlText.ts
Adds a left-to-right scanner that replaces markup and comments, handles configured line breaks, and removes script and style bodies when a usable closing tag is found.
Text conversion call sites
src/lib/rag/chunkers/HTMLChunker.ts, src/lib/rag/chunking/htmlChunker.ts, src/lib/rag/document/loaders.ts, docs-site/scripts/build-llms-txt.ts
RAG converters use the shared utility with path-specific line-break options. Documentation formatting uses a local helper to remove nonempty tag spans.
Offline validation and CI wiring
test/continuous-test-suite-rag-html-markup.ts, package.json, .github/workflows/ci.yml
Adds offline checks for three HTML conversion paths, registers the test script, and runs it in the credential-free CI suites.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: pdogra1299


Merge Risk | ⚪ Minimal · up to 49c56

Merge Risk: ⚪ Minimal · up to 49c56

No identified issue currently blocks merging. Complete the normal checks before release.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 49c56

The reviewed changes consolidate text extraction without adding network authority or making the output safe to render as HTML. No material security regression was identified in the inspected paths, but broader security assurance remains incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Attacker-controlled HTML reaching the existing converters can affect extracted document or chunk text and conversion resource consumption. The inspected change does not grant additional fetch authority or introduce execution of that HTML; no tenant, datastore, or environment-wide exposure can be established from the supplied evidence.

Trust Boundaries and Controls

  • observed — WebLoader’s existing URL validation, request headers, timeout, and response checks remain outside the changed conversion implementation. Delegation therefore does not replace those retrieval controls or establish a new rendering trust boundary.
  • observed — The new test listener binds only to loopback and serves exact entries from a private fixture map. Routes are assigned synchronously before requests, preventing overlapping calls from sharing route identities. Credential isolation is imported before SDK modules.

Resilience and Maintainability Implications

  • observed — Normal and failing suite execution reaches awaited listener cleanup. Startup occurs outside that cleanup block, and abrupt termination relies on process-level socket cleanup. These exceptional lifecycle limitations concern a loopback fixture server, not a persistent production resource or credential-bearing authority.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: replacing regex-based HTML-to-text conversion with a single linear pass.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. (2 skipped: 2 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR


🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/lib/rag/chunkers/HTMLChunker.ts

Parsing error: Unable to parse the specified 'tsconfig' file. Ensure it's correct and has valid syntax.

error TS5012: Cannot read file '/.svelte-kit/tsconfig.json': ENOENT: no such file or directory, open '/.svelte-kit/tsconfig.json'.


src/lib/rag/chunking/htmlChunker.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).


src/lib/rag/document/loaders.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).


  • 2 others


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Documentation Validation Results

🚀 Documentation validation passed!

Check Status Result
Frontmatter Validation ✅ Passed
TypeScript Check ✅ Passed
Build ✅ Passed
Link Validation ✅ Passed

📦 Build artifact uploaded successfully. Ready for deployment preview.

Commit: 724225bb46805c4465a2d5f0d7cc8b29d580d511 | Workflow: View logs

This branch has not been deployed

No deployments
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