Fix review blockers: cache key, bulk card-one, Int AVET bounds

docs/design-tx-window-index.md

@@ -236,6 +236,9 @@ to AEVT/AVET + tx filter (correct, slower).
 
 ### Implemented (this branch)
 
+Shipped vs design: sections below this subsection (Mode B, TEAV alternatives,
+app-level “recent N days” product plans) remain design notes unless listed here.
+
 - `| Tave` index; key order `tx | a | v | e | added` (BLOB KV on LMDB `ds/tave` + SQLite `ds_tave`)
 - Written on every append alongside EAVT/AEVT/(AVET when indexed)
 - `fold_tave_range` / `prune_tave_before`; retention default 30 days

impl/datascript.mli

@@ -611,7 +611,8 @@ val last_query_exec_path : unit -> query_exec_path
 (** Run [f] with fused [Query_exec] disabled so [q] uses the relational fallback. *)
 val with_force_relation_fallback : (unit -> 'a) -> 'a
 
-(** Datalevin-style query result cache (general; keyed by db epoch + physical query).
+(** Datalevin-style query result cache (general; keyed by db uid, temporal/filter
+    view, epoch, and physical query/inputs).
     Enabled by default; set [DATASCRIPT_QUERY_RESULT_CACHE=0] to disable.
     Set [DATASCRIPT_QUERY_DEBUG=1] for plan/exec/cache stderr traces. *)
 val clear_query_result_cache : unit -> unit

impl/query_api.ml

@@ -19,8 +19,10 @@ let with_force_relation_fallback f =
   force_relation_fallback := true;
   Fun.protect ~finally:(fun () -> force_relation_fallback := previous) f
 
-(* Datalevin-style result cache: general, keyed by db epoch + physical query/inputs.
-   Avoid structural hashing of [query] — where clauses may embed function values. *)
+(* Datalevin-style result cache: keyed by db identity + temporal/filter view +
+   epoch + physical query/inputs. Avoid structural hashing of [query] — where
+   clauses may embed function values. [filter_pred] is compared by physical
+   identity (closures are not structurally comparable). *)
 let result_cache_enabled =
   match Sys.getenv_opt "DATASCRIPT_QUERY_RESULT_CACHE" with
   | Some ("0" | "false" | "no" | "off") -> ref false
@@ -32,12 +34,17 @@ let query_debug_enabled =
   | _ -> false
 
 type result_cache_entry =
-  { max_e : entity_id
-  ; max_tx : int
-  ; query : query
-  ; inputs : query_arg list
-  ; path : query_exec_path
-  ; rows : query_result list list
+  { cache_db_uid : int
+  ; cache_max_e : entity_id
+  ; cache_max_tx : int
+  ; cache_as_of_tx : tx option
+  ; cache_since_tx : tx option
+  ; cache_history : bool
+  ; cache_filter_pred : (datom -> bool) option
+  ; cache_query : query
+  ; cache_inputs : query_arg list
+  ; cache_path : query_exec_path
+  ; cache_rows : query_result list list
   }
 
 let result_cache_entries : result_cache_entry list ref = ref []
@@ -65,33 +72,52 @@ let path_label = function
   | Relation_fallback -> "relation"
   | Binding_interpreter -> "bindings"
 
+let same_filter_pred left right =
+  match left, right with
+  | None, None -> true
+  | Some a, Some b -> a == b
+  | _ -> false
+
+let same_result_cache_key entry db query inputs =
+  entry.cache_db_uid = db.db_uid
+  && entry.cache_max_e = db.max_datom_e
+  && entry.cache_max_tx = db.max_tx
+  && entry.cache_as_of_tx = db.as_of_tx
+  && entry.cache_since_tx = db.since_tx
+  && entry.cache_history = db.history
+  && same_filter_pred entry.cache_filter_pred db.filter_pred
+  && entry.cache_query == query
+  && entry.cache_inputs == inputs
+
 let lookup_result_cache db query inputs =
   if (not !result_cache_enabled) || !force_relation_fallback then None
   else
     List.find_map
       (fun entry ->
-        if
-          entry.max_e = db.max_datom_e
-          && entry.max_tx = db.max_tx
-          && entry.query == query
-          && entry.inputs == inputs
-        then Some (entry.path, entry.rows)
+        if same_result_cache_key entry db query inputs then
+          Some (entry.cache_path, entry.cache_rows)
         else None)
       !result_cache_entries
 
 let store_result_cache db query inputs path rows =
   if !result_cache_enabled && not !force_relation_fallback then (
     let entry =
-      { max_e = db.max_datom_e; max_tx = db.max_tx; query; inputs; path; rows }
+      { cache_db_uid = db.db_uid
+      ; cache_max_e = db.max_datom_e
+      ; cache_max_tx = db.max_tx
+      ; cache_as_of_tx = db.as_of_tx
+      ; cache_since_tx = db.since_tx
+      ; cache_history = db.history
+      ; cache_filter_pred = db.filter_pred
+      ; cache_query = query
+      ; cache_inputs = inputs
+      ; cache_path = path
+      ; cache_rows = rows
+      }
     in
     let rest =
       List.filter
-        (fun e ->
-          not
-            (e.query == query
-             && e.inputs == inputs
-             && e.max_e = entry.max_e
-             && e.max_tx = entry.max_tx))
+        (fun e -> not (same_result_cache_key e db query inputs))
         !result_cache_entries
     in
     let rec take n = function

impl/query_exec.ml

@@ -746,7 +746,7 @@ end) = struct
 
   let avet_index_start predicate threshold =
     match predicate, threshold with
-    | GreaterThan, Int n -> Some (Int (n + 1))
+    | GreaterThan, Int n when n < max_int -> Some (Int (n + 1))
     | GreaterOrEqual, value | GreaterThan, value -> Some value
     | _ -> None
 
@@ -812,7 +812,9 @@ end) = struct
           (function
             | ComparisonPredicate (predicate, left, right) -> (
               match range_predicate_for_var value_var predicate left right with
-              | Some (GreaterThan, Int _) | Some (LessThan, Int _) -> false
+              (* Tightened Int bounds (n±1) are exact only when they do not overflow. *)
+              | Some (GreaterThan, Int n) when n < max_int -> false
+              | Some (LessThan, Int n) when n > min_int -> false
               | Some _ -> true
               | None -> true)
             | _ -> false)

impl/query_where.ml

@@ -1063,7 +1063,7 @@ end) = struct
 
   let avet_index_start predicate threshold =
     match predicate, threshold with
-    | GreaterThan, Int n -> Some (Int (n + 1))
+    | GreaterThan, Int n when n < max_int -> Some (Int (n + 1))
     | GreaterOrEqual, value | GreaterThan, value -> Some value
     | _ -> None
 
@@ -1078,7 +1078,9 @@ end) = struct
       (function
         | ComparisonPredicate (predicate, left, right) -> (
           match range_predicate_for_var value_var predicate left right with
-          | Some (GreaterThan, Int _) | Some (LessThan, Int _) -> false
+          (* Tightened Int bounds (n±1) are exact only when they do not overflow. *)
+          | Some (GreaterThan, Int n) when n < max_int -> false
+          | Some (LessThan, Int n) when n > min_int -> false
           | Some _ -> true
           | None -> true)
         | _ -> false)

impl/transact.ml

@@ -1479,7 +1479,8 @@ let apply_tx context tx_ops db =
          else
            (* Build tx_data in O(n): [acc @ pieces] was O(n²) and dominated Share
               SQLite bulk loads (20k entities ≈ 100k facts). Index same-tx (e,a)
-              facts for card-one retraction without scanning the full acc list. *)
+              pieces (newest-first) and mirror [existing_attr_datoms] semantics:
+              from_db excludes in-tx retracts; from_acc is live added facts only. *)
            let tx_data =
              let by_ea = Hashtbl.create (List.length facts) in
              let acc_rev =
@@ -1488,11 +1489,22 @@ let apply_tx context tx_ops db =
                    let d =
                      { fact with v = context.resolve_context.normalize_value fact.v }
                    in
-                   let from_db = context.existing_entity_attr_datoms db d.e d.a in
+                   let same_tx_pieces =
+                     Option.value (Hashtbl.find_opt by_ea (d.e, d.a)) ~default:[]
+                   in
+                   let retracted_in_tx ex =
+                     List.exists
+                       (fun pd -> not pd.added && context.same_fact pd ex)
+                       same_tx_pieces
+                   in
+                   let from_db =
+                     context.existing_entity_attr_datoms db d.e d.a
+                     |> List.filter (fun ex -> not (retracted_in_tx ex))
+                   in
                    let from_acc =
-                     match Hashtbl.find_opt by_ea (d.e, d.a) with
-                     | None -> []
-                     | Some ds -> List.rev ds
+                     same_tx_pieces
+                     |> List.filter (fun pd -> pd.added && not (retracted_in_tx pd))
+                     |> List.rev
                    in
                    let existing = from_db @ from_acc in
                    let same_fact_exists = List.exists (context.same_fact d) existing in

test/dune

@@ -49,6 +49,16 @@
  (modules test_query_exec_parity)
  (libraries datascript-ocaml-native test_support alcotest))
 
+(test
+ (name test_query_result_cache)
+ (modules test_query_result_cache)
+ (libraries datascript-ocaml-native test_support alcotest))
+
+(test
+ (name test_bulk_card_one)
+ (modules test_bulk_card_one)
+ (libraries datascript-ocaml-native test_support alcotest))
+
 (test
  (name test_shared_queries)
  (modules test_shared_queries)

test/test_bulk_card_one.ml

@@ -0,0 +1,96 @@
+(** Bulk card-one tx_data: same-tx re-assert after update must keep the final value. *)
+
+open Alcotest
+open Datascript
+open Test_alcotest_support
+
+let indexed =
+  { cardinality = One
+  ; unique = None
+  ; indexed = true
+  ; is_component = false
+  ; no_history = false
+  ; doc = None
+  ; value_type = None
+  ; tuple_attrs = None
+  ; tuple_types = None
+  }
+
+let int_ages db e =
+  datoms db Eavt ~e ~a:"age" ()
+  |> List.of_seq
+  |> List.map (fun d -> match d.v with Int n -> n | _ -> -1)
+  |> List.sort compare
+
+let test_bulk_same_tx_reassert_original_after_update () =
+  (* Regression for O(n) by_ea path: from_db must ignore in-tx retracts, and
+     from_acc must only include live added facts. Otherwise age 20→30→20 drops
+     the final assert because from_db still looks like age=20 is live. *)
+  let db =
+    empty_db ~schema:[ "age", indexed; "name", indexed ] ()
+    |> db_with
+         [ Add (Entity_id 1, "name", String "Alice")
+         ; Add (Entity_id 1, "age", Int 20)
+         ]
+  in
+  let report =
+    transact db
+      [ Add (Entity_id 1, "age", Int 30); Add (Entity_id 1, "age", Int 20) ]
+  in
+  check_int_list "live age is final re-assert 20" [ 20 ] (int_ages report.db_after 1);
+  let asserted_ages =
+    report.tx_data
+    |> List.filter (fun d -> d.a = "age" && d.added)
+    |> List.map (fun d -> match d.v with Int n -> n | _ -> -1)
+  in
+  check_int_list "tx_data asserts both intermediate and final ages" [ 30; 20 ] asserted_ages;
+  let final_assert_present =
+    List.exists
+      (fun d -> d.a = "age" && d.added && d.v = Int 20)
+      report.tx_data
+  in
+  check_bool "final age=20 assert present in tx_data" true final_assert_present
+
+let test_bulk_same_tx_multiple_updates () =
+  let db =
+    empty_db ~schema:[ "age", indexed ] ()
+    |> db_with [ Add (Entity_id 1, "age", Int 1) ]
+  in
+  let report =
+    transact db
+      [ Add (Entity_id 1, "age", Int 2)
+      ; Add (Entity_id 1, "age", Int 3)
+      ; Add (Entity_id 1, "age", Int 4)
+      ]
+  in
+  check_int_list "live age is last write" [ 4 ] (int_ages report.db_after 1)
+
+let test_bulk_new_entity_card_one_updates () =
+  (* Bulk path with tempids: same-tx card-one updates on a newly allocated entity. *)
+  let report =
+    transact
+      (empty_db ~schema:[ "age", indexed; "name", indexed ] ())
+      [ Entity
+          { db_id = Some (Temp_id "a")
+          ; attrs =
+              [ "name", One_value (String "Ada")
+              ; "age", One_value (Int 10)
+              ]
+          }
+      ; Add (Temp_id "a", "age", Int 11)
+      ; Add (Temp_id "a", "age", Int 10)
+      ]
+  in
+  match List.assoc_opt "a" report.tempids with
+  | None -> fail "tempid a should resolve"
+  | Some e -> check_int_list "new entity ends at age 10" [ 10 ] (int_ages report.db_after e)
+
+let () =
+  run "bulk_card_one"
+    [ ( "same_tx"
+      , [ test_case "reassert original after update" `Quick
+            test_bulk_same_tx_reassert_original_after_update
+        ; test_case "multiple updates keep last" `Quick test_bulk_same_tx_multiple_updates
+        ; test_case "new entity tempid updates" `Quick test_bulk_new_entity_card_one_updates
+        ] )
+    ]