diff --git a/report_analyst/core/analysis_result_utils.py b/report_analyst/core/analysis_result_utils.py new file mode 100644 index 00000000..722d3c8d --- /dev/null +++ b/report_analyst/core/analysis_result_utils.py @@ -0,0 +1,69 @@ +"""Detect and partition failed vs successful analysis results.""" + +from __future__ import annotations + +from typing import Any + +_ERROR_ANSWER_PREFIXES = ( + "Error analyzing document:", + "Error parsing analysis:", +) + + +def result_payload(entry: dict[str, Any]) -> dict[str, Any]: + """Unwrap cache/get_analysis entries that nest payload under ``result``.""" + if not isinstance(entry, dict): + return {} + nested = entry.get("result") + return nested if isinstance(nested, dict) else entry + + +def is_stored_analysis_error(result: dict[str, Any] | None) -> bool: + """True when a result dict represents a failed analysis, not a real answer.""" + if not result: + return False + if result.get("analysis_status") == "error": + return True + if result.get("error"): + return True + answer = str(result.get("ANSWER", "")) + return any(answer.startswith(prefix) for prefix in _ERROR_ANSWER_PREFIXES) + + +def analysis_error_message(result: dict[str, Any]) -> str: + if result.get("error"): + return str(result["error"]) + return str(result.get("ANSWER", "Analysis failed")) + + +def split_analysis_results(results: dict[str, Any]) -> tuple[dict[str, Any], dict[str, str]]: + """Return (successful_results, question_id -> error_message).""" + successes: dict[str, Any] = {} + failures: dict[str, str] = {} + for question_id, entry in results.items(): + payload = result_payload(entry) + if is_stored_analysis_error(payload): + failures[question_id] = analysis_error_message(payload) + else: + successes[question_id] = entry + return successes, failures + + +def filter_successful_analysis_results(results: dict[str, Any]) -> dict[str, Any]: + successes, _ = split_analysis_results(results) + return successes + + +def normalize_results_container(results: Any) -> dict[str, Any]: + """Normalize UI session results to ``{"answers": {question_id: entry}}``.""" + if not isinstance(results, dict): + return {"answers": {}} + answers = results.get("answers") + if isinstance(answers, dict): + return results + return {"answers": dict(results)} + + +def session_answers_map(results: dict[str, Any]) -> dict[str, Any]: + """Return question_id -> entry from nested or legacy flat session results.""" + return normalize_results_container(results)["answers"] diff --git a/report_analyst/core/analyzer.py b/report_analyst/core/analyzer.py index 7a7c0423..d4fd0828 100644 --- a/report_analyst/core/analyzer.py +++ b/report_analyst/core/analyzer.py @@ -26,6 +26,15 @@ # Setup logging at the top of the file logger = logging.getLogger(__name__) +# Legacy: map question_set id → YAML question id prefix (when ids differ from set name) +QUESTION_SET_ID_PREFIX = { + "everest": "ev", + "tcfd": "tcfd", + "s4m": "s4m", + "lucia": "lucia", + "climretrieve": "climretr", +} + # Load environment variables load_dotenv() @@ -107,6 +116,13 @@ class DocumentAnalyzer: _instance = None _initialized = False + @classmethod + def _reset_singleton_if_stale(cls) -> None: + """Drop cached singleton after hot reload when instance predates new methods.""" + if cls._instance is not None and not hasattr(cls._instance, "_ensure_llm_client"): + cls._instance = None + cls._initialized = False + @classmethod def reset_instance(cls): """Reset the singleton so new API keys can be picked up after Settings changes.""" @@ -114,6 +130,7 @@ def reset_instance(cls): cls._initialized = False def __new__(cls): + cls._reset_singleton_if_stale() if cls._instance is None: cls._instance = super(DocumentAnalyzer, cls).__new__(cls) return cls._instance @@ -204,6 +221,8 @@ def _initialize_llm_clients(self) -> None: model_name=self.default_model, cache_dir=str(self.llm_cache_path), ) + self._llm_client_model = self.default_model + self._llm_api_key = os.getenv(self._api_key_env_for_model(self.default_model)) current_openai_key = os.getenv("OPENAI_API_KEY") if APIKeyManager.is_configured_key(current_openai_key): @@ -213,6 +232,7 @@ def _initialize_llm_clients(self) -> None: model_name=os.getenv("OPENAI_EMBEDDING_MODEL", "text-embedding-ada-002"), embed_batch_size=100, ) + self._embeddings_api_key = current_openai_key Settings.embed_model = self.embeddings else: logger.warning("No OpenAI API key - embedding functionality will be limited") @@ -354,6 +374,7 @@ async def score_chunk_relevance(self, question: str, chunk_text: str) -> float: Settings.ingestion_cache = None try: + self._ensure_llm_client() response = await self.llm.achat( prompt=( "As a senior equity analyst with expertise in climate science evaluating a company's " @@ -413,6 +434,7 @@ async def score_chunk_relevance_batch(self, question: str, chunks: List[Dict], s # Batch scoring - all chunks in one call chunks_text = "\n\n".join([f"[CHUNK {i + 1}]\n{chunk['text']}" for i, chunk in enumerate(chunks)]) + self._ensure_llm_client() response = await self.llm.achat( prompt=( "As a senior equity analyst with expertise in climate science evaluating a company's " @@ -547,7 +569,7 @@ def _save_cached_answers(self, file_path: str, answers: Dict) -> None: async def process_document( self, file_path: str, - selected_questions: List[int], + selected_questions: List[str | int], use_llm_scoring: bool = False, single_call: bool = True, force_recompute: bool = False, @@ -557,7 +579,7 @@ async def process_document( Args: file_path: Path to document file or URN for backend resources - selected_questions: List of question numbers to process + selected_questions: Question ids (``tcfd_1``, ``esrs_e1_climate_examples_9``) or legacy numbers use_llm_scoring: Whether to use LLM for chunk scoring single_call: Whether to use single LLM call per question force_recompute: Whether to force recomputation @@ -660,33 +682,28 @@ async def process_document( yield {"status": f"Document loaded with {len(chunks)} chunks"} - # 2. Process each question - for question_number in selected_questions: + if not selected_questions: + yield {"error": "Select at least one question to analyze."} + return + + question_ids = self.normalize_question_ids(selected_questions) + for bad_ref in self.unresolved_question_refs(selected_questions): + yield {"error": f"Question {bad_ref} not found"} + if not question_ids: + return + + # 2. Process each question by canonical id + for question_id in question_ids: try: - logger.info(f"[ANALYSIS] Processing question number {question_number}") - question_data = self.get_question_by_number(question_number) - if not question_data: - logger.warning(f"[ANALYSIS] Question {question_number} not found") - yield {"error": f"Question {question_number} not found"} + resolved = self.resolve_question(question_id) + if not resolved: + logger.warning(f"[ANALYSIS] Question {question_id} not found") + yield {"error": f"Question {question_id} not found"} continue - # Use the same prefix extraction logic as get_question_by_number - question_set_mapping = { - "everest": "ev", - "tcfd": "tcfd", - "s4m": "s4m", - "lucia": "lucia", - "climretrieve": "climretr", - } - question_prefix = question_set_mapping.get(self.question_set, self.question_set) - # If still not found in mapping, try to extract prefix from actual question IDs - if question_prefix == self.question_set and self.questions: - first_qid = next(iter(self.questions.keys()), "") - if first_qid and "_" in first_qid: - question_prefix = first_qid.split("_")[0] - - question_id = f"{question_prefix}_{question_number}" - logger.info(f"[ANALYSIS] Question ID: {question_id}") + question_id, question_data = resolved + question_number = self._question_number_from_id(question_id) + logger.info(f"[ANALYSIS] Processing question {question_id}") yield {"status": f"Processing question {question_number}: {question_data['text'][:50]}..."} @@ -817,15 +834,31 @@ async def process_document( except Exception as e: logger.error( - f"[ANALYSIS] Error processing question {question_number}: {e!s}", + f"[ANALYSIS] Error processing question {question_id}: {e!s}", exc_info=True, ) - yield {"error": f"Error processing question {question_number}: {e!s}"} + yield {"error": f"Error processing question {question_id}: {e!s}"} except Exception as e: logger.error(f"[ANALYSIS] Error processing document: {e!s}", exc_info=True) yield {"error": f"Error processing document: {e!s}"} + def _ensure_embeddings_client(self) -> None: + """Initialize or refresh OpenAI embeddings client from current OPENAI_API_KEY.""" + openai_key = os.getenv("OPENAI_API_KEY") + if not APIKeyManager.is_configured_key(openai_key): + raise RuntimeError("OpenAI embeddings unavailable — set OPENAI_API_KEY.") + cached_key = getattr(self, "_embeddings_api_key", None) + if self.embeddings is None or cached_key != openai_key: + self.embeddings = OpenAIEmbedding( + api_key=openai_key, + api_base=os.getenv("OPENAI_API_BASE"), + model_name=os.getenv("OPENAI_EMBEDDING_MODEL", "text-embedding-ada-002"), + embed_batch_size=100, + ) + self._embeddings_api_key = openai_key + Settings.embed_model = self.embeddings + def _create_chunks(self, file_path: str) -> List[Dict[str, Any]]: """Create document chunks with embeddings""" try: @@ -856,6 +889,8 @@ def _create_chunks(self, file_path: str) -> List[Dict[str, Any]]: logger.info(f"Created {len(text_chunks)} chunks") + self._ensure_embeddings_client() + # Compute embeddings in batches BATCH_SIZE = 100 # Process 100 chunks at a time chunks_data = [] @@ -995,6 +1030,7 @@ async def _analyze_chunks( # Get LLM response try: + self._ensure_llm_client() response = await self.llm.achat(messages) response_text = response.message.content # Changed from response.content to response.message.content logger.info("=== LLM Response ===") @@ -1093,19 +1129,25 @@ async def _analyze_chunks( except Exception as e: logger.error(f"Error analyzing chunks: {e!s}", exc_info=True) - return { - "ANSWER": f"Error analyzing document: {e!s}", - "SCORE": 0, - "EVIDENCE": [], - "GAPS": ["Error during analysis"], - "SOURCES": [], - "chunks": processed_chunks if "processed_chunks" in locals() else [], - "question_text": question_data["text"], - "guidelines": question_data.get("guidelines", ""), - } + raise def _load_questions(self) -> dict: - """Load questions from YAML files""" + """Load questions from YAML files (storage path via question_loader, then bundled).""" + from report_analyst.core.question_loader import get_question_loader + + qset = get_question_loader().get_question_set(self.question_set) + if qset: + questions = { + q_id: { + "text": q_data.get("text", ""), + "guidelines": q_data.get("guidelines", ""), + } + for q_id, q_data in qset.questions.items() + } + log_analysis_step(f"✓ Loaded {len(questions)} questions for {self.question_set} via question_loader") + return questions + + # Fallback: bundled questionsets only (no QUESTIONSETS_PATH override) # Look for question set file in multiple possible locations possible_paths = [ Path(__file__).parent.parent / "questionsets" / f"{self.question_set}_questions.yaml", # app/questionsets @@ -1149,42 +1191,72 @@ def _load_questions(self) -> dict: logger.exception("Full error:") # This will log the full traceback return {} - def get_question_by_number(self, number: int) -> Optional[Dict]: - """Get question data by its number.""" - try: - # Handle question set to prefix mapping - question_set_mapping = { - "everest": "ev", - "tcfd": "tcfd", - "s4m": "s4m", - "lucia": "lucia", - "climretrieve": "climretr", # Map climretrieve to climretr shortcut - } + def question_id_for_number(self, number: int) -> Optional[str]: + """Legacy: map 1-based question number to canonical question id in self.questions.""" + prefix = QUESTION_SET_ID_PREFIX.get(self.question_set, self.question_set) + key = f"{prefix}_{number}" + if key in self.questions: + return key + # Fall back to prefix from loaded IDs (e.g. climretr when set is climretrieve) + if self.questions: + first_qid = next(iter(self.questions.keys()), "") + if "_" in first_qid: + extracted = first_qid[: first_qid.rfind("_")] + key = f"{extracted}_{number}" + if key in self.questions: + return key + return None + + def resolve_question(self, ref: str | int) -> Optional[tuple[str, Dict]]: + """Resolve a question reference to (canonical_id, question_data). + + Accepts full YAML ids (``tcfd_1``, ``esrs_e1_climate_examples_9``, ``ev_1``) + or legacy 1-based numbers / bare digit strings when question_set is set. + """ + if isinstance(ref, str): + ref = ref.strip() + if ref in self.questions: + return ref, self.questions[ref] + if ref.isdigit(): + return self.resolve_question(int(ref)) + return None - # Get the correct prefix for the question set - question_prefix = question_set_mapping.get(self.question_set, self.question_set) - - # If still not found in mapping, try to extract prefix from actual question IDs - if question_prefix == self.question_set and self.questions: - # Extract prefix from first question ID (e.g., "climretr_1" -> "climretr") - first_qid = next(iter(self.questions.keys()), "") - if first_qid and "_" in first_qid: - extracted_prefix = first_qid.split("_")[0] - logger.info( - f"[ANALYSIS] Extracted prefix '{extracted_prefix}' from question IDs " - f"(question_set='{self.question_set}')" - ) - question_prefix = extracted_prefix + qid = self.question_id_for_number(ref) + if qid is None: + return None + return qid, self.questions[qid] + + def normalize_question_ids(self, selected: List[str | int]) -> List[str]: + """Return canonical question ids, preserving order and dropping duplicates.""" + seen: set[str] = set() + ids: List[str] = [] + for ref in selected: + resolved = self.resolve_question(ref) + if resolved is None: + continue + qid = resolved[0] + if qid not in seen: + seen.add(qid) + ids.append(qid) + return ids + + def unresolved_question_refs(self, selected: List[str | int]) -> List[str]: + """Return selection entries that did not resolve to a loaded question.""" + unresolved: List[str] = [] + for ref in selected: + if self.resolve_question(ref) is None: + unresolved.append(str(ref)) + return unresolved - question_key = f"{question_prefix}_{number}" - logger.debug(f"Looking for question {number} with key: {question_key}") - logger.debug(f"Available question keys: {list(self.questions.keys())}") + @staticmethod + def _question_number_from_id(question_id: str) -> Optional[int]: + suffix = question_id.rsplit("_", 1)[-1] + return int(suffix) if suffix.isdigit() else None - return self.questions.get(question_key) - except Exception as e: - logger.error(f"Error getting question {number}: {e!s}") - logger.exception("Full error:") - return None + def get_question_by_number(self, number: int) -> Optional[Dict]: + """Get question data by its number (legacy — prefer resolve_question with full id).""" + resolved = self.resolve_question(number) + return resolved[1] if resolved else None def update_parameters(self, chunk_size: int, chunk_overlap: int, top_k: int): """Update analysis parameters and recreate text splitter.""" @@ -1201,6 +1273,39 @@ def update_parameters(self, chunk_size: int, chunk_overlap: int, top_k: int): logger.info("Updated parameters and recreated text splitter") + def _api_key_env_for_model(self, model_name: str) -> str: + if model_name.startswith("gpt-"): + return "OPENAI_API_KEY" + if model_name.startswith("gemini-") or model_name.startswith("models/gemini-"): + return "GOOGLE_API_KEY" + raise ValueError(f"Unsupported model: {model_name}") + + def _llm_model_name(self) -> str: + if self.llm and hasattr(self.llm, "model"): + return self.llm.model + return self.default_model + + def _ensure_llm_client(self, model_name: str | None = None) -> None: + """Initialize or refresh the LLM client when model or API key changes.""" + if self.use_backend_llm: + return + model_name = model_name or self._llm_model_name() + env_var = self._api_key_env_for_model(model_name) + current_key = os.getenv(env_var) + cached_key = getattr(self, "_llm_api_key", None) + cached_model = getattr(self, "_llm_client_model", None) + if self.llm is None or cached_model != model_name or cached_key != current_key: + if not self._has_key_for_model(model_name): + self.llm = None + logger.warning("Cannot initialize %s without its API key", model_name) + return + self.llm = get_llm( + model_name=model_name, + cache_dir=str(self.llm_cache_path), + ) + self._llm_api_key = current_key + self._llm_client_model = model_name + async def retrieve_chunks( self, file_path: str, @@ -1226,18 +1331,8 @@ async def retrieve_chunks( def update_llm_model(self, model_name: str): """Update the LLM model.""" logger.info(f"Updating LLM model to: {model_name}") - self.default_model = model_name - if not self._has_key_for_model(model_name): - self.llm = None - logger.warning("Cannot initialize %s without its API key", model_name) - return - - # Initialize LLM with caching using the provider factory - self.llm = get_llm( - model_name=model_name, - cache_dir=str(self.llm_cache_path), - ) + self._ensure_llm_client(model_name) def get_all_cached_answers(self, question_set: str) -> Dict[str, Any]: """Get all cached answers for a question set""" @@ -1353,6 +1448,7 @@ async def _get_similar_chunks(self, query_text: str, chunks: List[Dict], top_k: try: logger.info(f"Getting similar chunks for query: {query_text[:50]}...") + self._ensure_embeddings_client() # Get embedding for the query query_embedding = self.embeddings.get_text_embedding(query_text) @@ -1514,13 +1610,7 @@ def _parse_analysis_response(self, response_text: str) -> Dict[str, Any]: except Exception as e: logger.error(f"Error parsing analysis response: {e!s}", exc_info=True) - return { - "ANSWER": f"Error parsing analysis: {e!s}", - "SCORE": 0, - "EVIDENCE": [], - "GAPS": ["Error during analysis"], - "SOURCES": [], - } + raise ValueError(f"Error parsing analysis: {e!s}") from e def create_analysis_dataframes(results: Dict) -> pd.DataFrame: diff --git a/report_analyst/core/cache_manager.py b/report_analyst/core/cache_manager.py index cd585742..10253693 100644 --- a/report_analyst/core/cache_manager.py +++ b/report_analyst/core/cache_manager.py @@ -7,8 +7,9 @@ import numpy as np from llama_index.core import Document -from sqlalchemy import text +from sqlalchemy import bindparam, text +from .analysis_result_utils import is_stored_analysis_error from .database_manager import DatabaseManager from .database_schema import indexes, metadata @@ -266,6 +267,14 @@ async def get_similar_chunks( def save_analysis(self, file_path: str, question_id: str, result: Dict, config: Dict): """Save analysis result to cache with improved logging""" + if is_stored_analysis_error(result): + logger.error( + "Refusing to cache failed analysis for %s - %s: %s", + file_path, + question_id, + result.get("ANSWER", result.get("error", "unknown error")), + ) + return try: logger.info(f"Saving analysis for {file_path} - {question_id}") logger.info(f"Configuration: {json.dumps(config, indent=2)}") @@ -606,7 +615,104 @@ def get_analysis(self, file_path: str, config: Dict, question_ids: Optional[List "chunks": result.get("chunks", []), } - return results + # Now get the chunk information for each question + if results: + chunk_query = text(""" + SELECT + ac.question_id, + dc.chunk_text, + dc.metadata as chunk_metadata, + cr.chunk_order, + cr.similarity_score, + cr.llm_score, + cr.is_evidence, + cr.evidence_order, + cr.metadata as relevance_metadata + FROM analysis_cache ac + JOIN questions q ON q.question_id = ac.question_id + JOIN question_analysis qa + ON qa.question_id = q.id + AND qa.file_path = ac.file_path + AND qa.model = ac.model + AND qa.top_k = ac.top_k + JOIN chunk_relevance cr ON cr.question_analysis_id = qa.id + JOIN document_chunks dc ON cr.document_chunk_id = dc.id + WHERE ac.file_path = :file_path + AND ac.chunk_size = :chunk_size + AND ac.chunk_overlap = :chunk_overlap + AND ac.top_k = :top_k + AND ac.model = :model + AND ac.question_set = :question_set + AND ac.question_id IN :question_ids + ORDER BY ac.question_id, cr.chunk_order + """).bindparams(bindparam("question_ids", expanding=True)) + + chunk_params = { + "file_path": str(file_path), + "chunk_size": config["chunk_size"], + "chunk_overlap": config["chunk_overlap"], + "top_k": config["top_k"], + "model": config["model"], + "question_set": db_question_set, + } + chunk_params["question_ids"] = list(results.keys()) + + logger.info("Executing chunk query with params: %s", list(chunk_params.keys())) + chunk_result = conn.execute(chunk_query, chunk_params) + chunk_rows = chunk_result.fetchall() + logger.info(f"Retrieved {len(chunk_rows)} chunk rows") + + if chunk_rows: + for question_id in results: + results[question_id]["chunks"] = [] + + # Add chunks to their respective questions + for row in chunk_rows: + question_id = row[0] + is_evidence = row[6] + if is_evidence is not None: + is_evidence = bool(is_evidence) + chunk_info = { + "text": row[1], + "metadata": json.loads(row[2]) if row[2] else {}, + "chunk_order": row[3], + "similarity_score": row[4], # Raw similarity score from DB + "llm_score": row[5], # Raw LLM score from DB + "is_evidence": is_evidence, + "evidence_order": row[7], + "relevance_metadata": json.loads(row[8]) if row[8] else {}, + } + logger.info( + "Raw DB values for chunk - similarity_score: %s, llm_score: %s, is_evidence: %s", + row[4], + row[5], + row[6], + ) + results[question_id]["chunks"].append(chunk_info) + + # Sort chunks by their order + for question_id in results: + results[question_id]["chunks"].sort(key=lambda x: x["chunk_order"]) + logger.info("Question %s: %s chunks", question_id, len(results[question_id]["chunks"])) + if results[question_id]["chunks"]: + logger.info( + " Similarity range: %.4f - %.4f", + min(c["similarity_score"] for c in results[question_id]["chunks"]), + max(c["similarity_score"] for c in results[question_id]["chunks"]), + ) + + filtered: Dict[str, Any] = {} + for question_id, entry in results.items(): + payload = entry.get("result", entry) + if is_stored_analysis_error(payload): + logger.warning( + "Omitting cached analysis error for %s: %s", + question_id, + payload.get("ANSWER", payload.get("error", "")), + ) + continue + filtered[question_id] = entry + return filtered except Exception as e: logger.error(f"Error retrieving analysis: {e!s}", exc_info=True) diff --git a/report_analyst/core/llm_providers.py b/report_analyst/core/llm_providers.py index bc5a96e8..6ad3b418 100644 --- a/report_analyst/core/llm_providers.py +++ b/report_analyst/core/llm_providers.py @@ -4,6 +4,7 @@ import os from typing import Any, Optional +import tiktoken from llama_index.llms.google_genai import GoogleGenAI # LlamaIndex LLM imports @@ -13,6 +14,42 @@ logger = logging.getLogger(__name__) +def _tiktoken_encoding_name_for_model(model_name: str) -> str: + """Resolve a tiktoken encoding when ``encoding_for_model`` has no catalog entry.""" + explicit = os.getenv("OPENAI_TIKTOKEN_ENCODING", "").strip() + if explicit: + return explicit + + lowered = model_name.lower() + if lowered.startswith("gpt-5") or "4o" in lowered: + return "o200k_base" + if lowered.startswith("gpt-4") or lowered.startswith("gpt-3.5"): + return "cl100k_base" + return "o200k_base" + + +def _tiktoken_encoding_for_model(model_name: str) -> tiktoken.Encoding: + """Return tiktoken encoding for an OpenAI model ID, with fallback for new IDs.""" + try: + return tiktoken.encoding_for_model(model_name) + except KeyError: + encoding_name = _tiktoken_encoding_name_for_model(model_name) + logger.warning( + "tiktoken has no mapping for %r; using %s (override via OPENAI_TIKTOKEN_ENCODING)", + model_name, + encoding_name, + ) + return tiktoken.get_encoding(encoding_name) + + +class ReportAnalystOpenAI(OpenAI): + """OpenAI LLM with tiktoken fallback for model IDs ahead of tiktoken releases.""" + + @property + def _tokenizer(self) -> tiktoken.Encoding: + return _tiktoken_encoding_for_model(self._get_model_name()) + + def get_llm(model_name: str, cache_dir: Optional[str] = None, **kwargs) -> Any: """ Factory function to get LLM implementations based on model name. @@ -36,7 +73,7 @@ def get_llm(model_name: str, cache_dir: Optional[str] = None, **kwargs) -> Any: logger.error(f"Cannot initialize OpenAI model '{model_name}' - OPENAI_API_KEY environment variable is not set") raise ValueError("OPENAI_API_KEY environment variable is required for OpenAI models") - return OpenAI( + return ReportAnalystOpenAI( model=model_name, api_key=api_key, api_base=os.getenv("OPENAI_API_BASE"), diff --git a/report_analyst/streamlit_app.py b/report_analyst/streamlit_app.py index 80aabfbc..383dda55 100644 --- a/report_analyst/streamlit_app.py +++ b/report_analyst/streamlit_app.py @@ -85,6 +85,11 @@ def is_api_key_missing_message(message: str) -> bool: logger.info(f"Added {current_dir} to Python path") # Keep relative imports +from report_analyst.core.analysis_result_utils import ( + normalize_results_container, + session_answers_map, + split_analysis_results, +) from report_analyst.core.analyzer import DocumentAnalyzer from report_analyst.core.api_key_manager import APIKeyManager from report_analyst.core.dataframe_manager import ( @@ -245,17 +250,10 @@ async def analyze_document( # Update analyzer with the current questions self.analyzer.questions = questions - # Convert selected question IDs to numbers for the analyzer - selected_numbers = [questions[q_id]["number"] for q_id in selected_questions] - - # Get the question set prefix from the first selected question - question_set = selected_questions[0].split("_")[0] if selected_questions else "tcfd" - self.analyzer.question_set = question_set - - # Pass use_llm_scoring to process_document + # Pass canonical question ids through to the analyzer async for result in self.analyzer.process_document( file_path, - selected_numbers, + selected_questions, use_llm_scoring, single_call, force_recompute, @@ -269,7 +267,7 @@ async def analyze_document( # Handle results with question_number if "question_number" in result: question_number = result["question_number"] - question_id = f"{question_set}_{question_number}" + question_id = result.get("question_id") or f"{self.analyzer.question_set}_{question_number}" # Create a new result with the question_id new_result = { @@ -299,7 +297,7 @@ async def analyze_document( def process_document( self, file_path: str, - selected_questions: List[int] | None = None, + selected_questions: List[str | int] | None = None, use_llm_scoring: bool = False, single_call: bool = True, force_recompute: bool = False, @@ -480,6 +478,14 @@ def generate_file_key(file_path: str, st) -> str: ) +def selected_question_ids_from_editor(edited_df: pd.DataFrame) -> list[str]: + """Return QID values for rows checked in the question data editor.""" + if edited_df.empty or "Select" not in edited_df.columns or "QID" not in edited_df.columns: + return [] + selected = edited_df["Select"].fillna(False).astype(bool) + return edited_df.loc[selected, "QID"].astype(str).tolist() + + async def analyze_document_and_display( report_analyzer, file_path: str, @@ -539,6 +545,8 @@ async def analyze_document_and_display( for q_id, answer in cached_answers.items(): st.session_state.results["answers"][q_id] = answer + st.session_state.results = normalize_results_container(st.session_state.results) + # Update display with cached results logger.info(f"Creating dataframes with cached results for file_key: {file_key}") logger.info( @@ -549,7 +557,7 @@ async def analyze_document_and_display( f"llm_model={st.session_state.get('new_llm_model')}, " f"use_llm_scoring={st.session_state.get('new_llm_scoring')}" ) - analysis_df, chunks_df = create_analysis_dataframes(st.session_state.results["answers"], file_key) + analysis_df, chunks_df = create_analysis_dataframes(session_answers_map(st.session_state.results), file_key) st.session_state.analysis_df = analysis_df st.session_state.chunks_df = chunks_df @@ -608,7 +616,7 @@ async def analyze_document_and_display( f"llm_model={st.session_state.get('new_llm_model')}, " f"use_llm_scoring={st.session_state.get('new_llm_scoring')}" ) - analysis_df, chunks_df = create_analysis_dataframes(st.session_state.results["answers"], file_key) + analysis_df, chunks_df = create_analysis_dataframes(session_answers_map(st.session_state.results), file_key) st.session_state.analysis_df = analysis_df st.session_state.chunks_df = chunks_df @@ -929,19 +937,23 @@ def display_pdf_viewer( questions: Dict[str, Dict], raw_chunks: Optional[List[Dict[str, Any]]] = None, ) -> None: - chunks_by_question = {question_id: data.get("chunks", []) for question_id, data in results.items()} - questions_data = {question_id: question.get("text", question_id) for question_id, question in questions.items()} + chunks_by_question = {question_id: data.get("chunks", []) for question_id, data in (results or {}).items()} + questions_data = {question_id: question.get("text", question_id) for question_id, question in (questions or {}).items()} viewer_key = Path(str(file_path)).stem or "analysis" with st.expander("PDF Viewer with Chunks", expanded=True): - pdf_viewer( - pdf_path=str(file_path), - chunks_data=chunks_by_question, - questions_data=questions_data, - unmapped_chunks=raw_chunks, - key=f"pdf_viewer_{viewer_key}", - height=800, - ) + try: + pdf_viewer( + pdf_path=str(file_path), + chunks_data=chunks_by_question, + questions_data=questions_data, + unmapped_chunks=raw_chunks, + key=f"pdf_viewer_{viewer_key}", + height=800, + ) + except Exception as e: + logger.error(f"Error rendering PDF viewer: {e!s}", exc_info=True) + st.error(f"Error rendering PDF viewer: {e!s}") def display_consolidated_results(analyzer, question_set, file_path=None, selected_config=None): @@ -1423,6 +1435,129 @@ def update_analyzer_parameters(): st.error(f"Error updating parameters: {e!s}") +async def run_analysis(analyzer, file_path, selected_questions, progress_text): + """Run analysis and update the UI with progress""" + try: + APIKeyManager.sync_api_keys_to_env(st.session_state) + + # Get current configuration + config = { + "chunk_size": st.session_state.chunk_size, + "chunk_overlap": st.session_state.chunk_overlap, + "top_k": st.session_state.top_k, + "model": st.session_state.llm_model, + "question_set": st.session_state.question_set, + } + logger.info(f"[ANALYSIS] User triggered analysis for file: {file_path}") + logger.info(f"[ANALYSIS] Selected questions: {selected_questions}") + if "questions" in st.session_state: + logger.info( + "[ANALYSIS] Selected question texts: %s", + [st.session_state.questions[q]["text"] for q in selected_questions if q in st.session_state.questions], + ) + logger.info( + f"[CACHE] Looking up cache for file: {file_path} with config: {config} and questions: {selected_questions}" + ) + question_ids = list(selected_questions) + if selected_questions and not all("_" in q for q in selected_questions): + question_ids = [f"{config['question_set']}_{q}" for q in selected_questions] + # Check if we have cached results first + cached_results = analyzer.cache_manager.get_analysis( + file_path=file_path, + config=config, + question_ids=question_ids, + ) + if cached_results and not st.session_state.get("force_recompute", False): + logger.info(f"[CACHE] Cache HIT for config: {config}") + progress_text.success("Found stored results!") + st.session_state.results = normalize_results_container(cached_results) + logger.info(f"[ANALYSIS] Writing results to session state for file: {file_path}") + logger.info(f"[ANALYSIS] Attempting to display results for file: {file_path}") + return + else: + logger.info(f"[CACHE] Cache MISS for config: {config}") + # If no cached results or force recompute, run analysis + progress_text.info("Starting analysis...") + + # Log the LLM scoring setting + llm_scoring_enabled = st.session_state.get("new_llm_scoring", False) + progress_text.info(f"LLM scoring: {'Enabled' if llm_scoring_enabled else 'Disabled'}") + logger.info(f"Starting analysis with LLM scoring: {llm_scoring_enabled}") + + # Track results + all_results = {} + + # Check if we have pre-retrieved chunks (for backend resources) + pre_retrieved_chunks = st.session_state.get("backend_chunks") + + # First update the analyzer's process_document method to use progress_text instead of yielding status + async for result in analyzer.process_document( + file_path=file_path, + selected_questions=selected_questions, + use_llm_scoring=st.session_state.get("new_llm_scoring", False), # Use the checkbox value directly + force_recompute=st.session_state.get("force_recompute", False), + pre_retrieved_chunks=pre_retrieved_chunks, # Pass backend chunks if available + ): + # Handle errors by displaying them but not storing them + if "error" in result: + if is_api_key_missing_message(result["error"]): + render_api_key_missing_alert(progress_text, result["error"]) + else: + progress_text.error(f"Error: {result['error']}") + continue + + # Handle status updates by displaying them but not storing them + if "status" in result: + progress_text.info(result["status"]) + continue + + # Process actual analysis results + question_id = result.get("question_id") + if not question_id: + # Try to construct question_id from question_number + question_number = result.get("question_number") + if question_number: + question_id = result.get("question_id") or f"{config['question_set']}_{question_number}" + else: + # Skip results without question_id or question_number + continue + + progress_text.info(f"Completed analysis for question {question_id}") + + # Store only the actual result data + result_data = result.get("result", result) + all_results[question_id] = result_data + + # After all questions are processed, get the complete results with chunks + final_results = analyzer.cache_manager.get_analysis( + file_path=file_path, config=config, question_ids=list(all_results.keys()) + ) + + if not final_results: + # If no results from cache, use the ones we just processed + final_results = all_results + + # When writing results to session state + logger.info(f"[ANALYSIS] Writing results to session state for file: {file_path}") + st.session_state.results = normalize_results_container(final_results) + logger.info(f"[ANALYSIS] Attempting to display results for file: {file_path}") + + successes, failures = split_analysis_results(session_answers_map(st.session_state.results)) + if failures: + for qid, message in failures.items(): + progress_text.error(f"{qid}: {message}") + if successes: + progress_text.success(f"Analysis complete for {len(successes)} question(s)!") + elif failures: + progress_text.warning("Analysis finished with errors — see messages above.") + else: + progress_text.success("Analysis complete!") + + except Exception as e: + progress_text.error(f"Error during analysis: {e!s}") + logger.error(f"Error during analysis: {e!s}", exc_info=True) + + def main(): """Main function for the Streamlit app""" try: @@ -3831,7 +3966,7 @@ def main(): if analyze_clicked or reanalyze_clicked: # NOW sync the selection state from the widget # Get selected questions from the edited dataframe - selected_questions = edited_df.loc[edited_df["Select"], "QID"].tolist() + selected_questions = selected_question_ids_from_editor(edited_df) # Update session state for individual question checkboxes (for backward compatibility) for q_id in questions.keys(): @@ -3927,28 +4062,32 @@ def main(): ) st.error(f"Error during analysis: {e!s}") - viewer_results = fresh_viewer_results - if not viewer_results: - viewer_results = analyzer.analyzer.cache_manager.get_analysis( - file_path=analysis_file_path, - config=config, - ) - if viewer_results: - raw_chunks = [] - elif is_backend: - raw_chunks = st.session_state.get("backend_chunks") or [] - else: - raw_chunks = analyzer.analyzer.cache_manager.get_document_chunks( - file_path=analysis_file_path, - chunk_size=config["chunk_size"], - chunk_overlap=config["chunk_overlap"], + try: + viewer_results = fresh_viewer_results + if not viewer_results: + viewer_results = analyzer.analyzer.cache_manager.get_analysis( + file_path=analysis_file_path, + config=config, + ) + if viewer_results: + raw_chunks = [] + elif is_backend: + raw_chunks = st.session_state.get("backend_chunks") or [] + else: + raw_chunks = analyzer.analyzer.cache_manager.get_document_chunks( + file_path=analysis_file_path, + chunk_size=config["chunk_size"], + chunk_overlap=config["chunk_overlap"], + ) + display_pdf_viewer( + str(analysis_file_path), + viewer_results, + questions, + raw_chunks, ) - display_pdf_viewer( - str(analysis_file_path), - viewer_results, - questions, - raw_chunks, - ) + except Exception as e: + logger.error(f"Error displaying PDF viewer: {e!s}", exc_info=True) + st.error(f"Error displaying PDF viewer: {e!s}") else: # Show helpful error message if file_path is None: diff --git a/tests/conftest.py b/tests/conftest.py index c8cc7989..bd4bdb7c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -74,6 +74,17 @@ def pytest_configure(config): # Register custom markers config.addinivalue_line("markers", "postgres: mark test as requiring PostgreSQL") config.addinivalue_line("markers", "integration: mark test as integration test") + os.environ.setdefault("MPLBACKEND", "Agg") + + +@pytest.fixture(autouse=True) +def _reset_document_analyzer_singleton(): + """Keep DocumentAnalyzer from leaking cache/LLM state across tests.""" + from report_analyst.core.analyzer import DocumentAnalyzer + + DocumentAnalyzer.reset_instance() + yield + DocumentAnalyzer.reset_instance() # ============================================================================= diff --git a/tests/test_actwyser_pipeline_coverage.py b/tests/test_actwyser_pipeline_coverage.py new file mode 100644 index 00000000..bd621714 --- /dev/null +++ b/tests/test_actwyser_pipeline_coverage.py @@ -0,0 +1,196 @@ +"""Extra coverage for Actwyser pipeline bugfix changed lines.""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import numpy as np +import pytest + +from report_analyst.core import analysis_result_utils as aru +from report_analyst.core.analyzer import DocumentAnalyzer +from report_analyst.core.cache_manager import CacheManager +from report_analyst.core.llm_providers import _tiktoken_encoding_name_for_model + + +def test_result_payload_non_dict_and_nested(): + assert aru.result_payload("x") == {} + assert aru.result_payload({"result": {"ANSWER": "a"}}) == {"ANSWER": "a"} + assert aru.result_payload({"ANSWER": "flat"}) == {"ANSWER": "flat"} + + +def test_is_stored_analysis_error_branches(): + assert aru.is_stored_analysis_error(None) is False + assert aru.is_stored_analysis_error({}) is False + assert aru.is_stored_analysis_error({"analysis_status": "error"}) is True + assert aru.is_stored_analysis_error({"error": "boom"}) is True + + +def test_analysis_error_message_prefers_error_field(): + assert aru.analysis_error_message({"error": "x"}) == "x" + assert aru.analysis_error_message({"ANSWER": "y"}) == "y" + + +def test_filter_successful_analysis_results(): + results = { + "a": {"result": {"ANSWER": "Error analyzing document: z"}}, + "b": {"result": {"ANSWER": "ok"}}, + } + assert list(aru.filter_successful_analysis_results(results)) == ["b"] + + +def test_normalize_results_container_non_dict(): + assert aru.normalize_results_container(None) == {"answers": {}} + assert aru.normalize_results_container([1]) == {"answers": {}} + + +def test_tiktoken_encoding_name_gpt4_and_default(monkeypatch): + monkeypatch.delenv("OPENAI_TIKTOKEN_ENCODING", raising=False) + assert _tiktoken_encoding_name_for_model("gpt-4-turbo") == "cl100k_base" + assert _tiktoken_encoding_name_for_model("gpt-3.5-turbo") == "cl100k_base" + assert _tiktoken_encoding_name_for_model("unknown-model") == "o200k_base" + + +@pytest.fixture +def analyzer(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + DocumentAnalyzer.reset_instance() + with ( + patch("llama_index.embeddings.openai.OpenAIEmbedding"), + patch("llama_index.core.Settings"), + ): + inst = DocumentAnalyzer() + inst.use_backend_llm = False + inst.llm = Mock(model="gpt-4o-mini") + yield inst + DocumentAnalyzer.reset_instance() + + +def test_reset_singleton_if_stale_drops_old_instance(): + DocumentAnalyzer.reset_instance() + stale = SimpleNamespace() # missing _ensure_llm_client + DocumentAnalyzer._instance = stale + DocumentAnalyzer._initialized = True + DocumentAnalyzer._reset_singleton_if_stale() + assert DocumentAnalyzer._instance is None + assert DocumentAnalyzer._initialized is False + + +def test_question_id_for_number_prefix_fallback(analyzer): + analyzer.question_set = "climretrieve" + analyzer.questions = {"climretr_2": {"text": "Q2", "guidelines": ""}} + assert analyzer.question_id_for_number(2) == "climretr_2" + assert analyzer.question_id_for_number(99) is None + + +def test_resolve_question_digit_string_and_missing(analyzer): + analyzer.question_set = "tcfd" + analyzer.questions = {"tcfd_3": {"text": "Q3", "guidelines": ""}} + qid, data = analyzer.resolve_question("3") + assert qid == "tcfd_3" + assert data["text"] == "Q3" + assert analyzer.resolve_question("missing_id") is None + assert analyzer.resolve_question(99) is None + + +def test_normalize_skips_unresolved_and_reports_them(analyzer): + analyzer.question_set = "tcfd" + analyzer.questions = {"tcfd_1": {"text": "Q1", "guidelines": ""}} + assert analyzer.normalize_question_ids(["nope", "tcfd_1", 1]) == ["tcfd_1"] + assert analyzer.unresolved_question_refs(["nope", 2]) == ["nope", "2"] + + +def test_api_key_env_and_llm_model_name_helpers(analyzer, monkeypatch): + with pytest.raises(ValueError, match="Unsupported model"): + analyzer._api_key_env_for_model("claude-3") + analyzer.llm = None + assert analyzer._llm_model_name() == analyzer.default_model + analyzer.use_backend_llm = True + analyzer._ensure_llm_client("gpt-4o-mini") # early return + analyzer.use_backend_llm = False + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + analyzer.llm = Mock(model="gpt-4o-mini") + analyzer._llm_api_key = "old" + analyzer._llm_client_model = "other" + analyzer._ensure_llm_client("gpt-4o-mini") + assert analyzer.llm is None + + +def test_ensure_embeddings_client_requires_key_and_sets_client(analyzer, monkeypatch): + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + with pytest.raises(RuntimeError, match="embeddings unavailable"): + analyzer._ensure_embeddings_client() + monkeypatch.setenv("OPENAI_API_KEY", "sk-new") + with patch("report_analyst.core.analyzer.OpenAIEmbedding") as emb_cls, patch( + "report_analyst.core.analyzer.Settings" + ) as settings: + emb_cls.return_value = Mock(name="emb") + analyzer.embeddings = None + analyzer._ensure_embeddings_client() + assert analyzer.embeddings is emb_cls.return_value + assert analyzer._embeddings_api_key == "sk-new" + assert settings.embed_model is emb_cls.return_value + + +@pytest.mark.asyncio +async def test_process_document_empty_and_unresolved_selection(analyzer, tmp_path): + file_path = str(tmp_path / "doc.pdf") + embedded = [{"text": "c", "metadata": {}, "embedding": np.array([0.1], dtype=np.float32)}] + analyzer.question_set = "tcfd" + analyzer.questions = {"tcfd_1": {"text": "Q1", "guidelines": ""}} + analyzer.cache_manager = CacheManager(db_path=str(tmp_path / "c.db")) + + with patch.object(analyzer.cache_manager, "get_document_chunks", return_value=embedded): + events = [] + async for ev in analyzer.process_document(file_path=file_path, selected_questions=[]): + events.append(ev) + assert any(e.get("error") == "Select at least one question to analyze." for e in events) + + events = [] + async for ev in analyzer.process_document(file_path=file_path, selected_questions=["missing"]): + events.append(ev) + assert any(e.get("error") == "Question missing not found" for e in events) + + +@pytest.mark.asyncio +async def test_process_document_resolve_none_mid_loop(analyzer, tmp_path): + file_path = str(tmp_path / "doc.pdf") + embedded = [{"text": "c", "metadata": {}, "embedding": np.array([0.1], dtype=np.float32)}] + analyzer.question_set = "tcfd" + analyzer.questions = {"tcfd_1": {"text": "Q1", "guidelines": ""}} + analyzer.cache_manager = CacheManager(db_path=str(tmp_path / "c.db")) + + with patch.object(analyzer.cache_manager, "get_document_chunks", return_value=embedded), patch.object( + analyzer, "normalize_question_ids", return_value=["tcfd_1"] + ), patch.object(analyzer, "unresolved_question_refs", return_value=[]), patch.object( + analyzer, "resolve_question", return_value=None + ): + events = [] + async for ev in analyzer.process_document(file_path=file_path, selected_questions=["tcfd_1"]): + events.append(ev) + assert any(e.get("error") == "Question tcfd_1 not found" for e in events) + + +@pytest.mark.asyncio +async def test_score_chunk_relevance_batch_ensures_llm(analyzer): + analyzer.llm = MagicMock() + analyzer.llm.achat = AsyncMock(return_value=SimpleNamespace(message=SimpleNamespace(content="[1] 0.5"))) + with patch.object(analyzer, "_ensure_llm_client") as ensure: + scores = await analyzer.score_chunk_relevance_batch("q", [{"text": "chunk"}], single_call=True) + ensure.assert_called() + assert isinstance(scores, list) + + +def test_load_questions_fallback_path(analyzer): + """When question_loader returns no set, fall back to bundled YAML paths.""" + analyzer.question_set = "tcfd" + mock_loader = MagicMock() + mock_loader.get_question_set.return_value = None + with patch( + "report_analyst.core.question_loader.get_question_loader", + return_value=mock_loader, + ): + questions = analyzer._load_questions() + assert isinstance(questions, dict) + assert len(questions) > 0 diff --git a/tests/test_analysis_result_errors.py b/tests/test_analysis_result_errors.py new file mode 100644 index 00000000..699ad542 --- /dev/null +++ b/tests/test_analysis_result_errors.py @@ -0,0 +1,132 @@ +"""Tests for failed-analysis detection and cache filtering.""" + +import json +import shutil +import sqlite3 +import tempfile +from pathlib import Path + +import pytest + +from report_analyst.core.analysis_result_utils import ( + is_stored_analysis_error, + normalize_results_container, + session_answers_map, + split_analysis_results, +) +from report_analyst.core.cache_manager import CacheManager + + +@pytest.fixture +def temp_db(): + temp_dir = tempfile.mkdtemp() + db_path = Path(temp_dir) / "test_cache.db" + cache_manager = CacheManager(str(db_path)) + yield cache_manager + shutil.rmtree(temp_dir) + + +def test_is_stored_analysis_error_detects_exception_answer(): + assert is_stored_analysis_error({"ANSWER": "Error analyzing document: 'str' object is not callable"}) + + +def test_is_stored_analysis_error_detects_parse_failure_answer(): + assert is_stored_analysis_error({"ANSWER": "Error parsing analysis: invalid JSON"}) + + +def test_is_stored_analysis_error_ignores_valid_answer(): + assert not is_stored_analysis_error({"ANSWER": "The board oversees climate risk.", "SCORE": 0.8}) + + +def test_split_analysis_results_separates_failures(): + results = { + "tcfd_1": {"result": {"ANSWER": "Error analyzing document: boom", "SCORE": 0}}, + "tcfd_2": {"result": {"ANSWER": "Valid answer", "SCORE": 1}}, + } + successes, failures = split_analysis_results(results) + assert list(successes) == ["tcfd_2"] + assert failures["tcfd_1"].startswith("Error analyzing document:") + + +def test_cache_manager_refuses_to_save_error_result(temp_db): + error_result = { + "ANSWER": "Error analyzing document: boom", + "SCORE": 0, + "EVIDENCE": [], + "GAPS": ["Error during analysis"], + "SOURCES": [], + } + config = { + "chunk_size": 500, + "chunk_overlap": 20, + "top_k": 5, + "model": "gpt-4o-mini", + "question_set": "tcfd", + } + temp_db.save_analysis("test.pdf", "tcfd_1", error_result, config) + cached = temp_db.get_analysis("test.pdf", config, question_ids=["tcfd_1"]) + assert cached == {} + + +def test_cache_manager_filters_legacy_error_results(temp_db): + error_result = { + "ANSWER": "Error analyzing document: legacy failure", + "SCORE": 0, + "EVIDENCE": [], + "GAPS": ["Error during analysis"], + "SOURCES": [], + } + good_result = { + "ANSWER": "Board oversees climate risks.", + "SCORE": 0.9, + "EVIDENCE": [], + "GAPS": [], + "SOURCES": [], + "chunks": [], + } + config = { + "chunk_size": 500, + "chunk_overlap": 20, + "top_k": 5, + "model": "gpt-4o-mini", + "question_set": "tcfd", + } + with sqlite3.connect(temp_db.db_path) as conn: + conn.execute( + """ + INSERT INTO analysis_cache + (file_path, question_id, chunk_size, chunk_overlap, top_k, model, question_set, result, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, datetime('now')) + """, + ( + "test.pdf", + "tcfd_1", + config["chunk_size"], + config["chunk_overlap"], + config["top_k"], + config["model"], + config["question_set"], + json.dumps(error_result), + ), + ) + temp_db.save_analysis("test.pdf", "tcfd_2", good_result, config) + cached = temp_db.get_analysis("test.pdf", config, question_ids=["tcfd_1", "tcfd_2"]) + assert "tcfd_1" not in cached + assert "tcfd_2" in cached + + +def test_normalize_results_container_wraps_legacy_flat_shape(): + flat = {"tcfd_2": {"result": {"ANSWER": "yes"}}} + assert normalize_results_container(flat) == {"answers": flat} + + +def test_normalize_results_container_preserves_nested_shape(): + nested = {"answers": {"tcfd_1": {"result": {"ANSWER": "ok"}}}} + assert normalize_results_container(nested) == nested + + +def test_session_answers_map_reads_flat_or_nested(): + flat = {"tcfd_2": {"result": {"ANSWER": "flat"}}} + nested = {"answers": {"tcfd_2": {"result": {"ANSWER": "nested"}}}} + assert session_answers_map(flat) == flat + assert session_answers_map(nested) == nested["answers"] diff --git a/tests/test_llm_client_refresh.py b/tests/test_llm_client_refresh.py new file mode 100644 index 00000000..b2bfc582 --- /dev/null +++ b/tests/test_llm_client_refresh.py @@ -0,0 +1,61 @@ +"""LLM client must pick up API key changes mid-session.""" + +from __future__ import annotations + +from report_analyst.core.analyzer import DocumentAnalyzer + + +def _bare_analyzer(tmp_path, **attrs) -> DocumentAnalyzer: + analyzer = object.__new__(DocumentAnalyzer) + analyzer.use_backend_llm = False + analyzer.llm_cache_path = tmp_path / "llm_cache" + analyzer.default_model = "gpt-4o-mini" + analyzer.llm = None + analyzer._llm_api_key = None + analyzer._llm_client_model = None + for key, value in attrs.items(): + setattr(analyzer, key, value) + return analyzer + + +def test_ensure_llm_client_refreshes_when_openai_api_key_changes(monkeypatch, tmp_path): + analyzer = _bare_analyzer(tmp_path) + created: list[tuple[str, str | None]] = [] + + def fake_get_llm(model_name, cache_dir=None, **kwargs): + import os + + created.append((model_name, os.getenv("OPENAI_API_KEY"))) + return type("LLM", (), {"model": model_name})() + + monkeypatch.setattr("report_analyst.core.analyzer.get_llm", fake_get_llm) + + monkeypatch.setenv("OPENAI_API_KEY", "openai-old") + analyzer._ensure_llm_client("gpt-4o-mini") + assert created == [("gpt-4o-mini", "openai-old")] + + monkeypatch.setenv("OPENAI_API_KEY", "openai-new") + analyzer._ensure_llm_client("gpt-4o-mini") + assert created == [("gpt-4o-mini", "openai-old"), ("gpt-4o-mini", "openai-new")] + + +def test_ensure_llm_client_refreshes_when_google_api_key_changes(monkeypatch, tmp_path): + analyzer = _bare_analyzer(tmp_path) + created: list[tuple[str, str | None]] = [] + + def fake_get_llm(model_name, cache_dir=None, **kwargs): + import os + + key = os.getenv("GOOGLE_API_KEY") if model_name.startswith("gemini-") else os.getenv("OPENAI_API_KEY") + created.append((model_name, key)) + return type("LLM", (), {"model": model_name})() + + monkeypatch.setattr("report_analyst.core.analyzer.get_llm", fake_get_llm) + + monkeypatch.setenv("GOOGLE_API_KEY", "google-old") + analyzer._ensure_llm_client("gemini-2.0-flash") + assert created == [("gemini-2.0-flash", "google-old")] + + monkeypatch.setenv("GOOGLE_API_KEY", "google-new") + analyzer._ensure_llm_client("gemini-2.0-flash") + assert created == [("gemini-2.0-flash", "google-old"), ("gemini-2.0-flash", "google-new")] diff --git a/tests/test_llm_providers.py b/tests/test_llm_providers.py index 0ebaea0e..522403d1 100644 --- a/tests/test_llm_providers.py +++ b/tests/test_llm_providers.py @@ -1,3 +1,7 @@ +"""Tests for tiktoken fallback and Gemini adapter in LLM provider factory.""" + +from __future__ import annotations + from unittest.mock import Mock import pytest @@ -5,6 +9,26 @@ from report_analyst.core import llm_providers +def test_get_llm_gpt_54_mini_tokenizer_falls_back_to_o200k_base(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + monkeypatch.delenv("OPENAI_TIKTOKEN_ENCODING", raising=False) + + from report_analyst.core.llm_providers import get_llm + + llm = get_llm("gpt-5.4-mini") + assert llm._tokenizer.name == "o200k_base" + + +def test_get_llm_tokenizer_respects_openai_tiktoken_encoding_env(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + monkeypatch.setenv("OPENAI_TIKTOKEN_ENCODING", "cl100k_base") + + from report_analyst.core.llm_providers import get_llm + + llm = get_llm("gpt-5.4-mini") + assert llm._tokenizer.name == "cl100k_base" + + @pytest.mark.parametrize( ("requested_model", "google_model"), [ diff --git a/tests/test_pdf_viewer.py b/tests/test_pdf_viewer.py index e0a2d7ba..2e38e908 100644 --- a/tests/test_pdf_viewer.py +++ b/tests/test_pdf_viewer.py @@ -138,3 +138,20 @@ def test_display_pdf_viewer_opens_without_analysis(): component.assert_called_once() assert component.call_args.kwargs["chunks_data"] == {} assert component.call_args.kwargs["unmapped_chunks"] == [] + + +def test_display_pdf_viewer_survives_component_errors(): + with ( + patch("report_analyst.streamlit_app.st.expander", return_value=nullcontext()), + patch("report_analyst.streamlit_app.pdf_viewer", side_effect=RuntimeError("boom")), + patch("report_analyst.streamlit_app.st.error") as error, + ): + display_pdf_viewer( + file_path="report.pdf", + results=None, + questions=None, + raw_chunks=None, + ) + + error.assert_called_once() + assert "Error rendering PDF viewer" in error.call_args.args[0] diff --git a/tests/test_question_id_pipeline.py b/tests/test_question_id_pipeline.py new file mode 100644 index 00000000..2286a176 --- /dev/null +++ b/tests/test_question_id_pipeline.py @@ -0,0 +1,125 @@ +"""Tests for canonical question ID resolution in the analyzer pipeline.""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, Mock, patch + +import numpy as np +import pytest + +from report_analyst.core.analyzer import DocumentAnalyzer +from report_analyst.core.cache_manager import CacheManager + + +@pytest.fixture +def analyzer(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + DocumentAnalyzer.reset_instance() + with ( + patch("llama_index.embeddings.openai.OpenAIEmbedding"), + patch("llama_index.core.Settings"), + ): + inst = DocumentAnalyzer() + inst.use_backend_llm = False + inst.llm = Mock(model="gpt-4o-mini") + yield inst + DocumentAnalyzer.reset_instance() + + +def _with_questions(analyzer, question_set: str, questions: dict): + analyzer.question_set = question_set + analyzer.questions = questions + return analyzer + + +class TestQuestionIdPipeline: + def test_resolve_full_id_esrs(self, analyzer): + _with_questions( + analyzer, + "esrs_e1_climate_examples", + {"esrs_e1_climate_examples_9": {"text": "Scope 1?", "guidelines": ""}}, + ) + qid, data = analyzer.resolve_question("esrs_e1_climate_examples_9") + assert qid == "esrs_e1_climate_examples_9" + assert data["text"] == "Scope 1?" + + def test_resolve_full_id_everest_ev_prefix(self, analyzer): + _with_questions(analyzer, "everest", {"ev_1": {"text": "Everest Q1", "guidelines": ""}}) + qid, _ = analyzer.resolve_question("ev_1") + assert qid == "ev_1" + + def test_resolve_legacy_int(self, analyzer): + _with_questions( + analyzer, + "tcfd", + {"tcfd_9": {"text": "Metrics?", "guidelines": ""}}, + ) + qid, _ = analyzer.resolve_question(9) + assert qid == "tcfd_9" + + def test_normalize_dedupes_ids_and_legacy_numbers(self, analyzer): + _with_questions( + analyzer, + "tcfd", + { + "tcfd_1": {"text": "Q1", "guidelines": ""}, + "tcfd_9": {"text": "Q9", "guidelines": ""}, + }, + ) + assert analyzer.normalize_question_ids(["tcfd_9", "tcfd_1", 1]) == ["tcfd_9", "tcfd_1"] + + +@pytest.mark.asyncio +async def test_process_document_caches_by_full_question_id(analyzer, tmp_path): + file_path = str(tmp_path / "esrs.pdf") + clean_db = tmp_path / "cache.db" + embedded = [ + { + "text": "Scope 1 total 1200 tCO2e.", + "metadata": {"page": 1}, + "embedding": np.array([0.1, 0.2], dtype=np.float32), + } + ] + _with_questions( + analyzer, + "esrs_e1_climate_examples", + {"esrs_e1_climate_examples_9": {"text": "Scope 1?", "guidelines": ""}}, + ) + analyzer.cache_manager = CacheManager(db_path=str(clean_db)) + + with patch.object(analyzer.cache_manager, "get_document_chunks", return_value=embedded), patch.object( + analyzer, "_get_similar_chunks", AsyncMock(return_value=embedded) + ), patch.object( + analyzer, "_analyze_chunks", AsyncMock(return_value={"ANSWER": "1200 tCO2e", "EVIDENCE": []}) + ), patch.object( + analyzer.cache_manager, "save_analysis" + ) as mock_save: + async for _ in analyzer.process_document( + file_path=file_path, + selected_questions=["esrs_e1_climate_examples_9"], + ): + pass + + assert mock_save.call_args.kwargs["question_id"] == "esrs_e1_climate_examples_9" + + +@pytest.mark.asyncio +async def test_process_document_legacy_int_resolves_to_canonical_id(analyzer, tmp_path): + file_path = str(tmp_path / "legacy.pdf") + clean_db = tmp_path / "cache.db" + embedded = [{"text": "c", "metadata": {}, "embedding": np.array([0.5], dtype=np.float32)}] + _with_questions(analyzer, "tcfd", {"tcfd_1": {"text": "Q1", "guidelines": ""}}) + analyzer.cache_manager = CacheManager(db_path=str(clean_db)) + + with patch.object(analyzer.cache_manager, "get_document_chunks", return_value=embedded), patch.object( + analyzer, "_get_similar_chunks", AsyncMock(return_value=embedded) + ), patch.object(analyzer, "_analyze_chunks", AsyncMock(return_value={"ANSWER": "Yes", "EVIDENCE": []})), patch.object( + analyzer.cache_manager, "save_analysis" + ) as mock_save: + async for _ in analyzer.process_document( + file_path=file_path, + selected_questions=[1], + ): + pass + + assert mock_save.call_args.kwargs["question_id"] == "tcfd_1" diff --git a/tests/test_streamlit_app_backend_integration.py b/tests/test_streamlit_app_backend_integration.py index 1ebbfdde..0d55639b 100644 --- a/tests/test_streamlit_app_backend_integration.py +++ b/tests/test_streamlit_app_backend_integration.py @@ -9,7 +9,7 @@ def test_backend_integration_availability(): """Test that backend integration features are available""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # The app should load without errors, indicating backend integration is available assert not at.exception, "Backend integration failed to load" @@ -21,7 +21,7 @@ def test_backend_integration_availability(): def test_backend_configuration_display(): """Test backend configuration display""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check for backend configuration elements # Backend integration might add additional configuration options @@ -33,7 +33,7 @@ def test_backend_configuration_display(): def test_backend_flow_orchestrator(): """Test backend flow orchestrator functionality""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check that backend flow orchestrator is properly integrated # This is verified by the app loading without backend integration errors @@ -44,7 +44,7 @@ def test_backend_flow_orchestrator(): def test_backend_analysis_workflow(): """Test backend analysis workflow integration""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check for backend analysis workflow elements # Backend integration should provide additional analysis capabilities @@ -56,7 +56,7 @@ def test_backend_analysis_workflow(): def test_backend_error_handling(): """Test backend integration error handling""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Backend integration should handle errors gracefully # This is verified by the app loading without backend-related exceptions @@ -67,7 +67,7 @@ def test_backend_error_handling(): def test_backend_fallback_behavior(): """Test backend integration fallback behavior""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Backend integration should have proper fallback behavior # when backend services are not available @@ -79,7 +79,7 @@ def test_backend_fallback_behavior(): def test_backend_config_status(): """Test backend configuration status display""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check for backend configuration status elements # Backend integration might display configuration status @@ -91,7 +91,7 @@ def test_backend_config_status(): def test_backend_processing_result(): """Test backend processing result handling""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check that backend processing results are handled properly # Backend integration should process results correctly @@ -102,7 +102,7 @@ def test_backend_processing_result(): def test_backend_analysis_result(): """Test backend analysis result integration""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check for backend analysis result handling # Backend integration should integrate analysis results properly @@ -113,7 +113,7 @@ def test_backend_analysis_result(): def test_backend_integration_imports(): """Test that backend integration imports work correctly""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Backend integration imports should work without errors # This is verified by the app loading successfully @@ -124,7 +124,7 @@ def test_backend_integration_imports(): def test_backend_flow_selection(): """Test backend flow selection functionality""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check for backend flow selection elements # Backend integration might provide different analysis flows @@ -136,7 +136,7 @@ def test_backend_flow_selection(): def test_backend_local_analysis(): """Test backend local analysis functionality""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Check for backend local analysis capabilities # Backend integration should support local analysis @@ -147,7 +147,7 @@ def test_backend_local_analysis(): def test_backend_integration_compatibility(): """Test backend integration compatibility with main app""" at = AppTest.from_file("report_analyst/streamlit_app.py") - at.run(timeout=10) + at.run(timeout=30) # Backend integration should be compatible with the main app # This is verified by the app loading and functioning correctly @@ -157,7 +157,7 @@ def test_backend_integration_compatibility(): # Navigate to Report Analyst page to check for title at.session_state["nav_page"] = "Report Analyst" - at.run(timeout=10) + at.run(timeout=30) # Check that all main app features still work with backend integration assert len(at.title) > 0, "App title not found with backend integration" @@ -255,7 +255,7 @@ def mock_post_side_effect(url, **kwargs): mock_post.side_effect = mock_post_side_effect # Run app - at.run(timeout=10) + at.run(timeout=30) assert not at.exception, "App failed to load" # Set backend config in session state @@ -263,7 +263,7 @@ def mock_post_side_effect(url, **kwargs): # Navigate to Report Analyst page at.session_state["nav_page"] = "Report Analyst" - at.run(timeout=10) + at.run(timeout=30) assert not at.exception, "Failed to navigate to Report Analyst page" # Verify backend resources are listed diff --git a/tests/test_streamlit_question_selection.py b/tests/test_streamlit_question_selection.py new file mode 100644 index 00000000..975cab6d --- /dev/null +++ b/tests/test_streamlit_question_selection.py @@ -0,0 +1,56 @@ +"""Tests for question-table selection helpers in streamlit_app.""" + +from __future__ import annotations + +import numpy as np +import pandas as pd +import pytest + +from report_analyst.streamlit_app import selected_question_ids_from_editor + + +def test_is_true_on_select_column_is_not_element_wise_filter(): + """Regression: ``series is True`` compares object identity, not row values.""" + edited_df = pd.DataFrame( + { + "Select": [True, False, True], + "QID": ["tcfd_1", "tcfd_2", "tcfd_3"], + } + ) + with pytest.raises(KeyError, match="False"): + edited_df[edited_df["Select"] is True]["QID"].tolist() + + +def test_selected_question_ids_from_editor_returns_checked_qids(): + edited_df = pd.DataFrame( + { + "Select": [True, False, True], + "QID": ["tcfd_1", "tcfd_2", "tcfd_3"], + "QUESTION": ["Q1", "Q2", "Q3"], + } + ) + assert selected_question_ids_from_editor(edited_df) == ["tcfd_1", "tcfd_3"] + + +def test_selected_question_ids_from_editor_none_checked(): + edited_df = pd.DataFrame( + { + "Select": [False, False], + "QID": ["tcfd_1", "tcfd_2"], + } + ) + assert selected_question_ids_from_editor(edited_df) == [] + + +def test_selected_question_ids_from_editor_numpy_bool_column(): + edited_df = pd.DataFrame( + { + "Select": np.array([True, False], dtype=bool), + "QID": ["tcfd_1", "tcfd_2"], + } + ) + assert selected_question_ids_from_editor(edited_df) == ["tcfd_1"] + + +def test_selected_question_ids_from_editor_empty_dataframe(): + assert selected_question_ids_from_editor(pd.DataFrame()) == []