Skip to content

Investigate instructor client import errors#1818

Merged
jxnl merged 1 commit intomainfrom
cursor/investigate-instructor-client-import-errors-5628
Sep 25, 2025
Merged

Investigate instructor client import errors#1818
jxnl merged 1 commit intomainfrom
cursor/investigate-instructor-client-import-errors-5628

Conversation

@jxnl
Copy link
Copy Markdown
Collaborator

@jxnl jxnl commented Sep 25, 2025

fix: re-expose instructor.client for backward compatibility

Describe your changes

This PR addresses the critical breaking change where instructor.client was no longer directly accessible, causing AttributeError for users.

The changes re-expose client in instructor/__init__.py to restore backward compatibility, allowing instructor.client.Instructor and instructor.client.AsyncInstructor to function again. Deprecation warnings are in place to guide users towards the new direct import paths. Additionally, an internal reference in instructor/providers/writer/client.py was updated for consistency.

Issue ticket number and link

#1816

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • If it is a core feature, I have added documentation.

Slack Thread

Open in Cursor Open in Web


Important

Re-expose instructor.client for backward compatibility and update internal references in from_writer().

  • Behavior:
    • Re-expose client in instructor/__init__.py for backward compatibility, allowing instructor.client.Instructor and instructor.client.AsyncInstructor to work.
    • Add deprecation warnings to guide users to new import paths.
  • Internal References:
    • Update return type in from_writer() in instructor/providers/writer/client.py to use instructor.Instructor and instructor.AsyncInstructor directly.

This description was created by Ellipsis for 91245da. You can customize this summary. It will automatically update as commits are pushed.

@cursor
Copy link
Copy Markdown

cursor Bot commented Sep 25, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Sep 25, 2025

Claude finished @jxnl's task —— View job


Pull Request Review

I've completed my review of this PR that addresses the critical breaking change where instructor.client was no longer accessible. Here's my comprehensive feedback:

Todo List:

  • Read and analyze the changed files
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Evaluate security concerns
  • Review test coverage needs
  • Provide comprehensive feedback

🟢 Overall Assessment: LGTM

This is a well-implemented backward compatibility fix that addresses a critical breaking change. The solution is minimal, clean, and follows good deprecation practices.

Code Quality & Best Practices

  1. Clean Implementation: The fix is minimal and surgical - only adding 5 lines to __init__.py and fixing one internal reference.

  2. Proper Deprecation Pattern: The existing client.py module (instructor/client.py:14) already includes appropriate deprecation warnings that guide users to the new import paths:

    warnings.warn(
        f"Importing from 'instructor.client' is deprecated and will be removed in v2.0.0. "
        f"Please update your imports to use 'instructor.core.client.{name}' instead",
        DeprecationWarning,
        stacklevel=2,
    )
  3. Consistent Export: Adding "client" to __all__ (instructor/init.py:66) ensures the module is properly exposed for discovery tools.

Bug Prevention

  1. Fixes Critical Issue: Resolves AttributeError when users try to access instructor.client.Instructor or instructor.client.AsyncInstructor.

  2. Internal Consistency: The change in instructor/providers/writer/client.py:30 removes an incorrect internal reference (instructor.client.Instructor) that should have been updated during the original refactoring.

  3. No Regression Risk: The changes don't modify any core functionality - they only add backward compatibility.

Performance Considerations

  1. Minimal Overhead: The from . import client import (instructor/init.py:74) is lightweight and only executed once during module initialization.

  2. Lazy Loading: The client.py module uses __getattr__ for lazy loading, which means deprecated imports only trigger warnings when actually accessed.

Security Concerns

No security implications - this is purely a backward compatibility shim that doesn't introduce new attack vectors or expose sensitive functionality.

Test Coverage ⚠️

Recommendation: While the fix is straightforward, consider adding a test to ensure the backward compatibility works correctly:

def test_backward_compatibility_client_import():
    """Test that instructor.client.Instructor still works with deprecation warning."""
    import instructor
    
    with pytest.warns(DeprecationWarning, match="Importing from 'instructor.client' is deprecated"):
        # This should work and issue a warning
        client_instructor = instructor.client.Instructor
        assert client_instructor is instructor.Instructor

Minor Suggestions

  1. Documentation: Consider updating any migration guides or release notes to mention this backward compatibility restore.

  2. Consistency Check: Verify that other provider clients don't have similar incorrect internal references to instructor.client.*.

Files Reviewed

  • instructor/__init__.py: Clean backward compatibility addition
  • instructor/providers/writer/client.py: Correct internal reference fix
  • instructor/client.py: Existing deprecation mechanism (context)

Verdict: ✅ Approved - This is a well-implemented fix for a critical breaking change. The code is clean, follows good deprecation practices, and has minimal risk.


@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Sep 25, 2025

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
instructor 91245da Sep 25 2025, 02:50 PM

@github-actions github-actions Bot added bug Something isn't working python Pull requests that update python code size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 25, 2025
@jxnl jxnl marked this pull request as ready for review September 25, 2025 15:56
@jxnl jxnl merged commit ed2d642 into main Sep 25, 2025
11 of 14 checks passed
@jxnl jxnl deleted the cursor/investigate-instructor-client-import-errors-5628 branch September 25, 2025 15:57
Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 91245da in 1 minute and 22 seconds. Click for details.
  • Reviewed 34 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. instructor/__init__.py:66
  • Draft comment:
    Re-exporting 'client' for backward compatibility is clear. Consider adding a deprecation warning if this alias is temporary.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 85% None
2. instructor/providers/writer/client.py:30
  • Draft comment:
    Return type annotation updated to use instructor.Instructor instead of instructor.client.Instructor for consistency with the re-export.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 85% None

Workflow ID: wflow_sh3CmJj1tFOv9WnN

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

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

Labels

bug Something isn't working python Pull requests that update python code size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants