feat: guidelines

internal/pierre/guidelines_test.go

@@ -0,0 +1,70 @@
+package pierre
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func TestParseGuidelinesFromStringValid(t *testing.T) {
+	md := "# Rule One\n\n  - Item A  \n\n# Rule Two\n"
+	lines, err := parseGuidelinesFromString(md)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	expected := []string{"# Rule One", "- Item A", "# Rule Two"}
+	if got, want := fmt.Sprint(lines), fmt.Sprint(expected); got != want {
+		t.Fatalf("expected %v, got %v", expected, lines)
+	}
+}
+
+func TestParseGuidelinesFromStringEmpty(t *testing.T) {
+	md := "\n   \n"
+	lines, err := parseGuidelinesFromString(md)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(lines) != 0 {
+		t.Fatalf("expected empty slice, got %d elements", len(lines))
+	}
+}
+
+func TestParseGuidelinesFromStringTooManyLines(t *testing.T) {
+	// generate 1001 non-empty lines
+	var sb strings.Builder
+	for i := 0; i < 1001; i++ {
+		sb.WriteString(fmt.Sprintf("Line %d\n", i))
+	}
+	_, err := parseGuidelinesFromString(sb.String())
+	if err == nil {
+		t.Fatalf("expected error for exceeding line limit, got nil")
+	}
+}
+
+func TestWithGuidelinesSuccess(t *testing.T) {
+	svc := &Service{}
+	md := "First line\nSecond line\n"
+	if err := svc.WithGuidelines(md); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	expected := []string{"First line", "Second line"}
+	if got, want := fmt.Sprint(svc.guidelines), fmt.Sprint(expected); got != want {
+		t.Fatalf("expected guidelines %v, got %v", expected, svc.guidelines)
+	}
+}
+
+func TestWithGuidelinesError(t *testing.T) {
+	svc := &Service{guidelines: []string{"old"}}
+	var sb strings.Builder
+	for i := 0; i < 1001; i++ {
+		sb.WriteString("x\n")
+	}
+	err := svc.WithGuidelines(sb.String())
+	if err == nil {
+		t.Fatalf("expected error, got nil")
+	}
+	// ensure old guidelines unchanged
+	if len(svc.guidelines) != 1 || svc.guidelines[0] != "old" {
+		t.Fatalf("guidelines should remain unchanged on error")
+	}
+}

internal/pierre/overlap_test.go

@@ -0,0 +1,44 @@
+package pierre
+
+import (
+	"context"
+	"testing"
+
+	"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter"
+)
+
+type overlapChat struct{ callCount int }
+
+func (m *overlapChat) GenerateStructured(ctx context.Context, msgs []chatter.Message, target interface{}) error {
+	m.callCount++
+	if cSlice, ok := target.(*[]Comment); ok {
+		// Return two comments with same file and line to test deduplication
+		*cSlice = []Comment{{File: "dup.go", Line: 10, Message: "first"}, {File: "dup.go", Line: 10, Message: "second"}}
+		return nil
+	}
+	return nil
+}
+
+func (m *overlapChat) GetProviderName() string { return "mock" }
+
+func TestJudgePR_DeduplicationOverlap(t *testing.T) {
+	chat := &overlapChat{}
+	svc := &Service{
+		maxChunkSize: 1000,
+		guidelines:   nil,
+		git:          &mockGit{},
+		chat:         chat,
+	}
+	diffReader, err := svc.git.GetDiff(context.Background(), "", "", 0)
+	if err != nil {
+		t.Fatalf("failed to get diff: %v", err)
+	}
+	defer diffReader.Close()
+	comments, err := svc.judgePR(context.Background(), diffReader)
+	if err != nil {
+		t.Fatalf("judgePR error: %v", err)
+	}
+	if len(comments) != 1 {
+		t.Fatalf("expected 1 deduplicated comment, got %d", len(comments))
+	}
+}

internal/pierre/resource.go

@@ -2,7 +2,9 @@ package pierre
 
 import (
 	"context"
+	"fmt"
 	"io"
+	"strings"
 
 	"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter"
 )
@@ -14,22 +16,63 @@ import (
 // elsewhere in the future.
 type Service struct {
 	maxChunkSize    int
-	guidelines      []string
+	guidelines      []string // stored as slice of lines; legacy, see WithGuidelines
+	skipSanityCheck bool     // if true, skip LLM sanity‑check prompts per comment
 	disableComments bool
 	git             GitAdapter
 	chat            ChatAdapter
 }
 
 func New(chat ChatAdapter, git GitAdapter, maxChunkSize int, guidelines []string, disableComments bool) *Service {
+	// Existing constructor retains slice based guidelines for backward compatibility.
 	return &Service{
 		git:             git,
 		chat:            chat,
 		maxChunkSize:    maxChunkSize,
 		guidelines:      guidelines,
+		skipSanityCheck: false,
 		disableComments: disableComments,
 	}
 }
 
+// WithGuidelines parses a raw Markdown string (or any multiline string) into
+// individual guideline lines, validates the line‑count (max 1000 non‑empty lines),
+// and stores the result in the Service. It returns an error if validation fails.
+// This is a convenience mutator for callers that have the guidelines as a
+// single string.
+func (s *Service) WithGuidelines(md string) error {
+	lines, err := parseGuidelinesFromString(md)
+	if err != nil {
+		return err
+	}
+	s.guidelines = lines
+	return nil
+}
+
+// parseGuidelinesFromString splits a markdown string into trimmed, non‑empty
+// lines and ensures the total number of lines does not exceed 1000.
+func (s *Service) SetSanityCheck(enabled bool) {
+	s.skipSanityCheck = !enabled
+}
+
+// parseGuidelinesFromString splits a markdown string into trimmed, non‑empty
+func parseGuidelinesFromString(md string) ([]string, error) {
+	var result []string
+	// Split on newline. Handles both \n and \r\n because TrimSpace removes \r.
+	rawLines := strings.Split(md, "\n")
+	for _, l := range rawLines {
+		trimmed := strings.TrimSpace(l)
+		if trimmed == "" {
+			continue
+		}
+		result = append(result, trimmed)
+	}
+	if len(result) > 1000 {
+		return nil, fmt.Errorf("guidelines exceed 1000 lines (found %d)", len(result))
+	}
+	return result, nil
+}
+
 type GitAdapter interface {
 	GetDiff(ctx context.Context, owner, repo string, prID int) (io.ReadCloser, error)
 	AddComment(ctx context.Context, owner, repo string, prID int, comment Comment) error

internal/pierre/review.go

@@ -3,7 +3,7 @@ package pierre
 import (
 	"context"
 	"fmt"
-	"log"
+	"log/slog"
 
 	"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter"
 )
@@ -22,17 +22,17 @@ func (s *Service) MakeReview(ctx context.Context, organisation string, repo stri
 		return fmt.Errorf("error judging PR: %w", err)
 	}
 
-	// ---------- Sanity‑check step (always enabled) ----------
+	// ---------- Sanity‑check step ----------
 	headSHA, err := s.git.GetPRHeadSHA(ctx, organisation, repo, prID)
 	if err != nil {
-		log.Printf("warning: could not fetch PR head SHA (%v); skipping sanity check", err)
-	} else {
+		slog.Warn("could not fetch PR head SHA", "error", err)
+	} else if !s.skipSanityCheck {
 		filtered := []Comment{}
 		for _, c := range comments {
 			// Retrieve full file content at the PR head
 			fileContent, fErr := s.git.GetFileContent(ctx, organisation, repo, c.File, headSHA)
 			if fErr != nil {
-				log.Printf("failed to fetch file %s: %v – keeping original comment", c.File, fErr)
+				slog.Warn("failed to fetch file", "path", c.File, "error", fErr)
 				filtered = append(filtered, c)
 				continue
 			}
@@ -47,7 +47,7 @@ func (s *Service) MakeReview(ctx context.Context, organisation string, repo stri
 			}
 			var res sanityResult
 			if err := s.chat.GenerateStructured(ctx, []chatter.Message{{Role: chatter.RoleSystem, Content: systemPrompt}, {Role: chatter.RoleUser, Content: userPrompt}}, &res); err != nil {
-				log.Printf("sanity check error for %s:%d: %v – keeping comment", c.File, c.Line, err)
+				slog.Error("sanity check error", "file", c.File, "line", c.Line, "error", err)
 				filtered = append(filtered, c)
 				continue
 			}
@@ -56,7 +56,7 @@ func (s *Service) MakeReview(ctx context.Context, organisation string, repo stri
 				c.Message = fmt.Sprintf("%s (Reason: %s)", c.Message, res.Reason)
 				filtered = append(filtered, c)
 			} else {
-				log.Printf("comment on %s:%d discarded: %s", c.File, c.Line, res.Reason)
+				slog.Info("comment discarded", "file", c.File, "line", c.Line, "reason", res.Reason)
 			}
 		}
 		comments = filtered
@@ -75,7 +75,7 @@ func (s *Service) MakeReview(ctx context.Context, organisation string, repo stri
 
 		if !s.disableComments {
 			if err := s.git.AddComment(ctx, organisation, repo, prID, c); err != nil {
-				log.Printf("Failed to add comment: %v", err)
+				slog.Error("failed to add comment", "error", err)
 			}
 		}
 	}