Perf: precompute list indentation metadata to avoid per-<li> goquery traversal (#3)

* Perf: precompute list indentation metadata to avoid per-<li> goquery traversal

* test regression

* remove unused

* add perf test

* test file

Author: abimaelmartell

commonmark.go

@@ -65,7 +65,8 @@ func (c *Converter) InitializeCommonMarkRules() []Rule {
 
 				// `prefixCount` is not nessesarily the length of the empty string `prefix`
 				// but how much space is reserved for the prefixes of the siblings.
-				prefixCount, previousPrefixCounts := countListParents(opt, selec)
+				prefixCount, _ := strconv.Atoi(selec.AttrOr(attrListPrefixCount, "0"))
+				previousPrefixCounts, _ := strconv.Atoi(selec.AttrOr(attrListPrevPrefixCounts, "0"))
 
 				// if the prefix is not needed, balance it by adding the usual prefix spaces
 				if prefix == "" {

from.go

@@ -100,6 +100,10 @@ func validateOptions(opt Options) error {
 
 var (
 	attrListPrefix = "data-converter-list-prefix"
+	// Cached list indentation metadata, computed once in a pre-pass over the DOM.
+	// This avoids extremely expensive goquery traversal per <li> on large documents.
+	attrListPrefixCount       = "data-converter-prefix-count"
+	attrListPrevPrefixCounts  = "data-converter-prev-prefix-counts"
 )
 
 // NewConverter initializes a new converter and holds all the rules.
@@ -120,13 +124,6 @@ func NewConverter(domain string, enableCommonmark bool, options *Options) *Conve
 			s.SetAttr("data-index", strconv.Itoa(i+1))
 		})
 	})
-	conv.before = append(conv.before, func(selec *goquery.Selection) {
-		selec.Find("li").Each(func(i int, s *goquery.Selection) {
-			prefix := getListPrefix(options, s)
-
-			s.SetAttr(attrListPrefix, prefix)
-		})
-	})
 	conv.after = append(conv.after, func(markdown string) string {
 		markdown = strings.TrimSpace(markdown)
 		markdown = multipleNewLinesRegex.ReplaceAllString(markdown, "\n\n")
@@ -382,6 +379,10 @@ func (conv *Converter) Convert(selec *goquery.Selection) string {
 		hook(selec)
 	}
 
+	// Precompute list indentation metadata *after* before hooks (so any DOM mutations
+	// performed by user hooks are reflected).
+	annotateListIndentation(selec, &options)
+
 	res := conv.selecToMD(selec, &options)
 	markdown := res.Markdown
 

list_regression_test.go

@@ -0,0 +1,80 @@
+package md_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/PuerkitoBio/goquery"
+	md "github.com/firecrawl/html-to-markdown"
+)
+
+func TestListRegression_NonLiChildInOrderedListSkipsIndex(t *testing.T) {
+	conv := md.NewConverter("", true, nil)
+
+	html := `<ol><li>one</li><div></div><li>two</li></ol>`
+	out, err := conv.ConvertString(html)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// goquery's Index counts element siblings, so the second <li> becomes index 2 => "3."
+	if !strings.Contains(out, "1. one") {
+		t.Fatalf("expected output to contain %q, got:\n%s", "1. one", out)
+	}
+	if !strings.Contains(out, "3. two") {
+		t.Fatalf("expected output to contain %q (due to non-li sibling), got:\n%s", "3. two", out)
+	}
+	if strings.Contains(out, "2. two") {
+		t.Fatalf("did not expect output to contain %q, got:\n%s", "2. two", out)
+	}
+}
+
+func TestListRegression_WrapperListItemDoesNotEmitEmptyBullet(t *testing.T) {
+	conv := md.NewConverter("", true, nil)
+
+	html := `<ul><li><ul><li>Nested</li></ul></li></ul>`
+	out, err := conv.ConvertString(html)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !strings.Contains(out, "Nested") {
+		t.Fatalf("expected output to contain Nested, got:\n%s", out)
+	}
+	for _, line := range strings.Split(out, "\n") {
+		if strings.TrimSpace(line) == "-" {
+			t.Fatalf("unexpected empty list marker line, got:\n%s", out)
+		}
+	}
+}
+
+func TestListRegression_MalformedLiOutsideListDoesNotPanic(t *testing.T) {
+	conv := md.NewConverter("", true, nil)
+
+	html := `<div><li>Item</li></div>`
+	out, err := conv.ConvertString(html)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(out, "Item") {
+		t.Fatalf("expected output to contain Item, got:\n%s", out)
+	}
+}
+
+func TestListRegression_BeforeHookMutationIsIncludedInListMetadata(t *testing.T) {
+	conv := md.NewConverter("", true, nil)
+	conv.Before(func(selec *goquery.Selection) {
+		selec.Find("ol").First().AppendHtml("<li>b</li>")
+	})
+
+	out, err := conv.ConvertString(`<ol><li>a</li></ol>`)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !strings.Contains(out, "1. a") || !strings.Contains(out, "2. b") {
+		t.Fatalf("expected mutated list items to be numbered, got:\n%s", out)
+	}
+}
+
+

perf_big_html_test.go

@@ -19,9 +19,33 @@ import (
 func TestPerfBigHTML_Smoke(t *testing.T) {
 	p := filepath.Join("testdata", "Perf", "big.html")
 
-	if _, err := os.Stat(p); err != nil {
-		p = filepath.Join("testdata", "Perf", "test.html")
+	b, err := os.ReadFile(p)
+	if err != nil {
+		t.Fatalf("read %s: %v", p, err)
 	}
+	html := string(b)
+
+	conv := md.NewConverter("", true, nil)
+	out, err := conv.ConvertString(html)
+	if err != nil {
+		t.Fatalf("convert: %v", err)
+	}
+	if strings.TrimSpace(out) == "" {
+		t.Fatalf("expected non-empty markdown output")
+	}
+}
+
+// TestPerfBigList_Smoke runs a large list (random-ish content),
+// converts it once, and asserts we get non-empty output.
+//
+// This is intentionally a "smoke" perf test: no golden output, just "it renders".
+// Run it alone to track timings over time:
+//
+//	go test -run '^TestPerfBigList_Smoke$' -count=1
+//
+// The input file is a large list with 100,000 items.
+func TestPerfBigList_Smoke(t *testing.T) {
+	p := filepath.Join("testdata", "Perf", "big_list.html")
 	b, err := os.ReadFile(p)
 	if err != nil {
 		t.Fatalf("read %s: %v", p, err)