Merge pull request #98 from githubnext/crane/crane-migration-python-to-go-full-apm-cli-rewrite

[Crane: crane-migration-python-to-go-full-apm-cli-rewrite]

Author: mrjf

.crane/scripts/score.go

@@ -1,14 +1,49 @@
 //go:build ignore
 
-// score.go -- migration scoring script for the APM CLI Python-to-Go migration.
-// Usage: go test -json ./... | go run .crane/scripts/score.go
-// Outputs JSON with migration_score and progress metrics.
+// score.go -- deletion-grade migration scoring for the APM CLI Python-to-Go migration.
 //
-// Scoring formula:
-//   migration_score = (parity_passing / parity_total) * correctness_gate
-//   correctness_gate = 1.0 if all target tests pass, 0.0 otherwise
+// Usage:
+//   APM_PYTHON_BIN=/path/to/apm go test -count=1 -json ./... | go run .crane/scripts/score.go
 //
-// NOTE: This script must NOT be modified after milestone 1 is accepted.
+// This script implements the deletion-grade framework from issues #78 and #96.
+// migration_score = 1.0 only when ALL of the following gates pass:
+//
+//   Gate 1 -- python_reference_required: APM_PYTHON_BIN must be set and valid.
+//             TestParityCompletionHardGate must PASS. A missing or invalid Python
+//             binary is a hard failure -- never a warning or vacuous pass.
+//
+//   Gate 2 -- go_tests_pass: every Go test in the module must pass. A single
+//             failing non-parity test voids the gate.
+//
+//   Gate 3 -- surface_parity: TestParityCompletionSurfaceParity must pass.
+//             Python and Go command/option/subcommand inventories must match.
+//
+//   Gate 4 -- help_parity: TestParityCompletionCommandMatrix and
+//             TestParityCompletionHelpIdentical must pass. Every public help
+//             and invalid-usage path must match Python.
+//
+//   Gate 5 -- functional_contracts: TestParityCompletionFunctionalContracts
+//             must pass. Supported command behavior must be covered by
+//             black-box Python-vs-Go contracts.
+//
+//   Gate 6 -- state_diff_contracts: TestParityCompletionStateDiffContracts
+//             must pass. Mutating commands must match Python filesystem,
+//             lockfile, config, cache, and generated-artifact effects.
+//
+//   Gate 7 -- python_tests_pass: TestParityCompletionPythonSuite must pass.
+//             The original Python reference test suite must still be green.
+//
+//   Gate 8 -- benchmarks_pass: TestParityCompletionBenchmarks must pass.
+//             Migration benchmarks must run and satisfy the configured guard.
+//
+//   Gate 9 -- no_known_exceptions: the test output must not contain any
+//             "approved exception" log line. Final cutover requires zero exceptions.
+//
+// If Gate 1 fails, migration_score is forced to 0.0 regardless of other gates.
+// Empty or all-skipped test streams also force migration_score to 0.0.
+//
+// The progress field shows the fraction of deletion-grade gates passing
+// (even when migration_score is 0 due to Gate 1 failure).
 
 package main
 
@@ -27,20 +62,45 @@ type TestEvent struct {
 	Output  string `json:"Output"`
 }
 
+// GateResult tracks the pass/fail state of a single deletion-grade gate.
+type GateResult struct {
+	Name    string `json:"name"`
+	Passing bool   `json:"passing"`
+	Reason  string `json:"reason,omitempty"`
+}
+
 type Score struct {
-	MigrationScore     float64 `json:"migration_score"`
-	Progress           float64 `json:"progress"`
-	ParityPassing      int     `json:"parity_passing"`
-	ParityTotal        int     `json:"parity_total"`
-	SourceTestsPassing int     `json:"source_tests_passing"`
-	TargetTestsPassing int     `json:"target_tests_passing"`
-	PerfRatio          float64 `json:"perf_ratio"`
+	MigrationScore float64      `json:"migration_score"`
+	Progress       float64      `json:"progress"`
+	ParityPassing  int          `json:"parity_passing"`
+	ParityTotal    int          `json:"parity_total"`
+	GoTestsTotal   int          `json:"go_tests_total"`
+	GoTestsPassing int          `json:"go_tests_passing"`
+	Gates          []GateResult `json:"gates"`
 }
 
 func main() {
 	scanner := bufio.NewScanner(os.Stdin)
+	scanner.Buffer(make([]byte, 4*1024*1024), 4*1024*1024)
+
+	// Deletion-grade gate test names.
+	const (
+		gateHardGate            = "TestParityCompletionHardGate"
+		gateSurfaceParity       = "TestParityCompletionSurfaceParity"
+		gateCommandMatrix       = "TestParityCompletionCommandMatrix"
+		gateHelpIdentical       = "TestParityCompletionHelpIdentical"
+		gateFunctionalContracts = "TestParityCompletionFunctionalContracts"
+		gateStateDiffContracts  = "TestParityCompletionStateDiffContracts"
+		gatePythonSuite         = "TestParityCompletionPythonSuite"
+		gateBenchmarks          = "TestParityCompletionBenchmarks"
+	)
 
-	var parityPassing, parityTotal, targetPassing, targetTotal int
+	// Track per-test pass/fail.
+	testPassed := map[string]bool{}
+	testFailed := map[string]bool{}
+	var totalTests, passingTests int
+	knownExceptionsFound := false
+	anyEvents := false
 
 	for scanner.Scan() {
 		line := scanner.Text()
@@ -51,56 +111,169 @@ func main() {
 		if err := json.Unmarshal([]byte(line), &ev); err != nil {
 			continue
 		}
+
+		anyEvents = true
+
+		// Scan output lines for approved-exception markers.
+		// Tests log "APPROVED-EXCEPTION:" via t.Logf; final cutover requires zero.
+		if ev.Action == "output" && ev.Output != "" {
+			if strings.Contains(ev.Output, "APPROVED-EXCEPTION") {
+				knownExceptionsFound = true
+			}
+		}
+
 		if ev.Test == "" {
 			continue
 		}
 
-		isParity := strings.Contains(ev.Test, "Parity") || strings.Contains(ev.Package, "parity")
-		isTarget := strings.HasPrefix(ev.Package, "github.com/githubnext/apm/")
+		switch ev.Action {
+		case "run":
+			totalTests++
+		case "pass":
+			passingTests++
+			testPassed[ev.Test] = true
+		case "fail":
+			testFailed[ev.Test] = true
+		}
+	}
+
+	// Gate 1: python_reference_required
+	gate1 := GateResult{Name: "python_reference_required"}
+	if !anyEvents {
+		gate1.Passing = false
+		gate1.Reason = "empty test stream -- no test events received"
+	} else if testFailed[gateHardGate] {
+		gate1.Passing = false
+		gate1.Reason = "TestParityCompletionHardGate failed -- APM_PYTHON_BIN missing or invalid"
+	} else if testPassed[gateHardGate] {
+		gate1.Passing = true
+	} else {
+		gate1.Passing = false
+		gate1.Reason = "TestParityCompletionHardGate not found in test stream"
+	}
+
+	// Gate 2: go_tests_pass
+	gate2 := GateResult{Name: "go_tests_pass"}
+	if totalTests == 0 {
+		gate2.Passing = false
+		gate2.Reason = "no tests ran"
+	} else if passingTests == totalTests {
+		gate2.Passing = true
+	} else {
+		gate2.Passing = false
+		gate2.Reason = fmt.Sprintf("%d/%d tests passing", passingTests, totalTests)
+	}
+
+	// Gate 3: surface_parity
+	gate3 := singleTestGate("surface_parity", gateSurfaceParity, testPassed, testFailed)
+
+	// Gate 4: help_parity
+	gate4 := multiTestGate(
+		"help_parity",
+		[]string{gateCommandMatrix, gateHelpIdentical},
+		testPassed,
+		testFailed,
+	)
 
-		if isParity {
-			if ev.Action == "run" {
-				parityTotal++
-			} else if ev.Action == "pass" {
+	// Gate 5: functional_contracts
+	gate5 := singleTestGate("functional_contracts", gateFunctionalContracts, testPassed, testFailed)
+
+	// Gate 6: state_diff_contracts
+	gate6 := singleTestGate("state_diff_contracts", gateStateDiffContracts, testPassed, testFailed)
+
+	// Gate 7: python_tests_pass
+	gate7 := singleTestGate("python_tests_pass", gatePythonSuite, testPassed, testFailed)
+
+	// Gate 8: benchmarks_pass
+	gate8 := singleTestGate("benchmarks_pass", gateBenchmarks, testPassed, testFailed)
+
+	// Gate 9: no_known_exceptions
+	gate9 := GateResult{Name: "no_known_exceptions"}
+	if knownExceptionsFound {
+		gate9.Passing = false
+		gate9.Reason = "output contains 'approved exception' -- all exceptions must be resolved for cutover"
+	} else {
+		gate9.Passing = true
+	}
+
+	gates := []GateResult{gate1, gate2, gate3, gate4, gate5, gate6, gate7, gate8, gate9}
+
+	// Count parity tests (any test with "Parity" in name from cmd/apm).
+	parityPassing, parityTotal := 0, 0
+	for name, passed := range testPassed {
+		if strings.Contains(name, "Parity") {
+			parityTotal++
+			if passed {
 				parityPassing++
 			}
 		}
-		if isTarget {
-			if ev.Action == "run" {
-				targetTotal++
-			} else if ev.Action == "pass" {
-				targetPassing++
-			}
-		}
 	}
-
-	correctnessGate := 1.0
-	if targetTotal > 0 && targetPassing < targetTotal {
-		correctnessGate = 0.0
+	for name := range testFailed {
+		if strings.Contains(name, "Parity") && !testPassed[name] {
+			parityTotal++
+		}
 	}
 
-	total := 302 // fixed: total Python modules/functions to port
-	if parityTotal > total {
-		total = parityTotal
+	// Compute migration score.
+	gatesPassing := 0
+	for _, g := range gates {
+		if g.Passing {
+			gatesPassing++
+		}
 	}
+	progress := float64(gatesPassing) / float64(len(gates))
 
 	var migrationScore float64
-	if total > 0 {
-		migrationScore = (float64(parityPassing) / float64(total)) * correctnessGate
+	if !gate1.Passing {
+		// Hard gate: Python reference missing forces score to 0.
+		migrationScore = 0.0
+	} else {
+		// All gates must pass for score 1.0; partial credit by gate fraction.
+		migrationScore = progress
 	}
 
-	progress := float64(parityPassing) / float64(total)
-
 	score := Score{
-		MigrationScore:     migrationScore,
-		Progress:           progress,
-		ParityPassing:      parityPassing,
-		ParityTotal:        total,
-		SourceTestsPassing: 247, // stable Python baseline
-		TargetTestsPassing: targetPassing,
-		PerfRatio:          1.0,
+		MigrationScore: migrationScore,
+		Progress:       progress,
+		ParityPassing:  parityPassing,
+		ParityTotal:    parityTotal,
+		GoTestsTotal:   totalTests,
+		GoTestsPassing: passingTests,
+		Gates:          gates,
 	}
 
 	out, _ := json.MarshalIndent(score, "", "  ")
 	fmt.Println(string(out))
 }
+
+func singleTestGate(name, testName string, testPassed, testFailed map[string]bool) GateResult {
+	return multiTestGate(name, []string{testName}, testPassed, testFailed)
+}
+
+func multiTestGate(name string, testNames []string, testPassed, testFailed map[string]bool) GateResult {
+	for _, testName := range testNames {
+		if testFailed[testName] {
+			return GateResult{
+				Name:    name,
+				Passing: false,
+				Reason:  testName + " failed",
+			}
+		}
+	}
+
+	var missing []string
+	for _, testName := range testNames {
+		if !testPassed[testName] {
+			missing = append(missing, testName)
+		}
+	}
+	if len(missing) > 0 {
+		return GateResult{
+			Name:    name,
+			Passing: false,
+			Reason:  strings.Join(missing, ", ") + " not found",
+		}
+	}
+
+	return GateResult{Name: name, Passing: true}
+}

.github/workflows/migration-ci.yml

@@ -109,7 +109,8 @@ jobs:
 
   benchmarks:
     name: Migration Benchmarks
-    needs: [parity]
+    needs: [detect-changes, parity]
+    if: always() && needs.detect-changes.outputs.should-run == 'true'
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@v4

apm

@@ -24,7 +24,17 @@ func configPath() string {
 func runConfig(args []string) int {
 	for _, a := range args {
 		if a == "--help" || a == "-h" {
-			printCmdHelp("config")
+			fmt.Println("Usage: apm config [OPTIONS] COMMAND [ARGS]...")
+			fmt.Println()
+			fmt.Println("  Configure APM CLI")
+			fmt.Println()
+			fmt.Println("Options:")
+			fmt.Println("  --help  Show this message and exit.")
+			fmt.Println()
+			fmt.Println("Commands:")
+			fmt.Println("  get    Get a configuration value")
+			fmt.Println("  set    Set a configuration value")
+			fmt.Println("  unset  Unset a configuration value")
 			return 0
 		}
 	}

cmd/apm/cmd_config.go

@@ -75,7 +75,8 @@ func printMarketplaceHelp() {
 	fmt.Println("  validate  Validate a marketplace manifest")
 	fmt.Println()
 	fmt.Println("Authoring commands:")
-	fmt.Println("  init      Add a 'marketplace:' block to apm.yml")
+	fmt.Println("  init      Add a 'marketplace:' block to apm.yml (scaffolds apm.yml if")
+	fmt.Println("            missing)")
 	fmt.Println("  check     Validate marketplace entries are resolvable")
 	fmt.Println("  outdated  Show packages with available upgrades")
 	fmt.Println("  doctor    Run environment diagnostics for marketplace publishing")

cmd/apm/cmd_marketplace.go

@@ -8,10 +8,10 @@ import (
 )
 
 var mcpSubcommands = []struct{ name, desc string }{
-	{"install", "Install an MCP server"},
-	{"search", "Search MCP servers in registry"},
-	{"inspect", "Show detailed MCP server information"},
+	{"install", "Add an MCP server to apm.yml."},
 	{"list", "List all available MCP servers"},
+	{"search", "Search MCP servers in registry"},
+	{"show", "Show detailed MCP server information"},
 }
 
 func printMCPHelp() {
@@ -24,7 +24,7 @@ func printMCPHelp() {
 	fmt.Println()
 	fmt.Println("Commands:")
 	for _, sub := range mcpSubcommands {
-		fmt.Printf("  %-14s%s\n", sub.name, sub.desc)
+		fmt.Printf("  %-9s%s\n", sub.name, sub.desc)
 	}
 }
 
@@ -42,7 +42,7 @@ func runMCP(args []string) int {
 		return runMCPInstall(rest)
 	case "search":
 		return runMCPSearch(rest)
-	case "inspect":
+	case "inspect", "show":
 		return runMCPInspect(rest)
 	case "list":
 		return runMCPList(rest)
@@ -106,7 +106,7 @@ func runMCPSearch(args []string) int {
 func runMCPInspect(args []string) int {
 	for _, a := range args {
 		if a == "--help" || a == "-h" {
-			fmt.Println("Usage: apm mcp inspect [OPTIONS] NAME")
+			fmt.Println("Usage: apm mcp show [OPTIONS] NAME")
 			fmt.Println()
 			fmt.Println("  Show detailed MCP server information")
 			fmt.Println()