Conversation
Automated sync from stranske/Workflows Template hash: daec38e5dbf4 Changes synced from sync-manifest.yml
🤖 Keepalive Loop StatusPR #920 | Agent: Codex | Iteration 0/5 Current State
🔍 Failure Classification| Error type | infrastructure | |
Keepalive Work Log (click to expand)
|
There was a problem hiding this comment.
Pull request overview
Syncs workflow templates from stranske/Workflows, expanding the weekly metrics pipeline and related telemetry/coverage tooling to produce richer, machine-readable contracts and more detailed operational diagnostics.
Changes:
- Add a weekly artifact download manifest (JSON + markdown) and publish it alongside weekly metrics artifacts.
- Extend
aggregate_agent_metrics.pyto enrich records with artifact/source metadata, capture structured parse errors, and emit a JSON summary contract. - Enhance terminal disposition + bot-comment auth coverage reporting (priority-family status reporting, verifier model compatibility signals, wrapper terminal disposition artifact emission).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/aggregate_agent_metrics.py |
Enriches ingested metrics with artifact metadata, tracks parse error details, and writes a JSON summary contract (plus new verifier model stats). |
.github/workflows/agents-weekly-metrics.yml |
Adds download-manifest generation/recording and uploads additional JSON/markdown artifacts (including the new metrics JSON summary). |
.github/workflows/agents-bot-comment-handler.yml |
Emits a wrapper “review-thread terminal disposition” artifact and exposes a skip_reason output. |
.github/workflows/agents-81-gate-followups.yml |
Switches jq invocation to compact/no-input mode (jq -cn). |
.github/scripts/weekly_metrics_download_manifest.js |
New helper to initialize, record, and finalize weekly artifact download/unzip manifest + markdown rendering. |
.github/scripts/weekly_metrics_artifacts.js |
Adds priority-family missing/status reporting to the artifact selection contract and markdown output. |
.github/scripts/terminal_disposition_coverage.js |
Adds verifier model compatibility checks and richer artifact-selection normalization for terminal coverage reporting. |
.github/scripts/terminal_disposition.js |
Extends normalized terminal disposition records to include llm_model, model_selection_reason, and verifier_mode. |
.github/scripts/coverage_monitor_summary.js |
New weekly “coverage monitor” contract summarizing terminal + bot-auth monitor states. |
.github/scripts/bot_comment_auth_coverage.js |
Tracks skipped/missing organic requirements and adds wrapper-aware logic for reusable invocation expectations. |
.github/scripts/agents_pr_meta_update_body.js |
Adds issue metadata marker + “Closes #…” line to PR preamble when a source issue is present. |
| stats: { | ||
| selected_count: selected.length, | ||
| download_pass_count: 0, | ||
| download_failed_count: 0, | ||
| unzip_pass_count: 0, | ||
| unzip_failed_count: 0, | ||
| unzip_skipped_count: selected.length, | ||
| }, | ||
| artifacts: selected.map((artifact, index) => ({ | ||
| id: artifact.id, | ||
| name: cleanString(artifact.name), | ||
| family: cleanString(artifact.family), | ||
| created_at: cleanString(artifact.created_at), | ||
| updated_at: cleanString(artifact.updated_at), | ||
| selected_index: index, | ||
| artifact_dir: defaultArtifactDir(artifactsRoot, artifact), | ||
| zip_path: defaultZipPath(artifactsRoot, artifact), | ||
| download: { | ||
| status: 'pending', | ||
| bytes: null, | ||
| error: '', | ||
| }, | ||
| unzip: { | ||
| status: 'pending', | ||
| path: defaultArtifactDir(artifactsRoot, artifact), | ||
| error: '', | ||
| }, | ||
| })), |
There was a problem hiding this comment.
buildInitialManifest() initializes stats.unzip_skipped_count to selected.length while each artifact’s unzip.status is set to pending. This makes the manifest/markdown report claim unzips were “skipped” before any download work happens, and then finalizeManifest() recomputes a different value. Consider either tracking a *_pending_count in stats, or initializing unzip_skipped_count to 0 (and/or setting initial unzip.status to skipped) so the stats align with the per-artifact statuses.
| verifier_mode = entry.get("verifier_mode") | ||
| if verifier_mode: | ||
| verifier_modes[str(verifier_mode)] += 1 |
There was a problem hiding this comment.
verifier_modes is counted using the raw entry.get("verifier_mode") value, but earlier logic normalizes verifier modes with .strip().lower() (e.g., when deciding whether model metadata is required). This can split the same mode across multiple keys ("Evaluate" vs "evaluate") and make the summary inconsistent. Consider normalizing verifier_mode (trim/lowercase) before incrementing verifier_modes.
| verifier_mode = entry.get("verifier_mode") | |
| if verifier_mode: | |
| verifier_modes[str(verifier_mode)] += 1 | |
| verifier_mode = str(entry.get("verifier_mode") or "").strip().lower() | |
| if verifier_mode: | |
| verifier_modes[verifier_mode] += 1 |
| const model = cleanString(record.llm_model ?? record.model).toLowerCase(); | ||
| const reason = cleanString(record.model_selection_reason); | ||
| const verifierMode = cleanString(record.verifier_mode).toLowerCase(); | ||
| const requiresCodexModel = verifierMode !== 'evaluate'; | ||
| if (model) selectedModels[model] = (selectedModels[model] || 0) + 1; | ||
| if (reason) modelSelectionReasons[reason] = (modelSelectionReasons[reason] || 0) + 1; | ||
| if (!model && requiresCodexModel) { | ||
| const runId = cleanString(record.run_id); |
There was a problem hiding this comment.
In summarizeVerifierModelCompatibility, requiresCodexModel is computed as verifierMode !== 'evaluate'. When record.verifier_mode is missing/empty, this evaluates to true, so every verifier terminal disposition record without verifier_mode will be flagged as missing model metadata (even if the mode is effectively “evaluate” or otherwise unknown). Consider treating an empty verifier_mode as “unknown” and not applying the missing-model rule until the mode is present (or explicitly defaulting missing mode to evaluate if that’s the intended baseline).
|
pd-workloop-resume status: kept this as the canonical latest Workflows sync PR and closed the remaining superseded sync PRs (#911-#919; earlier duplicates were already closed, and #906 was already merged). Gate is green, but I am leaving this PR unmerged because Copilot has three actionable inline comments on synced Workflows-owned files. Per consumer repo policy, those should be fixed in stranske/Workflows first, then synced here. |
|
Automation cleanup update (pd-workloop-resume): deduplicated the superseded Workflows sync PR stack and left #920 as the canonical latest sync PR (template hash daec38e5dbf4).\n\nCurrent blocker: #920 still has 3 unresolved Copilot inline comments on Workflows-synced files (.github/scripts/weekly_metrics_download_manifest.js, scripts/aggregate_agent_metrics.py, and .github/scripts/terminal_disposition_coverage.js). Per this consumer repo's AGENTS.md source-of-truth policy, those fixes should land in stranske/Workflows first and then be re-synced here; I am not merging this consumer sync PR while those review threads remain unresolved. |
|
Automation update: the three unresolved Copilot threads on this consumer sync PR are on Workflows-owned synced files, so I opened the source-of-truth fix in stranske/Workflows#1883. This PR should not be duplicated or patched directly in the consumer repo; next step is to merge the Workflows source fix, let the consumer sync regenerate, then replace/advance this sync lane. |
|
Superseded by the latest Workflows sync lane #938; closing this older automated sync PR so review and CI focus on the canonical branch. |
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Manifest:
.github/sync-manifest.yml