feat(sync): publish maintenance efficiency SLOs - #2896
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis change adds Health 83 dependency-sync metrics, advisory SLO evaluation, bounded evidence collection, fingerprinted reports, scheduled and manual workflow publication, tests, and documentation. ChangesDependency sync efficiency
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Trigger
participant Health83Workflow
participant GitHub
participant MetricsScript
participant Tracker2897
participant Artifacts
Trigger->>Health83Workflow: start scheduled or manual run
Health83Workflow->>GitHub: collect bounded PR and workflow-run evidence
Health83Workflow->>MetricsScript: provide evidence snapshot
MetricsScript-->>Health83Workflow: return JSON, Markdown, and evidence fingerprint
Health83Workflow->>Tracker2897: post report when material evidence changes
Health83Workflow->>Artifacts: upload input and generated reports
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Runner dispatch state for codex on PR #2896. Do not edit. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2ffff5cfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds a new “Health 83” maintenance-efficiency surface to make dependency/sync maintenance cost visible via a scheduled/manual workflow, a hermetic metrics calculator, fixture-backed tests, and supporting documentation—intended to provide an advisory (non-blocking) SLO signal.
Changes:
- Introduces
scripts/dependency_sync_efficiency_metrics.pyto calculate lane counts, stale/replacement signals, amplification, and an evidence fingerprint from a portable snapshot. - Adds
.github/workflows/health-83-dependency-sync-efficiency.ymlto collect bounded GitHub evidence, publish JSON/Markdown artifacts, and comment to a durable tracker when the fingerprint changes. - Documents and registers the new workflow in CI/system docs and workflow naming tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/workflows/test_workflow_naming.py | Registers the new workflow display name for uniqueness coverage. |
| tests/workflows/test_dependency_sync_efficiency_workflow.py | Adds assertions that Health 83 is scheduled/manual and contains expected artifact + dedupe markers. |
| tests/scripts/test_dependency_sync_efficiency_metrics.py | Adds fixture-backed unit tests for lane classification, fingerprint stability, and breach rendering. |
| scripts/dependency_sync_efficiency_metrics.py | Implements the report calculation, fingerprinting, and Markdown rendering for advisory SLOs. |
| docs/ops/DEPENDENCY_SYNC_EFFICIENCY.md | Documents measures, advisory thresholds, exclusions, and bounded-history limitations. |
| docs/ci/WORKFLOWS.md | Adds Health 83 to the scheduled health workflow inventory. |
| docs/ci/WORKFLOW_SYSTEM.md | Adds Health 83 to the workflow system reference table. |
| .github/workflows/health-83-dependency-sync-efficiency.yml | New Health 83 workflow to collect evidence, generate report artifacts, and update a durable tracker on material change. |
|
Runner dispatch state for autofix on PR #2896. Do not edit. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/health-83-dependency-sync-efficiency.yml:
- Line 18: Update the actions/checkout@v7 step in the workflow to set
persist-credentials to false, while leaving the existing checkout behavior and
subsequent github-script token handling unchanged.
- Around line 28-82: Update the workflow’s API usage to satisfy Maint 52 by
adding the local setup-api-client step and initializing createTokenAwareRetry in
each github-script block that makes GitHub API calls. Replace the direct
pulls.list, actions.listWorkflowRunsForRepo, issues.listComments pagination, and
issues.createComment calls with the appropriate withRetry or paginateWithRetry
wrapper.
- Around line 41-53: Update the pull-request collection in the `pulls.push` flow
to parse the `workflows-consumer-sync:v1` marker from `pr.body` and map a stable
marker field such as `source_sha` to the calculator’s expected source key. Use
that same source key when associating entries in the `workflow_runs` collection,
rather than using `run.head_sha`, which identifies the consumer commit. Add
fixtures covering the marker format and the resulting workflow-run mapping.
In `@scripts/dependency_sync_efficiency_metrics.py`:
- Around line 43-48: Update labels() to handle both string items and
mapping-shaped label objects without calling .get on a string: use the string
value directly for string items, otherwise retain the existing name extraction
and lowercasing behavior. Add or update tests covering both label shapes and
mixed inputs, as this changes behavior in an untested helper.
- Around line 157-161: Update the avoidable_replacements Counter to key by the
same repository/batch identifier used by source_to_prs, reusing that existing
source/batch key computation rather than grouping only by repository. Preserve
the replacement filter and ensure both metrics report consistent
repository/batch granularity.
In `@tests/scripts/test_dependency_sync_efficiency_metrics.py`:
- Around line 14-37: Extend
test_fixture_classifies_all_generated_lanes_and_excludes_collab_admin to assert
that report["advisory_slo"]["breaches"]["avoidable_replacements_per_repo_batch"]
contains the expected breach for stranske/App, reflecting PR `#3`’s replacement
classification and threshold violation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e20ed9c-4fe7-496e-bb35-13700b816f38
📒 Files selected for processing (8)
.github/workflows/health-83-dependency-sync-efficiency.ymldocs/ci/WORKFLOWS.mddocs/ci/WORKFLOW_SYSTEM.mddocs/ops/DEPENDENCY_SYNC_EFFICIENCY.mdscripts/dependency_sync_efficiency_metrics.pytests/scripts/test_dependency_sync_efficiency_metrics.pytests/workflows/test_dependency_sync_efficiency_workflow.pytests/workflows/test_workflow_naming.py
|
Autofix updated these files:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/dependency_sync_efficiency_metrics.py`:
- Around line 236-239: Update the Markdown report generation around the existing
metrics summary to include avoidable-replacement evidence: list each violating
repository/batch key and its count whenever the zero-per-repository/batch target
is breached, even if the aggregate stale/replacement rate is compliant. Preserve
the existing summary metrics and add a test covering this breach path and its
rendered evidence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7cce2fde-b10d-4d0d-80b0-d4a088850769
📒 Files selected for processing (3)
scripts/dependency_sync_efficiency_metrics.pytests/scripts/test_dependency_sync_efficiency_metrics.pytests/workflows/test_dependency_sync_efficiency_workflow.py
💤 Files with no reviewable changes (1)
- tests/workflows/test_dependency_sync_efficiency_workflow.py
🤖 Bot Comment Handler
The agent has been assigned to this PR to address the bot review comments. Instructions for agent
The bot comment handler workflow has prepared context in the artifacts. |
057b4a2 to
39cfe7d
Compare
|
Closer recovery pushed 39cfe7d after rebasing onto current main. It routes Health 83 GitHub calls through createTokenAwareRetry/setup-api-client, pins external Actions and disables persisted checkout credentials, applies an explicit trailing seven-day evidence window, maps consumer-sync marker source IDs to linked workflow runs, compares only the latest Health 83 tracker fingerprint, and fixes label/replacement metric edge cases. Validation: 7 focused metric/workflow tests passed; check_api_wrapper_guard passed; workflow YAML parsed; actionlint passed; git diff --check passed. scripts/dev_check.sh reached syntax and workflow YAML validation but cannot continue locally because the installed macOS Bash lacks mapfile. |
|
Closer review recovery on
Validation: Resolved the 12 open review threads on this head (prior findings + the two current Copilot/CodeRabbit items above). |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/health-83-dependency-sync-efficiency.yml:
- Around line 96-103: Update the “Calculate advisory SLOs” step and its metrics
command to capture and reuse the fingerprint emitted by
dependency_sync_efficiency_metrics.py, rather than launching a second Python
process to recompute it. Preserve writing the captured fingerprint to
GITHUB_OUTPUT under the existing fingerprint key.
In `@scripts/dependency_sync_efficiency_metrics.py`:
- Around line 136-144: Update the later lane classification at line 184 to read
the cached pr["_lane"] value populated in the pulls loop instead of calling
lane_for(pr) again. Preserve the existing lane filtering and downstream
behavior, using the cached field as the sole source for that classification.
- Around line 171-176: Derive the `batch` value from the already-computed
`source` in the surrounding dependency metrics processing instead of calling
`first` with a duplicated fallback-key list. Keep the existing fallback to
`"unknown"` in the `source` computation, and use that same normalized value for
both `source_to_prs` and `avoidable_replacements_per_repo_batch` grouping.
- Around line 254-263: Update the stale_or_replacement_rate condition in the
breaches mapping to flag rates equal to or above
THRESHOLDS["stale_or_replacement_rate"], matching the documented “< 5%”
threshold while leaving the other breach checks unchanged. Add a fixture test
covering exactly 5% (such as 2 stale/replacement PRs out of 40 generated) and
assert that it breaches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: beaf4d99-9e89-4a8b-be22-efe749fdc0e4
📒 Files selected for processing (9)
.github/workflows/health-83-dependency-sync-efficiency.ymldocs/ci/WORKFLOWS.mddocs/ci/WORKFLOW_SYSTEM.mddocs/ops/DEPENDENCY_SYNC_EFFICIENCY.mddocs/ops/DURABLE_TRACKING_ISSUES.mdscripts/dependency_sync_efficiency_metrics.pytests/scripts/test_dependency_sync_efficiency_metrics.pytests/workflows/test_dependency_sync_efficiency_workflow.pytests/workflows/test_workflow_naming.py
|
Addressed the four current CodeRabbit findings in |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
Disagreement
Unique Insights
🔍 LangSmith Traces |
Closes #2883
Automated Status Summary
Scope
The maintenance system currently exposes individual workflow and campaign state, but it does not publish a durable efficiency view. Without PR amplification, stale/replacement rate, CI runs per delivered source change, and agent-exception episodes, a change can look operationally successful while total maintenance work grows.
The recent audit required a one-off reconstruction of those measures. This is a latent control defect: future Renovate, sync, or controller changes can regress productivity without a named test or SLO detecting the regression.
Tasks
scripts/dependency_sync_efficiency_metrics.pyto classify PRs by dependency-bot, sync-generated, dev-tool-sync, traditional, and Collab-Admin-excluded lanes..github/workflows/health-83-dependency-sync-efficiency.ymlto publish JSON and Markdown artifacts on a weekly schedule and by manual dispatch.docs/ops/DEPENDENCY_SYNC_EFFICIENCY.mdand link it from durable-tracker guidance.Acceptance criteria
python scripts/dev_check.py --action testand workflow validation successfully.Summary by CodeRabbit
New Features
Documentation
Tests