Fix a bug in catalog update and add catalog from MAS by wyz2368 · Pull Request #974 · gambitproject/gambit

wyz2368 · 2026-07-05T05:27:13Z

The bug is that build_support/catalog/update.py does not build a fresh catalog from Git directly. It calls:
gbt.catalog.games(include_descriptions=True), so it uses whatever pygambit your current python imports. If that Python has an old installed/editable pygambit, or an old build/lib.../pygambit/catalog_data, it will “remember” deleted games. The remembering is severe: even if one deletes the gambit locally and git clone a new one, reinstalling will still remember old games. Now this has been fixed by updating update.py.
After fixing the bug, EFGs from MAS and some original layouts were added.
I will close Add EFGs from Shoham and Leyton-Brown (2008) #936.

wyz2368 · 2026-07-05T05:38:29Z

@rahulsavani I reset my laptop to arm successfully. Please help review this.

Copilot

Pull request overview

This PR fixes build_support/catalog/update.py so catalog updates are generated from the checked-out catalog/ directory (not whatever catalog data happens to be bundled with / remembered by the currently-imported pygambit), and expands the catalog with additional EFGs plus DrawTree “original layout” .ef variants sourced from Multiagent Systems (Shoham & Leyton-Brown, 2008) and MAS-related material.

Changes:

Force pygambit.catalog to read from the repo’s catalog/ directory during catalog generation (and restore the previous resource afterward).
Add tests to ensure the catalog resource override is applied and restored correctly.
Add new Shoham & Leyton-Brown (2008) games + layout variants, and update bibliography/hierarchy metadata accordingly.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
doc/references.bib	Adds textbook categorization metadata for the Shoham & Leyton-Brown (2008) reference.
build_support/catalog/update.py	Introduces a scoped catalog-resource override to ensure catalog generation reads from the checked-out catalog.
build_support/catalog/test_update.py	Adds tests validating requested catalog-dir selection and restoration of the previous resource.
build_support/catalog/catalog.am	Includes newly added catalog game files and curated `.ef` layout variants in distribution lists.
build_support/catalog/catalog_hierarchy.yaml	Adds a hierarchy label entry for `books/shohambrown2008`.
catalog/journals/mor/vonstengelforges2008/fig1__Original_Layout.ef	Adds a curated DrawTree `.ef` “original layout” variant for the MAS figure.
catalog/books/shohambrown2008/fig6_8.efg	Adds the EFG transcription for Fig 6.8.
catalog/books/shohambrown2008/fig6_8__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 6.8.
catalog/books/shohambrown2008/fig6_2.efg	Adds the EFG transcription for Fig 6.2.
catalog/books/shohambrown2008/fig6_2__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 6.2.
catalog/books/shohambrown2008/fig5_9.efg	Adds the EFG transcription for Fig 5.9.
catalog/books/shohambrown2008/fig5_9__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 5.9.
catalog/books/shohambrown2008/fig5_2.efg	Adds the EFG transcription for Fig 5.2.
catalog/books/shohambrown2008/fig5_2__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 5.2.
catalog/books/shohambrown2008/fig5_15.efg	Adds the EFG transcription for Fig 5.15.
catalog/books/shohambrown2008/fig5_15__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 5.15.
catalog/books/shohambrown2008/fig5_12.efg	Adds the EFG transcription for Fig 5.12.
catalog/books/shohambrown2008/fig5_12__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 5.12.
catalog/books/shohambrown2008/fig5_11.efg	Adds the EFG transcription for Fig 5.11.
catalog/books/shohambrown2008/fig5_11__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 5.11.
catalog/books/shohambrown2008/fig5_10.efg	Adds the EFG transcription for Fig 5.10.
catalog/books/shohambrown2008/fig5_10__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 5.10.
catalog/books/shohambrown2008/fig5_1.efg	Adds the EFG transcription for Fig 5.1.
catalog/books/shohambrown2008/fig5_1__original_layout.ef	Adds the DrawTree “original layout” variant for Fig 5.1.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+This is the centipede game. In this game two players alternate in making decisions, at each
+turn choosing between going down and ending the game or going across and
+continuing it.
+This example is usd to explain the criticisms of backward induction for finding subgame-perfect equilibrium.


+
+"
+Figure 5.12 from :cite:p:`ShoLeyB08`.
+A game with imperfect recall. 


+
+"
+Figure 5.15 from :cite:p:`ShoLeyB08`.
+A game with imperfect information.  


tturocy · 2026-07-05T06:50:38Z

+level 0 node 1 player 1 
+level 2 node 1 xshift -3.58 from 0,1 move C 
+level 2 node 2 xshift 3.58 from 0,1 move D 


@wyz2368 Do you have the pre-commit hooks installed? I would think these trailing whitespace problems would automatically be cleaned up by that.

@tturocy I see. Let me install that. Thanks for mentioning this.

@rahulsavani Fixed.

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 10 comments.

rahulsavani · 2026-07-06T10:44:21Z

@wyz2368 I suspect that the pre-commit hooks are not setup to look at .ef files. You could perhaps look at that, or leave it to @tturocy and me, as you wish, just let us know.

As for this PR, to get this merged right away, I suggest you just accept the changes as suggested by co-polit, via the github interface, unless there is some reason not to?

rahulsavani · 2026-07-06T10:45:01Z

@wyz2368 It is a separate question how the trailing white space got included in these .ef files -- were you doing manual edits?

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

wyz2368 · 2026-07-06T14:54:58Z

@rahulsavani

@wyz2368 It is a separate question how the trailing white space got included in these .ef files -- were you doing manual edits?

I downloaded the .ef files for Chapter 5 MAS from gametheoryexplorer after I created them. For Chapter 6, I used ChatGPT to create the .ef files, which do not contain the trailing whitespaces.

@wyz2368 I suspect that the pre-commit hooks are not setup to look at .ef files. You could perhaps look at that, or leave it to @tturocy and me, as you wish, just let us know.

I installed pre-commit hooks using .pre-commit-config.yaml in gambit project and then scanned all files. But probably it did not scan the ef files for some reasons. I can figure that out and probably update the .pre-commit-config.yaml.

rahulsavani · 2026-07-09T14:11:33Z

I installed pre-commit hooks using .pre-commit-config.yaml in gambit project and then scanned all files. But probably it did not scan the ef files for some reasons. I can figure that out and probably update the .pre-commit-config.yaml.

Yes, if we can get it working on .ef files too that would be good.

tturocy · 2026-07-09T14:26:39Z

I installed pre-commit hooks using .pre-commit-config.yaml in gambit project and then scanned all files. But probably it did not scan the ef files for some reasons. I can figure that out and probably update the .pre-commit-config.yaml.

Yes, if we can get it working on .ef files too that would be good.

As far as I know it should work on .ef files - it applies to all text files, there is no filtering done on extensions.

tturocy · 2026-07-09T14:37:54Z

I installed pre-commit hooks using .pre-commit-config.yaml in gambit project and then scanned all files. But probably it did not scan the ef files for some reasons. I can figure that out and probably update the .pre-commit-config.yaml.

Yes, if we can get it working on .ef files too that would be good.

As far as I know it should work on .ef files - it applies to all text files, there is no filtering done on extensions.

I just cloned this branch down and ran

pre-commit run --all-files

on it. It found and fixed the trailing whitespace:

Fixing catalog/books/shohambrown2008/fig5_9__original_layout.ef
Fixing catalog/books/shohambrown2008/fig5_1__original_layout.ef
Fixing catalog/books/shohambrown2008/fig6_2__original_layout.ef
Fixing catalog/books/shohambrown2008/fig5_10__original_layout.ef
Fixing catalog/books/shohambrown2008/fig5_2__original_layout.ef

I haven't committed and pushed this so @wyz2368 can test it - but it looks like pre-commit is possibly not running?

wyz2368 · 2026-07-09T14:56:45Z

@tturocy This is interesting. I got the follow message after running pre-commit run --all-files. It says the trailing whitespace passed but ruff failed. I will investigate why.

(gambit_env) Yongzhaos-MacBook-Pro-14:shohambrown2008 wangyzh$ pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
ruff (legacy alias)......................................................Failed
- hook id: ruff
- exit code: 1

SIM300 [*] Yoda condition detected
   --> build_support/catalog/test_update.py:176:16
    |
175 |         assert calls == [(checkout_dir, {"include_descriptions": True})]
176 |         assert update.gbt.catalog._CATALOG_RESOURCE == stale_dir
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
177 |         assert list(df["Game"]) == ["checkout/game1"]
    |
help: Rewrite as `stale_dir == update.gbt.catalog._CATALOG_RESOURCE`

SIM300 [*] Yoda condition detected
   --> build_support/catalog/test_update.py:198:16
    |
197 |         assert observed == [checkout_dir]
198 |         assert update.gbt.catalog._CATALOG_RESOURCE == stale_dir
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
help: Rewrite as `stale_dir == update.gbt.catalog._CATALOG_RESOURCE`

SIM117 [*] Use a single `with` statement with multiple contexts instead of nested `with` statements
   --> build_support/catalog/update.py:347:5
    |
345 |       labels = load_hierarchy_labels()
346 |       tree = _build_slug_tree(df)
347 | /     with _using_catalog_dir(catalog_dir):
348 | |         with open(rst_path, "w", encoding="utf-8") as f:
    | |________________________________________________________^
349 |               _write_tree_level(
350 |                   f, tree, "", labels, catalog_dir, indent="", regenerate_images=regenerate_images
    |
help: Combine `with` statements

Found 3 errors.
[*] 3 fixable with the `--fix` option.

flake8...................................................................Passed
cython-lint..............................................................Passed
double-quote Cython strings..............................................Passed
clang-format.............................................................Passed

wyz2368 · 2026-07-09T15:00:49Z

@rahulsavani I might have done something with accepting Co-pilot's suggestions since I only see the trailing whitespace fixed in one .ef file. Can you run co-pilot again? I will search for how to accept all suggestions at a time.

tturocy · 2026-07-09T15:01:12Z

Don't worry about the ruff failure, that is something different which I'll deal with in due course.

Are you sure what you have locally is what's pushed onto GitHub?

… into fix/catalog-update-local-source

wyz2368 · 2026-07-09T15:13:15Z

@tturocy Good catch! Maybe the trailing whitespace was removed already when I tested pre-commit. That's why I cannot see the errors again. Pushed again.

rahulsavani

Apart from fixing the trailing whitespace issue, I have only two things I recommend to change:

fig6_8: here the infosets overal in both the default and original layouts:

Please fix these by changing the params and or .ef file to remove the overlap.

"Please read carefully about the definition of perfect recall, which infers three types of imperfect recall." Let's remove this sentence from the description of fig5_12. "Infers" does not work as the very hear, but also, telling the user to "read carefully" is out of style with other descriptions. If you do want to you could change the first sentence to "A game with imperfect recall, in particular absent-mindedness".

tturocy · 2026-07-10T06:26:26Z

@wyz2368 Can you please make this two PRs, one with the proposed fix to the catalog update and a separate one with the games? We should keep commits logically separate.

wyz2368 · 2026-07-10T09:44:57Z

@tturocy I have pushed a fixed version to this PR, but I will further create two PRs that separate the current PR.

Now I have created a fix_catalog_update branch from master and submitted a new PR for fixing the bug in update.py. Since adding EFGs to catalog depends on the fixed update.py, what is the best practice to create the second PR for adding EFGs? Should I further branch the fix_catalog_update branch, add the EFGs, build everything, and create the second PR?

tturocy · 2026-07-10T10:17:49Z

Adding games didn't require the other PR - we've been adding games for ages with no problem. But it's now been merged so basing off of master is fine.

wyz2368 · 2026-07-10T10:26:37Z

Adding games didn't require the other PR - we've been adding games for ages with no problem. But it's now been merged so basing off of master is fine.

@tturocy The development guide for catalog says: 6. Submit a pull request to GitHub with all changes. So what are the steps for adding games without creating a PR? Maybe I can create a PR to add the games following the guide for now and wait for your later clarification.

tturocy · 2026-07-10T10:39:41Z

What I meant was that the problem you flagged was indeed creating incorrect output in the documentation, but putting the games in the repository per se was not affected - you just need to stick the files in the prescribed places and update the manifest file used by Makefile.am. Then the games are "in the catalog" even if the documentation script was keeping on including deleted games!

tturocy · 2026-07-11T11:20:18Z

Separated into two other PRs.

wyz2368 added 3 commits July 5, 2026 13:01

Fix bug in update.py

3176353

Add MAS and original layouts

bd34dec

Add MAS and original layouts

6443a1c

wyz2368 mentioned this pull request Jul 5, 2026

Add EFGs from Shoham and Leyton-Brown (2008) #936

Closed

rahulsavani requested a review from Copilot July 5, 2026 05:50

Copilot started reviewing on behalf of rahulsavani July 5, 2026 05:50 View session

Copilot AI reviewed Jul 5, 2026

View reviewed changes

Fix trailing whitespaces and typos

aa2d033

rahulsavani requested a review from Copilot July 6, 2026 10:34

Copilot started reviewing on behalf of rahulsavani July 6, 2026 10:34 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Fix trailing whitespaces found by co-pilot.

d54c239

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

wyz2368 added 2 commits July 9, 2026 11:10

Fix trailing whitespaces and typos

aa9adc7

Merge remote-tracking branch 'origin/fix/catalog-update-local-source'…

d132512

… into fix/catalog-update-local-source

rahulsavani self-requested a review July 9, 2026 18:45

rahulsavani requested changes Jul 9, 2026

View reviewed changes

wyz2368 added 2 commits July 10, 2026 05:13

Update layouts

c4b53b6

Fix ruff

ce57268

tturocy closed this Jul 11, 2026

Uh oh!

Conversation

wyz2368 commented Jul 5, 2026

Uh oh!

wyz2368 commented Jul 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

tturocy Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

wyz2368 Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

wyz2368 Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rahulsavani commented Jul 6, 2026

Uh oh!

rahulsavani commented Jul 6, 2026

Uh oh!

wyz2368 commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rahulsavani commented Jul 9, 2026

Uh oh!

tturocy commented Jul 9, 2026

Uh oh!

tturocy commented Jul 9, 2026

Uh oh!

wyz2368 commented Jul 9, 2026

Uh oh!

wyz2368 commented Jul 9, 2026

Uh oh!

tturocy commented Jul 9, 2026

Uh oh!

wyz2368 commented Jul 9, 2026

Uh oh!

rahulsavani left a comment

Choose a reason for hiding this comment

Uh oh!

tturocy commented Jul 10, 2026

Uh oh!

wyz2368 commented Jul 10, 2026

Uh oh!

tturocy commented Jul 10, 2026

Uh oh!

wyz2368 commented Jul 10, 2026

Uh oh!

tturocy commented Jul 10, 2026

Uh oh!

tturocy commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wyz2368 commented Jul 6, 2026 •

edited

Loading