fix(trail): let tune rewrites drop cosmetic placeholders · Entire

fix(trail): let tune rewrites drop cosmetic placeholders

1be54e9→main·

Soph·3w ago·3 files·+40 added/-20 removed

`tune --run` was skipping most runners with "dropped placeholder(s): {{branch}}": the model legitimately stops naming {{branch}} in prose (the diff is taken against HEAD, and the command only needs {{base_branch}}), and the strict "preserve every placeholder" check rejected the otherwise-good rewrite.

Reframe the check around what's actually unsafe: an ADDED placeholder renders as literal {{junk}} (the backend only substitutes the known set) so it's still a hard reject; a DROPPED placeholder just leaves a substitution slot unused, which is safe — now allowed and surfaced as a "note: no longer references …" line so the user still sees it in the git diff.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

24fbe70ac57aView transcript

Changes

3

28 unmodified lines

var placeholderRe = regexp.MustCompile(`{{[^{}]+}}`)

// validateNewTemplate rejects a rewritten template that is empty or whose set
// of {{placeholder}} tokens differs from the original. The backend substitutes
// those placeholders at run time, so a dropped one silently breaks the runner
// and an invented one leaves unresolved template text in later runs — and the
// model is only *asked* to preserve them exactly, not forced to.
func validateNewTemplate(oldTemplate, newTemplate string) error {
    if strings.TrimSpace(newTemplate) == "" {
        return errors.New("rewritten template is empty")
    }
    oldSet := placeholderSet(oldTemplate)
    newSet := placeholderSet(newTemplate)

var missing, added []string
    for ph := range oldSet {
        if !newSet[ph] {
            missing = append(missing, ph)
        }
    }
    for ph := range newSet {
        if !oldSet[ph] {
            added = append(added, ph)
        }
    }
    sort.Strings(missing)
    sort.Strings(added)

if len(missing) > 0 {
        return fmt.Errorf("rewritten template dropped placeholder(s): %s", strings.Join(missing, ", "))
    }
    if len(added) > 0 {
        return fmt.Errorf("rewritten template added unknown placeholder(s): %s", strings.Join(added, ", "))
    }
    return nil
}

// droppedPlaceholders returns the placeholders present in oldTemplate but not in
// newTemplate, sorted. Used to inform the user when a rewrite stops using one.
func droppedPlaceholders(oldTemplate, newTemplate string) []string {
    newSet := placeholderSet(newTemplate)
    var dropped []string
    for ph := range placeholderSet(oldTemplate) {
        if !newSet[ph] {
            dropped = append(dropped, ph)
        }
    }
    sort.Strings(dropped)
    return dropped
}

func placeholderSet(s string) map[string]bool {
    set := make(map[string]bool)
    for _, ph := range placeholderRe.FindAllString(s, -1) {

Mcmd/entire/cli/trail_tune_apply.go+23/-19

107 unmodified lines

func TestDroppedPlaceholders(t *testing.T) {
    t.Parallel()

const old = "Analyze {{branch}} vs {{base_branch}}. Use {{previous_findings}}."
    got := droppedPlaceholders(old, "Analyze HEAD vs {{base_branch}}. Use {{previous_findings}}.")
    if len(got) != 1 || got[0] != "{{branch}}" {
        t.Errorf("dropped = %v, want [{{branch}}]", got)
    }
    if d := droppedPlaceholders(old, old); len(d) != 0 {
        t.Errorf("expected no drops for identical template, got %v", d)
    }
}

func TestParseTuneOutput(t *testing.T) {
    t.Parallel()

Mcmd/entire/cli/trail_tune_test.go+14/-1

162 unmodified lines

Mcmd/entire/cli/trail\_tune\_cmd.go+3