Problem
GET /api/v1/learning-log is declared async but performs an unbounded,
uncached filesystem walk directly on the event loop.
src/youtube_extension/backend/api/v1/router.py:1006
async def get_learning_log_v1(data_service: DataService = Depends(get_data_service)):
"""Get learning log from enhanced analysis files"""
try:
learning_log = data_service.get_learning_log() # <- blocking, not awaited
return learning_log
DataService.get_learning_log() (services/data_service.py:54-134) then does,
synchronously:
self.enhanced_analysis_dir.rglob("*_enhanced.md") — a fresh recursive walk
of the whole tree (L71).
- For every file found:
parent_dir.glob(f"{video_id}_*_metadata.json") (L79)
metadata_file.exists() (L83)
open(...) + json.load(...) (L85-86)
md_file.stat() (L93)
- An in-memory sort of the full result set (L127).
Because FastAPI runs async def handlers on the event loop itself, every one of
those syscalls holds the loop. No other request on the worker — including
health checks and unrelated endpoints — can make progress while the walk runs.
Why this one is worse than the sibling defects
Two properties make this the most severe instance of this pattern in the router:
- It is completely uncached.
DataService already has
_get_all_files_cached() (L136-160) with a 60s TTL, and the videos path uses
it. get_learning_log() ignores it and issues its own rglob on every
single request.
- It is unbounded. There is no
limit/offset. The cost scales linearly
with the total number of processed videos forever, whereas the videos endpoint
is at least capped at a page.
So the blocking cost is roughly 1 tree walk + 4 syscalls x N files + N JSON parses, with N growing without limit.
Why the existing tests do not catch it
tests/unit/test_v1_router_extended.py:191 installs
svc.get_learning_log.return_value = [...] on a MagicMock. The mock returns
instantly, so the handler never blocks under test and the suite stays green while
production stalls. tests/unit/test_data_service.py exercises the real method
but only synchronously — nothing asserts anything about the event loop.
This is the same blind spot that #1379 had, and it needs the same style of
regression test: one that asserts the work is dispatched to a worker thread, and
that the loop stays responsive while it is in flight.
Proposed fix
Offload the call with asyncio.to_thread, exactly as #1379 did for
/api/v1/videos:
learning_log = await asyncio.to_thread(data_service.get_learning_log)
asyncio is already imported in the router. The change is a single line plus
regression tests.
Deliberately out of scope (each is a separate, larger change):
- Routing
get_learning_log() through _get_all_files_cached().
- Adding pagination to the endpoint.
- Caching the parsed metadata.
Those are real follow-ups, but this issue is only about getting the blocking work
off the event loop, which is the part that degrades every other request.
Acceptance criteria
Problem
GET /api/v1/learning-logis declaredasyncbut performs an unbounded,uncached filesystem walk directly on the event loop.
src/youtube_extension/backend/api/v1/router.py:1006DataService.get_learning_log()(services/data_service.py:54-134) then does,synchronously:
self.enhanced_analysis_dir.rglob("*_enhanced.md")— a fresh recursive walkof the whole tree (L71).
parent_dir.glob(f"{video_id}_*_metadata.json")(L79)metadata_file.exists()(L83)open(...)+json.load(...)(L85-86)md_file.stat()(L93)Because FastAPI runs
async defhandlers on the event loop itself, every one ofthose syscalls holds the loop. No other request on the worker — including
health checks and unrelated endpoints — can make progress while the walk runs.
Why this one is worse than the sibling defects
Two properties make this the most severe instance of this pattern in the router:
DataServicealready has_get_all_files_cached()(L136-160) with a 60s TTL, and the videos path usesit.
get_learning_log()ignores it and issues its ownrglobon everysingle request.
limit/offset. The cost scales linearlywith the total number of processed videos forever, whereas the videos endpoint
is at least capped at a page.
So the blocking cost is roughly
1 tree walk + 4 syscalls x N files + N JSON parses, withNgrowing without limit.Why the existing tests do not catch it
tests/unit/test_v1_router_extended.py:191installssvc.get_learning_log.return_value = [...]on aMagicMock. The mock returnsinstantly, so the handler never blocks under test and the suite stays green while
production stalls.
tests/unit/test_data_service.pyexercises the real methodbut only synchronously — nothing asserts anything about the event loop.
This is the same blind spot that #1379 had, and it needs the same style of
regression test: one that asserts the work is dispatched to a worker thread, and
that the loop stays responsive while it is in flight.
Proposed fix
Offload the call with
asyncio.to_thread, exactly as #1379 did for/api/v1/videos:asynciois already imported in the router. The change is a single line plusregression tests.
Deliberately out of scope (each is a separate, larger change):
get_learning_log()through_get_all_files_cached().Those are real follow-ups, but this issue is only about getting the blocking work
off the event loop, which is the part that degrades every other request.
Acceptance criteria
called inline rather than offloaded.