fix(e2e): select Pi qualification for image inputs - #10523
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
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)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughPi qualification targets now own all files copied into Pi Docker images. End-to-end tests validate Dockerfile source ownership and confirm that copied Nemotron blueprint changes select both architecture-specific targets. ChangesPi image ownership
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to This PR narrowly updates Pi qualification ownership and adds regression coverage for the Dockerfile inputs; no actionable merge-blocking risk remains after normal checks and review. 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 finished for commit |
rsliter
left a comment
There was a problem hiding this comment.
Non-blocking sequencing note: #10390 removes derivePiImageSourcePaths and moves Pi source parsing to the canonical directDockerfileCopySources helper. If #10390 lands first, rebase this PR and preserve its two new target-ownership assertions using that helper. I am not approving this revision because the required checks gate fails in CLI shards 8 and 10. The failures appear outside this PR’s changed paths: shard 8 failed with spawn node ENOENT in e2e-fixture-context.test.ts, and shard 10 timed out in profile-list.test.ts.
Outcome
Changes to any checked-in source copied by the Pi Dockerfiles now select both AMD64 and ARM64 Pi qualification targets. A regression test reads the real Dockerfiles, so a future
COPYinput cannot silently reuse a stale candidate image receipt without selecting Pi validation.Reason
Both Pi jobs in main E2E run 33111284865 rejected a stale image receipt after
nemoclaw-blueprint/scripts/nemotron-inference-fix.jschanged. The Pi targets only ownedagents/pi/, so that copied blueprint source did not select either Pi qualification job on the introducing PR.Related issues
Part of #10153
Relates to #10390
Changes
COPYinputs to the shared Pi target ownership boundary.pi-agent-qualification-amd64andpi-agent-qualification-arm64.Verification
npx vitest run --project e2e-support test/e2e/support/pi-agent-qualification-events.test.ts— 11 tests passed after rebasing onto currentorigin/main.npm run check:diff— passed, including repository checks, codebase growth guardrails, commitlint, and CLI TypeScript.valid.Review notes
This is complementary to #10390. That PR refreshes the Pi qualification receipts and runtime assertions; this PR adds the missing source-to-target selection boundary that allowed those receipts to become stale. If #10390 lands first, the shared support-test change may need a mechanical rebase.
Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests