Skip to content

fix(intelligence): correct Ebbinghaus decay parameters and lifecycle logic - #995

Merged
lightzt99 merged 8 commits into
oceanbase:mainfrom
jfeng18:fix/ebbinghaus-decay-params
Jun 10, 2026
Merged

fix(intelligence): correct Ebbinghaus decay parameters and lifecycle logic#995
lightzt99 merged 8 commits into
oceanbase:mainfrom
jfeng18:fix/ebbinghaus-decay-params

Conversation

@jfeng18

@jfeng18 jfeng18 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix absurdly short Ebbinghaus half-lives (long_term was 3.3h, now 62d)
  • Remove 7-day hardcoded bypass that overrode algorithmic decay
  • Fix on_get() race: promote before forget (prevents premature soft-deletion)
  • Fix on_search(): search no longer triggers soft-deletion (read-only semantics)

Details

Tier Old half-life New half-life
working 50 min ~1 day
short_term 2.5 hours ~7 days
long_term 3.3 hours ~62 days

Formula unchanged: R = exp(-t / (24 * S)), only default parameters fixed.

Backward compatible: users with explicit INTELLIGENT_MEMORY_DECAY_RATE env var keep their value.

Test plan

  • 4 unit tests updated (test_ebbinghaus_decay_rate.py)
  • Static analysis: no other tests depend on old defaults
  • Mathematical verification of half-lives and forget thresholds
  • CI unit tests pass

🤖 Generated with Claude Code

…logic

The old defaults (decay_rate=0.1, multipliers working=0.5/short_term=1.5/
long_term=2.0) gave absurdly short half-lives: working ~50min, short_term
~2.5h, long_term ~3.3h. This made the entire forgetting curve unusable.

Changes:
- decay_rate default 0.1 → 1.5, multipliers → {1, 7, 60}
  (half-lives: working ~1d, short_term ~7d, long_term ~62d)
- Remove hardcoded 7-day bypass in should_forget() that overrode
  algorithmic decay for high-strength memories
- Fix on_get() ordering: evaluate promote before forget, so a
  promotable memory is not prematurely soft-deleted
- Fix on_search(): never propagate delete_flag — search is a read
  operation and should not mutate memory lifecycle state
- Update tests and documentation to match new defaults

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@CLAassistant

CLAassistant commented Jun 9, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

jfeng18 and others added 3 commits June 9, 2026 23:27
…rch ranking

process_search_results() was replacing the vector/RRF/reranker score with
a naive keyword-overlap ratio (split on spaces + word-in-list). This:
- Completely broke Chinese text (no word boundaries → score always 0)
- Discarded carefully computed hybrid search scores
- Was strictly worse than the storage-layer semantic signal

Now: final_score = original_score * decay_factor * forgotten_multiplier

The storage score (vector similarity, RRF fusion, or reranker output) is
preserved as the relevance signal, with Ebbinghaus time-decay and soft-
forget penalty applied on top.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests that previously relied on calculate_relevance() generating nonzero
base scores now need explicit score fields since the ranking formula uses
the storage-layer score directly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
A memory that was previously soft-forgotten (should_forget=True) carries
a permanent 0.1x search score penalty via forgotten_score_multiplier.
When such a memory is later promoted (accessed and upgraded to a higher
tier), the penalty was never cleared — contradicting the promotion
semantics.

Now: promotion clears should_forget and marked_for_forgetting_at in both
metadata JSON and top-level payload fields.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@wayyoungboy wayyoungboy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one blocking issue in the lifecycle change.

on_get() now checks promotion before forgetting, but EbbinghausAlgorithm.should_promote() still returns True for any memory older than 24 hours. That means an old low-importance working memory whose decay has already fallen below the forget threshold can be promoted to short_term instead of being soft-forgotten.

I verified this on the PR branch with a 50-hour-old working memory:

algo_should_forget=True
algo_should_promote=True
on_get() => delete_flag=False, memory_type=short_term

So the current implementation can convert an expired working memory into a stronger tier purely because it is old. That looks opposite to the intended decay behavior.

Suggested direction: keep the protection against deleting genuinely reinforced memories, but do not let the age-based promotion rule override should_forget(). For example, compute the forget decision first, and only allow an explicit reinforcement signal such as access count or high importance to override it. Please also add a plugin-level regression test for an old low-importance working memory that should still be soft-forgotten on get().

The storage-score ranking change looks reasonable, and the existing focused unit tests pass locally:

PYTHONPATH=src python3.11 -m pytest tests/unit/intelligence/test_ebbinghaus_decay_rate.py tests/unit/intelligence/test_ebbinghaus_search_metadata.py -q
21 passed

should_promote() returned True for any memory older than 24h regardless
of access_count. This allowed expired working memories (decay < 0.3) to
be promoted instead of forgotten when on_get checked promote first.

Now: the age>24h condition requires access_count > 0. Memories that were
never accessed can still be forgotten via the decay path. Memories that
were accessed at least once get the age-based promotion as before.

Adds regression test: 50h-old working memory with 0 accesses must be
soft-forgotten, not promoted.

Addresses review feedback from @wayyoungboy on PR oceanbase#995.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@jfeng18 jfeng18 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough review @wayyoungboy! You're right — the age-only promotion was letting expired working memories escape the forget path.

Fixed in 0486e5c: should_promote() now requires access_count > 0 for the age>24h condition to fire. A memory that was never accessed can no longer be promoted purely by aging.

Also added a regression test (test_old_unaccessed_working_memory_is_forgotten_not_promoted) that verifies a 50h-old working memory with 0 accesses gets delete_flag=True from on_get().

The other two promotion paths remain unchanged:

  • access_count >= 3 → promoted (genuine reinforcement)
  • importance >= 0.6 → promoted (high-value content)

Please take another look when you get a chance.

@wayyoungboy

Copy link
Copy Markdown
Member

Re-tested the latest head (0486e5c) locally with the project virtualenv.

source .venv/bin/activate
PYTHONPATH=src python -m pytest tests/unit -q
# 355 passed

PYTHONPATH=src python -m pytest tests/unit/intelligence/test_ebbinghaus_decay_rate.py tests/unit/intelligence/test_ebbinghaus_search_metadata.py -q
# 22 passed

PYTHONPATH=src python -m pytest tests/unit/intelligence tests/unit/test_postgres.py -q
# 92 passed

The previous age-only promotion issue is fixed now. However, I still think this PR should not merge yet because the search-ranking change exposes a backend score semantics bug for PGVector.

process_search_results() now treats score as similarity where larger is better, but PGVector.search() returns vector <=> query AS distance, where smaller is better. With intelligent memory enabled, PGVector results can be reordered in the wrong direction.

Minimal repro output:

[('far', 0.9, 0.875344), ('near', 0.1, 0.09726)]

Here near has distance 0.1 and should rank before far, but it is ranked after far.

Suggested fix: normalize PGVector's returned score to similarity before it reaches process_search_results(), or add an explicit score-kind/normalization layer so intelligent ranking only multiplies comparable similarity scores. Please also add a regression test for this distance-vs-similarity case.

pgvector <=> returns cosine distance [0, 2] where lower = more similar.
process_search_results() expects similarity [0, 1] where higher = better.
Without conversion, intelligence-enabled ranking inverts pgvector results.

Convert with `max(1 - distance/2, 0)` matching OceanBase's formula for
cross-backend consistency.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jfeng18

jfeng18 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Good catch @wayyoungboy — the pgvector distance-vs-similarity issue is real.

Fixed in d40ce51: PGVectorStore.search() now converts cosine distance to similarity with max(1 - distance/2, 0), matching OceanBase's formula. Updated both psycopg2/psycopg3 search tests to assert on the converted similarity values (distance=0.1 → score=0.95, distance=0.2 → score=0.9).

This ensures process_search_results() receives comparable similarity scores regardless of backend.

@wayyoungboy

Copy link
Copy Markdown
Member

Re-tested the latest head (d40ce51) locally.

The Ebbinghaus lifecycle changes look good now:

source .venv/bin/activate
PYTHONPATH=src python -m pytest tests/unit/intelligence -q
# 42 passed

The remaining blocker I see is only in the PGVector unit tests. The implementation now correctly converts cosine distance to similarity, but six existing assertions in tests/unit/test_postgres.py still expect the old raw distance values:

0.1 -> now 0.95
0.2 -> now 0.9

Current local result:

PYTHONPATH=src python -m pytest tests/unit/intelligence/test_ebbinghaus_decay_rate.py tests/unit/intelligence/test_ebbinghaus_search_metadata.py tests/unit/test_postgres.py -q
# 6 failed, 66 passed

Failing tests:

  • test_search_with_filters_psycopg2
  • test_search_with_filters_psycopg3
  • test_search_with_single_filter_psycopg2
  • test_search_with_single_filter_psycopg3
  • test_search_with_no_filters_psycopg2
  • test_search_with_no_filters_psycopg3

I think this just needs the remaining assertions updated from 0.1/0.2 to 0.95/0.9 using assertAlmostEqual, matching the new score semantics. I do not see another implementation-level blocker at this point.

…conversion

Six filter-related pgvector tests still asserted on raw cosine distance
values (0.1/0.2). Updated to match the converted similarity scores
(0.95/0.9) using assertAlmostEqual.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jfeng18

jfeng18 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @wayyoungboy — updated the remaining 6 filter-related pgvector test assertions in 706203e. All should use assertAlmostEqual with converted similarity values now.

@wayyoungboy wayyoungboy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed latest head 706203e.

The previously reported issues are resolved:

  • age-only promotion no longer allows old never-accessed working memories to bypass the forget path;
  • PGVector now converts cosine distance to similarity before intelligent ranking;
  • the remaining PGVector unit assertions were updated to the new similarity values.

Local verification:

PYTHONPATH=src python -m pytest tests/unit/intelligence tests/unit/test_postgres.py -q
# 92 passed

PYTHONPATH=src python -m pytest tests/unit -q
# 355 passed, 1 warning

Approved from the implementation review side.

@wayyoungboy

Copy link
Copy Markdown
Member

The frontend build failure is caused by MDX parsing in docs/guides/0003-configuration.md.

The failing line is the decay-rate description:

Memory decay strength (S in R=e^{-t/24S}). Larger values mean slower decay

MDX treats {...} as a JSX expression, so e^{-t/24S} fails to parse. Wrapping the formula in inline code should fix it:

Memory decay strength (S in `R=e^{-t/24S}`). Larger values mean slower decay

After that, rerun the frontend build check.

`e^{-t/24S}` was parsed as a JSX expression by MDX, breaking the
frontend build. Wrapped in inline code backticks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jfeng18

jfeng18 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @wayyoungboy — fixed in 690c9a6. Wrapped the formula in inline code backticks to avoid MDX JSX parsing.

@lightzt99
lightzt99 merged commit 2c511be into oceanbase:main Jun 10, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

should_promote() promotes never-accessed memories based solely on age

4 participants