Skip to content

Fix retro reuse existing analysis#2859

Merged
veloce merged 2 commits into
lichess-org:mainfrom
sviluppatoredisoftware:fix-retro-reuse-existing-analysis
Mar 31, 2026
Merged

Fix retro reuse existing analysis#2859
veloce merged 2 commits into
lichess-org:mainfrom
sviluppatoredisoftware:fix-retro-reuse-existing-analysis

Conversation

@sviluppatoredisoftware

@sviluppatoredisoftware sviluppatoredisoftware commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Related to #2564 and #2750

What this changes

This PR tries to fix the case where Learn from your mistakes can get stuck / keep loading after a computer analysis was already requested first.

The issue seems to come from the retro flow attaching too late to analysis progress and potentially trying to request / restart analysis again for the same game.

Changes

  • avoid restarting server analysis if the same game is already being analyzed
  • attach the retro listener earlier
  • reuse an already available analysis progress event for the same game
  • keep tracking the current game even when the server responds with 400 already requested

Why

This should help with the cases described in:

veloce
veloce reviewed Mar 31, 2026
View reviewed changes
Comment thread lib/src/model/analysis/server_analysis_service.dart
// of analyses is reached.
if (e.statusCode == 400) {
debugPrint('Analysis already requested for game $id');
_currentAnalysis.value = id.gameId;

@veloce veloce Mar 31, 2026

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.

Does not look useful: we already assign it in line 96.

veloce
veloce approved these changes Mar 31, 2026
View reviewed changes

@veloce veloce 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.

Thanks!

@veloce veloce merged commit ad05273 into lichess-org:main Mar 31, 2026
1 check passed
This was referenced Apr 9, 2026
Open
Open
@tom-anders tom-anders mentioned this pull request Apr 27, 2026
Merged
tom-anders added a commit to tom-anders/lichess-mobile that referenced this pull request Apr 27, 2026
@tom-anders
refactor: simpler fix for lichess-org#2564 and lichess-org#2750
a8cd68b 
Previous fix was in lichess-org#2859, but this PR fixes the root cause instead of
adding a workaround.

The root cause of this problem was that we check
`_game.serverAnalysis == null` to check if the RetroController needs to
trigger server analysis in case it isn't already running. When the
server analysis was already triggered by the `AnalysisController`, the
problem was that `archivedGameProvider` is not invalidated and still
returns `null` for the server analysis field. By using the
`GameRepository` directly instead, we get a fresh response from the
server which correctly has a non-null `serverAnalysis`.

An alternative fix would be to add
`red.invalidate(archivedGameProvider(gameId))` when a server analysis
request is triggered in the analysis screen, however this leads to the
analysis screen being rebuilt when triggering server analysis, which
causes a short flicker of the screen during the reload.

To verify that everything still works, I successfully tested the
following scenarios:

1) Trigger server analysis in analysis screen and immediately open
   "Learn From Your Mistakes"
   -> The loading bar correctly displays server analysis progress,
      once analysis is finished Learn from your mistakes is available.
2) Trigger server analysis in analysis screen, wait for it to finish
   and only then open "Learn From Your Mistakes"
   -> Learn from your mistakes is available immediately without any
      loading bar.
3) Trigger server analysis, wait for it to finish and then close the
   analysis screen. Open analysis again for the same game and open
  "Learn from your Mistakes".
   -> Learn from your mistakes is available immediately without any
      loading bar.
tom-anders added a commit to tom-anders/lichess-mobile that referenced this pull request Apr 27, 2026
@tom-anders
refactor: simpler fix for lichess-org#2564 and lichess-org#2750
f4f2077 
Previous fix was in lichess-org#2859, but this PR fixes the root cause instead of
adding a workaround.

The root cause of this problem was that we check
`_game.serverAnalysis == null` to check if the RetroController needs to
trigger server analysis in case it isn't already running. When the
server analysis was already triggered by the `AnalysisController`, the
problem was that `archivedGameProvider` is not invalidated and still
returns `null` for the server analysis field. By using the
`GameRepository` directly instead, we get a fresh response from the
server which correctly has a non-null `serverAnalysis`.

An alternative fix would be to add
`red.invalidate(archivedGameProvider(gameId))` when a server analysis
request is triggered in the analysis screen, however this leads to the
analysis screen being rebuilt when triggering server analysis, which
causes a short flicker of the screen during the reload.

To verify that everything still works, I successfully tested the
following scenarios:

1) Trigger server analysis in analysis screen and immediately open
   "Learn From Your Mistakes"
   -> The loading bar correctly displays server analysis progress,
      once analysis is finished Learn from your mistakes is available.
2) Trigger server analysis in analysis screen, wait for it to finish
   and only then open "Learn From Your Mistakes"
   -> Learn from your mistakes is available immediately without any
      loading bar.
3) Trigger server analysis, wait for it to finish and then close the
   analysis screen. Open analysis again for the same game and open
  "Learn from your Mistakes".
   -> Learn from your mistakes is available immediately without any
      loading bar.
tom-anders added a commit to tom-anders/lichess-mobile that referenced this pull request Apr 27, 2026
@tom-anders
refactor: simpler fix for lichess-org#2564 and lichess-org#2750
b9e2f1d 
Previous fix was in lichess-org#2859, but this PR fixes the root cause instead of
adding a workaround.

The root cause of this problem was that we check
`_game.serverAnalysis == null` to check if the RetroController needs to
trigger server analysis in case it isn't already running. When the
server analysis was already triggered by the `AnalysisController`, the
problem was that `archivedGameProvider` is not invalidated and still
returns `null` for the server analysis field. By using the
`GameRepository` directly instead, we get a fresh response from the
server which correctly has a non-null `serverAnalysis`.

An alternative fix would be to add
`red.invalidate(archivedGameProvider(gameId))` when a server analysis
request is triggered in the analysis screen, however this leads to the
analysis screen being rebuilt when triggering server analysis, which
causes a short flicker of the screen during the reload.

To verify that everything still works, I successfully tested the
following scenarios:

1) Trigger server analysis in analysis screen and immediately open
   "Learn From Your Mistakes"
   -> The loading bar correctly displays server analysis progress,
      once analysis is finished Learn from your mistakes is available.
2) Trigger server analysis in analysis screen, wait for it to finish
   and only then open "Learn From Your Mistakes"
   -> Learn from your mistakes is available immediately without any
      loading bar.
3) Trigger server analysis, wait for it to finish and then close the
   analysis screen. Open analysis again for the same game and open
  "Learn from your Mistakes".
   -> Learn from your mistakes is available immediately without any
      loading bar.
tom-anders added a commit to tom-anders/lichess-mobile that referenced this pull request Apr 27, 2026
@tom-anders
refactor: simpler fix for lichess-org#2564 and lichess-org#2750
d3024e6 
Previous fix was in lichess-org#2859, but this PR fixes the root cause instead of
adding a workaround.

The root cause of this problem was that we check
`_game.serverAnalysis == null` to check if the RetroController needs to
trigger server analysis in case it isn't already running. When the
server analysis was already triggered by the `AnalysisController`, the
problem was that `archivedGameProvider` is not invalidated and still
returns `null` for the server analysis field. By using the
`GameRepository` directly instead, we get a fresh response from the
server which correctly has a non-null `serverAnalysis`.

An alternative fix would be to add
`red.invalidate(archivedGameProvider(gameId))` when a server analysis
request is triggered in the analysis screen, however this leads to the
analysis screen being rebuilt when triggering server analysis, which
causes a short flicker of the screen during the reload.

To verify that everything still works, I successfully tested the
following scenarios:

1) Trigger server analysis in analysis screen and immediately open
   "Learn From Your Mistakes"
   -> The loading bar correctly displays server analysis progress,
      once analysis is finished Learn from your mistakes is available.
2) Trigger server analysis in analysis screen, wait for it to finish
   and only then open "Learn From Your Mistakes"
   -> Learn from your mistakes is available immediately without any
      loading bar.
3) Trigger server analysis, wait for it to finish and then close the
   analysis screen. Open analysis again for the same game and open
  "Learn from your Mistakes".
   -> Learn from your mistakes is available immediately without any
      loading bar.
tom-anders added a commit to tom-anders/lichess-mobile that referenced this pull request Apr 27, 2026
@tom-anders
refactor: simpler fix for lichess-org#2564 and lichess-org#2750
14f0046 
Previous fix was in lichess-org#2859, but this PR fixes the root cause instead of
adding a workaround.

The root cause of this problem was that we check
`_game.serverAnalysis == null` to check if the RetroController needs to
trigger server analysis in case it isn't already running. When the
server analysis was already triggered by the `AnalysisController`, the
problem was that `archivedGameProvider` is not invalidated and still
returns `null` for the server analysis field. By using the
`GameRepository` directly instead, we get a fresh response from the
server which correctly has a non-null `serverAnalysis`.

An alternative fix would be to add
`red.invalidate(archivedGameProvider(gameId))` when a server analysis
request is triggered in the analysis screen, however this leads to the
analysis screen being rebuilt when triggering server analysis, which
causes a short flicker of the screen during the reload.

To verify that everything still works, I successfully tested the
following scenarios:

1) Trigger server analysis in analysis screen and immediately open
   "Learn From Your Mistakes"
   -> The loading bar correctly displays server analysis progress,
      once analysis is finished Learn from your mistakes is available.
2) Trigger server analysis in analysis screen, wait for it to finish
   and only then open "Learn From Your Mistakes"
   -> Learn from your mistakes is available immediately without any
      loading bar.
3) Trigger server analysis, wait for it to finish and then close the
   analysis screen. Open analysis again for the same game and open
  "Learn from your Mistakes".
   -> Learn from your mistakes is available immediately without any
      loading bar.
tom-anders added a commit to tom-anders/lichess-mobile that referenced this pull request Apr 27, 2026
@tom-anders
refactor: simpler fix for lichess-org#2564 and lichess-org#2750
0090b7e 
Previous fix was in lichess-org#2859, but this PR fixes the root cause instead of
adding a workaround.

The root cause of this problem was that we check
`_game.serverAnalysis == null` to check if the RetroController needs to
trigger server analysis in case it isn't already running. When the
server analysis was already triggered by the `AnalysisController`, the
problem was that `archivedGameProvider` is not invalidated and still
returns `null` for the server analysis field. By using the
`GameRepository` directly instead, we get a fresh response from the
server which correctly has a non-null `serverAnalysis`.

An alternative fix would be to add
`red.invalidate(archivedGameProvider(gameId))` when a server analysis
request is triggered in the analysis screen, however this leads to the
analysis screen being rebuilt when triggering server analysis, which
causes a short flicker of the screen during the reload.

To verify that everything still works, I successfully tested the
following scenarios:

1) Trigger server analysis in analysis screen and immediately open
   "Learn From Your Mistakes"
   -> The loading bar correctly displays server analysis progress,
      once analysis is finished Learn from your mistakes is available.
2) Trigger server analysis in analysis screen, wait for it to finish
   and only then open "Learn From Your Mistakes"
   -> Learn from your mistakes is available immediately without any
      loading bar.
3) Trigger server analysis, wait for it to finish and then close the
   analysis screen. Open analysis again for the same game and open
  "Learn from your Mistakes".
   -> Learn from your mistakes is available immediately without any
      loading bar.
veloce pushed a commit that referenced this pull request Apr 28, 2026
@tom-anders
refactor: simpler fix for #2564 and #2750 (#3058)
26df922 
Previous fix was in #2859, but this PR fixes the root cause instead of
adding a workaround.

The root cause of this problem was that we check
`_game.serverAnalysis == null` to check if the RetroController needs to
trigger server analysis in case it isn't already running. When the
server analysis was already triggered by the `AnalysisController`, the
problem was that `archivedGameProvider` is not invalidated and still
returns `null` for the server analysis field. By using the
`GameRepository` directly instead, we get a fresh response from the
server which correctly has a non-null `serverAnalysis`.

An alternative fix would be to add
`red.invalidate(archivedGameProvider(gameId))` when a server analysis
request is triggered in the analysis screen, however this leads to the
analysis screen being rebuilt when triggering server analysis, which
causes a short flicker of the screen during the reload.

To verify that everything still works, I successfully tested the
following scenarios:

1) Trigger server analysis in analysis screen and immediately open
   "Learn From Your Mistakes"
   -> The loading bar correctly displays server analysis progress,
      once analysis is finished Learn from your mistakes is available.
2) Trigger server analysis in analysis screen, wait for it to finish
   and only then open "Learn From Your Mistakes"
   -> Learn from your mistakes is available immediately without any
      loading bar.
3) Trigger server analysis, wait for it to finish and then close the
   analysis screen. Open analysis again for the same game and open
  "Learn from your Mistakes".
   -> Learn from your mistakes is available immediately without any
      loading bar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@veloce veloce veloce approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sviluppatoredisoftware @veloce