Skip to content

Sandbox: .source broken by nnsight bump — SourceEnvoy gained required args (6 remote tests) #281

Description

@JadenFiotto-Kaufman

Rebuilding the image picks up nnsight's 0.8 branch tip (requirements.txt pins the branch, not a commit). The current tip breaks .source on the untrusted path.

Symptom

tests/test_nnsight_remote.py against a live deployment, untrusted path:

6  TypeError: SourceEnvoy.__init__() missing 2 required positional arguments: 'source' and 'line'
2  AttributeError: 'NoneType' object has no attribute 'modules'      (TestRemoteCache)
2  OutOfOrderError: '...grad.i0'                                     (TestRemoteGradients — known, documented)
1  TestRemotePeft::test_adapter_applied_remotely                     (fails on the trusted path too — separate)

11 failed, 40 passed

The trusted path is 50 passed, 1 failed (only the peft one). docs/developing/testing.md records the untrusted baseline as "46 passed, 2 failed, 2 skipped", so the source and cache failures are new.

Cause (source)

sandbox/nns.py:313 constructs a SourceEnvoy with three arguments:

return SourceEnvoy(self._envoy, name, f"{self._prefix}.{name}")

SourceEnvoy.__init__ takes five — envoy, name, path, source, line. This worked because source and line had defaults at the commit ndif used to pin:

# nnsight ad939e9 (the old pin)
source: str = "",
line: int | None = None,

nnsight b16ca0c3 ("vLLM: CUDA-graph taps, controller-only handoff, one request record, TP/DCP fixes") removed the defaults, making both required. The signature is otherwise unchanged.

IPCSource already round-trips a SOURCE event to the host, which source-instruments the real module and returns the operation names. The fix is presumably to return source/line alongside those names and pass them through — the host has them; the runner does not.

Cause (cache) — less certain

The two TestRemoteCache failures are AttributeError: 'NoneType' object has no attribute 'modules'. nnsight's cache.py also changed in this range. I have not traced this one to a specific commit; flagging it as part of the same bump rather than claiming the mechanism.

Broader point

This is the second time a branch-tip nnsight bump has silently broken the sandbox — the first was Mediator.pending becoming a Pending NamedTuple. Both were invisible until the untrusted path was exercised, because the trusted path shares no code with the runner shim. Worth considering either pinning nnsight to a commit again (the comment in requirements.txt argues for it) or running tests/test_nnsight_remote.py -p conftest_untrusted in CI against a local stack.

Observed on the Test deployment. ndif 0.8 @ be38d78, nnsight 0.8 @ 3bed88cc (previously 5be282b6, which passed).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions