Skip to content

chore: hygiene sweep for tracked runtime files and dead scripts - #68

Open
VedantMadane wants to merge 1 commit into
kadenstaker:mainfrom
VedantMadane:chore/hygiene-sweep-57
Open

chore: hygiene sweep for tracked runtime files and dead scripts#68
VedantMadane wants to merge 1 commit into
kadenstaker:mainfrom
VedantMadane:chore/hygiene-sweep-57

Conversation

@VedantMadane

Copy link
Copy Markdown

Summary

Addresses the safer parts of #57:

  • Untrack runtime/personal files: polyfish-rs/saved_state.json, cache/projects.json, *.css.bak / game_old.css
  • Delete dead one-shots: resolve_conflicts.py, pull.sh
  • Remove unused original_mcts_zero.rs (no mod / callers)
  • Replace Vite template polyfish-ui/README.md with a short project blurb
  • Expand .gitignore so these stay untracked

Left out of scope (larger/behavior-touching): root package.json dep rewrite, opening-book forcing call-site retirement, broader MCTS dead-code moves.

Test plan

  • Inspected git status / removed paths; no module references original_mcts_zero

Closes #57

Untrack runtime/personal artifacts that dirty git status and risk leaking
local state (saved_state.json, cache/projects.json, css backups). Remove
dead one-shot scripts and the unused original_mcts_zero module. Replace
the polyfish-ui Vite template README with a short project description.
Update .gitignore accordingly.

Closes kadenstaker#57

Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
@kadenstaker

Copy link
Copy Markdown
Owner

Reviewed this. Nine of the eleven deletions are clean - original_mcts_zero.rs isn't declared in ai/mod.rs so the build is unaffected, pull.sh / resolve_conflicts.py have no callers and aren't referenced by CI or scripts/check_cli_contract.py, the four game.css.bak / game_old.css blobs are 0 bytes and unlinked from any index.html, and the new polyfish-ui/README.md claim matches main.rs:187-195.

The one thing I'd change before merge is polyfish-rs/saved_state.json. It's classified here as a runtime/personal artifact, but it's a checked-in test fixture with two live consumers.

1. polyfish-rs/tests/elo_test.rs:11 breaks. The test resolves the fixture off CARGO_MANIFEST_DIR, so it's specifically this tracked file. Reproduced by moving the file aside on the base commit:

thread 'test_mcts_matches_ruin_capture_sequence' panicked at tests/elo_test.rs:12:52:
Failed to load saved_state.json: Os { code: 2, kind: NotFound }
test result: FAILED. 0 passed; 1 failed

#[ignore] keeps CI green, so this only surfaces when someone runs the command the test's own comment documents. The PR doesn't delete elo_test.rs, so it stays in the tree pointing at a file that's gone. The new .gitignore entry (plus the pre-existing polyfish-rs/*.json at line 57) also means the fixture can't be restored without git add -f.

2. POST /api/load regresses into a panicking handler. main.rs:967-972 does read_to_string("saved_state.json").expect(...) followed by an .expect() on the parse, and there's no CatchPanicLayer anywhere in src/. The path is CWD-relative and run-server.sh cds into polyfish-rs/, so it reads exactly the deleted file. Narrower than it sounds: save_game writes the same path, so anyone who has ever hit Save is fine - the broken window is a fresh clone where Load is clicked before Save, which was previously impossible because the file shipped. Startup (main.rs:58-68) degrades gracefully; only this endpoint is affected.

Two ways out. Either keep it tracked with a !polyfish-rs/saved_state.json negation after the ignore rules, or commit to the deletion and also drop elo_test.rs plus make load_game return {"status":"error"} on a missing file. I'd take the first - the .expect() in a request handler is worth fixing either way, but that turns a hygiene PR into a behavior change.

Minor, could ride along in the same push:

  • .gitignore:76 - cache/ has no leading slash, so it matches a cache directory at any depth, not just the root one that held cache/projects.json. /cache/ scopes it correctly.
  • .gitignore:77 - polyfish-rs/saved_state.json is already fully covered by polyfish-rs/*.json at line 57. Worth removing so nobody later assumes deleting just this line is what restores the fixture.
  • CLAUDE.md:89 and README.md:53 still describe original_mcts_zero.rs as live, and CLAUDE.md tells contributors to check whether a change needs to land in it. Since that's the agent instruction file it's worth updating here. (expert_pipeline_audit.md:717 mentions it too, but as a historical note, so that one can stay.)

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

Thanks for this - most of the sweep is good and I'd like to take it. Nine of the eleven deletions verify clean: original_mcts_zero.rs isn't declared in ai/mod.rs, pull.sh and resolve_conflicts.py have no callers in CI or the CLI-contract check, the four CSS blobs are 0 bytes and unlinked, and the README update matches what main.rs actually serves.

Blocking on one file: polyfish-rs/saved_state.json is a checked-in test fixture rather than a runtime artifact. Deleting it fails tests/elo_test.rs (reproduced - it resolves the path off CARGO_MANIFEST_DIR), and it regresses POST /api/load into a panicking handler, since main.rs:970 does read_to_string(...).expect(...) with no CatchPanicLayer in the router. Neither shows up in CI, which is why I'd rather catch it here.

Simplest fix is to keep it tracked with a !polyfish-rs/saved_state.json negation and drop the now-redundant ignore line. Details plus two minor .gitignore / stale-docs notes are in my comment above.

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.

Hygiene sweep: unrunnable root package.json, tracked runtime/personal files, dead scripts and binaries, dead search implementations

2 participants