Skip to content

perf(huggingface_hub): optimize how dict attributes are set#591

Merged
Abhijeet Prasad (AbhiPrasad) merged 1 commit into
mainfrom
fix/huggingface-hub-drop-eager-serialization
Jul 17, 2026
Merged

perf(huggingface_hub): optimize how dict attributes are set#591
Abhijeet Prasad (AbhiPrasad) merged 1 commit into
mainfrom
fix/huggingface-hub-drop-eager-serialization

Conversation

@starfolkai

@starfolkai starfolkai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Audit of py/src/braintrust/integrations/huggingface_hub/ against .agents/skills/sdk-integrations/SKILL.md. Two related fixes:

  • Drop eager _try_to_dict in stream tool-call merge. _merge_tool_call_delta was running _try_to_dict(entry) (Pydantic model_dump("python")) on every streaming tool-call delta chunk just to read index / id / type / function fields. SpanImpl.log_internal already runs bt_safe_deep_copy at send time, so the eager conversion was a redundant recursive walk on the hot path. Same pattern as fix(cohere): drop redundant _try_to_dict pass in tool-call iteration #586 (cohere) and fix(google_genai): add provider metadata, route tools to metadata, drop eager serialization #590 (google_genai).
  • Remove the _get_field(obj, key) dict-or-attr helper. HuggingFace's BaseInferenceType (parent of every response type and stream chunk on both matrix pins — latest=1.23.0 and floor=0.32.0) inherits from dict:
    >>> from huggingface_hub.inference._generated.types.base import BaseInferenceType
    >>> BaseInferenceType.__mro__
    (BaseInferenceType, dict, object)
    
    So .get() works uniformly on every response, chunk, delta, and nested field — the dict-or-attr dispatch was dead cleverness. The two places that read from an InferenceClient instance now use getattr(instance, key, None) directly.

Net diff: 60 insertions / 89 deletions, one file. Also picked up two small invariants that were previously implicit via _get_field:

  • Tool-call-delta loop now checks isinstance(entry, dict) (tighter than is None) and drops the corresponding isinstance(incoming_fn, dict) re-check inside.
  • _text_generation_extra_metadata gets an explicit if details is None: return {} guard at the top.

Everything else was already SKILL-aligned and needed no change:

  • metadata.provider always set (kwarg > instance > "huggingface" fallback), metadata.model set when known.
  • Tool definitions routed to metadata.tools via _CHAT_METADATA_KEYS allowlist, not into input.
  • All metadata extraction is allowlist-based — no denylists anywhere in the file.
  • context.span_origin.instrumentation.name = "huggingface-hub-auto" via local start_span helper.
  • Only spec-listed metric keys.
  • One accumulated span per stream, errors propagate, setup + wrap_huggingface_hub idempotent.

Test plan

  • CI=1 BRAINTRUST_TEST_PACKAGE_VERSION=latest pytest src/braintrust/integrations/huggingface_hub/test_huggingface_hub.py — 24/24 passed (huggingface-hub 1.23.0)
  • CI=1 BRAINTRUST_TEST_PACKAGE_VERSION=0.32.0 pytest src/braintrust/integrations/huggingface_hub/test_huggingface_hub.py — 22 passed, 2 pre-existing skips for text_generation on that pin (_skip_if_text_generation_unavailable)
  • No cassettes re-recorded, no new mocks/fakes — used the existing VCR-backed suite.

🤖 Generated with Claude Code

Created by abhijeet

Slack thread

…ge, remove _get_field indirection

- `_merge_tool_call_delta` no longer runs `_try_to_dict` (Pydantic
  `model_dump`) on every streaming tool-call delta chunk — `SpanImpl.log_internal`
  already runs `bt_safe_deep_copy` at send time. Same pattern as #586 (cohere)
  and #590 (google_genai).
- Removed the `_get_field(obj, key)` dict-or-attr helper. HuggingFace's
  `BaseInferenceType` (parent of every response type and stream chunk on
  both matrix pins, `latest`=1.23.0 and floor `0.32.0`) inherits from
  `dict`, so `.get()` works uniformly on every response / chunk / delta.
  The two places that read from an `InferenceClient` instance now use
  `getattr(instance, key, None)` directly.
- Net: 89 deletions / 60 insertions; also tightens the tool-call-delta
  loop to `isinstance(entry, dict)` and adds an explicit
  `if details is None: return {}` guard at the top of
  `_text_generation_extra_metadata` that was previously implicit via
  `_get_field`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@AbhiPrasad Abhijeet Prasad (AbhiPrasad) changed the title fix(huggingface_hub): drop eager _try_to_dict in stream tool-call merge, remove _get_field indirection perf(huggingface_hub): optimize how dict attributes are set Jul 17, 2026
@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) merged commit a978bb1 into main Jul 17, 2026
82 checks passed
@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) deleted the fix/huggingface-hub-drop-eager-serialization branch July 17, 2026 19:11
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.

2 participants