Skip to content

feat: implement issue #1900 — [Phase 3] Refresh the Opus 4.8 fallback-chain hops to Opus 5.5 in engine.sh - #2241

Merged
don-petry merged 1 commit into
mainfrom
dev-lead/issue-1900-20261011-0014
Oct 11, 2026
Merged

don-petry merged 1 commit into
mainfrom
dev-lead/issue-1900-20261011-0014

Conversation

@don-petry

@don-petry don-petry commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

[Phase 3] Refresh the Opus 4.8 fallback-chain hops to Opus 5.5 in engine.sh

From the issue: As a review-engine maintainer, I want the last Claude fallback hop that still falls back to Opus 4.8 from a non-Opus model to use Opus 5.5 instead, so that the action tier's last resort is the cheaper, proven model now that the deep-tier canary has passed (#1899).

Risk

Low — changes automation shell logic under scripts/, covered by shellcheck (--severity=warning) and the bats suite.

Test plan

Tests added/updated: tests/dev-lead/unit/test_engine_chain.bats,tests/dev-lead/unit/test_engine_models.bats tests/dev-lead/unit/test_engine_new_models.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Activation

Inert on merge. The action-tier default only takes effect once a release channel is cut onto this commit: dev-lead runs it from its dev-lead/v139-* channel tags and pr-review from pr-review/v1-*. Until a channel moves, every agent keeps the previous chain.

Monitoring

This PR's Lint (shellcheck) and bats checks show pass/fail. Behavioural monitoring starts after the channel cut, not after the merge: watch the writer's exit-2 (all hops rate-limited) rate, because the last hop now shares Opus 5.5's rate-limit bucket with the deep, audit and single primaries.

Closes #1900

View guided diff Turn on auto-fix

@don-petry
don-petry requested a review from a team as a code owner October 11, 2026 00:46
@don-petry don-petry added the auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label) label Oct 11, 2026
@chatgpt-codex-connector

This comment has been minimized.

@gemini-code-assist

This comment has been minimized.

@coderabbitai

This comment has been minimized.

@sonarqubecloud

This comment has been minimized.

@codeant-ai

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Collaborator Author

QA Lead — test-risk advisory

Risk tier: LOW. This changes one default string in the writer's last-resort fallback hop. It is easy to revert, and the cross-provider exit-2 path still sits behind it. The bats tests were updated to match, and the new != *opus-4-8* checks guard against the change being partly undone.

What I'd shore up (highest leverage first):

  • The effort setting on the new hop isn't tested. The test effort (#1898): action tier (run_writer) does NOT pass --effort only runs the case where sonnet answers first. On this branch, opus-5-5 becomes the action tier's last resort and runs at its CLI default, which is medium effort. That's a different setting from the high that deep, audit and single pin. Add a test where both sonnet hops throttle and opus-5-5 serves, and assert what it gets (--effort absent, or pinned). Then the choice is a tested decision rather than an accident.
  • Shared rate-limit bucket. The action tier's last hop now uses the same model, and so the same RPM/TPM bucket, as the deep, audit and single tiers' primary. If those tiers are already saturating opus-5-5, the writer's last hop gives less relief than opus-4-8 did. The comment block around line 253 of test_engine_chain.bats argues that separate buckets matter. No unit test can model this, so watch for writer exit-2 rates rising after merge.
  • Optional: add a small parity test that checks the docs/engine-configuration.md table against ai_models_default. The two drift by hand on every chain change.

Escalate? No.

Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture (bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label.

@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T01:52:12Z.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge October 11, 2026 00:55
@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry
don-petry disabled auto-merge October 11, 2026 01:14
@don-petry

Copy link
Copy Markdown
Collaborator Author

This is a rate-limit notice from the Gemini code-assist bot indicating it has reached its daily quota limit. No code changes are required — this is an operational notice, not a code finding. The PR's CI checks have all completed successfully.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
- Rate-limit notice: dispositioned as informational (no code changes required) [replied + disposition marker]
Files changed: (no changes made)
Skipped (informational): 1
```
**Analysis:**
The bot comment is a **rate-limit notice** — the Gemini code-assist bot has reached its daily quota and cannot process new requests. This is an **operational notice**, not a code finding. 
✅ **No code changes needed.** The PR's CI checks all passed (57 checks: Agent Security Scan, CodeQL, Lint, ShellCheck, SonarCloud, etc.), and there are no code issues to address.
✅ **Disposition posted:** I replied to the original comment explaining this is an operational notice requiring no code change, then applied the `informational` disposition marker so the maintainer-comment gate can clear it once the harness verifies it.

@don-petry
don-petry enabled auto-merge October 11, 2026 01:14
@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@don-petry
don-petry disabled auto-merge October 11, 2026 01:35
@don-petry

Copy link
Copy Markdown
Collaborator Author

This is a CodeRabbit review rate-limit notice — the free OSS review quota has been temporarily exhausted. No code findings are reported; the rate limit will reset and CodeRabbit can resume reviews. No action required on the PR.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
## Summary
**Bot:** coderabbitai[bot]  
**Issues addressed:** 0  
**Files changed:** None
**Disposition:** Posted `informational` disposition reply on PR comment [#6104252719](https://github.com/petry-projects/.github-private/pull/2241#issuecomment-6104252719)
This was a **rate-limit notice** — a pure operational notification that CodeRabbit's free OSS review quota has been temporarily exhausted. The comment contains no actionable code findings, only a throttle message and metadata. No code changes were needed. The original comment has been dispositioned with an `informational` marker so the maintainer-comment gate can resolve it and unblock the PR.

@don-petry
don-petry enabled auto-merge October 11, 2026 01:36
@don-petry

Copy link
Copy Markdown
Collaborator Author

Maintainer disposition of @chatgpt-codex-connector's status comment by @don-petry:

Codex usage-limit notice: no Codex review ran on this PR. There is no finding to act on.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Maintainer disposition of @sonarqubecloud's status comment by @don-petry:

SonarCloud clean status for ab23418: Quality Gate passed with 0 new issues and 0 security hotspots. Nothing to act on.

@chatgpt-codex-connector

Copy link
Copy Markdown

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

@don-petry

Copy link
Copy Markdown
Collaborator Author

Neutral overview with no actionable findings. All CI checks pass, and the summary correctly reflects that the fallback chain update is safe. No code change needed.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
**Bot:** codeant-ai[bot]
**Issues addressed:** 0 (no actionable findings)
**Disposition:** Informational overview with no code changes required
- **CodeAnt PR Risk: Low Risk comment** → Posted `informational` disposition
  - Reason: Neutral overview with no specific defect or actionable finding
  - All CI checks pass ✅
  - Comment correctly summarizes the safe fallback chain update
**Files changed:** None
**Disposition reply posted:** https://github.com/petry-projects/.github-private/pull/2241#issuecomment-6104929140
The maintainer-comment gate will clear once the harness verifies the disposition marker. No further action needed on this PR.

@don-petry
don-petry enabled auto-merge October 11, 2026 03:14
@donpetry-bot

donpetry-bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at ab23418410b5e987d651a126232ac5ad2b11823b — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: ab23418410b5e987d651a126232ac5ad2b11823b
Review mode: triage-approved (single reviewer)

Summary

Swaps the action tier's last Claude hop from claude-opus-4-8 to claude-opus-5-5 in ai_models_default (scripts/lib/engine-models.sh), with matching updates to the docs row and three bats suites. The code is correct and in scope, but the PR description is missing the channel-cut note that acceptance criterion 4 of #1900 explicitly requires. Only the PR body needs to change.

Linked issue analysis

Closes #1900 (re-scoped 2026-10-10).

Findings

Blocking (metadata only):

  1. The PR description is missing the channel-cut statement required by [Phase 3] Refresh the Opus 4.8 fallback-chain hops to Opus 5.5 in engine.sh #1900 AC4. Add a line such as: "Inert on merge: the action-tier default only takes effect once a channel tag (e.g. the dev-lead / pr-review v1-next channel) is cut onto this commit." Also fix the Monitoring section so the regression watch starts after the cut, not after the merge. No code change is needed.

Non-blocking / advisory:
2. Missing commit trailer. #1900's Dev Notes say the commit should carry a Test-Change-Justification: trailer citing #1900, because it changes existing test assertions. Commit ab234184 has no trailer. The test-tamper guard only enforces this on dev-lead fix-reviews pushes, so it does not gate this PR. Consider adding the trailer to the squash-merge message for the audit trail.
3. Effort level on the new hop (raised by the QA Lead advisory). When both sonnet hops throttle, opus-5-5 now serves the writer at its CLI default effort (medium). Deep, audit and single pin high. This seems acceptable for a last-resort writer hop, but a bats case asserting the effort for the opus-5-5-serves path would make it a deliberate, tested choice.
4. Shared rate-limit bucket. The writer's last hop now shares opus-5-5's RPM/TPM bucket with the deep, audit and single primaries, so it gives less relief when those tiers are saturated. After the channel cut, watch writer exit-2 rates.

Security: No secrets, auth or workflow changes. The run_secret_scanning MCP tool was not available in this run; the gitleaks CI check passed.

CI status

All required checks are green: Lint, ShellCheck, shellcheck, bats, unit-tests, unit, check-model-pins, CodeQL, SonarCloud (Quality Gate passed, 0 new issues), gitleaks, actionlint, AgentShield and Agent Security Scan. The cancelled sync-scope-guard and guard runs were superseded by successful re-runs. cubic reported NEUTRAL. CodeRabbit, Gemini and Codex were rate-limited and gave no findings; their notices have informational dispositions. There are no unresolved review threads.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed, or after a change to the PR body, labels, or linked issues.

@donpetry-bot

Copy link
Copy Markdown
Contributor

No description provided.

@donpetry-bot donpetry-bot 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.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: ab23418410b5e987d651a126232ac5ad2b11823b
Review mode: triage-approved (single reviewer)

Summary

Changes the action tier's last Claude hop from claude-opus-4-8 to claude-opus-5-5 in ai_models_default (scripts/lib/engine-models.sh). The docs row and three bats suites are updated to match. The previous review's only blocker was metadata: the PR body lacked the channel-cut note. The body now has an Activation section saying the change is inert on merge, and Monitoring now starts after the channel cut. Nothing blocking is left.

Linked issue analysis

Closes #1900 (re-scoped 2026-10-10). All four acceptance criteria are met.

  • AC1 — met. claude:action returns claude-sonnet-5-5,claude-sonnet-5,claude-opus-5-5.
  • AC2 — met. The deep, audit, single, triage and duck defaults are unchanged.
  • AC3 — met. test_engine_chain.bats, test_engine_models.bats, test_engine_new_models.bats and the action row in docs/engine-configuration.md are all updated. The new != *claude-opus-4-8* assertions guard against the swap being partly undone. The only remaining claude-sonnet-5,claude-opus-4-8 string in the tree is a duck-override test fixture (test_engine_models.bats:312), not a default assertion.
  • AC4 — met (resolved since the prior review). shellcheck and check-model-pins pass. The body's new Activation section says the change is inert on merge until the dev-lead/v139-* / pr-review/v1-* channel is cut, and Monitoring now starts after the cut.

Findings

Prior-review findings:

  1. Channel-cut statement missing from PR description (blocking, metadata only) — resolved. The Activation and Monitoring sections now cover it.

Non-blocking, carried forward as advisory:
2. Missing Test-Change-Justification: trailer. #1900's Dev Notes ask for it. The test-tamper guard does not gate this PR. Consider adding it to the squash-merge message for the audit trail.
3. Effort on the opus-5-5 hop. When both sonnet hops throttle, opus-5-5 serves the writer at its CLI default effort (medium), unlike the high pinned for deep, audit and single. That is reasonable for a last-resort writer hop. A bats case asserting this would make it a deliberate, tested choice (raised by the QA Lead advisory).
4. Shared rate-limit bucket. The writer's last hop now shares opus-5-5's RPM/TPM bucket with the deep, audit and single primaries. Watch writer exit-2 rates after the channel cut, as the PR's Monitoring section already says.

Security: No secrets, auth or workflow changes. The run_secret_scanning MCP tool was not available in this run; the gitleaks CI check passed.

CI status

All required checks are green: Lint, ShellCheck, shellcheck, bats, unit-tests, unit, check-model-pins, CodeQL, SonarCloud (Quality Gate passed, 0 new issues), gitleaks, actionlint, AgentShield and Agent Security Scan. The cancelled sync-scope-guard and guard runs were superseded by successful re-runs. cubic reported NEUTRAL. CodeRabbit, Gemini and Codex were rate-limited and gave no findings; their notices have informational dispositions. There are 0 unresolved review threads.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

@donpetry-bot

Copy link
Copy Markdown
Contributor

pr-review approved on PARTIAL advisory evidence: 4/5 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).

@don-petry
don-petry added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit 25bbf7e Oct 11, 2026
78 of 83 checks passed
@don-petry
don-petry deleted the dev-lead/issue-1900-20261011-0014 branch October 11, 2026 04:28
don-petry added a commit that referenced this pull request Oct 11, 2026
…2263)

main gained three negated commands that cannot fail their test after this
branch's CI last ran: two in test_claim_sweep.bats (#2231) and one in
test_engine_new_models.bats (#2241). The bats-negation job runs on the merge
with main, so it would have failed on the next run. Each is now
`! cmd || false`; all three assertions hold (45/45 across the two suites).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-rebase:ready Opts a non-draft PR into auto-rebase without an approval (auto-rebase ready_label)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Phase 3] Refresh the Opus 4.8 fallback-chain hops to Opus 5.5 in engine.sh

2 participants