[jdsika]

What

Fix an UnboundLocalError that makes every benchmark selection containing carla_cosim crash, and
enable the ruff lint hook — scoped to exactly the class of defect that bug belongs to — so the
next one is caught by a tool instead of a reviewer.

Why

load_benchmark_config appends parsed co-sim benchmarks to resolved_benchmarks, which is not
assigned until after the loop that does the appending:

for name in selected_names:
    ...
    resolved_benchmarks.append(parse_cosim_benchmark(name, benchmark))   # not yet bound
    continue
    ...
resolved_benchmarks = normalize_puffer_drive_benchmarks(...)

pufferlib/config/evaluation/benchmark.yaml ships a carla_cosim benchmark, so this is reachable
from the repo's own config, not a hypothetical:

UnboundLocalError: cannot access local variable 'resolved_benchmarks' where it is not associated with a value

Parsed co-sim entries now accumulate in their own list and are appended to the normalized replay
benchmarks, so a selection may mix both kinds — which is what the continue was always reaching for.

The second half is the reason this is one PR and not two. The bug is a name that does not resolve:
a linter finds it for free, and the ruff hook that would have found it has been sitting commented
out in .pre-commit-config.yaml since it was added. Enabling it is what turns the fix into a class
of bugs that cannot come back, and the fix is what lets the hook be enabled at all.

Why the lint is narrow

Under ruff's full default rule set the repo has 104 violations. 56 of them are E402 (imports
not at top of file), which is mostly deliberate sys.path manipulation in scripts and would need
real restructuring; the rest include 26 F841 and 8 E722, where "fixing" them means changing
exception and binding semantics in code this PR has no business touching.

So ruff.toml selects a deliberately narrow set — E9, F401, F811, F821, F822: names that
do not resolve, and imports that are never used. That is green today, it is the tier where a hit is
almost always a genuine defect rather than a style opinion, and widening it later is a decision that
can be taken alongside the fixes a wider set would demand.

How tested

• pre-commit run --all-files
• make rebuild (required if .c/.h changed — zero new warnings)
• make test-unit / make test-eval
• make test-c (C sim logic)
• make test-notebooks (notebooks touched)
• make test-docker-smoke (dynamics, rewards, observations, or training loop touched)
• make -C tests/drive perf (hot paths touched)
• make test before marking ready (unit + eval + c + notebooks — does not include
  docker-smoke or perf; those are the two boxes above)

$ pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
ruff check...............................................................Passed
ruff format..............................................................Passed
clang-format.............................................................Passed

$ python -m pytest tests/unit_tests -q
4 failed, 166 passed, 21 skipped in 205.56s (0:03:25)

The 4 failures are pre-existing and unrelated: 3 need pyarrow, which is not installed in this
checkout's venv, and 1 is the fused_slot_encoder/triton breakage fixed in #13. Confirmed by
running the same 4 on a clean tree with this commit stashed — identical failures, so this change
introduces none of them.

make test-notebooks could not run here (nbclient missing), but this commit removes two unused
imports from notebooks/notebook_utils.py and pufferlib/ocean/cosim/carla_cosim.py; the
Notebook Execution check covers it.

New behaviour is covered by a new test / the fix has a test that fails without it:

tests/unit_tests/test_cosim_config_contract.py::TestCosimBenchmarkResolution::test_shipped_carla_cosim_benchmark_resolves
— it loads the shipped benchmark.yaml and selects carla_cosim. Verified it fails without
the fix and passes with it:

$ git stash push -- pufferlib/ocean/evaluation_utils/evaluation_utils.py
$ python -m pytest tests/unit_tests/test_cosim_config_contract.py -q -k shipped_carla
>               resolved_benchmarks.append(parse_cosim_benchmark(name, benchmark))
E               UnboundLocalError: cannot access local variable 'resolved_benchmarks' where it is not associated with a value
1 failed, 10 deselected in 35.82s

$ git stash pop
$ python -m pytest tests/unit_tests/test_cosim_config_contract.py -q
11 passed in 58.49s

Stack hygiene

• forkctl check-commit passes (--chore on a fork-only branch)
• Conventional Commits subject, rebased on current integration/3.0

== check-commit
  PASS  exactly one commit on the branch
  PASS  rebased directly onto origin/integration/3.0
  PASS  subject is well formed: 'fix(eval): resolve carla_cosim benchmarks and lint for unresolved names'
  PASS  fork-only markers are well formed
  PASS  fork-only state is intact
  PASS  no registered fork-only file in the diff
  PASS  fork-only branch filters are untouched in shared workflows
  PASS  fork-only blocks in shared files are unchanged
== check-commit: OK

Merge gate

• Gate 1 — CI green on the final commit (no "green except a flake")
• Gate 2 — adversarial review posted by someone other than the author; findings fixed or
  explicitly accepted in the thread
• Gate 3 — upstream-readiness proven: forkctl mirror <slug> --sha HEAD passes, i.e. the
  commit cherry-picks onto pure upstream/3.0 and its diff is upstream-clean

Gate 3 result:

== mirror/enable-ruff-lint
        feature commit 67ba99968  fix(eval): resolve carla_cosim benchmarks and lint for unresolved names
  PASS  cherry-picks onto pure upstream without conflict
  PASS  no registered fork-only file in the mirror diff
  PASS  no fork-only marker added anywhere in the mirror diff
        mirror diff touches 14 file(s):
          .pre-commit-config.yaml
          data_utils/download_womd_data.py
          notebooks/notebook_utils.py
          pufferlib/__init__.py
          pufferlib/ocean/__init__.py
          pufferlib/ocean/evaluation_utils/evaluation_utils.py
          pufferlib/ocean/evaluation_utils/wosac/estimators.py
          pufferlib/ocean/cosim/carla_cosim.py
          pufferlib/pufferl.py
          ruff.toml
          scripts/eval/analyze_nuplan_cosim.py
          tests/unit_tests/test_cosim_config_contract.py
          tests/unit_tests/test_map_metrics.py
          tests/unit_tests/test_terminate_on_goal.py
== mirror/enable-ruff-lint: OK

Upstream disposition

• Mirror PR to Emerge-Lab/PufferDrive requested now — needs a human to verify the final
  title, body and diff first, and the body must carry the authorship disclosure from
  .github/fork/PROCESS.md → Agent authorship
• Contributable, but we hold it for now, because: the crash is upstream's and the fix applies
  cleanly to pure upstream/3.0, but opening an upstream PR needs explicit human approval of
  the title, body and diff (§5). The lint half is more of a judgement call for them than the
  fix half, and may be worth splitting if they want only one of the two.
• Never upstream — fork-only chore(fork): change, Gate 3 exempt

Risk / notes

Enabling the hook removed 8 genuinely unused imports across data_utils/download_womd_data.py,
notebooks/notebook_utils.py, pufferlib/ocean/cosim/carla_cosim.py,
pufferlib/ocean/evaluation_utils/wosac/estimators.py, scripts/eval/analyze_nuplan_cosim.py
and tests/unit_tests/test_map_metrics.py. Each was reviewed by hand: none is a re-export and
none has an import side effect, so removing them is inert.

Four import-availability probes gained # noqa: F401 — gymnasium/pygame in
pufferlib/__init__.py, torch and the Policy re-export in pufferlib/ocean/__init__.py, and
pufferlib._C in pufferlib/pufferl.py. These exist precisely to be imported and not used, so the
marker is the honest answer rather than a restructure.

tests/unit_tests/test_terminate_on_goal.py called pytest.main without importing pytest,
so its __main__ entry point had never worked. Now imported, matching every other test module.

The hook id is ruff-check, not the ruff alias the commented-out block used: the alias still
works at the pinned rev but prints a deprecation warning on every run.

--fix is kept from the original commented-out config, so the hook repairs what it safely can
rather than only complaining. Unsafe fixes stay off.

Ordering note: this PR and #13 are independent and both cut from integration/3.0; whichever lands
first, the other rebases cleanly (disjoint files).

[jdsika]

Gate 1 note — the two red checks are not this PR's.

Unit Tests and Notebook Execution fail here on test_drive_mean_action_is_exact_in_physical_units with ImportError: fused_slot_encoder=true requires triton. That is a repository-wide defect inherited from upstream (fused_slot_encoder: true in the shipped config, upstream PR Emerge-Lab#601) and it is red on every open PR, including ones that touch only documentation.

It is fixed by #13, which is now 12/12 green. Once #13 is stacked and this branch is rebased, Gate 1 can be satisfied here.

[jdsika]

Adversarial review (Track A) — posted on behalf of a fresh-context reviewer with no authoring history on this change, per .github/copilot-instructions.md §4.7 and §5.

Reviewed head bb5d3b03e; its patch is byte-identical to the originally reviewed d6b24e69d (the rebase changed only the base).

Verdict: approve as-is

No blocking, should-fix or optional code findings.

What was probed

Including the probes that found nothing, as §4.7 requires.

• Every source hunk — before/after AST comparison. Outside the benchmark-resolution fix and its new test, executable statements are unchanged apart from imports.
• The original crash — executing the unfixed base reproduces UnboundLocalError when selecting the shipped carla_cosim benchmark. The fix has a test that fails without it.
• Regression suite — the co-simulation contract module executed from immutable source blobs: 11 tests passed.
• Mixed selections — co-sim-only, native-only, both selection orders, interleaved, and duplicated. All resolve.
• Ordering — mixed selections return native benchmarks first, then co-sim, rather than the original cross-type selection order. Traced through the evaluator; no correctness or numerical regression established.
• Replay compatibility — native-only results compared against the base, including map-directory and scenario-count overrides, duplicates and whitespace-separated selections. Identical.
• Determinism and mutation — repeated mixed resolution returns identical values, preserves both the Python and NumPy RNG states, and does not mutate the input mapping.
• Fail-fast input handling — empty/invalid selections, unknown names, duplicate benchmark definitions, missing environment mappings, malformed and empty route lists, nonpositive ports and unsupported environment keys all raise explicit errors.
• Hot paths — no change to C stepping, observations, rewards, policy computation or training batching. The new list accumulation happens during configuration loading.
• Removed imports — contexts, retained dependency imports and consumers inspected. No lost initialisation side effect and no required re-export.
• Suppressions — all five added annotations are scoped to F401 availability/initialisation imports or the Policy export. No per-file or sim-wide suppression was added.
• Rule selection — the pinned ruff flags the original defect as F821 and duplicate classes as F811. The selection is narrow enough not to impose unrelated restructuring.
• Full lint and format — all 164 tracked Python blobs. On the rebased head: all checks pass, 164 files already formatted.
• Hook wiring — lint precedes formatting; both hooks pin v0.12.12, share file-type coverage and discover the root configuration; both retain Python 3.10 / 120 columns. No version skew.
• Automatic fixes — unused imports are removable, annotated probes survive, and the duplicate-class defect is not auto-deleted with unsafe fixes disabled. pre-commit rejects a commit when hooks modify files rather than silently staging rewrites.
• Merge-result CI — the earlier red check contained only the F811 inherited from the base, since fixed there. The rebased merge tree equals the reviewed head's tree and its full pre-commit workflow passes, clang-format included.

Stated verification limits: the source-rewriting pre-commit run --all-files was not executed locally (check-only commands plus the real CI run were used instead), and real CARLA / SLURM / nuPlan execution was not exercised.

Track

forkctl track --base 74a648b67 --head bb5d3b03e reports A, forced by six changed paths under pufferlib/. The reviewer agrees.