Goal: Implement GitHub issue #361 (Lessons digest is write-only: add content ded

docs/PRD.md

@@ -1071,6 +1071,28 @@ Webhook-triggered runs with HMAC verification and dedup, run dashboard/status co
 > head-gone, reopen-refused) and
 > `TestVerifyAndMergeRescueRefusesNonReopenablePR`
 > (`internal/loopdriver/run.go`, `issues.go`).
+>
+> 2026-09-17 (issue #361: the lessons ratchet dedupes on content, so a repeat
+> is not a write): the propose→evaluate→accept guard, the Q39 impact telemetry
+> and the held-out must-beat tier all shipped in the Go port
+> (`internal/lessons`, FR-GO-07 #194), but the loop's own write path did not
+> use them: the ratchet merge of the state branch collapsed only *exact*
+> duplicate lines (`stateSync.mergeLessons`), so a lesson re-landed with
+> reworded punctuation — `Lessons eval guard` vs `Lessons-eval-guard`, the
+> live `.selfbuild/lessons.md` regression — survived as a second copy and
+> spent the 4000-char `lessonsMaxChars` budget the digest is bounded by.
+> `lessons.DedupeLessonContent` now applies the guard's own similarity policy
+> (normalized word-trigram Jaccard at the 0.8
+> `DefaultLessonsDedupeSimilarity`, first occurrence wins) to the merged body,
+> so appending an existing lesson — verbatim or reworded — is a no-op;
+> structural lines (blank, `---`, headings) always survive so the ratchet
+> keeps its dated sections, and the merge is idempotent. Measured on the live
+> file: 123,009 → 122,448 bytes, and the digest's newest-40-line window
+> 15,320 → 14,759 bytes. Pinned by `TestDedupeLessonContent` and
+> `TestMergeLessonLinesNearDuplicateAppendIsNoop`. Named ceiling, unchanged
+> from the guard: the 0.4–0.7 near-dup band still lands on purpose, so a v2
+> rewrite of a shipped lesson can be re-measured
+> (`internal/lessons/guard.go`, `internal/loopdriver/state.go`).
 
 ~~- **Cross-board retry memory beyond the SHA guard** — commit 60638d3 stops re-issuing shipped goals, but re-queued failures still get a fresh attempt budget; carry the prior board's failure class onto the re-bridged goal so the scout deprioritizes until the root-cause fix lands (Q27).~~
 ~~- **Regression oracle before board merge** — gates judge single PRs and PR #108's committed STRIDE allowlist widens suppression paths; add a board-level "is the system at least as good?" check (full suite on the merged result) ahead of `autoMerge`, per the Kitchen Loop zero-regression rule.~~
@@ -1109,10 +1131,11 @@ Direction addendum 2026-09-03 (section 20): DevAgent becomes the local-first, BY
 > `herdr-sweep --orphan-brokers` remains Node-only per the HERDR.md status
 > note (#360); FR-CTX-01..05 remain unimplemented (#368, see §20.1); and
 > docs/PRODUCTION-READINESS.md still describes the retired Node tree, so no
-> Go-era readiness verdict exists (#362). Lower-priority convergence work,
-> also issue-first: lessons-digest dedupe + eval guard (#361), Windows
-> parity (#364), parity-shim convergence + vestigial exit-3 machinery
-> (#365), ClaudeCode adapter hardening (#366), internal/cli +
+> Go-era readiness verdict exists (#362); the lessons-digest dedupe + eval
+> guard gap (#361) closed 2026-09-17 on the ratchet write path (Phase 4
+> history entry above). Lower-priority convergence work, also issue-first:
+> Windows parity (#364), parity-shim convergence + vestigial exit-3
+> machinery (#365), ClaudeCode adapter hardening (#366), internal/cli +
 > cmd/devagent test coverage (#367), War Room as a first-class Go command
 > (#369).
 - **Easy handoff / control plane (FR-HAND, #145)** — Shipped 2026-09-08: cold path is exactly `devagent init` → `devagent tui` (optional `start` alias); TUI dispatch sheet (`n`, FR-HAND-02) + approve sheet (`g`, FR-HAND-07); `POST /dispatch` threads `autoPr` into the spawned `task --auto-pr` argv gated on `GITHUB_TOKEN` (FR-HAND-03); init adds worker-detect chips (FR-HAND-04), herdr default-on advisory (FR-HAND-05), Orca repo registration without worktree provisioning (FR-HAND-06). Look/feel bar: #146.
@@ -1756,6 +1779,7 @@ existing driver with validation surfaces (test, command, telemetry, chaos
 schedule) so "the driver works perfectly" is a checkable claim, not a hope.
 
 ---
+*Last updated: 2026-09-17 (issue #361: the lessons ratchet dedupes on content, not just on exact lines) — the selfbuild lessons file is no longer write-only on the loop's own path: `stateSync.mergeLessons` collapsed only exact duplicate lines, so a lesson re-landed with reworded punctuation (`Lessons eval guard` vs `Lessons-eval-guard` — the live `.selfbuild/lessons.md` regression) landed as a second copy and spent the 4000-char `lessonsMaxChars` digest budget. The merge now runs `lessons.DedupeLessonContent` — the guard's own normalized word-trigram Jaccard policy at the 0.8 `DefaultLessonsDedupeSimilarity`, first occurrence wins, structural lines always kept, idempotent — so appending an existing lesson, verbatim or reworded, is a no-op (measured on the live file: 122,448 bytes vs 123,009; the newest-40-line digest window 14,759 vs 15,320). The guard, the Q39 accept/reject × loop-result telemetry and the held-out must-beat tier were already shipped in the Go port; this closes the convergence gap on the loop's write path. Pinned by `TestDedupeLessonContent` and `TestMergeLessonLinesNearDuplicateAppendIsNoop` (`internal/lessons/guard.go`, `internal/loopdriver/state.go`).
 *Last updated: 2026-09-14 (the PRD became an input, and `up` became a health receipt: FR-SIMPLE-07/08, #370 + #371) — `devagent up` now covers the whole setup-to-running path (prerequisite gate through the `init` checks → state/queue dirs → `docs/PRD.md` intake → a `lane` census of pending rows / open PRD checkboxes / open `selfbuild` issues → daemon → detached driver launched from its own executable with `SELFBUILD_DEVAGENT_BIN` pinned to match) and **proves** the start rather than reporting a pid: `up` is the driver's parent, so a driver that halts at its own gate exits 0 and stays a zombie whose pid answers any liveness probe (measured 2026-09-14: `max iterations reached` on stderr while `up` printed a healthy pid for fifteen more seconds), so it now reaps its own child and waits (bounded by `--wait`, default 15s) until the loop lock AND a phase-naming heartbeat exist — `driver running (pid N) — holds the loop lock, iteration 2, phase queue-empty` — or fails the run with the halt line read out of `.selfbuild/logs/`. Work intake flipped the other way too: an open `- [ ]` checkbox in `docs/PRD.md` is now an instruction (`internal/prdintake` queues it with its heading as context, its sub-bullets as criteria and its section body as the task-PRD sidecar; the driver ingests at iteration head after the PRD-currency gate, `SELFBUILD_PRD_INTAKE=0` opts out) and the shipped PR ticks the checkbox it built, so a built item cannot re-enter the lane. Queue-first already outranked the tracker, so operator intent now outranks LLM self-selection — the value half of #355, whose other half (an empty lane logged and breadcrumb'd as `queue-empty` with the refill instructions instead of reading as progress) landed in the same change. `devagent create --scout/--tracker` also stopped baking the deleted `dist/src/cli.js` into LaunchAgent argv, and the scout cycle itself is live in Go at last (#372).
 *Last updated: 2026-09-13 (caveat on #349's revision stamp: linked-worktree builds mis-stamp — [#352](https://github.com/FreePeak/devagent/issues/352)) — while verifying #349's landing we found Go's `-buildvcs` resolves `vcs.revision` from the shared common gitdir in a linked worktree, so a `devagent` binary built inside a per-task worktree stamps the primary checkout's `refs/heads/main` tip rather than its own HEAD (`vcs.modified=true`). Proven A/B at the same commit `8ca0688`: a plain clone stamps `8ca0688`, a linked worktree stamps `50b03e8`. Scope: this is a latent defect confined to *worktree-built* binaries — the shipped driver is built by `scripts/self-update.sh` from the primary non-worktree checkout (`go build -trimpath ./cmd/devagent` after `git pull --ff-only`), so it stamps correctly and `RunLoop`'s stale-binary guard behaves as intended in the live loop; only a manually-built worktree binary writes a wrong `release-created` revision and false-trips the (advisory-only) WARN. #349's "a stale binary is loud" holds for the normal-checkout build path; the worktree edge is tracked in #352.
 *Last updated: 2026-09-13 (release-created rows carry `tag`/`sha` again: PR #349's Revision stamp landed as a silent data-loss regression) — #349 added a `Revision` field to the Go `ReleaseRecord` but dropped `Tag`/`SHA` from the `record release` literal while still printing `(tag @ sha)` to stdout, so every Go-written release row persisted `"tag":""` `"sha":""` (the Q24 fields the record exists to carry) and no test covered the write site, so it shipped green. Restored both fields; `internal/cli/actions_release_test.go` now pins the CLI row shape (proven red on the regressed literal, green after); regenerated `docs/PRD.html` so the styled mirror matches `PRD.md`.

internal/lessons/guard.go

@@ -713,8 +713,14 @@ func LessonShingles(text string) map[string]bool {
 // so an empty candidate can never bypass the guard by containing nothing to
 // compare.
 func LessonSimilarity(a string, b string) float64 {
-	sa := LessonShingles(a)
-	sb := LessonShingles(b)
+	return shingleSimilarity(LessonShingles(a), LessonShingles(b))
+}
+
+// shingleSimilarity is the trigram-Jaccard score of two already-built
+// shingle sets — the body of LessonSimilarity, split out so a caller
+// comparing one line against many (the ratchet dedupe below) reuses the kept
+// lines' shingles instead of rebuilding them for every pair.
+func shingleSimilarity(sa, sb map[string]bool) float64 {
 	if len(sa) == 0 && len(sb) == 0 {
 		return 1
 	}
@@ -731,6 +737,70 @@ func LessonSimilarity(a string, b string) float64 {
 	return float64(inter) / float64(union)
 }
 
+// isLessonContentLine reports whether a raw lessons-file line is lesson
+// content — the granularity the guard compares — rather than structure:
+// blank lines, `---` fences, and markdown headings carry no lesson, so they
+// are never dedupe candidates and always survive verbatim. The single filter
+// rule behind ReadLessonEntries and DedupeLessonContent.
+func isLessonContentLine(line string) bool {
+	trimmed := strings.TrimSpace(line)
+	if trimmed == "" || trimmed == "---" {
+		return false
+	}
+	return !lessonHeaderRe.MatchString(trimmed)
+}
+
+// DedupeLessonContent collapses content-similarity repeats in a lessons file
+// body (issue #361). The lessons ratchet merge is the self-build loop's write
+// path for machine appends; exact-line dedupe alone let near-duplicate
+// rewordings of a lesson the digest already carries accumulate until they
+// spent the LessonsMaxChars budget on repeats instead of on new lessons.
+//
+// The first occurrence of each content line wins (the ratchet's
+// `awk '!seen[$0]++'` convention: remote content merges ahead of local
+// appends); a later line whose normalized word-trigram similarity to an
+// already-kept content line is at or above threshold is dropped as a repeat.
+// Structural lines (see isLessonContentLine) always survive, so the merged
+// file keeps its dated section layout. threshold <= 0 means
+// DefaultLessonsDedupeSimilarity.
+//
+// Pure function. Line joining and the trailing newline mirror the line merge
+// it guards: "" for an empty result, otherwise a trailing "\n".
+func DedupeLessonContent(text string, threshold float64) string {
+	if threshold <= 0 {
+		threshold = DefaultLessonsDedupeSimilarity
+	}
+	lines := strings.Split(text, "\n")
+	if len(lines) > 0 && lines[len(lines)-1] == "" {
+		lines = lines[:len(lines)-1]
+	}
+	kept := make([]map[string]bool, 0, len(lines))
+	out := make([]string, 0, len(lines))
+	for _, line := range lines {
+		if !isLessonContentLine(line) {
+			out = append(out, line)
+			continue
+		}
+		shingles := LessonShingles(line)
+		repeat := false
+		for _, other := range kept {
+			if shingleSimilarity(shingles, other) >= threshold {
+				repeat = true
+				break
+			}
+		}
+		if repeat {
+			continue
+		}
+		kept = append(kept, shingles)
+		out = append(out, line)
+	}
+	if len(out) == 0 {
+		return ""
+	}
+	return strings.Join(out, "\n") + "\n"
+}
+
 // LessonsDedupeResult is the result of a lessons dedupe check (and, with
 // the gated-append fields set, of AppendLessonGuarded).
 type LessonsDedupeResult struct {
@@ -773,14 +843,7 @@ func ReadLessonEntries(lessonsPath string) []string {
 	}
 	out := []string{}
 	for _, line := range strings.Split(string(raw), "\n") {
-		trimmed := strings.TrimSpace(line)
-		if trimmed == "" {
-			continue
-		}
-		if trimmed == "---" {
-			continue
-		}
-		if lessonHeaderRe.MatchString(trimmed) {
+		if !isLessonContentLine(line) {
 			continue
 		}
 		out = append(out, strings.TrimRight(line, " \t\r\n"))

internal/lessons/guard_test.go

@@ -294,6 +294,66 @@ func TestCheckLessonsDedupe(t *testing.T) {
 	})
 }
 
+// ---------------------------------------------------------------------------
+// dedupeLessonContent (the ratchet merge's content-similarity dedupe, #361)
+// ---------------------------------------------------------------------------
+
+func TestDedupeLessonContent(t *testing.T) {
+	lesson := "- **Lessons eval guard is the single best next backlog item**: the digest is write-only, so a repeat spends the budget."
+	hyphenDup := strings.Replace(lesson, "Lessons eval guard", "Lessons-eval-guard", 1)
+	distinct := "- Fencing tokens kill double dispatch even after kill -9 and lease reclaim."
+
+	t.Run("appending an existing lesson is a no-op", func(t *testing.T) {
+		file := "## 2026-09-02\n\n" + lesson + "\n"
+		if got := DedupeLessonContent(file+lesson+"\n", 0); got != file {
+			t.Fatalf("exact repeat changed the file:\ngot  %q\nwant %q", got, file)
+		}
+	})
+
+	t.Run("a near-duplicate reword is dropped, the first occurrence wins", func(t *testing.T) {
+		file := "## 2026-09-02\n\n" + lesson + "\n"
+		if got := DedupeLessonContent(file+hyphenDup+"\n", 0); got != file {
+			t.Fatalf("near-duplicate changed the file:\ngot  %q\nwant %q", got, file)
+		}
+	})
+
+	t.Run("keeps a distinct lesson and every structural line", func(t *testing.T) {
+		file := "## 2026-09-02\n\n" + lesson + "\n---\n" + distinct + "\n"
+		if got := DedupeLessonContent(file, 0); got != file {
+			t.Fatalf("distinct content changed:\ngot  %q\nwant %q", got, file)
+		}
+	})
+
+	t.Run("honors the threshold override", func(t *testing.T) {
+		// The near-dup-band pair from TestCheckLessonsDedupe: admitted at the
+		// 0.8 default (a v2 rewrite is allowed to land so its effect can be
+		// re-measured), collapsed at 0.5.
+		first := "one two three four five six seven eight"
+		second := "one two three four five six seven nine"
+		file := first + "\n" + second + "\n"
+		if got := DedupeLessonContent(file, 0); got != file {
+			t.Fatalf("default threshold dropped a near-dup-band lesson: %q", got)
+		}
+		if got, want := DedupeLessonContent(file, 0.5), first+"\n"; got != want {
+			t.Fatalf("threshold 0.5 should collapse the near-dup band:\ngot  %q\nwant %q", got, want)
+		}
+	})
+
+	t.Run("is idempotent: the merged file is a fixed point of the next merge", func(t *testing.T) {
+		file := "## 2026-09-02\n\n" + lesson + "\n" + hyphenDup + "\n" + distinct + "\n"
+		once := DedupeLessonContent(file, 0)
+		if twice := DedupeLessonContent(once, 0); twice != once {
+			t.Fatalf("second merge changed the file:\ngot  %q\nwant %q", twice, once)
+		}
+	})
+
+	t.Run("empty input stays empty", func(t *testing.T) {
+		if got := DedupeLessonContent("", 0); got != "" {
+			t.Fatalf("got %q, want empty", got)
+		}
+	})
+}
+
 // ---------------------------------------------------------------------------
 // appendLessonGuarded (eval-gated append: impact → dedupe → evaluate)
 // ---------------------------------------------------------------------------

internal/loopdriver/state.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/FreePeak/devagent/internal/git"
+	"github.com/FreePeak/devagent/internal/lessons"
 )
 
 const (
@@ -175,16 +176,29 @@ func extractField(re *regexp.Regexp, line, marker string) string {
 	return strings.TrimPrefix(line[loc[0]:loc[1]], marker)
 }
 
+// mergeLessons is the lessons ratchet's write path: the union of the remote
+// state-branch file and the local one.
 func (s *stateSync) mergeLessons() {
 	remote := s.fileAtState(".selfbuild/lessons.md")
 	local, _ := os.ReadFile(s.lessonsPath())
 	_ = os.MkdirAll(filepath.Dir(s.lessonsPath()), 0o755)
 	// Bash touches the lessons file first, so an empty merge still leaves an
 	// (empty) file behind — mirror that.
-	merged := dedupeLines(remote + string(local))
+	merged := mergeLessonLines(remote, string(local))
 	_ = os.WriteFile(s.lessonsPath(), []byte(merged), 0o644)
 }
 
+// mergeLessonLines is the pure body of the lessons merge: the remote file
+// first, then the local one, with repeats collapsed — exact duplicates (the
+// `awk '!seen[$0]++'` ratchet port) and, since issue #361, near-duplicates on
+// content similarity, so an append that only rewords a lesson the digest
+// already carries is a no-op instead of a repeat that spends the
+// LessonsMaxChars budget. First occurrence wins; structural lines always
+// survive (DedupeLessonContent).
+func mergeLessonLines(remote, local string) string {
+	return lessons.DedupeLessonContent(dedupeLines(remote+local), lessons.DefaultLessonsDedupeSimilarity)
+}
+
 // dedupeLines ports `awk '!seen[$0]++'`: keep the first occurrence of every
 // line. Only the final empty element produced by a trailing newline is
 // dropped; interior blank lines are ordinary lines to awk and stay.

internal/loopdriver/state_test.go

@@ -58,6 +58,36 @@ func TestDedupeLinesKeepsFirstOccurrence(t *testing.T) {
 	}
 }
 
+// Issue #361: the ratchet merge is the loop's write path for machine appends,
+// so re-appending a lesson the file already carries — verbatim or reworded —
+// must leave the merged file byte-identical instead of spending the digest
+// budget on a repeat.
+func TestMergeLessonLinesNearDuplicateAppendIsNoop(t *testing.T) {
+	lesson := "- Lessons eval guard is the single best next backlog item: the digest is write-only, so repeats spend the budget."
+	// The live .selfbuild/lessons.md shape: the same lesson appended again as
+	// a hyphen-only reword.
+	reword := strings.Replace(lesson, "Lessons eval guard", "Lessons-eval-guard", 1)
+	distinct := "- Fencing tokens kill double dispatch even after kill -9 and lease reclaim."
+	remote := "## 2026-09-02\n\n" + lesson + "\n"
+
+	t.Run("appending an existing lesson is a no-op", func(t *testing.T) {
+		if got := mergeLessonLines(remote, remote); got != remote {
+			t.Fatalf("merge changed the file:\ngot  %q\nwant %q", got, remote)
+		}
+		if got := mergeLessonLines(remote, lesson+"\n"); got != remote {
+			t.Fatalf("re-appending the lesson changed the file:\ngot  %q\nwant %q", got, remote)
+		}
+	})
+
+	t.Run("a reworded repeat is merged away, a distinct lesson lands", func(t *testing.T) {
+		got := mergeLessonLines(remote, reword+"\n"+distinct+"\n")
+		want := remote + distinct + "\n"
+		if got != want {
+			t.Fatalf("merge mismatch:\ngot  %q\nwant %q", got, want)
+		}
+	})
+}
+
 func TestStatePullFreshRemote(t *testing.T) {
 	repo := initFixtureRepo(t)
 	addBareOrigin(t, repo)