Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/triggers/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1889,6 +1889,16 @@ export function registerApiTriggers(
: undefined;
const offset =
Number.isInteger(parsedOffset) && parsedOffset >= 0 ? parsedOffset : 0;
// kv.list returns rows in insertion order (oldest first), so slicing the
// head served the OLDEST memories — the viewer then showed stale data once
// the corpus exceeded the limit (#990). Sort newest-first (createdAt desc,
// fallback updatedAt) before the slice, mirroring the viewer-side fix from
// #674/#701.
filtered.sort((a, b) => {
const aKey = a.createdAt || a.updatedAt || "";
const bKey = b.createdAt || b.updatedAt || "";
return bKey.localeCompare(aKey);
});
const sliced =
limit !== undefined ? filtered.slice(offset, offset + limit) : filtered;

Expand Down
40 changes: 40 additions & 0 deletions test/memories-latest-sort-order.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, it, expect } from "vitest";
import { readFileSync } from "node:fs";

// api::memories returned rows in KV-insertion order (oldest first), so
// `?latest=true&limit=N` served the N OLDEST "latest" memories and the viewer
// showed stale data once the corpus exceeded the limit (#990). The handler now
// sorts newest-first (createdAt desc, fallback updatedAt) before it slices,
// mirroring the viewer-side fix that already landed for the Memories tab
// (#674/#701).
//
// Like the other api::memories tests in this repo, this asserts on the handler
// source: the SDK-registered handler is not invoked in isolation here, so the
// guard is that the sort exists, uses the agreed keys, and runs BEFORE the
// slice (slicing an unsorted list is the bug).
describe("api::memories sorts newest first before pagination (#990)", () => {
const api = readFileSync("src/triggers/api.ts", "utf-8");
const start = api.indexOf('registerFunction("api::memories"');
const end = api.indexOf('config: { api_path: "/agentmemory/memories"');
const handler = api.slice(start, end);

it("isolates the api::memories handler source", () => {
expect(start).toBeGreaterThan(-1);
expect(end).toBeGreaterThan(start);
});

it("sorts filtered memories before the offset/limit slice", () => {
const sortIdx = handler.search(/filtered\.sort\(/);
const sliceIdx = handler.search(/filtered\.slice\(offset/);
expect(sortIdx).toBeGreaterThan(-1);
expect(sliceIdx).toBeGreaterThan(-1);
// Slicing before sorting is the #990 bug — the sort must come first.
expect(sortIdx).toBeLessThan(sliceIdx);
});

it("sorts on createdAt desc with updatedAt fallback (matches the #701 viewer fix)", () => {
expect(handler).toMatch(/a\.createdAt \|\| a\.updatedAt/);
expect(handler).toMatch(/b\.createdAt \|\| b\.updatedAt/);
expect(handler).toMatch(/localeCompare/);
Comment on lines +35 to +38

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the comparator direction explicitly.

This still passes if the implementation flips to aKey.localeCompare(bKey), which would reintroduce oldest-first ordering. Check for bKey.localeCompare(aKey) (or evaluate the comparator on sample timestamps) so the regression actually locks newest-first behavior.

Suggested tightening
   it("sorts on createdAt desc with updatedAt fallback (matches the `#701` viewer fix)", () => {
     expect(handler).toMatch(/a\.createdAt \|\| a\.updatedAt/);
     expect(handler).toMatch(/b\.createdAt \|\| b\.updatedAt/);
-    expect(handler).toMatch(/localeCompare/);
+    expect(handler).toMatch(/bKey\.localeCompare\(aKey\)/);
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("sorts on createdAt desc with updatedAt fallback (matches the #701 viewer fix)", () => {
expect(handler).toMatch(/a\.createdAt \|\| a\.updatedAt/);
expect(handler).toMatch(/b\.createdAt \|\| b\.updatedAt/);
expect(handler).toMatch(/localeCompare/);
it("sorts on createdAt desc with updatedAt fallback (matches the `#701` viewer fix)", () => {
expect(handler).toMatch(/a\.createdAt \|\| a\.updatedAt/);
expect(handler).toMatch(/b\.createdAt \|\| b\.updatedAt/);
expect(handler).toMatch(/bKey\.localeCompare\(aKey\)/);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/memories-latest-sort-order.test.ts` around lines 35 - 38, The test in
memories-latest-sort-order.test.ts only checks for localeCompare usage, so it
can still pass if the comparator direction is reversed. Update the assertion
around the sort comparator in the relevant test to explicitly verify
newest-first ordering by checking for bKey.localeCompare(aKey) in the comparator
logic, or by asserting the comparator result on sample timestamps through the
handler used in the test. This should be anchored to the existing handler and
sort-order expectations so the regression lock targets the comparator direction
itself.

});
});