refactor(e2e): migrate agent-turn-latency to shared inference adapter (#5745) - #8619
Merged
cv merged 3 commits intoAug 8, 2026
Merged
Conversation
…NVIDIA#5745) agent-turn-latency-helpers.ts and agent-turn-latency.test.ts derived their own model, provider, and expected route from ad hoc NEMOCLAW_* env inspection, duplicating logic the shared E2E inference adapter (NVIDIA#5745) already centralizes. Route both through the adapter instead: - env(), installSandbox(), cleanupTurnSandbox(s)(), route(), openclawTurn(), and waitHermesHealth() now take an AgentTurnInference value (the adapter's env/expectedRouteProvider/model/mode/provider/redactionValues) instead of a bare apiKey and locally computed MODEL/PROVIDER/EXPECTED_ROUTE_PROVIDER. - The test skips itself when the adapter resolves to hermetic `mock` mode, since a real latency measurement is meaningless against a loopback stub; select `internal-nvidia` or `public-nvidia` via NEMOCLAW_E2E_INFERENCE_MODE to run it for real. e2e.yaml sets NEMOCLAW_E2E_INFERENCE_MODE=internal-nvidia for this job accordingly. - Progress-support unit tests exercise the same helpers with a small fakeInference() stand-in instead of a bare secret string. No behavior change for callers already setting NEMOCLAW_E2E_USE_HOSTED_INFERENCE; this only removes the duplicated model/provider/route derivation. Signed-off-by: Waqas Ahmed <wakqasahmed@protonmail.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe agent-turn latency E2E test now consumes injected inference adapter configuration. Helpers use adapter-provided environments and redaction values. The workflow selects internal NVIDIA inference mode, and support tests use a reusable fake inference fixture. ChangesAgent turn latency inference integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
…dary codebase-growth-guardrails flagged the mode skip added in the prior commit: it was an `if` inside the test body, which the guardrail requires to be a skipIf/runIf boundary instead. Move it there, matching the existing runIssue4434LiveTest pattern in issue-4434-tui-unreachable-inference.test.ts: resolve NEMOCLAW_E2E_INFERENCE_MODE once at module scope via the adapter's own normalizeMode() (now exported instead of duplicated) and gate the test with `test.skipIf(...)`. Biome's reformat of the now-differently-wrapped test() call accounts for most of the diff noise. Signed-off-by: Waqas Ahmed <wakqasahmed@protonmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
agent-turn-latency-helpers.tsandagent-turn-latency.test.tsderived their own model, provider, and expected route via ad hocNEMOCLAW_*env inspection, duplicating logic the shared E2E inference adapter (#5745) already centralizes. This migrates both to route through the adapter instead.Changes
env(),installSandbox(),cleanupTurnSandbox(s)(),route(),openclawTurn(), andwaitHermesHealth()now take anAgentTurnInferencevalue (the adapter'senv/expectedRouteProvider/model/mode/provider/redactionValues) instead of a bareapiKeyplus locally computedMODEL/PROVIDER/EXPECTED_ROUTE_PROVIDER.mockmode, since a real latency measurement is meaningless against a loopback stub. Selectinternal-nvidiaorpublic-nvidiaviaNEMOCLAW_E2E_INFERENCE_MODEto run it for real;e2e.yamlsetsNEMOCLAW_E2E_INFERENCE_MODE=internal-nvidiafor this job accordingly.fakeInference()stand-in instead of a bare secret string.No behavior change for callers already setting
NEMOCLAW_E2E_USE_HOSTED_INFERENCE; this only removes the duplicated model/provider/route derivation.Test plan
npx vitest run --project e2e-support test/e2e/support/agent-turn-latency-progress.test.ts— 8/8 passednpm run validate:prequivalent hooks (pre-commit, commit-msg, pre-push) all passed locally, including CLI/plugin TypeScript checksSigned-off-by: Waqas Ahmed wakqasahmed@protonmail.com
🤖 Generated with Claude Code
Summary by CodeRabbit