fix: prevent rerun flicker and fix runner configuration list order#7639
Conversation
gopu-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughRequest-flattening logic now explicitly separates folder and request items during traversal, sorting folders by name and requests by sequence, then recursively processing each folder's contents. A ref-based flag tracks "Run Again" operations to prevent premature display of empty-state UI during reruns. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (1)
packages/bruno-app/src/components/RunnerResults/index.jsx (1)
169-174: Defensive status check includes unused 'cancelled' value.Per the
runFolderEventreducer (context snippet 1), only'started'and'ended'statuses are set—'cancelled'is never assigned at the runner info level. The check for'cancelled'is effectively dead code today but provides forward-compatibility if that status is added later.The logic correctly handles the edge case where all requests are filtered out (empty
itemsbut status becomes'ended').📝 Optional: Add comment explaining the defensive check
useEffect(() => { if (isReRunningRef.current + // 'cancelled' not currently set by reducer but included for future-proofing && (items?.length > 0 || runnerInfo?.status === 'ended' || runnerInfo?.status === 'cancelled')) { isReRunningRef.current = false; } }, [items, runnerInfo?.status]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RunnerResults/index.jsx` around lines 169 - 174, The useEffect block that checks isReRunningRef.current and (items?.length > 0 || runnerInfo?.status === 'ended' || runnerInfo?.status === 'cancelled') contains a defensive check for a 'cancelled' status that isn't currently set by the runFolderEvent reducer; add a concise inline comment above this condition (near the useEffect and isReRunningRef usage) explaining that 'cancelled' is included intentionally for forward-compatibility because the reducer currently only emits 'started' and 'ended' but a future change may introduce 'cancelled', so the branch should remain to handle filtered-out items when runs terminate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsx`:
- Around line 192-194: The current sort for requestItems uses a comparator (a,
b) => a.seq - b.seq which yields NaN when seq is undefined; update the
comparator used when building requestItems so it defensively handles missing seq
(similar to sortByNameThenSequence for folders) by treating undefined seq as a
large value or falling back to sequence 0 and/or comparing names when seqs are
equal; adjust the sort call in the requestItems construction (the
.filter(...).sort(...) chain where isItemARequest is used) to use a comparator
that normalizes a.seq and b.seq (e.g., coerce to finite numbers or use nullish
coalescing) and ties break deterministically.
---
Nitpick comments:
In `@packages/bruno-app/src/components/RunnerResults/index.jsx`:
- Around line 169-174: The useEffect block that checks isReRunningRef.current
and (items?.length > 0 || runnerInfo?.status === 'ended' || runnerInfo?.status
=== 'cancelled') contains a defensive check for a 'cancelled' status that isn't
currently set by the runFolderEvent reducer; add a concise inline comment above
this condition (near the useEffect and isReRunningRef usage) explaining that
'cancelled' is included intentionally for forward-compatibility because the
reducer currently only emits 'started' and 'ended' but a future change may
introduce 'cancelled', so the branch should remain to handle filtered-out items
when runs terminate.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a728eb0c-b9e9-435f-8388-30c9de76caf8
📒 Files selected for processing (2)
packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsxpackages/bruno-app/src/components/RunnerResults/index.jsx
| const requestItems = items | ||
| .filter((item) => isItemARequest(item) && !item.partial && !item.isTransient) | ||
| .sort((a, b) => a.seq - b.seq); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are request items in the codebase that might lack a seq property
# Search for item creation patterns that might not set seq
rg -n --type=js --type=jsx -C3 'type:\s*["\x27](http-request|graphql-request)["\x27]' | head -100Repository: usebruno/bruno
Length of output: 85
🏁 Script executed:
cd packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel && wc -l index.jsxRepository: usebruno/bruno
Length of output: 71
🏁 Script executed:
cd packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel && sed -n '185,215p' index.jsxRepository: usebruno/bruno
Length of output: 1013
🏁 Script executed:
rg -n --type=js -C3 '"http-request"|"graphql-request"' packages/bruno-app --max-count=20Repository: usebruno/bruno
Length of output: 3171
🏁 Script executed:
rg -n 'seq.*=' packages/bruno-app/src --type=js --type=ts -A2 | grep -E '(seq|new.*Request|createItem)' | head -40Repository: usebruno/bruno
Length of output: 4801
🏁 Script executed:
rg -n "sortByNameThenSequence" packages/bruno-app/src --type=js --type=ts -A5 | head -50Repository: usebruno/bruno
Length of output: 5794
🏁 Script executed:
rg -n "const isSeqValid|isSeqValid" packages/bruno-app/src/utils/common/index.js -A2 -B2Repository: usebruno/bruno
Length of output: 607
🏁 Script executed:
rg -n "sortItemsBySequence" packages/bruno-app/src --type=js -A5 | head -40Repository: usebruno/bruno
Length of output: 3021
🏁 Script executed:
rg -n "isItemARequest" packages/bruno-app/src/utils --type=js -B2 -A5 | head -50Repository: usebruno/bruno
Length of output: 4361
Use defensive sort for request items to handle potentially undefined seq.
The sort comparator (a, b) => a.seq - b.seq returns NaN when either seq is undefined, leading to non-deterministic ordering. While sortByNameThenSequence (used for folders) validates seq explicitly, this direct sort does not. The importer logic shows seq can be missing in imported/legacy items and is conditionally assigned during hydration. Apply the defensive fix to align with folder handling:
const requestItems = items
.filter((item) => isItemARequest(item) && !item.partial && !item.isTransient)
- .sort((a, b) => a.seq - b.seq);
+ .sort((a, b) => (a.seq ?? Infinity) - (b.seq ?? Infinity));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsx`
around lines 192 - 194, The current sort for requestItems uses a comparator (a,
b) => a.seq - b.seq which yields NaN when seq is undefined; update the
comparator used when building requestItems so it defensively handles missing seq
(similar to sortByNameThenSequence for folders) by treating undefined seq as a
large value or falling back to sequence 0 and/or comparing names when seqs are
equal; adjust the sort call in the requestItems construction (the
.filter(...).sort(...) chain where isItemARequest is used) to use a comparator
that normalizes a.seq and b.seq (e.g., coerce to finite numbers or use nullish
coalescing) and ties break deterministically.
There was a problem hiding this comment.
The requestItems here is from redux store which is populated from parsed files. All the existing parsers already default missing seq fields (.bru → Number(sequence) || 1, .yml → info?.seq || 1) . Also, the sidebar component already uses the same (a, b) => a.seq - b.seq sorting logic without issues.
There was a problem hiding this comment.
@gopu-bruno, that's a fair point — since all items flow through the parsers before reaching the Redux store and both the .bru and .yml parsers guarantee a numeric fallback for seq, the undefined scenario I flagged won't occur in practice. The consistency with the sidebar's existing sort also confirms this is the right approach. Thanks for the clarification!
✏️ Learnings added
Learnt from: gopu-bruno
Repo: usebruno/bruno PR: 7639
File: packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsx:192-194
Timestamp: 2026-04-01T08:12:04.363Z
Learning: In Bruno's RunConfigurationPanel (`packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsx`), the sort comparator `(a, b) => a.seq - b.seq` used for request items is safe without a defensive `?? Infinity` guard. All request items in the Redux store are populated via parsers (`.bru` uses `Number(sequence) || 1`, `.yml` uses `info?.seq || 1`) which always assign a numeric default for `seq`. The sidebar component uses the identical comparator without issues.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6890
File: packages/bruno-electron/src/ipc/network/index.js:770-776
Timestamp: 2026-01-22T19:44:28.678Z
Learning: In Bruno's Electron IPC network handler (packages/bruno-electron/src/ipc/network/index.js), runner control signals from hooks (`skipRequest`, `nextRequestName`, `stopExecution`) are only relevant and handled in the `run-collection-folder` IPC event for collection/folder runs. For standalone request execution via `send-http-request` handler, these signals are intentionally not processed since they don't apply to single request runs where the user explicitly triggered one request.
Learnt from: abhishek-bruno
Repo: usebruno/bruno PR: 7483
File: packages/bruno-electron/src/ipc/openapi-sync.js:540-546
Timestamp: 2026-03-13T19:40:22.222Z
Learning: In `packages/bruno-electron/src/ipc/openapi-sync.js`, `compareRequestFields()` intentionally uses case-sensitive header name comparison. Both `specItem.request` and `actualRequest` originate from the same `openApiToBruno()` converter (for spec-vs-spec and spec-vs-disk comparisons), so casing is always consistent. A case difference can only arise from a manual user edit to a `.bru` file, which is a real collection change that drift detection should surface. Do not suggest normalizing header names to lowercase in this function.
3 participants
Description
Update Run Configuration ordering to mirror sidebar behavior by sorting folders and requests consistently, and avoid transient panel flicker during Run Again by guarding empty-result rerun transitions.
Screenshots
Screen.Recording.2026-03-31.at.1.55.18.PM.mov
Screen.Recording.2026-03-31.at.2.03.48.PM.mov
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit