[atao2004]

Closes #93

Summary

Revamps the LMCache + Valkey cookbook (previously PR #16, merged then removed in the repo's
blank-slate reorg) to the current cookbook structure and acceptance criteria.

Why a revamp, not a straight port: the original cookbook was written entirely from LMCache's
docs/source and was never executed — the author's own PR description said it "has not been
validated on hardware" because it required vLLM on a Linux machine with an NVIDIA GPU. This PR
takes a different approach: it scopes the cookbook to the parts of the LMCache↔Valkey integration
that are genuinely testable without a GPU — LMCache's real config-loading code
(lmcache.v1.config.load_engine_config_with_overrides) and real Valkey key-generation code
(lmcache.utils.CacheEngineKey) — and runs all of it against a real local Valkey container.
Running the full vLLM inference pipeline (with actual TTFT reduction) is out of scope and clearly
labeled as such throughout; readers are pointed to LMCache's own example for that.

Changes from the original PR #16

• Scope: config-loading + Valkey wire-key-format demonstration (CI-testable) instead of an
  untested vLLM/GPU walkthrough.
• Mode coverage: covers both the legacy in-process valkey:// connector (what most existing
  LMCache+Valkey deployments run today) and the newer MP-mode --l2-adapter adapter (LMCache's
  currently-documented recommended path, with an explicit note that MP-mode Valkey support is only
  a few weeks old).
• No unsourced performance claims: the original's "3-10x TTFT reduction" claim is removed; all
  remaining performance-adjacent language is qualitative, per repo policy.
• Structure: matches the current CONTRIBUTING.md template exactly (lead blockquote,
  difficulty badge, audience line, step headings, security callout, nav footer); meta.json
  dropped (confirmed removed from the current structure).

Validation

• 34 pytest tests, all passing against real Valkey containers (versions 8.1.1 and 9.1.0)
• All three sample scripts (getting_started.py, kv_cache_sharing.py,
  production_deployment.py) run end-to-end against a real Valkey container; output matches what's
  documented
• Every generated LMCache config (both the legacy YAML and the MP-mode JSON) is round-tripped
  through LMCache's own real parsers (load_engine_config_with_overrides and
  ValkeyL2AdapterConfig.from_dict) rather than just asserted to "look right"
• ruff check clean, markdownlint-cli2 clean
• Dependencies verified installable via pip (note: uv pip install fails on macOS for this exact
  LMCache version due to an upstream packaging quirk — documented in sample/README.md with a
  workaround)

Cookbook structure

cookbooks/framework-integrations/lmcache/
├── README.md
├── 01-getting-started.md
├── 02-kv-cache-sharing.md
├── 03-production-deployment.md
└── sample/
    ├── README.md
    ├── common.py
    ├── getting_started.py
    ├── kv_cache_sharing.py
    ├── production_deployment.py
    ├── test_lmcache_valkey.py
    ├── docker-compose.yml
    ├── requirements.txt
    ├── lmcache_config.yaml
    └── .gitignore

Plus a temporary .github/workflows/cookbook-python.yml (per the PR #45 shared-workflow pattern —
TODO comment included to remove this copy once PR #45 merges).

Vendor neutrality

LMCache is an independent open-source project (not affiliated with Valkey or any single cloud
vendor). Production deployment examples (03) use ElastiCache Serverless only as one illustrative
example of generic TLS/cluster-mode configuration, not as an exclusive or required path.

Version compatibility

• lmcache==0.5.2, torch==2.13.0, valkey-glide==2.5.0, pytest==8.3.5 — all pinned exact
• CI matrix runs against Valkey 8.1.1 and 9.1.0

Relationship to other PRs

Independent of other open cookbook PRs in this repo; no shared files touched besides the temporary
CI workflow (which only adds framework-integrations/lmcache to its own matrix, per the
established multi-PR-safe pattern).

Pre-Submit Checklist

• Builds and runs from a clean clone (via pip; see note above re: uv)
• Runs against current stable Valkey (tested against 8.1.1 and 9.1.0)
• No private links, internal references, or local paths
• No marketing language or uncited claims
• Vendor-neutral by default (LMCache's independent-project status implicit in it being the
  cookbook's subject; no exclusive cloud-vendor path)
• README explains the Valkey concept demonstrated, prerequisites, and how to run
• Dependencies pinned to specific versions
• CI validates the sample

[Jonathan-Improving]

Policy Compliance Review

Audited against sdd/POLICY.md. 2 variances found:

1. Markdown Page Structure (§ Markdown page structure)

• 01-getting-started.md: Navigation footer missing ← README back-link. Only has forward link to page 02.
• 03-production-deployment.md: Navigation footer missing forward link (→ README or end marker). Only has back-link to page 02.
• Policy: "Navigation footer linking to previous/next pages and README."

2. CI Workflow (§ CI workflow conventions)

• Workflow file: Named cookbook-python.yml instead of cookbook-lmcache.yml. Policy: "Each cookbook provides a temporary per-cookbook workflow (cookbook-<name>.yml)." Triggers are also broad (cookbooks/**/*.py) rather than scoped to cookbooks/framework-integrations/lmcache/sample/**.

---

All other checks pass: branch naming (cookbook-lmcache-revamp ✓), version pinning, Docker conventions, cookbook structure, vendor neutrality, writing style, no meta.json, no private links.

[jbrinkman]

Cookbook Review — LMCache

[AI Review] Reviewed against CONTRIBUTING.md standards:

• ❌ Directory structure & naming
• ✅ Issue context & requirements verification
• ✅ Upstream Valkey support verification
• ✅ Markdown structure & mandatory elements
• ✅ Acceptance criteria & vendor neutrality
• ✅ Sample code quality
• ✅ CI validation

Summary

• Total findings: 1
• Blocking: 1

Key Issues

• ❌ Missing python/ language directory — should be cookbooks/framework-integrations/python/lmcache/

Please address the blocking issue above before merging.

[jbrinkman]

Cookbook Review — LMCache

Reviewed against CONTRIBUTING.md standards and consistency with feedback on PRs #44, #46, #51, #71:

• ✅ Directory structure & naming (already has python/ level)
• ✅ Uses valkey-glide (not generic valkey client)
• ✅ Binds to 127.0.0.1 (not all interfaces)
• ✅ Pins exact dependency versions
• ✅ Real pytest tests (not go run | grep pattern)
• ❌ Vendor neutrality (ElastiCache/Serverless references throughout)
• ❌ Scope discipline (LMCache internals in a Valkey integration cookbook)

Summary

• Total findings: 8
• Blocking: 5
• Suggestions: 3

Theme

The cookbook's core (Getting Started, KV Cache Sharing mechanism, real LMCache code exercised against real Valkey) is solid. The issues are all in overreach: vendor-specific AWS content that doesn't belong, and LMCache tuning/sizing guidance that's the framework's own documentation's job, not a Valkey integration cookbook's.

[MatthiasHowellYopp]

Re-reviewed at ea23ec1 against the current cookbook guidelines. This is the cleanest of the current cookbook set — I verified the sample directly and want to flag that both outstanding CHANGES_REQUESTED reviews now appear stale against this head, since their blocking items are addressed:

Addressed (verified at ea23ec1):

• Language directory (both reviews' blocking finding): files are under cookbooks/framework-integrations/python/lmcache/ ✓
• Nav footers (Policy variance #1): 01 has ← README, 03 has README → ✓
• Vendor neutrality (jbrinkman): no ElastiCache/Serverless/serverless anywhere in docs, code, or tests; hosted services handled generically ("consult their provider's documentation") ✓
• PEP 8 naming (jbrinkman): valkey_client snake_case throughout; no valkeyClient ✓
• Serverless removal: generate_serverless_config() / --mode serverless / the serverless TLS test are gone ✓
• Deps pinned exactly (lmcache==0.5.2, torch==2.13.0, valkey-glide==2.5.0, pytest==8.3.5); 9.1.2 + 127.0.0.1; async GLIDE with close(); no KEYS/time.sleep ✓

Scope discipline — now a model example. jbrinkman's 03 concerns (capacity-planning math / performance-tuning internals) are addressed: 03 now keeps the Valkey-relevant production surface (cluster mode, TLS, credentials handling, config validation via LMCache's real parser, INFO STATS/INFO MEMORY monitoring, maxmemory + allkeys-lru eviction) and explicitly defers LMCache/model-specific KV sizing and tuning to LMCache's docs (03 lines ~122 and ~164). That's exactly the "scope each recipe to Valkey's role; point to the framework's docs for framework internals" pattern.

One genuinely open item (policy-doc, not a PR defect): Policy variance #2 flags the workflow as cookbook-python.yml rather than cookbook-lmcache.yml. That file isn't in this PR, and the CI PR intentionally moved to per-language auto-discovery workflows — so this reads as sdd/POLICY.md lagging the CI direction rather than a defect here. Worth reconciling POLICY.md with the auto-discovery decision so future cookbooks aren't flagged for it.

No blocking findings from me. Non-blocking COMMENT.

[MatthiasHowellYopp]

@Jonathan-Improving — re: the Policy Compliance Review, variance #2 (CI Workflow) looks like it's catching a rule in sdd/POLICY.md that predates a deliberate CI change, rather than a defect in this PR. Flagging so it can be corrected at the source ruleset.

What the policy expects (as quoted): "Each cookbook provides a temporary per-cookbook workflow (cookbook-<name>.yml)" with triggers scoped to that cookbook's sample/**.

What the repo actually does now: the CI work moved to per-language auto-discovery — one workflow per language (cookbook-python.yml, cookbook-go.yml, …) that discovers every cookbook sample/ directory of that language and runs them via a matrix, triggered broadly (cookbooks/**/*.py) on purpose so a newly-added cookbook is picked up with no CI edit.

Under that model, both sub-points of variance #2 invert:

• There is intentionally no cookbook-lmcache.yml — auto-discovery means individual cookbooks don't ship their own workflow (the earlier per-cookbook cookbook-go.yml / cookbook-python.yml files were removed for exactly this reason).
• The broad cookbooks/**/*.py trigger isn't over-broad by accident — it's the mechanism that makes auto-discovery work.

So no individual cookbook PR (including this one) can satisfy that clause without re-introducing the per-cookbook workflow files that were deliberately retired.

Suggested fix (at source): update the CI workflow conventions section of sdd/POLICY.md to describe per-language auto-discovery (per-language workflow name, matrix over discovered sample/ dirs, intentionally broad language-scoped trigger) instead of per-cookbook cookbook-<name>.yml files. That will stop future cookbooks from being flagged for a convention the repo has moved off of.

For the record, variance #1 (nav footers) appears already addressed at the current head (ea23ec17): 01 has the ← README back-link and 03 has the README → forward link.

[Jonathan-Improving]

QA Review — LMCache + Valkey Cookbook (ea23ec1)

Verdict: APPROVE ✅

Reviewed against CONTRIBUTING.md cookbook structure, GLIDE API correctness (via valkey-glide-review-guidance KB, based on v2.4.0), link integrity, code quality, and prior review feedback.

What was checked:

• GLIDE API usage: All imports (from glide import ...), client creation (GlideClient.create), configuration (GlideClientConfiguration, AdvancedGlideClientConfiguration, NodeAddress), and operations (set, get, exists, delete, dbsize, custom_command) verified correct against the GLIDE Python KB
• Resource management: All async clients use try/finally with await client.close() — proper cleanup ✓
• Link integrity: All 5 external URLs return HTTP 200; all internal cross-references resolve to valid files within the PR
• Cookbook structure: Fully compliant with CONTRIBUTING.md (lead blockquote, difficulty badge, audience line, step headings, security callout, nav footer, prerequisites) ✓
• Code quality: PEP 8 snake_case throughout, proper type annotations, input validation (CWE-1284 host parsing), comprehensive test coverage (34 tests per PR description)
• Vendor neutrality: No vendor-specific paths; TLS/cluster examples use localhost ✓
• Dependencies: All pinned exactly (lmcache==0.5.2, torch==2.13.0, valkey-glide==2.5.0, pytest==8.3.5) ✓
• Prior feedback: All blocking items from previous CHANGES_REQUESTED reviews are addressed (directory path, nav footers, vendor neutrality, PEP 8 naming, serverless removal)

Positives:

• Excellent scope discipline — sticks to Valkey's role and defers LMCache internals to upstream docs
• Config validation via LMCache's real parsers (not just "looks right") is a strong pattern for cookbook credibility
• Nested try/finally for multi-client scenarios (kv_cache_sharing.py) prevents resource leaks
• The parse_host / parse_startup_nodes input validation with regex demonstrates security-conscious coding for a tutorial

No blocking findings. Clean to merge.

[jbrinkman]

All comments addressed.

[cnuthalapati]

Please address the comments, need rework on the audience call-outs and restructuring to show end-to-end example

[MatthiasHowellYopp]

Thanks for the thorough review, @cnuthalapati — it reshaped the cookbook for the better. Pushed in cd9c2f9 (+ 6f53b17):

Format → Jupyter notebooks. All three chapters are now .ipynb, executed top-to-bottom in CI. The sample/ directory is gone; the notebooks plus their support files (common.py, requirements.txt, docker-compose.yml) live in one cookbook directory, following the notebook convention proposed in #94.

Genuine end-to-end on CPU. The notebooks drive LMCache's real ValkeyConnector — the same connector vLLM uses — to store a real KV-cache tensor into Valkey, confirm the hit, and load it back byte-for-byte. 02 shows two workers sharing one Valkey-backed cache. This mirrors the CPU round trip you verified.

Renames: 01-intro-to-kv-caching-with-valkey, 02-sharing-kv-caches-across-instances, 03-scaling-to-production.

Fixes: added valkey-glide-sync; corrected the valkey_adapter.py reference; switched the 03 production config off cachegen (GPU-only encoder) to naive.

Full vLLM inference path: documented as an optional Linux/GPU section in 03 rather than the runnable default — I tried the opt-125m CPU path first and it won't run in CI or on a typical laptop (no PyPI CPU wheel; prebuilt CPU image is AVX512-only). Details in the per-thread replies. Happy to promote it to a fully-executed example if we add an AVX512 Linux runner to CI.

Replied to and resolved each thread. Note the notebook format depends on #94.

[cnuthalapati]

A few comments on reader experience rather than correctness. The technical content here is solid, but a newcomer has to work fairly hard to get oriented. The OpenAI Agents API cookbook is a good bar for the readability I think we should aim at. I am not attached to any particular file layout, only to the reading experience.

[cnuthalapati]

Approving. The restructure landed well and the reading experience is much stronger, with the README now carrying the hook, the numbered setup and the clone step, and the notebook dropping to just the walkthrough. Thanks for taking the earlier feedback.

Three small things below. The KEYS * helper is the only one I'd ask you to action before merge; the other two are non-blocking.

Superseded. The audience call-outs and the end-to-end restructuring raised here have been addressed, and I have approved the current revision.

[rlunar]

I like this new cleaner version; but I'd strongly suggest to standardize on using uv for package management and have a pyproject.toml it help us lock both the Python version as well as the packages versions.

Another suggestion is to have proper naming conventions; cookbook is a collection of N recipes, for example this book PostgreSQL 16 Administration Cookbook: Solve real-world Database Administration challenges with 180+ practical recipes and best practices

[MatthiasHowellYopp]

@rlunar thanks, both are good suggestions.

On uv: agreed, and done. The cookbook now manages its Python dependencies with a pyproject.toml and a committed uv.lock, and requires-python is pinned so the interpreter gets reproduced along with the packages (3a9ea0b on this PR). Setup is uv sync / uv run, and uv is listed in the prerequisites with a pointer to install it. I carried the change through the other two PRs so it lands as a convention rather than a one-off: CONTRIBUTING now requires uv for Python cookbooks (#94, abb5d90), and CI builds and tests Python cookbooks with uv (#43, 02198ab).

Would you mind taking another look at the uv change when you get a chance?

Still thinking about the cookbook/recipe thing.

[MatthiasHowellYopp]

@rlunar I've addressed the Naming conventions in the top level read me - let me know if that works.

[alexey-temnikov]

Approved via gh cli.