-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(tools): 7-tool parity and description refresh #1432
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
base: chore/ci-python-sdk-tests
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,17 +1,18 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { beforeEach, describe, expect, it, vi } from "vitest" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Mock the Supermemory SDK so the Claude memory tool's `view`/`readFile` path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // can be exercised deterministically without any network access. We only need | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `search.execute` to return a single document with known multi-line content. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const searchExecute = vi.fn() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `client.search()` to return a single document with known multi-line content. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const searchMock = vi.fn() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const addMock = vi.fn() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vi.mock("supermemory", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: class MockSupermemory { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| search = { execute: searchExecute } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| search = searchMock | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| add = addMock | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| memories = { forget: vi.fn() } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| documents = { delete: vi.fn() } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -23,18 +24,18 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const FILE_CONTENT = "line1\nline2\nline3\nline4\nline5" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function mockDocument(content: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `readFile` matches by `documentId === normalizePathToCustomId(path)`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `readFile` matches by `id === normalizePathToCustomId(path)`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // normalizePathToCustomId("/memories/notes.txt") -> "memories_notes_txt" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results: [{ documentId: "memories_notes_txt", content }], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results: [{ id: "memories_notes_txt", chunk: content }], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe("ClaudeMemoryTool view_range", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let tool: ClaudeMemoryTool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mockDocument(FILE_CONTENT) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tool = new ClaudeMemoryTool("test-api-key") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -89,16 +90,16 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let tool: ClaudeMemoryTool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| addMock.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tool = new ClaudeMemoryTool("test-api-key") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it("view finds the exact file even when a neighbour ranks first", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { documentId: "memories_notes_backup_txt", content: "backup stuff" }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { documentId: "memories_notes_txt", content: FILE_CONTENT }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { id: "memories_notes_backup_txt", chunk: "backup stuff" }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { id: "memories_notes_txt", chunk: FILE_CONTENT }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -115,9 +116,9 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it("view reports not-found instead of returning a different file", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Semantic search can surface a similarly-named file; that must not | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // be served as the requested one. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { documentId: "memories_notes_backup_txt", content: "backup stuff" }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { id: "memories_notes_backup_txt", chunk: "backup stuff" }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -131,9 +132,9 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it("str_replace refuses to modify a different file than requested", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { documentId: "memories_notes_backup_txt", content: "backup stuff" }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { id: "memories_notes_backup_txt", chunk: "backup stuff" }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -153,31 +154,28 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let tool: ClaudeMemoryTool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| addMock.mockReset() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchExecute.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results: [{ documentId: "memories_notes_txt", content: FILE_CONTENT }], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchMock.mockResolvedValue({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results: [{ id: "memories_notes_txt", chunk: FILE_CONTENT }], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tool = new ClaudeMemoryTool("test-api-key") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it.each([ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "$&", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "$'", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "$`", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "$$", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ])("stores %s literally instead of expanding it as a replacement pattern", async (dollarSequence) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await tool.handleCommand({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| command: "str_replace", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: FILE_PATH, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| old_str: "line3", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new_str: `price is ${dollarSequence} today`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.success).toBe(true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(addMock).toHaveBeenCalledTimes(1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const stored = addMock.mock.calls[0]?.[0]?.content as string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(stored).toContain(`price is ${dollarSequence} today`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(stored).not.toContain("line3") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it.each(["$&", "$'", "$`", "$$"])( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "stores %s literally instead of expanding it as a replacement pattern", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async (dollarSequence) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await tool.handleCommand({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| command: "str_replace", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: FILE_PATH, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| old_str: "line3", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new_str: `price is ${dollarSequence} today`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.success).toBe(true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(addMock).toHaveBeenCalledTimes(1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const stored = addMock.mock.calls[0]?.[0]?.content as string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(stored).toContain(`price is ${dollarSequence} today`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+165
to
+180
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. Test regression: the assertion expect(stored).toContain(`price is ${dollarSequence} today`)
expect(stored).not.toContain("line3") // Add this back
Suggested change
Spotted by Graphite |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,7 +140,7 @@ | |
| default: | ||
| return { | ||
| success: false, | ||
| error: `Unknown command: ${(command as any).command}`, | ||
| } | ||
| } | ||
| } catch (error) { | ||
|
|
@@ -194,11 +194,13 @@ | |
| private async listDirectory(dirPath: string): Promise<MemoryResponse> { | ||
| try { | ||
| // Search for all memory files | ||
| const response = await this.client.search.execute({ | ||
| const response = await this.client.search({ | ||
| q: "*", // Search for all | ||
| containerTags: this.containerTags, | ||
| ...(this.containerTags[0] | ||
| ? { containerTag: this.containerTags[0] } | ||
| : {}), | ||
| limit: 100, // Get many files (max allowed) | ||
| includeFullDocs: false, | ||
| searchMode: "hybrid", | ||
| }) | ||
|
|
||
| if (!response.results) { | ||
|
|
@@ -571,36 +573,48 @@ | |
| */ | ||
| private async getFileDocument(filePath: string): Promise<{ | ||
| success: boolean | ||
| document?: any | ||
| error?: string | ||
| }> { | ||
| try { | ||
| const normalizedId = this.normalizePathToCustomId(filePath) | ||
|
|
||
| const response = await this.client.search.execute({ | ||
| const response = await this.client.search({ | ||
| q: normalizedId, | ||
| containerTags: this.containerTags, | ||
| ...(this.containerTags[0] | ||
| ? { containerTag: this.containerTags[0] } | ||
| : {}), | ||
| limit: 5, | ||
| includeFullDocs: true, | ||
| searchMode: "hybrid", | ||
| }) | ||
|
|
||
| // Only accept the exact customId match. Falling back to the top | ||
| // semantic hit would let callers read — and worse, modify or | ||
| // delete — a different file than the one they asked for. | ||
| const document = response.results?.find( | ||
| (r) => r.documentId === normalizedId, | ||
| const match = response.results?.find( | ||
| (r) => | ||
| r.id === normalizedId || | ||
| r.documents?.some((d) => d.id === normalizedId), | ||
| ) | ||
|
|
||
| if (!document) { | ||
| if (!match) { | ||
| return { | ||
| success: false, | ||
| error: `File not found: ${filePath}`, | ||
| } | ||
| } | ||
|
|
||
| const content = match.chunk || match.memory || "" | ||
| const documentId = match.documents?.[0]?.id ?? match.id | ||
|
|
||
| return { | ||
| success: true, | ||
| document, | ||
| document: { | ||
| documentId, | ||
| content, | ||
| raw: content, | ||
| metadata: match.metadata, | ||
| }, | ||
|
Comment on lines
+607
to
+617
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. The Spotted by Graphite (based on CI logs) |
||
| } | ||
| } catch (error) { | ||
| return { | ||
|
|
||
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
includeFullDocsparameter is still being destructured from the function arguments but is no longer passed to the API call (replaced bysearchMode: 'hybrid'). RemoveincludeFullDocsfrom the destructured parameters to fix the unused variable lint warning.Spotted by Graphite (based on CI logs)

Is this helpful? React 👍 or 👎 to let us know.