Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto report puzzles with multiple solutions #16293

Merged
merged 26 commits into from
Nov 5, 2024

Conversation

kraktus
Copy link
Member

@kraktus kraktus commented Oct 30, 2024

When users ceval detect multiple good moves after a certain depth, offer to sent it to zulip like that
[content-site:puzzle reports] [Kraktus](https://lichess.org/@/Kraktus?mod&notes) reported [FJtVd](https://lichess.org/training/FJtVd) because after move 24. Kh2, depth 23, there're multiple solutions, pvs f8f5: -592, a8b8: -428

Currently the users need to reach us via the feedback forum/discord and we need to check back, which is time consuming for them and us. Hopefully making it integrated will allow to flag the few faulty puzzles much earlier and with better coverage. cc @TBestLittleHelper for what you'd think.

Screenshot 2024-10-30 at 09 54 19-edited
(The dialog need working)
The Hide this popup for a week is very important part, that way in case it is released with too many false positives, people can disable it easily, but once those tweaks are fixed, they will be able to contribute back after a while.

The code is pretty much WIP for the time being, but I want to make sure this would be accepted before going forward.

Testing data is a faulty puzzle recently reported on discord:

faulty-puzzle-check.json

{"_id":"FJtVd","gameId":"2ceTqBfV","fen":"r4q1k/pQp4p/3p2p1/8/2P2n2/1P3P1P/P4P2/R4RK1 w - - 1 24","themes":["crushing","defensiveMove","sacrifice","long","endgame"],"glicko":{"r":2229.5228893165554,"d":76.8391894980614,"v":0.09000724648992095},"plays":783,"vote":0.9432234168052673,"vu":1061,"vd":31,"line":"g1h2 f8f5 b7a8 h8g7 a8c8 f5c8","cp":629,"users":["bobinsquares","azel1117"]}

mongoimport -d lichess -c puzzle2_puzzle faulty-puzzle-check.json

game-associated-faulty-puzzle-check.json

{"_id":"2ceTqBfV","is":"Kw3Mw5Wv","us":["bobinsquares","azel1117"],"p0":{"e":1510,"d":-5},"p1":{"e":1525,"d":6},"s":31,"t":58,"c":{"$binary":{"base64":"BQAAahsAMnQ=","subType":"00"}},"cw":{"$binary":{"base64":"dqEzRktFka6yA3PEfFtbA4Xo4/h4TIbzkOwfpIRv8aoPDBjscREo","subType":"00"}},"cb":{"$binary":{"base64":"cYM8kRMpHlYB6ELhex0PA6DmcSkyoBcIiG4RB+CxGsZI0YA=","subType":"00"}},"ra":true,"ca":{"$date":"2022-07-24T08:16:54.203Z"},"ua":{"$date":"2022-07-24T08:23:42.669Z"},"so":12,"hp":{"$binary":{"base64":"FCN96c8/MKvnrS2Y60EwK90Gv3B4vIZbpPthhSw4nsA=","subType":"00"}},"w":false,"wid":"azel1117","an":true}

mongoimport -d lichess -c game5 game-associated-faulty-puzzle-check.json

then localhost:9663/training/FJtVd

conf/base.conf Outdated Show resolved Hide resolved
modules/puzzle/src/main/PuzzleApi.scala Outdated Show resolved Hide resolved
ui/puzzle/src/ctrl.ts Outdated Show resolved Hide resolved
@TBestLittleHelper
Copy link
Contributor

I like the idea

Should NOT use `show: modal`, otherwise the dialog promise only resolve once closed
@kraktus kraktus marked this pull request as ready for review November 2, 2024 17:19
…one in lichess-puzzler v49

Relevant extracts from the code:

```py3

pair_limit = chess.engine.Limit(depth = 50, time = 30, nodes = 25_000_000)

// ...

    # is pair.best the only continuation?
    def is_valid_attack(self, pair: NextMovePair) -> bool:
        return (
            pair.second is None or
            self.is_valid_mate_in_one(pair) or
            win_chances(pair.best.score) > win_chances(pair.second.score) + 0.7
        )

```
@kraktus
Copy link
Member Author

kraktus commented Nov 2, 2024

This is now feature-complete, and would gladly take some reviews.

The code (especially ts dialog) is harrowing, hoping for some help there.
There're plenty of console.log that will be cleaned afterwards.

How exactly to store the reports is also left undecided, I've went for separate collection for now so it's much easier to clean the db in case it does not make it, but there're several other options.

@kraktus kraktus requested a review from ornicar November 2, 2024 19:35
a reported puzzle should be disabled in less than a week.
we'll reassess this if we see too many duplicated reports
in zulip.
@ornicar
Copy link
Collaborator

ornicar commented Nov 4, 2024

It's a lot of code added to ctrl.ts. I think it should all be in a different report.ts file and class.

You could be using a simple <input type="checkbox"> instead of a cmn toggle to reduce complexity.

@kraktus
Copy link
Member Author

kraktus commented Nov 4, 2024

Moved everything to report.ts

how it looks
Screenshot 2024-11-04 at 14 40 02
since the toggle button styling is just 4 lines instead of one I kept it 🤷

@@ -37,6 +37,8 @@ final class PuzzleApi(
def setIssue(id: PuzzleId, issue: String): Fu[Boolean] =
colls.puzzle(_.updateField($id(id), Puzzle.BSONFields.issue, issue).map(_.n > 0))

val reportDedup = scalalib.cache.OnceEvery[PuzzleId](7.days)
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the cache is in heap in practice it'll more likely mean at most once per restart, but should still be good enough

Copy link
Member Author

@kraktus kraktus Nov 4, 2024

Choose a reason for hiding this comment

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

It also does not check if the puzzle id really exist anymore, but the potential for abuse is low

@schlawg
Copy link
Collaborator

schlawg commented Nov 4, 2024

It's a great idea to fuzz our puzzles with user browsers. I would not even hate a completely automated solution where javascript just auto reports them to the server in the background. But I know you're quite sensitive to tracking concerns and this dialog is 1000x better than the nothing we have now.

@ornicar ornicar merged commit 67de06b into lichess-org:master Nov 5, 2024
5 checks passed
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.

4 participants