diff --git a/cmd/knowledgehub/hooks.go b/cmd/knowledgehub/hooks.go index 065abe0..2e6781b 100644 --- a/cmd/knowledgehub/hooks.go +++ b/cmd/knowledgehub/hooks.go @@ -8,30 +8,54 @@ import ( ) func registerHooks(app *pocketbase.PocketBase) { - // On resource URL update, reset consecutive_failures to 0 and status to "healthy" + // On resource update, reset health on URL changes and clear fragment parsing + // state when fragment settings change so the next fetch can rebuild entries. app.OnRecordUpdate("resources").BindFunc(func(e *core.RecordEvent) error { - oldURL := e.Record.Original().GetString("url") - newURL := e.Record.GetString("url") - if oldURL != newURL { + oldRecord := e.Record.Original() + + if oldRecord.GetString("url") != e.Record.GetString("url") { e.Record.Set("consecutive_failures", 0) e.Record.Set("status", "healthy") } + + if fragmentConfigChanged(oldRecord, e.Record) { + e.Record.Set("fragment_hashes", "") + deleteFragmentEntries(e.App, e.Record.Id) + } + return e.Next() }) - // On resource delete, cascade delete associated entries + // On resource delete, cascade delete associated entries. app.OnRecordDelete("resources").BindFunc(func(e *core.RecordEvent) error { - resourceID := e.Record.Id - entries, err := e.App.FindRecordsByFilter("entries", "resource = {:id}", "", 0, 0, map[string]any{"id": resourceID}) - if err != nil { - log.Printf("Warning: could not find entries for resource %s: %v", resourceID, err) - return e.Next() - } - for _, entry := range entries { - if err := e.App.Delete(entry); err != nil { - log.Printf("Warning: could not delete entry %s: %v", entry.Id, err) - } - } + deleteAllResourceEntries(e.App, e.Record.Id) return e.Next() }) } + +func fragmentConfigChanged(oldRecord, newRecord *core.Record) bool { + return oldRecord.GetBool("fragment_feed") != newRecord.GetBool("fragment_feed") || + oldRecord.GetString("fragment_mode") != newRecord.GetString("fragment_mode") || + oldRecord.GetString("fragment_separator") != newRecord.GetString("fragment_separator") +} + +func deleteFragmentEntries(app core.App, resourceID string) { + deleteEntries(app, resourceID, "resource = {:id} && is_fragment = true") +} + +func deleteAllResourceEntries(app core.App, resourceID string) { + deleteEntries(app, resourceID, "resource = {:id}") +} + +func deleteEntries(app core.App, resourceID, filter string) { + entries, err := app.FindRecordsByFilter("entries", filter, "", 0, 0, map[string]any{"id": resourceID}) + if err != nil { + log.Printf("Warning: could not find entries for resource %s: %v", resourceID, err) + return + } + for _, entry := range entries { + if err := app.Delete(entry); err != nil { + log.Printf("Warning: could not delete entry %s: %v", entry.Id, err) + } + } +} diff --git a/cmd/knowledgehub/hooks_test.go b/cmd/knowledgehub/hooks_test.go new file mode 100644 index 0000000..378668a --- /dev/null +++ b/cmd/knowledgehub/hooks_test.go @@ -0,0 +1,76 @@ +package main + +import ( + "os" + "testing" + + "github.com/jgordijn/knowledgehub/internal/testutil" + "github.com/pocketbase/pocketbase" +) + +func newHooksTestApp(t *testing.T) (*pocketbase.PocketBase, func()) { + t.Helper() + + tempDir, err := os.MkdirTemp("", "kh_hooks_test_*") + if err != nil { + t.Fatalf("failed to create temp dir: %v", err) + } + + app := pocketbase.NewWithConfig(pocketbase.Config{DefaultDataDir: tempDir}) + if err := app.Bootstrap(); err != nil { + os.RemoveAll(tempDir) + t.Fatalf("failed to bootstrap app: %v", err) + } + + registerCollections(app) + registerHooks(app) + + cleanup := func() { + app.ResetBootstrapState() + os.RemoveAll(tempDir) + } + + return app, cleanup +} + +func TestRegisterHooks_ClearsFragmentStateOnFragmentConfigChange(t *testing.T) { + app, cleanup := newHooksTestApp(t) + defer cleanup() + + resource := testutil.CreateResource(t, app, "Moments", "https://example.com/feed.xml", "rss", "healthy", 0, true) + resource.Set("fragment_feed", true) + resource.Set("fragment_mode", "auto") + resource.Set("fragment_hashes", `{"guid-1":"hash-1"}`) + if err := app.Save(resource); err != nil { + t.Fatalf("failed to enable fragment feed: %v", err) + } + + fragEntry := testutil.CreateEntry(t, app, resource.Id, "Old fragment", "https://example.com/a", "frag-1") + fragEntry.Set("is_fragment", true) + if err := app.Save(fragEntry); err != nil { + t.Fatalf("failed to save fragment entry: %v", err) + } + + normalEntry := testutil.CreateEntry(t, app, resource.Id, "Normal entry", "https://example.com/b", "entry-1") + + resource.Set("fragment_mode", "separated") + resource.Set("fragment_separator", "~ ~ ~") + if err := app.Save(resource); err != nil { + t.Fatalf("failed to update fragment config: %v", err) + } + + updated, err := app.FindRecordById("resources", resource.Id) + if err != nil { + t.Fatalf("failed to reload resource: %v", err) + } + if got := updated.GetString("fragment_hashes"); got != "" { + t.Fatalf("fragment_hashes = %q, want empty", got) + } + + if _, err := app.FindRecordById("entries", fragEntry.Id); err == nil { + t.Fatal("expected fragment entry to be deleted") + } + if _, err := app.FindRecordById("entries", normalEntry.Id); err != nil { + t.Fatalf("expected normal entry to be preserved: %v", err) + } +} diff --git a/internal/engine/fragment.go b/internal/engine/fragment.go index bc2d5fb..67dafdf 100644 --- a/internal/engine/fragment.go +++ b/internal/engine/fragment.go @@ -93,22 +93,32 @@ func SplitFragments(html string) []Fragment { } // SplitFragmentsBySeparator splits HTML content into fragments using an explicit -// text separator. It walks the top-level DOM children and groups elements into -// fragments, splitting whenever an element's trimmed text content exactly matches -// the separator string. Separator elements are discarded. +// text separator. Separator matching is whitespace-normalized so feeds that emit +// inconsistent spacing like "~ ~ ~" still match a configured "~ ~ ~". The +// splitter also unwraps single top-level container elements (for example a +// wrapping
) until it reaches the level where sibling content blocks live. +// Separator elements are discarded. func SplitFragmentsBySeparator(html, separator string) []Fragment { doc, err := goquery.NewDocumentFromReader(strings.NewReader(html)) if err != nil { return nil } + separator = normalizeFragmentText(separator) + blocks := fragmentSplitBlocks(doc.Find("body"), separator) + var fragments []Fragment var current strings.Builder - doc.Find("body").Children().Each(func(i int, s *goquery.Selection) { - text := strings.TrimSpace(s.Text()) + blocks.Each(func(i int, s *goquery.Selection) { + text := normalizeFragmentText(s.Text()) + tagName := goquery.NodeName(s) - // Element is the separator — flush current fragment and discard separator + if tagName == "hr" { + return + } + + // Element is the separator — flush current fragment and discard separator. if text == separator { if current.Len() > 0 { fragments = append(fragments, newFragment(current.String())) @@ -121,7 +131,7 @@ func SplitFragmentsBySeparator(html, separator string) []Fragment { current.WriteString(h) }) - // Flush the last fragment + // Flush the last fragment. if current.Len() > 0 { fragments = append(fragments, newFragment(current.String())) } @@ -129,6 +139,34 @@ func SplitFragmentsBySeparator(html, separator string) []Fragment { return fragments } +func normalizeFragmentText(s string) string { + return strings.Join(strings.Fields(strings.TrimSpace(s)), " ") +} + +func fragmentSplitBlocks(root *goquery.Selection, separator string) *goquery.Selection { + blocks := root.Children() + for blocks.Length() == 1 && !hasSeparatorBlock(blocks, separator) { + next := blocks.First().Children() + if next.Length() == 0 { + break + } + blocks = next + } + return blocks +} + +func hasSeparatorBlock(blocks *goquery.Selection, separator string) bool { + found := false + blocks.EachWithBreak(func(_ int, s *goquery.Selection) bool { + if normalizeFragmentText(s.Text()) == separator { + found = true + return false + } + return true + }) + return found +} + // SplitFragmentsWithAI uses the heuristic splitter as a first pass, then asks // the LLM to re-group fragments that belong to the same topic. // Falls back to the heuristic result on any AI error. @@ -333,8 +371,6 @@ func titleWords(s string) map[string]bool { return words } - - // resolveContentLinks resolves relative href and src attributes in HTML content // to absolute URLs using the given base URL. This prevents relative links from // resolving against the knowledgehub domain when rendered in the browser. @@ -372,4 +408,4 @@ func resolveContentLinks(html, baseURLStr string) string { return html } return strings.TrimSpace(result) -} \ No newline at end of file +} diff --git a/internal/engine/fragment_test.go b/internal/engine/fragment_test.go index 2fc077f..7ee233b 100644 --- a/internal/engine/fragment_test.go +++ b/internal/engine/fragment_test.go @@ -285,7 +285,6 @@ func TestMergeFragments(t *testing.T) { } } - func TestResolveContentLinks(t *testing.T) { tests := []struct { name string @@ -390,7 +389,6 @@ func TestTitleSimilarity(t *testing.T) { } } - func TestContentSHA256(t *testing.T) { h1 := contentSHA256("

Hello

") h2 := contentSHA256("

Hello

") @@ -539,6 +537,43 @@ func TestSplitFragmentsBySeparator_DifferentSeparator(t *testing.T) { } } +func TestSplitFragmentsBySeparator_NormalizesInternalWhitespace(t *testing.T) { + html := `

First moment.

+

~ ~ ~

+

Second moment.

+

~ ~ ~

+

Third moment.

` + + frags := SplitFragmentsBySeparator(html, "~ ~ ~") + if len(frags) != 3 { + t.Fatalf("got %d fragments, want 3", len(frags)) + } + if !strings.Contains(frags[0].HTML, "First moment") { + t.Errorf("frag[0] should contain first moment, got %q", frags[0].HTML) + } + if !strings.Contains(frags[1].HTML, "Second moment") { + t.Errorf("frag[1] should contain second moment, got %q", frags[1].HTML) + } + if !strings.Contains(frags[2].HTML, "Third moment") { + t.Errorf("frag[2] should contain third moment, got %q", frags[2].HTML) + } +} + +func TestSplitFragmentsBySeparator_UnwrapsSingleContainer(t *testing.T) { + html := `

First moment.

~~~

Second moment.

` + + frags := SplitFragmentsBySeparator(html, "~~~") + if len(frags) != 2 { + t.Fatalf("got %d fragments, want 2", len(frags)) + } + if !strings.Contains(frags[0].HTML, "First moment") { + t.Errorf("frag[0] should contain first moment, got %q", frags[0].HTML) + } + if !strings.Contains(frags[1].HTML, "Second moment") { + t.Errorf("frag[1] should contain second moment, got %q", frags[1].HTML) + } +} + func TestSplitFragmentsBySeparator_MixedElements(t *testing.T) { html := `

Intro paragraph.

A quote.
@@ -556,4 +591,4 @@ func TestSplitFragmentsBySeparator_MixedElements(t *testing.T) { if !strings.Contains(frags[1].HTML, "Next topic") || !strings.Contains(frags[1].HTML, "Item one") { t.Errorf("frag[1] should contain next topic and list, got %q", frags[1].HTML) } -} \ No newline at end of file +} diff --git a/openspec/specs/content-fetching/spec.md b/openspec/specs/content-fetching/spec.md index 47d1d5d..aa0d6e0 100644 --- a/openspec/specs/content-fetching/spec.md +++ b/openspec/specs/content-fetching/spec.md @@ -87,12 +87,16 @@ The system SHALL discover RSS/Atom/JSON Feed URLs by parsing `