sts2-bot/docs/research/12-correctness-pass.md

# 12 — First correctness pass

Status: implemented; live gameplay quality is not yet measured.

This pass addresses the reproducible defects from [the prototype audit](11-prototype-hardening.md).
It keeps the current Python modules and synchronous loop. No runtime dependency or test framework was added.

## Combat changes

- Hand damage already includes attacker Strength and Weak. The damage calculation no longer applies them again.
- The fact layer and policy share one direct-damage subset search.
- Vulnerable rounding happens per hit. Block is absorbed once when comparing a line with enemy HP plus block.
- The direct-damage search accepts plain attacks with fixed energy costs.
  Compound/conditional descriptions, unknown/X costs, star spending, and unsupported powers are excluded.
- Individual killability remains separate from whole-combat lethal.
  `lethal_available` is `None` for multiple enemies; no joint resource-allocation search exists yet.
- Unknown/X costs remain visible in model context rather than becoming zero.
- Block planning maximizes displayed immediate block under the fixed energy budget.
  The policy starts that plan instead of selecting the largest block card independently.
- Fully covered incoming damage no longer forces additional defense.
- Estimated survival permits nonlethal HP loss; reaching zero HP does not count as survival.

These are limited mechanics calculations, not a full simulator.
Relic hooks, draw outcomes, card side effects, and dynamic sequences remain outside the model.
A false result means no supported line was found, not that every possible game line was ruled out.
Fight duration and projected future damage remain heuristics.

The stricter search can decline lines that the old code accepted, including compound attacks such as Bash.
Those cards remain available to the ordinary policy; they are not removed from playable actions.
This deliberately favors an explicit limitation over an unsupported lethal claim.

## Runner changes

`--dry-run` now sends no action POSTs, including both game-over dismissal paths.
It does not persist `deck.json`. It still reads state, can call Jev, and writes capture logs.
Some mod GET handlers have automatic UI behavior, so dry-run is not a game-state sandbox.

Combat model errors reach the runner. Below the failure limit, the runner uses `brain.decide(..., client=None)`
so combat has a real heuristic fallback. Only a successful model response resets the failure counter;
a code-only action or animation wait does not demonstrate recovery.

Missing model credentials no longer silently select heuristic-only play. Use `--no-jev` intentionally.
Unexpected policy errors stop the session instead of hiding programming defects behind another decision.
Game-over dismissals must return an accepted action result.

### Exit codes

| Code | Meaning |
|---|---|
| `0` | Requested bounded session or preview completed, or run end was observed |
| `1` | State/action/policy failure, repeated rejection, or unchanged-state timeout |
| `2` | Preflight blocker or invalid command-line arguments |
| `3` | Model initialization failed or consecutive model failures reached the limit |
| `4` | `--stop-on-run-end` reached the step limit without observing run end |

A rejected final action cannot become success merely because the step limit was reached.
Each initialized session records `exit_code`, `stop_reason`, and `dry_run` in its session row.
The trace also records `session_end` and model failures, including the failure that triggers an abort.
These are small additions for failure visibility, not the complete recording redesign.

## Client boundaries

The game client wraps connection failures, timeouts, and read failures as `Sts2Error`.
JSON responses must be objects. Markdown requests use the same transport error handling.
The client never retries an action POST: after an ambiguous failure, the action may already have reached the game.

The Jev parser rejects missing Noul values instead of converting them to a confident no.
It validates required answer fields, finite numeric ranges, Choice membership, Score levels, and usage counts.
Responses must match the requested question IDs, primitive types, and option/level sets.
Protocol failures raise `JevError` so the runner can apply its failure policy.

## Verification approach

Follow the project testing preference: integration/end-to-end checks first.
Keep only essential persistent regressions. Use temporary isolated probes for low-level edge cases.
The existing script tests remain; this pass does not migrate the legacy suite or add a permanent client unit-test suite.

Permanent checks cover the corrected hand-description contract, shared-energy ambiguity,
block planning, and the real runner's dry-run/fallback/failure paths.
Runner checks isolate files and external boundaries, but execute the actual policy loop.
History checks now use a temporary run record instead of reading a personal game history directory.

Temporary whole-process checks ran the real runner, facts, policy, and HTTP clients against local fixture endpoints.
Each process used a temporary working directory and capture/history paths.
Model credentials were dummy values, and the request wrapper rejected nonlocal URLs.
No real game or paid model was contacted.

| Whole-process scenario | Result |
|---|---|
| Model decision, action, then game-over dismissal | Exit 0; two game action POSTs; one model request |
| Dry-run on a parked game-over screen | Exit 0; no POSTs |
| Dry-run combat with a model response | Exit 0; one model request; no game action POST or deck write |
| Model outage, heuristic fallback, wait, second outage | Exit 3; one fallback action; two model requests |
| Malformed model answer | Exit 3; no game action |
| Connection closes during an action POST | Exit 1; exactly one action attempt |
| Action rejected at the step limit | Exit 1 |
| Run still active at the step limit | Exit 4 |

A Nix shell is optional for dependency isolation. It does not isolate game saves or prohibit network access.
The checks used the existing Python interpreter and standard library; temporary directories isolated their outputs.

Run the permanent checks:

```sh
python3 test_facts.py && python3 test_brain.py && python3 test_run.py
python3 utils/audit_prototype.py
python3 migrate.py --check-only
bash -n eval_batch.sh ab_card_skip.sh
```

Results: **272 assertions passed** across the three scripts. The dataset integrity and shell syntax checks passed.
The audit replayed **346 observations without policy exceptions**. Both captured Strength examples now calculate 8 damage,
and no captured state triggers forced defense after incoming damage is covered.
A temporary cross-check matched the block planner against exhaustive enumeration on 500 generated hands.

The audit remains diagnostic, not a pass/fail suite. It also reports known issues that belong to the next step.

## Next: policy state and recording

Update: [the policy-state pass](13-policy-state.md) addresses item 1 below.
The list records the remaining work at the end of this correctness pass:

1. Selection/shop/minigame memory can change when an action is proposed, before execution succeeds.
2. Captures cover only combat and reuse filenames across sessions.
3. Decisions lack an exact observation reference and a separate action-attempt identity.
4. Logged default gates can disagree with the gate actually used by a policy handler.
5. Combat-pile snapshots are still used as deck context without run identity or persistent-deck provenance.
6. Session identity is process-global, and session finalization still depends on `atexit`.

The next change should introduce explicit policy memory and reconcile attempts with action results and observations.
Then link every observation, proposal, attempt, and result without expanding the module structure unnecessarily.
Do not infer improved win rate from these correctness checks.