Skip to content

Commit 4e1e368

Browse files
Kamesh Akellacursoragent
authored and
kami619
committed
fix(langgraph): address remaining summarizer CodeRabbit findings
Fix the still-valid CodeRabbit items in the ci_failure_summarizer template, including example correctness issues, generic error responses, CronJob security context, Slack post idempotency, config normalization, and the remaining maintainability cleanups. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent cb114dd commit 4e1e368

22 files changed

Lines changed: 324 additions & 244 deletions

‎agents/langgraph/templates/ci_failure_summarizer/Makefile‎

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,21 @@ CHART_DIR := ../../deployment
44
VALUES_FILE := values.yaml
55
CONTAINER_CLI := $(shell command -v podman 2>/dev/null || command -v docker 2>/dev/null)
66
MODEL ?= llama3.1:8b
7+
HELM_ENV_SET = --set image.repository="$${IMAGE_REPO}" \
8+
--set image.tag="$${IMAGE_TAG}" \
9+
--set env.BASE_URL="$${BASE_URL}" \
10+
--set env.MODEL_ID="$${MODEL_ID}" \
11+
--set env.POSTGRES_HOST="$${POSTGRES_HOST}" \
12+
--set env.POSTGRES_PORT="$${POSTGRES_PORT}" \
13+
--set env.POSTGRES_DB="$${POSTGRES_DB}" \
14+
--set env.POSTGRES_USER="$${POSTGRES_USER}" \
15+
--set env.GITHUB_REPOSITORY="$${GITHUB_REPOSITORY}" \
16+
$${GITHUB_WORKFLOW:+--set env.GITHUB_WORKFLOW="$${GITHUB_WORKFLOW}"} \
17+
$${GITHUB_WORKFLOW_FILE:+--set env.GITHUB_WORKFLOW_FILE="$${GITHUB_WORKFLOW_FILE}"} \
18+
$${MLFLOW_TRACKING_URI:+--set env.MLFLOW_TRACKING_URI="$${MLFLOW_TRACKING_URI}"} \
19+
$${MLFLOW_EXPERIMENT_NAME:+--set env.MLFLOW_EXPERIMENT_NAME="$${MLFLOW_EXPERIMENT_NAME}"} \
20+
$${MLFLOW_TRACKING_INSECURE_TLS:+--set env.MLFLOW_TRACKING_INSECURE_TLS="$${MLFLOW_TRACKING_INSECURE_TLS}"} \
21+
$${MLFLOW_WORKSPACE:+--set env.MLFLOW_WORKSPACE="$${MLFLOW_WORKSPACE}"}
722

823
.PHONY: init re-init env ollama ogx-server run-app run-app-fresh run-cli build push build-openshift deploy undeploy test test-integration dry-run help
924

@@ -132,21 +147,7 @@ deploy: _check-env ## Deploy to OpenShift/K8s via Helm
132147
helm upgrade --install $(AGENT_NAME) $(CHART_DIR) \
133148
-f $(VALUES_FILE) \
134149
-f .helm-secrets.yaml \
135-
--set image.repository="$${IMAGE_REPO}" \
136-
--set image.tag="$${IMAGE_TAG}" \
137-
--set env.BASE_URL="$${BASE_URL}" \
138-
--set env.MODEL_ID="$${MODEL_ID}" \
139-
--set env.POSTGRES_HOST="$${POSTGRES_HOST}" \
140-
--set env.POSTGRES_PORT="$${POSTGRES_PORT}" \
141-
--set env.POSTGRES_DB="$${POSTGRES_DB}" \
142-
--set env.POSTGRES_USER="$${POSTGRES_USER}" \
143-
--set env.GITHUB_REPOSITORY="$${GITHUB_REPOSITORY}" \
144-
$${GITHUB_WORKFLOW:+--set env.GITHUB_WORKFLOW="$${GITHUB_WORKFLOW}"} \
145-
$${GITHUB_WORKFLOW_FILE:+--set env.GITHUB_WORKFLOW_FILE="$${GITHUB_WORKFLOW_FILE}"} \
146-
$${MLFLOW_TRACKING_URI:+--set env.MLFLOW_TRACKING_URI="$${MLFLOW_TRACKING_URI}"} \
147-
$${MLFLOW_EXPERIMENT_NAME:+--set env.MLFLOW_EXPERIMENT_NAME="$${MLFLOW_EXPERIMENT_NAME}"} \
148-
$${MLFLOW_TRACKING_INSECURE_TLS:+--set env.MLFLOW_TRACKING_INSECURE_TLS="$${MLFLOW_TRACKING_INSECURE_TLS}"} \
149-
$${MLFLOW_WORKSPACE:+--set env.MLFLOW_WORKSPACE="$${MLFLOW_WORKSPACE}"} && \
150+
$(HELM_ENV_SET) && \
150151
if command -v oc >/dev/null 2>&1; then \
151152
echo "" && echo "Waiting for rollout to complete..." && \
152153
if oc rollout status deployment/$(AGENT_NAME) --timeout=120s; then \
@@ -167,24 +168,10 @@ dry-run: _check-env ## Render Helm templates without deploying
167168
-f $(VALUES_FILE) \
168169
--set secrets.apiKey="REDACTED" \
169170
--set secrets.postgresPassword="REDACTED" \
170-
--set image.repository="$${IMAGE_REPO}" \
171-
--set image.tag="$${IMAGE_TAG}" \
172-
--set env.BASE_URL="$${BASE_URL}" \
173-
--set env.MODEL_ID="$${MODEL_ID}" \
174-
--set env.POSTGRES_HOST="$${POSTGRES_HOST}" \
175-
--set env.POSTGRES_PORT="$${POSTGRES_PORT}" \
176-
--set env.POSTGRES_DB="$${POSTGRES_DB}" \
177-
--set env.POSTGRES_USER="$${POSTGRES_USER}" \
178-
--set env.GITHUB_REPOSITORY="$${GITHUB_REPOSITORY}" \
179-
$${GITHUB_WORKFLOW:+--set env.GITHUB_WORKFLOW="$${GITHUB_WORKFLOW}"} \
180-
$${GITHUB_WORKFLOW_FILE:+--set env.GITHUB_WORKFLOW_FILE="$${GITHUB_WORKFLOW_FILE}"} \
181-
$${MLFLOW_TRACKING_URI:+--set env.MLFLOW_TRACKING_URI="$${MLFLOW_TRACKING_URI}"} \
171+
$(HELM_ENV_SET) \
182172
$${MLFLOW_TRACKING_TOKEN:+--set secrets.mlflowTrackingToken="REDACTED"} \
183173
$${GITHUB_TOKEN:+--set extraSecrets.GITHUB_TOKEN="REDACTED"} \
184174
$${SLACK_WEBHOOK_URL:+--set extraSecrets.SLACK_WEBHOOK_URL="REDACTED"} \
185-
$${MLFLOW_EXPERIMENT_NAME:+--set env.MLFLOW_EXPERIMENT_NAME="$${MLFLOW_EXPERIMENT_NAME}"} \
186-
$${MLFLOW_TRACKING_INSECURE_TLS:+--set env.MLFLOW_TRACKING_INSECURE_TLS="$${MLFLOW_TRACKING_INSECURE_TLS}"} \
187-
$${MLFLOW_WORKSPACE:+--set env.MLFLOW_WORKSPACE="$${MLFLOW_WORKSPACE}"}
188175

189176
undeploy: ## Remove deployment from cluster
190177
helm uninstall $(AGENT_NAME)

‎agents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.py‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@ def __init__(
1212
verbose: bool = True,
1313
) -> None:
1414
self.ai_service_invoke = ai_service_invoke
15-
self._ordered_list = lambda seq_: "\n".join(
16-
f"\t{i}) {k}" for i, k in enumerate(seq_, 1)
17-
)
1815
self._delta_start = False
1916
self.verbose = verbose
2017
self.stream = stream
@@ -24,14 +21,9 @@ def __init__(
2421
The following commands are supported:
2522
--> help | h : prints this help message
2623
--> quit | q : exits the prompt and ends the program
27-
--> list_questions : prints a list of available questions
2824
"""
2925
)
3026

31-
@property
32-
def questions(self) -> tuple:
33-
return self._questions
34-
3527
def _user_input_loop(self) -> Generator[tuple[str, str], None, None]:
3628
print(self._help_message)
3729

@@ -41,14 +33,15 @@ def _user_input_loop(self) -> Generator[tuple[str, str], None, None]:
4133
_ = yield q, "question"
4234

4335
def _print_message(self, choice: dict) -> None:
44-
if delta := choice.get("delta"):
36+
if "delta" in choice:
37+
delta = choice["delta"] or {}
38+
if not delta:
39+
return
4540
if not self._delta_start:
4641
header = f" {delta['role'].capitalize()} Message ".center(80, "=")
4742
print("\n", header)
4843
self._delta_start = (
49-
True
50-
and (choice.get("finish_reason") is None)
51-
and delta["role"] != "tool"
44+
choice.get("finish_reason") is None and delta["role"] != "tool"
5245
)
5346
print(delta.get("content") or delta.get("tool_calls"), flush=True, end="")
5447
else:

‎agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@ def get_formatted_message(
5454
}
5555
elif role == "ai": # this implies resp.additional_kwargs
5656
if additional_kw := resp.additional_kwargs:
57-
tool_call = additional_kw["tool_calls"][0]
57+
tool_calls = additional_kw.get("tool_calls")
58+
if not tool_calls:
59+
return None
60+
tool_call = tool_calls[0]
5861
if is_assistant:
5962
return {
6063
"role": "assistant",

‎agents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import uuid
22
from os import getenv
33

4+
from ci_failure_summarizer.config import normalize_base_url
5+
46
from ._interactive_chat import InteractiveChat
57
from .ai_service import ai_stream_service
68

@@ -30,9 +32,7 @@ def get_headers(self):
3032
model_id = getenv("MODEL_ID")
3133
api_key = getenv("API_KEY")
3234

33-
# Ensure base_url ends with /v1 if provided
34-
if base_url and not base_url.endswith("/v1"):
35-
base_url = base_url.rstrip("/") + "/v1"
35+
base_url = normalize_base_url(base_url)
3636

3737
context = SimpleContext()
3838
ai_service_resp_func = ai_stream_service(

‎agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yaml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,20 @@ spec:
1616
template:
1717
spec:
1818
restartPolicy: Never
19+
securityContext:
20+
runAsNonRoot: true
21+
seccompProfile:
22+
type: RuntimeDefault
1923
containers:
2024
- name: trigger
2125
image: image-registry.openshift-image-registry.svc:5000/ci-testing/langgraph-ci-failure-summarizer-agent:latest
2226
imagePullPolicy: Always
27+
securityContext:
28+
allowPrivilegeEscalation: false
29+
readOnlyRootFilesystem: true
30+
capabilities:
31+
drop:
32+
- ALL
2333
command:
2434
- python
2535
- -c

‎agents/langgraph/templates/ci_failure_summarizer/main.py‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from typing import Any
1111

1212
from ci_failure_summarizer.agent import get_graph_closure
13-
from ci_failure_summarizer.config import SummarizerConfig
13+
from ci_failure_summarizer.config import SummarizerConfig, normalize_base_url
1414
from ci_failure_summarizer.incident_store import IncidentStore
1515
from ci_failure_summarizer.models import SummaryResult
1616
from ci_failure_summarizer.orchestrator import SummarizerOrchestrator
@@ -187,12 +187,9 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
187187

188188
enable_tracing()
189189

190-
base_url = getenv("BASE_URL")
190+
base_url = normalize_base_url(getenv("BASE_URL"))
191191
model_id = getenv("MODEL_ID")
192192

193-
if base_url and not base_url.endswith("/v1"):
194-
base_url = base_url.rstrip("/") + "/v1"
195-
196193
DB_URI = get_database_uri()
197194

198195
with PostgresSaver.from_conn_string(DB_URI) as saver:
@@ -392,10 +389,9 @@ async def _handle_chat(
392389
"usage": _extract_usage(new_messages),
393390
}
394391

395-
except Exception as e:
396-
raise HTTPException(
397-
status_code=500, detail=f"Error processing request: {str(e)}"
398-
)
392+
except Exception:
393+
logger.exception("Error processing chat completion request")
394+
raise HTTPException(status_code=500, detail="Error processing request")
399395

400396

401397
async def _handle_stream(
@@ -614,11 +610,9 @@ async def summarize(request: SummarizeRequest):
614610
raise HTTPException(status_code=400, detail=str(exc)) from exc
615611
except RuntimeError as exc:
616612
raise HTTPException(status_code=502, detail=str(exc)) from exc
617-
except Exception as exc:
613+
except Exception:
618614
logger.exception("Error running CI failure summarizer")
619-
raise HTTPException(
620-
status_code=500, detail=f"Error running summarizer: {exc}"
621-
) from exc
615+
raise HTTPException(status_code=500, detail="Error running summarizer")
622616

623617

624618
# ── Playground UI ────────────────────────────────────────────────────────────
@@ -627,7 +621,7 @@ async def summarize(request: SummarizeRequest):
627621
# In Docker the images are copied to /opt/app-root/src/images; locally they live at the repo root
628622
_IMAGES_DIR = _BASE_DIR / "images"
629623
if not _IMAGES_DIR.is_dir():
630-
_IMAGES_DIR = _BASE_DIR.parent.parent.parent / "images"
624+
_IMAGES_DIR = _BASE_DIR.parent.parent.parent.parent / "images"
631625

632626

633627
@app.get("/", response_class=HTMLResponse, include_in_schema=False)

‎agents/langgraph/templates/ci_failure_summarizer/playground/app.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
logging.basicConfig(level=logging.DEBUG)
3434
logger = logging.getLogger(__name__)
3535

36-
IMAGES_DIR = Path(__file__).resolve().parents[4] / "images"
36+
IMAGES_DIR = Path(__file__).resolve().parents[5] / "images"
3737

3838
app = Flask(__name__)
3939

‎agents/langgraph/templates/ci_failure_summarizer/pyproject.toml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,9 @@ dependencies = [
2020
"psycopg[binary,pool]>=3.2.9",
2121
"openai>=2.21.0",
2222
"python-dotenv>=1.2.1",
23-
"pymilvus==2.5.7", # <-- Downgraded to the proven safe version
24-
"milvus-lite>=2.4.0", # <-- Matches the safe PyMilvus
2523
"setuptools>=80.9.0,<83.0.0",
2624
"typing-extensions>=4.15.0",
2725
"requests>=2.31.0",
28-
"chardet>=7.1.0",
29-
"pypdf>=6.9.0",
3026
"flask>=3.1.0",
3127
]
3228

‎agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from typing import Any, Callable
33

44
from ci_failure_summarizer import TOOLS
5+
from ci_failure_summarizer.config import normalize_base_url
56
from langchain.agents import create_agent
67
from langchain.agents.middleware import AgentMiddleware
78
from langchain_core.messages import BaseMessage
@@ -69,9 +70,7 @@ def get_graph_closure(
6970
if not model_id:
7071
model_id = getenv("MODEL_ID")
7172

72-
# Ensure base_url ends with /v1
73-
if base_url and not base_url.endswith("/v1"):
74-
base_url = base_url.rstrip("/") + "/v1"
73+
base_url = normalize_base_url(base_url)
7574

7675
# Validate API key for non-local environments
7776
if not base_url:

‎agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@
99
CI_DASHBOARD_URL = "https://red-hat-data-services.github.io/agentic-starter-kits/"
1010

1111

12+
def normalize_base_url(base_url: str | None) -> str | None:
13+
if not base_url:
14+
return base_url
15+
normalized = base_url.rstrip("/")
16+
if not normalized.endswith("/v1"):
17+
normalized = normalized + "/v1"
18+
return normalized
19+
20+
1221
@dataclass(frozen=True)
1322
class SummarizerConfig:
1423
repository: str
@@ -24,15 +33,14 @@ class SummarizerConfig:
2433
@classmethod
2534
def from_env(cls) -> SummarizerConfig:
2635
repository = getenv("GITHUB_REPOSITORY", "").strip()
27-
workflow_name = getenv(
28-
"GITHUB_WORKFLOW", "QG4: Agent Deployment Integration Tests"
29-
).strip()
36+
workflow_name = (
37+
getenv("GITHUB_WORKFLOW", "QG4: Agent Deployment Integration Tests").strip()
38+
or "QG4: Agent Deployment Integration Tests"
39+
)
3040
if not repository:
3141
raise ValueError("GITHUB_REPOSITORY is required")
3242

33-
base_url = getenv("BASE_URL")
34-
if base_url and not base_url.endswith("/v1"):
35-
base_url = base_url.rstrip("/") + "/v1"
43+
base_url = normalize_base_url(getenv("BASE_URL"))
3644

3745
return cls(
3846
repository=repository,

0 commit comments

Comments
 (0)