-
Notifications
You must be signed in to change notification settings - Fork 49
fix(llma): LangChain 1.0+ compatibility for CallbackHandler #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
posthog/ai/langchain/callbacks.py, line 31-41 (link)logic: inconsistent import strategy - these
langchain_coreimports will fail on older LangChain versions wherelangchain_coreisn't available, while lines 23-30 have fallbackseither these imports already worked in the previous version (suggesting
langchain_corewas always available), or they also need try/except fallbacks for true backward compatibility
1 file reviewed, 1 comment
Follow-up: CI Improvement SuggestionWhile implementing this fix, we noticed that CI currently only tests with LangChain 1.0+ (the versions in Proposed EnhancementAdd a matrix dimension to test both LangChain versions: tests:
name: Python ${{ matrix.python-version }} / LangChain ${{ matrix.langchain-version }} tests
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']
langchain-version: ['0.2', '1.0'] # NEW
steps:
# ... existing steps ...
- name: Install test dependencies
shell: bash
run: |
UV_PROJECT_ENVIRONMENT=$pythonLocation uv sync --extra test
- name: Install LangChain ${{ matrix.langchain-version }}
shell: bash
run: |
if [ "${{ matrix.langchain-version }}" = "0.2" ]; then
# LangChain 0.2.x (pre-1.0)
uv pip install --system 'langchain>=0.2.0,<1.0' 'langchain-community>=0.2.0,<0.3.0' 'langchain-openai>=0.1.0,<0.2.0' 'langchain-anthropic>=0.1.0,<0.2.0'
else
# LangChain 1.0+ already installed from test dependencies
echo "Using LangChain 1.0+ from test dependencies"
fi
- name: Run posthog tests
run: |
pytest --verbose --timeout=30Benefits
Trade-offs
Happy to implement this as a follow-up PR if the team thinks it's valuable! |
- Use try/except to import from langchain_core first (LangChain 1.0+) - Fall back to legacy langchain imports for older versions - Maintains backward compatibility with LangChain 0.x - All existing tests pass (45 passed) Fixes #362
- Tests that AgentAction and AgentFinish can be imported - Tests on_agent_action and on_agent_finish callbacks with mock data - Ensures compatibility with both LangChain 0.x and 1.0+ - Catches the import issue that was previously only tested with API keys This addresses a test coverage gap identified during code review.
e56f26b to
4ea52bb
Compare
The type: ignore comments were only needed when the except block executes, but CI runs with LangChain 1.0+ so the try block succeeds. Mypy flags these as unused-ignore errors.
|
@andrewm4894 |
I think for now it makes total sense to be backwards compatible and support both. As little friction for users as possible, its not that complex yet (if complexity grows can revisit). A lot of stuff last 12 months built on LC <1.0 so we should i think on balance be as flexible and easy for next 6 months or so and then can probably bump req to LC 1.0+ once most of the community on it. |
Radu-Raicea
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm approving this, but you need to bump the version first, before you merge.
Problem
The PostHog LangChain CallbackHandler is incompatible with LangChain 1.0+ due to deprecated import paths. LangChain 1.0 (released September 2024) moved modules from
langchain.callbacks.basetolangchain_core.callbacks.base.Fixes #362
Solution
Added backward-compatible imports using try/except:
langchain_core(LangChain 1.0+ and modern 0.x)langchainimports for older versionsThis approach maintains compatibility with both LangChain 0.x and 1.0+.
Testing
AgentAction/AgentFinishimportsTest Coverage Improvement
Added
test_agent_action_and_finish_imports()to address a gap in test coverage:langchain_corebut never testedAgentAction/AgentFinishtest_langgraph_agent) was skipped without API keysChanges