From 17693832f18ee6bd336995c57c15bd8d8c4d8128 Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Wed, 29 Jul 2026 23:17:19 -0500 Subject: [PATCH 1/7] feat(tapper): select node metadata and stats in listing formats Listing formats could only show five fields, all from the node index entry, so metadata that is already queryable -- type, status, and the rest -- could not be displayed at all. Single letters cannot address an arbitrary metadata key, so extend the format language rather than adding verbs. Field selectors now reuse the query expression vocabulary. In field position a bare word names a metadata key, a leading dot names a statistics field, and tags is reserved for the tag list; id and title are intrinsics. Formats frame selectors as %{...}, and the legacy verbs %i %d %c %a %t remain as aliases. Replace the chain of whole-line strings.Replace calls with a single left-to-right scanner. The old chain re-expanded values that happened to contain a verb, which adding selectors would have made far worse. Three fixes fall out of it: - %% renders a literal percent. It was documented in eight places and implemented in none. - Zero timestamps render empty instead of 0001-01-01T00:00:00Z, which parses as a real date and silently corrupts downstream sorting. - Control characters in expanded values collapse to spaces. Each rendered string is one output line, and .lead routinely contains newlines. The three index timestamps resolve from the index entry rather than stats.json, mirroring resolveStatsCompare, so a displayed value agrees with the same predicate in a query and the default format stays free of per-node reads. Only formats naming metadata or a non-timestamp stat perform I/O, and that pass is wrapped in one keg read boundary: the boundary is exclusive and re-entrant through the context, so without it a listing would acquire and release it twice per node. --- pkg/keg/format_boundary.go | 21 +++ pkg/keg/format_fields.go | 217 ++++++++++++++++++++++++ pkg/keg/format_fields_test.go | 243 +++++++++++++++++++++++++++ pkg/tapper/tap_list.go | 257 +++++++++++++++++++++-------- pkg/tapper/tap_list_format.go | 220 ++++++++++++++++++++++++ pkg/tapper/tap_list_format_test.go | 243 +++++++++++++++++++++++++++ 6 files changed, 1129 insertions(+), 72 deletions(-) create mode 100644 pkg/keg/format_boundary.go create mode 100644 pkg/keg/format_fields.go create mode 100644 pkg/keg/format_fields_test.go create mode 100644 pkg/tapper/tap_list_format.go create mode 100644 pkg/tapper/tap_list_format_test.go diff --git a/pkg/keg/format_boundary.go b/pkg/keg/format_boundary.go new file mode 100644 index 0000000..c7386a2 --- /dev/null +++ b/pkg/keg/format_boundary.go @@ -0,0 +1,21 @@ +package keg + +import "context" + +// WithReadBoundary runs fn inside a single keg read boundary when k has one. +// +// A local keg's read boundary is an exclusive lock, and every per-node read +// takes it. Batches of reads must therefore share one boundary rather than +// acquiring it per call: the boundary is re-entrant through the context, so +// nested reads inside fn short-circuit instead of relocking. Without this, a +// listing that reads metadata for N nodes performs 2N exclusive lock cycles and +// blocks every other process on the keg for the duration. +// +// A remote keg has no local boundary to hold, so fn runs directly. +func WithReadBoundary(ctx context.Context, k Keg, fn func(context.Context) error) error { + local, ok := k.(*LocalKeg) + if !ok || local == nil || local.Repo == nil { + return fn(ctx) + } + return local.Repo.WithKegRead(ctx, fn) +} diff --git a/pkg/keg/format_fields.go b/pkg/keg/format_fields.go new file mode 100644 index 0000000..f1e9f3c --- /dev/null +++ b/pkg/keg/format_fields.go @@ -0,0 +1,217 @@ +package keg + +import ( + "fmt" + "slices" + "strconv" + "strings" + "time" +) + +// Listing formats and query expressions name node fields with one vocabulary. +// A selector appears in two syntactically distinct positions, and a bare word +// means something different in each: +// +// - Predicate position (query expressions): a bare word is a TAG, because a +// metadata key is always written as key=value there. +// - Field position (listing formats): a bare word is a METADATA KEY, because +// there is no value to compare against and a tag is not a field. +// +// Statistics fields carry a leading dot in both positions, so the one selector +// that appears in both means the same thing in both. + +// FieldKind classifies a listing field selector by where its value comes from. +// The kind determines whether rendering a selector costs any I/O: intrinsics +// and index timestamps are served from the node index entry already in hand, +// while metadata and the remaining statistics fields require a per-node read. +type FieldKind int + +const ( + // FieldUnknown is the zero value and names no field. + FieldUnknown FieldKind = iota + // FieldID is the intrinsic node id, from NodeIndexEntry. + FieldID + // FieldTitle is the intrinsic node title, from NodeIndexEntry. + FieldTitle + // FieldIndexTime is a timestamp served from NodeIndexEntry without I/O. + FieldIndexTime + // FieldStat is a statistics field requiring a NodeStats read. + FieldStat + // FieldTags is the reserved tag-list selector, requiring a NodeMeta read. + FieldTags + // FieldMetaKey is an arbitrary metadata key, requiring a NodeMeta read. + FieldMetaKey +) + +// FieldSelector is one parsed listing field selector. Text is the selector as +// written; Key is the bare lookup name with any leading dot stripped. +type FieldSelector struct { + Text string + Kind FieldKind + Key string +} + +// NeedsMeta reports whether rendering this selector requires the node's +// metadata, which costs one read per node. +func (f FieldSelector) NeedsMeta() bool { + return f.Kind == FieldTags || f.Kind == FieldMetaKey +} + +// NeedsStats reports whether rendering this selector requires the node's +// statistics, which costs one read per node. Index timestamps do not, even +// though they are spelled like statistics fields. +func (f FieldSelector) NeedsStats() bool { + return f.Kind == FieldStat +} + +// IndexTimeFieldNames lists the statistics fields that resolve from the node +// index rather than from stats.json. These mirror resolveStatsCompare's +// no-I/O branch so a displayed value always agrees with the same predicate in +// a query expression. +var IndexTimeFieldNames = []string{"updated", "created", "accessed"} + +// ReservedFieldNames lists the bare words that do not name a metadata key. +// A node carrying metadata under one of these keys cannot address it in +// field position; the intrinsic wins. +var ReservedFieldNames = []string{"id", "title", "tags"} + +// LegacyFormatVerbs maps the historical single-letter format verbs onto +// selector text. These remain supported as aliases; no new letters are added, +// because a single letter cannot address an arbitrary metadata key. +var LegacyFormatVerbs = map[byte]string{ + 'i': "id", + 't': "title", + 'd': ".updated", + 'c': ".created", + 'a': ".accessed", +} + +// FormatVocabularyDescription is the one-line summary of the listing field +// vocabulary. It is duplicated as a literal in the MCP tool schemas, which +// require a constant struct tag; a test holds the two in agreement. +const FormatVocabularyDescription = "output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node." + +func isIndexTimeField(name string) bool { + return slices.Contains(IndexTimeFieldNames, name) +} + +func isStatsField(name string) bool { + return slices.Contains(StatsFieldNames, name) +} + +// ParseFieldSelector classifies raw as a listing field selector. Surrounding +// ASCII spaces are ignored so "%{ type }" behaves like "%{type}"; interior +// spaces are preserved because a YAML key may contain them. +// +// A bare word is always accepted, because metadata keys are open-ended and +// cannot be validated against a fixed list. A dotted name is rejected unless +// it is a known statistics field, since that vocabulary is closed. +func ParseFieldSelector(raw string) (FieldSelector, error) { + text := strings.Trim(raw, " \t") + if text == "" { + return FieldSelector{}, fmt.Errorf("empty selector") + } + // Predicate syntax in field position is the likely result of pasting a + // query expression, so name that specifically rather than failing as a + // generic bad character. + if strings.ContainsAny(text, "=<>!") { + return FieldSelector{}, fmt.Errorf("selector %q is a field name, not a predicate", text) + } + if strings.ContainsAny(text, "%{}") { + return FieldSelector{}, fmt.Errorf("invalid selector %q", text) + } + for i := 0; i < len(text); i++ { + if text[i] < 0x20 || text[i] == 0x7f { + return FieldSelector{}, fmt.Errorf("invalid selector %q", text) + } + } + + if strings.HasPrefix(text, ".") { + name := text[1:] + if !isStatsField(name) { + return FieldSelector{}, fmt.Errorf( + "unknown stats field %q (valid: %s)", text, strings.Join(StatsFieldNames, ", ")) + } + kind := FieldStat + if isIndexTimeField(name) { + kind = FieldIndexTime + } + return FieldSelector{Text: text, Kind: kind, Key: name}, nil + } + + switch text { + case "id": + return FieldSelector{Text: text, Kind: FieldID, Key: text}, nil + case "title": + return FieldSelector{Text: text, Kind: FieldTitle, Key: text}, nil + case "tags": + return FieldSelector{Text: text, Kind: FieldTags, Key: text}, nil + } + return FieldSelector{Text: text, Kind: FieldMetaKey, Key: text}, nil +} + +// StatsFieldValue renders the named statistics field for display. known +// reports whether name is a recognized statistics field; an empty value with +// known true means the field is absent or unset on this node. +// +// Absent values render empty rather than as a placeholder so a tabular format +// keeps a stable column count. accessCount is the one exception: it always +// renders its integer, including zero, because stats.json omits the key when +// it is zero and so absent and zero are indistinguishable on disk. +func StatsFieldValue(s *NodeStats, name string) (string, bool) { + if !isStatsField(name) { + return "", false + } + if s == nil { + if name == "accessCount" { + return "0", true + } + return "", true + } + + switch name { + case "updated": + return formatStatsTime(s.Updated()), true + case "created": + return formatStatsTime(s.Created()), true + case "accessed": + return formatStatsTime(s.Accessed()), true + case "hash": + return s.Hash(), true + case "lead": + return s.Lead(), true + case "accessCount": + return strconv.Itoa(s.AccessCount()), true + case "omega": + omega, ok := s.Omega() + if !ok { + return "", true + } + return strconv.FormatFloat(omega, 'f', -1, 64), true + } + return "", false +} + +// formatStatsTime renders a timestamp for display. A zero time renders empty +// rather than 0001-01-01T00:00:00Z, which parses as a real date and would +// silently corrupt downstream sorting and filtering. +func formatStatsTime(t time.Time) string { + if t.IsZero() { + return "" + } + return t.Format(time.RFC3339) +} + +// FormatSelectorSuggestions returns the closed part of the field vocabulary as +// ready-to-type format tokens, for shell completion. Metadata keys are +// open-ended and therefore absent. +func FormatSelectorSuggestions() []string { + out := make([]string, 0, len(ReservedFieldNames)+len(StatsFieldNames)) + for _, name := range ReservedFieldNames { + out = append(out, "%{"+name+"}") + } + for _, name := range StatsFieldNames { + out = append(out, "%{."+name+"}") + } + return out +} diff --git a/pkg/keg/format_fields_test.go b/pkg/keg/format_fields_test.go new file mode 100644 index 0000000..d0e3487 --- /dev/null +++ b/pkg/keg/format_fields_test.go @@ -0,0 +1,243 @@ +package keg_test + +import ( + "testing" + "time" + + "github.com/jlrickert/tapper/pkg/keg" +) + +func TestParseFieldSelector(t *testing.T) { + tests := []struct { + name string + raw string + wantKind keg.FieldKind + wantKey string + wantErr bool + }{ + {name: "id intrinsic", raw: "id", wantKind: keg.FieldID, wantKey: "id"}, + {name: "title intrinsic", raw: "title", wantKind: keg.FieldTitle, wantKey: "title"}, + {name: "tags reserved", raw: "tags", wantKind: keg.FieldTags, wantKey: "tags"}, + {name: "bare word is a meta key", raw: "type", wantKind: keg.FieldMetaKey, wantKey: "type"}, + {name: "meta key with spaces inside", raw: "my key", wantKind: keg.FieldMetaKey, wantKey: "my key"}, + {name: "surrounding space trimmed", raw: " status ", wantKind: keg.FieldMetaKey, wantKey: "status"}, + + // The three index timestamps must classify as FieldIndexTime, not + // FieldStat, or the default format would start reading stats.json. + {name: "updated is index time", raw: ".updated", wantKind: keg.FieldIndexTime, wantKey: "updated"}, + {name: "created is index time", raw: ".created", wantKind: keg.FieldIndexTime, wantKey: "created"}, + {name: "accessed is index time", raw: ".accessed", wantKind: keg.FieldIndexTime, wantKey: "accessed"}, + + {name: "hash is a stat", raw: ".hash", wantKind: keg.FieldStat, wantKey: "hash"}, + {name: "lead is a stat", raw: ".lead", wantKind: keg.FieldStat, wantKey: "lead"}, + {name: "accessCount is a stat", raw: ".accessCount", wantKind: keg.FieldStat, wantKey: "accessCount"}, + {name: "omega is a stat", raw: ".omega", wantKind: keg.FieldStat, wantKey: "omega"}, + + {name: "empty", raw: "", wantErr: true}, + {name: "only spaces", raw: " ", wantErr: true}, + {name: "unknown stats field", raw: ".bogus", wantErr: true}, + {name: "on-disk spelling is not a selector", raw: ".access_count", wantErr: true}, + {name: "equality predicate", raw: "type=plan", wantErr: true}, + {name: "inequality predicate", raw: "omega>=0.5", wantErr: true}, + {name: "brace in selector", raw: "ty{pe", wantErr: true}, + {name: "percent in selector", raw: "ty%pe", wantErr: true}, + {name: "control character", raw: "ty\npe", wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := keg.ParseFieldSelector(tc.raw) + if tc.wantErr { + if err == nil { + t.Fatalf("ParseFieldSelector(%q) = %+v, want error", tc.raw, got) + } + return + } + if err != nil { + t.Fatalf("ParseFieldSelector(%q): %v", tc.raw, err) + } + if got.Kind != tc.wantKind { + t.Errorf("Kind = %v, want %v", got.Kind, tc.wantKind) + } + if got.Key != tc.wantKey { + t.Errorf("Key = %q, want %q", got.Key, tc.wantKey) + } + }) + } +} + +func TestFieldSelectorNeeds(t *testing.T) { + // Only metadata and non-timestamp stats cost a read. Getting this wrong + // makes the default format perform per-node I/O. + tests := []struct { + raw string + wantMeta bool + wantStats bool + }{ + {raw: "id"}, + {raw: "title"}, + {raw: ".updated"}, + {raw: ".created"}, + {raw: ".accessed"}, + {raw: "tags", wantMeta: true}, + {raw: "type", wantMeta: true}, + {raw: ".hash", wantStats: true}, + {raw: ".omega", wantStats: true}, + {raw: ".accessCount", wantStats: true}, + {raw: ".lead", wantStats: true}, + } + for _, tc := range tests { + t.Run(tc.raw, func(t *testing.T) { + sel, err := keg.ParseFieldSelector(tc.raw) + if err != nil { + t.Fatalf("ParseFieldSelector(%q): %v", tc.raw, err) + } + if got := sel.NeedsMeta(); got != tc.wantMeta { + t.Errorf("NeedsMeta() = %v, want %v", got, tc.wantMeta) + } + if got := sel.NeedsStats(); got != tc.wantStats { + t.Errorf("NeedsStats() = %v, want %v", got, tc.wantStats) + } + }) + } +} + +func TestStatsFieldValueUnknownField(t *testing.T) { + if _, known := keg.StatsFieldValue(nil, "nope"); known { + t.Error("StatsFieldValue(nil, \"nope\") reported known, want unknown") + } +} + +func TestStatsFieldValueNilStats(t *testing.T) { + // Absent stats render empty, except accessCount, which has no absent + // state on disk and so always renders its integer. + for _, name := range keg.StatsFieldNames { + value, known := keg.StatsFieldValue(nil, name) + if !known { + t.Errorf("%s: known = false, want true", name) + continue + } + want := "" + if name == "accessCount" { + want = "0" + } + if value != want { + t.Errorf("%s: value = %q, want %q", name, value, want) + } + } +} + +func TestStatsFieldValuePopulated(t *testing.T) { + now := time.Date(2026, 7, 29, 12, 0, 0, 0, time.UTC) + stats := keg.NewStats(now) + stats.SetHash("abc123", &now) + stats.SetLead("a lead line") + stats.SetAccessCount(7) + stats.SetOmega(0.75) + + tests := map[string]string{ + "hash": "abc123", + "lead": "a lead line", + "accessCount": "7", + "omega": "0.75", + } + for name, want := range tests { + got, known := keg.StatsFieldValue(stats, name) + if !known { + t.Errorf("%s: known = false, want true", name) + continue + } + if got != want { + t.Errorf("%s = %q, want %q", name, got, want) + } + } +} + +func TestStatsFieldValueOmegaAbsentIsNotZero(t *testing.T) { + // omega is genuinely tri-state, so absent must not render as "0". + now := time.Date(2026, 7, 29, 12, 0, 0, 0, time.UTC) + stats := keg.NewStats(now) + stats.ClearOmega() + + got, known := keg.StatsFieldValue(stats, "omega") + if !known { + t.Fatal("omega: known = false, want true") + } + if got != "" { + t.Errorf("absent omega = %q, want empty", got) + } + + stats.SetOmega(0) + got, _ = keg.StatsFieldValue(stats, "omega") + if got != "0" { + t.Errorf("zero omega = %q, want %q", got, "0") + } +} + +func TestStatsFieldValueZeroTimeRendersEmpty(t *testing.T) { + // A zero time formatted as RFC3339 is 0001-01-01T00:00:00Z, which parses + // as a real date and would silently corrupt downstream sorting. + stats := keg.NewStats(time.Time{}) + for _, name := range keg.IndexTimeFieldNames { + got, known := keg.StatsFieldValue(stats, name) + if !known { + t.Errorf("%s: known = false, want true", name) + continue + } + if got != "" { + t.Errorf("%s zero time = %q, want empty", name, got) + } + } +} + +func TestIndexTimeFieldsAreStatsFields(t *testing.T) { + // IndexTimeFieldNames is a subset of StatsFieldNames: the same selector + // spelling, served from a cheaper source. + for _, name := range keg.IndexTimeFieldNames { + sel, err := keg.ParseFieldSelector("." + name) + if err != nil { + t.Errorf(".%s is not a valid selector: %v", name, err) + continue + } + if sel.Kind != keg.FieldIndexTime { + t.Errorf(".%s Kind = %v, want FieldIndexTime", name, sel.Kind) + } + } +} + +func TestLegacyFormatVerbsResolve(t *testing.T) { + // Every legacy verb must name a selector that still parses, or the + // compatibility aliases silently break. + want := map[byte]keg.FieldKind{ + 'i': keg.FieldID, + 't': keg.FieldTitle, + 'd': keg.FieldIndexTime, + 'c': keg.FieldIndexTime, + 'a': keg.FieldIndexTime, + } + if len(keg.LegacyFormatVerbs) != len(want) { + t.Fatalf("LegacyFormatVerbs has %d entries, want %d", len(keg.LegacyFormatVerbs), len(want)) + } + for verb, text := range keg.LegacyFormatVerbs { + sel, err := keg.ParseFieldSelector(text) + if err != nil { + t.Errorf("%%%c -> %q: %v", verb, text, err) + continue + } + if sel.Kind != want[verb] { + t.Errorf("%%%c -> %q Kind = %v, want %v", verb, text, sel.Kind, want[verb]) + } + } +} + +func TestFormatSelectorSuggestions(t *testing.T) { + got := keg.FormatSelectorSuggestions() + if len(got) != len(keg.ReservedFieldNames)+len(keg.StatsFieldNames) { + t.Fatalf("suggestions = %d, want %d", len(got), len(keg.ReservedFieldNames)+len(keg.StatsFieldNames)) + } + for _, s := range got { + if len(s) < 4 || s[:2] != "%{" || s[len(s)-1] != '}' { + t.Errorf("suggestion %q is not a ready-to-type token", s) + } + } +} diff --git a/pkg/tapper/tap_list.go b/pkg/tapper/tap_list.go index 4443ca6..4baa6e8 100644 --- a/pkg/tapper/tap_list.go +++ b/pkg/tapper/tap_list.go @@ -30,11 +30,12 @@ type ListOptions struct { // ("entity=plan"). When empty, all nodes are listed. Query string - // Format to use. %i is node id, %d - // %i is node id - // %d is date - // %t is node title - // %% for literal % + // Format is the output template. Legacy verbs %i (id), %t (title), + // %d (updated), %c (created), %a (accessed) remain supported, and %% + // renders a literal percent. Named selectors use %{...}: a bare word + // names a metadata key (%{type}), a leading dot names a statistics field + // (%{.accessCount}), and %{tags} is the tag list. Selectors other than + // id, title, and the three dates cost one read per node. Format string IdOnly bool @@ -58,10 +59,12 @@ type BacklinksOptions struct { // Results from all node IDs are merged and deduplicated. NodeIDs []string - // Format to use. %i is node id - // %d is date - // %t is node title - // %% for literal % + // Format is the output template. Legacy verbs %i (id), %t (title), + // %d (updated), %c (created), %a (accessed) remain supported, and %% + // renders a literal percent. Named selectors use %{...}: a bare word + // names a metadata key (%{type}), a leading dot names a statistics field + // (%{.accessCount}), and %{tags} is the tag list. Selectors other than + // id, title, and the three dates cost one read per node. Format string IdOnly bool @@ -82,10 +85,12 @@ type LinksOptions struct { // Results from all node IDs are merged and deduplicated. NodeIDs []string - // Format to use. %i is node id - // %d is date - // %t is node title - // %% for literal % + // Format is the output template. Legacy verbs %i (id), %t (title), + // %d (updated), %c (created), %a (accessed) remain supported, and %% + // renders a literal percent. Named selectors use %{...}: a bare word + // names a metadata key (%{type}), a leading dot names a statistics field + // (%{.accessCount}), and %{tags} is the tag list. Selectors other than + // id, title, and the three dates cost one read per node. Format string IdOnly bool @@ -105,10 +110,12 @@ type GrepOptions struct { // Query is the regex pattern used to search nodes. Query string - // Format to use. %i is node id - // %d is date - // %t is node title - // %% for literal % + // Format is the output template. Legacy verbs %i (id), %t (title), + // %d (updated), %c (created), %a (accessed) remain supported, and %% + // renders a literal percent. Named selectors use %{...}: a bare word + // names a metadata key (%{type}), a leading dot names a statistics field + // (%{.accessCount}), and %{tags} is the tag list. Selectors other than + // id, title, and the three dates cost one read per node. Format string IdOnly bool @@ -138,10 +145,12 @@ type TagsOptions struct { // ("entity=plan"). When empty, all tags are listed. Query string - // Format to use. %i is node id - // %d is date - // %t is node title - // %% for literal % + // Format is the output template. Legacy verbs %i (id), %t (title), + // %d (updated), %c (created), %a (accessed) remain supported, and %% + // renders a literal percent. Named selectors use %{...}: a bare word + // names a metadata key (%{type}), a leading dot names a statistics field + // (%{.accessCount}), and %{tags} is the tag list. Selectors other than + // id, title, and the three dates cost one read per node. Format string IdOnly bool @@ -221,49 +230,73 @@ func (t *Tap) List(ctx context.Context, opts ListOptions) ([]string, error) { entries = entries[:opts.Limit] } - return renderNodeEntries(entries, opts.Format, opts.IdOnly, opts.Reverse), nil + return t.renderNodeEntries(ctx, k, entries, renderOptions{ + Format: opts.Format, IdOnly: opts.IdOnly, Reverse: opts.Reverse, + }) } func (t *Tap) Backlinks(ctx context.Context, opts BacklinksOptions) ([]string, error) { if opts.Offset < 0 { return []string{}, fmt.Errorf("offset must be >= 0, got %d", opts.Offset) } - return t.resolveAndLookupLinks(ctx, opts.KegTargetOptions, opts.NodeIDs, - opts.Format, opts.IdOnly, opts.Reverse, opts.Limit, opts.Offset, keg.RelatedBacklinks) + return t.resolveAndLookupLinks(ctx, relatedListOptions{ + KegTargetOptions: opts.KegTargetOptions, + NodeIDs: opts.NodeIDs, + Render: renderOptions{Format: opts.Format, IdOnly: opts.IdOnly, Reverse: opts.Reverse}, + Limit: opts.Limit, + Offset: opts.Offset, + Direction: keg.RelatedBacklinks, + }) } func (t *Tap) Links(ctx context.Context, opts LinksOptions) ([]string, error) { if opts.Offset < 0 { return []string{}, fmt.Errorf("offset must be >= 0, got %d", opts.Offset) } - return t.resolveAndLookupLinks(ctx, opts.KegTargetOptions, opts.NodeIDs, - opts.Format, opts.IdOnly, opts.Reverse, opts.Limit, opts.Offset, keg.RelatedLinks) + return t.resolveAndLookupLinks(ctx, relatedListOptions{ + KegTargetOptions: opts.KegTargetOptions, + NodeIDs: opts.NodeIDs, + Render: renderOptions{Format: opts.Format, IdOnly: opts.IdOnly, Reverse: opts.Reverse}, + Limit: opts.Limit, + Offset: opts.Offset, + Direction: keg.RelatedLinks, + }) +} + +// relatedListOptions is the shared input for Backlinks and Links. +type relatedListOptions struct { + KegTargetOptions + + // NodeIDs are the nodes whose related nodes are looked up. + NodeIDs []string + + // Render carries the presentation knobs. + Render renderOptions + + // Limit caps the number of results returned. 0 means no limit. + Limit int + + // Offset skips the first N results before applying limit. + Offset int + + // Direction selects incoming or outgoing links. + Direction keg.RelatedDirection } // resolveAndLookupLinks is a shared helper for Backlinks and Links. It resolves // the keg, validates the node IDs, calls the provided lookup function against the // dex for each node, merges and deduplicates results, and renders the entries. -func (t *Tap) resolveAndLookupLinks( - ctx context.Context, - kegOpts KegTargetOptions, - nodeIDs []string, - format string, - idOnly bool, - reverse bool, - limit int, - offset int, - direction keg.RelatedDirection, -) ([]string, error) { - if len(nodeIDs) == 0 { +func (t *Tap) resolveAndLookupLinks(ctx context.Context, opts relatedListOptions) ([]string, error) { + if len(opts.NodeIDs) == 0 { return []string{}, fmt.Errorf("at least one node ID is required") } - k, err := t.resolveKeg(ctx, kegOpts) + k, err := t.resolveKeg(ctx, opts.KegTargetOptions) if err != nil { return []string{}, fmt.Errorf("unable to open keg: %w", err) } - ids := make([]keg.NodeId, 0, len(nodeIDs)) - for _, nodeID := range nodeIDs { + ids := make([]keg.NodeId, 0, len(opts.NodeIDs)) + for _, nodeID := range opts.NodeIDs { // Intentionally NOT routed through resolveNodeArg: a cross-keg ref would // produce related nodes owned by a different keg, but the dedup key // (rel.Path()) and the entry rendering below (dex.GetRef) both assume a @@ -277,7 +310,7 @@ func (t *Tap) resolveAndLookupLinks( ids = append(ids, id) } - entries, err := k.RelatedNodes(ctx, keg.RelatedNodesOptions{NodeIDs: ids, Direction: direction}) + entries, err := k.RelatedNodes(ctx, keg.RelatedNodesOptions{NodeIDs: ids, Direction: opts.Direction}) if err != nil { if strings.Contains(err.Error(), "keg not initialized") { return []string{}, err @@ -288,13 +321,13 @@ func (t *Tap) resolveAndLookupLinks( return []string{}, err } - entries = applyOffset(entries, offset) + entries = applyOffset(entries, opts.Offset) - if limit > 0 && len(entries) > limit { - entries = entries[:limit] + if opts.Limit > 0 && len(entries) > opts.Limit { + entries = entries[:opts.Limit] } - return renderNodeEntries(entries, format, idOnly, reverse), nil + return t.renderNodeEntries(ctx, k, entries, opts.Render) } func (t *Tap) Grep(ctx context.Context, opts GrepOptions) ([]string, error) { @@ -331,7 +364,9 @@ func (t *Tap) Grep(ctx context.Context, opts GrepOptions) ([]string, error) { matchedEntries = append(matchedEntries, match.entry) } if opts.IdOnly || opts.Format != "" { - return renderNodeEntries(matchedEntries, opts.Format, opts.IdOnly, opts.Reverse), nil + return t.renderNodeEntries(ctx, k, matchedEntries, renderOptions{ + Format: opts.Format, IdOnly: opts.IdOnly, Reverse: opts.Reverse, + }) } return renderGrepMatches(matches, opts.Reverse), nil } @@ -389,7 +424,9 @@ func (t *Tap) Tags(ctx context.Context, opts TagsOptions) ([]string, error) { entries = entries[:opts.Limit] } - return renderNodeEntries(entries, opts.Format, opts.IdOnly, opts.Reverse), nil + return t.renderNodeEntries(ctx, k, entries, renderOptions{ + Format: opts.Format, IdOnly: opts.IdOnly, Reverse: opts.Reverse, + }) } func grepContentLineMatches(re *regexp.Regexp, raw []byte) []string { @@ -441,41 +478,117 @@ func renderGrepMatches(matches []grepMatch, reverse bool) []string { return lines } -func renderNodeEntries(entries []keg.NodeIndexEntry, format string, idOnly bool, reverse bool) []string { - lines := make([]string, 0) +// renderOptions carries the presentation knobs shared by every listing surface. +type renderOptions struct { + Format string + IdOnly bool + Reverse bool +} - start := 0 - end := len(entries) - step := 1 - if reverse { - start = len(entries) - 1 - end = -1 - step = -1 +// enrichWarnThreshold is the number of nodes above which a format requiring +// per-node reads warns once, so a slow listing explains itself. +const enrichWarnThreshold = 200 + +// renderNodeEntries renders one line per entry using the compiled format. +// +// Formats naming only intrinsics, index timestamps, or legacy verbs — which +// includes the default — perform no additional I/O. A format naming metadata +// or a statistics field costs one read per node for each, so the whole pass is +// wrapped in a single keg read boundary: the boundary is exclusive and +// context-re-entrant, so without this the pass would acquire and release it +// twice per node and block every other process on the keg. +func (t *Tap) renderNodeEntries( + ctx context.Context, + k keg.Keg, + entries []keg.NodeIndexEntry, + opts renderOptions, +) ([]string, error) { + if opts.IdOnly { + return renderNodeIDs(entries, opts.Reverse), nil } - for i := start; i != end; i += step { - entry := entries[i] - if idOnly { - lines = append(lines, entry.ID) - continue + compiled, err := compileListFormat(opts.Format) + if err != nil { + return nil, err + } + + lines := make([]string, 0, len(entries)) + enrich := compiled.needsMeta || compiled.needsStats + + render := func(ctx context.Context) error { + start, end, step := iterationBounds(len(entries), opts.Reverse) + for i := start; i != end; i += step { + if enrich { + if err := ctx.Err(); err != nil { + return err + } + } + src := nodeFieldSource{entry: entries[i]} + if enrich { + t.loadNodeFields(ctx, k, &src, compiled) + } + lines = append(lines, expandFormat(compiled, src)) + } + return nil + } + + if !enrich { + if err := render(ctx); err != nil { + return nil, err } + return lines, nil + } - lineFormat := format - if lineFormat == "" { - lineFormat = "%i\t%d\t%t" + if len(entries) >= enrichWarnThreshold { + t.Runtime.Logger().Warn( + "listing format reads per-node metadata", + "nodes", len(entries), + "keg", describeKeg(k), + ) + } + if err := keg.WithReadBoundary(ctx, k, render); err != nil { + return nil, err + } + return lines, nil +} + +// loadNodeFields fetches only what the compiled format actually needs. A read +// failure leaves the value empty rather than failing the listing: the stale +// index path already anticipates nodes that are indexed but unreadable, and one +// broken node must not blank an entire listing. +func (t *Tap) loadNodeFields(ctx context.Context, k keg.Keg, src *nodeFieldSource, compiled compiledFormat) { + id, err := keg.ParseNode(src.entry.ID) + if err != nil || id == nil { + return + } + if compiled.needsMeta { + if meta, err := k.GetMeta(ctx, *id); err == nil { + src.meta = meta + } + } + if compiled.needsStats { + if stats, err := k.GetStats(ctx, *id); err == nil { + src.stats = stats } + } +} - line := lineFormat - line = strings.Replace(line, "%i", entry.ID, -1) - line = strings.Replace(line, "%d", entry.Updated.Format(time.RFC3339), -1) - line = strings.Replace(line, "%c", entry.Created.Format(time.RFC3339), -1) - line = strings.Replace(line, "%a", entry.Accessed.Format(time.RFC3339), -1) - line = strings.Replace(line, "%t", entry.Title, -1) - lines = append(lines, line) +func renderNodeIDs(entries []keg.NodeIndexEntry, reverse bool) []string { + lines := make([]string, 0, len(entries)) + start, end, step := iterationBounds(len(entries), reverse) + for i := start; i != end; i += step { + lines = append(lines, entries[i].ID) } return lines } +func iterationBounds(n int, reverse bool) (start, end, step int) { + if reverse { + return n - 1, -1, -1 + } + return 0, n, 1 +} + func sortNodeIndexEntriesByTime(entries []keg.NodeIndexEntry, timeFunc func(keg.NodeIndexEntry) time.Time) { for i := 1; i < len(entries); i++ { for j := i; j > 0; j-- { diff --git a/pkg/tapper/tap_list_format.go b/pkg/tapper/tap_list_format.go new file mode 100644 index 0000000..1914dd5 --- /dev/null +++ b/pkg/tapper/tap_list_format.go @@ -0,0 +1,220 @@ +package tapper + +import ( + "fmt" + "strings" + "time" + + "github.com/jlrickert/tapper/pkg/keg" +) + +// defaultListFormat is the format used when none is supplied. It is +// tab-separated so listing output stays machine-readable by default. +const defaultListFormat = "%i\t%d\t%t" + +// formatSegment is one piece of a compiled format: either literal text, or a +// field selector to expand per node. +type formatSegment struct { + literal string + sel keg.FieldSelector + isField bool +} + +// compiledFormat is a format string parsed once, ahead of any I/O. needsMeta +// and needsStats report whether expanding it requires per-node reads, which +// lets the renderer skip enrichment entirely for the common formats. +type compiledFormat struct { + segments []formatSegment + needsMeta bool + needsStats bool +} + +// compileListFormat parses a listing format into segments. +// +// The scanner is a single left-to-right pass rather than a chain of +// replacements. That is what makes expansion safe: an expanded value is +// appended to the output and never rescanned, so a node whose title contains +// "%c" renders literally instead of being expanded again. +// +// Escaping is deliberately asymmetric. "%{" is a new introducer with no +// legacy meaning, so a malformed one is an error and typos surface +// immediately. A bare "%" followed by anything else passes through as +// literal text, matching the historical behaviour, so no format string that +// works today starts failing. +func compileListFormat(format string) (compiledFormat, error) { + if format == "" { + format = defaultListFormat + } + + var out compiledFormat + var lit strings.Builder + + flush := func() { + if lit.Len() > 0 { + out.segments = append(out.segments, formatSegment{literal: lit.String()}) + lit.Reset() + } + } + appendField := func(sel keg.FieldSelector) { + flush() + out.segments = append(out.segments, formatSegment{sel: sel, isField: true}) + if sel.NeedsMeta() { + out.needsMeta = true + } + if sel.NeedsStats() { + out.needsStats = true + } + } + + for i := 0; i < len(format); { + c := format[i] + if c != '%' { + lit.WriteByte(c) + i++ + continue + } + // A trailing '%' is literal. + if i+1 >= len(format) { + lit.WriteByte('%') + i++ + continue + } + + switch next := format[i+1]; { + case next == '%': + lit.WriteByte('%') + i += 2 + + case next == '{': + end := strings.IndexByte(format[i+2:], '}') + if end < 0 { + return compiledFormat{}, fmt.Errorf("invalid format: unterminated %q", "%{") + } + inner := format[i+2 : i+2+end] + sel, err := keg.ParseFieldSelector(inner) + if err != nil { + return compiledFormat{}, fmt.Errorf("invalid format: %w", err) + } + appendField(sel) + i += 2 + end + 1 + + default: + name, ok := keg.LegacyFormatVerbs[next] + if !ok { + // Unknown verb: pass both bytes through untouched. + lit.WriteByte('%') + lit.WriteByte(next) + i += 2 + continue + } + sel, err := keg.ParseFieldSelector(name) + if err != nil { + return compiledFormat{}, fmt.Errorf("invalid format: %w", err) + } + appendField(sel) + i += 2 + } + } + flush() + + return out, nil +} + +// nodeFieldSource carries everything needed to expand a compiled format for +// one node. meta and stats are nil unless the format required them. +type nodeFieldSource struct { + entry keg.NodeIndexEntry + meta *keg.NodeMeta + stats *keg.NodeStats +} + +// fieldValue resolves one selector against a node. +// +// Intrinsics and index timestamps come from the index entry, never from +// stats.json, so a displayed value always agrees with the same predicate in a +// query expression and the default format stays free of per-node reads. +func fieldValue(sel keg.FieldSelector, src nodeFieldSource) string { + switch sel.Kind { + case keg.FieldID: + return src.entry.ID + case keg.FieldTitle: + return src.entry.Title + case keg.FieldIndexTime: + switch sel.Key { + case "updated": + return formatEntryTime(src.entry.Updated) + case "created": + return formatEntryTime(src.entry.Created) + case "accessed": + return formatEntryTime(src.entry.Accessed) + } + return "" + case keg.FieldStat: + value, _ := keg.StatsFieldValue(src.stats, sel.Key) + return value + case keg.FieldTags, keg.FieldMetaKey: + if src.meta == nil { + return "" + } + // An absent key, a non-scalar value, and an empty tag list all + // collapse to empty here. That keeps a tabular format's column count + // stable regardless of which nodes carry the key. + value, _ := src.meta.Get(sel.Key) + return value + } + return "" +} + +func formatEntryTime(t time.Time) string { + if t.IsZero() { + return "" + } + return t.Format(time.RFC3339) +} + +// expandFormat renders one line for a node. +func expandFormat(compiled compiledFormat, src nodeFieldSource) string { + var b strings.Builder + for _, seg := range compiled.segments { + if !seg.isField { + b.WriteString(seg.literal) + continue + } + b.WriteString(sanitizeFieldValue(fieldValue(seg.sel, src))) + } + return b.String() +} + +// sanitizeFieldValue collapses control characters in an expanded value to +// single spaces. +// +// This is required, not cosmetic: each rendered string is one output line, and +// values such as .lead or a YAML block scalar routinely contain newlines. An +// unsanitised value would silently emit extra lines and desynchronise every +// caller that counts lines. Only expanded values are sanitised — literal text +// the caller typed into the format string is left exactly as written, so an +// explicit "\t" separator survives. +func sanitizeFieldValue(value string) string { + if strings.IndexFunc(value, isControlRune) < 0 { + return value + } + var b strings.Builder + b.Grow(len(value)) + prevControl := false + for _, r := range value { + if isControlRune(r) { + if !prevControl { + b.WriteByte(' ') + } + prevControl = true + continue + } + prevControl = false + b.WriteRune(r) + } + return b.String() +} + +func isControlRune(r rune) bool { + return r < 0x20 || r == 0x7f +} diff --git a/pkg/tapper/tap_list_format_test.go b/pkg/tapper/tap_list_format_test.go new file mode 100644 index 0000000..0ab386e --- /dev/null +++ b/pkg/tapper/tap_list_format_test.go @@ -0,0 +1,243 @@ +package tapper + +import ( + "strings" + "testing" + "time" + + "github.com/jlrickert/tapper/pkg/keg" +) + +func testEntry() keg.NodeIndexEntry { + return keg.NodeIndexEntry{ + ID: "3", + Title: "A Node", + Updated: time.Date(2026, 7, 29, 12, 0, 0, 0, time.UTC), + Created: time.Date(2026, 7, 1, 9, 30, 0, 0, time.UTC), + Accessed: time.Date(2026, 7, 30, 8, 15, 0, 0, time.UTC), + } +} + +func renderOne(t *testing.T, format string, src nodeFieldSource) string { + t.Helper() + compiled, err := compileListFormat(format) + if err != nil { + t.Fatalf("compileListFormat(%q): %v", format, err) + } + return expandFormat(compiled, src) +} + +func TestCompileListFormatDefaultIsUnchanged(t *testing.T) { + // The default must stay byte-identical to the historical output, or every + // script parsing `tap list` breaks. + got := renderOne(t, "", nodeFieldSource{entry: testEntry()}) + want := "3\t2026-07-29T12:00:00Z\tA Node" + if got != want { + t.Errorf("default format = %q, want %q", got, want) + } +} + +func TestCompileListFormatLegacyVerbs(t *testing.T) { + src := nodeFieldSource{entry: testEntry()} + tests := map[string]string{ + "%i": "3", + "%t": "A Node", + "%d": "2026-07-29T12:00:00Z", + "%c": "2026-07-01T09:30:00Z", + "%a": "2026-07-30T08:15:00Z", + "%i|%t": "3|A Node", + "%i\t%c": "3\t2026-07-01T09:30:00Z", + "[%i] %t!": "[3] A Node!", + } + for format, want := range tests { + if got := renderOne(t, format, src); got != want { + t.Errorf("format %q = %q, want %q", format, got, want) + } + } +} + +func TestCompileListFormatNamedSelectorsMatchLegacyVerbs(t *testing.T) { + // The aliases must not drift from what they alias. + src := nodeFieldSource{entry: testEntry()} + pairs := [][2]string{ + {"%i", "%{id}"}, + {"%t", "%{title}"}, + {"%d", "%{.updated}"}, + {"%c", "%{.created}"}, + {"%a", "%{.accessed}"}, + } + for _, pair := range pairs { + legacy := renderOne(t, pair[0], src) + named := renderOne(t, pair[1], src) + if legacy != named { + t.Errorf("%s = %q but %s = %q; aliases must agree", pair[0], legacy, pair[1], named) + } + } +} + +func TestCompileListFormatLiteralPercent(t *testing.T) { + src := nodeFieldSource{entry: testEntry()} + tests := map[string]string{ + // Documented in the CLI help for a long time but never implemented. + "100%%": "100%", + "%%": "%", + "%%i": "%i", + "%%%i": "%3", + "%i%%": "3%", + "%%{id}": "%{id}", + } + for format, want := range tests { + if got := renderOne(t, format, src); got != want { + t.Errorf("format %q = %q, want %q", format, got, want) + } + } +} + +func TestCompileListFormatUnknownVerbPassesThrough(t *testing.T) { + // Historical behaviour: an unrecognised %X is literal text. Erroring here + // would break formats containing bare percents. + src := nodeFieldSource{entry: testEntry()} + tests := map[string]string{ + "%z": "%z", + "50% off": "50% off", + "%": "%", + "%i %z": "3 %z", + } + for format, want := range tests { + if got := renderOne(t, format, src); got != want { + t.Errorf("format %q = %q, want %q", format, got, want) + } + } +} + +func TestCompileListFormatErrors(t *testing.T) { + tests := map[string]string{ + "%{": "unterminated", + "%{id": "unterminated", + "%{}": "empty selector", + "%{ }": "empty selector", + "%{.bogus}": "unknown stats field", + "%{type=plan}": "not a predicate", + "%{omega>=0.5}": "not a predicate", + } + for format, wantSubstr := range tests { + _, err := compileListFormat(format) + if err == nil { + t.Errorf("compileListFormat(%q) succeeded, want error", format) + continue + } + if !strings.Contains(err.Error(), "invalid format") { + t.Errorf("compileListFormat(%q) error = %q, want it prefixed with %q", format, err, "invalid format") + } + if !strings.Contains(err.Error(), wantSubstr) { + t.Errorf("compileListFormat(%q) error = %q, want it to mention %q", format, err, wantSubstr) + } + } +} + +func TestExpandFormatDoesNotReexpandValues(t *testing.T) { + // The old implementation was a chain of strings.Replace over the whole + // line, so an expanded value containing a verb was expanded again. A + // single left-to-right pass makes that structurally impossible. + entry := testEntry() + entry.Title = "%c and %{id} and %%" + got := renderOne(t, "%t", nodeFieldSource{entry: entry}) + if got != "%c and %{id} and %%" { + t.Errorf("title = %q, want it rendered literally", got) + } +} + +func TestExpandFormatZeroTimestampsRenderEmpty(t *testing.T) { + // 0001-01-01T00:00:00Z parses as a real date and would poison sorting. + src := nodeFieldSource{entry: keg.NodeIndexEntry{ID: "1", Title: "T"}} + if got := renderOne(t, "%i|%d|%c|%a", src); got != "1|||" { + t.Errorf("zero timestamps = %q, want %q", got, "1|||") + } +} + +func TestExpandFormatSanitizesControlCharacters(t *testing.T) { + // Each rendered string is one output line. A value containing a newline + // would silently emit extra lines and desynchronise every line-counting + // caller. Literal text in the format is untouched, so an explicit tab + // separator must survive. + entry := testEntry() + entry.Title = "line one\nline two\ttabbed" + got := renderOne(t, "%i\t%t", nodeFieldSource{entry: entry}) + want := "3\tline one line two tabbed" + if got != want { + t.Errorf("sanitised = %q, want %q", got, want) + } + if strings.ContainsAny(strings.TrimPrefix(got, "3\t"), "\n\t") { + t.Errorf("expanded value still contains control characters: %q", got) + } +} + +func TestExpandFormatCollapsesRunsOfControlCharacters(t *testing.T) { + entry := testEntry() + entry.Title = "a\n\n\nb" + if got := renderOne(t, "%t", nodeFieldSource{entry: entry}); got != "a b" { + t.Errorf("got %q, want %q", got, "a b") + } +} + +func TestCompileListFormatNeedsFlags(t *testing.T) { + // These flags gate whether the renderer performs per-node I/O at all, so + // a false positive silently makes every listing slow. + tests := []struct { + format string + wantMeta bool + wantStats bool + }{ + {format: ""}, + {format: "%i\t%d\t%t"}, + {format: "%{id} %{title} %{.updated} %{.created} %{.accessed}"}, + {format: "%{type}", wantMeta: true}, + {format: "%{tags}", wantMeta: true}, + {format: "%{.hash}", wantStats: true}, + {format: "%{.omega}", wantStats: true}, + {format: "%{type} %{.omega}", wantMeta: true, wantStats: true}, + } + for _, tc := range tests { + compiled, err := compileListFormat(tc.format) + if err != nil { + t.Fatalf("compileListFormat(%q): %v", tc.format, err) + } + if compiled.needsMeta != tc.wantMeta { + t.Errorf("format %q needsMeta = %v, want %v", tc.format, compiled.needsMeta, tc.wantMeta) + } + if compiled.needsStats != tc.wantStats { + t.Errorf("format %q needsStats = %v, want %v", tc.format, compiled.needsStats, tc.wantStats) + } + } +} + +func TestExpandFormatAbsentMetaAndStatsRenderEmpty(t *testing.T) { + // Absent values keep a stable column count rather than using a sentinel + // that could collide with a real value. + src := nodeFieldSource{entry: testEntry()} + if got := renderOne(t, "%i|%{type}|%{tags}|%{.hash}", src); got != "3|||" { + t.Errorf("absent fields = %q, want %q", got, "3|||") + } +} + +func TestExpandFormatAccessCountRendersZero(t *testing.T) { + // accessCount has no absent state on disk, so it always renders a number. + now := time.Date(2026, 7, 29, 12, 0, 0, 0, time.UTC) + src := nodeFieldSource{entry: testEntry(), stats: keg.NewStats(now)} + if got := renderOne(t, "%{.accessCount}", src); got != "0" { + t.Errorf("accessCount = %q, want %q", got, "0") + } +} + +func TestExpandFormatIntrinsicsShadowMetadata(t *testing.T) { + // A node carrying a `title` metadata key still renders the index title. + // This is the documented cost of reserving id/title/tags. + meta := keg.NewMeta(t.Context(), time.Date(2026, 7, 29, 12, 0, 0, 0, time.UTC)) + if err := meta.Set(t.Context(), "title", "metadata title"); err != nil { + t.Fatalf("set title: %v", err) + } + src := nodeFieldSource{entry: testEntry(), meta: meta} + if got := renderOne(t, "%{title}", src); got != "A Node" { + t.Errorf("title = %q, want the intrinsic %q", got, "A Node") + } +} From 25ad4e21c160b25fadea98c986a18f2cbfa282b3 Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Wed, 29 Jul 2026 23:26:41 -0500 Subject: [PATCH 2/7] feat(cli,mcp): advertise the listing field vocabulary on both surfaces The format vocabulary was documented inconsistently and, on MCP, incorrectly. Three of the five commands sharing the formatter documented no placeholders at all, links and backlinks stated a default they did not use, and every MCP schema advertised %i, %d, and %t while the implementation had long supported %c and %a as well, so two working verbs were undiscoverable to agents. Share one help block across list, grep, tags, links, and backlinks, and give each a --format completer, which none had. Extract the two byte-identical --query completers into one helper so they cannot drift again. Point the five MCP schemas at the same vocabulary. The descriptions must be tag literals, so a test asserts the generated schema still matches keg.FormatVocabularyDescription, and a second test guards the reflector rule that a description beginning with a token containing "=" panics at server construction -- which documenting a type= or status= selector would otherwise trip. Add the cross-surface conformance cases the vocabulary needs: one parity case per selector kind, so CLI and MCP cannot disagree on what a field is named or resolves to. Document the vocabulary in docs/output-formats.md, and fill the gap in docs/query-expressions.md, which described neither the statistics fields nor the comparison operators. --- docs/README.md | 1 + docs/output-formats.md | 113 ++++++++++++++++++++++++ docs/query-expressions.md | 31 ++++++- pkg/cli/cmd_backlinks.go | 6 +- pkg/cli/cmd_grep.go | 10 ++- pkg/cli/cmd_links.go | 6 +- pkg/cli/cmd_list.go | 22 +---- pkg/cli/cmd_list_test.go | 94 ++++++++++++++++++++ pkg/cli/cmd_tags.go | 15 +--- pkg/cli/format_completion.go | 76 ++++++++++++++++ pkg/mcp/tools_format_schema_test.go | 87 +++++++++++++++++++ pkg/mcp/tools_read.go | 10 +-- pkg/parity/parity_read_test.go | 129 ++++++++++++++++++++++++++++ 13 files changed, 555 insertions(+), 45 deletions(-) create mode 100644 docs/output-formats.md create mode 100644 pkg/cli/format_completion.go create mode 100644 pkg/mcp/tools_format_schema_test.go diff --git a/docs/README.md b/docs/README.md index 31e2245..bc2acf3 100644 --- a/docs/README.md +++ b/docs/README.md @@ -132,5 +132,6 @@ example `--plugin tapper-dev`, to add optional plugins. `--scope` defaults to - [Node Snapshots](node-snapshots.md) - [Backups And Archives](backups-and-archives.md) - [Query Expressions](query-expressions.md) +- [Output Formats](output-formats.md) - [Troubleshooting](configuration/troubleshooting.md) - [Architecture Overview](architecture/README.md) diff --git a/docs/output-formats.md b/docs/output-formats.md new file mode 100644 index 0000000..756baa6 --- /dev/null +++ b/docs/output-formats.md @@ -0,0 +1,113 @@ +# Output Formats + +`tap list`, `tap grep`, `tap tags`, `tap links`, and `tap backlinks` all render +node listings through one `--format` template, and the MCP tools of the same +names accept the same string. + +## Quick reference + +```sh +tap list # id, updated, title (default) +tap list -f '%i %t' # id and title +tap list -f '%i\t%{type}\t%{status}' # metadata columns +tap list -f '%i\t%{tags}' # the tag list +tap list -f '%i\t%{.accessCount}' # a statistics field +tap list -f '100%%' # a literal percent +``` + +The default format is `"%i\t%d\t%t"`. + +## Field selectors + +Selectors share the vocabulary of [query expressions](query-expressions.md), so +one set of names covers both filtering and display. A selector appears in two +positions, and a bare word means something different in each: + +- **Predicate position** (a query expression) — a bare word is a **tag**, because + a metadata key is always written there as `key=value`. +- **Field position** (a format template) — a bare word is a **metadata key**, + because there is no value to compare against and a tag is not a field. + +Statistics fields carry a leading dot in both positions, so the one selector +that appears in both means the same thing in both. + +| Selector | Resolves to | Cost | +| --- | --- | --- | +| `%{id}` | the node id | free | +| `%{title}` | the node title | free | +| `%{.updated}`, `%{.created}`, `%{.accessed}` | index timestamps, RFC3339 | free | +| `%{.hash}`, `%{.lead}`, `%{.accessCount}`, `%{.omega}` | statistics fields | one read per node | +| `%{tags}` | the node's tags, comma separated | one read per node | +| `%{anything-else}` | that metadata key | one read per node | + +`id`, `title`, and `tags` are reserved. A node carrying metadata under one of +those keys cannot address it in field position; the intrinsic wins. + +### Legacy verbs + +The single-letter verbs remain supported as aliases. No new letters are added, +because a single letter cannot address an arbitrary metadata key. + +| Verb | Equivalent | +| --- | --- | +| `%i` | `%{id}` | +| `%t` | `%{title}` | +| `%d` | `%{.updated}` | +| `%c` | `%{.created}` | +| `%a` | `%{.accessed}` | +| `%%` | a literal `%` | + +An unrecognised `%X` passes through as literal text, so a format containing a +bare percent keeps working. + +## Absent values + +An absent value renders as the empty string rather than a placeholder, so a +tabular format keeps a stable column count no matter which nodes carry a key. +A sentinel such as `-` would be indistinguishable from a real value. + +```sh +tap list -f '%i\t%{type}' | cut -f2 # stays column 2 for every node +``` + +These all render empty: a metadata key the node does not have, a metadata value +that is not a scalar (a list or a map), an empty tag list, a zero timestamp, and +an absent `omega`. + +Two deliberate exceptions: + +- **`%{.accessCount}` always renders an integer, including `0`.** `stats.json` + omits the key when the count is zero, so absent and zero are the same state on + disk and there is no absence to represent. +- **`%{.omega}` distinguishes absent from zero**, because omega genuinely is + tri-state. An unset omega renders empty; an omega of `0` renders `0`. + +## Cost + +Formats naming only `id`, `title`, the three dates, or the legacy verbs — which +includes the default — read nothing beyond the index that a listing already +loads. + +Any other selector reads one file per node, for the nodes in the result window +only. Combine with `--limit` on a large keg. The same is true of the equivalent +query predicates: filtering on `entity=plan` or `.hash=` also reads per node. + +## Notes + +- **Spelling follows the query language, not the on-disk file.** The selector is + `%{.accessCount}`; the key inside `stats.json` is `access_count`. There is one + canonical vocabulary and it is the query language's. +- **`%{.created}` reads the index, not `stats.json`**, matching how `.created>…` + is evaluated in a query. If the index has drifted from `stats.json`, both show + the index value. Run `tap index rebuild` to reconcile. +- **Control characters in a rendered value collapse to single spaces.** Each + output line is one node, and a value such as `%{.lead}` can contain newlines. + Literal text in the template is untouched, so an explicit `\t` separator + survives. +- **Values are never re-scanned.** A node whose title contains `%c` renders it + literally. + +## See also + +- [Query Expressions](query-expressions.md) — the same vocabulary, in predicate + position. diff --git a/docs/query-expressions.md b/docs/query-expressions.md index 0eaf41b..6640b3f 100644 --- a/docs/query-expressions.md +++ b/docs/query-expressions.md @@ -17,7 +17,7 @@ A query expression is built from **terms** combined with **operators**. ### Terms -A term is either a tag name or a key=value attribute predicate. +A term is a tag name, a metadata predicate, or a statistics-field predicate. - **Tag**: a plain identifier that matches nodes carrying that tag. @@ -26,14 +26,37 @@ A term is either a tag name or a key=value attribute predicate. ``` - **Attribute predicate**: `key=value` matches nodes whose `meta.yaml` contains - the given key with the given value. + the given key with the given value. `!=` negates. ``` entity=plan + status!=draft ``` -Tags are resolved from the dex index (fast). Attribute predicates scan each -node's `meta.yaml` (slower on large kegs). +- **Statistics-field predicate**: a dot-prefixed field name, optionally compared + with `=`, `!=`, `<`, `<=`, `>`, or `>=`. With no operator it is a non-zero + existence check. + + ``` + .created>2026-01-01 + .accessCount>=5 + .hash=abc123 + .omega + ``` + + The recognized fields are `updated`, `created`, `accessed`, `hash`, + `accessCount`, `lead`, and `omega`. + +Tags are resolved from the dex index (fast), as are the `.updated`, `.created`, +and `.accessed` timestamps. Attribute predicates and the remaining statistics +fields scan each node's `meta.yaml` or `stats.json` (slower on large kegs). + +### Bare words + +A bare word means a **tag** here, because a metadata key is always written as +`key=value` in predicate position. The same vocabulary also names fields for +*display*, where a bare word instead names a metadata key — see +[Output Formats](output-formats.md). ### Operators diff --git a/pkg/cli/cmd_backlinks.go b/pkg/cli/cmd_backlinks.go index 0ba5676..4ce7b15 100644 --- a/pkg/cli/cmd_backlinks.go +++ b/pkg/cli/cmd_backlinks.go @@ -16,8 +16,9 @@ func NewBacklinksCmd(deps *Deps) *cobra.Command { Long: `List nodes that link to the given NODE_IDs. When multiple IDs are provided, results are merged and deduplicated. -Format placeholders: %i (node id), %d (date), %t (title), %% (literal %). -Default format: "%i %d %t".`, +` + formatHelp + ` + +Default format: "%i\t%d\t%t".`, Args: cobra.MinimumNArgs(1), ValidArgsFunction: nodeIDCompletionFunc(deps, 0), RunE: func(cmd *cobra.Command, args []string) error { @@ -40,6 +41,7 @@ Default format: "%i %d %t".`, cmd.Flags().IntVarP(&opts.Limit, "limit", "n", 0, "maximum number of results (0 for no limit)") cmd.Flags().IntVar(&opts.Offset, "offset", 0, "skip the first N results") cmd.Flags().StringVarP(&opts.Format, "format", "f", "", "output format") + registerFormatCompletion(cmd) return cmd } diff --git a/pkg/cli/cmd_grep.go b/pkg/cli/cmd_grep.go index f1943bd..a67fddf 100644 --- a/pkg/cli/cmd_grep.go +++ b/pkg/cli/cmd_grep.go @@ -13,8 +13,13 @@ func NewGrepCmd(deps *Deps) *cobra.Command { cmd := &cobra.Command{ Use: "grep QUERY", Short: "search node content by query", - Long: "Search node content with a regex and print matching lines grouped by node.", - Args: cobra.ExactArgs(1), + Long: `Search node content with a regex and print matching lines grouped by node. + +With --format or --id-only, matching nodes are listed instead of their +matching lines. + +` + formatHelp, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Query = args[0] applyKegTargetProfile(deps, &opts.KegTargetOptions) @@ -35,6 +40,7 @@ func NewGrepCmd(deps *Deps) *cobra.Command { cmd.Flags().IntVarP(&opts.Limit, "limit", "n", 0, "maximum number of results (0 for no limit)") cmd.Flags().IntVar(&opts.Offset, "offset", 0, "skip the first N results") cmd.Flags().StringVarP(&opts.Format, "format", "f", "", "output format") + registerFormatCompletion(cmd) cmd.Flags().BoolVarP(&opts.IgnoreCase, "ignore-case", "i", false, "perform case-insensitive matching") cmd.Flags().IntVar(&opts.MaxLines, "max-lines", 0, "maximum matched lines per node (0 for unlimited)") diff --git a/pkg/cli/cmd_links.go b/pkg/cli/cmd_links.go index 75f723a..3e1b777 100644 --- a/pkg/cli/cmd_links.go +++ b/pkg/cli/cmd_links.go @@ -16,8 +16,9 @@ func NewLinksCmd(deps *Deps) *cobra.Command { Long: `List nodes that the given NODE_IDs link to. When multiple IDs are provided, results are merged and deduplicated. -Format placeholders: %i (node id), %d (date), %t (title), %% (literal %). -Default format: "%i %d %t".`, +` + formatHelp + ` + +Default format: "%i\t%d\t%t".`, Args: cobra.MinimumNArgs(1), ValidArgsFunction: nodeIDCompletionFunc(deps, 0), RunE: func(cmd *cobra.Command, args []string) error { @@ -40,6 +41,7 @@ Default format: "%i %d %t".`, cmd.Flags().IntVarP(&opts.Limit, "limit", "n", 0, "maximum number of results (0 for no limit)") cmd.Flags().IntVar(&opts.Offset, "offset", 0, "skip the first N results") cmd.Flags().StringVarP(&opts.Format, "format", "f", "", "output format") + registerFormatCompletion(cmd) return cmd } diff --git a/pkg/cli/cmd_list.go b/pkg/cli/cmd_list.go index f8e014b..686a2a9 100644 --- a/pkg/cli/cmd_list.go +++ b/pkg/cli/cmd_list.go @@ -2,9 +2,7 @@ package cli import ( "fmt" - "strings" - "github.com/jlrickert/tapper/pkg/keg" "github.com/jlrickert/tapper/pkg/tapper" "github.com/spf13/cobra" ) @@ -17,13 +15,7 @@ func NewListCmd(deps *Deps) *cobra.Command { Short: "list all indexed nodes", Long: `List indexed nodes for the resolved keg. -Format placeholders: - %i node id - %d updated date - %c created date - %a accessed date - %t title - %% literal percent +` + formatHelp + ` Default format: "%i\t%d\t%t". @@ -67,16 +59,8 @@ Use --sort to order by "id", "updated", "created", or "accessed".`, mustRegisterFlagCompletion(cmd, "sort", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return []string{"id", "updated", "created", "accessed"}, cobra.ShellCompDirectiveNoFileComp }) - mustRegisterFlagCompletion(cmd, "query", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - if strings.HasPrefix(toComplete, ".") || toComplete == "" { - suggestions := make([]string, len(keg.StatsFieldNames)) - for i, name := range keg.StatsFieldNames { - suggestions[i] = "." + name - } - return suggestions, cobra.ShellCompDirectiveNoFileComp - } - return nil, cobra.ShellCompDirectiveNoFileComp - }) + registerQueryFieldCompletion(cmd) + registerFormatCompletion(cmd) return cmd } diff --git a/pkg/cli/cmd_list_test.go b/pkg/cli/cmd_list_test.go index aed294b..1a406e0 100644 --- a/pkg/cli/cmd_list_test.go +++ b/pkg/cli/cmd_list_test.go @@ -530,3 +530,97 @@ func TestListCommand_AttrCompare_MixedWithDotPrefix(t *testing.T) { require.NotContains(t, trimmed, "1", "plan node should NOT match entity!=plan") require.Contains(t, trimmed, "2", "task node should match entity!=plan and .created") } + +func TestListCommand_FormatLiteralPercent(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) + + // %% was documented in the help long before it was implemented; the old + // replace-chain left it untouched. + res := NewProcess(t, false, "list", "-f", "%i 100%%").Run(sb.Context(), sb.Runtime()) + require.NoError(t, res.Err) + + out := strings.TrimSpace(string(res.Stdout)) + require.NotEmpty(t, out) + for _, line := range strings.Split(out, "\n") { + require.True(t, strings.HasSuffix(line, " 100%"), + "expected %%%% to render one literal percent, got %q", line) + } +} + +func TestListCommand_FormatMetadataSelector(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("queryuser", "~")) + + res := NewProcess(t, false, "list", "-f", "%i\t%{entity}\t%{status}").Run(sb.Context(), sb.Runtime()) + require.NoError(t, res.Err) + out := strings.TrimSpace(string(res.Stdout)) + require.NotEmpty(t, out) + + // Node 9 carries entity=task, status=done in meta.yaml. Reaching an + // arbitrary metadata key is the whole point of the named selectors. + require.Contains(t, out, "9\ttask\tdone") + + // A node without those keys renders empty columns rather than dropping + // them, so the column count stays stable. + require.Contains(t, out, "0\t\t") +} + +func TestListCommand_FormatTagsSelector(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("queryuser", "~")) + + res := NewProcess(t, false, "list", "-f", "%i\t%{tags}").Run(sb.Context(), sb.Runtime()) + require.NoError(t, res.Err) + out := strings.TrimSpace(string(res.Stdout)) + + require.Contains(t, out, "9\tbackend,performance") +} + +func TestListCommand_FormatIntrinsicsShadowMetadata(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("queryuser", "~")) + + // %{title} is the intrinsic index title, never a `title` metadata key. + titles := NewProcess(t, false, "list", "-f", "%{title}").Run(sb.Context(), sb.Runtime()) + require.NoError(t, titles.Err) + legacy := NewProcess(t, false, "list", "-f", "%t").Run(sb.Context(), sb.Runtime()) + require.NoError(t, legacy.Err) + + require.Equal(t, string(legacy.Stdout), string(titles.Stdout), + "%{title} and %t must resolve identically") +} + +func TestListCommand_FormatUnknownStatsFieldErrors(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) + + // The stats vocabulary is closed, so a typo is reported rather than + // silently rendering an empty column. + res := NewProcess(t, false, "list", "-f", "%{.bogus}").Run(sb.Context(), sb.Runtime()) + require.Error(t, res.Err) + require.Contains(t, res.Err.Error(), "invalid format") + require.Contains(t, res.Err.Error(), "unknown stats field") +} + +func TestListCommand_FormatUnterminatedBraceErrors(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) + + res := NewProcess(t, false, "list", "-f", "%{id").Run(sb.Context(), sb.Runtime()) + require.Error(t, res.Err) + require.Contains(t, res.Err.Error(), "invalid format") +} + +func TestListCommand_FormatCompletionSuggestsSelectors(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("testuser", "~")) + + res := NewCompletionProcess(t, false, 0, "list", "--format", "").Run(sb.Context(), sb.Runtime()) + require.NoError(t, res.Err) + suggestions := parseCompletionSuggestions(string(res.Stdout)) + + require.Contains(t, suggestions, "%{id}") + require.Contains(t, suggestions, "%{tags}") + require.Contains(t, suggestions, "%{.accessCount}") +} diff --git a/pkg/cli/cmd_tags.go b/pkg/cli/cmd_tags.go index 5eedeba..ce77ad0 100644 --- a/pkg/cli/cmd_tags.go +++ b/pkg/cli/cmd_tags.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/jlrickert/tapper/pkg/keg" "github.com/jlrickert/tapper/pkg/tapper" "github.com/spf13/cobra" ) @@ -28,6 +27,8 @@ Expression language: - Grouping: parentheses () - Precedence: not > and > or +` + formatHelp + ` + Examples: tap tags tap tags fire @@ -91,16 +92,8 @@ Examples: cmd.Flags().IntVar(&opts.Offset, "offset", 0, "skip the first N results") cmd.Flags().StringVarP(&opts.Format, "format", "f", "", "output format when TAG is provided") cmd.Flags().StringVar(&opts.Query, "query", "", `boolean expression (see "tap docs query-expressions" for syntax)`) - mustRegisterFlagCompletion(cmd, "query", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - if strings.HasPrefix(toComplete, ".") || toComplete == "" { - suggestions := make([]string, len(keg.StatsFieldNames)) - for i, name := range keg.StatsFieldNames { - suggestions[i] = "." + name - } - return suggestions, cobra.ShellCompDirectiveNoFileComp - } - return nil, cobra.ShellCompDirectiveNoFileComp - }) + registerQueryFieldCompletion(cmd) + registerFormatCompletion(cmd) return cmd } diff --git a/pkg/cli/format_completion.go b/pkg/cli/format_completion.go new file mode 100644 index 0000000..9bd1967 --- /dev/null +++ b/pkg/cli/format_completion.go @@ -0,0 +1,76 @@ +package cli + +import ( + "strings" + + "github.com/spf13/cobra" + + "github.com/jlrickert/tapper/pkg/keg" +) + +// formatHelp is the shared --format documentation for every listing command. +// It is one block so the five commands cannot drift apart, which they had +// already done: three of them documented no placeholders at all, and two +// stated a default they did not use. +const formatHelp = `Format placeholders: + %i node id (same as %{id}) + %t title (same as %{title}) + %d updated date (same as %{.updated}) + %c created date (same as %{.created}) + %a accessed date (same as %{.accessed}) + %% literal percent + +Named selectors use %{...} and share the query expression vocabulary: + %{type}, %{status} any metadata key + %{tags} the node's tag list + %{.accessCount} a statistics field: updated, created, accessed, + hash, accessCount, lead, omega + +Selectors other than id, title, and the three dates read one file per +node. Absent values render empty.` + +// registerFormatCompletion offers the closed part of the field vocabulary for +// a --format flag. Metadata keys are open-ended and so cannot be suggested. +// +// NoSpace is set because a format string is composite: the user is usually +// building a longer template around the token they just accepted. +func registerFormatCompletion(cmd *cobra.Command) { + mustRegisterFlagCompletion(cmd, "format", func(_ *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) { + directive := cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveNoSpace + suggestions := keg.FormatSelectorSuggestions() + if toComplete == "" { + return suggestions, directive + } + // Suggest continuations of whatever selector the user has started, + // keeping any prefix they have already typed. + open := strings.LastIndex(toComplete, "%{") + if open < 0 || strings.Contains(toComplete[open:], "}") { + return nil, directive + } + prefix := toComplete[:open] + partial := toComplete[open:] + out := make([]string, 0, len(suggestions)) + for _, s := range suggestions { + if strings.HasPrefix(s, partial) { + out = append(out, prefix+s) + } + } + return out, directive + }) +} + +// registerQueryFieldCompletion offers the dot-prefix statistics fields for a +// --query flag. Tags and metadata keys are keg-specific and open-ended, so +// only the closed vocabulary is suggested. +func registerQueryFieldCompletion(cmd *cobra.Command) { + mustRegisterFlagCompletion(cmd, "query", func(_ *cobra.Command, _ []string, toComplete string) ([]string, cobra.ShellCompDirective) { + if strings.HasPrefix(toComplete, ".") || toComplete == "" { + suggestions := make([]string, len(keg.StatsFieldNames)) + for i, name := range keg.StatsFieldNames { + suggestions[i] = "." + name + } + return suggestions, cobra.ShellCompDirectiveNoFileComp + } + return nil, cobra.ShellCompDirectiveNoFileComp + }) +} diff --git a/pkg/mcp/tools_format_schema_test.go b/pkg/mcp/tools_format_schema_test.go new file mode 100644 index 0000000..fce25ab --- /dev/null +++ b/pkg/mcp/tools_format_schema_test.go @@ -0,0 +1,87 @@ +package mcp_test + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/jlrickert/tapper/pkg/keg" +) + +// schemaProperties decodes the description of each property in a tool's input +// schema. The schema is carried as an opaque value, so it is inspected through +// its JSON form rather than a concrete type. +func schemaProperties(t *testing.T, schema any) map[string]string { + t.Helper() + if schema == nil { + return nil + } + raw, err := json.Marshal(schema) + require.NoError(t, err) + + var decoded struct { + Properties map[string]struct { + Description string `json:"description"` + } `json:"properties"` + } + require.NoError(t, json.Unmarshal(raw, &decoded)) + + out := make(map[string]string, len(decoded.Properties)) + for name, prop := range decoded.Properties { + out[name] = prop.Description + } + return out +} + +// formatToolNames are the tools whose output is rendered through the shared +// listing formatter, and which therefore must advertise the shared vocabulary. +var formatToolNames = []string{"list", "grep", "tags", "backlinks", "links"} + +// TestMCP_FormatSchemaAdvertisesSharedVocabulary pins the generated tool +// schemas to the vocabulary the formatter actually implements. +// +// The struct tags carrying these descriptions must be literals, so they are +// duplicated from keg.FormatVocabularyDescription. This test is what keeps the +// copy honest: before it existed, the schemas advertised %i, %d, and %t while +// the implementation had also supported %c and %a for some time, so two working +// verbs were undiscoverable to agents. +func TestMCP_FormatSchemaAdvertisesSharedVocabulary(t *testing.T) { + t.Parallel() + session, ctx := newTestSession(t) + + res, err := session.ListTools(ctx, nil) + require.NoError(t, err) + + seen := map[string]bool{} + for _, tool := range res.Tools { + description, ok := schemaProperties(t, tool.InputSchema)["format"] + if !ok { + continue + } + seen[tool.Name] = true + require.Truef(t, + strings.HasPrefix(description, keg.FormatVocabularyDescription), + "tool %q format description drifted from keg.FormatVocabularyDescription:\n got: %s\nwant prefix: %s", + tool.Name, description, keg.FormatVocabularyDescription) + } + + for _, name := range formatToolNames { + require.Truef(t, seen[name], "tool %q exposes no format property", name) + } +} + +// TestMCP_FormatSchemaDescriptionIsReflectorSafe guards a sharp edge in schema +// generation: a description whose first whitespace-delimited token contains an +// equals sign is rejected by the reflector, and AddTool turns that into a panic +// at server construction, taking down the whole MCP server rather than one +// tool. Documenting a "type=" or "status=" selector is exactly the case that +// would trip it. +func TestMCP_FormatSchemaDescriptionIsReflectorSafe(t *testing.T) { + t.Parallel() + + first, _, _ := strings.Cut(keg.FormatVocabularyDescription, " ") + require.NotContainsf(t, first, "=", + "description must not begin with a token containing '='; got %q", first) +} diff --git a/pkg/mcp/tools_read.go b/pkg/mcp/tools_read.go index 09e207c..493e4f7 100644 --- a/pkg/mcp/tools_read.go +++ b/pkg/mcp/tools_read.go @@ -62,7 +62,7 @@ func registerCat(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type listInput struct { Query string `json:"query,omitempty" jsonschema:"boolean query expression to filter nodes. Supports tags ('golang'), key=value attributes ('entity=plan'), and dot-prefix stats fields ('.created>2026-01-01', '.accessCount>=5', '.hash=abc123'). Combine with 'and', 'or', 'not'."` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format (%i=id %d=date %t=title)"` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Sort string `json:"sort,omitempty" jsonschema:"sort order: 'id' (default), 'updated', 'created', or 'accessed'"` @@ -102,7 +102,7 @@ func registerList(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type grepInput struct { Query string `json:"query" jsonschema:"regex pattern to search node content"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format (%i=id %d=date %t=title); use id_only for compact MCP output"` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node. Use id_only for compact MCP output."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only (recommended for MCP to reduce token usage)"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` IgnoreCase bool `json:"ignore_case,omitempty" jsonschema:"case-insensitive matching"` @@ -144,7 +144,7 @@ func registerGrep(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type tagsInput struct { Query string `json:"query,omitempty" jsonschema:"boolean expression to filter by tags, attributes, and dot-prefix stats fields (e.g. '.created>2026-01-01 and entity=plan')"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format (%i=id %d=date %t=title)"` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Limit int `json:"limit,omitempty" jsonschema:"maximum results to return (default: 50, 0 in request means use default, -1 for unlimited)"` @@ -182,7 +182,7 @@ func registerTags(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type backlinksInput struct { NodeIDs []string `json:"node_ids" jsonschema:"target node IDs to find incoming links for (results merged and deduplicated)"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format (%i=id %d=date %t=title)"` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Limit int `json:"limit,omitempty" jsonschema:"maximum results to return (default: 50, 0 in request means use default, -1 for unlimited)"` @@ -220,7 +220,7 @@ func registerBacklinks(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults type linksInput struct { NodeIDs []string `json:"node_ids" jsonschema:"source node IDs to find outgoing links for (results merged and deduplicated)"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format (%i=id %d=date %t=title)"` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Limit int `json:"limit,omitempty" jsonschema:"maximum results to return (default: 50, 0 in request means use default, -1 for unlimited)"` diff --git a/pkg/parity/parity_read_test.go b/pkg/parity/parity_read_test.go index ddb7898..9a0ca35 100644 --- a/pkg/parity/parity_read_test.go +++ b/pkg/parity/parity_read_test.go @@ -140,6 +140,135 @@ func TestParity_ReadOperations(t *testing.T) { }, }, + // --- listing field vocabulary conformance --- + // One case per selector kind, so the CLI and MCP surfaces cannot drift + // on what a field is named or what it resolves to. Adding a selector + // without a case here leaves half the vocabulary unverified on one + // surface, which is how %c and %a came to work on both while only one + // advertised them. + { + Name: "list/format_legacy_verbs", + CLIArgs: []string{"list", "-f", "%i|%t|%d|%c|%a"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%i|%t|%d|%c|%a", + "limit": -1, + }, + }, + { + Name: "list/format_named_intrinsics", + CLIArgs: []string{"list", "-f", "%{id}|%{title}"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%{id}|%{title}", + "limit": -1, + }, + }, + { + Name: "list/format_index_times", + CLIArgs: []string{"list", "-f", "%{.updated}|%{.created}|%{.accessed}"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%{.updated}|%{.created}|%{.accessed}", + "limit": -1, + }, + }, + { + Name: "list/format_tags", + CLIArgs: []string{"list", "-f", "%i\t%{tags}"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%i\t%{tags}", + "limit": -1, + }, + }, + { + Name: "list/format_stats_hash", + CLIArgs: []string{"list", "-f", "%i\t%{.hash}"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%i\t%{.hash}", + "limit": -1, + }, + }, + { + Name: "list/format_stats_access_count", + CLIArgs: []string{"list", "-f", "%i\t%{.accessCount}"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%i\t%{.accessCount}", + "limit": -1, + }, + }, + { + // An absent metadata key renders empty on both surfaces, so a + // tabular format keeps a stable column count. + Name: "list/format_absent_meta_key", + CLIArgs: []string{"list", "-f", "%i|%{type}|end"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%i|%{type}|end", + "limit": -1, + }, + }, + { + // Documented in the help for a long time, implemented only now. + Name: "list/format_literal_percent", + CLIArgs: []string{"list", "-f", "%i 100%%"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%i 100%%", + "limit": -1, + }, + }, + { + Name: "list/format_unknown_stats_field", + CLIArgs: []string{"list", "-f", "%{.bogus}"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%{.bogus}", + "limit": -1, + }, + WantErr: true, + WantErrContains: "invalid format", + }, + { + Name: "list/format_unterminated_brace", + CLIArgs: []string{"list", "-f", "%{id"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%{id", + "limit": -1, + }, + WantErr: true, + WantErrContains: "invalid format", + }, + { + // Pasting query syntax into field position is the likely mistake, + // so both surfaces must name it rather than fail generically. + Name: "list/format_predicate_in_field_position", + CLIArgs: []string{"list", "-f", "%{type=plan}"}, + MCPTool: "list", + MCPInput: map[string]any{ + "format": "%{type=plan}", + "limit": -1, + }, + WantErr: true, + WantErrContains: "invalid format", + }, + { + // The vocabulary must be identical on every command sharing the + // formatter, not just list. + Name: "tags/format_named_selectors", + CLIArgs: []string{"tags", "hello", "-f", "%i\t%{tags}"}, + MCPTool: "tags", + MCPInput: map[string]any{ + "query": "hello", + "format": "%i\t%{tags}", + "limit": -1, + }, + }, + // --- list with sort (Tap.List) --- { Name: "list/sort_updated", From 4f4a477b79094b89b64bf7061789e7edf10d6944 Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 30 Jul 2026 00:21:05 -0500 Subject: [PATCH 3/7] feat(keg): build node listings on the server Rendering a metadata column read each node individually, and on a remote keg every read is an HTTP round trip. Measured against a real hub, a 610-node listing took ~49s where the same listing without metadata took 0.21s -- one round trip versus 610. The cause is architectural. Tap.List assembled the listing on the client: fetch entries, sort, offset, limit, then enrich row by row. That inverts the rule the hub already states, that handlers are the remote Keg surface and the server-side LocalKeg owns orchestration. The N+1 is what client-side orchestration costs once a row needs more than the index carries. Add ListView, which resolves a whole listing page server-side: filter by query, order, page, then project the requested field selectors. Ordering the operations that way means a listing that displays metadata reads only the rows it returns, not every node in the keg. Sorting by a metadata key still resolves a key per matching node, since the whole set must be ordered before it can be paged. Field resolution moves to keg.FieldValue so the client renderer and the server projection cannot disagree about what a selector means, and stays best-effort per row: listings render from an index that is allowed to drift, so one unreadable node yields empty values rather than failing the page. A hub without the route answers 404, reported as ErrListViewUnsupported so Tap.List degrades to assembling the listing itself instead of failing. --- pkg/keg/format_fields.go | 102 +++++++++++++++++ pkg/keg/keg_aggregate.go | 171 ++++++++++++++++++++++++++++ pkg/keg/keg_iface.go | 8 ++ pkg/keg/keg_remote_aggregate.go | 19 ++++ pkg/keg/keg_remote_listview_test.go | 122 ++++++++++++++++++++ pkg/tapper/tap_list.go | 91 +++++++++++---- pkg/tapper/tap_list_format.go | 106 ++++++++++------- 7 files changed, 554 insertions(+), 65 deletions(-) create mode 100644 pkg/keg/keg_remote_listview_test.go diff --git a/pkg/keg/format_fields.go b/pkg/keg/format_fields.go index f1e9f3c..6eb0cbe 100644 --- a/pkg/keg/format_fields.go +++ b/pkg/keg/format_fields.go @@ -202,6 +202,108 @@ func formatStatsTime(t time.Time) string { return t.Format(time.RFC3339) } +// ParseFieldSelectors classifies a list of selectors, rejecting the whole list +// if any entry is invalid so a listing never silently drops a column. +func ParseFieldSelectors(raw []string) ([]FieldSelector, error) { + if len(raw) == 0 { + return nil, nil + } + out := make([]FieldSelector, 0, len(raw)) + for _, text := range raw { + sel, err := ParseFieldSelector(text) + if err != nil { + return nil, err + } + out = append(out, sel) + } + return out, nil +} + +// ParseSortSelector classifies a sort key. An empty key yields the zero +// selector, meaning "leave the listing in its natural index order". +func ParseSortSelector(raw string) (FieldSelector, error) { + if strings.TrimSpace(raw) == "" { + return FieldSelector{}, nil + } + return ParseFieldSelector(raw) +} + +// SelectorNeeds reports whether a set of selectors requires reading node +// metadata or statistics. Callers use it to skip per-node reads entirely for +// listings that name only intrinsics and index timestamps. +func SelectorNeeds(selectors []FieldSelector) (meta, stats bool) { + for _, sel := range selectors { + if sel.NeedsMeta() { + meta = true + } + if sel.NeedsStats() { + stats = true + } + } + return meta, stats +} + +// FieldValue resolves one selector against a node's index entry and, when the +// selector requires them, its metadata and statistics. meta and stats may be +// nil; an unresolvable value renders empty so a tabular listing keeps a stable +// column count. +// +// Intrinsics and index timestamps come from the entry, never from stats.json, +// so a displayed value always agrees with the same predicate in a query +// expression and the default listing stays free of per-node reads. +func FieldValue(sel FieldSelector, entry NodeIndexEntry, meta *NodeMeta, stats *NodeStats) string { + switch sel.Kind { + case FieldID: + return entry.ID + case FieldTitle: + return entry.Title + case FieldIndexTime: + return formatStatsTime(entryTimeField(entry, sel.Key)) + case FieldStat: + value, _ := StatsFieldValue(stats, sel.Key) + return value + case FieldTags, FieldMetaKey: + if meta == nil { + return "" + } + value, _ := meta.Get(sel.Key) + return value + } + return "" +} + +// entryTimeField returns the index timestamp named by an index-time selector. +func entryTimeField(entry NodeIndexEntry, name string) time.Time { + switch name { + case "updated": + return entry.Updated + case "created": + return entry.Created + case "accessed": + return entry.Accessed + } + return time.Time{} +} + +// sortNodeIndexEntriesByID orders entries by numeric node id, matching the +// natural dex order. +func sortNodeIndexEntriesByID(entries []NodeIndexEntry) { + slices.SortStableFunc(entries, func(a, b NodeIndexEntry) int { + left, lerr := ParseNode(a.ID) + right, rerr := ParseNode(b.ID) + if lerr != nil || rerr != nil || left == nil || right == nil { + return strings.Compare(a.ID, b.ID) + } + switch { + case left.Lt(*right): + return -1 + case right.Lt(*left): + return 1 + } + return 0 + }) +} + // FormatSelectorSuggestions returns the closed part of the field vocabulary as // ready-to-type format tokens, for shell completion. Metadata keys are // open-ended and therefore absent. diff --git a/pkg/keg/keg_aggregate.go b/pkg/keg/keg_aggregate.go index 5af7526..13e0941 100644 --- a/pkg/keg/keg_aggregate.go +++ b/pkg/keg/keg_aggregate.go @@ -24,6 +24,52 @@ type ListEntriesResult struct { NodeCount int `json:"node_count"` } +// ListViewOptions configures a fully server-resolved listing page. +// +// Filtering, ordering, and paging all happen before field projection, so a +// listing that displays metadata reads only the rows it returns rather than +// every node in the keg. +type ListViewOptions struct { + // Query is an optional boolean query expression filtering the nodes. + Query string `json:"query,omitempty"` + + // Fields are field selectors to resolve per row, in the vocabulary of + // ParseFieldSelector ("type", ".omega", "tags"). Intrinsics and index + // timestamps cost nothing; other selectors are read per returned row. + Fields []string `json:"fields,omitempty"` + + // Sort is the field selector to order by. Empty orders by node id. + Sort string `json:"sort,omitempty"` + + // Desc reverses the sort order. + Desc bool `json:"desc,omitempty"` + + // Limit caps the returned rows. 0 means no limit. + Limit int `json:"limit,omitempty"` + + // Offset skips the first N matching rows before applying Limit. + Offset int `json:"offset,omitempty"` +} + +// ListViewRow is one resolved listing row: its index entry plus the values of +// the requested field selectors, keyed by selector text. +type ListViewRow struct { + Entry NodeIndexEntry `json:"entry"` + Fields map[string]string `json:"fields,omitempty"` +} + +// ListViewResult is a resolved listing page. TotalMatches counts the rows the +// query selected before Limit and Offset were applied, so callers can page +// without re-running the query. +type ListViewResult struct { + Query string `json:"query,omitempty"` + Rows []ListViewRow `json:"rows"` + Tags []string `json:"tags"` + TotalMatches int `json:"total_matches"` + IndexedCount int `json:"indexed_count"` + NodeCount int `json:"node_count"` +} + type ReadNodesOptions struct { NodeIDs []NodeId `json:"node_ids,omitempty"` Query string `json:"query,omitempty"` @@ -156,6 +202,131 @@ func (k *LocalKeg) ListEntries(ctx context.Context, opts ListEntriesOptions) (*L return withKegReadValue(ctx, k, func(ctx context.Context) (*ListEntriesResult, error) { return k.listEntries(ctx, opts) }) } +func (k *LocalKeg) ListView(ctx context.Context, opts ListViewOptions) (*ListViewResult, error) { + return withKegReadValue(ctx, k, func(ctx context.Context) (*ListViewResult, error) { return k.listView(ctx, opts) }) +} + +// listView resolves a whole listing page server-side. +// +// The order of operations is what makes this cheap: filter, sort, then page, +// and only then resolve fields. Projecting after paging means a listing that +// displays metadata reads the rows it returns, not every node in the keg. +func (k *LocalKeg) listView(ctx context.Context, opts ListViewOptions) (*ListViewResult, error) { + selectors, err := ParseFieldSelectors(opts.Fields) + if err != nil { + return nil, err + } + sortSel, err := ParseSortSelector(opts.Sort) + if err != nil { + return nil, err + } + + listing, err := k.listEntries(ctx, ListEntriesOptions{Query: opts.Query}) + if err != nil { + return nil, err + } + entries := listing.Entries + total := len(entries) + + // Sorting by metadata or a non-timestamp stat needs a value for every + // matching node, not just the returned page, so it is resolved before + // paging. Intrinsics and index timestamps stay free. + if sortSel.Kind != FieldUnknown { + if err := k.sortEntriesBySelector(ctx, entries, sortSel); err != nil { + return nil, err + } + } + if opts.Desc { + slices.Reverse(entries) + } + + if opts.Offset > 0 { + if opts.Offset >= len(entries) { + entries = nil + } else { + entries = entries[opts.Offset:] + } + } + if opts.Limit > 0 && len(entries) > opts.Limit { + entries = entries[:opts.Limit] + } + + rows := make([]ListViewRow, 0, len(entries)) + for _, entry := range entries { + row := ListViewRow{Entry: entry} + if len(selectors) > 0 { + row.Fields = k.resolveRowFields(ctx, entry, selectors) + } + rows = append(rows, row) + } + + return &ListViewResult{ + Query: strings.TrimSpace(opts.Query), + Rows: rows, + Tags: listing.Tags, + TotalMatches: total, + IndexedCount: listing.IndexedCount, + NodeCount: listing.NodeCount, + }, nil +} + +// resolveRowFields resolves one row's selectors best-effort. A node that is +// indexed but unreadable yields empty values rather than failing the listing: +// listings render from an index that is allowed to drift from the repository. +func (k *LocalKeg) resolveRowFields(ctx context.Context, entry NodeIndexEntry, selectors []FieldSelector) map[string]string { + var meta *NodeMeta + var stats *NodeStats + needMeta, needStats := SelectorNeeds(selectors) + + if needMeta || needStats { + if id, err := ParseNode(entry.ID); err == nil && id != nil { + if needMeta { + meta, _ = k.getMeta(ctx, *id) + } + if needStats { + stats, _ = k.getStats(ctx, *id) + } + } + } + + out := make(map[string]string, len(selectors)) + for _, sel := range selectors { + out[sel.Text] = FieldValue(sel, entry, meta, stats) + } + return out +} + +// sortEntriesBySelector orders entries in place. Intrinsic and index-timestamp +// selectors sort from values already in memory; anything else resolves a key +// per node first, which is why callers sort before paging. +func (k *LocalKeg) sortEntriesBySelector(ctx context.Context, entries []NodeIndexEntry, sel FieldSelector) error { + if sel.NeedsMeta() || sel.NeedsStats() { + keys := make(map[string]string, len(entries)) + for _, entry := range entries { + fields := k.resolveRowFields(ctx, entry, []FieldSelector{sel}) + keys[entry.ID] = fields[sel.Text] + } + slices.SortStableFunc(entries, func(a, b NodeIndexEntry) int { + return strings.Compare(keys[a.ID], keys[b.ID]) + }) + return nil + } + + switch sel.Kind { + case FieldID: + sortNodeIndexEntriesByID(entries) + case FieldTitle: + slices.SortStableFunc(entries, func(a, b NodeIndexEntry) int { + return strings.Compare(strings.ToLower(a.Title), strings.ToLower(b.Title)) + }) + case FieldIndexTime: + slices.SortStableFunc(entries, func(a, b NodeIndexEntry) int { + return entryTimeField(a, sel.Key).Compare(entryTimeField(b, sel.Key)) + }) + } + return nil +} + func (k *LocalKeg) listEntries(ctx context.Context, opts ListEntriesOptions) (*ListEntriesResult, error) { dex, err := k.Dex(ctx) if err != nil { diff --git a/pkg/keg/keg_iface.go b/pkg/keg/keg_iface.go index fbf5967..022ebaf 100644 --- a/pkg/keg/keg_iface.go +++ b/pkg/keg/keg_iface.go @@ -162,6 +162,14 @@ type Keg interface { // entries in dex order without changing the aggregate counts. ListEntries(ctx context.Context, opts ListEntriesOptions) (*ListEntriesResult, error) + // ListView returns one fully resolved listing page: filtered by the query, + // ordered, paged, and projected onto the requested field selectors. The + // server owns the whole projection so a caller displaying metadata does not + // read each node individually. Field resolution is best-effort: a node whose + // metadata or stats cannot be read yields empty values rather than failing + // the listing, because listings render from an index that may be stale. + ListView(ctx context.Context, opts ListViewOptions) (*ListViewResult, error) + // RelatedNodes returns the deduplicated union of links or backlinks for the // supplied nodes, ordered by node id. It fails if no ids are supplied, an id // is missing, or the direction is invalid. diff --git a/pkg/keg/keg_remote_aggregate.go b/pkg/keg/keg_remote_aggregate.go index 800f806..3507154 100644 --- a/pkg/keg/keg_remote_aggregate.go +++ b/pkg/keg/keg_remote_aggregate.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "net/http" ) @@ -16,6 +17,24 @@ func (k *RemoteKeg) ListEntries(ctx context.Context, opts ListEntriesOptions) (* return &out, nil } +// ErrListViewUnsupported reports that the hub predates the server-resolved +// listing endpoint. Callers degrade to assembling the listing client-side. +var ErrListViewUnsupported = errors.New("hub list view API is unavailable") + +// ListView resolves a whole listing page in one request. A hub that does not +// implement the route answers 404, which is reported as +// ErrListViewUnsupported so the caller can fall back rather than fail. +func (k *RemoteKeg) ListView(ctx context.Context, opts ListViewOptions) (*ListViewResult, error) { + var out ListViewResult + if err := k.postJSON(ctx, "/list/view", "ListView", opts, &out, http.StatusOK); err != nil { + if _, status := RemoteErrorCode(err); status == http.StatusNotFound { + return nil, fmt.Errorf("%w: %w", ErrListViewUnsupported, err) + } + return nil, err + } + return &out, nil +} + type remoteNodeView struct { ID string `json:"id"` Content string `json:"content"` diff --git a/pkg/keg/keg_remote_listview_test.go b/pkg/keg/keg_remote_listview_test.go new file mode 100644 index 0000000..1674120 --- /dev/null +++ b/pkg/keg/keg_remote_listview_test.go @@ -0,0 +1,122 @@ +package keg_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + kegpkg "github.com/jlrickert/tapper/pkg/keg" +) + +// pathRecorder records the path of every request it serves, so a test can +// assert how many round trips an operation costs. +type pathRecorder struct { + mu sync.Mutex + paths []string + handle func(w http.ResponseWriter, r *http.Request) + srv *httptest.Server +} + +func newPathRecorder(t *testing.T, handle func(w http.ResponseWriter, r *http.Request)) *pathRecorder { + t.Helper() + rec := &pathRecorder{handle: handle} + rec.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rec.mu.Lock() + rec.paths = append(rec.paths, r.URL.Path) + rec.mu.Unlock() + rec.handle(w, r) + })) + t.Cleanup(rec.srv.Close) + return rec +} + +func (rec *pathRecorder) recorded() []string { + rec.mu.Lock() + defer rec.mu.Unlock() + return append([]string(nil), rec.paths...) +} + +func newRecorderKeg(t *testing.T, rec *pathRecorder) *kegpkg.RemoteKeg { + t.Helper() + fx := NewSandbox(t) + return kegpkg.NewRemoteKeg(rec.srv.URL, "token", fx.Runtime()) +} + +// TestRemoteListViewIsOneRoundTrip is a regression guard for an N+1 that +// shipped once already: rendering a metadata column used to call GetMeta per +// node, so a 610-node listing made 610 sequential HTTP requests and took ~49s +// against a real hub. The whole point of ListView is that the server resolves +// the page, so the cost must not scale with the number of rows. +func TestRemoteListViewIsOneRoundTrip(t *testing.T) { + t.Parallel() + + const nodeCount = 200 + rows := make([]kegpkg.ListViewRow, 0, nodeCount) + for i := range nodeCount { + rows = append(rows, kegpkg.ListViewRow{ + Entry: kegpkg.NodeIndexEntry{ID: itoa(i), Title: "Node"}, + Fields: map[string]string{"type": "note"}, + }) + } + + rec := newPathRecorder(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(kegpkg.ListViewResult{ + Rows: rows, + TotalMatches: nodeCount, + IndexedCount: nodeCount, + NodeCount: nodeCount, + }) + }) + + k := newRecorderKeg(t, rec) + out, err := k.ListView(context.Background(), kegpkg.ListViewOptions{Fields: []string{"type"}}) + require.NoError(t, err) + require.Len(t, out.Rows, nodeCount) + require.Equal(t, "note", out.Rows[0].Fields["type"]) + + paths := rec.recorded() + require.Len(t, paths, 1, "ListView must cost exactly one request regardless of row count, got %v", paths) + require.Equal(t, "/list/view", paths[0]) + + // No per-node reads may leak in behind the projection. + for _, path := range paths { + require.NotContains(t, path, "/meta") + require.NotContains(t, path, "/stats") + } +} + +// TestRemoteListViewUnsupportedIsDetectable proves a hub that predates the +// route is reported distinctly, so callers degrade to assembling the listing +// themselves instead of surfacing a bare 404. +func TestRemoteListViewUnsupportedIsDetectable(t *testing.T) { + t.Parallel() + + rec := newPathRecorder(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + + k := newRecorderKeg(t, rec) + _, err := k.ListView(context.Background(), kegpkg.ListViewOptions{Fields: []string{"type"}}) + require.Error(t, err) + require.ErrorIs(t, err, kegpkg.ErrListViewUnsupported) +} + +func itoa(i int) string { + if i == 0 { + return "0" + } + var buf [8]byte + pos := len(buf) + for i > 0 { + pos-- + buf[pos] = byte('0' + i%10) + i /= 10 + } + return string(buf[pos:]) +} diff --git a/pkg/tapper/tap_list.go b/pkg/tapper/tap_list.go index 4baa6e8..f6d909c 100644 --- a/pkg/tapper/tap_list.go +++ b/pkg/tapper/tap_list.go @@ -178,6 +178,75 @@ func (t *Tap) List(ctx context.Context, opts ListOptions) ([]string, error) { if err != nil { return []string{}, fmt.Errorf("unable to open keg: %w", err) } + + sortSelector, err := listSortSelector(opts.Sort) + if err != nil { + return []string{}, err + } + compiled, err := compileListFormat(opts.Format) + if err != nil { + return []string{}, err + } + + // Ask the server for the finished page. It filters, orders, pages, and + // resolves the requested fields in one round trip, so displaying metadata + // costs the same as displaying a title. + view, err := k.ListView(ctx, keg.ListViewOptions{ + Query: opts.Query, + Fields: compiled.selectorTexts(opts.IdOnly), + Sort: sortSelector, + Limit: opts.Limit, + Offset: opts.Offset, + }) + switch { + case err == nil: + t.warnStaleIndex(view.IndexedCount, view.NodeCount) + return renderListView(compiled, view.Rows, renderOptions{ + Format: opts.Format, IdOnly: opts.IdOnly, Reverse: opts.Reverse, + }), nil + case errors.Is(err, keg.ErrListViewUnsupported): + // Hub predates the endpoint; assemble the listing here instead. + case strings.TrimSpace(opts.Query) != "": + return []string{}, fmt.Errorf("invalid query expression: %w", err) + default: + return []string{}, fmt.Errorf("unable to list keg: %w", err) + } + + return t.listClientSide(ctx, k, opts, compiled) +} + +// listSortSelector maps the CLI sort names onto field selectors. +func listSortSelector(sort ListSortType) (string, error) { + switch sort { + case SortByDefault, SortByID: + return "id", nil + case SortByUpdated: + return ".updated", nil + case SortByCreated: + return ".created", nil + case SortByAccessed: + return ".accessed", nil + } + return "", fmt.Errorf("unknown sort type: %q", sort) +} + +func (t *Tap) warnStaleIndex(indexed, total int) { + gap := total - indexed + threshold := max(total/10, 5) + if gap >= threshold { + t.Runtime.Logger().Warn( + "index appears stale: run `tap index rebuild` to fix", + "indexed", indexed, + "on_disk", total, + "missing", gap, + ) + } +} + +// listClientSide reproduces the listing locally for hubs that predate the +// server-resolved endpoint. It is strictly slower — field values cost a read +// per row — so it exists only to keep older deployments working. +func (t *Tap) listClientSide(ctx context.Context, k keg.Keg, opts ListOptions, compiled compiledFormat) ([]string, error) { listing, err := k.ListEntries(ctx, keg.ListEntriesOptions{Query: opts.Query}) if err != nil { if strings.TrimSpace(opts.Query) != "" { @@ -187,25 +256,7 @@ func (t *Tap) List(ctx context.Context, opts ListOptions) ([]string, error) { } entries := listing.Entries - - // Warn when the index appears significantly stale compared to on-disk nodes. - { - indexed := listing.IndexedCount - total := listing.NodeCount - gap := total - indexed - threshold := total / 10 // 10% - if threshold < 5 { - threshold = 5 - } - if gap >= threshold { - t.Runtime.Logger().Warn( - "index appears stale: run `tap index rebuild` to fix", - "indexed", indexed, - "on_disk", total, - "missing", gap, - ) - } - } + t.warnStaleIndex(listing.IndexedCount, listing.NodeCount) if strings.TrimSpace(opts.Query) != "" { sortNodeIndexEntries(entries) @@ -220,8 +271,6 @@ func (t *Tap) List(ctx context.Context, opts ListOptions) ([]string, error) { sortNodeIndexEntriesByTime(entries, func(e keg.NodeIndexEntry) time.Time { return e.Created }) case SortByAccessed: sortNodeIndexEntriesByTime(entries, func(e keg.NodeIndexEntry) time.Time { return e.Accessed }) - default: - return []string{}, fmt.Errorf("unknown sort type: %q", opts.Sort) } entries = applyOffset(entries, opts.Offset) diff --git a/pkg/tapper/tap_list_format.go b/pkg/tapper/tap_list_format.go index 1914dd5..459add0 100644 --- a/pkg/tapper/tap_list_format.go +++ b/pkg/tapper/tap_list_format.go @@ -3,7 +3,6 @@ package tapper import ( "fmt" "strings" - "time" "github.com/jlrickert/tapper/pkg/keg" ) @@ -120,56 +119,75 @@ func compileListFormat(format string) (compiledFormat, error) { return out, nil } -// nodeFieldSource carries everything needed to expand a compiled format for -// one node. meta and stats are nil unless the format required them. -type nodeFieldSource struct { - entry keg.NodeIndexEntry - meta *keg.NodeMeta - stats *keg.NodeStats +// selectorTexts returns the distinct field selectors this format needs the +// server to resolve. Intrinsics and index timestamps are omitted: they come +// from the index entry that every listing already carries, so asking for them +// would make the server do work the client can do for free. +func (c compiledFormat) selectorTexts(idOnly bool) []string { + if idOnly { + return nil + } + seen := make(map[string]struct{}, len(c.segments)) + out := make([]string, 0, len(c.segments)) + for _, seg := range c.segments { + if !seg.isField { + continue + } + if !seg.sel.NeedsMeta() && !seg.sel.NeedsStats() { + continue + } + if _, dup := seen[seg.sel.Text]; dup { + continue + } + seen[seg.sel.Text] = struct{}{} + out = append(out, seg.sel.Text) + } + return out } -// fieldValue resolves one selector against a node. -// -// Intrinsics and index timestamps come from the index entry, never from -// stats.json, so a displayed value always agrees with the same predicate in a -// query expression and the default format stays free of per-node reads. -func fieldValue(sel keg.FieldSelector, src nodeFieldSource) string { - switch sel.Kind { - case keg.FieldID: - return src.entry.ID - case keg.FieldTitle: - return src.entry.Title - case keg.FieldIndexTime: - switch sel.Key { - case "updated": - return formatEntryTime(src.entry.Updated) - case "created": - return formatEntryTime(src.entry.Created) - case "accessed": - return formatEntryTime(src.entry.Accessed) - } - return "" - case keg.FieldStat: - value, _ := keg.StatsFieldValue(src.stats, sel.Key) - return value - case keg.FieldTags, keg.FieldMetaKey: - if src.meta == nil { - return "" +// renderListView formats rows the server already resolved. No I/O happens +// here: every field value the format names is either on the index entry or in +// the row's resolved map. +func renderListView(compiled compiledFormat, rows []keg.ListViewRow, opts renderOptions) []string { + lines := make([]string, 0, len(rows)) + start, end, step := iterationBounds(len(rows), opts.Reverse) + for i := start; i != end; i += step { + if opts.IdOnly { + lines = append(lines, rows[i].Entry.ID) + continue } - // An absent key, a non-scalar value, and an empty tag list all - // collapse to empty here. That keeps a tabular format's column count - // stable regardless of which nodes carry the key. - value, _ := src.meta.Get(sel.Key) - return value + lines = append(lines, expandFormat(compiled, nodeFieldSource{ + entry: rows[i].Entry, + resolved: rows[i].Fields, + })) } - return "" + return lines +} + +// nodeFieldSource carries everything needed to expand a compiled format for +// one node. +// +// Values resolved by the server arrive already rendered in `resolved`, keyed by +// selector text. meta and stats are only populated on the fallback path, where +// the client had to read them itself. +type nodeFieldSource struct { + entry keg.NodeIndexEntry + resolved map[string]string + meta *keg.NodeMeta + stats *keg.NodeStats } -func formatEntryTime(t time.Time) string { - if t.IsZero() { - return "" +// fieldValue resolves one selector against a node, preferring a value the +// server already resolved. Resolution itself lives in pkg/keg so the client +// renderer and the server-side projection cannot disagree about what a +// selector means. +func fieldValue(sel keg.FieldSelector, src nodeFieldSource) string { + if src.resolved != nil { + if value, ok := src.resolved[sel.Text]; ok { + return value + } } - return t.Format(time.RFC3339) + return keg.FieldValue(sel, src.entry, src.meta, src.stats) } // expandFormat renders one line for a node. From b3a5daed0a20e50dfa98cd06b087023bfa5303cb Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 30 Jul 2026 00:30:29 -0500 Subject: [PATCH 4/7] feat(tapper): interpret backslash escapes in listing formats The help has always documented the default format as "%i\t%d\t%t", but a shell does not expand \t inside double quotes, so the obvious command produced a literal backslash and a t between columns. Getting tabs required $'...' quoting, which is not discoverable from the help text that suggests \t in the first place. Interpret \t, \n, \r, and \\ in the format string. An unrecognised escape passes through untouched, mirroring the existing rule for unrecognised percent verbs, so a Windows-style path in a template still renders. A real tab arriving from $'...' quoting or a script is unaffected. --- docs/output-formats.md | 21 ++++++++++++++ pkg/cli/format_completion.go | 4 +++ pkg/keg/format_fields.go | 2 +- pkg/mcp/tools_read.go | 10 +++---- pkg/tapper/tap_list_format.go | 29 +++++++++++++++++++ pkg/tapper/tap_list_format_test.go | 45 ++++++++++++++++++++++++++++++ 6 files changed, 105 insertions(+), 6 deletions(-) diff --git a/docs/output-formats.md b/docs/output-formats.md index 756baa6..cf7d9fc 100644 --- a/docs/output-formats.md +++ b/docs/output-formats.md @@ -60,6 +60,27 @@ because a single letter cannot address an arbitrary metadata key. An unrecognised `%X` passes through as literal text, so a format containing a bare percent keeps working. +### Escapes + +A shell does not expand `\t` inside double quotes, so `tap` interprets backslash +escapes itself. This is what makes the tab-separated default typeable at a +prompt: + +```sh +tap list -f "%{id}\t%{type}\t%{title}" # real tabs +``` + +| Escape | Renders | +| --- | --- | +| `\t` | tab | +| `\n` | newline | +| `\r` | carriage return | +| `\\` | a literal backslash | + +An unrecognised `\X` passes through untouched, the same rule as `%X`, so a +Windows-style path in a template survives. A real tab — from `$'...'` quoting or +a script — passes through unchanged. + ## Absent values An absent value renders as the empty string rather than a placeholder, so a diff --git a/pkg/cli/format_completion.go b/pkg/cli/format_completion.go index 9bd1967..5868624 100644 --- a/pkg/cli/format_completion.go +++ b/pkg/cli/format_completion.go @@ -26,6 +26,10 @@ Named selectors use %{...} and share the query expression vocabulary: %{.accessCount} a statistics field: updated, created, accessed, hash, accessCount, lead, omega +Escapes: \t tab, \n newline, \r return, \\ backslash. These are +interpreted by tap, so "%i\t%t" produces tabs inside ordinary shell +quotes without needing $'...'. + Selectors other than id, title, and the three dates read one file per node. Absent values render empty.` diff --git a/pkg/keg/format_fields.go b/pkg/keg/format_fields.go index 6eb0cbe..e394dd4 100644 --- a/pkg/keg/format_fields.go +++ b/pkg/keg/format_fields.go @@ -89,7 +89,7 @@ var LegacyFormatVerbs = map[byte]string{ // FormatVocabularyDescription is the one-line summary of the listing field // vocabulary. It is duplicated as a literal in the MCP tool schemas, which // require a constant struct tag; a test holds the two in agreement. -const FormatVocabularyDescription = "output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node." +const FormatVocabularyDescription = "output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Backslash escapes are interpreted: \\t tab, \\n newline, \\r return, \\\\ backslash. Selectors other than id, title, and the three dates read one file per node." func isIndexTimeField(name string) bool { return slices.Contains(IndexTimeFieldNames, name) diff --git a/pkg/mcp/tools_read.go b/pkg/mcp/tools_read.go index 493e4f7..e670367 100644 --- a/pkg/mcp/tools_read.go +++ b/pkg/mcp/tools_read.go @@ -62,7 +62,7 @@ func registerCat(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type listInput struct { Query string `json:"query,omitempty" jsonschema:"boolean query expression to filter nodes. Supports tags ('golang'), key=value attributes ('entity=plan'), and dot-prefix stats fields ('.created>2026-01-01', '.accessCount>=5', '.hash=abc123'). Combine with 'and', 'or', 'not'."` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Backslash escapes are interpreted: \\t tab, \\n newline, \\r return, \\\\ backslash. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Sort string `json:"sort,omitempty" jsonschema:"sort order: 'id' (default), 'updated', 'created', or 'accessed'"` @@ -102,7 +102,7 @@ func registerList(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type grepInput struct { Query string `json:"query" jsonschema:"regex pattern to search node content"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node. Use id_only for compact MCP output."` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Backslash escapes are interpreted: \\t tab, \\n newline, \\r return, \\\\ backslash. Selectors other than id, title, and the three dates read one file per node. Use id_only for compact MCP output."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only (recommended for MCP to reduce token usage)"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` IgnoreCase bool `json:"ignore_case,omitempty" jsonschema:"case-insensitive matching"` @@ -144,7 +144,7 @@ func registerGrep(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type tagsInput struct { Query string `json:"query,omitempty" jsonschema:"boolean expression to filter by tags, attributes, and dot-prefix stats fields (e.g. '.created>2026-01-01 and entity=plan')"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Backslash escapes are interpreted: \\t tab, \\n newline, \\r return, \\\\ backslash. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Limit int `json:"limit,omitempty" jsonschema:"maximum results to return (default: 50, 0 in request means use default, -1 for unlimited)"` @@ -182,7 +182,7 @@ func registerTags(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults) { type backlinksInput struct { NodeIDs []string `json:"node_ids" jsonschema:"target node IDs to find incoming links for (results merged and deduplicated)"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Backslash escapes are interpreted: \\t tab, \\n newline, \\r return, \\\\ backslash. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Limit int `json:"limit,omitempty" jsonschema:"maximum results to return (default: 50, 0 in request means use default, -1 for unlimited)"` @@ -220,7 +220,7 @@ func registerBacklinks(srv *sdkmcp.Server, tap *tapper.Tap, defaults KegDefaults type linksInput struct { NodeIDs []string `json:"node_ids" jsonschema:"source node IDs to find outgoing links for (results merged and deduplicated)"` Keg string `json:"keg,omitempty" jsonschema:"keg alias (uses default if empty)"` - Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Selectors other than id, title, and the three dates read one file per node."` + Format string `json:"format,omitempty" jsonschema:"output format template. Legacy verbs %i (id), %t (title), %d (updated), %c (created), %a (accessed); %% is a literal percent. Named selectors use %{...}: a bare word names a metadata key such as %{type} or %{status}, a leading dot names a statistics field such as %{.accessCount} or %{.omega}, and %{tags} is the node's tag list. Backslash escapes are interpreted: \\t tab, \\n newline, \\r return, \\\\ backslash. Selectors other than id, title, and the three dates read one file per node."` IdOnly bool `json:"id_only,omitempty" jsonschema:"return node IDs only"` Reverse bool `json:"reverse,omitempty" jsonschema:"reverse output order"` Limit int `json:"limit,omitempty" jsonschema:"maximum results to return (default: 50, 0 in request means use default, -1 for unlimited)"` diff --git a/pkg/tapper/tap_list_format.go b/pkg/tapper/tap_list_format.go index 459add0..8c1313c 100644 --- a/pkg/tapper/tap_list_format.go +++ b/pkg/tapper/tap_list_format.go @@ -11,6 +11,17 @@ import ( // tab-separated so listing output stays machine-readable by default. const defaultListFormat = "%i\t%d\t%t" +// formatEscapes are the backslash escapes a format string may contain. They +// exist because a shell does not expand "\t" inside double quotes, so the +// separator most listings want is otherwise impossible to type without +// resorting to $'...' quoting. +var formatEscapes = map[byte]byte{ + 't': '\t', + 'n': '\n', + 'r': '\r', + '\\': '\\', +} + // formatSegment is one piece of a compiled format: either literal text, or a // field selector to expand per node. type formatSegment struct { @@ -67,6 +78,24 @@ func compileListFormat(format string) (compiledFormat, error) { for i := 0; i < len(format); { c := format[i] + + // Interpret backslash escapes. A shell's double quotes do not expand + // "\t", so without this the documented default format "%i\t%d\t%t" + // cannot be typed at a prompt: it arrives as a literal backslash and + // a t. Unknown escapes pass through untouched, mirroring the rule for + // unknown percent verbs. + if c == '\\' && i+1 < len(format) { + if escaped, ok := formatEscapes[format[i+1]]; ok { + lit.WriteByte(escaped) + i += 2 + continue + } + lit.WriteByte('\\') + lit.WriteByte(format[i+1]) + i += 2 + continue + } + if c != '%' { lit.WriteByte(c) i++ diff --git a/pkg/tapper/tap_list_format_test.go b/pkg/tapper/tap_list_format_test.go index 0ab386e..0f17941 100644 --- a/pkg/tapper/tap_list_format_test.go +++ b/pkg/tapper/tap_list_format_test.go @@ -241,3 +241,48 @@ func TestExpandFormatIntrinsicsShadowMetadata(t *testing.T) { t.Errorf("title = %q, want the intrinsic %q", got, "A Node") } } + +func TestCompileListFormatBackslashEscapes(t *testing.T) { + // A shell does not expand "\t" inside double quotes, so a format typed at + // a prompt arrives with a literal backslash. The documented default is + // tab-separated, so the escape has to be interpreted here or the obvious + // command produces literal "\t" between columns. + src := nodeFieldSource{entry: testEntry()} + tests := map[string]string{ + `%i\t%t`: "3\tA Node", + `%i\n%t`: "3\nA Node", + `%i\r%t`: "3\rA Node", + `a\\b`: `a\b`, + `%i\t\t%t`: "3\t\tA Node", + } + for format, want := range tests { + if got := renderOne(t, format, src); got != want { + t.Errorf("format %q = %q, want %q", format, got, want) + } + } +} + +func TestCompileListFormatUnknownEscapePassesThrough(t *testing.T) { + // Mirrors the rule for unknown percent verbs: pass through untouched so a + // Windows-style path or a stray backslash in a template still renders. + src := nodeFieldSource{entry: testEntry()} + tests := map[string]string{ + `C:\path %i`: `C:\path 3`, + `%i\q`: `3\q`, + `%i\`: `3\`, + } + for format, want := range tests { + if got := renderOne(t, format, src); got != want { + t.Errorf("format %q = %q, want %q", format, got, want) + } + } +} + +func TestCompileListFormatRealTabIsUnaffected(t *testing.T) { + // $'...' quoting and Go string literals deliver a real tab; it must pass + // through unchanged rather than being double-processed. + src := nodeFieldSource{entry: testEntry()} + if got := renderOne(t, "%i\t%t", src); got != "3\tA Node" { + t.Errorf("real tab = %q, want %q", got, "3\tA Node") + } +} From 75f51a3586390562dfe4ce409ee3cf213456464b Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 30 Jul 2026 00:54:01 -0500 Subject: [PATCH 5/7] feat(keg): default listing fields per keg A keg whose nodes are distinguished by type and subkind had no way to say so: every caller had to pass --format, and the hosted node list hardcoded its own columns, so the two surfaces disagreed about what a keg looks like. Add listFields to the keg config. It uses the same selector vocabulary as --format and query expressions, and resolves as flag, then keg config, then the built-in default. One setting will drive both `tap list` and the Hub node list. Selectors are validated when the config is parsed rather than when a listing renders, so a typo is reported while editing instead of showing a silently blank column later. It lives on the config rather than on an index entry because normalizeIndexes rebuilds the index list from SystemIndexEntries on every parse, which carries only file and summary -- a field declared on the default nodes.tsv index would be discarded on the next read. --- docs/output-formats.md | 18 +++++++++++++ pkg/cli/cmd_list_test.go | 35 +++++++++++++++++++++++++ pkg/keg/format_fields_test.go | 49 +++++++++++++++++++++++++++++++++++ pkg/keg/keg_config.go | 27 +++++++++++++++++++ pkg/tapper/tap_list.go | 37 +++++++++++++++++++++++++- schemas/keg-config.json | 7 +++++ 6 files changed, 172 insertions(+), 1 deletion(-) diff --git a/docs/output-formats.md b/docs/output-formats.md index cf7d9fc..f5fa3aa 100644 --- a/docs/output-formats.md +++ b/docs/output-formats.md @@ -81,6 +81,24 @@ An unrecognised `\X` passes through untouched, the same rule as `%X`, so a Windows-style path in a template survives. A real tab — from `$'...'` quoting or a script — passes through unchanged. +## Per-keg defaults + +A keg can declare the columns its listings should show, so a keg whose nodes are +distinguished by `type` and `subkind` displays them without every caller passing +`--format`: + +```yaml +# keg +kegv: 2025-07 +listFields: [id, type, subkind, title] +``` + +The same setting drives the node list in Tapper Hub, so a keg looks the same on +both surfaces. Resolution order is `--format` → `listFields` → the built-in +default. Entries use the selector vocabulary above and are validated when the +config is saved, so a typo is reported at that point rather than rendering a +silently blank column. + ## Absent values An absent value renders as the empty string rather than a placeholder, so a diff --git a/pkg/cli/cmd_list_test.go b/pkg/cli/cmd_list_test.go index 1a406e0..9a41d17 100644 --- a/pkg/cli/cmd_list_test.go +++ b/pkg/cli/cmd_list_test.go @@ -624,3 +624,38 @@ func TestListCommand_FormatCompletionSuggestsSelectors(t *testing.T) { require.Contains(t, suggestions, "%{tags}") require.Contains(t, suggestions, "%{.accessCount}") } + +func TestListCommand_KegConfigListFieldsDrivesDefault(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("queryuser", "~")) + + // A keg whose nodes are distinguished by entity and status should show + // those columns without every caller having to pass --format. + sb.MustWriteFile("~/kegs/@local/query/keg", []byte( + "kegv: 2025-07\ntitle: Query\nlistFields:\n - id\n - entity\n - status\n - title\n"), 0o644) + + res := NewProcess(t, false, "list", "--keg", "query").Run(sb.Context(), sb.Runtime()) + require.NoError(t, res.Err) + out := strings.TrimSpace(string(res.Stdout)) + require.NotEmpty(t, out) + + // Node 9 carries entity=task, status=done. + require.Contains(t, out, "9\ttask\tdone") + + // The default format is not in play any more, so no RFC3339 timestamp. + require.NotContains(t, out, "T00:00:00Z") +} + +func TestListCommand_ExplicitFormatBeatsKegConfig(t *testing.T) { + t.Parallel() + sb := NewSandbox(t, testutils.WithFixture("queryuser", "~")) + + sb.MustWriteFile("~/kegs/@local/query/keg", []byte( + "kegv: 2025-07\ntitle: Query\nlistFields:\n - id\n - entity\n"), 0o644) + + res := NewProcess(t, false, "list", "--keg", "query", "-f", "%{id}|%{title}").Run(sb.Context(), sb.Runtime()) + require.NoError(t, res.Err) + out := strings.TrimSpace(string(res.Stdout)) + require.Contains(t, out, "|") + require.NotContains(t, out, "\t", "an explicit --format must win over the keg's listFields") +} diff --git a/pkg/keg/format_fields_test.go b/pkg/keg/format_fields_test.go index d0e3487..057f9e6 100644 --- a/pkg/keg/format_fields_test.go +++ b/pkg/keg/format_fields_test.go @@ -1,6 +1,7 @@ package keg_test import ( + "strings" "testing" "time" @@ -241,3 +242,51 @@ func TestFormatSelectorSuggestions(t *testing.T) { } } } + +func TestConfigListFieldsRoundTrip(t *testing.T) { + raw := []byte("kegv: 2025-07\nlistFields:\n - id\n - type\n - subkind\n - title\n") + cfg, err := keg.ParseKegConfig(raw) + if err != nil { + t.Fatalf("ParseKegConfig: %v", err) + } + want := []string{"id", "type", "subkind", "title"} + if len(cfg.ListFields) != len(want) { + t.Fatalf("ListFields = %v, want %v", cfg.ListFields, want) + } + for i, field := range want { + if cfg.ListFields[i] != field { + t.Errorf("ListFields[%d] = %q, want %q", i, cfg.ListFields[i], field) + } + } + + // The setting must survive a serialize/parse cycle or editing any other + // field through the settings form would silently drop it. + out, err := cfg.ToYAML() + if err != nil { + t.Fatalf("ToYAML: %v", err) + } + again, err := keg.ParseKegConfig(out) + if err != nil { + t.Fatalf("reparse: %v", err) + } + if len(again.ListFields) != len(want) { + t.Errorf("after round trip ListFields = %v, want %v", again.ListFields, want) + } +} + +func TestConfigListFieldsRejectsBadSelector(t *testing.T) { + // Rejecting at parse time means a typo surfaces when the config is saved + // rather than as a silently blank column at render time. + raw := []byte("kegv: 2025-07\nlistFields:\n - type\n - .bogus\n") + if _, err := keg.ParseKegConfig(raw); err == nil { + t.Fatal("ParseKegConfig accepted an unknown stats selector, want error") + } else if !strings.Contains(err.Error(), "listFields") { + t.Errorf("error = %q, want it to name the offending field", err) + } +} + +func TestConfigListFieldsEmptyIsValid(t *testing.T) { + if _, err := keg.ParseKegConfig([]byte("kegv: 2025-07\n")); err != nil { + t.Fatalf("config without listFields should parse: %v", err) + } +} diff --git a/pkg/keg/keg_config.go b/pkg/keg/keg_config.go index bb83b9c..924075b 100644 --- a/pkg/keg/keg_config.go +++ b/pkg/keg/keg_config.go @@ -85,6 +85,14 @@ type ConfigV2 struct { // Indexes is a list of index entries that link to related files or nodes. Indexes []IndexEntry `yaml:"indexes,omitempty" json:"indexes,omitempty"` + // ListFields are the field selectors a node listing shows by default, in + // the vocabulary of ParseFieldSelector: a bare word names a metadata key + // ("type", "subkind"), a leading dot names a statistics field (".omega"), + // and "id", "title", and "tags" are reserved. One setting drives both the + // default `tap list` format and the columns of the hosted node list, so a + // keg presents the same shape everywhere. Empty means the built-in default. + ListFields []string `yaml:"listFields,omitempty" json:"list_fields,omitempty"` + // Timezone is the IANA timezone for resolving ambiguous timestamps // within this keg (e.g. "America/Chicago"). Defaults to "UTC". Timezone string `yaml:"timezone,omitempty" json:"timezone,omitempty"` @@ -289,6 +297,9 @@ func parseKegConfig(data []byte, strict bool) (*Config, error) { if err := cfg.validateSnapshots(); err != nil { return cfg, err } + if err := cfg.validateListFields(); err != nil { + return cfg, err + } if err := cfg.normalizeIndexes(strict); err != nil { return cfg, err } @@ -305,6 +316,9 @@ func parseKegConfig(data []byte, strict bool) (*Config, error) { if err := configV2.validateSnapshots(); err != nil { return &configV2, err } + if err := configV2.validateListFields(); err != nil { + return &configV2, err + } if err := configV2.normalizeIndexes(strict); err != nil { return &configV2, err } @@ -340,6 +354,19 @@ func (kc *ConfigV2) validateSnapshots() error { return err } +// validateListFields rejects an unusable selector when the config is parsed +// rather than when a listing is rendered, so a typo surfaces at the point of +// editing instead of silently blanking a column later. +func (kc *ConfigV2) validateListFields() error { + if kc == nil || len(kc.ListFields) == 0 { + return nil + } + if _, err := ParseFieldSelectors(kc.ListFields); err != nil { + return fmt.Errorf("listFields: %w", err) + } + return nil +} + func (kc *ConfigV2) normalizeIndexes(strict bool) error { if kc == nil { return nil diff --git a/pkg/tapper/tap_list.go b/pkg/tapper/tap_list.go index f6d909c..ef73b51 100644 --- a/pkg/tapper/tap_list.go +++ b/pkg/tapper/tap_list.go @@ -183,7 +183,7 @@ func (t *Tap) List(ctx context.Context, opts ListOptions) ([]string, error) { if err != nil { return []string{}, err } - compiled, err := compileListFormat(opts.Format) + compiled, err := compileListFormat(t.resolveListFormat(ctx, k, opts.Format)) if err != nil { return []string{}, err } @@ -215,6 +215,41 @@ func (t *Tap) List(ctx context.Context, opts ListOptions) ([]string, error) { return t.listClientSide(ctx, k, opts, compiled) } +// resolveListFormat picks the format for a listing: an explicit --format wins, +// then the keg's own listFields, then the built-in default. +// +// Reading the keg's preference means a keg whose nodes are distinguished by +// type or subkind shows those columns without every caller having to know it. +// The lookup is best-effort — a keg with no config, or an unreadable one, falls +// through to the default rather than failing the listing. +func (t *Tap) resolveListFormat(ctx context.Context, k keg.Keg, explicit string) string { + if strings.TrimSpace(explicit) != "" { + return explicit + } + cfg, err := k.Config(ctx) + if err != nil || cfg == nil || len(cfg.ListFields) == 0 { + return explicit + } + return formatFromFieldSelectors(cfg.ListFields) +} + +// formatFromFieldSelectors renders a selector list as a tab-separated format +// string, so keg configuration and --format share one language. +func formatFromFieldSelectors(fields []string) string { + parts := make([]string, 0, len(fields)) + for _, field := range fields { + field = strings.TrimSpace(field) + if field == "" { + continue + } + parts = append(parts, "%{"+field+"}") + } + if len(parts) == 0 { + return "" + } + return strings.Join(parts, "\t") +} + // listSortSelector maps the CLI sort names onto field selectors. func listSortSelector(sort ListSortType) (string, error) { switch sort { diff --git a/schemas/keg-config.json b/schemas/keg-config.json index 9753735..aec2e11 100644 --- a/schemas/keg-config.json +++ b/schemas/keg-config.json @@ -59,6 +59,13 @@ "additionalProperties": true } }, + "listFields": { + "type": "array", + "description": "Field selectors shown by default in node listings, used by both `tap list` and the hosted node list. A bare word names a metadata key (\"type\", \"subkind\"); a leading dot names a statistics field (\".omega\"); \"id\", \"title\", and \"tags\" are reserved.", + "items": { + "type": "string" + } + }, "indexes": { "type": "array", "description": "Configured index outputs and optional tag filters.", From c6ce1605fa3a448fdccec1ca11abf5b27035fbcf Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 30 Jul 2026 00:59:41 -0500 Subject: [PATCH 6/7] feat(keg): read listing metadata in batches Showing or sorting by a metadata column read one node at a time. A listing that sorts needs a key for every matching node, not just the returned page, so on a 610-node keg that was 610 reads -- and on the hosted backend roughly 1220 queries, because each read also checks existence first. Add RepositoryBatchRead as an optional capability, detected by type assertion in the same way SupportsConcurrentAccess already is. A backend that can answer a whole set at once implements it; a plain filesystem repository does not and keeps reading node by node, so local kegs behave exactly as before. listView now loads values for the page as a whole after paging, and the sort path loads keys for the matched set in one go. A metadata listing costs one batch; sorting then projecting costs two, regardless of keg size. Listings naming only intrinsics and index timestamps still read nothing at all. Reads stay best-effort: a node the index names but the repository cannot produce contributes empty values rather than failing the listing. --- pkg/keg/keg_aggregate.go | 116 +++++++++++++++--- pkg/keg/keg_listview_batch_test.go | 185 +++++++++++++++++++++++++++++ pkg/keg/repository.go | 28 +++++ 3 files changed, 313 insertions(+), 16 deletions(-) create mode 100644 pkg/keg/keg_listview_batch_test.go diff --git a/pkg/keg/keg_aggregate.go b/pkg/keg/keg_aggregate.go index 13e0941..ddf8efc 100644 --- a/pkg/keg/keg_aggregate.go +++ b/pkg/keg/keg_aggregate.go @@ -252,10 +252,13 @@ func (k *LocalKeg) listView(ctx context.Context, opts ListViewOptions) (*ListVie } rows := make([]ListViewRow, 0, len(entries)) + // Values are loaded for the page as a whole, after paging, so a listing + // that displays metadata reads one batch rather than one node at a time. + values := k.loadFieldValues(ctx, entries, selectors) for _, entry := range entries { row := ListViewRow{Entry: entry} if len(selectors) > 0 { - row.Fields = k.resolveRowFields(ctx, entry, selectors) + row.Fields = resolveRowFields(entry, selectors, values) } rows = append(rows, row) } @@ -270,23 +273,102 @@ func (k *LocalKeg) listView(ctx context.Context, opts ListViewOptions) (*ListVie }, nil } -// resolveRowFields resolves one row's selectors best-effort. A node that is -// indexed but unreadable yields empty values rather than failing the listing: -// listings render from an index that is allowed to drift from the repository. -func (k *LocalKeg) resolveRowFields(ctx context.Context, entry NodeIndexEntry, selectors []FieldSelector) map[string]string { - var meta *NodeMeta - var stats *NodeStats +// fieldValues holds the metadata and statistics a set of selectors needs, +// keyed by node id. A missing entry means the node had none, which callers +// render as empty. +type fieldValues struct { + meta map[string]*NodeMeta + stats map[string]*NodeStats +} + +// loadFieldValues fetches whatever the selectors require for every entry, in as +// few repository operations as the backend allows. +// +// This is the difference between a listing that costs two operations and one +// that costs two per row. A backend implementing RepositoryBatchRead answers +// the whole set at once; otherwise each node is read individually, which is the +// only option for a plain filesystem keg. +// +// Reads are best-effort throughout: a node that is indexed but unreadable +// contributes no value rather than failing the listing, because listings render +// from an index that is allowed to drift from the repository. +func (k *LocalKeg) loadFieldValues(ctx context.Context, entries []NodeIndexEntry, selectors []FieldSelector) *fieldValues { + out := &fieldValues{} needMeta, needStats := SelectorNeeds(selectors) + if !needMeta && !needStats || len(entries) == 0 { + return out + } - if needMeta || needStats { + ids := make([]NodeId, 0, len(entries)) + for _, entry := range entries { if id, err := ParseNode(entry.ID); err == nil && id != nil { - if needMeta { - meta, _ = k.getMeta(ctx, *id) + ids = append(ids, *id) + } + } + if len(ids) == 0 { + return out + } + + if batch, ok := repositoryBatchRead(k.Repo); ok { + if needMeta { + out.meta = make(map[string]*NodeMeta, len(ids)) + if raw, err := batch.ReadMetaBatch(ctx, ids); err == nil { + for key, data := range raw { + if meta, perr := ParseMeta(ctx, data); perr == nil { + out.meta[key] = meta + } + } } - if needStats { - stats, _ = k.getStats(ctx, *id) + } + if needStats { + if stats, err := batch.ReadStatsBatch(ctx, ids); err == nil { + out.stats = stats + } else { + out.stats = map[string]*NodeStats{} } } + return out + } + + if needMeta { + out.meta = make(map[string]*NodeMeta, len(ids)) + } + if needStats { + out.stats = make(map[string]*NodeStats, len(ids)) + } + for _, id := range ids { + if needMeta { + if meta, err := k.getMeta(ctx, id); err == nil { + out.meta[id.Path()] = meta + } + } + if needStats { + if stats, err := k.getStats(ctx, id); err == nil { + out.stats[id.Path()] = stats + } + } + } + return out +} + +// nodeKey returns the key an entry's values are stored under, or "" when the +// id cannot be parsed. +func nodeKey(entry NodeIndexEntry) string { + id, err := ParseNode(entry.ID) + if err != nil || id == nil { + return "" + } + return id.Path() +} + +// resolveRowFields renders one row's selectors from already-loaded values. +func resolveRowFields(entry NodeIndexEntry, selectors []FieldSelector, values *fieldValues) map[string]string { + key := nodeKey(entry) + var meta *NodeMeta + var stats *NodeStats + if values != nil && key != "" { + meta = values.meta[key] + stats = values.stats[key] } out := make(map[string]string, len(selectors)) @@ -297,14 +379,16 @@ func (k *LocalKeg) resolveRowFields(ctx context.Context, entry NodeIndexEntry, s } // sortEntriesBySelector orders entries in place. Intrinsic and index-timestamp -// selectors sort from values already in memory; anything else resolves a key -// per node first, which is why callers sort before paging. +// selectors sort from values already in memory; a metadata or statistics +// selector needs a key for every entry, which is why callers sort before paging +// and why the values are loaded in one batch. func (k *LocalKeg) sortEntriesBySelector(ctx context.Context, entries []NodeIndexEntry, sel FieldSelector) error { if sel.NeedsMeta() || sel.NeedsStats() { + selectors := []FieldSelector{sel} + values := k.loadFieldValues(ctx, entries, selectors) keys := make(map[string]string, len(entries)) for _, entry := range entries { - fields := k.resolveRowFields(ctx, entry, []FieldSelector{sel}) - keys[entry.ID] = fields[sel.Text] + keys[entry.ID] = resolveRowFields(entry, selectors, values)[sel.Text] } slices.SortStableFunc(entries, func(a, b NodeIndexEntry) int { return strings.Compare(keys[a.ID], keys[b.ID]) diff --git a/pkg/keg/keg_listview_batch_test.go b/pkg/keg/keg_listview_batch_test.go new file mode 100644 index 0000000..7b9b3f5 --- /dev/null +++ b/pkg/keg/keg_listview_batch_test.go @@ -0,0 +1,185 @@ +package keg_test + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + kegpkg "github.com/jlrickert/tapper/pkg/keg" +) + +// countingRepo wraps a repository and records how many per-node metadata reads +// it serves, plus how many batch reads. It deliberately does NOT implement +// keg.RepositoryBatchRead unless batch is true, so one test fixture can +// exercise both paths. +type countingRepo struct { + kegpkg.Repository + perNode int + batches int +} + +func (r *countingRepo) ReadMeta(ctx context.Context, id kegpkg.NodeId) ([]byte, error) { + r.perNode++ + return r.Repository.ReadMeta(ctx, id) +} + +// batchingRepo adds the batch capability on top of countingRepo. +type batchingRepo struct{ *countingRepo } + +func (r *batchingRepo) ReadMetaBatch(ctx context.Context, ids []kegpkg.NodeId) (map[string][]byte, error) { + r.batches++ + out := make(map[string][]byte, len(ids)) + for _, id := range ids { + // Read through the embedded repository directly so the per-node + // counter stays a measure of what the caller did, not of this helper. + data, err := r.countingRepo.Repository.ReadMeta(ctx, id) + if err != nil { + continue + } + out[id.Path()] = data + } + return out, nil +} + +func (r *batchingRepo) ReadStatsBatch(ctx context.Context, ids []kegpkg.NodeId) (map[string]*kegpkg.NodeStats, error) { + r.batches++ + out := make(map[string]*kegpkg.NodeStats, len(ids)) + for _, id := range ids { + stats, err := r.countingRepo.Repository.ReadStats(ctx, id) + if err != nil { + continue + } + out[id.Path()] = stats + } + return out, nil +} + +// seedNodes creates n nodes carrying a `type` metadata key. +func seedNodes(t *testing.T, k *kegpkg.LocalKeg, n int) { + t.Helper() + ctx := context.Background() + for i := range n { + _, err := k.Create(ctx, &kegpkg.CreateOptions{ + Title: fmt.Sprintf("Node %d", i), + Attrs: map[string]any{"type": fmt.Sprintf("kind%d", i%3)}, + }) + require.NoError(t, err) + } +} + +// TestListViewBatchesMetadataReads is the guard for the cost model that makes +// metadata columns usable. Without batching, showing `type` on a 610-node keg +// meant a read per node -- and on the hosted backend, two queries per node. +func TestListViewBatchesMetadataReads(t *testing.T) { + t.Parallel() + const nodeCount = 12 + + fx := NewSandbox(t) + base := kegpkg.NewMemoryRepo(fx.Runtime()) + counter := &countingRepo{Repository: base} + k := kegpkg.NewLocalKeg(&batchingRepo{countingRepo: counter}, fx.Runtime()) + require.NoError(t, k.Init(context.Background())) + seedNodes(t, k, nodeCount) + + counter.perNode = 0 + counter.batches = 0 + + out, err := k.ListView(context.Background(), kegpkg.ListViewOptions{Fields: []string{"type"}}) + require.NoError(t, err) + // Init seeds node 0, so the keg holds one more node than we created. + require.Len(t, out.Rows, nodeCount+1) + require.Equal(t, "kind0", out.Rows[1].Fields["type"]) + + require.Equal(t, 1, counter.batches, + "projecting a metadata column must issue exactly one batch read, got %d", counter.batches) +} + +// TestListViewSortBatchesMetadataReads covers the more expensive path: sorting +// needs a key for every matching node, not just the returned page, so it is the +// case most likely to regress into a per-node loop. +func TestListViewSortBatchesMetadataReads(t *testing.T) { + t.Parallel() + const nodeCount = 12 + + fx := NewSandbox(t) + base := kegpkg.NewMemoryRepo(fx.Runtime()) + counter := &countingRepo{Repository: base} + k := kegpkg.NewLocalKeg(&batchingRepo{countingRepo: counter}, fx.Runtime()) + require.NoError(t, k.Init(context.Background())) + seedNodes(t, k, nodeCount) + + counter.perNode = 0 + counter.batches = 0 + + out, err := k.ListView(context.Background(), kegpkg.ListViewOptions{ + Fields: []string{"type"}, + Sort: "type", + Limit: 3, + }) + require.NoError(t, err) + require.Len(t, out.Rows, 3) + require.Equal(t, nodeCount+1, out.TotalMatches, "TotalMatches counts before paging") + + // One batch to resolve the sort key across all entries, one to project the + // returned page. Never one per node. + require.Equal(t, 2, counter.batches, + "sorting then projecting must issue two batch reads, got %d", counter.batches) + + // Ascending by type, so the page is non-decreasing. Node 0 carries no + // type at all and sorts first, which is the empty-value contract. + for i := 1; i < len(out.Rows); i++ { + require.LessOrEqual(t, out.Rows[i-1].Fields["type"], out.Rows[i].Fields["type"], + "rows must be ordered by the sort selector") + } + require.Equal(t, "", out.Rows[0].Fields["type"]) +} + +// TestListViewFallsBackWithoutBatchCapability proves a repository that cannot +// batch -- a plain filesystem keg -- still works, just node by node. +func TestListViewFallsBackWithoutBatchCapability(t *testing.T) { + t.Parallel() + const nodeCount = 6 + + fx := NewSandbox(t) + base := kegpkg.NewMemoryRepo(fx.Runtime()) + counter := &countingRepo{Repository: base} + k := kegpkg.NewLocalKeg(counter, fx.Runtime()) + require.NoError(t, k.Init(context.Background())) + seedNodes(t, k, nodeCount) + + counter.perNode = 0 + + out, err := k.ListView(context.Background(), kegpkg.ListViewOptions{Fields: []string{"type"}}) + require.NoError(t, err) + require.Len(t, out.Rows, nodeCount+1) + require.Equal(t, "kind0", out.Rows[1].Fields["type"]) + require.Equal(t, nodeCount+1, counter.perNode, + "without the capability the fallback reads each node once") +} + +// TestListViewIntrinsicsReadNothing confirms the common case stays free: a +// listing naming only intrinsics and index timestamps must touch no node. +func TestListViewIntrinsicsReadNothing(t *testing.T) { + t.Parallel() + + fx := NewSandbox(t) + base := kegpkg.NewMemoryRepo(fx.Runtime()) + counter := &countingRepo{Repository: base} + k := kegpkg.NewLocalKeg(&batchingRepo{countingRepo: counter}, fx.Runtime()) + require.NoError(t, k.Init(context.Background())) + seedNodes(t, k, 5) + + counter.perNode = 0 + counter.batches = 0 + + out, err := k.ListView(context.Background(), kegpkg.ListViewOptions{ + Fields: []string{"id", "title", ".updated"}, + Sort: ".created", + }) + require.NoError(t, err) + require.Len(t, out.Rows, 6) + require.Equal(t, 0, counter.batches, "intrinsics must not trigger any read") + require.Equal(t, 0, counter.perNode, "intrinsics must not trigger any read") +} diff --git a/pkg/keg/repository.go b/pkg/keg/repository.go index 24f4893..35d8409 100644 --- a/pkg/keg/repository.go +++ b/pkg/keg/repository.go @@ -120,6 +120,34 @@ func repositorySupportsConcurrentAccess(ctx context.Context, repo Repository) bo return !ok || capability.SupportsConcurrentAccess(ctx) } +// RepositoryBatchRead optionally reads many nodes' metadata or statistics in +// one operation. +// +// It exists because listings need a value for every matching node when they +// sort or filter on a metadata key, and the per-node path costs a round trip +// each — on a database-backed repository, two queries per node, since each read +// also checks existence. A backend that can answer the whole set at once +// implements this; one that cannot simply omits it and callers fall back to +// reading node by node. +// +// Implementations return only the nodes they found. A missing entry means the +// node has no metadata or statistics, which callers treat as empty rather than +// as an error: listings render from an index that is allowed to drift. +type RepositoryBatchRead interface { + // ReadMetaBatch returns raw metadata keyed by node id path. + ReadMetaBatch(ctx context.Context, ids []NodeId) (map[string][]byte, error) + + // ReadStatsBatch returns parsed statistics keyed by node id path. + ReadStatsBatch(ctx context.Context, ids []NodeId) (map[string]*NodeStats, error) +} + +// repositoryBatchRead returns the batch-read capability when the repository has +// one. +func repositoryBatchRead(repo Repository) (RepositoryBatchRead, bool) { + capability, ok := repo.(RepositoryBatchRead) + return capability, ok +} + // RepositoryFiles provides optional per-node file attachment access. type RepositoryFiles interface { // ListFiles lists file attachment names for a node. From 61ce6732b086ba159201decd8b2e650badcec57d Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 30 Jul 2026 01:22:23 -0500 Subject: [PATCH 7/7] feat(keg): filter listings by title substring The hosted node list offers a plain title search alongside the expression filter, for the common "I half-remember the name" case. It cannot be applied by the caller, because the server pages the result: filtering after paging would report the wrong match count and show the wrong slice. Add TitleContains to ListView, applied after the query and before sorting and paging. It costs nothing, since titles are already carried by the index. --- pkg/keg/keg_aggregate.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/keg/keg_aggregate.go b/pkg/keg/keg_aggregate.go index ddf8efc..e54b876 100644 --- a/pkg/keg/keg_aggregate.go +++ b/pkg/keg/keg_aggregate.go @@ -33,6 +33,13 @@ type ListViewOptions struct { // Query is an optional boolean query expression filtering the nodes. Query string `json:"query,omitempty"` + // TitleContains further narrows the result to titles containing this + // text, case-insensitively. It is a plain substring rather than an + // expression, for the common "I half-remember the name" search, and it + // costs nothing because titles are already carried by the index. Applying + // it here rather than in the caller keeps paging and TotalMatches correct. + TitleContains string `json:"title_contains,omitempty"` + // Fields are field selectors to resolve per row, in the vocabulary of // ParseFieldSelector ("type", ".omega", "tags"). Intrinsics and index // timestamps cost nothing; other selectors are read per returned row. @@ -226,6 +233,15 @@ func (k *LocalKeg) listView(ctx context.Context, opts ListViewOptions) (*ListVie return nil, err } entries := listing.Entries + if needle := strings.ToLower(strings.TrimSpace(opts.TitleContains)); needle != "" { + kept := make([]NodeIndexEntry, 0, len(entries)) + for _, entry := range entries { + if strings.Contains(strings.ToLower(entry.Title), needle) { + kept = append(kept, entry) + } + } + entries = kept + } total := len(entries) // Sorting by metadata or a non-timestamp stat needs a value for every