-
Notifications
You must be signed in to change notification settings - Fork 284
Add handler logic to ner #774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the /classify and /classify_batch endpoints to support Named Entity Recognition (NER) by returning raw NER entities and linkable fields, and implements NER postprocessing and regex-based transaction annotation in the inference layer.
- Switch classification inputs from plain strings to
ClassifyInputwithoriginal_descriptionandamount, and add optionalparameters. - Change handlers to return a structured
ClassifyResponse(withraw_nerandlinkable_fields) instead of raw entity lists. - Introduce NER postprocessing utilities in
infer.rs(usinglazy_staticandRegex) and wire them intoInfer::classifyand streaming batch classify.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| router/src/server.rs | Updated HTTP handlers to consume ClassifyInput, record NER metrics, and return ClassifyResponse. |
| router/src/lib.rs | Defined ClassifyInput, ClassifyParameters, ClassifyResponse, and BatchClassifyResponse types. |
| router/src/infer.rs | Added NER postprocessing (postprocess_entity_rust, regex annotation, linkable-fields builder) and updated Infer methods. |
| router/Cargo.toml | Added lazy_static dependency for static regex initializations. |
Comments suppressed due to low confidence (6)
router/src/infer.rs:42
- [nitpick] Suffix
_RUSTon constant names is redundant and may confuse readers. Consider renaming toMIN_LENGTH_PER_ENTITYfor clarity and consistency.
static ref MIN_LENGTH_PER_ENTITY_RUST: HashMap<&'static str, usize> = {
router/src/infer.rs:49
- The key "store number" includes a space, whereas other entity types use single words or snake_case. Verify this matches upstream
entity_groupvalues or consider usingstore_numberfor consistency.
m.insert("store number", 5); // Note: Rust variable names typically use snake_case
router/src/infer.rs:1111
- Using
expecthere will panic the server if an ID is missing. Consider handling this case more gracefully (e.g., returning an error) to avoid runtime panics.
let request_id = id.expect("Classify response in batch missing ID. This is a bug.");
router/src/server.rs:1875
- The new
parametersfield inClassifyRequestis never accessed inside the handler. Consider validating or passing it to the inference layer if intended, or remove until needed.
Json(req): Json<ClassifyRequest>,
router/src/lib.rs:1221
- The optional
parametersfield is defined in the request types but never read or validated by the handlers. Consider wiring it through or removing until it’s needed.
struct ClassifyParameters {
router/src/server.rs:1952
- In the
utoipa::pathmacro,body = Vec<ClassifyResponse>may not be recognized as a valid type reference. Verify the OpenAPI schema generates correctly or use a wrapper type.
(status = 200, description = "Classifications", body = Vec<ClassifyResponse>),
| #[derive(Debug, Serialize)] | ||
| struct BatchClassifyResponse { | ||
| responses: Vec<ClassifyResponse>, | ||
| } | ||
|
|
Copilot
AI
May 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BatchClassifyResponse struct is added but never used by the handlers (they now return Vec<ClassifyResponse> directly). Consider removing it to reduce dead code.
| #[derive(Debug, Serialize)] | |
| struct BatchClassifyResponse { | |
| responses: Vec<ClassifyResponse>, | |
| } |
No description provided.