[andreatnvidia]

What

Honor session-cookie invalidation between Gym lifecycle calls and cancel active/queued Pi turns when the service stops. Prepare data-designer-gym for its first 0.1.0 package release with catalog registration, shared reviewer ownership, current dependency requirements, clean source archives, installation docs, and automated Pi adapter tests.

Fixes #106
Fixes #107

Why

Deleted cookies could be sent again during execution or cleanup. The Pi server could also wait indefinitely for active requests before reaching its cancellation hook. Address these lifecycle issues and the remaining packaging gaps before publishing 0.1.0.

Usage

After publishing data-designer-gym/v0.1.0, install into an active Python >=3.10 environment:

uv pip install \
  --default-index https://pypi.org/simple/ \
  --index https://nvidia-nemo.github.io/DataDesignerPlugins/simple/ \
  data-designer-gym

The plugin requires DataDesigner >=0.9.7. Gym services and the optional Pi example keep their separate runtimes.

How

• Reconcile cookie expiry before carrying live cookies between trusted services, preserving replacements and row isolation. Parse deletion headers with the standard-library parser used by HTTPX, then apply HTTPX's live-cookie acceptance policy without changing the original headers.
• Cancel tracked turns on SIGTERM/SIGINT before Uvicorn drains requests, reject new turns during shutdown, and finish the existing shielded Pi process cleanup. Expose the active request count, including queued turns, in /stats.
• Register the package's three entry points, compatibility requirements, documentation URL, and package index in the catalog. Include the Data Designer reviewers team alongside the package maintainer in CODEOWNERS.
• Require DataDesigner >=0.9.7 to match the repository baseline. Exclude local demo data, downloaded Pi runtimes, and smoke-test session files from source distributions.
• Document package-index installation in the README and site docs, and update the tested runtime combinations.
• Run the optional Pi adapter's deterministic tests in a separate Python 3.14.4 CI job using its existing Makefile targets.

Validation

• Cookie regressions reproduce unrelated-domain deletions using both expiry forms during execution and intermediate initialization, and cover accepted parent-domain/case variants, encoded values, extension attributes such as Priority, and row isolation.
• Core plugin and devtools suites: 330 passed on Python 3.12 with DD 0.9.7.
• Installed wheel: 171 plugin tests passed on Python 3.10 with DD 0.9.7; verified imports resolve to the installed package.
• Fresh isolated Pi environment: make install test passed all 27 adapter tests.
• Both real Pi/DD restart and timeout smokes passed with the installed wheel and DD 0.9.7.
• The README's resource-only quickstart passed against the pinned Gym runtime and installed wheel, including downstream evidence consumption and session cleanup.
• Built wheel and source archive passed Twine checks. Building from the exercised checkout now produces a 32-file source archive of about 61 KB, retaining examples and excluding local runtime/session files.
• Ruff, plugin/release/catalog validation, generated docs and ownership, license headers, uv lock --check, strict documentation build, and git diff --check passed.

[nabinchha]

Thanks for putting this together, @andreatnvidia!

Summary

This PR fixes cookie invalidation between Gym lifecycle calls, cancels active and queued Pi turns during shutdown, and registers Gym's three entry points for its first release. The implementation matches the stated intent, with one remaining correctness issue in cookie deletion validation.

Findings

Critical — Let's fix these before merge

plugins/data-designer-gym/src/data_designer_gym/columns.py:380–381 — Validate cookie domains before applying deletions

• What: The new expiry loop removes a carried cookie by name without validating its Domain against the responding host. A header such as Set-Cookie: episode=; Max-Age=0; Domain=unrelated.example; Path=/ deletes the live episode cookie, although HTTPX rejects a replacement with that domain. I reproduced this through the existing lifecycle test with both Max-Age=0 and a past Expires.
• Why: Subsequent lifecycle requests lose the valid session cookie. In the reproduction, cleanup receives no cookie, which can prevent the service from identifying and releasing the allocated session.
• Suggestion: Validate deletion headers against the response origin consistently with accepted replacements before mutating the carried cookies. Extend the existing parametrized regression test with unrelated-domain deletion cases for both expiry forms, during execution and intermediate initialization.

What Looks Good

• Shutdown cancellation runs before Uvicorn drains requests, and the admission checks prevent queued turns from starting after shutdown begins.
• The real-server regression tests exercise blocked and queued turns, interrupted state, process termination, and preservation and replay of committed responses.
• The catalog entry matches the package's three entry points and compatibility metadata; all three installed entry points passed plugin validation locally.

Residual Risk

• Local validation passed: 149 tests through make test-plugin PLUGIN=data-designer-gym, make lint with locked Ruff 0.15.11, catalog validation, and validation of all three Gym entry points. GitHub CI passed at reviewed commit 5c6e7ba7705e81cc60ef566c89329245e3fa4abd.
• Local tests used Python 3.11.9. Python 3.10, the optional Pi adapter suite, documentation builds, and the full local CI aggregate were not independently run; verification focused on the affected plugin, lint, and catalog contracts.

Verdict

Needs changes — Validate cookie deletion domains and add regression coverage before merging. No additional defects were confirmed, and there were no existing PR reviews or inline comments to duplicate when checked.

---

This review was generated by an AI assistant.

[andreatnvidia]

Thanks for catching this, @nabinchha. Fixed in the latest commits. Deletions now use HTTPX’s cookie parser and domain checks before removing a carried cookie. Added regressions for both expiry forms during execution and intermediate initialization, including headers with extension attributes like Priority.

[nabinchha]

Thanks for addressing the feedback, @andreatnvidia!

Summary

Reviewed the follow-up changes at d14be65. Cookie deletions now use HTTPX's acceptance policy, resolving the finding in my earlier review. The packaging, ownership, documentation, and CI updates match the stated release-preparation intent.

Findings

No remaining confirmed findings.

What Looks Good

• Domain validation is consistent with cookie replacements, with coverage for both expiry forms, extension attributes, and intermediate initialization.
• The optional Pi adapter tests now run in a separate CI job.
• Package requirements, catalog metadata, generated ownership, and installation documentation are consistent; source archive exclusions target local runtime and test artifacts.

Residual Risk

All 36 updated lifecycle cookie regression cases passed locally, and all applicable CI checks passed at d14be65, including the Pi adapter job. The full updated plugin and Pi suites were not rerun locally; those results were verified through CI.

Verdict

Ship it — The original finding is resolved, and the follow-up review found no additional blockers. This approval supersedes the Needs changes verdict in my earlier comment.

---

This review was generated by an AI assistant.