Skip to content

fix(xpath): pre-compile expressions and sort union (|) results in DOM document order - #138

Open
Divyanshu Tiwari (divyanshu-tiwari) wants to merge 7 commits into
mainfrom
fix/xpath-union-document-order
Open

Divyanshu Tiwari (divyanshu-tiwari) wants to merge 7 commits into
mainfrom
fix/xpath-union-document-order

Conversation

@divyanshu-tiwari

@divyanshu-tiwari Divyanshu Tiwari (divyanshu-tiwari) commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • github.com/antchfx/xpath evaluates union (|) expressions branch-by-branch from left to right and concatenates the results rather than returning the node-set in global DOM document order as required by the XPath 1.0 specification.
  • Because the xpath task assigns node_index (and populates multi-value field arrays) directly from the returned slice order, permuting the branches of a | expression previously altered node_index values and output ordering.
  • Pre-compile container and fields XPath expressions once during xpath.Init() in internal/pkg/pipeline/task/xpath/xpath.go so invalid XPath syntax fails fast at pipeline initialization instead of panicking at runtime, and worker goroutines avoid per-query global LRU mutex contention.
  • Add internal/pkg/pipeline/task/xpath/query.go (compiledXPath, hasUnionOperator, matchIndex, sortMatchesInDocumentOrder) to detect unquoted | operators and order union matches (elements, text, attributes via their owning DOM element and tag attribute order, and out-of-container/document-root matches) via a pre-order DOM traversal with zero overhead for non-union XPath expressions.
  • Update internal/pkg/pipeline/task/xpath/README.md to document that matched container and field nodes are returned in DOM document order.

Benchmarks

Evaluated on darwin/arm64 (Apple M5 Pro) using multi-branch union (|) XPath expressions against two complete live HTML documents:

Document Size Matched Nodes Metric Before (htmlquery.Find) After (compiledXPath.find) Isolated Sort (sortMatchesInDocumentOrder)
1.43 MB HTML 28 nodes Latency 4.28 ms/op (4,282,326 ns/op) 4.59 ms/op (4,587,528 ns/op) 81.4 µs/op (81,422 ns/op, ~1.9% of query time)
1.43 MB HTML 28 nodes Memory / Allocs 9,655,432 B/op (336,883 allocs/op) 9,656,835 B/op (336,889 allocs/op) 888 B/op (5 allocs/op)
2.46 MB HTML 31 nodes Latency 5.39 ms/op (5,389,265 ns/op) 5.39 ms/op (5,394,093 ns/op) 69.5 µs/op (69,516 ns/op, ~1.3% of query time)
2.46 MB HTML 31 nodes Memory / Allocs 10,690,434 B/op (387,751 allocs/op) 10,692,442 B/op (387,757 allocs/op) 1,496 B/op (5 allocs/op)

Copilot AI balanced review requested due to automatic review settings November 6, 2026 07:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

XPath union ordering remains incorrect because detached attributes are appended after DOM nodes and selected document nodes can appear after their descendant container.

2 open issues
What changed in this PR

This PR addresses branch-dependent XPath union ordering so container node_index values and field arrays follow DOM document order.

Changes:

  • Routes container and field queries through a shared findNodes helper.
  • Adds pre-order DOM traversal to sort multi-node union results.
  • Documents document-order behavior for matches and container indices.
File Description
internal/​pkg/​pipeline/​task/​xpath/​xpath.go Adds union-result sorting for container and field queries.
internal/​pkg/​pipeline/​task/​xpath/​README.md Documents match ordering and node_index semantics.

Review effort: Balanced

@divyanshu-tiwari Divyanshu Tiwari (divyanshu-tiwari) changed the title fix(xpath): sort union (|) query results in DOM document order fix(xpath): pre-compile expressions and sort union (|) results in DOM document order Nov 6, 2026
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