diff --git a/internal/cli/command/pull_requests/summarize.go b/internal/cli/command/pull_requests/summarize.go index 6a887a6..9ade36f 100644 --- a/internal/cli/command/pull_requests/summarize.go +++ b/internal/cli/command/pull_requests/summarize.go @@ -41,7 +41,7 @@ func (c *SummarizeCommand) CreateCommand(t *i18n.Translations, _ *cfg.Config) *c } prNumber := command.Int("pr-number") - summary, err := prService.SummarizePR(ctx, int(prNumber)) + summary, err := prService.SummarizePR(ctx, prNumber) if err != nil { return fmt.Errorf(t.GetMessage("error.pr_summary_error", 0, nil)+": %w", err) } diff --git a/internal/domain/models/pr.go b/internal/domain/models/pr.go index 8262fa9..2dc3f23 100644 --- a/internal/domain/models/pr.go +++ b/internal/domain/models/pr.go @@ -3,10 +3,13 @@ package models type ( // PRData contiene la información extraída de una Pull Request. PRData struct { - ID int - Creator string - Commits []Commit - Diff string + ID int + Creator string + Commits []Commit + Diff string + BranchName string + RelatedIssues []Issue + PRDescription string } // Commit representa un commit incluido en el PR. diff --git a/internal/domain/ports/vcs_client.go b/internal/domain/ports/vcs_client.go index 3f35dd4..f793a3f 100644 --- a/internal/domain/ports/vcs_client.go +++ b/internal/domain/ports/vcs_client.go @@ -36,4 +36,6 @@ type VCSClient interface { GetIssue(ctx context.Context, issueNumber int) (*models.Issue, error) // GetFileAtTag obtiene el contenido de un archivo en un tag específico GetFileAtTag(ctx context.Context, tag, filepath string) (string, error) + // GetPRIssues obtiene issues relacionadas con un PR basándose en branch name, commits y descripción + GetPRIssues(ctx context.Context, branchName string, commits []string, prDescription string) ([]models.Issue, error) } diff --git a/internal/i18n/locales/active.en.toml b/internal/i18n/locales/active.en.toml index 031155a..db3220b 100644 --- a/internal/i18n/locales/active.en.toml +++ b/internal/i18n/locales/active.en.toml @@ -485,3 +485,16 @@ branch_not_detected = "Could not detect branch name" get_repo_url = "Error getting repository URL: {{.Error}}" get_commits = "Error getting commits: {{.Error}}" extract_repo_info = "Could not extract owner and repo from URL: {{.Url}}" + +# PR Service - Issue linking +[pr_detected_issues] +other = "🔍 Issues detected in PR #{{.Number}}: {{.Issues}}" + +[pr_issues_will_close_on_merge] +other = "✅ {{.Count}} issue(s) will close automatically when PR is merged" + +[pr_breaking_changes_detected] +other = "⚠️ Breaking changes detected in {{.Count}} commits" + +[pr_test_plan_generated] +other = "📋 Test plan generated automatically" diff --git a/internal/i18n/locales/active.es.toml b/internal/i18n/locales/active.es.toml index e40ab22..251461e 100644 --- a/internal/i18n/locales/active.es.toml +++ b/internal/i18n/locales/active.es.toml @@ -492,3 +492,16 @@ branch_not_detected = "No se pudo detectar el nombre de la branch" get_repo_url = "Error al obtener la URL del repositorio: {{.Error}}" get_commits = "Error al obtener commits: {{.Error}}" extract_repo_info = "No se pudo extraer el propietario y el repositorio de la URL: {{.Url}}" + +# PR Service - Issue linking +[pr_detected_issues] +other = "🔍 Issues detectadas en PR #{{.Number}}: {{.Issues}}" + +[pr_issues_will_close_on_merge] +other = "✅ {{.Count}} issue(s) se cerrarán automáticamente al mergear el PR" + +[pr_breaking_changes_detected] +other = "⚠️ Breaking changes detectados en {{.Count}} commits" + +[pr_test_plan_generated] +other = "📋 Test plan generado automáticamente" \ No newline at end of file diff --git a/internal/infrastructure/ai/prompts.go b/internal/infrastructure/ai/prompts.go index 11ae3fc..88fe1bb 100644 --- a/internal/infrastructure/ai/prompts.go +++ b/internal/infrastructure/ai/prompts.go @@ -1,5 +1,12 @@ package ai +import ( + "fmt" + "strings" + + "github.com/Tomas-vilte/MateCommit/internal/domain/models" +) + // Issue reference instructions const ( issueReferenceInstructionsES = `Si hay un issue asociado (#%d), DEBES incluir la referencia en el título del commit: @@ -503,3 +510,93 @@ func GetIssueReferenceInstructions(lang string) string { return issueReferenceInstructionsEN } } + +const ( + prIssueContextInstructionsES = ` + **IMPORTANTE - Contexto de Issues/Tickets:** + Este PR está relacionado con los siguientes issues: + %s + + **INSTRUCCIONES OBLIGATORIAS:** + 1. DEBES incluir AL INICIO del resumen (primeras líneas) las referencias de cierre: + - Si resuelve bugs: "Fixes #N" + - Si implementa features: "Closes #N" + - Si solo relaciona: "Relates to #N" + - Formato: "Closes #39, Fixes #41" (separados por comas) + + 2. En la sección de cambios clave, menciona explícitamente cómo cada cambio aborda el issue + + 3. Usa el formato correcto para que GitHub auto-enlace los issues en la sección "Development" + + **Ejemplo de formato correcto:** + Closes #39 + + - **Primer cambio clave:** + - Propósito: Resolver el problema reportado en #39... + - Impacto técnico: ... + ` + + prIssueContextInstructionsEN = ` + **IMPORTANT - Issue/Ticket Context:** + This PR is related to the following issues: + %s + + **MANDATORY INSTRUCTIONS:** + 1. You MUST include at the BEGINNING of the summary (first lines) the closing references: + - If fixing bugs: "Fixes #N" + - If implementing features: "Closes #N" + - If just relating: "Relates to #N" + - Format: "Closes #39, Fixes #41" (comma separated) + + 2. In the key changes section, explicitly mention how each change addresses the issue + + 3. Use the correct format so GitHub auto-links the issues in the "Development" section + + **Example of correct format:** + Closes #39 + + - **First key change:** + - Purpose: Resolve the problem reported in #39... + - Technical impact: ... + ` +) + +// GetPRIssueContextInstructions devuelve las instrucciones de contexto de issues para PRs +func GetPRIssueContextInstructions(locale string) string { + if locale == "es" { + return prIssueContextInstructionsES + } + return prIssueContextInstructionsEN +} + +// FormatIssuesForPrompt formatea la lista de issues para incluir en el prompt +func FormatIssuesForPrompt(issues []models.Issue, locale string) string { + if len(issues) == 0 { + return "" + } + + var result strings.Builder + for _, issue := range issues { + if locale == "es" { + result.WriteString(fmt.Sprintf("- Issue #%d: %s\n", issue.Number, issue.Title)) + if issue.Description != "" { + desc := issue.Description + if len(desc) > 200 { + desc = desc[:200] + "..." + } + result.WriteString(fmt.Sprintf(" Descripción: %s\n", desc)) + } + } else { + result.WriteString(fmt.Sprintf("- Issue #%d: %s\n", issue.Number, issue.Title)) + if issue.Description != "" { + desc := issue.Description + if len(desc) > 200 { + desc = desc[:200] + "..." + } + result.WriteString(fmt.Sprintf(" Description: %s\n", desc)) + } + } + } + + return result.String() +} diff --git a/internal/infrastructure/dependency/gomod_analyzer_test.go b/internal/infrastructure/dependency/gomod_analyzer_test.go index 88202bc..63e4bd2 100644 --- a/internal/infrastructure/dependency/gomod_analyzer_test.go +++ b/internal/infrastructure/dependency/gomod_analyzer_test.go @@ -94,6 +94,11 @@ func (m *MockVCSClient) GetIssue(ctx context.Context, issueNumber int) (*models. return args.Get(0).(*models.Issue), args.Error(1) } +func (m *MockVCSClient) GetPRIssues(ctx context.Context, branchName string, commits []string, prDescription string) ([]models.Issue, error) { + args := m.Called(ctx, branchName, commits, prDescription) + return args.Get(0).([]models.Issue), args.Error(1) +} + func TestGoModAnalyzer_Name(t *testing.T) { analyzer := NewGoModAnalyzer() assert.Equal(t, "go.mod", analyzer.Name()) diff --git a/internal/infrastructure/factory/pr_service_factory.go b/internal/infrastructure/factory/pr_service_factory.go index 72edec5..718307b 100644 --- a/internal/infrastructure/factory/pr_service_factory.go +++ b/internal/infrastructure/factory/pr_service_factory.go @@ -59,5 +59,5 @@ func (f *prServiceFactory) CreatePRService(ctx context.Context) (ports.PRService return nil, fmt.Errorf("proveedor de VCS no compatible: %s", provider) } - return services.NewPRService(vcsClient, f.aiService, f.trans), nil + return services.NewPRService(vcsClient, f.aiService, f.trans, f.config), nil } diff --git a/internal/infrastructure/vcs/github/github_service.go b/internal/infrastructure/vcs/github/github_service.go index c23d72c..a582fff 100644 --- a/internal/infrastructure/vcs/github/github_service.go +++ b/internal/infrastructure/vcs/github/github_service.go @@ -4,12 +4,15 @@ import ( "context" "fmt" "net/http" + "regexp" "sort" + "strconv" "strings" "github.com/Tomas-vilte/MateCommit/internal/domain/models" "github.com/Tomas-vilte/MateCommit/internal/domain/ports" "github.com/Tomas-vilte/MateCommit/internal/i18n" + "github.com/Tomas-vilte/MateCommit/internal/infrastructure/httpclient" "github.com/google/go-github/v80/github" "golang.org/x/oauth2" ) @@ -52,6 +55,8 @@ type GitHubClient struct { owner string repo string trans *i18n.Translations + token string + httpClient httpclient.HTTPClient } var allowedLabels = map[string]struct { @@ -82,6 +87,8 @@ func NewGitHubClient(owner, repo, token string, trans *i18n.Translations) *GitHu owner: owner, repo: repo, trans: trans, + token: token, + httpClient: httpClient, } } @@ -102,6 +109,8 @@ func NewGitHubClientWithServices( owner: owner, repo: repo, trans: trans, + token: "", + httpClient: &http.Client{}, } } @@ -176,10 +185,12 @@ func (ghc *GitHubClient) GetPR(ctx context.Context, prNumber int) (models.PRData } return models.PRData{ - ID: prNumber, - Creator: pr.GetUser().GetLogin(), - Commits: prCommits, - Diff: diff, + ID: prNumber, + Creator: pr.GetUser().GetLogin(), + Commits: prCommits, + Diff: diff, + BranchName: pr.GetHead().GetRef(), + PRDescription: pr.GetBody(), }, nil } @@ -527,6 +538,77 @@ func (ghc *GitHubClient) GetFileAtTag(ctx context.Context, tag, filepath string) return content, nil } +func (ghc *GitHubClient) GetPRIssues(ctx context.Context, branchName string, commits []string, prDescription string) ([]models.Issue, error) { + issueNumbers := make(map[int]bool) + + branchPatterns := []string{ + `#(\d+)`, + `issue[/-](\d+)`, + `^(\d+)-`, + `/(\d+)-`, + `-(\d+)-`, + } + + for _, pattern := range branchPatterns { + re := regexp.MustCompile(pattern) + if matches := re.FindStringSubmatch(branchName); len(matches) > 1 { + if num, err := strconv.Atoi(matches[1]); err == nil { + issueNumbers[num] = true + } + } + } + + if prDescription != "" { + descPatterns := []string{ + `(?i)(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)`, + `#(\d+)`, + } + + for _, pattern := range descPatterns { + re := regexp.MustCompile(pattern) + matches := re.FindAllStringSubmatch(prDescription, -1) + for _, match := range matches { + if len(match) > 1 { + if num, err := strconv.Atoi(match[1]); err == nil { + issueNumbers[num] = true + } + } + } + } + } + + commitPatterns := []string{ + `(?i)(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)`, + `\(#(\d+)\)`, + `#(\d+)`, + } + + for _, commit := range commits { + for _, pattern := range commitPatterns { + re := regexp.MustCompile(pattern) + matches := re.FindAllStringSubmatch(commit, -1) + for _, match := range matches { + if len(match) > 1 { + if num, err := strconv.Atoi(match[1]); err == nil { + issueNumbers[num] = true + } + } + } + } + } + + var issues []models.Issue + for issueNum := range issueNumbers { + issue, err := ghc.GetIssue(ctx, issueNum) + if err != nil { + continue + } + issues = append(issues, *issue) + } + + return issues, nil +} + func (ghc *GitHubClient) labelExists(existingLabels []string, target string) bool { for _, l := range existingLabels { if strings.EqualFold(l, target) { diff --git a/internal/infrastructure/vcs/github/github_service_test.go b/internal/infrastructure/vcs/github/github_service_test.go index 9a7df0f..9079e85 100644 --- a/internal/infrastructure/vcs/github/github_service_test.go +++ b/internal/infrastructure/vcs/github/github_service_test.go @@ -1149,3 +1149,122 @@ func TestGitHubClient_GetDiffFromCommits(t *testing.T) { assert.Error(t, err) }) } + +func TestGitHubClient_GetPRIssues(t *testing.T) { + t.Run("should extract issues from branch name, commits and description", func(t *testing.T) { + mockPR := &MockPRService{} + mockIssues := &MockIssuesService{} + mockRelease := &MockReleaseService{} + client := newTestClient(mockPR, mockIssues, mockRelease) + + branchName := "feature/123-new-feature" + prDescription := "Closes #456" + commits := []string{"Fix bug #789", "Update readme"} + + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 123). + Return(&github.Issue{ + Number: github.Ptr(123), + Title: github.Ptr("Issue 123"), + State: github.Ptr("open"), + User: &github.User{Login: github.Ptr("user1")}, + HTMLURL: github.Ptr("http://github.com/owner/repo/issues/123"), + }, &github.Response{}, nil) + + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 456). + Return(&github.Issue{ + Number: github.Ptr(456), + Title: github.Ptr("Issue 456"), + State: github.Ptr("closed"), + User: &github.User{Login: github.Ptr("user2")}, + HTMLURL: github.Ptr("http://github.com/owner/repo/issues/456"), + }, &github.Response{}, nil) + + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 789). + Return(&github.Issue{ + Number: github.Ptr(789), + Title: github.Ptr("Issue 789"), + State: github.Ptr("open"), + User: &github.User{Login: github.Ptr("user3")}, + HTMLURL: github.Ptr("http://github.com/owner/repo/issues/789"), + }, &github.Response{}, nil) + + issues, err := client.GetPRIssues(context.Background(), branchName, commits, prDescription) + + assert.NoError(t, err) + assert.Len(t, issues, 3) + + issueMap := make(map[int]models.Issue) + for _, issue := range issues { + issueMap[issue.Number] = issue + } + + assert.Contains(t, issueMap, 123) + assert.Contains(t, issueMap, 456) + assert.Contains(t, issueMap, 789) + + mockIssues.AssertExpectations(t) + }) + + t.Run("should deduplicate issues", func(t *testing.T) { + mockPR := &MockPRService{} + mockIssues := &MockIssuesService{} + mockRelease := &MockReleaseService{} + client := newTestClient(mockPR, mockIssues, mockRelease) + + branchName := "feature/123-fix" + prDescription := "Fixes #123" + commits := []string{"Fix #123"} + + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 123). + Return(&github.Issue{ + Number: github.Ptr(123), + Title: github.Ptr("Issue 123"), + }, &github.Response{}, nil).Once() + + issues, err := client.GetPRIssues(context.Background(), branchName, commits, prDescription) + + assert.NoError(t, err) + assert.Len(t, issues, 1) + assert.Equal(t, 123, issues[0].Number) + mockIssues.AssertExpectations(t) + }) + + t.Run("should ignore issues that fail to be retrieved", func(t *testing.T) { + mockPR := &MockPRService{} + mockIssues := &MockIssuesService{} + mockRelease := &MockReleaseService{} + client := newTestClient(mockPR, mockIssues, mockRelease) + + branchName := "" + prDescription := "Closes #999" + commits := []string{} + + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 999). + Return((*github.Issue)(nil), &github.Response{}, assert.AnError) + + issues, err := client.GetPRIssues(context.Background(), branchName, commits, prDescription) + + assert.NoError(t, err) + assert.Empty(t, issues) + mockIssues.AssertExpectations(t) + }) + + t.Run("should support various patterns", func(t *testing.T) { + mockPR := &MockPRService{} + mockIssues := &MockIssuesService{} + mockRelease := &MockReleaseService{} + client := newTestClient(mockPR, mockIssues, mockRelease) + + branchName := "issue/111" + commits := []string{"(#222)"} + prDescription := "resolve #333" + + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 111).Return(&github.Issue{Number: github.Ptr(111)}, &github.Response{}, nil) + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 222).Return(&github.Issue{Number: github.Ptr(222)}, &github.Response{}, nil) + mockIssues.On("Get", mock.Anything, "test-owner", "test-repo", 333).Return(&github.Issue{Number: github.Ptr(333)}, &github.Response{}, nil) + + issues, err := client.GetPRIssues(context.Background(), branchName, commits, prDescription) + assert.NoError(t, err) + assert.Len(t, issues, 3) + }) +} diff --git a/internal/services/mocks.go b/internal/services/mocks.go index 6bb7d54..7380e36 100644 --- a/internal/services/mocks.go +++ b/internal/services/mocks.go @@ -185,6 +185,11 @@ func (m *MockVCSClient) GetFileAtTag(ctx context.Context, tag, filepath string) return args.String(0), args.Error(1) } +func (m *MockVCSClient) GetPRIssues(ctx context.Context, branchName string, commits []string, prDescription string) ([]models.Issue, error) { + args := m.Called(ctx, branchName, commits, prDescription) + return args.Get(0).([]models.Issue), args.Error(1) +} + func (m *MockPRSummarizer) GeneratePRSummary(ctx context.Context, prompt string) (models.PRSummary, error) { args := m.Called(ctx, prompt) return args.Get(0).(models.PRSummary), args.Error(1) diff --git a/internal/services/pull_request_service.go b/internal/services/pull_request_service.go index 512940f..f147f80 100644 --- a/internal/services/pull_request_service.go +++ b/internal/services/pull_request_service.go @@ -3,10 +3,13 @@ package services import ( "context" "fmt" + "strings" + "github.com/Tomas-vilte/MateCommit/internal/config" "github.com/Tomas-vilte/MateCommit/internal/domain/models" "github.com/Tomas-vilte/MateCommit/internal/domain/ports" "github.com/Tomas-vilte/MateCommit/internal/i18n" + "github.com/Tomas-vilte/MateCommit/internal/infrastructure/ai" ) var _ ports.PRService = (*PRService)(nil) @@ -15,13 +18,16 @@ type PRService struct { vcsClient ports.VCSClient aiService ports.PRSummarizer trans *i18n.Translations + config *config.Config } -func NewPRService(vcsClient ports.VCSClient, aiService ports.PRSummarizer, trans *i18n.Translations) *PRService { +func NewPRService(vcsClient ports.VCSClient, aiService ports.PRSummarizer, trans *i18n.Translations, + cfg *config.Config) *PRService { return &PRService{ vcsClient: vcsClient, aiService: aiService, trans: trans, + config: cfg, } } @@ -30,6 +36,7 @@ func (s *PRService) SummarizePR(ctx context.Context, prNumber int) (models.PRSum msg := s.trans.GetMessage("ai_missing_for_pr", 0, nil) return models.PRSummary{}, fmt.Errorf("%s", msg) } + prData, err := s.vcsClient.GetPR(ctx, prNumber) if err != nil { msg := s.trans.GetMessage("pr_service.error_get_pr", 0, map[string]interface{}{ @@ -38,6 +45,25 @@ func (s *PRService) SummarizePR(ctx context.Context, prNumber int) (models.PRSum return models.PRSummary{}, fmt.Errorf("%s", msg) } + var commitMessages []string + for _, commit := range prData.Commits { + commitMessages = append(commitMessages, commit.Message) + } + + issues, err := s.vcsClient.GetPRIssues(ctx, prData.BranchName, commitMessages, prData.PRDescription) + if err == nil && len(issues) > 0 { + prData.RelatedIssues = issues + + issueNums := make([]string, len(issues)) + for i, issue := range issues { + issueNums[i] = fmt.Sprintf("#%d", issue.Number) + } + fmt.Printf("%s\n", s.trans.GetMessage("pr_detected_issues", 0, map[string]interface{}{ + "Number": prNumber, + "Issues": strings.Join(issueNums, ", "), + })) + } + prompt := s.buildPRPrompt(prData) summary, err := s.aiService.GeneratePRSummary(ctx, prompt) @@ -48,6 +74,28 @@ func (s *PRService) SummarizePR(ctx context.Context, prNumber int) (models.PRSum return models.PRSummary{}, fmt.Errorf("%s", msg) } + if len(prData.RelatedIssues) > 0 { + summary = s.ensurePRIssueReferences(summary, prData.RelatedIssues) + + fmt.Printf("%s\n", s.trans.GetMessage("pr_issues_will_close_on_merge", 0, map[string]interface{}{ + "Count": len(prData.RelatedIssues), + })) + } + + breakingChanges := s.detectBreakingChanges(prData.Commits) + if len(breakingChanges) > 0 { + fmt.Printf("%s\n", s.trans.GetMessage("pr_breaking_changes_detected", 0, map[string]interface{}{ + "Count": len(breakingChanges), + })) + summary = s.addBreakingChangesToSummary(summary, breakingChanges) + } + + testPlan := s.generateTestPlan(prData) + if testPlan != "" { + fmt.Printf("%s\n", s.trans.GetMessage("pr_test_plan_generated", 0, nil)) + summary.Body += testPlan + } + err = s.vcsClient.UpdatePR(ctx, prNumber, summary) if err != nil { msg := s.trans.GetMessage("pr_service.error_update_pr", 0, map[string]interface{}{ @@ -62,7 +110,33 @@ func (s *PRService) SummarizePR(ctx context.Context, prNumber int) (models.PRSum func (s *PRService) buildPRPrompt(prData models.PRData) string { var prompt string - prompt += fmt.Sprintf("PR #%d by %s\n\n", prData.ID, prData.Creator) + prompt += fmt.Sprintf("PR #%d by %s\n", prData.ID, prData.Creator) + prompt += fmt.Sprintf("Branch: %s\n\n", prData.BranchName) + + commitCount := len(prData.Commits) + diffLines := strings.Count(prData.Diff, "\n") + filesChanged := strings.Count(prData.Diff, "diff --git") + + prompt += "📊 **Métricas:**\n" + prompt += fmt.Sprintf("- %d commits\n", commitCount) + prompt += fmt.Sprintf("- %d archivos cambiados\n", filesChanged) + prompt += fmt.Sprintf("- ~%d líneas en diff\n\n", diffLines) + + breakingChanges := s.detectBreakingChanges(prData.Commits) + if len(breakingChanges) > 0 { + prompt += "⚠️ **Breaking Changes detectados:**\n" + for _, bc := range breakingChanges { + prompt += fmt.Sprintf("- %s\n", bc) + } + prompt += "\n" + } + + if len(prData.RelatedIssues) > 0 { + locale := s.config.Language + issuesFormatted := ai.FormatIssuesForPrompt(prData.RelatedIssues, locale) + issueContext := fmt.Sprintf(ai.GetPRIssueContextInstructions(locale), issuesFormatted) + prompt += issueContext + "\n\n" + } prompt += "Commits:\n" for _, commit := range prData.Commits { @@ -70,8 +144,111 @@ func (s *PRService) buildPRPrompt(prData models.PRData) string { } prompt += "\n" - prompt += "Changes:\n" + prompt += "Archivos principales modificados:\n" + lines := strings.Split(prData.Diff, "\n") + fileCount := 0 + for _, line := range lines { + if strings.HasPrefix(line, "diff --git") { + parts := strings.Fields(line) + if len(parts) >= 4 { + file := strings.TrimPrefix(parts[2], "a/") + prompt += fmt.Sprintf("- %s\n", file) + fileCount++ + if fileCount >= 20 { + break + } + } + } + } + prompt += "\n" + + prompt += "Changes (diff completo):\n" prompt += prData.Diff return prompt } + +func (s *PRService) ensurePRIssueReferences(summary models.PRSummary, issues []models.Issue) models.PRSummary { + if len(issues) == 0 { + return summary + } + + var closingRefs []string + for _, issue := range issues { + keyword := "Closes" + issueTitleLower := strings.ToLower(issue.Title) + if strings.Contains(issueTitleLower, "bug") || + strings.Contains(issueTitleLower, "fix") || + strings.Contains(issueTitleLower, "error") { + keyword = "Fixes" + } + closingRefs = append(closingRefs, fmt.Sprintf("%s #%d", keyword, issue.Number)) + } + + closingLine := strings.Join(closingRefs, ", ") + + bodyLower := strings.ToLower(summary.Body) + hasClosingRefs := false + for _, issue := range issues { + if strings.Contains(bodyLower, fmt.Sprintf("closes #%d", issue.Number)) || + strings.Contains(bodyLower, fmt.Sprintf("fixes #%d", issue.Number)) || + strings.Contains(bodyLower, fmt.Sprintf("resolves #%d", issue.Number)) { + hasClosingRefs = true + break + } + } + + if !hasClosingRefs { + summary.Body = closingLine + "\n\n" + summary.Body + } + + return summary +} + +func (s *PRService) detectBreakingChanges(commits []models.Commit) []string { + var breaking []string + for _, commit := range commits { + msgLower := strings.ToLower(commit.Message) + if strings.Contains(msgLower, "breaking change") || + strings.Contains(msgLower, "breaking:") || + strings.Contains(msgLower, "!:") { + breaking = append(breaking, commit.Message) + } + } + return breaking +} + +func (s *PRService) generateTestPlan(prData models.PRData) string { + if len(prData.RelatedIssues) == 0 { + return "" + } + + var testPlan strings.Builder + testPlan.WriteString("\n\n## Test Plan\n\n") + + for _, issue := range prData.RelatedIssues { + testPlan.WriteString(fmt.Sprintf("- [ ] Verificar que #%d esté resuelto\n", issue.Number)) + } + + testPlan.WriteString("- [ ] Ejecutar tests existentes\n") + testPlan.WriteString("- [ ] Verificar que no hay regresiones\n") + + return testPlan.String() +} + +// addBreakingChangesToSummary agrega sección de breaking changes al resumen +func (s *PRService) addBreakingChangesToSummary(summary models.PRSummary, breakingChanges []string) models.PRSummary { + if len(breakingChanges) == 0 { + return summary + } + + var breakingSection strings.Builder + breakingSection.WriteString("\n\n## ⚠️ Breaking Changes\n\n") + + for _, change := range breakingChanges { + breakingSection.WriteString(fmt.Sprintf("- %s\n", change)) + } + + summary.Body += breakingSection.String() + return summary +} diff --git a/internal/services/pull_request_service_test.go b/internal/services/pull_request_service_test.go index 50cfeaf..5ddee44 100644 --- a/internal/services/pull_request_service_test.go +++ b/internal/services/pull_request_service_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "os" + "strings" "testing" "github.com/Tomas-vilte/MateCommit/internal/config" @@ -24,6 +25,7 @@ func TestPRService_SummarizePR_Success(t *testing.T) { mockVCS := new(MockVCSClient) mockAI := new(MockPRSummarizer) trans, err := i18n.NewTranslations("es", "../i18n/locales") + cfg := &config.Config{} require.NoError(t, err) prData := models.PRData{ @@ -43,10 +45,11 @@ func TestPRService_SummarizePR_Success(t *testing.T) { } mockVCS.On("GetPR", ctx, prNumber).Return(prData, nil) + mockVCS.On("GetPRIssues", ctx, mock.Anything, mock.Anything, mock.Anything).Return([]models.Issue(nil), nil) mockAI.On("GeneratePRSummary", ctx, mock.AnythingOfType("string")).Return(expectedSummary, nil) mockVCS.On("UpdatePR", ctx, prNumber, expectedSummary).Return(nil) - service := NewPRService(mockVCS, mockAI, trans) + service := NewPRService(mockVCS, mockAI, trans, cfg) // Act result, err := service.SummarizePR(ctx, prNumber) @@ -65,13 +68,14 @@ func TestPRService_SummarizePR_GetPRError(t *testing.T) { mockVCS := new(MockVCSClient) mockAI := new(MockPRSummarizer) trans, err := i18n.NewTranslations("es", "../i18n/locales") + cfg := &config.Config{} require.NoError(t, err) expectedError := errors.New("API error") mockVCS.On("GetPR", ctx, prNumber).Return(models.PRData{}, expectedError) - service := NewPRService(mockVCS, mockAI, trans) + service := NewPRService(mockVCS, mockAI, trans, cfg) // act _, err = service.SummarizePR(ctx, prNumber) @@ -89,6 +93,7 @@ func TestPRService_SummarizePR_GenerateError(t *testing.T) { mockVCS := new(MockVCSClient) mockAI := new(MockPRSummarizer) + cfg := &config.Config{} trans, err := i18n.NewTranslations("es", "../i18n/locales") require.NoError(t, err) @@ -102,9 +107,10 @@ func TestPRService_SummarizePR_GenerateError(t *testing.T) { expectedError := errors.New("AI failure") mockVCS.On("GetPR", ctx, prNumber).Return(prData, nil) + mockVCS.On("GetPRIssues", ctx, mock.Anything, mock.Anything, mock.Anything).Return([]models.Issue(nil), nil) mockAI.On("GeneratePRSummary", ctx, mock.Anything).Return(models.PRSummary{}, expectedError) - service := NewPRService(mockVCS, mockAI, trans) + service := NewPRService(mockVCS, mockAI, trans, cfg) // Act _, err = service.SummarizePR(ctx, prNumber) @@ -124,6 +130,7 @@ func TestPRService_SummarizePR_UpdateError(t *testing.T) { mockVCS := new(MockVCSClient) mockAI := new(MockPRSummarizer) trans, err := i18n.NewTranslations("es", "../i18n/locales") + cfg := &config.Config{} require.NoError(t, err) prData := models.PRData{ @@ -141,10 +148,11 @@ func TestPRService_SummarizePR_UpdateError(t *testing.T) { expectedError := errors.New("update failed") mockVCS.On("GetPR", ctx, prNumber).Return(prData, nil) + mockVCS.On("GetPRIssues", ctx, mock.Anything, mock.Anything, mock.Anything).Return([]models.Issue(nil), nil) mockAI.On("GeneratePRSummary", ctx, mock.Anything).Return(summary, nil) mockVCS.On("UpdatePR", ctx, prNumber, summary).Return(expectedError) - service := NewPRService(mockVCS, mockAI, trans) + service := NewPRService(mockVCS, mockAI, trans, cfg) _, err = service.SummarizePR(ctx, prNumber) @@ -153,6 +161,115 @@ func TestPRService_SummarizePR_UpdateError(t *testing.T) { mockAI.AssertExpectations(t) } +func TestPRService_SummarizePR_NilAIService(t *testing.T) { + ctx := context.Background() + trans, err := i18n.NewTranslations("es", "../i18n/locales") + require.NoError(t, err) + cfg := &config.Config{} + + service := NewPRService(nil, nil, trans, cfg) + + _, err = service.SummarizePR(ctx, 123) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "ai_missing_for_pr") +} + +func TestPRService_SummarizePR_WithRelatedIssues(t *testing.T) { + ctx := context.Background() + prNumber := 123 + + mockVCS := new(MockVCSClient) + mockAI := new(MockPRSummarizer) + trans, err := i18n.NewTranslations("es", "../i18n/locales") + require.NoError(t, err) + cfg := &config.Config{Language: "es"} + + prData := models.PRData{ + ID: prNumber, + BranchName: "fix/123-bug", + PRDescription: "Closes #456", + Commits: []models.Commit{{Message: "Fix #789"}}, + } + + relatedIssues := []models.Issue{ + {Number: 123, Title: "Bug 1"}, + {Number: 456, Title: "Bug 2"}, + {Number: 789, Title: "Bug 3"}, + } + + expectedSummary := models.PRSummary{ + Title: "Fix bugs", + Body: "Summary content", + } + + mockVCS.On("GetPR", ctx, prNumber).Return(prData, nil) + mockVCS.On("GetPRIssues", ctx, prData.BranchName, []string{"Fix #789"}, prData.PRDescription). + Return(relatedIssues, nil) + + mockAI.On("GeneratePRSummary", ctx, mock.MatchedBy(func(prompt string) bool { + return contextContains(prompt, "Bug 1", "Bug 2", "Bug 3") + })).Return(expectedSummary, nil) + + mockVCS.On("UpdatePR", ctx, prNumber, mock.MatchedBy(func(s models.PRSummary) bool { + return contextContains(s.Body, "Summary content", "Fixes #123", "Fixes #456", "Fixes #789", "## Test Plan") + })).Return(nil) + + service := NewPRService(mockVCS, mockAI, trans, cfg) + + _, err = service.SummarizePR(ctx, prNumber) + + assert.NoError(t, err) + mockVCS.AssertExpectations(t) + mockAI.AssertExpectations(t) +} + +func TestPRService_SummarizePR_BreakingChanges(t *testing.T) { + ctx := context.Background() + prNumber := 123 + + mockVCS := new(MockVCSClient) + mockAI := new(MockPRSummarizer) + trans, err := i18n.NewTranslations("es", "../i18n/locales") + require.NoError(t, err) + cfg := &config.Config{} + + prData := models.PRData{ + ID: prNumber, + Commits: []models.Commit{{Message: "feat!: breaking change here"}}, + } + + expectedSummary := models.PRSummary{Title: "Title", Body: "Body"} + + mockVCS.On("GetPR", ctx, prNumber).Return(prData, nil) + mockVCS.On("GetPRIssues", ctx, mock.Anything, mock.Anything, mock.Anything).Return([]models.Issue(nil), nil) + + mockAI.On("GeneratePRSummary", ctx, mock.MatchedBy(func(prompt string) bool { + return contextContains(prompt, "Breaking Changes detectados", "feat!: breaking change here") + })).Return(expectedSummary, nil) + + mockVCS.On("UpdatePR", ctx, prNumber, mock.MatchedBy(func(s models.PRSummary) bool { + return contextContains(s.Body, "## ⚠️ Breaking Changes", "- feat!: breaking change here") + })).Return(nil) + + service := NewPRService(mockVCS, mockAI, trans, cfg) + + _, err = service.SummarizePR(ctx, prNumber) + + assert.NoError(t, err) + mockVCS.AssertExpectations(t) + mockAI.AssertExpectations(t) +} + +func contextContains(s string, substrs ...string) bool { + for _, sub := range substrs { + if !strings.Contains(s, sub) { + return false + } + } + return true +} + func TestBuildPRPrompt(t *testing.T) { // Arrange prData := models.PRData{ @@ -172,12 +289,21 @@ func TestBuildPRPrompt(t *testing.T) { // Assert expected := `PR #456 by dev123 +Branch: + +📊 **Métricas:** +- 2 commits +- 1 archivos cambiados +- ~0 líneas en diff Commits: - feat: add new API - docs: update readme -Changes: +Archivos principales modificados: +- api.go + +Changes (diff completo): diff --git a/api.go b/api.go` assert.Equal(t, expected, prompt) @@ -237,7 +363,7 @@ func setupServices(t *testing.T, testConfig TestConfig) (*PRService, error) { return nil, err } - prService := NewPRService(githubClient, geminiSummarizer, trans) + prService := NewPRService(githubClient, geminiSummarizer, trans, cfg) return prService, nil }