mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 17:33:51 +02:00
Fix memory record counts and operation links
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
bd9ae30a01
commit
a01a3ce2ed
4 files changed
+154
-14
No files matched your search
@@ -130,6 +130,14 @@ export const memoryProviderConfigMetadataSchema = z
|
||||
})
|
||||
.strict();
|
||||
|
||||
const queryBooleanSchema = z.preprocess((value) => {
|
||||
if (typeof value !== "string") return value;
|
||||
const normalized = value.trim().toLowerCase();
|
||||
if (normalized === "true") return true;
|
||||
if (normalized === "false") return false;
|
||||
return value;
|
||||
}, z.boolean());
|
||||
|
||||
export const memoryHookPolicySchema = z
|
||||
.object({
|
||||
enabled: z.boolean().optional(),
|
||||
@@ -295,10 +303,10 @@ export const memoryListRecordsQuerySchema = z
|
||||
runId: z.string().uuid().optional(),
|
||||
sourceKind: z.enum(MEMORY_SOURCE_KINDS).optional(),
|
||||
q: z.string().trim().min(1).max(500).optional(),
|
||||
includeDeleted: z.coerce.boolean().optional().default(false),
|
||||
includeRevoked: z.coerce.boolean().optional().default(false),
|
||||
includeExpired: z.coerce.boolean().optional().default(false),
|
||||
includeSuperseded: z.coerce.boolean().optional().default(false),
|
||||
includeDeleted: queryBooleanSchema.optional().default(false),
|
||||
includeRevoked: queryBooleanSchema.optional().default(false),
|
||||
includeExpired: queryBooleanSchema.optional().default(false),
|
||||
includeSuperseded: queryBooleanSchema.optional().default(false),
|
||||
limit: z.coerce.number().int().positive().max(200).optional().default(50),
|
||||
count: z.enum(["only"]).optional(),
|
||||
})
|
||||
|
||||
@@ -258,7 +258,7 @@ describe("memory routes", () => {
|
||||
|
||||
const res = await request(app)
|
||||
.get(`/api/companies/${companyA}/memory/records`)
|
||||
.query({ count: "only", reviewState: "pending" })
|
||||
.query({ count: "only", reviewState: "pending", includeRevoked: "false", includeExpired: "false" })
|
||||
.set("Origin", "http://localhost:3100");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
@@ -269,6 +269,7 @@ describe("memory routes", () => {
|
||||
count: "only",
|
||||
reviewState: "pending",
|
||||
includeRevoked: false,
|
||||
includeExpired: false,
|
||||
}),
|
||||
expect.objectContaining({ actorType: "user", userId: "board-user" }),
|
||||
);
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { companies, createDb } from "@paperclipai/db";
|
||||
import type { MemoryListRecordsQuery } from "@paperclipai/shared";
|
||||
import { memoryService } from "../services/memory.js";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
|
||||
function createQueuedSelectDb(selectResults: unknown[][]) {
|
||||
const calls: Array<{ kind: string }> = [];
|
||||
@@ -382,7 +389,7 @@ describe("memoryService hook policies", () => {
|
||||
updatedValues.push(value);
|
||||
return {
|
||||
where: vi.fn().mockReturnValue({
|
||||
returning: vi.fn().mockResolvedValue(updateResults.shift() ?? []),
|
||||
returning: vi.fn().mockImplementation(() => Promise.resolve(updateResults.shift() ?? [])),
|
||||
}),
|
||||
};
|
||||
}),
|
||||
@@ -420,6 +427,7 @@ describe("memoryService hook policies", () => {
|
||||
});
|
||||
expect(insertedValues[1]).toMatchObject({
|
||||
reviewState: "accepted",
|
||||
createdByOperationId: null,
|
||||
metadata: {
|
||||
extraction: {
|
||||
mode: "paperclip_managed",
|
||||
@@ -428,7 +436,12 @@ describe("memoryService hook policies", () => {
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(updatedValues[0]).toMatchObject({
|
||||
expect(updatedValues).toContainEqual(
|
||||
expect.objectContaining({
|
||||
createdByOperationId: "77777777-7777-4777-8777-777777777777",
|
||||
}),
|
||||
);
|
||||
expect(updatedValues.find((value) => (value as { status?: string }).status === "succeeded")).toMatchObject({
|
||||
status: "succeeded",
|
||||
operationId: "77777777-7777-4777-8777-777777777777",
|
||||
resultJson: {
|
||||
@@ -438,3 +451,92 @@ describe("memoryService hook policies", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
describeEmbeddedPostgres("memoryService local basic persistence", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-memory-service-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
it("captures records with a valid operation id and counts explicit revoked filters", async () => {
|
||||
const companyId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip Memory Test",
|
||||
issuePrefix: `M${companyId.replace(/-/g, "").slice(0, 5).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
|
||||
const service = memoryService(db);
|
||||
const actor = {
|
||||
actorType: "user" as const,
|
||||
actorId: "board-user",
|
||||
agentId: null,
|
||||
userId: "board-user",
|
||||
runId: null,
|
||||
};
|
||||
const binding = await service.createBinding(companyId, {
|
||||
key: "local-test",
|
||||
name: "Local test",
|
||||
providerKey: "local_basic",
|
||||
config: {},
|
||||
enabled: true,
|
||||
});
|
||||
await service.setCompanyDefault(companyId, binding.id);
|
||||
|
||||
const captured = await service.capture(
|
||||
companyId,
|
||||
{
|
||||
bindingKey: "local-test",
|
||||
scope: { scopeType: "org", scopeId: companyId },
|
||||
scopeType: "org",
|
||||
scopeId: companyId,
|
||||
source: { kind: "manual_note", externalRef: "memory-service-test" },
|
||||
title: "Revoked filter regression",
|
||||
content: "A memory record that will be revoked for count filter coverage.",
|
||||
reviewState: "pending",
|
||||
},
|
||||
actor,
|
||||
);
|
||||
|
||||
expect(captured.records).toHaveLength(1);
|
||||
expect(captured.records[0].createdByOperationId).toBe(captured.operation.id);
|
||||
|
||||
await service.revoke(
|
||||
companyId,
|
||||
{
|
||||
selector: { recordIds: [captured.records[0].id] },
|
||||
reason: "Verify revoked filters can see explicit revoked retention state",
|
||||
},
|
||||
actor,
|
||||
);
|
||||
|
||||
const revokedFilters: MemoryListRecordsQuery = {
|
||||
retentionState: "revoked",
|
||||
includeDeleted: false,
|
||||
includeRevoked: true,
|
||||
includeExpired: false,
|
||||
includeSuperseded: false,
|
||||
limit: 50,
|
||||
};
|
||||
|
||||
await expect(service.countRecords(companyId, revokedFilters, actor)).resolves.toEqual({ count: 1 });
|
||||
|
||||
const revokedRecords = await service.listRecords(companyId, revokedFilters, actor);
|
||||
expect(revokedRecords.map((record) => record.id)).toEqual([captured.records[0].id]);
|
||||
});
|
||||
});
|
||||
@@ -924,7 +924,7 @@ export function memoryService(
|
||||
metadata?: Record<string, unknown>;
|
||||
reviewState?: MemoryRecord["reviewState"];
|
||||
},
|
||||
operationId: string,
|
||||
operationId: string | null,
|
||||
) {
|
||||
const scopeType = input.scopeType ?? normalizeScopeType(scope);
|
||||
const scopeId = input.scopeId ?? normalizeScopeId(binding.companyId, scopeType, scope);
|
||||
@@ -1122,6 +1122,27 @@ export function memoryService(
|
||||
});
|
||||
}
|
||||
|
||||
async function attachCreatedByOperation(
|
||||
companyId: string,
|
||||
records: MemoryRecord[],
|
||||
operationId: string,
|
||||
) {
|
||||
if (records.length === 0) return records;
|
||||
await db
|
||||
.update(memoryLocalRecords)
|
||||
.set({
|
||||
createdByOperationId: operationId,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(
|
||||
and(
|
||||
eq(memoryLocalRecords.companyId, companyId),
|
||||
inArray(memoryLocalRecords.id, records.map((record) => record.id)),
|
||||
),
|
||||
);
|
||||
return records.map((record) => ({ ...record, createdByOperationId: operationId }));
|
||||
}
|
||||
|
||||
function buildLocalBasicConditions(companyId: string, filters: MemoryListRecordsQuery, actor: ActorInfo) {
|
||||
const conditions = [eq(memoryLocalRecords.companyId, companyId)];
|
||||
if (filters.bindingId) conditions.push(eq(memoryLocalRecords.bindingId, filters.bindingId));
|
||||
@@ -1159,7 +1180,9 @@ export function memoryService(
|
||||
if (!filters.includeRevoked) conditions.push(isNull(memoryLocalRecords.revokedAt));
|
||||
if (!filters.includeExpired) {
|
||||
conditions.push(or(isNull(memoryLocalRecords.expiresAt), sql`${memoryLocalRecords.expiresAt} > now()`)!);
|
||||
conditions.push(or(isNull(memoryLocalRecords.retentionState), eq(memoryLocalRecords.retentionState, "active"))!);
|
||||
if (!filters.retentionState) {
|
||||
conditions.push(or(isNull(memoryLocalRecords.retentionState), eq(memoryLocalRecords.retentionState, "active"))!);
|
||||
}
|
||||
}
|
||||
if (!filters.includeSuperseded) conditions.push(isNull(memoryLocalRecords.supersededByRecordId));
|
||||
|
||||
@@ -2257,7 +2280,7 @@ export function memoryService(
|
||||
metadata: data.metadata ?? {},
|
||||
reviewState: data.reviewState,
|
||||
},
|
||||
operationId,
|
||||
null,
|
||||
);
|
||||
} else {
|
||||
if (!pluginMemoryProviders) {
|
||||
@@ -2284,7 +2307,7 @@ export function memoryService(
|
||||
records = providerResult.records;
|
||||
usage = providerResult.usage ?? [];
|
||||
providerResultJson = providerResult.resultJson ?? null;
|
||||
await persistCatalogRecords(binding, records, operationId);
|
||||
await persistCatalogRecords(binding, records, null);
|
||||
}
|
||||
const operation = await logOperation({
|
||||
id: operationId,
|
||||
@@ -2318,6 +2341,7 @@ export function memoryService(
|
||||
recordCount: records.length,
|
||||
usage,
|
||||
});
|
||||
records = await attachCreatedByOperation(companyId, records, operation.id);
|
||||
return { operation, records } satisfies MemoryCaptureResult;
|
||||
},
|
||||
|
||||
@@ -2537,7 +2561,7 @@ export function memoryService(
|
||||
reviewNote: parsed.reason,
|
||||
citationJson: parsed.citation === undefined ? originalRow.citationJson : normalizeCitation(parsed.citation),
|
||||
supersedesRecordId: originalRow.id,
|
||||
createdByOperationId: operationId,
|
||||
createdByOperationId: null,
|
||||
createdAt: correctedAt,
|
||||
updatedAt: correctedAt,
|
||||
})
|
||||
@@ -2568,11 +2592,16 @@ export function memoryService(
|
||||
},
|
||||
recordCount: 1,
|
||||
});
|
||||
const [attachedCorrectedRow] = await db
|
||||
.update(memoryLocalRecords)
|
||||
.set({ createdByOperationId: operation.id, updatedAt: correctedAt })
|
||||
.where(and(eq(memoryLocalRecords.companyId, companyId), eq(memoryLocalRecords.id, correctedRecordId)))
|
||||
.returning();
|
||||
|
||||
return {
|
||||
operation,
|
||||
originalRecord: mapRecord({ ...originalRow, supersededByRecordId: correctedRecordId, updatedAt: correctedAt }),
|
||||
correctedRecord: mapRecord(correctedRow),
|
||||
correctedRecord: mapRecord(attachedCorrectedRow ?? { ...correctedRow, createdByOperationId: operation.id }),
|
||||
} satisfies MemoryCorrectResult;
|
||||
},
|
||||
|
||||
|
||||
Reference in new issue
Block a user