From 6720e009506c93047a2eec922afd37fcd58d6dc5 Mon Sep 17 00:00:00 2001 From: LamzQ Date: Wed, 2 Sep 2026 09:50:22 +0800 Subject: [PATCH 1/3] fix(skill): back off crystallization retries after repeated failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A crystallize or verify failure leaves the policy untouched, so every skill tick retried the same policy forever. The 2026-08-28 audit found 2,640 repeated failures over 25 days — the bulk of wasted skill invocations. Count consecutive failures per policy in kv. After 3 consecutive failures the policy skill_eligible flag is turned off and the eligibility gate skips it with a dedicated reason that distinguishes backoff trips from manual toggles. A successful crystallization clears the counter, and the new setSkillEligible repo method deliberately leaves updated_at untouched so the rebuild heuristic for existing skills is not triggered as a side effect. The llm-disabled skip reason is a global configuration state, not a policy failure, and never counts toward the backoff. --- .../core/skill/eligibility.ts | 13 +- apps/memos-local-plugin/core/skill/skill.ts | 32 +++++ .../core/storage/repos/policies.ts | 13 ++ .../tests/unit/skill/backoff.test.ts | 132 ++++++++++++++++++ 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 apps/memos-local-plugin/tests/unit/skill/backoff.test.ts diff --git a/apps/memos-local-plugin/core/skill/eligibility.ts b/apps/memos-local-plugin/core/skill/eligibility.ts index 4e54ba5db..b4bdfd551 100644 --- a/apps/memos-local-plugin/core/skill/eligibility.ts +++ b/apps/memos-local-plugin/core/skill/eligibility.ts @@ -66,6 +66,18 @@ function decide( existing: SkillRow | null, cfg: SkillConfig, ): EligibilityDecision { + // WHY: skill_eligible=false can mean either the crystallize-failure + // backoff (3 consecutive failures) or a manual toggle. Reporting it as + // its own skip reason keeps the two sources distinguishable from the + // experience-type success-anchor check below. + if (policy.skillEligible === false) { + return { + policy, + existingSkill: existing, + action: "skip", + reason: "policy.skillEligible=false (backoff or manual)", + }; + } if (policy.status !== "active") { return { policy, @@ -125,7 +137,6 @@ function decide( } function hasSuccessAnchor(policy: PolicyRow): boolean { - if (policy.skillEligible === false) return false; const type = policy.experienceType ?? "success_pattern"; if (type === "failure_avoidance" || type === "repair_instruction" || type === "preference") { return false; diff --git a/apps/memos-local-plugin/core/skill/skill.ts b/apps/memos-local-plugin/core/skill/skill.ts index d0cdc48ae..fc900776f 100644 --- a/apps/memos-local-plugin/core/skill/skill.ts +++ b/apps/memos-local-plugin/core/skill/skill.ts @@ -28,6 +28,7 @@ import type { Repos } from "../storage/repos/index.js"; import { now as nowMs } from "../time.js"; import { ids } from "../id.js"; import type { + PolicyId, PolicyRow, SkillId, SkillRow, @@ -154,6 +155,12 @@ export async function runSkill( reason: crystResult.skippedReason, modelRefusal: crystResult.modelRefusal, }); + // WHY: "llm-disabled" is a global configuration state, not a failure of + // this policy. Counting it would trip the whole candidate pool while + // the LLM is merely switched off — with no recovery path back. + if (crystResult.skippedReason !== "llm-disabled") { + bumpFailureBackoff(deps, decision.policy.id); + } continue; } @@ -176,6 +183,7 @@ export async function runSkill( skillId: "sk_placeholder" as SkillId, reason: verdict.reason ?? "verify-failed", }); + bumpFailureBackoff(deps, decision.policy.id); continue; } @@ -225,6 +233,10 @@ export async function runSkill( } timings.persist += nowMs() - tPersist; + // A successful crystallization resets the failure counter so later + // failures start counting from zero again. + deps.repos.kv.del(failCountKey(decision.policy.id)); + if (decision.action === "rebuild") rebuilt += 1; else crystallized += 1; @@ -338,6 +350,26 @@ export function applySkillFeedback( // ─── Helpers ────────────────────────────────────────────────────────────── +// WHY: a crystallize/verify failure leaves the policy untouched, so every +// trigger retries it forever (2026-08-28 audit: 2640 repeated failures over +// 25 days, the bulk of wasted skill invocations). Count consecutive failures +// in kv; after 3, flip skill_eligible=false so the policy is skipped. A +// successful crystallization clears the counter. +const SKILL_FAILURE_BACKOFF_LIMIT = 3; + +function failCountKey(policyId: string): string { + return `skill.failCount:${policyId}`; +} + +function bumpFailureBackoff(deps: RunSkillDeps, policyId: PolicyId): void { + const count = deps.repos.kv.get(failCountKey(policyId), 0) + 1; + deps.repos.kv.set(failCountKey(policyId), count); + if (count >= SKILL_FAILURE_BACKOFF_LIMIT) { + deps.repos.policies.setSkillEligible(policyId, false); + deps.log.warn("skill.backoff.exhausted", { policyId, count }); + } +} + function gatherPolicies(input: RunSkillInput, repos: Repos): PolicyRow[] { if (input.policyId) { const single = repos.policies.getById(input.policyId); diff --git a/apps/memos-local-plugin/core/storage/repos/policies.ts b/apps/memos-local-plugin/core/storage/repos/policies.ts index 29920f60a..383f0832c 100644 --- a/apps/memos-local-plugin/core/storage/repos/policies.ts +++ b/apps/memos-local-plugin/core/storage/repos/policies.ts @@ -73,6 +73,12 @@ export function makePoliciesRepo(db: StorageDb) { columns: ["id", "support", "gain", "status", "updated_at"], }), ); + // WHY: the skill-crystallize backoff flips eligibility for a single policy. + // updated_at is deliberately left untouched so the rebuild heuristic for + // existing skills is not triggered as a side effect. + const setSkillEligibleStmt = db.prepare<{ id: string; eligible: number }>( + `UPDATE policies SET skill_eligible=@eligible WHERE id=@id`, + ); const selectById = db.prepare<{ id: string }, RawPolicyRow>( `SELECT ${COLUMNS.join(", ")} FROM policies WHERE id=@id`, ); @@ -104,6 +110,13 @@ export function makePoliciesRepo(db: StorageDb) { }); }, + // WHY: the skill-crystallize backoff needs to close crystallization + // eligibility for one policy without bumping updated_at (which would + // trigger the rebuild heuristic for an existing skill). + setSkillEligible(id: PolicyId, eligible: boolean): void { + setSkillEligibleStmt.run({ id, eligible: eligible ? 1 : 0 }); + }, + getById(id: PolicyId): PolicyRow | null { const r = selectById.get({ id }); if (!r) return null; diff --git a/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts b/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts new file mode 100644 index 000000000..d9f1ae0e4 --- /dev/null +++ b/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts @@ -0,0 +1,132 @@ +import { describe, it, expect, afterEach } from "vitest"; + +import { rootLogger } from "../../../core/logger/index.js"; +import { runSkill, type RunSkillDeps } from "../../../core/skill/index.js"; +import { fakeLlm } from "../../helpers/fake-llm.js"; +import { makeTmpDb, type TmpDbHandle } from "../../helpers/tmp-db.js"; +import type { EpisodeId, PolicyId } from "../../../core/types.js"; +import { makeDraft, makeSkillConfig, seedPolicy, seedSessionOnly, seedTrace } from "./_helpers.js"; + +let handle: TmpDbHandle | null = null; + +function open(): TmpDbHandle { + handle = makeTmpDb(); + return handle; +} + +afterEach(() => { + handle?.cleanup(); + handle = null; +}); + +function makeDeps(h: TmpDbHandle, llm: RunSkillDeps["llm"]): RunSkillDeps { + return { + repos: h.repos, + embedder: null, + llm, + log: rootLogger.child({ channel: "core.skill.backoff-test" }), + bus: { emit: () => {} } as unknown as RunSkillDeps["bus"], + config: makeSkillConfig(), + }; +} + +// Scripted model refusal — crystallize returns { ok:false, skippedReason:"llm-refusal" }. +// (We deliberately do NOT drive failures with llm=null: that path returns +// "llm-disabled", a global config state the backoff must not count.) +function refusingLlm(): RunSkillDeps["llm"] { + // NB: the refusal detector scans the raw JSON string, so a plain-string + // response is what actually trips it (an object response starts with "{"). + return fakeLlm({ completeJson: { "skill.crystallize": "I cannot assist with this request." } }); +} + +function seedCandidate(h: TmpDbHandle): PolicyId { + const sessionId = "s_backoff"; + seedSessionOnly(h, sessionId); + const episodeId = "ep_backoff" as EpisodeId; + seedTrace(h, { + episodeId, + sessionId, + userText: "pip install cryptography failing", + agentText: "apk add openssl-dev libffi-dev, retry pip install", + reflection: "install system libs before pip", + value: 0.9, + }); + seedTrace(h, { + episodeId, + sessionId, + userText: "cryptography install retry", + agentText: "apk add openssl-dev && pip install cryptography", + reflection: "musl wheels need system libs", + value: 0.8, + }); + return seedPolicy(h, { sourceEpisodeIds: [episodeId] }).id; +} + +function failCount(h: TmpDbHandle, policyId: PolicyId): number { + return h.repos.kv.get(`skill.failCount:${policyId}`, 0); +} + +describe("skill crystallize failure backoff", () => { + it("flips skill_eligible off after SKILL_FAILURE_BACKOFF_LIMIT consecutive failures", async () => { + const h = open(); + const policyId = seedCandidate(h); + const deps = makeDeps(h, refusingLlm()); + + for (let i = 1; i <= 2; i++) { + const r = await runSkill({ trigger: "manual", policyId }, deps); + expect(r.crystallized).toBe(0); + expect(failCount(h, policyId)).toBe(i); + // below the limit the policy stays eligible + expect(h.repos.policies.getById(policyId)!.skillEligible).not.toBe(false); + } + + await runSkill({ trigger: "manual", policyId }, deps); + expect(failCount(h, policyId)).toBe(3); + expect(h.repos.policies.getById(policyId)!.skillEligible).toBe(false); + }); + + it("reports the tripped backoff as its own skip reason on later runs", async () => { + const h = open(); + const policyId = seedCandidate(h); + const deps = makeDeps(h, refusingLlm()); + for (let i = 0; i < 3; i++) await runSkill({ trigger: "manual", policyId }, deps); + + const r = await runSkill({ trigger: "manual", policyId }, deps); + // the eligibility gate now skips before crystallize — the run counts + // the policy as skipped (not evaluated) and the failure counter + // stays frozen at 3 + expect(r.evaluated).toBe(0); + expect(r.crystallized).toBe(0); + expect(failCount(h, policyId)).toBe(3); + expect(h.repos.policies.getById(policyId)!.skillEligible).toBe(false); + }); + + it("clears the counter after a successful crystallization", async () => { + const h = open(); + const policyId = seedCandidate(h); + + await runSkill({ trigger: "manual", policyId }, makeDeps(h, refusingLlm())); + expect(failCount(h, policyId)).toBe(1); + + await runSkill({ trigger: "manual", policyId }, makeDeps(h, fakeLlm({ completeJson: { "skill.crystallize": makeDraft() } }))); + expect(failCount(h, policyId)).toBe(0); + // (fewer-than-limit failures not tripping the backoff is covered by + // the second run of the first test above) + }); + + it("does not count llm-disabled toward the backoff", async () => { + const h = open(); + const policyId = seedCandidate(h); + // llm=null => crystallize returns { ok:false, skippedReason:"llm-disabled" }: + // a global configuration state. Even after many such ticks the policy + // must stay eligible and the counter untouched. + const deps = makeDeps(h, null); + + for (let i = 0; i < 4; i++) { + const r = await runSkill({ trigger: "manual", policyId }, deps); + expect(r.crystallized).toBe(0); + } + expect(failCount(h, policyId)).toBe(0); + expect(h.repos.policies.getById(policyId)!.skillEligible).not.toBe(false); + }); +}); From 3d2fe4865e4d64c884ad49249bcc9662c864fd61 Mon Sep 17 00:00:00 2001 From: LamzQ Date: Wed, 2 Sep 2026 09:58:32 +0800 Subject: [PATCH 2/3] fix(skill): clear the backoff counter on trip so a manual re-enable gets a fresh window Co-Authored-By: LamzQ --- apps/memos-local-plugin/core/skill/skill.ts | 3 +++ .../tests/unit/skill/backoff.test.ts | 22 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/apps/memos-local-plugin/core/skill/skill.ts b/apps/memos-local-plugin/core/skill/skill.ts index fc900776f..ff691a56e 100644 --- a/apps/memos-local-plugin/core/skill/skill.ts +++ b/apps/memos-local-plugin/core/skill/skill.ts @@ -366,6 +366,9 @@ function bumpFailureBackoff(deps: RunSkillDeps, policyId: PolicyId): void { deps.repos.kv.set(failCountKey(policyId), count); if (count >= SKILL_FAILURE_BACKOFF_LIMIT) { deps.repos.policies.setSkillEligible(policyId, false); + // Clear the counter on trip: a later manual re-enable must get a fresh + // window, not instant re-trip on the next single failure. + deps.repos.kv.del(failCountKey(policyId)); deps.log.warn("skill.backoff.exhausted", { policyId, count }); } } diff --git a/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts b/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts index d9f1ae0e4..3a0d7d8e5 100644 --- a/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts +++ b/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts @@ -81,8 +81,25 @@ describe("skill crystallize failure backoff", () => { } await runSkill({ trigger: "manual", policyId }, deps); - expect(failCount(h, policyId)).toBe(3); + // the counter is cleared on trip — the re-enable path gets a fresh window + expect(failCount(h, policyId)).toBe(0); + expect(h.repos.policies.getById(policyId)!.skillEligible).toBe(false); + }); + + it("gives a manually re-enabled policy a fresh backoff window", async () => { + const h = open(); + const policyId = seedCandidate(h); + const deps = makeDeps(h, refusingLlm()); + for (let i = 0; i < 3; i++) await runSkill({ trigger: "manual", policyId }, deps); expect(h.repos.policies.getById(policyId)!.skillEligible).toBe(false); + expect(failCount(h, policyId)).toBe(0); + + // admin re-enables the policy: the next failure must count from 0, + // not instantly re-trip off the stale counter + h.repos.policies.setSkillEligible(policyId, true); + await runSkill({ trigger: "manual", policyId }, deps); + expect(h.repos.policies.getById(policyId)!.skillEligible).not.toBe(false); + expect(failCount(h, policyId)).toBe(1); }); it("reports the tripped backoff as its own skip reason on later runs", async () => { @@ -97,7 +114,8 @@ describe("skill crystallize failure backoff", () => { // stays frozen at 3 expect(r.evaluated).toBe(0); expect(r.crystallized).toBe(0); - expect(failCount(h, policyId)).toBe(3); + // the counter was cleared when the backoff tripped + expect(failCount(h, policyId)).toBe(0); expect(h.repos.policies.getById(policyId)!.skillEligible).toBe(false); }); From 1d40b887c17054d94dec44c039f75ab004413e73 Mon Sep 17 00:00:00 2001 From: LamzQ Date: Wed, 2 Sep 2026 10:37:33 +0800 Subject: [PATCH 3/3] =?UTF-8?q?test(skill):=20fix=20stale=20comment=20?= =?UTF-8?q?=E2=80=94=20counter=20is=20cleared=20on=20trip,=20not=20frozen?= =?UTF-8?q?=20at=203?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: LamzQ --- apps/memos-local-plugin/tests/unit/skill/backoff.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts b/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts index 3a0d7d8e5..20bd5df18 100644 --- a/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts +++ b/apps/memos-local-plugin/tests/unit/skill/backoff.test.ts @@ -111,7 +111,7 @@ describe("skill crystallize failure backoff", () => { const r = await runSkill({ trigger: "manual", policyId }, deps); // the eligibility gate now skips before crystallize — the run counts // the policy as skipped (not evaluated) and the failure counter - // stays frozen at 3 + // stays cleared at 0 (cleared when the backoff tripped) expect(r.evaluated).toBe(0); expect(r.crystallized).toBe(0); // the counter was cleared when the backoff tripped