feat(research): add immutable prediction dispatcher - #247
Conversation
📝 WalkthroughWalkthroughAdds ChangesPrediction dispatch
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant alpha-harness
participant kubectl
participant Kubernetes
User->>alpha-harness: submit JSON with context and namespace
alpha-harness->>kubectl: check result identity
kubectl->>Kubernetes: list matching Jobs
Kubernetes-->>kubectl: existing Job metadata
alpha-harness->>kubectl: create Secret and ACK Job
kubectl->>Kubernetes: submit manifest
Kubernetes-->>alpha-harness: creation result
alpha-harness-->>User: submission status
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 `@rust_hft/alpha-harness/app/src/prediction_dispatch.rs`:
- Around line 87-93: The prediction input Secret creation flow around
kubectl_with_input and ensure_kubectl_success must be retry-safe. Handle
AlreadyExists by reading the existing Secret and verifying it matches the
deterministically rendered, digest-bound submission identity; proceed only on an
exact match, and fail closed on any mismatch or read error, while preserving
normal creation behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eeda8866-9a14-401e-bd6d-dd5c9685e086
📒 Files selected for processing (4)
deployment/aliyun/research/README.mdrust_hft/alpha-harness/app/src/cli.rsrust_hft/alpha-harness/app/src/main.rsrust_hft/alpha-harness/app/src/prediction_dispatch.rs
| let output = kubectl_with_input( | ||
| &args.context, | ||
| &args.namespace, | ||
| ["create", "-f", "-"], | ||
| &secret_body, | ||
| )?; | ||
| ensure_kubectl_success(output, "create immutable prediction input Secret")?; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Interrupted submit orphans the immutable input Secret and deadlocks retries.
The Secret is created (Line 87–93) before the Job, and ensure_result_available (Line 84) only inspects Jobs. If the process is killed between a successful Secret create and Job success, the immutable Secret is left behind with no Job. On retry the availability check passes (no Job), but kubectl create of the Secret then fails with AlreadyExists — and because it is immutable: true it cannot be replaced — leaving the operator stuck until they manually delete prediction-<label>-inputs.
Since the Secret content is deterministically derived from the (already validated, digest-bound) submission, consider making the Secret create idempotent: on AlreadyExists, read back the existing Secret and proceed only if it matches the rendered identity, otherwise fail closed. That keeps retries safe without a manual cleanup step.
🤖 Prompt for 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.
In `@rust_hft/alpha-harness/app/src/prediction_dispatch.rs` around lines 87 - 93,
The prediction input Secret creation flow around kubectl_with_input and
ensure_kubectl_success must be retry-safe. Handle AlreadyExists by reading the
existing Secret and verifying it matches the deterministically rendered,
digest-bound submission identity; proceed only on an exact match, and fail
closed on any mismatch or read error, while preserving normal creation behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f14892429a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "spec": { | ||
| "backoffLimit": 0, | ||
| "activeDeadlineSeconds": ACTIVE_DEADLINE_SECONDS, | ||
| "ttlSecondsAfterFinished": 86400, |
There was a problem hiding this comment.
When the same result URL is submitted after 24 hours and the operator has followed the README's terminal-state Secret cleanup, the TTL controller has deleted the only Job record queried by existing_result_jobs, so the duplicate check passes and launches the experiment again. The immutable OSS PUT prevents overwriting only after the duplicate run has consumed its resources; retain the result identity in a non-expiring resource or do not expire the duplicate ledger.
AGENTS.md reference: AGENTS.md:L93-L96
Useful? React with 👍 / 👎.
| validate_identifier("mission id", &submission.mission_id)?; | ||
| validate_identifier("evaluator version", &submission.evaluator_version)?; |
There was a problem hiding this comment.
When a submission's mission_id or evaluator_version differs from the authenticated mission, these syntax-only checks still accept it and neither expected value is passed to prediction execute. A valid mission can therefore complete while the Job annotations claim different identities from execution-evidence.json (and even a non-digest evaluator value such as foo is accepted), corrupting the immutable audit metadata; cross-check both fields after fetching the mission.
AGENTS.md reference: AGENTS.md:L50-L54
Useful? React with 👍 / 👎.
| ["create", "-f", "-"], | ||
| &secret_body, | ||
| )?; | ||
| ensure_kubectl_success(output, "create immutable prediction input Secret")?; |
There was a problem hiding this comment.
If the API server creates the Secret but kubectl reports an error because its response is lost or times out, this immediate return performs no readback. No Job is launched, and every retry passes the Job duplicate check but then fails because the deterministic Secret already exists, leaving the submission stuck until manual cleanup; reconcile an existing exact Secret just as the following Job-create path reconciles an ambiguous outcome.
AGENTS.md reference: AGENTS.md:L93-L96
Useful? React with 👍 / 👎.
| }, | ||
| "spec": { | ||
| "backoffLimit": 0, | ||
| "activeDeadlineSeconds": ACTIVE_DEADLINE_SECONDS, |
There was a problem hiding this comment.
For a valid prediction mission whose search_budget.max_seconds exceeds 1,800 seconds—the runtime accepts values up to 86,400—the Job controller reaches this deadline and terminates the container before the governed loop can finish. Because that kills alpha-harness externally rather than letting it observe the runner's exit, it also bypasses the promised result/evidence upload; either reject missions outside this resource profile before launch or set a deadline that covers the accepted budget plus staging and publication time.
AGENTS.md reference: AGENTS.md:L93-L96
Useful? React with 👍 / 👎.
| let mission_object = canonical_https_object("mission", &submission.mission_url)?; | ||
| let snapshot_object = canonical_https_object("snapshot", &submission.snapshot_url)?; | ||
| let result_object = canonical_https_object("result", &submission.result_put_url)?; |
There was a problem hiding this comment.
When the query-free result object equals the mission, snapshot, or resume object, all URL checks pass even though the immutable PUT cannot succeed: the input object already exists, so x-oss-forbid-overwrite: true rejects publication after the run has consumed its resources. This is especially easy during resume because GET and PUT signatures for the same object have different queries; compare the canonical object identities and require the output to be distinct from every input.
AGENTS.md reference: AGENTS.md:L50-L54
Useful? React with 👍 / 👎.
| let (repository, digest) = image | ||
| .rsplit_once("@sha256:") | ||
| .context("prediction image must be pinned by @sha256 digest")?; |
There was a problem hiding this comment.
When the repository portion is malformed, such as repo name@sha256:<64 hex> or repo@sha256:dead@sha256:<64 hex>, this split-and-digest check still accepts the submission because it only requires a nonempty prefix. Kubernetes can create the Job but the kubelet then reports an invalid image name and never starts the runner, while the Job reserves the result identity; parse and validate the complete OCI reference before creating cluster resources.
AGENTS.md reference: AGENTS.md:L50-L54
Useful? React with 👍 / 👎.
Parent: #240
Closes #242
Change contract
Add alpha-harness prediction dispatch render|submit so one authenticated, digest-bound prediction research submission deterministically renders and creates one immutable Secret plus one isolated Kubernetes Job, with fail-closed duplicate and create-outcome reconciliation.
Out of scope
Read-only status/evidence reporting (#246), collector or production deployment changes, snapshot construction, evaluator/MCTS behavior, promotion, and live execution.
Dependency or merge order
None. #246 may build on this after merge.
Focused validation
Rollout / rollback impact
No automatic rollout and no collector/runtime mutation. Operators must explicitly provide context and namespace to submit. Roll back by reverting this PR; Jobs already created remain independently auditable by immutable result identity.
Scope guardrail
4 files, 747 insertions and 2 deletions; below the 750 non-generated changed-line split threshold. Status remains isolated in #246.
Summary by CodeRabbit