[DERCBOT-1676] Fix dialog date filters and add comprehensive tests #1969

antoine-billiet-arkea-partnre · 2025-12-22T09:02:08Z

This pull request introduces new support for filtering dialogs by their activity period overlap, in addition to creation date, and provides a reusable DSL for building MongoDB aggregation expressions. It also adds comprehensive unit tests to ensure correct mapping of date parameters in dialog search queries.

Dialog filtering enhancements:

Added dialogActivityFrom and dialogActivityTo fields to both DialogsSearchQuery and DialogReportQuery to allow filtering dialogs based on whether their activity period (from first to last action) overlaps a given range. This is documented and mapped in the conversion method. [1] [2] [3]

MongoDB aggregation improvements:

Introduced a new MongoAgg object providing a DSL for building MongoDB aggregation expressions, including helpers for date aggregation in nested arrays and for filtering by creation date or activity period overlap. This makes aggregation queries more readable and maintainable.

Testing:

Added a new test class DialogSearchServiceTest to verify that all date filtering parameters (creation and activity periods) are correctly mapped from DialogsSearchQuery to DialogReportQuery, including handling of nulls and combinations.

Code organization:

Updated imports in UserTimelineMongoDAO.kt to include explicit Kotlin standard library imports for better clarity and maintainability.

assouktim

Thanks for the update. A few changes to consider.

assouktim · 2025-12-30T10:27:32Z

bot/storage-mongo/src/main/kotlin/MongoAgg.kt

+ * ))
+ * ```
+ */
+object MongoAgg {


This utility class is not necessary in this case.
The functional requirement is simply to answer the following question:
“Is there at least one action in all the dialogue stories whose date meets the condition?”

The current code already meets this requirement in a simple, readable, and efficient way, without introducing additional logic to process arrays via $reduce, $cond, or $ifNull.

Stories.actions.date lt dialogCreationDateTo?.toInstant(): This filter, for example, applies directly to the field contained in the stories and actions arrays.
MongoDB will therefore match documents that contain at least one action whose date meets the condition.

This PR addresses two distinct filtering requirements:

1. Filter by Dialog Creation Date (dialogCreationDateFrom / dialogCreationDateTo)

Requirement: Filter dialogs where the creation date (= the date of the first/oldest action) falls within a specified period.

Condition: creationDateFrom <= min(actions.date) <= creationDateTo

For the upper bound (dialogCreationDateTo), the simple approach works because if min(date) <= toDate, then at least one action exists with date <= toDate.

However, for the lower bound (dialogCreationDateFrom), the simple approach fails:

Counter-example:

Dialog with actions at dates: [2025-01-01, 2025-01-15, 2025-01-30]

Filter: dialogCreationDateFrom = 2025-01-10 (dialogs created from January 10th)

Approach Result Correct?

Simple (Stories.actions.date gte 2025-01-10) Matches (Jan 15th action satisfies) Wrong

Aggregation (min(actions.date) >= 2025-01-10) No match (Jan 1st < Jan 10th) Correct

The dialog was created on January 1st, so it should NOT be returned when filtering for dialogs created from January 10th.

The Agg class with $reduce/$min is required for this filter.

2. Filter by Dialog Activity (dialogActivityFrom / dialogActivityTo)

Requirement: Filter dialogs that had activity during the specified period, meaning:

At least one action on or after the lower bound (activityFrom)

At least one action before the upper bound (activityTo)

These two conditions do not need to be on the same action.

Condition:

∃ action >= activityFrom (at least one action after the lower bound)

∃ action < activityTo (at least one action before the upper bound)

Mathematical equivalence:

Statement Equivalent to

∃ action >= activityFrom max(actions.date) >= activityFrom

∃ action < activityTo min(actions.date) < activityTo

This means both approaches produce the same result for the activity filter:

// Simple approach Stories.actions.date gte activityFrom Stories.actions.date lt activityTo // Aggregation approach (current implementation) Agg.filterByPeriodOverlap(...) // max >= from AND min < to

You are correct that the simple approach would work for this filter. I chose the aggregation approach for consistency with the creation date filter and to make the intent explicit (computing min/max), but both are functionally equivalent.

Summary

Filter Requirement Simple approach works? Agg required?

dialogCreationDate min(actions.date) within period No (fails for from) Yes

dialogActivity ∃ action >= from AND ∃ action < to Yes No (but used for consistency)

I prefer the simplest option.

assouktim · 2025-12-30T10:37:47Z

bot/storage-mongo/src/main/kotlin/UserTimelineMongoDAO.kt

-                                    } else {
-                                        Stories.actions.date lt dialogCreationDateTo?.toInstant()
-                                    },
+                                    buildDialogCreationDateFilter(


The gte and lte operators must be used to avoid the strict nature of the condition, especially since filtering is now performed at specific times.

You're right, there is an inconsistency in the date filtering operators across the codebase.

Current state of date filters

Filter from operator to operator Interval

annotationCreationDate gt (exclusive) lt (exclusive) (from, to)

dialogCreationDate gte (inclusive) lte (inclusive) [from, to]

dialogActivity gte (inclusive) lt (exclusive) [from, to)

The problem

Three different conventions for three different filters:

Annotations: Open interval (from, to) — both bounds are exclusive

Creation date: Closed interval [from, to] — both bounds are inclusive

Activity: Half-open interval [from, to) — lower bound inclusive, upper bound exclusive

This is indeed confusing and error-prone, especially when filtering at specific timestamps.

Proposed harmonization

I suggest adopting [from, to] (closed interval with gte/lte) for all date filters:

More intuitive for users: "from January 1st to January 31st" includes both dates

Consistent behavior across all filters

Avoids edge cases where exact timestamp matches are unexpectedly excluded

I will update:

annotationCreationDate: change gt → gte and lt → lte

dialogActivity: change lt → lte (and update MongoAgg.filterByPeriodOverlap)

Do you agree with this approach, or would you prefer a different convention (e.g., half-open [from, to) everywhere)?

Yes, a closed interval with gte/lte for all date filters.

assouktim · 2025-12-30T10:55:19Z

bot/storage-mongo/src/main/kotlin/UserTimelineMongoDAO.kt

+import kotlin.text.take
+import kotlin.text.trim
+import kotlin.to
+import kotlin.with


We discussed this block:
if (from == null) null else DialogCol_.LastUpdateDate gt from?.toInstant(), if (to == null) null else DialogCol_.LastUpdateDate lt to?.toInstant(),

we said that it was redundant and, above all, useless because the from and to contain dead code. Please double-check, and if that's correct, remove them from the query and filtering.

assouktim · 2025-12-30T11:04:28Z

bot/storage-mongo/src/test/kotlin/UserTimelineMongoDAOTest.kt

                UserTimelineMongoDAO.loadWithoutDialogs("namespace", newId).playerId,
            )
        }
+


I will validate this test later on. I am waiting for your feedback on my first review, as it will significantly impact this unit test.

assouktim · 2025-12-30T11:29:49Z

bot/admin/server/src/main/kotlin/model/DialogsSearchQuery.kt

    val dialogCreationDateTo: ZonedDateTime? = null,
+    /**
+     * Filter dialogs by activity period overlap.
+     * A dialog is included if its activity period (from first to last action) overlaps the filter range.


I'm not sure that's right. Normally, at least one action must be within the range, not all of the actions in the dialogue.

A dialogue must be included if at least one action is within the range.

see first comment.

assouktim · 2025-12-30T13:15:06Z

ktlint fails, try reformatting your code by doing: mvn clean antrun:run@ktlint-format
pre-commit too, see what's done here: https://github.com/theopenconversationkit/tock/pull/1962/files

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.

Copilot · 2026-01-13T13:39:06Z

bot/storage-mongo/src/test/kotlin/UserTimelineMongoDAOTest.kt

+                assertEquals(
+                    3L,
+                    resultToOnly.total,
+                    "DialogA, DialogB, and DialogC should be included when only dialogActivityTo is set (oldest < Dec 13). Found: ${resultToOnly.total}",


The test assertion message states "oldest < Dec 13" using strict less-than, but the implementation uses lte (<=), making the actual condition "oldest <= Dec 13". Update the message to reflect the inclusive bound for consistency with the documented behavior.

Suggested change

"DialogA, DialogB, and DialogC should be included when only dialogActivityTo is set (oldest < Dec 13). Found: ${resultToOnly.total}",

"DialogA, DialogB, and DialogC should be included when only dialogActivityTo is set (oldest <= Dec 13). Found: ${resultToOnly.total}",

Copilot · 2026-01-13T13:39:06Z

bot/storage-mongo/src/test/kotlin/UserTimelineMongoDAOTest.kt

+                // Dialog A: oldest=Dec 10, youngest=Dec 12 -> Dec 11 <= Dec 12 AND Dec 10 < Dec 13 -> TRUE
+                // Dialog B: oldest=Dec 12, youngest=Dec 14 -> Dec 11 <= Dec 14 AND Dec 12 < Dec 13 -> TRUE
+                // Dialog C: oldest=Dec 8, youngest=Dec 9 -> Dec 11 <= Dec 9 -> FALSE
+                // Dialog D: oldest=Dec 15, youngest=Dec 16 -> Dec 15 < Dec 13 -> FALSE


The test comment uses strict less-than for the toDate comparison "Dec 10 < Dec 13", "Dec 12 < Dec 13", "Dec 15 < Dec 13" but the implementation uses lte (<=). Update these to use <= for consistency with the inclusive bounds: "Dec 10 <= Dec 13 AND Dec 10 <= Dec 13 -> TRUE", "Dec 11 <= Dec 14 AND Dec 12 <= Dec 13 -> TRUE", "Dec 15 <= Dec 13 -> FALSE".

Copilot · 2026-01-13T13:39:07Z

bot/storage-mongo/src/test/kotlin/MongoAggregationDSLTest.kt

@@ -0,0 +1 @@
+


The MongoAggregationDSLTest.kt file appears to be essentially empty (only contains a newline). Given that MongoAgg.kt introduces a new DSL with complex aggregation logic for date filtering, comprehensive unit tests should be added to verify the correctness of functions like filterByOldestDateInPeriod, filterByPeriodOverlap, and the various aggregation helper functions (min, max, reduce, etc.). While UserTimelineMongoDAOTest.kt tests the integration, dedicated unit tests for MongoAgg would improve maintainability and make it easier to verify the DSL produces correct MongoDB expressions.

Copilot · 2026-01-13T13:39:07Z

bot/storage-mongo/src/test/kotlin/UserTimelineMongoDAOTest.kt

+                // Test 3: Only dialogActivityTo set -> condition: oldestDate < toDate
+                // Dialog A: oldest=Dec 10 -> Dec 10 < Dec 13 -> TRUE
+                // Dialog B: oldest=Dec 12 -> Dec 12 < Dec 13 -> TRUE
+                // Dialog C: oldest=Dec 8 -> Dec 8 < Dec 13 -> TRUE
+                // Dialog D: oldest=Dec 15 -> Dec 15 < Dec 13 -> FALSE


The test comment states "condition: oldestDate < toDate" using strict less-than, but the actual implementation uses lte (less than or equal), making the condition "oldestDate <= toDate". This is inconsistent with the comment. The comment should be updated to reflect the inclusive bound: "condition: oldestDate <= toDate". This is consistent with the documentation that states "Both bounds are inclusive" (lines 70-77 in DialogReportQuery.kt).

Suggested change

// Test 3: Only dialogActivityTo set -> condition: oldestDate < toDate

// Dialog A: oldest=Dec 10 -> Dec 10 < Dec 13 -> TRUE

// Dialog B: oldest=Dec 12 -> Dec 12 < Dec 13 -> TRUE

// Dialog C: oldest=Dec 8 -> Dec 8 < Dec 13 -> TRUE

// Dialog D: oldest=Dec 15 -> Dec 15 < Dec 13 -> FALSE

// Test 3: Only dialogActivityTo set -> condition: oldestDate <= toDate

// Dialog A: oldest=Dec 10 -> Dec 10 <= Dec 13 -> TRUE

// Dialog B: oldest=Dec 12 -> Dec 12 <= Dec 13 -> TRUE

// Dialog C: oldest=Dec 8 -> Dec 8 <= Dec 13 -> TRUE

// Dialog D: oldest=Dec 15 -> Dec 15 <= Dec 13 -> FALSE

Copilot · 2026-01-13T13:39:07Z

bot/storage-mongo/src/test/kotlin/UserTimelineMongoDAOTest.kt

+                    "DialogA, DialogB, and DialogC should be included when only dialogActivityTo is set (oldest < Dec 13). Found: ${resultToOnly.total}",
+                )
+
+                // Test 4: Both dates set -> condition: fromDate <= youngestDate AND oldestDate < toDate


The test comment states "condition: fromDate <= youngestDate AND oldestDate < toDate" using strict less-than for the toDate condition, but the actual implementation uses lte (less than or equal), making it "oldestDate <= toDate". The comment should be updated to reflect the inclusive bound: "fromDate <= youngestDate AND oldestDate <= toDate". This is consistent with the documentation that states "Both bounds are inclusive".

Copilot · 2026-01-13T13:39:07Z

bot/storage-mongo/src/test/kotlin/UserTimelineMongoDAOTest.kt

+                // Test 3: Only dialogActivityTo set -> condition: oldestDate < toDate
+                // Dialog A: oldest=Dec 10 -> Dec 10 < Dec 13 -> TRUE
+                // Dialog B: oldest=Dec 12 -> Dec 12 < Dec 13 -> TRUE
+                // Dialog C: oldest=Dec 8 -> Dec 8 < Dec 13 -> TRUE
+                // Dialog D: oldest=Dec 15 -> Dec 15 < Dec 13 -> FALSE


The test comment uses strict less-than "Dec 10 < Dec 13", "Dec 12 < Dec 13", "Dec 8 < Dec 13", "Dec 15 < Dec 13" but the implementation uses lte (<=). For consistency with the inclusive bounds documented elsewhere, these comparisons should use <= in the comments. For example: "Dialog A: oldest=Dec 10 -> Dec 10 <= Dec 13 -> TRUE".

Suggested change

// Test 3: Only dialogActivityTo set -> condition: oldestDate < toDate

// Dialog A: oldest=Dec 10 -> Dec 10 < Dec 13 -> TRUE

// Dialog B: oldest=Dec 12 -> Dec 12 < Dec 13 -> TRUE

// Dialog C: oldest=Dec 8 -> Dec 8 < Dec 13 -> TRUE

// Dialog D: oldest=Dec 15 -> Dec 15 < Dec 13 -> FALSE

// Test 3: Only dialogActivityTo set -> condition: oldestDate <= toDate

// Dialog A: oldest=Dec 10 -> Dec 10 <= Dec 13 -> TRUE

// Dialog B: oldest=Dec 12 -> Dec 12 <= Dec 13 -> TRUE

// Dialog C: oldest=Dec 8 -> Dec 8 <= Dec 13 -> TRUE

// Dialog D: oldest=Dec 15 -> Dec 15 <= Dec 13 -> FALSE

Copilot · 2026-01-13T13:39:08Z

bot/storage-mongo/src/main/kotlin/UserTimelineMongoDAO.kt

+     * Condition: activityFrom <= youngest(actions.date) AND oldest(actions.date) < activityTo
+     *
+     * @param activityFrom optional start date filter (inclusive)
+     * @param activityTo optional end date filter (exclusive)


The documentation states that the activityTo parameter is "exclusive", but the implementation uses lte (less than or equal), which is inclusive. This creates a discrepancy between the documented and actual behavior. The MongoAgg.filterByPeriodOverlap function also documents toDate as "inclusive" (line 340 of MongoAgg.kt), which is consistent with the implementation but contradicts this comment. Either update the documentation here to say "inclusive" or change the implementation to use lt for exclusive behavior. Based on the test cases in UserTimelineMongoDAOTest.kt (lines 679-694), the actual behavior is inclusive, so the documentation should be corrected.

Suggested change

* Condition: activityFrom <= youngest(actions.date) AND oldest(actions.date) < activityTo

*

* @param activityFrom optional start date filter (inclusive)

* @param activityTo optional end date filter (exclusive)

* Condition: activityFrom <= youngest(actions.date) AND oldest(actions.date) <= activityTo

*

* @param activityFrom optional start date filter (inclusive)

* @param activityTo optional end date filter (inclusive)

bot/storage-mongo/src/main/kotlin/UserTimelineMongoDAO.kt

bot/engine/src/main/kotlin/admin/dialog/DialogReportQuery.kt

- Fix dialog creation date filter to use oldest action date (min) - Fix dialog activity filter to check at least one action in period - Make all date filters inclusive on both bounds (gte/lte) - Simplify dialog activity filter implementation - Add comprehensive unit tests for MongoAgg DSL - Remove dead code related to old from/to date filters - Fix ktlint formatting issues

assouktim requested changes Dec 30, 2025

View reviewed changes

vsct-jburet self-requested a review January 5, 2026 13:43

vsct-jburet requested a review from Copilot January 13, 2026 09:44

Copilot AI reviewed Jan 13, 2026

View reviewed changes

vsct-jburet requested a review from Copilot January 13, 2026 13:33

Copilot started reviewing on behalf of vsct-jburet January 13, 2026 13:33 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

antoine-billiet-arkea-partnre force-pushed the fix/DERCBOT-1676_fix_search_inconsistency branch from da148eb to afcdb5e Compare January 14, 2026 15:07

assouktim changed the title ~~Fix/dercbot 1676 fix search inconsistency~~ [DERCBOT-1676] Fix dialog date filters and add comprehensive tests Jan 16, 2026

Approach	Result	Correct?
Simple (`Stories.actions.date gte 2025-01-10`)	Matches (Jan 15th action satisfies)	Wrong
Aggregation (`min(actions.date) >= 2025-01-10`)	No match (Jan 1st < Jan 10th)	Correct

Statement	Equivalent to
`∃ action >= activityFrom`	`max(actions.date) >= activityFrom`
`∃ action < activityTo`	`min(actions.date) < activityTo`

Filter	Requirement	Simple approach works?	`Agg` required?
dialogCreationDate	`min(actions.date)` within period	No (fails for `from`)	Yes
dialogActivity	∃ action >= from AND ∃ action < to	Yes	No (but used for consistency)

Filter	`from` operator	`to` operator	Interval
annotationCreationDate	`gt` (exclusive)	`lt` (exclusive)	`(from, to)`
dialogCreationDate	`gte` (inclusive)	`lte` (inclusive)	`[from, to]`
dialogActivity	`gte` (inclusive)	`lt` (exclusive)	`[from, to)`

	"DialogA, DialogB, and DialogC should be included when only dialogActivityTo is set (oldest < Dec 13). Found: ${resultToOnly.total}",
	"DialogA, DialogB, and DialogC should be included when only dialogActivityTo is set (oldest <= Dec 13). Found: ${resultToOnly.total}",

[DERCBOT-1676] Fix dialog date filters and add comprehensive tests #1969

Are you sure you want to change the base?

[DERCBOT-1676] Fix dialog date filters and add comprehensive tests #1969

Uh oh!

Conversation

antoine-billiet-arkea-partnre commented Dec 22, 2025 • edited by Benvii Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

assouktim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antoine-billiet-arkea-partnre Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

1. Filter by Dialog Creation Date (dialogCreationDateFrom / dialogCreationDateTo)

2. Filter by Dialog Activity (dialogActivityFrom / dialogActivityTo)

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Current state of date filters

The problem

Proposed harmonization

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

assouktim commented Dec 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

antoine-billiet-arkea-partnre commented Dec 22, 2025 •

edited by Benvii

Loading

antoine-billiet-arkea-partnre Jan 5, 2026 •

edited

Loading

1. Filter by Dialog Creation Date (`dialogCreationDateFrom` / `dialogCreationDateTo`)

2. Filter by Dialog Activity (`dialogActivityFrom` / `dialogActivityTo`)