Accept bounded external token proposals in local Skippy generation

[i386]

Embedded Skippy consumers can now attach any in-process source of causal, width-one token proposals and have the target runtime verify, commit, repair, and report each decision authoritatively.

The source receives only committed token history plus a strict deadline and bounded proposal budget. Skippy owns acceptance classification, emits the exact target-authoritative committed prefix, trims branch-conditioned suffix rows after a mismatch or callback stop, and reports timing and position evidence back against the source's opaque decision ID.

Architecture

• Adds the source-neutral LinearProposalIngress trait and bounded configuration.
• Admits proposals only for greedy local generation without grammar, generation hooks, or native MTP.
• Rejects late, empty, oversized, and invalid-token proposals before verification.
• Uses the native verify-window classifier as the sole acceptance authority.
• Repairs runtime position before surfacing callback failures and retires a session if repair fails.
• Leaves existing generation unchanged unless an ingress source is configured.

Protocol

This is an additive in-process API. It does not change the mesh wire protocol or the Skippy ABI.

Dependency

The diff is independent of #1078, but canonical Metal behavior for its batched target-verification call depends on #1078. This draft should merge after #1078.

Validation

• cargo fmt --all -- --check
• cargo test -p skippy-server --lib (309 passed)
• cargo check -p skippy-server
• cargo check -p mesh-llm
• cargo clippy -p skippy-server --all-targets -- -D warnings
• cargo clippy -p mesh-llm --all-targets -- -D warnings

Summary by CodeRabbit

• New Features
  • Added optional “linear proposal” ingress support for embedded OpenAI generation, including configurable deadlines and max token limits.
  • Added end-to-end proposal flow: query, local execution, disposition/receipt reporting, and terminal discard handling.
  • Exposed linear proposal types for consumption via public crate exports.
• Bug Fixes
  • Improved token verification and per-session token count tracking used during proposal processing.
• Tests
  • Added unit coverage for decision-id bounds, ingress config validation, query/discard behavior, and execution sequencing.

[github-actions]

This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds linear proposal ingress contracts, validation, local execution and repair, telemetry receipts, generation-loop integration, runtime token accounting, public exports, and embedded-serving configuration.

Changes

Linear proposal serving

Layer / File(s)	Summary
Proposal contracts and validation crates/skippy-server/src/frontend.rs, crates/skippy-server/src/frontend/linear_proposal.rs, crates/skippy-server/src/lib.rs	Defines proposal types, ingress traits and configuration, validation and discard behavior, admission checks, receipts, telemetry, and unit tests.
Local proposal execution crates/skippy-server/src/frontend/linear_proposal.rs, crates/skippy-server/src/runtime_state.rs	Verifies tokens, classifies proposal outcomes, repairs session state when needed, records receipt timings, and tracks session token counts.
Generation-loop integration crates/skippy-server/src/frontend/generation/*, crates/skippy-server/src/frontend/local_generation.rs, crates/skippy-server/src/frontend/native_mtp/mod.rs	Wires optional ingress configuration into stage backends and queries, executes and reports proposals during local decoding, updates committed context, and preserves native MTP exports.
Embedded-serving compatibility wiring crates/mesh-llm-host-runtime/src/inference/skippy/*, crates/skippy-server/src/binary_transport/binary_messaging.rs, crates/skippy-server/src/frontend/tests/multimodal.rs	Initializes the new optional ingress field across embedded argument builders, transport startup, and multimodal test backends.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LocalGeneration
  participant LinearProposalIngress
  participant StageOpenAiBackend
  participant RuntimeState
  LocalGeneration->>LinearProposalIngress: Query committed-token context
  LinearProposalIngress-->>LocalGeneration: Return LinearProposal
  LocalGeneration->>StageOpenAiBackend: Execute proposal
  StageOpenAiBackend->>RuntimeState: Verify and update tokens
  RuntimeState-->>StageOpenAiBackend: Return predicted tokens
  StageOpenAiBackend-->>LocalGeneration: Return LinearProposalReceipt
  LocalGeneration->>LinearProposalIngress: Report receipt

Loading

Possibly related PRs

• Mesh-LLM/mesh-llm#1080: Shares embedded OpenAI configuration and local generation-path changes.

Suggested labels: experimental

Suggested reviewers: michaelneale

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.98% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main change: accepting bounded external token proposals during local Skippy generation.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch agent/linear-proposal-ingress

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

crates/skippy-server/src/frontend/linear_proposal.rs (1)

558-599: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting the repair block into a named helper.

execute_local_linear_proposal_inner is ~140 lines and mixes verification, callback commit, and suffix repair/retire. The repair closure (Lines 568-599) is self-contained — trim_session, position re-check, and session retirement — and would read better as e.g. trim_branch_suffix_or_retire(...) returning the timing tuple, keeping the outer function within the configured Clippy complexity limits.

As per coding guidelines: "Do not add Rust methods or functions over the configured Clippy line-count or cognitive-complexity limits; split complex logic into semantically named helpers."

🤖 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-server/src/frontend/linear_proposal.rs` around lines 558 - 599,
Extract the self-contained repair closure in execute_local_linear_proposal_inner
into a semantically named helper such as trim_branch_suffix_or_retire, passing
the runtime, session ID, and canonical position it needs. Have the helper
perform trim_session, validate the repaired token count, retire the session on
failure or mismatch, and return the repair timing values; update the caller to
apply those results while preserving existing error messages and behavior.

Source: Coding guidelines

🤖 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/skippy-server/src/frontend/linear_proposal.rs`:
- Around line 283-312: Make the deadline behavior explicit in
LinearProposalIngressConfig::new documentation by stating that deadline is
advisory and source::propose is responsible for honoring it. Add distinct
telemetry for proposals that return after deadline, alongside the existing
LinearProposalDiscardReason::DeadlineExceeded discard path, using the measured
proposal_elapsed_us and preserving the current discard and return behavior.
- Around line 396-407: Remove the #[allow(clippy::too_many_arguments)]
attributes from both affected functions, including
StageOpenAiBackend::execute_local_linear_proposal and the second function near
the same change. Introduce a small parameter struct for the coherent
session/decode values (session_id, current, base_position, generated_len, and
max_new_tokens), update callers and function bodies to use it, and preserve
existing behavior without warning suppression.

In `@crates/skippy-server/src/frontend/local_generation.rs`:
- Around line 606-609: Update the receipt-reporting call in the local generation
flow around config.source().report(&receipt) so reporting failures are non-fatal
after tokens have been committed and streamed: log the error and continue, or
pass the failure through the existing terminal-discard handling path. Do not
propagate the report error with ?, and preserve the source decision’s
receipt/discard contract.

---

Nitpick comments:
In `@crates/skippy-server/src/frontend/linear_proposal.rs`:
- Around line 558-599: Extract the self-contained repair closure in
execute_local_linear_proposal_inner into a semantically named helper such as
trim_branch_suffix_or_retire, passing the runtime, session ID, and canonical
position it needs. Have the helper perform trim_session, validate the repaired
token count, retire the session on failure or mismatch, and return the repair
timing values; update the caller to apply those results while preserving
existing error messages and 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 44909aa2-acd8-4f5d-9704-a1bca8ee96e0

📥 Commits

Reviewing files that changed from the base of the PR and between cdd793b and efebe1e.

📒 Files selected for processing (12)

• crates/mesh-llm-host-runtime/src/inference/skippy/mod.rs
• crates/mesh-llm-host-runtime/src/inference/skippy/resolver/translation.rs
• crates/skippy-server/src/binary_transport/binary_messaging.rs
• crates/skippy-server/src/frontend.rs
• crates/skippy-server/src/frontend/generation/server.rs
• crates/skippy-server/src/frontend/generation/types.rs
• crates/skippy-server/src/frontend/linear_proposal.rs
• crates/skippy-server/src/frontend/local_generation.rs
• crates/skippy-server/src/frontend/native_mtp/mod.rs
• crates/skippy-server/src/frontend/tests/multimodal.rs
• crates/skippy-server/src/lib.rs
• crates/skippy-server/src/runtime_state.rs

[ndizazzo]

@i386 we have a couple runtime /telemetry fixes on this one

[michaelneale]

Reviewed this — overall low risk to main, because the feature is inert by default: all construction sites pass linear_proposal_ingress: None, nothing in-tree sets it, and the gate is computed once outside the decode loop. The "leaves existing generation unchanged" claim holds, and I confirmed no proto/, gossip, or skippy-ffi ABI surface is touched.

A few things I'd want addressed before this stops being inert:

1. The risky code is exactly the untested code. The 10 new tests all cover pure helpers via a fake ingress. execute_local_linear_proposal_inner, trim_branch_suffix_or_retire, and RuntimeState::verify_tokens — everything that mutates real KV state — have no coverage. Would like an integration test driving a real session through full-accept, first-mismatch, and callback-stop, asserting session_token_count afterward.

2. Position mismatch fails the request instead of falling back. Each guard returns OpenAiError::backend, so a mismatch (KV eviction, sliding window, concurrent trim) kills the request mid-stream. Since this path inherently races proactive eviction, falling back to serial decode seems strictly more robust than 500ing. This is the design call I'd push back on most.

3. base_position rests on an implicit prefill invariant. prompt_token_ids.len().saturating_sub(1) + decoded_tokens is only correct because the two prefill modes differ in decoded_tokens. Both cases do land right, and the session_token_count check means it fails loudly rather than corrupting KV — but it deserves a comment, since it'll break quietly if prefill changes.

4. Tokens stream before the KV repair. on_token() fires for all committed tokens, then trim_branch_suffix_or_retire() runs. If the trim fails the session is retired and an error returned after the client already received tokens. The tokens are target-authoritative so the stream isn't wrong, but the client sees a partial response plus an error.

5. The deadline is unenforceable. propose() runs synchronously on the decode thread with no preemption, so a hung source hangs the request; lateness is only detected after it returns. Honestly documented, but no config bound can fix it.

6. EmbeddedOpenAiArgs is pub with pub fields and no #[non_exhaustive]. Adding a field is a semver break for embedders. No in-tree SDK/FFI consumers today, so low impact, but this struct will keep growing — worth #[non_exhaustive] or a builder now.

7. Merge ordering. Please confirm this lands after #1078 — out of order risks wrong Metal results for the batched verification call, and per (1) no test would catch it.

Minor: linear_proposal_disposition's condition is hard to read and Stopped absorbs several distinct outcomes; accepted_proposal_tokens.min(committed_tokens.len()) can understate acceptance on early stop (telemetry-only); two raw eprintln!s where the repo has a telemetry/OutputEvent contract and stderr is TUI territory; runtime_state.rs was already past 1k lines, so per the repo rule this change arguably wanted an extraction.

Merging as-is is defensible because it's inert, but I'd treat it as unvalidated until something in-tree constructs an ingress — at which point 1–3 become blocking.

[ndizazzo]

Follow-up status after stacking this PR on #1080 (63b668177):

• caps proposal width to the active runtime n_batch - 1 window;
• falls back to serial decode when the session position drifts before verification;
• documents the prefill/base-position invariant and advisory deadline;
• keeps receipt-report failures non-fatal after tokens commit and makes proposal telemetry privacy-safe/bounded;
• keeps branch repair in the named trim_branch_suffix_or_retire path;
• marks the new public proposal contracts non-exhaustive.

Merge ordering is now explicit: #1078 is already on main, and #1081 is based on #1080. Local validation passed skippy-server's 327 tests, mesh-llm-host-runtime's 1,710 tests (8 ignored), warning-denying Clippy for both skippy-server and mesh-llm, and workspace formatting.

The feature remains inert by default. A fixture-backed real-model test covering full accept / first mismatch / callback stop is still best treated as an activation prerequisite because the repository does not carry a hermetic model fixture; the existing optional native-runtime smoke fixtures cannot make that coverage run in normal CI. Likewise, changing the already-public EmbeddedOpenAiArgs construction contract needs a dedicated builder migration rather than making current cross-crate struct literals uncompilable in this stack.