From 733c13c31c78cb1e6fbcdf463c893d58eba96179 Mon Sep 17 00:00:00 2001 From: Lars Bogner Date: Wed, 12 Aug 2026 15:31:17 +0200 Subject: [PATCH] Mark issues.md Issue 5 as fixed Co-Authored-By: Claude Sonnet 5 --- issues.md | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/issues.md b/issues.md index 7684edf..7d5a085 100644 --- a/issues.md +++ b/issues.md @@ -44,7 +44,7 @@ architecture matrix grows, not about rot or breakage. | 2 | No unknown-key validation — a typo in `config.toml` silently trains the wrong model | **High** | Small | **Fixed** | | 3 | `cli.py:train()` is a 58-parameter, 510-line fat controller | **High** | Medium | **Fixed** (`2bfb1ab`) | | 4 | `cli.py` is at 35.8 % coverage and holds untested override-precedence logic | **High** | Medium | **Fixed** (`2bfb1ab`, partial — see status note) | -| 5 | Inference bootstrap is duplicated verbatim between `predict` and `rollout` | **High** | Small | Open | +| 5 | Inference bootstrap is duplicated verbatim between `predict` and `rollout` | **High** | Small | **Fixed** | | 6 | Two independent v0.2→v0.3 migration surfaces encode the same knowledge | Medium | Medium | Open | | 7 | Positional tuple contracts (9-tuple, 7-tuple) between data, model and training layers | Medium | Small | Open | | 8 | `network.py` is 1745 lines holding three distinct modules | Medium | Small | Open | @@ -622,6 +622,39 @@ extraction. ## Issue 5 — The inference bootstrap is duplicated verbatim between `predict` and `rollout` +> **Status: Fixed.** `giant/checkpoint_io.py` now holds a single +> `load_for_inference(checkpoint, device, command_name, weights="raw", require_stage2=True)` +> plus an `InferenceContext` dataclass (stage1/stage2 models, all three normalizers, both +> vocab maps, both top-N maps, both conditioning axes, `k_max`, both stages' ddpm step +> counts, `other_policy`, the raw `model_config`, and `epoch`/`best_val_loss` for +> `rollout`'s YAML sidecar) — exactly the design this issue proposed, verified against the +> current (not `55332db`-era) code before writing it. `predict`/`rollout` in `cli.py` +> each shrink to one `try/except CheckpointCompatibilityError` call plus a block of +> `ctx.` unpacks; `cli.py` lost `_conditioning_axes`, `_stage_cfg`, `_ddpm_steps`, +> `_particle_type_other_policy`, `_load_pdg_topn_map`/`_load_mat_topn_map`, and +> `_load_model_weights` entirely (net ~180 lines off `cli.py`). The independent third copy +> in `giant/analysis/router_gating.py` was deleted in favour of a lazy +> `from giant.checkpoint_io import conditioning_axes` inside `load_router`'s existing +> lazy-import block, preserving that module's "polars/numpy only at module scope" +> contract (`checkpoint_io.py` imports torch eagerly, so it must never be imported at +> `router_gating.py` module scope). Two guard orderings from the two commands' drifted +> copies were consolidated into one (`warn_if_checkpoint_config_mismatch` now always +> runs right after the existence guards, and `predict`'s `--batch-size auto` estimate now +> reads `ctx.model_config` after the checkpoint loads rather than before) — both are +> console-output-order changes only, no error text or model behavior changed, confirmed by +> diffing `giant predict --help`/`giant rollout --help` byte-for-byte before and after (no +> flag touched) and re-reading both rewritten command bodies field-by-field against the +> original. `require_stage2` exists as a real parameter, exercised by a new test, even +> though both current callers pass the default `True`. New `tests/test_checkpoint_io.py` +> (17 tests: happy path, every guard individually with exact message-text assertions, the +> `require_stage2=False`/inactive-stage2 path, `conditioning_axes`/`stage_cfg` directly) +> plus thin `CliRunner` smoke tests in `tests/test_cli_predict.py` and the new +> `tests/test_cli_rollout.py` confirming `CheckpointCompatibilityError` actually surfaces +> as `typer.Exit(1)` through the CLI — previously this entire code path had zero test +> coverage. `uv run pytest -q` (803 passed, up from 784), `ruff check`, `ruff format +> --check`, and `ty check` all clean. Everything below this point describes the pre-fix +> state and is kept for historical context. + **Severity: High. Effort: Small. Risk if unfixed: silent train/inference skew.** **Location:** `giant/cli.py:1121-1201` (`predict`), `giant/cli.py:1534-1593` (`rollout`),