Phase 0 completion: NT8 SDK core framework with risk management and position sizing
Some checks failed
Build and Test / build (push) Has been cancelled
Some checks failed
Build and Test / build (push) Has been cancelled
This commit is contained in:
105
CODE_REVIEW_CHECKLIST.md
Normal file
105
CODE_REVIEW_CHECKLIST.md
Normal file
@@ -0,0 +1,105 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user