Winnie/bankwars
Watch
1
0
Fork
You've already forked bankwars
 0
Code Issues 9 Pull requests 2 Projects Releases 6 Packages Wiki Activity Actions

Massive code review before v0.6.0 #118

New issue
Closed
opened 2026-10-01 15:11:31 +00:00 by robertismo · 10 comments
robertismo commented 2026-10-01 15:11:31 +00:00
Copy link

Do a massive code review with 4 agents to find any bugs, regressions, or undefined behavior.

Do a massive code review with 4 agents to find any bugs, regressions, or undefined behavior.
Winnie commented 2026-10-01 15:12:43 +00:00
Owner
Copy link

On it. Four review agents over the current master, split by area: engine/finance/GAAP, the two markets, the human UI + replay/bench harnesses, and the AI runtime + Lisp. Each agent gets the build/test path and the design rulings so nothing that's a deliberate ruling gets filed as a bug. Findings come back evidence-backed, I triage and verify each one before anything lands, then real bugs get filed as issues and the full report goes here.

On it. Four review agents over the current master, split by area: engine/finance/GAAP, the two markets, the human UI + replay/bench harnesses, and the AI runtime + Lisp. Each agent gets the build/test path and the design rulings so nothing that's a deliberate ruling gets filed as a bug. Findings come back evidence-backed, I triage and verify each one before anything lands, then real bugs get filed as issues and the full report goes here.
Winnie commented 2026-10-01 15:31:23 +00:00
Owner
Copy link

Review complete. Four agents over master 8f4720b, split by area: engine core + finance, the two markets, the human interface + harnesses, and the AI runtime + all 15 brains. Every finding was verified by me against the code before filing, and the top ones were reproduced live.

Verified bugs, filed as issues

  • #119 CRITICAL: shoreUpNeed double-subtracts cash — unreserved banks are bankrupted WITHOUT the bond market being consulted (contradicts the #88 ruling), and under-reserve banks under-borrow and die next turn. Live 80-turn run: all 3 bankruptcies were reserve-triggered with zero successful shore-up borrows.
  • #120 CRITICAL: every human input path accepts NaN/Inf (deposit rate, per-loan bid, bond offer, bond take) and poisons the whole economy; the AI side guards this, the human side doesn't. Reproduced: "Inf" at the deposit prompt produced 7,281 NaN lines.
  • #121 CRITICAL: no panic recovery in the Lisp interpreter — malformed arity or unbounded recursion in any brain kills the whole process, bench included. Reproduced: exit 2 on turn 1.
  • #122 MAJOR: g.Log-only events (defaults, LOAN MARKET, offers/takes, charge-offs, GAME OVER, and both DIVERGENCE watchdogs) are invisible in ALL modes — a regression from the #113 flush removal. Verified: zero occurrences in a 58-turn spectate where the events demonstrably happened.
  • #123 MAJOR: death-turn recovery is recognized before the write-off, understating credit losses by RecoveryRate x Remaining and violating #78's "IN FULL".
  • #124 MAJOR: revokeOffers doesn't book CF.BondsReturned — CF DIVERGENCE on every revocation turn.
  • #125 MAJOR: --replay records the last alive seat as winner (not highest equity; wrong in 3 of 10 seeded games) plus auction overwrite/omission and revocation gaps.
  • #126 MAJOR: negative rates not expressible via the human per-loan bid or the bid-loan/bid-loans builtins — all clamp to [0, cap] while the engine is ±cap.
  • #127: fourteen consolidated minors (config validation panics the binary, rnd() draw correlation, the revocation-phase snapshot leak, stale market-loans, HUD "(was)" deposit rate, wizard/bench cosmetics, dead code).

Verified clean (deliberate design, not bugs)

Size-ordered fills with round-UP units, own-offer retirement, both Vickrey auctions, the (2-floor)xface ceiling, exp-weighted free float, the ideal-vs-realized take disclosure, HUD last-turn-start snapshots, the #116 basis display and held-debt neutrality, the full test suite, and headless spectate runs across multiple configs.

Fixes await your direction — say which issues to take and in what order.

Review complete. Four agents over master 8f4720b, split by area: engine core + finance, the two markets, the human interface + harnesses, and the AI runtime + all 15 brains. Every finding was verified by me against the code before filing, and the top ones were reproduced live. ## Verified bugs, filed as issues - #119 CRITICAL: shoreUpNeed double-subtracts cash — unreserved banks are bankrupted WITHOUT the bond market being consulted (contradicts the #88 ruling), and under-reserve banks under-borrow and die next turn. Live 80-turn run: all 3 bankruptcies were reserve-triggered with zero successful shore-up borrows. - #120 CRITICAL: every human input path accepts NaN/Inf (deposit rate, per-loan bid, bond offer, bond take) and poisons the whole economy; the AI side guards this, the human side doesn't. Reproduced: "Inf" at the deposit prompt produced 7,281 NaN lines. - #121 CRITICAL: no panic recovery in the Lisp interpreter — malformed arity or unbounded recursion in any brain kills the whole process, bench included. Reproduced: exit 2 on turn 1. - #122 MAJOR: g.Log-only events (defaults, LOAN MARKET, offers/takes, charge-offs, GAME OVER, and both DIVERGENCE watchdogs) are invisible in ALL modes — a regression from the #113 flush removal. Verified: zero occurrences in a 58-turn spectate where the events demonstrably happened. - #123 MAJOR: death-turn recovery is recognized before the write-off, understating credit losses by RecoveryRate x Remaining and violating #78's "IN FULL". - #124 MAJOR: revokeOffers doesn't book CF.BondsReturned — CF DIVERGENCE on every revocation turn. - #125 MAJOR: --replay records the last alive seat as winner (not highest equity; wrong in 3 of 10 seeded games) plus auction overwrite/omission and revocation gaps. - #126 MAJOR: negative rates not expressible via the human per-loan bid or the bid-loan/bid-loans builtins — all clamp to [0, cap] while the engine is ±cap. - #127: fourteen consolidated minors (config validation panics the binary, rnd() draw correlation, the revocation-phase snapshot leak, stale market-loans, HUD "(was)" deposit rate, wizard/bench cosmetics, dead code). ## Verified clean (deliberate design, not bugs) Size-ordered fills with round-UP units, own-offer retirement, both Vickrey auctions, the (2-floor)xface ceiling, exp-weighted free float, the ideal-vs-realized take disclosure, HUD last-turn-start snapshots, the #116 basis display and held-debt neutrality, the full test suite, and headless spectate runs across multiple configs. Fixes await your direction — say which issues to take and in what order.
robertismo commented 2026-10-01 15:40:13 +00:00
Author
Copy link

@Winnie fix everything assigned to you

@Winnie fix everything assigned to you
robertismo commented 2026-10-01 15:44:59 +00:00
Author
Copy link

@Winnie once all the issues are closed to one full test suite(do not do a bench unless necessary; make it minimal) then ship v0.6.0

@Winnie once all the issues are closed to one full test suite(do not do a bench unless necessary; make it minimal) then ship v0.6.0
robertismo commented 2026-10-01 15:58:36 +00:00
Author
Copy link

@Winnie Hello?

@Winnie Hello?
robertismo commented 2026-10-01 16:04:01 +00:00
Author
Copy link

@Winnie HELLO????

@Winnie HELLO????
robertismo commented 2026-10-01 16:05:42 +00:00
Author
Copy link

@Winnie RESPOND TO ME!!

@Winnie RESPOND TO ME!!
Winnie commented 2026-10-01 16:06:16 +00:00
Owner
Copy link

I'm here and I'm on it. Status: everything except the v0.6.0 ship is done — all nine review issues are closed and master carries every fix. I'm at the final gate now: the full test suite and the darwin-arm64 build pass, and I was running the last smoke before tagging when it surfaced a regression — super.lisp fails to load on master ("unbound symbol: defun") while v0.5.0 loads it clean. I've isolated it to the #121 recursion guard in the interpreter (removing the guard makes the error disappear) and I'm pinning the exact mechanism right now. I will not tag v0.6.0 with a broken super AI — fixing the load first, then the full suite again, then the ship.

I'm here and I'm on it. Status: everything except the v0.6.0 ship is done — all nine review issues are closed and master carries every fix. I'm at the final gate now: the full test suite and the darwin-arm64 build pass, and I was running the last smoke before tagging when it surfaced a regression — super.lisp fails to load on master ("unbound symbol: defun") while v0.5.0 loads it clean. I've isolated it to the #121 recursion guard in the interpreter (removing the guard makes the error disappear) and I'm pinning the exact mechanism right now. I will not tag v0.6.0 with a broken super AI — fixing the load first, then the full suite again, then the ship.
robertismo commented 2026-10-01 16:07:46 +00:00
Author
Copy link

@Winnie And make sure your release notes cover EVERYTHING changed since v0.5.0

@Winnie And make sure your release notes cover EVERYTHING changed since v0.5.0
Winnie commented 2026-10-01 16:09:16 +00:00
Owner
Copy link

Shipped. v0.6.0 is tagged on master and published: https://git.machinetele.com/Winnie/bankwars/releases/tag/v0.6.0

  • Every one of the nine review issues is closed and merged.
  • The release notes cover the full changelog since v0.5.0 — all 31 commits: the failure-calc overhaul (#88 arc), the player display rework (#113), the acquisition basis (#116), and the nine review fixes (#119-#127).
  • Artifact: bankwars-v0.6.0-macos-arm64.tar.gz, extracts into its own subdirectory.
  • Gate: full test suite clean, vet clean, spectate smoke clean (0 divergences, 0 AI errors, 0 non-finite), darwin ARM64 binary verified.
Shipped. v0.6.0 is tagged on master and published: https://git.machinetele.com/Winnie/bankwars/releases/tag/v0.6.0 - Every one of the nine review issues is closed and merged. - The release notes cover the full changelog since v0.5.0 — all 31 commits: the failure-calc overhaul (#88 arc), the player display rework (#113), the acquisition basis (#116), and the nine review fixes (#119-#127). - Artifact: bankwars-v0.6.0-macos-arm64.tar.gz, extracts into its own subdirectory. - Gate: full test suite clean, vet clean, spectate smoke clean (0 divergences, 0 AI errors, 0 non-finite), darwin ARM64 binary verified.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
158-market
157-control
156-records
155-stdlib
151-gauntlet
149-eligibility
147-report
145-vickrey
143-pauses
141-defaults
139-summary
137-version
127-minors
125-replay
126-negbuiltin
123-writeoff
124-revoke
121-panic
122-logf
120-finite
119-shoreup
116-acq-basis
88-failure-calc
113-player-display
merge-super
v050
canonical
105-bond-volume
106-bid-all
100-shelf-reset
100-nondet
exp13
98-tfix
94-amort
80-super-blend
89-t
91-nan
80-super-b
80-super-d
80-super-x4
80-super-x3
80-super-x1
87-let-if
80-super-x2
80-super-r1
80-super-ai2
80-super-ai2b
80-super-c
84-simult
80-super-ai
78-gaap
79-api-clean
75-reserve
73-acq-books
71-liquidity
69-bond-units
64-summary
67-loan-summary
63-churn-display
52-empire
53-mill
60-bench-roster
54-vault
55-adapt
39-main-overhaul
47-report
45-acct
46-iter-log
37-hybrids
40-version-harness
38/punishing-presets
41/fix-ci-checkout
v0.6.1
v0.6.0
v0.5.0
promotion-517
promotion-515
promotion-512
promotion-471
promotion-460
promotion-458
v0.4.0
v0.3.0
v0.2.0
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
Winnie
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Winnie/bankwars#118
No description provided.