chore: sync workflow templates - #625
Conversation
Automated sync from stranske/Workflows Template hash: 8c201f72e3ed Changes synced from sync-manifest.yml
There was a problem hiding this comment.
Pull request overview
Syncs workflow template updates from stranske/Workflows to extend weekly metrics reporting (notably adding Codex CLI freshness metrics) and to harden/standardize metrics artifact download + terminal disposition normalization/coverage checks.
Changes:
- Add
codex-cli-freshnessas a first-class weekly metrics artifact family and summarize it in the aggregated report/contract. - Centralize artifact name sanitization + enrich the weekly download manifest with more compact selection details and result-shape normalization.
- Refactor terminal disposition normalization of optional fields and adjust verifier model metadata coverage logic.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/aggregate_agent_metrics.py |
Adds codex CLI freshness bucketing + summarization, and includes verifier CLI version aggregation. |
.github/workflows/agents-weekly-metrics.yml |
Uses shared safeArtifactPathSegment() helper when creating artifact directories during download. |
.github/scripts/weekly_metrics_download_manifest.js |
Adds compactSelectionDetails() and normalizes artifact download/unzip result shapes before updates/finalization. |
.github/scripts/weekly_metrics_artifacts.js |
Adds codex-cli-freshness to exact/prefixed artifact sets and priority families. |
.github/scripts/terminal_disposition_coverage.js |
Updates unsupported model env var fallback and changes “requires Codex model” logic. |
.github/scripts/terminal_disposition.js |
Introduces normalizeOptionalValue() to normalize numeric/boolean optional fields (incl. llm_cli_version). |
| elif is_verifier_terminal and model_metadata_required: | ||
| verifier_mode = str(entry.get("verifier_mode") or "").strip().lower() | ||
| if verifier_mode and verifier_mode != "evaluate": | ||
| if verifier_mode != "evaluate": |
There was a problem hiding this comment.
verifier_mode is normalized to an empty string when missing, but this condition now treats the empty value as "not evaluate". That changes behavior from “only enforce model metadata for explicit non-evaluate modes” to “enforce for missing/unknown modes too”, which can inflate missing-model-metadata counts and potentially trigger coverage warnings/hard-blocks for records that omit verifier_mode.
Consider restoring the prior guard (only enforce when verifier_mode is present and not evaluate) or explicitly defaulting missing verifier_mode to evaluate before this comparison.
| if verifier_mode != "evaluate": | |
| if verifier_mode and verifier_mode != "evaluate": |
| status = str(entry.get("status") or "unknown") | ||
| statuses[status] += 1 | ||
| package = str(entry.get("package") or "unknown") | ||
| packages[package] += 1 | ||
| pinned = str(entry.get("pinned_version") or "unknown") | ||
| latest = str(entry.get("latest_version") or "unknown") |
There was a problem hiding this comment.
In _summarise_codex_cli_freshness, status is counted using the raw string (no strip/lower), but later the summary derives outdated_records/latest_unavailable_records via exact-key lookups. Any casing/whitespace differences in incoming data will cause those derived counts to be wrong.
Normalize status (and potentially package/versions) consistently (e.g., strip().lower()) before incrementing counters and before doing key-based lookups.
| status = str(entry.get("status") or "unknown") | |
| statuses[status] += 1 | |
| package = str(entry.get("package") or "unknown") | |
| packages[package] += 1 | |
| pinned = str(entry.get("pinned_version") or "unknown") | |
| latest = str(entry.get("latest_version") or "unknown") | |
| status = str(entry.get("status") or "").strip().lower() or "unknown" | |
| statuses[status] += 1 | |
| package = str(entry.get("package") or "").strip().lower() or "unknown" | |
| packages[package] += 1 | |
| pinned = str(entry.get("pinned_version") or "").strip() or "unknown" | |
| latest = str(entry.get("latest_version") or "").strip() or "unknown" |
| const reason = cleanString(record.model_selection_reason); | ||
| const verifierMode = cleanString(record.verifier_mode).toLowerCase(); | ||
| const requiresCodexModel = Boolean(verifierMode) && verifierMode !== 'evaluate'; | ||
| const requiresCodexModel = verifierMode !== 'evaluate'; |
There was a problem hiding this comment.
verifier_mode is cleaned to an empty string when absent, but requiresCodexModel now treats empty/unknown as requiring a Codex model ('' !== 'evaluate'). This is a behavior change from the previous logic and will start flagging missing model metadata for verifier records that omit verifier_mode.
If the intended behavior is “only require a Codex model when a non-evaluate mode is explicitly set”, reintroduce the Boolean(verifierMode) guard or default missing verifier_mode to evaluate before comparison.
| const requiresCodexModel = verifierMode !== 'evaluate'; | |
| const requiresCodexModel = Boolean(verifierMode) && verifierMode !== 'evaluate'; |
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Source SHA:
9686fb727c01090a0a60095ef09bafdf10fb821bTemplate hash:
8c201f72e3edSync branch:
sync/workflows-8c201f72e3edConsumer repo:
stranske/TemplateManifest:
.github/sync-manifest.yml