Skip to content

Fix AnimatedList.separated assert when removing last item mid-removal…#186389

Merged
mbcorona merged 1 commit into
flutter:masterfrom
mbcorona:fix-animated-list-separated-186361
May 20, 2026
Merged

Fix AnimatedList.separated assert when removing last item mid-removal…#186389
mbcorona merged 1 commit into
flutter:masterfrom
mbcorona:fix-animated-list-separated-186361

Conversation

@mbcorona

Copy link
Copy Markdown
Member

Summary

AnimatedList.separated.removeItem used the sliver's total child count (_itemsCount) to decide whether the just-removed item was the last item in the list. That count still includes children animating out from earlier removeItem calls, so while a previous removal was in flight the new tail of the visible list was misclassified as a middle item. The separator removal then targeted the wrong index, and once the item being removed was added to _outgoingItems the next _indexToItemIndex lookup ran past _itemsCount and tripped the itemIndex >= 0 && itemIndex < _itemsCount assertion inside _SliverAnimatedMultiBoxAdaptorState.removeItem.

The fix captures _itemsCount - _outgoingItems.length before mutating the sliver and uses that visible count for both the "are there enough items to have a separator?" and "is this the new last item?" checks. When no removal is in flight _outgoingItems is empty and behavior is unchanged, so existing .separated coverage and the removeAllItems regression test from #176452 continue to pass.

Related issues

Fixes #186361.

Related prior fix for the non-separated removeAllItems variant: #176452 (issue #176362).

Test plan

  • Added a regression test in packages/flutter/test/widgets/animated_list_test.dart that reproduces the assertion on master and passes with this change: starts a 10s removal on a non-last item, pumps part of the animation, then removes the new last item, and asserts no exception is thrown.
  • flutter test packages/flutter/test/widgets/animated_list_test.dart packages/flutter/test/widgets/animated_grid_test.dart — 34 tests pass, including the existing comprehensive AnimatedList.separated test and the removeAllItems regression test from Fix(AnimatedScrollView): exclude outgoing items in removeAllItems #176452.
  • dart analyze on the touched files: no issues.
  • Manually verified with the reproducer from the issue: before the fix the assertion fires in the debug console; after the fix the second removal animates out cleanly.

AI disclosure

Written with assistance from Claude Code (Anthropic). I reviewed every change, ran the tests, and verified the fix manually before submitting.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label May 12, 2026
@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels May 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request fixes an assertion failure in AnimatedList.separated that occurred when removing the last item while a previous removal animation was still active. The fix involves introducing _outgoingItemsCount to accurately determine the current visible item count. A regression test has been added to cover this scenario. Feedback was provided to correct a spelling variation in a comment to match the American English requirement of the style guide.

Comment thread packages/flutter/lib/src/widgets/animated_scroll_view.dart Outdated
@mbcorona mbcorona self-assigned this May 12, 2026
@mbcorona mbcorona force-pushed the fix-animated-list-separated-186361 branch from 2fdcd6f to 8871713 Compare May 12, 2026 04:05
@github-actions github-actions Bot removed the CICD Run CI/CD label May 12, 2026
navaronbracke
navaronbracke previously approved these changes May 12, 2026

@navaronbracke navaronbracke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with nit

Comment thread packages/flutter/test/widgets/animated_list_test.dart
// All removal animations finished and the surviving item is alone.
expect(find.text('removing item 0'), findsNothing);
expect(find.text('removing item 2'), findsNothing);
expect(find.text('item 0'), findsOneWidget);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here for asserting on the two items that are now gone

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @navaronbracke , asserts were added in both places.

@mbcorona mbcorona force-pushed the fix-animated-list-separated-186361 branch from 8871713 to 6e4a7c7 Compare May 12, 2026 13:39
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label May 12, 2026
@QuncCccccc QuncCccccc self-requested a review May 14, 2026 19:00
@mbcorona mbcorona requested a review from navaronbracke May 20, 2026 12:45
@mbcorona mbcorona added this pull request to the merge queue May 20, 2026
Merged via the queue into flutter:master with commit 31f6edd May 20, 2026
85 checks passed
@mbcorona mbcorona deleted the fix-animated-list-separated-186361 branch May 20, 2026 16:51
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request May 20, 2026
flutter/flutter@259aeae...e03b91f

2026-05-20 engine-flutter-autoroll@skia.org Roll Packages from ade10ca to 1dfbada (6 revisions) (flutter/flutter#186811)
2026-05-20 brunocorona.alcantar@gmail.com Fix AnimatedList.separated assert when removing last item mid-removal… (flutter/flutter#186389)
2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from d45969a5752e to 5f4f454b9662 (2 revisions) (flutter/flutter#186809)
2026-05-20 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from -F9Ci3Opxt06MixDl... to iKCvaD58jKStYGla0... (flutter/flutter#186796)
2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 19ad9707e5c6 to d45969a5752e (2 revisions) (flutter/flutter#186792)
2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 3471ebf5af0c to 19ad9707e5c6 (9 revisions) (flutter/flutter#186772)
2026-05-20 mdebbar@google.com [web] Refactor webparagraph painters to separate CK properly (flutter/flutter#186684)
2026-05-19 31859944+LongCatIsLooong@users.noreply.github.com Enable Swift testing in the iOS embedder (flutter/flutter#185712)
2026-05-19 mdebbar@google.com [web] Rename WebParagraph goldens (flutter/flutter#186680)
2026-05-19 engine-flutter-autoroll@skia.org Roll Skia from f1b406860c5e to 3471ebf5af0c (5 revisions) (flutter/flutter#186745)
2026-05-19 154381524+flutteractionsbot@users.noreply.github.com Revert "Ship gen_snapshot for linux-arm64 hosts targeting Android" (flutter/flutter#186693)
2026-05-19 bkonyi@google.com [ Tool ] Remove legacy analytics code (flutter/flutter#184994)
2026-05-19 chingjun@google.com Update Vulkan enum values (flutter/flutter#186694)
2026-05-19 1961493+harryterkelsen@users.noreply.github.com fix(web): Fixes CSS override detection when the browser has a default font size (flutter/flutter#186474)
2026-05-19 30870216+gaaclarke@users.noreply.github.com adds linux impeller hello world integration test (flutter/flutter#186715)
2026-05-19 jason-simmons@users.noreply.github.com Update the list of binaries in the code signing verification test to include new Dart snapshots (flutter/flutter#186754)
2026-05-19 brunocorona.alcantar@gmail.com Make EdgeDraggingAutoScroller respect ScrollPhysics (flutter/flutter#186541)
2026-05-19 bkonyi@google.com [ Widget Preview ] Fix inspector split resize focus loss over WebViews (flutter/flutter#186432)
2026-05-19 engine-flutter-autoroll@skia.org Roll Packages from b9bdd37 to ade10ca (1 revision) (flutter/flutter#186746)
2026-05-19 jason-simmons@users.noreply.github.com Manual Dart roll from 8e30b88e4d5a to 66873d2da857 (flutter/flutter#186690)
2026-05-19 bkonyi@google.com [ Widget Preview ] Improve zoom behavior and add zoom slider (flutter/flutter#186422)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
matthewhendrix pushed a commit to matthewhendrix/flutter that referenced this pull request May 23, 2026
flutter#186389)

## Summary

`AnimatedList.separated.removeItem` used the sliver's total child count
(`_itemsCount`) to decide whether the just-removed item was the last
item in the list. That count still includes children animating out from
earlier `removeItem` calls, so while a previous removal was in flight
the new tail of the visible list was misclassified as a middle item. The
separator removal then targeted the wrong index, and once the item being
removed was added to `_outgoingItems` the next `_indexToItemIndex`
lookup ran past `_itemsCount` and tripped the `itemIndex >= 0 &&
itemIndex < _itemsCount` assertion inside
`_SliverAnimatedMultiBoxAdaptorState.removeItem`.

The fix captures `_itemsCount - _outgoingItems.length` before mutating
the sliver and uses that visible count for both the "are there enough
items to have a separator?" and "is this the new last item?" checks.
When no removal is in flight `_outgoingItems` is empty and behavior is
unchanged, so existing `.separated` coverage and the `removeAllItems`
regression test from flutter#176452 continue to pass.

## Related issues

Fixes flutter#186361.

Related prior fix for the non-separated `removeAllItems` variant:
flutter#176452 (issue flutter#176362).

## Test plan

- [x] Added a regression test in
`packages/flutter/test/widgets/animated_list_test.dart` that reproduces
the assertion on master and passes with this change: starts a 10s
removal on a non-last item, pumps part of the animation, then removes
the new last item, and asserts no exception is thrown.
- [x] `flutter test
packages/flutter/test/widgets/animated_list_test.dart
packages/flutter/test/widgets/animated_grid_test.dart` — 34 tests pass,
including the existing comprehensive `AnimatedList.separated` test and
the `removeAllItems` regression test from flutter#176452.
- [x] `dart analyze` on the touched files: no issues.
- [x] Manually verified with the reproducer from the issue: before the
fix the assertion fires in the debug console; after the fix the second
removal animates out cleanly.

## AI disclosure

Written with assistance from Claude Code (Anthropic). I reviewed every
change, ran the tests, and verified the fix manually before submitting.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
…r#11747)

flutter/flutter@259aeae...e03b91f

2026-05-20 engine-flutter-autoroll@skia.org Roll Packages from ade10ca to 1dfbada (6 revisions) (flutter/flutter#186811)
2026-05-20 brunocorona.alcantar@gmail.com Fix AnimatedList.separated assert when removing last item mid-removal… (flutter/flutter#186389)
2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from d45969a5752e to 5f4f454b9662 (2 revisions) (flutter/flutter#186809)
2026-05-20 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from -F9Ci3Opxt06MixDl... to iKCvaD58jKStYGla0... (flutter/flutter#186796)
2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 19ad9707e5c6 to d45969a5752e (2 revisions) (flutter/flutter#186792)
2026-05-20 engine-flutter-autoroll@skia.org Roll Skia from 3471ebf5af0c to 19ad9707e5c6 (9 revisions) (flutter/flutter#186772)
2026-05-20 mdebbar@google.com [web] Refactor webparagraph painters to separate CK properly (flutter/flutter#186684)
2026-05-19 31859944+LongCatIsLooong@users.noreply.github.com Enable Swift testing in the iOS embedder (flutter/flutter#185712)
2026-05-19 mdebbar@google.com [web] Rename WebParagraph goldens (flutter/flutter#186680)
2026-05-19 engine-flutter-autoroll@skia.org Roll Skia from f1b406860c5e to 3471ebf5af0c (5 revisions) (flutter/flutter#186745)
2026-05-19 154381524+flutteractionsbot@users.noreply.github.com Revert "Ship gen_snapshot for linux-arm64 hosts targeting Android" (flutter/flutter#186693)
2026-05-19 bkonyi@google.com [ Tool ] Remove legacy analytics code (flutter/flutter#184994)
2026-05-19 chingjun@google.com Update Vulkan enum values (flutter/flutter#186694)
2026-05-19 1961493+harryterkelsen@users.noreply.github.com fix(web): Fixes CSS override detection when the browser has a default font size (flutter/flutter#186474)
2026-05-19 30870216+gaaclarke@users.noreply.github.com adds linux impeller hello world integration test (flutter/flutter#186715)
2026-05-19 jason-simmons@users.noreply.github.com Update the list of binaries in the code signing verification test to include new Dart snapshots (flutter/flutter#186754)
2026-05-19 brunocorona.alcantar@gmail.com Make EdgeDraggingAutoScroller respect ScrollPhysics (flutter/flutter#186541)
2026-05-19 bkonyi@google.com [ Widget Preview ] Fix inspector split resize focus loss over WebViews (flutter/flutter#186432)
2026-05-19 engine-flutter-autoroll@skia.org Roll Packages from b9bdd37 to ade10ca (1 revision) (flutter/flutter#186746)
2026-05-19 jason-simmons@users.noreply.github.com Manual Dart roll from 8e30b88e4d5a to 66873d2da857 (flutter/flutter#186690)
2026-05-19 bkonyi@google.com [ Widget Preview ] Improve zoom behavior and add zoom slider (flutter/flutter#186422)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
via-guy pushed a commit to via-guy/flutter that referenced this pull request Jun 26, 2026
flutter#186389)

## Summary

`AnimatedList.separated.removeItem` used the sliver's total child count
(`_itemsCount`) to decide whether the just-removed item was the last
item in the list. That count still includes children animating out from
earlier `removeItem` calls, so while a previous removal was in flight
the new tail of the visible list was misclassified as a middle item. The
separator removal then targeted the wrong index, and once the item being
removed was added to `_outgoingItems` the next `_indexToItemIndex`
lookup ran past `_itemsCount` and tripped the `itemIndex >= 0 &&
itemIndex < _itemsCount` assertion inside
`_SliverAnimatedMultiBoxAdaptorState.removeItem`.

The fix captures `_itemsCount - _outgoingItems.length` before mutating
the sliver and uses that visible count for both the "are there enough
items to have a separator?" and "is this the new last item?" checks.
When no removal is in flight `_outgoingItems` is empty and behavior is
unchanged, so existing `.separated` coverage and the `removeAllItems`
regression test from flutter#176452 continue to pass.

## Related issues

Fixes flutter#186361.

Related prior fix for the non-separated `removeAllItems` variant:
flutter#176452 (issue flutter#176362).

## Test plan

- [x] Added a regression test in
`packages/flutter/test/widgets/animated_list_test.dart` that reproduces
the assertion on master and passes with this change: starts a 10s
removal on a non-last item, pumps part of the animation, then removes
the new last item, and asserts no exception is thrown.
- [x] `flutter test
packages/flutter/test/widgets/animated_list_test.dart
packages/flutter/test/widgets/animated_grid_test.dart` — 34 tests pass,
including the existing comprehensive `AnimatedList.separated` test and
the `removeAllItems` regression test from flutter#176452.
- [x] `dart analyze` on the touched files: no issues.
- [x] Manually verified with the reproducer from the issue: before the
fix the assertion fires in the debug console; after the fix the second
removal animates out cleanly.

## AI disclosure

Written with assistance from Claude Code (Anthropic). I reviewed every
change, ran the tests, and verified the fix manually before submitting.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AnimatedList throws 'itemIndex >= 0 && itemIndex < _itemsCount': is not true. when removing last item while another item is being removed at the moment

3 participants