Skip to content

feat: overridable results_dir/cache_dir roots; anchor SWE artifacts and setup trees on them - #2448

Merged
ananthsub merged 10 commits into
NVIDIA-NeMo:mainfrom
michal2409:mfutrega/swe-agents-base-results-dir
Aug 18, 2026
Merged

feat: overridable results_dir/cache_dir roots; anchor SWE artifacts and setup trees on them#2448
ananthsub merged 10 commits into
NVIDIA-NeMo:mainfrom
michal2409:mfutrega/swe-agents-base-results-dir

Conversation

@michal2409

@michal2409 michal2409 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Rewritten per review discussion — the original opt-in results_root_dir field carved out only one of the three kinds of state this server anchors at Path(__file__).parent:

  1. packaged read-only assets (setup scripts, prompts, configs) — install-relative is correct;
  2. reusable multi-GB setup trees (clones, venvs, toolchains) — a cache, reused across runs and bind-mounted into eval containers by host path;
  3. per-run results — written by the server and read across the run, from other nodes on multinode deployments.

Only (1) belongs next to the code, and (2)/(3) often need opposite placement (results on a shared filesystem, caches on fast local or baked container storage) — so one knob can't cover both.

What this does

  • nemo_gym: exposes the already-existing RESULTS_DIR / CACHE_DIR constants as overridable top-level config keys (results_dir, cache_dir), defaulted with the same setdefault pattern as uv_cache_dir three lines above, and added to the reserved-keys list. Any server can use them; nothing else changes unless a run overrides them.
  • swe_agents: derives both roots from the global config instead of the install dir — run artifacts under results_dir, all setup trees under cache_dir/swe_agents/ via a setup_root property on the shared processor base. parent_dir remains for packaged assets. Both reads fall back to the module constants when the config dict bypassed the parser defaults (env-provided dicts, tests).
  • Companion race fix: the OpenHands setup dir is now keyed by agent_framework_commit. file_lock is only held during setup(), so an unkeyed shared checkout could be reset --hard to a different commit by a later run while an earlier one was still executing from it.
  • Unit tests: parser defaults + override preservation; the existing setup-cache-hit test now patches setup_root.

Behavior note

Default locations move from the package directory to WORKING_DIR/{results,cache} — the layout the nemo_gym module header documents. Deployments that relied on in-package paths can pin the old locations via the new keys; otherwise setup trees rebuild once at the new default.

We run the multinode configuration in production (64-node Ray clusters with the SWE agents), where results must live on a shared filesystem while the setup trees are baked into the container image — the two-key split is load-bearing for that.

Scope. The keys cover directories consulted through the global config: the SWE agents' artifacts and setup trees, the uv_cache_dir default, and the W&B run dir. Import-time environment defaults (e.g. HF_HOME/HF_DATASETS_CACHE, set in nemo_gym/__init__.py before any config is parsed) intentionally keep using the module constants.

@copy-pr-bot

copy-pr-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@michal2409
michal2409 force-pushed the mfutrega/swe-agents-base-results-dir branch from 45bbac7 to 97838e1 Compare August 10, 2026 18:23
@michal2409 michal2409 changed the title feat(swe_agents): add base_results_dir option for SWE run artifacts feat(swe_agents): add results_root_dir option for SWE run artifacts Aug 10, 2026
@michal2409
michal2409 force-pushed the mfutrega/swe-agents-base-results-dir branch 2 times, most recently from 6865cc4 to 28ccbd2 Compare August 10, 2026 18:45
@michal2409 michal2409 changed the title feat(swe_agents): add results_root_dir option for SWE run artifacts feat: overridable results_dir/cache_dir roots; anchor SWE artifacts and setup trees on them Aug 11, 2026
@michal2409
michal2409 force-pushed the mfutrega/swe-agents-base-results-dir branch from 28ccbd2 to 9a29669 Compare August 11, 2026 10:18
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Aug 11, 2026
@michal2409
michal2409 force-pushed the mfutrega/swe-agents-base-results-dir branch from 7516baf to 228af91 Compare August 12, 2026 11:40
@laszkiewiczp
laszkiewiczp requested a review from ananthsub August 12, 2026 13:07
@github-actions github-actions Bot removed the sla:triage-overdue Review assignment is over the one-business-day SLA label Aug 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🌿 Preview your docs: https://nvidia-preview-mfutrega-swe-agents-base-results-dir.docs.buildwithfern.com/nemo/gym

Here are the markdown pages you've updated:

@michal2409
michal2409 force-pushed the mfutrega/swe-agents-base-results-dir branch from 0c92995 to 3e61878 Compare August 13, 2026 12:49
@github-actions github-actions Bot added the sla:review-overdue Review response is over the one-business-day SLA label Aug 13, 2026
Comment thread responses_api_agents/swe_agents/app.py Outdated
Comment thread fern/versions/latest/pages/reference/configuration.mdx Outdated
Comment thread responses_api_agents/swe_agents/app.py
@ananthsub
ananthsub requested a review from sdevare-nv August 13, 2026 13:53
@github-actions github-actions Bot added sla:review-overdue Review response is over the one-business-day SLA and removed sla:review-overdue Review response is over the one-business-day SLA labels Aug 13, 2026
michal2409 and others added 8 commits August 18, 2026 15:53
…nd setup trees on them

responses_api_agents/swe_agents anchored three kinds of state with
different lifecycles at Path(__file__).parent: packaged read-only assets
(setup scripts, prompts, configs), reusable multi-GB setup trees
(clones, venvs, toolchains), and per-run results. Only the first belongs
next to the code. On multinode deployments the other two have hard
placement requirements the install dir cannot always satisfy: results
are written by the server and read across the run (shared filesystem),
and setup trees are bind-mounted into eval containers by host path.

nemo_gym already defines RESULTS_DIR and CACHE_DIR (WORKING_DIR/results,
WORKING_DIR/cache). Expose them as overridable top-level config keys
(results_dir / cache_dir), defaulted with the same setdefault pattern as
uv_cache_dir, so a run can point results at a shared filesystem while
caches stay on fast local (or baked container) storage - independently,
because the two often need opposite placement.

swe_agents then derives both roots from the global config instead of
the install dir: run artifacts under results_dir, and all setup trees
under cache_dir/swe_agents/ via a setup_root property on the shared
processor base (parent_dir remains for packaged assets). Both reads
fall back to the module constants when a config dict bypassed the
parser defaults.

Companion fix while moving the trees: the OpenHands setup dir is now
keyed by agent_framework_commit. file_lock is only held during setup(),
so an unkeyed shared checkout could be reset --hard to a different
commit by a later run while an earlier one was still executing from it.

Note this moves the default locations of the setup trees and results
(from the package directory to WORKING_DIR/cache and WORKING_DIR/results
- the layout the nemo_gym module header documents). Existing deployments
that relied on the in-package paths can pin the old locations via the
new keys; setup trees rebuild once at the new default otherwise.

Signed-off-by: Michal Futrega <mfutrega@nvidia.com>
The parser defaulted uv_cache_dir from the CACHE_DIR constant and wandb's
run dir from the RESULTS_DIR constant, so overriding cache_dir/results_dir
moved the SWE trees but left these two behind. Default uv_cache_dir under
the (possibly overridden) cache root and point the wandb dir at the
results root. Behavior is unchanged when the keys are not overridden.

Signed-off-by: Michal Futrega <michal.futrega@gmail.com>
…path normalization, non-parsing config accessor, docs

- resolve_setup_dir prefers the cache-root tree but keeps using setup
  trees pre-staged next to the package (pre-cache_dir layouts, e.g. baked
  container images), instead of silently re-cloning multi-GB trees
- OpenHands setup dir is keyed by commit only when the value is an
  immutable hex object id; HEAD/branch refs keep the shared tree and the
  ensure-commit flow, and the why-OpenHands-only rationale is documented
- results_dir/cache_dir are validated (non-empty strings) and normalized
  to absolute paths at parse time so children with different cwds agree
- new maybe_get_global_config_dict() consults the config without ever
  falling through to a CLI parse; setup_root uses it so bare processors
  can't trigger hydra
- document both keys (fern configuration reference) and the swe_agents
  artifact locations/cleanup story; note why uv_venv_dir stays anchored
  at WORKING_DIR
- tests: setup_root default/override, legacy fallback precedence,
  sha vs mutable-ref keying, path normalization, non-string rejection

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Michal Futrega <michal.futrega@gmail.com>
…lass, synthetic sha in tests

_is_pinned_commit had landed between @contextmanager and file_lock,
stealing the decorator (every 'with file_lock' then failed); the new
test class had split TestBaseDatasetHarnessProcessor mid-body; the
pinned-commit test sha tripped the hex-high-entropy secrets detector.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Michal Futrega <michal.futrega@gmail.com>
… pre-staged trees, multinode path docs

- resolve_setup_dir: a pre-staged install-relative tree always wins while
  it exists (presence is fixed at image-build time, so resolution is
  stable for a deployment's lifetime and a crashed cache attempt can't
  flip it); log the decision
- OpenHands: single _openhands_setup_target — a valid pre-staged tree
  wins; anything else builds under the (sha-keyed) cache target, so the
  rmtree rebuild can never rewrite a shared pre-staged tree other nodes
  execute from
- rebench postprocess uses the setup dir resolved at server startup
  (carried in the instance config) instead of re-resolving at verify time
- docs: generated commands embed these host paths and run on other nodes
  (Ray SPREAD), so results_dir and cache_dir must be same-path-visible on
  every rollout node; head-node-local caches break remote rollouts; log
  the per-run results root at startup

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Michal Futrega <michal.futrega@gmail.com>
…of legacy trees, opt-in docs

- agent_framework_commit is resolved to a full commit SHA against the
  configured repository at startup (git ls-remote for branches/HEAD; full
  40-hex SHAs pass through, anything unresolvable fails fast), and the
  OpenHands cache tree is keyed by repository identity + that SHA — runs
  configured with different refs or forks can never reset a tree that
  running containers still execute from; the checkout sync now targets
  the resolved SHA, so a moved remote branch can't be mistaken for
  current
- the pre-staged-tree fallback applies only while cache_dir is left at
  its default: an explicitly configured cache_dir wins (resolve_setup_dir
  and the OpenHands target both gate on it)
- document results_dir/cache_dir as opt-in keys with their exact current
  consumers (swe_agents, wandb dir, uv_cache_dir default); other artifact
  producers and the RESULTS_DIR/CACHE_DIR constants are unchanged
- tests: repo+SHA keying, explicit-cache_dir opt-out at both call sites,
  ls-remote resolution (passthrough, resolve, no-repo error, unresolvable
  error)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Michal Futrega <michal.futrega@gmail.com>
…; pin a SHA in the test fixture

Round 3's eager ref resolution ran before the pre-staged tree was even
considered, so the default config (agent_framework_commit=HEAD, no repo)
failed at server start — breaking every wrapper-constructing test and
regressing baked images that legitimately run with the default config.
setup() now uses a valid pre-staged tree as shipped when there is no
remote to resolve against (the old local-HEAD sync was a no-op there
anyway); everything else keeps the strict resolve-then-key behavior.
The shared test fixture pins a synthetic full SHA, which short-circuits
resolution without network.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Michal Futrega <michal.futrega@gmail.com>
With results on a shared root, the per-instance persistent_dir may not
exist yet on the executing node when the log dir is created.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Michal Futrega <michal.futrega@gmail.com>
@ananthsub
ananthsub force-pushed the mfutrega/swe-agents-base-results-dir branch from 34a0748 to d5c38c1 Compare August 18, 2026 22:53
@ananthsub
ananthsub enabled auto-merge (squash) August 18, 2026 22:54
@ananthsub
ananthsub merged commit 082ebb1 into NVIDIA-NeMo:main Aug 18, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sla:review-overdue Review response is over the one-business-day SLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants