Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/231da9f0-236b-45e7-9f52-eeb3b0891152 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Hey A few things to address before this is ready to merge:
If you'd like a hand finishing up, you can use this prompt:
|
|
@copilot ci-coach must take into account ci.yml, cgo.yml and cjs.yml as those actions have been split |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9b3acc7d-c594-429e-abcf-8a33a2640408 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Updated. ci-coach now explicitly covers Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot merge main and recompile ci-coach workflow |
There was a problem hiding this comment.
Pull request overview
Reduces ci-coach token usage by shrinking prompt/tool surface area and front-loading deterministic CI run summarization, with updated guidance for split CI workflows.
Changes:
- Narrowed GitHub toolsets and enabled
cli-proxyto reduce prompt/tool schema overhead. - Updated shared CI data collection to aggregate runs across
ci.yml,cgo.yml, andcjs.yml, and generate/tmp/ci-summary.jsonfor summary-first analysis. - Simplified/trimmed optimization guidance and PR template scaffold; recompiled
ci-coach.lock.yml.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/shared/ci-optimization-strategies.md |
Updates analysis guidance to account for split CI workflow files and trims prompt surface. |
.github/workflows/shared/ci-data-analysis.md |
Aggregates run metadata across split workflows, reduces run volume, and adds summary JSON generation for deterministic pre-analysis. |
.github/workflows/ci-coach.md |
Narrows toolsets, adds early-exit gate, updates guidance to use CI summary first, and updates PR scaffold. |
.github/workflows/ci-coach.lock.yml |
Recompiled lockfile reflecting the markdown/frontmatter/import/tooling changes (incl. cli-proxy). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 4
| status_counts: (group_by(.status) | map({status: .[0].status, count: length})), | ||
| conclusion_counts: (map(select(.conclusion != null)) | group_by(.conclusion) | map({conclusion: .[0].conclusion, count: length})), | ||
| branch_counts: (group_by(.headBranch) | map({branch: .[0].headBranch, count: length}) | sort_by(-.count) | .[0:10]), |
There was a problem hiding this comment.
The CI summary uses group_by(...) on .status, .conclusion, and .headBranch without sorting first. In jq, group_by only groups adjacent equal keys, so counts can be incorrect unless the array is pre-sorted by the same key. Sort the array (e.g., sort_by(.status) | group_by(.status)) or use an alternate counting approach.
| status_counts: (group_by(.status) | map({status: .[0].status, count: length})), | |
| conclusion_counts: (map(select(.conclusion != null)) | group_by(.conclusion) | map({conclusion: .[0].conclusion, count: length})), | |
| branch_counts: (group_by(.headBranch) | map({branch: .[0].headBranch, count: length}) | sort_by(-.count) | .[0:10]), | |
| status_counts: (sort_by(.status) | group_by(.status) | map({status: .[0].status, count: length})), | |
| conclusion_counts: (map(select(.conclusion != null)) | sort_by(.conclusion) | group_by(.conclusion) | map({conclusion: .[0].conclusion, count: length})), | |
| branch_counts: (sort_by(.headBranch) | group_by(.headBranch) | map({branch: .[0].headBranch, count: length}) | sort_by(-.count) | .[0:10]), |
| # Download workflow runs for split CI workflows (ci, cgo, cjs) | ||
| gh run list --repo "$GITHUB_REPOSITORY" --workflow=ci.yml --limit 30 --json databaseId,status,conclusion,createdAt,updatedAt,displayTitle,headBranch,event,url,workflowDatabaseId,number > /tmp/ci-runs-ci.json | ||
| gh run list --repo "$GITHUB_REPOSITORY" --workflow=cgo.yml --limit 30 --json databaseId,status,conclusion,createdAt,updatedAt,displayTitle,headBranch,event,url,workflowDatabaseId,number > /tmp/ci-runs-cgo.json | ||
| gh run list --repo "$GITHUB_REPOSITORY" --workflow=cjs.yml --limit 30 --json databaseId,status,conclusion,createdAt,updatedAt,displayTitle,headBranch,event,url,workflowDatabaseId,number > /tmp/ci-runs-cjs.json | ||
| jq -s 'add | sort_by(.createdAt) | reverse | .[0:60]' /tmp/ci-runs-ci.json /tmp/ci-runs-cgo.json /tmp/ci-runs-cjs.json > /tmp/ci-runs.json |
There was a problem hiding this comment.
These gh run list calls are labeled as “last 7 days”, but they only limit by count and don’t apply any date filter. If the intent is a time-bounded sample (and to keep prompt size predictable), consider adding a --created filter (or update the wording if count-based sampling is intended).
| 5. **Test Results**: `/tmp/gh-aw/test-results.json` - Test performance data | ||
| 6. **Fuzz Results**: `/tmp/ci-artifacts/*/fuzz-results/` - Fuzz test output and corpus data | ||
| 1. **CI Runs**: `/tmp/ci-runs.json` - Last 60 workflow runs | ||
| 2. **CI Summary**: `/tmp/ci-summary.json` - Pre-computed failure patterns, duration stats, and top opportunities |
There was a problem hiding this comment.
/tmp/ci-summary.json currently contains run counts, branch distribution, average duration, and recent failures, but it doesn’t compute “top opportunities”. To avoid misleading guidance, update this description to match the fields actually produced by the summary step.
| 2. **CI Summary**: `/tmp/ci-summary.json` - Pre-computed failure patterns, duration stats, and top opportunities | |
| 2. **CI Summary**: `/tmp/ci-summary.json` - Pre-computed run counts, branch distribution, average duration, and recent failures |
| gh run list --repo "$GITHUB_REPOSITORY" --workflow=ci.yml --status success --limit 2 --json databaseId | ||
| gh run list --repo "$GITHUB_REPOSITORY" --workflow=cgo.yml --status success --limit 2 --json databaseId | ||
| gh run list --repo "$GITHUB_REPOSITORY" --workflow=cjs.yml --status success --limit 2 --json databaseId | ||
| } | jq -s 'add | .[].databaseId' -r | while read -r run_id; do |
There was a problem hiding this comment.
The artifact download pipeline calls jq with -r after the filter (jq -s '...' -r). In jq, options must come before the filter; as written, -r is treated like a filename and will likely cause the step to fail (or produce no IDs), preventing artifact downloads.
| } | jq -s 'add | .[].databaseId' -r | while read -r run_id; do | |
| } | jq -r -s 'add | .[].databaseId' | while read -r run_id; do |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Done. I merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
ci-coachwas running at ~2.8M tokens/run (4.7x target), driven by large prompt payloads, broad tool schema overhead, and iterative data gathering. This PR reduces recurring context size and constrains analysis flow so the agent can exit earlier when no actionable optimization exists.Prompt + toolset minimization
defaultto onlyissuesandpull_requestscli-proxyin featuresDeterministic pre-analysis in shared import
shared/ci-data-analysis.mdto build/tmp/ci-summary.jsonfrom run data (status/conclusion counts, avg duration, recent failures)Split CI workflow coverage (feedback update)
ci-coachanalysis guidance to explicitly account for.github/workflows/ci.yml,.github/workflows/cgo.yml, and.github/workflows/cjs.yml/tmp/ci-runs.jsonand/tmp/ci-summary.jsonci.yml,cgo.yml, andcjs.ymlExecution discipline / early-stop behavior
ci-coach.md:Compiled workflow update
ci-coach.lock.ymlto reflect markdown/frontmatter/import changes> [!WARNING]
>
>