feat(skippy): add Poolside Laguna staged-runtime candidate - #1095
feat(skippy): add Poolside Laguna staged-runtime candidate#1095michaelneale wants to merge 20 commits into
Conversation
📝 WalkthroughWalkthroughThe changes add source-revision propagation and upstream license metadata to model packaging, introduce TCP prediction-return handling for correctness runs, and add Laguna family inference, staged llama.cpp execution, validation, and parity records. ChangesRevision-aware model packaging
Prediction return listener integration
Laguna staged execution support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
![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 `@crates/mesh-llm-commands/src/model_package.rs`:
- Line 99: Update run_list_quants so every listed quant preserves the requested
revision: emit examples using the repo@revision:quant format and include
sourceRevision in JSON output. Apply this consistently to both output paths,
using the existing source_revision value.
In `@crates/skippy-correctness/src/runner/prediction_return.rs`:
- Around line 75-82: Ensure PredictionReturnListener::drop cannot block
indefinitely when a connected peer stalls: add a read timeout to the accepted
stream and update the handshake/reply loop using read_stage_message and
recv_reply to surface timeout or WouldBlock results, re-checking shutdown and
exiting promptly. Preserve normal handshake and reply processing while allowing
thread.join() to complete after shutdown.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c25d1f26-957c-42f0-a4ef-35dad64e550e
📒 Files selected for processing (16)
crates/mesh-llm-commands/src/model_package.rscrates/model-package/src/bin/queue-unsloth-layer-packages.rscrates/model-package/src/jobs.rscrates/model-package/src/prepare.rscrates/model-package/src/script.rscrates/model-package/src/scripts/split-model-job.shcrates/skippy-correctness/src/runner/mod.rscrates/skippy-correctness/src/runner/prediction_return.rscrates/skippy-correctness/src/runner/split_chain.rscrates/skippy-topology/src/family_capability.rscrates/skippy-topology/src/lib.rscrates/skippy-topology/src/tests.rsdocs/skippy/FAMILY_STATUS.mddocs/skippy/LLAMA_PARITY.mddocs/skippy/llama-parity-candidates.jsonthird_party/llama.cpp/patches/0046-Support-Laguna-staged-execution.patch
| // This path doesn't need HF_TOKEN — works for public repos. | ||
| if source_quant.is_none() { | ||
| return run_list_quants(&hf_client, source_repo, json).await; | ||
| return run_list_quants(&hf_client, source_repo, source_revision, json).await; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the selected revision in list output.
The quants are now discovered at the requested revision, but the follow-up example still emits repo:quant, and JSON omits the revision. Copying that example can package a different commit. Use the existing repo@revision:quant form and include sourceRevision in JSON.
Proposed fix
serde_json::to_string_pretty(&json!({
"sourceRepo": source_repo,
+ "sourceRevision": source_revision,
"quants": quants,
}))?
- eprintln!(" mesh-llm models package {}:{}", source_repo, quants[0].name);
+ let source_ref = source_revision
+ .map(|revision| format!("{source_repo}@{revision}:{}", quants[0].name))
+ .unwrap_or_else(|| format!("{source_repo}:{}", quants[0].name));
+ eprintln!(" mesh-llm models package {source_ref}");Also applies to: 256-262
🤖 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 `@crates/mesh-llm-commands/src/model_package.rs` at line 99, Update
run_list_quants so every listed quant preserves the requested revision: emit
examples using the repo@revision:quant format and include sourceRevision in JSON
output. Apply this consistently to both output paths, using the existing
source_revision value.
| impl Drop for PredictionReturnListener { | ||
| fn drop(&mut self) { | ||
| self.shutdown.store(true, Ordering::SeqCst); | ||
| if let Some(thread) = self.thread.take() { | ||
| let _ = thread.join(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Drop can hang indefinitely if the peer stalls after the handshake.
shutdown is only checked while polling the nonblocking accept() loop (lines 92-97). Once a connection is accepted, the stream is switched to blocking mode (lines 102-104) and the handshake/reply loop (read_stage_message, recv_reply) has no timeout and never re-checks shutdown. If the connected peer stops sending data without closing the socket, Drop::drop (lines 75-82) will block forever in thread.join().
Today this is only avoided because, in split_chain.rs, the ChildGuards for the stage processes happen to be declared (and thus dropped/killed) before PredictionReturnListener — an implicit ordering invariant that isn't documented or enforced here, and would silently break under a future refactor.
🔒 Proposed fix: apply a read timeout and re-check shutdown in the reply loop
stream
.set_nonblocking(false)
.context("set direct prediction return stream blocking")?;
+ stream
+ .set_read_timeout(Some(Duration::from_millis(200)))
+ .context("set direct prediction return read timeout")?;
consume_optional_client_ready_hello(&mut stream)?;
send_ready(&mut stream).context("send direct prediction return ready")?;
stream.flush().ok();
- let open =
- read_stage_message(&mut stream, 0).context("read direct prediction return open message")?;
+ let open = loop {
+ match read_stage_message(&mut stream, 0) {
+ Ok(message) => break message,
+ Err(error)
+ if matches!(error.kind(), io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut) =>
+ {
+ if shutdown.load(Ordering::SeqCst) {
+ return Ok(());
+ }
+ }
+ Err(error) => return Err(error).context("read direct prediction return open message"),
+ }
+ };
if open.kind != WireMessageKind::PredictionReturnOpen {
bail!("expected direct prediction return open message");
}
loop {
match recv_reply(&mut stream) {
Ok(reply) => {
if sender.send(Ok(reply)).is_err() {
return Ok(());
}
}
+ Err(error)
+ if matches!(error.kind(), io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut) =>
+ {
+ if shutdown.load(Ordering::SeqCst) {
+ return Ok(());
+ }
+ }
Err(error) if error.kind() == io::ErrorKind::UnexpectedEof => return Ok(()),
Err(error) => return Err(error).context("read direct prediction return reply"),
}
}(Requires read_stage_message/recv_reply error kinds to surface WouldBlock/TimedOut, or alternatively store a cloneable handle to the stream so Drop can call stream.shutdown(Shutdown::Both) to force-unblock the read.)
Also applies to: 84-124
🤖 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 `@crates/skippy-correctness/src/runner/prediction_return.rs` around lines 75 -
82, Ensure PredictionReturnListener::drop cannot block indefinitely when a
connected peer stalls: add a read timeout to the accepted stream and update the
handshake/reply loop using read_stage_message and recv_reply to surface timeout
or WouldBlock results, re-checking shutdown and exiting promptly. Preserve
normal handshake and reply processing while allowing thread.join() to complete
after shutdown.
|
Review findings at
The latter two overlap CodeRabbit comments and are independently confirmed. |
|
Reviewed the update range
The router/wrapper propagation added in this commit otherwise looks internally consistent. |
Extract the recurrent checkpoint work from the Inkling branch into the common llama.cpp patch queue. Snapshot partial recurrent state, trim hybrid attention directly, retain pipelined checkpoints, and replay only the accepted prefix. This avoids copying the full long-context attention cache for each speculative verify window. Assisted-by: codex
A generation that failed mid-decode on a split topology (e.g. a runtime trim error) tore down the binary stage connection before the graceful Stop message, so the stage never called drop_session_timed for that session. The leaked RuntimeState lane stayed 'active' forever; retried requests used fresh session ids, so leaked lanes accumulated until every admission wedged (EAGAIN fast-fails / reply timeouts) and only a process restart recovered. Track sessions created per binary connection and drop any that never saw a graceful Stop when the connection handler exits, returning their execution lanes to the pool so the next request gets a fresh lane. Assisted-by: goose
Keep orphaned-lane reclamation separate from the already oversized binary connection message loop, with its focused unit tests beside the tracker. Assisted-by: codex
|
The three review findings on the Laguna/package change are fixed at
Validation completed successfully:
The later timeout-cancellation review has two separate, still-open generic findings. The PR description now calls those out explicitly; they are not being treated as resolved by this commit. |
i386
left a comment
i386
left a comment
There was a problem hiding this comment.
Review of current head 624fc7b9.
[blocking] Add an explicit Laguna cache-policy assertion. The new reviewed laguna capability is not covered by family_policy::tests::every_reviewed_family_has_an_explicit_cache_policy; the current CI run reproduces this as reviewed family laguna has no explicit policy assertion. Please add the Laguna policy expectation (and keep its intended K/V defaults explicit) so adding it to the reviewed catalog cannot silently leave the loader/planner policy unguarded.
The rest of this pass found no additional blocking issue.
Paused Vast-only default-policy checkpoint (2026-07-29)The follow-on was deliberately paused before inference. This is a partial startup result, not a completed runtime gate. What completed:
An important lab finding: without Not completed or claimed:
Exact resume gate:
Cleanup completed: both Poolside processes exited cleanly, and the Poolside builder, 5090, and Pro 6000 instances were destroyed. The existing public-mesh RTX 3090 remains running and untouched. |
|
Combined replacement branch is now open as #1118. It carries the current work from this PR onto current |
|
Superseded by #1118 |
3 participants
Status
This draft adds staged-runtime support for the pinned Poolside Laguna S 2.1 Q4_K_M artifact and records real M5 + Vast Mesh evidence. The model has passed strict local two- and three-stage parity and ordinary two-node inference over Iroh/QUIC.
Suffix N-gram depth
2is now published in the pinned package as its default serving policy, and this branch teaches the runtime to consume that package field. A no-config resolver test proves the package selects suffix N-gram and depth2. A Vast-only cold start on the exact revision reached stable direct-Iroh placement and downstream-stage readiness, then was deliberately paused before stage 0 finished downloading or any inference ran. Live package-default consumption therefore remains unproven.The PR remains a draft because it also carries generic dependencies and timeout-cancellation changes with open review findings. Those must land or be extracted before merge.
Pinned artifact
poolside/Laguna-S-2.1-GGUF@edd093522473dc7313b0738d8b4116b7f8b9745fa34c74e46688122bef83122f4133031bababbefcf57436dde97048c91e2cc6ffmeshllm/laguna-s-2.1-Q4_K_M-layers@0c467ad441ee94cb5a76f626294d963c4048507d0250cfb54ceeb94a9c71e48df447f780e32fc625553844d6403770f315be02375..32, proposal cap48, fixed verify window1..32, pipeline depth2The Hugging Face policy update is immutable at commit
0c467ad. All evidence is limited to this Q4_K_M artifact and the revisions above.What changes
pipeline_depth; omission preserves the legacy depth of1What is proven
Strict local correctness on M5 Max / Metal
0..24 / 24..48: full and staged execution both predicted token6740..16 / 16..32 / 32..48: full and staged execution both predicted token6740without allowing mismatchesReal M5 + Australian Vast Mesh serving
mesh-llmnodes ran on M5 Max/Metal and an Australian Vast RTX 6000 Ada/CUDA worker0..36 / 36..48262144, F16 activation wire, F16 KV, four lanes,n_batch=512, andn_ubatch=1282RTT varied materially during the run, including transient spikes; this PR does not claim a single fixed latency figure.
Published default policy
pipeline_depthresolve to depth1ngram-suffixand verify pipeline depth2without a local speculative overrideWhat is not proven
In the failed tool cases Laguna often emitted textual
<tool_call>content that was not projected into OpenAItool_calls; one sequential response ended withfinish_reason=stop. This is not a native split crash, but it means this PR does not claim tool-use certification.Remaining merge work
0c467ad…, with explicit--max-vramcaps matching physical GPU memory and no speculative strategy/depth override. Wait for both stages, run a small smoke plus the 44,460-token N-gram request, and require telemetry to report effectivengram-suffix, pipeline depth2, successful inference, and clean shutdown. Metal correctness is already covered by strict M5 parity; a fresh M5 live launch can follow if desired.Validation
just buildjust release-buildskippy-correctness single-stepand three-stagechaincargo test -p skippy-model-package --bin skippy-model-package— 51 passedcargo test -p skippy-topology --lib— 33 passedcargo test -p mesh-llm-host-runtime --lib package_suffix_strategy_resolves_as_a_standalone_proposer— passedcargo check -p skippy-model-package -p skippy-runtime -p skippy-topology -p mesh-llmcargo clippy -p skippy-model-package -p skippy-runtime -p skippy-topology -p mesh-llm --all-targets -- -D warningscargo fmt --all --checkTracks #1090. Depends on #1100.