-
Notifications
You must be signed in to change notification settings - Fork 0
feat: unified recall across notes and trailers (GIT-68) #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7dd735b
4080eec
8fcd93f
4e83310
709f654
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,34 +2,204 @@ | |||||||||||||||||||||||||||||||||
| * MemoryService | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Application service that orchestrates memory operations. | ||||||||||||||||||||||||||||||||||
| * Delegates storage to IMemoryRepository. | ||||||||||||||||||||||||||||||||||
| * Dual-writes to git notes (rich JSON) and commit trailers | ||||||||||||||||||||||||||||||||||
| * (lightweight, natively queryable metadata). | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import type { IMemoryService } from '../interfaces/IMemoryService'; | ||||||||||||||||||||||||||||||||||
| import type { IMemoryRepository, IMemoryQueryOptions, IMemoryQueryResult } from '../../domain/interfaces/IMemoryRepository'; | ||||||||||||||||||||||||||||||||||
| import type { IMemoryEntity, ICreateMemoryOptions } from '../../domain/entities/IMemoryEntity'; | ||||||||||||||||||||||||||||||||||
| import type { IMemoryEntity, ICreateMemoryOptions, MemoryType } from '../../domain/entities/IMemoryEntity'; | ||||||||||||||||||||||||||||||||||
| import type { ITrailerService, ICommitTrailers } from '../../domain/interfaces/ITrailerService'; | ||||||||||||||||||||||||||||||||||
| import type { ITrailer } from '../../domain/entities/ITrailer'; | ||||||||||||||||||||||||||||||||||
| import { AI_TRAILER_KEYS, AI_TRAILER_PREFIX } from '../../domain/entities/ITrailer'; | ||||||||||||||||||||||||||||||||||
| import type { ConfidenceLevel } from '../../domain/types/IMemoryQuality'; | ||||||||||||||||||||||||||||||||||
| import type { ILogger } from '../../domain/interfaces/ILogger'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const MEMORY_TYPE_TO_TRAILER_KEY: Record<MemoryType, string> = { | ||||||||||||||||||||||||||||||||||
| decision: AI_TRAILER_KEYS.DECISION, | ||||||||||||||||||||||||||||||||||
| gotcha: AI_TRAILER_KEYS.GOTCHA, | ||||||||||||||||||||||||||||||||||
| convention: AI_TRAILER_KEYS.CONVENTION, | ||||||||||||||||||||||||||||||||||
| fact: AI_TRAILER_KEYS.FACT, | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const TRAILER_KEY_TO_MEMORY_TYPE: Record<string, MemoryType> = { | ||||||||||||||||||||||||||||||||||
| [AI_TRAILER_KEYS.DECISION]: 'decision', | ||||||||||||||||||||||||||||||||||
| [AI_TRAILER_KEYS.GOTCHA]: 'gotcha', | ||||||||||||||||||||||||||||||||||
| [AI_TRAILER_KEYS.CONVENTION]: 'convention', | ||||||||||||||||||||||||||||||||||
| [AI_TRAILER_KEYS.FACT]: 'fact', | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export class MemoryService implements IMemoryService { | ||||||||||||||||||||||||||||||||||
| constructor( | ||||||||||||||||||||||||||||||||||
| private readonly memoryRepository: IMemoryRepository, | ||||||||||||||||||||||||||||||||||
| private readonly logger?: ILogger, | ||||||||||||||||||||||||||||||||||
| private readonly trailerService?: ITrailerService, | ||||||||||||||||||||||||||||||||||
| ) {} | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| remember(text: string, options?: ICreateMemoryOptions): IMemoryEntity { | ||||||||||||||||||||||||||||||||||
| const memory = this.memoryRepository.create(text, options); | ||||||||||||||||||||||||||||||||||
| this.logger?.info('Memory stored', { id: memory.id, type: memory.type, sha: memory.sha }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Dual-write: also add AI-* trailers to the commit (opt-out via trailers: false) | ||||||||||||||||||||||||||||||||||
| if (options?.trailers !== false && this.trailerService) { | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| const trailers = this.buildTrailers(memory); | ||||||||||||||||||||||||||||||||||
| this.trailerService.addTrailers(trailers, options?.cwd); | ||||||||||||||||||||||||||||||||||
| this.logger?.info('Trailers written', { count: trailers.length, sha: memory.sha }); | ||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||
| // Trailer write failure is non-fatal (commit may be pushed already) | ||||||||||||||||||||||||||||||||||
| this.logger?.warn('Trailer write failed', { | ||||||||||||||||||||||||||||||||||
| error: err instanceof Error ? err.message : String(err), | ||||||||||||||||||||||||||||||||||
| sha: memory.sha, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return memory; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private buildTrailers(memory: IMemoryEntity): ITrailer[] { | ||||||||||||||||||||||||||||||||||
| const trailers: ITrailer[] = [ | ||||||||||||||||||||||||||||||||||
| { key: MEMORY_TYPE_TO_TRAILER_KEY[memory.type], value: memory.content }, | ||||||||||||||||||||||||||||||||||
| { key: AI_TRAILER_KEYS.CONFIDENCE, value: memory.confidence }, | ||||||||||||||||||||||||||||||||||
| { key: AI_TRAILER_KEYS.MEMORY_ID, value: memory.id }, | ||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (memory.tags.length > 0) { | ||||||||||||||||||||||||||||||||||
| trailers.push({ key: AI_TRAILER_KEYS.TAGS, value: memory.tags.join(', ') }); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return trailers; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| recall(query?: string, options?: IMemoryQueryOptions): IMemoryQueryResult { | ||||||||||||||||||||||||||||||||||
| const effectiveQuery = query ?? options?.query; | ||||||||||||||||||||||||||||||||||
| const result = this.memoryRepository.query({ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // 1. Search notes (existing path) | ||||||||||||||||||||||||||||||||||
| const notesResult = this.memoryRepository.query({ | ||||||||||||||||||||||||||||||||||
| ...options, | ||||||||||||||||||||||||||||||||||
| query: effectiveQuery, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| this.logger?.info('Memory recall', { query: effectiveQuery, count: result.memories.length, total: result.total }); | ||||||||||||||||||||||||||||||||||
| return result; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // 2. Search trailers if service available | ||||||||||||||||||||||||||||||||||
| if (!this.trailerService) { | ||||||||||||||||||||||||||||||||||
| this.logger?.info('Memory recall', { query: effectiveQuery, count: notesResult.memories.length, total: notesResult.total }); | ||||||||||||||||||||||||||||||||||
| return notesResult; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const trailerMemories = this.recallFromTrailers(effectiveQuery, options, notesResult.memories); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // 3. Merge results (notes first, then trailer-only) | ||||||||||||||||||||||||||||||||||
| const allMemories = [...notesResult.memories, ...trailerMemories]; | ||||||||||||||||||||||||||||||||||
| const limit = options?.limit ?? allMemories.length; | ||||||||||||||||||||||||||||||||||
| const merged = allMemories.slice(0, limit); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+78
to
+95
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| this.logger?.info('Memory recall', { | ||||||||||||||||||||||||||||||||||
| query: effectiveQuery, | ||||||||||||||||||||||||||||||||||
| notesCount: notesResult.memories.length, | ||||||||||||||||||||||||||||||||||
| trailerCount: trailerMemories.length, | ||||||||||||||||||||||||||||||||||
| total: allMemories.length, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||
| memories: merged, | ||||||||||||||||||||||||||||||||||
| total: allMemories.length, | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private recallFromTrailers( | ||||||||||||||||||||||||||||||||||
| query: string | undefined, | ||||||||||||||||||||||||||||||||||
| options: IMemoryQueryOptions | undefined, | ||||||||||||||||||||||||||||||||||
| notesMemories: readonly IMemoryEntity[], | ||||||||||||||||||||||||||||||||||
| ): IMemoryEntity[] { | ||||||||||||||||||||||||||||||||||
| if (!this.trailerService) return []; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| const trailerCommits = this.trailerService.queryTrailers(AI_TRAILER_PREFIX, { | ||||||||||||||||||||||||||||||||||
| cwd: options?.cwd, | ||||||||||||||||||||||||||||||||||
| since: options?.since, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // IDs already present in notes results (for deduplication) | ||||||||||||||||||||||||||||||||||
| const noteIds = new Set(notesMemories.map(m => m.id)); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const results: IMemoryEntity[] = []; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| for (const commit of trailerCommits) { | ||||||||||||||||||||||||||||||||||
| const entities = this.trailerCommitToEntities(commit); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| for (const entity of entities) { | ||||||||||||||||||||||||||||||||||
| // Deduplicate: skip if AI-Memory-Id matches a notes entry | ||||||||||||||||||||||||||||||||||
| if (noteIds.has(entity.id)) continue; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Apply query filter | ||||||||||||||||||||||||||||||||||
| if (query && !this.matchesQuery(entity, query)) continue; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+132
to
+136
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Apply type filter | ||||||||||||||||||||||||||||||||||
| if (options?.type && entity.type !== options.type) continue; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Apply tag filter | ||||||||||||||||||||||||||||||||||
| if (options?.tag && !entity.tags.some(t => t.toLowerCase() === options.tag!.toLowerCase())) continue; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+141
to
+142
|
||||||||||||||||||||||||||||||||||
| // Apply tag filter | |
| if (options?.tag && !entity.tags.some(t => t.toLowerCase() === options.tag!.toLowerCase())) continue; | |
| // Apply tag filter (case-sensitive, consistent with MemoryRepository) | |
| if (options?.tag && !entity.tags.includes(options.tag)) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — trailer tag filter is now case-sensitive using tags.includes(options.tag), consistent with MemoryRepository behavior. See 97190a7.
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shared metadata (confidence, tags) from commit trailers is applied to all memory entities derived from that commit. This assumes all type trailers on a single commit share the same confidence and tags, which is correct for dual-write scenarios but may be incorrect for manually-added trailers. For example, if someone manually adds "AI-Decision: Use Redis\nAI-Gotcha: Watch for memory leaks\nAI-Confidence: high\nAI-Tags: cache", both memories will get confidence 'high' and tags ['cache'], even though the gotcha might have different intended metadata. Consider documenting this limitation or parsing per-memory metadata if needed.
| // Shared metadata from the commit's trailers | |
| // Shared metadata from the commit's trailers. | |
| // | |
| // NOTE: We intentionally treat AI-Confidence and AI-Tags as *commit-level* metadata. | |
| // All memory entities derived from this commit will share the same confidence and tags. | |
| // | |
| // This matches the dual-write behavior where a single remember() call emits both | |
| // rich JSON notes and lightweight commit trailers, and all trailers for a commit | |
| // are expected to describe the same logical change. | |
| // | |
| // Limitation: If someone manually adds multiple memory-type trailers (e.g. | |
| // AI-Decision and AI-Gotcha) with different intended confidences/tags in the same | |
| // commit, we do *not* attempt to parse or align per-memory metadata here; instead | |
| // we use a single shared AI-Confidence/AI-Tags value for every memory entity. | |
| // If per-memory trailer metadata is ever required, this is the place to extend | |
| // the parsing logic to support it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added documentation comment explaining that AI-Confidence and AI-Tags are treated as commit-level metadata, matching the dual-write behavior. The limitation for manually-added multi-type trailers is now explicitly documented. See 97190a7.
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The confidence level from trailer may not be a valid ConfidenceLevel value. If a trailer has AI-Confidence set to an invalid value (e.g., 'super-high'), the type assertion will silently succeed but the value will violate the type contract. Consider validating that the confidence value is one of the valid ConfidenceLevel values before the type assertion, or add a fallback that defaults to 'high' if the value is not recognized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — now uses isValidConfidence() from domain types to validate the trailer confidence value. Falls back to 'high' for invalid values. See 97190a7.
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Synthetic ID generation may produce duplicate IDs if a commit has multiple manually-added trailers of the same type. For example, two AI-Decision trailers without AI-Memory-Ids would both get the same synthetic ID (trailer:SHA:decision), violating the uniqueness constraint on memory IDs. Consider appending the index or a hash of the content to ensure uniqueness, e.g., trailer:${commit.sha}:${type}:${i} or using the content itself as part of the ID.
| // Pair with AI-Memory-Id by position, or generate synthetic ID | |
| const id = memoryIds[i] || `trailer:${commit.sha}:${type}`; | |
| // Pair with AI-Memory-Id by position, or generate synthetic ID (unique per commit/type/index) | |
| const id = memoryIds[i] || `trailer:${commit.sha}:${type}:${i}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — synthetic IDs now include the index: trailer:${sha}:${type}:${i}. Also added a test verifying uniqueness for multiple same-type trailers. See 97190a7.
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailer-sourced memories use the current timestamp instead of the commit's author or committer date. This creates incorrect temporal data: a commit from 6 months ago will appear to have been created today when recalled from trailers. The createdAt and updatedAt fields should reflect the actual commit timestamp to maintain accurate chronology. Consider fetching the commit date from git (via git log --format=%aI or %cI) and using it here instead of new Date().toISOString().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged — added a documentation comment noting this as a known limitation. Using the commit's author date would require an extra git log call per commit in the trailer recall path, which we're deferring for now. The commit SHA is available for date lookup if needed.
Uh oh!
There was an error while loading. Please reload this page.