schreifuchs/pierre-bot
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat: guidelines

Browse Source
This commit is contained in:
u80864958
2026-02-13 18:20:57 +01:00
parent 2e12c39786
commit 9f6c6830a6
3 changed files with 172 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
AGENTS.md
View File

@@ -16,7 +16,7 @@ layout: default
## 1️⃣ Build / Run Commands
| Command | Description |
|--------|-------------|
| --------------------------------------------- | --------------------------------------------------- |
| `go build -o bin/pierre ./cmd/pierre/main.go` | Build the binary into `./bin/pierre`. |
| `go run ./cmd/pierre/main.go --help` | Run the CLI with the help flag (fast sanity check). |
| `./bin/pierre --version` | Verify the built binary reports its version. |
@@ -30,7 +30,7 @@ layout: default
The project uses the standard Go testing framework.
| Command | Use case |
|---------|----------|
| ------------------------------------- | -------------------------------------------------------------------- |
| `go test ./...` | Run **all** unit and integration tests. |
| `go test ./... -run ^TestJudgePR$` | Run a **single** test (replace `TestJudgePR` with the desired name). |
| `go test -cover ./...` | Run tests and emit a coverage report. |
@@ -47,7 +47,7 @@ We rely on two lightweight built‑in tools plus optional `golangci-lint` when
available.
| Tool | Command | Description |
|------|---------|-------------|
| -------------------------- | ------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `go vet ./...` | `go vet ./...` | Detect obvious bugs, misuse of `fmt.Printf`, etc. |
| `staticcheck ./...` | `staticcheck ./...` | Deeper static analysis (must be installed via `go install honnef.co/go/tools/cmd/staticcheck@latest`). |
| `golangci-lint` (optional) | `golangci-lint run` | Runs a configurable suite of linters. Install with `brew install golangci-lint` or `go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest`. |
@@ -61,20 +61,23 @@ use the same commands.
## 4️⃣ Code‑Style Guidelines (Go)
All changes must satisfy the following conventions. They are enforced by
`go fmt`, `go vet`, and the optional `golangci-lint` config.
`go fmt`, `go vet`, and the optional `golangci-lint` config. The code does not have
to be backwards compatible.
### 4.1 Formatting & Imports
* Run `go fmt ./...` before committing. The repository uses **tab‑indented** Go code.
* Imports are grouped in three blocks, separated by a blank line:
- Run `go fmt ./...` before committing. The repository uses **tab‑indented** Go code.
- Imports are grouped in three blocks, separated by a blank line:
1. **Standard library**
2. **External dependencies** (e.g., `github.com/alecthomas/kong`)
3. **Internal packages** (e.g., `github.com/yourorg/pierre/internal/...`)
* Within each block, imports are sorted alphabetically.
* Use `goimports` (or `golangci‑lint --fast`) to automatically fix import ordering.
- Within each block, imports are sorted alphabetically.
- Use `goimports` (or `golangci‑lint --fast`) to automatically fix import ordering.
### 4.2 Naming Conventions
| Entity | Rule |
|--------|------|
| --------------------- | ---------------------------------------------------------------------- |
| Packages | lower‑case, no underscores or hyphens. |
| Files | snake_case (`*_test.go` for tests). |
| Types / Structs | PascalCase, descriptive (e.g., `ReviewConfig`). |
@@ -85,41 +88,46 @@ All changes must satisfy the following conventions. They are enforced by
| Test Functions | `TestXxx` and optionally `BenchmarkXxx`. |
### 4.3 Error Handling
* Errors are **never** ignored. Use the blank identifier only when the value is truly irrelevant.
* Wrap contextual information using `fmt.Errorf("context: %w", err)` or `errors.Wrap` if the project imports `github.com/pkg/errors` (currently not used).
* Return errors **as soon as** they are detected – "guard clause" style.
* In public APIs, prefer error values over panics. Panics are limited to unrecoverable
- Errors are **never** ignored. Use the blank identifier only when the value is truly irrelevant.
- Wrap contextual information using `fmt.Errorf("context: %w", err)` or `errors.Wrap` if the project imports `github.com/pkg/errors` (currently not used).
- Return errors **as soon as** they are detected – "guard clause" style.
- In public APIs, prefer error values over panics. Panics are limited to unrecoverable
programming errors (e.g., nil‑pointer dereference in `init`).
### 4.4 Documentation & Comments
* Exported identifiers **must** have a preceding doc comment beginning with the name (e.g., `// JudgePR reviews a PR and returns comments.`).
* Inline comments should be sentence‑case and end with a period.
* Use `//go:generate` directives sparingly; they must be accompanied by a test that ensures the generated file is up‑to‑date.
- Exported identifiers **must** have a preceding doc comment beginning with the name (e.g., `// JudgePR reviews a PR and returns comments.`).
- Inline comments should be sentence‑case and end with a period.
- Use `//go:generate` directives sparingly; they must be accompanied by a test that ensures the generated file is up‑to‑date.
### 4.5 Testing Practices
* Keep tests **table‑driven** where possible; this yields concise, readable test suites.
* Use the `testing` package only; avoid third‑party test frameworks.
* Prefer `t.Fatalf` for fatal errors and `t.Errorf` for non‑fatal assertions.
* When comparing complex structs, use `github.com/google/go-cmp/cmp` (already in `go.mod`).
* Test coverage should be **≥ 80 %** for new code.
* All tests must be **deterministic** – no reliance on external services; use mocks or fakes.
- Keep tests **table‑driven** where possible; this yields concise, readable test suites.
- Use the `testing` package only; avoid third‑party test frameworks.
- Prefer `t.Fatalf` for fatal errors and `t.Errorf` for non‑fatal assertions.
- When comparing complex structs, use `github.com/google/go-cmp/cmp` (already in `go.mod`).
- Test coverage should be **≥ 80 %** for new code.
- All tests must be **deterministic** – no reliance on external services; use mocks or fakes.
### 4.6 Dependency Management
* All dependencies are managed via Go modules (`go.mod`).
* Run `go mod tidy` after adding/removing imports.
* Do not commit `vendor/` unless a vendoring strategy is explicitly adopted.
- All dependencies are managed via Go modules (`go.mod`).
- Run `go mod tidy` after adding/removing imports.
- Do not commit `vendor/` unless a vendoring strategy is explicitly adopted.
### 4.7 Logging & Output
* Use the standard library `log` package for user‑visible output.
* For structured logs (debug mode), wrap with a small helper that respects the `--debug` flag.
* CLI output must be **machine‑parsable** when `--json-output` is set (future feature).
- Use the standard library `log` package for user‑visible output.
- For structured logs (debug mode), wrap with a small helper that respects the `--debug` flag.
- CLI output must be **machine‑parsable** when `--json-output` is set (future feature).
---
## 5️⃣ Git & Pull‑Request Workflow (for agents)
1. **Branch naming** – `feature/<short‑desc>` or `bugfix/<short‑desc>`.
2. **Commits** – One logical change per commit, with a concise subject line (<50 chars) and an optional body explaining *why* the change was needed.
2. **Commits** – One logical change per commit, with a concise subject line (<50 chars) and an optional body explaining _why_ the change was needed.
3. **CI** – Every PR must pass `go test ./...`, `go vet ./...`, and `golangci‑lint run` (if configured).
4. **Review** – Agents should add comments only via the LLM; do not edit code generated by the LLM unless a test fails.
5. **Rebasing** – Keep a linear history; use `git rebase -i` locally before merging.
@@ -159,4 +167,5 @@ go mod tidy
---
*End of AGENTS.md*
_End of AGENTS.md_

70
internal/pierre/guidelines_test.go Normal file
View File

@@ -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")
}
}

38
internal/pierre/resource.go
View File

@@ -2,7 +2,9 @@ package pierre
import (
"context"
"fmt"
"io"
"strings"
"git.schreifuchs.ch/schreifuchs/pierre-bot/internal/chatter"
)
@@ -14,13 +16,14 @@ import (
// elsewhere in the future.
type Service struct {
maxChunkSize int
guidelines []string
guidelines []string // stored as slice of lines; legacy, see WithGuidelines
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,
@@ -30,6 +33,39 @@ func New(chat ChatAdapter, git GitAdapter, maxChunkSize int, guidelines []string
}
}
// 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 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