Skip to content

fix: harden synced registry and keepalive helpers - #2793

Merged
stranske merged 4 commits into
mainfrom
fix/sync-review-debt
Jul 19, 2026
Merged

fix: harden synced registry and keepalive helpers#2793
stranske merged 4 commits into
mainfrom
fix/sync-review-debt

Conversation

@stranske

@stranske stranske commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Fixes the source-of-truth review debt found in the current consumer sync wave.

  • Treat bundled stale LLM slot pins as debug-only while retaining warnings and fail-closed behavior for explicit allowlists.
  • Ignore malformed capability bundle entries before producing keepalive metrics.

Validated with python -m pytest tests/tools/test_llm_registry_selection.py -q and node --test .github/scripts/__tests__/capability-bundle-contract.test.js.

Summary by CodeRabbit

  • Bug Fixes

    • Improved capability metrics handling by ignoring malformed capability bundle entries and only recording well-formed capability data.
    • Normalized slot conflict messaging: bundled legacy model pin conflicts are treated as advisory (debug-only) when no explicit slot configuration is set; explicit mismatches still warn and fail safely.
    • Improved selection and slot loading by normalizing evidence IDs and always emitting slot definitions per provider index.
  • Tests

    • Added contract coverage for malformed capability bundle entries.
    • Added coverage for debug-only bundled stale slot pin behavior and exact template sync validation.
  • Chores

    • Updated template sync manifest to enforce exact syncing for the LLM registry tool.

Copilot AI review requested due to automatic review settings July 19, 2026 14:29
@cursor

cursor Bot commented Jul 19, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ca29a834-a624-4d85-b3dd-a010145c6922

📥 Commits

Reviewing files that changed from the base of the PR and between 6fcf425 and a19c555.

📒 Files selected for processing (3)
  • .github/sync-manifest.yml
  • templates/consumer-repo/tools/llm_registry.py
  • tests/scripts/test_validate_template_sync.py

📝 Walkthrough

Walkthrough

Keepalive metrics now exclude malformed capability bundle entries. Slot loading treats stale bundled model pins as advisory while retaining strict handling for explicit runtime slot configuration. Registry selection normalization and exact consumer-template synchronization checks were also added.

Changes

Capability metrics sanitization

Layer / File(s) Summary
Sanitize capability bundle metrics
.github/scripts/keepalive_loop.js, templates/consumer-repo/.github/scripts/keepalive_loop.js, .github/scripts/__tests__/capability-bundle-contract.test.js
Both keepalive implementations filter applied and rejected capability entries to valid objects, with tests verifying malformed entries are ignored.

Slot and template alignment

Layer / File(s) Summary
Normalize slot selection data
templates/consumer-repo/tools/llm_registry.py
Evidence identifiers are stripped individually, and default slots are emitted for every provider index with an empty model when unresolved.
Handle stale bundled pins
tools/llm_registry.py, templates/consumer-repo/tools/llm_registry.py, tests/tools/test_llm_registry_selection.py
Explicit slot configuration mismatches still warn and skip, while bundled mismatches log at debug level and retain the reviewed registry selection; a test verifies the bundled behavior.
Enforce exact template synchronization
.github/sync-manifest.yml, tests/scripts/test_validate_template_sync.py
The registry tool requires exact template synchronization, with validator coverage for detecting drift and reporting the update command.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: maintenance, sync, agents:keepalive, verify:compare

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: hardening synced registry behavior and keepalive helper handling.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sync-review-debt

Comment @coderabbitai help to get the list of available commands.

@stranske-keepalive

Copy link
Copy Markdown
Contributor

Workflow source needed

PR #2793 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely.

Please do one of:

  • Add <!-- meta:issue:123 --> or a normal Closes #123 / Related to #123 line.
  • Check one Workflow Source option in the PR body.
  • Add a hidden marker such as <!-- workflow-source:local_request -->, <!-- workflow-source:manual_remote -->, <!-- workflow-source:review_followup -->, <!-- workflow-source:sync_campaign -->, or <!-- workflow-source:dependabot -->.
  • Add a workflow source label such as workflow:source-direct-pr, workflow:source-local-request, workflow:source-review-followup, workflow:source-sync, or workflow:no-automation.

Once a valid source is present, this warning will not be reposted.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@tests/tools/test_llm_registry_selection.py`:
- Around line 249-263: Update test_bundled_stale_slot_pin_is_debug_only to
capture logs at DEBUG level and assert that caplog contains the advisory message
“Ignoring advisory bundled slot model pin.” Retain the existing assertion that
“Skipping unresolved slot model pin” is absent at warning level, ensuring
coverage verifies the message is emitted specifically at DEBUG.
🪄 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: 1b6b54d0-73a2-4e76-9023-96b8827b26a9

📥 Commits

Reviewing files that changed from the base of the PR and between 0dcd6bb and b791d82.

📒 Files selected for processing (5)
  • .github/scripts/__tests__/capability-bundle-contract.test.js
  • .github/scripts/keepalive_loop.js
  • templates/consumer-repo/.github/scripts/keepalive_loop.js
  • tests/tools/test_llm_registry_selection.py
  • tools/llm_registry.py

Comment thread tests/tools/test_llm_registry_selection.py

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

Hardens two consumer-facing surfaces in this repo: (1) LLM slot config resolution (bundled vs explicit allowlist semantics) and (2) keepalive metrics generation when capability bundle inputs are malformed, with corresponding test coverage.

Changes:

  • Adjust load_slot_config() to treat bundled stale slot model pins as advisory (debug-only) while keeping fail-closed behavior (and warnings) for explicit slot allowlists.
  • Ignore malformed capability bundle entries before computing keepalive metrics, in both the repo script and the consumer template copy.
  • Add/extend Python + Node tests to lock in the new behaviors.

Reviewed changes

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

Show a summary per file
File Description
tools/llm_registry.py Makes bundled stale slot pins advisory (debug-only) while preserving fail-closed allowlist semantics for ENV_SLOT_CONFIG.
tests/tools/test_llm_registry_selection.py Adds coverage ensuring bundled stale slot pin warnings are not emitted at WARNING level.
.github/scripts/keepalive_loop.js Filters out malformed capability bundle entries before emitting metrics fields.
templates/consumer-repo/.github/scripts/keepalive_loop.js Mirrors the keepalive metrics hardening change for consumer template sync.
.github/scripts/tests/capability-bundle-contract.test.js Adds a contract test ensuring malformed bundle entries are ignored in metrics.

Comment thread tools/llm_registry.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9adfeb6633

ℹ️ 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".

Comment thread .github/scripts/keepalive_loop.js
@agents-workflows-bot

agents-workflows-bot Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Automated Status Summary

Head SHA: f505b13
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 75.70%
Baseline 85.00%
Delta -9.30%
Minimum 70.00%
Status ✅ Pass

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
scripts/issue_dedup_smoke.py 0.0% 4
scripts/runner_lib/__main__.py 0.0% 3
scripts/validate_template_sync.py 0.0% 81
scripts/langchain/topic_splitter.py 19.1% 57
tools/codex_log_analyzer.py 19.6% 140
scripts/repo_review_round2_runner.py 25.1% 344
scripts/prune_agent_stubs.py 39.7% 26
scripts/repo_review_round1_runner.py 40.7% 133
tools/ensure_workflow_timeout_variables.py 42.1% 74
scripts/sync_label_docs.py 42.9% 64
tools/discover_model_catalog.py 44.8% 55
scripts/repo_review_backlog_scan.py 45.3% 116
scripts/repo_review_body_writer.py 46.5% 86
tools/codex_session_analyzer.py 47.9% 59
scripts/create_verifier_labels.py 48.3% 58

Low Coverage Files (<50.0%)

File Coverage Missing
scripts/issue_dedup_smoke.py 0.0% 4
scripts/runner_lib/__main__.py 0.0% 3
scripts/validate_template_sync.py 0.0% 81
scripts/langchain/topic_splitter.py 19.1% 57
tools/codex_log_analyzer.py 19.6% 140
scripts/repo_review_round2_runner.py 25.1% 344
scripts/prune_agent_stubs.py 39.7% 26
scripts/repo_review_round1_runner.py 40.7% 133
tools/ensure_workflow_timeout_variables.py 42.1% 74
scripts/sync_label_docs.py 42.9% 64
tools/discover_model_catalog.py 44.8% 55
scripts/repo_review_backlog_scan.py 45.3% 116
scripts/repo_review_body_writer.py 46.5% 86
tools/codex_session_analyzer.py 47.9% 59
scripts/create_verifier_labels.py 48.3% 58

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


Keepalive checklist

Scope

No scope information available

Tasks

  • No tasks defined

Acceptance criteria

  • No acceptance criteria defined

@stranske
stranske temporarily deployed to agent-standard July 19, 2026 16:27 — with GitHub Actions Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@templates/consumer-repo/tools/llm_registry.py`:
- Around line 361-379: Add a parity test or CI sync check covering
templates/consumer-repo/tools/llm_registry.py, alongside the existing
tools/llm_registry.py synchronization coverage. Ensure the check detects drift
between the consumer template copy and its source, including the updated
unresolved slot model pin handling.
🪄 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: 77bb27a0-4bc0-4570-b864-dc0d04ad6bd4

📥 Commits

Reviewing files that changed from the base of the PR and between b791d82 and 6fcf425.

📒 Files selected for processing (2)
  • templates/consumer-repo/tools/llm_registry.py
  • tests/tools/test_llm_registry_selection.py

Comment thread templates/consumer-repo/tools/llm_registry.py
@stranske
stranske temporarily deployed to agent-standard July 19, 2026 20:29 — with GitHub Actions Inactive
@stranske
stranske merged commit f2d175d into main Jul 19, 2026
60 checks passed
@stranske
stranske deleted the fix/sync-review-debt branch July 19, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants