← back to Cli Printing Press
fix(cli): path-aware dogfood novel-feature matcher (#195)
126b00d055f31b4189123ffe52862b23ba119a2d · 2026-04-12 20:23:30 -0700 · Trevin Chow
* fix(cli): path-aware dogfood novel-feature matcher — closes #191
The leaf-only flat-map matcher had two failure modes:
1. False positives. Planned "portfolio perf" (leaf "perf") would
match any "perf" anywhere in the tree, including unrelated
sibling subtrees. With the growing set of commands in embossed
CLIs, leaf collisions became a real risk.
2. False negatives. Any nesting signal was thrown away: planned
"auth login --chrome" reduced to leaf "login", and matching
relied on hyphen-prefix against ALL registered leaves, not just
siblings of "auth".
Replaces the matcher with path-aware matching:
- Build full command PATHS from the CLI source by walking
AddCommand(newXxxCmd) relationships. Roots come from rootCmd.
AddCommand(...) calls (Execute() / any helper — scanned across
the full file, not just newXxxCmd bodies).
- Match strategies in order: exact full path → hyphen-prefix on the
LAST segment restricted to the same parent subtree → aliases
with the same rules.
- Leaf-only fallback when path construction is empty, preserving
backward compatibility for unusual CLI layouts and existing tests.
- matchNovelFeature now returns (matched, reason) so callers can
log WHY a feature matched or failed.
New tests:
- TestMatchNovelFeature_PathAware — exact-path + sibling-hyphen-prefix
- TestMatchNovelFeature_LeafFallback — preserves old behavior
- TestMatchNovelFeature_NoFalsePositive_CrossParent — planned
"portfolio perf" must NOT match "analytics perf" across parents
- TestMatchNovelFeature_NoFalsePositive_BareLeaf — "op" must not
prefix-match "options"
- TestCollectRegisteredCommandPaths — full tree walker test with
synthetic CLI fixture mirroring yahoo-finance layout (auth/
login-chrome, portfolio/perf, options vs options-chain siblings)
- TestCommandPath — flag-stripping path extractor
Backward compatibility:
- commandLeaf retained as a thin wrapper for any external callers.
- When registeredPaths is empty, matchNovelFeature falls back to
the old leaf-only behavior against registeredLeaves.
Full test suite: 1637 pass across 22 packages. golangci-lint clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* refactor(cli): simplify matcher after review — fewer maps, less narration
Applied review feedback from /simplify on the path-aware matcher.
Quality:
- matchNovelFeature no longer returns (bool, string) — the reason
was write-only; no caller consumed it. Simpler signature.
- Added matchLeaf as a named dual of matchPath, with a `try`
closure in matchNovelFeature that dispatches based on which map
is populated. Eliminates the copy-paste alias fallback block.
- Deleted commandLeaf (no internal callers; backward-compat claim
was fiction — it's unexported).
- Dropped lastPathSegment helper — only used inside splitCommandPath
callers, and using splitCommandPath directly is just as short.
- BFS seen-map now uses a struct key {funcName, prefix} instead of
"funcName@prefix" concatenation; check moved to top of loop so
we can't do redundant work before dedup.
- Aggressive comment pruning. Removed ~40 lines of narration that
just restated what the regex or function name already says.
Efficiency:
- Merged collectRegisteredCommandPaths + collectRegisteredCommands
into one pass. Old code read every internal/cli/*.go file twice.
Now reads once, returns (paths, leaves).
- Extracted extractFuncBody helper for the brace-balancing body
scanner. Colocated near listGoFiles so checkCommandTree can adopt
it later if it ever needs parent-scoped command info.
Tests:
- Consolidated the 5-case overlap between TestMatchNovelFeature_
LeafFallback and TestMatchNovelFeature_PathAware into a shared
matcherCommonCases table. Both tests append mode-specific cases.
- Renamed TestCollectRegisteredCommandPaths → TestCollect
RegisteredCommands_Tree to match the function rename; asserts
both paths and leaves are populated from the same pass.
All 1641 pipeline tests pass. golangci-lint clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(cli): always consult leaves after path-match miss, per codex review
Codex review flagged a real regression: the prior 'paths if non-empty
else leaves' dispatch caused false negatives on CLIs where command
tree reconstruction is only partial.
Tree reconstruction parses direct literal AddCommand calls:
parent.AddCommand(newFooCmd())
It does NOT capture the variable-indirection pattern used in several
shipped CLIs:
sub := newFooCmd(flags)
parent.AddCommand(sub)
For those CLIs, paths ends up partial (one captured command is enough
to make len(paths) > 0) so the matcher stopped consulting leaves —
even though leaves is populated from a separate regex scan of every
Use: field and is effectively complete. Net: novel features
wired through the variable pattern (e.g. yahoo-finance's auth
login-chrome flow, hackernews's show/ask commands) were marked
missing by dogfood and stripped from novel_features_built.
Fix: always try leaves after paths. path-match still runs first
(preferring the more specific parent-scoped match when it fires);
leaves is the safety net.
Tradeoff: re-introduces the possibility of cross-parent leaf false
positives when two commands share a leaf name under different
parents AND only one ended up in paths. That's a rare corner case;
the much more common case is that paths is partial for legitimate
built commands. Leaf-fallback is the pragmatically correct default.
Regression test TestMatchNovelFeature_FallsBackToLeavesWhenPathsPartial
locks in the behavior so future matcher changes can't drop it again.
All 1642 tests pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Files touched
M internal/pipeline/dogfood.goM internal/pipeline/novel_features_matcher_test.go
Diff
commit 126b00d055f31b4189123ffe52862b23ba119a2d
Author: Trevin Chow <trevin@trevinchow.com>
Date: Sun Apr 12 20:23:30 2026 -0700
fix(cli): path-aware dogfood novel-feature matcher (#195)
* fix(cli): path-aware dogfood novel-feature matcher — closes #191
The leaf-only flat-map matcher had two failure modes:
1. False positives. Planned "portfolio perf" (leaf "perf") would
match any "perf" anywhere in the tree, including unrelated
sibling subtrees. With the growing set of commands in embossed
CLIs, leaf collisions became a real risk.
2. False negatives. Any nesting signal was thrown away: planned
"auth login --chrome" reduced to leaf "login", and matching
relied on hyphen-prefix against ALL registered leaves, not just
siblings of "auth".
Replaces the matcher with path-aware matching:
- Build full command PATHS from the CLI source by walking
AddCommand(newXxxCmd) relationships. Roots come from rootCmd.
AddCommand(...) calls (Execute() / any helper — scanned across
the full file, not just newXxxCmd bodies).
- Match strategies in order: exact full path → hyphen-prefix on the
LAST segment restricted to the same parent subtree → aliases
with the same rules.
- Leaf-only fallback when path construction is empty, preserving
backward compatibility for unusual CLI layouts and existing tests.
- matchNovelFeature now returns (matched, reason) so callers can
log WHY a feature matched or failed.
New tests:
- TestMatchNovelFeature_PathAware — exact-path + sibling-hyphen-prefix
- TestMatchNovelFeature_LeafFallback — preserves old behavior
- TestMatchNovelFeature_NoFalsePositive_CrossParent — planned
"portfolio perf" must NOT match "analytics perf" across parents
- TestMatchNovelFeature_NoFalsePositive_BareLeaf — "op" must not
prefix-match "options"
- TestCollectRegisteredCommandPaths — full tree walker test with
synthetic CLI fixture mirroring yahoo-finance layout (auth/
login-chrome, portfolio/perf, options vs options-chain siblings)
- TestCommandPath — flag-stripping path extractor
Backward compatibility:
- commandLeaf retained as a thin wrapper for any external callers.
- When registeredPaths is empty, matchNovelFeature falls back to
the old leaf-only behavior against registeredLeaves.
Full test suite: 1637 pass across 22 packages. golangci-lint clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* refactor(cli): simplify matcher after review — fewer maps, less narration
Applied review feedback from /simplify on the path-aware matcher.
Quality:
- matchNovelFeature no longer returns (bool, string) — the reason
was write-only; no caller consumed it. Simpler signature.
- Added matchLeaf as a named dual of matchPath, with a `try`
closure in matchNovelFeature that dispatches based on which map
is populated. Eliminates the copy-paste alias fallback block.
- Deleted commandLeaf (no internal callers; backward-compat claim
was fiction — it's unexported).
- Dropped lastPathSegment helper — only used inside splitCommandPath
callers, and using splitCommandPath directly is just as short.
- BFS seen-map now uses a struct key {funcName, prefix} instead of
"funcName@prefix" concatenation; check moved to top of loop so
we can't do redundant work before dedup.
- Aggressive comment pruning. Removed ~40 lines of narration that
just restated what the regex or function name already says.
Efficiency:
- Merged collectRegisteredCommandPaths + collectRegisteredCommands
into one pass. Old code read every internal/cli/*.go file twice.
Now reads once, returns (paths, leaves).
- Extracted extractFuncBody helper for the brace-balancing body
scanner. Colocated near listGoFiles so checkCommandTree can adopt
it later if it ever needs parent-scoped command info.
Tests:
- Consolidated the 5-case overlap between TestMatchNovelFeature_
LeafFallback and TestMatchNovelFeature_PathAware into a shared
matcherCommonCases table. Both tests append mode-specific cases.
- Renamed TestCollectRegisteredCommandPaths → TestCollect
RegisteredCommands_Tree to match the function rename; asserts
both paths and leaves are populated from the same pass.
All 1641 pipeline tests pass. golangci-lint clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(cli): always consult leaves after path-match miss, per codex review
Codex review flagged a real regression: the prior 'paths if non-empty
else leaves' dispatch caused false negatives on CLIs where command
tree reconstruction is only partial.
Tree reconstruction parses direct literal AddCommand calls:
parent.AddCommand(newFooCmd())
It does NOT capture the variable-indirection pattern used in several
shipped CLIs:
sub := newFooCmd(flags)
parent.AddCommand(sub)
For those CLIs, paths ends up partial (one captured command is enough
to make len(paths) > 0) so the matcher stopped consulting leaves —
even though leaves is populated from a separate regex scan of every
Use: field and is effectively complete. Net: novel features
wired through the variable pattern (e.g. yahoo-finance's auth
login-chrome flow, hackernews's show/ask commands) were marked
missing by dogfood and stripped from novel_features_built.
Fix: always try leaves after paths. path-match still runs first
(preferring the more specific parent-scoped match when it fires);
leaves is the safety net.
Tradeoff: re-introduces the possibility of cross-parent leaf false
positives when two commands share a leaf name under different
parents AND only one ended up in paths. That's a rare corner case;
the much more common case is that paths is partial for legitimate
built commands. Leaf-fallback is the pragmatically correct default.
Regression test TestMatchNovelFeature_FallsBackToLeavesWhenPathsPartial
locks in the behavior so future matcher changes can't drop it again.
All 1642 tests pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
internal/pipeline/dogfood.go | 257 ++++++++++++++++------
internal/pipeline/novel_features_matcher_test.go | 268 +++++++++++++++--------
2 files changed, 364 insertions(+), 161 deletions(-)
diff --git a/internal/pipeline/dogfood.go b/internal/pipeline/dogfood.go
index 808ea4c3..6cbf43c9 100644
--- a/internal/pipeline/dogfood.go
+++ b/internal/pipeline/dogfood.go
@@ -184,15 +184,14 @@ func checkNovelFeatures(cliDir, researchDir string) NovelFeaturesCheckResult {
return NovelFeaturesCheckResult{Skipped: true}
}
- // Build set of registered command Use: names from the CLI source.
- registeredCmds := collectRegisteredCommands(cliDir)
+ paths, leaves := collectRegisteredCommands(cliDir)
result := NovelFeaturesCheckResult{
Planned: len(research.NovelFeatures),
}
built := make([]NovelFeature, 0)
for _, nf := range research.NovelFeatures {
- if matchNovelFeature(nf, registeredCmds) {
+ if matchNovelFeature(nf, paths, leaves) {
result.Found++
built = append(built, nf)
} else {
@@ -209,108 +208,230 @@ func checkNovelFeatures(cliDir, researchDir string) NovelFeaturesCheckResult {
return result
}
-// matchNovelFeature runs the three-pass matcher — exact, prefix, alias —
-// against the set of registered command Use: names. Returns true if the
-// planned feature has a plausible corresponding built command.
+// matchNovelFeature reports whether a planned novel feature has a
+// corresponding built command. When paths are available it matches on
+// full command paths (so "portfolio perf" does not collide with
+// "analytics perf"); when paths are empty it falls back to leaf-only
+// matching against the flat leaves set for CLIs where the tree walker
+// couldn't construct a tree.
//
-// Why this is three passes:
-// - Exact: planned command leaf equals a registered Use: name. The happy
-// path (e.g. planned "portfolio perf", built "perf" subcommand).
-// - Prefix: during implementation, planners frequently hyphenate or
-// specialize the command. Planned "auth login --chrome" becomes built
-// "login-chrome"; planned "options --moneyness" becomes built
-// "options-chain". Either direction of prefix relationship counts.
-// - Alias: escape hatch for cases the matcher can't infer — planner records
-// alternate names in research.json's `aliases: []`.
+// Match strategies in both modes: exact match, then hyphen-prefix on
+// the last segment (sibling specialization — "auth login" → "auth
+// login-chrome"). Aliases run the same rules.
//
-// The matcher strips flag tokens (anything starting with "-") from the planned
-// command before extracting the leaf, because flags aren't command names.
-// "options --moneyness otm --max-dte 45" reduces to leaf "options".
-func matchNovelFeature(nf NovelFeature, registered map[string]bool) bool {
- leaf := strings.ToLower(commandLeaf(nf.Command))
- if leaf == "" {
+// Paths and leaves are both consulted. Path matching is preferred when
+// it fires because it's more specific, but leaf matching runs as a
+// fallback on every planned command — tree reconstruction only sees
+// commands wired via direct `rootCmd.AddCommand(newFooCmd())` /
+// `cmd.AddCommand(newFooCmd())` literals, so any CLI using variable-
+// based or late-bound registration (`sub := newFooCmd(); parent.
+// AddCommand(sub)`) will have a partial paths map. Treating a
+// non-empty paths map as "complete" would false-negative legitimate
+// built commands.
+func matchNovelFeature(nf NovelFeature, paths, leaves map[string]bool) bool {
+ plan := commandPath(nf.Command)
+ if plan == "" {
return false
}
-
- // Pass 1: exact
- if registered[leaf] {
+ try := func(p string) bool {
+ return matchPath(p, paths) || matchLeaf(p, leaves)
+ }
+ if try(plan) {
return true
}
-
- // Pass 2: prefix — either direction. Built command may be a hyphenated
- // specialization (planned "options" → built "options-chain") or the planner
- // may have been more specific than the actual binding (planned "earnings-
- // calendar" is substring-matched by built "earnings"). Only hyphen
- // boundaries count as prefix to avoid substring false positives like
- // "options" matching "op".
- for use := range registered {
- if strings.HasPrefix(use, leaf+"-") || strings.HasPrefix(leaf, use+"-") {
+ for _, alias := range nf.Aliases {
+ if ap := commandPath(alias); ap != "" && try(ap) {
return true
}
}
+ return false
+}
- // Pass 3: aliases declared in research.json. Each alias is matched with
- // the same exact + prefix rules so planners don't need to list every
- // hyphen variant.
- for _, alias := range nf.Aliases {
- aliasLeaf := strings.ToLower(commandLeaf(alias))
- if aliasLeaf == "" {
+// matchPath matches a planned path against a set of built paths:
+// exact match, or sibling hyphen-prefix (same parent, leaf ↔ leaf-foo).
+func matchPath(plan string, paths map[string]bool) bool {
+ if paths[plan] {
+ return true
+ }
+ parent, leaf := splitCommandPath(plan)
+ if leaf == "" {
+ return false
+ }
+ for path := range paths {
+ pp, pl := splitCommandPath(path)
+ if pp != parent || pl == "" {
continue
}
- if registered[aliasLeaf] {
+ if strings.HasPrefix(pl, leaf+"-") || strings.HasPrefix(leaf, pl+"-") {
return true
}
- for use := range registered {
- if strings.HasPrefix(use, aliasLeaf+"-") || strings.HasPrefix(aliasLeaf, use+"-") {
- return true
- }
- }
}
+ return false
+}
+// matchLeaf is the legacy leaf-only matcher used as a fallback when the
+// built command tree can't be reconstructed. Ignores nesting.
+func matchLeaf(plan string, leaves map[string]bool) bool {
+ _, leaf := splitCommandPath(plan)
+ if leaf == "" {
+ return false
+ }
+ if leaves[leaf] {
+ return true
+ }
+ for use := range leaves {
+ if strings.HasPrefix(use, leaf+"-") || strings.HasPrefix(leaf, use+"-") {
+ return true
+ }
+ }
return false
}
-// commandLeaf strips flag tokens (anything beginning with "-") from a command
-// string and returns the last remaining token. "auth login --chrome" → "login";
-// "options --moneyness otm" → "options" (subsequent tokens are flag values).
-//
-// The rule "stop at first flag" mirrors how users describe commands in plans —
-// the command itself appears before any flag annotation.
-func commandLeaf(cmd string) string {
- tokens := strings.Fields(cmd)
- base := make([]string, 0, len(tokens))
+// commandPath strips flag tokens from a command string and joins the
+// remaining leading non-flag tokens into a space-separated path. Stops
+// at the first flag because any bare word that follows is a flag value,
+// not a command token — "options --moneyness otm" → "options".
+func commandPath(cmd string) string {
+ tokens := strings.Fields(strings.ToLower(cmd))
+ path := make([]string, 0, len(tokens))
for _, t := range tokens {
if strings.HasPrefix(t, "-") {
break
}
- base = append(base, t)
+ path = append(path, t)
}
- if len(base) == 0 {
- return ""
+ return strings.Join(path, " ")
+}
+
+// splitCommandPath returns the parent and leaf segments of a space-
+// separated path. "portfolio perf" → ("portfolio", "perf"), "digest" →
+// ("", "digest").
+func splitCommandPath(path string) (parent, leaf string) {
+ i := strings.LastIndex(path, " ")
+ if i < 0 {
+ return "", path
}
- return base[len(base)-1]
+ return path[:i], path[i+1:]
}
-// collectRegisteredCommands returns a set of cobra Use: names found in the
-// CLI's internal/cli/*.go files.
-func collectRegisteredCommands(dir string) map[string]bool {
+// collectRegisteredCommands reads the CLI's internal/cli/*.go files once
+// and returns two views of its registered cobra commands:
+//
+// - paths: full space-separated command paths (e.g. "portfolio perf",
+// "auth login-chrome"), reconstructed by walking AddCommand edges.
+// Empty when the tree walker can't identify roots or the source
+// doesn't follow the expected pattern.
+// - leaves: a flat set of every Use: name (e.g. "perf", "login-chrome")
+// regardless of nesting. Used by callers as a leaf-only fallback
+// when paths is empty.
+//
+// Callers prefer paths when non-empty for accuracy, and fall through to
+// leaves when the tree walker produced nothing.
+func collectRegisteredCommands(dir string) (paths, leaves map[string]bool) {
cliDir := filepath.Join(dir, "internal", "cli")
files := listGoFiles(cliDir)
- useFieldRe := regexp.MustCompile(`(?m)Use:\s*"([^"\s]+)`)
- cmds := make(map[string]bool)
+
+ type cmdFunc struct {
+ use string
+ children []string
+ }
+ funcs := map[string]*cmdFunc{}
+ var rootFuncs []string
+ leaves = map[string]bool{}
+
+ // Root detection scans the full file because the wiring function
+ // (Execute / helpers) isn't a new*Cmd constructor.
+ rootAddRe := regexp.MustCompile(`rootCmd\.AddCommand\(\s*(new\w+Cmd)\b`)
+ funcHeaderRe := regexp.MustCompile(`func\s+(new\w+Cmd)\s*\(`)
+ useRe := regexp.MustCompile(`Use:\s*"([^"\s]+)`)
+ addChildRe := regexp.MustCompile(`\.AddCommand\(\s*(new\w+Cmd)\b`)
+
for _, file := range files {
data, err := os.ReadFile(file)
if err != nil {
continue
}
- for _, m := range useFieldRe.FindAllStringSubmatch(string(data), -1) {
- name := strings.Fields(m[1])[0]
- if name != "" {
- cmds[name] = true
+ src := string(data)
+ for _, rm := range rootAddRe.FindAllStringSubmatch(src, -1) {
+ rootFuncs = append(rootFuncs, rm[1])
+ }
+ for _, u := range useRe.FindAllStringSubmatch(src, -1) {
+ if name := strings.Fields(u[1])[0]; name != "" {
+ leaves[name] = true
+ }
+ }
+ for _, m := range funcHeaderRe.FindAllStringSubmatchIndex(src, -1) {
+ name := src[m[2]:m[3]]
+ body := extractFuncBody(src, m[1])
+ if body == "" {
+ continue
}
+ entry := &cmdFunc{}
+ if u := useRe.FindStringSubmatch(body); u != nil {
+ entry.use = strings.Fields(u[1])[0]
+ }
+ for _, cm := range addChildRe.FindAllStringSubmatch(body, -1) {
+ entry.children = append(entry.children, cm[1])
+ }
+ funcs[name] = entry
}
}
- return cmds
+
+ paths = map[string]bool{}
+ if len(rootFuncs) == 0 || len(funcs) == 0 {
+ return paths, leaves
+ }
+
+ type qItem struct{ funcName, prefix string }
+ queue := make([]qItem, 0, len(rootFuncs))
+ for _, rf := range rootFuncs {
+ queue = append(queue, qItem{funcName: rf})
+ }
+ seen := map[qItem]bool{}
+ for len(queue) > 0 {
+ it := queue[0]
+ queue = queue[1:]
+ if seen[it] {
+ continue
+ }
+ seen[it] = true
+ fn, ok := funcs[it.funcName]
+ if !ok || fn.use == "" {
+ continue
+ }
+ path := fn.use
+ if it.prefix != "" {
+ path = it.prefix + " " + fn.use
+ }
+ paths[path] = true
+ for _, child := range fn.children {
+ queue = append(queue, qItem{funcName: child, prefix: path})
+ }
+ }
+ return paths, leaves
+}
+
+// extractFuncBody returns the balanced-brace body of a Go function given
+// the position of its header (regex match end). Empty string if no body
+// is found.
+func extractFuncBody(src string, headerEnd int) string {
+ i := strings.Index(src[headerEnd:], "{")
+ if i < 0 {
+ return ""
+ }
+ start := headerEnd + i + 1
+ depth := 1
+ end := start
+ for end < len(src) && depth > 0 {
+ switch src[end] {
+ case '{':
+ depth++
+ case '}':
+ depth--
+ }
+ end++
+ }
+ return src[start:end]
}
func LoadDogfoodResults(dir string) (*DogfoodReport, error) {
diff --git a/internal/pipeline/novel_features_matcher_test.go b/internal/pipeline/novel_features_matcher_test.go
index e2066945..125ecd95 100644
--- a/internal/pipeline/novel_features_matcher_test.go
+++ b/internal/pipeline/novel_features_matcher_test.go
@@ -1,129 +1,211 @@
package pipeline
import (
+ "os"
+ "path/filepath"
"testing"
)
-// TestMatchNovelFeature covers the three-pass matcher — exact, prefix, alias —
-// exercising the yahoo-finance retro scenarios where the previous literal
-// matcher false-negatived features that were actually built.
-func TestMatchNovelFeature(t *testing.T) {
- registered := map[string]bool{
- "perf": true, // under `portfolio perf`
- "gains": true,
- "digest": true,
- "login-chrome": true, // under `auth login-chrome`
- "options": true,
- "options-chain": true,
- "sparkline": true,
- "earnings": true, // registered but planned was "earnings-calendar"
- }
+// Cases that should match identically under both matching modes.
+var matcherCommonCases = []struct {
+ name string
+ feature NovelFeature
+ want bool
+}{
+ {"exact leaf match", NovelFeature{Command: "digest"}, true},
+ {"exact after flag strip", NovelFeature{Command: "digest --watchlist tech"}, true},
+ {"no match when absent", NovelFeature{Command: "insiders --recent 30d"}, false},
+ {"empty command", NovelFeature{Command: ""}, false},
+ {"flag-only command", NovelFeature{Command: "--json"}, false},
+ {"case insensitive", NovelFeature{Command: "DIGEST"}, true},
+ {"alias exact", NovelFeature{Command: "portfolio dividends", Aliases: []string{"digest"}}, true},
+}
- cases := []struct {
+func TestMatchNovelFeature_LeafFallback(t *testing.T) {
+ leaves := map[string]bool{
+ "perf": true, "gains": true, "digest": true, "login-chrome": true,
+ "options": true, "options-chain": true, "sparkline": true, "earnings": true,
+ }
+ cases := append([]struct {
name string
feature NovelFeature
want bool
- pass string
}{
- {
- name: "exact match on leaf (portfolio perf → perf)",
- feature: NovelFeature{Command: "portfolio perf"},
- want: true,
- pass: "exact",
- },
- {
- name: "exact match after flag strip (digest --watchlist tech → digest)",
- feature: NovelFeature{Command: "digest --watchlist tech"},
- want: true,
- pass: "exact",
- },
- {
- name: "prefix match: planned auth login --chrome → built login-chrome",
- feature: NovelFeature{Command: "auth login --chrome"},
- want: true,
- pass: "prefix",
- },
- {
- name: "prefix match: planned options --moneyness → built options or options-chain",
- feature: NovelFeature{Command: "options --moneyness otm --max-dte 45"},
- want: true,
- pass: "exact (options registered) or prefix (options-chain)",
- },
- {
- name: "no match when truly absent",
- feature: NovelFeature{Command: "insiders --recent 30d --net-buying"},
- want: false,
- },
- {
- name: "alias rescues a rename",
- feature: NovelFeature{Command: "portfolio dividends", Aliases: []string{"portfolio gains"}},
- want: true,
- pass: "alias (gains built as renamed feature)",
- },
- {
- name: "alias prefix match",
- feature: NovelFeature{Command: "portfolio dividends", Aliases: []string{"auth login-chrome"}},
- want: true,
- pass: "alias exact (login-chrome registered)",
- },
- {
- name: "empty command returns false",
- feature: NovelFeature{Command: ""},
- want: false,
- },
- {
- name: "flag-only command returns false (nothing to match)",
- feature: NovelFeature{Command: "--json"},
- want: false,
- },
- {
- name: "case insensitive",
- feature: NovelFeature{Command: "PORTFOLIO PERF"},
- want: true,
- pass: "exact (case folded)",
- },
+ {"leaf-only: portfolio perf matches perf regardless of parent", NovelFeature{Command: "portfolio perf"}, true},
+ {"leaf-only hyphen-prefix: auth login → login-chrome", NovelFeature{Command: "auth login --chrome"}, true},
+ {"bare leaf registered: options", NovelFeature{Command: "options --moneyness otm"}, true},
+ }, matcherCommonCases...)
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ if got := matchNovelFeature(tc.feature, nil, leaves); got != tc.want {
+ t.Errorf("matchNovelFeature(%q) = %v, want %v", tc.feature.Command, got, tc.want)
+ }
+ })
}
+}
+
+func TestMatchNovelFeature_PathAware(t *testing.T) {
+ paths := map[string]bool{
+ "portfolio": true, "portfolio perf": true, "portfolio gains": true, "portfolio add": true,
+ "auth": true, "auth login": true, "auth login-chrome": true, "auth logout": true,
+ "options": true, "options-chain": true,
+ "digest": true,
+ "watchlist": true, "watchlist create": true, "watchlist add": true,
+ }
+ cases := append([]struct {
+ name string
+ feature NovelFeature
+ want bool
+ }{
+ {"exact full path", NovelFeature{Command: "portfolio perf"}, true},
+ {"exact hyphenated leaf", NovelFeature{Command: "auth login-chrome"}, true},
+ {"sibling hyphen-prefix under parent", NovelFeature{Command: "auth login --chrome"}, true},
+ {"sibling hyphen-prefix at root", NovelFeature{Command: "options --moneyness otm"}, true},
+ {"alias rescues missing primary", NovelFeature{Command: "portfolio dividends", Aliases: []string{"portfolio gains"}}, true},
+ {"case insensitive path", NovelFeature{Command: "Portfolio Perf"}, true},
+ {"wrong parent does not match", NovelFeature{Command: "nonexistent perf"}, false},
+ }, matcherCommonCases...)
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
- got := matchNovelFeature(tc.feature, registered)
- if got != tc.want {
- t.Errorf("matchNovelFeature(%q, aliases=%v) = %v, want %v (pass: %s)",
- tc.feature.Command, tc.feature.Aliases, got, tc.want, tc.pass)
+ if got := matchNovelFeature(tc.feature, paths, nil); got != tc.want {
+ t.Errorf("matchNovelFeature(%q) = %v, want %v", tc.feature.Command, got, tc.want)
}
})
}
}
-// TestCommandLeaf covers the flag-stripping helper.
-func TestCommandLeaf(t *testing.T) {
+// A planned leaf must not match a registered leaf under a different parent.
+// This is the accuracy guarantee that path-awareness buys over the old
+// flat-map matcher.
+func TestMatchNovelFeature_NoFalsePositive_CrossParent(t *testing.T) {
+ paths := map[string]bool{
+ "analytics": true,
+ "analytics perf": true,
+ }
+ nf := NovelFeature{Command: "portfolio perf"}
+ if matchNovelFeature(nf, paths, nil) {
+ t.Error("planned 'portfolio perf' should not match 'analytics perf' across parents")
+ }
+}
+
+// "op" must not prefix-match "options" — hyphen boundary required.
+func TestMatchNovelFeature_NoFalsePositive_BareLeaf(t *testing.T) {
+ paths := map[string]bool{"options": true}
+ nf := NovelFeature{Command: "op"}
+ if matchNovelFeature(nf, paths, nil) {
+ t.Error("planned 'op' should not prefix-match 'options'")
+ }
+}
+
+// Tree reconstruction only captures commands wired via direct
+// `parent.AddCommand(newFooCmd())` literals; CLIs that use variable
+// indirection (`sub := newFooCmd(); parent.AddCommand(sub)`) produce a
+// partial paths map. The matcher must still fall back to the
+// Use-field-scanned leaves so legitimately-built features aren't
+// reported missing. Regression guard for a pattern shipped in yahoo-
+// finance and hackernews.
+func TestMatchNovelFeature_FallsBackToLeavesWhenPathsPartial(t *testing.T) {
+ // paths has the directly-wired command only; `ask` is present in
+ // the source but wired through a variable, so it shows up in leaves
+ // but not paths.
+ paths := map[string]bool{"show": true}
+ leaves := map[string]bool{"show": true, "ask": true}
+ nf := NovelFeature{Command: "ask"}
+ if !matchNovelFeature(nf, paths, leaves) {
+ t.Error("'ask' (present in leaves, absent from partial paths) should still match")
+ }
+}
+
+func TestCommandPath(t *testing.T) {
cases := map[string]string{
- "portfolio perf": "perf",
- "auth login --chrome": "login",
+ "portfolio perf": "portfolio perf",
+ "auth login --chrome": "auth login",
"options --moneyness otm --max-dte 45": "options",
"digest": "digest",
"digest --watchlist tech": "digest",
"earnings-calendar --watchlist": "earnings-calendar",
"": "",
"--json": "",
- " padded perf ": "perf",
+ " padded perf ": "padded perf",
+ "PORTFOLIO PERF": "portfolio perf",
}
for in, want := range cases {
- if got := commandLeaf(in); got != want {
- t.Errorf("commandLeaf(%q) = %q, want %q", in, got, want)
+ if got := commandPath(in); got != want {
+ t.Errorf("commandPath(%q) = %q, want %q", in, got, want)
}
}
}
-// TestMatchNovelFeature_PrefixDoesNotFalsePositive guards against over-eager
-// prefix matching. "op" should NOT match "options" — only hyphen-bounded
-// prefixes count.
-func TestMatchNovelFeature_PrefixDoesNotFalsePositive(t *testing.T) {
- registered := map[string]bool{
- "options": true,
+// End-to-end: synthetic CLI fixture + tree walker should emit every
+// nested path the matcher needs.
+func TestCollectRegisteredCommands_Tree(t *testing.T) {
+ dir := t.TempDir()
+ cli := filepath.Join(dir, "internal", "cli")
+ if err := os.MkdirAll(cli, 0o755); err != nil {
+ t.Fatal(err)
}
- nf := NovelFeature{Command: "op"}
- if matchNovelFeature(nf, registered) {
- t.Error("commandLeaf 'op' should not prefix-match 'options' (no hyphen boundary)")
+
+ writeTestFile(t, filepath.Join(cli, "root.go"), `package cli
+import "github.com/spf13/cobra"
+func Execute() {
+ rootCmd := &cobra.Command{Use: "tool"}
+ rootCmd.AddCommand(newAuthCmd())
+ rootCmd.AddCommand(newPortfolioCmd())
+ rootCmd.AddCommand(newDigestCmd())
+ rootCmd.AddCommand(newOptionsCmd())
+ rootCmd.AddCommand(newOptionsChainCmd())
+}
+`)
+ writeTestFile(t, filepath.Join(cli, "auth.go"), `package cli
+import "github.com/spf13/cobra"
+func newAuthCmd() *cobra.Command {
+ cmd := &cobra.Command{Use: "auth"}
+ cmd.AddCommand(newAuthLoginCmd())
+ cmd.AddCommand(newAuthLoginChromeCmd())
+ cmd.AddCommand(newAuthLogoutCmd())
+ return cmd
+}
+func newAuthLoginCmd() *cobra.Command { return &cobra.Command{Use: "login"} }
+func newAuthLoginChromeCmd() *cobra.Command { return &cobra.Command{Use: "login-chrome"} }
+func newAuthLogoutCmd() *cobra.Command { return &cobra.Command{Use: "logout"} }
+`)
+ writeTestFile(t, filepath.Join(cli, "portfolio.go"), `package cli
+import "github.com/spf13/cobra"
+func newPortfolioCmd() *cobra.Command {
+ cmd := &cobra.Command{Use: "portfolio"}
+ cmd.AddCommand(newPortfolioPerfCmd())
+ cmd.AddCommand(newPortfolioGainsCmd())
+ return cmd
+}
+func newPortfolioPerfCmd() *cobra.Command { return &cobra.Command{Use: "perf"} }
+func newPortfolioGainsCmd() *cobra.Command { return &cobra.Command{Use: "gains"} }
+`)
+ writeTestFile(t, filepath.Join(cli, "misc.go"), `package cli
+import "github.com/spf13/cobra"
+func newDigestCmd() *cobra.Command { return &cobra.Command{Use: "digest"} }
+func newOptionsCmd() *cobra.Command { return &cobra.Command{Use: "options <symbol>"} }
+func newOptionsChainCmd() *cobra.Command { return &cobra.Command{Use: "options-chain <symbol>"} }
+`)
+
+ paths, leaves := collectRegisteredCommands(dir)
+
+ wantPaths := []string{
+ "auth", "auth login", "auth login-chrome", "auth logout",
+ "portfolio", "portfolio perf", "portfolio gains",
+ "digest", "options", "options-chain",
+ }
+ for _, w := range wantPaths {
+ if !paths[w] {
+ t.Errorf("expected path %q, got paths: %v", w, paths)
+ }
+ }
+
+ wantLeaves := []string{"auth", "login", "login-chrome", "logout", "portfolio", "perf", "gains", "digest", "options", "options-chain"}
+ for _, w := range wantLeaves {
+ if !leaves[w] {
+ t.Errorf("expected leaf %q, got leaves: %v", w, leaves)
+ }
}
}
← 297c3c14 feat(scripts): add verify-skill — static SKILL.md validator
·
back to Cli Printing Press
·
fix(cli): promoted-command presence check uses promoted type 4ebfa088 →