Pr/UI fresh #15
Pr/UI fresh #15
Conversation
WalkthroughThis pull request introduces token tracking and timing metadata throughout the logging system, alongside a new collapsible process group UI for the frontend. Changes include: adding Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
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 help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
agent.pypython/extensions/before_main_llm_call/_10_log_for_stream.pypython/extensions/response_stream/_10_log_from_stream.pypython/extensions/response_stream/_20_live_response.pypython/helpers/log.pywebui/components/messages/process-group/process-group-store.jswebui/components/messages/process-group/process-group.csswebui/components/sidebar/bottom/preferences/preferences-store.jswebui/css/messages.csswebui/index.csswebui/index.htmlwebui/index.jswebui/js/messages.js
🧰 Additional context used
🧬 Code graph analysis (4)
webui/js/messages.js (2)
webui/components/sidebar/bottom/preferences/preferences-store.js (1)
processGroupStore(170-170)webui/components/messages/process-group/process-group-store.js (2)
key(70-70)key(76-76)
agent.py (2)
python/helpers/log.py (1)
add_tokens(170-175)python/helpers/tokens.py (1)
approximate_tokens(22-25)
webui/index.js (1)
webui/js/messages.js (2)
setMessage(32-160)content(1149-1149)
python/extensions/response_stream/_10_log_from_stream.py (1)
python/extensions/before_main_llm_call/_10_log_for_stream.py (2)
build_heading(22-23)build_default_heading(25-26)
🪛 Ruff (0.14.10)
python/extensions/response_stream/_20_live_response.py
33-33: f-string without any placeholders
Remove extraneous f prefix
(F541)
python/extensions/before_main_llm_call/_10_log_for_stream.py
22-22: Missing return type annotation for private function build_heading
Add return type annotation: str
(ANN202)
22-22: Unused function argument: agent
(ARG001)
agent.py
434-434: Missing return type annotation for private function tokens_callback
Add return type annotation: None
(ANN202)
434-434: Unused function argument: text
(ARG001)
python/helpers/log.py
140-140: Missing return type annotation for special method __post_init__
(ANN204)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (27)
python/extensions/response_stream/_10_log_from_stream.py (2)
25-25: LGTM: Simplified tool heading.The heading now uses "Using {tool_name}" instead of "Using tool {tool_name}", which is more concise. This aligns with the broader heading simplification across the PR.
27-27: LGTM: Simplified thoughts heading.The thoughts branch now uses the default heading instead of constructing a custom "Thinking..." heading. This simplification is consistent with the PR's unified heading approach.
webui/index.css (1)
199-212: LGTM: Enhanced time/date typography.The typography improvements make the time/date display more prominent and readable:
- Increased font size and weight for #time-date
- Added monospace font for consistency with process-group metadata
- Reduced and styled #user-date for subtler presentation
These changes align with the broader UI enhancements for displaying timing metadata in the process-group UI.
webui/css/messages.css (2)
137-239: LGTM: Well-structured terminal-style code execution block.The new terminal-style styling for
.message-code-exeis comprehensive and includes:
- Dark terminal background with gradient header
- Monospace font with appropriate sizing
- Light mode variants for accessibility
- Command prompt indicator ($) with semantic color
This aligns well with the PR's goal of creating a more terminal-like UI experience.
116-165: LGTM: Message styling simplification.The changes remove backgrounds, shadows, and rounded corners from message containers, creating a flatter, more streamlined appearance. This is consistent with the broader UI refresh described in the PR objectives.
agent.py (1)
789-806: LGTM: Backward-compatible signature extension.The
call_chat_modelsignature extension properly adds token tracking support:
- Optional
tokens_callbackparameter maintains backward compatibility- Rate limiter callback handling correctly conditioned on
backgroundflag- Token callback properly propagated to underlying model call
python/helpers/log.py (3)
135-143: LGTM: Well-designed timing and token metadata.The new fields are properly initialized:
timestampcaptured at creation time in__post_init__duration_mscalculated later when next item is logged- Token fields initialized to zero for accumulation
This design cleanly supports the PR's goal of tracking per-step timing and token usage.
170-175: LGTM: Accumulative token tracking.The
add_tokensmethod provides a clean API for incrementally updating token counts during streaming operations. The implementation correctly:
- Accumulates tokens_in and tokens_out
- Marks the item as updated in the log's update list
- Guards against stale log items by checking guid
235-239: LGTM: Automatic duration calculation.The duration calculation is elegantly implemented:
- Computed automatically when a new log item is created
- Previous item's duration set based on timestamp difference
- Previous item marked as updated to propagate the change
This ensures duration is always calculated without requiring manual tracking by callers.
webui/index.html (1)
12-12: LGTM: Process-group stylesheet integration.The new stylesheet is properly integrated to support the process-group UI feature introduced in this PR. The placement in the loading order is appropriate, ensuring styles cascade correctly with other component styles.
webui/components/messages/process-group/process-group-store.js (2)
104-124: Defensive fallback ingetStepCodehandles edge cases well.The fallback
type?.toUpperCase()?.slice(0, 4) || 'GEN'correctly handles null/undefined types. Good use of optional chaining.
14-27: Error handling ininit()is appropriate.The try/catch with console.error provides resilience against corrupted localStorage data without crashing the UI.
webui/components/sidebar/bottom/preferences/preferences-store.js (3)
131-142: DOM queries for dynamic elements may not cover newly added process steps.The
querySelectorAllcalls in_applyShowThoughts,_applyShowJson, and_applyShowUtilsonly affect elements that exist at the time the preference is toggled. Newly created process steps receive the correct initial state viapreferencesStore.showThoughtschecks inmessages.js, so this works correctly for the current implementation.
166-177: Defensive access to Alpine store is appropriate.The try/catch with optional chaining (
window.Alpine?.store("processGroup")) handles the case where the store may not be initialized yet during app startup.
62-70: NewcollapseProcessGroupspreference follows established patterns.The getter/setter pattern with backing field and apply method is consistent with other preferences in this store.
webui/index.js (3)
188-195: ExtendedsetMessagesignature correctly forwards all parameters.The wrapper function properly passes all new metadata fields (timestamp, durationMs, tokensIn, tokensOut) to the underlying
msgs.setMessagefunction.
293-300: Process group reset on chat reset prevents stale UI state.Calling
msgs.resetProcessGroups()when the chat is reset ensures the collapsible groups don't carry over from a previous session.
491-493: Process group reset on context switch is correct.Resetting process groups when switching contexts prevents UI state from leaking between different chat sessions.
webui/components/messages/process-group/process-group.css (3)
336-361: Grid-based expand/collapse animation is performant.Using
grid-template-rowsfor the expand/collapse animation avoids layout thrashing that can occur withheight: autotransitions. Good approach.
243-266: CSS-only spinner avoids icon font dependency.Using a CSS border-based spinner with animation is efficient and removes the need for loading an icon font just for the loading indicator.
833-857: Preference visibility controls align with store toggle logic.The CSS rules for
.message-util,.msg-thoughts, and.msg-jsoncorrectly use class-based visibility toggling that matches the JavaScript implementation in the preferences store.webui/js/messages.js (6)
20-30: Agent response detection logic is correct.The
isMainAgentResponsefunction correctly identifies the main agent (A0) vs subordinate agents (A1, A2, etc.) using a regex pattern. The fallback totruewhen no marker is found is sensible.
42-64: Process group routing for process types is well-structured.The logic correctly handles:
- User messages reset the current process group
- Existing steps are updated rather than recreated
- New process groups are created on demand
- Steps are properly added to the current group
84-103: Main response handling correctly embeds the process group.When the main agent (A0) responds, the current process group is embedded into the response container and marked as complete. The logic properly handles both new and existing containers.
1766-1769:resetProcessGroupsexport provides necessary cleanup API.This function correctly resets both module-level state variables when called from
index.json context switches or chat resets.
1774-1789: Token formatting handles edge cases correctly.The
formatTokenCountfunction properly handles:
- Both input and output tokens present
- Only input tokens (with ↓ indicator)
- Only output tokens (with ↑ indicator)
- Neither present (returns "--")
The compact notation (k/m suffixes) is user-friendly.
464-468: Heading removal in user messages aligns with design intent.The comment indicates this change is per target design. The implementation correctly removes the heading element if present.
|
|
||
| def __post_init__(self): | ||
| self.guid = self.log.guid | ||
| self.timestamp = time.time() |
There was a problem hiding this comment.
Deserialized timestamps are overwritten with current time
The LogItem.__post_init__ method unconditionally sets self.timestamp = time.time(), which overwrites any timestamp passed to the constructor. When _deserialize_log creates LogItem objects with saved timestamps from persisted chat data, the original timestamps are immediately replaced with the current time. This corrupts all time-related data on chat restoration, making duration calculations incorrect and showing wrong timing information in the UI.
Additional Locations (1)
| existingHeading.remove(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Error message headings silently discarded in _drawMessage
The _drawMessage function accepts a heading parameter but no longer renders it anywhere in the function body - the heading rendering code was removed. This affects error and rate_limit message types which are in MAIN_TYPES and don't go through the process group system, causing their headings (which contain important error context like "Error in code execution") to be silently lost. Users will see error content without the descriptive heading that explains what type of error occurred.
3clyp50
force-pushed
the
pr/UI-fresh
branch
2 times, most recently
from
5477d8f to
1b87c9f
Compare
- backend-driven step duration with live counter - Add duration_ms field to LogItem (python/helpers/log.py) - Pass duration_ms through API to frontend - Live timer for in-progress steps, duration_ms final value for completed
remove token counter; unified cleanText; icons coverage
remove leftover user status label
duration and timestamp persist log and process groups cleanup
|
|
||
| const mins = Math.floor(durationMs / 60000); | ||
| const secs = Math.round((durationMs % 60000) / 1000); | ||
| return `${mins}m${secs}s`; |
There was a problem hiding this comment.
Duration formatting produces invalid "1m60s" output
The formatDuration function can produce invalid output like "1m60s" when the seconds component rounds up to 60. For example, durationMs = 119500 (1 min 59.5 sec) calculates mins = 1 and secs = Math.round(59.5) = 60, resulting in "1m60s" instead of the correct "2m0s". The rounding of seconds needs to carry over to minutes when it reaches 60.
| stepHeader.addEventListener("click", (e) => { | ||
| e.stopPropagation(); | ||
| processGroupStore.toggleStep(groupId, id); | ||
| step.classList.toggle("step-expanded", processGroupStore.isStepExpanded(groupId, id)); |
There was a problem hiding this comment.
First click to collapse agent steps does nothing
When agent steps are created with showThoughts preference enabled, they're expanded via the preference check (type === "agent" && preferencesStore.showThoughts) but the processGroupStore.expandedSteps map doesn't track this state. When a user clicks to collapse, toggleStep flips undefined to true, and isStepExpanded returns true, so the class toggle does nothing visible. A second click is required to actually collapse the step. The store's isStepExpanded check doesn't account for the preference-based expansion that occurred at creation time.
Note
Introduces a structured, collapsible “process group” view for agent activity and wires it to richer backend log data.
index.js/messages now pass throughtimestamp,duration_ms, andagent_number; polling resets process groups on context changes; response embeds the completed process group.LogItemgainstimestamp,duration_ms,agent_number;Log.logcomputes duration for the previous item; persistence (serialize/deserialize) updated accordingly.Written by Cursor Bugbot for commit 74595c6. This will update automatically on new commits. Configure here.