Repository navigation
test(e2e): tolerate Kimi final punctuation - #5625
Merged
Merged
Conversation
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)
📝 WalkthroughWalkthroughTwo e2e test files for Kimi inference compatibility are updated to tolerate a missing trailing period in the final assistant text. Both the shell script ( ChangesKimi Inference Compat — Final Text Assertion Normalization
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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 |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
This was referenced Jun 25, 2026
jyaunches
added a commit
that referenced
this pull request
Jun 25, 2026
## Summary Restore the Kimi-specific issue #5800 parity work for package `P0-D`; existing recovery and scope-upgrade package rows are explicitly mapped as pre-existing coverage and revalidated context, but not changed acceptance scope in this PR. ## Related Issues Refs #5800 Refs #5098 Refs #5342 Refs #5401 Refs #5406 Refs #5412 Refs #5413 Refs #5625 Refs #5760 ## Scope gate - Package: `P0-D — Recovery, Kimi, and scope-upgrade parity` - Included PRs all merged and touched `test/e2e`: yes - Changed acceptance scope in this PR: Kimi public-NVIDIA/mock parity (`D2`, `D3`) - Existing package rows revalidated without diff changes: recovery (`D1`) and scope-upgrade (`D4`) - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR #5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | D1 | #5342, #5401 | Recovery proxy env sourcing, missing proxy-env warning, guard retention, ciao/networkInterfaces preload, and crash-loop stability are pre-existing package coverage. | `hermetic-default` | Existing `test/e2e-scenario/live/issue-2478-crash-loop-recovery.test.ts`, `test/e2e-scenario/support-tests/e2e-recovery-helpers.test.ts`; selective run `28186561267` job `issue-2478-crash-loop-recovery-vitest` passed. No diff changes here. | existing / revalidated context | | D2 | #5401 | Kimi remains a public-NVIDIA model/provider contract when run in trusted selective CI, while retaining mock fallback for local/untrusted validation. | `public-nvidia required` | `.github/workflows/e2e-vitest-scenarios.yaml`, `test/e2e-scenario/live/kimi-inference-compat.test.ts`, `test/e2e-scenario/support-tests/kimi-inference-compat-helpers.test.ts`, `test/e2e-script-workflow.test.ts` | covered / changed | | D3 | #5413, #5625 | Kimi multiturn tool calls split `hostname; date; uptime`, preserve tool-result flow, reject abandoned/continue traces, and normalize final punctuation. | `public-nvidia required` with mock fallback | `test/e2e-scenario/live/kimi-inference-compat-helpers.ts` trajectory assertions; selective run `28190216767` job `kimi-inference-compat-vitest` passed on the previous head; latest run `28193896380` passed on `f36fef6da`. | covered / changed | | D4 | #5406, #5412, #5760 | Scope-upgrade approval tolerates preapproved / not-reproduced states, denies `operator.admin` leakage, stays on gateway/no embedded fallback, and accepts whitespace-normalized `42`; this is pre-existing package coverage. | `hosted-compatible capable` | Existing `test/e2e-scenario/live/issue-4462-scope-upgrade-approval.test.ts`; selective run `28186561267` job `issue-4462-scope-upgrade-approval-vitest` passed. No diff changes here. | existing / revalidated context | ## Inference mode support - Default mode for touched live target: Kimi `mock` unless workflow selects `public-nvidia`. - Real inference support preserved: yes for Kimi public NVIDIA; yes for existing scope-upgrade hosted-compatible; not required for recovery. - Modes validated in this PR: Kimi public NVIDIA via selective workflows `28188683830`, `28190216767`; latest follow-up validation `28193896380` is running for head `f36fef6da`. Kimi helper/mock behavior via local support tests. - Source-of-truth contract: `NEMOCLAW_E2E_INFERENCE_MODE` is the canonical selector; absent selector defaults to mock for local/untrusted validation; unknown explicit values now fail closed; legacy `NEMOCLAW_KIMI_USE_MOCK=0` remains only as a temporary shell-lane compatibility alias until shell retirement. - Secret boundary: public Kimi workflow passes only `NVIDIA_API_KEY`; helper probe envs are secret-free by default; raw public NVIDIA key handoff is limited to onboard; sandbox `openclaw agent` now runs with a secret-free env and uses the configured `nvidia-prod` route. ## Validation - [x] `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/kimi-inference-compat-helpers.test.ts` - [x] `npx vitest run test/e2e-script-workflow.test.ts` - [x] `npm run typecheck:cli` - [x] `npm run test-conditionals:scan -- --top 25` - [x] `npx prek run --all-files --stage pre-push --skip tsc-plugin --skip tsc-js --skip tsc-cli --skip version-tag-sync --skip test-cli --skip test-plugin --skip source-shape-test-budget --skip test-file-size-budget --skip test-skills-yaml` - [x] `git diff --check` - [x] Kimi selective E2E / Vitest Scenarios on previous head: https://github.com/NVIDIA/NemoClaw/actions/runs/28190216767 - [x] Kimi selective E2E / Vitest Scenarios after review-gap fixes: https://github.com/NVIDIA/NemoClaw/actions/runs/28193896380 - [x] Existing recovery/scope rows revalidated in selective run: https://github.com/NVIDIA/NemoClaw/actions/runs/28186561267 (`issue-2478-crash-loop-recovery-vitest` ✅, `issue-4462-scope-upgrade-approval-vitest` ✅; Kimi in that stale run was superseded) - [ ] Local live mock Kimi: attempted but blocked by local Docker daemon unavailable (`Cannot connect to the Docker daemon at unix:///Users/jyaunches/.docker/run/docker.sock`). CI selective run is the live validation path for this head. ## Follow-ups / waivers - None. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for running Kimi compatibility e2e checks in either mock or public NVIDIA mode. * The live scenario now adapts its setup, redaction, and traffic validation based on the selected mode. * **Bug Fixes** * Improved handling of API key propagation so public NVIDIA runs use the expected credentials without exposing secrets in other paths. * **Tests** * Added coverage for mode selection, API key validation, workflow environment wiring, and the new public NVIDIA Vitest lane. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Restore the Kimi-specific issue NVIDIA#5800 parity work for package `P0-D`; existing recovery and scope-upgrade package rows are explicitly mapped as pre-existing coverage and revalidated context, but not changed acceptance scope in this PR. ## Related Issues Refs NVIDIA#5800 Refs NVIDIA#5098 Refs NVIDIA#5342 Refs NVIDIA#5401 Refs NVIDIA#5406 Refs NVIDIA#5412 Refs NVIDIA#5413 Refs NVIDIA#5625 Refs NVIDIA#5760 ## Scope gate - Package: `P0-D — Recovery, Kimi, and scope-upgrade parity` - Included PRs all merged and touched `test/e2e`: yes - Changed acceptance scope in this PR: Kimi public-NVIDIA/mock parity (`D2`, `D3`) - Existing package rows revalidated without diff changes: recovery (`D1`) and scope-upgrade (`D4`) - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR NVIDIA#5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | D1 | NVIDIA#5342, NVIDIA#5401 | Recovery proxy env sourcing, missing proxy-env warning, guard retention, ciao/networkInterfaces preload, and crash-loop stability are pre-existing package coverage. | `hermetic-default` | Existing `test/e2e-scenario/live/issue-2478-crash-loop-recovery.test.ts`, `test/e2e-scenario/support-tests/e2e-recovery-helpers.test.ts`; selective run `28186561267` job `issue-2478-crash-loop-recovery-vitest` passed. No diff changes here. | existing / revalidated context | | D2 | NVIDIA#5401 | Kimi remains a public-NVIDIA model/provider contract when run in trusted selective CI, while retaining mock fallback for local/untrusted validation. | `public-nvidia required` | `.github/workflows/e2e-vitest-scenarios.yaml`, `test/e2e-scenario/live/kimi-inference-compat.test.ts`, `test/e2e-scenario/support-tests/kimi-inference-compat-helpers.test.ts`, `test/e2e-script-workflow.test.ts` | covered / changed | | D3 | NVIDIA#5413, NVIDIA#5625 | Kimi multiturn tool calls split `hostname; date; uptime`, preserve tool-result flow, reject abandoned/continue traces, and normalize final punctuation. | `public-nvidia required` with mock fallback | `test/e2e-scenario/live/kimi-inference-compat-helpers.ts` trajectory assertions; selective run `28190216767` job `kimi-inference-compat-vitest` passed on the previous head; latest run `28193896380` passed on `f36fef6da`. | covered / changed | | D4 | NVIDIA#5406, NVIDIA#5412, NVIDIA#5760 | Scope-upgrade approval tolerates preapproved / not-reproduced states, denies `operator.admin` leakage, stays on gateway/no embedded fallback, and accepts whitespace-normalized `42`; this is pre-existing package coverage. | `hosted-compatible capable` | Existing `test/e2e-scenario/live/issue-4462-scope-upgrade-approval.test.ts`; selective run `28186561267` job `issue-4462-scope-upgrade-approval-vitest` passed. No diff changes here. | existing / revalidated context | ## Inference mode support - Default mode for touched live target: Kimi `mock` unless workflow selects `public-nvidia`. - Real inference support preserved: yes for Kimi public NVIDIA; yes for existing scope-upgrade hosted-compatible; not required for recovery. - Modes validated in this PR: Kimi public NVIDIA via selective workflows `28188683830`, `28190216767`; latest follow-up validation `28193896380` is running for head `f36fef6da`. Kimi helper/mock behavior via local support tests. - Source-of-truth contract: `NEMOCLAW_E2E_INFERENCE_MODE` is the canonical selector; absent selector defaults to mock for local/untrusted validation; unknown explicit values now fail closed; legacy `NEMOCLAW_KIMI_USE_MOCK=0` remains only as a temporary shell-lane compatibility alias until shell retirement. - Secret boundary: public Kimi workflow passes only `NVIDIA_API_KEY`; helper probe envs are secret-free by default; raw public NVIDIA key handoff is limited to onboard; sandbox `openclaw agent` now runs with a secret-free env and uses the configured `nvidia-prod` route. ## Validation - [x] `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/kimi-inference-compat-helpers.test.ts` - [x] `npx vitest run test/e2e-script-workflow.test.ts` - [x] `npm run typecheck:cli` - [x] `npm run test-conditionals:scan -- --top 25` - [x] `npx prek run --all-files --stage pre-push --skip tsc-plugin --skip tsc-js --skip tsc-cli --skip version-tag-sync --skip test-cli --skip test-plugin --skip source-shape-test-budget --skip test-file-size-budget --skip test-skills-yaml` - [x] `git diff --check` - [x] Kimi selective E2E / Vitest Scenarios on previous head: https://github.com/NVIDIA/NemoClaw/actions/runs/28190216767 - [x] Kimi selective E2E / Vitest Scenarios after review-gap fixes: https://github.com/NVIDIA/NemoClaw/actions/runs/28193896380 - [x] Existing recovery/scope rows revalidated in selective run: https://github.com/NVIDIA/NemoClaw/actions/runs/28186561267 (`issue-2478-crash-loop-recovery-vitest` ✅, `issue-4462-scope-upgrade-approval-vitest` ✅; Kimi in that stale run was superseded) - [ ] Local live mock Kimi: attempted but blocked by local Docker daemon unavailable (`Cannot connect to the Docker daemon at unix:///Users/jyaunches/.docker/run/docker.sock`). CI selective run is the live validation path for this head. ## Follow-ups / waivers - None. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for running Kimi compatibility e2e checks in either mock or public NVIDIA mode. * The live scenario now adapts its setup, redaction, and traffic validation based on the selected mode. * **Bug Fixes** * Improved handling of API key propagation so public NVIDIA runs use the expected credentials without exposing secrets in other paths. * **Tests** * Added coverage for mode selection, API key validation, workflow environment wiring, and the new public NVIDIA Vitest lane. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
wscurran
added
area: e2e
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
Relax the Kimi E2E final-answer assertion so the trajectory acceptance tolerates a missing trailing period while still requiring the expected sentence. This keeps the test focused on the tool-call split and successful final response instead of brittle punctuation from the hosted model.
Changes
test/e2e/test-kimi-inference-compat.shbefore comparing the expected Kimi completion sentence.test/e2e-scenario/live/kimi-inference-compat-helpers.ts.Type of Change
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit