[kbhardwaj-nvidia]

Moved to NVIDIA's internal GitLab. This change runs internal datasets and does not belong in the public repository.

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-5109-8cec7e8d1517.docs.buildwithfern.com/nemo/gym

[svcnvidia-nemo-ci]

SHIP WITH CARE — one RISK and one NOTE.

Reviewed all nine changed files and their affected preparation, task-schema, tool-execution, seeding, and SQL-transport paths. Findings concern SQL dump text preservation and benchmark versioning.

Static review only: tests and reported rollouts were not executed, and the private datasets were unavailable. One delegated advisory was not surfaced; its preparation/tool-contract topic was independently inspected by the parent.

---

Inline comments (escaped v1):

• resources_servers/enterpriseops_gym/app.py line 147: RISK — Preserve embedded line endings when reading SQL dumps

  Path.read_text() performs universal-newline conversion, unlike the previous archive reader's UTF-8 decode. For a valid dump containing INSERT INTO notes VALUES ('a\r\nb'); with actual CRLF characters inside the SQL literal, this reader changes the value to a\nb before sending it to the database. Tool responses and verifier outcomes can consequently differ for CR/CRLF-containing text. This is a static counterexample to the configurable reader; I could not check whether the pinned private dumps contain it.

  Could this line retain the former decoding contract?

          return path.read_bytes().decode("utf-8")
  

  The invariant is that reading and transporting a dump must not rewrite characters inside SQL values. This replacement retains strict UTF-8 decoding, ordinary LF-only dumps, the preceding absolute/.. rejection, and valid snapshot-to-blob symlinks; it does not reinstate a resolved-path containment check that would reject those symlinks.

  Acceptance — proposed, not executed: create the dump using write_bytes, with actual CRLF inside the inserted value; seed a single-gym task through /seed_session and assert SELECT hex(text) FROM notes returns 610D0A62, not 610A62. Also retain the symlinked-fixture seed/tool behavior, the ../outside.sql → 422 assertion, and LF-only dump behavior. Please verify the suggestion before adopting it.

• benchmarks/enterpriseops_gym/manifest.yaml line 2: NOTE — Suggestion: version the changed benchmark protocol

  The manifest still declares 0.1.0, identical to the base manifest, although preparation now produces 1,117 rather than 649 tasks. Catalog entries and validation reports therefore expose the same name/version for two non-comparable task sets. The benchmark guide explicitly requires a new version for comparison-affecting changes.

  Could you give the full-set protocol a distinct version, for example:

  version: 0.2.0
  

  Preserve the benchmark name/routing, three repeats, five unchanged public example rows, image pins, and full-set licensing; only the protocol version needs changing.

  Acceptance — proposed, not executed: confirm the version differs from base, then—with the dataset prepared—check that catalog inspection and gym env validate enterpriseops_gym --kind benchmark report the new version and validate 1,117 benchmark rows plus five examples. Confirm the benchmark repeat count remains three. Please verify the suggested version before adopting it.

Review generated by codex. This was a full review. Request a full review with /review reset=true.