Optimize chat timeline, streaming, Hermes, and native navigation#578
Conversation
📝 WalkthroughWalkthroughThis PR adds avatar template metadata across Flutter and native platforms, refactors profile navigation and settings presentation, reworks chat list anchoring and streaming rendering, updates Hermes session/history handling, and restores persisted tool activity into assistant status history. ChangesNative profile sheets and settings
Chat layout and streaming
Hermes transport and persisted messages
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/chat/views/chat_bottom_anchor_controller.dart`:
- Around line 13-20: Update updateAnchor so a false hasScrollableContent does
not automatically set isAnchoredToBottom back to true after detachByUser() has
explicitly detached the anchor. Preserve the existing distanceFromBottom
threshold behavior for scrollable content, while keeping short conversations
detached until an explicit reattach action occurs.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6995e78d-79d1-495d-a226-16ac9ed87b67
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
android/app/src/main/kotlin/app/cogwheel/conduit/ConduitPlatformApis.g.ktios/Runner/ConduitPlatformApis.g.swiftios/Runner/NativeSheetBridge.swiftios/RunnerTests/RunnerTests.swiftlib/core/platform/conduit_platform_apis.g.dartlib/core/services/native_sheet_bridge.dartlib/core/services/navigation_service.dartlib/features/auth/widgets/adaptive_auth_scaffold.dartlib/features/chat/views/chat_bottom_anchor_controller.dartlib/features/chat/views/chat_page.dartlib/features/chat/views/chat_timeline_render_model.dartlib/features/navigation/widgets/sidebar_user_pill.dartlib/features/profile/models/settings_taxonomy.dartlib/features/profile/views/profile_page.dartlib/main.dartpigeons/conduit_platform_apis.dartpubspec.yamltest/core/router/direct_connections_router_policy_test.darttest/core/services/native_sheet_bridge_test.darttest/features/auth/views/adaptive_auth_flow_test.darttest/features/chat/views/chat_page_layout_metadata_test.darttest/features/chat/views/chat_timeline_render_model_test.darttest/features/profile/models/settings_taxonomy_test.darttest/features/profile/profile_page_hermes_test.darttest/features/workspace/views/workspace_shell_test.dart
💤 Files with no reviewable changes (2)
- test/features/profile/models/settings_taxonomy_test.dart
- lib/features/profile/models/settings_taxonomy.dart
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/chat/providers/chat_providers.dart (1)
1-1: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
responseStartsNewChainshould also respect the bound Hermes session
existingMessagesalone can mark a continued Responses turn as a new chain, which clearssessionIdand invalidateshermesSessionsProvidereven when the conversation already has a boundhermesSessionId.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/chat/providers/chat_providers.dart` at line 1, Update responseStartsNewChain to account for the conversation’s bound hermesSessionId in addition to existingMessages. A non-null bound Hermes session must prevent the turn from being classified as a new chain, preserving sessionId and hermesSessionsProvider state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/chat/providers/chat_providers.dart`:
- Around line 11277-11295: Update responseStartsNewChain in the Hermes response
flow to consider the bound hermesSessionId before treating a missing
responsePreviousResponseId as a new chain. Preserve continuity for resumed chats
with an existing session, and avoid clearing sessionId or invalidating
hermesSessionsProvider in that case.
---
Outside diff comments:
In `@lib/features/chat/providers/chat_providers.dart`:
- Line 1: Update responseStartsNewChain to account for the conversation’s bound
hermesSessionId in addition to existingMessages. A non-null bound Hermes session
must prevent the turn from being classified as a new chain, preserving sessionId
and hermesSessionsProvider state.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81f0895e-fdae-4633-9870-1c4777c1c251
📒 Files selected for processing (11)
lib/features/chat/providers/chat_providers.dartlib/features/hermes/services/hermes_api_service.dartlib/features/hermes/services/hermes_message_mapper.dartlib/features/hermes/services/hermes_run_transport.darttest/features/chat/providers/chat_messages_notifier_streaming_seams_test.darttest/features/chat/providers/hermes_run_remap_test.darttest/features/chat/providers/hermes_session_provenance_test.darttest/features/chat/widgets/hermes_approval_ownership_test.darttest/features/hermes/hermes_api_service_test.darttest/features/hermes/hermes_run_transport_test.darttest/features/hermes/hermes_sessions_test.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/core/services/structured_output_renderer.dart`:
- Around line 206-224: Bound recursive traversal in _valueLogicalLength and the
nearby _deepEquals helper using the same depth or node-cap approach as
_HermesProjectionSizeEstimator. Ensure nested List/Map values stop or return a
safe result once the bound is reached, while preserving existing comparisons and
length calculations for payloads within the limit.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a295443-c7ba-4b5d-9594-2ea1f9d9988e
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
lib/core/services/streaming_helper.dartlib/core/services/structured_output_renderer.dartlib/features/chat/providers/chat_providers.dartlib/features/chat/widgets/assistant_message_widget.dartlib/shared/widgets/markdown/streaming_markdown_widget.darttest/core/services/openwebui_stream_parser_test.darttest/core/services/streaming_helper_transport_test.darttest/features/chat/providers/chat_messages_notifier_dedupe_test.darttest/shared/widgets/markdown/renderer/markdown_performance_test.dart
cogwheel0
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/core/services/structured_output_renderer.dart`:
- Around line 231-237: Enforce node-budget accounting before shortcut returns:
update _measure to increment/check _nodes before the null return, and update the
comparison logic around identical(left, right) || left == right similarly before
either shortcut can return. Add broad flat null and equal-scalar regression
coverage in test/core/services/openwebui_stream_parser_test.dart lines 947-1009,
verifying traversal fails closed at the configured node limit.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8e9c6c3b-df31-4b51-b10f-0f4f8213d3fe
📒 Files selected for processing (2)
lib/core/services/structured_output_renderer.darttest/core/services/openwebui_stream_parser_test.dart
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches✨ Simplify code
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/core/services/openwebui_stream_parser_test.dart`:
- Around line 1056-1059: Refactor the assertions for the
StructuredOutputStreamingReplace revision to use the checks package’s `.has()`
property assertion, replacing the separate type check and manual cast while
preserving the expected content comparison with after.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f091421-1fce-44f8-825b-9a2ac7450a11
📒 Files selected for processing (4)
ios/Runner/NativeSheetBridge.swiftios/RunnerTests/RunnerTests.swiftlib/core/services/structured_output_renderer.darttest/core/services/openwebui_stream_parser_test.dart
Buffer owner projection deltas so Hermes no longer copies the accumulated response for every chunk. Re-measure bottom targets after animations and cancel stale settle generations so response or composer growth cannot leave content hidden after navigation or user interaction.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/features/chat/providers/chat_providers.dart (1)
372-391: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winClear
contentBufferon finalization to avoid retaining a duplicate of the content.
finalizematerializes the buffer intomessagebut leavescontentBufferholding the same characters. For finalized projections retained in the recovery cache (bounded bymaxRetainedProjections/maxRetainedBytes), each entry then keepsmessage.contentand aStringBuffercopy of it. Since_estimateHermesProjectionBytesonly measuresmessage.content, the retention budget undercounts, and the cache can hold roughly 2× the content memory it accounts for.The buffer is never read after finalization (
appendContentno-ops oncefinalized, and_materializeContentreturns early when!contentBufferDirty), so clearing it here is safe and also removes the stale-buffer edge case where_compactResolvingApprovalsetscontent: ''while the buffer still holds prior content.♻️ Proposed fix
final current = _materializeContent(projection); projection ..message = current.copyWith(isStreaming: false) ..finalized = true ..persistenceRevision += 1 ..retainedBytes = _estimateHermesProjectionBytes(projection.message); + projection.contentBuffer.clear();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/chat/providers/chat_providers.dart` around lines 372 - 391, Update finalize in _HermesRunProjection to clear contentBuffer immediately after materializing the final message, while preserving the existing finalized state and retention accounting. Ensure finalized projections no longer retain a duplicate buffer, and keep appendContent and subsequent materialization behavior unchanged.test/features/chat/providers/chat_messages_notifier_streaming_seams_test.dart (1)
2402-2407: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAssert the exact trusted document key, not only its count.
This passes if an unrelated key is persisted, leaving the document-trust guarantee untested.
Proposed assertion
- check( - HermesLocalDocumentTrustStore.trustedDocumentKeys( - connectionIdentity: connectionIdentity, - sessionId: 'responses-session', - ), - ).length.equals(1); + check( + HermesLocalDocumentTrustStore.trustedDocumentKeys( + connectionIdentity: connectionIdentity, + sessionId: 'responses-session', + ), + ).deepEquals(<String>{ + HermesLocalDocumentTrustStore.documentTrustKey( + messageId: 'server-user-1', + promptText: prompt, + documentEnvelope: envelope, + startOffset: prompt.length - envelope.length, + ), + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/chat/providers/chat_messages_notifier_streaming_seams_test.dart` around lines 2402 - 2407, Update the assertion around HermesLocalDocumentTrustStore.trustedDocumentKeys to verify the returned collection contains the expected trusted document key for connectionIdentity and sessionId 'responses-session', rather than asserting only that its length is one; preserve the existing lookup and ensure unrelated persisted keys cannot satisfy the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/features/chat/providers/chat_providers.dart`:
- Around line 372-391: Update finalize in _HermesRunProjection to clear
contentBuffer immediately after materializing the final message, while
preserving the existing finalized state and retention accounting. Ensure
finalized projections no longer retain a duplicate buffer, and keep
appendContent and subsequent materialization behavior unchanged.
In
`@test/features/chat/providers/chat_messages_notifier_streaming_seams_test.dart`:
- Around line 2402-2407: Update the assertion around
HermesLocalDocumentTrustStore.trustedDocumentKeys to verify the returned
collection contains the expected trusted document key for connectionIdentity and
sessionId 'responses-session', rather than asserting only that its length is
one; preserve the existing lookup and ensure unrelated persisted keys cannot
satisfy the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4b7054a-515f-4337-a4bb-a4c58cf7e98d
📒 Files selected for processing (5)
lib/features/chat/providers/chat_providers.dartlib/features/chat/views/chat_bottom_anchor_controller.dartlib/features/chat/views/chat_page.darttest/features/chat/providers/chat_messages_notifier_streaming_seams_test.darttest/features/chat/views/chat_page_layout_metadata_test.dart
Release finalized Hermes projection buffers so recovery accounting matches retained memory, and strengthen the affected regression assertions.
1 participant
Summary
Verification
Note
Optimize chat timeline layout, streaming buffering, Hermes history, and native avatar rendering
ChatBottomScrollSettlerand a synthetic composer spacer inside theSuperSliverList, reducing scroll jitter during live streamingStringBufferper projection and materializes to an immutable string only when needed, reducing allocations and redundant content rewrites during streamingStructuredOutputStreamingProjectorthat appends plain-text suffixes when safe and forces full replacements only on structural/status transitions, bounding render work during structured output streamingpreviousRunIdto sending an explicitconversationHistorypayload; Responses API new chains omit the pre-created session and include conversation history insteadstatusHistoryentries on their associated assistant messages rather than emitting them as standalone chat messagesscheduleFrame+addPostFrameCallbackwith a singlescheduleFrameCallbackin assistant message and streaming markdown widgets, applying updates within the scheduled frame and removing one extra frame per updateavatarIsTemplatesupport to the native profile sheet: Flutter passes the flag through the Pigeon API; iOS renders template avatars with.labeltint and aspect-fit content modepreviousRunId; regeneration branches fromcontinuityMessagesonly, which changes session continuity behavior for existing run-based flowsMacroscope summarized e024dbe.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Greptile Summary
This PR updates chat scrolling, streaming output, Hermes sessions, and native profile navigation. The main changes are:
SuperSliverListwith layout-time bottom anchoring.Confidence Score: 5/5
This PR appears safe to merge.
No blocking correctness or security issues were identified in the reviewed changed paths. The higher-risk chat timeline, streaming, Hermes, and native avatar changes include bounded guards and targeted tests.
No files require special attention.
What T-Rex did
Important Files Changed
SuperSliverListwith layout-time bottom anchoring and extent reconciliation.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant ChatPage participant StreamingHelper participant Projector as StructuredOutputStreamingProjector participant Hermes as HermesApiService participant UI as Managed SuperSliverList User->>ChatPage: Send message / open Hermes turn ChatPage->>Hermes: create run or stream response with conversationHistory Hermes-->>StreamingHelper: token, reasoning, tool, output events StreamingHelper->>Projector: project cumulative structured output alt plain tail append Projector-->>StreamingHelper: append escaped fragment StreamingHelper->>ChatPage: append assistant content/status else structural transition or bounded replacement Projector-->>StreamingHelper: replace authoritative snapshot StreamingHelper->>ChatPage: replace assistant content/status end ChatPage->>UI: update keyed timeline rows and extents UI-->>ChatPage: layout metrics and bottom distance ChatPage->>UI: StickTarget.bottom while anchored Hermes-->>StreamingHelper: terminal output/session id StreamingHelper->>Projector: finish final authoritative render StreamingHelper->>ChatPage: complete streaming UI%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant ChatPage participant StreamingHelper participant Projector as StructuredOutputStreamingProjector participant Hermes as HermesApiService participant UI as Managed SuperSliverList User->>ChatPage: Send message / open Hermes turn ChatPage->>Hermes: create run or stream response with conversationHistory Hermes-->>StreamingHelper: token, reasoning, tool, output events StreamingHelper->>Projector: project cumulative structured output alt plain tail append Projector-->>StreamingHelper: append escaped fragment StreamingHelper->>ChatPage: append assistant content/status else structural transition or bounded replacement Projector-->>StreamingHelper: replace authoritative snapshot StreamingHelper->>ChatPage: replace assistant content/status end ChatPage->>UI: update keyed timeline rows and extents UI-->>ChatPage: layout metrics and bottom distance ChatPage->>UI: StickTarget.bottom while anchored Hermes-->>StreamingHelper: terminal output/session id StreamingHelper->>Projector: finish final authoritative render StreamingHelper->>ChatPage: complete streaming UIComments Outside Diff (1)
ios/Runner/NativeSheetBridge.swift, line 5397-5404 (link)Header-scoped avatars collide
NativeSheetImageLoadercaches images byrawUrlonly (cacheKey = rawUrl), but this new profile-avatar path passes auth-bearingavatarHeaders. When two accounts or servers reuse the same avatar URL with different headers, the second native sheet receives the first cached image without making an authenticated request, exposing the wrong user's avatar.Artifacts
Repro: standalone cache/header collision harness
Repro: execution log showing second header-scoped load served from first cached value
Reviews (11): Last reviewed commit: "Address final review feedback" | Re-trigger Greptile