feat(langgraph): add deterministic CI failure summarizer spike#272
feat(langgraph): add deterministic CI failure summarizer spike#272kami619 wants to merge 18 commits into
Conversation
2ff7257 to
1b73345
Compare
|
Large PR detected (7672 lines changed) This PR exceeds 1200 lines of code changes (excluding lock files, generated content, and images). Large PRs are harder to review thoroughly and are more likely to introduce bugs. Consider splitting this PR into smaller, focused changes. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a LangGraph CI Failure Summarizer template with OpenAI-compatible chat APIs, GitHub Actions failure ingestion, evidence grouping, PostgreSQL persistence, Slack notifications, deployment tooling, a browser playground, examples, documentation, and unit, behavioral, and integration tests. ChangesCI Failure Summarizer agent
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Trigger
participant SummarizerOrchestrator
participant GitHubActionsClient
participant IncidentStore
participant Slack
Trigger->>SummarizerOrchestrator: POST /summarize or run trigger
SummarizerOrchestrator->>GitHubActionsClient: Resolve workflow and fetch failures
GitHubActionsClient-->>SummarizerOrchestrator: Jobs, logs, and evidence
SummarizerOrchestrator->>IncidentStore: Upsert incidents and record summary
SummarizerOrchestrator->>Slack: Post summary when enabled
Slack-->>SummarizerOrchestrator: Delivery result
sequenceDiagram
participant Browser
participant Playground
participant FastAPI
participant LangGraph
participant PostgreSQL
Browser->>Playground: Send chat message
Playground->>FastAPI: POST /chat/completions
FastAPI->>PostgreSQL: Load thread checkpoint
FastAPI->>LangGraph: Stream agent events
LangGraph-->>FastAPI: Token deltas
FastAPI-->>Playground: SSE response chunks
Playground-->>Browser: Render assistant response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fix the ci_failure_summarizer template issues called out on PR #272 by making the local CLI example type-checkable, guarding nullable summary inserts, hardening image path serving, and cleaning the remaining template lint violations. Co-authored-by: Cursor <cursoragent@cursor.com>
Format the ci_failure_summarizer files that were still failing the PR #272 lint workflow while keeping the earlier lint, type-check, and CodeQL fixes intact. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (9)
agents/langgraph/templates/ci_failure_summarizer/Makefile (1)
118-187: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared
--setflag list to cut duplication betweendeployanddry-run.The non-secret
--set env.*flags (image, BASE_URL, MODEL_ID, POSTGRES_, GITHUB_REPOSITORY, and the optional GITHUB_WORKFLOW/MLFLOW_ vars) are duplicated almost verbatim betweendeploy(Lines 135-149) anddry-run(Lines 170-187). Any new env var must be added in both places or the targets drift.Suggested refactor
+HELM_ENV_SET = --set image.repository="$${IMAGE_REPO}" \ + --set image.tag="$${IMAGE_TAG}" \ + --set env.BASE_URL="$${BASE_URL}" \ + --set env.MODEL_ID="$${MODEL_ID}" \ + --set env.POSTGRES_HOST="$${POSTGRES_HOST}" \ + --set env.POSTGRES_PORT="$${POSTGRES_PORT}" \ + --set env.POSTGRES_DB="$${POSTGRES_DB}" \ + --set env.POSTGRES_USER="$${POSTGRES_USER}" \ + --set env.GITHUB_REPOSITORY="$${GITHUB_REPOSITORY}" \ + $${GITHUB_WORKFLOW:+--set env.GITHUB_WORKFLOW="$${GITHUB_WORKFLOW}"} \ + $${GITHUB_WORKFLOW_FILE:+--set env.GITHUB_WORKFLOW_FILE="$${GITHUB_WORKFLOW_FILE}"} \ + $${MLFLOW_TRACKING_URI:+--set env.MLFLOW_TRACKING_URI="$${MLFLOW_TRACKING_URI}"} \ + $${MLFLOW_EXPERIMENT_NAME:+--set env.MLFLOW_EXPERIMENT_NAME="$${MLFLOW_EXPERIMENT_NAME}"} \ + $${MLFLOW_TRACKING_INSECURE_TLS:+--set env.MLFLOW_TRACKING_INSECURE_TLS="$${MLFLOW_TRACKING_INSECURE_TLS}"} \ + $${MLFLOW_WORKSPACE:+--set env.MLFLOW_WORKSPACE="$${MLFLOW_WORKSPACE}"}Then reference
$(HELM_ENV_SET)in bothdeployanddry-run, appending only the secrets-specific flags in each.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/Makefile` around lines 118 - 187, Extract the duplicated non-secret Helm environment flags from deploy and dry-run into a shared HELM_ENV_SET variable, including image, required env.* values, and optional GITHUB_WORKFLOW/MLFLOW_* flags. Replace both inline flag lists with $(HELM_ENV_SET), retaining each target’s existing secrets-specific flags separately.agents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.py (1)
58-87: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRestrict permissions on the generated
.envfile containing secrets.
_write_env_filewritesPOSTGRES_PASSWORD,GITHUB_TOKEN, andSLACK_WEBHOOK_URLintoagent_dir/.envwith default file permissions. The Makefile'sdeploytarget appliesumask 077when writing its own secrets file (.helm-secrets.yaml) — this test fixture should do the same for consistency and to avoid leaving secrets world/group-readable during the test run.Fix
env_path.write_text("\n".join(lines) + "\n") + env_path.chmod(0o600) return env_path🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.py` around lines 58 - 87, The _write_env_file function currently creates the generated .env file with default permissions, potentially exposing secrets. Ensure the file is created or its permissions are restricted with a 077 umask before writing, while preserving the existing environment content and return behavior.agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py (1)
193-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
dataclasses.replaceover__dict__spread.More idiomatic and resilient to future field changes than reconstructing via
**{**failure.__dict__, ...}.- failure = _sample_failure() - failure = FailureRecord( - **{**failure.__dict__, "evidence": FailureEvidence( - source="github_job_log", - excerpt="ERROR: health check failed", - markers=("ERROR: health check failed",), - signature="sig-123", - run_id=123456789, - )} - ) + failure = dataclasses.replace( + _sample_failure(), + evidence=FailureEvidence( + source="github_job_log", + excerpt="ERROR: health check failed", + markers=("ERROR: health check failed",), + signature="sig-123", + run_id=123456789, + ), + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py` around lines 193 - 202, Update the FailureRecord reconstruction in the test to use dataclasses.replace on the existing failure instance, replacing only its evidence field with the new FailureEvidence value; remove the __dict__ spread while preserving all other failure fields.agents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.py (1)
85-140: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
pytest.raisesover manual try/except/else.The three tests here (
test_request_surfaces_rate_limit_for_metadata_fetch,test_request_surfaces_not_found_for_missing_run,test_request_surfaces_network_error_for_metadata_fetch) usetry/except RuntimeError ... else: raise AssertionErrorinstead ofpytest.raises, which is the idiomatic pattern and gives clearer failure output.♻️ Example refactor for one instance
- try: - client.get_run(123456789) - except RuntimeError as exc: - assert "rate limit" in str(exc).lower() - assert "60" in str(exc) - assert "actions/runs/123456789" in str(exc) - else: - raise AssertionError("expected RuntimeError") + with pytest.raises(RuntimeError, match=r"(?i)rate limit"): + client.get_run(123456789)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.py` around lines 85 - 140, Refactor the three tests—test_request_surfaces_rate_limit_for_metadata_fetch, test_request_surfaces_not_found_for_missing_run, and test_request_surfaces_network_error_for_metadata_fetch—to use pytest.raises(RuntimeError) as a context manager. Keep each test’s existing exception-message assertions inside the context and remove the manual try/except/else blocks, adding the pytest import if needed.agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.py (1)
189-206: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead exception branch:
requests.RequestExceptioncan never reach here.
_request(Lines 119-145) already catchesrequests.RequestExceptioninternally and re-raises asRuntimeError, so callers of_requestnever see a rawRequestException. Theexcept requests.RequestExceptionbranch here is unreachable.♻️ Proposed cleanup
try: response = self._request( "GET", path, allow_statuses=frozenset({403, 404}), ) - except requests.RequestException as exc: - return LogFetchResult( - job_id=job_id, - available=False, - error=str(exc), - ) except RuntimeError as exc: return LogFetchResult( job_id=job_id, available=False, error=str(exc), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.py` around lines 189 - 206, Remove the unreachable requests.RequestException handler from the log-fetch flow around _request, leaving RuntimeError handling to produce the existing unavailable LogFetchResult. Keep the _request call and error-result behavior unchanged.agents/langgraph/templates/ci_failure_summarizer/main.py (1)
342-343: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant per-request checkpoint schema setup.
Both
_handle_chatand_handle_streamcallawait saver.setup()on theAsyncPostgresSaverfor every chat request, even though schema setup already runs once at startup (Lines 198-199) via the syncPostgresSaver. This adds an unnecessary DB round-trip to every chat message.Please confirm
PostgresSaverandAsyncPostgresSaverfromlanggraph-checkpoint-postgresmanage the same checkpoint schema, then drop the per-requestsetup()calls:Proposed fix
async with AsyncPostgresSaver.from_conn_string(DB_URI) as saver: - await saver.setup() - if system_prompt:(apply the same removal in
_handle_stream)Also applies to: 416-417
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/main.py` around lines 342 - 343, Remove the per-request await saver.setup() calls from both _handle_chat and _handle_stream after confirming PostgresSaver and AsyncPostgresSaver use the same checkpoint schema. Retain the single startup schema initialization performed by PostgresSaver, while preserving each handler’s existing saver context and request behavior.agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py (1)
35-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
BASE_URL/v1-suffix normalization is duplicated verbatim and shares a trailing-slash edge case bug.Both sites apply
if base_url and not base_url.endswith("/v1"): base_url = base_url.rstrip("/") + "/v1". IfBASE_URLalready ends with/v1/(trailing slash),.endswith("/v1")isFalse, and the code appends again, producing.../v1/v1. Extract a shared helper (e.g., inutils.py) to fix once and avoid drift.
agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py#L35-L37: replace with a call to a sharednormalize_base_url()helper.agents/langgraph/templates/ci_failure_summarizer/main.py#L190-L194: replace with the same shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py` around lines 35 - 37, Extract the duplicated BASE_URL normalization into a shared normalize_base_url() helper, handling trailing slashes so values ending in /v1/ normalize to a single /v1 suffix. Replace the inline logic in agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py lines 35-37 and agents/langgraph/templates/ci_failure_summarizer/main.py lines 190-194 with calls to that helper.agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py (1)
14-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused
postgres_db_connection_idparameter.This parameter is never referenced in the function body —
DB_URIis always derived fromget_database_uri()(line 22). It misleads callers into thinking they can target a specific DB connection. Remove it or wire it through.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py` around lines 14 - 16, Remove the unused postgres_db_connection_id parameter from ai_stream_service and update all callers to match the revised signature, since the function always obtains DB_URI through get_database_uri().agents/langgraph/templates/ci_failure_summarizer/pyproject.toml (1)
9-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrune the unused vector/PDF deps. If
pymilvus,milvus-lite,chardet, andpypdfaren’t needed outside this template, drop them fromagents/langgraph/templates/ci_failure_summarizer/pyproject.toml:23-29; they don’t appear anywhere undersrc/ortests/, and the inline notes look inherited from a different template.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/pyproject.toml` around lines 9 - 31, Remove the unused pymilvus, milvus-lite, chardet, and pypdf entries from the dependencies list in the ci_failure_summarizer pyproject configuration, including their inline comments. Leave all other dependencies unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@agents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.py`:
- Around line 15-33: Remove the unwired canned-questions scaffolding from the
interactive chat template: delete the list_questions entry from _help_message,
remove the unused _ordered_list helper and questions property, and eliminate any
related references while preserving the remaining help commands and run()
behavior.
- Around line 43-59: Update _print_message to distinguish a missing delta from
an explicitly provided empty delta. Handle the terminal streaming chunk with
delta={} without entering the non-streaming choice['message'] branch or raising
KeyError, while preserving existing output for populated deltas and
non-streaming messages.
In `@agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py`:
- Around line 55-57: Update the AI-message handling in the response formatting
logic around the role == "ai" branch to access tool_calls only when that key
exists and contains at least one entry. For empty-content responses with other
additional_kwargs such as refusal metadata, avoid indexing tool_calls and
preserve the non-tool-call handling in both generate() and generate_stream().
In
`@agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yaml`:
- Around line 17-41: Add a pod/container securityContext to the trigger
container in the CronJob template, configuring it to run as non-root with
privilege escalation disabled and a read-only root filesystem. Preserve the
existing command, arguments, image, and restart behavior.
In `@agents/langgraph/templates/ci_failure_summarizer/main.py`:
- Around line 395-398: Stop exposing raw exception text from the `_handle_chat`
and `/summarize` error handlers. Replace `str(e)`/`{exc}` in their HTTP 500
details with a generic client-safe message, while logging the original exception
details through the existing server-side logging path if available.
- Around line 624-630: The shared images-directory fallback is one directory too
shallow. In agents/langgraph/templates/ci_failure_summarizer/main.py, update the
_IMAGES_DIR fallback to use _BASE_DIR.parent.parent.parent.parent / "images"; in
agents/langgraph/templates/ci_failure_summarizer/playground/app.py, update the
fallback to use Path(__file__).resolve().parents[5] / "images" so both resolve
to the repository-root images directory.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py`:
- Around line 1-11: Run Ruff formatting on config.py and apply the formatter’s
changes so the file passes `ruff format --check`; preserve the existing
configuration declarations and behavior.
- Around line 29-31: Update the workflow_name initialization to apply the same
empty-string fallback pattern used by workflow_file and the
slack_webhook_url/github_token settings, ensuring a blank GITHUB_WORKFLOW
resolves to the default workflow name while preserving stripping of non-empty
values.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.py`:
- Around line 1-9: Run Ruff formatting on failure_evidence.py lines 1-9 and
github_client.py lines 1-11, applying the formatter’s output so both files pass
ruff format --check; no behavioral changes are needed.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.py`:
- Around line 39-134: The run method lacks an idempotency guard before posting
summaries to Slack. Before invoking maybe_post_summary, query incident_store for
an existing summary for run.id with slack_posted=True; when found, skip the post
and preserve the existing posted state and appropriate skip reason, while
retaining the current posting flow for runs without a prior successful Slack
post.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/summary_composer.py`:
- Line 1: Run Ruff formatting and commit the resulting changes for
agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/summary_composer.py,
agents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.py,
agents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.py,
agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py,
and
agents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.py;
no logic changes are required.
In
`@agents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.py`:
- Around line 66-69: Insert a blank line between the env_path assignment and the
nested shell_assign function definition in the surrounding fixture setup,
matching Ruff/Black formatting requirements.
---
Nitpick comments:
In `@agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py`:
- Around line 14-16: Remove the unused postgres_db_connection_id parameter from
ai_stream_service and update all callers to match the revised signature, since
the function always obtains DB_URI through get_database_uri().
In `@agents/langgraph/templates/ci_failure_summarizer/main.py`:
- Around line 342-343: Remove the per-request await saver.setup() calls from
both _handle_chat and _handle_stream after confirming PostgresSaver and
AsyncPostgresSaver use the same checkpoint schema. Retain the single startup
schema initialization performed by PostgresSaver, while preserving each
handler’s existing saver context and request behavior.
In `@agents/langgraph/templates/ci_failure_summarizer/Makefile`:
- Around line 118-187: Extract the duplicated non-secret Helm environment flags
from deploy and dry-run into a shared HELM_ENV_SET variable, including image,
required env.* values, and optional GITHUB_WORKFLOW/MLFLOW_* flags. Replace both
inline flag lists with $(HELM_ENV_SET), retaining each target’s existing
secrets-specific flags separately.
In `@agents/langgraph/templates/ci_failure_summarizer/pyproject.toml`:
- Around line 9-31: Remove the unused pymilvus, milvus-lite, chardet, and pypdf
entries from the dependencies list in the ci_failure_summarizer pyproject
configuration, including their inline comments. Leave all other dependencies
unchanged.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py`:
- Around line 35-37: Extract the duplicated BASE_URL normalization into a shared
normalize_base_url() helper, handling trailing slashes so values ending in /v1/
normalize to a single /v1 suffix. Replace the inline logic in
agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py
lines 35-37 and agents/langgraph/templates/ci_failure_summarizer/main.py lines
190-194 with calls to that helper.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.py`:
- Around line 189-206: Remove the unreachable requests.RequestException handler
from the log-fetch flow around _request, leaving RuntimeError handling to
produce the existing unavailable LogFetchResult. Keep the _request call and
error-result behavior unchanged.
In
`@agents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.py`:
- Around line 58-87: The _write_env_file function currently creates the
generated .env file with default permissions, potentially exposing secrets.
Ensure the file is created or its permissions are restricted with a 077 umask
before writing, while preserving the existing environment content and return
behavior.
In
`@agents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.py`:
- Around line 85-140: Refactor the three
tests—test_request_surfaces_rate_limit_for_metadata_fetch,
test_request_surfaces_not_found_for_missing_run, and
test_request_surfaces_network_error_for_metadata_fetch—to use
pytest.raises(RuntimeError) as a context manager. Keep each test’s existing
exception-message assertions inside the context and remove the manual
try/except/else blocks, adding the pytest import if needed.
In
`@agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py`:
- Around line 193-202: Update the FailureRecord reconstruction in the test to
use dataclasses.replace on the existing failure instance, replacing only its
evidence field with the new FailureEvidence value; remove the __dict__ spread
while preserving all other failure fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 2df34a10-a41e-4270-a2bd-15a57c3222d7
⛔ Files ignored due to path filters (1)
agents/langgraph/templates/ci_failure_summarizer/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (64)
README.mdagents/langgraph/README.mdagents/langgraph/templates/ci_failure_summarizer/.dockerignoreagents/langgraph/templates/ci_failure_summarizer/.env.exampleagents/langgraph/templates/ci_failure_summarizer/.gitignoreagents/langgraph/templates/ci_failure_summarizer/Dockerfileagents/langgraph/templates/ci_failure_summarizer/LICENSEagents/langgraph/templates/ci_failure_summarizer/Makefileagents/langgraph/templates/ci_failure_summarizer/README.mdagents/langgraph/templates/ci_failure_summarizer/agent.yamlagents/langgraph/templates/ci_failure_summarizer/evalhub/tool_use.yamlagents/langgraph/templates/ci_failure_summarizer/examples/__init__.pyagents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.pyagents/langgraph/templates/ci_failure_summarizer/examples/ai_service.pyagents/langgraph/templates/ci_failure_summarizer/examples/clear_thread_history.pyagents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.pyagents/langgraph/templates/ci_failure_summarizer/examples/query_existing_deployment.pyagents/langgraph/templates/ci_failure_summarizer/examples/test_memory_fifo.ipynbagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_after_qg4.shagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yamlagents/langgraph/templates/ci_failure_summarizer/examples/trigger_summary.pyagents/langgraph/templates/ci_failure_summarizer/main.pyagents/langgraph/templates/ci_failure_summarizer/playground/app.pyagents/langgraph/templates/ci_failure_summarizer/playground/templates/index.htmlagents/langgraph/templates/ci_failure_summarizer/pyproject.tomlagents/langgraph/templates/ci_failure_summarizer/src/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/grouping.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/incident_store.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/models.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/tracing.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/utils.pyagents/langgraph/templates/ci_failure_summarizer/tests/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/fixtures/golden_queries.yamlagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_cost_latency.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_isolation.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_persistence.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_reliability.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_response_quality.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_streaming_parity.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_tool_usage.pyagents/langgraph/templates/ci_failure_summarizer/tests/fixtures/github_failure_run.jsonagents/langgraph/templates/ci_failure_summarizer/tests/integration/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/test_deployment.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_api_contract.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_config.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summarize_api.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_utils.pyagents/langgraph/templates/ci_failure_summarizer/values.yaml
|
@CodeRabbit I have fixed all your previous review items, can you take another look ? |
Fix the ci_failure_summarizer template issues called out on PR #272 by making the local CLI example type-checkable, guarding nullable summary inserts, hardening image path serving, and cleaning the remaining template lint violations. Co-authored-by: Cursor <cursoragent@cursor.com>
Format the ci_failure_summarizer files that were still failing the PR #272 lint workflow while keeping the earlier lint, type-check, and CodeQL fixes intact. Co-authored-by: Cursor <cursoragent@cursor.com>
9207982 to
4e1e368
Compare
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/langgraph/templates/ci_failure_summarizer/agent.yaml`:
- Around line 8-17: Ensure MODEL_ID is validated consistently at startup: either
add a fail-fast check in get_graph_closure() or lifespan() before constructing
ChatOpenAI, or remove MODEL_ID from env.required if it is intentionally
optional. Align SummarizerConfig.from_env() and the required environment
contract so MODEL_ID cannot unexpectedly flow through as None.
In `@agents/langgraph/templates/ci_failure_summarizer/Makefile`:
- Around line 139-146: Update the secret-generation block in the Makefile target
that writes .helm-secrets.yaml so API_KEY, POSTGRES_PASSWORD,
MLFLOW_TRACKING_TOKEN, GITHUB_TOKEN, and SLACK_WEBHOOK_URL are YAML-escaped or
serialized safely before insertion. Preserve the existing conditional inclusion
of optional secrets and ensure values containing quotes, backslashes, or
newlines produce valid YAML without altering the secret content.
In `@agents/langgraph/templates/ci_failure_summarizer/playground/app.py`:
- Around line 50-75: Align the frontend requests in index.html with the Flask
routes defined by health and chat: update the UI to call /api/health and
/api/chat (including the chat request path) instead of /health and
/chat/completions. Preserve the existing request behavior and payload handling.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.py`:
- Around line 107-128: The Slack duplicate-post guard in run must use an atomic
store-layer claim rather than the separate get_latest_summary_for_run check and
record_summary write. Add or reuse an IncidentStore operation that atomically
claims Slack posting for the run, and call maybe_post_summary only when that
claim succeeds; preserve the skipped reason for already-claimed summaries and
ensure the summary record is still persisted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 5ae29ebc-0c04-4710-881d-01b6b93fd53a
⛔ Files ignored due to path filters (1)
agents/langgraph/templates/ci_failure_summarizer/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
README.mdagents/langgraph/README.mdagents/langgraph/templates/ci_failure_summarizer/.dockerignoreagents/langgraph/templates/ci_failure_summarizer/.env.exampleagents/langgraph/templates/ci_failure_summarizer/.gitignoreagents/langgraph/templates/ci_failure_summarizer/Dockerfileagents/langgraph/templates/ci_failure_summarizer/LICENSEagents/langgraph/templates/ci_failure_summarizer/Makefileagents/langgraph/templates/ci_failure_summarizer/README.mdagents/langgraph/templates/ci_failure_summarizer/agent.yamlagents/langgraph/templates/ci_failure_summarizer/evalhub/tool_use.yamlagents/langgraph/templates/ci_failure_summarizer/examples/__init__.pyagents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.pyagents/langgraph/templates/ci_failure_summarizer/examples/ai_service.pyagents/langgraph/templates/ci_failure_summarizer/examples/clear_thread_history.pyagents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.pyagents/langgraph/templates/ci_failure_summarizer/examples/query_existing_deployment.pyagents/langgraph/templates/ci_failure_summarizer/examples/test_memory_fifo.ipynbagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_after_qg4.shagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yamlagents/langgraph/templates/ci_failure_summarizer/examples/trigger_summary.pyagents/langgraph/templates/ci_failure_summarizer/main.pyagents/langgraph/templates/ci_failure_summarizer/playground/app.pyagents/langgraph/templates/ci_failure_summarizer/playground/templates/index.htmlagents/langgraph/templates/ci_failure_summarizer/pyproject.tomlagents/langgraph/templates/ci_failure_summarizer/src/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/grouping.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/incident_store.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/models.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/tracing.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/utils.pyagents/langgraph/templates/ci_failure_summarizer/tests/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/fixtures/golden_queries.yamlagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_cost_latency.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_isolation.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_persistence.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_reliability.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_response_quality.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_streaming_parity.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_tool_usage.pyagents/langgraph/templates/ci_failure_summarizer/tests/fixtures/github_failure_run.jsonagents/langgraph/templates/ci_failure_summarizer/tests/integration/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/test_deployment.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_api_contract.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_config.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_examples.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summarize_api.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_utils.pyagents/langgraph/templates/ci_failure_summarizer/values.yaml
🚧 Files skipped from review as they are similar to previous changes (43)
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/init.py
- agents/langgraph/templates/ci_failure_summarizer/tests/integration/test_deployment.py
- agents/langgraph/templates/ci_failure_summarizer/values.yaml
- agents/langgraph/templates/ci_failure_summarizer/evalhub/tool_use.yaml
- agents/langgraph/templates/ci_failure_summarizer/tests/test_utils.py
- README.md
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_streaming_parity.py
- agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_after_qg4.sh
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_cost_latency.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/fixtures/golden_queries.yaml
- agents/langgraph/templates/ci_failure_summarizer/tests/fixtures/github_failure_run.json
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_persistence.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.py
- agents/langgraph/templates/ci_failure_summarizer/pyproject.toml
- agents/langgraph/templates/ci_failure_summarizer/.gitignore
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_isolation.py
- agents/langgraph/README.md
- agents/langgraph/templates/ci_failure_summarizer/LICENSE
- agents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.py
- agents/langgraph/templates/ci_failure_summarizer/.dockerignore
- agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yaml
- agents/langgraph/templates/ci_failure_summarizer/examples/query_existing_deployment.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_response_quality.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/grouping.py
- agents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_summarize_api.py
- agents/langgraph/templates/ci_failure_summarizer/Dockerfile
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_tool_usage.py
- agents/langgraph/templates/ci_failure_summarizer/examples/clear_thread_history.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/utils.py
- agents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.py
- agents/langgraph/templates/ci_failure_summarizer/examples/test_memory_fifo.ipynb
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_reliability.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/models.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_api_contract.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.py
- agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py
Fix the ci_failure_summarizer template issues called out on PR #272 by making the local CLI example type-checkable, guarding nullable summary inserts, hardening image path serving, and cleaning the remaining template lint violations. Co-authored-by: Cursor <cursoragent@cursor.com>
Format the ci_failure_summarizer files that were still failing the PR #272 lint workflow while keeping the earlier lint, type-check, and CodeQL fixes intact. Co-authored-by: Cursor <cursoragent@cursor.com>
4e1e368 to
2a9f69c
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py (1)
161-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
finalize_slack_post_claim(posted=True),get_incident, andget_latest_summary_for_run.Only the failed-post (
DELETE) branch offinalize_slack_post_claimis tested. The success-pathUPDATE ... SET posted_at = COALESCE(...)branch — which guards against overwriting an already-setposted_at— and the two read methods have no test coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py` around lines 161 - 175, Extend the IncidentStore tests with coverage for finalize_slack_post_claim(123, posted=True), asserting it executes the success UPDATE with COALESCE and commits; add tests for get_incident and get_latest_summary_for_run that verify their queries and returned values using the mocked connection. Preserve the existing failed-post deletion test.agents/langgraph/templates/ci_failure_summarizer/main.py (1)
195-199: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant per-request
saver.setup()call.
PostgresSaver.setup()already runs once at startup (lines 195-196). Callingawait saver.setup()again inside_handle_chat/_handle_streamon every request adds an unnecessary DB round-trip to the hot path.Also applies to: 339-340, 412-413
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langgraph/templates/ci_failure_summarizer/main.py` around lines 195 - 199, The startup initialization already invokes PostgresSaver.setup(), so remove the duplicate saver.setup() calls from the per-request _handle_chat and _handle_stream paths. Keep the single startup setup invocation and leave IncidentStore initialization unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/langgraph/templates/ci_failure_summarizer/main.py`:
- Around line 105-126: Add a context field to ChatCompletionResponse matching
the type and optionality of the context_messages value returned by _handle_chat,
so FastAPI preserves it in /chat/completions responses. Keep the existing
response fields and context construction unchanged.
In `@agents/langgraph/templates/ci_failure_summarizer/Makefile`:
- Around line 139-144: Fix the inline Python script in the deploy command so it
compiles by replacing the semicolon-chained if statements with valid conditional
expressions or a multiline script. Preserve the existing conditional inclusion
of mlflowTrackingToken and extraSecrets, and ensure .helm-secrets.yaml is
written before Helm runs.
In `@agents/langgraph/templates/ci_failure_summarizer/README.md`:
- Line 400: Update the OpenShift Deployment link in the README to use the
correct relative path ../../../../docs/openshift-deployment.md instead of
../../../docs/openshift-deployment.md.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/incident_store.py`:
- Around line 67-70: Update IncidentStore._connection to pass a finite
connect_timeout to psycopg.connect alongside db_uri and row_factory=dict_row.
Use the existing configuration mechanism or an appropriate bounded default so
unreachable PostgreSQL instances cannot block calls indefinitely.
---
Nitpick comments:
In `@agents/langgraph/templates/ci_failure_summarizer/main.py`:
- Around line 195-199: The startup initialization already invokes
PostgresSaver.setup(), so remove the duplicate saver.setup() calls from the
per-request _handle_chat and _handle_stream paths. Keep the single startup setup
invocation and leave IncidentStore initialization unchanged.
In
`@agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py`:
- Around line 161-175: Extend the IncidentStore tests with coverage for
finalize_slack_post_claim(123, posted=True), asserting it executes the success
UPDATE with COALESCE and commits; add tests for get_incident and
get_latest_summary_for_run that verify their queries and returned values using
the mocked connection. Preserve the existing failed-post deletion test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: ef40f283-49c1-4b4a-b6de-c232cbb435fe
⛔ Files ignored due to path filters (1)
agents/langgraph/templates/ci_failure_summarizer/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (67)
README.mdagents/langgraph/README.mdagents/langgraph/templates/ci_failure_summarizer/.dockerignoreagents/langgraph/templates/ci_failure_summarizer/.env.exampleagents/langgraph/templates/ci_failure_summarizer/.gitignoreagents/langgraph/templates/ci_failure_summarizer/Dockerfileagents/langgraph/templates/ci_failure_summarizer/LICENSEagents/langgraph/templates/ci_failure_summarizer/Makefileagents/langgraph/templates/ci_failure_summarizer/README.mdagents/langgraph/templates/ci_failure_summarizer/agent.yamlagents/langgraph/templates/ci_failure_summarizer/evalhub/tool_use.yamlagents/langgraph/templates/ci_failure_summarizer/examples/__init__.pyagents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.pyagents/langgraph/templates/ci_failure_summarizer/examples/ai_service.pyagents/langgraph/templates/ci_failure_summarizer/examples/clear_thread_history.pyagents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.pyagents/langgraph/templates/ci_failure_summarizer/examples/query_existing_deployment.pyagents/langgraph/templates/ci_failure_summarizer/examples/test_memory_fifo.ipynbagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_after_qg4.shagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yamlagents/langgraph/templates/ci_failure_summarizer/examples/trigger_summary.pyagents/langgraph/templates/ci_failure_summarizer/main.pyagents/langgraph/templates/ci_failure_summarizer/playground/app.pyagents/langgraph/templates/ci_failure_summarizer/playground/templates/index.htmlagents/langgraph/templates/ci_failure_summarizer/pyproject.tomlagents/langgraph/templates/ci_failure_summarizer/src/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/grouping.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/incident_store.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/models.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/tracing.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/utils.pyagents/langgraph/templates/ci_failure_summarizer/tests/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/fixtures/golden_queries.yamlagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_cost_latency.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_isolation.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_persistence.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_reliability.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_response_quality.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_streaming_parity.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_tool_usage.pyagents/langgraph/templates/ci_failure_summarizer/tests/fixtures/github_failure_run.jsonagents/langgraph/templates/ci_failure_summarizer/tests/integration/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/test_deployment.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_agent.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_api_contract.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_config.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_examples.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_playground.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summarize_api.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_utils.pyagents/langgraph/templates/ci_failure_summarizer/values.yaml
🚧 Files skipped from review as they are similar to previous changes (43)
- agents/langgraph/templates/ci_failure_summarizer/agent.yaml
- agents/langgraph/templates/ci_failure_summarizer/values.yaml
- agents/langgraph/templates/ci_failure_summarizer/.gitignore
- agents/langgraph/templates/ci_failure_summarizer/examples/clear_thread_history.py
- agents/langgraph/README.md
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/init.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/fixtures/golden_queries.yaml
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_response_quality.py
- agents/langgraph/templates/ci_failure_summarizer/tests/integration/test_deployment.py
- agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yaml
- agents/langgraph/templates/ci_failure_summarizer/tests/test_utils.py
- agents/langgraph/templates/ci_failure_summarizer/Dockerfile
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_cost_latency.py
- agents/langgraph/templates/ci_failure_summarizer/LICENSE
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/utils.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.py
- agents/langgraph/templates/ci_failure_summarizer/tests/fixtures/github_failure_run.json
- agents/langgraph/templates/ci_failure_summarizer/evalhub/tool_use.yaml
- agents/langgraph/templates/ci_failure_summarizer/examples/query_existing_deployment.py
- agents/langgraph/templates/ci_failure_summarizer/.dockerignore
- agents/langgraph/templates/ci_failure_summarizer/pyproject.toml
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_persistence.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/grouping.py
- agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_after_qg4.sh
- agents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_isolation.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_tool_usage.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_summarize_api.py
- agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/models.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.py
- agents/langgraph/templates/ci_failure_summarizer/examples/test_memory_fifo.ipynb
- agents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.py
- README.md
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_reliability.py
- agents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_api_contract.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.py
|
@CodeRabbit can you take another look at your comments and the latest commit based fixes ? |
|
✅ Action performedReview finished.
|
|
@mpk-droid this should be in a state to start the review and the flow itself is working based on the deployment done from this branch on the team cluster. |
Fix the ci_failure_summarizer template issues called out on PR #272 by making the local CLI example type-checkable, guarding nullable summary inserts, hardening image path serving, and cleaning the remaining template lint violations. Co-authored-by: Cursor <cursoragent@cursor.com>
Format the ci_failure_summarizer files that were still failing the PR #272 lint workflow while keeping the earlier lint, type-check, and CodeQL fixes intact. Co-authored-by: Cursor <cursoragent@cursor.com>
40e8f94 to
5b3c8a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/langgraph/templates/ci_failure_summarizer/README.md`:
- Around line 339-349: Update the README login examples to avoid exposing
passwords on the command line: use interactive oc login without a password
argument, and change the Docker example to provide the password through
--password-stdin while preserving the existing username and registry targets.
- Around line 299-309: Update the documented environment variable assignments in
the CI failure summarizer README to remove spaces around each “=” so the file is
valid when loaded by “source .env”. Apply this consistently to API_KEY,
BASE_URL, MODEL_ID, CONTAINER_IMAGE, and all POSTGRES_* settings.
- Around line 31-33: Protect the /summarize endpoint before exposing it through
the default-enabled OpenShift Route: add authentication or an enforced
default-deny network boundary, and ensure unauthenticated requests cannot
trigger GitHub ingestion, post to Slack, or read responses. Update the relevant
route and auth configuration associated with the template agent rather than
relying only on README guidance.
In
`@agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/incident_store.py`:
- Around line 217-250: Update claim_slack_post and finalize_slack_post_claim to
prevent abandoned claims from permanently suppressing Slack summaries: add an
expiring lease with an explicit expiry/ownership field, allow claim_slack_post
to reclaim expired unposted claims, and ensure finalize_slack_post_claim only
finalizes or removes the caller’s active claim. Preserve the chosen delivery
semantics consistently, favoring retryable at-least-once delivery for crashed
workers.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 10d1ef34-e785-4521-9735-8a535718773e
⛔ Files ignored due to path filters (1)
agents/langgraph/templates/ci_failure_summarizer/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
README.mdagents/langgraph/README.mdagents/langgraph/templates/ci_failure_summarizer/.dockerignoreagents/langgraph/templates/ci_failure_summarizer/.env.exampleagents/langgraph/templates/ci_failure_summarizer/.gitignoreagents/langgraph/templates/ci_failure_summarizer/Dockerfileagents/langgraph/templates/ci_failure_summarizer/LICENSEagents/langgraph/templates/ci_failure_summarizer/Makefileagents/langgraph/templates/ci_failure_summarizer/README.mdagents/langgraph/templates/ci_failure_summarizer/agent.yamlagents/langgraph/templates/ci_failure_summarizer/evalhub/tool_use.yamlagents/langgraph/templates/ci_failure_summarizer/examples/__init__.pyagents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.pyagents/langgraph/templates/ci_failure_summarizer/examples/ai_service.pyagents/langgraph/templates/ci_failure_summarizer/examples/clear_thread_history.pyagents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.pyagents/langgraph/templates/ci_failure_summarizer/examples/query_existing_deployment.pyagents/langgraph/templates/ci_failure_summarizer/examples/test_memory_fifo.ipynbagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_after_qg4.shagents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yamlagents/langgraph/templates/ci_failure_summarizer/examples/trigger_summary.pyagents/langgraph/templates/ci_failure_summarizer/main.pyagents/langgraph/templates/ci_failure_summarizer/playground/app.pyagents/langgraph/templates/ci_failure_summarizer/playground/templates/index.htmlagents/langgraph/templates/ci_failure_summarizer/pyproject.tomlagents/langgraph/templates/ci_failure_summarizer/src/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/__init__.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/grouping.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/incident_store.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/models.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/tracing.pyagents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/utils.pyagents/langgraph/templates/ci_failure_summarizer/tests/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/fixtures/golden_queries.yamlagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_cost_latency.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_isolation.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_persistence.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_reliability.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_response_quality.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_streaming_parity.pyagents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_tool_usage.pyagents/langgraph/templates/ci_failure_summarizer/tests/fixtures/github_failure_run.jsonagents/langgraph/templates/ci_failure_summarizer/tests/integration/__init__.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.pyagents/langgraph/templates/ci_failure_summarizer/tests/integration/test_deployment.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_agent.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_api_contract.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_config.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_examples.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_makefile.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_orchestrator.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_playground.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_slack_notifier.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summarize_api.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.pyagents/langgraph/templates/ci_failure_summarizer/tests/test_utils.pyagents/langgraph/templates/ci_failure_summarizer/values.yaml
🚧 Files skipped from review as they are similar to previous changes (43)
- agents/langgraph/templates/ci_failure_summarizer/tests/test_playground.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/init.py
- agents/langgraph/templates/ci_failure_summarizer/LICENSE
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_cost_latency.py
- agents/langgraph/templates/ci_failure_summarizer/evalhub/tool_use.yaml
- agents/langgraph/templates/ci_failure_summarizer/tests/fixtures/github_failure_run.json
- agents/langgraph/templates/ci_failure_summarizer/.dockerignore
- agents/langgraph/templates/ci_failure_summarizer/tests/test_utils.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/fixtures/golden_queries.yaml
- agents/langgraph/templates/ci_failure_summarizer/values.yaml
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_response_quality.py
- agents/langgraph/templates/ci_failure_summarizer/tests/integration/test_deployment.py
- agents/langgraph/templates/ci_failure_summarizer/.gitignore
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_isolation.py
- agents/langgraph/templates/ci_failure_summarizer/Dockerfile
- agents/langgraph/templates/ci_failure_summarizer/agent.yaml
- agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_after_qg4.sh
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/utils.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_memory_persistence.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_streaming_parity.py
- agents/langgraph/templates/ci_failure_summarizer/examples/execute_ai_service_locally.py
- agents/langgraph/templates/ci_failure_summarizer/examples/trigger_daily_summary_cronjob.yaml
- agents/langgraph/templates/ci_failure_summarizer/tests/test_grouping.py
- agents/langgraph/templates/ci_failure_summarizer/examples/clear_thread_history.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/agent.py
- agents/langgraph/templates/ci_failure_summarizer/pyproject.toml
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_tool_usage.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/orchestrator.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_summary_composer.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/config.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/grouping.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/models.py
- agents/langgraph/templates/ci_failure_summarizer/examples/_interactive_chat.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_summarize_api.py
- agents/langgraph/templates/ci_failure_summarizer/examples/query_existing_deployment.py
- agents/langgraph/templates/ci_failure_summarizer/tests/behavioral/test_reliability.py
- agents/langgraph/templates/ci_failure_summarizer/tests/test_incident_store.py
- agents/langgraph/templates/ci_failure_summarizer/tests/integration/conftest.py
- agents/langgraph/templates/ci_failure_summarizer/examples/test_memory_fifo.ipynb
- agents/langgraph/templates/ci_failure_summarizer/tests/test_github_client.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/github_client.py
- agents/langgraph/templates/ci_failure_summarizer/examples/ai_service.py
- agents/langgraph/templates/ci_failure_summarizer/src/ci_failure_summarizer/failure_evidence.py
| - No auth on `/summarize` (same open pattern as other template agents) | ||
|
|
||
| **Spike operational posture:** `/summarize` is **unauthenticated** and intended for operator- or platform-managed triggering after QG4 completes (see examples below). There is no API key gate or production hardening; treat the route as operator-only on a trusted network. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the Helm chart and template values for route exposure and access controls.
rg -n -C3 'route|ingress|NetworkPolicy|oauth|auth|summarize' \
agents/langgraph/templates/ci_failure_summarizer \
agents/langgraph/deployment 2>/dev/nullRepository: red-hat-data-services/agentic-starter-kits
Length of output: 50401
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- values.yaml ---'
sed -n '30,90p' agents/langgraph/deployment/values.yaml | cat -n
echo
echo '--- route.yaml ---'
sed -n '1,120p' agents/langgraph/deployment/templates/route.yaml | cat -n
echo
echo '--- auth-related deployment templates ---'
rg -n -C2 'auth\.enabled|NetworkPolicy|route.enabled|ingress.enabled|auth:' agents/langgraph/deployment/templates agents/langgraph/deployment/values.yamlRepository: red-hat-data-services/agentic-starter-kits
Length of output: 4424
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- summarize route definition ---'
sed -n '560,620p' agents/langgraph/templates/ci_failure_summarizer/main.py | cat -n
echo
echo '--- auth-related symbols in main.py ---'
rg -n -C2 'Depends|AUTH_ENABLED|Authorization|api key|middleware|auth' agents/langgraph/templates/ci_failure_summarizer/main.pyRepository: red-hat-data-services/agentic-starter-kits
Length of output: 3861
Add auth or a default-deny boundary before exposing /summarize. The chart enables the OpenShift Route by default, auth.enabled is false, and /summarize remains unauthenticated; anyone reaching the route can trigger GitHub ingestion and Slack posts and read the triage response. README text alone does not make this operator-only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agents/langgraph/templates/ci_failure_summarizer/README.md` around lines 31 -
33, Protect the /summarize endpoint before exposing it through the
default-enabled OpenShift Route: add authentication or an enforced default-deny
network boundary, and ensure unauthenticated requests cannot trigger GitHub
ingestion, post to Slack, or read responses. Update the relevant route and auth
configuration associated with the template agent rather than relying only on
README guidance.
Source: Path instructions
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fix the ci_failure_summarizer template issues called out on PR #272 by making the local CLI example type-checkable, guarding nullable summary inserts, hardening image path serving, and cleaning the remaining template lint violations. Co-authored-by: Cursor <cursoragent@cursor.com>
Format the ci_failure_summarizer files that were still failing the PR #272 lint workflow while keeping the earlier lint, type-check, and CodeQL fixes intact. Co-authored-by: Cursor <cursoragent@cursor.com>
6648bd0 to
54fdfa0
Compare
…template Establish the LangGraph CI failure summarizer spike agent with renamed package metadata and GitHub/Slack env surface, preserving the standard API and deployment flow before Task 2 ingest logic. Co-authored-by: Cursor <cursoragent@cursor.com>
…ld naming Pass GITHUB_REPOSITORY, GITHUB_WORKFLOW, and SLACK_WEBHOOK_URL through Makefile deploy/dry-run Helm --set flags and align docstrings/OpenAPI/README with the new agent identity. Co-authored-by: Cursor <cursoragent@cursor.com>
…mary flow Add GitHub Actions ingest with log degradation, deterministic incident fingerprinting, PostgreSQL incident store, LLM triage summaries, and manual trigger via POST /summarize plus an example script. Co-authored-by: Cursor <cursoragent@cursor.com>
Move blocking orchestration off the event loop, record summary history when Slack delivery fails, and degrade to metadata-only summaries on LLM errors. Co-authored-by: Cursor <cursoragent@cursor.com>
Document honest spike scope, ci-testing deployment flow, and post-QG4 trigger examples; add incident-store and /summarize route tests for local confidence. Co-authored-by: Cursor <cursoragent@cursor.com>
Make Slack/GitHub env vars optional in agent.yaml, forward GITHUB_TOKEN and GITHUB_WORKFLOW_FILE through Helm deploy/dry-run, dedupe dry-run config via without_slack_posting(), and distinguish GitHub rate-limit 403s from admin-access log degradation. Co-authored-by: Cursor <cursoragent@cursor.com>
Correct workflow path normalization, validate explicit run_id against the configured workflow, surface actionable metadata API errors, and align integration deploy env with agent.yaml. Co-authored-by: Cursor <cursoragent@cursor.com>
Prevent webhook URL leakage in Slack errors, canonicalize workflow paths, skip occurrence_count bumps on same run_id re-runs, and record non-failed summary history for audit consistency. Co-authored-by: Cursor <cursoragent@cursor.com>
Add root README entry, remove misleading inherited search tool, and document unauthenticated manual /summarize and IncidentStore connection-per-call trade-offs. Co-authored-by: Cursor <cursoragent@cursor.com>
Share one typed evidence path across extraction, persistence, and rendering so the summarizer stays simpler as more deterministic rules are added. Add an OpenShift CronJob example and docs for the 11 AM IST /summarize trigger path used in ci-testing. Co-authored-by: Cursor <cursoragent@cursor.com>
Tighten the example runner imports and summary evidence refactor so the ci_failure_summarizer template passes the PR lint and type-check workflows. Keep the deterministic summary and CronJob changes intact while making the template CI-clean. Co-authored-by: Cursor <cursoragent@cursor.com>
Fix the ci_failure_summarizer template issues called out on PR #272 by making the local CLI example type-checkable, guarding nullable summary inserts, hardening image path serving, and cleaning the remaining template lint violations. Co-authored-by: Cursor <cursoragent@cursor.com>
Format the ci_failure_summarizer files that were still failing the PR #272 lint workflow while keeping the earlier lint, type-check, and CodeQL fixes intact. Co-authored-by: Cursor <cursoragent@cursor.com>
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>
Address the still-valid CodeRabbit findings on the synced PR branch by enforcing MODEL_ID at startup, pointing the playground UI at the Flask /api routes, serializing deploy secrets safely, and replacing the Slack duplicate-post read/write check with an atomic claim flow. Co-authored-by: Cursor <cursoragent@cursor.com>
Preserve chat context in completions and avoid deploy or persistence regressions from redundant saver setup, invalid deploy script syntax, or unbounded DB connects. Co-authored-by: Cursor <cursoragent@cursor.com>
Fixed 3 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Co-authored-by: Cursor <cursoragent@cursor.com>
7c3c11f to
1929b91
Compare
| def list_jobs(self, run_id: int, *, per_page: int = 100) -> list[WorkflowJob]: | ||
| path = f"/repos/{self.repository}/actions/runs/{run_id}/jobs" | ||
| # Spike assumption: QG4 runs have <=100 jobs; no Link pagination yet. | ||
| payload = self._request("GET", path, params={"per_page": per_page}).json() |
There was a problem hiding this comment.
nit: per_page=100 with no pagination — if the job count ever exceeds 100, results are silently truncated (no error, no warning). Not urgent today (~15 agents), but worth adding a log warning when the response count equals per_page so truncation doesn't go unnoticed as the repo grows.
Assisted by Claude Opus 4.6 (1M context)
There was a problem hiding this comment.
good point, I will add that suggestion.
| def get_latest_run(self, workflow_file: str) -> WorkflowRun | None: | ||
| workflow_ref = workflow_api_ref(workflow_file) | ||
| path = f"/repos/{self.repository}/actions/workflows/{workflow_ref}/runs" | ||
| payload = self._request("GET", path, params={"per_page": 1}).json() |
There was a problem hiding this comment.
nit: get_latest_run fetches only the single most recent workflow run (per_page: 1). Currently we have one nightly QG4 run so this is fine, but if we ever add more frequent runs (or re-runs), any runs between CronJob ticks would be skipped silently.
This is mainly a limitation of the CronJob-based trigger model — it assumes exactly one run happened since the last tick. A trigger-based approach (e.g., a post-failure step in the QG4 workflow itself calling /summarize?run_id=$GITHUB_RUN_ID) would eliminate this entirely and also resolve the schedule timing issue in the CronJob example.
Assisted by Claude Opus 4.6 (1M context)
There was a problem hiding this comment.
that is correct and your intuition is right about having this as a push model instead of pull would be ideal and having a step at the end of each of the workflow would make this more scalable. However the look-up logic to see all the jobs will still remain as individual /summarize calls would only make this execute and provide for each failure run separately but won't be a catch all job. I would leave this out of scope for now and we can add a new ticket, WDYT ?
Description
Add a new
langgraphspike template,ci_failure_summarizer, for daily QG4 failure triage on OpenShift.This PR:
agents/langgraph/templates/ci_failure_summarizer/summarizeoutput deterministically (no LLM used in the summarization path)/summarizedaily at 11 AM ISTci-testing, including authenticated log access, Slack posting, and manual CronJob smoke executionMotivation:
Jira Ticket
RHAIENG-6286Testing
make testpasses (run from the affected agent directory)Additional verification:
make testinagents/langgraph/templates/ci_failure_summarizer->61 passedci-testingand verifiedGET /healthPOST /summarizeagainst QG4 run29716559471examples/trigger_daily_summary_cronjob.yaml/summarizeexecutionChecklist
.envor secret files are included in this PRNew Agent Proposal
TBD / not run yetTBDReview Guidance
Please focus on:
src/ci_failure_summarizer/failure_evidence.pyfor the shared typed evidence modelsrc/ci_failure_summarizer/github_client.pyfor failed-step log extractionsrc/ci_failure_summarizer/summary_composer.pyfor deterministic summary renderingsrc/ci_failure_summarizer/orchestrator.pyfor persistence + summary-history wiringexamples/trigger_daily_summary_cronjob.yamlas an example operator-managed scheduler, not a Helm-managed resourceDesign notes:
/summarizeis intentionally deterministic in this PR; the inherited/chat/completionspath remains from the base template, but CI triage does not depend on an LLMtimeZone: Asia/KolkataRelated PRs
None