From e6493245ebbd791cc76a0ce5090bcb2f4365ba5a Mon Sep 17 00:00:00 2001 From: Akeem Jenkins Date: Fri, 26 Jun 2026 23:02:48 -0600 Subject: [PATCH 1/2] feat: graph and backlinks parse frontmatter links field --- internal/backlinks/backlinks.go | 5 +- internal/backlinks/backlinks_test.go | 15 ++++++ internal/concept/concept.go | 1 + internal/graph/graph.go | 7 ++- internal/graph/graph_test.go | 79 ++++++++++++++++++++++++++++ internal/validate/validate.go | 15 ++++++ internal/validate/validate_test.go | 25 +++++++++ 7 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 internal/graph/graph_test.go diff --git a/internal/backlinks/backlinks.go b/internal/backlinks/backlinks.go index 8e4b458..fc9686f 100644 --- a/internal/backlinks/backlinks.go +++ b/internal/backlinks/backlinks.go @@ -21,8 +21,9 @@ func Backlinks(b *bundle.Bundle, conceptID string) []string { if c.ID == conceptID { continue // don't self-report } - links := validate.ExtractLinks(c.Body) - for _, link := range links { + bodyLinks := validate.ExtractLinks(c.Body) + fmLinks := validate.ExtractFrontmatterLinks(c.Frontmatter.Links) + for _, link := range append(bodyLinks, fmLinks...) { target := validate.ResolveLink(c.ID, link) if target == conceptID { if !seen[c.ID] { diff --git a/internal/backlinks/backlinks_test.go b/internal/backlinks/backlinks_test.go index 430e876..519f04d 100644 --- a/internal/backlinks/backlinks_test.go +++ b/internal/backlinks/backlinks_test.go @@ -96,6 +96,21 @@ func TestBacklinks_DeduplicatesLinks(t *testing.T) { } } +func TestBacklinks_FrontmatterLinks(t *testing.T) { + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\nlinks:\n - /b\n---\n\nNo body links here.", + "b.md": "---\ntype: T\ntitle: B\n---\n\nbody", + }) + + links := Backlinks(b, "b") + if len(links) != 1 { + t.Fatalf("got %d backlinks, want 1 (from frontmatter link)", len(links)) + } + if links[0] != "a" { + t.Errorf("got %s, want a", links[0]) + } +} + // --- helpers --- func testBundle(t *testing.T, files map[string]string) *bundle.Bundle { diff --git a/internal/concept/concept.go b/internal/concept/concept.go index 6e376da..168f1cf 100644 --- a/internal/concept/concept.go +++ b/internal/concept/concept.go @@ -22,6 +22,7 @@ type Frontmatter struct { Resource string `yaml:"resource"` Tags []string `yaml:"tags"` Timestamp time.Time `yaml:"timestamp"` + Links []string `yaml:"links"` Extensions map[string]any `yaml:",inline"` } diff --git a/internal/graph/graph.go b/internal/graph/graph.go index 3499881..c90b783 100644 --- a/internal/graph/graph.go +++ b/internal/graph/graph.go @@ -45,8 +45,11 @@ func Build(b *bundle.Bundle) *Graph { edgeSet := make(map[string]bool) // "from\x00to" dedup for _, c := range b.Concepts { - links := validate.ExtractLinks(c.Body) - for _, link := range links { + // Collect body links and frontmatter links, then process them + // uniformly. The edgeSet dedup handles overlaps. + bodyLinks := validate.ExtractLinks(c.Body) + fmLinks := validate.ExtractFrontmatterLinks(c.Frontmatter.Links) + for _, link := range append(bodyLinks, fmLinks...) { target := resolveLink(c.ID, link) if target == "" || !b.HasConcept(target) { continue diff --git a/internal/graph/graph_test.go b/internal/graph/graph_test.go new file mode 100644 index 0000000..4cf9841 --- /dev/null +++ b/internal/graph/graph_test.go @@ -0,0 +1,79 @@ +package graph + +import ( + "os" + "path/filepath" + "testing" + + "github.com/okfcli/okf/internal/bundle" +) + +func TestBuild_BodyLinks(t *testing.T) { + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\n---\n\nLinks to [B](b.md).", + "b.md": "---\ntype: T\ntitle: B\n---\n\nbody", + }) + + g := Build(b) + if len(g.Edges) != 1 { + t.Fatalf("got %d edges, want 1", len(g.Edges)) + } + if g.Edges[0].From != "a" || g.Edges[0].To != "b" { + t.Errorf("edge = %v, want a->b", g.Edges[0]) + } +} + +func TestBuild_FrontmatterLinksOnly(t *testing.T) { + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\nlinks:\n - /b\n---\n\nNo body links here.", + "b.md": "---\ntype: T\ntitle: B\n---\n\nbody", + }) + + g := Build(b) + if len(g.Edges) != 1 { + t.Fatalf("got %d edges, want 1 (from frontmatter link)", len(g.Edges)) + } + if g.Edges[0].From != "a" || g.Edges[0].To != "b" { + t.Errorf("edge = %v, want a->b", g.Edges[0]) + } + // Backlinks should also be populated. + if len(g.Backlinks["b"]) != 1 || g.Backlinks["b"][0] != "a" { + t.Errorf("backlinks[b] = %v, want [a]", g.Backlinks["b"]) + } +} + +func TestBuild_FrontmatterAndBodyDeduped(t *testing.T) { + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\nlinks:\n - /b\n---\n\nAlso links in [body](b.md).", + "b.md": "---\ntype: T\ntitle: B\n---\n\nbody", + }) + + g := Build(b) + if len(g.Edges) != 1 { + t.Fatalf("got %d edges, want 1 (deduped frontmatter+body)", len(g.Edges)) + } + if g.Edges[0].From != "a" || g.Edges[0].To != "b" { + t.Errorf("edge = %v, want a->b", g.Edges[0]) + } +} + +// --- helpers --- + +func testBundle(t *testing.T, files map[string]string) *bundle.Bundle { + t.Helper() + dir := t.TempDir() + for path, content := range files { + full := filepath.Join(dir, path) + if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(full, []byte(content), 0644); err != nil { + t.Fatalf("write: %v", err) + } + } + b, err := bundle.Load(dir) + if err != nil { + t.Fatalf("Load: %v", err) + } + return b +} diff --git a/internal/validate/validate.go b/internal/validate/validate.go index 408a3c5..66acdf7 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -121,6 +121,21 @@ func ExtractLinks(body string) []Link { return extractLinks(body) } +// ExtractFrontmatterLinks converts a frontmatter "links:" list (concept IDs or +// absolute /paths) into Link structs. Empty strings are skipped. The Text is +// empty (frontmatter links have no link text); the Target is preserved as-is, +// including any leading "/" — ResolveLink handles the stripping. +func ExtractFrontmatterLinks(links []string) []Link { + out := make([]Link, 0, len(links)) + for _, l := range links { + if strings.TrimSpace(l) == "" { + continue + } + out = append(out, Link{Text: "", Target: l}) + } + return out +} + // extractLinks parses all markdown links [text](target) from a body. func extractLinks(body string) []Link { var links []Link diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index 3964853..df76d25 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -67,6 +67,31 @@ func TestResolveLink_WithFragment(t *testing.T) { } } +func TestExtractFrontmatterLinks(t *testing.T) { + links := ExtractFrontmatterLinks([]string{"/tables/events_", "users", "", "/playbooks/check"}) + if len(links) != 3 { + t.Fatalf("got %d links, want 3 (empty skipped): %+v", len(links), links) + } + if links[0].Text != "" || links[0].Target != "/tables/events_" { + t.Errorf("link[0] = %+v, want Text=\"\" Target=/tables/events_", links[0]) + } + if links[1].Target != "users" { + t.Errorf("link[1] target = %q, want users", links[1].Target) + } + if links[2].Target != "/playbooks/check" { + t.Errorf("link[2] target = %q, want /playbooks/check", links[2].Target) + } +} + +func TestExtractFrontmatterLinks_Empty(t *testing.T) { + if links := ExtractFrontmatterLinks(nil); len(links) != 0 { + t.Fatalf("got %d links, want 0", len(links)) + } + if links := ExtractFrontmatterLinks([]string{}); len(links) != 0 { + t.Fatalf("got %d links, want 0", len(links)) + } +} + func TestNormalizePath(t *testing.T) { tests := []struct { in, want string From 1ccca768a30f0cb227f60e8e8004c4397a735799 Mon Sep 17 00:00:00 2001 From: Akeem Jenkins Date: Wed, 8 Jul 2026 10:48:20 -0600 Subject: [PATCH 2/2] Make frontmatter links robust and validated - concept: add StringList type with a custom UnmarshalYAML so `links` accepts both a scalar string (`links: /b`) and a sequence, and tolerates a malformed shape (e.g. a mapping) as empty instead of failing the whole bundle load. Missing/empty stays a nil slice. - validate: validateLinks now also checks frontmatter links via ExtractFrontmatterLinks, so a frontmatter link to a nonexistent concept is reported instead of silently dropped. - graph: skip self-edges (target == concept ID); combine body and frontmatter links into a fresh slice to avoid append aliasing. - tests: scalar/sequence/missing/malformed links parsing, deduped and self-link graph cases, and frontmatter broken-link validation. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/concept/concept.go | 43 ++++++++++++++++++++++- internal/concept/concept_test.go | 54 +++++++++++++++++++++++++++++ internal/graph/graph.go | 9 +++-- internal/graph/graph_test.go | 48 ++++++++++++++++++++++++++ internal/validate/validate.go | 9 ++++- internal/validate/validate_test.go | 55 ++++++++++++++++++++++++++++++ 6 files changed, 213 insertions(+), 5 deletions(-) create mode 100644 internal/concept/concept_test.go diff --git a/internal/concept/concept.go b/internal/concept/concept.go index 168f1cf..e787d0d 100644 --- a/internal/concept/concept.go +++ b/internal/concept/concept.go @@ -22,10 +22,51 @@ type Frontmatter struct { Resource string `yaml:"resource"` Tags []string `yaml:"tags"` Timestamp time.Time `yaml:"timestamp"` - Links []string `yaml:"links"` + Links StringList `yaml:"links"` Extensions map[string]any `yaml:",inline"` } +// StringList is a []string that unmarshals from YAML flexibly: it accepts both +// a single scalar string (links: /b) and a sequence of strings (links: [/b, /c]). +// A missing/empty value stays a nil/empty slice, and a malformed shape (e.g. a +// mapping) is tolerated as an empty slice rather than failing the whole parse — +// so one badly-shaped concept cannot take down an entire bundle load. +type StringList []string + +// UnmarshalYAML implements yaml.Unmarshaler, accepting scalar-or-sequence. +func (s *StringList) UnmarshalYAML(value *yaml.Node) error { + switch value.Kind { + case yaml.ScalarNode: + // A single string, or an explicit null. Treat null as empty. + if value.Tag == "!!null" { + *s = nil + return nil + } + var single string + if err := value.Decode(&single); err != nil { + // Non-string scalar (e.g. a number) — tolerate as empty. + *s = nil + return nil + } + *s = StringList{single} + return nil + case yaml.SequenceNode: + var list []string + if err := value.Decode(&list); err != nil { + // A sequence of non-strings — tolerate as empty rather than + // failing the entire bundle load. + *s = nil + return nil + } + *s = StringList(list) + return nil + default: + // Mapping or any other unexpected shape — tolerate as empty. + *s = nil + return nil + } +} + // Concept is a parsed concept document. type Concept struct { // ID is the file path within the bundle with .md removed, e.g. "tables/users". diff --git a/internal/concept/concept_test.go b/internal/concept/concept_test.go new file mode 100644 index 0000000..ccecd30 --- /dev/null +++ b/internal/concept/concept_test.go @@ -0,0 +1,54 @@ +package concept + +import ( + "reflect" + "testing" +) + +func TestParse_LinksScalar(t *testing.T) { + raw := []byte("---\ntype: T\ntitle: A\nlinks: /b\n---\n\nbody") + c, err := ParseBytes(raw, "a.md", "/tmp/a.md") + if err != nil { + t.Fatalf("ParseBytes: %v", err) + } + want := StringList{"/b"} + if !reflect.DeepEqual(c.Frontmatter.Links, want) { + t.Errorf("Links = %v, want %v", c.Frontmatter.Links, want) + } +} + +func TestParse_LinksSequence(t *testing.T) { + raw := []byte("---\ntype: T\ntitle: A\nlinks:\n - /b\n - /c\n---\n\nbody") + c, err := ParseBytes(raw, "a.md", "/tmp/a.md") + if err != nil { + t.Fatalf("ParseBytes: %v", err) + } + want := StringList{"/b", "/c"} + if !reflect.DeepEqual(c.Frontmatter.Links, want) { + t.Errorf("Links = %v, want %v", c.Frontmatter.Links, want) + } +} + +func TestParse_LinksMissing(t *testing.T) { + raw := []byte("---\ntype: T\ntitle: A\n---\n\nbody") + c, err := ParseBytes(raw, "a.md", "/tmp/a.md") + if err != nil { + t.Fatalf("ParseBytes: %v", err) + } + if len(c.Frontmatter.Links) != 0 { + t.Errorf("Links = %v, want empty", c.Frontmatter.Links) + } +} + +func TestParse_LinksMalformedShapeTolerated(t *testing.T) { + // A mapping (or otherwise unexpected) shape must not fail the parse — it is + // tolerated as an empty list so one bad concept cannot kill a bundle load. + raw := []byte("---\ntype: T\ntitle: A\nlinks:\n target: /b\n---\n\nbody") + c, err := ParseBytes(raw, "a.md", "/tmp/a.md") + if err != nil { + t.Fatalf("ParseBytes should tolerate malformed links, got error: %v", err) + } + if len(c.Frontmatter.Links) != 0 { + t.Errorf("Links = %v, want empty for malformed shape", c.Frontmatter.Links) + } +} diff --git a/internal/graph/graph.go b/internal/graph/graph.go index c90b783..fc18940 100644 --- a/internal/graph/graph.go +++ b/internal/graph/graph.go @@ -49,10 +49,13 @@ func Build(b *bundle.Bundle) *Graph { // uniformly. The edgeSet dedup handles overlaps. bodyLinks := validate.ExtractLinks(c.Body) fmLinks := validate.ExtractFrontmatterLinks(c.Frontmatter.Links) - for _, link := range append(bodyLinks, fmLinks...) { + links := make([]validate.Link, 0, len(bodyLinks)+len(fmLinks)) + links = append(links, bodyLinks...) + links = append(links, fmLinks...) + for _, link := range links { target := resolveLink(c.ID, link) - if target == "" || !b.HasConcept(target) { - continue + if target == "" || target == c.ID || !b.HasConcept(target) { + continue // skip external, unresolved, self, and dangling links } key := c.ID + "\x00" + target if !edgeSet[key] { diff --git a/internal/graph/graph_test.go b/internal/graph/graph_test.go index 4cf9841..3dfd003 100644 --- a/internal/graph/graph_test.go +++ b/internal/graph/graph_test.go @@ -57,6 +57,54 @@ func TestBuild_FrontmatterAndBodyDeduped(t *testing.T) { } } +func TestBuild_FrontmatterLinkScalar(t *testing.T) { + // A single-string `links: /b` (scalar, not a sequence) must load fine and + // produce the same edge as the sequence form. + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\nlinks: /b\n---\n\nNo body links here.", + "b.md": "---\ntype: T\ntitle: B\n---\n\nbody", + }) + + g := Build(b) + if len(g.Edges) != 1 { + t.Fatalf("got %d edges, want 1 (from scalar frontmatter link)", len(g.Edges)) + } + if g.Edges[0].From != "a" || g.Edges[0].To != "b" { + t.Errorf("edge = %v, want a->b", g.Edges[0]) + } +} + +func TestBuild_FrontmatterLinksDeduped(t *testing.T) { + // Duplicate frontmatter links collapse to a single edge. + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\nlinks:\n - /b\n - /b\n---\n\nbody", + "b.md": "---\ntype: T\ntitle: B\n---\n\nbody", + }) + + g := Build(b) + if len(g.Edges) != 1 { + t.Fatalf("got %d edges, want 1 (duplicate frontmatter links deduped)", len(g.Edges)) + } + if len(g.Backlinks["b"]) != 1 { + t.Errorf("backlinks[b] = %v, want a single entry", g.Backlinks["b"]) + } +} + +func TestBuild_NoSelfEdge(t *testing.T) { + // A concept linking to itself must not produce an a->a self-edge. + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\nlinks:\n - /a\n---\n\nAlso [self](a.md).", + }) + + g := Build(b) + if len(g.Edges) != 0 { + t.Fatalf("got %d edges, want 0 (no self-edge): %+v", len(g.Edges), g.Edges) + } + if len(g.Backlinks["a"]) != 0 { + t.Errorf("backlinks[a] = %v, want empty (no self-backlink)", g.Backlinks["a"]) + } +} + // --- helpers --- func testBundle(t *testing.T, files map[string]string) *bundle.Bundle { diff --git a/internal/validate/validate.go b/internal/validate/validate.go index 66acdf7..e6ac371 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -97,7 +97,14 @@ func validateBody(r *Report, c *concept.Concept) { // are checked. func validateLinks(r *Report, b *bundle.Bundle) { for _, c := range b.Concepts { - links := extractLinks(c.Body) + // Validate both body links and frontmatter links, mirroring how graph + // and backlinks consume them, so a frontmatter link to a nonexistent + // concept is reported instead of being silently dropped. + bodyLinks := extractLinks(c.Body) + fmLinks := ExtractFrontmatterLinks(c.Frontmatter.Links) + links := make([]Link, 0, len(bodyLinks)+len(fmLinks)) + links = append(links, bodyLinks...) + links = append(links, fmLinks...) for _, link := range links { target := resolveLink(c.ID, link) if target == "" { diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index df76d25..6d1c0af 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -1,7 +1,12 @@ package validate import ( + "os" + "path/filepath" + "strings" "testing" + + "github.com/okfcli/okf/internal/bundle" ) func TestExtractLinks(t *testing.T) { @@ -92,6 +97,56 @@ func TestExtractFrontmatterLinks_Empty(t *testing.T) { } } +func TestValidate_FrontmatterLinkToNonexistentConcept(t *testing.T) { + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\ndescription: d\ntags: [x]\nlinks:\n - /does-not-exist\n---\n\nbody", + }) + + r := Validate(b) + found := false + for _, f := range r.Findings { + if f.Severity == SeverityError && strings.Contains(f.Message, "does-not-exist") { + found = true + } + } + if !found { + t.Fatalf("expected a broken-link error for the frontmatter link, findings = %+v", r.Findings) + } +} + +func TestValidate_FrontmatterLinkResolves(t *testing.T) { + b := testBundle(t, map[string]string{ + "a.md": "---\ntype: T\ntitle: A\ndescription: d\ntags: [x]\nlinks:\n - /b\n---\n\nbody", + "b.md": "---\ntype: T\ntitle: B\ndescription: d\ntags: [x]\n---\n\nbody", + }) + + r := Validate(b) + for _, f := range r.Findings { + if f.Severity == SeverityError { + t.Fatalf("unexpected error finding for a resolvable frontmatter link: %+v", f) + } + } +} + +func testBundle(t *testing.T, files map[string]string) *bundle.Bundle { + t.Helper() + dir := t.TempDir() + for path, content := range files { + full := filepath.Join(dir, path) + if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(full, []byte(content), 0644); err != nil { + t.Fatalf("write: %v", err) + } + } + b, err := bundle.Load(dir) + if err != nil { + t.Fatalf("Load: %v", err) + } + return b +} + func TestNormalizePath(t *testing.T) { tests := []struct { in, want string