perf: extract transcript chunks with bounded concurrency

[groupthinking]

Canonical issue

Closes #1337

Scope

One production file and one new test file.

• src/youtube_extension/backend/api/v1/router.py — the extract_events handler only.
• tests/unit/test_events_extract_concurrency.py — new.

No other handler, no signature change, no new dependency, no config surface.

Outcome

POST /api/v1/events/extract chunks long transcripts into overlapping 24 000-character
windows and extracted them strictly one at a time:

for chunk in transcript_chunks:
    if len(events) >= _MAX_EVENTS:
        break
    for ev in await _extract_chunk(chunk):
        ...

Every _extract_chunk call is an independent, billed Gemini round-trip, so wall-clock
latency was the sum of all chunk latencies. Chunks are now walked in bounded windows
of _EXTRACT_CONCURRENCY = 4 via asyncio.gather.

Chunk count scales with transcript length (stride _CHUNK_SIZE - _CHUNK_OVERLAP = 23 500):

Transcript	Chunks	Before (~3 s/chunk)	After
30 min talk	2	~6 s	~3 s
90 min podcast	~6	~18 s	~6 s
4 h stream	~22	~66 s	~18 s

Single-chunk transcripts — the common case — are completely unaffected: one window, one
call, identical behaviour.

Risk

Correctness is preserved exactly, and each guarantee has a named mechanism:

• Order. asyncio.gather returns results in input order, not completion order. Merging
  a window's results therefore reproduces the serial walk's dedup and _MAX_EVENTS cut-off
  for every chunk actually processed. test_events_are_merged_in_chunk_order proves this by
  giving later chunks shorter latency, so a completion-ordered merge would visibly reverse
  the output.
• Failure isolation. _extract_chunk ends in except Exception as exc: logger.warning(...)
  and returns chunk_events, so it never raises. The default return_exceptions=False
  consequently has no path to abort a sibling chunk. Covered by
  test_one_failing_chunk_does_not_abort_siblings.
• Budget. len(events) >= _MAX_EVENTS is re-checked at the top of every window.

Disclosed tradeoff — up to 3 extra billed Gemini calls. The serial loop stopped issuing
calls the instant the 50-event budget filled. A window of 4 dispatches all 4 before the
budget can be re-checked, so extraction can issue up to _EXTRACT_CONCURRENCY - 1 = 3
more billed calls than before on transcripts that fill the budget early. This is deliberate:
the window is kept small precisely to bound that overshoot, and the budget is re-checked
between windows so it cannot compound. test_event_budget_is_rechecked_between_windows
asserts the bound explicitly (≤ 4 calls, not the full 12-chunk walk).

Peak in-flight billed calls rises from 1 to at most 4 per request, which is why the fan-out
is windowed rather than a single unbounded gather over every chunk.

Verification

.venv/bin/python -m pytest tests/unit/test_events_extract_concurrency.py \
  tests/unit/test_events_extract_ai_path.py tests/unit/test_v1_router_extended.py \
  tests/unit/test_backend_worker.py --override-ini="addopts=" -p no:cacheprovider -q

• New tests: 6 passed.

• Pre-existing extract_events coverage: 146 passed, unchanged.

• Prove-fail — new tests run against the pre-change source via
  git stash push -- src/youtube_extension/backend/api/v1/router.py:
  3 failed, 3 passed.

  The 3 failures are exactly the concurrency assertions the serial loop cannot satisfy:
  test_chunks_are_extracted_concurrently, test_concurrency_is_bounded,
  test_events_are_merged_in_chunk_order.

  The 3 that pass are deliberate regression guards, not weak proof — they assert
  behaviour the serial loop already had (budget respected, failures isolated, single-chunk
  path unchanged) and exist to prove this PR does not break it.

• ruff check on router.py reports 25 findings, identical to origin/main — none
  introduced, none in the changed range. ruff check on the new test file: clean.

Production evidence

src/youtube_extension/backend/api/v1/router.py is on the live HTTP surface:
backend/main.py:35 imports v1_router and main.py:164 calls
app.include_router(v1_router). The tests drive the real route through
TestClient(app) against the real FastAPI application — not the handler function in
isolation — so the mounted path, request validation, and response envelope are all
exercised.

Concurrency is measured, not assumed: the stand-in HybridProcessorService.process
increments an in-flight counter, records the peak, sleeps, and decrements in finally.
A serial loop can never record a peak above 1, so max_inflight > 1 is unambiguous.

REAL_MODE_ONLY: the stand-in reports backend="gemini" solely so the router's own
mock-rejection guard does not divert to the heuristic path. No assertion here concerns AI
output quality — every assertion is about the router's scheduling.

Agent handoff

Next in this perf series: batching per-metric SQLite writes in
ingest_performance_report_v1 (PerformanceMonitor.record_metric opens, writes, commits
and closes one connection per metric, so one web-vitals report costs N fsyncs).

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
v0-uvai	Ready	Preview, v0	Aug 4, 2026 4:04am

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)

• [‘architecture-gap’, ‘bug’, ‘ci-cd’, ‘ci/cd’, ‘copilot-rabbit’, ‘documentation’, ‘duplicate’, ‘enhancement’, ‘frontend’, ‘github_actions’, ‘good first issue’, ‘help wanted’, ‘high-priority’, ‘invalid’, ‘javascript’, ‘ml-model’, ‘needs-triage’, ‘pipeline-critical’, ‘placeholder-code’, ‘priority:high’, ‘python’, ‘python:uv’, ‘question’, ‘styling’, ‘tests’, ‘v0’]

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: df37a9b9-4662-42cb-a349-a1e993c13257

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA e47a5a2.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

[groupthinking]

@linear-code @coderabbitai review

Three specific things I'd like challenged rather than a general pass:

1. Is the ≤3 extra billed Gemini call overshoot acceptable? The serial loop stopped
   issuing the instant the 50-event budget filled; a window of 4 can dispatch up to 3 past
   that point. It's disclosed in ## Risk and bounded by re-checking the budget between
   windows, but this is a billed provider path. If you think that's the wrong trade,
   _EXTRACT_CONCURRENCY = 2 halves the worst case at the cost of half the speedup.

2. Confirm gather order-preservation makes selection identical to serial. My claim is
   that because asyncio.gather returns in input order, merging window results reproduces
   the serial walk's dedup + _MAX_EVENTS cut-off exactly. Please check I haven't missed a
   path where a chunk that would have been skipped by the serial early-break now
   contributes an event.

3. Confirm return_exceptions=False is safe here. I'm relying on _extract_chunk's
   terminal except Exception making it non-raising. If there's a way for it to propagate
   (e.g. BaseException/CancelledError), the fan-out would abort siblings and I should
   switch to return_exceptions=True.

Note on the prove-fail split: 3 failed / 3 passed against pre-change source is expected.
The 3 passers are regression guards over behaviour the serial loop already had, not weak
assertions — detail in ## Verification.

[github-actions]

Agent Completion Truth Gate: NOT_APPLICABLE

Evidence agrees.

Machine-readable verdict

{
  "details": {},
  "reasons": [],
  "verdict": "not_applicable"
}

Workflow evidence

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- PR diff (target files) ---'
git diff --find-renames origin/main...HEAD -- \
  src/youtube_extension/backend/api/v1/router.py \
  tests/unit/test_events_extract_concurrency.py

printf '%s\n' '--- router outline ---'
ast-grep outline src/youtube_extension/backend/api/v1/router.py \
  --items all --type function --match 'extract_events|_extract_chunk' || true

printf '%s\n' '--- handler and helper references ---'
rg -n -A100 -B15 \
  'async def extract_events\b|async def _extract_chunk\b|_EXTRACT_CONCURRENCY|asyncio\.gather|CancelledError' \
  src/youtube_extension/backend/api/v1/router.py

printf '%s\n' '--- new test outline and contents ---'
ast-grep outline tests/unit/test_events_extract_concurrency.py --items all || true
cat -n tests/unit/test_events_extract_concurrency.py

printf '%s\n' '--- project Python support metadata ---'
rg -n -C2 'python_requires|requires-python|target-version|python_version' \
  pyproject.toml setup.cfg setup.py tox.ini 2>/dev/null || true

Length of output: 23690

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- checkout status and changed paths ---'
git status --short
git diff --name-status origin/main...HEAD
git ls-tree -r --name-only HEAD | rg '(^|/)test_events_extract_concurrency\.py$' || true

printf '%s\n' '--- router imports ---'
sed -n '1,150p' src/youtube_extension/backend/api/v1/router.py | nl -ba

printf '%s\n' '--- Python and tool configuration files ---'
fd -HI -t f '^(pyproject\.toml|setup\.cfg|setup\.py|tox\.ini|\.python-version|Pipfile|poetry\.lock)$' . \
  -E .git -E node_modules -E dist -E build \
  | while IFS= read -r file; do
      echo "--- $file ---"
      rg -n -C2 'requires-python|python_requires|python_version|target-version|python' "$file" || true
    done

printf '%s\n' '--- cancellation-specific handlers or task creation in extraction dependencies ---'
rg -n -C3 \
  'CancelledError|create_task\s*\(|asyncio\.shield\s*\(|asyncio\.wait_for\s*\(' \
  src/youtube_extension/services src/youtube_extension/backend 2>/dev/null || true

Length of output: 1565

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- router imports from HEAD ---'
git show HEAD:src/youtube_extension/backend/api/v1/router.py | sed -n '1,150p'

printf '%s\n' '--- committed concurrency test key assertions ---'
git show HEAD:tests/unit/test_events_extract_concurrency.py \
  | grep -n -E -A8 -B4 \
    'test_(chunks_are_extracted_concurrently|concurrency_is_bounded|events_are_merged_in_chunk_order|event_budget_is_rechecked_between_windows|one_failing_chunk_does_not_abort_siblings)|CancelledError|gather'

printf '%s\n' '--- Python metadata ---'
find . -path ./.git -prune -o \
  \( -name pyproject.toml -o -name setup.cfg -o -name setup.py -o -name tox.ini -o -name .python-version -o -name Pipfile \) \
  -type f -print \
  | while IFS= read -r file; do
      echo "--- $file ---"
      grep -n -E -C2 'requires-python|python_requires|python_version|target-version|python' "$file" || true
    done

printf '%s\n' '--- cancellation handling in source ---'
grep -R -n -E -C3 \
  'CancelledError|create_task[[:space:]]*\(|asyncio\.shield[[:space:]]*\(|asyncio\.wait_for[[:space:]]*\(' \
  src/youtube_extension/services src/youtube_extension/backend 2>/dev/null || true

Length of output: 56419

---

@groupthinking Targeted review complete. I found no blocking correctness defect in the windowing logic.

1. Billed-call overshoot

_EXTRACT_CONCURRENCY = 4 gives the stated bound: one window can issue at most three calls that the serial loop would not issue after it reaches _MAX_EVENTS. The check before each window prevents additional overshoot in later windows.

I consider this acceptable only if the product accepts that bounded direct-provider cost increase. The bound is three calls, not a fixed currency amount. A 24,000-character chunk can have a non-trivial per-call cost. The PR documents this trade-off clearly. I would keep 4 for the stated latency target.

2. Selection and merge order

The claim is correct, with one precise condition: it holds for the same per-chunk results.

asyncio.gather returns results in the order of its input awaitables. The code passes chunks in transcript order and merges window_results in that same order. Therefore, title deduplication and the 50-event cutoff produce the same returned event list as the serial walk.

A chunk that the serial loop would skip after the budget fills can run in the same concurrent window. It cannot contribute an event because the merge condition still requires len(events) < _MAX_EVENTS. This changes billed calls only. It does not change selected events.

3. return_exceptions=False and cancellation

return_exceptions=False is safe for normal provider failures. _extract_chunk catches every ordinary Exception from HybridProcessorService.process, parsing, and event construction, then returns an empty list. The test covers that path.

It is not correct to say that _extract_chunk “never raises.” The project supports Python 3.10+, where asyncio.CancelledError inherits from BaseException. A cancellation can propagate from _extract_chunk. KeyboardInterrupt and SystemExit can also propagate.

This behavior is appropriate for request cancellation. Do not replace the call with return_exceptions=True without explicit result handling. That change would convert a child CancelledError into a result value and could accidentally suppress cancellation.

I recommend changing the adjacent comment to say that _extract_chunk isolates ordinary Exception failures. Add a cancellation test if request-cancellation behavior is part of the endpoint contract. The implementation itself should continue to allow CancelledError to propagate.

I verified the changed source and committed tests statically. I could not run the test command in this inspection sandbox.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Copilot]

Pull request overview

Adds bounded concurrency to reduce long-transcript event extraction latency while preserving ordered merging and event limits.

Changes:

• Processes transcript chunks concurrently in windows of four.
• Adds regression tests for concurrency, ordering, limits, failures, and short transcripts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/youtube_extension/backend/api/v1/router.py	Adds bounded concurrent chunk extraction.
tests/unit/test_events_extract_concurrency.py	Tests scheduling and preserved behavior.

[groupthinking]

Independent review — no blocking defects

I read the windowed loop and the full _extract_chunk body at 03d7ebe and verified your three challenges against the source directly. All three hold. One non-blocking comment-accuracy nit.

1. ≤3 billed-call overshoot — acceptable and correctly bounded.
The budget is re-checked at the top of every window (if len(events) >= _MAX_EVENTS: break), so overshoot cannot compound across windows; it is confined to a single window, worst case _EXTRACT_CONCURRENCY - 1 = 3 extra calls when the budget fills on the first chunk of a window. The bound is on calls, not cost, and a 24k-char chunk is a non-trivial Gemini round-trip — but that's a disclosed product tradeoff, not a correctness issue, and _EXTRACT_CONCURRENCY = 4 is the right knob if the worst case ever needs halving. Keep 4 for the stated latency target.

2. gather order-preservation ⇒ identical selection — confirmed.
asyncio.gather returns in input order; chunks are passed in transcript order and window_results is merged in that same order, with the merge guard ev.title not in seen_titles and len(events) < _MAX_EVENTS unchanged. I specifically checked the budget-fills-mid-window case you flagged: a chunk the serial walk would have skipped after the budget filled can now execute in the same window, but it contributes zero events because the len(events) < _MAX_EVENTS guard still gates every append. So the selected event set is identical to the serial walk — only billed-call count differs. No path where a would-be-skipped chunk leaks an event.

3. return_exceptions=False is safe — but tighten the adjacent comment.
Verified _extract_chunk ends in except Exception as exc: logger.warning(...); return chunk_events, so every ordinary provider/parsing/construction failure is swallowed and the default return_exceptions=False has no ordinary-exception path to abort a sibling. Do not switch to return_exceptions=True — that would turn a child CancelledError into a result value and swallow request cancellation, which you correctly want to propagate (CancelledError is BaseException on 3.10+).

The one imprecision is the inline comment, which slightly overstates on two counts:

#   _extract_chunk never raises (it catches Exception and returns []), so the
#   default return_exceptions=False cannot abort a sibling chunk.

• "never raises" → true only for Exception; CancelledError/KeyboardInterrupt/SystemExit (BaseException) still propagate, and that's the intended behavior.
• "returns []" → it returns chunk_events, which on a mid-parse failure holds the events accumulated before the exception, not necessarily an empty list.

Suggested wording:

#   _extract_chunk isolates ordinary Exception failures (logs and returns
#   whatever it parsed so far), so the default return_exceptions=False cannot
#   abort a sibling chunk on a provider error. A BaseException such as
#   CancelledError still propagates, which correctly cancels the request.

Non-blocking — behavior is already correct; this only makes the invariant the comment claims match what the code guarantees. If request-cancellation is part of the endpoint contract, a cancellation test would lock that in, but that's optional and out of scope for this PR.

Tests: the 6 new tests exercise the real mounted route through TestClient(app) with a concurrency-aware stand-in that records peak in-flight — max_inflight > 1 is genuinely unreachable under a serial loop, so test_chunks_are_extracted_concurrently / test_concurrency_is_bounded are real proofs, and invert_latency makes test_events_are_merged_in_chunk_order fail loudly under a completion-ordered merge. The 3-fail/3-pass prove-fail split is exactly what a correct change should produce.

Verdict: correct, well-scoped, well-tested. Ready to merge once un-drafted for merge sign-off — I'm not merging automatically, since main is protected and this is an unattended run with no live human approval. Nothing here should block that sign-off.

---

Generated by Claude Code

[groupthinking]

Independent remediation pass — red-team of the diff on top of CodeRabbit's review. All three of your challenge points hold; I pushed one comment-only fix (e47a5a2) and am holding the merge for you.

1. ≤3-call overshoot — acceptable, and the bound is exactly as stated. A window can dispatch at most _EXTRACT_CONCURRENCY - 1 = 3 calls past the point the serial loop would have stopped, and the len(events) >= _MAX_EVENTS check at the top of each window stops any later window from adding to that. The overshoot is one window deep and cannot compound. Keeping 4 is the right latency/cost trade; 2 stays the lever if the billed-call ceiling ever outweighs latency.

2. gather order-preservation ⇒ identical selection — confirmed. asyncio.gather returns in input (chunk) order; window_results is merged in that order and the inner guard is still len(events) < _MAX_EVENTS. A chunk the serial loop would have skipped after the budget filled can now run, but it can never contribute an event — so the returned event list is identical to the serial walk; only the billed-call count differs. I found no path where a would-be-skipped chunk leaks an event.

3. return_exceptions=False is correct — but the comment was wrong; fixed. The claim "_extract_chunk never raises" was inaccurate: it isolates ordinary Exception, but CancelledError (a BaseException) can still propagate — and that propagation is intended so request cancellation tears the whole fan-out down. Switching to return_exceptions=True would be a regression: it would convert a child CancelledError into a result value and swallow cancellation. So I kept the code unchanged and corrected the adjacent comment to say it isolates ordinary Exception failures while BaseException/CancelledError propagates by design.

Optional, non-blocking follow-up: a test asserting a CancelledError from one chunk propagates out of POST /events/extract would lock point 3 into the endpoint contract. Say the word and I'll add it.

Status: truth-gate green, CI green, no conflicts, review addressed. This sits at the PUBLISH GATE — merging to protected main needs your sign-off (no automerge label), so I'm not auto-merging. Staged for you:

gh pr merge 1338 --squash

---

Generated by Claude Code

[groupthinking]

⚠️ CI note — the test check failure is pre-existing on main, not introduced by this PR.

FAILED tests/unit/test_gh_aw_workflow_governance.py::test_ci_investigator_requires_dedicated_codex_credential
  FileNotFoundError: .github/workflows/eventrelay-ci-investigator.md
= 1 failed, 7961 passed, 6 deselected, 5 xpassed in 217.24s =

Evidence it isn't from #1338:

• The referenced file .github/workflows/eventrelay-ci-investigator.md is absent on both this branch and origin/main — I verified with git cat-file -e.
• The failing test tests/unit/test_gh_aw_workflow_governance.py exists on origin/main unchanged; it predates this PR.
• This PR's diff touches only src/youtube_extension/backend/api/v1/router.py and tests/unit/test_events_extract_concurrency.py — neither the failing test nor the referenced workflow file. Every other check on the fix commit is green (truth-gate, build, lint-python, guards, all security scans), the 6 new concurrency tests pass, and the failure would reproduce identically on main.

This is a separate main-branch governance breakage — a governance test expecting a workflow file that doesn't exist in the repo — not a regression here. I'm not resolving it inside this perf PR: it's out of scope, and because it concerns an agentic CI-investigator workflow gated on a dedicated Codex credential, whether to add the workflow file or adjust the test is your call.

If test is a required check, branch protection will block the merge until that base-branch failure is cleared, even though #1338's own gates are all green. Happy to open a separate PR for the governance test once you decide the intended fix.

---

Generated by Claude Code

[groupthinking]

CI status correction — the earlier "CI green" note above was premature. The test check finished after that comment and is red, on a single failure:

FAILED tests/unit/test_gh_aw_workflow_governance.py::test_ci_investigator_requires_dedicated_codex_credential
  FileNotFoundError: .github/workflows/eventrelay-ci-investigator.md
= 1 failed, 7961 passed

This is not this PR's diff. The failure is a base-branch issue: test_gh_aw_workflow_governance.py still hard-requires .github/workflows/eventrelay-ci-investigator.md (and its .lock.yml), but that workflow was removed from main. This PR only touches router.py + a new concurrency test; its own 6 new tests and the 146 pre-existing extract_events tests all pass. The same failure reproduces on #1336 and will red the test check on essentially every PR based on current main.

The fix is owned elsewhere — the investigator-removal cleanup in draft PRs #1317 / #1320 (and the gh-aw-validation reference at test_gh_aw_workflow_governance.py:205). Landing one of those clears this check across the queue.

Publish gate unchanged: this still awaits your merge sign-off (no automerge label, protected main) — but test should be green first, so the staged gh pr merge 1338 --squash should wait until the base-branch cleanup lands or the check is confirmed non-required. Not auto-merging; no code pushed.

---

Generated by Claude Code

[linear-code]

GRV-302