Add council review report command

2026-03-19 15:42:36 +08:00
parent 740a7b4acd
commit 7f3eb1c24b
8 changed files with 851 additions and 13 deletions
@@ -11,5 +11,6 @@ func newCouncilCmd(root *rootOptions) *cobra.Command {
 	cmd.AddCommand(newCouncilStartCmd(root))
 	cmd.AddCommand(newCouncilWaitCmd(root))
 	cmd.AddCommand(newCouncilTallyCmd(root))
+	cmd.AddCommand(newCouncilReportCmd(root))
 	return cmd
 }
@@ -0,0 +1,111 @@
+package orch
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"ai-workflow-skill/internal/protocol"
+	"ai-workflow-skill/internal/store"
+
+	"github.com/spf13/cobra"
+)
+
+type councilReportOptions struct {
+	runID string
+	show  string
+}
+
+func newCouncilReportCmd(root *rootOptions) *cobra.Command {
+	opts := &councilReportOptions{}
+
+	cmd := &cobra.Command{
+		Use:   "report",
+		Short: "Render the final grouped council output",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			ctx := cmd.Context()
+
+			sqlDB, err := openOrchDB(ctx, root.dbPath)
+			if err != nil {
+				return err
+			}
+			defer sqlDB.Close()
+
+			orchStore := store.NewOrchStore(sqlDB)
+			result, err := orchStore.BuildCouncilReport(ctx, store.CouncilReportInput{
+				RunID: opts.runID,
+				Show:  opts.show,
+			})
+			if err != nil {
+				return err
+			}
+
+			reportPath := councilReportArtifactPath(root.dbPath, result.RunID)
+			if err := os.MkdirAll(filepath.Dir(reportPath), 0o755); err != nil {
+				return fmt.Errorf("create council report directory: %w", err)
+			}
+			if err := os.WriteFile(reportPath, []byte(result.Markdown), 0o644); err != nil {
+				return fmt.Errorf("write council report artifact: %w", err)
+			}
+
+			result.ReportArtifacts = []store.CouncilReportArtifact{
+				{
+					Kind: "markdown",
+					Path: reportPath,
+				},
+			}
+			if err := orchStore.PersistCouncilReport(ctx, store.CouncilPersistReportInput{
+				RunID:        result.RunID,
+				Show:         result.Show,
+				Summary:      result.Summary,
+				MarkdownPath: reportPath,
+			}); err != nil {
+				return err
+			}
+
+			resp := protocol.Success{
+				OK:      true,
+				Command: "council report",
+				Data: map[string]any{
+					"run_id":                  result.RunID,
+					"show":                    result.Show,
+					"summary":                 result.Summary,
+					"grouped_recommendations": result.GroupedRecommendations,
+					"report_artifacts":        result.ReportArtifacts,
+				},
+			}
+			if root.json {
+				return protocol.WriteJSON(cmd.OutOrStdout(), resp)
+			}
+
+			_, err = fmt.Fprint(cmd.OutOrStdout(), result.Markdown)
+			return err
+		},
+	}
+
+	cmd.Flags().StringVar(&opts.runID, "run", "", "Council run ID")
+	cmd.Flags().StringVar(&opts.show, "show", "", "Buckets to show: consensus,majority,minority,all")
+	_ = cmd.MarkFlagRequired("run")
+
+	return cmd
+}
+
+func councilReportArtifactPath(dbPath, runID string) string {
+	baseDir := "."
+	dbDir := filepath.Dir(dbPath)
+	switch {
+	case dbDir == "", dbDir == ".":
+		baseDir = "."
+	case filepath.Base(dbDir) == ".agents":
+		baseDir = filepath.Dir(dbDir)
+	default:
+		baseDir = dbDir
+	}
+	if baseDir == "" {
+		baseDir = "."
+	}
+
+	fileName := strings.ReplaceAll(strings.TrimSpace(runID), string(os.PathSeparator), "_") + ".md"
+	return filepath.Join(baseDir, ".orch", "reports", fileName)
+}
@@ -1870,6 +1870,186 @@ func TestOrchCouncilTallyStrictKeepsDistinctProposals(t *testing.T) {
 	}
 }

+func TestOrchCouncilReportDefaultShowsConsensusAndMajority(t *testing.T) {
+	t.Parallel()
+
+	dbPath := filepath.Join(t.TempDir(), "coord.db")
+	runID := "council_blog_report_001"
+	seedCouncilReportRun(t, dbPath, runID)
+
+	reportOut := runOrchCommand(
+		t,
+		"--db", dbPath,
+		"council", "report",
+		"--run", runID,
+	)
+
+	if !strings.Contains(reportOut, "# Council Review Report") {
+		t.Fatalf("expected markdown report header, got %q", reportOut)
+	}
+	if !strings.Contains(reportOut, "## Consensus") {
+		t.Fatalf("expected consensus section, got %q", reportOut)
+	}
+	if !strings.Contains(reportOut, "## Majority") {
+		t.Fatalf("expected majority section, got %q", reportOut)
+	}
+	if strings.Contains(reportOut, "## Minority") {
+		t.Fatalf("did not expect minority section in default report, got %q", reportOut)
+	}
+
+	reportPath := councilReportArtifactPath(dbPath, runID)
+	reportBytes, err := os.ReadFile(reportPath)
+	if err != nil {
+		t.Fatalf("read council report artifact: %v", err)
+	}
+	if string(reportBytes) != reportOut {
+		t.Fatalf("expected stdout markdown to match artifact contents")
+	}
+
+	sqlDB, err := openOrchDB(t.Context(), dbPath)
+	if err != nil {
+		t.Fatalf("open orch db: %v", err)
+	}
+	defer sqlDB.Close()
+
+	var showJSON string
+	var summaryJSON string
+	var markdownPath string
+	if err := sqlDB.QueryRowContext(
+		t.Context(),
+		`SELECT show_json, summary_json, markdown_path
+		 FROM council_reports
+		 WHERE run_id = ?`,
+		runID,
+	).Scan(&showJSON, &summaryJSON, &markdownPath); err != nil {
+		t.Fatalf("query council report metadata: %v", err)
+	}
+	if markdownPath != reportPath {
+		t.Fatalf("expected report path %q, got %q", reportPath, markdownPath)
+	}
+
+	var show []string
+	mustDecodeJSON(t, showJSON, &show)
+	if len(show) != 2 || show[0] != "consensus" || show[1] != "majority" {
+		t.Fatalf("expected default show buckets [consensus majority], got %#v", show)
+	}
+
+	var summary map[string]any
+	mustDecodeJSON(t, summaryJSON, &summary)
+	if got, _ := summary["consensus"].(float64); got != 1 {
+		t.Fatalf("expected one consensus group, got %#v", summary["consensus"])
+	}
+	if got, _ := summary["majority"].(float64); got != 1 {
+		t.Fatalf("expected one majority group, got %#v", summary["majority"])
+	}
+	if got, _ := summary["minority"].(float64); got != 1 {
+		t.Fatalf("expected one minority group, got %#v", summary["minority"])
+	}
+}
+
+func TestOrchCouncilReportShowAllIncludesMinority(t *testing.T) {
+	t.Parallel()
+
+	dbPath := filepath.Join(t.TempDir(), "coord.db")
+	runID := "council_blog_report_002"
+	seedCouncilReportRun(t, dbPath, runID)
+
+	reportOut := runOrchCommand(
+		t,
+		"--db", dbPath,
+		"council", "report",
+		"--run", runID,
+		"--show", "all",
+	)
+
+	if !strings.Contains(reportOut, "## Consensus") {
+		t.Fatalf("expected consensus section, got %q", reportOut)
+	}
+	if !strings.Contains(reportOut, "## Majority") {
+		t.Fatalf("expected majority section, got %q", reportOut)
+	}
+	if !strings.Contains(reportOut, "## Minority") {
+		t.Fatalf("expected minority section when --show all is used, got %q", reportOut)
+	}
+	if !strings.Contains(reportOut, "Add regression tests for council report JSON output.") {
+		t.Fatalf("expected minority proposal in report output, got %q", reportOut)
+	}
+}
+
+func TestOrchCouncilReportJSONShape(t *testing.T) {
+	t.Parallel()
+
+	dbPath := filepath.Join(t.TempDir(), "coord.db")
+	runID := "council_blog_report_003"
+	seedCouncilReportRun(t, dbPath, runID)
+
+	reportOut := runOrchCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"council", "report",
+		"--run", runID,
+	)
+
+	var reportResp map[string]any
+	mustDecodeJSON(t, reportOut, &reportResp)
+	if ok, _ := reportResp["ok"].(bool); !ok {
+		t.Fatalf("expected ok=true, got %#v", reportResp)
+	}
+	if got, _ := reportResp["command"].(string); got != "council report" {
+		t.Fatalf("expected command council report, got %#v", reportResp["command"])
+	}
+	if got := nestedString(t, reportResp, "data", "run_id"); got != runID {
+		t.Fatalf("expected run id %q, got %q", runID, got)
+	}
+
+	show := nestedArray(t, reportResp, "data", "show")
+	if len(show) != 2 || show[0] != "consensus" || show[1] != "majority" {
+		t.Fatalf("expected default show buckets in JSON output, got %#v", show)
+	}
+
+	summary, ok := nestedValue(t, reportResp, "data", "summary").(map[string]any)
+	if !ok {
+		t.Fatalf("expected summary object, got %#v", nestedValue(t, reportResp, "data", "summary"))
+	}
+	if got, _ := summary["consensus"].(float64); got != 1 {
+		t.Fatalf("expected one consensus group, got %#v", summary["consensus"])
+	}
+	if got, _ := summary["majority"].(float64); got != 1 {
+		t.Fatalf("expected one majority group, got %#v", summary["majority"])
+	}
+	if got, _ := summary["minority"].(float64); got != 1 {
+		t.Fatalf("expected one minority group, got %#v", summary["minority"])
+	}
+
+	artifacts := nestedArray(t, reportResp, "data", "report_artifacts")
+	if len(artifacts) != 1 {
+		t.Fatalf("expected one report artifact, got %#v", artifacts)
+	}
+	artifact, ok := artifacts[0].(map[string]any)
+	if !ok {
+		t.Fatalf("expected report artifact object, got %#v", artifacts[0])
+	}
+	if got, _ := artifact["kind"].(string); got != "markdown" {
+		t.Fatalf("expected markdown artifact, got %#v", artifact["kind"])
+	}
+	if got, _ := artifact["path"].(string); got != councilReportArtifactPath(dbPath, runID) {
+		t.Fatalf("expected report artifact path %q, got %#v", councilReportArtifactPath(dbPath, runID), artifact["path"])
+	}
+
+	groups := nestedArray(t, reportResp, "data", "grouped_recommendations")
+	if len(groups) != 2 {
+		t.Fatalf("expected two grouped recommendations in default JSON report, got %#v", groups)
+	}
+	firstGroup, ok := groups[0].(map[string]any)
+	if !ok {
+		t.Fatalf("expected first group object, got %#v", groups[0])
+	}
+	if got, _ := firstGroup["bucket"].(string); got != "consensus" {
+		t.Fatalf("expected first reported group to be consensus, got %#v", firstGroup["bucket"])
+	}
+}
+
 func completeCouncilReviewer(t *testing.T, dbPath, runID, reviewerRole, bodyJSON string) {
 	t.Helper()

@@ -1916,6 +2096,50 @@ func completeCouncilReviewer(t *testing.T, dbPath, runID, reviewerRole, bodyJSON
 	)
 }

+func seedCouncilReportRun(t *testing.T, dbPath, runID string) {
+	t.Helper()
+
+	runOrchCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"council", "start",
+		"--run", runID,
+		"--target", "Review the council reporting flow.",
+	)
+
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		runID,
+		"architecture-reviewer",
+		`{"reviewer_role":"architecture-reviewer","findings":[{"title":"Split contracts","summary":"Transport contracts are mixed into UI code.","proposal":"Move API contract definitions into a dedicated module.","rationale":"This lowers coupling.","confidence":"high","tags":["architecture"],"target_refs":{"repo_path":"."}},{"title":"Share helpers","summary":"Council report rendering paths are repeated.","proposal":"Introduce shared council coordinator helpers for report rendering.","rationale":"This keeps report assembly consistent.","confidence":"medium","tags":["reporting"],"target_refs":{"repo_path":"."}}]}`,
+	)
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		runID,
+		"implementation-reviewer",
+		`{"reviewer_role":"implementation-reviewer","findings":[{"title":"Extract contracts","summary":"Shared transport shapes are duplicated.","proposal":"Move API contract definitions into dedicated module","rationale":"This reduces duplication.","confidence":"high","tags":["maintainability"],"target_refs":{"repo_path":"."}},{"title":"Reuse report helpers","summary":"Formatting logic should stay shared.","proposal":"Introduce shared council coordinator helpers for report rendering","rationale":"This avoids formatter drift.","confidence":"medium","tags":["reporting"],"target_refs":{"repo_path":"."}}]}`,
+	)
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		runID,
+		"risk-reviewer",
+		`{"reviewer_role":"risk-reviewer","findings":[{"title":"Lock contracts","summary":"Contract drift becomes risky over time.","proposal":"Move API contract definitions into a dedicated module.","rationale":"This reduces integration regressions.","confidence":"high","tags":["risk"],"target_refs":{"repo_path":"."}},{"title":"Cover JSON output","summary":"The council report response should stay stable.","proposal":"Add regression tests for council report JSON output.","rationale":"This catches contract regressions earlier.","confidence":"high","tags":["testing"],"target_refs":{"repo_path":"."}}]}`,
+	)
+
+	runOrchCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"council", "tally",
+		"--run", runID,
+		"--similarity", "normal",
+	)
+}
+
 func runInboxCommandEventually(t *testing.T, args ...string) string {
 	t.Helper()