Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/twinkle/server/tinker/common/megatron_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,13 @@ def load(self, checkpoint_dir: str, **kwargs):

# Create checkpoint manager with the token
checkpoint_manager = create_checkpoint_manager(token)

# Use resolve_load_path to handle path resolution
resolved = checkpoint_manager.resolve_load_path(checkpoint_dir)

if resolved.is_twinkle_path:
# Load from twinkle checkpoint
return super().load(name=resolved.checkpoint_name, output_dir=str(resolved.checkpoint_dir), **kwargs)
return super().load(name=resolved.checkpoint_name, output_dir=resolved.checkpoint_dir, **kwargs)
else:
# Load from hub
return super().load(name=resolved.checkpoint_name, **kwargs)
Expand Down
2 changes: 1 addition & 1 deletion src/twinkle/server/tinker/common/transformers_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def load(self, checkpoint_dir: str, **kwargs):

if resolved.is_twinkle_path:
# Load from twinkle checkpoint
return super().load(name=resolved.checkpoint_name, output_dir=str(resolved.checkpoint_dir), **kwargs)
return super().load(name=resolved.checkpoint_name, output_dir=resolved.checkpoint_dir, **kwargs)
else:
# Load from hub
return super().load(name=resolved.checkpoint_name, **kwargs)
10 changes: 4 additions & 6 deletions src/twinkle/server/utils/io_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -912,18 +912,16 @@ def resolve_load_path(self, path: str, validate_exists: bool = True) -> Resolved
f"Checkpoint not found or access denied: {path}"
)

# Get the checkpoint directory
checkpoint_dir = str(self.get_ckpt_dir(training_run_id, checkpoint_id))
# Get the checkpoint directory parent path (no checkpoint name in the path)
checkpoint_dir = self.get_ckpt_dir(training_run_id, checkpoint_id).parent

Comment on lines +915 to 917
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

resolve_load_path now returns the parent of get_ckpt_dir(...) (the weights/ or sampler_weights/ directory) rather than the specific checkpoint directory. This behavior is likely correct for callers that do os.path.join(output_dir, name), but the variable name checkpoint_dir and surrounding docstrings/comments become misleading. Please clarify the contract (e.g., document that this is the "output_dir" root) to avoid future misuse.

Copilot uses AI. Check for mistakes.
if validate_exists:
# Verify the directory exists
from pathlib import Path as PathLib
if not PathLib(checkpoint_dir).exists():
if not checkpoint_dir.exists():
raise ValueError(f"Checkpoint directory not found: {checkpoint_dir}")

return ResolvedLoadPath(
checkpoint_name=checkpoint_name,
checkpoint_dir=checkpoint_dir,
checkpoint_dir=checkpoint_dir.as_posix(),
is_twinkle_path=True,
training_run_id=training_run_id,
checkpoint_id=checkpoint_id
Expand Down
2 changes: 1 addition & 1 deletion src/twinkle/server/utils/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ async def verify_request_token(request: Request, call_next):
Returns:
JSONResponse with error if validation fails, otherwise the response from call_next
"""
authorization = request.headers.get("Authorization")
authorization = request.headers.get("Twinkle-Authorization")
token = authorization[7:] if authorization and authorization.startswith("Bearer ") else authorization
if not is_token_valid(token):
Comment on lines +25 to 27
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

verify_request_token now only checks Twinkle-Authorization. This breaks existing clients sending the standard Authorization header and can lead to token=None being passed into custom is_token_valid implementations. Consider accepting both headers (e.g., prefer Twinkle-Authorization if present, else fall back to Authorization) and returning a clear 401/403 when neither header is provided. Also update the docstring which still says it reads Authorization.

Copilot uses AI. Check for mistakes.
return JSONResponse(status_code=403, content={"detail": "Invalid token"})
Expand Down
1 change: 1 addition & 0 deletions src/twinkle_client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def init_tinker_compat_client(base_url: Optional[str] = None, api_key: Optional[
default_headers = {
"X-Ray-Serve-Request-Id": get_request_id(),
"Authorization": 'Bearer ' + api_key,
"Twinkle-Authorization": 'Bearer ' + api_key, # For server compatibility
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.

medium

With the addition of the Twinkle-Authorization header, the logic for constructing default headers is now duplicated between this file and src/twinkle_client/http/http_utils.py. To improve maintainability and ensure consistency, consider centralizing this header creation logic. For example, the _build_headers function in http_utils.py could be enhanced and used in both locations.

} | kwargs.pop("default_headers", {})

service_client = ServiceClient(base_url=base_url, api_key=api_key, default_headers=default_headers, **kwargs)
Expand Down
1 change: 1 addition & 0 deletions src/twinkle_client/http/http_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def _build_headers(additional_headers: Optional[Dict[str, str]] = None) -> Dict[
headers = {
"X-Ray-Serve-Request-Id": get_request_id(),
"Authorization": 'Bearer ' + get_api_key(),
"Twinkle-Authorization": 'Bearer ' + get_api_key(), # For server compatibility
Comment on lines 17 to +20
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

get_api_key() is called twice when building headers. Store it in a local variable and reuse it for both Authorization and Twinkle-Authorization to avoid repeated context/env lookups and ensure both headers are always identical.

Suggested change
headers = {
"X-Ray-Serve-Request-Id": get_request_id(),
"Authorization": 'Bearer ' + get_api_key(),
"Twinkle-Authorization": 'Bearer ' + get_api_key(), # For server compatibility
api_key = get_api_key()
headers = {
"X-Ray-Serve-Request-Id": get_request_id(),
"Authorization": 'Bearer ' + api_key,
"Twinkle-Authorization": 'Bearer ' + api_key, # For server compatibility

Copilot uses AI. Check for mistakes.
}

if additional_headers:
Expand Down
Loading