Skip to content

fix(slurm): restore plugin activation boundary - #930

Merged
nabinchha merged 5 commits into
feat/slurm-executionfrom
codex/slurm-plugin-activation-boundary
Sep 10, 2026
Merged

fix(slurm): restore plugin activation boundary#930
nabinchha merged 5 commits into
feat/slurm-executionfrom
codex/slurm-plugin-activation-boundary

Conversation

@nabinchha

@nabinchha nabinchha commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📋 Summary

Restore the client plugin activation boundary for Slurm generation. The runtime wrapper now starts the plugin-aware client worker in a fresh Python interpreter, so the verified dependency overlay is activated before plugin and Data Designer configuration modules are imported.

This is needed because the runtime wrapper has already imported Data Designer configuration modules before it reaches the client phase. Activating a dynamic plugin overlay later in that same interpreter cannot retroactively rebuild the already-imported configuration type registry. As a result, a valid plugin-provided column can pass scheduling and endpoint readiness but still be rejected when generation starts. A fresh worker interpreter makes overlay activation happen before the plugin and configuration imports that depend on it.

Post-#929 one-node acceptance established that:

  • The base path completes successfully.
  • The plugin path reaches full endpoint readiness, then fails during configuration validation without this change.
  • An allocation-local diagnostic reproduced premature configuration import as the cause.

Post-restack real one-node acceptance completed successfully: the plugin was installed and discovered, the model backend became ready, the fresh client worker generated all four requested records with the plugin-provided marker, and the winner was published automatically. The later review-response commit changes tests only, so the accepted runtime source tree is unchanged at the current head.

🔗 Related Issue

Post-#929 plugin activation regression; no separate issue.

🔄 Changes

  • Add a bootstrap-safe ClientWorkerProcess component that owns the fresh-interpreter contract without importing configuration, interface, plugin, or worker modules.
  • Compose that public component into the runtime client phase instead of hiding subprocess behavior in the runtime entrypoint.
  • Refresh Python's public import caches before inventorying an attempt-local overlay, covering filesystems that preserve a directory timestamp across installation.
  • Keep workspace leasing, candidate validation, and result publication in the runtime wrapper.
  • Inject the component directly where narrow runtime tests need a controlled process result.
  • Replace the synthetic plugin probe with an end-to-end regression that installs a locked fake-plugin wheel, runs preflight and _client through the default process component, verifies the generated plugin column, and verifies result/candidate publication.

🧪 Testing

  • make test passes (not run; Slurm-only change)
  • Unit tests added/updated
  • E2E tests added/updated (not applicable)
  • Default client-process plugin handoff regression
  • make test-slurm: 1,430 passed after restacking on the latest shared base
  • make check-slurm
  • make test-slurm-wheel-install
  • Pre-restack real one-node plugin acceptance: 4/4 records generated and plugin marker verified
  • Post-restack real one-node plugin acceptance on the unchanged runtime source tree: 4/4 records generated and plugin marker verified
  • git diff --check

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable; existing activation boundary is preserved)

@nabinchha
nabinchha requested a review from a team as a code owner September 10, 2026 14:18
@nabinchha nabinchha changed the title [Slurm] Restore plugin activation boundary for generation fix(slurm): restore plugin activation boundary for generation Sep 10, 2026
@nabinchha nabinchha changed the title fix(slurm): restore plugin activation boundary for generation fix(slurm): restore plugin activation boundary Sep 10, 2026
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness or repository-rule violations identified.

Summary

  • Introduces a bootstrap-safe ClientWorkerProcess abstraction with a fixed module invocation.
  • Delegates the runtime client phase to the fresh worker while retaining lifecycle and publication responsibilities in the runtime wrapper.
  • Invalidates import caches before inspecting an attempt-local dependency overlay.
  • Adds realistic coverage for installing, discovering, validating, and executing a plugin-provided column.

Diagram

sequenceDiagram
    participant Runtime as Slurm runtime wrapper
    participant Process as ClientWorkerProcess
    participant Child as Fresh Python interpreter
    participant Env as Dependency overlay
    participant Plugin as Plugin/config registry
    participant Engine as Generation engine

    Runtime->>Process: run(worker arguments)
    Process->>Child: python -m data_designer.slurm.client.worker
    Child->>Env: prepare and activate verified overlay
    Env-->>Child: overlay available on import path
    Child->>Plugin: import and discover plugin/config types
    Plugin-->>Child: plugin column registered
    Child->>Engine: validate configuration and generate records
    Engine-->>Runtime: candidate manifests and dataset
    Runtime->>Runtime: validate and publish candidate
Loading

Reviews (6) · Last reviewed commit: "test(slurm): exercise plugin worker hand..."

@nabinchha

Copy link
Copy Markdown
Contributor Author

Exact-head real one-node/one-GPU acceptance completed successfully.

  • Plugin dependency installed and was discovered during preflight.
  • The model backend reached readiness.
  • Generation ran through the fresh client-worker process.
  • All 4 requested records were produced with the plugin-provided marker verified.
  • The winning output was published automatically.

No internal environment identifiers are included here; detailed evidence is retained privately.

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
@nabinchha
nabinchha force-pushed the codex/slurm-plugin-activation-boundary branch from cccc66d to c33eb0d Compare September 10, 2026 16:03
Comment thread packages/data-designer-slurm/tests/runtime/test_entrypoint.py Outdated
@nabinchha

Copy link
Copy Markdown
Contributor Author

Post-restack exact-head real one-node/one-GPU acceptance also completed successfully. Plugin preflight, backend readiness, fresh-worker generation, 4/4 records with the plugin marker, and automatic winner publication all passed. Detailed environment evidence remains private.

@nabinchha
nabinchha requested review from a team and andreatnvidia September 10, 2026 16:17
Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
@nabinchha
nabinchha requested review from a team and andreatnvidia and removed request for a team and andreatnvidia September 10, 2026 17:01

@andreatnvidia andreatnvidia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated regression now exercises the full plugin handoff through the default worker process, including installation, generation, and result publication. The activation boundary looks correct, and the original concern is covered. Looks good to me.

@nabinchha
nabinchha merged commit dad27fe into feat/slurm-execution Sep 10, 2026
13 checks passed
@nabinchha
nabinchha deleted the codex/slurm-plugin-activation-boundary branch September 10, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants