Reviewing Llama Stack Code

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:

1. Recordings: Confirmed recordings folders ignored, CI status checked ✓
2. Backward Compatibility: API changes are backward compatible ✓
3. Distributed Systems: Race conditions, consistency, failures handled ✓
4. Error Handling: Errors propagate correctly with context ✓
5. Integration Patterns: Follows established patterns ✓
6. Performance: Acceptable impact for distributed workloads ✓
7. 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:

1. Summarize findings specific to Llama Stack patterns
2. Call out any violations of distributed system best practices
3. Confirm recordings folders were handled correctly (ignored, CI checked)
4. Note any backward compatibility concerns
5. Provide overall recommendation with Llama Stack context

Remember: The most common mistake is spending time reviewing recordings folder content. Always check CI status instead.