test(e2e): harden full inference probe - #8269
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
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. |
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a shared inference probe for model-specific requests, response parsing, bounded retries, failure classification, integer-42 matching, and evidence generation. The live E2E test uses the probe, and support tests cover its behavior. ChangesInference probe
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant full-e2e.test.ts
participant runFullE2eInferenceProbe
participant sandbox inference command
participant inference.local
full-e2e.test.ts->>runFullE2eInferenceProbe: start inference probe
runFullE2eInferenceProbe->>sandbox inference command: execute generated request
sandbox inference command->>inference.local: send non-streaming inference request
inference.local-->>sandbox inference command: return completion response
sandbox inference command-->>runFullE2eInferenceProbe: return command result
runFullE2eInferenceProbe-->>full-e2e.test.ts: return outcome and evidence
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
1 terminology difference from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit d4471a2 in the TypeScript / code-coverage/cliThe overall coverage in commit d4471a2 in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@test/e2e/live/full-e2e.test.ts`:
- Around line 497-501: Update the sandbox inference evidence flow around
fullE2eInferenceProbeEvidence and artifacts.writeJson to project only an
allowlisted set of evidence fields, apply captureLimitBytes to potentially
unbounded content, reasoning_content, and usage data, and enforce a total byte
limit before storage. Pass the bounded projection to writeJson while preserving
the existing artifact name and inference-attempt evidence.
In `@test/e2e/support/full-e2e-inference-probe.test.ts`:
- Around line 109-115: Expand the assertions for fullE2eInferenceProbeEvidence
in the successful probe case to verify the complete artifact contract, including
schemaVersion, attempt identifiers, token budgets, exit codes, model, reasoning
content, usage, and serialized parseError for both attempts. Add a
response-failure result through the public serializer boundary and assert its
parseError is retained.
🪄 Autofix
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: 2d45ee97-7707-4c43-b18d-4d66c2d249c4
📒 Files selected for processing (4)
test/e2e/live/full-e2e-inference-probe.tstest/e2e/live/full-e2e.test.tstest/e2e/mock-parity.jsontest/e2e/support/full-e2e-inference-probe.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
senthilr-nv
added
chore
senthilr-nv
left a comment
senthilr-nv
left a comment
There was a problem hiding this comment.
Reviewed at d4471a2. Focused probe tests, shared answer assertions, CLI type-checking, and E2E mock/live parity pass. No blocking correctness, security, or product-scope findings.
2 participants
Summary
Stabilize the full E2E sandbox inference probe after the OpenClaw
security-posture lane received a successful Nemotron completion containing
Theinstead of the requested integer42. The same job passed when rerun atthe same SHA, which identifies a live model-output flake rather than a product,
runner, credential, policy, or routing failure.
The probe now uses deterministic sampling where the model supports it, gives
reasoning models a larger bounded reply budget, retries one valid but
semantically wrong completion, and preserves structured completion evidence.
Runner types, workflow definitions, schedules, and E2E cadence are unchanged.
Failure Evidence
0d1cb93888c817daec44b2cc996afa75eebcbd46Changes
curl | pythonprojection with directcurlargv socommand/HTTP failures remain unambiguous and the raw response remains in the
normal redacted shell artifacts.
temperature: 0for models usingmax_tokens, use the shared tokenfield resolver for incompatible model families, and bound attempts at 512
then 1024 reply tokens.
does not contain integer
42. Transport, HTTP/auth/policy, invalid JSON, andinvalid completion-shape failures remain immediate.
parse error, answer match, model,
finish_reason, content,reasoning_content, and usage.Theresponse, reasoning-budgettruncation, strict failures, bounded exhaustion, unique artifacts, response
parsing, and the existing whitespace-tolerant
4\n2behavior.Type of Change
Quality Gates
probe and its diagnostics; CLI/API/configuration and supported user behavior
do not change.
onboarding, inference, runner, sandbox, or messaging)
reviewer/approval link/justification: Independent Codex Desktop review found
no correctness, credential, redaction, retry-masking, or security blocker.
ShellProbe redaction remains authoritative for command output and
ArtifactSink re-redacts the structured summary.
name, approval link, and follow-up issue:
Documentation Writer Review
no-docs-neededconfiguration, runner type, workflow cadence, or supported surface. Naming,
test titles, and diagnostic fields clearly describe the internal retry
boundary.
DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appearsas
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passedinference-probe tests and 3/3 shared answer-assertion tests passed.
probe change with focused fast coverage.
typecheck:cli, semantic E2E phase,project membership, title style, source-shape, test-size, mock-parity, Biome,
repository, and secret-scan checks passed.
npm run docsbuilds without warnings (doc changes only)guide (doc
changes only)
Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit