Title: Colorize get pr insight and group sections by risk

modules/code/code.go

@@ -21,4 +21,5 @@ func ModuleInit(reg registry.ModuleRegistrar) {
 	reg.RegisterFlagResolveFn(resolvePrincipalIDFnID, resolvePrincipalID)
 	reg.RegisterWorkflow(getPRWorkflowID, GetPRWorkflow)
 	reg.RegisterTextFormatter(reviewGroupTextFormatterID, reviewGroupTextFormatter)
+	reg.RegisterTextFormatter(insightTextFormatterID, insightTextFormatter)
 }

modules/code/insight.go

@@ -6,102 +6,17 @@ package code
 import (
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/harness/cli/pkg/cmdctx"
-	"github.com/harness/cli/pkg/exprenv"
-	"github.com/harness/cli/pkg/hlog"
-	"github.com/harness/cli/pkg/registry"
+	"github.com/harness/cli/pkg/console"
 )
 
 const (
-	getPRWorkflowID            = "get_pr"
 	reviewGroupTextFormatterID = "pr_review_group_text"
+	insightTextFormatterID     = "pr_insight_text"
 )
 
-// insightSections lists the Harness Code review-insight sub-commands appended to "get pr" output,
-// each best-effort: a failure only omits that section, it never fails the command.
-var insightSections = []struct {
-	verb, noun, label string
-}{
-	{"get", "pr:insight", "Insight"},
-}
-
-// isMachineFormat mirrors exprenv.isMachineFormat (unexported): these formats are
-// meant for structured consumption, so insight sections (extra, ad hoc text) are skipped.
-func isMachineFormat(format string) bool {
-	switch format {
-	case "json", "jsonl", "csv", "tsv", "markdown", "ui":
-		return true
-	}
-	return false
-}
-
-// GetPRWorkflow implements "get pr". It fetches and renders the base pull request
-// exactly as the old handler_type: endpoint command did (hard fail on error, unchanged
-// output), then best-effort appends Harness Code review-insight sections below it. Any insight
-// endpoint failure is logged as a warning and the section is omitted — it never fails
-// the command.
-func GetPRWorkflow(ctx *cmdctx.Ctx) error {
-	baseSpec := ctx.Resolver.GetSpec("get", "pr")
-	if baseSpec == nil || baseSpec.Endpoint == nil {
-		return fmt.Errorf("get pr command spec not found")
-	}
-
-	if isMachineFormat(ctx.FormatFlags.Format) || cmdctx.GetBool(ctx.FlagValues, "list-fields") {
-		_, err := registry.RunEndpoint(ctx, baseSpec.Endpoint)
-		return err
-	}
-
-	// Fetch (hard-fail on error, same as before) but don't render yet — the base
-	// PR block now prints last, under "PR Details", after the Insight section.
-	pr, err := registry.CallEndpoint(ctx, baseSpec.Endpoint)
-	if err != nil {
-		return err
-	}
-
-	origNoun, origFieldsNoun := ctx.Noun, ctx.FieldsNoun
-	defer func() { ctx.Noun, ctx.FieldsNoun = origNoun, origFieldsNoun }()
-
-	for _, section := range insightSections {
-		cs := ctx.Resolver.GetSpec(section.verb, section.noun)
-		if cs == nil || cs.Endpoint == nil {
-			hlog.Warn("insight section spec not found, omitting from get pr", "verb", section.verb, "noun", section.noun)
-			continue
-		}
-		ctx.Noun, ctx.FieldsNoun = cs.Noun, cs.FieldsNoun
-		// Probe with a fetch-only call first so a failure never prints a section
-		// header with nothing under it; RunEndpoint's own render then re-fetches
-		// (cheap: these are all idempotent GETs).
-		if _, err := registry.CallEndpoint(ctx, cs.Endpoint); err != nil {
-			hlog.Warn("insight fetch failed, omitting from get pr", "section", section.label, "err", err)
-			continue
-		}
-		fmt.Printf("\n--- %s ---\n", section.label)
-		ep := *cs.Endpoint
-		ep.TextFooter = ""
-		if _, err := registry.RunEndpoint(ctx, &ep); err != nil {
-			hlog.Warn("insight fetch failed, omitting from get pr", "section", section.label, "err", err)
-		}
-	}
-
-	ctx.Noun, ctx.FieldsNoun = origNoun, origFieldsNoun
-
-	fmt.Print("\n--- PR Details ---\n")
-	baseEP := *baseSpec.Endpoint
-	baseEP.TextFooter = ""
-	if _, err := registry.RunEndpoint(ctx, &baseEP); err != nil {
-		return err
-	}
-
-	if footer := baseSpec.Endpoint.TextFooter; footer != "" {
-		env := exprenv.WithIt(exprenv.Make(ctx), pr)
-		if text, err := exprenv.ResolvePath(env, footer); err == nil {
-			fmt.Print(text)
-		}
-	}
-	return nil
-}
-
 // reviewGroupTextFormatter renders the risk-bucketed review groups for a pull
 // request as a readable report: one block per group with its title, risk,
 // description, and the full list of changed file paths.
@@ -110,7 +25,7 @@ func reviewGroupTextFormatter(w io.Writer, d cmdctx.DataAccessor) error {
 	if len(groups) == 0 {
 		fmt.Fprintln(w, "No review groups.")
 	}
-	for i, raw := range groups {
+	for _, raw := range groups {
 		g, ok := raw.(map[string]any)
 		if !ok {
 			continue
@@ -121,11 +36,15 @@ func reviewGroupTextFormatter(w io.Writer, d cmdctx.DataAccessor) error {
 		if tags, ok := g["tags"].(map[string]any); ok {
 			risk, _ = tags["risk"].(string)
 		}
-		fmt.Fprintf(w, "\nGroup %d: %s", i+1, title)
+		riskTag := ""
 		if risk != "" {
-			fmt.Fprintf(w, " [risk: %s]", risk)
+			riskTag = fmt.Sprintf(" [%s]", risk)
+		}
+		line := fmt.Sprintf("● %s%s", title, riskTag)
+		if c := riskColor(risk); c != 0 {
+			line = console.WithColor(c, line)
 		}
-		fmt.Fprintln(w)
+		fmt.Fprintf(w, "\n%s\n", line)
 		if desc != "" {
 			fmt.Fprintln(w, desc)
 		}
@@ -149,3 +68,43 @@ func reviewGroupTextFormatter(w io.Writer, d cmdctx.DataAccessor) error {
 	}
 	return nil
 }
+
+// riskColor maps a risk bucket ("low"/"medium"/"high", case-insensitive) to the
+// color it's displayed in. Returns 0 (no color) for any other value, including empty.
+func riskColor(risk string) console.Color {
+	switch strings.ToLower(risk) {
+	case "low":
+		return console.ColorGreen
+	case "medium":
+		return console.ColorYellow
+	case "high":
+		return console.ColorRed
+	default:
+		return 0
+	}
+}
+
+// insightTextFormatter renders the AI code-review overview for a pull request as a
+// colorized (by risk) header/footer section marker around the review content.
+// The content is printed verbatim: wrapping is left to the terminal so words and
+// URLs are never split, and the output stays copy-paste clean.
+func insightTextFormatter(w io.Writer, d cmdctx.DataAccessor) error {
+	renderInsight(w, d.GetString("it.risk"), d.GetString("it.content"))
+	return nil
+}
+
+// renderInsight is the shared rendering step behind insightTextFormatter and
+// DebugRenderInsightHandler, so the debug command exercises the exact same
+// styling without needing a fake cmdctx.DataAccessor.
+func renderInsight(w io.Writer, risk, content string) {
+	heading := "AI Code Review"
+	if risk != "" {
+		heading += fmt.Sprintf(" [%s risk]", risk)
+	}
+	console.RenderTextBox(w, console.TextBox{
+		Icon:        "✨",
+		Header:      heading,
+		HeaderColor: riskColor(risk),
+		Text:        strings.TrimSpace(content),
+	})
+}

modules/code/insight_test.go

@@ -58,10 +58,14 @@ func (s spyResolver) GetSpec(verb, noun string) *spec.CommandSpec {
 }
 
 func (s spyResolver) ResolveTextFormatter(id string) cmdctx.TextFormatterFn {
-	if id == reviewGroupTextFormatterID {
+	switch id {
+	case reviewGroupTextFormatterID:
 		return reviewGroupTextFormatter
+	case insightTextFormatterID:
+		return insightTextFormatter
+	default:
+		return nil
 	}
-	return nil
 }
 
 // testNounURLPath is a stand-in url_path template shared by test nouns: it resolves
@@ -94,7 +98,10 @@ func insightSpec(path string) *spec.CommandSpec {
 		Command: "get pr:insight", Verb: "get", VerbHandler: "get",
 		Noun: "pr", NounVariant: "insight", FieldsNoun: "pr_insight", Module: "code",
 		HandlerType: spec.HandlerEndpoint,
-		Endpoint:    &spec.EndpointSpec{Method: "GET", Path: path, ItemExpr: "it", TextFooter: "\n{{url(it)}}\n"},
+		Endpoint: &spec.EndpointSpec{
+			Method: "GET", Path: path, ItemExpr: "it", TextFooter: "\n{{url(it)}}\n",
+			TextFormatter: insightTextFormatterID,
+		},
 	}
 }
 
@@ -211,7 +218,7 @@ func TestGetPRWorkflow_InsightFailureOmitsSectionButSucceeds(t *testing.T) {
 	if err != nil {
 		t.Fatalf("get pr must succeed even when an insight endpoint fails, got: %v", err)
 	}
-	if strings.Contains(out, "Insight") {
+	if strings.Contains(out, "AI Code Review") {
 		t.Fatalf("output must omit failed sections, got:\n%s", out)
 	}
 }
@@ -235,23 +242,23 @@ func TestGetPRWorkflow_InsightSuccessRendersSection(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if !strings.Contains(out, "Insight") {
-		t.Fatalf("output must contain the Insight section, got:\n%s", out)
+	if !strings.Contains(out, "AI Code Review") {
+		t.Fatalf("output must contain the AI Code Review heading, got:\n%s", out)
 	}
-	if !strings.Contains(out, "PR Details") {
-		t.Fatalf("output must contain the PR Details section, got:\n%s", out)
+	if !strings.Contains(out, "Number:") {
+		t.Fatalf("output must contain the PR's labeled fields, got:\n%s", out)
 	}
 
-	// Insight must render first, PR Details last (right before the link), and the
+	// Labeled fields must render first, Insight last (right before the link), and the
 	// PR link must print exactly once, at the very end.
-	insightIdx := strings.Index(out, "Insight")
-	prDetailsIdx := strings.Index(out, "PR Details")
-	if insightIdx == -1 || prDetailsIdx == -1 || prDetailsIdx < insightIdx {
-		t.Fatalf("expected Insight to render before PR Details, got:\n%s", out)
+	numberIdx := strings.Index(out, "Number:")
+	insightIdx := strings.Index(out, "AI Code Review")
+	if insightIdx == -1 || numberIdx == -1 || insightIdx < numberIdx {
+		t.Fatalf("expected labeled fields to render before Insight, got:\n%s", out)
 	}
 	lastLinkIdx := strings.LastIndex(out, "/pulls/42")
-	if lastLinkIdx == -1 || lastLinkIdx < prDetailsIdx {
-		t.Fatalf("expected the PR link to appear after the PR Details section, got:\n%s", out)
+	if lastLinkIdx == -1 || lastLinkIdx < insightIdx {
+		t.Fatalf("expected the PR link to appear after the Insight section, got:\n%s", out)
 	}
 	if linkCount := strings.Count(out, "/pulls/42"); linkCount != 1 {
 		t.Fatalf("expected exactly one PR link (sections must not duplicate it), got %d in:\n%s", linkCount, out)
@@ -281,6 +288,62 @@ func TestReviewGroupCommand_StandaloneRendersLink(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// insightTextFormatter
+// ---------------------------------------------------------------------------
+
+type fakeDataAccessor struct {
+	values map[string]string
+	slices map[string][]any
+}
+
+func (f fakeDataAccessor) GetString(path string) string { return f.values[path] }
+func (f fakeDataAccessor) GetInt64(string) int64        { return 0 }
+func (f fakeDataAccessor) GetBool(string) bool          { return false }
+func (f fakeDataAccessor) GetTs(string) string          { return "" }
+func (f fakeDataAccessor) GetData() any                 { return nil }
+func (f fakeDataAccessor) GetSlice(path string) []any   { return f.slices[path] }
+
+// ---------------------------------------------------------------------------
+// reviewGroupTextFormatter
+// ---------------------------------------------------------------------------
+
+func TestReviewGroupTextFormatter_ColorizesBulletAndRiskTag(t *testing.T) {
+	groups := []any{
+		map[string]any{
+			"title":       "Auth middleware changes",
+			"description": "Touches token validation.",
+			"tags":        map[string]any{"risk": "high"},
+			"files": []any{
+				map[string]any{"path": "auth/middleware.go"},
+			},
+		},
+	}
+	out := captureStdout(t, func() {
+		err := reviewGroupTextFormatter(os.Stdout, fakeDataAccessor{
+			slices: map[string][]any{"it.groups": groups},
+		})
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+	})
+	if strings.Contains(out, "Group 1") || strings.Contains(out, "Group ") {
+		t.Fatalf("output must not contain a numbered \"Group\" label, got:\n%s", out)
+	}
+	if !strings.Contains(out, "●") {
+		t.Fatalf("output must contain the risk bullet, got:\n%s", out)
+	}
+	if !strings.Contains(out, "Auth middleware changes") {
+		t.Fatalf("output must contain the group title, got:\n%s", out)
+	}
+	if !strings.Contains(out, "[high]") {
+		t.Fatalf("output must contain the risk tag, got:\n%s", out)
+	}
+	if !strings.Contains(out, "auth/middleware.go") {
+		t.Fatalf("output must still list the file path, got:\n%s", out)
+	}
+}
+
 // ---------------------------------------------------------------------------
 // ModuleInit
 // ---------------------------------------------------------------------------