Skip to content

Efrat/fruit defect#335

Open
efrat99 wants to merge 2 commits intomainfrom
efrat/fruit-defect
Open

Efrat/fruit defect#335
efrat99 wants to merge 2 commits intomainfrom
efrat/fruit-defect

Conversation

@efrat99
Copy link
Copy Markdown
Collaborator

@efrat99 efrat99 commented Nov 10, 2025

No description provided.

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR restructures the fruit defect detection service and refactors the inference system to support multiple models. The fruit defect code was moved from services/inference_http/models/fruit_defect/ to a standalone services/fruit_defect/ directory, and the inference HTTP service was generalized to support multi-model deployment.

Key Changes

  • Service Reorganization: Moved fruit defect module to services/fruit_defect/ as a standalone service
  • Multi-Model Architecture: Refactored inference HTTP service to dynamically load model adapters via registry pattern instead of team-based routing
  • Flink Integration: Updated HTTP dispatcher to support multiple input topics with mapping.yml config for dynamic model routing
  • New Endpoints: Added /infer_upload/{name} endpoint for direct file uploads alongside existing /infer_json/{name} for MinIO references

Critical Issues Found

  1. services/inference_http/adapters/fruit_defect_runner.py - Contains model_registry.py code instead of the FruitDefectRunner class implementation. This will cause immediate runtime failure when the fruit_defect model is invoked.

  2. services/fruit_defect/configs/fruit_defect.yaml - Contains hardcoded Windows paths (C:/Users/...) that won't work in Docker/production environments.

  3. services/inference_http/Dockerfile - Copies empty models directory and missing dependency on services/fruit_defect needed for imports.

Minor Issues

  • Duplicate numpy==1.26.4 entry in requirements.txt
  • Import path changes needed: models.fruit_defectservices.fruit_defect
  • Method name change needed: run()predict() to match app.py expectations

Confidence Score: 0/5

  • UNSAFE: This PR contains critical bugs that will cause immediate runtime failures
  • Score of 0 reflects blocking issues: fruit_defect_runner.py contains completely wrong code (model_registry content instead of adapter class), hardcoded Windows paths in config, and missing Docker dependencies. The service will fail immediately when fruit_defect endpoint is called.
  • CRITICAL: services/inference_http/adapters/fruit_defect_runner.py (wrong file content), services/fruit_defect/configs/fruit_defect.yaml (Windows paths), services/inference_http/Dockerfile (missing dependencies)

Important Files Changed

File Analysis

Filename Score Overview
services/inference_http/adapters/fruit_defect_runner.py 0/5 CRITICAL: Contains wrong code - model_registry content instead of FruitDefectRunner class, will cause runtime failure
services/inference_http/app.py 3/5 Refactored to multi-model architecture with dynamic adapter loading, removed TEAM env requirement, added upload endpoint
services/inference_http/Dockerfile 2/5 Copies empty models directory, missing fruit_defect service dependency needed for imports
services/fruit_defect/configs/fruit_defect.yaml 1/5 Contains hardcoded Windows paths that won't work in production/Docker environment
streaming/flink/jobs/http_dispatcher.py 4/5 Refactored to multi-model dispatcher using mapping.yml, supports multiple input topics, improved architecture
docker-compose.yml 4/5 Added inference-http service and flink-dispatcher with mapping config, properly configured dependencies and volumes

Sequence Diagram

sequenceDiagram
    participant MinIO as MinIO (imagery bucket)
    participant Kafka as Kafka
    participant Flink as Flink HTTP Dispatcher
    participant Mapping as mapping.yml Config
    participant InfHTTP as Inference HTTP Service
    participant Adapter as FruitDefectRunner
    participant Model as PyTorch Model
    participant KafkaOut as Kafka (output topics)

    Note over MinIO: Image uploaded to<br/>imagery/fruits/ folder
    MinIO->>Kafka: Publish event to<br/>imagery.new.fruit topic
    
    Kafka->>Flink: Consume event<br/>{bucket, key, event_id}
    
    Flink->>Mapping: Lookup topic mapping
    Mapping-->>Flink: Return {name: fruit_defect,<br/>url: .../infer_json/fruit_defect}
    
    Flink->>Flink: Validate bucket/key present<br/>Generate idempotency-key
    
    Flink->>InfHTTP: POST /infer_json/fruit_defect<br/>{bucket, key}<br/>Headers: Idempotency-Key, X-Correlation-ID
    
    InfHTTP->>InfHTTP: get_adapter("fruit_defect")<br/>from model_registry
    InfHTTP->>MinIO: Fetch image bytes<br/>from bucket/key
    MinIO-->>InfHTTP: Return image data
    
    InfHTTP->>Adapter: adapter.predict(image_bytes)
    
    Note over Adapter: ERROR: File contains<br/>wrong code (model_registry<br/>instead of FruitDefectRunner)
    
    Adapter->>Model: Load weights, preprocess,<br/>run inference
    Model-->>Adapter: {status, prob_defect,<br/>confidence, latency_ms}
    
    Adapter-->>InfHTTP: Return prediction result
    InfHTTP-->>Flink: {ok: true, result, latency_ms}
    
    alt Success (200 status)
        Flink->>KafkaOut: Publish to inference.dispatched<br/>{event_id, name, result, event}
    else Failure (4xx/5xx or exception)
        Flink->>KafkaOut: Publish to dlq.inference.http<br/>{event_id, error, status}
    end
Loading

Additional Comments (3)

  1. services/inference_http/adapters/fruit_defect_runner.py, line 1-13 (link)

    syntax: This file contains the wrong code - it has model_registry.py content instead of the FruitDefectRunner class. The file should contain:

    Note: import path changed from models.fruit_defect to services.fruit_defect, method renamed from run to predict to match app.py expectations, and weights path updated to match new location.

  2. services/fruit_defect/configs/fruit_defect.yaml, line 2-4 (link)

    logic: Hardcoded Windows paths in config that won't work in Docker/production

  3. services/inference_http/Dockerfile, line 38 (link)

    logic: The Dockerfile copies models directory but it's empty (models were moved to services/fruit_defect/). Either remove this line or update to copy the fruit_defect service:

    Or remove if not needed.

22 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +7 to 8
python-multipart
numpy==1.26.4 No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: Duplicate numpy==1.26.4 entry

Suggested change
python-multipart
numpy==1.26.4
numpy==1.26.4

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.

1 participant