Skip to content

fix: #498 weak-overflow learning + #499 P1a affinity persistence + P1b cache-collapse warn - #504

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-03_fix-498-weak-overflow
Sep 4, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-03_fix-498-weak-overflow

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Closes #498. Implements the bili-side half of the #499 fix family (paired with acp-kernel PR acp-kernel#196).

1. #498 — learn from weak overflow signals

Problem: non-4xx overflow evidence (stream truncation without a completion event, upstream HTTP errors on oversized requests) never taught the proxy anything, so an oversized session could fail forever without a strong 400 signal.

Fix — new src/weak-overflow.ts (noteWeakOverflow):

  • Fires on: loop/core.ts truncation without done-event (both adapters) + UpstreamHttpError catch, and both plugin passthrough pipes (chat: [DONE]/message_stop = terminal; responses: done-family events = terminal).
  • Qualifies at input/window ≥ 90% where window = min(learned, effective); 3 events in 15 min confirm.
  • On confirm: learns a shrink-only conservative window (per-model map when model known), arms the emergency shrink (lastInputTokens = max(...)), marks dirty, logs the decision.
  • Strong 4xx learning (existing !upstream.ok path) remains the primary teacher; this is the fallback.

2. #499 P1a — prefix-affinity persistence

Problem: prefix-affinity was pure in-memory (LRU 256, 7-day TTL) → a proxy restart re-forked every anonymous session (#351: pfa-8588 one-shot ingest of 1746 msgs / 458 662 tok, raw 458K resend, cache hit 0%).

Fix — src/prefix-affinity.ts gains exportSnapshot()/importSnapshot(); new src/affinity-persist.ts: snapshot to ${XDG_STATE_HOME}/billion-context/prefix-affinity.json ({version:1, entries}), atomic tmp+rename, 5 s debounced write after each note(), flush in finishShutdown(), hydrate on startup. Import validates entry shape, drops TTL-expired chains, re-imposes LRU/item caps.

3. #499 P1b — prompt-cache collapse warn (diagnostic)

Problem: cache hit 0% on the mirror is a symptom of re-forking (or upstream KV drop); nothing surfaced it.

Fix — new src/cache-warn.ts (warnCacheCollapse): after a session has seen ≥50% hit on a ≥8k-token request, 5 consecutive large requests at ≤10% hit log ONE warning pointing at the likely causes (restart re-fork / upstream KV drop) and what to check (upstream session id, compress.modelContextLimit). Hooks: plugin.ts applyUsageSample + loop/core.ts recordUsage. Diagnostics only — no behavior change.

Tests

20 new tests across 4 files: tests/weak-overflow.test.ts (7), tests/plugin-truncation-weak-overflow.test.ts (5), tests/cache-warn.test.ts (4), tests/affinity-persist.test.ts (4).

Pre-flight: npm run typecheck ✅ · npm test 1018/1020 (2 known permanent env fails in plugin-agent.test.ts, unrelated) ✅ · npm run build ✅

Out of scope (deliberate)

…on cache collapse

- #498: noteWeakOverflow fires on stream truncation / upstream errors at
  >=90% window usage; 3 events in 15min learn a shrink-only conservative
  window and arm the emergency shrink (strong 4xx signals remain the
  primary teacher). Hooks in loop/core.ts (both adapters' truncation and
  UpstreamHttpError) and both plugin passthrough pipes (terminal-event
  detection per wire format).
- #499 P1a: prefix-affinity snapshot persisted to
  ${XDG_STATE_HOME}/billion-context/prefix-affinity.json (atomic write,
  5s debounce, flush on shutdown, hydrate on startup) so a proxy restart
  no longer orphans anonymous sessions.
- #499 P1b: cache-warn detects prompt-cache collapse (5 consecutive
  large requests at ~0% hit after earlier >=50% hits) and logs a
  diagnostic pointer at #499 causes.

Tests: 20 new (weak-overflow, plugin-truncation, cache-warn,
affinity-persist). Full suite 1018/1020 (2 known env fails in
plugin-agent.test.ts).
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-03_fix-498-weak-overflow (64d605f)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-504

Each push to this PR publishes a new version under the pr-504 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr504.tgz
npm install -g package

This comment is automatically updated on each push.

ranxianglei pushed a commit that referenced this pull request Sep 3, 2026
…erequisite)

migrateLegacyIds() re-keys sessions whose stored label differs from their id
(#286 cleanup), deleting all but the newest sibling per label. Anonymous
prefix-affinity sessions (#309) all carry the shared display label
"prefix-affinity" != their pfa- id, so every boot re-keyed them all to the
single id "prefix-affinity" and deleted every sibling but the newest — silent
loss of saved compression state for anonymous clients on every restart.

Exclude pfa- ids from the migration. True legacy derived-hash sessions are
still re-keyed as before (regression-tested alongside the guard).

Prerequisite for #504 P1a affinity-snapshot persistence: without it, routing
restored across restarts lands on session files that no longer exist, so the
folded view cannot be rebuilt.
@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 PR #504 review — pre-flight re-run on 2026-09-03_fix-498-weak-overflow @ 64d605f. Code matches the description; one blocking coordination point (P1a needs #502's guard to actually work end-to-end), plus minor nits below.

Pre-flight (verified locally)

Check Result
npm run typecheck ✅ clean
npm test 1019/1020 — single failure is the known env-dependent launcher.test.ts codex-PATH test (/usr/bin/codex vs codex; unrelated to this diff). All 20 new tests pass; plugin-agent.test.ts 27/27 here
npm run build ✅ self-contained dist/index.js
E2E ⚠️ not run here (needs a local Responses upstream). This touches src/loop/core.ts + both plugin pipes → per AGENTS.md, please run the 4-phase e2e-codex suite before merging

Blocking: P1a alone does not fix #499 — needs #502's migration guard

This PR persists the routing table (prefix-affinity.json, hydrated at server.ts:390), but on every startup migrateLegacyIds() (src/persist.ts:211) still destroys all pfa-* session state files: anonymous sessions carry label "prefix-affinity" ≠ id pfa-xxxx, violating the label === id invariant checked at persist.ts:224 → all pfa files get re-keyed into one id and siblings deleted, before hydration runs. Net effect after restart: replay resolves to a pfa-xxxx id whose persisted blocks no longer exist → fresh empty session → raw resend continues. The exact #351 symptom (#499's motivating event) persists.

The fix already exists in open PR #502 (skip pfa- ids in migration + regression test). Two options:

  1. Fold fix: stop legacy-id migration destroying anonymous pfa sessions (#499 prerequisite) #502's guard + regression test into this PR, then close fix: stop legacy-id migration destroying anonymous pfa sessions (#499 prerequisite) #502 as superseded (avoids two parallel persistence mechanisms — per-session-metadata reseed vs snapshot file).
  2. Merge fix: stop legacy-id migration destroying anonymous pfa sessions (#499 prerequisite) #502 first, then this one.

Your call — flagging because P1a's stated goal ("restart resumes the same compressed context") is unreachable without whichever option ships.

Minor (non-blocking)

  • src/loop/core.ts:531 — the truncation path (if (!sawDone || truncatedDone)) has no signal?.aborted guard. If the client aborts while the generator is executing between yields (inner stream loop breaks at line 279, no pending yield), control falls through to line 531 and records a false weak-overflow signal. Common case is safe (consumer break → generator .return() unwinds before reaching 531), but it contradicts the 高 usage 下的非 400 失败(流截断/超时)无任何 overflow 兜底 —— 会话直接死 #498 floor-3 design ("客户端主动断开(IDE cancel)不计入"). One-line fix: add && !signal?.aborted. (The plugin pipes do this right via res.destroyed || res.writableEnded.)
  • src/plugin.ts:897 — inner if (type === completed || failed || incomplete) duplicates its outer condition verbatim; harmless redundancy.
  • resetWeakOverflow(sessionId) is exported but only called from tests — never wired to session GC/removal. Bounded leak (≤512 entries) so acceptable for v1, just noting.
  • Multi-instance: the snapshot file lives under the shared XDG state dir, so two concurrently-running bili instances clobber each other's affinity snapshots. Degrades to today's behavior (no corruption — chains are content-derived); fine for v1, worth a README footnote.

Verified correct

  • Weak-overflow learning writes the same metadata fields the strong-signal path uses (learnedContextLimit scalar / learnedContextLimits[model] map) — read/write sites consistent, so nudge/preflight consume them identically.
  • Hooks cover both compression modes: proxy mode via recordUsage + the re-request UpstreamHttpError catch in loop/core.ts; plugin mode via both passthrough pipes with proper terminal detection ([DONE]/message_stop; done-family events). Strong 4xx remains the primary teacher as designed.
  • importSnapshot validates entry shape, drops TTL-expired chains, re-imposes LRU (256) / item (128) caps; writes are atomic (tmp+rename) with an unref'd 5 s debounce and a flush on shutdown.
  • cache-warn is diagnostics-only (no state/behavior change), fires exactly once per session, and is hooked in both modes.

Verdict: implementation is solid and faithful to the spec. Mergeable once (a) the #502-guard question above is resolved and (b) the e2e suite passes. I can't merge PRs (human-only) — happy to apply the signal?.aborted one-liner or fold in #502's guard if you say go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

高 usage 下的非 400 失败(流截断/超时)无任何 overflow 兜底 —— 会话直接死

1 participant