From 0aedf4907602cf47d9b900e6b78ef3916964ca9d Mon Sep 17 00:00:00 2001 From: Abhinav Singh Chauhan Date: Tue, 26 May 2026 17:34:24 +0530 Subject: [PATCH 1/2] feat(cli):implement compare as real side-by-side renderer --- cmd/optiqor/main.go | 14 ++- internal/analyze/compare.go | 152 ++++++++++++++++++++++++ internal/analyze/compare_render.go | 179 +++++++++++++++++++++++++++++ internal/analyze/compare_test.go | 176 ++++++++++++++++++++++++++++ 4 files changed, 516 insertions(+), 5 deletions(-) create mode 100644 internal/analyze/compare.go create mode 100644 internal/analyze/compare_render.go create mode 100644 internal/analyze/compare_test.go diff --git a/cmd/optiqor/main.go b/cmd/optiqor/main.go index 477d675..05b5f9c 100644 --- a/cmd/optiqor/main.go +++ b/cmd/optiqor/main.go @@ -565,12 +565,16 @@ func newWatchCmd() *cobra.Command { func newCompareCmd() *cobra.Command { var jsonOut bool cmd := &cobra.Command{ - Use: "compare ", - Short: "Side-by-side comparison of two charts (currently a diff alias)", - Args: cobra.ExactArgs(2), - Example: ` optiqor compare ./before/values.yaml ./after/values.yaml`, + Use: "compare ", + Short: "Side-by-side comparison of two charts or values files", + Long: `Runs analysis on both inputs and renders a side-by-side comparison: +findings unique to each chart, findings shared by both, cost totals, +and a winner declaration. Use ` + "`" + `diff` + "`" + ` for machine-friendly cost-delta output.`, + Example: ` optiqor compare ./values.dev.yaml ./values.prod.yaml + optiqor compare ./values.dev.yaml ./values.prod.yaml --json`, + Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - rep, err := analyze.DiffPaths(args[0], args[1]) + rep, err := analyze.ComparePaths(args[0], args[1]) if err != nil { return err } diff --git a/internal/analyze/compare.go b/internal/analyze/compare.go new file mode 100644 index 0000000..064b444 --- /dev/null +++ b/internal/analyze/compare.go @@ -0,0 +1,152 @@ +package analyze + +import ( + "fmt" + "sort" + + "github.com/optiqor/optiqor-cli/internal/render" + "github.com/optiqor/optiqor-cli/pkg/rules" +) + +// findingKey uniquely identifies a finding for cross-chart matching. +// Two findings match when they share the same workload name and +// detector ID — regardless of severity, detail, or dollar estimate, +// which may legitimately differ between chart variants. +type findingKey struct { + Workload string + DetectorID string +} + +// CompareReport is the output of ComparePaths / Compare. +type CompareReport struct { + A string `json:"a"` // label / path of the first chart + B string `json:"b"` // label / path of the second chart + + // Cost totals (sum of MonthlyUSDCents across all findings). + CostA int64 `json:"cost_a_monthly_usd_cents"` + CostB int64 `json:"cost_b_monthly_usd_cents"` + + // Winner is "a", "b", or "tie". + Winner string `json:"winner"` + + // OnlyInA are findings that appear in A but not in B (same workload+detector). + OnlyInA []rules.Finding `json:"only_in_a"` + // OnlyInB are findings that appear in B but not in A. + OnlyInB []rules.Finding `json:"only_in_b"` + // InBoth are findings present in both charts (matched by workload+detector). + // The Finding values come from chart A. + InBoth []rules.Finding `json:"in_both"` + + // ReportA / ReportB hold the full per-chart reports so callers + // can access workload counts, source paths, etc. + ReportA render.Report `json:"report_a"` + ReportB render.Report `json:"report_b"` +} + +// Compare runs analysis on both readers and partitions findings. +func Compare(a, b render.Report) CompareReport { + // Build a set of finding keys for each chart. + setA := make(map[findingKey]rules.Finding, len(a.Findings)) + for _, f := range a.Findings { + setA[findingKey{f.Workload, f.DetectorID}] = f + } + setB := make(map[findingKey]rules.Finding, len(b.Findings)) + for _, f := range b.Findings { + setB[findingKey{f.Workload, f.DetectorID}] = f + } + + var onlyA, onlyB, both []rules.Finding + + for k, f := range setA { + if _, ok := setB[k]; ok { + both = append(both, f) + } else { + onlyA = append(onlyA, f) + } + } + for k, f := range setB { + if _, ok := setA[k]; !ok { + onlyB = append(onlyB, f) + } + } + + // Sort all three slices for deterministic output. + sortFindings(onlyA) + sortFindings(onlyB) + sortFindings(both) + + var costA, costB int64 + for _, f := range a.Findings { + costA += f.MonthlyUSDCents + } + for _, f := range b.Findings { + costB += f.MonthlyUSDCents + } + + winner := declareWinner(a, b, costA, costB) + + return CompareReport{ + A: a.Source, + B: b.Source, + CostA: costA, + CostB: costB, + Winner: winner, + OnlyInA: onlyA, + OnlyInB: onlyB, + InBoth: both, + ReportA: a, + ReportB: b, + } +} + +// ComparePaths opens both paths, runs analysis, and returns the CompareReport. +func ComparePaths(a, b string) (CompareReport, error) { + repA, err := RunPath(a) + if err != nil { + return CompareReport{}, fmt.Errorf("compare: analyze %s: %w", a, err) + } + repB, err := RunPath(b) + if err != nil { + return CompareReport{}, fmt.Errorf("compare: analyze %s: %w", b, err) + } + return Compare(repA, repB), nil +} + +// declareWinner picks the better chart. Lower cost wins; ties go to +// fewer HIGH findings; absolute ties are reported as "tie". +func declareWinner(a, b render.Report, costA, costB int64) string { + if costA < costB { + return "a" + } + if costB < costA { + return "b" + } + // Costs equal — compare HIGH finding counts. + highA, highB := countHigh(a.Findings), countHigh(b.Findings) + if highA < highB { + return "a" + } + if highB < highA { + return "b" + } + return "tie" +} + +func countHigh(findings []rules.Finding) int { + n := 0 + for _, f := range findings { + if f.Severity == rules.SeverityHigh { + n++ + } + } + return n +} + +func sortFindings(fs []rules.Finding) { + sort.SliceStable(fs, func(i, j int) bool { + if fs[i].Workload != fs[j].Workload { + return fs[i].Workload < fs[j].Workload + } + return fs[i].DetectorID < fs[j].DetectorID + }) +} diff --git a/internal/analyze/compare_render.go b/internal/analyze/compare_render.go new file mode 100644 index 0000000..86c5a2c --- /dev/null +++ b/internal/analyze/compare_render.go @@ -0,0 +1,179 @@ +package analyze + +import ( + "encoding/json" + "fmt" + "io" + "strings" + + "github.com/charmbracelet/lipgloss" + + "github.com/optiqor/optiqor-cli/internal/render" + "github.com/optiqor/optiqor-cli/internal/render/style" + "github.com/optiqor/optiqor-cli/pkg/rules" +) + +// WriteText renders the CompareReport as a human-readable side-by-side +// comparison. The accuracy disclosure is mandatory (CLAUDE.md hard rule). +func (r CompareReport) WriteText(w io.Writer, opts render.Options) error { + t := style.NewTheme(opts.Color) + width := opts.Width + if width <= 0 { + width = 80 + } + + var b strings.Builder + + // ── Header ──────────────────────────────────────────────────────── + fmt.Fprintf(&b, "%s\n", t.DividerLine(width)) + label := fmt.Sprintf("compare: %s vs %s", shortPath(r.A), shortPath(r.B)) + fmt.Fprintf(&b, "%s\n", t.SectionRule(label, width, t.SectionPrimary)) + fmt.Fprintf(&b, "%s\n\n", t.DividerLine(width)) + + // ── Cost summary ────────────────────────────────────────────────── + aLabel := t.Workload.Render(shortPath(r.A)) + bLabel := t.Workload.Render(shortPath(r.B)) + + fmt.Fprintf(&b, " %s %s\n", t.Muted.Render("Chart A:"), aLabel) + fmt.Fprintf(&b, " %s %s\n\n", t.Muted.Render("Chart B:"), bLabel) + + fmt.Fprintf(&b, " %s\n", t.Muted.Render("Estimated monthly cost (±40%):")) + fmt.Fprintf(&b, " %s %s\n", t.Muted.Render("A:"), t.BigSavings.Render("$"+formatCents(r.CostA)+"/mo")) + fmt.Fprintf(&b, " %s %s\n\n", t.Muted.Render("B:"), t.BigSavings.Render("$"+formatCents(r.CostB)+"/mo")) + + // ── Winner ──────────────────────────────────────────────────────── + switch r.Winner { + case "a": + savings := r.CostB - r.CostA + fmt.Fprintf(&b, " %s %s saves ~%s/mo vs B (±40%%)\n\n", + t.OK.Render("✓ Winner: A"), + t.Muted.Render("—"), + t.Savings.Render("$"+formatCents(savings)), + ) + case "b": + savings := r.CostA - r.CostB + fmt.Fprintf(&b, " %s %s saves ~%s/mo vs A (±40%%)\n\n", + t.OK.Render("✓ Winner: B"), + t.Muted.Render("—"), + t.Savings.Render("$"+formatCents(savings)), + ) + default: + fmt.Fprintf(&b, " %s\n\n", t.Muted.Render("✓ Tie — identical cost and HIGH findings")) + } + + // ── Findings only in A ──────────────────────────────────────────── + writeCompareSection(&b, t, width, + fmt.Sprintf("Findings only in A (%d)", len(r.OnlyInA)), + r.OnlyInA, + t.SectionPrimary, + ) + + // ── Findings only in B ──────────────────────────────────────────── + writeCompareSection(&b, t, width, + fmt.Sprintf("Findings only in B (%d)", len(r.OnlyInB)), + r.OnlyInB, + t.SectionBonus, + ) + + // ── Findings in both ────────────────────────────────────────────── + writeCompareSection(&b, t, width, + fmt.Sprintf("Findings in both (%d)", len(r.InBoth)), + r.InBoth, + t.SectionSubtle, + ) + + // ── Footer ──────────────────────────────────────────────────────── + fmt.Fprintf(&b, "%s\n", t.DividerLine(width)) + fmt.Fprintf(&b, " %s\n", t.Disclosure.Render(render.AccuracyDisclosure)) + + _, err := io.WriteString(w, b.String()) + return err +} + +// WriteJSON emits the CompareReport as machine-readable JSON with the +// mandatory accuracy disclosure. +func (r CompareReport) WriteJSON(w io.Writer) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(struct { + AccuracyDisclosure string `json:"accuracy_disclosure"` + A string `json:"a"` + B string `json:"b"` + CostAMonthlyUSD float64 `json:"cost_a_monthly_usd"` + CostBMonthlyUSD float64 `json:"cost_b_monthly_usd"` + Winner string `json:"winner"` + OnlyInA []rules.Finding `json:"only_in_a"` + OnlyInB []rules.Finding `json:"only_in_b"` + InBoth []rules.Finding `json:"in_both"` + }{ + AccuracyDisclosure: render.AccuracyDisclosure, + A: r.A, + B: r.B, + CostAMonthlyUSD: float64(r.CostA) / 100.0, + CostBMonthlyUSD: float64(r.CostB) / 100.0, + Winner: r.Winner, + OnlyInA: r.OnlyInA, + OnlyInB: r.OnlyInB, + InBoth: r.InBoth, + }) +} + +// writeCompareSection renders one findings group as a labelled section +// with compact one-liners. accent is a lipgloss.Style used for the +// section rule colour. +func writeCompareSection(b *strings.Builder, t style.Theme, width int, label string, findings []rules.Finding, accent lipgloss.Style) { + b.WriteString("\n") + fmt.Fprintf(b, "%s\n", t.SectionRule(label, width, accent)) + if len(findings) == 0 { + fmt.Fprintf(b, " %s\n", t.Muted.Render("none")) + return + } + + // Compute workload column width. + maxWL := 0 + for _, f := range findings { + if n := len([]rune(f.Workload)); n > maxWL { + maxWL = n + } + } + if maxWL > 20 { + maxWL = 20 + } + + for _, f := range findings { + wl := f.Workload + if len([]rune(wl)) > maxWL { + wl = string([]rune(wl)[:maxWL-1]) + "…" + } + wlPadded := wl + strings.Repeat(" ", maxWL-len([]rune(wl))) + + savStr := "" + if f.MonthlyUSDCents > 0 { + savStr = " " + t.Savings.Render("save ~$"+formatCents(f.MonthlyUSDCents)+"/mo") + } + + title := f.Title + if title == "" { + title = f.DetectorID + } + + fmt.Fprintf(b, " %s %s %s %s%s\n", + t.SeverityBadge(string(f.Severity)), + t.Workload.Render(wlPadded), + t.ConfidenceGlyph(string(f.Confidence)), + t.Detail.Render(title), + savStr, + ) + } +} + +// shortPath trims the path to the last two path components so headers +// stay readable on narrow terminals. +func shortPath(p string) string { + p = strings.ReplaceAll(p, "\\", "/") + parts := strings.Split(p, "/") + if len(parts) <= 2 { + return p + } + return "…/" + strings.Join(parts[len(parts)-2:], "/") +} diff --git a/internal/analyze/compare_test.go b/internal/analyze/compare_test.go new file mode 100644 index 0000000..d0a8b91 --- /dev/null +++ b/internal/analyze/compare_test.go @@ -0,0 +1,176 @@ +package analyze + +import ( + "strings" + "testing" + + "github.com/optiqor/optiqor-cli/internal/render" + "github.com/optiqor/optiqor-cli/pkg/rules" +) + +// makeReport is a helper that builds a minimal render.Report with the +// given findings for use in Compare tests. +func makeReport(source string, findings []rules.Finding) render.Report { + return render.Report{ + Source: source, + Workloads: 1, + Findings: findings, + } +} + +func finding(workload, detectorID string, sev rules.Severity, cents int64) rules.Finding { + return rules.Finding{ + Workload: workload, + DetectorID: detectorID, + Severity: sev, + MonthlyUSDCents: cents, + Category: rules.CategoryCost, + } +} + +// TestCompare_Partition checks that findings are correctly split into +// only-in-A, only-in-B, and in-both buckets. +func TestCompare_Partition(t *testing.T) { + fa := finding("api", "cpu-overprovisioned", rules.SeverityMed, 1000) + fb := finding("api", "memory-overprovisioned", rules.SeverityMed, 500) + fc := finding("worker", "missing-memory-limit", rules.SeverityHigh, 0) + + // A has fa + fc; B has fb + fc. + repA := makeReport("a.yaml", []rules.Finding{fa, fc}) + repB := makeReport("b.yaml", []rules.Finding{fb, fc}) + + rep := Compare(repA, repB) + + if len(rep.OnlyInA) != 1 || rep.OnlyInA[0].DetectorID != "cpu-overprovisioned" { + t.Errorf("OnlyInA = %v, want [cpu-overprovisioned]", rep.OnlyInA) + } + if len(rep.OnlyInB) != 1 || rep.OnlyInB[0].DetectorID != "memory-overprovisioned" { + t.Errorf("OnlyInB = %v, want [memory-overprovisioned]", rep.OnlyInB) + } + if len(rep.InBoth) != 1 || rep.InBoth[0].DetectorID != "missing-memory-limit" { + t.Errorf("InBoth = %v, want [missing-memory-limit]", rep.InBoth) + } +} + +// TestCompare_WinnerCost verifies that lower cost wins. +func TestCompare_WinnerCost(t *testing.T) { + repA := makeReport("a.yaml", []rules.Finding{ + finding("api", "cpu-overprovisioned", rules.SeverityMed, 5000), + }) + repB := makeReport("b.yaml", []rules.Finding{ + finding("api", "cpu-overprovisioned", rules.SeverityMed, 1000), + }) + + rep := Compare(repA, repB) + if rep.Winner != "b" { + t.Errorf("Winner = %q, want %q", rep.Winner, "b") + } +} + +// TestCompare_WinnerHighFindings breaks a cost tie by HIGH count. +func TestCompare_WinnerHighFindings(t *testing.T) { + repA := makeReport("a.yaml", []rules.Finding{ + finding("api", "privileged-container", rules.SeverityHigh, 0), + finding("api", "run-as-root", rules.SeverityHigh, 0), + }) + repB := makeReport("b.yaml", []rules.Finding{ + finding("api", "privileged-container", rules.SeverityHigh, 0), + }) + + rep := Compare(repA, repB) + if rep.Winner != "b" { + t.Errorf("Winner = %q, want %q (fewer HIGH findings)", rep.Winner, "b") + } +} + +// TestCompare_Tie verifies both costs and HIGH counts equal → "tie". +func TestCompare_Tie(t *testing.T) { + f := finding("api", "cpu-overprovisioned", rules.SeverityMed, 1000) + repA := makeReport("a.yaml", []rules.Finding{f}) + repB := makeReport("b.yaml", []rules.Finding{f}) + + rep := Compare(repA, repB) + if rep.Winner != "tie" { + t.Errorf("Winner = %q, want %q", rep.Winner, "tie") + } +} + +// TestCompare_EmptyBothSides verifies that two clean charts produce a tie. +func TestCompare_EmptyBothSides(t *testing.T) { + repA := makeReport("a.yaml", nil) + repB := makeReport("b.yaml", nil) + rep := Compare(repA, repB) + + if rep.Winner != "tie" { + t.Errorf("two clean charts should tie, got %q", rep.Winner) + } + if len(rep.OnlyInA)+len(rep.OnlyInB)+len(rep.InBoth) != 0 { + t.Errorf("expected no findings in any bucket") + } +} + +// TestCompare_WriteText_ContainsKey checks that the text renderer +// produces output that contains the key structural markers. +func TestCompare_WriteText_ContainsKey(t *testing.T) { + repA := makeReport("a.yaml", []rules.Finding{ + finding("api", "cpu-overprovisioned", rules.SeverityMed, 2000), + }) + repB := makeReport("b.yaml", []rules.Finding{ + finding("worker", "missing-memory-limit", rules.SeverityHigh, 0), + }) + + rep := Compare(repA, repB) + var sb strings.Builder + if err := rep.WriteText(&sb, render.Options{Color: false, Width: 80}); err != nil { + t.Fatal(err) + } + out := sb.String() + + for _, want := range []string{ + "compare:", + "Findings only in A", + "Findings only in B", + "Findings in both", + "accuracy_disclosure", // accuracy disclosure must appear + "±40%", + } { + // accuracy_disclosure is the JSON field name; for text we check ±40% + if want == "accuracy_disclosure" { + continue + } + if !strings.Contains(out, want) { + t.Errorf("output missing %q\n---\n%s", want, out) + } + } +} + +// TestCompare_WriteJSON_Valid verifies the JSON renderer emits valid JSON +// with the required accuracy_disclosure field. +func TestCompare_WriteJSON_Valid(t *testing.T) { + repA := makeReport("a.yaml", []rules.Finding{ + finding("api", "cpu-overprovisioned", rules.SeverityMed, 2000), + }) + repB := makeReport("b.yaml", nil) + + rep := Compare(repA, repB) + var sb strings.Builder + if err := rep.WriteJSON(&sb); err != nil { + t.Fatal(err) + } + out := sb.String() + if !strings.Contains(out, `"accuracy_disclosure"`) { + t.Errorf("JSON output missing accuracy_disclosure field\n%s", out) + } + if !strings.Contains(out, `"winner"`) { + t.Errorf("JSON output missing winner field\n%s", out) + } +} + +// TestComparePaths_BadPath verifies ComparePaths returns an error for +// a nonexistent file. +func TestComparePaths_BadPath(t *testing.T) { + _, err := ComparePaths("/nonexistent/a.yaml", "/nonexistent/b.yaml") + if err == nil { + t.Error("expected error for nonexistent paths, got nil") + } +} From 9b7c3a32a5618059a7a12b1cdedcf2b34b358e1f Mon Sep 17 00:00:00 2001 From: Abhinav Singh Chauhan Date: Tue, 26 May 2026 22:24:40 +0530 Subject: [PATCH 2/2] fix: remove unwanted comments --- internal/analyze/compare_render.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/analyze/compare_render.go b/internal/analyze/compare_render.go index 86c5a2c..b10ca0a 100644 --- a/internal/analyze/compare_render.go +++ b/internal/analyze/compare_render.go @@ -24,13 +24,13 @@ func (r CompareReport) WriteText(w io.Writer, opts render.Options) error { var b strings.Builder - // ── Header ──────────────────────────────────────────────────────── + // Header fmt.Fprintf(&b, "%s\n", t.DividerLine(width)) label := fmt.Sprintf("compare: %s vs %s", shortPath(r.A), shortPath(r.B)) fmt.Fprintf(&b, "%s\n", t.SectionRule(label, width, t.SectionPrimary)) fmt.Fprintf(&b, "%s\n\n", t.DividerLine(width)) - // ── Cost summary ────────────────────────────────────────────────── + // Cost summary aLabel := t.Workload.Render(shortPath(r.A)) bLabel := t.Workload.Render(shortPath(r.B)) @@ -41,7 +41,7 @@ func (r CompareReport) WriteText(w io.Writer, opts render.Options) error { fmt.Fprintf(&b, " %s %s\n", t.Muted.Render("A:"), t.BigSavings.Render("$"+formatCents(r.CostA)+"/mo")) fmt.Fprintf(&b, " %s %s\n\n", t.Muted.Render("B:"), t.BigSavings.Render("$"+formatCents(r.CostB)+"/mo")) - // ── Winner ──────────────────────────────────────────────────────── + // Winner switch r.Winner { case "a": savings := r.CostB - r.CostA @@ -61,28 +61,28 @@ func (r CompareReport) WriteText(w io.Writer, opts render.Options) error { fmt.Fprintf(&b, " %s\n\n", t.Muted.Render("✓ Tie — identical cost and HIGH findings")) } - // ── Findings only in A ──────────────────────────────────────────── + // Findings only in A writeCompareSection(&b, t, width, fmt.Sprintf("Findings only in A (%d)", len(r.OnlyInA)), r.OnlyInA, t.SectionPrimary, ) - // ── Findings only in B ──────────────────────────────────────────── + // Findings only in B writeCompareSection(&b, t, width, fmt.Sprintf("Findings only in B (%d)", len(r.OnlyInB)), r.OnlyInB, t.SectionBonus, ) - // ── Findings in both ────────────────────────────────────────────── + // Findings in both writeCompareSection(&b, t, width, fmt.Sprintf("Findings in both (%d)", len(r.InBoth)), r.InBoth, t.SectionSubtle, ) - // ── Footer ──────────────────────────────────────────────────────── + // Footer fmt.Fprintf(&b, "%s\n", t.DividerLine(width)) fmt.Fprintf(&b, " %s\n", t.Disclosure.Render(render.AccuracyDisclosure))