Skip to content

Report Skippy artifact cold-start costs - #815

Merged
i386 merged 2 commits into
mainfrom
codex/skippy-artifact-cold-start-diagnostics
Jun 13, 2026
Merged

Report Skippy artifact cold-start costs#815
i386 merged 2 commits into
mainfrom
codex/skippy-artifact-cold-start-diagnostics

Conversation

@i386

@i386 i386 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add cold-start artifact cost diagnostics to Skippy topology planning.

This PR is only about explaining the artifact cost of an accepted package-aware plan. It does not change stage ordering, stage roles, runtime loading, or the binary protocol.

Before

Package-aware planning could use cache and missing-artifact signals to affect placement, but the accepted plan did not summarize what startup would cost.

That left operators with a plan but not an explanation:

  • how much of the selected topology is already cached
  • how many bytes are missing
  • how many missing bytes can come from peers
  • how many bytes must fall back to remote download

How It Works

artifact_diagnostics runs after stages are selected.

It aggregates selected stages:

  • cached_slice_bytes
  • missing_artifact_bytes
  • transfer-supported missing bytes
  • remote-download fallback bytes

It emits a single info diagnostic when artifact signals are present:

[info] artifact cold-start plan: cached=64 bytes, missing=48 bytes, peer-transfer-eligible=32 bytes, remote-download-fallback=16 bytes

Plain weighted plans without artifact signals stay quiet, so non-package topology planning does not get noisy diagnostics.

Why This Is Good

This makes package-aware planning explainable.

The planner can already choose a node because it has cached layers or because missing artifacts are cheap to transfer. This PR lets the accepted topology say that out loud. That matters for Skippy cold starts because slow startup is often artifact movement, not model execution.

It also prepares the next step toward metadata-first package planning: resolve package inventory, plan from cache/transfer signals, then materialize only the accepted stage artifacts.

Validation

  • cargo fmt --all -- --check
  • cargo test -p skippy-topology --lib
  • cargo clippy -p skippy-topology --all-targets -- -D warnings

Summary by CodeRabbit

  • New Features

    • Enhanced diagnostic reporting for artifact planning: adds detailed breakdowns of total cached bytes, total missing bytes, how much is eligible for peer transfer versus remote-download fallback, and an overall diagnostic severity/code reflecting artifact transfer impact.
  • Tests

    • Added unit tests that validate artifact diagnostic messages and transfer-support detection across placement scenarios, and ensure no diagnostics are emitted when artifact signals are absent.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b9738a10-614b-47d4-998c-e4d553e5dd17

📥 Commits

Reviewing files that changed from the base of the PR and between cef154b and 7b6cfb3.

📒 Files selected for processing (2)
  • crates/skippy-topology/src/artifact_diagnostics.rs
  • crates/skippy-topology/src/tests.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/skippy-topology/src/artifact_diagnostics.rs
  • crates/skippy-topology/src/tests.rs

📝 Walkthrough

Walkthrough

This PR adds artifact cold-start diagnostics to the skippy-topology planning pipeline. A new artifact_diagnostics module computes cached and missing artifact byte totals across stages, determines peer-transfer eligibility via placement signals, and appends diagnostic entries with byte breakdowns and appropriate reason codes. The diagnostics pipeline is updated to invoke this new computation and pass placement signals through.

Changes

Artifact Cold-Start Diagnostics

Layer / File(s) Summary
Core artifact diagnostics implementation
crates/skippy-topology/src/artifact_diagnostics.rs
append_artifact_diagnostics computes total cached and missing artifact bytes, exits early when both are zero, splits missing bytes into peer-transfer-eligible and remote-download-fallback amounts based on placement signals, and appends an Info-severity diagnostic with ArtifactTransferPenalty or CacheLocalityPreferred reason code. Helper missing_bytes_by_transfer_support filters stages by node and transfer support matching, then sums missing bytes.
Diagnostics pipeline integration
crates/skippy-topology/src/lib.rs
Declares the new artifact_diagnostics module, updates diagnostics_for to accept and use placement_signals to call append_artifact_diagnostics before returning diagnostics, and updates the plan_ranges_with_signals call site to pass placement_signals into the diagnostics pipeline.
Test coverage
crates/skippy-topology/src/tests.rs
package_aware_plan_reports_cold_start_artifact_totals constructs placement signals with mixed cached/missing artifacts and transfer support, validates that the resulting plan emits an ArtifactTransferPenalty diagnostic containing expected byte totals. weighted_plan_without_artifact_signals_stays_diagnostic_quiet ensures planning without artifact signals produces an empty diagnostics list.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Report Skippy artifact cold-start costs' accurately and concisely summarizes the main objective of the PR, which is to add diagnostics reporting artifact cold-start costs in Skippy topology planning.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 codex/skippy-artifact-cold-start-diagnostics

Comment @coderabbitai help to get the list of available commands and usage tips.

@i386 i386 changed the title [codex] Report Skippy artifact cold-start costs Report Skippy artifact cold-start costs Jun 10, 2026
@i386
i386 marked this pull request as ready for review June 10, 2026 10:43
@i386

i386 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

@michaelneale could you review this one when you get a chance?

Comment thread crates/skippy-topology/src/artifact_diagnostics.rs
@i386
i386 merged commit 012b569 into main Jun 13, 2026
25 checks passed
@i386
i386 deleted the codex/skippy-artifact-cold-start-diagnostics branch June 13, 2026 02:47
michaelneale added a commit that referenced this pull request Jun 13, 2026
* origin/main:
  Add transport-aware Skippy stage ordering (#814)
  Share Skippy stage wire byte accounting (#818)
  Report Skippy artifact cold-start costs (#815)
  fix: debug output capturing for TUI / panics (#827)
  fix(hero): visual corrections for iPhone SE size devices (#838)
  Add Skippy stage role metadata (#816)
  Add Skippy request cache epoch telemetry (#817)
  Consolidate agent skills and fix stale docs (Windows deploy, repo map, design docs) (#836)
  feature(version): normalize version markers for different build types (#831)
  fix(website): fix visual regressions (#835)
  fix(gh): change micn to michaelneale in auto_assign.yml
  Revert "fix(gh): replace micn with IvGolovach in auto_assign.yml (not a collaborator)"
  fix(gh): replace micn with IvGolovach in auto_assign.yml (not a collaborator)
michaelneale added a commit that referenced this pull request Jun 14, 2026
* origin/main: (29 commits)
  MoA: don't let small-model consensus pre-empt a still-running large model (#837)
  fix(console): render thinking traces as markdown
  Add bounded direct path repair (#846)
  Fix skippy smoke PR gate (#850)
  Stabilize skippy smoke chain startup (#849)
  fix(ci): switch back to auto-assign workflow
  fix(website): polish longform visual explainer (#843)
  fix: gemma thinking
  Carry GLM llama MTP patches (#840)
  Refresh llama.cpp canary patch queue (#839)
  Add transport-aware Skippy stage ordering (#814)
  Share Skippy stage wire byte accounting (#818)
  Report Skippy artifact cold-start costs (#815)
  fix: debug output capturing for TUI / panics (#827)
  fix(hero): visual corrections for iPhone SE size devices (#838)
  Add Skippy stage role metadata (#816)
  Add Skippy request cache epoch telemetry (#817)
  Consolidate agent skills and fix stale docs (Windows deploy, repo map, design docs) (#836)
  feature(version): normalize version markers for different build types (#831)
  fix(website): fix visual regressions (#835)
  ...

# Conflicts:
#	AGENTS.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants