Conversation
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.
Memtensor-AI
added
area:plugin
🤖 Open Code ReviewTarget: PR #2326 🔍 OpenCodeReview found 1 issue(s) in this PR. 1.
|
…ets a fresh window Co-Authored-By: LamzQ <linxlam@foxmail.com>
|
Thanks — confirmed and fixed in 3d2fe48: the counter is now cleared when the backoff trips, so a manual re-enable starts a fresh window instead of instantly re-tripping off the stale count. Locked by a new test (gives a manually re-enabled policy a fresh backoff window) plus two assertion updates reflecting the cleared-on-trip state. skill domain: 74 passed (12 files), tsc clean. |
…en at 3 Co-Authored-By: LamzQ <linxlam@foxmail.com>
|
|
Thanks for this. It fixes a real waste problem, and I'm running it locally on top of v2.0.20 (as a Hermes Agent memory provider against a self-hosted vLLM endpoint). I found one issue while reviewing it: transport-level failures count toward the backoff. Suggested fix: classify by the existing typed error codes and skip the bump for transient failures. Content failures (refusals, unusable drafts, verify failures) still count. // crystallize.ts
const TRANSIENT_LLM_CODES: ReadonlySet<string> = new Set([
ERROR_CODES.LLM_TIMEOUT,
ERROR_CODES.LLM_UNAVAILABLE,
ERROR_CODES.LLM_RATE_LIMITED,
]);
const TRANSIENT_NETWORK_RE =
/\b(ECONNREFUSED|ECONNRESET|ETIMEDOUT|EAI_AGAIN|ENOTFOUND|EPIPE|UND_ERR_[A-Z_]+)\b|fetch failed|socket hang up|network error/i;
export function isTransientLlmError(err: unknown): boolean {
if (err instanceof MemosError) return TRANSIENT_LLM_CODES.has(err.code);
const message = err instanceof Error ? err.message : String(err);
return TRANSIENT_NETWORK_RE.test(message);
}
// …both `llm-failed` returns gain: transient: isTransientLlmError(err)
// …and CrystallizeResult's failure arm gains: transient?: boolean
// skill.ts
if (crystResult.skippedReason !== "llm-disabled" && !crystResult.transient) {
bumpFailureBackoff(deps, decision.policy.id);
}Tests added to
Both transient tests fail without the |
|
Thanks @zorrobyte — great catch, and thanks for verifying it against a real deployment. You're right: transport-level failures (timeout, connection refused, rate limiting) say nothing about the quality of the policy itself, so letting them trip the backoff can retire good policies with no recovery path. That was never the intent — the counter should only track content-level failures (refusals, unusable drafts, verification failures). Please go ahead with a follow-up PR against this branch ( |
4 participants
Summary
A crystallize or verify failure leaves the policy untouched, so every skill
tick retried the same policy forever. On one long-running production install,
the 2026-08-28 audit counted 2,640 repeated crystallize failures over
25 days — the bulk of wasted skill invocations. This PR bounds that retry
cost: after 3 consecutive failures a policy is taken off the crystallization
path; at any earlier point a successful crystallization clears the counter
and failures start counting from zero again.
Problem
runSkilltreats both failure modes (crystallizer skip and verifierrejection) as advisory: it logs a warning and moves on, but the policy's
skill_eligibleflag staystrue. The next trigger re-selects the samepolicy and the same failure repeats — indefinitely for policies whose failure
cause is permanent (e.g. evidence that can never satisfy the verifier).
Change
core/skill/skill.ts— count consecutive failures per policy in kv(
skill.failCount:<id>); both failure paths callbumpFailureBackoff()before
continue, and a successful crystallization clears the counter.After
SKILL_FAILURE_BACKOFF_LIMIT(3) consecutive failures the policy'sskill_eligibleflag is turned off. The one deliberate exception is"llm-disabled": that skip reason is a global configuration state, not afailure of the policy, so it never counts (otherwise switching the LLM off
would trip the whole candidate pool with no recovery path).
core/skill/eligibility.ts—decide()now reportsskill_eligible=falseas its own skip reason (
"policy.skillEligible=false (backoff or manual)"),so a tripped backoff stays distinguishable from a manual toggle; the hidden
check inside
hasSuccessAnchor()is removed so the two sources don't blur.core/storage/repos/policies.ts— newsetSkillEligible()repo method. Itdeliberately leaves
updated_atuntouched: bumping it would flip therebuild heuristic for an existing skill as a side effect.
tests/unit/skill/backoff.test.ts— new regression suite (5 tests): tripafter the 3rd consecutive failure and not before, tripped policies are
skipped by the eligibility gate with the counter cleared on trip, a
successful crystallization clears the counter, a manually re-enabled
policy gets a fresh backoff window, and
llm-disabledticks never count.The counter is cleared when the backoff trips, so re-enabling the
skill_eligibleflag gives the policy a fresh 3-failure window.Tests
npx vitest run tests/unit/skill→ 74 passed (12 files) (69pre-existing + 5 new)
npx tsc --noEmit→ clean (exit 0)Related
failures (validator rejections of valid-but-incomplete drafts); this PR
bounds the retry cost of the failures that remain (LLM refusal, verifier
mismatches, permanently unqualifying evidence). Fixes fix: skill crystallization retries the same failing policies forever #2319.
Type of change
How Has This Been Tested?
npx vitest run tests/unit/skill— 74 passed)tsc --noEmitclean)Checklist