105 lines
3.3 KiB
Markdown
105 lines
3.3 KiB
Markdown
# NT8 SDK - Code Review Checklist
|
|
|
|
## Pre-Commit Checklist for AI Agents
|
|
|
|
### ✅ Compatibility Check
|
|
- [ ] All projects target `net48`
|
|
- [ ] Language version set to `5.0` in all projects
|
|
- [ ] No modern C# features used (records, nullable refs, string interpolation)
|
|
- [ ] No .NET Core packages added
|
|
- [ ] `.\verify-build.bat` passes with zero errors
|
|
|
|
### ✅ Architecture Compliance
|
|
- [ ] All trading logic goes through IRiskManager
|
|
- [ ] Strategies are thin plugins (signal generation only)
|
|
- [ ] No direct market access from strategies
|
|
- [ ] Proper error handling and logging
|
|
|
|
### ✅ Code Quality
|
|
- [ ] All classes have proper constructors (no auto-properties with initializers)
|
|
- [ ] Dictionary initialization uses `.Add()` method
|
|
- [ ] String formatting uses `String.Format()` not interpolation
|
|
- [ ] All interfaces properly implemented
|
|
- [ ] Unit tests included for new functionality
|
|
|
|
### ✅ Testing Requirements
|
|
- [ ] MSTest framework used (not xUnit)
|
|
- [ ] Test coverage >80% for core components
|
|
- [ ] All risk scenarios tested
|
|
- [ ] Integration tests work with mock data
|
|
|
|
## Common Rejection Reasons
|
|
|
|
### ❌ Automatic Rejection
|
|
1. **Uses C# 6+ features** (records, nullable refs, etc.)
|
|
2. **Targets wrong framework** (.NET Core instead of Framework 4.8)
|
|
3. **Adds incompatible packages** (Microsoft.Extensions.*)
|
|
4. **Bypasses risk management** (direct order submission)
|
|
5. **Build fails** (compilation errors, warnings)
|
|
|
|
### ⚠️ Review Required
|
|
1. **Complex inheritance hierarchies**
|
|
2. **Performance-critical code** (may need optimization later)
|
|
3. **External dependencies** (evaluate NT8 compatibility)
|
|
4. **Configuration changes** (verify impact on existing functionality)
|
|
|
|
## Review Process
|
|
|
|
### For Human Reviewers
|
|
1. **Run verification**: `.\verify-build.bat`
|
|
2. **Check guidelines**: Verify compliance with `AI_DEVELOPMENT_GUIDELINES.md`
|
|
3. **Test coverage**: Ensure new features have adequate tests
|
|
4. **Architecture review**: Confirm risk-first design maintained
|
|
|
|
### For AI Agents
|
|
1. **Self-check**: Use this checklist before submitting
|
|
2. **Build verification**: Always run build verification
|
|
3. **Pattern matching**: Follow existing code patterns in the repo
|
|
4. **Documentation**: Update relevant docs if needed
|
|
|
|
## Phase-Specific Guidelines
|
|
|
|
### Current Phase (Phase 1)
|
|
- Focus on NT8 integration only
|
|
- Implement basic order management
|
|
- Enhance risk controls (Tier 2)
|
|
- No UI work or advanced features
|
|
|
|
### Future Phases
|
|
- Will be specified in separate guidelines
|
|
- Do not implement features from future phases
|
|
- Stay within current phase scope
|
|
|
|
## Examples of Good vs Bad Code
|
|
|
|
### ✅ Good (C# 5.0 Compatible)
|
|
```csharp
|
|
public class RiskDecision
|
|
{
|
|
public bool Allow { get; set; }
|
|
public string RejectReason { get; set; }
|
|
|
|
public RiskDecision(bool allow, string rejectReason)
|
|
{
|
|
Allow = allow;
|
|
RejectReason = rejectReason;
|
|
}
|
|
}
|
|
|
|
var metrics = new Dictionary<string, object>();
|
|
metrics.Add("risk", 100.0);
|
|
metrics.Add("reason", "Daily limit");
|
|
```
|
|
|
|
### ❌ Bad (Modern C# - Will Not Compile)
|
|
```csharp
|
|
public record RiskDecision(bool Allow, string? RejectReason);
|
|
|
|
var metrics = new Dictionary<string, object>
|
|
{
|
|
["risk"] = 100.0,
|
|
["reason"] = "Daily limit"
|
|
};
|
|
```
|
|
|
|
This checklist should be used by AI agents before every commit and by human reviewers for all pull requests. |