From e0c53f8719bcc992a55a4df50910bf6b602b98ce Mon Sep 17 00:00:00 2001 From: Mathias Date: Sun, 4 Oct 2026 22:41:21 +0200 Subject: [PATCH] feat(uploads): an auto-language job is planned without the engines it never runs (#61) PlanFor(lang) on the Ensemble and a LanguagePlanner the worker prefers: the page lists the other engine (and the fallback if configured), not the base engine, the comparison and the decisions as coming. Found by the PR #62 review. Co-Authored-By: Claude Sonnet 5.5 --- internal/adapters/transcribe/ensemble.go | 26 +++++++---- .../transcribe/ensemble_progress_test.go | 19 ++++++++ internal/uploads/worker.go | 17 ++++++- internal/uploads/worker_progress_test.go | 44 +++++++++++++++++++ 4 files changed, 96 insertions(+), 10 deletions(-) diff --git a/internal/adapters/transcribe/ensemble.go b/internal/adapters/transcribe/ensemble.go index 27387be..23854a1 100644 --- a/internal/adapters/transcribe/ensemble.go +++ b/internal/adapters/transcribe/ensemble.go @@ -32,14 +32,24 @@ func NewEnsemble(base, alt, fallback Backend, adj reconcile.Adjudicator, opts re return &Ensemble{base: base, alt: alt, fallback: fallback, adj: adj, opts: opts} } -// Plan implements uploads.Planner: the phases this ensemble reports, so the page can -// show them before they run. The fallback is a phase only when one is configured. -func (e *Ensemble) Plan() []progress.Planned { - plan := []progress.Planned{ - {Key: "engine_a", Label: "Transcribing with " + e.base.Name()}, - {Key: "engine_b", Label: "Transcribing with " + e.alt.Name()}, - {Key: "compare", Label: "Comparing the two transcripts"}, - {Key: "decide", Label: "Deciding the differences"}, +// Plan implements uploads.Planner: the phases of a Swedish job, the two-engine one. The +// fallback is a phase only when one is configured. +func (e *Ensemble) Plan() []progress.Planned { return e.PlanFor(domain.LangSwedish) } + +// PlanFor implements uploads.LanguagePlanner. A recording that is not Swedish never runs the +// base engine, the comparison or the decisions (transcribeOther), so the page lists only +// what can run: the other engine, then the fallback if one is configured. +func (e *Ensemble) PlanFor(lang domain.Language) []progress.Planned { + var plan []progress.Planned + if lang == domain.LangSwedish || lang == "" { + plan = []progress.Planned{ + {Key: "engine_a", Label: "Transcribing with " + e.base.Name()}, + {Key: "engine_b", Label: "Transcribing with " + e.alt.Name()}, + {Key: "compare", Label: "Comparing the two transcripts"}, + {Key: "decide", Label: "Deciding the differences"}, + } + } else { + plan = []progress.Planned{{Key: "engine_b", Label: "Transcribing with " + e.alt.Name()}} } if e.fallback != nil { plan = append(plan, progress.Planned{Key: "fallback", Label: "Cloud fallback (" + e.fallback.Name() + ")"}) diff --git a/internal/adapters/transcribe/ensemble_progress_test.go b/internal/adapters/transcribe/ensemble_progress_test.go index 4c779cf..0a72aa1 100644 --- a/internal/adapters/transcribe/ensemble_progress_test.go +++ b/internal/adapters/transcribe/ensemble_progress_test.go @@ -160,3 +160,22 @@ func TestEnsembleProgress_TheReconcileCountsAreNotedForTheOperator(t *testing.T) assert.Equal(t, 0, n["unsure"]) assert.Equal(t, 0, n["errors"]) } + +// An auto-language upload never runs the base engine, the comparison or the decisions, so the +// page must not list them as coming (tapir#61, PR #62 review). +func TestEnsemblePlanFor_AnAutoLanguageListsOnlyThePhasesThatCanRun(t *testing.T) { + with := newEnsemble("a", "a", nil, nil, true, &verdictAdj{}).e + without := newEnsemble("a", "a", nil, nil, false, &verdictAdj{}).e + keys := func(p []progress.Planned) []string { + var k []string + for _, s := range p { + k = append(k, s.Key) + } + return k + } + + assert.Equal(t, []string{"engine_b", "fallback"}, keys(with.PlanFor(domain.LangAuto))) + assert.Equal(t, []string{"engine_b"}, keys(without.PlanFor(domain.LangAuto)), "no fallback configured: none promised") + assert.Equal(t, with.Plan(), with.PlanFor(domain.LangSwedish), "Swedish is the plan it always was") + assert.Contains(t, with.PlanFor(domain.LangAuto)[0].Label, "iguana/kb-whisper") +} diff --git a/internal/uploads/worker.go b/internal/uploads/worker.go index 3ab7976..917f5fb 100644 --- a/internal/uploads/worker.go +++ b/internal/uploads/worker.go @@ -33,6 +33,13 @@ type Transcriber interface { Transcribe(ctx context.Context, wavPath string) (domain.Transcription, error) } +// LanguagePlanner is a Planner whose phases depend on the language of the job: a recording +// that is not Swedish skips the second engine and the reconcile, and the page should not list +// them as coming (tapir#61). The worker prefers it over Planner. +type LanguagePlanner interface { + PlanFor(lang domain.Language) []progress.Planned +} + // Planner is an optional extra on a Transcriber: it declares its own phases (for // example one per engine) so the page can show them. A Transcriber that does not // implement it gets a single "transcribing" phase from the worker. Whatever a Planner @@ -251,8 +258,14 @@ func (w *Worker) Process(ctx context.Context, job Job) { } plan := []progress.Planned{{Key: "received", Label: "Received"}, {Key: "audio", Label: "Preparing the audio"}} + lang := job.Language + if lang == "" { + lang = domain.LangSwedish // queued before the choice existed + } planner, planned := w.tr.(Planner) - if planned { + if lp, ok := w.tr.(LanguagePlanner); ok { + plan = append(plan, lp.PlanFor(lang)...) + } else if planned { plan = append(plan, planner.Plan()...) } else { plan = append(plan, progress.Planned{Key: "transcribe", Label: "Transcribing"}) @@ -313,7 +326,7 @@ func (w *Worker) Process(ctx context.Context, job Job) { if !planned { tr.Begin("transcribe", 0) } - t, err := w.tr.Transcribe(domain.WithLanguage(progress.With(jobCtx, tr), job.Language), wav) + t, err := w.tr.Transcribe(domain.WithLanguage(progress.With(jobCtx, tr), lang), wav) if err != nil { log.Error("upload: transcribe", "kind", errkind.Of(err), "elapsed", time.Since(start).Round(time.Second)) abort(domain.UploadErrTranscriptionFailed) diff --git a/internal/uploads/worker_progress_test.go b/internal/uploads/worker_progress_test.go index 58faf8c..7168bb7 100644 --- a/internal/uploads/worker_progress_test.go +++ b/internal/uploads/worker_progress_test.go @@ -213,3 +213,47 @@ func TestWorkerProgress_AStoreFailureLogsItsSQLStateNeverTheValueItQuotes(t *tes assert.Contains(t, out, "upload: store transcript") assert.Contains(t, out, "kind=sql_22P02") } + +// langPlannedTr plans per language, like the Ensemble: an auto job has no base engine. +type langPlannedTr struct{ *fakeTranscriber } + +func (langPlannedTr) Plan() []progress.Planned { return plannedTr{}.Plan() } +func (langPlannedTr) PlanFor(l domain.Language) []progress.Planned { + if l == domain.LangAuto { + return []progress.Planned{{Key: "engine_b", Label: "Transcribing with B"}} + } + return plannedTr{}.Plan() +} + +func (p langPlannedTr) Transcribe(ctx context.Context, wav string) (domain.Transcription, error) { + r := progress.From(ctx) + if domain.LanguageFrom(ctx) != domain.LangAuto { + r.Begin("engine_a", 0) + r.Done("engine_a") + } + r.Begin("engine_b", 0) + r.Done("engine_b") + return p.fakeTranscriber.Transcribe(ctx, wav) +} + +func TestWorkerProgress_AJobGetsThePlanOfItsLanguage(t *testing.T) { + for name, tt := range map[string]struct { + lang domain.Language + want []string + }{ + "auto": {domain.LangAuto, []string{"received:done", "audio:done", "engine_b:done", "save:done"}}, + "swedish": {domain.LangSwedish, []string{"received:done", "audio:done", "engine_a:done", "engine_b:done", "save:done"}}, + "no language (queued before the choice existed)": {"", []string{"received:done", "audio:done", "engine_a:done", "engine_b:done", "save:done"}}, + } { + t.Run(name, func(t *testing.T) { + r := newWRig(t, time.Minute) + w := newProgressWorker(t, r, langPlannedTr{r.tr}) + job := r.spool(t, "a") + job.Language = tt.lang + + w.Process(context.Background(), job) + + assert.Equal(t, tt.want, keysAndStates(r.st.lastSteps())) + }) + } +}