[object Object]

← back to Cli Printing Press

fix(cli): score structural scorer behavior (#732)

979510c21b29be9e93fda3321e60154d2eb5233c · 2026-05-08 09:31:47 -0700 · Trevin Chow

Files touched

Diff

commit 979510c21b29be9e93fda3321e60154d2eb5233c
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 8 09:31:47 2026 -0700

    fix(cli): score structural scorer behavior (#732)
---
 internal/pipeline/mcp_size.go             |   3 +
 internal/pipeline/mcp_size_test.go        |  54 +++++++++
 internal/pipeline/scorecard.go            |   4 +-
 internal/pipeline/scorecard_structural.go | 195 ++++++++++++++++++++++++++++++
 internal/pipeline/scorecard_tier2_test.go | 133 ++++++++++++++++++++
 5 files changed, 387 insertions(+), 2 deletions(-)

diff --git a/internal/pipeline/mcp_size.go b/internal/pipeline/mcp_size.go
index af1a86cc..d707699c 100644
--- a/internal/pipeline/mcp_size.go
+++ b/internal/pipeline/mcp_size.go
@@ -79,6 +79,9 @@ func canonicalMCPSurfacePath(dir string) string {
 	if _, err := os.Stat(codeOrchPath); err != nil {
 		return toolsPath
 	}
+	if codeOrchDelegatedFromRegisterTools(dir) {
+		return codeOrchPath
+	}
 	mainPath := mcpMainPath(dir)
 	if mainPath == "" {
 		return toolsPath
diff --git a/internal/pipeline/mcp_size_test.go b/internal/pipeline/mcp_size_test.go
index a4d039a8..f139d610 100644
--- a/internal/pipeline/mcp_size_test.go
+++ b/internal/pipeline/mcp_size_test.go
@@ -206,6 +206,60 @@ func TestScoreMCPTokenEfficiency_PartialCreditMedium(t *testing.T) {
 // endpoint-mirror in tools.go. The token-efficiency score must reflect
 // the actual default surface, not the static tools.go.
 func TestEstimateMCPTokens_RuntimeSurfaceSelection(t *testing.T) {
+	t.Run("code orchestration delegation selects code_orch.go", func(t *testing.T) {
+		dir := t.TempDir()
+		mcpDir := filepath.Join(dir, "internal", "mcp")
+		require.NoError(t, os.MkdirAll(mcpDir, 0o755))
+
+		toolsBody := `package mcp
+
+import mcplib "github.com/mark3labs/mcp-go/mcp"
+
+func RegisterTools(s *server.MCPServer) {
+	RegisterCodeOrchestrationTools(s)
+}
+
+func helperOne() string { return "` + strings.Repeat("x", 900) + `" }
+func helperTwo() string { return "` + strings.Repeat("y", 900) + `" }
+`
+		require.NoError(t, os.WriteFile(filepath.Join(mcpDir, "tools.go"), []byte(toolsBody), 0o644))
+
+		codeOrchBody := `package mcp
+
+import mcplib "github.com/mark3labs/mcp-go/mcp"
+
+func RegisterCodeOrchestrationTools(s *server.MCPServer) {
+	s.AddTool(mcplib.NewTool("gohighlevel_search", mcplib.WithDescription("Search the API.")), nil)
+	s.AddTool(mcplib.NewTool("gohighlevel_execute", mcplib.WithDescription("Execute one endpoint.")), nil)
+}
+`
+		require.NoError(t, os.WriteFile(filepath.Join(mcpDir, "code_orch.go"), []byte(codeOrchBody), 0o644))
+
+		cmdDir := filepath.Join(dir, "cmd", "gohighlevel-pp-mcp")
+		require.NoError(t, os.MkdirAll(cmdDir, 0o755))
+		mainBody := `package main
+
+import (
+	"os"
+	mcptools "demo-pp-cli/internal/mcp"
+	"github.com/mark3labs/mcp-go/server"
+)
+
+func main() {
+	s := server.NewMCPServer("Demo", "1.0.0")
+	mcptools.RegisterTools(s)
+	_ = os.Getenv("PP_MCP_TRANSPORT")
+}
+`
+		require.NoError(t, os.WriteFile(filepath.Join(cmdDir, "main.go"), []byte(mainBody), 0o644))
+		require.NoError(t, os.WriteFile(filepath.Join(dir, ".printing-press.json"),
+			[]byte(`{"cli_name":"gohighlevel-pp-cli"}`), 0o644))
+
+		est := estimateMCPTokens(dir)
+		require.Equal(t, 2, est.ToolCount, "current code-orch CLIs delegate from RegisterTools into code_orch.go")
+		assert.Less(t, est.TotalTokens, 100)
+	})
+
 	t.Run("thin default selects code_orch.go for token counting", func(t *testing.T) {
 		dir := t.TempDir()
 		mcpDir := filepath.Join(dir, "internal", "mcp")
diff --git a/internal/pipeline/scorecard.go b/internal/pipeline/scorecard.go
index a9033592..4b42099b 100644
--- a/internal/pipeline/scorecard.go
+++ b/internal/pipeline/scorecard.go
@@ -308,7 +308,7 @@ func scoreOutputModes(dir string) int {
 		score += 2
 	}
 	// Quality tier: pagination progress events
-	if strings.Contains(helpersContent, "page_fetch") || strings.Contains(helpersContent, "ndjson") {
+	if strings.Contains(helpersContent, "page_fetch") || strings.Contains(helpersContent, "ndjson") || hasPageProgressStructure(filepath.Join(dir, "internal", "cli")) {
 		score += 1
 	}
 	// Quality tier: tabwriter for aligned output
@@ -2161,7 +2161,7 @@ func scoreSyncCorrectness(dir string) int {
 	if strings.Contains(content, "SaveSyncState") {
 		score++
 	}
-	if strings.Contains(content, "paginatedGet") || strings.Contains(content, "hasNextPage") || strings.Contains(content, "endCursor") || strings.Contains(content, "cursor") {
+	if hasSyncPaginationStructure(cliDir) {
 		score += 2
 	}
 	// URL path parameters only count when other sync signals are present,
diff --git a/internal/pipeline/scorecard_structural.go b/internal/pipeline/scorecard_structural.go
new file mode 100644
index 00000000..82a4fa51
--- /dev/null
+++ b/internal/pipeline/scorecard_structural.go
@@ -0,0 +1,195 @@
+package pipeline
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"path/filepath"
+	"slices"
+	"strconv"
+	"strings"
+)
+
+func cliGoASTFiles(dir string) []*ast.File {
+	files := listGoFiles(dir)
+	parsed := make([]*ast.File, 0, len(files))
+	for _, path := range files {
+		file, err := parser.ParseFile(token.NewFileSet(), path, nil, 0)
+		if err != nil {
+			continue
+		}
+		parsed = append(parsed, file)
+	}
+	return parsed
+}
+
+func hasSyncPaginationStructure(cliDir string) bool {
+	for _, file := range cliGoASTFiles(cliDir) {
+		var found bool
+		ast.Inspect(file, func(n ast.Node) bool {
+			if found {
+				return false
+			}
+			loop, ok := n.(*ast.ForStmt)
+			if !ok || loop.Body == nil {
+				return true
+			}
+			found = loopLooksLikePaginatedFetch(loop.Body)
+			return !found
+		})
+		if found {
+			return true
+		}
+	}
+	return false
+}
+
+func loopLooksLikePaginatedFetch(body *ast.BlockStmt) bool {
+	var hasFetchCall, hasCursorSignal, hasStateSave, hasExit bool
+	ast.Inspect(body, func(n ast.Node) bool {
+		switch node := n.(type) {
+		case *ast.CallExpr:
+			if selectorName(node.Fun, "Get", "GetWithHeaders", "Do", "Execute", "Query") || strings.Contains(strings.ToLower(callName(node.Fun)), "fetch") {
+				hasFetchCall = true
+			}
+			if selectorName(node.Fun, "SaveSyncState", "SaveSyncCursor") {
+				hasStateSave = true
+			}
+		case *ast.Ident:
+			if paginationName(node.Name) {
+				hasCursorSignal = true
+			}
+		case *ast.SelectorExpr:
+			if paginationName(node.Sel.Name) {
+				hasCursorSignal = true
+			}
+		case *ast.BasicLit:
+			if node.Kind == token.STRING && paginationLiteral(node.Value) {
+				hasCursorSignal = true
+			}
+		case *ast.BranchStmt:
+			if node.Tok == token.BREAK {
+				hasExit = true
+			}
+		case *ast.ReturnStmt:
+			hasExit = true
+		}
+		return true
+	})
+	return hasFetchCall && hasCursorSignal && (hasStateSave || hasExit)
+}
+
+func hasPageProgressStructure(cliDir string) bool {
+	for _, file := range cliGoASTFiles(cliDir) {
+		var found bool
+		ast.Inspect(file, func(n ast.Node) bool {
+			if found {
+				return false
+			}
+			loop, ok := n.(*ast.ForStmt)
+			if !ok || loop.Body == nil {
+				return true
+			}
+			found = loopPrintsPageProgress(loop.Body)
+			return !found
+		})
+		if found {
+			return true
+		}
+	}
+	return false
+}
+
+func loopPrintsPageProgress(body *ast.BlockStmt) bool {
+	var found bool
+	ast.Inspect(body, func(n ast.Node) bool {
+		if found {
+			return false
+		}
+		call, ok := n.(*ast.CallExpr)
+		if !ok || !selectorName(call.Fun, "Printf", "Fprintf", "Fprintln", "Println") {
+			return true
+		}
+		found = slices.ContainsFunc(call.Args, exprMentionsPage)
+		return !found
+	})
+	return found
+}
+
+func selectorName(expr ast.Expr, names ...string) bool {
+	got := callName(expr)
+	return slices.Contains(names, got)
+}
+
+func callName(expr ast.Expr) string {
+	switch fn := expr.(type) {
+	case *ast.SelectorExpr:
+		return fn.Sel.Name
+	case *ast.Ident:
+		return fn.Name
+	default:
+		return ""
+	}
+}
+
+func exprMentionsPage(expr ast.Expr) bool {
+	switch e := expr.(type) {
+	case *ast.BasicLit:
+		if e.Kind != token.STRING {
+			return false
+		}
+		v, err := strconv.Unquote(e.Value)
+		if err != nil {
+			v = e.Value
+		}
+		return strings.Contains(strings.ToLower(v), "page")
+	case *ast.Ident:
+		return strings.Contains(strings.ToLower(e.Name), "page")
+	case *ast.SelectorExpr:
+		return strings.Contains(strings.ToLower(e.Sel.Name), "page")
+	default:
+		return false
+	}
+}
+
+func paginationName(name string) bool {
+	name = strings.ToLower(name)
+	return strings.Contains(name, "cursor") ||
+		strings.Contains(name, "nextpage") ||
+		strings.Contains(name, "pagetoken") ||
+		strings.Contains(name, "hasmore") ||
+		strings.Contains(name, "hasnext")
+}
+
+func paginationLiteral(value string) bool {
+	v, err := strconv.Unquote(value)
+	if err != nil {
+		v = value
+	}
+	v = strings.ToLower(v)
+	return strings.Contains(v, "cursor") || strings.Contains(v, "next_page") || strings.Contains(v, "nextpage") || strings.Contains(v, "page_token") || strings.Contains(v, "pagetoken")
+}
+
+func codeOrchDelegatedFromRegisterTools(dir string) bool {
+	toolsPath := filepath.Join(dir, "internal", "mcp", "tools.go")
+	file, err := parser.ParseFile(token.NewFileSet(), toolsPath, nil, 0)
+	if err != nil {
+		return false
+	}
+	for _, decl := range file.Decls {
+		fn, ok := decl.(*ast.FuncDecl)
+		if !ok || fn.Body == nil || fn.Name == nil || fn.Name.Name != "RegisterTools" {
+			continue
+		}
+		var delegates bool
+		ast.Inspect(fn.Body, func(n ast.Node) bool {
+			if call, ok := n.(*ast.CallExpr); ok && selectorName(call.Fun, "RegisterCodeOrchestrationTools") {
+				delegates = true
+				return false
+			}
+			return true
+		})
+		return delegates
+	}
+	return false
+}
diff --git a/internal/pipeline/scorecard_tier2_test.go b/internal/pipeline/scorecard_tier2_test.go
index 0f0e99eb..2cdef980 100644
--- a/internal/pipeline/scorecard_tier2_test.go
+++ b/internal/pipeline/scorecard_tier2_test.go
@@ -300,10 +300,143 @@ func runSync(store interface {
 	paginatedGet(path, cursor)
 	store.SaveSyncState("messages", "next")
 }
+
+func paginatedGet(path, cursor string) error {
+	for {
+		params := map[string]string{}
+		if cursor != "" {
+			params["after"] = cursor
+		}
+		_, nextCursor, hasMore := fetchPage(path, params)
+		if !hasMore || nextCursor == "" {
+			break
+		}
+		cursor = nextCursor
+	}
+	return nil
+}
+`)
+
+		assert.Equal(t, 10, scoreSyncCorrectness(dir))
+	})
+
+	t.Run("scores structural pagination without canonical helper name", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeScorecardFixture(t, dir, "internal/cli/sync.go", `
+package cli
+
+func defaultSyncResources() []string {
+	return []string{"channels", "messages"}
+}
+
+func runSync(store interface {
+	GetSyncState(string) string
+	SaveSyncState(string, string)
+}, client interface {
+	Get(string, map[string]string) ([]byte, error)
+}) error {
+	path := "/guilds/{guild_id}/messages"
+	cursor := store.GetSyncState("messages")
+	for {
+		params := map[string]string{}
+		if cursor != "" {
+			params["after"] = cursor
+		}
+		data, err := client.Get(path, params)
+		if err != nil {
+			return err
+		}
+		nextCursor, hasMore := extractNextCursor(data)
+		store.SaveSyncState("messages", nextCursor)
+		if !hasMore || nextCursor == "" {
+			break
+		}
+		cursor = nextCursor
+	}
+	return nil
+}
 `)
 
 		assert.Equal(t, 10, scoreSyncCorrectness(dir))
 	})
+
+	t.Run("does not score a named pagination stub as real pagination", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeScorecardFixture(t, dir, "internal/cli/sync.go", `
+package cli
+
+func defaultSyncResources() []string {
+	return []string{"messages"}
+}
+
+func runSync(store interface {
+	GetSyncState(string) string
+	SaveSyncState(string, string)
+}) {
+	path := "/guilds/{guild_id}/messages"
+	cursor := store.GetSyncState("messages")
+	paginatedGet(path, cursor)
+	store.SaveSyncState("messages", "next")
+}
+
+func paginatedGet(path, cursor string) error {
+	return nil
+}
+`)
+
+		assert.Less(t, scoreSyncCorrectness(dir), 10)
+	})
+}
+
+func TestScoreOutputModes(t *testing.T) {
+	t.Run("scores structural page progress without page_fetch literal", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeScorecardFixture(t, dir, "internal/cli/root.go", `
+package cli
+
+func init() {
+	rootCmd.PersistentFlags().Bool("json", false, "JSON")
+	rootCmd.PersistentFlags().Bool("plain", false, "Plain")
+	rootCmd.PersistentFlags().String("select", "", "Select")
+	rootCmd.PersistentFlags().Bool("csv", false, "CSV")
+	rootCmd.PersistentFlags().Bool("quiet", false, "Quiet")
+}
+`)
+		writeScorecardFixture(t, dir, "internal/cli/helpers.go", `
+package cli
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"text/tabwriter"
+)
+
+func filterFields(data json.RawMessage, fields string) json.RawMessage {
+	var v any
+	_ = json.Unmarshal(data, &v)
+	return data
+}
+
+func fetchEveryPage() {
+	page := 0
+	for {
+		page++
+		fmt.Fprintf(os.Stderr, "fetching page %d...\n", page)
+		break
+	}
+}
+
+func newTabWriter() *tabwriter.Writer {
+	return tabwriter.NewWriter(os.Stdout, 2, 4, 2, ' ', 0)
+}
+`)
+
+		assert.Equal(t, 10, scoreOutputModes(dir))
+	})
 }
 
 func TestScorePathValidity(t *testing.T) {

← a4b0eb30 fix(cli): anchor openapi loader normalization (#730)  ·  back to Cli Printing Press  ·  fix(cli): handle wrapped paginated responses (#731) b4bdcf68 →