OpenConceptLab/ocl_issues#2388 | encoder_model param for reranking #839
OpenConceptLab/ocl_issues#2388 | encoder_model param for reranking #839snyaggarwal wants to merge 2 commits intomasterfrom
Conversation
paynejd
left a comment
There was a problem hiding this comment.
Review — OpenConceptLab/ocl_issues#2388
1. Narrow the exception handler (core/concepts/views.py)
The except Exception as e catches everything including SystemExit, OOM, and DB errors. Narrow to the specific failures that model loading/inference can raise:
except (ValueError, RuntimeError, OSError) as e:
return Response({'detail': str(e)}, status=status.HTTP_400_BAD_REQUEST)2. Change Closes to Ref in PR body
Both this PR and oclmap#8 say "Closes OpenConceptLab/ocl_issues#2388". Whichever merges first will auto-close the issue before the other ships. Suggest changing this PR body to "Ref OpenConceptLab/ocl_issues#2388" and letting oclmap#8 (the final piece) carry the Closes.
3. Consider API-level permission check for encoder_model
The issue specifies only core team users should access custom reranker config. The oclmap PR gates the UI on isCoreUser, but the $rerank/ endpoint accepts encoder_model from anyone — a non-core user could call the API directly with a custom model. Is this intentionally open at the API level? If not, add a permission check.
4. No input validation on encoder_model
An arbitrary string can be saved to the encoder_model field. The clean() method resets empty values to the default, but garbage strings persist in the DB and will cause errors on every subsequent rerank attempt. Consider a basic format check (e.g. must contain /, strip whitespace) or validate that the model can be loaded before saving.
5. Include the encoder model used in the rerank response
For traceability and debugging, include the model name in the rerank response so the frontend can log which model produced the scores for a given row:
return Response({'results': results, 'encoder_model': reranker.model_name or self.default_model})This supports the frontend logging which model was used per row — important when users are experimenting with different reranker models.
Related follow-up tickets filed:
- OpenConceptLab/ocl_issues#2463 — Upgrade
sentence-transformersfrom 3.3.1 to 5.4+ - OpenConceptLab/ocl_issues#2464 — Add Qwen3-Reranker models (blocked by #2463)
Linked Issue
Closes OpenConceptLab/ocl_issues#2388