fix(promoter): cluster by domain:family + repair upsertRequirement field-binding
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) <noreply@anthropic.com>
This commit is contained in:
parent
83c28b756c
commit
f92022730b
2 changed files with 210 additions and 7 deletions
|
|
@ -76,16 +76,30 @@ export function promoteFeedbackToRequirements(basePath = process.cwd()) {
|
||||||
!e.resolvedAt && e.repoIdentity === "forge" && new Date(e.ts) >= cutoff,
|
!e.resolvedAt && e.repoIdentity === "forge" && new Date(e.ts) >= cutoff,
|
||||||
);
|
);
|
||||||
if (eligible.length === 0) return empty;
|
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();
|
const clusters = new Map();
|
||||||
for (const e of eligible) {
|
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) {
|
if (existing) {
|
||||||
existing.entries.push(e);
|
existing.entries.push(e);
|
||||||
existing.distinctMilestones.add(e.occurredIn?.milestone);
|
existing.distinctMilestones.add(e.occurredIn?.milestone);
|
||||||
} else {
|
} else {
|
||||||
clusters.set(e.kind, {
|
clusters.set(clusterKey, {
|
||||||
kind: e.kind,
|
kind: clusterKey,
|
||||||
entries: [e],
|
entries: [e],
|
||||||
distinctMilestones: new Set([e.occurredIn?.milestone]),
|
distinctMilestones: new Set([e.occurredIn?.milestone]),
|
||||||
});
|
});
|
||||||
|
|
@ -110,13 +124,29 @@ export function promoteFeedbackToRequirements(basePath = process.cwd()) {
|
||||||
const notes = `Source IDs: ${sourceIds}`;
|
const notes = `Source IDs: ${sourceIds}`;
|
||||||
if (isDbAvailable()) {
|
if (isDbAvailable()) {
|
||||||
try {
|
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({
|
upsertRequirement({
|
||||||
id: reqId,
|
id: reqId,
|
||||||
title,
|
|
||||||
description: notes,
|
|
||||||
status: "active",
|
|
||||||
class: "operational",
|
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",
|
source: "sf-promoter",
|
||||||
|
primary_owner: null,
|
||||||
|
supporting_slices: null,
|
||||||
|
validation: null,
|
||||||
|
notes,
|
||||||
|
full_content: null,
|
||||||
|
superseded_by: null,
|
||||||
});
|
});
|
||||||
} catch {
|
} catch {
|
||||||
// non-fatal
|
// non-fatal
|
||||||
|
|
|
||||||
173
src/resources/extensions/sf/tests/requirement-promoter.test.mjs
Normal file
173
src/resources/extensions/sf/tests/requirement-promoter.test.mjs
Normal file
|
|
@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Add table
Reference in a new issue