STO-408: run Evaluators on Qwen 3.8 preview - #24
Conversation
📝 WalkthroughWalkthroughEvaluator-A and Evaluator-B now use the OpenCode runtime with ChangesEvaluator runtime migration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/sync-topology.test.py (1)
87-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRetain a Grok-origin migration fixture.
The PR migrates evaluators from Grok to OpenCode, but the stub now exposes only
r-opencode, the legacy scenario seedsa-evalalready at the desired state, and the identity test starts from Claude. No test proves that an existing Grok evaluator keeps its identity while moving to OpenCode andqwen3.8-max-preview.As per coding guidelines, tests must protect business intent. Based on the PR objective, the migration path is specifically Grok to OpenCode.
Also applies to: 209-215, 253-261
🤖 Prompt for 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. In `@tests/sync-topology.test.py` around lines 87 - 91, Retain a Grok-origin evaluator in the runtime-list stub and update the relevant migration and identity test fixtures so an existing Grok evaluator is migrated to OpenCode with model qwen3.8-max-preview while preserving its evaluator identity. Ensure the scenarios at the referenced sections start from Grok rather than an already-migrated or Claude evaluator and assert the resulting identity and configuration.Source: Coding guidelines
🤖 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/sync-topology.test.py`:
- Around line 51-52: Update the agent create stub to parse and persist the
requested --model value instead of writing an empty model, then extend the
create-if-missing test to assert the created evaluator retains
qwen3.8-max-preview. Preserve the existing fixture and other topology
assertions.
---
Nitpick comments:
In `@tests/sync-topology.test.py`:
- Around line 87-91: Retain a Grok-origin evaluator in the runtime-list stub and
update the relevant migration and identity test fixtures so an existing Grok
evaluator is migrated to OpenCode with model qwen3.8-max-preview while
preserving its evaluator identity. Ensure the scenarios at the referenced
sections start from Grok rather than an already-migrated or Claude evaluator and
assert the resulting identity and configuration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d70e6313-2619-4f0f-8fa4-772943af44db
📒 Files selected for processing (5)
AGENTS.mdCLAUDE.mddeployments/agents.jsondocs/operating-model.mdtests/sync-topology.test.py
| "runtime_provider": "opencode", | ||
| "model": "qwen3.8-max-preview", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist the requested model in the create stub.
The fixture now declares qwen3.8-max-preview, but the agent create stub at Line 104 still writes model: "". The create-if-missing test can therefore pass while the fake topology has no model, leaving the new deployment contract unverified. Parse --model in the stub and assert the created evaluator’s model.
As per coding guidelines, tests must protect business intent rather than merely exercise implementation details.
Proposed fixture fix
elif args[:2] == ["agent", "create"]:
name = args[args.index("--name") + 1]
runtime = args[args.index("--runtime-id") + 1]
- created = {"id":"a-eval","name":name,"runtime_id":runtime,"model":""}
+ model = args[args.index("--model") + 1] if "--model" in args else ""
+ created = {"id":"a-eval","name":name,"runtime_id":runtime,"model":model}🤖 Prompt for 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.
In `@tests/sync-topology.test.py` around lines 51 - 52, Update the agent create
stub to parse and persist the requested --model value instead of writing an
empty model, then extend the create-if-missing test to assert the created
evaluator retains qwen3.8-max-preview. Preserve the existing fixture and other
topology assertions.
Source: Coding guidelines
1 participant
Originating Multica issue: STO-408
Summary
Evaluator-A and Evaluator-B now target the online OpenCode runtime with the pinned
qwen3.8-max-previewmodel instead of the unavailable Grok setup. Their logical identities and Squad memberships remain unchanged.Why
The Grok evaluator lane currently lacks usable tokens. STO-408 records the bounded decision to use Qwen 3.8 preview first and reconsider Codex only if evaluation quality is insufficient.
Approach
Updated the mirrored company contract, deployment desired state, and operating-model roster. The topology regression fixture now models OpenCode/Qwen, directly asserts that both Evaluator instances use the exact same provider/model, and proves an existing Evaluator identity can migrate runtime and model together.
Accepted choice: Route both Evaluators through the sole online OpenCode runtime and pin
qwen3.8-max-preview.Rejected alternatives:
qwenas the runtime provider — fresh Multica reads exposeopencode, not aqwenprovider; the requested Qwen value belongs in the model field.Constraint: Multica topology is repository-managed and may be applied only after this desired state reaches clean
main == origin/main.How I Tested
End-to-end test cases
tests/sync-topology.test.py:169opencode+qwen3.8-max-previewtests/sync-topology.test.py:232tests/sync-topology.test.py:312Verbatim test output
Read-only Multica plans
The two Evaluator rows are this change. The Orchestrator row is unrelated pre-existing drift and must not be applied as part of STO-408 without a separate decision. All Squad and member rows were
up-to-date.Existing tests
tests/sync-topology.test.py→ passbash tests/sync-multica.test.sh→ passbash tests/pr-sweep.test.sh→ passNew tests added
tests/sync-topology.test.py:169— locks both Evaluator instances to the requested provider/model.tests/sync-topology.test.py:232— strengthens the existing identity migration case to assert runtime and model update arguments.Lint / typecheck
Rollback Plan
Revert the merge commit with
git revert <merge-commit>. If the desired topology was already applied, merge the revert and run the normal topology plan/apply/verify from cleanmainto restore the previous tracked runtime/model intent.Maximum blast radius: single feature — independent Evaluator execution quality and availability.
Time-to-rollback: one topology-sync cycle after the revert reaches
main.Out of Scope
Summary by CodeRabbit
Configuration
qwen3.8-max-previewmodel.Tests