Skip to content

Pr/5 OpenAI compatible summarizers - #20

Closed
bryangruneberg wants to merge 3 commits into
jamespember:mainfrom
bryangruneberg:pr/5-openai-compatible-summarizers
Closed

bryangruneberg wants to merge 3 commits into
jamespember:mainfrom
bryangruneberg:pr/5-openai-compatible-summarizers

Conversation

@bryangruneberg

Copy link
Copy Markdown
Contributor

No description provided.

bryangruneberg and others added 3 commits September 16, 2026 13:31
A response cut off at its output-token limit was parsed as if complete.
With a reasoning model the whole budget can go on thinking, so the visible
text is empty and the note reads "No overview generated"; with a long
meeting the summary stops mid-sentence and loses its action items. Either
way the note looks finished and nothing reports a problem.

- Anthropic: max_tokens 2000 -> 16000 (billed only as used; stays under the
  SDK's non-streaming time guard).
- All providers: raise SummaryTruncated on stop_reason "max_tokens" /
  finish_reason "length". NoteMaker already turns summarizer exceptions
  into its ai_error path, so the user is told and the fallback summary is
  used. Truncation is not retried: the same request truncates the same way.

Found with Claude Sonnet 5 through an OpenAI-compatible endpoint: at 2000
tokens a 34-minute meeting came back with finish_reason=length and an empty
message; it needed ~5,400 output tokens for ~1,100 tokens of summary.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…tional extra

Adds a second transcription backend next to local Whisper, selected with
`transcriber: whisper | assemblyai` (default unchanged: whisper).

AssemblyAITranscriber has the same interface as WhisperTranscriber and adds:
- speaker diarisation: segments carry a speaker, the transcript renders as
  **[mm:ss] Speaker A:** ..., and the summariser is given "Speaker A: ..."
  lines (summary_input) so action items can name an owner;
- upload as 16 kHz mono FLAC via ffmpeg. The recorder's 48 kHz WAV is
  ~11 MB/min, and a 34-minute meeting (377 MB) had its upload cut off
  mid-TLS; FLAC is lossless for what the model hears and ~10x smaller;
- upload retries on a dropped connection;
- submit()/wait() split so a caller can keep the transcript id and resume
  polling instead of re-uploading; AssemblyAIError carries status_code /
  transient / remote_failed for callers deciding whether to retry;
- a missing API key is reported at transcription time, so the app still
  opens and records without one.

build_transcriber(config) replaces the two duplicated construction blocks in
app.py (startup and settings-saved). Settings gains a transcription picker.

Packaging: openai-whisper moves from core dependencies to a `whisper`
extra (torch is several GB and not always available for the newest Python).
`all` still includes it, so setup.sh's `pip install -e .[all]` installs
exactly what it did before; a new `assemblyai` extra is the cloud-only set.

CI installs requests and runs tests/test_assemblyai.py.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…way and DeepInfra

OpenAICompatibleSummarizer runs the shared prompt and parser against any
OpenAI chat-completions endpoint: a subclass sets BASE_URL, ENV_VAR, LABEL
and a tier -> model-id table. Two providers use it, both serving Claude with
ai_model haiku | sonnet | opus:

- assemblyai: AssemblyAI's LLM Gateway, authenticated with the same key as
  AssemblyAI transcription, so one key covers the whole pipeline.
- deepinfra: DeepInfra's OpenAI-compatible API.

Choices that matter for current models:
- no temperature: Claude 5 models on the gateway reject it;
- MAX_TOKENS = 16000: reasoning models spend output tokens before writing
  text (a 34-minute meeting needed ~5,400 for ~1,100 visible);
- finish_reason "length" raises SummaryTruncated rather than parsing a
  partial response.

The same base would also cover other OpenAI-compatible services, e.g. the
Copilot request in jamespember#2 / jamespember#10.

Config validation, provider_api_key(), Settings (shared Claude tier buttons,
key inputs) and NoteMaker are wired for both; tests use fake clients only.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@jamespember

Copy link
Copy Markdown
Owner

Continuing to be impressed with the design here @bryangruneberg. The OpenAICompatibleSummarizer base is exactly the right abstraction — three concrete providers now (OpenAI proper via the existing subclass pattern, AssemblyAI's LLM Gateway, and DeepInfra) all reduce to BASE_URL + ENV_VAR + LABEL + MODELS, and adding a fourth (Together, Groq, OpenRouter's own OpenAI-compatible endpoint, self-hosted vLLM…) is a ~15-line subclass.

Rebased on top of #18 and #19 which just merged — the two "already-applied" commits got skipped cleanly, leaving one commit and a much smaller diff (~360 lines vs the original 1,626). Local tests: 159 passed, ruff clean.

Design notes I want to call out:

  • AssemblyAI reuses one key across the pipeline. ASSEMBLYAI_API_KEY authenticates both transcription and (via the LLM Gateway) summarisation. That's a genuinely nice UX win — one paste in Settings, whole pipeline works. And the shared key input in the settings screen (render_assemblyai_key_input) makes sure changing it in one place updates the other.
  • Skipping temperature on OpenAICompatibleSummarizer. Claude 5 models on the AssemblyAI gateway reject any temperature value; the SDK default is fine for summaries everywhere else. Good comment explaining why.
  • MAX_TOKENS = 16000. Same reasoning as fix(summaries): treat a truncated AI summary as an error #18 — reasoning models spend output tokens before writing text. The concrete example in the comment (5,420 tokens for 1,100 tokens of visible summary) is exactly the sort of context a future reader needs.
  • Truncation handling inline rather than via _raise_if_truncated. Slight duplication with the base helper you added in fix(summaries): treat a truncated AI summary as an error #18, but the inline version has a nicer message (includes MAX_TOKENS in the error). Fine either way.
  • DeepInfra as a second concrete example — good sanity check that the base class actually is generic; if adding DeepInfra had needed base-class changes, we'd know the abstraction was leaky.

Test coverage:

  • test_summarizer_uses_gateway_and_parses — confirms the base URL, model id, prompt content and parsed action items flow correctly.
  • test_summarizer_requires_key, test_deepinfra_requires_its_own_key — the DeepInfra one is particularly good because it sets ASSEMBLYAI_API_KEY to prove it doesn't fall back to the wrong provider's key.
  • test_validate_accepts_deepinfra_with_assemblyai_transcription — the mix-and-match case (DeepInfra summariser + AssemblyAI transcriber) is a real user config, worth locking down.
  • test_note_maker_summarises_speaker_text — verifies that summary_input (the speaker-labelled form) is what actually reaches the summariser, not the raw transcript_text. This is the invariant that makes speaker labels useful for action items.
  • test_output_budget_leaves_room_for_reasoning — locks the 16k budget so a future well-meaning cleanup doesn't regress it.
  • test_assemblyai_provider_and_transcriber_switching in test_textual_smoke.py — drives the settings screen through eight provider/transcriber toggles and asserts the shared key input mounts exactly once. Great regression guard for the DOM interaction that's easy to get subtly wrong.

Merging locally (same workflow scope issue as #19 — my gh token can't touch .github/workflows/ci.yml even though this PR doesn't actually touch it after rebase; hitting it safely just in case).

@jamespember

Copy link
Copy Markdown
Owner

Merged as commit fcdd9be on main (same local-squash workflow as #19). GitHub marks the branch closed rather than merged; contribution is preserved via Co-authored-by: on the squash commit. Thanks again @bryangruneberg — this is a well-shaped abstraction.

@bryangruneberg

Copy link
Copy Markdown
Contributor Author

Nice one 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants