fix(llm): run default AI roles on completed PipelineRuns

Author: BoseKarthikeyan

docs/content/docs/guides/llm-analysis/_index.md

@@ -93,7 +93,7 @@ Each role defines a specific analysis scenario. You can configure multiple roles
 | `name` | string | Yes | Unique identifier for this role |
 | `prompt` | string | Yes | Prompt template for the LLM |
 | `model` | string | No | Model name (consult provider documentation for available models). Uses provider default if not specified. |
-| `on_cel` | string | No | CEL expression for conditional triggering. If not specified, the role runs only for failed PipelineRuns. |
+| `on_cel` | string | No | CEL expression for conditional triggering. If not specified, the role runs for completed PipelineRuns (both successful and failed). |
 | `output` | string | Yes | Output destination (currently only `pr-comment` is supported) |
 | `context_items` | object | No | Configuration for context inclusion |
 

docs/content/docs/guides/llm-analysis/model-and-triggers.md

@@ -50,19 +50,19 @@ settings:
 
 ## CEL Expressions for Triggers
 
-By default, Pipelines-as-Code runs LLM analysis only for failed PipelineRuns. CEL (Common Expression Language) expressions in the `on_cel` field let you refine this behavior -- for example, restricting analysis to a specific branch or enabling it for successful runs too.
+By default, Pipelines-as-Code runs LLM analysis for completed PipelineRuns (both successful and failed). CEL (Common Expression Language) expressions in the `on_cel` field let you refine this behavior -- for example, restricting analysis to a specific branch, only successful runs, or only failed runs.
 
-If you omit `on_cel`, the role executes for all failed PipelineRuns.
+If you omit `on_cel`, the role executes for all completed PipelineRuns.
 
 ### Overriding the Default Behavior
 
-To run analysis for **all PipelineRuns** (both successful and failed), set `on_cel: 'true'`:
+To run analysis for **all completed PipelineRuns** (both successful and failed), set `on_cel: 'true'`:
 
 ```yaml
 roles:
   - name: "pipeline-summary"
     prompt: "Generate a summary of this pipeline run..."
-    on_cel: 'true'  # Runs for ALL pipeline runs, not just failures
+    on_cel: 'true'  # Runs for all completed pipeline runs
     output: "pr-comment"
 ```
 
@@ -76,13 +76,13 @@ This is useful when you want to:
 ### Example CEL Expressions
 
 ```yaml
-# Run on ALL pipeline runs (overrides default failed-only behavior)
+# Run on ALL pipeline runs (overrides default completed-only behavior)
 on_cel: 'true'
 
 # Only on successful runs (e.g., for generating success reports)
 on_cel: 'body.pipelineRun.status.conditions[0].reason == "Succeeded"'
 
-# Only on pull requests (in addition to default failed-only check)
+# Only on pull requests
 on_cel: 'body.event.event_type == "pull_request"'
 
 # Only on main branch

pkg/llm/analyzer.go

@@ -344,11 +344,10 @@ func countFailedResults(results []AnalysisResult) int {
 }
 
 // shouldTriggerRole evaluates the CEL expression to determine if a role should be triggered.
-// If no on_cel is provided, defaults to triggering only for failed PipelineRuns.
+// If no on_cel is provided, the role runs only after the PipelineRun completes.
 func shouldTriggerRole(role v1alpha1.AnalysisRole, celContext map[string]any, pr *tektonv1.PipelineRun) (bool, error) {
 	if role.OnCEL == "" {
-		succeededCondition := pr.Status.GetCondition(apis.ConditionSucceeded)
-		return succeededCondition != nil && succeededCondition.Status == corev1.ConditionFalse, nil
+		return isCompletedPipelineRun(pr), nil
 	}
 
 	result, err := cel.Value(role.OnCEL, celContext["body"],
@@ -366,6 +365,16 @@ func shouldTriggerRole(role v1alpha1.AnalysisRole, celContext map[string]any, pr
 	return false, fmt.Errorf("CEL expression '%s' did not return boolean value", role.OnCEL)
 }
 
+// isCompletedPipelineRun checks whether a PipelineRun has finished,
+// regardless of success or failure.
+func isCompletedPipelineRun(pr *tektonv1.PipelineRun) bool {
+	if pr == nil {
+		return false
+	}
+	c := pr.Status.GetCondition(apis.ConditionSucceeded)
+	return c != nil && (c.Status == corev1.ConditionTrue || c.Status == corev1.ConditionFalse)
+}
+
 // validateAnalysisConfig validates the AI analysis configuration.
 func validateAnalysisConfig(config *v1alpha1.AIAnalysisConfig) error {
 	if config.Provider == "" {

pkg/llm/analyzer_test.go

@@ -380,7 +380,7 @@ func TestShouldTriggerRoleEvaluations(t *testing.T) {
 		wantError bool
 	}{
 		{
-			name: "no expression defaults to failed pipelines only",
+			name: "no expression defaults to completed pipelines",
 			role: v1alpha1.AnalysisRole{},
 			want: true,
 		},
@@ -423,6 +423,9 @@ func TestShouldTriggerRole(t *testing.T) {
 	succeededPR := &tektonv1.PipelineRun{}
 	succeededPR.Status.Conditions = append(succeededPR.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded, Status: "True"})
 
+	pendingPR := &tektonv1.PipelineRun{}
+	pendingPR.Status.Conditions = append(pendingPR.Status.Conditions, apis.Condition{Type: apis.ConditionSucceeded, Status: "Unknown"})
+
 	tests := []struct {
 		name        string
 		role        v1alpha1.AnalysisRole
@@ -439,10 +442,31 @@ func TestShouldTriggerRole(t *testing.T) {
 			wantTrigger: true,
 		},
 		{
-			name:        "no cel expression - skips succeeded pipeline",
+			name:        "no cel expression - triggers for succeeded pipeline",
 			role:        v1alpha1.AnalysisRole{Name: "test-role"},
 			celContext:  map[string]any{},
 			pr:          succeededPR,
+			wantTrigger: true,
+		},
+		{
+			name:        "no cel expression - skips pending pipeline",
+			role:        v1alpha1.AnalysisRole{Name: "test-role"},
+			celContext:  map[string]any{},
+			pr:          pendingPR,
+			wantTrigger: false,
+		},
+		{
+			name:        "no cel expression - skips nil pipelinerun",
+			role:        v1alpha1.AnalysisRole{Name: "test-role"},
+			celContext:  map[string]any{},
+			pr:          nil,
+			wantTrigger: false,
+		},
+		{
+			name:        "no cel expression - skips pipelinerun without status",
+			role:        v1alpha1.AnalysisRole{Name: "test-role"},
+			celContext:  nil,
+			pr:          &tektonv1.PipelineRun{},
 			wantTrigger: false,
 		},
 		{