Skip to content

fix(storage): bind datetimes in time comparisons so they select by value - #191

Merged
acidkill merged 2 commits into
acidkill:mainfrom
RobertSigmundsson:fix/time-range-filter-binds-datetimes-v380
Sep 3, 2026
Merged

fix(storage): bind datetimes in time comparisons so they select by value#191
acidkill merged 2 commits into
acidkill:mainfrom
RobertSigmundsson:fix/time-range-filter-binds-datetimes-v380

Conversation

@RobertSigmundsson

Copy link
Copy Markdown
Contributor

Summary

  • Two SurrealDB queries bound a datetime bound as an ISO string against a datetime column, which makes the comparison resolve by type rank instead of by value. Both now bind the datetime.
  • find_neurons(time_range=...) returned nothing at all, for any window.
  • today_fibers_count counted every fiber the brain had ever held, not the ones created today.
  • Adds live regression tests for both, since the in-memory backend cannot reproduce either.

Why

created_at is a datetime column on both neuron and fiber. SurrealDB resolves a comparison between two different types by type rank rather than by value, so binding a string makes the predicate a constant - and which constant depends on the operator:

RETURN [ <datetime>'2020-01-01T00:00:00Z' >= '1900-01-01T00:00:00Z',   -- true
         <datetime>'2020-01-01T00:00:00Z' <= '2100-01-01T00:00:00Z' ]  -- false

The result does not depend on the string's contents, so no format of ISO string makes it work.

1. find_neurons(time_range=...) matched nothing. The two bounds are combined with AND, and they become constants in opposite directions, so the conjunction is never satisfied - including for rows squarely inside the window. Measured on surrealdb:v3.2.4 with three neurons created at that moment:

window far in the FUTURE -> 0 rows   (correct: 0)
window far in the PAST   -> 0 rows   (correct: 0)
window AROUND now        -> 0 rows   (correct: 3)   <-- the bug

This is silent: an empty result from a time-filtered query is indistinguishable from "nothing matched". The affected readers are the temporal recall hints in retrieval.py and two dashboard timeline queries in server/routes/dashboard_api.py.

2. today_fibers_count counted everything. get_enhanced_stats uses the one-sided >=, where the constant is true, so the "created today" counter returned every fiber in the brain. Measured with three fibers - one created today, one ten days old, one over a year old - the counter returned 3; with the datetime bound it returns 1. This one is worse than an empty read, because it fabricates a plausible number, and it is user-facing through smem info and the dashboard.

I scanned every datetime-column comparison in storage/surrealdb/: alerts.py, activity.py, tool_events.py, retrieval_trace.py, reasoning_traces.py, keyword_entity.py, review_schedules.py, depth_priors.py, typed_memory.py and the two $cutoff queries in store.py all bind datetime objects already. The remaining .isoformat() calls in that package are read-path serialisers, not binds. These two were the only sites converting a bind to a string. (shared_store.py also calls .isoformat() on the same two bounds, but those are HTTP query parameters for the remote-storage client, not SurrealDB binds, and are correct as they are.)

Backend parity, not new semantics. InMemoryStorage has always implemented start <= created_at <= end and created_at >= today directly on Python objects, so this change makes the SurrealDB backend agree with the backend the tests have been asserting against, rather than introducing a new contract.

A behaviour change worth flagging

Fixing the filter activates a code path that has never fired against SurrealDB, and I would rather point at it than let it be discovered.

_find_similar_time_neuron in the encode pipeline uses this filter to decide whether to create a TIME neuron. It searches a ±1h window around the referenced time (hint.midpoint) but compares it against created_at, the row's insert time - two different quantities - and it is content-blind (type=TIME, limit=1). So it is not a de-duplicator in the sense the name suggests, and this PR does not turn it into one: I ran the step twice against a live engine on a sentence carrying three hints, and today and this afternoon were still created afresh on the second encode. What did change is that one hint (this evening) was suppressed by an unrelated TIME neuron that happened to be inserted within an hour of that hint's midpoint.

So: some TIME neurons will now be suppressed, on a criterion that is arguably not the intended one. That behaviour is pre-existing and orthogonal to this fix - the guard simply could not run before - but it becomes observable with it, and it may deserve its own look. I have deliberately not changed that logic here, to keep this to one change.

The same mismatch exists on the recall side, and this fix activates it too. retrieval.py resolves a temporal hint with find_neurons(type=TIME, time_range=(hint.absolute_start, hint.absolute_end), limit=5) - again matching the hint's referenced range against rows' insert time. Previously those anchors contributed nothing; now they contribute at full retriever weight. I checked what that does with two TIME neurons - one reading yesterday (inserted today) and one reading next week (inserted yesterday) - and the hint for yesterday returned next week and not yesterday. So temporal anchors now participate in ranking on a criterion that does not mean what the hint means.

Both of these are the pre-existing semantics of _find_similar_time_neuron and the temporal retriever, and both match what InMemoryStorage has always done, so this PR restores parity rather than inventing behaviour - and returning the wrong-but-related anchor is arguably still better than the previous "always nothing". I am flagging them because they become live with this change and I would rather they were known than found later; whether the insert-time comparison should become a metadata-range comparison seems worth a separate issue, which I am happy to open.

Test plan

  • pytest tests/ -m "not stress" -n auto passes locally: 7079 passed, 150 skipped, 1 xfailed. The skip delta against main is +4 - exactly the new tests, which skip when SURREALDB_URL is unset.
  • ruff check src/ tests/ clean; ruff format --check clean.
  • mypy src/ --ignore-missing-imports - Success: no issues found in 353 source files.
  • Negative control: with store.py restored to its state on main, three of the four new tests fail - the in-window query returns nothing, the pipeline's TIME lookup returns nothing, and the counter returns 2 where 1 is correct. With the fix, all four pass.
  • The fourth test (test_time_range_excludes_rows_outside_the_window) passes with or without the fix, because the broken filter also returned nothing. It is there to stop the fix over-correcting into false positives, not to demonstrate the bug. Its windows are one and two days out rather than decades, so a merely coarse comparison would not satisfy it.
  • Verified on surrealdb:v3.2.4 against a real schema, in a fresh process, with a fresh brain per run.
  • Naive/aware datetimes checked: utcnow() returns naive UTC and stored values are written from it; a naive bind is treated as UTC (a local-time interpretation would have returned 0 rows for the ±1h window) and an offset-aware bind is normalised correctly.

Verified by

@RobertSigmundsson


Thanks for the pace on v3.7.0 and v3.8.0 - both out on 2026-08-27. This branch is rebased onto
ae8e8743, so the failure and the fix are measured against what you shipped, not against an older
base. Applies cleanly; no overlap with #186's storage changes.

Two queries bound a datetime bound as an ISO string against a datetime column.
SurrealDB resolves a cross-type comparison by type rank rather than by value, so
the predicate became constant - and which constant depends on the operator:
datetime >= string is always true, datetime <= string always false.

find_neurons(time_range=...) combines both, so it matched nothing at all, for
any window. get_enhanced_stats uses the one-sided >=, so today_fibers_count
reported every fiber the brain had ever held as created today - a fabricated
number surfaced by smem info and the dashboard.

InMemoryStorage already implemented both comparisons on Python objects, so this
restores parity between the backends rather than changing the contract.

Covered by live regression tests; the schemaless in-memory backend compares
Python objects and cannot reproduce either fault.
@acidkill
acidkill merged commit 79ae016 into acidkill:main Sep 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants