refactor: use appstream data keywords only for keywords bc. other data (summary, description etc.) was too messy and thus messed up search rankings

Author: m1rm

ARCHITECTURE.md

@@ -52,9 +52,9 @@ Six CLI subcommands fetch data from external sources, invoked by external system
 
 ## Search
 
-FTS5 indexes: name, base, description, groups, provides (denormalized from package relations). Hyphenated queries are split into individual terms for tokenizer compatibility.
+FTS5 indexes: name, base, description, groups, provides, keywords (AppStream `<keyword>` inside `<keywords>` blocks whose `xml:lang` is absent or en/de). Hyphenated queries are split into individual terms for tokenizer compatibility.
 
-Ranking: exact name match first, then `bm25(10,5,1,1,3) - ln(1+popularity)` — name-weighted with log-scaled popularity boost.
+Ranking: exact name match first, then `bm25` with higher weights on name/description than on keywords (so long keyword blobs do not bury short pacman descriptions), minus `ln(1+popularity)`.
 
 ## Middleware Stack
 

internal/appstream/parse.go

@@ -2,11 +2,12 @@
 // https://sources.archlinux.org/other/packages/archlinux-appstream-data/) and
 // builds per-pkgname search text for SQLite FTS.
 //
-// Parsing model: encoding/xml streams tokens (start/end/CharData); we keep one
-// open component in docParser.cur. stack + muteLeaf track the path so
-// text is attributed to the right element; name/summary with xml:lang outside
-// en/de are skipped. flush runs at </component> (and before the next <component>)
-// to emit (pkgname, parts); the caller merges duplicate pkgnames.
+// Parsing model: encoding/xml streams tokens; we keep one open <component> in
+// docParser.cur. Only <pkgname> and <keywords>/<keyword> text are read; other
+// elements (name, summary, description, categories) are ignored for indexing.
+// AppStream puts xml:lang on <keywords> blocks (not each <keyword>). Only blocks
+// with no lang or en/de are indexed; per-<keyword> xml:lang is also respected when present.
+// flush runs at </component> (and before the next <component>) to emit (pkgname, parts).
 package appstream
 
 import (
@@ -22,8 +23,8 @@ const (
 )
 
 // ParseComponentsXML streams the decoder and calls fn once per completed
-// <component> (same pkgname may appear many times). fn receives raw text
-// fragments in parts; dedupeWords runs in the caller after merge.
+// <component> (same pkgname may appear many times). parts contains only
+// <keyword> text; dedupeWords runs in the caller after merge.
 func ParseComponentsXML(r io.Reader, fn func(pkgname string, parts []string) error) error {
 	d := xml.NewDecoder(r)
 	d.Strict = false
@@ -52,18 +53,18 @@ func ParseComponentsXML(r io.Reader, fn func(pkgname string, parts []string) err
 }
 
 // docParser holds decoder state between tokens.
-// stack/muteLeaf are parallel: element names and whether that leaf skips CharData (non-en/de name/summary).
-// inKeywords/inKeyword/inCats/inDesc gate text from nested sections. cur is the open <component> or nil.
+// keywordsBlockSkip drops a whole <keywords xml:lang="…"> block when not en/de/neutral.
+// keywordSkip does the same for a single <keyword> when it carries xml:lang.
+// cur is the open <component> or nil.
 type docParser struct {
-	fn         func(string, []string) error
-	dec        *xml.Decoder
-	stack      []string
-	muteLeaf   []bool
-	inKeywords bool
-	inKeyword  bool
-	inCats     bool
-	inDesc     int
-	cur        *component
+	fn                func(string, []string) error
+	dec               *xml.Decoder
+	stack             []string
+	inKeywords        bool
+	keywordsBlockSkip bool
+	inKeyword         bool
+	keywordSkip       bool
+	cur               *component
 }
 
 // flush emits cur via fn and clears it. EOF calls flush for the last component.
@@ -80,23 +81,10 @@ func (p *docParser) flush() error {
 	return p.fn(name, parts)
 }
 
-// startElement pushes stack/muteLeaf; on <component> flushes the previous component then starts a new cur.
+// startElement pushes stack; on <component> flushes the previous component then starts a new cur.
 func (p *docParser) startElement(t xml.StartElement) error {
 	local := t.Name.Local
 	p.stack = append(p.stack, local)
-	muted := false
-	if local == "name" || local == "summary" {
-		for _, a := range t.Attr {
-			if a.Name.Local != "lang" || a.Value == "" {
-				continue
-			}
-			if a.Value != "en" && a.Value != "de" {
-				muted = true
-				break
-			}
-		}
-	}
-	p.muteLeaf = append(p.muteLeaf, muted)
 
 	switch local {
 	case "component":
@@ -106,56 +94,43 @@ func (p *docParser) startElement(t xml.StartElement) error {
 		p.cur = &component{}
 	case "keywords":
 		p.inKeywords = true
+		p.keywordsBlockSkip = !keywordLangAccepted(t.Attr)
 	case elKeyword:
 		if p.inKeywords {
 			p.inKeyword = true
+			p.keywordSkip = !keywordLangAccepted(t.Attr)
 		}
-	case "categories":
-		p.inCats = true
-	case "description":
-		p.inDesc++
 	}
 	return nil
 }
 
-// endElement pops stack/muteLeaf; on </component> flushes the finished component.
+// endElement pops stack; on </component> flushes the finished component.
 func (p *docParser) endElement(t xml.EndElement) error {
 	local := t.Name.Local
 	if len(p.stack) == 0 {
 		return nil
 	}
 	p.stack = p.stack[:len(p.stack)-1]
-	if len(p.muteLeaf) > 0 {
-		p.muteLeaf = p.muteLeaf[:len(p.muteLeaf)-1]
-	}
 
 	switch local {
 	case "component":
 		return p.flush()
 	case "keywords":
 		p.inKeywords = false
 		p.inKeyword = false
+		p.keywordsBlockSkip = false
 	case elKeyword:
 		p.inKeyword = false
-	case "categories":
-		p.inCats = false
-	case "description":
-		if p.inDesc > 0 {
-			p.inDesc--
-		}
+		p.keywordSkip = false
 	}
 	return nil
 }
 
-// charData routes text to pkgname or parts by parent element name (stack tip).
+// charData collects pkgname and <keyword> text only.
 func (p *docParser) charData(t xml.CharData) {
 	if p.cur == nil {
 		return
 	}
-	muted := len(p.muteLeaf) > 0 && p.muteLeaf[len(p.muteLeaf)-1]
-	if muted {
-		return
-	}
 	text := strings.TrimSpace(string(t))
 	if text == "" {
 		return
@@ -169,21 +144,27 @@ func (p *docParser) charData(t xml.CharData) {
 	switch parent {
 	case "pkgname":
 		p.cur.pkgname += text
-	case "name", "summary":
-		p.cur.parts = append(p.cur.parts, text)
-	case "category":
-		if p.inCats {
-			p.cur.parts = append(p.cur.parts, text)
-		}
 	case elKeyword:
-		if p.inKeyword {
+		if p.inKeyword && !p.keywordsBlockSkip && !p.keywordSkip {
 			p.cur.parts = append(p.cur.parts, text)
 		}
-	case "p":
-		if p.inDesc > 0 {
-			p.cur.parts = append(p.cur.parts, text)
+	}
+}
+
+// keywordLangAccepted is used for both <keywords> and <keyword> start tags: true if
+// there is no xml:lang, or it is en/de (including BCP47 prefixes like de-DE).
+func keywordLangAccepted(attrs []xml.Attr) bool {
+	for _, a := range attrs {
+		if a.Name.Local != "lang" || a.Value == "" {
+			continue
+		}
+		v := strings.ToLower(strings.TrimSpace(a.Value))
+		if i := strings.IndexByte(v, '-'); i > 0 {
+			v = v[:i]
 		}
+		return v == "en" || v == "de"
 	}
+	return true
 }
 
 // component is one AppStream <component> being accumulated until flush.

internal/appstream/parse_test.go

@@ -1,27 +1,26 @@
 package appstream
 
 import (
+	"encoding/xml"
 	"strings"
 	"testing"
 )
 
-func TestParseComponentsXML(t *testing.T) {
+func TestParseComponentsXML_KeywordsOnly(t *testing.T) {
 	const xml = `<?xml version="1.0"?>
 <components>
 <component type="desktop-application">
   <pkgname>firefox</pkgname>
   <name>Firefox</name>
   <summary>Web browser</summary>
-  <summary xml:lang="de">Webbrowser</summary>
-  <summary xml:lang="fr">Navigateur</summary>
   <description><p>Free software web browser.</p></description>
-  <categories><category>WebBrowser</category><category>Network</category></categories>
+  <categories><category>WebBrowser</category></categories>
   <keywords><keyword>internet</keyword><keyword>www</keyword></keywords>
 </component>
 <component type="desktop-application">
   <pkgname>firefox</pkgname>
   <name>Firefox ESR</name>
-  <summary>Extended support</summary>
+  <keywords><keyword>mozilla</keyword></keywords>
 </component>
 </components>`
 
@@ -37,13 +36,71 @@ func TestParseComponentsXML(t *testing.T) {
 	if got == "" {
 		t.Fatal("expected merged keywords for firefox")
 	}
-	if !strings.Contains(got, "Webbrowser") {
-		t.Errorf("expected German summary term, got %q", got)
+	if !strings.Contains(got, "internet") || !strings.Contains(got, "www") || !strings.Contains(got, "mozilla") {
+		t.Errorf("expected AppStream <keyword> terms only, got %q", got)
 	}
-	if strings.Contains(strings.ToLower(got), "navigateur") {
-		t.Errorf("did not expect French summary, got %q", got)
+	if strings.Contains(strings.ToLower(got), "browser") || strings.Contains(got, "WebBrowser") {
+		t.Errorf("did not expect description/name/category text in keywords, got %q", got)
 	}
-	if !strings.Contains(got, "Network") || !strings.Contains(got, "internet") {
-		t.Errorf("expected category and keyword terms, got %q", got)
+}
+
+func TestParseComponentsXML_KeywordLangFilter(t *testing.T) {
+	// AppStream often sets xml:lang on <keywords>, not on each <keyword>.
+	const xml = `<?xml version="1.0"?>
+<components>
+<component>
+  <pkgname>demo</pkgname>
+  <keywords>
+    <keyword>neutral</keyword>
+  </keywords>
+  <keywords xml:lang="en">
+    <keyword>english</keyword>
+  </keywords>
+  <keywords xml:lang="de">
+    <keyword>deutsch</keyword>
+  </keywords>
+  <keywords xml:lang="de-DE">
+    <keyword>deutsch2</keyword>
+  </keywords>
+  <keywords xml:lang="fr">
+    <keyword>francais</keyword>
+  </keywords>
+</component>
+</components>`
+	acc := make(map[string][]string)
+	err := ParseComponentsXML(strings.NewReader(xml), func(name string, parts []string) error {
+		acc[name] = append(acc[name], parts...)
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := dedupeWords(acc["demo"])
+	for _, need := range []string{"neutral", "english", "deutsch", "deutsch2"} {
+		if !strings.Contains(got, need) {
+			t.Errorf("missing %q in %q", need, got)
+		}
+	}
+	if strings.Contains(got, "francais") {
+		t.Errorf("did not want fr keyword, got %q", got)
+	}
+}
+
+func TestKeywordLangAccepted(t *testing.T) {
+	tests := []struct {
+		attrs []xml.Attr
+		want  bool
+	}{
+		{nil, true},
+		{[]xml.Attr{{Name: xml.Name{Local: "lang"}, Value: "en"}}, true},
+		{[]xml.Attr{{Name: xml.Name{Local: "lang"}, Value: "de"}}, true},
+		{[]xml.Attr{{Name: xml.Name{Local: "lang"}, Value: "de-AT"}}, true},
+		{[]xml.Attr{{Name: xml.Name{Local: "lang"}, Value: "fr"}}, false},
+		{[]xml.Attr{{Name: xml.Name{Local: "lang"}, Value: "pl"}}, false},
+	}
+	for _, tt := range tests {
+		if got := keywordLangAccepted(tt.attrs); got != tt.want {
+			t.Errorf("keywordLangAccepted(%v) = %v, want %v", tt.attrs, got, tt.want)
+		}
 	}
 }

internal/packages/repository.go

@@ -4,11 +4,25 @@ import (
 	"context"
 	"database/sql"
 	"errors"
+	"fmt"
 	"strings"
 
 	fts "archded/internal/search"
 )
 
+// bm25Weights are per FTS5 column: name, base, description, groups, provides, keywords.
+// Long AppStream keyword fields increase BM25 document length; description (short pacman
+// text) must stay heavily weighted so queries like "browser" still rank packages that
+// only match strongly there.
+const (
+	bm25Name        = 12
+	bm25Base        = 5
+	bm25Description = 10
+	bm25Groups      = 1
+	bm25Provides    = 3
+	bm25Keywords    = 0.5
+)
+
 type PackageSummary struct {
 	Repository    string
 	Architecture  string
@@ -93,8 +107,12 @@ func (r *Repository) Search(ctx context.Context, search, repo, arch string, limi
 		}
 
 		countQuery = `SELECT COUNT(*) ` + baseWhere
+		bm25 := fmt.Sprintf(
+			"bm25(package_fts, %d, %d, %d, %d, %d, %g)",
+			bm25Name, bm25Base, bm25Description, bm25Groups, bm25Provides, bm25Keywords,
+		)
 		dataQuery = `SELECT r.name, r.architecture, p.name, p.version, p.description, p.build_date, p.popularity_recent, r.testing
-			` + baseWhere + ` ORDER BY (p.name = ?) DESC, bm25(package_fts, 10, 5, 1, 1, 3, 2) - ln(1 + p.popularity_recent), p.build_date DESC LIMIT ? OFFSET ?`
+			` + baseWhere + ` ORDER BY (p.name = ?) DESC, ` + bm25 + ` - ln(1 + p.popularity_recent), p.build_date DESC LIMIT ? OFFSET ?`
 		dataArgs = append(dataArgs, search, limit, offset)
 	} else {
 		baseWhere := `FROM package p