Skip to content

Commit ecc6f91

Browse files
committed
refactor: extract additional duplicated code patterns
Code quality improvement to further reduce duplication: - Add model name constants (MODEL_SMALL_LLM, MODEL_LARGE_JUDGE, MODEL_OCR) - Extract QualityScore creation into _create_quality_score_from_validation() - Extract mock data response creation into _create_mock_data_response() - Extract empty extraction response creation into _create_empty_extraction_response() - Replace 5+ hardcoded model name strings with constants - Replace 60+ lines of duplicated QualityScore creation logic - Replace 6+ duplicated mock data response patterns - Replace 3+ duplicated empty extraction response patterns Benefits: - Reduced code duplication significantly (~100+ lines removed) - Better maintainability: centralized model names and response creation - Consistent responses across all functions - Easier to update: changes in one place affect all usages
1 parent 18abf82 commit ecc6f91

1 file changed

Lines changed: 110 additions & 118 deletions

File tree

src/api/agents/document/action_tools.py

Lines changed: 110 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ def _sanitize_log_data(data: Union[str, Any], max_length: int = 500) -> str:
6767

6868
class DocumentActionTools:
6969
"""Document processing action tools for MCP framework."""
70+
71+
# Model name constants
72+
MODEL_SMALL_LLM = "Llama Nemotron Nano VL 8B"
73+
MODEL_LARGE_JUDGE = "Llama 3.1 Nemotron 70B"
74+
MODEL_OCR = "NeMoRetriever-OCR-v1"
7075

7176
def __init__(self):
7277
self.nim_client = None
@@ -93,6 +98,82 @@ def _create_error_response(self, operation: str, error: Exception) -> Dict[str,
9398
"message": f"Failed to {operation}",
9499
}
95100

101+
def _create_mock_data_response(self, reason: Optional[str] = None, message: Optional[str] = None) -> Dict[str, Any]:
102+
"""Create standardized mock data response with optional reason and message."""
103+
response = {**self._get_mock_extraction_data(), "is_mock": True}
104+
if reason:
105+
response["reason"] = reason
106+
if message:
107+
response["message"] = message
108+
return response
109+
110+
def _create_empty_extraction_response(
111+
self, reason: str, message: str
112+
) -> Dict[str, Any]:
113+
"""Create empty extraction response structure for error/in-progress cases."""
114+
return {
115+
"extraction_results": [],
116+
"confidence_scores": {},
117+
"stages": [],
118+
"quality_score": None,
119+
"routing_decision": None,
120+
"is_mock": True,
121+
"reason": reason,
122+
"message": message,
123+
}
124+
125+
def _create_quality_score_from_validation(
126+
self, validation_data: Union[Dict[str, Any], Any]
127+
) -> Any:
128+
"""Create QualityScore from validation data (handles both dict and object)."""
129+
from .models.document_models import QualityScore, QualityDecision
130+
131+
# Handle object with attributes
132+
if hasattr(validation_data, "overall_score"):
133+
reasoning_text = getattr(validation_data, "reasoning", "")
134+
if isinstance(reasoning_text, str):
135+
reasoning_data = {"summary": reasoning_text, "details": reasoning_text}
136+
else:
137+
reasoning_data = reasoning_text if isinstance(reasoning_text, dict) else {}
138+
139+
return QualityScore(
140+
overall_score=getattr(validation_data, "overall_score", 0.0),
141+
completeness_score=getattr(validation_data, "completeness_score", 0.0),
142+
accuracy_score=getattr(validation_data, "accuracy_score", 0.0),
143+
compliance_score=getattr(validation_data, "compliance_score", 0.0),
144+
quality_score=getattr(
145+
validation_data,
146+
"quality_score",
147+
getattr(validation_data, "overall_score", 0.0),
148+
),
149+
decision=QualityDecision(getattr(validation_data, "decision", "REVIEW")),
150+
reasoning=reasoning_data,
151+
issues_found=getattr(validation_data, "issues_found", []),
152+
confidence=getattr(validation_data, "confidence", 0.0),
153+
judge_model=self.MODEL_LARGE_JUDGE,
154+
)
155+
156+
# Handle dictionary
157+
reasoning_data = validation_data.get("reasoning", {})
158+
if isinstance(reasoning_data, str):
159+
reasoning_data = {"summary": reasoning_data, "details": reasoning_data}
160+
161+
return QualityScore(
162+
overall_score=validation_data.get("overall_score", 0.0),
163+
completeness_score=validation_data.get("completeness_score", 0.0),
164+
accuracy_score=validation_data.get("accuracy_score", 0.0),
165+
compliance_score=validation_data.get("compliance_score", 0.0),
166+
quality_score=validation_data.get(
167+
"quality_score",
168+
validation_data.get("overall_score", 0.0),
169+
),
170+
decision=QualityDecision(validation_data.get("decision", "REVIEW")),
171+
reasoning=reasoning_data,
172+
issues_found=validation_data.get("issues_found", []),
173+
confidence=validation_data.get("confidence", 0.0),
174+
judge_model=self.MODEL_LARGE_JUDGE,
175+
)
176+
96177
def _parse_hours_range(self, time_str: str) -> Optional[int]:
97178
"""Parse hours range format (e.g., '4-8 hours') and return average in seconds."""
98179
parts = time_str.split("-")
@@ -765,9 +846,7 @@ async def _get_extraction_data(self, document_id: str) -> Dict[str, Any]:
765846
},
766847
confidence_score=ocr_data.get("confidence", 0.0),
767848
processing_time_ms=0, # OCR doesn't track processing time yet
768-
model_used=ocr_data.get(
769-
"model_used", "NeMoRetriever-OCR-v1"
770-
),
849+
model_used=ocr_data.get("model_used", self.MODEL_OCR),
771850
metadata={
772851
"layout_enhanced": ocr_data.get(
773852
"layout_enhanced", False
@@ -809,7 +888,7 @@ async def _get_extraction_data(self, document_id: str) -> Dict[str, Any]:
809888
processing_time_ms=llm_data.get(
810889
"processing_time_ms", 0
811890
),
812-
model_used="Llama Nemotron Nano VL 8B",
891+
model_used=self.MODEL_SMALL_LLM,
813892
metadata=llm_data.get("metadata", {}),
814893
)
815894
)
@@ -820,72 +899,7 @@ async def _get_extraction_data(self, document_id: str) -> Dict[str, Any]:
820899
validation_data = results["validation"]
821900

822901
# Handle both JudgeEvaluation object and dictionary
823-
if hasattr(validation_data, "overall_score"):
824-
# It's a JudgeEvaluation object
825-
reasoning_text = getattr(validation_data, "reasoning", "")
826-
quality_score = QualityScore(
827-
overall_score=getattr(
828-
validation_data, "overall_score", 0.0
829-
),
830-
completeness_score=getattr(
831-
validation_data, "completeness_score", 0.0
832-
),
833-
accuracy_score=getattr(
834-
validation_data, "accuracy_score", 0.0
835-
),
836-
compliance_score=getattr(
837-
validation_data, "compliance_score", 0.0
838-
),
839-
quality_score=getattr(
840-
validation_data,
841-
"quality_score",
842-
getattr(validation_data, "overall_score", 0.0),
843-
),
844-
decision=QualityDecision(
845-
getattr(validation_data, "decision", "REVIEW")
846-
),
847-
reasoning={
848-
"summary": reasoning_text,
849-
"details": reasoning_text,
850-
},
851-
issues_found=getattr(
852-
validation_data, "issues_found", []
853-
),
854-
confidence=getattr(validation_data, "confidence", 0.0),
855-
judge_model="Llama 3.1 Nemotron 70B",
856-
)
857-
else:
858-
# It's a dictionary
859-
reasoning_data = validation_data.get("reasoning", {})
860-
if isinstance(reasoning_data, str):
861-
reasoning_data = {
862-
"summary": reasoning_data,
863-
"details": reasoning_data,
864-
}
865-
866-
quality_score = QualityScore(
867-
overall_score=validation_data.get("overall_score", 0.0),
868-
completeness_score=validation_data.get(
869-
"completeness_score", 0.0
870-
),
871-
accuracy_score=validation_data.get(
872-
"accuracy_score", 0.0
873-
),
874-
compliance_score=validation_data.get(
875-
"compliance_score", 0.0
876-
),
877-
quality_score=validation_data.get(
878-
"quality_score",
879-
validation_data.get("overall_score", 0.0),
880-
),
881-
decision=QualityDecision(
882-
validation_data.get("decision", "REVIEW")
883-
),
884-
reasoning=reasoning_data,
885-
issues_found=validation_data.get("issues_found", []),
886-
confidence=validation_data.get("confidence", 0.0),
887-
judge_model="Llama 3.1 Nemotron 70B",
888-
)
902+
quality_score = self._create_quality_score_from_validation(validation_data)
889903

890904
# Routing Decision
891905
routing_decision = None
@@ -961,58 +975,36 @@ async def _get_extraction_data(self, document_id: str) -> Dict[str, Any]:
961975
if current_status in processing_stages:
962976
logger.info(f"Document {_sanitize_log_data(document_id)} is still being processed by NeMo pipeline. Status: {_sanitize_log_data(str(current_status))}")
963977
# Return a message indicating processing is in progress
964-
return {
965-
"extraction_results": [],
966-
"confidence_scores": {},
967-
"stages": [],
968-
"quality_score": None,
969-
"routing_decision": None,
970-
"is_mock": True,
971-
"reason": "processing_in_progress",
972-
"message": "Document is still being processed by NVIDIA NeMo pipeline. Please check again in a moment."
973-
}
978+
return self._create_empty_extraction_response(
979+
"processing_in_progress",
980+
"Document is still being processed by NVIDIA NeMo pipeline. Please check again in a moment."
981+
)
974982
elif current_status == ProcessingStage.COMPLETED:
975983
# Status says COMPLETED but no processing_results - this shouldn't happen
976984
# but if it does, wait a bit and check again (race condition)
977985
logger.warning(f"Document {_sanitize_log_data(document_id)} status is COMPLETED but no processing_results found. This may be a race condition.")
978-
return {
979-
"extraction_results": [],
980-
"confidence_scores": {},
981-
"stages": [],
982-
"quality_score": None,
983-
"routing_decision": None,
984-
"is_mock": True,
985-
"reason": "results_not_ready",
986-
"message": "Processing completed but results are not ready yet. Please check again in a moment."
987-
}
986+
return self._create_empty_extraction_response(
987+
"results_not_ready",
988+
"Processing completed but results are not ready yet. Please check again in a moment."
989+
)
988990
elif current_status == ProcessingStage.FAILED:
989991
# Processing failed
990992
error_msg = doc_status.get("error_message", "Unknown error")
991993
logger.warning(f"Document {_sanitize_log_data(document_id)} processing failed: {_sanitize_log_data(error_msg)}")
992-
return {
993-
"extraction_results": [],
994-
"confidence_scores": {},
995-
"stages": [],
996-
"quality_score": None,
997-
"routing_decision": None,
998-
"is_mock": True,
999-
"reason": "processing_failed",
1000-
"message": f"Document processing failed: {error_msg}"
1001-
}
994+
return self._create_empty_extraction_response(
995+
"processing_failed",
996+
f"Document processing failed: {error_msg}"
997+
)
1002998
else:
1003999
logger.warning(f"Document {_sanitize_log_data(document_id)} has no processing results and status is {_sanitize_log_data(str(current_status))}. NeMo pipeline may have failed.")
10041000
# Return mock data with clear indication that NeMo pipeline didn't complete
1005-
mock_data = self._get_mock_extraction_data()
1006-
mock_data["is_mock"] = True
1007-
mock_data["reason"] = "nemo_pipeline_incomplete"
1008-
mock_data["message"] = "NVIDIA NeMo pipeline did not complete processing. Please check server logs for errors."
1009-
return mock_data
1001+
return self._create_mock_data_response(
1002+
"nemo_pipeline_incomplete",
1003+
"NVIDIA NeMo pipeline did not complete processing. Please check server logs for errors."
1004+
)
10101005
else:
10111006
logger.error(f"Document {document_id} not found in status tracking")
1012-
mock_data = self._get_mock_extraction_data()
1013-
mock_data["is_mock"] = True
1014-
mock_data["reason"] = "document_not_found"
1015-
return mock_data
1007+
return self._create_mock_data_response("document_not_found")
10161008

10171009
except Exception as e:
10181010
logger.error(
@@ -1026,7 +1018,7 @@ async def _process_document_locally(self, document_id: str) -> Dict[str, Any]:
10261018
# Get document info from status
10271019
if document_id not in self.document_statuses:
10281020
logger.error(f"Document {document_id} not found in status tracking")
1029-
return {**self._get_mock_extraction_data(), "is_mock": True}
1021+
return self._create_mock_data_response()
10301022

10311023
doc_status = self.document_statuses[document_id]
10321024
file_path = doc_status.get("file_path")
@@ -1035,7 +1027,7 @@ async def _process_document_locally(self, document_id: str) -> Dict[str, Any]:
10351027
logger.warning(f"File not found for document {_sanitize_log_data(document_id)}: {_sanitize_log_data(file_path)}")
10361028
logger.info(f"Attempting to use document filename: {_sanitize_log_data(doc_status.get('filename', 'N/A'))}")
10371029
# Return mock data but mark it as such
1038-
return {**self._get_mock_extraction_data(), "is_mock": True, "reason": "file_not_found"}
1030+
return self._create_mock_data_response("file_not_found")
10391031

10401032
# Try to process the document locally
10411033
try:
@@ -1044,7 +1036,7 @@ async def _process_document_locally(self, document_id: str) -> Dict[str, Any]:
10441036

10451037
if not result["success"]:
10461038
logger.error(f"Local processing failed for {_sanitize_log_data(document_id)}: {_sanitize_log_data(str(result.get('error', 'Unknown error')))}")
1047-
return {**self._get_mock_extraction_data(), "is_mock": True, "reason": "processing_failed"}
1039+
return self._create_mock_data_response("processing_failed")
10481040
except ImportError as e:
10491041
logger.warning(f"Local processor not available (missing dependencies): {_sanitize_log_data(str(e))}")
10501042
missing_module = str(e).replace("No module named ", "").strip("'\"")
@@ -1054,10 +1046,10 @@ async def _process_document_locally(self, document_id: str) -> Dict[str, Any]:
10541046
logger.info("Install Pillow (PIL) for image processing: pip install Pillow")
10551047
else:
10561048
logger.info(f"Install missing dependency: pip install {_sanitize_log_data(missing_module)}")
1057-
return {**self._get_mock_extraction_data(), "is_mock": True, "reason": "dependencies_missing"}
1049+
return self._create_mock_data_response("dependencies_missing")
10581050
except Exception as e:
10591051
logger.error(f"Local processing error for {_sanitize_log_data(document_id)}: {_sanitize_log_data(str(e))}")
1060-
return {**self._get_mock_extraction_data(), "is_mock": True, "reason": "processing_error"}
1052+
return self._create_mock_data_response("processing_error")
10611053

10621054
# Convert local processing result to expected format
10631055
from .models.document_models import ExtractionResult, QualityScore, RoutingDecision, QualityDecision
@@ -1129,7 +1121,7 @@ async def _process_document_locally(self, document_id: str) -> Dict[str, Any]:
11291121

11301122
except Exception as e:
11311123
logger.error(f"Failed to process document locally: {_sanitize_log_data(str(e))}", exc_info=True)
1132-
return {**self._get_mock_extraction_data(), "is_mock": True, "reason": "exception"}
1124+
return self._create_mock_data_response("exception")
11331125

11341126
def _get_mock_extraction_data(self) -> Dict[str, Any]:
11351127
"""Fallback mock extraction data that matches the expected API response format."""
@@ -1196,7 +1188,7 @@ def _get_mock_extraction_data(self) -> Dict[str, Any]:
11961188
},
11971189
confidence_score=0.96,
11981190
processing_time_ms=1200,
1199-
model_used="NeMoRetriever-OCR-v1",
1191+
model_used=self.MODEL_OCR,
12001192
metadata={"page_count": 1, "language": "en", "field_count": 8},
12011193
),
12021194
ExtractionResult(
@@ -1217,7 +1209,7 @@ def _get_mock_extraction_data(self) -> Dict[str, Any]:
12171209
},
12181210
confidence_score=0.94,
12191211
processing_time_ms=800,
1220-
model_used="Llama Nemotron Nano VL 8B",
1212+
model_used=self.MODEL_SMALL_LLM,
12211213
metadata={"entity_count": 4, "validation_passed": True},
12221214
),
12231215
],
@@ -1248,7 +1240,7 @@ def _get_mock_extraction_data(self) -> Dict[str, Any]:
12481240
},
12491241
issues_found=["Minor formatting inconsistencies"],
12501242
confidence=0.91,
1251-
judge_model="Llama 3.1 Nemotron 70B",
1243+
judge_model=self.MODEL_LARGE_JUDGE,
12521244
),
12531245
"routing_decision": RoutingDecision(
12541246
routing_action=RoutingAction.AUTO_APPROVE,

0 commit comments

Comments
 (0)