Skip to content

OpenConceptLab/ocl_issues#2388 | encoder_model param for reranking #839

Open
snyaggarwal wants to merge 2 commits intomasterfrom
issues#2388
Open

OpenConceptLab/ocl_issues#2388 | encoder_model param for reranking #839
snyaggarwal wants to merge 2 commits intomasterfrom
issues#2388

Conversation

@snyaggarwal
Copy link
Copy Markdown
Contributor

Linked Issue

Closes OpenConceptLab/ocl_issues#2388

@snyaggarwal snyaggarwal requested a review from paynejd April 6, 2026 05:59
Copy link
Copy Markdown
Member

@paynejd paynejd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

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.

Add option to configure a custom reranker in OCL Mapper Project Config

3 participants