v0.6.0 minors from the review: config validation panics, rnd() correlation, revocation snapshot leak, stale market-loans, HUD (was) rate, wizard/bench cosmetics

Winnie commented

2026-10-01 15:31:08 +00:00

Owner

Consolidated minor findings from the #118 review (each verified, none ruling-breaking):

Config.validate (config.go:74-97) doesn't check loan_term_min <= loan_term_max → rand.Intn(negative) panics the binary (verified: "panic: invalid argument to Intn", exit 2). Also unchecked: loan_min/loan_max (negative offers corrupt the economy; loan_min=0 makes vault.lisp's cushion divide by zero), bond_face/bond_term (bond_term=0 → divide-by-zero displays), free_float range, negative starts.
rnd() (game.go:260-265) builds a new generator from the same per-turn seed on EVERY call: the die roll and the first loan draw consume the identical 63-bit draw, so the whole loan shelf is a deterministic function of the die. Deterministic across runs (replayable), but the phases' randomness is fully correlated. Seed==0 games also can't be reproduced from the replay's recorded seed.
The revocation phase (main.go:344-361) has no bond snapshot (bondSnapSet false there): a later seat's revoke? hook reads the live market and sees earlier seats' current-turn revocations — the one simultaneity leak in phase 4.
market-loans serves a stale shelf in phases 4-7 (no snapshot, no post-award clear) — brains condition on loans that no longer exist. No leak (all seats see the same data), but the signal is wrong.
HUD "deposit rate (was ...)" uses PrevRates, which the deposit phase overwrites with the current rate — every HUD after phase 2 shows "(was)" equal to the just-set value. The turn-start snapshot works for everything else.
The take-bonds builtin executes its fill immediately at seat order, jumping the size-order queue and draining the market before later declarers fill.
The finance-phase forced borrow from a higher-seat issuer ages the new BondAsset in the same pass while the mirror BondDebt waits a turn — discount income recognized a turn early, face received a turn before repayment (seat-order-dependent, conservation-preserving).
AI faults are turn-lagged: a bank faulting in an early phase keeps acting the rest of the turn; an auction-bid fault dies only next turn's pass.
super.lisp's bankruptcy-bid horizon goes negative when max_turns=0 (the wizard's "0 = unlimited" default) → super never bids at auctions in unlimited-turn games.
Wizard: the "Play as a bank yourself" question is a no-op (answer discarded), and the seed prompt defaults to 0 = random, which a replay can never reproduce.
benchvs prints benchVsGames (40) instead of the env-resolved count; gauntlet play counts double the spec's two seats; runBench's preset order varies (map iteration).
The seat prompt silently spectates on any invalid input (a typo converts a human game into an all-AI game with no warning).
The bond-offer sheet previews the raw typed price/size while placeOffers silently clamps/floors/skips — the preview can disagree with what gets placed.
Dead code and cosmetics: promptRate (unbounded EOF busy-loop if ever wired), the no-op if-branch in the final ranking, TrimPrefix(x, ""), unused humanTakeWant/humanTakeRetired, "or turn 0" for unlimited games, churn-captured % can exceed 100% unlabeled, corpse books retain shared offer pointers after acquisition (double-displayed commitments in the final ranking).

Consolidated minor findings from the #118 review (each verified, none ruling-breaking): 1. Config.validate (config.go:74-97) doesn't check loan_term_min <= loan_term_max → rand.Intn(negative) panics the binary (verified: "panic: invalid argument to Intn", exit 2). Also unchecked: loan_min/loan_max (negative offers corrupt the economy; loan_min=0 makes vault.lisp's cushion divide by zero), bond_face/bond_term (bond_term=0 → divide-by-zero displays), free_float range, negative starts. 2. rnd() (game.go:260-265) builds a new generator from the same per-turn seed on EVERY call: the die roll and the first loan draw consume the identical 63-bit draw, so the whole loan shelf is a deterministic function of the die. Deterministic across runs (replayable), but the phases' randomness is fully correlated. Seed==0 games also can't be reproduced from the replay's recorded seed. 3. The revocation phase (main.go:344-361) has no bond snapshot (bondSnapSet false there): a later seat's revoke? hook reads the live market and sees earlier seats' current-turn revocations — the one simultaneity leak in phase 4. 4. market-loans serves a stale shelf in phases 4-7 (no snapshot, no post-award clear) — brains condition on loans that no longer exist. No leak (all seats see the same data), but the signal is wrong. 5. HUD "deposit rate (was ...)" uses PrevRates, which the deposit phase overwrites with the current rate — every HUD after phase 2 shows "(was)" equal to the just-set value. The turn-start snapshot works for everything else. 6. The take-bonds builtin executes its fill immediately at seat order, jumping the size-order queue and draining the market before later declarers fill. 7. The finance-phase forced borrow from a higher-seat issuer ages the new BondAsset in the same pass while the mirror BondDebt waits a turn — discount income recognized a turn early, face received a turn before repayment (seat-order-dependent, conservation-preserving). 8. AI faults are turn-lagged: a bank faulting in an early phase keeps acting the rest of the turn; an auction-bid fault dies only next turn's pass. 9. super.lisp's bankruptcy-bid horizon goes negative when max_turns=0 (the wizard's "0 = unlimited" default) → super never bids at auctions in unlimited-turn games. 10. Wizard: the "Play as a bank yourself" question is a no-op (answer discarded), and the seed prompt defaults to 0 = random, which a replay can never reproduce. 11. benchvs prints benchVsGames (40) instead of the env-resolved count; gauntlet play counts double the spec's two seats; runBench's preset order varies (map iteration). 12. The seat prompt silently spectates on any invalid input (a typo converts a human game into an all-AI game with no warning). 13. The bond-offer sheet previews the raw typed price/size while placeOffers silently clamps/floors/skips — the preview can disagree with what gets placed. 14. Dead code and cosmetics: promptRate (unbounded EOF busy-loop if ever wired), the no-op if-branch in the final ranking, TrimPrefix(x, ""), unused humanTakeWant/humanTakeRetired, "or turn 0" for unlimited games, churn-captured % can exceed 100% unlabeled, corpse books retain shared offer pointers after acquisition (double-displayed commitments in the final ranking).

Winnie referenced this issue

2026-10-01 15:31:23 +00:00

Massive code review before v0.6.0 #118

Winnie was assigned by robertismo

2026-10-01 15:39:41 +00:00

robertismo commented

2026-10-01 15:58:18 +00:00

@Winnie update?

Winnie referenced this issue from a commit

2026-10-01 15:59:58 +00:00

v0.6.0 minor sweep: config validation, RNG, snapshots, HUD, wizard, benches (Closes #127)

Winnie referenced this issue from a pull request that will close it,

2026-10-01 16:00:16 +00:00

v0.6.0 minor sweep: config validation, RNG, snapshots, HUD, wizard, benches (Closes #127) #136

Winnie commented

2026-10-01 16:00:41 +00:00

Author

Owner

#127 is done and up as PR #136 (branch 127-minors). That was the last open review issue — every one of the nine filed from the #118 review now has a PR:

#119 shore-up math → merged
#120 NaN/Inf inputs → merged
#121 Lisp panic recovery → merged
#122 invisible g.Log events → merged
#123 write-off ordering → merged
#124 revoke CF → merged
#125 replay → merged
#126 builtin clamps → merged
#127 minors → PR #136, ready for review

#127's highlights: config validation now catches the loan-term panic as a clean error (verified live), rnd() is one stream per game so the die and loan shelf no longer share a draw (determinism re-verified byte-identical), the revocation phase gets its simultaneity snapshot, the HUD deposit-rate (was) is fixed, and the bench/wizard/seat-prompt cosmetics are cleaned.

Once you merge #136, all issues are closed and I'll run the single full test suite and ship v0.6.0.

#127 is done and up as PR #136 (branch 127-minors). That was the last open review issue — every one of the nine filed from the #118 review now has a PR: - #119 shore-up math → merged - #120 NaN/Inf inputs → merged - #121 Lisp panic recovery → merged - #122 invisible g.Log events → merged - #123 write-off ordering → merged - #124 revoke CF → merged - #125 replay → merged - #126 builtin clamps → merged - #127 minors → PR #136, ready for review #127's highlights: config validation now catches the loan-term panic as a clean error (verified live), rnd() is one stream per game so the die and loan shelf no longer share a draw (determinism re-verified byte-identical), the revocation phase gets its simultaneity snapshot, the HUD deposit-rate (was) is fixed, and the bench/wizard/seat-prompt cosmetics are cleaned. Once you merge #136, all issues are closed and I'll run the single full test suite and ship v0.6.0.

robertismo closed this issue

2026-10-01 16:00:49 +00:00

robertismo referenced this issue from a commit

2026-10-01 16:00:50 +00:00

Merge pull request 'v0.6.0 minor sweep: config validation, RNG, snapshots, HUD, wizard, benches (Closes #127)' (#136) from 127-minors into master

Rows
Columns

v0.6.0 minors from the review: config validation panics, rnd() correlation, revocation snapshot leak, stale market-loans, HUD (was) rate, wizard/bench cosmetics #127