Llama Stack Code Review Guidelines
This skill provides specialized knowledge for reviewing code in the Llama Stack project (https://github.com/llamastack/llama-stack).
When to Use This Skill
Invoke this skill when:
- •Reviewing pull requests in the llama-stack repository
- •Analyzing code changes in llama-stack
- •Providing feedback on llama-stack contributions
- •Understanding llama-stack architecture patterns
Critical: Recordings Folders
MOST IMPORTANT: Llama Stack uses recordings/ folders containing JSON response files for CI testing.
Review Approach for Recordings
- •Ignore all content in
recordings/folders and any JSON files within them - •Do NOT review the content of recording files - they are auto-generated test fixtures
- •Instead: Verify that CI checks are passing
- •Focus on: Actual code changes, not recorded responses
Why This Matters
- •Recording files can be hundreds or thousands of lines of JSON
- •They are generated automatically and don't require human review
- •Reviewing them wastes time and provides no value
- •CI validates that recordings work correctly
Example
# DO NOT review in detail: tests/recordings/test_foo/response_1.json (500 lines of JSON) tests/recordings/test_bar/response_2.json (800 lines of JSON) # Instead, confirm: ✓ CI checks passing ✓ Test code changes are valid ✓ Actual implementation code is reviewed
Llama Stack Architecture Patterns
API Backward Compatibility
Llama Stack emphasizes API stability:
- •Verify new API changes maintain backward compatibility
- •Check that existing clients won't break
- •Flag breaking changes explicitly and ensure they're intentional
- •Validate deprecation paths are provided for removed features
Distributed System Considerations
Llama Stack operates in distributed environments:
- •Race conditions: Check for proper synchronization and locking
- •Eventual consistency: Verify code handles delayed updates correctly
- •Network failures: Ensure retries and timeout handling
- •Partial failures: Check that failures in one component don't cascade
Error Propagation
Errors must flow correctly across component boundaries:
- •Verify errors include sufficient context for debugging
- •Check that error types are appropriate for the abstraction level
- •Ensure errors aren't swallowed or logged without handling
- •Validate error messages are actionable for users
Integration Points
Llama Stack has well-defined integration patterns:
- •Verify new integrations follow established patterns
- •Check that provider interfaces are implemented completely
- •Ensure configuration handling is consistent
- •Validate that new components integrate with existing telemetry/logging
Performance for Distributed Workloads
Performance is critical at scale:
- •Confirm performance impact is acceptable for distributed deployments
- •Check for N+1 query problems in data access
- •Verify caching strategies are appropriate
- •Ensure batch operations are used where possible
Review Checklist for Llama Stack PRs
Use this checklist in addition to standard code review criteria:
- •Recordings: Confirmed recordings folders ignored, CI status checked ✓
- •Backward Compatibility: API changes are backward compatible ✓
- •Distributed Systems: Race conditions, consistency, failures handled ✓
- •Error Handling: Errors propagate correctly with context ✓
- •Integration Patterns: Follows established patterns ✓
- •Performance: Acceptable impact for distributed workloads ✓
- •Testing: Tests cover distributed scenarios, not just happy path ✓
Common Llama Stack Patterns
Provider Interface Pattern
Llama Stack uses provider interfaces for extensibility:
class MyProvider(BaseProvider):
async def initialize(self):
# Setup logic
pass
async def execute(self, request):
# Implementation
pass
Verify:
- •All required methods are implemented
- •Async patterns are used consistently
- •Error handling follows provider conventions
Configuration Pattern
Configuration is hierarchical and validated:
- •Check that new config options are documented
- •Verify defaults are sensible
- •Ensure validation happens early with clear error messages
Anti-Patterns to Flag
- •Blocking I/O: Should use async patterns
- •Hardcoded timeouts: Should be configurable
- •Missing telemetry: New operations should emit metrics/logs
- •Tight coupling: Components should use defined interfaces
- •Ignored recordings: Don't spend time reviewing recording files
Testing Expectations
Llama Stack has specific testing patterns:
- •Unit tests for individual components
- •Integration tests using the recordings pattern
- •Distributed scenario tests for multi-component features
- •Performance benchmarks for critical paths
Verify that:
- •New features include appropriate test coverage
- •Tests cover error cases, not just success paths
- •Recordings are generated for integration tests (but don't review the recordings content)
Documentation Standards
Code changes should include:
- •API documentation for public interfaces
- •Architecture decision context for significant changes
- •Migration guides for breaking changes
- •Performance characteristics for new features
Final Recommendation
After reviewing a Llama Stack PR using these guidelines:
- •Summarize findings specific to Llama Stack patterns
- •Call out any violations of distributed system best practices
- •Confirm recordings folders were handled correctly (ignored, CI checked)
- •Note any backward compatibility concerns
- •Provide overall recommendation with Llama Stack context
Remember: The most common mistake is spending time reviewing recordings folder content. Always check CI status instead.