feat(memory): add lifecycle inspector and archive preview - #1858
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughAdds lifecycle derivation for memory rows, archive-candidate preview APIs, renderer UI for lifecycle inspection, locale strings, and tests covering the new data flow. ChangesMemory Lifecycle Feature
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/shared/contracts/routes/memory.routes.ts (2)
316-320: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftUse a singular payload for the single-memory route.
memoryIdidentifies one row, but this contract exposeslifecycles: MemoryLifecycle[]. That weakens the new API boundary by allowing accidental duplicates to validate and forces callers to treat a single-resource lookup as a collection. A{ lifecycle: MemoryLifecycleSchema.nullable() }shape would better match the route semantics before this API hardens across layers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/contracts/routes/memory.routes.ts` around lines 316 - 320, The memory.getLifecycle contract currently returns a collection-shaped payload even though it targets a single memoryId. Update memoryGetLifecycleRoute in the route contract to expose a singular lifecycle field, ideally nullable, instead of lifecycles: MemoryLifecycle[], and make sure any schema consumers or validators referenced by memoryGetLifecycleRoute are aligned with that single-resource shape.
208-227: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEnforce the “archive candidate” invariant in the preview schema.
MemoryArchiveCandidateLifecyclePreviewSchemacurrently accepts anyMemoryLifecycle, including rows wherearchiveEligibility.eligibleis false.src/main/presenter/memoryPresenter/index.ts:2345-2347already filters these out, so that guarantee belongs in the shared contract too. Tightening the element schema here will catch regressions at the IPC boundary instead of letting non-candidates leak to the renderer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/contracts/routes/memory.routes.ts` around lines 208 - 227, `MemoryArchiveCandidateLifecyclePreviewSchema` is too permissive because its `lifecycles` array still allows any `MemoryLifecycle`, including non-candidates. Tighten the shared contract by refining the element schema inside `MemoryArchiveCandidateLifecyclePreviewSchema` so each item requires `archiveEligibility.eligible` to be true, matching the filtering already done in `memoryPresenter` and preventing invalid rows from crossing the IPC boundary. Keep the existing preview and scan checks intact while updating the schema definition to enforce this invariant at validation time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/presenter/memoryPresenter/lifecycle.ts`:
- Around line 48-52: The lifecycle eligibility checks in the memory presenter
use strict comparisons, so rows exactly on the archive-age or decay threshold
are still marked ineligible even though deriveArchiveGaps() shows zero
remaining. Update the eligibility logic in lifecycle.ts to use inclusive
boundary checks for oldEnough and decayedEnough, and make sure deriveDecayTier()
uses the same threshold semantics so the UI and eligibility state stay
consistent. Apply the same fix at the other call sites referenced by the review
so all archive/decay decisions use the same boundary behavior.
In `@src/renderer/settings/components/MemoryHealthSection.vue`:
- Around line 321-334: The archive candidate preview logic in
archiveCandidatePreviewLimitMessage is inferring truncation from
lifecycles.length reaching previewLimit, which can show a “limited” message even
when nothing was actually dropped. Update the shared preview DTO returned to
MemoryHealthSection.vue to expose an explicit previewTruncated (or equivalent
eligible-count) flag from the route, and use that flag in this computed branch
instead of comparing lifecycles.length to previewLimit. Keep scanTruncated
handling unchanged and only show the preview-limited message when the new
explicit signal says the eligible list overflowed the display cap.
In `@src/renderer/settings/components/MemoryManagerPanel.vue`:
- Around line 313-320: The lifecycle toggle/panel in MemoryManagerPanel’s row
rendering is shown for every memory, even when
runtime.memoryPresenter.getLifecycle() returns an empty result for working
memories. Update the logic around the Button and the expanded lifecycle panel to
only render for supported kinds, or show a dedicated unsupported-state message
instead; use the existing memory.kind and isLifecycleExpanded/toggleLifecycle
paths to locate the affected UI.
In `@test/main/presenter/agentMemoryTable.test.ts`:
- Around line 714-736: The archive-candidate lifecycle test is relying on
AgentMemoryTable.insert()’s default Date.now() timestamps, so
listArchiveCandidateLifecycleRows('a', 5000, 10) filters everything out. Update
this test to seed deterministic createdAt values on the inserted rows
(especially the ones meant to qualify before the cutoff) so the call to
listArchiveCandidateLifecycleRows exercises the intended projection and
filtering logic. Use the insert() calls and listArchiveCandidateLifecycleRows()
as the main anchors when adjusting the fixture data.
In `@test/renderer/components/MemoryLifecyclePanel.test.ts`:
- Around line 71-77: The locale coverage in the MemoryLifecyclePanel test is too
narrow because getLifecycleMessages only inspects memoryManager.lifecycle, so it
misses new copy like health.archivePrediction and related strings. Update the
test assertions around getLifecycleMessages and the locale regex checks to
include the additional strings/scripts added here, and broaden the Chinese
matching so zh-HK/zh-TW variants such as 複習, 晉級, 回顧, and 倒數 are validated
instead of passing silently.
---
Nitpick comments:
In `@src/shared/contracts/routes/memory.routes.ts`:
- Around line 316-320: The memory.getLifecycle contract currently returns a
collection-shaped payload even though it targets a single memoryId. Update
memoryGetLifecycleRoute in the route contract to expose a singular lifecycle
field, ideally nullable, instead of lifecycles: MemoryLifecycle[], and make sure
any schema consumers or validators referenced by memoryGetLifecycleRoute are
aligned with that single-resource shape.
- Around line 208-227: `MemoryArchiveCandidateLifecyclePreviewSchema` is too
permissive because its `lifecycles` array still allows any `MemoryLifecycle`,
including non-candidates. Tighten the shared contract by refining the element
schema inside `MemoryArchiveCandidateLifecyclePreviewSchema` so each item
requires `archiveEligibility.eligible` to be true, matching the filtering
already done in `memoryPresenter` and preventing invalid rows from crossing the
IPC boundary. Keep the existing preview and scan checks intact while updating
the schema definition to enforce this invariant at validation time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ce04072-a662-4503-81e4-3837a944dd5b
📒 Files selected for processing (42)
src/main/presenter/memoryPresenter/index.tssrc/main/presenter/memoryPresenter/lifecycle.tssrc/main/presenter/memoryPresenter/lifecycleConstants.tssrc/main/presenter/memoryPresenter/scoring.tssrc/main/presenter/memoryPresenter/types.tssrc/main/presenter/sqlitePresenter/tables/agentMemory.tssrc/main/routes/index.tssrc/renderer/api/MemoryClient.tssrc/renderer/settings/components/MemoryHealthSection.vuesrc/renderer/settings/components/MemoryLifecyclePanel.vuesrc/renderer/settings/components/MemoryManagerPanel.vuesrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/de-DE/settings.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/es-ES/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/id-ID/settings.jsonsrc/renderer/src/i18n/it-IT/settings.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/ms-MY/settings.jsonsrc/renderer/src/i18n/pl-PL/settings.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/tr-TR/settings.jsonsrc/renderer/src/i18n/vi-VN/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/shared/contracts/routes.tssrc/shared/contracts/routes/memory.routes.tstest/main/presenter/agentMemoryTable.test.tstest/main/presenter/fakes/memoryFakes.tstest/main/presenter/memoryLifecycle.test.tstest/main/routes/dispatcher.test.tstest/main/routes/memoryDto.test.tstest/renderer/api/clients.test.tstest/renderer/components/MemoryHealthSection.test.tstest/renderer/components/MemoryLifecyclePanel.test.tstest/renderer/components/MemoryManagerDialog.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
memory.getLifecycle(agentId, memoryId)as a single-memory lifecycle route withmemoryIdrequired.memory.getArchiveCandidateLifecyclePreview.scanLimit=200scanned rows andpreviewLimit=25rendered lifecycles.Summary by CodeRabbit