test(e2e): retry inference switch verification#4152
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughWalkthroughA new shared Bash E2E helper library detects transient inference-switch failures (timeouts, connection errors, DNS issues, 5xx responses) and retries failed commands up to a configurable limit with linear backoff. Both Hermes and OpenClaw test scripts now integrate this helper, replacing direct ChangesInference switch retry resilience
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
PR Review AdvisorFindings: 0 needs attention, 1 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/test-hermes-inference-switch.sh (1)
47-77: ⚡ Quick winConsider extracting retry helpers to a shared library.
The
is_transient_inference_set_failure()andrun_inference_set_with_retry()functions are duplicated between this file andtest/e2e/test-openclaw-inference-switch.sh(lines 47-77 in both). The only difference is the inference set command invocation (nemohermesvsnemoclaw).Extracting these to a shared library (e.g.,
test/e2e/lib/inference-switch-retry.sh) with a parameterized command would eliminate duplication and make future maintenance easier.♻️ Example extraction approach
In
test/e2e/lib/inference-switch-retry.sh:is_transient_inference_set_failure() { grep -qiE 'timed? out|timeout|ETIMEDOUT|ECONNRESET|EAI_AGAIN|ENOTFOUND|502|503|504|temporar' <<<"$1" } run_inference_set_with_retry() { local cmd="$1" shift local attempt rc output fallback_output local attempts="${NEMOCLAW_SWITCH_SET_ATTEMPTS:-3}" for attempt in $(seq 1 "$attempts"); do output=$("$cmd" "$@" 2>&1) rc=$? # ... rest of logic done }Then source and invoke with:
run_inference_set_with_retry nemohermes inference set --provider "$SWITCH_PROVIDER" --model "$SWITCH_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 `@test/e2e/test-hermes-inference-switch.sh` around lines 47 - 77, Extract the duplicated functions is_transient_inference_set_failure and run_inference_set_with_retry into a shared script (e.g., test/e2e/lib/inference-switch-retry.sh), make run_inference_set_with_retry accept the full command and its args (e.g., run_inference_set_with_retry nemohermes inference set --provider "$SWITCH_PROVIDER" --model "$SWITCH_MODEL"), preserve the existing behavior including using NEMOCLAW_SWITCH_SET_ATTEMPTS and the fallback invocation with --no-verify, and update both test/e2e/test-hermes-inference-switch.sh and test/e2e/test-openclaw-inference-switch.sh to source the new lib and call the parameterized run_inference_set_with_retry instead of their local copies of nemohermes/nemoclaw-specific implementations.
🤖 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.
Nitpick comments:
In `@test/e2e/test-hermes-inference-switch.sh`:
- Around line 47-77: Extract the duplicated functions
is_transient_inference_set_failure and run_inference_set_with_retry into a
shared script (e.g., test/e2e/lib/inference-switch-retry.sh), make
run_inference_set_with_retry accept the full command and its args (e.g.,
run_inference_set_with_retry nemohermes inference set --provider
"$SWITCH_PROVIDER" --model "$SWITCH_MODEL"), preserve the existing behavior
including using NEMOCLAW_SWITCH_SET_ATTEMPTS and the fallback invocation with
--no-verify, and update both test/e2e/test-hermes-inference-switch.sh and
test/e2e/test-openclaw-inference-switch.sh to source the new lib and call the
parameterized run_inference_set_with_retry instead of their local copies of
nemohermes/nemoclaw-specific implementations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a928bf82-f3ef-412e-b2c4-efad71a08cbe
📒 Files selected for processing (2)
test/e2e/test-hermes-inference-switch.shtest/e2e/test-openclaw-inference-switch.sh
|
Addressed feedback:
Validation:
Optional selective E2E is running for |
Selective E2E Results — ✅ All requested jobs passedRun: 26355753234
|
Summary
The nightly flake sweep showed
openclaw-inference-switch-e2eandhermes-inference-switch-e2eas high-frequency failures, mostly due to transient live endpoint verification timeouts duringinference set. This PR retries transient verification failures and falls back to--no-verifyonly after repeated transient failures, while keeping the later route/config/live-request assertions as the real correctness gate.Changes
test/e2e/lib/inference-switch-retry.sh.NEMOCLAW_SWITCH_SET_ATTEMPTSas a positive integer before retrying.nemoclaw inference set/nemohermes inference setattempts when failures look transient.--no-verify; subsequent route, config, sandbox inference, and agent/API request checks still validate the switched route.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com