Skip to content

advisor: enforce manifest-authoritative runtime adapter loading#51

Merged
sarvesh1327 merged 2 commits into
mainfrom
manifest-authoritative-runtime-adapter
Apr 24, 2026
Merged

advisor: enforce manifest-authoritative runtime adapter loading#51
sarvesh1327 merged 2 commits into
mainfrom
manifest-authoritative-runtime-adapter

Conversation

@sarvesh1327
Copy link
Copy Markdown
Owner

  • Add a bounded Phase 7 runtime adapter-loading proof plan.
  • Make MLX runtime validate active checkpoint checkpoint.json manifest artifact_paths.adapter_model as the source of truth.
  • Add regression coverage proving runtime refuses missing manifest adapter artifacts even when stray adapter files exist in the checkpoint directory.
  • Preserve legacy checkpoint-directory adapter scanning only when no manifest adapter_model is present.

Testing

  • pytest tests/agent/advisor/test_runtime_mlx.py -k 'manifest_adapter_artifact or active_checkpoint_without_real_adapter or active_profile_adapter_metadata or loads_active_profile_adapter' -q
  • pytest tests/agent/advisor/test_runtime_mlx.py -q
  • pytest tests/agent/advisor/test_training_runtime.py tests/agent/advisor/test_operator_runtime.py -q
  • ruff check .
  • python -m pytest tests/agent/advisor -q
  • `git diff --check

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5913ca2958

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread agent/advisor/runtime/runtime_mlx.py Outdated
adapter_model_path = Path(str(artifact_paths["adapter_model"])).expanduser()
if not adapter_model_path.is_absolute():
adapter_model_path = Path(checkpoint_metadata["checkpoint_path"]) / adapter_model_path
if not adapter_model_path.exists():
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Validate adapter_model as a file path

In resolve_manifest_adapter_artifact, existence is treated as sufficient validation, so malformed manifests like "adapter_model": "" (or a directory path) are accepted because the resolved checkpoint directory exists. I traced this through _ensure_loaded: the returned directory path becomes adapter_artifact_path, then adapter_dir is computed as its parent, and mlx_lm_load is called with the wrong adapter directory. This can cause active checkpoints to fail loading or load from an unintended location instead of failing fast; requiring a non-empty file path (e.g., is_file()) would prevent this.

Useful? React with 👍 / 👎.

@sarvesh1327 sarvesh1327 merged commit 9ed7ce6 into main Apr 24, 2026
1 check passed
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