feat: time-range filtering for memory_recall, memory_smart_search, memory_sessions (#392)#414
Open
RajvardhanPatil07 wants to merge 4 commits into
Open
Conversation
…mory_sessions (rohitg00#392) Adds optional ISO 8601 `start_time` and `end_time` arguments (both inclusive) to the three time-aware memory tools so agents can answer 'what did I work on last week?' without keyword guessing. - New shared utility `src/state/time-filter.ts` exposing `parseTimeRange`, `inTimeRange`, `filterSessionsByTime`, and `TimeRangeError` (with an HTTP-friendly `code` field for uniform 400s). - `mem::search` (memory_recall): validates and applies the time range after enrichment, over-fetches 10x when any filter is active so recall@K does not collapse on narrow windows. - `mem::smart-search` (memory_smart_search): forwards the parsed range to the search closure; HybridSearch.search/searchWithExpansion accept `options.timeRange`, the triple-stream search over-fetches BM25/vector/graph by 5x under filtering, the diversifyBySession cap rises to 5/session, and the post-enrichment `inTimeRange` filter trims results before `slice(0, limit)`. - `api::sessions` (GET /agentmemory/sessions): accepts `?start_time=&end_time=&limit=`; sessions are matched by lifetime overlap with the window (a session that started Apr 30 and ended May 2 still matches 2026-05-01..2026-05-07; active sessions are treated as still running) and returned sorted by startedAt descending. - `api::search` and `api::smart-search`: validate and forward the new arguments. - MCP server (memory_recall, memory_smart_search, memory_sessions): tool schemas extended; handlers validate the range up front and return 400 / `code: invalid_time_range` before retrieval runs. - MCP shim (`@agentmemory/mcp`): Validated extended; proxy mode forwards the arguments to the running server (POST body for smart-search/recall, query string for sessions); local InMemoryKV fallback applies the filter against `memory.createdAt` (recall) and `session.startedAt` / `endedAt` (sessions). - 38 new tests (test/time-filter.test.ts, test/time-filter-mcp-shim.test.ts) covering parseTimeRange (valid/invalid inputs, single-bound), inTimeRange (bounds inclusivity, malformed timestamps), filterSessionsByTime (lifetime overlap, sort order, active sessions), the search/smart-search integration paths, and the shim's proxy + local-fallback plumbing. Total: 992/992 tests pass; build is clean. - README.md: tool descriptions, REST endpoint table, and a new 'Time-range filtering' subsection with curl examples. CHANGELOG.md: Unreleased entry. Closes rohitg00#392.
|
@RajvardhanPatil07 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds optional ISO 8601 inclusive Changes Time-range filtering feature
Sequence Diagram (high-level flow) sequenceDiagram
participant Client
participant API
participant MCP as MCP_Standalone
participant AgentMem as AgentMemory_Service
participant Hybrid as HybridSearch
Client->>API: GET /agentmemory/sessions?start_time&end_time&limit
API->>MCP: mcp::tools::call(memory_sessions, {start_time,end_time,limit})
MCP->>AgentMem: proxy /agentmemory/sessions?start_time&end_time&limit
AgentMem->>Hybrid: hybridSearch.search(query, limit, { timeRange })
Hybrid-->>AgentMem: filtered results (post-enrichment inTimeRange)
AgentMem-->>MCP: sessions JSON
MCP-->>API: sessions JSON
API-->>Client: return sessions
Estimated code review effort Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/mcp/standalone.ts (1)
101-103: ⚡ Quick winReplace WHAT-comments with clearer naming/extraction.
These comments restate behavior already expressed by nearby code paths; prefer self-describing names/helpers instead of explanatory WHAT-comments.
As per coding guidelines, "Avoid code comments explaining WHAT — use clear naming instead".
Also applies to: 290-293
🤖 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 `@src/mcp/standalone.ts` around lines 101 - 103, The comment block describing the time-range forwarding is a WHAT-comment; replace it with self-describing names or a small extractor to make the behavior explicit: extract the optional ISO 8601 strings and their parsed numeric fallback into well-named symbols (for example isoTimeRangeRaw / parsedTimeRangeMs or a helper function extractIsoTimeRange / buildProxyTimeRangeParams) and then use those symbols when building the proxy URL and local fallback logic; apply the same renaming/extraction to the other occurrence noted (around the 290-293 comment) so both sites read clearly without explanatory WHAT-comments.src/state/hybrid-search.ts (1)
238-244: 💤 Low valueComment mentions adjusting rerank window but code doesn't change it.
The comment states "Bump retrieval depth + diversify cap + rerank window" but
rerankWindowremains 20 on line 244. Since reranking happens after time filtering (line 255), the rerank window doesn't affect headroom for the time filter. Consider removing "rerank window" from the comment for accuracy.📝 Suggested comment fix
- // Bump retrieval depth + diversify cap + rerank window when a time - // range is set so the post-enrichment filter has enough headroom to - // surface `limit` matches even when the window is narrow. + // Bump retrieval depth + diversify cap when a time range is set so + // the post-enrichment filter has enough headroom to surface `limit` + // matches even when the window is narrow.🤖 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 `@src/state/hybrid-search.ts` around lines 238 - 244, The comment above retrievalDepth incorrectly mentions "rerank window" even though rerankWindow is unchanged; update the comment near retrievalDepth/timeRange to remove "rerank window" (or alternatively adjust the rerankWindow variable if you intended to change it) so it accurately reflects the behavior of retrievalDepth, diversify cap, and rerankWindow; refer to the identifiers retrievalDepth, rerankWindow, timeRange, limit, and candidateMultiplier to locate and fix the text.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/mcp/standalone.ts`:
- Around line 148-159: The memory_sessions limit in this shim is still being
capped at the old global max because parseLimit is called with 20 (and shared
parseLimit elsewhere uses a 100 cap); update the call in standalone.ts that sets
v.limit to enforce the new contract by passing the correct max (1000) — i.e.,
change the invocation that sets v.limit (parseLimit(args["limit"], 20)) to use
the new maximum (parseLimit(args["limit"], 1000)) so proxy/local behavior no
longer silently truncates requested session limits above 100; keep all other
parsing/validation (e.g., parseTimeRange, startTime/endTime trimming) unchanged.
In `@src/state/time-filter.ts`:
- Line 125: The overlap check treats session lifetime as half-open [startedAt,
endedAt) but currently uses "if (range.start !== undefined && endMs <
range.start) continue;", which wrongly includes sessions where endedAt ==
range.start; change the boundary to exclude those by using a non-strict
comparison (i.e., make the condition test endMs <= range.start) so sessions
ending exactly at the window start are skipped. Ensure this update is applied
where endMs and range.start are used in the time-filter overlap logic (the code
handling startedAt/endedAt and range.start/range.end).
- Around line 109-129: The loop calls Date.now() repeatedly for active or
invalid-ended sessions; capture a single timestamp once before iterating (e.g.,
const nowMs = Date.now()) and use nowMs wherever the code currently calls
Date.now() (the places that set endMs when s.endedAt parses to NaN or is absent)
so sessions, filtered, range, and byStartedAtDesc logic reuse the same timestamp
for consistency and efficiency.
In `@src/triggers/api.ts`:
- Line 935: The call to sdk.trigger({ function_id: "mem::smart-search", payload:
req.body }) sends the entire request body; instead construct a whitelisted
payload object and pass that to sdk.trigger. Replace payload: req.body with a
new object that explicitly picks and validates fields (e.g., query:
req.body.query as string | undefined, expandIds: req.body.expandIds as string[]
| undefined, limit: req.body.limit as number | undefined, start_time: typeof
req.body.start_time === "string" ? req.body.start_time : undefined, end_time:
typeof req.body.end_time === "string" ? req.body.end_time : undefined) and pass
that object to sdk.trigger for function_id "mem::smart-search".
- Around line 915-934: Whitelist fields from req.body into a clean payload
before calling sdk.trigger(): copy only the allowed keys (e.g., query,
expandIds, start_time, end_time, limit, etc.) into a new object and pass that to
sdk.trigger() instead of raw req.body; keep using parseTimeRange/TimeRangeError
to validate time ranges when query is present, and additionally detect the
expandIds-only branch (when query is absent but expandIds is provided) and
return a 400 if start_time or end_time are supplied (or otherwise explicitly
ignore them) to make the contract clear. Ensure you reference sdk.trigger,
req.body, parseTimeRange, TimeRangeError, and expandIds when implementing the
changes.
---
Nitpick comments:
In `@src/mcp/standalone.ts`:
- Around line 101-103: The comment block describing the time-range forwarding is
a WHAT-comment; replace it with self-describing names or a small extractor to
make the behavior explicit: extract the optional ISO 8601 strings and their
parsed numeric fallback into well-named symbols (for example isoTimeRangeRaw /
parsedTimeRangeMs or a helper function extractIsoTimeRange /
buildProxyTimeRangeParams) and then use those symbols when building the proxy
URL and local fallback logic; apply the same renaming/extraction to the other
occurrence noted (around the 290-293 comment) so both sites read clearly without
explanatory WHAT-comments.
In `@src/state/hybrid-search.ts`:
- Around line 238-244: The comment above retrievalDepth incorrectly mentions
"rerank window" even though rerankWindow is unchanged; update the comment near
retrievalDepth/timeRange to remove "rerank window" (or alternatively adjust the
rerankWindow variable if you intended to change it) so it accurately reflects
the behavior of retrievalDepth, diversify cap, and rerankWindow; refer to the
identifiers retrievalDepth, rerankWindow, timeRange, limit, and
candidateMultiplier to locate and fix the text.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bf31817-f270-4920-af68-b08196051bbe
📒 Files selected for processing (13)
CHANGELOG.mdREADME.mdsrc/functions/search.tssrc/functions/smart-search.tssrc/index.tssrc/mcp/server.tssrc/mcp/standalone.tssrc/mcp/tools-registry.tssrc/state/hybrid-search.tssrc/state/time-filter.tssrc/triggers/api.tstest/time-filter-mcp-shim.test.tstest/time-filter.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/mcp/server.ts (1)
35-37: 💤 Low valueConsider reusing
asNonEmptyStringinstead of duplicating the logic.The implementation is identical to
asNonEmptyString(lines 25-27). While the semantic naming aids readability, you could eliminate the duplication by callingasNonEmptyStringdirectly at the call sites.♻️ Proposed consolidation
-function asOptionalTimeBound(value: unknown): string | undefined { - return typeof value === "string" && value.trim() ? value.trim() : undefined; -} -Then replace
asOptionalTimeBound(args.start_time)withasNonEmptyString(args.start_time)at the call sites (lines 141-142, 324-325).🤖 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 `@src/mcp/server.ts` around lines 35 - 37, The function asOptionalTimeBound duplicates asNonEmptyString; remove the duplication by either deleting asOptionalTimeBound and replacing its call sites with asNonEmptyString (e.g., where asOptionalTimeBound(args.start_time) is used) or make asOptionalTimeBound simply delegate to asNonEmptyString (return asNonEmptyString(value)); update all call sites accordingly (replace asOptionalTimeBound(...) with asNonEmptyString(...)) and remove the unused duplicate function if you choose the delete approach.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@src/mcp/server.ts`:
- Around line 35-37: The function asOptionalTimeBound duplicates
asNonEmptyString; remove the duplication by either deleting asOptionalTimeBound
and replacing its call sites with asNonEmptyString (e.g., where
asOptionalTimeBound(args.start_time) is used) or make asOptionalTimeBound simply
delegate to asNonEmptyString (return asNonEmptyString(value)); update all call
sites accordingly (replace asOptionalTimeBound(...) with asNonEmptyString(...))
and remove the unused duplicate function if you choose the delete approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31a7a107-ad71-42db-b39d-73316a305c54
📒 Files selected for processing (5)
src/mcp/server.tssrc/mcp/standalone.tssrc/triggers/api.tstest/mcp-standalone.test.tstest/time-filter.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/mcp/standalone.ts
- src/triggers/api.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/mcp/server.ts (1)
120-120: ⚡ Quick winRemove the comment explaining WHAT.
The code is self-documenting; the comment violates the guideline to avoid comments that explain WHAT the code does. As per coding guidelines, use clear naming instead.
♻️ Proposed fix
- // Validate time range up front for a clean 400 (issue `#392`). try {🤖 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 `@src/mcp/server.ts` at line 120, Remove the redundant explanatory comment "// Validate time range up front for a clean 400 (issue `#392`)." in src/mcp/server.ts—leave the existing validation code intact (the self-documenting logic should stand on its own); do not add a WHAT-style comment, optionally ensure any surrounding variable or function names (e.g., the time range validation block or function handling request validation) are clear if you feel naming needs minor clarification.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@src/mcp/server.ts`:
- Line 120: Remove the redundant explanatory comment "// Validate time range up
front for a clean 400 (issue `#392`)." in src/mcp/server.ts—leave the existing
validation code intact (the self-documenting logic should stand on its own); do
not add a WHAT-style comment, optionally ensure any surrounding variable or
function names (e.g., the time range validation block or function handling
request validation) are clear if you feel naming needs minor clarification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4866e2a4-b636-4c57-a3f9-6cce30c75d76
📒 Files selected for processing (6)
src/mcp/server.tssrc/mcp/standalone.tssrc/state/hybrid-search.tssrc/state/time-filter.tssrc/triggers/api.tstest/time-filter.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/state/hybrid-search.ts
- src/triggers/api.ts
- test/time-filter.test.ts
- src/state/time-filter.ts
- src/mcp/standalone.ts
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
1 participant
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.
Closes #392.
Summary
Adds optional ISO 8601
start_timeandend_timearguments (both inclusive) tomemory_recall,memory_smart_search, andmemory_sessionsso agents can answer "what did I work on last week?" without keyword guessing.memory_sessionsalso gains alimitargument and now sorts bystartedAtdescending.The same arguments flow through every layer that exposes these tools: REST API, MCP server, and the
@agentmemory/mcpshim (both proxy and local-fallback modes).Behaviour
start_time/end_timeare optional ISO 8601 strings (e.g.2026-05-01T00:00:00Zor2026-05-01). Both bounds are inclusive.2026-05-01..2026-05-07. Active sessions (noendedAt) are treated as still running.start_time > end_time, non-string) is rejected with HTTP 400 /code: invalid_time_rangebefore retrieval runs, so a malformed window never costs you a search.mem::searchover-fetches 10× from the BM25 index,HybridSearchover-fetches BM25/vector/graph by 5×, and the diversify-by-session cap rises to 5/session when a time range is active. This is the same pattern already used forproject/cwdfiltering; recall@K does not collapse on narrow windows.Acceptance criteria from #392
memory_sessions(start_time, end_time, limit)memory_recall(query, start_time, end_time, format, limit, token_budget)memory_smart_search(query, start_time, end_time, expandIds, limit)Surface area
REST
GET/agentmemory/sessions?start_time=&end_time=&limit=query params.POST/agentmemory/searchstart_time/end_timein body.POST/agentmemory/smart-searchstart_time/end_timein body.MCP tools (server + shim)
memory_recall,memory_smart_search,memory_sessionsschemas extended.src/mcp/server.ts) validates the range and forwards tomem::search/mem::smart-search/ directkv.list(KV.sessions)+filterSessionsByTime.@agentmemory/mcp,src/mcp/standalone.ts):memory_smart_searchandmemory_recall, query string formemory_sessions.memory.createdAtfor recall andsession.startedAt/endedAtfor sessions, so the 7-tool fallback set still respects the new arguments.Internals
src/state/time-filter.ts:parseTimeRange(input) -> TimeRange | null(throwsTimeRangeErrorwith an HTTP-friendlycodefield).inTimeRange(timestamp, range)— defensive against malformed timestamps in the store.filterSessionsByTime(sessions, range)— lifetime-overlap predicate + descending sort.HybridSearch.search/searchWithExpansionnow acceptoptions.timeRange.tripleStreamSearchover-fetches each stream by 5× under filtering, bumps the retrieval depth tomax(limit*5, 100), raises the diversify-by-session cap to 5, and appliesinTimeRangepost-enrichment.Examples
The same arguments are accepted via MCP
tools/call.What was tested
test/time-filter.test.ts(31 tests):parseTimeRangehappy/sad paths (valid ISO, date-only, single-bound, whitespace, unparseable,start > end, non-string, equal bounds),inTimeRange(inclusivity, malformed timestamps, single-bound),filterSessionsByTime(lifetime overlap, descending sort, active sessions),mem::search(no-filter / both-bound / one-bound / error cases),mem::smart-search(forwardstimeRange, validates input, returns 400-style error).test/time-filter-mcp-shim.test.ts(7 tests): proxy-mode forwarding for all three tools, validation rejects bad input before the proxy call goes out, local InMemoryKV fallback applies the filter formemory_sessionsandmemory_recall.npm run buildclean.Out of scope / follow-ups
last_week/since:24h. The issue asks for ISO 8601 strings; a future enhancement could add a small parser layer on top ofparseTimeRange.memory_timelinealready takes an anchor;memory_auditcould growdateFrom/dateTomirroringGovernanceFilter). Tracked separately if desired.mem::sessionsiii function. Today the session list is materialized inline inapi::sessionsand the MCPmemory_sessionshandler; promoting it to a function would let the iii console invoke it directly. Left for a follow-up to keep this PR scoped.Summary by CodeRabbit
New Features
Documentation
Tests