Skip to content

fix: address refreshed sync review blockers #1945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stranske merged 1 commit into from
Apr 27, 2026
Merged

fix: address refreshed sync review blockers #1945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions .github/scripts/__tests__/terminal-disposition-coverage.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ test('does not require verifier model metadata when mode is unknown', () => {
assert.deepEqual(report.enforcement.blockers, []);
});

test('requires verifier model metadata for post-contract records with unknown mode', () => {
test('does not require verifier model metadata for post-contract records with unknown mode', () => {
const report = summarizeTerminalDispositionCoverage(
[
{
Expand All @@ -394,12 +394,11 @@ test('requires verifier model metadata for post-contract records with unknown mo
);
const markdown = formatTerminalDispositionCoverageMarkdown(report);

assert.equal(report.status, 'warning');
assert.equal(report.verifier_model_compatibility.missing_model_record_count, 1);
assert.equal(report.verifier_model_compatibility.missing_model_unknown_mode_record_count, 1);
assert.deepEqual(report.enforcement.blockers, ['missing-verifier-model-metadata']);
assert.equal(report.verifier_model_compatibility.missing_model_records[0].verifier_mode, 'unknown');
assert.match(markdown, /Missing verifier model metadata records with unknown mode: 1/);
assert.equal(report.status, 'pass');
assert.equal(report.verifier_model_compatibility.missing_model_record_count, 0);
assert.equal(report.verifier_model_compatibility.missing_model_unknown_mode_record_count, 0);
assert.deepEqual(report.enforcement.blockers, []);
assert.match(markdown, /Missing verifier model metadata records with unknown mode: 0/);
});

test('suppresses pre-contract verifier terminal records missing model metadata', () => {
Expand Down
2 changes: 1 addition & 1 deletion .github/scripts/terminal_disposition_coverage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ function summarizeVerifierModelCompatibility(records = [], options = {}) {
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';
const requiresCodexModel = Boolean(verifierMode) && verifierMode !== 'evaluate';
if (model) selectedModels[model] = (selectedModels[model] || 0) + 1;
Comment on lines 176 to 180

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing_model_unknown_mode_record_count can no longer be incremented after changing requiresCodexModel to Boolean(verifierMode) && ...: the only increment site is guarded by if (!model && requiresCodexModel ...), so !verifierMode is impossible. This makes the reported/markdowned "Missing verifier model metadata records with unknown mode" value always 0 (and leaves dead code). Consider separating "is verifier mode known" from "should enforce missing model metadata" so you can still count unknown-mode records (or remove the unused counter/markdown line entirely).

Copilot uses AI. Check for mistakes.
if (reason) modelSelectionReasons[reason] = (modelSelectionReasons[reason] || 0) + 1;
if (!model && requiresCodexModel && modelMetadataContract.model_metadata_required) {
Expand Down
3 changes: 3 additions & 0 deletions docs/LABELS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ This document describes all labels that trigger automated workflows or affect CI
| `agents:formatted` | Auto-applied | Indicates issue has been formatted |
| `agents:optimize` | Issue labeled | Analyzes issue and posts suggestions |
| `agents:apply-suggestions` | Issue labeled | Applies optimization suggestions |
| `agents:auto-pilot` | Issue labeled | Runs issue-to-PR automation |
| `agents:auto-pilot-pause` | Issue labeled | Pauses auto-pilot dispatch |
| `agents:paused` | PR labeled | Pauses keepalive loop on PR |
| `agents:keepalive` | PR labeled | Enables keepalive loop on PR |
| `runner:<agent>` | Issue labeled | Selects an auto-pilot runner without triggering issue intake |
| `verify:checkbox` | PR labeled | Runs verifier checkbox mode after merge |
| `verify:evaluate` | PR labeled | Runs verifier evaluation mode after merge |
| `verify:compare` | PR labeled | Runs verifier comparison mode after merge |
Expand Down
1 change: 1 addition & 0 deletions scripts/aggregate_agent_metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"agents-verifier-metrics",
"agents-verifier-disposition-metrics",
"codex-cli-freshness",
"pr-source-context",
}
_PREFIXED_ARTIFACT_FAMILIES = (
"autopilot-metrics-",
Expand Down
2 changes: 1 addition & 1 deletion templates/consumer-repo/.github/scripts/terminal_disposition_coverage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ function summarizeVerifierModelCompatibility(records = [], options = {}) {
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';
const requiresCodexModel = Boolean(verifierMode) && verifierMode !== 'evaluate';
if (model) selectedModels[model] = (selectedModels[model] || 0) + 1;
Comment on lines 176 to 180

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing_model_unknown_mode_record_count can no longer be incremented after changing requiresCodexModel to Boolean(verifierMode) && ...: the only increment site is guarded by if (!model && requiresCodexModel ...), so !verifierMode is impossible. This makes the reported/markdowned "Missing verifier model metadata records with unknown mode" value always 0 (and leaves dead code). Consider separating "is verifier mode known" from "should enforce missing model metadata" so you can still count unknown-mode records (or remove the unused counter/markdown line entirely).

Copilot uses AI. Check for mistakes.
if (reason) modelSelectionReasons[reason] = (modelSelectionReasons[reason] || 0) + 1;
if (!model && requiresCodexModel && modelMetadataContract.model_metadata_required) {
Expand Down
6 changes: 3 additions & 3 deletions templates/consumer-repo/WORKFLOW_USER_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Issue: "Add user authentication"
4. Use a `workflow:source-*` label if you need to classify an existing PR from the PR list

**What Happens:**
- Workflows treats valid non-issue sources as intentional work instead of missing process state
- Workflows treat valid non-issue sources as intentional work instead of missing process state
- PR metadata automation avoids repeated missing-source repair comments
- Keepalive can continue without a linked issue when the PR has a valid non-issue workflow source context

Expand Down Expand Up @@ -138,7 +138,7 @@ Issue: "Add user authentication"
### User-Applied Labels (Triggers)

| Label | Where | What It Does |
|-------|-------|--------------||
|-------|-------|--------------|
| `autofix` | PR | Auto-fixes formatting/lint errors |
| `autofix:clean` | PR | Aggressive autofix with cosmetic repairs |
| `agent:codex` | Issue | Creates PR from issue |
Expand All @@ -159,7 +159,7 @@ Issue: "Add user authentication"
### Auto-Applied Labels (Status)

| Label | Meaning |
|-------|---------||
|-------|---------|
| `agents:formatted` | Issue has been formatted |
| `agents:keepalive` | Keepalive is monitoring this PR |
| `agent:needs-attention` | Human intervention required |
Expand Down
1 change: 1 addition & 0 deletions templates/consumer-repo/scripts/aggregate_agent_metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"agents-verifier-metrics",
"agents-verifier-disposition-metrics",
"codex-cli-freshness",
"pr-source-context",
}
_PREFIXED_ARTIFACT_FAMILIES = (
"autopilot-metrics-",
Expand Down
4 changes: 4 additions & 0 deletions tests/scripts/test_aggregate_agent_metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,10 @@ def test_classify_entry_prefers_explicit_type() -> None:
assert aggregate_agent_metrics._classify_entry({"other": "value"}) == "unknown"


def test_artifact_family_classifies_pr_source_context() -> None:
assert aggregate_agent_metrics._artifact_family("pr-source-context") == "pr-source-context"


def test_safe_number_helpers() -> None:
assert aggregate_agent_metrics._safe_int("3") == 3
assert aggregate_agent_metrics._safe_int("bad") is None
Expand Down
Loading