You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(frontend): fix five /w playground and prompts bugs found in live QA - #8108
Five bugs in the /w app that live QA found. Each one is old (also on main), and each fix is its own commit with a test that fails before the fix.
What users saw
A reload after closing an evaluator drawer lost the test set and the drafts. In the /w playground, open an evaluator drawer (header "Evaluator" > Create new, or New Evaluation > Create new evaluator) and close it. The URL lost ?revisions= and the #pgSnapshot hash, so a reload showed one empty row and no test set.
The /w evaluator playground let you leave with unsaved edits. The app playground asks before the page unloads; the evaluator playground did not.
/w compare mode logged "Maximum update depth exceeded" 100 to 400 times per load or run.
The /w Prompts row menu logged "Maximum call stack size exceeded" when Move, Rename or Delete (folder) opened its dialog. The action still worked.
/w "Add all matching to queue" Retry sent the traces that were already queued again. The API does not drop duplicate trace ids, so the queue got duplicate scenarios.
Why, and what changed
Closing the drawer cleared revisionId from the URL even when it was not there. That navigation rebuilds the URL from the Next router query, which does not know about ?revisions= and the hash, because the playground writes those with history.replaceState. useQueryParamState now does nothing when it clears a param that is not in the URL.
ConfigureEvaluator now calls useLocalDraftWarning, like Playground.tsx.
PanelSessionInspectorButton (one per compare column) called executionController.selectors.backendSessionId(...) in render. That atom family keys on an object, so every render got a new atom, and useAtomValue rendered again without end. The atom is now kept with useMemo.
The row menu (Radix, from @agenta/ui/table) traps the focus until it closes, and the antd dialog traps it too. The dialog opened while the menu still held the trap, and the two traps moved the focus back and forth. The four menu items that open an antd dialog now open it one tick later, after the menu has closed. App Rename and Archive did not show the loop and are unchanged.
state/appState/hooks.test.tsx: clearing an absent param does not navigate; clearing a present one still does.
ConfigureEvaluator/index.test.tsx: the page uses the leave warning.
PanelSessionInspectorButton.test.tsx: counts commits; 103 before the fix, under 5 after.
usePromptsColumns.test.tsx: Move (app and folder), Rename and Delete call their handler only after the menu has closed.
useBatchAddTracesToQueue.test.tsx: real scan and real queue add with HTTP mocked; after a failed second batch, Retry sends only the 50 traces that failed.
Checks: @agenta/oss and @agenta/mobile type checks pass, lint has 0 errors.
Live check
On a fresh OSS dev stack, before and after the fix:
Bug
Before
After
1
Close drawer: revisions gone, hash empty; reload shows no test set
URL kept; reload keeps the test set and 2 rows (header path and New Evaluation path)
2
Unsaved evaluator edit: beforeunload not prevented
prevented
3
Click Compare: 243 "Maximum update depth" errors
0
4
Each Move/Rename/Delete dialog: 10 page errors
0; a real move still lands in the target folder
5
Not run live (needs a failing queue add); covered by the unit test
Not fixed here
If the drawer's prev/next writes revisionId into the playground URL, closing it still rebuilds the URL from the router query. The playground drawers do not use prev/next today.
The object-keyed atom families in @agenta/playground (backendSessionIdAtomFamily and others) have no key comparator, so every inline call builds a new atom. A comparator there would remove this kind of loop at the source; it is a shared-package change for a later PR.
Closing the drawer cleared revisionId even when the URL had none. That
navigation rebuilt the URL from the router query, which lacks the
?revisions= and #pgSnapshot that the playground writes itself, so a
reload lost the connected test set and the drafts. Clearing a param that
is not in the URL is now a no-op.
PanelSessionInspectorButton read executionController.selectors.backendSessionId
inline. Its atom family keys on an object, so each render got a new atom,
and useAtomValue rendered again without end ("Maximum update depth
exceeded" 100 to 400 times per load). The atom is now memoized per panel.
The row menu (Radix) and the antd dialogs both trap the focus. Move,
Rename and Delete opened their dialog while the menu still trapped it,
and the two traps moved the focus back and forth until "Maximum call
stack size exceeded". These items now open the dialog on the next tick.
Retry ran the whole scan again and sent the traces that the failed run had
already added, which made duplicate scenarios in the queue. The hook now
keeps the ids each run added, skips them on Retry, lowers the per-run cap
by their number, and counts them in the toasts.
Review loop result for head df8860523c (base d1c6dd2e67), 1 round (Opus and Codex on slop and simplify; Codex on correctness).
Lens
Final rating
Slop
Low (Codex Low; Opus Low, one item it marked Medium)
Simplify
Low
Correctness
Low
Fixed: the "Queue add incomplete" toast now counts the same way as the other toasts (earlier runs plus this run). The comment on afterMenuCloses now says why it is a timeout and which items do not need it. Left on purpose: app Rename and Archive are not deferred: a live check of both dialogs showed no focus loop. useDeferredMenuSelect is not reused: the menu lives in @agenta/ui and has no per-item close callback, and this PR does not change shared packages. If drawer prev/next writes revisionId into the playground URL, closing it still rebuilds the URL from the router query; the playground drawers do not use prev/next today. A key comparator for the object-keyed atom families in @agenta/playground is a later shared-package change.
Reviewing files that changed from the base of the PR and between df88605 and df88605.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
Summary
Summary by CodeRabbit
New Features
Added a warning when leaving evaluator configuration with unsaved changes.
Bug Fixes
Batch trace retries now skip traces already added and account for them in progress and limits.
Prompt actions that open dialogs now wait for their menus to close.
Improved session inspector rendering stability and prevented unnecessary navigation when clearing an absent revision parameter.
This pull request updates five web behaviors: evaluator draft warnings, session inspector selector creation, trace queue retries, prompt row actions, and query parameter navigation. It also adds tests for these changes.
ConfigureEvaluatorPageInner calls useLocalDraftWarning(). A test checks that the hook is called when the page renders.
Session inspector selector
Layer / File(s)
Summary
Memoize the session selector web/oss/src/components/SessionInspector/PanelSessionInspectorButton.tsx, web/oss/src/components/SessionInspector/PanelSessionInspectorButton.test.tsx
The button memoizes the backend session ID selector atom using loadableId and entityId. A profiler test checks the render commit count.
Trace queue retries
Layer / File(s)
Summary
Carry added trace IDs across retries web/oss/src/components/pages/observability/components/ObservabilityHeader/useBatchAddTracesToQueue.tsx, web/oss/src/components/pages/observability/components/ObservabilityHeader/useBatchAddTracesToQueue.test.tsx
Retries reuse successfully added trace IDs, exclude them from later scans, and include them in progress and notification counts. The test checks trace IDs and notifications after a failed batch and retry.
Prompt row actions
Layer / File(s)
Summary
Defer actions until the menu closes web/oss/src/components/pages/prompts/hooks/usePromptsColumns.tsx, web/oss/src/components/pages/prompts/hooks/usePromptsColumns.test.tsx
Folder Rename, Move, and Delete actions and app Move defer their callbacks until the menu closes. Tests check that the menu is closed when the callbacks run.
Query parameter navigation
Layer / File(s)
Summary
Skip navigation for absent parameters web/oss/src/state/appState/hooks.ts, web/oss/src/state/appState/hooks.test.tsx
useQueryParamState skips navigation when both the resolved and current values are nullish. Tests cover clearing absent and present revisionId parameters.
Agenta-AI/agenta#8070: Changes the shared useLocalDraftWarning hook; this pull request calls that hook from ConfigureEvaluatorPageInner.
Agenta-AI/agenta#8012: Adds the shared playground leave guard and changes the dirty-state contract used by useLocalDraftWarning.
Agenta-AI/agenta#8075: Changes playground URL selection and snapshot restoration, related to this pull request's handling of absent revisionId query parameters.
Merge Risk: Minimal · up to df886
The fixes can merge after normal checks. Shorten the two focus-trap comments to comply with the frontend guidance.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly identifies a frontend fix covering five bugs in the playground and Prompts areas.
Description check
The description directly explains all five fixes, their causes, tests, validation results, and out-of-scope items.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 10 files.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
The ordering constraint is genuinely surprising, so the guidance permits more than one short line. It permits only up to two sentences, however.
Suggested fix
-/**- * The row menu (Radix) and the antd dialogs both trap the focus. When Move, Rename or Delete- * opened its dialog while the menu was still open, each trap moved the focus back in turn, and the- * /w Prompts page logged "Maximum call stack size exceeded". A dialog must open only after the- * menu has closed.- */+/**+ * Radix and antd both trap focus, so opening a dialog before the menu closes can cause a focus loop+ * and log "Maximum call stack size exceeded" on the /w Prompts page. Open dialogs only after the+ * menu closes.+ */
The focus-trap ordering and missing callback contract justify a longer comment, but five sentences exceed the two-sentence limit. Keep both points concise.
Suggested fix
-/**- * The row menu traps the focus until it has closed, and an antd dialog traps it too. A dialog- * opened from the menu's click handler made the two move the focus back and forth without end- * ("Maximum call stack size exceeded"). So the menu items that open an antd dialog wait one tick,- * until the menu has closed. A timeout, because the menu lives in `@agenta/ui` and gives no close- * callback per item. The app Rename and Archive dialogs do not show the loop, so they open- * directly.- */+// Both the row menu and antd dialogs trap focus, so defer dialog-opening actions until the menu closes.+// `ActionItem` exposes no per-item close callback, so `setTimeout(..., 0)` supplies the ordering.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.
Preview use finished or expired. Environment absence verified; retained storage requires separate provider accounting.
Post /preview as a new comment to request a one-hour preview. Repeating it while active does not extend the hour.
Preview data is disposable. Expiry is enforced by the next successful cleanup sweep, not an exact timer.
Environment deletion stops its compute. Provider-retained storage and permanent services may still be billed.
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Five bugs in the /w app that live QA found. Each one is old (also on
main), and each fix is its own commit with a test that fails before the fix.What users saw
?revisions=and the#pgSnapshothash, so a reload showed one empty row and no test set.Why, and what changed
revisionIdfrom the URL even when it was not there. That navigation rebuilds the URL from the Next router query, which does not know about?revisions=and the hash, because the playground writes those withhistory.replaceState.useQueryParamStatenow does nothing when it clears a param that is not in the URL.ConfigureEvaluatornow callsuseLocalDraftWarning, likePlayground.tsx.PanelSessionInspectorButton(one per compare column) calledexecutionController.selectors.backendSessionId(...)in render. That atom family keys on an object, so every render got a new atom, anduseAtomValuerendered again without end. The atom is now kept withuseMemo.@agenta/ui/table) traps the focus until it closes, and the antd dialog traps it too. The dialog opened while the menu still held the trap, and the two traps moved the focus back and forth. The four menu items that open an antd dialog now open it one tick later, after the menu has closed. App Rename and Archive did not show the loop and are unchanged.excludeTraceIds(the scan already supports it), lowers the per-run cap by their number, and the toasts count them. This is the /w half of what fix(frontend): queue scan stops on a failed trace read, Retry sends only the rest, live filter drops annotation fields #8098 does for /m.No shared package changes, so /m does not change.
Tests
Each test fails on
mainand passes with the fix:state/appState/hooks.test.tsx: clearing an absent param does not navigate; clearing a present one still does.ConfigureEvaluator/index.test.tsx: the page uses the leave warning.PanelSessionInspectorButton.test.tsx: counts commits; 103 before the fix, under 5 after.usePromptsColumns.test.tsx: Move (app and folder), Rename and Delete call their handler only after the menu has closed.useBatchAddTracesToQueue.test.tsx: real scan and real queue add with HTTP mocked; after a failed second batch, Retry sends only the 50 traces that failed.Checks:
@agenta/ossand@agenta/mobiletype checks pass, lint has 0 errors.Live check
On a fresh OSS dev stack, before and after the fix:
revisionsgone, hash empty; reload shows no test setbeforeunloadnot preventedNot fixed here
revisionIdinto the playground URL, closing it still rebuilds the URL from the router query. The playground drawers do not use prev/next today.@agenta/playground(backendSessionIdAtomFamilyand others) have no key comparator, so every inline call builds a new atom. A comparator there would remove this kind of loop at the source; it is a shared-package change for a later PR.