fix(naming): derive memory name as a real label, not a text truncation#41
Merged
Conversation
goal_set, high-salience observe promotion, and the dream create phase
each derived a memory's `name` by slicing its definition. goal_set did a
raw mid-word `slice(0, 60)` with no word boundary and no ellipsis, so
names rendered as broken fragments ("…verifiable tr"); the paths also
named identical-length memories inconsistently.
Add engines/naming.ts centralising naming. deriveName(text, llm) mints a
genuine short concept label via the LLM at creation time (the intended
behaviour), falling back to deriveNameHeuristic(text) — first-sentence
preference, word-boundary truncation, ellipsis on elision — whenever the
LLM is unavailable, errors, or returns nothing usable. Wire it into
goal.ts, observe.ts, and cognition.ts; add the versioned label-concept
prompt and unit tests.
Closes #40
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXWLLrZdEzJZRPHyy1U8Dv
There was a problem hiding this comment.
Pull request overview
Updates memory minting so Memory.name becomes a true short label (LLM-generated with deterministic fallback) instead of inconsistent raw text truncation across creation paths. This aligns goal_set, high-salience observe promotion, and the dream create phase to use a shared naming helper.
Changes:
- Added a centralized naming helper (
deriveName+deriveNameHeuristic) and adopted it in all three memory-creation paths. - Added a versioned
label-conceptprompt to the prompt registry and pinned its version in tests. - Added targeted unit tests for the naming heuristic and LLM-fallback behavior; documented the change in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/observe.ts | Uses shared deriveName for high-salience novel memory creation. |
| src/tools/goal.ts | Uses shared deriveName for goal_set memory names. |
| src/engines/prompts.ts | Adds the label-concept prompt and registers it. |
| src/engines/prompts.test.ts | Pins the new prompt id/version in the registry version test. |
| src/engines/naming.ts | Introduces centralized naming + sanitization + heuristic fallback logic. |
| src/engines/naming.test.ts | Adds tests covering heuristic truncation shape and LLM fallback/sanitization. |
| src/engines/cognition.ts | Uses shared deriveName in the dream create/promotion path. |
| CHANGELOG.md | Documents the behavioral change for upcoming release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…dary words Address review feedback on the naming heuristic: - Word-boundary truncation no longer drops a full word that ends exactly on the cut when the next character is punctuation. The previous slice+`replace(/\s+\S*$/, '')` stripped the trailing token unconditionally; it now accumulates whole words and ignores trailing punctuation on the boundary word when measuring fit. - Align on a single limit: NAME_MAX_LEN is 60 (matching the label-concept prompt and the CLI's 60-char display clip) and the ellipsis is budgeted inside it, so a derived name never exceeds 60 chars and a downstream `name.slice(0, 60)` can't reintroduce a mid-word cut. Add a regression test for the punctuation-boundary case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXWLLrZdEzJZRPHyy1U8Dv
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…elper CodeQL flagged the `/[...]+$/` trailing-trim patterns as polynomial-ReDoS: an unanchored end-anchored `+` over a character class is retried from every start position, so a long run of matching characters (e.g. many '!' or tabs) in the exported, caller-supplied text runs in O(n²). Introduce `trimEndChars(s, re)` — a linear loop that tests one character at a time from the end — and use it for every trailing trim (first-sentence punctuation, word-accumulation boundary, base clip, label sanitization). Leading `^`-anchored and global trims are already linear and left as-is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXWLLrZdEzJZRPHyy1U8Dv
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
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
Closes #40. A memory's
namewas derived by truncating its definition, not by generating a concise label.goal_setdid a raw mid-wordslice(0, 60)with no word boundary and no ellipsis, so names rendered as visibly broken fragments (…verifiable tr); the high-salienceobservepromotion and the dreamcreatephase each truncated differently, so identical-length memories got differently-shaped names.This implements the issue's intended behavior: generate a genuine short concept label at mint time, with a solid deterministic fallback, and factor the logic into one shared helper used by all three creation paths.
Changes
src/engines/naming.tscentralising name derivation:deriveName(text, llm)— asks the LLM for a real title-like label at creation time (temperature 0.2, tight token budget), sanitizes the response (strips wrapping quotes,Title:prefixes, trailing punctuation), and never throws — any LLM failure, empty response, or over-long label degrades to the heuristic.deriveNameHeuristic(text)— the deterministic fallback: prefer the first sentence when it fits, otherwise truncate on a word boundary and append an ellipsis so a shortened name reads as deliberately short, not broken mid-word.label-conceptprompt added to the versioned prompt registry (engines/prompts.ts) and its version snapshot test.tools/goal.ts(goal_set),tools/observe.ts(novel high-salience promotion), andengines/cognition.ts(dream create phase) now callderiveName(...)instead of aslice(0, 60).src/engines/naming.test.tspins the heuristic's shape (word-boundary truncation, ellipsis, first-sentence preference) and the LLM-with-fallback behavior, including the exact issue repro.Display-only fallbacks in the CLI (
wander-cmd.ts,maintain-cmd.ts) are left as-is — they defensively handle legacy un-named memories and are no longer papering over broken names for newly-minted ones.Testing
npm run build— clean.npm test— 25 files, 212 tests pass (16 new).🤖 Generated with Claude Code
Generated by Claude Code