Add council review tally command

2026-03-19 15:26:11 +08:00
parent dd6a0e31b6
commit 740a7b4acd
7 changed files with 810 additions and 10 deletions
@@ -10,5 +10,6 @@ func newCouncilCmd(root *rootOptions) *cobra.Command {

 	cmd.AddCommand(newCouncilStartCmd(root))
 	cmd.AddCommand(newCouncilWaitCmd(root))
+	cmd.AddCommand(newCouncilTallyCmd(root))
 	return cmd
 }
@@ -0,0 +1,64 @@
+package orch
+
+import (
+	"fmt"
+
+	"ai-workflow-skill/internal/protocol"
+	"ai-workflow-skill/internal/store"
+
+	"github.com/spf13/cobra"
+)
+
+type councilTallyOptions struct {
+	runID      string
+	similarity string
+}
+
+func newCouncilTallyCmd(root *rootOptions) *cobra.Command {
+	opts := &councilTallyOptions{}
+
+	cmd := &cobra.Command{
+		Use:   "tally",
+		Short: "Group reviewer findings and compute council support counts",
+		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()
+
+			result, err := store.NewOrchStore(sqlDB).TallyCouncil(ctx, store.CouncilTallyInput{
+				RunID:      opts.runID,
+				Similarity: opts.similarity,
+			})
+			if err != nil {
+				return err
+			}
+
+			resp := protocol.Success{
+				OK:      true,
+				Command: "council tally",
+				Data: map[string]any{
+					"run_id":                  result.RunID,
+					"similarity":              result.Similarity,
+					"counts":                  result.Counts,
+					"grouped_recommendations": result.GroupedRecommendations,
+				},
+			}
+			if root.json {
+				return protocol.WriteJSON(cmd.OutOrStdout(), resp)
+			}
+
+			_, err = fmt.Fprintf(cmd.OutOrStdout(), "tallied council run %s into %d groups\n", result.RunID, len(result.GroupedRecommendations))
+			return err
+		},
+	}
+
+	cmd.Flags().StringVar(&opts.runID, "run", "", "Council run ID")
+	cmd.Flags().StringVar(&opts.similarity, "similarity", "normal", "Grouping mode: strict or normal")
+	_ = cmd.MarkFlagRequired("run")
+
+	return cmd
+}
@@ -1712,6 +1712,210 @@ func TestOrchCouncilWaitTimesOutWhenReviewersIncomplete(t *testing.T) {
 	}
 }

+func TestOrchCouncilTallyGroupsReviewerFindingsNormal(t *testing.T) {
+	t.Parallel()
+
+	dbPath := filepath.Join(t.TempDir(), "coord.db")
+
+	runOrchCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"council", "start",
+		"--run", "council_blog_tally_001",
+		"--target", "Review the current blog architecture.",
+	)
+
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		"council_blog_tally_001",
+		"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","coupling"],"target_refs":{"repo_path":"."}}]}`,
+	)
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		"council_blog_tally_001",
+		"implementation-reviewer",
+		`{"reviewer_role":"implementation-reviewer","findings":[{"title":"Extract API contracts","summary":"Shared transport shapes are duplicated.","proposal":"Move API contract definitions into dedicated module","rationale":"This reduces duplication.","confidence":"medium","tags":["maintainability"],"target_refs":{"repo_path":"."}}]}`,
+	)
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		"council_blog_tally_001",
+		"risk-reviewer",
+		`{"reviewer_role":"risk-reviewer","findings":[{"title":"Add auth integration tests","summary":"Login regressions are hard to catch.","proposal":"Add integration tests for auth flows.","rationale":"This catches regressions earlier.","confidence":"high","tags":["risk","testing"],"target_refs":{"repo_path":"."}}]}`,
+	)
+
+	tallyOut := runOrchCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"council", "tally",
+		"--run", "council_blog_tally_001",
+		"--similarity", "normal",
+	)
+
+	var tallyResp map[string]any
+	mustDecodeJSON(t, tallyOut, &tallyResp)
+	if got := nestedString(t, tallyResp, "data", "similarity"); got != "normal" {
+		t.Fatalf("expected normal similarity, got %q", got)
+	}
+	counts, ok := nestedValue(t, tallyResp, "data", "counts").(map[string]any)
+	if !ok {
+		t.Fatalf("expected counts object, got %#v", nestedValue(t, tallyResp, "data", "counts"))
+	}
+	if got, _ := counts["majority"].(float64); got != 1 {
+		t.Fatalf("expected one majority group, got %#v", counts["majority"])
+	}
+	if got, _ := counts["minority"].(float64); got != 1 {
+		t.Fatalf("expected one minority group, got %#v", counts["minority"])
+	}
+
+	groups := nestedArray(t, tallyResp, "data", "grouped_recommendations")
+	if len(groups) != 2 {
+		t.Fatalf("expected two grouped recommendations, got %#v", groups)
+	}
+	firstGroup, ok := groups[0].(map[string]any)
+	if !ok {
+		t.Fatalf("expected group object, got %#v", groups[0])
+	}
+	if got, _ := firstGroup["bucket"].(string); got != "majority" {
+		t.Fatalf("expected first group majority, got %#v", firstGroup["bucket"])
+	}
+	if got, _ := firstGroup["support_count"].(float64); got != 2 {
+		t.Fatalf("expected support_count 2, got %#v", firstGroup["support_count"])
+	}
+
+	sqlDB, err := openOrchDB(t.Context(), dbPath)
+	if err != nil {
+		t.Fatalf("open orch db: %v", err)
+	}
+	defer sqlDB.Close()
+
+	var findingsCount int
+	if err := sqlDB.QueryRowContext(t.Context(), `SELECT COUNT(*) FROM council_findings WHERE run_id = ?`, "council_blog_tally_001").Scan(&findingsCount); err != nil {
+		t.Fatalf("count council_findings: %v", err)
+	}
+	if findingsCount != 3 {
+		t.Fatalf("expected 3 council findings, got %d", findingsCount)
+	}
+	var groupsCount int
+	if err := sqlDB.QueryRowContext(t.Context(), `SELECT COUNT(*) FROM council_groups WHERE run_id = ?`, "council_blog_tally_001").Scan(&groupsCount); err != nil {
+		t.Fatalf("count council_groups: %v", err)
+	}
+	if groupsCount != 2 {
+		t.Fatalf("expected 2 council groups, got %d", groupsCount)
+	}
+}
+
+func TestOrchCouncilTallyStrictKeepsDistinctProposals(t *testing.T) {
+	t.Parallel()
+
+	dbPath := filepath.Join(t.TempDir(), "coord.db")
+
+	runOrchCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"council", "start",
+		"--run", "council_blog_tally_002",
+		"--target", "Review the current blog architecture.",
+	)
+
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		"council_blog_tally_002",
+		"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":"."}}]}`,
+	)
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		"council_blog_tally_002",
+		"implementation-reviewer",
+		`{"reviewer_role":"implementation-reviewer","findings":[{"title":"Extract API contracts","summary":"Shared transport shapes are duplicated.","proposal":"Move API contract definitions into dedicated module","rationale":"This reduces duplication.","confidence":"medium","tags":["maintainability"],"target_refs":{"repo_path":"."}}]}`,
+	)
+	completeCouncilReviewer(
+		t,
+		dbPath,
+		"council_blog_tally_002",
+		"risk-reviewer",
+		`{"reviewer_role":"risk-reviewer","findings":[{"title":"Add auth integration tests","summary":"Login regressions are hard to catch.","proposal":"Add integration tests for auth flows.","rationale":"This catches regressions earlier.","confidence":"high","tags":["risk"],"target_refs":{"repo_path":"."}}]}`,
+	)
+
+	tallyOut := runOrchCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"council", "tally",
+		"--run", "council_blog_tally_002",
+		"--similarity", "strict",
+	)
+
+	var tallyResp map[string]any
+	mustDecodeJSON(t, tallyOut, &tallyResp)
+	counts, ok := nestedValue(t, tallyResp, "data", "counts").(map[string]any)
+	if !ok {
+		t.Fatalf("expected counts object, got %#v", nestedValue(t, tallyResp, "data", "counts"))
+	}
+	if got, _ := counts["minority"].(float64); got != 3 {
+		t.Fatalf("expected three minority groups in strict mode, got %#v", counts["minority"])
+	}
+	groups := nestedArray(t, tallyResp, "data", "grouped_recommendations")
+	if len(groups) != 3 {
+		t.Fatalf("expected three distinct groups in strict mode, got %#v", groups)
+	}
+}
+
+func completeCouncilReviewer(t *testing.T, dbPath, runID, reviewerRole, bodyJSON string) {
+	t.Helper()
+
+	sqlDB, err := openOrchDB(t.Context(), dbPath)
+	if err != nil {
+		t.Fatalf("open orch db: %v", err)
+	}
+
+	var threadID string
+	if err := sqlDB.QueryRowContext(
+		t.Context(),
+		`SELECT a.thread_id
+		 FROM council_reviewers cr
+		 JOIN task_attempts a
+		   ON a.run_id = cr.run_id
+		  AND a.task_id = cr.task_id
+		  AND a.attempt_no = 1
+		 WHERE cr.run_id = ? AND cr.reviewer_role = ?`,
+		runID,
+		reviewerRole,
+	).Scan(&threadID); err != nil {
+		sqlDB.Close()
+		t.Fatalf("query council reviewer thread: %v", err)
+	}
+	sqlDB.Close()
+
+	runInboxCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"claim",
+		"--agent", reviewerRole,
+		"--thread", threadID,
+	)
+	runInboxCommand(
+		t,
+		"--db", dbPath,
+		"--json",
+		"done",
+		"--agent", reviewerRole,
+		"--thread", threadID,
+		"--summary", "Review complete",
+		"--body", bodyJSON,
+	)
+}
+
 func runInboxCommandEventually(t *testing.T, args ...string) string {
 	t.Helper()