voyage/lib/review/recurrence.mjs

// lib/review/recurrence.mjs
// P5 — the project's memory, fed by review (ordre 5).
//
// A finding class (the same catalogue `rule_key`) that shows up in TWO review.md
// files in the target repo, as two DIFFERENT findings (distinct finding IDs),
// becomes ONE proposal; one sighting is no proposal, and one unfixed finding
// seen again in the next iteration is a persisting defect, not a recurrence.
// Catch-all keys (route null) and findings whose key was normalised from an
// unknown one (`original_rule_key`) are never counted.
// The history is the review.md files the repo already has — the current
// `.claude/projects/*/review.md` plus, where git tracks them, their earlier
// committed versions. No new store is created.
//
// Routing (the catalogue's `memory_route`): mechanical → the lesson names a
// deterministic check; judgement → a line in the target repo's `.claude/rules/`
// file; CLAUDE.md gets only a pointer. Review-hygiene classes (route null) are
// never proposed. Format of every line: mistake · fix · how to check.
//
// Writes: only between Voyage's own markers, under a size budget; past the
// budget the write is refused and the lesson belongs in a skill (operator's
// call). Every line is approved by the operator first (/trekreview Phase 8.7,
// AskUserQuestion) — this module never decides that on its own.
//
// Borrowed (credited, MIT where it applies):
//   - routing check / standard / pointer: Pocock `skills/in-progress/retro/SKILL.md:19,41`
//   - the three-part line "mistake · fix · how to check": ai-native-sdlc `assets/CLAUDE.md:27-29`
//   - the managed block between START/END markers: Spec Kit `extensions/agent-context/README.md:5`
//   - evidence (two finding IDs) per proposal, schema-check before writing:
//     Kiro https://kiro.dev/blog/trust-agent-triage/ (docs only, closed product)
//   - review → memory, and the size budget ("or make a skill"): Boris Cherny's
//     public talks/posts (sources A, B; see docs/praksisanalyse)
//
// CLI:
//   node lib/review/recurrence.mjs propose --repo <dir>
//     → {"proposals":[…], "reviews_read": N, "skipped": [...]} on stdout, exit 0; exit 2 = could not run
//   node lib/review/recurrence.mjs apply --repo <dir> --rule-key <KEY> [--line "<edited line>"]
//     → {"written":true|false, "reason":…} on stdout; exit 0 written, 1 refused, 2 could not run

import {
  readFileSync, writeFileSync, renameSync, existsSync, mkdirSync, readdirSync, statSync, realpathSync, lstatSync,
} from 'node:fs';
import { join, relative, dirname } from 'node:path';
import { execFileSync } from 'node:child_process';
import { validateReviewContent } from '../validators/review-validator.mjs';
import { extractFindingsBlock } from './findings-schema.mjs';
import { getRule } from './rule-catalogue.mjs';
import { isMain } from '../util/is-main.mjs';

export const LESSONS_DIR = join('.claude', 'rules');
export const LESSONS_FILE_NAME = 'voyage-review-lessons.md';
export const MARK_START = '<!-- voyage:review-lessons:start -->';
export const MARK_END = '<!-- voyage:review-lessons:end -->';
export const POINTER_START = '<!-- voyage:review-lessons-pointer:start -->';
export const POINTER_END = '<!-- voyage:review-lessons-pointer:end -->';
export const POINTER_LINE =
  `Recurring review lessons (operator-approved, managed by Voyage /trekreview): \`${LESSONS_DIR}/${LESSONS_FILE_NAME}\`.`;

/** Size budget for the managed block. Past it, a lesson belongs in a skill. */
export const DEFAULT_BUDGET_ENTRIES = 20;
export const DEFAULT_BUDGET_CHARS = 6000;
export const LINE_MAX = 700;
export const RECURRENCE_THRESHOLD = 2;

const BLOCK_HEADER = 'Each line: a finding class /trekreview saw in two reviews, approved by the operator. Format: mistake · fix · how to check.';

/** How to check a mechanical class. Judgement classes get the review itself. */
const CHECK_HINTS = Object.freeze({
  MISSING_TEST: 'a coverage check on the changed files with the repo\'s own test tool — red when a changed source file has no test that exercises it.',
  PLACEHOLDER_IN_CODE: '`git grep -nE "TODO|FIXME|XXX|debugger" -- <changed files>` returns nothing.',
  UNDECLARED_DEPENDENCY: 'every import in the changed files resolves to a declared dependency (the repo\'s own dependency checker).',
  BROKEN_SUCCESS_CRITERION: 'run each Success Criterion\'s verification command before the step is called done; only exit 0 counts.',
});

// ---- reading the history ----------------------------------------------------

/**
 * Pull the kept findings out of one review.md text.
 * @param {string} text
 * @returns {{ ok: boolean, identity?: string, findings?: object[], reason?: string }}
 */
export function parseReview(text) {
  const r = validateReviewContent(text, { strict: false });
  if (!r.valid) return { ok: false, reason: (r.errors ?? []).map((e) => e.code).join(',') || 'invalid' };
  const fm = r.parsed.frontmatter ?? {};
  const block = extractFindingsBlock(r.parsed.body ?? '');
  if (block === null) return { ok: false, reason: 'no-json-block' };
  let payload;
  try { payload = JSON.parse(block); } catch { return { ok: false, reason: 'json-parse-error' }; }
  const findings = (Array.isArray(payload?.findings) ? payload.findings : [])
    .map((f) => ({ ...f, rule_key: f?.rule_key ?? f?.rule }))
    .filter((f) => typeof f.rule_key === 'string');
  const identity = `${fm.project_dir ?? fm.slug ?? '?'}@${fm.scope_sha_end ?? '?'}`;
  return { ok: true, identity, findings };
}

function git(repo, args) {
  return execFileSync('git', ['-C', repo, ...args], { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] });
}

/**
 * Every review.md the repo already holds: current files under
 * `.claude/projects/<dir>/review.md`, plus earlier committed versions of those
 * files when git tracks them. Deduplicated by review identity
 * (project_dir @ scope_sha_end).
 * @param {string} repo
 * @returns {{ reviews: Array<{ identity: string, source: string, findings: object[] }>, skipped: Array<{ source: string, reason: string }> }}
 */
export function collectReviews(repo) {
  const sources = [];
  const projects = join(repo, '.claude', 'projects');
  if (existsSync(projects)) {
    for (const name of readdirSync(projects).sort()) {
      const p = join(projects, name, 'review.md');
      if (existsSync(p) && statSync(p).isFile()) {
        sources.push({ source: relative(repo, p), text: readFileSync(p, 'utf-8') });
      }
    }
  }
  // Earlier versions (review.md is overwritten per iteration; git keeps the rest).
  for (const { source } of [...sources]) {
    let shas = [];
    try { shas = git(repo, ['log', '--format=%H', '--', source]).split('\n').filter(Boolean); } catch { shas = []; }
    for (const sha of shas) {
      try {
        // `./` makes the path relative to --repo, not to the enclosing git root.
        sources.push({ source: `${source}@${sha.slice(0, 12)}`, text: git(repo, ['show', `${sha}:./${source}`]) });
      } catch { /* not in that commit's tree */ }
    }
  }
  const seen = new Set();
  const reviews = [];
  const skipped = [];
  for (const s of sources) {
    const parsed = parseReview(s.text);
    if (!parsed.ok) { skipped.push({ source: s.source, reason: parsed.reason }); continue; }
    if (seen.has(parsed.identity)) continue;
    seen.add(parsed.identity);
    reviews.push({ identity: parsed.identity, source: s.source, findings: parsed.findings });
  }
  return { reviews, skipped };
}

// ---- the managed block ------------------------------------------------------

function lessonsPath(repo) {
  return join(repo, LESSONS_DIR, LESSONS_FILE_NAME);
}

function count(text, needle) {
  return text.split(needle).length - 1;
}

/** The managed block, null when absent, or { damaged: true } when the markers are not exactly one START before one END. */
function readBlock(text) {
  const starts = count(text, MARK_START);
  const ends = count(text, MARK_END);
  if (starts === 0 && ends === 0) return null;
  const start = text.indexOf(MARK_START);
  const end = text.indexOf(MARK_END);
  if (starts !== 1 || ends !== 1 || end < start) return { damaged: true };
  return { start, end, inner: text.slice(start + MARK_START.length, end) };
}

function blockEntries(inner) {
  return inner.split('\n').filter((l) => l.startsWith('- ['));
}

/** The rule_keys already written as lessons. */
export function writtenClasses(repo) {
  const p = lessonsPath(repo);
  if (!existsSync(p)) return new Set();
  const block = readBlock(readFileSync(p, 'utf-8'));
  if (!block || block.damaged) return new Set();
  const keys = new Set();
  for (const line of blockEntries(block.inner)) {
    const m = line.match(/^- \[([A-Z_]+)\]/);
    if (m) keys.add(m[1]);
  }
  return keys;
}

// ---- proposals --------------------------------------------------------------

function clip(s, n) {
  const t = String(s ?? '').replace(/<!--|-->/g, '').replace(/[\u0000-\u001f\u007f\u2028\u2029\s]+/g, ' ').trim();
  return t.length > n ? `${t.slice(0, n - 1)}…` : t;
}

function composeLine(rule, evidence) {
  const seen = evidence.map((e) => `"${clip(e.title ?? e.rule_key, 80)}" (${clip(e.file, 60)})`).join(', ');
  const fix = clip(evidence[evidence.length - 1].recommended_action
    ?? evidence.find((e) => e.recommended_action)?.recommended_action
    ?? 'Address the class before the step is called done.', 160);
  const how = rule.memory_route === 'check'
    ? CHECK_HINTS[rule.rule_key] ?? `a deterministic check for ${rule.rule_key}.`
    : `/trekreview flags it as ${rule.rule_key}; read the diff for it before the step is called done.`;
  const ids = evidence.map((e) => clip(String(e.id ?? '').slice(0, 8), 8)).join(', ');
  const body = `Mistake: ${clip(rule.description, 160)} Seen: ${seen}. · Fix: ${fix} · How to check: ${how} (evidence: ${ids})`;
  const line = `- [${rule.rule_key}] (${rule.memory_route}) ${body}`;
  if (line.length <= LINE_MAX) return line;
  // Shorten the free-text parts, never the three labels.
  const short = evidence.map((e) => `"${clip(e.title ?? e.rule_key, 40)}" (${clip(e.file, 40)})`).join(', ');
  return `- [${rule.rule_key}] (${rule.memory_route}) Mistake: ${clip(rule.description, 120)} Seen: ${short}. · Fix: ${clip(fix, 120)} · How to check: ${clip(how, 160)} (evidence: ${ids})`;
}

/**
 * Validate a lesson line (generated or operator-edited). Schema-check before
 * any write (Kiro's lesson-capture fix).
 * @param {string} line
 * @returns {{ valid: boolean, reason?: string }}
 */
export function validateLessonLine(line) {
  if (typeof line !== 'string' || line.trim().length === 0) return { valid: false, reason: 'empty' };
  if (/[\u0000-\u001f\u007f\u2028\u2029]/.test(line)) return { valid: false, reason: 'control or line-break character' };
  if (line.length > LINE_MAX) return { valid: false, reason: `longer than ${LINE_MAX}` };
  if (line.includes('<!--') || line.includes('-->')) return { valid: false, reason: 'marker text' };
  const a = line.indexOf('Mistake:');
  const b = line.indexOf('Fix:');
  const c = line.indexOf('How to check:');
  if (a === -1 || b === -1 || c === -1 || !(a < b && b < c)) {
    return { valid: false, reason: 'needs "Mistake:", "Fix:", "How to check:" in that order' };
  }
  return { valid: true };
}

/**
 * Find recurring finding classes in the repo's review history.
 * @param {string} repo
 * @returns {{ proposals: object[], reviews_read: number, skipped: object[] }}
 */
export function proposeLessons(repo) {
  const { reviews, skipped } = collectReviews(repo);
  const already = writtenClasses(repo);
  const byKey = new Map();
  for (const review of reviews) {
    const perKey = new Map();
    for (const f of review.findings) {
      if (f.original_rule_key !== undefined) continue; // normalised from an unknown key: not a class
      if (!perKey.has(f.rule_key)) perKey.set(f.rule_key, []);
      perKey.get(f.rule_key).push(f);
    }
    for (const [key, fs] of perKey) {
      if (!byKey.has(key)) byKey.set(key, []);
      byKey.get(key).push({ review: review.identity, source: review.source, findings: fs });
    }
  }
  const proposals = [];
  for (const [key, sightings] of [...byKey].sort(([a], [b]) => a.localeCompare(b))) {
    // Two reviews, and two DIFFERENT findings: pick, per review, a finding whose
    // id has not been used yet. The same unfixed finding twice is one defect.
    const usedIds = new Set();
    const evidence = [];
    for (const s of sightings) {
      const f = s.findings.find((x) => !usedIds.has(x.id ?? `${x.file}:${x.line}`));
      if (!f) continue;
      usedIds.add(f.id ?? `${f.file}:${f.line}`);
      evidence.push({ ...f, review: s.review, source: s.source });
    }
    if (evidence.length < RECURRENCE_THRESHOLD) continue;
    const rule = getRule(key);
    if (!rule || rule.memory_route === null || rule.memory_route === undefined) continue;
    if (already.has(key)) continue;
    const ev = evidence.slice(0, RECURRENCE_THRESHOLD);
    const line = composeLine(rule, ev);
    if (!validateLessonLine(line).valid) continue; // never propose a line apply would refuse
    proposals.push({
      rule_key: key,
      route: rule.memory_route,
      target: `${LESSONS_DIR}/${LESSONS_FILE_NAME}`,
      line,
      evidence: evidence.map((e) => ({ id: e.id ?? null, review: e.review, source: e.source, file: e.file ?? null, title: e.title ?? null })),
    });
  }
  return { proposals, reviews_read: reviews.length, skipped };
}

// ---- writing an approved lesson ---------------------------------------------

function atomicWriteText(path, text) {
  // Write THROUGH a symlink (CLAUDE.md -> AGENTS.md is common): rename onto the
  // real target, so the link survives and the target gets the change.
  if (existsSync(path) && lstatSync(path).isSymbolicLink()) path = realpathSync(path);
  mkdirSync(dirname(path), { recursive: true });
  const tmp = `${path}.tmp-${process.pid}`;
  writeFileSync(tmp, text);
  renameSync(tmp, path);
}

function withPointer(claudeText) {
  if (claudeText.includes(POINTER_START) || claudeText.includes(POINTER_END)) return null;
  const sep = claudeText.endsWith('\n') ? '\n' : '\n\n';
  return `${claudeText}${sep}${POINTER_START}\n${POINTER_LINE}\n${POINTER_END}\n`;
}

/**
 * Write ONE operator-approved lesson. Call only after the operator said yes to
 * this exact line.
 * @param {string} repo
 * @param {string} ruleKey
 * @param {{ line?: string, budgetEntries?: number, budgetChars?: number }} [opts]
 * @returns {{ written: boolean, reason: string, path?: string, pointer?: string }}
 */
export function applyLesson(repo, ruleKey, opts = {}) {
  const budgetEntries = opts.budgetEntries ?? DEFAULT_BUDGET_ENTRIES;
  const budgetChars = opts.budgetChars ?? DEFAULT_BUDGET_CHARS;
  if (writtenClasses(repo).has(ruleKey)) return { written: false, reason: 'already-present' };
  const proposal = proposeLessons(repo).proposals.find((p) => p.rule_key === ruleKey);
  if (!proposal) return { written: false, reason: 'no-recurrence' };

  let line = proposal.line;
  if (opts.line !== undefined) {
    const body = String(opts.line).replace(/^- \[[A-Z_]+\]\s*(\((check|rule)\)\s*)?/, '');
    line = `- [${ruleKey}] (${proposal.route}) ${body.trim()}`;
  }
  const check = validateLessonLine(line);
  if (!check.valid) return { written: false, reason: 'invalid-line', detail: check.reason };

  const path = lessonsPath(repo);
  const current = existsSync(path) ? readFileSync(path, 'utf-8') : '';
  let block = readBlock(current);
  if (block && block.damaged) {
    return { written: false, reason: 'markers-damaged', detail: `${LESSONS_DIR}/${LESSONS_FILE_NAME} must hold exactly one start marker before one end marker — fix it by hand` };
  }
  let base = current;
  if (!block) {
    const sep = current.length === 0 || current.endsWith('\n') ? '' : '\n';
    base = `${current}${sep}${MARK_START}\n${BLOCK_HEADER}\n${MARK_END}\n`;
    block = readBlock(base);
  }
  const entries = blockEntries(block.inner);
  const innerAfter = `${block.inner.replace(/\n*$/, '')}\n${line}\n`;
  if (entries.length + 1 > budgetEntries || innerAfter.length > budgetChars) {
    return { written: false, reason: 'over-budget', detail: 'the managed block is full — the lessons belong in a skill now (operator decision)' };
  }
  const next = base.slice(0, block.start + MARK_START.length) + innerAfter + base.slice(block.end);
  atomicWriteText(path, next);

  let pointer = 'no-claude-md';
  const claudePath = join(repo, 'CLAUDE.md');
  if (existsSync(claudePath)) {
    const updated = withPointer(readFileSync(claudePath, 'utf-8'));
    if (updated === null) pointer = 'present';
    else { atomicWriteText(claudePath, updated); pointer = 'written'; }
  }
  return { written: true, reason: 'written', path: relative(repo, path), pointer };
}

// ---- CLI --------------------------------------------------------------------

function flag(args, name) {
  const i = args.indexOf(name);
  return i === -1 ? undefined : args[i + 1];
}

if (isMain(import.meta.url)) {
  const [cmd, ...args] = process.argv.slice(2);
  const repo = flag(args, '--repo');
  const out = (o) => process.stdout.write(JSON.stringify(o, null, 2) + '\n');
  if (!['propose', 'apply'].includes(cmd) || !repo) {
    process.stderr.write('Usage: recurrence.mjs propose --repo <dir> | apply --repo <dir> --rule-key <KEY> [--line "<text>"]\n');
    process.exit(2);
  }
  if (!existsSync(repo) || !statSync(repo).isDirectory()) {
    process.stderr.write(`recurrence: repo not found: ${repo}\n`);
    process.exit(2);
  }
  try {
    if (cmd === 'propose') {
      out(proposeLessons(repo));
      process.exit(0);
    }
    const key = flag(args, '--rule-key');
    if (!key) {
      process.stderr.write('recurrence apply: --rule-key is required\n');
      process.exit(2);
    }
    const r = applyLesson(repo, key, { line: flag(args, '--line') });
    out(r);
    process.exit(r.written ? 0 : 1);
  } catch (e) {
    process.stderr.write(`recurrence: could not run: ${e.message}\n`);
    process.exit(2);
  }
}