Skip to content

Fix #2362: [Bug] attachSkillSubscriber wired with bgLlm, not bgReflectLlm — skillEvolver de - #2365

Open
Memtensor-AI wants to merge 1 commit into
MemTensor:dev-v2.0.34from
Memtensor-AI:bugfix/autodev-2362-20260914203903343
Open

Memtensor-AI wants to merge 1 commit into
MemTensor:dev-v2.0.34from
Memtensor-AI:bugfix/autodev-2362-20260914203903343

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixes #2362attachSkillSubscriber in apps/memos-local-plugin/core/pipeline/deps.ts was wired with llm: bgLlm (the main model) instead of bgReflectLlm (the dedicated client built from the skillEvolver.* config block). As a result, operators configuring a distinct skillEvolver.provider / skillEvolver.model were silently ignored — skill crystallization and evolution ran on the main model regardless — and overview.skillEvolver.lastOkAt was pinned to null because the dedicated client was never invoked. This is a distinct instance of the wiring-bug class fixed in #2148 (opposite direction: that fix corrected captureRunner's slot; this one corrects the skill subscriber's slot). Verified via git log -L in the issue: this call site has never been wired to bgReflectLlm since the v2.0 Reflect2Evolve rewrite.

Change is a single line at deps.ts:339: llm: bgReflectLlm ?? bgLlm. The ?? fallback preserves existing behaviour for installs without a distinct skillEvolver config block.

Guarded by a new dedicated regression test apps/memos-local-plugin/tests/unit/pipeline/skill-reflect-llm-wiring.test.ts (2 cases) mirroring the pattern of capture-reflect-llm-wiring.test.ts (the sibling #2148 guard). RED was reproduced before the fix (expected 'main-llm' to be 'skill-evolver-llm'); GREEN confirmed after. Full pipeline + skill-subscriber suite: 14 files / 92 tests pass, including the sibling capture-reflect-llm-wiring and health-model-display neighbours. npm run lint (tsc --noEmit) clean.

Path classification: Bug quick-fix — one-line DI correction, no openspec artifacts required. Task file archived to sibling specs repo (memos-autodev-specs / main, commit ff25a7f). Reviewer: @WeiminLee.

Related Issue (Required): Fixes #2362

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • Documentation update

How Has This Been Tested?

Not run; documentation-only change.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)

Checklist

  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have created related documentation issue/PR in MemOS-Docs (if applicable)
  • I have linked the issue to this PR (if applicable)
  • I have mentioned the person who will review this PR

@WeiminLee please review this PR.

Reviewer Checklist

`attachSkillSubscriber` in core/pipeline/deps.ts was wired with `llm: bgLlm`
(the main model), never `bgReflectLlm` (the dedicated client built from the
`skillEvolver.*` config block). As a result:

- Operators configuring a distinct `skillEvolver.provider` / `skillEvolver.model`
  were silently ignored — skill crystallization / evolution ran on the main
  model regardless.
- `overview.skillEvolver.lastOkAt` was pinned to `null` because the dedicated
  client was never invoked, making it useless as a liveness signal.

Change: `llm: bgReflectLlm ?? bgLlm`. Fallback preserves current behaviour
for installs without a distinct `skillEvolver` block.

Guarded by a new dedicated regression test
(`tests/unit/pipeline/skill-reflect-llm-wiring.test.ts`, 2 cases) mirroring
the pattern of `capture-reflect-llm-wiring.test.ts` (the sibling fix MemTensor#2148,
opposite-direction wiring bug in captureRunner).

Fixes MemTensor#2362
@Memtensor-AI Memtensor-AI added ai:generated Generated or modified by AI | 由 AI 生成或修改 area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Sep 14, 2026
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2365
Task: 864b1ca0f6c45de2
Base: dev-v2.0.34
Head: bugfix/autodev-2362-20260914203903343
Head SHA: d5b3beac98e6b4a1c343777be7e22f82d231604c

OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

✅ Automated Test Results: PASSED

All tests passed (74/74 executed, 5 skipped). memos_github_open_source/smoke: 1/1, memos_local_plugin/unit: 2/2, memos_python_core/changed-repo-python: 71 passed, 5 skipped. Duration: 21s

Branch: bugfix/autodev-2362-20260914203903343

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:generated Generated or modified by AI | 由 AI 生成或修改 area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants