diff --git a/docs/adr/54846-unify-outcome-status-enum.md b/docs/adr/54846-unify-outcome-status-enum.md new file mode 100644 index 00000000000..5901eef5a97 --- /dev/null +++ b/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.* diff --git a/pkg/cli/outcome_domain_breakdown.go b/pkg/cli/outcome_domain_breakdown.go index a0677045d99..f7df7743622 100644 --- a/pkg/cli/outcome_domain_breakdown.go +++ b/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++ } } diff --git a/pkg/cli/outcome_domain_breakdown_test.go b/pkg/cli/outcome_domain_breakdown_test.go index 5fa78e92c54..01e64397995 100644 --- a/pkg/cli/outcome_domain_breakdown_test.go +++ b/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"}, }, } diff --git a/pkg/cli/outcome_eval.go b/pkg/cli/outcome_eval.go index f6f79597c3b..32dc73d68d8 100644 --- a/pkg/cli/outcome_eval.go +++ b/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 { diff --git a/pkg/cli/outcome_eval_agent.go b/pkg/cli/outcome_eval_agent.go index a42ffe25a78..aec14691ab3 100644 --- a/pkg/cli/outcome_eval_agent.go +++ b/pkg/cli/outcome_eval_agent.go @@ -23,7 +23,7 @@ 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 } @@ -31,7 +31,7 @@ func evalAssignToAgent(ctx context.Context, item CreatedItemReport, repoOverride // 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" } diff --git a/pkg/cli/outcome_eval_comment.go b/pkg/cli/outcome_eval_comment.go index 55c1531fa4d..7fb09e4e875 100644 --- a/pkg/cli/outcome_eval_comment.go +++ b/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" } diff --git a/pkg/cli/outcome_eval_formal_test.go b/pkg/cli/outcome_eval_formal_test.go index 1c5ee86d167..11b90a67579 100644 --- a/pkg/cli/outcome_eval_formal_test.go +++ b/pkg/cli/outcome_eval_formal_test.go @@ -22,14 +22,14 @@ import ( "github.com/stretchr/testify/require" ) -// TestFormalOutcomeDomainInvariant verifies that every OutcomeResult produced by +// TestFormalOutcomeDomainInvariant verifies that every OutcomeStatus produced by // the evaluation engine is within the six outcome categories defined in the spec, // or is a recognized internal state that must be normalized before external emission. // // Formal predicate (TLA+): // // OutcomeDomain ≜ -// ∀ r ∈ OutcomeResult : +// ∀ r ∈ OutcomeStatus : // r ∈ {"accepted","rejected","ignored","pending","lifecycle","lifecycle_close"} // ∨ r ∈ {"unknown","error"} (* internal-only; normalized before OTel emission *) // @@ -49,27 +49,28 @@ func TestFormalOutcomeDomainInvariant(t *testing.T) { internalOnly := map[string]bool{ "unknown": true, "error": true, + "skipped": true, } - // Every declared OutcomeResult constant must be in the spec domain or internal. - allResults := []OutcomeResult{ - OutcomeAccepted, OutcomeRejected, OutcomeIgnored, OutcomePending, - OutcomeLifecycle, OutcomeLifecycleClose, OutcomeUnknown, OutcomeError, + // Every declared OutcomeStatus constant must be in the spec domain or internal. + allResults := []OutcomeStatus{ + OutcomeStatusAccepted, OutcomeStatusRejected, OutcomeStatusIgnored, OutcomeStatusPending, + OutcomeStatusLifecycle, OutcomeStatusLifecycleClose, OutcomeStatusUnknown, OutcomeStatusSkipped, OutcomeStatusError, } for _, r := range allResults { s := string(r) assert.True(t, specDomain[s] || internalOnly[s], - "P1: OutcomeResult %q must be in spec domain or recognized internal state", s) + "P1: OutcomeStatus %q must be in spec domain or recognized internal state", s) } // ComputeOutcomeSummary must count each spec-defined outcome correctly. reports := []OutcomeReport{ - {Type: "create_pull_request", Result: OutcomeAccepted}, - {Type: "create_issue", Result: OutcomeRejected}, - {Type: "add_comment", Result: OutcomeIgnored}, - {Type: "add_labels", Result: OutcomePending}, - {Type: "close_issue", Result: OutcomeLifecycle}, - {Type: "close_pull_request", Result: OutcomeLifecycleClose}, + {Type: "create_pull_request", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted}}, + {Type: "create_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected}}, + {Type: "add_comment", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusIgnored}}, + {Type: "add_labels", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}}, + {Type: "close_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycle}}, + {Type: "close_pull_request", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycleClose}}, } summary := ComputeOutcomeSummary(reports, github.DefaultObjectiveMapping()) assert.Equal(t, 6, summary.Total, "P1: total must cover the six spec-defined non-internal outcomes") @@ -89,7 +90,7 @@ func TestFormalOutcomeDomainInvariant(t *testing.T) { // item:CreatedItemReport → apiErr:HTTPError → // Tot OutcomeReport // (requires apiErr.status ∈ {500, 502, 503, 429}) -// (ensures fun r → r.Result ≠ OutcomeAccepted ∧ r.Result ≠ OutcomeRejected) +// (ensures fun r → r.OutcomeStatus ≠ OutcomeStatusAccepted ∧ r.OutcomeStatus ≠ OutcomeStatusRejected) // // Specification reference: specs/safe-output-outcome-evaluation.md §Norms (rules 2, 4) func TestFormalAPIFailurePending(t *testing.T) { @@ -114,9 +115,9 @@ func TestFormalAPIFailurePending(t *testing.T) { item := CreatedItemReport{Type: "close_issue", Number: 99, Repo: "owner/repo"} report := evalCloseSticky(context.Background(), item, "owner/repo") - assert.NotEqual(t, OutcomeAccepted, report.Result, + assert.NotEqual(t, OutcomeStatusAccepted, report.OutcomeStatus, "P2: API error %q must not yield accepted", tc.errText) - assert.NotEqual(t, OutcomeRejected, report.Result, + assert.NotEqual(t, OutcomeStatusRejected, report.OutcomeStatus, "P2: API error %q must not yield rejected", tc.errText) }) } @@ -139,9 +140,9 @@ func TestFormal404Classification(t *testing.T) { // Persistent object: "deleted" detail maps to rejected (persistent 404 classification). t.Run("persistent object deleted → rejected", func(t *testing.T) { report := OutcomeReport{ - Type: "create_issue", - Result: OutcomeRejected, - Detail: "deleted", + Type: "create_issue", + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected}, + Detail: "deleted", } eval := normalizeOutcomeEvaluation(report) assert.Equal(t, OutcomeStatusRejected, eval.OutcomeStatus, @@ -153,9 +154,9 @@ func TestFormal404Classification(t *testing.T) { // Transient target: "no engagement" maps to ignored (transient 404 classification). t.Run("transient target no engagement → ignored", func(t *testing.T) { report := OutcomeReport{ - Type: "add_comment", - Result: OutcomeIgnored, - Detail: "no engagement", + Type: "add_comment", + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusIgnored}, + Detail: "no engagement", } eval := normalizeOutcomeEvaluation(report) assert.Equal(t, OutcomeStatusIgnored, eval.OutcomeStatus, @@ -171,7 +172,7 @@ func TestFormal404Classification(t *testing.T) { } item := CreatedItemReport{Type: "close_issue", Number: 99, Repo: "owner/repo"} report := evalCloseSticky(context.Background(), item, "owner/repo") - assert.NotEqual(t, OutcomeAccepted, report.Result, + assert.NotEqual(t, OutcomeStatusAccepted, report.OutcomeStatus, "P3: 404 API error must not yield accepted") }) } @@ -228,25 +229,25 @@ func TestFormalPRMergeAcceptance(t *testing.T) { cases := []struct { name string pr map[string]any - wantResult OutcomeResult + wantResult OutcomeStatus wantDetail string }{ { name: "merged PR → accepted", pr: map[string]any{"merged": true, "state": "closed"}, - wantResult: OutcomeAccepted, + wantResult: OutcomeStatusAccepted, wantDetail: "merged", }, { name: "closed PR without merge → rejected", pr: map[string]any{"merged": false, "state": "closed"}, - wantResult: OutcomeRejected, + wantResult: OutcomeStatusRejected, wantDetail: "closed without merge", }, { name: "open PR → pending", pr: map[string]any{"merged": false, "state": "open"}, - wantResult: OutcomePending, + wantResult: OutcomeStatusPending, wantDetail: "open", }, } @@ -269,11 +270,11 @@ func TestFormalPRMergeAcceptance(t *testing.T) { report := evalCreatePullRequest(context.Background(), CreatedItemReport{ Type: "create_pull_request", Number: 1, Repo: "owner/repo", }, "owner/repo") - assert.Equal(t, tc.wantResult, report.Result, + assert.Equal(t, tc.wantResult, report.OutcomeStatus, "P5: PR state must yield %s", tc.wantResult) assert.Equal(t, tc.wantDetail, report.Detail, "P5: PR state must set detail %q", tc.wantDetail) - if tc.wantResult != OutcomeAccepted { + if tc.wantResult != OutcomeStatusAccepted { assert.False(t, report.ZeroTouch, "P5: a non-accepted PR must not be marked zero-touch") } @@ -294,7 +295,7 @@ func TestFormalAPIErrorNotTerminal(t *testing.T) { Type: "create_pull_request", Number: 1, Repo: "owner/repo", }, "owner/repo") - assert.Equal(t, OutcomeError, report.Result, + assert.Equal(t, OutcomeStatusError, report.OutcomeStatus, "P14: an API error must not produce a terminal outcome") assert.NotEmpty(t, report.EvalError, "P14: an API error must be recorded") } @@ -359,7 +360,7 @@ func TestFormalZeroTouchRequiresNoReviews(t *testing.T) { Type: "create_pull_request", Number: 1, Repo: "owner/repo", }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result, "P15: test PR must be accepted") + assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus, "P15: test PR must be accepted") assert.Equal(t, tc.wantZeroTouch, report.ZeroTouch, "P15: zero_touch requires no non-bot comments and no reviews") }) @@ -382,28 +383,28 @@ func TestFormalZeroTouchRequiresNoReviews(t *testing.T) { func TestFormalIssueBotCloseLifecycle(t *testing.T) { cases := []struct { name string - result OutcomeResult + result OutcomeStatus detail string wantStatus OutcomeStatus wantSignal string }{ { name: "bot closed not_planned → lifecycle signal", - result: OutcomeLifecycle, + result: OutcomeStatusLifecycle, detail: "closed by bot (lifecycle)", wantStatus: OutcomeStatusLifecycle, wantSignal: "lifecycle", }, { name: "human closed not_planned → rejected", - result: OutcomeRejected, + result: OutcomeStatusRejected, detail: "closed as not planned", wantStatus: OutcomeStatusRejected, wantSignal: "closed_not_planned", }, { name: "resolved as completed → accepted", - result: OutcomeAccepted, + result: OutcomeStatusAccepted, detail: "completed", wantStatus: OutcomeStatusAccepted, wantSignal: "completed", @@ -412,9 +413,9 @@ func TestFormalIssueBotCloseLifecycle(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { report := OutcomeReport{ - Type: "create_issue", - Result: tc.result, - Detail: tc.detail, + Type: "create_issue", + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: tc.result}, + Detail: tc.detail, } eval := normalizeOutcomeEvaluation(report) assert.Equal(t, tc.wantStatus, eval.OutcomeStatus, @@ -560,7 +561,7 @@ func TestFormalCloseStickyReopenRejection(t *testing.T) { name string state string events []map[string]any - wantResult OutcomeResult + wantResult OutcomeStatus wantDetail string }{ { @@ -569,7 +570,7 @@ func TestFormalCloseStickyReopenRejection(t *testing.T) { events: []map[string]any{ {"event": "closed", "actor": map[string]any{"login": "github-actions[bot]"}}, }, - wantResult: OutcomeLifecycleClose, + wantResult: OutcomeStatusLifecycleClose, wantDetail: "closed by bot (lifecycle_close)", }, { @@ -578,19 +579,19 @@ func TestFormalCloseStickyReopenRejection(t *testing.T) { events: []map[string]any{ {"event": "closed", "actor": map[string]any{"login": "octocat"}}, }, - wantResult: OutcomeRejected, + wantResult: OutcomeStatusRejected, wantDetail: "closed by non-bot", }, { name: "reopened → rejected", state: "open", - wantResult: OutcomeRejected, + wantResult: OutcomeStatusRejected, wantDetail: "reopened", }, { name: "missing close event provenance → error", state: "closed", - wantResult: OutcomeError, + wantResult: OutcomeStatusError, wantDetail: "close provenance unavailable", }, } @@ -614,7 +615,7 @@ func TestFormalCloseStickyReopenRejection(t *testing.T) { item := CreatedItemReport{Type: "close_issue", Number: 99, Repo: "owner/repo"} report := evalCloseSticky(context.Background(), item, "owner/repo") - assert.Equal(t, tc.wantResult, report.Result, + assert.Equal(t, tc.wantResult, report.OutcomeStatus, "P9: state=%q must yield %s", tc.state, tc.wantResult) assert.Equal(t, tc.wantDetail, report.Detail, "P9: state=%q must set detail %q", tc.state, tc.wantDetail) @@ -638,7 +639,7 @@ func TestFormalCloseStickyRejectsMergedPullRequest(t *testing.T) { report := evalCloseSticky(context.Background(), CreatedItemReport{Type: "close_pull_request", Number: 99, Repo: "owner/repo"}, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result, "P9: merged PR must be rejected for close_pull_request") + assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus, "P9: merged PR must be rejected for close_pull_request") assert.Equal(t, "merged", report.Detail, "P9: merged PR must record merged detail") assert.Empty(t, report.EvalError, "P9: merged PR classification must not depend on close provenance lookup") } @@ -652,19 +653,19 @@ func TestFormalLifecycleNormalizationFallbacks(t *testing.T) { }{ { name: "lifecycle result fallback without detail", - report: OutcomeReport{Type: "close_issue", Result: OutcomeLifecycle}, + report: OutcomeReport{Type: "close_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycle}}, wantStatus: OutcomeStatusLifecycle, wantSignal: "lifecycle", }, { name: "lifecycle_close result fallback without detail", - report: OutcomeReport{Type: "close_issue", Result: OutcomeLifecycleClose}, + report: OutcomeReport{Type: "close_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycleClose}}, wantStatus: OutcomeStatusLifecycleClose, wantSignal: "lifecycle_close", }, { name: "lifecycle_close detail maps before generic bot close", - report: OutcomeReport{Type: "close_issue", Result: OutcomeUnknown, Detail: "closed by bot (lifecycle_close)"}, + report: OutcomeReport{Type: "close_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusUnknown}, Detail: "closed by bot (lifecycle_close)"}, wantStatus: OutcomeStatusLifecycleClose, wantSignal: "lifecycle_close", }, @@ -698,9 +699,9 @@ func TestFormalLifecycleNormalizationFallbacks(t *testing.T) { func TestFormalDerivedMetricsConsistency(t *testing.T) { t.Run("acceptance_rate = accepted / (accepted + rejected)", func(t *testing.T) { reports := []OutcomeReport{ - {Type: "create_pull_request", Result: OutcomeAccepted}, - {Type: "create_pull_request", Result: OutcomeAccepted}, - {Type: "create_issue", Result: OutcomeRejected}, + {Type: "create_pull_request", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted}}, + {Type: "create_pull_request", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted}}, + {Type: "create_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected}}, } summary := ComputeOutcomeSummary(reports, github.DefaultObjectiveMapping()) // 2 accepted, 1 rejected → acceptance_rate = 2/3 @@ -710,10 +711,10 @@ func TestFormalDerivedMetricsConsistency(t *testing.T) { t.Run("waste_rate = rejected / total", func(t *testing.T) { reports := []OutcomeReport{ - {Type: "create_pull_request", Result: OutcomeAccepted}, - {Type: "create_issue", Result: OutcomeRejected}, - {Type: "add_comment", Result: OutcomeIgnored}, - {Type: "add_labels", Result: OutcomePending}, + {Type: "create_pull_request", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted}}, + {Type: "create_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected}}, + {Type: "add_comment", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusIgnored}}, + {Type: "add_labels", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}}, } summary := ComputeOutcomeSummary(reports, github.DefaultObjectiveMapping()) // 1 rejected / 4 total → waste_rate = 0.25 @@ -731,8 +732,8 @@ func TestFormalDerivedMetricsConsistency(t *testing.T) { t.Run("division-by-zero safety: only pending outcomes", func(t *testing.T) { reports := []OutcomeReport{ - {Type: "add_labels", Result: OutcomePending}, - {Type: "add_labels", Result: OutcomePending}, + {Type: "add_labels", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}}, + {Type: "add_labels", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}}, } summary := ComputeOutcomeSummary(reports, github.DefaultObjectiveMapping()) assert.InDelta(t, 0.0, summary.AcceptanceRate, 1e-12, @@ -752,7 +753,7 @@ func TestFormalDerivedMetricsConsistency(t *testing.T) { // (requires True) // (ensures fun r → // r.Type ≠ "" ∧ -// r.Result ∈ KnownOutcomeResults ∧ +// r.OutcomeStatus ∈ KnownOutcomeStatuses ∧ // normalizeOutcomeEvaluation(r).OutcomeStatus ≠ "" ∧ // normalizeOutcomeEvaluation(r).EvidenceStrength ≠ "") // @@ -765,9 +766,9 @@ func TestFormalOTelGracefulDegradation(t *testing.T) { return nil, errors.New("transport error: connection refused") } - validResults := map[OutcomeResult]bool{ - OutcomeAccepted: true, OutcomeRejected: true, OutcomeIgnored: true, - OutcomePending: true, OutcomeLifecycle: true, OutcomeLifecycleClose: true, OutcomeUnknown: true, OutcomeError: true, + validResults := map[OutcomeStatus]bool{ + OutcomeStatusAccepted: true, OutcomeStatusRejected: true, OutcomeStatusIgnored: true, + OutcomeStatusPending: true, OutcomeStatusLifecycle: true, OutcomeStatusLifecycleClose: true, OutcomeStatusUnknown: true, OutcomeStatusError: true, } items := []CreatedItemReport{ @@ -781,8 +782,8 @@ func TestFormalOTelGracefulDegradation(t *testing.T) { // P11: outcome must always be produced — never discarded on transport failure. assert.NotEmpty(t, report.Type, "P11: report.Type must be set regardless of transport availability") - assert.True(t, validResults[report.Result], - "P11: result %q must be a recognized OutcomeResult even when transport fails", report.Result) + assert.True(t, validResults[report.OutcomeStatus], + "P11: result %q must be a recognized OutcomeStatus even when transport fails", report.OutcomeStatus) // The outcome must always be normalizable (audit log entry is always writable). eval := normalizeOutcomeEvaluation(report) @@ -826,7 +827,7 @@ func TestFormalConformanceClassCoverage(t *testing.T) { return []map[string]any{{"event": "closed", "actor": map[string]any{"login": "github-actions[bot]"}}}, nil } report := evalCloseSticky(context.Background(), CreatedItemReport{Type: "close_issue", Number: 1, Repo: "o/r"}, "o/r") - assert.Equal(t, OutcomeLifecycleClose, report.Result, + assert.Equal(t, OutcomeStatusLifecycleClose, report.OutcomeStatus, "P12 Class A: lifecycle-bot-closed issue must be lifecycle_close") }) @@ -844,7 +845,7 @@ func TestFormalConformanceClassCoverage(t *testing.T) { return nil, nil } report := evalCloseSticky(context.Background(), CreatedItemReport{Type: "close_issue", Number: 1, Repo: "o/r"}, "o/r") - assert.Equal(t, OutcomeRejected, report.Result, + assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus, "P12 Class A: reopened issue must be rejected") }) @@ -860,15 +861,15 @@ func TestFormalConformanceClassCoverage(t *testing.T) { AfterState: map[string]any{"title": "New title", "body_hash": mutableBodyHash(""), "state": "open", "labels": []any{}, "assignees": []any{}}, } report := evalUpdateIssue(context.Background(), item, "o/r") - assert.Equal(t, OutcomeAccepted, report.Result, + assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus, "P12 Class A: retained update must be accepted") }) t.Run("Class B: lifecycle bot-close carries lifecycle signal", func(t *testing.T) { report := OutcomeReport{ - Type: "close_issue", - Result: OutcomeLifecycle, - Detail: "closed by bot (lifecycle)", + Type: "close_issue", + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycle}, + Detail: "closed by bot (lifecycle)", } eval := normalizeOutcomeEvaluation(report) assert.Equal(t, "lifecycle", eval.Signal, @@ -882,9 +883,9 @@ func TestFormalConformanceClassCoverage(t *testing.T) { return nil, errors.New("gh api: 500 Internal Server Error") } report := evalCloseSticky(context.Background(), CreatedItemReport{Type: "close_issue", Number: 1, Repo: "o/r"}, "o/r") - assert.NotEqual(t, OutcomeAccepted, report.Result, + assert.NotEqual(t, OutcomeStatusAccepted, report.OutcomeStatus, "P12 Class C: 5xx error must not yield accepted") - assert.NotEqual(t, OutcomeRejected, report.Result, + assert.NotEqual(t, OutcomeStatusRejected, report.OutcomeStatus, "P12 Class C: 5xx error must not yield rejected") }) @@ -900,9 +901,9 @@ func TestFormalConformanceClassCoverage(t *testing.T) { AfterState: map[string]any{"title": "New"}, } report := evalUpdateIssue(context.Background(), item, "o/r") - assert.NotEqual(t, OutcomeAccepted, report.Result, + assert.NotEqual(t, OutcomeStatusAccepted, report.OutcomeStatus, "P12 Class C: rate limit must not yield accepted") - assert.NotEqual(t, OutcomeRejected, report.Result, + assert.NotEqual(t, OutcomeStatusRejected, report.OutcomeStatus, "P12 Class C: rate limit must not yield rejected") }) } diff --git a/pkg/cli/outcome_eval_generic.go b/pkg/cli/outcome_eval_generic.go index e293a0d05dd..718e3692bee 100644 --- a/pkg/cli/outcome_eval_generic.go +++ b/pkg/cli/outcome_eval_generic.go @@ -24,7 +24,7 @@ func evalCloseSticky(ctx context.Context, item CreatedItemReport, repoOverride s Repo: repo, } if num == 0 || repo == "" { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing number or repo" return report } @@ -36,7 +36,7 @@ func evalCloseSticky(ctx context.Context, item CreatedItemReport, repoOverride s data, err := closeStickyGHAPIGet(ctx, endpoint, repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -44,30 +44,30 @@ func evalCloseSticky(ctx context.Context, item CreatedItemReport, repoOverride s state, _ := data["state"].(string) merged, _ := data["merged"].(bool) if state != "closed" { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "reopened" return report } if merged { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "merged" return report } closedByBot, err := isClosedByLifecycleBot(ctx, num, repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() report.Detail = "close provenance unavailable" return report } if closedByBot { - report.Result = OutcomeLifecycleClose + report.OutcomeStatus = OutcomeStatusLifecycleClose report.Detail = "closed by bot (lifecycle_close)" } else { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "closed by non-bot" } return report @@ -82,33 +82,33 @@ func isClosedByLifecycleBot(ctx context.Context, number int, repo string) (bool, func evalCloseDiscussion(ctx context.Context, item CreatedItemReport, repoOverride string) OutcomeReport { // Discussions require GraphQL; for now return pending with a note return OutcomeReport{ - Type: item.Type, - ObjectURL: item.URL, - Repo: resolveItemRepo(item, repoOverride), - Result: OutcomePending, - Detail: "discussion outcome check requires GraphQL (not yet implemented)", + Type: item.Type, + ObjectURL: item.URL, + Repo: resolveItemRepo(item, repoOverride), + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}, + Detail: "discussion outcome check requires GraphQL (not yet implemented)", } } // evalCreateDiscussion checks whether a discussion received replies. func evalCreateDiscussion(ctx context.Context, item CreatedItemReport, repoOverride string) OutcomeReport { return OutcomeReport{ - Type: item.Type, - ObjectURL: item.URL, - Repo: resolveItemRepo(item, repoOverride), - Result: OutcomePending, - Detail: "discussion outcome check requires GraphQL (not yet implemented)", + Type: item.Type, + ObjectURL: item.URL, + Repo: resolveItemRepo(item, repoOverride), + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}, + Detail: "discussion outcome check requires GraphQL (not yet implemented)", } } // evalHideComment checks whether a hidden comment is still hidden. func evalHideComment(ctx context.Context, item CreatedItemReport, repoOverride string) OutcomeReport { return OutcomeReport{ - Type: item.Type, - ObjectURL: item.URL, - Repo: resolveItemRepo(item, repoOverride), - Result: OutcomePending, - Detail: "hidden comment check requires GraphQL (not yet implemented)", + Type: item.Type, + ObjectURL: item.URL, + Repo: resolveItemRepo(item, repoOverride), + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}, + Detail: "hidden comment check requires GraphQL (not yet implemented)", } } @@ -124,23 +124,23 @@ func evalAssignMilestone(ctx context.Context, item CreatedItemReport, repoOverri Repo: repo, } if num == 0 || repo == "" { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing number or repo" return report } data, err := ghAPIGet(ctx, fmt.Sprintf("issues/%d", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } if data["milestone"] != nil { - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "milestone still assigned" } else { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "milestone removed" } return report @@ -149,22 +149,22 @@ func evalAssignMilestone(ctx context.Context, item CreatedItemReport, repoOverri // evalReviewComment checks whether a PR review comment thread was resolved or engaged. func evalReviewComment(ctx context.Context, item CreatedItemReport, repoOverride string) OutcomeReport { return OutcomeReport{ - Type: item.Type, - ObjectURL: item.URL, - Repo: resolveItemRepo(item, repoOverride), - Result: OutcomePending, - Detail: "review thread check requires GraphQL (not yet implemented)", + Type: item.Type, + ObjectURL: item.URL, + Repo: resolveItemRepo(item, repoOverride), + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}, + Detail: "review thread check requires GraphQL (not yet implemented)", } } // evalResolveThread checks whether a resolved review thread stayed resolved. func evalResolveThread(ctx context.Context, item CreatedItemReport, repoOverride string) OutcomeReport { return OutcomeReport{ - Type: item.Type, - ObjectURL: item.URL, - Repo: resolveItemRepo(item, repoOverride), - Result: OutcomePending, - Detail: "resolve thread check requires GraphQL (not yet implemented)", + Type: item.Type, + ObjectURL: item.URL, + Repo: resolveItemRepo(item, repoOverride), + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}, + Detail: "resolve thread check requires GraphQL (not yet implemented)", } } @@ -180,34 +180,34 @@ func evalMarkReady(ctx context.Context, item CreatedItemReport, repoOverride str Repo: repo, } if num == 0 || repo == "" { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing number or repo" return report } reviews, err := ghAPIGetArray(ctx, fmt.Sprintf("pulls/%d/reviews", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } if len(reviews) > 0 { - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = fmt.Sprintf("%d reviews submitted", len(reviews)) } else { data, derr := ghAPIGet(ctx, fmt.Sprintf("pulls/%d", num), repo) if derr == nil { state, _ := data["state"].(string) if state == "open" { - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "awaiting review" } else { - report.Result = OutcomeIgnored + report.OutcomeStatus = OutcomeStatusIgnored report.Detail = "closed/merged without review" } } else { - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "no reviews yet" } } @@ -226,14 +226,14 @@ func evalPushToPRBranch(ctx context.Context, item CreatedItemReport, repoOverrid Repo: repo, } if num == 0 || repo == "" { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing PR number or repo" return report } data, err := ghAPIGet(ctx, fmt.Sprintf("pulls/%d", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -244,13 +244,13 @@ func evalPushToPRBranch(ctx context.Context, item CreatedItemReport, repoOverrid switch { case merged: - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "PR merged" case state == "closed": - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "PR closed without merge" default: - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "PR still open" } return report @@ -270,19 +270,19 @@ func evalGenericSticky(ctx context.Context, item CreatedItemReport, repoOverride if num == 0 || repo == "" { // No number to check — just report what we know - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "no object reference to check" return report } _, err := genericOutcomeGHAPIGet(ctx, fmt.Sprintf("issues/%d", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "object still exists" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, diff --git a/pkg/cli/outcome_eval_issue.go b/pkg/cli/outcome_eval_issue.go index 646d6d0ad4b..ed55cf21b81 100644 --- a/pkg/cli/outcome_eval_issue.go +++ b/pkg/cli/outcome_eval_issue.go @@ -23,14 +23,14 @@ func evalCreateIssue(ctx context.Context, item CreatedItemReport, repoOverride s } if num == 0 || repo == "" { outcomeEvalIssueLog.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 } data, err := ghAPIGet(ctx, fmt.Sprintf("issues/%d", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -47,7 +47,7 @@ func evalCreateIssue(ctx context.Context, item CreatedItemReport, repoOverride s switch { case state == "closed" && stateReason == "completed": - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "completed" if closedAt != "" && item.Timestamp != "" { report.TimeToOutcomeHours = timeBetween(item.Timestamp, closedAt) @@ -58,10 +58,10 @@ func evalCreateIssue(ctx context.Context, item CreatedItemReport, repoOverride s closedByBot := isClosedByBot(ctx, num, repo) outcomeEvalIssueLog.Printf("Issue #%d closed as not_planned, closed_by_bot=%v", num, closedByBot) if closedByBot { - report.Result = OutcomeLifecycle + report.OutcomeStatus = OutcomeStatusLifecycle report.Detail = "closed by bot (lifecycle)" } else { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "closed as not planned" } if closedAt != "" && item.Timestamp != "" { @@ -69,22 +69,22 @@ func evalCreateIssue(ctx context.Context, item CreatedItemReport, repoOverride s } case state == "closed": - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "closed" if closedAt != "" && item.Timestamp != "" { report.TimeToOutcomeHours = timeBetween(item.Timestamp, closedAt) } case state == "open" && report.HumanComments > 0: - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = fmt.Sprintf("open, %d human comments", report.HumanComments) case state == "open" && int(comments) > 0: - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "open with comments" default: - report.Result = OutcomeIgnored + report.OutcomeStatus = OutcomeStatusIgnored report.Detail = "open, no engagement" } diff --git a/pkg/cli/outcome_eval_jsonl.go b/pkg/cli/outcome_eval_jsonl.go index 8d3c119006e..fbbe793a964 100644 --- a/pkg/cli/outcome_eval_jsonl.go +++ b/pkg/cli/outcome_eval_jsonl.go @@ -30,7 +30,6 @@ func writeOutcomeJSONL(dir string, runID int64, reports []OutcomeReport) { entry := map[string]any{ "run_id": runID, "type": r.Type, - "result": r.Result, "outcome_status": eval.OutcomeStatus, "evidence_strength": eval.EvidenceStrength, "signal": eval.Signal, diff --git a/pkg/cli/outcome_eval_label.go b/pkg/cli/outcome_eval_label.go index d084034fc76..90fa42debff 100644 --- a/pkg/cli/outcome_eval_label.go +++ b/pkg/cli/outcome_eval_label.go @@ -25,7 +25,7 @@ func evalReplaceLabel(ctx context.Context, item CreatedItemReport, repoOverride Repo: repo, } if num == 0 || repo == "" || item.BeforeState == nil || item.AfterState == nil { - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "missing execution state" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, @@ -43,7 +43,7 @@ func evalReplaceLabel(ctx context.Context, item CreatedItemReport, repoOverride removed := labelSetDiff(beforeLabels, afterLabels) if len(added) == 0 && len(removed) == 0 { - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "no label delta" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, @@ -55,7 +55,7 @@ func evalReplaceLabel(ctx context.Context, item CreatedItemReport, repoOverride currentState, _, err := extractCurrentIssueUpdateState(ctx, repo, num) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -67,7 +67,7 @@ func evalReplaceLabel(ctx context.Context, item CreatedItemReport, repoOverride removedStillAbsent := !labelSetContainsAny(currentLabels, removed) if addedRetained && removedStillAbsent { - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "label replacement retained" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusAccepted, @@ -81,7 +81,7 @@ func evalReplaceLabel(ctx context.Context, item CreatedItemReport, repoOverride addedReverted := !labelSetContainsAny(currentLabels, added) removedBack := labelSetContainsAll(currentLabels, removed) if addedReverted && removedBack { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "label replacement reverted" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusRejected, @@ -91,7 +91,7 @@ func evalReplaceLabel(ctx context.Context, item CreatedItemReport, repoOverride return report } - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "label replacement replaced" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusRejected, @@ -149,7 +149,7 @@ func evalAddLabels(ctx context.Context, item CreatedItemReport, repoOverride str } if num == 0 || repo == "" { outcomeEvalLabelLog.Print("Missing issue number or repo, returning error outcome") - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing issue number or repo" return report } @@ -157,7 +157,7 @@ func evalAddLabels(ctx context.Context, item CreatedItemReport, repoOverride str labels, err := ghAPIGetArray(ctx, fmt.Sprintf("issues/%d/labels", num), repo) if err != nil { outcomeEvalLabelLog.Printf("Failed to fetch labels for %s#%d: %v", repo, num, err) - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -167,13 +167,13 @@ func evalAddLabels(ctx context.Context, item CreatedItemReport, repoOverride str // pending rather than accepted, because the current labels could differ entirely // from the ones we added. Only an empty label list is a clear rejection signal. if len(labels) > 0 { - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "cannot evaluate label retention (added labels not recorded; extend manifest to include label names)" } else { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "all labels removed" } - outcomeEvalLabelLog.Printf("Label evaluation result: result=%s, label_count=%d", report.Result, len(labels)) + outcomeEvalLabelLog.Printf("Label evaluation result: result=%s, label_count=%d", report.OutcomeStatus, len(labels)) return report } diff --git a/pkg/cli/outcome_eval_pr.go b/pkg/cli/outcome_eval_pr.go index 990ff966703..6a72a7470a6 100644 --- a/pkg/cli/outcome_eval_pr.go +++ b/pkg/cli/outcome_eval_pr.go @@ -71,14 +71,14 @@ func evalCreatePullRequest(ctx context.Context, item CreatedItemReport, repoOver } if num == 0 || repo == "" { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing PR number or repo" return report } data, err := outcomeEvalPRGHAPIGet(ctx, fmt.Sprintf("pulls/%d", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -90,19 +90,19 @@ func evalCreatePullRequest(ctx context.Context, item CreatedItemReport, repoOver switch { case merged: - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "merged" if mergedAt != "" && item.Timestamp != "" { report.TimeToOutcomeHours = timeBetween(item.Timestamp, mergedAt) } case state == "closed": - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "closed without merge" if closedAt != "" && item.Timestamp != "" { report.TimeToOutcomeHours = timeBetween(item.Timestamp, closedAt) } default: - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "open" } @@ -117,7 +117,7 @@ func evalCreatePullRequest(ctx context.Context, item CreatedItemReport, repoOver report.HumanReviews = len(reviews) } - if report.Result == OutcomeAccepted { + if report.OutcomeStatus == OutcomeStatusAccepted { report.ZeroTouch = report.HumanComments == 0 && report.HumanReviews == 0 } diff --git a/pkg/cli/outcome_eval_review.go b/pkg/cli/outcome_eval_review.go index bd05546c5a3..ea72e2dba54 100644 --- a/pkg/cli/outcome_eval_review.go +++ b/pkg/cli/outcome_eval_review.go @@ -26,7 +26,7 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s } outcomeReviewLog.Printf("Evaluating add-reviewer outcome: repo=%s, pr=%d", repo, num) if num == 0 || repo == "" { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing PR number or repo" return report } @@ -36,14 +36,14 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s reviews, err := outcomeReviewGHAPIGetArray(ctx, fmt.Sprintf("pulls/%d/reviews", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } requested, err := outcomeReviewGHAPIGet(ctx, fmt.Sprintf("pulls/%d/requested_reviewers", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -88,7 +88,7 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s switch { case approvedReviewer != "": - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = fmt.Sprintf("requested reviewer %s approved", approvedReviewer) report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusAccepted, @@ -97,7 +97,7 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s } return report case submittedReviewer != "": - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = fmt.Sprintf("requested reviewer %s submitted a review", submittedReviewer) report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusAccepted, @@ -110,7 +110,7 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s // We cannot cheaply verify team membership for each reviewer from this endpoint, // so any submitted post-request review counts as medium-evidence team activity. if len(requestedTeams) > 0 && hasReviewAfterTimestamp(reviews, item.Timestamp) { - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "team review request received a review" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusAccepted, @@ -124,7 +124,7 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s currentTeams := extractTeamSlugs(requested["teams"]) stillPending := intersectsFold(requestedReviewers, currentUsers) || intersectsFold(requestedTeams, currentTeams) if stillPending { - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "review request still pending" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusPending, @@ -135,7 +135,7 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s } if len(requestedReviewers) > 0 || len(requestedTeams) > 0 { - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "review request removed without submitted review" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusRejected, @@ -145,7 +145,7 @@ func evalAddReviewer(ctx context.Context, item CreatedItemReport, repoOverride s return report } - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "no persisted reviewer request metadata" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, @@ -166,20 +166,20 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re } outcomeReviewLog.Printf("Evaluating submit-review outcome: repo=%s, pr=%d", repo, num) if num == 0 || repo == "" { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = "missing PR number or repo" return report } pr, err := outcomeReviewGHAPIGet(ctx, fmt.Sprintf("pulls/%d", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } reviews, err := outcomeReviewGHAPIGetArray(ctx, fmt.Sprintf("pulls/%d/reviews", num), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -191,7 +191,7 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re } if review == nil { outcomeReviewLog.Printf("Submitted review not found for PR #%d (reviewID=%d, reviews=%d)", num, reviewID, len(reviews)) - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "submitted review not found" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, @@ -208,7 +208,7 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re switch { case reviewState == "DISMISSED": //nolint:tolowerequalfold - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "review dismissed by repo admin" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusRejected, @@ -217,7 +217,7 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re } return report case prMerged && reviewState == "APPROVED": //nolint:tolowerequalfold - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "approved review followed by merge" report.TimeToOutcomeHours = timeBetween(item.Timestamp, outcomeString(pr["merged_at"])) report.OutcomeEvaluation = OutcomeEvaluation{ @@ -229,7 +229,7 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re case prMerged && reviewState == "CHANGES_REQUESTED": //nolint:tolowerequalfold commits, err := outcomeReviewGHAPIGetArray(ctx, fmt.Sprintf("pulls/%d/commits", num), repo) if err == nil && hasCommitAfterTimestamp(commits, reviewSubmittedAt) { - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "changes requested, updated, and merged" report.TimeToOutcomeHours = timeBetween(item.Timestamp, outcomeString(pr["merged_at"])) report.OutcomeEvaluation = OutcomeEvaluation{ @@ -240,7 +240,7 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re return report } case prState == "closed" && !prMerged: - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "PR closed without merge after review submission" report.TimeToOutcomeHours = timeBetween(item.Timestamp, outcomeString(pr["closed_at"])) report.OutcomeEvaluation = OutcomeEvaluation{ @@ -250,7 +250,7 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re } return report case prState == "open" && isLatestReview(reviews, review): - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "review is latest review on open PR" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusPending, @@ -260,7 +260,7 @@ func evalSubmitPullRequestReview(ctx context.Context, item CreatedItemReport, re return report } - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "review outcome could not be determined" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, diff --git a/pkg/cli/outcome_eval_test.go b/pkg/cli/outcome_eval_test.go index d3e70d7e0ff..d5850097cb5 100644 --- a/pkg/cli/outcome_eval_test.go +++ b/pkg/cli/outcome_eval_test.go @@ -21,12 +21,12 @@ import ( func TestComputeOutcomeSummary(t *testing.T) { reports := []OutcomeReport{ - {Type: "create_pull_request", Result: OutcomeAccepted, ZeroTouch: true, TimeToOutcomeHours: 2.0}, - {Type: "create_pull_request", Result: OutcomeAccepted, ZeroTouch: false, TimeToOutcomeHours: 8.0}, - {Type: "create_issue", Result: OutcomeRejected, TimeToOutcomeHours: 24.0}, - {Type: "add_comment", Result: OutcomeIgnored}, - {Type: "assign_to_agent", Result: OutcomePending}, - {Type: "close_issue", Result: OutcomeLifecycle}, + {Type: "create_pull_request", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted}, ZeroTouch: true, TimeToOutcomeHours: 2.0}, + {Type: "create_pull_request", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted}, ZeroTouch: false, TimeToOutcomeHours: 8.0}, + {Type: "create_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected}, TimeToOutcomeHours: 24.0}, + {Type: "add_comment", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusIgnored}}, + {Type: "assign_to_agent", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending}}, + {Type: "close_issue", OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycle}}, } s := ComputeOutcomeSummary(reports, github.DefaultObjectiveMapping()) @@ -337,7 +337,7 @@ func TestEvaluateOutcomesErrorOnMissingData(t *testing.T) { reports := EvaluateOutcomes(context.Background(), items, "", github.DefaultObjectiveMapping()) assert.Len(t, reports, 1, "should produce one report") - assert.Equal(t, OutcomeError, reports[0].Result, "should error on missing repo and number") + assert.Equal(t, OutcomeStatusError, reports[0].OutcomeStatus, "should error on missing repo and number") } func TestEnrichOutcomeWithObjectiveValue_TracesPullRequestToRootIssue(t *testing.T) { @@ -559,9 +559,9 @@ func TestEnrichOutcomeWithObjectiveValue_MultipleClosingIssuesRemainAmbiguous(t func TestNormalizeOutcomeEvaluationTargetExistsOnly(t *testing.T) { report := OutcomeReport{ - Type: "add_labels", - Result: OutcomeUnknown, - Detail: "object still exists", + Type: "add_labels", + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusUnknown}, + Detail: "object still exists", } eval := normalizeOutcomeEvaluation(report) @@ -584,7 +584,6 @@ func TestEvalGenericStickyTargetExistsOnlyFallback(t *testing.T) { "owner/repo", ) - assert.Equal(t, OutcomeUnknown, report.Result) assert.Equal(t, OutcomeStatusUnknown, report.OutcomeStatus) assert.Equal(t, EvidenceWeak, report.EvidenceStrength) assert.Equal(t, "target_exists_only", report.Signal) @@ -593,8 +592,7 @@ func TestEvalGenericStickyTargetExistsOnlyFallback(t *testing.T) { func TestOutcomeSummaryExcludesExistsOnlyFromAccepted(t *testing.T) { reports := []OutcomeReport{ { - Type: "add_labels", - Result: OutcomeUnknown, + Type: "add_labels", OutcomeEvaluation: OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, EvidenceStrength: EvidenceWeak, @@ -602,8 +600,7 @@ func TestOutcomeSummaryExcludesExistsOnlyFromAccepted(t *testing.T) { }, }, { - Type: "create_pull_request", - Result: OutcomeAccepted, + Type: "create_pull_request", OutcomeEvaluation: OutcomeEvaluation{ OutcomeStatus: OutcomeStatusAccepted, EvidenceStrength: EvidenceStrong, @@ -619,12 +616,45 @@ func TestOutcomeSummaryExcludesExistsOnlyFromAccepted(t *testing.T) { assert.Equal(t, 1, s.FallbackExistsOnlyCount) } +func TestOutcomeReportJSONCarriesSingleOutcomeStatus(t *testing.T) { + report := OutcomeReport{ + Type: "dispatch_workflow", + OutcomeEvaluation: OutcomeEvaluation{ + OutcomeStatus: OutcomeStatusError, + EvidenceStrength: EvidenceWeak, + Signal: "evaluation_error", + }, + } + + data, err := json.Marshal(report) + require.NoError(t, err) + + var payload map[string]any + require.NoError(t, json.Unmarshal(data, &payload)) + assert.Equal(t, "error", payload["outcome_status"]) + assert.NotContains(t, payload, "result") +} + +func TestOutcomeSummaryCountsEvalErrorsFromNormalizedStatus(t *testing.T) { + reports := []OutcomeReport{ + { + Type: "dispatch_workflow", + OutcomeEvaluation: OutcomeEvaluation{ + OutcomeStatus: OutcomeStatusUnknown, + }, + EvalError: "connection refused", + }, + } + + s := ComputeOutcomeSummary(reports, github.DefaultObjectiveMapping()) + assert.Equal(t, 1, s.Errors) +} + func TestWriteOutcomeJSONLEmitsNormalizedFields(t *testing.T) { dir := t.TempDir() reports := []OutcomeReport{ { - Type: "add_labels", - Result: OutcomeUnknown, + Type: "add_labels", OutcomeEvaluation: OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, EvidenceStrength: EvidenceWeak, @@ -645,6 +675,7 @@ func TestWriteOutcomeJSONLEmitsNormalizedFields(t *testing.T) { assert.Equal(t, "unknown", entry["outcome_status"]) assert.Equal(t, "weak", entry["evidence_strength"]) assert.Equal(t, "target_exists_only", entry["signal"]) + assert.NotContains(t, entry, "result") } func TestEvalAddReviewerAcceptedWithApproval(t *testing.T) { @@ -678,7 +709,6 @@ func TestEvalAddReviewerAcceptedWithApproval(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "review_approved", report.Signal) @@ -709,7 +739,6 @@ func TestEvalAddReviewerRejectedWhenRequestRemoved(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result) assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "review_request_removed", report.Signal) @@ -741,7 +770,6 @@ func TestEvalSubmitPullRequestReviewDismissed(t *testing.T) { Metadata: map[string]any{"review_id": float64(101)}, }, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result) assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "review_dismissed", report.Signal) @@ -786,7 +814,6 @@ func TestEvalSubmitPullRequestReviewChangesRequestedMergedAfterPush(t *testing.T Metadata: map[string]any{"review_id": float64(101)}, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "changes_requested_addressed", report.Signal) @@ -819,7 +846,6 @@ func TestEvalSubmitPullRequestReviewPendingWhenLatestOnOpenPR(t *testing.T) { Metadata: map[string]any{"review_id": float64(101)}, }, "owner/repo") - assert.Equal(t, OutcomePending, report.Result) assert.Equal(t, OutcomeStatusPending, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "latest_review_pending", report.Signal) @@ -853,7 +879,6 @@ func TestEvalAddReviewerPendingWhenRequestStillOutstanding(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomePending, report.Result) assert.Equal(t, OutcomeStatusPending, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "awaiting_review", report.Signal) @@ -887,7 +912,6 @@ func TestEvalAddReviewerUsesLatestReviewerState(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "review_submitted", report.Signal) @@ -942,7 +966,6 @@ func TestEvalSubmitPullRequestReviewChangesRequestedMissingCommitDatesStaysUnkno Metadata: map[string]any{"review_id": float64(101)}, }, "owner/repo") - assert.Equal(t, OutcomeUnknown, report.Result) assert.Equal(t, OutcomeStatusUnknown, report.OutcomeStatus) assert.Equal(t, EvidenceWeak, report.EvidenceStrength) assert.Equal(t, "unknown", report.Signal) @@ -978,7 +1001,6 @@ func TestEvalSubmitPullRequestReviewApprovedMergedUsesSharedSignal(t *testing.T) Metadata: map[string]any{"review_id": float64(101)}, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "review_approved", report.Signal) @@ -1011,7 +1033,6 @@ func TestEvalSubmitPullRequestReviewPendingIgnoresUnsubmittedDrafts(t *testing.T Metadata: map[string]any{"review_id": float64(101)}, }, "owner/repo") - assert.Equal(t, OutcomePending, report.Result) assert.Equal(t, OutcomeStatusPending, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "latest_review_pending", report.Signal) diff --git a/pkg/cli/outcome_eval_update.go b/pkg/cli/outcome_eval_update.go index 4ddfe0aafdc..53cb94ff30e 100644 --- a/pkg/cli/outcome_eval_update.go +++ b/pkg/cli/outcome_eval_update.go @@ -37,7 +37,7 @@ func evalRetainedUpdate(ctx context.Context, item CreatedItemReport, repoOverrid } if num == 0 || repo == "" { outcomeEvalUpdateLog.Printf("Missing execution state: num=%d, repo=%s", num, repo) - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "missing execution state" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, @@ -47,7 +47,7 @@ func evalRetainedUpdate(ctx context.Context, item CreatedItemReport, repoOverrid return report } if item.BeforeState == nil || item.AfterState == nil { - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "missing execution state" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, @@ -59,7 +59,7 @@ func evalRetainedUpdate(ctx context.Context, item CreatedItemReport, repoOverrid currentState, merged, err := load(ctx, repo, num) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -68,7 +68,7 @@ func evalRetainedUpdate(ctx context.Context, item CreatedItemReport, repoOverrid outcomeEvalUpdateLog.Printf("State comparison for %s #%d: changed=%d, retained=%d, reverted=%d, replaced=%d, merged=%v", objectKind, num, len(comparison.Changed), len(comparison.Retained), len(comparison.Reverted), len(comparison.Replaced), merged) if len(comparison.Changed) == 0 { - report.Result = OutcomeUnknown + report.OutcomeStatus = OutcomeStatusUnknown report.Detail = "no persisted state delta" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusUnknown, @@ -80,7 +80,7 @@ func evalRetainedUpdate(ctx context.Context, item CreatedItemReport, repoOverrid switch { case len(comparison.Retained) == len(comparison.Changed): - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted if strongOnMerge && merged { report.Detail = objectKind + " update retained and merged" report.OutcomeEvaluation = OutcomeEvaluation{ @@ -98,7 +98,7 @@ func evalRetainedUpdate(ctx context.Context, item CreatedItemReport, repoOverrid } return report case len(comparison.Reverted) == len(comparison.Changed): - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = objectKind + " update reverted" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusRejected, @@ -107,7 +107,7 @@ func evalRetainedUpdate(ctx context.Context, item CreatedItemReport, repoOverrid } return report default: - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = objectKind + " update replaced" report.OutcomeEvaluation = OutcomeEvaluation{ OutcomeStatus: OutcomeStatusRejected, diff --git a/pkg/cli/outcome_eval_update_test.go b/pkg/cli/outcome_eval_update_test.go index 01be99a4ae1..f33e4b5cc54 100644 --- a/pkg/cli/outcome_eval_update_test.go +++ b/pkg/cli/outcome_eval_update_test.go @@ -49,7 +49,6 @@ func TestEvalUpdateIssueRetained(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "state_retained", report.Signal) @@ -84,7 +83,6 @@ func TestEvalUpdateIssueReverted(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result) assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "state_reverted", report.Signal) @@ -129,7 +127,6 @@ func TestEvalUpdatePullRequestRetainedAndMerged(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "state_retained_and_merged", report.Signal) @@ -174,7 +171,6 @@ func TestEvalUpdatePullRequestReplaced(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result) assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "state_replaced", report.Signal) @@ -187,7 +183,6 @@ func TestEvalRetainedUpdateMissingExecutionStateUsesEvidenceNone(t *testing.T) { Repo: "owner/repo", }, "owner/repo") - assert.Equal(t, OutcomeUnknown, report.Result) assert.Equal(t, OutcomeStatusUnknown, report.OutcomeStatus) assert.Equal(t, EvidenceNone, report.EvidenceStrength) assert.Equal(t, "missing_execution_state", report.Signal) @@ -219,7 +214,6 @@ func TestEvalReplaceLabelRetained(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "state_retained", report.Signal) @@ -255,7 +249,6 @@ func TestEvalReplaceLabelRetainedWithExtraLabel(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Equal(t, EvidenceMedium, report.EvidenceStrength) assert.Equal(t, "state_retained", report.Signal) @@ -289,7 +282,6 @@ func TestEvalReplaceLabelReverted(t *testing.T) { }, }, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result) assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus) assert.Equal(t, EvidenceStrong, report.EvidenceStrength) assert.Equal(t, "state_reverted", report.Signal) diff --git a/pkg/cli/outcome_eval_workflow.go b/pkg/cli/outcome_eval_workflow.go index dbb8d9d682a..7522b9f9952 100644 --- a/pkg/cli/outcome_eval_workflow.go +++ b/pkg/cli/outcome_eval_workflow.go @@ -53,14 +53,14 @@ func evalDispatchWorkflow(ctx context.Context, item CreatedItemReport, repoOverr if runID <= 0 { // No run ID available — workflow may not have been dispatched or ID not captured - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "no run ID available; dispatch may still be queued" return report } data, err := workflowOutcomeGHAPIGet(ctx, fmt.Sprintf("actions/runs/%d", runID), repo) if err != nil { - report.Result = OutcomeError + report.OutcomeStatus = OutcomeStatusError report.EvalError = err.Error() return report } @@ -71,19 +71,19 @@ func evalDispatchWorkflow(ctx context.Context, item CreatedItemReport, repoOverr switch { case status == "completed" && conclusion == "success": - report.Result = OutcomeAccepted + report.OutcomeStatus = OutcomeStatusAccepted report.Detail = "workflow run completed with success" case status == "completed" && (conclusion == "failure" || conclusion == "timed_out" || conclusion == "cancelled" || conclusion == "action_required"): // action_required means the run is blocked and requires manual intervention; // treat it as rejected rather than ignored since it does not self-resolve. - report.Result = OutcomeRejected + report.OutcomeStatus = OutcomeStatusRejected report.Detail = "workflow run completed with " + conclusion case status == "completed": // neutral and skipped indicate the run did not contribute meaningful output. - report.Result = OutcomeIgnored + report.OutcomeStatus = OutcomeStatusIgnored report.Detail = "workflow run completed with " + conclusion default: - report.Result = OutcomePending + report.OutcomeStatus = OutcomeStatusPending report.Detail = "workflow run status: " + status } return report @@ -91,15 +91,15 @@ func evalDispatchWorkflow(ctx context.Context, item CreatedItemReport, repoOverr // evalUpdateDiscussion checks whether a discussion edit stuck. // Full evaluation requires GraphQL (same pattern as evalCloseDiscussion). -// Until the GraphQL evaluator is implemented this returns OutcomeIgnored so that +// Until the GraphQL evaluator is implemented this returns OutcomeStatusIgnored so that // callers do not retry indefinitely. // Spec: specs/safe-output-outcome-evaluation.md §12 func evalUpdateDiscussion(ctx context.Context, item CreatedItemReport, repoOverride string) OutcomeReport { return OutcomeReport{ - Type: item.Type, - ObjectURL: item.URL, - Repo: resolveItemRepo(item, repoOverride), - Result: OutcomeIgnored, - Detail: "discussion update check requires GraphQL (not yet implemented); outcome is advisory only", + Type: item.Type, + ObjectURL: item.URL, + Repo: resolveItemRepo(item, repoOverride), + OutcomeEvaluation: OutcomeEvaluation{OutcomeStatus: OutcomeStatusIgnored}, + Detail: "discussion update check requires GraphQL (not yet implemented); outcome is advisory only", } } diff --git a/pkg/cli/outcome_eval_workflow_test.go b/pkg/cli/outcome_eval_workflow_test.go index fecf02211dc..fe3b962fb04 100644 --- a/pkg/cli/outcome_eval_workflow_test.go +++ b/pkg/cli/outcome_eval_workflow_test.go @@ -17,7 +17,7 @@ func TestEvalDispatchWorkflowNoRunID(t *testing.T) { Repo: "owner/repo", }, "owner/repo") - assert.Equal(t, OutcomePending, report.Result) + assert.Equal(t, OutcomeStatusPending, report.OutcomeStatus) assert.Contains(t, report.Detail, "no run ID available") } @@ -29,7 +29,7 @@ func TestEvalDispatchWorkflowRunIDZero(t *testing.T) { Metadata: map[string]any{"run_id": float64(0)}, }, "owner/repo") - assert.Equal(t, OutcomePending, report.Result) + assert.Equal(t, OutcomeStatusPending, report.OutcomeStatus) } func TestEvalDispatchWorkflowAPIError(t *testing.T) { @@ -45,7 +45,7 @@ func TestEvalDispatchWorkflowAPIError(t *testing.T) { Metadata: map[string]any{"run_id": float64(12345678)}, }, "owner/repo") - assert.Equal(t, OutcomeError, report.Result) + assert.Equal(t, OutcomeStatusError, report.OutcomeStatus) assert.Contains(t, report.EvalError, "connection refused") } @@ -65,7 +65,7 @@ func TestEvalDispatchWorkflowCompletedSuccess(t *testing.T) { Metadata: map[string]any{"run_id": float64(12345678)}, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) + assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) assert.Contains(t, report.Detail, "success") } @@ -87,7 +87,7 @@ func TestEvalDispatchWorkflowCompletedFailure(t *testing.T) { Metadata: map[string]any{"run_id": float64(99999)}, }, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result) + assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus) assert.Contains(t, report.Detail, conclusion) }) } @@ -109,7 +109,7 @@ func TestEvalDispatchWorkflowCompletedOtherConclusion(t *testing.T) { Metadata: map[string]any{"run_id": float64(42)}, }, "owner/repo") - assert.Equal(t, OutcomeIgnored, report.Result) + assert.Equal(t, OutcomeStatusIgnored, report.OutcomeStatus) assert.Contains(t, report.Detail, "skipped") } @@ -129,7 +129,7 @@ func TestEvalDispatchWorkflowInProgress(t *testing.T) { Metadata: map[string]any{"run_id": float64(77)}, }, "owner/repo") - assert.Equal(t, OutcomePending, report.Result) + assert.Equal(t, OutcomeStatusPending, report.OutcomeStatus) assert.Contains(t, report.Detail, "in_progress") } @@ -150,12 +150,12 @@ func TestEvalDispatchWorkflowRunIDInt64(t *testing.T) { Metadata: map[string]any{"run_id": int64(9876543210)}, }, "owner/repo") - assert.Equal(t, OutcomeAccepted, report.Result) + assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) } func TestEvalDispatchWorkflowFloat64OverflowGuard(t *testing.T) { // A float64 value above 2^53 cannot represent integers exactly and must be - // treated as an invalid run_id (OutcomePending) rather than silently truncated. + // treated as an invalid run_id (OutcomeStatusPending) rather than silently truncated. // Use 2^53 + 2 (= 9007199254740994): consecutive integers around 2^53 collapse // to the same float64, so this value would be mangled if cast to int64 directly. aboveMaxSafeInt := float64(maxSafeFloat64Int) + 2 @@ -166,13 +166,13 @@ func TestEvalDispatchWorkflowFloat64OverflowGuard(t *testing.T) { Metadata: map[string]any{"run_id": aboveMaxSafeInt}, }, "owner/repo") - assert.Equal(t, OutcomePending, report.Result, - "a float64 run_id above 2^53 must be treated as invalid (OutcomePending)") + assert.Equal(t, OutcomeStatusPending, report.OutcomeStatus, + "a float64 run_id above 2^53 must be treated as invalid (OutcomeStatusPending)") } func TestEvalDispatchWorkflowActionRequired(t *testing.T) { // action_required is a blocking conclusion that requires manual intervention; - // it must map to OutcomeRejected (not OutcomeIgnored). + // it must map to OutcomeStatusRejected (not OutcomeStatusIgnored). old := workflowOutcomeGHAPIGet t.Cleanup(func() { workflowOutcomeGHAPIGet = old }) workflowOutcomeGHAPIGet = func(_ context.Context, _ string, _ string) (map[string]any, error) { @@ -188,20 +188,20 @@ func TestEvalDispatchWorkflowActionRequired(t *testing.T) { Metadata: map[string]any{"run_id": float64(12345678)}, }, "owner/repo") - assert.Equal(t, OutcomeRejected, report.Result, - "action_required conclusion must map to OutcomeRejected") + assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus, + "action_required conclusion must map to OutcomeStatusRejected") assert.Contains(t, report.Detail, "action_required") } func TestEvalUpdateDiscussionReturnsIgnored(t *testing.T) { - // evalUpdateDiscussion must return OutcomeIgnored (not OutcomePending) so that + // evalUpdateDiscussion must return OutcomeStatusIgnored (not OutcomeStatusPending) so that // callers do not enter an infinite retry loop waiting for a terminal status. report := evalUpdateDiscussion(context.Background(), CreatedItemReport{ Type: "update_discussion", URL: "https://github.com/owner/repo/discussions/1", }, "owner/repo") - assert.Equal(t, OutcomeIgnored, report.Result, - "evalUpdateDiscussion must return OutcomeIgnored to prevent infinite retry") + assert.Equal(t, OutcomeStatusIgnored, report.OutcomeStatus, + "evalUpdateDiscussion must return OutcomeStatusIgnored to prevent infinite retry") assert.NotEmpty(t, report.Detail) } diff --git a/pkg/cli/outcome_evaluation.go b/pkg/cli/outcome_evaluation.go index 4d64de5e61b..5ad2098f5b4 100644 --- a/pkg/cli/outcome_evaluation.go +++ b/pkg/cli/outcome_evaluation.go @@ -20,6 +20,7 @@ const ( OutcomeStatusUnknown OutcomeStatus = "unknown" OutcomeStatusLifecycle OutcomeStatus = "lifecycle" OutcomeStatusLifecycleClose OutcomeStatus = "lifecycle_close" + OutcomeStatusError OutcomeStatus = "error" ) // EvidenceStrength describes how confidently the outcome can be inferred. @@ -34,7 +35,7 @@ const ( // OutcomeEvaluation is the shared normalized outcome model. type OutcomeEvaluation struct { - OutcomeStatus OutcomeStatus `json:"outcome_status"` + OutcomeStatus OutcomeStatus `json:"outcome_status" console:"header:Outcome"` EvidenceStrength EvidenceStrength `json:"evidence_strength"` Signal string `json:"signal,omitempty"` } @@ -44,11 +45,11 @@ func normalizeOutcomeEvaluation(report OutcomeReport) OutcomeEvaluation { return report.OutcomeEvaluation } - outcomeEvaluationLog.Printf("Normalizing outcome from heuristics: type=%s, result=%s, detail=%q", report.Type, report.Result, report.Detail) + outcomeEvaluationLog.Printf("Normalizing outcome from heuristics: type=%s, result=%s, detail=%q", report.Type, report.OutcomeStatus, report.Detail) - if report.EvalError != "" || report.Result == OutcomeError { + if report.EvalError != "" || report.OutcomeStatus == OutcomeStatusError { return OutcomeEvaluation{ - OutcomeStatus: OutcomeStatusUnknown, + OutcomeStatus: OutcomeStatusError, EvidenceStrength: EvidenceWeak, Signal: "evaluation_error", } @@ -90,27 +91,31 @@ func normalizeOutcomeEvaluation(report OutcomeReport) OutcomeEvaluation { case strings.Contains(detail, "open"): return OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending, EvidenceStrength: EvidenceMedium, Signal: "open"} case strings.Contains(detail, "closed"): - if report.Result == OutcomeRejected { + if report.OutcomeStatus == OutcomeStatusRejected { return OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected, EvidenceStrength: EvidenceStrong, Signal: "closed"} } return OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted, EvidenceStrength: EvidenceStrong, Signal: "closed"} } - switch report.Result { - case OutcomeAccepted: + switch report.OutcomeStatus { + case OutcomeStatusAccepted: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusAccepted, EvidenceStrength: EvidenceMedium, Signal: "acted_on"} - case OutcomeRejected: + case OutcomeStatusRejected: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusRejected, EvidenceStrength: EvidenceMedium, Signal: "rejected"} - case OutcomePending: + case OutcomeStatusPending: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusPending, EvidenceStrength: EvidenceMedium, Signal: "pending"} - case OutcomeIgnored: + case OutcomeStatusIgnored: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusIgnored, EvidenceStrength: EvidenceMedium, Signal: "ignored"} - case OutcomeLifecycle: + case OutcomeStatusLifecycle: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycle, EvidenceStrength: EvidenceMedium, Signal: "lifecycle"} - case OutcomeLifecycleClose: + case OutcomeStatusLifecycleClose: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusLifecycleClose, EvidenceStrength: EvidenceMedium, Signal: "lifecycle_close"} - case OutcomeUnknown: + case OutcomeStatusUnknown: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusUnknown, EvidenceStrength: EvidenceWeak, Signal: "unknown"} + case OutcomeStatusSkipped: + return OutcomeEvaluation{OutcomeStatus: OutcomeStatusSkipped, EvidenceStrength: EvidenceNone, Signal: "skipped"} + case OutcomeStatusError: + return OutcomeEvaluation{OutcomeStatus: OutcomeStatusError, EvidenceStrength: EvidenceWeak, Signal: "evaluation_error"} default: return OutcomeEvaluation{OutcomeStatus: OutcomeStatusUnknown, EvidenceStrength: EvidenceWeak, Signal: "unknown"} } diff --git a/pkg/cli/outcomes_command.go b/pkg/cli/outcomes_command.go index f760e1a6b7e..56bb9363643 100644 --- a/pkg/cli/outcomes_command.go +++ b/pkg/cli/outcomes_command.go @@ -211,7 +211,7 @@ func RunOutcomes(ctx context.Context, config OutcomesConfig) error { // Render the items fmt.Fprintln(os.Stderr) for _, r := range reports { - resultStr := string(r.Result) + resultStr := string(r.OutcomeStatus) detail := r.Detail if detail != "" { resultStr += " (" + detail + ")"