[Yunnglin]

PR type

• Bug Fix
• New Feature
• Document Updates
• More Models or Datasets Support

PR information

Seven server defects reproduced on main@96dab686 are fixed here, one commit per defect. Each fix is minimal (root cause only), covered by a regression test, and re-verified against a live Ray Serve deployment.

1. checkpoint delete could remove parent directories

Root cause CheckpointManager.delete() only rejected .. and then joined checkpoint_id as a relative path, so weights, sampler_weights or . deleted real weight directories.

Fix delete() now resolves the checkpoint metadata first and returns False unless checkpoint_id names a real checkpoint. Aliases (latest), plain directories and traversal are rejected. Twinkle and Tinker routes share the implementation.

2. Concurrent add_adapter_to_model bypassed full-mode exclusivity

Root cause The exclusivity check ran before queueing, while the resource registration happened inside the task after await state.register_model(...), so two requests could both pass the check.

Fix New ModelManagement.claim_full_mode_resource() performs check + registration without an await in between; the full-mode creation task calls it right after register_model, making the transition atomic.

3. OpenAI non-streaming responses had empty choices

Root cause /twinkle/sample answers with a TaskEnvelope, but the non-streaming branch read the top-level samples key and never polled an unfinished task.

Fix The envelope is parsed and, when non-terminal, polled through the shared lifecycle state until it reaches a terminal status. completed is translated as before; failed/cancelled map to OpenAI-shaped errors; a completed task without samples returns 502 instead of an empty 200.

4. Dedup claim TTL did not cover queue wait

Root cause TTL was effective_execution_timeout + 60, so a claim expired while the first copy was still queued.

Fix TTL is now queue_timeout + effective_execution_timeout + 60.

5. One rank failing the loss check hung the other rank

Root cause Each rank ran forward → loss → backward independently; a rank that raised at the loss check left the peer entering DDP backward collective alone.

Fix Losses gained validate_inputs(); GRPOLoss validates the alignment rules for the whole batch. The shared backend base class exposes preflight_loss_inputs with remote_function(dispatch='all', collect='none'), so every backend (Transformers and Megatron) inherits one implementation, and it also understands the tinker payload (set the loss up from loss_fn, convert datums, extract their RL features). All three entry points call it before the sliced backend: /twinkle/forward_backward, /twinkle/forward_backward_from_data_plane, and /tinker/forward_backward (GRPO batches only; plain SFT pays no extra actor round trip). Every actor validates the complete batch before any rank enters forward, so failures are symmetric and fast. A backend without the hook now logs a warning instead of silently skipping the check.

6. Client lost the underlying cause of a task failure

Root cause The server summarised a RayTaskError by its first line (the Ray invocation header), and TaskFailedError dropped ErrorPayload.traceback.

Fix The worker now summarises the wrapped cause (type: str(cause)), and TaskFailedError carries the remote traceback (populated by _future._unwrap).

7. Automatic template setup always returned 422

Root cause _ensure_template put an undeclared model_id at the top level of the sampler's strict request schema.

Fix Only template_cls and adapter_name are sent; the sampler injects its own model_id when the template constructor needs one. The per-model cache is still only populated on 200.

Review follow-ups

• Bounded the non-stream sampler wait. The new polling loop had no total budget and never checked the client, so a stuck task could keep a gateway coroutine (and its shared-state reads) alive for the record's 24h retention. It now honours TWINKLE_OPENAI_SAMPLE_TIMEOUT (default 1800s), stops on disconnect, and answers 504/timeout; a lost future record answers 500 instead of 404-with-server_error.
• Unwrap only RayTaskError in the failure summary. Taking __cause__ for every exception replaced deliberate raise X from e messages; the summary now unwraps Ray's transport wrapper only.
• Dropped a duplicated terminal-status set in the OpenAI handlers in favour of protocol.types.TERMINAL_STATUSES (the gateway use-case layer keeps its own copy by design; a guard test forbids protocol imports there).
• Strengthened the tests: the full-mode test now drives two real add_adapter_to_model calls through the route (reverting the handler to register_resource fails it), the dedup TTL expectation is derived from the configured windows, the preflight test uses three inputs and asserts the full batch reaches preflight (plus a negative SFT case), and the backend entry point is asserted for all four concrete model classes.

Experiment results

Environment: PPU, twinkle_ppu_vllm. Earlier runs used A (LoRA + sampler), B (full mode) and C (LoRA DP=2); the tinker/Megatron checks used a 2-GPU Qwen3.5-4B Megatron DP=2 deployment and the same A cluster with the Transformers backend.

• BUG-01 (CPU): out-of-scope deletes (weights, sampler_weights, .) all return 404, run directory and weights intact.
• BUG-02: two concurrent creations → one success, one FullModeBusyError, /twinkle/status reports models == 1.
• BUG-03: non-streaming choices non-empty ("2 + 3 equals 5.", usage 40/9/49); streaming unchanged.
• BUG-04 (CPU): claim TTL 2160s, second submit dedup_hit=True, enqueued_copies=1.
• BUG-05: four normal forward_backward calls and clip_grad_and_step succeed; a malformed call fails in 0.25s; zero_grad and the next forward_backward succeed; deep health stays 200 with no timeout.
• BUG-05 on the tinker route (/tinker/forward_backward, Megatron DP=2, Qwen3.5-4B): baseline step succeeds; a partially malformed GRPO batch fails in 0.75s with the alignment assertion; the next batch succeeds in 1.05s; deep health 200.
• BUG-05 on the tinker route (Transformers): malformed batch fails in 0.65s, the next batch succeeds in 0.65s, deep health 200.
• BUG-06: malformed call fails in 0.09s; str(e), e.traceback, and the HTTP envelope's error.error/error.traceback all contain mismatch; recovery training and deep health succeed.
• BUG-07: the sampler's strict schema returns 422 for the old body (details point at body.model_id) and 200 for the body the gateway now sends; no gateway-originated set_template 422 in the server log and two consecutive OpenAI requests succeed.

Tests: pytest tests/server tests/loss tests/twinkle_client → 787 passed, 71 skipped, 1 failed. The single failure (test_liger_fused_linear_ce.py::test_fused_kernel_success_does_not_mark_broken) reproduces on a clean 96dab686 worktree — the fused liger kernel is unsupported on this device and falls back — so it is unrelated to this PR. pre-commit run --files <changed> passes.

Scope notes

• BUG-05 covers the twinkle-native routes on both backends and the tinker route; the same full-batch preflight is inherited by TwinkleCompatMegatronModel, TwinkleCompatFullMegatronModel, TwinkleCompatTransformersModel and TwinkleCompatFullTransformersModel.
• BUG-07: the sampler already sets its own template at construction, so the gateway's _ensure_template is a safety net; the fix guarantees the body it sends is schema-valid.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.