Skip to content

fix: address sync review source context regressions - #1957

Merged
stranske merged 2 commits into
mainfrom
codex/fix-sync-review-comments-pr342
Apr 27, 2026
Merged

fix: address sync review source context regressions#1957
stranske merged 2 commits into
mainfrom
codex/fix-sync-review-comments-pr342

Conversation

@stranske

@stranske stranske commented Apr 27, 2026

Copy link
Copy Markdown
Owner

Source: Issue #1836

Related to campaign issue #1836

Automated Status Summary

Scope

Sync/Dependabot Campaign Queue

Remote GitHub Actions owns discovery for sync-generated and Dependabot PR rounds. Local Codex should only claim items from this issue when needs-local-codex work is queued.

Summary

Context for Agent

Related Issues/PRs

Tasks

  • Updated: 2026-04-27T07:14:52.962Z
  • Current sync hash: 76ebdcce08bb
  • Repos checked: 11/11
  • Open sync PRs: 732
  • Open Dependabot PRs: 0
  • Active review threads queued: 1342
  • Items needing local Codex: 1
  • Actionable local Codex items: 1
  • Claimable local Codex items: 1
  • Source-fixed candidates: 0
  • Superseded sync candidates: 119
  • Source sync states: current=1, superseded=119
  • Finished local results without published source changes: 0
  • Claimed local Codex items: 0
  • Next claim lease expires: -

Acceptance criteria

  • Kind: sync-review-comments
  • Source repo: stranske/Workflows
  • Preferred local workdir: Workflows
  • Head: sync/workflows-76ebdcce08bb (cff20c3e7db3)
  • Source sync state: current (PR 76ebdcce08bb / current 76ebdcce08bb)
  • Attempts: 0

Head SHA: ef3879c
Latest Runs: ⏹️ cancelled — Gate
Required: gate: ⏹️ cancelled

Workflow / Job Result Logs
Agents Auto-Pilot ⏭️ skipped View run
Agents Bot Comment Handler ✅ success View run
Agents Keepalive Loop ⏹️ cancelled View run
Agents Verifier ✅ success View run
CI Autofix Loop ⏹️ cancelled View run
Create Issue from Verification (Enhanced) ⏭️ skipped View run
Create New PR from Verification ⏭️ skipped View run
Gate ⏹️ cancelled View run
Health 40 Sweep ✅ success View run
Health 44 Gate Branch Protection ❔ in progress View run
Health 45 Agents Guard ✅ success View run
Health 50 Security Scan ✅ success View run
Health 72 Template Sync ✅ success View run
Maint 52 Validate Workflows ✅ success View run
PR 11 - Minimal invariant CI ✅ success View run
Selftest CI ✅ success View run
Validate Sync Manifest ✅ success View run

Copilot AI review requested due to automatic review settings April 27, 2026 07:32
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@stranske-automation-bot

stranske-automation-bot commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

Automated Status Summary

Head SHA: aae9aaf
Latest Runs: ⏳ pending — Gate
Required contexts: Gate / gate, Health 45 Agents Guard / guard
Required: core tests (3.12): ⏳ pending, core tests (3.13): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job Result Logs
(no jobs reported) ⏳ pending

Coverage Overview

  • Coverage history entries: 1

Coverage Trend

Metric Value
Current 93.12%
Baseline 85.00%
Delta +8.12%
Minimum 70.00%
Status ✅ Pass

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
src/cli_parser.py 81.8% 4
src/percentile_calculator.py 95.0% 1
src/aggregator.py 95.0% 2
src/__init__.py 100.0% 0
src/ndjson_parser.py 100.0% 0

Updated automatically; will refresh on subsequent CI/Docker completions.


Keepalive checklist

Scope

Sync/Dependabot Campaign Queue

Remote GitHub Actions owns discovery for sync-generated and Dependabot PR rounds. Local Codex should only claim items from this issue when needs-local-codex work is queued.

Summary

Context for Agent

Related Issues/PRs

Tasks

  • Updated: 2026-04-27T07:14:52.962Z
  • Current sync hash: 76ebdcce08bb
  • Repos checked: 11/11
  • Open sync PRs: 732
  • Open Dependabot PRs: 0
  • Active review threads queued: 1342
  • Items needing local Codex: 1
  • Actionable local Codex items: 1
  • Claimable local Codex items: 1
  • Source-fixed candidates: 0
  • Superseded sync candidates: 119
  • Source sync states: current=1, superseded=119
  • Finished local results without published source changes: 0
  • Claimed local Codex items: 0
  • Next claim lease expires: -

Acceptance criteria

  • Kind: sync-review-comments
  • Source repo: stranske/Workflows
  • Preferred local workdir: Workflows
  • Head: sync/workflows-76ebdcce08bb (cff20c3e7db3)
  • Source sync state: current (PR 76ebdcce08bb / current 76ebdcce08bb)
  • Attempts: 0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes regressions in PR source-context and PR-body-sync behavior so legacy “human-only” opt-out wording is recognized again, and issue-number heuristics keep PRs on the issue-backed body-sync path even when a non-issue workflow-source marker exists. Also updates the consumer template copies for the touched scripts.

Changes:

  • Restore legacy “Human-only / Human only” detection as a no-automation signal in source context parsing.
  • Change non-issue body-sync gating to always yield to any detected issue number (including branch/title/body-inferred).
  • Update and adjust unit tests to cover the restored legacy wording and the new body-sync precedence behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
templates/consumer-repo/.github/scripts/source_context.js Extends no-automation checkbox detection to include legacy “human-only” wording in the consumer template copy.
templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js Makes explicit non-issue workflow-source markers stop skipping issue-backed body sync whenever an issue number is present (template copy).
.github/scripts/source_context.js Extends no-automation checkbox detection to include legacy “human-only” wording.
.github/scripts/agents_pr_meta_update_body.js Ensures explicit non-issue markers do not override issue-number-based body-sync behavior.
.github/scripts/__tests__/source-context.test.js Adds a regression test for legacy “Human-only” no-automation wording.
.github/scripts/__tests__/agents-pr-meta-update-body.test.js Updates expectations/tests to reflect the new precedence rules (issue number wins).

Comment thread .github/scripts/__tests__/agents-pr-meta-update-body.test.js
@stranske

Copy link
Copy Markdown
Owner Author

Closer update: reviewed the latest consumer sync review threads and updated this existing Workflows follow-up PR rather than opening a duplicate. The common blockers were the synced consumer label guide drifting from active labels, verifier-mode aggregation edge cases, and the existing body-sync positive test gap. Targeted validation passed locally: python -m pytest tests/scripts/test_aggregate_agent_metrics.py -q, node --test .github/scripts/__tests__/agents-pr-meta-update-body.test.js .github/scripts/__tests__/source-context.test.js, python scripts/validate_template_sync.py, and python scripts/validate_template_completeness.py --manifest .github/sync-manifest.yml --source sync-manifest.

@stranske
stranske merged commit aae9aaf into main Apr 27, 2026
102 of 110 checks passed
@stranske
stranske deleted the codex/fix-sync-review-comments-pr342 branch April 27, 2026 10:57
@stranske stranske added the verify:compare Compare multiple LLM evaluations label Apr 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Provider Comparison Report

Provider Summary

Provider Model Verdict Confidence Summary
openai gpt-5.4 PASS 95% The merged changes satisfy the stated acceptance criteria for the sync review source-context regressions. The code now correctly treats any resolved issueNumber as taking precedence over explicit n...
anthropic claude-sonnet-4-6 PASS 92% Both acceptance criteria are directly addressed: (1) resolveNonIssueWorkflowSourceContextForBodySync now returns null whenever issueNumber != null, correctly yielding to any branch-inferred iss...
📋 Full Provider Details (click to expand)

openai

  • Model: gpt-5.4
  • Verdict: PASS
  • Confidence: 95%
  • Scores:
    • Correctness: 9.0/10
    • Completeness: 9.0/10
    • Quality: 9.0/10
    • Testing: 9.0/10
    • Risks: 8.0/10
  • Summary: The merged changes satisfy the stated acceptance criteria for the sync review source-context regressions. The code now correctly treats any resolved issueNumber as taking precedence over explicit non-issue workflow-source markers in PR body sync, which addresses the review concern around branch-inferred issue numbers overriding non-issue context. It also restores legacy support for the 'human-only' / 'human only' no-automation wording in source-context parsing, addressing the second review concern. Both primary fixes are mirrored in template consumer-repo copies, improving consistency. Tests were updated to assert the new precedence behavior and to cover the legacy no-automation phrasing. Additional aggregate_agent_metrics/doc changes appear unrelated but are internally consistent and covered by tests where applicable. Overall, the implementation is small, readable, and aligned with the documented acceptance criteria, with only minor compatibility risk from the intentional behavior change that any non-null issueNumber now suppresses non-issue source context.

anthropic

  • Model: claude-sonnet-4-6
  • Verdict: PASS
  • Confidence: 92%
  • Scores:
    • Correctness: 9.0/10
    • Completeness: 9.0/10
    • Quality: 9.0/10
    • Testing: 9.0/10
    • Risks: 8.0/10
  • Summary: Both acceptance criteria are directly addressed: (1) resolveNonIssueWorkflowSourceContextForBodySync now returns null whenever issueNumber != null, correctly yielding to any branch-inferred issue number as the review comment requested; (2) NO_AUTOMATION_CHECKBOX_PATTERN is extended with |\bhuman[- ]only\b to restore support for the previously-dropped 'human-only' opt-out phrasing. Tests cover both changes explicitly. The implementation is clean, template copies are synchronized, and the bundled Python metrics improvements are well-tested. The main concern is that the issueNumber check is intentionally broader than a minimal fix, but this matches the stated intent of the review comment.
  • Concerns:
    • The change to if (issueNumber != null) is broader than strictly necessary — it suppresses non-issue source context whenever any issue number is present, even if that number doesn't match the explicit issue references in the PR body. This is intentional per the review comment but represents a more aggressive behavioral change than a minimal fix would require.
    • The _is_known_verifier_mode helper is removed without checking if it's used elsewhere in the codebase — the diff only shows its definition and the call site in _verifier_mode_requires_model_metadata, so this appears safe, but warrants confirmation.
    • Template copies (.github/scripts and scripts/) are kept in sync with source, which is correct, but the LABELS.md template diverges significantly from the source docs/LABELS.md in structure (table formatting changes, large content additions). This is likely intentional but increases maintenance surface.

Agreement

  • Verdict: PASS (all providers)
  • Correctness: scores within 1 point (avg 9.0/10, range 9.0-9.0)
  • Completeness: scores within 1 point (avg 9.0/10, range 9.0-9.0)
  • Quality: scores within 1 point (avg 9.0/10, range 9.0-9.0)
  • Testing: scores within 1 point (avg 9.0/10, range 9.0-9.0)
  • Risks: scores within 1 point (avg 8.0/10, range 8.0-8.0)

Disagreement

No major disagreements detected.

Unique Insights

  • openai: The merged changes satisfy the stated acceptance criteria for the sync review source-context regressions. The code now correctly treats any resolved issueNumber as taking precedence over explicit non-issue workflow-source markers in PR body sync, which addresses the review concern around branch-i...
  • anthropic: The change to if (issueNumber != null) is broader than strictly necessary — it suppresses non-issue source context whenever any issue number is present, even if that number doesn't match the explicit issue references in the PR body. This is intentional per the review comment but represents a more aggressive behavioral change than a minimal fix would require.; The _is_known_verifier_mode helper is removed without checking if it's used elsewhere in the codebase — the diff only shows its definition and the call site in _verifier_mode_requires_model_metadata, so this appears safe, but warrants confirmation.; Template copies (.github/scripts and scripts/) are kept in sync with source, which is correct, but the LABELS.md template diverges significantly from the source docs/LABELS.md in structure (table formatting changes, large content additions). This is likely intentional but increases maintenance surface.

🔍 LangSmith Traces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex codex-automation verify:compare Compare multiple LLM evaluations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants