Repository navigation
feat(backends): Add native Hugging Face transformers backend resolver - #168
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTrustLens adds Hugging Face text-classification pipeline support, including dependency extras, framework detection and dispatch, prediction resolution, metadata generation, and integration tests. ChangesHugging Face backend
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Sequence Diagram(s)sequenceDiagram
participant Pipeline as Hugging Face Pipeline
participant Resolver as huggingface.resolve
participant Report as TrustReport
Pipeline->>Resolver: Provide text inputs and classification outputs
Resolver->>Resolver: Parse probabilities and derive labels
Resolver->>Report: Return PredictionBundle metadata
Report-->>Resolver: Preserve resolved or manual predictions
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Approve with suggestions
This PR adds a well-structured HuggingFace transformers backend resolver, closely following existing patterns. However, it exposes a pre-existing core pipeline bug in binary calibration that crashes on string labels, and has two untested code paths in the new resolver. See the table below for details.
🌟 Strengths
- Solid test coverage with mocked pipelines and an integration test.
- Lazy imports and optional dependency configuration correctly follow project conventions.
| Priority | File | Category | Impact Summary (≤12 words) | Anchors |
|---|---|---|---|---|
| P2 | trustlens/backends/huggingface.py |
Bug | Exposes binary calibration crash on string labels | path:trustlens/core/pipeline.py, symbol:_encode_labels_for_probability_columns |
| P2 | trustlens/backends/huggingface.py |
Testing | Untested fallback label order derivation path | |
| P2 | trustlens/backends/huggingface.py |
Testing | No test for pre-supplied y_prob parameter |
🔍 Notable Themes
- Pre-existing core bug exposed: The new resolver returns string-encoded y_pred, triggering a crash in the binary calibration path of
_run_analysis_pipeline(missing label encoding). This is a pre-existing issue that should be tracked separately. - Untested code paths: Two less common code paths in the resolver (fallback label ordering from output and pre-supplied y_prob) lack coverage, reducing confidence for edge cases.
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
trustlens/backends/huggingface.py (1)
44-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOrdered-label derivation from
id2labelis duplicated three times.The
sorted(id2label)+[id2label[i] for i in ordered_ids]pattern appears in_parse_pipeline_output(lines 52-53), they_predbranch ofresolve(lines 139-140), and theresolved_class_labelsbranch (lines 157-158) — the last two compute the identical array twice whenid2labelis truthy.♻️ Proposed refactor
+def _ordered_labels_from_id2label(id2label: dict) -> np.ndarray: + """Derive a stable, ordered label array from an id2label mapping.""" + ordered_ids = sorted(id2label) + return np.asarray([id2label[i] for i in ordered_ids]) + + def _parse_pipeline_output(model: Any, raw_output: list) -> tuple[np.ndarray, Optional[np.ndarray]]: ... if id2label: - ordered_ids = sorted(id2label) - ordered_labels = [id2label[i] for i in ordered_ids] + ordered_labels = list(_ordered_labels_from_id2label(id2label)) label2col = {label: col for col, label in enumerate(ordered_labels)}And in
resolve, reuse the same helper (and cache the result once) instead of recomputing it for bothy_predandresolved_class_labels.Also applies to: 84-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@trustlens/backends/huggingface.py` around lines 44 - 53, Centralize ordered label derivation in a shared helper used by _parse_pipeline_output and resolve, replacing each duplicated sorted(id2label) and label-list construction. In resolve, compute the ordered labels once when id2label is available and reuse that cached result for both the y_pred and resolved_class_labels branches, preserving existing fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@trustlens/backends/huggingface.py`:
- Around line 103-110: Update the ImportError message in the Hugging Face
dependency import block to reference the defined `nlp` extra instead of the
non-existent `transformers` extra, while preserving the existing torch
dependency hint and message context.
---
Nitpick comments:
In `@trustlens/backends/huggingface.py`:
- Around line 44-53: Centralize ordered label derivation in a shared helper used
by _parse_pipeline_output and resolve, replacing each duplicated
sorted(id2label) and label-list construction. In resolve, compute the ordered
labels once when id2label is available and reuse that cached result for both the
y_pred and resolved_class_labels branches, preserving existing fallback
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a601f962-d68d-4b41-8bc0-a80abffee85f
📒 Files selected for processing (4)
pyproject.tomltests/test_backend_huggingface.pytrustlens/backends/huggingface.pytrustlens/backends/registry.py
Khanz9664
left a comment
Khanz9664
left a comment
There was a problem hiding this comment.
Great work on this PR! This is a well-designed addition that fits the existing backend architecture very naturally. I reviewed the implementation, the follow-up commits, and the automated review feedback.
The follow-up changes addressed all of the actionable review comments:
- Fixed the installation instructions to use the correct
trustlens[nlp]optional dependency. - Added coverage for the
id2labelfallback path, including the inconsistent-label failure case. - Added dedicated tests for the pre-supplied
y_probpath (both 2D inputs and binary 1D normalization), ensuring the resolver correctly bypasses pipeline execution when probabilities are provided.
I also like that the resolver stays consistent with the existing backend design by:
- using lazy imports,
- returning a standard
PredictionBundle, - preserving semantic class labels through
id2label, - falling back to deterministic label ordering when
id2labelis unavailable, and - keeping Transformers as an optional dependency rather than expanding the core installation.
While testing this backend, you also uncovered two unrelated issues in the current TrustLens core:
-
Binary calibration with string labels — the binary calibration path currently doesn't encode semantic labels before calling the calibration metrics, unlike the multiclass path. This is a pre-existing pipeline issue that this backend simply exposes.
-
confidence_gapon 0%-accuracy predictions — the current implementation guards the 100%-accuracy case but not the symmetric 0%-accuracy case, allowing aNaNto propagate into Trust Score computation. Again, this is an existing core issue rather than something introduced by this PR.
I don't consider either of these blockers for the Hugging Face resolver itself, but I think they're worth tracking separately as follow-up issues.
From the backend perspective, this looks in very good shape.
One thing I'm still waiting on before approving is the GitHub Actions workflow. I haven't been able to verify the CI results yet, so I'll hold off on approval until the workflow completes successfully. Assuming the checks come back green, I don't have any further concerns with this implementation. Nice work!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_backend_huggingface.py`:
- Around line 194-205: Prevent shared mock-class mutation in both
test_huggingface_resolver_explicit_y_prob_2d and
test_huggingface_resolver_explicit_y_prob_1d_normalized: inject pytest’s
monkeypatch fixture and replace each direct model.__class__.__call__ assignment
with monkeypatch.setattr(model.__class__, "__call__", _boom). Apply this change
at tests/test_backend_huggingface.py lines 194-205 and 217-226.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7cc31921-d856-4ac8-8533-f170b8e6c3ea
📒 Files selected for processing (2)
tests/test_backend_huggingface.pytrustlens/backends/huggingface.py
🚧 Files skipped from review as they are similar to previous changes (1)
- trustlens/backends/huggingface.py
|
Thanks! The implementation itself looks good from my review, but GitHub Actions is still reporting failures for |
…Probha18/TrustLens into feat/transformers-resolver
|
hey @Khanz9664 |
Khanz9664
left a comment
Khanz9664
left a comment
There was a problem hiding this comment.
Thanks for the update! I can see the linting issues are resolved now. However, the Type Checking workflow is still failing with one remaining mypy error:
trustlens/backends/huggingface.py:50:
Returning Any from function declared to return "ndarray[Any, dtype[Any]]"
Once that final type-checking issue is addressed and the workflow is green, I'll be happy to approve.
Hey @Khanz9664 |
Khanz9664
left a comment
Khanz9664
left a comment
There was a problem hiding this comment.
Thanks for working through all the follow-up feedback and CI issues.
I pulled the latest changes into my review, and the remaining type-checking issue has now been resolved. With the implementation review complete and all GitHub Actions checks passing, I don't have any further concerns.
This backend integrates cleanly with the existing resolver architecture, keeps the optional dependency boundary intact, and preserves the standard PredictionBundle interface expected by the pipeline.
Nice work, and thanks for iterating on the feedback. Approved.
|
Thanks for the excellent work on this contribution and for sticking with the review process all the way through to the finish line. This PR adds a substantial new capability to TrustLens by introducing native Hugging Face Transformers support while fitting cleanly into the backend abstraction introduced by the prediction resolver architecture. I particularly appreciated that the implementation remained consistent with the existing design principles:
Throughout the review process you were also very responsive to feedback. The follow-up commits addressed the installation documentation, expanded coverage for the One thing I also appreciated was that, while validating this backend, it exposed two unrelated edge cases in the existing TrustLens core (binary calibration with semantic labels and the After merging, I also pulled the latest Overall, this is a well-designed addition that integrates naturally with the existing architecture while maintaining backward compatibility and good test coverage. Thank you for your patience throughout the review iterations and for the quality of the implementation. Looking forward to your future contributions to TrustLens! |
…ibutor to contributors block of the README.md (#171)
2 participants
Summary
Adds a native backend resolver so
TrustLens.analyze()can audittransformers.TextClassificationPipelinemodels directly, passing raw string inputs instead of requiring users to manually extract logits, apply softmax, align labels, and convert everything to NumPy arrays. This mirrors the existing native resolvers forsklearn,xgboost,lightgbm, andcatboost.Related Issue
Type of Change
Please check the type of change:
Changes Made
trustlens/backends/huggingface.py(new) -resolve()implementation matchingsklearn.py's signature. Validates the model is aTextClassificationPipeline, runs it over raw text (unlessy_probis already supplied), parses the{'label', 'score'}output into a rectangular(n_samples, n_classes)probability matrix, and derives predictions. Column order is deterministic, sourced from the pipeline'smodel.config.id2labelwhenever available.y_predis mapped back to semantic string labels (viaid2label), not left as raw ordinal indices - mirrorssklearn.py'sclasses_arr[y_pred_indices]pattern and the existing raw-xgboost.Boosterclass_labelsmapping, so equality-based downstream metrics (misclassification_summary,class_imbalance_report,subgroup_performance, ...) work correctly against a naturally string-labeledy_true. Alltransformersimports are insideresolve(), following the existing lazy-import convention.trustlens/backends/registry.py-"transformers": "huggingface"added toFRAMEWORK_MAPPING,"huggingface"added toIMPLEMENTED_RESOLVERS, lazy-import dispatch branch added toget_resolver(). No other logic touched.trustlens/backends/types.py- unchanged; resolver reusesPredictionBundle/UnsupportedModelErroras-is.pyproject.toml- new standalonetransformersextra (single-package, matching thexgboost/lightgbm/catboostconvention), added tofull, reuses the existingtorchextra rather than duplicating it. Install viapip install trustlens[nlp].tests/test_backend_huggingface.py(new) - detection, basic resolver, end-to-endanalyze()integration, manual override, and rejection-of-non-classification-pipeline tests, mirroringtest_backend_xgboost.py's structure. All mocked (subclassed real pipeline classes with__init__bypassed), no network access, no real model weights.Testing
Pre-submit Checklist
Before submitting, please ensure you have:
pre-commit run --all-filesand all hooks passed.make test).CHANGELOG.mdif applicable.Notes for Reviewers
While building out the integration test, I ran into two behaviors in the core pipeline/metrics that seem unrelated to this backend but affect any framework. Not fixed here - flagging to check whether they're intentional or worth separate issues:
1. Binary calibration crashes on string
y_true.In
trustlens/core/pipeline.py::_run_analysis_pipeline, the multiclass calibration branch (y_prob.shape[1] > 2) encodesy_truevia_encode_labels_for_probability_columnsbefore computing anything. The binary branch (shape[1] == 2) does not - it passesy_truestraight intobrier_score/expected_calibration_error/maximum_calibration_error/reliability_curve, all of which donp.asarray(y_true, dtype=float). A binaryy_truelike["POSITIVE", "NEGATIVE"]raisesValueError: could not convert string to float. Is binary calibration expected to require pre-encoded0/1labels, or should the binary path also route throughclass_labelsencoding like the multiclass path does?2.
confidence_gapdivides by an empty set on 0%-accuracy predictions.In
trustlens/metrics/failure.py:This only guards against
incorrect_confbeing empty (100% correct). It doesn't guard againstcorrect_confbeing empty (100% wrong) -.mean()on an empty array returnsNaN, which propagates through tocompute_trust_score'sint(round(np.clip(raw_score, 0, 100)))and raisesValueError: cannot convert float NaN to integer. Net effect: any model with 0% accuracy on the eval set crashesanalyze()entirely, regardless of framework or label type. Is this expected, or should this degrade gracefully the same way thelen(incorrect_conf) > 0branch already does for the opposite extreme?Summary by CodeRabbit
nlpinstallation extra (and updatedfull) to include Transformers (transformers>=4.30).y_pred/y_prob(including 1D and 2D probability inputs).