Skip to content

feat: implement issue #1898 — [Phase 2] Swap the deep tier to claude-opus-5-5 in engine.sh (model, chain, label, effort) - #1935

Merged
don-petry merged 6 commits into
mainfrom
dev-lead/issue-1898-20260925-0504
Sep 25, 2026
Merged

don-petry merged 6 commits into
mainfrom
dev-lead/issue-1898-20260925-0504

Conversation

@don-petry

@don-petry don-petry commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

[Phase 2] Swap the deep tier to claude-opus-5-5 in engine.sh (model, chain, label, effort)

From the issue: As a review-engine maintainer, I want point ENGINE_DEEP_MODEL and the deep fallback chain at claude-opus-5-5 and update the engine label, so that the highest-cost agentic tier (deep review, dev-lead fix-issue, persona advisory) runs on the cheaper, higher-benchmark model.

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_mcp.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).

Monitoring

This PR's Lint (shellcheck) and bats checks show pass/fail; watch subsequent dev-lead / pr-review runs for behavioral regressions.

Closes #1898

Review in cubic

Summary by CodeRabbit

  • Improvements
    • Deep-tier Claude runs now use Claude Opus 5.5 with high reasoning effort. If that model is rate-limited, the system can fall back to Claude Sonnet 5.0.
    • Advisory runs now explicitly use Claude Opus 5.5 with high reasoning effort.
    • Other Claude tiers continue without a reasoning-effort override.

…opus-5-5 in engine.sh (model, chain, label, effort)
@qodo-code-review

This comment has been minimized.

@coderabbitai

This comment has been minimized.

@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-09-25T06:22:12Z.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #1935
No changes were committed, but the PR still can't be marked done: required checks CodeQL, SonarCloud are still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-09-25T05:52:12Z

@don-petry

Copy link
Copy Markdown
Collaborator Author

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required checks CodeQL, SonarCloud are still pending. I'll re-check automatically.
Next attempt after: 2026-09-25T05:52:12Z

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 05:22
@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates the Claude deep tier model from claude-opus-4-8 to claude-opus-5-5 across the engine configuration, fallback chains, and tests. It also introduces a reasoning-effort pin (--effort high) specifically for the deep tier to override the model's default medium effort. The review feedback suggests adding robust error handling for mktemp calls in the newly added unit tests to prevent silent failures under set -e.

Comment thread tests/dev-lead/unit/test_engine_chain.bats Outdated
Comment thread tests/dev-lead/unit/test_engine_chain.bats Outdated
Comment thread tests/dev-lead/unit/test_engine_chain.bats Outdated
Comment thread tests/dev-lead/unit/test_engine_chain.bats Outdated
@don-petry
don-petry disabled auto-merge September 25, 2026 05:22

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread tests/dev-lead/unit/test_engine_chain.bats Outdated
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (applied)

Changes committed and pushed. Requested items addressed:

  • tests/dev-lead/unit/test_engine_chain.bats:411 — applied
  • tests/dev-lead/unit/test_engine_chain.bats:424 — applied
  • tests/dev-lead/unit/test_engine_chain.bats:437 — applied
  • tests/dev-lead/unit/test_engine_chain.bats:450 — applied

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 05:24

@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: 0d287521d5

ℹ️ 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 scripts/engine.sh
@don-petry
don-petry disabled auto-merge September 25, 2026 05:26
@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-09-25T06:26:13Z.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Acknowledged — this is a Qodo trial-ended billing notice, not a code finding on this PR. No action required in this repository; resuming Qodo reviews is a workspace-admin billing decision, outside this PR's scope.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Acknowledged — this is a CodeRabbit rate-limit/review-limit notice, not an actionable code finding. No change is required; the review will resume automatically once the limit window resets.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Acknowledged — SonarCloud's Quality Gate passed. This is an informational status notice with no findings to address.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (not-applied)

A commit was pushed, but it did not touch any region the review named — the requested changes were not applied. Per requested item:

  • scripts/engine.sh:1152 — not applied

@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.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 05:29
@don-petry
don-petry disabled auto-merge September 25, 2026 05:30
@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-09-25T06:30:34Z.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread .github/workflows/persona-runner-reusable.yml
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (applied)

Changes committed and pushed.

@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.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 05:32
@don-petry
don-petry disabled auto-merge September 25, 2026 05:33
@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-09-25T06:33:35Z.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (applied)

Changes committed and pushed.

@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-09-25T06:34:10Z.

@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.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 05:34

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread .github/workflows/persona-runner-reusable.yml Outdated
@don-petry
don-petry disabled auto-merge September 25, 2026 05:34
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (applied)

Changes committed and pushed.

@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.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 05:36
@don-petry
don-petry disabled auto-merge September 25, 2026 05:37
@sonarqubecloud

Copy link
Copy Markdown

@donpetry-bot

donpetry-bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 0d8b8e5fcb90b3da969a93f54438a312f4c98609 — 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: MEDIUM
Reviewed commit: 00ce776def79995bff70d9fc8f76bc31158a987f
Cascade: triage → deep (triage: haiku 4.5 [sonnet 5] → deep: opus 4.8 [sonnet 5] + duck: o4-mini → audit: fable 5)

Summary

Phase-2 deep-tier model swap (opus-4-8 -> opus-5-5) in engine.sh plus an --effort high pin and matching persona-runner-reusable.yml runtime flags. The logic and test coverage are sound and the triage escalation reason (missing --model pin) is already resolved at the current head e920589, which now sets --model claude-opus-5-5. However CI is red: the newly added [ $? -eq 0 ] || exit 1 after _out=$(mktemp) in the workflow run block trips SC2181, failing both the Lint and actionlint gates. Escalate on this gate failure; no security-audit tier needed.

Findings

  • major: New [ $? -eq 0 ] || exit 1 immediately after _out=$(mktemp) in the persona-runner run: block trips shellcheck SC2181 (check exit code directly, e.g. if ! _out=$(mktemp); then exit 1; fi). This fails the Lint and actionlint gates (CI red, mergeStateStatus BLOCKED). Fix the anti-pattern or the merge stays blocked. Reproduced locally: shellcheck emits SC2181 and exits 1 on the exact snippet.
  • info: Triage escalated on cubic P1 'workflow adds --effort high but does not pin --model'. That comment was at the prior head (00ce776); the current head (e920589) already adds --model claude-opus-5-5 --fallback-model opus --effort high, so the primary model is now pinned and the concern is resolved. No further action needed for the model-pin signal.
  • info: engine.sh deep-tier changes are internally consistent: ENGINE_DEEP_MODEL, CLAUDE_DEEP_MODEL_CHAIN, and ENGINE_LABEL all move opus-4-8 -> opus-5-5 while keeping sonnet-5-0 as the 2nd hop; --effort high is scoped to the deep tier only via _effort_args with the safe empty-array expansion ${_effort_args[@]+"${_effort_args[@]}"}. audit/action/single/triage keep the CLI default, and the added bats tests assert exactly this scoping. No correctness defect found in the shell logic. [auditable: repro unverifiable]

Reviewed by the PR-review cascade (triage: haiku 4.5 [sonnet 5] → deep: opus 4.8 [sonnet 5] + duck: o4-mini → audit: fable 5). 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.

@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-09-25T06:38:10Z.

@don-petry

Copy link
Copy Markdown
Collaborator Author

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

Agent reasoning
Issues addressed: 0
- Codex usage limit: Not a code defect — service quota notification, skipped
Files changed: None
Skipped (non-actionable): 1
```

@don-petry
don-petry enabled auto-merge (squash) September 25, 2026 05:38

@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: MEDIUM
Reviewed commit: 0d8b8e5fcb90b3da969a93f54438a312f4c98609
Review mode: triage-approved (single reviewer)

Summary

Phase-2 deep-tier model swap (opus-4-8 → opus-5-5) in scripts/engine.sh: ENGINE_DEEP_MODEL, CLAUDE_DEEP_MODEL_CHAIN default, and ENGINE_LABEL all move consistently, with sonnet-5-0 kept as the 2nd fallback hop. Adds a deep-tier-only --effort high pin (Story-2 AC-4 finding: opus-5-5 CLI default is medium), guarded by bats tests asserting audit/action/triage do NOT get the flag. The persona-runner-reusable.yml step pins --model claude-opus-5-5 --effort high for eval parity — a reviewed deviation from the story's 'no workflow YAML changes' note, driven by a (now-resolved) Codex P1 thread. The prior cycle's sole blocking finding (SC2181 from '[ $? -eq 0 ] || exit 1' after mktemp in the workflow) is fixed at this head via 'mktemp || exit 1'; Lint, actionlint, and ShellCheck are green. Triage's low-risk assessment is confirmed.

Linked issue analysis

Closes #1898 ([Phase 2] Swap deep tier to claude-opus-5-5). AC-1 (ENGINE_DEEP_MODEL + chain default) ✓; AC-2 (ENGINE_LABEL 'deep: opus 5.5') ✓; AC-3 (--effort high, deep tier only, decision documented inline at the invoke site) ✓; AC-4 (chain bats tests updated opus-4-8 → opus-5-5, bats CI green) ✓; AC-5 (shellcheck clean) ✓. Minor: AC-5 also asked the PR to state that a channel cut (Story 4) is required for activation — the engine.sh comments reference the story but the PR body omits an explicit inertness statement. Non-blocking: the code change itself is inert-on-merge per AGENTS.md channel-tag flow, and activation remains Story 4's responsibility. Audit/action/single Fable-5 assignments are untouched (Phase 3), and the audit-chain opus-4-8 fallback hop is correctly left alone.

Findings

  • resolved (prior major): SC2181 anti-pattern in persona-runner-reusable.yml — now _out=$(mktemp) || exit 1; Lint + actionlint + ShellCheck green at this head. The two commits since the prior review (00ce776 → 0d8b8e5) touched only this file.
  • resolved (prior info): Codex P1 'match persona runtime before raising eval effort' — workflow now pins --model claude-opus-5-5 --effort high; thread resolved.
  • info: engine.sh effort scoping uses the set-u-safe empty-array expansion ${_effort_args[@]+"${_effort_args[@]}"} and is asserted deep-tier-only by 4 new bats tests. No correctness defect found.
  • info: All 8 review threads are resolved; no unanswered human-reviewer questions (all human-authored comments are dev-lead automation dispositions).
  • note: Secret-scan MCP tool (run_secret_scanning) not available in this run; gitleaks CI check is green.
  • minor (non-blocking): PR body omits the AC-5 'inert until channel cut' statement; engine.sh comments carry the story reference. Consider adding one line to the PR body.

CI status

All required checks green: CodeQL, SonarCloud (Quality Gate passed), Lint, actionlint, ShellCheck, shellcheck, bats, unit-tests, gitleaks, CodeQL Analyze (actions/python), agent-shield, and all repo policy gates SUCCESS. CodeRabbit is PENDING (advisory third-party reviewer, previously rate-limited; not a required check — dev-lead automation identified required checks as CodeQL + SonarCloud, both green). dev-lead dispatch/ci-relay/resume show CANCELLED — these are superseded automation plumbing jobs, not test gates. Skipped dependency-audit jobs are ecosystem-not-detected skips.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

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.

[Phase 2] Swap the deep tier to claude-opus-5-5 in engine.sh (model, chain, label, effort)

2 participants