fix: Optimization of the skill accumulation chain for negative examples

[whipser030]

Automated PR from mem-agent-0520-niu to mem-agent-0520.

[Memtensor-AI]

Let me review this PR carefully.

Needs changes (minor) — solid design overall

The repair-candidate concept is well-thought-out: constructive negatives mint unproven skills that must earn trust through stricter trials. The dedup logic, strict/loose trial resolution, and η preservation on rebuild are all sound. A few issues worth addressing:

Bugs / Correctness

1. repair-candidate.ts — list({ limit: 500 }) dedup is fragile

   const already = deps.repos.skills
     .list({ limit: 500 })
     .some((s) => s.status !== "archived" && s.sourcePolicyIds.includes(policy.id));

   If there are >500 skills, the dedup check silently misses later ones and you could mint duplicates. Consider a targeted query (listBySourcePolicyId) or at minimum assert/log when the limit is hit.

2. subscriber.ts — computeStrictOutcome scans feedback rows but deps.repos.feedback isn't declared in SkillSubscriberDeps
   The interface (SkillSubscriberDeps) extends Omit<RunSkillDeps, "log" | "bus">. If feedback isn't on RunSkillDeps, this will compile but fail at runtime when the repo isn't injected. Verify the type covers it, or add feedback explicitly.

3. subscriber.ts — strict trial resolves to "unknown" → trial stays pending forever
   When computeStrictOutcome returns "unknown" (no verifier payload on the episode), the trial is resolved as "unknown". Does applySkillFeedback / lifecycle handle that gracefully, or does the trial count toward trialsAttempted without moving η? If it counts, a repair candidate could exhaust its trial budget on episodes that simply lack verifier data and get archived unfairly. Consider skipping resolution (leaving the trial pending) when strict outcome is unknown.

4. feedback-builder.ts — regex ordering means the broad pattern swallows everything
   CORRECTIVE_CLAUSE_PATTERNS[3] (/\b(?:use|prefer|switch to|apply)\s+(.{4,240})/i) is a superset of pattern [1]. Since you iterate in order and return on first match, pattern 1 is effectively dead code. If the intent is to prefer the narrower "X instead" form, swap the order or merge them.

Migration

5. 013-skill-repair-origin.sql is truncated in the diff — the file ends mid-comment. Confirm the actual migration adds the columns with proper defaults (repair_origin INTEGER DEFAULT 0, strict_trial INTEGER DEFAULT 0) and that the migrator registers version 13.

Minor / Style

6. memory-core.ts — mintRepairCandidate is sync but called inside an async function — not a bug, but the try/catch around it won't catch promise rejections if it ever becomes async. Fine for now, just noting.

7. skill.ts rebuild η logic — the comment says "take the higher of earned vs. recomputed" which makes sense, but if a repair candidate was archived (η dropped to floor) and then a positive feedback triggers a rebuild, Math.max(0.05, recomputed) would still use recomputed. That's correct since repairOrigin is only true for non-archived rows, but worth a unit test to lock in.

8. constructiveFix falls through to classified.constraint — constraints are typically boundaries ("max 100 tokens"), not corrective procedures. Minting a repair skill from a constraint string could produce odd guidance. Consider dropping it from the candidate list or gating on length/content.

What looks good

• The strict vs. loose trial bifurcation is clean and lazy-evaluated.
• η preservation on rebuild (Q4) prevents trust-wipe for validated repairs.
• The substantive() filter is a nice guard against garbage captures.
• Good test coverage for the new module based on the test file list.