Skip to content

Fix #2370: [Bug] meta.rewardDirty is re-set on episode reopen and never cleared when the ep - #2371

Open
Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.34from
Memtensor-AI:bugfix/autodev-2370-20260915213740784
Open

Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.34from
Memtensor-AI:bugfix/autodev-2370-20260915213740784

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixed a permanent meta.rewardDirty marker on episodes that were terminal-skipped and then reopened (issue #2370).

Root cause: episode-manager.ts reopen() correctly sets rewardDirty: {reason:"episode_reopened",...} on the episode meta, but left meta.reward.skipped: true untouched from the earlier heuristic-skip scoring pass. episodeRewardIsDirty() in memory-core.ts evaluates rewardWasSkipped() before hasRewardDirtyMarker(), so the skip flag permanently short-circuited the function to false, making the episode invisible to every rescore scan path (init recovery loop, periodic dirty-closed scan, backoff filter). The marker was therefore never cleared.

Fix: In episode-manager.ts reopen(), when setting rewardDirty, also spread reward.skipped: undefined into the reward object if the episode had previously been terminal-skipped. This is a targeted one-shot patch to the object written to meta; it does not change any other reward fields. The existing collectDirtyClosedEpisodes bounded-page cleanup already handles any persisted rows that have the contradictory marker pair from before this fix.

Tests: Added two regression tests to tests/unit/session/episode-manager.test.ts: one asserting that reward.skipped is cleared and rewardDirty is set correctly after reopen on a previously-skipped episode, and one asserting that rewardDirty is not set when the episode was never scored. All 176 tests across 13 files pass.

Related Issue (Required): Fixes #2370

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

@whipser030, @hijzy please review this PR.

Reviewer Checklist

…MemTensor#2370)

When an episode is terminal-skipped (meta.reward.skipped=true) and then
reopened, episode-manager.ts correctly sets rewardDirty but leaves
reward.skipped untouched. episodeRewardIsDirty() in memory-core.ts calls
rewardWasSkipped() before hasRewardDirtyMarker(), short-circuiting to false
and making the episode permanently invisible to every rescore scan.

Fix: in reopen(), when setting rewardDirty, also clear reward.skipped so
the marker is evaluated and the episode is queued for rescoring.

The collectDirtyClosedEpisodes bounded-page cleanup already handles existing
persisted rows with the contradictory marker pair; this fix closes the
write-path gap so new reopens don't produce the stale state in the first
place.

Regression test: reopen clears reward.skipped when setting rewardDirty (MemTensor#2370)
@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 15, 2026
@Memtensor-AI

Memtensor-AI commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2371
Task: 88cdc35c69732572
Base: dev-v2.0.34
Head: bugfix/autodev-2370-20260915213740784
Head SHA: 78c784798f20219d89139e5115e66252b80e7bde

🔍 OpenCodeReview found 1 issue(s) in this PR.


1. apps/memos-local-plugin/core/session/episode-manager.ts (L385-L388)

Previous OCR finding is still unresolved. Setting skipped: undefined via object spread does NOT delete the property — the key still exists as an own enumerable property on the resulting object ('skipped' in result === true, Object.hasOwn(result, 'skipped') === true). Any consumer that checks key existence rather than strict truthiness (e.g., if ('skipped' in reward) or reward.hasOwnProperty('skipped')) will still see the flag as present.

This is semantically different from deletion. To truly remove the key, use explicit deletion or filter it out:

const { skipped: _removed, ...rewardWithoutSkipped } = snap.meta.reward as Record<string, unknown>;
reward: rewardWithoutSkipped,

Or via Object.fromEntries:

reward: Object.fromEntries(
  Object.entries(snap.meta.reward as Record<string, unknown>).filter(([k]) => k !== 'skipped')
),

Additionally, whether this fix is effective even at the persistence layer depends on how episodesRepo.reopen() serializes nested objects — if JSON.stringify is applied to the full meta at that point, undefined-valued keys are stripped and the stored JSON will lack the key. But the in-memory snapshot object passed to event bus listeners and other in-process consumers will still carry skipped as an own property, meaning rewardWasSkipped() can still return true for the in-memory object if it uses a key-existence check.

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🔧 Open Code Review requested Agent fix

Open Code Review found 1 issue(s). I have resumed the development Agent to fix them.

  • Task: 88cdc35c69732572
  • Fix attempt: 1/2
  • Finding delta: 0 repeated / 1 new / 0 likely resolved

The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed.

…emTensor#2370)

reopen() re-sets meta.rewardDirty, but EpisodeManager.finalize() and
abandon() passed snap.meta through to the repo without clearing it. Any
terminal path that did not happen to run the reward write — e.g. the
episode resumed with rTask != null, so the reward fallback was skipped —
left the marker on a fully resolved episode. episodeRewardIsDirty() and
consistency checks built on json_type(meta_json,'$.rewardDirty') then
reported the row as dirty indefinitely (reporter saw 17 days), which is
the durable half of MemTensor#2370 that clearing reward.skipped on reopen does
not address on its own.

Overwrite the key with `undefined` rather than dropping it or setting
`null`: the repo merges the patch into the existing meta_json (so an
omitted key would leave the stale value), and toJsonText uses
JSON.stringify, which omits undefined-valued keys — leaving the key
absent instead of present-and-null, which would still satisfy
`IS NOT NULL`.

Regression tests cover both terminal paths and assert on the
JSON-round-tripped meta, i.e. the shape the reporter's watchdog reads.
@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Sep 15, 2026
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:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants