feat(soup): add date ast filters#2893
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds a new Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
seanaye
force-pushed
the
seanaye/feat/soup-date-ast
branch
2 times, most recently
from
8241505 to
ef5ff0b
Compare
![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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/cloud-storage/frecency/src/outbound/postgres/dynamic.rs`:
- Around line 195-206: Add integration tests in dynamic_tests.rs that exercise
the new DateLiteral filter arms in dynamic.rs: construct filter expressions
using filter_ast::ExprFrame::Literal with DocumentLiteral::CreatedAt and
DocumentLiteral::UpdatedAt combined with DateLiteral::GreaterThan and
DateLiteral::LessThan, insert sample Document rows with known
createdAt/updatedAt timestamps, invoke the same query-building path (the
function in dynamic.rs that turns a filter into SQL) and assert the generated
SQL contains the expected timestamp comparison and that the runtime query
returns the correct rows; ensure you cover all four combinations (CreatedAt
GreaterThan/LessThan and UpdatedAt GreaterThan/LessThan).
In `@rust/cloud-storage/item_filters/src/ast/chat.rs`:
- Around line 28-33: The enum ChatLiteral mixes serde renames (CreatedAt ->
"ca", UpdatedAt -> "ua") with default-serialized variants (ProjectId, Role,
ChatId, Owner, Importance, NotificationDone, NotificationSeen), causing
inconsistent JSON shapes; update the enum(s) to use a consistent serialization
strategy: either remove the #[serde(rename = "...")] attributes from CreatedAt
and UpdatedAt so ChatLiteral (and ProjectLiteral) all serialize by their Rust
variant names, or add explicit short renames for every variant (ProjectId, Role,
ChatId, Owner, Importance, NotificationDone, NotificationSeen, CreatedAt,
UpdatedAt) to match the DocumentLiteral style — apply the same change in
rust/cloud-storage/item_filters/src/ast/project.rs to keep both enums
consistent.
In `@rust/cloud-storage/item_filters/src/ast/date.rs`:
- Around line 6-13: The DateLiteral enum only exposes strict comparisons
(GreaterThan/gt and LessThan/lt), which forces clients to nudge timestamps; add
inclusive boundary variants GreaterThanOrEqual (serde rename "gte") and
LessThanOrEqual (serde rename "lte") to DateLiteral to support >=/<= semantics,
update any pattern matches and consumers of DateLiteral (e.g., match arms and
predicate builders) to handle the new variants, add serde-renames exactly
"gte"/"lte" so the enum remains compatible with external clients, and add/adjust
unit tests and serialization/deserialization tests to cover the new variants.
In `@rust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/dynamic.rs`:
- Around line 350-361: The four nearly identical match arms handling
filter_ast::ExprFrame::Literal for DocumentLiteral::CreatedAt and
DocumentLiteral::UpdatedAt using DateLiteral::GreaterThan/LessThan should be
consolidated into a small helper function (e.g., date_predicate) that takes the
column SQL name (like r#"d."createdAt""# or r#"d."updatedAt""#) and a
&DateLiteral and returns the formatted timestamptz comparison string; implement
date_predicate to match on DateLiteral::GreaterThan/DateLiteral::LessThan and
use dt.to_rfc3339(), then replace the four arms in dynamic.rs with two arms that
call date_predicate("d.\"createdAt\"", lit) and
date_predicate("d.\"updatedAt\"", lit) respectively (referencing the match
patterns filter_ast::ExprFrame::Literal, DocumentLiteral::CreatedAt/UpdatedAt,
and DateLiteral).
In `@rust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/tests.rs`:
- Around line 5375-6401: The tests duplicate constructing EntityFilterAst,
calling expanded_dynamic_cursor_soup, and counting SoupItem variants; extract
small helpers to centralize this: add a constructor like
EntityFilterAst::mock_empty() (or a local fn mock_empty_ast()) and per-type
builders (e.g., doc_ast(lit: DocumentLiteral), chat_ast(lit: ChatLiteral),
project_ast(lit: ProjectLiteral)) that set the specific filter and use
..mock_empty(); add an async helper run_and_count(db: &PgPool, ast:
EntityFilterAst) -> anyhow::Result<(Vec<SoupItem>, usize, usize, usize)> which
calls expanded_dynamic_cursor_soup and returns items plus (doc_count,
chat_count, project_count); then rewrite each test
(test_date_filter_document_created_at_gt, test_date_filter_chat_updated_at_lt,
test_date_filter_project_updated_at_range, etc.) to build the AST with the new
builders and call run_and_count, keeping only the cutoff, assertions on the
entity timestamps, and expected counts.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8315e26e-96fc-4b1f-ad2b-86bb9bd24200
⛔ Files ignored due to path filters (4)
rust/cloud-storage/.sqlx/query-0f2057f8430788bb20f5d0f144445f28fba6434c9a2fe208e0a4a45b892dea2b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/.sqlx/query-418d7b5ab3d820694166d0a3e3b17b9fc9b9bc8e769d50fddf4c06f09e855b9b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/.sqlx/query-50332c40ef917c839c4539f12419035115d61a4c9e8146a894e41db59b0f0daa.jsonis excluded by!**/.sqlx/**rust/cloud-storage/.sqlx/query-b470add8d87f6fff56d158c2aa9bf5dd871973fd3b9b6e1f25033f24a2cde3bd.jsonis excluded by!**/.sqlx/**
📒 Files selected for processing (8)
rust/cloud-storage/frecency/src/outbound/postgres/dynamic.rsrust/cloud-storage/item_filters/src/ast.rsrust/cloud-storage/item_filters/src/ast/chat.rsrust/cloud-storage/item_filters/src/ast/date.rsrust/cloud-storage/item_filters/src/ast/document.rsrust/cloud-storage/item_filters/src/ast/project.rsrust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/dynamic.rsrust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/tests.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust/cloud-storage/frecency/src/outbound/postgres/dynamic_tests.rs`:
- Around line 755-1109: Add tests covering inclusive date operators by mirroring
the existing document date tests
(test_dynamic_filter_document_date_created_at_gt/lt and
test_dynamic_filter_document_date_updated_at_gt/lt) but using
DateLiteral::GreaterThanOrEqual and DateLiteral::LessThanOrEqual with the cutoff
set exactly equal to one inserted document's timestamp (e.g., use doc_mid's
timestamp as the cutoff). Ensure the GreaterThanOrEqual test asserts the doc
with timestamp equal to cutoff is included (alongside later docs), and the
LessThanOrEqual test asserts the doc with timestamp equal to cutoff is included
(alongside earlier docs); reuse the same insert_doc_with_timestamps,
setup_date_filter_user, FrecencyPgStorage::set_aggregate, and get_top_entities
flow so the tests exercise SQL operator mapping for CreatedAt and UpdatedAt.
In `@rust/cloud-storage/frecency/src/outbound/postgres/dynamic.rs`:
- Around line 195-218: The match arms for
filter_ast::ExprFrame::Literal(DocumentLiteral::CreatedAt(...)) and
DocumentLiteral::UpdatedAt(...) duplicate the DateLiteral -> SQL operator
mapping across many branches; extract a single helper (e.g. date_literal_sql_op
or format_date_filter) that takes the field name ("createdAt" or "updatedAt")
and a DateLiteral and returns the complete SQL snippet using dt.to_rfc3339() and
the correct operator, then replace each repeated arm with calls to that helper;
ensure the helper implements the same operator mapping for DateLiteral
(GreaterThan, LessThan, GreaterThanOrEqual, LessThanOrEqual) as used elsewhere
(the centralized mapping) so behavior remains identical.
In `@rust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/tests.rs`:
- Around line 5451-6047: Add tests that cover the new inclusive date operators:
duplicate a few existing date tests (e.g.,
test_date_filter_document_created_at_gt/lt, chat/project createdAt/updatedAt
range tests) but use DateLiteral::GreaterThanOrEqual and
DateLiteral::LessThanOrEqual (and inclusive ranges via Expr::And with those
literals) so the GreaterThanOrEqual / LessThanOrEqual branches in
expanded/dynamic.rs are exercised; construct cutoffs equal to exact fixture
timestamps (the timestamps used in the fixtures, e.g. the 11:00/12:00
boundaries) and assert both the counts and that each returned SoupItem
(Document/Chat/Project) satisfies >= or <= against the cutoff using
run_and_count, doc_ast/chat_ast/project_ast or an EntityFilterAst with
Expr::Literal(DocumentLiteral::CreatedAt(...)) as appropriate; after adding
tests run just prepare_db to refresh SQLx cache.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 3ced7d1d-58b4-4652-94c0-f468c310c26b
📒 Files selected for processing (7)
rust/cloud-storage/frecency/src/outbound/postgres/dynamic.rsrust/cloud-storage/frecency/src/outbound/postgres/dynamic_tests.rsrust/cloud-storage/item_filters/src/ast/chat.rsrust/cloud-storage/item_filters/src/ast/date.rsrust/cloud-storage/item_filters/src/ast/project.rsrust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/dynamic.rsrust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/tests.rs
Comment on lines
+755
to
+1109
| #[sqlx::test(migrator = "MACRO_DB_MIGRATIONS")] | ||
| async fn test_dynamic_filter_document_date_created_at_gt(pool: PgPool) { | ||
| let storage = FrecencyPgStorage::new(pool.clone()); | ||
| let user_id = MacroUserIdStr::parse_from_str("macro|test@example.com").unwrap(); | ||
| setup_date_filter_user(&pool, &user_id).await; | ||
|
|
||
| let doc_early = Uuid::new_v4(); | ||
| let doc_mid = Uuid::new_v4(); | ||
| let doc_late = Uuid::new_v4(); | ||
|
|
||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_early, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T10:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_mid, | ||
| user_id.as_ref(), | ||
| "2023-01-01T12:00:00Z", | ||
| "2023-01-01T12:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_late, | ||
| user_id.as_ref(), | ||
| "2023-01-01T14:00:00Z", | ||
| "2023-01-01T14:00:00Z", | ||
| ) | ||
| .await; | ||
|
|
||
| for (id, score) in [ | ||
| (doc_early.to_string(), 100.0), | ||
| (doc_mid.to_string(), 90.0), | ||
| (doc_late.to_string(), 80.0), | ||
| ] { | ||
| storage | ||
| .set_aggregate(AggregateFrecency { | ||
| id: AggregateId { | ||
| entity: EntityType::Document.with_entity_string(id), | ||
| user_id: user_id.clone(), | ||
| }, | ||
| data: FrecencyData { | ||
| event_count: 1, | ||
| frecency_score: score, | ||
| first_event: Utc::now(), | ||
| recent_events: VecDeque::new(), | ||
| }, | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
| } | ||
|
|
||
| let cutoff: DateTime<Utc> = DateTime::parse_from_rfc3339("2023-01-01T11:00:00Z") | ||
| .unwrap() | ||
| .into(); | ||
| let filter = EntityFilterAst { | ||
| document_filter: Some(Arc::new(Expr::Literal(DocumentLiteral::CreatedAt( | ||
| DateLiteral::GreaterThan(cutoff), | ||
| )))), | ||
| project_filter: None, | ||
| chat_filter: None, | ||
| email_filter: None, | ||
| channel_filter: None, | ||
| call_filter: None, | ||
| properties_filter: None, | ||
| }; | ||
|
|
||
| let results = storage | ||
| .get_top_entities(FrecencyPageRequest { | ||
| user_id: user_id.copied(), | ||
| from_score: None, | ||
| limit: 10, | ||
| filters: Some(filter), | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(results.len(), 2); | ||
| let ids: Vec<_> = results | ||
| .iter() | ||
| .map(|r| r.id.entity.entity_id.clone()) | ||
| .collect(); | ||
| assert!(ids.contains(&doc_mid.to_string())); | ||
| assert!(ids.contains(&doc_late.to_string())); | ||
| } | ||
|
|
||
| #[sqlx::test(migrator = "MACRO_DB_MIGRATIONS")] | ||
| async fn test_dynamic_filter_document_date_created_at_lt(pool: PgPool) { | ||
| let storage = FrecencyPgStorage::new(pool.clone()); | ||
| let user_id = MacroUserIdStr::parse_from_str("macro|test@example.com").unwrap(); | ||
| setup_date_filter_user(&pool, &user_id).await; | ||
|
|
||
| let doc_early = Uuid::new_v4(); | ||
| let doc_mid = Uuid::new_v4(); | ||
| let doc_late = Uuid::new_v4(); | ||
|
|
||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_early, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T10:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_mid, | ||
| user_id.as_ref(), | ||
| "2023-01-01T12:00:00Z", | ||
| "2023-01-01T12:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_late, | ||
| user_id.as_ref(), | ||
| "2023-01-01T14:00:00Z", | ||
| "2023-01-01T14:00:00Z", | ||
| ) | ||
| .await; | ||
|
|
||
| for (id, score) in [ | ||
| (doc_early.to_string(), 100.0), | ||
| (doc_mid.to_string(), 90.0), | ||
| (doc_late.to_string(), 80.0), | ||
| ] { | ||
| storage | ||
| .set_aggregate(AggregateFrecency { | ||
| id: AggregateId { | ||
| entity: EntityType::Document.with_entity_string(id), | ||
| user_id: user_id.clone(), | ||
| }, | ||
| data: FrecencyData { | ||
| event_count: 1, | ||
| frecency_score: score, | ||
| first_event: Utc::now(), | ||
| recent_events: VecDeque::new(), | ||
| }, | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
| } | ||
|
|
||
| let cutoff: DateTime<Utc> = DateTime::parse_from_rfc3339("2023-01-01T11:00:00Z") | ||
| .unwrap() | ||
| .into(); | ||
| let filter = EntityFilterAst { | ||
| document_filter: Some(Arc::new(Expr::Literal(DocumentLiteral::CreatedAt( | ||
| DateLiteral::LessThan(cutoff), | ||
| )))), | ||
| project_filter: None, | ||
| chat_filter: None, | ||
| email_filter: None, | ||
| channel_filter: None, | ||
| call_filter: None, | ||
| properties_filter: None, | ||
| }; | ||
|
|
||
| let results = storage | ||
| .get_top_entities(FrecencyPageRequest { | ||
| user_id: user_id.copied(), | ||
| from_score: None, | ||
| limit: 10, | ||
| filters: Some(filter), | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(results.len(), 1); | ||
| assert_eq!(results[0].id.entity.entity_id, doc_early.to_string()); | ||
| } | ||
|
|
||
| #[sqlx::test(migrator = "MACRO_DB_MIGRATIONS")] | ||
| async fn test_dynamic_filter_document_date_updated_at_gt(pool: PgPool) { | ||
| let storage = FrecencyPgStorage::new(pool.clone()); | ||
| let user_id = MacroUserIdStr::parse_from_str("macro|test@example.com").unwrap(); | ||
| setup_date_filter_user(&pool, &user_id).await; | ||
|
|
||
| let doc_early = Uuid::new_v4(); | ||
| let doc_mid = Uuid::new_v4(); | ||
| let doc_late = Uuid::new_v4(); | ||
|
|
||
| // createdAt is old for all; updatedAt varies | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_early, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T10:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_mid, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T12:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_late, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T14:00:00Z", | ||
| ) | ||
| .await; | ||
|
|
||
| for (id, score) in [ | ||
| (doc_early.to_string(), 100.0), | ||
| (doc_mid.to_string(), 90.0), | ||
| (doc_late.to_string(), 80.0), | ||
| ] { | ||
| storage | ||
| .set_aggregate(AggregateFrecency { | ||
| id: AggregateId { | ||
| entity: EntityType::Document.with_entity_string(id), | ||
| user_id: user_id.clone(), | ||
| }, | ||
| data: FrecencyData { | ||
| event_count: 1, | ||
| frecency_score: score, | ||
| first_event: Utc::now(), | ||
| recent_events: VecDeque::new(), | ||
| }, | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
| } | ||
|
|
||
| let cutoff: DateTime<Utc> = DateTime::parse_from_rfc3339("2023-01-01T11:00:00Z") | ||
| .unwrap() | ||
| .into(); | ||
| let filter = EntityFilterAst { | ||
| document_filter: Some(Arc::new(Expr::Literal(DocumentLiteral::UpdatedAt( | ||
| DateLiteral::GreaterThan(cutoff), | ||
| )))), | ||
| project_filter: None, | ||
| chat_filter: None, | ||
| email_filter: None, | ||
| channel_filter: None, | ||
| call_filter: None, | ||
| properties_filter: None, | ||
| }; | ||
|
|
||
| let results = storage | ||
| .get_top_entities(FrecencyPageRequest { | ||
| user_id: user_id.copied(), | ||
| from_score: None, | ||
| limit: 10, | ||
| filters: Some(filter), | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(results.len(), 2); | ||
| let ids: Vec<_> = results | ||
| .iter() | ||
| .map(|r| r.id.entity.entity_id.clone()) | ||
| .collect(); | ||
| assert!(ids.contains(&doc_mid.to_string())); | ||
| assert!(ids.contains(&doc_late.to_string())); | ||
| } | ||
|
|
||
| #[sqlx::test(migrator = "MACRO_DB_MIGRATIONS")] | ||
| async fn test_dynamic_filter_document_date_updated_at_lt(pool: PgPool) { | ||
| let storage = FrecencyPgStorage::new(pool.clone()); | ||
| let user_id = MacroUserIdStr::parse_from_str("macro|test@example.com").unwrap(); | ||
| setup_date_filter_user(&pool, &user_id).await; | ||
|
|
||
| let doc_early = Uuid::new_v4(); | ||
| let doc_mid = Uuid::new_v4(); | ||
| let doc_late = Uuid::new_v4(); | ||
|
|
||
| // createdAt is old for all; updatedAt varies | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_early, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T10:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_mid, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T12:00:00Z", | ||
| ) | ||
| .await; | ||
| insert_doc_with_timestamps( | ||
| &pool, | ||
| doc_late, | ||
| user_id.as_ref(), | ||
| "2023-01-01T10:00:00Z", | ||
| "2023-01-01T14:00:00Z", | ||
| ) | ||
| .await; | ||
|
|
||
| for (id, score) in [ | ||
| (doc_early.to_string(), 100.0), | ||
| (doc_mid.to_string(), 90.0), | ||
| (doc_late.to_string(), 80.0), | ||
| ] { | ||
| storage | ||
| .set_aggregate(AggregateFrecency { | ||
| id: AggregateId { | ||
| entity: EntityType::Document.with_entity_string(id), | ||
| user_id: user_id.clone(), | ||
| }, | ||
| data: FrecencyData { | ||
| event_count: 1, | ||
| frecency_score: score, | ||
| first_event: Utc::now(), | ||
| recent_events: VecDeque::new(), | ||
| }, | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
| } | ||
|
|
||
| let cutoff: DateTime<Utc> = DateTime::parse_from_rfc3339("2023-01-01T11:00:00Z") | ||
| .unwrap() | ||
| .into(); | ||
| let filter = EntityFilterAst { | ||
| document_filter: Some(Arc::new(Expr::Literal(DocumentLiteral::UpdatedAt( | ||
| DateLiteral::LessThan(cutoff), | ||
| )))), | ||
| project_filter: None, | ||
| chat_filter: None, | ||
| email_filter: None, | ||
| channel_filter: None, | ||
| call_filter: None, | ||
| properties_filter: None, | ||
| }; | ||
|
|
||
| let results = storage | ||
| .get_top_entities(FrecencyPageRequest { | ||
| user_id: user_id.copied(), | ||
| from_score: None, | ||
| limit: 10, | ||
| filters: Some(filter), | ||
| }) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| assert_eq!(results.len(), 1); | ||
| assert_eq!(results[0].id.entity.entity_id, doc_early.to_string()); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add coverage for gte/lte and exact-cutoff boundary behavior.
Nice coverage for gt/lt, but the new DateLiteral surface also includes inclusive operators. Add at least one equality-boundary case (timestamp exactly equal to cutoff) for GreaterThanOrEqual and LessThanOrEqual to catch SQL operator mapping regressions early.
Proposed test additions
+#[sqlx::test(migrator = "MACRO_DB_MIGRATIONS")]
+async fn test_dynamic_filter_document_date_created_at_gte_includes_equal(pool: PgPool) {
+ // seed docs at 10:00, 12:00, 14:00 + frecency aggregates (same pattern as existing tests)
+ // cutoff = 12:00
+ // filter: DocumentLiteral::CreatedAt(DateLiteral::GreaterThanOrEqual(cutoff))
+ // assert includes 12:00 and 14:00, excludes 10:00
+}
+
+#[sqlx::test(migrator = "MACRO_DB_MIGRATIONS")]
+async fn test_dynamic_filter_document_date_updated_at_lte_includes_equal(pool: PgPool) {
+ // seed docs with updatedAt at 10:00, 12:00, 14:00 + frecency aggregates
+ // cutoff = 12:00
+ // filter: DocumentLiteral::UpdatedAt(DateLiteral::LessThanOrEqual(cutoff))
+ // assert includes 10:00 and 12:00, excludes 14:00
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/frecency/src/outbound/postgres/dynamic_tests.rs` around
lines 755 - 1109, Add tests covering inclusive date operators by mirroring the
existing document date tests (test_dynamic_filter_document_date_created_at_gt/lt
and test_dynamic_filter_document_date_updated_at_gt/lt) but using
DateLiteral::GreaterThanOrEqual and DateLiteral::LessThanOrEqual with the cutoff
set exactly equal to one inserted document's timestamp (e.g., use doc_mid's
timestamp as the cutoff). Ensure the GreaterThanOrEqual test asserts the doc
with timestamp equal to cutoff is included (alongside later docs), and the
LessThanOrEqual test asserts the doc with timestamp equal to cutoff is included
(alongside earlier docs); reuse the same insert_doc_with_timestamps,
setup_date_filter_user, FrecencyPgStorage::set_aggregate, and get_top_entities
flow so the tests exercise SQL operator mapping for CreatedAt and UpdatedAt.
Comment on lines
+5451
to
+6047
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_document_created_at_gt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // cutoff between bbbb(11:00) and cccc(12:00) → expect cccc, dddd, eeee = 3 docs | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T11:30:00Z")?.into(); | ||
| let (items, doc_count, chat_count, project_count) = run_and_count( | ||
| &db, | ||
| doc_ast(DocumentLiteral::CreatedAt(DateLiteral::GreaterThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Document(d) = item { | ||
| assert!( | ||
| d.created_at > cutoff, | ||
| "doc {} created_at {:?} should be > cutoff", | ||
| d.id, | ||
| d.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 3, "expected 3 docs with createdAt after cutoff"); | ||
| assert_eq!(chat_count, 4, "chats unaffected by document date filter"); | ||
| assert_eq!( | ||
| project_count, 4, | ||
| "projects unaffected by document date filter" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_document_created_at_lt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // cutoff between bbbb(11:00) and cccc(12:00) → expect aaaa, bbbb = 2 docs | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T11:30:00Z")?.into(); | ||
| let (items, doc_count, chat_count, project_count) = run_and_count( | ||
| &db, | ||
| doc_ast(DocumentLiteral::CreatedAt(DateLiteral::LessThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Document(d) = item { | ||
| assert!( | ||
| d.created_at < cutoff, | ||
| "doc {} created_at {:?} should be < cutoff", | ||
| d.id, | ||
| d.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 2, "expected 2 docs with createdAt before cutoff"); | ||
| assert_eq!(chat_count, 4, "chats unaffected by document date filter"); | ||
| assert_eq!( | ||
| project_count, 4, | ||
| "projects unaffected by document date filter" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_chat_created_at_gt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // cutoff between b2b2(11:00) and c3c3(12:00) → expect c3c3, d4d4 = 2 chats | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-06T11:30:00Z")?.into(); | ||
| let (items, doc_count, chat_count, project_count) = run_and_count( | ||
| &db, | ||
| chat_ast(ChatLiteral::CreatedAt(DateLiteral::GreaterThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Chat(c) = item { | ||
| assert!( | ||
| c.created_at > cutoff, | ||
| "chat {} created_at {:?} should be > cutoff", | ||
| c.id, | ||
| c.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 5, "docs unaffected by chat date filter"); | ||
| assert_eq!( | ||
| chat_count, 2, | ||
| "expected 2 chats with createdAt after cutoff" | ||
| ); | ||
| assert_eq!(project_count, 4, "projects unaffected by chat date filter"); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_project_created_at_gt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // cutoff between 2222(Jan-01 11:00) and 3333(Jan-01 12:00) → expect 3333, 4444 = 2 projects | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-01T11:30:00Z")?.into(); | ||
| let (items, doc_count, chat_count, project_count) = run_and_count( | ||
| &db, | ||
| project_ast(ProjectLiteral::CreatedAt(DateLiteral::GreaterThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Project(p) = item { | ||
| assert!( | ||
| p.created_at > cutoff, | ||
| "project {} created_at {:?} should be > cutoff", | ||
| p.id, | ||
| p.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 5, "docs unaffected by project date filter"); | ||
| assert_eq!(chat_count, 4, "chats unaffected by project date filter"); | ||
| assert_eq!( | ||
| project_count, 2, | ||
| "expected 2 projects with createdAt after cutoff" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // document / createdAt / AND(gt, lt) | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_document_created_at_range(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // lower after aaaa(10:00), upper before eeee(14:00) → bbbb, cccc, dddd = 3 docs | ||
| let lower: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T10:30:00Z")?.into(); | ||
| let upper: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T13:30:00Z")?.into(); | ||
| let ast = EntityFilterAst { | ||
| document_filter: Some(Arc::new(Expr::And( | ||
| Box::new(Expr::Literal(DocumentLiteral::CreatedAt( | ||
| DateLiteral::GreaterThan(lower), | ||
| ))), | ||
| Box::new(Expr::Literal(DocumentLiteral::CreatedAt( | ||
| DateLiteral::LessThan(upper), | ||
| ))), | ||
| ))), | ||
| ..mock_empty_ast() | ||
| }; | ||
| let (items, doc_count, chat_count, project_count) = run_and_count(&db, ast).await?; | ||
| for item in &items { | ||
| if let SoupItem::Document(d) = item { | ||
| assert!( | ||
| d.created_at > lower && d.created_at < upper, | ||
| "doc {} created_at {:?} outside [{:?}, {:?}]", | ||
| d.id, | ||
| d.created_at, | ||
| lower, | ||
| upper | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 3, "expected 3 docs in range"); | ||
| assert_eq!(chat_count, 4, "chats unaffected"); | ||
| assert_eq!(project_count, 4, "projects unaffected"); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // document / updatedAt / gt | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_document_updated_at_gt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // fixture has updatedAt == createdAt; cutoff same as createdAt gt test → cccc, dddd, eeee = 3 | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T11:30:00Z")?.into(); | ||
| let (items, doc_count, _, _) = run_and_count( | ||
| &db, | ||
| doc_ast(DocumentLiteral::UpdatedAt(DateLiteral::GreaterThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Document(d) = item { | ||
| assert!( | ||
| d.updated_at > cutoff, | ||
| "doc {} updated_at {:?} should be > cutoff", | ||
| d.id, | ||
| d.updated_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 3, "expected 3 docs with updatedAt after cutoff"); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // document / updatedAt / lt | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_document_updated_at_lt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T11:30:00Z")?.into(); | ||
| let (items, doc_count, _, _) = run_and_count( | ||
| &db, | ||
| doc_ast(DocumentLiteral::UpdatedAt(DateLiteral::LessThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Document(d) = item { | ||
| assert!( | ||
| d.updated_at < cutoff, | ||
| "doc {} updated_at {:?} should be < cutoff", | ||
| d.id, | ||
| d.updated_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 2, "expected 2 docs with updatedAt before cutoff"); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // document / updatedAt / AND(gt, lt) | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_document_updated_at_range(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| let lower: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T10:30:00Z")?.into(); | ||
| let upper: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-05T13:30:00Z")?.into(); | ||
| let ast = EntityFilterAst { | ||
| document_filter: Some(Arc::new(Expr::And( | ||
| Box::new(Expr::Literal(DocumentLiteral::UpdatedAt( | ||
| DateLiteral::GreaterThan(lower), | ||
| ))), | ||
| Box::new(Expr::Literal(DocumentLiteral::UpdatedAt( | ||
| DateLiteral::LessThan(upper), | ||
| ))), | ||
| ))), | ||
| ..mock_empty_ast() | ||
| }; | ||
| let (items, doc_count, _, _) = run_and_count(&db, ast).await?; | ||
| for item in &items { | ||
| if let SoupItem::Document(d) = item { | ||
| assert!( | ||
| d.updated_at > lower && d.updated_at < upper, | ||
| "doc {} updated_at {:?} outside range", | ||
| d.id, | ||
| d.updated_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(doc_count, 3, "expected 3 docs with updatedAt in range"); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // chat / createdAt / lt | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_chat_created_at_lt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // cutoff between b2b2(11:00) and c3c3(12:00) → a1a1, b2b2 = 2 chats | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-06T11:30:00Z")?.into(); | ||
| let (items, _, chat_count, _) = run_and_count( | ||
| &db, | ||
| chat_ast(ChatLiteral::CreatedAt(DateLiteral::LessThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Chat(c) = item { | ||
| assert!( | ||
| c.created_at < cutoff, | ||
| "chat {} created_at {:?} should be < cutoff", | ||
| c.id, | ||
| c.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!( | ||
| chat_count, 2, | ||
| "expected 2 chats with createdAt before cutoff" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // chat / createdAt / AND(gt, lt) | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_chat_created_at_range(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // lower after a1a1(10:00), upper before d4d4(13:00) → b2b2, c3c3 = 2 chats | ||
| let lower: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-06T10:30:00Z")?.into(); | ||
| let upper: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-06T12:30:00Z")?.into(); | ||
| let ast = EntityFilterAst { | ||
| chat_filter: Some(Arc::new(Expr::And( | ||
| Box::new(Expr::Literal(ChatLiteral::CreatedAt( | ||
| DateLiteral::GreaterThan(lower), | ||
| ))), | ||
| Box::new(Expr::Literal(ChatLiteral::CreatedAt( | ||
| DateLiteral::LessThan(upper), | ||
| ))), | ||
| ))), | ||
| ..mock_empty_ast() | ||
| }; | ||
| let (items, _, chat_count, _) = run_and_count(&db, ast).await?; | ||
| for item in &items { | ||
| if let SoupItem::Chat(c) = item { | ||
| assert!( | ||
| c.created_at > lower && c.created_at < upper, | ||
| "chat {} created_at {:?} outside range", | ||
| c.id, | ||
| c.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(chat_count, 2, "expected 2 chats in createdAt range"); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // chat / updatedAt / gt | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_chat_updated_at_gt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-06T11:30:00Z")?.into(); | ||
| let (items, _, chat_count, _) = run_and_count( | ||
| &db, | ||
| chat_ast(ChatLiteral::UpdatedAt(DateLiteral::GreaterThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Chat(c) = item { | ||
| assert!( | ||
| c.updated_at > cutoff, | ||
| "chat {} updated_at {:?} should be > cutoff", | ||
| c.id, | ||
| c.updated_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!( | ||
| chat_count, 2, | ||
| "expected 2 chats with updatedAt after cutoff" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // chat / updatedAt / AND(gt, lt) | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_chat_updated_at_range(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| let lower: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-06T10:30:00Z")?.into(); | ||
| let upper: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-06T12:30:00Z")?.into(); | ||
| let ast = EntityFilterAst { | ||
| chat_filter: Some(Arc::new(Expr::And( | ||
| Box::new(Expr::Literal(ChatLiteral::UpdatedAt( | ||
| DateLiteral::GreaterThan(lower), | ||
| ))), | ||
| Box::new(Expr::Literal(ChatLiteral::UpdatedAt( | ||
| DateLiteral::LessThan(upper), | ||
| ))), | ||
| ))), | ||
| ..mock_empty_ast() | ||
| }; | ||
| let (items, _, chat_count, _) = run_and_count(&db, ast).await?; | ||
| for item in &items { | ||
| if let SoupItem::Chat(c) = item { | ||
| assert!( | ||
| c.updated_at > lower && c.updated_at < upper, | ||
| "chat {} updated_at {:?} outside range", | ||
| c.id, | ||
| c.updated_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!(chat_count, 2, "expected 2 chats with updatedAt in range"); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // project / createdAt / lt | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_project_created_at_lt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // cutoff between 2222(Jan-01 11:00) and 3333(Jan-01 12:00) → 1111, 2222 = 2 projects | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-01T11:30:00Z")?.into(); | ||
| let (items, _, _, project_count) = run_and_count( | ||
| &db, | ||
| project_ast(ProjectLiteral::CreatedAt(DateLiteral::LessThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Project(p) = item { | ||
| assert!( | ||
| p.created_at < cutoff, | ||
| "project {} created_at {:?} should be < cutoff", | ||
| p.id, | ||
| p.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!( | ||
| project_count, 2, | ||
| "expected 2 projects with createdAt before cutoff" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // project / createdAt / AND(gt, lt) | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_project_created_at_range(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| // lower after 1111(Jan-01 10:00), upper before 4444(Jan-02 10:00) → 2222, 3333 = 2 projects | ||
| let lower: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-01T10:30:00Z")?.into(); | ||
| let upper: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-01T12:30:00Z")?.into(); | ||
| let ast = EntityFilterAst { | ||
| project_filter: Some(Arc::new(Expr::And( | ||
| Box::new(Expr::Literal(ProjectLiteral::CreatedAt( | ||
| DateLiteral::GreaterThan(lower), | ||
| ))), | ||
| Box::new(Expr::Literal(ProjectLiteral::CreatedAt( | ||
| DateLiteral::LessThan(upper), | ||
| ))), | ||
| ))), | ||
| ..mock_empty_ast() | ||
| }; | ||
| let (items, _, _, project_count) = run_and_count(&db, ast).await?; | ||
| for item in &items { | ||
| if let SoupItem::Project(p) = item { | ||
| assert!( | ||
| p.created_at > lower && p.created_at < upper, | ||
| "project {} created_at {:?} outside range", | ||
| p.id, | ||
| p.created_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!( | ||
| project_count, 2, | ||
| "expected 2 projects with createdAt in range" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // project / updatedAt / gt | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_project_updated_at_gt(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| let cutoff: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-01T11:30:00Z")?.into(); | ||
| let (items, _, _, project_count) = run_and_count( | ||
| &db, | ||
| project_ast(ProjectLiteral::UpdatedAt(DateLiteral::GreaterThan(cutoff))), | ||
| ) | ||
| .await?; | ||
| for item in &items { | ||
| if let SoupItem::Project(p) = item { | ||
| assert!( | ||
| p.updated_at > cutoff, | ||
| "project {} updated_at {:?} should be > cutoff", | ||
| p.id, | ||
| p.updated_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!( | ||
| project_count, 2, | ||
| "expected 2 projects with updatedAt after cutoff" | ||
| ); | ||
| Ok(()) | ||
| } | ||
|
|
||
| // project / updatedAt / AND(gt, lt) | ||
| #[sqlx::test( | ||
| fixtures( | ||
| path = "../../../../../macro_db_client/fixtures", | ||
| scripts("entity_filter_tests") | ||
| ), | ||
| migrator = "MACRO_DB_MIGRATIONS" | ||
| )] | ||
| async fn test_date_filter_project_updated_at_range(db: PgPool) -> anyhow::Result<()> { | ||
| use chrono::DateTime; | ||
| let lower: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-01T10:30:00Z")?.into(); | ||
| let upper: chrono::DateTime<chrono::Utc> = | ||
| DateTime::parse_from_rfc3339("2023-01-01T12:30:00Z")?.into(); | ||
| let ast = EntityFilterAst { | ||
| project_filter: Some(Arc::new(Expr::And( | ||
| Box::new(Expr::Literal(ProjectLiteral::UpdatedAt( | ||
| DateLiteral::GreaterThan(lower), | ||
| ))), | ||
| Box::new(Expr::Literal(ProjectLiteral::UpdatedAt( | ||
| DateLiteral::LessThan(upper), | ||
| ))), | ||
| ))), | ||
| ..mock_empty_ast() | ||
| }; | ||
| let (items, _, _, project_count) = run_and_count(&db, ast).await?; | ||
| for item in &items { | ||
| if let SoupItem::Project(p) = item { | ||
| assert!( | ||
| p.updated_at > lower && p.updated_at < upper, | ||
| "project {} updated_at {:?} outside range", | ||
| p.id, | ||
| p.updated_at | ||
| ); | ||
| } | ||
| } | ||
| assert_eq!( | ||
| project_count, 2, | ||
| "expected 2 projects with updatedAt in range" | ||
| ); | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add equality-boundary coverage for >= and <= date filters.
This suite exercises GreaterThan, LessThan, and strict ranges, but it never hits the new GreaterThanOrEqual / LessThanOrEqual branches in expanded/dynamic.rs. A couple of boundary assertions using exact fixture timestamps would catch the most likely regression here.
As per coding guidelines: When making changes to a database crate, always update tests and run just prepare_db to update the SQLx cache.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/soup/src/outbound/pg_soup_repo/expanded/tests.rs` around
lines 5451 - 6047, Add tests that cover the new inclusive date operators:
duplicate a few existing date tests (e.g.,
test_date_filter_document_created_at_gt/lt, chat/project createdAt/updatedAt
range tests) but use DateLiteral::GreaterThanOrEqual and
DateLiteral::LessThanOrEqual (and inclusive ranges via Expr::And with those
literals) so the GreaterThanOrEqual / LessThanOrEqual branches in
expanded/dynamic.rs are exercised; construct cutoffs equal to exact fixture
timestamps (the timestamps used in the fixtures, e.g. the 11:00/12:00
boundaries) and assert both the counts and that each returned SoupItem
(Document/Chat/Project) satisfies >= or <= against the cutoff using
run_and_count, doc_ast/chat_ast/project_ast or an EntityFilterAst with
Expr::Literal(DocumentLiteral::CreatedAt(...)) as appropriate; after adding
tests run just prepare_db to refresh SQLx cache.
seanaye
force-pushed
the
seanaye/feat/soup-date-ast
branch
from
c3a172a to
865e008
Compare
seanaye
force-pushed
the
seanaye/feat/soup-date-ast
branch
from
865e008 to
d5ec2b0
Compare
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.
This PR adds date ast filters to soup