Unify safe-output outcome status enums

docs/adr/54846-unify-outcome-status-enum.md

@@ -0,0 +1,43 @@
+# ADR-54846: Unify Outcome Classification Onto a Single OutcomeStatus Enum
+
+**Date**: 2026-08-22
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+`pkg/cli` maintained two parallel outcome classification types: `OutcomeResult` (a standalone `string` type with constants such as `OutcomeAccepted`, `OutcomeRejected`, etc.) and `OutcomeStatus` (embedded in `OutcomeEvaluation`). `OutcomeReport` carried both — a `Result OutcomeResult` field and the embedded `OutcomeEvaluation.OutcomeStatus` — so every evaluator had to set two fields that were semantically equivalent. This dual representation made safe-output evaluation ambiguous and caused JSONL output to emit duplicate classification data under both `result` and `outcome_status` keys, leaving downstream consumers uncertain about which field to trust.
+
+### Decision
+
+We will eliminate `OutcomeResult` and its constants entirely, extend the existing `OutcomeStatus` enum to cover the values previously unique to `OutcomeResult` (specifically `OutcomeStatusError`, which was `OutcomeError`), and remove the `Result` field from `OutcomeReport`. All evaluators will set `report.OutcomeStatus` (accessed through the embedded `OutcomeEvaluation`) as the sole classification field. JSONL output will emit only `outcome_status`, not `result`.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep Both Enums, Add Synchronization Logic
+
+Keep `OutcomeResult` and `OutcomeStatus` as separate types, but introduce a mapping function that sets both fields consistently whenever an evaluator sets one. This would prevent API breakage for consumers of `Result` but leaves the dual-representation ambiguity in place and adds an indirection layer that future maintainers must remember to invoke.
+
+#### Alternative 2: Deprecate OutcomeStatus in Favor of OutcomeResult
+
+Reverse the direction: remove `OutcomeStatus` from `OutcomeEvaluation` and standardize on `OutcomeResult`. This avoids the breakage direction chosen, but `OutcomeEvaluation` already carried normalized signal and evidence-strength metadata not present on `OutcomeResult`, so going this route would require re-introducing those fields under a different home — net complexity gain with no benefit.
+
+### Consequences
+
+#### Positive
+- Single source of truth for outcome classification; evaluators set exactly one field and there is no ambiguity about which value is authoritative
+- Cleaner serialized schema: `OutcomeReport` JSON exposes only `outcome_status`; JSONL audit entries no longer emit a duplicate `result` key alongside `outcome_status`
+- Reduced cognitive overhead — readers of evaluator code no longer need to track two parallel classification fields and their relationship
+
+#### Negative
+- Breaking schema change for existing JSONL consumers that read the `result` field; any pipeline or dashboard filtering on `result` must migrate to `outcome_status`
+- High diff volume across many evaluator files, even though each individual change is a mechanical rename with no semantic complexity; this increases review surface area
+
+#### Neutral
+- `OutcomeStatusSkipped` was already present in `OutcomeStatus`; this change absorbs it into the unified enum with no behavioral change, but its presence is now formally part of the consolidated domain invariant verified by tests
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/outcome_domain_breakdown.go

@@ -47,13 +47,13 @@ func ComputeDomainBreakdowns(reports []OutcomeReport) []DomainBreakdown {
 			domain.Attempted++
 			domain.TotalObjectiveValue += report.ObjectiveValue
 
-			switch report.Result {
-			case OutcomeAccepted:
+			switch report.OutcomeStatus {
+			case OutcomeStatusAccepted:
 				domain.Accepted++
 				domain.AcceptedObjectiveValue += report.ObjectiveValue
-			case OutcomeRejected:
+			case OutcomeStatusRejected:
 				domain.Rejected++
-			case OutcomePending:
+			case OutcomeStatusPending:
 				domain.Pending++
 			}
 		}
@@ -68,12 +68,12 @@ func ComputeDomainBreakdowns(reports []OutcomeReport) []DomainBreakdown {
 			domain := domains["unmapped"]
 			domain.Attempted++
 
-			switch report.Result {
-			case OutcomeAccepted:
+			switch report.OutcomeStatus {
+			case OutcomeStatusAccepted:
 				domain.Accepted++
-			case OutcomeRejected:
+			case OutcomeStatusRejected:
 				domain.Rejected++
-			case OutcomePending:
+			case OutcomeStatusPending:
 				domain.Pending++
 			}
 		}

pkg/cli/outcome_domain_breakdown_test.go

@@ -12,18 +12,18 @@ import (
 func TestComputeDomainBreakdowns_SortsByValueThenLabel(t *testing.T) {
 	reports := []OutcomeReport{
 		{
-			Result:          OutcomeAccepted,
-			ObjectiveValue:  20,
-			ObjectiveLabels: []string{"beta"},
+			OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted},
+			ObjectiveValue:    20,
+			ObjectiveLabels:   []string{"beta"},
 		},
 		{
-			Result:          OutcomeAccepted,
-			ObjectiveValue:  20,
-			ObjectiveLabels: []string{"alpha"},
+			OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted},
+			ObjectiveValue:    20,
+			ObjectiveLabels:   []string{"alpha"},
 		},
 		{
-			Result:          OutcomeRejected,
-			ObjectiveLabels: []string{"gamma"},
+			OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected},
+			ObjectiveLabels:   []string{"gamma"},
 		},
 	}
 

pkg/cli/outcome_eval.go

@@ -23,42 +23,27 @@ var outcomeEvalLog = logger.New("cli:outcome_eval")
 var objectiveMappingGHAPIGetArray = ghAPIGetArray
 var objectiveMappingGHAPIGraphQL = ghAPIGraphQL
 
-// OutcomeResult classifies what happened to a safe output after execution.
-type OutcomeResult string
-
-const (
-	OutcomeAccepted       OutcomeResult = "accepted"
-	OutcomeRejected       OutcomeResult = "rejected"
-	OutcomeIgnored        OutcomeResult = "ignored"
-	OutcomePending        OutcomeResult = "pending"
-	OutcomeUnknown        OutcomeResult = "unknown"
-	OutcomeLifecycle      OutcomeResult = "lifecycle"
-	OutcomeLifecycleClose OutcomeResult = "lifecycle_close"
-	OutcomeError          OutcomeResult = "error"
-)
-
 // OutcomeReport is the result of evaluating one safe output item.
 type OutcomeReport struct {
 	OutcomeEvaluation
-	Type               string        `json:"type" console:"header:Type"`
-	ObjectURL          string        `json:"object_url,omitempty" console:"header:URL,omitempty"`
-	ObjectNumber       int           `json:"object_number,omitempty" console:"header:#,omitempty"`
-	TracedRootURL      string        `json:"traced_root_url,omitempty" console:"-"`
-	AttributionStatus  string        `json:"attribution_status,omitempty" console:"-"`
-	AttributionSource  string        `json:"attribution_source,omitempty" console:"-"`
-	Repo               string        `json:"repo,omitempty" console:"header:Repo,omitempty"`
-	Result             OutcomeResult `json:"result" console:"header:Outcome"`
-	Detail             string        `json:"detail,omitempty" console:"header:Detail,omitempty"`
-	TimeToOutcomeHours float64       `json:"time_to_outcome_hours,omitempty" console:"header:Time,omitempty"`
-	HumanComments      int           `json:"human_comments,omitempty" console:"header:Comments,omitempty"`
-	HumanEdits         int           `json:"human_edits,omitempty" console:"header:Edits,omitempty"`
-	HumanReviews       int           `json:"human_reviews,omitempty" console:"header:Reviews,omitempty"`
-	ZeroTouch          bool          `json:"zero_touch,omitempty" console:"header:Zero-touch,omitempty"`
-	ObjectiveValue     int           `json:"objective_value,omitempty" console:"header:Obj Value,omitempty"`
-	ObjectiveLabels    []string      `json:"objective_labels,omitempty" console:"-"`
-	CreatedAt          string        `json:"created_at" console:"-"`
-	CheckedAt          string        `json:"checked_at" console:"-"`
-	EvalError          string        `json:"eval_error,omitempty" console:"-"`
+	Type               string   `json:"type" console:"header:Type"`
+	ObjectURL          string   `json:"object_url,omitempty" console:"header:URL,omitempty"`
+	ObjectNumber       int      `json:"object_number,omitempty" console:"header:#,omitempty"`
+	TracedRootURL      string   `json:"traced_root_url,omitempty" console:"-"`
+	AttributionStatus  string   `json:"attribution_status,omitempty" console:"-"`
+	AttributionSource  string   `json:"attribution_source,omitempty" console:"-"`
+	Repo               string   `json:"repo,omitempty" console:"header:Repo,omitempty"`
+	Detail             string   `json:"detail,omitempty" console:"header:Detail,omitempty"`
+	TimeToOutcomeHours float64  `json:"time_to_outcome_hours,omitempty" console:"header:Time,omitempty"`
+	HumanComments      int      `json:"human_comments,omitempty" console:"header:Comments,omitempty"`
+	HumanEdits         int      `json:"human_edits,omitempty" console:"header:Edits,omitempty"`
+	HumanReviews       int      `json:"human_reviews,omitempty" console:"header:Reviews,omitempty"`
+	ZeroTouch          bool     `json:"zero_touch,omitempty" console:"header:Zero-touch,omitempty"`
+	ObjectiveValue     int      `json:"objective_value,omitempty" console:"header:Obj Value,omitempty"`
+	ObjectiveLabels    []string `json:"objective_labels,omitempty" console:"-"`
+	CreatedAt          string   `json:"created_at" console:"-"`
+	CheckedAt          string   `json:"checked_at" console:"-"`
+	EvalError          string   `json:"eval_error,omitempty" console:"-"`
 }
 
 // OutcomeSummary aggregates outcomes across multiple safe output items.
@@ -188,10 +173,10 @@ func ComputeOutcomeSummary(reports []OutcomeReport, mapping *github.ObjectiveMap
 		if eval.Signal == "target_exists_only" {
 			s.FallbackExistsOnlyCount++
 		}
-		switch r.Result {
-		case OutcomeLifecycle, OutcomeLifecycleClose:
+		switch eval.OutcomeStatus {
+		case OutcomeStatusLifecycle, OutcomeStatusLifecycleClose:
 			s.Lifecycle++
-		case OutcomeError:
+		case OutcomeStatusError:
 			s.Errors++
 		}
 		if r.TimeToOutcomeHours > 0 {

pkg/cli/outcome_eval_agent.go

@@ -23,15 +23,15 @@ func evalAssignToAgent(ctx context.Context, item CreatedItemReport, repoOverride
 	}
 	if num == 0 || repo == "" {
 		outcomeEvalAgentLog.Printf("Missing issue number or repo: num=%d, repo=%s", num, repo)
-		report.Result = OutcomeError
+		report.OutcomeStatus = OutcomeStatusError
 		report.EvalError = "missing issue number or repo"
 		return report
 	}
 
 	// Check issue state first
 	issueData, err := ghAPIGet(ctx, fmt.Sprintf("issues/%d", num), repo)
 	if err != nil {
-		report.Result = OutcomeError
+		report.OutcomeStatus = OutcomeStatusError
 		report.EvalError = err.Error()
 		return report
 	}
@@ -87,21 +87,21 @@ func evalAssignToAgent(ctx context.Context, item CreatedItemReport, repoOverride
 
 				switch {
 				case merged:
-					report.Result = OutcomeAccepted
+					report.OutcomeStatus = OutcomeStatusAccepted
 					report.Detail = fmt.Sprintf("agent PR #%d merged", prNumber)
 					if mergedAt != "" && item.Timestamp != "" {
 						report.TimeToOutcomeHours = timeBetween(item.Timestamp, mergedAt)
 					}
 					return report
 				case prState == "closed":
-					report.Result = OutcomeRejected
+					report.OutcomeStatus = OutcomeStatusRejected
 					report.Detail = fmt.Sprintf("agent PR #%d closed without merge", prNumber)
 					if closedAt != "" && item.Timestamp != "" {
 						report.TimeToOutcomeHours = timeBetween(item.Timestamp, closedAt)
 					}
 					return report
 				default:
-					report.Result = OutcomePending
+					report.OutcomeStatus = OutcomeStatusPending
 					report.Detail = fmt.Sprintf("agent PR #%d open", prNumber)
 					return report
 				}
@@ -112,17 +112,17 @@ func evalAssignToAgent(ctx context.Context, item CreatedItemReport, repoOverride
 	// No agent PR found — check if issue was resolved by other means
 	switch {
 	case state == "closed" && stateReason == "completed":
-		report.Result = OutcomeAccepted
+		report.OutcomeStatus = OutcomeStatusAccepted
 		report.Detail = "issue resolved (no agent PR found)"
 		closedAt, _ := issueData["closed_at"].(string)
 		if closedAt != "" && item.Timestamp != "" {
 			report.TimeToOutcomeHours = timeBetween(item.Timestamp, closedAt)
 		}
 	case state == "closed":
-		report.Result = OutcomeRejected
+		report.OutcomeStatus = OutcomeStatusRejected
 		report.Detail = "issue closed without resolution, no agent PR"
 	default:
-		report.Result = OutcomeIgnored
+		report.OutcomeStatus = OutcomeStatusIgnored
 		report.Detail = "no agent PR created"
 	}
 

pkg/cli/outcome_eval_comment.go

@@ -25,7 +25,7 @@ func evalAddComment(ctx context.Context, item CreatedItemReport, repoOverride st
 	commentID := extractCommentID(item.URL)
 	if commentID == "" {
 		outcomeEvalCommentLog.Printf("Unable to extract comment ID from URL: %s", item.URL)
-		report.Result = OutcomeError
+		report.OutcomeStatus = OutcomeStatusError
 		report.EvalError = "cannot extract comment ID from URL"
 		return report
 	}
@@ -35,11 +35,11 @@ func evalAddComment(ctx context.Context, item CreatedItemReport, repoOverride st
 		// 404 means deleted
 		if errorutil.IsNotFoundError(err) {
 			outcomeEvalCommentLog.Printf("Comment %s deleted (404)", commentID)
-			report.Result = OutcomeRejected
+			report.OutcomeStatus = OutcomeStatusRejected
 			report.Detail = "deleted"
 			return report
 		}
-		report.Result = OutcomeError
+		report.OutcomeStatus = OutcomeStatusError
 		report.EvalError = err.Error()
 		return report
 	}
@@ -73,10 +73,10 @@ func evalAddComment(ctx context.Context, item CreatedItemReport, repoOverride st
 
 	switch {
 	case totalReactions > 0 || replyCount > 0:
-		report.Result = OutcomeAccepted
+		report.OutcomeStatus = OutcomeStatusAccepted
 		report.Detail = fmt.Sprintf("%d reactions, %d replies", totalReactions, replyCount)
 	default:
-		report.Result = OutcomeIgnored
+		report.OutcomeStatus = OutcomeStatusIgnored
 		report.Detail = "no engagement"
 	}