Skip to content

Prevent persisting and clean up empty analyses, and disable Analyze for zero-move games#667

Draft
Copilot wants to merge 18 commits into
masterfrom
copilot/prevent-empty-analysis-persistence
Draft

Prevent persisting and clean up empty analyses, and disable Analyze for zero-move games#667
Copilot wants to merge 18 commits into
masterfrom
copilot/prevent-empty-analysis-persistence

Conversation

Copilot AI commented May 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Inspect the current branch state and determine whether the PR has merge conflicts with master
  • Fetch the latest remote history and merge origin/master into this branch if needed
  • Resolve any merge conflicts while preserving the PR changes
  • Run targeted validation after conflict resolution
  • Run final review/security validation and reply to the new PR comment

Copilot AI linked an issue May 17, 2026 that may be closed by this pull request
Prevent empty analysis from being persisted #114
Open
Copilot AI changed the title [WIP] Fix issue preventing empty analysis from being persisted Prevent persisting empty analyses and disable Analyze for zero-move games May 17, 2026
Copilot AI requested a review from benckx May 17, 2026 19:09
Copilot finished work on behalf of benckx May 17, 2026 19:09
benckx
benckx reviewed May 17, 2026
View reviewed changes
Comment thread ...-service-layer/src/test/kotlin/io/elephantchess/servicelayer/services/AnalysisServiceTest.kt Outdated

@benckx benckx May 17, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot remove that test

Copilot AI May 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed in 402b12a.

benckx
benckx requested changes May 17, 2026
View reviewed changes

@benckx benckx left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot we should grey out the "save analysis" button in the analysis board when there is no move to save

Copilot finished work on behalf of benckx May 17, 2026 21:01
Copilot AI requested a review from benckx May 17, 2026 21:01

Copilot AI commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

@copilot we should grey out the "save analysis" button in the analysis board when there is no move to save

Done in 0e3f19a. The Save Analysis button is now greyed out and disabled whenever the move tree is empty, including after clearing/import flows. Screenshot captured locally: /tmp/save-analysis-button-disabled.png.

Copilot finished work on behalf of benckx May 17, 2026 21:09
@benckx

benckx commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved in 6508dc5. The merge conflicts are fixed and the branch now includes current master while preserving the empty-analysis/zero-move gating changes. Existing UI screenshot for the save-analysis state: /tmp/save-analysis-button-disabled.png.

Copilot finished work on behalf of benckx May 25, 2026 08:59
@benckx

benckx commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved in d725496. The branch now includes the latest master, the follow-up merge conflicts are cleared, and the PR behavior is preserved. Screenshot reference: /tmp/save-analysis-button-disabled.png.

Copilot finished work on behalf of benckx May 25, 2026 09:09
Copilot finished work on behalf of benckx May 25, 2026 09:38
@benckx

benckx commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot Add a changeset in liquibase to remove empty analysis and update changelog with today date and issue number

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Add a changeset in liquibase to remove empty analysis and update changelog with today date and issue number

Done in 32a7fe7. I added the Liquibase changeset to delete already-persisted empty analyses, regenerated the derived changelog, and added today’s changelog entry linked from PR #667. Screenshot: https://github.com/user-attachments/assets/70e30dda-fce2-410f-a99c-dc6a7abc98fd

Copilot AI changed the title Prevent persisting empty analyses and disable Analyze for zero-move games Prevent persisting and clean up empty analyses, and disable Analyze for zero-move games May 25, 2026
Copilot finished work on behalf of benckx May 25, 2026 10:26
@benckx

benckx commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot Also update validateAnalysisShouldStart in GameDataService

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Also update validateAnalysisShouldStart in GameDataService

Done in 5e26b26. validateAnalysisShouldStart now rejects zero-move games with the same backend guard, and I added a regression test for startGameAnalysis on a zero-move PvP game.

Copilot finished work on behalf of benckx May 25, 2026 10:40
benckx
benckx reviewed May 25, 2026
View reviewed changes
Comment thread ...-service-layer/src/test/kotlin/io/elephantchess/servicelayer/services/GameDataServiceTest.kt Outdated
@BeforeTest
fun before() = runTest {
userId = UserId(AUTHENTICATED, signUpTestUser().second)
gameDataService = GameDataService(

@benckx benckx May 25, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot you can just inject GameDataService, no?

Copilot AI May 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in be28620. The test now injects GameDataService directly; I moved the mocked EnginePool override into the shared ServiceTest Koin bootstrap so the service can be wired normally without starting real engines.

Copilot finished work on behalf of benckx May 25, 2026 14:44
Copilot AI requested a review from benckx May 25, 2026 14:44
@benckx

benckx commented May 25, 2026

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

Copilot AI and others added 2 commits May 25, 2026 15:24
@benckx
Merge origin/master into prevent-empty-analysis-persistence
47d4ee9 
Co-authored-by: benckx <8626080+benckx@users.noreply.github.com>
@benckx
Fix post-merge review notes
4ac4421 
Agent-Logs-Url: https://github.com/benckx/elephantchess/sessions/b546fa68-d8aa-40e3-953d-5ce2619cd663

Co-authored-by: benckx <8626080+benckx@users.noreply.github.com>

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved in 4ac4421. The branch now includes the latest master, the changelog conflict is resolved while preserving this PR’s entry, and the branch revalidated with :webapp-service-layer:test passing.

Copilot finished work on behalf of benckx May 25, 2026 15:32
@benckx

benckx commented Jun 7, 2026

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

Copilot finished work on behalf of benckx June 7, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benckx benckx Awaiting requested review from benckx

Assignees

@benckx benckx

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Prevent empty analysis from being persisted

2 participants

@benckx