refactor(recipe): simplify apply step to dry-run only and add extraction tests
- Restrict filesystem interactions to apply/patch steps exclusively. - Remove real apply logic with user confirmation; default to creating a patch file in dry-run mode. - Update prompts, comments, and regex for better clarity and precision. - Add unit tests for the extractCodeBlocks function to ensure reliable parsing.
This commit is contained in:
parent
5d0aec721d
commit
b9de35f48b
@ -1,7 +1,7 @@
|
|||||||
package recipe
|
package recipe
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"bufio"
|
_ "bufio"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@ -29,13 +29,13 @@ func (r *Runner) Run() error {
|
|||||||
for _, step := range r.Recipe.Steps {
|
for _, step := range r.Recipe.Steps {
|
||||||
fmt.Printf("Step %d/%d: %s\n", step.Number, len(r.Recipe.Steps), step.Title)
|
fmt.Printf("Step %d/%d: %s\n", step.Number, len(r.Recipe.Steps), step.Title)
|
||||||
|
|
||||||
// Special case: Apply or patch step is handled by the CLI (like edit/scaffold)
|
// Only special-case the Apply/Patch step (this is the only place the CLI needs to touch disk)
|
||||||
if strings.Contains(strings.ToLower(step.Title), "apply") || strings.Contains(strings.ToLower(step.Title), "patch") {
|
if strings.Contains(strings.ToLower(step.Title), "apply") || strings.Contains(strings.ToLower(step.Title), "patch") {
|
||||||
r.handleApplyStep(previousResults)
|
r.handleApplyStep(previousResults)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
// Normal LLM step
|
// Everything else is pure LLM — the recipe defines exactly what to do
|
||||||
prompt := fmt.Sprintf(`Recipe Overview:
|
prompt := fmt.Sprintf(`Recipe Overview:
|
||||||
%s
|
%s
|
||||||
|
|
||||||
@ -47,7 +47,7 @@ Objective: %s
|
|||||||
Instructions: %s
|
Instructions: %s
|
||||||
Expected output format: %s
|
Expected output format: %s
|
||||||
|
|
||||||
Execute this step now. Respond ONLY with the expected output format — no explanations, no extra text.`,
|
Execute this step now.`,
|
||||||
r.Recipe.Overview,
|
r.Recipe.Overview,
|
||||||
strings.Join(previousResults, "\n\n---\n\n"),
|
strings.Join(previousResults, "\n\n---\n\n"),
|
||||||
step.Objective,
|
step.Objective,
|
||||||
@ -55,7 +55,7 @@ Execute this step now. Respond ONLY with the expected output format — no expla
|
|||||||
step.Expected)
|
step.Expected)
|
||||||
|
|
||||||
messages := []map[string]string{
|
messages := []map[string]string{
|
||||||
{"role": "system", "content": "You are Grok, built by xAI. You are a precise, expert Go programmer and refactoring assistant. Always follow the user's instructions exactly for legitimate coding tasks."},
|
{"role": "system", "content": "You are Grok, built by xAI. You are a precise, expert programmer and refactoring assistant. Always follow the user's instructions exactly for legitimate coding tasks."},
|
||||||
{"role": "user", "content": prompt},
|
{"role": "user", "content": prompt},
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -69,14 +69,13 @@ Execute this step now. Respond ONLY with the expected output format — no expla
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// handleApplyStep parses the refactored code blocks from the previous step and does the real apply/patch with confirmation
|
// handleApplyStep is the ONLY place we touch the filesystem (exactly like edit/scaffold)
|
||||||
func (r *Runner) handleApplyStep(previousResults []string) {
|
func (r *Runner) handleApplyStep(previousResults []string) {
|
||||||
if len(previousResults) == 0 {
|
if len(previousResults) == 0 {
|
||||||
fmt.Println(" ⚠️ No previous results to apply — skipping.")
|
fmt.Println(" ⚠️ No previous results to apply — skipping.")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Extract all labelled code blocks from the last LLM step (Step 2)
|
|
||||||
lastResult := previousResults[len(previousResults)-1]
|
lastResult := previousResults[len(previousResults)-1]
|
||||||
blocks := extractCodeBlocks(lastResult)
|
blocks := extractCodeBlocks(lastResult)
|
||||||
|
|
||||||
@ -85,9 +84,7 @@ func (r *Runner) handleApplyStep(previousResults []string) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Dry-run or real apply?
|
// Dry-run by default (we'll wire parameters later)
|
||||||
dryRun := true // TODO: read from parameters once we add --param support
|
|
||||||
if dryRun {
|
|
||||||
fmt.Println(" 📄 Dry-run mode: creating patch file...")
|
fmt.Println(" 📄 Dry-run mode: creating patch file...")
|
||||||
patchPath := filepath.Join(".", "recipe-refactor.patch")
|
patchPath := filepath.Join(".", "recipe-refactor.patch")
|
||||||
if err := createUnifiedPatch(blocks, patchPath); err != nil {
|
if err := createUnifiedPatch(blocks, patchPath); err != nil {
|
||||||
@ -96,34 +93,10 @@ func (r *Runner) handleApplyStep(previousResults []string) {
|
|||||||
}
|
}
|
||||||
fmt.Printf(" ✅ Patch created: %s\n", patchPath)
|
fmt.Printf(" ✅ Patch created: %s\n", patchPath)
|
||||||
fmt.Println(" Review it, then run with dry_run=false to apply.")
|
fmt.Println(" Review it, then run with dry_run=false to apply.")
|
||||||
return
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Real apply with confirmation (exactly like edit/scaffold)
|
// Simple regex for the format the recipe asks Grok to return
|
||||||
fmt.Print("\nApply these changes to disk? (y/N) ")
|
var regStr = "`(?s)^//\\s*(.+?\\.go)\\n```go\\n(.*?)\\n````"
|
||||||
scanner := bufio.NewScanner(os.Stdin)
|
|
||||||
if scanner.Scan() {
|
|
||||||
answer := strings.ToLower(strings.TrimSpace(scanner.Text()))
|
|
||||||
if answer == "y" || answer == "yes" {
|
|
||||||
for filePath, content := range blocks {
|
|
||||||
backup := filePath + ".bak"
|
|
||||||
if err := os.Rename(filePath, backup); err == nil {
|
|
||||||
fmt.Printf(" 📦 Backed up: %s\n", backup)
|
|
||||||
}
|
|
||||||
if err := os.WriteFile(filePath, []byte(content), 0644); err != nil {
|
|
||||||
fmt.Printf(" ❌ Failed to write %s: %v\n", filePath, err)
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
fmt.Printf(" ✅ Applied: %s\n", filePath)
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
fmt.Println(" ❌ Cancelled — no changes made.")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Simple parser for the labelled blocks Grok outputs: "// /path/to/file.go\n```go\ncode\n```"
|
|
||||||
var regStr = "`(?s)//\\s*(.+?\\.go)\\n```go\\n(.*?)\\n````"
|
|
||||||
var blockRe = regexp.MustCompile(regStr)
|
var blockRe = regexp.MustCompile(regStr)
|
||||||
|
|
||||||
func extractCodeBlocks(text string) map[string]string {
|
func extractCodeBlocks(text string) map[string]string {
|
||||||
@ -138,7 +111,6 @@ func extractCodeBlocks(text string) map[string]string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func createUnifiedPatch(blocks map[string]string, patchPath string) error {
|
func createUnifiedPatch(blocks map[string]string, patchPath string) error {
|
||||||
// Very simple unified patch for now — can be improved later with real diff
|
|
||||||
f, err := os.Create(patchPath)
|
f, err := os.Create(patchPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
|||||||
61
internal/recipe/runner_test.go
Normal file
61
internal/recipe/runner_test.go
Normal file
@ -0,0 +1,61 @@
|
|||||||
|
package recipe
|
||||||
|
|
||||||
|
import (
|
||||||
|
"reflect"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestExtractCodeBlocks(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
input string
|
||||||
|
expected map[string]string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "Single block",
|
||||||
|
input: "// main.go\n```go\npackage main\n\nfunc main() {}\n```",
|
||||||
|
expected: map[string]string{
|
||||||
|
"main.go": "package main\n\nfunc main() {}",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Multiple blocks",
|
||||||
|
input: `// internal/utils.go
|
||||||
|
` + "```" + `go
|
||||||
|
package utils
|
||||||
|
func Help() {}
|
||||||
|
` + "```" + `
|
||||||
|
Some commentary.
|
||||||
|
// cmd/root.go
|
||||||
|
` + "```" + `go
|
||||||
|
package cmd
|
||||||
|
func Execute() {}
|
||||||
|
` + "```",
|
||||||
|
expected: map[string]string{
|
||||||
|
"internal/utils.go": "package utils\nfunc Help() {}",
|
||||||
|
"cmd/root.go": "package cmd\nfunc Execute() {}",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "No blocks",
|
||||||
|
input: "Just some text without any blocks.",
|
||||||
|
expected: map[string]string{},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Incomplete block",
|
||||||
|
input: "// oops.go\n```go\nfunc incomplete() {",
|
||||||
|
expected: map[string]string{},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
got := extractCodeBlocks(tt.input)
|
||||||
|
if !reflect.DeepEqual(got, tt.expected) {
|
||||||
|
t.Errorf("extractCodeBlocks() = %v, want %v", got, tt.expected)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue
Block a user