Skip to content

Commit 607ad5e

Browse files
committed
Grade against a stale baseline, or say why not
The Stop hook emitted only on a regression, so a baseline that could not be compared was indistinguishable from a clean change. - shouldAutoPin refreshes a baseline that is blocking-incomparable, not just one whose tree moved. Previously an upgrade left an unusable baseline in place on a clean tree, and the session was never graded. Auto-pinned baselines only; a deliberate pin is left alone. - The hook now emits on StatusIncomparable, naming the cause and the remedy. Verdict.DeclineReason reuses the table `enola check` prints from, so both surfaces give the same explanation. - Reported once per cause. hookstate records the decline identity; a successful grade clears it, so a recurrence is reported again. - doctor reports baseline usability from metadata, before a session rather than after. - Updated the installed instruction, which said the hook stays silent unless something regressed. Advisory warnings are unchanged: a stale baseline still grades.
1 parent d0472ca commit 607ad5e

12 files changed

Lines changed: 445 additions & 23 deletions

File tree

cmd/enola/doctor.go

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import (
88
"path/filepath"
99
"time"
1010

11+
"github.com/enola-labs/enola/internal/diff"
12+
"github.com/enola-labs/enola/internal/engine"
1113
"github.com/enola-labs/enola/internal/hookstate"
14+
"github.com/enola-labs/enola/pkg/bootstrap"
15+
"github.com/enola-labs/enola/pkg/check"
1216
)
1317

1418
// runDoctor is `enola doctor`: does the loop actually run in this repository?
@@ -61,12 +65,14 @@ func runDoctor(args []string) {
6165
outDir := hookOutputDir(repoDir)
6266
state := hookstate.Load(outDir)
6367
installed := hooksConfigured(repoDir)
68+
baselineIssue := baselineUsability(repoDir, outDir)
6469

6570
if *asJSON {
6671
out := map[string]any{
67-
"repo": repoDir,
68-
"hooks_configured": installed,
69-
"state": state,
72+
"repo": repoDir,
73+
"hooks_configured": installed,
74+
"state": state,
75+
"baseline_unusable": baselineIssue,
7076
}
7177
enc := json.NewEncoder(os.Stdout)
7278
enc.SetIndent("", " ")
@@ -75,6 +81,20 @@ func runDoctor(args []string) {
7581
}
7682

7783
fmt.Printf("enola doctor: %s\n\n", repoDir)
84+
85+
// Asked and answered BEFORE a session ends, which is the difference between this
86+
// and reading the last outcome below: an unusable baseline makes the hooks decline
87+
// to grade, and they decline quietly. Knowing now beats finding out afterwards.
88+
fmt.Println("Baseline")
89+
if baselineIssue == "" {
90+
fmt.Println(" comparable — the gate can grade against it")
91+
} else {
92+
fmt.Printf(" NOT COMPARABLE: %s\n", baselineIssue)
93+
fmt.Println(" Nothing will be graded against it until it is re-pinned:")
94+
fmt.Println(" enola baseline pin")
95+
}
96+
fmt.Println()
97+
7898
fmt.Println("Session hooks")
7999

80100
if !installed {
@@ -137,6 +157,27 @@ func runDoctor(args []string) {
137157
}
138158
}
139159

160+
// baselineUsability returns why the pinned baseline could not be graded against, or
161+
// "" when it can. It answers from metadata alone — no files are parsed — so `doctor`
162+
// stays a report rather than becoming a snapshot.
163+
func baselineUsability(repoDir, outDir string) string {
164+
base, err := bootstrap.LoadSnapshotDir(engine.ResolveBaselineDir(outDir, "pinned"))
165+
if err != nil {
166+
return "" // no baseline pinned; the hooks section covers that case
167+
}
168+
eng, cfg, err := bootstrap.NewEngine(bootstrap.Options{ConfigPath: configForRepo(repoDir)})
169+
if err != nil {
170+
return ""
171+
}
172+
cfg.Repo, cfg.Repos = repoDir, nil
173+
current := eng.CurrentMeta(repoDir)
174+
if current == nil {
175+
return ""
176+
}
177+
v := check.Evaluate(&diff.SnapshotDiff{Comparability: diff.CompareMeta(base.Meta, *current)}, check.Policy{})
178+
return v.DeclineReason()
179+
}
180+
140181
// hooksConfigured reports whether .claude/settings.json carries an entry enola owns.
141182
//
142183
// Deliberately a shallow scan for the marker rather than a shape check: whether the

cmd/enola/hook.go

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,44 @@ func runStopHook(ctx context.Context) {
7777
// paths are the ones worth recording: a hook that never fires and a hook that fires
7878
// and finds nothing are indistinguishable in a session, and only one of them is
7979
// broken. See internal/hookstate and DEFECTS_FOUND.md.
80-
hookstate.RecordFired(outDir, hookstate.EventStop, stopOutcome(verdict, ok))
80+
declineKey := ""
81+
if ok {
82+
declineKey = verdict.DeclineKey()
83+
}
84+
// ShouldReport is asked BEFORE recording, because recording is what makes the next
85+
// identical decline a repeat.
86+
sayDecline := ok && declineKey != "" && hookstate.ShouldReport(outDir, hookstate.EventStop, declineKey)
87+
hookstate.RecordFiredWithReason(outDir, hookstate.EventStop, stopOutcome(verdict, ok), declineKey)
88+
89+
var context string
90+
switch {
91+
case ok && verdict.Status == check.StatusRegression:
92+
context = "enola graded the architectural change made in this session and found a structural " +
93+
"regression. This was not necessarily intended — review it before considering the task " +
94+
"finished, and either fix it or say why it is deliberate.\n\n" + verdict.Render()
8195

82-
if !ok || verdict.Status != check.StatusRegression {
96+
case sayDecline:
97+
// The gate could not grade at all, and saying nothing would be indistinguishable
98+
// from grading it clean. `enola check` spends a whole exit code (3) keeping those
99+
// apart so "I refuse to grade this" is never read as "your change is bad"; a hook
100+
// that stays silent collapses the same distinction in the other direction, and
101+
// leaves someone believing the loop is protecting them when it is not.
102+
//
103+
// Said once per distinct reason, not once per session — see hookstate.ShouldReport.
104+
context = "enola could NOT grade the architectural change made in this session: " +
105+
verdict.DeclineReason() + ".\n\n" +
106+
"This is NOT a statement about your change — the comparison itself was untrustworthy, " +
107+
"so no verdict was reached in either direction. Re-pin the baseline to restore grading " +
108+
"(`enola baseline pin`, or the set_baseline tool), and `enola doctor` reports whether " +
109+
"the hooks are grading again."
110+
111+
default:
83112
return
84113
}
85114

86115
var out stopHookOutput
87116
out.HookSpecificOutput.HookEventName = "Stop"
88-
out.HookSpecificOutput.AdditionalContext =
89-
"enola graded the architectural change made in this session and found a structural " +
90-
"regression. This was not necessarily intended — review it before considering the task " +
91-
"finished, and either fix it or say why it is deliberate.\n\n" + verdict.Render()
117+
out.HookSpecificOutput.AdditionalContext = context
92118

93119
encoded, err := json.Marshal(out)
94120
if err != nil {
@@ -189,7 +215,7 @@ func pinBaselineSingleFlight(ctx context.Context, repoDir string) {
189215
defer lock.Release()
190216

191217
baselineDir := engine.ResolveBaselineDir(outDir, "pinned")
192-
if !shouldAutoPin(baselineDir, anchor, cfg.Output.Dir) {
218+
if !shouldAutoPin(baselineDir, anchor, cfg.Output.Dir, eng.CurrentMeta(anchor)) {
193219
return
194220
}
195221

@@ -225,14 +251,29 @@ func pinBaselineSingleFlight(ctx context.Context, repoDir string) {
225251
//
226252
// A dirty tree is never treated as current: "dirty" says the content is not identified by
227253
// the commit, so two dirty trees at the same commit may differ arbitrarily.
228-
func shouldAutoPin(baselineDir, repoDir, outputDir string) bool {
254+
//
255+
// The third rule is about usefulness rather than freshness: an auto-pinned baseline that
256+
// can no longer be COMPARED to a current snapshot — a different enola version, a changed
257+
// extractor set or ignore globs — is not a baseline at all, and refreshing it costs one
258+
// snapshot where leaving it costs the session's entire grading, silently. Tree movement
259+
// alone missed this: a session starting on a clean unchanged tree after an upgrade
260+
// graded against an unusable baseline and said nothing.
261+
//
262+
// Still only ever applied to baselines this hook created. A deliberate pin stays
263+
// untouched even when unusable — replacing it would discard the "before" of a refactor
264+
// that may span days, which is a worse outcome than a Stop hook that has to explain
265+
// itself. That case is reported instead.
266+
func shouldAutoPin(baselineDir, repoDir, outputDir string, current *facts.SnapshotMeta) bool {
229267
base, err := bootstrap.LoadSnapshotDir(baselineDir)
230268
if err != nil {
231269
return true // no baseline yet — this is exactly what the hook is for
232270
}
233271
if _, err := os.Stat(filepath.Join(baselineDir, autoPinMarker)); err != nil {
234272
return false // deliberately pinned; not ours to replace
235273
}
274+
if current != nil && baselineIsUnusable(base.Meta, *current) {
275+
return true
276+
}
236277
now := engine.GitState(repoDir, outputDir)
237278
if now == nil || base.Meta.Git == nil {
238279
return true // cannot prove it is current, so refresh
@@ -262,6 +303,16 @@ func stopOutcome(v check.Verdict, ok bool) hookstate.Outcome {
262303
}
263304
}
264305

306+
// baselineIsUnusable reports whether a BLOCKING comparability warning stands between
307+
// these two snapshots — the same classification `enola check` uses to decline, so the
308+
// hook refreshes exactly what the gate would have refused to grade against. Advisory
309+
// warnings (a stale baseline) are deliberately not included: those still grade, and
310+
// re-pinning on staleness would destroy the multi-day baseline the staleness warning
311+
// exists to permit.
312+
func baselineIsUnusable(base, current facts.SnapshotMeta) bool {
313+
return len(check.BlockingKinds(diff.CompareMeta(base, current))) > 0
314+
}
315+
265316
// gradeQuietly runs the gate, returning ok=false for every reason a hook should stay
266317
// silent rather than report a problem.
267318
//

cmd/enola/hook_sessionstart_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ func TestShouldAutoPin_NeverReplacesADeliberatePin(t *testing.T) {
4545
dir := filepath.Join(t.TempDir(), "baseline")
4646
writeBaseline(t, dir, &facts.GitInfo{Commit: "deadbeef"}, false) // no auto marker
4747

48-
if shouldAutoPin(dir, t.TempDir(), ".enola") {
48+
if shouldAutoPin(dir, t.TempDir(), ".enola", nil) {
4949
t.Error("a deliberately pinned baseline was scheduled for replacement")
5050
}
5151
}
5252

5353
// TestShouldAutoPin_WithNoBaselineYet — the case the hook exists for.
5454
func TestShouldAutoPin_WithNoBaselineYet(t *testing.T) {
55-
if !shouldAutoPin(filepath.Join(t.TempDir(), "baseline"), t.TempDir(), ".enola") {
55+
if !shouldAutoPin(filepath.Join(t.TempDir(), "baseline"), t.TempDir(), ".enola", nil) {
5656
t.Error("no baseline exists, so one should be pinned")
5757
}
5858
}
@@ -67,7 +67,7 @@ func TestShouldAutoPin_SkipsWhenTreeHasNotMoved(t *testing.T) {
6767

6868
// GitState cannot read a non-git directory, so the decision must fail toward
6969
// refreshing rather than toward silently keeping a baseline that may be wrong.
70-
if !shouldAutoPin(dir, repo, ".enola") {
70+
if !shouldAutoPin(dir, repo, ".enola", nil) {
7171
t.Error("with git state unavailable, the baseline must be refreshed rather than trusted")
7272
}
7373
}
@@ -79,7 +79,7 @@ func TestShouldAutoPin_DirtyTreeIsNeverCurrent(t *testing.T) {
7979
dir := filepath.Join(t.TempDir(), "baseline")
8080
writeBaseline(t, dir, &facts.GitInfo{Commit: "deadbeef", Dirty: true}, true)
8181

82-
if !shouldAutoPin(dir, t.TempDir(), ".enola") {
82+
if !shouldAutoPin(dir, t.TempDir(), ".enola", nil) {
8383
t.Error("a dirty baseline must never be treated as current")
8484
}
8585
}
@@ -96,7 +96,7 @@ func TestSessionStartHook_IsSilentOnEveryFailurePath(t *testing.T) {
9696
dir := filepath.Join(t.TempDir(), "baseline")
9797
// shouldAutoPin is the only part reachable without spawning; exercising it
9898
// with hostile inputs must not panic.
99-
_ = shouldAutoPin(dir, cwd, ".enola")
99+
_ = shouldAutoPin(dir, cwd, ".enola", nil)
100100
})
101101
}
102102
}

0 commit comments

Comments
 (0)