From f92022730b19c8ccf932e6f60a784ab388d2fee0 Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Thu, 14 May 2026 05:34:13 +0200 Subject: [PATCH] fix(promoter): cluster by domain:family + repair upsertRequirement field-binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes that complete AC4 of sf-mp4rxkwt-sfthez (kind taxonomy, commit 83c28b756): 1. Cluster by domain:family prefix instead of exact kind string. The promoter was clustering on the full `kind` value, which after the taxonomy enforcement means every entry like gap:routing:tiebreak-cost-only and gap:routing:agentic-axis-partial- coverage stayed in cluster size 1. Empirical confirmation: live ledger 2026-05-14 had 10 open entries, max cluster size 1 under exact-string matching — promoter could never fire on real diverse data. New behavior: extract first two segments as the cluster key. Entries sharing domain:family group together; legacy single-segment kinds cluster as themselves. With this change, the live ledger's gap:routing family would include 3 entries today. 2. Repair the silently-broken upsertRequirement call (LATENT BUG). The promoter was calling upsertRequirement with only {id, title, description, status, class, source} — but the schema binds every column positionally including {why, primary_owner, supporting_slices, validation, notes, full_content, superseded_by}. SQLite cannot bind `undefined`, so EVERY upsert attempt threw — caught silently by the surrounding try/catch ("non-fatal") with no log line. Result: the promoter has never successfully created a requirement row in this project's history, regardless of clustering threshold. Fix: pass all schema columns explicitly with null defaults for unused ones. Also encode the human-readable cluster title into description's first line since the requirements table has no title column (separate schema-evolution concern, out of scope here). Tests: new tests/requirement-promoter.test.mjs (5 tests) covers domain:family clustering when count>=5, no cross-family clustering, legacy single-segment kinds, below-threshold returns 0, non-forge bail. The first test would have caught both the prefix clustering miss AND the upsertRequirement field-binding bug — runs end-to-end through upsertRequirement → getActiveRequirements. 1601/1601 SF extension tests pass; typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../extensions/sf/requirement-promoter.js | 44 ++++- .../sf/tests/requirement-promoter.test.mjs | 173 ++++++++++++++++++ 2 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 src/resources/extensions/sf/tests/requirement-promoter.test.mjs diff --git a/src/resources/extensions/sf/requirement-promoter.js b/src/resources/extensions/sf/requirement-promoter.js index c9c44dcff..a16611ccd 100644 --- a/src/resources/extensions/sf/requirement-promoter.js +++ b/src/resources/extensions/sf/requirement-promoter.js @@ -76,16 +76,30 @@ export function promoteFeedbackToRequirements(basePath = process.cwd()) { !e.resolvedAt && e.repoIdentity === "forge" && new Date(e.ts) >= cutoff, ); if (eligible.length === 0) return empty; - // Cluster by kind + // Cluster by kind PREFIX (domain:family from the canonical + // domain:family[:specific] taxonomy enforced at recordSelfFeedback in + // commit 83c28b756 — sf-mp4rxkwt-sfthez). Without this, every entry + // has a unique full kind string and clusters are size-1 forever, so + // the promoter can never fire (verified empirically on the live + // ledger 2026-05-14: 10 open entries, max cluster size 1 under + // exact-string matching). Using the first two segments groups e.g. + // gap:routing:tiebreak-cost-only with gap:routing:agentic-axis- + // partial-coverage as one "gap:routing" cluster while still + // distinguishing them from gap:upstream-bridge-no-closure-memory. + // Single-segment legacy kinds are clustered as themselves (no + // prefix to extract). const clusters = new Map(); for (const e of eligible) { - const existing = clusters.get(e.kind); + const segments = (e.kind ?? "").split(":"); + const clusterKey = + segments.length >= 2 ? `${segments[0]}:${segments[1]}` : (e.kind ?? ""); + const existing = clusters.get(clusterKey); if (existing) { existing.entries.push(e); existing.distinctMilestones.add(e.occurredIn?.milestone); } else { - clusters.set(e.kind, { - kind: e.kind, + clusters.set(clusterKey, { + kind: clusterKey, entries: [e], distinctMilestones: new Set([e.occurredIn?.milestone]), }); @@ -110,13 +124,29 @@ export function promoteFeedbackToRequirements(basePath = process.cwd()) { const notes = `Source IDs: ${sourceIds}`; if (isDbAvailable()) { try { + // upsertRequirement binds every column positionally, so all + // fields the schema names must be supplied — undefined cannot + // bind to SQLite. Latent bug observed during AC4 work for + // sf-mp4rxkwt-sfthez: the promoter has been silently swallowing + // upsert failures in the try/catch below, which is why it + // "never fired" in production — the upsert was throwing on + // every attempt because partial fields were left undefined. + // `title` is computed for log/notify but the requirements + // schema has no title column; we encode it as the first line + // of `description` so it survives. upsertRequirement({ id: reqId, - title, - description: notes, - status: "active", class: "operational", + status: "active", + description: `${title}\n\n${notes}`, + why: `Threshold reached: ${count} entr${count === 1 ? "y" : "ies"} of kind cluster ${cluster.kind} ${milestoneCount > 1 ? `across ${milestoneCount} milestones` : ""}`.trim(), source: "sf-promoter", + primary_owner: null, + supporting_slices: null, + validation: null, + notes, + full_content: null, + superseded_by: null, }); } catch { // non-fatal diff --git a/src/resources/extensions/sf/tests/requirement-promoter.test.mjs b/src/resources/extensions/sf/tests/requirement-promoter.test.mjs new file mode 100644 index 000000000..e6c90380e --- /dev/null +++ b/src/resources/extensions/sf/tests/requirement-promoter.test.mjs @@ -0,0 +1,173 @@ +/** + * requirement-promoter.test.mjs — clusters self-feedback by domain:family + * prefix and promotes recurring clusters to requirement rows. + * + * Covers AC4 of sf-mp4rxkwt-sfthez (kind taxonomy enforcement, commit + * 83c28b756): the threshold-promoter MUST cluster by domain:family prefix + * (not exact kind) so 5 entries with the same family but different + * specific suffixes can fire the COUNT_THRESHOLD=5 trigger. + */ +import { + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, test } from "vitest"; +import { promoteFeedbackToRequirements } from "../requirement-promoter.js"; +import { + closeDatabase, + getActiveRequirements, + listSelfFeedbackEntries, + openDatabase, +} from "../sf-db.js"; +import { recordSelfFeedback } from "../self-feedback.js"; + +const tmpDirs = []; + +afterEach(() => { + closeDatabase(); + while (tmpDirs.length > 0) { + const dir = tmpDirs.pop(); + if (dir) rmSync(dir, { recursive: true, force: true }); + } +}); + +function makeForgeProject() { + const dir = mkdtempSync(join(tmpdir(), "sf-req-promoter-")); + tmpDirs.push(dir); + mkdirSync(join(dir, ".sf"), { recursive: true }); + writeFileSync( + join(dir, "package.json"), + JSON.stringify({ name: "singularity-forge" }), + ); + openDatabase(join(dir, ".sf", "sf.db")); + return dir; +} + +describe("promoteFeedbackToRequirements", () => { + test("clusters by domain:family prefix and promotes when count >= 5", () => { + const project = makeForgeProject(); + + // 5 entries sharing the same domain:family (gap:routing) but with + // distinct :specific suffixes — exact-string clustering would see + // these as 5 separate kinds and never fire. domain:family clustering + // groups them as one "gap:routing" cluster of size 5. + const specifics = [ + "tiebreak-cost-only", + "agentic-axis-partial-coverage", + "adr-0079-step-4-escalation", + "upstream-bridge-no-closure-memory", + "missing-fallback-policy", + ]; + for (const specific of specifics) { + const filed = recordSelfFeedback( + { + kind: `gap:routing:${specific}`, + severity: "medium", + summary: `synthetic ${specific}`, + }, + project, + ); + expect(filed).toBeTruthy(); + } + + const result = promoteFeedbackToRequirements(project); + expect(result.promoted).toBe(1); + expect(result.requirementIds).toHaveLength(1); + + // The requirement row was created. NOTE: requirements schema has no + // `title` column — the promoter's title computation is lost at the + // upsert layer (separate bug worth filing). We assert source + + // description (which carries the source IDs list). + const reqs = getActiveRequirements(); + const promoted = reqs.find((r) => + result.requirementIds.includes(r.id), + ); + expect(promoted).toBeTruthy(); + expect(promoted.source).toBe("sf-promoter"); + expect(promoted.description).toContain("Source IDs:"); + + // Each contributing entry should be marked resolved with + // promoted-to-requirement evidence + const entries = listSelfFeedbackEntries(); + const promotedEntries = entries.filter((e) => + specifics.some((s) => e.kind === `gap:routing:${s}`), + ); + expect(promotedEntries).toHaveLength(5); + for (const e of promotedEntries) { + expect(e.resolvedAt).toBeTruthy(); + expect(e.resolvedEvidence?.kind).toBe("promoted-to-requirement"); + } + }); + + test("does NOT cluster across different families even within the same domain", () => { + const project = makeForgeProject(); + + // Mixed: 3 gap:routing + 3 gap:reflection — neither family has 5, + // neither hits MILESTONE_THRESHOLD=3 since occurredIn.milestone is + // undefined. Total is 6 but split across families = no promotion. + for (const f of ["routing", "reflection"]) { + for (let i = 0; i < 3; i++) { + recordSelfFeedback( + { + kind: `gap:${f}:specific-${i}`, + severity: "medium", + summary: `synthetic ${f} ${i}`, + }, + project, + ); + } + } + + const result = promoteFeedbackToRequirements(project); + expect(result.promoted).toBe(0); + }); + + test("legacy single-segment kinds cluster as themselves (no prefix to extract)", () => { + const project = makeForgeProject(); + + // 5 single-segment legacy kinds — each entry has kind="runaway-loop" + // with no specific suffix. These cluster as "runaway-loop" itself. + for (let i = 0; i < 5; i++) { + recordSelfFeedback( + { + kind: "runaway-loop", + severity: "medium", + summary: `legacy entry ${i}`, + }, + project, + ); + } + + const result = promoteFeedbackToRequirements(project); + expect(result.promoted).toBe(1); + }); + + test("returns 0 when below count threshold and no milestone-distinct trigger", () => { + const project = makeForgeProject(); + for (let i = 0; i < 4; i++) { + recordSelfFeedback( + { + kind: `gap:routing:s${i}`, + severity: "low", + summary: `synthetic ${i}`, + }, + project, + ); + } + expect(promoteFeedbackToRequirements(project).promoted).toBe(0); + }); + + test("returns 0 when basePath isn't the forge repo", () => { + const dir = mkdtempSync(join(tmpdir(), "sf-non-forge-")); + tmpDirs.push(dir); + writeFileSync( + join(dir, "package.json"), + JSON.stringify({ name: "some-other-app" }), + ); + expect(promoteFeedbackToRequirements(dir).promoted).toBe(0); + }); +});