LazyHuman10 commited on
Commit
63d77fa
·
1 Parent(s): 88f8aa7

fix: resolve concurrency and caching bugs in API

Browse files
Files changed (3) hide show
  1. main.py +47 -21
  2. rag.py +91 -42
  3. requirements.txt +1 -0
main.py CHANGED
@@ -10,13 +10,13 @@ The heavy resources (index + embedding model) are loaded ONCE at startup via
10
  FastAPI's lifespan context manager and shared across all requests.
11
  """
12
 
 
13
  import os
14
  import time
15
  from contextlib import asynccontextmanager
16
- from functools import lru_cache
17
 
18
- import requests
19
- from fastapi import FastAPI, HTTPException, Request
20
  from fastapi.middleware.cors import CORSMiddleware
21
  from fastapi.responses import JSONResponse
22
  from pydantic import BaseModel, Field
@@ -50,7 +50,10 @@ async def lifespan(app: FastAPI):
50
  """Load the RAG index at startup; release on shutdown."""
51
  print("Loading RAG index from GitHub…")
52
  t0 = time.time()
53
- index, error = load_index()
 
 
 
54
  elapsed = round(time.time() - t0, 2)
55
 
56
  if error:
@@ -118,25 +121,41 @@ class RetrieveResponse(BaseModel):
118
 
119
 
120
  # ---------------------------------------------------------------------------
121
- # Manifest caching (simple in-memory, 5-minute TTL)
122
  # ---------------------------------------------------------------------------
123
  _manifest_cache: dict = {"data": None, "fetched_at": 0}
124
  MANIFEST_TTL = 300 # seconds
 
125
 
126
 
127
- def _get_manifest() -> dict:
128
- now = time.time()
129
- if _manifest_cache["data"] and (now - _manifest_cache["fetched_at"]) < MANIFEST_TTL:
130
- return _manifest_cache["data"]
 
 
 
131
 
132
- url = f"https://raw.githubusercontent.com/{MATERIALS_REPO}/{MANIFEST_BRANCH}/manifest.json"
133
- resp = requests.get(url, timeout=15)
134
- resp.raise_for_status()
135
- data = resp.json()
 
 
 
136
 
137
- _manifest_cache["data"] = data
138
- _manifest_cache["fetched_at"] = now
139
- return data
 
 
 
 
 
 
 
 
 
140
 
141
 
142
  # ---------------------------------------------------------------------------
@@ -156,22 +175,22 @@ def health():
156
 
157
 
158
  @app.get("/manifest")
159
- def get_manifest():
160
  """
161
  Proxy and cache the study materials manifest.json from GitHub.
162
  The Cloudflare Worker also caches this in KV — this is a double layer.
163
  """
164
  try:
165
- data = _get_manifest()
166
  return JSONResponse(content=data)
167
- except requests.HTTPError as err:
168
  raise HTTPException(status_code=502, detail=f"GitHub fetch failed: {err}")
169
  except Exception as err:
170
  raise HTTPException(status_code=500, detail=str(err))
171
 
172
 
173
  @app.post("/retrieve", response_model=RetrieveResponse)
174
- def retrieve(body: RetrieveRequest):
175
  """
176
  Core RAG endpoint.
177
 
@@ -179,10 +198,17 @@ def retrieve(body: RetrieveRequest):
179
  2. Searches the pre-built LlamaIndex vector store
180
  3. Filters results by semester + subject metadata
181
  4. Returns top-k chunks + a formatted context string for the LLM prompt
 
 
 
182
  """
183
  index = _state.get("index")
184
 
185
- chunks = retrieve_chunks(
 
 
 
 
186
  index=index,
187
  query=body.query,
188
  semester=body.semester,
 
10
  FastAPI's lifespan context manager and shared across all requests.
11
  """
12
 
13
+ import asyncio
14
  import os
15
  import time
16
  from contextlib import asynccontextmanager
 
17
 
18
+ import httpx # API-BUG-2: async HTTP client — replaces blocking requests
19
+ from fastapi import FastAPI, HTTPException
20
  from fastapi.middleware.cors import CORSMiddleware
21
  from fastapi.responses import JSONResponse
22
  from pydantic import BaseModel, Field
 
50
  """Load the RAG index at startup; release on shutdown."""
51
  print("Loading RAG index from GitHub…")
52
  t0 = time.time()
53
+ # API-BUG-3: load_index() makes blocking HTTP calls (requests.get) and does
54
+ # heavy CPU work (embedding model init). Running it in a thread pool keeps
55
+ # the async event loop from freezing during the 30-60 second startup.
56
+ index, error = await asyncio.to_thread(load_index)
57
  elapsed = round(time.time() - t0, 2)
58
 
59
  if error:
 
121
 
122
 
123
  # ---------------------------------------------------------------------------
124
+ # Manifest caching async, 5-min TTL, mutex to prevent stampede
125
  # ---------------------------------------------------------------------------
126
  _manifest_cache: dict = {"data": None, "fetched_at": 0}
127
  MANIFEST_TTL = 300 # seconds
128
+ _manifest_lock = asyncio.Lock() # API-BUG-1: one coroutine fetches at a time
129
 
130
 
131
+ async def _get_manifest() -> dict:
132
+ """
133
+ Fetch and in-memory cache manifest.json from GitHub.
134
+
135
+ asyncio.Lock (API-BUG-1): when the cache expires, only ONE coroutine
136
+ actually calls GitHub; all other concurrent waiters block on the lock and
137
+ then immediately return the freshly-written result — no stampede.
138
 
139
+ httpx.AsyncClient (API-BUG-2): non-blocking HTTP so the event loop stays
140
+ responsive while the GitHub call is in flight.
141
+ """
142
+ async with _manifest_lock:
143
+ now = time.time()
144
+ if _manifest_cache["data"] and (now - _manifest_cache["fetched_at"]) < MANIFEST_TTL:
145
+ return _manifest_cache["data"]
146
 
147
+ url = (
148
+ f"https://raw.githubusercontent.com/{MATERIALS_REPO}"
149
+ f"/{MANIFEST_BRANCH}/manifest.json"
150
+ )
151
+ async with httpx.AsyncClient(timeout=15.0) as client:
152
+ resp = await client.get(url)
153
+ resp.raise_for_status()
154
+
155
+ data = resp.json()
156
+ _manifest_cache["data"] = data
157
+ _manifest_cache["fetched_at"] = now
158
+ return data
159
 
160
 
161
  # ---------------------------------------------------------------------------
 
175
 
176
 
177
  @app.get("/manifest")
178
+ async def get_manifest():
179
  """
180
  Proxy and cache the study materials manifest.json from GitHub.
181
  The Cloudflare Worker also caches this in KV — this is a double layer.
182
  """
183
  try:
184
+ data = await _get_manifest()
185
  return JSONResponse(content=data)
186
+ except httpx.HTTPStatusError as err:
187
  raise HTTPException(status_code=502, detail=f"GitHub fetch failed: {err}")
188
  except Exception as err:
189
  raise HTTPException(status_code=500, detail=str(err))
190
 
191
 
192
  @app.post("/retrieve", response_model=RetrieveResponse)
193
+ async def retrieve(body: RetrieveRequest):
194
  """
195
  Core RAG endpoint.
196
 
 
198
  2. Searches the pre-built LlamaIndex vector store
199
  3. Filters results by semester + subject metadata
200
  4. Returns top-k chunks + a formatted context string for the LLM prompt
201
+
202
+ CPU-bound embedding + vector search runs in a thread pool via
203
+ asyncio.to_thread — the event loop is never blocked (API-BUG-3).
204
  """
205
  index = _state.get("index")
206
 
207
+ # API-BUG-3: retrieve_chunks is synchronous (HuggingFace embedding model
208
+ # + LlamaIndex vector search). Run it in a thread so FastAPI can handle
209
+ # other requests concurrently while this one is working.
210
+ chunks = await asyncio.to_thread(
211
+ retrieve_chunks,
212
  index=index,
213
  query=body.query,
214
  semester=body.semester,
rag.py CHANGED
@@ -11,8 +11,10 @@ Handles everything related to the LlamaIndex vector index:
11
 
12
  import io
13
  import os
 
14
  import tempfile
15
  from pathlib import Path
 
16
 
17
  import requests
18
 
@@ -27,6 +29,20 @@ try:
27
  except ImportError:
28
  LLAMA_INDEX_AVAILABLE = False
29
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
30
  try:
31
  import PyPDF2
32
 
@@ -53,7 +69,17 @@ DEFAULT_TOP_K = 5
53
 
54
 
55
  # ---------------------------------------------------------------------------
56
- # Index loading (called once at FastAPI startup)
 
 
 
 
 
 
 
 
 
 
57
  # ---------------------------------------------------------------------------
58
 
59
  def load_index():
@@ -62,6 +88,10 @@ def load_index():
62
  VectorStoreIndex ready for querying.
63
 
64
  Returns (index, error_msg). index is None if loading failed.
 
 
 
 
65
  """
66
  if not LLAMA_INDEX_AVAILABLE:
67
  return None, "llama-index-core is not installed."
@@ -71,33 +101,33 @@ def load_index():
71
  )
72
  index_dir = tempfile.mkdtemp(prefix="plexi_index_")
73
 
74
- for filename in INDEX_FILES:
75
- url = f"{index_base_url}/{filename}"
76
- try:
77
- resp = requests.get(url, timeout=30)
78
- resp.raise_for_status()
79
- with open(os.path.join(index_dir, filename), "wb") as fh:
80
- fh.write(resp.content)
81
- except Exception as err:
82
- return None, f"Failed to download index file '{filename}': {err}"
83
-
84
  try:
85
- embed_model = HuggingFaceEmbedding(model_name=EMBED_MODEL_ID)
86
- Settings.embed_model = embed_model
87
- Settings.llm = None
88
-
89
- storage_ctx = StorageContext.from_defaults(persist_dir=index_dir)
90
- index = load_index_from_storage(storage_ctx)
91
- return index, None
92
- except Exception as err:
93
- return None, f"Failed to load index from storage: {err}"
 
 
 
 
 
 
 
94
 
 
 
 
 
 
95
 
96
- def load_embed_model():
97
- """Load and return the HuggingFace embedding model (for health checks)."""
98
- if not LLAMA_INDEX_AVAILABLE:
99
- return None
100
- return HuggingFaceEmbedding(model_name=EMBED_MODEL_ID)
101
 
102
 
103
  # ---------------------------------------------------------------------------
@@ -119,32 +149,49 @@ def retrieve_chunks(
119
  semester: str,
120
  subject: str,
121
  top_k: int = DEFAULT_TOP_K,
122
- ) -> list[dict]:
123
  """
124
  Embed the query, retrieve top-k chunks from the index scoped to the
125
  given semester + subject.
126
 
127
- Returns a list of dicts:
128
- { text, score, filename, subject }
 
 
 
 
 
 
 
129
  """
130
  if index is None:
131
  return []
132
 
133
  try:
134
- # Fetch more than needed so we have room to filter by scope
135
- retriever = index.as_retriever(similarity_top_k=max(top_k * 5, 10))
136
- nodes = retriever.retrieve(query)
137
-
138
- scoped = [n for n in nodes if _matches_scope(n, semester, subject)]
 
 
 
 
 
 
 
 
 
 
139
 
140
  return [
141
- {
142
- "text": node.node.get_content(),
143
- "score": round(float(node.score), 4) if node.score is not None else None,
144
- "filename": (getattr(node.node, "metadata", {}) or {}).get("filename"),
145
- "subject": (getattr(node.node, "metadata", {}) or {}).get("subject"),
146
- }
147
- for node in scoped[:top_k]
148
  ]
149
  except Exception as err:
150
  print(f"Retrieval error: {err}")
@@ -155,7 +202,7 @@ def retrieve_chunks(
155
  # Context formatting (for system prompt injection)
156
  # ---------------------------------------------------------------------------
157
 
158
- def format_context(chunks: list[dict]) -> str:
159
  """Format retrieved chunks as a numbered block for the LLM system prompt."""
160
  if not chunks:
161
  return "(No relevant context retrieved for this query.)"
@@ -170,7 +217,9 @@ def format_context(chunks: list[dict]) -> str:
170
 
171
 
172
  # ---------------------------------------------------------------------------
173
- # PDF text extraction (used for full-context fallback loading)
 
 
174
  # ---------------------------------------------------------------------------
175
 
176
  def read_pdf_text(pdf_bytes: bytes) -> str:
 
11
 
12
  import io
13
  import os
14
+ import shutil # API-BUG-7: temp-dir cleanup after index is in memory
15
  import tempfile
16
  from pathlib import Path
17
+ from typing import TypedDict # API-BUG-4: explicit typed return from retrieve_chunks
18
 
19
  import requests
20
 
 
29
  except ImportError:
30
  LLAMA_INDEX_AVAILABLE = False
31
 
32
+ # API-BUG-5: MetadataFilters let the vector store do scope-filtering internally,
33
+ # avoiding the over-fetch window that could miss relevant chunks.
34
+ try:
35
+ from llama_index.core.vector_stores.types import MetadataFilter, MetadataFilters
36
+
37
+ METADATA_FILTERS_AVAILABLE = True
38
+ except ImportError:
39
+ try:
40
+ from llama_index.core.vector_stores import MetadataFilter, MetadataFilters
41
+
42
+ METADATA_FILTERS_AVAILABLE = True
43
+ except ImportError:
44
+ METADATA_FILTERS_AVAILABLE = False
45
+
46
  try:
47
  import PyPDF2
48
 
 
69
 
70
 
71
  # ---------------------------------------------------------------------------
72
+ # Typed chunk return (API-BUG-4)
73
+ # ---------------------------------------------------------------------------
74
+ class ChunkDict(TypedDict):
75
+ text: str
76
+ score: float | None
77
+ filename: str | None
78
+ subject: str | None
79
+
80
+
81
+ # ---------------------------------------------------------------------------
82
+ # Index loading (called once at FastAPI startup via asyncio.to_thread)
83
  # ---------------------------------------------------------------------------
84
 
85
  def load_index():
 
88
  VectorStoreIndex ready for querying.
89
 
90
  Returns (index, error_msg). index is None if loading failed.
91
+
92
+ API-BUG-7: The temp directory is always removed in the finally block.
93
+ After load_index_from_storage() returns, all data is in memory — the
94
+ files on disk are no longer needed.
95
  """
96
  if not LLAMA_INDEX_AVAILABLE:
97
  return None, "llama-index-core is not installed."
 
101
  )
102
  index_dir = tempfile.mkdtemp(prefix="plexi_index_")
103
 
 
 
 
 
 
 
 
 
 
 
104
  try:
105
+ # Download each index shard from GitHub
106
+ for filename in INDEX_FILES:
107
+ url = f"{index_base_url}/{filename}"
108
+ try:
109
+ resp = requests.get(url, timeout=30)
110
+ resp.raise_for_status()
111
+ with open(os.path.join(index_dir, filename), "wb") as fh:
112
+ fh.write(resp.content)
113
+ except Exception as err:
114
+ return None, f"Failed to download index file '{filename}': {err}"
115
+
116
+ # Build the in-memory index from the downloaded shards
117
+ try:
118
+ embed_model = HuggingFaceEmbedding(model_name=EMBED_MODEL_ID)
119
+ Settings.embed_model = embed_model
120
+ Settings.llm = None
121
 
122
+ storage_ctx = StorageContext.from_defaults(persist_dir=index_dir)
123
+ index = load_index_from_storage(storage_ctx)
124
+ return index, None
125
+ except Exception as err:
126
+ return None, f"Failed to load index from storage: {err}"
127
 
128
+ finally:
129
+ # API-BUG-7: always wipe the temp dir data is now in RAM.
130
+ shutil.rmtree(index_dir, ignore_errors=True)
 
 
131
 
132
 
133
  # ---------------------------------------------------------------------------
 
149
  semester: str,
150
  subject: str,
151
  top_k: int = DEFAULT_TOP_K,
152
+ ) -> list[ChunkDict]:
153
  """
154
  Embed the query, retrieve top-k chunks from the index scoped to the
155
  given semester + subject.
156
 
157
+ API-BUG-4: Returns list[ChunkDict] (TypedDict) instead of list[dict] so
158
+ type errors surface at the source rather than silently at Pydantic
159
+ serialization time.
160
+
161
+ API-BUG-5: Primary path uses MetadataFilters so the vector store does the
162
+ scope-gating internally — no risk of the over-fetch window failing to reach
163
+ chunks that belong to the active subject. Falls back to the generous
164
+ over-fetch + manual filter approach when MetadataFilters are unavailable
165
+ (e.g., older llama-index-core builds).
166
  """
167
  if index is None:
168
  return []
169
 
170
  try:
171
+ if METADATA_FILTERS_AVAILABLE:
172
+ # Primary: vector store filters by metadata at query time.
173
+ filters = MetadataFilters(
174
+ filters=[
175
+ MetadataFilter(key="semester", value=semester),
176
+ MetadataFilter(key="subject", value=subject),
177
+ ]
178
+ )
179
+ retriever = index.as_retriever(similarity_top_k=top_k, filters=filters)
180
+ nodes = retriever.retrieve(query)
181
+ else:
182
+ # Fallback: over-fetch (generous 10× window) + manual scope filter.
183
+ retriever = index.as_retriever(similarity_top_k=max(top_k * 10, 50))
184
+ nodes = retriever.retrieve(query)
185
+ nodes = [n for n in nodes if _matches_scope(n, semester, subject)]
186
 
187
  return [
188
+ ChunkDict(
189
+ text=node.node.get_content(),
190
+ score=round(float(node.score), 4) if node.score is not None else None,
191
+ filename=(getattr(node.node, "metadata", {}) or {}).get("filename"),
192
+ subject=(getattr(node.node, "metadata", {}) or {}).get("subject"),
193
+ )
194
+ for node in nodes[:top_k]
195
  ]
196
  except Exception as err:
197
  print(f"Retrieval error: {err}")
 
202
  # Context formatting (for system prompt injection)
203
  # ---------------------------------------------------------------------------
204
 
205
+ def format_context(chunks: list[ChunkDict]) -> str:
206
  """Format retrieved chunks as a numbered block for the LLM system prompt."""
207
  if not chunks:
208
  return "(No relevant context retrieved for this query.)"
 
217
 
218
 
219
  # ---------------------------------------------------------------------------
220
+ # PDF text extraction
221
+ # NOTE: read_pdf_text() is currently unused by the API routes — retained for
222
+ # future use (CQ-4). load_embed_model() removed — was dead code (API-BUG-6).
223
  # ---------------------------------------------------------------------------
224
 
225
  def read_pdf_text(pdf_bytes: bytes) -> str:
requirements.txt CHANGED
@@ -2,6 +2,7 @@ fastapi>=0.115.0,<1.0.0
2
  uvicorn[standard]>=0.30.0,<1.0.0
3
  pydantic>=2.0.0,<3.0.0
4
  requests>=2.31.0,<3.0.0
 
5
  python-dotenv>=1.0.0
6
  PyPDF2>=3.0.0,<4.0.0
7
  llama-index-core>=0.11.0,<0.13.0
 
2
  uvicorn[standard]>=0.30.0,<1.0.0
3
  pydantic>=2.0.0,<3.0.0
4
  requests>=2.31.0,<3.0.0
5
+ httpx>=0.27.0,<1.0.0
6
  python-dotenv>=1.0.0
7
  PyPDF2>=3.0.0,<4.0.0
8
  llama-index-core>=0.11.0,<0.13.0