helm: drop the codec import and relax the nltk pin, fixing the extra on Python 3.14

[borgr]

Independent of the converter stack (#245, #246) — nothing in it depends on this, but a reviewer running the full suite on Python 3.14 will want this first.

What / source

Three changes that make the helm extra work outside the locked 3.12/3.13 environment, plus the lockfile sync the last one needs. No conversion output changes.

• The converter no longer imports helm.common.codec. It needs six HELM modules, which pull in cattrs and nothing else. helm.common.codec pulls torch, spacy, nltk and regex — for one from_json call, in a file that already decodes three other HELM dataclasses with dacite. The fourth now decodes the same way.
• nltk<3.10.1 → nltk!=3.10.1. 3.10.1 is the only release carrying the inisec import guard; 3.10.2 replaced it with a pathsec guard that covers corpus paths, not imports. The cap was holding the loose matrix rows to an nltk from before the fix, for a problem that release fixed.
• The HELM generation-args tests skip on the converter's own _HELM_IMPORT_ERROR, like the rest of the HELM tests, instead of pytest.importorskip('helm') — which passes on an install where the top-level package imports but a submodule does not, so the tests failed at collection instead of skipping.

Why it matters, measured

_HELM_IMPORT_ERROR on Python 3.14 with --all-extras:

	main	this branch
_HELM_IMPORT_ERROR	ConfigError('unable to infer type for attribute "REGEX"')	None
pytest tests	5 failed, 446 passed, 16 skipped	467 passed

That error comes from thinc/spacy, reached only through helm.common.codec. Dropping the import is what makes the HELM converter run on 3.14 at all — crfm-helm's spacy dependency ships no 3.14 wheel, and it was never needed.

Unchanged elsewhere: 3.13 + all extras 467 passed on both. Core install 411 passed on both, skips 20 → 24 (the generation-args module now skipping as a unit rather than half-collecting).

nltk 3.10.2 in a venv under the CWD — the layout that tripped the guard — imports helm.common.codec cleanly, so the relaxed specifier is not re-opening the problem the cap was for.

Review lane

• Fast — one converter file, one test file, packaging; 35 lines changed.
• Needs a human

Checklist

• offline unit test added + full uv run pytest tests green (and green on 3.14, where main is not)
• uv run ruff check clean
• n/a — no data conversion, no new records, no registry ids

Decisions & coverage

• Decision / where: decode per_instance_stats with dacite.from_dict per entry, in converters/helm/adapter.py.
  Chose / instead of: keeping helm.common.codec.from_json and documenting 3.14 as unsupported.
  Confidence: high — the file already decodes RunSpec, ScenarioState and Scenario this way, and the HELM case's 80 pinned sidecar rows and 24 pinned results go through this path.
  General? no.
• Decision / where: nltk!=3.10.1 rather than nltk>=3.10.2.
  Chose / instead of: a floor, which would drop the 3.9.x the locked rows resolve to for no reason.
  Confidence: high.
  General? no.

Coverage: unchanged — same records, same values; this is an import path and a specifier.

[mrshu]

⚒️ review-anvil report

Review decision: COMMENT — This is a focused compatibility improvement;
one medium-priority test-coverage suggestion remains.
Result: The codec removal and dependency relaxation look well targeted. The
review found one opportunity to make the full-dependency CI signal more robust.
Scope: Reviewed the four-file PR at d66facf1, including the HELM decoder,
dependency metadata, lockfile, and generation-argument tests.
Checks: Reproduction was not needed because two independent reviewers
identified the same concrete behavior from the exact head.
Second check: Targeted challenge with 2 Codex reviewers; both upheld the
finding, and one refined the proposed fix to cover local full-dependency runs
as well as CI.

Actionable suggestion

RAV-RUN1-R1-F001 [medium] Keep a failing signal for broken installed HELM dependencies

tests/test_helm_generation_args.py:17 now applies the
_HELM_IMPORT_ERROR skip to the whole module. That is useful for core installs,
but it also means the full CI rows can stay green if HELM is installed and one
of the guarded converter imports is broken—the same import-compatibility class
this PR is designed to improve. The other HELM tests skip on the same condition,
so they do not provide an independent full-install sentinel.

A small way to preserve both behaviors would be to move the skip marker onto
the generation-argument tests and add one unmarked sentinel test. That sentinel
can skip only when the top-level helm package is absent; when it is present,
it can call _require_helm_dependencies() and allow a broken converter import
to fail. This keeps core installs optional while making --all-extras CI and
equivalent local test runs verify that the installed HELM extra is usable.

Would-apply plan

• Keep the current generation-argument cases skipped when HELM is genuinely
  unavailable.
• Add one unmarked test that skips only for an absent helm package and
  otherwise calls _require_helm_dependencies().
• Confirm core-only runs skip cleanly and full-extra runs fail on a broken
  guarded HELM import.

Run details

• Target: PR #243 (fix/helm-extra-imports, 4 files, +35/-19)
• Head reviewed: d66facf1a24e53e9f84172425c7e1368789ea9a2
• Rounds: 1/1 completed
• Mix: 3 codex-exec reviewers
• Reviewer timeout: 1200 seconds
• Adversarial mode: targeted (2 Codex challengers)
• Focus: constructive, positive review language and optional suggestions while
  preserving technical accuracy
• Earlier review comments: none

---

Reviewed with review-anvil.

[mrshu]

RAV-RUN1-R1-F001 [medium] tests — Nice improvement over checking only import helm: the converter guard is the right signal for these cases. One follow-up worth considering is preserving a failing signal when HELM is installed but a guarded converter import is broken. Because this module-wide marker skips for every _HELM_IMPORT_ERROR, the full --all-extras CI rows can also pass with a skipped HELM suite. Could we move this marker onto the generation-argument tests and add one unmarked sentinel that skips only when the top-level helm package is absent, but otherwise calls _require_helm_dependencies()? That would keep core installs optional while checking that a full HELM install is actually usable.