Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cace7d179d | ||
|
|
550764d43c | ||
|
|
4affeb12e8 | ||
|
|
460e07719d | ||
|
|
7b7da650d4 |
@@ -0,0 +1,69 @@
|
||||
# ADR-034 second blind check: protocol (ready to run; tapir#33, D9, D10)
|
||||
|
||||
This repo is public and the recordings are private, so this file holds the method only. The
|
||||
sheet, the key and the filled-in verdicts stay outside the repo, and the trial data is deleted
|
||||
after the check. Results go back here as aggregate numbers, as in
|
||||
`reconcile-blind-check-2026-10-02.md`.
|
||||
|
||||
## What the check decides
|
||||
|
||||
ADR-034 (two engines, a local model reconciles their disagreements) is `Proposed` and off in
|
||||
production. Two conditions were set for keeping it. The first run answered the first one on one
|
||||
reading only and did not answer the second:
|
||||
|
||||
1. **The reconciled reading is right at least 85% of the time.** Open flaw in the bar: it did not
|
||||
say how ties count. The first run gave 95% (decided items only) or 70% (all 30), so it was met on
|
||||
one reading and missed on the other.
|
||||
2. **It is no worse than the better single engine.** Not answered: before ADR-035 the spots the
|
||||
model *kept* as the base were not stored, so kb-whisper alone could not be compared.
|
||||
|
||||
ADR-035 now stores every judged spot (`reconciliation.contested`: both readings, which one was
|
||||
used, unsure or not, a few words of left context, a time). That makes condition 2 measurable.
|
||||
|
||||
## Rules to fix BEFORE looking at any result (the owner confirms or changes them)
|
||||
|
||||
| Rule | Proposed |
|
||||
|---|---|
|
||||
| Who judges | The speaker, hearing the audio where the text cannot settle it (D9). Not Opus: it cannot hear, and its agreement with the speaker was only shown on items picked as decidable. |
|
||||
| Sample | 30 spots drawn at random with a fixed seed, from ALL judged spots (changed and kept-base alike), not only the changed ones. Insertions and deletions that cannot be placed are listed separately and not sampled. |
|
||||
| Verdict per spot | `x`, `y` (the two readings, shown in random order, key kept apart), `neither`, or `cannot tell`. |
|
||||
| Condition 1 | Reconciled is right on at least 85% of the spots where the judge could tell (`cannot tell` excluded, reported, and `neither` counts as not right). Also report all-30. Pass needs the decided-only figure at 85% or more AND the all-30 figure above 70%. |
|
||||
| Condition 2 | Over the same spots, the reconciled text is right at least as often as the better single engine (base engine alone is right where the base reading is right; the other engine alone where the alt reading is right). Ties favour the single engine. |
|
||||
| If it fails | Switch reconcile off by configuration; the single-engine path is unchanged (ADR-034 reversibility). |
|
||||
|
||||
Why a fixed all-30 floor: the literal 70% last time sat well under the bar, and "decided only" can
|
||||
hide a model that mostly lands on spelling differences.
|
||||
|
||||
## What a run needs (D10: the owner is present)
|
||||
|
||||
- The same real recording as before (about 60 minutes). Only the owner has it.
|
||||
- A **throwaway** tapir instance with its own Postgres, not the production pod, so production's
|
||||
key pin and deployment are untouched. Settings: `TAPIR_RECONCILE_BASE_MODEL=iguana/pianissimo`,
|
||||
`TAPIR_TRANSCRIBE_MODEL=iguana/kb-whisper`, `TAPIR_RECONCILE_MODEL=iguana/gemma4-31b`, cloud
|
||||
fallbacks empty. Job time was 21 min 49 s per 63.6 min last run.
|
||||
- iguana with Low Power Mode off and gemma4-31b able to load (the production-iguana model load
|
||||
that needs a quiet window; D6).
|
||||
|
||||
## Steps
|
||||
|
||||
1. Start the throwaway instance, upload the recording, wait for `done` (about 22 minutes).
|
||||
2. Read `reconciliation.contested` from the throwaway database. From it build, **outside the repo**:
|
||||
`sheet` (spot number, time, left context, reading x, reading y, random order) and `key` (which
|
||||
of x/y is the base, which is the other engine, which one the reconciler used).
|
||||
3. The speaker fills in the sheet (about 20 to 30 minutes for 30 spots, audio open at the times).
|
||||
The agent does not see the key until the sheet is returned.
|
||||
4. Score with the rules above. Write the aggregate numbers into a dated note next to this one,
|
||||
and amend ADR-034's validation section (kept, or switched off, and the tie rule used).
|
||||
5. Delete the throwaway instance, its database and the sheet, as last time.
|
||||
6. Only if the check passes and the owner wants it in production: enabling reconcile there is
|
||||
the separate D10 step (it needs `iguana/pianissimo` and the reconciler added to the tapir key
|
||||
pin in the infra repo plus the env vars on the deployment, `risk/medium`, owner present).
|
||||
|
||||
## Open and owner-only
|
||||
|
||||
- Confirm or change the rules table above before the run.
|
||||
- The **tool that builds the sheet and key** is the one piece not in the repo (last time it was a
|
||||
one-off script). It can be built test-first on request; until then the agent builds it ad hoc
|
||||
outside the repo.
|
||||
- A Berget adjudicator (D8, tapir#45) is a different question with its own measurement and is
|
||||
not part of this check: this check judges the local setup that ADR-034 describes.
|
||||
@@ -1,7 +1,8 @@
|
||||
# Sprint: audio upload follow-ups (Sweep Sprint, 2026-10)
|
||||
|
||||
Method: Sweep Sprint (phase 1 parallel read-only research per issue: **done**; phase 2 sequential
|
||||
execution, one issue per commit: **not started, waits for the owner's go**). Closing oath below.
|
||||
execution, one issue per commit: **v0.31.1 and v0.32.0 shipped 2026-10-03/04, the owner-held items
|
||||
below remain open**). Closing oath below.
|
||||
|
||||
Milestones: [v0.31.1 - security and robustness](https://git.d-ma.be/mathias/tapir/milestone/10),
|
||||
[v0.32.0 - audio upload follow-ups](https://git.d-ma.be/mathias/tapir/milestone/11).
|
||||
@@ -63,8 +64,8 @@ Release v0.31.1: one adversarial review round over 2-5, then tag, push, observe.
|
||||
| D5 | #38: Berget length measurement needs a throwaway key | Yes, throwaway key | #38 |
|
||||
| D6 | #38: load large models on production iguana | Yes, in a quiet window with the owner present | #38 |
|
||||
| D7 | #46: where the ffmpeg tarball lives | Do nothing now; file an issue | none |
|
||||
| D8 | #45: evaluate Berget as the reconciler | No; stay local-only | #45 |
|
||||
| D9 | Who judges ADR-034's second blind check | The owner; do not reuse the one-off Opus authorisation | ADR-034 |
|
||||
| D8 | #45: evaluate Berget as the reconciler | **Decided 2026-10-04 by the owner: yes, Berget may be used, and each user must be able to keep a recording on local models only** (reverses the recommendation). Opt-out first: #60; #45 stays as the model measurement | #60, #45 |
|
||||
| D9 | Who judges ADR-034's second blind check | The owner; do not reuse the one-off Opus authorisation. Protocol: `docs/research/reconcile-second-check-protocol.md` | ADR-034 |
|
||||
| D10 | When to enable reconcile in production | Needs `iguana/pianissimo` in the key pin and env vars; owner present | ADR-034 |
|
||||
|
||||
## Dropped or reduced, with reasons
|
||||
@@ -86,3 +87,29 @@ Release v0.31.1: one adversarial review round over 2-5, then tag, push, observe.
|
||||
SemVer: v0.31.1 patch (fixes, no new behaviour); v0.32.0 minor (nav link, delete, cloud labelling).
|
||||
Every push to tapir main redeploys the pod and fails any job in flight: push when no upload is
|
||||
running (check the pod log first).
|
||||
|
||||
## Status (2026-10-04)
|
||||
|
||||
**Shipped and deployed:** v0.31.1 (PR #55, merge e037c6b) and v0.32.0 (PR #58, merge 85f4cc3; pod
|
||||
tapir-597dbfccc-ng78q 1/1, 0 restarts, image 85f4cc3). Each went through an adversarial review
|
||||
round by agent with the verdicts recorded on the PR (v0.31.1: BLOCK then APPROVE; v0.32.0: BLOCK,
|
||||
BLOCK, APPROVE, APPROVE; every block was a real defect, fixed test-first).
|
||||
|
||||
**Live evidence:** a foreign-origin `POST` to `/account/delete` and to `/uploads/{id}/delete` is
|
||||
refused (403) on production, a same-origin one reaches the login redirect (302).
|
||||
|
||||
**Not observed live (needs an authenticated session, so it is the owner's look; closed on word or screenshot):**
|
||||
|
||||
- #52, cloud side: a transcript served by the Berget fallback names `berget`; a reconciled all-local
|
||||
one shows no note. The local side was observed on 2026-10-04 (a new clip served by iguana: chip
|
||||
`iguana/…`, no cloud note). The cloud side is carried as an acceptance item in #60.
|
||||
|
||||
**Observed live since:** #37 (owner, 2026-10-04: link, confirm-then-delete, no control while transcribing; closed). #50: at the 2026-10-04 rollout the old pod went `Terminating` then `Completed`
|
||||
(not `Error`), 0 restarts; #50 closed. No job was in flight, so a drained job is still unobserved
|
||||
(a recorded limitation, D2).
|
||||
|
||||
**Still open (owner's call, milestone 11):** #36, #38, #45 (now the Berget adjudicator measurement), #60 (per-recording local-only choice, 4 sub-decisions), #46 remainder, #56 (slow-drip bodies hold
|
||||
a receive slot), #57 (shutdown logging, second signal). Decisions D4-D6, D9, D10 above are unchanged; D8 is decided (above).
|
||||
|
||||
**Deferred and filed from the reviews:** #56, #57, #59 (start-up exits when the identity provider's OIDC
|
||||
discovery returns 503 (seen once at 19:40 on 2026-10-03; the pod restarted itself and recovered)).
|
||||
|
||||
@@ -33,9 +33,9 @@ require (
|
||||
github.com/rogpeppe/go-internal v1.15.0 // indirect
|
||||
github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect
|
||||
go.yaml.in/yaml/v2 v2.4.2 // indirect
|
||||
golang.org/x/sync v0.18.0 // indirect
|
||||
golang.org/x/sync v0.21.0 // indirect
|
||||
golang.org/x/sys v0.41.0 // indirect
|
||||
golang.org/x/text v0.31.0 // indirect
|
||||
golang.org/x/text v0.39.0 // indirect
|
||||
google.golang.org/protobuf v1.36.8 // indirect
|
||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||
)
|
||||
|
||||
@@ -113,12 +113,12 @@ go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
|
||||
go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
|
||||
golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs=
|
||||
golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q=
|
||||
golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I=
|
||||
golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
|
||||
golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM=
|
||||
golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
|
||||
golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k=
|
||||
golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
|
||||
golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM=
|
||||
golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
|
||||
golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus=
|
||||
golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM=
|
||||
golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U=
|
||||
golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno=
|
||||
google.golang.org/protobuf v1.36.8 h1:xHScyCOEuuwZEc6UtSOvPbAT4zRh0xcNRYekJwfqyMc=
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
package oidc_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.d-ma.be/mathias/tapir/internal/web/oidc"
|
||||
)
|
||||
|
||||
func testConfig(issuer string) oidc.Config {
|
||||
return oidc.Config{
|
||||
Issuer: issuer,
|
||||
ClientID: testClientID,
|
||||
ClientSecret: "test-client-secret",
|
||||
RedirectURL: "http://tapir.test/auth/callback",
|
||||
SessionSecret: "test-session-secret-please-change",
|
||||
}
|
||||
}
|
||||
|
||||
// #59: a discovery 503 at start-up is retried instead of failing New.
|
||||
func TestNewRetriesDiscoveryAfter503UntilIssuerAnswers(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 2
|
||||
|
||||
auth, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(5, time.Millisecond))
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, auth)
|
||||
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||
}
|
||||
|
||||
func TestNewReturnsDiscoveryErrorAfterAttemptsExhausted(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 1000
|
||||
|
||||
_, err := oidc.New(context.Background(), testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(3, time.Millisecond))
|
||||
|
||||
require.ErrorContains(t, err, "503")
|
||||
require.Equal(t, int32(3), f.discoveryHits.Load())
|
||||
}
|
||||
|
||||
func TestNewStopsDiscoveryBackoffWhenContextCancelled(t *testing.T) {
|
||||
f := newFakeIssuer(t)
|
||||
f.unavailableFor = 1000
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
|
||||
defer cancel()
|
||||
|
||||
start := time.Now()
|
||||
_, err := oidc.New(ctx, testConfig(f.server.URL),
|
||||
oidc.WithDiscoveryRetry(5, time.Hour))
|
||||
|
||||
require.True(t, errors.Is(err, context.DeadlineExceeded), "err = %v", err)
|
||||
require.Less(t, time.Since(start), 5*time.Second)
|
||||
require.Equal(t, int32(1), f.discoveryHits.Load())
|
||||
}
|
||||
+70
-20
@@ -57,6 +57,10 @@ const (
|
||||
pendingTTL = 10 * time.Minute
|
||||
sessionCookie = "tapir_session"
|
||||
loginPath = "/auth/login"
|
||||
|
||||
// Discovery retry (#59): 5 tries, 2+4+8+16 s back-off ≈ 30 s, then fail.
|
||||
defaultDiscoveryAttempts = 5
|
||||
defaultDiscoveryDelay = 2 * time.Second
|
||||
)
|
||||
|
||||
// DexAuth implements web.Auth against Dex.
|
||||
@@ -70,6 +74,9 @@ type DexAuth struct {
|
||||
sessionTTL time.Duration
|
||||
now func() time.Time
|
||||
insecure bool // tests only: drop the Secure attribute so cookies flow over http
|
||||
|
||||
discoveryAttempts int
|
||||
discoveryDelay time.Duration
|
||||
}
|
||||
|
||||
var _ web.Auth = (*DexAuth)(nil)
|
||||
@@ -96,14 +103,55 @@ func WithSessionTTL(ttl time.Duration) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// WithDiscoveryRetry overrides the discovery attempt count and the initial
|
||||
// back-off delay (doubled after each failed attempt).
|
||||
func WithDiscoveryRetry(attempts int, delay time.Duration) Option {
|
||||
return func(d *DexAuth) {
|
||||
if attempts > 0 {
|
||||
d.discoveryAttempts = attempts
|
||||
}
|
||||
if delay > 0 {
|
||||
d.discoveryDelay = delay
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// WithInsecureCookies drops the Secure cookie attribute. TEST ONLY — it lets
|
||||
// the session cookie travel over plain http; never use it in production.
|
||||
func WithInsecureCookies() Option {
|
||||
return func(d *DexAuth) { d.insecure = true }
|
||||
}
|
||||
|
||||
// New discovers the issuer and returns a configured DexAuth. ctx bounds the
|
||||
// discovery request only.
|
||||
// discover runs OIDC discovery up to attempts times, doubling delay between
|
||||
// tries (#59: an issuer briefly unavailable behind its proxy answered 503 and
|
||||
// serve exited). A cancelled ctx ends the back-off immediately with ctx's error.
|
||||
func discover(ctx context.Context, issuer string, attempts int, delay time.Duration) (*oidc.Provider, error) {
|
||||
var lastErr error
|
||||
for i := range attempts {
|
||||
if i > 0 {
|
||||
t := time.NewTimer(delay)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
t.Stop()
|
||||
return nil, fmt.Errorf("after %d attempts (last: %v): %w", i, lastErr, ctx.Err())
|
||||
case <-t.C:
|
||||
}
|
||||
delay *= 2
|
||||
}
|
||||
p, err := oidc.NewProvider(ctx, issuer)
|
||||
if err == nil {
|
||||
return p, nil
|
||||
}
|
||||
lastErr = err
|
||||
if ctx.Err() != nil {
|
||||
return nil, fmt.Errorf("after %d attempts: %w", i+1, ctx.Err())
|
||||
}
|
||||
}
|
||||
return nil, fmt.Errorf("after %d attempts: %w", attempts, lastErr)
|
||||
}
|
||||
|
||||
// New discovers the issuer and returns a configured DexAuth. ctx bounds discovery,
|
||||
// including its retry back-off.
|
||||
func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||
for name, val := range map[string]string{
|
||||
"issuer": cfg.Issuer,
|
||||
@@ -117,29 +165,31 @@ func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) {
|
||||
}
|
||||
}
|
||||
|
||||
provider, err := oidc.NewProvider(ctx, cfg.Issuer)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("oidc: discover issuer: %w", err)
|
||||
}
|
||||
|
||||
d := &DexAuth{
|
||||
cfg: cfg,
|
||||
verifier: provider.Verifier(&oidc.Config{ClientID: cfg.ClientID}),
|
||||
oauth: &oauth2.Config{
|
||||
ClientID: cfg.ClientID,
|
||||
ClientSecret: cfg.ClientSecret,
|
||||
Endpoint: provider.Endpoint(),
|
||||
RedirectURL: cfg.RedirectURL,
|
||||
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
|
||||
},
|
||||
pending: newPendingStore(),
|
||||
secret: []byte(cfg.SessionSecret),
|
||||
sessionTTL: defaultSessionTTL,
|
||||
now: time.Now,
|
||||
cfg: cfg,
|
||||
pending: newPendingStore(),
|
||||
secret: []byte(cfg.SessionSecret),
|
||||
sessionTTL: defaultSessionTTL,
|
||||
now: time.Now,
|
||||
discoveryAttempts: defaultDiscoveryAttempts,
|
||||
discoveryDelay: defaultDiscoveryDelay,
|
||||
}
|
||||
for _, o := range opts {
|
||||
o(d)
|
||||
}
|
||||
|
||||
provider, err := discover(ctx, cfg.Issuer, d.discoveryAttempts, d.discoveryDelay)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("oidc: discover issuer: %w", err)
|
||||
}
|
||||
d.verifier = provider.Verifier(&oidc.Config{ClientID: cfg.ClientID})
|
||||
d.oauth = &oauth2.Config{
|
||||
ClientID: cfg.ClientID,
|
||||
ClientSecret: cfg.ClientSecret,
|
||||
Endpoint: provider.Endpoint(),
|
||||
RedirectURL: cfg.RedirectURL,
|
||||
Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
|
||||
}
|
||||
return d, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -34,6 +35,11 @@ type fakeIssuer struct {
|
||||
sub string
|
||||
email string
|
||||
nonce string
|
||||
|
||||
// discoveryHits counts requests to the discovery document; the first
|
||||
// unavailableFor of them answer 503 (issuer briefly down behind its proxy).
|
||||
discoveryHits atomic.Int32
|
||||
unavailableFor int32
|
||||
}
|
||||
|
||||
const testKID = "test-key"
|
||||
@@ -47,6 +53,10 @@ func newFakeIssuer(t *testing.T) *fakeIssuer {
|
||||
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) {
|
||||
if f.discoveryHits.Add(1) <= f.unavailableFor {
|
||||
http.Error(w, "no available server", http.StatusServiceUnavailable)
|
||||
return
|
||||
}
|
||||
writeJSON(t, w, map[string]any{
|
||||
"issuer": f.server.URL,
|
||||
"authorization_endpoint": f.server.URL + "/authorize",
|
||||
|
||||
Reference in New Issue
Block a user