feat(persona): add role-based review personas (#51)
CI / test (pull_request) Successful in 15s
CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 17s
CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Failing after 17s
CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 16s
CI / review (/openai/v1, gpt-5-mini, gpt5-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 14s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m28s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m41s
CI / test (pull_request) Successful in 15s
CI / review (/openai/v1, gpt-4.1, gpt41, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 17s
CI / review (/anthropic/v1, claude-sonnet-4-6, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Failing after 17s
CI / review (/openai/v1, gpt-4.1-mini, gpt41-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 16s
CI / review (/openai/v1, gpt-5-mini, gpt5-mini, openai, GPT_REVIEW_TOKEN) (pull_request) Failing after 14s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m28s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m41s
Implement role-based review personas that provide specialized review focus: - Security: vulnerabilities, auth, secrets, injection attacks - Architect: design patterns, code organization, API contracts - Docs: documentation quality, API clarity, error messages Changes: - Add persona loading from JSON files and embedded built-ins - Add --persona and --persona-file CLI flags (mutually exclusive) - Add BuildPersonaSystemPrompt for persona-specific prompts - Add FormatMarkdownWithDisplay for persona display names - Update action.yml with persona and persona-file inputs - Add comprehensive tests for all new functionality - Document personas in README with examples The persona system replaces the generic 'You are an expert code reviewer' prompt with domain-specific identity, focus areas, ignore list, and severity calibration. This reduces redundancy between multiple reviewers and catches domain-specific issues that generic reviewers miss. Closes #51
This commit is contained in:
@@ -0,0 +1,157 @@
|
||||
package review
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestBuildPersonaSystemPrompt(t *testing.T) {
|
||||
p := &Persona{
|
||||
Name: "security",
|
||||
DisplayName: "Security Specialist",
|
||||
Identity: "You are a security specialist.",
|
||||
Focus: []string{"injection attacks", "auth bypass"},
|
||||
Ignore: []string{"code style", "performance"},
|
||||
Severity: Severity{
|
||||
Major: "exploitable vulnerabilities",
|
||||
Minor: "defense in depth",
|
||||
Nit: "theoretical risks",
|
||||
},
|
||||
}
|
||||
|
||||
prompt := BuildPersonaSystemPrompt(p)
|
||||
|
||||
// Check identity is included
|
||||
if !strings.Contains(prompt, "You are a security specialist.") {
|
||||
t.Error("prompt should contain identity")
|
||||
}
|
||||
|
||||
// Check focus areas
|
||||
if !strings.Contains(prompt, "Focus Areas") {
|
||||
t.Error("prompt should contain Focus Areas section")
|
||||
}
|
||||
if !strings.Contains(prompt, "injection attacks") {
|
||||
t.Error("prompt should contain focus item")
|
||||
}
|
||||
|
||||
// Check ignore section
|
||||
if !strings.Contains(prompt, "Out of Scope") {
|
||||
t.Error("prompt should contain Out of Scope section")
|
||||
}
|
||||
if !strings.Contains(prompt, "code style") {
|
||||
t.Error("prompt should contain ignore item")
|
||||
}
|
||||
|
||||
// Check severity calibration
|
||||
if !strings.Contains(prompt, "Severity Calibration") {
|
||||
t.Error("prompt should contain Severity Calibration section")
|
||||
}
|
||||
if !strings.Contains(prompt, "exploitable vulnerabilities") {
|
||||
t.Error("prompt should contain major severity definition")
|
||||
}
|
||||
|
||||
// Check JSON output format is included
|
||||
if !strings.Contains(prompt, `"verdict"`) {
|
||||
t.Error("prompt should contain JSON output format")
|
||||
}
|
||||
if !strings.Contains(prompt, "APPROVE") {
|
||||
t.Error("prompt should mention APPROVE verdict")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPersonaSystemPromptMinimal(t *testing.T) {
|
||||
// Minimal persona with only required fields
|
||||
p := &Persona{
|
||||
Name: "minimal",
|
||||
Identity: "You are a minimal reviewer.",
|
||||
}
|
||||
|
||||
prompt := BuildPersonaSystemPrompt(p)
|
||||
|
||||
// Should still work without optional fields
|
||||
if !strings.Contains(prompt, "You are a minimal reviewer.") {
|
||||
t.Error("prompt should contain identity")
|
||||
}
|
||||
|
||||
// Should not have empty sections
|
||||
if strings.Contains(prompt, "Focus Areas") && !strings.Contains(prompt, "Concentrate your review on:") {
|
||||
t.Error("should not have Focus Areas header without content")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildSystemPromptWithPersona(t *testing.T) {
|
||||
t.Run("with persona", func(t *testing.T) {
|
||||
p := &Persona{
|
||||
Name: "test",
|
||||
Identity: "Test persona identity.",
|
||||
Focus: []string{"testing"},
|
||||
}
|
||||
|
||||
prompt := BuildSystemPromptWithPersona(p, "test conventions", "test patterns")
|
||||
|
||||
if !strings.Contains(prompt, "Test persona identity.") {
|
||||
t.Error("should contain persona identity")
|
||||
}
|
||||
if !strings.Contains(prompt, "test conventions") {
|
||||
t.Error("should contain conventions")
|
||||
}
|
||||
if !strings.Contains(prompt, "test patterns") {
|
||||
t.Error("should contain patterns")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("without persona", func(t *testing.T) {
|
||||
prompt := BuildSystemPromptWithPersona(nil, "test conventions", "test patterns")
|
||||
|
||||
// Should use default system base
|
||||
if !strings.Contains(prompt, "expert code reviewer") {
|
||||
t.Error("should contain default system base when no persona")
|
||||
}
|
||||
if !strings.Contains(prompt, "test conventions") {
|
||||
t.Error("should contain conventions")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("empty conventions and patterns", func(t *testing.T) {
|
||||
p := &Persona{
|
||||
Name: "test",
|
||||
Identity: "Test identity.",
|
||||
}
|
||||
|
||||
prompt := BuildSystemPromptWithPersona(p, "", "")
|
||||
|
||||
if strings.Contains(prompt, "Language Patterns") {
|
||||
t.Error("should not contain patterns section when empty")
|
||||
}
|
||||
if strings.Contains(prompt, "Repository Conventions") {
|
||||
t.Error("should not contain conventions section when empty")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestPersonaPromptContainsOutputRules(t *testing.T) {
|
||||
p := &Persona{
|
||||
Name: "test",
|
||||
Identity: "Test.",
|
||||
}
|
||||
|
||||
prompt := BuildPersonaSystemPrompt(p)
|
||||
|
||||
// Must contain the critical output rules
|
||||
requiredStrings := []string{
|
||||
"APPROVE",
|
||||
"REQUEST_CHANGES",
|
||||
"MAJOR",
|
||||
"MINOR",
|
||||
"NIT",
|
||||
"verdict",
|
||||
"findings",
|
||||
"CI",
|
||||
}
|
||||
|
||||
for _, s := range requiredStrings {
|
||||
if !strings.Contains(prompt, s) {
|
||||
t.Errorf("prompt should contain %q", s)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user