diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md index ac76ab5..d3c35de 100644 --- a/docs/roadmap/audit/catalog.md +++ b/docs/roadmap/audit/catalog.md @@ -1,6 +1,6 @@ # Consolidated audit catalog -Status: **11 distinct open findings: 11 P3**, consolidated from 30 +Status: **9 distinct open findings: 9 P3**, consolidated from 30 numbered entries in seven audit documents. Reviewed audit state: `db732e3a1de55c844f77870e1c6db683c2685d5d`. @@ -296,6 +296,10 @@ selected routing and discovery semantics. ## F14 — P3: Discovery-context preparation repeatedly rebuilds shared prefixes +**Complete.** Distinct trie prefixes are enumerated and classified once; +focused discovery and resolver tests pass, and traversal was reviewed for +elimination of repeated per-declaration prefix construction. + R alias sources sharing a depth-D proper prefix cause Θ(RD²) copying and key construction before result deduplication, with repeated classification as well. The source declarations contain only Θ(RD) tokens. @@ -313,12 +317,16 @@ Reuse the tree's routing-snapshot lifetime; do not introduce a parallel index or a cross-invocation result cache. The repeated preparation and help-time scan remain distinct findings. The [joint analysis](remediation-analysis.md#f14f15--joint-source-prefix-design) records the accepted design. The performance-testing decision under F13 applies -to both; implementation remains pending. +to both. **Original:** [02-efficiency E3](02-efficiency.md#e3--p3-discovery-context-preparation-rebuilds-shared-prefixes-repeatedly). ## F15 — P3: Alias help scans the entire source catalog for each intermediate route +**Complete.** Help queries strict descendants from each resolution path instead +of scanning the complete catalog. Focused alias/help tests pass, and the tree +traversal was reviewed for the removal of unrelated source scans. + Help discovery examines all A alias sources at each of P rewrite paths, doing PA examinations and repeated copies even when there are no descendants. A valid single-token alias chain produces Θ(A²) work to discover an empty alias section diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 886eca3..a2d6ba6 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Audit remediation implementation plan -Status: in progress; Stages 1–15 complete. +Status: in progress; Stages 1–16 complete. Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint, intervening changes recorded decisions and retired the completed older @@ -527,6 +527,12 @@ Complete when repeated suffix traversal is removed without routing changes. ## Stage 16 — Reuse the resolver tree for prefix discovery and help +**Complete.** Discovery classifies each eligible trie prefix once (F14), and +help visits only strict descendants for each resolution path (F15). Source +review confirms both changes use the existing resolver tree; focused alias, +discovery, help and resolver tests pass offline with Go 1.27.0 and +`-mod=readonly`, including returned-path ownership and exact aliases with children. + Findings: F14 and F15, one coordinated change. Sources: `internal/alias/resolver.go`, `flag_aliases.go`, and `help_view.go`. diff --git a/flag_aliases.go b/flag_aliases.go index 921175f..ed09c09 100644 --- a/flag_aliases.go +++ b/flag_aliases.go @@ -66,6 +66,7 @@ func (spelling capturedFlagSpelling) token() string { func compileFlagAliases(rules []RoutingRule, roots map[string]*canonicalNode, aliases interface { Resolve(commandpath.Path) commandpath.Path + ProperSourcePrefixes() []commandpath.Path }) (map[string]resolvedFlagAliases, map[string]resolvedFlagAliases, error) { canonicalNodes := indexCanonicalNodes(roots) global := make(map[string]flagAliasMapping) @@ -170,6 +171,7 @@ func canonicalFlagSpellings(command Command) map[string]capturedFlagSpelling { func discoverFlagContexts(rules []RoutingRule, roots map[string]*canonicalNode, aliases interface { Resolve(commandpath.Path) commandpath.Path + ProperSourcePrefixes() []commandpath.Path }, canonicalNodes map[string]*canonicalNode) map[string]flagAliasContext { commandSources := make(map[string]struct{}) prefixes := make(map[string]commandpath.Path) @@ -178,26 +180,20 @@ func discoverFlagContexts(rules []RoutingRule, roots map[string]*canonicalNode, commandSources[rule.record.sourcePath.Key()] = struct{}{} } } - for _, rule := range rules { - if rule.record.kind != commandAliasRule { + for _, prefix := range aliases.ProperSourcePrefixes() { + key := prefix.Key() + if canonicalNodes[key] != nil { continue } - for length := 1; length < len(rule.record.sourcePath); length++ { - prefix := rule.record.sourcePath[:length].Clone() - key := prefix.Key() - if canonicalNodes[key] != nil { - continue - } - if _, exactAlias := commandSources[key]; exactAlias { - continue - } - resolved := aliases.Resolve(prefix) - trace := traceCanonicalPath(roots, resolved) - if trace.exact != nil || (trace.operandOwner != nil && acceptsOperandPrefix(trace.operandOwner.command, len(resolved)-len(trace.operandOwner.path))) { - continue - } - prefixes[key] = prefix + if _, exactAlias := commandSources[key]; exactAlias { + continue } + resolved := aliases.Resolve(prefix) + trace := traceCanonicalPath(roots, resolved) + if trace.exact != nil || (trace.operandOwner != nil && acceptsOperandPrefix(trace.operandOwner.command, len(resolved)-len(trace.operandOwner.path))) { + continue + } + prefixes[key] = prefix } contexts := make(map[string]flagAliasContext, len(prefixes)) for key, path := range prefixes { diff --git a/help_view.go b/help_view.go index 09df5fe..bc0a036 100644 --- a/help_view.go +++ b/help_view.go @@ -36,11 +36,8 @@ func routeHelpAliases(snapshot *routingSnapshot, route routeResult) []commandmet } bySource := make(map[string]commandmeta.CommandAlias) for _, prefix := range snapshot.aliases.ResolutionPaths(route.enteredPath) { - for _, alias := range snapshot.commandAliases { - source := commandpath.FromTokens(alias.Source) - if len(source) <= len(prefix) || !source.HasPrefix(prefix) { - continue - } + for _, alias := range snapshot.aliases.Descendants(prefix) { + source := alias.Source suffix := source[len(prefix):] entered := append(route.enteredPath.Clone(), suffix...) target := snapshot.aliases.Resolve(entered) diff --git a/internal/alias/resolver.go b/internal/alias/resolver.go index a6d8468..511e689 100644 --- a/internal/alias/resolver.go +++ b/internal/alias/resolver.go @@ -182,3 +182,50 @@ func (resolver *Resolver) ResolveWithProvenance(path commandpath.Path) (commandp origins = nextOrigins } } + +// ProperSourcePrefixes returns distinct nonempty proper prefixes of source paths. +// Returned paths are owned by the caller. +func (resolver *Resolver) ProperSourcePrefixes() []commandpath.Path { + if resolver == nil { + return nil + } + var paths []commandpath.Path + for index := 1; index < len(resolver.trie); index++ { + node := &resolver.trie[index] + if len(node.children) != 0 { + paths = append(paths, node.path.Clone()) + } + } + return paths +} + +// Descendants returns owned mappings whose sources are strictly below prefix. +func (resolver *Resolver) Descendants(prefix commandpath.Path) []Mapping { + if resolver == nil { + return nil + } + index := 0 + for _, component := range prefix { + next, ok := resolver.trie[index].children[component] + if !ok { + return nil + } + index = next + } + var mappings []Mapping + var visit func(int) + visit = func(index int) { + node := &resolver.trie[index] + if node.mapping >= 0 { + mapping := resolver.mappings[node.mapping] + mappings = append(mappings, Mapping{Source: mapping.Source.Clone(), Target: mapping.Target.Clone()}) + } + for _, child := range node.children { + visit(child) + } + } + for _, child := range resolver.trie[index].children { + visit(child) + } + return mappings +} diff --git a/internal/alias/resolver_test.go b/internal/alias/resolver_test.go index 2e4ee7e..d6ea52e 100644 --- a/internal/alias/resolver_test.go +++ b/internal/alias/resolver_test.go @@ -314,3 +314,34 @@ func (machine *Machine) classify(token string) symbol { } return symbol{kind: otherSymbol} } + +func TestResolverSourcePrefixesAndDescendantsAreOwned(t *testing.T) { + resolver, err := NewResolver([]Mapping{ + {Source: commandpath.Path{"a"}, Target: commandpath.Path{"root"}}, + {Source: commandpath.Path{"a", "b", "one"}, Target: commandpath.Path{"root", "one"}}, + {Source: commandpath.Path{"a", "b", "two"}, Target: commandpath.Path{"root", "two"}}, + {Source: commandpath.Path{"other"}, Target: commandpath.Path{"root"}}, + }) + if err != nil { + t.Fatal(err) + } + prefixes := resolver.ProperSourcePrefixes() + if len(prefixes) != 2 || !prefixes[0].Equal(commandpath.Path{"a"}) || !prefixes[1].Equal(commandpath.Path{"a", "b"}) { + t.Fatalf("prefixes = %#v", prefixes) + } + prefixes[0][0] = "mutated" + descendants := resolver.Descendants(commandpath.Path{"a"}) + if len(descendants) != 2 { + t.Fatalf("descendants = %#v", descendants) + } + for _, mapping := range descendants { + if len(mapping.Source) != 3 || mapping.Source[0] != "a" { + t.Fatalf("mapping = %#v", mapping) + } + } + descendants[0].Source[0] = "mutated" + descendants[0].Target[0] = "mutated" + if !resolver.Resolve(commandpath.Path{"a", "b", "one"}).Equal(commandpath.Path{"root", "one"}) || len(resolver.Descendants(commandpath.Path{"missing"})) != 0 { + t.Fatal("resolver was mutated or missing branch returned mappings") + } +}