Skip to content

Persist training-client user_metadata and return it from training_run - #19

Draft
kevintli wants to merge 1 commit into
mainfrom
devin/1790984689-training-run-user-metadata
Draft

kevintli wants to merge 1 commit into
mainfrom
devin/1790984689-training-run-user-metadata

Conversation

@kevintli

@kevintli kevintli commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

create_lora_training_client(user_metadata=...) was accepted and stored on the control-plane model record, but never reached checkpoints, so GET /training_runs/{id} had no user_metadata and Tinker's TrainingRun.user_metadata was always None. tinker_cookbook relies on this to recover a run's renderer from a checkpoint path (get_renderer_name_from_checkpoint_async reads user_metadata["renderer_name"]); on Spindle that always fell back to the caller's configured renderer.

Flow after this PR:

model spec {"user_metadata": {...}}           # already stored by the control plane
  -> parse_model_spec -> ModelSpec.user_metadata
  -> backend job state (megatron_lora / miles_lora / megatron_fft)
  -> checkpoint metadata.json {"user_metadata": {...}}   # existing writer, no schema change
  -> checkpoint entry["metadata"]
  -> _training_run(): {"user_metadata": metadata.get("user_metadata"), ...}

Also folds in the sampler-path lookup from #8 (now closed): sampler paths are minted as tinker://<model_id>:train:0/sampler_weights/... but checkpoints are filed under the bare model_id, so get_training_run_by_tinker_path(sampler_path) 404'd. training_run() now retries with :train:0 stripped when the literal ID has no entries.

Absent metadata stays None end to end; old checkpoints without the key return user_metadata: None.

Tests: control-plane unit test covers present/absent metadata and the :train:0 alias; the real-SDK e2e test creates a training client with user_metadata, then checks get_training_run and get_training_run_by_tinker_path(<sampler path>) both return it; Miles/FFT checkpoint-metadata tests assert the new key. Full suite: 696 passed, 1 skipped.

Link to Devin session: https://modal.devinenterprise.com/sessions/f53cfabb210146de8f0338fe388d7973
Open in Devin Desktop: https://modal.devinenterprise.com/desktop/session/f53cfabb210146de8f0338fe388d7973?variant=devin
Requested by: @kevintli

…om training_run

Also resolve training runs looked up by the sampler-path run ID (<model_id>:train:0).

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

This branch has not been deployed

No deployments
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.

1 participant