[object Object]

← back to Cli Printing Press

fix(cli): scorer accuracy — insight prefix, MCP dir selection, freshness coupling (#486)

9f265510549d5563804d13872301f9b94831f6b5 · 2026-05-01 21:29:33 -0700 · Trevin Chow

* fix(cli): scorer accuracy — insight prefix, MCP dir selection, freshness coupling

Closes #479. Three independent scorer bugs surfaced by the producthunt v3.2.1 redo:

- scoreInsight now credits files whose Cobra Use: matches a leaf in the
  manifest's novel_features[].command. The prefix list cannot enumerate every
  reasonable insight verb across APIs (trajectory, snapshot, calendar,
  lookalike, watch, context, etc.), and renaming would game the scorer per
  polish guidance. The agent's planning-time classification (dogfood-verified
  into .printing-press.json) is the stronger signal.
- detectMCPSurface now resolves the canonical cmd/<cli>-pp-mcp/ via the
  manifest's cli_name (replacing -pp-cli with -pp-mcp). The previous
  os.ReadDir + suffix-and-break loop picked lexically-first dirs, so a stale
  shadow dir left by spec/name rewrites would shadow the canonical surface
  and report stale transport assessments.
- scoreCacheFreshness drops the share export/import component (d). Sharing
  is a separate feature, not a freshness contract; coupling them capped a
  fully-freshness-equipped CLI at 8/10. Auto-refresh's weight is bumped from
  3 to 5 to keep the dimension's max at 10.

Tests: new TestScoreInsight cases for manifest-credit + registration gate,
new TestScoreMCPRemoteTransport cases for canonical-dir selection + manifest-
absent fallback, updated TestScoreCacheFreshness to assert max-without-share
and share-without-store-zero.

* refactor(cli): simplify scorer fixes — naming pkg, shared regex, single stat

Post-review cleanup of the #479 fixes:

- mcpMainPath: use naming.CurrentCLISuffix / naming.MCP / naming.TrimCLISuffix /
  naming.MCPSuffix instead of literal "-pp-cli"/"-pp-mcp" string ops; drop the
  redundant cmdDir parameter (derive inside); always stat the candidate so the
  caller doesn't need a second os.Stat.
- detectMCPSurface: drop the now-unnecessary post-stat — mcpMainPath only
  returns existing paths.
- manifestNovelFeatureLeaves: use the existing CLIManifest type instead of an
  inline anonymous unmarshal struct; collapse the redundant intermediate var.
- cobraUseLeafRe: lift to package-level var (compiled once instead of per
  scoreInsight call).
- Trim noisy "Signal N (existing/planning-aware)" parentheticals and the
  verbose novelLeaves preamble — the function's doc comment carries the why.
- Test: rephrase "the old suffix-and-break loop would pick" comment to
  describe the invariant being asserted, not the bug being fixed.

Files touched

Diff

commit 9f265510549d5563804d13872301f9b94831f6b5
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 1 21:29:33 2026 -0700

    fix(cli): scorer accuracy — insight prefix, MCP dir selection, freshness coupling (#486)
    
    * fix(cli): scorer accuracy — insight prefix, MCP dir selection, freshness coupling
    
    Closes #479. Three independent scorer bugs surfaced by the producthunt v3.2.1 redo:
    
    - scoreInsight now credits files whose Cobra Use: matches a leaf in the
      manifest's novel_features[].command. The prefix list cannot enumerate every
      reasonable insight verb across APIs (trajectory, snapshot, calendar,
      lookalike, watch, context, etc.), and renaming would game the scorer per
      polish guidance. The agent's planning-time classification (dogfood-verified
      into .printing-press.json) is the stronger signal.
    - detectMCPSurface now resolves the canonical cmd/<cli>-pp-mcp/ via the
      manifest's cli_name (replacing -pp-cli with -pp-mcp). The previous
      os.ReadDir + suffix-and-break loop picked lexically-first dirs, so a stale
      shadow dir left by spec/name rewrites would shadow the canonical surface
      and report stale transport assessments.
    - scoreCacheFreshness drops the share export/import component (d). Sharing
      is a separate feature, not a freshness contract; coupling them capped a
      fully-freshness-equipped CLI at 8/10. Auto-refresh's weight is bumped from
      3 to 5 to keep the dimension's max at 10.
    
    Tests: new TestScoreInsight cases for manifest-credit + registration gate,
    new TestScoreMCPRemoteTransport cases for canonical-dir selection + manifest-
    absent fallback, updated TestScoreCacheFreshness to assert max-without-share
    and share-without-store-zero.
    
    * refactor(cli): simplify scorer fixes — naming pkg, shared regex, single stat
    
    Post-review cleanup of the #479 fixes:
    
    - mcpMainPath: use naming.CurrentCLISuffix / naming.MCP / naming.TrimCLISuffix /
      naming.MCPSuffix instead of literal "-pp-cli"/"-pp-mcp" string ops; drop the
      redundant cmdDir parameter (derive inside); always stat the candidate so the
      caller doesn't need a second os.Stat.
    - detectMCPSurface: drop the now-unnecessary post-stat — mcpMainPath only
      returns existing paths.
    - manifestNovelFeatureLeaves: use the existing CLIManifest type instead of an
      inline anonymous unmarshal struct; collapse the redundant intermediate var.
    - cobraUseLeafRe: lift to package-level var (compiled once instead of per
      scoreInsight call).
    - Trim noisy "Signal N (existing/planning-aware)" parentheticals and the
      verbose novelLeaves preamble — the function's doc comment carries the why.
    - Test: rephrase "the old suffix-and-break loop would pick" comment to
      describe the invariant being asserted, not the bug being fixed.
---
 internal/pipeline/mcp_scoring.go          | 50 +++++++++++-----
 internal/pipeline/mcp_scoring_test.go     | 23 ++++++++
 internal/pipeline/scorecard.go            | 67 ++++++++++++++++-----
 internal/pipeline/scorecard_tier2_test.go | 97 ++++++++++++++++++++++++++++++-
 4 files changed, 203 insertions(+), 34 deletions(-)

diff --git a/internal/pipeline/mcp_scoring.go b/internal/pipeline/mcp_scoring.go
index 8ff45409..d75a95f2 100644
--- a/internal/pipeline/mcp_scoring.go
+++ b/internal/pipeline/mcp_scoring.go
@@ -4,6 +4,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+
+	"github.com/mvanhorn/cli-printing-press/v3/internal/naming"
 )
 
 // Thresholds for the tool-design dimension. Intent-grouping is only worth
@@ -31,35 +33,51 @@ type mcpSurface struct {
 	intentTools     int    // count of intent tool registrations inside intents.go
 }
 
-// detectMCPSurface walks the printed CLI's generated layout and returns a
-// summary of the MCP surface. A nil return (present=false) signals that the
-// CLI does not emit an MCP server — the caller should mark every MCP-only
-// dimension as unscored.
-func detectMCPSurface(dir string) mcpSurface {
-	var s mcpSurface
-
-	// Find the cmd/<name>-pp-mcp/main.go by matching the suffix rather than
-	// deriving the name; keeps this scorer decoupled from naming.CLI / naming.MCP.
+// mcpMainPath returns the path to the canonical cmd/<cli>-pp-mcp/main.go, or
+// "" when none exists. Prefers the dir derived from .printing-press.json's
+// cli_name so duplicate or shadow dirs left behind by spec/name rewrites
+// can't shadow the canonical surface. Falls back to the first dir matching
+// naming.MCPSuffix when the manifest is missing — preserves the legacy
+// behavior for fixtures and trees that never had a manifest written.
+//
+// Always stats the candidate before returning so callers can trust the path.
+func mcpMainPath(dir string) string {
 	cmdDir := filepath.Join(dir, "cmd")
+	if cli := ReadCLIBinaryName(dir); strings.HasSuffix(cli, naming.CurrentCLISuffix) {
+		candidate := filepath.Join(cmdDir, naming.MCP(naming.TrimCLISuffix(cli)), "main.go")
+		if _, err := os.Stat(candidate); err == nil {
+			return candidate
+		}
+	}
 	entries, err := os.ReadDir(cmdDir)
 	if err != nil {
-		return s
+		return ""
 	}
 	for _, e := range entries {
 		if !e.IsDir() {
 			continue
 		}
-		if strings.HasSuffix(e.Name(), "-pp-mcp") {
-			s.mainPath = filepath.Join(cmdDir, e.Name(), "main.go")
-			if _, err := os.Stat(s.mainPath); err == nil {
-				s.present = true
+		if strings.HasSuffix(e.Name(), naming.MCPSuffix) {
+			candidate := filepath.Join(cmdDir, e.Name(), "main.go")
+			if _, err := os.Stat(candidate); err == nil {
+				return candidate
 			}
-			break
 		}
 	}
-	if !s.present {
+	return ""
+}
+
+// detectMCPSurface walks the printed CLI's generated layout and returns a
+// summary of the MCP surface. A nil return (present=false) signals that the
+// CLI does not emit an MCP server — the caller should mark every MCP-only
+// dimension as unscored.
+func detectMCPSurface(dir string) mcpSurface {
+	var s mcpSurface
+	s.mainPath = mcpMainPath(dir)
+	if s.mainPath == "" {
 		return s
 	}
+	s.present = true
 
 	s.toolsPath = filepath.Join(dir, "internal", "mcp", "tools.go")
 	if data, err := os.ReadFile(s.toolsPath); err == nil {
diff --git a/internal/pipeline/mcp_scoring_test.go b/internal/pipeline/mcp_scoring_test.go
index 284ba2ed..edf5ab48 100644
--- a/internal/pipeline/mcp_scoring_test.go
+++ b/internal/pipeline/mcp_scoring_test.go
@@ -80,6 +80,29 @@ func TestScoreMCPRemoteTransport(t *testing.T) {
 		assert.True(t, scored)
 		assert.Equal(t, 10, score)
 	})
+
+	t.Run("manifest cli_name selects canonical mcp dir over duplicates", func(t *testing.T) {
+		dir := t.TempDir()
+		// Duplicate -pp-mcp dirs must not shadow the canonical one. The
+		// duplicate sorts lexically first, so a suffix-only scan would pick
+		// it and read the wrong main.go.
+		writeMCPFile(t, dir, "cmd/demo-pp-cli-pp-mcp/main.go", stdioOnlyMain)
+		writeMCPFile(t, dir, "cmd/demo-pp-mcp/main.go", bothTransportsMain)
+		writeMCPFile(t, dir, ".printing-press.json", `{"cli_name": "demo-pp-cli"}`)
+
+		score, scored := scoreMCPRemoteTransport(dir)
+		assert.True(t, scored)
+		assert.Equal(t, 10, score, "scorer must read the canonical demo-pp-mcp main.go, not the lexically-first duplicate")
+	})
+
+	t.Run("falls back to suffix scan when manifest is missing", func(t *testing.T) {
+		dir := t.TempDir()
+		writeMCPFile(t, dir, "cmd/demo-pp-mcp/main.go", bothTransportsMain)
+
+		score, scored := scoreMCPRemoteTransport(dir)
+		assert.True(t, scored)
+		assert.Equal(t, 10, score, "no manifest → legacy suffix scan still works")
+	})
 }
 
 // buildToolsGo fabricates an internal/mcp/tools.go containing `n` endpoint
diff --git a/internal/pipeline/scorecard.go b/internal/pipeline/scorecard.go
index cc1667f8..1c773f79 100644
--- a/internal/pipeline/scorecard.go
+++ b/internal/pipeline/scorecard.go
@@ -810,7 +810,7 @@ func scoreLocalCache(dir string) int {
 // scoreCacheFreshness rewards the discrawl-inspired Phase 1-3 capabilities.
 // Returns (score, true) when the CLI has a local store; (0, false) otherwise
 // so the dimension is marked N/A and excluded from the tier1 denominator.
-// Each sub-capability is worth 2-3 points; the total is capped at 10.
+// Each sub-capability is worth 2-5 points; the total is capped at 10.
 //
 // The signals are string-matched against well-known template artifacts
 // rather than imported symbols because the scorer must not depend on
@@ -841,23 +841,15 @@ func scoreCacheFreshness(dir string) (int, bool) {
 
 	// (c) Auto-refresh wired: the PersistentPreRunE hook invokes
 	// autoRefreshIfStale, and the cliutil freshness helper exists. Worth
-	// 3 pts because this is the user's headline ask.
+	// 5 pts because this is the user's headline ask — a CLI that ships
+	// schema gating + doctor cache + auto-refresh has perfect freshness
+	// behavior and should top the dimension out at 10/10.
 	autoRefreshPath := filepath.Join(dir, "internal", "cli", "auto_refresh.go")
 	autoRefreshContent := readFileContent(autoRefreshPath)
 	freshnessPath := filepath.Join(dir, "internal", "cliutil", "freshness.go")
 	freshnessContent := readFileContent(freshnessPath)
 	if strings.Contains(autoRefreshContent, "autoRefreshIfStale") && strings.Contains(freshnessContent, "EnsureFresh") {
-		score += 3
-	}
-
-	// (d) Share export/import present: internal/share/share.go + share
-	// subcommands provide the git-backed sharing surface.
-	sharePath := filepath.Join(dir, "internal", "share", "share.go")
-	shareContent := readFileContent(sharePath)
-	shareCmdsPath := filepath.Join(dir, "internal", "cli", "share_commands.go")
-	shareCmdsContent := readFileContent(shareCmdsPath)
-	if strings.Contains(shareContent, "func Export") && strings.Contains(shareCmdsContent, "newShareCmd") {
-		score += 2
+		score += 5
 	}
 
 	if score > 10 {
@@ -1176,6 +1168,36 @@ func scoreVision(dir string) int {
 	return score
 }
 
+// cobraUseLeafRe extracts the first whitespace-delimited token from a Cobra
+// `Use: "..."` literal — the leaf command name (e.g., `"trajectory <slug>"`
+// → `trajectory`).
+var cobraUseLeafRe = regexp.MustCompile(`Use:\s*"([^"\s]+)`)
+
+// manifestNovelFeatureLeaves returns the leaves of every novel_features[].command
+// in dir/.printing-press.json. Returns nil when the manifest is missing,
+// unparseable, or carries no novel features.
+func manifestNovelFeatureLeaves(dir string) map[string]bool {
+	data, err := os.ReadFile(filepath.Join(dir, CLIManifestFilename))
+	if err != nil {
+		return nil
+	}
+	var m CLIManifest
+	if err := json.Unmarshal(data, &m); err != nil {
+		return nil
+	}
+	if len(m.NovelFeatures) == 0 {
+		return nil
+	}
+	leaves := make(map[string]bool, len(m.NovelFeatures))
+	for _, nf := range m.NovelFeatures {
+		_, leaf := splitCommandPath(commandPath(nf.Command))
+		if leaf != "" {
+			leaves[leaf] = true
+		}
+	}
+	return leaves
+}
+
 // registeredCommandFiles returns the set of cli/*.go filenames whose command
 // constructor is referenced by root.go. Files without a registered constructor
 // should not inflate workflow/insight scores even if they match prefix or
@@ -1341,6 +1363,11 @@ func scoreInsight(dir string) int {
 		"stats", "conflicts", "stale", "analytics", "busiest", "velocity",
 		"utilization", "coverage", "gaps", "noshow"}
 
+	// novelLeaves credits files whose Cobra Use matches an agent-declared
+	// transcendence command. The prefix list above can't enumerate every
+	// reasonable insight verb across APIs.
+	novelLeaves := manifestNovelFeatureLeaves(dir)
+
 	found := 0
 	for _, e := range entries {
 		if e.IsDir() || !strings.HasSuffix(e.Name(), ".go") {
@@ -1354,7 +1381,7 @@ func scoreInsight(dir string) int {
 		}
 		name := strings.ToLower(e.Name())
 
-		// Signal 1: filename prefix match (supplementary — kept for backward compat)
+		// Signal 1: filename prefix match
 		prefixMatch := false
 		for _, prefix := range insightPrefixes {
 			if strings.HasPrefix(name, prefix) {
@@ -1372,7 +1399,15 @@ func scoreInsight(dir string) int {
 			continue
 		}
 
-		// Signal 2 (existing): store + SQL aggregation
+		// Signal 2: file declares a Cobra Use: matching an agent-declared novel feature.
+		if len(novelLeaves) > 0 {
+			if m := cobraUseLeafRe.FindStringSubmatch(content); m != nil && novelLeaves[strings.ToLower(m[1])] {
+				found++
+				continue
+			}
+		}
+
+		// Signal 3: store + SQL aggregation
 		usesStore := strings.Contains(content, "/store") || strings.Contains(content, "store.Open") || strings.Contains(content, "store.New")
 		rateRe := regexp.MustCompile(`\brate\b|\bRate\b`)
 		hasSQLAgg := strings.Contains(content, "COUNT(") || strings.Contains(content, "SUM(") ||
@@ -1383,7 +1418,7 @@ func scoreInsight(dir string) int {
 			continue
 		}
 
-		// Signal 3 (new): behavioral — command produces derived/aggregated output.
+		// Signal 4: behavioral — command produces derived/aggregated output.
 		// Detects Go-level aggregation: sorting, percentage calculations, comparisons,
 		// summary statistics. Requires multi-source input (2+ API calls or store usage)
 		// to avoid counting simple pass-through commands.
diff --git a/internal/pipeline/scorecard_tier2_test.go b/internal/pipeline/scorecard_tier2_test.go
index 297d2192..30b0358d 100644
--- a/internal/pipeline/scorecard_tier2_test.go
+++ b/internal/pipeline/scorecard_tier2_test.go
@@ -1026,7 +1026,7 @@ func Open() {}`)
 		assert.Equal(t, 0, score)
 	})
 
-	t.Run("schema version + doctor + auto-refresh + share scores 10", func(t *testing.T) {
+	t.Run("schema version + doctor + auto-refresh scores 10", func(t *testing.T) {
 		dir := t.TempDir()
 		writeScorecardFixture(t, dir, "internal/store/store.go", `package store
 
@@ -1044,6 +1044,17 @@ func autoRefreshIfStale() {}`)
 		writeScorecardFixture(t, dir, "internal/cliutil/freshness.go", `package cliutil
 
 func EnsureFresh() {}`)
+
+		score, scored := scoreCacheFreshness(dir)
+		assert.True(t, scored)
+		assert.Equal(t, 10, score, "all three freshness signals should top the dimension out without an unrelated share feature")
+	})
+
+	t.Run("share feature does not influence freshness", func(t *testing.T) {
+		dir := t.TempDir()
+		writeScorecardFixture(t, dir, "internal/store/store.go", `package store
+
+func Open() {}`)
 		writeScorecardFixture(t, dir, "internal/share/share.go", `package share
 
 func Export() {}`)
@@ -1053,7 +1064,7 @@ func newShareCmd() {}`)
 
 		score, scored := scoreCacheFreshness(dir)
 		assert.True(t, scored)
-		assert.Equal(t, 10, score)
+		assert.Equal(t, 0, score, "share is a separate feature and must not award freshness points")
 	})
 
 	t.Run("schema version + doctor only scores 5", func(t *testing.T) {
@@ -1257,6 +1268,88 @@ func runLookup(db *store.DB) {
 
 		assert.Equal(t, 0, scoreInsight(dir))
 	})
+
+	t.Run("credits manifest novel features whose Use leaves don't match prefix list", func(t *testing.T) {
+		dir := t.TempDir()
+
+		// root.go registers each constructor so registeredCommandFiles can
+		// verify the files are actually wired in.
+		writeScorecardFixture(t, dir, "internal/cli/root.go", `package cli
+
+func register() {
+	rootCmd.AddCommand(newTrajectoryCmd())
+	rootCmd.AddCommand(newSnapshotCmd())
+	rootCmd.AddCommand(newCalendarCmd())
+	rootCmd.AddCommand(newLookalikeCmd())
+}`)
+
+		// None of these names match insightPrefixes; they would score 0 today
+		// without manifest crediting.
+		writeScorecardFixture(t, dir, "internal/cli/posts_trajectory.go", `package cli
+
+func newTrajectoryCmd() *cobra.Command {
+	return &cobra.Command{Use: "trajectory <slug>"}
+}`)
+		writeScorecardFixture(t, dir, "internal/cli/category_snapshot.go", `package cli
+
+func newSnapshotCmd() *cobra.Command {
+	return &cobra.Command{Use: "snapshot <slug>"}
+}`)
+		writeScorecardFixture(t, dir, "internal/cli/launches_calendar.go", `package cli
+
+func newCalendarCmd() *cobra.Command {
+	return &cobra.Command{Use: "calendar"}
+}`)
+		writeScorecardFixture(t, dir, "internal/cli/posts_lookalike.go", `package cli
+
+func newLookalikeCmd() *cobra.Command {
+	return &cobra.Command{Use: "lookalike <slug>"}
+}`)
+
+		// Manifest declares all four as novel features.
+		writeScorecardFixture(t, dir, ".printing-press.json", `{
+  "schema_version": 1,
+  "api_name": "demo",
+  "cli_name": "demo-pp-cli",
+  "novel_features": [
+    {"name": "Trajectory", "command": "posts trajectory", "description": "x"},
+    {"name": "Snapshot", "command": "category snapshot", "description": "x"},
+    {"name": "Calendar", "command": "launches calendar", "description": "x"},
+    {"name": "Lookalike", "command": "posts lookalike", "description": "x"}
+  ]
+}`)
+
+		assert.Equal(t, 8, scoreInsight(dir), "4 novel-feature commands should map to 8/10 via the found→score table")
+	})
+
+	t.Run("ignores manifest novel features whose files are not registered", func(t *testing.T) {
+		dir := t.TempDir()
+
+		// root.go registers a different command — registeredCommandFiles is
+		// non-empty, so the registration filter applies and orphaned files
+		// must be skipped even when they appear in novel_features.
+		writeScorecardFixture(t, dir, "internal/cli/root.go", `package cli
+
+func register() {
+	rootCmd.AddCommand(newOtherCmd())
+}`)
+		writeScorecardFixture(t, dir, "internal/cli/other.go", `package cli
+
+func newOtherCmd() *cobra.Command {
+	return &cobra.Command{Use: "other"}
+}`)
+		writeScorecardFixture(t, dir, "internal/cli/posts_trajectory.go", `package cli
+
+func newTrajectoryCmd() *cobra.Command {
+	return &cobra.Command{Use: "trajectory"}
+}`)
+		writeScorecardFixture(t, dir, ".printing-press.json", `{
+  "cli_name": "demo-pp-cli",
+  "novel_features": [{"name": "T", "command": "posts trajectory", "description": "x"}]
+}`)
+
+		assert.Equal(t, 0, scoreInsight(dir), "novel features must still pass the registeredCommandFiles gate")
+	})
 }
 
 func TestEvaluateAuthProtocol_InferredAuth(t *testing.T) {

← 40cefcb5 feat(cli): add curated Shopify wrapper spec (#476)  ·  back to Cli Printing Press  ·  fix(cli): mcp-sync stops conflating API slug with binary nam 2899613d →