feat: Added API support for BE endpoints#12423
Merged
Merged
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughRemoves local mock memories infrastructure and refactors the frontend memories integration: adds DTO mapping and types, converts list queries to paginated infinite queries, replaces custom request processors with TanStack React Query hooks and direct cache updates, and shifts memory selection and debounced auto-capture state into components/hooks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 4 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
@CodeRabbit summary |
Contributor
✅ Actions performedSummary regeneration triggered. |
Codecov Report❌ Patch coverage is ❌ Your project check has failed because the head coverage (50.61%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## release-1.10.0 #12423 +/- ##
==================================================
+ Coverage 54.03% 54.26% +0.23%
==================================================
Files 2081 2090 +9
Lines 190867 191493 +626
Branches 27255 29027 +1772
==================================================
+ Hits 103137 103920 +783
+ Misses 86575 86418 -157
Partials 1155 1155
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Collaborator
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
Collaborator
Author
|
@CodeRabbit summary |
Contributor
✅ Actions performedSummary regeneration triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (11)
src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDocumentPanel.test.tsx (3)
28-36: Improve type safety by defining a proper type.The
as anycast on Line 35 bypasses TypeScript's type checking. Consider defining a proper type or interface for the test document object.♻️ Type-safe alternative
+type TestDocument = { + message_id: string; + session_id: string; + sender: string; + timestamp: string; + content: string; +}; + it("renders selected document details", () => { render( <MemoryDocumentPanel open onOpenChange={jest.fn()} selectedDocument={ { message_id: "msg-1", session_id: "session-1", sender: "user", timestamp: "2025-01-01T10:00:00.000Z", content: "hello world", - } as any + } as TestDocument } />, );Alternatively, if the component exports a proper type for
selectedDocument, import and use that type instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDocumentPanel.test.tsx` around lines 28 - 36, The test currently uses a loose cast ("as any") for selectedDocument; replace it with a proper typed object by importing the component's exported type (e.g., SelectedDocument or MemoryDocumentPanelProps) or by declaring a small local interface that matches fields (message_id, session_id, sender, timestamp, content) and using that type for the test object in MemoryDocumentPanel.test; update the selectedDocument prop to be typed accordingly instead of using "as any" so TypeScript enforces shape correctness.
10-44: Add test documentation comments.The test cases lack descriptive comments explaining their purpose, scenario, and expected outcomes. While the test names are clear, adding documentation comments would improve maintainability and align with coding guidelines.
📝 Suggested documentation
describe("MemoryDocumentPanel", () => { + /** + * Verifies that the MemoryDocumentPanel displays an empty state message + * when no document/chunk is selected. + * + * Scenario: Panel is open with selectedDocument set to null + * Expected: "No chunk selected." message is displayed + */ it("renders empty state when no document is selected", () => { render( <MemoryDocumentPanel open onOpenChange={jest.fn()} selectedDocument={null} />, ); expect(screen.getByText("No chunk selected.")).toBeInTheDocument(); }); + /** + * Verifies that the MemoryDocumentPanel correctly displays document chunk details + * when a document is selected. + * + * Scenario: Panel is open with a valid document object containing message metadata + * Expected: Shows "Chunk Details" heading and renders message_id and content + */ it("renders selected document details", () => {As per coding guidelines: "Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDocumentPanel.test.tsx` around lines 10 - 44, Add brief docstring-style comments above each test case in the MemoryDocumentPanel test file describing purpose, scenario, and expected outcome: for the first test (the "renders empty state when no document is selected" it should state that when selectedDocument is null and panel is open it should show the empty-state message "No chunk selected."), and for the second test (the "renders selected document details") it should state that when a selectedDocument object is provided the panel shows "Chunk Details" and displays the message_id and content. Place the comments immediately above the respective it(...) blocks so they reference the MemoryDocumentPanel component and the specific assertions in each test.
4-8: Consider using real dialog components and improve type safety.The DialogTitle mock on Line 7 completes the dialog component mocking pattern. However:
- Type safety: Use
React.ReactNodeinstead ofanyfor thechildrenparameter across all mock components.- Real integrations: Per learnings, prefer real integrations over mocks when possible for more reliable tests. Since these are UI components from your component library (not external APIs), consider importing and rendering the actual Dialog components to test real behavior and integration.
♻️ Proposed improvements
Option 1: Improve type safety while keeping mocks
jest.mock("@/components/ui/dialog", () => ({ - Dialog: ({ children }: any) => <div>{children}</div>, - DialogContent: ({ children }: any) => <div>{children}</div>, - DialogTitle: ({ children }: any) => <div>{children}</div>, + Dialog: ({ children }: { children: React.ReactNode }) => <div>{children}</div>, + DialogContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>, + DialogTitle: ({ children }: { children: React.ReactNode }) => <div>{children}</div>, }));Option 2 (Preferred): Remove mocks and use real components
-jest.mock("@/components/ui/dialog", () => ({ - Dialog: ({ children }: any) => <div>{children}</div>, - DialogContent: ({ children }: any) => <div>{children}</div>, - DialogTitle: ({ children }: any) => <div>{children}</div>, -}));Then verify the tests still pass with the real Dialog components. If the real components require additional setup (like providers), add that setup to the test.
Based on learnings: "Avoid mocking in tests when possible; prefer real integrations for more reliable tests"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDocumentPanel.test.tsx` around lines 4 - 8, Replace the inline mock of Dialog/DialogContent/DialogTitle in MemoryDocumentPanel.test.tsx: either (a) make the mock typesafe by changing the children param type from any to React.ReactNode for the mocked components (Dialog, DialogContent, DialogTitle) or (preferred) remove the jest.mock block entirely and import the real Dialog components from "@/components/ui/dialog" so tests exercise real behavior; if using real components, add any required providers/wrappers (theme, context, or portal setup) to the test render so Dialog renders correctly.src/frontend/src/pages/FlowPage/components/MemoriesMainContent/__tests__/helpers.test.ts (2)
39-46: Add one regression case for spaced timezone suffixes.Please add a case like
"2025-01-15 10:30:00 GMT+0000 (UTC)"to validate normalization behavior when timezone tokens contain spaces.Based on learnings: "Applies to **/*.test.ts?(x) : Consider including edge cases and error conditions for comprehensive test coverage".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/__tests__/helpers.test.ts` around lines 39 - 46, Add a regression test that passes a datetime string containing a spaced timezone token (e.g. "2025-01-15 10:30:00 GMT+0000 (UTC)") to the existing formatDate and formatTimestamp assertions to validate normalization; update the test in the same it("formats valid dates", ...) block to call formatDate("2025-01-15 10:30:00 GMT+0000 (UTC)") and formatTimestamp("2025-01-15 10:30:00 GMT+0000 (UTC)") and assert the same normalized outputs ("Jan 15, 10:30" for formatDate and "Jan 15, 10:30:00" for formatTimestamp) so the functions formatDate and formatTimestamp handle spaced timezone suffixes.
9-27: Usevi.useFakeTimersfor per-test Date mocking instead of global prototype mutation.Overriding
Date.prototype.toLocaleStringinbeforeAll/afterAllcauses global side effects that can leak across other tests in the same worker. Usevi.useFakeTimerswithbeforeEach/afterEachinstead—this is the official Vitest pattern for deterministic date/time testing.♻️ Suggested change
- const originalToLocaleString = Date.prototype.toLocaleString; - - beforeAll(() => { - // Force deterministic formatting regardless of machine locale/timezone. - Date.prototype.toLocaleString = function ( - _locales?: Intl.LocalesArgument, - options?: Intl.DateTimeFormatOptions, - ) { - return originalToLocaleString.call(this, "en-US", { - ...(options ?? {}), - timeZone: "UTC", - hour12: false, - }); - }; - }); - - afterAll(() => { - Date.prototype.toLocaleString = originalToLocaleString; - }); + beforeEach(() => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date("2025-01-15T00:00:00Z")); + }); + + afterEach(() => { + vi.useRealTimers(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/__tests__/helpers.test.ts` around lines 9 - 27, The test currently mutates Date.prototype.toLocaleString in beforeAll/afterAll using originalToLocaleString, which causes global side effects; switch to using Vitest's vi.useFakeTimers/vi.setSystemTime within beforeEach and restore with vi.useRealTimers in afterEach to get deterministic UTC date/time per test instead of overriding Date.prototype.toLocaleString globally (locate the setup in the test file where Date.prototype.toLocaleString, originalToLocaleString, beforeAll and afterAll are used and replace that logic with vi.useFakeTimers/vi.setSystemTime + vi.useRealTimers in beforeEach/afterEach).src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/__tests__/useMemoriesData.test.tsx (1)
176-225: Move fake-timer cleanup into shared teardown.If either test fails before the footer runs, the suite stays on fake timers and later cases start failing for unrelated reasons. Reset timers in
afterEach(ortry/finally) instead of inside each test body. Based on learnings: Tests should be organized logically with proper setup and teardown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/__tests__/useMemoriesData.test.tsx` around lines 176 - 225, The tests set fake timers inside individual it blocks but call jest.useRealTimers() only at the end of each test, which can leave the suite running with fake timers if a test fails; add a shared teardown by adding afterEach(() => jest.useRealTimers()) (or a try/finally around tests) in this test file so timers are always restored, and remove the per-test jest.useRealTimers() calls; update references to useMemoriesData, handleToggleActive, and updateMemoryMutation in the tests to keep their current behavior while relying on the global afterEach cleanup.src/frontend/src/controllers/API/queries/memories/use-create-memory.ts (2)
130-133: Invalidating mutation key["useCreateMemory"]has no effect.This invalidation targets the mutation's own key, not a query key. If cache staleness is a concern, consider invalidating relevant query keys or removing this line since the
onSuccesshandler already updates caches directly.Proposed fix
onSettled: (data, error, variables, context) => { - // Keep parity with other mutations that used to invalidate onSettled. - queryClient.invalidateQueries({ queryKey: ["useCreateMemory"] }); userOnSettled?.(data, error, variables, context); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/memories/use-create-memory.ts` around lines 130 - 133, The invalidateQueries call inside use-create-memory's onSettled is targeting the mutation key ["useCreateMemory"] which does nothing; remove that line (queryClient.invalidateQueries({ queryKey: ["useCreateMemory"] })) or replace it with invalidation of the actual query keys your caches use (e.g. queryClient.invalidateQueries({ queryKey: ["memories"] }) or the specific query keys updated in onSuccess). Update the onSettled in the useCreateMemory hook to either drop the invalidation or target the relevant query keys so cache staleness is handled correctly; keep the existing userOnSettled invocation.
66-66: Excessive use ofanytype reduces type safety.Consider using proper types or
unknownwith type guards instead of casting toany. The infinite query data structure is well-defined and could be typed explicitly.Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/memories/use-create-memory.ts` at line 66, Replace the unsafe cast "const anyOld = old as any" in use-create-memory (and similar casts) with a properly typed representation of the infinite query result (or use "unknown" + runtime type guards) so you preserve type-safety when reading/updating pages and items; locate the code around the useCreateMemory mutation/update logic (symbols: useCreateMemory, old, anyOld) and define an explicit interface for the infinite query shape (pages array, nextCursor, items/memories) or narrow "old" with a type-guard function before accessing its properties, then update subsequent uses (e.g., the code that maps/updates pages) to use the correctly typed value instead of any.src/frontend/src/controllers/API/queries/memories/use-delete-memory.ts (2)
35-98: Cache updates performed insidemutationFnrisk inconsistency on partial failure.If an error occurs after the API call succeeds but before
deleteMemoryFncompletes, the cache will have been modified but the caller won't receive the success signal. Consider moving cache updates toonSuccessfor transactional integrity, consistent with standard React Query patterns.Suggested approach
const deleteMemoryFn = async (params: DeleteMemoryParams): Promise<void> => { await api.delete(`${getURL("MEMORIES")}/${params.memoryId}`); - - // Avoid refetching a resource that no longer exists. - await queryClient.cancelQueries({ - queryKey: ["useGetMemory", params.memoryId], - }); - queryClient.removeQueries({ queryKey: ["useGetMemory", params.memoryId] }); - - // Keep cached lists consistent without forcing a refetch. - queryClient.setQueriesData( - { queryKey: ["useGetMemoriesInfinite"] }, - (old: unknown) => { - // ... cache update logic - }, - ); }; const mutation = useMutation<void, unknown, DeleteMemoryParams, unknown>({ mutationKey: ["useDeleteMemory"], mutationFn: deleteMemoryFn, ...restOptions, + onSuccess: (data, variables) => { + queryClient.cancelQueries({ + queryKey: ["useGetMemory", variables.memoryId], + }); + queryClient.removeQueries({ queryKey: ["useGetMemory", variables.memoryId] }); + // ... cache update logic here + }, onSettled: (data, error, variables, context) => { userOnSettled?.(data, error, variables, context); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/memories/use-delete-memory.ts` around lines 35 - 98, The cache mutations are being performed inside deleteMemoryFn (the mutation function) which risks partial-success inconsistency; change deleteMemoryFn to only perform the API call (await api.delete(...)) and move all queryClient actions (cancelQueries, removeQueries, setQueriesData logic that touches ["useGetMemory", params.memoryId] and ["useGetMemoriesInfinite"]) into the mutation's onSuccess handler so cache updates run only after a confirmed successful delete; update any mutation creation (where deleteMemoryFn is used) to provide the onSuccess callback that implements the existing removal/filtering logic currently in deleteMemoryFn.
26-33: Helper functions recreated on every render.
isRecordandgetStringIdare defined inside the hook body, causing new function instances on each render. Consider moving them outside the hook or memoizing them.Proposed refactor
+const isRecord = (value: unknown): value is Record<string, unknown> => + typeof value === "object" && value !== null; + +const getStringId = (value: unknown): string | undefined => { + if (!isRecord(value)) return undefined; + const id = value.id; + return typeof id === "string" ? id : undefined; +}; + export const useDeleteMemory: useMutationFunctionType< undefined, DeleteMemoryParams, void, unknown > = (options?) => { const queryClient = useQueryClient(); // ... rest of the hook - const isRecord = (value: unknown): value is Record<string, unknown> => - typeof value === "object" && value !== null; - - const getStringId = (value: unknown): string | undefined => { - if (!isRecord(value)) return undefined; - const id = value.id; - return typeof id === "string" ? id : undefined; - };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/memories/use-delete-memory.ts` around lines 26 - 33, The helper functions isRecord and getStringId are recreated on every render inside the hook; move them to module scope (outside the hook) so they are defined once and reused, or alternatively memoize them with useCallback if they must capture hook state — update references in use-delete-memory.ts to call the module-level isRecord and getStringId (or the memoized callbacks) instead of local declarations to avoid allocating new function instances each render.src/frontend/src/controllers/API/queries/memories/use-update-memory.ts (1)
27-34: Duplicate helper functions across hooks.
isRecordandgetStringIdare duplicated inuse-delete-memory.tsanduse-update-memory.ts. Consider extracting them to a shared utility module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/memories/use-update-memory.ts` around lines 27 - 34, The helpers isRecord and getStringId are duplicated across hooks; extract them into a single shared utility (e.g., create a new helper file exporting isRecord and getStringId) and replace the local definitions in use-update-memory.ts and use-delete-memory.ts with imports from that shared module; update the hooks to import the functions and remove the inline declarations so both hooks call the same exported isRecord and getStringId symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frontend/src/controllers/API/queries/memories/mappers.ts`:
- Around line 28-30: DEFAULTS currently holds shared array instances for
documents and document_sessions that get shallow-copied into each MemoryInfo via
spread, causing mutations in one mapped result to leak into others; update the
mapper that uses DEFAULTS (and any code in the block referenced around lines
33-48) to create fresh arrays per mapped object instead of reusing the shared
instances — e.g., replace direct spreading of DEFAULTS.documents /
DEFAULTS.document_sessions with explicit new arrays for each output (like
documents: [...(DEFAULTS.documents || [])] or documents: [], and similarly for
document_sessions), or convert DEFAULTS into a factory function that returns a
new object with fresh arrays for documents and document_sessions.
In `@src/frontend/src/controllers/API/queries/memories/use-create-memory.ts`:
- Line 46: The mutation callback signatures are wrong: update the options passed
to useMutation in use-create-memory so onSuccess uses three params (data,
variables, context) not four, and onSettled uses four params (data, error,
variables, context) not five; locate the onSuccess and onSettled handlers in the
useCreateMemory hook and change their parameter lists accordingly (remove the
extra param in onSuccess and remove the extra param in onSettled) and adjust any
internal references to those parameters to use the correct names (data,
variables, context for onSuccess; data, error, variables, context for
onSettled).
In `@src/frontend/src/controllers/API/queries/memories/use-delete-memory.ts`:
- Around line 105-106: The onSettled callback in the useDeleteMemory mutation is
using an incorrect signature with five parameters (including onMutateResult);
update the onSettled definition in useDeleteMemory to the correct React Query
signature (data, error, variables, context) and call userOnSettled with those
four arguments (i.e., userOnSettled?.(data, error, variables, context)),
removing the extra onMutateResult parameter so it matches the expected
useMutation onSettled parameters.
In `@src/frontend/src/controllers/API/queries/memories/use-get-memories.ts`:
- Around line 43-52: The hook is constructing an absolute URL with
window.location.origin which bypasses the axios baseURL; change
use-get-memories.ts to build query parameters on the relative path returned by
getURL("MEMORIES") (e.g., use URL or URLSearchParams with getURL("MEMORIES") as
the base) and pass that relative URL string into api.get instead of an absolute
url; update the code around getURL("MEMORIES"), url.searchParams.set("flow_id",
flowId), pageParam and DEFAULT_PAGE_SIZE so api.get<GetMemoriesApiResponse>(...)
receives a relative path and then return mapGetMemoriesApiResponse(res.data) as
before.
In `@src/frontend/src/controllers/API/queries/memories/use-update-memory.ts`:
- Around line 101-103: Fix the onSettled handler signature and the invalidation
target: change the onSettled callback in useUpdateMemory to use the correct
four-arg signature (data, error, variables, context) instead of a five-arg form,
and stop invalidating the mutation's own key; either remove
invalidateQueries(["useUpdateMemory"]) or replace it with the appropriate query
key(s) that should refetch (for example ["useGetMemoriesInfinite"]) if you need
a refetch — keep the direct cache updates intact if refetching is unnecessary.
Ensure you update the function referenced as onSettled in useUpdateMemory and
the call to queryClient.invalidateQueries to reference the correct query key(s).
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoriesSidebar.tsx`:
- Around line 22-29: The current pagination only triggers inside handleScroll
for the filteredMemories container, so when filtering reduces the list height
there is no scroll event and fetchNextPage never runs; update the component to
also perform the same "remaining < 160" check whenever the filteredMemories list
or mount changes (e.g. in a useEffect that reads filteredMemories, hasNextPage,
isFetchingNextPage and calls fetchNextPage when needed), or replace the
scroll-based approach with a sentinel + IntersectionObserver that calls
fetchNextPage when the sentinel becomes visible; reference handleScroll,
filteredMemories container, fetchNextPage, hasNextPage and isFetchingNextPage
when implementing the fix.
In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.ts`:
- Around line 35-40: The IIFE that builds normalized currently collapses all
whitespace with parts.slice(1).join("") which removes spaces (e.g., "GMT") and
can break timezone formats; update the normalized computation inside the IIFE
(the block that reads `const normalized = (() => { ... })();`, referencing
variables `trimmed` and `parts`) to preserve spacing between the non-date/time
tail (use parts.slice(1).join(" ") or otherwise reinsert a single space) so the
remainder after the time token is kept intact, while keeping the existing
parse-fallback logic unchanged.
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/fixedPrefillMessages.ts`:
- Around line 1-35: The FIXED_PREFILL_MESSAGES fixture is being used
unconditionally as a fallback in the production memory flow (imported/consumed
by useMemoriesData), causing fabricated conversation records to appear when a
memory has no messages; remove the unconditional use of FIXED_PREFILL_MESSAGES
by either relocating this constant to a test/dev-only module or wrapping its
usage behind an environment guard (e.g., only use FIXED_PREFILL_MESSAGES when
process.env.NODE_ENV !== 'production' or a testing flag is set), and update
useMemoriesData to render a real empty state or appropriate UI when messages are
missing instead of injecting the synthetic messages.
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts`:
- Line 209: The useMemo for docsData in useMemoriesData.ts currently lists
flowMessages in its dependency array but does not reference it in the memo body,
causing unnecessary recalculations; update the docsData memo by either removing
flowMessages from the dependency array (so dependencies become [memory,
activeSearch]) or, if flowMessages was intended to affect docsData, incorporate
flowMessages into the memo computation (referencing the flowMessages symbol
inside the memo) so the dependency is meaningful.
- Around line 165-177: The mapping from prefillMessages to MemoryDocumentItem is
producing empty strings for required fields; update the logic around
rawDocuments (and the prefillMessages handling) to validate and/or supply
meaningful defaults: first filter prefillMessages for entries where required
fields are present/non-empty (e.g., m.text, m.id, m.session_id, m.sender,
m.timestamp) and only map those into MemoryDocumentItem, or if you prefer
defaults, provide explicit sane defaults (e.g., sender = "unknown", session_id =
generated/empty-safe value, timestamp = current ISO) instead of relying on
String() coercion; apply this change in the block that references
memoryDocuments, prefillMessages, and rawDocuments so downstream consumers
receive fully populated MemoryDocumentItem objects.
In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/index.tsx`:
- Around line 17-24: The selectedMemoryId can be stale for one render when
currentFlowId changes, causing useGetMemory/useMemoriesData to receive an old
ID; change the selection state to include the flow context or gate hooks by flow
so the hook never sees a mismatched id — e.g., replace selectedMemoryId with a
composite state like { flowId: currentFlowId, memoryId: string | null } or
initialize/reset selection synchronously based on currentFlowId (update
setSelectedMemoryId/onSelectMemory to manage the composite key) and ensure
useGetMemory/useMemoriesData read the memoryId only when flowId matches
currentFlowId.
---
Nitpick comments:
In `@src/frontend/src/controllers/API/queries/memories/use-create-memory.ts`:
- Around line 130-133: The invalidateQueries call inside use-create-memory's
onSettled is targeting the mutation key ["useCreateMemory"] which does nothing;
remove that line (queryClient.invalidateQueries({ queryKey: ["useCreateMemory"]
})) or replace it with invalidation of the actual query keys your caches use
(e.g. queryClient.invalidateQueries({ queryKey: ["memories"] }) or the specific
query keys updated in onSuccess). Update the onSettled in the useCreateMemory
hook to either drop the invalidation or target the relevant query keys so cache
staleness is handled correctly; keep the existing userOnSettled invocation.
- Line 66: Replace the unsafe cast "const anyOld = old as any" in
use-create-memory (and similar casts) with a properly typed representation of
the infinite query result (or use "unknown" + runtime type guards) so you
preserve type-safety when reading/updating pages and items; locate the code
around the useCreateMemory mutation/update logic (symbols: useCreateMemory, old,
anyOld) and define an explicit interface for the infinite query shape (pages
array, nextCursor, items/memories) or narrow "old" with a type-guard function
before accessing its properties, then update subsequent uses (e.g., the code
that maps/updates pages) to use the correctly typed value instead of any.
In `@src/frontend/src/controllers/API/queries/memories/use-delete-memory.ts`:
- Around line 35-98: The cache mutations are being performed inside
deleteMemoryFn (the mutation function) which risks partial-success
inconsistency; change deleteMemoryFn to only perform the API call (await
api.delete(...)) and move all queryClient actions (cancelQueries, removeQueries,
setQueriesData logic that touches ["useGetMemory", params.memoryId] and
["useGetMemoriesInfinite"]) into the mutation's onSuccess handler so cache
updates run only after a confirmed successful delete; update any mutation
creation (where deleteMemoryFn is used) to provide the onSuccess callback that
implements the existing removal/filtering logic currently in deleteMemoryFn.
- Around line 26-33: The helper functions isRecord and getStringId are recreated
on every render inside the hook; move them to module scope (outside the hook) so
they are defined once and reused, or alternatively memoize them with useCallback
if they must capture hook state — update references in use-delete-memory.ts to
call the module-level isRecord and getStringId (or the memoized callbacks)
instead of local declarations to avoid allocating new function instances each
render.
In `@src/frontend/src/controllers/API/queries/memories/use-update-memory.ts`:
- Around line 27-34: The helpers isRecord and getStringId are duplicated across
hooks; extract them into a single shared utility (e.g., create a new helper file
exporting isRecord and getStringId) and replace the local definitions in
use-update-memory.ts and use-delete-memory.ts with imports from that shared
module; update the hooks to import the functions and remove the inline
declarations so both hooks call the same exported isRecord and getStringId
symbols.
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/__tests__/helpers.test.ts`:
- Around line 39-46: Add a regression test that passes a datetime string
containing a spaced timezone token (e.g. "2025-01-15 10:30:00 GMT+0000 (UTC)")
to the existing formatDate and formatTimestamp assertions to validate
normalization; update the test in the same it("formats valid dates", ...) block
to call formatDate("2025-01-15 10:30:00 GMT+0000 (UTC)") and
formatTimestamp("2025-01-15 10:30:00 GMT+0000 (UTC)") and assert the same
normalized outputs ("Jan 15, 10:30" for formatDate and "Jan 15, 10:30:00" for
formatTimestamp) so the functions formatDate and formatTimestamp handle spaced
timezone suffixes.
- Around line 9-27: The test currently mutates Date.prototype.toLocaleString in
beforeAll/afterAll using originalToLocaleString, which causes global side
effects; switch to using Vitest's vi.useFakeTimers/vi.setSystemTime within
beforeEach and restore with vi.useRealTimers in afterEach to get deterministic
UTC date/time per test instead of overriding Date.prototype.toLocaleString
globally (locate the setup in the test file where Date.prototype.toLocaleString,
originalToLocaleString, beforeAll and afterAll are used and replace that logic
with vi.useFakeTimers/vi.setSystemTime + vi.useRealTimers in
beforeEach/afterEach).
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDocumentPanel.test.tsx`:
- Around line 28-36: The test currently uses a loose cast ("as any") for
selectedDocument; replace it with a proper typed object by importing the
component's exported type (e.g., SelectedDocument or MemoryDocumentPanelProps)
or by declaring a small local interface that matches fields (message_id,
session_id, sender, timestamp, content) and using that type for the test object
in MemoryDocumentPanel.test; update the selectedDocument prop to be typed
accordingly instead of using "as any" so TypeScript enforces shape correctness.
- Around line 10-44: Add brief docstring-style comments above each test case in
the MemoryDocumentPanel test file describing purpose, scenario, and expected
outcome: for the first test (the "renders empty state when no document is
selected" it should state that when selectedDocument is null and panel is open
it should show the empty-state message "No chunk selected."), and for the second
test (the "renders selected document details") it should state that when a
selectedDocument object is provided the panel shows "Chunk Details" and displays
the message_id and content. Place the comments immediately above the respective
it(...) blocks so they reference the MemoryDocumentPanel component and the
specific assertions in each test.
- Around line 4-8: Replace the inline mock of Dialog/DialogContent/DialogTitle
in MemoryDocumentPanel.test.tsx: either (a) make the mock typesafe by changing
the children param type from any to React.ReactNode for the mocked components
(Dialog, DialogContent, DialogTitle) or (preferred) remove the jest.mock block
entirely and import the real Dialog components from "@/components/ui/dialog" so
tests exercise real behavior; if using real components, add any required
providers/wrappers (theme, context, or portal setup) to the test render so
Dialog renders correctly.
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/__tests__/useMemoriesData.test.tsx`:
- Around line 176-225: The tests set fake timers inside individual it blocks but
call jest.useRealTimers() only at the end of each test, which can leave the
suite running with fake timers if a test fails; add a shared teardown by adding
afterEach(() => jest.useRealTimers()) (or a try/finally around tests) in this
test file so timers are always restored, and remove the per-test
jest.useRealTimers() calls; update references to useMemoriesData,
handleToggleActive, and updateMemoryMutation in the tests to keep their current
behavior while relying on the global afterEach cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a26b4a5-cf0c-408e-82b9-8fb229171801
📒 Files selected for processing (31)
src/frontend/src/controllers/API/mocks/memories.helpers.tssrc/frontend/src/controllers/API/mocks/memories.tssrc/frontend/src/controllers/API/queries/memories/mappers.tssrc/frontend/src/controllers/API/queries/memories/types.tssrc/frontend/src/controllers/API/queries/memories/use-add-messages-to-memory.tssrc/frontend/src/controllers/API/queries/memories/use-create-memory.tssrc/frontend/src/controllers/API/queries/memories/use-delete-memory.tssrc/frontend/src/controllers/API/queries/memories/use-get-memories.tssrc/frontend/src/controllers/API/queries/memories/use-get-memory.tssrc/frontend/src/controllers/API/queries/memories/use-update-memory.tssrc/frontend/src/modals/createMemoryModal/__tests__/useCreateMemoryModal.test.tsxsrc/frontend/src/modals/createMemoryModal/index.tsxsrc/frontend/src/modals/createMemoryModal/useCreateMemoryModal.tssrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/__tests__/helpers.test.tssrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/__tests__/index.test.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoriesSidebar.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryDetails.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryDetailsHeader.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryKnowledgeBaseSection.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryStatusBanners.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/NoMemorySelected.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDetailsHeader.test.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryDocumentPanel.test.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/__tests__/MemoryStatusBanners.test.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.tssrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/__tests__/useMemoriesData.test.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/fixedPrefillMessages.tssrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.tssrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/index.tsxsrc/frontend/src/pages/FlowPage/components/MemoriesMainContent/types.tssrc/frontend/src/pages/FlowPage/index.tsx
💤 Files with no reviewable changes (5)
- src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/tests/MemoryStatusBanners.test.tsx
- src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryStatusBanners.tsx
- src/frontend/src/pages/FlowPage/components/MemoriesMainContent/components/MemoryDetails.tsx
- src/frontend/src/controllers/API/mocks/memories.ts
- src/frontend/src/controllers/API/mocks/memories.helpers.ts
Comment on lines
+28
to
+30
| documents: [], | ||
| documents_total: 0, | ||
| document_sessions: [], |
Contributor
There was a problem hiding this comment.
Don’t reuse shared default arrays across mapped memories.
documents and document_sessions inside DEFAULTS are single array instances. Every ...DEFAULTS spread copies those same references into each MemoryInfo, so any in-place mutation on one mapped memory leaks into every other result produced by this mapper.
🛠️ Create fresh arrays per mapped object
const DEFAULTS: Omit<
MemoryInfo,
| "id"
| "name"
| "kb_name"
| "embedding_model"
| "user_id"
| "flow_id"
| "created_at"
> = {
description: undefined,
embedding_provider: "",
is_active: true,
status: "idle",
error_message: undefined,
total_messages_processed: 0,
total_chunks: 0,
sessions_count: 0,
batch_size: 1,
preprocessing_enabled: false,
preprocessing_model: undefined,
preproc_instructions: undefined,
pending_messages_count: 0,
updated_at: undefined,
last_generated_at: undefined,
- documents: [],
documents_total: 0,
- document_sessions: [],
};
export const mapMemoryApiToMemoryInfo = (dto: MemoryApiDTO): MemoryInfo => {
return {
...DEFAULTS,
+ documents: [],
+ document_sessions: [],
id: dto.id,
name: dto.name,
kb_name: dto.kb_name,Also applies to: 33-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/controllers/API/queries/memories/mappers.ts` around lines 28
- 30, DEFAULTS currently holds shared array instances for documents and
document_sessions that get shallow-copied into each MemoryInfo via spread,
causing mutations in one mapped result to leak into others; update the mapper
that uses DEFAULTS (and any code in the block referenced around lines 33-48) to
create fresh arrays per mapped object instead of reusing the shared instances —
e.g., replace direct spreading of DEFAULTS.documents /
DEFAULTS.document_sessions with explicit new arrays for each output (like
documents: [...(DEFAULTS.documents || [])] or documents: [], and similarly for
document_sessions), or convert DEFAULTS into a factory function that returns a
new object with fresh arrays for documents and document_sessions.
| mutationKey: ["useCreateMemory"], | ||
| mutationFn: createMemoryFn, | ||
| ...restOptions, | ||
| onSuccess: (data, variables, onMutateResult, context) => { |
Contributor
There was a problem hiding this comment.
Incorrect callback signatures for onSuccess and onSettled.
onSuccessreceives 3 parameters:(data, variables, context), not 4.onSettledreceives 4 parameters:(data, error, variables, context), not 5.
Proposed fix
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
// ... existing logic ...
- userOnSuccess?.(data, variables, onMutateResult, context);
+ userOnSuccess?.(data, variables, context);
},
- onSettled: (data, error, variables, onMutateResult, context) => {
+ onSettled: (data, error, variables, context) => {
// Keep parity with other mutations that used to invalidate onSettled.
queryClient.invalidateQueries({ queryKey: ["useCreateMemory"] });
- userOnSettled?.(data, error, variables, onMutateResult, context);
+ userOnSettled?.(data, error, variables, context);
},Also applies to: 128-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/controllers/API/queries/memories/use-create-memory.ts` at
line 46, The mutation callback signatures are wrong: update the options passed
to useMutation in use-create-memory so onSuccess uses three params (data,
variables, context) not four, and onSettled uses four params (data, error,
variables, context) not five; locate the onSuccess and onSettled handlers in the
useCreateMemory hook and change their parameter lists accordingly (remove the
extra param in onSuccess and remove the extra param in onSettled) and adjust any
internal references to those parameters to use the correct names (data,
variables, context for onSuccess; data, error, variables, context for
onSettled).
Comment on lines
+105
to
+106
| onSettled: (data, error, variables, onMutateResult, context) => { | ||
| userOnSettled?.(data, error, variables, onMutateResult, context); |
Contributor
There was a problem hiding this comment.
Incorrect onSettled callback signature.
The onSettled callback for useMutation receives 4 parameters: (data, error, variables, context). The current code passes 5 parameters including onMutateResult, which is incorrect.
Proposed fix
- onSettled: (data, error, variables, onMutateResult, context) => {
- userOnSettled?.(data, error, variables, onMutateResult, context);
+ onSettled: (data, error, variables, context) => {
+ userOnSettled?.(data, error, variables, context);
},📝 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
| onSettled: (data, error, variables, onMutateResult, context) => { | |
| userOnSettled?.(data, error, variables, onMutateResult, context); | |
| onSettled: (data, error, variables, context) => { | |
| userOnSettled?.(data, error, variables, context); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/controllers/API/queries/memories/use-delete-memory.ts`
around lines 105 - 106, The onSettled callback in the useDeleteMemory mutation
is using an incorrect signature with five parameters (including onMutateResult);
update the onSettled definition in useDeleteMemory to the correct React Query
signature (data, error, variables, context) and call userOnSettled with those
four arguments (i.e., userOnSettled?.(data, error, variables, context)),
removing the extra onMutateResult parameter so it matches the expected
useMutation onSettled parameters.
Comment on lines
+43
to
+52
| const baseUrl = getURL("MEMORIES"); | ||
| const url = new URL(baseUrl, window.location.origin); | ||
| if (flowId) { | ||
| url.searchParams.set("flow_id", flowId); | ||
| } | ||
| url.searchParams.set("page", String(pageParam)); | ||
| url.searchParams.set("size", String(DEFAULT_PAGE_SIZE)); | ||
|
|
||
| const url = params?.flowId | ||
| ? `${getURL("MEMORIES")}/?flow_id=${params.flowId}` | ||
| : `${getURL("MEMORIES")}/`; | ||
| const res = await api.get(url); | ||
| return res.data; | ||
| const res = await api.get<GetMemoriesApiResponse>(url.toString()); | ||
| return mapGetMemoriesApiResponse(res.data); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other memory hooks construct URLs and how api is configured
echo "=== Checking URL patterns in other memory hooks ==="
rg -n "api\.(get|post|patch|delete)" src/frontend/src/controllers/API/queries/memories/
echo ""
echo "=== Checking api instance configuration ==="
fd -t f "api.ts" src/frontend/src/controllers/API/ --exec cat {}Repository: langflow-ai/langflow
Length of output: 11981
🏁 Script executed:
#!/bin/bash
# Get the actual implementations to see what arguments are passed to api calls
echo "=== use-update-memory.ts implementation ==="
sed -n '30,50p' src/frontend/src/controllers/API/queries/memories/use-update-memory.ts
echo ""
echo "=== use-create-memory.ts implementation ==="
sed -n '25,45p' src/frontend/src/controllers/API/queries/memories/use-create-memory.ts
echo ""
echo "=== use-get-memory.ts implementation ==="
sed -n '10,30p' src/frontend/src/controllers/API/queries/memories/use-get-memory.ts
echo ""
echo "=== use-delete-memory.ts implementation ==="
sed -n '30,40p' src/frontend/src/controllers/API/queries/memories/use-delete-memory.tsRepository: langflow-ai/langflow
Length of output: 2479
Align URL construction with the pattern used by other memory hooks.
All other memory hooks (use-delete-memory.ts, use-update-memory.ts, use-create-memory.ts, use-get-memory.ts) pass relative URLs directly by using getURL("MEMORIES") as an argument to the API call. The use-get-memories.ts hook uniquely constructs an absolute URL with window.location.origin, which bypasses the axios baseURL configuration and deviates from the established pattern.
Since the api instance is configured with a baseURL and expects relative URLs (as evidenced by the request interceptor logic), pass getURL("MEMORIES") directly with query parameters instead of constructing an absolute URL:
const baseUrl = getURL("MEMORIES");
const url = new URL(baseUrl, window.location.origin);
should be:
const url = new URL(getURL("MEMORIES"), window.location.origin);
Or more simply, build query params on the relative URL and let axios handle the baseURL.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/controllers/API/queries/memories/use-get-memories.ts` around
lines 43 - 52, The hook is constructing an absolute URL with
window.location.origin which bypasses the axios baseURL; change
use-get-memories.ts to build query parameters on the relative path returned by
getURL("MEMORIES") (e.g., use URL or URLSearchParams with getURL("MEMORIES") as
the base) and pass that relative URL string into api.get instead of an absolute
url; update the code around getURL("MEMORIES"), url.searchParams.set("flow_id",
flowId), pageParam and DEFAULT_PAGE_SIZE so api.get<GetMemoriesApiResponse>(...)
receives a relative path and then return mapGetMemoriesApiResponse(res.data) as
before.
Comment on lines
+101
to
+103
| onSettled: (data, error, variables, onMutateResult, context) => { | ||
| queryClient.invalidateQueries({ queryKey: ["useUpdateMemory"] }); | ||
| userOnSettled?.(data, error, variables, onMutateResult, context); |
Contributor
There was a problem hiding this comment.
onSettled signature incorrect and mutation key invalidation is a no-op.
Two issues here:
onSettledreceives 4 parameters(data, error, variables, context), not 5.invalidateQueries({ queryKey: ["useUpdateMemory"] })invalidates the mutation's own key, which has no effect. If the intent is to trigger refetches, invalidate the relevant query keys (e.g.,["useGetMemoriesInfinite"]), though this may be unnecessary since cache is already updated directly.
Proposed fix
- onSettled: (data, error, variables, onMutateResult, context) => {
- queryClient.invalidateQueries({ queryKey: ["useUpdateMemory"] });
- userOnSettled?.(data, error, variables, onMutateResult, context);
+ onSettled: (data, error, variables, context) => {
+ userOnSettled?.(data, error, variables, context);
},📝 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
| onSettled: (data, error, variables, onMutateResult, context) => { | |
| queryClient.invalidateQueries({ queryKey: ["useUpdateMemory"] }); | |
| userOnSettled?.(data, error, variables, onMutateResult, context); | |
| onSettled: (data, error, variables, context) => { | |
| userOnSettled?.(data, error, variables, context); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/controllers/API/queries/memories/use-update-memory.ts`
around lines 101 - 103, Fix the onSettled handler signature and the invalidation
target: change the onSettled callback in useUpdateMemory to use the correct
four-arg signature (data, error, variables, context) instead of a five-arg form,
and stop invalidating the mutation's own key; either remove
invalidateQueries(["useUpdateMemory"]) or replace it with the appropriate query
key(s) that should refetch (for example ["useGetMemoriesInfinite"]) if you need
a refetch — keep the direct cache updates intact if refetching is unnecessary.
Ensure you update the function referenced as onSettled in useUpdateMemory and
the call to queryClient.invalidateQueries to reference the correct query key(s).
Comment on lines
+35
to
+40
| const normalized = (() => { | ||
| if (trimmed.includes("T")) return trimmed; | ||
| const parts = trimmed.split(/\s+/g); | ||
| if (parts.length < 2) return trimmed; | ||
| return `${parts[0]}T${parts.slice(1).join("")}`; | ||
| })(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JavaScript/ECMAScript, is parsing non-ISO date strings implementation-dependent, and are strings like "2025-01-15T10:30:00GMT+0000(UTC)" guaranteed to parse consistently across engines?
💡 Result:
Yes, parsing non-ISO date strings in JavaScript/ECMAScript is implementation-dependent. No, strings like "2025-01-15T10:30:00GMT+0000(UTC)" are not guaranteed to parse consistently across engines.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/parse
- 2: https://tc39.es/ecma262/multipage/numbers-and-dates.html
- 3: https://learn.microsoft.com/en-us/openspecs/ie_standards/ms-es5/3fbe8262-3f94-4d8d-8bb0-69d05b66a1d8?redirectedfrom=MSDN
- 4: https://dateandtimeconverter.com/guides/javascript-date-parsing/
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and read the relevant section
if [ -f "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.ts" ]; then
echo "=== File exists, reading content ==="
wc -l "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.ts"
echo ""
echo "=== Lines 30-45 (context around lines 35-40) ==="
sed -n '30,45p' "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.ts" | cat -n
else
echo "File not found"
fiRepository: langflow-ai/langflow
Length of output: 852
🏁 Script executed:
#!/bin/bash
# Search for where this normalization function is used
echo "=== Searching for calls to the normalization function ==="
rg -n "normalized" "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/" -A 2 -B 2Repository: langflow-ai/langflow
Length of output: 1245
🏁 Script executed:
#!/bin/bash
# Check what types of timestamp strings are being passed
echo "=== Looking for timestamp sources and usage patterns ==="
rg -n "const normalized|new Date|Date\.parse" "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.ts" -B 3 -A 5Repository: langflow-ai/langflow
Length of output: 962
🏁 Script executed:
#!/bin/bash
# Check where formatTimestamp is called and what formats are passed
echo "=== Finding calls to formatTimestamp ==="
rg -n "formatTimestamp" --type ts --type tsx -B 2 -A 2Repository: langflow-ai/langflow
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Check for any API responses or type definitions that might tell us timestamp format
echo "=== Looking for timestamp type definitions and API responses ==="
rg -n "timestamp|createdAt|updatedAt" "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/" -B 1 -A 1 | head -50Repository: langflow-ai/langflow
Length of output: 3889
🏁 Script executed:
#!/bin/bash
# Test the actual behavior with different timestamp formats
python3 << 'EOF'
# Simulate the normalization logic
def normalize_timestamp(ts):
if not ts:
return None
trimmed = ts.strip()
if "T" in trimmed:
return trimmed
parts = trimmed.split()
if len(parts) < 2:
return trimmed
return f"{parts[0]}T{''.join(parts[1:])}"
# Test cases
test_cases = [
"2025-01-15 10:30:00",
"2025-01-15 10:30:00 GMT",
"2025-01-15 10:30:00 GMT+0000",
"2025-01-15T10:30:00",
"2025-01-15 10:30:00 +00:00",
"2025-01-15 10:30:00.123",
]
print("Normalization results:")
for test in test_cases:
result = normalize_timestamp(test)
print(f" '{test}' -> '{result}'")
# Test with JavaScript Date parsing (via Node simulation)
print("\nNote: JavaScript Date.parse() behavior for these normalized strings:")
print("- '2025-01-15T10:30:00' -> Valid ISO format")
print("- '2025-01-15T10:30:00GMT' -> Non-standard, implementation-dependent")
print("- '2025-01-15T10:30:00GMT+0000' -> Non-standard, implementation-dependent")
print("- '2025-01-15T10:30:00+00:00' -> Valid ISO 8601 format")
EOFRepository: langflow-ai/langflow
Length of output: 739
🏁 Script executed:
#!/bin/bash
# Read the test file to understand actual timestamp formats being tested
echo "=== Full test file for helpers ==="
cat "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/__tests__/helpers.test.ts" | cat -nRepository: langflow-ai/langflow
Length of output: 1981
🏁 Script executed:
#!/bin/bash
# Check the full function context to see if there are edge cases
echo "=== Full formatTimestamp function ==="
sed -n '31,53p' "src/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.ts" | cat -nRepository: langflow-ai/langflow
Length of output: 889
🏁 Script executed:
#!/bin/bash
# Test the actual normalization with the format from the test case
python3 << 'EOF'
# Simulate the exact normalization logic
def normalize(ts):
trimmed = ts.strip()
if "T" in trimmed:
return trimmed
parts = trimmed.split() # /\s+/g in JS
if len(parts) < 2:
return trimmed
# The critical line: join with empty string
result = f"{parts[0]}T" + "".join(parts[1:])
return result
# Test with actual cases from codebase
test_cases = [
"2025-01-15T10:00:00.000Z", # Already has T
"2025-01-15 10:30:00Z", # From test file
"2025-01-15 10:30:00", # Basic format
"2025-01-15 10:30:00 GMT", # With timezone keyword
"2025-01-15 10:30:00 +00:00", # With offset
]
for test in test_cases:
normalized = normalize(test)
print(f"'{test}'")
print(f" -> normalized: '{normalized}'")
# Check what JavaScript would do
print(f" parts after split: {test.strip().split()}")
print()
EOFRepository: langflow-ai/langflow
Length of output: 668
Consider preserving spacing in timezone normalization for robustness.
The normalization removes all spaces between parts via join(""), which can make timezone-bearing formats unparseable (e.g., "2025-01-15 10:30:00 GMT" becomes "2025-01-15T10:30:00GMT"). While the fallback to the original string on parse failure provides safety, preserving the remainder after the time token is more robust and clearer in intent.
Suggested refactor
- const normalized = (() => {
- if (trimmed.includes("T")) return trimmed;
- const parts = trimmed.split(/\s+/g);
- if (parts.length < 2) return trimmed;
- return `${parts[0]}T${parts.slice(1).join("")}`;
- })();
+ const normalized = (() => {
+ if (trimmed.includes("T")) return trimmed;
+ const match = trimmed.match(/^(\S+)\s+(\S+)(.*)$/);
+ if (!match) return trimmed;
+ const [, datePart, timePart, remainder] = match;
+ return `${datePart}T${timePart}${remainder}`;
+ })();📝 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
| const normalized = (() => { | |
| if (trimmed.includes("T")) return trimmed; | |
| const parts = trimmed.split(/\s+/g); | |
| if (parts.length < 2) return trimmed; | |
| return `${parts[0]}T${parts.slice(1).join("")}`; | |
| })(); | |
| const normalized = (() => { | |
| if (trimmed.includes("T")) return trimmed; | |
| const match = trimmed.match(/^(\S+)\s+(\S+)(.*)$/); | |
| if (!match) return trimmed; | |
| const [, datePart, timePart, remainder] = match; | |
| return `${datePart}T${timePart}${remainder}`; | |
| })(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/helpers.ts`
around lines 35 - 40, The IIFE that builds normalized currently collapses all
whitespace with parts.slice(1).join("") which removes spaces (e.g., "GMT") and
can break timezone formats; update the normalized computation inside the IIFE
(the block that reads `const normalized = (() => { ... })();`, referencing
variables `trimmed` and `parts`) to preserve spacing between the non-date/time
tail (use parts.slice(1).join(" ") or otherwise reinsert a single space) so the
remainder after the time token is kept intact, while keeping the existing
parse-fallback logic unchanged.
Comment on lines
+1
to
+35
| export type PrefillFlowMessage = { | ||
| id?: string; | ||
| session_id?: string; | ||
| sender?: string; | ||
| text?: string; | ||
| timestamp?: string; | ||
| }; | ||
|
|
||
| const FIXED_SESSIONS = [ | ||
| "fixed-session-1", | ||
| "fixed-session-2", | ||
| "fixed-session-3", | ||
| ] as const; | ||
|
|
||
| const MESSAGES_PER_SESSION = 20; | ||
|
|
||
| const buildTimestamp = (offsetSeconds: number) => | ||
| new Date(Date.UTC(2026, 2, 31, 0, 0, offsetSeconds)).toISOString(); | ||
|
|
||
| export const FIXED_PREFILL_MESSAGES: PrefillFlowMessage[] = | ||
| FIXED_SESSIONS.flatMap((sessionId, sessionIdx) => | ||
| Array.from({ length: MESSAGES_PER_SESSION }, (_, messageIdx) => { | ||
| const indexWithinSession = messageIdx + 1; | ||
| const globalOffsetSeconds = | ||
| sessionIdx * MESSAGES_PER_SESSION + indexWithinSession; | ||
|
|
||
| return { | ||
| id: `fixed-${sessionId}-msg-${indexWithinSession}`, | ||
| session_id: sessionId, | ||
| sender: messageIdx % 2 === 0 ? "user" : "assistant", | ||
| text: `Fixed prefill message ${indexWithinSession} (${sessionId})`, | ||
| timestamp: buildTimestamp(globalOffsetSeconds), | ||
| }; | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expect only test/dev usages. Any runtime import (especially from useMemoriesData.ts)
# confirms that synthetic messages are entering the production path.
rg -n -C2 'FIXED_PREFILL_MESSAGES|fixedPrefillMessages' src/frontendRepository: langflow-ai/langflow
Length of output: 2202
🏁 Script executed:
cat -n src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts | sed -n '155,175p'Repository: langflow-ai/langflow
Length of output: 945
🏁 Script executed:
cat -n src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts | sed -n '1,30p'Repository: langflow-ai/langflow
Length of output: 1466
🏁 Script executed:
cat -n src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts | sed -n '150,200p'Repository: langflow-ai/langflow
Length of output: 2151
🏁 Script executed:
grep -n -i 'dev\|env\|process\|demo\|test\|debug' src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts | head -20Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
cat -n src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts | sed -n '200,250p'Repository: langflow-ai/langflow
Length of output: 1916
Remove synthetic message fallback from production memory flow.
The FIXED_PREFILL_MESSAGES dataset is imported and used unconditionally as a fallback in useMemoriesData.ts (lines 163–177). When a memory document has no messages, the UI renders 60 fabricated conversation records instead of displaying an empty state. These synthetic IDs and fixed timestamps cannot correspond to real backend data and will mislead users about actual memory content.
Move this fixture data to a test/dev-only location or guard it behind an environment flag. The production path should either show a true empty state or handle missing data appropriately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/fixedPrefillMessages.ts`
around lines 1 - 35, The FIXED_PREFILL_MESSAGES fixture is being used
unconditionally as a fallback in the production memory flow (imported/consumed
by useMemoriesData), causing fabricated conversation records to appear when a
memory has no messages; remove the unconditional use of FIXED_PREFILL_MESSAGES
by either relocating this constant to a test/dev-only module or wrapping its
usage behind an environment guard (e.g., only use FIXED_PREFILL_MESSAGES when
process.env.NODE_ENV !== 'production' or a testing flag is set), and update
useMemoriesData to render a real empty state or appropriate UI when messages are
missing instead of injecting the synthetic messages.
| sessions, | ||
| }; | ||
| }, [memory, activeSearch]); | ||
| }, [memory, flowMessages, activeSearch]); |
Contributor
There was a problem hiding this comment.
Unused dependency flowMessages in docsData memo.
flowMessages is listed in the dependency array but isn't used in the memo computation. This causes unnecessary recalculations when flowMessages changes. Either remove it from dependencies or use it in the computation if intended.
Proposed fix
- }, [memory, flowMessages, activeSearch]);
+ }, [memory, activeSearch]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts`
at line 209, The useMemo for docsData in useMemoriesData.ts currently lists
flowMessages in its dependency array but does not reference it in the memo body,
causing unnecessary recalculations; update the docsData memo by either removing
flowMessages from the dependency array (so dependencies become [memory,
activeSearch]) or, if flowMessages was intended to affect docsData, incorporate
flowMessages into the memo computation (referencing the flowMessages symbol
inside the memo) so the dependency is meaningful.
Comment on lines
+17
to
+24
| const [selectedMemoryId, setSelectedMemoryId] = useState<string | null>(null); | ||
| const onSelectMemory = useCallback((id: string | null) => { | ||
| setSelectedMemoryId(id); | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| setSelectedMemoryId(null); | ||
| }, [currentFlowId]); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/frontend/src/pages/FlowPage/components/MemoriesMainContent/index.tsx | head -50Repository: langflow-ai/langflow
Length of output: 1934
🏁 Script executed:
# Search for useMemoriesData hook definition
rg "useMemoriesData" --type ts --type tsx -A 10 -B 2 | head -100Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
# Check if there are other uses of selectedMemoryId in the file
rg "selectedMemoryId" src/frontend/src/pages/FlowPage/components/MemoriesMainContent/index.tsx -B 2 -A 2Repository: langflow-ai/langflow
Length of output: 708
🏁 Script executed:
cat -n src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.tsRepository: langflow-ai/langflow
Length of output: 12217
🏁 Script executed:
# Also check the full signature and how selectedMemoryId is used in the hook
rg "selectedMemoryId" src/frontend/src/pages/FlowPage/components/MemoriesMainContent/hooks/useMemoriesData.ts -B 3 -A 3Repository: langflow-ai/langflow
Length of output: 1581
Stale memory selection on flow switch causes unnecessary fetch with previous flow's memory ID.
When currentFlowId changes, selectedMemoryId clears only after the first render via useEffect. During that render, useGetMemory executes with the old memory ID from the previous flow, causing an unnecessary query and potential brief misalignment. Synchronously derive or gate the memory selection by currentFlowId (e.g., keying the selection as { flowId, memoryId } or resetting at the component level before the hook receives it) to prevent the stale ID from reaching useMemoriesData.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/pages/FlowPage/components/MemoriesMainContent/index.tsx`
around lines 17 - 24, The selectedMemoryId can be stale for one render when
currentFlowId changes, causing useGetMemory/useMemoriesData to receive an old
ID; change the selection state to include the flow context or gate hooks by flow
so the hook never sees a mismatched id — e.g., replace selectedMemoryId with a
composite state like { flowId: currentFlowId, memoryId: string | null } or
initialize/reset selection synchronously based on currentFlowId (update
setSelectedMemoryId/onSelectMemory to manage the composite key) and ensure
useGetMemory/useMemoriesData read the memoryId only when flowId matches
currentFlowId.
viktoravelino
approved these changes
Apr 27, 2026
Member
|
A few behaviour changes I suggested over DM to Ola, but posting here for posterity/visibility.
|
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
olayinkaadelakun
dismissed
Cristhianzl’s stale review
I have fixed changes that were requested
github-actions
Bot
added
enhancement
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Test
Backend: Ensure you are on the memory-system-apis branch.
Ensure all migrations are applied.
Start the server: make backend.
Frontend: Switch to the apis-memory branch (ensure it is pointing to your local backend API).
Run make frontend (usually on port 3000).