diff --git a/.dockerignore b/.dockerignore index 17ae867..00b7e3f 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,17 +1,64 @@ .git .github + +# ----------------------- +# Secrets -- must never enter an image layer. +# docker/Dockerfile.api does `COPY apps/api .`, so an unignored apps/api/.env +# would be baked into the image along with its API keys. +# ----------------------- +.env +.env.* +**/.env +**/.env.* +!.env.example + +# ----------------------- +# Dependencies / virtualenvs +# Both spellings: the checked-out venv is `.venv`, and only `venv` was listed +# before, so a ~470MB Darwin-built venv was being sent to a Linux image. +# ----------------------- node_modules **/node_modules +venv +.venv +**/venv +**/.venv + +# ----------------------- +# Build outputs +# ----------------------- apps/web/.next apps/web/tsconfig.tsbuildinfo apps/web/next-env.d.ts apps/web/pnpm-lock.yaml apps/web/pnpm-workspace.yaml -apps/api/venv -apps/api/.pytest_cache -apps/api/.ruff_cache -apps/api/.coverage +dist +build +**/__pycache__ +**/*.py[cod] + +# ----------------------- +# Test / coverage artifacts +# ----------------------- +**/.pytest_cache +**/.ruff_cache +**/.coverage +**/.coverage* +**/coverage.xml +**/htmlcov +**/test_output.txt + +# ----------------------- +# Data & local state +# ----------------------- apps/api/data data Documents *.log + +# ----------------------- +# Docs (a 6.7MB PNG lives here; nothing in either image needs it) +# ----------------------- +docs +*.md +!README.md diff --git a/.env.example b/.env.example index 65648ab..e3d06ee 100644 --- a/.env.example +++ b/.env.example @@ -1,13 +1,24 @@ # =========================================== # CodebaseQA Environment Configuration # =========================================== -# Copy this file to .env and fill in your values # Never commit .env to version control! - +# +# WHERE THIS FILE GOES depends on how you run the app -- the two are different: +# +# Local dev: apps/api/.env +# config.py loads env_file=".env" relative to the working directory, +# and the README starts uvicorn from apps/api. +# +# Docker: docker/.env +# docker-compose.yml expands ${VAR} from the file next to itself. +# A .env at the repo root is NOT picked up for that interpolation. +# # ----------------------- # LLM Providers (at least one required) # ----------------------- -OPENAI_API_KEY=sk +# Leave commented until you have a real key: a placeholder is still a non-empty +# value, so the app would believe it is configured and fail only on first use. +# OPENAI_API_KEY=sk-... # ANTHROPIC_API_KEY=sk-ant-... # For local LLM (optional, no key needed) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5919581..d8112c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,15 +27,22 @@ jobs: run: | python -m pip install --upgrade pip if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - pip install pytest pytest-asyncio httpx pytest-mock pytest-cov ruff - + # Only the linter is not a project dependency; everything the tests need + # (pytest, pytest-asyncio, pytest-cov, httpx) is declared in requirements.txt + # so a fresh clone can run the suite too. Pinned so an unrelated PR cannot + # go red the day ruff ships a new rule. + pip install ruff==0.16.2 + - name: Lint with Ruff run: | ruff check src tests - + - name: Run Tests + # `tests`, not `tests/unit tests/integration`: tests/test_parser.py sits at the + # tests/ root and was silently excluded, so CI never built a single tree-sitter + # parser and local runs (pyproject testpaths = ["tests"]) covered more than CI. run: | - pytest tests/unit tests/integration --cov=src --cov-report=xml + pytest tests --cov=src --cov-report=xml - name: Upload coverage reports uses: codecov/codecov-action@v4 @@ -59,9 +66,12 @@ jobs: node-version: '20' cache: 'pnpm' + # --frozen-lockfile matches apps/web/vercel.json. With --no-frozen-lockfile CI + # silently repaired lockfile drift that the Vercel build then rejected, so a PR + # could go green and the production deploy fail on the same commit. - name: Install dependencies - run: pnpm install --no-frozen-lockfile - + run: pnpm install --frozen-lockfile + - name: Lint run: pnpm --filter web lint diff --git a/apps/api/requirements.txt b/apps/api/requirements.txt index 6c65e12..805e4c5 100644 --- a/apps/api/requirements.txt +++ b/apps/api/requirements.txt @@ -16,7 +16,8 @@ sqlalchemy>=2.0.0 aiosqlite>=0.19.0 # Vector Store -chromadb>=0.4.0 +# >=1.0: chroma_store.py references chromadb.errors.NotFoundError, absent in 0.4.x. +chromadb>=1.0.0 # LLM Providers openai>=1.12.0 @@ -24,7 +25,11 @@ anthropic>=0.18.0 tiktoken>=0.6.0 # Code Parsing -tree-sitter>=0.21.0 +# >=0.22: tree_sitter_parser.py uses Parser(language) and single-argument +# Language(capsule); both were introduced in 0.22.0, so the old >=0.21.0 floor +# permitted a version where importing the parser raises TypeError and the API +# cannot start at all. +tree-sitter>=0.22.0 tree-sitter-python>=0.21.0 tree-sitter-javascript>=0.21.0 tree-sitter-typescript>=0.21.0 @@ -44,7 +49,12 @@ cachetools>=5.3.0 structlog>=24.1.0 tenacity>=8.2.0 redis>=5.0.0 +# Imported by main.py for the /openapi.yaml route; previously only present +# transitively, so a dependency bump could have 500'd that endpoint. +pyyaml>=6.0 # Testing pytest>=8.0.0 pytest-asyncio>=0.23.0 +# pyproject.toml's addopts hardcodes --cov, so pytest cannot start without this. +pytest-cov>=4.1.0 diff --git a/apps/api/src/api/routes/learning.py b/apps/api/src/api/routes/learning.py index 2626c0d..9773a1a 100644 --- a/apps/api/src/api/routes/learning.py +++ b/apps/api/src/api/routes/learning.py @@ -1,7 +1,8 @@ +import logging from typing import Dict, List, Optional from fastapi import APIRouter, Depends, HTTPException, Query, Request -from pydantic import BaseModel +from pydantic import BaseModel, Field from sqlalchemy.orm import Session from src.core.demo_mode import assert_demo_repo_access @@ -14,6 +15,7 @@ from src.services.learning_service import LearningService router = APIRouter(tags=["learning"]) +logger = logging.getLogger(__name__) def _assert_learning_repo_access(service: LearningService, repo_id: str) -> None: @@ -138,24 +140,32 @@ async def get_lesson( raise HTTPException(status_code=500, detail=str(e)) class GenerateQuizRequest(BaseModel): - context_content: str + # Bounded like ChatMessageCreate.content (schemas.py): this string goes straight + # into an LLM prompt, so an unbounded body is an unmetered spend vector. + context_content: str = Field(..., max_length=20000) @router.post("/{repo_id}/lessons/{lesson_id}/quiz") async def generate_quiz( repo_id: str, lesson_id: str, request: GenerateQuizRequest, + http_request: Request, service: LearningService = Depends(get_learning_service) ): """Generate a quiz for a lesson.""" _assert_learning_repo_access(service, repo_id) + # This is an LLM call; it needs the same demo throttling as chat/lesson/graph. + await enforce_demo_soft_limit(http_request, "quiz") try: quiz = await service.generate_quiz(repo_id, lesson_id, request.context_content) if not quiz: raise HTTPException(status_code=500, detail="Failed to generate quiz") return quiz + except HTTPException: + raise except Exception as e: - raise HTTPException(status_code=500, detail=str(e)) + logger.exception("Quiz generation failed for repo=%s lesson=%s", repo_id, lesson_id) + raise HTTPException(status_code=500, detail="Failed to generate quiz") from e @router.get("/{repo_id}/lessons/{lesson_id}/export/codetour", response_model=CodeTour) async def export_codetour( diff --git a/apps/api/src/config.py b/apps/api/src/config.py index 1547d71..e77528c 100644 --- a/apps/api/src/config.py +++ b/apps/api/src/config.py @@ -3,12 +3,34 @@ Designed for easy self-hosting with sensible defaults. """ +import json from functools import lru_cache from typing import List, Optional -from pydantic import field_validator +from pydantic import Field, ValidationInfo, field_validator from pydantic_settings import BaseSettings, SettingsConfigDict +DEFAULT_CORS_ORIGINS = ["http://localhost:3000", "http://127.0.0.1:3000"] + +# Settings where a blank value in .env is always a mistake rather than an intent. +# pydantic-settings takes `FOO=` from the environment as the empty string, which +# silently replaces the declared default -- e.g. a blank LOCAL_EMBEDDING_MODEL +# turns into a request for the model named "". +_BLANK_MEANS_DEFAULT = ( + "llm_provider", + "embedding_provider", + "openai_model", + "openai_embedding_model", + "anthropic_model", + "ollama_model", + "ollama_base_url", + "local_embedding_model", + "database_url", + "chroma_persist_dir", + "repos_dir", + "vector_db_type", +) + class Settings(BaseSettings): """ @@ -25,7 +47,12 @@ class Settings(BaseSettings): port: int = 8000 # CORS - cors_origins: List[str] = ["http://localhost:3000", "http://127.0.0.1:3000"] + # Stored as a raw string: pydantic-settings JSON-decodes complex types (List[str]) + # inside EnvSettingsSource *before* field validators run, so a comma-separated + # CORS_ORIGINS would raise SettingsError at import and kill the process before + # uvicorn binds. Keeping it a str and parsing in the property accepts both the + # comma-separated form (docker-compose, .env.example) and a JSON array. + cors_origins_raw: Optional[str] = Field(default=None, validation_alias="CORS_ORIGINS") # Database database_url: str = "sqlite:///./data/codebaseqa.db" @@ -58,10 +85,16 @@ class Settings(BaseSettings): openai_embedding_rate_limit_max_backoff_seconds: float = 30.0 voyage_api_key: Optional[str] = None voyage_model: str = "voyage-code-3" - local_embedding_model: str = "nomic-ai/nomic-embed-text-v1.5" + # Must be an Ollama *tag* (as in `ollama pull `), not a HuggingFace repo id. + # A HF id such as "nomic-ai/nomic-embed-text-v1.5" 404s on every request, and with + # fail_open every chunk then lands in the index as a zero vector. + local_embedding_model: str = "nomic-embed-text" ollama_embedding_num_ctx: int = 2048 # Smaller context improves stability ollama_embedding_max_chars: int = 3000 # Safety cap per chunk for Ollama ollama_embedding_fail_open: bool = True # Continue indexing on occasional failures + # Ceiling on fail-open: abort the batch once this fraction of chunks has failed, + # so a total misconfiguration cannot silently produce an all-zero-vector index. + ollama_embedding_max_failure_ratio: float = 0.2 # GitHub github_token: Optional[str] = None @@ -172,26 +205,48 @@ class Settings(BaseSettings): demo_graph_window_seconds: int = 60 demo_challenge_requests: int = 10 demo_challenge_window_seconds: int = 60 + demo_quiz_requests: int = 8 + demo_quiz_window_seconds: int = 60 # Learning V2 controls learning_v2_enabled: bool = False learning_cache_ttl_days: int = 7 learning_prompt_version: str = "learning_v2_1" - @field_validator("cors_origins", mode="before") + @field_validator(*_BLANK_MEANS_DEFAULT, mode="before") @classmethod - def parse_cors_origins(cls, value): + def _blank_falls_back_to_default(cls, value, info: ValidationInfo): + """Treat a blank env assignment (FOO=) as 'unset' rather than the empty string.""" + if isinstance(value, str) and not value.strip(): + field = cls.model_fields.get(info.field_name) + if field is not None and field.default is not None: + return field.default + return value + + @property + def cors_origins(self) -> List[str]: """ - Accept JSON lists or comma-separated strings for CORS_ORIGINS. + Allowed CORS origins. Accepts a JSON array or a comma-separated string. + Falls back to the localhost defaults when unset or blank. """ - if isinstance(value, str): - stripped = value.strip() - if not stripped: - return [] - if stripped.startswith("["): - return value - return [origin.strip() for origin in stripped.split(",") if origin.strip()] - return value + raw = self.cors_origins_raw + if raw is None: + return list(DEFAULT_CORS_ORIGINS) + + stripped = raw.strip() + if not stripped: + return [] + + if stripped.startswith("["): + try: + parsed = json.loads(stripped) + except json.JSONDecodeError: + return list(DEFAULT_CORS_ORIGINS) + if isinstance(parsed, list): + return [str(origin).strip() for origin in parsed if str(origin).strip()] + return list(DEFAULT_CORS_ORIGINS) + + return [origin.strip() for origin in stripped.split(",") if origin.strip()] model_config = SettingsConfigDict( env_file=".env", diff --git a/apps/api/src/core/cache/chat_cache.py b/apps/api/src/core/cache/chat_cache.py index 7402ba3..8367744 100644 --- a/apps/api/src/core/cache/chat_cache.py +++ b/apps/api/src/core/cache/chat_cache.py @@ -101,6 +101,7 @@ def _key_answer( intent: str, top_chunk_ids: List[str], model: str, + history_digest: str = "none", ) -> str: digest = _hash_payload( { @@ -109,6 +110,10 @@ def _key_answer( "intent": intent, "chunk_ids": top_chunk_ids[:12], "model": model, + # The prompt includes conversation history, so the key must too -- + # otherwise the same question in two different sessions collides and + # one session is served the other's history-conditioned answer. + "history": history_digest, } ) return f"chat:answer:{digest}" @@ -167,8 +172,9 @@ async def get_answer( intent: str, top_chunk_ids: List[str], model: str, + history_digest: str = "none", ) -> Optional[str]: - key = self._key_answer(repo_id, question, intent, top_chunk_ids, model) + key = self._key_answer(repo_id, question, intent, top_chunk_ids, model, history_digest) value = await self._redis_get(key) if value is None: value = self._local_get(self._answer_cache, key) @@ -186,8 +192,9 @@ async def set_answer( top_chunk_ids: List[str], model: str, answer: str, + history_digest: str = "none", ) -> None: - key = self._key_answer(repo_id, question, intent, top_chunk_ids, model) + key = self._key_answer(repo_id, question, intent, top_chunk_ids, model, history_digest) self._local_set(self._answer_cache, key, answer) await self._redis_set(key, answer, settings.chat_answer_cache_ttl_seconds) diff --git a/apps/api/src/core/embeddings/factory.py b/apps/api/src/core/embeddings/factory.py index b1710b1..ccf3ef5 100644 --- a/apps/api/src/core/embeddings/factory.py +++ b/apps/api/src/core/embeddings/factory.py @@ -28,6 +28,7 @@ def create_embedding_service() -> BaseEmbeddings: max_chars=settings.ollama_embedding_max_chars, num_ctx=settings.ollama_embedding_num_ctx, fail_open=settings.ollama_embedding_fail_open, + max_failure_ratio=settings.ollama_embedding_max_failure_ratio, ) else: # Fallback/Default or Raise diff --git a/apps/api/src/core/embeddings/ollama_embeddings.py b/apps/api/src/core/embeddings/ollama_embeddings.py index 4b3a240..2e32335 100644 --- a/apps/api/src/core/embeddings/ollama_embeddings.py +++ b/apps/api/src/core/embeddings/ollama_embeddings.py @@ -7,6 +7,17 @@ logger = logging.getLogger(__name__) + +class OllamaModelNotFound(RuntimeError): + """ + Ollama does not have the requested model. + + Deliberately not an httpx exception: the retry policy below retries httpx errors, + and a missing model is permanent, so retrying it just burns the whole backoff + budget (10 attempts, up to 30s apart) on every single chunk before failing. + """ + + class OllamaEmbeddings(BaseEmbeddings): """Ollama embedding service for local embeddings.""" @@ -20,6 +31,9 @@ class OllamaEmbeddings(BaseEmbeddings): "llama3.1": 4096, } + # Don't judge the failure ratio off the first one or two chunks. + FAILURE_RATIO_MIN_SAMPLE = 5 + def __init__( self, base_url: str = "http://localhost:11434", @@ -27,6 +41,7 @@ def __init__( max_chars: int | None = None, num_ctx: int | None = None, fail_open: bool = True, + max_failure_ratio: float = 0.2, ): self._base_url = base_url.rstrip("/") self._model = model @@ -34,6 +49,7 @@ def __init__( self._max_chars = max_chars self._num_ctx = num_ctx self._fail_open = fail_open + self._max_failure_ratio = max(0.0, min(1.0, float(max_failure_ratio))) # Try to infer dimensions if model name contains typical hints, or defaulting @property @@ -87,7 +103,10 @@ async def _embed_one(client, text): if response.status_code == 404: logger.error(f"Model {self._model} not found in Ollama. Please run: ollama pull {self._model}") - raise httpx.HTTPStatusError("Model not found", request=response.request, response=response) + raise OllamaModelNotFound( + f"Ollama has no model '{self._model}'. Run: ollama pull {self._model} " + f"(LOCAL_EMBEDDING_MODEL must be an Ollama tag, not a HuggingFace repo id)." + ) if response.status_code >= 500: logger.error(f"Ollama Server Error ({response.status_code}): {response.text}") @@ -106,6 +125,7 @@ async def _embed_one(client, text): return payload["embedding"] embeddings = [] + failures = 0 # Use a longer timeout for the client session overall, though per-request applies async with httpx.AsyncClient(timeout=120.0) as client: total = len(texts) @@ -122,9 +142,28 @@ async def _embed_one(client, text): if (i + 1) % 10 == 0: logger.debug(f"Embedded {i+1}/{total} chunks") + except OllamaModelNotFound: + # Permanent misconfiguration: never fail open, never retry. + raise except Exception as e: logger.error(f"Ollama embedding failed at index {i} (text length: {len(val)}): {e}") if self._fail_open: + failures += 1 + # Fail-open is meant to absorb the occasional hiccup, not a + # total misconfiguration (e.g. a model name Ollama cannot + # resolve, which 404s on every chunk). Past the ceiling, stop: + # an index of zero vectors scores every chunk identically and + # makes retrieval arbitrary while reporting full confidence. + attempted = i + 1 + if attempted >= self.FAILURE_RATIO_MIN_SAMPLE and ( + failures / attempted + ) > self._max_failure_ratio: + raise RuntimeError( + f"Ollama embedding aborted: {failures}/{attempted} chunks failed " + f"(> {self._max_failure_ratio:.0%} ceiling). " + f"Check that model '{self._model}' is pulled and reachable at " + f"{self._base_url} -- it must be an Ollama tag, not a HuggingFace id." + ) from e embeddings.append([0.0] * self.dimensions) continue raise diff --git a/apps/api/src/core/github/repo_manager.py b/apps/api/src/core/github/repo_manager.py index 39c3b1e..1036651 100644 --- a/apps/api/src/core/github/repo_manager.py +++ b/apps/api/src/core/github/repo_manager.py @@ -10,12 +10,17 @@ import subprocess from pathlib import Path from typing import Optional, Tuple -from urllib.parse import unquote, urlparse +from urllib.parse import unquote, urlparse, urlunparse from src.config import settings logger = logging.getLogger(__name__) +# Hosts that may receive the GitHub token. Must be matched exactly against the +# parsed hostname -- a substring test such as `"github.com" in url` also matches +# lookalike hosts like `github.com.attacker.tld`, which would hand the token to them. +GITHUB_TOKEN_HOSTS = frozenset({"github.com", "www.github.com"}) + class RepoManager: """Manages GitHub repository operations.""" @@ -24,6 +29,38 @@ def __init__(self): self._repos_dir = Path(settings.repos_dir) self._repos_dir.mkdir(parents=True, exist_ok=True) + @staticmethod + def _authenticated_url(github_url: str) -> str: + """ + Return the clone URL with the GitHub token injected, but only when the URL's + host is exactly a GitHub host. Any other host gets the URL unchanged. + """ + token = settings.github_token + if not token: + return github_url + + parsed = urlparse(github_url) + hostname = (parsed.hostname or "").lower() + if parsed.scheme not in {"http", "https"} or hostname not in GITHUB_TOKEN_HOSTS: + return github_url + + netloc = f"{token}@{hostname}" + if parsed.port: + netloc = f"{netloc}:{parsed.port}" + return urlunparse(parsed._replace(netloc=netloc)) + + @staticmethod + def _redact(text: str) -> str: + """ + Strip the GitHub token from subprocess output before it is logged, stored in + Repository.indexing_error, or streamed to a client. git echoes the full clone + URL -- credentials included -- in messages like 'fatal: repository ... not found'. + """ + token = settings.github_token + if not token or not text: + return text + return text.replace(token, "***") + @staticmethod def _sanitize_repo_segment(value: str, label: str) -> str: segment = (value or "").strip() @@ -56,12 +93,7 @@ def get_local_path(self, owner: str, name: str) -> Path: async def get_default_branch(self, github_url: str) -> str: """Get the default branch of a repository using git ls-remote.""" - url = github_url - if settings.github_token and "github.com" in github_url: - url = github_url.replace( - "https://github.com", - f"https://{settings.github_token}@github.com" - ) + url = self._authenticated_url(github_url) try: result = subprocess.run( @@ -114,19 +146,8 @@ async def clone_repository( "--branch", branch, ] - # Add token if available - if settings.github_token: - # Insert token into URL for private repos - if "github.com" in github_url: - url_with_auth = github_url.replace( - "https://github.com", - f"https://{settings.github_token}@github.com" - ) - cmd.append(url_with_auth) - else: - cmd.append(github_url) - else: - cmd.append(github_url) + # Inject the token only for genuine GitHub hosts (see _authenticated_url). + cmd.append(self._authenticated_url(github_url)) cmd.append(str(local_path)) @@ -141,7 +162,9 @@ async def clone_repository( ) if result.returncode != 0: - raise Exception(f"Git clone failed: {result.stderr}") + # Redact: stderr contains the credentialed clone URL, and this message + # is persisted to Repository.indexing_error and served over the API. + raise Exception(f"Git clone failed: {self._redact(result.stderr)}") return local_path diff --git a/apps/api/src/core/llm/anthropic_llm.py b/apps/api/src/core/llm/anthropic_llm.py index 425e234..8afff59 100644 --- a/apps/api/src/core/llm/anthropic_llm.py +++ b/apps/api/src/core/llm/anthropic_llm.py @@ -4,7 +4,7 @@ from anthropic import AsyncAnthropic -from src.core.llm.base import BaseLLM +from src.core.llm.base import BaseLLM, stream_error_text logger = logging.getLogger(__name__) @@ -82,7 +82,11 @@ async def generate_stream(self, messages: List[Dict[str, str]]) -> AsyncGenerato await asyncio.sleep(wait_time) continue logger.error(f"Anthropic streaming failed: {e}") - yield f"\n\n[Error: {str(e)[:100]}]" + if not yielded: + # See openai_llm.generate_stream: raise so the route reports a real + # error rather than caching/persisting the error text as an answer. + raise + yield stream_error_text(e) return async def health_check(self) -> bool: diff --git a/apps/api/src/core/llm/base.py b/apps/api/src/core/llm/base.py index dab6d48..eedde3f 100644 --- a/apps/api/src/core/llm/base.py +++ b/apps/api/src/core/llm/base.py @@ -1,6 +1,17 @@ from abc import ABC, abstractmethod from typing import AsyncGenerator, Dict, List +# Marker a provider emits inline when a stream fails *after* it has already sent +# tokens (a total failure raises instead). Callers must not cache or persist a +# response containing this -- otherwise one transient upstream error is served to +# every subsequent asker for the life of the cache entry. +STREAM_ERROR_MARKER = "[stream-error]" + + +def stream_error_text(exc: Exception) -> str: + """Inline text appended to a partially-delivered stream that then failed.""" + return f"\n\n{STREAM_ERROR_MARKER} generation was interrupted: {str(exc)[:100]}" + class BaseLLM(ABC): """Abstract base class for LLM providers.""" diff --git a/apps/api/src/core/llm/ollama_llm.py b/apps/api/src/core/llm/ollama_llm.py index 9e0e120..c3cf63b 100644 --- a/apps/api/src/core/llm/ollama_llm.py +++ b/apps/api/src/core/llm/ollama_llm.py @@ -5,7 +5,7 @@ import httpx -from src.core.llm.base import BaseLLM +from src.core.llm.base import BaseLLM, stream_error_text logger = logging.getLogger(__name__) @@ -83,7 +83,11 @@ async def generate_stream(self, messages: List[Dict[str, str]]) -> AsyncGenerato await asyncio.sleep(wait_time) continue logger.error(f"Ollama streaming failed: {e}") - yield f"\n\n[Error: {str(e)[:100]}]" + if not yielded: + # See openai_llm.generate_stream: raise so the route reports a real + # error rather than caching/persisting the error text as an answer. + raise + yield stream_error_text(e) return async def health_check(self) -> bool: diff --git a/apps/api/src/core/llm/openai_llm.py b/apps/api/src/core/llm/openai_llm.py index 6d8a95e..df62596 100644 --- a/apps/api/src/core/llm/openai_llm.py +++ b/apps/api/src/core/llm/openai_llm.py @@ -8,7 +8,7 @@ from openai import AsyncOpenAI -from src.core.llm.base import BaseLLM +from src.core.llm.base import BaseLLM, stream_error_text logger = logging.getLogger(__name__) @@ -112,7 +112,13 @@ async def generate_stream( await asyncio.sleep(wait_time) continue logger.error(f"Streaming generation failed: {e}") - yield f"\n\n[Error: {str(e)[:100]}]" + if not yielded: + # Nothing reached the client yet. Raise so the route emits a real + # SSE error event with a code, instead of a "successful" stream + # whose entire content is an error string that then gets cached + # and persisted as the assistant's answer. + raise + yield stream_error_text(e) return async def health_check(self) -> bool: diff --git a/apps/api/src/core/parser/tree_sitter_parser.py b/apps/api/src/core/parser/tree_sitter_parser.py index 593e7c2..d1f90fa 100644 --- a/apps/api/src/core/parser/tree_sitter_parser.py +++ b/apps/api/src/core/parser/tree_sitter_parser.py @@ -49,6 +49,10 @@ class ParseResult: imports: List[str] exports: List[str] line_count: int = 0 + # True when tree-sitter produced ERROR nodes. Tree-sitter does not raise on + # syntax it cannot handle -- it returns a tree containing ERROR nodes -- so + # callers must check this explicitly or they will silently index garbage chunks. + has_errors: bool = False class TreeSitterParser: @@ -76,13 +80,23 @@ class TreeSitterParser: "class_body_types": ["class_body"], }, "typescript": { - "extensions": [".ts", ".tsx"], + "extensions": [".ts"], "language": Language(tstypescript.language_typescript()), "function_types": ["function_declaration", "method_definition", "arrow_function"], "class_types": ["class_declaration", "interface_declaration", "enum_declaration"], "import_types": ["import_declaration", "import_statement"], "class_body_types": ["class_body"], }, + # .tsx needs the dedicated TSX grammar: the plain TypeScript grammar cannot + # parse JSX, so every .tsx file produced an ERROR tree and mis-aligned chunks. + "tsx": { + "extensions": [".tsx"], + "language": Language(tstypescript.language_tsx()), + "function_types": ["function_declaration", "method_definition", "arrow_function"], + "class_types": ["class_declaration", "interface_declaration", "enum_declaration"], + "import_types": ["import_declaration", "import_statement"], + "class_body_types": ["class_body"], + }, "java": { "extensions": [".java"], "language": Language(tsjava.language()), @@ -212,6 +226,7 @@ def visit(node: Node, in_class: bool = False) -> None: imports=imports, exports=[], line_count=content.count('\n') + 1, + has_errors=bool(root.has_error), ) def _process_function(self, node: Node, content: str, context: str) -> Optional[CodeChunk]: @@ -338,7 +353,19 @@ def _config_list(self, key: str, default: Sequence[str]) -> List[str]: return list(default) def _get_text(self, node: Node, content: str) -> str: - return content[node.start_byte:node.end_byte] + """ + Return a node's source text. + + Uses node.text (the parser's own byte slice) rather than + content[node.start_byte:node.end_byte]. start_byte/end_byte are *byte* + offsets, so slicing the decoded str with them desynchronises as soon as the + file contains a single multi-byte character -- every chunk after that point + gets the wrong boundaries. + """ + if node.text is not None: + return node.text.decode("utf-8", errors="replace") + # Fallback: re-encode and slice by byte offset (correct, just slower). + return content.encode("utf-8")[node.start_byte:node.end_byte].decode("utf-8", errors="replace") @lru_cache(maxsize=20) diff --git a/apps/api/src/core/rag/pipeline.py b/apps/api/src/core/rag/pipeline.py index 48357f1..9abb0c1 100644 --- a/apps/api/src/core/rag/pipeline.py +++ b/apps/api/src/core/rag/pipeline.py @@ -5,6 +5,7 @@ from __future__ import annotations +import hashlib import json import logging import re @@ -15,6 +16,7 @@ from src.config import settings from src.core.cache.chat_cache import ChatCache +from src.core.llm.base import STREAM_ERROR_MARKER logger = logging.getLogger(__name__) @@ -712,7 +714,27 @@ def _build_messages( def _llm_model_name(self) -> str: return str(getattr(self._llm, "_model", self._llm.__class__.__name__)) - async def _get_cached_answer(self, query: str, context: RetrievalResult) -> Optional[str]: + def _history_digest(self, history: Optional[List[Dict[str, str]]]) -> str: + """ + Fingerprint the history that will actually go into the prompt. + + The answer cache key must include this: _build_messages feeds history into + the prompt, so two sessions asking the same question with different history + get different answers. Without it, the second session is served the first + session's history-conditioned answer. + """ + budgeted = self._apply_history_budget(history) + if not budgeted: + return "none" + payload = json.dumps(budgeted, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(payload.encode()).hexdigest() + + async def _get_cached_answer( + self, + query: str, + context: RetrievalResult, + history: Optional[List[Dict[str, str]]] = None, + ) -> Optional[str]: if not self._chat_cache: return None top_chunk_ids = [chunk.id for chunk in context.chunks[:12]] @@ -722,11 +744,23 @@ async def _get_cached_answer(self, query: str, context: RetrievalResult) -> Opti intent=context.intent, top_chunk_ids=top_chunk_ids, model=self._llm_model_name(), + history_digest=self._history_digest(history), ) - async def _set_cached_answer(self, query: str, context: RetrievalResult, answer: str) -> None: + async def _set_cached_answer( + self, + query: str, + context: RetrievalResult, + answer: str, + history: Optional[List[Dict[str, str]]] = None, + ) -> None: if not self._chat_cache: return + # Never cache a response that carries the stream-failure marker: it would be + # replayed to everyone asking this question until the entry expires. + if STREAM_ERROR_MARKER in answer: + logger.warning("Skipping answer cache write for repo=%s: response was interrupted", self._repo_id) + return top_chunk_ids = [chunk.id for chunk in context.chunks[:12]] await self._chat_cache.set_answer( repo_id=self._repo_id, @@ -735,6 +769,7 @@ async def _set_cached_answer(self, query: str, context: RetrievalResult, answer: top_chunk_ids=top_chunk_ids, model=self._llm_model_name(), answer=answer, + history_digest=self._history_digest(history), ) async def generate( @@ -744,13 +779,13 @@ async def generate( history: Optional[List[Dict[str, str]]] = None, ) -> str: """Generate a non-streaming response.""" - cached = await self._get_cached_answer(query=query, context=context) + cached = await self._get_cached_answer(query=query, context=context, history=history) if cached is not None: return cached messages = self._build_messages(query=query, context=context, history=history) result = await self._llm.generate(messages) - await self._set_cached_answer(query=query, context=context, answer=result) + await self._set_cached_answer(query=query, context=context, answer=result, history=history) return result async def generate_stream( @@ -760,7 +795,7 @@ async def generate_stream( history: Optional[List[Dict[str, str]]] = None, ) -> AsyncGenerator[str, None]: """Generate a streaming response.""" - cached = await self._get_cached_answer(query=query, context=context) + cached = await self._get_cached_answer(query=query, context=context, history=history) if cached is not None: for i in range(0, len(cached), 320): yield cached[i : i + 320] @@ -773,4 +808,9 @@ async def generate_stream( yield token if pieces: - await self._set_cached_answer(query=query, context=context, answer="".join(pieces)) + await self._set_cached_answer( + query=query, + context=context, + answer="".join(pieces), + history=history, + ) diff --git a/apps/api/src/core/rate_limit.py b/apps/api/src/core/rate_limit.py index 2a59505..8ebd8cb 100644 --- a/apps/api/src/core/rate_limit.py +++ b/apps/api/src/core/rate_limit.py @@ -25,6 +25,7 @@ def _bucket_limits() -> Dict[str, BucketConfig]: "lesson": (settings.demo_lesson_requests, settings.demo_lesson_window_seconds), "graph": (settings.demo_graph_requests, settings.demo_graph_window_seconds), "challenge": (settings.demo_challenge_requests, settings.demo_challenge_window_seconds), + "quiz": (settings.demo_quiz_requests, settings.demo_quiz_window_seconds), } diff --git a/apps/api/src/main.py b/apps/api/src/main.py index 819665c..731f639 100644 --- a/apps/api/src/main.py +++ b/apps/api/src/main.py @@ -137,7 +137,10 @@ async def health_check(): llm = get_llm_service() if hasattr(llm, 'health_check'): is_healthy = await llm.health_check() - checks["llm_provider"] = "ok" if is_healthy else "unavailable/unreachable" + # Must contain "error" so the critical_ok test below actually fails -- + # the previous "unavailable/unreachable" wording passed that check, so an + # unreachable LLM was reported as healthy. + checks["llm_provider"] = "ok" if is_healthy else "error: provider unreachable" except Exception as e: checks["llm_provider"] = f"error: {str(e)[:50]}" @@ -172,7 +175,13 @@ async def health_check(): if not demo_repo: checks["demo_repo"] = "initializing" else: - checks["demo_repo"] = f"{demo_repo.status} ({demo_repo.github_owner}/{demo_repo.github_name})" + # .value, not the member: IndexingStatus is a str-Enum, and since + # Python 3.11 f"{member}" renders "IndexingStatus.COMPLETED", so the + # startswith("completed") test below could never match and demo mode + # reported "degraded" permanently. + checks["demo_repo"] = ( + f"{demo_repo.status.value} ({demo_repo.github_owner}/{demo_repo.github_name})" + ) except Exception as e: checks["demo_repo"] = f"error: {str(e)[:50]}" finally: diff --git a/apps/api/src/services/indexing_service.py b/apps/api/src/services/indexing_service.py index 9150f85..4f6f141 100644 --- a/apps/api/src/services/indexing_service.py +++ b/apps/api/src/services/indexing_service.py @@ -416,6 +416,18 @@ async def _parse_file( logger.warning("Parser failed for %s, falling back to raw indexing: %s", file_path, exc) return await self._index_raw_file(repo, file_path, repo_path, content) + # tree-sitter returns an ERROR-node tree rather than raising when it cannot + # parse the file, so the except above never fires for a grammar mismatch. + # Chunks carved out of an ERROR tree have wrong boundaries -- raw indexing + # is strictly better than mis-aligned AST chunks. + if result.has_errors: + logger.warning( + "Parser produced an ERROR tree for %s (language=%s); falling back to raw indexing", + file_path, + result.language, + ) + return await self._index_raw_file(repo, file_path, repo_path, content) + # Create CodeFile record relative_path = str(file_path.relative_to(repo_path)) content_hash = hashlib.sha256(content.encode()).hexdigest() diff --git a/apps/api/tests/conftest.py b/apps/api/tests/conftest.py index 95a8196..4792ff0 100644 --- a/apps/api/tests/conftest.py +++ b/apps/api/tests/conftest.py @@ -2,7 +2,7 @@ import pytest from fastapi.testclient import TestClient -from httpx import AsyncClient +from httpx import ASGITransport, AsyncClient from src.main import app @@ -14,8 +14,15 @@ def client() -> TestClient: @pytest.fixture async def async_client() -> AsyncGenerator[AsyncClient, None]: - """Asynchronous test client for testing async endpoints.""" - async with AsyncClient(app=app, base_url="http://test") as ac: + """ + Asynchronous test client for testing async endpoints. + + Uses ASGITransport explicitly: the `app=` shortcut was deprecated in httpx 0.27 + and removed in 0.28, so `AsyncClient(app=app)` raises TypeError on any currently + installable httpx. + """ + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://test") as ac: yield ac @pytest.fixture diff --git a/apps/web/package.json b/apps/web/package.json index 46185e0..3d42bf3 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -7,8 +7,10 @@ "build": "next build --webpack", "start": "next start", "lint": "eslint", - "test": "vitest", - "type-check": "tsc --noEmit" + "test": "vitest run", + "test:watch": "vitest", + "type-check": "next typegen && tsc --noEmit", + "clean": "rm -rf .next node_modules tsconfig.tsbuildinfo" }, "dependencies": { "@types/react-syntax-highlighter": "^15.5.13", diff --git a/apps/web/src/lib/api-client.ts b/apps/web/src/lib/api-client.ts index 180b28b..d0d04f3 100644 --- a/apps/web/src/lib/api-client.ts +++ b/apps/web/src/lib/api-client.ts @@ -298,31 +298,59 @@ class ApiClient { const retryAfterHeader = res.headers.get('Retry-After'); const retryAfter = retryAfterHeader ? Number.parseInt(retryAfterHeader, 10) : undefined; + // Read the body exactly once. res.json() consumes the stream even when parsing + // fails, so a subsequent res.text() throws "body stream already read" -- which is + // why every non-JSON error body (an nginx 502 page, a plain-text 500) used to be + // silently replaced by the generic fallback. + let raw = ''; try { - const payload = await res.json(); - if (payload?.detail && typeof payload.detail === 'object') { - return new ApiError( - payload.detail.message || fallbackMessage, - res.status, - payload.detail.code, - payload.detail.retry_after_seconds ?? retryAfter, - ); - } - - if (payload?.detail && typeof payload.detail === 'string') { - return new ApiError(payload.detail, res.status, undefined, retryAfter); - } + raw = await res.text(); } catch { - // Fall back to text parsing below. + return new ApiError(fallbackMessage, res.status, undefined, retryAfter); } + let payload: unknown; try { - const text = await res.text(); - const clean = text.trim(); - return new ApiError(clean || fallbackMessage, res.status, undefined, retryAfter); + payload = JSON.parse(raw); } catch { - return new ApiError(fallbackMessage, res.status, undefined, retryAfter); + // Not JSON: surface the body itself, which is more useful than a generic string. + return new ApiError(raw.trim() || fallbackMessage, res.status, undefined, retryAfter); } + + const detail = (payload as { detail?: unknown } | null)?.detail; + + if (typeof detail === 'string') { + return new ApiError(detail, res.status, undefined, retryAfter); + } + + // FastAPI validation errors send `detail` as an *array* of {loc, msg, type}. + // typeof [] === 'object', so the object branch below used to swallow these and + // read .message off an array -- turning every 422 into the generic fallback. + if (Array.isArray(detail)) { + const messages = detail + .map((item) => { + const entry = item as { loc?: unknown[]; msg?: string } | null; + if (!entry?.msg) return null; + const field = Array.isArray(entry.loc) + ? entry.loc.filter((p) => p !== 'body').join('.') + : ''; + return field ? `${field}: ${entry.msg}` : entry.msg; + }) + .filter((m): m is string => Boolean(m)); + return new ApiError(messages.join('; ') || fallbackMessage, res.status, 'VALIDATION_ERROR', retryAfter); + } + + if (detail && typeof detail === 'object') { + const obj = detail as { message?: string; code?: string; retry_after_seconds?: number }; + return new ApiError( + obj.message || fallbackMessage, + res.status, + obj.code, + obj.retry_after_seconds ?? retryAfter, + ); + } + + return new ApiError(fallbackMessage, res.status, undefined, retryAfter); } private async ensureOk(res: Response, fallbackMessage: string): Promise { diff --git a/docker/README.md b/docker/README.md index df71214..97b52f7 100644 --- a/docker/README.md +++ b/docker/README.md @@ -2,9 +2,15 @@ ## Quick Start -1. Create a `.env` file in this directory: +1. Create a `.env` file **in this directory** (`docker/.env`). Compose expands + `${VAR}` from the file next to `docker-compose.yml`; a `.env` at the repo root is + not used for that. See `.env.example` for the full set of supported variables. ```bash OPENAI_API_KEY=sk-... +# optional: switch providers, clone private repos +# LLM_PROVIDER=anthropic +# ANTHROPIC_API_KEY=sk-ant-... +# GITHUB_TOKEN=ghp_... ``` 2. Build and run: diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index b4f99a1..e102069 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -25,9 +25,29 @@ services: - DATABASE_URL=sqlite:///./data/codebaseqa.db - CHROMA_PERSIST_DIR=./data/chroma - REDIS_URL=${REDIS_URL:-redis://redis:6379/0} + - DEBUG=${DEBUG:-false} + # --- provider selection: without these the container can only ever use OpenAI --- + - LLM_PROVIDER=${LLM_PROVIDER:-openai} + - EMBEDDING_PROVIDER=${EMBEDDING_PROVIDER:-openai} - OPENAI_API_KEY=${OPENAI_API_KEY} - OPENAI_MODEL=${OPENAI_MODEL:-gpt-4o} + - OPENAI_BASE_URL=${OPENAI_BASE_URL:-} - OPENAI_EMBEDDING_MODEL=${OPENAI_EMBEDDING_MODEL:-text-embedding-3-small} + - ANTHROPIC_API_KEY=${ANTHROPIC_API_KEY:-} + - ANTHROPIC_MODEL=${ANTHROPIC_MODEL:-claude-sonnet-4-20250514} + - OLLAMA_BASE_URL=${OLLAMA_BASE_URL:-http://localhost:11434} + - OLLAMA_MODEL=${OLLAMA_MODEL:-llama3.1} + - LOCAL_EMBEDDING_MODEL=${LOCAL_EMBEDDING_MODEL:-nomic-embed-text} + # --- required to clone private repositories --- + - GITHUB_TOKEN=${GITHUB_TOKEN:-} + # --- embedding throughput / rate-limit controls --- + - OPENAI_EMBEDDING_REQUEST_CONCURRENCY=${OPENAI_EMBEDDING_REQUEST_CONCURRENCY:-1} + - OPENAI_EMBEDDING_MAX_TEXTS_PER_REQUEST=${OPENAI_EMBEDDING_MAX_TEXTS_PER_REQUEST:-128} + - OPENAI_EMBEDDING_MAX_TOKENS_PER_REQUEST=${OPENAI_EMBEDDING_MAX_TOKENS_PER_REQUEST:-250000} + - OPENAI_EMBEDDING_MIN_SECONDS_BETWEEN_REQUESTS=${OPENAI_EMBEDDING_MIN_SECONDS_BETWEEN_REQUESTS:-0.0} + - OPENAI_EMBEDDING_RATE_LIMIT_MAX_RETRIES=${OPENAI_EMBEDDING_RATE_LIMIT_MAX_RETRIES:-6} + - OPENAI_EMBEDDING_RATE_LIMIT_BASE_BACKOFF_SECONDS=${OPENAI_EMBEDDING_RATE_LIMIT_BASE_BACKOFF_SECONDS:-1.0} + - OPENAI_EMBEDDING_RATE_LIMIT_MAX_BACKOFF_SECONDS=${OPENAI_EMBEDDING_RATE_LIMIT_MAX_BACKOFF_SECONDS:-30.0} - CHAT_INTENT_ROUTING_ENABLED=${CHAT_INTENT_ROUTING_ENABLED:-true} - CHAT_CONTENT_RERANK_ENABLED=${CHAT_CONTENT_RERANK_ENABLED:-true} - CHAT_DOCS_FIRST_OVERVIEW_ENABLED=${CHAT_DOCS_FIRST_OVERVIEW_ENABLED:-true} diff --git a/turbo.json b/turbo.json index 67684c1..9f5660a 100644 --- a/turbo.json +++ b/turbo.json @@ -1,9 +1,12 @@ { "$schema": "https://turbo.build/schema.json", "globalDependencies": [ - ".env.local" + "apps/web/.env.local" ], "tasks": { + "clean": { + "cache": false + }, "build": { "dependsOn": [ "^build" @@ -25,7 +28,7 @@ }, "test": { "dependsOn": [ - "build" + "^build" ] }, "type-check": {