feat(ui): implement duplicate request mitigation and input debounce controls #266 - #458
feat(ui): implement duplicate request mitigation and input debounce controls #266#458Shubham-Gotawade wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds cross-screen duplicate-request guarding with a 2‑second debounce to analysis triggers, tracks analysis state in ChangesAnalysis Duplicate-Request Prevention with Debounce
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs:
Suggested labels:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/ui/app_test.go (1)
32-37: ⚡ Quick winDuplicate-request checks are not exercising the input-state guards directly
These duplicate assertions run after the model has already moved to loading states, so they can pass even if the
stateInput/stateCompareInputguard logic regresses. Add explicit cases that keep state at the entry boundary (e.g., setm.state = stateInputwithanalysisInProgress = true, and similarly forstateCompareInput) and assertcmd == nil.Also applies to: 84-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/ui/app_test.go` around lines 32 - 37, The test's duplicate-request assertions check after the model already transitioned to loading, so add explicit cases that exercise the input-state guards directly: set m.state = stateInput and m.analysisInProgress = true, call m.Update(duplicateMsg) and assert returned cmd is nil and model remains MainModel.analysisInProgress true; repeat the same pattern for m.state = stateCompareInput (and the duplicate compare message) to cover the other guard (also apply same fix for the similar assertions around the block referenced at 84-87). Ensure you call m.Update(...) on the same MainModel instance and assert cmd == nil and analysisInProgress unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/ui/app.go`:
- Around line 354-362: The compare starts work without a cancelable context
(m.analysisInProgress is cleared on ESC but the running compareRepos continues),
so make compareRepos cancellable and wire a cancel func into the model: add a
field like m.compareCancel (context.CancelFunc), create a ctx, cancel :=
context.WithCancel(context.Background()) before setting m.analysisInProgress and
m.lastSubmitTime and pass ctx into compareRepos (instead of starting it without
cancellation), store cancel in m.compareCancel, and on ESC (where
analysisInProgress is cleared) call m.compareCancel() and set it to nil; also
ensure compareRepos clears m.analysisInProgress and nils m.compareCancel on
normal completion or error so a new submission cannot start while the previous
compare is still running.
---
Nitpick comments:
In `@internal/ui/app_test.go`:
- Around line 32-37: The test's duplicate-request assertions check after the
model already transitioned to loading, so add explicit cases that exercise the
input-state guards directly: set m.state = stateInput and m.analysisInProgress =
true, call m.Update(duplicateMsg) and assert returned cmd is nil and model
remains MainModel.analysisInProgress true; repeat the same pattern for m.state =
stateCompareInput (and the duplicate compare message) to cover the other guard
(also apply same fix for the similar assertions around the block referenced at
84-87). Ensure you call m.Update(...) on the same MainModel instance and assert
cmd == nil and analysisInProgress unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b7e4081-a597-4f98-a234-8652f5822c4e
📒 Files selected for processing (2)
internal/ui/app.gointernal/ui/app_test.go
…minate zombie background routines
|
Resolved in commit edc728b. Added a dedicated |
Contributed as part of GSSoC '26.
Problem Description
The application lacked an ingress-throttling architecture for database and API analysis commands. If a user rapidly double-clicked execution buttons, spammed shortcuts, or re-submitted processing targets during active loading routines, multiple parallel background worker threads were dispatched for the exact same entity. This behavior triggered redundant HTTP loads, accelerated GitHub token rate-limit depletion, and introduced potential data race hazards across internal view states. Fixes #266.
Solution Implemented
MainModelstruct insideinternal/ui/app.gowith ananalysisInProgresslogical gate and alastSubmitTimetracking stamp.stateInput,stateCompareInput,stateFavorites/History, and theDashboardhotkey handler). Submissions are instantly dropped if an analysis is active or iftime.Since(lastSubmitTime)falls below a strict 2-second debounce threshold.stateLoadingandstateCompareLoadingto clear the active execution token under all exit vectors: complete payload delivery, execution failures/exceptions, or manual user-driven aborts (ESC/BackToMenuMsg).internal/ui/app_test.go(TestAnalysisDeduplicationAndDebounceandTestCompareDeduplication) to assert that rapid sequential events are dropped seamlessly.Verification & Test Logs
Validated across localized system paths with passing results:
go test -v ./internal/ui/...: PASSED (Ingress guards and debounce clocks verified)go build ./...: PASSED (Clean binary compilation)Summary by CodeRabbit
Bug Fixes
Tests