diff --git a/docs/internal/validation.md b/docs/internal/validation.md index b1fc038..f4766e7 100644 --- a/docs/internal/validation.md +++ b/docs/internal/validation.md @@ -36,7 +36,10 @@ It follows committed symlink declarations through the captured tree and fails with an inspection cause when their selected contents are unavailable. A replacement with no parsed version is reported with its go.mod path, line, directive, and target; versioned replacements are allowed. It runs after the -workspace-file check using the same candidate inventory. +workspace-file check using the same candidate inventory. Resolved module bodies +are parsed once per blob object ID; only owned filesystem-replacement targets +and line numbers remain cached. Each declaration path receives findings from +that summary in its own path context. The optional file-size check uses each blob's captured Git size, including symlink link-text bytes, and skips entries without a blob size such as gitlinks. It applies normalized literal file/directory exclusions with path-boundary @@ -63,13 +66,16 @@ autolinks, images, titles, and their source use sites, and to build heading anchors from visible AST text. It supports ATX and Setext headings, decodes Markdown text through the parser, and assigns collision-free numeric suffixes to repeated anchors. Link locations use one-based line and byte-column values. -A per-path cache retains parsed records and anchors for candidate documents. +The Markdown cache shares parsed records and anchors by regular blob object ID, +with a path lookup for callers. Retained destinations and titles own their +string storage, and parsed records contain content-relative locations only. `checkMarkdownLinks` examines regular `.md` blobs from the captured tree. It skips schemed and protocol-relative URIs, strips query components, and decodes percent escapes once in local paths and fragments. It resolves destinations -through `CandidateTree.ResolveDestination`; decoded fragments are compared in -lowercase against anchors from the resolved regular Markdown document. Missing +through `CandidateTree.ResolveDestination` relative to the current source path; +decoded fragments are compared in lowercase against anchors from the resolved +regular Markdown document. Missing paths, symlink cycles, and missing anchors produce `markdown-links` findings at the source use site. Outside-checkout links produce out-of-scope observations by default and findings when boundary enforcement is selected. Submodule-content diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 40dddc1..c8a0506 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Computational Efficiency Implementation Plan -**Status:** Stages 1–6 complete; Stages 7–11 pending. +**Status:** Stages 1–7 complete; Stages 8–11 pending. ## Purpose And Baseline @@ -407,6 +407,21 @@ correct paths/lines. Reuse parser and boundary/cycle coverage. Re-run large-document and repeated-content benchmarks and retained-memory observations. Update the validation document's cache description to match implemented ownership. +**Completion:** Complete. Parsed Markdown documents are shared by regular blob +object ID with path lookups; destinations and titles own their string storage, +and link locations remain content-relative while resolution uses each current +source path. Module declarations resolve to a regular committed entry before +reading; successfully parsed filesystem replacements are cached as owned +target/line summaries by body object ID and applied to every declaration path. +Validation, candidate, and facade tests passed, including shared Markdown +resolved from distinct directories and two module symlinks receiving the +correct findings at both paths and lines. Three large-document runs measured +10.85–11.22 ms and 2.29 MB/op for two identical 1 MiB documents and 1,000 +repeated links. Three retained-memory observations measured 110,568–120,280 +bytes with the candidate and parsed Markdown cache alive. These fixture results +include the retained per-document links, anchors, and candidate metadata; they +are workload-specific observations on the recorded shared machine. + ## Stage 8: Consolidate Delivery Preflight Hashing **Owners:** [delivery declarations](../../internal/delivery/assets.go), diff --git a/internal/validation/local_replacements.go b/internal/validation/local_replacements.go index 4fafd19..38842fc 100644 --- a/internal/validation/local_replacements.go +++ b/internal/validation/local_replacements.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path" + "strings" "gitea.maximumdirect.net/eric/ReleaseKit/internal/git" "golang.org/x/mod/modfile" @@ -11,12 +12,16 @@ import ( func checkLocalReplacements(ctx context.Context, tree *CandidateTree) error { var findings []Finding + replacementsByObjectID := make(map[string][]moduleReplacementSummary) for _, entry := range tree.Entries() { + if err := ctx.Err(); err != nil { + return &Failure{Check: LocalReplacements, Findings: findings, Cause: err} + } if path.Base(entry.Path) != "go.mod" { continue } - contents, err := readModuleDeclaration(ctx, tree, entry) + declaration, err := resolveModuleDeclaration(ctx, tree, entry) if err != nil { return &Failure{ Check: LocalReplacements, @@ -24,26 +29,49 @@ func checkLocalReplacements(ctx context.Context, tree *CandidateTree) error { Cause: fmt.Errorf("inspect committed %s: %w", entry.Path, err), } } - moduleFile, err := modfile.Parse(entry.Path, contents, nil) - if err != nil { - return &Failure{ - Check: LocalReplacements, - Findings: []Finding{{Path: entry.Path, Message: "could not parse committed module declaration"}}, - Cause: fmt.Errorf("parse committed %s: %w", entry.Path, err), + replacements, ok := replacementsByObjectID[declaration.ObjectID] + if !ok { + contents, err := tree.ReadFile(ctx, declaration.Path) + if err != nil { + return &Failure{ + Check: LocalReplacements, + Findings: []Finding{{Path: entry.Path, Message: "could not inspect committed module declaration"}}, + Cause: fmt.Errorf("inspect committed %s: read module declaration: %w", entry.Path, err), + } } + moduleFile, err := modfile.Parse(entry.Path, contents, nil) + if err != nil { + return &Failure{ + Check: LocalReplacements, + Findings: []Finding{{Path: entry.Path, Message: "could not parse committed module declaration"}}, + Cause: fmt.Errorf("parse committed %s: %w", entry.Path, err), + } + } + replacements = make([]moduleReplacementSummary, 0, len(moduleFile.Replace)) + for _, replacement := range moduleFile.Replace { + if replacement.New.Version != "" { + continue + } + line := 0 + if replacement.Syntax != nil { + line = replacement.Syntax.Start.Line + } + replacements = append(replacements, moduleReplacementSummary{ + target: strings.Clone(replacement.New.Path), + line: line, + }) + } + contents = nil + moduleFile = nil + replacementsByObjectID[declaration.ObjectID] = replacements } - for _, replacement := range moduleFile.Replace { - if replacement.New.Version != "" { - continue - } + for _, replacement := range replacements { finding := Finding{ Path: entry.Path, Message: "filesystem module replacement is committed", - Target: replacement.New.Path, + Target: replacement.target, Directive: "replace", - } - if replacement.Syntax != nil { - finding.Line = replacement.Syntax.Start.Line + Line: replacement.line, } findings = append(findings, finding) } @@ -54,39 +82,36 @@ func checkLocalReplacements(ctx context.Context, tree *CandidateTree) error { return nil } -func readModuleDeclaration(ctx context.Context, tree *CandidateTree, entry git.TreeEntry) ([]byte, error) { +type moduleReplacementSummary struct { + target string + line int +} + +func resolveModuleDeclaration(ctx context.Context, tree *CandidateTree, entry git.TreeEntry) (git.TreeEntry, error) { if entry.Type != "blob" { - return nil, fmt.Errorf("entry has type %q, not a blob", entry.Type) + return git.TreeEntry{}, fmt.Errorf("entry has type %q, not a blob", entry.Type) } if entry.Mode != "120000" { if entry.Mode != "100644" && entry.Mode != "100755" { - return nil, fmt.Errorf("entry has unsupported mode %q", entry.Mode) + return git.TreeEntry{}, fmt.Errorf("entry has unsupported mode %q", entry.Mode) } - contents, err := tree.ReadFile(ctx, entry.Path) - if err != nil { - return nil, fmt.Errorf("read module declaration: %w", err) - } - return contents, nil + return entry, nil } target, err := tree.readSymlinkTarget(ctx, entry) if err != nil { - return nil, fmt.Errorf("read symlink declaration: %w", err) + return git.TreeEntry{}, fmt.Errorf("read symlink declaration: %w", err) } destination, err := tree.ResolveDestination(ctx, entry.Path, target) if err != nil { - return nil, fmt.Errorf("resolve symlink declaration target %q: %w", target, err) + return git.TreeEntry{}, fmt.Errorf("resolve symlink declaration target %q: %w", target, err) } if destination.Kind != DestinationEntry { - return nil, fmt.Errorf("symlink declaration target %q resolves to %s", target, destination.Kind) + return git.TreeEntry{}, fmt.Errorf("symlink declaration target %q resolves to %s", target, destination.Kind) } resolved, ok := tree.Entry(destination.Path) if !ok || resolved.Type != "blob" || (resolved.Mode != "100644" && resolved.Mode != "100755") { - return nil, fmt.Errorf("symlink declaration target %q does not resolve to a regular committed file", target) + return git.TreeEntry{}, fmt.Errorf("symlink declaration target %q does not resolve to a regular committed file", target) } - contents, err := tree.ReadFile(ctx, destination.Path) - if err != nil { - return nil, fmt.Errorf("read resolved module declaration %q: %w", destination.Path, err) - } - return contents, nil + return resolved, nil } diff --git a/internal/validation/local_replacements_test.go b/internal/validation/local_replacements_test.go index c609aa2..986617e 100644 --- a/internal/validation/local_replacements_test.go +++ b/internal/validation/local_replacements_test.go @@ -74,6 +74,57 @@ func TestCheckLocalReplacementsFollowsCommittedModuleSymlinks(t *testing.T) { } } +func TestCheckLocalReplacementsSharesSummariesAcrossModuleSymlinks(t *testing.T) { + repository := newTreeRepository(t) + writeTreeFile(t, repository, "shared/module.mod", []byte( + "module shared.example/module\n"+ + "go 1.25.0\n"+ + "replace example.org/one => ../one\n"+ + "replace example.org/two => ../two\n", + )) + for _, directory := range []string{"first", "second"} { + if err := os.MkdirAll(filepath.Join(repository, directory), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../shared/module.mod", filepath.Join(repository, directory, "go.mod")); err != nil { + t.Fatal(err) + } + } + commit := commitCandidateTree(t, repository) + tree, err := ReadCandidateTree(context.Background(), repository, commit) + if err != nil { + t.Fatal(err) + } + cleanupCandidateTree(t, tree) + + err = checkLocalReplacements(context.Background(), tree) + var failure *Failure + if !errors.As(err, &failure) || failure.Check != LocalReplacements || len(failure.Findings) != 4 { + t.Fatalf("checkLocalReplacements() error = %v, want four reused findings", err) + } + want := []struct { + path string + line int + target string + }{ + {"first/go.mod", 3, "../one"}, + {"first/go.mod", 4, "../two"}, + {"second/go.mod", 3, "../one"}, + {"second/go.mod", 4, "../two"}, + } + for index, finding := range failure.Findings { + if finding.Path != want[index].path || finding.Line != want[index].line || finding.Target != want[index].target { + t.Errorf("finding[%d] = %#v, want %#v", index, finding, want[index]) + } + } + first, _ := tree.Entry("first/go.mod") + second, _ := tree.Entry("second/go.mod") + shared, _ := tree.Entry("shared/module.mod") + if first.ObjectID != second.ObjectID || first.ObjectID == shared.ObjectID { + t.Fatalf("declaration object IDs = %q, %q; module body = %q", first.ObjectID, second.ObjectID, shared.ObjectID) + } +} + func TestCheckLocalReplacementsReportsUnavailableSymlinkContents(t *testing.T) { repository := newTreeRepository(t) if err := os.Symlink("missing.mod", filepath.Join(repository, "go.mod")); err != nil { diff --git a/internal/validation/markdown.go b/internal/validation/markdown.go index 48f95d3..4e927f7 100644 --- a/internal/validation/markdown.go +++ b/internal/validation/markdown.go @@ -2,6 +2,7 @@ package validation import ( "context" + "fmt" "sort" "strconv" "strings" @@ -30,19 +31,29 @@ type markdownDocument struct { } type markdownCache struct { - tree *CandidateTree - documents map[string]*markdownDocument + tree *CandidateTree + byPath map[string]*markdownDocument + byObjectID map[string]*markdownDocument } func newMarkdownCache(tree *CandidateTree) *markdownCache { return &markdownCache{ - tree: tree, - documents: make(map[string]*markdownDocument), + tree: tree, + byPath: make(map[string]*markdownDocument), + byObjectID: make(map[string]*markdownDocument), } } func (cache *markdownCache) document(ctx context.Context, path string) (*markdownDocument, error) { - if document, ok := cache.documents[path]; ok { + if document, ok := cache.byPath[path]; ok { + return document, nil + } + entry, ok := cache.tree.Entry(path) + if !ok || !isRegularFile(entry) { + return nil, fmt.Errorf("committed path %q is not a regular file", path) + } + if document, ok := cache.byObjectID[entry.ObjectID]; ok { + cache.byPath[path] = document return document, nil } source, err := cache.tree.ReadFile(ctx, path) @@ -50,7 +61,9 @@ func (cache *markdownCache) document(ctx context.Context, path string) (*markdow return nil, err } document := parseMarkdown(source) - cache.documents[path] = document + source = nil + cache.byObjectID[entry.ObjectID] = document + cache.byPath[path] = document return document, nil } @@ -69,15 +82,15 @@ func parseMarkdown(source []byte) *markdownDocument { switch node := node.(type) { case *ast.Link: document.links = append(document.links, markdownLink{ - Destination: node.Destination.Value(source), - Title: node.Title.Value(source), + Destination: strings.Clone(string(node.Destination.Value(source))), + Title: strings.Clone(string(node.Title.Value(source))), Reference: node.Reference != nil, Location: markdownLocationAt(lineStarts, node.Pos()), }) case *ast.Image: document.links = append(document.links, markdownLink{ - Destination: node.Destination.Value(source), - Title: node.Title.Value(source), + Destination: strings.Clone(string(node.Destination.Value(source))), + Title: strings.Clone(string(node.Title.Value(source))), Image: true, Reference: node.Reference != nil, Location: markdownLocationAt(lineStarts, node.Pos()), @@ -86,7 +99,7 @@ func parseMarkdown(source []byte) *markdownDocument { return ast.WalkSkipChildren, nil case *ast.AutoLink: document.links = append(document.links, markdownLink{ - Destination: node.Destination.Value(source), + Destination: strings.Clone(string(node.Destination.Value(source))), Location: markdownLocationAt(lineStarts, node.Pos()), }) case *ast.Heading: diff --git a/internal/validation/markdown_check_test.go b/internal/validation/markdown_check_test.go index d04d592..8c80f5c 100644 --- a/internal/validation/markdown_check_test.go +++ b/internal/validation/markdown_check_test.go @@ -150,6 +150,24 @@ func TestCheckMarkdownLinksUsesResolvedMarkdownIdentity(t *testing.T) { } } +func TestCheckMarkdownLinksUsesCurrentPathForSharedDocuments(t *testing.T) { + shared := []byte("[target](target.md)\n") + _, tree := committedMarkdownTree(t, map[string][]byte{ + "one/guide.md": shared, + "one/target.md": []byte("# Target\n"), + "two/guide.md": shared, + }) + + err := checkMarkdownLinks(context.Background(), tree, false, nil) + var failure *Failure + if !errors.As(err, &failure) || failure.Check != MarkdownLinks || len(failure.Findings) != 1 { + t.Fatalf("checkMarkdownLinks() error = %v, want one path-specific finding", err) + } + if finding := failure.Findings[0]; finding.Path != "two/guide.md" || finding.Target != "target.md" { + t.Fatalf("shared-document finding = %#v, want missing target relative to two/guide.md", finding) + } +} + func TestCheckMarkdownLinksReportsBoundaryAndSubmoduleObservations(t *testing.T) { repository, tree := committedMarkdownTree(t, map[string][]byte{ "guide.md": []byte("[outside](../../outside.md)\n[submodule](deps/module/README.md)\n"), diff --git a/internal/validation/markdown_test.go b/internal/validation/markdown_test.go index 4ae7d83..71ddb04 100644 --- a/internal/validation/markdown_test.go +++ b/internal/validation/markdown_test.go @@ -87,11 +87,13 @@ func TestParseMarkdownHeadingAnchors(t *testing.T) { } } -func TestMarkdownCacheKeepsParsedDocumentsPerCandidatePath(t *testing.T) { +func TestMarkdownCacheSharesParsedDocumentsByBlobObjectID(t *testing.T) { repository := newTreeRepository(t) - writeTreeFile(t, repository, "guide.md", []byte("# Guide\n[other](other.md)\n")) - writeTreeFile(t, repository, "other.md", []byte("# Other\n")) - gitTree(t, repository, "add", "guide.md", "other.md") + shared := []byte("# Guide\n[other](other.md \"title\")\n") + writeTreeFile(t, repository, "one/guide.md", shared) + writeTreeFile(t, repository, "two/guide.md", shared) + writeTreeFile(t, repository, "one/other.md", []byte("# Other\n")) + gitTree(t, repository, "add", "one/guide.md", "two/guide.md", "one/other.md") gitTree(t, repository, "commit", "-m", "add markdown files") commit := strings.TrimSpace(gitTree(t, repository, "rev-parse", "HEAD")) @@ -101,25 +103,28 @@ func TestMarkdownCacheKeepsParsedDocumentsPerCandidatePath(t *testing.T) { } cleanupCandidateTree(t, tree) cache := newMarkdownCache(tree) - first, err := cache.document(context.Background(), "guide.md") + first, err := cache.document(context.Background(), "one/guide.md") if err != nil { t.Fatal(err) } - second, err := cache.document(context.Background(), "guide.md") + second, err := cache.document(context.Background(), "two/guide.md") if err != nil { t.Fatal(err) } if first != second { - t.Fatal("repeated document lookup returned a different parsed document") + t.Fatal("identical Markdown blobs returned different parsed documents") } - other, err := cache.document(context.Background(), "other.md") + other, err := cache.document(context.Background(), "one/other.md") if err != nil { t.Fatal(err) } if other == first { - t.Fatal("different candidate paths shared a parsed document") + t.Fatal("different Markdown blobs shared a parsed document") } if _, ok := other.anchors["other"]; !ok { t.Fatalf("other document anchors = %#v", other.anchors) } + if got := first.links[0]; got.Destination != "other.md" || got.Title != "title" || got.Location.Line != 2 { + t.Fatalf("shared Markdown link = %#v, want content-relative destination, title, and location", got) + } }