Skip to content

fix: discard active list on field discard#1049

Merged
amhsirak merged 2 commits intodevelopfrom
discard-list
May 7, 2026
Merged

fix: discard active list on field discard#1049
amhsirak merged 2 commits intodevelopfrom
discard-list

Conversation

@RohitR311
Copy link
Copy Markdown
Collaborator

@RohitR311 RohitR311 commented May 3, 2026

What this PR does?

When a user discards a capture list action by clicking the ✕ button on its tab in the output preview section (instead of the Discard button in the right panel), the right panel continues showing the capture list steps UI as if the action is still in progress.

When the ✕ button is clicked on the active capture list tab, the same cleanup logic that runs when clicking Discard in the right panel now also runs, resetting all related state so the right panel correctly disappears.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of list action removal with enhanced UI state cleanup and reset operations.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 3, 2026

Walkthrough

The InterpretationLog component is enhanced to track the currently active list action and perform coordinated cleanup when that action is removed. Hook destructuring expands to include additional control functions and state, and the removal handler conditionally resets list-capture UI state and calls cleanup only when the removed action matches the current one.

Changes

List Action Cleanup and State Tracking

Layer / File(s) Summary
Imports & Dependencies
src/components/run/InterpretationLog.tsx (18–24)
Removed unused SidePanelHeader import and added clientSelectorGenerator to support cleanup operations.
Hook State Expansion
src/components/run/InterpretationLog.tsx (62–66)
useActionContext() now provides pagination/limit visibility setters and setCaptureStage. useGlobalInfoStore() now provides currentListActionId and setCurrentListActionId to track the active list action.
Cleanup Logic
src/components/run/InterpretationLog.tsx (179–189)
handleRemoveListAction() now performs a conditional reset sequence (stop list/text capture, hide UI options, reset capture stage, clear action ID, run clientSelectorGenerator.cleanup()) when the removed action matches currentListActionId.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Type: Bug, Scope: Recorder, Scope: UI/UX

Suggested reviewers

  • amhsirak

Poem

A rabbit hops through capture logs,
When actions fade, no more the fog—
Cleanup cascades, state resets clean, 🐰
Pagination vanished, stages convene,
List actions leap to harmony's song! 🎵

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: discard active list on field discard' directly and clearly summarizes the main change: ensuring that discarding a capture list tab triggers proper cleanup logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch discard-list

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/run/InterpretationLog.tsx`:
- Around line 179-182: When cleaning up the active list in InterpretationLog,
call stopGetList() last to avoid races with pagination/limit toggles: currently
the block comparing actionId === currentListActionId calls stopGetList() before
stopPaginationMode() and stopLimitMode(), but stopPaginationMode() reads the
current render's getList and can re-emit setGetList:true; reorder so
stopPaginationMode() and stopLimitMode() execute first and then call
stopGetList() to ensure the socket-side list capture is truly disabled. Use the
existing symbols currentListActionId, stopPaginationMode, stopLimitMode, and
stopGetList to locate and change the call order.
🪄 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: 3ae18304-cc3d-4be2-afe3-522ee5988daf

📥 Commits

Reviewing files that changed from the base of the PR and between 1f0da54 and dfe64dc.

📒 Files selected for processing (1)
  • src/components/run/InterpretationLog.tsx

Comment on lines +179 to +182
if (actionId === currentListActionId) {
stopGetList();
stopPaginationMode();
stopLimitMode();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Call stopGetList() last in this cleanup path.

stopPaginationMode() still consults the current render's getList value and can emit setGetList: true right after stopGetList() emits false, so discarding the active list can leave list capture re-enabled on the socket side. Reordering these calls avoids the conflicting emits.

Suggested fix
     if (actionId === currentListActionId) {
-      stopGetList();
       stopPaginationMode();
       stopLimitMode();
+      stopGetList();
       setShowPaginationOptions(false);
       setShowLimitOptions(false);
       setCaptureStage('initial');
       setCurrentListActionId('');
       clientSelectorGenerator.cleanup();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (actionId === currentListActionId) {
stopGetList();
stopPaginationMode();
stopLimitMode();
if (actionId === currentListActionId) {
stopPaginationMode();
stopLimitMode();
stopGetList();
setShowPaginationOptions(false);
setShowLimitOptions(false);
setCaptureStage('initial');
setCurrentListActionId('');
clientSelectorGenerator.cleanup();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/run/InterpretationLog.tsx` around lines 179 - 182, When
cleaning up the active list in InterpretationLog, call stopGetList() last to
avoid races with pagination/limit toggles: currently the block comparing
actionId === currentListActionId calls stopGetList() before stopPaginationMode()
and stopLimitMode(), but stopPaginationMode() reads the current render's getList
and can re-emit setGetList:true; reorder so stopPaginationMode() and
stopLimitMode() execute first and then call stopGetList() to ensure the
socket-side list capture is truly disabled. Use the existing symbols
currentListActionId, stopPaginationMode, stopLimitMode, and stopGetList to
locate and change the call order.

@RohitR311 RohitR311 added Type: Bug Something isn't working Scope: UI/UX Issues/PRs related to UI/UX labels May 3, 2026
@amhsirak amhsirak merged commit ca5702a into develop May 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: UI/UX Issues/PRs related to UI/UX Type: Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants