removed the log summary -different than ai-summary#1704
Merged
Conversation
WalkthroughThis PR removes the log summary feature across the codebase, including summary-related components, types, tests, and configuration, simplifying the flow run history view to use only standard log display. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Flows/SingleFlowRunHistory.tsx (2)
80-80:⚠️ Potential issue | 🟡 MinorSuspicious empty
displayvalue.
display=""is an empty string which is likely unintentional. Should this bedisplay="flex"ordisplay="block"to work with thegapproperty?🐛 Proposed fix
- <Box display="" gap="10px" sx={{ marginTop: '20px' }} data-testid="single-flow-run-logs"> + <Box display="flex" flexDirection="column" gap="10px" sx={{ marginTop: '20px' }} data-testid="single-flow-run-logs">Note:
gaponly applies when using flex or grid layout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Flows/SingleFlowRunHistory.tsx` at line 80, In SingleFlowRunHistory.tsx the Box element rendered with data-testid "single-flow-run-logs" has an empty display prop (display="") which prevents the gap from applying; update the Box's display prop to a valid layout such as "flex" (or "grid") so gap="10px" takes effect, e.g., change the display value on that Box component to "flex" to enable the spacing behavior.
29-96:⚠️ Potential issue | 🟡 MinorUpdate stale test case name that references removed functionality.
The test case
'calls errorToast when fetchLogSummaries fails'insrc/components/Flows/__tests__/SingleFlowRunHistory.test.tsx(lines 81-87) has a misleading name. It referencesfetchLogSummaries, which no longer exists in the component. The test implementation correctly testshttpGeterror handling, but the name should be updated to reflect this—for example,'calls errorToast when fetchLogs fails'(or consider removing it if it duplicates the previous test).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Flows/SingleFlowRunHistory.tsx` around lines 29 - 96, Rename the misleading test title that references removed fetchLogSummaries to reflect the current behavior: in the SingleFlowRunHistory test suite update the test name string "calls errorToast when fetchLogSummaries fails" to "calls errorToast when fetchLogs fails" (or remove the test if it duplicates another); ensure the test still asserts httpGet error handling and errorToast invocation for the fetchLogs flowRun/error path.
🧹 Nitpick comments (2)
src/components/Flows/SingleFlowRunHistory.tsx (2)
38-71: Redundant nested async IIFE.
fetchLogsis already an async function, so wrapping the body in another async IIFE is unnecessary. The pattern adds indentation and complexity without benefit.♻️ Proposed simplification
const fetchLogs = async () => { if (!flowRun) { return; } setExpandLogs(true); - (async () => { - try { - const data = await httpGet( - session, - `prefect/flow_runs/${flowRun.id}/logs?offset=${Math.max( - flowRunOffset, - 0 - )}&limit=${flowRunLogsOffsetLimit}` - ); - - if (data?.logs?.logs && data.logs.logs.length >= 0) { - const newlogs = flowRunOffset <= 0 ? data.logs.logs : logs.concat(data.logs.logs); - setLogs(newlogs); - - // increment the offset by 200 if we have more to fetch - // otherwise set it to -1 i.e. no more logs to show - const offsetToUpdate = - data.logs.logs.length >= flowRunLogsOffsetLimit - ? flowRunOffset + flowRunLogsOffsetLimit - : -1; - setFlowRunOffset(offsetToUpdate); - setRerender(!rerender); - } - } catch (err: any) { - console.error(err); - errorToast(err.message, [], globalContext); - } - })(); + try { + const data = await httpGet( + session, + `prefect/flow_runs/${flowRun.id}/logs?offset=${Math.max( + flowRunOffset, + 0 + )}&limit=${flowRunLogsOffsetLimit}` + ); + + if (data?.logs?.logs && data.logs.logs.length >= 0) { + const newlogs = flowRunOffset <= 0 ? data.logs.logs : logs.concat(data.logs.logs); + setLogs(newlogs); + + // increment the offset by 200 if we have more to fetch + // otherwise set it to -1 i.e. no more logs to show + const offsetToUpdate = + data.logs.logs.length >= flowRunLogsOffsetLimit + ? flowRunOffset + flowRunLogsOffsetLimit + : -1; + setFlowRunOffset(offsetToUpdate); + } + } catch (err: any) { + console.error(err); + errorToast(err.message, [], globalContext); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Flows/SingleFlowRunHistory.tsx` around lines 38 - 71, The fetchLogs function contains a redundant nested async IIFE; remove the inner immediately-invoked async function and move its try/catch body directly into the async fetchLogs function so the await httpGet(...) call and subsequent logic (checks on data?.logs?.logs, building newlogs with logs.concat, setLogs, computing offsetToUpdate and calling setFlowRunOffset/setRerender) execute directly inside fetchLogs; keep existing error handling (console.error and errorToast) and all referenced calls (httpGet, setLogs, setFlowRunOffset, setRerender, setExpandLogs) intact.
33-33:rerenderstate appears to be dead code.The
rerenderstate is toggled at line 64 (setRerender(!rerender)) but is never read anywhere in the component. If this was previously used for log summary logic, consider removing it as part of this cleanup.🧹 Proposed cleanup
const [rerender, setRerender] = useState<boolean>(false);If truly unused, remove the state entirely and the
setRerender(!rerender)call at line 64.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Flows/SingleFlowRunHistory.tsx` at line 33, The "rerender" useState in the SingleFlowRunHistory component is dead code; remove the const [rerender, setRerender] = useState<boolean>(false) declaration and delete the setRerender(!rerender) toggle call (the update at the location that toggles rerender) so that no unused state remains; ensure no other references to "rerender" or "setRerender" exist in the component after removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/Flows/SingleFlowRunHistory.tsx`:
- Line 80: In SingleFlowRunHistory.tsx the Box element rendered with data-testid
"single-flow-run-logs" has an empty display prop (display="") which prevents the
gap from applying; update the Box's display prop to a valid layout such as
"flex" (or "grid") so gap="10px" takes effect, e.g., change the display value on
that Box component to "flex" to enable the spacing behavior.
- Around line 29-96: Rename the misleading test title that references removed
fetchLogSummaries to reflect the current behavior: in the SingleFlowRunHistory
test suite update the test name string "calls errorToast when fetchLogSummaries
fails" to "calls errorToast when fetchLogs fails" (or remove the test if it
duplicates another); ensure the test still asserts httpGet error handling and
errorToast invocation for the fetchLogs flowRun/error path.
---
Nitpick comments:
In `@src/components/Flows/SingleFlowRunHistory.tsx`:
- Around line 38-71: The fetchLogs function contains a redundant nested async
IIFE; remove the inner immediately-invoked async function and move its try/catch
body directly into the async fetchLogs function so the await httpGet(...) call
and subsequent logic (checks on data?.logs?.logs, building newlogs with
logs.concat, setLogs, computing offsetToUpdate and calling
setFlowRunOffset/setRerender) execute directly inside fetchLogs; keep existing
error handling (console.error and errorToast) and all referenced calls (httpGet,
setLogs, setFlowRunOffset, setRerender, setExpandLogs) intact.
- Line 33: The "rerender" useState in the SingleFlowRunHistory component is dead
code; remove the const [rerender, setRerender] = useState<boolean>(false)
declaration and delete the setRerender(!rerender) toggle call (the update at the
location that toggles rerender) so that no unused state remains; ensure no other
references to "rerender" or "setRerender" exist in the component after removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41a8348a-afc8-4c8d-8c8c-dcf9327591db
📒 Files selected for processing (5)
src/components/Flows/SingleFlowRunHistory.tsxsrc/components/Logs/LogSummaryBlock.tsxsrc/components/Logs/LogSummaryCard.tsxsrc/components/Logs/__tests__/LogSummaryBlock.test.tsxsrc/config/constant.ts
💤 Files with no reviewable changes (4)
- src/components/Logs/tests/LogSummaryBlock.test.tsx
- src/components/Logs/LogSummaryBlock.tsx
- src/components/Logs/LogSummaryCard.tsx
- src/config/constant.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1704 +/- ##
==========================================
+ Coverage 75.32% 75.39% +0.06%
==========================================
Files 107 105 -2
Lines 7991 7936 -55
Branches 1948 1925 -23
==========================================
- Hits 6019 5983 -36
+ Misses 1847 1830 -17
+ Partials 125 123 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Ishankoradia
approved these changes
Mar 16, 2026
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
2 participants
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.
Currently the logs components are in the flow_logs.tsx and connectionSyncHistory.tsx files.
Summary by CodeRabbit
Chores
Tests