-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add loadWithQuery to MemoryContextLoader (GIT-100) #66
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
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,17 +2,20 @@ | |||||||||||||
| * MemoryContextLoader | ||||||||||||||
| * | ||||||||||||||
| * Loads and filters memories for hook context injection. | ||||||||||||||
| * Delegates to IMemoryRepository for actual data access. | ||||||||||||||
| * Delegates to IMemoryRepository for general loads and | ||||||||||||||
| * IMemoryService for query-based searches (notes + trailers). | ||||||||||||||
| */ | ||||||||||||||
|
|
||||||||||||||
| import type { IMemoryContextLoader, IMemoryContextOptions, IMemoryContextResult } from '../../domain/interfaces/IMemoryContextLoader'; | ||||||||||||||
| import type { IMemoryRepository } from '../../domain/interfaces/IMemoryRepository'; | ||||||||||||||
| import type { IMemoryService } from '../interfaces/IMemoryService'; | ||||||||||||||
| import type { ILogger } from '../../domain/interfaces/ILogger'; | ||||||||||||||
|
|
||||||||||||||
| export class MemoryContextLoader implements IMemoryContextLoader { | ||||||||||||||
| constructor( | ||||||||||||||
| private readonly memoryRepository: IMemoryRepository, | ||||||||||||||
| private readonly logger?: ILogger, | ||||||||||||||
| private readonly memoryService?: IMemoryService, | ||||||||||||||
| ) {} | ||||||||||||||
|
|
||||||||||||||
| load(options?: IMemoryContextOptions): IMemoryContextResult { | ||||||||||||||
|
|
@@ -46,4 +49,32 @@ export class MemoryContextLoader implements IMemoryContextLoader { | |||||||||||||
| filtered: result.memories.length, | ||||||||||||||
| }; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| loadWithQuery(query: string, limit?: number, cwd?: string): IMemoryContextResult { | ||||||||||||||
| // Use MemoryService.recall() to search both notes and trailers | ||||||||||||||
| if (!this.memoryService) { | ||||||||||||||
| this.logger?.warn('MemoryService not available for query search, falling back to repository'); | ||||||||||||||
| return this.load({ limit, cwd }); | ||||||||||||||
|
Comment on lines
+53
to
+57
|
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Get total count for stats | ||||||||||||||
| const allResult = this.memoryRepository.query({ cwd }); | ||||||||||||||
| const total = allResult.total; | ||||||||||||||
|
Comment on lines
+60
to
+62
|
||||||||||||||
| // Get total count for stats | |
| const allResult = this.memoryRepository.query({ cwd }); | |
| const total = allResult.total; | |
| // Get total count for stats across notes and trailers | |
| const totalResult = this.memoryService.recall('', { cwd }); | |
| const total = totalResult.total; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,4 +30,15 @@ export interface IMemoryContextResult { | |
| export interface IMemoryContextLoader { | ||
| /** Load memories with optional filters. */ | ||
| load(options?: IMemoryContextOptions): IMemoryContextResult; | ||
|
|
||
| /** | ||
| * Load memories filtered by a search query. | ||
| * Searches both git notes and commit trailers for matching memories. | ||
| * | ||
| * @param query - Search keywords (e.g., "authentication, GIT-95, LoginHandler") | ||
| * @param limit - Maximum memories to return | ||
| * @param cwd - Working directory for git operations | ||
| * @returns Matching memories from both notes and trailers | ||
| */ | ||
| loadWithQuery(query: string, limit?: number, cwd?: string): IMemoryContextResult; | ||
|
Comment on lines
+33
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider using an options object for API consistency. The existing interface ILoadWithQueryOptions {
readonly query: string;
readonly limit?: number;
readonly cwd?: string;
}
loadWithQuery(options: ILoadWithQueryOptions): IMemoryContextResult;This is a minor API design consideration—the current approach works and is acceptable if you prefer the simpler signature. 🤖 Prompt for AI Agents |
||
| } | ||
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.
In the MemoryService-not-available fallback,
loadWithQueryignores the providedqueryand returns the same result asload(). This breaks the contract of a query-filtered load; consider falling back tomemoryRepository.query({ query, limit, cwd })(notes-only) so at least notes are filtered correctly, and keep the total/filtered semantics consistent.