fix(langchain): extract model name for ChatHuggingFace#1728
fix(langchain): extract model name for ChatHuggingFace#1728vismaytiwari wants to merge 2 commits into
Conversation
ChatHuggingFace is not LangChain-serializable, so it is passed to the callback as a not_implemented stub with no kwargs; the model id is only present in the repr string as model_id='...'. _extract_model_name had no entry for it, so generations from a ChatHuggingFace model were recorded without a model name. Add a repr-pattern entry that reads model_id.
|
|
||
| def test_extract_model_name_chat_huggingface(): | ||
| serialized = _chat_huggingface_serialized("Qwen/Qwen2.5-Coder-32B-Instruct") | ||
|
|
There was a problem hiding this comment.
Test only covers HuggingFaceEndpoint backend shape
The PR description correctly notes that ChatHuggingFace works with three backends — HuggingFaceEndpoint, HuggingFaceHub, and HuggingFacePipeline. HuggingFacePipeline itself has a model_id attribute in its own repr, so a wrapped repr could look like ChatHuggingFace(llm=HuggingFacePipeline(model_id='...', ...), model_id='...'). The regex uses re.search (first match), meaning it would pick up the inner HuggingFacePipeline's model_id rather than ChatHuggingFace's. In practice both should be the same value, but adding a parameterised test for each backend shape would guard against any future repr change where the two values might differ.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/test_langchain_utils.py
Line: 29-32
Comment:
**Test only covers HuggingFaceEndpoint backend shape**
The PR description correctly notes that `ChatHuggingFace` works with three backends — `HuggingFaceEndpoint`, `HuggingFaceHub`, and `HuggingFacePipeline`. `HuggingFacePipeline` itself has a `model_id` attribute in its own repr, so a wrapped repr could look like `ChatHuggingFace(llm=HuggingFacePipeline(model_id='...', ...), model_id='...')`. The regex uses `re.search` (first match), meaning it would pick up the inner `HuggingFacePipeline`'s `model_id` rather than `ChatHuggingFace`'s. In practice both should be the same value, but adding a parameterised test for each backend shape would guard against any future repr change where the two values might differ.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Good catch. I parameterised the test to cover all three backends (HuggingFaceEndpoint, HuggingFaceHub, HuggingFacePipeline). The HuggingFacePipeline case specifically exercises the double model_id repr you flagged, where re.search matches the inner one; since both ids resolve to the same model the extracted value is correct, and the test now guards against future drift between them.
What does this PR do?
Fixes langfuse/langfuse#14103
When a generation runs through
ChatHuggingFace, Langfuse recorded it without a model name and logged "Langfuse was not able to parse the LLM model".ChatHuggingFaceis not LangChain-serializable, so it reaches the callback as anot_implementedstub with nokwargs; the model id is only present in thereprstring asmodel_id='...'._extract_model_namehad no entry for it, so extraction fell through and returnedNone.This adds a repr-pattern entry that reads
model_id, matching how the other repr-only models (HuggingFaceHub,Ollama, etc.) are already handled.ChatHuggingFaceexposesmodel_idregardless of the underlying backend (HuggingFaceEndpoint,HuggingFaceHub, orHuggingFacePipeline), so this covers those cases.Type of change
Verification
I confirmed the model id is only available in the
reprby dumping a realChatHuggingFacevialangchain_core.load.dumpd, then wrote a deterministic unit test using that serialized shape so it needs no live HuggingFace call. The test returnsNonebefore the change and the correct model id after.Checklist
code_review.md..env.templateif needed.Greptile Summary
Adds
ChatHuggingFaceto the repr-based model-name extraction table so that Langfuse records a model name when LangChain callbacks arrive fromChatHuggingFace, which serialises as anot_implementedstub with the model id only available in thereprstring.langfuse/langchain/utils.py: Inserts(\"ChatHuggingFace\", \"model_id\", None)intomodels_by_pattern, following the identical approach already used forHuggingFaceHub,Ollama,DeepInfra, and others that expose their model identifier only viarepr.tests/unit/test_langchain_utils.py: Adds a new unit test file with a deterministic serialized stub matching the realdumpdshape ofChatHuggingFace, verifying the regex extracts the correctmodel_id.Confidence Score: 5/5
Safe to merge — a one-line addition to a lookup table with a corresponding unit test.
The change is minimal and self-contained: one new tuple in models_by_pattern using the same repr-regex mechanism already proven by multiple other entries. The unit test confirms the extraction works correctly for the primary backend shape. The only gap is that the HuggingFacePipeline-backend repr variant is not explicitly tested, but since both occurrences carry the same value in practice this is a test-coverage gap rather than a functional defect.
No files require special attention.
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[_extract_model_name called] --> B{Match by ID path\nmodels_by_id list} B -- match --> Z[Return model name] B -- no match --> C{AzureOpenAI\nspecial case?} C -- yes --> D[Extract from\ninvocation_params / kwargs] D --> Z C -- no --> E{Match by repr\nmodels_by_pattern list} E -- includes new ChatHuggingFace entry\nre.search model_id='.+' in repr --> F{repr contains\nmodel_id='...'?} F -- yes --> Z F -- no --> G[Return None default] E -- no match --> H{Catch-all path\nkwargs / serialized} H -- found --> Z H -- not found --> I[Return None]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[_extract_model_name called] --> B{Match by ID path\nmodels_by_id list} B -- match --> Z[Return model name] B -- no match --> C{AzureOpenAI\nspecial case?} C -- yes --> D[Extract from\ninvocation_params / kwargs] D --> Z C -- no --> E{Match by repr\nmodels_by_pattern list} E -- includes new ChatHuggingFace entry\nre.search model_id='.+' in repr --> F{repr contains\nmodel_id='...'?} F -- yes --> Z F -- no --> G[Return None default] E -- no match --> H{Catch-all path\nkwargs / serialized} H -- found --> Z H -- not found --> I[Return None]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(langchain): extract model name for C..." | Re-trigger Greptile