Skip to content

fix: address Greptile review feedback on PR #21018 - #21057

Merged
ishaan-jaff merged 1 commit into
litellm_mcp_ui_logs_page_fixesfrom
litellm_fix_greptile_21018
Feb 12, 2026
Merged

fix: address Greptile review feedback on PR #21018#21057
ishaan-jaff merged 1 commit into
litellm_mcp_ui_logs_page_fixesfrom
litellm_fix_greptile_21018

Conversation

@ghost

@ghost ghost commented Feb 12, 2026

Copy link
Copy Markdown

Fixes Greptile code review feedback from #21018.

Changes

  • Session time range bug: Use Math.min/Math.max across all sessionLogs entries instead of [0]/[length-1] (array is sorted by type, not time)

Already addressed in the branch

The other 5 Greptile comments were already resolved:

  1. LogDetailContent.tsx exists
  2. ✅ Clipboard call already wrapped in try/catch
  3. ✅ Dedup uses O(1) Map lookup (not O(n²))
  4. model_dump() serialization documented
  5. ✅ GROUP BY performance comment present

- Fix session time range calculation: use Math.min/Math.max across all
  entries instead of relying on array order (sessionLogs is sorted by
  type, not time).

Other Greptile comments were already addressed in the branch:
- LogDetailContent.tsx exists
- Clipboard call already wrapped in try/catch
- Dedup already uses O(1) Map lookup
- model_dump() serialization is documented
- GROUP BY performance comment already present
@vercel

vercel Bot commented Feb 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Building Building Preview, Comment Feb 12, 2026 5:07pm

Request Review

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ishaan-jaff
ishaan-jaff merged commit 79d4d7b into litellm_mcp_ui_logs_page_fixes Feb 12, 2026
3 of 49 checks passed
@greptile-apps

greptile-apps Bot commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

Fixes an incorrect session time range calculation in LogDetailsDrawer.tsx. The sessionLogs array is sorted by call type (LLM before MCP) then by startTime within each group, so using [0] and [length-1] to get session start/end times was incorrect — a MCP call could have the earliest start or latest end time. The fix correctly uses Math.min/Math.max across all entries to compute the true session time bounds.

  • Replaced index-based time access (sessionLogs[0].startTime / sessionLogs[length-1].endTime) with Math.min(...sessionLogs.map(...)) / Math.max(...sessionLogs.map(...))

Confidence Score: 5/5

  • This PR is safe to merge — it's a small, targeted bug fix with correct logic.
  • The change is a 4-line fix replacing incorrect index-based access with Math.min/Math.max. The logic is correct, the scope is minimal, and it doesn't introduce any new dependencies or side effects. The existing null-safety pattern (checking sessionLogs.length > 0) is preserved.
  • No files require special attention.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/view_logs/LogDetailsDrawer/LogDetailsDrawer.tsx Fixed session time range calculation to use Math.min/Math.max instead of array index access, since sessionLogs is sorted by call type (LLM vs MCP) first, not purely by time.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

ishaan-jaff added a commit that referenced this pull request Feb 12, 2026
…way (#21018)

* commit new expansion

* fix MCP

* fix: LiteLLMProxyRequestSetup

* _process_mcp_tools_without_openai_transform

* UI fixes

* UI refactor view logs/sessions

* index

* _add_mcp_tool_metadata_to_final_chunk

* add badges

* add getEventDisplayName

* ui fixes

* backend fix

* fix

* UI fix

* UI fix

* fix row

* fix: address Greptile review feedback on PR #21018 (#21057)

- Fix session time range calculation: use Math.min/Math.max across all
  entries instead of relying on array order (sessionLogs is sorted by
  type, not time).

Other Greptile comments were already addressed in the branch:
- LogDetailContent.tsx exists
- Clipboard call already wrapped in try/catch
- Dedup already uses O(1) Map lookup
- model_dump() serialization is documented
- GROUP BY performance comment already present

---------

Co-authored-by: shin-bot-litellm <shin-bot-litellm@berri.ai>
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…way (BerriAI#21018)

* commit new expansion

* fix MCP

* fix: LiteLLMProxyRequestSetup

* _process_mcp_tools_without_openai_transform

* UI fixes

* UI refactor view logs/sessions

* index

* _add_mcp_tool_metadata_to_final_chunk

* add badges

* add getEventDisplayName

* ui fixes

* backend fix

* fix

* UI fix

* UI fix

* fix row

* fix: address Greptile review feedback on PR BerriAI#21018 (BerriAI#21057)

- Fix session time range calculation: use Math.min/Math.max across all
  entries instead of relying on array order (sessionLogs is sorted by
  type, not time).

Other Greptile comments were already addressed in the branch:
- LogDetailContent.tsx exists
- Clipboard call already wrapped in try/catch
- Dedup already uses O(1) Map lookup
- model_dump() serialization is documented
- GROUP BY performance comment already present

---------

Co-authored-by: shin-bot-litellm <shin-bot-litellm@berri.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants