Skip to content

fix: textarena nltk compatibility - #1116

Merged
burtenshaw merged 7 commits into
mainfrom
dependabot/uv/envs/textarena_env/nltk-3.10.3
Sep 10, 2026
Merged

fix: textarena nltk compatibility#1116
burtenshaw merged 7 commits into
mainfrom
dependabot/uv/envs/textarena_env/nltk-3.10.3

Conversation

@dependabot

@dependabot dependabot Bot commented on behalf of github Sep 1, 2026

Copy link
Copy Markdown
Contributor

This PR updates TextArena to NLTK 3.10.3 and isolates NO_PROXY-only downloads while preserving proxy security checks.


Note

Medium Risk
Startup now shells out for NLTK data in a narrow proxy/NO_PROXY case and changes dependency/version behavior; mistakes could affect TextArena boot or corporate proxy setups.

Overview
Bumps the TextArena env dependency to NLTK ≥3.10.3 (lockfile refreshed, including NLTK’s new defusedxml dependency).

_ensure_nltk_data no longer only calls nltk.download inline. For NLTK 3.10.3 when proxy env looks like NO_PROXY-only (NLTK #3748), it runs python -m nltk.downloader in a subprocess with NO_PROXY/no_proxy stripped from that child env so the server process keeps its proxy settings. Real HTTP(S) proxies and explicit urllib proxy handlers still use in-process nltk.download(..., raise_on_error=True).

After download, it verifies words and averaged_perceptron_tagger_eng via nltk.data.find before setting _NLTK_DOWNLOADED, so a zero-exit CLI failure does not mark startup as successful.

Adds tests/envs/test_textarena_nltk.py covering subprocess isolation, proxy security paths, subprocess failures, and missing-corpus verification.

Reviewed by Cursor Bugbot for commit ef98ae1. Bugbot is set up for automated code reviews on this repo. Configure here.

Bumps [nltk](https://github.com/nltk/nltk) from 3.9.4 to 3.10.3.
- [Release notes](https://github.com/nltk/nltk/releases)
- [Changelog](https://github.com/nltk/nltk/blob/develop/ChangeLog)
- [Commits](nltk/nltk@3.9.4...v3.10.3)

---
updated-dependencies:
- dependency-name: nltk
  dependency-version: 3.10.3
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot Bot added Dependencies python:uv Pull requests that update python:uv code labels Sep 1, 2026
@burtenshaw burtenshaw added environment size: small Small pull request labels Sep 1, 2026 — with Cursor
@bot-ci-comment

bot-ci-comment Bot commented Sep 1, 2026

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 89a31c9. Configure here.

Comment thread envs/textarena_env/uv.lock

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alignment Review Report

Dependabot lock-only bump of nltk 3.9.4 → 3.10.3 in envs/textarena_env (single file: uv.lock). Verified end-to-end — clean and strongly security-positive. This supersedes the now-closed #1031 (which targeted 3.10.0 and never merged; the floor on main is still nltk>=3.9.3, which 3.10.3 satisfies, so no pyproject.toml edit was needed).

Automated Checks

  • Lint: PASS (for this PR). lint.sh exits non-zero only on ~26 pre-existing envs/**/*.py formatting-drift files; this PR touches only uv.lock (TOML/lock aren't linted). CI's lint job is green.
  • Debug code: CLEAN. check-debug.sh scans src/ only; no .py changed here.

Open RFCs Context

Scanned rfcs/: 000/001/002/003/005/008 In Review, 010 Draft, 004 (no status). None cover packaging / dependency management / PyPI, so a dep bump has no RFC surface.

Tier 1: Fixes Required

None.

  • uv lock --check passes (127 pkgs); nltk + defusedxml hashes verified against PyPI (see inline).
  • Born-clean lock: revision 3 unchanged, 0 mirror refs (all pypi.org/simple) → no index-flip, no revision bump.
  • Package-set delta = only defusedxml 0.7.1 added + nltk changed; no other artifact hashes touched.
  • CI is fully green (lint, test 3.11/3.12, validate-env-locks, build, check-env-docs, pkg smoke-test).

Tier 2: Alignment Discussion

Principle Conflicts

None identified. A dependency bump touches no Gym API / rewards / client-server / MCP-WS / credential surface. The new defusedxml hardens XML parsing (aligns with the "cutting-edge for stability" trade-off and the security posture).

RFC Conflicts

None identified.

Non-blocking notes (unchanged files → not fixable in this PR)

  • Dependabot process mismatch (recurring): .github/dependabot.yml sets exclude-paths: ["envs/**"] (lines 8-9), yet this native dependabot/uv/envs/textarena_env/... PR still edits envs/textarena_env/uv.lock. The uv ecosystem isn't honoring exclude-paths (valid key, platform limitation); the intended path for env bumps is the codex/dependabot-envs-* aggregate. cc @burtenshaw.
  • Deploy: server/Dockerfile consumes the lock via uv sync --frozen (L45), but the revision is unchanged (already 3) so there's no new base-uv requirement; nltk 3.10.3 + defusedxml install with valid hashes and the CVE fix lands at deploy.
  • Compat: server/environment.py uses only nltk.download("words") + nltk.download("averaged_perceptron_tagger_eng") (stable API, current _eng tagger name); nltk is in the CI test install list, so it's exercised.

Summary

  • 0 mechanical issues to fix
  • 0 alignment points for human review
  • 0 RFC conflicts

Security-positive (clears 24 nltk advisories, 10 of them fixed only in 3.10.3). Suggested reviewers: @Darktex (security/reproducibility), @burtenshaw (dependabot process). Recommendation: safe to merge.

Open in Web View Automation 

Sent by Cursor Automation: Pre-review

[[package]]
name = "nltk"
version = "3.9.4"
version = "3.10.3"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nltk 3.9.4 → 3.10.3 — verified against PyPI:

  • wheel sha256:ff9598a8…caf7c (1,798,643 B) and sdist sha256:bb9327a4…4ea4 (5,137,152 B) match the lock exactly; yanked=false; requires_python >=3.10 matches the env.
  • Runtime deps = defusedxml, click, joblib, regex>=2021.8.3, tqdm — all already locked; defusedxml is the only addition.

Strong security fix. nltk 3.9.4 now carries 24 advisories, all cleared in 3.10.3 (0 vulns reported). Notably 10 are fixed only in 3.10.3 (not 3.10.0, the earlier #1031 target) — e.g. CVE-2026-81724 / -81722 / -81726 / -81727, CVE-2026-79657 / -79674 / -79675 / -79676, CVE-2026-78680 / -78682 — so landing exactly 3.10.3 (rather than 3.10.0) matters. uv lock --check passes.

]

[[package]]
name = "defusedxml"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New transitive dep defusedxml 0.7.1, pulled in because nltk 3.10.x adds defusedxml to its runtime requirements. Verified against PyPI:

  • wheel sha256:a352e7e4…6a61 (25,604 B) and sdist sha256:1bb3032d…a3e69 (75,520 B) match the lock;
  • zero dependencies (adds nothing else to the tree), requires_python >=2.7 (compatible with the env's >=3.10), yanked=false, 0 known vulns.

It's a security-hardening XML parser (XXE / billion-laughs mitigation), so this addition is net-positive.

dependencies = [
{ name = "cryptography", marker = "(python_full_version < '3.11' and sys_platform == 'emscripten') or (python_full_version < '3.11' and sys_platform == 'win32') or (sys_platform != 'emscripten' and sys_platform != 'win32')" },
{ name = "jeepney", marker = "(python_full_version < '3.11' and sys_platform == 'emscripten') or (python_full_version < '3.11' and sys_platform == 'win32') or (sys_platform != 'emscripten' and sys_platform != 'win32')" },
{ name = "cryptography" },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI (not a concern): beyond the nltk bump, the re-lock also normalized environment markers on secretstorage (here) and pandas (two blocks above) — e.g. dropping the (python_full_version < '3.11' and …) or (sys_platform != 'emscripten' and sys_platform != 'win32') guards on cryptography/jeepney. No wheel/sdist/hash changed for these packages, and the effective resolution on the linux deploy target is identical; it's a benign metadata simplification from a newer uv rewriting the lock. Flagging only so the extra hunks don't read as an unrelated change.

cursor[bot]
cursor Bot previously requested changes Sep 9, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Hold this update: NLTK 3.10.3 has a confirmed startup regression when NO_PROXY is set. nltk/nltk#3748 reproduces nltk.download(...) failing with a security violation in Docker-style environments; the fix (6bffb2d) merged only after the 3.10.3 release.

This is directly relevant here: textarena_env calls nltk.download("words") and nltk.download("averaged_perceptron_tagger_eng") during environment initialization. Keep 3.9.4 for now, or update to a release containing the upstream fix and add a non-skipped startup regression with NO_PROXY set. The security fixes in 3.10.3 are desirable, but this exact version is not deploy-safe for TextArena.

Open in Web View Automation 

Sent by Cursor Automation: Release

@burtenshaw burtenshaw changed the title chore(deps): bump nltk from 3.9.4 to 3.10.3 in /envs/textarena_env fix: textarena nltk compatibility Sep 10, 2026
@burtenshaw
burtenshaw dismissed cursor[bot]’s stale review September 10, 2026 08:50

Addressed in fd769a3: isolated NO_PROXY-only downloads without changing parent proxy settings or weakening NLTK security. Ten regressions and two existing TextArena tests pass. Fresh uppercase/lowercase NO_PROXY startup/reset/step passed; real HTTP/HTTPS proxy downloads still fail closed. Standard suite: 1631 passed.

cursor[bot]
cursor Bot previously requested changes Sep 10, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking exact head 57c14c033b7f07e7a8b861a2c5923adb775737c7: the subprocess success check does not detect NLTK download failures.

NLTK 3.10.3's CLI calls Downloader.download(..., halt_on_error=True), breaks when it returns False, and then exits with status 0. I reproduced this with the installed 3.10.3 CLI: an actual security/download error printed to stderr but the process return code was 0. Therefore subprocess.run(..., check=True) returns normally and _NLTK_DOWNLOADED is cached as True even though one or both corpora are absent. The current failure test mocks a CalledProcessError, so it does not exercise the real CLI behavior.

After the subprocess, verify both resources with nltk.data.find(...) (using their real resource paths) before setting _NLTK_DOWNLOADED, and add a regression where the subprocess returns 0 but the resources are absent. Then rerun the real clean-data NO_PROXY startup smoke plus exact-head CI.

The proxy-detection direction itself is sound: the current head preserves genuine proxy rejection, and the real proxyless download succeeds.

Open in Web View Automation 

Sent by Cursor Automation: Release

@burtenshaw
burtenshaw dismissed cursor[bot]’s stale review September 10, 2026 09:02

Fixed in 73f16b3: verify corpora/words and taggers/averaged_perceptron_tagger_eng with nltk.data.find before caching success. Both missing-resource regression cases fail before the fix and pass after. All 15 TextArena tests pass. Reproduced real NLTK CLI zero-exit failure and verified initialization now rejects it. Fresh NO_PROXY/no_proxy startup/reset/step and real proxy rejection smokes pass.

@cursor
cursor Bot requested a review from Darktex September 10, 2026 09:06
@burtenshaw
burtenshaw merged commit da59295 into main Sep 10, 2026
11 checks passed
@dependabot
dependabot Bot deleted the dependabot/uv/envs/textarena_env/nltk-3.10.3 branch September 10, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Dependencies environment python:uv Pull requests that update python:uv code size: small Small pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant