Skip to content

Commit 464edff

Browse files
committed
Adopt from starred repos: checks-as-files, filter modes, custom concerns
From continuedev/continue: - Checks-as-markdown-files: .sight/checks/*.md with YAML frontmatter - LoadChecks(), WithCustomChecks(), WithCustomChecksFromRepo() options - Custom checks inject as additional LLM review concerns From reviewdog/reviewdog: - FilterMode (FilterAdded, FilterDiffContext, FilterFile, FilterNone) - WithFilterMode() option controls which lines get comments - MapToInlineFiltered() with backward-compatible MapToInline() All tests pass including new filter mode and checks tests.
1 parent 9282adb commit 464edff

6 files changed

Lines changed: 585 additions & 20 deletions

File tree

checks.go

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
package sight
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
8+
"github.com/GrayCodeAI/sight/internal/review"
9+
)
10+
11+
// CustomCheck represents a user-defined review check loaded from a markdown file
12+
// in the .sight/checks/ directory. Each file becomes a check whose content is
13+
// injected into the LLM prompt as an additional concern.
14+
type CustomCheck struct {
15+
// Name is derived from the filename (e.g., "no-console-log" from no-console-log.md).
16+
Name string
17+
18+
// Prompt is the markdown body that describes the check rules and gets
19+
// injected into the LLM system prompt.
20+
Prompt string
21+
22+
// Severity is the default severity for findings from this check.
23+
// Parsed from YAML frontmatter; defaults to "medium".
24+
Severity string
25+
26+
// Languages restricts the check to files matching these extensions
27+
// (e.g., ["go", "py"]). Empty means all languages.
28+
Languages []string
29+
30+
// Enabled controls whether the check is active. Defaults to true.
31+
Enabled bool
32+
}
33+
34+
// LoadChecks reads all markdown files from the given directory (typically
35+
// ".sight/checks/") and parses them into CustomCheck values. Each .md file
36+
// becomes one check. YAML frontmatter between --- delimiters is parsed for
37+
// metadata (severity, languages, enabled); the remaining body becomes the
38+
// check prompt.
39+
//
40+
// Returns an empty slice (not an error) if the directory does not exist.
41+
func LoadChecks(dir string) ([]CustomCheck, error) {
42+
entries, err := os.ReadDir(dir)
43+
if err != nil {
44+
if os.IsNotExist(err) {
45+
return nil, nil
46+
}
47+
return nil, err
48+
}
49+
50+
var checks []CustomCheck
51+
for _, entry := range entries {
52+
if entry.IsDir() {
53+
continue
54+
}
55+
if !strings.HasSuffix(entry.Name(), ".md") {
56+
continue
57+
}
58+
59+
path := filepath.Join(dir, entry.Name())
60+
data, err := os.ReadFile(path)
61+
if err != nil {
62+
continue
63+
}
64+
65+
name := strings.TrimSuffix(entry.Name(), ".md")
66+
check := parseCheckFile(name, string(data))
67+
checks = append(checks, check)
68+
}
69+
70+
return checks, nil
71+
}
72+
73+
// LoadChecksFromRepo is a convenience that looks for .sight/checks/ relative
74+
// to the given repository root directory.
75+
func LoadChecksFromRepo(repoDir string) ([]CustomCheck, error) {
76+
return LoadChecks(filepath.Join(repoDir, ".sight", "checks"))
77+
}
78+
79+
// CustomChecksToConcerns converts loaded custom checks into internal Concern
80+
// values suitable for the review pipeline. Only enabled checks are included.
81+
// If languages is non-empty, the concern prompt notes which languages apply.
82+
func CustomChecksToConcerns(checks []CustomCheck) []review.Concern {
83+
var concerns []review.Concern
84+
for _, c := range checks {
85+
if !c.Enabled {
86+
continue
87+
}
88+
prompt := c.Prompt
89+
if len(c.Languages) > 0 {
90+
prompt += "\n\nThis check applies only to files with these extensions: " +
91+
strings.Join(c.Languages, ", ")
92+
}
93+
if c.Severity != "" {
94+
prompt += "\n\nDefault severity for issues found by this check: " + c.Severity
95+
}
96+
concerns = append(concerns, review.Concern{
97+
Name: "custom:" + c.Name,
98+
Prompt: prompt,
99+
})
100+
}
101+
return concerns
102+
}
103+
104+
// WithCustomChecks loads checks from the given directory and appends them as
105+
// additional concerns to the review. This is the primary integration point:
106+
//
107+
// sight.Review(ctx, diff, sight.WithCustomChecks(".sight/checks"))
108+
func WithCustomChecks(dir string) Option {
109+
return optFunc(func(c *config) {
110+
checks, err := LoadChecks(dir)
111+
if err != nil || len(checks) == 0 {
112+
return
113+
}
114+
concerns := CustomChecksToConcerns(checks)
115+
for _, concern := range concerns {
116+
c.concerns = append(c.concerns, concern.Name)
117+
}
118+
c.customConcerns = append(c.customConcerns, concerns...)
119+
})
120+
}
121+
122+
// WithCustomChecksFromRepo loads checks from .sight/checks/ within the repo root.
123+
func WithCustomChecksFromRepo(repoDir string) Option {
124+
return WithCustomChecks(filepath.Join(repoDir, ".sight", "checks"))
125+
}
126+
127+
// parseCheckFile parses a markdown file into a CustomCheck. It extracts YAML
128+
// frontmatter between --- delimiters for metadata and uses the remaining
129+
// content as the prompt.
130+
func parseCheckFile(name, content string) CustomCheck {
131+
check := CustomCheck{
132+
Name: name,
133+
Enabled: true,
134+
Severity: "medium",
135+
}
136+
137+
content = strings.TrimSpace(content)
138+
139+
// Parse YAML frontmatter if present
140+
if strings.HasPrefix(content, "---") {
141+
parts := strings.SplitN(content[3:], "---", 2)
142+
if len(parts) == 2 {
143+
parseFrontmatter(&check, strings.TrimSpace(parts[0]))
144+
content = strings.TrimSpace(parts[1])
145+
}
146+
}
147+
148+
check.Prompt = content
149+
return check
150+
}
151+
152+
// parseFrontmatter extracts metadata from a simplified YAML frontmatter block.
153+
// Supports: severity, languages (comma-separated), enabled (true/false).
154+
func parseFrontmatter(check *CustomCheck, fm string) {
155+
for _, line := range strings.Split(fm, "\n") {
156+
line = strings.TrimSpace(line)
157+
if line == "" || strings.HasPrefix(line, "#") {
158+
continue
159+
}
160+
161+
parts := strings.SplitN(line, ":", 2)
162+
if len(parts) != 2 {
163+
continue
164+
}
165+
key := strings.TrimSpace(parts[0])
166+
value := strings.TrimSpace(parts[1])
167+
168+
switch key {
169+
case "severity":
170+
value = strings.ToLower(value)
171+
switch value {
172+
case "info", "low", "medium", "high", "critical":
173+
check.Severity = value
174+
}
175+
case "languages":
176+
langs := strings.Split(value, ",")
177+
for _, l := range langs {
178+
l = strings.TrimSpace(l)
179+
l = strings.Trim(l, "[]\"'")
180+
if l != "" {
181+
check.Languages = append(check.Languages, l)
182+
}
183+
}
184+
case "enabled":
185+
check.Enabled = strings.ToLower(value) != "false"
186+
}
187+
}
188+
}

checks_test.go

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
package sight
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
func TestLoadChecks_NoDirectory(t *testing.T) {
10+
checks, err := LoadChecks("/nonexistent/path/to/checks")
11+
if err != nil {
12+
t.Fatalf("expected nil error for missing dir, got %v", err)
13+
}
14+
if len(checks) != 0 {
15+
t.Errorf("expected 0 checks, got %d", len(checks))
16+
}
17+
}
18+
19+
func TestLoadChecks_BasicFile(t *testing.T) {
20+
dir := t.TempDir()
21+
content := `---
22+
severity: high
23+
languages: go, py
24+
enabled: true
25+
---
26+
Do not use console.log or fmt.Println for debugging.
27+
Remove all debug logging before merge.
28+
`
29+
if err := os.WriteFile(filepath.Join(dir, "no-debug-logging.md"), []byte(content), 0644); err != nil {
30+
t.Fatal(err)
31+
}
32+
33+
checks, err := LoadChecks(dir)
34+
if err != nil {
35+
t.Fatalf("LoadChecks failed: %v", err)
36+
}
37+
if len(checks) != 1 {
38+
t.Fatalf("expected 1 check, got %d", len(checks))
39+
}
40+
41+
c := checks[0]
42+
if c.Name != "no-debug-logging" {
43+
t.Errorf("expected name 'no-debug-logging', got %q", c.Name)
44+
}
45+
if c.Severity != "high" {
46+
t.Errorf("expected severity 'high', got %q", c.Severity)
47+
}
48+
if len(c.Languages) != 2 {
49+
t.Fatalf("expected 2 languages, got %d", len(c.Languages))
50+
}
51+
if c.Languages[0] != "go" || c.Languages[1] != "py" {
52+
t.Errorf("expected [go, py], got %v", c.Languages)
53+
}
54+
if !c.Enabled {
55+
t.Error("expected enabled=true")
56+
}
57+
if c.Prompt == "" {
58+
t.Error("expected non-empty prompt")
59+
}
60+
}
61+
62+
func TestLoadChecks_NoFrontmatter(t *testing.T) {
63+
dir := t.TempDir()
64+
content := "Check that all errors are wrapped with context before returning."
65+
if err := os.WriteFile(filepath.Join(dir, "wrap-errors.md"), []byte(content), 0644); err != nil {
66+
t.Fatal(err)
67+
}
68+
69+
checks, err := LoadChecks(dir)
70+
if err != nil {
71+
t.Fatalf("LoadChecks failed: %v", err)
72+
}
73+
if len(checks) != 1 {
74+
t.Fatalf("expected 1 check, got %d", len(checks))
75+
}
76+
77+
c := checks[0]
78+
if c.Name != "wrap-errors" {
79+
t.Errorf("expected name 'wrap-errors', got %q", c.Name)
80+
}
81+
if c.Severity != "medium" {
82+
t.Errorf("expected default severity 'medium', got %q", c.Severity)
83+
}
84+
if !c.Enabled {
85+
t.Error("expected enabled=true by default")
86+
}
87+
if c.Prompt != content {
88+
t.Errorf("expected prompt to match content")
89+
}
90+
}
91+
92+
func TestLoadChecks_DisabledCheck(t *testing.T) {
93+
dir := t.TempDir()
94+
content := `---
95+
enabled: false
96+
---
97+
This check is disabled.
98+
`
99+
if err := os.WriteFile(filepath.Join(dir, "disabled-check.md"), []byte(content), 0644); err != nil {
100+
t.Fatal(err)
101+
}
102+
103+
checks, err := LoadChecks(dir)
104+
if err != nil {
105+
t.Fatalf("LoadChecks failed: %v", err)
106+
}
107+
if len(checks) != 1 {
108+
t.Fatalf("expected 1 check, got %d", len(checks))
109+
}
110+
if checks[0].Enabled {
111+
t.Error("expected enabled=false")
112+
}
113+
}
114+
115+
func TestLoadChecks_SkipsNonMarkdown(t *testing.T) {
116+
dir := t.TempDir()
117+
os.WriteFile(filepath.Join(dir, "check.md"), []byte("valid check"), 0644)
118+
os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("not a check"), 0644)
119+
os.WriteFile(filepath.Join(dir, "data.json"), []byte("{}"), 0644)
120+
121+
checks, err := LoadChecks(dir)
122+
if err != nil {
123+
t.Fatalf("LoadChecks failed: %v", err)
124+
}
125+
if len(checks) != 1 {
126+
t.Errorf("expected 1 check (only .md), got %d", len(checks))
127+
}
128+
}
129+
130+
func TestCustomChecksToConcerns_FiltersDisabled(t *testing.T) {
131+
checks := []CustomCheck{
132+
{Name: "active", Prompt: "check this", Enabled: true, Severity: "high"},
133+
{Name: "inactive", Prompt: "skip this", Enabled: false},
134+
}
135+
136+
concerns := CustomChecksToConcerns(checks)
137+
if len(concerns) != 1 {
138+
t.Fatalf("expected 1 concern, got %d", len(concerns))
139+
}
140+
if concerns[0].Name != "custom:active" {
141+
t.Errorf("expected 'custom:active', got %q", concerns[0].Name)
142+
}
143+
}
144+
145+
func TestLoadChecksFromRepo(t *testing.T) {
146+
dir := t.TempDir()
147+
checksDir := filepath.Join(dir, ".sight", "checks")
148+
if err := os.MkdirAll(checksDir, 0755); err != nil {
149+
t.Fatal(err)
150+
}
151+
os.WriteFile(filepath.Join(checksDir, "test.md"), []byte("test check"), 0644)
152+
153+
checks, err := LoadChecksFromRepo(dir)
154+
if err != nil {
155+
t.Fatalf("LoadChecksFromRepo failed: %v", err)
156+
}
157+
if len(checks) != 1 {
158+
t.Errorf("expected 1 check, got %d", len(checks))
159+
}
160+
}

0 commit comments

Comments
 (0)