FirestoreSessionService compares event timestamps as text, reordering or skipping same-second events
Maintainer thường phản hồi trong vòng 1 ngày
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 3/5
- Thời gian dự kiến
- 1-2 ngày
- Mức phù hợp với người mới
- 50/100
Hướng nghiên cứu
Start in contrib/firestore-session-service/src/main/java/com/google/adk/sessions/FirestoreSessionService.java: the timestamp write near line 411, the orderBy calls near lines 234 and 611, and the afterTimestamp filter near lines 236-240. Run the two reproduction tests in FirestoreSessionServiceTest, starting with getSession_withAfterTimestamp_appliesFilterToQuery. Done means the stored strings sort in append order and the cursor filter is inclusive. Point 2 (tie-breaking) needs a maintainer decision, so leave it out of the first change.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
🔴 Required Information
Describe the Bug:
FirestoreSessionService stores each event's time as text, Instant.ofEpochMilli(event.timestamp()).toString() (FirestoreSessionService.java:411), and then sorts and filters events on that text. getSession and listEvents order by it (:234, :611), numRecentEvents takes the last N of that order (:242-244), and afterTimestamp becomes whereGreaterThan("timestamp", afterTimestamp.toString()) (:236-240). Firestore orders strings by their UTF-8 bytes (data types). That causes three problems:
- Events in the same second can come back out of order (variable-width text).
Instant.toString()writes no fraction when the milliseconds are zero, so an event at 05:00:05.000 is stored as2026-10-10T05:00:05Zand a later one at 05:00:05.400 as2026-10-10T05:00:05.400Z. Because.(0x2E) sorts beforeZ(0x5A), the later event comes first. - Events in the same millisecond come back in arbitrary order (no tie-breaker). In Standard edition, Firestore orders equal values by document name (
StructuredQuery.orderBy), and Enterprise edition does not guarantee a stable order. Event documents get random auto-IDs (document()with no argument,:709-715), so tied events come back in the order of those random IDs, not the order they were appended. Onmain(not in 1.11.0), a fixedInstantSourcepassed toRunner.Builder.instantSource(Runner.java:195-205), as in tests, gives every event the runner creates the same timestamp. afterTimestampreturns the wrong events (text comparison, and>instead of>=). With a whole-second cursor such as2026-10-10T05:00:05Z, every later event in that second (…05.001Zto…05.999Z) sorts below the cursor and is left out. With a millisecond cursor, an earlier whole-second event in the same second is included and an event at exactly the cursor is left out. A cursor with microseconds (…05.200300Z) includes an earlier…05.200Zevent.InMemorySessionServicekeeps the events at or after the cursor (InMemorySessionService.java:223-229), andVertexAiSessionServicesends an inclusivetimestamp>=filter (VertexAiSessionService.java:263-272).
Runner.runAsync reads the session on every run (Runner.java:556-560), and Contents builds the history in list order (Contents.java:170-177), so points 1 and 2 reach the next model request. Once #1643 stores call IDs, function responses are moved back after their calls by ID (Contents.java:641-751); on main today, reloaded responses have no IDs and are dropped (#1640). Either way, other events, such as user and model text, keep the wrong order. The Runner passes Optional.empty() as the session config, so point 3 and numRecentEvents only affect code that calls getSession with a GetSessionConfig.
The stored times themselves are correct: eventFromMap reads both forms back with Instant.parse (:301).
Steps to Reproduce:
- Use
google-adk-firestore-session-service1.11.0 ormainat189d463a. - Append three events whose
timestampis 2026-10-10T05:00:05.000Z, 05:00:05.400Z and 05:00:06.000Z (in epoch milliseconds), and capture thetimestampvaluesappendEventwrites. A mockedFirestoreas inFirestoreSessionServiceTestis enough (first test below). - Sort those values the way Firestore sorts strings. They are ASCII, so
Stringorder is the same as UTF-8 byte order. - For
afterTimestamp, compare the stored values with the cursor thatgetSessionpasses,afterTimestamp.toString()(second test below). The existing testgetSession_withAfterTimestamp_appliesFilterToQuerychecks that call (FirestoreSessionServiceTest.java:273-295).
I did not run this against a real Firestore database or the emulator. The order and the result sets below come from the strings the code writes and Firestore's documented ordering rules.
Expected Behavior:
getSession and listEvents return events in the order they were appended, and afterTimestamp returns the events at or after the cursor, as it does in InMemorySessionService and VertexAiSessionService. ADK Python's Firestore session service stores timestamp as a Firestore timestamp (firestore_session_service.py:609-611), orders by it (:320) and filters with >= (:329).
Observed Behavior:
The first test below prints:
stored: [2026-10-10T05:00:05Z, 2026-10-10T05:00:05.400Z, 2026-10-10T05:00:06Z]
ordered: [2026-10-10T05:00:05.400Z, 2026-10-10T05:00:05Z, 2026-10-10T05:00:06Z]
The second prints:
2026-10-10T05:00:05.001Z > 2026-10-10T05:00:05Z: false
2026-10-10T05:00:05.400Z > 2026-10-10T05:00:05Z: false
2026-10-10T05:00:05.999Z > 2026-10-10T05:00:05Z: false
2026-10-10T05:00:06Z > 2026-10-10T05:00:05Z: true
So with afterTimestamp at 05:00:05.000, Firestore would return only the events from 05:00:06 on, while InMemorySessionService returns all four for the same cursor (checked in a separate test).
Environment Details:
- ADK Library Version (see maven dependency):
google-adk-firestore-session-service1.11.0 andmainat189d463a - OS: Windows 11 (not OS-specific)
- TS Version (tsc --version): N/A (Java: Microsoft OpenJDK 17.0.19)
Model Information:
- Which model is being used: N/A (session service bug, model independent)
🟡 Optional Information
Regression:
No. The text timestamp, the orderBy, the whereGreaterThan filter and the auto-ID event documents have been there since the service was added in 0.4.0 (b75608ff).
Additional Context:
- Proposed fix for points 1 and 3, keeping the field a string: write the timestamp with a fixed three-digit fraction (
new DateTimeFormatterBuilder().appendInstant(3).toFormatter()gives2026-10-10T05:00:05.000Z, whichInstant.parsestill reads). Format theafterTimestampcursor the same way, rounding it up to the millisecond first becauseappendInstant(3)truncates extra digits, and usewhereGreaterThanOrEqualTo.FirestoreMemoryServicereads the field as a string (FirestoreMemoryService.java:154) and keeps working. Events stored before the fix keep their old strings, so an old pair like…05Zand…05.400Zstays misordered, and a later cursor in the same second still includes the old…05Zevent, unless the old documents are rewritten. - Storing a Firestore timestamp, as ADK Python does, would also fix new data, but sessions with older events would mix types: Firestore orders timestamps before strings, so in such a session every new event would sort before every old one, and
afterTimestampwould not compare across the two types.eventFromMapandFirestoreMemoryServiceread the field as a string and would need to change too. - Point 2 needs a tie-breaker either way, for example a per-session sequence number or document IDs that sort in append order, named explicitly in
orderBybecause Enterprise edition does not guarantee a stable order otherwise. A new order field would need old documents to be backfilled (orderByskips documents without the field) and a composite index if it followstimestamp. Re-sorting on the client byEvent.timestamp(), asVertexAiSessionServicedoes (VertexAiSessionService.java:281-284), fixes the list order for point 1 but not ties, andlimitToLastandafterTimestampwould still select documents by the stored text. ADK Python has the same gap: ties fall back to the document ID, a randomevent.id, but its microsecond timestamps make ties much rarer. - #1643 changes other parts of
FirestoreSessionService.java(constants,eventFromMap, andeventToMapfrom line 443) but none of the lines above. I'm happy to send a PR for points 1 and 3 if this direction looks right, and to follow your choice for point 2.
Minimal Reproduction Code:
Both tests go into FirestoreSessionServiceTest and use its mocks, constants and imports:
@Test
@SuppressWarnings("unchecked")
void eventTimestampsSortAsText() {
Session session =
Session.builder(SESSION_ID)
.appName(APP_NAME)
.userId(USER_ID)
.state(new ConcurrentHashMap<>())
.build();
when(mockSessionsCollection.document(SESSION_ID)).thenReturn(mockSessionDocRef);
when(mockEventsCollection.document()).thenReturn(mockEventDocRef);
when(mockEventDocRef.getId()).thenReturn(EVENT_ID);
when(mockEventsCollection.document(EVENT_ID)).thenReturn(mockEventDocRef);
long t = Instant.parse("2026-10-10T05:00:05Z").toEpochMilli();
for (long timestamp : new long[] {t, t + 400, t + 1_000}) {
Event event =
Event.builder()
.author("model")
.timestamp(timestamp)
.content(Content.fromParts(Part.fromText("hi")))
.build();
sessionService.appendEvent(session, event).blockingGet();
}
ArgumentCaptor<Map<String, Object>> saved = ArgumentCaptor.forClass(Map.class);
verify(mockEventDocRef, times(3)).set(saved.capture());
List<String> stored =
saved.getAllValues().stream()
.map(data -> (String) data.get(Constants.KEY_TIMESTAMP))
.toList();
System.out.println("stored: " + stored);
// Firestore orders strings by UTF-8 bytes; for these ASCII strings that is String order.
System.out.println("ordered: " + stored.stream().sorted().toList());
}
@Test
void afterTimestampCursorComparesAsText() {
// getSession passes afterTimestamp.toString() to whereGreaterThan.
String cursor = Instant.parse("2026-10-10T05:00:05Z").toString();
for (String stored :
List.of(
"2026-10-10T05:00:05.001Z",
"2026-10-10T05:00:05.400Z",
"2026-10-10T05:00:05.999Z",
"2026-10-10T05:00:06Z")) {
System.out.println(stored + " > " + cursor + ": " + (stored.compareTo(cursor) > 0));
}
}
How often has this issue occurred?:
- Intermittently (<50%). Point 1 needs an event whose timestamp is a whole second (about 1 in 1000 events, if milliseconds are spread evenly) followed by another event in the same second. Point 2 needs two events in the same millisecond. Point 3 needs a caller that passes
afterTimestamp: with a whole-second cursor, every later event in that second is affected; with any other cursor, only an event in the cursor's millisecond or one at the start of that second. I have not measured how often this happens in a deployment.
- Ngôn ngữ chính
- Java
- Star
- 1.7k
- Fork
- 433
- Merge trung bình
- 3 ngày 13 giờ
- Pull request đã merge (30 ngày)
- 42
Chuẩn bị môi trường
Khởi chạy dev container của dự án ngay trên trình duyệt, bằng tài khoản GitHub của bạn.
- Không có Dockerfile hay tệp Docker Compose
- Có mẫu pull request
- Đọc hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của google/adk-java
-
GeminiUtil placeholder user turn ("Continue output. DO NOT look at this line ...") is flagged by prompt injection filtersCó thể đã có người làm @hemasekhar-p đã nhận 3 ngày trước. Đang mởneeds review
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 76/100
google/adk-java#1628 · 1 bình luận · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
[spring-ai] ToolConverter silently drops enum and items from tool parameter schemasCó thể đã có người làm @hirematha đã nhận 6 ngày trước. Đang mởneeds review
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 76/100
google/adk-java#1609 · 2 bình luận · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
[spring-ai] Streaming responses ending with CJK punctuation (。!?) are misclassified as partial and never persisted to the sessionCó thể đã có người làm @hirematha đã nhận 6 ngày trước. Đang mởneeds review
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
google/adk-java#1608 · 3 bình luận · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 45/100
Maintainer thường phản hồi trong vòng 1 ngày
-
FirestoreSessionService loses event fields on reload, breaking later turns and tool confirmationsCó thể đã có người làm @innoprej đã nhận 1 ngày trước. Đang mở
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 22/100
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của google/adk-java
Issue tương tự
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
objectionary/eo-graphs#85 ·
-
BoxAttachmentMulti parsing leaks IOException / ArrayIndexOutOfBoundsException on malformed content instead of IllegalArgumentExceptionCó thể đã có người làm @Kshot3000 đã nhận hôm nay. Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 74/100
ergoplatform/ergo-appkit#272 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 64/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 66/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 64/100
utopia-rise/godot-jvm#1004 ·
Maintainer thường phản hồi trong vòng 1 ngày