feat: add memory metacognition foundation#679
Open
RayShark wants to merge 3 commits into
Open
Conversation
|
@RayShark is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughThis PR implements Phase 1 of Hermes Memory Fusion, a policy-driven memory metacognition system. It introduces KV-backed policy management for query expansion, a shadow write-candidate pipeline with regex-based extraction and review, readback verification to validate candidate searchability, and REST API endpoints for policy and candidate workflows, alongside comprehensive test coverage. ChangesHermes Memory Fusion Phase 1
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-05-27-hermes-memory-fusion.md (1)
852-864: ⚡ Quick winResolve stale “pre-implementation” wording in Open Design Decisions.
This section says decisions must be made before implementation starts, but this PR already implements Phase 1. Please rephrase to reflect current status (e.g., “for follow-up phases”) to avoid process confusion.
🤖 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 `@docs/superpowers/plans/2026-05-27-hermes-memory-fusion.md` around lines 852 - 864, The “must be answered before implementation starts” phrasing is stale; update the Open Design Decisions section to reflect that Phase 1 has been implemented and that these questions apply to follow-up phases. Change the lead sentence to something like “These questions should be resolved for follow-up phases” and keep the four numbered items and Recommended defaults as-is, ensuring references to Phase 1, readback verification (mem::search, mem::smart-search), approved candidates (memory_save vs mem::write-candidates-apply), and policy scope wording remain accurate and present.
🤖 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/functions/memory-policy.ts`:
- Around line 48-52: The stored policy's updatedAt timestamp is being
overwritten on reads; update readPolicy/normalizePolicy so that when readPolicy
retrieves a stored MemoryPolicy it preserves stored.updatedAt instead of
re-stamping to now—either pass stored.updatedAt into normalizePolicy or change
normalizePolicy to keep an existing updatedAt if present; ensure the
MemoryPolicy returned by readPolicy (and the mem::policy-get flow) retains the
original updatedAt value when stored.updatedAt exists, while still falling back
to defaultPolicy() behavior when no stored policy exists.
In `@src/functions/readback.ts`:
- Around line 192-205: The current Promise.all over queries will reject if any
sdk.trigger call throws, so modify the queries.map handler used to build
queryResults to catch errors per query (inside the async function passed to
queries.map) around the sdk.trigger/topIdsFromResult logic; on error return an
object with the original query, topIds as empty array (or null), matched: false,
and optionally log the error (e.g., via processLogger or console.error) so
mem::readback-verify can still persist/audit results even when individual
searches fail. Use the existing function names (sdk.trigger, topIdsFromResult)
and preserve the shape { query, topIds, matched } in the catch branch.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-05-27-hermes-memory-fusion.md`:
- Around line 852-864: The “must be answered before implementation starts”
phrasing is stale; update the Open Design Decisions section to reflect that
Phase 1 has been implemented and that these questions apply to follow-up phases.
Change the lead sentence to something like “These questions should be resolved
for follow-up phases” and keep the four numbered items and Recommended defaults
as-is, ensuring references to Phase 1, readback verification (mem::search,
mem::smart-search), approved candidates (memory_save vs
mem::write-candidates-apply), and policy scope wording remain accurate and
present.
🪄 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: 4c84549f-b3f1-4639-b12d-eaf0a4a7bd53
📒 Files selected for processing (15)
AGENTS.mdREADME.mddocs/superpowers/plans/2026-05-27-hermes-memory-fusion.mdsrc/functions/memory-policy.tssrc/functions/readback.tssrc/functions/write-candidates.tssrc/index.tssrc/state/schema.tssrc/triggers/api.tssrc/types.tstest/api-memory-metacognition.test.tstest/memory-policy-types.test.tstest/memory-policy.test.tstest/readback.test.tstest/write-candidates.test.ts
Comment on lines
+48
to
+52
| async function readPolicy(kv: StateKV): Promise<MemoryPolicy> { | ||
| const stored = await kv.get<MemoryPolicy>(KV.memoryPolicy, POLICY_ID); | ||
| if (!stored) return defaultPolicy(); | ||
| return normalizePolicy(stored); | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve updatedAt when normalizing stored policy on reads.
Line 51 normalizes stored data, but Line 153/Line 166 always re-stamp updatedAt to “now”. mem::policy-get can therefore return a different policy metadata value on every read even when nothing changed.
Also applies to: 152-167
🤖 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/functions/memory-policy.ts` around lines 48 - 52, The stored policy's
updatedAt timestamp is being overwritten on reads; update
readPolicy/normalizePolicy so that when readPolicy retrieves a stored
MemoryPolicy it preserves stored.updatedAt instead of re-stamping to now—either
pass stored.updatedAt into normalizePolicy or change normalizePolicy to keep an
existing updatedAt if present; ensure the MemoryPolicy returned by readPolicy
(and the mem::policy-get flow) retains the original updatedAt value when
stored.updatedAt exists, while still falling back to defaultPolicy() behavior
when no stored policy exists.
Comment on lines
+192
to
+205
| const queryResults = await Promise.all( | ||
| queries.map(async (query) => { | ||
| const searchResult = await sdk.trigger({ | ||
| function_id: mode === "smart-search" ? "mem::smart-search" : "mem::search", | ||
| payload: { query, limit }, | ||
| }); | ||
| const topIds = topIdsFromResult(searchResult, limit); | ||
| return { | ||
| query, | ||
| topIds, | ||
| matched: topIds.includes(memory.id), | ||
| }; | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
Isolate per-query search failures so verify still persists a result.
If one search call throws, Promise.all rejects and mem::readback-verify exits without persisting/auditing a readback. Handle failures per query and mark that query as unmatched instead of failing the whole verification.
Suggested patch
const queryResults = await Promise.all(
queries.map(async (query) => {
- const searchResult = await sdk.trigger({
- function_id: mode === "smart-search" ? "mem::smart-search" : "mem::search",
- payload: { query, limit },
- });
- const topIds = topIdsFromResult(searchResult, limit);
- return {
- query,
- topIds,
- matched: topIds.includes(memory.id),
- };
+ try {
+ const searchResult = await sdk.trigger({
+ function_id: mode === "smart-search" ? "mem::smart-search" : "mem::search",
+ payload: { query, limit },
+ });
+ const topIds = topIdsFromResult(searchResult, limit);
+ return {
+ query,
+ topIds,
+ matched: topIds.includes(memory.id),
+ };
+ } catch {
+ return { query, topIds: [], matched: false };
+ }
}),
);🤖 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/functions/readback.ts` around lines 192 - 205, The current Promise.all
over queries will reject if any sdk.trigger call throws, so modify the
queries.map handler used to build queryResults to catch errors per query (inside
the async function passed to queries.map) around the
sdk.trigger/topIdsFromResult logic; on error return an object with the original
query, topIds as empty array (or null), matched: false, and optionally log the
error (e.g., via processLogger or console.error) so mem::readback-verify can
still persist/audit results even when individual searches fail. Use the existing
function names (sdk.trigger, topIdsFromResult) and preserve the shape { query,
topIds, matched } in the catch branch.
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
1 participant
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.
Summary
Scope notes
mem::remember.Related
Testing
npm test -- test/api-memory-metacognition.test.ts test/consistency.test.ts test/memory-policy.test.ts test/memory-policy-types.test.ts test/write-candidates.test.ts test/readback.test.tsnpm run buildenv HOME="$(mktemp -d)" npm test(113 files, 1192 tests)git diff --checkSummary by CodeRabbit
New Features
Documentation