Skip to content

refactor(transcription): share source-aware provider execution - #409

Merged
steipete merged 1 commit into
mainfrom
codex/unify-transcription-runner
Sep 5, 2026
Merged

refactor(transcription): share source-aware provider execution#409
steipete merged 1 commit into
mainfrom
codex/unify-transcription-runner

Conversation

@steipete

@steipete steipete commented Sep 5, 2026

Copy link
Copy Markdown
Owner

What changed

Replace the parallel file/byte orchestration stacks with one source-aware transcription run. Local engines, cloud provider ordering, decode retries, progress, and accumulated notes now have one owner. File and byte adapters remain only where their I/O and upload policies genuinely differ.

This removes 724 production lines from the orchestration cluster and 669 lines overall, including additional regression coverage. No public API or provider configuration changes.

Preserved boundaries

Keep native-file providers lazy, read full input only when a byte provider requires it, and retain provider-specific upload/chunking policies. A failed chunking result remains terminal. Decode retries replace the source only after a completed retry; a thrown retry preserves the original source. Nested segment calls clear parent duration, progress, and diarization.

Validation

  • Build, formatting, lint, and type checking pass.
  • Eight new source-routing/decode-retry regressions pass; local engine coverage is now shared across file and byte inputs.
  • Independent Codex review: no actionable P0–P2 findings.
  • Final local full gate: 3,048 passed, 43 skipped; 94.21% line coverage and 85.25% branch coverage. An earlier run hit filesystem timeouts on a heavily loaded host; all 86 tests in those suites and then the full gate passed unchanged. Timeout settings remain unchanged.
  • Hosted CI passed on the exact PR head: Node 24, Chromium extension E2E, Firefox extension smoke, and GitGuardian.

@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 5, 2026
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 4, 2026, 8:56 PM ET / September 5, 2026, 00:56 UTC.

ClawSweeper review

What this changes

Consolidates file and in-memory transcription into shared provider execution, local-engine handling, and decode retries, with updated documentation and regression tests.

Merge readiness

Ready for maintainer review

Keep open: this remains useful owner-authored work, and the pinned main and latest supplied release still use separate orchestration paths. No actionable introduced defect or merge blocker was found.

Priority: P3
Reviewed head: f47974eac713815dd3aa01d0c030f8e0d01f4d8e

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-covered consolidation with no blocking correctness or security findings.
Proof confidence 🌊 off-meta tidepool Not applicable: The OWNER-authored internal refactor is exempt from ordinary contributor runtime proof and does not materially change authorization. Routing and retry tests plus captured exact-head CI support validation but are not presented as live transcription proof.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The OWNER-authored internal refactor is exempt from ordinary contributor runtime proof and does not materially change authorization. Routing and retry tests plus captured exact-head CI support validation but are not presented as live transcription proof.
Evidence reviewed 10 items Repository policy and scope: Verified the origin repository, read the complete root AGENTS.md, and found no nested AGENTS.md files or applicable maintainer-notes directory. Applied the core-library boundary and read-only checkout guidance.
Pinned introduction and review continuity: The checkout is the exact original PR head, whose raw recorded parent is d7faa6a. The previous completed review inspected this same head and retained no findings; the comparison against HEAD is empty.
Shared execution preserves caller boundaries: Both existing entrypoints feed the shared runner. Diarization remains separate, Groq precedes local engines, and segment calls explicitly clear parent duration, progress, and diarization. Podcast and downloaded-video callers retain their existing arguments and result handling.
Findings None None.
Security None None.

How this fits together

Summarize's core transcription layer receives downloaded media files or bytes and configured provider credentials. It selects local or cloud transcription engines and returns text, timestamps, progress, and diagnostics to the transcript-first summarization pipeline.

flowchart TD
  A[Media file or bytes] --> B[Shared transcription run]
  B --> C{Speaker labels requested?}
  C -->|Yes| D[Diarization providers]
  C -->|No| E[Groq then local engines]
  E -->|Fallback| F[Ordered cloud providers]
  F --> G[Source-specific uploads and retries]
  D --> H[Transcript and diagnostics]
  E -->|Success| H
  G --> H
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test LOC Production −724; tests +53; documentation +2; total −669 The measured reduction matches the stated consolidation goal while increasing regression-test code.

Technical review

Best possible solution:

Maintain one transcription runner while keeping file-specific upload limits, full-input fallback, provider selection, and progress semantics explicit.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR refactors existing behavior rather than reporting a failing workflow; source comparison and existing regression cases provide the relevant checks.

Is this the best way to solve the issue?

Yes: centralizing orchestration while retaining source-specific I/O policies removes duplication without replacing the existing provider contracts or configuration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d7faa6a0f4bc.

Labels

Label justifications:

  • P3: This is maintenance-oriented consolidation with preserved behavior and no identified urgent user-facing regression.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The OWNER-authored internal refactor is exempt from ordinary contributor runtime proof and does not materially change authorization. Routing and retry tests plus captured exact-head CI support validation but are not presented as live transcription proof.

Evidence

What I checked:

  • Repository policy and scope: Verified the origin repository, read the complete root AGENTS.md, and found no nested AGENTS.md files or applicable maintainer-notes directory. Applied the core-library boundary and read-only checkout guidance. (AGENTS.md:1, f47974eac713)
  • Pinned introduction and review continuity: The checkout is the exact original PR head, whose raw recorded parent is d7faa6a. The previous completed review inspected this same head and retained no findings; the comparison against HEAD is empty. (f47974eac713)
  • Shared execution preserves caller boundaries: Both existing entrypoints feed the shared runner. Diarization remains separate, Groq precedes local engines, and segment calls explicitly clear parent duration, progress, and diarization. Podcast and downloaded-video callers retain their existing arguments and result handling. (packages/core/src/transcription/whisper/core.ts:56, f47974eac713)
  • Upload and fallback invariants: Compared both pre-refactor remote orchestration paths with the shared loop. Native-file dispatch remains deferred until needed; byte-only providers trigger materialization; oversized OpenAI files retain the existing chunking or full-input Deepgram fallback policy; chunk results remain terminal. (packages/core/src/transcription/whisper/remote.ts:43, f47974eac713)
  • Decode retries and full-input preservation: The extracted retry helper adopts converted bytes only after a completed retry and preserves the original input when retrying throws. OpenAI restores the original oversized source after a failed truncated attempt, matching the previous implementation. (packages/core/src/transcription/whisper/remote-provider-attempts.ts:111, f47974eac713)
  • Focused regression coverage: Read all eight new routing/retry cases and the parameterized local-engine tests. They cover lazy file fallback, one-time byte conversion, terminal chunk failure, retry source ownership, progress, accumulated notes, and temporary-file cleanup. Existing entrypoint tests also cover oversized Deepgram fallback and Groq call counts; tests were not executed during this read-only review. (tests/transcription.remote-run.test.ts:71, f47974eac713)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-05T00:42:11.855Z sha f47974e :: needs maintainer review before merge. :: none

@steipete
steipete merged commit 56d9c76 into main Sep 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant