Compare commits
78
Commits
7029dd1d7f
...
aa54089230
@@ -127,6 +127,9 @@ The repo is **green and shipping** — last tag `v0.15.0`. `task check` passes (
|
||||
implement `uploads.Planner` to declare its own phases. The page tells running / quiet / lost from
|
||||
two timestamps (`alive_at` heartbeat vs `progress_at`) and never shows a percentage that does not
|
||||
come from real counts. **Nothing logged may carry transcript text, words from it, or the spool path.**
|
||||
Errors reach a log, a row or a page as `errkind.Of(err)` (a kind such as `timeout` or `http_502`),
|
||||
never `err.Error()`: error text can hold a provider's response body, the spool path or transcript
|
||||
words (ADR-035 decision 8). Local-only for anything that carries transcript text: `config.IsLocalModel`.
|
||||
`TAPIR_SHOT_DIR=… go test -run TestRenderUploadProgressStatesForReview ./internal/web` renders every
|
||||
state for a screenshot review.
|
||||
**Lint caveat:** `task check` does not fail on lint (#41: the Taskfile `|| echo` swallows
|
||||
|
||||
+44
-4
@@ -1461,8 +1461,9 @@ an hour of Swedish *was* an unknown here; it is now measured, see decision 8.)
|
||||
- **The opening model.** The offered list leads with the summarizer default `koala/phi4-mini`,
|
||||
whose 8,192-token context returns HTTP 400 on an hour of speech (~13,000 tokens). Chat over
|
||||
an upload opens on `TAPIR_UPLOAD_CHAT_MODEL` (default `iguana/gemma4-26b`); the switcher
|
||||
still lists every model, and picking `phi4-mini` on a long recording fails loudly and
|
||||
inline. A per-model budget is not built.
|
||||
lists **local models only** (amended by ADR-035 after the adversarial review found it listed
|
||||
the cloud model too: the whole private transcript goes to the one chosen), and picking
|
||||
`phi4-mini` on a long recording fails loudly and inline. A per-model budget is not built.
|
||||
Past ~90,000 chars (≈100 minutes) no offered model can answer by stuffing; see triggers.
|
||||
|
||||
9. **Not decided here.** Rich `AVSNITT`/`ALTERNATIV` analysis (gated on #30 — `analyze_srt.py`
|
||||
@@ -1513,13 +1514,20 @@ deferred. The scope cut — chat first, no rich analysis, no video — is what k
|
||||
ADR-016 window, it was the interesting thing, not the useful one — remove it.
|
||||
|
||||
**Open items this ADR creates.**
|
||||
- **DECISION NEEDED — ffmpeg in the runtime image.** *Options:* (A) static ffmpeg/ffprobe copied
|
||||
- **DECIDED (Mathias, 2026-10-02): option A.** Static ffmpeg and ffprobe 7.0.2 are copied into the
|
||||
distroless image, pinned by version and sha256 (Dockerfile in this PR; #34). Measured at that time:
|
||||
**about 160 MB** added, not the 70 MB first estimated (80 MB per binary); the build is **GPLv3**;
|
||||
the checksum proves the bytes reviewed, not the publisher; upstream's `releases/` URL moves on, so
|
||||
vendoring the verified tarball is the durable fix (not done). Uploads stay off until the deployment
|
||||
sets `TAPIR_UPLOADS_ENABLED`. *The question as it was put:* *Options:* (A) static ffmpeg/ffprobe copied
|
||||
into distroless **(recommended — the only option that satisfies "accept m4a/mp3/opus/mp4,
|
||||
validate duration, reject video"; pure-Go decoders cover wav/mp3 only)**; (B) accept WAV
|
||||
only, convert client-side (fails "I just want to upload it"); (C) a transcode sidecar
|
||||
(second deployable, ADR-018's reasoning against). *Blocks:* the Dockerfile slice, not the
|
||||
tests, which use the port.
|
||||
- **DECISION NEEDED — berget fallback for private audio.** *Options:* (A) enabled by default as
|
||||
- **DECIDED (Mathias, 2026-10-02): option A, enabled by default (#35).** Scope: the audio
|
||||
transcription fallback only. Reconciliation text and chat over a recording never go to a cloud
|
||||
model (ADR-034, ADR-035). *The question as it was put:* *Options:* (A) enabled by default as
|
||||
#33 specifies, with the serving endpoint shown **(recommended, matches the issue)**; (B)
|
||||
disabled by default, a failed iguana job fails honestly and the user retries. *Blocks:*
|
||||
the production default only; the adapter ships either way.
|
||||
@@ -1642,6 +1650,12 @@ recording.
|
||||
~30 disputes where the reconciled text differs from pianissimo alone. Keep the feature only if the
|
||||
reconciled reading is right at least 85% of the time **and** no worse than the better single engine.
|
||||
If not, switch it off (config) and keep the single-engine path.
|
||||
**Decision (Mathias, 2026-10-02), on the first result below: keep it as an opt-in feature**, reading
|
||||
the bar as 95% of decided items, with the second condition to be answered on the next real run.
|
||||
For deployment the reconciler is `iguana/gemma4-31b` (97% against the speaker's answers, about 22
|
||||
minutes for the 63.6-minute recording with Low Power Mode off). A cloud reconciler was raised and is
|
||||
**not adopted**: it would send private transcript text to a cloud API on every upload; recorded as
|
||||
an evaluation question in #45.
|
||||
**First result, 2026-10-02, before merge** (`docs/research/reconcile-blind-check-2026-10-02.md`): 30
|
||||
random replacement spots on the real 63.6-minute recording, judged blind by Opus from text (not by the
|
||||
speaker, who delegated it): 21 right, 1 worse than pianissimo, 3 neither reading right, 5 undecidable.
|
||||
@@ -1711,6 +1725,32 @@ wording while only `content` is merged.
|
||||
a segment edge), they stay the base's and the result says so in the log. This replaces ADR-034
|
||||
decision 6's "segments stay untouched".
|
||||
|
||||
8. **Hardening from the adversarial review** (`skills:pr-gatekeeper`, head `7029dd1`: BLOCK; recorded on
|
||||
PR #39). Each finding was reproduced by a failing test first.
|
||||
- **Chat over a recording offers and accepts local models only** (it listed `berget/mistral-small`);
|
||||
`TAPIR_UPLOAD_CHAT_MODEL` must be local at start-up; with no local model, nothing is sent.
|
||||
- **A model alias counts as local only if it is one plain name after `koala/` or `iguana/`**
|
||||
(`koala/../berget/x`, `koala//x`, `koala/x?host=…` were accepted by a bare prefix check).
|
||||
- **An error reaches a log line, the row or the page as a kind** (`timeout`, `canceled`, `http_502`,
|
||||
`network`, `error`), never its text, which can hold a provider's response body, the spool path or
|
||||
a word from the transcript (`internal/errkind`). The stored reconcile reason and an engine's
|
||||
failure reason are kinds; the page no longer shows provider text.
|
||||
- **Every local engine call has its own deadline** (`TAPIR_TRANSCRIBE_ENGINE_TIMEOUT`, default 25
|
||||
minutes), so a wedged engine fails while the job lives and the fallback can run; the job's own
|
||||
deadline or a cancel still never reaches the cloud.
|
||||
- **The duration limit is checked against what is converted**, not the file's header: ffmpeg is
|
||||
capped at the limit plus a minute, a longer result fails the job (`audio_too_long`), and a result
|
||||
under 80% of the claimed length and more than two minutes short fails it
|
||||
(`audio_incomplete`) instead of returning a partial transcript with no warning.
|
||||
- **The queue is bounded**, 20 waiting in total and 3 per user; a full queue is a 429 with a typed
|
||||
reason, never a stuck row or a server error.
|
||||
- **Bad input is a 4xx**: a non-UUID id is a 404, a malformed multipart header is a 400 whose text is
|
||||
not logged, and a title or file name with control bytes or invalid UTF-8 is cleaned, not a 500.
|
||||
Not done, filed as follow-ups: a vendored ffmpeg tarball, CSRF cookie review, ffprobe CPU and
|
||||
stream-count limits, mov external references, `alive_at` clock skew, a hung database blocking the
|
||||
tracker, an unbounded gateway response body, a user-facing notice for a recording that is partial but
|
||||
inside the tolerance.
|
||||
|
||||
**Alternatives rejected.** Server-sent events or a websocket: a long-lived connection through the
|
||||
ingress, with buffering to get wrong, for a page that changes every few seconds at best; polling is what
|
||||
the page already does. A percentage from the engine calls: there is no data behind it. A separate
|
||||
|
||||
+36
@@ -16,12 +16,48 @@ COPY . .
|
||||
# no libc. Trim symbols/DWARF to shrink the layer.
|
||||
RUN CGO_ENABLED=0 GOOS=linux go build -trimpath -ldflags='-s -w' -o /out/tapir ./cmd/tapir
|
||||
|
||||
# ── ffmpeg ──────────────────────────────────────────────────────────────────
|
||||
# Audio upload probes files with ffprobe and converts them with ffmpeg (ADR-033,
|
||||
# #34 option A). Static builds, so they run in the distroless image below.
|
||||
#
|
||||
# Pinned by version AND sha256: a changed upstream file fails the build instead of
|
||||
# shipping. The checksum proves these are the bytes that were reviewed (2026-10-02;
|
||||
# the publisher's md5 matched too). It does NOT prove authenticity: the publisher
|
||||
# signs nothing we can check. amd64 only (the cluster is one amd64 node).
|
||||
#
|
||||
# These builds are GPLv3 (--enable-gpl --enable-version3). The service is private and
|
||||
# the image is not distributed; the licence text is copied into the image anyway.
|
||||
#
|
||||
# The release URL moves on when upstream releases a newer version, so the pinned
|
||||
# version is tried in releases/ and then old-releases/. If neither has it the build
|
||||
# fails; vendoring the verified tarball is the durable fix.
|
||||
FROM golang:1.26 AS ffmpeg
|
||||
|
||||
ARG FFMPEG_VERSION=7.0.2
|
||||
ARG FFMPEG_SHA256=abda8d77ce8309141f83ab8edf0596834087c52467f6badf376a6a2a4c87cf67
|
||||
|
||||
RUN set -eu; \
|
||||
mkdir -p /out; \
|
||||
for dir in releases old-releases; do \
|
||||
curl -fsSL -o /out/ffmpeg.tar.xz "https://johnvansickle.com/ffmpeg/${dir}/ffmpeg-${FFMPEG_VERSION}-amd64-static.tar.xz" && break; \
|
||||
done; \
|
||||
echo "${FFMPEG_SHA256} /out/ffmpeg.tar.xz" | sha256sum -c -; \
|
||||
tar -xJf /out/ffmpeg.tar.xz -C /out --strip-components=1 --wildcards '*/ffmpeg' '*/ffprobe' '*/GPLv3.txt'; \
|
||||
rm /out/ffmpeg.tar.xz
|
||||
|
||||
# ── runtime ─────────────────────────────────────────────────────────────────
|
||||
# distroless static + nonroot: no shell, no package manager, runs as uid 65532.
|
||||
# ca-certificates are bundled, which the OIDC/HTTPS clients need.
|
||||
FROM gcr.io/distroless/static-debian12:nonroot
|
||||
|
||||
COPY --from=build /out/tapir /tapir
|
||||
COPY --from=ffmpeg /out/ffmpeg /out/ffprobe /usr/local/bin/
|
||||
COPY --from=ffmpeg /out/GPLv3.txt /usr/share/doc/ffmpeg/GPLv3.txt
|
||||
|
||||
# Where the service finds them. Uploads stay OFF until TAPIR_UPLOADS_ENABLED is set
|
||||
# (a deployment decision, not an image one).
|
||||
ENV TAPIR_FFMPEG_BIN=/usr/local/bin/ffmpeg \
|
||||
TAPIR_FFPROBE_BIN=/usr/local/bin/ffprobe
|
||||
|
||||
EXPOSE 8080
|
||||
USER nonroot:nonroot
|
||||
|
||||
@@ -137,6 +137,20 @@ func chatModels(cfg config.Config) []string {
|
||||
return models
|
||||
}
|
||||
|
||||
// uploadChatModels is the chat switcher for a PRIVATE uploaded recording: the same
|
||||
// ordered set as chatModels, local models only. The whole transcript (up to 90,000
|
||||
// characters) is sent to the one chosen, so a cloud model the video chat offers must not
|
||||
// be offered here; with none local, none is offered (ADR-035 review).
|
||||
func uploadChatModels(cfg config.Config) []string {
|
||||
var local []string
|
||||
for _, m := range chatModels(cfg) {
|
||||
if config.IsLocalModel(m) {
|
||||
local = append(local, m)
|
||||
}
|
||||
}
|
||||
return local
|
||||
}
|
||||
|
||||
// buildChat wires the per-video chat service (ADR-027): a Completer factory over
|
||||
// the SAME LiteLLM gateway the summarizer uses (a different alias per model, not a
|
||||
// second client config) and the same transcript-truncation budget. It returns nil
|
||||
|
||||
+16
-4
@@ -55,8 +55,8 @@ func buildUploads(ctx context.Context, cfg config.Config, st uploadsStore, log *
|
||||
log.Warn("uploads: marked interrupted by a restart (audio is not retained; they must be re-uploaded)", "count", n)
|
||||
}
|
||||
|
||||
worker := uploads.NewWorker(st, &audio.FFmpeg{Bin: cfg.FFmpegBin}, buildTranscriber(cfg),
|
||||
cfg.UploadTmpDir, cfg.TranscribeTimeout)
|
||||
worker := uploads.NewWorker(st, newConverter(cfg), buildTranscriber(cfg),
|
||||
cfg.UploadTmpDir, cfg.TranscribeTimeout, uploads.WithMaxDuration(cfg.UploadMaxDuration))
|
||||
intake := uploads.NewIntake(st, &audio.FFprobe{Bin: cfg.FFprobeBin}, worker,
|
||||
uploadLimits(cfg), cfg.UploadTmpDir)
|
||||
return intake, worker, nil
|
||||
@@ -91,6 +91,13 @@ func wireUploads(app *web.App, cfg config.Config, in *uploads.Intake) {
|
||||
app.UploadLimits = uploadLimits(cfg)
|
||||
app.UploadChatMaxChars = cfg.MaxUploadedTranscriptChars
|
||||
app.UploadChatModel = cfg.UploadChatModel
|
||||
app.UploadChatModels = uploadChatModels(cfg)
|
||||
}
|
||||
|
||||
// newConverter is the ffmpeg adapter capped at the configured maximum duration, so a file
|
||||
// whose header lies about its length cannot become hours of WAV (ADR-035 review).
|
||||
func newConverter(cfg config.Config) *audio.FFmpeg {
|
||||
return &audio.FFmpeg{Bin: cfg.FFmpegBin, MaxSeconds: int(cfg.UploadMaxDuration.Seconds())}
|
||||
}
|
||||
|
||||
// adjudicatorTimeout bounds one adjudication call. A call takes ~2 s on gemma4-26b and ~10 s
|
||||
@@ -107,7 +114,12 @@ func buildTranscriber(cfg config.Config) uploads.Transcriber {
|
||||
// No overall client timeout: a 90 minute recording takes minutes. The deadline is
|
||||
// the worker's per-job context.
|
||||
client := &http.Client{}
|
||||
primary := transcribe.NewGateway(cfg.GatewayURL, cfg.GatewayKey, cfg.TranscribeModel, client)
|
||||
// Each LOCAL engine call gets its own deadline: a wedged engine does not answer, and
|
||||
// without one it used the whole job before the fallback could run.
|
||||
local := func(model string) transcribe.Backend {
|
||||
return transcribe.WithTimeout(transcribe.NewGateway(cfg.GatewayURL, cfg.GatewayKey, model, client), cfg.TranscribeEngineTimeout)
|
||||
}
|
||||
primary := local(cfg.TranscribeModel)
|
||||
var fallback transcribe.Backend
|
||||
if cfg.TranscribeFallbackModel != "" && cfg.TranscribeFallbackModel != cfg.TranscribeModel {
|
||||
fallback = transcribe.NewGateway(cfg.GatewayURL, cfg.GatewayKey, cfg.TranscribeFallbackModel, client)
|
||||
@@ -116,7 +128,7 @@ func buildTranscriber(cfg config.Config) uploads.Transcriber {
|
||||
return transcribe.NewChain(primary, fallback)
|
||||
}
|
||||
|
||||
base := transcribe.NewGateway(cfg.GatewayURL, cfg.GatewayKey, cfg.ReconcileBaseModel, client)
|
||||
base := local(cfg.ReconcileBaseModel)
|
||||
// Temperature 0 and a short reply: the adjudicator's accuracy was measured that way.
|
||||
chat := llm.New(cfg.GatewayURL, cfg.GatewayKey, cfg.ReconcileModel, adjudicatorTimeout,
|
||||
llm.WithTemperature(0), llm.WithMaxTokens(200))
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/config"
|
||||
)
|
||||
|
||||
// The chat over an uploaded recording sends the whole private transcript to the model
|
||||
// the user picks, so the switcher for it may offer local models only (ADR-035 review).
|
||||
|
||||
func TestUploadChatModels_NeverOffersACloudModelButKeepsTheLocalOrder(t *testing.T) {
|
||||
var cfg config.Config
|
||||
cfg.SummarizerModel = "koala/phi4-mini"
|
||||
cfg.FallbackModel = "iguana/gemma4-26b"
|
||||
cfg.CloudFallbackModel = "berget/mistral-small"
|
||||
|
||||
got := uploadChatModels(cfg)
|
||||
|
||||
want := []string{"koala/phi4-mini", "iguana/gemma4-26b"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Errorf("uploadChatModels = %v, want %v: the cloud model that the video chat offers must not be offered for a private recording", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadChatModels_ACloudPrimaryIsDroppedToo(t *testing.T) {
|
||||
var cfg config.Config
|
||||
cfg.SummarizerModel = "berget/mistral-small"
|
||||
cfg.FallbackModel = "iguana/gemma4-26b"
|
||||
|
||||
got := uploadChatModels(cfg)
|
||||
|
||||
if !reflect.DeepEqual(got, []string{"iguana/gemma4-26b"}) {
|
||||
t.Errorf("uploadChatModels = %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadChatModels_NoLocalModelMeansNoneOfferedNotACloudDefault(t *testing.T) {
|
||||
var cfg config.Config
|
||||
cfg.SummarizerModel = "berget/mistral-small"
|
||||
|
||||
if got := uploadChatModels(cfg); len(got) != 0 {
|
||||
t.Errorf("uploadChatModels = %v, want none", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,26 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/config"
|
||||
)
|
||||
|
||||
// The converter is told the configured maximum, so a file whose header lies about its
|
||||
// length cannot become hours of WAV (adversarial review F2).
|
||||
|
||||
func TestNewConverter_IsCappedAtTheConfiguredMaximumDuration(t *testing.T) {
|
||||
var cfg config.Config
|
||||
cfg.FFmpegBin = "/usr/local/bin/ffmpeg"
|
||||
cfg.UploadMaxDuration = 90 * time.Minute
|
||||
|
||||
c := newConverter(cfg)
|
||||
|
||||
if c.Bin != "/usr/local/bin/ffmpeg" {
|
||||
t.Errorf("Bin = %q", c.Bin)
|
||||
}
|
||||
if c.MaxSeconds != 5400 {
|
||||
t.Errorf("MaxSeconds = %v, want 5400", c.MaxSeconds)
|
||||
}
|
||||
}
|
||||
@@ -20,15 +20,17 @@ import (
|
||||
// fakeUploadsStore satisfies the store surface buildUploads needs, and counts the
|
||||
// start-up sweep.
|
||||
type fakeUploadsStore struct {
|
||||
sweeps int
|
||||
sweepErr error
|
||||
sweeps int
|
||||
sweepErr error
|
||||
failedKinds []domain.UploadErrorKind
|
||||
}
|
||||
|
||||
func (f *fakeUploadsStore) CreateUpload(context.Context, string, string, bool) (string, error) {
|
||||
return "id", nil
|
||||
}
|
||||
|
||||
func (f *fakeUploadsStore) FailUpload(context.Context, string, string, domain.UploadErrorKind) error {
|
||||
func (f *fakeUploadsStore) FailUpload(_ context.Context, _, _ string, k domain.UploadErrorKind) error {
|
||||
f.failedKinds = append(f.failedKinds, k)
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// The wiring of the per-engine deadline: a gateway whose primary model never answers
|
||||
// must still reach the fallback model inside the job's own deadline (adversarial review
|
||||
// F4). The fake gateway answers per model from the multipart `model` field.
|
||||
|
||||
func TestBuildTranscriber_AWedgedPrimaryReachesTheFallbackBeforeTheJobDeadline(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_ = r.ParseMultipartForm(1 << 20)
|
||||
if r.FormValue("model") == "iguana/kb-whisper" {
|
||||
<-r.Context().Done() // wedged: never answers
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"text":"hej","duration":1,"segments":[{"start":0,"end":1,"text":"hej"}]}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
cfg := uploadCfg(t, true)
|
||||
cfg.GatewayURL = srv.URL
|
||||
cfg.TranscribeModel = "iguana/kb-whisper"
|
||||
cfg.TranscribeFallbackModel = "berget/kb-whisper"
|
||||
cfg.TranscribeEngineTimeout = 150 * time.Millisecond
|
||||
wav := filepath.Join(t.TempDir(), "a.wav")
|
||||
require.NoError(t, os.WriteFile(wav, []byte("RIFF....WAVEfmt "), 0o600))
|
||||
job, cancel := context.WithTimeout(context.Background(), 20*time.Second)
|
||||
defer cancel()
|
||||
|
||||
got, err := buildTranscriber(cfg).Transcribe(job, wav)
|
||||
|
||||
require.NoError(t, err, "the wedged primary used the whole job and the fallback never ran")
|
||||
assert.Equal(t, "berget/kb-whisper", got.Endpoint)
|
||||
}
|
||||
@@ -0,0 +1,39 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||
)
|
||||
|
||||
// The worker buildUploads returns enforces the configured maximum on what was really
|
||||
// converted (adversarial review F2). A stub ffmpeg writes a sparse 100-minute WAV however
|
||||
// short the file claimed to be; the gateway is never reached because the job fails first.
|
||||
|
||||
func TestBuildUploads_TheWorkerRefusesAudioLongerThanTheConfiguredMaximum(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
stub := filepath.Join(dir, "ffmpeg")
|
||||
require.NoError(t, os.WriteFile(stub, []byte("#!/bin/sh\nfor a; do last=\"$a\"; done\ntruncate -s 192000044 \"$last\"\n"), 0o700))
|
||||
cfg := uploadCfg(t, true)
|
||||
cfg.FFmpegBin = stub
|
||||
cfg.UploadMaxDuration = 90 * time.Minute
|
||||
cfg.TranscribeTimeout = time.Minute
|
||||
st := &fakeUploadsStore{}
|
||||
_, w, err := buildUploads(context.Background(), cfg, st, quietLog())
|
||||
require.NoError(t, err)
|
||||
src := filepath.Join(cfg.UploadTmpDir, "upload-x")
|
||||
require.NoError(t, os.WriteFile(src, []byte("audio"), 0o600))
|
||||
|
||||
w.Process(context.Background(), uploads.Job{UserID: "u", UploadID: "id", Path: src, Duration: 10 * time.Second})
|
||||
|
||||
assert.Equal(t, []domain.UploadErrorKind{domain.UploadErrTooLong}, st.failedKinds,
|
||||
"a file that claimed 10 s but converted to 100 minutes must fail, not be sent to an engine")
|
||||
}
|
||||
@@ -23,15 +23,13 @@ it** — endpoints and aliases drift, and this file is a snapshot (2026-06-06),
|
||||
`{"error":"failed to process audio"}` string — it has had at least five causes. Tapir's knobs:
|
||||
`TAPIR_UPLOADS_ENABLED` (off by default), `TAPIR_UPLOAD_MAX_MB` (200), `TAPIR_UPLOAD_MAX_MINUTES`
|
||||
(90), `TAPIR_TRANSCRIBE_MODEL`, `TAPIR_TRANSCRIBE_FALLBACK_MODEL` (empty disables the cloud
|
||||
fallback), `TAPIR_TRANSCRIBE_TIMEOUT` (30m), `TAPIR_UPLOAD_TMP_DIR`, `TAPIR_FFMPEG_BIN`,
|
||||
fallback), `TAPIR_TRANSCRIBE_TIMEOUT` (60m, the whole job), `TAPIR_TRANSCRIBE_ENGINE_TIMEOUT` (25m, one engine call: a wedged engine then fails while the job lives and the fallback can run), `TAPIR_UPLOAD_TMP_DIR`, `TAPIR_FFMPEG_BIN`,
|
||||
`TAPIR_FFPROBE_BIN`, `TAPIR_MAX_UPLOADED_TRANSCRIPT_CHARS` (90000 — the measured ceiling of
|
||||
`iguana/gemma4-26b`, which silently drops the head above it), `TAPIR_UPLOAD_CHAT_MODEL`
|
||||
(`iguana/gemma4-26b`; `koala/phi4-mini`'s 8,192-token context 400s on an hour of speech). Needs `ffmpeg` + `ffprobe`
|
||||
in the runtime image; Tapir's own ingress body limit for a 200 MB upload is unverified (infra).
|
||||
(`iguana/gemma4-26b`; must be a local `koala/` or `iguana/` alias, since the whole private transcript is sent to it; `koala/phi4-mini`'s 8,192-token context 400s on an hour of speech). `ffmpeg` + `ffprobe` are in the runtime image (static, pinned by sha256; #34 decided A on 2026-10-02); Tapir's own ingress body limit for a 200 MB upload is unverified (infra).
|
||||
**Two-engine mode (ADR-034), off unless `TAPIR_RECONCILE_BASE_MODEL` is set:** it runs
|
||||
`iguana/pianissimo` (the base text) and `TAPIR_TRANSCRIBE_MODEL` (kb-whisper) on the same audio, then a
|
||||
LOCAL chat model (`TAPIR_RECONCILE_MODEL`, e.g. `iguana/gemma4-26b`: 92% against a speaker's blind answers
|
||||
at ~2 s a call; `iguana/gemma4-31b` 97% but ~5x slower) chooses between their disagreements.
|
||||
LOCAL chat model (`TAPIR_RECONCILE_MODEL`) chooses between their disagreements. **Deployment choice (2026-10-02): `iguana/gemma4-31b`** (97% against a speaker's blind answers; 92% for `iguana/gemma4-26b`). With iguana's macOS **Low Power Mode off** the 31B decodes at 22 tok/s and reconciled a 63.6-minute recording in 21 min 49 s end to end; with it on it ran at 8 tok/s. Check `pmset -g | grep lowpowermode` before blaming a model or an engine for slowness.
|
||||
`TAPIR_RECONCILE_CHECK_ORDER=true` asks twice per dispute with the order swapped (doubles the time).
|
||||
Every model must start `koala/` or `iguana/`: a cloud alias is a start-up error. The default job timeout is
|
||||
60 minutes. `iguana/pianissimo` is KlangAI's Swedish ASR (a Parakeet/FastConformer on MLX, served by a small
|
||||
|
||||
@@ -29,7 +29,7 @@ ingress configuration, which lives in `infra`. "About 10 MB" is an inference.
|
||||
## Method
|
||||
|
||||
- **Audio.** No known-good Swedish recording is available that is safe to use: the only real
|
||||
one (`IMG_1233.wav`) is the private `/bygge` recording (#31 custody still open) and was
|
||||
one is a private recording (#31 custody still open) and was
|
||||
deliberately not touched. Instead: macOS `say -v Alva` (Swedish) on iguana, a 1,171-char
|
||||
five-paragraph meeting script → 82.5 s of 16 kHz mono PCM WAV (`ffmpeg -ar 16000 -ac 1`).
|
||||
Concatenated ×4, ×8, ×16, ×32, ×44, ×66 with Python `wave` (the issue's own recipe).
|
||||
|
||||
@@ -214,3 +214,44 @@ Feature: Upload an audio recording, get a private Swedish transcript, discuss it
|
||||
When the model changes a word
|
||||
Then the segment containing it carries the reconciled word
|
||||
And the segment's timing is unchanged
|
||||
|
||||
# ADR-035 decision 8: hardening from the adversarial review
|
||||
|
||||
Scenario: Chat about a recording is offered with local models only
|
||||
When I open the chat for my recording
|
||||
Then the model list holds only models that run on the homelab
|
||||
And a cloud model the shared chat service knows is not in it
|
||||
|
||||
Scenario: A forged cloud model is never sent the transcript
|
||||
When a chat request names a cloud model
|
||||
Then a local model answers instead, and nothing was sent to the cloud one
|
||||
|
||||
Scenario: A wedged engine does not use up the whole job
|
||||
When the primary engine never answers
|
||||
Then it fails at its own deadline while the job is still alive
|
||||
And the fallback runs, unless the job itself was cancelled or timed out
|
||||
|
||||
Scenario: A file that lies about its length is refused
|
||||
When a file claims to be short but converts to more than the limit
|
||||
Then the job fails with a plain explanation and no engine receives the audio
|
||||
|
||||
Scenario: A truncated recording is refused, not half transcribed
|
||||
When a file converts to far less audio than it claimed
|
||||
Then the job fails and asks for the complete recording
|
||||
|
||||
Scenario: A full queue is a retry, not an error
|
||||
When too many recordings are already waiting
|
||||
Then the upload is answered as busy, its row is failed honestly and its audio is deleted
|
||||
|
||||
Scenario: One user cannot fill the queue for everyone
|
||||
When one user has several recordings waiting
|
||||
Then further uploads from that user are refused and other users are not affected
|
||||
|
||||
Scenario: Errors are shown as kinds, never their text
|
||||
When a model or an engine fails
|
||||
Then the log, the stored reason and the page carry a kind such as timeout or http_502
|
||||
And never the provider's response, a file path or a word from the transcript
|
||||
|
||||
Scenario: Input that cannot be right is a client error
|
||||
When the id is not an id, or a header or title holds control bytes
|
||||
Then the answer is a 4xx or the title is cleaned, never a server error
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
@@ -17,8 +18,17 @@ import (
|
||||
type FFmpeg struct {
|
||||
// Bin is the ffmpeg executable ("ffmpeg" resolves via PATH).
|
||||
Bin string
|
||||
// MaxSeconds caps the converted length when positive. The duration limit is a claim
|
||||
// in the file's header and can be lied about (a 2 h file claiming 10 s converted to a
|
||||
// 120-minute WAV); this bounds what a lie can cost. A minute of slack is added so a
|
||||
// file at the limit is never cut, and the worker measures the result.
|
||||
MaxSeconds int
|
||||
}
|
||||
|
||||
// lengthSlackSeconds is added to MaxSeconds so ffmpeg's rounding never clips a file that
|
||||
// is really at the limit.
|
||||
const lengthSlackSeconds = 60
|
||||
|
||||
// ToWAV writes src, converted, to dst. Video streams are dropped (-vn): by now the
|
||||
// file has already been probed as audio-only, but a cover-art stream must not end
|
||||
// up in the WAV.
|
||||
@@ -29,7 +39,12 @@ func (f *FFmpeg) ToWAV(ctx context.Context, src, dst string) error {
|
||||
}
|
||||
// Input restrictions go BEFORE -i: placed after it they would apply to the output.
|
||||
args := append([]string{"-y", "-v", "error"}, inputRestrictions...)
|
||||
args = append(args, "-i", src, "-vn", "-ac", "1", "-ar", "16000", "-c:a", "pcm_s16le", "-f", "wav", dst)
|
||||
args = append(args, "-i", src, "-vn", "-ac", "1", "-ar", "16000", "-c:a", "pcm_s16le")
|
||||
if f.MaxSeconds > 0 {
|
||||
// After -i: an OUTPUT cap. Before it, it would limit what is read, not written.
|
||||
args = append(args, "-t", strconv.Itoa(f.MaxSeconds+lengthSlackSeconds))
|
||||
}
|
||||
args = append(args, "-f", "wav", dst)
|
||||
cmd := exec.CommandContext(ctx, bin, args...)
|
||||
cmd.WaitDelay = killGrace
|
||||
if out, err := cmd.CombinedOutput(); err != nil {
|
||||
|
||||
@@ -77,3 +77,35 @@ printf data > "$last"`)
|
||||
assert.Less(t, pw, in, "input options must come BEFORE -i or ffmpeg applies them to the output")
|
||||
assert.Less(t, fw, in)
|
||||
}
|
||||
|
||||
func TestFFmpegToWAV_CapsTheOutputLengthWhenAMaximumIsSet(t *testing.T) {
|
||||
// The duration limit is a CLAIM in the file's header; a file that lies about it can
|
||||
// convert to hours of audio (adversarial review F2: a 2 h file claiming 10 s became a
|
||||
// 120-minute WAV). The cap is an OUTPUT option, after -i, with a minute of slack.
|
||||
rec := filepath.Join(t.TempDir(), "argv")
|
||||
bin := stub(t, `printf '%s\n' "$@" > `+rec+`
|
||||
for a; do last="$a"; done
|
||||
printf data > "$last"`)
|
||||
|
||||
err := (&audio.FFmpeg{Bin: bin, MaxSeconds: 5400}).ToWAV(context.Background(), "/spool/in", filepath.Join(t.TempDir(), "out.wav"))
|
||||
|
||||
require.NoError(t, err)
|
||||
raw, err := os.ReadFile(rec)
|
||||
require.NoError(t, err)
|
||||
joined := " " + strings.Join(strings.Split(strings.TrimSpace(string(raw)), "\n"), " ") + " "
|
||||
assert.Contains(t, joined, " -t 5460 ", "limit plus a minute of slack")
|
||||
assert.Less(t, strings.Index(joined, " -i /spool/in "), strings.Index(joined, " -t 5460 "), "-t after -i is an output cap; before it, an input one")
|
||||
}
|
||||
|
||||
func TestFFmpegToWAV_WithNoMaximumThereIsNoLengthCap(t *testing.T) {
|
||||
rec := filepath.Join(t.TempDir(), "argv")
|
||||
bin := stub(t, `printf '%s\n' "$@" > `+rec+`
|
||||
for a; do last="$a"; done
|
||||
printf data > "$last"`)
|
||||
|
||||
require.NoError(t, (&audio.FFmpeg{Bin: bin}).ToWAV(context.Background(), "/spool/in", filepath.Join(t.TempDir(), "out.wav")))
|
||||
|
||||
raw, err := os.ReadFile(rec)
|
||||
require.NoError(t, err)
|
||||
assert.NotContains(t, " "+strings.ReplaceAll(string(raw), "\n", " ")+" ", " -t ")
|
||||
}
|
||||
|
||||
@@ -55,6 +55,29 @@ func WithMaxTokens(n int) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// maxErrorBody bounds how much of a failing response ends up in an error message.
|
||||
const maxErrorBody = 512
|
||||
|
||||
// HTTPError is a non-200 answer from the gateway. Its text keeps the form callers that
|
||||
// log it already rely on, with the body bounded; its Code is what code may branch on and
|
||||
// what errkind turns into a kind. The body is provider text and can echo the prompt, so
|
||||
// nothing private may persist or display Error() (ADR-035 review F3).
|
||||
type HTTPError struct {
|
||||
Code int
|
||||
Body string
|
||||
}
|
||||
|
||||
func (e *HTTPError) Error() string {
|
||||
body := e.Body
|
||||
if len(body) > maxErrorBody {
|
||||
body = body[:maxErrorBody] + "…"
|
||||
}
|
||||
return fmt.Sprintf("LLM returned %d: %s", e.Code, body)
|
||||
}
|
||||
|
||||
// HTTPStatus implements errkind.HTTPStatuser.
|
||||
func (e *HTTPError) HTTPStatus() int { return e.Code }
|
||||
|
||||
// WithTemperature overrides the sampling temperature. Zero IS a valid value and is sent
|
||||
// as zero: the reconciler's accuracy was measured at temperature 0 and must run there.
|
||||
func WithTemperature(t float64) Option {
|
||||
@@ -166,7 +189,7 @@ func (c *Client) Complete(ctx context.Context, system, user string) (string, err
|
||||
return "", fmt.Errorf("read response: %w", err)
|
||||
}
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return "", fmt.Errorf("LLM returned %d: %s", resp.StatusCode, out)
|
||||
return "", &HTTPError{Code: resp.StatusCode, Body: string(out)}
|
||||
}
|
||||
|
||||
var cr chatResponse
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
package llm
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
)
|
||||
|
||||
// A non-200 from the gateway is an HTTP status error: its kind can be logged and stored
|
||||
// without its body, which can echo the prompt (adversarial review F3).
|
||||
|
||||
func failingGateway(t *testing.T, status int, body string) *httptest.Server {
|
||||
t.Helper()
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(status)
|
||||
_, _ = w.Write([]byte(body))
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
return srv
|
||||
}
|
||||
|
||||
func TestClient_ANon200IsAnHTTPStatusErrorWhoseKindCarriesNoBody(t *testing.T) {
|
||||
srv := failingGateway(t, http.StatusBadRequest, `{"error":"prompt was: Anders ansvarar för virket"}`)
|
||||
|
||||
_, err := New(srv.URL, "", "m", 5*time.Second).Complete(context.Background(), "s", "u")
|
||||
|
||||
if err == nil {
|
||||
t.Fatal("expected an error")
|
||||
}
|
||||
if got := errkind.Of(err); got != "http_400" {
|
||||
t.Errorf("errkind.Of = %q, want http_400", got)
|
||||
}
|
||||
var he *HTTPError
|
||||
if !errors.As(err, &he) || he.Code != http.StatusBadRequest {
|
||||
t.Errorf("error is not an *HTTPError with the status code: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestClient_TheErrorTextIsUnchangedForCallersThatLogIt(t *testing.T) {
|
||||
srv := failingGateway(t, http.StatusBadGateway, "upstream down")
|
||||
|
||||
_, err := New(srv.URL, "", "m", 5*time.Second).Complete(context.Background(), "s", "u")
|
||||
|
||||
if err == nil || err.Error() != "LLM returned 502: upstream down" {
|
||||
t.Errorf("error text = %v: callers that log it already rely on this form", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestClient_AnEnormousErrorBodyIsBoundedInTheErrorText(t *testing.T) {
|
||||
srv := failingGateway(t, http.StatusInternalServerError, strings.Repeat("x", 100_000))
|
||||
|
||||
_, err := New(srv.URL, "", "m", 5*time.Second).Complete(context.Background(), "s", "u")
|
||||
|
||||
if err == nil || len(err.Error()) > 1024 {
|
||||
t.Errorf("error text is %d bytes: a gateway's whole body must not become an error message", len(err.Error()))
|
||||
}
|
||||
}
|
||||
@@ -242,8 +242,8 @@ func TestUploadLifecycle_ReconciliationRoundTripsWithItsContestedSpots(t *testin
|
||||
Status: domain.ReconcileDone, Base: "iguana/pianissimo", Alt: "iguana/kb-whisper", Reconciler: "iguana/gemma4-26b",
|
||||
Disputes: 380, AutoResolved: 90, Adjudicated: 285, Changed: 120, Unsure: 5, Errors: 0,
|
||||
Contested: []domain.ContestedSpot{
|
||||
{Seconds: 2210.5, Base: "SEB", Alt: "SCB", Used: "alt"},
|
||||
{Seconds: 3400, Base: "T+1", Alt: "en T", Used: "base", Unsure: true},
|
||||
{Seconds: 2210.5, Base: "Axel", Alt: "Alex", Used: "alt"},
|
||||
{Seconds: 3400, Base: "A+2", Alt: "en A", Used: "base", Unsure: true},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"fmt"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
"git.d-ma.be/mathias/tapir/internal/progress"
|
||||
"git.d-ma.be/mathias/tapir/internal/reconcile"
|
||||
)
|
||||
@@ -72,9 +73,9 @@ func (e *Ensemble) Transcribe(ctx context.Context, wavPath string) (domain.Trans
|
||||
}
|
||||
return out, nil
|
||||
case berr == nil:
|
||||
return e.single(b, e.base.Name(), fmt.Sprintf("%s failed: %v", e.alt.Name(), aerr)), nil
|
||||
return e.single(b, e.base.Name(), fmt.Sprintf("%s failed (%s)", e.alt.Name(), errkind.Of(aerr))), nil
|
||||
case aerr == nil:
|
||||
return e.single(a, e.alt.Name(), fmt.Sprintf("%s failed: %v", e.base.Name(), berr)), nil
|
||||
return e.single(a, e.alt.Name(), fmt.Sprintf("%s failed (%s)", e.base.Name(), errkind.Of(berr))), nil
|
||||
}
|
||||
|
||||
both := errors.Join(fmt.Errorf("%s: %w", e.base.Name(), berr), fmt.Errorf("%s: %w", e.alt.Name(), aerr))
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
package transcribe_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/adapters/transcribe"
|
||||
"git.d-ma.be/mathias/tapir/internal/reconcile"
|
||||
)
|
||||
|
||||
// When one engine is down the page says so and why; the "why" is a kind, not the
|
||||
// engine's error text, which can carry provider output (adversarial review F3).
|
||||
|
||||
func TestEnsemble_AFailedEnginesReasonIsAKindNotItsErrorText(t *testing.T) {
|
||||
r := newEnsemble("text", "text", nil, errors.New("kb-whisper said: zebrakaka secret words"), false, &verdictAdj{v: reconcile.VerdictAlt})
|
||||
|
||||
got, err := r.e.Transcribe(context.Background(), "/a.wav")
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.NotContains(t, got.Reconciliation.Reason, "zebrakaka")
|
||||
assert.Contains(t, got.Reconciliation.Reason, "iguana/kb-whisper failed", "which engine failed is still said")
|
||||
}
|
||||
|
||||
func TestEnsemble_AnHTTPFailureShowsItsStatusCodeAsTheKind(t *testing.T) {
|
||||
r := newEnsemble("text", "text", &transcribe.StatusError{Code: 502, Snippet: "zebrakaka"}, nil, false, &verdictAdj{})
|
||||
|
||||
got, err := r.e.Transcribe(context.Background(), "/a.wav")
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, got.Reconciliation.Reason, "http_502")
|
||||
assert.NotContains(t, got.Reconciliation.Reason, "zebrakaka")
|
||||
}
|
||||
@@ -31,6 +31,9 @@ type StatusError struct {
|
||||
Snippet string
|
||||
}
|
||||
|
||||
// HTTPStatus implements errkind.HTTPStatuser: the code, never the snippet.
|
||||
func (e *StatusError) HTTPStatus() int { return e.Code }
|
||||
|
||||
func (e *StatusError) Error() string {
|
||||
return fmt.Sprintf("transcribe: gateway returned HTTP %d: %s", e.Code, e.Snippet)
|
||||
}
|
||||
|
||||
@@ -3,11 +3,13 @@ package transcribe_test
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -15,6 +17,7 @@ import (
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/adapters/transcribe"
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
)
|
||||
|
||||
// The httptest server stands in for the LiteLLM gateway: it is the process
|
||||
@@ -230,3 +233,25 @@ func TestGateway_WithNoSegmentsTheTextFieldIsUsedWithWhitespaceCollapsed(t *test
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "Hej allihopa. Vi börjar.", got.Text)
|
||||
}
|
||||
|
||||
func TestStatusError_ExposesItsCodeAsAKindAndNotItsSnippet(t *testing.T) {
|
||||
err := fmt.Errorf("transcribe: %w", &transcribe.StatusError{Code: 502, Snippet: "echoed: Anders ansvarar"})
|
||||
|
||||
assert.Equal(t, "http_502", errkind.Of(err))
|
||||
}
|
||||
|
||||
func TestGateway_AnErrorBodyIsBoundedToTwoHundredBytesInTheStatusError(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
_, _ = w.Write([]byte(strings.Repeat("x", 100_000)))
|
||||
}))
|
||||
defer srv.Close()
|
||||
wav := filepath.Join(t.TempDir(), "a.wav")
|
||||
require.NoError(t, os.WriteFile(wav, []byte("RIFF....WAVEfmt "), 0o600))
|
||||
|
||||
_, err := transcribe.NewGateway(srv.URL, "", "iguana/kb-whisper", srv.Client()).Transcribe(context.Background(), wav)
|
||||
|
||||
var se *transcribe.StatusError
|
||||
require.ErrorAs(t, err, &se)
|
||||
assert.LessOrEqual(t, len(se.Snippet), 200, "a gateway's whole body must not become part of an error")
|
||||
}
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
package transcribe
|
||||
|
||||
import (
|
||||
"context"
|
||||
"time"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
)
|
||||
|
||||
// WithTimeout gives every call to b its own deadline. A wedged engine does not answer at
|
||||
// all (iguana after a Metal OOM, infra#273), and with only the job's deadline it used up
|
||||
// the whole job before a fallback could run -- and Chain then refuses the fallback because
|
||||
// the job context has expired. With its own deadline the call fails while the job is still
|
||||
// alive, which is exactly when the fallback, or the other engine, can take over.
|
||||
//
|
||||
// A zero or negative d returns b unchanged. The wrapper is invisible: Name is b's.
|
||||
func WithTimeout(b Backend, d time.Duration) Backend {
|
||||
if d <= 0 {
|
||||
return b
|
||||
}
|
||||
return timeoutBackend{Backend: b, d: d}
|
||||
}
|
||||
|
||||
type timeoutBackend struct {
|
||||
Backend
|
||||
d time.Duration
|
||||
}
|
||||
|
||||
func (t timeoutBackend) Transcribe(ctx context.Context, wavPath string) (domain.Transcription, error) {
|
||||
ctx, cancel := context.WithTimeout(ctx, t.d)
|
||||
defer cancel()
|
||||
return t.Backend.Transcribe(ctx, wavPath)
|
||||
}
|
||||
@@ -0,0 +1,104 @@
|
||||
package transcribe_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/adapters/transcribe"
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
"git.d-ma.be/mathias/tapir/internal/reconcile"
|
||||
)
|
||||
|
||||
// A wedged engine (iguana after a Metal OOM, infra#273) does not answer at all. With only
|
||||
// the job's deadline, the primary used up the whole job before any fallback could run, and
|
||||
// the chain then refused the fallback because the job context had expired. Each engine call
|
||||
// gets its own deadline (adversarial review F4).
|
||||
|
||||
type hangingBackend struct{ name string }
|
||||
|
||||
func (h hangingBackend) Name() string { return h.name }
|
||||
func (hangingBackend) Transcribe(ctx context.Context, _ string) (domain.Transcription, error) {
|
||||
<-ctx.Done()
|
||||
return domain.Transcription{}, ctx.Err()
|
||||
}
|
||||
|
||||
func TestWithTimeout_AHungBackendFailsAtItsOwnDeadlineWhileTheJobIsStillAlive(t *testing.T) {
|
||||
job, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
b := transcribe.WithTimeout(hangingBackend{name: "iguana/kb-whisper"}, 40*time.Millisecond)
|
||||
|
||||
start := time.Now()
|
||||
_, err := b.Transcribe(job, "/a.wav")
|
||||
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, "timeout", errkind.Of(err))
|
||||
assert.Less(t, time.Since(start), 2*time.Second, "it must give up at its own deadline, not wait for the job's")
|
||||
assert.NoError(t, job.Err(), "the job context is untouched, which is what lets a fallback run")
|
||||
assert.Equal(t, "iguana/kb-whisper", b.Name(), "the wrapper is invisible in the recorded endpoint")
|
||||
}
|
||||
|
||||
func TestWithTimeout_ASuccessPassesStraightThrough(t *testing.T) {
|
||||
calls := &[]string{}
|
||||
inner := &orderedBackend{name: "iguana/kb-whisper", text: "hej", calls: calls}
|
||||
|
||||
got, err := transcribe.WithTimeout(inner, time.Minute).Transcribe(context.Background(), "/a.wav")
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "hej", got.Text)
|
||||
}
|
||||
|
||||
func TestWithTimeout_ZeroMeansNoDeadlineOfItsOwn(t *testing.T) {
|
||||
inner := &orderedBackend{name: "x", text: "hej", calls: &[]string{}}
|
||||
|
||||
assert.Same(t, inner, transcribe.WithTimeout(inner, 0), "a zero timeout must not wrap or change anything")
|
||||
}
|
||||
|
||||
func TestChain_AHungPrimaryReachesTheFallbackWhenItHasItsOwnDeadline(t *testing.T) {
|
||||
calls := &[]string{}
|
||||
fb := &orderedBackend{name: "berget/kb-whisper", text: "fallback text", calls: calls}
|
||||
c := transcribe.NewChain(transcribe.WithTimeout(hangingBackend{name: "iguana/kb-whisper"}, 40*time.Millisecond), fb)
|
||||
job, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
got, err := c.Transcribe(job, "/a.wav")
|
||||
|
||||
require.NoError(t, err, "a wedged primary is exactly what the fallback exists for")
|
||||
assert.Equal(t, "berget/kb-whisper", got.Endpoint)
|
||||
}
|
||||
|
||||
func TestChain_AJobThatEndedWhileThePrimaryHungStillNeverReachesTheFallback(t *testing.T) {
|
||||
calls := &[]string{}
|
||||
fb := &orderedBackend{name: "berget/kb-whisper", text: "fallback text", calls: calls}
|
||||
c := transcribe.NewChain(transcribe.WithTimeout(hangingBackend{name: "iguana/kb-whisper"}, time.Minute), fb)
|
||||
job, cancel := context.WithTimeout(context.Background(), 40*time.Millisecond)
|
||||
defer cancel()
|
||||
|
||||
_, err := c.Transcribe(job, "/a.wav")
|
||||
|
||||
require.Error(t, err)
|
||||
assert.Empty(t, *calls, "the job's own deadline or a cancel must still stop the audio from reaching a cloud endpoint")
|
||||
}
|
||||
|
||||
func TestEnsemble_AHungEngineIsASoftFailureAndTheOtherEnginesTextIsUsed(t *testing.T) {
|
||||
calls := &[]string{}
|
||||
alt := &orderedBackend{name: "iguana/kb-whisper", text: "text from the other engine", calls: calls}
|
||||
e := transcribe.NewEnsemble(
|
||||
transcribe.WithTimeout(hangingBackend{name: "iguana/pianissimo"}, 40*time.Millisecond), alt, nil,
|
||||
&verdictAdj{v: reconcile.VerdictAlt}, ensembleOpts)
|
||||
job, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
got, err := e.Transcribe(job, "/a.wav")
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "text from the other engine", got.Text)
|
||||
assert.Equal(t, domain.ReconcileSingle, got.Reconciliation.Status)
|
||||
assert.Contains(t, got.Reconciliation.Reason, "timeout")
|
||||
assert.False(t, errors.Is(err, context.DeadlineExceeded))
|
||||
}
|
||||
+30
-12
@@ -3,6 +3,7 @@ package config
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -27,6 +28,10 @@ type UploadConfig struct {
|
||||
// TranscribeTimeout bounds one job (conversion + transcription); a 90 minute
|
||||
// recording took ~7.5 minutes on iguana.
|
||||
TranscribeTimeout time.Duration
|
||||
// TranscribeEngineTimeout bounds ONE engine call, so a wedged engine fails while the
|
||||
// job is still alive and the fallback (or the other engine) can take over. The job
|
||||
// timeout alone cannot do that (ADR-035 review).
|
||||
TranscribeEngineTimeout time.Duration
|
||||
// UploadTmpDir is where uploads are spooled and converted. Audio is deleted
|
||||
// after transcription, so this holds at most the in-flight files.
|
||||
UploadTmpDir string
|
||||
@@ -61,6 +66,7 @@ const (
|
||||
defaultTranscribeModel = "iguana/kb-whisper"
|
||||
defaultTranscribeFallbackModel = "berget/kb-whisper"
|
||||
defaultTranscribeTimeout = 60 * time.Minute
|
||||
defaultTranscribeEngineTimeout = 25 * time.Minute
|
||||
defaultMaxUploadedChars = 90000
|
||||
defaultUploadChatModel = "iguana/gemma4-26b"
|
||||
)
|
||||
@@ -106,6 +112,15 @@ func loadUploadConfig() (UploadConfig, error) {
|
||||
}
|
||||
u.TranscribeTimeout = timeout
|
||||
|
||||
engineTimeout, err := durationOr("TAPIR_TRANSCRIBE_ENGINE_TIMEOUT", defaultTranscribeEngineTimeout)
|
||||
if err != nil {
|
||||
return UploadConfig{}, err
|
||||
}
|
||||
if engineTimeout <= 0 {
|
||||
return UploadConfig{}, fmt.Errorf("config: TAPIR_TRANSCRIBE_ENGINE_TIMEOUT must be positive, got %v", engineTimeout)
|
||||
}
|
||||
u.TranscribeEngineTimeout = engineTimeout
|
||||
|
||||
if v := os.Getenv("TAPIR_RECONCILE_CHECK_ORDER"); v != "" {
|
||||
on, err := strconv.ParseBool(v)
|
||||
if err != nil {
|
||||
@@ -116,6 +131,9 @@ func loadUploadConfig() (UploadConfig, error) {
|
||||
if err := u.validateReconcile(); err != nil {
|
||||
return UploadConfig{}, err
|
||||
}
|
||||
if !IsLocalModel(u.UploadChatModel) {
|
||||
return UploadConfig{}, fmt.Errorf("config: TAPIR_UPLOAD_CHAT_MODEL=%q is not a local model (koala/ or iguana/): the whole private transcript is sent to it", u.UploadChatModel)
|
||||
}
|
||||
|
||||
chars, err := intOr("TAPIR_MAX_UPLOADED_TRANSCRIPT_CHARS", defaultMaxUploadedChars)
|
||||
if err != nil {
|
||||
@@ -141,18 +159,18 @@ func positiveInt(key string, fallback int) (int, error) {
|
||||
return n, nil
|
||||
}
|
||||
|
||||
// localPrefixes are the host prefixes of the models that run on the homelab. Private
|
||||
// meeting content may be sent to these and to nothing else for reconciliation
|
||||
// (ADR-034): a cloud alias is refused at start-up rather than discovered later.
|
||||
var localPrefixes = []string{"koala/", "iguana/"}
|
||||
// localAlias is the shape of a model alias that runs on the homelab: a local host
|
||||
// prefix, then one plain name. A bare prefix check let through aliases that are shaped
|
||||
// like an escape (koala/../berget/x, koala//x, koala/x?host=berget): whether the
|
||||
// gateway would route them elsewhere is unknown, so they are refused (ADR-035 review).
|
||||
var localAlias = regexp.MustCompile(`^(koala|iguana)/[A-Za-z0-9][A-Za-z0-9._:-]*$`)
|
||||
|
||||
func isLocalModel(alias string) bool {
|
||||
for _, p := range localPrefixes {
|
||||
if strings.HasPrefix(alias, p) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
// IsLocalModel reports whether alias is a local model. Private meeting content may be
|
||||
// sent to these and to nothing else: reconciliation (ADR-034) and chat over an uploaded
|
||||
// recording (ADR-035) are both held to it, and a cloud alias is refused at start-up
|
||||
// rather than discovered later.
|
||||
func IsLocalModel(alias string) bool {
|
||||
return localAlias.MatchString(alias) && !strings.Contains(alias, "..")
|
||||
}
|
||||
|
||||
// validateReconcile checks the two-engine settings. They are inert while no base
|
||||
@@ -168,7 +186,7 @@ func (u UploadConfig) validateReconcile() error {
|
||||
{"TAPIR_RECONCILE_BASE_MODEL", u.ReconcileBaseModel},
|
||||
{"TAPIR_RECONCILE_MODEL", u.ReconcileModel},
|
||||
} {
|
||||
if !isLocalModel(m.alias) {
|
||||
if !IsLocalModel(m.alias) {
|
||||
return fmt.Errorf("config: %s=%q is not a local model (koala/ or iguana/): private meeting content must not leave the homelab for reconciliation", m.name, m.alias)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// What counts as a LOCAL model alias, and where that is demanded (ADR-034, ADR-035 review).
|
||||
// A private transcript may be sent to a local model and to nothing else; the check is a
|
||||
// guard against a misconfiguration or a crafted alias, so it must be strict about shape,
|
||||
// not just about a prefix.
|
||||
|
||||
func TestIsLocalModel_AcceptsRealLocalAliasesAndNothingShapedLikeAnEscape(t *testing.T) {
|
||||
tests := []struct {
|
||||
alias string
|
||||
want bool
|
||||
}{
|
||||
{"koala/phi4-mini", true},
|
||||
{"iguana/gemma4-26b", true},
|
||||
{"iguana/gemma4-31b-think", true},
|
||||
{"koala/qwen36-35b-a3b", true},
|
||||
{"iguana/kb-whisper", true},
|
||||
{"iguana/hf.co-model_v1:q4", true},
|
||||
{"", false},
|
||||
{"koala/", false},
|
||||
{"iguana/", false},
|
||||
{"gemma4-26b", false},
|
||||
{"berget/mistral-small", false},
|
||||
{"berget/koala/x", false},
|
||||
{"xkoala/x", false},
|
||||
{"Koala/x", false},
|
||||
{" koala/x", false},
|
||||
{"koala/x ", false},
|
||||
{"koala/x y", false},
|
||||
{"koala/x\n", false},
|
||||
{"koala/../berget/kb", false},
|
||||
{"koala//berget/mistral-small", false},
|
||||
{"koala/x/y", false},
|
||||
{"koala/x?host=berget", false},
|
||||
{"koala/x#frag", false},
|
||||
{"koala/x@evil", false},
|
||||
{"koalа/x", false}, // Cyrillic а
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.alias, func(t *testing.T) {
|
||||
if got := IsLocalModel(tt.alias); got != tt.want {
|
||||
t.Errorf("IsLocalModel(%q) = %v, want %v", tt.alias, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoad_UploadChatModelMustBeLocalBecauseTheWholeTranscriptIsSentToIt(t *testing.T) {
|
||||
for _, alias := range []string{"berget/mistral-small", "gemini/gemini-2.5-pro", "gemma4-26b", "koala/../berget/kb"} {
|
||||
t.Run(alias, func(t *testing.T) {
|
||||
clearUploadEnv(t)
|
||||
setEnv(t, map[string]string{"TAPIR_UPLOAD_CHAT_MODEL": alias})
|
||||
|
||||
if _, err := Load(); err == nil {
|
||||
t.Errorf("Load accepted TAPIR_UPLOAD_CHAT_MODEL=%q: a private transcript of up to 90,000 characters would be sent to it", alias)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoad_ALocalUploadChatModelIsAccepted(t *testing.T) {
|
||||
clearUploadEnv(t)
|
||||
setEnv(t, map[string]string{"TAPIR_UPLOAD_CHAT_MODEL": "iguana/gemma4-31b"})
|
||||
|
||||
cfg, err := Load()
|
||||
if err != nil {
|
||||
t.Fatalf("Load: %v", err)
|
||||
}
|
||||
if cfg.UploadChatModel != "iguana/gemma4-31b" {
|
||||
t.Errorf("UploadChatModel = %q", cfg.UploadChatModel)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoad_TheEngineTimeoutDefaultsToTwentyFiveMinutesAndMustBePositive(t *testing.T) {
|
||||
clearUploadEnv(t)
|
||||
cfg, err := Load()
|
||||
if err != nil {
|
||||
t.Fatalf("Load: %v", err)
|
||||
}
|
||||
if cfg.TranscribeEngineTimeout != 25*time.Minute {
|
||||
t.Errorf("TranscribeEngineTimeout = %v, want 25m: a 90-minute recording takes about 8 minutes on kb-whisper", cfg.TranscribeEngineTimeout)
|
||||
}
|
||||
|
||||
setEnv(t, map[string]string{"TAPIR_TRANSCRIBE_ENGINE_TIMEOUT": "7m"})
|
||||
cfg, err = Load()
|
||||
if err != nil || cfg.TranscribeEngineTimeout != 7*time.Minute {
|
||||
t.Errorf("override: %v, %v", cfg.TranscribeEngineTimeout, err)
|
||||
}
|
||||
|
||||
for _, bad := range []string{"0", "-1m", "soon"} {
|
||||
setEnv(t, map[string]string{"TAPIR_TRANSCRIBE_ENGINE_TIMEOUT": bad})
|
||||
if _, err := Load(); err == nil {
|
||||
t.Errorf("Load accepted TAPIR_TRANSCRIBE_ENGINE_TIMEOUT=%q", bad)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -12,7 +12,7 @@ func clearUploadEnv(t *testing.T) {
|
||||
t.Helper()
|
||||
for _, k := range []string{
|
||||
"TAPIR_UPLOADS_ENABLED", "TAPIR_UPLOAD_MAX_MB", "TAPIR_UPLOAD_MAX_MINUTES",
|
||||
"TAPIR_TRANSCRIBE_MODEL", "TAPIR_TRANSCRIBE_FALLBACK_MODEL", "TAPIR_TRANSCRIBE_TIMEOUT",
|
||||
"TAPIR_TRANSCRIBE_MODEL", "TAPIR_TRANSCRIBE_FALLBACK_MODEL", "TAPIR_TRANSCRIBE_TIMEOUT", "TAPIR_TRANSCRIBE_ENGINE_TIMEOUT",
|
||||
"TAPIR_UPLOAD_TMP_DIR", "TAPIR_FFMPEG_BIN", "TAPIR_FFPROBE_BIN",
|
||||
"TAPIR_MAX_UPLOADED_TRANSCRIPT_CHARS", "TAPIR_UPLOAD_CHAT_MODEL",
|
||||
"TAPIR_RECONCILE_BASE_MODEL", "TAPIR_RECONCILE_MODEL", "TAPIR_RECONCILE_CHECK_ORDER",
|
||||
|
||||
@@ -226,6 +226,17 @@ const (
|
||||
UploadErrInterrupted UploadErrorKind = "interrupted"
|
||||
// UploadErrTranscriptionFailed: every configured transcription endpoint failed.
|
||||
UploadErrTranscriptionFailed UploadErrorKind = "transcription_failed"
|
||||
// UploadErrTooLong: the audio, once converted, is longer than the limit. The limit is
|
||||
// first checked against the file's header, which a hostile or damaged file can lie
|
||||
// about (ADR-035 review).
|
||||
UploadErrTooLong UploadErrorKind = "audio_too_long"
|
||||
// UploadErrBusy: too many recordings were already waiting, so this one was not
|
||||
// queued. The user's to retry later (ADR-035 review).
|
||||
UploadErrBusy UploadErrorKind = "queue_full"
|
||||
// UploadErrIncomplete: the audio, once converted, ends far earlier than the file
|
||||
// claimed: a truncated recording that would otherwise give a partial transcript with
|
||||
// no warning.
|
||||
UploadErrIncomplete UploadErrorKind = "audio_incomplete"
|
||||
)
|
||||
|
||||
// UploadedTranscript is a user's PRIVATE transcript of audio they uploaded. It is
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
// Package errkind reduces an error to a short kind that is safe to log, store and show.
|
||||
//
|
||||
// An error's own text can hold a provider's response body (a gateway that echoes the
|
||||
// prompt), a file path (ffmpeg names the spool file), or a word from a private
|
||||
// transcript. None of that may reach a log line, a database column or a page about a
|
||||
// private recording (ADR-033: "typed, never a provider's string"; ADR-035). Of returns
|
||||
// only a fixed vocabulary, so it can never carry any.
|
||||
package errkind
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
)
|
||||
|
||||
// HTTPStatuser is implemented by errors that are a non-2xx answer from a service.
|
||||
type HTTPStatuser interface{ HTTPStatus() int }
|
||||
|
||||
// Of is the kind of err: "timeout", "canceled", "http_<code>", "network", or "error".
|
||||
// It is empty for nil.
|
||||
func Of(err error) string {
|
||||
if err == nil {
|
||||
return ""
|
||||
}
|
||||
var hs HTTPStatuser
|
||||
var ne net.Error
|
||||
switch {
|
||||
case errors.Is(err, context.DeadlineExceeded):
|
||||
return "timeout"
|
||||
case errors.Is(err, context.Canceled):
|
||||
return "canceled"
|
||||
case errors.As(err, &hs):
|
||||
return fmt.Sprintf("http_%d", hs.HTTPStatus())
|
||||
case errors.As(err, &ne):
|
||||
if ne.Timeout() {
|
||||
return "timeout"
|
||||
}
|
||||
return "network"
|
||||
}
|
||||
return "error"
|
||||
}
|
||||
@@ -0,0 +1,64 @@
|
||||
package errkind_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
)
|
||||
|
||||
// Of reduces an error to a short kind that is safe to log, store and show: it never
|
||||
// carries the error's own text, because that text can hold a provider's response body, a
|
||||
// file path or a word from a private transcript (ADR-035 review F3, F5).
|
||||
|
||||
type statusErr struct{ code int }
|
||||
|
||||
func (e statusErr) Error() string {
|
||||
return fmt.Sprintf("gateway said: %d and echoed the transcript", e.code)
|
||||
}
|
||||
func (e statusErr) HTTPStatus() int { return e.code }
|
||||
|
||||
type timeoutNet struct{}
|
||||
|
||||
func (timeoutNet) Error() string { return "i/o timeout to 10.0.0.1" }
|
||||
func (timeoutNet) Timeout() bool { return true }
|
||||
func (timeoutNet) Temporary() bool { return true }
|
||||
|
||||
func TestOf_ReducesAnErrorToAKindAndNeverItsText(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want string
|
||||
}{
|
||||
{"nothing", nil, ""},
|
||||
{"a deadline", context.DeadlineExceeded, "timeout"},
|
||||
{"a wrapped deadline", fmt.Errorf("transcribe: %w", context.DeadlineExceeded), "timeout"},
|
||||
{"a cancellation", fmt.Errorf("job: %w", context.Canceled), "canceled"},
|
||||
{"an HTTP status", statusErr{code: 502}, "http_502"},
|
||||
{"a wrapped HTTP status", fmt.Errorf("model: %w", statusErr{code: 400}), "http_400"},
|
||||
{"a network timeout", timeoutNet{}, "timeout"},
|
||||
{"a network failure", &net.OpError{Op: "dial", Err: errors.New("connection refused")}, "network"},
|
||||
{"a url error wrapping a refusal", &url.Error{Op: "Post", URL: "http://gateway/secret-path", Err: errors.New("refused")}, "network"},
|
||||
{"anything else", errors.New("moov atom not found in /spool/upload-555"), "error"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equal(t, tt.want, errkind.Of(tt.err))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestOf_AKindCarriesNoWordFromTheError(t *testing.T) {
|
||||
err := fmt.Errorf("convert /spool/upload-555: %w", errors.New("Anders ansvarar för virket"))
|
||||
|
||||
got := errkind.Of(err)
|
||||
|
||||
assert.NotContains(t, got, "Anders")
|
||||
assert.NotContains(t, got, "spool")
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"time"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
)
|
||||
|
||||
// Planned is one phase of a job, declared up front so the page can show what is coming.
|
||||
@@ -146,7 +147,7 @@ func (t *Tracker) Fail(key string, err error) {
|
||||
return
|
||||
}
|
||||
t.close(s, domain.StepFailed)
|
||||
t.log.Warn("step failed", "step", key, "elapsed", s.FinishedAt.Sub(s.StartedAt), "err", err)
|
||||
t.log.Warn("step failed", "step", key, "elapsed", s.FinishedAt.Sub(s.StartedAt), "kind", errkind.Of(err))
|
||||
t.save()
|
||||
}
|
||||
|
||||
|
||||
@@ -197,7 +197,7 @@ func TestTracker_FailRunningMarksTheRunningStepFailedAndLeavesTheRestPending(t *
|
||||
require.Equal(t, domain.StepPending, stateOf(steps, "save"), "what never started stays pending: it shows where the job stopped")
|
||||
}
|
||||
|
||||
func TestTracker_AKeyedFailMarksOneStepFailedLogsWhyAndTheJobGoesOn(t *testing.T) {
|
||||
func TestTracker_AKeyedFailMarksOneStepFailedLogsItsKindAndTheJobGoesOn(t *testing.T) {
|
||||
tr, sink, _, buf := newTracker(t)
|
||||
tr.Start()
|
||||
tr.Begin("audio", 0)
|
||||
@@ -209,7 +209,8 @@ func TestTracker_AKeyedFailMarksOneStepFailedLogsWhyAndTheJobGoesOn(t *testing.T
|
||||
require.Equal(t, domain.StepFailed, stateOf(steps, "audio"))
|
||||
require.Equal(t, domain.StepRunning, stateOf(steps, "decide"), "a soft failure must not stop the next step from running")
|
||||
require.Contains(t, buf.String(), "level=WARN")
|
||||
require.Contains(t, buf.String(), "gateway said no")
|
||||
require.NotContains(t, buf.String(), "gateway said no", "an error's text can carry provider output or a path: the log gets its kind")
|
||||
require.Contains(t, buf.String(), "kind=error")
|
||||
require.Contains(t, buf.String(), "step=audio")
|
||||
}
|
||||
|
||||
@@ -259,12 +260,56 @@ func TestTracker_EveryTransitionIsLoggedByTheSameCall(t *testing.T) {
|
||||
tr.Begin("audio", 0)
|
||||
clk.tick(3 * time.Second)
|
||||
tr.Done("audio")
|
||||
tr.Begin("decide", 5)
|
||||
clk.tick(2 * time.Second)
|
||||
tr.FailRunning()
|
||||
tr.Finish()
|
||||
|
||||
out := buf.String()
|
||||
require.Contains(t, out, "step=audio")
|
||||
require.Contains(t, out, "started")
|
||||
require.Contains(t, out, "finished")
|
||||
require.Contains(t, out, "elapsed=3s")
|
||||
for _, want := range []string{
|
||||
`msg="job started"`,
|
||||
`msg="step started" step=audio`,
|
||||
`msg="step finished" step=audio elapsed=3s`,
|
||||
`msg="step started" step=decide total=5`,
|
||||
`msg="step failed" step=decide elapsed=2s`,
|
||||
`msg="job finished"`,
|
||||
} {
|
||||
require.Contains(t, out, want, "a transition with no log line is invisible to the operator")
|
||||
}
|
||||
}
|
||||
|
||||
func TestTracker_AKeyedFailIsSavedAtOnceNotOnTheNextTransition(t *testing.T) {
|
||||
tr, sink, _, _ := newTracker(t)
|
||||
tr.Start()
|
||||
tr.Begin("audio", 0)
|
||||
before := sink.saveCount()
|
||||
|
||||
tr.Fail("audio", errors.New("down"))
|
||||
|
||||
require.Equal(t, before+1, sink.saveCount(), "a soft-failed phase must show as failed on the page now")
|
||||
require.Equal(t, domain.StepFailed, stateOf(sink.last(t), "audio"))
|
||||
}
|
||||
|
||||
// retainingSink keeps the very slice it is handed, like a sink that queues it for later.
|
||||
type retainingSink struct{ got [][]domain.ProgressStep }
|
||||
|
||||
func (r *retainingSink) SaveProgress(_ context.Context, steps []domain.ProgressStep, _ time.Time) error {
|
||||
r.got = append(r.got, steps)
|
||||
return nil
|
||||
}
|
||||
func (r *retainingSink) Beat(context.Context) error { return nil }
|
||||
|
||||
func TestTracker_ASnapshotHandedToTheSinkIsNotChangedByLaterTransitions(t *testing.T) {
|
||||
sink := &retainingSink{}
|
||||
tr := progress.NewTracker(context.Background(), sink, slog.New(slog.NewTextHandler(&syncBuf{}, nil)), plan(),
|
||||
progress.WithClock(newClock().now))
|
||||
tr.Start()
|
||||
first := sink.got[0]
|
||||
|
||||
tr.Begin("audio", 0)
|
||||
tr.Done("audio")
|
||||
|
||||
require.Equal(t, domain.StepPending, first[0].State, "a sink that keeps a snapshot must not see it change under it")
|
||||
}
|
||||
|
||||
func TestTracker_AdvanceIsLoggedAtMostOncePerMinute(t *testing.T) {
|
||||
|
||||
@@ -32,7 +32,7 @@ func (f *fakeCompleter) Complete(_ context.Context, system, user string) (string
|
||||
return f.replies[i], nil
|
||||
}
|
||||
|
||||
var req = reconcile.Request{Before: "kunderna ska då", After: "De ska kunna betala", Base: "onboardade", Alt: "omvårdade"}
|
||||
var req = reconcile.Request{Before: "grannarna ska då", After: "De ska kunna måla", Base: "målade", Alt: "mölade"}
|
||||
|
||||
func TestLLMAdjudicator_SendsTheMeasuredPromptWithTheReadingsInTheRequestedOrder(t *testing.T) {
|
||||
fc := &fakeCompleter{replies: []string{"ANSWER: 1"}}
|
||||
@@ -45,10 +45,10 @@ func TestLLMAdjudicator_SendsTheMeasuredPromptWithTheReadingsInTheRequestedOrder
|
||||
assert.Contains(t, fc.systems[0], "two automatic transcriptions of the same Swedish speech")
|
||||
assert.Contains(t, fc.systems[0], "ANSWER: 1")
|
||||
u := fc.users[0]
|
||||
assert.Contains(t, u, "Text before the spot:\nkunderna ska då")
|
||||
assert.Contains(t, u, "Text before the spot:\ngrannarna ska då")
|
||||
assert.Contains(t, u, "[[ THE SPOT ]]")
|
||||
assert.Contains(t, u, "Text after the spot:\nDe ska kunna betala")
|
||||
assert.Contains(t, u, "Reading 1: onboardade\nReading 2: omvårdade", "base first when AltFirst is false")
|
||||
assert.Contains(t, u, "Text after the spot:\nDe ska kunna måla")
|
||||
assert.Contains(t, u, "Reading 1: målade\nReading 2: mölade", "base first when AltFirst is false")
|
||||
}
|
||||
|
||||
func TestLLMAdjudicator_AltFirstSwapsWhichReadingIsNumberOne(t *testing.T) {
|
||||
@@ -59,7 +59,7 @@ func TestLLMAdjudicator_AltFirstSwapsWhichReadingIsNumberOne(t *testing.T) {
|
||||
_, err := reconcile.NewLLMAdjudicator(fc).Adjudicate(context.Background(), r)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Contains(t, fc.users[0], "Reading 1: omvårdade\nReading 2: onboardade")
|
||||
assert.Contains(t, fc.users[0], "Reading 1: mölade\nReading 2: målade")
|
||||
}
|
||||
|
||||
func TestLLMAdjudicator_MapsTheAnswerBackToBaseOrAltWhicheverOrderWasShown(t *testing.T) {
|
||||
@@ -134,8 +134,8 @@ func TestOrderChecked_AsksTwiceWithTheOrderSwappedAndKeepsOnlyAgreement(t *testi
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tt.want, got)
|
||||
require.Len(t, fc.users, 2)
|
||||
assert.True(t, strings.Contains(fc.users[0], "Reading 1: onboardade"), "first call shows the base first")
|
||||
assert.True(t, strings.Contains(fc.users[1], "Reading 1: omvårdade"), "second call shows the alternative first")
|
||||
assert.True(t, strings.Contains(fc.users[0], "Reading 1: målade"), "first call shows the base first")
|
||||
assert.True(t, strings.Contains(fc.users[1], "Reading 1: mölade"), "second call shows the alternative first")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,12 +19,12 @@ func TestFindDisputes_NoneWhenTheTextsAgreeWordForWord(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestFindDisputes_OneReplacedWordIsOneDisputeCarryingBothReadings(t *testing.T) {
|
||||
got := disputes("Vi ska onboardade kunderna.", "Vi ska omvårdade kunderna.")
|
||||
got := disputes("Vi ska målade grannarna.", "Vi ska mölade grannarna.")
|
||||
|
||||
require.Len(t, got, 1)
|
||||
d := got[0]
|
||||
assert.Equal(t, "onboardade", d.Base)
|
||||
assert.Equal(t, "omvårdade", d.Alt)
|
||||
assert.Equal(t, "målade", d.Base)
|
||||
assert.Equal(t, "mölade", d.Alt)
|
||||
assert.Equal(t, 2, d.BaseStart)
|
||||
assert.Equal(t, 3, d.BaseEnd)
|
||||
assert.Equal(t, 2, d.AltStart)
|
||||
@@ -32,11 +32,11 @@ func TestFindDisputes_OneReplacedWordIsOneDisputeCarryingBothReadings(t *testing
|
||||
}
|
||||
|
||||
func TestFindDisputes_ReadingsKeepTheirOwnSpellingAndPunctuation(t *testing.T) {
|
||||
got := disputes("Det blir T+1, alltså.", "Det blir T 1, alltså.")
|
||||
got := disputes("Det blir A+2, alltså.", "Det blir A 2, alltså.")
|
||||
|
||||
require.Len(t, got, 1)
|
||||
assert.Equal(t, "T+1,", got[0].Base)
|
||||
assert.Equal(t, "T 1,", got[0].Alt, "the alternative's words replace the span as the engine wrote them")
|
||||
assert.Equal(t, "A+2,", got[0].Base)
|
||||
assert.Equal(t, "A 2,", got[0].Alt, "the alternative's words replace the span as the engine wrote them")
|
||||
}
|
||||
|
||||
func TestFindDisputes_AWordOnlyOneEngineHeardHasAnEmptyOtherSide(t *testing.T) {
|
||||
@@ -90,12 +90,12 @@ func TestAutoResolve(t *testing.T) {
|
||||
base, alt string
|
||||
resolved bool
|
||||
}{
|
||||
{"a compound written as two words or one", "i crossborder delen", "i crossborderdelen", true},
|
||||
{"a compound with a hyphen", "crossborder-delen", "crossborder delen", true},
|
||||
{"a compound written as two words or one", "i takrenns delen", "i takrennsdelen", true},
|
||||
{"a compound with a hyphen", "takrenns-delen", "takrenns delen", true},
|
||||
{"a filler only the alternative heard", "vi ska ju betala", "vi ska betala", true},
|
||||
{"a filler only the base heard", "vi ska betala", "vi ska ju betala", true},
|
||||
{"two fillers in a row", "vi ska ja ju betala", "vi ska betala", true},
|
||||
{"a different word is for the model", "vi ska onboardade", "vi ska omvårdade", false},
|
||||
{"a different word is for the model", "vi ska målade", "vi ska mölade", false},
|
||||
{"a different number is for the model", "totalt 1050 kronor", "totalt 2050 kronor", false},
|
||||
{"a content word only one side heard is for the model", "ett förslag", "ett coolt förslag", false},
|
||||
{"three filler words are too many to wave through", "vi ska ju då liksom betala", "vi ska betala", false},
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
"git.d-ma.be/mathias/tapir/internal/progress"
|
||||
)
|
||||
|
||||
@@ -101,9 +102,8 @@ func Reconcile(ctx context.Context, base, alt domain.Transcription, adj Adjudica
|
||||
rec.Errors++
|
||||
consecutive++
|
||||
if consecutive >= breakAfter {
|
||||
failure := fmt.Errorf("the model failed %d times in a row: %w", consecutive, err)
|
||||
rep.Fail("decide", failure)
|
||||
return skipped(base, rec, failure.Error()), nil
|
||||
rep.Fail("decide", err)
|
||||
return skipped(base, rec, fmt.Sprintf("the model failed %d times in a row (%s)", consecutive, errkind.Of(err))), nil
|
||||
}
|
||||
v = VerdictUnsure
|
||||
} else {
|
||||
|
||||
@@ -48,8 +48,8 @@ func reconcileWith(t *testing.T, ctx context.Context, base, alt string, adj reco
|
||||
|
||||
// two compound/filler disputes the model is never asked about, then two it is.
|
||||
const (
|
||||
progBase = "i crossborder delen ska vi ju betala totalt 1015 kronor till SEB idag"
|
||||
progAlt = "i crossborderdelen ska vi betala totalt 1050 kronor till SCB idag"
|
||||
progBase = "i takrenns delen ska vi ju betala totalt 1015 kronor till Axel idag"
|
||||
progAlt = "i takrennsdelen ska vi betala totalt 1050 kronor till Alex idag"
|
||||
)
|
||||
|
||||
func TestReconcileProgress_TheDecidingPhaseTotalIsTheModelDecisionsNotTheDisputes(t *testing.T) {
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
package reconcile_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/reconcile"
|
||||
)
|
||||
|
||||
// The reason a reconciliation was skipped is stored and shown on a page about a private
|
||||
// recording, so it carries a kind, never the model's error text, which can echo the
|
||||
// prompt (adversarial review F3).
|
||||
|
||||
func TestReconcile_TheBreakerReasonCarriesAKindNotTheModelsText(t *testing.T) {
|
||||
adj := &scripted{fn: func(int, reconcile.Request) (reconcile.Verdict, error) {
|
||||
return reconcile.VerdictBase, errors.New("gateway echoed the prompt: zebrakaka secret words")
|
||||
}}
|
||||
small := opts
|
||||
small.BreakAfter = 2
|
||||
|
||||
out, err := reconcile.Reconcile(context.Background(), tr(progBase), tr(progAlt), adj, small)
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, domain.ReconcileSkipped, out.Reconciliation.Status)
|
||||
assert.NotContains(t, out.Reconciliation.Reason, "zebrakaka", "provider text must not be stored or shown")
|
||||
assert.Contains(t, out.Reconciliation.Reason, "2 times in a row", "the reason still says what happened")
|
||||
assert.Contains(t, out.Reconciliation.Reason, "error", "and carries a kind")
|
||||
}
|
||||
@@ -31,13 +31,13 @@ func TestReconcileSpots_AKeptSpotAndAChangedSpotAreBothStoredWithTheirVerdict(t
|
||||
}
|
||||
|
||||
func TestReconcileSpots_AutoResolvedDisputesAreNotStored(t *testing.T) {
|
||||
out := merge(t, tr("i crossborder delen ska vi ju betala"), tr("i crossborderdelen ska vi betala"), always(reconcile.VerdictAlt))
|
||||
out := merge(t, tr("i takrenns delen ska vi ju betala"), tr("i takrennsdelen ska vi betala"), always(reconcile.VerdictAlt))
|
||||
|
||||
assert.Empty(t, out.Reconciliation.Contested, "compounds and fillers were never judged; they are counted, not listed")
|
||||
}
|
||||
|
||||
func TestReconcileSpots_ASpotCarriesTheWordsJustBeforeIt(t *testing.T) {
|
||||
out := merge(t, tr("vi skickar in ett special meddelande till taxsystemet"), tr("vi skickar in ett CAMT meddelande till taxsystemet"), always(reconcile.VerdictAlt))
|
||||
out := merge(t, tr("vi skickar in ett vanligt meddelande till kassasystemet"), tr("vi skickar in ett KORT meddelande till kassasystemet"), always(reconcile.VerdictAlt))
|
||||
|
||||
require.Len(t, out.Reconciliation.Contested, 1)
|
||||
assert.Equal(t, "vi skickar in ett", out.Reconciliation.Contested[0].Before)
|
||||
|
||||
@@ -58,10 +58,10 @@ func TestReconcile_IdenticalTextsNeedNoModelAndComeBackUnchanged(t *testing.T) {
|
||||
func TestReconcile_TrivialDisputesAreResolvedWithoutAskingTheModel(t *testing.T) {
|
||||
adj := always(reconcile.VerdictAlt)
|
||||
|
||||
out := merge(t, tr("i crossborder delen ska vi ju betala"), tr("i crossborderdelen ska vi betala"), adj)
|
||||
out := merge(t, tr("i takrenns delen ska vi ju betala"), tr("i takrennsdelen ska vi betala"), adj)
|
||||
|
||||
assert.Empty(t, adj.calls, "a compound split and a filler particle are not worth a model call")
|
||||
assert.Equal(t, "i crossborder delen ska vi ju betala", out.Text, "the base reading is kept")
|
||||
assert.Equal(t, "i takrenns delen ska vi ju betala", out.Text, "the base reading is kept")
|
||||
assert.Equal(t, 2, out.Reconciliation.Disputes)
|
||||
assert.Equal(t, 2, out.Reconciliation.AutoResolved)
|
||||
assert.Zero(t, out.Reconciliation.Adjudicated)
|
||||
@@ -76,13 +76,13 @@ func TestReconcile_AltVerdictReplacesTheSpanWithTheAlternativesOwnWords(t *testi
|
||||
}
|
||||
|
||||
func TestReconcile_BaseAndUnsureVerdictsKeepTheBaseButOnlyUnsureIsFlagged(t *testing.T) {
|
||||
base := tr("Totalt 1015 kronor till SEB.")
|
||||
alt := tr("Totalt 1050 kronor till SCB.")
|
||||
base := tr("Totalt 1015 kronor till Axel.")
|
||||
alt := tr("Totalt 1050 kronor till Alex.")
|
||||
|
||||
keptBase := merge(t, base, alt, always(reconcile.VerdictBase))
|
||||
unsure := merge(t, base, alt, always(reconcile.VerdictUnsure))
|
||||
|
||||
assert.Equal(t, "Totalt 1015 kronor till SEB.", keptBase.Text)
|
||||
assert.Equal(t, "Totalt 1015 kronor till Axel.", keptBase.Text)
|
||||
assert.Zero(t, keptBase.Reconciliation.Unsure)
|
||||
require.Len(t, keptBase.Reconciliation.Contested, 2,
|
||||
"every judged spot is stored, kept ones too: without them the reconciler cannot be compared with either engine alone (ADR-035)")
|
||||
@@ -90,7 +90,7 @@ func TestReconcile_BaseAndUnsureVerdictsKeepTheBaseButOnlyUnsureIsFlagged(t *tes
|
||||
assert.Equal(t, "base", c.Used)
|
||||
assert.False(t, c.Unsure, "a confident 'the base is right' is stored but is not an uncertain spot")
|
||||
}
|
||||
assert.Equal(t, "Totalt 1015 kronor till SEB.", unsure.Text, "unsure keeps the base")
|
||||
assert.Equal(t, "Totalt 1015 kronor till Axel.", unsure.Text, "unsure keeps the base")
|
||||
assert.Equal(t, 2, unsure.Reconciliation.Unsure)
|
||||
require.Len(t, unsure.Reconciliation.Contested, 2)
|
||||
assert.True(t, unsure.Reconciliation.Contested[0].Unsure)
|
||||
@@ -98,12 +98,12 @@ func TestReconcile_BaseAndUnsureVerdictsKeepTheBaseButOnlyUnsureIsFlagged(t *tes
|
||||
}
|
||||
|
||||
func TestReconcile_ChangedSpotsAreRecordedWithBothReadings(t *testing.T) {
|
||||
out := merge(t, tr("det blir T+1 imorgon"), tr("det blir en T imorgon"), always(reconcile.VerdictAlt))
|
||||
out := merge(t, tr("det blir A+2 imorgon"), tr("det blir en A imorgon"), always(reconcile.VerdictAlt))
|
||||
|
||||
require.Len(t, out.Reconciliation.Contested, 1)
|
||||
c := out.Reconciliation.Contested[0]
|
||||
assert.Equal(t, "T+1", c.Base)
|
||||
assert.Equal(t, "en T", c.Alt)
|
||||
assert.Equal(t, "A+2", c.Base)
|
||||
assert.Equal(t, "en A", c.Alt)
|
||||
assert.Equal(t, "alt", c.Used)
|
||||
assert.False(t, c.Unsure)
|
||||
}
|
||||
@@ -122,16 +122,16 @@ func TestReconcile_AWordOnlyTheBaseHeardCanBeRemoved(t *testing.T) {
|
||||
|
||||
func TestReconcile_SeveralDisputesAreAppliedIndependentlyAndInOrder(t *testing.T) {
|
||||
adj := &scripted{fn: func(i int, r reconcile.Request) (reconcile.Verdict, error) {
|
||||
if r.Alt == "omvårdade" || r.Alt == "SCB" {
|
||||
if r.Alt == "mölade" || r.Alt == "Alex" {
|
||||
return reconcile.VerdictAlt, nil
|
||||
}
|
||||
return reconcile.VerdictBase, nil
|
||||
}}
|
||||
|
||||
out := merge(t, tr("kunderna onboardade nu och sen betalas till SEB via ombud med 1050 kronor"),
|
||||
tr("kunderna omvårdade nu och sen betalas till SCB via ombud med 2050 kronor"), adj)
|
||||
out := merge(t, tr("kunderna målade nu och sen betalas till Axel via ombud med 1050 kronor"),
|
||||
tr("kunderna mölade nu och sen betalas till Alex via ombud med 2050 kronor"), adj)
|
||||
|
||||
assert.Equal(t, "kunderna omvårdade nu och sen betalas till SCB via ombud med 1050 kronor", out.Text)
|
||||
assert.Equal(t, "kunderna mölade nu och sen betalas till Alex via ombud med 1050 kronor", out.Text)
|
||||
assert.Equal(t, 3, out.Reconciliation.Disputes)
|
||||
assert.Equal(t, 2, out.Reconciliation.Changed)
|
||||
}
|
||||
@@ -193,7 +193,8 @@ func TestReconcile_ConsecutiveModelErrorsStopTheWholeReconciliationInsteadOfWait
|
||||
assert.Len(t, adj.calls, 3, "three consecutive failures and it stops asking")
|
||||
assert.Equal(t, strings.Join(a, " "), out.Text, "the base text, with none of the partial changes")
|
||||
assert.Equal(t, domain.ReconcileSkipped, out.Reconciliation.Status)
|
||||
assert.Contains(t, out.Reconciliation.Reason, "gateway down")
|
||||
assert.Contains(t, out.Reconciliation.Reason, "3 times in a row")
|
||||
assert.NotContains(t, out.Reconciliation.Reason, "gateway down", "the reason is shown on the page: a kind, not the model's own text")
|
||||
}
|
||||
|
||||
func TestReconcile_ASuccessBetweenFailuresResetsTheBreaker(t *testing.T) {
|
||||
|
||||
@@ -34,9 +34,9 @@ func TestTokenize_KeepsTheWordAsWrittenAndKeysItWithoutCaseOrPunctuation(t *test
|
||||
}
|
||||
|
||||
func TestTokenize_KeysIgnoreHyphensAndPlusSoOnlyWordsDecideEquality(t *testing.T) {
|
||||
got := reconcile.Tokenize("end-to-end T+1 5 000 kr")
|
||||
got := reconcile.Tokenize("end-to-end A+2 5 000 kr")
|
||||
|
||||
assert.Equal(t, []string{"endtoend", "t1", "5", "000", "kr"}, keys(got))
|
||||
assert.Equal(t, []string{"endtoend", "a2", "5", "000", "kr"}, keys(got))
|
||||
}
|
||||
|
||||
func TestTokenize_APunctuationOnlyWordJoinsThePreviousWordInsteadOfBecomingOne(t *testing.T) {
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"time"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
)
|
||||
@@ -33,6 +34,9 @@ type Job struct {
|
||||
UserID string
|
||||
UploadID string
|
||||
Path string
|
||||
// Duration is what the file CLAIMED when probed. The worker compares it with what
|
||||
// was really converted; zero means unknown (ADR-035 review).
|
||||
Duration time.Duration
|
||||
}
|
||||
|
||||
// Submitter hands accepted jobs to the in-process worker (ADR-033).
|
||||
@@ -103,7 +107,15 @@ func (in *Intake) Accept(ctx context.Context, userID, title string, src io.Reade
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("uploads: create: %w", err)
|
||||
}
|
||||
if err := in.sub.Submit(Job{UserID: userID, UploadID: id, Path: path}); err != nil {
|
||||
if err := in.sub.Submit(Job{UserID: userID, UploadID: id, Path: path, Duration: probe.Duration}); err != nil {
|
||||
if errors.Is(err, ErrQueueFull) {
|
||||
// The user's to retry, not a server fault: refuse it as such, and do not leave
|
||||
// the row queued -- that is the stuck spinner.
|
||||
if ferr := in.store.FailUpload(ctx, userID, id, domain.UploadErrBusy); ferr != nil {
|
||||
return "", fmt.Errorf("uploads: submit: %w (and failing the row: %v)", err, ferr)
|
||||
}
|
||||
return "", &RejectError{Reason: ReasonBusy, Message: "Too many recordings are waiting to be transcribed. Please try again in a few minutes."}
|
||||
}
|
||||
// A row that will never run must not sit queued: that is the stuck spinner.
|
||||
if ferr := in.store.FailUpload(ctx, userID, id, domain.UploadErrInterrupted); ferr != nil {
|
||||
return "", fmt.Errorf("uploads: submit: %w (and failing the row: %v)", err, ferr)
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
package uploads_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||
)
|
||||
|
||||
// A full queue is the user's to retry, not a server fault: a typed rejection, the row
|
||||
// failed honestly, the audio removed (adversarial review F6).
|
||||
|
||||
func TestAccept_AFullQueueIsABusyRejectionNotAServerErrorAndLeavesNothingBehind(t *testing.T) {
|
||||
r := newRig(t)
|
||||
r.sub.err = uploads.ErrQueueFull
|
||||
|
||||
_, err := r.in.Accept(context.Background(), "user-1", "t", strings.NewReader("data"))
|
||||
|
||||
var rej *uploads.RejectError
|
||||
require.True(t, errors.As(err, &rej), "a full queue must be a rejection the web layer can answer with 429: %v", err)
|
||||
assert.Equal(t, uploads.ReasonBusy, rej.Reason)
|
||||
assert.Contains(t, rej.Message, "waiting")
|
||||
assert.Equal(t, domain.UploadErrBusy, r.st.failed["up-1"], "the row must not sit queued: that is the stuck spinner")
|
||||
assert.Empty(t, r.tmpFiles(t), "the audio is deleted")
|
||||
}
|
||||
@@ -106,7 +106,8 @@ func TestAccept_ValidAudioIsStoredProbedQueuedAndSubmitted(t *testing.T) {
|
||||
assert.Equal(t, audio, r.pr.seen, "the prober must see the complete uploaded bytes, on disk")
|
||||
require.Len(t, r.sub.jobs, 1)
|
||||
job := r.sub.jobs[0]
|
||||
assert.Equal(t, uploads.Job{UserID: "user-1", UploadID: "up-1", Path: job.Path}, job)
|
||||
assert.Equal(t, uploads.Job{UserID: "user-1", UploadID: "up-1", Path: job.Path, Duration: 10 * time.Minute}, job,
|
||||
"the job carries what the file CLAIMED, so the worker can compare it with what was converted")
|
||||
assert.Equal(t, filepath.Dir(job.Path), r.tmp, "the temp file lives in the configured directory")
|
||||
got, err := os.ReadFile(job.Path)
|
||||
require.NoError(t, err)
|
||||
|
||||
@@ -34,6 +34,8 @@ const (
|
||||
ReasonTooLong Reason = "too_long"
|
||||
ReasonTooLarge Reason = "too_large"
|
||||
ReasonUnreadable Reason = "unreadable"
|
||||
// ReasonBusy: the queue is full. Not about the file: the user retries later.
|
||||
ReasonBusy Reason = "busy"
|
||||
)
|
||||
|
||||
// RejectError is a synchronous, user-presentable refusal. Message is written for
|
||||
|
||||
+82
-11
@@ -10,12 +10,18 @@ import (
|
||||
"time"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
"git.d-ma.be/mathias/tapir/internal/progress"
|
||||
)
|
||||
|
||||
// ErrStopped is returned by Submit once the worker has stopped.
|
||||
var ErrStopped = errors.New("uploads: worker stopped")
|
||||
|
||||
// ErrQueueFull is returned by Submit when the queue, or one user's share of it, is at
|
||||
// its cap. Every queued job holds up to the upload cap on disk until its turn, so an
|
||||
// unbounded queue is a disk-filling lever (ADR-035 review).
|
||||
var ErrQueueFull = errors.New("uploads: queue full")
|
||||
|
||||
// Converter turns the uploaded file into the 16 kHz mono PCM WAV whisper wants.
|
||||
type Converter interface {
|
||||
ToWAV(ctx context.Context, src, dst string) error
|
||||
@@ -53,10 +59,24 @@ func WithHeartbeat(beat, still time.Duration) WorkerOption {
|
||||
return func(w *Worker) { w.beatEvery, w.stillEvery = beat, still }
|
||||
}
|
||||
|
||||
// WithQueueLimits bounds how many jobs may WAIT: in total, and per user. A job being
|
||||
// processed is not waiting. The defaults are 20 and 3 (one instance, one job at a time).
|
||||
func WithQueueLimits(total, perUser int) WorkerOption {
|
||||
return func(w *Worker) { w.maxQueued, w.maxPerUser = total, perUser }
|
||||
}
|
||||
|
||||
// WithMaxDuration makes the worker measure the converted audio and fail a job whose real
|
||||
// length exceeds the limit, or falls far short of what the file claimed. Zero (the
|
||||
// default) measures nothing. The limit is first checked against the file's header, which
|
||||
// can lie (ADR-035 review).
|
||||
func WithMaxDuration(d time.Duration) WorkerOption { return func(w *Worker) { w.maxDuration = d } }
|
||||
|
||||
// WithLogger replaces slog.Default() as the worker's logger.
|
||||
func WithLogger(l *slog.Logger) WorkerOption { return func(w *Worker) { w.log = l } }
|
||||
|
||||
const (
|
||||
defaultMaxQueued = 20
|
||||
defaultMaxPerUser = 3
|
||||
defaultBeatEvery = 10 * time.Second
|
||||
defaultStillEvery = 5 * time.Minute
|
||||
)
|
||||
@@ -87,6 +107,8 @@ type Worker struct {
|
||||
|
||||
log *slog.Logger
|
||||
beatEvery, stillEvery time.Duration
|
||||
maxDuration time.Duration
|
||||
maxQueued, maxPerUser int
|
||||
|
||||
mu sync.Mutex
|
||||
queue []Job
|
||||
@@ -100,6 +122,7 @@ func NewWorker(s WorkerStore, c Converter, t Transcriber, tmpDir string, timeout
|
||||
w := &Worker{
|
||||
store: s, conv: c, tr: t, tmpDir: tmpDir, timeout: timeout, wake: make(chan struct{}, 1),
|
||||
beatEvery: defaultBeatEvery, stillEvery: defaultStillEvery,
|
||||
maxQueued: defaultMaxQueued, maxPerUser: defaultMaxPerUser,
|
||||
}
|
||||
for _, o := range opts {
|
||||
o(w)
|
||||
@@ -114,6 +137,18 @@ func (w *Worker) Submit(j Job) error {
|
||||
if w.stopped {
|
||||
return ErrStopped
|
||||
}
|
||||
if len(w.queue) >= w.maxQueued {
|
||||
return ErrQueueFull
|
||||
}
|
||||
mine := 0
|
||||
for _, q := range w.queue {
|
||||
if q.UserID == j.UserID {
|
||||
mine++
|
||||
}
|
||||
}
|
||||
if mine >= w.maxPerUser {
|
||||
return ErrQueueFull
|
||||
}
|
||||
w.queue = append(w.queue, j)
|
||||
select {
|
||||
case w.wake <- struct{}{}:
|
||||
@@ -190,7 +225,7 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
|
||||
if err := w.store.StartTranscribing(ctx, job.UserID, job.UploadID); err != nil {
|
||||
log.Error("upload: start transcribing", "err", err)
|
||||
w.fail(ctx, job, log)
|
||||
w.fail(ctx, job, log, domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -217,17 +252,17 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
|
||||
// abort records where the job stopped. A shutdown (the parent context ended) is not
|
||||
// a failure: the row is left for the start-up sweep, so the step stays as it was.
|
||||
abort := func() {
|
||||
abort := func(kind domain.UploadErrorKind) {
|
||||
if ctx.Err() == nil {
|
||||
tr.FailRunning()
|
||||
}
|
||||
w.fail(ctx, job, log)
|
||||
w.fail(ctx, job, log, kind)
|
||||
}
|
||||
|
||||
wav, err := w.tempWAV()
|
||||
if err != nil {
|
||||
log.Error("upload: temp wav", "err", err)
|
||||
abort()
|
||||
abort(domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
defer func() { _ = os.Remove(wav) }()
|
||||
@@ -236,8 +271,13 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
defer cancel()
|
||||
tr.Begin("audio", 0)
|
||||
if err := w.conv.ToWAV(jobCtx, job.Path, wav); err != nil {
|
||||
log.Error("upload: convert", "err", err)
|
||||
abort()
|
||||
log.Error("upload: convert", "kind", errkind.Of(err))
|
||||
abort(domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
if kind, bad := w.audioLengthProblem(wav, job.Duration); bad {
|
||||
abort(kind)
|
||||
log.Error("upload: converted audio length rejected", "kind", string(kind))
|
||||
return
|
||||
}
|
||||
tr.Done("audio")
|
||||
@@ -247,8 +287,8 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
}
|
||||
t, err := w.tr.Transcribe(progress.With(jobCtx, tr), wav)
|
||||
if err != nil {
|
||||
log.Error("upload: transcribe", "err", err, "elapsed", time.Since(start).Round(time.Second))
|
||||
abort()
|
||||
log.Error("upload: transcribe", "kind", errkind.Of(err), "elapsed", time.Since(start).Round(time.Second))
|
||||
abort(domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
if !planned {
|
||||
@@ -258,7 +298,7 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
tr.Begin("save", 0)
|
||||
if err := w.store.CompleteUpload(ctx, job.UserID, job.UploadID, t); err != nil {
|
||||
log.Error("upload: store transcript", "err", err)
|
||||
abort()
|
||||
abort(domain.UploadErrTranscriptionFailed)
|
||||
return
|
||||
}
|
||||
tr.Done("save")
|
||||
@@ -269,11 +309,11 @@ func (w *Worker) Process(ctx context.Context, job Job) {
|
||||
|
||||
// fail records a transcription failure — unless the PARENT context ended, which
|
||||
// is a shutdown, not a failure: the row is left for the start-up sweep.
|
||||
func (w *Worker) fail(ctx context.Context, job Job, log *slog.Logger) {
|
||||
func (w *Worker) fail(ctx context.Context, job Job, log *slog.Logger, kind domain.UploadErrorKind) {
|
||||
if ctx.Err() != nil {
|
||||
return
|
||||
}
|
||||
if err := w.store.FailUpload(ctx, job.UserID, job.UploadID, domain.UploadErrTranscriptionFailed); err != nil {
|
||||
if err := w.store.FailUpload(ctx, job.UserID, job.UploadID, kind); err != nil {
|
||||
log.Error("upload: record failure", "err", err)
|
||||
}
|
||||
}
|
||||
@@ -290,3 +330,34 @@ func (w *Worker) tempWAV() (string, error) {
|
||||
}
|
||||
return name, nil
|
||||
}
|
||||
|
||||
const (
|
||||
// tooLongSlack is how far over the limit the converted audio may be before the claim
|
||||
// in the file's header is called a lie: ffmpeg rounds.
|
||||
tooLongSlack = 30 * time.Second
|
||||
// A file counts as truncated when it converts to under 80% of the length it claimed
|
||||
// AND more than two minutes short. Probe estimates for variable-bitrate files are
|
||||
// imprecise; refusing real recordings would be worse than the problem.
|
||||
incompleteBelow = 0.8
|
||||
incompleteShortfall = 2 * time.Minute
|
||||
)
|
||||
|
||||
// audioLengthProblem measures the converted WAV (16 kHz mono 16-bit PCM is 32,000 bytes a
|
||||
// second after a 44-byte header) against the limit and against what the file claimed.
|
||||
func (w *Worker) audioLengthProblem(wav string, claimed time.Duration) (domain.UploadErrorKind, bool) {
|
||||
if w.maxDuration <= 0 {
|
||||
return "", false
|
||||
}
|
||||
info, err := os.Stat(wav)
|
||||
if err != nil {
|
||||
return "", false
|
||||
}
|
||||
got := time.Duration(float64(max(info.Size()-44, 0)) / 32000 * float64(time.Second))
|
||||
switch {
|
||||
case got > w.maxDuration+tooLongSlack:
|
||||
return domain.UploadErrTooLong, true
|
||||
case float64(got) < incompleteBelow*float64(claimed) && claimed-got > incompleteShortfall:
|
||||
return domain.UploadErrIncomplete, true
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
package uploads_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||
)
|
||||
|
||||
// The duration limit is a claim in the file's header. The worker measures what was
|
||||
// really converted and refuses both a recording that is longer than the limit (the
|
||||
// header lied low) and one that ends far earlier than the file claimed (it was truncated
|
||||
// and would otherwise yield a partial transcript with no warning) -- adversarial review F2.
|
||||
|
||||
// sizedConverter writes a (sparse) 16 kHz mono PCM WAV of the given length, without
|
||||
// actually allocating it: 32,000 bytes per second plus a 44-byte header.
|
||||
type sizedConverter struct{ seconds float64 }
|
||||
|
||||
func (c sizedConverter) ToWAV(_ context.Context, _, dst string) error {
|
||||
f, err := os.Create(dst)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer func() { _ = f.Close() }()
|
||||
return f.Truncate(44 + int64(c.seconds*32000))
|
||||
}
|
||||
|
||||
func durationWorker(t *testing.T, r *wrig, wavSeconds float64, max time.Duration) *uploads.Worker {
|
||||
t.Helper()
|
||||
opts := []uploads.WorkerOption{}
|
||||
if max > 0 {
|
||||
opts = append(opts, uploads.WithMaxDuration(max))
|
||||
}
|
||||
return uploads.NewWorker(r.st, sizedConverter{seconds: wavSeconds}, r.tr, r.tmp, time.Minute, opts...)
|
||||
}
|
||||
|
||||
func jobWith(r *wrig, t *testing.T, claimed time.Duration) uploads.Job {
|
||||
j := r.spool(t, "a")
|
||||
j.Duration = claimed
|
||||
return j
|
||||
}
|
||||
|
||||
func TestWorkerDuration_AConvertedLengthOverTheLimitFailsTheJobBeforeAnyEngineSeesIt(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, 100*60, 90*time.Minute)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, 10*time.Second))
|
||||
|
||||
_, failed := r.st.snapshot()
|
||||
assert.Equal(t, domain.UploadErrTooLong, failed["a"], "a header that claimed 10 s must not let 100 minutes through")
|
||||
assert.Empty(t, r.tr.paths, "no engine may be sent audio that is over the limit")
|
||||
assert.Empty(t, r.leftovers(t), "the audio is still deleted")
|
||||
}
|
||||
|
||||
func TestWorkerDuration_JustOverTheLimitWithinTheSlackStillTranscribes(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, 90*60+20, 90*time.Minute)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, 90*time.Minute))
|
||||
|
||||
completed, failed := r.st.snapshot()
|
||||
assert.Empty(t, failed)
|
||||
assert.Contains(t, completed, "a", "ffmpeg rounds: 20 s over a 90-minute limit is not a lie")
|
||||
}
|
||||
|
||||
func TestWorkerDuration_AFileThatEndsFarEarlierThanItClaimedIsRefusedNotHalfTranscribed(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, 20*60, 90*time.Minute)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, 60*time.Minute))
|
||||
|
||||
_, failed := r.st.snapshot()
|
||||
assert.Equal(t, domain.UploadErrIncomplete, failed["a"], "a recording truncated at a third must not return a third of a transcript with no warning")
|
||||
assert.Empty(t, r.tr.paths)
|
||||
}
|
||||
|
||||
func TestWorkerDuration_AnEstimateThatIsMerelyALittleOffIsNotTruncation(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
claimed time.Duration
|
||||
wav float64
|
||||
}{
|
||||
{"five percent short", 60 * time.Minute, 57 * 60},
|
||||
{"under two minutes short of a long file", 60 * time.Minute, 59 * 60},
|
||||
{"unknown claimed length", 0, 20 * 60},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, tt.wav, 90*time.Minute)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, tt.claimed))
|
||||
|
||||
completed, failed := r.st.snapshot()
|
||||
assert.Empty(t, failed)
|
||||
assert.Contains(t, completed, "a")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkerDuration_WithNoMaximumConfiguredNothingIsMeasured(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := durationWorker(t, r, 100*60, 0)
|
||||
|
||||
w.Process(context.Background(), jobWith(r, t, 0))
|
||||
|
||||
completed, _ := r.st.snapshot()
|
||||
assert.Contains(t, completed, "a")
|
||||
require.NotNil(t, w)
|
||||
}
|
||||
@@ -146,7 +146,7 @@ func TestWorkerProgress_TheHeartbeatBeatsWhileAnEngineCallIsStuckAndStopsWhenThe
|
||||
assert.Equal(t, after, r.st.beatCount(), "no heartbeat may outlive its job")
|
||||
}
|
||||
|
||||
func TestWorkerProgress_TheLogNamesEveryPhaseInOrderAndNeverTheTranscriptOrThePath(t *testing.T) {
|
||||
func TestWorkerProgress_OnASuccessfulJobTheLogNamesEveryPhaseInOrderAndNeverTheTranscriptOrThePath(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
r.tr.result = domain.Transcription{Text: "zebrakaka secret meeting words", Endpoint: "iguana/kb-whisper", AudioSeconds: 3}
|
||||
buf := &syncLog{}
|
||||
@@ -162,4 +162,38 @@ func TestWorkerProgress_TheLogNamesEveryPhaseInOrderAndNeverTheTranscriptOrThePa
|
||||
assert.Contains(t, out, "upload=a", "every line says which upload it is about")
|
||||
assert.NotContains(t, out, "zebrakaka", "no transcript text in the log")
|
||||
assert.NotContains(t, out, r.tmp, "no spool path, which carries the upload's identity, in the log")
|
||||
assert.Contains(t, out, "upload transcribed", "the job's own summary line")
|
||||
assert.Contains(t, out, "endpoint=iguana/kb-whisper", "which endpoint served it is a quality fact")
|
||||
}
|
||||
|
||||
func TestWorkerProgress_AFailureLogsAKindNeverTheErrorTextOrThePath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
setup func(*wrig)
|
||||
line string
|
||||
}{
|
||||
{"conversion fails and names the spool file", func(r *wrig) {
|
||||
r.conv.err = errors.New("Error opening input file " + r.tmp + "/upload-a: Invalid data zebrakaka")
|
||||
}, "upload: convert"},
|
||||
{"every engine fails and the gateway echoed a word", func(r *wrig) {
|
||||
r.tr.err = errors.New("gateway echoed: zebrakaka secret words")
|
||||
}, "upload: transcribe"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
tt.setup(r)
|
||||
buf := &syncLog{}
|
||||
w := newProgressWorker(t, r, r.tr, uploads.WithLogger(slog.New(slog.NewTextHandler(buf, nil))))
|
||||
|
||||
w.Process(context.Background(), r.spool(t, "a"))
|
||||
|
||||
out := buf.String()
|
||||
assert.NotContains(t, out, "zebrakaka", "provider text or file contents in the log")
|
||||
assert.NotContains(t, out, r.tmp, "the spool path in the log")
|
||||
assert.Contains(t, out, tt.line, "the failure must be logged at all, not only scrubbed")
|
||||
assert.Contains(t, out, "kind=error")
|
||||
assert.Contains(t, out, "upload=a")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
package uploads_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||
)
|
||||
|
||||
// The queue was append-only: 100,000 submits with the worker idle all succeeded, and each
|
||||
// queued job holds up to the upload cap on disk until its turn (adversarial review F6).
|
||||
|
||||
func queueWorker(t *testing.T, total, perUser int) *uploads.Worker {
|
||||
t.Helper()
|
||||
r := newWRig(t, time.Minute)
|
||||
return uploads.NewWorker(r.st, r.conv, r.tr, r.tmp, time.Minute, uploads.WithQueueLimits(total, perUser))
|
||||
}
|
||||
|
||||
func submit(w *uploads.Worker, user string, n int) error {
|
||||
return w.Submit(uploads.Job{UserID: user, UploadID: user + "-" + strconv.Itoa(n), Path: "/nonexistent"})
|
||||
}
|
||||
|
||||
func TestWorkerSubmit_RefusesBeyondTheTotalQueueCap(t *testing.T) {
|
||||
w := queueWorker(t, 3, 10)
|
||||
|
||||
for i := 0; i < 3; i++ {
|
||||
require.NoError(t, submit(w, "u"+strconv.Itoa(i), i))
|
||||
}
|
||||
|
||||
err := submit(w, "u9", 9)
|
||||
assert.ErrorIs(t, err, uploads.ErrQueueFull)
|
||||
}
|
||||
|
||||
func TestWorkerSubmit_OneUserCannotFillTheQueueForEveryoneElse(t *testing.T) {
|
||||
w := queueWorker(t, 100, 2)
|
||||
|
||||
require.NoError(t, submit(w, "alice", 1))
|
||||
require.NoError(t, submit(w, "alice", 2))
|
||||
errAlice := submit(w, "alice", 3)
|
||||
errBob := submit(w, "bob", 1)
|
||||
|
||||
assert.True(t, errors.Is(errAlice, uploads.ErrQueueFull), "a third waiting upload from one user is refused")
|
||||
assert.NoError(t, errBob, "another user is not affected by alice's queue")
|
||||
}
|
||||
|
||||
func TestWorkerSubmit_ThereIsADefaultCapSoANeglectedWorkerCannotBeFilledForever(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := uploads.NewWorker(r.st, r.conv, r.tr, r.tmp, time.Minute)
|
||||
|
||||
refused := 0
|
||||
for i := 0; i < 1000; i++ {
|
||||
if err := submit(w, "u"+strconv.Itoa(i), i); errors.Is(err, uploads.ErrQueueFull) {
|
||||
refused++
|
||||
}
|
||||
}
|
||||
|
||||
assert.Positive(t, refused, "with the defaults, 1,000 submits to an idle worker must not all be accepted")
|
||||
}
|
||||
|
||||
func TestWorkerSubmit_AJobThatHasLeftTheQueueFreesItsPlace(t *testing.T) {
|
||||
r := newWRig(t, time.Minute)
|
||||
w := uploads.NewWorker(r.st, r.conv, r.tr, r.tmp, time.Minute, uploads.WithQueueLimits(1, 1))
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
done := make(chan struct{})
|
||||
go func() { defer close(done); w.Run(ctx) }()
|
||||
defer func() { cancel(); <-done }()
|
||||
|
||||
job := r.spool(t, "first")
|
||||
require.NoError(t, w.Submit(job))
|
||||
require.Eventually(t, func() bool { c, _ := r.st.snapshot(); _, ok := c["first"]; return ok },
|
||||
5*time.Second, 5*time.Millisecond, "the first job never completed")
|
||||
|
||||
assert.NoError(t, w.Submit(r.spool(t, "second")), "the cap counts jobs waiting, not jobs ever submitted")
|
||||
}
|
||||
@@ -23,8 +23,10 @@ type wStore struct {
|
||||
completed map[string]domain.Transcription
|
||||
failed map[string]domain.UploadErrorKind
|
||||
startErr error
|
||||
saves [][]domain.ProgressStep
|
||||
beats int
|
||||
// completeErr makes CompleteUpload fail, as a database outage would.
|
||||
completeErr error
|
||||
saves [][]domain.ProgressStep
|
||||
beats int
|
||||
// beatDelay makes each heartbeat write slow, so one is still in flight when a job
|
||||
// ends -- which is what proves the worker waits for its heartbeat to finish.
|
||||
beatDelay time.Duration
|
||||
@@ -44,6 +46,9 @@ func (s *wStore) StartTranscribing(_ context.Context, _, id string) error {
|
||||
func (s *wStore) CompleteUpload(_ context.Context, _, id string, t domain.Transcription) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
if s.completeErr != nil {
|
||||
return s.completeErr
|
||||
}
|
||||
s.completed[id] = t
|
||||
return nil
|
||||
}
|
||||
@@ -208,6 +213,10 @@ func TestWorkerProcess_FailuresAreTypedAndStillDeleteTheAudio(t *testing.T) {
|
||||
}{
|
||||
{"conversion fails", func(r *wrig) { r.conv.err = errors.New("moov atom not found") }},
|
||||
{"every transcription endpoint fails", func(r *wrig) { r.tr.err = errors.New("iguana down; berget down") }},
|
||||
{"storing the finished transcript fails", func(r *wrig) { r.st.completeErr = errors.New("db down") }},
|
||||
{"the temporary WAV cannot be created", func(r *wrig) {
|
||||
r.w = uploads.NewWorker(r.st, r.conv, r.tr, filepath.Join(r.tmp, "missing"), time.Minute)
|
||||
}},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
||||
@@ -122,6 +122,11 @@ type App struct {
|
||||
// summarizer's (koala/phi4-mini, 8,192-token context) and cannot hold an hour of
|
||||
// speech.
|
||||
UploadChatModel string
|
||||
// UploadChatModels is the set of models chat over an uploaded recording offers and
|
||||
// accepts. The whole private transcript goes to the one chosen, so it must hold
|
||||
// LOCAL models only, and the caller decides that. Nil or empty fails closed: no
|
||||
// model is offered and none is called (ADR-035 review).
|
||||
UploadChatModels []string
|
||||
// Processing tracks in-flight immediate summarizations so the status endpoint
|
||||
// shows the animation until the summary lands. The zero value is ready to use.
|
||||
Processing ProcessingSet
|
||||
|
||||
+94
-11
@@ -8,8 +8,10 @@ import (
|
||||
"math"
|
||||
"net/http"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
"unicode"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/a-h/templ"
|
||||
@@ -17,6 +19,7 @@ import (
|
||||
"git.d-ma.be/mathias/tapir/internal/adapters/chat"
|
||||
"git.d-ma.be/mathias/tapir/internal/adapters/store"
|
||||
"git.d-ma.be/mathias/tapir/internal/domain"
|
||||
"git.d-ma.be/mathias/tapir/internal/errkind"
|
||||
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||
)
|
||||
|
||||
@@ -91,13 +94,26 @@ func (a *App) handleUploadPost(w http.ResponseWriter, r *http.Request) {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
var tooBig *http.MaxBytesError
|
||||
if !errors.As(err, &tooBig) {
|
||||
// A malformed multipart body (a header with a control byte, a truncated
|
||||
// part) is the sender's to fix: a 400, not a server error, and the error
|
||||
// text is never logged: it quotes the headers, which carry the file name.
|
||||
a.logger().Info("upload rejected", "reason", "malformed")
|
||||
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "That upload could not be read. Please try again.")
|
||||
return
|
||||
}
|
||||
a.uploadFailure(w, r, userID, err)
|
||||
return
|
||||
}
|
||||
switch part.FormName() {
|
||||
case "title":
|
||||
b, _ := io.ReadAll(io.LimitReader(part, maxTitleRunes*utf8.UTFMax))
|
||||
title = clipRunes(strings.TrimSpace(string(b)), maxTitleRunes)
|
||||
b, err := io.ReadAll(io.LimitReader(part, maxTitleRunes*utf8.UTFMax))
|
||||
if err != nil {
|
||||
a.renderUploadsPage(w, r, userID, http.StatusBadRequest, "That upload could not be read. Please try again.")
|
||||
return
|
||||
}
|
||||
title = cleanTitle(string(b))
|
||||
case "file":
|
||||
if title == "" {
|
||||
title = titleFromFilename(part.FileName())
|
||||
@@ -125,8 +141,11 @@ func (a *App) uploadFailure(w http.ResponseWriter, r *http.Request, userID strin
|
||||
// Logged by reason only: the filename and title are private content.
|
||||
a.logger().Info("upload rejected", "reason", string(rej.Reason))
|
||||
status := http.StatusUnprocessableEntity
|
||||
if rej.Reason == uploads.ReasonTooLarge {
|
||||
switch rej.Reason {
|
||||
case uploads.ReasonTooLarge:
|
||||
status = http.StatusRequestEntityTooLarge
|
||||
case uploads.ReasonBusy:
|
||||
status = http.StatusTooManyRequests
|
||||
}
|
||||
a.renderUploadsPage(w, r, userID, status, rej.Message)
|
||||
case errors.As(err, &tooBig):
|
||||
@@ -134,7 +153,9 @@ func (a *App) uploadFailure(w http.ResponseWriter, r *http.Request, userID strin
|
||||
a.renderUploadsPage(w, r, userID, http.StatusRequestEntityTooLarge,
|
||||
"This file is larger than the upload limit.")
|
||||
default:
|
||||
a.serverError(w, r, "upload", err)
|
||||
// Not err: it can carry the file's name, a spool path or a database value.
|
||||
a.logger().Error("upload failed", "kind", errkind.Of(err))
|
||||
http.Error(w, "internal error", http.StatusInternalServerError)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -165,7 +186,14 @@ func (a *App) loadOwnedUpload(w http.ResponseWriter, r *http.Request) (*domain.U
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
u, err := a.Store.GetUpload(r.Context(), userID, r.PathValue("id"))
|
||||
id := r.PathValue("id")
|
||||
if !uuidShape.MatchString(id) {
|
||||
// Not an id at all: the same 404 as an unknown or foreign one, never a database
|
||||
// error that surfaces as a 500 (and no oracle about which ids exist).
|
||||
http.NotFound(w, r)
|
||||
return nil, false
|
||||
}
|
||||
u, err := a.Store.GetUpload(r.Context(), userID, id)
|
||||
if errors.Is(err, store.ErrUploadNotFound) {
|
||||
http.NotFound(w, r)
|
||||
return nil, false
|
||||
@@ -180,11 +208,29 @@ func (a *App) loadOwnedUpload(w http.ResponseWriter, r *http.Request) (*domain.U
|
||||
// titleFromFilename is the default title: the file's name without its extension.
|
||||
func titleFromFilename(name string) string {
|
||||
base := filepath.Base(name)
|
||||
t := strings.TrimSuffix(base, filepath.Ext(base))
|
||||
t := cleanTitle(strings.TrimSuffix(base, filepath.Ext(base)))
|
||||
if t == "" || t == "." {
|
||||
return "Untitled recording"
|
||||
}
|
||||
return clipRunes(t, maxTitleRunes)
|
||||
return t
|
||||
}
|
||||
|
||||
// uuidShape is the form of an upload id. Anything else cannot be one, so it is a 404.
|
||||
var uuidShape = regexp.MustCompile(`^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$`)
|
||||
|
||||
// cleanTitle makes a title safe to store and show: invalid UTF-8 dropped, control
|
||||
// characters (NUL, newlines, escapes) turned into spaces, runs of space collapsed, trimmed
|
||||
// and clipped. A title is cosmetic; bad bytes in it must never fail an upload, and Postgres
|
||||
// refuses NUL in text.
|
||||
func cleanTitle(s string) string {
|
||||
s = strings.ToValidUTF8(s, "")
|
||||
s = strings.Map(func(r rune) rune {
|
||||
if unicode.IsControl(r) {
|
||||
return ' '
|
||||
}
|
||||
return r
|
||||
}, s)
|
||||
return clipRunes(strings.Join(strings.Fields(s), " "), maxTitleRunes)
|
||||
}
|
||||
|
||||
func clipRunes(s string, n int) string {
|
||||
@@ -200,6 +246,12 @@ func uploadFailureMessage(k domain.UploadErrorKind) string {
|
||||
switch k {
|
||||
case domain.UploadErrInterrupted:
|
||||
return "The server restarted before this recording was transcribed. Please upload it again."
|
||||
case domain.UploadErrBusy:
|
||||
return "Too many recordings were waiting to be transcribed when this one was uploaded. Please upload it again in a few minutes."
|
||||
case domain.UploadErrTooLong:
|
||||
return "This recording is longer than the limit once its audio is read (the file's header said otherwise). Please upload a shorter recording."
|
||||
case domain.UploadErrIncomplete:
|
||||
return "This recording ends much earlier than the file says it should; it may be cut off or damaged. Please upload the complete recording."
|
||||
case domain.UploadErrTranscriptionFailed:
|
||||
return "Transcription failed on every available service. Please upload it again later."
|
||||
default:
|
||||
@@ -220,7 +272,8 @@ func (a *App) handleUploadChat(w http.ResponseWriter, r *http.Request) {
|
||||
http.Redirect(w, r, "/uploads/"+u.ID, http.StatusSeeOther)
|
||||
return
|
||||
}
|
||||
a.render(w, r, UploadChatPage(*u, a.uploadChatView(*u, a.Chat.DefaultModel(a.UploadChatModel), nil, false, "")))
|
||||
model, _ := a.resolveUploadModel("")
|
||||
a.render(w, r, UploadChatPage(*u, a.uploadChatView(*u, model, nil, false, "")))
|
||||
}
|
||||
|
||||
// handleUploadChatMessage answers one question against the owner's uploaded
|
||||
@@ -240,7 +293,13 @@ func (a *App) handleUploadChatMessage(w http.ResponseWriter, r *http.Request) {
|
||||
http.Error(w, "bad form", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
model := a.resolveModel(r.FormValue("model"), a.UploadChatModel)
|
||||
model, ok := a.resolveUploadModel(r.FormValue("model"))
|
||||
if !ok {
|
||||
a.logger().Error("upload chat: no local model is configured; nothing was sent", "upload", u.ID)
|
||||
view := a.uploadChatView(*u, "", nil, false, "No local model is available for chat about a recording.")
|
||||
a.render(w, r, UploadChatPage(*u, view))
|
||||
return
|
||||
}
|
||||
history := parseHistory(r.Form["hq"], r.Form["ha"])
|
||||
question := strings.TrimSpace(r.FormValue("question"))
|
||||
|
||||
@@ -254,7 +313,7 @@ func (a *App) handleUploadChatMessage(w http.ResponseWriter, r *http.Request) {
|
||||
MaxChars: a.UploadChatMaxChars,
|
||||
})
|
||||
if err != nil {
|
||||
a.logger().Error("upload chat answer", "upload", u.ID, "model", model, "err", err)
|
||||
a.logger().Error("upload chat answer", "upload", u.ID, "model", model, "kind", errkind.Of(err))
|
||||
view.Error = "That model couldn't answer just now. Try again, or switch models."
|
||||
} else {
|
||||
view.History = appendTurn(history, chat.Turn{Question: question, Answer: reply.Answer})
|
||||
@@ -273,7 +332,7 @@ func (a *App) uploadChatView(u domain.UploadedTranscript, model string, history
|
||||
VideoID: u.ID,
|
||||
Title: u.Title,
|
||||
Available: true,
|
||||
Models: a.Chat.Models(),
|
||||
Models: a.UploadChatModels,
|
||||
Selected: model,
|
||||
History: history,
|
||||
Truncated: truncated,
|
||||
@@ -401,3 +460,27 @@ func minutesAgo(d time.Duration) string {
|
||||
}
|
||||
return "under a minute"
|
||||
}
|
||||
|
||||
// resolveUploadModel picks the model for chat over an uploaded recording from the
|
||||
// offered LOCAL set only: the posted one when it is in the set, else the configured
|
||||
// default when it is, else the first. ok is false when no model is offered, in which
|
||||
// case nothing may be sent anywhere (the whole private transcript is the payload).
|
||||
func (a *App) resolveUploadModel(posted string) (model string, ok bool) {
|
||||
offered := func(m string) bool {
|
||||
for _, o := range a.UploadChatModels {
|
||||
if o == m {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
switch {
|
||||
case posted != "" && offered(posted):
|
||||
return posted, true
|
||||
case a.UploadChatModel != "" && offered(a.UploadChatModel):
|
||||
return a.UploadChatModel, true
|
||||
case len(a.UploadChatModels) > 0:
|
||||
return a.UploadChatModels[0], true
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
package web_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// Input that cannot be right is the caller's to fix: a 4xx, never a 500 (adversarial
|
||||
// review F7). No cross-tenant oracle either way: unknown and foreign ids look the same.
|
||||
|
||||
func TestUploadRoutes_ANonUUIDIdIs404NotA500(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
for _, path := range []string{"/uploads/not-a-uuid", "/uploads/not-a-uuid/status", "/uploads/not-a-uuid/chat", "/uploads/%00/status", "/uploads/1/status"} {
|
||||
t.Run(path, func(t *testing.T) {
|
||||
rec := do(t, app, get(path))
|
||||
|
||||
assert.Equal(t, http.StatusNotFound, rec.Code)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadPost_ATitleWithControlCharactersOrInvalidUTF8IsCleanedNotA500(t *testing.T) {
|
||||
tests := []struct{ name, title, filename string }{
|
||||
{"a NUL in the title", "Sprint\x00review", "a.m4a"},
|
||||
{"invalid UTF-8 in the title", "Sprint \xff\xfe review", "a.m4a"},
|
||||
{"a newline in the title", "Sprint\nreview", "a.m4a"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
app, sub := newUploadApp(t, audioOK, 200<<20)
|
||||
|
||||
rec := do(t, app, uploadReq(t, tt.title, tt.filename, []byte("synthetic audio bytes")))
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, rec.Code, "bad bytes in a cosmetic title must not fail the upload: %s", body(t, rec))
|
||||
require.Len(t, sub.jobs, 1)
|
||||
var title string
|
||||
require.NoError(t, rawPool(t).QueryRow(context.Background(), `SELECT title FROM uploaded_transcripts WHERE id = $1`, sub.jobs[0].UploadID).Scan(&title))
|
||||
assert.NotContains(t, title, "\x00")
|
||||
assert.NotContains(t, title, "\n")
|
||||
assert.True(t, len(title) > 0, "a title is still shown")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUploadPost_AMalformedMultipartHeaderIsA400AndItsTextIsNotLogged(t *testing.T) {
|
||||
// A control byte in the part's file name makes the multipart body malformed. It is the
|
||||
// sender's to fix, and the parser's error quotes the header, which carries the name.
|
||||
app, sub := newUploadApp(t, audioOK, 200<<20)
|
||||
buf := &lockedBuf{}
|
||||
app.Log = slog.New(slog.NewTextHandler(buf, nil))
|
||||
|
||||
rec := do(t, app, uploadReq(t, "", "zebrakaka\x00weekly.m4a", []byte("x")))
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, rec.Code)
|
||||
assert.Empty(t, sub.jobs)
|
||||
assert.NotContains(t, buf.String(), "zebrakaka", "a file name is private content")
|
||||
}
|
||||
@@ -0,0 +1,31 @@
|
||||
package web_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/uploads"
|
||||
)
|
||||
|
||||
func TestUploadPost_ABusyRejectionIs429WithItsMessage(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
app.Uploads = failingUploader{err: &uploads.RejectError{Reason: uploads.ReasonBusy, Message: "Too many recordings are waiting. Try again in a few minutes."}}
|
||||
|
||||
rec := do(t, app, uploadReq(t, "t", "a.m4a", []byte("x")))
|
||||
|
||||
require.Equal(t, http.StatusTooManyRequests, rec.Code, "the user's to retry: not a 500, not a 422")
|
||||
assert.Contains(t, body(t, rec), "Too many recordings are waiting")
|
||||
}
|
||||
|
||||
func TestUploadStatus_ABusyFailureIsExplainedInWords(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
id := seedUpload(t, userID, "m", "failed", "queue_full")
|
||||
|
||||
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
|
||||
|
||||
assert.Contains(t, b, "waiting")
|
||||
assert.NotContains(t, b, "This upload failed. Please upload it again.")
|
||||
}
|
||||
@@ -31,6 +31,7 @@ func newUploadChatApp(t *testing.T) (*web.App, *fakeChatter) {
|
||||
}
|
||||
app.Chat = fc
|
||||
app.UploadChatMaxChars = 120000
|
||||
app.UploadChatModels = fc.models
|
||||
return app, fc
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
package web_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/adapters/chat"
|
||||
"git.d-ma.be/mathias/tapir/internal/web"
|
||||
)
|
||||
|
||||
// The whole private transcript goes to the model chosen in the form, so the upload chat
|
||||
// offers and accepts local models only, even when the shared chat service knows a cloud
|
||||
// one (ADR-035 review). The cloud model here is only a name; no network is involved.
|
||||
|
||||
func newUploadChatAppWithCloud(t *testing.T) (*web.App, *fakeChatter) {
|
||||
t.Helper()
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
fc := &fakeChatter{
|
||||
models: []string{"koala/phi4-mini", "iguana/gemma4-26b", "berget/mistral-small"},
|
||||
reply: chat.Reply{Answer: "Anders owns the timber order."},
|
||||
}
|
||||
app.Chat = fc
|
||||
app.UploadChatMaxChars = 120000
|
||||
app.UploadChatModels = []string{"koala/phi4-mini", "iguana/gemma4-26b"}
|
||||
return app, fc
|
||||
}
|
||||
|
||||
func TestUploadChatPage_DoesNotOfferACloudModelEvenThoughTheServiceKnowsOne(t *testing.T) {
|
||||
app, _ := newUploadChatAppWithCloud(t)
|
||||
id := seedUpload(t, userID, "Roadmap sync", "done", meetingText)
|
||||
|
||||
b := body(t, do(t, app, get("/uploads/"+id+"/chat")))
|
||||
|
||||
assert.Contains(t, b, "iguana/gemma4-26b")
|
||||
assert.NotContains(t, b, "berget/mistral-small", "the switcher must not list a model the transcript must not be sent to")
|
||||
}
|
||||
|
||||
func TestUploadChatPost_AForgedCloudModelIsReplacedByALocalOneBeforeTheTranscriptIsSent(t *testing.T) {
|
||||
app, fc := newUploadChatAppWithCloud(t)
|
||||
id := seedUpload(t, userID, "Roadmap sync", "done", meetingText)
|
||||
|
||||
rec := do(t, app, chatPost("/uploads/"+id+"/chat", url.Values{"question": {"Who owns timber?"}, "model": {"berget/mistral-small"}}))
|
||||
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
require.Equal(t, 1, fc.callCount)
|
||||
assert.NotEqual(t, "berget/mistral-small", fc.gotReq.Model, "a cloud model reached the chat service with the private transcript")
|
||||
assert.Contains(t, []string{"koala/phi4-mini", "iguana/gemma4-26b"}, fc.gotReq.Model)
|
||||
}
|
||||
|
||||
func TestUploadChatPost_ALocalModelThatIsOfferedIsHonoured(t *testing.T) {
|
||||
app, fc := newUploadChatAppWithCloud(t)
|
||||
id := seedUpload(t, userID, "Roadmap sync", "done", meetingText)
|
||||
|
||||
rec := do(t, app, chatPost("/uploads/"+id+"/chat", url.Values{"question": {"q"}, "model": {"koala/phi4-mini"}}))
|
||||
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
assert.Equal(t, "koala/phi4-mini", fc.gotReq.Model)
|
||||
}
|
||||
|
||||
func TestUploadChat_WithNoLocalModelConfiguredNothingIsSentAnywhere(t *testing.T) {
|
||||
app, fc := newUploadChatAppWithCloud(t)
|
||||
app.UploadChatModels = nil
|
||||
id := seedUpload(t, userID, "Roadmap sync", "done", meetingText)
|
||||
|
||||
rec := do(t, app, chatPost("/uploads/"+id+"/chat", url.Values{"question": {"q"}, "model": {"berget/mistral-small"}}))
|
||||
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
assert.Zero(t, fc.callCount, "failing closed: no local model offered means no model is called")
|
||||
}
|
||||
@@ -132,7 +132,7 @@ func TestUploadPost_AcceptedAudioQueuesARowAndRedirectsToIt(t *testing.T) {
|
||||
assert.Equal(t, "Sprint review", title)
|
||||
assert.Equal(t, "/uploads/"+id, rec.Header().Get("Location"))
|
||||
require.Len(t, sub.jobs, 1)
|
||||
assert.Equal(t, uploads.Job{UserID: userID, UploadID: id, Path: sub.jobs[0].Path}, sub.jobs[0])
|
||||
assert.Equal(t, uploads.Job{UserID: userID, UploadID: id, Path: sub.jobs[0].Path, Duration: audioOK.Duration}, sub.jobs[0])
|
||||
spooled, err := os.ReadFile(sub.jobs[0].Path)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "synthetic audio bytes", string(spooled))
|
||||
@@ -366,8 +366,8 @@ func setReconciliation(t *testing.T, id, json string) {
|
||||
const reconciledJSON = `{"status":"reconciled","base":"iguana/pianissimo","alt":"iguana/kb-whisper","reconciler":"iguana/gemma4-26b",
|
||||
"disputes":380,"auto_resolved":90,"adjudicated":285,"changed":120,"unsure":5,"errors":0,
|
||||
"contested":[
|
||||
{"seconds":2210.5,"base":"SEB","alt":"SCB","used":"alt","unsure":false},
|
||||
{"seconds":3400,"base":"T+1","alt":"en T","used":"base","unsure":true}]}`
|
||||
{"seconds":2210.5,"base":"Axel","alt":"Alex","used":"alt","unsure":false},
|
||||
{"seconds":3400,"base":"A+2","alt":"en A","used":"base","unsure":true}]}`
|
||||
|
||||
func TestUploadStatus_ReconciledSaysWhatWasDoneInPlainWords(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
@@ -394,11 +394,11 @@ func TestUploadStatus_ListsTheContestedSpotsWithAMinuteMarkBothReadingsAndWhichW
|
||||
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
|
||||
|
||||
assert.Contains(t, b, "~37 min", "2210.5 s")
|
||||
assert.Contains(t, b, "SEB")
|
||||
assert.Contains(t, b, "SCB")
|
||||
assert.Contains(t, b, "Axel")
|
||||
assert.Contains(t, b, "Alex")
|
||||
assert.Contains(t, b, "~57 min", "3400 s")
|
||||
assert.Contains(t, b, "T+1")
|
||||
assert.Contains(t, b, "en T")
|
||||
assert.Contains(t, b, "A+2")
|
||||
assert.Contains(t, b, "en A")
|
||||
assert.Contains(t, b, "uncertain", "an unsure spot is marked so the reader knows to check it")
|
||||
}
|
||||
|
||||
@@ -460,3 +460,21 @@ func TestUploadStatus_AVeryLongListOfContestedSpotsIsShortenedWithItsTrueCount(t
|
||||
assert.NotContains(t, b, ">b119<", "the page does not render every one")
|
||||
assert.Contains(t, b, "70 more", "120 stored, 50 shown")
|
||||
}
|
||||
|
||||
func TestUploadStatus_ALengthFailureIsExplainedInWordsAndNotAsAGenericError(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
tests := []struct{ kind, want string }{
|
||||
{"audio_too_long", "longer than the limit"},
|
||||
{"audio_incomplete", "ends much earlier than"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.kind, func(t *testing.T) {
|
||||
id := seedUpload(t, userID, "m-"+tt.kind, "failed", tt.kind)
|
||||
|
||||
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
|
||||
|
||||
assert.Contains(t, b, tt.want)
|
||||
assert.NotContains(t, b, "This upload failed. Please upload it again.")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
package web_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// Error paths of the upload routes log a kind, never the error's text, which can carry
|
||||
// a file name, a path, a database value or a provider's output (adversarial review F5).
|
||||
|
||||
type lockedBuf struct {
|
||||
mu sync.Mutex
|
||||
b bytes.Buffer
|
||||
}
|
||||
|
||||
func (l *lockedBuf) Write(p []byte) (int, error) {
|
||||
l.mu.Lock()
|
||||
defer l.mu.Unlock()
|
||||
return l.b.Write(p)
|
||||
}
|
||||
|
||||
func (l *lockedBuf) String() string {
|
||||
l.mu.Lock()
|
||||
defer l.mu.Unlock()
|
||||
return l.b.String()
|
||||
}
|
||||
|
||||
type failingUploader struct{ err error }
|
||||
|
||||
func (f failingUploader) Accept(context.Context, string, string, io.Reader) (string, error) {
|
||||
return "", f.err
|
||||
}
|
||||
|
||||
func TestUploadPost_AnUnexpectedFailureLogsAKindAndNotItsText(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
buf := &lockedBuf{}
|
||||
app.Log = slog.New(slog.NewTextHandler(buf, nil))
|
||||
app.Uploads = failingUploader{err: errors.New(`ffprobe: Error opening input file /spool/upload-555 for "zebrakaka-meeting.m4a"`)}
|
||||
|
||||
rec := do(t, app, uploadReq(t, "t", "zebrakaka-meeting.m4a", []byte("x")))
|
||||
|
||||
assert.Equal(t, http.StatusInternalServerError, rec.Code)
|
||||
out := buf.String()
|
||||
assert.NotContains(t, out, "zebrakaka", "a file name is private content")
|
||||
assert.NotContains(t, out, "/spool/", "a spool path carries the upload's identity")
|
||||
assert.Contains(t, out, "kind=error")
|
||||
}
|
||||
|
||||
func TestUploadChatPost_AModelFailureLogsAKindAndNotItsText(t *testing.T) {
|
||||
app, fc := newUploadChatApp(t)
|
||||
buf := &lockedBuf{}
|
||||
app.Log = slog.New(slog.NewTextHandler(buf, nil))
|
||||
fc.err = errors.New("LLM returned 400: the prompt was zebrakaka secret words")
|
||||
id := seedUpload(t, userID, "m", "done", meetingText)
|
||||
|
||||
rec := do(t, app, chatPost("/uploads/"+id+"/chat", url.Values{"question": {"q"}}))
|
||||
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
out := buf.String()
|
||||
assert.NotContains(t, out, "zebrakaka")
|
||||
assert.Contains(t, out, "kind=error")
|
||||
}
|
||||
@@ -229,16 +229,25 @@ func TestUploadProgress_ADoneJobKeepsItsRecordOfWhatWasDoneAndHowLongEachTook(t
|
||||
assert.NotContains(t, b, "spinner")
|
||||
}
|
||||
|
||||
func TestUploadProgress_TheFullPageRendersTheSameDisplayAsTheFragment(t *testing.T) {
|
||||
// stepsBlock is the progress list of a page: from its opening tag to its closing one.
|
||||
func stepsBlock(t *testing.T, html string) string {
|
||||
t.Helper()
|
||||
start := strings.Index(html, `<ol class="steps"`)
|
||||
require.GreaterOrEqual(t, start, 0, "no progress list in:\n%s", html)
|
||||
end := strings.Index(html[start:], "</ol>")
|
||||
require.GreaterOrEqual(t, end, 0)
|
||||
return html[start : start+end+len("</ol>")]
|
||||
}
|
||||
|
||||
func TestUploadProgress_TheFullPageCarriesTheVerySameProgressListAsTheFragment(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
fixedClock(app)
|
||||
id := seedUpload(t, userID, "m", "transcribing", "")
|
||||
seedProgress(t, id, twoEngineRun(), progressNow.Add(-10*time.Second), progressNow.Add(-4*time.Second))
|
||||
|
||||
rec := do(t, app, get("/uploads/"+id))
|
||||
full := body(t, do(t, app, get("/uploads/"+id)))
|
||||
fragment := statusBody(t, app, id)
|
||||
|
||||
require.Equal(t, 200, rec.Code)
|
||||
b := body(t, rec)
|
||||
assert.Contains(t, b, "Transcribing with iguana/kb-whisper", "the page a user opens, not only the polled fragment, shows the phases")
|
||||
assert.Contains(t, b, "3m 12s")
|
||||
assert.Equal(t, stepsBlock(t, fragment), stepsBlock(t, full),
|
||||
"the page a user opens and the fragment HTMX swaps in must show the same list, or the display jumps on the first poll")
|
||||
}
|
||||
|
||||
@@ -62,11 +62,11 @@ func TestUploadStatus_ASpotShowsTheWordsJustBeforeIt(t *testing.T) {
|
||||
app, _ := newUploadApp(t, audioOK, 200<<20)
|
||||
id := seedUpload(t, userID, "m", "done", "Synthetic transcript.")
|
||||
setReconciliation(t, id, `{"status":"reconciled","base":"a","alt":"b","reconciler":"c","disputes":1,"changed":1,
|
||||
"contested":[{"seconds":2640,"base":"special","alt":"CAMT","used":"alt","unsure":false,"before":"vi skickar in ett"}]}`)
|
||||
"contested":[{"seconds":2640,"base":"vanligt","alt":"KORT","used":"alt","unsure":false,"before":"vi skickar in ett"}]}`)
|
||||
|
||||
b := body(t, do(t, app, get("/uploads/"+id+"/status")))
|
||||
|
||||
assert.Contains(t, b, "vi skickar in ett", "the words before the spot are how a reader finds it in the audio")
|
||||
assert.Contains(t, b, "special")
|
||||
assert.Contains(t, b, "CAMT")
|
||||
assert.Contains(t, b, "vanligt")
|
||||
assert.Contains(t, b, "KORT")
|
||||
}
|
||||
|
||||
@@ -143,8 +143,17 @@ var scenarioCoverage = map[string]string{
|
||||
"A failed job shows where it stopped": "TestUploadProgress_AFailedJobShowsWhichPhaseFailedAndWhatNeverStarted",
|
||||
"A finished job keeps its record of what was done": "TestUploadProgress_ADoneJobKeepsItsRecordOfWhatWasDoneAndHowLongEachTook",
|
||||
"The heartbeat runs on its own while an engine call is stuck": "TestWorkerProgress_TheHeartbeatBeatsWhileAnEngineCallIsStuckAndStopsWhenTheJobEnds",
|
||||
"The log names every phase and never the transcript": "TestWorkerProgress_TheLogNamesEveryPhaseInOrderAndNeverTheTranscriptOrThePath",
|
||||
"The log names every phase and never the transcript": "TestWorkerProgress_OnASuccessfulJobTheLogNamesEveryPhaseInOrderAndNeverTheTranscriptOrThePath",
|
||||
"Every judged spot is stored but only the ones to check are listed": "TestUploadStatus_OnlyChangedAndUncertainSpotsAreListedNotTheKeptOnes",
|
||||
"Chat about a recording is offered with local models only": "TestUploadChatPage_DoesNotOfferACloudModelEvenThoughTheServiceKnowsOne",
|
||||
"A forged cloud model is never sent the transcript": "TestUploadChatPost_AForgedCloudModelIsReplacedByALocalOneBeforeTheTranscriptIsSent",
|
||||
"A wedged engine does not use up the whole job": "TestChain_AHungPrimaryReachesTheFallbackWhenItHasItsOwnDeadline",
|
||||
"A file that lies about its length is refused": "TestWorkerDuration_AConvertedLengthOverTheLimitFailsTheJobBeforeAnyEngineSeesIt",
|
||||
"A truncated recording is refused, not half transcribed": "TestWorkerDuration_AFileThatEndsFarEarlierThanItClaimedIsRefusedNotHalfTranscribed",
|
||||
"A full queue is a retry, not an error": "TestAccept_AFullQueueIsABusyRejectionNotAServerErrorAndLeavesNothingBehind",
|
||||
"One user cannot fill the queue for everyone": "TestWorkerSubmit_OneUserCannotFillTheQueueForEveryoneElse",
|
||||
"Errors are shown as kinds, never their text": "TestReconcile_TheBreakerReasonCarriesAKindNotTheModelsText",
|
||||
"Input that cannot be right is a client error": "TestUploadRoutes_ANonUUIDIdIs404NotA500",
|
||||
"The saved segments carry the merged words": "TestReconcileSegments_AChangedWordReplacesTheBaseWordInItsOwnSegmentOnly",
|
||||
|
||||
// summarize_new_video.feature
|
||||
|
||||
Reference in New Issue
Block a user