feat: temporal filtering for channel_recall + self-channel recall#284
Conversation
…ecall channel_recall only supported fetching the N most recent messages with no time-based filtering, and all docs/prompts described it as cross-channel only. This meant the bot couldn't answer questions like 'find my first message' and didn't know it could query the current channel's history. - Add before, after (RFC 3339), and oldest_first params to channel_recall - Update SQL in load_channel_transcript to support dynamic WHERE/ORDER - Fix all prompts and docs that said 'another channel' to say 'any channel' - Add anti-deflection rule to channel prompt so the bot uses its tools instead of suggesting users check things manually
WalkthroughAdds temporal filtering (before/after RFC3339) and an oldest_first option to channel_recall and the transcript loader, expands channel_recall to query any channel (including the current one), updates related docs, and introduces multiple new public tool modules under src/tools (exec, browser, cron, task_* variants, spacebot_docs, config_inspect). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| } | ||
| if after.is_some() { | ||
| sql.push_str(" AND created_at > ?"); | ||
| } |
There was a problem hiding this comment.
Minor correctness gotcha: created_at is coming from SQLite CURRENT_TIMESTAMP (YYYY-MM-DD HH:MM:SS), so comparing it directly to RFC3339 input via < ?/> ? can behave oddly (esp. same-day comparisons because of the space vs T). Using datetime(?) keeps RFC3339 working without changing storage format.
| } | |
| if before.is_some() { | |
| sql.push_str(" AND created_at < datetime(?)"); | |
| } | |
| if after.is_some() { | |
| sql.push_str(" AND created_at > datetime(?)"); | |
| } |
| // Load transcript | ||
| let messages = self | ||
| .conversation_logger | ||
| .load_channel_transcript(&channel.id, limit) | ||
| .load_channel_transcript( | ||
| &channel.id, | ||
| limit, | ||
| args.before.as_deref(), | ||
| args.after.as_deref(), | ||
| args.oldest_first, | ||
| ) |
There was a problem hiding this comment.
Might be worth validating before/after up-front. As-is, an invalid RFC3339 string will just turn into "no matches" at the SQL layer, which is hard to debug from the agent side.
| // Load transcript | |
| let messages = self | |
| .conversation_logger | |
| .load_channel_transcript(&channel.id, limit) | |
| .load_channel_transcript( | |
| &channel.id, | |
| limit, | |
| args.before.as_deref(), | |
| args.after.as_deref(), | |
| args.oldest_first, | |
| ) | |
| if let Some(before) = args.before.as_deref() { | |
| chrono::DateTime::parse_from_rfc3339(before) | |
| .map_err(|e| ChannelRecallError(format!("Invalid `before` timestamp: {e}")))?; | |
| } | |
| if let Some(after) = args.after.as_deref() { | |
| chrono::DateTime::parse_from_rfc3339(after) | |
| .map_err(|e| ChannelRecallError(format!("Invalid `after` timestamp: {e}")))?; | |
| } | |
| // Load transcript | |
| let messages = self | |
| .conversation_logger | |
| .load_channel_transcript( | |
| &channel.id, | |
| limit, | |
| args.before.as_deref(), | |
| args.after.as_deref(), | |
| args.oldest_first, | |
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/conversation/history.rs`:
- Around line 194-199: The temporal filters use raw string comparison (" AND
created_at < ?" / " AND created_at > ?") which misorders RFC3339 vs
space-separated datetimes; update the sql.append calls to normalize datetimes
using SQLite's datetime() like the existing pattern at lines ~413–415: replace
the two sql.push_str invocations that append " AND created_at < ?" and " AND
created_at > ?" with " AND datetime(created_at) < datetime(?)" and " AND
datetime(created_at) > datetime(?)" respectively so created_at and the bound
parameter are compared as normalized datetimes (keep the same bound parameter
variables before/after).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
AGENTS.mdprompts/en/channel.md.j2prompts/en/tools/channel_recall_description.md.j2src/conversation/history.rssrc/tools/channel_recall.rs
| if before.is_some() { | ||
| sql.push_str(" AND created_at < ?"); | ||
| } | ||
| if after.is_some() { | ||
| sql.push_str(" AND created_at > ?"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and get line count
wc -l src/conversation/history.rsRepository: spacedriveapp/spacebot
Length of output: 97
🏁 Script executed:
# Read the relevant section (lines 180-220 for context)
sed -n '180,220p' src/conversation/history.rsRepository: spacedriveapp/spacebot
Length of output: 1309
🏁 Script executed:
# Check the function signature and parameter documentation
sed -n '170,200p' src/conversation/history.rsRepository: spacedriveapp/spacebot
Length of output: 1081
🏁 Script executed:
# Look for the referenced timeline query at lines 413-415
sed -n '410,420p' src/conversation/history.rsRepository: spacedriveapp/spacebot
Length of output: 377
🏁 Script executed:
# Search for how before/after are documented or typed
rg -n "before|after" src/conversation/history.rs | head -20Repository: spacedriveapp/spacebot
Length of output: 810
🏁 Script executed:
# Run the verification script to demonstrate SQLite comparison behavior
python3 << 'PY'
import sqlite3
conn = sqlite3.connect(":memory:")
cur = conn.cursor()
cur.execute("CREATE TABLE t(created_at TEXT)")
cur.executemany("INSERT INTO t(created_at) VALUES (?)", [
("2026-01-15 00:30:00",),
("2026-01-15 23:59:59",),
])
bound = "2026-01-15T00:00:00Z"
print(f"Testing comparison with bound value: {bound}")
print()
plain = cur.execute(
"SELECT created_at FROM t WHERE created_at < ? ORDER BY created_at",
(bound,)
).fetchall()
normalized = cur.execute(
"SELECT created_at FROM t WHERE datetime(created_at) < datetime(?) ORDER BY created_at",
(bound,)
).fetchall()
print("Plain text compare (created_at < ?):", plain)
print("datetime() compare (datetime(created_at) < datetime(?)):", normalized)
print()
print("Analysis:")
print(f" 'T' (ASCII {ord('T')}) vs ' ' (ASCII {ord(' ')}) in string comparison")
print(f" 2026-01-15T00:00:00Z > 2026-01-15 (as strings): {bound > '2026-01-15'}")
PYRepository: spacedriveapp/spacebot
Length of output: 396
🏁 Script executed:
# Verify the table schema to confirm created_at is stored as TEXT
rg -n "conversation_messages|CREATE TABLE" src/conversation/history.rs | head -10Repository: spacedriveapp/spacebot
Length of output: 553
🏁 Script executed:
# Check if there are other places where before/after are used to see the broader pattern
rg -n "load_channel_transcript|RFC 3339" src/conversation/history.rsRepository: spacedriveapp/spacebot
Length of output: 204
Use SQLite datetime normalization in temporal filters.
Lines 194-195 and 198 compare created_at to raw RFC 3339 string parameters without datetime normalization. String comparison of "2026-01-15T00:00:00Z" (where 'T' = ASCII 84) with "2026-01-15 HH:MM:SS" (where ' ' = ASCII 32) produces inverted filter results: rows are included when they should be excluded.
🛠️ Proposed fix
if before.is_some() {
- sql.push_str(" AND created_at < ?");
+ sql.push_str(" AND datetime(created_at) < datetime(?)");
}
if after.is_some() {
- sql.push_str(" AND created_at > ?");
+ sql.push_str(" AND datetime(created_at) > datetime(?)");
}A reference implementation with the correct pattern already exists in the same file (lines 413–415).
📝 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.
| if before.is_some() { | |
| sql.push_str(" AND created_at < ?"); | |
| } | |
| if after.is_some() { | |
| sql.push_str(" AND created_at > ?"); | |
| } | |
| if before.is_some() { | |
| sql.push_str(" AND datetime(created_at) < datetime(?)"); | |
| } | |
| if after.is_some() { | |
| sql.push_str(" AND datetime(created_at) > datetime(?)"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/conversation/history.rs` around lines 194 - 199, The temporal filters use
raw string comparison (" AND created_at < ?" / " AND created_at > ?") which
misorders RFC3339 vs space-separated datetimes; update the sql.append calls to
normalize datetimes using SQLite's datetime() like the existing pattern at lines
~413–415: replace the two sql.push_str invocations that append " AND created_at
< ?" and " AND created_at > ?" with " AND datetime(created_at) < datetime(?)"
and " AND datetime(created_at) > datetime(?)" respectively so created_at and the
bound parameter are compared as normalized datetimes (keep the same bound
parameter variables before/after).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/conversation/history.rs (1)
194-199:⚠️ Potential issue | 🟠 MajorUse SQLite
datetime()for temporal comparisons.Raw string comparison between RFC 3339 input (
2026-01-15T00:00:00Z) and SQLite'sCURRENT_TIMESTAMPformat (2026-01-15 00:30:00) produces incorrect results due to ASCII ordering ('T'>' '). The same file already uses the correct pattern at lines 469-472 inload_channel_timeline.🐛 Proposed fix
if before.is_some() { - sql.push_str(" AND created_at < ?"); + sql.push_str(" AND datetime(created_at) < datetime(?)"); } if after.is_some() { - sql.push_str(" AND created_at > ?"); + sql.push_str(" AND datetime(created_at) > datetime(?)"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/conversation/history.rs` around lines 194 - 199, The SQL temporal comparisons currently use raw string comparisons for created_at with placeholders (the if before.is_some()/if after.is_some() blocks), which yields incorrect ordering for RFC3339 strings; update those conditions to use SQLite's datetime() on both sides — e.g., change " AND created_at < ?" to " AND datetime(created_at) < datetime(?)" and similarly for the after branch — matching the pattern used in load_channel_timeline; keep the same parameter placeholders and types so callers need not change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/conversation/history.rs`:
- Around line 194-199: The SQL temporal comparisons currently use raw string
comparisons for created_at with placeholders (the if before.is_some()/if
after.is_some() blocks), which yields incorrect ordering for RFC3339 strings;
update those conditions to use SQLite's datetime() on both sides — e.g., change
" AND created_at < ?" to " AND datetime(created_at) < datetime(?)" and similarly
for the after branch — matching the pattern used in load_channel_timeline; keep
the same parameter placeholders and types so callers need not change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53d78588-db18-49af-9b39-e0246a3f5dd3
📒 Files selected for processing (3)
AGENTS.mdprompts/en/channel.md.j2src/conversation/history.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- prompts/en/channel.md.j2
…recall-temporal feat: temporal filtering for channel_recall + self-channel recall
1 participant
Summary
before,after(RFC 3339 timestamps), andoldest_firstparams tochannel_recallso the bot can query specific time ranges and find earliest messageschannel_recallworks on any channel including the current one, and the bot didn't know thatTriggered by Kegan asking the bot to recall its first message ever. The bot couldn't do it (no temporal params, only fetches N most recent) and then deflected to manual workarounds instead of saying it couldn't.
Changes
src/tools/channel_recall.rs— newbefore,after,oldest_firstargs + updated JSON schemasrc/conversation/history.rs— dynamic SQL with optional temporalWHEREclauses andASC/DESCorderingprompts/en/tools/channel_recall_description.md.j2— documents temporal filtering, notes it queries the full persisted DBprompts/en/channel.md.j2— "another channel" → "any channel (including this one)", anti-deflection ruleAGENTS.md— module map description fix