Skip to content

fix(types): remove duplicate source field in RetentionScore (closes #277)#326

Merged
rohitg00 merged 1 commit into
mainfrom
fix/277-duplicate-source-retentionscore
May 13, 2026
Merged

fix(types): remove duplicate source field in RetentionScore (closes #277)#326
rohitg00 merged 1 commit into
mainfrom
fix/277-duplicate-source-retentionscore

Conversation

@rohitg00
Copy link
Copy Markdown
Owner

@rohitg00 rohitg00 commented May 13, 2026

Verified bug

src/types.ts:835 and :842 both declared source?: "episodic" | "semantic" on RetentionScore. TypeScript silently accepts duplicate property declarations when types are identical — but the documented JSDoc on the first declaration (referencing #124 / pre-0.8.10 row back-compat) was effectively shadowed.

// before
export interface RetentionScore {
  memoryId: string;
  // ... see #124 back-compat note ...
  source?: "episodic" | "semantic";
  score: number;
  salience: number;
  temporalDecay: number;
  reinforcementBoost: number;
  lastAccessed: string;
  accessCount: number;
  source?: "episodic" | "semantic";  // ← dup
}

Verified safe to remove

grep -rn "\.source\s*=\|source:" src/ | grep -i retention returned zero writers setting source on RetentionScore. No path could plausibly write to both shadowed declarations. Single source of truth restored, JSDoc back-compat note preserved.

Test plan

  • npm test877 / 877.
  • npm run build — tsdown clean.

Closes #277. Thanks @cl0ckt0wer for the precise file:line trace.

Summary by CodeRabbit

  • Chores
    • Updated retention tracking data structure to support enhanced metrics including reinforcement scoring, access timing, and usage frequency monitoring.

Review Change Stack

)

cl0ckt0wer reported in #277 that RetentionScore declared source twice
(src/types.ts:835 and :842), both as ?"episodic" | "semantic". TS
silently accepts duplicate property declarations when types are
identical; runtime treats the second as the canonical declaration,
which means the documented JSDoc on the FIRST one (referring to #124,
pre-0.8.10 rows, scope probing semantics) was effectively dead code
to anyone reading the type definition.

Verified across src/ that no code path WRITES to source twice: grep
returned zero callers setting source on RetentionScore, so removing
the duplicate is a no-op at runtime. The kept copy retains the
#124-referencing JSDoc so callers still see the back-compat note.

Test impact: none (type-only). Full suite 877/877 unchanged.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agentmemory Ready Ready Preview, Comment May 13, 2026 11:00am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cfaa7991-866e-44b1-806f-5e6537293e46

📥 Commits

Reviewing files that changed from the base of the PR and between 25dddc4 and 9419fcf.

📒 Files selected for processing (1)
  • src/types.ts
💤 Files with no reviewable changes (1)
  • src/types.ts

📝 Walkthrough

Walkthrough

The RetentionScore interface in src/types.ts is extended with three new fields: reinforcementBoost (number), lastAccessed (string), and accessCount (number) to track additional retention-related metrics alongside existing properties.

Changes

RetentionScore Type Extension

Layer / File(s) Summary
Add retention metrics to RetentionScore
src/types.ts
RetentionScore interface extended with reinforcementBoost, lastAccessed, and accessCount fields to capture reinforcement boost, last access timestamp, and cumulative access count.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • rohitg00/agentmemory#132: Both PRs modify the RetentionScore type in src/types.ts, extending it with different retention-related properties and their usage in eviction logic.

Poem

🐰 Three metrics hop into the Score today,
Boost and access time come to play,
Retention strengthens with each new field,
Memory's garden richly tilled! 📊✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR adds new fields (reinforcementBoost, lastAccessed, accessCount) to RetentionScore but the issue #277 only requires removing the duplicate source field; the new fields appear unrelated to the stated objective. Verify whether the new fields (reinforcementBoost, lastAccessed, accessCount) were intentionally added to address #277 requirements, or remove them if they belong in a separate PR.
Out of Scope Changes check ⚠️ Warning The addition of three new fields (reinforcementBoost, lastAccessed, accessCount) to RetentionScore appears out of scope, as issue #277 specifically requests only the removal of the duplicate source field. Clarify whether the three new fields are intentional for this PR or should be moved to a separate feature branch and PR.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing a duplicate source field in RetentionScore, and correctly references the closed issue #277.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/277-duplicate-source-retentionscore

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rohitg00 rohitg00 merged commit cba234d into main May 13, 2026
5 checks passed
@rohitg00 rohitg00 deleted the fix/277-duplicate-source-retentionscore branch May 13, 2026 11:35
rohitg00 added a commit that referenced this pull request May 13, 2026
…ardening

Three landed PRs since v0.9.11:
  - #327 (#295) — BM25 tokenizer now accepts non-ASCII (Greek,
    accented Latin, Hebrew, Arabic, Cyrillic), VectorIndex.add now
    actually called at runtime via vectorIndexAddGuarded helper with
    dim guard + input clip, migrateVectorIndex for dim migrations.
  - #326 (#277) — RetentionScore type no longer declares source
    twice; JSDoc back-compat note no longer shadowed.
  - #335 (#323) — viewer drops Google Fonts <link> (CSP-blocked),
    loadDashboard now surfaces load errors inline instead of
    sticking on "Loading dashboard…".

Bumping 0.9.11 -> 0.9.12 across the 9 standard files:
- package.json
- packages/mcp/package.json
- plugin/.claude-plugin/plugin.json
- plugin/.codex-plugin/plugin.json
- src/version.ts
- src/types.ts (ExportData.version literal)
- src/functions/export-import.ts (supportedVersions)
- test/export-import.test.ts (round-trip expectation)
- CHANGELOG.md (new 0.9.12 entry)

886 / 886 tests pass. Build clean.
rohitg00 added a commit that referenced this pull request May 13, 2026
…ardening (#337)

Three landed PRs since v0.9.11:
  - #327 (#295) — BM25 tokenizer now accepts non-ASCII (Greek,
    accented Latin, Hebrew, Arabic, Cyrillic), VectorIndex.add now
    actually called at runtime via vectorIndexAddGuarded helper with
    dim guard + input clip, migrateVectorIndex for dim migrations.
  - #326 (#277) — RetentionScore type no longer declares source
    twice; JSDoc back-compat note no longer shadowed.
  - #335 (#323) — viewer drops Google Fonts <link> (CSP-blocked),
    loadDashboard now surfaces load errors inline instead of
    sticking on "Loading dashboard…".

Bumping 0.9.11 -> 0.9.12 across the 9 standard files:
- package.json
- packages/mcp/package.json
- plugin/.claude-plugin/plugin.json
- plugin/.codex-plugin/plugin.json
- src/version.ts
- src/types.ts (ExportData.version literal)
- src/functions/export-import.ts (supportedVersions)
- test/export-import.test.ts (round-trip expectation)
- CHANGELOG.md (new 0.9.12 entry)

886 / 886 tests pass. Build clean.
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.

CRITICAL: Duplicate source property in RetentionScore interface

1 participant