Skip to content

✨ feat(memory): reattributeClaim + node description override + summarizeNode#74

Merged
marcelsamyn merged 3 commits into
mainfrom
feat/reattribute-claim-and-summary
Jun 17, 2026
Merged

✨ feat(memory): reattributeClaim + node description override + summarizeNode#74
marcelsamyn merged 3 commits into
mainfrom
feat/reattribute-claim-and-summary

Conversation

@marcelsamyn

Copy link
Copy Markdown
Owner

What & why

Three additions to @marcelsamyn/memory that power a new Petals feature: correcting the memory graph from the Explore view — fixing the "facts attributed to a different person who's also named Marcel" mis-attribution, and fixing wrong node summaries. All three are app-server primitives (Petals calls them via its server functions; no public proxy / n8n surface).

1. reattributeClaim (atomic) — POST /claim/reattribute

Move one endpoint of a claim onto a different node. In a single transaction: retracts the original (status: retracted, kept for history — not deleted) and creates a user_confirmed replacement preserving predicate / statement / objectValue / description / metadata / objectInstant / sourceId / scope / statedAt / validFrom / validTo, with the chosen endpoint (subject | object) swapped and assertedByNodeId rewired on subject replacement. Runs claim lifecycle + atlas invalidation + embedding.

Guards: rejects replace: "object" on attribute claims (400), unknown / cross-user newNodeId (422 NodesNotFoundError), cross-scope personal/reference mixes (409, reuses CrossScopeMergeError), missing claim → null (404).

2. updateNode description override — POST /node/update

updateNode now accepts an optional description, persisted to node_metadata.description (empty string clears to NULL; omitting it leaves the value unchanged). Removes the old 405 that rejected description edits (descriptions were previously claim-derived only). Lets a user hand-correct a node summary.

3. summarizeNode (per-node summary from claims) — POST /node/summarize

Re-derives a node's summary from its active claims (relational connections + attribute facts) via the existing createCompletionClient + conversation_summary model seam, and returns the text without persisting (the caller persists separately via updateNode({ description })). No active claims → { summary: "" } (no LLM call); node not found / not owned → null (404). This is distinct from the existing summarize(), which is a user-wide Conversation-source summarizer, not a per-node regenerator.

How to test

  • pnpm test --run529 tests green, including new coverage: reattributeClaim subject/object swap fidelity, original-retracted-not-deleted, attribute+object rejection, cross-scope rejection, missing node/claim; updateNode description override + clear; summarizeNode not-found→null, no-claims short-circuit, and with-claims prompt grounding (includes active facts, excludes retracted).
  • pnpm run build:check · pnpm run lint · pnpm run format · pnpm run build · pnpm run build-sdk — all green; SDK .d.ts declares the new methods + types.

Checklist

  • build:check / lint / prettier / unit tests green
  • nitro build + build-sdk verified
  • publish a new version after merge (so Petals can bump the dep)

Petals consumption

import { MemoryClient } from "@marcelsamyn/memory";
const c = new MemoryClient(/* ... */);
await c.reattributeClaim({ userId, claimId, replace, newNodeId }); // => { claim }
await c.updateNode({ userId, nodeId, description }); // empty string clears
await c.summarizeNode({ userId, nodeId }); // => { summary }  ("" when no active claims)

Add an atomic reattributeClaim primitive that re-points a claim's subject
or object endpoint at a different node: it retracts the original (kept for
history) and creates a replacement carrying over every other field, with
user_confirmed provenance and the merge cross-scope guard. Wire it through
the claim schema, /claim/reattribute route, and the SDK client.

Allow /node/update to accept an optional description override (persisted to
node_metadata.description; "" clears it), replacing the previous 405 block.
Add a summarizeNode primitive that re-derives a concise summary for a
single node from its own ACTIVE claims via the LLM and RETURNS the text —
it never persists. Callers (e.g. Explore's "Regenerate from claims") save
it separately via updateNode({ description }).

Loads the node via getNodeById (active claims only), formats each
relational/attribute claim into a compact factual context, and prompts the
cheap conversation_summary model for a 1–3 sentence, third-person summary
grounded only in those facts. Returns null when the node is absent (route
→ 404) and { summary: "" } when there are no active claims (short-circuits
before the LLM). LLM seam mirrors source-title so the extraction-client
test override covers it. Wired through the node schema, /node/summarize
route, and the SDK client.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces functionality to atomically reattribute claim endpoints (subject or object) and to generate concise node summaries from active claims using an LLM. It also enables direct user-authored description overrides on nodes, removing the previous restriction. The review feedback suggests two key improvements: restricting the reattribution process to active claims only to prevent resurrecting inactive claims, and capping the number of claims passed to the LLM during node summarization to avoid potential context window overflow and high token costs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/lib/claim.ts
.where(and(eq(claims.id, input.claimId), eq(claims.userId, input.userId)))
.limit(1);

if (!original) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The reattributeClaim function currently allows reattributing claims regardless of their status. If the original claim is already retracted, superseded, or contradicted, reattributing it will retract it again (which is redundant) and create a new active claim. This effectively resurrects inactive claims, violating the historical integrity of the graph.

We should restrict reattribution to only active claims by throwing an error if the original claim is not active, rather than returning null, to fail fast on invalid inputs.

if (!original || original.status !== "active") {
  throw new Error("Cannot reattribute an inactive claim");
}
References
  1. Prefer failing fast over silently swallowing invalid inputs (e.g., returning empty arrays or null) in internal helper functions.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 06829d5. reattributeClaim now rejects non-active originals with a typed InactiveClaimReattributionError mapped to 409, placed after the not-found->null check so not-found and inactive stay distinct. Used a typed error + route mapping (matching the existing CrossScopeMergeError / NodesNotFoundError guards) rather than a bare throw, which would have surfaced as a 500. Covered by parametrized tests for retracted/superseded/contradicted (each asserts no new claim is created and the original status is preserved).

Comment thread src/lib/node.ts Outdated
Comment on lines +286 to +288
if (result.claims.length === 0) return { summary: "" };

const factLines = result.claims.map(formatClaimForSummary).join("\n");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If a node has a very large number of active claims, passing all of them to the LLM in summarizeNode can lead to context window overflow, high token costs, or degraded summary quality.

Consider capping the number of claims processed (e.g., to the most recent 100 claims) to ensure robust and cost-effective summarization.

if (result.claims.length === 0) return { summary: "" };

const maxClaimsToSummarize = 100;
const claimsToProcess = result.claims.slice(0, maxClaimsToSummarize);
const factLines = claimsToProcess.map(formatClaimForSummary).join("\n");

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 06829d5. summarizeNode now sorts active claims by statedAt descending and caps to MAX_CLAIMS_FOR_SUMMARY = 100 before building the prompt. Sort happens before the slice, so the kept claims are genuinely the most recent (the original suggestion sliced unsorted input). Test seeds 150 claims with increasing statedAt and asserts the newest 100 are present while older ones are dropped. External signature unchanged.

…e claims

reattributeClaim now throws InactiveClaimReattributionError (mapped to 409)
when the original claim is not active, instead of re-retracting dead history
and minting a fresh active clone. summarizeNode sorts active claims by
statedAt desc and caps the prompt at the 100 most recent
(MAX_CLAIMS_FOR_SUMMARY) to avoid context-window overflow on high-claim nodes.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@marcelsamyn marcelsamyn merged commit b00f617 into main Jun 17, 2026
1 check passed
@marcelsamyn marcelsamyn deleted the feat/reattribute-claim-and-summary branch June 17, 2026 10:02
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.

1 participant