perf(onboard): calibrate provider validation timeouts - #6488
Conversation
|
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 (8)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds adaptive validation timeout calibration for onboarding probes, threads the derived timing through probe execution paths, enables calibration in selection validation, and updates tests and fixtures for the new calibration request flow. ChangesAdaptive Timeout Calibration
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/inference/probe-http-helpers.ts`:
- Around line 74-91: The calibrated timing path in
buildValidationProbeTimingProfile is allowing validation maxTimeSeconds to fall
below the existing 15s baseline, which can make /responses and /chat/completions
probes timeout too early. Update the logic in buildValidationProbeTimingProfile
(and any related calibration constants) so the calibrated profile never reduces
validation below 15 seconds, either by raising the computed floor or by
preventing calibration from lowering the existing standard.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1fa68fbf-13f8-442b-a298-31a829b698a9
📒 Files selected for processing (6)
src/lib/inference/onboard-probes.tssrc/lib/inference/probe-http-helpers.test.tssrc/lib/inference/probe-http-helpers.tssrc/lib/onboard/inference-selection-validation.test.tssrc/lib/onboard/inference-selection-validation.tstest/wsl2-probe-timeout.test.ts
853a314 to
a8aaecc
Compare
apurvvkumaria
left a comment
There was a problem hiding this comment.
The calibration direction is sound, but the exact head has contributor-compliance and behavioral-test blockers:
dco-checkis failing because the PR body has no contributor declaration. Please addSigned-off-by: Ho Lim <subhoya@gmail.com>to the PR description yourself; maintainers cannot do that on a contributor's behalf.- CLI shard 1 fails on the new behavior. Two
onboard-selectionassertions do not expectcalibrateTimeouts: true, andstrict-tool-call-probe-driver.tsstill expects one request even though calibration adds another. Update the focused tests to assert the intended two-request/calibrated contract and make the strict probe pass without weakening it. Fixes #3770still lacks the acceptance item requiring #2001 implementation notes or before/after timing evidence. Add that evidence/update, or change the linkage to a partial/mitigation statement so the issue remains open.- The exact-head static gate is also red because
test/langchain-deepagents-code-image.test.tsexceeds the 1500-line budget by one line. Rebase onto a green base or split/ratchet that test without raising the ceiling.
Please rerun the focused probe tests and the normal PR gate after these changes.
Signed-off-by: Ho Lim <subhoya@gmail.com>
a8aaecc to
b718e6b
Compare
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
left a comment
There was a problem hiding this comment.
Exact-head review at e9400cd43 is clean. The main sync was conflict-free and preserved the contributor patch byte-for-byte by stable patch ID. All 30 checks are green; DCO and both Verified commits pass; CodeRabbit has no unresolved major findings. The nine-category security review confirmed that calibration is secret-free, retains SSRF address pinning and proxy bypass, forbids redirects, preserves bounded curl/process timeouts, cleans temporary response data, and cannot make endpoint validation fail open. Focused evidence: 18 changed tests, 76 integration tests, and 119 curl/SSRF boundary tests passed, plus CLI build/typecheck.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - #6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - #6271 and #6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - #6465, #6539, #6570, and #6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - #6523, #6551, #6484, #6488, #6324, and #6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - #6559, #6538, #6560, #6568, #6552, #6567, and #6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - #6541, #5415, #6246, #6496, and #6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - #6253, #6572, #6444, #6536, and #5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - #6508, #6527, #5506, #6588, #6446, #6447, #6582, #6296, #6367, #6397, and #6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests not applicable, release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Refs NVIDIA#3770. ## Summary - add an opt-in OpenAI-compatible provider validation calibration probe before onboard validation requests - derive standard validation curl timeout budgets from observed transport latency, with a safe widened fallback when calibration cannot complete - keep direct probe callers on existing defaults unless they opt in, preserve the WSL fallback path, and record the selected timing profile in existing `NEMOCLAW_TRACE` output ## Notes - calibration uses a secret-free `/models` request and treats any HTTP response as a useful transport timing sample - special slow-model timeout budgets remain unchanged; the adaptive profile only covers the standard validation path ## Validation - `npx @biomejs/biome check src/lib/inference/probe-http-helpers.ts src/lib/inference/probe-http-helpers.test.ts src/lib/inference/onboard-probes.ts src/lib/onboard/inference-selection-validation.ts src/lib/onboard/inference-selection-validation.test.ts test/wsl2-probe-timeout.test.ts` - `npx vitest run --project cli --project integration src/lib/inference/probe-http-helpers.test.ts src/lib/inference/onboard-probes.test.ts src/lib/onboard/inference-selection-validation.test.ts test/wsl2-probe-timeout.test.ts` - `npm run build:cli` - `npm run test-size:check` - `npm run test-conditionals:scan -- --top 25` - `npm run check:diff` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Onboarding validation probes now use calibrated, profile-based connect/max timeouts, adjusting automatically from observed latency for OpenAI-like endpoints. * OpenAI-compatible endpoint validation now always enables timeout calibration during probing. * **Bug Fixes** * Validation probe curl timing is more consistent and safely falls back when calibration data isn’t available. * Retry attempts now preserve the calibrated timing behavior. * **Tests** * Expanded coverage for calibrated vs fallback timing profiles, WSL handling, and retry behavior, including updated probe request expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - NVIDIA#6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - NVIDIA#6271 and NVIDIA#6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - NVIDIA#6465, NVIDIA#6539, NVIDIA#6570, and NVIDIA#6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - NVIDIA#6523, NVIDIA#6551, NVIDIA#6484, NVIDIA#6488, NVIDIA#6324, and NVIDIA#6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - NVIDIA#6559, NVIDIA#6538, NVIDIA#6560, NVIDIA#6568, NVIDIA#6552, NVIDIA#6567, and NVIDIA#6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - NVIDIA#6541, NVIDIA#5415, NVIDIA#6246, NVIDIA#6496, and NVIDIA#6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - NVIDIA#6253, NVIDIA#6572, NVIDIA#6444, NVIDIA#6536, and NVIDIA#5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - NVIDIA#6508, NVIDIA#6527, NVIDIA#5506, NVIDIA#6588, NVIDIA#6446, NVIDIA#6447, NVIDIA#6582, NVIDIA#6296, NVIDIA#6367, NVIDIA#6397, and NVIDIA#6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests not applicable, release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Refs #3770.
Summary
NEMOCLAW_TRACEoutputNotes
/modelsrequest and treats any HTTP response as a useful transport timing sampleValidation
npx @biomejs/biome check src/lib/inference/probe-http-helpers.ts src/lib/inference/probe-http-helpers.test.ts src/lib/inference/onboard-probes.ts src/lib/onboard/inference-selection-validation.ts src/lib/onboard/inference-selection-validation.test.ts test/wsl2-probe-timeout.test.tsnpx vitest run --project cli --project integration src/lib/inference/probe-http-helpers.test.ts src/lib/inference/onboard-probes.test.ts src/lib/onboard/inference-selection-validation.test.ts test/wsl2-probe-timeout.test.tsnpm run build:clinpm run test-size:checknpm run test-conditionals:scan -- --top 25npm run check:diffSummary by CodeRabbit
New Features
Bug Fixes
Tests
Signed-off-by: Ho Lim subhoya@gmail.com