docs(local-models) [DO NOT MERGE]: add vllm hidden-state serving notes#13
docs(local-models) [DO NOT MERGE]: add vllm hidden-state serving notes#13sabhatinas wants to merge 2 commits into
Conversation
|
00164a6 to
bab0ebe
Compare
|
@sir-merge-a-lot groom |
|
🤖 sir-merge-a-lot Rebase succeededRebased Why now: this PR needed to be rebased onto 📖 How rebases run · Force-with-lease and backup refs🤖 sir-merge-a-lot · why this happened? · pause / opt out · Updated 2026-07-09 18:41:04 UTC |
ab7ed14 to
1977ff4
Compare
WalkthroughThis PR adds a new documentation page, ChangesHidden-State Serving Docs
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 `@docs/vllm-serve-hidden-state.md`:
- Around line 81-97: The smoke test still selects the newest .safetensors file
instead of using the hidden-state path returned by vLLM. Update the Python
snippet in the hidden-state docs to read and open the actual path from
kv_transfer_params.hidden_states_path, and use that value consistently when
printing tensor metadata so the test validates the correct run.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 91b5ffaf-496b-478c-92bd-16fd32ed21cd
📒 Files selected for processing (2)
docs/vllm-serve-hidden-state.mdmkdocs.yml
| Read the path from the response rather than assuming a filename. vLLM may choose the concrete safetensors file name. | ||
|
|
||
| ```bash | ||
| uv run python - <<'PY_INNER' | ||
| from pathlib import Path | ||
| from safetensors import safe_open | ||
|
|
||
| path = Path("/tmp/vllm-hidden-states") | ||
| files = sorted(path.glob("*.safetensors"), key=lambda item: item.stat().st_mtime) | ||
| if not files: | ||
| raise SystemExit("no safetensors files written") | ||
|
|
||
| with safe_open(files[-1], framework="numpy") as handle: | ||
| for key in handle.keys(): | ||
| tensor = handle.get_tensor(key) | ||
| print(files[-1], key, tensor.shape, tensor.dtype) | ||
| PY_INNER |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the returned hidden-state path in the smoke test.
The prose says to read kv_transfer_params.hidden_states_path, but the Python snippet still opens the newest .safetensors file in the directory. Leftover files can make this validate the wrong run.
Suggested fix
-curl http://localhost:8000/v1/chat/completions \
+curl -s http://localhost:8000/v1/chat/completions \
-H "Content-Type: application/json" \
-d '{
@@
- }'
+ }' > /tmp/vllm-response.json uv run python - <<'PY_INNER'
+import json
from pathlib import Path
from safetensors import safe_open
-path = Path("/tmp/vllm-hidden-states")
-files = sorted(path.glob("*.safetensors"), key=lambda item: item.stat().st_mtime)
-if not files:
- raise SystemExit("no safetensors files written")
-
-with safe_open(files[-1], framework="numpy") as handle:
+response = json.loads(Path("/tmp/vllm-response.json").read_text())
+path = Path(response["kv_transfer_params"]["hidden_states_path"])
+with safe_open(path, framework="numpy") as handle:
for key in handle.keys():
tensor = handle.get_tensor(key)
- print(files[-1], key, tensor.shape, tensor.dtype)
+ print(path, key, tensor.shape, tensor.dtype)
PY_INNER🤖 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 `@docs/vllm-serve-hidden-state.md` around lines 81 - 97, The smoke test still
selects the newest .safetensors file instead of using the hidden-state path
returned by vLLM. Update the Python snippet in the hidden-state docs to read and
open the actual path from kv_transfer_params.hidden_states_path, and use that
value consistently when printing tensor metadata so the test validates the
correct run.
Summary
vllm servecommand.Validation
uv run --only-group docs mkdocs build --strictSummary by CodeRabbit