agent: move "sketch-base" into git
The agent's notion of "initial commit" is kind of special, in that it
is used as the "base" for a bunch of git operations. It's hard for
the user to change (we only provide a workflow via restart), yet
sometimes you want to do just that.
So, instead we put it as data inside of it, named as a tag sketch-base.
It's abusing tags, but branches are no better.
diff --git a/claudetool/codereview/codereview.go b/claudetool/codereview/codereview.go
index 8051bf7..2745957 100644
--- a/claudetool/codereview/codereview.go
+++ b/claudetool/codereview/codereview.go
@@ -17,7 +17,7 @@
// A CodeReviewer manages quality checks.
type CodeReviewer struct {
repoRoot string
- initialCommit string
+ sketchBaseRef string
initialStatus []fileStatus // git status of files at initial commit, absolute paths
reviewed []string // history of all commits which have been reviewed
initialWorktree string // git worktree at initial commit, absolute path
@@ -29,17 +29,17 @@
DoLLMReview = true
)
-func NewCodeReviewer(ctx context.Context, repoRoot, initialCommit string, llmReview bool) (*CodeReviewer, error) {
+func NewCodeReviewer(ctx context.Context, repoRoot, sketchBaseRef string, llmReview bool) (*CodeReviewer, error) {
r := &CodeReviewer{
repoRoot: repoRoot,
- initialCommit: initialCommit,
+ sketchBaseRef: sketchBaseRef,
llmReview: llmReview,
}
if r.repoRoot == "" {
return nil, fmt.Errorf("NewCodeReviewer: repoRoot must be non-empty")
}
- if r.initialCommit == "" {
- return nil, fmt.Errorf("NewCodeReviewer: initialCommit must be non-empty")
+ if r.sketchBaseRef == "" {
+ return nil, fmt.Errorf("NewCodeReviewer: sketchBaseRef must be non-empty")
}
// Confirm that root is in fact the git repo root.
root, err := claudetool.FindRepoRoot(r.repoRoot)
@@ -65,7 +65,7 @@
// It is best-effort only.
func (r *CodeReviewer) autoformat(ctx context.Context) []string {
// Refuse to format if initial commit is not an ancestor of HEAD
- err := r.requireHEADDescendantOfInitialCommit(ctx)
+ err := r.requireHEADDescendantOfSketchBaseRef(ctx)
if err != nil {
slog.WarnContext(ctx, "CodeReviewer.Autoformat refusing to format", "err", err)
return nil
@@ -83,7 +83,7 @@
}
// Retrieve a list of all files changed
// TODO: instead of one git diff --name-only and then N --name-status, do one --name-status.
- changedFiles, err := r.changedFiles(ctx, r.initialCommit, head)
+ changedFiles, err := r.changedFiles(ctx, r.sketchBaseRef, head)
if err != nil {
slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to get changed files", "err", err)
return nil
@@ -252,7 +252,7 @@
// gitFileStatus returns the status of a file (A for added, M for modified, D for deleted, etc.)
func (r *CodeReviewer) gitFileStatus(ctx context.Context, file string) (string, error) {
- cmd := exec.CommandContext(ctx, "git", "diff", "--name-status", r.initialCommit, "HEAD", "--", file)
+ cmd := exec.CommandContext(ctx, "git", "diff", "--name-status", r.sketchBaseRef, "HEAD", "--", file)
cmd.Dir = r.repoRoot
out, err := cmd.CombinedOutput()
if err != nil {
@@ -361,7 +361,7 @@
// ModTidy runs go mod tidy if go module files have changed.
// Returns a list of files changed by go mod tidy (empty if none).
func (r *CodeReviewer) ModTidy(ctx context.Context) ([]string, error) {
- err := r.requireHEADDescendantOfInitialCommit(ctx)
+ err := r.requireHEADDescendantOfSketchBaseRef(ctx)
if err != nil {
return nil, fmt.Errorf("cannot run ModTidy: %w", err)
}
@@ -372,7 +372,7 @@
return nil, fmt.Errorf("failed to get current commit: %w", err)
}
- changedFiles, err := r.changedFiles(ctx, r.initialCommit, currentCommit)
+ changedFiles, err := r.changedFiles(ctx, r.sketchBaseRef, currentCommit)
if err != nil {
return nil, fmt.Errorf("failed to get changed files: %w", err)
}
diff --git a/claudetool/codereview/differential.go b/claudetool/codereview/differential.go
index 37728d4..f12b4f0 100644
--- a/claudetool/codereview/differential.go
+++ b/claudetool/codereview/differential.go
@@ -64,7 +64,7 @@
// This should help avoid the model getting blocked by a broken code review tool.
r.reviewed = append(r.reviewed, currentCommit)
- changedFiles, err := r.changedFiles(ctx, r.initialCommit, currentCommit)
+ changedFiles, err := r.changedFiles(ctx, r.sketchBaseRef, currentCommit)
if err != nil {
slog.DebugContext(ctx, "CodeReviewer.Run: failed to get changed files", "err", err)
return nil, err
@@ -154,7 +154,7 @@
if err != nil {
return err
}
- worktreeCmd := exec.CommandContext(ctx, "git", "worktree", "add", "--detach", tmpDir, r.initialCommit)
+ worktreeCmd := exec.CommandContext(ctx, "git", "worktree", "add", "--detach", tmpDir, r.sketchBaseRef)
worktreeCmd.Dir = r.repoRoot
out, err := worktreeCmd.CombinedOutput()
if err != nil {
@@ -274,7 +274,7 @@
}
// Check if the file exists in the initial commit
- checkCmd := exec.CommandContext(ctx, "git", "cat-file", "-e", fmt.Sprintf("%s:%s", r.initialCommit, relFile))
+ checkCmd := exec.CommandContext(ctx, "git", "cat-file", "-e", fmt.Sprintf("%s:%s", r.sketchBaseRef, relFile))
checkCmd.Dir = r.repoRoot
if err := checkCmd.Run(); err == nil {
// File exists in initial commit
@@ -491,21 +491,21 @@
}
func (r *CodeReviewer) IsInitialCommit(commit string) bool {
- return commit == r.initialCommit
+ return commit == r.sketchBaseRef
}
-// requireHEADDescendantOfInitialCommit returns an error if HEAD is not a descendant of r.initialCommit.
+// requireHEADDescendantOfSketchBaseRef returns an error if HEAD is not a descendant of r.initialCommit.
// This serves two purposes:
// - ensures we're not still on the initial commit
// - ensures we're not on a separate branch or upstream somewhere, which would be confusing
-func (r *CodeReviewer) requireHEADDescendantOfInitialCommit(ctx context.Context) error {
+func (r *CodeReviewer) requireHEADDescendantOfSketchBaseRef(ctx context.Context) error {
head, err := r.CurrentCommit(ctx)
if err != nil {
return err
}
// Note: Git's merge-base --is-ancestor checks strict ancestry (i.e., <), so a commit is NOT an ancestor of itself.
- cmd := exec.CommandContext(ctx, "git", "merge-base", "--is-ancestor", r.initialCommit, head)
+ cmd := exec.CommandContext(ctx, "git", "merge-base", "--is-ancestor", r.sketchBaseRef, head)
cmd.Dir = r.repoRoot
err = cmd.Run()
if err != nil {
@@ -890,7 +890,7 @@
}
buf := new(strings.Builder)
- fmt.Fprintf(buf, "Test regressions detected between initial commit (%s) and HEAD:\n\n", r.initialCommit)
+ fmt.Fprintf(buf, "Test regressions detected between initial commit (%s) and HEAD:\n\n", r.sketchBaseRef)
for i, reg := range regressions {
fmt.Fprintf(buf, "%d: %v: ", i+1, reg.Source())
diff --git a/claudetool/codereview/llm_review.go b/claudetool/codereview/llm_review.go
index 32a8e00..df8af9b 100644
--- a/claudetool/codereview/llm_review.go
+++ b/claudetool/codereview/llm_review.go
@@ -17,7 +17,7 @@
// doLLMReview analyzes the diff using an LLM subagent.
func (r *CodeReviewer) doLLMReview(ctx context.Context) (string, error) {
// Get the full diff between initial commit and HEAD
- cmd := exec.CommandContext(ctx, "git", "diff", r.initialCommit, "HEAD")
+ cmd := exec.CommandContext(ctx, "git", "diff", r.sketchBaseRef, "HEAD")
cmd.Dir = r.repoRoot
out, err := cmd.CombinedOutput()
if err != nil {