feat: update session draft handling and improve trace visibility features - #487
Merged
Merged
Conversation
…ures - Added `!session::set-draft` permission to `iii-permissions.yaml` for managing draft states. - Updated `Cargo.lock` to use version 0.21.3 for `iii-helpers` and `iii-sdk`. - Enhanced `timeline-span-tags.md` documentation to include `iii.tag.hidden` for internal span management. - Implemented draft persistence in the chat composer, allowing users to restore unsent messages after a page refresh. - Introduced `setSessionDraft` API for saving composer drafts without affecting session timestamps. - Updated `useConversations` hook to manage draft text and ensure it is restored correctly. - Added functionality to toggle visibility of internal spans in trace views, improving user experience in the traces interface.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
sergiofilhowz
changed the title
Contributor
skill-check — worker0 verified, 42 skipped (no docs/).
Four for four. Nicely done. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe changes add persisted conversation drafts, hidden trace-span metadata and filtering, hierarchical Traces V2 timelines with follow-live-turn behavior, and reactive in-memory configuration snapshots for llm-router. ChangesConversation draft persistence
Traces V2 and hidden tracing
Reactive llm-router configuration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ConfigEvent as configuration:updated
participant OnChanged as make_on_config_changed
participant Entry as read_entry_value
participant Snapshot as ConfigCell
participant Provider as provider refresh_models
ConfigEvent->>OnChanged: advisory change notification
OnChanged->>Entry: fetch authoritative configuration
Entry-->>OnChanged: configuration value
OnChanged->>Snapshot: apply_config
OnChanged->>Provider: debounced refresh for changed provider slices
sequenceDiagram
participant Composer
participant Conversations as useConversations
participant SessionAPI as setSessionDraft
participant SessionManager as session::set-draft
Composer->>Conversations: report text change
Conversations->>SessionAPI: debounce draft save
SessionAPI->>SessionManager: persist session draft
SessionManager-->>Conversations: stored draft metadata
Conversations-->>Composer: restore draft on mount
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
console/web/src/components/chat/Composer.tsx (1)
335-367: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep a queue-submit control while streaming.
When
queueWhileStreamingis true, the editor says “queue a message…” but the send button is removed and only Stop remains. Restore a queue/send button alongside Stop so mouse and touch users can submit queued messages.🤖 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 `@console/web/src/components/chat/Composer.tsx` around lines 335 - 367, Update the Composer button rendering around isStreaming and queueWhileStreaming so streaming mode retains the existing Stop control while also rendering an enabled send/queue button when queueWhileStreaming is true. Keep the current Stop-only behavior when queueWhileStreaming is false, and route the additional button through handleSubmit with the existing blocked state, accessibility label, and styling.llm-router/src/config/on_changed.rs (1)
88-110: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not abort a flush after it has drained pending work.
A new event can abort the previous task after line 96 has removed IDs from
pendingbut before lines 99-109 finish. The remaining IDs are then neither queued nor re-detected because line 74 already advanced their fingerprints. Only cancel the sleep phase, or requeue unprocessed IDs on cancellation.🤖 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 `@llm-router/src/config/on_changed.rs` around lines 88 - 110, Update the debounce task around the pending drain and trigger loop so cancellation cannot lose IDs after pending work has been removed. Limit abort handling to the sleep phase, or ensure every ID not yet processed when the task is cancelled is reinserted into pending; preserve the existing refresh triggering behavior for drained IDs.
🧹 Nitpick comments (2)
harness/src/functions/mod.rs (1)
160-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider delegating
register_internaltoregister_meta.Now that
register_metaprovides the unified registration path,register_internalduplicates the same closure-construction + description + metadata logic. It could be simplified to a one-liner delegation, eliminating ~15 lines of duplicated code.♻️ Optional refactor
fn register_internal<Req, Resp, F, Fut>( iii: &Arc<IIIClient>, deps: &Arc<Deps>, id: &str, description: &str, handler: F, ) where Req: DeserializeOwned + JsonSchema + Send + 'static, Resp: Serialize + JsonSchema + Send + 'static, F: Fn(Arc<Deps>, Req) -> Fut + Send + Sync + Clone + 'static, Fut: Future<Output = Result<Resp, HarnessError>> + Send + 'static, { - let deps = deps.clone(); - iii.register_function( - id, - RegisterFunction::new_async(move |req: Req| { - let deps = deps.clone(); - let handler = handler.clone(); - async move { handler(deps, req).await.map_err(Error::from) } - }) - .description(description) - .metadata(serde_json::json!({ "internal": true })), - ); + register_meta( + iii, + deps, + id, + description, + Some(serde_json::json!({ "internal": true })), + handler, + ); }🤖 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 `@harness/src/functions/mod.rs` around lines 160 - 183, Update register_internal to delegate registration to the existing register_meta function instead of constructing its own async closure, description, and internal metadata. Pass through the same id, description, dependencies, and handler while preserving the current generic bounds and behavior.console/web/src/pages/TracesV2/components/timeline/TimelineStrip.tsx (1)
155-306: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider extracting the shared subtree-packing/hierarchy layout.
packSubtrees+buildHierarchyLayouthere are a near-verbatim copy ofpackSubtrees+buildLayoutinTraceTimeline.tsx(the comments even call it "the same rule asTraceTimeline.tsx"). The two only differ in the span shape they key off (TimelineSpanvsVisualizationSpan+byId). Because the packing/collision math is subtle, keeping two copies invites silent divergence. A generic helper (parameterized byid/children/startTime/extentaccessors) would keep both views in lockstep.🤖 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 `@console/web/src/pages/TracesV2/components/timeline/TimelineStrip.tsx` around lines 155 - 306, The subtree packing and hierarchy layout logic is duplicated between packSubtrees/buildHierarchyLayout here and packSubtrees/buildLayout in TraceTimeline.tsx. Extract the shared traversal, collision, and line-placement behavior into a generic helper parameterized by the span identity, child relationships, start time, and occupancy extent accessors, then update both views to use it while preserving their existing span-specific shapes and layout results.
🤖 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 `@console/web/src/hooks/use-conversations.ts`:
- Around line 897-910: Update setDraftText so newly created conversations marked
by conv.draft also persist non-empty draft text durably instead of returning
after updating draftTextsRef. Reuse the existing draft persistence mechanism
where possible, while preserving the in-memory ref behavior and avoiding session
materialization unless required by the current storage design.
- Around line 880-894: Update flushDraft so setSessionDraft writes are
serialized per session, ensuring older saves cannot complete after newer saves;
only update lastSavedDraftRef after the corresponding RPC succeeds, and leave
failed values eligible for retry. Preserve the existing timer cleanup,
deduplication, and development warning behavior.
In `@console/web/src/pages/TracesV2/lib/followTurn.ts`:
- Around line 117-124: Update the early-return branch in the trace-following
logic to add every currently visible trace ID from traces to state.seenTraceIds
before returning when newest is null or already seen. Preserve the existing
return value, and add a regression test covering a delayed older trace arriving
after a newer trace has already been seen.
In `@llm-router/src/config/on_changed.rs`:
- Around line 62-68: Update the Err(error) branch of the authoritative
configuration fetch in the on-change handler to schedule a bounded retry with
backoff instead of returning a successful RouterAck immediately. Reuse the
existing retry/queue mechanism and preserve the previous snapshot while
retrying; only acknowledge according to the established retry semantics.
In `@llm-router/src/register.rs`:
- Around line 240-244: Update the bootstrap reconciliation block guarded by
entry_lock to use the same snapshot-and-fingerprint synchronization operation as
make_on_config_changed, rather than calling apply_config alone. Ensure the newer
value read by read_entry_value updates both the applied configuration and the
handler’s fingerprint baseline, so later changes are diffed against that
reconciled state and refresh_models is triggered when appropriate.
---
Outside diff comments:
In `@console/web/src/components/chat/Composer.tsx`:
- Around line 335-367: Update the Composer button rendering around isStreaming
and queueWhileStreaming so streaming mode retains the existing Stop control
while also rendering an enabled send/queue button when queueWhileStreaming is
true. Keep the current Stop-only behavior when queueWhileStreaming is false, and
route the additional button through handleSubmit with the existing blocked
state, accessibility label, and styling.
In `@llm-router/src/config/on_changed.rs`:
- Around line 88-110: Update the debounce task around the pending drain and
trigger loop so cancellation cannot lose IDs after pending work has been
removed. Limit abort handling to the sleep phase, or ensure every ID not yet
processed when the task is cancelled is reinserted into pending; preserve the
existing refresh triggering behavior for drained IDs.
---
Nitpick comments:
In `@console/web/src/pages/TracesV2/components/timeline/TimelineStrip.tsx`:
- Around line 155-306: The subtree packing and hierarchy layout logic is
duplicated between packSubtrees/buildHierarchyLayout here and
packSubtrees/buildLayout in TraceTimeline.tsx. Extract the shared traversal,
collision, and line-placement behavior into a generic helper parameterized by
the span identity, child relationships, start time, and occupancy extent
accessors, then update both views to use it while preserving their existing
span-specific shapes and layout results.
In `@harness/src/functions/mod.rs`:
- Around line 160-183: Update register_internal to delegate registration to the
existing register_meta function instead of constructing its own async closure,
description, and internal metadata. Pass through the same id, description,
dependencies, and handler while preserving the current generic bounds and
behavior.
🪄 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: CHILL
Plan: Pro
Run ID: 55ac5c97-1314-44b9-906c-c7ea9e2eb32b
⛔ Files ignored due to path filters (10)
approval-gate/Cargo.lockis excluded by!**/*.lockharness/Cargo.lockis excluded by!**/*.lockllm-router/Cargo.lockis excluded by!**/*.lockprovider-anthropic/Cargo.lockis excluded by!**/*.lockprovider-llamacpp/Cargo.lockis excluded by!**/*.lockprovider-openai-codex/Cargo.lockis excluded by!**/*.lockprovider-openai/Cargo.lockis excluded by!**/*.lockprovider-xai/Cargo.lockis excluded by!**/*.lockprovider-zai/Cargo.lockis excluded by!**/*.locksession-manager/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (103)
console/docs/timeline-span-tags.mdconsole/src/configuration.rsconsole/web/src/components/chat/ChatView.tsxconsole/web/src/components/chat/Composer.tsxconsole/web/src/hooks/use-conversations.test.tsconsole/web/src/hooks/use-conversations.tsconsole/web/src/lib/sessions/api.tsconsole/web/src/lib/sessions/types.tsconsole/web/src/lib/storage.tsconsole/web/src/lib/trace-hidden-functions.tsconsole/web/src/pages/TracesV2/components/WaterfallChart.tsxconsole/web/src/pages/TracesV2/components/timeline/SpanFilterMenu.tsxconsole/web/src/pages/TracesV2/components/timeline/Timeline.tsxconsole/web/src/pages/TracesV2/components/timeline/TimelineStrip.tsxconsole/web/src/pages/TracesV2/components/timeline/TraceTimeline.tsxconsole/web/src/pages/TracesV2/components/timeline/layout.test.tsconsole/web/src/pages/TracesV2/components/timeline/layout.tsconsole/web/src/pages/TracesV2/components/timeline/spanVisibility.test.tsconsole/web/src/pages/TracesV2/components/timeline/spanVisibility.tsconsole/web/src/pages/TracesV2/components/timeline/spanVisuals.tsxconsole/web/src/pages/TracesV2/hooks/useAllSpans.test.tsconsole/web/src/pages/TracesV2/hooks/useAllSpans.tsconsole/web/src/pages/TracesV2/hooks/useFollowLiveTurn.tsconsole/web/src/pages/TracesV2/hooks/useSpanFilterSelection.tsconsole/web/src/pages/TracesV2/hooks/useTraceViews.tsconsole/web/src/pages/TracesV2/index.tsxconsole/web/src/pages/TracesV2/lib/followTurn.test.tsconsole/web/src/pages/TracesV2/lib/followTurn.tsconsole/web/src/pages/TracesV2/lib/functionCallFromSpan.tsconsole/web/src/pages/TracesV2/lib/spanFilters.test.tsconsole/web/src/pages/TracesV2/lib/spanFilters.tsconsole/web/src/pages/TracesV2/lib/spanLabel.test.tsconsole/web/src/pages/TracesV2/lib/spanLabel.tsconsole/web/src/pages/TracesV2/lib/timelineSpans.test.tsconsole/web/src/pages/TracesV2/lib/timelineSpans.tsconsole/web/src/pages/TracesV2/lib/traceTimelineFilters.tsconsole/web/src/pages/TracesV2/lib/tracesViews.tsconsole/web/src/pages/TracesV2/stories/timeline/Timeline.stories.tsxconsole/web/src/pages/TracesV2/stories/timeline/TimelineStrip.stories.tsxconsole/web/src/pages/TracesV2/stories/timeline/TraceTimeline.stories.tsxconsole/web/src/pages/TracesV2/stories/timeline/liveFeed.tsconsole/web/src/types/chat.tscontext-manager/src/configuration.rscontext-manager/src/functions/mod.rsdocs/sops/README.mddocs/sops/trace-hidden-functions.mdharness/Cargo.tomlharness/src/clients/session.rsharness/src/functions/mod.rsharness/src/lib.rsharness/src/state.rsharness/src/trace_tags.rsiii-permissions.yamlllm-router/Cargo.tomlllm-router/README.mdllm-router/src/chat/chat.rsllm-router/src/config/entry.rsllm-router/src/config/mod.rsllm-router/src/config/on_changed.rsllm-router/src/config/state.rsllm-router/src/register.rsllm-router/src/registry/availability.rsllm-router/src/registry/resolve.rsllm-router/src/routing.rsllm-router/src/surface.rsllm-router/src/system_prompt.rsllm-router/src/types/router.rsllm-router/tests/golden/schemas/router.on_config_changed.jsonllm-router/tests/integration.rsprovider-anthropic/Cargo.tomlprovider-llamacpp/Cargo.tomlprovider-openai-codex/Cargo.tomlprovider-openai/Cargo.tomlprovider-xai/Cargo.tomlprovider-zai/Cargo.tomlsession-manager/Cargo.tomlsession-manager/architecture/README.mdsession-manager/architecture/integration.mdsession-manager/architecture/internals.mdsession-manager/src/configuration.rssession-manager/src/events.rssession-manager/src/functions/mod.rssession-manager/src/functions/set_draft.rssession-manager/src/functions/store_protocol.rssession-manager/src/resync.rssession-manager/src/service.rssession-manager/src/store/fs.rssession-manager/src/surface.rssession-manager/src/types.rssession-manager/tests/common/world.rssession-manager/tests/features/draft.featuresession-manager/tests/golden/schemas/session.create.jsonsession-manager/tests/golden/schemas/session.ensure.jsonsession-manager/tests/golden/schemas/session.fork.jsonsession-manager/tests/golden/schemas/session.get.jsonsession-manager/tests/golden/schemas/session.list.jsonsession-manager/tests/golden/schemas/session.set-draft.jsonsession-manager/tests/golden/schemas/session.set-meta.jsonsession-manager/tests/golden/schemas/session.store.get-meta.jsonsession-manager/tests/golden/schemas/session.store.list-metas.jsonsession-manager/tests/golden/schemas/session.store.put-meta.jsonsession-manager/tests/schemas.rstech-specs/2026-06-agentic/llm-router.md
💤 Files with no reviewable changes (3)
- console/web/src/pages/TracesV2/components/timeline/layout.test.ts
- console/web/src/pages/TracesV2/stories/timeline/Timeline.stories.tsx
- console/web/src/pages/TracesV2/components/timeline/Timeline.tsx
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test nacked with a 30ms backoff and immediately asserted the job was not ready; a >30ms stall between nack's clock capture and the dequeue (easy on a loaded CI runner, the FileStore backend persists in between) made the job ready early. 500ms backoff / 600ms sleep keeps the same coverage with real headroom. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 15, 2026
Merged
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.
!session::set-draftpermission toiii-permissions.yamlfor managing draft states.Cargo.lockto use version 0.21.3 foriii-helpersandiii-sdk.timeline-span-tags.mddocumentation to includeiii.tag.hiddenfor internal span management.setSessionDraftAPI for saving composer drafts without affecting session timestamps.useConversationshook to manage draft text and ensure it is restored correctly.Need to update cargo to point to the latest version of iii when we release it
Summary by CodeRabbit
iii.tag.display_namehandling to suppress unintended “echoes.”