diff --git a/README.md b/README.md index 8a6a1b0fd..16823c1e8 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ On [LoCoMo](https://arxiv.org/abs/2402.17753), the public long-context memory be - πŸ“₯ **Captures** every session's prompts, tool calls, and responses as structured traces in Deeplake - 🧠 **Codifies** patterns into reusable `SKILL.md` files, available to every agent on your team -- πŸ” **Searches** traces and skills with hybrid lexical + semantic retrieval (BM25 fallback when embeddings off) +- πŸ” **Searches** traces and skills with hybrid lexical + semantic retrieval: a `UNION ALL` of `LIKE`/`ILIKE` substring rows (sentinel score 1.0, capped by `HIVEMIND_HYBRID_LEXICAL_LIMIT`) and cosine-similarity rows (real 0–1 score), ordered by score β€” so exact keyword matches always lead while semantic hits fill in below. When embeddings are off, falls back to lexical `LIKE`/`ILIKE` only. (BM25 was evaluated but dropped: its unbounded score scale (~1–3) overwhelmed cosine in a shared `ORDER BY`, requiring rank-based fusion (RRF) or score normalisation to use safely.) - πŸ”— **Propagates** capability across sessions, agents, teammates, and machines in real time - πŸ“ **Intercepts** file operations on `~/.deeplake/memory/` through a virtual filesystem backed by SQL - πŸ“ **Summarizes** sessions into AI-generated wiki pages via a background worker at session end diff --git a/package-lock.json b/package-lock.json index 14a0e8ec4..77e21de25 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1118,28 +1118,6 @@ "node": ">=0.1.90" } }, - "node_modules/@emnapi/core": { - "version": "1.9.1", - "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.9.1.tgz", - "integrity": "sha512-mukuNALVsoix/w1BJwFzwXBN/dHeejQtuVzcDsfOEsdpCumXb/E9j8w11h5S54tT1xhifGfbbSm/ICrObRb3KA==", - "dev": true, - "license": "MIT", - "optional": true, - "dependencies": { - "@emnapi/wasi-threads": "1.2.0", - "tslib": "^2.4.0" - } - }, - "node_modules/@emnapi/runtime": { - "version": "1.9.1", - "resolved": "https://registry.npmjs.org/@emnapi/runtime/-/runtime-1.9.1.tgz", - "integrity": "sha512-VYi5+ZVLhpgK4hQ0TAjiQiZ6ol0oe4mBx7mVv7IflsiEp0OWoVsp/+f9Vc1hOhE0TtkORVrI1GvzyreqpgWtkA==", - "license": "MIT", - "optional": true, - "dependencies": { - "tslib": "^2.4.0" - } - }, "node_modules/@emnapi/wasi-threads": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/@emnapi/wasi-threads/-/wasi-threads-1.2.0.tgz", @@ -1721,9 +1699,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1740,9 +1715,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1759,9 +1731,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1778,9 +1747,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1797,9 +1763,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1832,9 +1795,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1867,9 +1827,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1892,9 +1849,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1917,9 +1871,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1942,9 +1893,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1967,9 +1915,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2014,9 +1959,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2573,9 +2515,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2593,9 +2532,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2613,9 +2549,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2633,9 +2566,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3633,6 +3563,7 @@ "integrity": "sha512-/MBdrkA8t6hbdCWFKs09dPik774xvs4Z6L4bycdCxYNLHM8oZuRyosumQMG19LUlBsB6GeVpL1q4kFFazvyKGA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@bcoe/v8-coverage": "^1.0.2", "@vitest/utils": "4.1.3", @@ -4634,6 +4565,7 @@ "dev": true, "hasInstallScript": true, "license": "MIT", + "peer": true, "bin": { "esbuild": "bin/esbuild" }, @@ -4807,6 +4739,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -5328,6 +5261,7 @@ "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.15.tgz", "integrity": "sha512-qM0jDhFEaCBb4TxoW7f53Qrpv9RBiayUHo0S52JudprkhvpjIrGoU1mnnr29Fvd1U335ZFPZQY1wlkqgfGXyLg==", "license": "MIT", + "peer": true, "engines": { "node": ">=16.9.0" } @@ -5922,9 +5856,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -5946,9 +5877,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -6720,34 +6648,6 @@ "dev": true, "license": "MIT" }, - "node_modules/pg": { - "version": "8.20.0", - "resolved": "https://registry.npmjs.org/pg/-/pg-8.20.0.tgz", - "integrity": "sha512-ldhMxz2r8fl/6QkXnBD3CR9/xg694oT6DZQ2s6c/RI28OjtSOpxnPrUCGOBJ46RCUxcWdx3p6kw/xnDHjKvaRA==", - "license": "MIT", - "optional": true, - "dependencies": { - "pg-connection-string": "^2.12.0", - "pg-pool": "^3.13.0", - "pg-protocol": "^1.13.0", - "pg-types": "2.2.0", - "pgpass": "1.0.5" - }, - "engines": { - "node": ">= 16.0.0" - }, - "optionalDependencies": { - "pg-cloudflare": "^1.3.0" - }, - "peerDependencies": { - "pg-native": ">=3.0.1" - }, - "peerDependenciesMeta": { - "pg-native": { - "optional": true - } - } - }, "node_modules/pg-cloudflare": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/pg-cloudflare/-/pg-cloudflare-1.3.0.tgz", @@ -8162,18 +8062,6 @@ "url": "https://github.com/sponsors/Borewit" } }, - "node_modules/tree-sitter": { - "version": "0.21.1", - "resolved": "https://registry.npmjs.org/tree-sitter/-/tree-sitter-0.21.1.tgz", - "integrity": "sha512-7dxoA6kYvtgWw80265MyqJlkRl4yawIjO7S5MigytjELkX43fV2WsAXzsNfO7sBpPPCF5Gp0+XzHk0DwLCq3xQ==", - "hasInstallScript": true, - "license": "MIT", - "optional": true, - "dependencies": { - "node-addon-api": "^8.0.0", - "node-gyp-build": "^4.8.0" - } - }, "node_modules/tree-sitter-c": { "version": "0.23.2", "resolved": "https://registry.npmjs.org/tree-sitter-c/-/tree-sitter-c-0.23.2.tgz", @@ -9000,6 +8888,7 @@ "integrity": "sha512-P1PbweD+2/udplnThz3btF4cf6AgPky7kk23RtHUkJIU5BIxwPprhRGmOAHs6FTI7UiGbTNrgNP6jSYD6JaRnw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "lightningcss": "^1.32.0", "picomatch": "^4.0.4", @@ -9078,6 +8967,7 @@ "integrity": "sha512-DBc4Tx0MPNsqb9isoyOq00lHftVx/KIU44QOm2q59npZyLUkENn8TMFsuzuO+4U2FUa9rgbbPt3udrP25GcjXw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "4.1.3", "@vitest/mocker": "4.1.3", @@ -9311,6 +9201,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-4.3.6.tgz", "integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/src/embeddings/disable.ts b/src/embeddings/disable.ts index a96741975..7af74a0f3 100644 --- a/src/embeddings/disable.ts +++ b/src/embeddings/disable.ts @@ -23,7 +23,9 @@ import { getEmbeddingsEnabled } from "../user-config.js"; * * In either case: SessionStart skips the warmup, capture / wiki-worker * write rows with NULL in the embedding column, and `Grep` falls back to - * BM25 / ILIKE matching on text columns. Existing rows' embeddings remain + * lexical `LIKE`/`ILIKE` matching on text columns (BM25 was evaluated but + * dropped β€” its score scale (~1..3) is incompatible with cosine in a single + * ORDER BY without RRF or normalisation; see grep-core.ts PR-NOTES F4c). * readable. * * Read-once: the status is cached for the lifetime of the (short-lived) diff --git a/src/skillify/skillopt-improve.ts b/src/skillify/skillopt-improve.ts index 70782fa2a..ae9c73bb0 100644 --- a/src/skillify/skillopt-improve.ts +++ b/src/skillify/skillopt-improve.ts @@ -81,7 +81,22 @@ export interface ImproveOpts { now: string; prior?: (name: string, author: string) => string[]; alreadyProposed?: (name: string, author: string, edits: Edit[]) => boolean; - recordEdit?: (name: string, author: string, edits: Edit[]) => void; + recordEdit?: (name: string, author: string, edits: Edit[], publishedVersion: number) => void; + /** + * Called after the judge runs to close the meta-learning loop. When the judge + * says the task PASSED (`"applied"`), the most recently proposed edit for this + * skill gets credit. When the task FAILED again and we are publishing a replacement, + * the prior edit is marked as not having helped. + * + * `priorVersion`: the version of the skill that was current when this judgment ran. + * When provided, resolution is pinned to the fingerprint that produced `priorVersion` + * (via `fingerprintForVersion`) rather than log recency, preventing a delayed judgment + * for version N from marking a later version's edit as reverted. + * Omit (or pass undefined) for the `"applied"` path where log-recency is correct: + * the currently-live edit is the right thing to credit when a task passes. + * Best-effort β€” a failure here must not affect the improvement result. + */ + resolveEdit?: (name: string, author: string, status: "applied" | "reverted", priorVersion?: number) => void; // Deeplake insertβ†’read lag tolerance: the invocation row is written by a SEPARATE process // (capture.js) and lands in Deeplake on a short visibility lag (expected, not a defect), so a // worker firing on a fast reaction can read stale. Poll findInvocation with linear backoff @@ -129,25 +144,39 @@ export async function improveSkillIfFailed(opts: ImproveOpts): Promise/skillopt/meta.jsonl. Pure helpers + injected path, * so it's unit-tested with a tmp file. @@ -20,6 +28,15 @@ export interface MetaEntry { fingerprint: string; // stable hash of the edits, for dedup proposedAt: string; status: MetaStatus; + /** Present only on patch entries written by `patchMeta`; absent on originals. */ + resolvedAt?: string; + /** + * The skill version published when this edit was recorded (set by `recordEdit` after a + * successful `publishImprovedSkill`). Used by `fingerprintForVersion` to resolve the + * correct entry for a given judgment, preventing a delayed judgment for version N from + * accidentally marking a later version's edit as reverted. + */ + publishedVersion?: number; } export const skillRef = (name: string, author: string) => `${name}--${author}`; @@ -59,6 +76,28 @@ export function appendMeta(file: string, entry: MetaEntry): void { fs.appendFileSync(file, JSON.stringify(entry) + "\n"); } +/** + * Append a patch entry that updates the status of an existing meta entry without + * mutating the original line (the JSONL is append-only). Readers fold by fingerprint + * (last-write wins), so this patch supersedes the original proposed entry. + * + * No-op (safe to call) when `fingerprint` is empty/null β€” callers should guard with + * `latestUnresolvedFingerprint` before calling. + */ +export function patchMeta( + file: string, + skill: string, + fingerprint: string, + status: "applied" | "reverted", + resolvedAt: string, +): void { + if (!fingerprint) return; + // Patch entries carry resolvedAt and the updated status, but keep ops empty to + // avoid re-emitting the full summary (the original entry already has it). + const patch: MetaEntry = { skill, ops: [], fingerprint, proposedAt: "", status, resolvedAt }; + appendMeta(file, patch); +} + /** Has this exact edit set already been proposed for this skill? (avoid churn) */ export function alreadyProposed(meta: MetaEntry[], name: string, author: string, edits: Edit[]): boolean { const ref = skillRef(name, author); @@ -66,19 +105,103 @@ export function alreadyProposed(meta: MetaEntry[], name: string, author: string, return meta.some((m) => m.skill === ref && m.fingerprint === fp); } -/** Summaries of edits previously tried for this skill β€” context for the proposer. */ +/** + * Build the resolved view of the meta log for a skill: fold by fingerprint (last-write + * wins), so patch entries override the originals. Returns a map from fingerprint β†’ + * resolved entry, preserving only entries for the given skill. + */ +function resolvedView(meta: MetaEntry[], ref: string): Map { + const map = new Map(); + for (const m of meta) { + if (m.skill !== ref) continue; + const prior = map.get(m.fingerprint); + if (!prior) { + map.set(m.fingerprint, m); + } else { + // Merge: prefer the patch's status/resolvedAt, but keep the original's + // ops/proposedAt/publishedVersion so summaries don't disappear when a patch + // entry (with ops:[]) supersedes the original. + map.set(m.fingerprint, { + ...prior, + status: m.status, + ...(m.resolvedAt ? { resolvedAt: m.resolvedAt } : {}), + }); + } + } + return map; +} + +/** + * Summaries of edits previously tried for this skill β€” context for the proposer. + * Each summary is annotated with its resolved outcome so the proposer can prefer + * edit shapes that were applied and avoid re-trying reverted ones. + * + * Format: "[applied] append: Always assert..." / "[reverted] replace @\"...\"" / "[proposed] ..." + */ export function priorEditSummaries(meta: MetaEntry[], name: string, author: string): string[] { const ref = skillRef(name, author); - return meta.filter((m) => m.skill === ref).flatMap((m) => m.ops); + const resolved = resolvedView(meta, ref); + const out: string[] = []; + for (const entry of resolved.values()) { + const label = `[${entry.status}]`; + for (const op of entry.ops) { + out.push(`${label} ${op}`); + } + } + return out; +} + +/** + * The fingerprint of the latest meta entry for this skill that has NOT yet been + * resolved (still `"proposed"`). Used by the worker to know which entry to patch + * when a verdict comes in. Returns null if there's no unresolved entry. + */ +export function latestUnresolvedFingerprint(meta: MetaEntry[], name: string, author: string): string | null { + const ref = skillRef(name, author); + const resolved = resolvedView(meta, ref); + // Walk meta in reverse to find the most recently proposed entry that is still unresolved. + for (let i = meta.length - 1; i >= 0; i--) { + const m = meta[i]; + if (m.skill !== ref) continue; + const r = resolved.get(m.fingerprint); + if (r && r.status === "proposed") return m.fingerprint; + } + return null; } /** Build a meta entry for a freshly-proposed edit set. */ -export function metaEntryFor(name: string, author: string, edits: Edit[], now: string): MetaEntry { +export function metaEntryFor( + name: string, author: string, edits: Edit[], now: string, + publishedVersion?: number, +): MetaEntry { return { skill: skillRef(name, author), ops: edits.map(summarizeEdit), fingerprint: fingerprintEdits(edits), proposedAt: now, status: "proposed", + ...(publishedVersion !== undefined ? { publishedVersion } : {}), }; } + +/** + * Find the fingerprint of the meta entry whose edit produced `version` of skill + * `name--author`. Returns null when no entry carries that publishedVersion (e.g. + * the entry pre-dates this field, or the version was never recorded). + * + * Used by the worker's `resolveEdit` closure to pin resolution to the exact edit + * that produced the version currently in effect during this judgment, rather than + * relying on log recency β€” which would incorrectly attribute a delayed judgment + * for version N to a newer edit at version N+k. + */ +export function fingerprintForVersion( + meta: MetaEntry[], name: string, author: string, version: number, +): string | null { + const ref = skillRef(name, author); + // Walk forward: the FIRST entry whose publishedVersion matches is the original + // (patch entries don't carry publishedVersion, so they won't match). + for (const m of meta) { + if (m.skill === ref && m.publishedVersion === version) return m.fingerprint; + } + return null; +} diff --git a/src/skillify/skillopt-worker.ts b/src/skillify/skillopt-worker.ts index afadba33a..4e066581d 100644 --- a/src/skillify/skillopt-worker.ts +++ b/src/skillify/skillopt-worker.ts @@ -18,7 +18,7 @@ import { DeeplakeApi } from "../deeplake-api.js"; import { getStateDir } from "./state-dir.js"; import { agentModel, detectScorerAgent } from "./agent-model.js"; import { improveSkillIfFailed } from "./skillopt-improve.js"; -import { loadMeta, appendMeta, priorEditSummaries, alreadyProposed, metaEntryFor } from "./skillopt-meta.js"; +import { loadMeta, appendMeta, patchMeta, priorEditSummaries, alreadyProposed, metaEntryFor, skillRef as mkSkillRef, latestUnresolvedFingerprint, fingerprintForVersion } from "./skillopt-meta.js"; import { tryAcquireWorkerLock, releaseWorkerLock } from "./state.js"; import { SKILLOPT_ENV } from "./skillopt-env.js"; @@ -99,7 +99,26 @@ async function main(): Promise { now, prior: (n, a) => priorEditSummaries(metaCache, n, a), alreadyProposed: (n, a, edits) => alreadyProposed(metaCache, n, a, edits), - recordEdit: (n, a, edits) => { const e = metaEntryFor(n, a, edits, now); appendMeta(metaFile, e); metaCache.push(e); }, + recordEdit: (n, a, edits, publishedVersion) => { + const e = metaEntryFor(n, a, edits, now, publishedVersion); + appendMeta(metaFile, e); + metaCache.push(e); + }, + resolveEdit: (n, a, status, priorVersion) => { + // When priorVersion is provided (the "reverted" pre-publish path), look up the + // fingerprint that produced that exact version β€” this prevents a delayed judgment + // for version N from accidentally marking a later version's edit as reverted. + // When priorVersion is absent (the "applied" path), log-recency is correct: the + // currently-live edit is the right one to credit when a task passes. + const fp = priorVersion !== undefined + ? fingerprintForVersion(metaCache, n, a, priorVersion) + : latestUnresolvedFingerprint(metaCache, n, a); + if (!fp) return; // nothing to resolve + const ref = mkSkillRef(n, a); + patchMeta(metaFile, ref, fp, status, now); + // Reflect the patch in the in-memory cache so this process sees the resolved view. + metaCache.push({ skill: ref, ops: [], fingerprint: fp, proposedAt: "", status, resolvedAt: now }); + }, }); if (r.improved) log(`improved ${skillRef} β†’ v${r.version} (${r.reason})`); diff --git a/tests/shared/skillopt-improve.test.ts b/tests/shared/skillopt-improve.test.ts index 925c4c210..d681d4f5e 100644 --- a/tests/shared/skillopt-improve.test.ts +++ b/tests/shared/skillopt-improve.test.ts @@ -75,7 +75,7 @@ describe("improveSkillIfFailed", () => { expect(inserts).toHaveLength(1); expect(inserts[0]).toContain("Always assert the real outbound HTTP request."); expect(inserts[0]).toContain("'team'"); // scope promoted on publish - expect(recordEdit).toHaveBeenCalled(); + expect(recordEdit).toHaveBeenCalledWith("posthog", "kamo", expect.any(Array), 3); // publishedVersion=3 }); it("judge says OK β†’ no improvement, no publish", async () => { @@ -186,4 +186,78 @@ describe("improveSkillIfFailed", () => { .rejects.toThrow(/402/); expect(sessionsCalls).toBe(1); // threw on the first query β€” no retry loop }); + + // ── resolveEdit outcome-signal tests ───────────────────────────────────────────── + describe("resolveEdit outcome signal", () => { + it("calls resolveEdit('applied') when the judge says the task passed", async () => { + const { query } = makeQuery(); + const resolveEdit = vi.fn(); + const r = await improveSkillIfFailed(base(query, { judge: PASS_JUDGE, resolveEdit })); + expect(r).toMatchObject({ judged: true, failed: false }); + expect(resolveEdit).toHaveBeenCalledOnce(); + expect(resolveEdit).toHaveBeenCalledWith("posthog", "kamo", "applied"); + }); + + it("does NOT call resolveEdit when failed and skill is not in the org table (infrastructure gap, not a verdict on the prior edit)", async () => { + const { query } = makeQuery({ skillRows: [] }); + const resolveEdit = vi.fn(); + const r = await improveSkillIfFailed(base(query, { resolveEdit })); + expect(r).toMatchObject({ judged: true, failed: true, improved: false, reason: "skill not in org table" }); + expect(resolveEdit).not.toHaveBeenCalled(); + }); + + it("does NOT call resolveEdit when failed and the proposer makes no change (can't produce a replacement, not a verdict on the prior edit)", async () => { + const { query } = makeQuery(); + const resolveEdit = vi.fn(); + const r = await improveSkillIfFailed(base(query, { proposerModel: async () => "[]", resolveEdit })); + expect(r).toMatchObject({ judged: true, failed: true, improved: false, reason: "proposer made no change" }); + expect(resolveEdit).not.toHaveBeenCalled(); + }); + + it("does NOT call resolveEdit when failed and dedup blocks the improvement (edit is still in play, not definitively bad)", async () => { + const { query } = makeQuery(); + const resolveEdit = vi.fn(); + const r = await improveSkillIfFailed(base(query, { alreadyProposed: () => true, resolveEdit })); + expect(r).toMatchObject({ judged: true, failed: true, improved: false, reason: "edit already proposed (dedup)" }); + expect(resolveEdit).not.toHaveBeenCalled(); + }); + + it("does NOT call resolveEdit when the invocation is not found (not judged)", async () => { + const { query } = makeQuery(); + const resolveEdit = vi.fn(); + await improveSkillIfFailed(base(query, { skillRef: "ghost--x", invocationRetries: 0, resolveEdit })); + expect(resolveEdit).not.toHaveBeenCalled(); + }); + + it("does NOT call resolveEdit for non-org skills (not judged)", async () => { + const { query } = makeQuery(); + const resolveEdit = vi.fn(); + await improveSkillIfFailed(base(query, { skillRef: "bare", resolveEdit })); + await improveSkillIfFailed(base(query, { skillRef: "hivemind:memory", resolveEdit })); + expect(resolveEdit).not.toHaveBeenCalled(); + }); + + it("swallows a resolveEdit error so it cannot affect the improvement result", async () => { + const { query } = makeQuery(); + const resolveEdit = vi.fn(() => { throw new Error("disk full"); }); + // Should still return a valid result despite the error in resolveEdit + const r = await improveSkillIfFailed(base(query, { judge: PASS_JUDGE, resolveEdit })); + expect(r).toMatchObject({ judged: true, failed: false, improved: false }); + }); + + it("calls resolveEdit('reverted', priorVersion) before publishing β€” version-stable resolution prevents cross-version race", async () => { + // resolveEdit must receive the current skill version (2, from SKILL_ROW) so the worker + // can use fingerprintForVersion to resolve the exact entry that produced that version, + // rather than log recency which would mark a newer edit if this is a delayed judgment. + const { query } = makeQuery(); + const resolveEdit = vi.fn(); + const recordEdit = vi.fn(); + const r = await improveSkillIfFailed(base(query, { resolveEdit, recordEdit })); + expect(r).toMatchObject({ judged: true, failed: true, improved: true }); + expect(recordEdit).toHaveBeenCalledWith("posthog", "kamo", expect.any(Array), 3); // publishedVersion=3 + expect(resolveEdit).toHaveBeenCalledOnce(); + // priorVersion=2 = current.version from SKILL_ROW; allows version-stable fingerprint lookup + expect(resolveEdit).toHaveBeenCalledWith("posthog", "kamo", "reverted", 2); + }); + }); }); diff --git a/tests/shared/skillopt-meta.test.ts b/tests/shared/skillopt-meta.test.ts index 881187845..09682ae4b 100644 --- a/tests/shared/skillopt-meta.test.ts +++ b/tests/shared/skillopt-meta.test.ts @@ -4,6 +4,7 @@ import os from "node:os"; import path from "node:path"; import { fingerprintEdits, alreadyProposed, priorEditSummaries, metaEntryFor, loadMeta, appendMeta, + patchMeta, latestUnresolvedFingerprint, fingerprintForVersion, } from "../../src/skillify/skillopt-meta.js"; import type { Edit } from "../../src/skillify/skill-edits.js"; @@ -39,6 +40,50 @@ describe("alreadyProposed / priorEditSummaries", () => { expect(prior.join(" ")).toMatch(/delete @"old rule"/); expect(prior.join(" ")).toMatch(/append: x/); }); + + it("annotates summaries with [proposed] status when no patch exists", () => { + const m = [metaEntryFor("posthog", "kamo", edits, "t1")]; + const prior = priorEditSummaries(m, "posthog", "kamo"); + expect(prior.every((s) => s.startsWith("[proposed]"))).toBe(true); + }); + + it("annotates summaries with [applied] after a patch marks the entry applied", () => { + const fp = fingerprintEdits(edits); + const m = [ + metaEntryFor("posthog", "kamo", edits, "t1"), + { skill: "posthog--kamo", ops: [], fingerprint: fp, proposedAt: "", status: "applied" as const, resolvedAt: "t2" }, + ]; + const prior = priorEditSummaries(m, "posthog", "kamo"); + // All ops from the original entry should now be annotated [applied] + expect(prior.every((s) => s.startsWith("[applied]"))).toBe(true); + expect(prior.length).toBe(2); // still both ops + }); + + it("annotates summaries with [reverted] after a patch marks the entry reverted", () => { + const fp = fingerprintEdits(edits); + const m = [ + metaEntryFor("posthog", "kamo", edits, "t1"), + { skill: "posthog--kamo", ops: [], fingerprint: fp, proposedAt: "", status: "reverted" as const, resolvedAt: "t2" }, + ]; + const prior = priorEditSummaries(m, "posthog", "kamo"); + expect(prior.every((s) => s.startsWith("[reverted]"))).toBe(true); + }); + + it("handles multiple different edit sets with mixed statuses independently", () => { + const edits2: Edit[] = [{ op: "insert_after", target: "## Rules", content: "new rule" }]; + const fp1 = fingerprintEdits(edits); + const m = [ + metaEntryFor("posthog", "kamo", edits, "t1"), + { skill: "posthog--kamo", ops: [], fingerprint: fp1, proposedAt: "", status: "applied" as const, resolvedAt: "t2" }, + metaEntryFor("posthog", "kamo", edits2, "t3"), + // edits2 stays "proposed" (no patch) + ]; + const prior = priorEditSummaries(m, "posthog", "kamo"); + const appliedOnes = prior.filter((s) => s.startsWith("[applied]")); + const proposedOnes = prior.filter((s) => s.startsWith("[proposed]")); + expect(appliedOnes.length).toBe(2); // edits has 2 ops + expect(proposedOnes.length).toBe(1); // edits2 has 1 op + }); }); describe("loadMeta / appendMeta", () => { @@ -60,3 +105,108 @@ describe("loadMeta / appendMeta", () => { expect(loadMeta(path.join(os.tmpdir(), "does-not-exist-xyz.jsonl"))).toEqual([]); }); }); + +describe("patchMeta", () => { + let file: string; + beforeEach(() => { file = path.join(fs.mkdtempSync(path.join(os.tmpdir(), "patch-")), "meta.jsonl"); }); + afterEach(() => { fs.rmSync(path.dirname(file), { recursive: true, force: true }); }); + + it("appends a patch entry and loadMeta reads it back with resolvedAt", () => { + const entry = metaEntryFor("skill", "auth", edits, "t1"); + appendMeta(file, entry); + patchMeta(file, "skill--auth", entry.fingerprint, "applied", "t2"); + const loaded = loadMeta(file); + expect(loaded).toHaveLength(2); + expect(loaded[0].status).toBe("proposed"); + expect(loaded[1].status).toBe("applied"); + expect(loaded[1].resolvedAt).toBe("t2"); + expect(loaded[1].ops).toEqual([]); // patch entries carry no ops (original does) + }); + + it("is a no-op when fingerprint is empty", () => { + appendMeta(file, metaEntryFor("skill", "auth", edits, "t1")); + patchMeta(file, "skill--auth", "", "applied", "t2"); + expect(loadMeta(file)).toHaveLength(1); // nothing appended + }); + + it("priorEditSummaries reflects the resolved status after patchMeta", () => { + const entry = metaEntryFor("skill", "auth", edits, "t1"); + appendMeta(file, entry); + patchMeta(file, "skill--auth", entry.fingerprint, "reverted", "t2"); + const meta = loadMeta(file); + const prior = priorEditSummaries(meta, "skill", "auth"); + expect(prior.every((s) => s.startsWith("[reverted]"))).toBe(true); + }); +}); + +describe("latestUnresolvedFingerprint", () => { + it("returns the fingerprint of the latest proposed entry", () => { + const e1 = metaEntryFor("sk", "au", edits, "t1"); + const e2 = metaEntryFor("sk", "au", [{ op: "append", content: "x" }], "t2"); + expect(latestUnresolvedFingerprint([e1, e2], "sk", "au")).toBe(e2.fingerprint); + }); + + it("returns null when no entries exist for this skill", () => { + expect(latestUnresolvedFingerprint([], "sk", "au")).toBeNull(); + const other = metaEntryFor("other", "au", edits, "t1"); + expect(latestUnresolvedFingerprint([other], "sk", "au")).toBeNull(); + }); + + it("returns null when the latest entry has already been resolved (applied)", () => { + const entry = metaEntryFor("sk", "au", edits, "t1"); + const patch = { ...entry, ops: [], status: "applied" as const, resolvedAt: "t2" }; + expect(latestUnresolvedFingerprint([entry, patch], "sk", "au")).toBeNull(); + }); + + it("returns null when the latest entry has already been resolved (reverted)", () => { + const entry = metaEntryFor("sk", "au", edits, "t1"); + const patch = { ...entry, ops: [], status: "reverted" as const, resolvedAt: "t2" }; + expect(latestUnresolvedFingerprint([entry, patch], "sk", "au")).toBeNull(); + }); + + it("returns an earlier entry's fingerprint if the most recent one is resolved but an older proposed exists", () => { + const e1 = metaEntryFor("sk", "au", edits, "t1"); + const e2 = metaEntryFor("sk", "au", [{ op: "append", content: "x" }], "t2"); + const patch2 = { ...e2, ops: [], status: "applied" as const, resolvedAt: "t3" }; + // e2 is resolved, e1 is still proposed + expect(latestUnresolvedFingerprint([e1, e2, patch2], "sk", "au")).toBe(e1.fingerprint); + }); +}); + +describe("fingerprintForVersion", () => { + it("returns the fingerprint of the entry that has the given publishedVersion", () => { + const e = metaEntryFor("sk", "au", edits, "t1", 4); + expect(fingerprintForVersion([e], "sk", "au", 4)).toBe(e.fingerprint); + }); + + it("returns null when no entry has the given publishedVersion", () => { + const e = metaEntryFor("sk", "au", edits, "t1"); // no publishedVersion + expect(fingerprintForVersion([e], "sk", "au", 4)).toBeNull(); + expect(fingerprintForVersion([], "sk", "au", 4)).toBeNull(); + }); + + it("returns null for a different skill even if the version matches", () => { + const e = metaEntryFor("other", "au", edits, "t1", 4); + expect(fingerprintForVersion([e], "sk", "au", 4)).toBeNull(); + }); + + it("metaEntryFor stores publishedVersion when provided", () => { + const e = metaEntryFor("sk", "au", edits, "t1", 7); + expect(e.publishedVersion).toBe(7); + const e2 = metaEntryFor("sk", "au", edits, "t1"); // omitted + expect(e2.publishedVersion).toBeUndefined(); + }); + + it("anti-regression: delayed judgment for version N does not mark a later version's edit (cross-version race)", () => { + // Simulate: Process A publishes E1 at v4; Process B starts after and sees E1 in its + // metaCache. Process B's judgment is for v3 (pre-E1 window), so it should resolve + // the v3 edit (which has no publishedVersion β€” nothing was published for v3 in meta) + // and NOT mark E1 as reverted. + const e1 = metaEntryFor("sk", "au", edits, "t1", 4); // E1 published at v4 by Process A + // Process B calls resolveEdit("reverted", priorVersion=3): look up fingerprint for v3 + const fp = fingerprintForVersion([e1], "sk", "au", 3); + expect(fp).toBeNull(); // v3 has no recorded meta entry β†’ no-op, E1 is NOT marked reverted + // E1's fingerprint is NOT returned: + expect(fp).not.toBe(e1.fingerprint); + }); +});