feat: overridable results_dir/cache_dir roots; anchor SWE artifacts and setup trees on them - #2448
Merged
ananthsub merged 10 commits intoAug 18, 2026
Conversation
michal2409
force-pushed
the
mfutrega/swe-agents-base-results-dir
branch
from
August 10, 2026 18:23
45bbac7 to
97838e1
Compare
michal2409
force-pushed
the
mfutrega/swe-agents-base-results-dir
branch
2 times, most recently
from
August 10, 2026 18:45
6865cc4 to
28ccbd2
Compare
michal2409
force-pushed
the
mfutrega/swe-agents-base-results-dir
branch
from
August 11, 2026 10:18
28ccbd2 to
9a29669
Compare
michal2409
force-pushed
the
mfutrega/swe-agents-base-results-dir
branch
from
August 12, 2026 11:40
7516baf to
228af91
Compare
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
force-pushed
the
mfutrega/swe-agents-base-results-dir
branch
from
August 13, 2026 12:49
0c92995 to
3e61878
Compare
ananthsub
reviewed
Aug 13, 2026
…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
force-pushed
the
mfutrega/swe-agents-base-results-dir
branch
from
August 18, 2026 22:53
34a0748 to
d5c38c1
Compare
marta-sd
approved these changes
Aug 18, 2026
ananthsub
enabled auto-merge (squash)
August 18, 2026 22:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rewritten per review discussion — the original opt-in
results_root_dirfield carved out only one of the three kinds of state this server anchors atPath(__file__).parent: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
RESULTS_DIR/CACHE_DIRconstants as overridable top-level config keys (results_dir,cache_dir), defaulted with the samesetdefaultpattern asuv_cache_dirthree lines above, and added to the reserved-keys list. Any server can use them; nothing else changes unless a run overrides them.results_dir, all setup trees undercache_dir/swe_agents/via asetup_rootproperty on the shared processor base.parent_dirremains for packaged assets. Both reads fall back to the module constants when the config dict bypassed the parser defaults (env-provided dicts, tests).agent_framework_commit.file_lockis only held duringsetup(), so an unkeyed shared checkout could bereset --hardto a different commit by a later run while an earlier one was still executing from it.setup_root.Behavior note
Default locations move from the package directory to
WORKING_DIR/{results,cache}— the layout thenemo_gymmodule 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_dirdefault, and the W&B run dir. Import-time environment defaults (e.g.HF_HOME/HF_DATASETS_CACHE, set innemo_gym/__init__.pybefore any config is parsed) intentionally keep using the module constants.