From 0deddaa58427f628f0afefa68bc6bedd6149275e Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:36:33 -0400 Subject: [PATCH 1/4] test: drain learned-quirk writers before simulating a restart --- internal/provider/quirks_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/provider/quirks_test.go b/internal/provider/quirks_test.go index 1818b6b390..e7ffdfd7c3 100644 --- a/internal/provider/quirks_test.go +++ b/internal/provider/quirks_test.go @@ -143,8 +143,14 @@ func TestLearnedQuirksSurviveTheProcessThatLearnedThem(t *testing.T) { t.Fatalf("memo = %s, want the model that refused", raw) } - // A fresh process reads it and never sends the refused shape at all. + // A fresh process reads it and never sends the refused shape at all. The + // synchronous save above does not join the separately scheduled writer; + // drain that writer before discarding the old process's memory, or it can + // overwrite the verified file with the deliberately emptied map. + quirks.settle() + quirks.mutex.Lock() quirks.mandatory = map[string]time.Time{} + quirks.mutex.Unlock() LoadQuirks(dir) if !ReasoningMandatory("strict/persisted") { t.Fatal("a memo on disk must be believed at startup") @@ -176,6 +182,9 @@ func TestASilentlyIgnoredDisableSurvivesTheProcessThatLearnedIt(t *testing.T) { t.Fatalf("memo = %s, want the model that ignored the disable", raw) } + // The old process's scheduled save must finish before its memory is + // discarded, for the same reason as the mandatory-reasoning restart above. + quirks.settle() quirks.mutex.Lock() quirks.disableIgnored = map[string]time.Time{} quirks.mutex.Unlock() From 937e7a3c0e7512a664074d2fd3f27d419e80210a Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:41:50 -0400 Subject: [PATCH 2/4] fix: serialize learned-provider memo snapshots through replacement --- PERF.md | 13 ++++++ internal/manual/chat/models-and-cost.md | 13 ++++++ internal/provider/quirks.go | 8 ++++ internal/provider/quirks_persistence_test.go | 43 ++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 internal/provider/quirks_persistence_test.go diff --git a/PERF.md b/PERF.md index 9afc0805e9..3dff833578 100644 --- a/PERF.md +++ b/PERF.md @@ -2929,3 +2929,16 @@ receipts are atomically replaced and synced before committing deletion. Startup walks only the two saved-conversation directory levels, without following symlinked buckets. `internal/session/deletion_revision_test.go` drives completion and timeout with held channels and a zero test grace rather than sleeping for the production deadline. + +## Learned provider-fact persistence + +Within one process, `quirksStore.saveMutex` serializes saves from before the +snapshot through atomic file replacement, so an older snapshot cannot replace +newer learned facts. The memo-reader mutex is still held only while copying +maps, never across file I/O. Requests still schedule persistence asynchronously; +waiting writers do not move a disk wait onto the request path. The existing +unique temporary files and atomic rename remain unchanged. This is an in-process +ordering guarantee, not a merge of caches written by separate processes. + +Restart fixtures drain their owned scheduled writes before discarding process +memory. No timeout, response limit or request/retry budget changes. diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index ce26e86d46..23347eb2ff 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -2769,6 +2769,19 @@ An endpoint's explicit total limit is remembered by base URL, model and provider prompt's length is **not** a total context limit. Old model-wide `served_window` guesses are no longer used to size requests. +## Learned model request facts after restarting + +codeaf remembers request shapes an endpoint refused, such as a disabled reasoning +pass or a cache breakpoint, in `model-quirks.json`. Saves made by one running +process preserve its latest learned facts when the writes finish, even when +several requests learn at once. Saving stays in the background; the next request +uses the fact already held in memory. + +A missing, unreadable or unwritable memo is a cache miss: codeaf may learn the +endpoint's refusal again. Concurrent separate processes sharing the same profile +still replace the same cache file; their learned sets are not merged. Endpoint +context limits retain their existing 30-minute lifetime. + ## How much room is left for the answer — the output cap, max_tokens and the safety margin When no output cap was requested, the total output allowance is the smaller of the answer diff --git a/internal/provider/quirks.go b/internal/provider/quirks.go index adb07f0d2d..474814e213 100644 --- a/internal/provider/quirks.go +++ b/internal/provider/quirks.go @@ -40,6 +40,10 @@ const quirksFile = "model-quirks.json" type quirksStore struct { mutex sync.Mutex path string + // saveMutex orders each snapshot with its replacement on disk. Without + // it, an older snapshot can finish after a newer one and erase a fact + // already saved by this process. Readers never take this writer lock. + saveMutex sync.Mutex // mandatory is the learned set, keyed by normalized model. mandatory map[string]time.Time // disableIgnored is the answer-side twin of mandatory: models whose endpoint @@ -484,6 +488,10 @@ func (q *quirksStore) settle() { q.writes.Wait() } // fact, and a failure is silent: a cache that could not be written is a cache // that will be rebuilt, not an error the run should carry. func (q *quirksStore) save() { + // Take the writer lock before snapshotting, not only before renaming: a + // snapshot copied while another writer owns the file can already be stale. + q.saveMutex.Lock() + defer q.saveMutex.Unlock() path, wire := q.snapshot() if path == "" { return diff --git a/internal/provider/quirks_persistence_test.go b/internal/provider/quirks_persistence_test.go new file mode 100644 index 0000000000..936788115c --- /dev/null +++ b/internal/provider/quirks_persistence_test.go @@ -0,0 +1,43 @@ +package provider + +import ( + "fmt" + "path/filepath" + "sync" + "testing" + "time" +) + +// Concurrent request completions teach one process different facts. Once its +// scheduled writes have finished, a fresh process must recover every fact; +// atomic individual files alone do not establish that ordering. +func TestConcurrentQuirkSavesRetainEveryLearnedFactAtRestart(t *testing.T) { + store := &quirksStore{mandatory: map[string]time.Time{}} + path := filepath.Join(t.TempDir(), quirksFile) + store.load(path) + const facts = 32 + start := make(chan struct{}) + var learners sync.WaitGroup + for i := 0; i < facts; i++ { + learners.Add(1) + go func(i int) { + defer learners.Done() + <-start + if store.note(fmt.Sprintf("concurrent/model-%d", i), time.Now()) { + store.persist() + } + }(i) + } + close(start) + learners.Wait() + store.settle() + + fresh := &quirksStore{mandatory: map[string]time.Time{}} + fresh.load(path) + for i := 0; i < facts; i++ { + model := fmt.Sprintf("concurrent/model-%d", i) + if !fresh.knows(model) { + t.Errorf("a completed save lost %s at restart", model) + } + } +} From 430d3508e4e33086e34eaabf2639ebe564f5335d Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:45:59 -0400 Subject: [PATCH 3/4] changelog: record the restart fixture ownership repair --- docs/changes/unreleased/1823-quirks-restart-fixtures.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 docs/changes/unreleased/1823-quirks-restart-fixtures.md diff --git a/docs/changes/unreleased/1823-quirks-restart-fixtures.md b/docs/changes/unreleased/1823-quirks-restart-fixtures.md new file mode 100644 index 0000000000..69fae3d93d --- /dev/null +++ b/docs/changes/unreleased/1823-quirks-restart-fixtures.md @@ -0,0 +1,8 @@ +--- +kind: internal +title: provider restart fixtures drain learned-memo writers before resetting memory +pr: 1823 +surface: [build] +invalidates: + - "The restart fixtures forced a synchronous save but left a scheduled save running while they discarded process memory. They now drain the owned writer before resetting memory and reloading the memo, preserving the original startup and request-count assertions." +--- From a4804de6ce0d1d1e674e86c27f54f30c725eae57 Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:14:42 -0400 Subject: [PATCH 4/4] changelog: record ordered provider memo persistence --- .../unreleased/1824-ordered-provider-memo-saves.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 docs/changes/unreleased/1824-ordered-provider-memo-saves.md diff --git a/docs/changes/unreleased/1824-ordered-provider-memo-saves.md b/docs/changes/unreleased/1824-ordered-provider-memo-saves.md new file mode 100644 index 0000000000..4dacd5d9e5 --- /dev/null +++ b/docs/changes/unreleased/1824-ordered-provider-memo-saves.md @@ -0,0 +1,8 @@ +--- +kind: fixed +title: concurrent provider-memo saves preserve every learned fact at restart +pr: 1824 +surface: [engine, chat] +invalidates: + - "Atomic provider-memo replacement kept valid JSON but could let an older in-process snapshot overwrite a newer learned fact. Saves now serialize from before snapshot through replacement, while requests still persist asynchronously; separate processes do not merge their caches." +---