Skip to content

[fix] Keep a deleted revision out of the /m playground selection - #8086

Merged
mmabrouk merged 1 commit into
release/v0.128.6from
fix/m-playground-delete-selection
Nov 4, 2026
Merged

mmabrouk merged 1 commit into
release/v0.128.6from
fix/m-playground-delete-selection

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Nov 4, 2026

Copy link
Copy Markdown
Member

For the release after v0.128.5.

Answer: after a delete, the playground can select the deleted revision again. That is an app bug, not only a slow test.

After a user deletes the revision on screen, the /m playground should show the newest revision that is left. In the test run on release/v0.128.5, it sometimes selected the deleted revision again. The deleted revision then stayed on screen with a load error.

Two agents disagreed on this. Both saw the same cause. The first Delete test runs into the bug, waits for a selection that never comes, and hits the 5 s test timeout. The timed-out test keeps running into the next tests ("overlapping act() calls"), so the rest of the Delete block fails as well. With a 60 s timeout, the same test fails with expected ['rev-2'] to deeply equal ['rev-1']. Baseline on the release branch: the file alone failed 3 of 5 runs (9 failures each time: 8 timeouts and one "menu not found"), and the full suite failed 2 of 2 runs (5 or 6 failures).

Cause

The selection effect in usePlaygroundSelection took the latest revision from the rendered query result (latestId). The query library tells the screen about a new answer on a later task. After a delete, the dialog waits for the reload, and then the page takes the deleted revision out. That can happen before the screen hears about the reload. In that gap latestId is still the deleted revision, so the effect selected it again.

Changes

  • usePlaygroundSelection.ts: when no selected revision is left, the effect reads the latest revision from the query cache, not from the rendered value. A failed reload is still ignored, as before.
  • New test: it holds the query library's notifications, reloads the latest revision (rev-1), and takes rev-2 out. Before the fix, rev-2 comes back. After the fix, rev-1 shows. The test needs no timing.
  • The compare-view test opened the revision menu after one fixed wait. The compare view loads its code the first time two revisions show, so the menu was sometimes not there yet. click and openMenu now wait for the control to show.

Tests

  • playgroundRevisionActions.render.test.tsx alone: 5 of 5 runs pass (28 tests).
  • Full /m suite: 5 of 5 runs pass (2197 tests).
  • /m type check passes. /m lint: 0 errors. No package changed.
  • Live check on a local OSS stack with the playground developer view: with 3 revisions, deleting v3 shows v2 and puts v2 in ?revisions=. Deleting v2 while it is open shows v3, the newest left. A reload does not bring a deleted revision back. No error state.

Note: the page goes read only when the server confirms the delete, not when the user clicks Delete. While the request runs, the delete dialog stays open over the page with its buttons disabled. This PR does not change that.

What to QA

  • Open the /m playground for an app with 3 revisions. Delete the newest from the revision menu. The playground shows the next newest, and the URL names it.
  • Open an older revision, delete it. The playground shows the newest revision left.
  • Reload after each delete. The deleted revision does not come back.

…tion

After a delete, the selection effect took the latest revision from the
rendered query result. The query library tells the screen of the
reload's answer on a later task, so the rendered value could still be
the deleted revision, and the playground selected it again. The effect
now reads the latest revision from the query cache.

The compare-view test also waits for the revision menu to show, since
the compare view loads its code on first use.
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 4c4d159 and 4c4d159.

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

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Bug Fixes
    • Prevented the playground from retaining a deleted revision in the selection after a failed reload.
    • Improved handling of revision selection while updates are pending.

When no revision IDs remain selected, the hook reads the latest-revision query state from the cache and selects its data only when the query exists, has no error, and contains data. A regression test checks the selection before and after queued notifications.

Changes: Revision selection fallback

Layer / File(s) Summary
Read latest revision from cache and verify selection
web/mobile/src/features/playground/usePlaygroundSelection.ts, web/mobile/tests/unit/playgroundRevisionActions.render.test.tsx
When the selection is empty, the hook reads the cached latest-revision query state for the current app and project. It selects the query data only when the state exists, is not errored, and has truthy data. The test deletes rev-2, holds observer notifications, and checks that rev-1 remains selected before and after releasing them. Test control lookups now use a bounded retry helper.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Low

Possibly related PRs

  • Agenta-AI/agenta#8012: Adds the usePlaygroundSelection hook whose latest-revision selection logic this PR changes.
  • Agenta-AI/agenta#8041: Adds playgroundLatestRevisionKey and the latest-revision query used by this PR.
  • Agenta-AI/agenta#8083: Describes post-delete selection assertion failures in the same test file.
Merge Risk: Minimal · up to 4c4d1

No merge-blocking risk was identified in the revision-selection change.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title names the main change: a deleted revision is kept out of the /m playground selection. The raw summary confirms the selection effect now reads the latest revision from the query cache, which …
Description check Passed The description states the bug (the playground reselects a deleted revision after a delete), its cause (the selection effect reads the rendered latestId while query notifications are pending), the cod…
Docstring Coverage Passed 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 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.
Full details: Title check

Explanation

The title names the main change: a deleted revision is kept out of the /m playground selection. The raw summary confirms the selection effect now reads the latest revision from the query cache, which matches the title's subject. The title is specific and short.

Full details: Description check

Explanation

The description states the bug (the playground reselects a deleted revision after a delete), its cause (the selection effect reads the rendered latestId while query notifications are pending), the code change in usePlaygroundSelection, the new test, the compare-view test wait fix, test results, and QA steps. It clearly relates to the changeset.

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 2 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.

Share
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 3c8d1a7 and 4c4d159.

Files selected for processing: 2
  • web/mobile/src/features/playground/usePlaygroundSelection.ts
  • web/mobile/tests/unit/playgroundRevisionActions.render.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Railway Preview Environment

Item Value
Status stopped
Commit 4c4d1598fe35d5e7bdcfefa3d86202657e50ef34
Manual expiry (UTC) Not granted
CI deadline (UTC) Not granted
Workflow https://github.com/Agenta-AI/agenta/actions/runs/41832283685

Dependent tests finished. Post /preview for manual review. 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.

@mmabrouk
mmabrouk changed the base branch from release/v0.128.5 to release/v0.128.6 November 4, 2026 15:42
@mmabrouk
mmabrouk merged commit 4048536 into release/v0.128.6 Nov 4, 2026
83 of 86 checks passed
@mmabrouk

mmabrouk commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

🤖 The AI agent says: Live recheck passed on head 4c4d159.

Setup: one fresh OSS dev stack built from release/v0.128.5 with #8075, #8076, #8077 and #8086 merged in, developer views playground,prompts,variants,endpoints,observability-app-filter on, a new account, Chromium at 1440 px and 390 px. The API ran this batch's code.

  • I added v3 through the API, opened it in the /m playground, and deleted it. The page showed v2, the newest revision left, right after the delete and after a reload.

Merge order: this PR and #8075 change the same import block in usePlaygroundSelection.ts. Whichever merges second needs the release branch merged in again. The fix is the union of the two import lists.

Checks on this head: /m types, /w types, pnpm run lint in web/mobile (0 errors), playgroundRevisionActions.render.test.tsx (28 pass), and speed:check all pass. Needed CI is green. Only the web test jobs fail, and they do not gate. "Entire Gates" is an external check, and it shows as skipped or neutral on merged release PRs.

The screen recording (MP4) is saved with the QA evidence. I could not attach it here because the browser upload was not available in this session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant