From 79ac23bec20a41b8c958850b316bdf0af22f3013 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 09:50:33 +0000 Subject: [PATCH 1/7] Initial plan From 0e6e3eaf1dea0549ef57a7eccd99c719d73b6430 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:01:08 +0000 Subject: [PATCH 2/7] Optimize list operations and add caching for performance improvements Co-authored-by: Bryan-Roe <74067792+Bryan-Roe@users.noreply.github.com> --- datasets/chat/chat_logs/metadata.json | 14 ++++---- datasets/chat/chat_logs/test.json | 2 +- datasets/chat/chat_logs/train.json | 2 +- scripts/batch_evaluator.py | 48 ++++++++++++++------------- scripts/extract_chat_logs_dataset.py | 37 +++++++++++---------- scripts/status_dashboard.py | 45 ++++++++++++++++++++----- talk-to-ai/src/chat_providers.py | 9 ----- 7 files changed, 90 insertions(+), 67 deletions(-) diff --git a/datasets/chat/chat_logs/metadata.json b/datasets/chat/chat_logs/metadata.json index 0ed9e2d52..5e685feef 100644 --- a/datasets/chat/chat_logs/metadata.json +++ b/datasets/chat/chat_logs/metadata.json @@ -1,8 +1,8 @@ -{ - "log_files": [], - "total_examples": 1, - "train_examples": 1, - "test_examples": 1, - "seed": 42, - "context_window": 6 +{ + "log_files": [], + "total_examples": 1, + "train_examples": 1, + "test_examples": 1, + "seed": 42, + "context_window": 6 } \ No newline at end of file diff --git a/datasets/chat/chat_logs/test.json b/datasets/chat/chat_logs/test.json index 35bd87fa5..7df7f3c51 100644 --- a/datasets/chat/chat_logs/test.json +++ b/datasets/chat/chat_logs/test.json @@ -1 +1 @@ -{"messages": [{"role": "user", "content": "How does the local chat provider respond to a greeting?"}, {"role": "assistant", "content": "The local provider generates a concise heuristic reply; this synthetic example exists because no real logs were found."}], "hash": "fc25a50096b1833056a3c404", "source_file": ""} +{"messages": [{"role": "user", "content": "How does the local chat provider respond to a greeting?"}, {"role": "assistant", "content": "The local provider generates a concise heuristic reply; this synthetic example exists because no real logs were found."}], "hash": "fc25a50096b1833056a3c404", "source_file": ""} diff --git a/datasets/chat/chat_logs/train.json b/datasets/chat/chat_logs/train.json index 35bd87fa5..7df7f3c51 100644 --- a/datasets/chat/chat_logs/train.json +++ b/datasets/chat/chat_logs/train.json @@ -1 +1 @@ -{"messages": [{"role": "user", "content": "How does the local chat provider respond to a greeting?"}, {"role": "assistant", "content": "The local provider generates a concise heuristic reply; this synthetic example exists because no real logs were found."}], "hash": "fc25a50096b1833056a3c404", "source_file": ""} +{"messages": [{"role": "user", "content": "How does the local chat provider respond to a greeting?"}, {"role": "assistant", "content": "The local provider generates a concise heuristic reply; this synthetic example exists because no real logs were found."}], "hash": "fc25a50096b1833056a3c404", "source_file": ""} diff --git a/scripts/batch_evaluator.py b/scripts/batch_evaluator.py index f88849f1b..f2d46a2fa 100644 --- a/scripts/batch_evaluator.py +++ b/scripts/batch_evaluator.py @@ -78,30 +78,32 @@ def load_config(self, config_file: Path): with config_file.open("r") as f: config = yaml.safe_load(f) - for task_data in config.get("evaluation_tasks", []): - task = EvaluationTask(**task_data) - self.tasks.append(task) + # Use list comprehension for better performance + self.tasks.extend([ + EvaluationTask(**task_data) + for task_data in config.get("evaluation_tasks", []) + ]) print(f"[batch_eval] Loaded {len(self.tasks)} evaluation tasks") def scan_models(self) -> List[EvaluationTask]: """Scan for trained models and create evaluation tasks.""" - tasks = [] - - # Scan LoRA models + # Scan LoRA models - use list comprehension lora_dir = DATA_OUT.parent / "lora_training" + tasks = [] if lora_dir.exists(): - for model_dir in lora_dir.iterdir(): - if model_dir.is_dir() and (model_dir / "adapter_config.json").exists(): - task = EvaluationTask( - model_id=model_dir.name, - model_type="lora", - model_path=str(model_dir), - dataset="datasets/chat/mixed_chat", - metrics=["accuracy", "perplexity", "bleu"], - max_samples=100 - ) - tasks.append(task) + tasks = [ + EvaluationTask( + model_id=model_dir.name, + model_type="lora", + model_path=str(model_dir), + dataset="datasets/chat/mixed_chat", + metrics=["accuracy", "perplexity", "bleu"], + max_samples=100 + ) + for model_dir in lora_dir.iterdir() + if model_dir.is_dir() and (model_dir / "adapter_config.json").exists() + ] print(f"[batch_eval] Found {len(tasks)} models to evaluate") return tasks @@ -304,12 +306,12 @@ def export_json(self, output_file: Path): def compare_models(self, model_ids: List[str]) -> Dict: """Compare specific models side-by-side.""" - comparison = [] - - for model_id in model_ids: - result = next((r for r in self.results if r.model_id == model_id), None) - if result: - comparison.append(result) + # Use list comprehension for better performance + comparison = [ + r for model_id in model_ids + for r in self.results + if r.model_id == model_id + ] return { "models": [r.model_id for r in comparison], diff --git a/scripts/extract_chat_logs_dataset.py b/scripts/extract_chat_logs_dataset.py index c4f56b4ec..141f74986 100644 --- a/scripts/extract_chat_logs_dataset.py +++ b/scripts/extract_chat_logs_dataset.py @@ -53,16 +53,17 @@ def read_jsonl(path: Path) -> List[Dict]: def build_examples(messages: List[Dict], context_window: int) -> List[Dict]: - examples: List[Dict] = [] - # Simple user->assistant pairs + # Simple user->assistant pairs - build list directly + pairs = [] last_user = None for m in messages: if m.get("role") == "user": last_user = m elif m.get("role") == "assistant" and last_user: - pair = [last_user, m] - examples.append({"messages": pair}) - # Rolling windows + pairs.append({"messages": [last_user, m]}) + + # Rolling windows - use list comprehension for better performance + windows = [] if context_window > 2: for i in range(len(messages)): if messages[i].get("role") == "assistant": @@ -70,8 +71,10 @@ def build_examples(messages: List[Dict], context_window: int) -> List[Dict]: window = messages[start : i + 1] # Must contain at least one user+assistant if any(x.get("role") == "user" for x in window) and any(x.get("role") == "assistant" for x in window): - examples.append({"messages": window}) - return examples + windows.append({"messages": window}) + + # Combine lists efficiently with extend + return pairs + windows def hash_example(example: Dict) -> str: @@ -98,17 +101,15 @@ def main(): if not msgs: continue exs = build_examples(msgs, args.context_window) - for e in exs: - e["source_file"] = lf.name - e["hash"] = hash_example(e) - all_examples.append(e) - - # Deduplicate by hash - uniq = {} - for e in all_examples: - h = e["hash"] - if h not in uniq: - uniq[h] = e + # Add metadata and hash in batch using list comprehension + enriched_exs = [ + {**e, "source_file": lf.name, "hash": hash_example(e)} + for e in exs + ] + all_examples.extend(enriched_exs) + + # Deduplicate by hash - use dict comprehension for cleaner code + uniq = {e["hash"]: e for e in all_examples} examples = list(uniq.values()) if not examples: diff --git a/scripts/status_dashboard.py b/scripts/status_dashboard.py index 111607da8..5bfb861ee 100644 --- a/scripts/status_dashboard.py +++ b/scripts/status_dashboard.py @@ -45,6 +45,10 @@ "smart_orchestrator": REPO_ROOT / "data_out" / "smart_orchestrator", } +# Simple cache for status files to avoid repeated I/O (TTL: 5 seconds) +_status_cache: Dict[str, tuple[float, Optional[Dict[str, Any]]]] = {} +_STATUS_CACHE_TTL = 5.0 # seconds + @dataclass class OrchestratorStatus: @@ -76,15 +80,40 @@ def status_emoji(self) -> str: return "⚪" -def load_status(path: Path) -> Optional[Dict[str, Any]]: - """Load status JSON if it exists""" +def load_status(path: Path, use_cache: bool = True) -> Optional[Dict[str, Any]]: + """Load status JSON if it exists, with optional caching to reduce I/O. + + Args: + path: Path to status JSON file + use_cache: If True, use cache with 5s TTL to avoid repeated reads + + Returns: + Parsed JSON dict or None if file doesn't exist or is invalid + """ + path_str = str(path) + current_time = time.time() + + # Check cache if enabled + if use_cache and path_str in _status_cache: + cached_time, cached_data = _status_cache[path_str] + if current_time - cached_time < _STATUS_CACHE_TTL: + return cached_data + + # Load from file if not path.exists(): - return None - try: - with path.open("r") as f: - return json.load(f) - except Exception: - return None + result = None + else: + try: + with path.open("r") as f: + result = json.load(f) + except Exception: + result = None + + # Update cache + if use_cache: + _status_cache[path_str] = (current_time, result) + + return result def parse_autotrain_status() -> OrchestratorStatus: diff --git a/talk-to-ai/src/chat_providers.py b/talk-to-ai/src/chat_providers.py index e5361907c..c6158d793 100644 --- a/talk-to-ai/src/chat_providers.py +++ b/talk-to-ai/src/chat_providers.py @@ -57,15 +57,6 @@ def format_quota_message(exc: Any, service_name: str = "Azure OpenAI") -> str: RoleMessage = Dict[str, str] -# ------------------------------------------------------------------------- -# LM Studio availability cache to avoid repeated HTTP health checks -# ------------------------------------------------------------------------- - -_lmstudio_cache: Dict[str, Any] = { - "available": None, "checked_at": 0.0, "url": None} -_LMSTUDIO_CACHE_TTL = 30 # seconds - - def _check_lmstudio_available(url: str) -> bool: """Backward-compatible alias for the newer `_check_lm_studio_available` function. From a63a67dd39025cbb033bcef5c825d30d51b28a20 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:04:50 +0000 Subject: [PATCH 3/7] Add advanced optimizations: single-pass stats, cached limits, reduced syscalls Co-authored-by: Bryan-Roe <74067792+Bryan-Roe@users.noreply.github.com> --- scripts/collect_more_datasets.py | 9 ++++++-- scripts/download_datasets.py | 9 ++++++-- scripts/smart_orchestrator.py | 39 ++++++++++++++++++++------------ scripts/system_health_check.py | 13 +++++++++-- shared/subscription_manager.py | 12 ++++++---- 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/scripts/collect_more_datasets.py b/scripts/collect_more_datasets.py index c8b870c10..b65bbef4d 100644 --- a/scripts/collect_more_datasets.py +++ b/scripts/collect_more_datasets.py @@ -286,13 +286,18 @@ def _update_index(self, name: str, info: dict, path: Path): else: index = {"datasets": {}, "metadata": {}, "storage": {}} - # Add dataset + # Add dataset - optimize: single stat() call + try: + file_size = path.stat().st_size + except (FileNotFoundError, OSError): + file_size = 0 + index["datasets"][name] = { "category": "quantum", "filename": info["filename"], "path": str(path), "description": info["description"], - "size": path.stat().st_size if path.exists() else 0, + "size": file_size, "features": info["features"], "samples": info["samples"], "classes": info["classes"], diff --git a/scripts/download_datasets.py b/scripts/download_datasets.py index ec37ed3e1..17b48f260 100644 --- a/scripts/download_datasets.py +++ b/scripts/download_datasets.py @@ -135,13 +135,18 @@ def download_quantum_datasets(self): success = self._download_file(info["url"], dest, info["description"]) if success: - # Add to index + # Add to index - optimize: single stat() call + try: + file_size = dest.stat().st_size + except (FileNotFoundError, OSError): + file_size = 0 + self.index["datasets"][name] = { "category": "quantum", "filename": info["filename"], "path": str(dest), "description": info["description"], - "size": dest.stat().st_size if dest.exists() else 0 + "size": file_size } print(f"\n✅ Quantum datasets saved to: {self.quantum_dir}") diff --git a/scripts/smart_orchestrator.py b/scripts/smart_orchestrator.py index 0108f0beb..8f5e9dcc4 100644 --- a/scripts/smart_orchestrator.py +++ b/scripts/smart_orchestrator.py @@ -107,24 +107,35 @@ def is_complete(self) -> bool: ) def get_stats(self) -> Dict[str, Any]: - """Get pipeline statistics""" + """Get pipeline statistics - optimized single-pass aggregation""" + stats = { + "succeeded": 0, + "failed": 0, + "running": 0, + "pending": 0, + "skipped": 0, + "total_duration_sec": 0 + } + + # Single pass through jobs for all statistics + for j in self.jobs.values(): + status = j.status + if status in stats: + stats[status] += 1 + stats["total_duration_sec"] += j.duration_sec + total = len(self.jobs) - succeeded = sum(1 for j in self.jobs.values() if j.status == "succeeded") - failed = sum(1 for j in self.jobs.values() if j.status == "failed") - running = sum(1 for j in self.jobs.values() if j.status == "running") - pending = sum(1 for j in self.jobs.values() if j.status == "pending") - skipped = sum(1 for j in self.jobs.values() if j.status == "skipped") - total_time = sum(j.duration_sec for j in self.jobs.values()) + completed = stats["succeeded"] + stats["failed"] + stats["skipped"] return { "total": total, - "succeeded": succeeded, - "failed": failed, - "running": running, - "pending": pending, - "skipped": skipped, - "total_duration_sec": round(total_time, 2), - "completion_pct": int((succeeded + failed + skipped) / max(total, 1) * 100), + "succeeded": stats["succeeded"], + "failed": stats["failed"], + "running": stats["running"], + "pending": stats["pending"], + "skipped": stats["skipped"], + "total_duration_sec": round(stats["total_duration_sec"], 2), + "completion_pct": int(completed / max(total, 1) * 100), } diff --git a/scripts/system_health_check.py b/scripts/system_health_check.py index f9b288bd3..3a5f9c413 100644 --- a/scripts/system_health_check.py +++ b/scripts/system_health_check.py @@ -101,9 +101,18 @@ def check_documentation(self) -> Dict[str, Any]: status = {"files": {}} for doc, description in docs.items(): path = REPO_ROOT / doc + # Optimize: single stat() call with exception handling + try: + file_stat = path.stat() + exists = True + size_kb = file_stat.st_size / 1024 + except FileNotFoundError: + exists = False + size_kb = 0 + status["files"][doc] = { - "exists": path.exists(), - "size_kb": path.stat().st_size / 1024 if path.exists() else 0, + "exists": exists, + "size_kb": size_kb, "description": description, } diff --git a/shared/subscription_manager.py b/shared/subscription_manager.py index 7c950a68c..a656edf5c 100644 --- a/shared/subscription_manager.py +++ b/shared/subscription_manager.py @@ -132,6 +132,8 @@ def __init__( "websites_created": 0, } self.usage_reset_date = datetime.now() + timedelta(days=30) + # Cache tier limits to avoid repeated dictionary lookups + self._tier_limits = TIER_LIMITS.get(self.tier, {}) def is_active(self) -> bool: """Check if subscription is currently active""" @@ -148,7 +150,7 @@ def has_feature(self, feature: Feature) -> bool: return TIER_FEATURES.get(self.tier, {}).get(feature, False) def check_limit(self, resource: str, amount: int = 1) -> bool: - """Check if usage is within limits""" + """Check if usage is within limits - optimized with cached tier limits""" if not self.is_active(): return False @@ -156,7 +158,8 @@ def check_limit(self, resource: str, amount: int = 1) -> bool: if datetime.now() > self.usage_reset_date: self.reset_usage() - limit = TIER_LIMITS.get(self.tier, {}).get(resource, 0) + # Use cached tier limits + limit = self._tier_limits.get(resource, 0) if limit == -1: # unlimited return True @@ -182,8 +185,9 @@ def reset_usage(self): self.usage_reset_date = datetime.now() + timedelta(days=30) def get_usage_percentage(self, resource: str) -> float: - """Get usage as percentage of limit""" - limit = TIER_LIMITS.get(self.tier, {}).get(resource, 0) + """Get usage as percentage of limit - optimized with cached tier limits""" + # Use cached tier limits + limit = self._tier_limits.get(resource, 0) if limit == -1: return 0.0 # unlimited if limit == 0: From 05d5c6d23fd94c246e2f7a636d0cd414f659e9e7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:07:26 +0000 Subject: [PATCH 4/7] Address code review: fix cache staleness, improve algorithm complexity, preserve dedup order Co-authored-by: Bryan-Roe <74067792+Bryan-Roe@users.noreply.github.com> --- scripts/batch_evaluator.py | 12 +++++------- scripts/extract_chat_logs_dataset.py | 9 +++++++-- shared/subscription_manager.py | 2 ++ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/scripts/batch_evaluator.py b/scripts/batch_evaluator.py index f2d46a2fa..ca440137c 100644 --- a/scripts/batch_evaluator.py +++ b/scripts/batch_evaluator.py @@ -305,13 +305,11 @@ def export_json(self, output_file: Path): print(f"[batch_eval] Exported JSON to: {output_file}") def compare_models(self, model_ids: List[str]) -> Dict: - """Compare specific models side-by-side.""" - # Use list comprehension for better performance - comparison = [ - r for model_id in model_ids - for r in self.results - if r.model_id == model_id - ] + """Compare specific models side-by-side - optimized O(n+m) lookup.""" + # Convert model_ids to set for O(1) membership testing + model_ids_set = set(model_ids) + # Single pass through results for O(n+m) complexity instead of O(n*m) + comparison = [r for r in self.results if r.model_id in model_ids_set] return { "models": [r.model_id for r in comparison], diff --git a/scripts/extract_chat_logs_dataset.py b/scripts/extract_chat_logs_dataset.py index 141f74986..3a38eb0dd 100644 --- a/scripts/extract_chat_logs_dataset.py +++ b/scripts/extract_chat_logs_dataset.py @@ -108,8 +108,13 @@ def main(): ] all_examples.extend(enriched_exs) - # Deduplicate by hash - use dict comprehension for cleaner code - uniq = {e["hash"]: e for e in all_examples} + # Deduplicate by hash - keeps first occurrence for consistency + # (dict comprehension would keep last, so we iterate forward) + uniq = {} + for e in all_examples: + h = e["hash"] + if h not in uniq: + uniq[h] = e examples = list(uniq.values()) if not examples: diff --git a/shared/subscription_manager.py b/shared/subscription_manager.py index a656edf5c..fb4d7139f 100644 --- a/shared/subscription_manager.py +++ b/shared/subscription_manager.py @@ -278,6 +278,8 @@ def upgrade_subscription( """Upgrade user subscription""" sub = self.get_subscription(user_id) sub.tier = tier + # Update cached tier limits when tier changes + sub._tier_limits = TIER_LIMITS.get(tier, {}) sub.start_date = datetime.now() sub.end_date = datetime.now() + timedelta(days=duration_days) sub.payment_method = payment_method From 49f566b7270b8bcfab9ad4e4da900e68f68cb6b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:09:46 +0000 Subject: [PATCH 5/7] Add logging, documentation, and defensive checks for edge cases Co-authored-by: Bryan-Roe <74067792+Bryan-Roe@users.noreply.github.com> --- scripts/extract_chat_logs_dataset.py | 4 ++-- scripts/smart_orchestrator.py | 7 +++++++ scripts/status_dashboard.py | 1 + shared/subscription_manager.py | 2 ++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/extract_chat_logs_dataset.py b/scripts/extract_chat_logs_dataset.py index 3a38eb0dd..1b97f1530 100644 --- a/scripts/extract_chat_logs_dataset.py +++ b/scripts/extract_chat_logs_dataset.py @@ -108,8 +108,8 @@ def main(): ] all_examples.extend(enriched_exs) - # Deduplicate by hash - keeps first occurrence for consistency - # (dict comprehension would keep last, so we iterate forward) + # Deduplicate by hash - keeps first occurrence (Python 3.7+ dict is ordered) + # Explicit iteration ensures first occurrence is preserved uniq = {} for e in all_examples: h = e["hash"] diff --git a/scripts/smart_orchestrator.py b/scripts/smart_orchestrator.py index 8f5e9dcc4..f62155113 100644 --- a/scripts/smart_orchestrator.py +++ b/scripts/smart_orchestrator.py @@ -35,6 +35,7 @@ import argparse import json +import logging import subprocess import sys import time @@ -49,6 +50,9 @@ except ImportError: raise SystemExit("pyyaml required. Install: pip install pyyaml") +# Configure logging +logger = logging.getLogger(__name__) + REPO_ROOT = Path(__file__).resolve().parents[1] DATA_OUT = REPO_ROOT / "data_out" / "smart_orchestrator" @@ -122,6 +126,9 @@ def get_stats(self) -> Dict[str, Any]: status = j.status if status in stats: stats[status] += 1 + else: + # Unknown status - log for debugging but don't crash + logger.warning(f"Job {j.name} has unknown status: {status}") stats["total_duration_sec"] += j.duration_sec total = len(self.jobs) diff --git a/scripts/status_dashboard.py b/scripts/status_dashboard.py index 5bfb861ee..3d0835530 100644 --- a/scripts/status_dashboard.py +++ b/scripts/status_dashboard.py @@ -46,6 +46,7 @@ } # Simple cache for status files to avoid repeated I/O (TTL: 5 seconds) +# Note: Not thread-safe - if concurrent access needed, add threading.Lock _status_cache: Dict[str, tuple[float, Optional[Dict[str, Any]]]] = {} _STATUS_CACHE_TTL = 5.0 # seconds diff --git a/shared/subscription_manager.py b/shared/subscription_manager.py index fb4d7139f..c43eecfde 100644 --- a/shared/subscription_manager.py +++ b/shared/subscription_manager.py @@ -134,6 +134,8 @@ def __init__( self.usage_reset_date = datetime.now() + timedelta(days=30) # Cache tier limits to avoid repeated dictionary lookups self._tier_limits = TIER_LIMITS.get(self.tier, {}) + if not self._tier_limits: + logger.warning(f"Unknown tier {self.tier} - using empty limits") def is_active(self) -> bool: """Check if subscription is currently active""" From 02a0df98d2ef9774379ed8e220315ffdf2ab6a7f Mon Sep 17 00:00:00 2001 From: Bryan <74067792+Bryan-Roe@users.noreply.github.com> Date: Tue, 17 Feb 2026 06:49:58 -0800 Subject: [PATCH 6/7] Update scripts/extract_chat_logs_dataset.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- scripts/extract_chat_logs_dataset.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/extract_chat_logs_dataset.py b/scripts/extract_chat_logs_dataset.py index 1b97f1530..b7c20a7a8 100644 --- a/scripts/extract_chat_logs_dataset.py +++ b/scripts/extract_chat_logs_dataset.py @@ -74,7 +74,8 @@ def build_examples(messages: List[Dict], context_window: int) -> List[Dict]: windows.append({"messages": window}) # Combine lists efficiently with extend - return pairs + windows + pairs.extend(windows) + return pairs def hash_example(example: Dict) -> str: From 8f72891c66ae8cd83fb635dceaa1262aea5dde4a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Feb 2026 15:08:16 +0000 Subject: [PATCH 7/7] Address PR feedback: thread-safe cache, tier validation, unknown status tracking Co-authored-by: Bryan-Roe <74067792+Bryan-Roe@users.noreply.github.com> --- scripts/extract_chat_logs_dataset.py | 14 -------------- scripts/smart_orchestrator.py | 7 +++++-- scripts/status_dashboard.py | 24 +++++++++++++++--------- shared/subscription_manager.py | 24 ++++++++++++++++++------ 4 files changed, 38 insertions(+), 31 deletions(-) diff --git a/scripts/extract_chat_logs_dataset.py b/scripts/extract_chat_logs_dataset.py index 52fabd069..b7c20a7a8 100644 --- a/scripts/extract_chat_logs_dataset.py +++ b/scripts/extract_chat_logs_dataset.py @@ -76,20 +76,6 @@ def build_examples(messages: List[Dict], context_window: int) -> List[Dict]: # Combine lists efficiently with extend pairs.extend(windows) return pairs - # Must contain at least one user+assistant - check both roles in single pass - has_user = False - has_assistant = False - for x in window: - role = x.get("role") - if role == "user": - has_user = True - elif role == "assistant": - has_assistant = True - if has_user and has_assistant: # Early exit once both found - break - if has_user and has_assistant: - examples.append({"messages": window}) - return examples def hash_example(example: Dict) -> str: diff --git a/scripts/smart_orchestrator.py b/scripts/smart_orchestrator.py index f62155113..ccc2b3d6f 100644 --- a/scripts/smart_orchestrator.py +++ b/scripts/smart_orchestrator.py @@ -118,16 +118,18 @@ def get_stats(self) -> Dict[str, Any]: "running": 0, "pending": 0, "skipped": 0, + "unknown": 0, # Track unknown statuses separately "total_duration_sec": 0 } # Single pass through jobs for all statistics for j in self.jobs.values(): status = j.status - if status in stats: + if status in ("succeeded", "failed", "running", "pending", "skipped"): stats[status] += 1 else: - # Unknown status - log for debugging but don't crash + # Unknown status - count in separate bucket and log for debugging + stats["unknown"] += 1 logger.warning(f"Job {j.name} has unknown status: {status}") stats["total_duration_sec"] += j.duration_sec @@ -141,6 +143,7 @@ def get_stats(self) -> Dict[str, Any]: "running": stats["running"], "pending": stats["pending"], "skipped": stats["skipped"], + "unknown": stats["unknown"], "total_duration_sec": round(stats["total_duration_sec"], 2), "completion_pct": int(completed / max(total, 1) * 100), } diff --git a/scripts/status_dashboard.py b/scripts/status_dashboard.py index ecbea2c55..ac43016b4 100644 --- a/scripts/status_dashboard.py +++ b/scripts/status_dashboard.py @@ -23,6 +23,7 @@ import os import statistics import sys +import threading import time from dataclasses import dataclass, field from datetime import datetime @@ -46,9 +47,9 @@ "smart_orchestrator": REPO_ROOT / "data_out" / "smart_orchestrator", } -# Simple cache for status files to avoid repeated I/O (TTL: 5 seconds) -# Note: Not thread-safe - if concurrent access needed, add threading.Lock +# Thread-safe cache for status files to avoid repeated I/O (TTL: 5 seconds) _status_cache: Dict[str, tuple[float, Optional[Dict[str, Any]]]] = {} +_status_cache_lock = threading.Lock() _STATUS_CACHE_TTL = 5.0 # seconds @@ -85,6 +86,8 @@ def status_emoji(self) -> str: def load_status(path: Path, use_cache: bool = True) -> Optional[Dict[str, Any]]: """Load status JSON if it exists, with optional caching to reduce I/O. + Thread-safe implementation using threading.Lock for cache access. + Args: path: Path to status JSON file use_cache: If True, use cache with 5s TTL to avoid repeated reads @@ -95,11 +98,13 @@ def load_status(path: Path, use_cache: bool = True) -> Optional[Dict[str, Any]]: path_str = str(path) current_time = time.time() - # Check cache if enabled - if use_cache and path_str in _status_cache: - cached_time, cached_data = _status_cache[path_str] - if current_time - cached_time < _STATUS_CACHE_TTL: - return cached_data + # Check cache if enabled (thread-safe) + if use_cache: + with _status_cache_lock: + if path_str in _status_cache: + cached_time, cached_data = _status_cache[path_str] + if current_time - cached_time < _STATUS_CACHE_TTL: + return cached_data # Load from file if not path.exists(): @@ -111,9 +116,10 @@ def load_status(path: Path, use_cache: bool = True) -> Optional[Dict[str, Any]]: except Exception: result = None - # Update cache + # Update cache (thread-safe) if use_cache: - _status_cache[path_str] = (current_time, result) + with _status_cache_lock: + _status_cache[path_str] = (current_time, result) return result diff --git a/shared/subscription_manager.py b/shared/subscription_manager.py index c43eecfde..8edc712ea 100644 --- a/shared/subscription_manager.py +++ b/shared/subscription_manager.py @@ -119,7 +119,7 @@ def __init__( stripe_subscription_id: Optional[str] = None, ): self.user_id = user_id - self.tier = tier + self._tier = tier # Use private attribute self.start_date = start_date or datetime.now() self.end_date = end_date self.payment_method = payment_method @@ -133,9 +133,23 @@ def __init__( } self.usage_reset_date = datetime.now() + timedelta(days=30) # Cache tier limits to avoid repeated dictionary lookups - self._tier_limits = TIER_LIMITS.get(self.tier, {}) + self._tier_limits = TIER_LIMITS.get(self._tier, {}) if not self._tier_limits: - logger.warning(f"Unknown tier {self.tier} - using empty limits") + logger.warning(f"Unknown tier {self._tier} - using empty limits") + + @property + def tier(self) -> SubscriptionTier: + """Get subscription tier""" + return self._tier + + @tier.setter + def tier(self, value: SubscriptionTier): + """Set subscription tier and update cached limits""" + self._tier = value + # Update cached tier limits whenever tier changes + self._tier_limits = TIER_LIMITS.get(value, {}) + if not self._tier_limits: + logger.warning(f"Unknown tier {value} - using empty limits") def is_active(self) -> bool: """Check if subscription is currently active""" @@ -279,9 +293,7 @@ def upgrade_subscription( ) -> Subscription: """Upgrade user subscription""" sub = self.get_subscription(user_id) - sub.tier = tier - # Update cached tier limits when tier changes - sub._tier_limits = TIER_LIMITS.get(tier, {}) + sub.tier = tier # Property setter automatically updates _tier_limits sub.start_date = datetime.now() sub.end_date = datetime.now() + timedelta(days=duration_days) sub.payment_method = payment_method