test: assert what every voyage-event outcome does to the voyage

[dmccoystephenson]

Summary

• The voyage-event outcome handlers in src/business/adventures.py were covered only by narration assertions. test_every_choice_returns_narration and test_every_outcome_branch_is_reachable both assert nothing beyond resolveChoice returning a truthy string, so every payout, repair, hull hit and crew loss in the event table was unverified. As reported in test: 30 voyage-event handlers are asserted only to return a non-empty string, so their payouts are unverified #156, deleting _merchantShadow's $400-900 payout left the whole suite green.
• A state-effect table has been added to tests/business/test_adventures.py. For all 29 outcome handlers, and each of the 38 branches they can take, the table names which part of the voyage state should move and by how much. random.randint is pinned to each end of the handler's documented range in turn, so both bounds are asserted, and random.choice is pinned so the hand that is lost is a named one that can be checked.
• Anything a handler is not expected to move is asserted not to move, so a narration-only choice that quietly damaged the hull would now fail.
• A coverage guard, test_every_outcome_in_the_event_table_has_a_state_spec, compares the handlers reachable from EVENTS against the table in both directions. A choice added to EVENTS without its effects being written down fails there, which is what keeps the table from going stale.
• Boarding odds are checked separately: _merchantBoard's success chance scales with the number of hands aboard, and a roll that a lone hand cannot clear is now shown to be cleared by a full boat.
• The existing sweeps have been kept rather than replaced — they remain valuable as reachability checks, and the issue recommended adding to them rather than substituting for them.

The structural alternative floated in the issue — having each handler return its state delta alongside its narration — was not taken. It would change production code under what is a coverage gap, and the table gets the same table-driven assertions without touching src/.

Test plan

• python3 -m pytest (with SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy): 806 passed, up from 803.
• python3 -m compileall -q src tests clean.
• black --check tests/business/test_adventures.py clean.
• Mutation-checked. Five deliberate defects were introduced one at a time into src/business/adventures.py and each was caught by the new tests, having been green before them:
  • _merchantShadow's payout deleted (gain(voyage, money=0)) — the exact mutation reported in test: 30 voyage-event handlers are asserted only to return a non-empty string, so their payouts are unverified #156
  • _leakShipwright calling damage where repair was meant
  • _goodGroundsNets paying in coin instead of fish
  • _patrolFight's payout range widened at the low end
  • _merchantBoard losing the wrong hand
• No new test depends on unseeded random; every roll in the new tests is patched.

Scope

Test-only: one file, 386 added lines, no change under src/. The soft scope ceiling counts non-test net LOC, which is zero here.

Closes #156

Issue #153 (the web front-end showing "lost connection" instead of the ended screen) was deferred this cycle. It is a production change across src/fishE.py and both browser front-ends, and its own body records an unresolved design question — cleanup() shuts the socket down before the client's next 300 ms poll, so the fix needs a grace period or a client acknowledgement to be visible at all. That warrants a cycle of its own rather than being batched with an unrelated test expansion.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Review

Reviewed adversarially against the repo's own conventions, with CI green on the head commit (3c551ed, run 31301233862) as the external anchor.

Rubric

• Scope — PASS. git diff --name-only origin/main...HEAD lists exactly one file, tests/business/test_adventures.py. Nothing under src/, no unrelated reformatting, no renames.
• Tests-new — PASS. Three new tests, all exercised: test_every_outcome_in_the_event_table_has_a_state_spec, test_every_outcome_handler_moves_exactly_what_the_table_says, test_boarding_a_merchantman_is_likelier_with_more_hands_aboard. The two new module-level helpers (createSweepVoyage, measureHandler) are called by them.
• Tests-fix (empirical) — PASS. Rather than stashing a fix that does not exist in a test-only PR, the inverse was measured: seven defects were injected into src/business/adventures.py one at a time and the suite re-run against the branch head. All seven were caught, and the first five were confirmed green on main before this branch. The mutations were _merchantShadow's payout deleted, _leakShipwright calling damage where repair was meant, _goodGroundsNets paying in coin instead of fish, _patrolFight's range widened at the low end, _merchantBoard losing the wrong hand, _squallHugCoast no longer eating supplies, and _passengerIgnore quietly taking 3% off the hull. src/ was left untouched afterwards (git diff --quiet src/ clean).
• Sibling structure — PASS after a fix. No new file was created, and the added code follows the file's existing # prepare / # call / # check structure and its create<Thing>() helper convention. One deviation was found and corrected during review, noted below.
• Sibling renames — no signal this cycle. Nothing was renamed.
• Docs — PASS. Every row of the documentation sources-of-truth table was checked against the diff. README.md describes gameplay, which is unchanged. No field is persisted, so schemas/player.json, schemas/stats.json, schemas/shop.json and schemas/timeService.json are untouched and correctly so. PLANNING.md lists no goal that this completes. version.txt stays at 3.0.0-SNAPSHOT-8-8-2026; test coverage is not release-worthy on its own.
• Issue resolution — PASS. Issue test: 30 voyage-event handlers are asserted only to return a non-empty string, so their payouts are unverified #156 names six handlers as the highest-value targets (_merchantBoard, _merchantShadow, _patrolFight, _goodGroundsFish, _goodGroundsNets, _leakShipwright) plus every handler calling loseCrew. All six appear in the table by name, and all four loseCrew branches assert both that the roster shrinks by one and which named hand is gone. The issue's own reproduction (deleting _merchantShadow's payout leaving the suite green) now fails.
• CI — PASS. Green on 3c551ed, the current head.

Repo-specific rubric

• Schema-sync — not applicable. Nothing is persisted by this change; no *JsonReaderWriter and no schemas/*.json file is touched.
• Money-format — not applicable. No money is displayed. Note for the record that voyage money is integer-only here (random.randint throughout adventures.py), so the %.2f rule is not in play.
• Deterministic-tests — PASS. Every roll reachable from the new tests is patched: random.random, random.randint and random.choice are all replaced inside each with block. No new test observes unseeded randomness.
• Headless-pygame — not applicable. No pygame-touching path is added.
• camelCase — PASS. createSweepVoyage, measureHandler, pickBound, crewLost, handsLost, SWEEP_ABOARD follow the surrounding naming. Test function names stay snake_case, matching every sibling test in the file.

Findings addressed during review

Two were found and fixed in 3c551ed rather than left for a follow-up, both mechanical:

• tests/business/test_adventures.py:344 — SWEEP_ABOARD was defined as len(SWEEP_CREW), but berths are capped by boat tier (maxCrew for tier 3 is 12, and the roster is currently exactly 12). Had a thirteenth villager been added, hireWorker would have refused the last hire and the four supplies-eating expectations would have gone quietly wrong, failing with a confusing off-by-one rather than a real signal. The value is now read off the constructed voyage with adventures.crewAboard(), and SWEEP_FIRST_LOST likewise off voyage["crew"][0].
• tests/business/test_adventures.py:711 — the boarding-odds test indexed the fixture tuple as createVoyage(...)[2], where all 20-odd sibling tests unpack it as _, _, voyage =. Changed to match.

Observations not blocking

• itertools.cycle is used for the forced random.random rolls, so a handler that grows an extra roll will silently reuse a value instead of raising StopIteration. This is deliberate and commented: an exhausted side_effect reads as a broken test rather than as the outcome under examination, and a branch taken in error is still caught by the state assertion.
• handsLost is asserted to stay at zero on every table row and is never expected non-zero, because the sweep crew is entirely named villagers. That is coverage of a different kind — no handler may touch the unnamed headcount — and the unnamed-hand loss path itself remains covered by the pre-existing test_unnamed_hands_lost_at_sea_are_taken_off_the_headcount.
• The table depends on the fixture sailing out at hull = 60, which leaves room for the largest damage in the table (40, from _patrolFight) and the largest repair (16, from _leakShipwright) without clamping at either end. Widening a range past that margin would fail here, which is the intended behaviour rather than a fragility.
• The structural alternative suggested in test: 30 voyage-event handlers are asserted only to return a non-empty string, so their payouts are unverified #156, returning a state delta from each handler alongside its narration, was not adopted. It would mean changing production code to close a coverage gap, and the table achieves the same table-driven assertions with src/ untouched.

Merge readiness

Test-only, one file, no production change, CI green, and the gap reported in #156 demonstrably closed by measurement rather than by inspection. No path on the do-not-auto-merge list is touched: nothing under schemas/, src/saveFileManager.py, src/**/*JsonReaderWriter.py, .github/workflows/, and no file loses more than 50 lines — the diff is 392 insertions and no deletions at all.

This review was performed and posted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson